From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>,
Linus Torvalds <torvalds@linux-foundation.org>
Subject: [PATCH 1/3] Makefile: add DC_SHA1 knob
Date: Fri, 17 Mar 2017 10:09:36 -0700 [thread overview]
Message-ID: <20170317170938.20593-2-gitster@pobox.com> (raw)
In-Reply-To: <20170317170938.20593-1-gitster@pobox.com>
From: Jeff King <peff@peff.net>
This knob lets you use the sha1dc implementation from:
https://github.com/cr-marcstevens/sha1collisiondetection
which can detect certain types of collision attacks (even
when we only see half of the colliding pair). So it
mitigates any attack which consists of getting the "good"
half of a collision into a trusted repository, and then
later replacing it with the "bad" half. The "good" half is
rejected by the victim's version of Git (and even if they
run an old version of Git, any sha1dc-enabled git will
complain loudly if it ever has to interact with the object).
The big downside is that it's slower than either the openssl
or block-sha1 implementations.
Here are some timings based off of linux.git:
- compute sha1 over whole packfile
sha1dc: 3.580s
blk-sha1: 2.046s (-43%)
openssl: 1.335s (-62%)
- rev-list --all --objects
sha1dc: 33.512s
blk-sha1: 33.514s (+0.0%)
openssl: 33.650s (+0.4%)
- git log --no-merges -10000 -p
sha1dc: 8.124s
blk-sha1: 7.986s (-1.6%)
openssl: 8.203s (+0.9%)
- index-pack --verify
sha1dc: 4m19s
blk-sha1: 2m57s (-32%)
openssl: 2m19s (-42%)
So overall the sha1 computation with collision detection is
about 1.75x slower than block-sha1, and 2.7x slower than
sha1. But of course most operations do more than just sha1.
Normal object access isn't really slowed at all (both the
+/- changes there are well within the run-to-run noise); any
changes are drowned out by the other work Git is doing.
The most-affected operation is `index-pack --verify`, which
is essentially just computing the sha1 on every object. This
is similar to the `index-pack` invocation that the receiver
of a push or fetch would perform. So clearly there's some
extra CPU load here.
There will also be some latency for the user, though keep in
mind that such an operation will generally be network bound
(this is about a 1.2GB packfile). Some of that extra CPU is
"free" in the sense that we use it while the pack is
streaming in anyway. But most of it comes during the
delta-resolution phase, after the whole pack has been
received. So we can imagine that for this (quite large)
push, the user might have to wait an extra 100 seconds over
openssl (which is what we use now). If we assume they can
push to us at 20Mbit/s, that's 480s for a 1.2GB pack, which
is only 20% slower.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Makefile | 10 ++++++++++
hash.h | 2 ++
sha1dc/sha1.c | 20 ++++++++++++++++++++
sha1dc/sha1.h | 15 +++++++++++++++
4 files changed, 47 insertions(+)
diff --git a/Makefile b/Makefile
index 25c21f08b1..05a96d7177 100644
--- a/Makefile
+++ b/Makefile
@@ -142,6 +142,10 @@ all::
# Define PPC_SHA1 environment variable when running make to make use of
# a bundled SHA1 routine optimized for PowerPC.
#
+# Define DC_SHA1 to unconditionally enable the collision-detecting sha1
+# algorithm. This is slower, but may detect attempted collision attacks.
+# Takes priority over other *_SHA1 knobs.
+#
# Define SHA1_MAX_BLOCK_SIZE to limit the amount of data that will be hashed
# in one call to the platform's SHA1_Update(). e.g. APPLE_COMMON_CRYPTO
# wants 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined.
@@ -1386,6 +1390,11 @@ ifdef APPLE_COMMON_CRYPTO
SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
endif
+ifdef DC_SHA1
+ LIB_OBJS += sha1dc/sha1.o
+ LIB_OBJS += sha1dc/ubc_check.o
+ BASIC_CFLAGS += -DSHA1_DC
+else
ifdef BLK_SHA1
LIB_OBJS += block-sha1/sha1.o
BASIC_CFLAGS += -DSHA1_BLK
@@ -1403,6 +1412,7 @@ else
endif
endif
endif
+endif
ifdef SHA1_MAX_BLOCK_SIZE
LIB_OBJS += compat/sha1-chunked.o
diff --git a/hash.h b/hash.h
index f0d9ddd0c2..a11fc9233f 100644
--- a/hash.h
+++ b/hash.h
@@ -7,6 +7,8 @@
#include <CommonCrypto/CommonDigest.h>
#elif defined(SHA1_OPENSSL)
#include <openssl/sha.h>
+#elif defined(SHA1_DC)
+#include "sha1dc/sha1.h"
#else /* SHA1_BLK */
#include "block-sha1/sha1.h"
#endif
diff --git a/sha1dc/sha1.c b/sha1dc/sha1.c
index 8ff2321dfb..6dd0da3608 100644
--- a/sha1dc/sha1.c
+++ b/sha1dc/sha1.c
@@ -1786,3 +1786,23 @@ int SHA1DCFinal(unsigned char output[20], SHA1_CTX *ctx)
output[19] = (unsigned char)(ctx->ihv[4]);
return ctx->found_collision;
}
+
+void git_SHA1DCFinal(unsigned char hash[20], SHA1_CTX *ctx)
+{
+ if (!SHA1DCFinal(hash, ctx))
+ return;
+ die("SHA-1 appears to be part of a collision attack: %s",
+ sha1_to_hex(hash));
+}
+
+void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *vdata, unsigned long len)
+{
+ const char *data = vdata;
+ /* We expect an unsigned long, but sha1dc only takes an int */
+ while (len > INT_MAX) {
+ SHA1DCUpdate(ctx, data, INT_MAX);
+ data += INT_MAX;
+ len -= INT_MAX;
+ }
+ SHA1DCUpdate(ctx, data, len);
+}
diff --git a/sha1dc/sha1.h b/sha1dc/sha1.h
index 7d4d423b9d..bd8bd928fb 100644
--- a/sha1dc/sha1.h
+++ b/sha1dc/sha1.h
@@ -100,6 +100,21 @@ void SHA1DCUpdate(SHA1_CTX*, const char*, size_t);
/* returns: 0 = no collision detected, otherwise = collision found => warn user for active attack */
int SHA1DCFinal(unsigned char[20], SHA1_CTX*);
+/*
+ * Same as SHA1DCFinal, but convert collision attack case into a verbose die().
+ */
+void git_SHA1DCFinal(unsigned char [20], SHA1_CTX *);
+
+/*
+ * Same as SHA1DCUpdate, but adjust types to match git's usual interface.
+ */
+void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *data, unsigned long len);
+
+#define platform_SHA_CTX SHA1_CTX
+#define platform_SHA1_Init SHA1DCInit
+#define platform_SHA1_Update git_SHA1DCUpdate
+#define platform_SHA1_Final git_SHA1DCFinal
+
#if defined(__cplusplus)
}
#endif
--
2.12.0-317-g32c43f595f
next prev parent reply other threads:[~2017-03-17 17:09 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
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 ` Junio C Hamano [this message]
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170317170938.20593-2-gitster@pobox.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
--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.
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 NNTP newsgroup(s).