Re: [PATCH (v2.47 regression)] hash.h: set NEEDS_CLONE_HELPER_UNSAFE in fallback mode

[Taylor Blau <me@ttaylorr.com>]

On Wed, Oct 02, 2024 at 08:21:40PM -0400, Jeff King wrote:
> On Thu, Oct 03, 2024 at 12:13:47AM +0000, brian m. carlson wrote:
>
> > On 2024-10-02 at 23:26:18, Jeff King wrote:
> > > This is a regression in v2.47.0-rc0. As mentioned above, I kind of doubt
> > > anybody will hit it in practice (I only did because I was trying to do
> > > some timing tests between the fast and dc variants). And it is almost
> > > tempting to leave it as a wake-up call for anybody who is still not
> > > using a collision-detecting sha1. ;)
> >
> > I think this is a fine fix for 2.47.  I have a branch on my remote
> > (sha1-dc-only), which I'll send out after it passes CI (probably later
> > this week), that removes support for the everything but SHA-1-DC (except
> > for the unsafe code).
> >
> > I don't think there's a reasonable configuration where people can use
> > Git with other SHA-1 code except in extremely limited circumstances we
> > shouldn't have to maintain code for.  The code is open source, so people
> > who really must have maximum performance with all of the vulnerabilities
> > can patch it back in themselves.
>
> Yeah, I feel the same way. I only happened to try this because it was
> the easiest way to speed-compare different implementations using
> "test-tool sha1". ;)

I imagine that you both mean that non-collision detecting variants are
unsuitable for the "safe" SHA-1 implementation, and that the "unsafe"
variant can still be driven with BLK_SHA1, OpenSSL, etc.

And reading the patch at the tip of brian's 'sha1-dc-only' branch, that
looks to be the case. So I'm in agreement with the both of you ;-).

> Possibly that helper could grow an option to use the unsafe variant,
> though even that is probably not a high priority.

Yeah, that would be nice. Though I agree it's not a huge priority.

Thanks,
Taylor