Multipath: complete server-side NAT rebinding migration

src/connection/connection.rs

@@ -871,6 +871,13 @@ impl Connection {
                         path.scid_seq = None;
                     }
                 }
+
+                // RFC 9000 Section 5.1.1: an endpoint SHOULD supply a new
+                // connection ID when the peer retires one. Without this the
+                // pool only ever shrinks — e.g. repeated NAT rebindings
+                // (each of which retires the superseded path's CID) would
+                // eventually exhaust it.
+                self.events.add(Event::ScidToAdvertise(1));
             }
 
             Frame::PathChallenge { data } => {
@@ -883,6 +890,30 @@ impl Connection {
                     if let Some(ref mut scheduler) = self.multipath_scheduler {
                         scheduler.on_path_updated(&mut self.paths, PathEvent::Validated(path_id));
                     }
+                    // NAT rebinding completed: this path supersedes the one
+                    // its CID previously lived on. Retire the stale path
+                    // (deactivate + free its DCID) so schedulers stop
+                    // feeding the dead address and repeated rebindings can't
+                    // exhaust max_paths or the CID pool.
+                    let old_pid = self
+                        .paths
+                        .get_mut(path_id)
+                        .ok()
+                        .and_then(|p| p.migrated_from.take());
+                    if let Some(old_pid) = old_pid {
+                        if old_pid != path_id {
+                            if let Ok(old) = self.paths.get_mut(old_pid) {
+                                if let Some(seq) = old.dcid_seq.take() {
+                                    self.cids.mark_dcid_to_retire(seq, true);
+                                }
+                            }
+                            self.paths.retire_path(old_pid);
+                            debug!(
+                                "{} path {} superseded by rebind path {}",
+                                self.trace_id, old_pid, path_id
+                            );
+                        }
+                    }
                 }
             }
 
@@ -3197,19 +3228,42 @@ impl Connection {
                     let space_id = self.spaces.add();
                     path.space_id = space_id;
                 }
-                Some(cid_pid) => {
-                    // Found NAT rebinding: If path migration occurs, the new path
-                    // will simply share the same packet number space with the
-                    // original path.
-                    path.space_id = self.paths.get(cid_pid)?.space_id;
-                }
+                Some(cid_pid) => match self.paths.get(cid_pid) {
+                    Ok(p) => {
+                        // Found NAT rebinding: If path migration occurs, the new path
+                        // will simply share the same packet number space with the
+                        // original path. Remember the stale path so it can be
+                        // retired once this one validates.
+                        path.space_id = p.space_id;
+                        path.migrated_from = Some(cid_pid);
+                    }
+                    // The bound path was already evicted — treat the packet
+                    // as opening a genuinely new path.
+                    Err(_) => path.space_id = self.spaces.add(),
+                },
             }
         }
 
         let pid = self.paths.insert_path(path)?;
         self.paths.get_mut(pid)?.update_trace_id(pid);
-        if cid_pid.is_none() {
-            self.cids.mark_scid_used(cid_seq, pid)?;
+        // Bind the packet's SCID to the new path — also on NAT rebinding, so
+        // a later rebinding of the same CID chains from the freshest path
+        // (and never dereferences an evicted one).
+        self.cids.mark_scid_used(cid_seq, pid)?;
+
+        // A server-created path needs a DCID before select_send_path will
+        // probe it: without one its PATH_CHALLENGE is never sent, it never
+        // validates, and user data keeps flowing to the stale address after
+        // a NAT rebinding.
+        if !self.cids.zero_length_dcid() && self.paths.get(pid)?.dcid_seq.is_none() {
+            if let Some(seq) = self.cids.lowest_unused_dcid_seq() {
+                self.paths.get_mut(pid)?.dcid_seq = Some(seq);
+                self.cids.mark_dcid_used(seq, pid)?;
+                debug!(
+                    "{} assign dcid seq {} to server-created path {}",
+                    self.trace_id, seq, pid
+                );
+            }
         }
         Ok(pid)
     }
@@ -3781,11 +3835,24 @@ impl Connection {
             None => return Ok(()),
         };
 
-        // TODO: check number of active path
+        // Refuse to abandon the last path still usable for sending.
+        let has_other = self
+            .paths
+            .iter()
+            .any(|(id, p)| id != pid && p.active());
+        if !has_other {
+            return Err(Error::InvalidOperation("last active path".into()));
+        }
 
-        // Mark the path as abandoned.
+        // Mark the path as abandoned, stop scheduling on it, free its DCID
+        // (RETIRE_CONNECTION_ID makes the peer issue a fresh one) and drop
+        // its 4-tuple mapping so the slot can be evicted and reused.
         let path = self.paths.get_mut(pid)?;
         path.is_abandon = true;
+        if let Some(seq) = path.dcid_seq.take() {
+            self.cids.mark_dcid_to_retire(seq, true);
+        }
+        self.paths.retire_path(pid);
         Ok(())
     }
 
@@ -7706,6 +7773,115 @@ pub(crate) mod tests {
         Ok(())
     }
 
+    /// Exchange one round of packets while emulating a NAT that maps the
+    /// client's `real` address to `public` (rewrites src on client->server
+    /// packets and dst on server->client packets).
+    fn natted_round(
+        test_pair: &mut TestPair,
+        real: SocketAddr,
+        public: SocketAddr,
+    ) -> Result<()> {
+        let mut packets = TestPair::conn_packets_out(&mut test_pair.client)?;
+        for (_, info) in packets.iter_mut() {
+            if info.src == real {
+                info.src = public;
+            }
+        }
+        TestPair::conn_packets_in(&mut test_pair.server, packets)?;
+
+        let mut packets = TestPair::conn_packets_out(&mut test_pair.server)?;
+        for (_, info) in packets.iter_mut() {
+            if info.dst == public {
+                info.dst = real;
+            }
+        }
+        TestPair::conn_packets_in(&mut test_pair.client, packets)
+    }
+
+    #[test]
+    fn conn_multipath_nat_rebind() -> Result<()> {
+        let mut client_config = TestPair::new_test_config(false)?;
+        client_config.set_cid_len(crate::MAX_CID_LEN);
+        client_config.enable_multipath(true);
+        client_config.set_multipath_algorithm(MultipathAlgorithm::MinRtt);
+        let mut server_config = TestPair::new_test_config(true)?;
+        server_config.set_cid_len(crate::MAX_CID_LEN);
+        server_config.enable_multipath(true);
+        server_config.set_multipath_algorithm(MultipathAlgorithm::MinRtt);
+
+        let mut test_pair = TestPair::new(&mut client_config, &mut server_config)?;
+        test_pair.handshake()?;
+        test_pair.advertise_new_cids()?;
+
+        let client_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 9443);
+        let server_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 443);
+        assert_eq!(test_pair.server.paths_iter().count(), 1);
+
+        // Two successive NAT rebindings: the server must migrate to each new
+        // 4-tuple and retire the superseded path.
+        for (round, port) in [9555u16, 9666].iter().enumerate() {
+            let public = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), *port);
+
+            // The second rebinding needs a fresh unused DCID: a live endpoint
+            // replenishes on ScidToAdvertise, but TestPair drives the
+            // connections directly, so add one manually. The NCID frame
+            // itself travels through the NAT inside the normal rounds below.
+            if round > 0 {
+                let scid = ConnectionId::random();
+                test_pair
+                    .client
+                    .cids
+                    .add_scid(scid, Some(100 + round as u128), true, None, true)?;
+            }
+
+            let mut converged = false;
+            for _ in 0..30 {
+                // Keep traffic flowing so the rebound tuple keeps appearing
+                // and the new path earns anti-amplification credit.
+                let _ = test_pair.client.ping(None);
+                natted_round(&mut test_pair, client_addr, public)?;
+
+                if let Ok(p) = test_pair.server.get_path(server_addr, public) {
+                    if p.active() && p.state() == PathState::Validated {
+                        converged = true;
+                        break;
+                    }
+                }
+            }
+            assert!(converged, "rebind to port {} never validated", port);
+
+            // The pre-rebinding tuple must be gone from the address table.
+            let stale = if round == 0 { client_addr.port() } else { 9555 };
+            let stale_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), stale);
+            assert!(test_pair.server.get_path(server_addr, stale_addr).is_err());
+
+            // Exactly one active server path (no dead-path accumulation).
+            let active = test_pair
+                .server
+                .paths
+                .iter()
+                .filter(|(_, p)| p.active())
+                .count();
+            assert_eq!(active, 1);
+
+            // The client's view never changes across a rebinding.
+            assert_eq!(test_pair.client.paths_iter().count(), 1);
+        }
+
+        // Data still flows end-to-end through the final rebound tuple.
+        let final_public = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 9666);
+        let data = Bytes::from_static(b"post-rebind payload");
+        assert_eq!(test_pair.client.stream_write(4, data.clone(), false), Ok(data.len()));
+        natted_round(&mut test_pair, client_addr, final_public)?;
+        let mut buf = vec![0; 128];
+        assert_eq!(
+            test_pair.server.stream_read(4, &mut buf)?,
+            (data.len(), false)
+        );
+        assert_eq!(&buf[..data.len()], &data[..]);
+        Ok(())
+    }
+
     #[test]
     #[cfg(feature = "qlog")]
     fn conn_write_qlog() -> Result<()> {

src/connection/path.rs

@@ -109,6 +109,11 @@ pub struct Path {
 
     /// Whether the path has been abandoned in MPQUIC mode.
     pub(super) is_abandon: bool,
+
+    /// The path this one superseded via NAT rebinding (same CID seen from a
+    /// new 4-tuple). Once this path validates, the stale path is retired so
+    /// repeated rebindings can't exhaust the path table.
+    pub(crate) migrated_from: Option<usize>,
 }
 
 impl Path {
@@ -154,6 +159,7 @@ impl Path {
             trace_id: trace_id.to_string(),
             space_id: SpaceId::Data,
             is_abandon: false,
+            migrated_from: None,
         }
     }
 
@@ -300,8 +306,18 @@ impl Path {
     }
 
     /// Whether PATH_CHALLENGE or PATH_RESPONSE should be sent on the path.
+    ///
+    /// The anti-amplification limit only applies while the peer's address is
+    /// unverified, mirroring inc/dec/cmp_anti_ampl_limit: once the first
+    /// PATH_RESPONSE arrives the limit is frozen, and consulting the frozen
+    /// value here would deadlock validation of a path that was opened by a
+    /// small packet (e.g. a NAT rebinding) — the path gets stuck in
+    /// ValidatingMTU because the padded challenge is never allowed out.
     pub(super) fn need_send_validation_frames(&self, is_server: bool) -> bool {
-        if is_server && self.anti_ampl_limit < MIN_PATH_PROBE_SIZE {
+        if is_server
+            && !self.verified_peer_address
+            && self.anti_ampl_limit < MIN_PATH_PROBE_SIZE
+        {
             return false;
         }
 
@@ -314,7 +330,10 @@ impl Path {
         if self.validated() {
             return false;
         }
-        if is_server && self.anti_ampl_limit <= self.recovery.max_datagram_size {
+        if is_server
+            && !self.verified_peer_address
+            && self.anti_ampl_limit <= self.recovery.max_datagram_size
+        {
             return false;
         }
         true
@@ -516,6 +535,24 @@ impl PathMap {
         Ok(pid)
     }
 
+    /// Retire a path: deactivate it and drop its 4-tuple mapping so the same
+    /// tuple can be re-created later (e.g. a NAT flapping back). The slab
+    /// entry stays until `insert_path` evicts it once it is `unused()` —
+    /// callers must clear `dcid_seq` themselves (the CID manager lives on
+    /// the connection).
+    pub fn retire_path(&mut self, path_id: usize) {
+        let key = match self.paths.get_mut(path_id) {
+            Some(p) => {
+                p.set_active(false);
+                (p.local_addr, p.remote_addr)
+            }
+            None => return,
+        };
+        if self.addrs.get(&key) == Some(&path_id) {
+            self.addrs.remove(&key);
+        }
+    }
+
     /// Return an immutable iterator over all existing paths.
     pub fn iter(&self) -> slab::Iter<Path> {
         self.paths.iter()