diff --git a/Cargo.lock b/Cargo.lock index f966ffd3..12db2708 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2665,7 +2665,7 @@ dependencies = [ [[package]] name = "digstore-chain" -version = "0.29.9" +version = "0.30.0" dependencies = [ "aes-gcm", "anyhow", @@ -2707,7 +2707,7 @@ dependencies = [ [[package]] name = "digstore-chunker" -version = "0.29.9" +version = "0.30.0" dependencies = [ "digstore-core", "hex", @@ -2717,7 +2717,7 @@ dependencies = [ [[package]] name = "digstore-cli" -version = "0.29.9" +version = "0.30.0" dependencies = [ "anstream 0.6.21", "anstyle", @@ -2786,7 +2786,7 @@ dependencies = [ [[package]] name = "digstore-core" -version = "0.29.9" +version = "0.30.0" dependencies = [ "aes-gcm-siv", "hex", @@ -2798,7 +2798,7 @@ dependencies = [ [[package]] name = "digstore-crypto" -version = "0.29.9" +version = "0.30.0" dependencies = [ "chia-bls 0.36.1", "digstore-core", @@ -2832,7 +2832,7 @@ dependencies = [ [[package]] name = "digstore-host" -version = "0.29.9" +version = "0.30.0" dependencies = [ "anyhow", "clap", @@ -2857,7 +2857,7 @@ dependencies = [ [[package]] name = "digstore-prover" -version = "0.29.9" +version = "0.30.0" dependencies = [ "bincode 1.3.3", "digstore-core", @@ -2873,7 +2873,7 @@ dependencies = [ [[package]] name = "digstore-remote" -version = "0.29.9" +version = "0.30.0" dependencies = [ "async-trait", "axum", @@ -2902,7 +2902,7 @@ dependencies = [ [[package]] name = "digstore-stage" -version = "0.29.9" +version = "0.30.0" dependencies = [ "digstore-chunker", "digstore-compiler", @@ -2918,7 +2918,7 @@ dependencies = [ [[package]] name = "digstore-store" -version = "0.29.9" +version = "0.30.0" dependencies = [ "digstore-chunker", "digstore-core", @@ -2933,7 +2933,7 @@ dependencies = [ [[package]] name = "digstore-subscription" -version = "0.29.9" +version = "0.30.0" dependencies = [ "async-trait", "digstore-core", diff --git a/Cargo.toml b/Cargo.toml index eb1f3c1d..ff11677d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ exclude = ["crates/digstore-prover/guest", "crates/dig-client-wasm"] [workspace.package] edition = "2021" -version = "0.29.9" +version = "0.30.0" license = "GPL-2.0-only" [workspace.dependencies] @@ -28,17 +28,17 @@ license = "GPL-2.0-only" # The version MUST equal `[workspace.package].version`, because every member inherits # that value. `scripts/check-workspace-dep-versions.sh` enforces it in CI, so a release # bump can never silently leave a published crate pointing at the previous version. -digstore-core = { path = "crates/digstore-core", version = "0.29.9" } -digstore-chain = { path = "crates/digstore-chain", version = "0.29.9" } -digstore-chunker = { path = "crates/digstore-chunker", version = "0.29.9" } -digstore-crypto = { path = "crates/digstore-crypto", version = "0.29.9" } -digstore-store = { path = "crates/digstore-store", version = "0.29.9" } -digstore-prover = { path = "crates/digstore-prover", version = "0.29.9" } -digstore-host = { path = "crates/digstore-host", version = "0.29.9" } -digstore-stage = { path = "crates/digstore-stage", version = "0.29.9" } -digstore-remote = { path = "crates/digstore-remote", version = "0.29.9" } -digstore-cli = { path = "crates/digstore-cli", version = "0.29.9" } -digstore-subscription = { path = "crates/digstore-subscription", version = "0.29.9" } +digstore-core = { path = "crates/digstore-core", version = "0.30.0" } +digstore-chain = { path = "crates/digstore-chain", version = "0.30.0" } +digstore-chunker = { path = "crates/digstore-chunker", version = "0.30.0" } +digstore-crypto = { path = "crates/digstore-crypto", version = "0.30.0" } +digstore-store = { path = "crates/digstore-store", version = "0.30.0" } +digstore-prover = { path = "crates/digstore-prover", version = "0.30.0" } +digstore-host = { path = "crates/digstore-host", version = "0.30.0" } +digstore-stage = { path = "crates/digstore-stage", version = "0.30.0" } +digstore-remote = { path = "crates/digstore-remote", version = "0.30.0" } +digstore-cli = { path = "crates/digstore-cli", version = "0.30.0" } +digstore-subscription = { path = "crates/digstore-subscription", version = "0.30.0" } sha2 = "0.10" proptest = "1" diff --git a/crates/digstore-cli/src/ops/remote_ops.rs b/crates/digstore-cli/src/ops/remote_ops.rs index e529ddf4..21a5eae2 100644 --- a/crates/digstore-cli/src/ops/remote_ops.rs +++ b/crates/digstore-cli/src/ops/remote_ops.rs @@ -385,9 +385,15 @@ pub async fn clone_from( // corrupted module therefore fails closed instead of being installed and // executed. Publisher authorization of the head is checked separately below. let pb = ui.progress_bar(0, "Downloading"); + // Pin the download to the head `fetch` just resolved (SPEC digstore-remote + // §4.7): the server refuses (before this closure or the progress callback + // ever runs) any generation but `remote_root`, so a head that advances + // between `fetch` and this call surfaces as a clean error instead of + // silently installing a newer generation than the one reported above. let (etag_root, module) = client - .clone_store( + .clone_store_at( &store_id, + Some(&remote_root), |bytes, served_root| { let id = digstore_compiler::verify_module_root(bytes, &store_id) .map_err(|e| format!("module identity verification failed: {e:?}"))?; diff --git a/crates/digstore-remote/SPEC.md b/crates/digstore-remote/SPEC.md index 95747076..ecf4adca 100644 --- a/crates/digstore-remote/SPEC.md +++ b/crates/digstore-remote/SPEC.md @@ -21,10 +21,11 @@ change (`CLAUDE.md` §4.2). - The DIG store format + read crypto (`digstore-core`/`-host`/`-crypto`/`-chain`/`-stage`), which this crate consumes and does not reimplement. -This document currently covers the **client→node connection-order + transport contract** -(`src/resolver.rs`) in full; the wire-level `§21` protocol (routes, status codes, auth headers) is -documented inline in `src/server.rs`/`src/client.rs`/`src/wire.rs` doc comments and is summarized -here only where the resolver depends on it (the `/health` route). +This document covers the **client→node connection-order + transport contract** (`src/resolver.rs`, +§1–§2) and the **whole-module read's root pinning** (`GET|HEAD /stores/{id}/module`, §4) in full; +the rest of the wire-level `§21` protocol (routes, status codes, auth headers) is documented inline +in `src/server.rs`/`src/client.rs`/`src/wire.rs` doc comments and is summarized here only where §1 +or §4 depends on it (the `/health` route, the `module` method tag). --- @@ -173,3 +174,237 @@ the CLI's transport-selection code) fails loudly instead of silently changing be - `dig-node/SPEC.md` §2.2 describes the SAME ladder from the perspective of a node that is itself a valid resolution target; the two documents MUST stay in agreement (a change to one that shifts the ladder's behavior updates the other in the same unit of work, per `CLAUDE.md` §4.2 "Layering"). +- A whole-module read (`GET|HEAD /stores/{id}/module`) MUST follow §4: a client that holds the head + pins it with `?root=`, refuses a served root other than the one it asked for BEFORE invoking its + verifier, and never follows a redirect; a `digstore serve` remote honours `?root=` (`422` + malformed → `404` not-served → `200` with `ETag` = the served root, in that precedence) and serves + its head on a rootless read with no redirect. + +--- + +## 4. Whole-module read — root pinning (§21.3 clone, §21.4 pull) + +`GET|HEAD /stores/{id}/module` is the whole-module read: it returns the compiled store module (the +`.dig` WASM bytes) for ONE generation of store `{id}`, identified by that generation's 32-byte merkle +root. This section fixes how a client names the generation it wants (the **root pin**), how the two +server implementations answer, and how the client fails when the answer is not the generation it +asked for. Every failure in this section fails CLOSED: the client never returns bytes labelled with a +root it did not ask for, and a server never substitutes its head for a root a client named. + +Clauses marked **[implemented]** are true of the code at the cited `file:line`; clauses marked +**[pending #1903]** are the contract the #1903 implementation MUST satisfy, and the marker is replaced +by a citation in the same PR that lands the code (§4.9). + +### 4.1 Terms and wire rendering + +- **Generation root** — the 32-byte merkle root of one generation of a store. The store's **served + head** is the root the remote currently serves as its confirmed head (`HeadState::served_root`). +- **`root = None` means the remote's current served head** — for `clone_store`, for + `clone_store_at(.., None, ..)`, and for a rootless server-side `GET|HEAD /module`. It is a request + for "whatever you serve now"; the client learns WHICH root that was only from the response `ETag`. +- **Root on the wire** — in a query string a root is `?root=<64 hex characters>` (32 bytes, + hex-encoded, no quotes). A client MUST emit lowercase hex (`Bytes32::to_hex`). A server MUST accept + lowercase; whether it accepts uppercase is implementation-defined and a client MUST NOT rely on it. +- **`ETag`** — the module's ETag is its generation root rendered as a strong quoted tag `"<64-hex>"` + (`etag_for_root`, `src/etag.rs:5-7`) and is parsed by `parse_if_none_match` (`src/etag.rs:12-16`), + which yields `None` for `*`, weak (`W/`) tags, and any value that is not a quoted 64-hex string. + **[implemented]** + +### 4.2 Client — `DigClient::clone_store_at` and `clone_store` + +```rust +pub async fn clone_store_at( + &self, + store_id: &Bytes32, + root: Option<&Bytes32>, + verify: V, + on_progress: Option<&(dyn Fn(u64, u64) + Send + Sync)>, +) -> Result<(Bytes32, Vec), ClientError> +where + V: FnOnce(&[u8], &Bytes32) -> Result<(), String>; +``` + +1. **Additive surface.** `clone_store_at` is a new public method. `clone_store(store_id, verify, + on_progress)` keeps its exact signature (`src/client.rs:316-323`) and IS + `clone_store_at(store_id, None, verify, on_progress)`. This is a SemVer MINOR change to + `digstore-remote`: no existing caller changes. **[pending #1903]** +2. **Request formation.** `root = Some(r)` → `GET {base}/stores/{id}/module?root=`. `root = None` → `GET {base}/stores/{id}/module` with no query — byte-identical to today's + request (`src/client.rs:325-331`). Both carry the §21.9 auth headers with method tag `module` when + the client has an identity (§4.6). **[pending #1903]** +3. **Redirects are never followed.** The HTTP client is built with `redirect::Policy::none()` + (`src/client.rs:196`) and this MUST NOT change: a redirect is a protocol error, not a hop (the + push-bounce / SSRF guard). Consequently ANY non-2xx status — 3xx included — is returned as + `ClientError::Status(code)` (`src/client.rs:335-337`), and a rootless read against a server that + resolves the head by redirect (the `rpc.dig.net` gateway, §4.5) fails with + `ClientError::Status(307)`. A caller that holds the head — every caller that has run `fetch` — + MUST pass it as `Some(&head)`. **[implemented]** (redirect policy + status mapping; the 307 + consequence is a direct reading of them). +4. **The ETag is the served root.** A 2xx response MUST carry an `ETag` that parses (§4.1) to a root; + missing or unparsable → `ClientError::Verification` (`src/client.rs:338-346`). **[implemented]** +5. **The pin.** When `root = Some(r)`, the parsed ETag root MUST equal `r`; otherwise the client + returns `ClientError::Verification`. This check is made on the response HEADERS, before the body is + read: on a mismatch the body is not downloaded, `on_progress` is not invoked, and `verify` is NOT + invoked. The caller's verifier is the LAST check, not the only one — a server that answers a request + for `r` with a different generation is refused before that generation's bytes reach caller code. + **[pending #1903]** +6. **The verifier.** After the pin check the body is downloaded and `verify(&bytes, &root)` is + invoked exactly once, with the root that will be returned — the served root, which equals `r` when + pinned. `Err(msg)` → `ClientError::Verification(msg)` (`src/client.rs:348-349`). **[implemented]** +7. **What success proves — and does not.** `Ok((root, bytes))` proves only that the remote served + `bytes` labelled `root`, that `root` is the one the caller asked for (when pinned), and that the + caller's `verify` accepted `bytes` against `root`. It does NOT prove that `root` is the store's + current head, that `root` is a confirmed on-chain generation, or that the publisher authorised it — + a caller that needs those facts establishes them from `fetch` (descriptor `current_root`, + `push_sig`, §21.6), as the CLI does (§4.7). + +Failure directions, pinned: a wrong-generation answer is `Verification` (never returned as data); an +absent generation is `Status(404)`; a redirecting server is `Status(3xx)`; a transport failure is +`Transport`. No path in `clone_store_at` returns bytes for a root the caller did not name. + +### 4.3 Client — `pull`'s full-module download (§21.4) + +`pull(store_id, local_root, prefer_delta, on_progress)` (`src/client.rs:359-430`) first runs `fetch` +and derives the remote head `remote_root` from the descriptor's `current_root` +(`src/client.rs:368-370`); the delta path is unchanged by this section. + +1. The full-module request MUST be `GET /stores/{id}/module?root=` — the head `pull` + already holds — instead of today's rootless GET (`src/client.rs:403-407`). **[pending #1903]** +2. `If-None-Match: ""` is sent when `local_root` is `Some`, and its semantics are unchanged + (`src/client.rs:408-413`): a `304` → `PullResult::UpToDate` (`src/client.rs:418-420`). Against a + conforming root-pinning server (§4.4) this branch is VACUOUS: `pull` only reaches the download when + `local_root != remote_root`, and a server honouring `?root=` serves `remote_root` or + 404s, so its served root can never equal `local_root`. The branch is retained because the client + MUST still handle a `304` correctly if a server sends one. **[implemented]** +3. On a 2xx the `ETag` root MUST equal `remote_root`; missing, unparsable or mismatching → + `ClientError::Verification`, and the body is not returned. `PullResult::Module { root, bytes }` + therefore carries a `root` the served `ETag` agreed with, not merely the descriptor's claim. + **[pending #1903]** +4. A head advance between `fetch` and the module GET surfaces on a head-only server (§4.4) as + `ClientError::Status(404)`; the caller re-runs `pull`. This is the intended fail-closed direction: + `pull` never returns bytes for a generation other than the one it reports. + +### 4.4 Server — `RemoteServer` (`digstore serve`) `GET|HEAD /stores/{id}/module` + +`RemoteServer::router()` mounts `get_module` and `head_module` at `/stores/:id/module` +(`src/server.rs:85-88`). Today both ignore any query string and serve the head via +`backend.module_bytes(&store_id, None)` (`src/handlers/module.rs:74`). The contract: + +1. **Optional `root` query.** GET and HEAD both accept an optional `root` query parameter — the same + `?root=` the PUT handler already parses for pushes (`src/handlers/module.rs:192, 214-224`; note the + PUT's `root` is the NEW root being pushed, a different meaning of the same name). **[pending #1903]** +2. **Precedence** — the first matching row wins; a later row is never evaluated: + + | # | condition | response | + |---|---|---| + | 1 | `{id}` is not 64-hex (`parse_store_id`, `src/server.rs:255`) | `400` | + | 2 | `root` present and not a 64-hex string (empty, `latest`, wrong length, non-hex) | `422` (`RemoteError::Validation`, `src/error.rs:42`) — evaluated BEFORE any backend lookup; a malformed root never reaches `head_state` | + | 3 | store unknown | `404` (`RemoteError::UnknownStore`, `src/error.rs:36`) | + | 4 | `root` present, well-formed, and not the root the remote serves for this store | `404` (`RemoteError::UnknownRoot`, `src/error.rs:36`), no `ETag` | + | 5 | GET only: `If-None-Match` parses to the served root (`matches_current`, `src/etag.rs:19-21`) | `304` + `ETag` of the served root | + | 6 | otherwise | `200`, `Content-Type: application/wasm`, `ETag` = the served root; GET carries the module bytes, HEAD carries `Content-Length` and no body | + + Rows 1, 3, 5, 6 are the existing behaviour (`src/handlers/module.rs:15-38, 40-87`); rows 2 and 4 + are **[pending #1903]**. HEAD does not evaluate `If-None-Match` (row 5 is GET-only) — unchanged. +3. **Rootless serves the head, with no redirect.** `root` absent → the served head with `200` + (row 6), exactly as today. A `digstore serve` remote MUST NOT answer a rootless read with a redirect: + the local dev/test flow and every existing rootless client depend on the `200`. **[implemented]** + (`src/handlers/module.rs:74-86`) +4. **Head-only.** A `digstore serve` remote serves exactly ONE module per store over this route — its + served head. `RemoteBackend::module_bytes(id, Some(r))` returns `UnknownRoot` for any + `r != served_root` (trait contract `src/backend.rs:83-89`; `InMemoryBackend` + `src/backend_inmem.rs:239-250`; `StoreBackend` `src/backend_store.rs:270-277`), INCLUDING a + historical generation whose bytes the backend still holds (`InMemoryBackend` keeps every generation + in `generations` and refuses the non-head ones deliberately, `src/backend_inmem.rs:247-250`). A + rooted read therefore succeeds against `digstore serve` only when `r` is the current head. This is + the point where this server and the gateway differ (§4.5). **[implemented]** +5. **No pruned/never-existed oracle.** The `404` of row 4 is the same `RemoteError::UnknownRoot` + response whether `r` is a generation the remote holds but does not serve, a generation it once + held, or a root that never existed — the variant carries no payload (`src/error.rs:9-10`), so the + body cannot distinguish them, and an implementation MUST NOT add a distinguishing body. A reader + MUST NOT infer from a `404` that a root never existed; from `digstore serve` it means only "not the + served head". **[implemented]** for the shared variant; reached from GET/HEAD once row 4 lands. +6. **A rooted request is never downgraded to the head.** With `?root=r` present, the served root MUST + be `r` (row 6 is reachable only after row 4 passed) and the `ETag` MUST be the root actually + served. The pre-#1903 behaviour — ignore `?root=`, serve the head under the head's `ETag` — is the + fail-OPEN direction this section forbids: a client without §4.2.5's pin check would install a + generation it did not ask for. **[pending #1903]** +7. **Auth.** The §21.9 method tag for GET and HEAD `/module` is `module`, with or without a query + (`src/server.rs:161`); the query is not part of the signed message (§4.6). **[implemented]** + +### 4.5 The two servers side by side + +The `rpc.dig.net` gateway (hub.dig.net `SPEC.md` §16) serves EVERY generation it holds, immutably per +root, and resolves a rootless read by redirect. `digstore serve` serves only its head and never +redirects. One client contract (§4.2/§4.3) works against both because the client always pins when it +can and treats everything but a matching `200` as failure: + +| request | `digstore serve` | `rpc.dig.net` gateway | client outcome (`clone_store_at`) | +|---|---|---|---| +| `GET /module` (rootless) | `200`, head, `ETag=""` | `307` → `?root=`, `no-store`, empty body | serve: `Ok((head, bytes))` · gateway: `Err(Status(307))` — pass the head | +| `GET /module?root=` | `200`, `ETag=""` | `200`, `ETag=""`, immutable | `Ok((root, bytes))` after pin + `verify` | +| `GET /module?root=` | `404` (`UnknownRoot`) | `200`, `ETag=""` | serve: `Err(Status(404))` · gateway: `Ok((root, bytes))` | +| `GET /module?root=` | `404` (`UnknownRoot`) — same body as the row above | `404` | `Err(Status(404))` | +| `GET /module?root=` (empty) or `?root=latest` | `422` | `307` → `?root=` | `Err(Status(422))` / `Err(Status(307))` — a conforming client never emits these | +| `GET /module?root=` | `422` | `400` | `Err(Status(422))` / `Err(Status(400))` | +| unknown store | `404` (`UnknownStore`) | `404` | `Err(Status(404))` | +| `200` whose `ETag` root ≠ the pinned root | does not occur on a conforming server | does not occur on a conforming server | `Err(Verification)`, `verify` and `on_progress` not invoked | +| GET with `If-None-Match` equal to the served root | `304` + `ETag` | per hub `SPEC.md` §16 | `pull`: `UpToDate` | + +The two servers answer the same request differently in rows 1, 3, 5 and 6. A conforming client MUST +NOT special-case the server: it pins the root whenever it holds one and maps every status other than +`200` (and, in `pull`, `304`) to `Status(code)`. + +### 4.6 What §21.9 does and does not sign + +The signed-request headers cover `request_signing_message(method_tag, store_id, timestamp, nonce)` +(`src/client.rs:261-272`) — the QUERY STRING IS NOT SIGNED. Adding `?root=` therefore changes no +signature and no method tag (`module`). A reader MUST NOT conclude that the root pin is +integrity-protected by the request's authentication: the pin is enforced by the CLIENT — the `ETag` +comparison (§4.2.5, §4.3.3) and the caller's `verify` (§4.2.6) — so a `root` altered in flight yields +`Verification`, never a silently different generation. **[implemented]** for the signing scope; the +enforcement clauses carry their own markers. + +### 4.7 CLI — `digstore clone` + +`clone_from` (`crates/digstore-cli/src/ops/remote_ops.rs:360`) runs `client.fetch` and derives +`remote_root` from `current_root` (`:376-378`) BEFORE downloading. It MUST download with +`clone_store_at(&store_id, Some(&remote_root), ..)`; today it calls the rootless `clone_store` +(`:389-410`). Its verifier (embedded `StoreId == store_id`, recomputed content root == served root, +`:391-402`) and its post-check `etag_root == remote_root` (`:415-419`) are unchanged; with the pin in +place the post-check can no longer fire (the pin already guarantees the equality) and is retained as +defence in depth, not as the mechanism. **[pending #1903]** + +`digstore pull` needs no CLI change: the pin lives inside `DigClient::pull` (§4.3). + +### 4.8 What this section does not specify + +- It does not make `digstore serve` retain or serve historical generations over `/module`; the + head-only rule (§4.4.4) IS the served contract, and changing it is a `RemoteBackend` trait-contract + change across both backends, decided separately. +- It does not change the gateway; §4.5 records the gateway's shipped behaviour (hub.dig.net `SPEC.md` + §16) so the client contract can be checked against it, and MUST be updated in the same unit of work + as any gateway change. +- It does not specify the delta path (`/delta`), content reads (`/content`), or pushes + (`PUT /module?root=`, `src/handlers/module.rs:184-189`). + +### 4.9 Implementation status + +Each pending row is replaced by its `file:line` citation in the PR that lands it; a row left pending +after that PR is a defect in this document. + +| clause | status | +|---|---| +| 4.2.1 `clone_store_at` exists; `clone_store` delegates to it | pending #1903 | +| 4.2.2 rooted request carries `?root=`; rootless request unchanged | pending #1903 | +| 4.2.3 redirects never followed; non-2xx → `Status(code)` | `src/client.rs:196, 335-337` | +| 4.2.4 `ETag` missing/unparsable → `Verification` | `src/client.rs:338-346` | +| 4.2.5 pin: `ETag` root ≠ requested → `Verification`, before body/`on_progress`/`verify` | pending #1903 | +| 4.2.6 `verify` invoked once with the served root | `src/client.rs:348-349` | +| 4.3.1 `pull` full GET carries `?root=` | pending #1903 | +| 4.3.3 `pull` checks `ETag` root == `remote_root` | pending #1903 | +| 4.4.2 rows 2 and 4 (422 before lookup; 404 for a non-served root) on GET and HEAD | pending #1903 | +| 4.4.3 rootless → `200` head, no redirect | `src/handlers/module.rs:74-86` | +| 4.4.4 head-only `module_bytes` | `src/backend_inmem.rs:239-250`, `src/backend_store.rs:270-277` | +| 4.4.6 rooted request never downgraded to the head | pending #1903 | +| 4.7 CLI passes `Some(&remote_root)` | pending #1903 | diff --git a/crates/digstore-remote/src/client.rs b/crates/digstore-remote/src/client.rs index 61a7783a..a5b02254 100644 --- a/crates/digstore-remote/src/client.rs +++ b/crates/digstore-remote/src/client.rs @@ -307,15 +307,27 @@ impl DigClient { }) } - /// §21.3 clone: download + verify the module. `verify` is called with - /// (module_bytes, served_root) and must return Ok(()) when the module + /// §21.3 clone, optionally pinned to a specific generation root (SPEC §4.2). + /// + /// `root = Some(r)` requests exactly that generation + /// (`GET /stores/{id}/module?root=`) and refuses any + /// response whose `ETag` names a different root: the pin is checked on the + /// response HEADERS, before the body is downloaded, so a mismatched answer + /// never reaches `on_progress` or `verify` (§4.2.5). `root = None` requests + /// the remote's current served head — byte-identical to the pre-#1903 + /// rootless request — and the caller learns which root was served only from + /// the returned `ETag`. + /// + /// `verify` is called with `(module_bytes, served_root)` — the served root, + /// which equals `r` when pinned — and must return `Ok(())` when the module /// validates to that root (full merkle verification lives in the caller). /// /// `on_progress`, when `Some`, is called as `(bytes_done, total_bytes)` after /// each received chunk (total is 0 when the server omits Content-Length). - pub async fn clone_store( + pub async fn clone_store_at( &self, store_id: &Bytes32, + root: Option<&Bytes32>, verify: V, on_progress: Option<&(dyn Fn(u64, u64) + Send + Sync)>, ) -> Result<(Bytes32, Vec), ClientError> @@ -323,12 +335,12 @@ impl DigClient { V: FnOnce(&[u8], &Bytes32) -> Result<(), String>, { let id = store_id.to_hex(); + let path = match root { + Some(r) => format!("/stores/{id}/module?root={}", r.to_hex()), + None => format!("/stores/{id}/module"), + }; let resp = self - .authed( - self.http.get(self.url(&format!("/stores/{id}/module"))), - "module", - store_id, - )? + .authed(self.http.get(self.url(&path)), "module", store_id)? .send() .await .map_err(|e| ClientError::Transport(e.to_string()))?; @@ -340,14 +352,53 @@ impl DigClient { .get(reqwest::header::ETAG) .and_then(|v| v.to_str().ok()) .map(|s| s.to_string()); - let root = etag + let served_root = etag .as_deref() .and_then(parse_if_none_match) .ok_or_else(|| ClientError::Verification("missing/invalid ETag".into()))?; + // §4.2.5 the pin: enforced on the headers, BEFORE the body is read, so a + // server that answers a request for `r` with a different generation is + // refused before that generation's bytes ever reach `on_progress` or the + // caller's `verify`. + if let Some(r) = root { + if served_root != *r { + return Err(ClientError::Verification(format!( + "served root {} != requested root {}", + served_root.to_hex(), + r.to_hex() + ))); + } + } let total = resp.content_length().unwrap_or(0); let bytes = download_with_progress(resp, total, on_progress).await?; - verify(&bytes, &root).map_err(ClientError::Verification)?; - Ok((root, bytes)) + verify(&bytes, &served_root).map_err(ClientError::Verification)?; + Ok((served_root, bytes)) + } + + /// §21.3 clone: download + verify the module at the remote's current served + /// head. `verify` is called with (module_bytes, served_root) and must return + /// Ok(()) when the module validates to that root (full merkle verification + /// lives in the caller). + /// + /// `on_progress`, when `Some`, is called as `(bytes_done, total_bytes)` after + /// each received chunk (total is 0 when the server omits Content-Length). + /// + /// Delegates to [`Self::clone_store_at`] with `root = None` (SPEC §4.2.1); + /// callers that already hold the head (every caller that has run `fetch`) + /// SHOULD call `clone_store_at(.., Some(&head), ..)` directly instead, since + /// a rootless read against a redirecting gateway fails with `Status(307)` + /// (SPEC §4.2.3/§4.5). + pub async fn clone_store( + &self, + store_id: &Bytes32, + verify: V, + on_progress: Option<&(dyn Fn(u64, u64) + Send + Sync)>, + ) -> Result<(Bytes32, Vec), ClientError> + where + V: FnOnce(&[u8], &Bytes32) -> Result<(), String>, + { + self.clone_store_at(store_id, None, verify, on_progress) + .await } /// §21.4 pull: advance the local head. `local_root` is the client's current @@ -399,9 +450,15 @@ impl DigClient { // fall through to full module on non-success delta. } } - // full module download with conditional request. + // Full module download, pinned to the remote head this call already + // resolved (SPEC §4.3.1) — never today's rootless GET. A head advance + // between `fetch` and this GET now surfaces as `Status(404)` against a + // root-pinning server (§4.4) rather than silently downloading a newer + // generation than the one this call reports; the caller re-runs `pull`. + let remote_root_hex = remote_root.to_hex(); let mut req = self.authed( - self.http.get(self.url(&format!("/stores/{id}/module"))), + self.http + .get(self.url(&format!("/stores/{id}/module?root={remote_root_hex}"))), "module", store_id, )?; @@ -421,6 +478,23 @@ impl DigClient { if !resp.status().is_success() { return Err(ClientError::Status(resp.status().as_u16())); } + // SPEC §4.3.3: the ETag MUST agree with the root this request pinned — + // missing, unparsable, or mismatching all fail closed, and the body is + // not returned. `PullResult::Module` therefore carries a root the served + // `ETag` itself agreed with, not merely the descriptor's earlier claim. + let etag_root = resp + .headers() + .get(reqwest::header::ETAG) + .and_then(|v| v.to_str().ok()) + .and_then(parse_if_none_match); + if etag_root != Some(remote_root) { + return Err(ClientError::Verification(format!( + "served root {} != remote head {remote_root_hex}", + etag_root + .map(|r| r.to_hex()) + .unwrap_or_else(|| "".into()) + ))); + } let total = resp.content_length().unwrap_or(0); let bytes = download_with_progress(resp, total, on_progress).await?; Ok(PullResult::Module { diff --git a/crates/digstore-remote/src/handlers/module.rs b/crates/digstore-remote/src/handlers/module.rs index 8fb8db5b..2c5e808a 100644 --- a/crates/digstore-remote/src/handlers/module.rs +++ b/crates/digstore-remote/src/handlers/module.rs @@ -12,40 +12,68 @@ use axum::{ use digstore_core::{Bytes32, Bytes96}; use std::collections::HashMap; -pub async fn head_module(State(s): State, Path(id): Path) -> Response { +/// Parse the optional whole-module-read `?root=` query parameter (SPEC §4.4.2 +/// row 2): present-but-malformed (empty, wrong length, non-hex) is a validation +/// error the caller maps to `422`, evaluated BEFORE any backend call; absent is +/// a rootless request (`Ok(None)`). Shared by `get_module`/`head_module` so the +/// two routes cannot drift on what counts as a well-formed root. +fn parse_requested_root(q: &HashMap) -> Result, RemoteError> { + q.get("root").map(|r| parse_b32(r)).transpose() +} + +pub async fn head_module( + State(s): State, + Path(id): Path, + Query(q): Query>, +) -> Response { let store_id = match parse_store_id(&id) { Ok(v) => v, Err(e) => return e.into_response(), }; + // SPEC §4.4.2 row 2: a malformed root is 422 before any backend call. + let requested_root = match parse_requested_root(&q) { + Ok(r) => r, + Err(e) => return e.into_response(), + }; let backend = s.backend.clone(); let res = run_blocking(move || backend.head_state(&store_id)).await; - match res { - Ok(hs) => { - let mut headers = HeaderMap::new(); - headers.insert( - header::ETAG, - etag_for_root(&hs.served_root).parse().unwrap(), - ); - headers.insert( - header::CONTENT_LENGTH, - hs.served_size.to_string().parse().unwrap(), - ); - headers.insert(header::CONTENT_TYPE, "application/wasm".parse().unwrap()); - (StatusCode::OK, headers).into_response() - } - Err(e) => e.into_response(), + let hs = match res { + Ok(hs) => hs, + Err(e) => return e.into_response(), // row 3: unknown store -> 404 + }; + // SPEC §4.4.2 row 4 / §4.4.6: a well-formed, non-head root 404s — a rooted + // HEAD is never silently downgraded to describing the served head. + if matches!(requested_root, Some(r) if r != hs.served_root) { + return RemoteError::UnknownRoot.into_response(); } + let mut headers = HeaderMap::new(); + headers.insert( + header::ETAG, + etag_for_root(&hs.served_root).parse().unwrap(), + ); + headers.insert( + header::CONTENT_LENGTH, + hs.served_size.to_string().parse().unwrap(), + ); + headers.insert(header::CONTENT_TYPE, "application/wasm".parse().unwrap()); + (StatusCode::OK, headers).into_response() } pub async fn get_module( State(s): State, Path(id): Path, + Query(q): Query>, headers: HeaderMap, ) -> Response { let store_id = match parse_store_id(&id) { Ok(v) => v, Err(e) => return e.into_response(), }; + // SPEC §4.4.2 row 2: a malformed root is 422 before any backend call. + let requested_root = match parse_requested_root(&q) { + Ok(r) => r, + Err(e) => return e.into_response(), + }; let backend = s.backend.clone(); let head = match run_blocking({ let b = backend.clone(); @@ -54,10 +82,18 @@ pub async fn get_module( .await { Ok(h) => h, - Err(e) => return e.into_response(), + Err(e) => return e.into_response(), // row 3: unknown store -> 404 }; - // §21.7: If-None-Match equal to current root -> 304. + // SPEC §4.4.2 row 4 / §4.4.6: a well-formed, non-head root 404s outright — + // never downgraded to serving the head under its own ETag — and this runs + // BEFORE the row-5 conditional-GET check below, so a stale/foreign root can + // never ride an `If-None-Match` to a 304 either. + if matches!(requested_root, Some(r) if r != head.served_root) { + return RemoteError::UnknownRoot.into_response(); + } + + // §21.7 row 5: If-None-Match equal to the served root -> 304 (GET only). if let Some(inm) = headers .get(header::IF_NONE_MATCH) .and_then(|v| v.to_str().ok()) @@ -71,7 +107,9 @@ pub async fn get_module( } } - let res = run_blocking(move || backend.module_bytes(&store_id, None)).await; + // Row 6: `requested_root` is `None` (rootless: serve the head) or `Some(r)` + // with `r == head.served_root` (the only way past the row-4 gate above). + let res = run_blocking(move || backend.module_bytes(&store_id, requested_root.as_ref())).await; match res { Ok(bytes) => ( StatusCode::OK, diff --git a/crates/digstore-remote/tests/client_roundtrip.rs b/crates/digstore-remote/tests/client_roundtrip.rs index 8a078d52..7c6325ec 100644 --- a/crates/digstore-remote/tests/client_roundtrip.rs +++ b/crates/digstore-remote/tests/client_roundtrip.rs @@ -1,8 +1,12 @@ mod test_helpers; use test_helpers::*; -use digstore_remote::{DigClient, InMemoryBackend, PullResult, PushResult, RemoteServer}; -use std::sync::Arc; +use digstore_core::Bytes32; +use digstore_remote::{ + ClientError, DigClient, InMemoryBackend, PullResult, PushResult, RemoteServer, +}; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Mutex}; async fn spawn_server(be: Arc) -> String { let app = RemoteServer::new(be).allow_anonymous().router(); @@ -211,3 +215,350 @@ async fn push_non_fast_forward_is_client_error() { Err(digstore_remote::ClientError::NonFastForward) )); } + +// --------------------------------------------------------------------------- +// SPEC §4 — whole-module read root pinning (#1903) +// --------------------------------------------------------------------------- + +/// A deliberately non-conforming `/stores/:id/module` route that always +/// answers `200` with `served_root`'s `ETag`, ignoring whatever `?root=` it was +/// asked for. No conforming `digstore serve`/gateway can produce this shape +/// (§4.4.6 forbids serving a rooted request under any ETag but the one it +/// named); this exists to prove the CLIENT's own pin check (§4.2.5), not +/// merely a well-behaved server's cooperation. +async fn spawn_wrong_etag_module_server(served_root: Bytes32, body: Vec) -> String { + use axum::{body::Body, response::IntoResponse, routing::get, Router}; + let etag = format!("\"{}\"", served_root.to_hex()); + let app = Router::new().route( + "/stores/:id/module", + get(move || { + let etag = etag.clone(); + let body = body.clone(); + async move { + ( + axum::http::StatusCode::OK, + [(axum::http::header::ETAG, etag)], + Body::from(body), + ) + .into_response() + } + }), + ); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + format!("http://{addr}") +} + +/// A hand-rolled remote for `pull`-focused tests: `/stores/:id` and +/// `/stores/:id/roots` answer like a normal descriptor (so `pull`'s internal +/// `fetch` succeeds), but `/stores/:id/module` is deliberately non-conforming — +/// it always serves `served_etag_root`'s ETag regardless of the requested +/// `?root=`, and records the query string it was asked with into +/// `captured_module_query`. This lets one test assert BOTH that `pull` sends +/// `?root=` (§4.3.1) AND that `pull` refuses a server answering a +/// different root than it claims to (§4.3.3) — a shape no conforming server +/// can produce, so it proves `pull`'s own check rather than a happy path any +/// well-behaved server would also satisfy. +async fn spawn_pull_probe_server( + descriptor_root: Bytes32, + served_etag_root: Bytes32, + module_body: Vec, + captured_module_query: Arc>>, +) -> String { + use axum::{ + body::Body, + extract::{OriginalUri, Path}, + response::IntoResponse, + routing::get, + Json, Router, + }; + + let desc_root_hex = descriptor_root.to_hex(); + let descriptor = move |Path(_id): Path| { + let current_root = desc_root_hex.clone(); + async move { + Json(serde_json::json!({ + "current_root": current_root, + "size": 0, + "public_key": "00".repeat(48), + "push_sig": "", + "tombstones": [], + })) + } + }; + let roots = |Path(_id): Path| async move { Json(serde_json::json!({ "roots": [] })) }; + let etag = format!("\"{}\"", served_etag_root.to_hex()); + let module = move |Path(_id): Path, uri: OriginalUri| { + let etag = etag.clone(); + let body = module_body.clone(); + let captured = captured_module_query.clone(); + async move { + *captured.lock().unwrap() = uri.0.query().map(|q| q.to_string()); + ( + axum::http::StatusCode::OK, + [(axum::http::header::ETAG, etag)], + Body::from(body), + ) + .into_response() + } + }; + + let app = Router::new() + .route("/stores/:id", get(descriptor)) + .route("/stores/:id/roots", get(roots)) + .route("/stores/:id/module", get(module)); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + format!("http://{addr}") +} + +#[tokio::test] +async fn clone_store_at_pins_to_the_served_head() { + let (be, id, _hex) = one_store(); + let base = spawn_server(be).await; + let client = DigClient::new(base); + let (root, bytes) = client + .clone_store_at( + &id, + Some(&b32(0x10)), + |b, r| { + if b.len() == 64 && *r == b32(0x10) { + Ok(()) + } else { + Err("size mismatch".into()) + } + }, + None, + ) + .await + .unwrap(); + assert_eq!(root, b32(0x10)); + assert_eq!(bytes.len(), 64); +} + +#[tokio::test] +async fn clone_store_at_of_a_held_non_head_generation_is_404() { + // The old genesis root is still HELD (InMemoryBackend keeps every + // generation) but no longer SERVED once 0x12 advances the head — §4.4.4 + // says a `digstore serve` remote refuses even a generation it still holds. + let (be, id, _hex) = one_store(); + be.add_generation( + &id, + b32(0x10), + b32(0x12), + vec![0u8; 8], + vec![], + vec![], + true, + ); + let base = spawn_server(be).await; + let client = DigClient::new(base); + let result = client + .clone_store_at(&id, Some(&b32(0x10)), |_, _| Ok(()), None) + .await; + assert!( + matches!(result, Err(ClientError::Status(404))), + "got {result:?}" + ); +} + +#[tokio::test] +async fn clone_store_at_of_a_never_existed_root_is_404() { + let (be, id, _hex) = one_store(); + let base = spawn_server(be).await; + let client = DigClient::new(base); + let result = client + .clone_store_at(&id, Some(&b32(0x99)), |_, _| Ok(()), None) + .await; + assert!( + matches!(result, Err(ClientError::Status(404))), + "got {result:?}" + ); +} + +#[tokio::test] +async fn clone_store_at_refuses_a_served_root_that_disagrees_with_the_pin() { + let requested_root = b32(0x10); + let served_wrong_root = b32(0xAA); // what the misbehaving server actually serves + let base = spawn_wrong_etag_module_server(served_wrong_root, vec![9u8; 4]).await; + let client = DigClient::new(base); + + let verify_called = Arc::new(AtomicBool::new(false)); + let vc = verify_called.clone(); + let progress_called = Arc::new(AtomicBool::new(false)); + let pc = progress_called.clone(); + let on_progress = move |_done: u64, _total: u64| { + pc.store(true, Ordering::SeqCst); + }; + + let result = client + .clone_store_at( + &b32(1), + Some(&requested_root), + move |_bytes: &[u8], _root: &Bytes32| -> Result<(), String> { + vc.store(true, Ordering::SeqCst); + Ok(()) + }, + Some(&on_progress), + ) + .await; + + assert!( + matches!(result, Err(ClientError::Verification(_))), + "got {result:?}" + ); + assert!( + !verify_called.load(Ordering::SeqCst), + "verify must not run on a pin mismatch" + ); + assert!( + !progress_called.load(Ordering::SeqCst), + "on_progress must not run on a pin mismatch" + ); +} + +#[tokio::test] +async fn pull_full_module_get_is_pinned_to_the_remote_head_and_refuses_a_wrong_etag() { + let remote_head = b32(0x42); + let wrong_served_root = b32(0x99); // the stub server's misbehavior + let captured_query = Arc::new(Mutex::new(None)); + let base = spawn_pull_probe_server( + remote_head, + wrong_served_root, + vec![1u8; 4], + captured_query.clone(), + ) + .await; + let client = DigClient::new(base); + + let result = client.pull(&b32(1), Some(b32(0x01)), false, None).await; + + assert!( + matches!(result, Err(ClientError::Verification(_))), + "got {result:?}" + ); + let query = captured_query.lock().unwrap().clone(); + assert_eq!( + query, + Some(format!("root={}", remote_head.to_hex())), + "pull's full-module GET must carry ?root= (SPEC §4.3.1)" + ); +} + +#[tokio::test] +async fn rooted_get_of_a_held_non_head_generation_is_404() { + let (be, id, hex) = one_store(); + be.add_generation( + &id, + b32(0x10), + b32(0x12), + vec![0u8; 8], + vec![], + vec![], + true, + ); + let base = spawn_server(be).await; + let http = reqwest::Client::new(); + let resp = http + .get(format!( + "{base}/stores/{hex}/module?root={}", + b32(0x10).to_hex() + )) + .send() + .await + .unwrap(); + assert_eq!(resp.status().as_u16(), 404); +} + +#[tokio::test] +async fn rooted_get_of_a_never_existed_root_is_404() { + let (be, _id, hex) = one_store(); + let base = spawn_server(be).await; + let http = reqwest::Client::new(); + let resp = http + .get(format!( + "{base}/stores/{hex}/module?root={}", + b32(0x99).to_hex() + )) + .send() + .await + .unwrap(); + assert_eq!(resp.status().as_u16(), 404); +} + +#[tokio::test] +async fn malformed_root_is_422_even_for_an_unknown_store() { + // A store id that was never registered on this backend at all — proves + // row 2 (malformed root) precedes row 3 (unknown store) in SPEC §4.4.2: + // a malformed root never reaches the store lookup. + let unknown_hex = b32(0xEE).to_hex(); + let (be, _id, _hex) = one_store(); + let base = spawn_server(be).await; + let http = reqwest::Client::new(); + for bad in ["zz", "", "abcd"] { + let resp = http + .get(format!("{base}/stores/{unknown_hex}/module?root={bad}")) + .send() + .await + .unwrap(); + assert_eq!( + resp.status().as_u16(), + 422, + "root={bad:?} against an unknown store must still 422" + ); + } +} + +#[tokio::test] +async fn head_rooted_at_the_served_head_is_200_with_etag() { + let (be, _id, hex) = one_store(); + let base = spawn_server(be).await; + let http = reqwest::Client::new(); + let resp = http + .head(format!( + "{base}/stores/{hex}/module?root={}", + b32(0x10).to_hex() + )) + .send() + .await + .unwrap(); + assert_eq!(resp.status().as_u16(), 200); + let etag = resp + .headers() + .get(reqwest::header::ETAG) + .unwrap() + .to_str() + .unwrap(); + assert_eq!(etag, format!("\"{}\"", b32(0x10).to_hex())); +} + +#[tokio::test] +async fn head_rooted_at_a_non_head_root_is_404() { + let (be, id, hex) = one_store(); + be.add_generation( + &id, + b32(0x10), + b32(0x12), + vec![0u8; 8], + vec![], + vec![], + true, + ); + let base = spawn_server(be).await; + let http = reqwest::Client::new(); + let resp = http + .head(format!( + "{base}/stores/{hex}/module?root={}", + b32(0x10).to_hex() + )) + .send() + .await + .unwrap(); + assert_eq!(resp.status().as_u16(), 404); +}