Merge pull request #41 from Lochnair/feat/directional-codecs

feat(proto): add directional packet codecs

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

@@ -218,10 +218,9 @@ impl SessionMachine {
         let request = EchoRequest {
             token: session.token,
             sequence: wire_seq,
-            params: session.negotiated.params.clone(),
             payload: vec![],
         };
-        let packet = encode_echo_request(&request, hmac_key)?;
+        let packet = encode_echo_request(&request, &session.negotiated.params, hmac_key)?;
         let send_result = send(&packet)?;
 
         let pending = PendingProbe {

crates/irtt-proto/src/close.rs

@@ -1,6 +1,8 @@
 use crate::{
-    flags::{FLAG_CLOSE, FLAG_HMAC},
-    hmac, write_header, Result, HMAC_SIZE,
+    envelope::{self, FlagRule},
+    flags::{has, FLAG_CLOSE, FLAG_OPEN, FLAG_REPLY},
+    layout::PacketLayout,
+    ProtoError, Result, TOKEN_SIZE,
 };
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -9,27 +11,53 @@ pub struct CloseRequest {
 }
 
 pub fn encode_close_request(request: &CloseRequest, hmac_key: Option<&[u8]>) -> Result<Vec<u8>> {
-    let mut flags = FLAG_CLOSE;
-    if hmac_key.is_some() {
-        flags |= FLAG_HMAC;
+    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)
 }
 
-    let mut out = Vec::new();
-    write_header(&mut out, flags);
-    if hmac_key.is_some() {
-        out.extend_from_slice(&[0; HMAC_SIZE]);
+pub fn decode_close_request(packet: &[u8], hmac_key: Option<&[u8]>) -> Result<CloseRequest> {
+    let envelope = envelope::decode(
+        packet,
+        hmac_key,
+        &[
+            FlagRule::Require(FLAG_CLOSE),
+            FlagRule::Reject(FLAG_OPEN),
+            FlagRule::Reject(FLAG_REPLY),
+        ],
+    )?;
+    let expected_len =
+        PacketLayout::close_request(has(envelope.flags, crate::FLAG_HMAC)).header_len();
+    if packet.len() != expected_len {
+        return Err(ProtoError::PacketLengthMismatch {
+            expected: expected_len,
+            actual: packet.len(),
+        });
     }
-    out.extend_from_slice(&request.token.to_le_bytes());
-    if let Some(key) = hmac_key {
-        hmac::compute_hmac_in_place(key, &mut out, hmac::hmac_offset())?;
-    }
-    Ok(out)
+    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};
+    use crate::{hmac, FLAG_HMAC, HMAC_SIZE};
 
     #[test]
     fn hmac_close_request_places_token_after_hmac() {

crates/irtt-proto/src/echo.rs

@@ -1,18 +1,15 @@
 use crate::{
-    flags::{has, FLAG_HMAC, FLAG_OPEN, FLAG_REPLY},
-    hmac,
+    envelope::{self, FlagRule},
+    flags::{FLAG_CLOSE, FLAG_OPEN, FLAG_REPLY},
     layout::{echo_packet_len, PacketLayout},
-    open::{check_hmac_presence, reject},
     params::Params,
-    validate_header, write_header, ProtoError, Result, HEADER_SIZE, HMAC_SIZE, RECV_COUNT_SIZE,
-    RECV_WINDOW_SIZE, SEQ_SIZE, TIMESTAMP_SIZE, TOKEN_SIZE,
+    ProtoError, Result, RECV_COUNT_SIZE, RECV_WINDOW_SIZE, SEQ_SIZE, TIMESTAMP_SIZE, TOKEN_SIZE,
 };
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct EchoRequest {
     pub token: u64,
     pub sequence: u32,
-    pub params: Params,
     pub payload: Vec<u8>,
 }
 
@@ -37,13 +34,13 @@ pub struct TimestampFields {
     pub send_mono: Option<i64>,
 }
 
-pub fn encode_echo_request(request: &EchoRequest, hmac_key: Option<&[u8]>) -> Result<Vec<u8>> {
-    let mut flags = 0;
-    if hmac_key.is_some() {
-        flags |= FLAG_HMAC;
-    }
-    let layout = PacketLayout::echo(hmac_key.is_some(), &request.params);
-    let len = echo_packet_len(hmac_key.is_some(), &request.params)?;
+pub fn encode_echo_request(
+    request: &EchoRequest,
+    params: &Params,
+    hmac_key: Option<&[u8]>,
+) -> Result<Vec<u8>> {
+    let layout = PacketLayout::echo(hmac_key.is_some(), params);
+    let len = echo_packet_len(hmac_key.is_some(), params)?;
     let payload_offset = layout.header_len();
     let available_payload_len = len.saturating_sub(payload_offset);
     if request.payload.len() > available_payload_len {
@@ -52,21 +49,83 @@ pub fn encode_echo_request(request: &EchoRequest, hmac_key: Option<&[u8]>) -> Re
             provided: request.payload.len(),
         });
     }
-    let mut out = Vec::with_capacity(len);
-    write_header(&mut out, flags);
-    if hmac_key.is_some() {
-        out.extend_from_slice(&[0; HMAC_SIZE]);
-    }
+    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);
 
-    if let Some(key) = hmac_key {
-        hmac::compute_hmac_in_place(key, &mut out, hmac::hmac_offset())?;
+    envelope::finish(out, hmac_key)
 }
-    Ok(out)
+
+pub fn decode_echo_request(
+    packet: &[u8],
+    params: &Params,
+    hmac_key: Option<&[u8]>,
+) -> Result<EchoRequest> {
+    let envelope = envelope::decode(
+        packet,
+        hmac_key,
+        &[
+            FlagRule::Reject(FLAG_OPEN),
+            FlagRule::Reject(FLAG_REPLY),
+            FlagRule::Reject(FLAG_CLOSE),
+        ],
+    )?;
+    let layout = PacketLayout::echo(hmac_key.is_some(), params);
+    validate_echo_length(packet, params, layout)?;
+    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,
+    hmac_key: Option<&[u8]>,
+) -> Result<Vec<u8>> {
+    let layout = PacketLayout::echo(hmac_key.is_some(), params);
+    let len = echo_packet_len(hmac_key.is_some(), params)?;
+    let payload_offset = layout.header_len();
+    let available_payload_len = len.saturating_sub(payload_offset);
+    if reply.payload.len() > available_payload_len {
+        return Err(ProtoError::PayloadTooLarge {
+            available: available_payload_len,
+            provided: reply.payload.len(),
+        });
+    }
+
+    let mut out = envelope::begin(
+        reply.flags,
+        hmac_key,
+        &[FlagRule::Reject(FLAG_OPEN), FlagRule::Require(FLAG_REPLY)],
+        len,
+    )?;
+    out.extend_from_slice(&reply.token.to_le_bytes());
+    out.extend_from_slice(&reply.sequence.to_le_bytes());
+    push_echo_reply_tail(reply, layout, &mut out)?;
+    out.resize(len, 0);
+    out[payload_offset..payload_offset + reply.payload.len()].copy_from_slice(&reply.payload);
+
+    envelope::finish(out, hmac_key)
 }
 
 pub fn decode_echo_reply(
@@ -74,35 +133,17 @@ pub fn decode_echo_reply(
     params: &Params,
     hmac_key: Option<&[u8]>,
 ) -> Result<EchoReply> {
-    let flags = validate_header(packet)?;
-    reject(flags, FLAG_OPEN)?;
-    crate::open::require(flags, FLAG_REPLY)?;
-    check_hmac_presence(flags, hmac_key)?;
+    let envelope = envelope::decode(
+        packet,
+        hmac_key,
+        &[FlagRule::Reject(FLAG_OPEN), FlagRule::Require(FLAG_REPLY)],
+    )?;
+    let layout = PacketLayout::echo(hmac_key.is_some(), params);
+    validate_echo_length(packet, params, layout)?;
+    envelope::verify(packet, hmac_key)?;
 
-    let layout = PacketLayout::echo(has(flags, FLAG_HMAC), params);
     let header_len = layout.header_len();
-    if packet.len() < header_len {
-        return Err(ProtoError::PacketTooShort {
-            needed: header_len,
-            actual: packet.len(),
-        });
-    }
-    let expected_len = echo_packet_len(has(flags, FLAG_HMAC), params)?;
-    if packet.len() != expected_len {
-        return Err(ProtoError::PacketLengthMismatch {
-            expected: expected_len,
-            actual: packet.len(),
-        });
-    }
-
-    if let Some(key) = hmac_key {
-        hmac::verify_hmac(key, packet, hmac::hmac_offset())?;
-    }
-
-    let mut pos = HEADER_SIZE;
-    if layout.hmac {
-        pos += HMAC_SIZE;
-    }
+    let mut pos = envelope.body_offset;
     let token = read_u64(packet, &mut pos);
     let sequence = read_u32(packet, &mut pos);
     let recv_count = layout.recv_count.then(|| read_u32(packet, &mut pos));
@@ -117,7 +158,7 @@ pub fn decode_echo_reply(
     };
 
     Ok(EchoReply {
-        flags,
+        flags: envelope.flags,
         token,
         sequence,
         recv_count,
@@ -127,6 +168,24 @@ pub fn decode_echo_reply(
     })
 }
 
+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 {
+            needed: header_len,
+            actual: packet.len(),
+        });
+    }
+    let expected_len = echo_packet_len(layout.hmac, params)?;
+    if packet.len() != expected_len {
+        return Err(ProtoError::PacketLengthMismatch {
+            expected: expected_len,
+            actual: packet.len(),
+        });
+    }
+    Ok(())
+}
+
 fn push_zeroed_layout_tail(layout: PacketLayout, out: &mut Vec<u8>) {
     if layout.recv_count {
         out.extend_from_slice(&[0; RECV_COUNT_SIZE]);
@@ -139,6 +198,47 @@ fn push_zeroed_layout_tail(layout: PacketLayout, out: &mut Vec<u8>) {
     }
 }
 
+fn push_echo_reply_tail(reply: &EchoReply, layout: PacketLayout, out: &mut Vec<u8>) -> Result<()> {
+    if let Some(value) = field_value(layout.recv_count, reply.recv_count, "recv_count")? {
+        out.extend_from_slice(&value.to_le_bytes());
+    }
+    if let Some(value) = field_value(layout.recv_window, reply.recv_window, "recv_window")? {
+        out.extend_from_slice(&value.to_le_bytes());
+    }
+
+    let timestamps = &reply.timestamps;
+    for (present, value, name) in [
+        (layout.recv_wall, timestamps.recv_wall, "recv_wall"),
+        (layout.recv_mono, timestamps.recv_mono, "recv_mono"),
+        (
+            layout.midpoint_wall,
+            timestamps.midpoint_wall,
+            "midpoint_wall",
+        ),
+        (
+            layout.midpoint_mono,
+            timestamps.midpoint_mono,
+            "midpoint_mono",
+        ),
+        (layout.send_wall, timestamps.send_wall, "send_wall"),
+        (layout.send_mono, timestamps.send_mono, "send_mono"),
+    ] {
+        if let Some(value) = field_value(present, value, name)? {
+            out.extend_from_slice(&value.to_le_bytes());
+        }
+    }
+    Ok(())
+}
+
+fn field_value<T: Copy>(present: bool, value: Option<T>, name: &'static str) -> Result<Option<T>> {
+    match (present, value) {
+        (true, Some(value)) => Ok(Some(value)),
+        (true, None) => Err(ProtoError::MissingField(name)),
+        (false, Some(_)) => Err(ProtoError::UnexpectedField(name)),
+        (false, None) => Ok(None),
+    }
+}
+
 fn read_u32(packet: &[u8], pos: &mut usize) -> u32 {
     let value = u32::from_le_bytes(packet[*pos..*pos + SEQ_SIZE].try_into().unwrap());
     *pos += SEQ_SIZE;
@@ -160,7 +260,11 @@ fn read_i64(packet: &[u8], pos: &mut usize) -> i64 {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::params::{Clock, ReceivedStats, StampAt};
+    use crate::{
+        hmac,
+        params::{Clock, ReceivedStats, StampAt},
+        write_header, FLAG_HMAC, HMAC_SIZE,
+    };
 
     fn default_params() -> Params {
         Params {
@@ -179,24 +283,24 @@ mod tests {
         }
     }
 
-    fn echo_request_with_payload(payload_space: usize, payload: Vec<u8>) -> EchoRequest {
+    fn echo_request_with_payload(payload: Vec<u8>) -> EchoRequest {
         EchoRequest {
             token: 0x7896_b6ab_8771_5213,
             sequence: 9,
-            params: params_with_payload_space(payload_space),
             payload,
         }
     }
 
     #[test]
     fn echo_request_encodes_default_placeholders() {
+        let params = default_params();
         let packet = encode_echo_request(
             &EchoRequest {
                 token: 0x7896_b6ab_8771_5213,
                 sequence: 0,
-                params: default_params(),
                 payload: Vec::new(),
             },
+            &params,
             None,
         )
         .unwrap();
@@ -208,29 +312,32 @@ mod tests {
 
     #[test]
     fn echo_request_encodes_exact_fit_payload() {
-        let request = echo_request_with_payload(4, vec![1, 2, 3, 4]);
-        let packet = encode_echo_request(&request, None).unwrap();
-        let payload_offset = PacketLayout::echo(false, &request.params).header_len();
+        let params = params_with_payload_space(4);
+        let request = echo_request_with_payload(vec![1, 2, 3, 4]);
+        let packet = encode_echo_request(&request, &params, None).unwrap();
+        let payload_offset = PacketLayout::echo(false, &params).header_len();
 
         assert_eq!(&packet[payload_offset..], &[1, 2, 3, 4]);
     }
 
     #[test]
     fn echo_request_encodes_shorter_payload_and_zero_fills_remainder() {
-        let request = echo_request_with_payload(4, vec![1, 2]);
-        let packet = encode_echo_request(&request, None).unwrap();
-        let payload_offset = PacketLayout::echo(false, &request.params).header_len();
+        let params = params_with_payload_space(4);
+        let request = echo_request_with_payload(vec![1, 2]);
+        let packet = encode_echo_request(&request, &params, None).unwrap();
+        let payload_offset = PacketLayout::echo(false, &params).header_len();
 
         assert_eq!(&packet[payload_offset..], &[1, 2, 0, 0]);
     }
 
     #[test]
     fn echo_request_rejects_oversized_payload() {
-        let request = echo_request_with_payload(4, vec![1, 2, 3, 4, 5]);
+        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, None),
+            encode_echo_request(&request, &params, None),
             Err(ProtoError::PayloadTooLarge {
                 available: 4,
                 provided: 5,
@@ -241,24 +348,28 @@ mod tests {
 
     #[test]
     fn echo_request_rejects_negative_requested_length() {
-        let mut request = echo_request_with_payload(0, Vec::new());
-        request.params.length = -1;
+        let request = echo_request_with_payload(Vec::new());
+        let params = Params {
+            length: -1,
+            ..Params::default()
+        };
 
         assert_eq!(
-            encode_echo_request(&request, None),
+            encode_echo_request(&request, &params, None),
             Err(ProtoError::NegativePacketLength { length: -1 })
         );
     }
 
     #[test]
     fn hmac_echo_request_places_token_and_sequence_after_hmac() {
+        let params = default_params();
         let packet = encode_echo_request(
             &EchoRequest {
                 token: 0x7896_b6ab_8771_5213,
                 sequence: 9,
-                params: default_params(),
                 payload: Vec::new(),
             },
+            &params,
             Some(b"testkey"),
         )
         .unwrap();

crates/irtt-proto/src/envelope.rs

@@ -0,0 +1,180 @@
+use crate::{
+    flags::{self, has, FLAG_HMAC},
+    hmac, validate_header, write_header, ProtoError, Result, HEADER_SIZE, HMAC_SIZE,
+};
+
+#[derive(Debug, Clone, Copy)]
+pub(crate) enum FlagRule {
+    Require(u8),
+    Reject(u8),
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub(crate) struct Envelope {
+    pub(crate) flags: u8,
+    pub(crate) body_offset: usize,
+}
+
+pub(crate) fn decode(
+    packet: &[u8],
+    hmac_key: Option<&[u8]>,
+    rules: &[FlagRule],
+) -> Result<Envelope> {
+    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 },
+    })
+}
+
+pub(crate) fn begin(
+    flags: u8,
+    hmac_key: Option<&[u8]>,
+    rules: &[FlagRule],
+    capacity: usize,
+) -> Result<Vec<u8>> {
+    // The encoder key is authoritative: authenticated encoders set FLAG_HMAC,
+    // 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)?;
+
+    let minimum_capacity = HEADER_SIZE + if hmac_key.is_some() { HMAC_SIZE } else { 0 };
+    let mut out = Vec::with_capacity(capacity.max(minimum_capacity));
+    write_header(&mut out, flags);
+    if hmac_key.is_some() {
+        out.extend_from_slice(&[0; HMAC_SIZE]);
+    }
+    Ok(out)
+}
+
+pub(crate) fn verify(packet: &[u8], hmac_key: Option<&[u8]>) -> Result<()> {
+    if let Some(key) = hmac_key {
+        hmac::verify_hmac(key, packet, hmac::hmac_offset())?;
+    }
+    Ok(())
+}
+
+pub(crate) fn finish(mut packet: Vec<u8>, hmac_key: Option<&[u8]>) -> Result<Vec<u8>> {
+    if let Some(key) = hmac_key {
+        hmac::compute_hmac_in_place(key, &mut packet, hmac::hmac_offset())?;
+    }
+    Ok(packet)
+}
+
+fn validate_packet_type(flags: u8, rules: &[FlagRule]) -> Result<()> {
+    for rule in rules {
+        match *rule {
+            FlagRule::Require(flag) if !has(flags, flag) => {
+                return Err(ProtoError::MissingFlag(flag));
+            }
+            FlagRule::Reject(flag) if has(flags, flag) => {
+                return Err(ProtoError::UnexpectedFlag(flag));
+            }
+            FlagRule::Require(_) | FlagRule::Reject(_) => {}
+        }
+    }
+    Ok(())
+}
+
+fn check_hmac_presence(flags: u8, hmac_key: Option<&[u8]>) -> Result<()> {
+    if has(flags, FLAG_HMAC) == hmac_key.is_some() {
+        Ok(())
+    } else {
+        Err(ProtoError::HmacPresenceMismatch)
+    }
+}
+
+fn with_hmac_flag(flags: u8, authenticated: bool) -> u8 {
+    if authenticated {
+        flags | FLAG_HMAC
+    } else {
+        flags & !FLAG_HMAC
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::{
+        flags::{FLAG_OPEN, FLAG_REPLY},
+        MAGIC,
+    };
+
+    const KEY: &[u8] = b"testkey";
+    const OPEN_REQUEST_RULES: &[FlagRule] =
+        &[FlagRule::Require(FLAG_OPEN), FlagRule::Reject(FLAG_REPLY)];
+
+    fn header(flags: u8) -> Vec<u8> {
+        let mut packet = MAGIC.to_vec();
+        packet.push(flags);
+        packet
+    }
+
+    #[test]
+    fn decode_rejects_bad_magic_and_reserved_flags() {
+        assert_eq!(
+            decode(&[0, MAGIC[1], MAGIC[2], FLAG_OPEN], None, &[]),
+            Err(ProtoError::BadMagic)
+        );
+        assert_eq!(
+            decode(&header(FLAG_OPEN | 0x10), None, &[]),
+            Err(ProtoError::ReservedFlags(0x10))
+        );
+    }
+
+    #[test]
+    fn decode_enforces_required_and_rejected_flags() {
+        assert_eq!(
+            decode(&header(0), None, OPEN_REQUEST_RULES),
+            Err(ProtoError::MissingFlag(FLAG_OPEN))
+        );
+        assert_eq!(
+            decode(&header(FLAG_OPEN | FLAG_REPLY), None, OPEN_REQUEST_RULES),
+            Err(ProtoError::UnexpectedFlag(FLAG_REPLY))
+        );
+    }
+
+    #[test]
+    fn decode_rejects_hmac_presence_mismatch() {
+        let mut authenticated = header(FLAG_OPEN | FLAG_HMAC);
+        authenticated.extend_from_slice(&[0; HMAC_SIZE]);
+        assert_eq!(
+            decode(&authenticated, None, OPEN_REQUEST_RULES),
+            Err(ProtoError::HmacPresenceMismatch)
+        );
+        assert_eq!(
+            decode(&header(FLAG_OPEN), Some(KEY), OPEN_REQUEST_RULES),
+            Err(ProtoError::HmacPresenceMismatch)
+        );
+    }
+
+    #[test]
+    fn verify_rejects_bad_hmac() {
+        let mut packet = begin(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;
+
+        decode(&packet, Some(KEY), OPEN_REQUEST_RULES).unwrap();
+        assert_eq!(verify(&packet, Some(KEY)), Err(ProtoError::BadHmac));
+    }
+
+    #[test]
+    fn encoder_key_authoritatively_normalizes_hmac_flag() {
+        let plain = begin(FLAG_OPEN | FLAG_HMAC, None, OPEN_REQUEST_RULES, HEADER_SIZE).unwrap();
+        assert_eq!(plain[3], FLAG_OPEN);
+
+        let authenticated = begin(
+            FLAG_OPEN,
+            Some(KEY),
+            OPEN_REQUEST_RULES,
+            HEADER_SIZE + HMAC_SIZE,
+        )
+        .unwrap();
+        assert_eq!(authenticated[3], FLAG_OPEN | FLAG_HMAC);
+    }
+}

crates/irtt-proto/src/error.rs

@@ -40,4 +40,8 @@ pub enum ProtoError {
     MalformedParams,
     #[error("payload is too large: {provided} bytes provided, {available} bytes available")]
     PayloadTooLarge { available: usize, provided: usize },
+    #[error("required packet field {0} is missing")]
+    MissingField(&'static str),
+    #[error("packet field {0} is not present in the negotiated layout")]
+    UnexpectedField(&'static str),
 }

crates/irtt-proto/src/lib.rs

@@ -15,11 +15,14 @@
 //! The crate also provides packet layout calculation, open/echo/close 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.
 //!
 #![forbid(unsafe_code)]
 
 pub mod close;
 pub mod echo;
+mod envelope;
 pub mod error;
 pub mod flags;
 pub mod hmac;
@@ -28,13 +31,19 @@ pub mod open;
 pub mod params;
 pub mod varint;
 
-pub use close::{encode_close_request, CloseRequest};
-pub use echo::{decode_echo_reply, encode_echo_request, EchoReply, EchoRequest, TimestampFields};
+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 error::{ProtoError, Result};
 pub use flags::*;
 pub use hmac::{compute_hmac, compute_hmac_in_place, verify_hmac};
 pub use layout::{echo_header_len, echo_packet_len, PacketLayout};
-pub use open::{decode_open_reply, encode_open_request, OpenReply, OpenRequest};
+pub use open::{
+    decode_open_reply, decode_open_request, encode_open_reply, encode_open_request, OpenReply,
+    OpenRequest,
+};
 pub use params::{Clock, Params, ReceivedStats, ServerFill, StampAt, MAX_SERVER_FILL_BYTES};
 
 pub const MAGIC: [u8; 3] = [0x14, 0xA7, 0x5B];

crates/irtt-proto/src/open.rs

@@ -1,8 +1,9 @@
 use crate::{
-    flags::{has, FLAG_CLOSE, FLAG_HMAC, FLAG_OPEN, FLAG_REPLY},
-    hmac,
+    envelope::{self, FlagRule},
+    flags::{has, FLAG_CLOSE, FLAG_OPEN, FLAG_REPLY},
+    layout::PacketLayout,
     params::Params,
-    validate_header, write_header, ProtoError, Result, HEADER_SIZE, HMAC_SIZE, TOKEN_SIZE,
+    ProtoError, Result, TOKEN_SIZE,
 };
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -23,36 +24,57 @@ pub fn encode_open_request(request: &OpenRequest, hmac_key: Option<&[u8]>) -> Re
     if request.close {
         flags |= FLAG_CLOSE;
     }
-    if hmac_key.is_some() {
-        flags |= FLAG_HMAC;
+    let params = request.params.encode();
+    let mut out = envelope::begin(
+        flags,
+        hmac_key,
+        &[FlagRule::Require(FLAG_OPEN), FlagRule::Reject(FLAG_REPLY)],
+        PacketLayout::open_request(hmac_key.is_some()).header_len() + params.len(),
+    )?;
+    out.extend_from_slice(&params);
+    envelope::finish(out, hmac_key)
 }
 
-    let mut out = Vec::new();
-    write_header(&mut out, flags);
-    if hmac_key.is_some() {
-        out.extend_from_slice(&[0; HMAC_SIZE]);
-    }
-    out.extend_from_slice(&request.params.encode());
+pub fn decode_open_request(packet: &[u8], hmac_key: Option<&[u8]>) -> Result<OpenRequest> {
+    let envelope = envelope::decode(
+        packet,
+        hmac_key,
+        &[FlagRule::Require(FLAG_OPEN), FlagRule::Reject(FLAG_REPLY)],
+    )?;
+    envelope::verify(packet, hmac_key)?;
 
-    if let Some(key) = hmac_key {
-        hmac::compute_hmac_in_place(key, &mut out, hmac::hmac_offset())?;
+    Ok(OpenRequest {
+        params: Params::decode(&packet[envelope.body_offset..])?,
+        close: has(envelope.flags, FLAG_CLOSE),
+    })
 }
-    Ok(out)
+
+pub fn encode_open_reply(reply: &OpenReply, hmac_key: Option<&[u8]>) -> Result<Vec<u8>> {
+    let params = reply.params.encode();
+    let mut out = envelope::begin(
+        reply.flags,
+        hmac_key,
+        &[FlagRule::Require(FLAG_OPEN), FlagRule::Require(FLAG_REPLY)],
+        PacketLayout::open_reply(hmac_key.is_some()).header_len() + params.len(),
+    )?;
+    if reply.token == 0 && !has(reply.flags, FLAG_CLOSE) {
+        return Err(ProtoError::ZeroToken);
+    }
+
+    out.extend_from_slice(&reply.token.to_le_bytes());
+    out.extend_from_slice(&params);
+    envelope::finish(out, hmac_key)
 }
 
 pub fn decode_open_reply(packet: &[u8], hmac_key: Option<&[u8]>) -> Result<OpenReply> {
-    let flags = validate_header(packet)?;
-    require(flags, FLAG_OPEN)?;
-    require(flags, FLAG_REPLY)?;
-    check_hmac_presence(flags, hmac_key)?;
-    if let Some(key) = hmac_key {
-        hmac::verify_hmac(key, packet, hmac::hmac_offset())?;
-    }
+    let envelope = envelope::decode(
+        packet,
+        hmac_key,
+        &[FlagRule::Require(FLAG_OPEN), FlagRule::Require(FLAG_REPLY)],
+    )?;
+    envelope::verify(packet, hmac_key)?;
 
-    let mut pos = HEADER_SIZE;
-    if has(flags, FLAG_HMAC) {
-        pos += HMAC_SIZE;
-    }
+    let mut pos = envelope.body_offset;
     let needed = pos + TOKEN_SIZE;
     if packet.len() < needed {
         return Err(ProtoError::PacketTooShort {
@@ -62,45 +84,25 @@ pub fn decode_open_reply(packet: &[u8], hmac_key: Option<&[u8]>) -> Result<OpenR
     }
     let token = u64::from_le_bytes(packet[pos..pos + TOKEN_SIZE].try_into().unwrap());
     pos += TOKEN_SIZE;
-    if token == 0 && !has(flags, FLAG_CLOSE) {
+    if token == 0 && !has(envelope.flags, FLAG_CLOSE) {
         return Err(ProtoError::ZeroToken);
     }
 
     Ok(OpenReply {
-        flags,
+        flags: envelope.flags,
         token,
         params: Params::decode(&packet[pos..])?,
     })
 }
 
-pub(crate) fn require(flags: u8, flag: u8) -> Result<()> {
-    if has(flags, flag) {
-        Ok(())
-    } else {
-        Err(ProtoError::MissingFlag(flag))
-    }
-}
-
-pub(crate) fn reject(flags: u8, flag: u8) -> Result<()> {
-    if has(flags, flag) {
-        Err(ProtoError::UnexpectedFlag(flag))
-    } else {
-        Ok(())
-    }
-}
-
-pub(crate) fn check_hmac_presence(flags: u8, hmac_key: Option<&[u8]>) -> Result<()> {
-    if has(flags, FLAG_HMAC) == hmac_key.is_some() {
-        Ok(())
-    } else {
-        Err(ProtoError::HmacPresenceMismatch)
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::params::{Clock, ReceivedStats, StampAt};
+    use crate::{
+        hmac,
+        params::{Clock, ReceivedStats, StampAt},
+        FLAG_HMAC, HMAC_SIZE,
+    };
 
     fn default_params() -> Params {
         Params {

crates/irtt-proto/tests/directional_codecs.rs

@@ -0,0 +1,361 @@
+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, Clock, CloseRequest, EchoReply, EchoRequest, OpenReply,
+    OpenRequest, PacketLayout, Params, ProtoError, ReceivedStats, StampAt, TimestampFields,
+    FLAG_CLOSE, FLAG_HMAC, FLAG_OPEN, FLAG_REPLY, MAGIC,
+};
+
+const KEY: &[u8] = b"testkey";
+const TOKEN: u64 = 0x7896_b6ab_8771_5213;
+
+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()
+    }
+}
+
+fn echo_request(payload: Vec<u8>) -> EchoRequest {
+    EchoRequest {
+        token: TOKEN,
+        sequence: 17,
+        payload,
+    }
+}
+
+fn echo_reply(params: &Params, flags: u8) -> EchoReply {
+    let layout = PacketLayout::echo(false, params);
+    EchoReply {
+        flags,
+        token: TOKEN,
+        sequence: u32::MAX,
+        recv_count: layout.recv_count.then_some(u32::MAX),
+        recv_window: layout.recv_window.then_some(u64::MAX),
+        timestamps: TimestampFields {
+            recv_wall: layout.recv_wall.then_some(i64::MIN),
+            recv_mono: layout.recv_mono.then_some(-2),
+            midpoint_wall: layout.midpoint_wall.then_some(-1),
+            midpoint_mono: layout.midpoint_mono.then_some(0),
+            send_wall: layout.send_wall.then_some(1),
+            send_mono: layout.send_mono.then_some(i64::MAX),
+        },
+        payload: Vec::new(),
+    }
+}
+
+#[test]
+fn authenticated_reply_encoders_set_hmac_from_key() {
+    let params = params();
+    let open_reply = OpenReply {
+        flags: FLAG_OPEN | FLAG_REPLY,
+        token: TOKEN,
+        params: params.clone(),
+    };
+    let packet = encode_open_reply(&open_reply, Some(KEY)).unwrap();
+    assert_eq!(packet[3], FLAG_OPEN | FLAG_REPLY | FLAG_HMAC);
+    assert_eq!(
+        decode_open_reply(&packet, Some(KEY)),
+        Ok(OpenReply {
+            flags: FLAG_OPEN | FLAG_REPLY | FLAG_HMAC,
+            ..open_reply
+        })
+    );
+
+    let echo_reply = echo_reply(&params, FLAG_REPLY);
+    let packet = encode_echo_reply(&echo_reply, &params, Some(KEY)).unwrap();
+    assert_eq!(packet[3], FLAG_REPLY | FLAG_HMAC);
+    assert_eq!(
+        decode_echo_reply(&packet, &params, Some(KEY)),
+        Ok(EchoReply {
+            flags: FLAG_REPLY | FLAG_HMAC,
+            ..echo_reply
+        })
+    );
+}
+
+#[test]
+fn rejected_open_reply_round_trips_and_zero_token_requires_close() {
+    let rejected = OpenReply {
+        flags: FLAG_OPEN | FLAG_REPLY | FLAG_CLOSE,
+        token: 0,
+        params: Params {
+            protocol_version: 1,
+            ..Params::default()
+        },
+    };
+    let packet = encode_open_reply(&rejected, None).unwrap();
+    assert_eq!(decode_open_reply(&packet, None), Ok(rejected.clone()));
+
+    assert_eq!(
+        encode_open_reply(
+            &OpenReply {
+                flags: FLAG_OPEN | FLAG_REPLY,
+                ..rejected
+            },
+            None,
+        ),
+        Err(ProtoError::ZeroToken)
+    );
+}
+
+#[test]
+fn peer_close_echo_reply_round_trips_without_a_close_reply_codec() {
+    let params = Params::default();
+    let reply = echo_reply(&params, FLAG_REPLY | FLAG_CLOSE);
+    let packet = encode_echo_reply(&reply, &params, None).unwrap();
+
+    assert_eq!(packet[3], FLAG_REPLY | FLAG_CLOSE);
+    assert_eq!(decode_echo_reply(&packet, &params, None), Ok(reply));
+}
+
+#[test]
+fn inverse_codecs_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 {
+                flags: FLAG_REPLY,
+                token: TOKEN,
+                params: params.clone(),
+            },
+            None,
+        ),
+        Err(ProtoError::MissingFlag(FLAG_OPEN))
+    );
+
+    let echo_reply_packet =
+        encode_echo_reply(&echo_reply(&params, FLAG_REPLY), &params, None).unwrap();
+    assert_eq!(
+        decode_echo_request(&echo_reply_packet, &params, None),
+        Err(ProtoError::UnexpectedFlag(FLAG_REPLY))
+    );
+    assert_eq!(
+        encode_echo_reply(
+            &EchoReply {
+                flags: 0,
+                ..echo_reply(&params, FLAG_REPLY)
+            },
+            &params,
+            None,
+        ),
+        Err(ProtoError::MissingFlag(FLAG_REPLY))
+    );
+
+    let close_packet = encode_close_request(&CloseRequest { token: TOKEN }, None).unwrap();
+    assert_eq!(
+        decode_echo_request(&close_packet, &params, None),
+        Err(ProtoError::UnexpectedFlag(FLAG_CLOSE))
+    );
+
+    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))
+    );
+}
+
+#[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)
+    );
+
+    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());
+    malformed_open_reply.extend_from_slice(&[1, 0x80]);
+    assert_eq!(
+        decode_open_reply(&malformed_open_reply, None),
+        Err(ProtoError::TruncatedVarint)
+    );
+
+    let params = Params {
+        length: 20,
+        ..Params::default()
+    };
+    let echo_packet = encode_echo_request(&echo_request(Vec::new()), &params, None).unwrap();
+    assert_eq!(
+        decode_echo_request(&echo_packet[..19], &params, None),
+        Err(ProtoError::PacketLengthMismatch {
+            expected: 20,
+            actual: 19,
+        })
+    );
+    let mut long_echo_packet = echo_packet;
+    long_echo_packet.push(0);
+    assert_eq!(
+        decode_echo_request(&long_echo_packet, &params, None),
+        Err(ProtoError::PacketLengthMismatch {
+            expected: 20,
+            actual: 21,
+        })
+    );
+
+    let close_packet = encode_close_request(&CloseRequest { token: TOKEN }, None).unwrap();
+    assert_eq!(
+        decode_close_request(&close_packet[..11], None),
+        Err(ProtoError::PacketLengthMismatch {
+            expected: 12,
+            actual: 11,
+        })
+    );
+    let mut long_close_packet = close_packet;
+    long_close_packet.push(0);
+    assert_eq!(
+        decode_close_request(&long_close_packet, None),
+        Err(ProtoError::PacketLengthMismatch {
+            expected: 12,
+            actual: 13,
+        })
+    );
+}
+
+#[test]
+fn echo_layout_combinations_round_trip() {
+    for received_stats in [
+        ReceivedStats::None,
+        ReceivedStats::Count,
+        ReceivedStats::Window,
+        ReceivedStats::Both,
+    ] {
+        for stamp_at in [
+            StampAt::None,
+            StampAt::Send,
+            StampAt::Receive,
+            StampAt::Both,
+            StampAt::Midpoint,
+        ] {
+            for clock in [Clock::Wall, Clock::Monotonic, Clock::Both] {
+                for key in [None, Some(KEY)] {
+                    let params = Params {
+                        protocol_version: 1,
+                        received_stats,
+                        stamp_at,
+                        clock,
+                        ..Params::default()
+                    };
+                    let reply = echo_reply(&params, FLAG_REPLY);
+                    let packet = encode_echo_reply(&reply, &params, key).unwrap();
+                    let decoded = decode_echo_reply(&packet, &params, key).unwrap();
+
+                    assert_eq!(decoded.token, reply.token);
+                    assert_eq!(decoded.sequence, reply.sequence);
+                    assert_eq!(decoded.recv_count, reply.recv_count);
+                    assert_eq!(decoded.recv_window, reply.recv_window);
+                    assert_eq!(decoded.timestamps, reply.timestamps);
+                    assert_eq!(decoded.payload, reply.payload);
+                }
+            }
+        }
+    }
+}
+
+#[test]
+fn echo_reply_requires_exact_negotiated_optional_fields() {
+    let params = Params {
+        received_stats: ReceivedStats::Count,
+        stamp_at: StampAt::Send,
+        clock: Clock::Wall,
+        ..Params::default()
+    };
+    let missing_count = EchoReply {
+        recv_count: None,
+        ..echo_reply(&params, FLAG_REPLY)
+    };
+    assert_eq!(
+        encode_echo_reply(&missing_count, &params, None),
+        Err(ProtoError::MissingField("recv_count"))
+    );
+
+    let no_optional_params = Params::default();
+    let unexpected_timestamp = EchoReply {
+        timestamps: TimestampFields {
+            send_wall: Some(1),
+            ..TimestampFields::default()
+        },
+        ..echo_reply(&no_optional_params, FLAG_REPLY)
+    };
+    assert_eq!(
+        encode_echo_reply(&unexpected_timestamp, &no_optional_params, None),
+        Err(ProtoError::UnexpectedField("send_wall"))
+    );
+}
+
+#[test]
+fn echo_payload_is_copied_zero_filled_and_bounded_in_both_directions() {
+    let params = Params {
+        length: 20,
+        ..Params::default()
+    };
+
+    let request = echo_request(vec![1, 2]);
+    let packet = encode_echo_request(&request, &params, None).unwrap();
+    assert_eq!(&packet[16..], &[1, 2, 0, 0]);
+    let decoded = decode_echo_request(&packet, &params, None).unwrap();
+    assert_eq!(decoded.payload, vec![1, 2, 0, 0]);
+    assert_eq!(
+        encode_echo_request(&decoded, &params, None).unwrap(),
+        packet
+    );
+    assert_eq!(
+        encode_echo_request(&echo_request(vec![0; 5]), &params, None),
+        Err(ProtoError::PayloadTooLarge {
+            available: 4,
+            provided: 5,
+        })
+    );
+
+    let reply = EchoReply {
+        payload: vec![3, 4],
+        ..echo_reply(&params, FLAG_REPLY)
+    };
+    let packet = encode_echo_reply(&reply, &params, None).unwrap();
+    assert_eq!(&packet[16..], &[3, 4, 0, 0]);
+    let decoded = decode_echo_reply(&packet, &params, None).unwrap();
+    assert_eq!(decoded.payload, vec![3, 4, 0, 0]);
+    assert_eq!(encode_echo_reply(&decoded, &params, None).unwrap(), packet);
+    assert_eq!(
+        encode_echo_reply(
+            &EchoReply {
+                payload: vec![0; 5],
+                ..echo_reply(&params, FLAG_REPLY)
+            },
+            &params,
+            None,
+        ),
+        Err(ProtoError::PayloadTooLarge {
+            available: 4,
+            provided: 5,
+        })
+    );
+}

crates/irtt-proto/tests/vectors.rs

@@ -1,7 +1,8 @@
 use irtt_proto::{
-    decode_echo_reply, decode_open_reply, encode_close_request, encode_echo_request,
-    encode_open_request, verify_hmac, Clock, CloseRequest, EchoRequest, OpenRequest, Params,
-    ReceivedStats, StampAt,
+    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,
 };
 
 fn hex(input: &str) -> Vec<u8> {
@@ -50,6 +51,13 @@ fn vector_1_open_request_no_hmac() {
     )
     .unwrap();
     assert_eq!(packet, expected);
+    assert_eq!(
+        decode_open_request(&expected, None),
+        Ok(OpenRequest {
+            params: default_params_3s(),
+            close: false,
+        })
+    );
 }
 
 #[test]
@@ -59,6 +67,7 @@ fn vector_2_open_reply_no_hmac() {
     let reply = decode_open_reply(&packet, None).unwrap();
     assert_eq!(reply.token, 0x7896_b6ab_8771_5213);
     assert_eq!(reply.params, default_params_3s());
+    assert_eq!(encode_open_reply(&reply, None).unwrap(), packet);
 }
 
 #[test]
@@ -71,13 +80,21 @@ fn vector_3_echo_request_no_hmac() {
         &EchoRequest {
             token: 0x7896_b6ab_8771_5213,
             sequence: 0,
-            params: default_params_3s(),
             payload: Vec::new(),
         },
+        &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(),
+        })
+    );
 }
 
 #[test]
@@ -90,6 +107,10 @@ fn vector_4_echo_reply_no_hmac() {
     assert_eq!(reply.sequence, 2);
     assert_eq!(reply.recv_count, Some(3));
     assert_eq!(reply.recv_window, Some(7));
+    assert_eq!(
+        encode_echo_reply(&reply, &default_params_3s(), None).unwrap(),
+        packet
+    );
 }
 
 #[test]
@@ -103,6 +124,12 @@ fn vector_5_close_request_no_hmac() {
     )
     .unwrap();
     assert_eq!(packet, expected);
+    assert_eq!(
+        decode_close_request(&expected, None),
+        Ok(CloseRequest {
+            token: 0x7896_b6ab_8771_5213,
+        })
+    );
 }
 
 #[test]
@@ -120,6 +147,13 @@ fn vector_6_hmac_open_request() {
     .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,
+        })
+    );
 }
 
 #[test]
@@ -133,13 +167,21 @@ fn vector_7_hmac_echo_request() {
         &EchoRequest {
             token: 0x4387_e9eb_5d3f_ca59,
             sequence: 0,
-            params: default_params_3s(),
             payload: Vec::new(),
         },
+        &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(),
+        })
+    );
 }
 
 #[test]
@@ -154,6 +196,12 @@ fn vector_8_hmac_close_request() {
     )
     .unwrap();
     assert_eq!(packet, expected);
+    assert_eq!(
+        decode_close_request(&expected, Some(b"testkey")),
+        Ok(CloseRequest {
+            token: 0x4387_e9eb_5d3f_ca59,
+        })
+    );
 }
 
 #[test]
@@ -169,6 +217,13 @@ fn vector_9_no_test_open_close_request() {
     )
     .unwrap();
     assert_eq!(packet, expected);
+    assert_eq!(
+        decode_open_request(&expected, None),
+        Ok(OpenRequest {
+            params: default_params_60s(),
+            close: true,
+        })
+    );
 }
 
 #[test]
@@ -179,6 +234,7 @@ fn vector_10_no_test_open_close_reply() {
     let reply = decode_open_reply(&packet, None).unwrap();
     assert_eq!(reply.token, 0);
     assert_eq!(reply.params, default_params_60s());
+    assert_eq!(encode_open_reply(&reply, None).unwrap(), packet);
 }
 
 #[test]
@@ -188,13 +244,21 @@ fn vector_11_minimal_echo_packet() {
         &EchoRequest {
             token: 0xa031_6fa2_5c61_154e,
             sequence: 0,
-            params: Params::default(),
             payload: Vec::new(),
         },
+        &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(),
+        })
+    );
 }
 
 #[test]
@@ -214,6 +278,7 @@ fn vector_12_midpoint_reply() {
     assert!(reply.timestamps.midpoint_wall.is_some());
     assert!(reply.timestamps.recv_wall.is_none());
     assert!(reply.timestamps.send_wall.is_none());
+    assert_eq!(encode_echo_reply(&reply, &params, None).unwrap(), packet);
 }
 
 #[test]
@@ -235,27 +300,51 @@ fn spec_18_1_hmac_echo_request_fixture() {
         &EchoRequest {
             token: 0x886b_c9a7_22b3_3eea,
             sequence: 0x6fe2_a1bb,
-            params,
             payload: [0xff, 0xfe, 0xfd, 0xfc].repeat(4),
         },
+        &params,
         Some(&[0x3c, 0x68, 0x1d, 0x39, 0x41, 0x1d, 0x72, 0x43]),
     )
     .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)
+    );
 }
 
 #[test]
-fn spec_18_2_hmac_echo_reply_fixture() {
+fn spec_18_2_ambiguous_hmac_fixture_omits_reply_flag() {
     let packet = hex("14 a7 5b 08 d2 98 a3 4a 6a 13 41 02 68 b2 67 a8
          d6 7e 28 25 c3 cb 6f 76 b6 ce 66 e6 06 07 3b 1d
          19 9f bc a3 9b 3b f8 86 e5 39 e9 d7 a1 75 2a ee
          3d f1 5a 52 9a c6 4a 3e 22 62 55 2d 69 3f 29 46
          d4 05 97 58 66 50 2c dd 2f f1 1d 46 ff fe fd fc
          ff fe fd fc ff fe fd fc ff fe fd fc");
-    verify_hmac(
-        &[0xda, 0xb3, 0xe9, 0x04, 0xa6, 0x87, 0x92, 0x49],
-        &packet,
-        4,
-    )
-    .unwrap();
+    let key = [0xda, 0xb3, 0xe9, 0x04, 0xa6, 0x87, 0x92, 0x49];
+    verify_hmac(&key, &packet, 4).unwrap();
+
+    let params = Params {
+        length: 92,
+        received_stats: ReceivedStats::Both,
+        stamp_at: StampAt::Both,
+        clock: Clock::Both,
+        ..Params::default()
+    };
+    assert_eq!(
+        decode_echo_reply(&packet, &params, Some(&key)),
+        Err(ProtoError::MissingFlag(FLAG_REPLY))
+    );
 }