Ensure we can decrypt the first packet
At the IETF interop in London we noticed that our server would always fail to decrypt the first packet when TLS is in use. This CL fixes that issue by installing the decrypter immediately on connection creation, and creates a test to make sure it doesn't happen again.
gfe-relnote: ensure first packet is decrypted, protected by disabled flag quic_supports_tls_handshake
PiperOrigin-RevId: 249437099
Change-Id: Iebcc0f5c18642a4f3db3265ab7bf669dd76e782b
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc
index 0e4bb8c..36ab085 100644
--- a/quic/core/http/end_to_end_test.cc
+++ b/quic/core/http/end_to_end_test.cc
@@ -731,6 +731,20 @@
EXPECT_EQ("200", client_->response_headers()->find(":status")->second);
}
+TEST_P(EndToEndTestWithTls, NoUndecryptablePackets) {
+ ASSERT_TRUE(Initialize());
+
+ EXPECT_EQ(kFooResponseBody, client_->SendSynchronousRequest("/foo"));
+ EXPECT_EQ("200", client_->response_headers()->find(":status")->second);
+
+ QuicConnectionStats client_stats =
+ client_->client()->client_session()->connection()->GetStats();
+ QuicConnectionStats server_stats = GetServerConnection()->GetStats();
+
+ EXPECT_EQ(0u, client_stats.undecryptable_packets_received);
+ EXPECT_EQ(0u, server_stats.undecryptable_packets_received);
+}
+
TEST_P(EndToEndTestWithTls, SeparateFinPacket) {
ASSERT_TRUE(Initialize());
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index a8672f5..73959f0 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -412,6 +412,19 @@
DCHECK(!GetQuicRestartFlag(quic_no_server_conn_ver_negotiation2) ||
perspective_ == Perspective::IS_CLIENT ||
supported_versions.size() == 1);
+ InstallInitialCrypters();
+}
+
+void QuicConnection::InstallInitialCrypters() {
+ if (version().handshake_protocol != PROTOCOL_TLS1_3) {
+ // Initial crypters are currently only supported with TLS.
+ return;
+ }
+ CrypterPair crypters;
+ CryptoUtils::CreateTlsInitialCrypters(perspective_, transport_version(),
+ server_connection_id_, &crypters);
+ SetEncrypter(ENCRYPTION_INITIAL, std::move(crypters.encrypter));
+ InstallDecrypter(ENCRYPTION_INITIAL, std::move(crypters.decrypter));
}
QuicConnection::~QuicConnection() {
@@ -770,6 +783,7 @@
void QuicConnection::OnRetryPacket(QuicConnectionId original_connection_id,
QuicConnectionId new_connection_id,
QuicStringPiece retry_token) {
+ DCHECK_EQ(Perspective::IS_CLIENT, perspective_);
if (original_connection_id != server_connection_id_) {
QUIC_DLOG(ERROR) << "Ignoring RETRY with original connection ID "
<< original_connection_id << " not matching expected "
@@ -792,12 +806,7 @@
packet_generator_.SetRetryToken(retry_token);
// Reinstall initial crypters because the connection ID changed.
- CrypterPair crypters;
- CryptoUtils::CreateTlsInitialCrypters(Perspective::IS_CLIENT,
- transport_version(),
- server_connection_id_, &crypters);
- SetEncrypter(ENCRYPTION_INITIAL, std::move(crypters.encrypter));
- InstallDecrypter(ENCRYPTION_INITIAL, std::move(crypters.decrypter));
+ InstallInitialCrypters();
}
bool QuicConnection::HasIncomingConnectionId(QuicConnectionId connection_id) {
@@ -1997,6 +2006,7 @@
// If we are unable to decrypt this packet, it might be
// because the CHLO or SHLO packet was lost.
if (framer_.error() == QUIC_DECRYPTION_FAILURE) {
+ ++stats_.undecryptable_packets_received;
if (encryption_level_ != ENCRYPTION_FORWARD_SECURE &&
undecryptable_packets_.size() < max_undecryptable_packets_) {
QueueUndecryptablePacket(packet);
@@ -3051,6 +3061,7 @@
// If we are unable to decrypt this packet, it might be
// because the CHLO or SHLO packet was lost.
if (framer_.error() == QUIC_DECRYPTION_FAILURE) {
+ ++stats_.undecryptable_packets_received;
if (encryption_level_ != ENCRYPTION_FORWARD_SECURE &&
undecryptable_packets_.size() < max_undecryptable_packets_) {
QueueUndecryptablePacket(*packet);
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index b274266..3d421f5 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -1143,6 +1143,9 @@
// Whether incoming_connection_ids_ contains connection_id.
bool HasIncomingConnectionId(QuicConnectionId connection_id);
+ // Install encrypter and decrypter for ENCRYPTION_INITIAL.
+ void InstallInitialCrypters();
+
QuicFramer framer_;
// Contents received in the current packet, especially used to identify
diff --git a/quic/core/quic_connection_stats.cc b/quic/core/quic_connection_stats.cc
index 4d5c578..246b9f1 100644
--- a/quic/core/quic_connection_stats.cc
+++ b/quic/core/quic_connection_stats.cc
@@ -29,6 +29,7 @@
slowstart_duration(QuicTime::Delta::Zero()),
slowstart_start_time(QuicTime::Zero()),
packets_dropped(0),
+ undecryptable_packets_received(0),
crypto_retransmit_count(0),
loss_timeout_count(0),
tlp_count(0),
@@ -71,6 +72,7 @@
os << " slowstart_packets_lost: " << s.slowstart_packets_lost;
os << " slowstart_bytes_lost: " << s.slowstart_bytes_lost;
os << " packets_dropped: " << s.packets_dropped;
+ os << " undecryptable_packets_received: " << s.undecryptable_packets_received;
os << " crypto_retransmit_count: " << s.crypto_retransmit_count;
os << " loss_timeout_count: " << s.loss_timeout_count;
os << " tlp_count: " << s.tlp_count;
diff --git a/quic/core/quic_connection_stats.h b/quic/core/quic_connection_stats.h
index df2f7f8..5317c7a 100644
--- a/quic/core/quic_connection_stats.h
+++ b/quic/core/quic_connection_stats.h
@@ -66,6 +66,10 @@
QuicTime slowstart_start_time;
QuicPacketCount packets_dropped; // Duplicate or less than least unacked.
+
+ // Packets that failed to decrypt when they were first received.
+ QuicPacketCount undecryptable_packets_received;
+
size_t crypto_retransmit_count;
// Count of times the loss detection alarm fired. At least one packet should
// be lost when the alarm fires.
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 0d09430..3e03e1f 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -938,6 +938,14 @@
peer_creator_.SetEncrypter(
level, QuicMakeUnique<NullEncrypter>(peer_framer_.perspective()));
}
+ if (version().handshake_protocol == PROTOCOL_TLS1_3) {
+ connection_.SetEncrypter(
+ ENCRYPTION_INITIAL,
+ QuicMakeUnique<NullEncrypter>(Perspective::IS_CLIENT));
+ connection_.InstallDecrypter(
+ ENCRYPTION_INITIAL,
+ QuicMakeUnique<NullDecrypter>(Perspective::IS_CLIENT));
+ }
QuicFramerPeer::SetLastSerializedServerConnectionId(
QuicConnectionPeer::GetFramer(&connection_), connection_id_);
if (version().transport_version > QUIC_VERSION_43) {
