Re: [PATCH 2/4] hash.h: scaffolding for _fast hashing variants
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Sep 3, 2024, 19:52 UTC
- Message-ID
- <ZtdpFWtWae2r4Lj4@nand.local>
- In-Reply-To
- <xmqqh6awlkac.fsf@gitster.g>
On Tue, Sep 03, 2024 at 10:27:39AM -0700, Junio C Hamano wrote:
Show 16 quoted lines
> 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.Show 6 quoted lines
> (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