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

quic/core/quic_connection.cc

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_);

quic/core/quic_connection.h

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.

quic/core/quic_connection_test.cc

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) {

quic/core/quic_framer.cc

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,

quic/core/quic_framer.h

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);
 

quic/core/quic_session.cc

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();