In QUIC client with TLS versions, delay retransmission of 0-RTT packets until 1-RTT keys are available. Protected by enabled gfe2_reloadable_flag_quic_do_not_retransmit_immediately_on_zero_rtt_reject as this only affects client side. PiperOrigin-RevId: 317142127 Change-Id: I21a297e9e1f181558fc27406960d56e3327a2778
diff --git a/quic/core/http/quic_spdy_client_session_test.cc b/quic/core/http/quic_spdy_client_session_test.cc index 26b40f9..c466a9f 100644 --- a/quic/core/http/quic_spdy_client_session_test.cc +++ b/quic/core/http/quic_spdy_client_session_test.cc
@@ -21,6 +21,7 @@ #include "net/third_party/quiche/src/quic/core/quic_versions.h" #include "net/third_party/quiche/src/quic/core/tls_client_handshaker.h" #include "net/third_party/quiche/src/quic/platform/api/quic_expect_bug.h" +#include "net/third_party/quiche/src/quic/platform/api/quic_flags.h" #include "net/third_party/quiche/src/quic/platform/api/quic_ptr_util.h" #include "net/third_party/quiche/src/quic/platform/api/quic_socket_address.h" #include "net/third_party/quiche/src/quic/platform/api/quic_test.h" @@ -237,8 +238,6 @@ if (GetParam().handshake_protocol == PROTOCOL_TLS1_3) { // This test relies on resumption and is QUIC crypto specific, so it is // disabled for TLS. - // TODO(nharper): Add support for resumption to the TLS handshake, and fix - // this test to not rely on QUIC crypto. return; } // Complete a handshake in order to prime the crypto config for 0-RTT. @@ -247,7 +246,6 @@ // Now create a second session using the same crypto config. Initialize(); - EXPECT_CALL(*connection_, OnCanWrite()); // Starting the handshake should move immediately to encryption // established and will allow streams to be created. session_->CryptoConnect(); @@ -1075,7 +1073,10 @@ } } +// Regression test for b/159168475 TEST_P(QuicSpdyClientSessionTest, RetransmitDataOnZeroRttReject) { + SetQuicReloadableFlag(quic_do_not_retransmit_immediately_on_zero_rtt_reject, + true); // This feature is TLS-only. if (session_->version().UsesQuicCrypto()) { return; @@ -1085,6 +1086,11 @@ // Create a second connection, but disable 0-RTT on the server. CreateConnection(); + ON_CALL(*connection_, OnCanWrite()) + .WillByDefault( + testing::Invoke(connection_, &MockQuicConnection::ReallyOnCanWrite)); + EXPECT_CALL(*connection_, OnCanWrite()).Times(0); + QuicConfig config = DefaultQuicConfig(); config.SetMaxUnidirectionalStreamsToSend(kDefaultMaxStreamsPerConnection); config.SetMaxBidirectionalStreamsToSend(kDefaultMaxStreamsPerConnection);
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index b30165d..38e117b 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -2338,7 +2338,10 @@ void QuicConnection::RetransmitZeroRttPackets() { sent_packet_manager_.RetransmitZeroRttPackets(); - WriteIfNotBlocked(); + if (!GetQuicReloadableFlag( + quic_do_not_retransmit_immediately_on_zero_rtt_reject)) { + WriteIfNotBlocked(); + } } void QuicConnection::NeuterUnencryptedPackets() {
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index b1409bf..d273bbe 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -4543,6 +4543,8 @@ } TEST_P(QuicConnectionTest, RetransmitPacketsWithInitialEncryption) { + SetQuicReloadableFlag(quic_do_not_retransmit_immediately_on_zero_rtt_reject, + true); use_tagging_decrypter(); connection_.SetEncrypter(ENCRYPTION_INITIAL, std::make_unique<TaggingEncrypter>(0x01)); @@ -4555,9 +4557,9 @@ connection_.SetDefaultEncryptionLevel(ENCRYPTION_ZERO_RTT); SendStreamDataToPeer(2, "bar", 0, NO_FIN, nullptr); - EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1); - + EXPECT_FALSE(notifier_.HasLostStreamData()); connection_.RetransmitZeroRttPackets(); + EXPECT_TRUE(notifier_.HasLostStreamData()); } TEST_P(QuicConnectionTest, BufferNonDecryptablePackets) {
diff --git a/quic/core/quic_crypto_client_stream_test.cc b/quic/core/quic_crypto_client_stream_test.cc index 6542382..c1bcf17 100644 --- a/quic/core/quic_crypto_client_stream_test.cc +++ b/quic/core/quic_crypto_client_stream_test.cc
@@ -299,7 +299,6 @@ // Recreate connection with the new config and verify a 0-RTT attempt. CreateConnection(); - EXPECT_CALL(*connection_, OnCanWrite()); EXPECT_CALL(*session_, OnProofValid(testing::_)); EXPECT_CALL(*session_, OnProofVerifyDetailsAvailable(testing::_)) .Times(testing::AnyNumber());
diff --git a/quic/core/tls_client_handshaker.cc b/quic/core/tls_client_handshaker.cc index cf754ec..23a74e4 100644 --- a/quic/core/tls_client_handshaker.cc +++ b/quic/core/tls_client_handshaker.cc
@@ -525,6 +525,8 @@ void TlsClientHandshaker::HandleZeroRttReject() { QUIC_LOG(INFO) << "0-RTT handshake attempted but was rejected by the server"; DCHECK(session_cache_); + // Disable encrytion to block outgoing data until 1-RTT keys are available. + encryption_established_ = false; handshaker_delegate()->OnZeroRttRejected(); SSL_reset_early_data_reject(ssl()); session_cache_->ClearEarlyData(server_id_);
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h index 686ea3a..c205b95 100644 --- a/quic/test_tools/quic_test_utils.h +++ b/quic/test_tools/quic_test_utils.h
@@ -676,6 +676,8 @@ QuicConnection::OnError(framer); } + void ReallyOnCanWrite() { QuicConnection::OnCanWrite(); } + void ReallyCloseConnection( QuicErrorCode error, const std::string& details,
diff --git a/quic/test_tools/simple_session_notifier.h b/quic/test_tools/simple_session_notifier.h index 1237f42..65aface 100644 --- a/quic/test_tools/simple_session_notifier.h +++ b/quic/test_tools/simple_session_notifier.h
@@ -78,6 +78,7 @@ bool IsFrameOutstanding(const QuicFrame& frame) const override; bool HasUnackedCryptoData() const override; bool HasUnackedStreamData() const override; + bool HasLostStreamData() const; private: struct StreamState { @@ -124,8 +125,6 @@ bool HasBufferedControlFrames() const; - bool HasLostStreamData() const; - bool StreamHasBufferedData(QuicStreamId id) const; QuicCircularDeque<QuicFrame> control_frames_;