Send version negotiation in response to short connection IDs
This code change is in response to the following spec change:
https://github.com/quicwg/base-drafts/pull/4187/files
We're now expected to not enforce the 64bit minimum on unknown versions.
Protected by FLAGS_quic_reloadable_flag_quic_send_version_negotiation_for_short_connection_ids.
PiperOrigin-RevId: 338099578
Change-Id: I9d28a38464fc8050c166d976bb12e7cafd62633c
diff --git a/quic/core/quic_dispatcher.cc b/quic/core/quic_dispatcher.cc
index ac40b0a..11c6753 100644
--- a/quic/core/quic_dispatcher.cc
+++ b/quic/core/quic_dispatcher.cc
@@ -465,7 +465,16 @@
// connection ID, the dispatcher picks a new one of its expected length.
// Therefore we should never receive a connection ID that is smaller
// than 64 bits and smaller than what we expect.
- if (server_connection_id.length() < kQuicMinimumInitialConnectionIdLength &&
+ bool should_check_short_connection_ids = true;
+ if (GetQuicReloadableFlag(
+ quic_send_version_negotiation_for_short_connection_ids)) {
+ QUIC_RELOADABLE_FLAG_COUNT(
+ quic_send_version_negotiation_for_short_connection_ids);
+ should_check_short_connection_ids =
+ packet_info.version_flag && packet_info.version.IsKnown();
+ }
+ if (should_check_short_connection_ids &&
+ server_connection_id.length() < kQuicMinimumInitialConnectionIdLength &&
server_connection_id.length() < expected_server_connection_id_length_ &&
!allow_short_initial_server_connection_ids_) {
DCHECK(packet_info.version_flag);
diff --git a/quic/core/quic_dispatcher_test.cc b/quic/core/quic_dispatcher_test.cc
index a88eadb..9f692a8 100644
--- a/quic/core/quic_dispatcher_test.cc
+++ b/quic/core/quic_dispatcher_test.cc
@@ -471,6 +471,10 @@
void TestTlsMultiPacketClientHello(bool add_reordering);
+ void TestVersionNegotiationForUnknownVersionInvalidShortInitialConnectionId(
+ const QuicConnectionId& server_connection_id,
+ const QuicConnectionId& client_connection_id);
+
ParsedQuicVersion version_;
MockQuicConnectionHelper mock_helper_;
MockAlarmFactory mock_alarm_factory_;
@@ -1054,7 +1058,7 @@
}
TEST_P(QuicDispatcherTestAllVersions,
- ProcessPacketWithInvalidShortInitialConnectionId) {
+ DropPacketWithKnownVersionAndInvalidShortInitialConnectionId) {
if (!version_.AllowsVariableLengthConnectionIds()) {
return;
}
@@ -1063,14 +1067,59 @@
QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1);
// dispatcher_ should drop this packet.
- EXPECT_CALL(*dispatcher_, CreateQuicSession(_, _, client_address, _, _))
- .Times(0);
+ EXPECT_CALL(*dispatcher_, CreateQuicSession(_, _, _, _, _)).Times(0);
EXPECT_CALL(*time_wait_list_manager_, ProcessPacket(_, _, _, _, _)).Times(0);
EXPECT_CALL(*time_wait_list_manager_, AddConnectionIdToTimeWait(_, _, _))
.Times(0);
ProcessFirstFlight(client_address, EmptyQuicConnectionId());
}
+void QuicDispatcherTestBase::
+ TestVersionNegotiationForUnknownVersionInvalidShortInitialConnectionId(
+ const QuicConnectionId& server_connection_id,
+ const QuicConnectionId& client_connection_id) {
+ SetQuicReloadableFlag(quic_send_version_negotiation_for_short_connection_ids,
+ true);
+ CreateTimeWaitListManager();
+
+ QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1);
+
+ EXPECT_CALL(*dispatcher_, CreateQuicSession(_, _, _, _, _)).Times(0);
+ EXPECT_CALL(*time_wait_list_manager_,
+ SendVersionNegotiationPacket(
+ server_connection_id, client_connection_id,
+ /*ietf_quic=*/true,
+ /*use_length_prefix=*/true, _, _, client_address, _))
+ .Times(1);
+ ProcessFirstFlight(ParsedQuicVersion::ReservedForNegotiation(),
+ client_address, server_connection_id,
+ client_connection_id);
+}
+
+TEST_P(QuicDispatcherTestOneVersion,
+ VersionNegotiationForUnknownVersionInvalidShortInitialConnectionId) {
+ TestVersionNegotiationForUnknownVersionInvalidShortInitialConnectionId(
+ EmptyQuicConnectionId(), EmptyQuicConnectionId());
+}
+
+TEST_P(QuicDispatcherTestOneVersion,
+ VersionNegotiationForUnknownVersionInvalidShortInitialConnectionId2) {
+ char server_connection_id_bytes[3] = {1, 2, 3};
+ QuicConnectionId server_connection_id(server_connection_id_bytes,
+ sizeof(server_connection_id_bytes));
+ TestVersionNegotiationForUnknownVersionInvalidShortInitialConnectionId(
+ server_connection_id, EmptyQuicConnectionId());
+}
+
+TEST_P(QuicDispatcherTestOneVersion,
+ VersionNegotiationForUnknownVersionInvalidShortInitialConnectionId3) {
+ char client_connection_id_bytes[8] = {1, 2, 3, 4, 5, 6, 7, 8};
+ QuicConnectionId client_connection_id(client_connection_id_bytes,
+ sizeof(client_connection_id_bytes));
+ TestVersionNegotiationForUnknownVersionInvalidShortInitialConnectionId(
+ EmptyQuicConnectionId(), client_connection_id);
+}
+
TEST_P(QuicDispatcherTestOneVersion, VersionsChangeInFlight) {
VerifyVersionNotSupported(QuicVersionReservedForNegotiation());
for (ParsedQuicVersion version : CurrentSupportedVersions()) {
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index 19b0cd5..03b27f2 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -6671,8 +6671,7 @@
return false;
}
if (destination_connection_id_length > kQuicMaxConnectionId4BitLength ||
- destination_connection_id_length <
- kQuicMinimumInitialConnectionIdLength) {
+ destination_connection_id_length < kQuicDefaultConnectionIdLength) {
QUIC_BUG << "Invalid connection_id_length";
return false;
}
diff --git a/quic/core/quic_versions.cc b/quic/core/quic_versions.cc
index d2b5774..98bef7f 100644
--- a/quic/core/quic_versions.cc
+++ b/quic/core/quic_versions.cc
@@ -664,6 +664,8 @@
// Enable necessary flags.
SetQuicRestartFlag(quic_enable_zero_rtt_for_tls_v2, true);
SetQuicReloadableFlag(quic_key_update_supported, true);
+ SetQuicReloadableFlag(quic_send_version_negotiation_for_short_connection_ids,
+ true);
}
void QuicEnableVersion(const ParsedQuicVersion& version) {
diff --git a/quic/test_tools/quic_test_utils.cc b/quic/test_tools/quic_test_utils.cc
index b286764..0357ee2 100644
--- a/quic/test_tools/quic_test_utils.cc
+++ b/quic/test_tools/quic_test_utils.cc
@@ -1572,11 +1572,6 @@
QUIC_BUG << "Invalid destination_connection_id_length_out";
return false;
}
- if (*destination_connection_id_length_out <
- kQuicMinimumInitialConnectionIdLength) {
- QUIC_BUG << "Invalid *destination_connection_id_length_out";
- return false;
- }
QuicEncryptedPacket encrypted_packet(packet_bytes, packet_length);
PacketHeaderFormat format;
@@ -1601,12 +1596,6 @@
QUIC_BUG << "Packet is not a long header";
return false;
}
- if (destination_connection_id.length() <
- kQuicMinimumInitialConnectionIdLength) {
- QUIC_BUG << "Invalid destination_connection_id length "
- << static_cast<int>(destination_connection_id.length());
- return false;
- }
if (*destination_connection_id_length_out <
destination_connection_id.length()) {
QUIC_BUG << "destination_connection_id_length_out too small";
