refactor(proto): unify request codecs

Replace the three per-kind request encoders and decoders with one
directional pair, ahead of introducing irtt-server.

Sender side: encode_request(RequestToEncode, hmac_key) constructs OPEN,
CLOSE and ECHO requests from borrowed values. Params and the ECHO payload
are borrowed rather than owned, removing a Params clone and a payload Vec
from the client's hot paths. Wire output is byte-identical; all captured
vectors still pass unchanged.

Receiver side: decode_request(packet) structurally classifies an inbound
request with no HMAC key, no negotiated Params, no session state and no
allocation. A server must extract a token before it knows which key
applies, so authentication is a separate step. Classification follows the
verified server spec: reserved bits and FLAG_REPLY are rejected, OPEN
takes precedence over CLOSE and over an echo-shaped body, and only the
structural minimums (4/12/16, plus 16 with FLAG_HMAC) are enforced.
Trailing bytes are tolerated, no negotiated length ceiling is applied, and
a zero token is structurally valid. OPEN parameters stay encoded bytes so
an authenticating receiver can verify before decoding untrusted data, and
everything after an ECHO sequence number is exposed as an opaque tail.

Add verify_packet_hmac(key, packet), which locates the standard field
itself and rejects a packet lacking FLAG_HMAC. hmac_present reports packet
layout, never authentication. The offset-based verify_hmac stays public as
the low-level primitive for hand-built and captured wire vectors, matching
the existing compute_hmac pair.

Refactor the private envelope into structural header decoding, codec flag
rules, HMAC presence policy and verification as separate steps. The reply
codecs compose the same steps in the same order, so decode_open_reply and
decode_echo_reply keep their exact error ordering and the client's
fatal-versus-ignore classification is unchanged. Reply decoding stays
state-specific: the client already knows which reply to expect, and the
two need different semantic context.

Migrate the production client and every fake peer to the new API; the
fake peers now admit requests the way a server does, instead of parsing
packets by hand.

crates/irtt-client/src/async_client/tests.rs

@@ -9,9 +9,8 @@ use std::{
 };
 
 use irtt_proto::{
-    decode_close_request, decode_echo_request, decode_open_request, encode_echo_reply,
+    decode_request, encode_echo_reply, encode_open_reply, flags, verify_packet_hmac,
-    encode_open_reply, flags, EchoReply, OpenReply, Params, ReceivedStats, StampAt,
+    DecodedRequestKind, EchoReply, OpenReply, Params, ReceivedStats, StampAt, TimestampFields,
-    TimestampFields,
 };
 use tokio::runtime::{Builder, Runtime};
 
@@ -102,6 +101,47 @@ fn recv_packet(socket: &UdpSocket, tx: &mpsc::Sender<Vec<u8>>) -> (Vec<u8>, Sock
     (packet, peer)
 }
 
+/// Admits a request the way a compliant server does: structural decode first,
+/// then HMAC presence policy, then authentication. Returns `None` for anything
+/// a server would silently discard.
+fn decode_server_request<'a>(
+    packet: &'a [u8],
+    key: Option<&[u8]>,
+) -> Option<DecodedRequestKind<'a>> {
+    let request = decode_request(packet).ok()?;
+    if request.hmac_present != key.is_some() {
+        return None;
+    }
+    if let Some(key) = key {
+        verify_packet_hmac(key, packet).ok()?;
+    }
+    Some(request.kind)
+}
+
+/// Returns the requested parameters and whether this was a no-test open.
+fn open_request(packet: &[u8], key: Option<&[u8]>) -> (Params, bool) {
+    match decode_server_request(packet, key) {
+        Some(DecodedRequestKind::Open { params, no_test }) => {
+            (Params::decode(params).unwrap(), no_test)
+        }
+        other => panic!("expected an authenticated open request, got {other:?}"),
+    }
+}
+
+fn echo_request_sequence(packet: &[u8], key: Option<&[u8]>) -> u32 {
+    match decode_server_request(packet, key) {
+        Some(DecodedRequestKind::Echo { sequence, .. }) => sequence,
+        other => panic!("expected an authenticated echo request, got {other:?}"),
+    }
+}
+
+fn close_request_token(packet: &[u8], key: Option<&[u8]>) -> u64 {
+    match decode_server_request(packet, key) {
+        Some(DecodedRequestKind::Close { token }) => token,
+        other => panic!("expected an authenticated close request, got {other:?}"),
+    }
+}
+
 fn send_open_reply(
     socket: &UdpSocket,
     peer: SocketAddr,
@@ -128,16 +168,16 @@ fn open_session(
     key: Option<&[u8]>,
 ) -> (Params, SocketAddr) {
     let (packet, peer) = recv_packet(socket, tx);
-    let request = decode_open_request(&packet, key).unwrap();
+    let (params, _) = open_request(&packet, key);
     send_open_reply(
         socket,
         peer,
-        request.params.clone(),
+        params.clone(),
         key,
         flags::FLAG_OPEN | flags::FLAG_REPLY,
         TOKEN,
     );
-    (request.params, peer)
+    (params, peer)
 }
 
 fn echo_reply(
@@ -175,12 +215,12 @@ fn start_open_server(key: Option<Vec<u8>>, packets_after_open: usize) -> TestSer
 fn start_no_test_server() -> TestServer {
     start_server(move |socket, tx| {
         let (open_packet, peer) = recv_packet(&socket, &tx);
-        let request = decode_open_request(&open_packet, None).unwrap();
+        let (params, no_test) = open_request(&open_packet, None);
-        assert!(request.close);
+        assert!(no_test);
         send_open_reply(
             &socket,
             peer,
-            request.params,
+            params,
             None,
             flags::FLAG_OPEN | flags::FLAG_REPLY | flags::FLAG_CLOSE,
             0,
@@ -192,12 +232,12 @@ fn start_peer_close_server() -> TestServer {
     start_server(move |socket, tx| {
         let (params, peer) = open_session(&socket, &tx, None);
         let (probe_packet, _) = recv_packet(&socket, &tx);
-        let probe = decode_echo_request(&probe_packet, &params, None).unwrap();
+        let sequence = echo_request_sequence(&probe_packet, None);
         socket
             .send_to(
                 &echo_reply(
                     &params,
-                    probe.sequence,
+                    sequence,
                     TOKEN,
                     flags::FLAG_REPLY | flags::FLAG_CLOSE,
                     None,
@@ -213,27 +253,16 @@ fn start_echo_close_server(key: Option<Vec<u8>>) -> TestServer {
         let (params, peer) = open_session(&socket, &tx, key.as_deref());
 
         let (probe_packet, _) = recv_packet(&socket, &tx);
-        let probe = decode_echo_request(&probe_packet, &params, key.as_deref()).unwrap();
+        let sequence = echo_request_sequence(&probe_packet, key.as_deref());
         socket
             .send_to(
-                &echo_reply(
+                &echo_reply(&params, sequence, TOKEN, flags::FLAG_REPLY, key.as_deref()),
-                    &params,
-                    probe.sequence,
-                    TOKEN,
-                    flags::FLAG_REPLY,
-                    key.as_deref(),
-                ),
                 peer,
             )
             .unwrap();
 
         let (close_packet, _) = recv_packet(&socket, &tx);
-        assert_eq!(
+        assert_eq!(close_request_token(&close_packet, key.as_deref()), TOKEN);
-            decode_close_request(&close_packet, key.as_deref())
-                .unwrap()
-                .token,
-            TOKEN
-        );
     })
 }
 
@@ -275,13 +304,13 @@ fn open_ignores_malformed_and_bad_hmac_without_resending() {
     let server_key = key.clone();
     let server = start_server(move |socket, tx| {
         let (open_packet, peer) = recv_packet(&socket, &tx);
-        let request = decode_open_request(&open_packet, Some(&server_key)).unwrap();
+        let (params, _) = open_request(&open_packet, Some(&server_key));
         socket.send_to(&[0_u8], peer).unwrap();
         let mut bad_hmac = encode_open_reply(
             &OpenReply {
                 flags: flags::FLAG_OPEN | flags::FLAG_REPLY,
                 token: TOKEN,
-                params: request.params.clone(),
+                params: params.clone(),
             },
             Some(&server_key),
         )
@@ -291,7 +320,7 @@ fn open_ignores_malformed_and_bad_hmac_without_resending() {
         send_open_reply(
             &socket,
             peer,
-            request.params,
+            params,
             Some(&server_key),
             flags::FLAG_OPEN | flags::FLAG_REPLY,
             TOKEN,
@@ -342,14 +371,14 @@ fn opening_poll_budget_yields_then_public_open_completes() {
     let (queued, queue_ready) = mpsc::channel();
     let server = start_server(move |socket, tx| {
         let (open_packet, peer) = recv_packet(&socket, &tx);
-        let request = decode_open_request(&open_packet, None).unwrap();
+        let (params, _) = open_request(&open_packet, None);
         for _ in 0..=OPEN_POLL_WORK_BUDGET {
             socket.send_to(&[0_u8], peer).unwrap();
         }
         send_open_reply(
             &socket,
             peer,
-            request.params,
+            params,
             None,
             flags::FLAG_OPEN | flags::FLAG_REPLY,
             TOKEN,
@@ -391,7 +420,7 @@ fn opening_poll_budget_yields_then_public_open_completes() {
 fn ignored_open_traffic_cannot_extend_the_absolute_attempt_deadline() {
     let server = start_server(move |socket, tx| {
         let (first_packet, peer) = recv_packet(&socket, &tx);
-        let request = decode_open_request(&first_packet, None).unwrap();
+        let (params, _) = open_request(&first_packet, None);
         socket
             .set_read_timeout(Some(Duration::from_millis(10)))
             .unwrap();
@@ -411,7 +440,7 @@ fn ignored_open_traffic_cannot_extend_the_absolute_attempt_deadline() {
                     send_open_reply(
                         &socket,
                         second_peer,
-                        request.params,
+                        params,
                         None,
                         flags::FLAG_OPEN | flags::FLAG_REPLY,
                         TOKEN,
@@ -445,11 +474,11 @@ fn ignored_open_traffic_cannot_extend_the_absolute_attempt_deadline() {
 fn authenticated_rejection_is_terminal_without_retry() {
     let server = start_server(move |socket, tx| {
         let (open_packet, peer) = recv_packet(&socket, &tx);
-        let request = decode_open_request(&open_packet, None).unwrap();
+        let (params, _) = open_request(&open_packet, None);
         send_open_reply(
             &socket,
             peer,
-            request.params,
+            params,
             None,
             flags::FLAG_OPEN | flags::FLAG_REPLY | flags::FLAG_CLOSE,
             0,
@@ -502,12 +531,12 @@ fn opening_cancellation_before_submission_is_transactional() {
 fn opening_cancellation_after_submission_can_retry() {
     let server = start_server(move |socket, tx| {
         let (first, _) = recv_packet(&socket, &tx);
-        let first = decode_open_request(&first, None).unwrap();
+        let (first_params, _) = open_request(&first, None);
         let (_, peer) = recv_packet(&socket, &tx);
         send_open_reply(
             &socket,
             peer,
-            first.params,
+            first_params,
             None,
             flags::FLAG_OPEN | flags::FLAG_REPLY,
             TOKEN,
@@ -541,7 +570,7 @@ fn adapter_failure_sends_preencoded_cleanup_without_committing_open() {
     let server = start_server(move |socket, tx| {
         open_session(&socket, &tx, None);
         let (close, _) = recv_packet(&socket, &tx);
-        assert_eq!(decode_close_request(&close, None).unwrap().token, TOKEN);
+        assert_eq!(close_request_token(&close, None), TOKEN);
     });
 
     runtime().block_on(async {
@@ -837,10 +866,10 @@ fn receive_retries_false_readiness_and_shares_packet_classification() {
     let server = start_server(move |socket, tx| {
         let (params, peer) = open_session(&socket, &tx, None);
         let (probe_packet, _) = recv_packet(&socket, &tx);
-        let probe = decode_echo_request(&probe_packet, &params, None).unwrap();
+        let sequence = echo_request_sequence(&probe_packet, None);
         socket
             .send_to(
-                &echo_reply(&params, probe.sequence, TOKEN, flags::FLAG_REPLY, None),
+                &echo_reply(&params, sequence, TOKEN, flags::FLAG_REPLY, None),
                 peer,
             )
             .unwrap();
@@ -891,10 +920,7 @@ fn recv_after_local_close_and_no_test_fails_on_first_poll() {
     let close_server = start_server(move |socket, tx| {
         open_session(&socket, &tx, None);
         let (close_packet, _) = recv_packet(&socket, &tx);
-        assert_eq!(
+        assert_eq!(close_request_token(&close_packet, None), TOKEN);
-            decode_close_request(&close_packet, None).unwrap().token,
-            TOKEN
-        );
     });
     runtime().block_on(async {
         let mut client = opened_client(close_server.addr, 0).await;

crates/irtt-client/src/client/tests/hmac.rs

@@ -8,7 +8,7 @@ fn hmac_open_success() {
     let params = params_from_config(&config).unwrap();
     let server = start_fake_server(move |socket, tx| {
         let (request, peer) = recv_request(&socket, &tx);
-        verify_hmac(&key, &request, HMAC_OFFSET).unwrap();
+        verify_packet_hmac(&key, &request).unwrap();
         let reply = open_reply(FLAG_OPEN | FLAG_REPLY, TOKEN, &params, Some(&key));
         socket.send_to(&reply, peer).unwrap();
     });
@@ -85,7 +85,7 @@ fn post_token_hmac_negotiation_failure_sends_authenticated_cleanup_close() {
     let packets: Vec<_> = server.rx.iter().take(2).collect();
     let cleanup = &packets[1];
     assert_eq!(cleanup[3], flags::FLAG_CLOSE | FLAG_HMAC);
-    verify_hmac(&key, cleanup, HMAC_OFFSET).unwrap();
+    verify_packet_hmac(&key, cleanup).unwrap();
     assert_eq!(
         u64::from_le_bytes(cleanup[4 + HMAC_SIZE..12 + HMAC_SIZE].try_into().unwrap()),
         TOKEN
@@ -113,7 +113,7 @@ fn hmac_close_packet_includes_valid_hmac() {
     let packets: Vec<_> = server.rx.iter().take(2).collect();
     let close = &packets[1];
     assert_eq!(close[3], flags::FLAG_CLOSE | FLAG_HMAC);
-    verify_hmac(&key, close, HMAC_OFFSET).unwrap();
+    verify_packet_hmac(&key, close).unwrap();
     assert_eq!(
         u64::from_le_bytes(close[4 + HMAC_SIZE..12 + HMAC_SIZE].try_into().unwrap()),
         TOKEN
@@ -191,7 +191,7 @@ fn hmac_echo_request_reply_works() {
         let mut buf = [0_u8; 2048];
         if let Ok((size, _)) = socket.recv_from(&mut buf) {
             tx.send(buf[..size].to_vec()).unwrap();
-            verify_hmac(&server_key, &buf[..size], HMAC_OFFSET).unwrap();
+            verify_packet_hmac(&server_key, &buf[..size]).unwrap();
             let seq = u32::from_le_bytes(
                 buf[4 + HMAC_SIZE + 8..4 + HMAC_SIZE + 12]
                     .try_into()

crates/irtt-client/src/client/tests/mod.rs

@@ -1,8 +1,8 @@
 use super::*;
 use crate::config::{NegotiationPolicy, DEFAULT_OPEN_TIMEOUTS};
 use irtt_proto::{
-    flags::FLAG_HMAC, flags::FLAG_OPEN, flags::FLAG_REPLY, verify_hmac, Clock, ReceivedStats,
+    flags::FLAG_HMAC, flags::FLAG_OPEN, flags::FLAG_REPLY, verify_packet_hmac, Clock,
-    StampAt, HMAC_SIZE, MAGIC,
+    ReceivedStats, StampAt, HMAC_SIZE, MAGIC,
 };
 use std::{thread, time::SystemTime};
 

crates/irtt-client/src/managed/blocking_tests.rs

@@ -6,8 +6,8 @@ use std::{
 };
 
 use irtt_proto::{
-    decode_close_request, decode_echo_request, decode_open_request, encode_echo_reply,
+    decode_request, encode_echo_reply, encode_open_reply, flags, DecodedRequestKind, EchoReply,
-    encode_open_reply, flags, EchoReply, OpenReply, ReceivedStats, StampAt, TimestampFields,
+    OpenReply, Params, ReceivedStats, StampAt, TimestampFields,
 };
 use tokio::{runtime::Builder, time::timeout};
 
@@ -69,8 +69,10 @@ impl EchoServer {
         let worker = thread::spawn(move || {
             let mut buffer = [0_u8; 8192];
             let (open_len, peer) = socket.recv_from(&mut buffer).unwrap();
-            let request = decode_open_request(&buffer[..open_len], None).unwrap();
+            let negotiated = match decode_request(&buffer[..open_len]).unwrap().kind {
-            let negotiated = request.params;
+                DecodedRequestKind::Open { params, .. } => Params::decode(params).unwrap(),
+                other => panic!("expected an open request, got {other:?}"),
+            };
             let reply = encode_open_reply(
                 &OpenReply {
                     flags: flags::FLAG_OPEN | flags::FLAG_REPLY,
@@ -84,14 +86,17 @@ impl EchoServer {
 
             loop {
                 let (len, packet_peer) = socket.recv_from(&mut buffer).unwrap();
-                let packet = &buffer[..len];
+                let Ok(request) = decode_request(&buffer[..len]) else {
-                if let Ok(probe) = decode_echo_request(packet, &negotiated, None) {
+                    continue;
+                };
+                match request.kind {
+                    DecodedRequestKind::Echo { sequence, .. } => {
                         worker_probe.set();
                         let reply = encode_echo_reply(
                             &EchoReply {
                                 flags: flags::FLAG_REPLY,
                                 token: TOKEN,
-                            sequence: probe.sequence,
+                                sequence,
                                 recv_count: None,
                                 recv_window: None,
                                 timestamps: TimestampFields::default(),
@@ -102,10 +107,13 @@ impl EchoServer {
                         )
                         .unwrap();
                         socket.send_to(&reply, packet_peer).unwrap();
-                } else if decode_close_request(packet, None).is_ok() {
+                    }
+                    DecodedRequestKind::Close { .. } => {
                         worker_close.set();
                         return;
                     }
+                    DecodedRequestKind::Open { .. } => {}
+                }
             }
         });
         Self {

crates/irtt-client/src/managed/tests.rs

@@ -10,8 +10,8 @@ use std::{
 };
 
 use irtt_proto::{
-    decode_close_request, decode_echo_request, decode_open_request, encode_echo_reply,
+    decode_request, encode_echo_reply, encode_open_reply, flags, verify_packet_hmac,
-    encode_open_reply, flags, EchoReply, OpenReply, ReceivedStats, StampAt, TimestampFields,
+    DecodedRequestKind, EchoReply, OpenReply, Params, ReceivedStats, StampAt, TimestampFields,
 };
 use tokio::runtime::{Builder, Runtime};
 
@@ -86,6 +86,31 @@ impl PacketGate {
         self.release_ready.notify_all();
     }
 }
+
+/// Admits a request the way a compliant server does: structural decode first,
+/// then HMAC presence policy, then authentication. Returns `None` for anything
+/// a server would silently discard.
+fn decode_server_request<'a>(
+    packet: &'a [u8],
+    key: Option<&[u8]>,
+) -> Option<DecodedRequestKind<'a>> {
+    let request = decode_request(packet).ok()?;
+    if request.hmac_present != key.is_some() {
+        return None;
+    }
+    if let Some(key) = key {
+        verify_packet_hmac(key, packet).ok()?;
+    }
+    Some(request.kind)
+}
+
+fn decode_open_params(packet: &[u8], key: Option<&[u8]>) -> Params {
+    match decode_server_request(packet, key) {
+        Some(DecodedRequestKind::Open { params, .. }) => Params::decode(params).unwrap(),
+        other => panic!("expected an authenticated open request, got {other:?}"),
+    }
+}
+
 fn start_server(behavior: ServerBehavior, key: Option<Vec<u8>>) -> TestServer {
     start_server_negotiating(behavior, key, None)
 }
@@ -124,11 +149,10 @@ fn start_server_with_gates(
             kind: PacketKind::Open,
             at: Instant::now(),
         });
-        let request = decode_open_request(&buffer[..open_len], key.as_deref()).unwrap();
+        let mut negotiated = decode_open_params(&buffer[..open_len], key.as_deref());
         if let Some(gate) = &open_gate {
             gate.arrive_and_wait();
         }
-        let mut negotiated = request.params.clone();
         if let Some(interval) = interval {
             negotiated.interval_ns = i64::try_from(interval.as_nanos()).unwrap();
         }
@@ -164,7 +188,8 @@ fn start_server_with_gates(
                 return;
             };
             let packet = &buffer[..len];
-            if let Ok(probe) = decode_echo_request(packet, &negotiated, key.as_deref()) {
+            match decode_server_request(packet, key.as_deref()) {
+                Some(DecodedRequestKind::Echo { sequence, .. }) => {
                     thread_records.lock().unwrap().push(PacketRecord {
                         kind: PacketKind::Probe,
                         at: Instant::now(),
@@ -186,11 +211,7 @@ fn start_server_with_gates(
                                 &EchoReply {
                                     flags: flags::FLAG_REPLY,
                                     token: TOKEN,
-                                sequence: if reply_index < 2 {
+                                    sequence: if reply_index < 2 { sequence } else { u32::MAX },
-                                    probe.sequence
-                                } else {
-                                    u32::MAX
-                                },
                                     recv_count: None,
                                     recv_window: None,
                                     timestamps: TimestampFields::default(),
@@ -210,9 +231,10 @@ fn start_server_with_gates(
                     let peer_close = matches!(behavior, ServerBehavior::PeerClose);
                     let reply = encode_echo_reply(
                         &EchoReply {
-                        flags: flags::FLAG_REPLY | if peer_close { flags::FLAG_CLOSE } else { 0 },
+                            flags: flags::FLAG_REPLY
+                                | if peer_close { flags::FLAG_CLOSE } else { 0 },
                             token: TOKEN,
-                        sequence: probe.sequence,
+                            sequence,
                             recv_count: None,
                             recv_window: None,
                             timestamps: TimestampFields::default(),
@@ -229,13 +251,16 @@ fn start_server_with_gates(
                     if peer_close {
                         return;
                     }
-            } else if decode_close_request(packet, key.as_deref()).is_ok() {
+                }
+                Some(DecodedRequestKind::Close { .. }) => {
                     thread_records.lock().unwrap().push(PacketRecord {
                         kind: PacketKind::Close,
                         at: Instant::now(),
                     });
                     return;
                 }
+                Some(DecodedRequestKind::Open { .. }) | None => {}
+            }
         }
     });
     TestServer {

crates/irtt-client/src/session/machine.rs

@@ -1,9 +1,8 @@
 use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
 
 use irtt_proto::{
-    close::CloseRequest, decode_echo_reply, echo_packet_len, encode_close_request,
+    decode_echo_reply, echo_packet_len, encode_request, flags, EchoReply, OpenReply, Params,
-    encode_echo_request, encode_open_request, flags, EchoReply, EchoRequest, OpenReply,
+    RequestToEncode, ServerFill, TimestampFields, PROTOCOL_VERSION,
-    OpenRequest, Params, ServerFill, TimestampFields, PROTOCOL_VERSION,
 };
 
 use crate::{
@@ -211,11 +210,13 @@ impl SessionMachine {
 
     pub(crate) fn prepare_open_request(&self) -> Result<PreparedOpenRequest, ClientError> {
         self.ensure_connected()?;
-        let request = OpenRequest {
+        let bytes = encode_request(
-            params: self.requested.clone(),
+            RequestToEncode::Open {
-            close: self.config.run_mode == RunMode::NoTest,
+                params: &self.requested,
-        };
+                no_test: self.config.run_mode == RunMode::NoTest,
-        let bytes = encode_open_request(&request, self.config.hmac_key.as_deref())?;
+            },
+            self.config.hmac_key.as_deref(),
+        )?;
         Ok(PreparedOpenRequest {
             bytes: bytes.into_boxed_slice(),
         })
@@ -252,8 +253,8 @@ impl SessionMachine {
 
         let reply_is_close = flags::has(reply.flags, flags::FLAG_CLOSE);
         let cleanup_close = if !reply_is_close && reply.token != 0 {
-            let bytes = encode_close_request(
+            let bytes = encode_request(
-                &CloseRequest { token: reply.token },
+                RequestToEncode::Close { token: reply.token },
                 self.config.hmac_key.as_deref(),
             )
             .map_err(ClientError::from)
@@ -310,14 +311,13 @@ impl SessionMachine {
 
     pub(crate) fn prepare_probe(&self) -> Result<Option<PreparedProbe>, ClientError> {
         let session = self.open_session()?;
-        let request = EchoRequest {
+        let bytes = encode_request(
+            RequestToEncode::Echo {
                 token: session.token,
                 sequence: session.next_wire_seq,
-            payload: vec![],
+                params: &session.negotiated.params,
-        };
+                payload: &[],
-        let bytes = encode_echo_request(
+            },
-            &request,
-            &session.negotiated.params,
             self.config.hmac_key.as_deref(),
         )?;
         Ok(Some(PreparedProbe {
@@ -1077,8 +1077,8 @@ mod tests {
         machine.state = MachineState::Open(Box::new(ActiveSession {
             token: 0x0102_0304_0506_0708,
             negotiated,
-            local_close_packet: encode_close_request(
+            local_close_packet: encode_request(
-                &CloseRequest {
+                RequestToEncode::Close {
                     token: 0x0102_0304_0506_0708,
                 },
                 None,
@@ -1452,8 +1452,8 @@ mod tests {
             let prepared = machine.prepare_close().unwrap();
             assert_eq!(
                 prepared.bytes,
-                encode_close_request(
+                encode_request(
-                    &CloseRequest {
+                    RequestToEncode::Close {
                         token: 0x0102_0304_0506_0708,
                     },
                     None,

crates/irtt-client/tests/hmac.rs

@@ -123,7 +123,14 @@ fn hmac_close_success_sends_authenticated_close_and_closes_session() {
 
 #[test]
 fn hmac_required_server_rejects_missing_or_wrong_client_key() {
-    for (hmac_key, expected_hmac_flag) in [(None, false), (Some(b"wrong-secret".to_vec()), true)] {
+    // Both rejections leave the client timing out, but they are different
+    // failures on the wire: a client with no key presents no HMAC field at
+    // all, while a client with the wrong key presents one that does not
+    // authenticate.
+    for (hmac_key, expected_hmac_flag, expected_bad_hmac) in [
+        (None, false, false),
+        (Some(b"wrong-secret".to_vec()), true, true),
+    ] {
         let server_key = b"compat-secret".to_vec();
         let server = start_hmac_required_open_drop_server(server_key, Duration::from_millis(250));
         let mut config = config_for_params(server.addr, &default_params());
@@ -134,13 +141,13 @@ fn hmac_required_server_rejects_missing_or_wrong_client_key() {
         assert!(matches!(client.open(), Err(ClientError::OpenTimeout)));
 
         let observations = server.observations(1);
-        assert!(matches!(
+        assert_eq!(
             observations.as_slice(),
             [ServerObservation::RejectedHmac {
-                hmac,
+                hmac: expected_hmac_flag,
-                bad_hmac: true
+                bad_hmac: expected_bad_hmac,
-            }] if *hmac == expected_hmac_flag
+            }]
-        ));
+        );
         server.join();
     }
 }

crates/irtt-client/tests/support/mod.rs

@@ -12,9 +12,10 @@ use std::{
 
 use irtt_client::{Client, ClientConfig, ClientEvent, NegotiatedParams, OpenOutcome, SocketConfig};
 use irtt_proto::{
-    compute_hmac_in_place, echo_packet_len, encode_echo_reply, encode_open_reply, flags,
+    compute_hmac_in_place, decode_request, echo_packet_len, encode_echo_reply, encode_open_reply,
-    verify_hmac, Clock, EchoReply, OpenReply, PacketLayout, Params, ProtoError, ReceivedStats,
+    flags, verify_packet_hmac, Clock, DecodedRequest, DecodedRequestKind, EchoReply, OpenReply,
-    ServerFill, StampAt, TimestampFields, HMAC_SIZE, MAGIC, PROTOCOL_VERSION,
+    PacketLayout, Params, ProtoError, ReceivedStats, ServerFill, StampAt, TimestampFields,
+    HMAC_SIZE, MAGIC, PROTOCOL_VERSION,
 };
 
 pub use real_irtt::RealIrtServer;
@@ -252,7 +253,7 @@ where
 pub fn start_open_server(params: Params, hmac_key: Option<Vec<u8>>) -> FakeServer {
     start_fake_server(move |socket, tx| {
         let (request, peer) = recv_request(&socket);
-        let (open_params, hmac) = decode_open_request_params(&request, hmac_key.as_deref());
+        let (open_params, hmac) = open_request_params(&request, hmac_key.as_deref());
         tx.send(ServerObservation::Open {
             params: open_params,
             hmac,
@@ -273,10 +274,11 @@ pub fn start_hmac_required_open_drop_server(key: Vec<u8>, wait: Duration) -> Fak
     start_fake_server(move |socket, tx| {
         socket.set_read_timeout(Some(wait)).unwrap();
         while let Some(request) = recv_request_timeout(&socket) {
-            let hmac = request
+            let hmac = decode_request(&request).is_ok_and(|request| request.hmac_present);
-                .get(3)
+            // A server with a key drops any request that fails authentication,
-                .is_some_and(|flags| flags & flags::FLAG_HMAC != 0);
+            // whether it carried no HMAC field at all or a field signed with
-            match verify_hmac(&key, &request, HMAC_OFFSET) {
+            // the wrong key.
+            match verify_packet_hmac(&key, &request) {
                 Ok(()) => {}
                 Err(error) => {
                     tx.send(ServerObservation::RejectedHmac {
@@ -293,7 +295,7 @@ pub fn start_hmac_required_open_drop_server(key: Vec<u8>, wait: Duration) -> Fak
 pub fn start_hmac_close_server(params: Params, key: Vec<u8>) -> FakeServer {
     start_fake_server(move |socket, tx| {
         let (request, peer) = recv_request(&socket);
-        let (open_params, hmac) = decode_open_request_params(&request, Some(&key));
+        let (open_params, hmac) = open_request_params(&request, Some(&key));
         tx.send(ServerObservation::Open {
             params: open_params,
             hmac,
@@ -323,7 +325,7 @@ pub fn start_hmac_close_server(params: Params, key: Vec<u8>) -> FakeServer {
 pub fn start_bad_hmac_echo_reply_server(params: Params, key: Vec<u8>) -> FakeServer {
     start_fake_server(move |socket, tx| {
         let (request, peer) = recv_request(&socket);
-        let (open_params, hmac) = decode_open_request_params(&request, Some(&key));
+        let (open_params, hmac) = open_request_params(&request, Some(&key));
         tx.send(ServerObservation::Open {
             params: open_params,
             hmac,
@@ -368,7 +370,7 @@ pub fn start_bad_hmac_open_reply_server(
 ) -> FakeServer {
     start_fake_server(move |socket, tx| {
         let (request, peer) = recv_request(&socket);
-        let (open_params, hmac) = decode_open_request_params(&request, Some(&request_key));
+        let (open_params, hmac) = open_request_params(&request, Some(&request_key));
         tx.send(ServerObservation::Open {
             params: open_params,
             hmac,
@@ -398,7 +400,7 @@ fn start_one_probe_server(
 ) -> FakeServer {
     start_fake_server(move |socket, tx| {
         let (request, peer) = recv_request(&socket);
-        let (open_params, hmac) = decode_open_request_params(&request, hmac_key.as_deref());
+        let (open_params, hmac) = open_request_params(&request, hmac_key.as_deref());
         tx.send(ServerObservation::Open {
             params: open_params,
             hmac,
@@ -463,52 +465,48 @@ fn assert_started(outcome: OpenOutcome) -> NegotiatedParams {
     }
 }
 
-fn decode_open_request_params(packet: &[u8], hmac_key: Option<&[u8]>) -> (Params, bool) {
+/// Admits a request the way a compliant server does: structural decode first,
-    assert_eq!(&packet[..3], &MAGIC);
+/// then HMAC presence policy, then authentication.
-    assert!(packet[3] & flags::FLAG_OPEN != 0);
+fn admit_request<'a>(packet: &'a [u8], hmac_key: Option<&[u8]>) -> DecodedRequest<'a> {
-    let hmac = packet[3] & flags::FLAG_HMAC != 0;
+    let request = decode_request(packet).expect("fake peers only receive well-formed requests");
-    assert_eq!(hmac, hmac_key.is_some());
+    assert_eq!(request.hmac_present, hmac_key.is_some());
     if let Some(key) = hmac_key {
-        verify_hmac(key, packet, HMAC_OFFSET).unwrap();
+        verify_packet_hmac(key, packet).unwrap();
+    }
+    request
+}
+
+fn open_request_params(packet: &[u8], hmac_key: Option<&[u8]>) -> (Params, bool) {
+    let request = admit_request(packet, hmac_key);
+    match request.kind {
+        DecodedRequestKind::Open { params, .. } => {
+            (Params::decode(params).unwrap(), request.hmac_present)
+        }
+        other => panic!("expected an open request, got {other:?}"),
     }
-    let params_offset = 4 + if hmac { HMAC_SIZE } else { 0 };
-    (Params::decode(&packet[params_offset..]).unwrap(), hmac)
 }
 
 fn observe_echo_request(packet: &[u8], hmac_key: Option<&[u8]>) -> ServerObservation {
-    assert_eq!(&packet[..3], &MAGIC);
+    let request = admit_request(packet, hmac_key);
-    assert_eq!(packet[3] & flags::FLAG_OPEN, 0);
+    match request.kind {
-    assert_eq!(packet[3] & flags::FLAG_REPLY, 0);
+        DecodedRequestKind::Echo {
-    assert_eq!(packet[3] & flags::FLAG_CLOSE, 0);
+            token, sequence, ..
-    let hmac = packet[3] & flags::FLAG_HMAC != 0;
+        } => ServerObservation::Echo {
-    assert_eq!(hmac, hmac_key.is_some());
-    if let Some(key) = hmac_key {
-        verify_hmac(key, packet, HMAC_OFFSET).unwrap();
-    }
-
-    let mut pos = 4 + if hmac { HMAC_SIZE } else { 0 };
-    let token = u64::from_le_bytes(packet[pos..pos + 8].try_into().unwrap());
-    pos += 8;
-    let sequence = u32::from_le_bytes(packet[pos..pos + 4].try_into().unwrap());
-
-    ServerObservation::Echo {
             len: packet.len(),
-        hmac,
+            hmac: request.hmac_present,
             token,
             sequence,
+        },
+        other => panic!("expected an echo request, got {other:?}"),
     }
 }
 
 fn observe_close_request(packet: &[u8], hmac_key: &[u8]) -> ServerObservation {
-    assert_eq!(&packet[..3], &MAGIC);
+    let request = admit_request(packet, Some(hmac_key));
-    assert_eq!(packet[3] & flags::FLAG_CLOSE, flags::FLAG_CLOSE);
+    match request.kind {
-    assert_eq!(packet[3] & flags::FLAG_HMAC, flags::FLAG_HMAC);
+        DecodedRequestKind::Close { token } => ServerObservation::Close { hmac: true, token },
-    verify_hmac(hmac_key, packet, HMAC_OFFSET).unwrap();
+        other => panic!("expected a close request, got {other:?}"),
-
+    }
-    let token_offset = 4 + HMAC_SIZE;
-    let token = u64::from_le_bytes(packet[token_offset..token_offset + 8].try_into().unwrap());
-
-    ServerObservation::Close { hmac: true, token }
 }
 
 // irtt-proto has no encode_close_reply/CloseReply API yet, so this stays a

crates/irtt-proto/src/close.rs

@@ -1,79 +0,0 @@
-use crate::{
-    envelope::{self, FlagRule},
-    flags::{has, FLAG_CLOSE, FLAG_OPEN, FLAG_REPLY},
-    layout::PacketLayout,
-    ProtoError, Result, TOKEN_SIZE,
-};
-
-#[derive(Debug, Clone, PartialEq, Eq)]
-pub struct CloseRequest {
-    pub token: u64,
-}
-
-pub fn encode_close_request(request: &CloseRequest, hmac_key: Option<&[u8]>) -> Result<Vec<u8>> {
-    let mut out = envelope::begin(
-        FLAG_CLOSE,
-        hmac_key,
-        &[
-            FlagRule::Require(FLAG_CLOSE),
-            FlagRule::Reject(FLAG_OPEN),
-            FlagRule::Reject(FLAG_REPLY),
-        ],
-        PacketLayout::close_request(hmac_key.is_some()).header_len(),
-    )?;
-    out.extend_from_slice(&request.token.to_le_bytes());
-    envelope::finish(out, hmac_key)
-}
-
-pub fn decode_close_request(packet: &[u8], hmac_key: Option<&[u8]>) -> Result<CloseRequest> {
-    let envelope = envelope::decode(
-        packet,
-        hmac_key,
-        &[
-            FlagRule::Require(FLAG_CLOSE),
-            FlagRule::Reject(FLAG_OPEN),
-            FlagRule::Reject(FLAG_REPLY),
-        ],
-    )?;
-    let expected_len =
-        PacketLayout::close_request(has(envelope.flags, crate::FLAG_HMAC)).header_len();
-    if packet.len() != expected_len {
-        return Err(ProtoError::PacketLengthMismatch {
-            expected: expected_len,
-            actual: packet.len(),
-        });
-    }
-    envelope::verify(packet, hmac_key)?;
-
-    Ok(CloseRequest {
-        token: u64::from_le_bytes(
-            packet[envelope.body_offset..envelope.body_offset + TOKEN_SIZE]
-                .try_into()
-                .unwrap(),
-        ),
-    })
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use crate::{hmac, FLAG_HMAC, HMAC_SIZE};
-
-    #[test]
-    fn hmac_close_request_places_token_after_hmac() {
-        let packet = encode_close_request(
-            &CloseRequest {
-                token: 0x7896_b6ab_8771_5213,
-            },
-            Some(b"testkey"),
-        )
-        .unwrap();
-
-        assert_eq!(packet[3], FLAG_CLOSE | FLAG_HMAC);
-        assert_eq!(
-            &packet[4 + HMAC_SIZE..],
-            &0x7896_b6ab_8771_5213u64.to_le_bytes()
-        );
-        hmac::verify_hmac(b"testkey", &packet, hmac::hmac_offset()).unwrap();
-    }
-}

crates/irtt-proto/src/echo.rs

@@ -1,9 +1,9 @@
 use crate::{
     envelope::{self, FlagRule},
-    flags::{FLAG_CLOSE, FLAG_OPEN, FLAG_REPLY},
+    flags::{FLAG_OPEN, FLAG_REPLY},
     layout::{echo_packet_len, PacketLayout},
     params::{Clock, Params, StampAt},
-    ProtoError, Result, RECV_COUNT_SIZE, RECV_WINDOW_SIZE, SEQ_SIZE, TIMESTAMP_SIZE, TOKEN_SIZE,
+    ProtoError, Result, SEQ_SIZE, TIMESTAMP_SIZE, TOKEN_SIZE,
 };
 
 /// Upstream irtt 0.9.1 always emits both midpoint timestamp fields (wall,
@@ -34,13 +34,6 @@ fn midpoint_compat_packet_len(layout: PacketLayout, normal_len: usize) -> usize
     (layout.header_len() + MIDPOINT_COMPAT_EXTRA).max(normal_len)
 }
 
-#[derive(Debug, Clone, PartialEq, Eq)]
-pub struct EchoRequest {
-    pub token: u64,
-    pub sequence: u32,
-    pub payload: Vec<u8>,
-}
-
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct EchoReply {
     pub flags: u8,
@@ -62,69 +55,6 @@ pub struct TimestampFields {
     pub send_mono: Option<i64>,
 }
 
-pub fn encode_echo_request(
-    request: &EchoRequest,
-    params: &Params,
-    hmac_key: Option<&[u8]>,
-) -> Result<Vec<u8>> {
-    let layout = PacketLayout::echo(hmac_key.is_some(), params);
-    let len = echo_packet_len(hmac_key.is_some(), params)?;
-    let payload_offset = layout.header_len();
-    let available_payload_len = len.saturating_sub(payload_offset);
-    if request.payload.len() > available_payload_len {
-        return Err(ProtoError::PayloadTooLarge {
-            available: available_payload_len,
-            provided: request.payload.len(),
-        });
-    }
-    let mut out = envelope::begin(
-        0,
-        hmac_key,
-        &[
-            FlagRule::Reject(FLAG_OPEN),
-            FlagRule::Reject(FLAG_REPLY),
-            FlagRule::Reject(FLAG_CLOSE),
-        ],
-        len,
-    )?;
-    out.extend_from_slice(&request.token.to_le_bytes());
-    out.extend_from_slice(&request.sequence.to_le_bytes());
-    push_zeroed_layout_tail(layout, &mut out);
-    out.resize(len, 0);
-    out[payload_offset..payload_offset + request.payload.len()].copy_from_slice(&request.payload);
-
-    envelope::finish(out, hmac_key)
-}
-
-pub fn decode_echo_request(
-    packet: &[u8],
-    params: &Params,
-    hmac_key: Option<&[u8]>,
-) -> Result<EchoRequest> {
-    let envelope = envelope::decode(
-        packet,
-        hmac_key,
-        &[
-            FlagRule::Reject(FLAG_OPEN),
-            FlagRule::Reject(FLAG_REPLY),
-            FlagRule::Reject(FLAG_CLOSE),
-        ],
-    )?;
-    let layout = PacketLayout::echo(hmac_key.is_some(), params);
-    validate_echo_length(packet, params, layout, false)?;
-    envelope::verify(packet, hmac_key)?;
-
-    let mut pos = envelope.body_offset;
-    let token = read_u64(packet, &mut pos);
-    let sequence = read_u32(packet, &mut pos);
-
-    Ok(EchoRequest {
-        token,
-        sequence,
-        payload: packet[layout.header_len()..].to_vec(),
-    })
-}
-
 pub fn encode_echo_reply(
     reply: &EchoReply,
     params: &Params,
@@ -141,7 +71,7 @@ pub fn encode_echo_reply(
         });
     }
 
-    let mut out = envelope::begin(
+    let mut out = envelope::begin_checked(
         reply.flags,
         hmac_key,
         &[FlagRule::Reject(FLAG_OPEN), FlagRule::Require(FLAG_REPLY)],
@@ -183,7 +113,7 @@ pub fn decode_echo_reply(
         &[FlagRule::Reject(FLAG_OPEN), FlagRule::Require(FLAG_REPLY)],
     )?;
     let layout = PacketLayout::echo(hmac_key.is_some(), params);
-    let midpoint_compat = validate_echo_length(packet, params, layout, true)?;
+    let midpoint_compat = validate_echo_length(packet, params, layout)?;
     envelope::verify(packet, hmac_key)?;
 
     let header_len = layout.header_len()
@@ -238,18 +168,12 @@ pub fn decode_echo_reply(
 
 /// Validates the packet length against the negotiated layout. Returns
 /// `Ok(true)` when the packet matched the upstream midpoint compatibility
-/// length rather than the exact negotiated length (only possible when
+/// length rather than the exact negotiated length.
-/// `allow_midpoint_compat` is set).
 ///
 /// The negotiated length is always checked first, so when the compatibility
 /// form is not longer than the negotiated form the two are indistinguishable
 /// by length and normal negotiated parsing wins. See [`decode_echo_reply`].
-fn validate_echo_length(
+fn validate_echo_length(packet: &[u8], params: &Params, layout: PacketLayout) -> Result<bool> {
-    packet: &[u8],
-    params: &Params,
-    layout: PacketLayout,
-    allow_midpoint_compat: bool,
-) -> Result<bool> {
     let header_len = layout.header_len();
     if packet.len() < header_len {
         return Err(ProtoError::PacketTooShort {
@@ -261,7 +185,7 @@ fn validate_echo_length(
     if packet.len() == expected_len {
         return Ok(false);
     }
-    if allow_midpoint_compat && is_midpoint_single_clock(params) {
+    if is_midpoint_single_clock(params) {
         let compat_len = midpoint_compat_packet_len(layout, expected_len);
         if compat_len > expected_len && packet.len() == compat_len {
             return Ok(true);
@@ -273,18 +197,6 @@ fn validate_echo_length(
     })
 }
 
-fn push_zeroed_layout_tail(layout: PacketLayout, out: &mut Vec<u8>) {
-    if layout.recv_count {
-        out.extend_from_slice(&[0; RECV_COUNT_SIZE]);
-    }
-    if layout.recv_window {
-        out.extend_from_slice(&[0; RECV_WINDOW_SIZE]);
-    }
-    for _ in 0..layout.timestamp_count() {
-        out.extend_from_slice(&[0; TIMESTAMP_SIZE]);
-    }
-}
-
 fn push_echo_reply_tail(reply: &EchoReply, layout: PacketLayout, out: &mut Vec<u8>) -> Result<()> {
     if let Some(value) = field_value(layout.recv_count, reply.recv_count, "recv_count")? {
         out.extend_from_slice(&value.to_le_bytes());
@@ -350,7 +262,7 @@ mod tests {
     use crate::{
         hmac,
         params::{Clock, ReceivedStats, StampAt},
-        write_header, FLAG_HMAC, HMAC_SIZE,
+        write_header, FLAG_HMAC, HMAC_SIZE, RECV_COUNT_SIZE, RECV_WINDOW_SIZE,
     };
 
     fn default_params() -> Params {
@@ -370,108 +282,18 @@ mod tests {
         }
     }
 
-    fn echo_request_with_payload(payload: Vec<u8>) -> EchoRequest {
+    /// Zero-fills every negotiated reply field, matching what a peer that
-        EchoRequest {
+    /// reports no optional values would put on the wire.
-            token: 0x7896_b6ab_8771_5213,
+    fn push_zeroed_layout_tail(layout: PacketLayout, out: &mut Vec<u8>) {
-            sequence: 9,
+        if layout.recv_count {
-            payload,
+            out.extend_from_slice(&[0; RECV_COUNT_SIZE]);
         }
+        if layout.recv_window {
+            out.extend_from_slice(&[0; RECV_WINDOW_SIZE]);
         }
-
+        for _ in 0..layout.timestamp_count() {
-    #[test]
+            out.extend_from_slice(&[0; TIMESTAMP_SIZE]);
-    fn echo_request_encodes_default_placeholders() {
-        let params = default_params();
-        let packet = encode_echo_request(
-            &EchoRequest {
-                token: 0x7896_b6ab_8771_5213,
-                sequence: 0,
-                payload: Vec::new(),
-            },
-            &params,
-            None,
-        )
-        .unwrap();
-        assert_eq!(packet.len(), 60);
-        assert_eq!(&packet[..4], &[0x14, 0xa7, 0x5b, 0x00]);
-        assert_eq!(&packet[4..12], &0x7896_b6ab_8771_5213u64.to_le_bytes());
-        assert!(packet[12..].iter().all(|byte| *byte == 0));
         }
-
-    #[test]
-    fn echo_request_encodes_exact_fit_payload() {
-        let params = params_with_payload_space(4);
-        let request = echo_request_with_payload(vec![1, 2, 3, 4]);
-        let packet = encode_echo_request(&request, &params, None).unwrap();
-        let payload_offset = PacketLayout::echo(false, &params).header_len();
-
-        assert_eq!(&packet[payload_offset..], &[1, 2, 3, 4]);
-    }
-
-    #[test]
-    fn echo_request_encodes_shorter_payload_and_zero_fills_remainder() {
-        let params = params_with_payload_space(4);
-        let request = echo_request_with_payload(vec![1, 2]);
-        let packet = encode_echo_request(&request, &params, None).unwrap();
-        let payload_offset = PacketLayout::echo(false, &params).header_len();
-
-        assert_eq!(&packet[payload_offset..], &[1, 2, 0, 0]);
-    }
-
-    #[test]
-    fn echo_request_rejects_oversized_payload() {
-        let params = params_with_payload_space(4);
-        let request = echo_request_with_payload(vec![1, 2, 3, 4, 5]);
-        let original = request.clone();
-
-        assert_eq!(
-            encode_echo_request(&request, &params, None),
-            Err(ProtoError::PayloadTooLarge {
-                available: 4,
-                provided: 5,
-            })
-        );
-        assert_eq!(request, original);
-    }
-
-    #[test]
-    fn echo_request_rejects_negative_requested_length() {
-        let request = echo_request_with_payload(Vec::new());
-        let params = Params {
-            length: -1,
-            ..Params::default()
-        };
-
-        assert_eq!(
-            encode_echo_request(&request, &params, None),
-            Err(ProtoError::NegativePacketLength { length: -1 })
-        );
-    }
-
-    #[test]
-    fn hmac_echo_request_places_token_and_sequence_after_hmac() {
-        let params = default_params();
-        let packet = encode_echo_request(
-            &EchoRequest {
-                token: 0x7896_b6ab_8771_5213,
-                sequence: 9,
-                payload: Vec::new(),
-            },
-            &params,
-            Some(b"testkey"),
-        )
-        .unwrap();
-
-        assert_eq!(packet.len(), 76);
-        assert_eq!(&packet[..4], &[0x14, 0xa7, 0x5b, FLAG_HMAC]);
-        assert_eq!(
-            &packet[4 + HMAC_SIZE..4 + HMAC_SIZE + TOKEN_SIZE],
-            &0x7896_b6ab_8771_5213u64.to_le_bytes()
-        );
-        assert_eq!(
-            &packet[4 + HMAC_SIZE + TOKEN_SIZE..4 + HMAC_SIZE + TOKEN_SIZE + SEQ_SIZE],
-            &9_u32.to_le_bytes()
-        );
-        hmac::verify_hmac(b"testkey", &packet, hmac::hmac_offset()).unwrap();
     }
 
     #[test]

crates/irtt-proto/src/envelope.rs

@@ -1,3 +1,17 @@
+//! Shared structural envelope handling.
+//!
+//! Layering, from packet bytes upwards:
+//!
+//! 1. [`decode_structural`] — magic, reserved flag bits, and the HMAC-dependent
+//!    body offset. No key, no packet-type semantics.
+//! 2. [`require_flags`] — codec-specific semantic flag rules.
+//! 3. [`check_hmac_presence`] — HMAC presence *policy* derived from a key.
+//! 4. [`verify`] — cryptographic verification.
+//!
+//! [`decode`] composes 1–3 for the reply codecs, which know the packet type
+//! they expect and hold the applicable key. Request decoding uses only step 1,
+//! because a server must extract a token before it knows which key applies.
+
 use crate::{
     flags::{self, has, FLAG_HMAC},
     hmac, validate_header, write_header, ProtoError, Result, HEADER_SIZE, HMAC_SIZE,
@@ -9,28 +23,52 @@ pub(crate) enum FlagRule {
     Reject(u8),
 }
 
+/// Structural result of parsing the fixed 4-byte protocol header.
+///
+/// `hmac_present` reports only that `FLAG_HMAC` was set, and therefore that the
+/// packet is laid out with an authentication field before its body. It carries
+/// no claim about the contents of that field.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub(crate) struct Envelope {
     pub(crate) flags: u8,
+    pub(crate) hmac_present: bool,
     pub(crate) body_offset: usize,
 }
 
+/// The single source of truth for protocol header structure: minimum length,
+/// magic, reserved flag bits, and the HMAC-dependent body offset.
+pub(crate) fn decode_structural(packet: &[u8]) -> Result<Envelope> {
+    let flags = validate_header(packet)?;
+    let hmac_present = has(flags, FLAG_HMAC);
+    Ok(Envelope {
+        flags,
+        hmac_present,
+        body_offset: HEADER_SIZE + if hmac_present { HMAC_SIZE } else { 0 },
+    })
+}
+
+/// Structural decode plus the packet-type and HMAC-presence checks a codec that
+/// already knows both the expected packet type and the applicable key performs.
 pub(crate) fn decode(
     packet: &[u8],
     hmac_key: Option<&[u8]>,
     rules: &[FlagRule],
 ) -> Result<Envelope> {
-    let flags = validate_header(packet)?;
+    let envelope = decode_structural(packet)?;
-    validate_packet_type(flags, rules)?;
+    require_flags(envelope.flags, rules)?;
-    check_hmac_presence(flags, hmac_key)?;
+    check_hmac_presence(envelope.flags, hmac_key)?;
-
+    Ok(envelope)
-    Ok(Envelope {
-        flags,
-        body_offset: HEADER_SIZE + if has(flags, FLAG_HMAC) { HMAC_SIZE } else { 0 },
-    })
 }
 
-pub(crate) fn begin(
+/// Begins a packet whose flags the encoder derived itself, so no packet-type
+/// rule can fail. The key is authoritative for `FLAG_HMAC`.
+pub(crate) fn begin(flags: u8, hmac_key: Option<&[u8]>, capacity: usize) -> Result<Vec<u8>> {
+    begin_checked(flags, hmac_key, &[], capacity)
+}
+
+/// Begins a packet from caller-supplied flags, which must satisfy the codec's
+/// packet-type rules.
+pub(crate) fn begin_checked(
     flags: u8,
     hmac_key: Option<&[u8]>,
     rules: &[FlagRule],
@@ -40,7 +78,7 @@ pub(crate) fn begin(
     // while unauthenticated encoders clear any caller-supplied FLAG_HMAC.
     let flags = with_hmac_flag(flags, hmac_key.is_some());
     flags::validate_flags(flags)?;
-    validate_packet_type(flags, rules)?;
+    require_flags(flags, rules)?;
 
     let minimum_capacity = HEADER_SIZE + if hmac_key.is_some() { HMAC_SIZE } else { 0 };
     let mut out = Vec::with_capacity(capacity.max(minimum_capacity));
@@ -65,7 +103,7 @@ pub(crate) fn finish(mut packet: Vec<u8>, hmac_key: Option<&[u8]>) -> Result<Vec
     Ok(packet)
 }
 
-fn validate_packet_type(flags: u8, rules: &[FlagRule]) -> Result<()> {
+pub(crate) fn require_flags(flags: u8, rules: &[FlagRule]) -> Result<()> {
     for rule in rules {
         match *rule {
             FlagRule::Require(flag) if !has(flags, flag) => {
@@ -80,7 +118,10 @@ fn validate_packet_type(flags: u8, rules: &[FlagRule]) -> Result<()> {
     Ok(())
 }
 
-fn check_hmac_presence(flags: u8, hmac_key: Option<&[u8]>) -> Result<()> {
+/// Checks structural HMAC presence against the caller's expectation. This is
+/// policy, not authentication: it only compares `FLAG_HMAC` with whether a key
+/// was supplied.
+pub(crate) fn check_hmac_presence(flags: u8, hmac_key: Option<&[u8]>) -> Result<()> {
     if has(flags, FLAG_HMAC) == hmac_key.is_some() {
         Ok(())
     } else {
@@ -152,9 +193,23 @@ mod tests {
         );
     }
 
+    #[test]
+    fn structural_decode_reports_hmac_presence_without_a_key() {
+        let mut authenticated = header(FLAG_OPEN | FLAG_HMAC);
+        authenticated.extend_from_slice(&[0xff; HMAC_SIZE]);
+
+        let envelope = decode_structural(&authenticated).unwrap();
+        assert!(envelope.hmac_present);
+        assert_eq!(envelope.body_offset, HEADER_SIZE + HMAC_SIZE);
+
+        let envelope = decode_structural(&header(FLAG_OPEN)).unwrap();
+        assert!(!envelope.hmac_present);
+        assert_eq!(envelope.body_offset, HEADER_SIZE);
+    }
+
     #[test]
     fn verify_rejects_bad_hmac() {
-        let mut packet = begin(FLAG_OPEN, Some(KEY), OPEN_REQUEST_RULES, 22).unwrap();
+        let mut packet = begin_checked(FLAG_OPEN, Some(KEY), OPEN_REQUEST_RULES, 22).unwrap();
         packet.extend_from_slice(&[1, 2]);
         let mut packet = finish(packet, Some(KEY)).unwrap();
         *packet.last_mut().unwrap() ^= 0x80;
@@ -165,10 +220,11 @@ mod tests {
 
     #[test]
     fn encoder_key_authoritatively_normalizes_hmac_flag() {
-        let plain = begin(FLAG_OPEN | FLAG_HMAC, None, OPEN_REQUEST_RULES, HEADER_SIZE).unwrap();
+        let plain =
+            begin_checked(FLAG_OPEN | FLAG_HMAC, None, OPEN_REQUEST_RULES, HEADER_SIZE).unwrap();
         assert_eq!(plain[3], FLAG_OPEN);
 
-        let authenticated = begin(
+        let authenticated = begin_checked(
             FLAG_OPEN,
             Some(KEY),
             OPEN_REQUEST_RULES,

crates/irtt-proto/src/hmac.rs

@@ -2,7 +2,7 @@ use hmac::{Hmac, KeyInit, Mac};
 use md5::Md5;
 use subtle::ConstantTimeEq;
 
-use crate::{ProtoError, Result, HEADER_SIZE, HMAC_SIZE};
+use crate::{envelope, flags::FLAG_HMAC, ProtoError, Result, HEADER_SIZE, HMAC_SIZE};
 
 // HMAC-MD5 is used for compatibility with the IRTT wire protocol, not as a
 // new cryptographic recommendation. Verification uses HMAC with constant-time
@@ -34,6 +34,12 @@ pub fn compute_hmac_in_place(key: &[u8], packet: &mut [u8], hmac_offset: usize)
     Ok(())
 }
 
+/// Verifies an HMAC field at an explicit offset.
+///
+/// This is the low-level primitive, kept for hand-built and captured wire
+/// vectors that name their own field position. Code handling real packets
+/// should use [`verify_packet_hmac`], which locates the field itself and
+/// rejects a packet that does not carry `FLAG_HMAC`.
 pub fn verify_hmac(key: &[u8], packet: &[u8], hmac_offset: usize) -> Result<()> {
     if packet.len().saturating_sub(hmac_offset) < HMAC_SIZE {
         return Err(ProtoError::InvalidHmacOffset);
@@ -47,6 +53,29 @@ pub fn verify_hmac(key: &[u8], packet: &[u8], hmac_offset: usize) -> Result<()>
     }
 }
 
+/// Verifies the authentication field of a whole IRTT packet.
+///
+/// The field is the standard 16 bytes immediately after the 4-byte header, so
+/// callers never compute an offset themselves. This is cryptographic
+/// verification only: it says nothing about which key *should* apply to the
+/// packet, nor about session state. Choosing the key is the caller's policy.
+///
+/// # Errors
+///
+/// Returns [`ProtoError::PacketTooShort`], [`ProtoError::BadMagic`] or
+/// [`ProtoError::ReservedFlags`] for a structurally invalid header,
+/// [`ProtoError::MissingFlag`] when the packet does not carry `FLAG_HMAC`,
+/// [`ProtoError::InvalidHmacOffset`] when the field is truncated, and
+/// [`ProtoError::BadHmac`] when the MAC does not match. The packet is never
+/// modified.
+pub fn verify_packet_hmac(key: &[u8], packet: &[u8]) -> Result<()> {
+    let envelope = envelope::decode_structural(packet)?;
+    if !envelope.hmac_present {
+        return Err(ProtoError::MissingFlag(FLAG_HMAC));
+    }
+    verify_hmac(key, packet, hmac_offset())
+}
+
 pub(crate) fn hmac_offset() -> usize {
     HEADER_SIZE
 }

crates/irtt-proto/src/lib.rs

@@ -12,15 +12,28 @@
 //! performs no additional validation, so callers that build `Params` manually
 //! are responsible for validating those values before sending them.
 //!
-//! The crate also provides packet layout calculation, open/echo/close packet
+//! The crate also provides packet layout calculation, packet encoding and
-//! encoding and decoding, and optional HMAC placement, computation, and
+//! decoding, and optional HMAC placement, computation, and verification
-//! verification helpers.
+//! helpers.
 //! Encoder `hmac_key` arguments are authoritative: `Some(key)` adds
 //! `FLAG_HMAC`, while `None` removes it from caller-supplied reply flags.
 //!
+//! # Directional model
+//!
+//! Requests use one encoder and one decoder for all three request kinds:
+//! [`encode_request`] builds a request from borrowed sender-side values, and
+//! [`decode_request`] structurally classifies an inbound request without a key,
+//! negotiated [`Params`], or any session state. [`verify_packet_hmac`]
+//! authenticates a packet separately, once the applicable key is known —
+//! `FLAG_HMAC` being present is a statement about layout, not about validity.
+//!
+//! Replies keep type-specific decoders ([`decode_open_reply`],
+//! [`decode_echo_reply`]) because a client already knows from its session state
+//! which reply kind to expect, and the two need genuinely different semantic
+//! context.
+//!
 #![forbid(unsafe_code)]
 
-pub mod close;
 pub mod echo;
 mod envelope;
 pub mod error;
@@ -29,22 +42,19 @@ pub mod hmac;
 pub mod layout;
 pub mod open;
 pub mod params;
+pub mod request;
 pub mod varint;
 
-pub use close::{decode_close_request, encode_close_request, CloseRequest};
+pub use echo::{decode_echo_reply, encode_echo_reply, EchoReply, TimestampFields};
-pub use echo::{
-    decode_echo_reply, decode_echo_request, encode_echo_reply, encode_echo_request, EchoReply,
-    EchoRequest, TimestampFields,
-};
 pub use error::{ProtoError, Result};
 pub use flags::*;
-pub use hmac::{compute_hmac, compute_hmac_in_place, verify_hmac};
+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::{
+pub use open::{decode_open_reply, encode_open_reply, OpenReply};
-    decode_open_reply, decode_open_request, encode_open_reply, encode_open_request, OpenReply,
-    OpenRequest,
-};
 pub use params::{Clock, Params, ReceivedStats, ServerFill, StampAt, MAX_SERVER_FILL_BYTES};
+pub use request::{
+    decode_request, encode_request, DecodedRequest, DecodedRequestKind, RequestToEncode,
+};
 
 pub const MAGIC: [u8; 3] = [0x14, 0xA7, 0x5B];
 pub const PROTOCOL_VERSION: i64 = 1;

crates/irtt-proto/src/open.rs

@@ -6,12 +6,6 @@ use crate::{
     ProtoError, Result, TOKEN_SIZE,
 };
 
-#[derive(Debug, Clone, PartialEq, Eq)]
-pub struct OpenRequest {
-    pub params: Params,
-    pub close: bool,
-}
-
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct OpenReply {
     pub flags: u8,
@@ -19,39 +13,9 @@ pub struct OpenReply {
     pub params: Params,
 }
 
-pub fn encode_open_request(request: &OpenRequest, hmac_key: Option<&[u8]>) -> Result<Vec<u8>> {
-    let mut flags = FLAG_OPEN;
-    if request.close {
-        flags |= FLAG_CLOSE;
-    }
-    let params = request.params.encode();
-    let mut out = envelope::begin(
-        flags,
-        hmac_key,
-        &[FlagRule::Require(FLAG_OPEN), FlagRule::Reject(FLAG_REPLY)],
-        PacketLayout::open_request(hmac_key.is_some()).header_len() + params.len(),
-    )?;
-    out.extend_from_slice(&params);
-    envelope::finish(out, hmac_key)
-}
-
-pub fn decode_open_request(packet: &[u8], hmac_key: Option<&[u8]>) -> Result<OpenRequest> {
-    let envelope = envelope::decode(
-        packet,
-        hmac_key,
-        &[FlagRule::Require(FLAG_OPEN), FlagRule::Reject(FLAG_REPLY)],
-    )?;
-    envelope::verify(packet, hmac_key)?;
-
-    Ok(OpenRequest {
-        params: Params::decode(&packet[envelope.body_offset..])?,
-        close: has(envelope.flags, FLAG_CLOSE),
-    })
-}
-
 pub fn encode_open_reply(reply: &OpenReply, hmac_key: Option<&[u8]>) -> Result<Vec<u8>> {
     let params = reply.params.encode();
-    let mut out = envelope::begin(
+    let mut out = envelope::begin_checked(
         reply.flags,
         hmac_key,
         &[FlagRule::Require(FLAG_OPEN), FlagRule::Require(FLAG_REPLY)],
@@ -116,42 +80,6 @@ mod tests {
         }
     }
 
-    #[test]
-    fn open_request_has_no_token() {
-        let packet = encode_open_request(
-            &OpenRequest {
-                params: default_params(),
-                close: false,
-            },
-            None,
-        )
-        .unwrap();
-
-        assert_eq!(packet.len(), 24);
-        assert_eq!(&packet[..4], &[0x14, 0xa7, 0x5b, 0x01]);
-        assert_eq!(&packet[4..], &default_params().encode());
-    }
-
-    #[test]
-    fn hmac_open_request_inserts_hmac_before_params() {
-        let packet = encode_open_request(
-            &OpenRequest {
-                params: default_params(),
-                close: false,
-            },
-            Some(b"testkey"),
-        )
-        .unwrap();
-
-        assert_eq!(
-            packet.len(),
-            4 + HMAC_SIZE + default_params().encode().len()
-        );
-        assert_eq!(&packet[..4], &[0x14, 0xa7, 0x5b, FLAG_OPEN | FLAG_HMAC]);
-        assert_eq!(&packet[4 + HMAC_SIZE..], &default_params().encode());
-        hmac::verify_hmac(b"testkey", &packet, hmac::hmac_offset()).unwrap();
-    }
-
     #[test]
     fn open_reply_decodes_token() {
         let mut packet = vec![0x14, 0xa7, 0x5b, 0x03];

crates/irtt-proto/src/request.rs

@@ -0,0 +1,245 @@
+//! Directional request codecs: one sender-side encoder and one receiver-side
+//! structural decoder.
+//!
+//! The two directions are deliberately asymmetric. Encoding an ECHO request
+//! needs the negotiated [`Params`] because the sender constructs a
+//! negotiated-length datagram; decoding one does not, because a receiver reads
+//! only the token and sequence number and treats the rest as opaque.
+
+use crate::{
+    envelope,
+    flags::{has, FLAG_CLOSE, FLAG_OPEN, FLAG_REPLY},
+    layout::{echo_packet_len, PacketLayout},
+    params::Params,
+    ProtoError, Result, SEQ_SIZE, TOKEN_SIZE,
+};
+
+/// A request to construct, described in sender-side semantic terms.
+///
+/// Every variant borrows what it needs; the encoder owns nothing.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum RequestToEncode<'a> {
+    /// Session-opening request. `no_test` selects the *no-test* form, which
+    /// adds `FLAG_CLOSE` and asks the peer to answer without creating a
+    /// session.
+    Open { params: &'a Params, no_test: bool },
+    /// Session teardown request.
+    Close { token: u64 },
+    /// Round-trip probe. `params` supplies the negotiated packet layout and
+    /// length, and `payload` is placed at the negotiated payload offset.
+    Echo {
+        token: u64,
+        sequence: u32,
+        params: &'a Params,
+        payload: &'a [u8],
+    },
+}
+
+/// A structurally decoded inbound request, borrowed from the packet.
+///
+/// `hmac_present` reports only that `FLAG_HMAC` was set and that the packet is
+/// laid out with an authentication field. It is **not** an authentication
+/// result: the field may hold arbitrary bytes. Use [`verify_packet_hmac`] to
+/// check the MAC once the applicable key is known.
+///
+/// [`verify_packet_hmac`]: crate::verify_packet_hmac
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct DecodedRequest<'a> {
+    pub hmac_present: bool,
+    pub kind: DecodedRequestKind<'a>,
+}
+
+/// The request kinds a receiver can distinguish from the packet alone.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum DecodedRequestKind<'a> {
+    /// `FLAG_OPEN` was set. `params` is the still-encoded parameter payload;
+    /// see [`decode_request`] for why it is not decoded here.
+    Open { no_test: bool, params: &'a [u8] },
+    /// `FLAG_CLOSE` was set without `FLAG_OPEN`. Trailing bytes after the token
+    /// are accepted and not exposed, because they carry no meaning.
+    Close { token: u64 },
+    /// Neither `FLAG_OPEN` nor `FLAG_CLOSE` was set. `tail` is everything after
+    /// the sequence number, which is opaque to a receiver.
+    Echo {
+        token: u64,
+        sequence: u32,
+        tail: &'a [u8],
+    },
+}
+
+/// Encodes an outbound request.
+///
+/// This is sender-side semantic construction: the variant determines the flags,
+/// so callers never assemble flag bits themselves.
+///
+/// `hmac_key` is authoritative. `Some(key)` sets `FLAG_HMAC`, reserves the
+/// 16-byte authentication field after the header, and signs the finished
+/// datagram; `None` produces an unauthenticated packet.
+///
+/// [`RequestToEncode::Echo`] requires the negotiated [`Params`] because the
+/// sender must build a datagram of the negotiated length with the payload at
+/// the negotiated offset. The receiving side needs no `Params` — see
+/// [`decode_request`].
+///
+/// # Errors
+///
+/// Returns [`ProtoError::PayloadTooLarge`] when an ECHO payload does not fit
+/// the negotiated length, and [`ProtoError::NegativePacketLength`] when the
+/// negotiated length is negative.
+pub fn encode_request(request: RequestToEncode<'_>, hmac_key: Option<&[u8]>) -> Result<Vec<u8>> {
+    match request {
+        RequestToEncode::Open { params, no_test } => encode_open(params, no_test, hmac_key),
+        RequestToEncode::Close { token } => encode_close(token, hmac_key),
+        RequestToEncode::Echo {
+            token,
+            sequence,
+            params,
+            payload,
+        } => encode_echo(token, sequence, params, payload, hmac_key),
+    }
+}
+
+/// Structurally decodes an inbound request.
+///
+/// This is packet structure only — it is **not** admission control. It performs
+/// no token or session lookup, applies no server policy, and needs no HMAC key,
+/// precisely so that a receiver can extract a token *before* it knows which
+/// session and key apply.
+///
+/// Classification, for a packet with valid magic and no reserved flag bits set:
+///
+/// - `FLAG_REPLY` set — rejected; a reply is never an inbound request.
+/// - `FLAG_OPEN` set — [`DecodedRequestKind::Open`], with `no_test` from
+///   `FLAG_CLOSE`. **Open takes precedence over Close and over an echo-shaped
+///   body**: an `OPEN | CLOSE` packet is a no-test open, and bytes that look
+///   like a token and sequence number are parameter data.
+/// - otherwise `FLAG_CLOSE` set — [`DecodedRequestKind::Close`].
+/// - otherwise — [`DecodedRequestKind::Echo`].
+///
+/// `FLAG_HMAC` is orthogonal to all of the above and is reported as
+/// [`DecodedRequest::hmac_present`], which indicates field presence rather than
+/// valid authentication.
+///
+/// Open parameters are returned as encoded bytes rather than a decoded
+/// [`Params`], so that an authenticating receiver can verify a packet before
+/// spending effort on untrusted parameter data. A packet with malformed
+/// parameters decodes structurally and fails later at [`Params::decode`].
+///
+/// Only structural minimum lengths are enforced — 4/12/16 bytes for
+/// Open/Close/Echo, plus 16 with `FLAG_HMAC`. Trailing bytes beyond a kind's
+/// fields are tolerated, no negotiated length ceiling is applied, and a zero
+/// token is accepted; discarding stale, unknown, or oversized requests is the
+/// receiving layer's policy.
+///
+/// # Errors
+///
+/// Returns [`ProtoError::PacketTooShort`], [`ProtoError::BadMagic`],
+/// [`ProtoError::ReservedFlags`], or [`ProtoError::UnexpectedFlag`] for
+/// `FLAG_REPLY`.
+pub fn decode_request(packet: &[u8]) -> Result<DecodedRequest<'_>> {
+    let envelope = envelope::decode_structural(packet)?;
+    if has(envelope.flags, FLAG_REPLY) {
+        return Err(ProtoError::UnexpectedFlag(FLAG_REPLY));
+    }
+
+    let body = envelope.body_offset;
+    let kind = if has(envelope.flags, FLAG_OPEN) {
+        // Open wins over Close and over an echo-shaped body.
+        require_len(packet, body)?;
+        DecodedRequestKind::Open {
+            no_test: has(envelope.flags, FLAG_CLOSE),
+            params: &packet[body..],
+        }
+    } else if has(envelope.flags, FLAG_CLOSE) {
+        require_len(packet, body + TOKEN_SIZE)?;
+        DecodedRequestKind::Close {
+            token: read_token(packet, body),
+        }
+    } else {
+        let tail_offset = body + TOKEN_SIZE + SEQ_SIZE;
+        require_len(packet, tail_offset)?;
+        DecodedRequestKind::Echo {
+            token: read_token(packet, body),
+            sequence: read_sequence(packet, body + TOKEN_SIZE),
+            tail: &packet[tail_offset..],
+        }
+    };
+
+    Ok(DecodedRequest {
+        hmac_present: envelope.hmac_present,
+        kind,
+    })
+}
+
+fn encode_open(params: &Params, no_test: bool, hmac_key: Option<&[u8]>) -> Result<Vec<u8>> {
+    let flags = if no_test {
+        FLAG_OPEN | FLAG_CLOSE
+    } else {
+        FLAG_OPEN
+    };
+    let encoded = params.encode();
+    let mut out = envelope::begin(
+        flags,
+        hmac_key,
+        PacketLayout::open_request(hmac_key.is_some()).header_len() + encoded.len(),
+    )?;
+    out.extend_from_slice(&encoded);
+    envelope::finish(out, hmac_key)
+}
+
+fn encode_close(token: u64, hmac_key: Option<&[u8]>) -> Result<Vec<u8>> {
+    let mut out = envelope::begin(
+        FLAG_CLOSE,
+        hmac_key,
+        PacketLayout::close_request(hmac_key.is_some()).header_len(),
+    )?;
+    out.extend_from_slice(&token.to_le_bytes());
+    envelope::finish(out, hmac_key)
+}
+
+fn encode_echo(
+    token: u64,
+    sequence: u32,
+    params: &Params,
+    payload: &[u8],
+    hmac_key: Option<&[u8]>,
+) -> Result<Vec<u8>> {
+    let layout = PacketLayout::echo(hmac_key.is_some(), params);
+    let len = echo_packet_len(hmac_key.is_some(), params)?;
+    let payload_offset = layout.header_len();
+    let available_payload_len = len.saturating_sub(payload_offset);
+    if payload.len() > available_payload_len {
+        return Err(ProtoError::PayloadTooLarge {
+            available: available_payload_len,
+            provided: payload.len(),
+        });
+    }
+
+    let mut out = envelope::begin(0, hmac_key, len)?;
+    out.extend_from_slice(&token.to_le_bytes());
+    out.extend_from_slice(&sequence.to_le_bytes());
+    // Everything from here to the negotiated length is zeroed: the reply-field
+    // positions the sender does not fill in, then the payload region.
+    out.resize(len, 0);
+    out[payload_offset..payload_offset + payload.len()].copy_from_slice(payload);
+
+    envelope::finish(out, hmac_key)
+}
+
+fn require_len(packet: &[u8], needed: usize) -> Result<()> {
+    if packet.len() < needed {
+        return Err(ProtoError::PacketTooShort {
+            needed,
+            actual: packet.len(),
+        });
+    }
+    Ok(())
+}
+
+fn read_token(packet: &[u8], offset: usize) -> u64 {
+    u64::from_le_bytes(packet[offset..offset + TOKEN_SIZE].try_into().unwrap())
+}
+
+fn read_sequence(packet: &[u8], offset: usize) -> u32 {
+    u32::from_le_bytes(packet[offset..offset + SEQ_SIZE].try_into().unwrap())
+}

crates/irtt-proto/tests/directional_codecs.rs

@@ -1,10 +1,8 @@
 use irtt_proto::{
-    decode_close_request, decode_echo_reply, decode_echo_request, decode_open_reply,
+    decode_echo_reply, decode_open_reply, decode_request, echo_packet_len, encode_echo_reply,
-    decode_open_request, echo_packet_len, encode_close_request, encode_echo_reply,
+    encode_open_reply, encode_request, hmac::compute_hmac_in_place, Clock, DecodedRequestKind,
-    encode_echo_request, encode_open_reply, encode_open_request, hmac::compute_hmac_in_place,
+    EchoReply, OpenReply, PacketLayout, Params, ProtoError, ReceivedStats, RequestToEncode,
-    Clock, CloseRequest, EchoReply, EchoRequest, OpenReply, OpenRequest, PacketLayout, Params,
+    StampAt, TimestampFields, FLAG_CLOSE, FLAG_HMAC, FLAG_OPEN, FLAG_REPLY, MAGIC,
-    ProtoError, ReceivedStats, StampAt, TimestampFields, FLAG_CLOSE, FLAG_HMAC, FLAG_OPEN,
-    FLAG_REPLY, MAGIC,
 };
 
 /// Wire offset of the HMAC digest within an ECHO reply: the fixed 4-byte
@@ -26,10 +24,11 @@ fn params() -> Params {
     }
 }
 
-fn echo_request(payload: Vec<u8>) -> EchoRequest {
+fn echo_request<'a>(params: &'a Params, payload: &'a [u8]) -> RequestToEncode<'a> {
-    EchoRequest {
+    RequestToEncode::Echo {
         token: TOKEN,
         sequence: 17,
+        params,
         payload,
     }
 }
@@ -120,21 +119,8 @@ fn peer_close_echo_reply_round_trips_without_a_close_reply_codec() {
 }
 
 #[test]
-fn inverse_codecs_supply_their_packet_type_rules() {
+fn reply_encoders_supply_their_packet_type_rules() {
     let params = Params::default();
-    let open_reply_packet = encode_open_reply(
-        &OpenReply {
-            flags: FLAG_OPEN | FLAG_REPLY,
-            token: TOKEN,
-            params: params.clone(),
-        },
-        None,
-    )
-    .unwrap();
-    assert_eq!(
-        decode_open_request(&open_reply_packet, None),
-        Err(ProtoError::UnexpectedFlag(FLAG_REPLY))
-    );
     assert_eq!(
         encode_open_reply(
             &OpenReply {
@@ -146,13 +132,6 @@ fn inverse_codecs_supply_their_packet_type_rules() {
         ),
         Err(ProtoError::MissingFlag(FLAG_OPEN))
     );
-
-    let echo_reply_packet =
-        encode_echo_reply(&echo_reply(&params, FLAG_REPLY), &params, None).unwrap();
-    assert_eq!(
-        decode_echo_request(&echo_reply_packet, &params, None),
-        Err(ProtoError::UnexpectedFlag(FLAG_REPLY))
-    );
     assert_eq!(
         encode_echo_reply(
             &EchoReply {
@@ -164,37 +143,45 @@ fn inverse_codecs_supply_their_packet_type_rules() {
         ),
         Err(ProtoError::MissingFlag(FLAG_REPLY))
     );
+}
 
-    let close_packet = encode_close_request(&CloseRequest { token: TOKEN }, None).unwrap();
+/// Every packet the reply encoders produce must be rejected by the inbound
-    assert_eq!(
+/// request decoder: a server never answers a datagram carrying `FLAG_REPLY`.
-        decode_echo_request(&close_packet, &params, None),
+#[test]
-        Err(ProtoError::UnexpectedFlag(FLAG_CLOSE))
+fn encoded_replies_are_never_admitted_as_inbound_requests() {
-    );
+    let params = Params::default();
 
-    let no_test_packet = encode_open_request(
+    for reply_flags in [FLAG_OPEN | FLAG_REPLY, FLAG_OPEN | FLAG_REPLY | FLAG_CLOSE] {
-        &OpenRequest {
+        let packet = encode_open_reply(
-            params,
+            &OpenReply {
-            close: true,
+                flags: reply_flags,
+                token: if reply_flags & FLAG_CLOSE == 0 {
+                    TOKEN
+                } else {
+                    0
+                },
+                params: params.clone(),
             },
             None,
         )
         .unwrap();
         assert_eq!(
-        decode_close_request(&no_test_packet, None),
+            decode_request(&packet),
-        Err(ProtoError::UnexpectedFlag(FLAG_OPEN))
+            Err(ProtoError::UnexpectedFlag(FLAG_REPLY))
         );
+    }
+
+    for reply_flags in [FLAG_REPLY, FLAG_REPLY | FLAG_CLOSE] {
+        let packet = encode_echo_reply(&echo_reply(&params, reply_flags), &params, None).unwrap();
+        assert_eq!(
+            decode_request(&packet),
+            Err(ProtoError::UnexpectedFlag(FLAG_REPLY))
+        );
+    }
 }
 
 #[test]
-fn codec_specific_malformed_bodies_and_exact_lengths_are_rejected() {
+fn reply_bodies_are_decoded_semantically_and_reject_malformed_params() {
-    let mut malformed_open_request = MAGIC.to_vec();
-    malformed_open_request.push(FLAG_OPEN);
-    malformed_open_request.extend_from_slice(&[1, 0x80]);
-    assert_eq!(
-        decode_open_request(&malformed_open_request, None),
-        Err(ProtoError::TruncatedVarint)
-    );
-
     let mut malformed_open_reply = MAGIC.to_vec();
     malformed_open_reply.push(FLAG_OPEN | FLAG_REPLY);
     malformed_open_reply.extend_from_slice(&TOKEN.to_le_bytes());
@@ -203,46 +190,6 @@ fn codec_specific_malformed_bodies_and_exact_lengths_are_rejected() {
         decode_open_reply(&malformed_open_reply, None),
         Err(ProtoError::TruncatedVarint)
     );
-
-    let params = Params {
-        length: 20,
-        ..Params::default()
-    };
-    let echo_packet = encode_echo_request(&echo_request(Vec::new()), &params, None).unwrap();
-    assert_eq!(
-        decode_echo_request(&echo_packet[..19], &params, None),
-        Err(ProtoError::PacketLengthMismatch {
-            expected: 20,
-            actual: 19,
-        })
-    );
-    let mut long_echo_packet = echo_packet;
-    long_echo_packet.push(0);
-    assert_eq!(
-        decode_echo_request(&long_echo_packet, &params, None),
-        Err(ProtoError::PacketLengthMismatch {
-            expected: 20,
-            actual: 21,
-        })
-    );
-
-    let close_packet = encode_close_request(&CloseRequest { token: TOKEN }, None).unwrap();
-    assert_eq!(
-        decode_close_request(&close_packet[..11], None),
-        Err(ProtoError::PacketLengthMismatch {
-            expected: 12,
-            actual: 11,
-        })
-    );
-    let mut long_close_packet = close_packet;
-    long_close_packet.push(0);
-    assert_eq!(
-        decode_close_request(&long_close_packet, None),
-        Err(ProtoError::PacketLengthMismatch {
-            expected: 12,
-            actual: 13,
-        })
-    );
 }
 
 #[test]
@@ -323,17 +270,10 @@ fn echo_payload_is_copied_zero_filled_and_bounded_in_both_directions() {
         ..Params::default()
     };
 
-    let request = echo_request(vec![1, 2]);
+    let packet = encode_request(echo_request(&params, &[1, 2]), None).unwrap();
-    let packet = encode_echo_request(&request, &params, None).unwrap();
     assert_eq!(&packet[16..], &[1, 2, 0, 0]);
-    let decoded = decode_echo_request(&packet, &params, None).unwrap();
-    assert_eq!(decoded.payload, vec![1, 2, 0, 0]);
     assert_eq!(
-        encode_echo_request(&decoded, &params, None).unwrap(),
+        encode_request(echo_request(&params, &[0; 5]), None),
-        packet
-    );
-    assert_eq!(
-        encode_echo_request(&echo_request(vec![0; 5]), &params, None),
         Err(ProtoError::PayloadTooLarge {
             available: 4,
             provided: 5,
@@ -611,18 +551,23 @@ fn non_midpoint_stamp_at_has_no_longer_form_exception() {
     );
 }
 
+/// The midpoint compatibility length is a *reply* concept. An inbound ECHO
+/// request of that length is just a longer request, which a receiver accepts
+/// without applying any negotiated length rule.
 #[test]
-fn echo_request_decoding_rejects_the_compat_length() {
+fn echo_request_decoding_has_no_compat_length_rule() {
     let params = midpoint_params(Clock::Wall, 0);
-    let packet = encode_echo_request(&echo_request(Vec::new()), &params, None).unwrap();
+    let packet = encode_request(echo_request(&params, &[]), None).unwrap();
 
     let mut too_long = packet.clone();
     too_long.extend_from_slice(&[0; 8]);
+    let request = decode_request(&too_long).unwrap();
     assert_eq!(
-        decode_echo_request(&too_long, &params, None),
+        request.kind,
-        Err(ProtoError::PacketLengthMismatch {
+        DecodedRequestKind::Echo {
-            expected: packet.len(),
+            token: TOKEN,
-            actual: too_long.len(),
+            sequence: 17,
-        })
+            tail: &too_long[16..],
+        }
     );
 }

crates/irtt-proto/tests/request_codec.rs

@@ -0,0 +1,470 @@
+//! Coverage for the unified request codec: sender-side [`encode_request`] and
+//! receiver-side structural [`decode_request`], plus the packet-level HMAC
+//! verifier that separates authentication from structure.
+
+use irtt_proto::{
+    decode_request, encode_request, hmac::compute_hmac_in_place, verify_packet_hmac, Clock,
+    DecodedRequest, DecodedRequestKind, Params, ProtoError, ReceivedStats, RequestToEncode,
+    StampAt, FLAG_CLOSE, FLAG_HMAC, FLAG_OPEN, FLAG_REPLY, HMAC_SIZE, MAGIC,
+};
+
+const KEY: &[u8] = b"testkey";
+const TOKEN: u64 = 0x7896_b6ab_8771_5213;
+const SEQUENCE: u32 = 0x1122_3344;
+
+fn params() -> Params {
+    Params {
+        protocol_version: 1,
+        duration_ns: 3_000_000_000,
+        interval_ns: 1_000_000_000,
+        received_stats: ReceivedStats::Both,
+        stamp_at: StampAt::Both,
+        clock: Clock::Both,
+        ..Params::default()
+    }
+}
+
+/// Builds a raw request of exactly `len` bytes: header, then an HMAC field when
+/// flagged, then `body`, then zero padding.
+fn raw(flags: u8, body: &[u8], len: usize) -> Vec<u8> {
+    let mut packet = MAGIC.to_vec();
+    packet.push(flags);
+    if flags & FLAG_HMAC != 0 {
+        packet.extend_from_slice(&[0xAA; HMAC_SIZE]);
+    }
+    packet.extend_from_slice(body);
+    packet.resize(len, 0);
+    packet
+}
+
+fn header(flags: u8) -> Vec<u8> {
+    raw(flags, &[], if flags & FLAG_HMAC != 0 { 20 } else { 4 })
+}
+
+fn token_and_sequence() -> Vec<u8> {
+    let mut body = TOKEN.to_le_bytes().to_vec();
+    body.extend_from_slice(&SEQUENCE.to_le_bytes());
+    body
+}
+
+// ---------- A. Classification ----------
+
+fn classify(kind: &DecodedRequestKind<'_>) -> &'static str {
+    match kind {
+        DecodedRequestKind::Open { no_test: false, .. } => "open",
+        DecodedRequestKind::Open { no_test: true, .. } => "no-test open",
+        DecodedRequestKind::Close { .. } => "close",
+        DecodedRequestKind::Echo { .. } => "echo",
+    }
+}
+
+#[test]
+fn flag_combinations_classify_requests_with_orthogonal_hmac_presence() {
+    let cases = [
+        (0, false, "echo"),
+        (FLAG_HMAC, true, "echo"),
+        (FLAG_CLOSE, false, "close"),
+        (FLAG_CLOSE | FLAG_HMAC, true, "close"),
+        (FLAG_OPEN, false, "open"),
+        (FLAG_OPEN | FLAG_CLOSE, false, "no-test open"),
+        (FLAG_OPEN | FLAG_HMAC, true, "open"),
+        (FLAG_OPEN | FLAG_CLOSE | FLAG_HMAC, true, "no-test open"),
+    ];
+
+    for (flags, hmac_present, expected) in cases {
+        // Long enough for any kind, so only the flags decide classification.
+        let packet = raw(flags, &token_and_sequence(), 64);
+        let request = decode_request(&packet).unwrap_or_else(|error| {
+            panic!("flags 0x{flags:02x} must decode structurally, got {error:?}")
+        });
+
+        assert_eq!(
+            request.hmac_present, hmac_present,
+            "flags 0x{flags:02x} HMAC presence"
+        );
+        assert_eq!(
+            classify(&request.kind),
+            expected,
+            "flags 0x{flags:02x} classification"
+        );
+    }
+}
+
+// ---------- B. Invalid header / flags ----------
+
+#[test]
+fn structurally_invalid_headers_are_rejected() {
+    assert_eq!(
+        decode_request(&MAGIC),
+        Err(ProtoError::PacketTooShort {
+            needed: 4,
+            actual: 3,
+        })
+    );
+    assert_eq!(
+        decode_request(&[0x00, MAGIC[1], MAGIC[2], FLAG_OPEN]),
+        Err(ProtoError::BadMagic)
+    );
+
+    for reserved in [0x10_u8, 0x20, 0x40, 0x80, 0xF0] {
+        assert_eq!(
+            decode_request(&header(reserved)),
+            Err(ProtoError::ReservedFlags(reserved)),
+            "reserved bits 0x{reserved:02x}"
+        );
+    }
+}
+
+#[test]
+fn any_reply_flagged_datagram_is_rejected_as_a_request() {
+    for flags in [
+        FLAG_REPLY,
+        FLAG_OPEN | FLAG_REPLY,
+        FLAG_CLOSE | FLAG_REPLY,
+        FLAG_OPEN | FLAG_CLOSE | FLAG_REPLY,
+        FLAG_REPLY | FLAG_HMAC,
+        FLAG_OPEN | FLAG_REPLY | FLAG_HMAC,
+        FLAG_CLOSE | FLAG_REPLY | FLAG_HMAC,
+        FLAG_OPEN | FLAG_CLOSE | FLAG_REPLY | FLAG_HMAC,
+    ] {
+        assert_eq!(
+            decode_request(&raw(flags, &token_and_sequence(), 64)),
+            Err(ProtoError::UnexpectedFlag(FLAG_REPLY)),
+            "flags 0x{flags:02x}"
+        );
+    }
+}
+
+// ---------- C. Structural minimum lengths ----------
+
+#[test]
+fn structural_minimum_lengths_are_enforced_per_kind() {
+    // (flags, minimum accepted length)
+    let cases = [
+        (FLAG_OPEN, 4_usize),
+        (FLAG_CLOSE, 12),
+        (0, 16),
+        (FLAG_OPEN | FLAG_HMAC, 20),
+        (FLAG_CLOSE | FLAG_HMAC, 28),
+        (FLAG_HMAC, 32),
+    ];
+
+    for (flags, minimum) in cases {
+        let accepted = raw(flags, &token_and_sequence(), minimum);
+        assert_eq!(accepted.len(), minimum);
+        assert!(
+            decode_request(&accepted).is_ok(),
+            "flags 0x{flags:02x} length {minimum} must be accepted"
+        );
+
+        // One byte short of the minimum, for every kind that has bytes to lose.
+        let short_len = minimum - 1;
+        if short_len >= 4 {
+            assert_eq!(
+                decode_request(&accepted[..short_len]),
+                Err(ProtoError::PacketTooShort {
+                    needed: minimum,
+                    actual: short_len,
+                }),
+                "flags 0x{flags:02x} length {short_len}"
+            );
+        }
+    }
+}
+
+// ---------- D. Field offsets ----------
+
+#[test]
+fn token_and_sequence_are_read_from_the_hmac_dependent_offsets() {
+    for hmac in [false, true] {
+        let close_flags = FLAG_CLOSE | if hmac { FLAG_HMAC } else { 0 };
+        let close = raw(
+            close_flags,
+            &TOKEN.to_le_bytes(),
+            if hmac { 28 } else { 12 },
+        );
+        assert_eq!(
+            decode_request(&close).unwrap(),
+            DecodedRequest {
+                hmac_present: hmac,
+                kind: DecodedRequestKind::Close { token: TOKEN },
+            }
+        );
+
+        let echo_flags = if hmac { FLAG_HMAC } else { 0 };
+        let echo = raw(
+            echo_flags,
+            &token_and_sequence(),
+            if hmac { 32 } else { 16 },
+        );
+        assert_eq!(
+            decode_request(&echo).unwrap(),
+            DecodedRequest {
+                hmac_present: hmac,
+                kind: DecodedRequestKind::Echo {
+                    token: TOKEN,
+                    sequence: SEQUENCE,
+                    tail: &[],
+                },
+            }
+        );
+    }
+}
+
+// ---------- E. Open parameter slice ----------
+
+#[test]
+fn open_params_are_borrowed_after_the_header_and_decoded_later() {
+    let encoded = params().encode();
+
+    for hmac in [false, true] {
+        let flags = FLAG_OPEN | if hmac { FLAG_HMAC } else { 0 };
+        let expected_offset = if hmac { 4 + HMAC_SIZE } else { 4 };
+        let packet = raw(flags, &encoded, expected_offset + encoded.len());
+
+        match decode_request(&packet).unwrap().kind {
+            DecodedRequestKind::Open { params, .. } => {
+                assert_eq!(params, &packet[expected_offset..]);
+                assert_eq!(Params::decode(params).unwrap(), self::params());
+            }
+            other => panic!("expected an open request, got {other:?}"),
+        }
+    }
+}
+
+#[test]
+fn empty_and_malformed_open_params_decode_structurally() {
+    let empty = header(FLAG_OPEN);
+    match decode_request(&empty).unwrap().kind {
+        DecodedRequestKind::Open { params, .. } => assert!(params.is_empty()),
+        other => panic!("expected an open request, got {other:?}"),
+    }
+
+    // A truncated varint: structurally fine, semantically not. A receiver must
+    // be able to authenticate before spending effort on this.
+    let malformed = raw(FLAG_OPEN, &[1, 0x80], 6);
+    match decode_request(&malformed).unwrap().kind {
+        DecodedRequestKind::Open { params, .. } => {
+            assert_eq!(params, &[1, 0x80]);
+            assert_eq!(Params::decode(params), Err(ProtoError::TruncatedVarint));
+        }
+        other => panic!("expected an open request, got {other:?}"),
+    }
+}
+
+// ---------- F. Open precedence ----------
+
+#[test]
+fn open_precedence_beats_close_and_an_echo_shaped_body() {
+    // An OPEN|CLOSE datagram whose body looks exactly like a token and
+    // sequence number is still a no-test open, and those bytes are parameters.
+    let packet = raw(FLAG_OPEN | FLAG_CLOSE, &token_and_sequence(), 16);
+
+    assert_eq!(
+        decode_request(&packet).unwrap(),
+        DecodedRequest {
+            hmac_present: false,
+            kind: DecodedRequestKind::Open {
+                no_test: true,
+                params: &packet[4..],
+            },
+        }
+    );
+}
+
+// ---------- G. Trailing data ----------
+
+#[test]
+fn trailing_bytes_are_tolerated_and_no_length_ceiling_applies() {
+    let mut close = raw(FLAG_CLOSE, &TOKEN.to_le_bytes(), 12);
+    close.extend_from_slice(&[0xDE; 20]);
+    assert_eq!(
+        decode_request(&close).unwrap().kind,
+        DecodedRequestKind::Close { token: TOKEN }
+    );
+
+    let mut echo = raw(0, &token_and_sequence(), 16);
+    echo.extend_from_slice(&[0xBE; 4]);
+    assert_eq!(
+        decode_request(&echo).unwrap().kind,
+        DecodedRequestKind::Echo {
+            token: TOKEN,
+            sequence: SEQUENCE,
+            tail: &[0xBE; 4],
+        }
+    );
+
+    // Far longer than any negotiated length: still just a request with a long
+    // opaque tail. No `Params` are consulted at any point.
+    let long = raw(0, &token_and_sequence(), 4096);
+    match decode_request(&long).unwrap().kind {
+        DecodedRequestKind::Echo { tail, .. } => assert_eq!(tail.len(), 4096 - 16),
+        other => panic!("expected an echo request, got {other:?}"),
+    }
+}
+
+// ---------- H. Zero token ----------
+
+#[test]
+fn a_zero_token_is_structurally_valid() {
+    assert_eq!(
+        decode_request(&raw(FLAG_CLOSE, &0_u64.to_le_bytes(), 12))
+            .unwrap()
+            .kind,
+        DecodedRequestKind::Close { token: 0 }
+    );
+    assert_eq!(
+        decode_request(&raw(0, &[0; 12], 16)).unwrap().kind,
+        DecodedRequestKind::Echo {
+            token: 0,
+            sequence: 0,
+            tail: &[],
+        }
+    );
+}
+
+// ---------- HMAC presence is not authentication ----------
+
+#[test]
+fn hmac_presence_is_reported_independently_of_mac_validity() {
+    // A structurally complete authenticated echo whose MAC is garbage.
+    let garbage = raw(FLAG_HMAC, &token_and_sequence(), 32);
+    let request = decode_request(&garbage).unwrap();
+    assert!(request.hmac_present);
+    assert!(matches!(request.kind, DecodedRequestKind::Echo { .. }));
+
+    // The same packet fails verification.
+    assert_eq!(verify_packet_hmac(KEY, &garbage), Err(ProtoError::BadHmac));
+
+    // Correctly signed: accepted, and verification leaves the bytes alone.
+    let mut signed = garbage.clone();
+    compute_hmac_in_place(KEY, &mut signed, 4).unwrap();
+    let before = signed.clone();
+    verify_packet_hmac(KEY, &signed).unwrap();
+    assert_eq!(signed, before);
+    assert_eq!(
+        verify_packet_hmac(b"other-key", &signed),
+        Err(ProtoError::BadHmac)
+    );
+}
+
+#[test]
+fn packet_verification_requires_a_present_and_complete_hmac_field() {
+    // No FLAG_HMAC: there is no field to verify.
+    let unauthenticated = raw(0, &token_and_sequence(), 16);
+    assert_eq!(
+        verify_packet_hmac(KEY, &unauthenticated),
+        Err(ProtoError::MissingFlag(FLAG_HMAC))
+    );
+
+    // Flagged but truncated inside the field.
+    let truncated = raw(FLAG_HMAC, &[], 12);
+    assert_eq!(
+        verify_packet_hmac(KEY, &truncated),
+        Err(ProtoError::InvalidHmacOffset)
+    );
+    // Structural decoding rejects it too, for the kind's minimum length.
+    assert_eq!(
+        decode_request(&truncated),
+        Err(ProtoError::PacketTooShort {
+            needed: 32,
+            actual: 12,
+        })
+    );
+
+    // A malformed header is rejected before any MAC work.
+    assert_eq!(
+        verify_packet_hmac(KEY, &[0x00, MAGIC[1], MAGIC[2], FLAG_HMAC]),
+        Err(ProtoError::BadMagic)
+    );
+}
+
+// ---------- Encode -> decode cross-checks ----------
+
+#[test]
+fn encoded_requests_decode_back_to_their_sender_side_identity() {
+    for key in [None, Some(KEY)] {
+        for no_test in [false, true] {
+            let packet = encode_request(
+                RequestToEncode::Open {
+                    params: &params(),
+                    no_test,
+                },
+                key,
+            )
+            .unwrap();
+            let request = decode_request(&packet).unwrap();
+            assert_eq!(request.hmac_present, key.is_some());
+            match request.kind {
+                DecodedRequestKind::Open {
+                    no_test: decoded,
+                    params: encoded,
+                } => {
+                    assert_eq!(decoded, no_test);
+                    assert_eq!(Params::decode(encoded).unwrap(), params());
+                }
+                other => panic!("expected an open request, got {other:?}"),
+            }
+            if let Some(key) = key {
+                verify_packet_hmac(key, &packet).unwrap();
+            }
+        }
+
+        let packet = encode_request(RequestToEncode::Close { token: TOKEN }, key).unwrap();
+        let request = decode_request(&packet).unwrap();
+        assert_eq!(request.hmac_present, key.is_some());
+        assert_eq!(request.kind, DecodedRequestKind::Close { token: TOKEN });
+
+        // The ECHO tail is deliberately not compared against the logical
+        // payload: the sender placed it at a negotiated offset the receiver
+        // cannot recover without `Params`.
+        let echo_params = Params {
+            length: 96,
+            ..params()
+        };
+        let packet = encode_request(
+            RequestToEncode::Echo {
+                token: TOKEN,
+                sequence: SEQUENCE,
+                params: &echo_params,
+                payload: &[1, 2, 3, 4],
+            },
+            key,
+        )
+        .unwrap();
+        let request = decode_request(&packet).unwrap();
+        assert_eq!(request.hmac_present, key.is_some());
+        match request.kind {
+            DecodedRequestKind::Echo {
+                token, sequence, ..
+            } => {
+                assert_eq!(token, TOKEN);
+                assert_eq!(sequence, SEQUENCE);
+            }
+            other => panic!("expected an echo request, got {other:?}"),
+        }
+        if let Some(key) = key {
+            verify_packet_hmac(key, &packet).unwrap();
+        }
+    }
+}
+
+#[test]
+fn echo_encoding_rejects_a_negative_negotiated_length() {
+    let params = Params {
+        length: -1,
+        ..Params::default()
+    };
+    assert_eq!(
+        encode_request(
+            RequestToEncode::Echo {
+                token: TOKEN,
+                sequence: SEQUENCE,
+                params: &params,
+                payload: &[],
+            },
+            None,
+        ),
+        Err(ProtoError::NegativePacketLength { length: -1 })
+    );
+}

crates/irtt-proto/tests/vectors.rs

@@ -1,10 +1,55 @@
 use irtt_proto::{
-    decode_close_request, decode_echo_reply, decode_echo_request, decode_open_reply,
+    decode_echo_reply, decode_open_reply, decode_request, encode_echo_reply, encode_open_reply,
-    decode_open_request, encode_close_request, encode_echo_reply, encode_echo_request,
+    encode_request, verify_hmac, verify_packet_hmac, Clock, DecodedRequestKind, Params, ProtoError,
-    encode_open_reply, encode_open_request, verify_hmac, Clock, CloseRequest, EchoRequest,
+    ReceivedStats, RequestToEncode, StampAt, FLAG_REPLY,
-    OpenRequest, Params, ProtoError, ReceivedStats, StampAt, FLAG_REPLY,
 };
 
+/// Asserts that `packet` structurally decodes as an OPEN request carrying
+/// `params`, with the expected no-test and HMAC-presence flags.
+#[track_caller]
+fn assert_open_request(packet: &[u8], params: &Params, no_test: bool, hmac_present: bool) {
+    let request = decode_request(packet).unwrap();
+    assert_eq!(request.hmac_present, hmac_present);
+    match request.kind {
+        DecodedRequestKind::Open {
+            no_test: decoded_no_test,
+            params: encoded,
+        } => {
+            assert_eq!(decoded_no_test, no_test);
+            assert_eq!(&Params::decode(encoded).unwrap(), params);
+        }
+        other => panic!("expected an open request, got {other:?}"),
+    }
+}
+
+/// Asserts that `packet` structurally decodes as an ECHO request with the
+/// expected token and sequence number. The tail is deliberately not compared
+/// against a logical payload: recovering the sender's payload offset needs
+/// `Params`, which structural decoding does not take.
+#[track_caller]
+fn assert_echo_request(packet: &[u8], token: u64, sequence: u32, hmac_present: bool) {
+    let request = decode_request(packet).unwrap();
+    assert_eq!(request.hmac_present, hmac_present);
+    match request.kind {
+        DecodedRequestKind::Echo {
+            token: decoded_token,
+            sequence: decoded_sequence,
+            ..
+        } => {
+            assert_eq!(decoded_token, token);
+            assert_eq!(decoded_sequence, sequence);
+        }
+        other => panic!("expected an echo request, got {other:?}"),
+    }
+}
+
+#[track_caller]
+fn assert_close_request(packet: &[u8], token: u64, hmac_present: bool) {
+    let request = decode_request(packet).unwrap();
+    assert_eq!(request.hmac_present, hmac_present);
+    assert_eq!(request.kind, DecodedRequestKind::Close { token });
+}
+
 fn hex(input: &str) -> Vec<u8> {
     input
         .split_whitespace()
@@ -42,22 +87,16 @@ fn hmac_params_2s() -> Params {
 fn vector_1_open_request_no_hmac() {
     let expected = hex("14 a7 5b 01 01 02 02 80 f8 82 ad 16 03 80 a8 d6
          b9 07 05 06 06 06 07 06");
-    let packet = encode_open_request(
+    let packet = encode_request(
-        &OpenRequest {
+        RequestToEncode::Open {
-            params: default_params_3s(),
+            params: &default_params_3s(),
-            close: false,
+            no_test: false,
         },
         None,
     )
     .unwrap();
     assert_eq!(packet, expected);
-    assert_eq!(
+    assert_open_request(&expected, &default_params_3s(), false, false);
-        decode_open_request(&expected, None),
-        Ok(OpenRequest {
-            params: default_params_3s(),
-            close: false,
-        })
-    );
 }
 
 #[test]
@@ -76,25 +115,18 @@ fn vector_3_echo_request_no_hmac() {
          00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
          00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
          00 00 00 00 00 00 00 00 00 00 00 00");
-    let packet = encode_echo_request(
+    let packet = encode_request(
-        &EchoRequest {
+        RequestToEncode::Echo {
             token: 0x7896_b6ab_8771_5213,
             sequence: 0,
-            payload: Vec::new(),
+            params: &default_params_3s(),
+            payload: &[],
         },
-        &default_params_3s(),
         None,
     )
     .unwrap();
     assert_eq!(packet, expected);
-    assert_eq!(
+    assert_echo_request(&expected, 0x7896_b6ab_8771_5213, 0, false);
-        decode_echo_request(&expected, &default_params_3s(), None),
-        Ok(EchoRequest {
-            token: 0x7896_b6ab_8771_5213,
-            sequence: 0,
-            payload: Vec::new(),
-        })
-    );
 }
 
 #[test]
@@ -116,20 +148,15 @@ fn vector_4_echo_reply_no_hmac() {
 #[test]
 fn vector_5_close_request_no_hmac() {
     let expected = hex("14 a7 5b 04 13 52 71 87 ab b6 96 78");
-    let packet = encode_close_request(
+    let packet = encode_request(
-        &CloseRequest {
+        RequestToEncode::Close {
             token: 0x7896_b6ab_8771_5213,
         },
         None,
     )
     .unwrap();
     assert_eq!(packet, expected);
-    assert_eq!(
+    assert_close_request(&expected, 0x7896_b6ab_8771_5213, false);
-        decode_close_request(&expected, None),
-        Ok(CloseRequest {
-            token: 0x7896_b6ab_8771_5213,
-        })
-    );
 }
 
 #[test]
@@ -137,23 +164,18 @@ fn vector_6_hmac_open_request() {
     let expected = hex("14 a7 5b 09 ff 90 16 a7 aa 53 78 16 9e c3 a2 d5
          54 dc 30 36 01 02 02 80 d0 ac f3 0e 03 80 a8 d6
          b9 07 05 06 06 06 07 06");
-    let packet = encode_open_request(
+    let packet = encode_request(
-        &OpenRequest {
+        RequestToEncode::Open {
-            params: hmac_params_2s(),
+            params: &hmac_params_2s(),
-            close: false,
+            no_test: false,
         },
         Some(b"testkey"),
     )
     .unwrap();
     assert_eq!(packet, expected);
     verify_hmac(b"testkey", &packet, 4).unwrap();
-    assert_eq!(
+    verify_packet_hmac(b"testkey", &packet).unwrap();
-        decode_open_request(&expected, Some(b"testkey")),
+    assert_open_request(&expected, &hmac_params_2s(), false, true);
-        Ok(OpenRequest {
-            params: hmac_params_2s(),
-            close: false,
-        })
-    );
 }
 
 #[test]
@@ -163,67 +185,51 @@ fn vector_7_hmac_echo_request() {
          00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
          00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
          00 00 00 00 00 00 00 00 00 00 00 00");
-    let packet = encode_echo_request(
+    let packet = encode_request(
-        &EchoRequest {
+        RequestToEncode::Echo {
             token: 0x4387_e9eb_5d3f_ca59,
             sequence: 0,
-            payload: Vec::new(),
+            params: &default_params_3s(),
+            payload: &[],
         },
-        &default_params_3s(),
         Some(b"testkey"),
     )
     .unwrap();
     assert_eq!(packet, expected);
-    assert_eq!(
+    verify_packet_hmac(b"testkey", &packet).unwrap();
-        decode_echo_request(&expected, &default_params_3s(), Some(b"testkey")),
+    assert_echo_request(&expected, 0x4387_e9eb_5d3f_ca59, 0, true);
-        Ok(EchoRequest {
-            token: 0x4387_e9eb_5d3f_ca59,
-            sequence: 0,
-            payload: Vec::new(),
-        })
-    );
 }
 
 #[test]
 fn vector_8_hmac_close_request() {
     let expected = hex("14 a7 5b 0c f5 cd 0f a9 de 9d 7d 66 8d 91 a5 32
          48 0e 42 e0 59 ca 3f 5d eb e9 87 43");
-    let packet = encode_close_request(
+    let packet = encode_request(
-        &CloseRequest {
+        RequestToEncode::Close {
             token: 0x4387_e9eb_5d3f_ca59,
         },
         Some(b"testkey"),
     )
     .unwrap();
     assert_eq!(packet, expected);
-    assert_eq!(
+    verify_packet_hmac(b"testkey", &packet).unwrap();
-        decode_close_request(&expected, Some(b"testkey")),
+    assert_close_request(&expected, 0x4387_e9eb_5d3f_ca59, true);
-        Ok(CloseRequest {
-            token: 0x4387_e9eb_5d3f_ca59,
-        })
-    );
 }
 
 #[test]
 fn vector_9_no_test_open_close_request() {
     let expected = hex("14 a7 5b 05 01 02 02 80 e0 ba 84 bf 03 03 80 a8
          d6 b9 07 05 06 06 06 07 06");
-    let packet = encode_open_request(
+    let packet = encode_request(
-        &OpenRequest {
+        RequestToEncode::Open {
-            params: default_params_60s(),
+            params: &default_params_60s(),
-            close: true,
+            no_test: true,
         },
         None,
     )
     .unwrap();
     assert_eq!(packet, expected);
-    assert_eq!(
+    assert_open_request(&expected, &default_params_60s(), true, false);
-        decode_open_request(&expected, None),
-        Ok(OpenRequest {
-            params: default_params_60s(),
-            close: true,
-        })
-    );
 }
 
 #[test]
@@ -240,25 +246,18 @@ fn vector_10_no_test_open_close_reply() {
 #[test]
 fn vector_11_minimal_echo_packet() {
     let expected = hex("14 a7 5b 00 4e 15 61 5c a2 6f 31 a0 00 00 00 00");
-    let packet = encode_echo_request(
+    let packet = encode_request(
-        &EchoRequest {
+        RequestToEncode::Echo {
             token: 0xa031_6fa2_5c61_154e,
             sequence: 0,
-            payload: Vec::new(),
+            params: &Params::default(),
+            payload: &[],
         },
-        &Params::default(),
         None,
     )
     .unwrap();
     assert_eq!(packet, expected);
-    assert_eq!(
+    assert_echo_request(&expected, 0xa031_6fa2_5c61_154e, 0, false);
-        decode_echo_request(&expected, &Params::default(), None),
-        Ok(EchoRequest {
-            token: 0xa031_6fa2_5c61_154e,
-            sequence: 0,
-            payload: Vec::new(),
-        })
-    );
 }
 
 #[test]
@@ -296,33 +295,20 @@ fn spec_18_1_hmac_echo_request_fixture() {
         clock: Clock::Both,
         ..Params::default()
     };
-    let packet = encode_echo_request(
+    let key = [0x3c, 0x68, 0x1d, 0x39, 0x41, 0x1d, 0x72, 0x43];
-        &EchoRequest {
+    let packet = encode_request(
+        RequestToEncode::Echo {
             token: 0x886b_c9a7_22b3_3eea,
             sequence: 0x6fe2_a1bb,
-            payload: [0xff, 0xfe, 0xfd, 0xfc].repeat(4),
+            params: &params,
+            payload: &[0xff, 0xfe, 0xfd, 0xfc].repeat(4),
         },
-        &params,
+        Some(&key),
-        Some(&[0x3c, 0x68, 0x1d, 0x39, 0x41, 0x1d, 0x72, 0x43]),
     )
     .unwrap();
     assert_eq!(packet, expected);
-    assert_eq!(
+    verify_packet_hmac(&key, &packet).unwrap();
-        decode_echo_request(
+    assert_echo_request(&expected, 0x886b_c9a7_22b3_3eea, 0x6fe2_a1bb, true);
-            &expected,
-            &Params {
-                length: 92,
-                received_stats: ReceivedStats::Both,
-                stamp_at: StampAt::Both,
-                clock: Clock::Both,
-                ..Params::default()
-            },
-            Some(&[0x3c, 0x68, 0x1d, 0x39, 0x41, 0x1d, 0x72, 0x43]),
-        )
-        .unwrap()
-        .payload,
-        [0xff, 0xfe, 0xfd, 0xfc].repeat(4)
-    );
 }
 
 #[test]