gfe-relnote: In QUIC, only do minimum validation of coalesced packets (only validate connection IDs). Protected by gfe2_reloadable_flag_quic_minimum_validation_of_coalesced_packets.
Also do not consider unprocessable coalesced packet as QUIC_PEER_BUG, because garbage content could be padding outside QUIC packet..
PiperOrigin-RevId: 297414444
Change-Id: Ifc6359e62a561746012fc718aaf24a821a7cfedf
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index 280e464..6e7e589 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -1580,23 +1580,39 @@
QuicPacketHeader coalesced_header;
if (!ProcessIetfPacketHeader(&coalesced_reader, &coalesced_header)) {
- QUIC_PEER_BUG << ENDPOINT
- << "Failed to parse received coalesced header of length "
- << coalesced_data_length << " with error: " << detailed_error_
- << ": "
- << quiche::QuicheTextUtils::HexEncode(coalesced_data,
- coalesced_data_length)
- << " previous header was " << header;
+ // Some implementations pad their INITIAL packets by sending random invalid
+ // data after the INITIAL, and that is allowed by the specification. If we
+ // fail to parse a subsequent coalesced packet, simply ignore it.
+ QUIC_DLOG(INFO) << ENDPOINT
+ << "Failed to parse received coalesced header of length "
+ << coalesced_data_length
+ << " with error: " << detailed_error_ << ": "
+ << quiche::QuicheTextUtils::HexEncode(coalesced_data,
+ coalesced_data_length)
+ << " previous header was " << header;
return;
}
- if (coalesced_header.destination_connection_id !=
- header.destination_connection_id ||
- (coalesced_header.form != IETF_QUIC_SHORT_HEADER_PACKET &&
- coalesced_header.version != header.version)) {
- QUIC_PEER_BUG << ENDPOINT << "Received mismatched coalesced header "
- << coalesced_header << " previous header was " << header;
- return;
+ if (GetQuicReloadableFlag(quic_minimum_validation_of_coalesced_packets)) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_minimum_validation_of_coalesced_packets);
+ if (coalesced_header.destination_connection_id !=
+ header.destination_connection_id) {
+ // Drop coalesced packets with mismatched connection IDs.
+ QUIC_DLOG(INFO) << ENDPOINT << "Received mismatched coalesced header "
+ << coalesced_header << " previous header was " << header;
+ QUIC_CODE_COUNT(
+ quic_received_coalesced_packets_with_mismatched_connection_id);
+ return;
+ }
+ } else {
+ if (coalesced_header.destination_connection_id !=
+ header.destination_connection_id ||
+ (coalesced_header.form != IETF_QUIC_SHORT_HEADER_PACKET &&
+ coalesced_header.version != header.version)) {
+ QUIC_PEER_BUG << ENDPOINT << "Received mismatched coalesced header "
+ << coalesced_header << " previous header was " << header;
+ return;
+ }
}
QuicEncryptedPacket coalesced_packet(coalesced_data, coalesced_data_length,
diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc
index beac097..ec45837 100644
--- a/quic/core/quic_framer_test.cc
+++ b/quic/core/quic_framer_test.cc
@@ -12282,6 +12282,272 @@
CheckStreamFrameData("HELLO_WORLD?", visitor_.stream_frames_[1].get());
}
+TEST_P(QuicFramerTest, CoalescedPacketWithUdpPadding) {
+ if (!framer_.version().HasLongHeaderLengths()) {
+ return;
+ }
+ SetDecrypterLevel(ENCRYPTION_ZERO_RTT);
+ // clang-format off
+ unsigned char packet[] = {
+ // first coalesced packet
+ // public flags (long header with packet type ZERO_RTT_PROTECTED and
+ // 4-byte packet number)
+ 0xD3,
+ // version
+ QUIC_VERSION_BYTES,
+ // destination connection ID length
+ 0x08,
+ // destination connection ID
+ 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10,
+ // source connection ID length
+ 0x00,
+ // long header packet length
+ 0x1E,
+ // packet number
+ 0x12, 0x34, 0x56, 0x78,
+ // frame type (stream frame with fin)
+ 0xFE,
+ // stream id
+ 0x02, 0x03, 0x04,
+ // offset
+ 0x3A, 0x98, 0xFE, 0xDC, 0x32, 0x10, 0x76, 0x54,
+ // data length
+ 0x00, 0x0c,
+ // data
+ 'h', 'e', 'l', 'l',
+ 'o', ' ', 'w', 'o',
+ 'r', 'l', 'd', '!',
+ // padding
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ };
+ unsigned char packet99[] = {
+ // first coalesced packet
+ // public flags (long header with packet type ZERO_RTT_PROTECTED and
+ // 4-byte packet number)
+ 0xD3,
+ // version
+ QUIC_VERSION_BYTES,
+ // destination connection ID length
+ 0x08,
+ // destination connection ID
+ 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10,
+ // source connection ID length
+ 0x00,
+ // long header packet length
+ 0x1E,
+ // packet number
+ 0x12, 0x34, 0x56, 0x78,
+ // frame type (IETF_STREAM frame with FIN, LEN, and OFFSET bits set)
+ 0x08 | 0x01 | 0x02 | 0x04,
+ // stream id
+ kVarInt62FourBytes + 0x00, 0x02, 0x03, 0x04,
+ // offset
+ kVarInt62EightBytes + 0x3A, 0x98, 0xFE, 0xDC,
+ 0x32, 0x10, 0x76, 0x54,
+ // data length
+ kVarInt62OneByte + 0x0c,
+ // data
+ 'h', 'e', 'l', 'l',
+ 'o', ' ', 'w', 'o',
+ 'r', 'l', 'd', '!',
+ // padding
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00,
+ };
+ // clang-format on
+
+ unsigned char* p = packet;
+ size_t p_length = QUICHE_ARRAYSIZE(packet);
+ if (framer_.version().HasIetfQuicFrames()) {
+ p = packet99;
+ p_length = QUICHE_ARRAYSIZE(packet99);
+ }
+
+ QuicEncryptedPacket encrypted(AsChars(p), p_length, false);
+ EXPECT_TRUE(framer_.ProcessPacket(encrypted));
+
+ EXPECT_THAT(framer_.error(), IsQuicNoError());
+ ASSERT_TRUE(visitor_.header_.get());
+
+ ASSERT_EQ(1u, visitor_.stream_frames_.size());
+ EXPECT_EQ(0u, visitor_.ack_frames_.size());
+
+ // Stream ID should be the last 3 bytes of kStreamId.
+ EXPECT_EQ(0x00FFFFFF & kStreamId, visitor_.stream_frames_[0]->stream_id);
+ EXPECT_TRUE(visitor_.stream_frames_[0]->fin);
+ EXPECT_EQ(kStreamOffset, visitor_.stream_frames_[0]->offset);
+ CheckStreamFrameData("hello world!", visitor_.stream_frames_[0].get());
+
+ EXPECT_EQ(visitor_.coalesced_packets_.size(), 0u);
+}
+
+TEST_P(QuicFramerTest, CoalescedPacketWithDifferentVersion) {
+ if (!QuicVersionHasLongHeaderLengths(framer_.transport_version())) {
+ return;
+ }
+ SetQuicReloadableFlag(quic_minimum_validation_of_coalesced_packets, true);
+ SetDecrypterLevel(ENCRYPTION_ZERO_RTT);
+ // clang-format off
+ unsigned char packet[] = {
+ // first coalesced packet
+ // public flags (long header with packet type ZERO_RTT_PROTECTED and
+ // 4-byte packet number)
+ 0xD3,
+ // version
+ QUIC_VERSION_BYTES,
+ // destination connection ID length
+ 0x08,
+ // destination connection ID
+ 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10,
+ // source connection ID length
+ 0x00,
+ // long header packet length
+ 0x1E,
+ // packet number
+ 0x12, 0x34, 0x56, 0x78,
+ // frame type (stream frame with fin)
+ 0xFE,
+ // stream id
+ 0x02, 0x03, 0x04,
+ // offset
+ 0x3A, 0x98, 0xFE, 0xDC, 0x32, 0x10, 0x76, 0x54,
+ // data length
+ 0x00, 0x0c,
+ // data
+ 'h', 'e', 'l', 'l',
+ 'o', ' ', 'w', 'o',
+ 'r', 'l', 'd', '!',
+ // second coalesced packet
+ // public flags (long header with packet type ZERO_RTT_PROTECTED and
+ // 4-byte packet number)
+ 0xD3,
+ // garbage version
+ 'G', 'A', 'B', 'G',
+ // destination connection ID length
+ 0x08,
+ // destination connection ID
+ 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10,
+ // source connection ID length
+ 0x00,
+ // long header packet length
+ 0x1E,
+ // packet number
+ 0x12, 0x34, 0x56, 0x79,
+ // frame type (stream frame with fin)
+ 0xFE,
+ // stream id
+ 0x02, 0x03, 0x04,
+ // offset
+ 0x3A, 0x98, 0xFE, 0xDC, 0x32, 0x10, 0x76, 0x54,
+ // data length
+ 0x00, 0x0c,
+ // data
+ 'H', 'E', 'L', 'L',
+ 'O', '_', 'W', 'O',
+ 'R', 'L', 'D', '?',
+ };
+ unsigned char packet99[] = {
+ // first coalesced packet
+ // public flags (long header with packet type ZERO_RTT_PROTECTED and
+ // 4-byte packet number)
+ 0xD3,
+ // version
+ QUIC_VERSION_BYTES,
+ // destination connection ID length
+ 0x08,
+ // destination connection ID
+ 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10,
+ // source connection ID length
+ 0x00,
+ // long header packet length
+ 0x1E,
+ // packet number
+ 0x12, 0x34, 0x56, 0x78,
+ // frame type (IETF_STREAM frame with FIN, LEN, and OFFSET bits set)
+ 0x08 | 0x01 | 0x02 | 0x04,
+ // stream id
+ kVarInt62FourBytes + 0x00, 0x02, 0x03, 0x04,
+ // offset
+ kVarInt62EightBytes + 0x3A, 0x98, 0xFE, 0xDC,
+ 0x32, 0x10, 0x76, 0x54,
+ // data length
+ kVarInt62OneByte + 0x0c,
+ // data
+ 'h', 'e', 'l', 'l',
+ 'o', ' ', 'w', 'o',
+ 'r', 'l', 'd', '!',
+ // second coalesced packet
+ // public flags (long header with packet type ZERO_RTT_PROTECTED and
+ // 4-byte packet number)
+ 0xD3,
+ // garbage version
+ 'G', 'A', 'B', 'G',
+ // destination connection ID length
+ 0x08,
+ // destination connection ID
+ 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10,
+ // source connection ID length
+ 0x00,
+ // long header packet length
+ 0x1E,
+ // packet number
+ 0x12, 0x34, 0x56, 0x79,
+ // frame type (IETF_STREAM frame with FIN, LEN, and OFFSET bits set)
+ 0x08 | 0x01 | 0x02 | 0x04,
+ // stream id
+ kVarInt62FourBytes + 0x00, 0x02, 0x03, 0x04,
+ // offset
+ kVarInt62EightBytes + 0x3A, 0x98, 0xFE, 0xDC,
+ 0x32, 0x10, 0x76, 0x54,
+ // data length
+ kVarInt62OneByte + 0x0c,
+ // data
+ 'H', 'E', 'L', 'L',
+ 'O', '_', 'W', 'O',
+ 'R', 'L', 'D', '?',
+ };
+ // clang-format on
+
+ unsigned char* p = packet;
+ size_t p_length = QUICHE_ARRAYSIZE(packet);
+ if (framer_.version().HasIetfQuicFrames()) {
+ p = packet99;
+ p_length = QUICHE_ARRAYSIZE(packet99);
+ }
+
+ QuicEncryptedPacket encrypted(AsChars(p), p_length, false);
+ EXPECT_TRUE(framer_.ProcessPacket(encrypted));
+
+ EXPECT_THAT(framer_.error(), IsQuicNoError());
+ ASSERT_TRUE(visitor_.header_.get());
+
+ ASSERT_EQ(1u, visitor_.stream_frames_.size());
+ EXPECT_EQ(0u, visitor_.ack_frames_.size());
+
+ // Stream ID should be the last 3 bytes of kStreamId.
+ EXPECT_EQ(0x00FFFFFF & kStreamId, visitor_.stream_frames_[0]->stream_id);
+ EXPECT_TRUE(visitor_.stream_frames_[0]->fin);
+ EXPECT_EQ(kStreamOffset, visitor_.stream_frames_[0]->offset);
+ CheckStreamFrameData("hello world!", visitor_.stream_frames_[0].get());
+
+ ASSERT_EQ(visitor_.coalesced_packets_.size(), 1u);
+ EXPECT_TRUE(framer_.ProcessPacket(*visitor_.coalesced_packets_[0].get()));
+
+ EXPECT_THAT(framer_.error(), IsQuicNoError());
+ ASSERT_TRUE(visitor_.header_.get());
+
+ ASSERT_EQ(1u, visitor_.stream_frames_.size());
+ // Verify version mismatch gets reported.
+ EXPECT_EQ(1, visitor_.version_mismatch_);
+}
+
TEST_P(QuicFramerTest, UndecryptablePacketWithoutDecrypter) {
QuicFramerPeer::SetPerspective(&framer_, Perspective::IS_CLIENT);
@@ -12798,8 +13064,12 @@
QuicEncryptedPacket encrypted(AsChars(p), p_length, false);
- EXPECT_QUIC_PEER_BUG(EXPECT_TRUE(framer_.ProcessPacket(encrypted)),
- "Server: Received mismatched coalesced header.*");
+ if (GetQuicReloadableFlag(quic_minimum_validation_of_coalesced_packets)) {
+ EXPECT_TRUE(framer_.ProcessPacket(encrypted));
+ } else {
+ EXPECT_QUIC_PEER_BUG(EXPECT_TRUE(framer_.ProcessPacket(encrypted)),
+ "Server: Received mismatched coalesced header.*");
+ }
EXPECT_THAT(framer_.error(), IsQuicNoError());
ASSERT_TRUE(visitor_.header_.get());
@@ -12904,8 +13174,7 @@
QuicEncryptedPacket encrypted(AsChars(p), p_length, false);
- EXPECT_QUIC_PEER_BUG(EXPECT_TRUE(framer_.ProcessPacket(encrypted)),
- "Server: Failed to parse received coalesced header.*");
+ EXPECT_TRUE(framer_.ProcessPacket(encrypted));
EXPECT_THAT(framer_.error(), IsQuicNoError());
ASSERT_TRUE(visitor_.header_.get());
@@ -12975,9 +13244,8 @@
false);
// Make sure we discard the subsequent zeroes.
- EXPECT_QUIC_PEER_BUG(EXPECT_TRUE(framer_.ProcessPacket(encrypted)),
- "Server: (Failed to parse received|Received mismatched) "
- "coalesced header.*");
+ EXPECT_TRUE(framer_.ProcessPacket(encrypted));
+ EXPECT_TRUE(visitor_.coalesced_packets_.empty());
}
TEST_P(QuicFramerTest, ClientReceivesInvalidVersion) {
