Skip to content

fix(relay): authenticate relay claims via signed tickets to prevent endpoint cache poisoning - #169

Merged
JamesEjembi merged 1 commit into
VeriNode-Labs:mainfrom
Cyber-Mitch:fix/140-stun-turn-relay-cache-poisoning
Aug 27, 2026
Merged

fix(relay): authenticate relay claims via signed tickets to prevent endpoint cache poisoning#169
JamesEjembi merged 1 commit into
VeriNode-Labs:mainfrom
Cyber-Mitch:fix/140-stun-turn-relay-cache-poisoning

Conversation

@Cyber-Mitch

@Cyber-Mitch Cyber-Mitch commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

closes #140

The STUN/TURN relay endpoint cache accepted binding updates from any peer with no relay-origin authentication. This adds a signed-ticket scheme so only an authorized relay can write a given target's cache entry, plus a sliding-window blacklist that actually evicts an offending relay's existing poisoned entries.

The issue's navigation guide is entirely stale — this is a greenfield module

None of the four named paths (src/net/relay/endpoint-cache.rs, relay-registry.rs, stun-bind.rs, src/attestation/relay-ticket.rs) exist. grep -rniE '\bstun\b|\bturn\b|endpoint.?cache|relay.?ticket|relay.?registry' src tests returns zero matches — there is no STUN/TURN, NAT-traversal, or endpoint-cache code anywhere in this repo today. This PR builds the module fresh, mapping the issue's src/net/ onto the repo's actual src/network/ (the existing wire-codec home) rather than creating a parallel top-level domain, and keeping the issue's hyphenated filenames since that's a real, existing convention here (#[path]-wired, e.g. src/db/mod.rs:6).

Deviation from the issue's own blueprint — it had a hole

The blueprint signs (relay_id, target_id, epoch, expiry). That authenticates who may write which cache key, but leaves the endpoint value itself unsigned — so an attacker who captures or relays one genuine ticket could swap the reflexive address in the STUN response and poison the cache under a fully-valid signature. That's the exact traffic-misdirection attack this issue exists to stop, just moved one field over.

Fixed by binding the endpoint into the signed tuple: (relay_id, target_id, endpoint, epoch, expiry) — a strict superset of the blueprint, +19 bytes, no downside. Mutation-proven load-bearing: removing this binding (mutation M5) lets a "reflexive address swapped, genuine ticket kept" attack straight through undetected.

Crypto: SHA-256 keyed MAC, not Ed25519 — a deliberate substitution

This crate is no_std/WASM with exactly two dependencies (dlmalloc, soroban-sdk); Ed25519 appears only as a transitive dependency of soroban-env-host's test path, never imported by crate code. src/attestation/verifier.rs already documents a deliberate zero-crypto-dependency stance and establishes the substitute in use elsewhere in this crate: sign(key, root) = SHA-256(key || signing_root), constant-time compared.

Reused that model rather than introducing the crate's first real crypto dependency. Extracted sign_root/verify_root from verifier.rs and made ct_eq public (zero behavior change to existing attestation signing — confirmed via the untouched suites below) instead of duplicating a second constant-time comparator.

Trade-off, stated plainly: ticket forgery is now equivalent to a relay-registry compromise — the registry holds the same key material a relay signs with. This is symmetric with the registry's existing trust model (no PKI, no revocation today — registry membership is simply assumed trusted, a real pre-existing gap noted below, not introduced here).

Five load-bearing properties — each independently proven

# Property Standalone test Mutation proof
1 Ticket binding is exact — valid signature for peer A rejected for peer B relay-ticket.rs (verify-for-A-fails-for-B, only reachable with a genuinely valid signature) M4 (disable target-binding) fails the property test and the cache-poisoning test independently; M6 (signature-only verification) lets malicious updates through
2 Expired/stale-epoch/future-epoch tickets rejected as hard as forged ones :618, epoch cases inline M7 (tighten epoch window > to >=) fails legitimate-traffic test with a specific round/relay identified; flipping_any_signed_byte proves expires_at is actually in the signed root
3 Penalty counter is a genuine sliding 60s window, not a monotonic counter or fixed bucket aging, threshold, pacing, release cases M1 (prune → no-op, i.e. the forbidden monotonic counter) fails; M2 (fixed per-minute bucket) fails the boundary-pacing test specifically
4 Blacklisting a peer actually evicts existing entries, not just blocks future writes blacklisting_deletes_the_entries_the_offender_already_wrote — verified by reading the cache back, not asserting a flag M3 (evict nothing / soft flag) fails; M8 (indiscriminate eviction, wiping an unrelated relay's entries too) fails
5 Cache size (10,000) and per-peer (16) caps enforced independently of poisoning logic capacity-rejection tests; a capacity rejection is confirmed to NOT feed the penalty counter covered by the capacity test suite directly

Fuzz test — and a self-caught flaw in my first version

Required 1,000-forged-ticket fuzz test: 0 successful poisonings. Broken down across nine attack classes (wrong signature, wrong target, expired, stale epoch, future epoch, swapped endpoint, replayed ticket, unregistered relay, etc.), deterministic via a seeded SHA-256 counter-mode stream (seed 0x140, reproducible). Verdict is the cache's actual contents — a record-by-record BTreeMap comparison against a pre-run snapshot, not just checking return values. Companion: 2,000 legitimate updates (800 arriving a full epoch late) → 0 rejected, 0 blacklisted — proving the fix isn't over-aggressive.

Worth being honest about: my first version of this fuzz test passed while proving nothing about Property 1. Two bugs — attacker identities weren't registered, so six of the nine attack classes never reached the check they were meant to exercise; and the assertion loop only checked Err generically rather than which specific error fired. Fixed both: attackers are now registered relays attempting to claim targets they can't speak for, and each attack class declares its required rejection error explicitly. Flagging this because a fuzz test that passes for the wrong reason is worse than no fuzz test — it looks like coverage while providing none.

Changed/new files

Cargo.toml | 2 ++
src/attestation/mod.rs | 6 ++++++
src/attestation/verifier.rs | sign_root/verify_root extracted, ct_eq made pub
src/network/mod.rs | registration
src/attestation/relay-ticket.rs | 851 (new)
src/network/relay/endpoint-cache.rs | 1151 (new)
src/network/relay/relay-registry.rs | 128 (new)
src/network/relay/stun-bind.rs | 480 (new)
src/network/relay/mod.rs | 25 (new)
tests/network/relay_endpoint_cache_poisoning_test.rs | 643 (new)

Scope held exactly to the issue's file set plus unavoidable module wiring and the one approved verifier.rs extraction. Nothing else touched.

Verification

Check Result
cargo build --workspace ✅ 0 errors
cargo fmt --all -- --check
cargo clippy --all-targets --all-features -- -D warnings
cargo test --workspace ✅ 0 failures — 888 baseline → 926 (+37, +the fuzz test)

Zero behavior change to existing attestation code, confirmed via untouched suites: domain_separation_test 5/5, attestation_key_rotation_test 5/5, bls_comprehensive_test 19/19.

cargo-llvm-cov --fail-under-lines 80 (CI's coverage gate) was not run locally — the tool wasn't installed in this environment. New code is ~3,278 lines, of which ~1,600 are tests covering every branch; expected to clear the threshold comfortably, but flagging that this specific gate should be confirmed by CI rather than assumed.

Found, not fixed — flagging for the maintainer

  1. Relay registry has no PKI, no signed enrollment, no revocation. "Authoritative" today means simply "a key present in the registry map." Registry membership is assumed trusted. This PR's ticket scheme is only as strong as that assumption — a pre-existing gap, out of this issue's scope, but worth its own issue.
  2. Residual flood surface: one registered relay minting valid tickets for up to 10,000 distinct targets is still possible within the per-target cap — tested and bounded by the total cache cap, but worth noting as a softer DoS surface than outright poisoning, since it's rate-limited by the total-entries cap rather than prevented outright.
  3. "per-peer (16)" in the issue was read as "per target peer" (max 16 cached endpoints per target), not "per submitting relay" — the reading that makes the cap meaningful against an endpoint flood. Flagging in case the intended semantics differ.

@JamesEjembi
JamesEjembi merged commit dc4b334 into VeriNode-Labs:main Aug 27, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

STUN/TURN Endpoint Cache Poisoning via Malicious Peer Relay Claims

2 participants