git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Junio C Hamano <gitster@pobox.com>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH 0/2] Re-integrate sha1dc
Date: Thu, 16 Mar 2017 18:04:56 -0400	[thread overview]
Message-ID: <20170316220456.m4yz2kbvzv6waokn@sigill.intra.peff.net> (raw)
In-Reply-To: <alpine.LFD.2.20.1703161315310.18484@i7.lan>

On Thu, Mar 16, 2017 at 01:24:02PM -0700, Linus Torvalds wrote:

> I suspect the first patch will not make it to the list since it's over 
> 100kB in size, but oh well.. Junio and Jeff will see it.

Yep, it didn't make it, but I got it.

> It "WorksForMe(tm)" and the integration patches are now fairly trivial, 
> since upstream already did the dieting and some of the semantic changes to 
> gits more traditional C code.

There are a few things I think are worth changing. The die() message
should mention the sha1 we computed. That will be a big help if an old
version of git tries to unknowingly push a colliding object to a newer
version. The user will see "collision on sha1 1234.." which gives them a
starting point to figure out where they got the bad object from.

And to make that work, we have to disable the safe_hash feature (which
intentionally corrupts a colliding sha1). We _could_ rip it out
entirely, but since it only kicks in when we see a collision, I doubt
it's impacting anything.

I also updated the timings in my commit message, and added a basic test.

> I did leave the C++ wrapper lines that the sha1dc header files have grown 
> in the meantime, I debated removing them but felt that "closer to 
> upstream" was worth it.

Yeah, I independently made the same decision.

So here's my version. It's on top of the hash.h tweak, as well.

  [1/5]: add collision-detecting sha1 implementation
  [2/5]: sha1dc: adjust header includes for git
  [3/5]: sha1dc: disable safe_hash feature
  [4/5]: Makefile: add USE_SHA1DC knob
  [5/5]: t0013: add a basic sha1 collision detection test

 Makefile                |   11 +
 hash.h                  |    2 +
 sha1dc/LICENSE.txt      |   30 +
 sha1dc/sha1.c           | 1808 +++++++++++++++++++++++++++++++++++++++++++++++
 sha1dc/sha1.h           |  122 ++++
 sha1dc/ubc_check.c      |  363 ++++++++++
 sha1dc/ubc_check.h      |   44 ++
 t/t0013-sha1dc.sh       |   19 +
 t/t0013/shattered-1.pdf |  Bin 0 -> 422435 bytes
 9 files changed, 2399 insertions(+)
 create mode 100644 sha1dc/LICENSE.txt
 create mode 100644 sha1dc/sha1.c
 create mode 100644 sha1dc/sha1.h
 create mode 100644 sha1dc/ubc_check.c
 create mode 100644 sha1dc/ubc_check.h
 create mode 100755 t/t0013-sha1dc.sh
 create mode 100644 t/t0013/shattered-1.pdf

-Peff

  reply	other threads:[~2017-03-16 22:06 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-16 20:24 [PATCH 0/2] Re-integrate sha1dc Linus Torvalds
2017-03-16 22:04 ` Jeff King [this message]
2017-03-16 22:08   ` [PATCH 2/5] sha1dc: adjust header includes for git Jeff King
2017-03-16 22:08   ` [PATCH 3/5] sha1dc: disable safe_hash feature Jeff King
2017-03-16 22:09   ` [PATCH 4/5] Makefile: add USE_SHA1DC knob Jeff King
2017-03-16 22:43     ` Junio C Hamano
2017-03-17  0:11       ` Jeff King
2017-03-17  5:24         ` Junio C Hamano
2017-03-17 11:18           ` Jeff King
2017-03-17 17:09             ` [RFC PATCH 0/3] Git integration update for DC-SHA1 Junio C Hamano
2017-03-17 17:09               ` [PATCH 1/3] Makefile: add DC_SHA1 knob Junio C Hamano
2017-03-17 17:09               ` [PATCH 3/3] Makefile: make DC_SHA1 the default Junio C Hamano
2017-03-17 17:41               ` [RFC PATCH 0/3] Git integration update for DC-SHA1 Junio C Hamano
2017-03-17 17:45               ` Jeff King
2017-03-16 22:10   ` [PATCH 0/2] Re-integrate sha1dc Jeff King
2017-03-16 22:23     ` Junio C Hamano
2017-03-17  0:14       ` Jeff King
2017-03-17  5:22         ` Junio C Hamano
2017-03-17 11:22           ` Jeff King
2017-03-16 22:30   ` Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170316220456.m4yz2kbvzv6waokn@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).