On Thu, Feb 23, 2017 at 11:32 AM, Jeff King <peff@xxxxxxxx> wrote: > > Yeah, they're not expensive. We've discussed enabling them by default. > The sticking point is that there is old history with minor bugs which > triggers some warnings (e.g., malformed committer names), and it would > be annoying to start rejecting that unconditionally. > > So I think we would need a good review of what is a "warning" versus an > "error", and to only reject on errors (right now the NUL thing is a > warning, and it should probably upgraded). I think even a warning (as opposed to failing the operation) is already a big deal. If people start saying "why do I get this odd warning", and start looking into it, that's going to be a pretty strong defense against bad behavior. SCM attacks depend on flying under the radar. >> So a very powerful defense is to just look for those bit patterns in >> the objects, and just warn about them. Those patterns don't tend to >> exist in normal inputs anyway, but particularly if you just warn, it's >> a heads-ups that "ok, something iffy is going on" > > Yes, that would be a wonderful hardening to put into Git if we know what > those patterns look like. That part isn't clear to me. There's actually already code for that, pointed to by the shattered project: https://github.com/cr-marcstevens/sha1collisiondetection the "meat" of that check is in lib/ubc_check.c. Linus