gfe-relnote: In QUIC, do not add connection ID of packets with unknown connection ID and no version to time wait list, instead, send appropriate reset depending on the packets' sizes and drop them. Protected by gfe2_reloadable_flag_quic_reject_unprocessable_packets_statelessly.
PiperOrigin-RevId: 257470130
Change-Id: I0897dc244bac24f442685eddd795a7152ee5242f
diff --git a/quic/core/quic_dispatcher.cc b/quic/core/quic_dispatcher.cc
index 0789567..f7973e0 100644
--- a/quic/core/quic_dispatcher.cc
+++ b/quic/core/quic_dispatcher.cc
@@ -434,6 +434,8 @@
buffered_packets_.DiscardPackets(server_connection_id);
break;
+ case kFateDrop:
+ break;
}
}
@@ -452,6 +454,11 @@
QUIC_DLOG(INFO)
<< "Packet without version arrived for unknown connection ID "
<< packet_info.destination_connection_id;
+ if (GetQuicReloadableFlag(quic_reject_unprocessable_packets_statelessly)) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_reject_unprocessable_packets_statelessly);
+ MaybeResetPacketsWithNoVersion(packet_info);
+ return kFateDrop;
+ }
return kFateTimeWait;
}
@@ -877,4 +884,24 @@
return false;
}
+void QuicDispatcher::MaybeResetPacketsWithNoVersion(
+ const ReceivedPacketInfo& packet_info) {
+ DCHECK(!packet_info.version_flag);
+ const size_t MinValidPacketLength =
+ kPacketHeaderTypeSize + expected_server_connection_id_length_ +
+ PACKET_1BYTE_PACKET_NUMBER + /*payload size=*/1 + /*tag size=*/12;
+ if (packet_info.packet.length() < MinValidPacketLength) {
+ // The packet size is too small.
+ QUIC_CODE_COUNT(drop_too_small_packets);
+ return;
+ }
+ // TODO(fayang): Consider rate limiting reset packets if reset packet size >
+ // packet_length.
+
+ time_wait_list_manager()->SendPublicReset(
+ packet_info.self_address, packet_info.peer_address,
+ packet_info.destination_connection_id,
+ packet_info.form != GOOGLE_QUIC_PACKET, GetPerPacketContext());
+}
+
} // namespace quic
diff --git a/quic/core/quic_dispatcher.h b/quic/core/quic_dispatcher.h
index 8dd2d99..cfaafa0 100644
--- a/quic/core/quic_dispatcher.h
+++ b/quic/core/quic_dispatcher.h
@@ -153,12 +153,16 @@
virtual bool MaybeDispatchPacket(const ReceivedPacketInfo& packet_info);
// Values to be returned by ValidityChecks() to indicate what should be done
- // with a packet.
+ // with a packet. Fates with greater values are considered to be higher
+ // priority. ValidityChecks should return fate based on the priority order
+ // (i.e., returns higher priority fate first)
enum QuicPacketFate {
// Process the packet normally, which is usually to establish a connection.
kFateProcess,
// Put the connection ID into time-wait state and send a public reset.
kFateTimeWait,
+ // Drop the packet.
+ kFateDrop,
};
// This method is called by ProcessHeader on packets not associated with a
@@ -301,6 +305,10 @@
// Returns true if |version| is a supported protocol version.
bool IsSupportedVersion(const ParsedQuicVersion version);
+ // Sends public/stateless reset packets with no version and unknown
+ // connection ID according to the packet's size.
+ void MaybeResetPacketsWithNoVersion(const ReceivedPacketInfo& packet_info);
+
void set_new_sessions_allowed_per_event_loop(
int16_t new_sessions_allowed_per_event_loop) {
new_sessions_allowed_per_event_loop_ = new_sessions_allowed_per_event_loop;
diff --git a/quic/core/quic_dispatcher_test.cc b/quic/core/quic_dispatcher_test.cc
index ccac80c..b18bddb 100644
--- a/quic/core/quic_dispatcher_test.cc
+++ b/quic/core/quic_dispatcher_test.cc
@@ -679,15 +679,62 @@
// list manager.
EXPECT_CALL(*dispatcher_, CreateQuicSession(_, _, QuicStringPiece("hq"), _))
.Times(0);
- EXPECT_CALL(*time_wait_list_manager_,
- ProcessPacket(_, _, connection_id, _, _))
- .Times(1);
- EXPECT_CALL(*time_wait_list_manager_,
- AddConnectionIdToTimeWait(_, _, _, _, _))
- .Times(1);
+ if (GetQuicReloadableFlag(quic_reject_unprocessable_packets_statelessly)) {
+ EXPECT_CALL(*time_wait_list_manager_,
+ ProcessPacket(_, _, connection_id, _, _))
+ .Times(0);
+ EXPECT_CALL(*time_wait_list_manager_,
+ AddConnectionIdToTimeWait(_, _, _, _, _))
+ .Times(0);
+ EXPECT_CALL(*time_wait_list_manager_, SendPublicReset(_, _, _, _, _))
+ .Times(1);
+ } else {
+ EXPECT_CALL(*time_wait_list_manager_,
+ ProcessPacket(_, _, connection_id, _, _))
+ .Times(1);
+ EXPECT_CALL(*time_wait_list_manager_,
+ AddConnectionIdToTimeWait(_, _, _, _, _))
+ .Times(1);
+ }
ProcessPacket(client_address, connection_id, false, SerializeCHLO());
}
+TEST_F(QuicDispatcherTest,
+ DonotTimeWaitPacketsWithUnknownConnectionIdAndNoVersion) {
+ QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1);
+ CreateTimeWaitListManager();
+
+ char short_packet[22] = {0x70, 0xa7, 0x02, 0x6b};
+ QuicReceivedPacket packet(short_packet, 22, QuicTime::Zero());
+ char valid_size_packet[23] = {0x70, 0xa7, 0x02, 0x6c};
+ QuicReceivedPacket packet2(valid_size_packet, 23, QuicTime::Zero());
+ EXPECT_CALL(*dispatcher_, CreateQuicSession(_, _, _, _)).Times(0);
+ if (GetQuicReloadableFlag(quic_reject_unprocessable_packets_statelessly)) {
+ EXPECT_CALL(*time_wait_list_manager_, ProcessPacket(_, _, _, _, _))
+ .Times(0);
+ EXPECT_CALL(*time_wait_list_manager_,
+ AddConnectionIdToTimeWait(_, _, _, _, _))
+ .Times(0);
+ } else {
+ EXPECT_CALL(*time_wait_list_manager_, ProcessPacket(_, _, _, _, _))
+ .Times(2);
+ EXPECT_CALL(*time_wait_list_manager_,
+ AddConnectionIdToTimeWait(_, _, _, _, _))
+ .Times(2);
+ }
+ if (GetQuicReloadableFlag(quic_reject_unprocessable_packets_statelessly)) {
+ // Verify small packet is silently dropped.
+ EXPECT_CALL(*time_wait_list_manager_, SendPublicReset(_, _, _, _, _))
+ .Times(0);
+ }
+ dispatcher_->ProcessPacket(server_address_, client_address, packet);
+ if (GetQuicReloadableFlag(quic_reject_unprocessable_packets_statelessly)) {
+ EXPECT_CALL(*time_wait_list_manager_, SendPublicReset(_, _, _, _, _))
+ .Times(1);
+ }
+ dispatcher_->ProcessPacket(server_address_, client_address, packet2);
+}
+
// Makes sure nine-byte connection IDs are replaced by 8-byte ones.
TEST_F(QuicDispatcherTest, LongConnectionIdLengthReplaced) {
if (!QuicUtils::VariableLengthConnectionIdAllowedForVersion(
diff --git a/quic/core/quic_time_wait_list_manager.h b/quic/core/quic_time_wait_list_manager.h
index 2d621d9..c53632c 100644
--- a/quic/core/quic_time_wait_list_manager.h
+++ b/quic/core/quic_time_wait_list_manager.h
@@ -130,13 +130,6 @@
const QuicSocketAddress& peer_address,
std::unique_ptr<QuicPerPacketContext> packet_context);
- // Return a non-owning pointer to the packet writer.
- QuicPacketWriter* writer() { return writer_; }
-
- protected:
- virtual std::unique_ptr<QuicEncryptedPacket> BuildPublicReset(
- const QuicPublicResetPacket& packet);
-
// Creates a public reset packet and sends it or queues it to be sent later.
virtual void SendPublicReset(
const QuicSocketAddress& self_address,
@@ -145,6 +138,13 @@
bool ietf_quic,
std::unique_ptr<QuicPerPacketContext> packet_context);
+ // Return a non-owning pointer to the packet writer.
+ QuicPacketWriter* writer() { return writer_; }
+
+ protected:
+ virtual std::unique_ptr<QuicEncryptedPacket> BuildPublicReset(
+ const QuicPublicResetPacket& packet);
+
virtual void GetEndpointId(std::string* /*endpoint_id*/) {}
// Returns a stateless reset token which will be included in the public reset
diff --git a/quic/test_tools/mock_quic_time_wait_list_manager.h b/quic/test_tools/mock_quic_time_wait_list_manager.h
index 8a39442..b46e68b 100644
--- a/quic/test_tools/mock_quic_time_wait_list_manager.h
+++ b/quic/test_tools/mock_quic_time_wait_list_manager.h
@@ -53,6 +53,13 @@
const QuicSocketAddress& server_address,
const QuicSocketAddress& client_address,
std::unique_ptr<QuicPerPacketContext> packet_context));
+
+ MOCK_METHOD5(SendPublicReset,
+ void(const QuicSocketAddress&,
+ const QuicSocketAddress&,
+ QuicConnectionId,
+ bool,
+ std::unique_ptr<QuicPerPacketContext>));
};
} // namespace test
