Re: [PATCH 2/2] reftable/stack: accept insecure random bytes

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 8, 2025, 17:40 UTC
Message-ID: <xmqqo70hmcet.fsf@gitster.g>
In-Reply-To: <Z36l--QUjaYYb6Uf@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 13 quoted lines

> Hm. The problem is when Git dies in the middle of a transaction:
>
>   1. We write the temporary table.
>   2. We compute the not-so-random suffix.
>   3. We write the temporary "tables.list" file.
>   4. We move the temporary table into place using the not-so-random
>      suffix.
>   5. Git dies before updating "tables.list".
>
> Now we have the temporary table moved into place, but "tables.list"
> hasn't been updated yet. When the next Git process comes along and wants
> to update the table it would result in an error if it computed the same
> suffix.

Here, I hear that we _do_ depend on the suffix being relatively unique. Once our random number generator decides to give the same number twice to cause collision, the reftable data gets corrupt?

Show 6 quoted lines

> The reftable library knows to clean up such stale tables when not
> referenced by the "tables.list" file, but it doesn't do so on every
> write. So this would likely still cause issues in practice.
>
> I already though about this scenario when writing my mail, but didn't
> really think about it as "correctness". But I guess it is.

Hmph. I am not sure how I should feel about this. Our reliance on hash functions (which can be made to collide) not colliding is one thing, but is it sensibly safe to rely on a cryptographically unpredictable random generator not to yield the same suffix twice during the lifetime of an previous invocation for correctness?