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

From: Junio C Hamano <[email-masked]>
Date: Wed, 08 Jan 2025 09:40:58 -0800

Patrick Steinhardt <[email-masked]> writes:

> 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?

> 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?