diff --git a/plan/adversarial.md b/plan/adversarial.md index 4be85f90..7adebf17 100644 --- a/plan/adversarial.md +++ b/plan/adversarial.md @@ -53,17 +53,6 @@ A sibling zone (`evilfoo.bar` against `foo.bar`) and a suffix sharing no label boundary are the two shapes `containment_respects_label_boundaries` and `a_suffix_is_not_containment` already refuse in isolation. -- [x] **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 - call to the containment function is refused at the point a real - attacker would submit it — a DID document lookup, a handle resolution, - or a vouch naming a hostname just outside the zone. Also covers a - hostname containing the zone as a substring rather than a suffix, an - uppercase sibling (proving `validate_handle` refuses it outright rather - than a silent lowercase fold), and the overlapping-zone case reached - through `ZoneManager::add_zone` rather than `ZoneRegistry::new` - directly. See `crates/didbot/tests/zone_containment.rs`. - [ ] **The two containment checks agree.** `didbot-identity` and vibescrobble.com's index each implement containment against the rule described in `agent-accounts.md`'s Done list — "compared label by @@ -77,37 +66,41 @@ label boundary are the two shapes `containment_respects_label_boundaries` and ## DPoP binding: the seam between mint and verify -- [x] **A proof minted for one key, replayed with a different key's - signature but the original `jti`.** `dpop_seam.rs`'s replay-window unit - test presumably reuses the same key; the adversarial case is the - combination — replay *and* a key swap in the same attempt — which is - the shape an attacker who captured a proof but not the private key - would actually try. See - `crates/didbot-serve/src/tests.rs`'s `dpop_binding` module. -- [x] **A token minted with `cnf.jkt` bound to key A, presented with a valid, - well-formed proof signed by key B.** Reframed by what was actually - found: no resource route consuming an OAuth access token exists yet - (`crate::auth::Credential` has no DPoP variant), and - `OAuthTokenStore::validate_access` — the only function that answers - "which account does this token authenticate as" — takes no proof - parameter at all. The test proves the sharper version of this item: a - bare stolen token authenticates with *zero* proof of possession, key B - or otherwise. See - `a_bare_stolen_access_token_authenticates_with_no_proof_of_possession_at_all`. -- [x] **The nonce and replay window survive a request that crosses - `authorize.rs` and `token.rs`.** No router this crate builds wires - `dpop_seam::DpopVerifier` to the real `dpop::DpopVerifier` yet - (`OAuthState::default` uses `RefusingDpopVerifier`), so every real - `/oauth/token` request refuses before authorize.rs is reachable at - all; see `the_servers_htu_never_matches_what_a_conformant_client_signs` - for why, once that one-line wiring is done, it would refuse *every* - request regardless. The replay window itself is proven across two - `OAuthTokenStore` calls sharing one verifier (mint, then refresh) in - the same test as the first item above. - ## Stamp forgery: the seam between the model's tool call and the hook's check -- [x] **A model supplying its own `_vibescrobble*` fields, inside a real tool +## Done + +- **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 + call to the containment function is refused at the point a real + attacker would submit it — a DID document lookup, a handle resolution, + or a vouch naming a hostname just outside the zone. Also covers a + hostname containing the zone as a substring rather than a suffix, an + uppercase sibling (proving `validate_handle` refuses it outright rather + than a silent lowercase fold), and the overlapping-zone case reached + through `ZoneManager::add_zone` rather than `ZoneRegistry::new` + directly. See `crates/didbot/tests/zone_containment.rs`. +- **A proof minted for one key, replayed with a different key's + signature but the original `jti`.** The adversarial case is the + combination — replay *and* a key swap in the same attempt — which is + what an attacker holding a captured proof but not the private key + would actually try. Refused. See `crates/didbot-serve/src/tests.rs`'s + `dpop_binding` module. +- **A token minted with `cnf.jkt` bound to key A, presented with a valid, + well-formed proof signed by key B.** Found first as something sharper: + `OAuthTokenStore::validate_access` took no proof parameter at all, so a + bare stolen token authenticated with zero proof of possession. Its + signature now requires a verified thumbprint, so the unsafe call does + not compile, and the token's `cnf.jkt` is checked against it on the + repo write path. +- **The nonce and replay window survive a request that crosses + `authorize.rs` and `token.rs`.** Reachable now that `OAuthState` + wires the real verifier: `dpop_seam.rs`'s note that the replay window + and the clock-skew window are the same number holds across a proof that + has travelled both stages, rather than one fed to a single verifier + call. +- **A model supplying its own `_vibescrobble*` fields, inside a real tool call the harness's hook intercepts**, not a call the test constructs with the forged fields pre-attached to `tool_input`. The distinction matters because `stamp_conflict`'s own documentation (per @@ -120,7 +113,7 @@ label boundary are the two shapes `containment_respects_label_boundaries` and subprocess (`CARGO_BIN_EXE_didbot-hook`), the actual process boundary a model's tool call crosses. See `crates/didbot-hookd/tests/stamp_forgery.rs`. -- [x] **A forged value that matches the format of a genuine one.** A model +- **A forged value that matches the format of a genuine one.** A model that has seen its own previous scrobbles knows what `_didbotAgentDid` looks like; a forged DID string that is syntactically well-formed and resembles the agent's own is the adversarial case, not an obviously @@ -128,7 +121,7 @@ label boundary are the two shapes `containment_respects_label_boundaries` and `_vibescrobbleParentDid` names a real, currently-provisioned DID belonging to a second, separately-minted account, not merely a well-formed string. -- [x] **The MCP server's own record of what the hook said, versus what +- **The MCP server's own record of what the hook said, versus what arrives at the write path.** [stamp-contract](stamp-contract.md) is about the *contract* between the hook and the scrobble server; this epic's job is a test that drives a full round trip — tool call, hook @@ -140,17 +133,14 @@ label boundary are the two shapes `containment_respects_label_boundaries` and trip to a real written record and checks its DID against the harness's own value. -## Done - - Zone containment adversarial suite added (`crates/didbot/tests/zone_containment.rs`); see that section above for what it covers. - DPoP binding adversarial suite added (`crates/didbot-serve/src/tests.rs`'s - `dpop_binding` module); see that section above for what it found. - `crates/didbot-serve/src/oauth/dpop_seam.rs` is still wired to - `RefusingDpopVerifier` in every router this crate builds, and - `OAuthTokenStore::validate_access` still takes no DPoP proof — both - documented as findings on the tests that prove them, not fixed here. + `dpop_binding` module). It found three things — the verifier was never + wired, the server's `htu` could not match a conformant client's, and + `validate_access` took no proof — and all three are closed; see that + section above. - Stamp forgery adversarial suite added (`crates/didbot-hookd/tests/stamp_forgery.rs`); see that section above for what it covers. diff --git a/plan/oauth.md b/plan/oauth.md index c2ea754c..36cbd85d 100644 --- a/plan/oauth.md +++ b/plan/oauth.md @@ -65,10 +65,16 @@ Without them an authorization server can only approve everything. `plan/scope-policy.md`'s ceiling at `authorize`, and the consent seam's own policy check — refuses by construction until those epics land; see `oauth::par::RefuseAllAdmission`, `oauth::authorize::RefuseAllScopePolicy` - and `oauth::consent::RefuseAllConsent`. DPoP verification - (`oauth::dpop_seam::DpopVerifier`) is a seam for the same reason: no - token can be minted until a real verifier is wired in, because this - profile's tokens must be DPoP-bound. + and `oauth::consent::RefuseAllConsent`. +- [x] **DPoP, wired end to end.** `OAuthState::default` builds the real + verifier (`oauth::dpop::DpopVerifier`, via its `impl + oauth::dpop_seam::DpopVerifier`), `/oauth/token` binds a proof's `htu` + to this deployment's own zone service document rather than a header + (`oauth::discovery::request_htu`), and `OAuthTokenStore::validate_access` + requires the caller to supply a proven thumbprint, not a bare token + string. `com.atproto.repo.*`'s write routes accept a DPoP-bound access + token now, through `auth::require_agent_token_or_dpop`. See + `plan/adversarial.md`'s DPoP-binding tests for the seam this closed. - [x] **Pick an access-token lifetime and write it down.** The profile says under 30 minutes, under 15 without individual revocation, 5 recommended. The number decides how fast pausing issuance drains a swarm.