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

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