diff mbox

Support building with BoringSSL.

Message ID 20141006223527.GB12576@w1.fi
State Accepted
Headers show

Commit Message

Jouni Malinen Oct. 6, 2014, 10:35 p.m. UTC 
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, I applied this with couple of changes. I had to do following
additional edits to make this compile and link in my tests:

Comments

Adam Langley Oct. 6, 2014, 10:44 p.m. UTC | #1 
On Mon, Oct 6, 2014 at 3:35 PM, Jouni Malinen <j@w1.fi> wrote:
> diff --git a/src/crypto/tls_openssl.c b/src/crypto/tls_openssl.c
> index 4436fb8..7335033 100644
> --- a/src/crypto/tls_openssl.c
> +++ b/src/crypto/tls_openssl.c
> @@ -1217,7 +1224,7 @@ static int tls_match_suffix(X509 *cert, const char *match)
>
>         ext = X509_get_ext_d2i(cert, NID_subject_alt_name, NULL, NULL);
>
> -       for (i = 0; ext && i < sk_GENERAL_NAME_num(ext); i++) {
> +       for (i = 0; ext && i < (int) sk_GENERAL_NAME_num(ext); i++) {
>                 gen = sk_GENERAL_NAME_value(ext, i);
>                 if (gen->type != GEN_DNS)
>                         continue;

Thanks! But I'm confused by this! Firstly, you're correct that there's
sort of a problem here: I didn't notice because Android hasn't updated
to include this code yet.

However, casting to an int is something that you would need to do if
sk_GENERAL_NAME_num was returning a size_t, which it does in
BoringSSL, but you're not building with BoringSSL.

But, rather than cast, the "int i" in this function can be changed to
a stack_index_t.

> @@ -3392,9 +3399,15 @@ unsigned int tls_capabilities(void *tls_ctx)
>   * commented out unless explicitly needed for EAP-FAST in order to be able to
>   * build this file with unmodified openssl. */
>
> +#ifdef OPENSSL_IS_BORINGSSL
> +static int tls_sess_sec_cb(SSL *s, void *secret, int *secret_len,
> +                          STACK_OF(SSL_CIPHER) *peer_ciphers,
> +                          const SSL_CIPHER **cipher, void *arg)
> +#else /* OPENSSL_IS_BORINGSSL */
>  static int tls_sess_sec_cb(SSL *s, void *secret, int *secret_len,
>                            STACK_OF(SSL_CIPHER) *peer_ciphers,
>                            SSL_CIPHER **cipher, void *arg)
> +#endif /* OPENSSL_IS_BORINGSSL */
>  {
>         struct tls_connection *conn = arg;
>         int ret;

This also seems correct, but only needed for BoringSSL. But I can't
imagine that you got it building with BoringSSL.

(Again, I think Android is omitting this code which is why I didn't hit it.)


Cheers

AGL
Jouni Malinen Oct. 7, 2014, 8:42 a.m. UTC | #2 
On Mon, Oct 06, 2014 at 03:44:48PM -0700, Adam Langley wrote:
> On Mon, Oct 6, 2014 at 3:35 PM, Jouni Malinen <j@w1.fi> wrote:
> > -       for (i = 0; ext && i < sk_GENERAL_NAME_num(ext); i++) {
> > +       for (i = 0; ext && i < (int) sk_GENERAL_NAME_num(ext); i++) {

> However, casting to an int is something that you would need to do if
> sk_GENERAL_NAME_num was returning a size_t, which it does in
> BoringSSL, but you're not building with BoringSSL.

I was.. I tested the build against OpenSSL 0.9.8, 1.0.1, 1.0.2-beta, and
the current snapshot of BoringSSL.

> But, rather than cast, the "int i" in this function can be changed to
> a stack_index_t.

Ah, yes, that could be reasonable. I gave up on trying to figure out the
details when I hit the sk_GENERAL_NAME_num macro in OpenSSL.. Example
users of that in OpenSSL use int while BoringSSL seems to be using
size_t.

> > +#ifdef OPENSSL_IS_BORINGSSL
> > +static int tls_sess_sec_cb(SSL *s, void *secret, int *secret_len,
> > +                          STACK_OF(SSL_CIPHER) *peer_ciphers,
> > +                          const SSL_CIPHER **cipher, void *arg)
> > +#else /* OPENSSL_IS_BORINGSSL */
> >  static int tls_sess_sec_cb(SSL *s, void *secret, int *secret_len,
> >                            STACK_OF(SSL_CIPHER) *peer_ciphers,
> >                            SSL_CIPHER **cipher, void *arg)
> > +#endif /* OPENSSL_IS_BORINGSSL */

> This also seems correct, but only needed for BoringSSL. But I can't
> imagine that you got it building with BoringSSL.

The version I pushed to hostap.git did indeed build successfully with
the current snapshot of BoringSSL.

> (Again, I think Android is omitting this code which is why I didn't hit it.)

Yeah, I assumed that was the case. By the way, the android-kk branch in
hostap.git is a snapshot of the current wpa_supplicant development
version in a form that can be built on Android. I'm using that to verify
compilation and some testing purposes on Android.
diff mbox

Patch

diff --git a/src/crypto/crypto_openssl.c b/src/crypto/crypto_openssl.c
index 028efc8..b4c59d1 100644
--- a/src/crypto/crypto_openssl.c
+++ b/src/crypto/crypto_openssl.c
@@ -137,7 +130,7 @@  void des_encrypt(const u8 *clear, const u8 *key, u8 *cypher)
 	}
 	pkey[i] = next | 1;
 
-	DES_set_key(&pkey, &ks);
+	DES_set_key((DES_cblock *) &pkey, &ks);
 	DES_ecb_encrypt((DES_cblock *) clear, (DES_cblock *) cypher, &ks,
 			DES_ENCRYPT);
 }
diff --git a/src/crypto/tls_openssl.c b/src/crypto/tls_openssl.c
index 4436fb8..7335033 100644
--- a/src/crypto/tls_openssl.c
+++ b/src/crypto/tls_openssl.c
@@ -1217,7 +1224,7 @@  static int tls_match_suffix(X509 *cert, const char *match)
 
 	ext = X509_get_ext_d2i(cert, NID_subject_alt_name, NULL, NULL);
 
-	for (i = 0; ext && i < sk_GENERAL_NAME_num(ext); i++) {
+	for (i = 0; ext && i < (int) sk_GENERAL_NAME_num(ext); i++) {
 		gen = sk_GENERAL_NAME_value(ext, i);
 		if (gen->type != GEN_DNS)
 			continue;
@@ -3392,9 +3399,15 @@  unsigned int tls_capabilities(void *tls_ctx)
  * commented out unless explicitly needed for EAP-FAST in order to be able to
  * build this file with unmodified openssl. */
 
+#ifdef OPENSSL_IS_BORINGSSL
+static int tls_sess_sec_cb(SSL *s, void *secret, int *secret_len,
+			   STACK_OF(SSL_CIPHER) *peer_ciphers,
+			   const SSL_CIPHER **cipher, void *arg)
+#else /* OPENSSL_IS_BORINGSSL */
 static int tls_sess_sec_cb(SSL *s, void *secret, int *secret_len,
 			   STACK_OF(SSL_CIPHER) *peer_ciphers,
 			   SSL_CIPHER **cipher, void *arg)
+#endif /* OPENSSL_IS_BORINGSSL */
 {
 	struct tls_connection *conn = arg;
 	int ret;