feat(s2n-quic): allow disabling active connection migration support

.github/workflows/ci.yml

@@ -15,7 +15,7 @@ env:
  RUST_BACKTRACE: 1
  # Pin the nightly toolchain to prevent breakage.
  # This should be occasionally updated.
- RUST_NIGHTLY_TOOLCHAIN: nightly-2024-03-21
+ RUST_NIGHTLY_TOOLCHAIN: nightly-2024-04-16
  CDN: https://dnglbrstg7yg.cloudfront.net
  # enable unstable features for testing
  S2N_UNSTABLE_CRYPTO_OPT_TX: 100

quic/s2n-quic-core/src/connection/limits.rs

@@ -9,7 +9,7 @@ use crate::{
  AckDelayExponent, ActiveConnectionIdLimit, InitialFlowControlLimits, InitialMaxData,
  InitialMaxStreamDataBidiLocal, InitialMaxStreamDataBidiRemote, InitialMaxStreamDataUni,
  InitialMaxStreamsBidi, InitialMaxStreamsUni, InitialStreamLimits, MaxAckDelay,
- MaxDatagramFrameSize, MaxIdleTimeout, TransportParameters,
+ MaxDatagramFrameSize, MaxIdleTimeout, MigrationSupport, TransportParameters,
  },
 };
 use core::time::Duration;
@@ -66,6 +66,7 @@ pub struct Limits {
  pub(crate) max_keep_alive_period: Duration,
  pub(crate) max_datagram_frame_size: MaxDatagramFrameSize,
  pub(crate) initial_round_trip_time: Duration,
+ pub(crate) migration_support: MigrationSupport,
 }
 
 impl Default for Limits {
@@ -110,6 +111,7 @@ impl Limits {
  max_keep_alive_period: MAX_KEEP_ALIVE_PERIOD_DEFAULT,
  max_datagram_frame_size: MaxDatagramFrameSize::DEFAULT,
  initial_round_trip_time: recovery::DEFAULT_INITIAL_RTT,
+ migration_support: MigrationSupport::RECOMMENDED,
  }
  }
 
@@ -236,6 +238,21 @@ impl Limits {
  Duration
  );
  setter!(with_max_keep_alive_period, max_keep_alive_period, Duration);
+ /// Sets whether active connection migration is supported for a server endpoint (default: true)
+ ///
+ /// If set to false, the `disable_active_migration` transport parameter will be sent to the
+ /// peer, and any attempt by the peer to perform an active connection migration will be ignored.
+ pub fn with_active_connection_migration(
+ mut self,
+ enabled: bool,
+ ) -> Result<Self, ValidationError> {
+ if enabled {
+ self.migration_support = MigrationSupport::Enabled
+ } else {
+ self.migration_support = MigrationSupport::Disabled
+ }
+ Ok(self)
+ }
 
  /// Sets the initial round trip time (RTT) for use in recovery mechanisms prior to
  /// measuring an actual RTT sample.
@@ -335,6 +352,12 @@ impl Limits {
  pub fn initial_round_trip_time(&self) -> Duration {
  self.initial_round_trip_time
  }
+
+ #[doc(hidden)]
+ #[inline]
+ pub fn active_migration_enabled(&self) -> bool {
+ matches!(self.migration_support, MigrationSupport::Enabled)
+ }
 }
 
 /// Creates limits for a given connection

quic/s2n-quic-core/src/transport/parameters/mod.rs

@@ -860,6 +860,10 @@ pub enum MigrationSupport {
  Disabled,
 }
 
+impl MigrationSupport {
+ pub const RECOMMENDED: Self = Self::Enabled;
+}
+
 impl TransportParameter for MigrationSupport {
  type CodecValue = ();
 
@@ -878,7 +882,7 @@ impl TransportParameter for MigrationSupport {
  }
 
  fn default_value() -> Self {
- MigrationSupport::Enabled
+ Self::default()
  }
 }
 
@@ -1462,5 +1466,6 @@ impl<
  load!(ack_delay_exponent, ack_delay_exponent);
  load!(max_active_connection_ids, active_connection_id_limit);
  load!(max_datagram_frame_size, max_datagram_frame_size);
+ load!(migration_support, migration_support);
  }
 }

quic/s2n-quic-tls/src/tests.rs

@@ -403,7 +403,7 @@ fn s2n_client_no_client_auth_s2n_server_requires_client_auth_test() {
  // but the client does not support it.
  assert!(test_result.is_err());
  let e = test_result.unwrap_err();
- assert_eq!(e.description().unwrap(), "UNEXPECTED_MESSAGE");
+ assert_eq!(e.description().unwrap(), "CERTIFICATE_REQUIRED");
 }
 
 #[test]

quic/s2n-quic-transport/src/connection/connection_impl.rs

@@ -1154,7 +1154,7 @@ impl<Config: endpoint::Config> connection::Trait for ConnectionImpl<Config> {
  congestion_controller_endpoint,
  path_migration,
  mtu_config,
- self.limits.initial_round_trip_time(),
+ &self.limits,
  &mut publisher,
  )?;
 

quic/s2n-quic-transport/src/path/manager.rs

@@ -9,10 +9,10 @@ use crate::{
  path::{challenge, Path},
  transmission,
 };
-use core::time::Duration;
 use s2n_quic_core::{
  ack,
- connection::{self, PeerId},
+ connection::{self, Limits, PeerId},
+ ensure,
  event::{
  self,
  builder::{DatagramDropReason, MtuUpdatedCause},
@@ -245,7 +245,7 @@ impl<Config: endpoint::Config> Manager<Config> {
  congestion_controller_endpoint: &mut Config::CongestionControllerEndpoint,
  migration_validator: &mut Config::PathMigrationValidator,
  mtu_config: mtu::Config,
- initial_rtt: Duration,
+ limits: &Limits,
  publisher: &mut Pub,
  ) -> Result<(Id, AmplificationOutcome), DatagramDropReason> {
  let valid_initial_received = self.valid_initial_received();
@@ -308,7 +308,7 @@ impl<Config: endpoint::Config> Manager<Config> {
  congestion_controller_endpoint,
  migration_validator,
  mtu_config,
- initial_rtt,
+ limits,
  publisher,
  )
  }
@@ -321,7 +321,7 @@ impl<Config: endpoint::Config> Manager<Config> {
  congestion_controller_endpoint: &mut Config::CongestionControllerEndpoint,
  migration_validator: &mut Config::PathMigrationValidator,
  mtu_config: mtu::Config,
- initial_rtt: Duration,
+ limits: &Limits,
  publisher: &mut Pub,
  ) -> Result<(Id, AmplificationOutcome), DatagramDropReason> {
  //= https://www.rfc-editor.org/rfc/rfc9000#section-9
@@ -332,6 +332,17 @@ impl<Config: endpoint::Config> Manager<Config> {
  let local_address = path_handle.local_address();
  let active_local_addr = self.active_path().local_address();
  let active_remote_addr = self.active_path().remote_address();
+ // The peer has intentionally tried to migrate to a new path because they changed
+ // their destination_connection_id. This is considered an "active" migration.
+ let active_migration =
+ self.active_path().local_connection_id != datagram.destination_connection_id;
+
+ if active_migration {
+ ensure!(
+ limits.active_migration_enabled(),
+ Err(DatagramDropReason::RejectedConnectionMigration)
+ )
+ }
 
  // TODO set alpn if available
  let attempt: migration::Attempt = migration::AttemptBuilder {
@@ -403,19 +414,21 @@ impl<Config: endpoint::Config> Manager<Config> {
  // estimator for the new path, and they are initialized with initial values,
  // we do not need to reset congestion controller and round-trip time estimator
  // again on confirming the peer's ownership of its new address.
- let rtt = self.active_path().rtt_estimator.for_new_path(initial_rtt);
+ let rtt = self
+ .active_path()
+ .rtt_estimator
+ .for_new_path(limits.initial_round_trip_time());
  let path_info =
  congestion_controller::PathInfo::new(mtu_config.initial_mtu, &remote_address);
  let cc = congestion_controller_endpoint.new_congestion_controller(path_info);
 
  let peer_connection_id = {
- if self.active_path().local_connection_id != datagram.destination_connection_id {
+ if active_migration {
  //= https://www.rfc-editor.org/rfc/rfc9000#section-9.5
  //# Similarly, an endpoint MUST NOT reuse a connection ID when sending to
  //# more than one destination address.
 
- // Peer has intentionally tried to migrate to this new path because they changed
- // their destination_connection_id, so we will change our destination_connection_id as well.
+ // Active connection migrations must use a new connection ID
  self.peer_id_registry
  .consume_new_id_for_new_path()
  .ok_or(DatagramDropReason::InsufficientConnectionIds)?

quic/s2n-quic-transport/src/path/manager/fuzz_target.rs

@@ -15,7 +15,6 @@ use s2n_quic_core::{
  inet::{DatagramInfo, ExplicitCongestionNotification},
  random,
  random::testing::Generator,
- recovery::DEFAULT_INITIAL_RTT,
  time::{testing::Clock, Clock as _},
  transport,
 };
@@ -178,7 +177,7 @@ impl Model {
  &mut Default::default(),
  &mut migration_validator,
  mtu::Config::default(),
- DEFAULT_INITIAL_RTT,
+ &Limits::default(),
  &mut publisher,
  ) {
  Ok(_) => {

quic/s2n-quic-transport/src/path/manager/snapshots/quic__s2n-quic-transport__src__path__manager__tests__events__active_connection_migration_disabled.snap

@@ -0,0 +1,9 @@
+---
+source: quic/s2n-quic-transport/src/path/manager/tests.rs
+expression: ""
+---
+ConnectionIdUpdated { path_id: 0, cid_consumer: Local, previous: 0x01, current: 0x69643032 }
+PathCreated { active: Path { local_addr: 0.0.0.0:0, local_cid: 0x4c6f63616c4900000000000000004c6f63616c49, remote_addr: 127.0.0.1:1, remote_cid: 0x69643032, id: 0, is_active: true }, new: Path { local_addr: 0.0.0.0:0, local_cid: 0x69643032, remote_addr: 127.0.0.2:1, remote_cid: 0x69643033, id: 1, is_active: false } }
+MtuUpdated { path_id: 1, mtu: 1200, cause: NewPath }
+PathCreated { active: Path { local_addr: 0.0.0.0:0, local_cid: 0x4c6f63616c4900000000000000004c6f63616c49, remote_addr: 127.0.0.1:1, remote_cid: 0x69643032, id: 0, is_active: true }, new: Path { local_addr: 0.0.0.0:0, local_cid: 0x4c6f63616c4900000000000000004c6f63616c49, remote_addr: 127.0.0.3:1, remote_cid: 0x69643032, id: 2, is_active: false } }
+MtuUpdated { path_id: 2, mtu: 1200, cause: NewPath }