Skip to content

Commit 94c3acb

Browse files
committed
fix: align STAMP TLV flag and reflected header handling
- Set sender TLV U-flag defaults per RFC 8972 and rederive reflector U/M/I flags - Preserve parser-detected malformed TLVs through reflector flag clearing - Encode reflected header requests with zero-filled advertised capacity - Preserve reflected header response length with truncation/zero-padding - Accept decimal or 0x-prefixed hex for LAG member link IDs - Update tests and changelog for the corrected TLV behavior
1 parent 1a6fc60 commit 94c3acb

13 files changed

Lines changed: 880 additions & 143 deletions

File tree

CHANGELOG.md

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1818
- New typed TLVs `ReflectedFixedHdrTlv` and `ReflectedIpv6ExtHdrTlv`
1919
implementing `TypedTlv`, plus `TlvType::ReflectedFixedHdr` /
2020
`ReflectedIpv6ExtHdr` enum variants and length validation that treats
21-
the Value as variable-length (empty = sender request, populated =
22-
reflector response).
21+
the Value as variable-length (sender pre-allocates a zero-filled
22+
Value sized to the expected header length per draft-ietf-ippm-stamp-ext-hdr;
23+
populated bytes on response).
2324
- New `CapturedHeaders` struct threaded through `ProcessingContext` so the
2425
reflector's TLV processing can see the raw IP-layer bytes captured at
2526
receive time.
@@ -39,8 +40,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3940
empty Value on an IPv4 packet or an IPv6 packet without extension
4041
headers is legitimate and does **not** set the U-flag.
4142
- Sender CLI: `--reflected-fixed-hdr`, `--reflected-ipv6-ext-hdr` (plus
42-
matching `FileConfiguration` TOML fields). Sender attaches empty-request
43-
TLVs; reflector fills them on the pnet backend or U-flags them on nix.
43+
matching `FileConfiguration` TOML fields). Sender attaches a zero-filled
44+
request TLV sized to the destination's IP family for Type 247 and an
45+
8-byte default (one option's worth) for Type 246; reflector fills them
46+
on the pnet backend or U-flags them on nix.
47+
48+
### Fixed
49+
50+
- **RFC 8972 §4.4.1 sender flag default**: `TlvFlags::for_sender()` now
51+
returns `U=1, M=0, I=0` instead of all-zero. The RFC requires the
52+
Session-Sender to send every TLV with the U flag set; the reflector then
53+
overwrites it. Sender-built TLVs (`RawTlv::new`) inherit the corrected
54+
default. Visible on the wire as the leading flag byte of every outgoing
55+
TLV flipping from `0x00` to `0x80`.
56+
- **RFC 8972 §4.4.1 reflector flag overwrite**: the reflector now clears
57+
U, M, and I on every parsed TLV before the type-recognition / length-
58+
validation / HMAC-verification pass re-derives them. Previously each
59+
flag was only ever set to 1; the RFC's three "Otherwise … MUST set …
60+
to 0" clauses require them to be cleared as well. Practically: an
61+
echoed CoS TLV (or any other recognized type) now reports `U=0` to the
62+
sender even when the sender obeyed the §4.4.1 mandate to send `U=1`.
63+
The C-flag (`conformant_reflected`, draft-ietf-ippm-asymmetrical-pkts)
64+
and parser-detected truncation M-flag are preserved across the clear.
65+
- **draft-ietf-ippm-stamp-ext-hdr Type 246 / 247 sender request encoding**:
66+
Session-Sender now pre-allocates the Value field with zeros sized to the
67+
expected header length (20 for IPv4 fixed header, 40 for IPv6 fixed
68+
header, 8-byte capacity for IPv6 ext-header chain) per the draft.
69+
Previously sent length=0; conforming reflectors that validate the
70+
request's Length field rejected it as malformed.
4471

4572
### Changed
4673

@@ -52,6 +79,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5279
internals — `nix::ifaddrs::getifaddrs` on Unix, `pnet::datalink::interfaces`
5380
on Windows. Removes ~60 lines of near-duplicate code and a class of
5481
drift bugs.
82+
- `--micro-session-id` and `--reflector-member-link-id` (RFC 9534 LAG
83+
identifiers) now accept `0x`-prefixed hex (`0xff`, `0XFF`, `0x00ab`)
84+
in addition to decimal. Aligns with the conventional way these wire
85+
fields are written.
86+
87+
### Breaking
88+
89+
- `ReflectedFixedHdrTlv::request()` removed; replaced by
90+
`ReflectedFixedHdrTlv::request_for(IpAddr)` (chooses 20 / 40 bytes from
91+
the destination address family) or
92+
`ReflectedFixedHdrTlv::request_with_capacity(usize)` (explicit zero-fill
93+
size). The old API produced an empty-Value TLV that did not match the
94+
draft's request format.
95+
- `ReflectedIpv6ExtHdrTlv::request()` removed; replaced by
96+
`ReflectedIpv6ExtHdrTlv::request_with_capacity(usize)` so the caller
97+
picks the zero-filled Value size to match the path's expected
98+
extension-header chain. The default size for the sender flag
99+
(`--reflected-ipv6-ext-hdr`) is exposed as
100+
`tlv::DEFAULT_IPV6_EXT_HDR_REQUEST_CAPACITY` (8 bytes — one option).
101+
- Reflector behavior change: when populating Type 246 / 247 responses,
102+
the reflector now preserves the sender-advertised Length, zero-padding
103+
short captures and truncating long ones. Callers that depended on the
104+
response length matching the captured-bytes length should size the
105+
request appropriately.
55106

56107
### Documentation
57108

flake.nix

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@
1010
flake-utils.lib.eachDefaultSystem (system:
1111
let
1212
pkgs = nixpkgs.legacyPackages.${system};
13+
14+
# Cargo features compiled into the nix-built binary and exercised
15+
# by the check phase. Mirrors `cargo build/test --all-features`.
16+
allFeatures = [ "ttl-nix" "ttl-pnet" "metrics" "snmp" ];
1317
in
1418
{
1519
packages = {
@@ -19,24 +23,58 @@
1923

2024
src = self;
2125

22-
2326
cargoHash = "sha256-9xwRzJyuz2BMk+1ThemzKqO/gVsX+aLhBL2oLWMyVwc=";
2427

28+
buildFeatures = allFeatures;
29+
# Honour --all-features for the cargo test phase too so the
30+
# metrics / snmp feature-gated tests run alongside the rest.
31+
cargoTestFlags = [ "--all-features" ];
32+
2533
meta = with pkgs.lib; {
2634
description = "Simple Two-Way Active Measurement Protocol (STAMP) implementation";
2735
homepage = "https://github.com/asmie/stamp-suite";
2836
license = licenses.mit;
2937
mainProgram = "stamp-suite";
38+
platforms = platforms.unix;
3039
};
3140
};
3241
};
3342

43+
# `nix flake check` exercises the package build (which includes the
44+
# cargo test phase) plus a clippy-with-warnings-as-errors gate that
45+
# mirrors CI.
46+
checks = {
47+
build = self.packages.${system}.default;
48+
49+
clippy = pkgs.rustPlatform.buildRustPackage {
50+
pname = "stamp-suite-clippy";
51+
version = "0.6.1";
52+
src = self;
53+
cargoHash = "sha256-9xwRzJyuz2BMk+1ThemzKqO/gVsX+aLhBL2oLWMyVwc=";
54+
buildFeatures = allFeatures;
55+
nativeBuildInputs = [ pkgs.clippy ];
56+
buildPhase = ''
57+
cargo clippy --all --all-features --tests -- -D warnings
58+
'';
59+
doCheck = false;
60+
installPhase = "mkdir -p $out";
61+
};
62+
63+
fmt = pkgs.runCommand "stamp-suite-fmt"
64+
{ nativeBuildInputs = [ pkgs.rustfmt pkgs.cargo ]; } ''
65+
cd ${self}
66+
cargo fmt --all -- --check
67+
touch $out
68+
'';
69+
};
70+
3471
devShells.default = pkgs.mkShell {
3572
buildInputs = with pkgs; [
3673
cargo
3774
rustc
3875
rustfmt
3976
clippy
77+
rust-analyzer
4078
];
4179
};
4280
}

src/configuration.rs

Lines changed: 101 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,12 +275,14 @@ pub struct Configuration {
275275

276276
/// Sender micro-session member link ID for LAG measurement (RFC 9534).
277277
/// When set, includes a Micro-session ID TLV in test packets.
278-
#[clap(long, value_parser = clap::value_parser!(u16).range(1..))]
278+
/// Accepts decimal (e.g. `255`) or `0x`-prefixed hex (e.g. `0xff`).
279+
#[clap(long, value_parser = parse_u16_nonzero_dec_or_hex)]
279280
pub micro_session_id: Option<u16>,
280281

281282
/// Reflector member link ID for LAG micro-sessions (RFC 9534).
282283
/// When set, the reflector fills this ID into reflected Micro-session ID TLVs.
283-
#[clap(long, value_parser = clap::value_parser!(u16).range(1..))]
284+
/// Accepts decimal (e.g. `171`) or `0x`-prefixed hex (e.g. `0xab`).
285+
#[clap(long, value_parser = parse_u16_nonzero_dec_or_hex)]
284286
pub reflector_member_link_id: Option<u16>,
285287

286288
/// Maximum packets per second per source (0 = unlimited).
@@ -706,6 +708,29 @@ pub fn is_auth(mode: AuthMode) -> bool {
706708
mode.is_authenticated()
707709
}
708710

711+
/// clap value_parser: parse a u16 from decimal or `0x`-prefixed hex, rejecting 0.
712+
///
713+
/// Accepts: `255`, `0xff`, `0XFF`, `0x00ab`. Rejects: `0`, `0x0`, empty, `ff`,
714+
/// out-of-range. Used by LAG identifier flags where the RFC 9534 wire field is
715+
/// commonly written in hex.
716+
fn parse_u16_nonzero_dec_or_hex(s: &str) -> Result<u16, String> {
717+
let trimmed = s.trim();
718+
let parsed = if let Some(rest) = trimmed
719+
.strip_prefix("0x")
720+
.or_else(|| trimmed.strip_prefix("0X"))
721+
{
722+
u16::from_str_radix(rest, 16).map_err(|e| format!("invalid hex value `{s}`: {e}"))?
723+
} else {
724+
trimmed
725+
.parse::<u16>()
726+
.map_err(|e| format!("invalid value `{s}`: {e}"))?
727+
};
728+
if parsed == 0 {
729+
return Err(format!("value `{s}` must be in range 1..=65535"));
730+
}
731+
Ok(parsed)
732+
}
733+
709734
#[cfg(test)]
710735
mod tests {
711736
use clap::Parser;
@@ -1542,6 +1567,80 @@ mod tests {
15421567
assert!(err.to_string().contains("reflector_member_link_id"));
15431568
}
15441569

1570+
#[test]
1571+
fn test_parse_u16_nonzero_dec_or_hex_accepts_decimal() {
1572+
assert_eq!(parse_u16_nonzero_dec_or_hex("1").unwrap(), 1);
1573+
assert_eq!(parse_u16_nonzero_dec_or_hex("255").unwrap(), 255);
1574+
assert_eq!(parse_u16_nonzero_dec_or_hex("65535").unwrap(), 65535);
1575+
}
1576+
1577+
#[test]
1578+
fn test_parse_u16_nonzero_dec_or_hex_accepts_hex() {
1579+
assert_eq!(parse_u16_nonzero_dec_or_hex("0x1").unwrap(), 1);
1580+
assert_eq!(parse_u16_nonzero_dec_or_hex("0xff").unwrap(), 255);
1581+
assert_eq!(parse_u16_nonzero_dec_or_hex("0xFF").unwrap(), 255);
1582+
assert_eq!(parse_u16_nonzero_dec_or_hex("0X00ab").unwrap(), 0xab);
1583+
assert_eq!(parse_u16_nonzero_dec_or_hex("0xffff").unwrap(), 65535);
1584+
}
1585+
1586+
#[test]
1587+
fn test_parse_u16_nonzero_dec_or_hex_rejects_zero() {
1588+
assert!(parse_u16_nonzero_dec_or_hex("0").is_err());
1589+
assert!(parse_u16_nonzero_dec_or_hex("0x0").is_err());
1590+
assert!(parse_u16_nonzero_dec_or_hex("0x0000").is_err());
1591+
}
1592+
1593+
#[test]
1594+
fn test_parse_u16_nonzero_dec_or_hex_rejects_garbage() {
1595+
assert!(parse_u16_nonzero_dec_or_hex("").is_err());
1596+
assert!(parse_u16_nonzero_dec_or_hex("ff").is_err()); // hex without 0x prefix
1597+
assert!(parse_u16_nonzero_dec_or_hex("0x1g").is_err());
1598+
assert!(parse_u16_nonzero_dec_or_hex("0x10000").is_err()); // > u16::MAX
1599+
assert!(parse_u16_nonzero_dec_or_hex("65536").is_err());
1600+
// Empty string after stripping `0x` prefix → from_str_radix rejects.
1601+
assert!(parse_u16_nonzero_dec_or_hex("0x").is_err());
1602+
assert!(parse_u16_nonzero_dec_or_hex("0X").is_err());
1603+
}
1604+
1605+
#[test]
1606+
fn test_parse_u16_nonzero_dec_or_hex_handles_whitespace() {
1607+
// clap doesn't usually pass whitespace, but the parser trims defensively
1608+
// (e.g. when values are loaded from the TOML config file).
1609+
assert_eq!(parse_u16_nonzero_dec_or_hex(" 0xff").unwrap(), 0xff);
1610+
assert_eq!(parse_u16_nonzero_dec_or_hex("0xff ").unwrap(), 0xff);
1611+
assert_eq!(parse_u16_nonzero_dec_or_hex(" 255 ").unwrap(), 255);
1612+
assert_eq!(parse_u16_nonzero_dec_or_hex("\t0x1\n").unwrap(), 1);
1613+
}
1614+
1615+
#[test]
1616+
fn test_micro_session_id_accepts_hex_on_cli() {
1617+
let conf = load_from_args(&[
1618+
"test",
1619+
"--remote-addr",
1620+
"127.0.0.1",
1621+
"--micro-session-id",
1622+
"0xff",
1623+
"--reflector-member-link-id",
1624+
"0xab",
1625+
])
1626+
.unwrap();
1627+
assert_eq!(conf.micro_session_id, Some(0xff));
1628+
assert_eq!(conf.reflector_member_link_id, Some(0xab));
1629+
}
1630+
1631+
#[test]
1632+
fn test_micro_session_id_accepts_decimal_on_cli() {
1633+
let conf = load_from_args(&[
1634+
"test",
1635+
"--remote-addr",
1636+
"127.0.0.1",
1637+
"--micro-session-id",
1638+
"255",
1639+
])
1640+
.unwrap();
1641+
assert_eq!(conf.micro_session_id, Some(255));
1642+
}
1643+
15451644
#[test]
15461645
fn test_validate_rejects_return_path_cc_with_sr_mpls_from_file() {
15471646
let dir = tempfile::tempdir().unwrap();

src/receiver/mod.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,10 +1088,12 @@ fn apply_semantic_tlv_processing(
10881088
}
10891089
});
10901090

1091-
// Compute fresh HMAC for response (must be last, after all TLV mutations)
1091+
// Compute fresh HMAC for response (must be last, after all TLV mutations).
1092+
// Use the reflector variant so the regenerated HMAC TLV carries U=0 per
1093+
// RFC 8972 §4.4.1 — the reflector recognizes the HMAC type by construction.
10921094
if let Some(key) = tlv_hmac_key {
10931095
let response_seq_bytes = &base_bytes[..4];
1094-
tlvs.set_hmac(key, response_seq_bytes);
1096+
tlvs.set_hmac_response(key, response_seq_bytes);
10951097
}
10961098

10971099
Some(SemanticResult {
@@ -3055,10 +3057,14 @@ mod tests {
30553057

30563058
let rp_tlv = ReturnPathTlv::with_return_address("10.0.0.5".parse().unwrap());
30573059

3060+
let mut raw = rp_tlv.to_raw();
3061+
// Simulate post-clear state (apply_reflector_flags has already run);
3062+
// sender default is U=1 per RFC 8972 §4.4.1, but the U-flag toggle
3063+
// tested here is the send-path "set after clear" path.
3064+
raw.clear_reflector_flags();
30583065
let mut data = sender_packet.to_bytes().to_vec();
3059-
data.extend_from_slice(&rp_tlv.to_raw().to_bytes());
3066+
data.extend_from_slice(&raw.to_bytes());
30603067

3061-
// U-flag should not be set initially
30623068
assert_eq!(data[UNAUTH_BASE_SIZE] & 0x80, 0);
30633069

30643070
let updated = set_return_path_u_flag_in_response(&mut data, UNAUTH_BASE_SIZE);

src/receiver/pnet.rs

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -372,10 +372,8 @@ fn handle_packet(ethernet: &EthernetPacket, config: &CaptureConfig, send_ctx: &P
372372
// (including options); the draft reflects only
373373
// the fixed 20-byte header, so clamp to that.
374374
let ipv4_bytes = header.packet();
375-
let fixed_len = std::cmp::min(
376-
ipv4_bytes.len(),
377-
crate::tlv::IPV4_FIXED_HEADER_SIZE,
378-
);
375+
let fixed_len =
376+
std::cmp::min(ipv4_bytes.len(), crate::tlv::IPV4_FIXED_HEADER_SIZE);
379377
let captured = super::CapturedHeaders {
380378
fixed_header: ipv4_bytes[..fixed_len].to_vec(),
381379
ipv6_ext_headers: Vec::new(),
@@ -403,13 +401,9 @@ fn handle_packet(ethernet: &EthernetPacket, config: &CaptureConfig, send_ctx: &P
403401
// (NextHeader=0) or Destination Options (NextHeader=60)
404402
// extension headers for TLV Types 247/246.
405403
let ipv6_bytes = header.packet();
406-
let fixed_len = std::cmp::min(
407-
ipv6_bytes.len(),
408-
crate::tlv::IPV6_FIXED_HEADER_SIZE,
409-
);
404+
let fixed_len = std::cmp::min(ipv6_bytes.len(), crate::tlv::IPV6_FIXED_HEADER_SIZE);
410405
let fixed_header = ipv6_bytes[..fixed_len].to_vec();
411-
let (ext_headers, final_next, payload_offset) =
412-
extract_ipv6_ext_headers(&header);
406+
let (ext_headers, final_next, payload_offset) = extract_ipv6_ext_headers(&header);
413407

414408
if final_next == IpNextHeaderProtocols::Udp {
415409
let payload = &ipv6_bytes[payload_offset..];
@@ -482,9 +476,7 @@ fn extract_ipv6_ext_headers(
482476
// Emit: previous NextHeader byte | HdrExtLen byte | remaining option bytes.
483477
out.push(next_header_byte);
484478
out.push(hdr_ext_len);
485-
out.extend_from_slice(
486-
&payload[offset_in_payload + 2..offset_in_payload + ext_len_bytes],
487-
);
479+
out.extend_from_slice(&payload[offset_in_payload + 2..offset_in_payload + ext_len_bytes]);
488480

489481
next_header_byte = this_next;
490482
offset_in_payload += ext_len_bytes;

src/sender.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ use crate::{
2828
DestinationNodeAddressTlv, DirectMeasurementTlv, ExtraPaddingTlv, FollowUpTelemetryTlv,
2929
LocationTlv, MicroSessionIdTlv, RawTlv, ReflectedControlTlv, ReflectedFixedHdrTlv,
3030
ReflectedIpv6ExtHdrTlv, ReturnPathTlv, SyncSource, TimestampInfoTlv, TimestampMethod,
31-
TlvList, TypedTlv,
31+
TlvList, TypedTlv, DEFAULT_IPV6_EXT_HDR_REQUEST_CAPACITY, IPV4_FIXED_HEADER_SIZE,
32+
IPV6_FIXED_HEADER_SIZE,
3233
},
3334
};
3435

@@ -271,12 +272,18 @@ pub async fn run_sender(
271272
// reflector populates them when it has raw-capture access to IP headers,
272273
// or echoes them with U-flag otherwise.
273274
if conf.reflected_fixed_hdr {
274-
extra_tlvs.push(ReflectedFixedHdrTlv::request().to_raw());
275-
log::info!("Reflected Fixed Header TLV (Type 247) requested");
275+
extra_tlvs.push(ReflectedFixedHdrTlv::request_for(conf.remote_addr).to_raw());
276+
let bytes = if conf.remote_addr.is_ipv4() {
277+
IPV4_FIXED_HEADER_SIZE
278+
} else {
279+
IPV6_FIXED_HEADER_SIZE
280+
};
281+
log::info!("Reflected Fixed Header TLV (Type 247) requested ({bytes} header bytes)");
276282
}
277283
if conf.reflected_ipv6_ext_hdr {
278-
extra_tlvs.push(ReflectedIpv6ExtHdrTlv::request().to_raw());
279-
log::info!("Reflected IPv6 Ext Header TLV (Type 246) requested");
284+
let cap = DEFAULT_IPV6_EXT_HDR_REQUEST_CAPACITY;
285+
extra_tlvs.push(ReflectedIpv6ExtHdrTlv::request_with_capacity(cap).to_raw());
286+
log::info!("Reflected IPv6 Ext Header TLV (Type 246) requested ({cap}-byte capacity)");
280287
}
281288

282289
// Check if we need to include TLV extensions.

0 commit comments

Comments
 (0)