From 5be8143126f8cfa8a483d4a5ae475b9a46053fa1 Mon Sep 17 00:00:00 2001
From: Alessandro Ghedini <alessandro@ghedini.me>
Date: Tue, 9 Jan 2024 12:25:03 +0000
Subject: [PATCH] cid: always retire CIDs with seqnum lower than retire prior
 to

When a NEW_CONNECTION_ID frame with a seqnum lower than previously
received retire prior to is received, the new CID needs to be
immediately retired. However, if the CID was already queued to be
retired we would keep processing it instead of just ignoring it.
---
 quiche/src/cid.rs | 44 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 40 insertions(+), 4 deletions(-)

diff --git a/quiche/src/cid.rs b/quiche/src/cid.rs
index 325b15bd18..aa0caa8a7c 100644
--- a/quiche/src/cid.rs
+++ b/quiche/src/cid.rs
@@ -455,10 +455,11 @@ impl ConnectionIdentifiers {
         // received NEW_CONNECTION_ID frame MUST send a corresponding
         // RETIRE_CONNECTION_ID frame that retires the newly received connection
         // ID, unless it has already done so for that sequence number.
-        if seq < self.largest_peer_retire_prior_to &&
-            !self.retire_dcid_seqs.contains(&seq)
-        {
-            self.retire_dcid_seqs.push_back(seq);
+        if seq < self.largest_peer_retire_prior_to {
+            if !self.retire_dcid_seqs.contains(&seq) {
+                self.retire_dcid_seqs.push_back(seq);
+            }
+
             return Ok(retired_path_ids);
         }
 
@@ -917,6 +918,41 @@ mod tests {
         assert_eq!(ids.dcids.len(), 1);
     }
 
+    #[test]
+    fn new_dcid_reordered() {
+        let (scid, _) = create_cid_and_reset_token(16);
+        let (dcid, _) = create_cid_and_reset_token(16);
+
+        let mut ids = ConnectionIdentifiers::new(2, &scid, 0, None);
+        ids.set_initial_dcid(dcid, None, Some(0));
+
+        assert_eq!(ids.available_dcids(), 0);
+        assert_eq!(ids.dcids.len(), 1);
+
+        // Skip DCID #1 (e.g due to packet loss) and insert DCID #2.
+        let (dcid, rt) = create_cid_and_reset_token(16);
+        assert!(ids.new_dcid(dcid.clone(), 2, rt, 1).is_ok());
+        assert_eq!(ids.dcids.len(), 1);
+
+        let (dcid, rt) = create_cid_and_reset_token(16);
+        assert!(ids.new_dcid(dcid.clone(), 3, rt, 2).is_ok());
+        assert_eq!(ids.dcids.len(), 2);
+
+        let (dcid, rt) = create_cid_and_reset_token(16);
+        assert!(ids.new_dcid(dcid.clone(), 4, rt, 3).is_ok());
+        assert_eq!(ids.dcids.len(), 2);
+
+        // Insert DCID #1 (e.g due to packet reordering).
+        let (dcid, rt) = create_cid_and_reset_token(16);
+        assert!(ids.new_dcid(dcid.clone(), 1, rt, 0).is_ok());
+        assert_eq!(ids.dcids.len(), 2);
+
+        // Try inserting DCID #1 again (e.g. due to retransmission).
+        let (dcid, rt) = create_cid_and_reset_token(16);
+        assert!(ids.new_dcid(dcid.clone(), 1, rt, 0).is_ok());
+        assert_eq!(ids.dcids.len(), 2);
+    }
+
     #[test]
     fn retire_scids() {
         let (scid, _) = create_cid_and_reset_token(16);