Reject NEW_CONNECTION_ID frames with empty connection IDs and close the connection, per RFC9000. Based on cr/896368856 Protected by quic_reloadable_flag_quic_reject_empty_cid_in_ncid. PiperOrigin-RevId: 897724648
diff --git a/quiche/common/quiche_feature_flags_list.h b/quiche/common/quiche_feature_flags_list.h index 7a41589..d0aa747 100755 --- a/quiche/common/quiche_feature_flags_list.h +++ b/quiche/common/quiche_feature_flags_list.h
@@ -61,6 +61,7 @@ QUICHE_FLAG(bool, quiche_reloadable_flag_quic_parse_cert_compression_algos_from_chlo, true, true, "If true, parse offered cert compression algorithms from received CHLOs.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_priority_respect_incremental, false, false, "If true, respect the incremental parameter of each stream in QuicWriteBlockedList.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_receive_ack_frequency, false, false, "When true, advertises support for ACK_FREQUENCY and IMMEDIATE_ACK from draft-ietf-quic-ack-frequency-10 and processes them correctly.") +QUICHE_FLAG(bool, quiche_reloadable_flag_quic_reject_empty_cid_in_ncid, false, false, "If true, QUIC framer will reject NEW_CONNECTION_ID frames with empty connection IDs.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_require_handshake_confirmation, true, true, "If true, require handshake confirmation for QUIC connections, functionally disabling 0-rtt handshakes.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_test_peer_addr_change_after_normalize, false, false, "If true, QuicConnection::ProcessValidatedPacket will use normalized address to test peer address changes.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_testonly_default_false, false, false, "A testonly reloadable flag that will always default to false.")
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h index c1f61a9..12fdafc 100644 --- a/quiche/quic/core/quic_connection.h +++ b/quiche/quic/core/quic_connection.h
@@ -810,7 +810,8 @@ std::unique_ptr<QuicEncrypter> CreateCurrentOneRttEncrypter() override; // Whether destination connection ID is required but missing in the packet - // creator. + // creator. This can occur if we're trying to send a PATH_RESPONSE, but the + // peer hasn't provided enough connection IDs. bool IsMissingDestinationConnectionID() const; // QuicPacketCreator::DelegateInterface bool ShouldGeneratePacket(HasRetransmittableData retransmittable,
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc index 9281193..8555867 100644 --- a/quiche/quic/core/quic_connection_test.cc +++ b/quiche/quic/core/quic_connection_test.cc
@@ -16081,6 +16081,7 @@ QuicPathChallengeFrame path_challenge_frame; QuicNewConnectionIdFrame new_connection_id_frame; new_connection_id_frame.sequence_number = 1u; + new_connection_id_frame.connection_id = TestConnectionId(789); QuicRetireConnectionIdFrame retire_connection_id_frame; retire_connection_id_frame.sequence_number = 1u; QuicStopSendingFrame stop_sending_frame;
diff --git a/quiche/quic/core/quic_framer.cc b/quiche/quic/core/quic_framer.cc index d0b9e64..2eaa480 100644 --- a/quiche/quic/core/quic_framer.cc +++ b/quiche/quic/core/quic_framer.cc
@@ -6359,6 +6359,13 @@ return false; } + if (GetQuicReloadableFlag(quic_reject_empty_cid_in_ncid) && + frame->connection_id.IsEmpty()) { + QUIC_RELOADABLE_FLAG_COUNT(quic_reject_empty_cid_in_ncid); + set_detailed_error("Connection IDs in NEW_CONNECTION_ID cannot be empty."); + return false; + } + if (!QuicUtils::IsConnectionIdValidForVersion(frame->connection_id, transport_version())) { set_detailed_error("Invalid new connection ID length for version.");
diff --git a/quiche/quic/core/quic_framer_test.cc b/quiche/quic/core/quic_framer_test.cc index 95b750e..320421b 100644 --- a/quiche/quic/core/quic_framer_test.cc +++ b/quiche/quic/core/quic_framer_test.cc
@@ -37,6 +37,7 @@ #include "quiche/quic/test_tools/quic_framer_peer.h" #include "quiche/quic/test_tools/quic_test_utils.h" #include "quiche/quic/test_tools/simple_data_producer.h" +#include "quiche/common/platform/api/quiche_flags.h" #include "quiche/common/quiche_endian.h" #include "quiche/common/test_tools/quiche_test_utils.h" @@ -10593,6 +10594,48 @@ EXPECT_EQ("Retire_prior_to > sequence_number.", framer_.detailed_error()); } +TEST_P(QuicFramerTest, InvalidEmptyNewConnectionIdFrame) { + if (!VersionIsIetfQuic(framer_.transport_version())) { + // The NEW_CONNECTION_ID frame is only for IETF QUIC. + return; + } + SetQuicheReloadableFlag(quic_reject_empty_cid_in_ncid, true); + SetDecrypterLevel(ENCRYPTION_FORWARD_SECURE); + // clang-format off + PacketFragments packet_ietf = { + // type (short header, 4 byte packet number) + {"", + {0x43}}, + // connection_id + {"", + {0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10}}, + // packet number + {"", + {0x12, 0x34, 0x56, 0x78}}, + // frame type (IETF_NEW_CONNECTION_ID frame) + {"", + {0x18}}, + // error code + {"Unable to read new connection ID frame sequence number.", + {kVarInt62OneByte + 0x11}}, + {"Unable to read new connection ID frame retire_prior_to.", + {kVarInt62OneByte + 0x0a}}, + {"Connection ID with zero length", + {0x00}}, // connection ID length + {"Can not read new connection ID frame reset token.", + {0x50, 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57, + 0x58, 0x59, 0x5a, 0x5b, 0x5c, 0x5d, 0x5e, 0x5f}} + }; + // clang-format on + + std::unique_ptr<QuicEncryptedPacket> encrypted( + AssemblePacketFromFragments(packet_ietf)); + EXPECT_FALSE(framer_.ProcessPacket(*encrypted)); + EXPECT_THAT(framer_.error(), IsError(QUIC_INVALID_NEW_CONNECTION_ID_DATA)); + EXPECT_EQ("Connection IDs in NEW_CONNECTION_ID cannot be empty.", + framer_.detailed_error()); +} + TEST_P(QuicFramerTest, BuildNewConnectionIdFramePacket) { if (!VersionIsIetfQuic(framer_.transport_version())) { // This frame is only for IETF QUIC only.