All of lore.kernel.org
 help / color / mirror / 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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.