threads / patch / 33867

v8, 3 partsBegin replacing OpenSSL with CommonCrypto

Subject: [PATCH v8 0/3] Begin replacing OpenSSL with CommonCrypto

## tl;dr

8 messages between May 19, 2013 and May 21, 2013. Diffs are folded; open one to read it.

replies: 7people: 5as markdown or json

Eric Sunshine· May 19, 2013, 10:23 UTC · lore

This is a re-roll of David Aguilar's patch series [1] which eliminates some of the OpenSSL deprecation warnings on Mac OS X.

Changes since v7:
- Avoid double-negation (#ifndef NO_APPLE_COMMON_CRYPTO)
- Don't break imap-send.c for platforms other than Apple
[1]: http://thread.gmane.org/gmane.comp.version-control.git/224744
David Aguilar (3):
  Makefile: add support for Apple CommonCrypto facility
  cache.h: eliminate SHA-1 deprecation warnings on Mac OS X
  imap-send: eliminate HMAC deprecation warnings on Mac OS X
 Makefile    | 14 ++++++++++++++
 imap-send.c | 10 ++++++++++
 2 files changed, 24 insertions(+)
-- 
1.8.2.3
Eric Sunshine· May 19, 2013, 10:23 UTC · re: Eric Sunshine · lore

[PATCH v8 1/3] Makefile: add support for Apple CommonCrypto facility

From: David Aguilar <davvid@gmail.com>

As of Mac OS X 10.7, Apple deprecated all OpenSSL functions due to OpenSSL ABI instability, thus leading to build warnings. As a replacement, Apple encourages developers to migrate to its own (stable) CommonCrypto facility.

Introduce boilerplate which controls whether Apple's CommonCrypto facility is employed (enabled by default). Also add a NO_APPLE_COMMON_CRYPTO build flag with which the user can opt out to use OpenSSL instead.

[es: extracted CommonCrypto-related Makefile boilerplate into separate introductory patch]

Signed-off-by: David Aguilar <davvid@gmail.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 Makefile | 8 ++++++++
 1 file changed, 8 insertions(+)
Show changes to Makefile +8 −0
diff --git a/Makefile b/Makefile
index f698c1a..cd24c94 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,10 @@ ifeq ($(uname_S),Darwin)
 			BASIC_LDFLAGS += -L/opt/local/lib
 		endif
 	endif
+	ifndef NO_APPLE_COMMON_CRYPTO
+		APPLE_COMMON_CRYPTO = YesPlease
+		COMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO
+	endif
 	NO_REGEX = YesPlease
 	PTHREAD_LIBS =
 endif

-- 
1.8.2.3
Eric Sunshine· May 19, 2013, 10:23 UTC · re: Eric Sunshine · lore

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

From: David Aguilar <davvid@gmail.com>

As of Mac OS X 10.7, Apple deprecated all OpenSSL functions due to OpenSSL ABI instability, thus leading to build diagnostics such as:

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

Silence the warnings by using Apple's CommonCrypto SHA-1 replacement functions for SHA1_Init(), SHA1_Update(), and SHA1_Final().

COMMON_DIGEST_FOR_OPENSSL is defined to instruct <CommonCrypto/CommonDigest.h> to provide compatibility macros associating OpenSSL SHA-1 functions with their CommonCrypto counterparts.

[es: reworded commit message]
Signed-off-by: David Aguilar <davvid@gmail.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 Makefile | 6 ++++++
 1 file changed, 6 insertions(+)
Show changes to Makefile +6 −0
diff --git a/Makefile b/Makefile
index cd24c94..5e7cadf 100644
--- a/Makefile
+++ b/Makefile
@@ -1397,10 +1397,16 @@ ifdef PPC_SHA1
 	LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
 	LIB_H += ppc/sha1.h
 else
+ifdef APPLE_COMMON_CRYPTO
+	COMPAT_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.2.3
Eric Sunshine· May 19, 2013, 10:23 UTC · re: Eric Sunshine · lore

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

From: David Aguilar <davvid@gmail.com>

As of Mac OS X 10.7, Apple deprecated all OpenSSL functions due to OpenSSL ABI instability. Silence the warnings by using Apple's CommonCrypto HMAC replacement functions.

[es: reworded commit message; check APPLE_COMMON_CRYPTO instead of abusing COMMON_DIGEST_FOR_OPENSSL]

Signed-off-by: David Aguilar <davvid@gmail.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
Junio Hamano writes:
> Doesn't this mean people on platforms that do not care what Apple
> does have to define NO_APPLE_COMMON_CRYPTO?
It certainly does break other platforms. Thanks for catching.
> How about stopping this double-negation and do it more like this?
Double-negation begone.
 imap-send.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
Show changes to imap-send.c +10 −0
diff --git a/imap-send.c b/imap-send.c
index d9bcfb4..8ea180f 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -29,8 +29,18 @@
 #ifdef NO_OPENSSL
 typedef void *SSL;
 #else
+#ifdef APPLE_COMMON_CRYPTO
+#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.2.3
Torsten Bögershausen· May 21, 2013, 19:19 UTC · re: Junio C Hamano · lore

Re: [PATCH v8 0/3] Begin replacing OpenSSL with CommonCrypto

On 2013-05-21 00.52, Junio C Hamano wrote:
> Thanks, will replace da/darwin with this round.

(May be late response, not sure if this is the right email thread. I eventually managed to compile under 10.6, what we have on pu)

One minor nit, or 2: imap-send.c: In function ‘cram’: imap-send.c:913: warning: statement with no effect

This fixes it:
Show changes to imap-send.c +1 −1
diff --git a/imap-send.c b/imap-send.c
index 8ea180f..11577c9 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -35,7 +35,7 @@ typedef void *SSL;
#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 HMAC_CTX_cleanup(c)
#define EVP_md5() kCCHmacAlgMD5
#else
#include <openssl/evp.h>


(And I think there are more minor nits:
#define HMAC_Final(hmac, hash, ptr) CCHmacFinal(hmac, hash)
could be written as
#define HMAC_Final(hmac, hash, ptr) CCHmacFinal((hmac), (hash))
(Use paranthese around each parameter)
Similar change for HMAC_Init()

/Torsten
Jonathan Nieder· May 21, 2013, 19:36 UTC · re: Torsten Bögershausen · lore

Re: [PATCH v8 0/3] Begin replacing OpenSSL with CommonCrypto

Torsten Bögershausen wrote:
Show 19 quoted lines
> One minor nit, or 2:
> imap-send.c: In function ‘cram’:
> imap-send.c:913: warning: statement with no effect
>
> This fixes it:
>
> diff --git a/imap-send.c b/imap-send.c
> index 8ea180f..11577c9 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -35,7 +35,7 @@ typedef void *SSL;
> #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 HMAC_CTX_cleanup(c)
> #define EVP_md5() kCCHmacAlgMD5
> #else
> #include <openssl/evp.h>
Good catch.  Thanks.
Show 6 quoted lines
> (And I think there are more minor nits:
> #define HMAC_Final(hmac, hash, ptr) CCHmacFinal(hmac, hash)
> could be written as
> #define HMAC_Final(hmac, hash, ptr) CCHmacFinal((hmac), (hash))
> (Use paranthese around each parameter)
> Similar change for HMAC_Init()

Not needed --- the comma operator has lower precedence than any other, and any expression containing commas would have to already be surrounded by parentheses to be an argument to this function-like macro.

Jonathan
David Aguilar· May 21, 2013, 22:39 UTC · re: Torsten Bögershausen · lore

Re: [PATCH v8 0/3] Begin replacing OpenSSL with CommonCrypto

On Tue, May 21, 2013 at 12:19 PM, Torsten Bögershausen <tboegi@web.de> wrote:
Show 24 quoted lines
> On 2013-05-21 00.52, Junio C Hamano wrote:
>> Thanks, will replace da/darwin with this round.
> (May be late response, not sure if this is the right email thread.
> I eventually managed to compile under 10.6, what we have on pu)
>
> One minor nit, or 2:
> imap-send.c: In function ‘cram’:
> imap-send.c:913: warning: statement with no effect
>
> This fixes it:
>
> diff --git a/imap-send.c b/imap-send.c
> index 8ea180f..11577c9 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -35,7 +35,7 @@ typedef void *SSL;
> #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 HMAC_CTX_cleanup(c)
> #define EVP_md5() kCCHmacAlgMD5
> #else
> #include <openssl/evp.h>

Thanks. This change compiles fine on Mountain Lion (10.8) as well. -- David

← back to recent threads