From 93491e776234f76e4a46a6b5decc21e25d2a43d7 Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Wed, 12 Aug 2026 00:21:59 +0700 Subject: [PATCH 1/3] refactor(proto): preserve open parameter presence `Params::decode` seeded `Params::default()` and overwrote only the tags it saw, so an omitted tag and one explicitly encoded as zero were indistinguishable afterwards. The protocol gives those the same value but not the same meaning, and a server cannot apply its open admission rules without telling them apart: an absent Duration or Interval is accepted as the wire default zero, while one explicitly encoded as zero is invalid. Add `Params::decode_with_presence`, returning the decoded values alongside a `ParamPresence` recording which known tags appeared. `Params::decode` now delegates to it, so there is still exactly one parser and the two cannot diverge. Unknown tags stay ignored and untracked, and repeated known tags keep last-value-wins. Add `Clock::Unspecified = 0` as the default, which is the wire default for an absent Clock tag and what an empty parameter payload decodes to. An explicit Clock tag encoding zero remains an invalid enum value, so `Unspecified` is only ever reached by omission; `encode` omits the tag for it rather than emitting an explicit zero. It selects no clock, so `has_wall`/`has_mono` are false and no timestamp field is laid out. `ClientConfig::clock` is public and therefore now constructible as `Unspecified`, which is a wire default rather than a client mode. Reject it during config validation. The client default stays `Clock::Both`, so normal client-generated params and wire vectors are unchanged. --- crates/irtt-client/src/client/tests/config.rs | 10 + crates/irtt-client/src/config.rs | 4 + crates/irtt-client/src/session/machine.rs | 7 +- crates/irtt-client/tests/protocol_options.rs | 1 + .../tests/protocol_options/backend_smoke.rs | 3 + crates/irtt-proto/src/echo.rs | 9 +- crates/irtt-proto/src/layout.rs | 1 + crates/irtt-proto/src/lib.rs | 11 +- crates/irtt-proto/src/params.rs | 291 +++++++++++++++++- crates/irtt-proto/tests/directional_codecs.rs | 4 +- 10 files changed, 325 insertions(+), 16 deletions(-) diff --git a/crates/irtt-client/src/client/tests/config.rs b/crates/irtt-client/src/client/tests/config.rs index dcdcbee..07fe6d4 100644 --- a/crates/irtt-client/src/client/tests/config.rs +++ b/crates/irtt-client/src/client/tests/config.rs @@ -139,6 +139,16 @@ fn params_from_config_rejects_invalid_values() { }, "dscp", ), + ( + // The unspecified clock is the absent-tag wire default, not a + // client mode; a client that needs timestamps must name a clock. + "unspecified clock", + ClientConfig { + clock: Clock::Unspecified, + ..ClientConfig::default() + }, + "clock must be wall, monotonic, or both", + ), ( "empty server fill", ClientConfig { diff --git a/crates/irtt-client/src/config.rs b/crates/irtt-client/src/config.rs index cd69eb3..ca4505d 100644 --- a/crates/irtt-client/src/config.rs +++ b/crates/irtt-client/src/config.rs @@ -77,6 +77,10 @@ pub struct ClientConfig { /// Requested timestamp placement in echo replies. pub stamp_at: StampAt, /// Requested server clock sources for timestamp fields. + /// + /// [`Clock::Unspecified`] is the wire default meaning "no clock tag was + /// sent" and is not a client mode; opening a session with it is rejected. + /// Request [`Clock::Wall`], [`Clock::Monotonic`], or [`Clock::Both`]. pub clock: Clock, /// Requested DSCP codepoint. /// diff --git a/crates/irtt-client/src/session/machine.rs b/crates/irtt-client/src/session/machine.rs index 91c0255..a7a7775 100644 --- a/crates/irtt-client/src/session/machine.rs +++ b/crates/irtt-client/src/session/machine.rs @@ -1,7 +1,7 @@ use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use irtt_proto::{ - decode_echo_reply, echo_packet_len, encode_request, flags, EchoReply, OpenReply, Params, + decode_echo_reply, echo_packet_len, encode_request, flags, Clock, EchoReply, OpenReply, Params, RequestToEncode, ServerFill, TimestampFields, PROTOCOL_VERSION, }; @@ -989,6 +989,11 @@ fn validate_protocol_config(config: &ClientConfig) -> Result<(), ClientError> { reason: "interval must be greater than zero".to_owned(), }); } + if config.clock == Clock::Unspecified { + return Err(ClientError::InvalidConfig { + reason: "clock must be wall, monotonic, or both".to_owned(), + }); + } if config.dscp > MAX_DSCP_CODEPOINT { return Err(ClientError::InvalidConfig { reason: format!("dscp must be <= {MAX_DSCP_CODEPOINT}"), diff --git a/crates/irtt-client/tests/protocol_options.rs b/crates/irtt-client/tests/protocol_options.rs index 18b987b..c43ca36 100644 --- a/crates/irtt-client/tests/protocol_options.rs +++ b/crates/irtt-client/tests/protocol_options.rs @@ -121,6 +121,7 @@ fn clock_modes_drive_reply_event_clock_fields() { assert_timing_fields(reply.server_timing, true, true, false, false, true, true); assert_one_way_presence(reply.one_way, true, true); } + Clock::Unspecified => unreachable!("clients never request an unspecified clock"), } assert_processing_subtracted(reply); } diff --git a/crates/irtt-client/tests/protocol_options/backend_smoke.rs b/crates/irtt-client/tests/protocol_options/backend_smoke.rs index 3607b9e..ea89459 100644 --- a/crates/irtt-client/tests/protocol_options/backend_smoke.rs +++ b/crates/irtt-client/tests/protocol_options/backend_smoke.rs @@ -127,6 +127,9 @@ fn backend_clock_smoke() { assert!(st.send_wall_ns.is_some()); assert!(st.send_mono_ns.is_some()); } + Clock::Unspecified => { + unreachable!("clients never request an unspecified clock") + } } } other => panic!("expected EchoReply, got {other:?}"), diff --git a/crates/irtt-proto/src/echo.rs b/crates/irtt-proto/src/echo.rs index bcedbbe..a17731d 100644 --- a/crates/irtt-proto/src/echo.rs +++ b/crates/irtt-proto/src/echo.rs @@ -13,8 +13,13 @@ use crate::{ /// header. const MIDPOINT_COMPAT_EXTRA: usize = TIMESTAMP_SIZE; +/// Whether the negotiation selects a midpoint timestamp from exactly one clock, +/// which is the only case the upstream dual-field compatibility form applies to. +/// +/// [`Clock::Unspecified`] selects no clock and therefore lays out no midpoint +/// field at all, so it is not a single-clock negotiation. fn is_midpoint_single_clock(params: &Params) -> bool { - params.stamp_at == StampAt::Midpoint && !matches!(params.clock, Clock::Both) + params.stamp_at == StampAt::Midpoint && matches!(params.clock, Clock::Wall | Clock::Monotonic) } /// Total datagram length of the upstream dual-midpoint compatibility form. @@ -135,7 +140,7 @@ pub fn decode_echo_reply( match params.clock { Clock::Wall => (Some(wall), None), Clock::Monotonic => (None, Some(mono)), - Clock::Both => { + Clock::Unspecified | Clock::Both => { unreachable!("midpoint compat only applies to single-clock negotiations") } } diff --git a/crates/irtt-proto/src/layout.rs b/crates/irtt-proto/src/layout.rs index 02b41e3..e712f18 100644 --- a/crates/irtt-proto/src/layout.rs +++ b/crates/irtt-proto/src/layout.rs @@ -142,6 +142,7 @@ mod tests { ReceivedStats::Both => RECV_COUNT_SIZE + RECV_WINDOW_SIZE, }; let clock_count = match clock { + Clock::Unspecified => 0, Clock::Wall | Clock::Monotonic => 1, Clock::Both => 2, }; diff --git a/crates/irtt-proto/src/lib.rs b/crates/irtt-proto/src/lib.rs index 646c579..b1ebc17 100644 --- a/crates/irtt-proto/src/lib.rs +++ b/crates/irtt-proto/src/lib.rs @@ -12,6 +12,12 @@ //! performs no additional validation, so callers that build `Params` manually //! are responsible for validating those values before sending them. //! +//! Parameters are optional on the wire, and an absent tag takes its wire +//! default — zero for every integer field, and [`Clock::Unspecified`] for the +//! clock. A receiver that must distinguish an omitted tag from one explicitly +//! encoded as zero uses [`Params::decode_with_presence`], which runs the same +//! parser and additionally reports a [`ParamPresence`]. +//! //! The crate also provides packet layout calculation, packet encoding and //! decoding, and optional HMAC placement, computation, and verification //! helpers. @@ -51,7 +57,10 @@ pub use flags::*; pub use hmac::{compute_hmac, compute_hmac_in_place, verify_hmac, verify_packet_hmac}; pub use layout::{echo_header_len, echo_packet_len, PacketLayout}; pub use open::{decode_open_reply, encode_open_reply, OpenReply}; -pub use params::{Clock, Params, ReceivedStats, ServerFill, StampAt, MAX_SERVER_FILL_BYTES}; +pub use params::{ + Clock, DecodedParams, ParamPresence, Params, ReceivedStats, ServerFill, StampAt, + MAX_SERVER_FILL_BYTES, +}; pub use request::{ decode_request, encode_request, DecodedRequest, DecodedRequestKind, RequestToEncode, }; diff --git a/crates/irtt-proto/src/params.rs b/crates/irtt-proto/src/params.rs index 86ddcb3..6ead82d 100644 --- a/crates/irtt-proto/src/params.rs +++ b/crates/irtt-proto/src/params.rs @@ -11,6 +11,11 @@ pub const MAX_SERVER_FILL_BYTES: usize = 32; /// Higher-level callers should validate user or configuration input before /// encoding. The normal `irtt-client` configuration path enforces /// [`MAX_SERVER_FILL_BYTES`] for `server_fill`. +/// +/// [`Params::default`] is the **wire** default: every integer field is zero and +/// [`clock`](Params::clock) is [`Clock::Unspecified`], which is what an open +/// request with an empty parameter payload means. It is not a set of sensible +/// client settings; a client builds its request from its own configuration. #[derive(Debug, Clone, PartialEq, Eq, Default)] pub struct Params { pub protocol_version: i64, @@ -63,21 +68,67 @@ impl Params { /// /// This includes invalid enum values, malformed UTF-8, and `server_fill` /// values longer than [`MAX_SERVER_FILL_BYTES`]. + /// + /// Absent parameters take their wire default, so a caller cannot tell an + /// omitted tag from one explicitly encoded as zero. Use + /// [`decode_with_presence`](Params::decode_with_presence) when that + /// distinction matters; both share one parser. pub fn decode(input: &[u8]) -> Result { + Self::decode_with_presence(input).map(|decoded| decoded.params) + } + + /// Decodes parameters and additionally reports which known tags appeared. + /// + /// This is the same parser and the same validation as [`decode`], with the + /// presence of each known tag retained. A receiver needs it because the + /// protocol gives an omitted tag and an explicit zero the same value but + /// not the same meaning: an absent Duration or Interval is accepted as the + /// wire default zero, while one explicitly encoded as zero is invalid. + /// + /// Presence means the tag appeared at least once. Repeated known tags keep + /// last-value-wins, and unknown tags remain ignored and untracked. + /// + /// [`decode`]: Params::decode + pub fn decode_with_presence(input: &[u8]) -> Result { let mut params = Self::default(); + let mut presence = ParamPresence::default(); let mut pos = 0; while pos < input.len() { let (tag, used) = varint::decode_uvarint(&input[pos..])?; pos += used; match tag { - 1 => params.protocol_version = read_int(input, &mut pos)?, - 2 => params.duration_ns = read_int(input, &mut pos)?, - 3 => params.interval_ns = read_int(input, &mut pos)?, - 4 => params.length = read_int(input, &mut pos)?, - 5 => params.received_stats = ReceivedStats::try_from(read_int(input, &mut pos)?)?, - 6 => params.stamp_at = StampAt::try_from(read_int(input, &mut pos)?)?, - 7 => params.clock = Clock::try_from(read_int(input, &mut pos)?)?, - 8 => params.dscp = read_int(input, &mut pos)?, + 1 => { + params.protocol_version = read_int(input, &mut pos)?; + presence.protocol_version = true; + } + 2 => { + params.duration_ns = read_int(input, &mut pos)?; + presence.duration_ns = true; + } + 3 => { + params.interval_ns = read_int(input, &mut pos)?; + presence.interval_ns = true; + } + 4 => { + params.length = read_int(input, &mut pos)?; + presence.length = true; + } + 5 => { + params.received_stats = ReceivedStats::try_from(read_int(input, &mut pos)?)?; + presence.received_stats = true; + } + 6 => { + params.stamp_at = StampAt::try_from(read_int(input, &mut pos)?)?; + presence.stamp_at = true; + } + 7 => { + params.clock = Clock::try_from(read_int(input, &mut pos)?)?; + presence.clock = true; + } + 8 => { + params.dscp = read_int(input, &mut pos)?; + presence.dscp = true; + } 9 => { let (len, used) = varint::decode_uvarint(&input[pos..])?; pos += used; @@ -97,6 +148,7 @@ impl Params { .to_owned(); pos += len; params.server_fill = Some(ServerFill { value }); + presence.server_fill = true; } _ => { let (_, used) = varint::decode_uvarint(&input[pos..])?; @@ -104,10 +156,38 @@ impl Params { } } } - Ok(params) + Ok(DecodedParams { params, presence }) } } +/// Decoded parameters together with which known tags the payload carried. +/// +/// Produced by [`Params::decode_with_presence`]. +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub struct DecodedParams { + /// The decoded values. Absent parameters hold their wire default. + pub params: Params, + /// Which known tags appeared in the payload. + pub presence: ParamPresence, +} + +/// Which known parameter tags a decoded payload carried. +/// +/// A field is `true` when its tag appeared at least once, whatever value it +/// carried. Unknown tags are ignored and are not represented here. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub struct ParamPresence { + pub protocol_version: bool, + pub duration_ns: bool, + pub interval_ns: bool, + pub length: bool, + pub received_stats: bool, + pub stamp_at: bool, + pub clock: bool, + pub dscp: bool, + pub server_fill: bool, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct ServerFill { pub value: String, @@ -179,12 +259,31 @@ impl TryFrom for StampAt { } } +/// Which server clock sources supply timestamp fields. +/// +/// # The zero value +/// +/// [`Clock::Unspecified`] is the **wire default**, meaning the Clock tag was +/// absent from an open parameter payload — which is valid, and is what an empty +/// payload decodes to. It does not mean a peer may send an explicit Clock tag +/// encoding zero: [`TryFrom`](Clock::try_from) still rejects an explicit +/// zero as an invalid enum value, so this state is only ever reached by +/// omission. [`encode`](Params::encode) omits the tag for `Unspecified` rather +/// than emitting an explicit zero, which round-trips absence faithfully. +/// +/// `Unspecified` selects no clock, so [`has_wall`](Clock::has_wall) and +/// [`has_mono`](Clock::has_mono) are both false for it and no timestamp field +/// is laid out. A client that wants timestamps must request a real clock; the +/// `irtt-client` configuration path rejects `Unspecified`. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] #[repr(i64)] pub enum Clock { + /// Wire default for an absent Clock tag. Never produced from an explicit + /// encoded value. + #[default] + Unspecified = 0, Wall = 1, Monotonic = 2, - #[default] Both = 3, } @@ -201,6 +300,11 @@ impl Clock { impl TryFrom for Clock { type Error = ProtoError; + /// Converts an **explicitly encoded** Clock value. + /// + /// Zero is rejected: it is only valid as an absent tag, never as an encoded + /// one. [`Clock::Unspecified`] is therefore unreachable through this + /// conversion by design. fn try_from(value: i64) -> Result { match value { 1 => Ok(Self::Wall), @@ -244,6 +348,13 @@ mod tests { encoded } + fn encode_int(tag: u64, value: i64) -> Vec { + let mut encoded = Vec::new(); + varint::encode_uvarint(tag, &mut encoded); + varint::encode_varint(value, &mut encoded); + encoded + } + #[test] fn params_round_trip() { let params = Params { @@ -452,6 +563,166 @@ mod tests { value: 0, }) ); + assert_eq!( + Params::decode_with_presence(&encoded), + Err(ProtoError::InvalidEnum { + name: "Clock", + value: 0, + }) + ); + } + + #[test] + fn explicit_clock_values_one_to_three_are_accepted() { + for (value, expected) in [(1, Clock::Wall), (2, Clock::Monotonic), (3, Clock::Both)] { + let decoded = Params::decode_with_presence(&encode_int(7, value)).unwrap(); + assert_eq!(decoded.params.clock, expected); + assert!(decoded.presence.clock); + } + } + + #[test] + fn empty_payload_decodes_to_wire_defaults_with_nothing_present() { + let decoded = Params::decode_with_presence(&[]).unwrap(); + + assert_eq!(decoded.params, Params::default()); + assert_eq!(decoded.params.protocol_version, 0); + assert_eq!(decoded.params.duration_ns, 0); + assert_eq!(decoded.params.interval_ns, 0); + assert_eq!(decoded.params.length, 0); + assert_eq!(decoded.params.dscp, 0); + assert_eq!(decoded.params.received_stats, ReceivedStats::None); + assert_eq!(decoded.params.stamp_at, StampAt::None); + // The wire default clock is zero, not `Both`. + assert_eq!(decoded.params.clock, Clock::Unspecified); + assert!(!decoded.params.clock.has_wall()); + assert!(!decoded.params.clock.has_mono()); + assert_eq!(decoded.params.server_fill, None); + + assert_eq!(decoded.presence, ParamPresence::default()); + assert_eq!( + decoded.presence, + ParamPresence { + protocol_version: false, + duration_ns: false, + interval_ns: false, + length: false, + received_stats: false, + stamp_at: false, + clock: false, + dscp: false, + server_fill: false, + } + ); + } + + #[test] + fn absent_and_explicitly_zero_parameters_share_a_value_but_not_a_presence() { + // Duration and Interval are the pair a server must tell apart: absent is + // accepted, explicitly zero is not. + let absent = Params::decode_with_presence(&encode_int(1, 1)).unwrap(); + assert_eq!(absent.params.duration_ns, 0); + assert!(!absent.presence.duration_ns); + assert_eq!(absent.params.interval_ns, 0); + assert!(!absent.presence.interval_ns); + assert_eq!(absent.params.clock, Clock::Unspecified); + assert!(!absent.presence.clock); + + let explicit_duration = Params::decode_with_presence(&encode_int(2, 0)).unwrap(); + assert_eq!(explicit_duration.params.duration_ns, 0); + assert!(explicit_duration.presence.duration_ns); + + let explicit_interval = Params::decode_with_presence(&encode_int(3, 0)).unwrap(); + assert_eq!(explicit_interval.params.interval_ns, 0); + assert!(explicit_interval.presence.interval_ns); + } + + #[test] + fn every_known_tag_reports_its_own_presence() { + let mut encoded = Vec::new(); + for (tag, value) in [ + (1, 1), + (2, 5), + (3, 6), + (4, 7), + (5, 3), + (6, 4), + (7, 3), + (8, 184), + ] { + encoded.extend_from_slice(&encode_int(tag, value)); + } + encoded.extend_from_slice(&encode_server_fill_value(b"rand")); + + let decoded = Params::decode_with_presence(&encoded).unwrap(); + assert_eq!( + decoded.presence, + ParamPresence { + protocol_version: true, + duration_ns: true, + interval_ns: true, + length: true, + received_stats: true, + stamp_at: true, + clock: true, + dscp: true, + server_fill: true, + } + ); + } + + #[test] + fn unspecified_clock_is_encoded_by_omission() { + let params = Params { + protocol_version: 1, + clock: Clock::Unspecified, + ..Params::default() + }; + let encoded = params.encode(); + + assert_eq!(encoded, encode_int(1, 1)); + assert!( + !encoded.windows(2).any(|bytes| bytes[0] == 7), + "an unspecified clock must omit the tag rather than encode an explicit zero" + ); + assert_round_trip(params); + } + + #[test] + fn decode_and_decode_with_presence_agree() { + let mut payloads = vec![ + Vec::new(), + encode_int(1, 1), + encode_int(2, 0), + encode_server_fill_value(b"rand"), + ]; + payloads.push( + Params { + protocol_version: 1, + duration_ns: 3_000_000_000, + interval_ns: 1_000_000_000, + length: 1472, + received_stats: ReceivedStats::Both, + stamp_at: StampAt::Both, + clock: Clock::Both, + dscp: 184, + server_fill: Some(ServerFill { + value: "rand".to_owned(), + }), + } + .encode(), + ); + // Malformed payloads must fail identically through both entry points. + payloads.push(vec![1, 0x80]); + payloads.push(encode_int(7, 0)); + + for payload in payloads { + assert_eq!( + Params::decode(&payload), + Params::decode_with_presence(&payload).map(|decoded| decoded.params), + "decode and decode_with_presence disagreed on {payload:02x?}" + ); + } } #[test] diff --git a/crates/irtt-proto/tests/directional_codecs.rs b/crates/irtt-proto/tests/directional_codecs.rs index b0cbd71..cfdd467 100644 --- a/crates/irtt-proto/tests/directional_codecs.rs +++ b/crates/irtt-proto/tests/directional_codecs.rs @@ -413,7 +413,7 @@ fn midpoint_dual_field_reply_is_accepted_wherever_length_identifies_it() { // The first wire field must not be mistaken for the // negotiated monotonic value. Clock::Monotonic => (None, Some(222)), - Clock::Both => unreachable!(), + Clock::Unspecified | Clock::Both => unreachable!(), }; assert_eq!( reply.timestamps.midpoint_wall, expected_wall, @@ -463,7 +463,7 @@ fn midpoint_dual_field_reply_of_equal_length_is_accepted_but_not_correctable() { assert_eq!(reply.timestamps.midpoint_wall, None); assert_eq!(reply.timestamps.midpoint_mono, Some(111)); } - Clock::Both => unreachable!(), + Clock::Unspecified | Clock::Both => unreachable!(), } } } -- 2.54.0 From 59f6e8800576d5a9b1546a632cdfdcbcb34a6347 Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Wed, 12 Aug 2026 00:46:17 +0700 Subject: [PATCH 2/3] feat(server): add open session core Add `irtt-server` with the deterministic protocol and session engine the upcoming Tokio runtime will call. `ServerCore::handle_datagram` takes a source endpoint and the bytes of one datagram and returns the bytes to send back, or nothing. It performs no I/O and keeps no clock, so every decision is a pure function of its state and that input apart from drawing session tokens. Rejection is silence. The protocol defines no error reply, no reset and no NACK, so malformed, unauthenticated and policy-refused datagrams are discarded without a response and without disturbing any live session; a client tells "rejected" from "lost" only by timing out. `ServerError` is reserved for internal failures on the server's own side, so an inbound `ProtoError` never surfaces as a server failure. Open handling authenticates before decoding parameters, so a packet that fails authentication is discarded without parsing untrusted data and no reply behavior reveals which stage rejected it. Duration and Interval are accepted when absent and rejected when explicitly present and not positive, which is what the presence-aware parameter decode is for. Negotiation currently rewrites only the protocol version; the restriction knobs the specification describes are deliberately left for their own slices, with one obvious place to add them. Sessions are bounded to a finite default. A single unauthenticated datagram creates one and opens are never deduplicated, so an unbounded table is a liability rather than a compatibility feature; upstream's absence of session bounds is explicitly not a target. A full table drops new opens silently and evicts nothing. No-test opens keep working at capacity because they create no state. Session tokens come from the operating system's random source, since a token is the only thing standing between an off-path attacker and a usable session on an unauthenticated server. A drawn value that is zero or collides with a live session is redrawn within a finite budget; there is no predictable fallback, and a colliding token never overwrites the session that holds it. The reply is encoded before the session is inserted, so an internal failure cannot leave a half-created session behind. Echo and close requests pass admission and are then deliberately ignored: this slice creates sessions, and lifecycle and receive semantics belong to the slices that implement them. Tests pin that so a half-implemented lifecycle cannot appear by accident. --- AGENTS.md | 8 +- Cargo.lock | 9 + Cargo.toml | 2 + crates/irtt-server/AGENTS.md | 77 +++++++ crates/irtt-server/Cargo.toml | 17 ++ crates/irtt-server/src/config.rs | 69 ++++++ crates/irtt-server/src/core.rs | 229 ++++++++++++++++++++ crates/irtt-server/src/error.rs | 40 ++++ crates/irtt-server/src/lib.rs | 53 +++++ crates/irtt-server/src/negotiate.rs | 24 ++ crates/irtt-server/src/session.rs | 41 ++++ crates/irtt-server/src/tests/hmac.rs | 156 +++++++++++++ crates/irtt-server/src/tests/mod.rs | 18 ++ crates/irtt-server/src/tests/no_test.rs | 153 +++++++++++++ crates/irtt-server/src/tests/open.rs | 197 +++++++++++++++++ crates/irtt-server/src/tests/params.rs | 158 ++++++++++++++ crates/irtt-server/src/tests/sessions.rs | 96 ++++++++ crates/irtt-server/src/tests/support.rs | 179 +++++++++++++++ crates/irtt-server/src/tests/tokens.rs | 86 ++++++++ crates/irtt-server/src/tests/unsupported.rs | 93 ++++++++ crates/irtt-server/src/token.rs | 43 ++++ 21 files changed, 1744 insertions(+), 4 deletions(-) create mode 100644 crates/irtt-server/AGENTS.md create mode 100644 crates/irtt-server/Cargo.toml create mode 100644 crates/irtt-server/src/config.rs create mode 100644 crates/irtt-server/src/core.rs create mode 100644 crates/irtt-server/src/error.rs create mode 100644 crates/irtt-server/src/lib.rs create mode 100644 crates/irtt-server/src/negotiate.rs create mode 100644 crates/irtt-server/src/session.rs create mode 100644 crates/irtt-server/src/tests/hmac.rs create mode 100644 crates/irtt-server/src/tests/mod.rs create mode 100644 crates/irtt-server/src/tests/no_test.rs create mode 100644 crates/irtt-server/src/tests/open.rs create mode 100644 crates/irtt-server/src/tests/params.rs create mode 100644 crates/irtt-server/src/tests/sessions.rs create mode 100644 crates/irtt-server/src/tests/support.rs create mode 100644 crates/irtt-server/src/tests/tokens.rs create mode 100644 crates/irtt-server/src/tests/unsupported.rs create mode 100644 crates/irtt-server/src/token.rs diff --git a/AGENTS.md b/AGENTS.md index 013b9d8..8a15cda 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -39,7 +39,7 @@ For compatibility questions, the verified spec and black-box observations define ## Architecture -Main boundary (`irtt-server` is planned, not yet implemented): +Main boundary: > `irtt-proto` — wire semantics shared by client/server. `irtt-client` — client/session behavior. `irtt-server` — server/session behavior and Tokio runtime/orchestration. `irtt-stats` — statistics over client-facing events/results as currently designed. `irtt-cli` — presentation/orchestration. @@ -49,7 +49,7 @@ Main boundary (`irtt-server` is planned, not yet implemented): - `AsyncClient`: optional Tokio low-level adapter. - `ManagedClientTask` / `ManagedClientHandle`: unified Tokio managed driver/control surface. - `BlockingManagedClient`: synchronous owner using a dedicated current-thread Tokio runtime. -- `irtt-server`: planned; see "Server architecture (`irtt-server`)" below. +- `irtt-server`: exists; see "Server architecture (`irtt-server`)" below. - `irtt-stats`: cumulative/rolling/finite statistics over client events. No wire/session state machine logic. - `irtt-cli`: argument parsing, managed orchestration, output, summaries, TUI, diagnostics, and exit behavior. Do not duplicate protocol/session state machines here. @@ -57,7 +57,7 @@ Tokio must remain optional for `irtt-client`; default builds must remain runtime ### Server architecture (`irtt-server`) -`irtt-server` is a planned first-class reusable server crate. It does not exist yet; this section describes the intended boundary for it, not an implemented API. +`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. - 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. @@ -67,7 +67,7 @@ Tokio must remain optional for `irtt-client`; default builds must remain runtime - Runtime/server orchestration belongs around that core and may freely use Tokio for UDP sockets, `recv_from`/`send_to`, IPv4/IPv6 listeners, timers, session expiry, max-duration deadlines, shutdown, bounded control/config channels where useful, and `select!`-style orchestration. - The CLI server applet, when added, should remain thin orchestration/configuration over reusable `irtt-server`, not contain a second server state machine. - Resource policy: the server should use bounded session/resource policy. Upstream's observed lack of useful default session/per-peer bounds (see `IRTT_SERVER_PROTOCOL_SPEC.md`) is not an interoperability requirement and must not be reproduced merely for compatibility. -- Testing: once a real `irtt-server` exists, prefer it for normal compliant fake-peer behavior where practical. Keep narrow adversarial/raw peers for intentionally malformed, ambiguous, or non-compliant wire behavior. +- Testing: prefer `irtt-server` for normal compliant fake-peer behavior where practical, as far as its implemented behavior reaches. Keep narrow adversarial/raw peers for intentionally malformed, ambiguous, or non-compliant wire behavior. ## Engineering rules diff --git a/Cargo.lock b/Cargo.lock index 66ef0f5..590c779 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -726,6 +726,15 @@ dependencies = [ "thiserror 2.0.20", ] +[[package]] +name = "irtt-server" +version = "0.4.0" +dependencies = [ + "getrandom 0.4.3", + "irtt-proto", + "thiserror 2.0.20", +] + [[package]] name = "irtt-stats" version = "0.4.0" diff --git a/Cargo.toml b/Cargo.toml index 7a11ed4..57f47b0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,6 +2,7 @@ members = [ "crates/irtt-proto", "crates/irtt-client", + "crates/irtt-server", "crates/irtt-stats", "crates/irtt-cli", ] @@ -18,6 +19,7 @@ readme = "README.md" [workspace.dependencies] irtt-proto = { version = "0.4.0", path = "crates/irtt-proto" } irtt-client = { version = "0.4.0", path = "crates/irtt-client" } +irtt-server = { version = "0.4.0", path = "crates/irtt-server" } irtt-stats = { version = "0.4.0", path = "crates/irtt-stats" } [profile.release] diff --git a/crates/irtt-server/AGENTS.md b/crates/irtt-server/AGENTS.md new file mode 100644 index 0000000..4def913 --- /dev/null +++ b/crates/irtt-server/AGENTS.md @@ -0,0 +1,77 @@ +# `irtt-server` guidance + +Repository-wide guidance in the root `AGENTS.md` also applies here. + +## Where server behavior comes from + +Server wire behavior comes from the sanitized clean server documents, not from +upstream implementation material: + +- `docs/protocol/IRTT_SERVER_PROTOCOL_SPEC.md` +- `docs/protocol/test-vectors/SERVER_BEHAVIORAL_VECTORS.md` +- `docs/protocol/BLACKBOX_VERIFICATION_REPORT.md` +- `docs/protocol/IRTT_CLIENT_PROTOCOL_SPEC.md` for the shared wire format + +Never derive behavior from upstream `heistp/irtt` source or tests. If the clean +material does not establish a semantic, state the ambiguity instead of guessing. + +## Boundaries + +- Protocol behavior comes from `irtt-proto`. `irtt-server` must **not** depend + on `irtt-client` to reuse wire logic; a needed primitive belongs in + `irtt-proto`, where both sides get it. +- Do not reimplement magic validation, flag rules, HMAC placement, request + classification, token/header offsets or parameter parsing here. +- No statistics, CLI or presentation concerns. +- `SocketAddr` belongs here: endpoint identity is session state. It does not + belong in `irtt-proto`. + +## Core versus runtime + +- Deterministic protocol and session logic — admission, negotiation, the session + table, resource decisions, packet construction — belongs in the core. +- Socket and runtime orchestration — `UdpSocket`, IPv4/IPv6 listeners, + `recv_from`/`send_to`, timers, expiry, shutdown, `select!`, socket options — + belongs around the core. +- The server is intentionally **Tokio-native**. Tokio does not need to be + optional for this crate. +- Do not add a blocking server, an alternate-runtime variant, a transport trait, + a runtime abstraction layer, or a mirrored client-style API family. There is + no product requirement for any of them, and `ServerCore` being public is not a + promise of runtime independence. + +## Rejection is silence + +Malformed, unauthenticated, unsupported and policy-refused input is discarded +without a reply and without disturbing any live session. The protocol defines no +error reply, no reset and no NACK. Reserve the error type for internal failures +on the server's own side; an inbound `ProtoError` must never surface as a server +failure. Hostile UDP must leave the server running. + +Do not let reply behavior reveal which stage rejected a datagram — in +particular, an authentication failure must be indistinguishable from an unknown +token. + +## Resource policy + +Total session and resource state must stay bounded. A single unauthenticated +datagram creates a session and opens are never deduplicated, so an unbounded +table is a liability. Upstream's observed absence of session and per-peer bounds +is explicitly **not** a compatibility target and must not be reproduced. + +Keep fallible preparation before irreversible state mutation: a reply is +prepared before a session is inserted, so an internal failure cannot leave a +half-created session. + +## Testing + +- Drive the core with real packets built by the production encoders. Do not + maintain shadow encoders for well-formed wire data. +- Hand-build packets only where the point is a payload a compliant encoder + cannot produce — truncated varints, out-of-range enums, corrupted MACs. +- Token generation is the only nondeterministic part; tests inject a scripted + source so identity, collisions and allocation failure are assertable. Keep + that seam private. +- As the server gains behavior, prefer it over fake peers for normal compliant + behavior in other crates' tests. Keep small raw/adversarial peers for + intentionally malformed or non-compliant wire behavior. diff --git a/crates/irtt-server/Cargo.toml b/crates/irtt-server/Cargo.toml new file mode 100644 index 0000000..3ad8b5a --- /dev/null +++ b/crates/irtt-server/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "irtt-server" +version = "0.4.0" +description = "IRTT-compatible Rust server library" +edition.workspace = true +license.workspace = true +rust-version.workspace = true +repository.workspace = true +homepage.workspace = true +readme.workspace = true +keywords = ["irtt", "networking", "latency", "server"] +categories = ["network-programming"] + +[dependencies] +irtt-proto.workspace = true +getrandom = "0.4.3" +thiserror = "2.0.20" diff --git a/crates/irtt-server/src/config.rs b/crates/irtt-server/src/config.rs new file mode 100644 index 0000000..591a78a --- /dev/null +++ b/crates/irtt-server/src/config.rs @@ -0,0 +1,69 @@ +/// Default bound on the number of simultaneously live sessions. +/// +/// This is `irtt-rs` resource policy, not an interoperability requirement. The +/// protocol has no rejection response, so a server refuses by staying silent +/// and the client sees an open timeout it already has to tolerate. Upstream was +/// observed to impose no session bound at all; the server specification +/// explicitly records that as an upstream policy choice which a compatible +/// server is not expected to reproduce. +pub const DEFAULT_MAX_SESSIONS: usize = 1024; + +/// Configuration for a [`ServerCore`](crate::ServerCore). +/// +/// Fields are private and set through consuming builder methods, so later +/// slices can add configuration without breaking construction. Only the +/// settings this OPEN/session slice actually uses exist yet; parameter +/// restriction knobs, lifetime policy and rate limits arrive with the slices +/// that implement them. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ServerConfig { + hmac_key: Option>, + max_sessions: usize, +} + +impl ServerConfig { + /// Sets the HMAC key, enabling authentication for this server. + /// + /// Authentication is global: with a key configured, every request must + /// carry `FLAG_HMAC` and a valid MAC, and every reply is authenticated. + /// Without one, any request carrying `FLAG_HMAC` is dropped. + #[must_use] + pub fn with_hmac_key(mut self, key: impl Into>) -> Self { + self.hmac_key = Some(key.into()); + self + } + + /// Sets the maximum number of simultaneously live sessions. + /// + /// Once the table holds this many sessions, a session-creating open is + /// silently dropped; nothing is evicted. Zero refuses every + /// session-creating open while still answering no-test opens, which create + /// no state. There is deliberately no unbounded setting. + #[must_use] + pub fn with_max_sessions(mut self, max_sessions: usize) -> Self { + self.max_sessions = max_sessions; + self + } + + /// The configured HMAC key, if this server authenticates. + #[must_use] + pub fn hmac_key(&self) -> Option<&[u8]> { + self.hmac_key.as_deref() + } + + /// The maximum number of simultaneously live sessions. + #[must_use] + pub fn max_sessions(&self) -> usize { + self.max_sessions + } +} + +impl Default for ServerConfig { + /// An unauthenticated server bounded to [`DEFAULT_MAX_SESSIONS`] sessions. + fn default() -> Self { + Self { + hmac_key: None, + max_sessions: DEFAULT_MAX_SESSIONS, + } + } +} diff --git a/crates/irtt-server/src/core.rs b/crates/irtt-server/src/core.rs new file mode 100644 index 0000000..f622d70 --- /dev/null +++ b/crates/irtt-server/src/core.rs @@ -0,0 +1,229 @@ +use std::{collections::HashMap, net::SocketAddr}; + +use irtt_proto::{ + decode_request, encode_open_reply, verify_packet_hmac, DecodedParams, DecodedRequestKind, + OpenReply, Params, FLAG_CLOSE, FLAG_OPEN, FLAG_REPLY, +}; + +use crate::{ + config::ServerConfig, + error::ServerError, + negotiate::negotiate_params, + session::Session, + token::{OsTokenSource, TokenSource, TOKEN_ATTEMPTS}, +}; + +/// The deterministic protocol and session engine of the server. +/// +/// `ServerCore` owns admission, negotiation, the session table and reply +/// construction. It performs no I/O and keeps no clock: a caller hands it a +/// source endpoint and the bytes of one received datagram, and gets back the +/// bytes to send to that endpoint, or nothing. Everything it does is a pure +/// function of its state and that input, except for drawing session tokens. +/// +/// The Tokio runtime slice will wrap it directly: +/// +/// ```text +/// recv_from -> core.handle_datagram(peer, packet) -> Some(reply) -> send_to +/// -> None -> send nothing +/// ``` +/// +/// It is public as a low-level engine for a pre-1.0 crate, not as a promise of +/// runtime independence. The server is intentionally Tokio-native; this is not +/// a transport abstraction, and there is no blocking or alternate-runtime +/// counterpart. +/// +/// # 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. +#[derive(Debug)] +pub struct ServerCore { + config: ServerConfig, + sessions: HashMap, + tokens: Box, +} + +impl ServerCore { + /// Creates a server core drawing session tokens from the operating system's + /// random source. + #[must_use] + pub fn new(config: ServerConfig) -> Self { + Self::with_token_source(config, Box::new(OsTokenSource)) + } + + pub(crate) fn with_token_source(config: ServerConfig, tokens: Box) -> Self { + Self { + config, + sessions: HashMap::new(), + tokens, + } + } + + /// This core's configuration. + #[must_use] + pub fn config(&self) -> &ServerConfig { + &self.config + } + + /// The number of live sessions. + #[must_use] + pub fn session_count(&self) -> usize { + self.sessions.len() + } + + /// Looks up a live session by token. + /// + /// 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. + #[must_use] + pub fn session(&self, token: u64) -> Option<&Session> { + self.sessions.get(&token) + } + + /// Handles one received datagram, returning the reply to send, if any. + /// + /// `Ok(None)` means "answer nothing". The protocol defines no error reply, + /// no reset and no NACK, so every rejection is a silent discard and a + /// client distinguishes "rejected" from "lost" only by timing out. A + /// datagram is discarded when it is structurally invalid, carries the reply + /// flag, disagrees with this server's authentication configuration, fails + /// authentication, carries a malformed or out-of-range open parameter + /// payload, would exceed the session bound, or is a request kind this slice + /// does not implement. None of that mutates any live session. + /// + /// 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 + /// reveal which stage rejected it. + /// + /// # Errors + /// + /// Returns [`ServerError`] only for an internal failure — the random source + /// failing, no unique token being found within the retry budget, or a + /// server-generated reply failing to encode. Rejected input is never an + /// error. + pub fn handle_datagram( + &mut self, + peer: SocketAddr, + packet: &[u8], + ) -> Result>, ServerError> { + let Ok(request) = decode_request(packet) else { + return Ok(None); + }; + // The HMAC flag must agree with this server's configuration in both + // directions of mismatch, and the MAC itself is verified separately. + if request.hmac_present != self.config.hmac_key().is_some() { + return Ok(None); + } + if let Some(key) = self.config.hmac_key() { + if verify_packet_hmac(key, packet).is_err() { + return Ok(None); + } + } + + 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), + } + } + + fn handle_open( + &mut self, + peer: SocketAddr, + no_test: bool, + encoded_params: &[u8], + ) -> Result>, ServerError> { + // The parameter decoder already rejects a truncated tag or value, a + // varint overflow, an out-of-range stats/stamp-at/clock enum, and a + // server-fill value that is oversized, short-buffered or not UTF-8. + let Ok(decoded) = Params::decode_with_presence(encoded_params) else { + return Ok(None); + }; + if !open_params_are_admissible(&decoded) { + return Ok(None); + } + let params = negotiate_params(decoded.params, &self.config); + + if no_test { + // No-test validates parameters without running a test: same + // negotiation, a zero token, and no state at all. It stays + // serviceable at capacity precisely because it creates nothing. + let reply = OpenReply { + flags: FLAG_OPEN | FLAG_REPLY | FLAG_CLOSE, + token: 0, + params, + }; + return self.encode_reply(&reply).map(Some); + } + + if self.sessions.len() >= self.config.max_sessions() { + return Ok(None); + } + + // Every fallible step runs before the table is touched, so an internal + // failure cannot leave a half-created session behind. + let token = self.allocate_token()?; + let reply = OpenReply { + flags: FLAG_OPEN | FLAG_REPLY, + token, + params, + }; + let packet = self.encode_reply(&reply)?; + + // The reply is encoded before its params move into the session, so + // nothing is cloned to keep both. + self.sessions + .insert(token, Session::new(peer, reply.params)); + Ok(Some(packet)) + } + + /// 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()) + .map_err(|source| ServerError::ReplyEncoding { source }) + } + + /// Draws a token that is non-zero and unique among live sessions. + /// + /// Zero is reserved for no-test replies and is never issued to a session. A + /// value colliding with a live token is discarded rather than allowed to + /// overwrite that session. Both cases redraw, up to [`TOKEN_ATTEMPTS`]. + fn allocate_token(&mut self) -> Result { + for _ in 0..TOKEN_ATTEMPTS { + let token = self.tokens.next_token()?; + if token != 0 && !self.sessions.contains_key(&token) { + return Ok(token); + } + } + Err(ServerError::TokenExhausted { + attempts: TOKEN_ATTEMPTS, + }) + } +} + +/// Applies the open parameter rules the decoder cannot express. +/// +/// Duration and Interval are accepted when absent, taking the wire default +/// zero, and rejected when explicitly present and not positive. The presence +/// distinction is why the decoder reports it: testing the value alone would +/// wrongly reject an open that simply omitted the tag, which includes the empty +/// parameter payload. +/// +/// Every other known parameter is already validated or deliberately +/// unrestricted here. Protocol version is accepted at any value and rewritten +/// during negotiation. Length and DSCP are accepted as decoded, including zero, +/// negative and out-of-byte-range values; a later slice may restrict a DSCP it +/// cannot actually apply to the socket. Unknown tags were ignored by the +/// decoder and are not reflected in the reply. +fn open_params_are_admissible(decoded: &DecodedParams) -> bool { + let admissible = |present: bool, value: i64| !present || value > 0; + admissible(decoded.presence.duration_ns, decoded.params.duration_ns) + && admissible(decoded.presence.interval_ns, decoded.params.interval_ns) +} diff --git a/crates/irtt-server/src/error.rs b/crates/irtt-server/src/error.rs new file mode 100644 index 0000000..699fb76 --- /dev/null +++ b/crates/irtt-server/src/error.rs @@ -0,0 +1,40 @@ +use irtt_proto::ProtoError; +use thiserror::Error; + +/// An internal server failure. +/// +/// This type deliberately does **not** describe rejected traffic. Malformed, +/// unauthenticated, unsupported and policy-refused datagrams are not errors: +/// the protocol defines no rejection response, so they are answered with +/// silence and reported as `Ok(None)` from +/// [`ServerCore::handle_datagram`](crate::ServerCore::handle_datagram). An +/// inbound [`ProtoError`] never reaches a caller — that is the whole point of +/// the admission boundary, which turns hostile UDP into a dropped datagram +/// rather than a failure. +/// +/// Every variant here is instead something wrong on *this* side, where the +/// server could not do its job for a request it had already accepted. +#[derive(Debug, Error, PartialEq, Eq)] +pub enum ServerError { + /// The operating system's random source failed, so no session token could + /// be drawn. + #[error("the operating system random source failed: {reason}")] + RandomSource { reason: String }, + /// No unique non-zero token was found within the retry budget. + /// + /// Each attempt drew a fresh value and rejected it as zero or as a + /// collision with a live session. With a working random source this is + /// vanishingly unlikely; it exists so token allocation cannot spin forever + /// on a degenerate source. + #[error("no unique non-zero session token after {attempts} attempts")] + TokenExhausted { attempts: u32 }, + /// A reply the server itself constructed could not be encoded. + /// + /// The `ProtoError` here always describes server-generated bytes, never + /// anything a peer sent. + #[error("could not encode a server-generated reply: {source}")] + ReplyEncoding { + #[source] + source: ProtoError, + }, +} diff --git a/crates/irtt-server/src/lib.rs b/crates/irtt-server/src/lib.rs new file mode 100644 index 0000000..b72ae86 --- /dev/null +++ b/crates/irtt-server/src/lib.rs @@ -0,0 +1,53 @@ +//! IRTT-compatible server behavior. +//! +//! This crate shares wire semantics with the client through `irtt-proto` and +//! does not depend on `irtt-client`. +//! +//! # Structure +//! +//! [`ServerCore`] is the deterministic protocol and session engine: packet +//! admission, authentication policy, open negotiation, the session table, +//! resource decisions and reply construction. It performs no I/O. Socket and +//! runtime orchestration — the UDP listener, address handling, timers, expiry +//! and shutdown — will live around it and is intentionally Tokio-native. There +//! is no blocking or alternate-runtime counterpart and no transport +//! abstraction. +//! +//! # Rejection is silence +//! +//! The protocol defines no error reply, no reset and no NACK. Malformed, +//! unauthenticated and policy-refused datagrams are discarded without a +//! response and without disturbing any live session, and a client tells +//! "rejected" from "lost" only by timing out. [`ServerError`] is therefore +//! reserved for internal failures on the server's own side. +//! +//! # Resource policy +//! +//! Total session state is bounded (see [`DEFAULT_MAX_SESSIONS`]). A single +//! unauthenticated datagram creates a session and opens are never deduplicated, +//! so an unbounded table is a liability rather than a compatibility feature. +//! Upstream's observed lack of session bounds is explicitly not a compatibility +//! target. +//! +//! # 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. +#![forbid(unsafe_code)] + +mod config; +mod core; +mod error; +mod negotiate; +mod session; +mod token; + +#[cfg(test)] +mod tests; + +pub use config::{ServerConfig, DEFAULT_MAX_SESSIONS}; +pub use core::ServerCore; +pub use error::ServerError; +pub use session::Session; diff --git a/crates/irtt-server/src/negotiate.rs b/crates/irtt-server/src/negotiate.rs new file mode 100644 index 0000000..6485153 --- /dev/null +++ b/crates/irtt-server/src/negotiate.rs @@ -0,0 +1,24 @@ +use irtt_proto::{Params, PROTOCOL_VERSION}; + +use crate::config::ServerConfig; + +/// Produces the parameters the server returns in an open reply and will enforce +/// for the session. +/// +/// This slice applies the minimum the specification requires: the requested +/// known parameters, with the protocol version rewritten to +/// [`PROTOCOL_VERSION`]. A server must return version 1 so that version-1 +/// clients accept the session; any requested value, including an absent, +/// zero, negative or future one, is accepted and answered with 1. Detecting a +/// version mismatch is therefore client-side only. +/// +/// Nothing else is restricted yet. A server *may* reduce or replace requested +/// parameters, and later policy slices will add the maximum test duration +/// clamp, minimum interval, idle-timeout interval cap, maximum packet length, +/// timestamp allowance, DSCP policy and server-fill allow-list. This function +/// is where they belong, which is why the configuration is threaded through +/// even though no knob reads it yet. +pub(crate) fn negotiate_params(mut requested: Params, _config: &ServerConfig) -> Params { + requested.protocol_version = PROTOCOL_VERSION; + requested +} diff --git a/crates/irtt-server/src/session.rs b/crates/irtt-server/src/session.rs new file mode 100644 index 0000000..9c7d7e7 --- /dev/null +++ b/crates/irtt-server/src/session.rs @@ -0,0 +1,41 @@ +use std::net::SocketAddr; + +use irtt_proto::Params; + +/// A live session created by an accepted open request. +/// +/// This slice records only what identifies the session and what was negotiated +/// for it. Echo receive state (counts, windows, sequence tracking) and lifetime +/// state (activity, deadlines, expiry) belong to the slices that implement +/// those behaviors, and are deliberately absent rather than present as +/// placeholders. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Session { + peer: SocketAddr, + params: Params, +} + +impl Session { + pub(crate) fn new(peer: SocketAddr, params: Params) -> Self { + Self { peer, params } + } + + /// 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. Echo and close processing, once + /// implemented, must require both a token match and an exact endpoint match + /// before touching a session. + #[must_use] + pub fn peer(&self) -> SocketAddr { + self.peer + } + + /// The negotiated parameters the server returned for this session and will + /// enforce for it. + #[must_use] + pub fn params(&self) -> &Params { + &self.params + } +} diff --git a/crates/irtt-server/src/tests/hmac.rs b/crates/irtt-server/src/tests/hmac.rs new file mode 100644 index 0000000..9970da5 --- /dev/null +++ b/crates/irtt-server/src/tests/hmac.rs @@ -0,0 +1,156 @@ +use irtt_proto::{verify_packet_hmac, FLAG_HMAC}; + +use super::support::{ + client_params, core_with_tokens, expect_normal_open_reply, open_request, + open_request_with_raw_params, param_int, peer, ScriptedTokens, KEY, OTHER_KEY, +}; +use crate::ServerConfig; + +const TOKEN_A: u64 = 0x0102_0304_0506_0708; + +/// Asserts a datagram is silently discarded, leaving no trace at all: no reply, +/// no session, and no session token drawn. +fn assert_discarded(config: ServerConfig, name: &str, packet: &[u8]) { + let tokens = ScriptedTokens::new([TOKEN_A]); + let mut core = core_with_tokens(config, tokens.clone()); + + let reply = core + .handle_datagram(peer(), packet) + .expect("a rejected datagram is not a server error"); + + assert!(reply.is_none(), "{name} must be answered with silence"); + assert_eq!(core.session_count(), 0, "{name} must create no session"); + assert_eq!( + tokens.remaining(), + 1, + "{name} must not consume session capacity or a token" + ); +} + +#[test] +fn an_authenticating_server_discards_everything_that_does_not_authenticate() { + let config = ServerConfig::default().with_hmac_key(KEY); + + assert_discarded( + config.clone(), + "open with no MAC field at all", + &open_request(&client_params(), None), + ); + assert_discarded( + config.clone(), + "open signed with a different key", + &open_request(&client_params(), Some(OTHER_KEY)), + ); + + let mut corrupted = open_request(&client_params(), Some(KEY)); + corrupted[5] ^= 0x80; + assert_discarded( + config.clone(), + "open with one bit flipped in the MAC", + &corrupted, + ); + + let mut zeroed = open_request(&client_params(), Some(KEY)); + zeroed[4..20].fill(0); + assert_discarded(config.clone(), "open with an all-zero MAC", &zeroed); + + let mut truncated = open_request(&client_params(), Some(KEY)); + truncated.drain(4..12); + assert_discarded( + config.clone(), + "open with a truncated MAC field", + &truncated, + ); + + let mut flag_cleared = open_request(&client_params(), Some(KEY)); + flag_cleared[3] &= !FLAG_HMAC; + assert_discarded( + config, + "open with a valid MAC but the HMAC flag cleared", + &flag_cleared, + ); +} + +#[test] +fn a_non_authenticating_server_discards_authenticated_requests() { + assert_discarded( + ServerConfig::default(), + "authenticated open to a server with no key", + &open_request(&client_params(), Some(KEY)), + ); +} + +#[test] +fn an_authenticating_server_answers_a_valid_open_with_an_authenticated_reply() { + let mut core = core_with_tokens( + ServerConfig::default().with_hmac_key(KEY), + ScriptedTokens::new([TOKEN_A]), + ); + + let packet = core + .handle_datagram(peer(), &open_request(&client_params(), Some(KEY))) + .unwrap() + .expect("a correctly authenticated open must be answered"); + + let reply = expect_normal_open_reply(&packet, Some(KEY)); + assert_eq!(reply.token, TOKEN_A); + assert_ne!(packet[3] & FLAG_HMAC, 0, "the reply must set FLAG_HMAC"); + verify_packet_hmac(KEY, &packet).expect("the reply must carry a valid MAC"); + assert_eq!(core.session_count(), 1); +} + +#[test] +fn a_non_authenticating_server_answers_an_ordinary_open() { + let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + + let packet = core + .handle_datagram(peer(), &open_request(&client_params(), None)) + .unwrap() + .expect("an unauthenticated open must be answered"); + + expect_normal_open_reply(&packet, None); + assert_eq!(packet[3] & FLAG_HMAC, 0); + assert_eq!(core.session_count(), 1); +} + +#[test] +fn authentication_is_checked_before_parameters_are_decoded() { + // Both of these are dropped, and neither reveals which stage rejected it. + // The pairing is the point: bad parameters under a valid MAC still drop, + // and a bad MAC drops without the parameters ever being parsed. + let config = ServerConfig::default().with_hmac_key(KEY); + let bad_params = param_int(2, 0); + + assert_discarded( + config.clone(), + "authenticated open with an explicit zero duration", + &open_request_with_raw_params(&bad_params, Some(KEY)), + ); + assert_discarded( + config, + "wrongly keyed open with an explicit zero duration", + &open_request_with_raw_params(&bad_params, Some(OTHER_KEY)), + ); +} + +#[test] +fn a_failed_authentication_leaves_a_live_session_untouched() { + let mut core = core_with_tokens( + ServerConfig::default().with_hmac_key(KEY), + ScriptedTokens::new([TOKEN_A]), + ); + core.handle_datagram(peer(), &open_request(&client_params(), Some(KEY))) + .unwrap() + .expect("the session-creating open must succeed"); + let session = core.session(TOKEN_A).cloned().expect("session must exist"); + + for bad in [ + open_request(&client_params(), None), + open_request(&client_params(), Some(OTHER_KEY)), + ] { + assert_eq!(core.handle_datagram(peer(), &bad).unwrap(), None); + } + + assert_eq!(core.session_count(), 1); + assert_eq!(core.session(TOKEN_A), Some(&session)); +} diff --git a/crates/irtt-server/src/tests/mod.rs b/crates/irtt-server/src/tests/mod.rs new file mode 100644 index 0000000..d820706 --- /dev/null +++ b/crates/irtt-server/src/tests/mod.rs @@ -0,0 +1,18 @@ +//! Deterministic tests for the server core. +//! +//! The core takes a source endpoint and datagram bytes and returns reply bytes, +//! so these drive it directly; no socket is bound. Requests are built with the +//! production encoders wherever the wire form is well-formed, and by hand only +//! where the point of the test is a payload a compliant encoder cannot produce. +//! +//! Tokens come from a scripted source so session identity, collisions and +//! allocation failure are all assertable. + +mod hmac; +mod no_test; +mod open; +mod params; +mod sessions; +mod support; +mod tokens; +mod unsupported; diff --git a/crates/irtt-server/src/tests/no_test.rs b/crates/irtt-server/src/tests/no_test.rs new file mode 100644 index 0000000..ea4602d --- /dev/null +++ b/crates/irtt-server/src/tests/no_test.rs @@ -0,0 +1,153 @@ +use irtt_proto::{verify_packet_hmac, Params}; + +use super::support::{ + client_params, core_with_tokens, expect_no_test_reply, no_test_request, open_request, + open_request_with_raw_params, param_int, peer, ScriptedTokens, KEY, +}; +use crate::ServerConfig; + +const TOKEN_A: u64 = 0x0102_0304_0506_0708; +const TOKEN_B: u64 = 0x1112_1314_1516_1718; + +#[test] +fn a_no_test_open_replies_with_a_zero_token_and_creates_nothing() { + let tokens = ScriptedTokens::new([TOKEN_A]); + let mut core = core_with_tokens(ServerConfig::default(), tokens.clone()); + + let packet = core + .handle_datagram(peer(), &no_test_request(&client_params(), None)) + .unwrap() + .expect("a no-test open must be answered"); + + let reply = expect_no_test_reply(&packet, None); + assert_eq!( + reply.params, + Params { + protocol_version: 1, + ..client_params() + } + ); + assert_eq!(core.session_count(), 0, "no-test creates no session"); + assert_eq!( + tokens.remaining(), + 1, + "no-test must not draw a session token" + ); +} + +#[test] +fn a_no_test_open_negotiates_exactly_as_a_normal_open_does() { + let requested = client_params(); + + let mut normal_core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + let normal = normal_core + .handle_datagram(peer(), &open_request(&requested, None)) + .unwrap() + .expect("normal open must be answered"); + + let mut no_test_core = + core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + let no_test = no_test_core + .handle_datagram(peer(), &no_test_request(&requested, None)) + .unwrap() + .expect("no-test open must be answered"); + + assert_eq!( + expect_no_test_reply(&no_test, None).params, + super::support::decode_reply(&normal, None).params + ); +} + +#[test] +fn a_no_test_open_is_rejected_on_the_same_parameters_a_normal_open_is() { + let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + + let request = { + let mut packet = no_test_request(&client_params(), None); + // Replace the parameter payload with an explicit zero duration. + packet.truncate(4); + packet.extend_from_slice(¶m_int(2, 0)); + packet + }; + + assert_eq!(core.handle_datagram(peer(), &request).unwrap(), None); + assert_eq!(core.session_count(), 0); +} + +#[test] +fn a_no_test_open_is_still_answered_when_no_session_could_be_created() { + // No-test consumes no capacity, so it must keep working exactly where a + // session-creating open would be refused. + let mut refuses_everything = core_with_tokens( + ServerConfig::default().with_max_sessions(0), + ScriptedTokens::new([TOKEN_A]), + ); + assert_eq!( + refuses_everything + .handle_datagram(peer(), &open_request(&client_params(), None)) + .unwrap(), + None, + "max_sessions 0 refuses a session-creating open" + ); + let packet = refuses_everything + .handle_datagram(peer(), &no_test_request(&client_params(), None)) + .unwrap() + .expect("no-test must be served at max_sessions 0"); + expect_no_test_reply(&packet, None); + + let mut full = core_with_tokens( + ServerConfig::default().with_max_sessions(1), + ScriptedTokens::new([TOKEN_A, TOKEN_B]), + ); + full.handle_datagram(peer(), &open_request(&client_params(), None)) + .unwrap() + .expect("the first open fills the table"); + assert_eq!(full.session_count(), 1); + + let packet = full + .handle_datagram(peer(), &no_test_request(&client_params(), None)) + .unwrap() + .expect("no-test must be served with a full session table"); + expect_no_test_reply(&packet, None); + assert_eq!(full.session_count(), 1, "no-test disturbed the table"); +} + +#[test] +fn an_authenticated_no_test_reply_is_authenticated_and_still_creates_nothing() { + let mut core = core_with_tokens( + ServerConfig::default().with_hmac_key(KEY), + ScriptedTokens::new([TOKEN_A]), + ); + + let packet = core + .handle_datagram(peer(), &no_test_request(&client_params(), Some(KEY))) + .unwrap() + .expect("an authenticated no-test open must be answered"); + + expect_no_test_reply(&packet, Some(KEY)); + verify_packet_hmac(KEY, &packet).expect("the no-test reply must carry a valid MAC"); + assert_eq!(core.session_count(), 0); +} + +#[test] +fn an_empty_no_test_open_is_answered() { + let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + + let mut request = open_request_with_raw_params(&[], None); + request[3] |= irtt_proto::FLAG_CLOSE; + + let packet = core + .handle_datagram(peer(), &request) + .unwrap() + .expect("an empty no-test open must be answered"); + + let reply = expect_no_test_reply(&packet, None); + assert_eq!( + reply.params, + Params { + protocol_version: 1, + ..Params::default() + } + ); + assert_eq!(core.session_count(), 0); +} diff --git a/crates/irtt-server/src/tests/open.rs b/crates/irtt-server/src/tests/open.rs new file mode 100644 index 0000000..c2876d7 --- /dev/null +++ b/crates/irtt-server/src/tests/open.rs @@ -0,0 +1,197 @@ +use irtt_proto::{Clock, Params, ReceivedStats, ServerFill, StampAt}; + +use super::support::{ + client_params, core_with_tokens, expect_normal_open_reply, open_request, + open_request_with_raw_params, param_int, peer, ScriptedTokens, +}; +use crate::ServerConfig; + +const TOKEN_A: u64 = 0x7896_b6ab_8771_5213; + +#[test] +fn an_ordinary_open_is_answered_and_creates_one_session() { + let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + + let packet = core + .handle_datagram(peer(), &open_request(&client_params(), None)) + .unwrap() + .expect("an ordinary open must be answered"); + + let reply = expect_normal_open_reply(&packet, None); + assert_eq!(reply.token, TOKEN_A); + assert_eq!(core.session_count(), 1); + + let session = core.session(TOKEN_A).expect("the session must be live"); + assert_eq!(session.params(), &reply.params); +} + +#[test] +fn a_session_records_the_exact_source_endpoint() { + // Address family, address, port and — where present — the IPv6 scope all + // form part of session identity, so the endpoint is stored verbatim rather + // than normalized to an address. + for endpoint in [ + "198.51.100.7:41234", + "198.51.100.7:41235", + "[2001:db8::1]:2112", + "[fe80::1%3]:2112", + ] { + let endpoint = endpoint.parse().unwrap(); + let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + + core.handle_datagram(endpoint, &open_request(&client_params(), None)) + .unwrap() + .expect("open must be answered"); + + assert_eq!(core.session(TOKEN_A).unwrap().peer(), endpoint); + } +} + +#[test] +fn any_requested_protocol_version_is_accepted_and_answered_with_one() { + // Version mismatch is detected client-side only: the server rewrites the + // version and creates a normal session whatever was asked for. + for requested in [Some(1), Some(0), Some(2), Some(-1), None] { + let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + let params = Params { + protocol_version: requested.unwrap_or(0), + ..client_params() + }; + let request = match requested { + // An explicit zero cannot be encoded by `Params::encode`, which + // omits zero-valued integers, so encode that tag by hand. + Some(0) => open_request_with_raw_params(¶m_int(1, 0), None), + Some(_) => open_request(¶ms, None), + None => open_request( + &Params { + protocol_version: 0, + ..params + }, + None, + ), + }; + + let packet = core + .handle_datagram(peer(), &request) + .unwrap() + .unwrap_or_else(|| panic!("protocol version {requested:?} must be answered")); + + let reply = expect_normal_open_reply(&packet, None); + assert_eq!(reply.params.protocol_version, 1); + assert_eq!(core.session_count(), 1); + } +} + +#[test] +fn an_empty_parameter_payload_is_answered_with_only_a_version() { + let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + + let packet = core + .handle_datagram(peer(), &open_request_with_raw_params(&[], None)) + .unwrap() + .expect("an empty parameter payload is a valid open"); + + // header (4) + token (8) + ProtocolVersion=1 (2). Every other parameter + // negotiates to its wire-default zero and is omitted entirely, which is + // only reachable if absent parameters really did decode as zero. + assert_eq!(packet.len(), 14); + + let reply = expect_normal_open_reply(&packet, None); + assert_eq!( + reply.params, + Params { + protocol_version: 1, + ..Params::default() + } + ); + assert_eq!(reply.params.clock, Clock::Unspecified); + assert_eq!(core.session_count(), 1); +} + +#[test] +fn unknown_parameter_tags_are_ignored_and_not_reflected_in_the_reply() { + let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + + let mut payload = param_int(1, 1); + payload.extend_from_slice(¶m_int(42, 7)); + payload.extend_from_slice(¶m_int(200, 9)); + + let packet = core + .handle_datagram(peer(), &open_request_with_raw_params(&payload, None)) + .unwrap() + .expect("unknown tags must not prevent an open"); + + let reply = expect_normal_open_reply(&packet, None); + assert_eq!( + reply.params, + Params { + protocol_version: 1, + ..Params::default() + } + ); + // The reply is the same 14 bytes an empty payload produces: no unknown tag + // was copied through. + assert_eq!(packet.len(), 14); + assert_eq!(core.session_count(), 1); +} + +#[test] +fn this_slice_restricts_nothing_but_the_protocol_version() { + // These are the values a later restriction-policy slice will deliberately + // start changing. Pinning them now makes that a visible decision rather + // than an accident. + let unrestricted = Params { + protocol_version: 2, + duration_ns: 3_600_000_000_000, + interval_ns: 1, + length: 65_535, + received_stats: ReceivedStats::Window, + stamp_at: StampAt::Midpoint, + clock: Clock::Monotonic, + dscp: 184, + server_fill: Some(ServerFill { + value: "pattern:abc".to_owned(), + }), + }; + + let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + let packet = core + .handle_datagram(peer(), &open_request(&unrestricted, None)) + .unwrap() + .expect("open must be answered"); + + let reply = expect_normal_open_reply(&packet, None); + assert_eq!( + reply.params, + Params { + protocol_version: 1, + ..unrestricted + } + ); +} + +#[test] +fn negative_and_out_of_range_length_and_dscp_survive_negotiation() { + // Length and DSCP are accepted as decoded in this slice, including values a + // socket could never carry. Restricting a DSCP the server cannot apply is a + // later policy decision, not an admission rule. + for (length, dscp) in [(-1, -1), (0, 0), (1472, 184), (-4096, 512)] { + let requested = Params { + protocol_version: 1, + length, + dscp, + ..Params::default() + }; + let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + + let packet = core + .handle_datagram(peer(), &open_request(&requested, None)) + .unwrap() + .unwrap_or_else(|| panic!("length {length} dscp {dscp} must be answered")); + + let reply = expect_normal_open_reply(&packet, None); + assert_eq!(reply.params.length, length); + assert_eq!(reply.params.dscp, dscp); + assert_eq!(core.session_count(), 1); + } +} diff --git a/crates/irtt-server/src/tests/params.rs b/crates/irtt-server/src/tests/params.rs new file mode 100644 index 0000000..d2158b0 --- /dev/null +++ b/crates/irtt-server/src/tests/params.rs @@ -0,0 +1,158 @@ +use irtt_proto::{Clock, Params, ReceivedStats, ServerFill, StampAt}; + +use super::support::{ + core_with_tokens, expect_normal_open_reply, open_request, open_request_with_raw_params, + param_int, param_server_fill, peer, ScriptedTokens, +}; +use crate::ServerConfig; + +const TOKEN_A: u64 = 0x0102_0304_0506_0708; + +fn reject_case(name: &str, payload: Vec) { + let tokens = ScriptedTokens::new([TOKEN_A]); + let mut core = core_with_tokens(ServerConfig::default(), tokens.clone()); + + let reply = core + .handle_datagram(peer(), &open_request_with_raw_params(&payload, None)) + .expect("rejected parameters are not a server error"); + + assert!(reply.is_none(), "{name} must be answered with silence"); + assert_eq!(core.session_count(), 0, "{name} must create no session"); + assert_eq!( + tokens.remaining(), + 1, + "{name} must not consume a session token" + ); +} + +#[test] +fn malformed_or_out_of_range_parameters_are_dropped_without_a_session() { + // Truncation and overflow. + reject_case("truncated tag varint", vec![0x80]); + reject_case("truncated value varint", vec![2]); + reject_case("truncated multi-byte value", vec![2, 0x80]); + reject_case( + "varint overflow", + vec![ + 1, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + ], + ); + + // Duration and Interval: present and not positive. + reject_case("explicit duration zero", param_int(2, 0)); + reject_case("negative duration", param_int(2, -1)); + reject_case("explicit interval zero", param_int(3, 0)); + reject_case("negative interval", param_int(3, -5)); + + // Enum values outside their defined range. + reject_case("received stats above range", param_int(5, 4)); + reject_case("negative received stats", param_int(5, -1)); + reject_case("stamp at above range", param_int(6, 5)); + reject_case("explicit clock zero", param_int(7, 0)); + reject_case("clock above range", param_int(7, 4)); + + // Server fill. + reject_case( + "server fill declaring more than the payload holds", + param_server_fill(10, b"ab"), + ); + reject_case( + "server fill longer than 32 bytes", + param_server_fill(33, b"0123456789abcdef0123456789abcdefx"), + ); + reject_case( + "server fill that is not utf-8", + param_server_fill(1, &[0xff]), + ); +} + +#[test] +fn absent_duration_interval_and_clock_are_accepted_as_wire_defaults() { + // The whole reason the decoder reports presence: an omitted tag and one + // explicitly encoded as zero share a value but not a verdict. + let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + + let packet = core + .handle_datagram( + peer(), + &open_request_with_raw_params(¶m_int(1, 1), None), + ) + .unwrap() + .expect("absent duration, interval and clock are all valid"); + + let reply = expect_normal_open_reply(&packet, None); + assert_eq!(reply.params.duration_ns, 0); + assert_eq!(reply.params.interval_ns, 0); + assert_eq!(reply.params.clock, Clock::Unspecified); + assert_eq!(core.session_count(), 1); +} + +#[test] +fn valid_optional_parameter_values_survive_unchanged() { + for (name, requested) in [ + ( + "clock wall", + Params { + clock: Clock::Wall, + ..Params::default() + }, + ), + ( + "clock monotonic", + Params { + clock: Clock::Monotonic, + ..Params::default() + }, + ), + ( + "clock both", + Params { + clock: Clock::Both, + ..Params::default() + }, + ), + ( + "received stats and stamp at", + Params { + received_stats: ReceivedStats::Both, + stamp_at: StampAt::Midpoint, + clock: Clock::Wall, + ..Params::default() + }, + ), + ( + "positive duration and interval", + Params { + duration_ns: 1, + interval_ns: 1, + ..Params::default() + }, + ), + ( + "maximum length server fill", + Params { + server_fill: Some(ServerFill { + value: "0123456789abcdef0123456789abcdef".to_owned(), + }), + ..Params::default() + }, + ), + ] { + let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::new([TOKEN_A])); + + let packet = core + .handle_datagram(peer(), &open_request(&requested, None)) + .unwrap() + .unwrap_or_else(|| panic!("{name} must be answered")); + + let reply = expect_normal_open_reply(&packet, None); + assert_eq!( + reply.params, + Params { + protocol_version: 1, + ..requested + }, + "{name} must survive negotiation unchanged" + ); + } +} diff --git a/crates/irtt-server/src/tests/sessions.rs b/crates/irtt-server/src/tests/sessions.rs new file mode 100644 index 0000000..2461eda --- /dev/null +++ b/crates/irtt-server/src/tests/sessions.rs @@ -0,0 +1,96 @@ +use super::support::{ + client_params, core_with_tokens, expect_normal_open_reply, open_request, other_peer, peer, + ScriptedTokens, +}; +use crate::{ServerConfig, DEFAULT_MAX_SESSIONS}; + +const TOKEN_A: u64 = 0x0102_0304_0506_0708; +const TOKEN_B: u64 = 0x1112_1314_1516_1718; +const TOKEN_C: u64 = 0x2122_2324_2526_2728; + +#[test] +fn the_default_session_bound_is_finite() { + let bound = ServerConfig::default().max_sessions(); + + assert_eq!(bound, DEFAULT_MAX_SESSIONS); + assert!(bound > 0, "the default bound must admit sessions"); + assert!( + bound < usize::MAX, + "there is deliberately no unlimited default" + ); +} + +#[test] +fn repeated_opens_from_one_endpoint_create_independent_sessions() { + // Opens are never deduplicated by endpoint or by parameters: a client + // retransmission legitimately produces a second, independent session. + let mut core = core_with_tokens( + ServerConfig::default(), + ScriptedTokens::new([TOKEN_A, TOKEN_B]), + ); + let request = open_request(&client_params(), None); + + let first = core + .handle_datagram(peer(), &request) + .unwrap() + .expect("first open must be answered"); + let second = core + .handle_datagram(peer(), &request) + .unwrap() + .expect("second open must be answered"); + + let first = expect_normal_open_reply(&first, None); + let second = expect_normal_open_reply(&second, None); + + assert_ne!(first.token, 0); + assert_ne!(second.token, 0); + assert_ne!(first.token, second.token); + assert_eq!(core.session_count(), 2); + assert_eq!(core.session(first.token).unwrap().peer(), peer()); + assert_eq!(core.session(second.token).unwrap().peer(), peer()); +} + +#[test] +fn a_full_session_table_drops_new_opens_without_evicting_anything() { + let tokens = ScriptedTokens::new([TOKEN_A, TOKEN_B, TOKEN_C]); + let mut core = core_with_tokens(ServerConfig::default().with_max_sessions(2), tokens.clone()); + let request = open_request(&client_params(), None); + + core.handle_datagram(peer(), &request) + .unwrap() + .expect("first open must be answered"); + core.handle_datagram(other_peer(), &request) + .unwrap() + .expect("second open must be answered"); + assert_eq!(core.session_count(), 2); + assert_eq!(core.session(TOKEN_A).unwrap().peer(), peer()); + assert_eq!(core.session(TOKEN_B).unwrap().peer(), other_peer()); + + // Refusal is silence: the protocol has no rejection reply, so the client + // simply sees an open timeout. + assert_eq!(core.handle_datagram(peer(), &request).unwrap(), None); + + assert_eq!(core.session_count(), 2, "nothing may be evicted"); + assert_eq!(core.session(TOKEN_A).unwrap().peer(), peer()); + assert_eq!(core.session(TOKEN_B).unwrap().peer(), other_peer()); + assert_eq!(core.session(TOKEN_C), None); + assert_eq!( + tokens.remaining(), + 1, + "a refused open must not commit a token" + ); +} + +#[test] +fn a_zero_session_bound_refuses_every_session_creating_open() { + let tokens = ScriptedTokens::new([TOKEN_A]); + let mut core = core_with_tokens(ServerConfig::default().with_max_sessions(0), tokens.clone()); + + assert_eq!( + core.handle_datagram(peer(), &open_request(&client_params(), None)) + .unwrap(), + None + ); + assert_eq!(core.session_count(), 0); + assert_eq!(tokens.remaining(), 1); +} diff --git a/crates/irtt-server/src/tests/support.rs b/crates/irtt-server/src/tests/support.rs new file mode 100644 index 0000000..bf35c19 --- /dev/null +++ b/crates/irtt-server/src/tests/support.rs @@ -0,0 +1,179 @@ +use std::{ + collections::VecDeque, + net::SocketAddr, + sync::{Arc, Mutex}, +}; + +use irtt_proto::{ + decode_open_reply, encode_request, varint, OpenReply, PacketLayout, Params, RequestToEncode, + FLAG_CLOSE, FLAG_HMAC, FLAG_OPEN, FLAG_REPLY, +}; + +use crate::{core::ServerCore, error::ServerError, token::TokenSource, ServerConfig}; + +pub(crate) const KEY: &[u8] = b"correctkey"; +pub(crate) const OTHER_KEY: &[u8] = b"wrongkey"; + +pub(crate) fn peer() -> SocketAddr { + "198.51.100.7:41234".parse().unwrap() +} + +pub(crate) fn other_peer() -> SocketAddr { + "198.51.100.7:41235".parse().unwrap() +} + +/// A token source handing out a fixed script of values, then failing. +/// +/// Draining the script is itself a failure rather than a wrap-around, so a test +/// that draws more tokens than it scripted is a loud test bug instead of a +/// silent repeat. +#[derive(Debug, Clone)] +pub(crate) struct ScriptedTokens { + values: Arc>>>, +} + +impl ScriptedTokens { + pub(crate) fn new(values: I) -> Self + where + I: IntoIterator, + { + Self { + values: Arc::new(Mutex::new(values.into_iter().map(Ok).collect())), + } + } + + /// A source whose every draw reports a random-source failure. + pub(crate) fn failing() -> Self { + Self { + values: Arc::new(Mutex::new(VecDeque::new())), + } + } + + /// How many scripted values are still unused. + pub(crate) fn remaining(&self) -> usize { + self.values.lock().unwrap().len() + } +} + +impl TokenSource for ScriptedTokens { + fn next_token(&mut self) -> Result { + self.values + .lock() + .unwrap() + .pop_front() + .unwrap_or(Err(ServerError::RandomSource { + reason: "scripted failure".to_owned(), + })) + } +} + +/// A core with deterministic tokens, so session identity is assertable. +pub(crate) fn core_with_tokens(config: ServerConfig, tokens: ScriptedTokens) -> ServerCore { + ServerCore::with_token_source(config, Box::new(tokens)) +} + +/// Encodes a normal open request carrying `params`. +pub(crate) fn open_request(params: &Params, hmac_key: Option<&[u8]>) -> Vec { + encode_request( + RequestToEncode::Open { + params, + no_test: false, + }, + hmac_key, + ) + .unwrap() +} + +/// Encodes a no-test open request carrying `params`. +pub(crate) fn no_test_request(params: &Params, hmac_key: Option<&[u8]>) -> Vec { + encode_request( + RequestToEncode::Open { + params, + no_test: true, + }, + hmac_key, + ) + .unwrap() +} + +/// Builds an open request around a raw parameter payload. +/// +/// Used for payloads a compliant encoder cannot produce, such as a truncated +/// varint or an out-of-range enum value. +pub(crate) fn open_request_with_raw_params(payload: &[u8], hmac_key: Option<&[u8]>) -> Vec { + let mut packet = irtt_proto::MAGIC.to_vec(); + packet.push(if hmac_key.is_some() { + FLAG_OPEN | FLAG_HMAC + } else { + FLAG_OPEN + }); + if hmac_key.is_some() { + packet.extend_from_slice(&[0; irtt_proto::HMAC_SIZE]); + } + packet.extend_from_slice(payload); + if let Some(key) = hmac_key { + irtt_proto::compute_hmac_in_place(key, &mut packet, hmac_field_offset()).unwrap(); + } + packet +} + +/// The authentication field sits immediately after the fixed header. +fn hmac_field_offset() -> usize { + PacketLayout::open_request(false).header_len() +} + +/// Encodes one integer parameter as `tag` + zigzag value. +pub(crate) fn param_int(tag: u64, value: i64) -> Vec { + let mut encoded = Vec::new(); + varint::encode_uvarint(tag, &mut encoded); + varint::encode_varint(value, &mut encoded); + encoded +} + +/// Encodes a `server_fill` parameter with an explicit declared length, so tests +/// can declare a length the payload does not actually carry. +pub(crate) fn param_server_fill(declared_len: u64, value: &[u8]) -> Vec { + let mut encoded = Vec::new(); + varint::encode_uvarint(9, &mut encoded); + varint::encode_uvarint(declared_len, &mut encoded); + encoded.extend_from_slice(value); + encoded +} + +/// Params a well-behaved client would request. +pub(crate) fn client_params() -> Params { + Params { + protocol_version: 1, + duration_ns: 3_000_000_000, + interval_ns: 1_000_000_000, + length: 1472, + received_stats: irtt_proto::ReceivedStats::Both, + stamp_at: irtt_proto::StampAt::Both, + clock: irtt_proto::Clock::Both, + dscp: 184, + server_fill: None, + } +} + +/// Decodes a reply the server produced, checking it against the server's key. +pub(crate) fn decode_reply(packet: &[u8], hmac_key: Option<&[u8]>) -> OpenReply { + decode_open_reply(packet, hmac_key).expect("server reply must decode") +} + +pub(crate) fn expect_normal_open_reply(packet: &[u8], hmac_key: Option<&[u8]>) -> OpenReply { + let reply = decode_reply(packet, hmac_key); + let expected = FLAG_OPEN | FLAG_REPLY | if hmac_key.is_some() { FLAG_HMAC } else { 0 }; + assert_eq!(reply.flags, expected, "normal open reply flags"); + assert_ne!(reply.token, 0, "a session-creating reply needs a token"); + assert_eq!(reply.params.protocol_version, 1); + reply +} + +pub(crate) fn expect_no_test_reply(packet: &[u8], hmac_key: Option<&[u8]>) -> OpenReply { + let reply = decode_reply(packet, hmac_key); + let expected = + FLAG_OPEN | FLAG_REPLY | FLAG_CLOSE | if hmac_key.is_some() { FLAG_HMAC } else { 0 }; + assert_eq!(reply.flags, expected, "no-test reply flags"); + assert_eq!(reply.token, 0, "a no-test reply carries a zero token"); + reply +} diff --git a/crates/irtt-server/src/tests/tokens.rs b/crates/irtt-server/src/tests/tokens.rs new file mode 100644 index 0000000..62b1b18 --- /dev/null +++ b/crates/irtt-server/src/tests/tokens.rs @@ -0,0 +1,86 @@ +use super::support::{ + client_params, core_with_tokens, expect_normal_open_reply, open_request, peer, ScriptedTokens, +}; +use crate::{token::TOKEN_ATTEMPTS, ServerConfig, ServerError}; + +const TOKEN_A: u64 = 0x0102_0304_0506_0708; +const TOKEN_B: u64 = 0x1112_1314_1516_1718; + +#[test] +fn a_zero_draw_is_discarded_and_redrawn() { + // Zero is reserved for no-test replies and is never issued to a session. + let mut core = core_with_tokens( + ServerConfig::default(), + ScriptedTokens::new([0, 0, TOKEN_A]), + ); + + let packet = core + .handle_datagram(peer(), &open_request(&client_params(), None)) + .unwrap() + .expect("a zero draw must be retried, not surfaced"); + + assert_eq!(expect_normal_open_reply(&packet, None).token, TOKEN_A); + assert_eq!(core.session_count(), 1); +} + +#[test] +fn a_colliding_draw_is_discarded_and_never_overwrites_the_live_session() { + let mut core = core_with_tokens( + ServerConfig::default(), + ScriptedTokens::new([TOKEN_A, TOKEN_A, TOKEN_B]), + ); + let request = open_request(&client_params(), None); + + core.handle_datagram(peer(), &request) + .unwrap() + .expect("first open must be answered"); + let first = core.session(TOKEN_A).cloned().expect("session must exist"); + + let packet = core + .handle_datagram(peer(), &request) + .unwrap() + .expect("a collision must be retried, not surfaced"); + + assert_eq!(expect_normal_open_reply(&packet, None).token, TOKEN_B); + assert_eq!(core.session_count(), 2); + assert_eq!( + core.session(TOKEN_A), + Some(&first), + "the colliding token's session must be untouched" + ); +} + +#[test] +fn exhausting_the_retry_budget_is_an_internal_error_that_creates_nothing() { + let mut core = core_with_tokens( + ServerConfig::default(), + ScriptedTokens::new([TOKEN_A, 0, 0, 0, 0, 0, 0, 0, 0]), + ); + let request = open_request(&client_params(), None); + + core.handle_datagram(peer(), &request) + .unwrap() + .expect("first open must be answered"); + let first = core.session(TOKEN_A).cloned().expect("session must exist"); + + assert_eq!( + core.handle_datagram(peer(), &request), + Err(ServerError::TokenExhausted { + attempts: TOKEN_ATTEMPTS + }) + ); + + assert_eq!(core.session_count(), 1, "no session may be half-created"); + assert_eq!(core.session(TOKEN_A), Some(&first)); +} + +#[test] +fn a_failing_random_source_is_reported_and_creates_nothing() { + let mut core = core_with_tokens(ServerConfig::default(), ScriptedTokens::failing()); + + assert!(matches!( + core.handle_datagram(peer(), &open_request(&client_params(), None)), + Err(ServerError::RandomSource { .. }) + )); + assert_eq!(core.session_count(), 0); +} diff --git a/crates/irtt-server/src/tests/unsupported.rs b/crates/irtt-server/src/tests/unsupported.rs new file mode 100644 index 0000000..4036ca2 --- /dev/null +++ b/crates/irtt-server/src/tests/unsupported.rs @@ -0,0 +1,93 @@ +//! 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. + +use irtt_proto::{encode_request, RequestToEncode}; + +use super::support::{client_params, core_with_tokens, open_request, peer, ScriptedTokens, KEY}; +use crate::ServerConfig; + +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"); + core +} + +#[test] +fn a_valid_echo_is_neither_answered_nor_counted_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 echo = encode_request( + RequestToEncode::Echo { + token: TOKEN_A, + sequence: 0, + params: session.params(), + payload: &[], + }, + hmac_key, + ) + .unwrap(); + + assert_eq!(core.handle_datagram(peer(), &echo).unwrap(), None); + assert_eq!(core.session_count(), 1); + assert_eq!(core.session(TOKEN_A), Some(&session)); + } +} + +#[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); + let session = core.session(TOKEN_A).cloned().expect("session must exist"); + + let mut bad_magic = open_request(&client_params(), None); + bad_magic[0] ^= 0x01; + + let mut reserved_flag = open_request(&client_params(), None); + reserved_flag[3] |= 0x10; + + let mut reply_flag = open_request(&client_params(), None); + reply_flag[3] |= irtt_proto::FLAG_REPLY; + + for (name, packet) in [ + ("empty datagram", Vec::new()), + ("datagram shorter than the header", vec![0x14, 0xa7, 0x5b]), + ("bad magic", bad_magic), + ("reserved flag bit", reserved_flag), + ("reply flag", reply_flag), + ] { + assert_eq!( + core.handle_datagram(peer(), &packet).unwrap(), + None, + "{name} must be discarded" + ); + } + + assert_eq!(core.session_count(), 1); + assert_eq!(core.session(TOKEN_A), Some(&session)); +} diff --git a/crates/irtt-server/src/token.rs b/crates/irtt-server/src/token.rs new file mode 100644 index 0000000..ef96181 --- /dev/null +++ b/crates/irtt-server/src/token.rs @@ -0,0 +1,43 @@ +//! Session token generation. +//! +//! A token is the only thing standing between an off-path attacker and a usable +//! session on an unauthenticated server, so production tokens come from the +//! operating system's cryptographically secure random source. Nothing here +//! derives a token from a counter, a timestamp, a hash of the source address, +//! or any other predictable value, and there is no fallback that would do so if +//! the random source fails. + +use std::fmt; + +use crate::error::ServerError; + +/// How many values token allocation may draw before giving up. +/// +/// Allocation rejects a drawn value that is zero (reserved for no-test replies) +/// or that collides with a live session, and draws again. With a working random +/// source neither outcome is realistically reachable even once; the bound +/// exists so a degenerate source cannot spin the allocator forever. +pub(crate) const TOKEN_ATTEMPTS: u32 = 8; + +/// A source of candidate session tokens. +/// +/// Deliberately private: it exists so tests can drive the zero, collision and +/// failure paths deterministically, not as a public extension point. The +/// production implementation is [`OsTokenSource`]. +pub(crate) trait TokenSource: fmt::Debug + Send { + /// Draws the next candidate token. The value is *not* required to be + /// non-zero or unique; that is the allocator's job. + fn next_token(&mut self) -> Result; +} + +/// Draws tokens from the operating system's random source. +#[derive(Debug, Default)] +pub(crate) struct OsTokenSource; + +impl TokenSource for OsTokenSource { + fn next_token(&mut self) -> Result { + getrandom::u64().map_err(|source| ServerError::RandomSource { + reason: source.to_string(), + }) + } +} -- 2.54.0 From b7e206f939567fdc6680639597ad72b2a062c548 Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Wed, 12 Aug 2026 01:10:56 +0700 Subject: [PATCH 3/3] refactor(server): keep session state crate-private MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Session` and the token lookup that returns it were public, which published internal session state before there is a consumer for it. The next slices are all but certain to add and restructure fields here — receive counts and windows, sequence tracking, activity and expiry state — and nothing outside the crate needs a session yet. Make `Session` and `ServerCore::session` crate-private. `session_count` stays public: how much session state the server holds against its bound is a sensible stable observable, unlike the sessions themselves. The endpoint accessors have no production reader until echo and close processing arrive, since a session's endpoint has to be captured at open time and is not recoverable later. They are marked accordingly rather than left to warn. --- crates/irtt-server/src/core.rs | 11 +++++++++-- crates/irtt-server/src/lib.rs | 1 - crates/irtt-server/src/session.rs | 23 ++++++++++++++++++----- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/crates/irtt-server/src/core.rs b/crates/irtt-server/src/core.rs index f622d70..2265937 100644 --- a/crates/irtt-server/src/core.rs +++ b/crates/irtt-server/src/core.rs @@ -68,6 +68,10 @@ impl ServerCore { } /// The number of live sessions. + /// + /// This is the observable a caller genuinely needs — how much session state + /// the server is holding against its bound. The sessions themselves stay + /// crate-private until their shape settles. #[must_use] pub fn session_count(&self) -> usize { self.sessions.len() @@ -78,8 +82,11 @@ 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. - #[must_use] - pub fn session(&self, token: u64) -> Option<&Session> { + // + // Called only by tests until echo and close processing arrive, which is + // what a token lookup is for. + #[allow(dead_code)] + pub(crate) fn session(&self, token: u64) -> Option<&Session> { self.sessions.get(&token) } diff --git a/crates/irtt-server/src/lib.rs b/crates/irtt-server/src/lib.rs index b72ae86..5f4fd6d 100644 --- a/crates/irtt-server/src/lib.rs +++ b/crates/irtt-server/src/lib.rs @@ -50,4 +50,3 @@ mod tests; pub use config::{ServerConfig, DEFAULT_MAX_SESSIONS}; pub use core::ServerCore; pub use error::ServerError; -pub use session::Session; diff --git a/crates/irtt-server/src/session.rs b/crates/irtt-server/src/session.rs index 9c7d7e7..b1e40de 100644 --- a/crates/irtt-server/src/session.rs +++ b/crates/irtt-server/src/session.rs @@ -9,8 +9,13 @@ use irtt_proto::Params; /// state (activity, deadlines, expiry) belong to the slices that implement /// those behaviors, and are deliberately absent rather than present as /// placeholders. +/// +/// Deliberately crate-private. The next slices are all but certain to add and +/// restructure fields here, and nothing outside the crate needs a session yet; +/// publishing internal session state before ECHO even exists would buy nothing +/// and commit us to a shape we are about to change. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct Session { +pub(crate) struct Session { peer: SocketAddr, params: Params, } @@ -27,15 +32,23 @@ impl Session { /// 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. - #[must_use] - pub fn peer(&self) -> SocketAddr { + // + // 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)] + pub(crate) fn peer(&self) -> SocketAddr { self.peer } /// The negotiated parameters the server returned for this session and will /// enforce for it. - #[must_use] - pub fn params(&self) -> &Params { + // + // Also awaiting its production consumer: echo replies are built from the + // negotiated params. + #[allow(dead_code)] + pub(crate) fn params(&self) -> &Params { &self.params } } -- 2.54.0