Send path validation responses to the correct remote

quinn-proto/src/connection/mod.rs

@@ -796,6 +796,38 @@ impl Connection {
                 break;
             }
 
+            // Send an off-path PATH_RESPONSE. Prioritized over on-path data to ensure that path
+            // validation can occur while the link is saturated.
+            if space_id == SpaceId::Data && num_datagrams == 1 {
+                if let Some((token, remote)) = self.path_responses.pop_off_path(&self.path.remote) {
+                    // `unwrap` guaranteed to succeed because `builder_storage` was populated just
+                    // above.
+                    let mut builder = builder_storage.take().unwrap();
+                    trace!("PATH_RESPONSE {:08x} (off-path)", token);
+                    buf.write(frame::Type::PATH_RESPONSE);
+                    buf.write(token);
+                    self.stats.frame_tx.path_response += 1;
+                    builder.pad_to(MIN_INITIAL_SIZE);
+                    builder.finish_and_track(
+                        now,
+                        self,
+                        Some(SentFrames {
+                            non_retransmits: true,
+                            ..SentFrames::default()
+                        }),
+                        buf,
+                    );
+                    self.stats.udp_tx.on_sent(1, buf.len());
+                    return Some(Transmit {
+                        destination: remote,
+                        size: buf.len(),
+                        ecn: None,
+                        segment_size: None,
+                        src_ip: self.local_ip,
+                    });
+                }
+            }
+
             let sent = self.populate_packet(
                 now,
                 space_id,
@@ -2583,7 +2615,7 @@ impl Connection {
                     close = Some(reason);
                 }
                 Frame::PathChallenge(token) => {
-                    self.path_responses.push(number, token);
+                    self.path_responses.push(number, token, remote);
                     if remote == self.path.remote {
                         // PATH_CHALLENGE on active path, possible off-path packet forwarding
                         // attack. Send a non-probing packet to recover the active path.
@@ -3003,7 +3035,7 @@ impl Connection {
 
         // PATH_RESPONSE
         if buf.len() + 9 < max_size && space_id == SpaceId::Data {
-            if let Some(token) = self.path_responses.pop() {
+            if let Some(token) = self.path_responses.pop_on_path(&self.path.remote) {
                 sent.non_retransmits = true;
                 sent.requires_padding = true;
                 trace!("PATH_RESPONSE {:08x}", token);

quinn-proto/src/connection/paths.rs

@@ -1,5 +1,7 @@
 use std::{cmp, net::SocketAddr, time::Duration, time::Instant};
 
+use tracing::trace;
+
 use super::{mtud::MtuDiscovery, pacing::Pacer};
 use crate::{config::MtuDiscoveryConfig, congestion, packet::SpaceId, TIMER_GRANULARITY};
 
@@ -167,27 +169,67 @@ impl RttEstimator {
 
 #[derive(Default)]
 pub(crate) struct PathResponses {
-    pending: Option<PathResponse>,
+    pending: Vec<PathResponse>,
 }
 
 impl PathResponses {
-    pub(crate) fn push(&mut self, packet: u64, token: u64) {
-        if self.pending.as_ref().map_or(true, |x| x.packet <= packet) {
-            self.pending = Some(PathResponse { packet, token });
+    pub(crate) fn push(&mut self, packet: u64, token: u64, remote: SocketAddr) {
+        /// Arbitrary permissive limit to prevent abuse
+        const MAX_PATH_RESPONSES: usize = 16;
+        let response = PathResponse {
+            packet,
+            token,
+            remote,
+        };
+        let existing = self.pending.iter_mut().find(|x| x.remote == remote);
+        if let Some(existing) = existing {
+            // Update a queued response
+            if existing.packet <= packet {
+                *existing = response;
+            }
+            return;
+        }
+        if self.pending.len() < MAX_PATH_RESPONSES {
+            self.pending.push(response);
+        } else {
+            // We don't expect to ever hit this with well-behaved peers, so we don't bother dropping
+            // older challenges.
+            trace!("ignoring excessive PATH_CHALLENGE");
         }
     }
 
-    pub(crate) fn pop(&mut self) -> Option<u64> {
-        Some(self.pending.take()?.token)
+    pub(crate) fn pop_off_path(&mut self, remote: &SocketAddr) -> Option<(u64, SocketAddr)> {
+        let response = *self.pending.last()?;
+        if response.remote == *remote {
+            // We don't bother searching further because we expect that the on-path response will
+            // get drained in the immediate future by a call to `pop_on_path`
+            return None;
+        }
+        self.pending.pop();
+        Some((response.token, response.remote))
+    }
+
+    pub(crate) fn pop_on_path(&mut self, remote: &SocketAddr) -> Option<u64> {
+        let response = *self.pending.last()?;
+        if response.remote != *remote {
+            // We don't bother searching further because we expect that the off-path response will
+            // get drained in the immediate future by a call to `pop_off_path`
+            return None;
+        }
+        self.pending.pop();
+        Some(response.token)
     }
 
     pub(crate) fn is_empty(&self) -> bool {
-        self.pending.is_none()
+        self.pending.is_empty()
     }
 }
 
+#[derive(Copy, Clone)]
 struct PathResponse {
     /// The packet number the corresponding PATH_CHALLENGE was received in
     packet: u64,
     token: u64,
+    /// The address the corresponding PATH_CHALLENGE was received from
+    remote: SocketAddr,
 }