Add post-handshake support for the QUIC API. Change-Id: I4956efabfb33f7bd60a4743a922c29ee4de18935 Reviewed-on: https://boringssl-review.googlesource.com/c/33004 Commit-Queue: Steven Valdez <svaldez@google.com> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 17153c2..cecf78f 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -3047,7 +3047,10 @@ // |SSL_QUIC_METHOD| to configure secrets and send data. If data is needed from // the peer, it will return |SSL_ERROR_WANT_READ|. When received, the caller // should call |SSL_provide_quic_data| and then |SSL_do_handshake| to continue -// the handshake. It is an error to call |SSL_read| and |SSL_write| in QUIC. +// the handshake. After the handshake is complete, the caller should call +// |SSL_provide_quic_data| for any post-handshake data, followed by +// |SSL_process_quic_post_handshake| to process it. It is an error to call +// |SSL_read| and |SSL_write| in QUIC. // // Note that secrets for an encryption level may be available to QUIC before the // level is active in TLS. Callers should use |SSL_quic_read_level| to determine @@ -3064,8 +3067,7 @@ // |SSL_quic_max_handshake_flight_len| to get the maximum buffer length at each // encryption level. // -// Note: 0-RTT and post-handshake tickets are not currently supported via this -// API. +// Note: 0-RTT is not currently supported via this API. // ssl_encryption_level_t represents a specific QUIC encryption level used to // transmit handshake messages. @@ -3139,6 +3141,11 @@ const uint8_t *data, size_t len); +// SSL_process_quic_post_handshake processes any data that QUIC has provided +// after the handshake has completed. This includes NewSessionTicket messages +// sent by the server. It returns one on success and zero on error. +OPENSSL_EXPORT int SSL_process_quic_post_handshake(SSL *ssl); + // SSL_CTX_set_quic_method configures the QUIC hooks. This should only be // configured with a minimum version of TLS 1.3. |quic_method| must remain valid // for the lifetime of |ctx|. It returns one on success and zero on error.
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc index 8a88802..01d64ed 100644 --- a/ssl/ssl_lib.cc +++ b/ssl/ssl_lib.cc
@@ -948,6 +948,33 @@ return 1; } +int SSL_process_quic_post_handshake(SSL *ssl) { + ssl_reset_error_state(ssl); + + if (SSL_in_init(ssl)) { + OPENSSL_PUT_ERROR(SSL, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); + return 0; + } + + // Replay post-handshake message errors. + if (!check_read_error(ssl)) { + return 0; + } + + // Process any buffered post-handshake messages. + SSLMessage msg; + while (ssl->method->get_message(ssl, &msg)) { + // Handle the post-handshake message and try again. + if (!ssl_do_post_handshake(ssl, msg)) { + ssl_set_read_error(ssl); + return 0; + } + ssl->method->next_message(ssl); + } + + return 1; +} + static int ssl_read_impl(SSL *ssl) { ssl_reset_error_state(ssl);
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index f945898..470379c 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc
@@ -4727,6 +4727,21 @@ return true; } + bool CreateSecondClientAndServer() { + client_.reset(SSL_new(client_ctx_.get())); + server_.reset(SSL_new(server_ctx_.get())); + if (!client_ || !server_) { + return false; + } + + SSL_set_connect_state(client_.get()); + SSL_set_accept_state(server_.get()); + + ex_data_.Set(client_.get(), second_transport_.client()); + ex_data_.Set(server_.get(), second_transport_.server()); + return true; + } + // The following functions may be configured on an |SSL_QUIC_METHOD| as // default implementations. @@ -4760,6 +4775,7 @@ static UnownedSSLExData<MockQUICTransport> ex_data_; MockQUICTransportPair transport_; + MockQUICTransportPair second_transport_; bssl::UniquePtr<SSL> client_; bssl::UniquePtr<SSL> server_; @@ -4776,6 +4792,10 @@ SendAlertCallback, }; + g_last_session = nullptr; + + SSL_CTX_set_session_cache_mode(client_ctx_.get(), SSL_SESS_CACHE_BOTH); + SSL_CTX_sess_set_new_cb(client_ctx_.get(), SaveLastSession); ASSERT_TRUE(SSL_CTX_set_quic_method(client_ctx_.get(), &quic_method)); ASSERT_TRUE(SSL_CTX_set_quic_method(server_ctx_.get(), &quic_method)); ASSERT_TRUE(CreateClientAndServer()); @@ -4807,13 +4827,43 @@ EXPECT_FALSE(transport_.server()->has_alert()); // The server sent NewSessionTicket messages in the handshake. - // - // TODO(davidben,svaldez): Add an API for the client to consume post-handshake - // messages and update these tests. - std::vector<uint8_t> new_session_ticket; - ASSERT_TRUE(transport_.client()->ReadHandshakeData( - &new_session_ticket, ssl_encryption_application)); - EXPECT_FALSE(new_session_ticket.empty()); + EXPECT_FALSE(g_last_session); + ASSERT_TRUE(ProvideHandshakeData(client_.get())); + EXPECT_EQ(SSL_process_quic_post_handshake(client_.get()), 1); + EXPECT_TRUE(g_last_session); + + // Create a second connection to verify resumption works. + ASSERT_TRUE(CreateSecondClientAndServer()); + bssl::UniquePtr<SSL_SESSION> session = std::move(g_last_session); + SSL_set_session(client_.get(), session.get()); + + for (;;) { + ASSERT_TRUE(ProvideHandshakeData(client_.get())); + int client_ret = SSL_do_handshake(client_.get()); + if (client_ret != 1) { + ASSERT_EQ(client_ret, -1); + ASSERT_EQ(SSL_get_error(client_.get(), client_ret), SSL_ERROR_WANT_READ); + } + + ASSERT_TRUE(ProvideHandshakeData(server_.get())); + int server_ret = SSL_do_handshake(server_.get()); + if (server_ret != 1) { + ASSERT_EQ(server_ret, -1); + ASSERT_EQ(SSL_get_error(server_.get(), server_ret), SSL_ERROR_WANT_READ); + } + + if (client_ret == 1 && server_ret == 1) { + break; + } + } + + EXPECT_EQ(SSL_do_handshake(client_.get()), 1); + EXPECT_EQ(SSL_do_handshake(server_.get()), 1); + EXPECT_TRUE(transport_.SecretsMatch(ssl_encryption_application)); + EXPECT_FALSE(transport_.client()->has_alert()); + EXPECT_FALSE(transport_.server()->has_alert()); + EXPECT_TRUE(SSL_session_reused(client_.get())); + EXPECT_TRUE(SSL_session_reused(server_.get())); } // Test only releasing data to QUIC one byte at a time on request, to maximize @@ -5073,6 +5123,56 @@ SSL_provide_quic_data(client_.get(), ssl_encryption_initial, &b, 1)); } +// Provide invalid post-handshake data. +TEST_F(QUICMethodTest, BadPostHandshake) { + const SSL_QUIC_METHOD quic_method = { + SetEncryptionSecretsCallback, + AddHandshakeDataCallback, + FlushFlightCallback, + SendAlertCallback, + }; + + g_last_session = nullptr; + + SSL_CTX_set_session_cache_mode(client_ctx_.get(), SSL_SESS_CACHE_BOTH); + SSL_CTX_sess_set_new_cb(client_ctx_.get(), SaveLastSession); + ASSERT_TRUE(SSL_CTX_set_quic_method(client_ctx_.get(), &quic_method)); + ASSERT_TRUE(SSL_CTX_set_quic_method(server_ctx_.get(), &quic_method)); + ASSERT_TRUE(CreateClientAndServer()); + + for (;;) { + ASSERT_TRUE(ProvideHandshakeData(client_.get())); + int client_ret = SSL_do_handshake(client_.get()); + if (client_ret != 1) { + ASSERT_EQ(client_ret, -1); + ASSERT_EQ(SSL_get_error(client_.get(), client_ret), SSL_ERROR_WANT_READ); + } + + ASSERT_TRUE(ProvideHandshakeData(server_.get())); + int server_ret = SSL_do_handshake(server_.get()); + if (server_ret != 1) { + ASSERT_EQ(server_ret, -1); + ASSERT_EQ(SSL_get_error(server_.get(), server_ret), SSL_ERROR_WANT_READ); + } + + if (client_ret == 1 && server_ret == 1) { + break; + } + } + + EXPECT_EQ(SSL_do_handshake(client_.get()), 1); + EXPECT_EQ(SSL_do_handshake(server_.get()), 1); + EXPECT_TRUE(transport_.SecretsMatch(ssl_encryption_application)); + EXPECT_FALSE(transport_.client()->has_alert()); + EXPECT_FALSE(transport_.server()->has_alert()); + + // Junk sent as part of post-handshake data should cause an error. + uint8_t kJunk[] = {0x17, 0x0, 0x0, 0x4, 0xB, 0xE, 0xE, 0xF}; + ASSERT_TRUE(SSL_provide_quic_data(client_.get(), ssl_encryption_application, + kJunk, sizeof(kJunk))); + EXPECT_EQ(SSL_process_quic_post_handshake(client_.get()), 0); +} + // TODO(davidben): Convert this file to GTest properly. TEST(SSLTest, AllTests) { if (!TestSSL_SESSIONEncoding(kOpenSSLSession) ||