From e4239f45c59198155c55320343a818c37e203638 Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Fri, 14 Aug 2026 12:58:30 +0700 Subject: [PATCH] feat(server): generate echo payload fill --- AGENTS.md | 2 +- README.md | 12 +- crates/irtt-server/AGENTS.md | 55 ++++- crates/irtt-server/src/config.rs | 16 +- crates/irtt-server/src/core.rs | 23 +- crates/irtt-server/src/fill.rs | 125 ++++++++++- crates/irtt-server/src/lib.rs | 19 +- crates/irtt-server/src/tests/echo.rs | 17 +- crates/irtt-server/src/tests/fill.rs | 251 +++++++++++++++++++++- crates/irtt-server/src/tests/lifecycle.rs | 21 +- crates/irtt-server/tests/async_client.rs | 5 + crates/irtt-server/tests/server_fill.rs | 216 +++++++++++++++++++ 12 files changed, 721 insertions(+), 41 deletions(-) create mode 100644 crates/irtt-server/tests/server_fill.rs diff --git a/AGENTS.md b/AGENTS.md index c2c54de..942a6fb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -57,7 +57,7 @@ Tokio must remain optional for `irtt-client`; default builds must remain runtime ### Server architecture (`irtt-server`) -`irtt-server` is a first-class reusable server crate. It contains the deterministic OPEN/ECHO/session core — packet admission, authentication policy, open negotiation, the bounded session table, open reply construction, echo processing with its per-session receive state and timestamps, per-session rate limiting, session lifetime with idle expiry, the maximum-duration server-initiated close, and client-initiated close — plus a reusable Tokio UDP `Server` that runs one sequential `ServerCore` per listener with caller-controlled shutdown and scheduled idle maintenance. Logical expiry remains exact on authenticated, structurally valid requests, while the runtime also reclaims expired sessions once per second when no traffic arrives. Each reply carries the raw traffic class it must be sent with, which the runtime applies to the listener before every send. A wildcard listener additionally recovers each request's local destination from packet metadata and sends that request's reply from it, on Linux, macOS and FreeBSD; a wildcard bind on a target without that path is refused at construction rather than served from a routing-table source address. Explicit-address listeners are unaffected everywhere. The full server fill policy remains unimplemented; see `crates/irtt-server/AGENTS.md`. The rest of this section describes the implemented boundary and the parts not yet written. +`irtt-server` is a first-class reusable server crate. It contains the deterministic OPEN/ECHO/session core — packet admission, authentication policy, open negotiation, the bounded session table, open reply construction, echo processing with its per-session receive state and timestamps, per-session rate limiting, session lifetime with idle expiry, the maximum-duration server-initiated close, and client-initiated close — plus a reusable Tokio UDP `Server` that runs one sequential `ServerCore` per listener with caller-controlled shutdown and scheduled idle maintenance. Logical expiry remains exact on authenticated, structurally valid requests, while the runtime also reclaims expired sessions once per second when no traffic arrives. Each reply carries the raw traffic class it must be sent with, which the runtime applies to the listener before every send. A wildcard listener additionally recovers each request's local destination from packet metadata and sends that request's reply from it, on Linux, macOS and FreeBSD; a wildcard bind on a target without that path is refused at construction rather than served from a routing-table source address. Explicit-address listeners are unaffected everywhere. Echo replies fill their payload from the session's negotiated server fill; the policy and its deliberate divergences are in `crates/irtt-server/AGENTS.md`. Multiple listeners in one process, and optional timestamp and DSCP restriction controls, are not implemented. - It shares wire semantics with the client through `irtt-proto` and follows the same clean-room boundary. - It must **not** depend on `irtt-client` merely to reuse protocol behavior. diff --git a/README.md b/README.md index 70b8ee5..e9f74fe 100644 --- a/README.md +++ b/README.md @@ -308,6 +308,12 @@ The client, event stream, machine-readable output, multi-target execution, local The server library and the server applet are implemented: open negotiation, session state, echo processing, per-session rate limiting, idle expiry, the maximum-duration close, HMAC authentication, the negotiated per-session reply -traffic class, which is applied on sockets that support it, and wildcard -reply-source selection on Linux, macOS and FreeBSD. The full server fill policy -and multiple listeners in one process are not implemented. +traffic class, which is applied on sockets that support it, server fill, and +wildcard reply-source selection on Linux, macOS and FreeBSD. Multiple listeners +in one process are not implemented, and optional timestamp and DSCP restriction +controls remain separate concerns. + +Server replies fill their payload with the negotiated ServerFill mode: `none`, +which is zero-filled, `rand`, or `pattern:` with a repeating hexadecimal +pattern. Every valid descriptor is honored; one the server cannot parse, and a +client that requests none, get the default `pattern:69727474`. diff --git a/crates/irtt-server/AGENTS.md b/crates/irtt-server/AGENTS.md index 1f9b178..3653d16 100644 --- a/crates/irtt-server/AGENTS.md +++ b/crates/irtt-server/AGENTS.md @@ -202,11 +202,56 @@ effective parameters must be safe for the server to run. - For a single-clock midpoint, `irtt-rs` emits the one negotiated field. It does not reproduce upstream 0.9.1's dual-field midpoint; `irtt-proto` still decodes that form from a peer. -- Reply payload bytes are zero, which is the initial safe fill policy: payload - carries no protocol meaning, and a server must never emit residue from another - request or client. Request payload bytes never reach a reply. -- The full `ServerFill` policy is a later slice. Do not add placeholder session - fields for it. +- Reply payload bytes come from the session's fill; see "Server fill" below. + Request payload bytes never reach a reply under any mode. + +## Server fill + +The ServerFill *string* is wire data `irtt-proto` carries verbatim, and it +already enforces the 32-byte, UTF-8 and truncation rules. What a descriptor +*means* is server policy and lives in `fill`. Do not move descriptor parsing +into `irtt-proto`, and do not duplicate the decoder's checks here. + +- **All three valid descriptor families are accepted**: `none`, `rand` and + `pattern:HH…`. Mode names are case-sensitive; only a pattern's hexadecimal + body is not. This is deliberately more permissive than the reference server's + configurable glob allow-list, which is upstream policy rather than an + interoperability requirement. Do **not** add glob, regex or allow-list + configuration, and do not add server fill CLI knobs. +- **Absent or empty means no preference.** The negotiated params keep their + representation exactly — absent stays absent, empty stays empty — while the + session uses the server default internally. Writing the default descriptor + into the reply would manufacture a restriction, which a strict client rightly + rejects. +- **The default and fallback descriptor is `pattern:69727474`**, the bytes + `irtt`. The descriptor and the bytes are separate constants; neither is + derived from the other at run time. +- **An unknown or malformed explicit descriptor is replaced** in the negotiated + params by that default descriptor, because the server really did change what + was asked for. The client models it as `ServerFillChanged`; do not weaken + client negotiation to hide it. +- **`none` zero-fills.** The clean evidence records the reference server leaving + the region as residual buffer content, returning other clients' bytes. Zero is + produced by handing the encoder an empty payload, not by copying zeroes over + an already-zeroed packet. +- **Pattern phase resets for every reply.** Upstream's observed cross-packet and + cross-listener continuity is explicitly not required of a conforming server, + and per-packet reset keeps the server free of global mutable fill state and + makes replies deterministic. There is no shared phase between sessions or + listeners, and adding one would be a regression. +- **`FillMode` belongs to the `Session`**, parsed once at open. Never re-derive + it from `params` later: that would lose the absent-descriptor-with-default- + behavior distinction. Keep the type crate-private. +- **Fill is generated before the reply is encoded**, like every other part of an + echo, so the packet's HMAC covers the fill bytes through the ordinary encoder + path and no session state commits before encoding succeeds. Never mutate a + payload after encoding. +- **A random-source failure zero-fills that one payload and is not fatal.** It + must never become a `ServerError`, terminate `Server::run` or drop the + session. The bytes exist to vary the payload and are not security state; + session tokens keep their own source. +- 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. ## Reply traffic class diff --git a/crates/irtt-server/src/config.rs b/crates/irtt-server/src/config.rs index 2cbd84c..21e5b00 100644 --- a/crates/irtt-server/src/config.rs +++ b/crates/irtt-server/src/config.rs @@ -56,9 +56,19 @@ pub const DEFAULT_IDLE_TIMEOUT: Duration = Duration::from_secs(60); /// Configuration for a [`ServerCore`](crate::ServerCore). /// /// Fields are private and set through consuming builder methods, so later -/// slices can add configuration without breaking construction. The remaining -/// parameter restriction knobs — the timestamp allowance, DSCP policy and the -/// server-fill allow-list — arrive with the slices that implement them. +/// slices can add configuration without breaking construction. Optional +/// timestamp-allowance and DSCP restriction controls would arrive with the +/// slices that implement them. +/// +/// **Server fill is deliberately not configurable.** The policy is fixed and +/// safe: every valid descriptor is honored, one that cannot be parsed falls back +/// to `pattern:69727474`, and a client expressing no preference is served that +/// same default. There is no allow-list to configure, because refusing a valid +/// mode would prevent nothing — the payload carries no protocol meaning, a +/// descriptor is bounded to 32 wire bytes, a random fill is bounded by +/// [`max_packet_length`](Self::max_packet_length) like every other reply, and +/// `none` is zero-filled here rather than left as residue. An operator +/// restriction can be added if a real use case turns up. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ServerConfig { hmac_key: Option>, diff --git a/crates/irtt-server/src/core.rs b/crates/irtt-server/src/core.rs index 594f64a..66cdeff 100644 --- a/crates/irtt-server/src/core.rs +++ b/crates/irtt-server/src/core.rs @@ -10,7 +10,7 @@ use crate::{ clock::{saturating_ns, ClockSample, ClockSource, SystemClock}, config::ServerConfig, error::ServerError, - fill::negotiate_server_fill, + fill::{echo_payload_len, negotiate_server_fill}, negotiate::negotiate_params, session::{RateDecision, Session}, token::{OsTokenSource, TokenSource, TOKEN_ATTEMPTS}, @@ -49,8 +49,8 @@ use crate::{ /// normal echo processing, per-session rate limiting, idle expiry, /// maximum-duration close and client-initiated close. It decides the traffic /// class each reply is to be sent with; actually applying it to a socket is the -/// runtime's job. The full server fill policy is a separate slice, and echo -/// payloads are zero-filled. Incoming authenticated requests enforce logical +/// runtime's job. It also settles each session's server fill and generates the +/// payload bytes of every echo reply. Incoming authenticated requests enforce logical /// expiry exactly; the runtime also calls a private maintenance hook to reclaim /// idle sessions when no traffic arrives. #[derive(Debug)] @@ -356,6 +356,10 @@ impl ServerCore { // it. let sent_at = self.clock.sample(); let params = session.params(); + // Fill bytes are prepared before the reply is built, so they go through + // the encoder's normal path and the packet's MAC covers them like every + // other byte. Nothing mutates a payload after encoding. + let payload_len = echo_payload_len(self.config.hmac_key().is_some(), params); // Read before the session can be released below: a maximum-duration // close is an ordinary echo reply and carries the session's marking like // any other. @@ -374,12 +378,13 @@ impl ServerCore { recv_count: params.received_stats.has_count().then_some(next.count), recv_window: params.received_stats.has_window().then_some(next.window), timestamps: timestamp_fields(params, request.received_at, sent_at), - // No payload bytes of our own: the encoder zero-fills the whole - // region between the field block and the negotiated length. That is - // the initial safe fill policy — payload bytes carry no protocol - // meaning, and zeroes cannot disclose residue from another request - // or another client. Request payload bytes never reach a reply. - payload: Vec::new(), + // The session's negotiated fill, generated fresh for this reply and + // sized from the layout rather than from `params.length`. A `none` + // session contributes no bytes and the encoder's zero-fill stands, + // which is what keeps a no-fill reply from disclosing residue. + // Nothing from the request reaches here: its payload is opaque, and + // ServerFill describes the server's own bytes. + payload: session.fill().payload(payload_len), }; let packet = encode_echo_reply(&reply, params, self.config.hmac_key()) .map_err(|source| ServerError::ReplyEncoding { source })?; diff --git a/crates/irtt-server/src/fill.rs b/crates/irtt-server/src/fill.rs index 0a8f5f5..82f89a7 100644 --- a/crates/irtt-server/src/fill.rs +++ b/crates/irtt-server/src/fill.rs @@ -6,7 +6,7 @@ //! [`FillMode`], which is deliberately crate-private: it is neither wire format //! nor public configuration. -use irtt_proto::{Params, ServerFill}; +use irtt_proto::{echo_packet_len, PacketLayout, Params, ServerFill}; /// The bytes of this server's default fill, repeated: `69 72 74 74`, or `irtt`. /// @@ -70,6 +70,34 @@ impl FillMode { .map(Self::Pattern), } } + + /// The `len` payload bytes an echo reply carries under this mode. + /// + /// **A pattern always starts at its first byte.** The phase resets for every + /// reply, which is `irtt-rs` policy: the clean evidence records the + /// reference implementation advancing one phase continuously across replies, + /// sessions and listeners, and states equally clearly that payload phase has + /// no protocol significance and that a conforming client must not depend on + /// it. Resetting is therefore interoperability-equivalent, and it buys + /// determinism — no global mutable fill state, no coupling between sessions + /// or listeners, and a reply whose bytes a test can state exactly. + /// + /// Infallible by construction. A pattern and an empty region cannot fail, + /// and a random draw that does falls back to zeroes rather than failing the + /// reply. + pub(crate) fn payload(&self, len: usize) -> Vec { + match self { + // No allocation and no copy: the encoder already zero-fills the + // whole region between the field block and the negotiated length, + // so an empty payload *is* the zero payload. Zero is the policy — + // the clean evidence records the reference server leaving this + // region as unspecified residual buffer content, which returned + // other traffic's bytes, and a compatible server must not. + Self::None => Vec::new(), + Self::Random => random_payload(len, getrandom::fill), + Self::Pattern(pattern) => pattern.iter().copied().cycle().take(len).collect(), + } + } } /// The pattern a `pattern:` descriptor's body names, or `None` if the body is @@ -102,6 +130,30 @@ fn hex_digit(digit: u8) -> Option { } } +/// `len` random bytes drawn with `draw`, or `len` zeroes if the draw failed. +/// +/// `draw` is a parameter rather than a direct call so the failure path is +/// assertable without a production hook: the caller passes the operating +/// system's random source and one unit test passes a scripted one. An empty +/// region asks the source for nothing at all. +/// +/// **A failed draw zero-fills and is not an error.** Payload bytes carry no +/// protocol meaning, so a random source having a bad afternoon must not cost a +/// session its reply, let alone take the server down: the reply is still +/// structurally valid and interoperable. The buffer is rewritten rather than +/// trusted, because a failed draw leaves its contents unspecified. +/// +/// The bytes exist to vary the payload, and nothing here is a security claim. +/// `getrandom` is used because the crate already depends on it; session tokens, +/// which *are* security state, keep their own separate source. +fn random_payload(len: usize, draw: impl FnOnce(&mut [u8]) -> Result<(), E>) -> Vec { + let mut payload = vec![0; len]; + if len > 0 && draw(&mut payload).is_err() { + payload.fill(0); + } + payload +} + /// Settles what fill a session will actually use, restricting `params` only /// where this server could not honor what was asked for. /// @@ -151,6 +203,25 @@ pub(crate) fn negotiate_server_fill(params: &mut Params) -> FillMode { } } +/// The payload region of one echo reply under `params`, in bytes. +/// +/// Derived from `irtt-proto`'s own layout and sizing rather than from +/// `params.length`, which is neither the packet length nor the payload length: +/// the mandatory field block, the negotiated statistics and timestamps, and +/// authentication's 16 bytes all consume packet space, and a length below the +/// field block asks for no payload at all. +/// +/// An unrepresentable length yields zero rather than an error. An acknowledged +/// session cannot reach it — the open path already required [`echo_packet_len`] +/// to succeed — and if one somehow did, encoding the reply would fail on that +/// same call, which is where the failure belongs. +/// +/// [`echo_packet_len`]: irtt_proto::echo_packet_len +pub(crate) fn echo_payload_len(hmac: bool, params: &Params) -> usize { + let header_len = PacketLayout::echo(hmac, params).header_len(); + echo_packet_len(hmac, params).map_or(0, |len| len.saturating_sub(header_len)) +} + #[cfg(test)] mod tests { use super::*; @@ -205,4 +276,56 @@ mod tests { Some(FillMode::default_fill()) ); } + + #[test] + fn a_pattern_repeats_from_its_first_byte_and_stops_at_the_region() { + let mode = FillMode::Pattern(vec![0xaa, 0xbb]); + assert_eq!(mode.payload(0), Vec::::new()); + assert_eq!(mode.payload(1), vec![0xaa]); + assert_eq!( + mode.payload(7), + vec![0xaa, 0xbb, 0xaa, 0xbb, 0xaa, 0xbb, 0xaa] + ); + } + + #[test] + fn no_fill_leaves_the_region_to_the_encoder() { + // Not a length-sized run of zeroes: the encoder has already zeroed the + // region, and copying zeroes over zeroes would only cost an allocation. + assert!(FillMode::None.payload(16).is_empty()); + } + + #[test] + fn a_failed_random_draw_yields_zeroes() { + // The real source cannot be made to fail without a production hook, and + // the fallback itself is the policy under test: a failed draw must + // produce a full-length payload of zeroes rather than an error or the + // buffer's unspecified contents. + let scribble = |buffer: &mut [u8]| { + buffer.fill(0xab); + Err(()) + }; + assert_eq!(random_payload(6, scribble), vec![0; 6]); + } + + #[test] + fn a_successful_random_draw_is_propagated_whole() { + // Deterministic on purpose. Nothing here asserts that real random bytes + // are nonzero, distinct or well distributed; those are properties of + // the operating system's source, and testing them would be a coin flip + // dressed up as an assertion. + let drawn = [1, 2, 3, 4, 5]; + let source = |buffer: &mut [u8]| -> Result<(), ()> { + assert_eq!(buffer.len(), drawn.len(), "the whole region is offered"); + buffer.copy_from_slice(&drawn); + Ok(()) + }; + assert_eq!(random_payload(drawn.len(), source), drawn); + } + + #[test] + fn an_empty_region_never_reaches_the_random_source() { + let source = |_: &mut [u8]| -> Result<(), ()> { panic!("no bytes were needed") }; + assert!(random_payload(0, source).is_empty()); + } } diff --git a/crates/irtt-server/src/lib.rs b/crates/irtt-server/src/lib.rs index bfd2eb7..0dc8247 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, -//! wildcard reply-source selection and Tokio UDP orchestration. The full server -//! fill policy and running several listeners in one process remain separate -//! slices. Echo payloads are zero-filled. +//! 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 +//! +//! An echo reply's payload region is filled from the session's negotiated +//! ServerFill descriptor: `none`, `rand`, or `pattern:` followed by a repeating +//! hexadecimal pattern. Every valid descriptor is accepted — there is no +//! allow-list to configure — and one this server cannot parse is answered with +//! its default, `pattern:69727474`, which is also what a client expressing no +//! preference is served without any change to the negotiated parameters. +//! +//! A `none` payload is zeroes. Payload bytes carry no protocol meaning, and a +//! server must never emit residue from another request or client. Request +//! payload bytes never reach a reply under any mode. #![forbid(unsafe_code)] mod clock; diff --git a/crates/irtt-server/src/tests/echo.rs b/crates/irtt-server/src/tests/echo.rs index b2247ed..61b21f7 100644 --- a/crates/irtt-server/src/tests/echo.rs +++ b/crates/irtt-server/src/tests/echo.rs @@ -725,7 +725,11 @@ fn a_single_clock_midpoint_emits_exactly_one_field() { } #[test] -fn a_reply_payload_is_zero_filled_and_never_reflects_the_request() { +fn a_reply_payload_is_server_generated_and_never_reflects_the_request() { + // The inbound tail is opaque, and ServerFill describes the *server's* bytes: + // a request's payload is neither echoed, mixed in, nor used to seed + // anything. This session expressed no fill preference, so the region + // carries the server's default fill; `fill` covers the modes themselves. let params = echo_params(ReceivedStats::None, StampAt::None, Clock::Unspecified, 64); let (mut core, token, negotiated) = session_with(¶ms); @@ -744,7 +748,14 @@ fn a_reply_payload_is_zero_filled_and_never_reflects_the_request() { assert_eq!(packet.bytes().len(), 64); assert_eq!(reply.payload.len(), 48); assert!( - reply.payload.iter().all(|byte| *byte == 0), - "the payload region must be zero-filled, never request or buffer residue" + !reply.payload.contains(&0xa5), + "no request byte may appear in the reply payload" + ); + assert!( + reply + .payload + .chunks(4) + .all(|chunk| chunk == &b"irtt"[..chunk.len()]), + "the region carries the server's default fill" ); } diff --git a/crates/irtt-server/src/tests/fill.rs b/crates/irtt-server/src/tests/fill.rs index 2504b73..e062557 100644 --- a/crates/irtt-server/src/tests/fill.rs +++ b/crates/irtt-server/src/tests/fill.rs @@ -1,21 +1,31 @@ -//! Server fill negotiation: what an open reply reports, and what the session -//! will actually fill with. +//! Server fill: what an open reply reports, what the session will fill with, +//! and the bytes an echo reply actually carries. //! -//! The two are deliberately different questions. A client that expressed no -//! preference keeps its absent or empty descriptor in the negotiated parameters -//! while the session uses this server's default fill, and only a descriptor the -//! server could not honor is rewritten. Asserting the reply alone would not -//! separate those, so these tests read the session's effective mode too. +//! The first two are deliberately different questions. A client that expressed +//! no preference keeps its absent or empty descriptor in the negotiated +//! parameters while the session uses this server's default fill, and only a +//! descriptor the server could not honor is rewritten. Asserting the reply alone +//! would not separate those, so the negotiation tests read the session's +//! effective mode too, and the payload tests then pin the bytes on the wire. +//! +//! Payload lengths here are derived from `irtt-proto`'s own layout rather than +//! written as offsets, and are deliberately not multiples of the pattern length. -use irtt_proto::{Params, ServerFill}; +use irtt_proto::{echo_header_len, Clock, Params, ReceivedStats, ServerFill, StampAt}; use super::support::{ - client_params, core_with_tokens, expect_no_test_reply, expect_normal_open_reply, - no_test_request, open_request, peer, unthrottled, ScriptedTokens, + client_params, core_for, core_with_tokens, echo_params, echo_request, expect_echo_reply, + expect_no_test_reply, expect_normal_open_reply, no_test_request, open_negotiated, open_request, + other_peer, peer, unthrottled, ScriptedTokens, KEY, }; use crate::{core::ServerCore, fill::FillMode}; const TOKEN_A: u64 = 0x0102_0304_0506_0708; +const TOKEN_B: u64 = 0x1112_1314_1516_1718; + +/// Request payload bytes an echo carries, so a reply reflecting them would be +/// obvious. +const REQUEST_FILLER: u8 = 0xa5; /// Params requesting `descriptor` as the server fill. fn requesting(descriptor: Option<&str>) -> Params { @@ -152,3 +162,224 @@ fn a_no_test_open_negotiates_fill_exactly_as_a_normal_open_does() { Some(crate::fill::DEFAULT_FILL_DESCRIPTOR) ); } + +/// Params for a session requesting `descriptor` whose echo replies have exactly +/// `payload_len` payload bytes. +/// +/// The negotiated length is the mandatory field block plus the region wanted, +/// asked of `irtt-proto` rather than written as a constant: the block grows with +/// the negotiated statistics, the timestamps and authentication's 16 bytes, and +/// a test that hard-coded an offset would silently drift from the encoder. +fn payload_params(descriptor: Option<&str>, payload_len: usize, hmac: bool) -> Params { + let mut params = Params { + server_fill: requesting(descriptor).server_fill, + ..echo_params(ReceivedStats::None, StampAt::None, Clock::Unspecified, 0) + }; + params.length = (echo_header_len(hmac, ¶ms) + payload_len) as i64; + params +} + +/// The payload regions of `count` successive echo replies on one session +/// requesting `descriptor`. +/// +/// Every request carries [`REQUEST_FILLER`] payload bytes, so a reply that +/// reflected its request would be visible in any of these tests rather than only +/// in the one about it. +fn echo_payloads( + descriptor: Option<&str>, + payload_len: usize, + hmac_key: Option<&[u8]>, + count: u32, +) -> Vec> { + let requested = payload_params(descriptor, payload_len, hmac_key.is_some()); + let mut core = core_for(hmac_key, ScriptedTokens::new([TOKEN_A])); + let (token, negotiated) = open_negotiated(&mut core, peer(), &requested, hmac_key); + + // The request fills its own payload region, which is the same size as the + // reply's: the strongest shape for "a reply never reflects its request". + let request_payload = vec![REQUEST_FILLER; payload_len]; + (0..count) + .map(|sequence| { + let request = echo_request(token, sequence, &negotiated, &request_payload, hmac_key); + let packet = core + .handle_datagram(peer(), &request) + .unwrap() + .expect("an admissible echo must be answered"); + let reply = expect_echo_reply(&packet, &negotiated, hmac_key); + assert_eq!( + packet.bytes().len() as i64, + negotiated.length, + "a reply is exactly the negotiated length" + ); + assert_eq!(reply.payload.len(), payload_len, "payload region length"); + reply.payload + }) + .collect() +} + +/// The payload region of one echo reply on a session requesting `descriptor`. +fn echo_payload(descriptor: Option<&str>, payload_len: usize) -> Vec { + echo_payloads(descriptor, payload_len, None, 1).remove(0) +} + +/// `pattern` repeated to `len` bytes, starting at its first byte. +fn repeated(pattern: &[u8], len: usize) -> Vec { + pattern.iter().copied().cycle().take(len).collect() +} + +#[test] +fn no_preference_fills_with_the_default_pattern() { + // The wire consequence of the negotiation test above: absent descriptor, + // default bytes. 7 is not a multiple of 4, so the last repeat is partial. + assert_eq!(echo_payload(None, 7), b"irttirt".to_vec()); +} + +#[test] +fn an_empty_preference_fills_with_the_default_pattern() { + assert_eq!(echo_payload(Some(""), 7), b"irttirt".to_vec()); +} + +#[test] +fn an_unknown_or_malformed_descriptor_fills_with_the_default_pattern() { + for descriptor in ["bogus", "RAND", "pattern:", "pattern:abc", "pattern:zz"] { + assert_eq!( + echo_payload(Some(descriptor), 7), + b"irttirt".to_vec(), + "{descriptor}" + ); + } +} + +#[test] +fn no_fill_produces_a_zero_payload_and_never_the_request() { + // `irtt-rs` policy, and a deliberate divergence: the clean evidence records + // the reference server leaving this region as unspecified residual buffer + // content — returning the requester's own payload, or fragments of earlier + // traffic from another client. Zeroes disclose nothing. + let payload = echo_payload(Some("none"), 7); + assert_eq!(payload, vec![0; 7]); + assert!(!payload.contains(&REQUEST_FILLER)); +} + +#[test] +fn patterns_repeat_from_their_first_byte() { + for (descriptor, expected) in [ + ("pattern:00", &[0x00][..]), + ("pattern:ff00", &[0xff, 0x00][..]), + // The reply follows the bytes the descriptor decodes to, whatever case + // it was written in. + ("pattern:AaBb", &[0xaa, 0xbb][..]), + ("pattern:69727474", b"irtt"), + ] { + assert_eq!( + echo_payload(Some(descriptor), 7), + repeated(expected, 7), + "{descriptor}" + ); + } +} + +#[test] +fn a_pattern_restarts_for_every_reply() { + // Deliberate `irtt-rs` policy. The clean evidence records the reference + // implementation advancing one phase continuously across replies, sessions + // and listeners, and states that payload phase carries no protocol meaning + // and that a conforming client must not depend on it — so this server keeps + // no fill state at all and every reply starts at byte zero. + let payloads = echo_payloads(Some("pattern:010203"), 5, None, 2); + assert_eq!(payloads[0], vec![0x01, 0x02, 0x03, 0x01, 0x02]); + assert_eq!( + payloads[1], payloads[0], + "the phase resets rather than continuing at 03 01 02 03 01" + ); +} + +#[test] +fn sessions_fill_independently() { + // Interleaved on one core, so a shared cursor or a global phase would show + // up as one session's bytes drifting into the other's. + let mut core = core_with_tokens(unthrottled(), ScriptedTokens::new([TOKEN_A, TOKEN_B])); + let a_requested = payload_params(Some("pattern:aabb"), 5, false); + let b_requested = payload_params(Some("pattern:112233"), 5, false); + let (a_token, a) = open_negotiated(&mut core, peer(), &a_requested, None); + let (b_token, b) = open_negotiated(&mut core, other_peer(), &b_requested, None); + + let mut payload_of = |endpoint, token, sequence, params: &Params| { + let packet = core + .handle_datagram(endpoint, &echo_request(token, sequence, params, &[], None)) + .unwrap() + .expect("an admissible echo must be answered"); + expect_echo_reply(&packet, params, None).payload + }; + + assert_eq!( + payload_of(peer(), a_token, 0, &a), + vec![0xaa, 0xbb, 0xaa, 0xbb, 0xaa] + ); + assert_eq!( + payload_of(other_peer(), b_token, 0, &b), + vec![0x11, 0x22, 0x33, 0x11, 0x22] + ); + assert_eq!( + payload_of(peer(), a_token, 1, &a), + vec![0xaa, 0xbb, 0xaa, 0xbb, 0xaa], + "session A is unaffected by session B's reply" + ); +} + +#[test] +fn a_random_session_fills_the_whole_region() { + // Deterministic assertions only: the region is the negotiated one and the + // reply decodes and authenticates. What the operating system's random + // source returns is not a property this suite can assert without flipping a + // coin, and `fill`'s unit tests cover propagation and the failure fallback. + for hmac_key in [None, Some(KEY)] { + // `echo_payloads` already asserts the datagram length and the region + // length of each reply, and decoding verifies the MAC. Nothing is + // asserted about the byte values, deliberately: a random payload + // containing any particular byte is chance, not behavior. + assert_eq!(echo_payloads(Some("rand"), 7, hmac_key, 2).len(), 2); + } +} + +#[test] +fn authentication_consumes_payload_space_at_the_same_negotiated_length() { + // One negotiated length, two field blocks: authentication's 16 bytes come + // out of the payload region, not out of the datagram. Every remaining + // payload byte is still filled, and the authenticated reply's MAC covers + // those bytes — `expect_echo_reply` verifies it while decoding. + let unauthenticated = payload_params(Some("pattern:aa"), 48, false); + let length = unauthenticated.length; + let authenticated = Params { + length, + ..payload_params(Some("pattern:aa"), 0, true) + }; + + let plain = { + let mut core = core_for(None, ScriptedTokens::new([TOKEN_A])); + let (token, negotiated) = open_negotiated(&mut core, peer(), &unauthenticated, None); + let packet = core + .handle_datagram(peer(), &echo_request(token, 0, &negotiated, &[], None)) + .unwrap() + .expect("an admissible echo must be answered"); + assert_eq!(packet.bytes().len() as i64, length); + expect_echo_reply(&packet, &negotiated, None).payload + }; + let signed = { + let mut core = core_for(Some(KEY), ScriptedTokens::new([TOKEN_A])); + let (token, negotiated) = open_negotiated(&mut core, peer(), &authenticated, Some(KEY)); + let packet = core + .handle_datagram(peer(), &echo_request(token, 0, &negotiated, &[], Some(KEY))) + .unwrap() + .expect("an admissible echo must be answered"); + assert_eq!(packet.bytes().len() as i64, length); + expect_echo_reply(&packet, &negotiated, Some(KEY)).payload + }; + + assert_eq!( + signed.len() + irtt_proto::HMAC_SIZE, + plain.len(), + "authentication takes its 16 bytes from the payload region" + ); + assert!(plain.iter().chain(&signed).all(|byte| *byte == 0xaa)); +} diff --git a/crates/irtt-server/src/tests/lifecycle.rs b/crates/irtt-server/src/tests/lifecycle.rs index 0d54449..068515b 100644 --- a/crates/irtt-server/src/tests/lifecycle.rs +++ b/crates/irtt-server/src/tests/lifecycle.rs @@ -15,7 +15,7 @@ use std::time::Duration; -use irtt_proto::{Clock, Params, ReceivedStats, StampAt, FLAG_CLOSE}; +use irtt_proto::{echo_header_len, Clock, Params, ReceivedStats, ServerFill, StampAt, FLAG_CLOSE}; use super::support::{ close_request, core_with_sources, echo_at, echo_params, echo_request, @@ -416,9 +416,20 @@ fn the_maximum_duration_deadline_starts_at_the_first_served_echo() { #[test] fn the_first_echo_past_the_maximum_duration_carries_close_and_ends_the_session() { + // The session negotiates a fill and a payload region, so this covers the + // close's payload as well: a server close is an ordinary echo reply with a + // flag added, and it fills exactly as the session's other replies do. + let requested = Params { + server_fill: Some(ServerFill { + value: "pattern:aabb".to_owned(), + }), + length: (echo_header_len(false, ¶ms_at(SECOND)) + 5) as i64, + ..params_at(SECOND) + }; + let patterned = vec![0xaa, 0xbb, 0xaa, 0xbb, 0xaa]; let (mut core, clock, token, negotiated) = manual_core( max_duration_of(Duration::from_secs(1)), - ¶ms_at(SECOND), + &requested, ScriptedTokens::new([TOKEN_A]), None, ); @@ -437,6 +448,7 @@ fn the_first_echo_past_the_maximum_duration_carries_close_and_ends_the_session() .expect("short of the deadline must be answered normally"); let reply = expect_echo_reply(&packet, &negotiated, None); assert_eq!(reply.flags & FLAG_CLOSE, 0, "not yet"); + assert_eq!(reply.payload, patterned, "an ordinary reply's fill"); // At the deadline exactly. let packet = echo_at(&mut core, &clock, 13 * SECOND, token, 2, &negotiated, None) @@ -449,7 +461,10 @@ fn the_first_echo_past_the_maximum_duration_carries_close_and_ends_the_session() (Some(3), Some(0x7)), "the triggering request is included in the statistics it reports" ); - assert!(reply.payload.is_empty()); + assert_eq!( + reply.payload, patterned, + "the close keeps the session's fill; it is not a special packet" + ); assert_eq!( packet.bytes().len(), irtt_proto::echo_packet_len(false, &negotiated).unwrap(), diff --git a/crates/irtt-server/tests/async_client.rs b/crates/irtt-server/tests/async_client.rs index 4d411a3..661ebb2 100644 --- a/crates/irtt-server/tests/async_client.rs +++ b/crates/irtt-server/tests/async_client.rs @@ -57,6 +57,11 @@ async fn exercise_client_server() { assert_eq!(negotiated.params.received_stats, ReceivedStats::Both); assert_eq!(negotiated.params.stamp_at, StampAt::Both); assert_eq!(negotiated.params.clock, Clock::Both); + // A default client requests no fill, and this open succeeded under + // the default strict negotiation policy — which it could not have + // if the server had answered an absent request with its own default + // descriptor. The server still fills the payload with it. + assert_eq!(negotiated.params.server_fill, None); assert!(matches!(event, ClientEvent::SessionStarted { .. })); } OpenOutcome::NoTestCompleted { .. } => panic!("normal client unexpectedly ran no-test"), diff --git a/crates/irtt-server/tests/server_fill.rs b/crates/irtt-server/tests/server_fill.rs new file mode 100644 index 0000000..7c54ec9 --- /dev/null +++ b/crates/irtt-server/tests/server_fill.rs @@ -0,0 +1,216 @@ +//! A negotiated fill over a real socket. +//! +//! The core tests are authoritative for fill bytes; this exists to prove the +//! whole path — negotiation, session, echo reply, UDP — puts the requested +//! pattern on the wire. It drives the server with the production protocol codecs +//! over a plain Tokio socket rather than through `AsyncClient`, because a +//! client's reply payload is not part of its event API and exposing it merely to +//! be asserted here would be the wrong trade. + +use std::time::Duration; + +use irtt_client::{ + AsyncClient, ClientConfig, NegotiationPolicy, NegotiationRestriction, OpenOutcome, SocketConfig, +}; +use irtt_proto::{ + decode_echo_reply, decode_open_reply, echo_header_len, encode_request, Clock, Params, + ReceivedStats, RequestToEncode, ServerFill, StampAt, +}; +use irtt_server::{Server, ServerConfig}; +use tokio::{net::UdpSocket, sync::oneshot, time::timeout}; + +const DEADBEEF: &[u8] = &[0xde, 0xad, 0xbe, 0xef]; + +#[tokio::test(flavor = "current_thread")] +async fn a_negotiated_pattern_reaches_the_wire() { + timeout(Duration::from_secs(2), exercise_patterned_session()) + .await + .expect("the fill integration exceeded its bounded runtime"); +} + +async fn exercise_patterned_session() { + let mut server = Server::bind("127.0.0.1:0".parse().unwrap(), ServerConfig::default()) + .await + .unwrap(); + let server_addr = server.local_addr().unwrap(); + let (shutdown_tx, shutdown_rx) = oneshot::channel(); + let server_task = tokio::spawn(async move { + server + .run(async { + let _ = shutdown_rx.await; + }) + .await + }); + + let socket = UdpSocket::bind("127.0.0.1:0").await.unwrap(); + socket.connect(server_addr).await.unwrap(); + + let requested = requested_params(24); + socket + .send( + &encode_request( + RequestToEncode::Open { + params: &requested, + no_test: false, + }, + None, + ) + .unwrap(), + ) + .await + .unwrap(); + let reply = decode_open_reply(&receive(&socket).await, None).unwrap(); + let negotiated = reply.params; + assert_eq!( + negotiated.server_fill, requested.server_fill, + "a valid descriptor is returned unchanged, so strict negotiation holds" + ); + + // The request carries its own payload bytes, none of which may come back. + socket + .send( + &encode_request( + RequestToEncode::Echo { + token: reply.token, + sequence: 0, + params: &negotiated, + payload: &[0xa5; 24], + }, + None, + ) + .unwrap(), + ) + .await + .unwrap(); + let packet = receive(&socket).await; + let echo = decode_echo_reply(&packet, &negotiated, None).unwrap(); + + assert_eq!(packet.len() as i64, negotiated.length); + assert_eq!( + echo.payload, + DEADBEEF + .iter() + .copied() + .cycle() + .take(24) + .collect::>() + ); + + socket + .send(&encode_request(RequestToEncode::Close { token: reply.token }, None).unwrap()) + .await + .unwrap(); + + shutdown_tx.send(()).unwrap(); + server_task.await.unwrap().unwrap(); +} + +#[tokio::test(flavor = "current_thread")] +async fn a_client_sees_the_fallback_as_a_negotiation_restriction() { + timeout(Duration::from_secs(2), exercise_fallback_negotiation()) + .await + .expect("the negotiation integration exceeded its bounded runtime"); +} + +/// The fallback is deliberately visible to a client rather than hidden: a +/// descriptor this server could not honor comes back as the default one, which a +/// strict client refuses and a loose client accepts with a restriction recorded. +async fn exercise_fallback_negotiation() { + let mut server = Server::bind("127.0.0.1:0".parse().unwrap(), ServerConfig::default()) + .await + .unwrap(); + let server_addr = server.local_addr().unwrap(); + let (shutdown_tx, shutdown_rx) = oneshot::channel(); + let server_task = tokio::spawn(async move { + server + .run(async { + let _ = shutdown_rx.await; + }) + .await + }); + + // `ClientConfig` accepts any non-empty descriptor within the wire bound, so + // an unknown one genuinely reaches the server. + let config = |policy| ClientConfig { + server_addr: server_addr.to_string(), + duration: Some(Duration::from_secs(1)), + interval: Duration::from_millis(100), + length: 64, + server_fill: Some("bogus".to_owned()), + 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() + }; + + let mut strict = AsyncClient::connect(config(NegotiationPolicy::Strict)) + .await + .unwrap(); + let rejected = strict + .open() + .await + .expect_err("a strict client must refuse the changed fill"); + assert!( + rejected.to_string().contains("fill"), + "the restriction names the fill: {rejected}" + ); + + let mut loose = AsyncClient::connect(config(NegotiationPolicy::Loose)) + .await + .unwrap(); + match loose.open().await.unwrap() { + OpenOutcome::Started { negotiated, .. } => { + assert_eq!( + negotiated + .params + .server_fill + .map(|fill| fill.value) + .as_deref(), + Some("pattern:69727474"), + "the server reports the default descriptor it fell back to" + ); + assert!(negotiated + .restrictions + .contains(&NegotiationRestriction::ServerFillChanged)); + } + OpenOutcome::NoTestCompleted { .. } => panic!("normal client unexpectedly ran no-test"), + } + loose.close().await.unwrap(); + + shutdown_tx.send(()).unwrap(); + server_task.await.unwrap().unwrap(); +} + +/// Params requesting a `deadbeef` fill and exactly `payload_len` payload bytes. +fn requested_params(payload_len: usize) -> Params { + let mut params = Params { + protocol_version: 1, + duration_ns: 1_000_000_000, + interval_ns: 100_000_000, + length: 0, + received_stats: ReceivedStats::Both, + stamp_at: StampAt::Both, + clock: Clock::Both, + dscp: 0, + server_fill: Some(ServerFill { + value: "pattern:deadbeef".to_owned(), + }), + }; + params.length = (echo_header_len(false, ¶ms) + payload_len) as i64; + params +} + +/// One datagram, with a bounded wait so a lost reply fails rather than hangs. +async fn receive(socket: &UdpSocket) -> Vec { + let mut buffer = vec![0; 2048]; + let read = timeout(Duration::from_secs(1), socket.recv(&mut buffer)) + .await + .expect("the server must answer within the test's bound") + .unwrap(); + buffer.truncate(read); + buffer +}