commit 68e7124ddf3680302650b7c10cccdef039a72202
author Adam Langley <agl@google.com> Mon Dec 12 11:06:16 2016 -0800
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Tue Dec 13 17:28:25 2016 +0000
tree a30d60e2d3e066b2e73ddf07ff739f641befbec0
parent 364f7a6d21f73d099f4f80800973174a8f71e90f

Hold certificates in an SSL_SESSION as CRYPTO_BUFFERSs as well.

This change adds a STACK_OF(CRYPTO_BUFFER) to an SSL_SESSION which
contains the raw form of the received certificates. The X509-based
members still exist, but their |enc| buffer will alias the
CRYPTO_BUFFERs.

(This is a second attempt at
https://boringssl-review.googlesource.com/#/c/12163/.)

BUG=chromium:671420

Change-Id: I508a8a46cab89a5a3fcc0c1224185d63e3d59cb8
Reviewed-on: https://boringssl-review.googlesource.com/12705
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>

ssl/tls13_both.c

diff --git a/ssl/tls13_both.c b/ssl/tls13_both.c
index 939e530..a47dd26 100644
--- a/ssl/tls13_both.c
+++ b/ssl/tls13_both.c
@@ -177,8 +177,8 @@
       ssl->server && ssl->retain_only_sha256_of_client_certs;
   int ret = 0;
 
-  STACK_OF(X509) *chain = sk_X509_new_null();
-  if (chain == 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);
     OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
     goto err;
@@ -193,28 +193,25 @@
   while (CBS_len(&certificate_list) > 0) {
     CBS certificate, extensions;
     if (!CBS_get_u24_length_prefixed(&certificate_list, &certificate) ||
-        !CBS_get_u16_length_prefixed(&certificate_list, &extensions)) {
+        !CBS_get_u16_length_prefixed(&certificate_list, &extensions) ||
+        CBS_len(&certificate) == 0) {
       ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
       OPENSSL_PUT_ERROR(SSL, SSL_R_CERT_LENGTH_MISMATCH);
       goto err;
     }
 
     /* Retain the hash of the leaf certificate if requested. */
-    if (sk_X509_num(chain) == 0 && retain_sha256) {
+    if (sk_CRYPTO_BUFFER_num(certs) == 0 && retain_sha256) {
       SHA256(CBS_data(&certificate), CBS_len(&certificate),
              ssl->s3->new_session->peer_sha256);
     }
 
-    X509 *x = ssl_parse_x509(&certificate);
-    if (x == NULL || CBS_len(&certificate) != 0) {
-      ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
-      X509_free(x);
-      goto err;
-    }
-    if (!sk_X509_push(chain, x)) {
+    CRYPTO_BUFFER *buf = CRYPTO_BUFFER_new_from_CBS(&certificate, NULL);
+    if (buf == NULL ||
+        !sk_CRYPTO_BUFFER_push(certs, buf)) {
+      CRYPTO_BUFFER_free(buf);
       ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
       OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
-      X509_free(x);
       goto err;
     }
 
@@ -253,7 +250,7 @@
         goto err;
       }
 
-      if (sk_X509_num(chain) == 1 &&
+      if (sk_CRYPTO_BUFFER_num(certs) == 1 &&
           !CBS_stow(&ocsp_response, &ssl->s3->new_session->ocsp_response,
                     &ssl->s3->new_session->ocsp_response_length)) {
         ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
@@ -274,7 +271,7 @@
         goto err;
       }
 
-      if (sk_X509_num(chain) == 1 &&
+      if (sk_CRYPTO_BUFFER_num(certs) == 1 &&
           !CBS_stow(&sct,
                     &ssl->s3->new_session->tlsext_signed_cert_timestamp_list,
                     &ssl->s3->new_session
@@ -291,7 +288,17 @@
     goto err;
   }
 
-  if (sk_X509_num(chain) == 0) {
+  sk_CRYPTO_BUFFER_pop_free(ssl->s3->new_session->certs, CRYPTO_BUFFER_free);
+  ssl->s3->new_session->certs = certs;
+  certs = NULL;
+
+  if (!ssl_session_x509_cache_objects(ssl->s3->new_session)) {
+    OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+    ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+    goto err;
+  }
+
+  if (sk_CRYPTO_BUFFER_num(ssl->s3->new_session->certs) == 0) {
     if (!allow_anonymous) {
       OPENSSL_PUT_ERROR(SSL, SSL_R_PEER_DID_NOT_RETURN_A_CERTIFICATE);
       ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_CERTIFICATE_REQUIRED);
@@ -310,25 +317,14 @@
   ssl->s3->new_session->peer_sha256_valid = retain_sha256;
 
   if (!ssl_verify_cert_chain(ssl, &ssl->s3->new_session->verify_result,
-                             chain)) {
+                             ssl->s3->new_session->x509_chain)) {
     goto err;
   }
 
-  X509_free(ssl->s3->new_session->x509_peer);
-  X509 *leaf = sk_X509_value(chain, 0);
-  X509_up_ref(leaf);
-  ssl->s3->new_session->x509_peer = leaf;
-
-  sk_X509_pop_free(ssl->s3->new_session->x509_chain, X509_free);
-  ssl->s3->new_session->x509_chain = chain;
-  chain = NULL;
-  sk_X509_pop_free(ssl->s3->new_session->x509_chain_without_leaf, X509_free);
-  ssl->s3->new_session->x509_chain_without_leaf = NULL;
-
   ret = 1;
 
 err:
-  sk_X509_pop_free(chain, X509_free);
+  sk_CRYPTO_BUFFER_pop_free(certs, CRYPTO_BUFFER_free);
   return ret;
 }