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());