Automated g4 rollback of changelist 436769765.
*** Reason for rollback ***
Blocking merge because there are several tests with this flag disabled.
*** Original change description ***
Deprecate --gfe2_reloadable_flag_quic_pass_path_response_to_validator.
***
PiperOrigin-RevId: 437128224
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc
index 2a0603d..b1b9b4f 100644
--- a/quic/core/http/end_to_end_test.cc
+++ b/quic/core/http/end_to_end_test.cc
@@ -5332,7 +5332,8 @@
TEST_P(EndToEndPacketReorderingTest, ReorderedPathChallenge) {
ASSERT_TRUE(Initialize());
- if (!version_.HasIetfQuicFrames()) {
+ if (!version_.HasIetfQuicFrames() ||
+ !client_->client()->session()->connection()->use_path_validator()) {
return;
}
client_.reset(EndToEndTest::CreateQuicClient(nullptr));
@@ -5392,7 +5393,8 @@
TEST_P(EndToEndPacketReorderingTest, PathValidationFailure) {
ASSERT_TRUE(Initialize());
- if (!version_.HasIetfQuicFrames()) {
+ if (!version_.HasIetfQuicFrames() ||
+ !client_->client()->session()->connection()->use_path_validator()) {
return;
}
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 17f4b7d..6edaa8f 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -625,7 +625,7 @@
QUIC_RELOADABLE_FLAG_COUNT(
quic_remove_connection_migration_connection_option);
}
- if (framer_.version().HasIetfQuicFrames() &&
+ if (framer_.version().HasIetfQuicFrames() && use_path_validator_ &&
count_bytes_on_alternative_path_separately_ &&
GetQuicReloadableFlag(quic_server_reverse_validate_new_path3) &&
(remove_connection_migration_connection_option ||
@@ -1751,8 +1751,19 @@
debug_visitor_->OnPathResponseFrame(frame);
}
MaybeUpdateAckTimeout();
+ if (use_path_validator_) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_pass_path_response_to_validator, 1, 4);
path_validator_.OnPathResponse(
frame.data_buffer, last_received_packet_info_.destination_address);
+ } else {
+ if (!transmitted_connectivity_probe_payload_ ||
+ *transmitted_connectivity_probe_payload_ != frame.data_buffer) {
+ // Is not for the probe we sent, ignore it.
+ return true;
+ }
+ // Have received the matching PATH RESPONSE, saved payload no longer valid.
+ transmitted_connectivity_probe_payload_ = nullptr;
+ }
return connected_;
}
@@ -2237,6 +2248,8 @@
QUICHE_DCHECK(version().HasIetfInvariantHeader());
QUICHE_DCHECK_EQ(perspective_, Perspective::IS_CLIENT);
+ if (use_path_validator_) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_pass_path_response_to_validator, 4, 4);
if (!IsDefaultPath(last_received_packet_info_.destination_address,
last_received_packet_info_.source_address)) {
// This packet is received on a probing path. Do not close connection.
@@ -2252,6 +2265,12 @@
}
return;
}
+ } else if (!visitor_->ValidateStatelessReset(
+ last_received_packet_info_.destination_address,
+ last_received_packet_info_.source_address)) {
+ // This packet is received on a probing path. Do not close connection.
+ return;
+ }
const std::string error_details = "Received stateless reset.";
QUIC_CODE_COUNT(quic_tear_down_local_connection_on_stateless_reset);
@@ -3518,7 +3537,8 @@
// The write failed, but the writer is not blocked, so return true.
return true;
}
- if (!send_on_current_path) {
+ if (use_path_validator_ && !send_on_current_path) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_pass_path_response_to_validator, 2, 4);
// Only handle MSG_TOO_BIG as error on current path.
return true;
}
@@ -4592,8 +4612,10 @@
// Cancel the alarms so they don't trigger any action now that the
// connection is closed.
CancelAllAlarms();
+ if (use_path_validator_) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_pass_path_response_to_validator, 3, 4);
CancelPathValidation();
-
+ }
peer_issued_cid_manager_.reset();
self_issued_cid_manager_.reset();
}
@@ -5005,12 +5027,16 @@
} else {
// IETF QUIC path challenge.
// Send a path probe request using IETF QUIC PATH_CHALLENGE frame.
- QuicPathFrameBuffer transmitted_connectivity_probe_payload;
- random_generator_->RandBytes(&transmitted_connectivity_probe_payload,
+ transmitted_connectivity_probe_payload_ =
+ std::make_unique<QuicPathFrameBuffer>();
+ random_generator_->RandBytes(transmitted_connectivity_probe_payload_.get(),
sizeof(QuicPathFrameBuffer));
probing_packet =
packet_creator_.SerializePathChallengeConnectivityProbingPacket(
- transmitted_connectivity_probe_payload);
+ *transmitted_connectivity_probe_payload_);
+ if (!probing_packet) {
+ transmitted_connectivity_probe_payload_ = nullptr;
+ }
}
QUICHE_DCHECK_EQ(IsRetransmittable(*probing_packet), NO_RETRANSMITTABLE_DATA);
return WritePacketUsingWriter(std::move(probing_packet), probing_writer,
@@ -6471,6 +6497,7 @@
void QuicConnection::ValidatePath(
std::unique_ptr<QuicPathValidationContext> context,
std::unique_ptr<QuicPathValidator::ResultDelegate> result_delegate) {
+ QUICHE_DCHECK(use_path_validator_);
if (!connection_migration_use_new_cid_ &&
perspective_ == Perspective::IS_CLIENT &&
!IsDefaultPath(context->self_address(), context->peer_address())) {
@@ -6587,14 +6614,17 @@
}
bool QuicConnection::HasPendingPathValidation() const {
+ QUICHE_DCHECK(use_path_validator_);
return path_validator_.HasPendingPathValidation();
}
QuicPathValidationContext* QuicConnection::GetPathValidationContext() const {
+ QUICHE_DCHECK(use_path_validator_);
return path_validator_.GetContext();
}
void QuicConnection::CancelPathValidation() {
+ QUICHE_DCHECK(use_path_validator_);
path_validator_.CancelPathValidation();
}
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index a0738e5..a38f2c0 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -141,6 +141,12 @@
// bandwidth. Returns true if data was sent, false otherwise.
virtual bool SendProbingData() = 0;
+ // Called when stateless reset packet is received. Returns true if the
+ // connection needs to be closed.
+ virtual bool ValidateStatelessReset(
+ const quic::QuicSocketAddress& self_address,
+ const quic::QuicSocketAddress& peer_address) = 0;
+
// Called when the connection experiences a change in congestion window.
virtual void OnCongestionWindowChange(QuicTime now) = 0;
@@ -1147,6 +1153,8 @@
// Can only be set if this is a client connection.
void EnableLegacyVersionEncapsulation(const std::string& server_name);
+ bool use_path_validator() const { return use_path_validator_; }
+
// If now is close to idle timeout, returns true and sends a connectivity
// probing packet to test the connection for liveness. Otherwise, returns
// false.
@@ -2137,6 +2145,12 @@
// Time this connection can release packets into the future.
QuicTime::Delta release_time_into_future_;
+ // Payload of most recently transmitted IETF QUIC connectivity
+ // probe packet (the PATH_CHALLENGE payload). This implementation transmits
+ // only one PATH_CHALLENGE per connectivity probe, so only one
+ // QuicPathFrameBuffer is needed.
+ std::unique_ptr<QuicPathFrameBuffer> transmitted_connectivity_probe_payload_;
+
// Payloads that were received in the most recent probe. This needs to be a
// Deque because the peer might no be using this implementation, and others
// might send a packet with more than one PATH_CHALLENGE, so all need to be
@@ -2195,6 +2209,9 @@
size_t anti_amplification_factor_ =
GetQuicFlag(FLAGS_quic_anti_amplification_factor);
+ bool use_path_validator_ =
+ GetQuicReloadableFlag(quic_pass_path_response_to_validator);
+
// True if AckFrequencyFrame is supported.
bool can_receive_ack_frequency_frame_ = false;
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 6a943c5..5effe44 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -2499,6 +2499,7 @@
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
if (GetParam().version.HasIetfQuicFrames() &&
+ connection_.use_path_validator() &&
GetQuicReloadableFlag(quic_count_bytes_on_alternative_path_seperately)) {
QuicByteCount bytes_sent =
QuicConnectionPeer::BytesSentOnAlternativePath(&connection_);
@@ -6954,6 +6955,9 @@
kTestStatelessResetToken));
std::unique_ptr<QuicReceivedPacket> received(
ConstructReceivedPacket(*packet, QuicTime::Zero()));
+ if (!connection_.use_path_validator()) {
+ EXPECT_CALL(visitor_, ValidateStatelessReset(_, _)).WillOnce(Return(true));
+ }
EXPECT_CALL(visitor_, OnConnectionClosed(_, ConnectionCloseSource::FROM_PEER))
.WillOnce(Invoke(this, &QuicConnectionTest::SaveConnectionCloseFrame));
connection_.ProcessUdpPacket(kSelfAddress, kPeerAddress, *received);
@@ -8890,7 +8894,8 @@
}
TEST_P(QuicConnectionTest, ClientResponseToPathChallengeOnAlternativeSocket) {
- if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version) ||
+ !connection_.use_path_validator()) {
return;
}
PathProbeTestInit(Perspective::IS_CLIENT);
@@ -11886,7 +11891,8 @@
}
TEST_P(QuicConnectionTest, PathValidationOnNewSocketSuccess) {
- if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version) ||
+ !connection_.use_path_validator()) {
return;
}
PathProbeTestInit(Perspective::IS_CLIENT);
@@ -11919,7 +11925,8 @@
}
TEST_P(QuicConnectionTest, NewPathValidationCancelsPreviousOne) {
- if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version) ||
+ !connection_.use_path_validator()) {
return;
}
PathProbeTestInit(Perspective::IS_CLIENT);
@@ -11977,7 +11984,8 @@
// Regression test for b/182571515.
TEST_P(QuicConnectionTest, PathValidationRetry) {
- if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version) ||
+ !connection_.use_path_validator()) {
return;
}
PathProbeTestInit(Perspective::IS_CLIENT);
@@ -12009,7 +12017,8 @@
}
TEST_P(QuicConnectionTest, PathValidationReceivesStatelessReset) {
- if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version) ||
+ !connection_.use_path_validator()) {
return;
}
PathProbeTestInit(Perspective::IS_CLIENT);
@@ -12095,7 +12104,8 @@
// Tests that PATH_CHALLENGE is dropped if it is sent via the default writer
// and the writer is blocked.
TEST_P(QuicConnectionTest, SendPathChallengeUsingBlockedDefaultSocket) {
- if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version) ||
+ !connection_.use_path_validator()) {
return;
}
PathProbeTestInit(Perspective::IS_SERVER);
@@ -12168,7 +12178,8 @@
// Tests that write error on the alternate socket should be ignored.
TEST_P(QuicConnectionTest, SendPathChallengeFailOnNewSocket) {
- if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version) ||
+ !connection_.use_path_validator()) {
return;
}
PathProbeTestInit(Perspective::IS_CLIENT);
@@ -12199,7 +12210,8 @@
// Tests that write error while sending PATH_CHALLANGE from the default socket
// should close the connection.
TEST_P(QuicConnectionTest, SendPathChallengeFailOnDefaultPath) {
- if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version) ||
+ !connection_.use_path_validator()) {
return;
}
PathProbeTestInit(Perspective::IS_CLIENT);
@@ -12233,7 +12245,8 @@
}
TEST_P(QuicConnectionTest, SendPathChallengeFailOnAlternativePeerAddress) {
- if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version) ||
+ !connection_.use_path_validator()) {
return;
}
PathProbeTestInit(Perspective::IS_CLIENT);
@@ -12264,7 +12277,8 @@
TEST_P(QuicConnectionTest,
SendPathChallengeFailPacketTooBigOnAlternativePeerAddress) {
- if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version) ||
+ !connection_.use_path_validator()) {
return;
}
PathProbeTestInit(Perspective::IS_CLIENT);
@@ -13457,7 +13471,8 @@
}
TEST_P(QuicConnectionTest, MigrateToNewPathDuringProbing) {
- if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) {
+ if (!VersionHasIetfQuicFrames(connection_.version().transport_version) ||
+ !connection_.use_path_validator()) {
return;
}
PathProbeTestInit(Perspective::IS_CLIENT);
@@ -14458,7 +14473,7 @@
TEST_P(
QuicConnectionTest,
ReplacePeerIssuedConnectionIdOnBothPathsTriggeredByNewConnectionIdFrame) {
- if (!version().HasIetfQuicFrames() ||
+ if (!version().HasIetfQuicFrames() || !connection_.use_path_validator() ||
!connection_.count_bytes_on_alternative_path_separately()) {
return;
}
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index f053116..ab6a5b3 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -75,6 +75,8 @@
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_ignore_max_push_id, true)
// If true, include stream information in idle timeout connection close detail.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_add_stream_info_to_idle_close_detail, true)
+// If true, pass the received PATH_RESPONSE payload to path validator to move forward the path validation.
+QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_pass_path_response_to_validator, true)
// If true, quic server will send ENABLE_CONNECT_PROTOCOL setting and and endpoint will validate required request/response headers and extended CONNECT mechanism and update code counts of valid/invalid headers.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_verify_request_headers_2, true)
// If true, record addresses that server has sent reset to recently, and do not send reset if the address lives in the set.
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h
index dd1fa42..b8f175e 100644
--- a/quic/core/quic_session.h
+++ b/quic/core/quic_session.h
@@ -139,6 +139,11 @@
bool is_connectivity_probe) override;
void OnCanWrite() override;
bool SendProbingData() override;
+ bool ValidateStatelessReset(
+ const quic::QuicSocketAddress& /*self_address*/,
+ const quic::QuicSocketAddress& /*peer_address*/) override {
+ return true;
+ }
void OnCongestionWindowChange(QuicTime /*now*/) override {}
void OnConnectionMigration(AddressChangeType /*type*/) override {}
// Adds a connection level WINDOW_UPDATE frame.
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h
index ddbc9b9..82262eb 100644
--- a/quic/test_tools/quic_test_utils.h
+++ b/quic/test_tools/quic_test_utils.h
@@ -454,6 +454,9 @@
MOCK_METHOD(void, OnWriteBlocked, (), (override));
MOCK_METHOD(void, OnCanWrite, (), (override));
MOCK_METHOD(bool, SendProbingData, (), (override));
+ MOCK_METHOD(bool, ValidateStatelessReset,
+ (const quic::QuicSocketAddress&, const quic::QuicSocketAddress&),
+ (override));
MOCK_METHOD(void, OnCongestionWindowChange, (QuicTime now), (override));
MOCK_METHOD(void, OnConnectionMigration, (AddressChangeType type),
(override));
diff --git a/quic/test_tools/simulator/quic_endpoint.h b/quic/test_tools/simulator/quic_endpoint.h
index 136410f..7fc4d87 100644
--- a/quic/test_tools/simulator/quic_endpoint.h
+++ b/quic/test_tools/simulator/quic_endpoint.h
@@ -51,6 +51,11 @@
void OnCryptoFrame(const QuicCryptoFrame& frame) override;
void OnCanWrite() override;
bool SendProbingData() override;
+ bool ValidateStatelessReset(
+ const quic::QuicSocketAddress& /*self_address*/,
+ const quic::QuicSocketAddress& /*peer_address*/) override {
+ return true;
+ }
bool WillingAndAbleToWrite() const override;
bool ShouldKeepConnectionAlive() const override;
diff --git a/quic/tools/quic_client_base.cc b/quic/tools/quic_client_base.cc
index e26f08c..e11e25b 100644
--- a/quic/tools/quic_client_base.cc
+++ b/quic/tools/quic_client_base.cc
@@ -289,7 +289,8 @@
bool QuicClientBase::ValidateAndMigrateSocket(const QuicIpAddress& new_host) {
QUICHE_DCHECK(VersionHasIetfQuicFrames(
- session_->connection()->version().transport_version));
+ session_->connection()->version().transport_version) &&
+ session_->connection()->use_path_validator());
if (!connected()) {
return false;
}
