{"thread":{"id":"34553","subject":"[PATCH] imap-send: use Apple's Security framework for base64 encoding","startedAt":"2013-07-27T20:31:32Z","lastAt":"2013-07-29T15:51:26Z","messageCount":5,"participants":["David Aguilar","Jeremy Huddleston Sequoia","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"224190","messageId":"1374957092-41505-1-git-send-email-davvid@gmail.com","threadId":"34553","inReplyTo":null,"subject":"[PATCH] imap-send: use Apple's Security framework for base64 encoding","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2013-07-27T20:31:32Z","receivedAt":"2013-07-27T20:31:32Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"From: Jeremy Huddleston <jeremyhu@apple.com>\n\nUse Apple's supported functions for base64 encoding instead\nof the deprecated OpenSSL functions.\n\nSigned-off-by: Jeremy Huddleston <jeremyhu@apple.com>\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\nThis is Jeremy's original patch rebased onto the latest master.\n\nJeremy, the only way I could get this to work was to suppress inclusion of\nopenssl/sha.h by defining HEADER_SHA_H.  This can be removed when we have\nreplacements for openssl/x509v3.h.\n\n Makefile    |  1 +\n imap-send.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---\n 2 files changed, 82 insertions(+), 4 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 0600eb4..4c40665 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1413,6 +1413,7 @@ ifdef PPC_SHA1\n \tLIB_H += ppc/sha1.h\n else\n ifdef APPLE_COMMON_CRYPTO\n+\tLIB_4_CRYPTO += -framework Security -framework CoreFoundation\n \tCOMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL\n \tSHA1_HEADER = <CommonCrypto/CommonDigest.h>\n else\ndiff --git a/imap-send.c b/imap-send.c\nindex d6b65e2..3fd9c0e 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -22,14 +22,11 @@\n  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA\n  */\n \n-#include \"cache.h\"\n-#include \"exec_cmd.h\"\n-#include \"run-command.h\"\n-#include \"prompt.h\"\n #ifdef NO_OPENSSL\n typedef void *SSL;\n #else\n #ifdef APPLE_COMMON_CRYPTO\n+/* git-compat-util.h overwrites ctype.h; this must be included first */\n #include <CommonCrypto/CommonHMAC.h>\n #define HMAC_CTX CCHmacContext\n #define HMAC_Init(hmac, key, len, algo) CCHmacInit(hmac, algo, key, len)\n@@ -37,12 +34,23 @@ typedef void *SSL;\n #define HMAC_Final(hmac, hash, ptr) CCHmacFinal(hmac, hash)\n #define HMAC_CTX_cleanup(ignore)\n #define EVP_md5() kCCHmacAlgMD5\n+\n+#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 1070\n+#define APPLE_LION_OR_NEWER\n+#include <Security/Security.h>\n+#define HEADER_SHA_H /* suppress inclusion of openssl/sha.h */\n+#endif\n+\n #else\n #include <openssl/evp.h>\n #include <openssl/hmac.h>\n #endif\n #include <openssl/x509v3.h>\n #endif\n+#include \"cache.h\"\n+#include \"exec_cmd.h\"\n+#include \"run-command.h\"\n+#include \"prompt.h\"\n \n static const char imap_send_usage[] = \"git imap-send < <mbox>\";\n \n@@ -877,6 +885,75 @@ static void imap_close_store(struct imap_store *ctx)\n \tfree(ctx);\n }\n \n+#ifdef APPLE_LION_OR_NEWER\n+#define EVP_DecodeBlock git_CC_EVP_DecodeBlock\n+#define EVP_EncodeBlock git_CC_EVP_EncodeBlock\n+#define error_check(pattern, err) \\\n+\tdo { \\\n+\t\tif (err) { \\\n+\t\t\tdie(pattern, (long)CFErrorGetCode(err)); \\\n+\t\t} \\\n+\t} while(0)\n+\n+static int git_CC_EVP_EncodeBlock(unsigned char *out,\n+\t\tconst unsigned char *in, int inlen)\n+{\n+\tCFErrorRef err;\n+\tSecTransformRef encoder;\n+\tCFDataRef input, output;\n+\tCFIndex length;\n+\n+\tencoder = SecEncodeTransformCreate(kSecBase64Encoding, &err);\n+\terror_check(\"SecEncodeTransformCreate failed: %ld\", err);\n+\n+\tinput = CFDataCreate(kCFAllocatorDefault, in, inlen);\n+\tSecTransformSetAttribute(encoder, kSecTransformInputAttributeName,\n+\t\t\tinput, &err);\n+\terror_check(\"SecTransformSetAttribute failed: %ld\", err);\n+\n+\toutput = SecTransformExecute(encoder, &err);\n+\terror_check(\"SecTransformExecute failed: %ld\", err);\n+\n+\tlength = CFDataGetLength(output);\n+\tCFDataGetBytes(output, CFRangeMake(0, length), out);\n+\n+\tCFRelease(output);\n+\tCFRelease(input);\n+\tCFRelease(encoder);\n+\n+\treturn (int)strlen((const char *)out);\n+}\n+\n+static int git_CC_EVP_DecodeBlock(unsigned char *out,\n+\t\tconst unsigned char *in, int inlen)\n+{\n+\tCFErrorRef err;\n+\tSecTransformRef decoder;\n+\tCFDataRef input, output;\n+\tCFIndex length;\n+\n+\tdecoder = SecDecodeTransformCreate(kSecBase64Encoding, &err);\n+\terror_check(\"SecEncodeTransformCreate failed: %ld\", err);\n+\n+\tinput = CFDataCreate(kCFAllocatorDefault, in, inlen);\n+\tSecTransformSetAttribute(decoder, kSecTransformInputAttributeName,\n+\t\t\tinput, &err);\n+\terror_check(\"SecTransformSetAttribute failed: %ld\", err);\n+\n+\toutput = SecTransformExecute(decoder, &err);\n+\terror_check(\"SecTransformExecute failed: %ld\", err);\n+\n+\tlength = CFDataGetLength(output);\n+\tCFDataGetBytes(output, CFRangeMake(0, length), out);\n+\n+\tCFRelease(output);\n+\tCFRelease(input);\n+\tCFRelease(decoder);\n+\n+\treturn (int)strlen((const char *)out);\n+}\n+#endif /* APPLE_LION_OR_NEWER */\n+\n #ifndef NO_OPENSSL\n \n /*\n-- \n1.8.3.2.804.g0da7a53.dirty\n"},{"id":"224194","messageId":"1206359C-E833-42D6-87C5-11C17BA6FCC4@apple.com","threadId":"34553","inReplyTo":"1374957092-41505-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH] imap-send: use Apple's Security framework for base64 encoding","fromName":"Jeremy Huddleston Sequoia","fromEmail":"jeremyhu@apple.com","sentAt":"2013-07-27T23:28:06Z","receivedAt":"2013-07-27T23:28:06Z","isPatch":true,"sender":{"key":"jeremyhu@apple.com","avatar":"https://avatars.githubusercontent.com/u/1258676?v=4"},"body":"Hi David,\n\nThanks for massaging it to apply to master and cleaning up the style conflicts.\n\nOn Jul 27, 2013, at 13:31, David Aguilar <davvid@gmail.com> wrote:\n\n> From: Jeremy Huddleston <jeremyhu@apple.com>\n> \n> Use Apple's supported functions for base64 encoding instead\n> of the deprecated OpenSSL functions.\n> \n> Signed-off-by: Jeremy Huddleston <jeremyhu@apple.com>\n> Signed-off-by: David Aguilar <davvid@gmail.com>\n> ---\n> This is Jeremy's original patch rebased onto the latest master.\n> \n> Jeremy, the only way I could get this to work was to suppress inclusion of\n> openssl/sha.h by defining HEADER_SHA_H.  This can be removed when we have\n> replacements for openssl/x509v3.h.\n> \n> Makefile    |  1 +\n> imap-send.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---\n> 2 files changed, 82 insertions(+), 4 deletions(-)\n> \n> diff --git a/Makefile b/Makefile\n> index 0600eb4..4c40665 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1413,6 +1413,7 @@ ifdef PPC_SHA1\n> \tLIB_H += ppc/sha1.h\n> else\n> ifdef APPLE_COMMON_CRYPTO\n> +\tLIB_4_CRYPTO += -framework Security -framework CoreFoundation\n> \tCOMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL\n> \tSHA1_HEADER = <CommonCrypto/CommonDigest.h>\n> else\n> diff --git a/imap-send.c b/imap-send.c\n> index d6b65e2..3fd9c0e 100644\n> --- a/imap-send.c\n> +++ b/imap-send.c\n> @@ -22,14 +22,11 @@\n>  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA\n>  */\n> \n> -#include \"cache.h\"\n> -#include \"exec_cmd.h\"\n> -#include \"run-command.h\"\n> -#include \"prompt.h\"\n> #ifdef NO_OPENSSL\n> typedef void *SSL;\n> #else\n> #ifdef APPLE_COMMON_CRYPTO\n> +/* git-compat-util.h overwrites ctype.h; this must be included first */\n> #include <CommonCrypto/CommonHMAC.h>\n> #define HMAC_CTX CCHmacContext\n> #define HMAC_Init(hmac, key, len, algo) CCHmacInit(hmac, algo, key, len)\n> @@ -37,12 +34,23 @@ typedef void *SSL;\n> #define HMAC_Final(hmac, hash, ptr) CCHmacFinal(hmac, hash)\n> #define HMAC_CTX_cleanup(ignore)\n> #define EVP_md5() kCCHmacAlgMD5\n> +\n> +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 1070\n> +#define APPLE_LION_OR_NEWER\n> +#include <Security/Security.h>\n> +#define HEADER_SHA_H /* suppress inclusion of openssl/sha.h */\n> +#endif\n> +\n> #else\n> #include <openssl/evp.h>\n> #include <openssl/hmac.h>\n> #endif\n> #include <openssl/x509v3.h>\n> #endif\n> +#include \"cache.h\"\n> +#include \"exec_cmd.h\"\n> +#include \"run-command.h\"\n> +#include \"prompt.h\"\n> \n> static const char imap_send_usage[] = \"git imap-send < <mbox>\";\n> \n> @@ -877,6 +885,75 @@ static void imap_close_store(struct imap_store *ctx)\n> \tfree(ctx);\n> }\n> \n> +#ifdef APPLE_LION_OR_NEWER\n> +#define EVP_DecodeBlock git_CC_EVP_DecodeBlock\n> +#define EVP_EncodeBlock git_CC_EVP_EncodeBlock\n> +#define error_check(pattern, err) \\\n> +\tdo { \\\n> +\t\tif (err) { \\\n> +\t\t\tdie(pattern, (long)CFErrorGetCode(err)); \\\n> +\t\t} \\\n> +\t} while(0)\n> +\n> +static int git_CC_EVP_EncodeBlock(unsigned char *out,\n> +\t\tconst unsigned char *in, int inlen)\n> +{\n> +\tCFErrorRef err;\n> +\tSecTransformRef encoder;\n> +\tCFDataRef input, output;\n> +\tCFIndex length;\n> +\n> +\tencoder = SecEncodeTransformCreate(kSecBase64Encoding, &err);\n> +\terror_check(\"SecEncodeTransformCreate failed: %ld\", err);\n> +\n> +\tinput = CFDataCreate(kCFAllocatorDefault, in, inlen);\n> +\tSecTransformSetAttribute(encoder, kSecTransformInputAttributeName,\n> +\t\t\tinput, &err);\n> +\terror_check(\"SecTransformSetAttribute failed: %ld\", err);\n> +\n> +\toutput = SecTransformExecute(encoder, &err);\n> +\terror_check(\"SecTransformExecute failed: %ld\", err);\n> +\n> +\tlength = CFDataGetLength(output);\n> +\tCFDataGetBytes(output, CFRangeMake(0, length), out);\n> +\n> +\tCFRelease(output);\n> +\tCFRelease(input);\n> +\tCFRelease(encoder);\n> +\n> +\treturn (int)strlen((const char *)out);\n> +}\n> +\n> +static int git_CC_EVP_DecodeBlock(unsigned char *out,\n> +\t\tconst unsigned char *in, int inlen)\n> +{\n> +\tCFErrorRef err;\n> +\tSecTransformRef decoder;\n> +\tCFDataRef input, output;\n> +\tCFIndex length;\n> +\n> +\tdecoder = SecDecodeTransformCreate(kSecBase64Encoding, &err);\n> +\terror_check(\"SecEncodeTransformCreate failed: %ld\", err);\n> +\n> +\tinput = CFDataCreate(kCFAllocatorDefault, in, inlen);\n> +\tSecTransformSetAttribute(decoder, kSecTransformInputAttributeName,\n> +\t\t\tinput, &err);\n> +\terror_check(\"SecTransformSetAttribute failed: %ld\", err);\n> +\n> +\toutput = SecTransformExecute(decoder, &err);\n> +\terror_check(\"SecTransformExecute failed: %ld\", err);\n> +\n> +\tlength = CFDataGetLength(output);\n> +\tCFDataGetBytes(output, CFRangeMake(0, length), out);\n> +\n> +\tCFRelease(output);\n> +\tCFRelease(input);\n> +\tCFRelease(decoder);\n> +\n> +\treturn (int)strlen((const char *)out);\n> +}\n> +#endif /* APPLE_LION_OR_NEWER */\n> +\n> #ifndef NO_OPENSSL\n> \n> /*\n> -- \n> 1.8.3.2.804.g0da7a53.dirty\n> \n\n"},{"id":"224207","messageId":"20130729033533.GB2890@elie.Belkin","threadId":"34553","inReplyTo":"1374957092-41505-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH] imap-send: use Apple's Security framework for base64 encoding","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-07-29T03:35:33Z","receivedAt":"2013-07-29T03:35:33Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nDavid Aguilar wrote:\n\n> --- a/imap-send.c\n> +++ b/imap-send.c\n> @@ -22,14 +22,11 @@\n>   *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA\n>   */\n>  \n> -#include \"cache.h\"\n> -#include \"exec_cmd.h\"\n> -#include \"run-command.h\"\n> -#include \"prompt.h\"\n>  #ifdef NO_OPENSSL\n>  typedef void *SSL;\n>  #else\n>  #ifdef APPLE_COMMON_CRYPTO\n> +/* git-compat-util.h overwrites ctype.h; this must be included first */\n>  #include <CommonCrypto/CommonHMAC.h>\n\nThanks for your work on this.\n\nCurrently each translation unit of git includes git-compat-util.h or a\nheader like cache.h that includes git-compat-util.h before doing\nanything else, since otherwise feature test macros are not set before\nthe first system header is included.\n\nThe above (CommonCrypto needing to be included before some of the\ndefinitions from git-compat-util.h) suggests to me that CommonCrypto\nshould just be included directly from git-compat-util.h in some\nappropriate place.  That way any other header that needs CommonCrypto\nroutines only has to include git-compat-util.h first as usual and\ndoesn't have to worry about the order of other #includes.  Could that\nwork?\n\nThanks and hope that helps,\nJonathan\n"},{"id":"224212","messageId":"CAJDDKr7=Y=zRHv_NRS64tXN=bw8hCZ503oVP+96TAkSzYbV9ug@mail.gmail.com","threadId":"34553","inReplyTo":"20130729033533.GB2890@elie.Belkin","subject":"Re: [PATCH] imap-send: use Apple's Security framework for base64 encoding","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2013-07-29T07:23:31Z","receivedAt":"2013-07-29T07:23:31Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Sun, Jul 28, 2013 at 8:35 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Hi,\n>\n> David Aguilar wrote:\n>\n>> --- a/imap-send.c\n>> +++ b/imap-send.c\n>> @@ -22,14 +22,11 @@\n>>   *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA\n>>   */\n>>\n>> -#include \"cache.h\"\n>> -#include \"exec_cmd.h\"\n>> -#include \"run-command.h\"\n>> -#include \"prompt.h\"\n>>  #ifdef NO_OPENSSL\n>>  typedef void *SSL;\n>>  #else\n>>  #ifdef APPLE_COMMON_CRYPTO\n>> +/* git-compat-util.h overwrites ctype.h; this must be included first */\n>>  #include <CommonCrypto/CommonHMAC.h>\n>\n> Thanks for your work on this.\n>\n> Currently each translation unit of git includes git-compat-util.h or a\n> header like cache.h that includes git-compat-util.h before doing\n> anything else, since otherwise feature test macros are not set before\n> the first system header is included.\n>\n> The above (CommonCrypto needing to be included before some of the\n> definitions from git-compat-util.h) suggests to me that CommonCrypto\n> should just be included directly from git-compat-util.h in some\n> appropriate place.  That way any other header that needs CommonCrypto\n> routines only has to include git-compat-util.h first as usual and\n> doesn't have to worry about the order of other #includes.  Could that\n> work?\n\nimap-send is currently the only user of this stuff; I believe this\nwould pull in these library dependencies for all builtins.\n\nIf we don't mind a new file, something like git-compat-crypto.h could\nbe a nice place to tuck all these #ifdefs away.  If we had another\ncommand that needed these then it'd be a easier to justify a new file.\n\nA test-crypto command in the test suite could be written to verify\nthat the implementations work as advertised.  That counts as \"another\ncommand\", albeit an internal one.  I don't believe imap-send has any\ntest coverage so pulling this stuff out would help make it more\ntestable, which is a net win.  Libifying might be a little premature,\nthough.\n\nThoughts?\n-- \nDavid\n"},{"id":"224230","messageId":"7vhafd1i0x.fsf@alter.siamese.dyndns.org","threadId":"34553","inReplyTo":"1374957092-41505-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH] imap-send: use Apple's Security framework for base64 encoding","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-29T15:51:26Z","receivedAt":"2013-07-29T15:51:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar <davvid@gmail.com> writes:\n\n> diff --git a/imap-send.c b/imap-send.c\n> index d6b65e2..3fd9c0e 100644\n> --- a/imap-send.c\n> +++ b/imap-send.c\n> @@ -22,14 +22,11 @@\n>   *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA\n>   */\n>  \n> -#include \"cache.h\"\n> -#include \"exec_cmd.h\"\n> -#include \"run-command.h\"\n> -#include \"prompt.h\"\n>  #ifdef NO_OPENSSL\n>  typedef void *SSL;\n>  #else\n>  #ifdef APPLE_COMMON_CRYPTO\n> +/* git-compat-util.h overwrites ctype.h; this must be included first */\n\nI do not like this part of the patch, for the exact reason Jonathan\nalready mentioned.  The concern this comment addresses is exactly\nthe kind of thing git-compat-util.h was introduced for.\n\nCould we keep the encapsulation of the logic to decide what to\ninclude and in what order in git-compat-util.h, please?\n"}]}