In quic, consider discarding keys as forward progress. protected by gfe2_reloadable_flag_quic_default_enable_5rto_blackhole_detection2 which replaces gfe2_reloadable_flag_quic_default_enable_5rto_blackhole_detection.

PiperOrigin-RevId: 315548676
Change-Id: Ia3db7908a74941108c20fa31d5cfe0c21e252bb9
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index e332a89..e7990d9 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -2338,6 +2338,13 @@
   sent_packet_manager_.NeuterUnencryptedPackets();
   // This may have changed the retransmission timer, so re-arm it.
   SetRetransmissionAlarm();
+  if (default_enable_5rto_blackhole_detection_) {
+    QUIC_RELOADABLE_FLAG_COUNT_N(quic_default_enable_5rto_blackhole_detection2,
+                                 1, 3);
+    // Consider this as forward progress since this is called when initial key
+    // gets discarded (or previous unencrypted data is not needed anymore).
+    OnForwardProgressMade();
+  }
   if (SupportsMultiplePacketNumberSpaces()) {
     // Stop sending ack of initial packet number space.
     uber_received_packet_manager_.ResetAckStates(ENCRYPTION_INITIAL);
@@ -2889,6 +2896,11 @@
   sent_packet_manager_.SetHandshakeConfirmed();
   // This may have changed the retransmission timer, so re-arm it.
   SetRetransmissionAlarm();
+  if (default_enable_5rto_blackhole_detection_) {
+    QUIC_RELOADABLE_FLAG_COUNT_N(quic_default_enable_5rto_blackhole_detection2,
+                                 2, 3);
+    OnForwardProgressMade();
+  }
   if (!SupportsMultiplePacketNumberSpaces()) {
     // The client should immediately ack the SHLO to confirm the handshake is
     // complete with the server.
@@ -4040,15 +4052,7 @@
   // have a better estimate of the current rtt than when it was set.
   SetRetransmissionAlarm();
   if (acked_new_packet) {
-    is_path_degrading_ = false;
-    if (sent_packet_manager_.HasInFlightPackets()) {
-      // Restart detections if forward progress has been made.
-      blackhole_detector_.RestartDetection(GetPathDegradingDeadline(),
-                                           GetNetworkBlackholeDeadline());
-    } else {
-      // Stop detections in quiecense.
-      blackhole_detector_.StopDetection();
-    }
+    OnForwardProgressMade();
   }
 
   if (send_stop_waiting) {
@@ -4346,6 +4350,18 @@
   }
 }
 
+void QuicConnection::OnForwardProgressMade() {
+  is_path_degrading_ = false;
+  if (sent_packet_manager_.HasInFlightPackets()) {
+    // Restart detections if forward progress has been made.
+    blackhole_detector_.RestartDetection(GetPathDegradingDeadline(),
+                                         GetNetworkBlackholeDeadline());
+  } else {
+    // Stop detections in quiecense.
+    blackhole_detector_.StopDetection();
+  }
+}
+
 QuicPacketNumber QuicConnection::GetLargestReceivedPacketWithAck() const {
   if (SupportsMultiplePacketNumberSpaces()) {
     return largest_seen_packets_with_ack_[QuicUtils::GetPacketNumberSpace(
@@ -4545,7 +4561,8 @@
   }
   // No blackhole detection before handshake completes.
   if (default_enable_5rto_blackhole_detection_) {
-    QUIC_RELOADABLE_FLAG_COUNT(quic_default_enable_5rto_blackhole_detection);
+    QUIC_RELOADABLE_FLAG_COUNT_N(quic_default_enable_5rto_blackhole_detection2,
+                                 3, 3);
     return IsHandshakeComplete();
   }
 
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 5d149b4..56f84c2 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -1261,6 +1261,10 @@
   // received packet number which contains an ACK frame.
   void SetLargestReceivedPacketWithAck(QuicPacketNumber new_value);
 
+  // Called when new packets have been acknowledged or old keys have been
+  // discarded.
+  void OnForwardProgressMade();
+
   // Returns largest received packet number which contains an ACK frame.
   QuicPacketNumber GetLargestReceivedPacketWithAck() const;
 
@@ -1701,7 +1705,7 @@
           : GetQuicFlag(FLAGS_quic_anti_amplification_factor);
 
   const bool default_enable_5rto_blackhole_detection_ =
-      GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection);
+      GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2);
 };
 
 }  // namespace quic
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 8d096eb..6602a8c 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -6117,7 +6117,7 @@
   connection_options.push_back(k5RTO);
   config.SetConnectionOptionsToSend(connection_options);
   QuicConfigPeer::SetNegotiated(&config, true);
-  if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+  if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
     EXPECT_CALL(visitor_, GetHandshakeState())
         .WillRepeatedly(Return(HANDSHAKE_COMPLETE));
   }
@@ -9930,7 +9930,7 @@
   }
   EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
   connection_.SetFromConfig(config);
-  if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+  if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
     EXPECT_CALL(visitor_, GetHandshakeState())
         .WillRepeatedly(Return(HANDSHAKE_COMPLETE));
   }
@@ -9981,7 +9981,7 @@
   }
   EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
   connection_.SetFromConfig(config);
-  if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+  if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
     EXPECT_CALL(visitor_, GetHandshakeState())
         .WillRepeatedly(Return(HANDSHAKE_COMPLETE));
   }
@@ -10031,7 +10031,7 @@
   config.SetConnectionOptionsToSend(connection_options);
   EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
   connection_.SetFromConfig(config);
-  if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+  if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
     EXPECT_CALL(visitor_, GetHandshakeState())
         .WillRepeatedly(Return(HANDSHAKE_COMPLETE));
   }
@@ -10962,7 +10962,7 @@
 }
 
 TEST_P(QuicConnectionTest, ClientOnlyBlackholeDetectionClient) {
-  if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+  if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
     return;
   }
   QuicConfig config;
@@ -10983,7 +10983,7 @@
 }
 
 TEST_P(QuicConnectionTest, ClientOnlyBlackholeDetectionServer) {
-  if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+  if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
     return;
   }
   set_perspective(Perspective::IS_SERVER);
@@ -11009,7 +11009,7 @@
 }
 
 TEST_P(QuicConnectionTest, 2RtoBlackholeDetection) {
-  if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+  if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
     return;
   }
   QuicConfig config;
@@ -11036,7 +11036,7 @@
 }
 
 TEST_P(QuicConnectionTest, 3RtoBlackholeDetection) {
-  if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+  if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
     return;
   }
   QuicConfig config;
@@ -11063,7 +11063,7 @@
 }
 
 TEST_P(QuicConnectionTest, 4RtoBlackholeDetection) {
-  if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+  if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
     return;
   }
   QuicConfig config;
@@ -11090,7 +11090,7 @@
 }
 
 TEST_P(QuicConnectionTest, 6RtoBlackholeDetection) {
-  if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+  if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
     return;
   }
   QuicConfig config;
@@ -11116,6 +11116,49 @@
             QuicConnectionPeer::GetBlackholeDetectionDeadline(&connection_));
 }
 
+// Regresstion test for b/158491591.
+TEST_P(QuicConnectionTest, MadeForwardProgressOnDiscardingKeys) {
+  if (!connection_.SupportsMultiplePacketNumberSpaces()) {
+    return;
+  }
+  use_tagging_decrypter();
+  // Send handshake packet.
+  connection_.SetEncrypter(ENCRYPTION_HANDSHAKE,
+                           std::make_unique<TaggingEncrypter>(0x02));
+  connection_.SetDefaultEncryptionLevel(ENCRYPTION_HANDSHAKE);
+  EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1);
+  QuicConfig config;
+  QuicTagVector connection_options;
+  connection_options.push_back(k5RTO);
+  config.SetConnectionOptionsToSend(connection_options);
+  QuicConfigPeer::SetNegotiated(&config, true);
+  if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
+    EXPECT_CALL(visitor_, GetHandshakeState())
+        .WillRepeatedly(Return(HANDSHAKE_COMPLETE));
+  }
+  if (connection_.version().AuthenticatesHandshakeConnectionIds()) {
+    QuicConfigPeer::SetReceivedOriginalConnectionId(
+        &config, connection_.connection_id());
+    QuicConfigPeer::SetReceivedInitialSourceConnectionId(
+        &config, connection_.connection_id());
+  }
+  EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
+  connection_.SetFromConfig(config);
+
+  connection_.SendCryptoDataWithString("foo", 0, ENCRYPTION_HANDSHAKE);
+  EXPECT_TRUE(connection_.BlackholeDetectionInProgress());
+  // Discard handshake keys.
+  connection_.OnHandshakeComplete();
+  if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
+    // Verify blackhole detection stops.
+    EXPECT_FALSE(connection_.BlackholeDetectionInProgress());
+  } else {
+    // Problematic: although there is nothing in flight, blackhole detection is
+    // still in progress.
+    EXPECT_TRUE(connection_.BlackholeDetectionInProgress());
+  }
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace quic