git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* git-credential-cache inner workings
@ 2021-05-10  6:33 Thijs Cramer
  2021-05-10 21:25 ` Jeff King
  0 siblings, 1 reply; 2+ messages in thread
From: Thijs Cramer @ 2021-05-10  6:33 UTC (permalink / raw)
  To: git

During debugging of a nasty recurring GIT code 128 error in our CI/CD
environment, we've found some design choices that have raised some
questions about the inner workings of the credential cache in GIT.
First, a little sketch of the situation.

We are using GitHub Enterprise Cloud on Jenkins using the Github
Branch Source Plugin.
This plugin uses "GitHub Apps" (Service Accounts if you will), which
use Application (Installation) Tokens to authenticate with GitHub. The
tokens are used as regular passwords in this case.

When a Jenkins job starts, it checks out the code from GitHub using
the GIT command line, passing it a token with a lifetime between 45
and 60 minutes remaining (tokens have a max lifetime of 60m). This
token is passed via the GIT_ASKPASS variable to give the commandline
CI/CD job the ability to use the token for subsequent GIT actions
locally during the job.

We are seeing GIT code 128's in rare cases when jobs follow each other
up at a fairly high tempo. The reason is:
https://github.com/git/git/blob/master/http.c#L1637 . This line
'approves' a credential that gives a successful result. This means
that Tokens that have been cached in the git-credential-cache-daemon
will have it's timeout refreshed even though they might expire
shortly.
The credential cache timeout is a user-input timeout, and not a token
timeout, and this structure collides with the functional lifetime of a
token.

There are several solutions to circumvent this issue:
1) We could ask the Plugin that manages the tokens to support managing
the Credential Cache as well. This makes it possible for the plugin to
actively manage tokens and revoke tokens from the cache that have
expired. Downside is that making plugins aware of the cache in git,
might not be a good idea, and there might even be cases where caching
is disabled completely, which would still require the ASKPASS method.
2) We could add an option to the GIT client that has the same
functionality as the ASKPASS variable, but one that takes precedence
before the cache. This way, one can preseed the GIT client with
credentials that will *always* be used, even if there is a cache that
has a credential for the given URL.
3) We could alter the cache daemon (or build a new one) that has more
elaborate options regarding it's timeout. Perhaps a daemon option like
"--keep-timeout-on-approve".
This would mean that if a credential is approved for a second time the
timeout is not refreshed.
Currently the daemon just removes any old credentials from it's cache
and adds the new one without questioning the intention of the user. We
could add a check that tests if the incoming credential is equal to
the currently cached credential, and skip updating the expiration date
if they are the same. (With an exception when receiving a new
password, then we should forcefully update). This means that the
--timeout option from the daemon will become a 'hard' timeout, instead
of a user-action timeout. Of course this should be done with a command
line option on the daemon to not change the default behaviour.

My C is a bit rusty, but I can help with adding one of the features, I
would just like to know some of your thoughts on this issue.

Thanks a lot,

- Thijs

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: git-credential-cache inner workings
  2021-05-10  6:33 git-credential-cache inner workings Thijs Cramer
@ 2021-05-10 21:25 ` Jeff King
  0 siblings, 0 replies; 2+ messages in thread
From: Jeff King @ 2021-05-10 21:25 UTC (permalink / raw)
  To: Thijs Cramer; +Cc: git

On Mon, May 10, 2021 at 08:33:38AM +0200, Thijs Cramer wrote:

> We are seeing GIT code 128's in rare cases when jobs follow each other
> up at a fairly high tempo. The reason is:
> https://github.com/git/git/blob/master/http.c#L1637 . This line
> 'approves' a credential that gives a successful result. This means
> that Tokens that have been cached in the git-credential-cache-daemon
> will have it's timeout refreshed even though they might expire
> shortly.
> The credential cache timeout is a user-input timeout, and not a token
> timeout, and this structure collides with the functional lifetime of a
> token.

Right. This was discussed a few months ago in this thread:

  https://lore.kernel.org/git/CAP8UukiL0niGSm3o7uYNBzL3FP-UEgfOuq-Tu=fksWJkerT5fg@mail.gmail.com/

which also links back to a much older discussion of various solutions.

> There are several solutions to circumvent this issue:
> 1) We could ask the Plugin that manages the tokens to support managing
> the Credential Cache as well. This makes it possible for the plugin to
> actively manage tokens and revoke tokens from the cache that have
> expired. Downside is that making plugins aware of the cache in git,
> might not be a good idea, and there might even be cases where caching
> is disabled completely, which would still require the ASKPASS method.

Yes, I agree it's much nicer if the plugin doesn't have to fiddle with
(possibly) configured helpers itself.

> 2) We could add an option to the GIT client that has the same
> functionality as the ASKPASS variable, but one that takes precedence
> before the cache. This way, one can preseed the GIT client with
> credentials that will *always* be used, even if there is a cache that
> has a credential for the given URL.

You can do that already by setting up a helper that provides the
credentials; Git will stop asking as soon as it has an answer. Inserting
a helper at the front of the list is tricky, but you can provide an
empty helper to reset the list. E.g.:

  git \
    -c credential.helper = \
    -c credential.helper = your-real-helper \
    fetch ...

would work.

> 3) We could alter the cache daemon (or build a new one) that has more
> elaborate options regarding it's timeout. Perhaps a daemon option like
> "--keep-timeout-on-approve".
> This would mean that if a credential is approved for a second time the
> timeout is not refreshed.

Yes, that would mostly work. There is a race condition where the cache
times out _during_ a client request. So the cache daemon sees:

  1. Client asks for credential; we provide it.

  2. Cache times out; credential is dropped.

  3. Client provides credential to store.

Then it appears "new". We could keep a tombstone entry for the timed-out
credential, but part of the point of timing out is that we drop the
secret data. You could perhaps do something clever with one-way hashes,
but my preference is to keep things as simple as possible.

> Currently the daemon just removes any old credentials from it's cache
> and adds the new one without questioning the intention of the user. We
> could add a check that tests if the incoming credential is equal to
> the currently cached credential, and skip updating the expiration date
> if they are the same. (With an exception when receiving a new
> password, then we should forcefully update). This means that the
> --timeout option from the daemon will become a 'hard' timeout, instead
> of a user-action timeout. Of course this should be done with a command
> line option on the daemon to not change the default behaviour.

The variant discussed in the thread I linked is to ask Git not to ask to
store credentials which came from a helper in the first place. I think
that solves the problem more directly, and gives the cache helper more
predictable semantics. But some people expressed a preference for the
current behavior in that thread. We could perhaps make it optional,
though (credential.storeFromHelpers or similar).

-Peff

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-05-10 21:25 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-10  6:33 git-credential-cache inner workings Thijs Cramer
2021-05-10 21:25 ` Jeff King

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.version-control.git
	nntp://ie5yzdi7fg72h7s4sdcztq5evakq23rdt33mfyfcddc5u3ndnw24ogqd.onion/inbox.comp.version-control.git
	nntp://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git