From 605641ed95b371bfc749a60f252d33844bcc5a8b Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@chromium.org>
Date: Sun, 3 May 2015 15:14:04 -0400
Subject: [PATCH] Move the NULL case in ssl_add_cert_chain up.

It's only called for client certificates with NULL. The interaction with
extra_certs is more obvious if we handle that case externally. (We
shouldn't attach extra_certs if there is no leaf.)

Change-Id: I9dc26f32f582be8c48a4da9aae0ceee8741813dc
Reviewed-on: https://boringssl-review.googlesource.com/4613
Reviewed-by: Adam Langley <agl@google.com>
---
 crypto/err/ssl.errordata |  1 +
 include/openssl/ssl.h    |  1 +
 ssl/s3_both.c            |  8 ++++-
 ssl/ssl_cert.c           | 71 ++++++++++++++++++----------------------
 4 files changed, 41 insertions(+), 40 deletions(-)

diff --git a/crypto/err/ssl.errordata b/crypto/err/ssl.errordata
index 0157072f..ee7b5a2d 100644
--- a/crypto/err/ssl.errordata
+++ b/crypto/err/ssl.errordata
@@ -103,6 +103,7 @@ SSL,function,192,ssl3_get_server_hello
 SSL,function,193,ssl3_get_server_key_exchange
 SSL,function,194,ssl3_get_v2_client_hello
 SSL,function,195,ssl3_handshake_mac
+SSL,function,275,ssl3_output_cert_chain
 SSL,function,196,ssl3_prf
 SSL,function,197,ssl3_read_bytes
 SSL,function,198,ssl3_read_n
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 4ee20843..12ce73f7 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2678,6 +2678,7 @@ OPENSSL_EXPORT const char *SSLeay_version(int unused);
 #define SSL_F_SSL_CTX_set1_tls_channel_id 272
 #define SSL_F_SSL_set1_tls_channel_id 273
 #define SSL_F_SSL_set_tlsext_host_name 274
+#define SSL_F_ssl3_output_cert_chain 275
 #define SSL_R_APP_DATA_IN_HANDSHAKE 100
 #define SSL_R_ATTEMPT_TO_REUSE_SESSION_IN_DIFFERENT_CONTEXT 101
 #define SSL_R_BAD_ALERT 102
diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index 889a7323..4dd9e526 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -305,7 +305,13 @@ int ssl3_output_cert_chain(SSL *s, CERT_PKEY *cpk) {
   uint8_t *p;
   unsigned long l = 3 + SSL_HM_HEADER_LENGTH(s);
 
-  if (!ssl_add_cert_chain(s, cpk, &l)) {
+  if (cpk == NULL) {
+    /* TLSv1 sends a chain with nothing in it, instead of an alert. */
+    if (!BUF_MEM_grow_clean(s->init_buf, 10)) {
+      OPENSSL_PUT_ERROR(SSL, ssl3_output_cert_chain, ERR_R_BUF_LIB);
+      return 0;
+    }
+  } else if (!ssl_add_cert_chain(s, cpk, &l)) {
     return 0;
   }
 
diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index fdab572a..3e8eaf17 100644
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -792,12 +792,13 @@ int ssl_add_cert_chain(SSL *s, CERT_PKEY *cpk, unsigned long *l) {
   int no_chain = 0;
   size_t i;
 
-  X509 *x = NULL;
-  STACK_OF(X509) * extra_certs;
+  X509 *x = cpk->x509;
+  STACK_OF(X509) *extra_certs;
   X509_STORE *chain_store;
 
-  if (cpk) {
-    x = cpk->x509;
+  if (x == NULL) {
+    OPENSSL_PUT_ERROR(SSL, ssl_add_cert_chain, SSL_R_NO_CERTIFICATE_SET);
+    return 0;
   }
 
   if (s->cert->chain_store) {
@@ -817,44 +818,36 @@ int ssl_add_cert_chain(SSL *s, CERT_PKEY *cpk, unsigned long *l) {
     no_chain = 1;
   }
 
-  /* TLSv1 sends a chain with nothing in it, instead of an alert. */
-  if (!BUF_MEM_grow_clean(buf, 10)) {
-    OPENSSL_PUT_ERROR(SSL, ssl_add_cert_chain, ERR_R_BUF_LIB);
-    return 0;
-  }
-
-  if (x != NULL) {
-    if (no_chain) {
-      if (!ssl_add_cert_to_buf(buf, l, x)) {
-        return 0;
-      }
-    } else {
-      X509_STORE_CTX xs_ctx;
-
-      if (!X509_STORE_CTX_init(&xs_ctx, chain_store, x, NULL)) {
-        OPENSSL_PUT_ERROR(SSL, ssl_add_cert_chain, ERR_R_X509_LIB);
-        return 0;
-      }
-      X509_verify_cert(&xs_ctx);
-      /* Don't leave errors in the queue */
-      ERR_clear_error();
-      for (i = 0; i < sk_X509_num(xs_ctx.chain); i++) {
-        x = sk_X509_value(xs_ctx.chain, i);
-
-        if (!ssl_add_cert_to_buf(buf, l, x)) {
-          X509_STORE_CTX_cleanup(&xs_ctx);
-          return 0;
-        }
-      }
-      X509_STORE_CTX_cleanup(&xs_ctx);
-    }
-  }
-
-  for (i = 0; i < sk_X509_num(extra_certs); i++) {
-    x = sk_X509_value(extra_certs, i);
+  if (no_chain) {
     if (!ssl_add_cert_to_buf(buf, l, x)) {
       return 0;
     }
+
+    for (i = 0; i < sk_X509_num(extra_certs); i++) {
+      x = sk_X509_value(extra_certs, i);
+      if (!ssl_add_cert_to_buf(buf, l, x)) {
+        return 0;
+      }
+    }
+  } else {
+    X509_STORE_CTX xs_ctx;
+
+    if (!X509_STORE_CTX_init(&xs_ctx, chain_store, x, NULL)) {
+      OPENSSL_PUT_ERROR(SSL, ssl_add_cert_chain, ERR_R_X509_LIB);
+      return 0;
+    }
+    X509_verify_cert(&xs_ctx);
+    /* Don't leave errors in the queue */
+    ERR_clear_error();
+    for (i = 0; i < sk_X509_num(xs_ctx.chain); i++) {
+      x = sk_X509_value(xs_ctx.chain, i);
+
+      if (!ssl_add_cert_to_buf(buf, l, x)) {
+        X509_STORE_CTX_cleanup(&xs_ctx);
+        return 0;
+      }
+    }
+    X509_STORE_CTX_cleanup(&xs_ctx);
   }
 
   return 1;