@matheusbsbs / matheus-bagliano / commits / 27db485c34

credential: clear expired c->credential, unify secret clearing

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. Update comments in credential.h to mention the new struct fields. Signed-off-by: Aaron Plattner <aplattner@nvidia.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Aaron Plattner committed Jun 6, 2024 at 11:35 UTC 27db485c34392df4fe6fbaf57a43f15bd7bf4a36

2 files changed +28 -22

credential.c

+10 -6

@@ -20,12 +20,11 @@ void credential_init(struct credential *c)

21 void credential_clear(struct credential *c)

22 {

23 + credential_clear_secrets(c);

24 free(c->protocol);

25 free(c->host);

26 free(c->path);

27 free(c->username);

27 - free(c->password);

28 - free(c->credential);

28 free(c->oauth_refresh_token);

29 free(c->authtype);

30 string_list_clear(&c->helpers, 0);

@@ -479,9 +478,15 @@ void credential_fill(struct credential *c, int all_capabilities)

478

479 for (i = 0; i < c->helpers.nr; i++) {

480 credential_do(c, c->helpers.items[i].string, "get");

481 +

482 if (c->password_expiry_utc < time(NULL)) {

483 - /* Discard expired password */

484 - FREE_AND_NULL(c->password);

483 + /*

484 + * Don't use credential_clear() here: callers such as

485 + * cmd_credential() expect to still be able to call

486 + * credential_write() on a struct credential whose

487 + * secrets have expired.

488 + */

489 + credential_clear_secrets(c);

490 /* Reset expiry to maintain consistency */

491 c->password_expiry_utc = TIME_MAX;

492 }

@@ -528,9 +533,8 @@ void credential_reject(struct credential *c)

533 for (i = 0; i < c->helpers.nr; i++)

534 credential_do(c, c->helpers.items[i].string, "erase");

535

536 + credential_clear_secrets(c);

537 FREE_AND_NULL(c->username);

532 - FREE_AND_NULL(c->password);

533 - FREE_AND_NULL(c->credential);

538 FREE_AND_NULL(c->oauth_refresh_token);

539 c->password_expiry_utc = TIME_MAX;

540 c->approved = 0;

credential.h

+18 -16

@@ -5,8 +5,8 @@

5 #include "strvec.h"

7 /**

8 - * The credentials API provides an abstracted way of gathering username and

9 - * password credentials from the user.

8 + * The credentials API provides an abstracted way of gathering

9 + * authentication credentials from the user.

10 *

11 * Typical setup

12 * -------------

@@ -116,11 +116,12 @@ struct credential_capability {

116 };

117

118 /**

119 - * This struct represents a single username/password combination

120 - * along with any associated context. All string fields should be

121 - * heap-allocated (or NULL if they are not known or not applicable).

122 - * The meaning of the individual context fields is the same as

123 - * their counterparts in the helper protocol.

119 + * This struct represents a single login credential (typically a

120 + * username/password combination) along with any associated

121 + * context. All string fields should be heap-allocated (or NULL if

122 + * they are not known or not applicable). The meaning of the

123 + * individual context fields is the same as their counterparts in

124 + * the helper protocol.

125 *

126 * This struct should always be initialized with `CREDENTIAL_INIT` or

127 * `credential_init`.

@@ -207,11 +208,12 @@ void credential_clear(struct credential *);

208

209 /**

210 * Instruct the credential subsystem to fill the username and

210 - * password fields of the passed credential struct by first

211 - * consulting helpers, then asking the user. After this function

212 - * returns, the username and password fields of the credential are

213 - * guaranteed to be non-NULL. If an error occurs, the function will

214 - * die().

211 + * password (or authtype and credential) fields of the passed

212 + * credential struct by first consulting helpers, then asking the

213 + * user. After this function returns, either the username and

214 + * password fields or the credential field of the credential are

215 + * guaranteed to be non-NULL. If an error occurs, the function

216 + * will die().

217 *

218 * If all_capabilities is set, this is an internal user that is prepared

219 * to deal with all known capabilities, and we should advertise that fact.

@@ -232,10 +234,10 @@ void credential_approve(struct credential *);

234 * have been rejected. This will cause the credential subsystem to

235 * notify any helpers of the rejection (which allows them, for

236 * example, to purge the invalid credentials from storage). It

235 - * will also free() the username and password fields of the

236 - * credential and set them to NULL (readying the credential for

237 - * another call to `credential_fill`). Any errors from helpers are

238 - * ignored.

237 + * will also free() the username, password, and credential fields

238 + * of the credential and set them to NULL (readying the credential

239 + * for another call to `credential_fill`). Any errors from helpers

240 + * are ignored.

241 */

242 void credential_reject(struct credential *);

243