Move enforcement of single RETRY from QuicConnection to QuicFramer
This change is a no-op. Before this change, we would first validate the retry integrity tag, and then check whether we had already received a RETRY. The issue with that was that it would QUIC_DLOG(ERROR) when failing to validate. With this change, we drop that second RETRY without validating it, removing the log. This log was only in debug builds so this behavior change does not impact production. This CL also adds a new flag to the toy client which helped debug this issue.
PiperOrigin-RevId: 347645803
Change-Id: Iea5943c115f1267ee906f476bca17a184dedee4d
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 9c49fef..1a39ec5 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -356,7 +356,6 @@
processing_ack_frame_(false),
supports_release_time_(false),
release_time_into_future_(QuicTime::Delta::Zero()),
- drop_incoming_retry_packets_(false),
bytes_received_before_address_validation_(0),
bytes_sent_before_address_validation_(0),
address_validated_(false),
@@ -939,12 +938,7 @@
return;
}
}
- if (drop_incoming_retry_packets_) {
- QUIC_DLOG(ERROR) << "Ignoring RETRY with token "
- << absl::BytesToHexString(retry_token);
- return;
- }
- drop_incoming_retry_packets_ = true;
+ framer_.set_drop_incoming_retry_packets(true);
stats_.retry_packet_processed = true;
QUIC_DLOG(INFO) << "Received RETRY, replacing connection ID "
<< server_connection_id_ << " with " << new_connection_id
@@ -1000,7 +994,7 @@
const QuicPacketHeader& header) {
// As soon as we receive an initial we start ignoring subsequent retries.
if (header.version_flag && header.long_packet_type == INITIAL) {
- drop_incoming_retry_packets_ = true;
+ framer_.set_drop_incoming_retry_packets(true);
}
QuicConnectionId server_connection_id =
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 56a1813..3424735 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -1875,9 +1875,6 @@
// the source connection ID from that packet.
absl::optional<QuicConnectionId> retry_source_connection_id_;
- // Indicates whether received RETRY packets should be dropped.
- bool drop_incoming_retry_packets_;
-
// Bytes received before address validation. Only used when
// EnforceAntiAmplificationLimit returns true.
size_t bytes_received_before_address_validation_;
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index 7803b59..536021e 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -1555,6 +1555,10 @@
bool QuicFramer::ProcessRetryPacket(QuicDataReader* reader,
const QuicPacketHeader& header) {
DCHECK_EQ(Perspective::IS_CLIENT, perspective_);
+ if (drop_incoming_retry_packets_) {
+ QUIC_DLOG(INFO) << "Ignoring received RETRY packet";
+ return true;
+ }
if (version_.UsesTls()) {
DCHECK(version_.HasLengthPrefixedConnectionIds()) << version_;
diff --git a/quic/core/quic_framer.h b/quic/core/quic_framer.h
index 9f12d2b..1c9483e 100644
--- a/quic/core/quic_framer.h
+++ b/quic/core/quic_framer.h
@@ -705,6 +705,10 @@
}
uint32_t peer_ack_delay_exponent() const { return peer_ack_delay_exponent_; }
+ void set_drop_incoming_retry_packets(bool drop_incoming_retry_packets) {
+ drop_incoming_retry_packets_ = drop_incoming_retry_packets;
+ }
+
private:
friend class test::QuicFramerPeer;
@@ -1151,6 +1155,9 @@
// Indicates whether this framer supports multiple packet number spaces.
bool supports_multiple_packet_number_spaces_;
+ // Indicates whether received RETRY packets should be dropped.
+ bool drop_incoming_retry_packets_ = false;
+
// The length in bytes of the last packet number written to an IETF-framed
// packet.
size_t last_written_packet_number_length_;
diff --git a/quic/tools/quic_toy_client.cc b/quic/tools/quic_toy_client.cc
index c25411d..03dcf65 100644
--- a/quic/tools/quic_toy_client.cc
+++ b/quic/tools/quic_toy_client.cc
@@ -154,6 +154,13 @@
DEFINE_QUIC_COMMAND_LINE_FLAG(
bool,
+ multi_packet_chlo,
+ false,
+ "If true, add a transport parameter to make the ClientHello span two "
+ "packets. Only works with QUIC+TLS.");
+
+DEFINE_QUIC_COMMAND_LINE_FLAG(
+ bool,
redirect_is_success,
true,
"If true, an HTTP response code of 3xx is considered to be a "
@@ -266,6 +273,15 @@
config.SetClientConnectionOptions(
ParseQuicTagVector(client_connection_options_string));
}
+ if (GetQuicFlag(FLAGS_multi_packet_chlo)) {
+ // Make the ClientHello span multiple packets by adding a custom transport
+ // parameter.
+ constexpr auto kCustomParameter =
+ static_cast<TransportParameters::TransportParameterId>(0x173E);
+ std::string custom_value(2000, '?');
+ config.custom_transport_parameters_to_send()[kCustomParameter] =
+ custom_value;
+ }
int address_family_for_lookup = AF_UNSPEC;
if (GetQuicFlag(FLAGS_ip_version_for_host_lookup) == "4") {
