commit 0c294254b53371eab9c805bf6a8f418a3d6ca9c5
author Adam Langley <agl@google.com> Mon Dec 12 11:46:09 2016 -0800
committer Adam Langley <agl@google.com> Wed Dec 14 17:49:10 2016 +0000
tree 5120ef6fbd02a64b0fab903b643153f9b3278483
parent 703aa16003006deef59672f271215d2ff7c27300

Don't use |X509_get_pubkey| in TLS 1.3 code either.

Change-Id: I7050c74ac38503f450760a857442e6fc0863d5df
Reviewed-on: https://boringssl-review.googlesource.com/12708
Reviewed-by: Adam Langley <agl@google.com>

ssl/internal.h

diff --git a/ssl/internal.h b/ssl/internal.h
index 76e37b4..43aeaad 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1067,8 +1067,8 @@
  * it returns one. Otherwise, it sends an alert and returns zero. */
 int tls13_check_message_type(SSL *ssl, int type);
 
-int tls13_process_certificate(SSL *ssl, int allow_anonymous);
-int tls13_process_certificate_verify(SSL *ssl);
+int tls13_process_certificate(SSL_HANDSHAKE *hs, int allow_anonymous);
+int tls13_process_certificate_verify(SSL_HANDSHAKE *hs);
 int tls13_process_finished(SSL_HANDSHAKE *hs);
 
 int tls13_prepare_certificate(SSL_HANDSHAKE *hs);

ssl/tls13_both.c

diff --git a/ssl/tls13_both.c b/ssl/tls13_both.c
index 25dc852..1be1897 100644
--- a/ssl/tls13_both.c
+++ b/ssl/tls13_both.c
@@ -163,7 +163,8 @@
   return 0;
 }
 
-int tls13_process_certificate(SSL *ssl, int allow_anonymous) {
+int tls13_process_certificate(SSL_HANDSHAKE *hs, int allow_anonymous) {
+  SSL *const ssl = hs->ssl;
   CBS cbs, context, certificate_list;
   CBS_init(&cbs, ssl->init_msg, ssl->init_num);
   if (!CBS_get_u8_length_prefixed(&cbs, &context) ||
@@ -177,6 +178,7 @@
       ssl->server && ssl->retain_only_sha256_of_client_certs;
   int ret = 0;
 
+  EVP_PKEY *pkey = NULL;
   STACK_OF(CRYPTO_BUFFER) *certs = sk_CRYPTO_BUFFER_new_null();
   if (certs == NULL) {
     ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
@@ -200,10 +202,19 @@
       goto err;
     }
 
-    /* Retain the hash of the leaf certificate if requested. */
-    if (sk_CRYPTO_BUFFER_num(certs) == 0 && retain_sha256) {
-      SHA256(CBS_data(&certificate), CBS_len(&certificate),
-             ssl->s3->new_session->peer_sha256);
+    if (sk_CRYPTO_BUFFER_num(certs) == 0) {
+      pkey = ssl_cert_parse_pubkey(&certificate);
+      if (pkey == NULL) {
+        ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+        OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+        goto err;
+      }
+
+      if (retain_sha256) {
+        /* Retain the hash of the leaf certificate if requested. */
+        SHA256(CBS_data(&certificate), CBS_len(&certificate),
+               ssl->s3->new_session->peer_sha256);
+      }
     }
 
     CRYPTO_BUFFER *buf =
@@ -289,6 +300,10 @@
     goto err;
   }
 
+  EVP_PKEY_free(hs->peer_pubkey);
+  hs->peer_pubkey = pkey;
+  pkey = NULL;
+
   sk_CRYPTO_BUFFER_pop_free(ssl->s3->new_session->certs, CRYPTO_BUFFER_free);
   ssl->s3->new_session->certs = certs;
   certs = NULL;
@@ -326,19 +341,17 @@
 
 err:
   sk_CRYPTO_BUFFER_pop_free(certs, CRYPTO_BUFFER_free);
+  EVP_PKEY_free(pkey);
   return ret;
 }
 
-int tls13_process_certificate_verify(SSL *ssl) {
+int tls13_process_certificate_verify(SSL_HANDSHAKE *hs) {
+  SSL *const ssl = hs->ssl;
   int ret = 0;
-  X509 *peer = ssl->s3->new_session->x509_peer;
-  EVP_PKEY *pkey = NULL;
   uint8_t *msg = NULL;
   size_t msg_len;
 
-  /* Filter out unsupported certificate types. */
-  pkey = X509_get_pubkey(peer);
-  if (pkey == NULL) {
+  if (hs->peer_pubkey == NULL) {
     goto err;
   }
 
@@ -369,7 +382,7 @@
 
   int sig_ok =
       ssl_public_key_verify(ssl, CBS_data(&signature), CBS_len(&signature),
-                            signature_algorithm, pkey, msg, msg_len);
+                            signature_algorithm, hs->peer_pubkey, msg, msg_len);
 #if defined(BORINGSSL_UNSAFE_FUZZER_MODE)
   sig_ok = 1;
   ERR_clear_error();
@@ -383,7 +396,6 @@
   ret = 1;
 
 err:
-  EVP_PKEY_free(pkey);
   OPENSSL_free(msg);
   return ret;
 }

ssl/tls13_client.c

diff --git a/ssl/tls13_client.c b/ssl/tls13_client.c
index f106c16..4a202d7 100644
--- a/ssl/tls13_client.c
+++ b/ssl/tls13_client.c
@@ -404,7 +404,7 @@
 static enum ssl_hs_wait_t do_process_server_certificate(SSL_HANDSHAKE *hs) {
   SSL *const ssl = hs->ssl;
   if (!tls13_check_message_type(ssl, SSL3_MT_CERTIFICATE) ||
-      !tls13_process_certificate(ssl, 0 /* certificate required */) ||
+      !tls13_process_certificate(hs, 0 /* certificate required */) ||
       !ssl_hash_current_message(ssl)) {
     return ssl_hs_error;
   }
@@ -417,7 +417,7 @@
     SSL_HANDSHAKE *hs) {
   SSL *const ssl = hs->ssl;
   if (!tls13_check_message_type(ssl, SSL3_MT_CERTIFICATE_VERIFY) ||
-      !tls13_process_certificate_verify(ssl) ||
+      !tls13_process_certificate_verify(hs) ||
       !ssl_hash_current_message(ssl)) {
     return ssl_hs_error;
   }

ssl/tls13_server.c

diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c
index 367ea5a..1aca634 100644
--- a/ssl/tls13_server.c
+++ b/ssl/tls13_server.c
@@ -547,7 +547,7 @@
       (ssl->verify_mode & SSL_VERIFY_FAIL_IF_NO_PEER_CERT) == 0;
 
   if (!tls13_check_message_type(ssl, SSL3_MT_CERTIFICATE) ||
-      !tls13_process_certificate(ssl, allow_anonymous) ||
+      !tls13_process_certificate(hs, allow_anonymous) ||
       !ssl_hash_current_message(ssl)) {
     return ssl_hs_error;
   }
@@ -566,7 +566,7 @@
   }
 
   if (!tls13_check_message_type(ssl, SSL3_MT_CERTIFICATE_VERIFY) ||
-      !tls13_process_certificate_verify(ssl) ||
+      !tls13_process_certificate_verify(hs) ||
       !ssl_hash_current_message(ssl)) {
     return ssl_hs_error;
   }