Re: [PATCH] credential: do not store credentials received from helpers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Shawn Pearce <spearce@xxxxxxxxxxx> writes:

> On Fri, Apr 6, 2012 at 20:34, Jeff King <peff@xxxxxxxx> wrote:
>>  2. If you use a time-based storage helper like
>>     "git-credential-cache", every time you run a git
>>     command which uses the credential, it will also
>>     re-insert the credential after use, freshening the
>>     cache timestamp. So the cache will eventually expire N
>>     time units after the last _use_, not after the time the
>>     user actually typed the password. This is probably not
>>     what most users expect or want (and if they do, we
>>     should do it explicitly by providing an option to
>>     refresh the timestamp on use).
>
> So if I use the cache helper, and its set to expire at the default of
> 15 minutes, I have to type my password in every 15 minutes, even if I
> am doing a Git operation roughly every 8 minutes during a work day?

I think what Peff meant was to set the expiration time of the cache helper
a lot longer (say 20 hours, as opposed to 15 minutes) but once it learns a
credential material from your helper, leave the timer running without
resetting it every time the user uses the credential (your 8 minutes).

So I do not see a fundamental problem in this part.

>> We can solve this by marking a credential that comes from a
>> helper, so we don't bother feeding it back to the helpers.
>> The credential struct already has an "approved" flag so
>> that we try to store it only once, rather than for each
>> successful http request. We can use the same flag to
>> "pre-approve" a credential which comes from a helper, and
>> enver try to store it at all.
>
> This breaks one of my credential helpers.
>
> I have a helper that generates a password by asking a remote system to
> generate a short lived password based on other authentication systems
> that I can't describe. Once I have that password, its good for $X
> time.

Well, $X is something your helper knows but other helpers don't, so...

> The helper just dumps it out to Git, and Git turns around and stores
> it into the cache for me. This means later requests will keep that
> credential in the cache, and avoid making that remote system call
> every time I do a Git network command. I guess I now need to change my
> helper to cache git credential-cache itself and store the password
> into the cache if it wants to use the cache?

... logically the knowledge of cache expiration time belongs to your
helper, but "after the first use, it is OK to keep using it for N hours"
is so common that the two-helper workaround is very tempting.

I am afraid that "do not trigger the cache helper" might be throwing the
baby with bathwater to solve the real problem the patch tries to address,
which is:

Peff>   2. If you use a time-based storage helper like
Peff>      "git-credential-cache", every time you run a git
Peff>      command which uses the credential, it will also
Peff>      re-insert the credential after use, freshening the
Peff>      cache timestamp. So the cache will eventually expire N
Peff>      time units after the last _use_, not after the time the
Peff>      user actually typed the password. This is probably not

Shouldn't the memory cache based helper already have enough clue to tell
when a new entry is first inserted vs when the existing entry it supplied
came back from the network layer after use?  If there is not enough clue
with the current network-layer-to-helper protocol, then wouldn't it be a
better approach to add that, so that the memory cache helper can make more
intelligent management of its timer?

Once that is fixed, I would imagine that you can tell your users to use
two helpers (yours and generic caching one) and configure them so that (1)
the caching one is asked first and then fall back to ask yours, and (2)
the expiration time of the caching one is set close to $X.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]