Defer early keys to QUIC clients to after certificate reverification.
On a client using SSL_CTX_set_reverify_on_resume, we currently release
the early data keys before reverification rather than afterwards. This
means the QUIC implementation needs to watch for SSL_do_handshake's
return value before using the keys we've released. It is better to be
robust, so defer releasing the keys in the first place.
To avoid oddities around TCP and QUIC differences, tweak the 0-RTT cert
reverification to not send an alert on error. Sending such an alert
under early data is somewhat questionable given the server may not be
able to read it anyway.
Bug: 303
Change-Id: I42c16f9f046322d0b03cb0b425e11471f2fbe52a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/38885
Reviewed-by: Nick Harper <nharper@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 6211c56..41d6bc2 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -5298,6 +5298,53 @@
}
}
+TEST_F(QUICMethodTest, NoZeroRTTKeysBeforeReverify) {
+ const SSL_QUIC_METHOD quic_method = {
+ SetEncryptionSecretsCallback,
+ AddHandshakeDataCallback,
+ FlushFlightCallback,
+ SendAlertCallback,
+ };
+
+ SSL_CTX_set_session_cache_mode(client_ctx_.get(), SSL_SESS_CACHE_BOTH);
+ SSL_CTX_set_early_data_enabled(client_ctx_.get(), 1);
+ SSL_CTX_set_reverify_on_resume(client_ctx_.get(), 1);
+ SSL_CTX_set_early_data_enabled(server_ctx_.get(), 1);
+ ASSERT_TRUE(SSL_CTX_set_quic_method(client_ctx_.get(), &quic_method));
+ ASSERT_TRUE(SSL_CTX_set_quic_method(server_ctx_.get(), &quic_method));
+
+ bssl::UniquePtr<SSL_SESSION> session = CreateClientSessionForQUIC();
+ ASSERT_TRUE(session);
+
+ ASSERT_TRUE(CreateClientAndServer());
+ SSL_set_session(client_.get(), session.get());
+
+ // Configure the certificate (re)verification to never complete. The client
+ // handshake should pause.
+ SSL_set_custom_verify(
+ client_.get(), SSL_VERIFY_PEER,
+ [](SSL *ssl, uint8_t *out_alert) -> ssl_verify_result_t {
+ return ssl_verify_retry;
+ });
+ ASSERT_EQ(SSL_do_handshake(client_.get()), -1);
+ ASSERT_EQ(SSL_get_error(client_.get(), -1),
+ SSL_ERROR_WANT_CERTIFICATE_VERIFY);
+
+ // The early data keys have not yet been released.
+ EXPECT_FALSE(transport_->client()->HasSecrets(ssl_encryption_early_data));
+
+ // After the verification completes, the handshake progresses to the 0-RTT
+ // point and releases keys.
+ SSL_set_custom_verify(
+ client_.get(), SSL_VERIFY_PEER,
+ [](SSL *ssl, uint8_t *out_alert) -> ssl_verify_result_t {
+ return ssl_verify_ok;
+ });
+ ASSERT_EQ(SSL_do_handshake(client_.get()), 1);
+ EXPECT_TRUE(SSL_in_early_data(client_.get()));
+ EXPECT_TRUE(transport_->client()->HasSecrets(ssl_encryption_early_data));
+}
+
// Test only releasing data to QUIC one byte at a time on request, to maximize
// state machine pauses. Additionally, test that existing asynchronous callbacks
// still work.
