Give CERT a destructor.

Change-Id: I97f5290d908e59ece75fe5b8fa72d51c3cf62148
Reviewed-on: https://boringssl-review.googlesource.com/27489
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>

ssl/handshake_client.cc

@@ -1422,7 +1422,8 @@ static enum ssl_hs_wait_t do_send_client_certificate_verify(SSL_HANDSHAKE *hs) {
       return ssl_hs_error;
     }
 
-    UniquePtr<EVP_PKEY_CTX> pctx(EVP_PKEY_CTX_new(ssl->cert->privatekey, NULL));
+    UniquePtr<EVP_PKEY_CTX> pctx(
+        EVP_PKEY_CTX_new(ssl->cert->privatekey.get(), nullptr));
     if (!pctx ||
         !EVP_PKEY_sign_init(pctx.get()) ||
         !EVP_PKEY_sign(pctx.get(), ptr, &sig_len, digest, digest_len)) {

ssl/handshake_server.cc

@@ -749,8 +749,8 @@ static enum ssl_hs_wait_t do_send_server_certificate(SSL_HANDSHAKE *hs) {
           !CBB_add_u8(&body, TLSEXT_STATUSTYPE_ocsp) ||
           !CBB_add_u24_length_prefixed(&body, &ocsp_response) ||
           !CBB_add_bytes(&ocsp_response,
-                         CRYPTO_BUFFER_data(ssl->cert->ocsp_response),
-                         CRYPTO_BUFFER_len(ssl->cert->ocsp_response)) ||
+                         CRYPTO_BUFFER_data(ssl->cert->ocsp_response.get()),
+                         CRYPTO_BUFFER_len(ssl->cert->ocsp_response.get())) ||
           !ssl_add_message_cbb(ssl, cbb.get())) {
         OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
         return ssl_hs_error;

ssl/internal.h

@@ -328,11 +328,11 @@ class Array {
 
   // CopyFrom replaces the array with a newly-allocated copy of |in|. It returns
   // true on success and false on error.
-  bool CopyFrom(Span<const uint8_t> in) {
+  bool CopyFrom(Span<const T> in) {
     if (!Init(in.size())) {
       return false;
     }
-    OPENSSL_memcpy(data_, in.data(), in.size());
+    OPENSSL_memcpy(data_, in.data(), sizeof(T) * in.size());
     return true;
   }
 
@@ -1766,7 +1766,12 @@ bool tls12_has_different_verify_sigalgs_for_certs(const SSL *ssl);
 #define NAMED_CURVE_TYPE 3
 
 struct CERT {
-  EVP_PKEY *privatekey;
+  static constexpr bool kAllowUniquePtr = true;
+
+  explicit CERT(const SSL_X509_METHOD *x509_method);
+  ~CERT();
+
+  UniquePtr<EVP_PKEY> privatekey;
 
   // chain contains the certificate chain, with the leaf at the beginning. The
   // first element of |chain| may be NULL to indicate that the leaf certificate
@@ -1774,35 +1779,34 @@ struct CERT {
   //   If |chain| != NULL -> len(chain) >= 1
   //   If |chain[0]| == NULL -> len(chain) >= 2.
   //   |chain[1..]| != NULL
-  STACK_OF(CRYPTO_BUFFER) *chain;
+  UniquePtr<STACK_OF(CRYPTO_BUFFER)> chain;
 
   // x509_chain may contain a parsed copy of |chain[1..]|. This is only used as
   // a cache in order to implement “get0” functions that return a non-owning
   // pointer to the certificate chain.
-  STACK_OF(X509) *x509_chain;
+  STACK_OF(X509) *x509_chain = nullptr;
 
   // x509_leaf may contain a parsed copy of the first element of |chain|. This
   // is only used as a cache in order to implement “get0” functions that return
   // a non-owning pointer to the certificate chain.
-  X509 *x509_leaf;
+  X509 *x509_leaf = nullptr;
 
   // x509_stash contains the last |X509| object append to the chain. This is a
   // workaround for some third-party code that continue to use an |X509| object
   // even after passing ownership with an “add0” function.
-  X509 *x509_stash;
+  X509 *x509_stash = nullptr;
 
   // key_method, if non-NULL, is a set of callbacks to call for private key
   // operations.
-  const SSL_PRIVATE_KEY_METHOD *key_method;
+  const SSL_PRIVATE_KEY_METHOD *key_method = nullptr;
 
   // x509_method contains pointers to functions that might deal with |X509|
   // compatibility, or might be a no-op, depending on the application.
-  const SSL_X509_METHOD *x509_method;
+  const SSL_X509_METHOD *x509_method = nullptr;
 
-  // sigalgs, if non-NULL, is the set of signature algorithms supported by
+  // sigalgs, if non-empty, is the set of signature algorithms supported by
   // |privatekey| in decreasing order of preference.
-  uint16_t *sigalgs;
-  size_t num_sigalgs;
+  Array<uint16_t> sigalgs;
 
   // Certificate setup callback: if set is called whenever a
   // certificate may be required (client or server). the callback
@@ -1810,23 +1814,23 @@ struct CERT {
   // certificates required. This allows advanced applications
   // to select certificates on the fly: for example based on
   // supported signature algorithms or curves.
-  int (*cert_cb)(SSL *ssl, void *arg);
-  void *cert_cb_arg;
+  int (*cert_cb)(SSL *ssl, void *arg) = nullptr;
+  void *cert_cb_arg = nullptr;
 
   // Optional X509_STORE for certificate validation. If NULL the parent SSL_CTX
   // store is used instead.
-  X509_STORE *verify_store;
+  X509_STORE *verify_store = nullptr;
 
   // Signed certificate timestamp list to be sent to the client, if requested
-  CRYPTO_BUFFER *signed_cert_timestamp_list;
+  UniquePtr<CRYPTO_BUFFER> signed_cert_timestamp_list;
 
   // OCSP response to be sent to the client, if requested.
-  CRYPTO_BUFFER *ocsp_response;
+  UniquePtr<CRYPTO_BUFFER> ocsp_response;
 
   // sid_ctx partitions the session space within a shared session cache or
   // ticket key. Only sessions with a matching value will be accepted.
-  uint8_t sid_ctx_length;
-  uint8_t sid_ctx[SSL_MAX_SID_CTX_LENGTH];
+  uint8_t sid_ctx_length = 0;
+  uint8_t sid_ctx[SSL_MAX_SID_CTX_LENGTH] = {0};
 
   // If enable_early_data is true, early data can be sent and accepted.
   bool enable_early_data:1;
@@ -2762,10 +2766,8 @@ struct SSLConnection {
 // kMaxEarlyDataSkipped in tls_record.c, which is measured in ciphertext.
 static const size_t kMaxEarlyDataAccepted = 14336;
 
-CERT *ssl_cert_new(const SSL_X509_METHOD *x509_method);
-CERT *ssl_cert_dup(CERT *cert);
+UniquePtr<CERT> ssl_cert_dup(CERT *cert);
 void ssl_cert_clear_certs(CERT *cert);
-void ssl_cert_free(CERT *cert);
 int ssl_set_cert(CERT *cert, UniquePtr<CRYPTO_BUFFER> buffer);
 int ssl_is_key_type_supported(int key_type);
 // ssl_compare_public_and_private_key returns one if |pubkey| is the public

ssl/ssl_cert.cc

@@ -135,16 +135,12 @@
 
 namespace bssl {
 
-CERT *ssl_cert_new(const SSL_X509_METHOD *x509_method) {
-  CERT *ret = (CERT *)OPENSSL_malloc(sizeof(CERT));
-  if (ret == NULL) {
-    OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
-    return NULL;
-  }
-  OPENSSL_memset(ret, 0, sizeof(CERT));
-  ret->x509_method = x509_method;
+CERT::CERT(const SSL_X509_METHOD *x509_method_arg)
+    : x509_method(x509_method_arg), enable_early_data(false) {}
 
-  return ret;
+CERT::~CERT() {
+  ssl_cert_clear_certs(this);
+  x509_method->cert_free(this);
 }
 
 static CRYPTO_BUFFER *buffer_up_ref(CRYPTO_BUFFER *buffer) {
@@ -152,47 +148,45 @@ static CRYPTO_BUFFER *buffer_up_ref(CRYPTO_BUFFER *buffer) {
   return buffer;
 }
 
-CERT *ssl_cert_dup(CERT *cert) {
-  CERT *ret = (CERT *)OPENSSL_malloc(sizeof(CERT));
-  if (ret == NULL) {
-    OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
-    return NULL;
+UniquePtr<CERT> ssl_cert_dup(CERT *cert) {
+  UniquePtr<CERT> ret = MakeUnique<CERT>(cert->x509_method);
+  if (!ret) {
+    return nullptr;
   }
-  OPENSSL_memset(ret, 0, sizeof(CERT));
 
-  ret->chain = sk_CRYPTO_BUFFER_deep_copy(cert->chain, buffer_up_ref,
-                                          CRYPTO_BUFFER_free);
+  if (cert->chain) {
+    ret->chain.reset(sk_CRYPTO_BUFFER_deep_copy(
+        cert->chain.get(), buffer_up_ref, CRYPTO_BUFFER_free));
+    if (!ret->chain) {
+      return nullptr;
+    }
+  }
 
-  if (cert->privatekey != NULL) {
-    EVP_PKEY_up_ref(cert->privatekey);
-    ret->privatekey = cert->privatekey;
+  if (cert->privatekey) {
+    EVP_PKEY_up_ref(cert->privatekey.get());
+    ret->privatekey.reset(cert->privatekey.get());
   }
 
   ret->key_method = cert->key_method;
-  ret->x509_method = cert->x509_method;
 
-  if (cert->sigalgs != NULL) {
-    ret->sigalgs = (uint16_t *)BUF_memdup(
-        cert->sigalgs, cert->num_sigalgs * sizeof(cert->sigalgs[0]));
-    if (ret->sigalgs == NULL) {
-      goto err;
-    }
+  if (!ret->sigalgs.CopyFrom(cert->sigalgs)) {
+    return nullptr;
   }
-  ret->num_sigalgs = cert->num_sigalgs;
 
   ret->cert_cb = cert->cert_cb;
   ret->cert_cb_arg = cert->cert_cb_arg;
 
-  ret->x509_method->cert_dup(ret, cert);
+  ret->x509_method->cert_dup(ret.get(), cert);
 
-  if (cert->signed_cert_timestamp_list != NULL) {
-    CRYPTO_BUFFER_up_ref(cert->signed_cert_timestamp_list);
-    ret->signed_cert_timestamp_list = cert->signed_cert_timestamp_list;
+  if (cert->signed_cert_timestamp_list) {
+    CRYPTO_BUFFER_up_ref(cert->signed_cert_timestamp_list.get());
+    ret->signed_cert_timestamp_list.reset(
+        cert->signed_cert_timestamp_list.get());
   }
 
-  if (cert->ocsp_response != NULL) {
-    CRYPTO_BUFFER_up_ref(cert->ocsp_response);
-    ret->ocsp_response = cert->ocsp_response;
+  if (cert->ocsp_response) {
+    CRYPTO_BUFFER_up_ref(cert->ocsp_response.get());
+    ret->ocsp_response.reset(cert->ocsp_response.get());
   }
 
   ret->sid_ctx_length = cert->sid_ctx_length;
@@ -201,10 +195,6 @@ CERT *ssl_cert_dup(CERT *cert) {
   ret->enable_early_data = cert->enable_early_data;
 
   return ret;
-
-err:
-  ssl_cert_free(ret);
-  return NULL;
 }
 
 // Free up and clear all certificates and chains
@@ -215,25 +205,9 @@ void ssl_cert_clear_certs(CERT *cert) {
 
   cert->x509_method->cert_clear(cert);
 
-  sk_CRYPTO_BUFFER_pop_free(cert->chain, CRYPTO_BUFFER_free);
-  cert->chain = NULL;
-  EVP_PKEY_free(cert->privatekey);
-  cert->privatekey = NULL;
-  cert->key_method = NULL;
-}
-
-void ssl_cert_free(CERT *cert) {
-  if (cert == NULL) {
-    return;
-  }
-
-  ssl_cert_clear_certs(cert);
-  cert->x509_method->cert_free(cert);
-  OPENSSL_free(cert->sigalgs);
-  CRYPTO_BUFFER_free(cert->signed_cert_timestamp_list);
-  CRYPTO_BUFFER_free(cert->ocsp_response);
-
-  OPENSSL_free(cert);
+  cert->chain.reset();
+  cert->privatekey.reset();
+  cert->key_method = nullptr;
 }
 
 static void ssl_cert_set_cert_cb(CERT *cert, int (*cb)(SSL *ssl, void *arg),
@@ -311,42 +285,37 @@ static int cert_set_chain_and_key(
       break;
   }
 
-  STACK_OF(CRYPTO_BUFFER) *certs_sk = sk_CRYPTO_BUFFER_new_null();
-  if (certs_sk == NULL) {
+  UniquePtr<STACK_OF(CRYPTO_BUFFER)> certs_sk(sk_CRYPTO_BUFFER_new_null());
+  if (!certs_sk) {
     return 0;
   }
 
   for (size_t i = 0; i < num_certs; i++) {
-    if (!sk_CRYPTO_BUFFER_push(certs_sk, certs[i])) {
-      sk_CRYPTO_BUFFER_pop_free(certs_sk, CRYPTO_BUFFER_free);
+    if (!sk_CRYPTO_BUFFER_push(certs_sk.get(), certs[i])) {
       return 0;
     }
     CRYPTO_BUFFER_up_ref(certs[i]);
   }
 
-  EVP_PKEY_free(cert->privatekey);
-  cert->privatekey = privkey;
-  if (privkey != NULL) {
+  if (privkey != nullptr) {
     EVP_PKEY_up_ref(privkey);
   }
+  cert->privatekey.reset(privkey);
   cert->key_method = privkey_method;
 
-  sk_CRYPTO_BUFFER_pop_free(cert->chain, CRYPTO_BUFFER_free);
-  cert->chain = certs_sk;
-
+  cert->chain = std::move(certs_sk);
   return 1;
 }
 
 int ssl_set_cert(CERT *cert, UniquePtr<CRYPTO_BUFFER> buffer) {
-  switch (check_leaf_cert_and_privkey(buffer.get(), cert->privatekey)) {
+  switch (check_leaf_cert_and_privkey(buffer.get(), cert->privatekey.get())) {
     case leaf_cert_and_privkey_error:
       return 0;
     case leaf_cert_and_privkey_mismatch:
       // don't fail for a cert/key mismatch, just free current private key
       // (when switching to a different cert & key, first this function should
       // be used, then |ssl_set_pkey|.
-      EVP_PKEY_free(cert->privatekey);
-      cert->privatekey = NULL;
+      cert->privatekey.reset();
       break;
     case leaf_cert_and_privkey_ok:
       break;
@@ -354,20 +323,19 @@ int ssl_set_cert(CERT *cert, UniquePtr<CRYPTO_BUFFER> buffer) {
 
   cert->x509_method->cert_flush_cached_leaf(cert);
 
-  if (cert->chain != NULL) {
-    CRYPTO_BUFFER_free(sk_CRYPTO_BUFFER_value(cert->chain, 0));
-    sk_CRYPTO_BUFFER_set(cert->chain, 0, buffer.release());
+  if (cert->chain != nullptr) {
+    CRYPTO_BUFFER_free(sk_CRYPTO_BUFFER_value(cert->chain.get(), 0));
+    sk_CRYPTO_BUFFER_set(cert->chain.get(), 0, buffer.release());
     return 1;
   }
 
-  cert->chain = sk_CRYPTO_BUFFER_new_null();
-  if (cert->chain == NULL) {
+  cert->chain.reset(sk_CRYPTO_BUFFER_new_null());
+  if (cert->chain == nullptr) {
     return 0;
   }
 
-  if (!PushToStack(cert->chain, std::move(buffer))) {
-    sk_CRYPTO_BUFFER_free(cert->chain);
-    cert->chain = NULL;
+  if (!PushToStack(cert->chain.get(), std::move(buffer))) {
+    cert->chain.reset();
     return 0;
   }
 
@@ -375,8 +343,8 @@ int ssl_set_cert(CERT *cert, UniquePtr<CRYPTO_BUFFER> buffer) {
 }
 
 int ssl_has_certificate(const SSL *ssl) {
-  return ssl->cert->chain != NULL &&
-         sk_CRYPTO_BUFFER_value(ssl->cert->chain, 0) != NULL &&
+  return ssl->cert->chain != nullptr &&
+         sk_CRYPTO_BUFFER_value(ssl->cert->chain.get(), 0) != nullptr &&
          ssl_has_private_key(ssl);
 }
 
@@ -455,7 +423,7 @@ int ssl_add_cert_chain(SSL *ssl, CBB *cbb) {
     return 0;
   }
 
-  STACK_OF(CRYPTO_BUFFER) *chain = ssl->cert->chain;
+  STACK_OF(CRYPTO_BUFFER) *chain = ssl->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;
@@ -558,19 +526,20 @@ int ssl_compare_public_and_private_key(const EVP_PKEY *pubkey,
 }
 
 int ssl_cert_check_private_key(const CERT *cert, const EVP_PKEY *privkey) {
-  if (privkey == NULL) {
+  if (privkey == nullptr) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_NO_PRIVATE_KEY_ASSIGNED);
     return 0;
   }
 
-  if (cert->chain == NULL ||
-      sk_CRYPTO_BUFFER_value(cert->chain, 0) == NULL) {
+  if (cert->chain == nullptr ||
+      sk_CRYPTO_BUFFER_value(cert->chain.get(), 0) == nullptr) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CERTIFICATE_ASSIGNED);
     return 0;
   }
 
   CBS cert_cbs;
-  CRYPTO_BUFFER_init_CBS(sk_CRYPTO_BUFFER_value(cert->chain, 0), &cert_cbs);
+  CRYPTO_BUFFER_init_CBS(sk_CRYPTO_BUFFER_value(cert->chain.get(), 0),
+                         &cert_cbs);
   UniquePtr<EVP_PKEY> pubkey = ssl_cert_parse_pubkey(&cert_cbs);
   if (!pubkey) {
     OPENSSL_PUT_ERROR(X509, X509_R_UNKNOWN_KEY_TYPE);
@@ -793,7 +762,8 @@ int ssl_on_certificate_selected(SSL_HANDSHAKE *hs) {
   }
 
   CBS leaf;
-  CRYPTO_BUFFER_init_CBS(sk_CRYPTO_BUFFER_value(ssl->cert->chain, 0), &leaf);
+  CRYPTO_BUFFER_init_CBS(sk_CRYPTO_BUFFER_value(ssl->cert->chain.get(), 0),
+                         &leaf);
 
   hs->local_pubkey = ssl_cert_parse_pubkey(&leaf);
   return hs->local_pubkey != NULL;
@@ -862,7 +832,7 @@ STACK_OF(CRYPTO_BUFFER) *SSL_get0_server_requested_CAs(const SSL *ssl) {
 }
 
 static int set_signed_cert_timestamp_list(CERT *cert, const uint8_t *list,
-                                           size_t list_len) {
+                                          size_t list_len) {
   CBS sct_list;
   CBS_init(&sct_list, list, list_len);
   if (!ssl_is_sct_list_valid(&sct_list)) {
@@ -870,10 +840,9 @@ static int set_signed_cert_timestamp_list(CERT *cert, const uint8_t *list,
     return 0;
   }
 
-  CRYPTO_BUFFER_free(cert->signed_cert_timestamp_list);
-  cert->signed_cert_timestamp_list =
-      CRYPTO_BUFFER_new(CBS_data(&sct_list), CBS_len(&sct_list), NULL);
-  return cert->signed_cert_timestamp_list != NULL;
+  cert->signed_cert_timestamp_list.reset(
+      CRYPTO_BUFFER_new(CBS_data(&sct_list), CBS_len(&sct_list), nullptr));
+  return cert->signed_cert_timestamp_list != nullptr;
 }
 
 int SSL_CTX_set_signed_cert_timestamp_list(SSL_CTX *ctx, const uint8_t *list,
@@ -888,16 +857,16 @@ int SSL_set_signed_cert_timestamp_list(SSL *ssl, const uint8_t *list,
 
 int SSL_CTX_set_ocsp_response(SSL_CTX *ctx, const uint8_t *response,
                               size_t response_len) {
-  CRYPTO_BUFFER_free(ctx->cert->ocsp_response);
-  ctx->cert->ocsp_response = CRYPTO_BUFFER_new(response, response_len, NULL);
-  return ctx->cert->ocsp_response != NULL;
+  ctx->cert->ocsp_response.reset(
+      CRYPTO_BUFFER_new(response, response_len, nullptr));
+  return ctx->cert->ocsp_response != nullptr;
 }
 
 int SSL_set_ocsp_response(SSL *ssl, const uint8_t *response,
                           size_t response_len) {
-  CRYPTO_BUFFER_free(ssl->cert->ocsp_response);
-  ssl->cert->ocsp_response = CRYPTO_BUFFER_new(response, response_len, NULL);
-  return ssl->cert->ocsp_response != NULL;
+  ssl->cert->ocsp_response.reset(
+      CRYPTO_BUFFER_new(response, response_len, nullptr));
+  return ssl->cert->ocsp_response != nullptr;
 }
 
 void SSL_CTX_set0_client_CAs(SSL_CTX *ctx, STACK_OF(CRYPTO_BUFFER) *name_list) {

ssl/ssl_lib.cc

@@ -549,7 +549,7 @@ SSL_CTX *SSL_CTX_new(const SSL_METHOD *method) {
 
   ret->max_cert_list = SSL_MAX_CERT_LIST_DEFAULT;
   ret->verify_mode = SSL_VERIFY_NONE;
-  ret->cert = ssl_cert_new(method->x509_method);
+  ret->cert = New<CERT>(method->x509_method);
   if (ret->cert == NULL) {
     goto err;
   }
@@ -623,7 +623,7 @@ void SSL_CTX_free(SSL_CTX *ctx) {
   CRYPTO_MUTEX_cleanup(&ctx->lock);
   lh_SSL_SESSION_free(ctx->sessions);
   ssl_cipher_preference_list_free(ctx->cipher_list);
-  ssl_cert_free(ctx->cert);
+  Delete(ctx->cert);
   sk_SSL_CUSTOM_EXTENSION_pop_free(ctx->client_custom_extensions,
                                    SSL_CUSTOM_EXTENSION_free);
   sk_SSL_CUSTOM_EXTENSION_pop_free(ctx->server_custom_extensions,
@@ -670,7 +670,7 @@ SSL *SSL_new(SSL_CTX *ctx) {
   ssl->mode = ctx->mode;
   ssl->max_cert_list = ctx->max_cert_list;
 
-  ssl->cert = ssl_cert_dup(ctx->cert);
+  ssl->cert = ssl_cert_dup(ctx->cert).release();
   if (ssl->cert == NULL) {
     goto err;
   }
@@ -769,7 +769,7 @@ void SSL_free(SSL *ssl) {
 
   SSL_SESSION_free(ssl->session);
 
-  ssl_cert_free(ssl->cert);
+  Delete(ssl->cert);
 
   OPENSSL_free(ssl->tlsext_hostname);
   SSL_CTX_free(ssl->session_ctx);
@@ -1606,12 +1606,12 @@ int SSL_pending(const SSL *ssl) {
 
 // Fix this so it checks all the valid key/cert options
 int SSL_CTX_check_private_key(const SSL_CTX *ctx) {
-  return ssl_cert_check_private_key(ctx->cert, ctx->cert->privatekey);
+  return ssl_cert_check_private_key(ctx->cert, ctx->cert->privatekey.get());
 }
 
 // Fix this function so that it takes an optional type parameter
 int SSL_check_private_key(const SSL *ssl) {
-  return ssl_cert_check_private_key(ssl->cert, ssl->cert->privatekey);
+  return ssl_cert_check_private_key(ssl->cert, ssl->cert->privatekey.get());
 }
 
 long SSL_get_default_timeout(const SSL *ssl) {
@@ -2184,7 +2184,7 @@ size_t SSL_get0_certificate_types(SSL *ssl, const uint8_t **out_types) {
 
 EVP_PKEY *SSL_get_privatekey(const SSL *ssl) {
   if (ssl->cert != NULL) {
-    return ssl->cert->privatekey;
+    return ssl->cert->privatekey.get();
   }
 
   return NULL;
@@ -2192,7 +2192,7 @@ EVP_PKEY *SSL_get_privatekey(const SSL *ssl) {
 
 EVP_PKEY *SSL_CTX_get0_privatekey(const SSL_CTX *ctx) {
   if (ctx->cert != NULL) {
-    return ctx->cert->privatekey;
+    return ctx->cert->privatekey.get();
   }
 
   return NULL;
@@ -2273,8 +2273,8 @@ SSL_CTX *SSL_set_SSL_CTX(SSL *ssl, SSL_CTX *ctx) {
     ctx = ssl->session_ctx;
   }
 
-  ssl_cert_free(ssl->cert);
-  ssl->cert = ssl_cert_dup(ctx->cert);
+  Delete(ssl->cert);
+  ssl->cert = ssl_cert_dup(ctx->cert).release();
 
   SSL_CTX_up_ref(ctx);
   SSL_CTX_free(ssl->ctx);

ssl/ssl_privkey.cc

@@ -82,16 +82,15 @@ static int ssl_set_pkey(CERT *cert, EVP_PKEY *pkey) {
     return 0;
   }
 
-  if (cert->chain != NULL &&
-      sk_CRYPTO_BUFFER_value(cert->chain, 0) != NULL &&
+  if (cert->chain != nullptr &&
+      sk_CRYPTO_BUFFER_value(cert->chain.get(), 0) != nullptr &&
       // Sanity-check that the private key and the certificate match.
       !ssl_cert_check_private_key(cert, pkey)) {
     return 0;
   }
 
-  EVP_PKEY_free(cert->privatekey);
   EVP_PKEY_up_ref(pkey);
-  cert->privatekey = pkey;
+  cert->privatekey.reset(pkey);
 
   return 1;
 }
@@ -136,7 +135,7 @@ static const SSL_SIGNATURE_ALGORITHM *get_signature_algorithm(uint16_t sigalg) {
 }
 
 int ssl_has_private_key(const SSL *ssl) {
-  return ssl->cert->privatekey != NULL || ssl->cert->key_method != NULL;
+  return ssl->cert->privatekey != nullptr || ssl->cert->key_method != nullptr;
 }
 
 static int pkey_supports_algorithm(const SSL *ssl, EVP_PKEY *pkey,
@@ -214,7 +213,8 @@ enum ssl_private_key_result_t ssl_private_key_sign(
 
   *out_len = max_out;
   ScopedEVP_MD_CTX ctx;
-  if (!setup_ctx(ssl, ctx.get(), ssl->cert->privatekey, sigalg, 0 /* sign */) ||
+  if (!setup_ctx(ssl, ctx.get(), ssl->cert->privatekey.get(), sigalg,
+                 0 /* sign */) ||
       !EVP_DigestSign(ctx.get(), out, out_len, in.data(), in.size())) {
     return ssl_private_key_failure;
   }
@@ -251,7 +251,7 @@ enum ssl_private_key_result_t ssl_private_key_decrypt(SSL_HANDSHAKE *hs,
     return ret;
   }
 
-  RSA *rsa = EVP_PKEY_get0_RSA(ssl->cert->privatekey);
+  RSA *rsa = EVP_PKEY_get0_RSA(ssl->cert->privatekey.get());
   if (rsa == NULL) {
     // Decrypt operations are only supported for RSA keys.
     OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
@@ -477,14 +477,12 @@ static int set_algorithm_prefs(uint16_t **out_prefs, size_t *out_num_prefs,
 
 int SSL_CTX_set_signing_algorithm_prefs(SSL_CTX *ctx, const uint16_t *prefs,
                                         size_t num_prefs) {
-  return set_algorithm_prefs(&ctx->cert->sigalgs, &ctx->cert->num_sigalgs,
-                             prefs, num_prefs);
+  return ctx->cert->sigalgs.CopyFrom(MakeConstSpan(prefs, num_prefs));
 }
 
 int SSL_set_signing_algorithm_prefs(SSL *ssl, const uint16_t *prefs,
                                     size_t num_prefs) {
-  return set_algorithm_prefs(&ssl->cert->sigalgs, &ssl->cert->num_sigalgs,
-                             prefs, num_prefs);
+  return ssl->cert->sigalgs.CopyFrom(MakeConstSpan(prefs, num_prefs));
 }
 
 int SSL_CTX_set_verify_algorithm_prefs(SSL_CTX *ctx, const uint16_t *prefs,

ssl/ssl_x509.cc

@@ -186,15 +186,11 @@ static UniquePtr<CRYPTO_BUFFER> x509_to_buffer(X509 *x509) {
 }
 
 // new_leafless_chain returns a fresh stack of buffers set to {NULL}.
-static STACK_OF(CRYPTO_BUFFER) *new_leafless_chain(void) {
-  STACK_OF(CRYPTO_BUFFER) *chain = sk_CRYPTO_BUFFER_new_null();
-  if (chain == NULL) {
-    return NULL;
-  }
-
-  if (!sk_CRYPTO_BUFFER_push(chain, NULL)) {
-    sk_CRYPTO_BUFFER_free(chain);
-    return NULL;
+static UniquePtr<STACK_OF(CRYPTO_BUFFER)> new_leafless_chain(void) {
+  UniquePtr<STACK_OF(CRYPTO_BUFFER)> chain(sk_CRYPTO_BUFFER_new_null());
+  if (!chain ||
+      !sk_CRYPTO_BUFFER_push(chain.get(), nullptr)) {
+    return nullptr;
   }
 
   return chain;
@@ -207,25 +203,25 @@ static STACK_OF(CRYPTO_BUFFER) *new_leafless_chain(void) {
 static int ssl_cert_set_chain(CERT *cert, STACK_OF(X509) *chain) {
   UniquePtr<STACK_OF(CRYPTO_BUFFER)> new_chain;
 
-  if (cert->chain != NULL) {
+  if (cert->chain != nullptr) {
     new_chain.reset(sk_CRYPTO_BUFFER_new_null());
     if (!new_chain) {
       return 0;
     }
 
-    CRYPTO_BUFFER *leaf = sk_CRYPTO_BUFFER_value(cert->chain, 0);
+    CRYPTO_BUFFER *leaf = sk_CRYPTO_BUFFER_value(cert->chain.get(), 0);
     if (!sk_CRYPTO_BUFFER_push(new_chain.get(), leaf)) {
       return 0;
     }
     // |leaf| might be NULL if it's a “leafless” chain.
-    if (leaf != NULL) {
+    if (leaf != nullptr) {
       CRYPTO_BUFFER_up_ref(leaf);
     }
   }
 
   for (X509 *x509 : chain) {
     if (!new_chain) {
-      new_chain.reset(new_leafless_chain());
+      new_chain = new_leafless_chain();
       if (!new_chain) {
         return 0;
       }
@@ -238,9 +234,7 @@ static int ssl_cert_set_chain(CERT *cert, STACK_OF(X509) *chain) {
     }
   }
 
-  sk_CRYPTO_BUFFER_pop_free(cert->chain, CRYPTO_BUFFER_free);
-  cert->chain = new_chain.release();
-
+  cert->chain = std::move(new_chain);
   return 1;
 }
 
@@ -441,13 +435,13 @@ static int ssl_crypto_x509_ssl_auto_chain_if_needed(SSL *ssl) {
   // isn't disabled.
   if ((ssl->mode & SSL_MODE_NO_AUTO_CHAIN) ||
       !ssl_has_certificate(ssl) ||
-      ssl->cert->chain == NULL ||
-      sk_CRYPTO_BUFFER_num(ssl->cert->chain) > 1) {
+      ssl->cert->chain == nullptr ||
+      sk_CRYPTO_BUFFER_num(ssl->cert->chain.get()) > 1) {
     return 1;
   }
 
-  UniquePtr<X509> leaf(
-      X509_parse_from_buffer(sk_CRYPTO_BUFFER_value(ssl->cert->chain, 0)));
+  UniquePtr<X509> leaf(X509_parse_from_buffer(
+      sk_CRYPTO_BUFFER_value(ssl->cert->chain.get(), 0)));
   if (!leaf) {
     OPENSSL_PUT_ERROR(SSL, ERR_R_X509_LIB);
     return 0;
@@ -750,7 +744,7 @@ static int ssl_cert_cache_leaf_cert(CERT *cert) {
     return 1;
   }
 
-  CRYPTO_BUFFER *leaf = sk_CRYPTO_BUFFER_value(cert->chain, 0);
+  CRYPTO_BUFFER *leaf = sk_CRYPTO_BUFFER_value(cert->chain.get(), 0);
   if (!leaf) {
     return 1;
   }
@@ -807,14 +801,13 @@ static int ssl_cert_append_cert(CERT *cert, X509 *x509) {
   }
 
   if (cert->chain != NULL) {
-    return PushToStack(cert->chain, std::move(buffer));
+    return PushToStack(cert->chain.get(), std::move(buffer));
   }
 
   cert->chain = new_leafless_chain();
-  if (cert->chain == NULL ||
-      !PushToStack(cert->chain, std::move(buffer))) {
-    sk_CRYPTO_BUFFER_free(cert->chain);
-    cert->chain = NULL;
+  if (!cert->chain ||
+      !PushToStack(cert->chain.get(), std::move(buffer))) {
+    cert->chain.reset();
     return 0;
   }
 
@@ -906,9 +899,9 @@ int SSL_clear_chain_certs(SSL *ssl) {
 static int ssl_cert_cache_chain_certs(CERT *cert) {
   assert(cert->x509_method);
 
-  if (cert->x509_chain != NULL ||
-      cert->chain == NULL ||
-      sk_CRYPTO_BUFFER_num(cert->chain) < 2) {
+  if (cert->x509_chain != nullptr ||
+      cert->chain != nullptr ||
+      sk_CRYPTO_BUFFER_num(cert->chain.get()) < 2) {
     return 1;
   }
 
@@ -917,8 +910,8 @@ static int ssl_cert_cache_chain_certs(CERT *cert) {
     return 0;
   }
 
-  for (size_t i = 1; i < sk_CRYPTO_BUFFER_num(cert->chain); i++) {
-    CRYPTO_BUFFER *buffer = sk_CRYPTO_BUFFER_value(cert->chain, i);
+  for (size_t i = 1; i < sk_CRYPTO_BUFFER_num(cert->chain.get()); i++) {
+    CRYPTO_BUFFER *buffer = sk_CRYPTO_BUFFER_value(cert->chain.get(), i);
     UniquePtr<X509> x509(X509_parse_from_buffer(buffer));
     if (!x509 ||
         !PushToStack(chain.get(), std::move(x509))) {

ssl/t1_lib.cc

@@ -1389,8 +1389,8 @@ static bool ext_sct_add_serverhello(SSL_HANDSHAKE *hs, CBB *out) {
          CBB_add_u16_length_prefixed(out, &contents) &&
          CBB_add_bytes(
              &contents,
-             CRYPTO_BUFFER_data(ssl->cert->signed_cert_timestamp_list),
-             CRYPTO_BUFFER_len(ssl->cert->signed_cert_timestamp_list)) &&
+             CRYPTO_BUFFER_data(ssl->cert->signed_cert_timestamp_list.get()),
+             CRYPTO_BUFFER_len(ssl->cert->signed_cert_timestamp_list.get())) &&
          CBB_flush(out);
 }
 
@@ -3588,8 +3588,8 @@ bool tls1_choose_signature_algorithm(SSL_HANDSHAKE *hs, uint16_t *out) {
   }
 
   Span<const uint16_t> sigalgs = kSignSignatureAlgorithms;
-  if (cert->sigalgs != nullptr) {
-    sigalgs = MakeConstSpan(cert->sigalgs, cert->num_sigalgs);
+  if (!cert->sigalgs.empty()) {
+    sigalgs = cert->sigalgs;
   }
 
   Span<const uint16_t> peer_sigalgs = hs->peer_sigalgs;

ssl/tls13_both.cc

@@ -368,7 +368,7 @@ int tls13_add_certificate(SSL_HANDSHAKE *hs) {
   }
 
   CERT *cert = ssl->cert;
-  CRYPTO_BUFFER *leaf_buf = sk_CRYPTO_BUFFER_value(cert->chain, 0);
+  CRYPTO_BUFFER *leaf_buf = sk_CRYPTO_BUFFER_value(cert->chain.get(), 0);
   CBB leaf, extensions;
   if (!CBB_add_u24_length_prefixed(&certificate_list, &leaf) ||
       !CBB_add_bytes(&leaf, CRYPTO_BUFFER_data(leaf_buf),
@@ -378,14 +378,14 @@ int tls13_add_certificate(SSL_HANDSHAKE *hs) {
     return 0;
   }
 
-  if (hs->scts_requested && ssl->cert->signed_cert_timestamp_list != NULL) {
+  if (hs->scts_requested && ssl->cert->signed_cert_timestamp_list != nullptr) {
     CBB contents;
     if (!CBB_add_u16(&extensions, TLSEXT_TYPE_certificate_timestamp) ||
         !CBB_add_u16_length_prefixed(&extensions, &contents) ||
         !CBB_add_bytes(
             &contents,
-            CRYPTO_BUFFER_data(ssl->cert->signed_cert_timestamp_list),
-            CRYPTO_BUFFER_len(ssl->cert->signed_cert_timestamp_list)) ||
+            CRYPTO_BUFFER_data(ssl->cert->signed_cert_timestamp_list.get()),
+            CRYPTO_BUFFER_len(ssl->cert->signed_cert_timestamp_list.get())) ||
         !CBB_flush(&extensions)) {
       OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
       return 0;
@@ -400,16 +400,16 @@ int tls13_add_certificate(SSL_HANDSHAKE *hs) {
         !CBB_add_u8(&contents, TLSEXT_STATUSTYPE_ocsp) ||
         !CBB_add_u24_length_prefixed(&contents, &ocsp_response) ||
         !CBB_add_bytes(&ocsp_response,
-                       CRYPTO_BUFFER_data(ssl->cert->ocsp_response),
-                       CRYPTO_BUFFER_len(ssl->cert->ocsp_response)) ||
+                       CRYPTO_BUFFER_data(ssl->cert->ocsp_response.get()),
+                       CRYPTO_BUFFER_len(ssl->cert->ocsp_response.get())) ||
         !CBB_flush(&extensions)) {
       OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
       return 0;
     }
   }
 
-  for (size_t i = 1; i < sk_CRYPTO_BUFFER_num(cert->chain); i++) {
-    CRYPTO_BUFFER *cert_buf = sk_CRYPTO_BUFFER_value(cert->chain, i);
+  for (size_t i = 1; i < sk_CRYPTO_BUFFER_num(cert->chain.get()); i++) {
+    CRYPTO_BUFFER *cert_buf = sk_CRYPTO_BUFFER_value(cert->chain.get(), i);
     CBB child;
     if (!CBB_add_u24_length_prefixed(&certificate_list, &child) ||
         !CBB_add_bytes(&child, CRYPTO_BUFFER_data(cert_buf),