Skip to content

Commit

Permalink
Send path validation responses to the correct remote
Browse files Browse the repository at this point in the history
  • Loading branch information
Ralith committed Jan 24, 2024
1 parent 2d9a648 commit c1ff4a6
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 9 deletions.
35 changes: 33 additions & 2 deletions quinn-proto/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,37 @@ impl Connection {
break;
}

// Send an off-path PATH_RESPONSE
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,
Expand Down Expand Up @@ -2583,7 +2614,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.
Expand Down Expand Up @@ -3003,7 +3034,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);
Expand Down
52 changes: 45 additions & 7 deletions quinn-proto/src/connection/paths.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -167,27 +169,63 @@ 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 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");
}
}

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 {
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 {
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,
}

0 comments on commit c1ff4a6

Please sign in to comment.