fix(relay): authenticate relay claims via signed tickets to prevent endpoint cache poisoning - #169
Merged
JamesEjembi merged 1 commit intoAug 27, 2026
Conversation
…ndpoint cache poisoning (closes VeriNode-Labs#140)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 testsreturns 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'ssrc/net/onto the repo's actualsrc/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 ofsoroban-env-host's test path, never imported by crate code.src/attestation/verifier.rsalready 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_rootfromverifier.rsand madect_eqpublic (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
relay-ticket.rs(verify-for-A-fails-for-B, only reachable with a genuinely valid signature):618, epoch cases inline>to>=) fails legitimate-traffic test with a specific round/relay identified;flipping_any_signed_byteprovesexpires_atis actually in the signed rootblacklisting_deletes_the_entries_the_offender_already_wrote— verified by reading the cache back, not asserting a flagFuzz 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-recordBTreeMapcomparison 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
Errgenerically 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.rsextraction. Nothing else touched.Verification
cargo build --workspacecargo fmt --all -- --checkcargo clippy --all-targets --all-features -- -D warningscargo test --workspaceZero behavior change to existing attestation code, confirmed via untouched suites:
domain_separation_test5/5,attestation_key_rotation_test5/5,bls_comprehensive_test19/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
"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.