Make QuicConnection to do reverse path validation based on IETF version instead of bool validate_client_addresses_.
PiperOrigin-RevId: 538238187
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc
index 8822a9a..fae2512 100644
--- a/quiche/quic/core/quic_connection_test.cc
+++ b/quiche/quic/core/quic_connection_test.cc
@@ -1774,7 +1774,7 @@
EXPECT_EQ(2 * default_init_rtt, rtt_stats->initial_rtt());
EXPECT_EQ(1u, manager_->GetConsecutivePtoCount());
EXPECT_EQ(manager_->GetSendAlgorithm(), send_algorithm_);
- if (connection_.validate_client_address()) {
+ if (version().HasIetfQuicFrames()) {
EXPECT_EQ(NO_CHANGE, connection_.active_effective_peer_migration_type());
EXPECT_EQ(1u, connection_.GetStats().num_validated_peer_migration);
EXPECT_EQ(1u, connection_.num_linkable_client_migration());
@@ -1783,7 +1783,7 @@
TEST_P(QuicConnectionTest, PeerIpAddressChangeAtServer) {
set_perspective(Perspective::IS_SERVER);
- if (!connection_.validate_client_address() ||
+ if (!version().SupportsAntiAmplificationLimit() ||
GetQuicFlag(quic_enforce_strict_amplification_factor)) {
return;
}
@@ -2056,7 +2056,7 @@
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kNewEffectivePeerAddress, connection_.effective_peer_address());
EXPECT_EQ(kPeerAddress, writer_->last_write_peer_address());
- if (connection_.validate_client_address()) {
+ if (GetParam().version.HasIetfQuicFrames()) {
EXPECT_EQ(NO_CHANGE, connection_.active_effective_peer_migration_type());
EXPECT_EQ(1u, connection_.GetStats().num_validated_peer_migration);
EXPECT_EQ(1u, connection_.num_linkable_client_migration());
@@ -2069,7 +2069,7 @@
connection_.ReturnEffectivePeerAddressForNextPacket(kNewEffectivePeerAddress);
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0);
- if (!connection_.validate_client_address()) {
+ if (!GetParam().version.HasIetfQuicFrames()) {
// ack_frame is used to complete the migration started by the last packet,
// we need to make sure a new migration does not start after the previous
// one is completed.
@@ -2095,7 +2095,7 @@
kFinalPeerAddress, ENCRYPTION_FORWARD_SECURE);
EXPECT_EQ(kFinalPeerAddress, connection_.peer_address());
EXPECT_EQ(kNewerEffectivePeerAddress, connection_.effective_peer_address());
- if (connection_.validate_client_address()) {
+ if (GetParam().version.HasIetfQuicFrames()) {
EXPECT_EQ(NO_CHANGE, connection_.active_effective_peer_migration_type());
EXPECT_EQ(send_algorithm_,
connection_.sent_packet_manager().GetSendAlgorithm());
@@ -2110,7 +2110,7 @@
connection_.ReturnEffectivePeerAddressForNextPacket(
kNewestEffectivePeerAddress);
EXPECT_CALL(visitor_, OnConnectionMigration(IPV6_TO_IPV4_CHANGE)).Times(1);
- if (!connection_.validate_client_address()) {
+ if (!GetParam().version.HasIetfQuicFrames()) {
EXPECT_CALL(*send_algorithm_, OnConnectionMigration()).Times(1);
}
ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
@@ -2119,7 +2119,7 @@
EXPECT_EQ(kNewestEffectivePeerAddress, connection_.effective_peer_address());
EXPECT_EQ(IPV6_TO_IPV4_CHANGE,
connection_.active_effective_peer_migration_type());
- if (connection_.validate_client_address()) {
+ if (GetParam().version.HasIetfQuicFrames()) {
EXPECT_NE(send_algorithm_,
connection_.sent_packet_manager().GetSendAlgorithm());
EXPECT_EQ(kFinalPeerAddress, writer_->last_write_peer_address());
@@ -2586,15 +2586,13 @@
.Times(1);
} else {
EXPECT_CALL(visitor_, OnPacketReceived(_, _, _)).Times(0);
- if (connection_.validate_client_address()) {
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _))
- .Times(AtLeast(1u))
- .WillOnce(Invoke([&]() {
- EXPECT_EQ(1u, writer_->path_challenge_frames().size());
- EXPECT_EQ(1u, writer_->path_response_frames().size());
- payload = writer_->path_challenge_frames().front().data_buffer;
- }));
- }
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _))
+ .Times(AtLeast(1u))
+ .WillOnce(Invoke([&]() {
+ EXPECT_EQ(1u, writer_->path_challenge_frames().size());
+ EXPECT_EQ(1u, writer_->path_response_frames().size());
+ payload = writer_->path_challenge_frames().front().data_buffer;
+ }));
}
// Process a probing packet from a new peer address on server side
// is effectively receiving a connectivity probing.
@@ -2636,24 +2634,7 @@
EXPECT_EQ(2 * received->length(),
QuicConnectionPeer::BytesReceivedOnAlternativePath(&connection_));
- bool success = false;
- if (!connection_.validate_client_address()) {
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _))
- .Times(AtLeast(1u))
- .WillOnce(Invoke([&]() {
- EXPECT_EQ(1u, writer_->path_challenge_frames().size());
- payload = writer_->path_challenge_frames().front().data_buffer;
- }));
-
- connection_.ValidatePath(
- std::make_unique<TestQuicPathValidationContext>(
- connection_.self_address(), kNewPeerAddress, writer_.get()),
- std::make_unique<TestValidationResultDelegate>(
- &connection_, connection_.self_address(), kNewPeerAddress,
- &success),
- PathValidationReason::kReasonUnknown);
- }
- EXPECT_EQ((connection_.validate_client_address() ? 2 : 3) * bytes_sent,
+ EXPECT_EQ(2 * bytes_sent,
QuicConnectionPeer::BytesSentOnAlternativePath(&connection_));
QuicFrames frames;
frames.push_back(QuicFrame(QuicPathResponseFrame(99, payload)));
@@ -2662,9 +2643,7 @@
ENCRYPTION_FORWARD_SECURE);
EXPECT_LT(2 * received->length(),
QuicConnectionPeer::BytesReceivedOnAlternativePath(&connection_));
- if (connection_.validate_client_address()) {
- EXPECT_TRUE(QuicConnectionPeer::IsAlternativePathValidated(&connection_));
- }
+ EXPECT_TRUE(QuicConnectionPeer::IsAlternativePathValidated(&connection_));
// Receiving another probing packet from a newer address with a different
// port shouldn't trigger another reverse path validation.
QuicSocketAddress kNewerPeerAddress(QuicIpAddress::Loopback4(),
@@ -2676,8 +2655,7 @@
clock_.Now()));
ProcessReceivedPacket(kSelfAddress, kNewerPeerAddress, *received);
EXPECT_FALSE(connection_.HasPendingPathValidation());
- EXPECT_EQ(connection_.validate_client_address(),
- QuicConnectionPeer::IsAlternativePathValidated(&connection_));
+ EXPECT_TRUE(QuicConnectionPeer::IsAlternativePathValidated(&connection_));
}
// Process another packet with the old peer address on server side will not
@@ -11630,13 +11608,11 @@
// This packet isn't sent actually, instead it is buffered in the
// connection.
EXPECT_EQ(1u, writer_->packets_write_attempts());
- if (connection_.validate_client_address()) {
- EXPECT_EQ(1u, writer_->path_response_frames().size());
- EXPECT_EQ(0,
- memcmp(&path_challenge_payload,
- &writer_->path_response_frames().front().data_buffer,
- sizeof(path_challenge_payload)));
- }
+ EXPECT_EQ(1u, writer_->path_response_frames().size());
+ EXPECT_EQ(0,
+ memcmp(&path_challenge_payload,
+ &writer_->path_response_frames().front().data_buffer,
+ sizeof(path_challenge_payload)));
EXPECT_EQ(1u, writer_->path_challenge_frames().size());
EXPECT_EQ(1u, writer_->padding_frames().size());
EXPECT_EQ(kNewPeerAddress, writer_->last_write_peer_address());
@@ -11645,25 +11621,13 @@
// Only one PATH_CHALLENGE should be sent out.
EXPECT_EQ(0u, writer_->path_challenge_frames().size());
}));
- bool success = false;
- if (connection_.validate_client_address()) {
- // Receiving a PATH_CHALLENGE from the new peer address should trigger
- // address validation.
- QuicFrames frames;
- frames.push_back(
- QuicFrame(QuicPathChallengeFrame(0, path_challenge_payload)));
- ProcessFramesPacketWithAddresses(frames, kSelfAddress, kNewPeerAddress,
- ENCRYPTION_FORWARD_SECURE);
- } else {
- // Manually start to validate the new peer address.
- connection_.ValidatePath(
- std::make_unique<TestQuicPathValidationContext>(
- connection_.self_address(), kNewPeerAddress, writer_.get()),
- std::make_unique<TestValidationResultDelegate>(
- &connection_, connection_.self_address(), kNewPeerAddress,
- &success),
- PathValidationReason::kReasonUnknown);
- }
+ // Receiving a PATH_CHALLENGE from the new peer address should trigger address
+ // validation.
+ QuicFrames frames;
+ frames.push_back(
+ QuicFrame(QuicPathChallengeFrame(0, path_challenge_payload)));
+ ProcessFramesPacketWithAddresses(frames, kSelfAddress, kNewPeerAddress,
+ ENCRYPTION_FORWARD_SECURE);
EXPECT_EQ(1u, writer_->packets_write_attempts());
// Try again with the new socket blocked from the beginning. The 2nd
@@ -11877,8 +11841,7 @@
/*port=*/23456);
EXPECT_CALL(visitor_, OnConnectionMigration(IPV6_TO_IPV4_CHANGE));
- EXPECT_CALL(*send_algorithm_, OnConnectionMigration())
- .Times(connection_.validate_client_address() ? 0u : 1u);
+ EXPECT_CALL(*send_algorithm_, OnConnectionMigration()).Times(0u);
EXPECT_CALL(visitor_, OnStreamFrame(_))
.WillOnce(Invoke([=](const QuicStreamFrame& frame) {
// Send some data on the stream. The STREAM_FRAME should be built into
@@ -11888,8 +11851,7 @@
return notifier_.WriteOrBufferData(frame.stream_id, data.length(),
NO_FIN);
}));
- EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _))
- .Times(connection_.validate_client_address() ? 0u : 1u);
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0u);
ProcessFramesPacketWithAddresses(frames, kSelfAddress, kNewPeerAddress,
ENCRYPTION_FORWARD_SECURE);
@@ -11897,20 +11859,16 @@
// PATH_RESPONSE_FRAME.
EXPECT_EQ(1u, writer_->stream_frames().size());
EXPECT_EQ(1u, writer_->path_response_frames().size());
- EXPECT_EQ(connection_.validate_client_address() ? 1u : 0u,
- writer_->path_challenge_frames().size());
+ EXPECT_EQ(1u, writer_->path_challenge_frames().size());
// The final check is to ensure that the random data in the response
// matches the random data from the challenge.
EXPECT_EQ(0, memcmp(path_frame_buffer.data(),
&(writer_->path_response_frames().front().data_buffer),
sizeof(path_frame_buffer)));
- EXPECT_EQ(connection_.validate_client_address() ? 1u : 0u,
- writer_->path_challenge_frames().size());
+ EXPECT_EQ(1u, writer_->path_challenge_frames().size());
EXPECT_EQ(1u, writer_->padding_frames().size());
EXPECT_EQ(kNewPeerAddress, writer_->last_write_peer_address());
- if (connection_.validate_client_address()) {
- EXPECT_TRUE(connection_.HasPendingPathValidation());
- }
+ EXPECT_TRUE(connection_.HasPendingPathValidation());
}
TEST_P(QuicConnectionTest, ReceiveStreamFrameFollowingPathChallenge) {
@@ -11939,16 +11897,14 @@
memcmp(path_frame_buffer.data(),
&(writer_->path_response_frames().front().data_buffer),
sizeof(path_frame_buffer)));
- EXPECT_EQ(connection_.validate_client_address() ? 1u : 0u,
- writer_->path_challenge_frames().size());
+ EXPECT_EQ(1u, writer_->path_challenge_frames().size());
EXPECT_EQ(1u, writer_->padding_frames().size());
EXPECT_EQ(kNewPeerAddress, writer_->last_write_peer_address());
received_packet_size =
QuicConnectionPeer::BytesReceivedOnAlternativePath(&connection_);
}));
EXPECT_CALL(visitor_, OnConnectionMigration(IPV6_TO_IPV4_CHANGE));
- EXPECT_CALL(*send_algorithm_, OnConnectionMigration())
- .Times(connection_.validate_client_address() ? 0u : 1u);
+ EXPECT_CALL(*send_algorithm_, OnConnectionMigration()).Times(0u);
EXPECT_CALL(visitor_, OnStreamFrame(_))
.WillOnce(Invoke([=](const QuicStreamFrame& frame) {
// Send some data on the stream. The STREAM_FRAME should be built into a
@@ -11961,9 +11917,6 @@
ProcessFramesPacketWithAddresses(frames, kSelfAddress, kNewPeerAddress,
ENCRYPTION_FORWARD_SECURE);
- if (!connection_.validate_client_address()) {
- return;
- }
EXPECT_TRUE(connection_.HasPendingPathValidation());
EXPECT_EQ(0u,
QuicConnectionPeer::BytesReceivedOnAlternativePath(&connection_));
@@ -14224,7 +14177,7 @@
TEST_P(QuicConnectionTest,
ProbedOnAnotherPathAfterPeerIpAddressChangeAtServer) {
PathProbeTestInit(Perspective::IS_SERVER);
- if (!connection_.validate_client_address()) {
+ if (!version().HasIetfQuicFrames()) {
return;
}
@@ -14987,7 +14940,7 @@
// Regression test for b/182571515
TEST_P(QuicConnectionTest, LostDataThenGetAcknowledged) {
set_perspective(Perspective::IS_SERVER);
- if (!connection_.validate_client_address() ||
+ if (!version().SupportsAntiAmplificationLimit() ||
GetQuicFlag(quic_enforce_strict_amplification_factor)) {
return;
}
@@ -15398,7 +15351,7 @@
// Regression test for b/201643321.
TEST_P(QuicConnectionTest, FailedToRetransmitShlo) {
- if (!version().HasIetfQuicFrames() ||
+ if (!version().SupportsAntiAmplificationLimit() ||
GetQuicFlag(quic_enforce_strict_amplification_factor)) {
return;
}
