gfe-relnote: In QUIC, actually remove encrypters when discarding old encryption keys with TLS handshake. Protected by blocked gfe2_reloadable_flag_quic_enable_version_t* flags.
PiperOrigin-RevId: 294442019
Change-Id: I1513cece5c74341ddbfb7b0debe01b6aad480a2f
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index b064902..1677990 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -2073,6 +2073,13 @@
sent_packet_manager_.NeuterUnencryptedPackets();
// This may have changed the retransmission timer, so re-arm it.
SetRetransmissionAlarm();
+ if (SupportsMultiplePacketNumberSpaces()) {
+ // Stop sending ack of initial packet number space.
+ uber_received_packet_manager_.ResetAckStates(ENCRYPTION_INITIAL);
+ // Re-arm ack alarm.
+ ack_alarm_->Update(uber_received_packet_manager_.GetEarliestAckTimeout(),
+ kAlarmGranularity);
+ }
}
bool QuicConnection::ShouldGeneratePacket(
@@ -2566,11 +2573,19 @@
sent_packet_manager_.SetHandshakeConfirmed();
// This may have changed the retransmission timer, so re-arm it.
SetRetransmissionAlarm();
- // The client should immediately ack the SHLO to confirm the handshake is
- // complete with the server.
- if (perspective_ == Perspective::IS_CLIENT && ack_frame_updated()) {
- ack_alarm_->Update(clock_->ApproximateNow(), QuicTime::Delta::Zero());
+ if (!SupportsMultiplePacketNumberSpaces()) {
+ // The client should immediately ack the SHLO to confirm the handshake is
+ // complete with the server.
+ if (perspective_ == Perspective::IS_CLIENT && ack_frame_updated()) {
+ ack_alarm_->Update(clock_->ApproximateNow(), QuicTime::Delta::Zero());
+ }
+ return;
}
+ // Stop sending ack of handshake packet number space.
+ uber_received_packet_manager_.ResetAckStates(ENCRYPTION_HANDSHAKE);
+ // Re-arm ack alarm.
+ ack_alarm_->Update(uber_received_packet_manager_.GetEarliestAckTimeout(),
+ kAlarmGranularity);
}
void QuicConnection::SendOrQueuePacket(SerializedPacket* packet) {
@@ -2727,6 +2742,10 @@
packet_creator_.SetEncrypter(level, std::move(encrypter));
}
+void QuicConnection::RemoveEncrypter(EncryptionLevel level) {
+ framer_.RemoveEncrypter(level);
+}
+
void QuicConnection::SetDiversificationNonce(
const DiversificationNonce& nonce) {
DCHECK_EQ(Perspective::IS_SERVER, perspective_);
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 12fa4e4..a3c7b8d 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -671,6 +671,9 @@
void SetEncrypter(EncryptionLevel level,
std::unique_ptr<QuicEncrypter> encrypter);
+ // Called to remove encrypter of encryption |level|.
+ void RemoveEncrypter(EncryptionLevel level);
+
// SetNonceForPublicHeader sets the nonce that will be transmitted in the
// header of each packet encrypted at the initial encryption level decrypted.
// This should only be called on the server side.
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index d81bf2b..40d5278 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -6661,7 +6661,12 @@
QuicConnectionPeer::SetPerspective(&connection_, Perspective::IS_CLIENT);
connection_.OnHandshakeComplete();
EXPECT_TRUE(connection_.GetAckAlarm()->IsSet());
- EXPECT_EQ(clock_.ApproximateNow(), connection_.GetAckAlarm()->deadline());
+ if (connection_.SupportsMultiplePacketNumberSpaces()) {
+ EXPECT_EQ(clock_.ApproximateNow() + DefaultDelayedAckTime(),
+ connection_.GetAckAlarm()->deadline());
+ } else {
+ EXPECT_EQ(clock_.ApproximateNow(), connection_.GetAckAlarm()->deadline());
+ }
}
TEST_P(QuicConnectionTest, SendDelayedAckOnSecondPacket) {
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index 1170595..5d9acca 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -4128,6 +4128,12 @@
encrypter_[level] = std::move(encrypter);
}
+void QuicFramer::RemoveEncrypter(EncryptionLevel level) {
+ QUIC_DVLOG(1) << ENDPOINT << "Removing encrypter of "
+ << EncryptionLevelToString(level);
+ encrypter_[level] = nullptr;
+}
+
void QuicFramer::SetInitialObfuscators(QuicConnectionId connection_id) {
CrypterPair crypters;
CryptoUtils::CreateInitialObfuscators(perspective_, version_, connection_id,
diff --git a/quic/core/quic_framer.h b/quic/core/quic_framer.h
index 5914e5a..9863dc1 100644
--- a/quic/core/quic_framer.h
+++ b/quic/core/quic_framer.h
@@ -520,6 +520,9 @@
void SetEncrypter(EncryptionLevel level,
std::unique_ptr<QuicEncrypter> encrypter);
+ // Called to remove encrypter of encryption |level|.
+ void RemoveEncrypter(EncryptionLevel level);
+
// Sets the encrypter and decrypter for the ENCRYPTION_INITIAL level.
void SetInitialObfuscators(QuicConnectionId connection_id);
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index aea4663..f9d186d 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -1374,7 +1374,6 @@
void QuicSession::DiscardOldDecryptionKey(EncryptionLevel level) {
if (!connection()->version().KnowsWhichDecrypterToUse()) {
- // TODO(fayang): actually discard keys.
return;
}
connection()->RemoveDecrypter(level);
@@ -1383,7 +1382,9 @@
void QuicSession::DiscardOldEncryptionKey(EncryptionLevel level) {
QUIC_DVLOG(1) << ENDPOINT << "Discard keys of "
<< EncryptionLevelToString(level);
- // TODO(fayang): actually discard keys.
+ if (connection()->version().handshake_protocol == PROTOCOL_TLS1_3) {
+ connection()->RemoveEncrypter(level);
+ }
switch (level) {
case ENCRYPTION_INITIAL:
NeuterUnencryptedData();
