feat(mirror): verify a peer's mirror-coin bond against chain on the download path - #467
feat(mirror): verify a peer's mirror-coin bond against chain on the download path#467MichaelTaylor3d wants to merge 10 commits into
Conversation
loop-security — IN PROGRESS, not the verdictAudited head: Posting as I go so nothing is lost to a stall. This is not the verdict. Finding 1 (LEAD) —
|
loop-security — IN PROGRESS (2/3), not the verdictHead still Finding 2 — the "cheap lookup" budget now buys up to ~170x its calibrated cost. Remote chain-source amplification.The per-requestor miss limiter runs in the right PLACE but is now sized for the wrong WORK.
This PR makes that same admitted lookup perform, per located record, up to two blocking chain RPCs How big the slate is, and who chooses it. The arithmetic. One admitted miss -> up to ~84 records -> up to ~84 The memoisation does not save it, and the attacker controls that too. Latency, on top of the read cost. The loop is sequential and each read is Severity HIGH. This is the amplification shape, not a perf note: a small remote request elicits large Finding 3 (MEDIUM) — a per-record disk read+parse that the cache cannot absorb
Still to do: mutation spot-check of the three failure directions, the |
Correction to the line citations in my previous commentSome
Correct as written, re-checked: |
loop-security — IN PROGRESS (3/3), not the verdictHead still Check 4 — the
|
loop-security — Finding 4, and it is the one that decides the gate (still IN PROGRESS)Head Finding 4 (HIGH) — the coin id is unauthenticated hearsay about a THIRD PARTY, so the ranking is steerable in both directionsThe PR's central safety argument is that reordering is a safe primitive because it "drops nothing": a lying
Exploit. An attacker positioned to answer one
Result: the attacker sets the rank of every holder in the slate, in both directions, for the cost of Why the existing hearsay mitigation does not cover this. So the answer to "is reordering enough for a first implementation?" is not that it is too weak. It is the The two fixes are different, and both are needed
This reframes the limitation the lane declared. The missing |
loop-security — VERDICT: CHANGES-REQUIREDAudited head: Method: grep + direct read. Mutation evidence (checks 1 and 2, both confirmed)Baseline
The other two failure directions are structurally verified rather than mutated: an ABSENT pointer returns What is right, and should not be relitigated
The problem is not the remedy. It is that the remedy's INPUT is attacker-controlled in both directions. GATING findings1. HIGH — the coin id is unauthenticated hearsay about a THIRD PARTY: a demotion primitive against any honest holder
State: honest holder H serves capsule The existing hearsay mitigation does not cover this. Fix: act on the pointer only when the record is AUTHORITATIVE (mTLS-verified announcement, or 2. HIGH —
|
loop-security — audit of the credit-only decision (NOT an implementation)Head read: I did not write code and will not. This gate's value is that it is independent of the maker 1. Your open question is RESOLVED: the dial does NOT pin
|
| field | value |
|---|---|
provider_peer_id |
H (honest, bonded) |
unverified_mirror_coin_id |
H's real coin — which declares H |
addresses |
the attacker's addresses |
Every check passes: the coin bonds the content, the coin declares peer H, the record claims peer H.
The record is promoted to first — and it points at the attacker. Because the dial does not pin
(section 1), the attacker then serves the traffic while holding H's rank. The method changed from
"copy the coin id onto my own peer id" to "copy the coin id and the peer id onto my own addresses";
the outcome — an unbonded stranger at top rank — did not.
Why your two tests miss it. Test 1 gives the liar its own peer_id, which is exactly the case
the memo binding fixes. Test 2 is about demotion. Neither varies the ADDRESSES while holding
peer_id and coin id honest, so both pass against a build with this hole wide open. A third test is
required:
a record naming an honest holder's
peer_idand its real coin id, but carrying different
addresses, must not be promoted — assert on the addresses that come back, not on the peer id,
because the peer id is identical in the passing and failing versions.
5. The real remedy costs a dig-dht release — and this is why it does NOT block this PR
The fix is to promote only on an authoritative record (an mTLS-verified announcement, or one
signature-checked through ingest_verified_provider, where dig-gossip's
holdings_announce.rs:568 already enforces SHA-256(provider_spki) == provider_peer_id). The binding
this needs therefore already exists — but dig-node cannot currently see it:
ProviderRecordhas exactly five fields —content_key,provider_peer_id,addresses,
expires_at,unverified_mirror_coin_id— and no provenance marker.- There is no authoritative-only per-content accessor.
find_providersmerges local and discovered
into oneVec(dig-dht-0.15.0/src/service.rs:243,merge_dedup_by_providerat:891);
cached_providersreturns hearsay only.
So the remedy is a release-first dig-dht 0.16 (expose provenance, or an authoritative-only read), the
same cascade shape the decision already accepts for dig-mirror-coin 0.8.0.
Scheduling — this is the part that makes the decision still shippable. The residual becomes live at
exactly the same moment promotion does. While no coin carries a dig-peer: term, nothing is ever
promoted, and an unpromotable layer cannot be captured. So: ship credit-only now; the
authoritative-record restriction must land before or with dig-mirror-coin 0.8.0 adoption, never
after. Track it as a blocker on the 0.8.0 adoption ticket, not as a follow-up.
6. A fork the implementer will hit in the first hour — decide it now
"Bonded is unreachable until 0.8.0" is true because no coin contains a dig-peer: term, not
because 0.7.0 cannot read one: MirrorCoin::urls() already returns that tail today. So the implementer
has two options and one of them is wrong:
- Parse
dig-peer:out ofurls()against 0.7.0 — makes promotion reachable immediately, and
creates a second parser for a formatdig-mirror-coin0.8.0 is about to own. That is the
rival-implementation rule (CLAUDE.md 2.0) violated on day one, on a security-critical parse. - Gate promotion off until 0.8.0's typed accessor exists. RECOMMENDED. The promotion tier is
present, tested, and unreachable because its peer-binding check has no sound source yet. That is
"inert-but-recorded" reached honestly, it avoids the rival parse, and it makes section 5's residual
provably unreachable in the interim rather than merely unlikely.
Whichever is chosen, say which in the PR body, because the two produce identical-looking green
suites and opposite security postures.
7. Finding 3 (amplification): bound it in THIS PR, and the interim makes the case stronger, not weaker
Credit-only does not touch it — the chain reads happen per record regardless of what the lattice does
with the answer. But it changes the cost/benefit sharply, in the direction of acting now:
While promotion is unreachable, every one of those up-to-84 blocking chain reads per locate is pure
cost with literally zero effect on any outcome — and a stranger picks the volume. Shipping a
remotely-triggered fan-out that cannot change a decision is the weakest possible position to defend.
Cheapest bounds, in order of value per line changed:
- Cap the records verified per locate to a small constant in source order. Kills the
attacker-chosen fan-out outright (mirror_bond.rs:122-124). - Move the cache probe ahead of
epoch_and_requirement()(bond_verify.rs:228before:238).
Removes a fullread_to_string+ line-by-lineserde_jsonparse of the epoch file
(collateral.rs:647-667) per record per locate, currently unabsorbable by the cache and executed as
blocking I/O on the async worker. - Evict instead of
cache.clear()(bond_verify.rs:178-179), so a stranger rotating >1024 ids
cannot discard honest verdicts. - Verify concurrently rather than sequentially, removing the N x RTT worker hold (
:254).
(1)+(2)+(3) are small and independent of both the lattice and 0.8.0. If only one lands, make it (1).
What I am handing back
- The decision is right in its core — the memo binding is sound and the credit-only lattice kills
the demotion primitive by construction. I endorse shipping it. - Two corrections: the claim that it "kills both HIGH findings by construction" does not hold for
Finding 2, and the two prescribed tests would not detect the remainder. A third test and the
section 5 restriction are needed, on the 0.8.0 schedule rather than this one. - Your unverified item is now verified: the dial does not pin
peer_id, so credit-stealing would
indeed be traffic redirection rather than a wasted dial once promotion is live. Section 5 is what
keeps that unreachable. - No code was written by me, no failing-then-passing runs to report, and the version/lockfile work
is untouched — that is an implementer lane's job, and it should re-gate against a fresh context
rather than this one.
Keeps this branch's 0.200.0 (main is at 0.198.0) and takes main's Cargo.lock as the base; the lockfile is refreshed in the follow-up commit. `mirror/mod.rs` keeps both sides: this branch's `bond_verify` module and main's `converge_tests`.
|
loop-implementer — progress (still DRAFT, do not merge) Branch Implemented, uncommitted-beyond-the-merge at time of writing:
Environment note: rustc on this host is crashing with Next action: finish the test runs (red-then-green with counts), refresh |
…ming peer (#466) `verdict_for` and `MirrorBondVerifier::verify` now take the claiming peer id. Without it the layer could only ask "does some coin bond this content", which a stranger passes truthfully by republishing an honest holder's coin id under its own record. The ranking becomes credit-only: `Bonded` promotes, and absent / `Unverified` / `Unbonded` are one baseline tier that preserves source order. A disproven pointer can no longer rank a holder below where no pointer would have -- otherwise attaching a bogus coin id to an honest holder's record is a demotion primitive any stranger gets for free. Promotion is gated off at `peer_declaration()` until dig-mirror-coin 0.8.0 exposes a typed `dig-peer:` accessor. `MirrorCoin::urls()` already returns that tail, so it could be parsed here -- and must not be: a second parser for a security-critical format, in the consumer, diverges silently rather than failing to compile. Amplification bounded: at most MAX_VERIFIED_PER_LOCATE (8) chain reads per locate, the verdict cache is probed before the epoch file is read, and cache overflow evicts one entry instead of clearing (a stranger rotating coin ids could otherwise discard every honest verdict). Also corrects peer.rs's claim that a dial fails closed on `peer_id mismatch`: every `expected_peer_id` in dig-gossip is test-only and production merely derives (DIG-Network/dig-gossip#85), so a split identity is not caught by the handshake.
…ce control survives `chain_bond_verdict` answers "does this coin bond this content"; `verdict_for` adds "and does it name the peer claiming it". The split keeps `tests/mirror_bond_verify.rs`'s `Bonded` control meaningful -- routed through `verdict_for` the honest coin and a coin nobody could look up would both answer `Unverified`, so every negative case would be equally explained by a fixture too broken to verify at all. Adds a test on the SAME on-chain fixture asserting both halves: the chain establishes the bond, and the claim is still not promoted to a peer the coin does not name.
|
Update — head 1. 2. A new test on the same on-chain fixture asserts both halves at once: Runs after the split, all unpiped,
Still DRAFT. The gate that produced the audit should re-audit this in a fresh context; it declined to implement, correctly. |
# Conflicts: # Cargo.lock # Cargo.toml
|
loop-implementer (resumed lane) — progress. Still DRAFT, do not merge. The prior lane died at the session cap with everything pushed and nothing uncommitted — its worktree was clean at What this session changed
Head is now Verification on the merged tree
Review of the inherited implementation against the two gatesRead against dig-node#473's DECIDED comment and this PR's loop-security audit; no correction was needed.
Next actionReport the lib-suite counts, update the PR body's version and head, and hand back to the orchestrator for a fresh gate round. This lane does not undraft and does not merge. |
1 participant
loop-implementer — the settled fix is implemented. Still DRAFT: this needs a fresh gate round.
Head
e21f617, onloop/mc-verify, merged withorigin/main@3ce4785. Version read fromCargo.tomlon disk after the merge: 0.200.0 (main is at 0.198.0).Cargo.locktaken from main and re-resolved;cargo update -w --offlinelocks 0 further packages.What changed
verdict_forandMirrorBondVerifier::verifytake the claiming peer id. That missing parameter was the root of both HIGH findings: without it the layer could only ask "does some coin bond this content", which a stranger passes truthfully by republishing an honest holder's coin id.Bondedpromotes; absent /Unverified/Unbondedare ONE baseline tier with source order preserved.BondVerdictlost itsOrdderive — the tier is now an explicitcredit_rank, so a future variant cannot silently acquire a ranking by its declaration position.peer_declaration()untildig-mirror-coin0.8.0 exposes a typeddig-peer:accessor, per the gate's section 6. No rival parser overMirrorCoin::urls(); the call site says why.MAX_VERIFIED_PER_LOCATE = 8(source order), the verdict cache is probed beforeepoch_and_requirement()'s file read + JSON parse, and cache overflow evicts one entry instead ofcache.clear()— a stranger rotating coin ids could otherwise discard every honest verdict this node has earned.peer.rs's fail-closed claims corrected. Two comments asserted every dial fails closed onpeer_id mismatch. Everyexpected_peer_idindig-gossipis#[cfg(test)]; production only derives (An outbound dial never checks the presented identity against the claimed peer_id dig-gossip#85). The residual is traffic redirection, not a wasted dial, and the comments now say so rather than telling the next reader the check is already made.SPEC.md§25.6a rewritten to the two-tier credit-only ranking, the coin-proves-the-bond-not-the-bearer rule, the bounded-work rule, and evict-not-clear.Blast radius
impactwas unavailable to this lane (gitnexus MCP tools not in the tool set, and the registered indexes are stale by construction), so the radius was measured by grep + direct read and by compiling every consumer.MirrorBondVerifier::verifyhas exactly three implementations —ChainBondVerifier(production),ByFirstByte(download.rs's engine-path test), and themirror_bondtest doubles — all updated.verdict_forhas one caller.BondRankingLocator::newhas one production call site (NodeContent::new,download.rs:1262).mirror_bondnow re-exports the discovery/download types the seam already speaks in, so a consumer needs no new dependency to implement or test it. Both crates compile and their full lib suites pass.Evidence — every test proven load-bearing by reverting ONLY its fix
Committed first; each revert applied to the committed tree, one at a time, then restored.
-p dig-node-core --lib mirror_bonda_bogus_pointer_leaves_an_honest_holder_exactly_where_no_pointer_would(["bb","aa","cc"]vs["aa","bb","cc"]),an_unreachable_chain_is_unverified_not_unbonded,a_locate_reads_at_most_the_budget_off_the_chain(left: 40, right: 8 — the chain-read COUNT, not a timeout)""instead ofrecord.provider_peer_id-p dig-node-core --lib mirror_bonda_liar_republishing_an_honest_holders_coin_id_is_not_promotedpeer_declarationreports the coin as declaring the claimant-p dig-node-service --lib bond_verifyan_honest_peer_id_with_attacker_addresses_is_not_promotedreturned["attacker.example","honest.example"]-p dig-node-core --lib mirror_bond-p dig-node-core -p dig-node-service --libCounts are stated because a filter matching nothing exits 0 printing
running 0 tests; each run above names a non-zero count.The third test, and where it had to live
The gate was right that neither prescribed test catches the addresses attack — and it turned out the lattice cannot catch it at all. Written first against
mirror_bond, the test failed with the fix in place: the double correctly reportedBonded(honest peer id, honest coin id) and the lattice promoted, exactly as designed. The defence is the promotion gate, which lives on the chain side. Soan_honest_peer_id_with_attacker_addresses_is_not_promotedsits inbond_verify.rsand drives a verifier through the realpeer_declaration()— every chain check answering YES, the production gate answering last. It asserts on the returned addresses, since the peer id is identical in the passing and failing versions.Carried forward, NOT done here
peer_declaration()returning anything butNotReadablemakes the section-5 residual live in the same moment.no_visible_term_promotes_a_claim_before_the_typed_accessor_existsis written to FAIL when that lands, so the authoritative-record restriction cannot be forgotten: it must ship with the 0.8.0 adoption, not after it.Environment note
rustc on this host repeatedly aborted with
STATUS_STACK_BUFFER_OVERRUN(noerror[E…]anywhere), including on unrelated dependency crates, and left a poisonedtracing-appenderartifact that produced 207 bogus "cannot findSend/Vec" errors untilcargo clean -p. Every run above was taken after that, withCARGO_INCREMENTAL=0 -j 2.