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

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

[Eric Sunshine]

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

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

[Eric Sunshine]

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(+)

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

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

[Eric Sunshine]

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(+)

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

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

[Eric Sunshine]

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(+)

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

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

[Junio C Hamano]

Thanks, will replace da/darwin with this round.

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

[Torsten Bögershausen]

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>


(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

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

[Jonathan Nieder]

Torsten Bögershausen wrote:

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

> (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

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

[David Aguilar]

On Tue, May 21, 2013 at 12:19 PM, Torsten Bögershausen <tboegi@web.de> wrote:
> 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