Skip to content

Commit

Permalink
feat(s2n-quic): allow application to configure the anti_amplification…
Browse files Browse the repository at this point in the history
…_multiplier (#2229)

* feat(s2n-quic): configurable amplification limited multiplier

* test the limits get applied

* clippy

* comments
  • Loading branch information
toidiu authored Jun 6, 2024
1 parent 79e5d8e commit 20977ee
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 5 deletions.
2 changes: 2 additions & 0 deletions quic/s2n-quic-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ probe-tracing = ["tracing"]
state-tracing = ["tracing"]
# This feature enables support for third party congestion controller implementations
unstable-congestion-controller = []
# This feature enables the use of unstable connection limits
unstable-limits = []
usdt = ["dep:probe"]

[dependencies]
Expand Down
26 changes: 26 additions & 0 deletions quic/s2n-quic-core/src/connection/limits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ const MAX_HANDSHAKE_DURATION_DEFAULT: Duration = Duration::from_secs(10);
//# middleboxes from losing state for UDP flows [GATEWAY].
const MAX_KEEP_ALIVE_PERIOD_DEFAULT: Duration = Duration::from_secs(30);

//= https://www.rfc-editor.org/rfc/rfc9000#section-8.1
//# Prior to validating the client address, servers MUST NOT send more
//# than three times as many bytes as the number of bytes they have
//# received.
pub const ANTI_AMPLIFICATION_MULTIPLIER: u8 = 3;

#[non_exhaustive]
#[derive(Debug)]
pub struct ConnectionInfo<'a> {
Expand Down Expand Up @@ -67,6 +73,7 @@ pub struct Limits {
pub(crate) max_datagram_frame_size: MaxDatagramFrameSize,
pub(crate) initial_round_trip_time: Duration,
pub(crate) migration_support: MigrationSupport,
pub(crate) anti_amplification_multiplier: u8,
}

impl Default for Limits {
Expand Down Expand Up @@ -112,6 +119,7 @@ impl Limits {
max_datagram_frame_size: MaxDatagramFrameSize::DEFAULT,
initial_round_trip_time: recovery::DEFAULT_INITIAL_RTT,
migration_support: MigrationSupport::RECOMMENDED,
anti_amplification_multiplier: ANTI_AMPLIFICATION_MULTIPLIER,
}
}

Expand Down Expand Up @@ -274,6 +282,18 @@ impl Limits {
Ok(self)
}

#[cfg(feature = "unstable-limits")]
setter!(
/// Limit how many bytes the Server sends prior to address validation (default: 3)
///
/// Prior to validating the client address, servers will not send more
/// than `anti_amplification_multiplier` times as many bytes as the
/// number of bytes it has received.
with_anti_amplification_multiplier,
anti_amplification_multiplier,
u8
);

// internal APIs

#[doc(hidden)]
Expand Down Expand Up @@ -358,6 +378,12 @@ impl Limits {
pub fn active_migration_enabled(&self) -> bool {
matches!(self.migration_support, MigrationSupport::Enabled)
}

#[doc(hidden)]
#[inline]
pub fn anti_amplification_multiplier(&self) -> u8 {
self.anti_amplification_multiplier
}
}

/// Creates limits for a given connection
Expand Down
1 change: 1 addition & 0 deletions quic/s2n-quic-transport/src/connection/connection_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,7 @@ impl<Config: endpoint::Config> connection::Trait for ConnectionImpl<Config> {
parameters.congestion_controller,
peer_validated,
parameters.mtu_config,
parameters.limits.anti_amplification_multiplier(),
);

let path_manager = path::Manager::new(initial_path, parameters.peer_id_registry);
Expand Down
1 change: 1 addition & 0 deletions quic/s2n-quic-transport/src/path/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,7 @@ impl<Config: endpoint::Config> Manager<Config> {
cc,
true,
mtu_config,
limits.anti_amplification_multiplier(),
);

let amplification_outcome = path.on_bytes_received(datagram.payload_len);
Expand Down
19 changes: 19 additions & 0 deletions quic/s2n-quic-transport/src/path/manager/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::{
};
use core::time::Duration;
use s2n_quic_core::{
connection::limits::ANTI_AMPLIFICATION_MULTIPLIER,
event::testing::Publisher,
inet::{DatagramInfo, ExplicitCongestionNotification, SocketAddress},
path::{migration, RemoteAddress},
Expand Down Expand Up @@ -60,6 +61,7 @@ fn get_path_by_address_test() {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
);

let second_conn_id = connection::PeerId::try_from_bytes(&[5, 4, 3, 2, 1]).unwrap();
Expand All @@ -71,6 +73,7 @@ fn get_path_by_address_test() {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
);

let mut manager = manager_server(first_path.clone());
Expand Down Expand Up @@ -102,6 +105,7 @@ fn test_invalid_path_fallback() {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
);
// simulate receiving a handshake packet to force path validation
first_path.on_handshake_packet();
Expand All @@ -118,6 +122,7 @@ fn test_invalid_path_fallback() {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
);
second_path.set_challenge(challenge);

Expand Down Expand Up @@ -501,6 +506,7 @@ fn silently_return_when_there_is_no_valid_path() {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
);
first_path.set_challenge(challenge);
let mut manager = manager_server(first_path);
Expand Down Expand Up @@ -708,6 +714,7 @@ fn test_adding_new_path() {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
);
let mut manager = manager_server(first_path);

Expand Down Expand Up @@ -771,6 +778,7 @@ fn do_not_add_new_path_if_handshake_not_confirmed() {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
);
let mut manager = manager_server(first_path);

Expand Down Expand Up @@ -833,6 +841,7 @@ fn do_not_add_new_path_if_client() {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
);
let mut manager = manager_client(first_path);
let mut publisher = Publisher::snapshot();
Expand Down Expand Up @@ -888,6 +897,7 @@ fn switch_destination_connection_id_after_first_server_response() {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
);
let mut manager = manager_client(zero_path);
assert_eq!(manager[zero_path_id].peer_connection_id, initial_cid);
Expand Down Expand Up @@ -926,6 +936,7 @@ fn limit_number_of_connection_migrations() {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
);
let mut manager = manager_server(first_path);
let mut total_paths = 1;
Expand Down Expand Up @@ -985,6 +996,7 @@ fn active_connection_migration_disabled() {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
);
let mut manager = manager_server(first_path);
// Give the path manager some new CIDs so it's able to use one for an active migration.
Expand Down Expand Up @@ -1092,6 +1104,7 @@ fn connection_migration_challenge_behavior() {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
);
let mut manager = manager_server(first_path);

Expand Down Expand Up @@ -1187,6 +1200,7 @@ fn connection_migration_use_max_ack_delay_from_active_path() {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
);
let mut manager = manager_server(first_path);

Expand Down Expand Up @@ -1267,6 +1281,7 @@ fn connection_migration_new_path_abandon_timer() {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
);
let mut manager = manager_server(first_path);

Expand Down Expand Up @@ -1389,6 +1404,7 @@ fn stop_using_a_retired_connection_id() {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
);
let mut manager = manager_server(first_path);

Expand Down Expand Up @@ -1485,6 +1501,7 @@ fn pending_paths_should_return_paths_pending_validation() {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
);
let expected_response_data = [0; 8];
third_path.on_path_challenge(&expected_response_data);
Expand Down Expand Up @@ -1550,6 +1567,7 @@ fn temporary_until_authenticated() {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
);
let mut manager = manager_server(first_path);

Expand Down Expand Up @@ -1936,6 +1954,7 @@ pub fn helper_path(peer_id: connection::PeerId) -> ServerPath {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
)
}

Expand Down
46 changes: 42 additions & 4 deletions quic/s2n-quic-transport/src/path/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ pub struct Path<Config: endpoint::Config> {

/// True if the path is currently active
is_active: bool,
anti_amplification_multiplier: u8,
}

impl<Config: endpoint::Config> Clone for Path<Config> {
Expand All @@ -97,6 +98,7 @@ impl<Config: endpoint::Config> Clone for Path<Config> {
response_data: self.response_data,
activated: self.activated,
is_active: self.is_active,
anti_amplification_multiplier: self.anti_amplification_multiplier,
}
}
}
Expand All @@ -113,6 +115,7 @@ impl<Config: endpoint::Config> Path<Config> {
congestion_controller: <Config::CongestionControllerEndpoint as congestion_controller::Endpoint>::CongestionController,
peer_validated: bool,
mtu_config: mtu::Config,
anti_amplification_multiplier: u8,
) -> Path<Config> {
let state = match Config::ENDPOINT_TYPE {
Type::Server => {
Expand Down Expand Up @@ -144,6 +147,7 @@ impl<Config: endpoint::Config> Path<Config> {
response_data: None,
activated: false,
is_active: false,
anti_amplification_multiplier,
}
}

Expand Down Expand Up @@ -212,9 +216,9 @@ impl<Config: endpoint::Config> Path<Config> {
//# Prior to validating the client address, servers MUST NOT send more
//# than three times as many bytes as the number of bytes they have
//# received.
//
if let State::AmplificationLimited { tx_allowance } = &mut self.state {
*tx_allowance += bytes.saturating_mul(3) as u32;
*tx_allowance +=
bytes.saturating_mul(self.anti_amplification_multiplier as usize) as u32;
}

let unblocked = was_at_amplification_limit && !self.at_amplification_limit();
Expand Down Expand Up @@ -595,7 +599,10 @@ impl<Config: endpoint::Config> transmission::interest::Provider for Path<Config>
pub mod testing {
use crate::{endpoint, path::Path};
use core::time::Duration;
use s2n_quic_core::{connection, path::mtu, recovery::RttEstimator};
use s2n_quic_core::{
connection, connection::limits::ANTI_AMPLIFICATION_MULTIPLIER, path::mtu,
recovery::RttEstimator,
};

pub fn helper_path_server() -> Path<endpoint::testing::Server> {
Path::new(
Expand All @@ -606,6 +613,7 @@ pub mod testing {
Default::default(),
true,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
)
}

Expand All @@ -618,6 +626,7 @@ pub mod testing {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
)
}
}
Expand All @@ -633,7 +642,9 @@ mod tests {
};
use core::time::Duration;
use s2n_quic_core::{
connection, endpoint,
connection,
connection::limits::ANTI_AMPLIFICATION_MULTIPLIER,
endpoint,
event::testing::Publisher,
path::MINIMUM_MAX_DATAGRAM_SIZE,
recovery::{CongestionController, RttEstimator},
Expand All @@ -643,6 +654,32 @@ mod tests {

type Path = super::Path<Config>;

#[test]
fn custom_anti_amplification_multiplier() {
let bytes_received = 100;

// allowance should increase based on the default anti_amplification_multiplier
let mut default_path = testing::helper_path_server();
let _ = default_path.on_bytes_received(bytes_received);
let allowance = match default_path.state {
path::State::AmplificationLimited { tx_allowance } => tx_allowance,
_ => unreachable!("path is amplification limited"),
};
let expected = ANTI_AMPLIFICATION_MULTIPLIER as u32 * bytes_received as u32;
assert_eq!(allowance, Counter::new(expected));

// allowance should increase based on the custom anti_amplification_multiplier
let mut custom_path = testing::helper_path_server();
custom_path.anti_amplification_multiplier = ANTI_AMPLIFICATION_MULTIPLIER + 10;
let _ = custom_path.on_bytes_received(bytes_received);
let allowance = match custom_path.state {
path::State::AmplificationLimited { tx_allowance } => tx_allowance,
_ => unreachable!("path is amplification limited"),
};
let expected = (ANTI_AMPLIFICATION_MULTIPLIER + 10) as u32 * bytes_received as u32;
assert_eq!(allowance, Counter::new(expected));
}

#[test]
fn response_data_should_only_be_sent_once() {
// Setup:
Expand Down Expand Up @@ -1123,6 +1160,7 @@ mod tests {
Default::default(),
false,
mtu::Config::default(),
ANTI_AMPLIFICATION_MULTIPLIER,
);
let now = NoopClock.get_time();
let random = &mut random::testing::Generator::default();
Expand Down
Loading

0 comments on commit 20977ee

Please sign in to comment.