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; }