Close connection if trying to send GOAWAY before encryption gets established. Protected by FLAGS_quic_reloadable_flag_quic_encrypted_goaway. PiperOrigin-RevId: 346378578 Change-Id: I35923b59ee19f33056405ba07cfb82cdeb91e34e
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc index 7f7deb2..a86aefa 100644 --- a/quic/core/http/quic_spdy_session.cc +++ b/quic/core/http/quic_spdy_session.cc
@@ -692,15 +692,25 @@ if (perspective() == Perspective::IS_SERVER && frame.stream_count >= QuicUtils::GetMaxStreamCount()) { DCHECK_EQ(frame.stream_count, QuicUtils::GetMaxStreamCount()); - SendHttp3GoAway(); + SendHttp3GoAway(QUIC_PEER_GOING_AWAY, "stream count too large"); } return true; } -void QuicSpdySession::SendHttp3GoAway() { +void QuicSpdySession::SendHttp3GoAway(QuicErrorCode error_code, + const std::string& reason) { DCHECK_EQ(perspective(), Perspective::IS_SERVER); DCHECK(VersionUsesHttp3(transport_version())); - + if (GetQuicReloadableFlag(quic_encrypted_goaway)) { + QUIC_RELOADABLE_FLAG_COUNT_N(quic_encrypted_goaway, 2, 2); + if (!IsEncryptionEstablished()) { + QUIC_CODE_COUNT(quic_h3_goaway_before_encryption_established); + connection()->CloseConnection( + error_code, reason, + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + return; + } + } QuicStreamId stream_id; if (goaway_with_max_stream_id_) { @@ -747,7 +757,7 @@ void QuicSpdySession::SendHttp3Shutdown() { if (goaway_with_max_stream_id_) { - SendHttp3GoAway(); + SendHttp3GoAway(QUIC_PEER_GOING_AWAY, "Server shutdown"); return; }
diff --git a/quic/core/http/quic_spdy_session.h b/quic/core/http/quic_spdy_session.h index 6e7ccb8..4ab4cfd 100644 --- a/quic/core/http/quic_spdy_session.h +++ b/quic/core/http/quic_spdy_session.h
@@ -230,8 +230,9 @@ // Write GOAWAY frame with maximum stream ID on the control stream. Called to // initite graceful connection shutdown. Do not use smaller stream ID, in // case client does not implement retry on GOAWAY. Do not send GOAWAY if one - // has already been sent. - void SendHttp3GoAway(); + // has already been sent. Send connection close with |error_code| and |reason| + // before encryption gets established. + void SendHttp3GoAway(QuicErrorCode error_code, const std::string& reason); // Same as SendHttp3GoAway(). TODO(bnc): remove when // gfe2_reloadable_flag_quic_goaway_with_max_stream_id flag is deprecated.
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc index 9f1a80b..3606949 100644 --- a/quic/core/http/quic_spdy_session_test.cc +++ b/quic/core/http/quic_spdy_session_test.cc
@@ -24,6 +24,7 @@ #include "net/third_party/quiche/src/quic/core/quic_config.h" #include "net/third_party/quiche/src/quic/core/quic_crypto_stream.h" #include "net/third_party/quiche/src/quic/core/quic_data_writer.h" +#include "net/third_party/quiche/src/quic/core/quic_error_codes.h" #include "net/third_party/quiche/src/quic/core/quic_packets.h" #include "net/third_party/quiche/src/quic/core/quic_stream.h" #include "net/third_party/quiche/src/quic/core/quic_types.h" @@ -1101,6 +1102,21 @@ EXPECT_TRUE(session_.GetOrCreateStream(kTestStreamId)); } +TEST_P(QuicSpdySessionTestServer, SendGoAwayWithoutEncryption) { + SetQuicReloadableFlag(quic_encrypted_goaway, true); + if (VersionHasIetfQuicFrames(transport_version())) { + // HTTP/3 GOAWAY has different semantic and thus has its own test. + return; + } + EXPECT_CALL( + *connection_, + CloseConnection(QUIC_PEER_GOING_AWAY, "Going Away.", + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET)); + EXPECT_CALL(*connection_, SendControlFrame(_)).Times(0); + session_.SendGoAway(QUIC_PEER_GOING_AWAY, "Going Away."); + EXPECT_FALSE(session_.goaway_sent()); +} + TEST_P(QuicSpdySessionTestServer, SendHttp3GoAway) { if (!VersionUsesHttp3(transport_version())) { return; @@ -1121,7 +1137,7 @@ // retried on a different connection. EXPECT_CALL(debug_visitor, OnGoAwayFrameSent(/* stream_id = */ 0)); } - session_.SendHttp3GoAway(); + session_.SendHttp3GoAway(QUIC_PEER_GOING_AWAY, "Goaway"); EXPECT_TRUE(session_.goaway_sent()); // New incoming stream is not reset. @@ -1132,7 +1148,20 @@ // No more GOAWAY frames are sent because they could not convey new // information to the client. - session_.SendHttp3GoAway(); + session_.SendHttp3GoAway(QUIC_PEER_GOING_AWAY, "Goaway"); +} + +TEST_P(QuicSpdySessionTestServer, SendHttp3GoAwayWithoutEncryption) { + SetQuicReloadableFlag(quic_encrypted_goaway, true); + if (!VersionUsesHttp3(transport_version())) { + return; + } + EXPECT_CALL( + *connection_, + CloseConnection(QUIC_PEER_GOING_AWAY, "Goaway", + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET)); + session_.SendHttp3GoAway(QUIC_PEER_GOING_AWAY, "Goaway"); + EXPECT_FALSE(session_.goaway_sent()); } TEST_P(QuicSpdySessionTestServer, SendHttp3GoAwayAfterStreamIsCreated) { @@ -1159,12 +1188,12 @@ // starting with stream ID = 4 can be retried on a different connection. EXPECT_CALL(debug_visitor, OnGoAwayFrameSent(/* stream_id = */ 4)); } - session_.SendHttp3GoAway(); + session_.SendHttp3GoAway(QUIC_PEER_GOING_AWAY, "Goaway"); EXPECT_TRUE(session_.goaway_sent()); // No more GOAWAY frames are sent because they could not convey new // information to the client. - session_.SendHttp3GoAway(); + session_.SendHttp3GoAway(QUIC_PEER_GOING_AWAY, "Goaway"); } TEST_P(QuicSpdySessionTestServer, SendHttp3Shutdown) { @@ -1212,7 +1241,7 @@ session_.SendHttp3Shutdown(); EXPECT_TRUE(session_.goaway_sent()); - session_.SendHttp3GoAway(); + session_.SendHttp3GoAway(QUIC_PEER_GOING_AWAY, "Goaway"); const QuicStreamId kTestStreamId = GetNthClientInitiatedBidirectionalStreamId(transport_version(), 0);
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h index 908aa12..4f435a6 100644 --- a/quic/core/quic_flags_list.h +++ b/quic/core/quic_flags_list.h
@@ -36,6 +36,7 @@ QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_enable_server_on_wire_ping, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_enable_token_based_address_validation, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_encrypted_control_frames, false) +QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_encrypted_goaway, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_extract_x509_subject_using_certificate_view, true) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_willing_and_able_to_write2, true) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_goaway_with_max_stream_id, false)
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index 73370d3..b2d59c5 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -931,6 +931,16 @@ const std::string& reason) { // GOAWAY frame is not supported in IETF QUIC. DCHECK(!VersionHasIetfQuicFrames(transport_version())); + if (GetQuicReloadableFlag(quic_encrypted_goaway)) { + QUIC_RELOADABLE_FLAG_COUNT_N(quic_encrypted_goaway, 1, 2); + if (!IsEncryptionEstablished()) { + QUIC_CODE_COUNT(quic_goaway_before_encryption_established); + connection_->CloseConnection( + error_code, reason, + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + return; + } + } if (transport_goaway_sent_) { return; }
diff --git a/quic/test_tools/quic_test_server.cc b/quic/test_tools/quic_test_server.cc index c69ec29..886d434 100644 --- a/quic/test_tools/quic_test_server.cc +++ b/quic/test_tools/quic_test_server.cc
@@ -229,7 +229,7 @@ void ImmediateGoAwaySession::OnStreamFrame(const QuicStreamFrame& frame) { if (VersionUsesHttp3(transport_version())) { - SendHttp3GoAway(); + SendHttp3GoAway(QUIC_PEER_GOING_AWAY, ""); } else { SendGoAway(QUIC_PEER_GOING_AWAY, ""); } @@ -253,7 +253,7 @@ std::move(encrypter)); if (VersionUsesHttp3(transport_version())) { if (IsEncryptionEstablished() && !goaway_sent()) { - SendHttp3GoAway(); + SendHttp3GoAway(QUIC_PEER_GOING_AWAY, ""); } } }