From 8308c077eb4d0ac295c583d810808cc4fd4a423b Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Fri, 14 Aug 2026 14:24:57 +0700 Subject: [PATCH] feat(server): add negotiation policy controls --- crates/irtt-server/AGENTS.md | 43 +++- crates/irtt-server/src/config.rs | 111 ++++++++- crates/irtt-server/src/core.rs | 14 +- crates/irtt-server/src/lib.rs | 23 +- crates/irtt-server/src/negotiate.rs | 54 +++- crates/irtt-server/src/tests/echo.rs | 59 ++++- crates/irtt-server/src/tests/negotiation.rs | 230 +++++++++++++++++- crates/irtt-server/src/tests/no_test.rs | 36 ++- crates/irtt-server/src/tests/open.rs | 66 ++++- .../irtt-server/tests/negotiation_policy.rs | 225 +++++++++++++++++ 10 files changed, 831 insertions(+), 30 deletions(-) create mode 100644 crates/irtt-server/tests/negotiation_policy.rs diff --git a/crates/irtt-server/AGENTS.md b/crates/irtt-server/AGENTS.md index 3653d16..7ed58a2 100644 --- a/crates/irtt-server/AGENTS.md +++ b/crates/irtt-server/AGENTS.md @@ -138,8 +138,10 @@ effective parameters must be safe for the server to run. - `Clock::Unspecified` alone is valid — it is just an absent tag. Only the combination is refused, and only an absent tag reaches it, since an explicit zero Clock is already out of range for the decoder. -- Run the check after negotiation, so that a later policy restricting `StampAt` - to none makes an omitted clock safe without this rule changing. +- Run the check after negotiation. That is what makes a configured timestamp + allowance of none accept this same request: its effective `StampAt` is none, so + the omitted clock selects nothing and the session is executable. See + "Capability restrictions" below. - Negative `Length` stays an accepted negotiated value, returned unchanged in the open reply and stored verbatim in the session. It needs no clamping to be executable: `irtt-proto` floors actual echo packet sizing at the required @@ -253,6 +255,43 @@ into `irtt-proto`, and do not duplicate the decoder's checks here. - Payload length comes from `irtt-proto`'s layout and `echo_packet_len`, never from `params.length`, and needs no second packet-size bound of its own. +## Capability restrictions + +Two optional negotiation policies restrict what a session may ask this server to +*provide*. Both default to off, so a configuration that sets neither negotiates +exactly what it did before they existed. Both live in `negotiate`, with the rest +of open negotiation, and neither reaches the runtime. + +- **`TimestampAllowance` is server policy, and deliberately not `StampAt`.** The + wire enum describes what a session requests and negotiates; the allowance + describes what this server is willing to hand out. Keep the two types apart, + and keep the allowance out of `irtt-proto`. +- The default is `Dual`, which honors every requested placement. `None` maps every + non-none placement to none. `Single` maps `Both` to **`Midpoint`** and leaves + every already-single placement alone — that one substitution is the only + interesting row; the rest is in the clean spec's Section 11.4. +- **`Clock` is never rewritten.** The evidence describes an allowance on timestamp + *placement*, not on clock domains, and the echo layout already carries no + timestamp field once the placement is none. Rewriting the clock would answer + with a session the client did not ask for. +- **Restriction runs before the effective-session executability check**, and that + ordering is load-bearing: it is what makes a request selecting timestamps with + an omitted Clock *safe* under a none allowance, where the same request is + refused under the default. Do not move the check onto the requested params. +- **DSCP permission defaults to allowed.** When disallowed, negotiation sets + `Params.dscp` to zero before the reply is encoded and before the `Session` + exists, for every requested value including ones outside `0..=255`. The open is + never refused over DSCP, and nothing is clamped or wrapped. +- **No second policy bit anywhere.** The negotiated params are the single source + of truth: the session stores zero, `OutboundDatagram`'s traffic class derives + from it as for any other zero-DSCP session, and the runtime never learns that + `ServerConfig::dscp_allowed` exists. `Params::encode` omits a zero integer, so + the reply simply carries no DSCP tag. +- **This is operator policy, not socket capability detection.** What a given host + can apply to a socket is settled per send by the runtime (see "Reply traffic + class"). Do not couple `ServerCore` negotiation to `socket2` or to runtime + capability state. + ## Reply traffic class The core hands each reply out as an `OutboundDatagram`: the bytes plus the diff --git a/crates/irtt-server/src/config.rs b/crates/irtt-server/src/config.rs index 21e5b00..76e5649 100644 --- a/crates/irtt-server/src/config.rs +++ b/crates/irtt-server/src/config.rs @@ -53,12 +53,36 @@ pub const DEFAULT_BURST_ALLOWANCE: u32 = 5; /// emits no final reply. See [`ServerConfig::with_idle_timeout`]. pub const DEFAULT_IDLE_TIMEOUT: Duration = Duration::from_secs(60); +/// How many timestamps a server is willing to provide. +/// +/// This is **server policy**, deliberately a separate type from the wire +/// [`StampAt`](irtt_proto::StampAt) a session requests and negotiates: one +/// describes what a client asked for, the other what this server will hand out. +/// Negotiation reduces the requested `StampAt` accordingly, and only it — the +/// requested [`Clock`](irtt_proto::Clock) is never rewritten, because the clean +/// evidence records a timestamp allowance restricting timestamp *placement*, not +/// the clock domains it is read from. +/// +/// See [`ServerConfig::with_timestamp_allowance`] for the mapping. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum TimestampAllowance { + /// No timestamps at all: every request is negotiated to + /// [`StampAt::None`](irtt_proto::StampAt::None). + None, + /// At most one timestamp field. A request for both receive and send becomes + /// [`StampAt::Midpoint`](irtt_proto::StampAt::Midpoint); every mode that is + /// already single is left alone. + Single, + /// Every requested placement is honored. This is the default, and the + /// behavior of a server that configures nothing. + #[default] + Dual, +} + /// Configuration for a [`ServerCore`](crate::ServerCore). /// /// Fields are private and set through consuming builder methods, so later -/// slices can add configuration without breaking construction. Optional -/// timestamp-allowance and DSCP restriction controls would arrive with the -/// slices that implement them. +/// slices can add configuration without breaking construction. /// /// **Server fill is deliberately not configurable.** The policy is fixed and /// safe: every valid descriptor is honored, one that cannot be parsed falls back @@ -78,6 +102,8 @@ pub struct ServerConfig { burst_allowance: u32, idle_timeout: Duration, max_test_duration: Option, + timestamp_allowance: TimestampAllowance, + dscp_allowed: bool, } impl ServerConfig { @@ -221,6 +247,65 @@ impl ServerConfig { self } + /// Sets how many timestamps this server is willing to provide. + /// + /// A requested placement above the allowance is reduced during negotiation, + /// so the open reply is honest about what the session's echo replies will + /// carry: + /// + /// | Requested | [`Dual`] | [`Single`] | [`None`] | + /// |-----------|----------|------------|----------| + /// | `None` | `None` | `None` | `None` | + /// | `Send` | `Send` | `Send` | `None` | + /// | `Receive` | `Receive` | `Receive` | `None` | + /// | `Both` | `Both` | **`Midpoint`** | `None` | + /// | `Midpoint` | `Midpoint` | `Midpoint` | `None` | + /// + /// The one substitution is the observed one: under a single-timestamp + /// allowance a request for both instants is answered with the *midpoint*, + /// not with whichever of the two the server felt like keeping. + /// + /// The requested [`Clock`](irtt_proto::Clock) is left exactly as it was in + /// every case. The allowance restricts placement, and the echo layout simply + /// carries no timestamp field once the placement is + /// [`StampAt::None`](irtt_proto::StampAt::None). + /// + /// Restriction runs *before* the effective-session check, which is what + /// makes a [`None`] allowance accept a request that selected timestamps + /// without sending a Clock: such a session is refused as requested, because + /// timestamps from no clock cannot be produced, but under this policy its + /// effective placement is none and the absent clock no longer matters. + /// + /// [`Dual`]: TimestampAllowance::Dual + /// [`Single`]: TimestampAllowance::Single + /// [`None`]: TimestampAllowance::None + #[must_use] + pub fn with_timestamp_allowance(mut self, timestamp_allowance: TimestampAllowance) -> Self { + self.timestamp_allowance = timestamp_allowance; + self + } + + /// Sets whether this server provides the traffic-class marking a session + /// requests. + /// + /// `true`, the default, negotiates the requested DSCP parameter unchanged — + /// including a raw value outside `0..=255`, which stays in the negotiated + /// parameters and is transported unmarked. + /// + /// `false` negotiates it to **zero**, whatever was requested, so the reply + /// tells the client its replies will be unmarked and the session's stored + /// parameters say the same. The open is never refused over DSCP, and nothing + /// is clamped or wrapped: this is an operator policy about providing + /// traffic-class marking at all, not a judgement about a particular value. + /// + /// It is also not socket capability detection. What a given host can apply + /// to a socket is a transport concern, settled per send by the runtime. + #[must_use] + pub fn with_dscp_allowed(mut self, dscp_allowed: bool) -> Self { + self.dscp_allowed = dscp_allowed; + self + } + /// The configured HMAC key, if this server authenticates. #[must_use] pub fn hmac_key(&self) -> Option<&[u8]> { @@ -267,6 +352,19 @@ impl ServerConfig { pub fn max_test_duration(&self) -> Option { self.max_test_duration } + + /// How many timestamps this server is willing to provide. + #[must_use] + pub fn timestamp_allowance(&self) -> TimestampAllowance { + self.timestamp_allowance + } + + /// Whether a requested DSCP value is negotiated as asked, rather than to + /// zero. + #[must_use] + pub fn dscp_allowed(&self) -> bool { + self.dscp_allowed + } } impl Default for ServerConfig { @@ -279,6 +377,11 @@ impl Default for ServerConfig { /// No maximum test duration is configured, matching the reference server's /// ordinary policy. A session is still bounded, by the table size and the /// idle timeout. + /// + /// Both capability restrictions are off: every requested timestamp placement + /// is honored ([`TimestampAllowance::Dual`]) and a requested DSCP value is + /// negotiated as asked. They are opt-in, so an existing configuration + /// negotiates exactly what it did before they existed. fn default() -> Self { Self { hmac_key: None, @@ -288,6 +391,8 @@ impl Default for ServerConfig { burst_allowance: DEFAULT_BURST_ALLOWANCE, idle_timeout: DEFAULT_IDLE_TIMEOUT, max_test_duration: None, + timestamp_allowance: TimestampAllowance::Dual, + dscp_allowed: true, } } } diff --git a/crates/irtt-server/src/core.rs b/crates/irtt-server/src/core.rs index 66cdeff..a840205 100644 --- a/crates/irtt-server/src/core.rs +++ b/crates/irtt-server/src/core.rs @@ -733,7 +733,9 @@ fn same_endpoint(session: SocketAddr, peer: SocketAddr) -> bool { /// during negotiation. Length and DSCP are accepted as decoded, including zero, /// negative and out-of-byte-range values; a DSCP the socket could never carry /// is negotiated unchanged and transported unmarked instead of being refused -/// (see [`transport_traffic_class`]). Unknown tags were ignored by the decoder +/// (see [`transport_traffic_class`]), and a server that disallows DSCP +/// negotiates zero rather than rejecting the request. 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; @@ -783,10 +785,12 @@ fn open_params_are_admissible(decoded: &DecodedParams) -> bool { /// default policy makes it unreachable for an acknowledged session anyway, since /// negotiation has already reduced the length to a representable maximum. /// -/// The check is deliberately *after* negotiation so that a later policy slice -/// restricting timestamps can restrict `stamp_at` to [`StampAt::None`], at -/// which point an absent clock is safe and this predicate holds without -/// changing. +/// The check is deliberately *after* negotiation, which is what makes a +/// configured timestamp allowance of +/// [`TimestampAllowance::None`](crate::TimestampAllowance::None) accept this +/// otherwise refused request: its effective `stamp_at` is [`StampAt::None`], so +/// the absent clock selects nothing that was going to be reported and the +/// session is executable exactly as negotiated. /// /// [`ServerError`]: crate::ServerError fn negotiated_params_are_admissible(params: &Params, config: &ServerConfig) -> bool { diff --git a/crates/irtt-server/src/lib.rs b/crates/irtt-server/src/lib.rs index 0dc8247..d568ba1 100644 --- a/crates/irtt-server/src/lib.rs +++ b/crates/irtt-server/src/lib.rs @@ -59,9 +59,22 @@ //! Open handling and negotiation, session creation, normal echo processing, //! per-session rate limiting, idle expiry, the maximum-duration //! server-initiated close, the negotiated per-session reply traffic class, -//! server fill, wildcard reply-source selection and Tokio UDP orchestration. -//! Running several listeners in one process remains a separate slice, as do -//! optional timestamp and DSCP restriction controls. +//! server fill, the optional timestamp-allowance and DSCP capability +//! restrictions, wildcard reply-source selection and Tokio UDP orchestration. +//! Running several listeners in one process remains a separate slice. +//! +//! # Capability restrictions +//! +//! Two optional negotiation policies restrict what a session may ask this server +//! to provide, and both are off by default, so a configuration that sets neither +//! negotiates exactly what it always did. +//! +//! [`TimestampAllowance`] reduces the requested timestamp placement: `Single` +//! provides at most one timestamp, answering a request for both instants with +//! the midpoint, and `None` provides no timestamps at all. The requested clock +//! is never rewritten. [`ServerConfig::with_dscp_allowed`] refuses to provide +//! traffic-class marking, negotiating any requested DSCP value to zero, after +//! which the session's replies leave unmarked like any other zero-DSCP session. //! //! # Server fill //! @@ -93,8 +106,8 @@ mod token; mod tests; pub use config::{ - ServerConfig, DEFAULT_BURST_ALLOWANCE, DEFAULT_IDLE_TIMEOUT, DEFAULT_MAX_PACKET_LENGTH, - DEFAULT_MAX_SESSIONS, DEFAULT_MIN_SEND_INTERVAL, + ServerConfig, TimestampAllowance, DEFAULT_BURST_ALLOWANCE, DEFAULT_IDLE_TIMEOUT, + DEFAULT_MAX_PACKET_LENGTH, DEFAULT_MAX_SESSIONS, DEFAULT_MIN_SEND_INTERVAL, }; pub use core::{OutboundDatagram, ServerCore}; pub use error::ServerError; diff --git a/crates/irtt-server/src/negotiate.rs b/crates/irtt-server/src/negotiate.rs index 78b22b1..a8e6b5c 100644 --- a/crates/irtt-server/src/negotiate.rs +++ b/crates/irtt-server/src/negotiate.rs @@ -1,6 +1,9 @@ -use irtt_proto::{Params, PROTOCOL_VERSION}; +use irtt_proto::{Params, StampAt, PROTOCOL_VERSION}; -use crate::{clock::saturating_ns, config::ServerConfig}; +use crate::{ + clock::saturating_ns, + config::{ServerConfig, TimestampAllowance}, +}; /// Produces the parameters the server returns in an open reply and will enforce /// for the session. @@ -48,7 +51,24 @@ use crate::{clock::saturating_ns, config::ServerConfig}; /// fill the session will use, which is not always what the descriptor in /// `Params` says. The two run back to back on the same open. /// -/// Later policy slices may add the timestamp allowance and DSCP policy. +/// **StampAt is reduced to the configured timestamp allowance**, by +/// [`restrict_stamp_at`]. The Clock is deliberately untouched: the allowance is +/// a policy about which timestamps this server provides, and the echo layout +/// carries no timestamp field once the placement is [`StampAt::None`], so there +/// is no clock to restrict as well. Rewriting it would answer with a session the +/// client did not ask for. +/// +/// **DSCP is forced to zero when the server disallows it**, and otherwise +/// returned exactly as requested — including a value outside `0..=255`, which +/// stays in the negotiated parameters and is transported unmarked. Zero is a +/// negotiated value like any other and `Params::encode` simply omits it, so a +/// restricted reply carries no DSCP tag and the session's stored parameters +/// carry zero. That one value is what the runtime derives an unmarked reply +/// from; no session or transport state records the policy separately. +/// +/// Both restrictions run here, before the core validates the effective session, +/// which is what makes a timestamp-disallowing server accept an open that +/// selected timestamps without sending a Clock. /// /// Negotiation is not by itself an acknowledgement: the core validates what this /// function returns and silently discards an open whose effective parameters it @@ -96,5 +116,33 @@ pub(crate) fn negotiate_params(mut requested: Params, config: &ServerConfig) -> requested.interval_ns = idle_cap; } + requested.stamp_at = restrict_stamp_at(requested.stamp_at, config.timestamp_allowance()); + if !config.dscp_allowed() { + requested.dscp = 0; + } + requested } + +/// The timestamp placement a server offering `allowance` provides for a session +/// that requested `requested`. +/// +/// The mapping is the observed one, from the clean specification's Section 11.4. +/// Only one row substitutes rather than removes: under [`Single`], a request for +/// both instants is answered with [`StampAt::Midpoint`] — one timestamp that +/// still describes both — and not with whichever of Send or Receive an +/// implementation happened to prefer. Every already-single placement passes +/// through, and [`None`] removes all of them. +/// +/// [`Single`]: TimestampAllowance::Single +/// [`None`]: TimestampAllowance::None +fn restrict_stamp_at(requested: StampAt, allowance: TimestampAllowance) -> StampAt { + match allowance { + TimestampAllowance::Dual => requested, + TimestampAllowance::Single => match requested { + StampAt::Both => StampAt::Midpoint, + other => other, + }, + TimestampAllowance::None => StampAt::None, + } +} diff --git a/crates/irtt-server/src/tests/echo.rs b/crates/irtt-server/src/tests/echo.rs index 61b21f7..bfb5baf 100644 --- a/crates/irtt-server/src/tests/echo.rs +++ b/crates/irtt-server/src/tests/echo.rs @@ -23,7 +23,7 @@ use super::support::{ open_negotiated, other_peer, peer, sample, unthrottled, ScriptedClock, ScriptedTokens, KEY, OTHER_KEY, }; -use crate::{clock::ClockSample, ServerConfig}; +use crate::{clock::ClockSample, ServerConfig, TimestampAllowance}; const TOKEN_A: u64 = 0x0102_0304_0506_0708; const UNKNOWN_TOKEN: u64 = 0x2122_2324_2526_2728; @@ -759,3 +759,60 @@ fn a_reply_payload_is_server_generated_and_never_reflects_the_request() { "the region carries the server's default fill" ); } + +#[test] +fn a_restricted_session_emits_the_restricted_timestamps_and_not_the_requested_ones() { + // Timestamp generation follows the session's *negotiated* placement, which + // is the point of restricting it during negotiation: a client asking for + // both instants from both clocks gets one midpoint field per clock under a + // single-timestamp allowance, and nothing at all under none — with the reply + // laid out and sized from the restriction either way, not from the request. + for (allowance, expected_stamp_at, expected, expected_len) in [ + ( + TimestampAllowance::Single, + StampAt::Midpoint, + TimestampFields { + midpoint_wall: Some(1_100), + midpoint_mono: Some(10_150), + ..TimestampFields::default() + }, + 32, + ), + ( + TimestampAllowance::None, + StampAt::None, + TimestampFields::default(), + 16, + ), + ] { + let mut core = core_with_sources( + unthrottled().with_timestamp_allowance(allowance), + ScriptedTokens::new([TOKEN_A]), + ScriptedClock::new([open_sample(), sample(1_000, 10_000), sample(1_200, 10_300)]), + ); + let requested = echo_params(ReceivedStats::None, StampAt::Both, Clock::Both, 0); + let (token, negotiated) = open_negotiated(&mut core, peer(), &requested, None); + assert_eq!(negotiated.stamp_at, expected_stamp_at, "{allowance:?}"); + assert_eq!(negotiated.clock, Clock::Both, "{allowance:?}"); + + let packet = core + .handle_datagram(peer(), &echo_request(token, 0, &negotiated, &[], None)) + .unwrap() + .unwrap_or_else(|| panic!("{allowance:?}: an admissible echo must be answered")); + let reply = expect_echo_reply(&packet, &negotiated, None); + + // Equality over the whole struct, so a leftover receive or send field is + // a failure as much as a missing midpoint one. + assert_eq!(reply.timestamps, expected, "{allowance:?}"); + assert_eq!( + packet.bytes().len(), + echo_packet_len(false, &negotiated).unwrap(), + "{allowance:?}: the restricted layout's length" + ); + assert_eq!( + packet.bytes().len(), + expected_len, + "{allowance:?}: no empty slot is kept for a restricted field" + ); + } +} diff --git a/crates/irtt-server/src/tests/negotiation.rs b/crates/irtt-server/src/tests/negotiation.rs index cc01fad..5e9c838 100644 --- a/crates/irtt-server/src/tests/negotiation.rs +++ b/crates/irtt-server/src/tests/negotiation.rs @@ -7,12 +7,13 @@ use std::time::Duration; -use irtt_proto::Params; +use irtt_proto::{Clock, Params, StampAt}; use super::support::{ - client_params, core_with_tokens, expect_normal_open_reply, open_request, peer, ScriptedTokens, + client_params, core_with_tokens, echo_request, expect_echo_reply, expect_normal_open_reply, + open_request, peer, unthrottled, ScriptedTokens, }; -use crate::ServerConfig; +use crate::{ServerConfig, TimestampAllowance}; const TOKEN_A: u64 = 0x2f45_9a01_c37d_6e88; @@ -166,3 +167,226 @@ fn a_zero_idle_timeout_caps_nothing() { 3_600_000_000_000 ); } + +#[test] +fn the_default_configuration_restricts_neither_capability() { + // Both controls are opt-in, so an existing configuration negotiates exactly + // what it did before they existed. + let config = ServerConfig::default(); + + assert_eq!(config.timestamp_allowance(), TimestampAllowance::Dual); + assert!(config.dscp_allowed()); + assert_eq!(TimestampAllowance::default(), TimestampAllowance::Dual); + + let restricted = config + .clone() + .with_timestamp_allowance(TimestampAllowance::Single) + .with_dscp_allowed(false); + assert_eq!(restricted.timestamp_allowance(), TimestampAllowance::Single); + assert!(!restricted.dscp_allowed()); + assert_ne!( + restricted, config, + "the policy is part of the configuration" + ); +} + +#[test] +fn a_requested_stamp_at_is_reduced_to_the_configured_allowance() { + // The whole observed mapping, driven through real opens and read out of the + // reply, because the reply is what a client enforces its own restriction + // policy against. Only one row substitutes rather than removes: a request + // for both instants under a single-timestamp allowance is answered with the + // *midpoint*, which still describes both, rather than with whichever of the + // two the server preferred. + for (requested, allowance, expected) in [ + (StampAt::None, TimestampAllowance::Dual, StampAt::None), + (StampAt::None, TimestampAllowance::Single, StampAt::None), + (StampAt::None, TimestampAllowance::None, StampAt::None), + (StampAt::Send, TimestampAllowance::Dual, StampAt::Send), + (StampAt::Send, TimestampAllowance::Single, StampAt::Send), + (StampAt::Send, TimestampAllowance::None, StampAt::None), + (StampAt::Receive, TimestampAllowance::Dual, StampAt::Receive), + ( + StampAt::Receive, + TimestampAllowance::Single, + StampAt::Receive, + ), + (StampAt::Receive, TimestampAllowance::None, StampAt::None), + (StampAt::Both, TimestampAllowance::Dual, StampAt::Both), + (StampAt::Both, TimestampAllowance::Single, StampAt::Midpoint), + (StampAt::Both, TimestampAllowance::None, StampAt::None), + ( + StampAt::Midpoint, + TimestampAllowance::Dual, + StampAt::Midpoint, + ), + ( + StampAt::Midpoint, + TimestampAllowance::Single, + StampAt::Midpoint, + ), + (StampAt::Midpoint, TimestampAllowance::None, StampAt::None), + ] { + let params = Params { + stamp_at: requested, + clock: Clock::Both, + ..client_params() + }; + let negotiated = negotiated( + ServerConfig::default().with_timestamp_allowance(allowance), + ¶ms, + ); + + assert_eq!( + negotiated.stamp_at, expected, + "{requested:?} under {allowance:?}" + ); + assert_eq!( + negotiated.clock, + Clock::Both, + "{requested:?} under {allowance:?}: the allowance restricts placement, not the clock" + ); + } +} + +#[test] +fn a_timestamp_restriction_never_rewrites_the_requested_clock() { + // The clean evidence describes a timestamp *allowance*, and says nothing + // about restricting clock domains. A server that quietly changed the clock + // too would report a session the client never asked for — and a client + // enforcing strict negotiation would refuse an otherwise serviceable one. + let single = negotiated( + ServerConfig::default().with_timestamp_allowance(TimestampAllowance::Single), + &Params { + stamp_at: StampAt::Both, + clock: Clock::Wall, + ..client_params() + }, + ); + assert_eq!(single.stamp_at, StampAt::Midpoint); + assert_eq!(single.clock, Clock::Wall); + + let none = negotiated( + ServerConfig::default().with_timestamp_allowance(TimestampAllowance::None), + &Params { + stamp_at: StampAt::Both, + clock: Clock::Both, + ..client_params() + }, + ); + assert_eq!(none.stamp_at, StampAt::None); + assert_eq!( + none.clock, + Clock::Both, + "the clock tag stays exactly as requested, selecting nothing" + ); +} + +#[test] +fn an_allowed_dscp_is_negotiated_stored_and_transported_unchanged() { + // The default, and the behavioral compatibility this policy must not + // disturb: the raw byte reaches the reply, the session and the transport. + let requested = Params { + dscp: 0xbb, + ..client_params() + }; + let mut core = core_with_tokens(unthrottled(), ScriptedTokens::new([TOKEN_A])); + let open = core + .handle_datagram(peer(), &open_request(&requested, None)) + .unwrap() + .expect("the open must be answered"); + + let negotiated = expect_normal_open_reply(&open, None).params; + assert_eq!(negotiated.dscp, 0xbb); + assert_eq!( + core.session(TOKEN_A) + .expect("the session must be live") + .params() + .dscp, + 0xbb + ); + + let reply = core + .handle_datagram(peer(), &echo_request(TOKEN_A, 0, &negotiated, &[], None)) + .unwrap() + .expect("an admissible echo must be answered"); + expect_echo_reply(&reply, &negotiated, None); + assert_eq!(i64::from(reply.traffic_class()), 0xbb); +} + +#[test] +fn a_disallowed_dscp_is_negotiated_to_zero_and_the_session_runs_unmarked() { + // The open is not refused: the server simply will not provide the marking, + // and says so in the value it returns. Nothing downstream needs a second + // record of the policy — the negotiated zero is what the transport reads. + let requested = Params { + dscp: 0xb8, + ..client_params() + }; + let mut core = core_with_tokens( + unthrottled().with_dscp_allowed(false), + ScriptedTokens::new([TOKEN_A]), + ); + let open = core + .handle_datagram(peer(), &open_request(&requested, None)) + .unwrap() + .expect("a disallowed DSCP must not refuse the open"); + + let negotiated = expect_normal_open_reply(&open, None).params; + assert_eq!(negotiated.dscp, 0, "restricted to zero"); + assert_eq!( + core.session(TOKEN_A) + .expect("the session must be live") + .params() + .dscp, + 0, + "and stored as the value the session will be run with" + ); + + let reply = core + .handle_datagram(peer(), &echo_request(TOKEN_A, 0, &negotiated, &[], None)) + .unwrap() + .expect("an unmarked session is an ordinary session"); + expect_echo_reply(&reply, &negotiated, None); + assert_eq!(reply.traffic_class(), 0); +} + +#[test] +fn a_disallowed_dscp_restricts_a_value_no_socket_could_carry_too() { + // Restriction runs during negotiation, before the transport's + // "out-of-range means unmarked" fallback is relevant at all: these values + // never reach the session, so the reply is honest rather than merely + // harmless. + for dscp in [-1, 256, i64::MAX] { + let requested = Params { + dscp, + ..client_params() + }; + let mut core = core_with_tokens( + unthrottled().with_dscp_allowed(false), + ScriptedTokens::new([TOKEN_A]), + ); + let open = core + .handle_datagram(peer(), &open_request(&requested, None)) + .unwrap() + .unwrap_or_else(|| panic!("an open requesting DSCP {dscp} must still be answered")); + + let negotiated = expect_normal_open_reply(&open, None).params; + assert_eq!(negotiated.dscp, 0, "requested {dscp}"); + assert_eq!( + core.session(TOKEN_A) + .expect("the session must be live") + .params() + .dscp, + 0, + "requested {dscp}" + ); + + let reply = core + .handle_datagram(peer(), &echo_request(TOKEN_A, 0, &negotiated, &[], None)) + .unwrap() + .unwrap_or_else(|| panic!("the session must serve echoes after restricting {dscp}")); + expect_echo_reply(&reply, &negotiated, None); + assert_eq!(reply.traffic_class(), 0, "requested {dscp}"); + } +} diff --git a/crates/irtt-server/src/tests/no_test.rs b/crates/irtt-server/src/tests/no_test.rs index ce3687f..f2bc691 100644 --- a/crates/irtt-server/src/tests/no_test.rs +++ b/crates/irtt-server/src/tests/no_test.rs @@ -4,7 +4,7 @@ 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, unthrottled, ScriptedTokens, KEY, }; -use crate::ServerConfig; +use crate::{ServerConfig, TimestampAllowance}; const TOKEN_A: u64 = 0x0102_0304_0506_0708; const TOKEN_B: u64 = 0x1112_1314_1516_1718; @@ -222,3 +222,37 @@ fn an_empty_no_test_open_is_answered() { ); assert_eq!(core.session_count(), 0); } + +#[test] +fn a_no_test_open_reports_the_restricted_capabilities() { + // No-test exists to tell a client what the session would be, so it reports + // exactly what the same open would have negotiated under this policy — + // while still creating nothing and drawing no token. Capability policy is + // not skipped merely because no measurement session follows. + let tokens = ScriptedTokens::new([TOKEN_A]); + let mut core = core_with_tokens( + ServerConfig::default() + .with_timestamp_allowance(TimestampAllowance::Single) + .with_dscp_allowed(false), + tokens.clone(), + ); + let requested = Params { + stamp_at: StampAt::Both, + clock: Clock::Both, + dscp: 184, + ..client_params() + }; + + let packet = core + .handle_datagram(peer(), &no_test_request(&requested, None)) + .unwrap() + .expect("a no-test open must be answered under a restricting policy"); + + let reply = expect_no_test_reply(&packet, None); + assert_eq!(reply.params.stamp_at, StampAt::Midpoint); + assert_eq!(reply.params.clock, Clock::Both, "the clock is untouched"); + assert_eq!(reply.params.dscp, 0); + assert_eq!(reply.token, 0); + assert_eq!(core.session_count(), 0, "no-test creates no session"); + assert_eq!(tokens.remaining(), 1, "no-test must not draw a token"); +} diff --git a/crates/irtt-server/src/tests/open.rs b/crates/irtt-server/src/tests/open.rs index a9ff45f..5d95f24 100644 --- a/crates/irtt-server/src/tests/open.rs +++ b/crates/irtt-server/src/tests/open.rs @@ -1,10 +1,10 @@ -use irtt_proto::{Clock, Params, ReceivedStats, ServerFill, StampAt}; +use irtt_proto::{Clock, Params, ReceivedStats, ServerFill, StampAt, TimestampFields}; use super::support::{ - client_params, core_with_tokens, expect_normal_open_reply, open_request, - open_request_with_raw_params, param_int, peer, unthrottled, ScriptedTokens, KEY, + client_params, core_with_tokens, echo_request, expect_echo_reply, expect_normal_open_reply, + open_request, open_request_with_raw_params, param_int, peer, unthrottled, ScriptedTokens, KEY, }; -use crate::{ServerConfig, DEFAULT_MAX_PACKET_LENGTH}; +use crate::{ServerConfig, TimestampAllowance, DEFAULT_MAX_PACKET_LENGTH}; const TOKEN_A: u64 = 0x7896_b6ab_8771_5213; @@ -154,6 +154,10 @@ fn selecting_timestamps_without_a_clock_is_silently_refused() { // Only an *absent* Clock tag reaches this: an explicit zero is already out // of range for the decoder, and a conforming client always sends a clock // when it selects timestamps. + // + // A server whose timestamp allowance is none accepts the very same request, + // because then no timestamp is negotiated to be unproducible; see + // `a_timestamp_disallowing_server_accepts_a_request_that_omitted_its_clock`. for stamp_at in [ StampAt::Send, StampAt::Receive, @@ -189,11 +193,59 @@ fn selecting_timestamps_without_a_clock_is_silently_refused() { } } +#[test] +fn a_timestamp_disallowing_server_accepts_a_request_that_omitted_its_clock() { + // The same request the test above refuses, against a server that provides no + // timestamps at all. The restriction runs before the effective-session + // check, so `stamp_at` is already none by the time executability is judged: + // the absent clock then selects nothing that was going to be reported, and + // the session is one the server can run exactly as negotiated. + // + // This is what the ordering exists for. Validating the *request* instead + // would refuse an open that this policy makes perfectly serviceable. + let requested = Params { + clock: Clock::Unspecified, + stamp_at: StampAt::Both, + ..client_params() + }; + let mut core = core_with_tokens( + unthrottled().with_timestamp_allowance(TimestampAllowance::None), + ScriptedTokens::new([TOKEN_A]), + ); + + let packet = core + .handle_datagram(peer(), &open_request(&requested, None)) + .unwrap() + .expect("an omitted clock is safe once no timestamps are negotiated"); + + let reply = expect_normal_open_reply(&packet, None); + assert_eq!(reply.params.stamp_at, StampAt::None); + assert_eq!( + reply.params.clock, + Clock::Unspecified, + "the absent clock tag stays absent" + ); + assert_eq!(core.session_count(), 1); + + let echo = core + .handle_datagram( + peer(), + &echo_request(reply.token, 0, &reply.params, &[], None), + ) + .unwrap() + .expect("the session must serve echoes"); + assert_eq!( + expect_echo_reply(&echo, &reply.params, None).timestamps, + TimestampFields::default(), + "no timestamps were negotiated, so the reply carries none" + ); +} + #[test] fn current_non_length_parameters_survive_negotiation_unchanged() { - // Statistics, timestamps and DSCP are the values a later restriction-policy - // slice will deliberately start changing. Pinning them now makes that a - // visible decision rather than an accident. Version, an oversized packet + // Statistics, timestamps and DSCP are what the capability restrictions + // change when they are configured; this is the default configuration, which + // restricts none of them. Version, an oversized packet // length, the duration maximum and the interval floor and cap are already // restricted and have their own tests; the length here is deliberately // under the default maximum, and the configuration below leaves duration diff --git a/crates/irtt-server/tests/negotiation_policy.rs b/crates/irtt-server/tests/negotiation_policy.rs new file mode 100644 index 0000000..1fde645 --- /dev/null +++ b/crates/irtt-server/tests/negotiation_policy.rs @@ -0,0 +1,225 @@ +//! The server's capability restrictions as a real client sees them. +//! +//! The core tests are authoritative for the negotiated values; these exist to +//! prove the two ends agree over a socket. A restriction this server applies has +//! to arrive at `irtt-client` as one of the restrictions it already models: a +//! strict client refuses the changed value, and a loose one accepts it, records +//! it and runs the session. Nothing in the client changed for this — that is +//! what is being checked. + +use std::time::Duration; + +use irtt_client::{ + AsyncClient, ClientConfig, NegotiationPolicy, NegotiationRestriction, OpenOutcome, SocketConfig, +}; +use irtt_proto::{Clock, ReceivedStats, StampAt}; +use irtt_server::{Server, ServerConfig, TimestampAllowance}; +use tokio::{sync::oneshot, time::timeout}; + +#[tokio::test(flavor = "current_thread")] +async fn a_single_timestamp_allowance_reaches_a_strict_and_a_loose_client() { + timeout(Duration::from_secs(2), exercise_timestamp_restriction()) + .await + .expect("the timestamp negotiation integration exceeded its bounded runtime"); +} + +async fn exercise_timestamp_restriction() { + let server = ServerConfig::default().with_timestamp_allowance(TimestampAllowance::Single); + let (server_addr, shutdown_tx, server_task) = serve(server).await; + + let mut strict = client(server_addr, NegotiationPolicy::Strict).await; + let rejected = strict + .open() + .await + .expect_err("a strict client must refuse the reduced timestamp placement"); + assert!( + rejected.to_string().contains("stamp-at"), + "the restriction names the timestamp placement: {rejected}" + ); + + let mut loose = client(server_addr, NegotiationPolicy::Loose).await; + let negotiated = match loose.open().await.unwrap() { + OpenOutcome::Started { negotiated, .. } => negotiated, + OpenOutcome::NoTestCompleted { .. } => panic!("normal client unexpectedly ran no-test"), + }; + assert_eq!( + negotiated.params.stamp_at, + StampAt::Midpoint, + "a request for both instants is answered with the midpoint" + ); + assert_eq!( + negotiated.params.clock, + Clock::Both, + "the clock is not restricted with it" + ); + assert!(negotiated + .restrictions + .contains(&NegotiationRestriction::StampAtChanged { + requested: StampAt::Both, + negotiated: StampAt::Midpoint, + })); + + probe(&mut loose).await; + loose.close().await.unwrap(); + + shutdown_tx.send(()).unwrap(); + server_task.await.unwrap().unwrap(); +} + +#[tokio::test(flavor = "current_thread")] +async fn a_disallowed_dscp_reaches_a_strict_and_a_loose_client() { + timeout(Duration::from_secs(2), exercise_dscp_restriction()) + .await + .expect("the DSCP negotiation integration exceeded its bounded runtime"); +} + +async fn exercise_dscp_restriction() { + let (server_addr, shutdown_tx, server_task) = serve( + ServerConfig::default() + .with_min_send_interval(Duration::ZERO) + .with_dscp_allowed(false), + ) + .await; + + // The client's configuration is a DSCP codepoint; 46 (EF) is raw byte 184 on + // the wire, and this test does not change either convention. + let with_dscp = |policy| ClientConfig { + dscp: 46, + ..test_config(server_addr, policy) + }; + + let mut strict = AsyncClient::connect(with_dscp(NegotiationPolicy::Strict)) + .await + .unwrap(); + let rejected = strict + .open() + .await + .expect_err("a strict client must refuse the removed marking"); + assert!( + rejected.to_string().contains("DSCP"), + "the restriction names DSCP: {rejected}" + ); + + let mut loose = AsyncClient::connect(with_dscp(NegotiationPolicy::Loose)) + .await + .unwrap(); + let negotiated = match loose.open().await.unwrap() { + OpenOutcome::Started { negotiated, .. } => negotiated, + OpenOutcome::NoTestCompleted { .. } => panic!("normal client unexpectedly ran no-test"), + }; + assert_eq!( + negotiated.params.dscp, 0, + "the raw wire parameter is negotiated to zero" + ); + assert!(negotiated + .restrictions + .contains(&NegotiationRestriction::DscpChanged { + requested: 46, + negotiated: 0, + })); + + probe(&mut loose).await; + loose.close().await.unwrap(); + + shutdown_tx.send(()).unwrap(); + server_task.await.unwrap().unwrap(); +} + +#[tokio::test(flavor = "current_thread")] +async fn a_default_server_restricts_nothing_an_ordinary_client_asks_for() { + timeout(Duration::from_secs(2), exercise_default_policy()) + .await + .expect("the default policy integration exceeded its bounded runtime"); +} + +/// The controls are opt-in, and this is what that has to mean: an ordinary +/// client against an unconfigured server negotiates with no restriction at all, +/// under the default *strict* policy that would refuse one. +async fn exercise_default_policy() { + let (server_addr, shutdown_tx, server_task) = serve(ServerConfig::default()).await; + + let mut client = AsyncClient::connect(ClientConfig { + dscp: 46, + ..test_config(server_addr, NegotiationPolicy::Strict) + }) + .await + .unwrap(); + let negotiated = match client.open().await.unwrap() { + OpenOutcome::Started { negotiated, .. } => negotiated, + OpenOutcome::NoTestCompleted { .. } => panic!("normal client unexpectedly ran no-test"), + }; + + assert_eq!(negotiated.restrictions, Vec::new()); + assert_eq!(negotiated.params.stamp_at, StampAt::Both); + assert_eq!(negotiated.params.clock, Clock::Both); + assert_eq!(negotiated.params.dscp, 184, "the raw byte for codepoint 46"); + + probe(&mut client).await; + client.close().await.unwrap(); + + shutdown_tx.send(()).unwrap(); + server_task.await.unwrap().unwrap(); +} + +/// Starts one listener and returns its address, its shutdown trigger and its +/// task. +async fn serve( + config: ServerConfig, +) -> ( + std::net::SocketAddr, + oneshot::Sender<()>, + tokio::task::JoinHandle>, +) { + let mut server = Server::bind("127.0.0.1:0".parse().unwrap(), config) + .await + .unwrap(); + let server_addr = server.local_addr().unwrap(); + let (shutdown_tx, shutdown_rx) = oneshot::channel(); + let task = tokio::spawn(async move { + server + .run(async { + let _ = shutdown_rx.await; + }) + .await + }); + (server_addr, shutdown_tx, task) +} + +/// A short deterministic client requesting both timestamps from both clocks. +fn test_config(server_addr: std::net::SocketAddr, policy: NegotiationPolicy) -> ClientConfig { + ClientConfig { + server_addr: server_addr.to_string(), + duration: Some(Duration::from_secs(1)), + interval: Duration::from_millis(100), + length: 64, + received_stats: ReceivedStats::Both, + stamp_at: StampAt::Both, + clock: Clock::Both, + negotiation_policy: policy, + open_timeouts: vec![Duration::from_millis(200)], + socket_config: SocketConfig { + recv_timeout: Some(Duration::from_millis(200)), + ..SocketConfig::default() + }, + probe_timeout: Duration::from_millis(200), + ..ClientConfig::default() + } +} + +async fn client(server_addr: std::net::SocketAddr, policy: NegotiationPolicy) -> AsyncClient { + AsyncClient::connect(test_config(server_addr, policy)) + .await + .unwrap() +} + +/// One probe, answered: a restricted session is an ordinary session. +async fn probe(client: &mut AsyncClient) { + client.send_probe().await.unwrap(); + let events = client.recv().await.unwrap(); + assert!( + events + .iter() + .any(|event| matches!(event, irtt_client::ClientEvent::EchoReply { seq: 0, .. })), + "the restricted session must answer an ordinary probe: {events:?}" + ); +}