{"thread":{"id":"45399","subject":"[PATCH 0/2] Re-integrate sha1dc","startedAt":"2017-03-16T20:24:48Z","lastAt":"2017-03-17T17:53:34Z","messageCount":20,"participants":["Linus Torvalds","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"314253","messageId":"alpine.LFD.2.20.1703161315310.18484@i7.lan","threadId":"45399","inReplyTo":null,"subject":"[PATCH 0/2] Re-integrate sha1dc","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2017-03-16T20:24:02Z","receivedAt":"2017-03-16T20:24:48Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nI suspect the first patch will not make it to the list since it's over \n100kB in size, but oh well.. Junio and Jeff will see it.\n\nThis is sent as two patches, just to have the original upstream code as a \nfirst step, and then the second patch does the small modifications to \nintegrate it with git.\n\nIt \"WorksForMe(tm)\" and the integration patches are now fairly trivial, \nsince upstream already did the dieting and some of the semantic changes to \ngits more traditional C code.\n\nI did leave the C++ wrapper lines that the sha1dc header files have grown \nin the meantime, I debated removing them but felt that \"closer to \nupstream\" was worth it.\n\n               Linus\n"},{"id":"314261","messageId":"20170316220456.m4yz2kbvzv6waokn@sigill.intra.peff.net","threadId":"45399","inReplyTo":"alpine.LFD.2.20.1703161315310.18484@i7.lan","subject":"Re: [PATCH 0/2] Re-integrate sha1dc","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-16T22:04:56Z","receivedAt":"2017-03-16T22:06:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 16, 2017 at 01:24:02PM -0700, Linus Torvalds wrote:\n\n> I suspect the first patch will not make it to the list since it's over \n> 100kB in size, but oh well.. Junio and Jeff will see it.\n\nYep, it didn't make it, but I got it.\n\n> It \"WorksForMe(tm)\" and the integration patches are now fairly trivial, \n> since upstream already did the dieting and some of the semantic changes to \n> gits more traditional C code.\n\nThere are a few things I think are worth changing. The die() message\nshould mention the sha1 we computed. That will be a big help if an old\nversion of git tries to unknowingly push a colliding object to a newer\nversion. The user will see \"collision on sha1 1234..\" which gives them a\nstarting point to figure out where they got the bad object from.\n\nAnd to make that work, we have to disable the safe_hash feature (which\nintentionally corrupts a colliding sha1). We _could_ rip it out\nentirely, but since it only kicks in when we see a collision, I doubt\nit's impacting anything.\n\nI also updated the timings in my commit message, and added a basic test.\n\n> I did leave the C++ wrapper lines that the sha1dc header files have grown \n> in the meantime, I debated removing them but felt that \"closer to \n> upstream\" was worth it.\n\nYeah, I independently made the same decision.\n\nSo here's my version. It's on top of the hash.h tweak, as well.\n\n  [1/5]: add collision-detecting sha1 implementation\n  [2/5]: sha1dc: adjust header includes for git\n  [3/5]: sha1dc: disable safe_hash feature\n  [4/5]: Makefile: add USE_SHA1DC knob\n  [5/5]: t0013: add a basic sha1 collision detection test\n\n Makefile                |   11 +\n hash.h                  |    2 +\n sha1dc/LICENSE.txt      |   30 +\n sha1dc/sha1.c           | 1808 +++++++++++++++++++++++++++++++++++++++++++++++\n sha1dc/sha1.h           |  122 ++++\n sha1dc/ubc_check.c      |  363 ++++++++++\n sha1dc/ubc_check.h      |   44 ++\n t/t0013-sha1dc.sh       |   19 +\n t/t0013/shattered-1.pdf |  Bin 0 -> 422435 bytes\n 9 files changed, 2399 insertions(+)\n create mode 100644 sha1dc/LICENSE.txt\n create mode 100644 sha1dc/sha1.c\n create mode 100644 sha1dc/sha1.h\n create mode 100644 sha1dc/ubc_check.c\n create mode 100644 sha1dc/ubc_check.h\n create mode 100755 t/t0013-sha1dc.sh\n create mode 100644 t/t0013/shattered-1.pdf\n\n-Peff\n"},{"id":"314264","messageId":"20170316220810.orlbvop53fj4g5wg@sigill.intra.peff.net","threadId":"45399","inReplyTo":"20170316220456.m4yz2kbvzv6waokn@sigill.intra.peff.net","subject":"[PATCH 2/5] sha1dc: adjust header includes for git","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-16T22:08:10Z","receivedAt":"2017-03-16T22:08:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We can replace system includes with git-compat-util.h or\ncache.h (and should make sure it is included first in all C\nfiles).  And we can drop includes from headers entirely, as\nevery C file should include git-compat-util.h itself.\n\nWe will add in new include guards around the header files,\nthough (otherwise you get into trouble including both\nsha1dc/sha1.h and cache.h).\n\nAnd finally, we'll use the full \"sha1dc/\" path for including\nrelated files. This isn't strictly necessary, but makes the\nexpected resolution more obvious.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThe cache.h thing is necessary if we want to use sha1_to_hex() later\n(which I think we should).\n\n sha1dc/sha1.c      | 10 +++-------\n sha1dc/sha1.h      |  6 ++++--\n sha1dc/ubc_check.c |  4 ++--\n sha1dc/ubc_check.h |  2 --\n 4 files changed, 9 insertions(+), 13 deletions(-)\n\ndiff --git a/sha1dc/sha1.c b/sha1dc/sha1.c\nindex 8d12b832b..da516c14c 100644\n--- a/sha1dc/sha1.c\n+++ b/sha1dc/sha1.c\n@@ -5,13 +5,9 @@\n * https://opensource.org/licenses/MIT\n ***/\n \n-#include <string.h>\n-#include <memory.h>\n-#include <stdio.h>\n-#include <stdlib.h>\n-\n-#include \"sha1.h\"\n-#include \"ubc_check.h\"\n+#include \"cache.h\"\n+#include \"sha1dc/sha1.h\"\n+#include \"sha1dc/ubc_check.h\"\n \n \n /* \ndiff --git a/sha1dc/sha1.h b/sha1dc/sha1.h\nindex e867724c0..8a5bf0847 100644\n--- a/sha1dc/sha1.h\n+++ b/sha1dc/sha1.h\n@@ -4,13 +4,13 @@\n * See accompanying file LICENSE.txt or copy at\n * https://opensource.org/licenses/MIT\n ***/\n+#ifndef SHA1DC_SHA1_H\n+#define SHA1DC_SHA1_H\n \n #if defined(__cplusplus)\n extern \"C\" {\n #endif\n \n-#include <stdint.h>\n-\n /* uses SHA-1 message expansion to expand the first 16 words of W[] to 80 words */\n /* void sha1_message_expansion(uint32_t W[80]); */\n \n@@ -103,3 +103,5 @@ int  SHA1DCFinal(unsigned char[20], SHA1_CTX*);\n #if defined(__cplusplus)\n }\n #endif\n+\n+#endif /* SHA1DC_SHA1_H */\ndiff --git a/sha1dc/ubc_check.c b/sha1dc/ubc_check.c\nindex 27d0976da..089dd4743 100644\n--- a/sha1dc/ubc_check.c\n+++ b/sha1dc/ubc_check.c\n@@ -24,8 +24,8 @@\n // ubc_check has been verified against ubc_check_verify using the 'ubc_check_test' program in the tools section\n */\n \n-#include <stdint.h>\n-#include \"ubc_check.h\"\n+#include \"git-compat-util.h\"\n+#include \"sha1dc/ubc_check.h\"\n \n static const uint32_t DV_I_43_0_bit \t= (uint32_t)(1) << 0;\n static const uint32_t DV_I_44_0_bit \t= (uint32_t)(1) << 1;\ndiff --git a/sha1dc/ubc_check.h b/sha1dc/ubc_check.h\nindex b349bed92..b64c306d7 100644\n--- a/sha1dc/ubc_check.h\n+++ b/sha1dc/ubc_check.h\n@@ -27,8 +27,6 @@\n extern \"C\" {\n #endif\n \n-#include <stdint.h>\n-\n #define DVMASKSIZE 1\n typedef struct { int dvType; int dvK; int dvB; int testt; int maski; int maskb; uint32_t dm[80]; } dv_info_t;\n extern dv_info_t sha1_dvs[];\n-- \n2.12.0.623.g86ec6c963\n\n"},{"id":"314265","messageId":"20170316220849.ye56ycpoly2o5fg6@sigill.intra.peff.net","threadId":"45399","inReplyTo":"20170316220456.m4yz2kbvzv6waokn@sigill.intra.peff.net","subject":"[PATCH 3/5] sha1dc: disable safe_hash feature","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-16T22:08:49Z","receivedAt":"2017-03-16T22:08:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The safe_hash feature is designed to make sha1dc a drop-in\nreplacement for sha1, where colliding entries will get a\npermuted hash to un-collide them. However, since we're\nhandling the collision case ourselves, this isn't helpful\n(and is actually harmful, as it means you get the wrong\nobject id if you want to show it in a log message).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nWe could also disable this at runtime, but there's not really any point.\nAnd this way we know that we won't miss a call.\n\n sha1dc/sha1.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/sha1dc/sha1.c b/sha1dc/sha1.c\nindex da516c14c..00760c352 100644\n--- a/sha1dc/sha1.c\n+++ b/sha1dc/sha1.c\n@@ -1661,7 +1661,7 @@ void SHA1DCInit(SHA1_CTX* ctx)\n \tctx->ihv[3] = 0x10325476;\n \tctx->ihv[4] = 0xC3D2E1F0;\n \tctx->found_collision = 0;\n-\tctx->safe_hash = 1;\n+\tctx->safe_hash = 0;\n \tctx->ubc_check = 1;\n \tctx->detect_coll = 1;\n \tctx->reduced_round_coll = 0;\n-- \n2.12.0.623.g86ec6c963\n\n"},{"id":"314266","messageId":"20170316220911.43zernzq643m5mmk@sigill.intra.peff.net","threadId":"45399","inReplyTo":"20170316220456.m4yz2kbvzv6waokn@sigill.intra.peff.net","subject":"[PATCH 4/5] Makefile: add USE_SHA1DC knob","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-16T22:09:12Z","receivedAt":"2017-03-16T22:09:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This knob lets you use the sha1dc implementation from:\n\n      https://github.com/cr-marcstevens/sha1collisiondetection\n\nwhich can detect certain types of collision attacks (even\nwhen we only see half of the colliding pair). So it\nmitigates any attack which consists of getting the \"good\"\nhalf of a collision into a trusted repository, and then\nlater replacing it with the \"bad\" half. The \"good\" half is\nrejected by the victim's version of Git (and even if they\nrun an old version of Git, any sha1dc-enabled git will\ncomplain loudly if it ever has to interact with the object).\n\nThe big downside is that it's slower than either the openssl\nor block-sha1 implementations.\n\nHere are some timings based off of linux.git:\n\n  - compute sha1 over whole packfile\n      sha1dc: 3.580s\n    blk-sha1: 2.046s (-43%)\n     openssl: 1.335s (-62%)\n\n  - rev-list --all --objects\n      sha1dc: 33.512s\n    blk-sha1: 33.514s (+0.0%)\n     openssl: 33.650s (+0.4%)\n\n  - git log --no-merges -10000 -p\n      sha1dc: 8.124s\n    blk-sha1: 7.986s (-1.6%)\n     openssl: 8.203s (+0.9%)\n\n  - index-pack --verify\n      sha1dc: 4m19s\n    blk-sha1: 2m57s (-32%)\n     openssl: 2m19s (-42%)\n\nSo overall the sha1 computation with collision detection is\nabout 1.75x slower than block-sha1, and 2.7x slower than\nsha1. But of course most operations do more than just sha1.\nNormal object access isn't really slowed at all (both the\n+/- changes there are well within the run-to-run noise); any\nchanges are drowned out by the other work Git is doing.\n\nThe most-affected operation is `index-pack --verify`, which\nis essentially just computing the sha1 on every object. This\nis similar to the `index-pack` invocation that the receiver\nof a push or fetch would perform. So clearly there's some\nextra CPU load here.\n\nThere will also be some latency for the user, though keep in\nmind that such an operation will generally be network bound\n(this is about a 1.2GB packfile). Some of that extra CPU is\n\"free\" in the sense that we use it while the pack is\nstreaming in anyway. But most of it comes during the\ndelta-resolution phase, after the whole pack has been\nreceived. So we can imagine that for this (quite large)\npush, the user might have to wait an extra 100 seconds over\nopenssl (which is what we use now). If we assume they can\npush to us at 20Mbit/s, that's 480s for a 1.2GB pack, which\nis only 20% slower.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Makefile      | 10 ++++++++++\n hash.h        |  2 ++\n sha1dc/sha1.c | 20 ++++++++++++++++++++\n sha1dc/sha1.h | 15 +++++++++++++++\n 4 files changed, 47 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex 9f9561147..0aa483890 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -140,6 +140,10 @@ all::\n # Define PPC_SHA1 environment variable when running make to make use of\n # a bundled SHA1 routine optimized for PowerPC.\n #\n+# Define USE_SHA1DC to unconditionally enable the collision-detecting sha1\n+# algorithm. This is slower, but may detect attempted collision attacks.\n+# Takes priority over other *_SHA1 knobs.\n+#\n # Define SHA1_MAX_BLOCK_SIZE to limit the amount of data that will be hashed\n # in one call to the platform's SHA1_Update(). e.g. APPLE_COMMON_CRYPTO\n # wants 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined.\n@@ -1383,6 +1387,11 @@ ifdef APPLE_COMMON_CRYPTO\n \tSHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L\n endif\n \n+ifdef USE_SHA1DC\n+\tLIB_OBJS += sha1dc/sha1.o\n+\tLIB_OBJS += sha1dc/ubc_check.o\n+\tBASIC_CFLAGS += -DSHA1_SHA1DC\n+else\n ifdef BLK_SHA1\n \tLIB_OBJS += block-sha1/sha1.o\n \tBASIC_CFLAGS += -DSHA1_BLK\n@@ -1400,6 +1409,7 @@ else\n endif\n endif\n endif\n+endif\n \n ifdef SHA1_MAX_BLOCK_SIZE\n \tLIB_OBJS += compat/sha1-chunked.o\ndiff --git a/hash.h b/hash.h\nindex f0d9ddd0c..3760f436e 100644\n--- a/hash.h\n+++ b/hash.h\n@@ -7,6 +7,8 @@\n #include <CommonCrypto/CommonDigest.h>\n #elif defined(SHA1_OPENSSL)\n #include <openssl/sha.h>\n+#elif defined(SHA1_SHA1DC)\n+#include \"sha1dc/sha1.h\"\n #else /* SHA1_BLK */\n #include \"block-sha1/sha1.h\"\n #endif\ndiff --git a/sha1dc/sha1.c b/sha1dc/sha1.c\nindex 00760c352..3ce0a4f39 100644\n--- a/sha1dc/sha1.c\n+++ b/sha1dc/sha1.c\n@@ -1786,3 +1786,23 @@ int SHA1DCFinal(unsigned char output[20], SHA1_CTX *ctx)\n \toutput[19] = (unsigned char)(ctx->ihv[4]);\n \treturn ctx->found_collision;\n }\n+\n+void git_SHA1DCFinal(unsigned char hash[20], SHA1_CTX *ctx)\n+{\n+\tif (!SHA1DCFinal(hash, ctx))\n+\t\treturn;\n+\tdie(\"SHA-1 appears to be part of a collision attack: %s\",\n+\t    sha1_to_hex(hash));\n+}\n+\n+void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *vdata, unsigned long len)\n+{\n+\tconst char *data = vdata;\n+\t/* We expect an unsigned long, but sha1dc only takes an int */\n+\twhile (len > INT_MAX) {\n+\t\tSHA1DCUpdate(ctx, data, INT_MAX);\n+\t\tdata += INT_MAX;\n+\t\tlen -= INT_MAX;\n+\t}\n+\tSHA1DCUpdate(ctx, data, len);\n+}\ndiff --git a/sha1dc/sha1.h b/sha1dc/sha1.h\nindex 8a5bf0847..aa883df01 100644\n--- a/sha1dc/sha1.h\n+++ b/sha1dc/sha1.h\n@@ -100,6 +100,21 @@ void SHA1DCUpdate(SHA1_CTX*, const char*, size_t);\n /* returns: 0 = no collision detected, otherwise = collision found => warn user for active attack */\n int  SHA1DCFinal(unsigned char[20], SHA1_CTX*); \n \n+/*\n+ * Same as SHA1DCFinal, but convert collision attack case into a verbose die().\n+ */\n+void git_SHA1DCFinal(unsigned char [20], SHA1_CTX *);\n+\n+/*\n+ * Same as SHA1DCUpdate, but adjust types to match git's usual interface.\n+ */\n+void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *data, unsigned long len);\n+\n+#define platform_SHA_CTX SHA1_CTX\n+#define platform_SHA1_Init SHA1DCInit\n+#define platform_SHA1_Update git_SHA1DCUpdate\n+#define platform_SHA1_Final git_SHA1DCFinal\n+\n #if defined(__cplusplus)\n }\n #endif\n-- \n2.12.0.623.g86ec6c963\n\n"},{"id":"314267","messageId":"20170316221044.ij5yuifmohktn6cl@sigill.intra.peff.net","threadId":"45399","inReplyTo":"20170316220456.m4yz2kbvzv6waokn@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] Re-integrate sha1dc","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-16T22:10:45Z","receivedAt":"2017-03-16T22:11:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 16, 2017 at 06:04:56PM -0400, Jeff King wrote:\n\n> So here's my version. It's on top of the hash.h tweak, as well.\n> \n>   [1/5]: add collision-detecting sha1 implementation\n>   [2/5]: sha1dc: adjust header includes for git\n>   [3/5]: sha1dc: disable safe_hash feature\n>   [4/5]: Makefile: add USE_SHA1DC knob\n>   [5/5]: t0013: add a basic sha1 collision detection test\n> [...]\n>  t/t0013/shattered-1.pdf |  Bin 0 -> 422435 bytes\n\nSo obviously I had the same 100K problem you did on the first patch, but\nthe fifth one also won't make it to the list. You can pull the whole\nthing from:\n\n  https://github.com/peff/git.git jk/sha1dc\n\n-Peff\n"},{"id":"314269","messageId":"xmqq37ecc134.fsf@gitster.mtv.corp.google.com","threadId":"45399","inReplyTo":"20170316221044.ij5yuifmohktn6cl@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] Re-integrate sha1dc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-16T22:23:59Z","receivedAt":"2017-03-16T22:24:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Mar 16, 2017 at 06:04:56PM -0400, Jeff King wrote:\n>\n>> So here's my version. It's on top of the hash.h tweak, as well.\n>> \n>>   [1/5]: add collision-detecting sha1 implementation\n>>   [2/5]: sha1dc: adjust header includes for git\n>>   [3/5]: sha1dc: disable safe_hash feature\n>>   [4/5]: Makefile: add USE_SHA1DC knob\n>>   [5/5]: t0013: add a basic sha1 collision detection test\n>> [...]\n>>  t/t0013/shattered-1.pdf |  Bin 0 -> 422435 bytes\n\nA 420k binary blob has become ~500k base85 binary patch, which is\nlarger than 100k.\n\n> So obviously I had the same 100K problem you did on the first patch, but\n> the fifth one also won't make it to the list. You can pull the whole\n> thing from:\n>\n>   https://github.com/peff/git.git jk/sha1dc\n\nThanks.  \n\nFor today's integration, I have the one from Linus only because it\ncame earlier and today's integration cycle was already running.  I\nagree with this series that disables the safe-hash thing.\n\nI am wondering if we should queue another one for .travis.yml on top\nto force use of USE_SHA1DC=YesPlease during the tests.  I expect\nthat we'd be encouraging its use for ordinary users without any\nspecific needs in the release notes in 2.13 release.\n\n\n"},{"id":"314282","messageId":"CA+55aFy1jPPPbKWOH_F31C1kGnWK6Rxhcm8+KARw2atMZs=stQ@mail.gmail.com","threadId":"45399","inReplyTo":"20170316220456.m4yz2kbvzv6waokn@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] Re-integrate sha1dc","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2017-03-16T22:30:18Z","receivedAt":"2017-03-16T22:40:11Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Thu, Mar 16, 2017 at 3:04 PM, Jeff King <peff@peff.net> wrote:\n>\n> There are a few things I think are worth changing. The die() message\n> should mention the sha1 we computed. That will be a big help if an old\n> version of git tries to unknowingly push a colliding object to a newer\n> version. The user will see \"collision on sha1 1234..\" which gives them a\n> starting point to figure out where they got the bad object from.\n>\n> And to make that work, we have to disable the safe_hash feature (which\n> intentionally corrupts a colliding sha1). We _could_ rip it out\n> entirely, but since it only kicks in when we see a collision, I doubt\n> it's impacting anything.\n>\n> I also updated the timings in my commit message, and added a basic test.\n\nNo complaints about your version.\n\n               Linus\n"},{"id":"314284","messageId":"xmqqtw6salmm.fsf@gitster.mtv.corp.google.com","threadId":"45399","inReplyTo":"20170316220911.43zernzq643m5mmk@sigill.intra.peff.net","subject":"Re: [PATCH 4/5] Makefile: add USE_SHA1DC knob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-16T22:43:13Z","receivedAt":"2017-03-16T22:43:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> +ifdef USE_SHA1DC\n> +\tLIB_OBJS += sha1dc/sha1.o\n> +\tLIB_OBJS += sha1dc/ubc_check.o\n> +\tBASIC_CFLAGS += -DSHA1_SHA1DC\n\nThe name of this CPP symbol is one difference between this and\nLinus's version.  Wouldn't \"-DSHA1_DC\" make more sense?\n\nAnother difference is that your version adds USE_SHA1DC to\nGIT-BUILD-OPTIONS in patch 5/5; I thought GIT-CFLAGS forces\nrebuilding and that was sufficient, but GIT-BUILD-OPTIONS is\navailable to tests for introspection, so adding it is needed\nfor that reason.\n\n\n"},{"id":"314293","messageId":"20170317001115.xau6nzp6i7ilkqxv@sigill.intra.peff.net","threadId":"45399","inReplyTo":"xmqqtw6salmm.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 4/5] Makefile: add USE_SHA1DC knob","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-17T00:11:16Z","receivedAt":"2017-03-17T00:18:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 16, 2017 at 03:43:13PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > +ifdef USE_SHA1DC\n> > +\tLIB_OBJS += sha1dc/sha1.o\n> > +\tLIB_OBJS += sha1dc/ubc_check.o\n> > +\tBASIC_CFLAGS += -DSHA1_SHA1DC\n> \n> The name of this CPP symbol is one difference between this and\n> Linus's version.  Wouldn't \"-DSHA1_DC\" make more sense?\n\nI'm fine with either. Somehow SHA1_DC felt too short, but it doesn't\nreally matter in practice.\n\n> Another difference is that your version adds USE_SHA1DC to\n> GIT-BUILD-OPTIONS in patch 5/5; I thought GIT-CFLAGS forces\n> rebuilding and that was sufficient, but GIT-BUILD-OPTIONS is\n> available to tests for introspection, so adding it is needed\n> for that reason.\n\nYep, exactly.\n\n-Peff\n"},{"id":"314296","messageId":"20170317001416.bthqvjbf554zhrj5@sigill.intra.peff.net","threadId":"45399","inReplyTo":"xmqq37ecc134.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/2] Re-integrate sha1dc","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-17T00:14:16Z","receivedAt":"2017-03-17T00:21:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 16, 2017 at 03:23:59PM -0700, Junio C Hamano wrote:\n\n> I am wondering if we should queue another one for .travis.yml on top\n> to force use of USE_SHA1DC=YesPlease during the tests.  I expect\n> that we'd be encouraging its use for ordinary users without any\n> specific needs in the release notes in 2.13 release.\n\nI don't think it would buy us much. There's not really any way for this\nbuild to interact with the rest of the code in any interesting way, so\neither it works as a SHA-1 implementation or it doesn't. If you just\nwant it exercised, I'll say that it's powering all of github.com right\nnow.\n\nI did wonder if we should ship with it as the default (instead of\nopenssl). It's definitely slower, but maybe widespread safety is a good\nthing. OTOH, I think we have a fair bit of time before we see any\nreal-life collisions, just given the time and expense of generating\nthem.\n\n-Peff\n"},{"id":"314304","messageId":"xmqqlgs4a35n.fsf@gitster.mtv.corp.google.com","threadId":"45399","inReplyTo":"20170317001416.bthqvjbf554zhrj5@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] Re-integrate sha1dc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-17T05:22:12Z","receivedAt":"2017-03-17T05:22:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Mar 16, 2017 at 03:23:59PM -0700, Junio C Hamano wrote:\n>\n>> I am wondering if we should queue another one for .travis.yml on top\n>> to force use of USE_SHA1DC=YesPlease during the tests.  I expect\n>> that we'd be encouraging its use for ordinary users without any\n>> specific needs in the release notes in 2.13 release.\n>\n> I don't think it would buy us much. There's not really any way for this\n> build to interact with the rest of the code in any interesting way, so\n> either it works as a SHA-1 implementation or it doesn't. If you just\n> want it exercised, I'll say that it's powering all of github.com right\n> now.\n>\n> I did wonder if we should ship with it as the default (instead of\n> openssl). It's definitely slower, but maybe widespread safety is a good\n> thing. OTOH, I think we have a fair bit of time before we see any\n> real-life collisions, just given the time and expense of generating\n> them.\n\nMy .travis.yml suggestion was about testing with SHA1DC in\npreparation for making it the default.  That would give us another\nincentive to keep an eye on its performance, too, before we make it\nthe default in Makefile, at which time the forced selection in the\ntravis configuration can be removed.\n\nThanks.\n\n\n"},{"id":"314305","messageId":"xmqqh92sa31r.fsf@gitster.mtv.corp.google.com","threadId":"45399","inReplyTo":"20170317001115.xau6nzp6i7ilkqxv@sigill.intra.peff.net","subject":"Re: [PATCH 4/5] Makefile: add USE_SHA1DC knob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-17T05:24:32Z","receivedAt":"2017-03-17T05:24:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Mar 16, 2017 at 03:43:13PM -0700, Junio C Hamano wrote:\n>\n>> Jeff King <peff@peff.net> writes:\n>> \n>> > +ifdef USE_SHA1DC\n>> > +\tLIB_OBJS += sha1dc/sha1.o\n>> > +\tLIB_OBJS += sha1dc/ubc_check.o\n>> > +\tBASIC_CFLAGS += -DSHA1_SHA1DC\n>> \n>> The name of this CPP symbol is one difference between this and\n>> Linus's version.  Wouldn't \"-DSHA1_DC\" make more sense?\n>\n> I'm fine with either. Somehow SHA1_DC felt too short, but it doesn't\n> really matter in practice.\n\nI'm fine with either, too.  It was just double SHA1 felt a bit\nstrange, when the naming convention was SHA1_ followed by the\ncharacteristic attribute of the implementation (e.g. came from\nMozilla, etc.) and I thought \"Detecting Collision\" was the notable\ncharacteristic of this one.\n\n>> Another difference is that your version adds USE_SHA1DC to\n>> GIT-BUILD-OPTIONS in patch 5/5; I thought GIT-CFLAGS forces\n>> rebuilding and that was sufficient, but GIT-BUILD-OPTIONS is\n>> available to tests for introspection, so adding it is needed\n>> for that reason.\n>\n> Yep, exactly.\n\nThanks.\n"},{"id":"314322","messageId":"20170317112228.rvqddumn5zairmh3@sigill.intra.peff.net","threadId":"45399","inReplyTo":"xmqqlgs4a35n.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/2] Re-integrate sha1dc","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-17T11:22:29Z","receivedAt":"2017-03-17T11:24:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 16, 2017 at 10:22:12PM -0700, Junio C Hamano wrote:\n\n> My .travis.yml suggestion was about testing with SHA1DC in\n> preparation for making it the default.  That would give us another\n> incentive to keep an eye on its performance, too, before we make it\n> the default in Makefile, at which time the forced selection in the\n> travis configuration can be removed.\n\nYeah, I'm not opposed to that, though it requires somebody actually\npaying attention to the timings differences. I'm not sure we can do that\nautomatically on Travis.\n\n-Peff\n"},{"id":"314326","messageId":"20170317111814.tkzeqfyr3aiyxsxr@sigill.intra.peff.net","threadId":"45399","inReplyTo":"xmqqh92sa31r.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 4/5] Makefile: add USE_SHA1DC knob","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-17T11:18:14Z","receivedAt":"2017-03-17T12:20:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 16, 2017 at 10:24:32PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > On Thu, Mar 16, 2017 at 03:43:13PM -0700, Junio C Hamano wrote:\n> >\n> >> Jeff King <peff@peff.net> writes:\n> >> \n> >> > +ifdef USE_SHA1DC\n> >> > +\tLIB_OBJS += sha1dc/sha1.o\n> >> > +\tLIB_OBJS += sha1dc/ubc_check.o\n> >> > +\tBASIC_CFLAGS += -DSHA1_SHA1DC\n> >> \n> >> The name of this CPP symbol is one difference between this and\n> >> Linus's version.  Wouldn't \"-DSHA1_DC\" make more sense?\n> >\n> > I'm fine with either. Somehow SHA1_DC felt too short, but it doesn't\n> > really matter in practice.\n> \n> I'm fine with either, too.  It was just double SHA1 felt a bit\n> strange, when the naming convention was SHA1_ followed by the\n> characteristic attribute of the implementation (e.g. came from\n> Mozilla, etc.) and I thought \"Detecting Collision\" was the notable\n> characteristic of this one.\n\nYeah, I thought \"DC\" by itself was funny, but I guess \"BLK\" by itself is\nno less funny. I'm happy to re-roll it if you prefer the other.\n\n-Peff\n"},{"id":"314349","messageId":"20170317170938.20593-2-gitster@pobox.com","threadId":"45399","inReplyTo":"20170317170938.20593-1-gitster@pobox.com","subject":"[PATCH 1/3] Makefile: add DC_SHA1 knob","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-17T17:09:36Z","receivedAt":"2017-03-17T17:09:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Jeff King <peff@peff.net>\n\nThis knob lets you use the sha1dc implementation from:\n\n      https://github.com/cr-marcstevens/sha1collisiondetection\n\nwhich can detect certain types of collision attacks (even\nwhen we only see half of the colliding pair). So it\nmitigates any attack which consists of getting the \"good\"\nhalf of a collision into a trusted repository, and then\nlater replacing it with the \"bad\" half. The \"good\" half is\nrejected by the victim's version of Git (and even if they\nrun an old version of Git, any sha1dc-enabled git will\ncomplain loudly if it ever has to interact with the object).\n\nThe big downside is that it's slower than either the openssl\nor block-sha1 implementations.\n\nHere are some timings based off of linux.git:\n\n  - compute sha1 over whole packfile\n      sha1dc: 3.580s\n    blk-sha1: 2.046s (-43%)\n     openssl: 1.335s (-62%)\n\n  - rev-list --all --objects\n      sha1dc: 33.512s\n    blk-sha1: 33.514s (+0.0%)\n     openssl: 33.650s (+0.4%)\n\n  - git log --no-merges -10000 -p\n      sha1dc: 8.124s\n    blk-sha1: 7.986s (-1.6%)\n     openssl: 8.203s (+0.9%)\n\n  - index-pack --verify\n      sha1dc: 4m19s\n    blk-sha1: 2m57s (-32%)\n     openssl: 2m19s (-42%)\n\nSo overall the sha1 computation with collision detection is\nabout 1.75x slower than block-sha1, and 2.7x slower than\nsha1. But of course most operations do more than just sha1.\nNormal object access isn't really slowed at all (both the\n+/- changes there are well within the run-to-run noise); any\nchanges are drowned out by the other work Git is doing.\n\nThe most-affected operation is `index-pack --verify`, which\nis essentially just computing the sha1 on every object. This\nis similar to the `index-pack` invocation that the receiver\nof a push or fetch would perform. So clearly there's some\nextra CPU load here.\n\nThere will also be some latency for the user, though keep in\nmind that such an operation will generally be network bound\n(this is about a 1.2GB packfile). Some of that extra CPU is\n\"free\" in the sense that we use it while the pack is\nstreaming in anyway. But most of it comes during the\ndelta-resolution phase, after the whole pack has been\nreceived. So we can imagine that for this (quite large)\npush, the user might have to wait an extra 100 seconds over\nopenssl (which is what we use now). If we assume they can\npush to us at 20Mbit/s, that's 480s for a 1.2GB pack, which\nis only 20% slower.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Makefile      | 10 ++++++++++\n hash.h        |  2 ++\n sha1dc/sha1.c | 20 ++++++++++++++++++++\n sha1dc/sha1.h | 15 +++++++++++++++\n 4 files changed, 47 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex 25c21f08b1..05a96d7177 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -142,6 +142,10 @@ all::\n # Define PPC_SHA1 environment variable when running make to make use of\n # a bundled SHA1 routine optimized for PowerPC.\n #\n+# Define DC_SHA1 to unconditionally enable the collision-detecting sha1\n+# algorithm. This is slower, but may detect attempted collision attacks.\n+# Takes priority over other *_SHA1 knobs.\n+#\n # Define SHA1_MAX_BLOCK_SIZE to limit the amount of data that will be hashed\n # in one call to the platform's SHA1_Update(). e.g. APPLE_COMMON_CRYPTO\n # wants 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined.\n@@ -1386,6 +1390,11 @@ ifdef APPLE_COMMON_CRYPTO\n \tSHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L\n endif\n \n+ifdef DC_SHA1\n+\tLIB_OBJS += sha1dc/sha1.o\n+\tLIB_OBJS += sha1dc/ubc_check.o\n+\tBASIC_CFLAGS += -DSHA1_DC\n+else\n ifdef BLK_SHA1\n \tLIB_OBJS += block-sha1/sha1.o\n \tBASIC_CFLAGS += -DSHA1_BLK\n@@ -1403,6 +1412,7 @@ else\n endif\n endif\n endif\n+endif\n \n ifdef SHA1_MAX_BLOCK_SIZE\n \tLIB_OBJS += compat/sha1-chunked.o\ndiff --git a/hash.h b/hash.h\nindex f0d9ddd0c2..a11fc9233f 100644\n--- a/hash.h\n+++ b/hash.h\n@@ -7,6 +7,8 @@\n #include <CommonCrypto/CommonDigest.h>\n #elif defined(SHA1_OPENSSL)\n #include <openssl/sha.h>\n+#elif defined(SHA1_DC)\n+#include \"sha1dc/sha1.h\"\n #else /* SHA1_BLK */\n #include \"block-sha1/sha1.h\"\n #endif\ndiff --git a/sha1dc/sha1.c b/sha1dc/sha1.c\nindex 8ff2321dfb..6dd0da3608 100644\n--- a/sha1dc/sha1.c\n+++ b/sha1dc/sha1.c\n@@ -1786,3 +1786,23 @@ int SHA1DCFinal(unsigned char output[20], SHA1_CTX *ctx)\n \toutput[19] = (unsigned char)(ctx->ihv[4]);\n \treturn ctx->found_collision;\n }\n+\n+void git_SHA1DCFinal(unsigned char hash[20], SHA1_CTX *ctx)\n+{\n+\tif (!SHA1DCFinal(hash, ctx))\n+\t\treturn;\n+\tdie(\"SHA-1 appears to be part of a collision attack: %s\",\n+\t    sha1_to_hex(hash));\n+}\n+\n+void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *vdata, unsigned long len)\n+{\n+\tconst char *data = vdata;\n+\t/* We expect an unsigned long, but sha1dc only takes an int */\n+\twhile (len > INT_MAX) {\n+\t\tSHA1DCUpdate(ctx, data, INT_MAX);\n+\t\tdata += INT_MAX;\n+\t\tlen -= INT_MAX;\n+\t}\n+\tSHA1DCUpdate(ctx, data, len);\n+}\ndiff --git a/sha1dc/sha1.h b/sha1dc/sha1.h\nindex 7d4d423b9d..bd8bd928fb 100644\n--- a/sha1dc/sha1.h\n+++ b/sha1dc/sha1.h\n@@ -100,6 +100,21 @@ void SHA1DCUpdate(SHA1_CTX*, const char*, size_t);\n /* returns: 0 = no collision detected, otherwise = collision found => warn user for active attack */\n int  SHA1DCFinal(unsigned char[20], SHA1_CTX*);\n \n+/*\n+ * Same as SHA1DCFinal, but convert collision attack case into a verbose die().\n+ */\n+void git_SHA1DCFinal(unsigned char [20], SHA1_CTX *);\n+\n+/*\n+ * Same as SHA1DCUpdate, but adjust types to match git's usual interface.\n+ */\n+void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *data, unsigned long len);\n+\n+#define platform_SHA_CTX SHA1_CTX\n+#define platform_SHA1_Init SHA1DCInit\n+#define platform_SHA1_Update git_SHA1DCUpdate\n+#define platform_SHA1_Final git_SHA1DCFinal\n+\n #if defined(__cplusplus)\n }\n #endif\n-- \n2.12.0-317-g32c43f595f\n\n"},{"id":"314350","messageId":"20170317170938.20593-1-gitster@pobox.com","threadId":"45399","inReplyTo":"20170317111814.tkzeqfyr3aiyxsxr@sigill.intra.peff.net","subject":"[RFC PATCH 0/3] Git integration update for DC-SHA1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-17T17:09:35Z","receivedAt":"2017-03-17T17:09:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Here are three patches to replace the last two patches from your\nseries.\n\n - The Makefile knob is named DC_SHA1, not USE_SHA1DC; this is to\n   keep it consistent with existing BLK_SHA1 and PPC_SHA1.\n\n - The CPP macro is called SHA1_DC, not SHA1_SHA1DC; again this is\n   for consistency with SHA1_BLK and SHA1_PPC.\n\n - Switch the default from OpenSSL's implementation to DC_SHA1.\n   Those who want OpenSSL's one can ask with OPENSSL_SHA1.\n\nJeff King (2):\n  Makefile: add DC_SHA1 knob\n  t0013: add a basic sha1 collision detection test\n\nJunio C Hamano (1):\n  Makefile: make DC_SHA1 the default\n\n Makefile                |  19 +++++++++++++++++--\n hash.h                  |   2 ++\n sha1dc/sha1.c           |  20 ++++++++++++++++++++\n sha1dc/sha1.h           |  15 +++++++++++++++\n t/t0013-sha1dc.sh       |  19 +++++++++++++++++++\n t/t0013/shattered-1.pdf | Bin 0 -> 422435 bytes\n 6 files changed, 73 insertions(+), 2 deletions(-)\n create mode 100755 t/t0013-sha1dc.sh\n create mode 100644 t/t0013/shattered-1.pdf\n\n-- \n2.12.0-317-g32c43f595f\n\n"},{"id":"314351","messageId":"20170317170938.20593-4-gitster@pobox.com","threadId":"45399","inReplyTo":"20170317170938.20593-1-gitster@pobox.com","subject":"[PATCH 3/3] Makefile: make DC_SHA1 the default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-17T17:09:38Z","receivedAt":"2017-03-17T17:09:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We used to use the SHA1 implementation from the OpenSSL library by\ndefault.  As we are trying to be careful against collision attacks\nafter the recent \"shattered\" announcement, switch the default to\nencourage people to use DC_SHA1 implementation instead.  Those who\nwant to use the implementation from OpenSSL can explicitly ask for\nit by OPENSSL_SHA1=YesPlease when running \"make\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Makefile | 16 ++++++++++------\n 1 file changed, 10 insertions(+), 6 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex fc9d89498b..fd4421eeb8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -146,6 +146,9 @@ all::\n # algorithm. This is slower, but may detect attempted collision attacks.\n # Takes priority over other *_SHA1 knobs.\n #\n+# Define OPENSSL_SHA1 environment variable when running make to link\n+# with the SHA1 routine from openssl library.\n+#\n # Define SHA1_MAX_BLOCK_SIZE to limit the amount of data that will be hashed\n # in one call to the platform's SHA1_Update(). e.g. APPLE_COMMON_CRYPTO\n # wants 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined.\n@@ -1390,10 +1393,9 @@ ifdef APPLE_COMMON_CRYPTO\n \tSHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L\n endif\n \n-ifdef DC_SHA1\n-\tLIB_OBJS += sha1dc/sha1.o\n-\tLIB_OBJS += sha1dc/ubc_check.o\n-\tBASIC_CFLAGS += -DSHA1_DC\n+ifdef OPENSSL_SHA1\n+\tEXTLIBS += $(LIB_4_CRYPTO)\n+\tBASIC_CFLAGS += -DSHA1_OPENSSL\n else\n ifdef BLK_SHA1\n \tLIB_OBJS += block-sha1/sha1.o\n@@ -1407,8 +1409,10 @@ ifdef APPLE_COMMON_CRYPTO\n \tCOMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL\n \tBASIC_CFLAGS += -DSHA1_APPLE\n else\n-\tEXTLIBS += $(LIB_4_CRYPTO)\n-\tBASIC_CFLAGS += -DSHA1_OPENSSL\n+\tDC_SHA1 := YesPlease\n+\tLIB_OBJS += sha1dc/sha1.o\n+\tLIB_OBJS += sha1dc/ubc_check.o\n+\tBASIC_CFLAGS += -DSHA1_DC\n endif\n endif\n endif\n-- \n2.12.0-317-g32c43f595f\n\n"},{"id":"314362","messageId":"xmqq4lyr94wt.fsf@gitster.mtv.corp.google.com","threadId":"45399","inReplyTo":"20170317170938.20593-1-gitster@pobox.com","subject":"Re: [RFC PATCH 0/3] Git integration update for DC-SHA1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-17T17:41:54Z","receivedAt":"2017-03-17T17:42:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Here are three patches to replace the last two patches from your\n> series.\n>\n>  - The Makefile knob is named DC_SHA1, not USE_SHA1DC; this is to\n>    keep it consistent with existing BLK_SHA1 and PPC_SHA1.\n>\n>  - The CPP macro is called SHA1_DC, not SHA1_SHA1DC; again this is\n>    for consistency with SHA1_BLK and SHA1_PPC.\n>\n>  - Switch the default from OpenSSL's implementation to DC_SHA1.\n>    Those who want OpenSSL's one can ask with OPENSSL_SHA1.\n>\n> Jeff King (2):\n>   Makefile: add DC_SHA1 knob\n>   t0013: add a basic sha1 collision detection test\n>\n> Junio C Hamano (1):\n>   Makefile: make DC_SHA1 the default\n>\n>  Makefile                |  19 +++++++++++++++++--\n>  hash.h                  |   2 ++\n>  sha1dc/sha1.c           |  20 ++++++++++++++++++++\n>  sha1dc/sha1.h           |  15 +++++++++++++++\n>  t/t0013-sha1dc.sh       |  19 +++++++++++++++++++\n>  t/t0013/shattered-1.pdf | Bin 0 -> 422435 bytes\n>  6 files changed, 73 insertions(+), 2 deletions(-)\n>  create mode 100755 t/t0013-sha1dc.sh\n>  create mode 100644 t/t0013/shattered-1.pdf\n\nFor a rather obvious reason, patch 2/3 cannot be seen on the list.\nThe \"interdiff\" between jk/sha1dc topic and applying patches 1 and 2\n(but not 3) looks like this.\n\ndiff --git a/Makefile b/Makefile\nindex b01111c581..fc9d89498b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -142,7 +142,7 @@ all::\n # Define PPC_SHA1 environment variable when running make to make use of\n # a bundled SHA1 routine optimized for PowerPC.\n #\n-# Define USE_SHA1DC to unconditionally enable the collision-detecting sha1\n+# Define DC_SHA1 to unconditionally enable the collision-detecting sha1\n # algorithm. This is slower, but may detect attempted collision attacks.\n # Takes priority over other *_SHA1 knobs.\n #\n@@ -1390,10 +1390,10 @@ ifdef APPLE_COMMON_CRYPTO\n \tSHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L\n endif\n \n-ifdef USE_SHA1DC\n+ifdef DC_SHA1\n \tLIB_OBJS += sha1dc/sha1.o\n \tLIB_OBJS += sha1dc/ubc_check.o\n-\tBASIC_CFLAGS += -DSHA1_SHA1DC\n+\tBASIC_CFLAGS += -DSHA1_DC\n else\n ifdef BLK_SHA1\n \tLIB_OBJS += block-sha1/sha1.o\n@@ -2236,7 +2236,7 @@ GIT-BUILD-OPTIONS: FORCE\n \t@echo NO_PYTHON=\\''$(subst ','\\'',$(subst ','\\'',$(NO_PYTHON)))'\\' >>$@+\n \t@echo NO_UNIX_SOCKETS=\\''$(subst ','\\'',$(subst ','\\'',$(NO_UNIX_SOCKETS)))'\\' >>$@+\n \t@echo PAGER_ENV=\\''$(subst ','\\'',$(subst ','\\'',$(PAGER_ENV)))'\\' >>$@+\n-\t@echo USE_SHA1DC=\\''$(subst ','\\'',$(subst ','\\'',$(USE_SHA1DC)))'\\' >>$@+\n+\t@echo DC_SHA1=\\''$(subst ','\\'',$(subst ','\\'',$(DC_SHA1)))'\\' >>$@+\n ifdef TEST_OUTPUT_DIRECTORY\n \t@echo TEST_OUTPUT_DIRECTORY=\\''$(subst ','\\'',$(subst ','\\'',$(TEST_OUTPUT_DIRECTORY)))'\\' >>$@+\n endif\ndiff --git a/hash.h b/hash.h\nindex 3760f436ec..a11fc9233f 100644\n--- a/hash.h\n+++ b/hash.h\n@@ -7,7 +7,7 @@\n #include <CommonCrypto/CommonDigest.h>\n #elif defined(SHA1_OPENSSL)\n #include <openssl/sha.h>\n-#elif defined(SHA1_SHA1DC)\n+#elif defined(SHA1_DC)\n #include \"sha1dc/sha1.h\"\n #else /* SHA1_BLK */\n #include \"block-sha1/sha1.h\"\ndiff --git a/t/t0013-sha1dc.sh b/t/t0013-sha1dc.sh\nindex 4d43f7bf64..6d655cb161 100755\n--- a/t/t0013-sha1dc.sh\n+++ b/t/t0013-sha1dc.sh\n@@ -4,9 +4,9 @@ test_description='test sha1 collision detection'\n . ./test-lib.sh\n TEST_DATA=\"$TEST_DIRECTORY/t0013\"\n \n-if test -z \"$USE_SHA1DC\"\n+if test -z \"$DC_SHA1\"\n then\n-\tskip_all='skipping sha1 collision tests, USE_SHA1DC not set'\n+\tskip_all='skipping sha1 collision tests, DC_SHA1 not set'\n \ttest_done\n fi\n \n\n\n\n"},{"id":"314366","messageId":"20170317174549.54jci3jpw6maj2xt@sigill.intra.peff.net","threadId":"45399","inReplyTo":"20170317170938.20593-1-gitster@pobox.com","subject":"Re: [RFC PATCH 0/3] Git integration update for DC-SHA1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-17T17:45:49Z","receivedAt":"2017-03-17T17:53:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 17, 2017 at 10:09:35AM -0700, Junio C Hamano wrote:\n\n> Here are three patches to replace the last two patches from your\n> series.\n> \n>  - The Makefile knob is named DC_SHA1, not USE_SHA1DC; this is to\n>    keep it consistent with existing BLK_SHA1 and PPC_SHA1.\n> \n>  - The CPP macro is called SHA1_DC, not SHA1_SHA1DC; again this is\n>    for consistency with SHA1_BLK and SHA1_PPC.\n> \n>  - Switch the default from OpenSSL's implementation to DC_SHA1.\n>    Those who want OpenSSL's one can ask with OPENSSL_SHA1.\n\nOK. DC_SHA1 exposed tothe user does look a little funny to me, but it is\nat least consistent with the other names. The patches themselves look\nobviously fine.\n\n-Peff\n"}]}