Send correct version negotiation to prober QuicDispatcher::MaybeDispatchPacket was incorrectly computing the value of use_length_prefix instead of using the one from packet_info. This CL also adds regression tests to prevent this issue from coming back. gfe-relnote: change length-prefix of version negotiation, protected by gfe2_reloadable_flag_quic_use_length_prefix_from_packet_info PiperOrigin-RevId: 263219043 Change-Id: I14fc1dbcfffb1dd0cd79c03c90cc982cdab96d9c
diff --git a/quic/core/quic_dispatcher.cc b/quic/core/quic_dispatcher.cc index c944862..4c73dfe 100644 --- a/quic/core/quic_dispatcher.cc +++ b/quic/core/quic_dispatcher.cc
@@ -431,12 +431,20 @@ // packet and stop processing the current packet. QuicConnectionId client_connection_id = packet_info.source_connection_id; + bool use_length_prefix = packet_info.use_length_prefix; + if (!GetQuicReloadableFlag(quic_use_length_prefix_from_packet_info)) { + use_length_prefix = packet_info.form != GOOGLE_QUIC_PACKET && + !QuicVersionLabelUses4BitConnectionIdLength( + packet_info.version_label); + } else { + QUIC_RELOADABLE_FLAG_COUNT(quic_use_length_prefix_from_packet_info); + // TODO(dschinazi) remove the client-side workaround in + // QuicFramer::ParseServerVersionNegotiationProbeResponse + // when quic_use_length_prefix_from_packet_info is deprecated. + } time_wait_list_manager()->SendVersionNegotiationPacket( server_connection_id, client_connection_id, - packet_info.form != GOOGLE_QUIC_PACKET, - packet_info.form != GOOGLE_QUIC_PACKET && - !QuicVersionLabelUses4BitConnectionIdLength( - packet_info.version_label), + packet_info.form != GOOGLE_QUIC_PACKET, use_length_prefix, GetSupportedVersions(), packet_info.self_address, packet_info.peer_address, GetPerPacketContext()); }
diff --git a/quic/core/quic_dispatcher_test.cc b/quic/core/quic_dispatcher_test.cc index 3d60a7a..7b09617 100644 --- a/quic/core/quic_dispatcher_test.cc +++ b/quic/core/quic_dispatcher_test.cc
@@ -1003,6 +1003,67 @@ dispatcher_->ProcessPacket(server_address_, client_address, packet); } +TEST_F(QuicDispatcherTest, VersionNegotiationProbeOld) { + SetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids, false); + SetQuicReloadableFlag(quic_use_length_prefix_from_packet_info, true); + QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1); + CreateTimeWaitListManager(); + char packet[1200]; + char destination_connection_id_bytes[] = {0x56, 0x4e, 0x20, 0x70, + 0x6c, 0x7a, 0x20, 0x21}; + EXPECT_TRUE(QuicFramer::WriteClientVersionNegotiationProbePacket( + packet, sizeof(packet), destination_connection_id_bytes, + sizeof(destination_connection_id_bytes))); + QuicEncryptedPacket encrypted(packet, sizeof(packet), false); + std::unique_ptr<QuicReceivedPacket> received_packet( + ConstructReceivedPacket(encrypted, mock_helper_.GetClock()->Now())); + QuicConnectionId client_connection_id = EmptyQuicConnectionId(); + QuicConnectionId server_connection_id( + destination_connection_id_bytes, sizeof(destination_connection_id_bytes)); + bool ietf_quic = true; + bool use_length_prefix = + GetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids); + EXPECT_CALL( + *time_wait_list_manager_, + SendVersionNegotiationPacket(server_connection_id, client_connection_id, + ietf_quic, use_length_prefix, _, _, _, _)) + .Times(1); + EXPECT_CALL(*dispatcher_, CreateQuicSession(_, _, _, _)).Times(0); + + dispatcher_->ProcessPacket(server_address_, client_address, *received_packet); +} + +TEST_F(QuicDispatcherTest, VersionNegotiationProbe) { + SetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids, true); + SetQuicReloadableFlag(quic_use_parse_public_header, true); + SetQuicReloadableFlag(quic_use_length_prefix_from_packet_info, true); + QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1); + CreateTimeWaitListManager(); + char packet[1200]; + char destination_connection_id_bytes[] = {0x56, 0x4e, 0x20, 0x70, + 0x6c, 0x7a, 0x20, 0x21}; + EXPECT_TRUE(QuicFramer::WriteClientVersionNegotiationProbePacket( + packet, sizeof(packet), destination_connection_id_bytes, + sizeof(destination_connection_id_bytes))); + QuicEncryptedPacket encrypted(packet, sizeof(packet), false); + std::unique_ptr<QuicReceivedPacket> received_packet( + ConstructReceivedPacket(encrypted, mock_helper_.GetClock()->Now())); + QuicConnectionId client_connection_id = EmptyQuicConnectionId(); + QuicConnectionId server_connection_id( + destination_connection_id_bytes, sizeof(destination_connection_id_bytes)); + bool ietf_quic = true; + bool use_length_prefix = + GetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids); + EXPECT_CALL( + *time_wait_list_manager_, + SendVersionNegotiationPacket(server_connection_id, client_connection_id, + ietf_quic, use_length_prefix, _, _, _, _)) + .Times(1); + EXPECT_CALL(*dispatcher_, CreateQuicSession(_, _, _, _)).Times(0); + + dispatcher_->ProcessPacket(server_address_, client_address, *received_packet); +} + // Verify the stopgap test: Packets with truncated connection IDs should be // dropped. class QuicDispatcherTestStrayPacketConnectionId : public QuicDispatcherTest {};
diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc index 2965e45..31b5ca5 100644 --- a/quic/core/quic_framer_test.cc +++ b/quic/core/quic_framer_test.cc
@@ -14396,6 +14396,33 @@ sizeof(destination_connection_id_bytes)); EXPECT_EQ(probe_payload_connection_id, visitor_.header_.get()->destination_connection_id); + + PacketHeaderFormat format = GOOGLE_QUIC_PACKET; + bool version_present = false, has_length_prefix = false; + QuicVersionLabel version_label = 0; + ParsedQuicVersion parsed_version = QuicVersionReservedForNegotiation(); + QuicConnectionId destination_connection_id = TestConnectionId(0x33); + QuicConnectionId source_connection_id = TestConnectionId(0x34); + bool retry_token_present = true; + QuicStringPiece retry_token; + std::string detailed_error = "foobar"; + + QuicErrorCode parse_result = QuicFramer::ParsePublicHeaderDispatcher( + encrypted, kQuicDefaultConnectionIdLength, &format, &version_present, + &has_length_prefix, &version_label, &parsed_version, + &destination_connection_id, &source_connection_id, &retry_token_present, + &retry_token, &detailed_error); + EXPECT_EQ(QUIC_NO_ERROR, parse_result); + EXPECT_EQ(IETF_QUIC_LONG_HEADER_PACKET, format); + EXPECT_TRUE(version_present); + EXPECT_FALSE(has_length_prefix); + EXPECT_EQ(0xcabadaba, version_label); + EXPECT_EQ(QUIC_VERSION_UNSUPPORTED, parsed_version.transport_version); + EXPECT_EQ(probe_payload_connection_id, destination_connection_id); + EXPECT_EQ(EmptyQuicConnectionId(), source_connection_id); + EXPECT_FALSE(retry_token_present); + EXPECT_EQ(QuicStringPiece(), retry_token); + EXPECT_EQ("", detailed_error); } TEST_P(QuicFramerTest, WriteClientVersionNegotiationProbePacket) {