{"thread":{"id":"47204","subject":"[PATCH] sha1: add gnutls as a sha1 provider","startedAt":"2017-11-14T09:34:27Z","lastAt":"2017-11-18T20:32:30Z","messageCount":3,"participants":["Shawn Landden","Todd Zullinger"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"332529","messageId":"20171114093402.29256-1-slandden@gmail.com","threadId":"47204","inReplyTo":null,"subject":"[PATCH] sha1: add gnutls as a sha1 provider","fromName":"Shawn Landden","fromEmail":"slandden@gmail.com","sentAt":"2017-11-14T09:34:02Z","receivedAt":"2017-11-14T09:34:27Z","isPatch":true,"sender":{"key":"slandden@gmail.com","avatar":null},"body":"GNUTLS uses the same cryptograms SHA1 routines (Cryptograms)\nby Andy Polyakov <appro@openssl.org> as OpenSSL, but with a license\nthat is acceptable for downstream packagers.\n\nThis is not the cleanest way to use the GNUTLS library,\nas it is reallocating the context every time, and GNUTLS itsself\nfudges an OpenSSL CTX to use the cryptograms code, HOWEVER\nin my benchmarks the code performs as well as both the OpenSSL library,\nand my own integration of cryptograms with git.\n\nI think this is preferrable to bringing the assembly routines into\nthe git code-base, as a way of getting access to these high-performance\nroutines to a git available in Debian, Ubuntu, or Fedora (which\nall use BLK_SHA1=1 due to GPLv2 + OpenSSL license considerations,\nsee Debian Bug #879459).\n\nI struggle with autotools, and I suspect something is wrong with that\npart of the patch.\n\nThis laptop is ancient, Intel(R) Core(TM) i5 CPU M 520.\nWhen I get arm64 hardware in a week I will update with new benchmarks.\nBuiltin (BLK_SHA1=1):\n~/git/git$ time git fsck\nChecking object directories: 100% (256/256), done.\nChecking objects: 100% (238410/238410), done.\nChecking connectivity: 236605, done.\n\nreal\t0m25.806s\nuser\t0m25.187s\nsys\t0m0.579s\n\nThis patch:\n~/git/git$ time ./git fsck\nChecking object directories: 100% (256/256), done.\nChecking objects: 100% (238410/238410), done.\nChecking connectivity: 236606, done.\n\nreal\t0m22.368s\nuser\t0m21.790s\nsys\t0m0.539s\n\nSigned-off-by: Shawn Landden <slandden@gmail.com>\n---\n Makefile           | 10 ++++++++++\n configure.ac       | 31 +++++++++++++++++++++++++++++++\n gnutls-sha1/sha1.c | 25 +++++++++++++++++++++++++\n gnutls-sha1/sha1.h | 12 ++++++++++++\n hash.h             |  2 ++\n 5 files changed, 80 insertions(+)\n create mode 100644 gnutls-sha1/sha1.c\n create mode 100644 gnutls-sha1/sha1.h\n\ndiff --git a/Makefile b/Makefile\nindex cd7598599..e23648dbd 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1252,7 +1252,9 @@ ifndef NO_OPENSSL\n \tendif\n else\n \tBASIC_CFLAGS += -DNO_OPENSSL\n+ifndef GNUTLS_SHA1\n \tBLK_SHA1 = 1\n+endif\n \tOPENSSL_LIBSSL =\n endif\n ifdef NO_OPENSSL\n@@ -1481,6 +1483,11 @@ ifdef BLK_SHA1\n \tLIB_OBJS += block-sha1/sha1.o\n \tBASIC_CFLAGS += -DSHA1_BLK\n else\n+ifdef GNUTLS_SHA1\n+\tLIB_OBJS += gnutls-sha1/sha1.o\n+\tBASIC_CFLAGS += -DSHA1_GNUTLS\n+\tEXTLIBS += -lgnutls\n+endif\n ifdef PPC_SHA1\n \tLIB_OBJS += ppc/sha1.o ppc/sha1ppc.o\n \tBASIC_CFLAGS += -DSHA1_PPC\n@@ -1488,6 +1495,8 @@ else\n ifdef APPLE_COMMON_CRYPTO\n \tCOMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL\n \tBASIC_CFLAGS += -DSHA1_APPLE\n+else\n+ifdef GNUTLS_SHA1\n else\n \tDC_SHA1 := YesPlease\n \tBASIC_CFLAGS += -DSHA1_DC\n@@ -1506,6 +1515,7 @@ ifdef DC_SHA1_SUBMODULE\n else\n \tLIB_OBJS += sha1dc/sha1.o\n \tLIB_OBJS += sha1dc/ubc_check.o\n+endif\n endif\n \tBASIC_CFLAGS += \\\n \t\t-DSHA1DC_NO_STANDARD_INCLUDES \\\ndiff --git a/configure.ac b/configure.ac\nindex 2f55237e6..109c4758d 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -250,6 +250,23 @@ AS_HELP_STRING([--with-openssl],[use OpenSSL library (default is YES)])\n AS_HELP_STRING([],              [ARG can be prefix for openssl library and headers]),\n GIT_PARSE_WITH([openssl]))\n \n+# Define GNUTLS_SHA1 if you have and want to use libgnutls. This offers\n+# similar sha1 routines as openssl.\n+AC_ARG_WITH(gnutls,\n+AS_HELP_STRING([--with-gnutls],[use GNUTLS library (default is YES)]),\n+    if test \"$withval\" = \"no\"; then\n+        USE_GNUTLS=\n+    elif test \"$withval\" = \"yes\"; then\n+\tUSE_GNUTLS=YesPlease\n+    else\n+\tUSE_GNUTLS=YesPlease\n+\tLIBGNUTLSDIR=$withval\n+\tAC_MSG_NOTICE([Setting LIBGNUTLSDIR to $LIBGNUTLSDIR])\n+        dnl USE_LIBGNUTLS can still be modified below, so don't substitute\n+        dnl it yet.\n+\tGIT_CONF_SUBST([LIBGNUTLSDIR])\n+    fi)\n+\n # Define USE_LIBPCRE if you have and want to use libpcre. Various\n # commands such as log and grep offer runtime options to use\n # Perl-compatible regular expressions instead of standard or extended\n@@ -540,6 +557,20 @@ GIT_UNSTASH_FLAGS($OPENSSLDIR)\n GIT_CONF_SUBST([NEEDS_SSL_WITH_CRYPTO])\n GIT_CONF_SUBST([NO_OPENSSL])\n \n+#\n+# Handle USE_GNUTLS from above\n+#\n+if test -n \"$USE_GNUTLS\"; then\n+\n+GIT_STASH_FLAGS($LIBGNUTLSDIR)\n+\n+AC_CHECK_LIB([gnutls], [gnutls_hash_init],\n+[GNUTLS_SHA1=YesPlease],\n+[GNUTLS_SHA1=])\n+\n+GIT_UNSTASH_FLAGS($LIBGNUTLSDIR)\n+\n+fi\n #\n # Handle the USE_LIBPCRE1 and USE_LIBPCRE2 options potentially set\n # above.\ndiff --git a/gnutls-sha1/sha1.c b/gnutls-sha1/sha1.c\nnew file mode 100644\nindex 000000000..f7ede4ddf\n--- /dev/null\n+++ b/gnutls-sha1/sha1.c\n@@ -0,0 +1,25 @@\n+/* this is only to get definitions for memcpy(), ntohl() and htonl() */\n+#include \"../git-compat-util.h\"\n+\n+#include <gnutls/gnutls.h>\n+#include <gnutls/crypto.h>\n+\n+#include \"sha1.h\"\n+\n+void gnutls_SHA1_Init(gnutls_SHA_CTX *ctx)\n+{\n+\tint ret;\n+\tret = gnutls_hash_init((void *) &ctx->handle, GNUTLS_DIG_SHA1);\n+\tif (ret < 0)\n+\t\tabort();\n+}\n+\n+void gnutls_SHA1_Update(gnutls_SHA_CTX *ctx, const void *data, unsigned long len)\n+{\n+\tgnutls_hash(ctx->handle, data, len);\n+}\n+\n+void gnutls_SHA1_Final(unsigned char hashout[20], gnutls_SHA_CTX *ctx)\n+{\n+\tgnutls_hash_deinit(ctx->handle, hashout);\n+}\ndiff --git a/gnutls-sha1/sha1.h b/gnutls-sha1/sha1.h\nnew file mode 100644\nindex 000000000..ade60fcbe\n--- /dev/null\n+++ b/gnutls-sha1/sha1.h\n@@ -0,0 +1,12 @@\n+typedef struct {\n+\tvoid *handle;\n+} gnutls_SHA_CTX;\n+\n+void gnutls_SHA1_Init(gnutls_SHA_CTX *ctx);\n+void gnutls_SHA1_Update(gnutls_SHA_CTX *ctx, const void *dataIn, unsigned long len);\n+void gnutls_SHA1_Final(unsigned char hashout[20], gnutls_SHA_CTX *ctx);\n+\n+#define platform_SHA_CTX\tgnutls_SHA_CTX\n+#define platform_SHA1_Init\tgnutls_SHA1_Init\n+#define platform_SHA1_Update\tgnutls_SHA1_Update\n+#define platform_SHA1_Final\tgnutls_SHA1_Final\ndiff --git a/hash.h b/hash.h\nindex 024d0d3d5..7e5e307fc 100644\n--- a/hash.h\n+++ b/hash.h\n@@ -9,6 +9,8 @@\n #include <openssl/sha.h>\n #elif defined(SHA1_DC)\n #include \"sha1dc_git.h\"\n+#elif defined(SHA1_GNUTLS)\n+#include \"gnutls-sha1/sha1.h\"\n #else /* SHA1_BLK */\n #include \"block-sha1/sha1.h\"\n #endif\n-- \n2.15.0.rc2\n\n"},{"id":"332563","messageId":"20171114194718.GY5144@zaya.teonanacatl.net","threadId":"47204","inReplyTo":"20171114093402.29256-1-slandden@gmail.com","subject":"Re: [PATCH] sha1: add gnutls as a sha1 provider","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2017-11-14T19:47:18Z","receivedAt":"2017-11-14T19:47:28Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Hi Shawn,\n\nShawn Landden wrote:\n> I think this is preferrable to bringing the assembly routines into \n> the git code-base, as a way of getting access to these high-performance \n> routines to a git available in Debian, Ubuntu, or Fedora (which \n> all use BLK_SHA1=1 due to GPLv2 + OpenSSL license considerations, \n> see Debian Bug #879459).\n\nWhile it seems like it could be useful to have the choice of using the \nfast SHA1 implementation without concern about licensing issues, \nthere's a few details I thought were worth mentioning.\n\nFedora moved from OpenSSL SHA1 to BLK_SHA1 to reduce the size of the \nbinaries and dependencies, not due to licensing issues (Fedora \nconsiders OpenSSL a system library and allows linking GPLv2 code).\n\nFedora now uses the default DC_SHA1 (the collision-detecting SHA1 \nimplementation).  DC_SHA1 is not, as far as I know, as fast as the \nOpenSSL/GnuTLS SHA1, but it's safer given the increasingly successful \nattacks against SHA1.  I don't envision changing that to gain \nperformance.  (And, of course, the speed of SHA1 should become less of \nan issue once git moves to a new, stronger hash.)\n\nIt looks like the Debian packages use the default DC_SHA1 \nimplementation as well.  Regardless of the licensing concerns \nregarding OpenSSL in Debian, I suspect they'll want to use the \ndefault, collision-detecting SHA1 implementation.  That doesn't mean a \npatch to add the option of GnuTLS isn't useful though.\n\nFedora does link with OpenSSL's libcrypto and libssl in Fedora for the \nremote-curl helpers and imap-send.  I believe the remote-curl helpers \njust link with curl, which happens to use OpenSSL on Fedora and could \nuse GnuTLS instead.  The imap-send command might also use curl and \nwhatever crypto library curl is built with too, but I'm not terribly \nfamiliar with imap-send. (I think those are the only uses of libcrypto \nor libssl in Fedora's packages, but I could be mistaken).\n\nThat's a lot of text without having anything to say about the actual \npatch.  Hopefully it's at least mildly useful to you or others. :)\n\n-- \nTodd\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nWhen we remember we are all mad, the mysteries of life disappear and\nlife stands explained.\n    -- Mark Twain\n\n"},{"id":"332822","messageId":"CA+49okpfdzFwB32VksO4nQJX+enc2MZzg8=xkeFgUEHhGvEBjw@mail.gmail.com","threadId":"47204","inReplyTo":"20171114194718.GY5144@zaya.teonanacatl.net","subject":"Re: [PATCH] sha1: add gnutls as a sha1 provider","fromName":"Shawn Landden","fromEmail":"slandden@gmail.com","sentAt":"2017-11-18T20:32:24Z","receivedAt":"2017-11-18T20:32:30Z","isPatch":true,"sender":{"key":"slandden@gmail.com","avatar":null},"body":"On Tue, Nov 14, 2017 at 11:47 AM, Todd Zullinger <tmz@pobox.com> wrote:\n>\n> Hi Shawn,\n>\n> Shawn Landden wrote:\n>>\n>> I think this is preferrable to bringing the assembly routines into the git code-base, as a way of getting access to these high-performance routines to a git available in Debian, Ubuntu, or Fedora (which all use BLK_SHA1=1 due to GPLv2 + OpenSSL license considerations, see Debian Bug #879459).\n>\n>\n> While it seems like it could be useful to have the choice of using the fast SHA1 implementation without concern about licensing issues, there's a few details I thought were worth mentioning.\n>\n> Fedora moved from OpenSSL SHA1 to BLK_SHA1 to reduce the size of the binaries and dependencies, not due to licensing issues (Fedora considers OpenSSL a system library and allows linking GPLv2 code).\n>\n> Fedora now uses the default DC_SHA1 (the collision-detecting SHA1 implementation).  DC_SHA1 is not, as far as I know, as fast as the OpenSSL/GnuTLS SHA1, but it's safer given the increasingly successful attacks against SHA1.  I don't envision changing that to gain performance.  (And, of course, the speed of SHA1 should become less of an issue once git moves to a new, stronger hash.)\n>\n> It looks like the Debian packages use the default DC_SHA1 implementation as well.  Regardless of the licensing concerns regarding OpenSSL in Debian, I suspect they'll want to use the default, collision-detecting SHA1 implementation.  That doesn't mean a patch to add the option of GnuTLS isn't useful though.\n>\n> Fedora does link with OpenSSL's libcrypto and libssl in Fedora for the remote-curl helpers and imap-send.  I believe the remote-curl helpers just link with curl, which happens to use OpenSSL on Fedora and could use GnuTLS instead.  The imap-send command might also use curl and whatever crypto library curl is built with too, but I'm not terribly familiar with imap-send. (I think those are the only uses of libcrypto or libssl in Fedora's packages, but I could be mistaken).\n>\n> That's a lot of text without having anything to say about the actual patch.  Hopefully it's at least mildly useful to you or others. :)\nIt is all appreciated. I just want to make note that I am still\ninterested in getting this patch in.\n"}]}