Re: [PATCH 0/7] PREVIEW: Introduce DC_AND_OPENSSL_SHA1 make flag

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> - After the SHAttered blog post became public, Linus first made the case
>   that it matters not all that much: the development of the Linux kernel
>   is based on trust, and nobody would pull from a person they do not trust.
>   This approach does obviously not extend to most other projects.
>
> - By switching the default to DC_SHA1 already in `master`, you now took
>   the exact opposite position: it *always* matters, even when you trust
>   people, and the 6x slowdown demonstrated by my perf test is something that
>   everybody needs to accept, even if it is spent for no benefit in return.

You seem to be trying very hard to make it look like as if Linus
said one thing and I decided to do something else, but if one digs
into the archive, it should be clear that that is not how the
current line of development has happened.  Even from the same
person, you can hear that "the social aspect of how the project is
structured makes the kernel less susceptible to the shattered
attack" [*1*] and that "mitigation at Git level is easy" [*2*].

The kernel as a project may not have to do much, but the world of
projects using Git is wider than the kernel alone, and mitigation
that applies to all was available and is easy to make it the
default.

> The approach I chose instead was to make the switch global, per command.
> Obviously, the next step is to identify the Git commands which accept
> objects from external sources (`clone`, `fetch`, `fast-import` and all the
> remote helpers come to mind) and let them set a global flag before asking
> git_default_config() to parse the core.enableSHA1DC setting, so that the
> special value `external` could trigger the collision detecting code for
> those, and only those, commands. That would still err on the safe side if
> these commands are used to hash objects originating from the same machine,
> but that is a price I would be willing to pay for the simplicity of this
> approach.
>
> Does my explanation manage to illustrate in a plausible way why I chose
> the approach that I did?

I agree that there may be rooms to tweak the degree of paranoia per
codepath (I said that already in the message you are responding to),
but as Linus and Peff already said in the old discussion thread
[*3*], I doubt that it needs to be runtime configurable.

In any case, I do not think you should blindly trust what end-users
add to the repository locally.  A vendor may send in a binary driver
update via a pull-request, and you would want to protect "clone" and
"fetch" client because of that.  The same driver update however may
come as a patch that is accepted by running "git am".  Or you may
get a vendor tarball ftp'ed over and "git add ." the whole thing.

These "local" operations still need the same degree of paranoia as
your "git fetch"; "per command" may not be such a good criterion
because of that.

That is why I mentioned "you want to be paranoid any time you add
new data to the object database" as a rule of thumb in the message
you are responding to.  It may be overly broad, but would be a
better starting point than "'git add' is always safe as it is
local".

So in short, I do agree with your idea of using "faster" hashing for
some codepaths and "paranoid" ones for others.  I think "slower but
collision-attack detecting" one (aka DC_SHA1) plus a choice of
"faster" one at compile time would be a sensible way to go, and if
one is truly paranoid, the "faster" one _could_ be sha1dc.c with
collision detection disabled.  On the other hand, if one is in a
closed environment, one may want both hashes configurable and let
OpenSSL implementation be chosen for both "slower but DC" and
"faster" hash.  And I am OK with that, too.


[Reference]

*1* https://public-inbox.org/git/CA+55aFz98r7NC_3BW_1HU9-0C-HcrFou3=0gmRcS38=-x8dmVw@mail.gmail.com/

*2* https://public-inbox.org/git/CA+55aFxmr6ntWGbJDa8tOyxXDX3H-yd4TQthgV_Tn1u91yyT8w@mail.gmail.com/

*3* https://public-inbox.org/git/20170323174750.xyucxmfhuc6dbrzc@sigill.intra.peff.net/