In quic with tls, remove the head of line blocking caused by an unprocessable packet. protected by gfe2_reloadable_flag_quic_fix_undecryptable_packets.
PiperOrigin-RevId: 317090147
Change-Id: Ib1376131fa7f69bad3c9b6a16fc1fad9341a52ee
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 9901afa..b30165d 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -1989,24 +1989,8 @@
++stats_.undecryptable_packets_received_before_handshake_complete;
}
- bool should_enqueue = true;
- if (encryption_level_ == ENCRYPTION_FORWARD_SECURE) {
- // We do not expect to install any further keys.
- should_enqueue = false;
- } else if (undecryptable_packets_.size() >= max_undecryptable_packets_) {
- // We do not queue more than max_undecryptable_packets_ packets.
- should_enqueue = false;
- } else if (has_decryption_key) {
- // We already have the key for this decryption level, therefore no
- // future keys will allow it be decrypted.
- should_enqueue = false;
- } else if (version().KnowsWhichDecrypterToUse() &&
- decryption_level <= encryption_level_) {
- // On versions that know which decrypter to use, we install keys in order
- // so we will not get newer keys for lower encryption levels.
- should_enqueue = false;
- }
-
+ const bool should_enqueue =
+ ShouldEnqueueUnDecryptablePacket(decryption_level, has_decryption_key);
if (should_enqueue) {
QueueUndecryptablePacket(packet, decryption_level);
}
@@ -2017,6 +2001,31 @@
}
}
+bool QuicConnection::ShouldEnqueueUnDecryptablePacket(
+ EncryptionLevel decryption_level,
+ bool has_decryption_key) const {
+ if (encryption_level_ == ENCRYPTION_FORWARD_SECURE) {
+ // We do not expect to install any further keys.
+ return false;
+ }
+ if (undecryptable_packets_.size() >= max_undecryptable_packets_) {
+ // We do not queue more than max_undecryptable_packets_ packets.
+ return false;
+ }
+ if (has_decryption_key) {
+ // We already have the key for this decryption level, therefore no
+ // future keys will allow it be decrypted.
+ return false;
+ }
+ if (version().KnowsWhichDecrypterToUse() &&
+ decryption_level <= encryption_level_) {
+ // On versions that know which decrypter to use, we install keys in order
+ // so we will not get newer keys for lower encryption levels.
+ return false;
+ }
+ return true;
+}
+
void QuicConnection::ProcessUdpPacket(const QuicSocketAddress& self_address,
const QuicSocketAddress& peer_address,
const QuicReceivedPacket& packet) {
@@ -3142,27 +3151,79 @@
return;
}
- while (connected_ && !undecryptable_packets_.empty()) {
- // Making sure there is no pending frames when processing next undecrypted
- // packet because the queued ack frame may change.
- packet_creator_.FlushCurrentPacket();
- if (!connected_) {
- return;
+ if (GetQuicReloadableFlag(quic_fix_undecryptable_packets)) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_fix_undecryptable_packets);
+ auto iter = undecryptable_packets_.begin();
+ while (connected_ && iter != undecryptable_packets_.end()) {
+ // Making sure there is no pending frames when processing next undecrypted
+ // packet because the queued ack frame may change.
+ packet_creator_.FlushCurrentPacket();
+ if (!connected_) {
+ return;
+ }
+ UndecryptablePacket* undecryptable_packet = &*iter;
+ ++iter;
+ if (undecryptable_packet->processed) {
+ continue;
+ }
+ QUIC_DVLOG(1) << ENDPOINT << "Attempting to process undecryptable packet";
+ if (debug_visitor_ != nullptr) {
+ debug_visitor_->OnAttemptingToProcessUndecryptablePacket(
+ undecryptable_packet->encryption_level);
+ }
+ if (framer_.ProcessPacket(*undecryptable_packet->packet)) {
+ QUIC_DVLOG(1) << ENDPOINT << "Processed undecryptable packet!";
+ undecryptable_packet->processed = true;
+ ++stats_.packets_processed;
+ continue;
+ }
+ const bool has_decryption_key =
+ version().KnowsWhichDecrypterToUse() &&
+ framer_.HasDecrypterOfEncryptionLevel(
+ undecryptable_packet->encryption_level);
+ if (framer_.error() == QUIC_DECRYPTION_FAILURE &&
+ ShouldEnqueueUnDecryptablePacket(
+ undecryptable_packet->encryption_level, has_decryption_key)) {
+ QUIC_DVLOG(1)
+ << ENDPOINT
+ << "Need to attempt to process this undecryptable packet later";
+ continue;
+ }
+ undecryptable_packet->processed = true;
}
- QUIC_DVLOG(1) << ENDPOINT << "Attempting to process undecryptable packet";
- const auto& undecryptable_packet = undecryptable_packets_.front();
- if (debug_visitor_ != nullptr) {
- debug_visitor_->OnAttemptingToProcessUndecryptablePacket(
- undecryptable_packet.encryption_level);
+ // Remove processed packets. We cannot remove elements in the while loop
+ // above because currently QuicCircularDeque does not support removing
+ // mid elements.
+ while (!undecryptable_packets_.empty()) {
+ if (!undecryptable_packets_.front().processed) {
+ break;
+ }
+ undecryptable_packets_.pop_front();
}
- if (!framer_.ProcessPacket(*undecryptable_packet.packet) &&
- framer_.error() == QUIC_DECRYPTION_FAILURE) {
- QUIC_DVLOG(1) << ENDPOINT << "Unable to process undecryptable packet...";
- break;
+ } else {
+ while (connected_ && !undecryptable_packets_.empty()) {
+ // Making sure there is no pending frames when processing next undecrypted
+ // packet because the queued ack frame may change.
+ packet_creator_.FlushCurrentPacket();
+ if (!connected_) {
+ return;
+ }
+ QUIC_DVLOG(1) << ENDPOINT << "Attempting to process undecryptable packet";
+ const auto& undecryptable_packet = undecryptable_packets_.front();
+ if (debug_visitor_ != nullptr) {
+ debug_visitor_->OnAttemptingToProcessUndecryptablePacket(
+ undecryptable_packet.encryption_level);
+ }
+ if (!framer_.ProcessPacket(*undecryptable_packet.packet) &&
+ framer_.error() == QUIC_DECRYPTION_FAILURE) {
+ QUIC_DVLOG(1) << ENDPOINT
+ << "Unable to process undecryptable packet...";
+ break;
+ }
+ QUIC_DVLOG(1) << ENDPOINT << "Processed undecryptable packet!";
+ ++stats_.packets_processed;
+ undecryptable_packets_.pop_front();
}
- QUIC_DVLOG(1) << ENDPOINT << "Processed undecryptable packet!";
- ++stats_.packets_processed;
- undecryptable_packets_.pop_front();
}
// Once forward secure encryption is in use, there will be no
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 118e9c4..8f4b113 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -1095,12 +1095,16 @@
struct QUIC_EXPORT_PRIVATE UndecryptablePacket {
UndecryptablePacket(const QuicEncryptedPacket& packet,
EncryptionLevel encryption_level)
- : packet(packet.Clone()), encryption_level(encryption_level) {}
+ : packet(packet.Clone()),
+ encryption_level(encryption_level),
+ processed(false) {}
std::unique_ptr<QuicEncryptedPacket> packet;
- // Currently, |encryption_level| is only used for logging and does not
- // affect processing of the packet.
EncryptionLevel encryption_level;
+ // This gets set to true if 1) connection sucessfully processed the packet
+ // or 2) connection failed to process the packet and will not try to process
+ // it later.
+ bool processed;
};
// Notifies the visitor of the close and marks the connection as disconnected.
@@ -1314,6 +1318,11 @@
bool ValidateConfigConnectionIds(const QuicConfig& config);
bool ValidateConfigConnectionIdsOld(const QuicConfig& config);
+ // Returns true if an undecryptable packet of |decryption_level| should be
+ // buffered (such that connection can try to decrypt it later).
+ bool ShouldEnqueueUnDecryptablePacket(EncryptionLevel decryption_level,
+ bool has_decryption_key) const;
+
QuicFramer framer_;
// Contents received in the current packet, especially used to identify
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index db402c7..b1409bf 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -11196,6 +11196,67 @@
}
}
+TEST_P(QuicConnectionTest, ProcessUndecryptablePacketsBasedOnEncryptionLevel) {
+ if (!connection_.SupportsMultiplePacketNumberSpaces()) {
+ return;
+ }
+ // SetFromConfig is always called after construction from InitializeSession.
+ EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
+ EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
+ EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(AnyNumber());
+ QuicConfig config;
+ connection_.SetFromConfig(config);
+ connection_.SetDefaultEncryptionLevel(ENCRYPTION_INITIAL);
+ connection_.RemoveDecrypter(ENCRYPTION_FORWARD_SECURE);
+ use_tagging_decrypter();
+
+ peer_framer_.SetEncrypter(ENCRYPTION_HANDSHAKE,
+ std::make_unique<TaggingEncrypter>(0x01));
+ peer_framer_.SetEncrypter(ENCRYPTION_FORWARD_SECURE,
+ std::make_unique<TaggingEncrypter>(0x02));
+
+ for (uint64_t i = 1; i <= 3; ++i) {
+ ProcessDataPacketAtLevel(i, !kHasStopWaiting, ENCRYPTION_HANDSHAKE);
+ }
+ ProcessDataPacketAtLevel(4, !kHasStopWaiting, ENCRYPTION_FORWARD_SECURE);
+ for (uint64_t j = 5; j <= 7; ++j) {
+ ProcessDataPacketAtLevel(j, !kHasStopWaiting, ENCRYPTION_HANDSHAKE);
+ }
+ EXPECT_EQ(7u, QuicConnectionPeer::NumUndecryptablePackets(&connection_));
+ EXPECT_FALSE(connection_.GetProcessUndecryptablePacketsAlarm()->IsSet());
+ SetDecrypter(ENCRYPTION_HANDSHAKE,
+ std::make_unique<StrictTaggingDecrypter>(0x01));
+ EXPECT_TRUE(connection_.GetProcessUndecryptablePacketsAlarm()->IsSet());
+ connection_.SetDefaultEncryptionLevel(ENCRYPTION_HANDSHAKE);
+ connection_.SetEncrypter(ENCRYPTION_HANDSHAKE,
+ std::make_unique<TaggingEncrypter>(0x01));
+ if (GetQuicReloadableFlag(quic_fix_undecryptable_packets)) {
+ // Verify all ENCRYPTION_HANDSHAKE packets get processed.
+ EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(6);
+ } else {
+ // Verify packets before 4 get processed.
+ EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(3);
+ }
+ connection_.GetProcessUndecryptablePacketsAlarm()->Fire();
+ EXPECT_EQ(4u, QuicConnectionPeer::NumUndecryptablePackets(&connection_));
+
+ SetDecrypter(ENCRYPTION_FORWARD_SECURE,
+ std::make_unique<StrictTaggingDecrypter>(0x02));
+ EXPECT_TRUE(connection_.GetProcessUndecryptablePacketsAlarm()->IsSet());
+ connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
+ connection_.SetEncrypter(ENCRYPTION_FORWARD_SECURE,
+ std::make_unique<TaggingEncrypter>(0x02));
+ if (GetQuicReloadableFlag(quic_fix_undecryptable_packets)) {
+ // Verify the 1-RTT packet gets processed.
+ EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1);
+ } else {
+ // Verify all packets get processed.
+ EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(4);
+ }
+ connection_.GetProcessUndecryptablePacketsAlarm()->Fire();
+ EXPECT_EQ(0u, QuicConnectionPeer::NumUndecryptablePackets(&connection_));
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index 726f1e5..847a166 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -2007,6 +2007,10 @@
return encrypter_[level] != nullptr;
}
+bool QuicFramer::HasDecrypterOfEncryptionLevel(EncryptionLevel level) const {
+ return decrypter_[level] != nullptr;
+}
+
bool QuicFramer::AppendPacketHeader(const QuicPacketHeader& header,
QuicDataWriter* writer,
size_t* length_field_offset) {
diff --git a/quic/core/quic_framer.h b/quic/core/quic_framer.h
index 8ace715..0aa5bbf 100644
--- a/quic/core/quic_framer.h
+++ b/quic/core/quic_framer.h
@@ -571,6 +571,8 @@
// Returns true if encrypter of |level| is available.
bool HasEncrypterOfEncryptionLevel(EncryptionLevel level) const;
+ // Returns true if decrypter of |level| is available.
+ bool HasDecrypterOfEncryptionLevel(EncryptionLevel level) const;
void set_validate_flags(bool value) { validate_flags_ = value; }
diff --git a/quic/test_tools/quic_connection_peer.cc b/quic/test_tools/quic_connection_peer.cc
index d276f63..54616b2 100644
--- a/quic/test_tools/quic_connection_peer.cc
+++ b/quic/test_tools/quic_connection_peer.cc
@@ -377,5 +377,10 @@
connection->InstallInitialCrypters(server_connection_id);
}
+// static
+size_t QuicConnectionPeer::NumUndecryptablePackets(QuicConnection* connection) {
+ return connection->undecryptable_packets_.size();
+}
+
} // namespace test
} // namespace quic
diff --git a/quic/test_tools/quic_connection_peer.h b/quic/test_tools/quic_connection_peer.h
index 9882f62..a5b94ef 100644
--- a/quic/test_tools/quic_connection_peer.h
+++ b/quic/test_tools/quic_connection_peer.h
@@ -147,6 +147,8 @@
static void SetServerConnectionId(
QuicConnection* connection,
const QuicConnectionId& server_connection_id);
+
+ static size_t NumUndecryptablePackets(QuicConnection* connection);
};
} // namespace test
