diff --git a/crates/irtt-proto/src/echo.rs b/crates/irtt-proto/src/echo.rs index c924551..f6e9a5f 100644 --- a/crates/irtt-proto/src/echo.rs +++ b/crates/irtt-proto/src/echo.rs @@ -2,10 +2,38 @@ use crate::{ envelope::{self, FlagRule}, flags::{FLAG_CLOSE, FLAG_OPEN, FLAG_REPLY}, layout::{echo_packet_len, PacketLayout}, - params::Params, + params::{Clock, Params, StampAt}, ProtoError, Result, RECV_COUNT_SIZE, RECV_WINDOW_SIZE, SEQ_SIZE, TIMESTAMP_SIZE, TOKEN_SIZE, }; +/// Upstream irtt 0.9.1 always emits both midpoint timestamp fields (wall, +/// then monotonic) in an ECHO reply for single-clock `StampAt::Midpoint` +/// negotiations, even though only one clock was negotiated. That makes the +/// compatibility header exactly one timestamp longer than the negotiated +/// header. +const MIDPOINT_COMPAT_EXTRA: usize = TIMESTAMP_SIZE; + +fn is_midpoint_single_clock(params: &Params) -> bool { + params.stamp_at == StampAt::Midpoint && !matches!(params.clock, Clock::Both) +} + +/// Total datagram length of the upstream dual-midpoint compatibility form. +/// +/// Both lengths are `max(negotiated_length, header)`, so this is *not* +/// `normal_len + TIMESTAMP_SIZE`. The extra midpoint field only lengthens the +/// datagram while the larger header is still pushing past the negotiated +/// length; beyond that it displaces payload instead, and the two forms have +/// identical length. The difference is therefore 8, 7..1, or 0 depending on +/// the negotiated length. +/// +/// Deriving this from `normal_len` keeps the identity exact — +/// `max(negotiated_length, compat_header) == max(compat_header, normal_len)` — +/// and cannot overflow, because `compat_header` is a small bounded header +/// size rather than an attacker-influenced packet length. +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, @@ -83,7 +111,7 @@ pub fn decode_echo_request( ], )?; let layout = PacketLayout::echo(hmac_key.is_some(), params); - validate_echo_length(packet, params, layout)?; + validate_echo_length(packet, params, layout, false)?; envelope::verify(packet, hmac_key)?; let mut pos = envelope.body_offset; @@ -128,6 +156,22 @@ pub fn encode_echo_reply( envelope::finish(out, hmac_key) } +/// Decodes an ECHO reply, accepting the upstream irtt 0.9.1 dual-midpoint +/// compatibility form when it is identifiable by datagram length. +/// +/// For single-clock `StampAt::Midpoint` negotiations upstream emits both +/// midpoint fields (wall, then monotonic). When that larger header makes the +/// datagram longer than the negotiated length, the compatibility form is +/// unambiguous and both fields are parsed, exposing only the negotiated +/// clock's value. +/// +/// When the negotiated length already covers the larger header the two forms +/// have identical length and are indistinguishable from the packet alone. The +/// reply is still accepted, parsed against the negotiated layout. For a +/// monotonic-only negotiation that means the value exposed as `midpoint_mono` +/// is whatever occupies the negotiated field position, which for an upstream +/// peer is its wall timestamp — this cannot be corrected deterministically +/// without out-of-band knowledge of the peer, so no heuristic is applied. pub fn decode_echo_reply( packet: &[u8], params: &Params, @@ -139,23 +183,40 @@ pub fn decode_echo_reply( &[FlagRule::Reject(FLAG_OPEN), FlagRule::Require(FLAG_REPLY)], )?; let layout = PacketLayout::echo(hmac_key.is_some(), params); - validate_echo_length(packet, params, layout)?; + let midpoint_compat = validate_echo_length(packet, params, layout, true)?; envelope::verify(packet, hmac_key)?; - let header_len = layout.header_len(); + let header_len = layout.header_len() + + if midpoint_compat { + MIDPOINT_COMPAT_EXTRA + } else { + 0 + }; 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)); let recv_window = layout.recv_window.then(|| read_u64(packet, &mut pos)); - let timestamps = TimestampFields { - recv_wall: layout.recv_wall.then(|| read_i64(packet, &mut pos)), - recv_mono: layout.recv_mono.then(|| read_i64(packet, &mut pos)), - midpoint_wall: layout.midpoint_wall.then(|| read_i64(packet, &mut pos)), - midpoint_mono: layout.midpoint_mono.then(|| read_i64(packet, &mut pos)), - send_wall: layout.send_wall.then(|| read_i64(packet, &mut pos)), - send_mono: layout.send_mono.then(|| read_i64(packet, &mut pos)), + let recv_wall = layout.recv_wall.then(|| read_i64(packet, &mut pos)); + let recv_mono = layout.recv_mono.then(|| read_i64(packet, &mut pos)); + let (midpoint_wall, midpoint_mono) = if midpoint_compat { + let wall = read_i64(packet, &mut pos); + let mono = read_i64(packet, &mut pos); + match params.clock { + Clock::Wall => (Some(wall), None), + Clock::Monotonic => (None, Some(mono)), + Clock::Both => { + unreachable!("midpoint compat only applies to single-clock negotiations") + } + } + } else { + ( + layout.midpoint_wall.then(|| read_i64(packet, &mut pos)), + layout.midpoint_mono.then(|| read_i64(packet, &mut pos)), + ) }; + let send_wall = layout.send_wall.then(|| read_i64(packet, &mut pos)); + let send_mono = layout.send_mono.then(|| read_i64(packet, &mut pos)); Ok(EchoReply { flags: envelope.flags, @@ -163,12 +224,32 @@ pub fn decode_echo_reply( sequence, recv_count, recv_window, - timestamps, + timestamps: TimestampFields { + recv_wall, + recv_mono, + midpoint_wall, + midpoint_mono, + send_wall, + send_mono, + }, payload: packet[header_len..].to_vec(), }) } -fn validate_echo_length(packet: &[u8], params: &Params, layout: PacketLayout) -> Result<()> { +/// 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). +/// +/// 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 { let header_len = layout.header_len(); if packet.len() < header_len { return Err(ProtoError::PacketTooShort { @@ -177,13 +258,19 @@ fn validate_echo_length(packet: &[u8], params: &Params, layout: PacketLayout) -> }); } let expected_len = echo_packet_len(layout.hmac, params)?; - if packet.len() != expected_len { - return Err(ProtoError::PacketLengthMismatch { - expected: expected_len, - actual: packet.len(), - }); + if packet.len() == expected_len { + return Ok(false); } - Ok(()) + if allow_midpoint_compat && 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); + } + } + Err(ProtoError::PacketLengthMismatch { + expected: expected_len, + actual: packet.len(), + }) } fn push_zeroed_layout_tail(layout: PacketLayout, out: &mut Vec) { diff --git a/crates/irtt-proto/tests/directional_codecs.rs b/crates/irtt-proto/tests/directional_codecs.rs index c8828e6..58587f7 100644 --- a/crates/irtt-proto/tests/directional_codecs.rs +++ b/crates/irtt-proto/tests/directional_codecs.rs @@ -1,11 +1,16 @@ 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, + 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, }; +/// Wire offset of the HMAC digest within an ECHO reply: the fixed 4-byte +/// header (magic + flags) always precedes it. +const HMAC_OFFSET: usize = 4; + const KEY: &[u8] = b"testkey"; const TOKEN: u64 = 0x7896_b6ab_8771_5213; @@ -359,3 +364,265 @@ fn echo_payload_is_copied_zero_filled_and_bounded_in_both_directions() { }) ); } + +// ---------- Upstream midpoint ECHO-reply compatibility ---------- +// +// Verified upstream irtt 0.9.1 behavior: for single-clock +// `StampAt::Midpoint` negotiations (`Clock::Wall` or `Clock::Monotonic`) +// upstream always emits BOTH midpoint timestamp fields (wall, then +// monotonic), so its header is one timestamp longer than the negotiated +// header. +// +// Crucially the datagram length is NOT `normal_len + 8`. Both forms are +// `max(negotiated_length, header)`, so the extra field lengthens the +// datagram only while the larger header still exceeds the negotiated +// length, and displaces payload beyond that: +// +// compat_len - normal_len == 8, then 7..1, then 0 +// +// `decode_echo_reply` must accept the longer form wherever it is +// identifiable by length, keep rejecting unrelated lengths, and fall back +// to plain negotiated parsing once the two forms are the same length. +// +// `encode_echo_reply` must never produce the dual-field shape, so these +// tests hand-build the wire form directly and narrowly. + +/// Midpoint params whose negotiated length sits `payload_space` bytes past +/// the normal (single-midpoint) header. +fn midpoint_params(clock: Clock, payload_space: usize) -> Params { + let base = Params { + protocol_version: 1, + received_stats: ReceivedStats::Count, + stamp_at: StampAt::Midpoint, + clock, + ..Params::default() + }; + let header_len = PacketLayout::echo(false, &base).header_len(); + Params { + length: (header_len + payload_space) as i64, + ..base + } +} + +fn midpoint_compat_len(params: &Params, hmac: bool) -> usize { + let compat_header = PacketLayout::echo(hmac, params).header_len() + 8; + compat_header.max(echo_packet_len(hmac, params).unwrap()) +} + +/// Hand-builds upstream's dual-midpoint ECHO reply: both midpoint fields on +/// the wire, total length `max(negotiated_length, normal_header + 8)`. +fn build_midpoint_dual_field_reply( + params: &Params, + flags: u8, + seq: u32, + midpoint_wire_fields: (i64, i64), + hmac_key: Option<&[u8]>, +) -> Vec { + let (wall, mono) = midpoint_wire_fields; + let hmac = hmac_key.is_some(); + let layout = PacketLayout::echo(hmac, params); + let compat_header_len = layout.header_len() + 8; + let compat_len = midpoint_compat_len(params, hmac); + + let mut packet = MAGIC.to_vec(); + packet.push(flags); + if hmac { + packet.extend_from_slice(&[0u8; 16]); + } + packet.extend_from_slice(&TOKEN.to_le_bytes()); + packet.extend_from_slice(&seq.to_le_bytes()); + if layout.recv_count { + packet.extend_from_slice(&7_u32.to_le_bytes()); + } + if layout.recv_window { + packet.extend_from_slice(&11_u64.to_le_bytes()); + } + packet.extend_from_slice(&wall.to_le_bytes()); + packet.extend_from_slice(&mono.to_le_bytes()); + assert_eq!(packet.len(), compat_header_len); + + packet.resize(compat_len, 0); + for (index, byte) in packet[compat_header_len..].iter_mut().enumerate() { + *byte = (index as u8).wrapping_add(1); + } + + if let Some(key) = hmac_key { + compute_hmac_in_place(key, &mut packet, HMAC_OFFSET).unwrap(); + } + packet +} + +#[test] +fn midpoint_dual_field_reply_is_accepted_wherever_length_identifies_it() { + // payload_space -> how much longer the compatibility datagram is. + // Covers the full +8 regime and the intermediate +7..+1 regimes. + for (payload_space, expected_extra) in [(0, 8), (1, 7), (4, 4), (7, 1)] { + for clock in [Clock::Wall, Clock::Monotonic] { + let params = midpoint_params(clock, payload_space); + let normal_len = echo_packet_len(false, ¶ms).unwrap(); + let packet = build_midpoint_dual_field_reply(¶ms, FLAG_REPLY, 3, (111, 222), None); + assert_eq!( + packet.len(), + normal_len + expected_extra, + "payload_space {payload_space} clock {clock:?}" + ); + + let reply = decode_echo_reply(&packet, ¶ms, None).unwrap(); + let (expected_wall, expected_mono) = match clock { + Clock::Wall => (Some(111), None), + // The first wire field must not be mistaken for the + // negotiated monotonic value. + Clock::Monotonic => (None, Some(222)), + Clock::Both => unreachable!(), + }; + assert_eq!( + reply.timestamps.midpoint_wall, expected_wall, + "payload_space {payload_space} clock {clock:?}" + ); + assert_eq!( + reply.timestamps.midpoint_mono, expected_mono, + "payload_space {payload_space} clock {clock:?}" + ); + // Payload begins after BOTH compatibility timestamp fields. + let compat_header_len = PacketLayout::echo(false, ¶ms).header_len() + 8; + assert_eq!(reply.payload, packet[compat_header_len..].to_vec()); + } + } +} + +#[test] +fn midpoint_dual_field_reply_of_equal_length_is_accepted_but_not_correctable() { + // Once the negotiated length covers the larger header, the conforming + // and upstream forms have identical length. The reply must still be + // accepted, parsed against the negotiated layout. + for payload_space in [8, 64] { + for clock in [Clock::Wall, Clock::Monotonic] { + let params = midpoint_params(clock, payload_space); + let normal_len = echo_packet_len(false, ¶ms).unwrap(); + let packet = build_midpoint_dual_field_reply(¶ms, FLAG_REPLY, 4, (111, 222), None); + assert_eq!( + packet.len(), + normal_len, + "compat and negotiated lengths coincide for payload_space {payload_space}" + ); + + let reply = decode_echo_reply(&packet, ¶ms, None).unwrap(); + match clock { + // Wall-only happens to line up: the negotiated field + // position holds upstream's wall timestamp. + Clock::Wall => { + assert_eq!(reply.timestamps.midpoint_wall, Some(111)); + assert_eq!(reply.timestamps.midpoint_mono, None); + } + // Monotonic-only cannot be corrected deterministically from + // the packet alone: the negotiated field position holds + // upstream's WALL timestamp, and nothing in the datagram + // distinguishes that from a conforming reply. Documented + // limitation - we accept rather than guess. + Clock::Monotonic => { + assert_eq!(reply.timestamps.midpoint_wall, None); + assert_eq!(reply.timestamps.midpoint_mono, Some(111)); + } + Clock::Both => unreachable!(), + } + } + } +} + +#[test] +fn midpoint_dual_field_reply_authenticates_over_full_packet() { + let params = midpoint_params(Clock::Wall, 0); + let packet = + build_midpoint_dual_field_reply(¶ms, FLAG_REPLY | FLAG_HMAC, 9, (555, 666), Some(KEY)); + assert_eq!(packet.len(), midpoint_compat_len(¶ms, true)); + + let reply = decode_echo_reply(&packet, ¶ms, Some(KEY)).unwrap(); + assert_eq!(reply.timestamps.midpoint_wall, Some(555)); + + let mut corrupted = packet; + corrupted[HMAC_OFFSET] ^= 0xFF; + assert_eq!( + decode_echo_reply(&corrupted, ¶ms, Some(KEY)), + Err(ProtoError::BadHmac) + ); +} + +#[test] +fn only_the_exact_compat_length_is_accepted_as_an_extension() { + let params = midpoint_params(Clock::Wall, 0); + let packet = build_midpoint_dual_field_reply(¶ms, FLAG_REPLY, 1, (10, 20), None); + let expected_len = echo_packet_len(false, ¶ms).unwrap(); + assert_eq!(packet.len(), expected_len + 8); + + // Every other length around the compatibility size stays rejected, + // including the intermediate +1..+7 sizes that are only valid under a + // different negotiated length. + for delta in [-7_i64, -1, 1, 92] { + let target_len = (packet.len() as i64 + delta) as usize; + let mut malformed = packet.clone(); + malformed.resize(target_len, 0); + assert_eq!( + decode_echo_reply(&malformed, ¶ms, None), + Err(ProtoError::PacketLengthMismatch { + expected: expected_len, + actual: target_len, + }), + "delta {delta} (target length {target_len}) must still be rejected" + ); + } +} + +#[test] +fn midpoint_both_clock_reply_has_no_additional_extension() { + let params = midpoint_params(Clock::Both, 0); + let reply = echo_reply(¶ms, FLAG_REPLY); + let packet = encode_echo_reply(&reply, ¶ms, None).unwrap(); + + let mut too_long = packet.clone(); + too_long.extend_from_slice(&[0; 8]); + assert_eq!( + decode_echo_reply(&too_long, ¶ms, None), + Err(ProtoError::PacketLengthMismatch { + expected: packet.len(), + actual: too_long.len(), + }) + ); +} + +#[test] +fn non_midpoint_stamp_at_has_no_longer_form_exception() { + let params = Params { + received_stats: ReceivedStats::None, + stamp_at: StampAt::Receive, + clock: Clock::Wall, + ..Params::default() + }; + let reply = echo_reply(¶ms, FLAG_REPLY); + let packet = encode_echo_reply(&reply, ¶ms, None).unwrap(); + + let mut too_long = packet.clone(); + too_long.extend_from_slice(&[0; 8]); + assert_eq!( + decode_echo_reply(&too_long, ¶ms, None), + Err(ProtoError::PacketLengthMismatch { + expected: packet.len(), + actual: too_long.len(), + }) + ); +} + +#[test] +fn echo_request_decoding_rejects_the_compat_length() { + let params = midpoint_params(Clock::Wall, 0); + let packet = encode_echo_request(&echo_request(Vec::new()), ¶ms, None).unwrap(); + + let mut too_long = packet.clone(); + too_long.extend_from_slice(&[0; 8]); + assert_eq!( + decode_echo_request(&too_long, ¶ms, None), + Err(ProtoError::PacketLengthMismatch { + expected: packet.len(), + actual: too_long.len(), + }) + ); +}