threads / patch / 46589

v2, 2 partssha1dc: Allow building with the external sha1dc library

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

## tl;dr

4 messages between Aug 15, 2017 and Aug 16, 2017. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Takashi Iwai· Aug 15, 2017, 12:04 UTC · lore

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

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
Takashi Iwai· Aug 15, 2017, 12:04 UTC · re: Takashi Iwai · lore

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(-)
Show changes to 3 files +33 −1

Makefile, sha1dc_git.c, sha1dc_git.h

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
Takashi Iwai· Aug 15, 2017, 12:04 UTC · re: Takashi Iwai · lore

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

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(-)
Show changes to 4 files +16 −22

Makefile, hash.h, sha1dc_git.c, sha1dc_git.h

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
Junio C Hamano· Aug 16, 2017, 21:47 UTC · re: Takashi Iwai · lore

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

Takashi Iwai <tiwai@suse.de> writes:
Show 14 quoted lines
> 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.

← back to recent threads