pr comments and cleanup old x509 reference

aws · Feb 20, 2025 · 88013e2 · 88013e2
1 parent 1097bfb
commit 88013e2
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 7 deletions.
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
@@ -1240,16 +1240,18 @@ OPENSSL_EXPORT int SSL_set_signing_algorithm_prefs(SSL *ssl,
                                                    size_t num_prefs);
 
 // SSL_CTX_use_cert_and_key sets |x509|, |privatekey|, and |chain| on |ctx|.
-// The pkey argument must be the private key of the certificate |x509|.
+// The |privatekey| argument must be the private key of the certificate |x509|.
 // If the override argument is 0, then |x509|, |privatekey|, and |chain| are
 // set only if all were not previously set. If override is non-0, then the
 // certificate, private key and chain certs are always set. |privatekey| and
-// |x509| are not copied or duplicated, their reference count is incremented.
+// |x509| are not copied or duplicated, their reference counts are incremented.
 // In OpenSSL, a shallow copy of |chain| is stored with a reference count
-// increment for all X509 objects in the chain. In AWS-LC,
+// increment for all |X509| objects in the chain. In AWS-LC,
 // we represent X509 chains as a CRYPTO_BUFFER stack. Therefore, we create a
 // an internal copy and leave the |chain| parameter untouched. This means,
 // changes to |chain| after this function is called will not update in |ctx|.
+// This is different from OpenSSL which stores a reference to the X509
+// certificates in the |chain| object.
 //
 // Returns one on success and zero on error.
 OPENSSL_EXPORT int SSL_CTX_use_cert_and_key(SSL_CTX *ctx, X509 *x509,

diff --git a/ssl/ssl_cert.cc b/ssl/ssl_cert.cc
@@ -1054,8 +1054,12 @@ int SSL_CTX_use_cert_and_key(SSL_CTX *ctx, X509 *x509, EVP_PKEY *privatekey,
     return 0;
   }
 
+  // Update the leaf certificate
+  if (cert_pkey->x509_leaf) {
+    X509_free(cert_pkey->x509_leaf);
+  }
   X509_up_ref(x509);
-  ctx->cert->cert_private_keys[idx].x509_leaf = x509;
+  cert_pkey->x509_leaf = x509;
 
   return 1;
 }

diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
@@ -6557,10 +6557,9 @@ TEST(SSLTest, SetLeafChainAndKey) {
 
   bssl::UniquePtr<EVP_PKEY> key = GetChainTestKey();
   ASSERT_TRUE(key);
-  bssl::UniquePtr<X509> leaf = X509FromBuffer(GetChainTestCertificateBuffer());
+  bssl::UniquePtr<X509> leaf = GetChainTestCertificate();
   ASSERT_TRUE(leaf);
-  bssl::UniquePtr<X509> intermediate =
-      X509FromBuffer(GetChainTestIntermediateBuffer());
+  bssl::UniquePtr<X509> intermediate = GetChainTestIntermediate();
   bssl::UniquePtr<STACK_OF(X509)> chain(sk_X509_new_null());
   ASSERT_TRUE(chain);
   ASSERT_TRUE(PushToStack(chain.get(), std::move(intermediate)));