[PATCH v2 0/2] Allow building with the external sha1dc library

[PATCH v2 0/2] Allow building with the external sha1dc library

[Takashi Iwai]

Hi,

this is the second attempt to allow linking with the external sha1dc
shlib.  Now I split to two patches: one for cleaning up of sha1dc
plumbing codes, and another for adding the option to link with the
external sha1dc lib.

Other changes from v1:
- Plumbing codes for external lib are also merged commonly in
  sha1dc_git.[ch]
- Check the conflict of extlib vs submodule
- Drop DC_SHA1_LINK, hoping that everyone is well-mannered
- Minor rephrasing / corrections of texts


thanks,

Takashi

===

Takashi Iwai (2):
  sha1dc: Build git plumbing code more explicitly
  sha1dc: Allow building with the external sha1dc library

 Makefile     | 18 +++++++++++++++---
 hash.h       |  6 +-----
 sha1dc_git.c | 18 ++++++++++++++++--
 sha1dc_git.h | 28 ++++++++++++++++------------
 4 files changed, 48 insertions(+), 22 deletions(-)

-- 
2.14.1

[PATCH v2 1/2] sha1dc: Build git plumbing code more explicitly

[Takashi Iwai]

The plumbing code between sha1dc and git is defined in
sha1dc_git.[ch], but these aren't compiled / included directly but
only via the indirect inclusion from sha1dc code.  This is slightly
confusing when you try to trace the build flow.

This patch brings the following changes for simplification:
- Make sha1dc_git.c stand-alone and build from Makefile
- sha1dc_git.h is the common header to include further sha1.h
  depending on the build condition
- Move comments for plumbing codes from the header to definitions

This is also meant as a preliminary work for further plumbing with
external sha1dc shlib.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 Makefile     |  5 ++---
 hash.h       |  6 +-----
 sha1dc_git.c |  9 ++++++---
 sha1dc_git.h | 18 +++++++-----------
 4 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/Makefile b/Makefile
index 461c845d33cb..5e7e9022bdd8 100644
--- a/Makefile
+++ b/Makefile
@@ -1472,6 +1472,8 @@ ifdef APPLE_COMMON_CRYPTO
 	BASIC_CFLAGS += -DSHA1_APPLE
 else
 	DC_SHA1 := YesPlease
+	BASIC_CFLAGS += -DSHA1_DC
+	LIB_OBJS += sha1dc_git.o
 ifdef DC_SHA1_SUBMODULE
 	LIB_OBJS += sha1collisiondetection/lib/sha1.o
 	LIB_OBJS += sha1collisiondetection/lib/ubc_check.o
@@ -1481,12 +1483,9 @@ else
 	LIB_OBJS += sha1dc/ubc_check.o
 endif
 	BASIC_CFLAGS += \
-		-DSHA1_DC \
 		-DSHA1DC_NO_STANDARD_INCLUDES \
 		-DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 \
 		-DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" \
-		-DSHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_C="\"sha1dc_git.c\"" \
-		-DSHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_H="\"sha1dc_git.h\"" \
 		-DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\""
 endif
 endif
diff --git a/hash.h b/hash.h
index bef3e630a093..024d0d3d50b1 100644
--- a/hash.h
+++ b/hash.h
@@ -8,11 +8,7 @@
 #elif defined(SHA1_OPENSSL)
 #include <openssl/sha.h>
 #elif defined(SHA1_DC)
-#ifdef DC_SHA1_SUBMODULE
-#include "sha1collisiondetection/lib/sha1.h"
-#else
-#include "sha1dc/sha1.h"
-#endif
+#include "sha1dc_git.h"
 #else /* SHA1_BLK */
 #include "block-sha1/sha1.h"
 #endif
diff --git a/sha1dc_git.c b/sha1dc_git.c
index 4d32b4f77e04..79466414f841 100644
--- a/sha1dc_git.c
+++ b/sha1dc_git.c
@@ -1,8 +1,8 @@
+#include "cache.h"
+
 /*
- * This code is included at the end of sha1dc/sha1.c with the
- * SHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_C macro.
+ * Same as SHA1DCFinal, but convert collision attack case into a verbose die().
  */
-
 void git_SHA1DCFinal(unsigned char hash[20], SHA1_CTX *ctx)
 {
 	if (!SHA1DCFinal(hash, ctx))
@@ -11,6 +11,9 @@ void git_SHA1DCFinal(unsigned char hash[20], SHA1_CTX *ctx)
 	    sha1_to_hex(hash));
 }
 
+/*
+ * Same as SHA1DCUpdate, but adjust types to match git's usual interface.
+ */
 void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *vdata, unsigned long len)
 {
 	const char *data = vdata;
diff --git a/sha1dc_git.h b/sha1dc_git.h
index a8a5c1da169e..af3e9514bc8e 100644
--- a/sha1dc_git.h
+++ b/sha1dc_git.h
@@ -1,16 +1,12 @@
-/*
- * This code is included at the end of sha1dc/sha1.h with the
- * SHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_H macro.
- */
+/* Plumbing with collition-detecting SHA1 code */
 
-/*
- * Same as SHA1DCFinal, but convert collision attack case into a verbose die().
- */
-void git_SHA1DCFinal(unsigned char [20], SHA1_CTX *);
+#ifdef DC_SHA1_SUBMODULE
+#include "sha1collisiondetection/lib/sha1.h"
+#else
+#include "sha1dc/sha1.h"
+#endif
 
-/*
- * Same as SHA1DCUpdate, but adjust types to match git's usual interface.
- */
+void git_SHA1DCFinal(unsigned char [20], SHA1_CTX *);
 void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *data, unsigned long len);
 
 #define platform_SHA_CTX SHA1_CTX
-- 
2.14.1

[PATCH v2 2/2] sha1dc: Allow building with the external sha1dc library

[Takashi Iwai]

Some distros provide SHA1 collision-detect code as a shared library.
It's the same code as we have in git tree (but may be with a different
init default for hash), and git can link with it as well; at least, it
may make maintenance easier, according to our security guys.

This patch allows user to build git linking with the external sha1dc
library instead of the built-in code.  User needs to define
DC_SHA1_EXTERNAL explicitly.  As default without it, the built-in
sha1dc code is used like before.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 Makefile     | 13 +++++++++++++
 sha1dc_git.c | 11 +++++++++++
 sha1dc_git.h | 10 +++++++++-
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 5e7e9022bdd8..9f492b5d1d37 100644
--- a/Makefile
+++ b/Makefile
@@ -162,6 +162,11 @@ all::
 # algorithm. This is slower, but may detect attempted collision attacks.
 # Takes priority over other *_SHA1 knobs.
 #
+# Define DC_SHA1_EXTERNAL in addition to DC_SHA1 if you want to build / link
+# git with the external SHA1 collision-detect library.
+# Without this option, i.e. the default behavior is to build git with its
+# own built-in code (or submodule).
+#
 # Define DC_SHA1_SUBMODULE in addition to DC_SHA1 to use the
 # sha1collisiondetection shipped as a submodule instead of the
 # non-submodule copy in sha1dc/. This is an experimental option used
@@ -1474,6 +1479,13 @@ else
 	DC_SHA1 := YesPlease
 	BASIC_CFLAGS += -DSHA1_DC
 	LIB_OBJS += sha1dc_git.o
+ifdef DC_SHA1_EXTERNAL
+	ifdef DC_SHA1_SUBMODULE
+$(error Only set DC_SHA1_EXTERNAL or DC_SHA1_SUBMODULE, not both)
+	endif
+	BASIC_CFLAGS += -DDC_SHA1_EXTERNAL
+	EXTLIBS += -lsha1detectcoll
+else
 ifdef DC_SHA1_SUBMODULE
 	LIB_OBJS += sha1collisiondetection/lib/sha1.o
 	LIB_OBJS += sha1collisiondetection/lib/ubc_check.o
@@ -1491,6 +1503,7 @@ endif
 endif
 endif
 endif
+endif
 
 ifdef SHA1_MAX_BLOCK_SIZE
 	LIB_OBJS += compat/sha1-chunked.o
diff --git a/sha1dc_git.c b/sha1dc_git.c
index 79466414f841..e0cc9d988c70 100644
--- a/sha1dc_git.c
+++ b/sha1dc_git.c
@@ -1,5 +1,16 @@
 #include "cache.h"
 
+#ifdef DC_SHA1_EXTERNAL
+/*
+ * Same as SHA1DCInit, but with default save_hash=0
+ */
+void git_SHA1DCInit(SHA1_CTX *ctx)
+{
+	SHA1DCInit(ctx);
+	SHA1DCSetSafeHash(ctx, 0);
+}
+#endif
+
 /*
  * Same as SHA1DCFinal, but convert collision attack case into a verbose die().
  */
diff --git a/sha1dc_git.h b/sha1dc_git.h
index af3e9514bc8e..a8c272927842 100644
--- a/sha1dc_git.h
+++ b/sha1dc_git.h
@@ -2,14 +2,22 @@
 
 #ifdef DC_SHA1_SUBMODULE
 #include "sha1collisiondetection/lib/sha1.h"
+#elif defined(DC_SHA1_EXTERNAL)
+#include <sha1dc/sha1.h>
 #else
 #include "sha1dc/sha1.h"
 #endif
 
+#ifdef DC_SHA1_EXTERNAL
+void git_SHA1DCInit(SHA1_CTX *);
+#else
+#define git_SHA1DCInit	SHA1DCInit
+#endif
+
 void git_SHA1DCFinal(unsigned char [20], SHA1_CTX *);
 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_Init git_SHA1DCInit
 #define platform_SHA1_Update git_SHA1DCUpdate
 #define platform_SHA1_Final git_SHA1DCFinal
-- 
2.14.1

Re: [PATCH v2 0/2] Allow building with the external sha1dc library

[Junio C Hamano]

Takashi Iwai <tiwai@suse.de> writes:

> this is the second attempt to allow linking with the external sha1dc
> shlib.  Now I split to two patches: one for cleaning up of sha1dc
> plumbing codes, and another for adding the option to link with the
> external sha1dc lib.
>
> Other changes from v1:
> - Plumbing codes for external lib are also merged commonly in
>   sha1dc_git.[ch]
> - Check the conflict of extlib vs submodule
> - Drop DC_SHA1_LINK, hoping that everyone is well-mannered
> - Minor rephrasing / corrections of texts
>
>
> thanks,

Thank you for an update.  

I think this round addresses the concerns Ævar had with the previous
round.  Let's wait to hear from him just to be sure.