Slightly simplify ssl_x509.cc We've got a few too many of these set1/set0 wrappers. Change-Id: I4bde492b1a2a90a151b26800076d085f7122f623 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66607 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com>

diff --git a/ssl/ssl_x509.cc b/ssl/ssl_x509.cc
index 43672ce..06baf88 100644
--- a/ssl/ssl_x509.cc
+++ b/ssl/ssl_x509.cc

@@ -194,11 +194,21 @@
   return chain;
 }
 
-// ssl_cert_set_chain sets elements 1.. of |cert->chain| to the serialised
+static void ssl_crypto_x509_cert_flush_cached_leaf(CERT *cert) {
+  X509_free(cert->x509_leaf);
+  cert->x509_leaf = nullptr;
+}
+
+static void ssl_crypto_x509_cert_flush_cached_chain(CERT *cert) {
+  sk_X509_pop_free(cert->x509_chain, X509_free);
+  cert->x509_chain = nullptr;
+}
+
+// ssl_cert_set1_chain sets elements 1.. of |cert->chain| to the serialised
 // forms of elements of |chain|. It returns one on success or zero on error, in
 // which case no change to |cert->chain| is made. It preverses the existing
 // leaf from |cert->chain|, if any.
-static bool ssl_cert_set_chain(CERT *cert, STACK_OF(X509) *chain) {
+static bool ssl_cert_set1_chain(CERT *cert, STACK_OF(X509) *chain) {
   UniquePtr<STACK_OF(CRYPTO_BUFFER)> new_chain;
 
   if (cert->chain != nullptr) {
@@ -230,19 +240,10 @@
   }
 
   cert->chain = std::move(new_chain);
+  ssl_crypto_x509_cert_flush_cached_chain(cert);
   return true;
 }
 
-static void ssl_crypto_x509_cert_flush_cached_leaf(CERT *cert) {
-  X509_free(cert->x509_leaf);
-  cert->x509_leaf = nullptr;
-}
-
-static void ssl_crypto_x509_cert_flush_cached_chain(CERT *cert) {
-  sk_X509_pop_free(cert->x509_chain, X509_free);
-  cert->x509_chain = nullptr;
-}
-
 static bool ssl_crypto_x509_check_client_CA_list(
     STACK_OF(CRYPTO_BUFFER) *names) {
   for (const CRYPTO_BUFFER *buffer : names) {
@@ -446,8 +447,9 @@
 static bool ssl_crypto_x509_ssl_auto_chain_if_needed(SSL_HANDSHAKE *hs) {
   // Only build a chain if there are no intermediates configured and the feature
   // isn't disabled.
-  if ((hs->ssl->mode & SSL_MODE_NO_AUTO_CHAIN) ||
-      !ssl_has_certificate(hs) || hs->config->cert->chain == NULL ||
+  SSL *const ssl = hs->ssl;
+  if ((ssl->mode & SSL_MODE_NO_AUTO_CHAIN) || !ssl_has_certificate(hs) ||
+      hs->config->cert->chain == NULL ||
       sk_CRYPTO_BUFFER_num(hs->config->cert->chain.get()) > 1) {
     return true;
   }
@@ -460,8 +462,8 @@
   }
 
   UniquePtr<X509_STORE_CTX> ctx(X509_STORE_CTX_new());
-  if (!ctx || !X509_STORE_CTX_init(ctx.get(), hs->ssl->ctx->cert_store,
-                                   leaf.get(), nullptr)) {
+  if (!ctx || !X509_STORE_CTX_init(ctx.get(), ssl->ctx->cert_store, leaf.get(),
+                                   nullptr)) {
     OPENSSL_PUT_ERROR(SSL, ERR_R_X509_LIB);
     return false;
   }
@@ -477,13 +479,7 @@
   }
   X509_free(sk_X509_shift(chain.get()));
 
-  if (!ssl_cert_set_chain(hs->config->cert.get(), chain.get())) {
-    return false;
-  }
-
-  ssl_crypto_x509_cert_flush_cached_chain(hs->config->cert.get());
-
-  return true;
+  return SSL_set1_chain(ssl, chain.get());
 }
 
 static void ssl_crypto_x509_ssl_ctx_flush_cached_client_CA(SSL_CTX *ctx) {
@@ -795,26 +791,7 @@
   return ssl_cert_get0_leaf(ctx->cert.get());
 }
 
-static int ssl_cert_set0_chain(CERT *cert, STACK_OF(X509) *chain) {
-  if (!ssl_cert_set_chain(cert, chain)) {
-    return 0;
-  }
-
-  sk_X509_pop_free(chain, X509_free);
-  ssl_crypto_x509_cert_flush_cached_chain(cert);
-  return 1;
-}
-
-static int ssl_cert_set1_chain(CERT *cert, STACK_OF(X509) *chain) {
-  if (!ssl_cert_set_chain(cert, chain)) {
-    return 0;
-  }
-
-  ssl_crypto_x509_cert_flush_cached_chain(cert);
-  return 1;
-}
-
-static int ssl_cert_append_cert(CERT *cert, X509 *x509) {
+static int ssl_cert_add1_chain_cert(CERT *cert, X509 *x509) {
   assert(cert->x509_method);
 
   UniquePtr<CRYPTO_BUFFER> buffer = x509_to_buffer(x509);
@@ -822,43 +799,38 @@
     return 0;
   }
 
-  if (cert->chain != NULL) {
-    return PushToStack(cert->chain.get(), std::move(buffer));
+  if (cert->chain == nullptr) {
+    cert->chain = new_leafless_chain();
+    if (cert->chain == nullptr) {
+      return 0;
+    }
   }
 
-  cert->chain = new_leafless_chain();
-  if (!cert->chain ||
-      !PushToStack(cert->chain.get(), std::move(buffer))) {
-    cert->chain.reset();
+  if (!PushToStack(cert->chain.get(), std::move(buffer))) {
     return 0;
   }
 
+  ssl_crypto_x509_cert_flush_cached_chain(cert);
   return 1;
 }
 
 static int ssl_cert_add0_chain_cert(CERT *cert, X509 *x509) {
-  if (!ssl_cert_append_cert(cert, x509)) {
+  if (!ssl_cert_add1_chain_cert(cert, x509)) {
     return 0;
   }
 
   X509_free(cert->x509_stash);
   cert->x509_stash = x509;
-  ssl_crypto_x509_cert_flush_cached_chain(cert);
-  return 1;
-}
-
-static int ssl_cert_add1_chain_cert(CERT *cert, X509 *x509) {
-  if (!ssl_cert_append_cert(cert, x509)) {
-    return 0;
-  }
-
-  ssl_crypto_x509_cert_flush_cached_chain(cert);
   return 1;
 }
 
 int SSL_CTX_set0_chain(SSL_CTX *ctx, STACK_OF(X509) *chain) {
   check_ssl_ctx_x509_method(ctx);
-  return ssl_cert_set0_chain(ctx->cert.get(), chain);
+  if (!ssl_cert_set1_chain(ctx->cert.get(), chain)) {
+    return 0;
+  }
+  sk_X509_pop_free(chain, X509_free);
+  return 1;
 }
 
 int SSL_CTX_set1_chain(SSL_CTX *ctx, STACK_OF(X509) *chain) {
@@ -871,7 +843,11 @@
   if (!ssl->config) {
     return 0;
   }
-  return ssl_cert_set0_chain(ssl->config->cert.get(), chain);
+  if (!ssl_cert_set1_chain(ssl->config->cert.get(), chain)) {
+    return 0;
+  }
+  sk_X509_pop_free(chain, X509_free);
+  return 1;
 }
 
 int SSL_set1_chain(SSL *ssl, STACK_OF(X509) *chain) {