Add source connection ID parsing to QuicFramer::ProcessPacketDispatcher This CL adds source connection ID parsing to ProcessPacketDispatcher when quic_do_not_override_connection_id is true. It also slightly refactors that function to remove the redundant destination_connection_id_length parameter and switches callers to use destination_connection_id.length() instead. gfe-relnote: add source connection ID parsing, protected by disabled flag quic_do_not_override_connection_id PiperOrigin-RevId: 250704987 Change-Id: I03b965250152175778d2bebbb0daf420d2c935a8
diff --git a/quic/core/quic_dispatcher.cc b/quic/core/quic_dispatcher.cc index 5bf072d..0e6a71d 100644 --- a/quic/core/quic_dispatcher.cc +++ b/quic/core/quic_dispatcher.cc
@@ -302,12 +302,11 @@ } QUIC_RESTART_FLAG_COUNT(quic_no_framer_object_in_dispatcher); QuicPacketHeader header; - uint8_t destination_connection_id_length; std::string detailed_error; const QuicErrorCode error = QuicFramer::ProcessPacketDispatcher( packet, expected_server_connection_id_length_, &header.form, &header.version_flag, &last_version_label_, - &destination_connection_id_length, &header.destination_connection_id, + &header.destination_connection_id, &header.source_connection_id, &detailed_error); if (error != QUIC_NO_ERROR) { // Packet has framing error. @@ -316,7 +315,7 @@ return; } header.version = ParseQuicVersionLabel(last_version_label_); - if (destination_connection_id_length != + if (header.destination_connection_id.length() != expected_server_connection_id_length_ && !should_update_expected_server_connection_id_length_ && !QuicUtils::VariableLengthConnectionIdAllowedForVersion( @@ -326,7 +325,8 @@ return; } if (should_update_expected_server_connection_id_length_) { - expected_server_connection_id_length_ = destination_connection_id_length; + expected_server_connection_id_length_ = + header.destination_connection_id.length(); } // TODO(fayang): Instead of passing in QuicPacketHeader, pass format, // version_flag, version and destination_connection_id. Combine
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc index 328e453..6048133 100644 --- a/quic/core/quic_framer.cc +++ b/quic/core/quic_framer.cc
@@ -6056,28 +6056,30 @@ // static QuicErrorCode QuicFramer::ProcessPacketDispatcher( const QuicEncryptedPacket& packet, - uint8_t expected_connection_id_length, + uint8_t expected_destination_connection_id_length, PacketHeaderFormat* format, bool* version_flag, QuicVersionLabel* version_label, - uint8_t* destination_connection_id_length, QuicConnectionId* destination_connection_id, + QuicConnectionId* source_connection_id, std::string* detailed_error) { QuicDataReader reader(packet.data(), packet.length()); + *source_connection_id = EmptyQuicConnectionId(); uint8_t first_byte; if (!reader.ReadBytes(&first_byte, 1)) { *detailed_error = "Unable to read first byte."; return QUIC_INVALID_PACKET_HEADER; } + uint8_t destination_connection_id_length = 0, source_connection_id_length = 0; if (!QuicUtils::IsIetfPacketHeader(first_byte)) { *format = GOOGLE_QUIC_PACKET; *version_flag = (first_byte & PACKET_PUBLIC_FLAGS_VERSION) != 0; - *destination_connection_id_length = + destination_connection_id_length = first_byte & PACKET_PUBLIC_FLAGS_8BYTE_CONNECTION_ID; - if (*destination_connection_id_length == 0 || + if (destination_connection_id_length == 0 || !reader.ReadConnectionId(destination_connection_id, - *destination_connection_id_length)) { + destination_connection_id_length)) { *detailed_error = "Unable to read ConnectionId."; return QUIC_INVALID_PACKET_HEADER; } @@ -6099,26 +6101,34 @@ } // Set should_update_expected_server_connection_id_length to true to bypass // connection ID lengths validation. - uint8_t unused_source_connection_id_length = 0; uint8_t unused_expected_server_connection_id_length = 0; if (!ProcessAndValidateIetfConnectionIdLength( &reader, ParseQuicVersionLabel(*version_label), Perspective::IS_SERVER, /*should_update_expected_server_connection_id_length=*/true, &unused_expected_server_connection_id_length, - destination_connection_id_length, - &unused_source_connection_id_length, detailed_error)) { + &destination_connection_id_length, &source_connection_id_length, + detailed_error)) { return QUIC_INVALID_PACKET_HEADER; } } else { - // For short header packets, expected_connection_id_length is used to - // determine the destination_connection_id_length. - *destination_connection_id_length = expected_connection_id_length; + // For short header packets, expected_destination_connection_id_length + // is used to determine the destination_connection_id_length. + destination_connection_id_length = + expected_destination_connection_id_length; + DCHECK_EQ(0, source_connection_id_length); } // Read destination connection ID. if (!reader.ReadConnectionId(destination_connection_id, - *destination_connection_id_length)) { - *detailed_error = "Unable to read Destination ConnectionId."; + destination_connection_id_length)) { + *detailed_error = "Unable to read destination connection ID."; + return QUIC_INVALID_PACKET_HEADER; + } + // Read source connection ID. + if (GetQuicRestartFlag(quic_do_not_override_connection_id) && + !reader.ReadConnectionId(source_connection_id, + source_connection_id_length)) { + *detailed_error = "Unable to read source connection ID."; return QUIC_INVALID_PACKET_HEADER; } return QUIC_NO_ERROR;
diff --git a/quic/core/quic_framer.h b/quic/core/quic_framer.h index e6eecf1..992f370 100644 --- a/quic/core/quic_framer.h +++ b/quic/core/quic_framer.h
@@ -374,18 +374,18 @@ QuicVariableLengthIntegerLength length_length); // Lightweight parsing of |packet| and populates |format|, |version_flag|, - // |version_label|, |destination_connection_id_length|, - // |destination_connection_id| and |detailed_error|. Please note, - // |expected_connection_id_length| is only used to determine IETF short header - // packet's destination connection ID length. + // |version_label|, |destination_connection_id|, |source_connection_id| and + // |detailed_error|. Please note, |expected_destination_connection_id_length| + // is only used to determine IETF short header packet's destination + // connection ID length. static QuicErrorCode ProcessPacketDispatcher( const QuicEncryptedPacket& packet, - uint8_t expected_connection_id_length, + uint8_t expected_destination_connection_id_length, PacketHeaderFormat* format, bool* version_flag, QuicVersionLabel* version_label, - uint8_t* destination_connection_id_length, QuicConnectionId* destination_connection_id, + QuicConnectionId* source_connection_id, std::string* detailed_error); // Serializes a packet containing |frames| into |buffer|.
diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc index 048cf95..5cc8470 100644 --- a/quic/core/quic_framer_test.cc +++ b/quic/core/quic_framer_test.cc
@@ -909,19 +909,19 @@ PacketHeaderFormat format; bool version_flag; - uint8_t destination_connection_id_length; - QuicConnectionId destination_connection_id; + QuicConnectionId destination_connection_id, source_connection_id; QuicVersionLabel version_label; std::string detailed_error; - EXPECT_EQ(QUIC_NO_ERROR, QuicFramer::ProcessPacketDispatcher( - *encrypted, kQuicDefaultConnectionIdLength, - &format, &version_flag, &version_label, - &destination_connection_id_length, - &destination_connection_id, &detailed_error)); + EXPECT_EQ(QUIC_NO_ERROR, + QuicFramer::ProcessPacketDispatcher( + *encrypted, kQuicDefaultConnectionIdLength, &format, + &version_flag, &version_label, &destination_connection_id, + &source_connection_id, &detailed_error)); EXPECT_EQ(GOOGLE_QUIC_PACKET, format); EXPECT_FALSE(version_flag); - EXPECT_EQ(kQuicDefaultConnectionIdLength, destination_connection_id_length); + EXPECT_EQ(kQuicDefaultConnectionIdLength, destination_connection_id.length()); EXPECT_EQ(FramerTestConnectionId(), destination_connection_id); + EXPECT_EQ(EmptyQuicConnectionId(), source_connection_id); } TEST_P(QuicFramerTest, LongPacketHeader) { @@ -987,19 +987,68 @@ PacketHeaderFormat format; bool version_flag; - uint8_t destination_connection_id_length; - QuicConnectionId destination_connection_id; + QuicConnectionId destination_connection_id, source_connection_id; QuicVersionLabel version_label; std::string detailed_error; - EXPECT_EQ(QUIC_NO_ERROR, QuicFramer::ProcessPacketDispatcher( - *encrypted, kQuicDefaultConnectionIdLength, - &format, &version_flag, &version_label, - &destination_connection_id_length, - &destination_connection_id, &detailed_error)); + EXPECT_EQ(QUIC_NO_ERROR, + QuicFramer::ProcessPacketDispatcher( + *encrypted, kQuicDefaultConnectionIdLength, &format, + &version_flag, &version_label, &destination_connection_id, + &source_connection_id, &detailed_error)); EXPECT_EQ(IETF_QUIC_LONG_HEADER_PACKET, format); EXPECT_TRUE(version_flag); - EXPECT_EQ(kQuicDefaultConnectionIdLength, destination_connection_id_length); + EXPECT_EQ(kQuicDefaultConnectionIdLength, destination_connection_id.length()); EXPECT_EQ(FramerTestConnectionId(), destination_connection_id); + EXPECT_EQ(EmptyQuicConnectionId(), source_connection_id); +} + +TEST_P(QuicFramerTest, LongPacketHeaderWithBothConnectionIds) { + if (framer_.transport_version() <= QUIC_VERSION_43) { + // This test requires an IETF long header. + return; + } + SetQuicRestartFlag(quic_do_not_override_connection_id, true); + SetDecrypterLevel(ENCRYPTION_ZERO_RTT); + const unsigned char type_byte = + framer_.transport_version() == QUIC_VERSION_44 ? 0xFC : 0xD3; + // clang-format off + unsigned char packet[] = { + // public flags (long header with packet type ZERO_RTT_PROTECTED and + // 4-byte packet number) + type_byte, + // version + QUIC_VERSION_BYTES, + // connection ID lengths + 0x55, + // destination connection ID + 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10, + // source connection ID + 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x11, + // long header packet length + 0x05, + // packet number + 0x12, 0x34, 0x56, 0x00, + // padding frame + 0x00, + }; + // clang-format on + + QuicEncryptedPacket encrypted(AsChars(packet), QUIC_ARRAYSIZE(packet), false); + PacketHeaderFormat format = GOOGLE_QUIC_PACKET; + bool version_flag = false; + QuicConnectionId destination_connection_id, source_connection_id; + QuicVersionLabel version_label = 0; + std::string detailed_error = ""; + EXPECT_EQ(QUIC_NO_ERROR, + QuicFramer::ProcessPacketDispatcher( + encrypted, kQuicDefaultConnectionIdLength, &format, + &version_flag, &version_label, &destination_connection_id, + &source_connection_id, &detailed_error)); + EXPECT_EQ("", detailed_error); + EXPECT_EQ(IETF_QUIC_LONG_HEADER_PACKET, format); + EXPECT_TRUE(version_flag); + EXPECT_EQ(FramerTestConnectionId(), destination_connection_id); + EXPECT_EQ(FramerTestConnectionIdPlusOne(), source_connection_id); } TEST_P(QuicFramerTest, PacketHeaderWith0ByteConnectionId) { @@ -13409,8 +13458,7 @@ PacketHeaderFormat format; bool version_flag; - uint8_t destination_connection_id_length; - QuicConnectionId destination_connection_id; + QuicConnectionId destination_connection_id, source_connection_id; QuicVersionLabel version_label; std::string detailed_error; EXPECT_EQ(QUIC_NO_ERROR, @@ -13418,22 +13466,24 @@ QuicEncryptedPacket(AsChars(long_header_packet), QUIC_ARRAYSIZE(long_header_packet)), kQuicDefaultConnectionIdLength, &format, &version_flag, - &version_label, &destination_connection_id_length, - &destination_connection_id, &detailed_error)); + &version_label, &destination_connection_id, + &source_connection_id, &detailed_error)); EXPECT_EQ(IETF_QUIC_LONG_HEADER_PACKET, format); EXPECT_TRUE(version_flag); - EXPECT_EQ(9, destination_connection_id_length); + EXPECT_EQ(9, destination_connection_id.length()); EXPECT_EQ(FramerTestConnectionIdNineBytes(), destination_connection_id); + EXPECT_EQ(EmptyQuicConnectionId(), source_connection_id); - EXPECT_EQ(QUIC_NO_ERROR, - QuicFramer::ProcessPacketDispatcher( - short_header_encrypted, 9, &format, &version_flag, - &version_label, &destination_connection_id_length, - &destination_connection_id, &detailed_error)); + EXPECT_EQ( + QUIC_NO_ERROR, + QuicFramer::ProcessPacketDispatcher( + short_header_encrypted, 9, &format, &version_flag, &version_label, + &destination_connection_id, &source_connection_id, &detailed_error)); EXPECT_EQ(IETF_QUIC_SHORT_HEADER_PACKET, format); EXPECT_FALSE(version_flag); - EXPECT_EQ(9, destination_connection_id_length); + EXPECT_EQ(9, destination_connection_id.length()); EXPECT_EQ(FramerTestConnectionIdNineBytes(), destination_connection_id); + EXPECT_EQ(EmptyQuicConnectionId(), source_connection_id); } TEST_P(QuicFramerTest, MultiplePacketNumberSpaces) {