From d60a6ec62027d0733a7072e1c9eca46eecc4aff0 Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Wed, 2 Sep 2026 21:38:00 -0400 Subject: [PATCH] docs(plan): record the provisioning race attack, held cases and open leads Documents what the adversarial pass on the provisioning lifecycle found: the caller-asserted-handle race now fixed, three designs that held under concurrency attack (same agent id, naming's own registry, the e-stop/lifecycle gates), and three leads left open for a later pass -- multi-label handles escaping a single-label wildcard, best- effort DNS rollback leaving stray records, and an unbounded hint-then- generate attempt budget. Co-Authored-By: Claude Opus 5 (1M context) Change-Id: I0f15aeb6a6e41c06fedc523ebb0cfa4fc4d2a44a --- plan/adversarial.md | 96 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/plan/adversarial.md b/plan/adversarial.md index be028e7d..fff04732 100644 --- a/plan/adversarial.md +++ b/plan/adversarial.md @@ -136,8 +136,104 @@ Where the remaining risk is, in rough order: the one place in the scope grammar where intersection is not the greatest lower bound its callers assume. +## Provisioning: two writers racing one identity + +`didbot-pds`'s provisioning sequence (`Provisioner::provision`) does a name +reservation, a DID mint, a DNS write, a key generation, two record writes and +a ledger append, several of them irreversible, and had never been attacked +concurrently — only sequentially, the way `crates/didbot-pds/tests/naming.rs` +and `provisioning.rs` already did before this pass. What held and what did +not, from actually racing it with real threads rather than reasoning about it: + +- **Held: two requests minting the same `agent_id`.** `AccountStore::insert` + is a single check-and-insert under one mutex, so exactly one of two + concurrent `provision()` calls for the same DID gets past it. The other's + rollback withdraws the DNS record it believes it just published — the + dangerous case would be that rollback deleting the *winner's* now-live DNS + record — but `InMemoryDns`/`LoopbackDns`/`WildcardDns` all refuse a second + `publish` for one hostname (`DnsError::AlreadyPublished`) rather than + overwriting, so the loser's own `dns.publish` fails first, before it has + touched the store, the ledger or anything worth rolling back. No test + added: this is a property of `Records::insert`'s existing + `publishing_twice_is_a_collision_not_an_overwrite` test, already covering + the mechanism this depends on. +- **Held: naming through `Naming::issue`.** `NameRegistry::claim` checks and + claims a name under one mutex, so two concurrent provisions cannot both + walk away believing they hold the same generated or hinted name. +- **Fixed: two requests minting *different* `agent_id`s for the *same* + caller-asserted handle, with no `Naming` configured.** + `check_requested_handle` scans the account store and the actual claim + (`AccountStore::insert`) happens several steps later — a mint, a keypair, a + DNS publish, a ledger open — far enough apart that two concurrent requests + could both pass the scan before either was stored, minting two DIDs whose + documents both claim one handle: exactly the situation + `two_accounts_may_not_claim_one_handle` asserts is refused, reached through + concurrency instead of sequencing. `handle_did` can then answer for only + one of the two, permanently breaking the bidirectional handle/DID promise + for whichever account it does not answer for, indistinguishably. Closed + with `Provisioner::handle_lock`, serializing the check against the claim + the way `NameRegistry::claim` already does for a deployment that names its + own agents. Regression test: + `two_accounts_racing_for_one_handle_concurrently_still_only_one_wins` in + `crates/didbot-pds/tests/naming.rs`, real threads on a `Barrier`, matching + the pattern `records.rs`'s `a_concurrent_write_between_the_check_and_the_commit_does_not_let_two_batches_win` + already uses for this class of race. Mutation-tested: reverting the lock + (`git stash` the fix) fails the test 5/5 runs; restoring it passes 3/3. +- **Held: the e-stop and lifecycle gates.** `docs/write-pipeline.md`'s + ordering — e-stop before lifecycle before anything is parsed — is exactly + what `crates/didbot-serve/src/routes.rs`'s `provision_agent` does, and + because both checks run before `registry.provision()` is ever called, "a + refusal leaves nothing behind" is trivial rather than tested: nothing was + started. Already covered end to end in `crates/didbot-serve/src/tests.rs` + (Revoke, Pause, and Pause-thrown-by-a-lapsed-operator-claim each refuse + `provisionAgent` with `Halted`, and Pause is shown leaving an + already-issued token alone). + +Left open, in rough order of how much a deployment should care: + +- [ ] **A caller-asserted handle need not be exactly one label below the + zone.** `check_requested_handle` accepts any handle + `hostname_is_at_or_below` the zone, which allows any depth — + `a.b.agents.example` passes as readily as `a.agents.example` — but a + real wildcard DNS record (`*.agents.example`) matches exactly one + label per RFC 1034, so a multi-label handle would never actually route + to this server. `WildcardDns::accepts` in `crates/didbot-dns` mirrors + the overly permissive check (`ends_with`, not "exactly one more + label"), so the fake would not catch it either. The account still + gets minted, its document still claims the handle, and the internal + `document.claims_handle` / `handle_did` agreement check still passes — + none of that notices that no request for the handle would ever arrive. + Fix shape: `check_requested_handle` should refuse a handle with more + than one label past the zone the same way it refuses one outside the + zone; a fix in `didbot-dns` is out of scope for this pass, since two + other sessions are already editing `crates/didbot-dns/` and + `crates/didbot-tls/`. +- [ ] **A rollback's DNS withdraw is best-effort and unaccounted for.** + Every rollback path in `Provisioner::provision` (a failed ledger open, + a failed store insert, a failed registration-record publish) withdraws + the DNS record(s) it published and logs, but does not return, a + withdraw failure. The name is always released regardless — correct, + since `release_name` never touches DNS — but a DNS provider that is + unreachable during rollback (the same outage that plausibly caused the + original failure) leaves a stray record pointing at a DID nothing will + ever serve again, with no queue or sweep that revisits it. Not a + security issue — the record is inert, not a takeover — but worth a + reconciler pass; `Route53Dns::resync`, already flagged above as having + no callers, is the natural place this would be caught if it were ever + invoked. +- [ ] **`Naming::issue`'s hint path and its from-scratch path do not budget + attempts the same way.** A caller-hinted name that collides falls + straight through to the generation loop with a full fresh budget of + `attempts`; nothing bounds the combined cost of "try the hint, then + try `attempts` more", which is fine at today's `DEFAULT_ATTEMPTS = 16` + but is a sharp edge for a deployment that raises it a lot expecting a + linear cost per request. + ## Done +- **Two concurrent provisions racing for one caller-asserted handle.** See + "Provisioning: two writers racing one identity" above for what was + attacked, what held, and what `handle_lock` closes. - **A request naming a sibling zone's hostname, driven through the actual server**, from HTTP request to whichever component decides containment, confirming the same string that would pass a unit test's -- 2.51.2