Skip to content

Commit

Permalink
Allow server to use NEW_TOKEN frames
Browse files Browse the repository at this point in the history
When a path becomes validated, the server may send the client NEW_TOKEN
frames. These may cause an Incoming to be validated.

- Adds TokenPayload::Validation variant
- Adds relevant configuration to ServerConfig
- Adds `TokenLog` object to server to mitigate token reuse

As of this commit, no implementation of TokenLog is provided, and it
defaults to None.
  • Loading branch information
gretchenfrage committed Dec 24, 2024
1 parent a9b978d commit 97d0cb9
Show file tree
Hide file tree
Showing 7 changed files with 268 additions and 21 deletions.
44 changes: 43 additions & 1 deletion quinn-proto/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::{
cid_generator::{ConnectionIdGenerator, HashedConnectionIdGenerator},
crypto::{self, HandshakeTokenKey, HmacKey},
shared::ConnectionId,
Duration, RandomConnectionIdGenerator, SystemTime, VarInt, VarIntBoundsExceeded,
Duration, RandomConnectionIdGenerator, SystemTime, TokenLog, VarInt, VarIntBoundsExceeded,
DEFAULT_SUPPORTED_VERSIONS, MAX_CID_SIZE,
};

Expand Down Expand Up @@ -209,6 +209,10 @@ pub struct ServerConfig {
/// rebinding. Enabled by default.
pub(crate) migration: bool,

pub(crate) validation_token_lifetime: Duration,
pub(crate) validation_token_log: Option<Arc<dyn TokenLog>>,
pub(crate) validation_tokens_sent: u32,

pub(crate) preferred_address_v4: Option<SocketAddrV4>,
pub(crate) preferred_address_v6: Option<SocketAddrV6>,

Expand All @@ -234,6 +238,10 @@ impl ServerConfig {

migration: true,

validation_token_lifetime: Duration::from_secs(2 * 7 * 24 * 60 * 60),
validation_token_log: None,
validation_tokens_sent: 0,

preferred_address_v4: None,
preferred_address_v6: None,

Expand Down Expand Up @@ -274,6 +282,37 @@ impl ServerConfig {
self
}

/// Duration after an address validation token was issued for which it's considered valid
///
/// This refers only to tokens sent in NEW_TOKEN frames, in contrast to retry tokens.
///
/// Defaults to 2 weeks.
pub fn validation_token_lifetime(&mut self, value: Duration) -> &mut Self {
self.validation_token_lifetime = value;
self
}

/// Set a custom [`TokenLog`]
///
/// Setting this to `None` makes the server ignore all address validation tokens (that is,
/// tokens originating from NEW_TOKEN frames--retry tokens may still be accepted).
///
/// Defaults to `None`.
pub fn validation_token_log(&mut self, log: Option<Arc<dyn TokenLog>>) -> &mut Self {
self.validation_token_log = log;
self
}

/// Number of address validation tokens sent to a client when its path is validated
///
/// This refers only to tokens sent in NEW_TOKEN frames, in contrast to retry tokens.
///
/// Defaults to 0.
pub fn validation_tokens_sent(&mut self, value: u32) -> &mut Self {
self.validation_tokens_sent = value;
self
}

/// The preferred IPv4 address that will be communicated to clients during handshaking
///
/// If the client is able to reach this address, it will switch to it.
Expand Down Expand Up @@ -392,6 +431,9 @@ impl fmt::Debug for ServerConfig {
// crypto not debug
// token not debug
.field("retry_token_lifetime", &self.retry_token_lifetime)
.field("validation_token_lifetime", &self.validation_token_lifetime)
// validation_token_log not debug
.field("validation_tokens_sent", &self.validation_tokens_sent)
.field("migration", &self.migration)
.field("preferred_address_v4", &self.preferred_address_v4)
.field("preferred_address_v6", &self.preferred_address_v6)
Expand Down
56 changes: 54 additions & 2 deletions quinn-proto/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use crate::{
ConnectionEvent, ConnectionEventInner, ConnectionId, DatagramConnectionEvent, EcnCodepoint,
EndpointEvent, EndpointEventInner,
},
token::ResetToken,
token::{ResetToken, Token, TokenPayload},
transport_parameters::TransportParameters,
Dir, Duration, EndpointConfig, Frame, Instant, Side, StreamId, Transmit, TransportError,
TransportErrorCode, VarInt, INITIAL_MTU, MAX_CID_SIZE, MAX_STREAM_COUNT, MIN_INITIAL_SIZE,
Expand Down Expand Up @@ -351,6 +351,9 @@ impl Connection {
stats: ConnectionStats::default(),
version,
};
if path_validated {
this.on_path_validated();
}
if side.is_client() {
// Kick off the connection
this.write_crypto();
Expand Down Expand Up @@ -2435,7 +2438,7 @@ impl Connection {
);
return Ok(());
}
self.path.validated = true;
self.on_path_validated();

self.process_early_payload(now, packet)?;
if self.state.is_closed() {
Expand Down Expand Up @@ -3242,6 +3245,43 @@ impl Connection {
self.datagrams.send_blocked = false;
}

// NEW_TOKEN
while let Some(remote_addr) = space.pending.new_tokens.pop() {
debug_assert_eq!(space_id, SpaceId::Data);
let ConnectionSide::Server { ref server_config } = self.side else {
panic!("NEW_TOKEN frames should not be enqueued by clients");
};

if remote_addr != self.path.remote {
// NEW_TOKEN frames contain tokens bound to a client's IP address, and are only
// useful if used from the same IP address. Thus, we abandon enqueued NEW_TOKEN
// frames upon an path change. Instead, when the new path becomes validated,
// NEW_TOKEN frames may be enqueued for the new path instead.
continue;
}

let payload = TokenPayload::Validation {
ip: remote_addr.ip(),
issued: server_config.time_source.now(),
};
let token = Token::new(payload, &mut self.rng).encode(&*server_config.token_key);
let new_token = NewToken {
token: token.into(),
};

if buf.len() + new_token.size() >= max_size {
space.pending.new_tokens.push(remote_addr);
break;
}

new_token.encode(buf);
sent.retransmits
.get_or_create()
.new_tokens
.push(remote_addr);
self.stats.frame_tx.new_token += 1;
}

// STREAM
if space_id == SpaceId::Data {
sent.stream_frames =
Expand Down Expand Up @@ -3603,6 +3643,18 @@ impl Connection {
// but that would needlessly prevent sending datagrams during 0-RTT.
key.map_or(16, |x| x.tag_len())
}

/// Mark the path as validated, and enqueue NEW_TOKEN frames to be sent as appropriate
fn on_path_validated(&mut self) {
self.path.validated = true;
if let ConnectionSide::Server { server_config } = &self.side {
let new_tokens = &mut self.spaces[SpaceId::Data as usize].pending.new_tokens;
new_tokens.clear();
for _ in 0..server_config.validation_tokens_sent {
new_tokens.push(self.path.remote);
}
}
}
}

impl fmt::Debug for Connection {
Expand Down
21 changes: 20 additions & 1 deletion quinn-proto/src/connection/spaces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use tracing::trace;
use super::assembler::Assembler;
use crate::{
connection::StreamsState, crypto::Keys, frame, packet::SpaceId, range_set::ArrayRangeSet,
shared::IssuedCid, Dir, Duration, Instant, StreamId, TransportError, VarInt,
shared::IssuedCid, Dir, Duration, Instant, SocketAddr, StreamId, TransportError, VarInt,
};

pub(super) struct PacketSpace {
Expand Down Expand Up @@ -309,6 +309,23 @@ pub struct Retransmits {
pub(super) retire_cids: Vec<u64>,
pub(super) ack_frequency: bool,
pub(super) handshake_done: bool,
/// For each enqueued NEW_TOKEN frame, a copy of the path's remote address
///
/// There are 2 reasons this is unusual:
///
/// - If the path changes, NEW_TOKEN frames bound for the old path are not retransmitted on the
/// new path. That is why this field stores the remote address: so that ones for old paths
/// can be filtered out.
/// - If a token is lost, a new randomly generated token is re-transmitted, rather than the
/// original. This is so that if both transmissions are received, the client won't risk
/// sending the same token twice. That is why this field does _not_ store any actual token.
///
/// It is true that a QUIC endpoint will only want to effectively have NEW_TOKEN frames
/// enqueued for its current path at a given point in time. Based on that, we could conceivably
/// change this from a vector to an `Option<(SocketAddr, usize)>` or just a `usize` or
/// something. However, due to the architecture of Quinn, it is considerably simpler to not do
/// that; consider what such a change would mean for implementing `BitOrAssign` on Self.
pub(super) new_tokens: Vec<SocketAddr>,
}

impl Retransmits {
Expand All @@ -326,6 +343,7 @@ impl Retransmits {
&& self.retire_cids.is_empty()
&& !self.ack_frequency
&& !self.handshake_done
&& self.new_tokens.is_empty()
}
}

Expand All @@ -347,6 +365,7 @@ impl ::std::ops::BitOrAssign for Retransmits {
self.retire_cids.extend(rhs.retire_cids);
self.ack_frequency |= rhs.ack_frequency;
self.handshake_done |= rhs.handshake_done;
self.new_tokens.extend_from_slice(&rhs.new_tokens);
}
}

Expand Down
12 changes: 12 additions & 0 deletions quinn-proto/src/frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,18 @@ pub(crate) struct NewToken {
pub(crate) token: Bytes,
}

impl NewToken {
pub(crate) fn encode<W: BufMut>(&self, out: &mut W) {
out.write(FrameType::NEW_TOKEN);
out.write_var(self.token.len() as u64);
out.put_slice(&self.token);
}

pub(crate) fn size(&self) -> usize {
1 + VarInt::from_u64(self.token.len() as u64).unwrap().size() + self.token.len()
}
}

pub(crate) struct Iter {
// TODO: ditch io::Cursor after bytes 0.5
bytes: io::Cursor<Bytes>,
Expand Down
1 change: 1 addition & 0 deletions quinn-proto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ pub use crate::cid_generator::{

mod token;
use token::ResetToken;
pub use token::{TokenLog, TokenReuseError};

#[cfg(feature = "arbitrary")]
use arbitrary::Arbitrary;
Expand Down
Loading

0 comments on commit 97d0cb9

Please sign in to comment.