Skip to content

Commit

Permalink
Allow packets with impossible CIDs to be ignored rather than reset
Browse files Browse the repository at this point in the history
  • Loading branch information
Ralith committed Apr 6, 2024
1 parent 7e8e0ad commit 0b81fe1
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 2 deletions.
12 changes: 12 additions & 0 deletions quinn-proto/src/cid_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ pub trait ConnectionIdGenerator: Send {
/// issuer) to correlate them with other connection IDs for the same
/// connection.
fn generate_cid(&mut self) -> ConnectionId;

/// Quickly determine whether `cid` could have been generated by this generator
///
/// False positives are permitted, but increase the cost of handling invalid packets.
fn validate(&self, _cid: &ConnectionId) -> Result<(), InvalidCid> {
Ok(())
}

/// Returns the length of a CID for connections created by this generator
fn cid_len(&self) -> usize;
/// Returns the lifetime of generated Connection IDs
Expand All @@ -22,6 +30,10 @@ pub trait ConnectionIdGenerator: Send {
fn cid_lifetime(&self) -> Option<Duration>;
}

/// The connection ID was not recognized by the [`ConnectionIdGenerator`]
#[derive(Debug, Copy, Clone)]
pub struct InvalidCid;

/// Generates purely random connection IDs of a certain length
#[derive(Debug, Clone, Copy)]
pub struct RandomConnectionIdGenerator {
Expand Down
4 changes: 3 additions & 1 deletion quinn-proto/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,9 @@ impl EndpointConfig {
///
/// Defaults to 20ms. Limits the impact of attacks which flood an endpoint with garbage packets,
/// e.g. [ISAKMP/IKE amplification]. Larger values provide a stronger defense, but may delay
/// detection of some error conditions by clients.
/// detection of some error conditions by clients. Using a [`ConnectionIdGenerator`] with a low
/// rate of false positives in [`validate`](ConnectionIdGenerator::validate) reduces the risk
/// incurred by a small minimum reset interval.
///
/// [ISAKMP/IKE
/// amplification]: https://bughunters.google.com/blog/5960150648750080/preventing-cross-service-udp-loops-in-quic#isakmp-ike-amplification-vs-quic
Expand Down
11 changes: 11 additions & 0 deletions quinn-proto/src/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,17 @@ impl Endpoint {
// If we got this far, we're a server receiving a seemingly valid packet for an unknown
// connection. Send a stateless reset if possible.
//

if !first_decode.is_initial()
&& self
.local_cid_generator
.validate(first_decode.dst_cid())
.is_err()
{
debug!("dropping packet with invalid CID");
return None;
}

if !dst_cid.is_empty() {
return self
.stateless_reset(now, datagram_len, addresses, dst_cid, buf)
Expand Down
2 changes: 1 addition & 1 deletion quinn-proto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ pub use crate::transport_error::{Code as TransportErrorCode, Error as TransportE
pub mod congestion;

mod cid_generator;
pub use crate::cid_generator::{ConnectionIdGenerator, RandomConnectionIdGenerator};
pub use crate::cid_generator::{ConnectionIdGenerator, InvalidCid, RandomConnectionIdGenerator};

mod token;
use token::{ResetToken, RetryToken};
Expand Down

0 comments on commit 0b81fe1

Please sign in to comment.