Add SSL_can_release_private_key.

Callers using private key callbacks may retain non-trivial state with a
private key. In many cases, the private key is no longer necessary
immediately after the first round-trip (e.g. non-HRR TLS 1.3
connections). Add a function that callers can query to drop the state a
hair earlier.

This is tested in two ways. First, the asserts in front of using the
key, combined with existing tests, ensure we don't start reporting it
too early. Second, I've added tests in ssl_test.cc to assert we report
it as early as we expect to.

In doing so, the number of parameters on ConnectClientAndServer()
started getting tedious, so I've split that into a
CreateClientAndServer() and CompleteHandshakes(). Callers that need to
configure weird things or drive the handshake manually can call
CreateClientAndServer() (which takes care of the BIO pair business) and
continue from there.

Bug: b/183734559
Change-Id: I05e1edb6d269c8468ba7cde7dc90e0856694a0ca
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47344
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>

include/openssl/ssl.h

@@ -1281,6 +1281,15 @@ OPENSSL_EXPORT void SSL_set_private_key_method(
 OPENSSL_EXPORT void SSL_CTX_set_private_key_method(
     SSL_CTX *ctx, const SSL_PRIVATE_KEY_METHOD *key_method);
 
+// SSL_can_release_private_key returns one if |ssl| will no longer call into the
+// private key and zero otherwise. If the function returns one, the caller can
+// release state associated with the private key.
+//
+// NOTE: This function assumes the caller does not use |SSL_clear| to reuse
+// |ssl| for a second connection. If |SSL_clear| is used, BoringSSL may still
+// use the private key on the second connection.
+OPENSSL_EXPORT int SSL_can_release_private_key(const SSL *ssl);
+
 
 // Cipher suites.
 //

ssl/handshake.cc

@@ -151,7 +151,8 @@ SSL_HANDSHAKE::SSL_HANDSHAKE(SSL *ssl_arg)
       handback(false),
       hints_requested(false),
       cert_compression_negotiated(false),
-      apply_jdk11_workaround(false) {
+      apply_jdk11_workaround(false),
+      can_release_private_key(false) {
   assert(ssl);
 }
 

ssl/handshake_client.cc

@@ -1482,6 +1482,7 @@ static enum ssl_hs_wait_t do_send_client_certificate_verify(SSL_HANDSHAKE *hs) {
 
 static enum ssl_hs_wait_t do_send_client_finished(SSL_HANDSHAKE *hs) {
   SSL *const ssl = hs->ssl;
+  hs->can_release_private_key = true;
   // Resolve Channel ID first, before any non-idempotent operations.
   if (ssl->s3->channel_id_valid) {
     if (!ssl_do_channel_id_callback(hs)) {

ssl/handshake_server.cc

@@ -891,6 +891,7 @@ static enum ssl_hs_wait_t do_select_parameters(SSL_HANDSHAKE *hs) {
     hs->ticket_expected = renew_ticket;
     ssl->session = std::move(session);
     ssl->s3->session_reused = true;
+    hs->can_release_private_key = true;
   } else {
     hs->ticket_expected = tickets_supported;
     ssl_set_session(ssl, NULL);
@@ -1196,6 +1197,7 @@ static enum ssl_hs_wait_t do_send_server_key_exchange(SSL_HANDSHAKE *hs) {
     }
   }
 
+  hs->can_release_private_key = true;
   if (!ssl_add_message_cbb(ssl, cbb.get())) {
     return ssl_hs_error;
   }
@@ -1528,6 +1530,7 @@ static enum ssl_hs_wait_t do_read_client_key_exchange(SSL_HANDSHAKE *hs) {
   }
   hs->new_session->extended_master_secret = hs->extended_master_secret;
   CONSTTIME_DECLASSIFY(hs->new_session->secret, hs->new_session->secret_length);
+  hs->can_release_private_key = true;
 
   ssl->method->next_message(ssl);
   hs->state = state12_read_client_certificate_verify;

ssl/internal.h

@@ -1964,6 +1964,10 @@ struct SSL_HANDSHAKE {
   // which implemented TLS 1.3 incorrectly.
   bool apply_jdk11_workaround : 1;
 
+  // can_release_private_key is true if the private key will no longer be used
+  // in this handshake.
+  bool can_release_private_key : 1;
+
   // client_version is the value sent or received in the ClientHello version.
   uint16_t client_version = 0;
 

ssl/ssl_lib.cc

@@ -463,7 +463,8 @@ static bool ssl_can_renegotiate(const SSL *ssl) {
     return false;
   }
 
-  if (ssl_protocol_version(ssl) >= TLS1_3_VERSION) {
+  if (ssl->s3->have_version &&
+      ssl_protocol_version(ssl) >= TLS1_3_VERSION) {
     return false;
   }
 
@@ -1788,6 +1789,9 @@ int SSL_renegotiate(SSL *ssl) {
     return 0;
   }
 
+  // We should not have told the caller to release the private key.
+  assert(!SSL_can_release_private_key(ssl));
+
   // Renegotiation is only supported at quiescent points in the application
   // protocol, namely in HTTPS, just before reading the HTTP response.
   // Require the record-layer be idle and avoid complexities of sending a
@@ -2840,6 +2844,17 @@ void SSL_CTX_set_current_time_cb(SSL_CTX *ctx,
   ctx->current_time_cb = cb;
 }
 
+int SSL_can_release_private_key(const SSL *ssl) {
+  if (ssl_can_renegotiate(ssl)) {
+    // If the connection can renegotiate (client only), the private key may be
+    // used in a future handshake.
+    return 0;
+  }
+
+  // Otherwise, this is determined by the current handshake.
+  return !ssl->s3->hs || ssl->s3->hs->can_release_private_key;
+}
+
 int SSL_is_init_finished(const SSL *ssl) {
   return !SSL_in_init(ssl);
 }

ssl/ssl_privkey.cc

@@ -203,6 +203,7 @@ enum ssl_private_key_result_t ssl_private_key_sign(
   SSL *const ssl = hs->ssl;
   const SSL_PRIVATE_KEY_METHOD *key_method = hs->config->cert->key_method;
   EVP_PKEY *privatekey = hs->config->cert->privatekey.get();
+  assert(!hs->can_release_private_key);
   if (ssl_signing_with_dc(hs)) {
     key_method = hs->config->cert->dc_key_method;
     privatekey = hs->config->cert->dc_privatekey.get();
@@ -254,6 +255,7 @@ enum ssl_private_key_result_t ssl_private_key_decrypt(SSL_HANDSHAKE *hs,
                                                       size_t max_out,
                                                       Span<const uint8_t> in) {
   SSL *const ssl = hs->ssl;
+  assert(!hs->can_release_private_key);
   if (hs->config->cert->key_method != NULL) {
     enum ssl_private_key_result_t ret;
     if (hs->pending_private_key_op) {

ssl/ssl_test.cc

@@ -1251,6 +1251,19 @@ static bssl::UniquePtr<EVP_PKEY> GetTestKey() {
   return KeyFromPEM(kKeyPEM);
 }
 
+static bssl::UniquePtr<SSL_CTX> CreateContextWithTestCertificate(
+    const SSL_METHOD *method) {
+  bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(TLS_method()));
+  bssl::UniquePtr<X509> cert = GetTestCertificate();
+  bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
+  if (!ctx || !cert || !key ||
+      !SSL_CTX_use_certificate(ctx.get(), cert.get()) ||
+      !SSL_CTX_use_PrivateKey(ctx.get(), key.get())) {
+    return nullptr;
+  }
+  return ctx;
+}
+
 static bssl::UniquePtr<X509> GetECDSATestCertificate() {
   static const char kCertPEM[] =
       "-----BEGIN CERTIFICATE-----\n"
@@ -1864,6 +1877,32 @@ static bool FlushNewSessionTickets(SSL *client, SSL *server) {
   }
 }
 
+// CreateClientAndServer creates a client and server |SSL| objects whose |BIO|s
+// are paired with each other. It does not run the handshake. The caller is
+// expected to configure the objects and drive the handshake as needed.
+static bool CreateClientAndServer(bssl::UniquePtr<SSL> *out_client,
+                                  bssl::UniquePtr<SSL> *out_server,
+                                  SSL_CTX *client_ctx, SSL_CTX *server_ctx) {
+  bssl::UniquePtr<SSL> client(SSL_new(client_ctx)), server(SSL_new(server_ctx));
+  if (!client || !server) {
+    return false;
+  }
+  SSL_set_connect_state(client.get());
+  SSL_set_accept_state(server.get());
+
+  BIO *bio1, *bio2;
+  if (!BIO_new_bio_pair(&bio1, 0, &bio2, 0)) {
+    return false;
+  }
+  // SSL_set_bio takes ownership.
+  SSL_set_bio(client.get(), bio1, bio1);
+  SSL_set_bio(server.get(), bio2, bio2);
+
+  *out_client = std::move(client);
+  *out_server = std::move(server);
+  return true;
+}
+
 struct ClientConfig {
   SSL_SESSION *session = nullptr;
   std::string servername;
@@ -1874,18 +1913,14 @@ static bool ConnectClientAndServer(bssl::UniquePtr<SSL> *out_client,
                                    bssl::UniquePtr<SSL> *out_server,
                                    SSL_CTX *client_ctx, SSL_CTX *server_ctx,
                                    const ClientConfig &config = ClientConfig(),
-                                   bool do_handshake = true,
                                    bool shed_handshake_config = true) {
-  bssl::UniquePtr<SSL> client(SSL_new(client_ctx)), server(SSL_new(server_ctx));
+  bssl::UniquePtr<SSL> client, server;
-  if (!client || !server) {
+  if (!CreateClientAndServer(&client, &server, client_ctx, server_ctx)) {
     return false;
   }
   if (config.early_data) {
     SSL_set_early_data_enabled(client.get(), 1);
   }
-  SSL_set_connect_state(client.get());
-  SSL_set_accept_state(server.get());
-
   if (config.session) {
     SSL_set_session(client.get(), config.session);
   }
@@ -1894,18 +1929,10 @@ static bool ConnectClientAndServer(bssl::UniquePtr<SSL> *out_client,
     return false;
   }
 
-  BIO *bio1, *bio2;
-  if (!BIO_new_bio_pair(&bio1, 0, &bio2, 0)) {
-    return false;
-  }
-  // SSL_set_bio takes ownership.
-  SSL_set_bio(client.get(), bio1, bio1);
-  SSL_set_bio(server.get(), bio2, bio2);
-
   SSL_set_shed_handshake_config(client.get(), shed_handshake_config);
   SSL_set_shed_handshake_config(server.get(), shed_handshake_config);
 
-  if (do_handshake && !CompleteHandshakes(client.get(), server.get())) {
+  if (!CompleteHandshakes(client.get(), server.get())) {
     return false;
   }
 
@@ -1951,7 +1978,7 @@ class SSLVersionTest : public ::testing::TestWithParam<VersionParam> {
 
   bool Connect(const ClientConfig &config = ClientConfig()) {
     return ConnectClientAndServer(&client_, &server_, client_ctx_.get(),
-                                  server_ctx_.get(), config, true,
+                                  server_ctx_.get(), config,
                                   shed_handshake_config_);
   }
 
@@ -2043,17 +2070,11 @@ TEST_P(SSLVersionTest, OneSidedShutdown) {
 
 TEST(SSLTest, SessionDuplication) {
   bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
-  bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_method()));
+  bssl::UniquePtr<SSL_CTX> server_ctx =
+      CreateContextWithTestCertificate(TLS_method());
   ASSERT_TRUE(client_ctx);
   ASSERT_TRUE(server_ctx);
 
-  bssl::UniquePtr<X509> cert = GetTestCertificate();
-  bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
-  ASSERT_TRUE(cert);
-  ASSERT_TRUE(key);
-  ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx.get(), cert.get()));
-  ASSERT_TRUE(SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()));
-
   bssl::UniquePtr<SSL> client, server;
   ASSERT_TRUE(ConnectClientAndServer(&client, &server, client_ctx.get(),
                                      server_ctx.get()));
@@ -2881,16 +2902,11 @@ TEST_P(SSLVersionTest, SNICallback) {
 
 // Test that the early callback can swap the maximum version.
 TEST(SSLTest, EarlyCallbackVersionSwitch) {
-  bssl::UniquePtr<X509> cert = GetTestCertificate();
+  bssl::UniquePtr<SSL_CTX> server_ctx =
-  bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
+      CreateContextWithTestCertificate(TLS_method());
-  bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_method()));
   bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
-  ASSERT_TRUE(cert);
-  ASSERT_TRUE(key);
   ASSERT_TRUE(server_ctx);
   ASSERT_TRUE(client_ctx);
-  ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx.get(), cert.get()));
-  ASSERT_TRUE(SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()));
   ASSERT_TRUE(SSL_CTX_set_max_proto_version(client_ctx.get(), TLS1_3_VERSION));
   ASSERT_TRUE(SSL_CTX_set_max_proto_version(server_ctx.get(), TLS1_3_VERSION));
 
@@ -3861,12 +3877,8 @@ class TicketAEADMethodTest
     : public ::testing::TestWithParam<TicketAEADMethodParam> {};
 
 TEST_P(TicketAEADMethodTest, Resume) {
-  bssl::UniquePtr<X509> cert = GetTestCertificate();
+  bssl::UniquePtr<SSL_CTX> server_ctx =
-  ASSERT_TRUE(cert);
+      CreateContextWithTestCertificate(TLS_method());
-  bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
-  ASSERT_TRUE(key);
-
-  bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_method()));
   ASSERT_TRUE(server_ctx);
   bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
   ASSERT_TRUE(client_ctx);
@@ -3876,8 +3888,6 @@ TEST_P(TicketAEADMethodTest, Resume) {
   const ssl_test_ticket_aead_failure_mode failure_mode =
       testing::get<2>(GetParam());
 
-  ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx.get(), cert.get()));
-  ASSERT_TRUE(SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()));
   ASSERT_TRUE(SSL_CTX_set_min_proto_version(client_ctx.get(), version));
   ASSERT_TRUE(SSL_CTX_set_max_proto_version(client_ctx.get(), version));
   ASSERT_TRUE(SSL_CTX_set_min_proto_version(server_ctx.get(), version));
@@ -4021,17 +4031,10 @@ TEST(SSLTest, SelectNextProto) {
 
 TEST(SSLTest, SealRecord) {
   bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLSv1_2_method())),
-      server_ctx(SSL_CTX_new(TLSv1_2_method()));
+      server_ctx(CreateContextWithTestCertificate(TLSv1_2_method()));
   ASSERT_TRUE(client_ctx);
   ASSERT_TRUE(server_ctx);
 
-  bssl::UniquePtr<X509> cert = GetTestCertificate();
-  bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
-  ASSERT_TRUE(cert);
-  ASSERT_TRUE(key);
-  ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx.get(), cert.get()));
-  ASSERT_TRUE(SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()));
-
   bssl::UniquePtr<SSL> client, server;
   ASSERT_TRUE(ConnectClientAndServer(&client, &server, client_ctx.get(),
                                      server_ctx.get()));
@@ -4064,17 +4067,10 @@ TEST(SSLTest, SealRecord) {
 
 TEST(SSLTest, SealRecordInPlace) {
   bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLSv1_2_method())),
-      server_ctx(SSL_CTX_new(TLSv1_2_method()));
+      server_ctx(CreateContextWithTestCertificate(TLSv1_2_method()));
   ASSERT_TRUE(client_ctx);
   ASSERT_TRUE(server_ctx);
 
-  bssl::UniquePtr<X509> cert = GetTestCertificate();
-  bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
-  ASSERT_TRUE(cert);
-  ASSERT_TRUE(key);
-  ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx.get(), cert.get()));
-  ASSERT_TRUE(SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()));
-
   bssl::UniquePtr<SSL> client, server;
   ASSERT_TRUE(ConnectClientAndServer(&client, &server, client_ctx.get(),
                                      server_ctx.get()));
@@ -4102,17 +4098,10 @@ TEST(SSLTest, SealRecordInPlace) {
 
 TEST(SSLTest, SealRecordTrailingData) {
   bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLSv1_2_method())),
-      server_ctx(SSL_CTX_new(TLSv1_2_method()));
+      server_ctx(CreateContextWithTestCertificate(TLSv1_2_method()));
   ASSERT_TRUE(client_ctx);
   ASSERT_TRUE(server_ctx);
 
-  bssl::UniquePtr<X509> cert = GetTestCertificate();
-  bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
-  ASSERT_TRUE(cert);
-  ASSERT_TRUE(key);
-  ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx.get(), cert.get()));
-  ASSERT_TRUE(SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()));
-
   bssl::UniquePtr<SSL> client, server;
   ASSERT_TRUE(ConnectClientAndServer(&client, &server, client_ctx.get(),
                                      server_ctx.get()));
@@ -4141,17 +4130,10 @@ TEST(SSLTest, SealRecordTrailingData) {
 
 TEST(SSLTest, SealRecordInvalidSpanSize) {
   bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLSv1_2_method())),
-      server_ctx(SSL_CTX_new(TLSv1_2_method()));
+      server_ctx(CreateContextWithTestCertificate(TLSv1_2_method()));
   ASSERT_TRUE(client_ctx);
   ASSERT_TRUE(server_ctx);
 
-  bssl::UniquePtr<X509> cert = GetTestCertificate();
-  bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
-  ASSERT_TRUE(cert);
-  ASSERT_TRUE(key);
-  ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx.get(), cert.get()));
-  ASSERT_TRUE(SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()));
-
   bssl::UniquePtr<SSL> client, server;
   ASSERT_TRUE(ConnectClientAndServer(&client, &server, client_ctx.get(),
                                      server_ctx.get()));
@@ -4275,18 +4257,11 @@ TEST_P(SSLVersionTest, SSLPending) {
 
 // Test that post-handshake tickets consumed by |SSL_shutdown| are ignored.
 TEST(SSLTest, ShutdownIgnoresTickets) {
-  bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(TLS_method()));
+  bssl::UniquePtr<SSL_CTX> ctx(CreateContextWithTestCertificate(TLS_method()));
   ASSERT_TRUE(ctx);
   ASSERT_TRUE(SSL_CTX_set_min_proto_version(ctx.get(), TLS1_3_VERSION));
   ASSERT_TRUE(SSL_CTX_set_max_proto_version(ctx.get(), TLS1_3_VERSION));
 
-  bssl::UniquePtr<X509> cert = GetTestCertificate();
-  bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
-  ASSERT_TRUE(cert);
-  ASSERT_TRUE(key);
-  ASSERT_TRUE(SSL_CTX_use_certificate(ctx.get(), cert.get()));
-  ASSERT_TRUE(SSL_CTX_use_PrivateKey(ctx.get(), key.get()));
-
   SSL_CTX_set_session_cache_mode(ctx.get(), SSL_SESS_CACHE_BOTH);
 
   bssl::UniquePtr<SSL> client, server;
@@ -4368,17 +4343,11 @@ static int XORDecompressFunc(SSL *ssl, CRYPTO_BUFFER **out,
 
 TEST(SSLTest, CertCompression) {
   bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
-  bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_method()));
+  bssl::UniquePtr<SSL_CTX> server_ctx(
+      CreateContextWithTestCertificate(TLS_method()));
   ASSERT_TRUE(client_ctx);
   ASSERT_TRUE(server_ctx);
 
-  bssl::UniquePtr<X509> cert = GetTestCertificate();
-  bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
-  ASSERT_TRUE(cert);
-  ASSERT_TRUE(key);
-  ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx.get(), cert.get()));
-  ASSERT_TRUE(SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()));
-
   ASSERT_TRUE(SSL_CTX_set_max_proto_version(client_ctx.get(), TLS1_3_VERSION));
   ASSERT_TRUE(SSL_CTX_set_max_proto_version(server_ctx.get(), TLS1_3_VERSION));
   ASSERT_TRUE(SSL_CTX_add_cert_compression_alg(
@@ -4410,7 +4379,8 @@ void MoveBIOs(SSL *dest, SSL *src) {
 TEST(SSLTest, Handoff) {
   bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
   bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_method()));
-  bssl::UniquePtr<SSL_CTX> handshaker_ctx(SSL_CTX_new(TLS_method()));
+  bssl::UniquePtr<SSL_CTX> handshaker_ctx(
+      CreateContextWithTestCertificate(TLS_method()));
   ASSERT_TRUE(client_ctx);
   ASSERT_TRUE(server_ctx);
   ASSERT_TRUE(handshaker_ctx);
@@ -4422,36 +4392,28 @@ TEST(SSLTest, Handoff) {
   SSL_CTX_get_tlsext_ticket_keys(server_ctx.get(), &keys, sizeof(keys));
   SSL_CTX_set_tlsext_ticket_keys(handshaker_ctx.get(), &keys, sizeof(keys));
 
-  bssl::UniquePtr<X509> cert = GetTestCertificate();
-  bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
-  ASSERT_TRUE(cert);
-  ASSERT_TRUE(key);
-  ASSERT_TRUE(SSL_CTX_use_certificate(handshaker_ctx.get(), cert.get()));
-  ASSERT_TRUE(SSL_CTX_use_PrivateKey(handshaker_ctx.get(), key.get()));
-
   for (bool early_data : {false, true}) {
     SCOPED_TRACE(early_data);
     for (bool is_resume : {false, true}) {
       SCOPED_TRACE(is_resume);
       bssl::UniquePtr<SSL> client, server;
-      auto config = ClientConfig();
+      ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(),
-      config.early_data = early_data;
+                                        server_ctx.get()));
+      SSL_set_early_data_enabled(client.get(), early_data);
       if (is_resume) {
         ASSERT_TRUE(g_last_session);
-        config.session = g_last_session.get();
+        SSL_set_session(client.get(), g_last_session.get());
-      }
+        if (early_data) {
-      if (is_resume && config.early_data) {
+          EXPECT_GT(g_last_session->ticket_max_early_data, 0u);
-        EXPECT_GT(g_last_session->ticket_max_early_data, 0u);
+        }
       }
-      ASSERT_TRUE(ConnectClientAndServer(&client, &server, client_ctx.get(),
+
-                                         server_ctx.get(), config,
-                                         false /* don't handshake */));
 
       int client_ret = SSL_do_handshake(client.get());
       int client_err = SSL_get_error(client.get(), client_ret);
 
       uint8_t byte_written;
-      if (config.early_data && is_resume) {
+      if (early_data && is_resume) {
         ASSERT_EQ(client_err, 0);
         EXPECT_TRUE(SSL_in_early_data(client.get()));
         // Attempt to write early data.
@@ -4504,7 +4466,7 @@ TEST(SSLTest, Handoff) {
       ASSERT_TRUE(CompleteHandshakes(client.get(), server2.get()));
       EXPECT_EQ(is_resume, SSL_session_reused(client.get()));
 
-      if (config.early_data && is_resume) {
+      if (early_data && is_resume) {
         // In this case, one byte of early data has already been written above.
         EXPECT_TRUE(SSL_early_data_accepted(client.get()));
       } else {
@@ -4525,24 +4487,17 @@ TEST(SSLTest, Handoff) {
 
 TEST(SSLTest, HandoffDeclined) {
   bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
-  bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_method()));
+  bssl::UniquePtr<SSL_CTX> server_ctx(
+      CreateContextWithTestCertificate(TLS_method()));
   ASSERT_TRUE(client_ctx);
   ASSERT_TRUE(server_ctx);
 
   SSL_CTX_set_handoff_mode(server_ctx.get(), 1);
   ASSERT_TRUE(SSL_CTX_set_max_proto_version(server_ctx.get(), TLS1_2_VERSION));
 
-  bssl::UniquePtr<X509> cert = GetTestCertificate();
-  bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
-  ASSERT_TRUE(cert);
-  ASSERT_TRUE(key);
-  ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx.get(), cert.get()));
-  ASSERT_TRUE(SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()));
-
   bssl::UniquePtr<SSL> client, server;
-  ASSERT_TRUE(ConnectClientAndServer(&client, &server, client_ctx.get(),
+  ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(),
-                                     server_ctx.get(), ClientConfig(),
+                                    server_ctx.get()));
-                                     false /* don't handshake */));
 
   int client_ret = SSL_do_handshake(client.get());
   int client_err = SSL_get_error(client.get(), client_ret);
@@ -4788,15 +4743,10 @@ TEST(SSLTest, ApplyHandoffRemovesUnsupportedCurves) {
 TEST(SSLTest, ZeroSizedWiteFlushesHandshakeMessages) {
   // If there are pending handshake mesages, an |SSL_write| of zero bytes should
   // flush them.
-  bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_method()));
+  bssl::UniquePtr<SSL_CTX> server_ctx(
+      CreateContextWithTestCertificate(TLS_method()));
   EXPECT_TRUE(SSL_CTX_set_max_proto_version(server_ctx.get(), TLS1_3_VERSION));
   EXPECT_TRUE(SSL_CTX_set_min_proto_version(server_ctx.get(), TLS1_3_VERSION));
-  bssl::UniquePtr<X509> cert = GetTestCertificate();
-  bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
-  ASSERT_TRUE(cert);
-  ASSERT_TRUE(key);
-  ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx.get(), cert.get()));
-  ASSERT_TRUE(SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()));
 
   bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
   EXPECT_TRUE(SSL_CTX_set_max_proto_version(client_ctx.get(), TLS1_3_VERSION));
@@ -5292,17 +5242,10 @@ class QUICMethodTest : public testing::Test {
  protected:
   void SetUp() override {
     client_ctx_.reset(SSL_CTX_new(TLS_method()));
-    server_ctx_.reset(SSL_CTX_new(TLS_method()));
+    server_ctx_ = CreateContextWithTestCertificate(TLS_method());
     ASSERT_TRUE(client_ctx_);
     ASSERT_TRUE(server_ctx_);
 
-    bssl::UniquePtr<X509> cert = GetTestCertificate();
-    bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
-    ASSERT_TRUE(cert);
-    ASSERT_TRUE(key);
-    ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx_.get(), cert.get()));
-    ASSERT_TRUE(SSL_CTX_use_PrivateKey(server_ctx_.get(), key.get()));
-
     SSL_CTX_set_min_proto_version(server_ctx_.get(), TLS1_3_VERSION);
     SSL_CTX_set_max_proto_version(server_ctx_.get(), TLS1_3_VERSION);
     SSL_CTX_set_min_proto_version(client_ctx_.get(), TLS1_3_VERSION);
@@ -6615,25 +6558,18 @@ TEST_P(SSLVersionTest, UnrelatedServerNoResume) {
 }
 
 TEST(SSLTest, WriteWhileExplicitRenegotiate) {
-  bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(TLS_method()));
+  bssl::UniquePtr<SSL_CTX> ctx(CreateContextWithTestCertificate(TLS_method()));
   ASSERT_TRUE(ctx);
 
-  bssl::UniquePtr<X509> cert = GetTestCertificate();
-  bssl::UniquePtr<EVP_PKEY> pkey = GetTestKey();
-  ASSERT_TRUE(cert);
-  ASSERT_TRUE(pkey);
-  ASSERT_TRUE(SSL_CTX_use_certificate(ctx.get(), cert.get()));
-  ASSERT_TRUE(SSL_CTX_use_PrivateKey(ctx.get(), pkey.get()));
   ASSERT_TRUE(SSL_CTX_set_min_proto_version(ctx.get(), TLS1_2_VERSION));
   ASSERT_TRUE(SSL_CTX_set_max_proto_version(ctx.get(), TLS1_2_VERSION));
   ASSERT_TRUE(SSL_CTX_set_strict_cipher_list(
       ctx.get(), "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256"));
 
   bssl::UniquePtr<SSL> client, server;
-  ASSERT_TRUE(ConnectClientAndServer(&client, &server, ctx.get(), ctx.get(),
+  ASSERT_TRUE(CreateClientAndServer(&client, &server, ctx.get(), ctx.get()));
-                                     ClientConfig(), true /* do_handshake */,
-                                     false /* don't shed handshake config */));
   SSL_set_renegotiate_mode(client.get(), ssl_renegotiate_explicit);
+  ASSERT_TRUE(CompleteHandshakes(client.get(), server.get()));
 
   static const uint8_t kInput[] = {'h', 'e', 'l', 'l', 'o'};
 
@@ -6751,17 +6687,11 @@ TEST(SSLTest, WriteWhileExplicitRenegotiate) {
 
 TEST(SSLTest, CopyWithoutEarlyData) {
   bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
-  bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_method()));
+  bssl::UniquePtr<SSL_CTX> server_ctx(
+      CreateContextWithTestCertificate(TLS_method()));
   ASSERT_TRUE(client_ctx);
   ASSERT_TRUE(server_ctx);
 
-  bssl::UniquePtr<X509> cert = GetTestCertificate();
-  bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
-  ASSERT_TRUE(cert);
-  ASSERT_TRUE(key);
-  ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx.get(), cert.get()));
-  ASSERT_TRUE(SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()));
-
   SSL_CTX_set_session_cache_mode(client_ctx.get(), SSL_SESS_CACHE_BOTH);
   SSL_CTX_set_session_cache_mode(server_ctx.get(), SSL_SESS_CACHE_BOTH);
   SSL_CTX_set_early_data_enabled(client_ctx.get(), 1);
@@ -6772,13 +6702,11 @@ TEST(SSLTest, CopyWithoutEarlyData) {
   ASSERT_TRUE(session);
 
   // The client should attempt early data with |session|.
-  auto config = ClientConfig();
-  config.early_data = true;
-  config.session = session.get();
   bssl::UniquePtr<SSL> client, server;
-  ASSERT_TRUE(ConnectClientAndServer(&client, &server, client_ctx.get(),
+  ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(),
-                                     server_ctx.get(), config,
+                                     server_ctx.get()));
-                                     /*do_handshake=*/false));
+  SSL_set_session(client.get(), session.get());
+  SSL_set_early_data_enabled(client.get(), 1);
   ASSERT_EQ(1, SSL_do_handshake(client.get()));
   EXPECT_TRUE(SSL_in_early_data(client.get()));
 
@@ -6788,9 +6716,11 @@ TEST(SSLTest, CopyWithoutEarlyData) {
       SSL_SESSION_copy_without_early_data(session.get()));
   ASSERT_TRUE(session2);
   EXPECT_NE(session.get(), session2.get());
-  config.session = session2.get();
+  ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(),
-  ASSERT_TRUE(ConnectClientAndServer(&client, &server, client_ctx.get(),
+                                    server_ctx.get()));
-                                     server_ctx.get(), config));
+  SSL_set_session(client.get(), session2.get());
+  SSL_set_early_data_enabled(client.get(), 1);
+  EXPECT_TRUE(CompleteHandshakes(client.get(), server.get()));
   EXPECT_TRUE(SSL_session_reused(client.get()));
   EXPECT_EQ(ssl_early_data_unsupported_for_session,
             SSL_get_early_data_reason(client.get()));
@@ -6804,18 +6734,15 @@ TEST(SSLTest, CopyWithoutEarlyData) {
 
 TEST(SSLTest, ProcessTLS13NewSessionTicket) {
   // Configure client and server to negotiate TLS 1.3 only.
-  bssl::UniquePtr<X509> cert = GetTestCertificate();
-  bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
   bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
-  bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_method()));
+  bssl::UniquePtr<SSL_CTX> server_ctx(
+      CreateContextWithTestCertificate(TLS_method()));
   ASSERT_TRUE(client_ctx);
   ASSERT_TRUE(server_ctx);
   ASSERT_TRUE(SSL_CTX_set_min_proto_version(client_ctx.get(), TLS1_3_VERSION));
   ASSERT_TRUE(SSL_CTX_set_min_proto_version(server_ctx.get(), TLS1_3_VERSION));
   ASSERT_TRUE(SSL_CTX_set_max_proto_version(client_ctx.get(), TLS1_3_VERSION));
   ASSERT_TRUE(SSL_CTX_set_max_proto_version(server_ctx.get(), TLS1_3_VERSION));
-  ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx.get(), cert.get()));
-  ASSERT_TRUE(SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()));
 
   bssl::UniquePtr<SSL> client, server;
   ASSERT_TRUE(ConnectClientAndServer(&client, &server, client_ctx.get(),
@@ -6869,17 +6796,11 @@ TEST(SSLTest, ProcessTLS13NewSessionTicket) {
 
 TEST(SSLTest, BIO) {
   bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
-  bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_method()));
+  bssl::UniquePtr<SSL_CTX> server_ctx(
+      CreateContextWithTestCertificate(TLS_method()));
   ASSERT_TRUE(client_ctx);
   ASSERT_TRUE(server_ctx);
 
-  bssl::UniquePtr<X509> cert = GetTestCertificate();
-  bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
-  ASSERT_TRUE(cert);
-  ASSERT_TRUE(key);
-  ASSERT_TRUE(SSL_CTX_use_certificate(server_ctx.get(), cert.get()));
-  ASSERT_TRUE(SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()));
-
   for (bool take_ownership : {true, false}) {
     // For simplicity, get the handshake out of the way first.
     bssl::UniquePtr<SSL> client, server;
@@ -7021,5 +6942,112 @@ RHrQbWsFUakETXL9QMlegh5t
   EXPECT_FALSE(SSL_CTX_use_certificate(ctx.get(), bad.get()));
 }
 
+// Test that |SSL_can_release_private_key| reports true as early as expected.
+// The internal asserts in the library check we do not report true too early.
+TEST(SSLTest, CanReleasePrivateKey) {
+  bssl::UniquePtr<SSL_CTX> client_ctx =
+      CreateContextWithTestCertificate(TLS_method());
+  ASSERT_TRUE(client_ctx);
+  SSL_CTX_set_session_cache_mode(client_ctx.get(), SSL_SESS_CACHE_BOTH);
+
+  // Note this assumes the transport buffer is large enough to fit the client
+  // and server first flights. We check this with |SSL_ERROR_WANT_READ|. If the
+  // transport buffer was too small it would return |SSL_ERROR_WANT_WRITE|.
+  auto check_first_server_round_trip = [&](SSL *client, SSL *server) {
+    // Write the ClientHello.
+    ASSERT_EQ(-1, SSL_do_handshake(client));
+    ASSERT_EQ(SSL_ERROR_WANT_READ, SSL_get_error(client, -1));
+
+    // Consume the ClientHello and write the server flight.
+    ASSERT_EQ(-1, SSL_do_handshake(server));
+    ASSERT_EQ(SSL_ERROR_WANT_READ, SSL_get_error(server, -1));
+
+    EXPECT_TRUE(SSL_can_release_private_key(server));
+  };
+
+  {
+    SCOPED_TRACE("TLS 1.2 ECDHE");
+    bssl::UniquePtr<SSL_CTX> server_ctx(
+        CreateContextWithTestCertificate(TLS_method()));
+    ASSERT_TRUE(server_ctx);
+    ASSERT_TRUE(
+        SSL_CTX_set_max_proto_version(server_ctx.get(), TLS1_2_VERSION));
+    ASSERT_TRUE(SSL_CTX_set_strict_cipher_list(
+        server_ctx.get(), "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"));
+    // Configure the server to request client certificates, so we can also test
+    // the client half.
+    SSL_CTX_set_custom_verify(
+        server_ctx.get(), SSL_VERIFY_PEER,
+        [](SSL *ssl, uint8_t *out_alert) { return ssl_verify_ok; });
+    bssl::UniquePtr<SSL> client, server;
+    ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(),
+                                      server_ctx.get()));
+    check_first_server_round_trip(client.get(), server.get());
+
+    // Consume the server flight and write the client response. The client still
+    // has a Finished message to consume but can also release its key early.
+    ASSERT_EQ(-1, SSL_do_handshake(client.get()));
+    ASSERT_EQ(SSL_ERROR_WANT_READ, SSL_get_error(client.get(), -1));
+    EXPECT_TRUE(SSL_can_release_private_key(client.get()));
+
+    // However, a client that has not disabled renegotiation can never release
+    // the key.
+    ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(),
+                                      server_ctx.get()));
+    SSL_set_renegotiate_mode(client.get(), ssl_renegotiate_freely);
+    check_first_server_round_trip(client.get(), server.get());
+    ASSERT_EQ(-1, SSL_do_handshake(client.get()));
+    ASSERT_EQ(SSL_ERROR_WANT_READ, SSL_get_error(client.get(), -1));
+    EXPECT_FALSE(SSL_can_release_private_key(client.get()));
+  }
+
+  {
+    SCOPED_TRACE("TLS 1.2 resumption");
+    bssl::UniquePtr<SSL_CTX> server_ctx(
+        CreateContextWithTestCertificate(TLS_method()));
+    ASSERT_TRUE(server_ctx);
+    ASSERT_TRUE(
+        SSL_CTX_set_max_proto_version(server_ctx.get(), TLS1_2_VERSION));
+    bssl::UniquePtr<SSL_SESSION> session =
+        CreateClientSession(client_ctx.get(), server_ctx.get());
+    ASSERT_TRUE(session);
+    bssl::UniquePtr<SSL> client, server;
+    ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(),
+                                      server_ctx.get()));
+    SSL_set_session(client.get(), session.get());
+    check_first_server_round_trip(client.get(), server.get());
+  }
+
+  {
+    SCOPED_TRACE("TLS 1.3 1-RTT");
+    bssl::UniquePtr<SSL_CTX> server_ctx(
+        CreateContextWithTestCertificate(TLS_method()));
+    ASSERT_TRUE(server_ctx);
+    ASSERT_TRUE(
+        SSL_CTX_set_max_proto_version(server_ctx.get(), TLS1_3_VERSION));
+    bssl::UniquePtr<SSL> client, server;
+    ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(),
+                                      server_ctx.get()));
+    check_first_server_round_trip(client.get(), server.get());
+  }
+
+  {
+    SCOPED_TRACE("TLS 1.3 resumption");
+    bssl::UniquePtr<SSL_CTX> server_ctx(
+        CreateContextWithTestCertificate(TLS_method()));
+    ASSERT_TRUE(server_ctx);
+    ASSERT_TRUE(
+        SSL_CTX_set_max_proto_version(server_ctx.get(), TLS1_3_VERSION));
+    bssl::UniquePtr<SSL_SESSION> session =
+        CreateClientSession(client_ctx.get(), server_ctx.get());
+    ASSERT_TRUE(session);
+    bssl::UniquePtr<SSL> client, server;
+    ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(),
+                                      server_ctx.get()));
+    SSL_set_session(client.get(), session.get());
+    check_first_server_round_trip(client.get(), server.get());
+  }
+}
+
 }  // namespace
 BSSL_NAMESPACE_END

ssl/tls13_client.cc

@@ -388,6 +388,7 @@ static enum ssl_hs_wait_t do_read_server_hello(SSL_HANDSHAKE *hs) {
     }
 
     ssl->s3->session_reused = true;
+    hs->can_release_private_key = true;
     // Only authentication information carries over in TLS 1.3.
     hs->new_session =
         SSL_SESSION_dup(ssl->session.get(), SSL_SESSION_DUP_AUTH_ONLY);
@@ -817,6 +818,7 @@ static enum ssl_hs_wait_t do_send_client_certificate_verify(SSL_HANDSHAKE *hs) {
 
 static enum ssl_hs_wait_t do_complete_second_flight(SSL_HANDSHAKE *hs) {
   SSL *const ssl = hs->ssl;
+  hs->can_release_private_key = true;
 
   // Send a Channel ID assertion if necessary.
   if (ssl->s3->channel_id_valid) {

ssl/tls13_server.cc

@@ -394,6 +394,7 @@ static enum ssl_hs_wait_t do_select_session(SSL_HANDSHAKE *hs) {
       }
 
       ssl->s3->session_reused = true;
+      hs->can_release_private_key = true;
 
       // Resumption incorporates fresh key material, so refresh the timeout.
       ssl_session_renew_timeout(ssl, hs->new_session.get(),
@@ -903,6 +904,7 @@ static enum ssl_hs_wait_t do_send_server_finished(SSL_HANDSHAKE *hs) {
     return ssl_hs_hints_ready;
   }
 
+  hs->can_release_private_key = true;
   if (!tls13_add_finished(hs) ||
       // Update the secret to the master secret and derive traffic keys.
       !tls13_advance_key_schedule(