From e47913e8e29fdeaa26ccdf2626894da5283d9bd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Mon, 7 Nov 2022 22:23:03 +0100 Subject: [PATCH 01/10] Makefile: always (re)set DC_SHA1 on fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix an edge case introduced in in e6b07da2780 (Makefile: make DC_SHA1 the default, 2017-03-17), when DC_SHA1 was made the default fallback we started unconditionally adding to BASIC_CFLAGS and LIB_OBJS, so we'd use the sha1collisiondetection by default. But the "DC_SHA1" variable remained unset, so e.g.: make test DC_SHA1= T=t0013*.sh Would skip the sha1collisiondetection tests, as we'd write "DC_SHA1=''" to "GIT-BUILD-OPTIONS", but if we manually removed that test prerequisite we'd pass the test (which we couldn't if we weren't using sha1collisiondetection). So let's have the fallback assignment use the 'override' directive instead of the ":=" simply expanded variable introduced in e6b07da2780. In this case we explicitly want to override the user's choice. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Taylor Blau --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 4927379184..0ad9a6c5bc 100644 --- a/Makefile +++ b/Makefile @@ -1826,7 +1826,7 @@ ifdef APPLE_COMMON_CRYPTO COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL BASIC_CFLAGS += -DSHA1_APPLE else - DC_SHA1 := YesPlease + override DC_SHA1 = YesPlease BASIC_CFLAGS += -DSHA1_DC LIB_OBJS += sha1dc_git.o ifdef DC_SHA1_EXTERNAL From 0ced11d32fea5caec3f014b4e60dd197b9c8aa0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Mon, 7 Nov 2022 22:23:04 +0100 Subject: [PATCH 02/10] INSTALL: remove discussion of SHA-1 backends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The claim that OpenSSL is the default SHA-1 backend hasn't been true since e6b07da2780 (Makefile: make DC_SHA1 the default, 2017-03-17), but more importantly tweaking the SHA-1 backend isn't something that's common enough to warrant discussing in the INSTALL document, so let's remove this paragraph. This discussion was originally added in c538d2d34ab (Add some installation notes in INSTALL, 2005-06-17) when tweaking the default backend was more common. The current wording was added in 5beb577db8c (INSTALL: Describe dependency knobs from Makefile, 2009-09-10). Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Taylor Blau --- INSTALL | 4 ---- 1 file changed, 4 deletions(-) diff --git a/INSTALL b/INSTALL index 89b15d71df..3344788397 100644 --- a/INSTALL +++ b/INSTALL @@ -133,10 +133,6 @@ Issues of note: you are using libcurl older than 7.34.0. Otherwise you can use NO_OPENSSL without losing git-imap-send. - By default, git uses OpenSSL for SHA1 but it will use its own - library (inspired by Mozilla's) with either NO_OPENSSL or - BLK_SHA1. - - "libcurl" library is used for fetching and pushing repositories over http:// or https://, as well as by git-imap-send if the curl version is >= 7.34.0. If you do From b425ba2380e9b5704e2f9753b80bc85205e44274 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Mon, 7 Nov 2022 22:23:05 +0100 Subject: [PATCH 03/10] Makefile: correct DC_SHA1 documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The claim that DC_SHA1 takes priority over other *_SHA1 knobs was true when it was added in [1], But that hasn't been the case since it was made the fallback default in [2]. We should be making it not only the default, but something that takes priority over other *_SHA1 knobs, but that's outside the scope of this change. For now let's correct the documentation to match reality. Let's also remove the "unconditionally enable" wording, per the above the enabling of "DC_SHA1" is conditional on these other flags. The "Define DC_SHA1" here is also a lie, actually it's "we don't care if you define DC_SHA1, just don't define anything else", but that's a more general issue that'll be addressed in a subsequent commit. Let's first stop pretending that this setting (which we actually don't even use) takes priority over anything else. 1. 8325e43b82d (Makefile: add DC_SHA1 knob, 2017-03-16) 2. e6b07da2780 (Makefile: make DC_SHA1 the default, 2017-03-17) Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Taylor Blau --- Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 0ad9a6c5bc..489327ecd9 100644 --- a/Makefile +++ b/Makefile @@ -155,9 +155,8 @@ include shared.mak # Define BLK_SHA1 environment variable to make use of the bundled # optimized C SHA1 routine. # -# Define DC_SHA1 to unconditionally enable the collision-detecting sha1 +# Define DC_SHA1 to enable the collision-detecting sha1 # 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. From 34b660e3e60b094cd456dbaf9484ba261a7a9faa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Mon, 7 Nov 2022 22:23:06 +0100 Subject: [PATCH 04/10] Makefile: create and use sections for "define" flag listing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since the "Define ..." template of comments at the top of the Makefile was started in 5bdac8b3269 ([PATCH] Improve the compilation-time settings interface, 2005-07-29) we've had a lot more flags added, including flags that come in "groups". Not having any obvious structure to the >500 line comment at the top of the Makefile has made it hard to follow. This change is almost entirely a move-only change, the two paragraphs at the start of the first two sections are new, and so are the added sections themselves, but other than that no lines are changed, only moved. We now list Makefile-only flags at the start, followed by stand-alone flags, and then cover "optional library" flags in their respective groups, followed by SHA-1 and SHA-256 flags, and finally DEVELOPER-specific flags. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Taylor Blau --- Makefile | 220 ++++++++++++++++++++++++++++++++----------------------- 1 file changed, 128 insertions(+), 92 deletions(-) diff --git a/Makefile b/Makefile index 489327ecd9..1923562468 100644 --- a/Makefile +++ b/Makefile @@ -4,8 +4,20 @@ all:: # Import tree-wide shared Makefile behavior and libraries include shared.mak +# == Makefile defines == +# +# These defines change the behavior of the Makefile itself, but have +# no impact on what it builds: +# # Define V=1 to have a more verbose compile. # +# == Portability and optional library defines == +# +# These defines indicate what Git can expect from the OS, what +# libraries are available etc. Much of this is auto-detected in +# config.mak.uname, or in configure.ac when using the optional "make +# configure && ./configure" (see INSTALL). +# # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken. # # Define SANE_TOOL_PATH to a colon-separated list of paths to prepend @@ -30,68 +42,8 @@ include shared.mak # # Define NO_OPENSSL environment variable if you do not have OpenSSL. # -# Define USE_LIBPCRE if you have and want to use libpcre. Various -# commands such as log and grep offer runtime options to use -# Perl-compatible regular expressions instead of standard or extended -# POSIX regular expressions. -# -# Only libpcre version 2 is supported. USE_LIBPCRE2 is a synonym for -# USE_LIBPCRE, support for the old USE_LIBPCRE1 has been removed. -# -# Define LIBPCREDIR=/foo/bar if your PCRE header and library files are -# in /foo/bar/include and /foo/bar/lib directories. -# # Define HAVE_ALLOCA_H if you have working alloca(3) defined in that header. # -# Define NO_CURL if you do not have libcurl installed. git-http-fetch and -# git-http-push are not built, and you cannot use http:// and https:// -# transports (neither smart nor dumb). -# -# Define CURLDIR=/foo/bar if your curl header and library files are in -# /foo/bar/include and /foo/bar/lib directories. -# -# Define CURL_CONFIG to curl's configuration program that prints information -# about the library (e.g., its version number). The default is 'curl-config'. -# -# Define CURL_LDFLAGS to specify flags that you need to link when using libcurl, -# if you do not want to rely on the libraries provided by CURL_CONFIG. The -# default value is a result of `curl-config --libs`. An example value for -# CURL_LDFLAGS is as follows: -# -# CURL_LDFLAGS=-lcurl -# -# Define NO_EXPAT if you do not have expat installed. git-http-push is -# not built, and you cannot push using http:// and https:// transports (dumb). -# -# Define EXPATDIR=/foo/bar if your expat header and library files are in -# /foo/bar/include and /foo/bar/lib directories. -# -# Define EXPAT_NEEDS_XMLPARSE_H if you have an old version of expat (e.g., -# 1.1 or 1.2) that provides xmlparse.h instead of expat.h. -# -# Define NO_GETTEXT if you don't want Git output to be translated. -# A translated Git requires GNU libintl or another gettext implementation, -# plus libintl-perl at runtime. -# -# Define USE_GETTEXT_SCHEME and set it to 'fallthrough', if you don't trust -# the installed gettext translation of the shell scripts output. -# -# Define HAVE_LIBCHARSET_H if you haven't set NO_GETTEXT and you can't -# trust the langinfo.h's nl_langinfo(CODESET) function to return the -# current character set. GNU and Solaris have a nl_langinfo(CODESET), -# FreeBSD can use either, but MinGW and some others need to use -# libcharset.h's locale_charset() instead. -# -# Define CHARSET_LIB to the library you need to link with in order to -# use locale_charset() function. On some platforms this needs to set to -# -lcharset, on others to -liconv . -# -# Define LIBC_CONTAINS_LIBINTL if your gettext implementation doesn't -# need -lintl when linking. -# -# Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt -# doesn't support GNU extensions like --check and --statistics -# # Define HAVE_PATHS_H if you have paths.h and want to use the default PATH # it specifies. # @@ -152,38 +104,6 @@ include shared.mak # and do not want to use Apple's CommonCrypto library. This allows you # to provide your own OpenSSL library, for example from MacPorts. # -# Define BLK_SHA1 environment variable to make use of the bundled -# optimized C SHA1 routine. -# -# Define DC_SHA1 to enable the collision-detecting sha1 -# algorithm. This is slower, but may detect attempted collision attacks. -# -# 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 -# by the git project to migrate to using sha1collisiondetection as a -# submodule. -# -# Define OPENSSL_SHA1 environment variable when running make to link -# with the SHA1 routine from openssl library. -# -# 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. -# -# Define BLK_SHA256 to use the built-in SHA-256 routines. -# -# Define NETTLE_SHA256 to use the SHA-256 routines in libnettle. -# -# Define GCRYPT_SHA256 to use the SHA-256 routines in libgcrypt. -# -# Define OPENSSL_SHA256 to use the SHA-256 routines in OpenSSL. -# # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin). # # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin). @@ -489,6 +409,122 @@ include shared.mak # to the "" of the corresponding `compat/fsmonitor/fsm-settings-.c` # that implements the `fsm_os_settings__*()` routines. # +# === Optional library: libintl === +# +# Define NO_GETTEXT if you don't want Git output to be translated. +# A translated Git requires GNU libintl or another gettext implementation, +# plus libintl-perl at runtime. +# +# Define USE_GETTEXT_SCHEME and set it to 'fallthrough', if you don't trust +# the installed gettext translation of the shell scripts output. +# +# Define HAVE_LIBCHARSET_H if you haven't set NO_GETTEXT and you can't +# trust the langinfo.h's nl_langinfo(CODESET) function to return the +# current character set. GNU and Solaris have a nl_langinfo(CODESET), +# FreeBSD can use either, but MinGW and some others need to use +# libcharset.h's locale_charset() instead. +# +# Define CHARSET_LIB to the library you need to link with in order to +# use locale_charset() function. On some platforms this needs to set to +# -lcharset, on others to -liconv . +# +# Define LIBC_CONTAINS_LIBINTL if your gettext implementation doesn't +# need -lintl when linking. +# +# Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt +# doesn't support GNU extensions like --check and --statistics +# +# === Optional library: libexpat === +# +# Define NO_EXPAT if you do not have expat installed. git-http-push is +# not built, and you cannot push using http:// and https:// transports (dumb). +# +# Define EXPATDIR=/foo/bar if your expat header and library files are in +# /foo/bar/include and /foo/bar/lib directories. +# +# Define EXPAT_NEEDS_XMLPARSE_H if you have an old version of expat (e.g., +# 1.1 or 1.2) that provides xmlparse.h instead of expat.h. + +# === Optional library: libcurl === +# +# Define NO_CURL if you do not have libcurl installed. git-http-fetch and +# git-http-push are not built, and you cannot use http:// and https:// +# transports (neither smart nor dumb). +# +# Define CURLDIR=/foo/bar if your curl header and library files are in +# /foo/bar/include and /foo/bar/lib directories. +# +# Define CURL_CONFIG to curl's configuration program that prints information +# about the library (e.g., its version number). The default is 'curl-config'. +# +# Define CURL_LDFLAGS to specify flags that you need to link when using libcurl, +# if you do not want to rely on the libraries provided by CURL_CONFIG. The +# default value is a result of `curl-config --libs`. An example value for +# CURL_LDFLAGS is as follows: +# +# CURL_LDFLAGS=-lcurl +# +# === Optional library: libpcre2 === +# +# Define USE_LIBPCRE if you have and want to use libpcre. Various +# commands such as log and grep offer runtime options to use +# Perl-compatible regular expressions instead of standard or extended +# POSIX regular expressions. +# +# Only libpcre version 2 is supported. USE_LIBPCRE2 is a synonym for +# USE_LIBPCRE, support for the old USE_LIBPCRE1 has been removed. +# +# Define LIBPCREDIR=/foo/bar if your PCRE header and library files are +# in /foo/bar/include and /foo/bar/lib directories. +# +# == SHA-1 and SHA-256 defines == +# +# === SHA-1 backend === +# +# ==== Options common to all SHA-1 implementations ==== +# +# 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. +# +# ==== SHA-1 implementations ==== +# +# Define DC_SHA1 to enable the collision-detecting sha1 +# algorithm. This is slower, but may detect attempted collision attacks. +# +# Define BLK_SHA1 environment variable to make use of the bundled +# optimized C SHA1 routine. +# +# Define OPENSSL_SHA1 environment variable when running make to link +# with the SHA1 routine from openssl library. +# +# ==== Options for the sha1collisiondetection library ==== +# +# 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 +# by the git project to migrate to using sha1collisiondetection as a +# submodule. +# +# === SHA-256 backend === +# +# ==== SHA-256 implementations ==== +# +# Define BLK_SHA256 to use the built-in SHA-256 routines. +# +# Define NETTLE_SHA256 to use the SHA-256 routines in libnettle. +# +# Define GCRYPT_SHA256 to use the SHA-256 routines in libgcrypt. +# +# Define OPENSSL_SHA256 to use the SHA-256 routines in OpenSSL. +# +# == DEVELOPER defines == +# # Define DEVELOPER to enable more compiler warnings. Compiler version # and family are auto detected, but could be overridden by defining # COMPILER_FEATURES (see config.mak.dev). You can still set From f569897cdac5c79639aaaa9f97207bc317e7624d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Mon, 7 Nov 2022 22:23:07 +0100 Subject: [PATCH 05/10] Makefile: rephrase the discussion of *_SHA1 knobs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the preceding commit the discussion of the *_SHA1 knobs was left as-is to benefit from a smaller diff, but since we're changing these let's use the same phrasing we use for most other knobs. E.g. "define X", not "define X environment variable", and get rid of the "when running make to link with" entirely. Furthermore the discussion of DC_SHA1* options is now under a "Options for the sha1collisiondetection implementation" heading, so we don't need to clarify that these options go along with DC_SHA1=Y, so let's rephrase them accordingly. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Taylor Blau --- Makefile | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 1923562468..251368b315 100644 --- a/Makefile +++ b/Makefile @@ -492,20 +492,20 @@ include shared.mak # Define DC_SHA1 to enable the collision-detecting sha1 # algorithm. This is slower, but may detect attempted collision attacks. # -# Define BLK_SHA1 environment variable to make use of the bundled -# optimized C SHA1 routine. +# Define BLK_SHA1 to make use of optimized C SHA-1 routines bundled +# with git (in the block-sha1/ directory). # -# Define OPENSSL_SHA1 environment variable when running make to link -# with the SHA1 routine from openssl library. +# Define OPENSSL_SHA1 to link to the SHA-1 routines from the OpenSSL +# library. # # ==== Options for the sha1collisiondetection library ==== # -# Define DC_SHA1_EXTERNAL in addition to DC_SHA1 if you want to build / link +# Define DC_SHA1_EXTERNAL 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 +# Define DC_SHA1_SUBMODULE to use the # sha1collisiondetection shipped as a submodule instead of the # non-submodule copy in sha1dc/. This is an experimental option used # by the git project to migrate to using sha1collisiondetection as a From 84d71c20214455c9c5ce9d5b4b0412f4915e650a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Mon, 7 Nov 2022 22:23:08 +0100 Subject: [PATCH 06/10] Makefile: document default SHA-256 backend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since 27dc04c5450 (sha256: add an SHA-256 implementation using libgcrypt, 2018-11-14) we've claimed to support a BLK_SHA256 flag, but there's no such SHA-256 backend. Instead we fall back on adding "sha256/block/sha256.o" to "LIB_OBJS" and adding "-DSHA256_BLK" to BASIC_CFLAGS. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Taylor Blau --- Makefile | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 251368b315..646fbe5b7d 100644 --- a/Makefile +++ b/Makefile @@ -515,14 +515,15 @@ include shared.mak # # ==== SHA-256 implementations ==== # -# Define BLK_SHA256 to use the built-in SHA-256 routines. -# # Define NETTLE_SHA256 to use the SHA-256 routines in libnettle. # # Define GCRYPT_SHA256 to use the SHA-256 routines in libgcrypt. # # Define OPENSSL_SHA256 to use the SHA-256 routines in OpenSSL. # +# If don't enable any of the *_SHA256 settings in this section, Git +# will default to its built-in sha256 implementation. +# # == DEVELOPER defines == # # Define DEVELOPER to enable more compiler warnings. Compiler version From ed605fa1a8a2afe47cda5f24b5059494b86379b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Mon, 7 Nov 2022 22:23:09 +0100 Subject: [PATCH 07/10] Makefile: document SHA-1 and SHA-256 default and selection order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For the *_SHA1 and *_SHA256 flags we've discussed the various flags, but not the fact that when you define multiple flags we'll pick one. Which one we pick depends on the order they're listed in the Makefile, which differed from the order we discussed them in this documentation. Let's be explicit about how we select these, and re-arrange the listings so that they're listed in the priority order we've picked. I'd personally prefer that the selection was more explicit, and that we'd error out if conflicting flags were provided, but per the discussion downhtread of[1] the consensus was to keep theses semantics. This behavior makes it easier to e.g. integrate with autoconf-like systems, where the configuration can provide everything it can support, and Git is tasked with picking the first one it prefers. 1. https://lore.kernel.org/git/220710.86mtdh81ty.gmgdl@evledraar.gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Taylor Blau --- Makefile | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 646fbe5b7d..9b5f872d10 100644 --- a/Makefile +++ b/Makefile @@ -481,6 +481,11 @@ include shared.mak # # === SHA-1 backend === # +# ==== Default SHA-1 backend ==== +# +# If no *_SHA1 backend is picked, the first supported one listed in +# "SHA-1 implementations" will be picked. +# # ==== Options common to all SHA-1 implementations ==== # # Define SHA1_MAX_BLOCK_SIZE to limit the amount of data that will be hashed @@ -489,14 +494,14 @@ include shared.mak # # ==== SHA-1 implementations ==== # -# Define DC_SHA1 to enable the collision-detecting sha1 -# algorithm. This is slower, but may detect attempted collision attacks. +# Define OPENSSL_SHA1 to link to the SHA-1 routines from the OpenSSL +# library. # # Define BLK_SHA1 to make use of optimized C SHA-1 routines bundled # with git (in the block-sha1/ directory). # -# Define OPENSSL_SHA1 to link to the SHA-1 routines from the OpenSSL -# library. +# Define DC_SHA1 to enable the collision-detecting sha1 +# algorithm. This is slower, but may detect attempted collision attacks. # # ==== Options for the sha1collisiondetection library ==== # @@ -515,12 +520,12 @@ include shared.mak # # ==== SHA-256 implementations ==== # +# Define OPENSSL_SHA256 to use the SHA-256 routines in OpenSSL. +# # Define NETTLE_SHA256 to use the SHA-256 routines in libnettle. # # Define GCRYPT_SHA256 to use the SHA-256 routines in libgcrypt. # -# Define OPENSSL_SHA256 to use the SHA-256 routines in OpenSSL. -# # If don't enable any of the *_SHA256 settings in this section, Git # will default to its built-in sha256 implementation. # From dc1cf3580e2ec7dd164c95cec2f5568beaf3324b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Mon, 7 Nov 2022 22:23:10 +0100 Subject: [PATCH 08/10] Makefile & test-tool: replace "DC_SHA1" variable with a "define" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the root cause of technical debt we've been carrying since sha1collisiondetection was made the default in [1]. In a preceding commit we narrowly fixed a bug where the "DC_SHA1" variable would be unset (in combination with "NO_APPLE_COMMON_CRYPTO=" on OSX), even though we had the sha1collisiondetection library enabled. But the only reason we needed to have such a user-exposed knob went away with [1], and it's been doing nothing useful since then. We don't care if you define DC_SHA1=*, we only care that you don't ask for any other SHA-1 implementation. If it turns out that you didn't, we'll use sha1collisiondetection, whether you had "DC_SHA1" set or not. As a result of this being confusing we had e.g. [2] for cmake and the recent [3] for ci/lib.sh setting "DC_SHA1" explicitly, even though this was always a NOOP. A much simpler way to do this is to stop having the Makefile and CMakeLists.txt set "DC_SHA1" to be picked up by the test-lib.sh, let's instead add a trivial "test-tool sha1-is-sha1dc". It returns zero if we're using sha1collisiondetection, non-zero otherwise. 1. e6b07da2780 (Makefile: make DC_SHA1 the default, 2017-03-17) 2. c4b2f41b5f5 (cmake: support for testing git with ctest, 2020-06-26) 3. 1ad5c3df35a (ci: use DC_SHA1=YesPlease on osx-clang job for CI, 2022-10-20) Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Taylor Blau --- Makefile | 8 ++++---- ci/lib.sh | 2 +- contrib/buildsystems/CMakeLists.txt | 2 -- sha1dc_git.h | 1 + t/helper/test-sha1.c | 8 ++++++++ t/helper/test-tool.c | 1 + t/helper/test-tool.h | 1 + t/t0013-sha1dc.sh | 6 ++++-- 8 files changed, 20 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index 9b5f872d10..0c79546712 100644 --- a/Makefile +++ b/Makefile @@ -500,8 +500,10 @@ include shared.mak # Define BLK_SHA1 to make use of optimized C SHA-1 routines bundled # with git (in the block-sha1/ directory). # -# Define DC_SHA1 to enable the collision-detecting sha1 -# algorithm. This is slower, but may detect attempted collision attacks. +# If don't enable any of the *_SHA1 settings in this section, Git will +# default to its built-in sha1collisiondetection library, which is a +# collision-detecting sha1 This is slower, but may detect attempted +# collision attacks. # # ==== Options for the sha1collisiondetection library ==== # @@ -1867,7 +1869,6 @@ ifdef APPLE_COMMON_CRYPTO COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL BASIC_CFLAGS += -DSHA1_APPLE else - override DC_SHA1 = YesPlease BASIC_CFLAGS += -DSHA1_DC LIB_OBJS += sha1dc_git.o ifdef DC_SHA1_EXTERNAL @@ -3030,7 +3031,6 @@ GIT-BUILD-OPTIONS: FORCE @echo NO_REGEX=\''$(subst ','\'',$(subst ','\'',$(NO_REGEX)))'\' >>$@+ @echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' >>$@+ @echo PAGER_ENV=\''$(subst ','\'',$(subst ','\'',$(PAGER_ENV)))'\' >>$@+ - @echo DC_SHA1=\''$(subst ','\'',$(subst ','\'',$(DC_SHA1)))'\' >>$@+ @echo SANITIZE_LEAK=\''$(subst ','\'',$(subst ','\'',$(SANITIZE_LEAK)))'\' >>$@+ @echo SANITIZE_ADDRESS=\''$(subst ','\'',$(subst ','\'',$(SANITIZE_ADDRESS)))'\' >>$@+ @echo X=\'$(X)\' >>$@+ diff --git a/ci/lib.sh b/ci/lib.sh index 1808e3b1ce..24d20a5d64 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -260,7 +260,7 @@ macos-latest) else MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python2)" MAKEFLAGS="$MAKEFLAGS NO_APPLE_COMMON_CRYPTO=NoThanks" - MAKEFLAGS="$MAKEFLAGS DC_SHA1=YesPlease NO_OPENSSL=NoThanks" + MAKEFLAGS="$MAKEFLAGS NO_OPENSSL=NoThanks" fi ;; esac diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 3957e4cf8c..2f6e0197ff 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -1025,7 +1025,6 @@ set(NO_PERL ) set(NO_PTHREADS ) set(NO_PYTHON ) set(PAGER_ENV "LESS=FRX LV=-c") -set(DC_SHA1 YesPlease) set(RUNTIME_PREFIX true) set(NO_GETTEXT ) @@ -1061,7 +1060,6 @@ file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_PERL='${NO_PERL}'\n") file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_PTHREADS='${NO_PTHREADS}'\n") file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_UNIX_SOCKETS='${NO_UNIX_SOCKETS}'\n") file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "PAGER_ENV='${PAGER_ENV}'\n") -file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "DC_SHA1='${DC_SHA1}'\n") file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "X='${EXE_EXTENSION}'\n") file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_GETTEXT='${NO_GETTEXT}'\n") file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "RUNTIME_PREFIX='${RUNTIME_PREFIX}'\n") diff --git a/sha1dc_git.h b/sha1dc_git.h index 41e1c3fd3f..60e3ce8439 100644 --- a/sha1dc_git.h +++ b/sha1dc_git.h @@ -17,6 +17,7 @@ void git_SHA1DCInit(SHA1_CTX *); void git_SHA1DCFinal(unsigned char [20], SHA1_CTX *); void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *data, unsigned long len); +#define platform_SHA_IS_SHA1DC /* used by "test-tool sha1-is-sha1dc" */ #define platform_SHA_CTX SHA1_CTX #define platform_SHA1_Init git_SHA1DCInit #define platform_SHA1_Update git_SHA1DCUpdate diff --git a/t/helper/test-sha1.c b/t/helper/test-sha1.c index d860c387c3..71fe5c6145 100644 --- a/t/helper/test-sha1.c +++ b/t/helper/test-sha1.c @@ -5,3 +5,11 @@ int cmd__sha1(int ac, const char **av) { return cmd_hash_impl(ac, av, GIT_HASH_SHA1); } + +int cmd__sha1_is_sha1dc(int argc UNUSED, const char **argv UNUSED) +{ +#ifdef platform_SHA_IS_SHA1DC + return 0; +#endif + return 1; +} diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c index 01cda9358d..775854c9f9 100644 --- a/t/helper/test-tool.c +++ b/t/helper/test-tool.c @@ -73,6 +73,7 @@ static struct test_cmd cmds[] = { { "scrap-cache-tree", cmd__scrap_cache_tree }, { "serve-v2", cmd__serve_v2 }, { "sha1", cmd__sha1 }, + { "sha1-is-sha1dc", cmd__sha1_is_sha1dc }, { "sha256", cmd__sha256 }, { "sigchain", cmd__sigchain }, { "simple-ipc", cmd__simple_ipc }, diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h index ca2948066f..7cf84eca12 100644 --- a/t/helper/test-tool.h +++ b/t/helper/test-tool.h @@ -66,6 +66,7 @@ int cmd__run_command(int argc, const char **argv); int cmd__scrap_cache_tree(int argc, const char **argv); int cmd__serve_v2(int argc, const char **argv); int cmd__sha1(int argc, const char **argv); +int cmd__sha1_is_sha1dc(int argc, const char **argv); int cmd__oid_array(int argc, const char **argv); int cmd__sha256(int argc, const char **argv); int cmd__sigchain(int argc, const char **argv); diff --git a/t/t0013-sha1dc.sh b/t/t0013-sha1dc.sh index 9ad76080aa..5324047689 100755 --- a/t/t0013-sha1dc.sh +++ b/t/t0013-sha1dc.sh @@ -6,9 +6,11 @@ TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh TEST_DATA="$TEST_DIRECTORY/t0013" -if test -z "$DC_SHA1" +test_lazy_prereq SHA1_IS_SHA1DC 'test-tool sha1-is-sha1dc' + +if ! test_have_prereq SHA1_IS_SHA1DC then - skip_all='skipping sha1 collision tests, DC_SHA1 not set' + skip_all='skipping sha1 collision tests, not using sha1collisiondetection' test_done fi From fb8d7add06792bd4cc0f00c032b44478b219ca90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Mon, 7 Nov 2022 22:23:11 +0100 Subject: [PATCH 09/10] Makefile: document default SHA-1 backend on OSX MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since [1] the default SHA-1 backend on OSX has been APPLE_COMMON_CRYPTO. Per [2] we'll skip using it on anything older than Mac OS X 10.4 "Tiger"[3]. When "DC_SHA1" was made the default in [4] this interaction between it and APPLE_COMMON_CRYPTO seems to have been missed in. Ever since DC_SHA1 was "made the default" we've still used Apple's CommonCrypto instead of sha1collisiondetection on modern versions of Darwin and OSX. 1. 61067954ce1 (cache.h: eliminate SHA-1 deprecation warnings on Mac OS X, 2013-05-19) 2. 9c7a0beee09 (config.mak.uname: set NO_APPLE_COMMON_CRYPTO on older systems, 2014-08-15) 3. We could probably drop "NO_APPLE_COMMON_CRYPTO", as nobody's likely to care about such on old version of OSX anymore. But let's leave that for now. 4. e6b07da2780 (Makefile: make DC_SHA1 the default, 2017-03-17) Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Taylor Blau --- Makefile | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Makefile b/Makefile index 0c79546712..7d0fa7adb6 100644 --- a/Makefile +++ b/Makefile @@ -500,6 +500,11 @@ include shared.mak # Define BLK_SHA1 to make use of optimized C SHA-1 routines bundled # with git (in the block-sha1/ directory). # +# Define NO_APPLE_COMMON_CRYPTO on OSX to opt-out of using the +# "APPLE_COMMON_CRYPTO" backend for SHA-1, which is currently the +# default on that OS. On macOS 01.4 (Tiger) or older, +# NO_APPLE_COMMON_CRYPTO is defined by default. +# # If don't enable any of the *_SHA1 settings in this section, Git will # default to its built-in sha1collisiondetection library, which is a # collision-detecting sha1 This is slower, but may detect attempted From d00fa5528b44a8a4e59cb16348f2ddb46f0190ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Mon, 7 Nov 2022 22:23:12 +0100 Subject: [PATCH 10/10] Makefile: discuss SHAttered in *_SHA{1,256} discussion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let's mention the SHAttered attack and more generally why we use the sha1collisiondetection backend by default, and note that for SHA-256 the user should feel free to pick any of the supported backends as far as hashing security is concerned. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Taylor Blau --- Makefile | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Makefile b/Makefile index 7d0fa7adb6..91596bac4c 100644 --- a/Makefile +++ b/Makefile @@ -481,6 +481,17 @@ include shared.mak # # === SHA-1 backend === # +# ==== Security ==== +# +# Due to the SHAttered (https://shattered.io) attack vector on SHA-1 +# it's strongly recommended to use the sha1collisiondetection +# counter-cryptanalysis library for SHA-1 hashing. +# +# If you know that you can trust the repository contents, or where +# potential SHA-1 attacks are otherwise mitigated the other backends +# listed in "SHA-1 implementations" are faster than +# sha1collisiondetection. +# # ==== Default SHA-1 backend ==== # # If no *_SHA1 backend is picked, the first supported one listed in @@ -525,6 +536,11 @@ include shared.mak # # === SHA-256 backend === # +# ==== Security ==== +# +# Unlike SHA-1 the SHA-256 algorithm does not suffer from any known +# vulnerabilities, so any implementation will do. +# # ==== SHA-256 implementations ==== # # Define OPENSSL_SHA256 to use the SHA-256 routines in OpenSSL.