{"thread":{"id":"46589","subject":"[PATCH v2 2/2] sha1dc: Allow building with the external sha1dc library","startedAt":"2017-08-15T12:04:32Z","lastAt":"2017-08-16T21:47:18Z","messageCount":4,"participants":["Takashi Iwai","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"326388","messageId":"20170815120417.31616-3-tiwai@suse.de","threadId":"46589","inReplyTo":"20170815120417.31616-1-tiwai@suse.de","subject":"[PATCH v2 2/2] sha1dc: Allow building with the external sha1dc library","fromName":"Takashi Iwai","fromEmail":"tiwai@suse.de","sentAt":"2017-08-15T12:04:17Z","receivedAt":"2017-08-15T12:04:32Z","isPatch":true,"sender":{"key":"tiwai@suse.de","avatar":"https://avatars.githubusercontent.com/u/306482?v=4"},"body":"Some distros provide SHA1 collision-detect code as a shared library.\nIt's the same code as we have in git tree (but may be with a different\ninit default for hash), and git can link with it as well; at least, it\nmay make maintenance easier, according to our security guys.\n\nThis patch allows user to build git linking with the external sha1dc\nlibrary instead of the built-in code. User needs to define\nDC_SHA1_EXTERNAL explicitly. As default without it, the built-in\nsha1dc code is used like before.\n\nSigned-off-by: Takashi Iwai \n---\n Makefile | 13 +++++++++++++\n sha1dc_git.c | 11 +++++++++++\n sha1dc_git.h | 10 +++++++++-\n 3 files changed, 33 insertions(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 5e7e9022bdd8..9f492b5d1d37 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -162,6 +162,11 @@ all::\n # algorithm. This is slower, but may detect attempted collision attacks.\n # Takes priority over other *_SHA1 knobs.\n #\n+# Define DC_SHA1_EXTERNAL in addition to DC_SHA1 if you want to build / link\n+# git with the external SHA1 collision-detect library.\n+# Without this option, i.e. the default behavior is to build git with its\n+# own built-in code (or submodule).\n+#\n # Define DC_SHA1_SUBMODULE in addition to DC_SHA1 to use the\n # sha1collisiondetection shipped as a submodule instead of the\n # non-submodule copy in sha1dc/. This is an experimental option used\n@@ -1474,6 +1479,13 @@ else\n \tDC_SHA1 := YesPlease\n \tBASIC_CFLAGS += -DSHA1_DC\n \tLIB_OBJS += sha1dc_git.o\n+ifdef DC_SHA1_EXTERNAL\n+\tifdef DC_SHA1_SUBMODULE\n+$(error Only set DC_SHA1_EXTERNAL or DC_SHA1_SUBMODULE, not both)\n+\tendif\n+\tBASIC_CFLAGS += -DDC_SHA1_EXTERNAL\n+\tEXTLIBS += -lsha1detectcoll\n+else\n ifdef DC_SHA1_SUBMODULE\n \tLIB_OBJS += sha1collisiondetection/lib/sha1.o\n \tLIB_OBJS += sha1collisiondetection/lib/ubc_check.o\n@@ -1491,6 +1503,7 @@ endif\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/sha1dc_git.c b/sha1dc_git.c\nindex 79466414f841..e0cc9d988c70 100644\n--- a/sha1dc_git.c\n+++ b/sha1dc_git.c\n@@ -1,5 +1,16 @@\n #include \"cache.h\"\n \n+#ifdef DC_SHA1_EXTERNAL\n+/*\n+ * Same as SHA1DCInit, but with default save_hash=0\n+ */\n+void git_SHA1DCInit(SHA1_CTX *ctx)\n+{\n+\tSHA1DCInit(ctx);\n+\tSHA1DCSetSafeHash(ctx, 0);\n+}\n+#endif\n+\n /*\n * Same as SHA1DCFinal, but convert collision attack case into a verbose die().\n */\ndiff --git a/sha1dc_git.h b/sha1dc_git.h\nindex af3e9514bc8e..a8c272927842 100644\n--- a/sha1dc_git.h\n+++ b/sha1dc_git.h\n@@ -2,14 +2,22 @@\n \n #ifdef DC_SHA1_SUBMODULE\n #include \"sha1collisiondetection/lib/sha1.h\"\n+#elif defined(DC_SHA1_EXTERNAL)\n+#include \n #else\n #include \"sha1dc/sha1.h\"\n #endif\n \n+#ifdef DC_SHA1_EXTERNAL\n+void git_SHA1DCInit(SHA1_CTX *);\n+#else\n+#define git_SHA1DCInit\tSHA1DCInit\n+#endif\n+\n void git_SHA1DCFinal(unsigned char [20], SHA1_CTX *);\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_Init git_SHA1DCInit\n #define platform_SHA1_Update git_SHA1DCUpdate\n #define platform_SHA1_Final git_SHA1DCFinal\n-- \n2.14.1\n\n"},{"id":"326389","messageId":"20170815120417.31616-1-tiwai@suse.de","threadId":"46589","inReplyTo":null,"subject":"[PATCH v2 0/2] Allow building with the external sha1dc library","fromName":"Takashi Iwai","fromEmail":"tiwai@suse.de","sentAt":"2017-08-15T12:04:15Z","receivedAt":"2017-08-15T12:04:34Z","isPatch":true,"sender":{"key":"tiwai@suse.de","avatar":"https://avatars.githubusercontent.com/u/306482?v=4"},"body":"Hi,\n\nthis is the second attempt to allow linking with the external sha1dc\nshlib. Now I split to two patches: one for cleaning up of sha1dc\nplumbing codes, and another for adding the option to link with the\nexternal sha1dc lib.\n\nOther changes from v1:\n- Plumbing codes for external lib are also merged commonly in\n sha1dc_git.[ch]\n- Check the conflict of extlib vs submodule\n- Drop DC_SHA1_LINK, hoping that everyone is well-mannered\n- Minor rephrasing / corrections of texts\n\n\nthanks,\n\nTakashi\n\n===\n\nTakashi Iwai (2):\n sha1dc: Build git plumbing code more explicitly\n sha1dc: Allow building with the external sha1dc library\n\n Makefile | 18 +++++++++++++++---\n hash.h | 6 +-----\n sha1dc_git.c | 18 ++++++++++++++++--\n sha1dc_git.h | 28 ++++++++++++++++------------\n 4 files changed, 48 insertions(+), 22 deletions(-)\n\n-- \n2.14.1\n\n"},{"id":"326390","messageId":"20170815120417.31616-2-tiwai@suse.de","threadId":"46589","inReplyTo":"20170815120417.31616-1-tiwai@suse.de","subject":"[PATCH v2 1/2] sha1dc: Build git plumbing code more explicitly","fromName":"Takashi Iwai","fromEmail":"tiwai@suse.de","sentAt":"2017-08-15T12:04:16Z","receivedAt":"2017-08-15T12:04:36Z","isPatch":true,"sender":{"key":"tiwai@suse.de","avatar":"https://avatars.githubusercontent.com/u/306482?v=4"},"body":"The plumbing code between sha1dc and git is defined in\nsha1dc_git.[ch], but these aren't compiled / included directly but\nonly via the indirect inclusion from sha1dc code. This is slightly\nconfusing when you try to trace the build flow.\n\nThis patch brings the following changes for simplification:\n- Make sha1dc_git.c stand-alone and build from Makefile\n- sha1dc_git.h is the common header to include further sha1.h\n depending on the build condition\n- Move comments for plumbing codes from the header to definitions\n\nThis is also meant as a preliminary work for further plumbing with\nexternal sha1dc shlib.\n\nSigned-off-by: Takashi Iwai \n---\n Makefile | 5 ++---\n hash.h | 6 +-----\n sha1dc_git.c | 9 ++++++---\n sha1dc_git.h | 18 +++++++-----------\n 4 files changed, 16 insertions(+), 22 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 461c845d33cb..5e7e9022bdd8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1472,6 +1472,8 @@ ifdef APPLE_COMMON_CRYPTO\n \tBASIC_CFLAGS += -DSHA1_APPLE\n else\n \tDC_SHA1 := YesPlease\n+\tBASIC_CFLAGS += -DSHA1_DC\n+\tLIB_OBJS += sha1dc_git.o\n ifdef DC_SHA1_SUBMODULE\n \tLIB_OBJS += sha1collisiondetection/lib/sha1.o\n \tLIB_OBJS += sha1collisiondetection/lib/ubc_check.o\n@@ -1481,12 +1483,9 @@ else\n \tLIB_OBJS += sha1dc/ubc_check.o\n endif\n \tBASIC_CFLAGS += \\\n-\t\t-DSHA1_DC \\\n \t\t-DSHA1DC_NO_STANDARD_INCLUDES \\\n \t\t-DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 \\\n \t\t-DSHA1DC_CUSTOM_INCLUDE_SHA1_C=\"\\\"cache.h\\\"\" \\\n-\t\t-DSHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_C=\"\\\"sha1dc_git.c\\\"\" \\\n-\t\t-DSHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_H=\"\\\"sha1dc_git.h\\\"\" \\\n \t\t-DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C=\"\\\"git-compat-util.h\\\"\"\n endif\n endif\ndiff --git a/hash.h b/hash.h\nindex bef3e630a093..024d0d3d50b1 100644\n--- a/hash.h\n+++ b/hash.h\n@@ -8,11 +8,7 @@\n #elif defined(SHA1_OPENSSL)\n #include \n #elif defined(SHA1_DC)\n-#ifdef DC_SHA1_SUBMODULE\n-#include \"sha1collisiondetection/lib/sha1.h\"\n-#else\n-#include \"sha1dc/sha1.h\"\n-#endif\n+#include \"sha1dc_git.h\"\n #else /* SHA1_BLK */\n #include \"block-sha1/sha1.h\"\n #endif\ndiff --git a/sha1dc_git.c b/sha1dc_git.c\nindex 4d32b4f77e04..79466414f841 100644\n--- a/sha1dc_git.c\n+++ b/sha1dc_git.c\n@@ -1,8 +1,8 @@\n+#include \"cache.h\"\n+\n /*\n- * This code is included at the end of sha1dc/sha1.c with the\n- * SHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_C macro.\n+ * Same as SHA1DCFinal, but convert collision attack case into a verbose die().\n */\n-\n void git_SHA1DCFinal(unsigned char hash[20], SHA1_CTX *ctx)\n {\n \tif (!SHA1DCFinal(hash, ctx))\n@@ -11,6 +11,9 @@ void git_SHA1DCFinal(unsigned char hash[20], SHA1_CTX *ctx)\n \t sha1_to_hex(hash));\n }\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 *vdata, unsigned long len)\n {\n \tconst char *data = vdata;\ndiff --git a/sha1dc_git.h b/sha1dc_git.h\nindex a8a5c1da169e..af3e9514bc8e 100644\n--- a/sha1dc_git.h\n+++ b/sha1dc_git.h\n@@ -1,16 +1,12 @@\n-/*\n- * This code is included at the end of sha1dc/sha1.h with the\n- * SHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_H macro.\n- */\n+/* Plumbing with collition-detecting SHA1 code */\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+#ifdef DC_SHA1_SUBMODULE\n+#include \"sha1collisiondetection/lib/sha1.h\"\n+#else\n+#include \"sha1dc/sha1.h\"\n+#endif\n \n-/*\n- * Same as SHA1DCUpdate, but adjust types to match git's usual interface.\n- */\n+void git_SHA1DCFinal(unsigned char [20], SHA1_CTX *);\n void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *data, unsigned long len);\n \n #define platform_SHA_CTX SHA1_CTX\n-- \n2.14.1\n\n"},{"id":"326563","messageId":"xmqqwp63i3s1.fsf@gitster.mtv.corp.google.com","threadId":"46589","inReplyTo":"20170815120417.31616-1-tiwai@suse.de","subject":"Re: [PATCH v2 0/2] Allow building with the external sha1dc library","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-16T21:47:10Z","receivedAt":"2017-08-16T21:47:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Takashi Iwai writes:\n\n> this is the second attempt to allow linking with the external sha1dc\n> shlib. Now I split to two patches: one for cleaning up of sha1dc\n> plumbing codes, and another for adding the option to link with the\n> external sha1dc lib.\n>\n> Other changes from v1:\n> - Plumbing codes for external lib are also merged commonly in\n> sha1dc_git.[ch]\n> - Check the conflict of extlib vs submodule\n> - Drop DC_SHA1_LINK, hoping that everyone is well-mannered\n> - Minor rephrasing / corrections of texts\n>\n>\n> thanks,\n\nThank you for an update. \n\nI think this round addresses the concerns Ævar had with the previous\nround. Let's wait to hear from him just to be sure.\n\n"}]}