Tidy up a few certificate-related utility functions.

These will all want to be shared with the TLS 1.3 handshake.

Change-Id: I4e50dc0ed2295d43c7ae800015d71c1406311801
Reviewed-on: https://boringssl-review.googlesource.com/8776
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

5 files changed, 64 insertions, 46 deletions

diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index 0f9a26e5..89d52dca 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -1503,12 +1503,6 @@ static int ssl3_get_server_hello_done(SSL *ssl) {
   return 1;
 }
 
-/* ssl3_has_client_certificate returns true if a client certificate is
- * configured. */
-static int ssl3_has_client_certificate(SSL *ssl) {
-  return ssl->cert && ssl->cert->x509 && ssl_has_private_key(ssl);
-}
-
 static int ssl_do_client_cert_cb(SSL *ssl, X509 **out_x509,
                                  EVP_PKEY **out_pkey) {
   if (ssl->ctx->client_cert_cb == NULL) {
@@ -1540,7 +1534,7 @@ static int ssl3_send_client_certificate(SSL *ssl) {
       }
     }
 
-    if (ssl3_has_client_certificate(ssl)) {
+    if (ssl_has_certificate(ssl)) {
       ssl->state = SSL3_ST_CW_CERT_C;
     } else {
       ssl->state = SSL3_ST_CW_CERT_B;
@@ -1570,7 +1564,7 @@ static int ssl3_send_client_certificate(SSL *ssl) {
   }
 
   if (ssl->state == SSL3_ST_CW_CERT_C) {
-    if (!ssl3_has_client_certificate(ssl)) {
+    if (!ssl_has_certificate(ssl)) {
       ssl->s3->tmp.cert_request = 0;
       /* Without a client certificate, the handshake buffer may be released. */
       ssl3_free_handshake_buffer(ssl);
@@ -1580,14 +1574,9 @@ static int ssl3_send_client_certificate(SSL *ssl) {
         ssl3_send_alert(ssl, SSL3_AL_WARNING, SSL_AD_NO_CERTIFICATE);
         return 1;
       }
+    }
 
-      CBB cbb, body;
-      if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_CERTIFICATE) ||
-          !CBB_add_u24(&body, 0 /* no certificates */) ||
-          !ssl->method->finish_message(ssl, &cbb)) {
-        return -1;
-      }
-    } else if (!ssl3_output_cert_chain(ssl)) {
+    if (!ssl3_output_cert_chain(ssl)) {
       return -1;
     }
     ssl->state = SSL3_ST_CW_CERT_D;
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index c6be07e8..c041bb82 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -922,6 +922,11 @@ static int ssl3_send_server_certificate(SSL *ssl) {
     return ssl->method->write_message(ssl);
   }
 
+  if (!ssl_has_certificate(ssl)) {
+    OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CERTIFICATE_SET);
+    return 0;
+  }
+
   if (!ssl3_output_cert_chain(ssl)) {
     return 0;
   }
@@ -1187,7 +1192,7 @@ static int ssl3_send_certificate_request(SSL *ssl) {
     return ssl->method->write_message(ssl);
   }
 
-  CBB cbb, body, cert_types, sigalgs_cbb, names_cbb, name_cbb;
+  CBB cbb, body, cert_types, sigalgs_cbb;
   if (!ssl->method->init_message(ssl, &cbb, &body,
                                  SSL3_MT_CERTIFICATE_REQUEST) ||
       !CBB_add_u8_length_prefixed(&body, &cert_types) ||
@@ -1210,29 +1215,8 @@ static int ssl3_send_certificate_request(SSL *ssl) {
     }
   }
 
-  if (!CBB_add_u16_length_prefixed(&body, &names_cbb)) {
-    goto err;
-  }
-
-  STACK_OF(X509_NAME) *sk = SSL_get_client_CA_list(ssl);
-  if (sk != NULL) {
-    size_t i;
-    for (i = 0; i < sk_X509_NAME_num(sk); i++) {
-      X509_NAME *name = sk_X509_NAME_value(sk, i);
-      int len = i2d_X509_NAME(name, NULL);
-      if (len < 0) {
-        goto err;
-      }
-      uint8_t *ptr;
-      if (!CBB_add_u16_length_prefixed(&names_cbb, &name_cbb) ||
-          !CBB_add_space(&name_cbb, &ptr, (size_t)len) ||
-          (len > 0 && i2d_X509_NAME(name, &ptr) < 0)) {
-        goto err;
-      }
-    }
-  }
-
-  if (!ssl->method->finish_message(ssl, &cbb)) {
+  if (!ssl_add_client_CA_list(ssl, &body) ||
+      !ssl->method->finish_message(ssl, &cbb)) {
     goto err;
   }
 
diff --git a/ssl/internal.h b/ssl/internal.h
index 4e1c4580..359191de 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -466,7 +466,7 @@ enum ssl_open_record_t ssl_process_alert(SSL *ssl, uint8_t *out_alert,
 
 /* ssl_has_private_key returns one if |ssl| has a private key
  * configured and zero otherwise. */
-int ssl_has_private_key(SSL *ssl);
+int ssl_has_private_key(const SSL *ssl);
 
 /* ssl_private_key_* call the corresponding function on the
  * |SSL_PRIVATE_KEY_METHOD| for |ssl|, if configured. Otherwise, they implement
@@ -734,10 +734,24 @@ void ssl_write_buffer_clear(SSL *ssl);
 
 /* Certificate functions. */
 
+/* ssl_has_certificate returns one if a certificate and private key are
+ * configured and zero otherwise. */
+int ssl_has_certificate(const SSL *ssl);
+
 /* ssl_add_cert_to_cbb adds |x509| to |cbb|. It returns one on success and zero
  * on error. */
 int ssl_add_cert_to_cbb(CBB *cbb, X509 *x509);
 
+/* ssl_add_cert_chain adds |ssl|'s certificate chain to |cbb| in the format used
+ * by a TLS Certificate message. If there is no certificate chain, it emits an
+ * empty certificate list. It returns one on success and zero on error. */
+int ssl_add_cert_chain(SSL *ssl, CBB *cbb);
+
+/* ssl_add_client_CA_list adds the configured CA list to |cbb| in the format
+ * used by a TLS CertificateRequest message. It returns one on success and zero
+ * on error. */
+int ssl_add_client_CA_list(SSL *ssl, CBB *cbb);
+
 
 /* Underdocumented functions.
  *
@@ -1014,7 +1028,6 @@ void ssl_cert_set_cert_cb(CERT *cert,
                           int (*cb)(SSL *ssl, void *arg), void *arg);
 
 int ssl_verify_cert_chain(SSL *ssl, STACK_OF(X509) *cert_chain);
-int ssl_add_cert_chain(SSL *ssl, CBB *cbb);
 void ssl_update_cache(SSL *ssl, int mode);
 
 /* ssl_get_compatible_server_ciphers determines the key exchange and
diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index dead3ba7..75f6ce00 100644
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -417,6 +417,10 @@ int SSL_CTX_add_client_CA(SSL_CTX *ctx, X509 *x509) {
   return add_client_CA(&ctx->client_CA, x509);
 }
 
+int ssl_has_certificate(const SSL *ssl) {
+  return ssl->cert->x509 != NULL && ssl_has_private_key(ssl);
+}
+
 int ssl_add_cert_to_cbb(CBB *cbb, X509 *x509) {
   int len = i2d_X509(x509, NULL);
   if (len < 0) {
@@ -441,12 +445,12 @@ static int ssl_add_cert_with_length(CBB *cbb, X509 *x509) {
 }
 
 int ssl_add_cert_chain(SSL *ssl, CBB *cbb) {
+  if (!ssl_has_certificate(ssl)) {
+    return CBB_add_u24(cbb, 0);
+  }
+
   CERT *cert = ssl->cert;
   X509 *x = cert->x509;
-  if (x == NULL) {
-    OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CERTIFICATE_SET);
-    return 0;
-  }
 
   CBB child;
   if (!CBB_add_u24_length_prefixed(cbb, &child)) {
@@ -497,6 +501,34 @@ int ssl_add_cert_chain(SSL *ssl, CBB *cbb) {
   return CBB_flush(cbb);
 }
 
+int ssl_add_client_CA_list(SSL *ssl, CBB *cbb) {
+  CBB child, name_cbb;
+  if (!CBB_add_u16_length_prefixed(cbb, &child)) {
+    return 0;
+  }
+
+  STACK_OF(X509_NAME) *sk = SSL_get_client_CA_list(ssl);
+  if (sk == NULL) {
+    return CBB_flush(cbb);
+  }
+
+  for (size_t i = 0; i < sk_X509_NAME_num(sk); i++) {
+    X509_NAME *name = sk_X509_NAME_value(sk, i);
+    int len = i2d_X509_NAME(name, NULL);
+    if (len < 0) {
+      return 0;
+    }
+    uint8_t *ptr;
+    if (!CBB_add_u16_length_prefixed(&child, &name_cbb) ||
+        !CBB_add_space(&name_cbb, &ptr, (size_t)len) ||
+        (len > 0 && i2d_X509_NAME(name, &ptr) < 0)) {
+      return 0;
+    }
+  }
+
+  return CBB_flush(cbb);
+}
+
 static int set_cert_store(X509_STORE **store_ptr, X509_STORE *new_store, int take_ref) {
   X509_STORE_free(*store_ptr);
   *store_ptr = new_store;
diff --git a/ssl/ssl_rsa.c b/ssl/ssl_rsa.c
index 90834e5f..3e0894e4 100644
--- a/ssl/ssl_rsa.c
+++ b/ssl/ssl_rsa.c
@@ -383,7 +383,7 @@ int SSL_set_private_key_digest_prefs(SSL *ssl, const int *digest_nids,
   return 1;
 }
 
-int ssl_has_private_key(SSL *ssl) {
+int ssl_has_private_key(const SSL *ssl) {
   return ssl->cert->privatekey != NULL || ssl->cert->key_method != NULL;
 }