When a struct credential expires, credential_fill() clears c->password so that clients don't try to use it later. However, a struct cred that uses an alternate authtype won't have a password, but might have a credential stored in c->credential. This is a problem, for example, when an OAuth2 bearer token is used. In the system I'm using, the OAuth2 configuration generates and caches a bearer token that is valid for an hour. After the token expires, git needs to call back into the credential helper to use a stored refresh token to get a new bearer token. But if c->credential is still non-NULL, git will instead try to use the expired token and fail with an error: fatal: Authentication failed for 'https://<oauth2-enabled-server>/repository' And on the server: [auth_openidc:error] [client <ip>:34012] oidc_proto_validate_exp: "exp" validation failure (1717522989): JWT expired 224 seconds ago Fix this by clearing both c->password and c->credential for an expired struct credential. While we're at it, use credential_clear_secrets() wherever both c->password and c->credential are being cleared, and use the full credential_clear() in credential_reject() after the credential has been erased from all of the helpers. v2: Unify secret clearing into credential_clear_secrets(), use credential_clear() in credential_reject(), add a comment about why we can't use credential_clear() in credential_fill(). Signed-off-by: Aaron Plattner <aplattner@xxxxxxxxxx> --- credential.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/credential.c b/credential.c index 758528b291..72c6f46b02 100644 --- a/credential.c +++ b/credential.c @@ -20,12 +20,11 @@ void credential_init(struct credential *c) void credential_clear(struct credential *c) { + credential_clear_secrets(c); free(c->protocol); free(c->host); free(c->path); free(c->username); - free(c->password); - free(c->credential); free(c->oauth_refresh_token); free(c->authtype); string_list_clear(&c->helpers, 0); @@ -479,9 +478,14 @@ void credential_fill(struct credential *c, int all_capabilities) for (i = 0; i < c->helpers.nr; i++) { credential_do(c, c->helpers.items[i].string, "get"); + if (c->password_expiry_utc < time(NULL)) { - /* Discard expired password */ - FREE_AND_NULL(c->password); + /* + * Don't use credential_clear() here: callers such as + * cmd_credential() expect to still be able to call + * credential_write() on a struct credential whose secrets have expired. + */ + credential_clear_secrets(c); /* Reset expiry to maintain consistency */ c->password_expiry_utc = TIME_MAX; } @@ -528,12 +532,7 @@ void credential_reject(struct credential *c) for (i = 0; i < c->helpers.nr; i++) credential_do(c, c->helpers.items[i].string, "erase"); - FREE_AND_NULL(c->username); - FREE_AND_NULL(c->password); - FREE_AND_NULL(c->credential); - FREE_AND_NULL(c->oauth_refresh_token); - c->password_expiry_utc = TIME_MAX; - c->approved = 0; + credential_clear(c); } static int check_url_component(const char *url, int quiet, -- 2.45.2.410.gb47f57dd90.dirty