git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Mike Hommey" <mh@glandium.org>,
	"brian m . carlson" <sandals@crustytoothpaste.net>,
	"Carlo Marcelo Arenas Belón" <carenas@gmail.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Glen Choo" <chooglen@google.com>,
	"Eric DeCosta" <edecosta@mathworks.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v2] Makefile: use sha1collisiondetection by default on OSX and Darwin
Date: Thu, 15 Dec 2022 09:43:05 +0100	[thread overview]
Message-ID: <patch-v2-1.1-3de7cdbd260-20221215T084129Z-avarab@gmail.com> (raw)
In-Reply-To: <patch-1.1-1f4e39be97b-20221020T225305Z-avarab@gmail.com>

When the sha1collisiondetection library was added and made the default
in [1] the interaction with APPLE_COMMON_CRYPTO added in [2] and [3]
seems to have been missed. On modern OSX and Darwin we are able to use
Apple's CommonCrypto both for SHA-1, and as a generic (but partial)
OpenSSL replacement.

This left OSX and Darwin without protection against the SHAttered
attack when building Git in its default configuration.

Let's also use sha1collisiondetection on OSX, to do so we'll need to
split up the "APPLE_COMMON_CRYPTO" flag into that flag and a new
"APPLE_COMMON_CRYPTO_SHA1".

Because of this we can stop conflating whether we want to use Apple's
CommonCrypto at all, and whether we want to use it for SHA-1.  This
makes the CI recipe added in [4] simpler.

1. e6b07da2780 (Makefile: make DC_SHA1 the default, 2017-03-17)
2. 4dcd7732db0 (Makefile: add support for Apple CommonCrypto facility, 2013-05-19)
3. 61067954ce1 (cache.h: eliminate SHA-1 deprecation warnings on Mac OS X, 2013-05-19)
4. 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>
---
Range-diff against v1:
1:  1f4e39be97b ! 1:  3de7cdbd260 Makefile: use sha1collisiondetection by default on OSX and Darwin
    @@ Commit message
         split up the "APPLE_COMMON_CRYPTO" flag into that flag and a new
         "APPLE_COMMON_CRYPTO_SHA1".
     
    -    Let's also change the CI for "osx-clang" to test with the new
    -    APPLE_COMMON_CRYPTO_SHA1 knob ("osx-gcc" uses the new
    -    sha1collisiondetection default).
    -
    -    In practice this will spot issues like the one noted in [4], 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.
    +    Because of this we can stop conflating whether we want to use Apple's
    +    CommonCrypto at all, and whether we want to use it for SHA-1.  This
    +    makes the CI recipe added in [4] simpler.
     
         1. e6b07da2780 (Makefile: make DC_SHA1 the default, 2017-03-17)
         2. 4dcd7732db0 (Makefile: add support for Apple CommonCrypto facility, 2013-05-19)
         3. 61067954ce1 (cache.h: eliminate SHA-1 deprecation warnings on Mac OS X, 2013-05-19)
    -    4. 32205655dc7 (fsmonitor OSX: compile with DC_SHA1=YesPlease, 2022-10-19)
    +    4. 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: 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.
     +# Define APPLE_COMMON_CRYPTO_SHA1 to use Apple's CommonCrypto for
     +# SHA-1.
      #
    - # === 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
     @@ Makefile: ifdef NO_POSIX_GOODIES
      	BASIC_CFLAGS += -DNO_POSIX_GOODIES
      endif
    @@ Makefile: ifdef BLK_SHA1
      else
     
      ## ci/lib.sh ##
    -@@ ci/lib.sh: macos-latest)
    - esac
    - 
    - case "$jobname" in
    -+osx-clang)
    -+	MAKEFLAGS="$MAKEFLAGS APPLE_COMMON_CRYPTO_SHA1=Yes"
    -+	;;
    - linux32)
    - 	CC=gcc
    +@@ ci/lib.sh: macos-*)
    + 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"
    + 	else
    + 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python2)"
    +-		MAKEFLAGS="$MAKEFLAGS NO_APPLE_COMMON_CRYPTO=NoThanks"
    +-		MAKEFLAGS="$MAKEFLAGS NO_OPENSSL=NoThanks"
    ++		MAKEFLAGS="$MAKEFLAGS APPLE_COMMON_CRYPTO_SHA1=Yes"
    + 	fi
      	;;
    + esac

 Makefile  | 10 ++++------
 ci/lib.sh |  3 +--
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index 0f7d7ab1fd2..db447d07383 100644
--- a/Makefile
+++ b/Makefile
@@ -511,10 +511,8 @@ 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.
+# Define APPLE_COMMON_CRYPTO_SHA1 to use Apple's CommonCrypto for
+# SHA-1.
 #
 # If don't enable any of the *_SHA1 settings in this section, Git will
 # default to its built-in sha1collisiondetection library, which is a
@@ -1911,7 +1909,7 @@ ifdef NO_POSIX_GOODIES
 	BASIC_CFLAGS += -DNO_POSIX_GOODIES
 endif
 
-ifdef APPLE_COMMON_CRYPTO
+ifdef APPLE_COMMON_CRYPTO_SHA1
 	# Apple CommonCrypto requires chunking
 	SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
 endif
@@ -1928,7 +1926,7 @@ ifdef BLK_SHA1
 	LIB_OBJS += block-sha1/sha1.o
 	BASIC_CFLAGS += -DSHA1_BLK
 else
-ifdef APPLE_COMMON_CRYPTO
+ifdef APPLE_COMMON_CRYPTO_SHA1
 	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
 	BASIC_CFLAGS += -DSHA1_APPLE
 else
diff --git a/ci/lib.sh b/ci/lib.sh
index 706e3ba7e93..db7105e8a8d 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -258,8 +258,7 @@ macos-*)
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"
 	else
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python2)"
-		MAKEFLAGS="$MAKEFLAGS NO_APPLE_COMMON_CRYPTO=NoThanks"
-		MAKEFLAGS="$MAKEFLAGS NO_OPENSSL=NoThanks"
+		MAKEFLAGS="$MAKEFLAGS APPLE_COMMON_CRYPTO_SHA1=Yes"
 	fi
 	;;
 esac
-- 
2.39.0.rc2.1048.g0e5493b8d5b


  parent reply	other threads:[~2022-12-15  8:43 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-20 23:01 [PATCH] Makefile: use sha1collisiondetection by default on OSX and Darwin Ævar Arnfjörð Bjarmason
2022-10-20 23:26 ` Junio C Hamano
2022-10-20 23:52   ` Ævar Arnfjörð Bjarmason
2022-12-15  8:43 ` Ævar Arnfjörð Bjarmason [this message]
2022-12-15 21:17   ` [PATCH v2] " Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=patch-v2-1.1-3de7cdbd260-20221215T084129Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=carenas@gmail.com \
    --cc=chooglen@google.com \
    --cc=edecosta@mathworks.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mh@glandium.org \
    --cc=sandals@crustytoothpaste.net \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).