diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index aef6b35e7..12db981e8 100644 --- a/include/openssl/ssl.h +++ b/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. // diff --git a/ssl/handshake.cc b/ssl/handshake.cc index 289c612a5..e6280005b 100644 --- a/ssl/handshake.cc +++ b/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); } diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc index 59ef6ec5f..4fbb48ac7 100644 --- a/ssl/handshake_client.cc +++ b/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)) { diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc index 5da6b40b9..74997fe78 100644 --- a/ssl/handshake_server.cc +++ b/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; diff --git a/ssl/internal.h b/ssl/internal.h index f1085a148..dffc38b9b 100644 --- a/ssl/internal.h +++ b/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; diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc index 260d3cd78..43510f120 100644 --- a/ssl/ssl_lib.cc +++ b/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); } diff --git a/ssl/ssl_privkey.cc b/ssl/ssl_privkey.cc index e800136d2..8462ebf00 100644 --- a/ssl/ssl_privkey.cc +++ b/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 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) { diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index 5f79e8605..96a8af717 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -1251,6 +1251,19 @@ static bssl::UniquePtr GetTestKey() { return KeyFromPEM(kKeyPEM); } +static bssl::UniquePtr CreateContextWithTestCertificate( + const SSL_METHOD *method) { + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr cert = GetTestCertificate(); + bssl::UniquePtr 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 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 *out_client, + bssl::UniquePtr *out_server, + SSL_CTX *client_ctx, SSL_CTX *server_ctx) { + bssl::UniquePtr 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 *out_client, bssl::UniquePtr *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 client(SSL_new(client_ctx)), server(SSL_new(server_ctx)); - if (!client || !server) { + bssl::UniquePtr 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 *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 { 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 client_ctx(SSL_CTX_new(TLS_method())); - bssl::UniquePtr server_ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr server_ctx = + CreateContextWithTestCertificate(TLS_method()); ASSERT_TRUE(client_ctx); ASSERT_TRUE(server_ctx); - bssl::UniquePtr cert = GetTestCertificate(); - bssl::UniquePtr 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 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 cert = GetTestCertificate(); - bssl::UniquePtr key = GetTestKey(); - bssl::UniquePtr server_ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr server_ctx = + CreateContextWithTestCertificate(TLS_method()); bssl::UniquePtr 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 {}; TEST_P(TicketAEADMethodTest, Resume) { - bssl::UniquePtr cert = GetTestCertificate(); - ASSERT_TRUE(cert); - bssl::UniquePtr key = GetTestKey(); - ASSERT_TRUE(key); - - bssl::UniquePtr server_ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr server_ctx = + CreateContextWithTestCertificate(TLS_method()); ASSERT_TRUE(server_ctx); bssl::UniquePtr 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 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 cert = GetTestCertificate(); - bssl::UniquePtr 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 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 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 cert = GetTestCertificate(); - bssl::UniquePtr 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 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 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 cert = GetTestCertificate(); - bssl::UniquePtr 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 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 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 cert = GetTestCertificate(); - bssl::UniquePtr 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 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 ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr 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 cert = GetTestCertificate(); - bssl::UniquePtr 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 client, server; @@ -4368,17 +4343,11 @@ static int XORDecompressFunc(SSL *ssl, CRYPTO_BUFFER **out, TEST(SSLTest, CertCompression) { bssl::UniquePtr client_ctx(SSL_CTX_new(TLS_method())); - bssl::UniquePtr server_ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr server_ctx( + CreateContextWithTestCertificate(TLS_method())); ASSERT_TRUE(client_ctx); ASSERT_TRUE(server_ctx); - bssl::UniquePtr cert = GetTestCertificate(); - bssl::UniquePtr 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 client_ctx(SSL_CTX_new(TLS_method())); bssl::UniquePtr server_ctx(SSL_CTX_new(TLS_method())); - bssl::UniquePtr handshaker_ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr 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 cert = GetTestCertificate(); - bssl::UniquePtr 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 client, server; - auto config = ClientConfig(); - config.early_data = early_data; + ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(), + 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(); - } - if (is_resume && config.early_data) { - EXPECT_GT(g_last_session->ticket_max_early_data, 0u); + SSL_set_session(client.get(), g_last_session.get()); + if (early_data) { + 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 client_ctx(SSL_CTX_new(TLS_method())); - bssl::UniquePtr server_ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr 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 cert = GetTestCertificate(); - bssl::UniquePtr 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 client, server; - ASSERT_TRUE(ConnectClientAndServer(&client, &server, client_ctx.get(), - server_ctx.get(), ClientConfig(), - false /* don't handshake */)); + ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(), + server_ctx.get())); 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 server_ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr 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 cert = GetTestCertificate(); - bssl::UniquePtr 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 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 cert = GetTestCertificate(); - bssl::UniquePtr 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 ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr ctx(CreateContextWithTestCertificate(TLS_method())); ASSERT_TRUE(ctx); - bssl::UniquePtr cert = GetTestCertificate(); - bssl::UniquePtr 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 client, server; - ASSERT_TRUE(ConnectClientAndServer(&client, &server, ctx.get(), ctx.get(), - ClientConfig(), true /* do_handshake */, - false /* don't shed handshake config */)); + ASSERT_TRUE(CreateClientAndServer(&client, &server, ctx.get(), ctx.get())); 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 client_ctx(SSL_CTX_new(TLS_method())); - bssl::UniquePtr server_ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr server_ctx( + CreateContextWithTestCertificate(TLS_method())); ASSERT_TRUE(client_ctx); ASSERT_TRUE(server_ctx); - bssl::UniquePtr cert = GetTestCertificate(); - bssl::UniquePtr 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 client, server; - ASSERT_TRUE(ConnectClientAndServer(&client, &server, client_ctx.get(), - server_ctx.get(), config, - /*do_handshake=*/false)); + ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(), + server_ctx.get())); + 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(ConnectClientAndServer(&client, &server, client_ctx.get(), - server_ctx.get(), config)); + ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(), + server_ctx.get())); + 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 cert = GetTestCertificate(); - bssl::UniquePtr key = GetTestKey(); bssl::UniquePtr client_ctx(SSL_CTX_new(TLS_method())); - bssl::UniquePtr server_ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr 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 client, server; ASSERT_TRUE(ConnectClientAndServer(&client, &server, client_ctx.get(), @@ -6869,17 +6796,11 @@ TEST(SSLTest, ProcessTLS13NewSessionTicket) { TEST(SSLTest, BIO) { bssl::UniquePtr client_ctx(SSL_CTX_new(TLS_method())); - bssl::UniquePtr server_ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr server_ctx( + CreateContextWithTestCertificate(TLS_method())); ASSERT_TRUE(client_ctx); ASSERT_TRUE(server_ctx); - bssl::UniquePtr cert = GetTestCertificate(); - bssl::UniquePtr 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 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 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 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 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 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 session = + CreateClientSession(client_ctx.get(), server_ctx.get()); + ASSERT_TRUE(session); + bssl::UniquePtr 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 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 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 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 session = + CreateClientSession(client_ctx.get(), server_ctx.get()); + ASSERT_TRUE(session); + bssl::UniquePtr 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 diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc index 496ae019f..a4a16fe06 100644 --- a/ssl/tls13_client.cc +++ b/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) { diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc index 6b9867d2c..dc4e65dce 100644 --- a/ssl/tls13_server.cc +++ b/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(