[PATCH] OpenSSL: BoringSSL has SSL_get_client_random, etc.

[Jouni Malinen]

On Thu, May 05, 2016 at 01:43:42PM -0400, David Benjamin wrote:
> Apologies for making your #ifdef soup even messier. The motivation
> here is we'd like to opaquify the SSL structs in BoringSSL (which
> should, in the long run, make wpa_supplicant less sensitive to changes
> on our end). To keep things simple, I'm mirroring OpenSSL 1.1.0's
> APIs. But, for the moment, BoringSSL's OPENSSL_VERSION_NUMBER still
> claims to be 1.0.2, so this will need some more conditionals.
> 
> I'm optimistic that someday we'll mimic enough of 1.1.0 that bumping
> OPENSSL_VERSION_NUMBER might make sense and then we won't need this
> special-case. For now, it and the ecosystem are enough of a moving
> target that I don't think it's feasible just yet.

This is problematic with existing versions of BoringSSL. As an example,
if I apply this and try to build against my previous BoringSSL build,
the compilation fails with:

  CC  ../src/eap_peer/eap_tls_common.c
../src/crypto/tls_openssl.c: In function ‘tls_connection_get_random’:
../src/crypto/tls_openssl.c:3079:2: error: implicit declaration of function ‘SSL_get_client_random’ [-Werror=implicit-function-declaration]
  keys->client_random_len = SSL_get_client_random(
  ^
../src/crypto/tls_openssl.c:3082:2: error: implicit declaration of function ‘SSL_get_server_random’ [-Werror=implicit-function-declaration]
  keys->server_random_len = SSL_get_server_random(
  ^
../src/crypto/tls_openssl.c: In function ‘openssl_tls_prf’:
../src/crypto/tls_openssl.c:3203:2: error: implicit declaration of function ‘SSL_SESSION_get_master_key’
[-Werror=implicit-function-declaration]
  master_key_len = SSL_SESSION_get_master_key(sess, master_key,
  ^


IMHO, it is quite unfortunate that BoringSSL is maintained in a manner
that prevents clean backwards compatibility with at least the versions
used in the recent past. Applying this patch would break various Android
cases where the BoringSSL version in the branch is not sufficiently
recent to have the macro defined.

Would there be any other way of using the pre-processor to automatically
determine whether BoringSSL is recent enough to include the new
commands? The "Add SSL_get_client_random and SSL_get_server_random"
doesn't seem to add anything to the header files to help the
pre-processor for this..

> BoringSSL added 1.1.0's SSL_get_client_random and friends in working towards
> opaquifying the SSL struct. But it, for the moment, still looks more like 1.0.2
> than 1.1.0 and advertises OPENSSL_VERSION_NUMBER as such. This means that there
> is no need to define those in BoringSSL and defining them causes conflicts. (C
> does not like having static and non-static functions with the same name.)

I guess the wrapper functions in src/crypto/tls_openssl.c could be
defined to us an alternative name and then use #define to override the
functions. Not that this is exactly nice, but at least it seems to build
with the current boringssl.git snapshot:

diff --git a/src/crypto/tls_openssl.c b/src/crypto/tls_openssl.c
index ebcc545..0f67290 100644
--- a/src/crypto/tls_openssl.c
+++ b/src/crypto/tls_openssl.c
@@ -57,8 +57,8 @@ typedef int stack_index_t;
  * 1.1.0. Provide compatibility wrappers for older versions.
  */
 
-static size_t SSL_get_client_random(const SSL *ssl, unsigned char *out,
-				    size_t outlen)
+static size_t _SSL_get_client_random(const SSL *ssl, unsigned char *out,
+				     size_t outlen)
 {
 	if (!ssl->s3 || outlen < SSL3_RANDOM_SIZE)
 		return 0;
@@ -66,9 +66,11 @@ static size_t SSL_get_client_random(const SSL *ssl, unsigned char *out,
 	return SSL3_RANDOM_SIZE;
 }
 
+#define SSL_get_client_random _SSL_get_client_random
 
-static size_t SSL_get_server_random(const SSL *ssl, unsigned char *out,
-				    size_t outlen)
+
+static size_t _SSL_get_server_random(const SSL *ssl, unsigned char *out,
+				     size_t outlen)
 {
 	if (!ssl->s3 || outlen < SSL3_RANDOM_SIZE)
 		return 0;
@@ -76,9 +78,10 @@ static size_t SSL_get_server_random(const SSL *ssl, unsigned char *out,
 	return SSL3_RANDOM_SIZE;
 }
 
+#define SSL_get_server_random _SSL_get_server_random
 
-static size_t SSL_SESSION_get_master_key(const SSL_SESSION *session,
-					 unsigned char *out, size_t outlen)
+static size_t _SSL_SESSION_get_master_key(const SSL_SESSION *session,
+					  unsigned char *out, size_t outlen)
 {
 	if (!session || session->master_key_length < 0 ||
 	    (size_t) session->master_key_length > outlen)
@@ -89,6 +92,8 @@ static size_t SSL_SESSION_get_master_key(const SSL_SESSION *session,
 	return outlen;
 }
 
+#define SSL_SESSION_get_master_key _SSL_SESSION_get_master_key
+
 #endif
 
 #ifdef ANDROID

-- 
Jouni Malinen                                            PGP id EFC895FA