Re: [PATCH] credential: clear expired c->credential in addition to c->password

From: Aaron Plattner <aplattner@nvidia.com>
Date: Jun 4, 2024, 18:51 UTC
Message-ID: <4639c4de-9915-4e3c-9c5f-9c55cfd46637@nvidia.com>
In-Reply-To: <xmqqed9cva5s.fsf@gitster.g>

On 6/4/24 11:24 AM, Junio C Hamano wrote:

Show 24 quoted lines

> Aaron Plattner <aplattner@nvidia.com> writes:
> 
>> 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. Clear that too.
> 
> Hmph, piling another thing on top of these selected "discard/reset"
> we already have should make us rethink a few things.
> 
>   - Is this the only place we discard/reset/clear?
> 
>   - Isn't there already a helper function that was DESIGNED to do
>     this for us?
> 
>   - Are all these places we discard/reset/clear using that helper
>     function?
> 
> For example, when we rejecting credential, shouldn't we be clearing
> the same members of the structure as we notice that the auth material
> is stale and has expired?
> 
> There is credential_clear() and credential_clear_secrets().  Would
> one of these want to be reused in this (and also reject) context?

Good questions.

As far as I can tell, credential_clear() is for when we're done with a struct credential completely and want to reuse that memory for something. credential_clear_secrets() is used when we just want to reject the secret part of the struct cred but reuse the rest of the fields. I'll go through and see if I can determine which is which and send a patch to unify some of these.

-- Aaron