From 1bd6e92b2a454009cc3243f5eb54f09c1929810c Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Mon, 12 Feb 2024 18:09:37 -0500
Subject: [PATCH] Remove some indirection in SSL_certs_clear

If we move SSL_certs_clear to ssl_cert.cc, ssl_cert_clear_certs does not
need to be in the header. Moreover, its only other caller, ~CERT(), does
not need to call it. Now that everything outside of SSL_X509_METHOD is
managed with scopers, the destructor does it automatically. And
cert_free on SSL_X509_METHOD already automatically calls cert_clear, so
it's a no-op to do it again.

Change-Id: Ief9c704cc45440288783564ac4db4a27fbec1bfc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66370
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
---
 ssl/internal.h  |  1 -
 ssl/ssl_cert.cc | 39 ++++++++++++++++++---------------------
 ssl/ssl_lib.cc  |  7 -------
 3 files changed, 18 insertions(+), 29 deletions(-)

diff --git a/ssl/internal.h b/ssl/internal.h
index dcc546bd3..35233af72 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -3197,7 +3197,6 @@ struct SSL_CONFIG {
 static const size_t kMaxEarlyDataAccepted = 14336;
 
 UniquePtr<CERT> ssl_cert_dup(CERT *cert);
-void ssl_cert_clear_certs(CERT *cert);
 bool ssl_set_cert(CERT *cert, UniquePtr<CRYPTO_BUFFER> buffer);
 bool ssl_is_key_type_supported(int key_type);
 // ssl_compare_public_and_private_key returns true if |pubkey| is the public
diff --git a/ssl/ssl_cert.cc b/ssl/ssl_cert.cc
index 9c403291a..80426d85f 100644
--- a/ssl/ssl_cert.cc
+++ b/ssl/ssl_cert.cc
@@ -137,10 +137,7 @@ BSSL_NAMESPACE_BEGIN
 CERT::CERT(const SSL_X509_METHOD *x509_method_arg)
     : x509_method(x509_method_arg) {}
 
-CERT::~CERT() {
-  ssl_cert_clear_certs(this);
-  x509_method->cert_free(this);
-}
+CERT::~CERT() { x509_method->cert_free(this); }
 
 static CRYPTO_BUFFER *buffer_up_ref(const CRYPTO_BUFFER *buffer) {
   CRYPTO_BUFFER_up_ref(const_cast<CRYPTO_BUFFER *>(buffer));
@@ -192,23 +189,6 @@ UniquePtr<CERT> ssl_cert_dup(CERT *cert) {
   return ret;
 }
 
-// Free up and clear all certificates and chains
-void ssl_cert_clear_certs(CERT *cert) {
-  if (cert == NULL) {
-    return;
-  }
-
-  cert->x509_method->cert_clear(cert);
-
-  cert->chain.reset();
-  cert->privatekey.reset();
-  cert->key_method = nullptr;
-
-  cert->dc.reset();
-  cert->dc_privatekey.reset();
-  cert->dc_key_method = nullptr;
-}
-
 static void ssl_cert_set_cert_cb(CERT *cert, int (*cb)(SSL *ssl, void *arg),
                                  void *arg) {
   cert->cert_cb = cb;
@@ -890,6 +870,23 @@ int SSL_CTX_set_chain_and_key(SSL_CTX *ctx, CRYPTO_BUFFER *const *certs,
                                 privkey_method);
 }
 
+void SSL_certs_clear(SSL *ssl) {
+  if (!ssl->config) {
+    return;
+  }
+
+  CERT *cert = ssl->config->cert.get();
+  cert->x509_method->cert_clear(cert);
+
+  cert->chain.reset();
+  cert->privatekey.reset();
+  cert->key_method = nullptr;
+
+  cert->dc.reset();
+  cert->dc_privatekey.reset();
+  cert->dc_key_method = nullptr;
+}
+
 const STACK_OF(CRYPTO_BUFFER) *SSL_CTX_get0_chain(const SSL_CTX *ctx) {
   return ctx->cert->chain.get();
 }
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index 91741fdf5..23fdccb38 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -1566,13 +1566,6 @@ const uint8_t *SSL_get0_session_id_context(const SSL *ssl, size_t *out_len) {
   return ssl->config->cert->sid_ctx;
 }
 
-void SSL_certs_clear(SSL *ssl) {
-  if (!ssl->config) {
-    return;
-  }
-  ssl_cert_clear_certs(ssl->config->cert.get());
-}
-
 int SSL_get_fd(const SSL *ssl) { return SSL_get_rfd(ssl); }
 
 int SSL_get_rfd(const SSL *ssl) {