Deprecate gfe2_restart_flag_quic_fix_stateless_reset2. PiperOrigin-RevId: 394339885
diff --git a/quic/core/quic_dispatcher.cc b/quic/core/quic_dispatcher.cc index 32c508b..89904c5 100644 --- a/quic/core/quic_dispatcher.cc +++ b/quic/core/quic_dispatcher.cc
@@ -1366,8 +1366,7 @@ void QuicDispatcher::MaybeResetPacketsWithNoVersion( const ReceivedPacketInfo& packet_info) { QUICHE_DCHECK(!packet_info.version_flag); - if (GetQuicRestartFlag(quic_fix_stateless_reset2) && - packet_info.form != GOOGLE_QUIC_PACKET) { + if (packet_info.form != GOOGLE_QUIC_PACKET) { // Drop IETF packets smaller than the minimal stateless reset length. if (packet_info.packet.length() <= QuicFramer::GetMinStatelessResetPacketLength()) {
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h index c195c62..8054ebf 100644 --- a/quic/core/quic_flags_list.h +++ b/quic/core/quic_flags_list.h
@@ -45,8 +45,6 @@ QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_default_on_pto, false) // If true, default-enable 5RTO blachole detection. QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_default_enable_5rto_blackhole_detection2, true) -// If true, determine stateless reset packet length based on the received packet length. -QUIC_FLAG(FLAGS_quic_restart_flag_quic_fix_stateless_reset2, true) // If true, disable QUIC version Q043. QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_disable_version_q043, false) // If true, disable QUIC version Q046.
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc index 125e128..e0e601a 100644 --- a/quic/core/quic_framer.cc +++ b/quic/core/quic_framer.cc
@@ -1300,81 +1300,45 @@ // static std::unique_ptr<QuicEncryptedPacket> QuicFramer::BuildIetfStatelessResetPacket( - QuicConnectionId /*connection_id*/, - size_t received_packet_length, + QuicConnectionId /*connection_id*/, size_t received_packet_length, StatelessResetToken stateless_reset_token) { QUIC_DVLOG(1) << "Building IETF stateless reset packet."; - if (GetQuicRestartFlag(quic_fix_stateless_reset2)) { - if (received_packet_length <= GetMinStatelessResetPacketLength()) { - QUICHE_DLOG(ERROR) - << "Tried to build stateless reset packet with received packet " - "length " - << received_packet_length; - return nullptr; - } - // To ensure stateless reset is indistinguishable from a valid packet, - // include the max connection ID length. - size_t len = std::min(received_packet_length - 1, - GetMinStatelessResetPacketLength() + 1 + - kQuicMaxConnectionIdWithLengthPrefixLength); - std::unique_ptr<char[]> buffer(new char[len]); - QuicDataWriter writer(len, buffer.get()); - // Append random bytes. This randomness only exists to prevent middleboxes - // from comparing the entire packet to a known value. Therefore it has no - // cryptographic use, and does not need a secure cryptographic pseudo-random - // number generator. It's therefore safe to use WriteInsecureRandomBytes. - if (!writer.WriteInsecureRandomBytes(QuicRandom::GetInstance(), - len - kStatelessResetTokenLength)) { - QUIC_BUG(362045737_2) << "Failed to append random bytes of length: " - << len - kStatelessResetTokenLength; - return nullptr; - } - // Change first 2 fixed bits to 01. - buffer[0] &= ~FLAGS_LONG_HEADER; - buffer[0] |= FLAGS_FIXED_BIT; - - // Append stateless reset token. - if (!writer.WriteBytes(&stateless_reset_token, - sizeof(stateless_reset_token))) { - QUIC_BUG(362045737_3) << "Failed to write stateless reset token"; - return nullptr; - } - QUIC_RESTART_FLAG_COUNT(quic_fix_stateless_reset2); - return std::make_unique<QuicEncryptedPacket>(buffer.release(), len, - /*owns_buffer=*/true); - } - - size_t len = kPacketHeaderTypeSize + kMinRandomBytesLengthInStatelessReset + - sizeof(stateless_reset_token); - std::unique_ptr<char[]> buffer(new char[len]); - QuicDataWriter writer(len, buffer.get()); - - uint8_t type = 0; - type |= FLAGS_FIXED_BIT; - type |= FLAGS_SHORT_HEADER_RESERVED_1; - type |= FLAGS_SHORT_HEADER_RESERVED_2; - type |= PacketNumberLengthToOnWireValue(PACKET_1BYTE_PACKET_NUMBER); - - // Append type byte. - if (!writer.WriteUInt8(type)) { + if (received_packet_length <= GetMinStatelessResetPacketLength()) { + QUICHE_DLOG(ERROR) + << "Tried to build stateless reset packet with received packet " + "length " + << received_packet_length; return nullptr; } - + // To ensure stateless reset is indistinguishable from a valid packet, + // include the max connection ID length. + size_t len = std::min(received_packet_length - 1, + GetMinStatelessResetPacketLength() + 1 + + kQuicMaxConnectionIdWithLengthPrefixLength); + std::unique_ptr<char[]> buffer(new char[len]); + QuicDataWriter writer(len, buffer.get()); // Append random bytes. This randomness only exists to prevent middleboxes // from comparing the entire packet to a known value. Therefore it has no // cryptographic use, and does not need a secure cryptographic pseudo-random // number generator. It's therefore safe to use WriteInsecureRandomBytes. if (!writer.WriteInsecureRandomBytes(QuicRandom::GetInstance(), - kMinRandomBytesLengthInStatelessReset)) { + len - kStatelessResetTokenLength)) { + QUIC_BUG(362045737_2) << "Failed to append random bytes of length: " + << len - kStatelessResetTokenLength; return nullptr; } + // Change first 2 fixed bits to 01. + buffer[0] &= ~FLAGS_LONG_HEADER; + buffer[0] |= FLAGS_FIXED_BIT; // Append stateless reset token. if (!writer.WriteBytes(&stateless_reset_token, sizeof(stateless_reset_token))) { + QUIC_BUG(362045737_3) << "Failed to write stateless reset token"; return nullptr; } - return std::make_unique<QuicEncryptedPacket>(buffer.release(), len, true); + return std::make_unique<QuicEncryptedPacket>(buffer.release(), len, + /*owns_buffer=*/true); } // static
diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc index 680cbe5..eae7e0e 100644 --- a/quic/core/quic_framer_test.cc +++ b/quic/core/quic_framer_test.cc
@@ -9260,8 +9260,7 @@ } TEST_P(QuicFramerTest, BuildIetfStatelessResetPacket) { - if (GetQuicRestartFlag(quic_fix_stateless_reset2)) { - // clang-format off + // clang-format off unsigned char packet[] = { // 1st byte 01XX XXXX 0x40, @@ -9271,76 +9270,45 @@ 0x50, 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57, 0x58, 0x59, 0x5a, 0x5b, 0x5c, 0x5d, 0x5e, 0x5f }; - // clang-format on - - // Build the minimal stateless reset packet. - std::unique_ptr<QuicEncryptedPacket> data( - framer_.BuildIetfStatelessResetPacket( - FramerTestConnectionId(), - QuicFramer::GetMinStatelessResetPacketLength() + 1, - kTestStatelessResetToken)); - ASSERT_TRUE(data); - EXPECT_EQ(QuicFramer::GetMinStatelessResetPacketLength(), data->length()); - // Verify the first 2 bits are 01. - EXPECT_FALSE(data->data()[0] & FLAGS_LONG_HEADER); - EXPECT_TRUE(data->data()[0] & FLAGS_FIXED_BIT); - // Verify stateless reset token. - quiche::test::CompareCharArraysWithHexError( - "constructed packet", - data->data() + data->length() - kStatelessResetTokenLength, - kStatelessResetTokenLength, - AsChars(packet) + ABSL_ARRAYSIZE(packet) - kStatelessResetTokenLength, - kStatelessResetTokenLength); - - // Packets with length <= minimal stateless reset does not trigger stateless - // reset. - std::unique_ptr<QuicEncryptedPacket> data2( - framer_.BuildIetfStatelessResetPacket( - FramerTestConnectionId(), - QuicFramer::GetMinStatelessResetPacketLength(), - kTestStatelessResetToken)); - ASSERT_FALSE(data2); - - // Do not send stateless reset >= minimal stateless reset + 1 + max - // connection ID length. - std::unique_ptr<QuicEncryptedPacket> data3( - framer_.BuildIetfStatelessResetPacket(FramerTestConnectionId(), 1000, - kTestStatelessResetToken)); - ASSERT_TRUE(data3); - EXPECT_EQ(QuicFramer::GetMinStatelessResetPacketLength() + 1 + - kQuicMaxConnectionIdWithLengthPrefixLength, - data3->length()); - return; - } - // clang-format off - unsigned char packet[] = { - // type (short header, 1 byte packet number) - 0x70, - // random packet number - 0xFE, - // stateless reset token - 0x50, 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57, - 0x58, 0x59, 0x5a, 0x5b, 0x5c, 0x5d, 0x5e, 0x5f, - }; // clang-format on + // Build the minimal stateless reset packet. std::unique_ptr<QuicEncryptedPacket> data( - framer_.BuildIetfStatelessResetPacket(FramerTestConnectionId(), 0, - kTestStatelessResetToken)); - ASSERT_TRUE(data != nullptr); - // Skip packet number byte which is random in stateless reset packet. - quiche::test::CompareCharArraysWithHexError( - "constructed packet", data->data(), 1, AsChars(packet), 1); - const size_t random_bytes_length = - data->length() - kPacketHeaderTypeSize - kStatelessResetTokenLength; - EXPECT_EQ(kMinRandomBytesLengthInStatelessReset, random_bytes_length); - // Verify stateless reset token is correct. + framer_.BuildIetfStatelessResetPacket( + FramerTestConnectionId(), + QuicFramer::GetMinStatelessResetPacketLength() + 1, + kTestStatelessResetToken)); + ASSERT_TRUE(data); + EXPECT_EQ(QuicFramer::GetMinStatelessResetPacketLength(), data->length()); + // Verify the first 2 bits are 01. + EXPECT_FALSE(data->data()[0] & FLAGS_LONG_HEADER); + EXPECT_TRUE(data->data()[0] & FLAGS_FIXED_BIT); + // Verify stateless reset token. quiche::test::CompareCharArraysWithHexError( "constructed packet", data->data() + data->length() - kStatelessResetTokenLength, kStatelessResetTokenLength, AsChars(packet) + ABSL_ARRAYSIZE(packet) - kStatelessResetTokenLength, kStatelessResetTokenLength); + + // Packets with length <= minimal stateless reset does not trigger stateless + // reset. + std::unique_ptr<QuicEncryptedPacket> data2( + framer_.BuildIetfStatelessResetPacket( + FramerTestConnectionId(), + QuicFramer::GetMinStatelessResetPacketLength(), + kTestStatelessResetToken)); + ASSERT_FALSE(data2); + + // Do not send stateless reset >= minimal stateless reset + 1 + max + // connection ID length. + std::unique_ptr<QuicEncryptedPacket> data3( + framer_.BuildIetfStatelessResetPacket(FramerTestConnectionId(), 1000, + kTestStatelessResetToken)); + ASSERT_TRUE(data3); + EXPECT_EQ(QuicFramer::GetMinStatelessResetPacketLength() + 1 + + kQuicMaxConnectionIdWithLengthPrefixLength, + data3->length()); } TEST_P(QuicFramerTest, EncryptPacket) {
diff --git a/quic/core/quic_time_wait_list_manager.cc b/quic/core/quic_time_wait_list_manager.cc index 80986a3..ef64125 100644 --- a/quic/core/quic_time_wait_list_manager.cc +++ b/quic/core/quic_time_wait_list_manager.cc
@@ -324,8 +324,7 @@ if (ietf_quic) { std::unique_ptr<QuicEncryptedPacket> ietf_reset_packet = BuildIetfStatelessResetPacket(connection_id, received_packet_length); - if (GetQuicRestartFlag(quic_fix_stateless_reset2) && - ietf_reset_packet == nullptr) { + if (ietf_reset_packet == nullptr) { // This could happen when trying to reject a short header packet of // a connection which is in the time wait list (and with no termination // packet).