Send path validation responses to the correct remote

quinn-proto/src/connection/mod.rs

@@ -197,7 +197,8 @@ pub struct Connection {
     //
     // Queued non-retransmittable 1-RTT data
     //
-    path_response: Option<PathResponse>,
+    /// Responses to PATH_CHALLENGE frames, and the addresses to send them to
+    path_responses: PathResponses,
     close: bool,
 
     //
@@ -336,7 +337,7 @@ impl Connection {
             #[cfg(not(test))]
             packet_number_filter: PacketNumberFilter::new(&mut rng),
 
-            path_response: None,
+            path_responses: PathResponses::default(),
             close: false,
 
             ack_frequency: AckFrequencyState::new(get_max_ack_delay(
@@ -795,6 +796,37 @@ impl Connection {
                 break;
             }
 
+            // Send an off-path PATH_RESPONSE. LIFO since old challenges might have been forgotten by the sender.
+            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,
@@ -2582,16 +2614,7 @@ impl Connection {
                     close = Some(reason);
                 }
                 Frame::PathChallenge(token) => {
-                    if self
-                        .path_response
-                        .as_ref()
-                        .map_or(true, |x| x.packet <= number)
-                    {
-                        self.path_response = Some(PathResponse {
-                            packet: 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.
@@ -3011,12 +3034,12 @@ impl Connection {
 
         // PATH_RESPONSE
         if buf.len() + 9 < max_size && space_id == SpaceId::Data {
-            if let Some(response) = self.path_response.take() {
+            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}", response.token);
+                trace!("PATH_RESPONSE {:08x}", token);
                 buf.write(frame::Type::PATH_RESPONSE);
-                buf.write(response.token);
+                buf.write(token);
                 self.stats.frame_tx.path_response += 1;
             }
         }
@@ -3414,7 +3437,7 @@ impl Connection {
                 .prev_path
                 .as_ref()
                 .map_or(false, |x| x.challenge_pending)
-            || self.path_response.is_some()
+            || !self.path_responses.pending.is_empty()
             || !self.datagrams.outgoing.is_empty()
     }
 
@@ -3612,10 +3635,63 @@ pub enum Event {
     DatagramReceived,
 }
 
+#[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,
+}
+
+#[derive(Default)]
+struct PathResponses {
+    pending: Vec<PathResponse>,
+}
+
+impl PathResponses {
+    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 an 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");
+        }
+    }
+
+    fn pop_off_path(&mut self, remote: &SocketAddr) -> Option<(u64, SocketAddr)> {
+        let response = *self.pending.last()?;
+        if response.remote == *remote {
+            return None;
+        }
+        self.pending.pop();
+        Some((response.token, response.remote))
+    }
+
+    fn pop_on_path(&mut self, remote: &SocketAddr) -> Option<u64> {
+        let response = *self.pending.last()?;
+        if response.remote != *remote {
+            return None;
+        }
+        self.pending.pop();
+        Some(response.token)
+    }
 }
 
 fn instant_saturating_sub(x: Instant, y: Instant) -> Duration {