Re: [PATCH 2/4] hash.h: scaffolding for _fast hashing variants

[Taylor Blau <me@ttaylorr.com>]

On Tue, Sep 03, 2024 at 10:27:39AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > While the property we care about in the context of this patch series
> > indeed is that the second hash is faster, I think the more important
> > property is that it's insecure. If I were seeing two APIs, one labelled
> > fast and one labelled slow, I would of course pick the fast one. So I
> > wonder whether we should rename things accordingly so that developers
> > aren't intrigued to pick the fast one without thinking, and also to have
> > a more useful signal that stands out to reviewers.
>
> I do not think this topic is going in the direction it set out to,
> but if we are to resurrect it by
>
>  (1) first to ensure that we won't overwrite existing on-disk files
>      and other things as needed to safely swap the tail sum to a
>      cryptographically insecure hash function;

I discussed this with brian in the sub-thread where I am talking to
them, but I think this is already the case. The pack is read in
index-pack and the checksum is verified without using the _fast hash
functions, so we would detect:

  - either half of a colliding pair of objects, when reading individual
    objects' contents to determine their SHA-1s, or

  - a colliding pack checksum, when computing the whole pack's checksum
    (which also does not use the _fast variants of these functions), and

  - a mismatched pack checksum, when verifying the pack's checksum
    against the one stored in the pack.

>  (2) devise a transition plan to use a hash function that computes a
>      value that is different from SHA-1 (or SHA-256 for that
>      matter); and
>
>  (3) pick a hash function that computes a lot faster but is insecure
>      and transition to it.

So I do not think that either of these two steps are necessary.

Thanks,
Taylor