[PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X

Subsystems: kernel build + files below scripts/ (unless maintained elsewhere), the rest

STALE3721d

6 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X

From: David Aguilar <hidden>
Date: 2016-06-15 22:57:15

Mac OS X 10.8 Mountain Lion prints warnings when building git:

	warning: 'SHA1_Init' is deprecated
	(declared at /usr/include/openssl/sha.h:121)

Silence the warnings by using the CommonCrytpo SHA-1
functions for SHA1_Init(), SHA1_Update(), and SHA1_Final().

COMMON_DIGEST_FOR_OPENSSL is defined to enable the OpenSSL
compatibility macros in CommonDigest.h.

Add a NO_APPLE_COMMON_CRYPTO option to the Makefile to allow
users to opt out of using this library.  When defined, Git will
use OpenSSL instead.

Helped-by: Eric Sunshine [off-list ref]
Signed-off-by: David Aguilar <redacted>
---
Both of these are replacement patches "pu".

Changes from last time:

It now uses a single APPLE_COMMON_CRYPTO definition.
Users can now opt-out by setting NO_APPLE_COMMON_CRYPTO.

 Makefile | 13 +++++++++++++
 1 file changed, 13 insertions(+)
diff --git a/Makefile b/Makefile
index f698c1a..8309c41 100644
--- a/Makefile
+++ b/Makefile
@@ -137,6 +137,10 @@ all::
 # specify your own (or DarwinPort's) include directories and
 # library directories by defining CFLAGS and LDFLAGS appropriately.
 #
+# Define NO_APPLE_COMMON_CRYPTO if you are building on Darwin/Mac OS X
+# 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.
 #
@@ -1054,6 +1058,9 @@ ifeq ($(uname_S),Darwin)
 			BASIC_LDFLAGS += -L/opt/local/lib
 		endif
 	endif
+	ifndef NO_APPLE_COMMON_CRYPTO
+		APPLE_COMMON_CRYPTO = YesPlease
+	endif
 	NO_REGEX = YesPlease
 	PTHREAD_LIBS =
 endif
@@ -1389,10 +1396,16 @@ ifdef PPC_SHA1
 	LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
 	LIB_H += ppc/sha1.h
 else
+ifdef APPLE_COMMON_CRYPTO
+	BASIC_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
+	SHA1_HEADER = <CommonCrypto/CommonDigest.h>
+else
 	SHA1_HEADER = <openssl/sha.h>
 	EXTLIBS += $(LIB_4_CRYPTO)
 endif
 endif
+endif
+
 ifdef NO_PERL_MAKEMAKER
 	export NO_PERL_MAKEMAKER
 endif
-- 
1.8.3.rc2.2.gbc955d1

[PATCH v5 2/2] imap-send: eliminate HMAC deprecation warnings on Mac OS X

From: David Aguilar <hidden>
Date: 2016-06-15 22:57:15

Mac OS X 10.8 Mountain Lion warns that HMAC_Init() and friends
are deprecated.  Detect the COMMON_CRYPTO_FOR_OPENSSL definition
and use CommonCrypto's HMAC functions to eliminate the warnings.

Signed-off-by: David Aguilar <redacted>
---
Changes since last time:

This version re-uses the existing COMMON_CRYPTO_FOR_OPENSSL define
instead of tweaking the Makefile to add a new one, so it's simpler.

My previous patch had Jonathan's reviewed-by tag, but he hasn't
reviewed this exact patch, so I removed it.  The C macros are unchanged.

 imap-send.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
diff --git a/imap-send.c b/imap-send.c
index d9bcfb4..96012b1 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -29,8 +29,18 @@
 #ifdef NO_OPENSSL
 typedef void *SSL;
 #else
+#ifdef COMMON_DIGEST_FOR_OPENSSL
+#include <CommonCrypto/CommonHMAC.h>
+#define HMAC_CTX CCHmacContext
+#define HMAC_Init(hmac, key, len, algo) CCHmacInit(hmac, algo, key, len)
+#define HMAC_Update CCHmacUpdate
+#define HMAC_Final(hmac, hash, ptr) CCHmacFinal(hmac, hash)
+#define HMAC_CTX_cleanup
+#define EVP_md5() kCCHmacAlgMD5
+#else
 #include <openssl/evp.h>
 #include <openssl/hmac.h>
+#endif
 #include <openssl/x509v3.h>
 #endif
 
-- 
1.8.3.rc2.2.gbc955d1

Re: [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X

From: Torsten Bögershausen <hidden>
Date: 2016-06-15 22:57:16

On 2013-05-15 09.11, David Aguilar wrote:
quoted hunk
Mac OS X 10.8 Mountain Lion prints warnings when building git:

	warning: 'SHA1_Init' is deprecated
	(declared at /usr/include/openssl/sha.h:121)

Silence the warnings by using the CommonCrytpo SHA-1
functions for SHA1_Init(), SHA1_Update(), and SHA1_Final().

COMMON_DIGEST_FOR_OPENSSL is defined to enable the OpenSSL
compatibility macros in CommonDigest.h.

Add a NO_APPLE_COMMON_CRYPTO option to the Makefile to allow
users to opt out of using this library.  When defined, Git will
use OpenSSL instead.

Helped-by: Eric Sunshine [off-list ref]
Signed-off-by: David Aguilar <redacted>
---
Both of these are replacement patches "pu".

Changes from last time:

It now uses a single APPLE_COMMON_CRYPTO definition.
Users can now opt-out by setting NO_APPLE_COMMON_CRYPTO.

 Makefile | 13 +++++++++++++
 1 file changed, 13 insertions(+)
diff --git a/Makefile b/Makefile
index f698c1a..8309c41 100644
--- a/Makefile
+++ b/Makefile
@@ -137,6 +137,10 @@ all::
 # specify your own (or DarwinPort's) include directories and
 # library directories by defining CFLAGS and LDFLAGS appropriately.
 #
+# Define NO_APPLE_COMMON_CRYPTO if you are building on Darwin/Mac OS X
+# 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.
 #
@@ -1054,6 +1058,9 @@ ifeq ($(uname_S),Darwin)
 			BASIC_LDFLAGS += -L/opt/local/lib
 		endif
 	endif
+	ifndef NO_APPLE_COMMON_CRYPTO
+		APPLE_COMMON_CRYPTO = YesPlease
+	endif
 	NO_REGEX = YesPlease
 	PTHREAD_LIBS =
 endif
@@ -1389,10 +1396,16 @@ ifdef PPC_SHA1
 	LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
 	LIB_H += ppc/sha1.h
 else
+ifdef APPLE_COMMON_CRYPTO
+	BASIC_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
+	SHA1_HEADER = <CommonCrypto/CommonDigest.h>
Would it make sense to replace APPLE_COMMON_CRYPTO
with COMMON_DIGEST_FOR_OPENSSL ?

In the spirit of other Makefile-defines becoming Compiler defines,
a random picked example:
ifdef NO_STRTOULL
	COMPAT_CFLAGS += -DNO_STRTOULL
endif

/Torsten

Re: [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X

From: Eric Sunshine <hidden>
Date: 2016-06-15 22:57:18

On Wed, May 15, 2013 at 1:56 PM, Torsten Bögershausen [off-list ref] wrote:
On 2013-05-15 09.11, David Aguilar wrote:
quoted
+     ifndef NO_APPLE_COMMON_CRYPTO
+             APPLE_COMMON_CRYPTO = YesPlease
+     endif
      NO_REGEX = YesPlease
      PTHREAD_LIBS =
 endif
@@ -1389,10 +1396,16 @@ ifdef PPC_SHA1
      LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
      LIB_H += ppc/sha1.h
 else
+ifdef APPLE_COMMON_CRYPTO
+     BASIC_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
+     SHA1_HEADER = <CommonCrypto/CommonDigest.h>
Would it make sense to replace APPLE_COMMON_CRYPTO
with COMMON_DIGEST_FOR_OPENSSL ?

In the spirit of other Makefile-defines becoming Compiler defines,
a random picked example:
ifdef NO_STRTOULL
        COMPAT_CFLAGS += -DNO_STRTOULL
endif
Not necessarily. Unlike NO_STRTOULL and cousins,
COMMON_DIGEST_FOR_OPENSSL is not a Git build tweak; it is merely a
(public) implementation detail of the Apple header [1] to magically
associate OpenSSL digest functions with CommonCrypto counterparts.
It's not the only such macro recognized by the Apple headers. For
instance, COMMON_DIGEST_FOR_RFC_1321 magically associates legacy MD5
digest functions with CommonCrypto counterparts.

Further, as Junio noted elsewhere, David is using CommonCrypto for
HMAC replacements, not just for digest replacements, so a Makefile
knob with DIGEST in its name is not really appropriate. More
generally, David would like to find CommonCrypto replacements for all
the OpenSSL functionality, so a Makefile knob named after DIGEST is
too specific.

These considerations motivated the original suggestion for a single
Git Makefile knob to enable/disable, as a unit, all CommonCrypto
replacements. Such a knob would naturally have COMMON_CRYPTO as part
of its name.

[1]: /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.8.sdk/usr/include/CommonCrypto/CommonDigest.h

-- ES

Re: [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X

From: David Aguilar <hidden>
Date: 2016-06-15 22:57:18

On Thu, May 16, 2013 at 11:18 PM, Eric Sunshine [off-list ref] wrote:
On Wed, May 15, 2013 at 1:56 PM, Torsten Bögershausen [off-list ref] wrote:
quoted
On 2013-05-15 09.11, David Aguilar wrote:
quoted
+     ifndef NO_APPLE_COMMON_CRYPTO
+             APPLE_COMMON_CRYPTO = YesPlease
+     endif
      NO_REGEX = YesPlease
      PTHREAD_LIBS =
 endif
@@ -1389,10 +1396,16 @@ ifdef PPC_SHA1
      LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
      LIB_H += ppc/sha1.h
 else
+ifdef APPLE_COMMON_CRYPTO
+     BASIC_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
+     SHA1_HEADER = <CommonCrypto/CommonDigest.h>
Would it make sense to replace APPLE_COMMON_CRYPTO
with COMMON_DIGEST_FOR_OPENSSL ?

In the spirit of other Makefile-defines becoming Compiler defines,
a random picked example:
ifdef NO_STRTOULL
        COMPAT_CFLAGS += -DNO_STRTOULL
endif
Not necessarily. Unlike NO_STRTOULL and cousins,
COMMON_DIGEST_FOR_OPENSSL is not a Git build tweak; it is merely a
(public) implementation detail of the Apple header [1] to magically
associate OpenSSL digest functions with CommonCrypto counterparts.
It's not the only such macro recognized by the Apple headers. For
instance, COMMON_DIGEST_FOR_RFC_1321 magically associates legacy MD5
digest functions with CommonCrypto counterparts.

Further, as Junio noted elsewhere, David is using CommonCrypto for
HMAC replacements, not just for digest replacements, so a Makefile
knob with DIGEST in its name is not really appropriate. More
generally, David would like to find CommonCrypto replacements for all
the OpenSSL functionality, so a Makefile knob named after DIGEST is
too specific.

These considerations motivated the original suggestion for a single
Git Makefile knob to enable/disable, as a unit, all CommonCrypto
replacements. Such a knob would naturally have COMMON_CRYPTO as part
of its name.

[1]: /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.8.sdk/usr/include/CommonCrypto/CommonDigest.h
This is a nice justification for taking v5 of this series over v6.

Sorry for all the churn in this series, Junio.  I wrote v5 so I
certainly felt it was a good idea at the time, and I feel bad for not
having waited longer before sending out v6 (which is what was
eventually queued in "pu").

Do you have advice on how we should proceed?  :sigh: sorry for wasting
so much maintainer time on this series already.  If you need any
resends or anything please let me know.  This time I'll wait for a
strong opinion before firing off patches.

My opinion: yeah, v5's ([NO_]APPLE_COMMON_CRYPTO) was probably where
we should have stopped painting.  Hindsight is 20/20.  Luckily it
never left "pu".
--
David

Re: [PATCH v5 1/2] cache.h: eliminate SHA-1 deprecation warnings on Mac OS X

From: Eric Sunshine <hidden>
Date: 2016-06-15 22:57:18

On Fri, May 17, 2013 at 4:21 AM, David Aguilar [off-list ref] wrote:
On Thu, May 16, 2013 at 11:18 PM, Eric Sunshine [off-list ref] wrote:
quoted
On Wed, May 15, 2013 at 1:56 PM, Torsten Bögershausen [off-list ref] wrote:
quoted
On 2013-05-15 09.11, David Aguilar wrote:
quoted
+     ifndef NO_APPLE_COMMON_CRYPTO
+             APPLE_COMMON_CRYPTO = YesPlease
+     endif
      NO_REGEX = YesPlease
      PTHREAD_LIBS =
 endif
@@ -1389,10 +1396,16 @@ ifdef PPC_SHA1
      LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
      LIB_H += ppc/sha1.h
 else
+ifdef APPLE_COMMON_CRYPTO
+     BASIC_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
+     SHA1_HEADER = <CommonCrypto/CommonDigest.h>
Would it make sense to replace APPLE_COMMON_CRYPTO
with COMMON_DIGEST_FOR_OPENSSL ?

In the spirit of other Makefile-defines becoming Compiler defines,
a random picked example:
ifdef NO_STRTOULL
        COMPAT_CFLAGS += -DNO_STRTOULL
endif
Not necessarily. Unlike NO_STRTOULL and cousins,
COMMON_DIGEST_FOR_OPENSSL is not a Git build tweak; it is merely a
(public) implementation detail of the Apple header [1] to magically
associate OpenSSL digest functions with CommonCrypto counterparts.
It's not the only such macro recognized by the Apple headers. For
instance, COMMON_DIGEST_FOR_RFC_1321 magically associates legacy MD5
digest functions with CommonCrypto counterparts.

Further, as Junio noted elsewhere, David is using CommonCrypto for
HMAC replacements, not just for digest replacements, so a Makefile
knob with DIGEST in its name is not really appropriate. More
generally, David would like to find CommonCrypto replacements for all
the OpenSSL functionality, so a Makefile knob named after DIGEST is
too specific.

These considerations motivated the original suggestion for a single
Git Makefile knob to enable/disable, as a unit, all CommonCrypto
replacements. Such a knob would naturally have COMMON_CRYPTO as part
of its name.
This is a nice justification for taking v5 of this series over v6.
You will consider this bike-shedding (I don't), but the above also is
good justification for revising your HMAC patch to _not_ rely on
COMMON_DIGEST_FOR_OPENSSL, which is an implementation detail of your
SHA patch, rather than a proper build knob.

Similar to NO_STRTOULL and cousins, you should have a #define (such as
NO_APPLE_COMMON_CRYPTO or NO_COMMON_CRYPTO) which is consulted by your
HMAC patch and any future patches you submit to map CommonCrypto
counterparts to OpenSSL functions. The fact that you also must #define
COMMON_DIGEST_FOR_OPENSSL for the SHA patch is just an implementation
detail of that one patch; it is not relevant to the other patches.

-- ES
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help