From d269f77c87b6e23eaee3ef399e62d2b4e8cd4a47 Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Wed, 12 Aug 2026 10:30:50 +0700 Subject: [PATCH 1/3] feat(server): handle client close MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Release the session a client close names. A close is bound to a session only when the token matches and the datagram arrived from that session's exact endpoint, so holding a token is not by itself authority to tear the session down: a close from a foreign source port, address, family or IPv6 scope leaves the session live, and one close cannot take down a sibling session the same peer opened separately. A token that is unknown, already closed or zero matches nothing and leaves the table untouched; zero is reserved for no-test replies and never issued. Nothing is sent either way. Protocol version 1 defines no acknowledgement for a client close, so a successful close returns the same `Ok(None)` as a rejected one and a delivered close is indistinguishable to the client from a lost one. A standalone `Reply|Close` datagram is tolerated by clients but has no demonstrated use, so none is encoded. Authentication still runs before the session is looked up. That keeps a bad MAC on a live token indistinguishable from an unknown token, and means a close that fails authentication — missing, wrongly keyed, corrupted, or unexpected on a server with no key — cannot remove a session. Trailing bytes after the token stay acceptable: the structural decoder already tolerates them and the server does not re-parse the request. The session table remains the only record of how full the server is, so a close frees capacity immediately and a refused open can succeed once one is released. Close draws no token and touches no other state; the endpoint lookup borrows the table only until the comparison is made, so the removal needs no clone. Echo remains ignored. The test that pinned close as a no-op is replaced by `tests/close.rs`, which covers removal, silence, repeated close, echo after close, endpoint binding, unknown/zero/stale tokens, authentication failure, trailing bytes, sibling sessions and capacity reuse; `unsupported.rs` keeps echo and structurally invalid traffic. --- AGENTS.md | 2 +- crates/irtt-server/src/core.rs | 54 +++- crates/irtt-server/src/lib.rs | 8 +- crates/irtt-server/src/session.rs | 12 +- crates/irtt-server/src/tests/close.rs | 274 ++++++++++++++++++++ crates/irtt-server/src/tests/mod.rs | 1 + crates/irtt-server/src/tests/support.rs | 48 ++++ crates/irtt-server/src/tests/unsupported.rs | 35 +-- 8 files changed, 380 insertions(+), 54 deletions(-) create mode 100644 crates/irtt-server/src/tests/close.rs diff --git a/AGENTS.md b/AGENTS.md index 8a15cda..016a5c2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -57,7 +57,7 @@ Tokio must remain optional for `irtt-client`; default builds must remain runtime ### Server architecture (`irtt-server`) -`irtt-server` is a first-class reusable server crate. It currently contains the deterministic OPEN/session core — packet admission, authentication policy, open negotiation, the bounded session table and open reply construction. ECHO processing, client/server close and session lifetime remain unimplemented, as does the Tokio runtime; see `crates/irtt-server/AGENTS.md`. The rest of this section describes the boundary the crate is built to, including the parts not yet written. +`irtt-server` is a first-class reusable server crate. It currently contains the deterministic OPEN/session core — packet admission, authentication policy, open negotiation, the bounded session table, open reply construction and client-initiated close. ECHO processing, server-initiated close and session lifetime remain unimplemented, as does the Tokio runtime; see `crates/irtt-server/AGENTS.md`. The rest of this section describes the boundary the crate is built to, including the parts not yet written. - It shares wire semantics with the client through `irtt-proto` and follows the same clean-room boundary. - It must **not** depend on `irtt-client` merely to reuse protocol behavior. diff --git a/crates/irtt-server/src/core.rs b/crates/irtt-server/src/core.rs index 2265937..fbcc117 100644 --- a/crates/irtt-server/src/core.rs +++ b/crates/irtt-server/src/core.rs @@ -35,9 +35,9 @@ use crate::{ /// /// # Scope /// -/// This slice implements open handling and session creation. Echo and close -/// requests are structurally recognized and then deliberately ignored — no -/// reply, no session mutation — until their own slices. +/// This slice implements open handling, session creation and client-initiated +/// close. Echo requests are structurally recognized and then deliberately +/// ignored — no reply, no session mutation — until their own slice. #[derive(Debug)] pub struct ServerCore { config: ServerConfig, @@ -81,11 +81,8 @@ impl ServerCore { /// /// This is a lookup by token alone and is therefore *not* an admission /// check: a request is bound to a session only when the token matches and - /// it arrived from the session's exact endpoint. - // - // Called only by tests until echo and close processing arrive, which is - // what a token lookup is for. - #[allow(dead_code)] + /// it arrived from the session's exact endpoint. Callers pair it with an + /// endpoint comparison, as close handling does. pub(crate) fn session(&self, token: u64) -> Option<&Session> { self.sessions.get(&token) } @@ -101,6 +98,10 @@ impl ServerCore { /// payload, would exceed the session bound, or is a request kind this slice /// does not implement. None of that mutates any live session. /// + /// `Ok(None)` is not only a rejection, though. A close request that does + /// release its session is answered with nothing as well, because protocol + /// version 1 defines no acknowledgement for one. + /// /// Authentication is checked before open parameters are decoded. That /// ordering is deliberate: a packet that fails authentication is discarded /// without parsing untrusted parameter data, and no reply behavior may @@ -133,11 +134,14 @@ impl ServerCore { match request.kind { DecodedRequestKind::Open { no_test, params } => self.handle_open(peer, no_test, params), - // Not implemented in this slice. A close must not remove a session - // and an echo must not advance receive state until the slices that - // implement those behaviors; emitting a placeholder reply now would - // be worse than silence. - DecodedRequestKind::Close { .. } | DecodedRequestKind::Echo { .. } => Ok(None), + DecodedRequestKind::Close { token } => { + self.handle_close(peer, token); + Ok(None) + } + // Not implemented in this slice. An echo must not advance receive + // state until the slice that implements it; emitting a placeholder + // reply now would be worse than silence. + DecodedRequestKind::Echo { .. } => Ok(None), } } @@ -191,6 +195,30 @@ impl ServerCore { Ok(Some(packet)) } + /// Releases the session a close request names, if that request owns one. + /// + /// A close is bound to a session only when the token matches *and* the + /// datagram arrived from that session's exact endpoint, so holding a token + /// is not by itself authority to tear the session down: a close from a + /// foreign endpoint leaves it live, and one close cannot take down a + /// sibling session that the same peer opened separately. A token that is + /// unknown, already closed, or zero — zero is reserved for no-test replies + /// and never issued — matches nothing and leaves the table untouched. + /// + /// Nothing is sent either way, which is why this returns no reply to + /// encode. Protocol version 1 defines no acknowledgement for a client + /// close, so a delivered close is indistinguishable to the client from a + /// lost one, and the freed capacity is immediately reusable because the + /// session table is the only record of it. + fn handle_close(&mut self, peer: SocketAddr, token: u64) { + let owns_session = self + .session(token) + .is_some_and(|session| session.peer() == peer); + if owns_session { + self.sessions.remove(&token); + } + } + /// Encodes a reply this server constructed. Any failure here is internal. fn encode_reply(&self, reply: &OpenReply) -> Result, ServerError> { encode_open_reply(reply, self.config.hmac_key()) diff --git a/crates/irtt-server/src/lib.rs b/crates/irtt-server/src/lib.rs index 5f4fd6d..c5b2e00 100644 --- a/crates/irtt-server/src/lib.rs +++ b/crates/irtt-server/src/lib.rs @@ -31,10 +31,10 @@ //! //! # Current scope //! -//! Open handling and session creation only. Echo processing, client-initiated -//! close, session lifetime and expiry, and the Tokio runtime are separate -//! slices; structurally valid echo and close requests are currently accepted by -//! the admission boundary and then ignored. +//! Open handling, session creation and client-initiated close. Echo +//! processing, server-initiated close, session lifetime and expiry, and the +//! Tokio runtime are separate slices; a structurally valid echo request is +//! currently accepted by the admission boundary and then ignored. #![forbid(unsafe_code)] mod config; diff --git a/crates/irtt-server/src/session.rs b/crates/irtt-server/src/session.rs index b1e40de..328e60c 100644 --- a/crates/irtt-server/src/session.rs +++ b/crates/irtt-server/src/session.rs @@ -29,15 +29,9 @@ impl Session { /// /// A session is bound to this endpoint, not merely to its address: address /// family, address, UDP source port and — for IPv6 — the scope identifier - /// all form part of its identity. Echo and close processing, once - /// implemented, must require both a token match and an exact endpoint match - /// before touching a session. - // - // Read only by tests until then. The endpoint has to be captured at open - // time — it is not recoverable later — so this slice stores state whose - // production consumer is the next one, and the tests prove it is stored - // exactly. - #[allow(dead_code)] + /// all form part of its identity. Close processing requires both a token + /// match and an exact endpoint match before removing a session, and echo + /// processing will require the same. pub(crate) fn peer(&self) -> SocketAddr { self.peer } diff --git a/crates/irtt-server/src/tests/close.rs b/crates/irtt-server/src/tests/close.rs new file mode 100644 index 0000000..e8abab7 --- /dev/null +++ b/crates/irtt-server/src/tests/close.rs @@ -0,0 +1,274 @@ +//! Client-initiated close. +//! +//! A close request that matches a live token *and* arrived from that session's +//! exact endpoint releases the session and is answered with nothing. Every +//! other close — unknown, stale or zero token, foreign endpoint, failed +//! authentication — is the same silence with the session left live, so these +//! tests assert the table as well as the reply. + +use irtt_proto::{encode_request, RequestToEncode}; + +use super::support::{ + client_params, close_request, close_request_with_trailing, core_for, core_with_tokens, + open_request, open_session, other_peer, peer, ScriptedTokens, KEY, OTHER_KEY, +}; +use crate::ServerConfig; + +const TOKEN_A: u64 = 0x0102_0304_0506_0708; +const TOKEN_B: u64 = 0x1112_1314_1516_1718; +const UNKNOWN_TOKEN: u64 = 0x2122_2324_2526_2728; + +#[test] +fn a_valid_close_removes_the_session_and_answers_nothing() { + for hmac_key in [None, Some(KEY)] { + let tokens = ScriptedTokens::new([TOKEN_A]); + let mut core = core_for(hmac_key, tokens.clone()); + let token = open_session(&mut core, peer(), hmac_key); + assert_eq!(core.session_count(), 1); + + // Exactly `None`, not an empty or acknowledging packet: version 1 has + // no close reply, so the client learns nothing about the outcome. + assert_eq!( + core.handle_datagram(peer(), &close_request(token, hmac_key)) + .unwrap(), + None + ); + + assert_eq!(core.session_count(), 0); + assert_eq!(core.session(token), None); + assert_eq!( + tokens.remaining(), + 0, + "a close must not draw from the token source" + ); + } +} + +#[test] +fn a_repeated_close_is_a_stale_token_and_changes_nothing() { + for hmac_key in [None, Some(KEY)] { + let mut core = core_for(hmac_key, ScriptedTokens::new([TOKEN_A])); + let token = open_session(&mut core, peer(), hmac_key); + let close = close_request(token, hmac_key); + + assert_eq!(core.handle_datagram(peer(), &close).unwrap(), None); + assert_eq!(core.handle_datagram(peer(), &close).unwrap(), None); + + assert_eq!(core.session_count(), 0, "nothing may be resurrected"); + assert_eq!(core.session(token), None); + } +} + +#[test] +fn an_echo_after_a_close_is_a_stale_token() { + // Echo processing does not exist yet; the point here is that the token is + // gone, so the request has no session to reach even once it does. + for hmac_key in [None, Some(KEY)] { + let mut core = core_for(hmac_key, ScriptedTokens::new([TOKEN_A])); + let token = open_session(&mut core, peer(), hmac_key); + let params = core.session(token).expect("session must exist").params(); + let echo = encode_request( + RequestToEncode::Echo { + token, + sequence: 0, + params, + payload: &[], + }, + hmac_key, + ) + .unwrap(); + + core.handle_datagram(peer(), &close_request(token, hmac_key)) + .unwrap(); + + assert_eq!(core.handle_datagram(peer(), &echo).unwrap(), None); + assert_eq!(core.session_count(), 0); + assert_eq!(core.session(token), None); + } +} + +#[test] +fn a_close_from_any_other_endpoint_leaves_the_session_live() { + // Endpoint identity is exact: family, address, UDP source port and IPv6 + // scope all count. Possessing the token is not authority to close. + for (bound, foreign) in [ + ("198.51.100.7:41234", "198.51.100.7:41235"), + ("198.51.100.7:41234", "198.51.100.9:41234"), + ("198.51.100.7:41234", "[::ffff:198.51.100.7]:41234"), + ("[2001:db8::1]:2112", "[2001:db8::2]:2112"), + ("[fe80::1%3]:2112", "[fe80::1%4]:2112"), + ] { + let bound = bound.parse().unwrap(); + let foreign = foreign.parse().unwrap(); + let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + let token = open_session(&mut core, bound, None); + let session = core.session(token).cloned().expect("session must exist"); + let close = close_request(token, None); + + assert_eq!( + core.handle_datagram(foreign, &close).unwrap(), + None, + "a close from {foreign} must be silent" + ); + assert_eq!(core.session_count(), 1); + assert_eq!( + core.session(token), + Some(&session), + "a close from {foreign} must not remove the session bound to {bound}" + ); + + // The same request from the session's own endpoint does close it, so + // the request itself was never the reason for the silence. + assert_eq!(core.handle_datagram(bound, &close).unwrap(), None); + assert_eq!(core.session_count(), 0); + } +} + +#[test] +fn an_unknown_or_zero_token_closes_nothing() { + // Zero is reserved for no-test replies and is never issued to a session, so + // it can only ever miss the table. + for (name, token) in [("an unknown token", UNKNOWN_TOKEN), ("a zero token", 0)] { + let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + let live = open_session(&mut core, peer(), None); + let session = core.session(live).cloned().expect("session must exist"); + + assert_eq!( + core.handle_datagram(peer(), &close_request(token, None)) + .unwrap(), + None, + "{name} must be answered with silence" + ); + assert_eq!(core.session_count(), 1, "{name} must remove nothing"); + assert_eq!(core.session(live), Some(&session)); + } +} + +#[test] +fn a_close_that_fails_authentication_leaves_the_session_live() { + // Authentication runs before the session is looked up, so a close that + // fails it is indistinguishable from one bearing an unknown token — and it + // must not be able to tear down an authenticated session. + let mut core = core_for(Some(KEY), ScriptedTokens::new([TOKEN_A])); + let token = open_session(&mut core, peer(), Some(KEY)); + let session = core.session(token).cloned().expect("session must exist"); + + let mut corrupted = close_request(token, Some(KEY)); + corrupted[5] ^= 0x80; + + for (name, packet) in [ + ( + "a close with no MAC field at all", + close_request(token, None), + ), + ( + "a close signed with a different key", + close_request(token, Some(OTHER_KEY)), + ), + ("a close with one bit flipped in the MAC", corrupted), + ] { + assert_eq!( + core.handle_datagram(peer(), &packet).unwrap(), + None, + "{name} must be answered with silence" + ); + assert_eq!(core.session_count(), 1, "{name} must not remove a session"); + assert_eq!(core.session(token), Some(&session)); + } +} + +#[test] +fn a_non_authenticating_server_ignores_an_authenticated_close() { + let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + let token = open_session(&mut core, peer(), None); + let session = core.session(token).cloned().expect("session must exist"); + + assert_eq!( + core.handle_datagram(peer(), &close_request(token, Some(KEY))) + .unwrap(), + None + ); + + assert_eq!(core.session_count(), 1); + assert_eq!(core.session(token), Some(&session)); +} + +#[test] +fn trailing_bytes_after_the_token_do_not_prevent_the_close() { + for hmac_key in [None, Some(KEY)] { + let mut core = core_for(hmac_key, ScriptedTokens::new([TOKEN_A])); + let token = open_session(&mut core, peer(), hmac_key); + + let close = close_request_with_trailing(token, &[0xde, 0xad, 0xbe, 0xef], hmac_key); + + assert_eq!(core.handle_datagram(peer(), &close).unwrap(), None); + assert_eq!(core.session_count(), 0); + } +} + +#[test] +fn a_close_removes_only_the_session_it_names() { + // Repeated opens from one endpoint are independent sessions, so a close + // must be resolved by token, never by endpoint. + let mut core = core_with_tokens( + ServerConfig::default(), + ScriptedTokens::new([TOKEN_A, TOKEN_B]), + ); + let first = open_session(&mut core, peer(), None); + let second = open_session(&mut core, peer(), None); + let survivor = core.session(second).cloned().expect("session must exist"); + assert_eq!(core.session_count(), 2); + + assert_eq!( + core.handle_datagram(peer(), &close_request(first, None)) + .unwrap(), + None + ); + assert_eq!(core.session_count(), 1, "only one session may be released"); + assert_eq!(core.session(first), None); + assert_eq!(core.session(second), Some(&survivor)); + + assert_eq!( + core.handle_datagram(peer(), &close_request(second, None)) + .unwrap(), + None + ); + assert_eq!(core.session_count(), 0); +} + +#[test] +fn a_close_frees_session_capacity_immediately() { + // The session table is the only record of how full the server is, so a + // removal is reclaimed at once with no separate counter to drift. + let tokens = ScriptedTokens::new([TOKEN_A, TOKEN_B]); + let mut core = core_with_tokens(ServerConfig::default().with_max_sessions(1), tokens.clone()); + let request = open_request(&client_params(), None); + + let first = open_session(&mut core, peer(), None); + assert_eq!( + core.handle_datagram(other_peer(), &request).unwrap(), + None, + "the bound is reached, so the second open is refused" + ); + assert_eq!(core.session_count(), 1); + assert_eq!( + tokens.remaining(), + 1, + "a refused open must not commit a token" + ); + + core.handle_datagram(peer(), &close_request(first, None)) + .unwrap(); + assert_eq!(core.session_count(), 0); + assert_eq!( + tokens.remaining(), + 1, + "a close must not draw from the token source" + ); + + let second = open_session(&mut core, other_peer(), None); + assert_eq!(second, TOKEN_B); + assert_ne!(second, 0); + assert_eq!(core.session_count(), 1); + assert_eq!(core.session(second).unwrap().peer(), other_peer()); +} diff --git a/crates/irtt-server/src/tests/mod.rs b/crates/irtt-server/src/tests/mod.rs index d820706..96687ed 100644 --- a/crates/irtt-server/src/tests/mod.rs +++ b/crates/irtt-server/src/tests/mod.rs @@ -8,6 +8,7 @@ //! Tokens come from a scripted source so session identity, collisions and //! allocation failure are all assertable. +mod close; mod hmac; mod no_test; mod open; diff --git a/crates/irtt-server/src/tests/support.rs b/crates/irtt-server/src/tests/support.rs index bf35c19..73c8d66 100644 --- a/crates/irtt-server/src/tests/support.rs +++ b/crates/irtt-server/src/tests/support.rs @@ -72,6 +72,31 @@ pub(crate) fn core_with_tokens(config: ServerConfig, tokens: ScriptedTokens) -> ServerCore::with_token_source(config, Box::new(tokens)) } +/// An otherwise default core that authenticates iff `hmac_key` is set. +/// +/// Most behavior has to hold identically with and without authentication, so +/// tests loop over both and build the core from the key. +pub(crate) fn core_for(hmac_key: Option<&[u8]>, tokens: ScriptedTokens) -> ServerCore { + let mut config = ServerConfig::default(); + if let Some(key) = hmac_key { + config = config.with_hmac_key(key); + } + core_with_tokens(config, tokens) +} + +/// Opens one ordinary session from `endpoint` and returns its token. +pub(crate) fn open_session( + core: &mut ServerCore, + endpoint: SocketAddr, + hmac_key: Option<&[u8]>, +) -> u64 { + let packet = core + .handle_datagram(endpoint, &open_request(&client_params(), hmac_key)) + .unwrap() + .expect("the session-creating open must be answered"); + expect_normal_open_reply(&packet, hmac_key).token +} + /// Encodes a normal open request carrying `params`. pub(crate) fn open_request(params: &Params, hmac_key: Option<&[u8]>) -> Vec { encode_request( @@ -96,6 +121,29 @@ pub(crate) fn no_test_request(params: &Params, hmac_key: Option<&[u8]>) -> Vec) -> Vec { + encode_request(RequestToEncode::Close { token }, hmac_key).unwrap() +} + +/// Encodes a close request carrying arbitrary bytes after the token. +/// +/// The decoder tolerates them, and this is the shape that proves it. The MAC +/// covers the whole datagram, so it is recomputed over the final form rather +/// than left behind by the encoder's shorter packet. +pub(crate) fn close_request_with_trailing( + token: u64, + trailing: &[u8], + hmac_key: Option<&[u8]>, +) -> Vec { + let mut packet = close_request(token, hmac_key); + packet.extend_from_slice(trailing); + if let Some(key) = hmac_key { + irtt_proto::compute_hmac_in_place(key, &mut packet, hmac_field_offset()).unwrap(); + } + packet +} + /// Builds an open request around a raw parameter payload. /// /// Used for payloads a compliant encoder cannot produce, such as a truncated diff --git a/crates/irtt-server/src/tests/unsupported.rs b/crates/irtt-server/src/tests/unsupported.rs index 4036ca2..4b69d46 100644 --- a/crates/irtt-server/src/tests/unsupported.rs +++ b/crates/irtt-server/src/tests/unsupported.rs @@ -1,25 +1,20 @@ //! Requests this slice deliberately does not implement. //! -//! Echo and close are recognized by the admission boundary and then ignored. -//! These tests exist to keep the scope boundary honest: a half-implemented -//! lifecycle must not appear by accident before the slices that own it. +//! Echo is recognized by the admission boundary and then ignored. These tests +//! exist to keep the scope boundary honest: receive state must not appear by +//! accident before the slice that owns it. use irtt_proto::{encode_request, RequestToEncode}; -use super::support::{client_params, core_with_tokens, open_request, peer, ScriptedTokens, KEY}; -use crate::ServerConfig; +use super::support::{ + client_params, core_for, open_request, open_session, peer, ScriptedTokens, KEY, +}; const TOKEN_A: u64 = 0x0102_0304_0506_0708; fn open_one_session(hmac_key: Option<&[u8]>) -> crate::ServerCore { - let mut config = ServerConfig::default(); - if let Some(key) = hmac_key { - config = config.with_hmac_key(key); - } - let mut core = core_with_tokens(config, ScriptedTokens::new([TOKEN_A])); - core.handle_datagram(peer(), &open_request(&client_params(), hmac_key)) - .unwrap() - .expect("the session-creating open must succeed"); + let mut core = core_for(hmac_key, ScriptedTokens::new([TOKEN_A])); + open_session(&mut core, peer(), hmac_key); core } @@ -46,20 +41,6 @@ fn a_valid_echo_is_neither_answered_nor_counted_yet() { } } -#[test] -fn a_valid_close_does_not_remove_the_session_yet() { - for hmac_key in [None, Some(KEY)] { - let mut core = open_one_session(hmac_key); - let session = core.session(TOKEN_A).cloned().expect("session must exist"); - - let close = encode_request(RequestToEncode::Close { token: TOKEN_A }, hmac_key).unwrap(); - - assert_eq!(core.handle_datagram(peer(), &close).unwrap(), None); - assert_eq!(core.session_count(), 1); - assert_eq!(core.session(TOKEN_A), Some(&session)); - } -} - #[test] fn structurally_invalid_datagrams_are_discarded() { let mut core = open_one_session(None); -- 2.54.0 From e79ea94318b9977c18f1579c923d1c5a9043f8d4 Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Wed, 12 Aug 2026 10:42:11 +0700 Subject: [PATCH 2/3] fix(server): exclude IPv6 flow information from endpoint identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Session identity is the address family, the IP address, the UDP source port and — for IPv6 — the scope identifier. `SocketAddrV6` equality additionally covers the flow label, so comparing endpoints with `==` admitted a field that identifies no endpoint: a client whose flow label changed mid-session could be refused its own close and strand the session, which nothing reclaims while expiry is unimplemented. It bought nothing in exchange, since the token, address, port and scope are what an off-path attacker would have to guess. Compare the specified components instead. Scope still counts, and the address is still stored verbatim, so a later reply goes back to exactly the endpoint the request came from. Not reachable through a default socket today: a receiver reports the flow label only when asked to, and there is no runtime slice yet. It is reachable through the public core, which takes caller-supplied addresses. --- crates/irtt-server/src/core.rs | 25 ++++++++++++++++++++- crates/irtt-server/src/tests/close.rs | 32 +++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) 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 -- 2.54.0 From 6c34fd3d2067f3862a5915b9651e90c0ff964a0d Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Wed, 12 Aug 2026 10:48:42 +0700 Subject: [PATCH 3/3] docs(server): record IPv6 zone identity as project policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Endpoint identity documented the IPv6 scope identifier as an established component alongside family, address and port. The server specification does list the zone, but its remaining-unknowns section records that multi-zone behavior could not be tested on a single-host platform, so no evidence settles how a reference server compares scoped addresses. Presenting the choice as verified compatibility overstated what is known, which the crate's own guidance forbids. Say what it is instead: a deliberate policy taken under a recorded unknown. Two peers reachable at the same link-local address in different zones are genuinely different peers, so treating the zone as identity is the conservative reading — the failure it prevents is one peer closing another's session, and the failure it risks is a scoped client having to reopen. The excluded flow label is labelled the same way, and `irtt-server` guidance now carries the rule so the echo slice reuses this comparison rather than inventing a second one. Behavior is unchanged; this is documentation and a test comment. --- crates/irtt-server/AGENTS.md | 8 +++++++ crates/irtt-server/src/core.rs | 32 ++++++++++++++++++++------- crates/irtt-server/src/session.rs | 16 +++++++++----- crates/irtt-server/src/tests/close.rs | 7 ++++-- 4 files changed, 48 insertions(+), 15 deletions(-) diff --git a/crates/irtt-server/AGENTS.md b/crates/irtt-server/AGENTS.md index 4def913..3069944 100644 --- a/crates/irtt-server/AGENTS.md +++ b/crates/irtt-server/AGENTS.md @@ -25,6 +25,14 @@ material does not establish a semantic, state the ambiguity instead of guessing. - No statistics, CLI or presentation concerns. - `SocketAddr` belongs here: endpoint identity is session state. It does not belong in `irtt-proto`. +- Endpoint identity is compared field by field, not with `SocketAddr` equality. + Family, address and port are established components. The IPv6 flow label is + excluded because it identifies no endpoint; the IPv6 zone is included as a + deliberate project policy, because the specification records multi-zone + behavior as untested and two peers sharing a link-local address in different + zones are genuinely different peers. Reuse that one comparison rather than + writing a second identity rule, and keep both choices labelled as policy + rather than as verified compatibility. ## Core versus runtime diff --git a/crates/irtt-server/src/core.rs b/crates/irtt-server/src/core.rs index fa45071..7320c0e 100644 --- a/crates/irtt-server/src/core.rs +++ b/crates/irtt-server/src/core.rs @@ -245,14 +245,30 @@ 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. +/// The address family, the IP address and the UDP source port are established +/// identity components: a request from a different port, address or family is +/// dropped and leaves its session live. +/// +/// The remaining two fields of [`SocketAddrV6`] are decided here rather than +/// observed, which is why this is not simply `==`. +/// +/// The flow label is excluded. It identifies no endpoint — it is routing +/// metadata a sender or forwarding path may legitimately vary within one flow, +/// and a receiver reports it only when asked to — so admitting it could strand +/// a session 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. +/// +/// The scope identifier is included, and that is **`irtt-rs` policy, not +/// verified compatibility behavior**. The server specification lists the IPv6 +/// zone as an identity component, but its remaining-unknowns section records +/// that multi-zone behavior could not be tested on a single-host platform, so +/// no evidence settles how a reference server compares scoped addresses. Two +/// peers reachable at the same link-local address in different zones are +/// genuinely different peers, so treating the zone as identity is the +/// conservative reading: the failure it prevents is one peer closing another's +/// session, while the failure it risks is a scoped client having to reopen. +/// Revisit if evidence ever settles the question. /// /// [`SocketAddrV6`]: std::net::SocketAddrV6 fn same_endpoint(session: SocketAddr, peer: SocketAddr) -> bool { diff --git a/crates/irtt-server/src/session.rs b/crates/irtt-server/src/session.rs index 328e60c..8411184 100644 --- a/crates/irtt-server/src/session.rs +++ b/crates/irtt-server/src/session.rs @@ -27,11 +27,17 @@ impl Session { /// The exact source endpoint the open request arrived from. /// - /// A session is bound to this endpoint, not merely to its address: address - /// family, address, UDP source port and — for IPv6 — the scope identifier - /// all form part of its identity. Close processing requires both a token - /// match and an exact endpoint match before removing a session, and echo - /// processing will require the same. + /// A session is bound to this endpoint, not merely to its address: the + /// address family, the address and the UDP source port all form part of its + /// identity, as — by `irtt-rs` policy rather than observed behavior — does + /// the IPv6 scope identifier. Close processing requires both a token match + /// and an endpoint match before removing a session, and echo processing + /// will require the same; `same_endpoint` in `core` is where the comparison + /// and its reasoning live. + /// + /// The address is stored exactly as received, including the flow label that + /// identity ignores, so a reply goes back to the endpoint the request + /// actually came from. pub(crate) fn peer(&self) -> SocketAddr { self.peer } diff --git a/crates/irtt-server/src/tests/close.rs b/crates/irtt-server/src/tests/close.rs index e6ca5a4..081d70b 100644 --- a/crates/irtt-server/src/tests/close.rs +++ b/crates/irtt-server/src/tests/close.rs @@ -91,8 +91,11 @@ fn an_echo_after_a_close_is_a_stale_token() { #[test] fn a_close_from_any_other_endpoint_leaves_the_session_live() { - // Endpoint identity is exact: family, address, UDP source port and IPv6 - // scope all count. Possessing the token is not authority to close. + // Possessing the token is not authority to close. The first four pairs + // differ in an established identity component; the last differs only in + // IPv6 zone, which is `irtt-rs` policy rather than observed behavior — see + // `same_endpoint`. It is pinned so the choice cannot drift silently, not + // because evidence settled it. for (bound, foreign) in [ ("198.51.100.7:41234", "198.51.100.7:41235"), ("198.51.100.7:41234", "198.51.100.9:41234"), -- 2.54.0