Skip to content

Commit

Permalink
Merge branch 'ap/credential-clear-fix' into next
Browse files Browse the repository at this point in the history
Upon expiration event, we forgot to clear in-core authentication
material other than password (whose support was added recently),
which has been corrected.

* ap/credential-clear-fix:
  credential: clear expired c->credential, unify secret clearing
  • Loading branch information
gitster committed Jun 7, 2024
2 parents 394989e + 27db485 commit 933f06b
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 22 deletions.
16 changes: 10 additions & 6 deletions credential.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -479,9 +478,15 @@ 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;
}
Expand Down Expand Up @@ -528,9 +533,8 @@ void credential_reject(struct credential *c)
for (i = 0; i < c->helpers.nr; i++)
credential_do(c, c->helpers.items[i].string, "erase");

credential_clear_secrets(c);
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;
Expand Down
34 changes: 18 additions & 16 deletions credential.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
#include "strvec.h"

/**
* The credentials API provides an abstracted way of gathering username and
* password credentials from the user.
* The credentials API provides an abstracted way of gathering
* authentication credentials from the user.
*
* Typical setup
* -------------
Expand Down Expand Up @@ -116,11 +116,12 @@ struct credential_capability {
};

/**
* This struct represents a single username/password combination
* along with any associated context. All string fields should be
* heap-allocated (or NULL if they are not known or not applicable).
* The meaning of the individual context fields is the same as
* their counterparts in the helper protocol.
* This struct represents a single login credential (typically a
* username/password combination) along with any associated
* context. All string fields should be heap-allocated (or NULL if
* they are not known or not applicable). The meaning of the
* individual context fields is the same as their counterparts in
* the helper protocol.
*
* This struct should always be initialized with `CREDENTIAL_INIT` or
* `credential_init`.
Expand Down Expand Up @@ -207,11 +208,12 @@ void credential_clear(struct credential *);

/**
* Instruct the credential subsystem to fill the username and
* password fields of the passed credential struct by first
* consulting helpers, then asking the user. After this function
* returns, the username and password fields of the credential are
* guaranteed to be non-NULL. If an error occurs, the function will
* die().
* password (or authtype and credential) fields of the passed
* credential struct by first consulting helpers, then asking the
* user. After this function returns, either the username and
* password fields or the credential field of the credential are
* guaranteed to be non-NULL. If an error occurs, the function
* will die().
*
* If all_capabilities is set, this is an internal user that is prepared
* to deal with all known capabilities, and we should advertise that fact.
Expand All @@ -232,10 +234,10 @@ void credential_approve(struct credential *);
* have been rejected. This will cause the credential subsystem to
* notify any helpers of the rejection (which allows them, for
* example, to purge the invalid credentials from storage). It
* will also free() the username and password fields of the
* credential and set them to NULL (readying the credential for
* another call to `credential_fill`). Any errors from helpers are
* ignored.
* will also free() the username, password, and credential fields
* of the credential and set them to NULL (readying the credential
* for another call to `credential_fill`). Any errors from helpers
* are ignored.
*/
void credential_reject(struct credential *);

Expand Down

0 comments on commit 933f06b

Please sign in to comment.