Don't depend on the X509 code for getting public keys.

This change removes the use of |X509_get_pubkey| from the TLS <= 1.2
code. That function is replaced with a shallow parse of the certificate
to extract the public key instead.

Change-Id: I8938c6c5a01b32038c6b6fa58eb065e5b44ca6d2
Reviewed-on: https://boringssl-review.googlesource.com/12707
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

crypto/err/ssl.errordata

@@ -22,6 +22,7 @@ SSL,261,BLOCK_CIPHER_PAD_IS_WRONG
 SSL,120,BN_LIB
 SSL,255,BUFFERED_MESSAGES_ON_CIPHER_CHANGE
 SSL,121,BUFFER_TOO_SMALL
+SSL,272,CANNOT_PARSE_LEAF_CERT
 SSL,122,CA_DN_LENGTH_MISMATCH
 SSL,123,CA_DN_TOO_LONG
 SSL,124,CCS_RECEIVED_EARLY

include/openssl/ssl.h

@@ -4587,6 +4587,7 @@ BORINGSSL_MAKE_DELETER(SSL_SESSION, SSL_SESSION_free)
 #define SSL_R_INVALID_SCT_LIST 269
 #define SSL_R_TOO_MUCH_SKIPPED_EARLY_DATA 270
 #define SSL_R_PSK_IDENTITY_BINDER_COUNT_MISMATCH 271
+#define SSL_R_CANNOT_PARSE_LEAF_CERT 272
 #define SSL_R_SSLV3_ALERT_CLOSE_NOTIFY 1000
 #define SSL_R_SSLV3_ALERT_UNEXPECTED_MESSAGE 1010
 #define SSL_R_SSLV3_ALERT_BAD_RECORD_MAC 1020

ssl/handshake_client.c

@@ -1044,8 +1044,10 @@ static int ssl3_get_server_certificate(SSL_HANDSHAKE *hs) {
 
   uint8_t alert;
   sk_CRYPTO_BUFFER_pop_free(ssl->s3->new_session->certs, CRYPTO_BUFFER_free);
-  ssl->s3->new_session->certs =
-      ssl_parse_cert_chain(&alert, NULL, &cbs, ssl->ctx->pool);
+  EVP_PKEY_free(hs->peer_pubkey);
+  hs->peer_pubkey = NULL;
+  ssl->s3->new_session->certs = ssl_parse_cert_chain(
+      &alert, &hs->peer_pubkey, NULL, &cbs, ssl->ctx->pool);
   if (ssl->s3->new_session->certs == NULL) {
     ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
     return -1;
@@ -1123,7 +1125,6 @@ static int ssl3_verify_server_cert(SSL_HANDSHAKE *hs) {
 static int ssl3_get_server_key_exchange(SSL_HANDSHAKE *hs) {
   SSL *const ssl = hs->ssl;
   int al;
-  EVP_PKEY *pkey = NULL;
   DH *dh = NULL;
   EC_KEY *ecdh = NULL;
   EC_POINT *srvr_ecpoint = NULL;
@@ -1276,11 +1277,6 @@ static int ssl3_get_server_key_exchange(SSL_HANDSHAKE *hs) {
 
   /* ServerKeyExchange should be signed by the server's public key. */
   if (ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
-    pkey = X509_get_pubkey(ssl->s3->new_session->x509_peer);
-    if (pkey == NULL) {
-      goto err;
-    }
-
     uint16_t signature_algorithm = 0;
     if (ssl3_protocol_version(ssl) >= TLS1_2_VERSION) {
       if (!CBS_get_u16(&server_key_exchange, &signature_algorithm)) {
@@ -1292,9 +1288,9 @@ static int ssl3_get_server_key_exchange(SSL_HANDSHAKE *hs) {
         goto f_err;
       }
       ssl->s3->tmp.peer_signature_algorithm = signature_algorithm;
-    } else if (pkey->type == EVP_PKEY_RSA) {
+    } else if (hs->peer_pubkey->type == EVP_PKEY_RSA) {
       signature_algorithm = SSL_SIGN_RSA_PKCS1_MD5_SHA1;
-    } else if (pkey->type == EVP_PKEY_EC) {
+    } else if (hs->peer_pubkey->type == EVP_PKEY_EC) {
       signature_algorithm = SSL_SIGN_ECDSA_SHA1;
     } else {
       al = SSL_AD_UNSUPPORTED_CERTIFICATE;
@@ -1327,7 +1323,7 @@ static int ssl3_get_server_key_exchange(SSL_HANDSHAKE *hs) {
 
     int sig_ok = ssl_public_key_verify(
         ssl, CBS_data(&signature), CBS_len(&signature), signature_algorithm,
-        pkey, transcript_data, transcript_len);
+        hs->peer_pubkey, transcript_data, transcript_len);
     OPENSSL_free(transcript_data);
 
 #if defined(BORINGSSL_UNSAFE_FUZZER_MODE)
@@ -1350,13 +1346,11 @@ static int ssl3_get_server_key_exchange(SSL_HANDSHAKE *hs) {
       goto f_err;
     }
   }
-  EVP_PKEY_free(pkey);
   return 1;
 
 f_err:
   ssl3_send_alert(ssl, SSL3_AL_FATAL, al);
 err:
-  EVP_PKEY_free(pkey);
   DH_free(dh);
   EC_POINT_free(srvr_ecpoint);
   EC_KEY_free(ecdh);
@@ -1557,20 +1551,12 @@ static int ssl3_send_client_key_exchange(SSL_HANDSHAKE *hs) {
       goto err;
     }
 
-    EVP_PKEY *pkey = X509_get_pubkey(ssl->s3->new_session->x509_peer);
-    if (pkey == NULL) {
-      goto err;
-    }
-
-    RSA *rsa = EVP_PKEY_get0_RSA(pkey);
+    RSA *rsa = EVP_PKEY_get0_RSA(hs->peer_pubkey);
     if (rsa == NULL) {
       OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
-      EVP_PKEY_free(pkey);
       goto err;
     }
 
-    EVP_PKEY_free(pkey);
-
     pms[0] = hs->client_version >> 8;
     pms[1] = hs->client_version & 0xff;
     if (!RAND_bytes(&pms[2], SSL_MAX_MASTER_KEY_LENGTH - 2)) {

ssl/handshake_server.c

@@ -1468,11 +1468,14 @@ static int ssl3_get_client_certificate(SSL_HANDSHAKE *hs) {
   CBS_init(&certificate_msg, ssl->init_msg, ssl->init_num);
 
   sk_CRYPTO_BUFFER_pop_free(ssl->s3->new_session->certs, CRYPTO_BUFFER_free);
+  EVP_PKEY_free(hs->peer_pubkey);
+  hs->peer_pubkey = NULL;
   uint8_t alert;
   ssl->s3->new_session->certs =
-      ssl_parse_cert_chain(&alert, ssl->retain_only_sha256_of_client_certs
-                                       ? ssl->s3->new_session->peer_sha256
-                                       : NULL,
+      ssl_parse_cert_chain(&alert, &hs->peer_pubkey,
+                           ssl->retain_only_sha256_of_client_certs
+                               ? ssl->s3->new_session->peer_sha256
+                               : NULL,
                            &certificate_msg, ssl->ctx->pool);
   if (ssl->s3->new_session->certs == NULL) {
     ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
@@ -1794,15 +1797,13 @@ err:
 
 static int ssl3_get_cert_verify(SSL_HANDSHAKE *hs) {
   SSL *const ssl = hs->ssl;
-  int al, ret = 0;
+  int al;
   CBS certificate_verify, signature;
-  X509 *peer = ssl->s3->new_session->x509_peer;
-  EVP_PKEY *pkey = NULL;
 
   /* Only RSA and ECDSA client certificates are supported, so a
    * CertificateVerify is required if and only if there's a client certificate.
    * */
-  if (peer == NULL) {
+  if (hs->peer_pubkey == NULL) {
     ssl3_free_handshake_buffer(ssl);
     return 1;
   }
@@ -1813,12 +1814,6 @@ static int ssl3_get_cert_verify(SSL_HANDSHAKE *hs) {
     return msg_ret;
   }
 
-  /* Filter out unsupported certificate types. */
-  pkey = X509_get_pubkey(peer);
-  if (pkey == NULL) {
-    goto err;
-  }
-
   CBS_init(&certificate_verify, ssl->init_msg, ssl->init_num);
 
   /* Determine the digest type if needbe. */
@@ -1833,9 +1828,9 @@ static int ssl3_get_cert_verify(SSL_HANDSHAKE *hs) {
       goto f_err;
     }
     ssl->s3->tmp.peer_signature_algorithm = signature_algorithm;
-  } else if (pkey->type == EVP_PKEY_RSA) {
+  } else if (hs->peer_pubkey->type == EVP_PKEY_RSA) {
     signature_algorithm = SSL_SIGN_RSA_PKCS1_MD5_SHA1;
-  } else if (pkey->type == EVP_PKEY_EC) {
+  } else if (hs->peer_pubkey->type == EVP_PKEY_EC) {
     signature_algorithm = SSL_SIGN_ECDSA_SHA1;
   } else {
     al = SSL_AD_UNSUPPORTED_CERTIFICATE;
@@ -1863,7 +1858,7 @@ static int ssl3_get_cert_verify(SSL_HANDSHAKE *hs) {
       goto err;
     }
 
-    EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new(pkey, NULL);
+    EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new(hs->peer_pubkey, NULL);
     sig_ok = pctx != NULL &&
              EVP_PKEY_verify_init(pctx) &&
              EVP_PKEY_CTX_set_signature_md(pctx, md) &&
@@ -1873,7 +1868,7 @@ static int ssl3_get_cert_verify(SSL_HANDSHAKE *hs) {
   } else {
     sig_ok = ssl_public_key_verify(
         ssl, CBS_data(&signature), CBS_len(&signature), signature_algorithm,
-        pkey, (const uint8_t *)ssl->s3->handshake_buffer->data,
+        hs->peer_pubkey, (const uint8_t *)ssl->s3->handshake_buffer->data,
         ssl->s3->handshake_buffer->length);
   }
 
@@ -1894,17 +1889,12 @@ static int ssl3_get_cert_verify(SSL_HANDSHAKE *hs) {
     goto err;
   }
 
-  ret = 1;
-
-  if (0) {
-  f_err:
-    ssl3_send_alert(ssl, SSL3_AL_FATAL, al);
-  }
+  return 1;
 
+f_err:
+  ssl3_send_alert(ssl, SSL3_AL_FATAL, al);
 err:
-  EVP_PKEY_free(pkey);
-
-  return ret;
+  return 0;
 }
 
 /* ssl3_get_next_proto reads a Next Protocol Negotiation handshake message. It

ssl/internal.h

@@ -757,10 +757,15 @@ int ssl_session_x509_cache_objects(SSL_SESSION *sess);
 /* ssl_parse_cert_chain parses a certificate list from |cbs| in the format used
  * by a TLS Certificate message. On success, it returns a newly-allocated
  * |CRYPTO_BUFFER| list and advances |cbs|. Otherwise, it returns NULL and sets
- * |*out_alert| to an alert to send to the peer. If the list is non-empty and
- * |out_leaf_sha256| is non-NULL, it writes the SHA-256 hash of the leaf to
- * |out_leaf_sha256|. */
+ * |*out_alert| to an alert to send to the peer.
+ *
+ * If the list is non-empty then |*out_pubkey| will be set to a freshly
+ * allocated public-key from the leaf certificate.
+ *
+ * If the list is non-empty and |out_leaf_sha256| is non-NULL, it writes the
+ * SHA-256 hash of the leaf to |out_leaf_sha256|. */
 STACK_OF(CRYPTO_BUFFER) *ssl_parse_cert_chain(uint8_t *out_alert,
+                                              EVP_PKEY **out_pubkey,
                                               uint8_t *out_leaf_sha256,
                                               CBS *cbs,
                                               CRYPTO_BUFFER_POOL *pool);
@@ -774,6 +779,11 @@ int ssl_add_cert_to_cbb(CBB *cbb, X509 *x509);
  * empty certificate list. It returns one on success and zero on error. */
 int ssl_add_cert_chain(SSL *ssl, CBB *cbb);
 
+/* ssl_cert_parse_pubkey extracts the public key from the DER-encoded, X.509
+ * certificate in |in|. It returns an allocated |EVP_PKEY| or else returns NULL
+ * and pushes to the error queue. */
+EVP_PKEY *ssl_cert_parse_pubkey(const CBS *in);
+
 /* ssl_parse_client_CA_list parses a CA list from |cbs| in the format used by a
  * TLS CertificateRequest message. On success, it returns a newly-allocated
  * |X509_NAME| list and advances |cbs|. Otherwise, it returns NULL and sets
@@ -980,6 +990,9 @@ struct ssl_handshake_st {
   /* hostname, on the server, is the value of the SNI extension. */
   char *hostname;
 
+  /* peer_pubkey is the public key parsed from the peer's leaf certificate. */
+  EVP_PKEY *peer_pubkey;
+
   /* key_block is the record-layer key block for TLS 1.2 and earlier. */
   uint8_t *key_block;
   uint8_t key_block_len;

ssl/s3_both.c

@@ -171,6 +171,7 @@ void ssl_handshake_free(SSL_HANDSHAKE *hs) {
   }
 
   OPENSSL_free(hs->hostname);
+  EVP_PKEY_free(hs->peer_pubkey);
   OPENSSL_free(hs);
 }
 

ssl/ssl_cert.c

@@ -120,8 +120,9 @@
 
 #include <openssl/bn.h>
 #include <openssl/buf.h>
-#include <openssl/ec_key.h>
+#include <openssl/bytestring.h>
 #include <openssl/dh.h>
+#include <openssl/ec_key.h>
 #include <openssl/err.h>
 #include <openssl/mem.h>
 #include <openssl/sha.h>
@@ -463,9 +464,12 @@ X509 *ssl_parse_x509(CBS *cbs) {
 }
 
 STACK_OF(CRYPTO_BUFFER) *ssl_parse_cert_chain(uint8_t *out_alert,
+                                              EVP_PKEY **out_pubkey,
                                               uint8_t *out_leaf_sha256,
                                               CBS *cbs,
                                               CRYPTO_BUFFER_POOL *pool) {
+  *out_pubkey = NULL;
+
   STACK_OF(CRYPTO_BUFFER) *ret = sk_CRYPTO_BUFFER_new_null();
   if (ret == NULL) {
     *out_alert = SSL_AD_INTERNAL_ERROR;
@@ -489,9 +493,16 @@ STACK_OF(CRYPTO_BUFFER) *ssl_parse_cert_chain(uint8_t *out_alert,
       goto err;
     }
 
-    /* Retain the hash of the leaf certificate if requested. */
-    if (sk_CRYPTO_BUFFER_num(ret) == 0 && out_leaf_sha256 != NULL) {
-      SHA256(CBS_data(&certificate), CBS_len(&certificate), out_leaf_sha256);
+    if (sk_CRYPTO_BUFFER_num(ret) == 0) {
+      *out_pubkey = ssl_cert_parse_pubkey(&certificate);
+      if (*out_pubkey == NULL) {
+        goto err;
+      }
+
+      /* Retain the hash of the leaf certificate if requested. */
+      if (out_leaf_sha256 != NULL) {
+        SHA256(CBS_data(&certificate), CBS_len(&certificate), out_leaf_sha256);
+      }
     }
 
     CRYPTO_BUFFER *buf =
@@ -512,6 +523,8 @@ STACK_OF(CRYPTO_BUFFER) *ssl_parse_cert_chain(uint8_t *out_alert,
   return ret;
 
 err:
+  EVP_PKEY_free(*out_pubkey);
+  *out_pubkey = NULL;
   sk_CRYPTO_BUFFER_pop_free(ret, CRYPTO_BUFFER_free);
   return NULL;
 }
@@ -594,6 +607,50 @@ int ssl_add_cert_chain(SSL *ssl, CBB *cbb) {
   return CBB_flush(cbb);
 }
 
+EVP_PKEY *ssl_cert_parse_pubkey(const CBS *in) {
+  /* From RFC 5280, section 4.1
+   *    Certificate  ::=  SEQUENCE  {
+   *      tbsCertificate       TBSCertificate,
+   *      signatureAlgorithm   AlgorithmIdentifier,
+   *      signatureValue       BIT STRING  }
+
+   * TBSCertificate  ::=  SEQUENCE  {
+   *      version         [0]  EXPLICIT Version DEFAULT v1,
+   *      serialNumber         CertificateSerialNumber,
+   *      signature            AlgorithmIdentifier,
+   *      issuer               Name,
+   *      validity             Validity,
+   *      subject              Name,
+   *      subjectPublicKeyInfo SubjectPublicKeyInfo,
+   *      ... } */
+  CBS buf = *in;
+
+  CBS toplevel, tbs_cert, spki;
+  if (!CBS_get_asn1(&buf, &toplevel, CBS_ASN1_SEQUENCE) ||
+      CBS_len(&buf) != 0 ||
+      !CBS_get_asn1(&toplevel, &tbs_cert, CBS_ASN1_SEQUENCE) ||
+      /* version */
+      !CBS_get_optional_asn1(
+          &tbs_cert, NULL, NULL,
+          CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 0) ||
+      /* serialNumber */
+      !CBS_get_asn1(&tbs_cert, NULL, CBS_ASN1_INTEGER) ||
+      /* signature algorithm */
+      !CBS_get_asn1(&tbs_cert, NULL, CBS_ASN1_SEQUENCE) ||
+      /* issuer */
+      !CBS_get_asn1(&tbs_cert, NULL, CBS_ASN1_SEQUENCE) ||
+      /* validity */
+      !CBS_get_asn1(&tbs_cert, NULL, CBS_ASN1_SEQUENCE) ||
+      /* subject */
+      !CBS_get_asn1(&tbs_cert, NULL, CBS_ASN1_SEQUENCE) ||
+      !CBS_get_asn1_element(&tbs_cert, &spki, CBS_ASN1_SEQUENCE)) {
+    OPENSSL_PUT_ERROR(SSL, SSL_R_CANNOT_PARSE_LEAF_CERT);
+    return NULL;
+  }
+
+  return EVP_parse_public_key(&spki);
+}
+
 static int ca_dn_cmp(const X509_NAME **a, const X509_NAME **b) {
   return X509_NAME_cmp(*a, *b);
 }