In IETF QUIC client: Delay retransmission of 0-RTT data to be after QuicSession config is set to the fresh server config.
This allows us to verify server config in QuicSession::OnConfigNegotiated() before attempting retransmission. Implementation will come as follow-up.
Also enable T050 in quic_spdy_client_session_test
Client side change only. not protected.
PiperOrigin-RevId: 315352910
Change-Id: I7357e217729c6e59187cce17a30b364517c2c632
diff --git a/quic/core/http/quic_server_session_base_test.cc b/quic/core/http/quic_server_session_base_test.cc
index 291884d..6af67a6 100644
--- a/quic/core/http/quic_server_session_base_test.cc
+++ b/quic/core/http/quic_server_session_base_test.cc
@@ -352,6 +352,7 @@
// more than the negotiated stream limit to deal with rare cases where a
// client FIN/RST is lost.
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
session_->OnConfigNegotiated();
if (!VersionHasIetfQuicFrames(transport_version())) {
// The slightly increased stream limit is set during config negotiation. It
@@ -404,6 +405,7 @@
// streams available. The server accepts slightly more than the negotiated
// stream limit to deal with rare cases where a client FIN/RST is lost.
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
session_->OnConfigNegotiated();
const size_t kAvailableStreamLimit =
session_->MaxAvailableBidirectionalStreams();
@@ -509,6 +511,7 @@
QuicTagVector copt;
copt.push_back(kBWRE);
QuicConfigPeer::SetReceivedConnectionOptions(session_->config(), copt);
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
session_->OnConfigNegotiated();
EXPECT_TRUE(
QuicServerSessionBasePeer::IsBandwidthResumptionEnabled(session_.get()));
@@ -699,6 +702,7 @@
QuicTagVector copt;
copt.push_back(kBWMX);
QuicConfigPeer::SetReceivedConnectionOptions(session_->config(), copt);
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
session_->OnConfigNegotiated();
EXPECT_TRUE(
QuicServerSessionBasePeer::IsBandwidthResumptionEnabled(session_.get()));
@@ -707,6 +711,7 @@
TEST_P(QuicServerSessionBaseTest, NoBandwidthResumptionByDefault) {
EXPECT_FALSE(
QuicServerSessionBasePeer::IsBandwidthResumptionEnabled(session_.get()));
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
session_->OnConfigNegotiated();
EXPECT_FALSE(
QuicServerSessionBasePeer::IsBandwidthResumptionEnabled(session_.get()));
diff --git a/quic/core/http/quic_spdy_client_session.cc b/quic/core/http/quic_spdy_client_session.cc
index 6127e1b..7440edc 100644
--- a/quic/core/http/quic_spdy_client_session.cc
+++ b/quic/core/http/quic_spdy_client_session.cc
@@ -186,7 +186,7 @@
return std::make_unique<QuicCryptoClientStream>(
server_id_, this,
crypto_config_->proof_verifier()->CreateDefaultContext(), crypto_config_,
- this, /*has_application_state = */ true);
+ this, /*has_application_state = */ version().UsesHttp3());
}
bool QuicSpdyClientSession::IsAuthorized(const std::string& /*authority*/) {
diff --git a/quic/core/http/quic_spdy_client_session_test.cc b/quic/core/http/quic_spdy_client_session_test.cc
index 34d95c9..1a6ed6c 100644
--- a/quic/core/http/quic_spdy_client_session_test.cc
+++ b/quic/core/http/quic_spdy_client_session_test.cc
@@ -974,23 +974,31 @@
}
TEST_P(QuicSpdyClientSessionTest, IetfZeroRttSetup) {
- // This feature is HTTP/3 only
- if (!VersionUsesHttp3(session_->transport_version())) {
+ // This feature is TLS-only.
+ if (session_->version().UsesQuicCrypto()) {
return;
}
+
CompleteCryptoHandshake();
EXPECT_FALSE(session_->GetCryptoStream()->IsResumption());
- SettingsFrame settings;
- settings.values[SETTINGS_QPACK_MAX_TABLE_CAPACITY] = 2;
- settings.values[SETTINGS_MAX_HEADER_LIST_SIZE] = 5;
- settings.values[256] = 4; // unknown setting
- session_->OnSettingsFrame(settings);
+ if (session_->version().UsesHttp3()) {
+ SettingsFrame settings;
+ settings.values[SETTINGS_QPACK_MAX_TABLE_CAPACITY] = 2;
+ settings.values[SETTINGS_MAX_HEADER_LIST_SIZE] = 5;
+ settings.values[256] = 4; // unknown setting
+ session_->OnSettingsFrame(settings);
+ }
CreateConnection();
// Session configs should be in initial state.
- EXPECT_EQ(0u, session_->flow_controller()->send_window_offset());
- EXPECT_EQ(std::numeric_limits<size_t>::max(),
- session_->max_outbound_header_list_size());
+ if (session_->version().UsesHttp3()) {
+ EXPECT_EQ(0u, session_->flow_controller()->send_window_offset());
+ EXPECT_EQ(std::numeric_limits<size_t>::max(),
+ session_->max_outbound_header_list_size());
+ } else {
+ EXPECT_EQ(kMinimumFlowControlSendWindow,
+ session_->flow_controller()->send_window_offset());
+ }
session_->CryptoConnect();
EXPECT_TRUE(session_->IsEncryptionEstablished());
EXPECT_EQ(ENCRYPTION_ZERO_RTT, session_->connection()->encryption_level());
@@ -999,17 +1007,23 @@
// succeeds.
EXPECT_EQ(kInitialSessionFlowControlWindowForTest,
session_->flow_controller()->send_window_offset());
- auto* id_manager = QuicSessionPeer::v99_streamid_manager(session_.get());
- EXPECT_EQ(kDefaultMaxStreamsPerConnection,
- id_manager->max_outgoing_bidirectional_streams());
- EXPECT_EQ(
- kDefaultMaxStreamsPerConnection + kHttp3StaticUnidirectionalStreamCount,
- id_manager->max_outgoing_unidirectional_streams());
- auto* control_stream =
- QuicSpdySessionPeer::GetSendControlStream(session_.get());
- EXPECT_EQ(kInitialStreamFlowControlWindowForTest,
- control_stream->flow_controller()->send_window_offset());
- EXPECT_EQ(5u, session_->max_outbound_header_list_size());
+ if (session_->version().UsesHttp3()) {
+ auto* id_manager = QuicSessionPeer::v99_streamid_manager(session_.get());
+ EXPECT_EQ(kDefaultMaxStreamsPerConnection,
+ id_manager->max_outgoing_bidirectional_streams());
+ EXPECT_EQ(
+ kDefaultMaxStreamsPerConnection + kHttp3StaticUnidirectionalStreamCount,
+ id_manager->max_outgoing_unidirectional_streams());
+ auto* control_stream =
+ QuicSpdySessionPeer::GetSendControlStream(session_.get());
+ EXPECT_EQ(kInitialStreamFlowControlWindowForTest,
+ control_stream->flow_controller()->send_window_offset());
+ EXPECT_EQ(5u, session_->max_outbound_header_list_size());
+ } else {
+ auto* id_manager = QuicSessionPeer::GetStreamIdManager(session_.get());
+ EXPECT_EQ(kDefaultMaxStreamsPerConnection,
+ id_manager->max_open_outgoing_streams());
+ }
// Complete the handshake with a different config.
QuicConfig config = DefaultQuicConfig();
@@ -1026,28 +1040,39 @@
EXPECT_TRUE(session_->GetCryptoStream()->IsResumption());
EXPECT_EQ(kInitialSessionFlowControlWindowForTest + 1,
session_->flow_controller()->send_window_offset());
- EXPECT_EQ(kDefaultMaxStreamsPerConnection + 1,
- id_manager->max_outgoing_bidirectional_streams());
- EXPECT_EQ(kDefaultMaxStreamsPerConnection +
- kHttp3StaticUnidirectionalStreamCount + 1,
- id_manager->max_outgoing_unidirectional_streams());
- EXPECT_EQ(kInitialStreamFlowControlWindowForTest + 1,
- control_stream->flow_controller()->send_window_offset());
+ if (session_->version().UsesHttp3()) {
+ auto* id_manager = QuicSessionPeer::v99_streamid_manager(session_.get());
+ auto* control_stream =
+ QuicSpdySessionPeer::GetSendControlStream(session_.get());
+ EXPECT_EQ(kDefaultMaxStreamsPerConnection + 1,
+ id_manager->max_outgoing_bidirectional_streams());
+ EXPECT_EQ(kDefaultMaxStreamsPerConnection +
+ kHttp3StaticUnidirectionalStreamCount + 1,
+ id_manager->max_outgoing_unidirectional_streams());
+ EXPECT_EQ(kInitialStreamFlowControlWindowForTest + 1,
+ control_stream->flow_controller()->send_window_offset());
+ } else {
+ auto* id_manager = QuicSessionPeer::GetStreamIdManager(session_.get());
+ EXPECT_EQ(kDefaultMaxStreamsPerConnection + 1,
+ id_manager->max_open_outgoing_streams());
+ }
}
TEST_P(QuicSpdyClientSessionTest, RetransmitDataOnZeroRttReject) {
- // This feature is HTTP/3 only
- if (!VersionUsesHttp3(session_->transport_version())) {
+ // This feature is TLS-only.
+ if (session_->version().UsesQuicCrypto()) {
return;
}
CompleteCryptoHandshake();
EXPECT_FALSE(session_->GetCryptoStream()->IsResumption());
- SettingsFrame settings;
- settings.values[SETTINGS_QPACK_MAX_TABLE_CAPACITY] = 2;
- settings.values[SETTINGS_MAX_HEADER_LIST_SIZE] = 5;
- settings.values[256] = 4; // unknown setting
- session_->OnSettingsFrame(settings);
+ if (session_->version().UsesHttp3()) {
+ SettingsFrame settings;
+ settings.values[SETTINGS_QPACK_MAX_TABLE_CAPACITY] = 2;
+ settings.values[SETTINGS_MAX_HEADER_LIST_SIZE] = 5;
+ settings.values[256] = 4; // unknown setting
+ session_->OnSettingsFrame(settings);
+ }
// Create a second connection, but disable 0-RTT on the server.
CreateConnection();
@@ -1056,12 +1081,13 @@
config.SetMaxBidirectionalStreamsToSend(kDefaultMaxStreamsPerConnection);
SSL_CTX_set_early_data_enabled(server_crypto_config_->ssl_ctx(), false);
- // 3 packets will be written: CHLO, HTTP/3 SETTINGS, and request data.
+ // Packets will be written: CHLO, HTTP/3 SETTINGS (H/3 only), and request
+ // data.
EXPECT_CALL(*connection_,
OnPacketSent(ENCRYPTION_INITIAL, NOT_RETRANSMISSION));
EXPECT_CALL(*connection_,
OnPacketSent(ENCRYPTION_ZERO_RTT, NOT_RETRANSMISSION))
- .Times(2);
+ .Times(session_->version().UsesHttp3() ? 2 : 1);
session_->CryptoConnect();
EXPECT_TRUE(session_->IsEncryptionEstablished());
EXPECT_EQ(ENCRYPTION_ZERO_RTT, session_->connection()->encryption_level());
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index 7b4e0d7..b391c06 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -1615,6 +1615,7 @@
return;
}
QuicConfigPeer::SetReceivedMaxUnidirectionalStreams(session_.config(), 2u);
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
EXPECT_CALL(
*connection_,
@@ -1629,6 +1630,7 @@
copt.push_back(kIFW7);
QuicConfigPeer::SetReceivedConnectionOptions(session_.config(), copt);
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
session_.OnConfigNegotiated();
EXPECT_EQ(192 * 1024u, QuicFlowControllerPeer::ReceiveWindowSize(
session_.flow_controller()));
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index fe79962..912b0cb 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -999,6 +999,21 @@
}
void QuicSession::OnConfigNegotiated() {
+ // In versions with TLS, the configs will be set twice if 0-RTT is available.
+ // In the second config setting, 1-RTT keys are guaranteed to be available.
+ if (GetQuicReloadableFlag(quic_enable_zero_rtt_for_tls) &&
+ version().UsesTls() && is_configured_ &&
+ connection_->encryption_level() != ENCRYPTION_FORWARD_SECURE) {
+ QUIC_BUG
+ << ENDPOINT
+ << "1-RTT keys missing when config is negotiated for the second time.";
+ connection_->CloseConnection(
+ QUIC_INTERNAL_ERROR,
+ "1-RTT keys missing when config is negotiated for the second time.",
+ ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ return;
+ }
+
QUIC_DVLOG(1) << ENDPOINT << "OnConfigNegotiated";
connection_->SetFromConfig(config_);
@@ -1150,11 +1165,16 @@
OnNewSessionFlowControlWindow(
config_.ReceivedInitialSessionFlowControlWindowBytes());
}
+
is_configured_ = true;
connection()->OnConfigNegotiated();
// Ask flow controllers to try again since the config could have unblocked us.
- if (connection_->version().AllowsLowFlowControlLimits()) {
+ // Or if this session is configured on TLS enabled QUIC versions,
+ // attempt to retransmit 0-RTT data if there's any.
+ if (connection_->version().AllowsLowFlowControlLimits() ||
+ (GetQuicReloadableFlag(quic_enable_zero_rtt_for_tls) &&
+ version().UsesTls())) {
OnCanWrite();
}
}
@@ -1382,11 +1402,6 @@
QUIC_DVLOG(1) << ENDPOINT << "Set default encryption level to " << level;
connection()->SetDefaultEncryptionLevel(level);
- if (perspective() == Perspective::IS_CLIENT &&
- level == ENCRYPTION_FORWARD_SECURE) {
- // 1-RTT write key is available. Retransmit 0-RTT data if there is any.
- OnCanWrite();
- }
}
void QuicSession::SetDefaultEncryptionLevel(EncryptionLevel level) {
@@ -1476,7 +1491,11 @@
// too many.
connection_->RetransmitZeroRttPackets();
if (connection_->encryption_level() == ENCRYPTION_FORWARD_SECURE) {
- OnCanWrite();
+ QUIC_BUG << "1-RTT keys already available when 0-RTT is rejected.";
+ connection_->CloseConnection(
+ QUIC_INTERNAL_ERROR,
+ "1-RTT keys already available when 0-RTT is rejected.",
+ ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
}
}
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc
index 5f0f9b7..70fda35 100644
--- a/quic/core/quic_session_test.cc
+++ b/quic/core/quic_session_test.cc
@@ -611,6 +611,7 @@
if (connection_->version().HasHandshakeDone()) {
EXPECT_CALL(*connection_, SendControlFrame(_));
}
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
session_.GetMutableCryptoStream()->OnHandshakeMessage(message);
EXPECT_TRUE(session_.OneRttKeysAvailable());
}
@@ -1006,6 +1007,7 @@
QuicTagVector copt;
copt.push_back(kH2PR);
QuicConfigPeer::SetReceivedConnectionOptions(session_.config(), copt);
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
session_.OnConfigNegotiated();
ASSERT_TRUE(session_.use_http2_priority_write_scheduler());
@@ -1088,6 +1090,7 @@
QuicTagVector copt;
copt.push_back(kRRWS);
QuicConfigPeer::SetReceivedConnectionOptions(session_.config(), copt);
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
session_.OnConfigNegotiated();
session_.set_writev_consumes_all_data(true);
@@ -1133,6 +1136,7 @@
CryptoHandshakeMessage msg;
MockPacketWriter* writer = static_cast<MockPacketWriter*>(
QuicConnectionPeer::GetWriter(session_.connection()));
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
session_.GetMutableCryptoStream()->OnHandshakeMessage(msg);
// Drive congestion control manually.
@@ -1511,6 +1515,7 @@
EXPECT_CALL(*connection_, SendControlFrame(_));
}
CryptoHandshakeMessage msg;
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
session_.GetMutableCryptoStream()->OnHandshakeMessage(msg);
EXPECT_EQ(kMaximumIdleTimeoutSecs + 3,
QuicConnectionPeer::GetNetworkTimeout(connection_).ToSeconds());
@@ -1791,6 +1796,7 @@
} else {
EXPECT_CALL(*connection_, CloseConnection(_, _, _)).Times(0);
}
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
session_.OnConfigNegotiated();
}
@@ -1800,6 +1806,7 @@
copt.push_back(kIFW7);
QuicConfigPeer::SetReceivedConnectionOptions(session_.config(), copt);
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
session_.OnConfigNegotiated();
EXPECT_EQ(192 * 1024u, QuicFlowControllerPeer::ReceiveWindowSize(
session_.flow_controller()));
@@ -2065,6 +2072,7 @@
kInvalidWindow);
EXPECT_CALL(*connection_,
CloseConnection(QUIC_FLOW_CONTROL_INVALID_WINDOW, _, _));
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
session_.OnConfigNegotiated();
}
@@ -2076,6 +2084,7 @@
QuicConfigPeer::SetReceivedMaxBidirectionalStreams(
session_.config(), kDefaultMaxStreamsPerConnection - 1);
EXPECT_CALL(*connection_, CloseConnection(QUIC_MAX_STREAMS_ERROR, _, _));
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
session_.OnConfigNegotiated();
}
@@ -2087,6 +2096,7 @@
QuicConfigPeer::SetReceivedMaxUnidirectionalStreams(
session_.config(), kDefaultMaxStreamsPerConnection - 1);
EXPECT_CALL(*connection_, CloseConnection(QUIC_MAX_STREAMS_ERROR, _, _));
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
session_.OnConfigNegotiated();
}
@@ -2104,6 +2114,8 @@
.WillOnce(
Invoke(connection_, &MockQuicConnection::ReallyCloseConnection));
EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _));
+
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
session_.OnConfigNegotiated();
}
@@ -2477,6 +2489,7 @@
EXPECT_CALL(*connection_, SendControlFrame(_));
}
CryptoHandshakeMessage handshake_message;
+ connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
session_.GetMutableCryptoStream()->OnHandshakeMessage(handshake_message);
EXPECT_TRUE(session_.OneRttKeysAvailable());
