Fold ssl_add_cert_chain into its caller

I'm not sure why we pulled that out separately. Also remove the
ERR_R_INTERNAL_ERRORs. Those are a remnant of when CBB did not
participate in the error queue and we wanted to leave something there.

Change-Id: Ic7db602ddce6e6fa873c892f742126d9a628494c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66547
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

5 files changed, 25 insertions, 41 deletions

diff --git a/ssl/handshake.cc b/ssl/handshake.cc
index ceb8eac..2a4fb11 100644
--- a/ssl/handshake.cc
+++ b/ssl/handshake.cc
@@ -563,18 +563,28 @@ bool ssl_send_finished(SSL_HANDSHAKE *hs) {
   return true;
 }
 
-bool ssl_output_cert_chain(SSL_HANDSHAKE *hs) {
+bool ssl_send_tls12_certificate(SSL_HANDSHAKE *hs) {
   ScopedCBB cbb;
-  CBB body;
+  CBB body, certs, cert;
   if (!hs->ssl->method->init_message(hs->ssl, cbb.get(), &body,
                                      SSL3_MT_CERTIFICATE) ||
-      !ssl_add_cert_chain(hs, &body) ||
-      !ssl_add_message_cbb(hs->ssl, cbb.get())) {
-    OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
+      !CBB_add_u24_length_prefixed(&body, &certs)) {
     return false;
   }
 
-  return true;
+  if (ssl_has_certificate(hs)) {
+    STACK_OF(CRYPTO_BUFFER) *chain = hs->config->cert->chain.get();
+    for (size_t i = 0; i < sk_CRYPTO_BUFFER_num(chain); i++) {
+      CRYPTO_BUFFER *buffer = sk_CRYPTO_BUFFER_value(chain, i);
+      if (!CBB_add_u24_length_prefixed(&certs, &cert) ||
+          !CBB_add_bytes(&cert, CRYPTO_BUFFER_data(buffer),
+                         CRYPTO_BUFFER_len(buffer))) {
+        return false;
+      }
+    }
+  }
+
+  return ssl_add_message_cbb(hs->ssl, cbb.get());
 }
 
 const SSL_SESSION *ssl_handshake_session(const SSL_HANDSHAKE *hs) {
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index 971ebd0..102fa85 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -1364,7 +1364,7 @@ static enum ssl_hs_wait_t do_send_client_certificate(SSL_HANDSHAKE *hs) {
   }
 
   if (!ssl_on_certificate_selected(hs) ||
-      !ssl_output_cert_chain(hs)) {
+      !ssl_send_tls12_certificate(hs)) {
     return ssl_hs_error;
   }
 
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc
index 7c84ef8..661aeda 100644
--- a/ssl/handshake_server.cc
+++ b/ssl/handshake_server.cc
@@ -1067,7 +1067,7 @@ static enum ssl_hs_wait_t do_send_server_certificate(SSL_HANDSHAKE *hs) {
       return ssl_hs_error;
     }
 
-    if (!ssl_output_cert_chain(hs)) {
+    if (!ssl_send_tls12_certificate(hs)) {
       return ssl_hs_error;
     }
 
diff --git a/ssl/internal.h b/ssl/internal.h
index 9e7a05b..2dd7859 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1361,11 +1361,6 @@ bool ssl_parse_cert_chain(uint8_t *out_alert,
                           uint8_t *out_leaf_sha256, CBS *cbs,
                           CRYPTO_BUFFER_POOL *pool);
 
-// ssl_add_cert_chain adds |hs->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 true on success and false on error.
-bool ssl_add_cert_chain(SSL_HANDSHAKE *hs, CBB *cbb);
-
 enum ssl_key_usage_t {
   key_usage_digital_signature = 0,
   key_usage_encipherment = 2,
@@ -2314,8 +2309,14 @@ enum ssl_verify_result_t ssl_reverify_peer_cert(SSL_HANDSHAKE *hs,
                                                 bool send_alert);
 
 enum ssl_hs_wait_t ssl_get_finished(SSL_HANDSHAKE *hs);
+
+// ssl_send_finished adds a Finished message to the current flight of messages.
+// It returns true on success and false on error.
 bool ssl_send_finished(SSL_HANDSHAKE *hs);
-bool ssl_output_cert_chain(SSL_HANDSHAKE *hs);
+
+// ssl_send_tls12_certificate adds a TLS 1.2 Certificate message to the current
+// flight of messages. It returns true on success and false on error.
+bool ssl_send_tls12_certificate(SSL_HANDSHAKE *hs);
 
 // ssl_handshake_session returns the |SSL_SESSION| corresponding to the current
 // handshake. Note, in TLS 1.2 resumptions, this session is immutable.
diff --git a/ssl/ssl_cert.cc b/ssl/ssl_cert.cc
index dbc4818..17363f3 100644
--- a/ssl/ssl_cert.cc
+++ b/ssl/ssl_cert.cc
@@ -385,33 +385,6 @@ bool ssl_parse_cert_chain(uint8_t *out_alert,
   return true;
 }
 
-bool ssl_add_cert_chain(SSL_HANDSHAKE *hs, CBB *cbb) {
-  if (!ssl_has_certificate(hs)) {
-    return CBB_add_u24(cbb, 0);
-  }
-
-  CBB certs;
-  if (!CBB_add_u24_length_prefixed(cbb, &certs)) {
-    OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
-    return false;
-  }
-
-  STACK_OF(CRYPTO_BUFFER) *chain = hs->config->cert->chain.get();
-  for (size_t i = 0; i < sk_CRYPTO_BUFFER_num(chain); i++) {
-    CRYPTO_BUFFER *buffer = sk_CRYPTO_BUFFER_value(chain, i);
-    CBB child;
-    if (!CBB_add_u24_length_prefixed(&certs, &child) ||
-        !CBB_add_bytes(&child, CRYPTO_BUFFER_data(buffer),
-                       CRYPTO_BUFFER_len(buffer)) ||
-        !CBB_flush(&certs)) {
-      OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
-      return false;
-    }
-  }
-
-  return CBB_flush(cbb);
-}
-
 // ssl_cert_skip_to_spki parses a DER-encoded, X.509 certificate from |in| and
 // positions |*out_tbs_cert| to cover the TBSCertificate, starting at the
 // subjectPublicKeyInfo.