diff --git a/crates/irtt-client/src/async_client/tests.rs b/crates/irtt-client/src/async_client/tests.rs index 5448b51..cd45c33 100644 --- a/crates/irtt-client/src/async_client/tests.rs +++ b/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, - encode_open_reply, flags, EchoReply, OpenReply, Params, ReceivedStats, StampAt, - TimestampFields, + decode_request, encode_echo_reply, encode_open_reply, flags, verify_packet_hmac, + DecodedRequestKind, EchoReply, OpenReply, Params, ReceivedStats, StampAt, TimestampFields, }; use tokio::runtime::{Builder, Runtime}; @@ -102,6 +101,47 @@ fn recv_packet(socket: &UdpSocket, tx: &mpsc::Sender>) -> (Vec, 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> { + 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>, 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(); - assert!(request.close); + let (params, no_test) = open_request(&open_packet, None); + 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, ¶ms, None).unwrap(); + let sequence = echo_request_sequence(&probe_packet, None); socket .send_to( &echo_reply( ¶ms, - probe.sequence, + sequence, TOKEN, flags::FLAG_REPLY | flags::FLAG_CLOSE, None, @@ -213,27 +253,16 @@ fn start_echo_close_server(key: Option>) -> 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, ¶ms, key.as_deref()).unwrap(); + let sequence = echo_request_sequence(&probe_packet, key.as_deref()); socket .send_to( - &echo_reply( - ¶ms, - probe.sequence, - TOKEN, - flags::FLAG_REPLY, - key.as_deref(), - ), + &echo_reply(¶ms, sequence, TOKEN, flags::FLAG_REPLY, key.as_deref()), peer, ) .unwrap(); let (close_packet, _) = recv_packet(&socket, &tx); - assert_eq!( - decode_close_request(&close_packet, key.as_deref()) - .unwrap() - .token, - TOKEN - ); + assert_eq!(close_request_token(&close_packet, key.as_deref()), 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, ¶ms, None).unwrap(); + let sequence = echo_request_sequence(&probe_packet, None); socket .send_to( - &echo_reply(¶ms, probe.sequence, TOKEN, flags::FLAG_REPLY, None), + &echo_reply(¶ms, 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!( - decode_close_request(&close_packet, None).unwrap().token, - TOKEN - ); + assert_eq!(close_request_token(&close_packet, None), TOKEN); }); runtime().block_on(async { let mut client = opened_client(close_server.addr, 0).await; diff --git a/crates/irtt-client/src/client/tests/hmac.rs b/crates/irtt-client/src/client/tests/hmac.rs index dc36a99..95cfd81 100644 --- a/crates/irtt-client/src/client/tests/hmac.rs +++ b/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, ¶ms, 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() diff --git a/crates/irtt-client/src/client/tests/mod.rs b/crates/irtt-client/src/client/tests/mod.rs index 7e4ebc8..0f38489 100644 --- a/crates/irtt-client/src/client/tests/mod.rs +++ b/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, - StampAt, HMAC_SIZE, MAGIC, + flags::FLAG_HMAC, flags::FLAG_OPEN, flags::FLAG_REPLY, verify_packet_hmac, Clock, + ReceivedStats, StampAt, HMAC_SIZE, MAGIC, }; use std::{thread, time::SystemTime}; diff --git a/crates/irtt-client/src/managed/blocking_tests.rs b/crates/irtt-client/src/managed/blocking_tests.rs index e3fe66d..d3df883 100644 --- a/crates/irtt-client/src/managed/blocking_tests.rs +++ b/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, - encode_open_reply, flags, EchoReply, OpenReply, ReceivedStats, StampAt, TimestampFields, + decode_request, encode_echo_reply, encode_open_reply, flags, DecodedRequestKind, EchoReply, + 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 = request.params; + let negotiated = match decode_request(&buffer[..open_len]).unwrap().kind { + 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]; - if let Ok(probe) = decode_echo_request(packet, &negotiated, None) { - worker_probe.set(); - let reply = encode_echo_reply( - &EchoReply { - flags: flags::FLAG_REPLY, - token: TOKEN, - sequence: probe.sequence, - recv_count: None, - recv_window: None, - timestamps: TimestampFields::default(), - payload: Vec::new(), - }, - &negotiated, - None, - ) - .unwrap(); - socket.send_to(&reply, packet_peer).unwrap(); - } else if decode_close_request(packet, None).is_ok() { - worker_close.set(); - return; + let Ok(request) = decode_request(&buffer[..len]) else { + continue; + }; + match request.kind { + DecodedRequestKind::Echo { sequence, .. } => { + worker_probe.set(); + let reply = encode_echo_reply( + &EchoReply { + flags: flags::FLAG_REPLY, + token: TOKEN, + sequence, + recv_count: None, + recv_window: None, + timestamps: TimestampFields::default(), + payload: Vec::new(), + }, + &negotiated, + None, + ) + .unwrap(); + socket.send_to(&reply, packet_peer).unwrap(); + } + DecodedRequestKind::Close { .. } => { + worker_close.set(); + return; + } + DecodedRequestKind::Open { .. } => {} } } }); diff --git a/crates/irtt-client/src/managed/tests.rs b/crates/irtt-client/src/managed/tests.rs index 0a52cee..284d0c2 100644 --- a/crates/irtt-client/src/managed/tests.rs +++ b/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, - encode_open_reply, flags, EchoReply, OpenReply, ReceivedStats, StampAt, TimestampFields, + decode_request, encode_echo_reply, encode_open_reply, flags, verify_packet_hmac, + 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> { + 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>) -> 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()) { - thread_records.lock().unwrap().push(PacketRecord { - kind: PacketKind::Probe, - at: Instant::now(), - }); - let (seen, ready) = &*thread_probe_seen; - *seen.lock().unwrap() = true; - ready.notify_all(); - if let ServerBehavior::DelayedEcho(delay) = behavior { - thread::sleep(delay); - } - if let ServerBehavior::DeferredBurst(count) = behavior { - if deferred_burst_sent { + match decode_server_request(packet, key.as_deref()) { + Some(DecodedRequestKind::Echo { sequence, .. }) => { + thread_records.lock().unwrap().push(PacketRecord { + kind: PacketKind::Probe, + at: Instant::now(), + }); + let (seen, ready) = &*thread_probe_seen; + *seen.lock().unwrap() = true; + ready.notify_all(); + if let ServerBehavior::DelayedEcho(delay) = behavior { + thread::sleep(delay); + } + 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; - 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 { - probe.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(); + if let Some(gate) = &reply_gate { + gate.arrive_and_wait(); + } + let peer_close = matches!(behavior, ServerBehavior::PeerClose); + let reply = encode_echo_reply( + &EchoReply { + flags: flags::FLAG_REPLY + | if peer_close { flags::FLAG_CLOSE } else { 0 }, + token: TOKEN, + 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; } - continue; } - if let Some(gate) = &reply_gate { - gate.arrive_and_wait(); - } - let peer_close = matches!(behavior, ServerBehavior::PeerClose); - let reply = encode_echo_reply( - &EchoReply { - flags: flags::FLAG_REPLY | if peer_close { flags::FLAG_CLOSE } else { 0 }, - 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 { + Some(DecodedRequestKind::Close { .. }) => { + thread_records.lock().unwrap().push(PacketRecord { + kind: PacketKind::Close, + at: Instant::now(), + }); return; } - } else if decode_close_request(packet, key.as_deref()).is_ok() { - thread_records.lock().unwrap().push(PacketRecord { - kind: PacketKind::Close, - at: Instant::now(), - }); - return; + Some(DecodedRequestKind::Open { .. }) | None => {} } } }); diff --git a/crates/irtt-client/src/session/machine.rs b/crates/irtt-client/src/session/machine.rs index fbf4db2..91c0255 100644 --- a/crates/irtt-client/src/session/machine.rs +++ b/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, - encode_echo_request, encode_open_request, flags, EchoReply, EchoRequest, OpenReply, - OpenRequest, Params, ServerFill, TimestampFields, PROTOCOL_VERSION, + decode_echo_reply, echo_packet_len, encode_request, flags, EchoReply, OpenReply, Params, + RequestToEncode, ServerFill, TimestampFields, PROTOCOL_VERSION, }; use crate::{ @@ -211,11 +210,13 @@ impl SessionMachine { pub(crate) fn prepare_open_request(&self) -> Result { self.ensure_connected()?; - let request = OpenRequest { - params: self.requested.clone(), - close: self.config.run_mode == RunMode::NoTest, - }; - let bytes = encode_open_request(&request, self.config.hmac_key.as_deref())?; + let bytes = encode_request( + RequestToEncode::Open { + params: &self.requested, + no_test: self.config.run_mode == RunMode::NoTest, + }, + 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( - &CloseRequest { token: reply.token }, + let bytes = encode_request( + 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, ClientError> { let session = self.open_session()?; - let request = EchoRequest { - token: session.token, - sequence: session.next_wire_seq, - payload: vec![], - }; - let bytes = encode_echo_request( - &request, - &session.negotiated.params, + let bytes = encode_request( + RequestToEncode::Echo { + token: session.token, + sequence: session.next_wire_seq, + params: &session.negotiated.params, + payload: &[], + }, 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( - &CloseRequest { + local_close_packet: encode_request( + 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( - &CloseRequest { + encode_request( + RequestToEncode::Close { token: 0x0102_0304_0506_0708, }, None, diff --git a/crates/irtt-client/tests/hmac.rs b/crates/irtt-client/tests/hmac.rs index 1ccd0a9..df65712 100644 --- a/crates/irtt-client/tests/hmac.rs +++ b/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, - bad_hmac: true - }] if *hmac == expected_hmac_flag - )); + hmac: expected_hmac_flag, + bad_hmac: expected_bad_hmac, + }] + ); server.join(); } } diff --git a/crates/irtt-client/tests/support/mod.rs b/crates/irtt-client/tests/support/mod.rs index 77f1487..1d1d592 100644 --- a/crates/irtt-client/tests/support/mod.rs +++ b/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, - verify_hmac, Clock, EchoReply, OpenReply, PacketLayout, Params, ProtoError, ReceivedStats, - ServerFill, StampAt, TimestampFields, HMAC_SIZE, MAGIC, PROTOCOL_VERSION, + compute_hmac_in_place, decode_request, echo_packet_len, encode_echo_reply, encode_open_reply, + flags, verify_packet_hmac, Clock, DecodedRequest, DecodedRequestKind, EchoReply, OpenReply, + 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>) -> 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, 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 - .get(3) - .is_some_and(|flags| flags & flags::FLAG_HMAC != 0); - match verify_hmac(&key, &request, HMAC_OFFSET) { + let hmac = decode_request(&request).is_ok_and(|request| request.hmac_present); + // A server with a key drops any request that fails authentication, + // whether it carried no HMAC field at all or a field signed with + // 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, wait: Duration) -> Fak pub fn start_hmac_close_server(params: Params, key: Vec) -> 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) -> FakeServer { pub fn start_bad_hmac_echo_reply_server(params: Params, key: Vec) -> 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) { - assert_eq!(&packet[..3], &MAGIC); - assert!(packet[3] & flags::FLAG_OPEN != 0); - let hmac = packet[3] & flags::FLAG_HMAC != 0; - assert_eq!(hmac, hmac_key.is_some()); +/// Admits a request the way a compliant server does: structural decode first, +/// then HMAC presence policy, then authentication. +fn admit_request<'a>(packet: &'a [u8], hmac_key: Option<&[u8]>) -> DecodedRequest<'a> { + let request = decode_request(packet).expect("fake peers only receive well-formed requests"); + 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); - assert_eq!(packet[3] & flags::FLAG_OPEN, 0); - assert_eq!(packet[3] & flags::FLAG_REPLY, 0); - assert_eq!(packet[3] & flags::FLAG_CLOSE, 0); - let hmac = packet[3] & flags::FLAG_HMAC != 0; - assert_eq!(hmac, hmac_key.is_some()); - if let Some(key) = hmac_key { - verify_hmac(key, packet, HMAC_OFFSET).unwrap(); - } - - let mut pos = 4 + if hmac { HMAC_SIZE } else { 0 }; - let token = u64::from_le_bytes(packet[pos..pos + 8].try_into().unwrap()); - pos += 8; - let sequence = u32::from_le_bytes(packet[pos..pos + 4].try_into().unwrap()); - - ServerObservation::Echo { - len: packet.len(), - hmac, - token, - sequence, + let request = admit_request(packet, hmac_key); + match request.kind { + DecodedRequestKind::Echo { + token, sequence, .. + } => ServerObservation::Echo { + len: packet.len(), + hmac: request.hmac_present, + token, + sequence, + }, + other => panic!("expected an echo request, got {other:?}"), } } fn observe_close_request(packet: &[u8], hmac_key: &[u8]) -> ServerObservation { - assert_eq!(&packet[..3], &MAGIC); - assert_eq!(packet[3] & flags::FLAG_CLOSE, flags::FLAG_CLOSE); - assert_eq!(packet[3] & flags::FLAG_HMAC, flags::FLAG_HMAC); - verify_hmac(hmac_key, packet, HMAC_OFFSET).unwrap(); - - 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 } + let request = admit_request(packet, Some(hmac_key)); + match request.kind { + DecodedRequestKind::Close { token } => ServerObservation::Close { hmac: true, token }, + other => panic!("expected a close request, got {other:?}"), + } } // irtt-proto has no encode_close_reply/CloseReply API yet, so this stays a diff --git a/crates/irtt-proto/src/close.rs b/crates/irtt-proto/src/close.rs deleted file mode 100644 index 9e031f0..0000000 --- a/crates/irtt-proto/src/close.rs +++ /dev/null @@ -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> { - 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 { - 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(); - } -} diff --git a/crates/irtt-proto/src/echo.rs b/crates/irtt-proto/src/echo.rs index f6e9a5f..bcedbbe 100644 --- a/crates/irtt-proto/src/echo.rs +++ b/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, -} - #[derive(Debug, Clone, PartialEq, Eq)] pub struct EchoReply { pub flags: u8, @@ -62,69 +55,6 @@ pub struct TimestampFields { pub send_mono: Option, } -pub fn encode_echo_request( - request: &EchoRequest, - params: &Params, - hmac_key: Option<&[u8]>, -) -> Result> { - 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 { - 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 -/// `allow_midpoint_compat` is set). +/// length rather than the exact negotiated length. /// /// 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( - packet: &[u8], - params: &Params, - layout: PacketLayout, - allow_midpoint_compat: bool, -) -> Result { +fn validate_echo_length(packet: &[u8], params: &Params, layout: PacketLayout) -> Result { 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) { - 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) -> 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) -> EchoRequest { - EchoRequest { - token: 0x7896_b6ab_8771_5213, - sequence: 9, - payload, + /// Zero-fills every negotiated reply field, matching what a peer that + /// reports no optional values would put on the wire. + fn push_zeroed_layout_tail(layout: PacketLayout, out: &mut Vec) { + 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]); } - } - - #[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(), - }, - ¶ms, - 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, ¶ms, None).unwrap(); - let payload_offset = PacketLayout::echo(false, ¶ms).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, ¶ms, None).unwrap(); - let payload_offset = PacketLayout::echo(false, ¶ms).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, ¶ms, 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, ¶ms, 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(), - }, - ¶ms, - 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] diff --git a/crates/irtt-proto/src/envelope.rs b/crates/irtt-proto/src/envelope.rs index cce9920..1b8a1b3 100644 --- a/crates/irtt-proto/src/envelope.rs +++ b/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 { + 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 { - let flags = validate_header(packet)?; - validate_packet_type(flags, rules)?; - check_hmac_presence(flags, hmac_key)?; - - Ok(Envelope { - flags, - body_offset: HEADER_SIZE + if has(flags, FLAG_HMAC) { HMAC_SIZE } else { 0 }, - }) + let envelope = decode_structural(packet)?; + require_flags(envelope.flags, rules)?; + check_hmac_presence(envelope.flags, hmac_key)?; + Ok(envelope) } -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> { + 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, hmac_key: Option<&[u8]>) -> Result 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, diff --git a/crates/irtt-proto/src/hmac.rs b/crates/irtt-proto/src/hmac.rs index 273ba3b..ca82fb8 100644 --- a/crates/irtt-proto/src/hmac.rs +++ b/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 } diff --git a/crates/irtt-proto/src/lib.rs b/crates/irtt-proto/src/lib.rs index 5922929..646c579 100644 --- a/crates/irtt-proto/src/lib.rs +++ b/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 -//! encoding and decoding, and optional HMAC placement, computation, and -//! verification helpers. +//! The crate also provides packet layout calculation, packet encoding and +//! decoding, and optional HMAC placement, computation, and verification +//! 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, decode_echo_request, encode_echo_reply, encode_echo_request, EchoReply, - EchoRequest, TimestampFields, -}; +pub use echo::{decode_echo_reply, encode_echo_reply, EchoReply, 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::{ - decode_open_reply, decode_open_request, encode_open_reply, encode_open_request, OpenReply, - OpenRequest, -}; +pub use open::{decode_open_reply, encode_open_reply, OpenReply}; 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; diff --git a/crates/irtt-proto/src/open.rs b/crates/irtt-proto/src/open.rs index d48f041..84e4791 100644 --- a/crates/irtt-proto/src/open.rs +++ b/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> { - 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(¶ms); - envelope::finish(out, hmac_key) -} - -pub fn decode_open_request(packet: &[u8], hmac_key: Option<&[u8]>) -> Result { - 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> { 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]; diff --git a/crates/irtt-proto/src/request.rs b/crates/irtt-proto/src/request.rs new file mode 100644 index 0000000..0e2c670 --- /dev/null +++ b/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> { + 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> { + 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> { + 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> { + 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> { + 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()) +} diff --git a/crates/irtt-proto/tests/directional_codecs.rs b/crates/irtt-proto/tests/directional_codecs.rs index 58587f7..b0cbd71 100644 --- a/crates/irtt-proto/tests/directional_codecs.rs +++ b/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_open_request, echo_packet_len, encode_close_request, encode_echo_reply, - encode_echo_request, encode_open_reply, encode_open_request, hmac::compute_hmac_in_place, - Clock, CloseRequest, EchoReply, EchoRequest, OpenReply, OpenRequest, PacketLayout, Params, - ProtoError, ReceivedStats, StampAt, TimestampFields, FLAG_CLOSE, FLAG_HMAC, FLAG_OPEN, - FLAG_REPLY, MAGIC, + decode_echo_reply, decode_open_reply, decode_request, echo_packet_len, encode_echo_reply, + encode_open_reply, encode_request, hmac::compute_hmac_in_place, Clock, DecodedRequestKind, + EchoReply, OpenReply, PacketLayout, Params, ProtoError, ReceivedStats, RequestToEncode, + 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) -> EchoRequest { - EchoRequest { +fn echo_request<'a>(params: &'a Params, payload: &'a [u8]) -> RequestToEncode<'a> { + 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(¶ms, FLAG_REPLY), ¶ms, None).unwrap(); - assert_eq!( - decode_echo_request(&echo_reply_packet, ¶ms, 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(); - assert_eq!( - decode_echo_request(&close_packet, ¶ms, None), - Err(ProtoError::UnexpectedFlag(FLAG_CLOSE)) - ); +/// Every packet the reply encoders produce must be rejected by the inbound +/// request decoder: a server never answers a datagram carrying `FLAG_REPLY`. +#[test] +fn encoded_replies_are_never_admitted_as_inbound_requests() { + let params = Params::default(); - let no_test_packet = encode_open_request( - &OpenRequest { - params, - close: true, - }, - None, - ) - .unwrap(); - assert_eq!( - decode_close_request(&no_test_packet, None), - Err(ProtoError::UnexpectedFlag(FLAG_OPEN)) - ); + for reply_flags in [FLAG_OPEN | FLAG_REPLY, FLAG_OPEN | FLAG_REPLY | FLAG_CLOSE] { + let packet = encode_open_reply( + &OpenReply { + flags: reply_flags, + token: if reply_flags & FLAG_CLOSE == 0 { + TOKEN + } else { + 0 + }, + params: params.clone(), + }, + 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(¶ms, reply_flags), ¶ms, None).unwrap(); + assert_eq!( + decode_request(&packet), + Err(ProtoError::UnexpectedFlag(FLAG_REPLY)) + ); + } } #[test] -fn codec_specific_malformed_bodies_and_exact_lengths_are_rejected() { - 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) - ); - +fn reply_bodies_are_decoded_semantically_and_reject_malformed_params() { 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()), ¶ms, None).unwrap(); - assert_eq!( - decode_echo_request(&echo_packet[..19], ¶ms, 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, ¶ms, 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_echo_request(&request, ¶ms, None).unwrap(); + let packet = encode_request(echo_request(¶ms, &[1, 2]), None).unwrap(); assert_eq!(&packet[16..], &[1, 2, 0, 0]); - let decoded = decode_echo_request(&packet, ¶ms, None).unwrap(); - assert_eq!(decoded.payload, vec![1, 2, 0, 0]); assert_eq!( - encode_echo_request(&decoded, ¶ms, None).unwrap(), - packet - ); - assert_eq!( - encode_echo_request(&echo_request(vec![0; 5]), ¶ms, None), + encode_request(echo_request(¶ms, &[0; 5]), 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()), ¶ms, None).unwrap(); + let packet = encode_request(echo_request(¶ms, &[]), 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, ¶ms, None), - Err(ProtoError::PacketLengthMismatch { - expected: packet.len(), - actual: too_long.len(), - }) + request.kind, + DecodedRequestKind::Echo { + token: TOKEN, + sequence: 17, + tail: &too_long[16..], + } ); } diff --git a/crates/irtt-proto/tests/request_codec.rs b/crates/irtt-proto/tests/request_codec.rs new file mode 100644 index 0000000..daa353d --- /dev/null +++ b/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 { + 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 { + raw(flags, &[], if flags & FLAG_HMAC != 0 { 20 } else { 4 }) +} + +fn token_and_sequence() -> Vec { + 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: ¶ms(), + 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: ¶ms, + payload: &[], + }, + None, + ), + Err(ProtoError::NegativePacketLength { length: -1 }) + ); +} diff --git a/crates/irtt-proto/tests/vectors.rs b/crates/irtt-proto/tests/vectors.rs index e97f2bf..b9febd4 100644 --- a/crates/irtt-proto/tests/vectors.rs +++ b/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_open_request, encode_close_request, encode_echo_reply, encode_echo_request, - encode_open_reply, encode_open_request, verify_hmac, Clock, CloseRequest, EchoRequest, - OpenRequest, Params, ProtoError, ReceivedStats, StampAt, FLAG_REPLY, + decode_echo_reply, decode_open_reply, decode_request, encode_echo_reply, encode_open_reply, + encode_request, verify_hmac, verify_packet_hmac, Clock, DecodedRequestKind, Params, ProtoError, + ReceivedStats, RequestToEncode, 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 { 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( - &OpenRequest { - params: default_params_3s(), - close: false, + let packet = encode_request( + RequestToEncode::Open { + params: &default_params_3s(), + no_test: false, }, None, ) .unwrap(); assert_eq!(packet, expected); - assert_eq!( - decode_open_request(&expected, None), - Ok(OpenRequest { - params: default_params_3s(), - close: false, - }) - ); + assert_open_request(&expected, &default_params_3s(), false, 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( - &EchoRequest { + let packet = encode_request( + 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!( - decode_echo_request(&expected, &default_params_3s(), None), - Ok(EchoRequest { - token: 0x7896_b6ab_8771_5213, - sequence: 0, - payload: Vec::new(), - }) - ); + assert_echo_request(&expected, 0x7896_b6ab_8771_5213, 0, false); } #[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( - &CloseRequest { + let packet = encode_request( + RequestToEncode::Close { token: 0x7896_b6ab_8771_5213, }, None, ) .unwrap(); assert_eq!(packet, expected); - assert_eq!( - decode_close_request(&expected, None), - Ok(CloseRequest { - token: 0x7896_b6ab_8771_5213, - }) - ); + assert_close_request(&expected, 0x7896_b6ab_8771_5213, false); } #[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( - &OpenRequest { - params: hmac_params_2s(), - close: false, + let packet = encode_request( + RequestToEncode::Open { + params: &hmac_params_2s(), + no_test: false, }, Some(b"testkey"), ) .unwrap(); assert_eq!(packet, expected); verify_hmac(b"testkey", &packet, 4).unwrap(); - assert_eq!( - decode_open_request(&expected, Some(b"testkey")), - Ok(OpenRequest { - params: hmac_params_2s(), - close: false, - }) - ); + verify_packet_hmac(b"testkey", &packet).unwrap(); + assert_open_request(&expected, &hmac_params_2s(), false, true); } #[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( - &EchoRequest { + let packet = encode_request( + 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!( - decode_echo_request(&expected, &default_params_3s(), Some(b"testkey")), - Ok(EchoRequest { - token: 0x4387_e9eb_5d3f_ca59, - sequence: 0, - payload: Vec::new(), - }) - ); + verify_packet_hmac(b"testkey", &packet).unwrap(); + assert_echo_request(&expected, 0x4387_e9eb_5d3f_ca59, 0, true); } #[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( - &CloseRequest { + let packet = encode_request( + RequestToEncode::Close { token: 0x4387_e9eb_5d3f_ca59, }, Some(b"testkey"), ) .unwrap(); assert_eq!(packet, expected); - assert_eq!( - decode_close_request(&expected, Some(b"testkey")), - Ok(CloseRequest { - token: 0x4387_e9eb_5d3f_ca59, - }) - ); + verify_packet_hmac(b"testkey", &packet).unwrap(); + assert_close_request(&expected, 0x4387_e9eb_5d3f_ca59, true); } #[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( - &OpenRequest { - params: default_params_60s(), - close: true, + let packet = encode_request( + RequestToEncode::Open { + params: &default_params_60s(), + no_test: true, }, None, ) .unwrap(); assert_eq!(packet, expected); - assert_eq!( - decode_open_request(&expected, None), - Ok(OpenRequest { - params: default_params_60s(), - close: true, - }) - ); + assert_open_request(&expected, &default_params_60s(), true, false); } #[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( - &EchoRequest { + let packet = encode_request( + 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!( - decode_echo_request(&expected, &Params::default(), None), - Ok(EchoRequest { - token: 0xa031_6fa2_5c61_154e, - sequence: 0, - payload: Vec::new(), - }) - ); + assert_echo_request(&expected, 0xa031_6fa2_5c61_154e, 0, false); } #[test] @@ -296,33 +295,20 @@ fn spec_18_1_hmac_echo_request_fixture() { clock: Clock::Both, ..Params::default() }; - let packet = encode_echo_request( - &EchoRequest { + let key = [0x3c, 0x68, 0x1d, 0x39, 0x41, 0x1d, 0x72, 0x43]; + let packet = encode_request( + RequestToEncode::Echo { token: 0x886b_c9a7_22b3_3eea, sequence: 0x6fe2_a1bb, - payload: [0xff, 0xfe, 0xfd, 0xfc].repeat(4), + params: ¶ms, + payload: &[0xff, 0xfe, 0xfd, 0xfc].repeat(4), }, - ¶ms, - Some(&[0x3c, 0x68, 0x1d, 0x39, 0x41, 0x1d, 0x72, 0x43]), + Some(&key), ) .unwrap(); assert_eq!(packet, expected); - assert_eq!( - decode_echo_request( - &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) - ); + verify_packet_hmac(&key, &packet).unwrap(); + assert_echo_request(&expected, 0x886b_c9a7_22b3_3eea, 0x6fe2_a1bb, true); } #[test]