git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/5] core: update our SHA-1 docs, use sha1collisiondetection on OSX too
@ 2022-04-22  9:53 Ævar Arnfjörð Bjarmason
  2022-04-22  9:53 ` [PATCH 1/5] Makefile: create and use sections for "define" flag listing Ævar Arnfjörð Bjarmason
                   ` (6 more replies)
  0 siblings, 7 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-04-22  9:53 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, brian m . carlson, Carlo Arenas, Mike Hommey,
	Eric Sunshine, Ævar Arnfjörð Bjarmason

I wasn't able to find any on-list references to it being intentional,
but it appears that while we made the sha1collisiondetection variant
of SHA-1 the default in early 2017 we've never updated the OSX builds
to do likewise.

I don't know what various git packages for OSX to, but our vanilla OSX
distribution definitely uses Apple Common Crypto, and won't detect the
https://shattered.io attack.

This series changes that, and while doing so in 2/5 updates our
documentation and Makefile interface for the SHA-1 selection. Our
INSTALL file was still claiming we used OpenSSL's SHA-1 by default.

Then since we'd made sha1collisiondetection the default we hadn't
changed the code's default fallback to be that, it was still
block-sha1. Now our fallback behavior is "error" instead, which makes
it less likely that we'll get some foot-gun like the "OSX not using
sha1collisiondetection" again.

The 4/5 and 5/5 then remove the PPC_SHA1 implementation. I submitted
this before as [1], and the range-diff is to that submission (it
wasn't picked up). I think it makes sense as part of this general
SHA-1 cleanup.

1. https://lore.kernel.org/git/patch-1.1-05dcdca3877-20220319T005952Z-avarab@gmail.com/

Ævar Arnfjörð Bjarmason (5):
  Makefile: create and use sections for "define" flag listing
  Makefile: really use and document sha1collisiondetection by default
  Makefile: rephrase the discussion of *_SHA1 knobs
  Makefile + hash.h: remove PPC_SHA1 implementation
  Makefile: use $(OBJECTS) instead of $(C_OBJ)

 INSTALL                             |  11 +-
 Makefile                            | 301 ++++++++++++++++------------
 configure.ac                        |   3 -
 contrib/buildsystems/CMakeLists.txt |   3 +-
 hash.h                              |  12 +-
 ppc/sha1.c                          |  72 -------
 ppc/sha1.h                          |  25 ---
 ppc/sha1ppc.S                       | 224 ---------------------
 t/t0013-sha1dc.sh                   |   4 +-
 9 files changed, 191 insertions(+), 464 deletions(-)
 delete mode 100644 ppc/sha1.c
 delete mode 100644 ppc/sha1.h
 delete mode 100644 ppc/sha1ppc.S

Range-diff:
-:  ----------- > 1:  f799d30e82e Makefile: create and use sections for "define" flag listing
-:  ----------- > 2:  5ffb68dc77b Makefile: really use and document sha1collisiondetection by default
-:  ----------- > 3:  d559e5212bc Makefile: rephrase the discussion of *_SHA1 knobs
1:  3a8caf62137 ! 4:  4b2d0b7b51a ppc: remove custom SHA-1 implementation
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Commit message ##
    -    ppc: remove custom SHA-1 implementation
    +    Makefile + hash.h: remove PPC_SHA1 implementation
     
         Remove the PPC_SHA1 implementation added in a6ef3518f9a ([PATCH] PPC
         assembly implementation of SHA1, 2005-04-22). When this was added
         Apple consumer hardware used the PPC architecture, and the
         implementation was intended to improve SHA-1 speed there.
     
    -    Since it was added we've moved to DC_SHA1 by default, and anyone
    -    wanting hard-rolled non-DC SHA-1 implementation can use OpenSSL's via
    -    the OPENSSL_SHA1 knob.
    +    Since it was added we've moved to using sha1collisiondetection by
    +    default, and anyone wanting hard-rolled non-DC SHA-1 implementation
    +    can use OpenSSL's via the OPENSSL_SHA1 knob.
     
    -    I'm unsure if this was ever supposed to work on 64-bit PPC. It clearly
    -    originally targeted 32 bit PPC, but there's some mailing list
    -    references to this being tried on G5 (PPC 970). I can't get it to do
    -    anything but segfault on the BE POWER8 machine in the GCC compile
    -    farm. Anyone caring about speed on PPC these days is likely to be
    -    using IBM's POWER, not PPC 970.
    +    Furthermore this doesn't run on the modern PPC processors which anyone
    +    who's concerned about performance on PPC is likely to be using these
    +    days, i.e. the IBM POWER series. It originally originally targeted 32
    +    bit PPC, but there's some mailing list references to this being tried
    +    on G5 (PPC 970).
     
    -    There have been proposals to entirely remove non-DC_SHA1
    +    I can't get it to do anything but segfault on both the BE and LE POWER
    +    machines in the GCC compile farm.
    +
    +    There have been proposals to entirely remove non-sha1collisiondetection
         implementations from the tree[1]. I think per [2] that would be a bit
         overzealous. I.e. there are various set-ups git's speed is going to be
         more important than the relatively implausible SHA-1 collision attack,
         or where such attacks are entirely mitigated by other means (e.g. by
         incoming objects being checked with DC_SHA1).
     
    -    The main reason for doing so at this point is to simplify follow-up
    -    Makefile change. Since PPC_SHA1 included the only in-tree *.S assembly
    -    file we needed to keep around special support for building objects
    -    from it. By getting rid of it we know we'll always build *.o from *.c
    -    files, which makes the build process simpler.
    +    But that really doesn't apply to PPC_SHA1 in particular, which seems
    +    to have outlived its usefulness.
    +
    +    As this gets rid of the only in-tree *.S assembly file we can remove
    +    the small bits of logic from the Makefile needed to build objects
    +    from *.S (as opposed to *.c)
     
    -    As an aside the code being removed here was also throwing warnings
    -    with the "-pedantic" flag, but let's remove it instead of fixing it,
    -    as 544d93bc3b4 (block-sha1: remove use of obsolete x86 assembly,
    -    2022-03-10) did for block-sha1/*.
    +    The code being removed here was also throwing warnings with the
    +    "-pedantic" flag, it could have been fixed as 544d93bc3b4 (block-sha1:
    +    remove use of obsolete x86 assembly, 2022-03-10) did for block-sha1/*,
    +    but as noted above let's remove it instead.
     
         1. https://lore.kernel.org/git/20200223223758.120941-1-mh@glandium.org/
         2. https://lore.kernel.org/git/20200224044732.GK1018190@coredump.intra.peff.net/
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    - ## INSTALL ##
    -@@ INSTALL: Issues of note:
    - 
    - 	  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.  Also included is a version optimized for PowerPC
    --	  (PPC_SHA1).
    -+	  BLK_SHA1.
    - 
    - 	- "libcurl" library is used for fetching and pushing
    - 	  repositories over http:// or https://, as well as by
    -
      ## Makefile ##
     @@ Makefile: include shared.mak
    - # Define BLK_SHA1 environment variable to make use of the bundled
    - # optimized C SHA1 routine.
    + # Define OPENSSL_SHA1 to link to the the SHA-1 routines from
    + # the OpenSSL library.
      #
    --# Define PPC_SHA1 environment variable when running make to make use of
    --# a bundled SHA1 routine optimized for PowerPC.
    +-# Define PPC_SHA1 to make use of optimized (in assembly)
    +-# PowerPC SHA-1 routines.
     -#
    - # Define DC_SHA1 to unconditionally enable the collision-detecting sha1
    - # algorithm. This is slower, but may detect attempted collision attacks.
    - # Takes priority over other *_SHA1 knobs.
    -@@ Makefile: ifdef OPENSSL_SHA1
    - 	EXTLIBS += $(LIB_4_CRYPTO)
    - 	BASIC_CFLAGS += -DSHA1_OPENSSL
    - else
    + # Define APPLE_SHA1 to use Apple's CommonCrypto SHA-1 routines on
    + # Darwin/Mac OS X.
    + #
    +@@ Makefile: endif
    + ifdef DC_SHA1
    + $(error the DC_SHA1 flag is no longer used, and has become the default. Adjust your build scripts accordingly)
    + endif
     +ifdef PPC_SHA1
    -+$(error PPC_SHA1 has been removed! Use DC_SHA1 instead, which is the default)
    ++$(error the PPC_SHA1 flag has been removed along with the PowerPC-specific SHA-1 implementation.)
     +endif
    - ifdef BLK_SHA1
    + ifndef NO_DC_SHA1
    +-	ifneq ($(OPENSSL_SHA1)$(BLK_SHA1)$(PPC_SHA1)$(APPLE_SHA1),)
    ++	ifneq ($(OPENSSL_SHA1)$(BLK_SHA1)$(APPLE_SHA1),)
    + $(error no other *_SHA1 option can be defined unless NO_DC_SHA1 is defined)
    + 	endif
    + 	LIB_OBJS += sha1dc_git.o
    +@@ Makefile: ifdef BLK_SHA1
      	LIB_OBJS += block-sha1/sha1.o
      	BASIC_CFLAGS += -DSHA1_BLK
      else
    @@ Makefile: ifdef OPENSSL_SHA1
     -	LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
     -	BASIC_CFLAGS += -DSHA1_PPC
     -else
    - ifdef APPLE_COMMON_CRYPTO
    + ifdef APPLE_SHA1
      	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
      	BASIC_CFLAGS += -DSHA1_APPLE
     @@ Makefile: endif
    @@ configure.ac: AC_MSG_NOTICE([CHECKS for site configuration])
     
      ## hash.h ##
     @@
    - #include "git-compat-util.h"
    - #include "repository.h"
      
    --#if defined(SHA1_PPC)
    + #if !defined(NO_SHA1_DC)
    + #include "sha1dc_git.h"
    +-#elif defined(SHA1_PPC)
     -#include "ppc/sha1.h"
    --#elif defined(SHA1_APPLE)
    -+#if defined(SHA1_APPLE)
    + #elif defined(SHA1_APPLE)
      #include <CommonCrypto/CommonDigest.h>
      #elif defined(SHA1_OPENSSL)
    - #include <openssl/sha.h>
     @@
       * platform's underlying implementation of SHA-1; could be OpenSSL,
       * blk_SHA, Apple CommonCrypto, etc...  Note that the relevant
-:  ----------- > 5:  0575faebc30 Makefile: use $(OBJECTS) instead of $(C_OBJ)
-- 
2.36.0.879.g56a83971f3f


^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH 1/5] Makefile: create and use sections for "define" flag listing
  2022-04-22  9:53 [PATCH 0/5] core: update our SHA-1 docs, use sha1collisiondetection on OSX too Ævar Arnfjörð Bjarmason
@ 2022-04-22  9:53 ` Ævar Arnfjörð Bjarmason
  2022-04-22  9:53 ` [PATCH 2/5] Makefile: really use and document sha1collisiondetection by default Ævar Arnfjörð Bjarmason
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-04-22  9:53 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, brian m . carlson, Carlo Arenas, Mike Hommey,
	Eric Sunshine, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 212 +++++++++++++++++++++++++++++++------------------------
 1 file changed, 118 insertions(+), 94 deletions(-)

diff --git a/Makefile b/Makefile
index f8bccfab5e9..246f6729022 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,40 +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 PPC_SHA1 environment variable when running make to make use of
-# a bundled SHA1 routine optimized for PowerPC.
-#
-# Define DC_SHA1 to unconditionally 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.
-# 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 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).
@@ -480,6 +398,112 @@ include shared.mak
 # `compat/fsmonitor/fsm-listen-<name>.c` that implements the
 # `fsm_listen__*()` 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 ==
+#
+# Define BLK_SHA1 environment variable to make use of the bundled
+# optimized C SHA1 routine.
+#
+# Define PPC_SHA1 environment variable when running make to make use of
+# a bundled SHA1 routine optimized for PowerPC.
+#
+# Define DC_SHA1 to unconditionally 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.
+# 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 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
-- 
2.36.0.879.g56a83971f3f


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH 2/5] Makefile: really use and document sha1collisiondetection by default
  2022-04-22  9:53 [PATCH 0/5] core: update our SHA-1 docs, use sha1collisiondetection on OSX too Ævar Arnfjörð Bjarmason
  2022-04-22  9:53 ` [PATCH 1/5] Makefile: create and use sections for "define" flag listing Ævar Arnfjörð Bjarmason
@ 2022-04-22  9:53 ` Ævar Arnfjörð Bjarmason
  2022-04-22  9:53 ` [PATCH 3/5] Makefile: rephrase the discussion of *_SHA1 knobs Ævar Arnfjörð Bjarmason
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-04-22  9:53 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, brian m . carlson, Carlo Arenas, Mike Hommey,
	Eric Sunshine, Ævar Arnfjörð Bjarmason

When the sha1collisiondetection library was added and made the default
in [1] we never updated the documentation added in [2] early in that
series once the default was flipped to DC_SHA1 in [3]. Furthermore the
INSTALL file has been claiming that we use OpenSSL by default since
[4], and hadn't been updated for the sha1collisiondetection switch.

The interaction between NO_APPLE_COMMON_CRYPTO and DC_SHA1 seems to
have been missed in [3], so ever since DC_SHA1 was made the default
we've still used Apple's CommonCrypto instead of
sha1collisiondetection on Darwin and Mac OS X.

Instead off all of this we now:

* Don't have a DC_SHA1 know anymore (using it is an error), you need
  to set NO_DC_SHA1 instead to use any optional *_SHA1 implementation.
* Re-arranged the algorithm inclusion in hash.h to correspond to
  NO_DC_SHA1, and "#error" if we have no defined SHA_*, rather than
  silently picking block-sha1/sha1.h as a fallback.
* Have an INSTALL that reflects reality. We were still claiming to use
  OpenSSL's SHA-1 hashing by default.
* Have Darwin and Mac OS X use sha1collisiondetection, like everywhere
  else. There is still a NO_APPLE_COMMON_CRYPTO knob, but it's used for
  things unrelated to SHA-1 (see [6]).
* Have a rewritten discussion of SHA-1 and SHA-256 in the Makefile
  which covers all of this.

1. 48b3693d3ce (Merge branch 'jk/sha1dc', 2017-03-24)
2. 8325e43b82d (Makefile: add DC_SHA1 knob, 2017-03-16)
3. e6b07da2780 (Makefile: make DC_SHA1 the default, 2017-03-17)
4. 5beb577db8c (INSTALL: Describe dependency knobs from Makefile,
   2009-09-10)
5. 4dcd7732db0 (Makefile: add support for Apple CommonCrypto facility,
   2013-05-19)
6. 3ef2bcad02e (imap-send: use Apple's Security framework for base64
   encoding, 2013-07-29)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 INSTALL                             |  11 +--
 Makefile                            | 108 ++++++++++++++++++----------
 contrib/buildsystems/CMakeLists.txt |   3 +-
 hash.h                              |  10 +--
 t/t0013-sha1dc.sh                   |   4 +-
 5 files changed, 88 insertions(+), 48 deletions(-)

diff --git a/INSTALL b/INSTALL
index 4140a3f5c8b..8fc350fc124 100644
--- a/INSTALL
+++ b/INSTALL
@@ -133,10 +133,13 @@ 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.  Also included is a version optimized for PowerPC
-	  (PPC_SHA1).
+	- Git uses an altered version of SHA-1 by default which
+          detects the SHAttered attack via the sha1collisiondetection
+          counter-cryptanalysis library. For SHA-256 we'll select a
+          working implementation (and ship a fallback
+          implementation). See the "SHA-1 and SHA-256 defines" section
+          in the Makefile for details. You should not need to tweak
+          those settings.
 
 	- "libcurl" library is used for fetching and pushing
 	  repositories over http:// or https://, as well as by
diff --git a/Makefile b/Makefile
index 246f6729022..12bb9edd70a 100644
--- a/Makefile
+++ b/Makefile
@@ -468,33 +468,65 @@ include shared.mak
 #
 # == SHA-1 and SHA-256 defines ==
 #
-# Define BLK_SHA1 environment variable to make use of the bundled
-# optimized C SHA1 routine.
+# === SHA-1 backend ===
 #
-# Define PPC_SHA1 environment variable when running make to make use of
-# a bundled SHA1 routine optimized for PowerPC.
+# Due to the SHAttered (https://shattered.io) attack vector on SHA-1
+# Git uses the sha1collisiondetection counter-cryptanalysis library
+# for SHA-1 hashing.
+#
+# You're strongly advised not to override this for any usage of Git
+# where you don't 100% trust the repository content.
+#
+# ==== 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.
 #
-# Define DC_SHA1 to unconditionally enable the collision-detecting sha1
-# algorithm. This is slower, but may detect attempted collision attacks.
-# Takes priority over other *_SHA1 knobs.
+# ===== Options for the default sha1collisiondetection implementations =====
 #
-# 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
 # submodule.
 #
+# ==== Alternate implementations ====
+#
+# Git still ships with alternate SHA-1 implementations. These are
+# faster than the default, which is useful when hashing speed
+# is imperative, consider using them if you're confident that you
+# won't need to worry about SHA-1 collision attacks.
+#
+# To use them you must define NO_DC_SHA1 and one of the *_SHA1
+# variables below:
+#
+# 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.
 #
-# 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 PPC_SHA1 environment variable when running make to make use of
+# a bundled SHA1 routine optimized for PowerPC.
+#
+# Define APPLE_SHA1 to use Apple's CommonCrypto SHA-1 routines on
+# Darwin/Mac OS X.
+#
+# The APPLE_SHA1 option is unrelated to the NO_APPLE_COMMON_CRYPTO
+# flag, which determines if Apple's crypto libraries are used for
+# things that aren't SHA-1.
+#
+# === SHA-256 backend ===
+#
+# Unlike SHA-1 the SHA-256 algorithm does not suffer from any known
+# vulnerabilities, so any implementation will do. BLK_SHA256 is
+# currently the default implementation (but that may change).
 #
 # Define BLK_SHA256 to use the built-in SHA-256 routines.
 #
@@ -1434,7 +1466,6 @@ ifeq ($(uname_S),Darwin)
 	endif
 	ifndef NO_APPLE_COMMON_CRYPTO
 		NO_OPENSSL = YesPlease
-		APPLE_COMMON_CRYPTO = YesPlease
 		COMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO
 	endif
 	NO_REGEX = YesPlease
@@ -1796,30 +1827,15 @@ ifdef NO_POSIX_GOODIES
 	BASIC_CFLAGS += -DNO_POSIX_GOODIES
 endif
 
-ifdef APPLE_COMMON_CRYPTO
-	# Apple CommonCrypto requires chunking
-	SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
+ifdef DC_SHA1
+$(error the DC_SHA1 flag is no longer used, and has become the default. Adjust your build scripts accordingly)
 endif
-
-ifdef OPENSSL_SHA1
-	EXTLIBS += $(LIB_4_CRYPTO)
-	BASIC_CFLAGS += -DSHA1_OPENSSL
-else
-ifdef BLK_SHA1
-	LIB_OBJS += block-sha1/sha1.o
-	BASIC_CFLAGS += -DSHA1_BLK
-else
-ifdef PPC_SHA1
-	LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
-	BASIC_CFLAGS += -DSHA1_PPC
-else
-ifdef APPLE_COMMON_CRYPTO
-	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
-	BASIC_CFLAGS += -DSHA1_APPLE
-else
-	DC_SHA1 := YesPlease
-	BASIC_CFLAGS += -DSHA1_DC
+ifndef NO_DC_SHA1
+	ifneq ($(OPENSSL_SHA1)$(BLK_SHA1)$(PPC_SHA1)$(APPLE_SHA1),)
+$(error no other *_SHA1 option can be defined unless NO_DC_SHA1 is defined)
+	endif
 	LIB_OBJS += sha1dc_git.o
+
 ifdef DC_SHA1_EXTERNAL
 	ifdef DC_SHA1_SUBMODULE
 		ifneq ($(DC_SHA1_SUBMODULE),auto)
@@ -1843,6 +1859,26 @@ endif
 		-DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" \
 		-DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\""
 endif
+else # !NO_DC_SHA1
+BASIC_CFLAGS += -DNO_SHA1_DC
+ifdef OPENSSL_SHA1
+	EXTLIBS += $(LIB_4_CRYPTO)
+	BASIC_CFLAGS += -DSHA1_OPENSSL
+else
+ifdef BLK_SHA1
+	LIB_OBJS += block-sha1/sha1.o
+	BASIC_CFLAGS += -DSHA1_BLK
+else
+ifdef PPC_SHA1
+	LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
+	BASIC_CFLAGS += -DSHA1_PPC
+else
+ifdef APPLE_SHA1
+	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
+	BASIC_CFLAGS += -DSHA1_APPLE
+else
+$(error when defining NO_DC_SHA1 another valid *_SHA1 variable must be defined!)
+endif
 endif
 endif
 endif
@@ -2884,7 +2920,7 @@ GIT-BUILD-OPTIONS: FORCE
 	@echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' >>$@+
 	@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' >>$@+
 	@echo PAGER_ENV=\''$(subst ','\'',$(subst ','\'',$(PAGER_ENV)))'\' >>$@+
-	@echo DC_SHA1=\''$(subst ','\'',$(subst ','\'',$(DC_SHA1)))'\' >>$@+
+	@echo NO_DC_SHA1=\''$(subst ','\'',$(subst ','\'',$(NO_DC_SHA1)))'\' >>$@+
 	@echo SANITIZE_LEAK=\''$(subst ','\'',$(subst ','\'',$(SANITIZE_LEAK)))'\' >>$@+
 	@echo X=\'$(X)\' >>$@+
 ifdef FSMONITOR_DAEMON_BACKEND
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 185f56f414f..10616188e4e 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -996,7 +996,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 )
 
@@ -1032,7 +1031,7 @@ 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 "NO_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/hash.h b/hash.h
index 5d40368f18a..20012167ce4 100644
--- a/hash.h
+++ b/hash.h
@@ -4,16 +4,18 @@
 #include "git-compat-util.h"
 #include "repository.h"
 
-#if defined(SHA1_PPC)
+#if !defined(NO_SHA1_DC)
+#include "sha1dc_git.h"
+#elif defined(SHA1_PPC)
 #include "ppc/sha1.h"
 #elif defined(SHA1_APPLE)
 #include <CommonCrypto/CommonDigest.h>
 #elif defined(SHA1_OPENSSL)
 #include <openssl/sha.h>
-#elif defined(SHA1_DC)
-#include "sha1dc_git.h"
-#else /* SHA1_BLK */
+#elif defined(SHA1_BLK)
 #include "block-sha1/sha1.h"
+#else
+#error "need a SHA1_* implementation defined"
 #endif
 
 #if defined(SHA256_GCRYPT)
diff --git a/t/t0013-sha1dc.sh b/t/t0013-sha1dc.sh
index 9ad76080aa4..539270a2665 100755
--- a/t/t0013-sha1dc.sh
+++ b/t/t0013-sha1dc.sh
@@ -6,9 +6,9 @@ TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 TEST_DATA="$TEST_DIRECTORY/t0013"
 
-if test -z "$DC_SHA1"
+if test -n "$NO_DC_SHA1"
 then
-	skip_all='skipping sha1 collision tests, DC_SHA1 not set'
+	skip_all='skipping sha1 collision tests, NO_DC_SHA1 set'
 	test_done
 fi
 
-- 
2.36.0.879.g56a83971f3f


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH 3/5] Makefile: rephrase the discussion of *_SHA1 knobs
  2022-04-22  9:53 [PATCH 0/5] core: update our SHA-1 docs, use sha1collisiondetection on OSX too Ævar Arnfjörð Bjarmason
  2022-04-22  9:53 ` [PATCH 1/5] Makefile: create and use sections for "define" flag listing Ævar Arnfjörð Bjarmason
  2022-04-22  9:53 ` [PATCH 2/5] Makefile: really use and document sha1collisiondetection by default Ævar Arnfjörð Bjarmason
@ 2022-04-22  9:53 ` Ævar Arnfjörð Bjarmason
  2022-04-22  9:53 ` [PATCH 4/5] Makefile + hash.h: remove PPC_SHA1 implementation Ævar Arnfjörð Bjarmason
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-04-22  9:53 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, brian m . carlson, Carlo Arenas, Mike Hommey,
	Eric Sunshine, Ævar Arnfjörð Bjarmason

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.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 12bb9edd70a..d317c6aba06 100644
--- a/Makefile
+++ b/Makefile
@@ -506,14 +506,14 @@ include shared.mak
 # To use them you must define NO_DC_SHA1 and one of the *_SHA1
 # variables below:
 #
-# 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 the SHA-1 routines from
+# the OpenSSL library.
 #
-# Define PPC_SHA1 environment variable when running make to make use of
-# a bundled SHA1 routine optimized for PowerPC.
+# Define PPC_SHA1 to make use of optimized (in assembly)
+# PowerPC SHA-1 routines.
 #
 # Define APPLE_SHA1 to use Apple's CommonCrypto SHA-1 routines on
 # Darwin/Mac OS X.
-- 
2.36.0.879.g56a83971f3f


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH 4/5] Makefile + hash.h: remove PPC_SHA1 implementation
  2022-04-22  9:53 [PATCH 0/5] core: update our SHA-1 docs, use sha1collisiondetection on OSX too Ævar Arnfjörð Bjarmason
                   ` (2 preceding siblings ...)
  2022-04-22  9:53 ` [PATCH 3/5] Makefile: rephrase the discussion of *_SHA1 knobs Ævar Arnfjörð Bjarmason
@ 2022-04-22  9:53 ` Ævar Arnfjörð Bjarmason
  2022-04-22  9:53 ` [PATCH 5/5] Makefile: use $(OBJECTS) instead of $(C_OBJ) Ævar Arnfjörð Bjarmason
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-04-22  9:53 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, brian m . carlson, Carlo Arenas, Mike Hommey,
	Eric Sunshine, Ævar Arnfjörð Bjarmason

Remove the PPC_SHA1 implementation added in a6ef3518f9a ([PATCH] PPC
assembly implementation of SHA1, 2005-04-22). When this was added
Apple consumer hardware used the PPC architecture, and the
implementation was intended to improve SHA-1 speed there.

Since it was added we've moved to using sha1collisiondetection by
default, and anyone wanting hard-rolled non-DC SHA-1 implementation
can use OpenSSL's via the OPENSSL_SHA1 knob.

Furthermore this doesn't run on the modern PPC processors which anyone
who's concerned about performance on PPC is likely to be using these
days, i.e. the IBM POWER series. It originally originally targeted 32
bit PPC, but there's some mailing list references to this being tried
on G5 (PPC 970).

I can't get it to do anything but segfault on both the BE and LE POWER
machines in the GCC compile farm.

There have been proposals to entirely remove non-sha1collisiondetection
implementations from the tree[1]. I think per [2] that would be a bit
overzealous. I.e. there are various set-ups git's speed is going to be
more important than the relatively implausible SHA-1 collision attack,
or where such attacks are entirely mitigated by other means (e.g. by
incoming objects being checked with DC_SHA1).

But that really doesn't apply to PPC_SHA1 in particular, which seems
to have outlived its usefulness.

As this gets rid of the only in-tree *.S assembly file we can remove
the small bits of logic from the Makefile needed to build objects
from *.S (as opposed to *.c)

The code being removed here was also throwing warnings with the
"-pedantic" flag, it could have been fixed as 544d93bc3b4 (block-sha1:
remove use of obsolete x86 assembly, 2022-03-10) did for block-sha1/*,
but as noted above let's remove it instead.

1. https://lore.kernel.org/git/20200223223758.120941-1-mh@glandium.org/
2. https://lore.kernel.org/git/20200224044732.GK1018190@coredump.intra.peff.net/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile      |  19 ++---
 configure.ac  |   3 -
 hash.h        |   4 +-
 ppc/sha1.c    |  72 ----------------
 ppc/sha1.h    |  25 ------
 ppc/sha1ppc.S | 224 --------------------------------------------------
 6 files changed, 6 insertions(+), 341 deletions(-)
 delete mode 100644 ppc/sha1.c
 delete mode 100644 ppc/sha1.h
 delete mode 100644 ppc/sha1ppc.S

diff --git a/Makefile b/Makefile
index d317c6aba06..6f8272ae4a4 100644
--- a/Makefile
+++ b/Makefile
@@ -512,9 +512,6 @@ include shared.mak
 # Define OPENSSL_SHA1 to link to the the SHA-1 routines from
 # the OpenSSL library.
 #
-# Define PPC_SHA1 to make use of optimized (in assembly)
-# PowerPC SHA-1 routines.
-#
 # Define APPLE_SHA1 to use Apple's CommonCrypto SHA-1 routines on
 # Darwin/Mac OS X.
 #
@@ -1830,8 +1827,11 @@ endif
 ifdef DC_SHA1
 $(error the DC_SHA1 flag is no longer used, and has become the default. Adjust your build scripts accordingly)
 endif
+ifdef PPC_SHA1
+$(error the PPC_SHA1 flag has been removed along with the PowerPC-specific SHA-1 implementation.)
+endif
 ifndef NO_DC_SHA1
-	ifneq ($(OPENSSL_SHA1)$(BLK_SHA1)$(PPC_SHA1)$(APPLE_SHA1),)
+	ifneq ($(OPENSSL_SHA1)$(BLK_SHA1)$(APPLE_SHA1),)
 $(error no other *_SHA1 option can be defined unless NO_DC_SHA1 is defined)
 	endif
 	LIB_OBJS += sha1dc_git.o
@@ -1869,10 +1869,6 @@ ifdef BLK_SHA1
 	LIB_OBJS += block-sha1/sha1.o
 	BASIC_CFLAGS += -DSHA1_BLK
 else
-ifdef PPC_SHA1
-	LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
-	BASIC_CFLAGS += -DSHA1_PPC
-else
 ifdef APPLE_SHA1
 	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
 	BASIC_CFLAGS += -DSHA1_APPLE
@@ -1882,7 +1878,6 @@ endif
 endif
 endif
 endif
-endif
 
 ifdef OPENSSL_SHA256
 	EXTLIBS += $(LIB_4_CRYPTO)
@@ -2621,14 +2616,10 @@ missing_compdb_dir =
 compdb_args =
 endif
 
-ASM_SRC := $(wildcard $(OBJECTS:o=S))
-ASM_OBJ := $(ASM_SRC:S=o)
-C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
+C_OBJ = $(OBJECTS)
 
 $(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
 	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
-$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
-	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
 
 %.s: %.c GIT-CFLAGS FORCE
 	$(QUIET_CC)$(CC) -o $@ -S $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
diff --git a/configure.ac b/configure.ac
index 316a31d2313..bbfbcc63af0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -237,9 +237,6 @@ AC_MSG_NOTICE([CHECKS for site configuration])
 # tests.  These tests take up a significant amount of the total test time
 # but are not needed unless you plan to talk to SVN repos.
 #
-# Define PPC_SHA1 environment variable when running make to make use of
-# a bundled SHA1 routine optimized for PowerPC.
-#
 # Define NO_OPENSSL environment variable if you do not have OpenSSL.
 #
 # Define OPENSSLDIR=/foo/bar if your openssl header and library files are in
diff --git a/hash.h b/hash.h
index 20012167ce4..248615df11d 100644
--- a/hash.h
+++ b/hash.h
@@ -6,8 +6,6 @@
 
 #if !defined(NO_SHA1_DC)
 #include "sha1dc_git.h"
-#elif defined(SHA1_PPC)
-#include "ppc/sha1.h"
 #elif defined(SHA1_APPLE)
 #include <CommonCrypto/CommonDigest.h>
 #elif defined(SHA1_OPENSSL)
@@ -32,7 +30,7 @@
  * platform's underlying implementation of SHA-1; could be OpenSSL,
  * blk_SHA, Apple CommonCrypto, etc...  Note that the relevant
  * SHA-1 header may have already defined platform_SHA_CTX for our
- * own implementations like block-sha1 and ppc-sha1, so we list
+ * own implementations like block-sha1, so we list
  * the default for OpenSSL compatible SHA-1 implementations here.
  */
 #define platform_SHA_CTX	SHA_CTX
diff --git a/ppc/sha1.c b/ppc/sha1.c
deleted file mode 100644
index 1b705cee1fe..00000000000
--- a/ppc/sha1.c
+++ /dev/null
@@ -1,72 +0,0 @@
-/*
- * SHA-1 implementation.
- *
- * Copyright (C) 2005 Paul Mackerras <paulus@samba.org>
- *
- * This version assumes we are running on a big-endian machine.
- * It calls an external sha1_core() to process blocks of 64 bytes.
- */
-#include <stdio.h>
-#include <string.h>
-#include "sha1.h"
-
-void ppc_sha1_core(uint32_t *hash, const unsigned char *p,
-		   unsigned int nblocks);
-
-int ppc_SHA1_Init(ppc_SHA_CTX *c)
-{
-	c->hash[0] = 0x67452301;
-	c->hash[1] = 0xEFCDAB89;
-	c->hash[2] = 0x98BADCFE;
-	c->hash[3] = 0x10325476;
-	c->hash[4] = 0xC3D2E1F0;
-	c->len = 0;
-	c->cnt = 0;
-	return 0;
-}
-
-int ppc_SHA1_Update(ppc_SHA_CTX *c, const void *ptr, unsigned long n)
-{
-	unsigned long nb;
-	const unsigned char *p = ptr;
-
-	c->len += (uint64_t) n << 3;
-	while (n != 0) {
-		if (c->cnt || n < 64) {
-			nb = 64 - c->cnt;
-			if (nb > n)
-				nb = n;
-			memcpy(&c->buf.b[c->cnt], p, nb);
-			if ((c->cnt += nb) == 64) {
-				ppc_sha1_core(c->hash, c->buf.b, 1);
-				c->cnt = 0;
-			}
-		} else {
-			nb = n >> 6;
-			ppc_sha1_core(c->hash, p, nb);
-			nb <<= 6;
-		}
-		n -= nb;
-		p += nb;
-	}
-	return 0;
-}
-
-int ppc_SHA1_Final(unsigned char *hash, ppc_SHA_CTX *c)
-{
-	unsigned int cnt = c->cnt;
-
-	c->buf.b[cnt++] = 0x80;
-	if (cnt > 56) {
-		if (cnt < 64)
-			memset(&c->buf.b[cnt], 0, 64 - cnt);
-		ppc_sha1_core(c->hash, c->buf.b, 1);
-		cnt = 0;
-	}
-	if (cnt < 56)
-		memset(&c->buf.b[cnt], 0, 56 - cnt);
-	c->buf.l[7] = c->len;
-	ppc_sha1_core(c->hash, c->buf.b, 1);
-	memcpy(hash, c->hash, 20);
-	return 0;
-}
diff --git a/ppc/sha1.h b/ppc/sha1.h
deleted file mode 100644
index 9b24b326159..00000000000
--- a/ppc/sha1.h
+++ /dev/null
@@ -1,25 +0,0 @@
-/*
- * SHA-1 implementation.
- *
- * Copyright (C) 2005 Paul Mackerras <paulus@samba.org>
- */
-#include <stdint.h>
-
-typedef struct {
-	uint32_t hash[5];
-	uint32_t cnt;
-	uint64_t len;
-	union {
-		unsigned char b[64];
-		uint64_t l[8];
-	} buf;
-} ppc_SHA_CTX;
-
-int ppc_SHA1_Init(ppc_SHA_CTX *c);
-int ppc_SHA1_Update(ppc_SHA_CTX *c, const void *p, unsigned long n);
-int ppc_SHA1_Final(unsigned char *hash, ppc_SHA_CTX *c);
-
-#define platform_SHA_CTX	ppc_SHA_CTX
-#define platform_SHA1_Init	ppc_SHA1_Init
-#define platform_SHA1_Update	ppc_SHA1_Update
-#define platform_SHA1_Final	ppc_SHA1_Final
diff --git a/ppc/sha1ppc.S b/ppc/sha1ppc.S
deleted file mode 100644
index 1711eef6e71..00000000000
--- a/ppc/sha1ppc.S
+++ /dev/null
@@ -1,224 +0,0 @@
-/*
- * SHA-1 implementation for PowerPC.
- *
- * Copyright (C) 2005 Paul Mackerras <paulus@samba.org>
- */
-
-/*
- * PowerPC calling convention:
- * %r0 - volatile temp
- * %r1 - stack pointer.
- * %r2 - reserved
- * %r3-%r12 - Incoming arguments & return values; volatile.
- * %r13-%r31 - Callee-save registers
- * %lr - Return address, volatile
- * %ctr - volatile
- *
- * Register usage in this routine:
- * %r0 - temp
- * %r3 - argument (pointer to 5 words of SHA state)
- * %r4 - argument (pointer to data to hash)
- * %r5 - Constant K in SHA round (initially number of blocks to hash)
- * %r6-%r10 - Working copies of SHA variables A..E (actually E..A order)
- * %r11-%r26 - Data being hashed W[].
- * %r27-%r31 - Previous copies of A..E, for final add back.
- * %ctr - loop count
- */
-
-
-/*
- * We roll the registers for A, B, C, D, E around on each
- * iteration; E on iteration t is D on iteration t+1, and so on.
- * We use registers 6 - 10 for this.  (Registers 27 - 31 hold
- * the previous values.)
- */
-#define RA(t)	(((t)+4)%5+6)
-#define RB(t)	(((t)+3)%5+6)
-#define RC(t)	(((t)+2)%5+6)
-#define RD(t)	(((t)+1)%5+6)
-#define RE(t)	(((t)+0)%5+6)
-
-/* We use registers 11 - 26 for the W values */
-#define W(t)	((t)%16+11)
-
-/* Register 5 is used for the constant k */
-
-/*
- * The basic SHA-1 round function is:
- * E += ROTL(A,5) + F(B,C,D) + W[i] + K;  B = ROTL(B,30)
- * Then the variables are renamed: (A,B,C,D,E) = (E,A,B,C,D).
- *
- * Every 20 rounds, the function F() and the constant K changes:
- * - 20 rounds of f0(b,c,d) = "bit wise b ? c : d" =  (^b & d) + (b & c)
- * - 20 rounds of f1(b,c,d) = b^c^d = (b^d)^c
- * - 20 rounds of f2(b,c,d) = majority(b,c,d) = (b&d) + ((b^d)&c)
- * - 20 more rounds of f1(b,c,d)
- *
- * These are all scheduled for near-optimal performance on a G4.
- * The G4 is a 3-issue out-of-order machine with 3 ALUs, but it can only
- * *consider* starting the oldest 3 instructions per cycle.  So to get
- * maximum performance out of it, you have to treat it as an in-order
- * machine.  Which means interleaving the computation round t with the
- * computation of W[t+4].
- *
- * The first 16 rounds use W values loaded directly from memory, while the
- * remaining 64 use values computed from those first 16.  We preload
- * 4 values before starting, so there are three kinds of rounds:
- * - The first 12 (all f0) also load the W values from memory.
- * - The next 64 compute W(i+4) in parallel. 8*f0, 20*f1, 20*f2, 16*f1.
- * - The last 4 (all f1) do not do anything with W.
- *
- * Therefore, we have 6 different round functions:
- * STEPD0_LOAD(t,s) - Perform round t and load W(s).  s < 16
- * STEPD0_UPDATE(t,s) - Perform round t and compute W(s).  s >= 16.
- * STEPD1_UPDATE(t,s)
- * STEPD2_UPDATE(t,s)
- * STEPD1(t) - Perform round t with no load or update.
- *
- * The G5 is more fully out-of-order, and can find the parallelism
- * by itself.  The big limit is that it has a 2-cycle ALU latency, so
- * even though it's 2-way, the code has to be scheduled as if it's
- * 4-way, which can be a limit.  To help it, we try to schedule the
- * read of RA(t) as late as possible so it doesn't stall waiting for
- * the previous round's RE(t-1), and we try to rotate RB(t) as early
- * as possible while reading RC(t) (= RB(t-1)) as late as possible.
- */
-
-/* the initial loads. */
-#define LOADW(s) \
-	lwz	W(s),(s)*4(%r4)
-
-/*
- * Perform a step with F0, and load W(s).  Uses W(s) as a temporary
- * before loading it.
- * This is actually 10 instructions, which is an awkward fit.
- * It can execute grouped as listed, or delayed one instruction.
- * (If delayed two instructions, there is a stall before the start of the
- * second line.)  Thus, two iterations take 7 cycles, 3.5 cycles per round.
- */
-#define STEPD0_LOAD(t,s) \
-add RE(t),RE(t),W(t); andc   %r0,RD(t),RB(t);  and    W(s),RC(t),RB(t); \
-add RE(t),RE(t),%r0;  rotlwi %r0,RA(t),5;      rotlwi RB(t),RB(t),30;   \
-add RE(t),RE(t),W(s); add    %r0,%r0,%r5;      lwz    W(s),(s)*4(%r4);  \
-add RE(t),RE(t),%r0
-
-/*
- * This is likewise awkward, 13 instructions.  However, it can also
- * execute starting with 2 out of 3 possible moduli, so it does 2 rounds
- * in 9 cycles, 4.5 cycles/round.
- */
-#define STEPD0_UPDATE(t,s,loadk...) \
-add RE(t),RE(t),W(t); andc   %r0,RD(t),RB(t); xor    W(s),W((s)-16),W((s)-3); \
-add RE(t),RE(t),%r0;  and    %r0,RC(t),RB(t); xor    W(s),W(s),W((s)-8);      \
-add RE(t),RE(t),%r0;  rotlwi %r0,RA(t),5;     xor    W(s),W(s),W((s)-14);     \
-add RE(t),RE(t),%r5;  loadk; rotlwi RB(t),RB(t),30;  rotlwi W(s),W(s),1;     \
-add RE(t),RE(t),%r0
-
-/* Nicely optimal.  Conveniently, also the most common. */
-#define STEPD1_UPDATE(t,s,loadk...) \
-add RE(t),RE(t),W(t); xor    %r0,RD(t),RB(t); xor    W(s),W((s)-16),W((s)-3); \
-add RE(t),RE(t),%r5;  loadk; xor %r0,%r0,RC(t);  xor W(s),W(s),W((s)-8);      \
-add RE(t),RE(t),%r0;  rotlwi %r0,RA(t),5;     xor    W(s),W(s),W((s)-14);     \
-add RE(t),RE(t),%r0;  rotlwi RB(t),RB(t),30;  rotlwi W(s),W(s),1
-
-/*
- * The naked version, no UPDATE, for the last 4 rounds.  3 cycles per.
- * We could use W(s) as a temp register, but we don't need it.
- */
-#define STEPD1(t) \
-                        add   RE(t),RE(t),W(t); xor    %r0,RD(t),RB(t); \
-rotlwi RB(t),RB(t),30;  add   RE(t),RE(t),%r5;  xor    %r0,%r0,RC(t);   \
-add    RE(t),RE(t),%r0; rotlwi %r0,RA(t),5;     /* spare slot */        \
-add    RE(t),RE(t),%r0
-
-/*
- * 14 instructions, 5 cycles per.  The majority function is a bit
- * awkward to compute.  This can execute with a 1-instruction delay,
- * but it causes a 2-instruction delay, which triggers a stall.
- */
-#define STEPD2_UPDATE(t,s,loadk...) \
-add RE(t),RE(t),W(t); and    %r0,RD(t),RB(t); xor    W(s),W((s)-16),W((s)-3); \
-add RE(t),RE(t),%r0;  xor    %r0,RD(t),RB(t); xor    W(s),W(s),W((s)-8);      \
-add RE(t),RE(t),%r5;  loadk; and %r0,%r0,RC(t);  xor W(s),W(s),W((s)-14);     \
-add RE(t),RE(t),%r0;  rotlwi %r0,RA(t),5;     rotlwi W(s),W(s),1;             \
-add RE(t),RE(t),%r0;  rotlwi RB(t),RB(t),30
-
-#define STEP0_LOAD4(t,s)		\
-	STEPD0_LOAD(t,s);		\
-	STEPD0_LOAD((t+1),(s)+1);	\
-	STEPD0_LOAD((t)+2,(s)+2);	\
-	STEPD0_LOAD((t)+3,(s)+3)
-
-#define STEPUP4(fn, t, s, loadk...)		\
-	STEP##fn##_UPDATE(t,s,);		\
-	STEP##fn##_UPDATE((t)+1,(s)+1,);	\
-	STEP##fn##_UPDATE((t)+2,(s)+2,);	\
-	STEP##fn##_UPDATE((t)+3,(s)+3,loadk)
-
-#define STEPUP20(fn, t, s, loadk...)	\
-	STEPUP4(fn, t, s,);		\
-	STEPUP4(fn, (t)+4, (s)+4,);	\
-	STEPUP4(fn, (t)+8, (s)+8,);	\
-	STEPUP4(fn, (t)+12, (s)+12,);	\
-	STEPUP4(fn, (t)+16, (s)+16, loadk)
-
-	.globl	ppc_sha1_core
-ppc_sha1_core:
-	stwu	%r1,-80(%r1)
-	stmw	%r13,4(%r1)
-
-	/* Load up A - E */
-	lmw	%r27,0(%r3)
-
-	mtctr	%r5
-
-1:
-	LOADW(0)
-	lis	%r5,0x5a82
-	mr	RE(0),%r31
-	LOADW(1)
-	mr	RD(0),%r30
-	mr	RC(0),%r29
-	LOADW(2)
-	ori	%r5,%r5,0x7999	/* K0-19 */
-	mr	RB(0),%r28
-	LOADW(3)
-	mr	RA(0),%r27
-
-	STEP0_LOAD4(0, 4)
-	STEP0_LOAD4(4, 8)
-	STEP0_LOAD4(8, 12)
-	STEPUP4(D0, 12, 16,)
-	STEPUP4(D0, 16, 20, lis %r5,0x6ed9)
-
-	ori	%r5,%r5,0xeba1	/* K20-39 */
-	STEPUP20(D1, 20, 24, lis %r5,0x8f1b)
-
-	ori	%r5,%r5,0xbcdc	/* K40-59 */
-	STEPUP20(D2, 40, 44, lis %r5,0xca62)
-
-	ori	%r5,%r5,0xc1d6	/* K60-79 */
-	STEPUP4(D1, 60, 64,)
-	STEPUP4(D1, 64, 68,)
-	STEPUP4(D1, 68, 72,)
-	STEPUP4(D1, 72, 76,)
-	addi	%r4,%r4,64
-	STEPD1(76)
-	STEPD1(77)
-	STEPD1(78)
-	STEPD1(79)
-
-	/* Add results to original values */
-	add	%r31,%r31,RE(0)
-	add	%r30,%r30,RD(0)
-	add	%r29,%r29,RC(0)
-	add	%r28,%r28,RB(0)
-	add	%r27,%r27,RA(0)
-
-	bdnz	1b
-
-	/* Save final hash, restore registers, and return */
-	stmw	%r27,0(%r3)
-	lmw	%r13,4(%r1)
-	addi	%r1,%r1,80
-	blr
-- 
2.36.0.879.g56a83971f3f


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH 5/5] Makefile: use $(OBJECTS) instead of $(C_OBJ)
  2022-04-22  9:53 [PATCH 0/5] core: update our SHA-1 docs, use sha1collisiondetection on OSX too Ævar Arnfjörð Bjarmason
                   ` (3 preceding siblings ...)
  2022-04-22  9:53 ` [PATCH 4/5] Makefile + hash.h: remove PPC_SHA1 implementation Ævar Arnfjörð Bjarmason
@ 2022-04-22  9:53 ` Ævar Arnfjörð Bjarmason
  2022-04-22 18:56 ` [PATCH 0/5] core: update our SHA-1 docs, use sha1collisiondetection on OSX too Junio C Hamano
  2022-10-19  1:03 ` [PATCH v2 0/4] " Ævar Arnfjörð Bjarmason
  6 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-04-22  9:53 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, brian m . carlson, Carlo Arenas, Mike Hommey,
	Eric Sunshine, Ævar Arnfjörð Bjarmason

In the preceding commit $(C_OBJ) added in c373991375a (Makefile: list
generated object files in OBJECTS, 2010-01-26) became synonymous with
$(OBJECTS). Let's avoid the indirection and use the $(OBJECTS)
variable directly instead.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 6f8272ae4a4..ff6bab134d2 100644
--- a/Makefile
+++ b/Makefile
@@ -2616,9 +2616,7 @@ missing_compdb_dir =
 compdb_args =
 endif
 
-C_OBJ = $(OBJECTS)
-
-$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
+$(OBJECTS): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
 	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
 
 %.s: %.c GIT-CFLAGS FORCE
@@ -2764,7 +2762,7 @@ XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell \
 	--keyword=gettextln --keyword=eval_gettextln
 XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --language=Perl \
 	--keyword=__ --keyword=N__ --keyword="__n:1,2"
-LOCALIZED_C = $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H)
+LOCALIZED_C = $(OBJECTS:o=c) $(LIB_H) $(GENERATED_H)
 LOCALIZED_SH = $(SCRIPT_SH)
 LOCALIZED_SH += git-sh-setup.sh
 LOCALIZED_PERL = $(SCRIPT_PERL)
@@ -3014,7 +3012,7 @@ t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS) $(REFTABLE_TEST_LIB)
 check-sha1:: t/helper/test-tool$X
 	t/helper/test-sha1.sh
 
-SP_OBJ = $(patsubst %.o,%.sp,$(C_OBJ))
+SP_OBJ = $(patsubst %.o,%.sp,$(OBJECTS))
 
 $(SP_OBJ): %.sp: %.c %.o
 	$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \
-- 
2.36.0.879.g56a83971f3f


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* Re: [PATCH 0/5] core: update our SHA-1 docs, use sha1collisiondetection on OSX too
  2022-04-22  9:53 [PATCH 0/5] core: update our SHA-1 docs, use sha1collisiondetection on OSX too Ævar Arnfjörð Bjarmason
                   ` (4 preceding siblings ...)
  2022-04-22  9:53 ` [PATCH 5/5] Makefile: use $(OBJECTS) instead of $(C_OBJ) Ævar Arnfjörð Bjarmason
@ 2022-04-22 18:56 ` Junio C Hamano
  2022-05-19 20:14   ` Ævar Arnfjörð Bjarmason
  2022-10-19  1:03 ` [PATCH v2 0/4] " Ævar Arnfjörð Bjarmason
  6 siblings, 1 reply; 55+ messages in thread
From: Junio C Hamano @ 2022-04-22 18:56 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason, git-packagers
  Cc: git, brian m . carlson, Carlo Arenas, Mike Hommey, Eric Sunshine

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> I wasn't able to find any on-list references to it being intentional,
> but it appears that while we made the sha1collisiondetection variant
> of SHA-1 the default in early 2017 we've never updated the OSX builds
> to do likewise.
>
> I don't know what various git packages for OSX to, but our vanilla OSX
> distribution definitely uses Apple Common Crypto, and won't detect the
> https://shattered.io attack.
>
> This series changes that, and while doing so in 2/5 updates our
> documentation and Makefile interface for the SHA-1 selection. Our
> INSTALL file was still claiming we used OpenSSL's SHA-1 by default.
>
> Then since we'd made sha1collisiondetection the default we hadn't
> changed the code's default fallback to be that, it was still
> block-sha1. Now our fallback behavior is "error" instead, which makes
> it less likely that we'll get some foot-gun like the "OSX not using
> sha1collisiondetection" again.
>
> The 4/5 and 5/5 then remove the PPC_SHA1 implementation. I submitted
> this before as [1], and the range-diff is to that submission (it
> wasn't picked up). I think it makes sense as part of this general
> SHA-1 cleanup.

Thanks for this effort.

I'd like to see somebody with "building Git for distributing to
macOS" background to comment (I am assuming that the mailing list
git-packagers@googlegroups.com is the way to reach them).


^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 0/5] core: update our SHA-1 docs, use sha1collisiondetection on OSX too
  2022-04-22 18:56 ` [PATCH 0/5] core: update our SHA-1 docs, use sha1collisiondetection on OSX too Junio C Hamano
@ 2022-05-19 20:14   ` Ævar Arnfjörð Bjarmason
  2022-05-26 19:02     ` Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-05-19 20:14 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git-packagers, git, brian m . carlson, Carlo Arenas, Mike Hommey,
	Eric Sunshine, git-packagers, Tim Harper


On Fri, Apr 22 2022, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> I wasn't able to find any on-list references to it being intentional,
>> but it appears that while we made the sha1collisiondetection variant
>> of SHA-1 the default in early 2017 we've never updated the OSX builds
>> to do likewise.
>>
>> I don't know what various git packages for OSX to, but our vanilla OSX
>> distribution definitely uses Apple Common Crypto, and won't detect the
>> https://shattered.io attack.
>>
>> This series changes that, and while doing so in 2/5 updates our
>> documentation and Makefile interface for the SHA-1 selection. Our
>> INSTALL file was still claiming we used OpenSSL's SHA-1 by default.
>>
>> Then since we'd made sha1collisiondetection the default we hadn't
>> changed the code's default fallback to be that, it was still
>> block-sha1. Now our fallback behavior is "error" instead, which makes
>> it less likely that we'll get some foot-gun like the "OSX not using
>> sha1collisiondetection" again.
>>
>> The 4/5 and 5/5 then remove the PPC_SHA1 implementation. I submitted
>> this before as [1], and the range-diff is to that submission (it
>> wasn't picked up). I think it makes sense as part of this general
>> SHA-1 cleanup.
>
> Thanks for this effort.
>
> I'd like to see somebody with "building Git for distributing to
> macOS" background to comment (I am assuming that the mailing list
> git-packagers@googlegroups.com is the way to reach them).

*Bump* in case anyone there would like to chime on this bit of OSX
portability.

I also sent an off-list E-Mail to Tim Harper today, and addresses which
I gather (from some old git-security@ traffic) are involved in packaging
the Apple Git shipped with OSX itself.

In the meantime do you mind if this topic were queued up? If there are
any lingering portability concerns getting it into CI and exposed to
anyone else building on OSX would be a good thing.

I don't see any reason for why we'd decide that OSX out of all our
supported platforms should be the only one where we're not mitigating
the SHAttered attack (and similar future attacks) by default.

So aside from any finer details of OSX portability the direction here of
building with sha1collisiondetection by default on OSX by default seems
like a safe bet, just as we do on the rest of our (checks out
config.mak.uname) 20+ supported platforms (per `uname -s`).

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH 0/5] core: update our SHA-1 docs, use sha1collisiondetection on OSX too
  2022-05-19 20:14   ` Ævar Arnfjörð Bjarmason
@ 2022-05-26 19:02     ` Ævar Arnfjörð Bjarmason
  0 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-05-26 19:02 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git-packagers, git, brian m . carlson, Carlo Arenas, Mike Hommey,
	Eric Sunshine, git-packagers, Tim Harper, Jeff King


On Thu, May 19 2022, Ævar Arnfjörð Bjarmason wrote:

> On Fri, Apr 22 2022, Junio C Hamano wrote:
>
>> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>>
>>> I wasn't able to find any on-list references to it being intentional,
>>> but it appears that while we made the sha1collisiondetection variant
>>> of SHA-1 the default in early 2017 we've never updated the OSX builds
>>> to do likewise.
>>>
>>> I don't know what various git packages for OSX to, but our vanilla OSX
>>> distribution definitely uses Apple Common Crypto, and won't detect the
>>> https://shattered.io attack.
>>>
>>> This series changes that, and while doing so in 2/5 updates our
>>> documentation and Makefile interface for the SHA-1 selection. Our
>>> INSTALL file was still claiming we used OpenSSL's SHA-1 by default.
>>>
>>> Then since we'd made sha1collisiondetection the default we hadn't
>>> changed the code's default fallback to be that, it was still
>>> block-sha1. Now our fallback behavior is "error" instead, which makes
>>> it less likely that we'll get some foot-gun like the "OSX not using
>>> sha1collisiondetection" again.
>>>
>>> The 4/5 and 5/5 then remove the PPC_SHA1 implementation. I submitted
>>> this before as [1], and the range-diff is to that submission (it
>>> wasn't picked up). I think it makes sense as part of this general
>>> SHA-1 cleanup.
>>
>> Thanks for this effort.
>>
>> I'd like to see somebody with "building Git for distributing to
>> macOS" background to comment (I am assuming that the mailing list
>> git-packagers@googlegroups.com is the way to reach them).
>
> *Bump* in case anyone there would like to chime on this bit of OSX
> portability.
>
> I also sent an off-list E-Mail to Tim Harper today, and addresses which
> I gather (from some old git-security@ traffic) are involved in packaging
> the Apple Git shipped with OSX itself.
>
> In the meantime do you mind if this topic were queued up? If there are
> any lingering portability concerns getting it into CI and exposed to
> anyone else building on OSX would be a good thing.
>
> I don't see any reason for why we'd decide that OSX out of all our
> supported platforms should be the only one where we're not mitigating
> the SHAttered attack (and similar future attacks) by default.
>
> So aside from any finer details of OSX portability the direction here of
> building with sha1collisiondetection by default on OSX by default seems
> like a safe bet, just as we do on the rest of our (checks out
> config.mak.uname) 20+ supported platforms (per `uname -s`).

Update: I didn't get a reply from Tim or the people I E-Mailed
@apple.com a week ago.

Junio: I still think it makes sense to just queue this anyway & finally
get the OSX build to use the SHAttered mitigation sha1collisiondetection
gives us by default.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH v2 0/4] core: update our SHA-1 docs, use sha1collisiondetection on OSX too
  2022-04-22  9:53 [PATCH 0/5] core: update our SHA-1 docs, use sha1collisiondetection on OSX too Ævar Arnfjörð Bjarmason
                   ` (5 preceding siblings ...)
  2022-04-22 18:56 ` [PATCH 0/5] core: update our SHA-1 docs, use sha1collisiondetection on OSX too Junio C Hamano
@ 2022-10-19  1:03 ` Ævar Arnfjörð Bjarmason
  2022-10-19  1:03   ` [PATCH v2 1/4] fsmonitor OSX: compile with DC_SHA1=YesPlease Ævar Arnfjörð Bjarmason
                     ` (4 more replies)
  6 siblings, 5 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-19  1:03 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

The sha1collisiondetection alternative to SHA-1 has been our default
everywhere for a while, except on OSX. As 3/4 notes this seems to have
been a mistake.

Furthermore our documentation (INSTALL) was still claiming that we
used OpenSSL for SHA-1 by default, etc. See 3/4 for all the details.

This series was previously submitted in April[1], at the time it
included a change to remove the SHA-1 implementation in PPC
assembly. That part has since landed on master in fd1ec82547d (Merge
branch 'ab/retire-ppc-sha1', 2022-09-09).

I meant to re-send this anyway, but was prompted by the failure
report(s) in [2]. The 1/4 here un-breaks "master", and could be peeled
off.

For a branch for this & passing CI see:
https://github.com/avar/git/tree/avar/sha1dc-really-everywhere-and-no-ppc-sha1-2

1. https://lore.kernel.org/git/cover-0.5-00000000000-20220422T094624Z-avarab@gmail.com/
2. https://lore.kernel.org/git/xmqqo7u9wyt7.fsf@gitster.g/

Ævar Arnfjörð Bjarmason (4):
  fsmonitor OSX: compile with DC_SHA1=YesPlease
  Makefile: create and use sections for "define" flag listing
  Makefile: really use and document sha1collisiondetection by default
  Makefile: rephrase the discussion of *_SHA1 knobs

 INSTALL                             |  10 +-
 Makefile                            | 288 +++++++++++++++++-----------
 ci/lib.sh                           |   3 +
 compat/fsmonitor/fsm-ipc-darwin.c   |  10 +-
 contrib/buildsystems/CMakeLists.txt |   3 +-
 hash.h                              |  10 +-
 t/t0013-sha1dc.sh                   |   4 +-
 7 files changed, 198 insertions(+), 130 deletions(-)

Range-diff against v1:
-:  ----------- > 1:  392fabdb456 fsmonitor OSX: compile with DC_SHA1=YesPlease
1:  3a80fcb6784 ! 2:  7ae22276aa7 Makefile: create and use sections for "define" flag listing
    @@ Makefile: include shared.mak
     -# Define BLK_SHA1 environment variable to make use of the bundled
     -# optimized C SHA1 routine.
     -#
    --# Define PPC_SHA1 environment variable when running make to make use of
    --# a bundled SHA1 routine optimized for PowerPC.
    --#
     -# Define DC_SHA1 to unconditionally enable the collision-detecting sha1
     -# algorithm. This is slower, but may detect attempted collision attacks.
     -# Takes priority over other *_SHA1 knobs.
    @@ Makefile: include shared.mak
     -#
     -# 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.
    @@ Makefile: include shared.mak
      #
      # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
     @@ Makefile: include shared.mak
    - # `compat/fsmonitor/fsm-listen-<name>.c` that implements the
    - # `fsm_listen__*()` routines.
    + # to the "<name>" of the corresponding `compat/fsmonitor/fsm-settings-<name>.c`
    + # that implements the `fsm_os_settings__*()` routines.
      #
     +# === Optional library: libintl ===
     +#
    @@ Makefile: include shared.mak
     +# Define BLK_SHA1 environment variable to make use of the bundled
     +# optimized C SHA1 routine.
     +#
    -+# Define PPC_SHA1 environment variable when running make to make use of
    -+# a bundled SHA1 routine optimized for PowerPC.
    -+#
     +# Define DC_SHA1 to unconditionally enable the collision-detecting sha1
     +# algorithm. This is slower, but may detect attempted collision attacks.
     +# Takes priority over other *_SHA1 knobs.
    @@ Makefile: include shared.mak
     +#
     +# 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.
2:  3e250dc9d09 ! 3:  78ef8636c57 Makefile: really use and document sha1collisiondetection by default
    @@ Commit message
         * Have a rewritten discussion of SHA-1 and SHA-256 in the Makefile
           which covers all of this.
     
    +    Let's also change the CI for "osx-clang" to test with the new
    +    APPLE_SHA1 knob ("osx-gcc" uses the new sha1collisiondetection
    +    default).
    +
    +    In practice this will spot issues like the one noted in [7], as
    +    testing with just two backends should be enough to spot unportable
    +    code. Ideally we'd have other CI jobs to test the various SHA-1
    +    combinations, but for now we have better CI coverage than before.
    +
         1. 48b3693d3ce (Merge branch 'jk/sha1dc', 2017-03-24)
         2. 8325e43b82d (Makefile: add DC_SHA1 knob, 2017-03-16)
         3. e6b07da2780 (Makefile: make DC_SHA1 the default, 2017-03-17)
    @@ Commit message
            2013-05-19)
         6. 3ef2bcad02e (imap-send: use Apple's Security framework for base64
            encoding, 2013-07-29)
    +    7. https://lore.kernel.org/git/kl6l7d0yyu6r.fsf@chooglen-macbookpro.roam.corp.google.com/
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    @@ INSTALL: Issues of note:
      
     -	  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.  Also included is a version optimized for PowerPC
    --	  (PPC_SHA1).
    +-	  BLK_SHA1.
     +	- Git uses an altered version of SHA-1 by default which
    -+          detects the SHAttered attack via the sha1collisiondetection
    -+          counter-cryptanalysis library. For SHA-256 we'll select a
    -+          working implementation (and ship a fallback
    -+          implementation). See the "SHA-1 and SHA-256 defines" section
    -+          in the Makefile for details. You should not need to tweak
    -+          those settings.
    ++	  detects the SHAttered attack via the sha1collisiondetection
    ++	  counter-cryptanalysis library. For SHA-256 we'll select a
    ++	  working implementation (and ship a fallback
    ++	  implementation). See the "SHA-1 and SHA-256 defines" section
    ++	  in the Makefile for details. You should not need to tweak
    ++	  those settings.
      
      	- "libcurl" library is used for fetching and pushing
      	  repositories over http:// or https://, as well as by
    @@ Makefile: include shared.mak
     -# Define BLK_SHA1 environment variable to make use of the bundled
     -# optimized C SHA1 routine.
     +# === SHA-1 backend ===
    - #
    --# Define PPC_SHA1 environment variable when running make to make use of
    --# a bundled SHA1 routine optimized for PowerPC.
    ++#
     +# Due to the SHAttered (https://shattered.io) attack vector on SHA-1
     +# Git uses the sha1collisiondetection counter-cryptanalysis library
     +# for SHA-1 hashing.
     +#
     +# You're strongly advised not to override this for any usage of Git
     +# where you don't 100% trust the repository content.
    -+#
    -+# ==== 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.
      #
     -# Define DC_SHA1 to unconditionally enable the collision-detecting sha1
     -# algorithm. This is slower, but may detect attempted collision attacks.
     -# Takes priority over other *_SHA1 knobs.
    -+# ===== Options for the default sha1collisiondetection implementations =====
    ++# ==== Options common to all SHA-1 implementations ====
      #
     -# Define DC_SHA1_EXTERNAL in addition to DC_SHA1 if you want to build / link
    ++# 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.
    ++#
    ++# ===== Options for the default sha1collisiondetection implementations =====
    ++#
     +# 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
    @@ Makefile: include shared.mak
     -# 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 PPC_SHA1 environment variable when running make to make use of
    -+# a bundled SHA1 routine optimized for PowerPC.
    -+#
     +# Define APPLE_SHA1 to use Apple's CommonCrypto SHA-1 routines on
     +# Darwin/Mac OS X.
     +#
    @@ Makefile: ifeq ($(uname_S),Darwin)
     -		APPLE_COMMON_CRYPTO = YesPlease
      		COMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO
      	endif
    - 	NO_REGEX = YesPlease
    + 	PTHREAD_LIBS =
     @@ Makefile: ifdef NO_POSIX_GOODIES
      	BASIC_CFLAGS += -DNO_POSIX_GOODIES
      endif
    @@ Makefile: ifdef NO_POSIX_GOODIES
     -ifdef APPLE_COMMON_CRYPTO
     -	# Apple CommonCrypto requires chunking
     -	SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
    -+ifdef DC_SHA1
    -+$(error the DC_SHA1 flag is no longer used, and has become the default. Adjust your build scripts accordingly)
    - endif
    +-endif
     -
    + ifdef PPC_SHA1
    + $(error the PPC_SHA1 flag has been removed along with the PowerPC-specific SHA-1 implementation.)
    + endif
    + 
     -ifdef OPENSSL_SHA1
     -	EXTLIBS += $(LIB_4_CRYPTO)
     -	BASIC_CFLAGS += -DSHA1_OPENSSL
    @@ Makefile: ifdef NO_POSIX_GOODIES
     -	LIB_OBJS += block-sha1/sha1.o
     -	BASIC_CFLAGS += -DSHA1_BLK
     -else
    --ifdef PPC_SHA1
    --	LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
    --	BASIC_CFLAGS += -DSHA1_PPC
    --else
     -ifdef APPLE_COMMON_CRYPTO
     -	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
     -	BASIC_CFLAGS += -DSHA1_APPLE
     -else
     -	DC_SHA1 := YesPlease
     -	BASIC_CFLAGS += -DSHA1_DC
    ++ifdef DC_SHA1
    ++$(error the DC_SHA1 flag is no longer used, and has become the default. Adjust your build scripts accordingly)
    ++endif
     +ifndef NO_DC_SHA1
    -+	ifneq ($(OPENSSL_SHA1)$(BLK_SHA1)$(PPC_SHA1)$(APPLE_SHA1),)
    ++	ifneq ($(OPENSSL_SHA1)$(BLK_SHA1)$(APPLE_SHA1),)
     +$(error no other *_SHA1 option can be defined unless NO_DC_SHA1 is defined)
     +	endif
      	LIB_OBJS += sha1dc_git.o
    @@ Makefile: endif
     +	LIB_OBJS += block-sha1/sha1.o
     +	BASIC_CFLAGS += -DSHA1_BLK
     +else
    -+ifdef PPC_SHA1
    -+	LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
    -+	BASIC_CFLAGS += -DSHA1_PPC
    -+else
     +ifdef APPLE_SHA1
     +	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
     +	BASIC_CFLAGS += -DSHA1_APPLE
    @@ Makefile: endif
      endif
      endif
     @@ Makefile: GIT-BUILD-OPTIONS: FORCE
    - 	@echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' >>$@+
    + 	@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)))'\' >>$@+
    @@ Makefile: GIT-BUILD-OPTIONS: FORCE
      	@echo SANITIZE_ADDRESS=\''$(subst ','\'',$(subst ','\'',$(SANITIZE_ADDRESS)))'\' >>$@+
      	@echo X=\'$(X)\' >>$@+
     
    + ## ci/lib.sh ##
    +@@ ci/lib.sh: macos-latest)
    + esac
    + 
    + case "$jobname" in
    ++osx-clang)
    ++	MAKEFLAGS="$MAKEFLAGS NO_DC_SHA1=Yes APPLE_SHA1=Yes"
    ++	;;
    + linux32)
    + 	CC=gcc
    + 	;;
    +
      ## contrib/buildsystems/CMakeLists.txt ##
     @@ contrib/buildsystems/CMakeLists.txt: set(NO_PERL )
      set(NO_PTHREADS )
    @@ hash.h
      #include "git-compat-util.h"
      #include "repository.h"
      
    --#if defined(SHA1_PPC)
    +-#if defined(SHA1_APPLE)
     +#if !defined(NO_SHA1_DC)
     +#include "sha1dc_git.h"
    -+#elif defined(SHA1_PPC)
    - #include "ppc/sha1.h"
    - #elif defined(SHA1_APPLE)
    ++#elif defined(SHA1_APPLE)
      #include <CommonCrypto/CommonDigest.h>
      #elif defined(SHA1_OPENSSL)
      #include <openssl/sha.h>
    @@ hash.h
     +#error "need a SHA1_* implementation defined"
      #endif
      
    - #if defined(SHA256_GCRYPT)
    + #if defined(SHA256_NETTLE)
     
      ## t/t0013-sha1dc.sh ##
     @@ t/t0013-sha1dc.sh: TEST_PASSES_SANITIZE_LEAK=true
3:  1422c6fd497 ! 4:  f1fb9775b33 Makefile: rephrase the discussion of *_SHA1 knobs
    @@ Makefile: include shared.mak
     +# Define OPENSSL_SHA1 to link to the the SHA-1 routines from
     +# the OpenSSL library.
      #
    --# Define PPC_SHA1 environment variable when running make to make use of
    --# a bundled SHA1 routine optimized for PowerPC.
    -+# Define PPC_SHA1 to make use of optimized (in assembly)
    -+# PowerPC SHA-1 routines.
    - #
      # Define APPLE_SHA1 to use Apple's CommonCrypto SHA-1 routines on
      # Darwin/Mac OS X.
4:  ae464dbd228 < -:  ----------- Makefile + hash.h: remove PPC_SHA1 implementation
5:  730d08da6fe < -:  ----------- Makefile: use $(OBJECTS) instead of $(C_OBJ)
-- 
2.38.0.1093.gcd4a685f0b1


^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH v2 1/4] fsmonitor OSX: compile with DC_SHA1=YesPlease
  2022-10-19  1:03 ` [PATCH v2 0/4] " Ævar Arnfjörð Bjarmason
@ 2022-10-19  1:03   ` Ævar Arnfjörð Bjarmason
  2022-10-19  1:03   ` [PATCH v2 2/4] Makefile: create and use sections for "define" flag listing Ævar Arnfjörð Bjarmason
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-19  1:03 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

As we'll address in subsequent commits the "DC_SHA1=YesPlease" is not
on by default on OSX, instead we use Apple Common Crypto's SHA-1
implementation.

In 6beb2688d33 (fsmonitor: relocate socket file if .git directory is
remote, 2022-10-04) the build was broken with "DC_SHA1=YesPlease" (and
probably other non-"APPLE_COMMON_CRYPTO" SHA-1 backends).

So let's extract the fix for this from [1] to get the build working
again with "DC_SHA1=YesPlease". In addition to the fix in [1] we also
need to replace "SHA_DIGEST_LENGTH" with "GIT_MAX_RAWSZ".

1. https://lore.kernel.org/git/c085fc15b314abcb5e5ca6b4ee5ac54a28327cab.1665326258.git.gitgitgadget@gmail.com/

Signed-off-by: Eric DeCosta <edecosta@mathworks.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 compat/fsmonitor/fsm-ipc-darwin.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/compat/fsmonitor/fsm-ipc-darwin.c b/compat/fsmonitor/fsm-ipc-darwin.c
index ce843d63348..d67b0ee50d3 100644
--- a/compat/fsmonitor/fsm-ipc-darwin.c
+++ b/compat/fsmonitor/fsm-ipc-darwin.c
@@ -10,10 +10,10 @@ static GIT_PATH_FUNC(fsmonitor_ipc__get_default_path, "fsmonitor--daemon.ipc")
 const char *fsmonitor_ipc__get_path(struct repository *r)
 {
 	static const char *ipc_path = NULL;
-	SHA_CTX sha1ctx;
+	git_SHA_CTX sha1ctx;
 	char *sock_dir = NULL;
 	struct strbuf ipc_file = STRBUF_INIT;
-	unsigned char hash[SHA_DIGEST_LENGTH];
+	unsigned char hash[GIT_MAX_RAWSZ];
 
 	if (!r)
 		BUG("No repository passed into fsmonitor_ipc__get_path");
@@ -28,9 +28,9 @@ const char *fsmonitor_ipc__get_path(struct repository *r)
 		return ipc_path;
 	}
 
-	SHA1_Init(&sha1ctx);
-	SHA1_Update(&sha1ctx, r->worktree, strlen(r->worktree));
-	SHA1_Final(hash, &sha1ctx);
+	git_SHA1_Init(&sha1ctx);
+	git_SHA1_Update(&sha1ctx, r->worktree, strlen(r->worktree));
+	git_SHA1_Final(hash, &sha1ctx);
 
 	repo_config_get_string(r, "fsmonitor.socketdir", &sock_dir);
 
-- 
2.38.0.1093.gcd4a685f0b1


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v2 2/4] Makefile: create and use sections for "define" flag listing
  2022-10-19  1:03 ` [PATCH v2 0/4] " Ævar Arnfjörð Bjarmason
  2022-10-19  1:03   ` [PATCH v2 1/4] fsmonitor OSX: compile with DC_SHA1=YesPlease Ævar Arnfjörð Bjarmason
@ 2022-10-19  1:03   ` Ævar Arnfjörð Bjarmason
  2022-10-19  1:03   ` [PATCH v2 3/4] Makefile: really use and document sha1collisiondetection by default Ævar Arnfjörð Bjarmason
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-19  1:03 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 210 +++++++++++++++++++++++++++++++------------------------
 1 file changed, 117 insertions(+), 93 deletions(-)

diff --git a/Makefile b/Makefile
index d93ad956e58..18ad487274e 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,39 +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 unconditionally 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.
-# 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).
@@ -490,6 +409,111 @@ include shared.mak
 # to the "<name>" of the corresponding `compat/fsmonitor/fsm-settings-<name>.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 ==
+#
+# 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
+# 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
+# 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.
+#
+# == 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
-- 
2.38.0.1093.gcd4a685f0b1


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v2 3/4] Makefile: really use and document sha1collisiondetection by default
  2022-10-19  1:03 ` [PATCH v2 0/4] " Ævar Arnfjörð Bjarmason
  2022-10-19  1:03   ` [PATCH v2 1/4] fsmonitor OSX: compile with DC_SHA1=YesPlease Ævar Arnfjörð Bjarmason
  2022-10-19  1:03   ` [PATCH v2 2/4] Makefile: create and use sections for "define" flag listing Ævar Arnfjörð Bjarmason
@ 2022-10-19  1:03   ` Ævar Arnfjörð Bjarmason
  2022-10-19  2:59     ` Eric Sunshine
  2022-10-19  1:03   ` [PATCH v2 4/4] Makefile: rephrase the discussion of *_SHA1 knobs Ævar Arnfjörð Bjarmason
  2022-10-20 22:43   ` [PATCH v3 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
  4 siblings, 1 reply; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-19  1:03 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

When the sha1collisiondetection library was added and made the default
in [1] we never updated the documentation added in [2] early in that
series once the default was flipped to DC_SHA1 in [3]. Furthermore the
INSTALL file has been claiming that we use OpenSSL by default since
[4], and hadn't been updated for the sha1collisiondetection switch.

The interaction between NO_APPLE_COMMON_CRYPTO and DC_SHA1 seems to
have been missed in [3], so ever since DC_SHA1 was made the default
we've still used Apple's CommonCrypto instead of
sha1collisiondetection on Darwin and Mac OS X.

Instead off all of this we now:

* Don't have a DC_SHA1 know anymore (using it is an error), you need
  to set NO_DC_SHA1 instead to use any optional *_SHA1 implementation.
* Re-arranged the algorithm inclusion in hash.h to correspond to
  NO_DC_SHA1, and "#error" if we have no defined SHA_*, rather than
  silently picking block-sha1/sha1.h as a fallback.
* Have an INSTALL that reflects reality. We were still claiming to use
  OpenSSL's SHA-1 hashing by default.
* Have Darwin and Mac OS X use sha1collisiondetection, like everywhere
  else. There is still a NO_APPLE_COMMON_CRYPTO knob, but it's used for
  things unrelated to SHA-1 (see [6]).
* Have a rewritten discussion of SHA-1 and SHA-256 in the Makefile
  which covers all of this.

Let's also change the CI for "osx-clang" to test with the new
APPLE_SHA1 knob ("osx-gcc" uses the new sha1collisiondetection
default).

In practice this will spot issues like the one noted in [7], as
testing with just two backends should be enough to spot unportable
code. Ideally we'd have other CI jobs to test the various SHA-1
combinations, but for now we have better CI coverage than before.

1. 48b3693d3ce (Merge branch 'jk/sha1dc', 2017-03-24)
2. 8325e43b82d (Makefile: add DC_SHA1 knob, 2017-03-16)
3. e6b07da2780 (Makefile: make DC_SHA1 the default, 2017-03-17)
4. 5beb577db8c (INSTALL: Describe dependency knobs from Makefile,
   2009-09-10)
5. 4dcd7732db0 (Makefile: add support for Apple CommonCrypto facility,
   2013-05-19)
6. 3ef2bcad02e (imap-send: use Apple's Security framework for base64
   encoding, 2013-07-29)
7. https://lore.kernel.org/git/kl6l7d0yyu6r.fsf@chooglen-macbookpro.roam.corp.google.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 INSTALL                             | 10 ++-
 Makefile                            | 98 ++++++++++++++++++++---------
 ci/lib.sh                           |  3 +
 contrib/buildsystems/CMakeLists.txt |  3 +-
 hash.h                              | 10 +--
 t/t0013-sha1dc.sh                   |  4 +-
 6 files changed, 86 insertions(+), 42 deletions(-)

diff --git a/INSTALL b/INSTALL
index 89b15d71df5..065ed81bd54 100644
--- a/INSTALL
+++ b/INSTALL
@@ -133,9 +133,13 @@ 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.
+	- Git uses an altered version of SHA-1 by default which
+	  detects the SHAttered attack via the sha1collisiondetection
+	  counter-cryptanalysis library. For SHA-256 we'll select a
+	  working implementation (and ship a fallback
+	  implementation). See the "SHA-1 and SHA-256 defines" section
+	  in the Makefile for details. You should not need to tweak
+	  those settings.
 
 	- "libcurl" library is used for fetching and pushing
 	  repositories over http:// or https://, as well as by
diff --git a/Makefile b/Makefile
index 18ad487274e..7a7411df8c3 100644
--- a/Makefile
+++ b/Makefile
@@ -479,30 +479,62 @@ include shared.mak
 #
 # == SHA-1 and SHA-256 defines ==
 #
-# Define BLK_SHA1 environment variable to make use of the bundled
-# optimized C SHA1 routine.
+# === SHA-1 backend ===
+#
+# Due to the SHAttered (https://shattered.io) attack vector on SHA-1
+# Git uses the sha1collisiondetection counter-cryptanalysis library
+# for SHA-1 hashing.
+#
+# You're strongly advised not to override this for any usage of Git
+# where you don't 100% trust the repository content.
 #
-# Define DC_SHA1 to unconditionally enable the collision-detecting sha1
-# algorithm. This is slower, but may detect attempted collision attacks.
-# Takes priority over other *_SHA1 knobs.
+# ==== Options common to all SHA-1 implementations ====
 #
-# Define DC_SHA1_EXTERNAL in addition to DC_SHA1 if you want to build / link
+# 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.
+#
+# ===== Options for the default sha1collisiondetection implementations =====
+#
+# 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
 # submodule.
 #
+# ==== Alternate implementations ====
+#
+# Git still ships with alternate SHA-1 implementations. These are
+# faster than the default, which is useful when hashing speed
+# is imperative, consider using them if you're confident that you
+# won't need to worry about SHA-1 collision attacks.
+#
+# To use them you must define NO_DC_SHA1 and one of the *_SHA1
+# variables below:
+#
+# 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.
 #
-# 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 APPLE_SHA1 to use Apple's CommonCrypto SHA-1 routines on
+# Darwin/Mac OS X.
+#
+# The APPLE_SHA1 option is unrelated to the NO_APPLE_COMMON_CRYPTO
+# flag, which determines if Apple's crypto libraries are used for
+# things that aren't SHA-1.
+#
+# === SHA-256 backend ===
+#
+# Unlike SHA-1 the SHA-256 algorithm does not suffer from any known
+# vulnerabilities, so any implementation will do. BLK_SHA256 is
+# currently the default implementation (but that may change).
 #
 # Define BLK_SHA256 to use the built-in SHA-256 routines.
 #
@@ -1464,7 +1496,6 @@ ifeq ($(uname_S),Darwin)
 	endif
 	ifndef NO_APPLE_COMMON_CRYPTO
 		NO_OPENSSL = YesPlease
-		APPLE_COMMON_CRYPTO = YesPlease
 		COMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO
 	endif
 	PTHREAD_LIBS =
@@ -1825,30 +1856,19 @@ ifdef NO_POSIX_GOODIES
 	BASIC_CFLAGS += -DNO_POSIX_GOODIES
 endif
 
-ifdef APPLE_COMMON_CRYPTO
-	# Apple CommonCrypto requires chunking
-	SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
-endif
-
 ifdef PPC_SHA1
 $(error the PPC_SHA1 flag has been removed along with the PowerPC-specific SHA-1 implementation.)
 endif
 
-ifdef OPENSSL_SHA1
-	EXTLIBS += $(LIB_4_CRYPTO)
-	BASIC_CFLAGS += -DSHA1_OPENSSL
-else
-ifdef BLK_SHA1
-	LIB_OBJS += block-sha1/sha1.o
-	BASIC_CFLAGS += -DSHA1_BLK
-else
-ifdef APPLE_COMMON_CRYPTO
-	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
-	BASIC_CFLAGS += -DSHA1_APPLE
-else
-	DC_SHA1 := YesPlease
-	BASIC_CFLAGS += -DSHA1_DC
+ifdef DC_SHA1
+$(error the DC_SHA1 flag is no longer used, and has become the default. Adjust your build scripts accordingly)
+endif
+ifndef NO_DC_SHA1
+	ifneq ($(OPENSSL_SHA1)$(BLK_SHA1)$(APPLE_SHA1),)
+$(error no other *_SHA1 option can be defined unless NO_DC_SHA1 is defined)
+	endif
 	LIB_OBJS += sha1dc_git.o
+
 ifdef DC_SHA1_EXTERNAL
 	ifdef DC_SHA1_SUBMODULE
 		ifneq ($(DC_SHA1_SUBMODULE),auto)
@@ -1872,6 +1892,22 @@ endif
 		-DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" \
 		-DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\""
 endif
+else # !NO_DC_SHA1
+BASIC_CFLAGS += -DNO_SHA1_DC
+ifdef OPENSSL_SHA1
+	EXTLIBS += $(LIB_4_CRYPTO)
+	BASIC_CFLAGS += -DSHA1_OPENSSL
+else
+ifdef BLK_SHA1
+	LIB_OBJS += block-sha1/sha1.o
+	BASIC_CFLAGS += -DSHA1_BLK
+else
+ifdef APPLE_SHA1
+	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
+	BASIC_CFLAGS += -DSHA1_APPLE
+else
+$(error when defining NO_DC_SHA1 another valid *_SHA1 variable must be defined!)
+endif
 endif
 endif
 endif
@@ -3009,7 +3045,7 @@ 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 NO_DC_SHA1=\''$(subst ','\'',$(subst ','\'',$(NO_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 1b0cc2b57db..320f992680a 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -264,6 +264,9 @@ macos-latest)
 esac
 
 case "$jobname" in
+osx-clang)
+	MAKEFLAGS="$MAKEFLAGS NO_DC_SHA1=Yes APPLE_SHA1=Yes"
+	;;
 linux32)
 	CC=gcc
 	;;
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 787738e6fa3..14ac3d49849 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,7 @@ 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 "NO_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/hash.h b/hash.h
index 36b64165fc9..a7337779949 100644
--- a/hash.h
+++ b/hash.h
@@ -4,14 +4,16 @@
 #include "git-compat-util.h"
 #include "repository.h"
 
-#if defined(SHA1_APPLE)
+#if !defined(NO_SHA1_DC)
+#include "sha1dc_git.h"
+#elif defined(SHA1_APPLE)
 #include <CommonCrypto/CommonDigest.h>
 #elif defined(SHA1_OPENSSL)
 #include <openssl/sha.h>
-#elif defined(SHA1_DC)
-#include "sha1dc_git.h"
-#else /* SHA1_BLK */
+#elif defined(SHA1_BLK)
 #include "block-sha1/sha1.h"
+#else
+#error "need a SHA1_* implementation defined"
 #endif
 
 #if defined(SHA256_NETTLE)
diff --git a/t/t0013-sha1dc.sh b/t/t0013-sha1dc.sh
index 9ad76080aa4..539270a2665 100755
--- a/t/t0013-sha1dc.sh
+++ b/t/t0013-sha1dc.sh
@@ -6,9 +6,9 @@ TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 TEST_DATA="$TEST_DIRECTORY/t0013"
 
-if test -z "$DC_SHA1"
+if test -n "$NO_DC_SHA1"
 then
-	skip_all='skipping sha1 collision tests, DC_SHA1 not set'
+	skip_all='skipping sha1 collision tests, NO_DC_SHA1 set'
 	test_done
 fi
 
-- 
2.38.0.1093.gcd4a685f0b1


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v2 4/4] Makefile: rephrase the discussion of *_SHA1 knobs
  2022-10-19  1:03 ` [PATCH v2 0/4] " Ævar Arnfjörð Bjarmason
                     ` (2 preceding siblings ...)
  2022-10-19  1:03   ` [PATCH v2 3/4] Makefile: really use and document sha1collisiondetection by default Ævar Arnfjörð Bjarmason
@ 2022-10-19  1:03   ` Ævar Arnfjörð Bjarmason
  2022-10-20 22:43   ` [PATCH v3 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
  4 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-19  1:03 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 7a7411df8c3..16367c72ba8 100644
--- a/Makefile
+++ b/Makefile
@@ -517,11 +517,11 @@ include shared.mak
 # To use them you must define NO_DC_SHA1 and one of the *_SHA1
 # variables below:
 #
-# 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 the SHA-1 routines from
+# the OpenSSL library.
 #
 # Define APPLE_SHA1 to use Apple's CommonCrypto SHA-1 routines on
 # Darwin/Mac OS X.
-- 
2.38.0.1093.gcd4a685f0b1


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* Re: [PATCH v2 3/4] Makefile: really use and document sha1collisiondetection by default
  2022-10-19  1:03   ` [PATCH v2 3/4] Makefile: really use and document sha1collisiondetection by default Ævar Arnfjörð Bjarmason
@ 2022-10-19  2:59     ` Eric Sunshine
  2022-10-19 16:28       ` Junio C Hamano
  0 siblings, 1 reply; 55+ messages in thread
From: Eric Sunshine @ 2022-10-19  2:59 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Glen Choo, Eric DeCosta

On Tue, Oct 18, 2022 at 9:03 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> When the sha1collisiondetection library was added and made the default
> in [1] we never updated the documentation added in [2] early in that
> series once the default was flipped to DC_SHA1 in [3]. Furthermore the
> INSTALL file has been claiming that we use OpenSSL by default since
> [4], and hadn't been updated for the sha1collisiondetection switch.
>
> The interaction between NO_APPLE_COMMON_CRYPTO and DC_SHA1 seems to
> have been missed in [3], so ever since DC_SHA1 was made the default
> we've still used Apple's CommonCrypto instead of
> sha1collisiondetection on Darwin and Mac OS X.
>
> Instead off all of this we now:
>
> * Don't have a DC_SHA1 know anymore (using it is an error), you need

s/know/knob/

>   to set NO_DC_SHA1 instead to use any optional *_SHA1 implementation.
> * Re-arranged the algorithm inclusion in hash.h to correspond to
>   NO_DC_SHA1, and "#error" if we have no defined SHA_*, rather than
>   silently picking block-sha1/sha1.h as a fallback.
> * Have an INSTALL that reflects reality. We were still claiming to use
>   OpenSSL's SHA-1 hashing by default.
> * Have Darwin and Mac OS X use sha1collisiondetection, like everywhere
>   else. There is still a NO_APPLE_COMMON_CRYPTO knob, but it's used for
>   things unrelated to SHA-1 (see [6]).
> * Have a rewritten discussion of SHA-1 and SHA-256 in the Makefile
>   which covers all of this.
>
> Let's also change the CI for "osx-clang" to test with the new
> APPLE_SHA1 knob ("osx-gcc" uses the new sha1collisiondetection
> default).
>
> In practice this will spot issues like the one noted in [7], as
> testing with just two backends should be enough to spot unportable
> code. Ideally we'd have other CI jobs to test the various SHA-1
> combinations, but for now we have better CI coverage than before.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> diff --git a/Makefile b/Makefile
> +ifdef DC_SHA1
> +$(error the DC_SHA1 flag is no longer used, and has become the default. Adjust your build scripts accordingly)
> +endif

bikeshedding: Do we really need to penalize (abuse) people merely for
asking us to do what we're already doing anyhow?

Or do you have some future plan which wasn't stated in the commit log
but which explains this sort of hash stance?

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH v2 3/4] Makefile: really use and document sha1collisiondetection by default
  2022-10-19  2:59     ` Eric Sunshine
@ 2022-10-19 16:28       ` Junio C Hamano
  2022-10-19 18:54         ` Ævar Arnfjörð Bjarmason
  2022-10-20 21:15         ` brian m. carlson
  0 siblings, 2 replies; 55+ messages in thread
From: Junio C Hamano @ 2022-10-19 16:28 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: Ævar Arnfjörð Bjarmason, git, Mike Hommey,
	brian m . carlson, Carlo Marcelo Arenas Belón, Glen Choo,
	Eric DeCosta

Eric Sunshine <sunshine@sunshineco.com> writes:

>> diff --git a/Makefile b/Makefile
>> +ifdef DC_SHA1
>> +$(error the DC_SHA1 flag is no longer used, and has become the default. Adjust your build scripts accordingly)
>> +endif
>
> bikeshedding: Do we really need to penalize (abuse) people merely for
> asking us to do what we're already doing anyhow?

A valid question.

I can understand and very much appreciate [1/4] as a very focused
fix to the problem.  Very small part of this step, namely, make the
DC_SHA1 the default everywhere, is also very much welcome.

Everything else I see in these patches are extra "while we are at
it" that should not exist.  These "while at it" changes tend to
somehow implement more subjective choices that will cause more
discussion and take more review resources.  Not all "white at it"
may be more subjective, but at least in this series, they appear
to be.

They distract us from the core changes and slows us down.  It is OK
to do them as totally unrelated clean-up changes long after the dust
settles, but not entangled with the other more important changes
like these patches.

Thanks.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH v2 3/4] Makefile: really use and document sha1collisiondetection by default
  2022-10-19 16:28       ` Junio C Hamano
@ 2022-10-19 18:54         ` Ævar Arnfjörð Bjarmason
  2022-10-19 19:43           ` Junio C Hamano
  2022-10-19 22:15           ` Junio C Hamano
  2022-10-20 21:15         ` brian m. carlson
  1 sibling, 2 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-19 18:54 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Eric Sunshine, git, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Glen Choo, Eric DeCosta


On Wed, Oct 19 2022, Junio C Hamano wrote:

> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>>> diff --git a/Makefile b/Makefile
>>> +ifdef DC_SHA1
>>> +$(error the DC_SHA1 flag is no longer used, and has become the default. Adjust your build scripts accordingly)
>>> +endif
>>
>> bikeshedding: Do we really need to penalize (abuse) people merely for
>> asking us to do what we're already doing anyhow?
>
> A valid question.
>
> I can understand and very much appreciate [1/4] as a very focused
> fix to the problem.  Very small part of this step, namely, make the
> DC_SHA1 the default everywhere, is also very much welcome.
>
> Everything else I see in these patches are extra "while we are at
> it" that should not exist.  These "while at it" changes tend to
> somehow implement more subjective choices that will cause more
> discussion and take more review resources.  Not all "white at it"
> may be more subjective, but at least in this series, they appear
> to be.
>
> They distract us from the core changes and slows us down.  It is OK
> to do them as totally unrelated clean-up changes long after the dust
> settles, but not entangled with the other more important changes
> like these patches.

There's things I can eject from this series, but I don't really find it
to be "while at it" changes, I suspect what you're thinking of is
one/some of:

 - Re-arranging the Makefile commentary into sections: I did that
   because now the structure is very much one-paragraph-per-flag. 

   So before 2/4 there's no good place to put that "Alternate
   implementations" in a way that clearly refers to the surrounding
   SHA{1,256} discussion. But yeah, I could try harder to keep that diff
   size down, or just not update the docs.

 - We're still claiming that we use OpenSSL by default, so I fixed the
   docs in general (not just the Makefile). Would you like this to be
   just a "we forgot OSX?" series?

 - Ditto asking to provide NO_DC_SHA1=Y now in addition to
   e.g. BLK_SHA1=Y if you *really* want to use that
   non-collision-detecting SHA-1 implementation.

   E.g. BLK_SHA1=Y was added in 2009. It's a small one-time bother to
   add NO_DC_SHA1=Y to your scripts if you *really* mean "I want the
   less safe SHA-1".

   But I think it's more likely that someone running into that error
   will be happy that the default has changed in a strong way.

   We've been *strongly* recommending sha1collisiondetection for a while
   now, but the structure of the build system has been the exact
   opposite, if you asked for anything else we'd avoid using it, and
   only default to it if nothing else was picked.

 - In particular the current flag on OSX is "APPLE_COMMON_CRYPTO", which
   is now split up into that & "APPLE_SHA1".

   Maybe changing our default flags is sufficient, and we don't need to
   worry about 3rd party build scripts having the previous "Yeah, I have
   that OS library" effectively meaning "...and use it for everything
   possible, including SHA-1".

Or maybe you're OK with topic(s) at large, i.e. "switch the default,
update the docs, make sure noone's left behind", but would just like it
done in more incremental steps?

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH v2 3/4] Makefile: really use and document sha1collisiondetection by default
  2022-10-19 18:54         ` Ævar Arnfjörð Bjarmason
@ 2022-10-19 19:43           ` Junio C Hamano
  2022-10-19 22:15           ` Junio C Hamano
  1 sibling, 0 replies; 55+ messages in thread
From: Junio C Hamano @ 2022-10-19 19:43 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Eric Sunshine, git, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Glen Choo, Eric DeCosta

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>> They distract us from the core changes and slows us down.  It is OK
>> to do them as totally unrelated clean-up changes long after the dust
>> settles, but not entangled with the other more important changes
>> like these patches.
>
> There's things I can eject from this series, but I don't really find it
> to be "while at it" changes, I suspect what you're thinking of is
> one/some of:
> ...
> Or maybe you're OK with topic(s) at large, i.e. "switch the default,
> update the docs, make sure noone's left behind", but would just like it
> done in more incremental steps?

I said something on this in a separate thread. Let me look it up.

 * Updating the build procedure so that sha1dc is used by default
   everywhere is a good idea, but that is less urgent and should be
   done separately, preferrably long after the dust settles from the
   above.


^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH v2 3/4] Makefile: really use and document sha1collisiondetection by default
  2022-10-19 18:54         ` Ævar Arnfjörð Bjarmason
  2022-10-19 19:43           ` Junio C Hamano
@ 2022-10-19 22:15           ` Junio C Hamano
  2022-10-19 22:27             ` Junio C Hamano
  1 sibling, 1 reply; 55+ messages in thread
From: Junio C Hamano @ 2022-10-19 22:15 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Eric Sunshine, git, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Glen Choo, Eric DeCosta

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> There's things I can eject from this series, but I don't really find it
> to be "while at it" changes, I suspect what you're thinking of is
> one/some of:
>
>  - Re-arranging the Makefile commentary into sections: I did that
>    because now the structure is very much one-paragraph-per-flag. 

And it does not have anything to do with "why doesn't macOS use
sha1dc as the default like everybody else?" fix.

>  - We're still claiming that we use OpenSSL by default, so I fixed the
>    docs in general (not just the Makefile). Would you like this to be
>    just a "we forgot OSX?" series?

A focused change s/We use OpenSSL as default/We use sha1dc as default/ 
goes well with "why doesn't macOS use sha1dc as the default?" fix.

>  - Ditto asking to provide NO_DC_SHA1=Y now in addition to

Those who explicitly opt into DC_SHA1 do not need any new $(error).
Those who have explicitly chosen something else should not be forced
to say NO_DC_SHA1.

Let's make it a focused change that does one thing very well without
wasting reviewer's time on discussion on non-essential things.

I do not mean by "non-essential" that reorganizing Makefile comments
to make it easier to find and write relevant documents, or making it
harder to turn conflicting knobs.  They may by themselves be
worthwhile thing to do.  But in the context of "why isn't sha1dc the
default everywhere", they are non-essential distractions.

"why isn't sha1dc the default on macOS?" is also non-essential
distraction in the context of fixing "oops, we no longer build on
macOS unless Apple Crypto is used".

I've queued [1/4] with an intention to fast-track it down to
'master' (after cooking it in 'next' for a few days).

Thanks.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH v2 3/4] Makefile: really use and document sha1collisiondetection by default
  2022-10-19 22:15           ` Junio C Hamano
@ 2022-10-19 22:27             ` Junio C Hamano
  0 siblings, 0 replies; 55+ messages in thread
From: Junio C Hamano @ 2022-10-19 22:27 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Eric Sunshine, git, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Glen Choo, Eric DeCosta

Junio C Hamano <gitster@pobox.com> writes:

> Let's make it a focused change that does one thing very well without
> wasting reviewer's time on discussion on non-essential things.
>
> I do not mean by "non-essential" that reorganizing Makefile comments
> to make it easier to find and write relevant documents, or making it
> harder to turn conflicting knobs.  They may by themselves be

Sorry, I forgot to finish the sentence.  I do not mean these other
changes I called "non-essential" are useless.  They are not
essential in the context of making sha1dc the default everywhere.

Thanks.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH v2 3/4] Makefile: really use and document sha1collisiondetection by default
  2022-10-19 16:28       ` Junio C Hamano
  2022-10-19 18:54         ` Ævar Arnfjörð Bjarmason
@ 2022-10-20 21:15         ` brian m. carlson
  1 sibling, 0 replies; 55+ messages in thread
From: brian m. carlson @ 2022-10-20 21:15 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Eric Sunshine, Ævar Arnfjörð Bjarmason, git,
	Mike Hommey, Carlo Marcelo Arenas Belón, Glen Choo,
	Eric DeCosta

[-- Attachment #1: Type: text/plain, Size: 1576 bytes --]

On 2022-10-19 at 16:28:55, Junio C Hamano wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
> >> diff --git a/Makefile b/Makefile
> >> +ifdef DC_SHA1
> >> +$(error the DC_SHA1 flag is no longer used, and has become the default. Adjust your build scripts accordingly)
> >> +endif
> >
> > bikeshedding: Do we really need to penalize (abuse) people merely for
> > asking us to do what we're already doing anyhow?
> 
> A valid question.
> 
> I can understand and very much appreciate [1/4] as a very focused
> fix to the problem.  Very small part of this step, namely, make the
> DC_SHA1 the default everywhere, is also very much welcome.
> 
> Everything else I see in these patches are extra "while we are at
> it" that should not exist.  These "while at it" changes tend to
> somehow implement more subjective choices that will cause more
> discussion and take more review resources.  Not all "white at it"
> may be more subjective, but at least in this series, they appear
> to be.

I will definitely say that we should be using SHA-1-DC by default
everywhere and I'm fine with that change.  I don't think we should fail
if someone sets the former knob, since I don't recall us doing that
elsewhere.  It also seems bad to bother folks if we're already doing
what they asked for (and what we want them to do anyway).

I don't have a strong attachment to the remainder of the series and
agree that maybe a different series would be a better location for those
patches.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH v3 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug
  2022-10-19  1:03 ` [PATCH v2 0/4] " Ævar Arnfjörð Bjarmason
                     ` (3 preceding siblings ...)
  2022-10-19  1:03   ` [PATCH v2 4/4] Makefile: rephrase the discussion of *_SHA1 knobs Ævar Arnfjörð Bjarmason
@ 2022-10-20 22:43   ` Ævar Arnfjörð Bjarmason
  2022-10-20 22:43     ` [PATCH v3 1/9] Makefile: always (re)set DC_SHA1 on fallback Ævar Arnfjörð Bjarmason
                       ` (9 more replies)
  4 siblings, 10 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-20 22:43 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

This v3 is a rewrite of the larger-in-scope v2[2] to only fix the
current Makefile & INSTALL documentation do to do with our SHA-1 and
SHA-256 knobs.

The only behavior change here is in 1/8, where I fix what's obviously
a bug in the current behavior (so that we don't need to document that
edge case). I'll submit a patch to change the behavior on OSX to use
sha1collisiondetection by default on top of this series.

1. https://lore.kernel.org/git/cover-v2-0.4-00000000000-20221019T010222Z-avarab@gmail.com/

Ævar Arnfjörð Bjarmason (9):
  Makefile: always (re)set DC_SHA1 on fallback
  INSTALL: remove discussion of SHA-1 backends
  Makefile: correct DC_SHA1 documentation
  Makefile: create and use sections for "define" flag listing
  Makefile: rephrase the discussion of *_SHA1 knobs
  Makefile: document default SHA-256 backend
  Makefile: document SHA-1 and SHA-256 default and selection order
  Makefile: document default SHA-1 backend on OSX
  Makefile: discuss SHAttered in *_SHA{1,256} discussion

 INSTALL  |   4 -
 Makefile | 260 +++++++++++++++++++++++++++++++++++--------------------
 2 files changed, 166 insertions(+), 98 deletions(-)

Range-diff against v2:
 1:  392fabdb456 <  -:  ----------- fsmonitor OSX: compile with DC_SHA1=YesPlease
 -:  ----------- >  1:  ef3c5be11e0 Makefile: always (re)set DC_SHA1 on fallback
 -:  ----------- >  2:  017a0a9791c INSTALL: remove discussion of SHA-1 backends
 -:  ----------- >  3:  62dd2d5708d Makefile: correct DC_SHA1 documentation
 2:  7ae22276aa7 !  4:  933bef576b3 Makefile: create and use sections for "define" flag listing
    @@ Makefile: 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.
    @@ Makefile: include shared.mak
     +#
     +# == SHA-1 and SHA-256 defines ==
     +#
    -+# Define BLK_SHA1 environment variable to make use of the bundled
    -+# optimized C SHA1 routine.
    ++# === 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.
    ++#
    ++# ==== Options for the sha1collisiondetection implementation ====
     +#
    -+# 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.
    @@ Makefile: include shared.mak
     +# by the git project to migrate to using sha1collisiondetection as a
     +# submodule.
     +#
    ++# ==== Other SHA-1 implementations ====
    ++#
    ++# 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.
     +#
    -+# 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-256 backend ===
     +#
     +# Define BLK_SHA256 to use the built-in SHA-256 routines.
     +#
 3:  78ef8636c57 <  -:  ----------- Makefile: really use and document sha1collisiondetection by default
 4:  f1fb9775b33 <  -:  ----------- Makefile: rephrase the discussion of *_SHA1 knobs
 -:  ----------- >  5:  5b18198c477 Makefile: rephrase the discussion of *_SHA1 knobs
 -:  ----------- >  6:  73685592aba Makefile: document default SHA-256 backend
 -:  ----------- >  7:  05edcfb9cd9 Makefile: document SHA-1 and SHA-256 default and selection order
 -:  ----------- >  8:  859e69fbe9f Makefile: document default SHA-1 backend on OSX
 -:  ----------- >  9:  c1f27255d3e Makefile: discuss SHAttered in *_SHA{1,256} discussion
-- 
2.38.0.1178.g509f5fa8ce0


^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH v3 1/9] Makefile: always (re)set DC_SHA1 on fallback
  2022-10-20 22:43   ` [PATCH v3 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
@ 2022-10-20 22:43     ` Ævar Arnfjörð Bjarmason
  2022-10-20 22:43     ` [PATCH v3 2/9] INSTALL: remove discussion of SHA-1 backends Ævar Arnfjörð Bjarmason
                       ` (8 subsequent siblings)
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-20 22:43 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index d93ad956e58..5440089db3d 100644
--- a/Makefile
+++ b/Makefile
@@ -1822,7 +1822,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
-- 
2.38.0.1178.g509f5fa8ce0


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v3 2/9] INSTALL: remove discussion of SHA-1 backends
  2022-10-20 22:43   ` [PATCH v3 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
  2022-10-20 22:43     ` [PATCH v3 1/9] Makefile: always (re)set DC_SHA1 on fallback Ævar Arnfjörð Bjarmason
@ 2022-10-20 22:43     ` Ævar Arnfjörð Bjarmason
  2022-10-20 22:43     ` [PATCH v3 3/9] Makefile: correct DC_SHA1 documentation Ævar Arnfjörð Bjarmason
                       ` (7 subsequent siblings)
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-20 22:43 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 INSTALL | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/INSTALL b/INSTALL
index 89b15d71df5..33447883974 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
-- 
2.38.0.1178.g509f5fa8ce0


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v3 3/9] Makefile: correct DC_SHA1 documentation
  2022-10-20 22:43   ` [PATCH v3 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
  2022-10-20 22:43     ` [PATCH v3 1/9] Makefile: always (re)set DC_SHA1 on fallback Ævar Arnfjörð Bjarmason
  2022-10-20 22:43     ` [PATCH v3 2/9] INSTALL: remove discussion of SHA-1 backends Ævar Arnfjörð Bjarmason
@ 2022-10-20 22:43     ` Ævar Arnfjörð Bjarmason
  2022-10-20 22:43     ` [PATCH v3 4/9] Makefile: create and use sections for "define" flag listing Ævar Arnfjörð Bjarmason
                       ` (6 subsequent siblings)
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-20 22:43 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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.

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 <avarab@gmail.com>
---
 Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 5440089db3d..e55dc870c5d 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.
-- 
2.38.0.1178.g509f5fa8ce0


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v3 4/9] Makefile: create and use sections for "define" flag listing
  2022-10-20 22:43   ` [PATCH v3 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
                       ` (2 preceding siblings ...)
  2022-10-20 22:43     ` [PATCH v3 3/9] Makefile: correct DC_SHA1 documentation Ævar Arnfjörð Bjarmason
@ 2022-10-20 22:43     ` Ævar Arnfjörð Bjarmason
  2022-10-20 22:43     ` [PATCH v3 5/9] Makefile: rephrase the discussion of *_SHA1 knobs Ævar Arnfjörð Bjarmason
                       ` (5 subsequent siblings)
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-20 22:43 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 218 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 126 insertions(+), 92 deletions(-)

diff --git a/Makefile b/Makefile
index e55dc870c5d..b3a717792cf 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,120 @@ include shared.mak
 # to the "<name>" of the corresponding `compat/fsmonitor/fsm-settings-<name>.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.
+#
+# ==== Options for the sha1collisiondetection implementation ====
+#
+# 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.
+#
+# ==== Other SHA-1 implementations ====
+#
+# 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.
+#
+# === SHA-256 backend ===
+#
+# 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
-- 
2.38.0.1178.g509f5fa8ce0


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v3 5/9] Makefile: rephrase the discussion of *_SHA1 knobs
  2022-10-20 22:43   ` [PATCH v3 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
                       ` (3 preceding siblings ...)
  2022-10-20 22:43     ` [PATCH v3 4/9] Makefile: create and use sections for "define" flag listing Ævar Arnfjörð Bjarmason
@ 2022-10-20 22:43     ` Ævar Arnfjörð Bjarmason
  2022-10-20 22:43     ` [PATCH v3 6/9] Makefile: document default SHA-256 backend Ævar Arnfjörð Bjarmason
                       ` (4 subsequent siblings)
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-20 22:43 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index b3a717792cf..61358f16acc 100644
--- a/Makefile
+++ b/Makefile
@@ -492,12 +492,12 @@ include shared.mak
 # 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
+# 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
@@ -505,11 +505,11 @@ include shared.mak
 #
 # ==== Other SHA-1 implementations ====
 #
-# 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 the SHA-1 routines from
+# the OpenSSL library.
 #
 # === SHA-256 backend ===
 #
-- 
2.38.0.1178.g509f5fa8ce0


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v3 6/9] Makefile: document default SHA-256 backend
  2022-10-20 22:43   ` [PATCH v3 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
                       ` (4 preceding siblings ...)
  2022-10-20 22:43     ` [PATCH v3 5/9] Makefile: rephrase the discussion of *_SHA1 knobs Ævar Arnfjörð Bjarmason
@ 2022-10-20 22:43     ` Ævar Arnfjörð Bjarmason
  2022-10-20 22:43     ` [PATCH v3 7/9] Makefile: document SHA-1 and SHA-256 default and selection order Ævar Arnfjörð Bjarmason
                       ` (3 subsequent siblings)
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-20 22:43 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 61358f16acc..0bc8eaa4cf9 100644
--- a/Makefile
+++ b/Makefile
@@ -513,7 +513,15 @@ include shared.mak
 #
 # === SHA-256 backend ===
 #
-# Define BLK_SHA256 to use the built-in SHA-256 routines.
+# ==== Default SHA-256 backend ====
+#
+# If no *_SHA256 backend is picked we'll fall fall back on using the
+# default.
+#
+# The default SHA-256 backend is shipped with Git. No flag is required
+# to enable it. To select it don't define any other *_SHA256 flag.
+#
+# ==== Other SHA-256 implementations ====
 #
 # Define NETTLE_SHA256 to use the SHA-256 routines in libnettle.
 #
-- 
2.38.0.1178.g509f5fa8ce0


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v3 7/9] Makefile: document SHA-1 and SHA-256 default and selection order
  2022-10-20 22:43   ` [PATCH v3 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
                       ` (5 preceding siblings ...)
  2022-10-20 22:43     ` [PATCH v3 6/9] Makefile: document default SHA-256 backend Ævar Arnfjörð Bjarmason
@ 2022-10-20 22:43     ` Ævar Arnfjörð Bjarmason
  2022-10-20 22:58       ` Eric Sunshine
  2022-10-20 22:43     ` [PATCH v3 8/9] Makefile: document default SHA-1 backend on OSX Ævar Arnfjörð Bjarmason
                       ` (2 subsequent siblings)
  9 siblings, 1 reply; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-20 22:43 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 make 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 <avarab@gmail.com>
---
 Makefile | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 0bc8eaa4cf9..8a9f3e561f8 100644
--- a/Makefile
+++ b/Makefile
@@ -481,6 +481,14 @@ include shared.mak
 #
 # === SHA-1 backend ===
 #
+# ==== Default SHA-1 backend ====
+#
+# If no *_SHA1 backend is picked we'll fall fall back on using the
+# default.
+#
+# Multiple *_SHA1 backends can be selected, the first supported one
+# listed in "Other 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
@@ -505,12 +513,12 @@ include shared.mak
 #
 # ==== Other SHA-1 implementations ====
 #
-# 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 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).
+#
 # === SHA-256 backend ===
 #
 # ==== Default SHA-256 backend ====
@@ -521,14 +529,17 @@ include shared.mak
 # The default SHA-256 backend is shipped with Git. No flag is required
 # to enable it. To select it don't define any other *_SHA256 flag.
 #
+# Multiple *_SHA256 backends can be selected, the first supported one
+# listed in "Other SHA-256 implementations" below will be picked.
+#
 # ==== Other 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.
-#
 # == DEVELOPER defines ==
 #
 # Define DEVELOPER to enable more compiler warnings. Compiler version
-- 
2.38.0.1178.g509f5fa8ce0


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v3 8/9] Makefile: document default SHA-1 backend on OSX
  2022-10-20 22:43   ` [PATCH v3 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
                       ` (6 preceding siblings ...)
  2022-10-20 22:43     ` [PATCH v3 7/9] Makefile: document SHA-1 and SHA-256 default and selection order Ævar Arnfjörð Bjarmason
@ 2022-10-20 22:43     ` Ævar Arnfjörð Bjarmason
  2022-10-20 23:01       ` Eric Sunshine
  2022-10-20 22:43     ` [PATCH v3 9/9] Makefile: discuss SHAttered in *_SHA{1,256} discussion Ævar Arnfjörð Bjarmason
  2022-10-26 14:56     ` [PATCH v4 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
  9 siblings, 1 reply; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-20 22:43 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Makefile b/Makefile
index 8a9f3e561f8..0ca8a9eb318 100644
--- a/Makefile
+++ b/Makefile
@@ -519,6 +519,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. We'll define NO_APPLE_COMMON_CRYPTO on Mac OS
+# 10.4 or older ("Tiger", released in early 2005).
+#
 # === SHA-256 backend ===
 #
 # ==== Default SHA-256 backend ====
-- 
2.38.0.1178.g509f5fa8ce0


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v3 9/9] Makefile: discuss SHAttered in *_SHA{1,256} discussion
  2022-10-20 22:43   ` [PATCH v3 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
                       ` (7 preceding siblings ...)
  2022-10-20 22:43     ` [PATCH v3 8/9] Makefile: document default SHA-1 backend on OSX Ævar Arnfjörð Bjarmason
@ 2022-10-20 22:43     ` Ævar Arnfjörð Bjarmason
  2022-10-26 14:56     ` [PATCH v4 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-20 22:43 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Makefile b/Makefile
index 0ca8a9eb318..36d6bffd1f1 100644
--- a/Makefile
+++ b/Makefile
@@ -481,6 +481,16 @@ 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 (DC_SHA1).
+#
+# If you know that you can trust the repository contents, or where
+# potential SHA-1 attacks are otherwise mitigated the backends listed
+# in "Other SHA-1 implementations" are faster than DC_SHA1.
+#
 # ==== Default SHA-1 backend ====
 #
 # If no *_SHA1 backend is picked we'll fall fall back on using the
@@ -526,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.
+#
 # ==== Default SHA-256 backend ====
 #
 # If no *_SHA256 backend is picked we'll fall fall back on using the
-- 
2.38.0.1178.g509f5fa8ce0


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* Re: [PATCH v3 7/9] Makefile: document SHA-1 and SHA-256 default and selection order
  2022-10-20 22:43     ` [PATCH v3 7/9] Makefile: document SHA-1 and SHA-256 default and selection order Ævar Arnfjörð Bjarmason
@ 2022-10-20 22:58       ` Eric Sunshine
  0 siblings, 0 replies; 55+ messages in thread
From: Eric Sunshine @ 2022-10-20 22:58 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Glen Choo, Eric DeCosta

On Thu, Oct 20, 2022 at 6:43 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> 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 make it easier to e.g. integrate with autoconf-like

s/make/makes/

> systems, where the configuration can provide everything it can
> support, and Git is tasked with picking the first one it prefers.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> diff --git a/Makefile b/Makefile
> @@ -481,6 +481,14 @@ include shared.mak
> +# ==== Default SHA-1 backend ====
> +#
> +# If no *_SHA1 backend is picked we'll fall fall back on using the
> +# default.

s/fall fall/fall/

>  # Define OPENSSL_SHA1 to link to the the SHA-1 routines from
>  # the OpenSSL library.

Not a problem introduced by this patch, but this could use a: s/the the/the/

^ permalink raw reply	[flat|nested] 55+ messages in thread

* Re: [PATCH v3 8/9] Makefile: document default SHA-1 backend on OSX
  2022-10-20 22:43     ` [PATCH v3 8/9] Makefile: document default SHA-1 backend on OSX Ævar Arnfjörð Bjarmason
@ 2022-10-20 23:01       ` Eric Sunshine
  0 siblings, 0 replies; 55+ messages in thread
From: Eric Sunshine @ 2022-10-20 23:01 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Glen Choo, Eric DeCosta

On Thu, Oct 20, 2022 at 6:43 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> 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.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> diff --git a/Makefile b/Makefile
> @@ -519,6 +519,11 @@ include shared.mak
> +# 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. We'll define NO_APPLE_COMMON_CRYPTO on Mac OS
> +# 10.4 or older ("Tiger", released in early 2005).

I found the "we'll define..." part somewhat confusing. I might have
written it as:

    On macOS 01.4 (Tiger) or older, NO_APPLE_COMMON_CRYPTO
    is defined by default.

or:

    ...is defined automatically.

Probably not worth a reroll.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH v4 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug
  2022-10-20 22:43   ` [PATCH v3 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
                       ` (8 preceding siblings ...)
  2022-10-20 22:43     ` [PATCH v3 9/9] Makefile: discuss SHAttered in *_SHA{1,256} discussion Ævar Arnfjörð Bjarmason
@ 2022-10-26 14:56     ` Ævar Arnfjörð Bjarmason
  2022-10-26 14:56       ` [PATCH v4 1/9] Makefile: always (re)set DC_SHA1 on fallback Ævar Arnfjörð Bjarmason
                         ` (9 more replies)
  9 siblings, 10 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-26 14:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

This is a documentation update to get to the eventual goal of making
DC_SHA1 the defaul on OSX. First we need to stop claiming that OpenSSL
is still our default everywhere, etc.

For v3 see:
https://lore.kernel.org/git/cover-v3-0.9-00000000000-20221020T223946Z-avarab@gmail.com/;
The updates here are all typos/grammar etc. issues spotted by Eric
Sunshine, thanks!

Junio: This v4 introduces a minor conflict with the subsequent DC_SHA1
topic[1], but you haven't picked that one up either, so I'm assuming
if you're interested in this at all it's better to cook this first.

In case you want both It's easily solved (just keep the other topic's
post-image). I.e. Eric pointed out a better wording for a paragraph,
but it's the one we're mostly removing/rewriting in [1].

1. https://lore.kernel.org/git/cover-v2-0.4-00000000000-20221019T010222Z-avarab@gmail.com/

Ævar Arnfjörð Bjarmason (9):
  Makefile: always (re)set DC_SHA1 on fallback
  INSTALL: remove discussion of SHA-1 backends
  Makefile: correct DC_SHA1 documentation
  Makefile: create and use sections for "define" flag listing
  Makefile: rephrase the discussion of *_SHA1 knobs
  Makefile: document default SHA-256 backend
  Makefile: document SHA-1 and SHA-256 default and selection order
  Makefile: document default SHA-1 backend on OSX
  Makefile: discuss SHAttered in *_SHA{1,256} discussion

 INSTALL  |   4 -
 Makefile | 259 +++++++++++++++++++++++++++++++++++--------------------
 2 files changed, 165 insertions(+), 98 deletions(-)

Range-diff against v3:
 1:  ef3c5be11e0 =  1:  11e92d15616 Makefile: always (re)set DC_SHA1 on fallback
 2:  017a0a9791c =  2:  abbe25f56b3 INSTALL: remove discussion of SHA-1 backends
 3:  62dd2d5708d =  3:  b0bd35987c0 Makefile: correct DC_SHA1 documentation
 4:  933bef576b3 =  4:  d0451d6c3a3 Makefile: create and use sections for "define" flag listing
 5:  5b18198c477 !  5:  b956d1c2640 Makefile: rephrase the discussion of *_SHA1 knobs
    @@ Makefile: include shared.mak
      #
     -# Define OPENSSL_SHA1 environment variable when running make to link
     -# with the SHA1 routine from openssl library.
    -+# Define OPENSSL_SHA1 to link to the the SHA-1 routines from
    -+# the OpenSSL library.
    ++# Define OPENSSL_SHA1 to link to the SHA-1 routines from the OpenSSL
    ++# library.
      #
      # === SHA-256 backend ===
      #
 6:  73685592aba =  6:  1e4695d0ba0 Makefile: document default SHA-256 backend
 7:  05edcfb9cd9 !  7:  847be3d32e2 Makefile: document SHA-1 and SHA-256 default and selection order
    @@ Commit message
         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 make it easier to e.g. integrate with autoconf-like
    +    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.
     
    @@ Makefile: include shared.mak
      #
     +# ==== Default SHA-1 backend ====
     +#
    -+# If no *_SHA1 backend is picked we'll fall fall back on using the
    -+# default.
    ++# If no *_SHA1 backend is picked we'll fall back on using the default.
     +#
     +# Multiple *_SHA1 backends can be selected, the first supported one
     +# listed in "Other SHA-1 implementations" will be picked.
    @@ Makefile: include shared.mak
     -# 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 the SHA-1 routines from
    - # the OpenSSL library.
    + # 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).
 8:  859e69fbe9f !  8:  0af3ea78eaf Makefile: document default SHA-1 backend on OSX
    @@ Makefile: include shared.mak
      #
     +# 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. We'll define NO_APPLE_COMMON_CRYPTO on Mac OS
    -+# 10.4 or older ("Tiger", released in early 2005).
    ++# default on that OS. On macOS 01.4 (Tiger) or older,
    ++# NO_APPLE_COMMON_CRYPTO is defined by default.
     +#
      # === SHA-256 backend ===
      #
 9:  c1f27255d3e !  9:  9045ff9c4ed Makefile: discuss SHAttered in *_SHA{1,256} discussion
    @@ Makefile: include shared.mak
     +#
      # ==== Default SHA-1 backend ====
      #
    - # If no *_SHA1 backend is picked we'll fall fall back on using the
    + # If no *_SHA1 backend is picked we'll fall back on using the default.
     @@ Makefile: include shared.mak
      #
      # === SHA-256 backend ===
-- 
2.38.0.1251.g3eefdfb5e7a


^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH v4 1/9] Makefile: always (re)set DC_SHA1 on fallback
  2022-10-26 14:56     ` [PATCH v4 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
@ 2022-10-26 14:56       ` Ævar Arnfjörð Bjarmason
  2022-10-26 14:56       ` [PATCH v4 2/9] INSTALL: remove discussion of SHA-1 backends Ævar Arnfjörð Bjarmason
                         ` (8 subsequent siblings)
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-26 14:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 85f03c6aed1..744bd4344f4 100644
--- a/Makefile
+++ b/Makefile
@@ -1823,7 +1823,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
-- 
2.38.0.1251.g3eefdfb5e7a


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v4 2/9] INSTALL: remove discussion of SHA-1 backends
  2022-10-26 14:56     ` [PATCH v4 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
  2022-10-26 14:56       ` [PATCH v4 1/9] Makefile: always (re)set DC_SHA1 on fallback Ævar Arnfjörð Bjarmason
@ 2022-10-26 14:56       ` Ævar Arnfjörð Bjarmason
  2022-10-26 14:56       ` [PATCH v4 3/9] Makefile: correct DC_SHA1 documentation Ævar Arnfjörð Bjarmason
                         ` (7 subsequent siblings)
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-26 14:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 INSTALL | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/INSTALL b/INSTALL
index 89b15d71df5..33447883974 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
-- 
2.38.0.1251.g3eefdfb5e7a


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v4 3/9] Makefile: correct DC_SHA1 documentation
  2022-10-26 14:56     ` [PATCH v4 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
  2022-10-26 14:56       ` [PATCH v4 1/9] Makefile: always (re)set DC_SHA1 on fallback Ævar Arnfjörð Bjarmason
  2022-10-26 14:56       ` [PATCH v4 2/9] INSTALL: remove discussion of SHA-1 backends Ævar Arnfjörð Bjarmason
@ 2022-10-26 14:56       ` Ævar Arnfjörð Bjarmason
  2022-10-26 14:56       ` [PATCH v4 4/9] Makefile: create and use sections for "define" flag listing Ævar Arnfjörð Bjarmason
                         ` (6 subsequent siblings)
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-26 14:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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.

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 <avarab@gmail.com>
---
 Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 744bd4344f4..a2762e583a1 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.
-- 
2.38.0.1251.g3eefdfb5e7a


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v4 4/9] Makefile: create and use sections for "define" flag listing
  2022-10-26 14:56     ` [PATCH v4 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
                         ` (2 preceding siblings ...)
  2022-10-26 14:56       ` [PATCH v4 3/9] Makefile: correct DC_SHA1 documentation Ævar Arnfjörð Bjarmason
@ 2022-10-26 14:56       ` Ævar Arnfjörð Bjarmason
  2022-10-26 14:56       ` [PATCH v4 5/9] Makefile: rephrase the discussion of *_SHA1 knobs Ævar Arnfjörð Bjarmason
                         ` (5 subsequent siblings)
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-26 14:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 218 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 126 insertions(+), 92 deletions(-)

diff --git a/Makefile b/Makefile
index a2762e583a1..a983a57fb51 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,120 @@ include shared.mak
 # to the "<name>" of the corresponding `compat/fsmonitor/fsm-settings-<name>.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.
+#
+# ==== Options for the sha1collisiondetection implementation ====
+#
+# 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.
+#
+# ==== Other SHA-1 implementations ====
+#
+# 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.
+#
+# === SHA-256 backend ===
+#
+# 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
-- 
2.38.0.1251.g3eefdfb5e7a


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v4 5/9] Makefile: rephrase the discussion of *_SHA1 knobs
  2022-10-26 14:56     ` [PATCH v4 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
                         ` (3 preceding siblings ...)
  2022-10-26 14:56       ` [PATCH v4 4/9] Makefile: create and use sections for "define" flag listing Ævar Arnfjörð Bjarmason
@ 2022-10-26 14:56       ` Ævar Arnfjörð Bjarmason
  2022-10-26 14:56       ` [PATCH v4 6/9] Makefile: document default SHA-256 backend Ævar Arnfjörð Bjarmason
                         ` (4 subsequent siblings)
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-26 14:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index a983a57fb51..870ecdb0d85 100644
--- a/Makefile
+++ b/Makefile
@@ -492,12 +492,12 @@ include shared.mak
 # 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
+# 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
@@ -505,11 +505,11 @@ include shared.mak
 #
 # ==== Other SHA-1 implementations ====
 #
-# 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.
 #
 # === SHA-256 backend ===
 #
-- 
2.38.0.1251.g3eefdfb5e7a


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v4 6/9] Makefile: document default SHA-256 backend
  2022-10-26 14:56     ` [PATCH v4 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
                         ` (4 preceding siblings ...)
  2022-10-26 14:56       ` [PATCH v4 5/9] Makefile: rephrase the discussion of *_SHA1 knobs Ævar Arnfjörð Bjarmason
@ 2022-10-26 14:56       ` Ævar Arnfjörð Bjarmason
  2022-10-26 14:56       ` [PATCH v4 7/9] Makefile: document SHA-1 and SHA-256 default and selection order Ævar Arnfjörð Bjarmason
                         ` (3 subsequent siblings)
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-26 14:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 870ecdb0d85..992eba9e0c8 100644
--- a/Makefile
+++ b/Makefile
@@ -513,7 +513,15 @@ include shared.mak
 #
 # === SHA-256 backend ===
 #
-# Define BLK_SHA256 to use the built-in SHA-256 routines.
+# ==== Default SHA-256 backend ====
+#
+# If no *_SHA256 backend is picked we'll fall fall back on using the
+# default.
+#
+# The default SHA-256 backend is shipped with Git. No flag is required
+# to enable it. To select it don't define any other *_SHA256 flag.
+#
+# ==== Other SHA-256 implementations ====
 #
 # Define NETTLE_SHA256 to use the SHA-256 routines in libnettle.
 #
-- 
2.38.0.1251.g3eefdfb5e7a


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v4 7/9] Makefile: document SHA-1 and SHA-256 default and selection order
  2022-10-26 14:56     ` [PATCH v4 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
                         ` (5 preceding siblings ...)
  2022-10-26 14:56       ` [PATCH v4 6/9] Makefile: document default SHA-256 backend Ævar Arnfjörð Bjarmason
@ 2022-10-26 14:56       ` Ævar Arnfjörð Bjarmason
  2022-10-26 22:30         ` Junio C Hamano
  2022-10-26 14:56       ` [PATCH v4 8/9] Makefile: document default SHA-1 backend on OSX Ævar Arnfjörð Bjarmason
                         ` (2 subsequent siblings)
  9 siblings, 1 reply; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-26 14:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 992eba9e0c8..9f7cf1f22d4 100644
--- a/Makefile
+++ b/Makefile
@@ -481,6 +481,13 @@ include shared.mak
 #
 # === SHA-1 backend ===
 #
+# ==== Default SHA-1 backend ====
+#
+# If no *_SHA1 backend is picked we'll fall back on using the default.
+#
+# Multiple *_SHA1 backends can be selected, the first supported one
+# listed in "Other 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
@@ -505,12 +512,12 @@ include shared.mak
 #
 # ==== Other SHA-1 implementations ====
 #
-# 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 BLK_SHA1 to make use of optimized C SHA-1 routines bundled
+# with git (in the block-sha1/ directory).
+#
 # === SHA-256 backend ===
 #
 # ==== Default SHA-256 backend ====
@@ -521,14 +528,17 @@ include shared.mak
 # The default SHA-256 backend is shipped with Git. No flag is required
 # to enable it. To select it don't define any other *_SHA256 flag.
 #
+# Multiple *_SHA256 backends can be selected, the first supported one
+# listed in "Other SHA-256 implementations" below will be picked.
+#
 # ==== Other 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.
-#
 # == DEVELOPER defines ==
 #
 # Define DEVELOPER to enable more compiler warnings. Compiler version
-- 
2.38.0.1251.g3eefdfb5e7a


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v4 8/9] Makefile: document default SHA-1 backend on OSX
  2022-10-26 14:56     ` [PATCH v4 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
                         ` (6 preceding siblings ...)
  2022-10-26 14:56       ` [PATCH v4 7/9] Makefile: document SHA-1 and SHA-256 default and selection order Ævar Arnfjörð Bjarmason
@ 2022-10-26 14:56       ` Ævar Arnfjörð Bjarmason
  2022-10-26 14:56       ` [PATCH v4 9/9] Makefile: discuss SHAttered in *_SHA{1,256} discussion Ævar Arnfjörð Bjarmason
  2022-11-07 21:23       ` [PATCH v5 00/10] Makefile, docs & code: document & fix SHA-{1,256} selection behavior Ævar Arnfjörð Bjarmason
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-26 14:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Makefile b/Makefile
index 9f7cf1f22d4..a0ca6456b85 100644
--- a/Makefile
+++ b/Makefile
@@ -518,6 +518,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.
+#
 # === SHA-256 backend ===
 #
 # ==== Default SHA-256 backend ====
-- 
2.38.0.1251.g3eefdfb5e7a


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v4 9/9] Makefile: discuss SHAttered in *_SHA{1,256} discussion
  2022-10-26 14:56     ` [PATCH v4 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
                         ` (7 preceding siblings ...)
  2022-10-26 14:56       ` [PATCH v4 8/9] Makefile: document default SHA-1 backend on OSX Ævar Arnfjörð Bjarmason
@ 2022-10-26 14:56       ` Ævar Arnfjörð Bjarmason
  2022-11-07 21:23       ` [PATCH v5 00/10] Makefile, docs & code: document & fix SHA-{1,256} selection behavior Ævar Arnfjörð Bjarmason
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-10-26 14:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Makefile b/Makefile
index a0ca6456b85..805e88ed5fd 100644
--- a/Makefile
+++ b/Makefile
@@ -481,6 +481,16 @@ 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 (DC_SHA1).
+#
+# If you know that you can trust the repository contents, or where
+# potential SHA-1 attacks are otherwise mitigated the backends listed
+# in "Other SHA-1 implementations" are faster than DC_SHA1.
+#
 # ==== Default SHA-1 backend ====
 #
 # If no *_SHA1 backend is picked we'll fall back on using the default.
@@ -525,6 +535,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.
+#
 # ==== Default SHA-256 backend ====
 #
 # If no *_SHA256 backend is picked we'll fall fall back on using the
-- 
2.38.0.1251.g3eefdfb5e7a


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* Re: [PATCH v4 7/9] Makefile: document SHA-1 and SHA-256 default and selection order
  2022-10-26 14:56       ` [PATCH v4 7/9] Makefile: document SHA-1 and SHA-256 default and selection order Ævar Arnfjörð Bjarmason
@ 2022-10-26 22:30         ` Junio C Hamano
  0 siblings, 0 replies; 55+ messages in thread
From: Junio C Hamano @ 2022-10-26 22:30 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> 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.

That makes sense.  What is not explicit enough, at least at this
stage, is that, even though we say "if you do not choose one, the
default is used" (which by the way is an awkward thing to see---that
is the definition of being the default, so the statement itself has
zero bit of useful information), we do not say what implementation
exactly is used as the default.

> 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.

Well explained.

> +# ==== Default SHA-1 backend ====
> +#
> +# If no *_SHA1 backend is picked we'll fall back on using the default.
> +#
> +# Multiple *_SHA1 backends can be selected, the first supported one
> +# listed in "Other SHA-1 implementations" will be picked.

^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH v5 00/10] Makefile, docs & code: document & fix SHA-{1,256} selection behavior
  2022-10-26 14:56     ` [PATCH v4 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
                         ` (8 preceding siblings ...)
  2022-10-26 14:56       ` [PATCH v4 9/9] Makefile: discuss SHAttered in *_SHA{1,256} discussion Ævar Arnfjörð Bjarmason
@ 2022-11-07 21:23       ` Ævar Arnfjörð Bjarmason
  2022-11-07 21:23         ` [PATCH v5 01/10] Makefile: always (re)set DC_SHA1 on fallback Ævar Arnfjörð Bjarmason
                           ` (9 more replies)
  9 siblings, 10 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-11-07 21:23 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

This is mostly a documentation update to get to the eventual goal of
making DC_SHA1 the default on OSX. First we need to stop claiming that
OpenSSL is still our default everywhere, etc.

For v4, see:
https://lore.kernel.org/git/cover-v4-0.9-00000000000-20221026T145255Z-avarab@gmail.com/

For v4 Junio pointed out that it wasn't very clear what the "default"
is. The root cause of that (which I was hoping to avoid, but bit the
bullet here) is that DC_SHA1=Y on master doesn't actually do anything
at all.

Instead, whether we use DC_SHA1 or not depends on whether or not we
asked for any *other* backend, it's just our default fallback.

The documentation now reflects this, and to reinforce that a new 8/10
here gets rid of the Makefile flag entirely.

The only thing it was actually used for was to tell the test suite
that it should be running the SHA1DC tests. Instead we just have a
trivial test helper that we can ask whether that's the case. It picks
up a macro that we define in sha1dc_git.h.

Ævar Arnfjörð Bjarmason (10):
  Makefile: always (re)set DC_SHA1 on fallback
  INSTALL: remove discussion of SHA-1 backends
  Makefile: correct DC_SHA1 documentation
  Makefile: create and use sections for "define" flag listing
  Makefile: rephrase the discussion of *_SHA1 knobs
  Makefile: document default SHA-256 backend
  Makefile: document SHA-1 and SHA-256 default and selection order
  Makefile & test-tool: replace "DC_SHA1" variable with a "define"
  Makefile: document default SHA-1 backend on OSX
  Makefile: discuss SHAttered in *_SHA{1,256} discussion

 INSTALL                             |   4 -
 Makefile                            | 252 +++++++++++++++++-----------
 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 +-
 9 files changed, 173 insertions(+), 104 deletions(-)

Range-diff against v4:
 1:  11e92d15616 =  1:  24d503e5a2b Makefile: always (re)set DC_SHA1 on fallback
 2:  abbe25f56b3 =  2:  2f6c37b90cd INSTALL: remove discussion of SHA-1 backends
 3:  b0bd35987c0 !  3:  d05fbf8c6d8 Makefile: correct DC_SHA1 documentation
    @@ Commit message
         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)
     
 4:  d0451d6c3a3 !  4:  9ab55314296 Makefile: create and use sections for "define" flag listing
    @@ Makefile: include shared.mak
     +# in one call to the platform's SHA1_Update(). e.g. APPLE_COMMON_CRYPTO
     +# wants 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined.
     +#
    -+# ==== Options for the sha1collisiondetection implementation ====
    ++# ==== 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
    @@ Makefile: include shared.mak
     +# by the git project to migrate to using sha1collisiondetection as a
     +# submodule.
     +#
    -+# ==== Other SHA-1 implementations ====
    -+#
    -+# 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.
    -+#
     +# === 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.
 5:  b956d1c2640 !  5:  5f1da5d7dc4 Makefile: rephrase the discussion of *_SHA1 knobs
    @@ Makefile: include shared.mak
      # 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
    -+# 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
    -@@ Makefile: include shared.mak
    - #
    - # ==== Other SHA-1 implementations ====
    - #
     -# 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
    @@ Makefile: include shared.mak
     +# Define OPENSSL_SHA1 to link to the SHA-1 routines from the OpenSSL
     +# library.
      #
    - # === SHA-256 backend ===
    + # ==== 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
 6:  1e4695d0ba0 <  -:  ----------- Makefile: document default SHA-256 backend
 -:  ----------- >  6:  2cadbddcc04 Makefile: document default SHA-256 backend
 7:  847be3d32e2 !  7:  0d62359649f Makefile: document SHA-1 and SHA-256 default and selection order
    @@ Makefile: include shared.mak
      #
     +# ==== Default SHA-1 backend ====
     +#
    -+# If no *_SHA1 backend is picked we'll fall back on using the default.
    -+#
    -+# Multiple *_SHA1 backends can be selected, the first supported one
    -+# listed in "Other SHA-1 implementations" will be picked.
    ++# 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
     @@ Makefile: include shared.mak
      #
    - # ==== Other SHA-1 implementations ====
    + # ==== SHA-1 implementations ====
      #
    --# 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.
    ++# 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).
    -+#
    - # === SHA-256 backend ===
    + # 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 ====
      #
    - # ==== Default SHA-256 backend ====
     @@ Makefile: include shared.mak
    - # The default SHA-256 backend is shipped with Git. No flag is required
    - # to enable it. To select it don't define any other *_SHA256 flag.
      #
    -+# Multiple *_SHA256 backends can be selected, the first supported one
    -+# listed in "Other SHA-256 implementations" below will be picked.
    -+#
    - # ==== Other SHA-256 implementations ====
    + # ==== SHA-256 implementations ====
      #
     +# Define OPENSSL_SHA256 to use the SHA-256 routines in OpenSSL.
     +#
    @@ Makefile: include shared.mak
      #
     -# Define OPENSSL_SHA256 to use the SHA-256 routines in OpenSSL.
     -#
    - # == DEVELOPER defines ==
    + # If don't enable any of the *_SHA256 settings in this section, Git
    + # will default to its built-in sha256 implementation.
      #
    - # Define DEVELOPER to enable more compiler warnings. Compiler version
 -:  ----------- >  8:  6daac578fd0 Makefile & test-tool: replace "DC_SHA1" variable with a "define"
 8:  0af3ea78eaf !  9:  8d3dddf88a4 Makefile: document default SHA-1 backend on OSX
    @@ Makefile: include shared.mak
     +# default on that OS. On macOS 01.4 (Tiger) or older,
     +# NO_APPLE_COMMON_CRYPTO is defined by default.
     +#
    - # === SHA-256 backend ===
    - #
    - # ==== Default SHA-256 backend ====
    + # 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
 9:  9045ff9c4ed ! 10:  55d3751faea Makefile: discuss SHAttered in *_SHA{1,256} discussion
    @@ Makefile: include shared.mak
     +#
     +# 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 (DC_SHA1).
    ++# 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 backends listed
    -+# in "Other SHA-1 implementations" are faster than DC_SHA1.
    ++# 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 we'll fall back on using the default.
    + # If no *_SHA1 backend is picked, the first supported one listed in
     @@ Makefile: include shared.mak
      #
      # === SHA-256 backend ===
    @@ Makefile: include shared.mak
     +# Unlike SHA-1 the SHA-256 algorithm does not suffer from any known
     +# vulnerabilities, so any implementation will do.
     +#
    - # ==== Default SHA-256 backend ====
    + # ==== SHA-256 implementations ====
      #
    - # If no *_SHA256 backend is picked we'll fall fall back on using the
    + # Define OPENSSL_SHA256 to use the SHA-256 routines in OpenSSL.
-- 
2.38.0.1464.gea6794aacbc


^ permalink raw reply	[flat|nested] 55+ messages in thread

* [PATCH v5 01/10] Makefile: always (re)set DC_SHA1 on fallback
  2022-11-07 21:23       ` [PATCH v5 00/10] Makefile, docs & code: document & fix SHA-{1,256} selection behavior Ævar Arnfjörð Bjarmason
@ 2022-11-07 21:23         ` Ævar Arnfjörð Bjarmason
  2022-11-07 21:23         ` [PATCH v5 02/10] INSTALL: remove discussion of SHA-1 backends Ævar Arnfjörð Bjarmason
                           ` (8 subsequent siblings)
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-11-07 21:23 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 4927379184c..0ad9a6c5bc1 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
-- 
2.38.0.1464.gea6794aacbc


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v5 02/10] INSTALL: remove discussion of SHA-1 backends
  2022-11-07 21:23       ` [PATCH v5 00/10] Makefile, docs & code: document & fix SHA-{1,256} selection behavior Ævar Arnfjörð Bjarmason
  2022-11-07 21:23         ` [PATCH v5 01/10] Makefile: always (re)set DC_SHA1 on fallback Ævar Arnfjörð Bjarmason
@ 2022-11-07 21:23         ` Ævar Arnfjörð Bjarmason
  2022-11-07 21:23         ` [PATCH v5 03/10] Makefile: correct DC_SHA1 documentation Ævar Arnfjörð Bjarmason
                           ` (7 subsequent siblings)
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-11-07 21:23 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 INSTALL | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/INSTALL b/INSTALL
index 89b15d71df5..33447883974 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
-- 
2.38.0.1464.gea6794aacbc


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v5 03/10] Makefile: correct DC_SHA1 documentation
  2022-11-07 21:23       ` [PATCH v5 00/10] Makefile, docs & code: document & fix SHA-{1,256} selection behavior Ævar Arnfjörð Bjarmason
  2022-11-07 21:23         ` [PATCH v5 01/10] Makefile: always (re)set DC_SHA1 on fallback Ævar Arnfjörð Bjarmason
  2022-11-07 21:23         ` [PATCH v5 02/10] INSTALL: remove discussion of SHA-1 backends Ævar Arnfjörð Bjarmason
@ 2022-11-07 21:23         ` Ævar Arnfjörð Bjarmason
  2022-11-07 21:23         ` [PATCH v5 04/10] Makefile: create and use sections for "define" flag listing Ævar Arnfjörð Bjarmason
                           ` (6 subsequent siblings)
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-11-07 21:23 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 0ad9a6c5bc1..489327ecd9b 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.
-- 
2.38.0.1464.gea6794aacbc


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v5 04/10] Makefile: create and use sections for "define" flag listing
  2022-11-07 21:23       ` [PATCH v5 00/10] Makefile, docs & code: document & fix SHA-{1,256} selection behavior Ævar Arnfjörð Bjarmason
                           ` (2 preceding siblings ...)
  2022-11-07 21:23         ` [PATCH v5 03/10] Makefile: correct DC_SHA1 documentation Ævar Arnfjörð Bjarmason
@ 2022-11-07 21:23         ` Ævar Arnfjörð Bjarmason
  2022-11-07 21:23         ` [PATCH v5 05/10] Makefile: rephrase the discussion of *_SHA1 knobs Ævar Arnfjörð Bjarmason
                           ` (5 subsequent siblings)
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-11-07 21:23 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 220 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 128 insertions(+), 92 deletions(-)

diff --git a/Makefile b/Makefile
index 489327ecd9b..19235624684 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 "<name>" of the corresponding `compat/fsmonitor/fsm-settings-<name>.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
-- 
2.38.0.1464.gea6794aacbc


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v5 05/10] Makefile: rephrase the discussion of *_SHA1 knobs
  2022-11-07 21:23       ` [PATCH v5 00/10] Makefile, docs & code: document & fix SHA-{1,256} selection behavior Ævar Arnfjörð Bjarmason
                           ` (3 preceding siblings ...)
  2022-11-07 21:23         ` [PATCH v5 04/10] Makefile: create and use sections for "define" flag listing Ævar Arnfjörð Bjarmason
@ 2022-11-07 21:23         ` Ævar Arnfjörð Bjarmason
  2022-11-07 21:23         ` [PATCH v5 06/10] Makefile: document default SHA-256 backend Ævar Arnfjörð Bjarmason
                           ` (4 subsequent siblings)
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-11-07 21:23 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 19235624684..251368b315d 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
-- 
2.38.0.1464.gea6794aacbc


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v5 06/10] Makefile: document default SHA-256 backend
  2022-11-07 21:23       ` [PATCH v5 00/10] Makefile, docs & code: document & fix SHA-{1,256} selection behavior Ævar Arnfjörð Bjarmason
                           ` (4 preceding siblings ...)
  2022-11-07 21:23         ` [PATCH v5 05/10] Makefile: rephrase the discussion of *_SHA1 knobs Ævar Arnfjörð Bjarmason
@ 2022-11-07 21:23         ` Ævar Arnfjörð Bjarmason
  2022-11-07 21:23         ` [PATCH v5 07/10] Makefile: document SHA-1 and SHA-256 default and selection order Ævar Arnfjörð Bjarmason
                           ` (3 subsequent siblings)
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-11-07 21:23 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 251368b315d..646fbe5b7dd 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
-- 
2.38.0.1464.gea6794aacbc


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v5 07/10] Makefile: document SHA-1 and SHA-256 default and selection order
  2022-11-07 21:23       ` [PATCH v5 00/10] Makefile, docs & code: document & fix SHA-{1,256} selection behavior Ævar Arnfjörð Bjarmason
                           ` (5 preceding siblings ...)
  2022-11-07 21:23         ` [PATCH v5 06/10] Makefile: document default SHA-256 backend Ævar Arnfjörð Bjarmason
@ 2022-11-07 21:23         ` Ævar Arnfjörð Bjarmason
  2022-11-07 21:23         ` [PATCH v5 08/10] Makefile & test-tool: replace "DC_SHA1" variable with a "define" Ævar Arnfjörð Bjarmason
                           ` (2 subsequent siblings)
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-11-07 21:23 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 646fbe5b7dd..9b5f872d107 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.
 #
-- 
2.38.0.1464.gea6794aacbc


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v5 08/10] Makefile & test-tool: replace "DC_SHA1" variable with a "define"
  2022-11-07 21:23       ` [PATCH v5 00/10] Makefile, docs & code: document & fix SHA-{1,256} selection behavior Ævar Arnfjörð Bjarmason
                           ` (6 preceding siblings ...)
  2022-11-07 21:23         ` [PATCH v5 07/10] Makefile: document SHA-1 and SHA-256 default and selection order Ævar Arnfjörð Bjarmason
@ 2022-11-07 21:23         ` Ævar Arnfjörð Bjarmason
  2022-11-07 21:23         ` [PATCH v5 09/10] Makefile: document default SHA-1 backend on OSX Ævar Arnfjörð Bjarmason
  2022-11-07 21:23         ` [PATCH v5 10/10] Makefile: discuss SHAttered in *_SHA{1,256} discussion Ævar Arnfjörð Bjarmason
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-11-07 21:23 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 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 9b5f872d107..0c79546712f 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 1808e3b1ce1..24d20a5d648 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 3957e4cf8cd..2f6e0197ffa 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 41e1c3fd3f7..60e3ce84395 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 d860c387c38..71fe5c61455 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 01cda9358df..775854c9f96 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 ca2948066fd..7cf84eca12e 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 9ad76080aa4..53240476896 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
 
-- 
2.38.0.1464.gea6794aacbc


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v5 09/10] Makefile: document default SHA-1 backend on OSX
  2022-11-07 21:23       ` [PATCH v5 00/10] Makefile, docs & code: document & fix SHA-{1,256} selection behavior Ævar Arnfjörð Bjarmason
                           ` (7 preceding siblings ...)
  2022-11-07 21:23         ` [PATCH v5 08/10] Makefile & test-tool: replace "DC_SHA1" variable with a "define" Ævar Arnfjörð Bjarmason
@ 2022-11-07 21:23         ` Ævar Arnfjörð Bjarmason
  2022-11-07 21:23         ` [PATCH v5 10/10] Makefile: discuss SHAttered in *_SHA{1,256} discussion Ævar Arnfjörð Bjarmason
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-11-07 21:23 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Makefile b/Makefile
index 0c79546712f..7d0fa7adb61 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
-- 
2.38.0.1464.gea6794aacbc


^ permalink raw reply related	[flat|nested] 55+ messages in thread

* [PATCH v5 10/10] Makefile: discuss SHAttered in *_SHA{1,256} discussion
  2022-11-07 21:23       ` [PATCH v5 00/10] Makefile, docs & code: document & fix SHA-{1,256} selection behavior Ævar Arnfjörð Bjarmason
                           ` (8 preceding siblings ...)
  2022-11-07 21:23         ` [PATCH v5 09/10] Makefile: document default SHA-1 backend on OSX Ævar Arnfjörð Bjarmason
@ 2022-11-07 21:23         ` Ævar Arnfjörð Bjarmason
  9 siblings, 0 replies; 55+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-11-07 21:23 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Mike Hommey, brian m . carlson,
	Carlo Marcelo Arenas Belón, Eric Sunshine, Glen Choo,
	Eric DeCosta, Ævar Arnfjörð Bjarmason

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 <avarab@gmail.com>
---
 Makefile | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Makefile b/Makefile
index 7d0fa7adb61..91596bac4c0 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.
-- 
2.38.0.1464.gea6794aacbc


^ permalink raw reply related	[flat|nested] 55+ messages in thread

end of thread, other threads:[~2022-11-07 21:24 UTC | newest]

Thread overview: 55+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-22  9:53 [PATCH 0/5] core: update our SHA-1 docs, use sha1collisiondetection on OSX too Ævar Arnfjörð Bjarmason
2022-04-22  9:53 ` [PATCH 1/5] Makefile: create and use sections for "define" flag listing Ævar Arnfjörð Bjarmason
2022-04-22  9:53 ` [PATCH 2/5] Makefile: really use and document sha1collisiondetection by default Ævar Arnfjörð Bjarmason
2022-04-22  9:53 ` [PATCH 3/5] Makefile: rephrase the discussion of *_SHA1 knobs Ævar Arnfjörð Bjarmason
2022-04-22  9:53 ` [PATCH 4/5] Makefile + hash.h: remove PPC_SHA1 implementation Ævar Arnfjörð Bjarmason
2022-04-22  9:53 ` [PATCH 5/5] Makefile: use $(OBJECTS) instead of $(C_OBJ) Ævar Arnfjörð Bjarmason
2022-04-22 18:56 ` [PATCH 0/5] core: update our SHA-1 docs, use sha1collisiondetection on OSX too Junio C Hamano
2022-05-19 20:14   ` Ævar Arnfjörð Bjarmason
2022-05-26 19:02     ` Ævar Arnfjörð Bjarmason
2022-10-19  1:03 ` [PATCH v2 0/4] " Ævar Arnfjörð Bjarmason
2022-10-19  1:03   ` [PATCH v2 1/4] fsmonitor OSX: compile with DC_SHA1=YesPlease Ævar Arnfjörð Bjarmason
2022-10-19  1:03   ` [PATCH v2 2/4] Makefile: create and use sections for "define" flag listing Ævar Arnfjörð Bjarmason
2022-10-19  1:03   ` [PATCH v2 3/4] Makefile: really use and document sha1collisiondetection by default Ævar Arnfjörð Bjarmason
2022-10-19  2:59     ` Eric Sunshine
2022-10-19 16:28       ` Junio C Hamano
2022-10-19 18:54         ` Ævar Arnfjörð Bjarmason
2022-10-19 19:43           ` Junio C Hamano
2022-10-19 22:15           ` Junio C Hamano
2022-10-19 22:27             ` Junio C Hamano
2022-10-20 21:15         ` brian m. carlson
2022-10-19  1:03   ` [PATCH v2 4/4] Makefile: rephrase the discussion of *_SHA1 knobs Ævar Arnfjörð Bjarmason
2022-10-20 22:43   ` [PATCH v3 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
2022-10-20 22:43     ` [PATCH v3 1/9] Makefile: always (re)set DC_SHA1 on fallback Ævar Arnfjörð Bjarmason
2022-10-20 22:43     ` [PATCH v3 2/9] INSTALL: remove discussion of SHA-1 backends Ævar Arnfjörð Bjarmason
2022-10-20 22:43     ` [PATCH v3 3/9] Makefile: correct DC_SHA1 documentation Ævar Arnfjörð Bjarmason
2022-10-20 22:43     ` [PATCH v3 4/9] Makefile: create and use sections for "define" flag listing Ævar Arnfjörð Bjarmason
2022-10-20 22:43     ` [PATCH v3 5/9] Makefile: rephrase the discussion of *_SHA1 knobs Ævar Arnfjörð Bjarmason
2022-10-20 22:43     ` [PATCH v3 6/9] Makefile: document default SHA-256 backend Ævar Arnfjörð Bjarmason
2022-10-20 22:43     ` [PATCH v3 7/9] Makefile: document SHA-1 and SHA-256 default and selection order Ævar Arnfjörð Bjarmason
2022-10-20 22:58       ` Eric Sunshine
2022-10-20 22:43     ` [PATCH v3 8/9] Makefile: document default SHA-1 backend on OSX Ævar Arnfjörð Bjarmason
2022-10-20 23:01       ` Eric Sunshine
2022-10-20 22:43     ` [PATCH v3 9/9] Makefile: discuss SHAttered in *_SHA{1,256} discussion Ævar Arnfjörð Bjarmason
2022-10-26 14:56     ` [PATCH v4 0/9] Makefile & docs: document SHA-{1,256} behavior, fix bug Ævar Arnfjörð Bjarmason
2022-10-26 14:56       ` [PATCH v4 1/9] Makefile: always (re)set DC_SHA1 on fallback Ævar Arnfjörð Bjarmason
2022-10-26 14:56       ` [PATCH v4 2/9] INSTALL: remove discussion of SHA-1 backends Ævar Arnfjörð Bjarmason
2022-10-26 14:56       ` [PATCH v4 3/9] Makefile: correct DC_SHA1 documentation Ævar Arnfjörð Bjarmason
2022-10-26 14:56       ` [PATCH v4 4/9] Makefile: create and use sections for "define" flag listing Ævar Arnfjörð Bjarmason
2022-10-26 14:56       ` [PATCH v4 5/9] Makefile: rephrase the discussion of *_SHA1 knobs Ævar Arnfjörð Bjarmason
2022-10-26 14:56       ` [PATCH v4 6/9] Makefile: document default SHA-256 backend Ævar Arnfjörð Bjarmason
2022-10-26 14:56       ` [PATCH v4 7/9] Makefile: document SHA-1 and SHA-256 default and selection order Ævar Arnfjörð Bjarmason
2022-10-26 22:30         ` Junio C Hamano
2022-10-26 14:56       ` [PATCH v4 8/9] Makefile: document default SHA-1 backend on OSX Ævar Arnfjörð Bjarmason
2022-10-26 14:56       ` [PATCH v4 9/9] Makefile: discuss SHAttered in *_SHA{1,256} discussion Ævar Arnfjörð Bjarmason
2022-11-07 21:23       ` [PATCH v5 00/10] Makefile, docs & code: document & fix SHA-{1,256} selection behavior Ævar Arnfjörð Bjarmason
2022-11-07 21:23         ` [PATCH v5 01/10] Makefile: always (re)set DC_SHA1 on fallback Ævar Arnfjörð Bjarmason
2022-11-07 21:23         ` [PATCH v5 02/10] INSTALL: remove discussion of SHA-1 backends Ævar Arnfjörð Bjarmason
2022-11-07 21:23         ` [PATCH v5 03/10] Makefile: correct DC_SHA1 documentation Ævar Arnfjörð Bjarmason
2022-11-07 21:23         ` [PATCH v5 04/10] Makefile: create and use sections for "define" flag listing Ævar Arnfjörð Bjarmason
2022-11-07 21:23         ` [PATCH v5 05/10] Makefile: rephrase the discussion of *_SHA1 knobs Ævar Arnfjörð Bjarmason
2022-11-07 21:23         ` [PATCH v5 06/10] Makefile: document default SHA-256 backend Ævar Arnfjörð Bjarmason
2022-11-07 21:23         ` [PATCH v5 07/10] Makefile: document SHA-1 and SHA-256 default and selection order Ævar Arnfjörð Bjarmason
2022-11-07 21:23         ` [PATCH v5 08/10] Makefile & test-tool: replace "DC_SHA1" variable with a "define" Ævar Arnfjörð Bjarmason
2022-11-07 21:23         ` [PATCH v5 09/10] Makefile: document default SHA-1 backend on OSX Ævar Arnfjörð Bjarmason
2022-11-07 21:23         ` [PATCH v5 10/10] Makefile: discuss SHAttered in *_SHA{1,256} discussion Ævar Arnfjörð Bjarmason

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).