diff mbox

OpenSSL: Fix OpenSSL 1.1.0 compatibility functions

Message ID CAF8qwaBNrWtn0MhXt0EqdwUkZMv=zyXffXWnCzmQtK+iUemK8A@mail.gmail.com
State Accepted
Headers show

Commit Message

David Benjamin Aug. 11, 2016, 2:49 p.m. UTC 
Attached since my mail client will probably mess it up otherwise.

To be consistent with OpenSSL 1.1.0, the free functions should
internally check for NULL. EVP_MD_CTX_free also was missing an
EVP_MD_CTX_cleanup, so this leaked a little.

OpenSSL 1.1.0 also has given get_rfc3526_prime_1536 a better namespace
with get_rfc3526_prime_1536 as a compatibility-only name. Use that
instead in 1.1.0.

Note this patch checks OPENSSL_VERSION_NUMBER for
BN_get_rfc3526_prime_1536 before OPENSSL_IS_BORINGSSL. This is
intentional. BoringSSL currently claims to be 1.0.2, so this won't
break existing BoringSSL's.

Eventually we hope to claim 1.1.0 compatibility. I think we originally
omitted get_rfc3526_prime_1536 because it was unnamespaced, but
BN_get_rfc3526_prime_1536 is a fine name so, when we claim 1.1.0, that
function will exist and you won't need the extra implementation. I'll
leave it to you all to decide when you drop support for older AOSP
releases, but my hope is that you can drop that ifdef, the
SSL_get_client_random one (sorry about that one!), and possibly
others, in time, and just rely on the advertised version number being
accurate. (Right now we're this awkward mix of mostly 1.0.2 with bits
of 1.1.0.)

You'll probably want to confirm I haven't broken 1.1.0. I've only
compile-tested this in Android.

David

Comments

Jouni Malinen Aug. 13, 2016, 9:28 p.m. UTC | #1 
On Thu, Aug 11, 2016 at 10:49:49AM -0400, David Benjamin wrote:
> To be consistent with OpenSSL 1.1.0, the free functions should
> internally check for NULL. EVP_MD_CTX_free also was missing an
> EVP_MD_CTX_cleanup, so this leaked a little.
> 
> OpenSSL 1.1.0 also has given get_rfc3526_prime_1536 a better namespace
> with get_rfc3526_prime_1536 as a compatibility-only name. Use that
> instead in 1.1.0.

Thanks, applied.
diff mbox

Patch

From dc7c9214d7c4db3ff019706df2f0b06c0f2baeef Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Wed, 10 Aug 2016 13:28:45 -0400
Subject: [PATCH] OpenSSL: Fix OpenSSL 1.1.0 compatibility functions

To be consistent with OpenSSL 1.1.0, the free functions should
internally check for NULL. EVP_MD_CTX_free also was missing an
EVP_MD_CTX_cleanup, so this leaked a little.

OpenSSL 1.1.0 also has given get_rfc3526_prime_1536 a better namespace
with get_rfc3526_prime_1536 as a compatibility-only name. Use that
instead in 1.1.0.

Signed-off-by: David Benjamin <davidben@google.com>
---
 src/crypto/crypto_openssl.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/crypto/crypto_openssl.c b/src/crypto/crypto_openssl.c
index 7f33686..19e0e2b 100644
--- a/src/crypto/crypto_openssl.c
+++ b/src/crypto/crypto_openssl.c
@@ -49,6 +49,8 @@  static HMAC_CTX * HMAC_CTX_new(void)
 
 static void HMAC_CTX_free(HMAC_CTX *ctx)
 {
+	if (!ctx)
+		return;
 	HMAC_CTX_cleanup(ctx);
 	bin_clear_free(ctx, sizeof(*ctx));
 }
@@ -67,6 +69,9 @@  static EVP_MD_CTX * EVP_MD_CTX_new(void)
 
 static void EVP_MD_CTX_free(EVP_MD_CTX *ctx)
 {
+	if (!ctx)
+		return;
+	EVP_MD_CTX_cleanup(ctx);
 	bin_clear_free(ctx, sizeof(*ctx));
 }
 
@@ -74,7 +79,11 @@  static void EVP_MD_CTX_free(EVP_MD_CTX *ctx)
 
 static BIGNUM * get_group5_prime(void)
 {
-#ifdef OPENSSL_IS_BORINGSSL
+#if OPENSSL_VERSION_NUMBER >= 0x10100000L && !defined(LIBRESSL_VERSION_NUMBER)
+	return BN_get_rfc3526_prime_1536(NULL);
+#elif !defined(OPENSSL_IS_BORINGSSL)
+	return get_rfc3526_prime_1536(NULL);
+#else
 	static const unsigned char RFC3526_PRIME_1536[] = {
 		0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xC9,0x0F,0xDA,0xA2,
 		0x21,0x68,0xC2,0x34,0xC4,0xC6,0x62,0x8B,0x80,0xDC,0x1C,0xD1,
@@ -94,9 +103,7 @@  static BIGNUM * get_group5_prime(void)
 		0xCA,0x23,0x73,0x27,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,
 	};
         return BN_bin2bn(RFC3526_PRIME_1536, sizeof(RFC3526_PRIME_1536), NULL);
-#else /* OPENSSL_IS_BORINGSSL */
-	return get_rfc3526_prime_1536(NULL);
-#endif /* OPENSSL_IS_BORINGSSL */
+#endif
 }
 
 #ifdef OPENSSL_NO_SHA256
-- 
2.8.0.rc3.226.g39d4020