Allow long connection IDs for unknown QUIC versions
In particular, if we're a server and a client sends us a packet with an unknown version and a connection ID of any length between 8 and 255, we must respond with a version negotiation packet that includes that same connection ID.
gfe-relnote: allow longer connection IDs, protected by new disabled flag gfe2_restart_flag_quic_allow_very_long_connection_ids
PiperOrigin-RevId: 286015388
Change-Id: I53ff4e602a19f30f56aba10ad7d877776597a5cf
diff --git a/quic/core/quic_connection_id.cc b/quic/core/quic_connection_id.cc
index c495b35..c514450 100644
--- a/quic/core/quic_connection_id.cc
+++ b/quic/core/quic_connection_id.cc
@@ -53,13 +53,19 @@
QuicConnectionId::QuicConnectionId() : QuicConnectionId(nullptr, 0) {}
QuicConnectionId::QuicConnectionId(const char* data, uint8_t length) {
- static_assert(kQuicMaxConnectionIdAllVersionsLength <=
- std::numeric_limits<uint8_t>::max(),
- "kQuicMaxConnectionIdAllVersionsLength too high");
- if (length > kQuicMaxConnectionIdAllVersionsLength) {
- QUIC_BUG << "Attempted to create connection ID of length "
- << static_cast<int>(length);
- length = kQuicMaxConnectionIdAllVersionsLength;
+ if (!GetQuicRestartFlag(quic_allow_very_long_connection_ids)) {
+ // TODO(dschinazi) remove kQuicMaxConnectionIdAllVersionsLength entirely
+ // when we deprecate quic_allow_very_long_connection_ids.
+ static_assert(kQuicMaxConnectionIdAllVersionsLength <=
+ std::numeric_limits<uint8_t>::max(),
+ "kQuicMaxConnectionIdAllVersionsLength too high");
+ if (length > kQuicMaxConnectionIdAllVersionsLength) {
+ QUIC_BUG << "Attempted to create connection ID of length "
+ << static_cast<int>(length);
+ length = kQuicMaxConnectionIdAllVersionsLength;
+ }
+ } else {
+ QUIC_RESTART_FLAG_COUNT_N(quic_allow_very_long_connection_ids, 3, 5);
}
length_ = length;
if (length_ == 0) {
@@ -109,10 +115,14 @@
}
void QuicConnectionId::set_length(uint8_t length) {
- if (length > kQuicMaxConnectionIdAllVersionsLength) {
- QUIC_BUG << "Attempted to set connection ID length to "
- << static_cast<int>(length);
- length = kQuicMaxConnectionIdAllVersionsLength;
+ if (!GetQuicRestartFlag(quic_allow_very_long_connection_ids)) {
+ if (length > kQuicMaxConnectionIdAllVersionsLength) {
+ QUIC_BUG << "Attempted to set connection ID length to "
+ << static_cast<int>(length);
+ length = kQuicMaxConnectionIdAllVersionsLength;
+ }
+ } else {
+ QUIC_RESTART_FLAG_COUNT_N(quic_allow_very_long_connection_ids, 4, 5);
}
char temporary_data[sizeof(data_short_)];
if (length > sizeof(data_short_)) {
diff --git a/quic/core/quic_connection_id_test.cc b/quic/core/quic_connection_id_test.cc
index 1703913..dbc8214 100644
--- a/quic/core/quic_connection_id_test.cc
+++ b/quic/core/quic_connection_id_test.cc
@@ -92,7 +92,8 @@
// Verify that any two all-zero connection IDs of different lengths never
// have the same hash.
- const char connection_id_bytes[kQuicMaxConnectionIdAllVersionsLength] = {};
+ SetQuicRestartFlag(quic_allow_very_long_connection_ids, true);
+ const char connection_id_bytes[255] = {};
for (uint8_t i = 0; i < sizeof(connection_id_bytes) - 1; ++i) {
QuicConnectionId connection_id_i(connection_id_bytes, i);
for (uint8_t j = i + 1; j < sizeof(connection_id_bytes); ++j) {
diff --git a/quic/core/quic_data_reader.cc b/quic/core/quic_data_reader.cc
index 306a8a9..3982cc9 100644
--- a/quic/core/quic_data_reader.cc
+++ b/quic/core/quic_data_reader.cc
@@ -139,10 +139,14 @@
bool QuicDataReader::ReadConnectionId(QuicConnectionId* connection_id,
uint8_t length) {
- if (length > kQuicMaxConnectionIdAllVersionsLength) {
- QUIC_BUG << "Attempted to read connection ID with length too high "
- << static_cast<int>(length);
- return false;
+ if (!GetQuicRestartFlag(quic_allow_very_long_connection_ids)) {
+ if (length > kQuicMaxConnectionIdAllVersionsLength) {
+ QUIC_BUG << "Attempted to read connection ID with length too high "
+ << static_cast<int>(length);
+ return false;
+ }
+ } else {
+ QUIC_RESTART_FLAG_COUNT_N(quic_allow_very_long_connection_ids, 1, 5);
}
if (length == 0) {
connection_id->set_length(0);
@@ -166,8 +170,12 @@
if (!ReadUInt8(&connection_id_length)) {
return false;
}
- if (connection_id_length > kQuicMaxConnectionIdAllVersionsLength) {
- return false;
+ if (!GetQuicRestartFlag(quic_allow_very_long_connection_ids)) {
+ if (connection_id_length > kQuicMaxConnectionIdAllVersionsLength) {
+ return false;
+ }
+ } else {
+ QUIC_RESTART_FLAG_COUNT_N(quic_allow_very_long_connection_ids, 2, 5);
}
return ReadConnectionId(connection_id, connection_id_length);
}
diff --git a/quic/core/quic_data_writer_test.cc b/quic/core/quic_data_writer_test.cc
index 7cfde88..3c38673 100644
--- a/quic/core/quic_data_writer_test.cc
+++ b/quic/core/quic_data_writer_test.cc
@@ -1166,6 +1166,9 @@
}
TEST_P(QuicDataWriterTest, InvalidConnectionIdLengthRead) {
+ SetQuicRestartFlag(quic_allow_very_long_connection_ids, false);
+ // TODO(dschinazi) delete this test when we deprecate
+ // quic_allow_very_long_connection_ids.
static const uint8_t bad_connection_id_length = 200;
static_assert(
bad_connection_id_length > kQuicMaxConnectionIdAllVersionsLength,
diff --git a/quic/core/quic_dispatcher_test.cc b/quic/core/quic_dispatcher_test.cc
index 1ea8036..4f797c2 100644
--- a/quic/core/quic_dispatcher_test.cc
+++ b/quic/core/quic_dispatcher_test.cc
@@ -547,6 +547,26 @@
CONNECTION_ID_PRESENT, PACKET_4BYTE_PACKET_NUMBER, 1);
}
+TEST_F(QuicDispatcherTest,
+ StatelessVersionNegotiationWithVeryLongConnectionId) {
+ SetQuicRestartFlag(quic_allow_very_long_connection_ids, true);
+ QuicConnectionId connection_id = QuicUtils::CreateRandomConnectionId(33);
+ CreateTimeWaitListManager();
+ QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1);
+
+ EXPECT_CALL(*dispatcher_, CreateQuicSession(_, _, _, _)).Times(0);
+ EXPECT_CALL(*time_wait_list_manager_,
+ SendVersionNegotiationPacket(connection_id, _, _, _, _, _, _, _))
+ .Times(1);
+ // Pad the CHLO message with enough data to make the packet large enough
+ // to trigger version negotiation.
+ std::string chlo = SerializeCHLO() + std::string(1200, 'a');
+ DCHECK_LE(1200u, chlo.length());
+ ProcessPacket(client_address, connection_id, true,
+ QuicVersionReservedForNegotiation(), chlo, true,
+ CONNECTION_ID_PRESENT, PACKET_4BYTE_PACKET_NUMBER, 1);
+}
+
TEST_F(QuicDispatcherTest, StatelessVersionNegotiationWithClientConnectionId) {
CreateTimeWaitListManager();
QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1);
diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc
index e9c682a..c3f5463 100644
--- a/quic/core/quic_framer_test.cc
+++ b/quic/core/quic_framer_test.cc
@@ -10472,8 +10472,13 @@
AssemblePacketFromFragments(packet99));
EXPECT_FALSE(framer_.ProcessPacket(*encrypted));
EXPECT_THAT(framer_.error(), IsError(QUIC_INVALID_NEW_CONNECTION_ID_DATA));
- EXPECT_EQ("Unable to read new connection ID frame connection id.",
- framer_.detailed_error());
+ if (!GetQuicRestartFlag(quic_allow_very_long_connection_ids)) {
+ EXPECT_EQ("Unable to read new connection ID frame connection id.",
+ framer_.detailed_error());
+ } else {
+ EXPECT_EQ("Invalid new connection ID length for version.",
+ framer_.detailed_error());
+ }
}
// Verifies that parsing a NEW_CONNECTION_ID frame with an invalid
diff --git a/quic/core/quic_utils.cc b/quic/core/quic_utils.cc
index a695c94..e37d019 100644
--- a/quic/core/quic_utils.cc
+++ b/quic/core/quic_utils.cc
@@ -545,6 +545,16 @@
static_cast<size_t>(std::numeric_limits<uint8_t>::max())) {
return false;
}
+
+ if (GetQuicRestartFlag(quic_allow_very_long_connection_ids)) {
+ QUIC_RESTART_FLAG_COUNT_N(quic_allow_very_long_connection_ids, 5, 5);
+ if (transport_version == QUIC_VERSION_UNSUPPORTED ||
+ transport_version == QUIC_VERSION_RESERVED_FOR_NEGOTIATION) {
+ // Unknown versions could allow connection ID lengths up to 255.
+ return true;
+ }
+ }
+
const uint8_t connection_id_length8 =
static_cast<uint8_t>(connection_id_length);
// Versions that do not support variable lengths only support length 8.
diff --git a/quic/core/quic_utils_test.cc b/quic/core/quic_utils_test.cc
index fa471f0..baaa3ae 100644
--- a/quic/core/quic_utils_test.cc
+++ b/quic/core/quic_utils_test.cc
@@ -184,7 +184,8 @@
TEST_F(QuicUtilsTest, ReplacementConnectionIdLengthIsCorrect) {
// Verify that all lengths get replaced by kQuicDefaultConnectionIdLength.
- const char connection_id_bytes[kQuicMaxConnectionIdAllVersionsLength] = {};
+ SetQuicRestartFlag(quic_allow_very_long_connection_ids, true);
+ const char connection_id_bytes[255] = {};
for (uint8_t i = 0; i < sizeof(connection_id_bytes) - 1; ++i) {
QuicConnectionId connection_id(connection_id_bytes, i);
QuicConnectionId replacement_connection_id =
