geoprobe: reject forged, replayed and spoofed geolocation input

[nikw9944]

Four defects in the RFC-16 geolocation chain accepted input that was never authenticated. Each fix has a test that fails without it.

1. Forged offsets were stored and served. geoprobe-target computed signatureValid and then never used it, so an unsigned UDP datagram to port 8923 reached the cache and location_offsets — a table the lake explorer aggregates with no filter on signature_valid, i.e. a public page. Invalid offsets are now dropped outright rather than written to a separate path: the writer has one table, an alternate one would be a schema change (out of scope), and abuse visibility comes from a Warn log instead.

While testing this I found the gate alone was bypassable. Go's ed25519.Verify does not screen the all-zero pubkey: it decodes to a valid order-4 point rather than being rejected, so with an all-zero signature the verification equation holds whenever the message hash lands on the right residue — about one message in four, which an attacker reaches by varying any field. A fully unsigned offset verified. The zero-key guard now covers every place in the measurement chain where a wire-supplied pubkey is used as the verification key: VerifyOffset, and ProbePacket.Verify / ReplyPacket.Verify in tools/twamp/pkg/signed. The fourth ed25519.Verify site, telemetry/state-ingest/pkg/server/auth.go, takes its key from device.MetricsPublisherPubKey after a successful lookup, so it is not attacker-supplied and is left alone.

Severity of the two packet-level sites, traced rather than assumed:

• ProbePacket.Verify at the reflector is not exploitable end to end today. The allowlist check runs before verification, and the geolocation program already rejects a default target_pk for inbound targets (add_target.rs:84), so the zero key cannot reach authorizedKeys. The guard removes the reflector's dependence on an invariant enforced in another program.
• ReplyPacket.Verify has real effect. LinuxSender.tryRecv pins reply.AuthorityPubkey to the expected probe key before verifying, but only on the verify=true path, which is Reply 1. Reply 0 is accepted unverified by design, and geoprobe-target-sender then calls Reply0.Verify() for audit logging — so a spoofed Reply 0 carrying a zero authority key was logged as reply0_sig=true.

2. RFC-16's replay mitigation was not implemented. MeasurementSlot was written and logged but never compared, and the only other freshness bound is receipt wall-clock, which a replay refreshes. The agent now rejects an inbound DZD offset unless its slot is within 15 min behind / 5 min ahead of the current ledger slot. The window comes from the caches on both ends: the DZD stamps offsets from a slot cached for SlotCacheTTL (5 min) and the agent compares against its own 5-min cache, so ±5 min of skew is legitimate before RPC and finalization lag; 15 min of lag leaves room for a degraded RPC without making the check meaningless. If no slot is available at all the offset is rejected — the agent cannot sign composite offsets in that state either.

Separately, offsetCache.Put used <= for best, and replacing best also resets its expiry clock, so a replayed offset held best forever. Now strictly <.

3. Unverified packets mutated per-sender reflector state. target_pk is public onchain, so ~2 spoofed packets per window could consume a paying sender's pair budget, repoint pairSourceIP, and clear its challenge nonce. The reflector verifies before touching state. Replying without verification is still allowed per RFC-16 — that path now runs off a throwaway state, so the nonce it issues authenticates nothing, and is capped at one reply per window per pubkey, since the reply is ~10x the probe size and an unlimited one would be a reflection amplifier.

4. Delinquent targets were measured forever. All-nil returns meant "scan skipped", which a completed-but-empty scan is indistinguishable from, so removing a user's last target or flipping them to Delinquent left the probe measuring until restart. discover now returns an explicit scanned bool.

Also: ICMP replies are matched on source address in addition to ID+seq.

No wire format or Borsh layout changed. RFC: rfc16-geolocation-verification.md — step 6 of the inbound flow is amended, since it stated the reflector does not verify probe signatures.

Testing Verification

• Each fix was reverted individually with its test in place and confirmed failing, then restored and confirmed passing: forged offset reaching the cache and the ClickHouse buffer; a replayed offset (slot outside the window) reaching the cache over a live UDP listener, and an equal-RTT put refreshing best's clock; spoofed probes starving a legitimate sender's pair so its Reply 1 never comes back Challenged; an empty-but-completed scan not propagating on any of the three channels; an ICMP reply from an unprobed host being credited as the target's RTT; an unsigned probe and an unsigned reply verifying.
• That revert check caught two weak tests. Because acceptance of the zero pair is message-dependent, TestVerifyOffset_ZeroPubkeyAndSignature and the first version of the probe test passed with their guard removed — their payloads hashed to a rejecting residue. All three zero-key tests now pin field values (offset slot 2, probe seq 1) that the unguarded code accepts, and each was re-confirmed failing without its guard.
• go test ./controlplane/telemetry/... ./tools/twamp/... — all pass except tools/twamp/pkg/light, which fails identically before my changes in this sandbox (TestTWAMP_Sender_* cannot write to 10.255.255.255: operation not permitted). I did not touch that package.
• golangci-lint run ./controlplane/telemetry/... ./tools/twamp/... clean.

[Copilot]

🟡 Changes recommended

The signed TWAMP reflector can still become a reflection amplifier when verifyInterval is set to 0 because the unverified-probe reply cap does not apply in that configuration.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR hardens the RFC-16 geolocation ingestion and measurement pipeline so that unauthenticated, replayed, and spoofed inputs cannot corrupt cached offsets, published ClickHouse data, or per-sender probe state.

Changes:

• Drop unsigned or invalid signature-chain LocationOffset datagrams before caching or writing to ClickHouse, and reject the all-zero Ed25519 authority pubkey case.
• Enforce replay mitigation for inbound offsets by validating MeasurementSlot against the current ledger slot, and prevent equal-RTT replays from refreshing the “best” cache entry.
• Prevent spoofed signed-TWAMP probes from mutating per-sender pair state, and tighten ICMP RTT attribution by matching reply source address in addition to ID and sequence.

File summaries

File	Description
tools/twamp/pkg/signed/reflector_test.go	Adds regression coverage for spoofed probes not disturbing a legitimate sender’s pair.
tools/twamp/pkg/signed/reflector_linux.go	Verifies probe signatures before touching per-sender pair state; adds unverified-reply handling.
rfcs/rfc16-geolocation-verification.md	Updates inbound flow documentation to reflect reflector verification and state isolation.
controlplane/telemetry/internal/geoprobe/target_discovery.go	Distinguishes “scan skipped” from “scan ran but matched nothing” via an explicit scanned return value.
controlplane/telemetry/internal/geoprobe/target_discovery_test.go	Updates call sites and adds an empty-scan propagation regression test.
controlplane/telemetry/internal/geoprobe/signer.go	Rejects zero authority pubkey prior to Ed25519 verification.
controlplane/telemetry/internal/geoprobe/signer_test.go	Adds coverage for the zero (pubkey, signature) acceptance edge case.
controlplane/telemetry/internal/geoprobe/metrics.go	Adds rejection reason labels for slot freshness enforcement.
controlplane/telemetry/internal/geoprobe/icmp_pinger.go	Matches ICMP replies on source IP to prevent spoofed RTT attribution.
controlplane/telemetry/internal/geoprobe/icmp_pinger_test.go	Adds tests to ensure mismatched-source replies are ignored.
controlplane/telemetry/internal/geoprobe/icmp_conn.go	Extends recv path to return sender IPv4 address from Recvmsg.
controlplane/telemetry/internal/geoprobe/icmp_conn_test.go	Updates tests for new recvEcho return values and validates source address.
controlplane/telemetry/internal/geoprobe/clickhouse.go	Adds BufferedRows() to support assertions for “dropped vs recorded” behavior in tests.
controlplane/telemetry/cmd/geoprobe-target/main.go	Drops invalid signature-chain offsets before cache and ClickHouse write.
controlplane/telemetry/cmd/geoprobe-target/main_test.go	New tests ensuring forged offsets do not reach cache or ClickHouse buffer.
controlplane/telemetry/cmd/geoprobe-agent/main.go	Enforces MeasurementSlot freshness, adjusts best-cache replacement semantics, and threads slot lookup into listener.
controlplane/telemetry/cmd/geoprobe-agent/main_test.go	Adds unit and listener-path tests for slot-window rejection and equal-RTT non-refresh.
CHANGELOG.md	Documents the geolocation hardening changes and their security rationale.

Review details

• Files reviewed: 18/18 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 76be6cf. Configure here.}

[nikw9944]

Addressed all four review points in 4206585, and rebased onto main (v0.39.0) to replace the Update Branch merge commit with linear history — the rebased tree is byte-identical to that merge commit, and the CHANGELOG entry moved into the new Unreleased section since the release cut turned the old one into v0.39.0.

The substantive one was Bugbot's stale-slot finding: getCurrentSlot falls back to its cache indefinitely on RPC failure, so the replay window I added froze around the last fetched slot — replays near it stayed acceptable for the length of an outage, and fresh offsets would eventually be rejected as too far ahead. Ingestion now rejects a slot reference older than two refresh periods (10m); composite offsets keep the old behavior so the probe still emits during a blip.

Copilot's amplifier point was also correct: the unverified-reply cap keyed off verifyInterval, which is 0 when pair rate limiting is disabled, leaving the path it was meant to protect uncapped. It now has a 1s floor. Both fixes have tests that fail against the previous code.

Chose differently on one: discover still returns scanned: false on an RPC error. Returning true would claim a scan ran when it did not, and discoverAndSend checks err first, so the comment now documents that scanned is meaningful only when err is nil.

go test ./controlplane/telemetry/... ./tools/twamp/... passes except tools/twamp/pkg/light, which fails in my sandbox for lack of raw-socket permissions and is untouched by this branch (git diff origin/main -- tools/twamp/pkg/light is empty).

[nikw9944]

Branch updated for main again (c5815ae5) — main had moved 8 commits and CHANGELOG.md was the only conflict, as before. Main added a CLI entry to the same Unreleased → Changes section; both blocks stay there, since this PR is unmerged and its entry does not belong in a released version. Verified mechanically: my Geolocation block is byte-identical to the previous head, the merged section is exactly that block plus main's verbatim, no entry from either side was dropped, and every released section matches main.

No code changed. For all 19 non-CHANGELOG files this PR touches, git diff 2d863301 HEAD -- <file> is empty.

No new review feedback since the last update — the four inline threads already carry replies, and nothing needing action arrived after them.

Local checks on the merge result: go test -race ./tools/twamp/pkg/signed/ and the four geolocation packages pass; golangci-lint run -c ./.golangci.yaml ./tools/twamp/pkg/signed/... ./controlplane/telemetry/... reports 0 issues. e2e still needs a Docker host and has not been run here.