diff mbox

Support building with BoringSSL.

Message ID 20140919172807.DB887E0650@agl-b.sfo.corp.google.com
State Superseded
Headers show

Commit Message

Adam Langley Sept. 19, 2014, 1:40 a.m. UTC
BoringSSL is Google's cleanup of OpenSSL and an attempt to unify
Chromium, Android and internal codebases around a single OpenSSL.

As part of moving Android to BoringSSL, the wpa_supplicant maintainers
in Android requested that I upstream the change. I've worked to reduce
the size of the patch a lot but I'm afraid that it still contains a
number of #ifdefs.

One change worth noting (which I didn't #ifdef) is the switch from the
deprecated ERR_remove_state to ERR_remove_thread_state. I think this is
generally applicable because upstream have deprecated it, but it does
require OpenSSL 1.0.0 or greater (i.e. not 0.9.8).

[1] https://www.imperialviolet.org/2014/06/20/boringssl.html
---
 src/crypto/crypto_openssl.c     |  6 +++++-
 src/crypto/tls_openssl.c        | 17 ++++++++++++-----
 src/eap_common/eap_pwd_common.c |  2 ++
 3 files changed, 19 insertions(+), 6 deletions(-)

Comments

Jouni Malinen Sept. 28, 2014, 5:31 p.m. UTC | #1
On Thu, Sep 18, 2014 at 06:40:03PM -0700, Adam Langley wrote:
> BoringSSL is Google's cleanup of OpenSSL and an attempt to unify
> Chromium, Android and internal codebases around a single OpenSSL.
> 
> As part of moving Android to BoringSSL, the wpa_supplicant maintainers
> in Android requested that I upstream the change. I've worked to reduce
> the size of the patch a lot but I'm afraid that it still contains a
> number of #ifdefs.

Thanks! This looks mostly reasonable. Could you please read the top
level CONTRIBUTIONS file (*) and provide Signed-off-by: line for the
patch so that I can apply this?

(*) http://w1.fi/cgit/hostap/plain/CONTRIBUTIONS

> One change worth noting (which I didn't #ifdef) is the switch from the
> deprecated ERR_remove_state to ERR_remove_thread_state. I think this is
> generally applicable because upstream have deprecated it, but it does
> require OpenSSL 1.0.0 or greater (i.e. not 0.9.8).

I'm still trying to support 0.9.8, so this will need to be made to use
suitable #ifdef or maybe the cleanest options would be to just do
something like this as a backwards compatibility wrapper:

#if OPENSSL_VERSION_NUMBER < 0x10000000L
#define ERR_remove_thread_state(tid) ERR_remove_state(0)
#endif

> diff --git a/src/crypto/tls_openssl.c b/src/crypto/tls_openssl.c
> @@ -38,14 +38,20 @@
> -#ifdef SSL_F_SSL_SET_SESSION_TICKET_EXT
> -#ifdef SSL_OP_NO_TICKET
> +#if (defined(SSL_F_SSL_SET_SESSION_TICKET_EXT) && defined(SSL_OP_NO_TICKET)) ||\
> +    defined(OPENSSL_IS_BORINGSSL)

I guess this is because of SSL_F_SSL_SET_SESSION_TICKET_EXT not being
defined in BoringSSL. This could be cleaner to convert to
OPENSSL_VERSION_NUMBER >= 0x10000000L (or something similar.. I don't
remember why I ended up using SSL_F_SSL_SET_SESSION_TICKET_EXT instead..
the early days (well, years..) of EAP-FAST support was somewhat of a
mess with OpenSSL).
Adam Langley Sept. 29, 2014, 9:25 p.m. UTC | #2
On Sun, Sep 28, 2014 at 10:31 AM, Jouni Malinen <j@w1.fi> wrote:
> Thanks! This looks mostly reasonable. Could you please read the top
> level CONTRIBUTIONS file (*) and provide Signed-off-by: line for the
> patch so that I can apply this?

Done.

> I'm still trying to support 0.9.8, so this will need to be made to use
> suitable #ifdef or maybe the cleanest options would be to just do
> something like this as a backwards compatibility wrapper:
>
> #if OPENSSL_VERSION_NUMBER < 0x10000000L
> #define ERR_remove_thread_state(tid) ERR_remove_state(0)
> #endif

Done.

> I guess this is because of SSL_F_SSL_SET_SESSION_TICKET_EXT not being
> defined in BoringSSL. This could be cleaner to convert to
> OPENSSL_VERSION_NUMBER >= 0x10000000L (or something similar.. I don't
> remember why I ended up using SSL_F_SSL_SET_SESSION_TICKET_EXT instead..
> the early days (well, years..) of EAP-FAST support was somewhat of a
> mess with OpenSSL).

I check when each of these defines was added to OpenSSL:

SSL_OP_NO_TICKET - bbfc6ac0 (Sat Aug 11 2007)
SSL_F_SSL_SET_SESSION_TICKET_EXT - 12bf56c0 (Sat Nov 15 2008)

Thus I believe that 1.0.0 was the first release that included them and
have updated the #if accordingly.

Will resend the patch in a second. Thanks!


Cheers

AGL
diff mbox

Patch

diff --git a/src/crypto/crypto_openssl.c b/src/crypto/crypto_openssl.c
index 8876ebf..da4d5ef 100644
--- a/src/crypto/crypto_openssl.c
+++ b/src/crypto/crypto_openssl.c
@@ -40,7 +40,7 @@ 
 
 static BIGNUM * get_group5_prime(void)
 {
-#if OPENSSL_VERSION_NUMBER < 0x00908000
+#if OPENSSL_VERSION_NUMBER < 0x00908000 || defined(OPENSSL_IS_BORINGSSL)
 	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,
@@ -199,8 +199,10 @@  static const EVP_CIPHER * aes_get_evp_cipher(size_t keylen)
 	switch (keylen) {
 	case 16:
 		return EVP_aes_128_ecb();
+#if !defined(OPENSSL_IS_BORINGSSL)
 	case 24:
 		return EVP_aes_192_ecb();
+#endif
 	case 32:
 		return EVP_aes_256_ecb();
 	}
@@ -378,9 +380,11 @@  struct crypto_cipher * crypto_cipher_init(enum crypto_cipher_alg alg,
 		case 16:
 			cipher = EVP_aes_128_cbc();
 			break;
+#if !defined(OPENSSL_IS_BORINGSSL)
 		case 24:
 			cipher = EVP_aes_192_cbc();
 			break;
+#endif
 		case 32:
 			cipher = EVP_aes_256_cbc();
 			break;
diff --git a/src/crypto/tls_openssl.c b/src/crypto/tls_openssl.c
index d2d6600..6db3755 100644
--- a/src/crypto/tls_openssl.c
+++ b/src/crypto/tls_openssl.c
@@ -38,14 +38,20 @@ 
 #define OPENSSL_SUPPORTS_CTX_APP_DATA
 #endif
 
-#ifdef SSL_F_SSL_SET_SESSION_TICKET_EXT
-#ifdef SSL_OP_NO_TICKET
+#if (defined(SSL_F_SSL_SET_SESSION_TICKET_EXT) && defined(SSL_OP_NO_TICKET)) ||\
+    defined(OPENSSL_IS_BORINGSSL)
 /*
  * Session ticket override patch was merged into OpenSSL 0.9.9 tree on
  * 2008-11-15. This version uses a bit different API compared to the old patch.
  */
 #define CONFIG_OPENSSL_TICKET_OVERRIDE
 #endif
+
+#if defined(OPENSSL_IS_BORINGSSL)
+/* stack_index_t is the return type of OpenSSL's sk_XXX_num() functions. */
+typedef size_t stack_index_t;
+#else
+typedef int stack_index_t;
 #endif
 
 #ifdef SSL_set_tlsext_status_type
@@ -853,7 +859,7 @@  void tls_deinit(void *ssl_ctx)
 		ENGINE_cleanup();
 #endif /* OPENSSL_NO_ENGINE */
 		CRYPTO_cleanup_all_ex_data();
-		ERR_remove_state(0);
+		ERR_remove_thread_state(NULL);
 		ERR_free_strings();
 		EVP_cleanup();
 		os_free(tls_global->ocsp_stapling_response);
@@ -1102,7 +1108,8 @@  static int tls_match_altsubject_component(X509 *cert, int type,
 {
 	GENERAL_NAME *gen;
 	void *ext;
-	int i, found = 0;
+	int found = 0;
+	stack_index_t i;
 
 	ext = X509_get_ext_d2i(cert, NID_subject_alt_name, NULL, NULL);
 
@@ -1639,7 +1646,7 @@  static int tls_connection_ca_cert(void *_ssl_ctx, struct tls_connection *conn,
 	if (ca_cert && os_strncmp("keystore://", ca_cert, 11) == 0) {
 		BIO *bio = BIO_from_keystore(&ca_cert[11]);
 		STACK_OF(X509_INFO) *stack = NULL;
-		int i;
+		stack_index_t i;
 
 		if (bio) {
 			stack = PEM_X509_INFO_read_bio(bio, NULL, NULL, NULL);
diff --git a/src/eap_common/eap_pwd_common.c b/src/eap_common/eap_pwd_common.c
index fdcff7f..d62303b 100644
--- a/src/eap_common/eap_pwd_common.c
+++ b/src/eap_common/eap_pwd_common.c
@@ -106,9 +106,11 @@  int compute_password_element(EAP_PWD_group *grp, u16 num,
         case 21:
 		nid = NID_secp521r1;
 		break;
+#if !defined(OPENSSL_IS_BORINGSSL)
         case 25:
 		nid = NID_X9_62_prime192v1;
 		break;
+#endif
         case 26:
 		nid = NID_secp224r1;
 		break;