Use next_connection_id_sequence_number to validate retired CID number as retiring an active CID would not guarantee that a new CID is issued due to CID collision in dispatcher.

Default enabled as a bug fix.

Protected by FLAGS_quic_reloadable_flag_quic_check_retire_cid_with_next_cid_sequence_number.

PiperOrigin-RevId: 487508836

quiche/quic/core/quic_connection_id_manager.cc

diff --git a/quiche/quic/core/quic_connection_id_manager.cc b/quiche/quic/core/quic_connection_id_manager.cc
index a2a3fbf..4776545 100644
--- a/quiche/quic/core/quic_connection_id_manager.cc
+++ b/quiche/quic/core/quic_connection_id_manager.cc
@@ -340,9 +340,19 @@
     const QuicRetireConnectionIdFrame& frame, QuicTime::Delta pto_delay,
     std::string* error_detail) {
   QUICHE_DCHECK(!active_connection_ids_.empty());
-  if (frame.sequence_number > active_connection_ids_.back().second) {
-    *error_detail = "To be retired connecton ID is never issued.";
-    return IETF_QUIC_PROTOCOL_VIOLATION;
+  if (GetQuicReloadableFlag(
+          quic_check_retire_cid_with_next_cid_sequence_number)) {
+    QUIC_RELOADABLE_FLAG_COUNT(
+        quic_check_retire_cid_with_next_cid_sequence_number);
+    if (frame.sequence_number >= next_connection_id_sequence_number_) {
+      *error_detail = "To be retired connecton ID is never issued.";
+      return IETF_QUIC_PROTOCOL_VIOLATION;
+    }
+  } else {
+    if (frame.sequence_number > active_connection_ids_.back().second) {
+      *error_detail = "To be retired connecton ID is never issued.";
+      return IETF_QUIC_PROTOCOL_VIOLATION;
+    }
   }
 
   auto it =

quiche/quic/core/quic_connection_id_manager_test.cc

diff --git a/quiche/quic/core/quic_connection_id_manager_test.cc b/quiche/quic/core/quic_connection_id_manager_test.cc
index 9df262c..2dd35bf 100644
--- a/quiche/quic/core/quic_connection_id_manager_test.cc
+++ b/quiche/quic/core/quic_connection_id_manager_test.cc
@@ -6,8 +6,10 @@
 
 #include <cstddef>
 
+#include "quiche/quic/core/frames/quic_retire_connection_id_frame.h"
 #include "quiche/quic/core/quic_connection_id.h"
 #include "quiche/quic/core/quic_error_codes.h"
+#include "quiche/quic/platform/api/quic_flags.h"
 #include "quiche/quic/platform/api/quic_test.h"
 #include "quiche/quic/test_tools/mock_clock.h"
 #include "quiche/quic/test_tools/mock_connection_id_generator.h"
@@ -1028,5 +1030,45 @@
   cid_manager_.MaybeSendNewConnectionIds();
 }
 
+// Regression test for b/258450534
+TEST_F(QuicSelfIssuedConnectionIdManagerTest,
+       RetireConnectionIdAfterConnectionIdCollisionIsFine) {
+  SetQuicReloadableFlag(quic_check_cid_collision_when_issue_new_cid, true);
+  QuicConnectionId cid0 = initial_connection_id_;
+  QuicConnectionId cid1 = CheckGenerate(cid0);
+  EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(cid1))
+      .WillOnce(Return(true));
+  EXPECT_CALL(cid_manager_visitor_, SendNewConnectionId(_))
+      .WillOnce(Return(true));
+  cid_manager_.MaybeSendNewConnectionIds();
+
+  QuicRetireConnectionIdFrame retire_cid_frame(/*control_frame_id=*/0,
+                                               /*sequence_number=*/1);
+  QuicConnectionId cid2 = CheckGenerate(cid1);
+  // This happens when cid2 is aleady present in the dispatcher map.
+  EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(cid2))
+      .WillOnce(Return(false));
+  std::string error_details;
+  EXPECT_EQ(
+      cid_manager_.OnRetireConnectionIdFrame(
+          retire_cid_frame, QuicTime::Delta::FromSeconds(1), &error_details),
+      QUIC_NO_ERROR)
+      << error_details;
+
+  if (GetQuicReloadableFlag(
+          quic_check_retire_cid_with_next_cid_sequence_number)) {
+    EXPECT_EQ(
+        cid_manager_.OnRetireConnectionIdFrame(
+            retire_cid_frame, QuicTime::Delta::FromSeconds(1), &error_details),
+        QUIC_NO_ERROR)
+        << error_details;
+  } else {
+    EXPECT_EQ(
+        cid_manager_.OnRetireConnectionIdFrame(
+            retire_cid_frame, QuicTime::Delta::FromSeconds(1), &error_details),
+        IETF_QUIC_PROTOCOL_VIOLATION);
+  }
+}
+
 }  // namespace
 }  // namespace quic::test

quiche/quic/core/quic_flags_list.h

diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h
index 88cd4a0..7f855cb 100644
--- a/quiche/quic/core/quic_flags_list.h
+++ b/quiche/quic/core/quic_flags_list.h
@@ -85,6 +85,8 @@
 QUIC_FLAG(quic_reloadable_flag_quic_use_ping_manager2, true)
 // If true, use new connection ID in connection migration.
 QUIC_FLAG(quic_reloadable_flag_quic_connection_migration_use_new_cid_v2, true)
+// If true, use next_connection_id_sequence_number to validate retired cid number.
+QUIC_FLAG(quic_reloadable_flag_quic_check_retire_cid_with_next_cid_sequence_number, true)
 // If true, uses conservative cwnd gain and pacing gain when cwnd gets bootstrapped.
 QUIC_FLAG(quic_reloadable_flag_quic_conservative_cwnd_and_pacing_gains, false)
 // Instead of assuming an incoming connection ID length for short headers, ask each time.