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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 30, 2017, 00:31 UTC
Message-ID: <xmqqr31f7gh8.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <alpine.DEB.2.20.1703291509450.48202@virtualbox>

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

Show 9 quoted lines

> - 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.

Show 13 quoted lines

> 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/