Merge pull request #73 from Lochnair/refactor/proto-request-codecs

refactor(proto): unify request codecs

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,27 +86,33 @@ 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;
-                    worker_probe.set();
+                };
-                    let reply = encode_echo_reply(
+                match request.kind {
-                        &EchoReply {
+                    DecodedRequestKind::Echo { sequence, .. } => {
-                            flags: flags::FLAG_REPLY,
+                        worker_probe.set();
-                            token: TOKEN,
+                        let reply = encode_echo_reply(
-                            sequence: probe.sequence,
+                            &EchoReply {
-                            recv_count: None,
+                                flags: flags::FLAG_REPLY,
-                            recv_window: None,
+                                token: TOKEN,
-                            timestamps: TimestampFields::default(),
+                                sequence,
-                            payload: Vec::new(),
+                                recv_count: None,
-                        },
+                                recv_window: None,
-                        &negotiated,
+                                timestamps: TimestampFields::default(),
-                        None,
+                                payload: Vec::new(),
-                    )
+                            },
-                    .unwrap();
+                            &negotiated,
-                    socket.send_to(&reply, packet_peer).unwrap();
+                            None,
-                } else if decode_close_request(packet, None).is_ok() {
+                        )
-                    worker_close.set();
+                        .unwrap();
-                    return;
+                        socket.send_to(&reply, packet_peer).unwrap();
+                    }
+                    DecodedRequestKind::Close { .. } => {
+                        worker_close.set();
+                        return;
+                    }
+                    DecodedRequestKind::Open { .. } => {}
                 }
             }
         });

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,77 +188,78 @@ 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()) {
-                thread_records.lock().unwrap().push(PacketRecord {
+                Some(DecodedRequestKind::Echo { sequence, .. }) => {
-                    kind: PacketKind::Probe,
+                    thread_records.lock().unwrap().push(PacketRecord {
-                    at: Instant::now(),
+                        kind: PacketKind::Probe,
-                });
+                        at: Instant::now(),
-                let (seen, ready) = &*thread_probe_seen;
+                    });
-                *seen.lock().unwrap() = true;
+                    let (seen, ready) = &*thread_probe_seen;
-                ready.notify_all();
+                    *seen.lock().unwrap() = true;
-                if let ServerBehavior::DelayedEcho(delay) = behavior {
+                    ready.notify_all();
-                    thread::sleep(delay);
+                    if let ServerBehavior::DelayedEcho(delay) = behavior {
-                }
+                        thread::sleep(delay);
-                if let ServerBehavior::DeferredBurst(count) = behavior {
+                    }
-                    if deferred_burst_sent {
+                    if let ServerBehavior::DeferredBurst(count) = behavior {
+                        if deferred_burst_sent {
+                            continue;
+                        }
+                        deferred_burst_sent = true;
+                        thread::sleep(Duration::from_millis(30));
+                        for reply_index in 0..count {
+                            let reply = encode_echo_reply(
+                                &EchoReply {
+                                    flags: flags::FLAG_REPLY,
+                                    token: TOKEN,
+                                    sequence: if reply_index < 2 { sequence } else { u32::MAX },
+                                    recv_count: None,
+                                    recv_window: None,
+                                    timestamps: TimestampFields::default(),
+                                    payload: Vec::new(),
+                                },
+                                &negotiated,
+                                key.as_deref(),
+                            )
+                            .unwrap();
+                            socket.send_to(&reply, packet_peer).unwrap();
+                        }
                         continue;
                     }
-                    deferred_burst_sent = true;
+                    if let Some(gate) = &reply_gate {
-                    thread::sleep(Duration::from_millis(30));
+                        gate.arrive_and_wait();
-                    for reply_index in 0..count {
+                    }
-                        let reply = encode_echo_reply(
+                    let peer_close = matches!(behavior, ServerBehavior::PeerClose);
-                            &EchoReply {
+                    let reply = encode_echo_reply(
-                                flags: flags::FLAG_REPLY,
+                        &EchoReply {
-                                token: TOKEN,
+                            flags: flags::FLAG_REPLY
-                                sequence: if reply_index < 2 {
+                                | if peer_close { flags::FLAG_CLOSE } else { 0 },
-                                    probe.sequence
+                            token: TOKEN,
-                                } else {
+                            sequence,
-                                    u32::MAX
+                            recv_count: None,
-                                },
+                            recv_window: None,
-                                recv_count: None,
+                            timestamps: TimestampFields::default(),
-                                recv_window: None,
+                            payload: Vec::new(),
-                                timestamps: TimestampFields::default(),
+                        },
-                                payload: Vec::new(),
+                        &negotiated,
-                            },
+                        key.as_deref(),
-                            &negotiated,
+                    )
-                            key.as_deref(),
+                    .unwrap();
-                        )
+                    socket.send_to(&reply, packet_peer).unwrap();
-                        .unwrap();
+                    let (sent, ready) = &*thread_reply_sent;
-                        socket.send_to(&reply, packet_peer).unwrap();
+                    *sent.lock().unwrap() = true;
+                    ready.notify_all();
+                    if peer_close {
+                        return;
                     }
-                    continue;
                 }
-                if let Some(gate) = &reply_gate {
+                Some(DecodedRequestKind::Close { .. }) => {
-                    gate.arrive_and_wait();
+                    thread_records.lock().unwrap().push(PacketRecord {
-                }
+                        kind: PacketKind::Close,
-                let peer_close = matches!(behavior, ServerBehavior::PeerClose);
+                        at: Instant::now(),
-                let reply = encode_echo_reply(
+                    });
-                    &EchoReply {
-                        flags: flags::FLAG_REPLY | if peer_close { flags::FLAG_CLOSE } else { 0 },
-                        token: TOKEN,
-                        sequence: probe.sequence,
-                        recv_count: None,
-                        recv_window: None,
-                        timestamps: TimestampFields::default(),
-                        payload: Vec::new(),
-                    },
-                    &negotiated,
-                    key.as_deref(),
-                )
-                .unwrap();
-                socket.send_to(&reply, packet_peer).unwrap();
-                let (sent, ready) = &*thread_reply_sent;
-                *sent.lock().unwrap() = true;
-                ready.notify_all();
-                if peer_close {
                     return;
                 }
-            } else if decode_close_request(packet, key.as_deref()).is_ok() {
+                Some(DecodedRequestKind::Open { .. }) | None => {}
-                thread_records.lock().unwrap().push(PacketRecord {
-                    kind: PacketKind::Close,
-                    at: Instant::now(),
-                });
-                return;
             }
         }
     });

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(
-            token: session.token,
+            RequestToEncode::Echo {
-            sequence: session.next_wire_seq,
+                token: session.token,
-            payload: vec![],
+                sequence: session.next_wire_seq,
-        };
+                params: &session.negotiated.params,
-        let bytes = encode_echo_request(
+                payload: &[],
-            &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());
+            len: packet.len(),
-    if let Some(key) = hmac_key {
+            hmac: request.hmac_present,
-        verify_hmac(key, packet, HMAC_OFFSET).unwrap();
+            token,
-    }
+            sequence,
-
+        },
-    let mut pos = 4 + if hmac { HMAC_SIZE } else { 0 };
+        other => panic!("expected an echo request, got {other:?}"),
-    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,
-        token,
-        sequence,
     }
 }
 
 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() {
+            out.extend_from_slice(&[0; TIMESTAMP_SIZE]);
         }
-    }
-
-    #[test]
-    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 {
-        None,
+                    TOKEN
-    )
+                } else {
-    .unwrap();
+                    0
-    assert_eq!(
+                },
-        decode_close_request(&no_test_packet, None),
+                params: params.clone(),
-        Err(ProtoError::UnexpectedFlag(FLAG_OPEN))
+            },
-    );
+            None,
+        )
+        .unwrap();
+        assert_eq!(
+            decode_request(&packet),
+            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]