Re: SHA1 collisions found

[Linus Torvalds <torvalds@linux-foundation.org>]

On Tue, Feb 28, 2017 at 11:07 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> In a way similar to 8415558f55 ("sha1dc: avoid c99
> declaration-after-statement", 2017-02-24), we would want this on
> top.

There's a few other simplifications that could be done:

 (1) make the symbols static that aren't used.

     The sha1.h header ends up declaring several things that shouldn't
have been exported.

     I suspect the code may have had some debug mode that got stripped
out from it before making it public (or that was never there, and was
just something the generating code could add).

 (2) get rid of the "safe mode" support.

     That one is meant for non-checking replacements where it
generates a *different* hash for input with the collision fingerpring,
but that's pointless for the git use when we abort on a collision
fingerprint.

I think the first one will show that the sha1_compression() function
isn't actually used, and with the removal of safe-mode I think
sha1_compression_W() also is unused.

Finally, only states 58 and 65 (out of all 80 states) are actually
used, and from what I can tell, the 'maski' value is always 0, so the
looping over 80 state masks is really just a loop over two.

The file has code top *generate* all the 80 sha1_recompression_step()
functions, and I don't think the compiler is smart enough to notice
that only two of them matter.

And because 'maski' is always zero, thisL

   ubc_dv_mask[sha1_dvs[i].maski]

code looks like it might as well just use ubc_dv_mask[0] - in fact the
ubc_dv_mask[] "array" really is just a single-entry array anyway:

   #define DVMASKSIZE 1

so that code has a few oddities in it. It's generated code, which is
probably why.

Basically, some of it could be improved. In particular, the "generate
code for 80 different recompression cases, but only ever use two of
them" really looks like it would blow up the code generation footprint
a lot.

I'm adding Marc Stevens and Dan Shumow to this email (bcc'd, so that
they don't get dragged into any unrelated email threads) in case they
want to comment.

I'm wondering if they perhaps have a cleaned-up version somewhere, or
maybe they can tell me that I'm just full of sh*t and missed
something.

                    Linus