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

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

[David Aguilar]

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

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

[Jeremy Huddleston Sequoia]

[-- Attachment #1: Type: text/plain, Size: 4890 bytes --]

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:

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


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4136 bytes --]

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

[Jonathan Nieder]

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?

Thanks and hope that helps,
Jonathan

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

[David Aguilar]

On Sun, Jul 28, 2013 at 8:35 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> 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

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

[Junio C Hamano]

David Aguilar <davvid@gmail.com> writes:

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