feat: emit migration deny reason with datagram drop event (#2456)

Author: camshaft

quic/s2n-quic-core/events/common.rs

@@ -784,7 +784,7 @@ enum DatagramDropReason {
     /// The peer initiated a connection migration before the handshake was confirmed.
     ConnectionMigrationDuringHandshake,
     /// The attempted connection migration was rejected.
-    RejectedConnectionMigration,
+    RejectedConnectionMigration { reason: MigrationDenyReason },
     /// The maximum number of paths per connection was exceeded.
     PathLimitExceeded,
     /// The peer initiated a connection migration without supplying enough connection IDs to use.

quic/s2n-quic-core/src/event/generated.rs

@@ -878,7 +878,7 @@ pub mod api {
         ConnectionMigrationDuringHandshake {},
         #[non_exhaustive]
         #[doc = " The attempted connection migration was rejected."]
-        RejectedConnectionMigration {},
+        RejectedConnectionMigration { reason: MigrationDenyReason },
         #[non_exhaustive]
         #[doc = " The maximum number of paths per connection was exceeded."]
         PathLimitExceeded {},
@@ -4985,7 +4985,7 @@ pub mod builder {
         #[doc = " The peer initiated a connection migration before the handshake was confirmed."]
         ConnectionMigrationDuringHandshake,
         #[doc = " The attempted connection migration was rejected."]
-        RejectedConnectionMigration,
+        RejectedConnectionMigration { reason: MigrationDenyReason },
         #[doc = " The maximum number of paths per connection was exceeded."]
         PathLimitExceeded,
         #[doc = " The peer initiated a connection migration without supplying enough connection IDs to use."]
@@ -5010,7 +5010,9 @@ pub mod builder {
                 Self::RejectedConnectionAttempt => RejectedConnectionAttempt {},
                 Self::UnknownServerAddress => UnknownServerAddress {},
                 Self::ConnectionMigrationDuringHandshake => ConnectionMigrationDuringHandshake {},
-                Self::RejectedConnectionMigration => RejectedConnectionMigration {},
+                Self::RejectedConnectionMigration { reason } => RejectedConnectionMigration {
+                    reason: reason.into_event(),
+                },
                 Self::PathLimitExceeded => PathLimitExceeded {},
                 Self::InsufficientConnectionIds => InsufficientConnectionIds {},
             }

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

@@ -336,10 +336,13 @@ impl<Config: endpoint::Config> Manager<Config> {
             self.active_path().local_connection_id != datagram.destination_connection_id;
 
         if active_migration {
-            ensure!(
-                limits.active_migration_enabled(),
-                Err(DatagramDropReason::RejectedConnectionMigration)
-            )
+            ensure!(limits.active_migration_enabled(), {
+                let reason = migration::DenyReason::ConnectionMigrationDisabled;
+                publisher.on_connection_migration_denied(reason.into_event());
+                Err(DatagramDropReason::RejectedConnectionMigration {
+                    reason: reason.into_event().reason,
+                })
+            })
         }
 
         // TODO set alpn if available
@@ -367,7 +370,9 @@ impl<Config: endpoint::Config> Manager<Config> {
             }
             migration::Outcome::Deny(reason) => {
                 publisher.on_connection_migration_denied(reason.into_event());
-                return Err(DatagramDropReason::RejectedConnectionMigration);
+                return Err(DatagramDropReason::RejectedConnectionMigration {
+                    reason: reason.into_event().reason,
+                });
             }
             _ => {
                 unimplemented!("unimplemented migration outcome");

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

@@ -196,7 +196,7 @@ impl Model {
                 match datagram_drop_reason {
                     // Ignore errors emitted by the migration::validator and peer_id_registry
                     DatagramDropReason::InsufficientConnectionIds => {}
-                    DatagramDropReason::RejectedConnectionMigration => {}
+                    DatagramDropReason::RejectedConnectionMigration { .. } => {}
                     DatagramDropReason::PathLimitExceeded => {}
                     datagram_drop_reason => panic!("{:?}", datagram_drop_reason),
                 };

quic/s2n-quic-transport/src/path/manager/snapshots/path__manager__tests__active_connection_migration_disabled__events.snap

@@ -3,6 +3,7 @@ source: quic/s2n-quic-core/src/event/snapshot.rs
 input_file: quic/s2n-quic-transport/src/path/manager/tests.rs
 ---
 ConnectionIdUpdated { path_id: 0, cid_consumer: Local, previous: 0x01, current: 0x69643032 }
+ConnectionMigrationDenied { reason: ConnectionMigrationDisabled }
 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, search_complete: false }
 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 } }

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

@@ -14,7 +14,7 @@ use crate::{
 use core::time::Duration;
 use s2n_quic_core::{
     connection::limits::ANTI_AMPLIFICATION_MULTIPLIER,
-    event::testing::Publisher,
+    event::{builder::MigrationDenyReason, testing::Publisher},
     inet::{DatagramInfo, ExplicitCongestionNotification, SocketAddress},
     path::{migration, RemoteAddress},
     random::{self, Generator},
@@ -1043,7 +1043,9 @@ fn active_connection_migration_disabled() {
     // The active migration is rejected
     assert!(matches!(
         res,
-        Err(DatagramDropReason::RejectedConnectionMigration)
+        Err(DatagramDropReason::RejectedConnectionMigration {
+            reason: MigrationDenyReason::ConnectionMigrationDisabled { .. }
+        })
     ));
     assert_eq!(1, manager.paths.len());
 

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

@@ -4,7 +4,7 @@
 use super::*;
 use s2n_codec::DecoderBufferMut;
 use s2n_quic_core::{
-    event::api::{DatagramDropReason, Subject},
+    event::api::{DatagramDropReason, MigrationDenyReason, Subject},
     packet::interceptor::{Datagram, Interceptor},
     path::{LocalAddress, RemoteAddress},
 };
@@ -373,3 +373,77 @@ fn rebind_ipv4_mapped_before_handshake_confirmed() {
         }
     }
 }
+
+/// Rebinds to a port after a specified number of packets
+struct RebindToPort {
+    port: u16,
+    after: usize,
+}
+
+impl Interceptor for RebindToPort {
+    fn intercept_rx_remote_address(&mut self, _subject: &Subject, addr: &mut RemoteAddress) {
+        if self.after == 0 {
+            addr.set_port(self.port);
+        }
+    }
+
+    fn intercept_rx_datagram<'a>(
+        &mut self,
+        _subject: &Subject,
+        _datagram: &Datagram,
+        payload: DecoderBufferMut<'a>,
+    ) -> DecoderBufferMut<'a> {
+        self.after = self.after.saturating_sub(1);
+        payload
+    }
+}
+
+/// Ensures that a blocked port is not migrated to
+#[test]
+fn rebind_blocked_port() {
+    let model = Model::default();
+    let subscriber = recorder::DatagramDropped::new();
+    let datagram_dropped_events = subscriber.events();
+
+    test(model, move |handle| {
+        let server = Server::builder()
+            .with_io(handle.builder().build()?)?
+            .with_tls(SERVER_CERTS)?
+            .with_event((tracing_events(), subscriber))?
+            .with_random(Random::with_seed(456))?
+            .with_packet_interceptor(RebindToPort { port: 53, after: 2 })?
+            .start()?;
+
+        let client = Client::builder()
+            .with_io(handle.builder().build()?)?
+            .with_tls(certificates::CERT_PEM)?
+            .with_event(tracing_events())?
+            .with_random(Random::with_seed(456))?
+            .start()?;
+
+        let addr = start_server(server)?;
+
+        primary::spawn(async move {
+            let mut connection = client
+                .connect(Connect::new(addr).with_server_name("localhost"))
+                .await
+                .unwrap();
+            let mut stream = connection.open_bidirectional_stream().await.unwrap();
+            let _ = stream.send(Bytes::from_static(b"hello")).await;
+            let _ = stream.finish();
+            let _ = stream.receive().await;
+        });
+
+        Ok(addr)
+    })
+    .unwrap();
+
+    let datagram_dropped_events = datagram_dropped_events.lock().unwrap();
+
+    assert!(!datagram_dropped_events.is_empty());
+    for event in datagram_dropped_events.iter() {
+        if let DatagramDropReason::RejectedConnectionMigration { reason, .. } = &event.reason {
+            assert!(matches!(reason, MigrationDenyReason::BlockedPort { .. }));
+        }
+    }
+}