diff --git a/crates/irtt-server/src/core.rs b/crates/irtt-server/src/core.rs index fbcc117..fa45071 100644 --- a/crates/irtt-server/src/core.rs +++ b/crates/irtt-server/src/core.rs @@ -213,7 +213,7 @@ impl ServerCore { fn handle_close(&mut self, peer: SocketAddr, token: u64) { let owns_session = self .session(token) - .is_some_and(|session| session.peer() == peer); + .is_some_and(|session| same_endpoint(session.peer(), peer)); if owns_session { self.sessions.remove(&token); } @@ -243,6 +243,29 @@ impl ServerCore { } } +/// Whether two source endpoints identify the same session endpoint. +/// +/// Identity is the address family, the IP address, the UDP source port and — +/// for IPv6 — the scope identifier. That is the whole list, so this is not +/// simply `==`: [`SocketAddrV6`] equality also covers the flow label, which +/// identifies no endpoint. It is routing metadata that a sender or a forwarding +/// path may legitimately vary within one flow, and a receiver reports it only +/// when asked to, so admitting it into identity could strand a session that its +/// own client can then never close. It grants nothing in exchange: the token, +/// address, port and scope are what an off-path attacker would have to guess. +/// +/// [`SocketAddrV6`]: std::net::SocketAddrV6 +fn same_endpoint(session: SocketAddr, peer: SocketAddr) -> bool { + match (session, peer) { + (SocketAddr::V6(session), SocketAddr::V6(peer)) => { + session.ip() == peer.ip() + && session.port() == peer.port() + && session.scope_id() == peer.scope_id() + } + (session, peer) => session == peer, + } +} + /// Applies the open parameter rules the decoder cannot express. /// /// Duration and Interval are accepted when absent, taking the wire default diff --git a/crates/irtt-server/src/tests/close.rs b/crates/irtt-server/src/tests/close.rs index e8abab7..e6ca5a4 100644 --- a/crates/irtt-server/src/tests/close.rs +++ b/crates/irtt-server/src/tests/close.rs @@ -6,6 +6,8 @@ //! authentication — is the same silence with the session left live, so these //! tests assert the table as well as the reply. +use std::net::{SocketAddr, SocketAddrV6}; + use irtt_proto::{encode_request, RequestToEncode}; use super::support::{ @@ -124,6 +126,36 @@ fn a_close_from_any_other_endpoint_leaves_the_session_live() { } } +#[test] +fn ipv6_flow_information_is_not_part_of_endpoint_identity() { + // Identity is family, address, port and scope. The flow label is routing + // metadata that may vary within one flow, so letting `SocketAddrV6` + // equality decide would strand a session its own client could never close. + let ip = "2001:db8::1".parse().unwrap(); + let unlabeled = SocketAddr::V6(SocketAddrV6::new(ip, 2112, 0, 0)); + let labeled = SocketAddr::V6(SocketAddrV6::new(ip, 2112, 0x000f_1234, 0)); + assert_ne!( + unlabeled, labeled, + "the two must differ only in flow information" + ); + + for (bound, closing) in [(unlabeled, labeled), (labeled, unlabeled)] { + let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + let token = open_session(&mut core, bound, None); + + assert_eq!( + core.handle_datagram(closing, &close_request(token, None)) + .unwrap(), + None + ); + assert_eq!( + core.session_count(), + 0, + "a close from {closing} must release the session opened from {bound}" + ); + } +} + #[test] fn an_unknown_or_zero_token_closes_nothing() { // Zero is reserved for no-test replies and is never issued to a session, so