threads / patch / 34553

patchimap-send: use Apple's Security framework for base64 encoding

Subject: [PATCH] imap-send: use Apple's Security framework for base64 encoding

## tl;dr

5 messages between Jul 27, 2013 and Jul 29, 2013. Diffs are folded; open one to read it.

replies: 4people: 4as markdown or json

David Aguilar· Jul 27, 2013, 20:31 UTC · lore
From: Jeremy Huddleston <jeremyhu@apple.com>

Use Apple's supported functions for base64 encoding instead of the deprecated OpenSSL functions.

Signed-off-by: Jeremy Huddleston <jeremyhu@apple.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
This is Jeremy's original patch rebased onto the latest master.

Jeremy, the only way I could get this to work was to suppress inclusion of openssl/sha.h by defining HEADER_SHA_H. This can be removed when we have replacements for openssl/x509v3.h.

 Makefile    |  1 +
 imap-send.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 82 insertions(+), 4 deletions(-)
Show changes to 2 files +82 −4

Makefile, imap-send.c

diff --git a/Makefile b/Makefile
index 0600eb4..4c40665 100644
--- a/Makefile
+++ b/Makefile
@@ -1413,6 +1413,7 @@ ifdef PPC_SHA1
 	LIB_H += ppc/sha1.h
 else
 ifdef APPLE_COMMON_CRYPTO
+	LIB_4_CRYPTO += -framework Security -framework CoreFoundation
 	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
 	SHA1_HEADER = <CommonCrypto/CommonDigest.h>
 else
diff --git a/imap-send.c b/imap-send.c
index d6b65e2..3fd9c0e 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -22,14 +22,11 @@
  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
-#include "cache.h"
-#include "exec_cmd.h"
-#include "run-command.h"
-#include "prompt.h"
 #ifdef NO_OPENSSL
 typedef void *SSL;
 #else
 #ifdef APPLE_COMMON_CRYPTO
+/* git-compat-util.h overwrites ctype.h; this must be included first */
 #include <CommonCrypto/CommonHMAC.h>
 #define HMAC_CTX CCHmacContext
 #define HMAC_Init(hmac, key, len, algo) CCHmacInit(hmac, algo, key, len)
@@ -37,12 +34,23 @@ typedef void *SSL;
 #define HMAC_Final(hmac, hash, ptr) CCHmacFinal(hmac, hash)
 #define HMAC_CTX_cleanup(ignore)
 #define EVP_md5() kCCHmacAlgMD5
+
+#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 1070
+#define APPLE_LION_OR_NEWER
+#include <Security/Security.h>
+#define HEADER_SHA_H /* suppress inclusion of openssl/sha.h */
+#endif
+
 #else
 #include <openssl/evp.h>
 #include <openssl/hmac.h>
 #endif
 #include <openssl/x509v3.h>
 #endif
+#include "cache.h"
+#include "exec_cmd.h"
+#include "run-command.h"
+#include "prompt.h"
 
 static const char imap_send_usage[] = "git imap-send < <mbox>";
 
@@ -877,6 +885,75 @@ static void imap_close_store(struct imap_store *ctx)
 	free(ctx);
 }
 
+#ifdef APPLE_LION_OR_NEWER
+#define EVP_DecodeBlock git_CC_EVP_DecodeBlock
+#define EVP_EncodeBlock git_CC_EVP_EncodeBlock
+#define error_check(pattern, err) \
+	do { \
+		if (err) { \
+			die(pattern, (long)CFErrorGetCode(err)); \
+		} \
+	} while(0)
+
+static int git_CC_EVP_EncodeBlock(unsigned char *out,
+		const unsigned char *in, int inlen)
+{
+	CFErrorRef err;
+	SecTransformRef encoder;
+	CFDataRef input, output;
+	CFIndex length;
+
+	encoder = SecEncodeTransformCreate(kSecBase64Encoding, &err);
+	error_check("SecEncodeTransformCreate failed: %ld", err);
+
+	input = CFDataCreate(kCFAllocatorDefault, in, inlen);
+	SecTransformSetAttribute(encoder, kSecTransformInputAttributeName,
+			input, &err);
+	error_check("SecTransformSetAttribute failed: %ld", err);
+
+	output = SecTransformExecute(encoder, &err);
+	error_check("SecTransformExecute failed: %ld", err);
+
+	length = CFDataGetLength(output);
+	CFDataGetBytes(output, CFRangeMake(0, length), out);
+
+	CFRelease(output);
+	CFRelease(input);
+	CFRelease(encoder);
+
+	return (int)strlen((const char *)out);
+}
+
+static int git_CC_EVP_DecodeBlock(unsigned char *out,
+		const unsigned char *in, int inlen)
+{
+	CFErrorRef err;
+	SecTransformRef decoder;
+	CFDataRef input, output;
+	CFIndex length;
+
+	decoder = SecDecodeTransformCreate(kSecBase64Encoding, &err);
+	error_check("SecEncodeTransformCreate failed: %ld", err);
+
+	input = CFDataCreate(kCFAllocatorDefault, in, inlen);
+	SecTransformSetAttribute(decoder, kSecTransformInputAttributeName,
+			input, &err);
+	error_check("SecTransformSetAttribute failed: %ld", err);
+
+	output = SecTransformExecute(decoder, &err);
+	error_check("SecTransformExecute failed: %ld", err);
+
+	length = CFDataGetLength(output);
+	CFDataGetBytes(output, CFRangeMake(0, length), out);
+
+	CFRelease(output);
+	CFRelease(input);
+	CFRelease(decoder);
+
+	return (int)strlen((const char *)out);
+}
+#endif /* APPLE_LION_OR_NEWER */
+
 #ifndef NO_OPENSSL
 
 /*
-- 
1.8.3.2.804.g0da7a53.dirty
Jeremy Huddleston Sequoia· Jul 27, 2013, 23:28 UTC · re: David Aguilar · lore

Re: [PATCH] imap-send: use Apple's Security framework for base64 encoding

Hi David,
Thanks for massaging it to apply to master and cleaning up the style conflicts.
On Jul 27, 2013, at 13:31, David Aguilar <davvid@gmail.com> wrote:
Show 153 quoted lines
> From: Jeremy Huddleston <jeremyhu@apple.com>
> 
> Use Apple's supported functions for base64 encoding instead
> of the deprecated OpenSSL functions.
> 
> Signed-off-by: Jeremy Huddleston <jeremyhu@apple.com>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
> This is Jeremy's original patch rebased onto the latest master.
> 
> Jeremy, the only way I could get this to work was to suppress inclusion of
> openssl/sha.h by defining HEADER_SHA_H.  This can be removed when we have
> replacements for openssl/x509v3.h.
> 
> Makefile    |  1 +
> imap-send.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 82 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 0600eb4..4c40665 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1413,6 +1413,7 @@ ifdef PPC_SHA1
> 	LIB_H += ppc/sha1.h
> else
> ifdef APPLE_COMMON_CRYPTO
> +	LIB_4_CRYPTO += -framework Security -framework CoreFoundation
> 	COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
> 	SHA1_HEADER = <CommonCrypto/CommonDigest.h>
> else
> diff --git a/imap-send.c b/imap-send.c
> index d6b65e2..3fd9c0e 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -22,14 +22,11 @@
>  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>  */
> 
> -#include "cache.h"
> -#include "exec_cmd.h"
> -#include "run-command.h"
> -#include "prompt.h"
> #ifdef NO_OPENSSL
> typedef void *SSL;
> #else
> #ifdef APPLE_COMMON_CRYPTO
> +/* git-compat-util.h overwrites ctype.h; this must be included first */
> #include <CommonCrypto/CommonHMAC.h>
> #define HMAC_CTX CCHmacContext
> #define HMAC_Init(hmac, key, len, algo) CCHmacInit(hmac, algo, key, len)
> @@ -37,12 +34,23 @@ typedef void *SSL;
> #define HMAC_Final(hmac, hash, ptr) CCHmacFinal(hmac, hash)
> #define HMAC_CTX_cleanup(ignore)
> #define EVP_md5() kCCHmacAlgMD5
> +
> +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 1070
> +#define APPLE_LION_OR_NEWER
> +#include <Security/Security.h>
> +#define HEADER_SHA_H /* suppress inclusion of openssl/sha.h */
> +#endif
> +
> #else
> #include <openssl/evp.h>
> #include <openssl/hmac.h>
> #endif
> #include <openssl/x509v3.h>
> #endif
> +#include "cache.h"
> +#include "exec_cmd.h"
> +#include "run-command.h"
> +#include "prompt.h"
> 
> static const char imap_send_usage[] = "git imap-send < <mbox>";
> 
> @@ -877,6 +885,75 @@ static void imap_close_store(struct imap_store *ctx)
> 	free(ctx);
> }
> 
> +#ifdef APPLE_LION_OR_NEWER
> +#define EVP_DecodeBlock git_CC_EVP_DecodeBlock
> +#define EVP_EncodeBlock git_CC_EVP_EncodeBlock
> +#define error_check(pattern, err) \
> +	do { \
> +		if (err) { \
> +			die(pattern, (long)CFErrorGetCode(err)); \
> +		} \
> +	} while(0)
> +
> +static int git_CC_EVP_EncodeBlock(unsigned char *out,
> +		const unsigned char *in, int inlen)
> +{
> +	CFErrorRef err;
> +	SecTransformRef encoder;
> +	CFDataRef input, output;
> +	CFIndex length;
> +
> +	encoder = SecEncodeTransformCreate(kSecBase64Encoding, &err);
> +	error_check("SecEncodeTransformCreate failed: %ld", err);
> +
> +	input = CFDataCreate(kCFAllocatorDefault, in, inlen);
> +	SecTransformSetAttribute(encoder, kSecTransformInputAttributeName,
> +			input, &err);
> +	error_check("SecTransformSetAttribute failed: %ld", err);
> +
> +	output = SecTransformExecute(encoder, &err);
> +	error_check("SecTransformExecute failed: %ld", err);
> +
> +	length = CFDataGetLength(output);
> +	CFDataGetBytes(output, CFRangeMake(0, length), out);
> +
> +	CFRelease(output);
> +	CFRelease(input);
> +	CFRelease(encoder);
> +
> +	return (int)strlen((const char *)out);
> +}
> +
> +static int git_CC_EVP_DecodeBlock(unsigned char *out,
> +		const unsigned char *in, int inlen)
> +{
> +	CFErrorRef err;
> +	SecTransformRef decoder;
> +	CFDataRef input, output;
> +	CFIndex length;
> +
> +	decoder = SecDecodeTransformCreate(kSecBase64Encoding, &err);
> +	error_check("SecEncodeTransformCreate failed: %ld", err);
> +
> +	input = CFDataCreate(kCFAllocatorDefault, in, inlen);
> +	SecTransformSetAttribute(decoder, kSecTransformInputAttributeName,
> +			input, &err);
> +	error_check("SecTransformSetAttribute failed: %ld", err);
> +
> +	output = SecTransformExecute(decoder, &err);
> +	error_check("SecTransformExecute failed: %ld", err);
> +
> +	length = CFDataGetLength(output);
> +	CFDataGetBytes(output, CFRangeMake(0, length), out);
> +
> +	CFRelease(output);
> +	CFRelease(input);
> +	CFRelease(decoder);
> +
> +	return (int)strlen((const char *)out);
> +}
> +#endif /* APPLE_LION_OR_NEWER */
> +
> #ifndef NO_OPENSSL
> 
> /*
> -- 
> 1.8.3.2.804.g0da7a53.dirty
> 
Jonathan Nieder· Jul 29, 2013, 03:35 UTC · re: David Aguilar · lore

Re: [PATCH] imap-send: use Apple's Security framework for base64 encoding

Hi,
David Aguilar wrote:
Show 16 quoted lines
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -22,14 +22,11 @@
>   *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>   */
>  
> -#include "cache.h"
> -#include "exec_cmd.h"
> -#include "run-command.h"
> -#include "prompt.h"
>  #ifdef NO_OPENSSL
>  typedef void *SSL;
>  #else
>  #ifdef APPLE_COMMON_CRYPTO
> +/* git-compat-util.h overwrites ctype.h; this must be included first */
>  #include <CommonCrypto/CommonHMAC.h>
Thanks for your work on this.

Currently each translation unit of git includes git-compat-util.h or a header like cache.h that includes git-compat-util.h before doing anything else, since otherwise feature test macros are not set before the first system header is included.

The above (CommonCrypto needing to be included before some of the definitions from git-compat-util.h) suggests to me that CommonCrypto should just be included directly from git-compat-util.h in some appropriate place. That way any other header that needs CommonCrypto routines only has to include git-compat-util.h first as usual and doesn't have to worry about the order of other #includes. Could that work?

Thanks and hope that helps, Jonathan

David Aguilar· Jul 29, 2013, 07:23 UTC · re: Jonathan Nieder · lore

Re: [PATCH] imap-send: use Apple's Security framework for base64 encoding

On Sun, Jul 28, 2013 at 8:35 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 35 quoted lines
> Hi,
>
> David Aguilar wrote:
>
>> --- a/imap-send.c
>> +++ b/imap-send.c
>> @@ -22,14 +22,11 @@
>>   *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>>   */
>>
>> -#include "cache.h"
>> -#include "exec_cmd.h"
>> -#include "run-command.h"
>> -#include "prompt.h"
>>  #ifdef NO_OPENSSL
>>  typedef void *SSL;
>>  #else
>>  #ifdef APPLE_COMMON_CRYPTO
>> +/* git-compat-util.h overwrites ctype.h; this must be included first */
>>  #include <CommonCrypto/CommonHMAC.h>
>
> Thanks for your work on this.
>
> Currently each translation unit of git includes git-compat-util.h or a
> header like cache.h that includes git-compat-util.h before doing
> anything else, since otherwise feature test macros are not set before
> the first system header is included.
>
> The above (CommonCrypto needing to be included before some of the
> definitions from git-compat-util.h) suggests to me that CommonCrypto
> should just be included directly from git-compat-util.h in some
> appropriate place.  That way any other header that needs CommonCrypto
> routines only has to include git-compat-util.h first as usual and
> doesn't have to worry about the order of other #includes.  Could that
> work?

imap-send is currently the only user of this stuff; I believe this would pull in these library dependencies for all builtins.

If we don't mind a new file, something like git-compat-crypto.h could be a nice place to tuck all these #ifdefs away. If we had another command that needed these then it'd be a easier to justify a new file.

A test-crypto command in the test suite could be written to verify that the implementations work as advertised. That counts as "another command", albeit an internal one. I don't believe imap-send has any test coverage so pulling this stuff out would help make it more testable, which is a net win. Libifying might be a little premature, though.

Thoughts?
-- 
David
Junio C Hamano· Jul 29, 2013, 15:51 UTC · re: David Aguilar · lore

Re: [PATCH] imap-send: use Apple's Security framework for base64 encoding

David Aguilar <davvid@gmail.com> writes:
Show 17 quoted lines
> diff --git a/imap-send.c b/imap-send.c
> index d6b65e2..3fd9c0e 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -22,14 +22,11 @@
>   *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>   */
>  
> -#include "cache.h"
> -#include "exec_cmd.h"
> -#include "run-command.h"
> -#include "prompt.h"
>  #ifdef NO_OPENSSL
>  typedef void *SSL;
>  #else
>  #ifdef APPLE_COMMON_CRYPTO
> +/* git-compat-util.h overwrites ctype.h; this must be included first */

I do not like this part of the patch, for the exact reason Jonathan already mentioned. The concern this comment addresses is exactly the kind of thing git-compat-util.h was introduced for.

Could we keep the encapsulation of the logic to decide what to include and in what order in git-compat-util.h, please?

← back to recent threads