On Tue, Sep 03, 2024 at 10:27:39AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@xxxxxx> 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
