git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 0/5] core: update our SHA-1 docs, use sha1collisiondetection on OSX too

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Apr 22, 2022, 09:53 UTC
Message-ID
<cover-0.5-00000000000-20220422T094624Z-avarab@gmail.com>

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
Next: Ævar Arnfjörð Bjarmason
Message 1 of 55 in “core: update our SHA-1 docs, use sha1collisiondetection on OSX too”
  1. 0/5 core: update our SHA-1 docs, use sha1collisiondetection on OSX tooÆvar Arnfjörð Bjarmason, Apr 22, 2022
  2. 1/5 Makefile: create and use sections for "define" flag listingÆvar Arnfjörð Bjarmason, Apr 22, 2022
  3. 3/5 Makefile: rephrase the discussion of *_SHA1 knobsÆvar Arnfjörð Bjarmason, Apr 22, 2022
  4. 5/5 Makefile: use $(OBJECTS) instead of $(C_OBJ)Ævar Arnfjörð Bjarmason, Apr 22, 2022
  5. 2/5 Makefile: really use and document sha1collisiondetection by defaultÆvar Arnfjörð Bjarmason, Apr 22, 2022
  6. 4/5 Makefile + hash.h: remove PPC_SHA1 implementationÆvar Arnfjörð Bjarmason, Apr 22, 2022
  7. Junio C HamanoApr 22, 2022
  8. Ævar Arnfjörð BjarmasonMay 19, 2022
  9. Ævar Arnfjörð BjarmasonMay 26, 2022
  10. 0/4 core: update our SHA-1 docs, use sha1collisiondetection on OSX tooÆvar Arnfjörð Bjarmason, Oct 19, 2022
  11. 1/4 fsmonitor OSX: compile with DC_SHA1=YesPleaseÆvar Arnfjörð Bjarmason, Oct 19, 2022
  12. 3/4 Makefile: really use and document sha1collisiondetection by defaultÆvar Arnfjörð Bjarmason, Oct 19, 2022
  13. Eric SunshineOct 19, 2022
  14. Junio C HamanoOct 19, 2022
  15. Ævar Arnfjörð BjarmasonOct 19, 2022
  16. Junio C HamanoOct 19, 2022
  17. Junio C HamanoOct 19, 2022
  18. Junio C HamanoOct 19, 2022
  19. brian m. carlsonOct 20, 2022
  20. 4/4 Makefile: rephrase the discussion of *_SHA1 knobsÆvar Arnfjörð Bjarmason, Oct 19, 2022
  21. 2/4 Makefile: create and use sections for "define" flag listingÆvar Arnfjörð Bjarmason, Oct 19, 2022
  22. 0/9 Makefile & docs: document SHA-{1,256} behavior, fix bugÆvar Arnfjörð Bjarmason, Oct 20, 2022
  23. 1/9 Makefile: always (re)set DC_SHA1 on fallbackÆvar Arnfjörð Bjarmason, Oct 20, 2022
  24. 2/9 INSTALL: remove discussion of SHA-1 backendsÆvar Arnfjörð Bjarmason, Oct 20, 2022
  25. 3/9 Makefile: correct DC_SHA1 documentationÆvar Arnfjörð Bjarmason, Oct 20, 2022
  26. 5/9 Makefile: rephrase the discussion of *_SHA1 knobsÆvar Arnfjörð Bjarmason, Oct 20, 2022
  27. 6/9 Makefile: document default SHA-256 backendÆvar Arnfjörð Bjarmason, Oct 20, 2022
  28. 7/9 Makefile: document SHA-1 and SHA-256 default and selection orderÆvar Arnfjörð Bjarmason, Oct 20, 2022
  29. Eric SunshineOct 20, 2022
  30. 4/9 Makefile: create and use sections for "define" flag listingÆvar Arnfjörð Bjarmason, Oct 20, 2022
  31. 8/9 Makefile: document default SHA-1 backend on OSXÆvar Arnfjörð Bjarmason, Oct 20, 2022
  32. Eric SunshineOct 20, 2022
  33. 9/9 Makefile: discuss SHAttered in *_SHA{1,256} discussionÆvar Arnfjörð Bjarmason, Oct 20, 2022
  34. 0/9 Makefile & docs: document SHA-{1,256} behavior, fix bugÆvar Arnfjörð Bjarmason, Oct 26, 2022
  35. 1/9 Makefile: always (re)set DC_SHA1 on fallbackÆvar Arnfjörð Bjarmason, Oct 26, 2022
  36. 2/9 INSTALL: remove discussion of SHA-1 backendsÆvar Arnfjörð Bjarmason, Oct 26, 2022
  37. 3/9 Makefile: correct DC_SHA1 documentationÆvar Arnfjörð Bjarmason, Oct 26, 2022
  38. 5/9 Makefile: rephrase the discussion of *_SHA1 knobsÆvar Arnfjörð Bjarmason, Oct 26, 2022
  39. 6/9 Makefile: document default SHA-256 backendÆvar Arnfjörð Bjarmason, Oct 26, 2022
  40. 7/9 Makefile: document SHA-1 and SHA-256 default and selection orderÆvar Arnfjörð Bjarmason, Oct 26, 2022
  41. Junio C HamanoOct 26, 2022
  42. 4/9 Makefile: create and use sections for "define" flag listingÆvar Arnfjörð Bjarmason, Oct 26, 2022
  43. 8/9 Makefile: document default SHA-1 backend on OSXÆvar Arnfjörð Bjarmason, Oct 26, 2022
  44. 9/9 Makefile: discuss SHAttered in *_SHA{1,256} discussionÆvar Arnfjörð Bjarmason, Oct 26, 2022
  45. 00/10 Makefile, docs & code: document & fix SHA-{1,256} selection behaviorÆvar Arnfjörð Bjarmason, Nov 7, 2022
  46. 01/10 Makefile: always (re)set DC_SHA1 on fallbackÆvar Arnfjörð Bjarmason, Nov 7, 2022
  47. 02/10 INSTALL: remove discussion of SHA-1 backendsÆvar Arnfjörð Bjarmason, Nov 7, 2022
  48. 03/10 Makefile: correct DC_SHA1 documentationÆvar Arnfjörð Bjarmason, Nov 7, 2022
  49. 06/10 Makefile: document default SHA-256 backendÆvar Arnfjörð Bjarmason, Nov 7, 2022
  50. 05/10 Makefile: rephrase the discussion of *_SHA1 knobsÆvar Arnfjörð Bjarmason, Nov 7, 2022
  51. 07/10 Makefile: document SHA-1 and SHA-256 default and selection orderÆvar Arnfjörð Bjarmason, Nov 7, 2022
  52. 04/10 Makefile: create and use sections for "define" flag listingÆvar Arnfjörð Bjarmason, Nov 7, 2022
  53. 08/10 Makefile & test-tool: replace "DC_SHA1" variable with a "define"Ævar Arnfjörð Bjarmason, Nov 7, 2022
  54. 09/10 Makefile: document default SHA-1 backend on OSXÆvar Arnfjörð Bjarmason, Nov 7, 2022
  55. 10/10 Makefile: discuss SHAttered in *_SHA{1,256} discussionÆvar Arnfjörð Bjarmason, Nov 7, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.