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 =