diff --git a/Cargo.lock b/Cargo.lock index 850b33d..71ff99a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2354,7 +2354,7 @@ dependencies = [ [[package]] name = "digstore-chain" -version = "0.23.2" +version = "0.24.0" dependencies = [ "aes-gcm", "anyhow", @@ -2392,7 +2392,7 @@ dependencies = [ [[package]] name = "digstore-chunker" -version = "0.23.2" +version = "0.24.0" dependencies = [ "digstore-core", "hex", @@ -2402,7 +2402,7 @@ dependencies = [ [[package]] name = "digstore-cli" -version = "0.23.2" +version = "0.24.0" dependencies = [ "anstream 0.6.21", "anstyle", @@ -2471,7 +2471,7 @@ dependencies = [ [[package]] name = "digstore-core" -version = "0.23.2" +version = "0.24.0" dependencies = [ "aes-gcm-siv", "hex", @@ -2568,7 +2568,7 @@ dependencies = [ [[package]] name = "digstore-remote" -version = "0.23.2" +version = "0.24.0" dependencies = [ "async-trait", "axum", @@ -2628,7 +2628,7 @@ dependencies = [ [[package]] name = "digstore-subscription" -version = "0.23.2" +version = "0.24.0" dependencies = [ "async-trait", "digstore-core", diff --git a/Cargo.toml b/Cargo.toml index b9d7e14..ac9c2b8 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.23.2" +version = "0.24.0" license = "GPL-2.0-only" [workspace.dependencies] diff --git a/SPEC.md b/SPEC.md index c98f8b8..2f19d01 100644 --- a/SPEC.md +++ b/SPEC.md @@ -700,6 +700,28 @@ bound-induced export failure (timeout vs fuel exhaustion), not as an opaque engi Each export call is armed with its own fresh budget; a serve sequence (alloc → call → read → dealloc) is deliberately NOT a single combined budget. +### 13.6 Host identity: never substituted, and required to attest, sign, or push + +A store's host identity is its BLS signing key (`signing_key.bin`) and the trusted host keys +(`trusted_keys.json`) persisted at init. + +- A host MUST NOT substitute a default, fixed, hardcoded, or all-zero value for a missing signing + key or public key. A fixed seed is reproducible by anyone with the source, so a host holding one + carries no identity at all rather than a weaker one, and an all-zero public key is a nonexistent + identity rather than a weak one. +- A host MUST refuse to attest, sign, or push when the identity is absent or unreadable. +- The signing key MUST be exactly 32 bytes. A shorter or longer file is malformed and MUST be + reported as a corrupt-identity error — never truncated, never padded, and never handed to key + derivation, which is permitted to abort the process on a short seed. +- Wherever a code path loads the identity, an unreadable or malformed identity MUST surface as an + error naming the offending file, rather than as a downstream symptom several layers away. + +The substitution ban holds regardless of whether a given path currently verifies the identity it +loads. A path that does not consume the identity today MUST NOT be treated as licence to supply a +placeholder, because the placeholder becomes forgeable identity the moment any consumer begins +verifying it. Where a path genuinely does not need an identity, the correct expression is to carry +no identity at all — not to carry a fabricated one. + ## 14. Client → node resolution (the origin) This section is normative for every command that must reach a DIG node: which endpoint is diff --git a/crates/digstore-cli/src/ops/remote_ops.rs b/crates/digstore-cli/src/ops/remote_ops.rs index 8888ea8..e529ddf 100644 --- a/crates/digstore-cli/src/ops/remote_ops.rs +++ b/crates/digstore-cli/src/ops/remote_ops.rs @@ -66,6 +66,13 @@ pub(crate) fn map_remote_err(e: ClientError) -> CliError { ClientError::Transport(msg) => CliError::Network(msg), ClientError::Verification(msg) => CliError::VerificationFailed(msg), ClientError::Decode(msg) => CliError::Network(format!("decode: {msg}")), + // The OS CSPRNG could not supply a §21.9 request nonce, so the request was + // refused rather than signed with a predictable one. This is a local fault, + // not a remote one — say so, so the operator looks at the right machine. + ClientError::Entropy(msg) => CliError::Other(anyhow::anyhow!( + "this machine's secure random source is unavailable, so the request could not be \ + signed safely: {msg}" + )), } } diff --git a/crates/digstore-cli/src/ops/serve.rs b/crates/digstore-cli/src/ops/serve.rs index 877748b..f09d27b 100644 --- a/crates/digstore-cli/src/ops/serve.rs +++ b/crates/digstore-cli/src/ops/serve.rs @@ -108,7 +108,19 @@ fn host_deps(store_id: Bytes32, pubkey: Bytes48, secret: BlsSecretKey) -> HostDe clock: Arc::new(FixedClock::new(1_700_000_000)), chain: Arc::new(chain), prover: Arc::new(prover), - rng_seed: Some([99u8; 32]), + // Draw real OS entropy rather than a constant seed, converging on the + // convention `digstore_host::serve_blind` already follows. + // + // SCOPE, stated honestly: this RNG backs `host_random_bytes`, whose only + // live consumer is the guest's oblivious-access cover traffic. The §12 + // attestation nonce draw is unreachable here — `digstore-guest`'s content + // path hardcodes `require_attestation: false` (dighub content is public + // and must be servable by any node), so no module this CLI compiles takes + // that branch. Nor is this closing a third-party attack: the party the + // cover-traffic shuffle hides access patterns from is the HOST, and the + // host is what supplies this randomness. The change removes a constant + // seed that had no business outside a test fixture. + rng_seed: None, instance_id: Bytes32([1u8; 32]), attestation: None, } @@ -124,12 +136,20 @@ fn instantiate_host( ) -> Result { let module_bytes = std::fs::read(module_path) .map_err(|_| CliError::NotFound(module_path.display().to_string()))?; - // §12.2: the host MUST attest with the store's host signing key — the same - // key whose public half the compiler embedded as the trusted key. Load the - // persisted seed (init wrote `signing_key.bin`) so the guest's attestation - // verification accepts this host; otherwise it would (correctly) serve decoys. - let secret = - store_ops::load_signing_key(ctx).unwrap_or_else(|_| BlsSecretKey::from_seed(&[42u8; 32])); + // §12.2: the host attests with the store's host signing key — the same key + // whose public half the compiler embedded as the trusted key. Load the + // persisted seed that `init` wrote to `signing_key.bin`. + // + // Note what this key does NOT do on the read path: `digstore-guest`'s content + // path hardcodes `require_attestation: false`, so the guest does not verify + // this host and would not serve decoys if the key were wrong. The key is + // genuinely consumed by `serve_proof` below (§13.7 "one key for both roles"). + // + // FAIL CLOSED anyway: no fallback key. A hardcoded seed is reproducible by + // anyone reading this source, so a host holding it carries no identity at + // all — surface the missing key instead of degrading into an anonymous host, + // and surface it HERE rather than in the proof path that is harder to reach. + let secret = store_ops::load_signing_key(ctx)?; HostRuntime::new( &module_bytes, HostImportsConfig::default(), @@ -155,7 +175,21 @@ pub fn serve_content_raw( urn: &Urn, ) -> Result, CliError> { let store_id = urn.store_id; - let pubkey = store_ops::load_host_pubkey(ctx).unwrap_or(Bytes48([0u8; 48])); + // FAIL CLOSED, same reason as the signing key one line below: a store that + // cannot produce its own host identity is a broken store, and an all-zero G1 + // is not a weaker identity but a nonexistent one. + // + // Do NOT weaken this on the theory that the old fallback failed closed + // downstream anyway. It did not, and the earlier version of this comment + // claiming otherwise was the most dangerous line in the file: it read as a + // license to restore `unwrap_or(Bytes48([0u8; 48]))`. The guest's content + // path hardcodes `require_attestation: false` (see `instantiate_host`), so + // the host pubkey is never read here and never checked against any trusted + // set. Measured against the released 0.23.0 binary: with `trusted_keys.json` + // deleted it exits 0 and serves the content, under an identity that exists + // nowhere. Refusing here IS the fix, not a diagnostic nicety layered over a + // default that was already safe. + let pubkey = store_ops::load_host_pubkey(ctx)?; let mut rt = instantiate_host(ctx, module_path, store_id, pubkey)?; // Drive the module's own serve flow. The request carries the ROOT-INDEPENDENT @@ -258,8 +292,9 @@ pub fn serve_proof( // attestation signing key (init wrote `signing_key.bin`) rather than minting // an independent prover key, so node attribution is bound to the attestation // identity by construction. - let node_sk = - store_ops::load_signing_key(ctx).unwrap_or_else(|_| BlsSecretKey::from_seed(&[42u8; 32])); + // FAIL CLOSED (see `instantiate_host`): a proof signed by a world-known + // fallback key attributes serving work to nobody. + let node_sk = store_ops::load_signing_key(ctx)?; let node_pk = node_sk.public_key(); let block = ChiaBlockRef { header_hash: Bytes32([0x55u8; 32]), @@ -281,6 +316,13 @@ pub fn serve_proof( // module's embedded §12 attestation trusted-key set, otherwise "one key for // both roles" is unenforced. Verify the binding against the persisted trusted // keys using the deterministic mock chain for freshness. + // `unwrap_or_default()` is DELIBERATE here and is not the fallback-habit + // defect the two loads above fix: an EMPTY trusted set is the strictest + // possible set, not a permissive one. `verify_node_attested` rejects any + // proof whose signer is absent from it (`NodeKeyNotAttested`, + // `digstore-prover/src/prover.rs`), so an unreadable `trusted_keys.json` + // makes the verification below fail rather than pass. Do not "fix" this into + // a `?`; it would change nothing about safety. let trusted_node_keys = store_ops::load_trusted_keys(ctx) .map(|ks| { ks.into_iter() @@ -296,3 +338,96 @@ pub fn serve_proof( let _ = module_bytes; Ok((proof, root)) } + +#[cfg(test)] +mod tests { + use super::*; + + /// Build a real committed store and return its context, root and module path + /// — the fixture both call-site tests below need, because neither can be + /// answered by inspecting a helper's return value. + fn committed_store() -> (tempfile::TempDir, CliContext, Bytes32, std::path::PathBuf) { + let td = tempfile::tempdir().unwrap(); + let ctx = CliContext::resolve(Some(td.path().to_path_buf()), false, false); + store_ops::init_store(&ctx, false, None, None, None, None, None, None).unwrap(); + + let f = td.path().join("hello.txt"); + std::fs::write(&f, b"hello serve").unwrap(); + store_ops::add_path(&ctx, &f, Some("hello".into())).unwrap(); + let res = store_ops::commit(&ctx, None, empty_manifest()).unwrap(); + + let store_id = ctx.find_store_id().unwrap(); + let module_path = store_ops::module_path_for(&ctx, &store_id, Some(res.roothash)).unwrap(); + (td, ctx, store_id, module_path) + } + + /// FAIL CLOSED: the runtime `instantiate_host` ACTUALLY BUILDS must never pin + /// the host RNG. + /// + /// Anchored at the call site on purpose. The obvious version of this test + /// asserts `host_deps(..).rng_seed.is_none()`, which an attacker-equivalent + /// refactor defeats trivially: inline a `HostDeps { rng_seed: Some(..), .. }` + /// literal in `instantiate_host` and stop calling `host_deps` at all. The + /// helper's contract stays intact, the production path is re-pinned, and the + /// test stays green. So we instantiate through the real function and ask the + /// runtime what it was built with — `HostRuntime::rng_is_deterministic` + /// exists because the RNG is not observable through any export (the miss-path + /// decoy is derived from the retrieval key, §14.2, so it is byte-stable + /// whatever the RNG does). + #[test] + fn the_host_instantiate_host_builds_draws_real_entropy() { + let (_td, ctx, store_id, module_path) = committed_store(); + let pubkey = store_ops::load_host_pubkey(&ctx).unwrap(); + + let rt = instantiate_host(&ctx, &module_path, store_id, pubkey) + .expect("an initialized store instantiates"); + + assert!( + !rt.rng_is_deterministic(), + "the serve runtime must draw OS entropy; a pinned seed makes every \ + host_random_bytes draw reproducible from this source file" + ); + } + + /// FAIL CLOSED on a missing host PUBLIC key, the sibling of the signing-key + /// load one line away in the same function. + /// + /// Before this, `load_host_pubkey` fell back to an all-zero `Bytes48`, so a + /// store whose `trusted_keys.json` had gone missing served on with a host + /// identity that does not exist — and it SUCCEEDED. The pre-fix outcome was + /// not "an error, misattributed a few layers away"; it was a 200-equivalent. + /// Attestation is hardcoded off on the content path, so the zero key was + /// never read, let alone rejected. Confirmed against the released 0.23.0 + /// binary, which serves the file with `trusted_keys.json` deleted. + /// + /// That is why this has to be a call-site test: the difference it detects is + /// served-content versus refusal, which no assertion on `load_host_pubkey`'s + /// return value alone would show. + #[test] + fn a_missing_trusted_key_file_refuses_to_serve() { + let (_td, ctx, store_id, module_path) = committed_store(); + let urn = Urn { + chain: "chia".into(), + store_id, + root_hash: None, + resource_key: Some("hello".into()), + }; + // Control: the intact store serves. + serve_content_raw(&ctx, &module_path, &urn).expect("an intact store serves"); + + std::fs::remove_file(ctx.dig_dir.join("trusted_keys.json")).unwrap(); + + let err = serve_content_raw(&ctx, &module_path, &urn) + .expect_err("a store with no host identity must refuse to serve"); + // The refusal is only half the value; the other half is that the message + // names the file that is gone. A bare io error ("the system cannot find + // the file specified") fails just as closed and tells an operator + // nothing, so assert the subject, not merely the failure. + let msg = format!("{err:?}"); + assert!( + msg.contains("trusted_keys.json"), + "the error must name the missing identity file, not a downstream \ + symptom or a bare io error: {msg}" + ); + } +} diff --git a/crates/digstore-cli/src/ops/store_ops.rs b/crates/digstore-cli/src/ops/store_ops.rs index 86bc49d..61718b8 100644 --- a/crates/digstore-cli/src/ops/store_ops.rs +++ b/crates/digstore-cli/src/ops/store_ops.rs @@ -1429,7 +1429,18 @@ fn serialize_keys(keys: &[TrustedHostKey]) -> Vec { pub(crate) fn load_trusted_keys(ctx: &CliContext) -> Result, CliError> { let path = ctx.dig_dir.join("trusted_keys.json"); - let text = fs::read_to_string(&path).map_err(|e| CliError::Other(e.into()))?; + // Name the file. The serve path now REFUSES rather than substituting an + // all-zero host key, so this text is what an operator sees when the store's + // identity is missing — and a bare "the system cannot find the file + // specified", with no path and no subject, sends them looking at the content + // instead of at the one file that is gone. + let text = fs::read_to_string(&path).map_err(|e| { + CliError::Other(anyhow::anyhow!( + "cannot read the store's trusted host keys at {} ({e}) — the store may not have \ + been initialized (`dig init`), or the file was removed", + path.display() + )) + })?; let stored: Vec = serde_json::from_str(&text).map_err(|e| CliError::Other(e.into()))?; let mut out = Vec::with_capacity(stored.len()); @@ -1455,13 +1466,39 @@ pub(crate) fn load_host_pubkey(ctx: &CliContext) -> Result { Ok(Bytes48(k.public_key)) } -/// Load the host BLS signing key (seed) persisted at init. +/// Load the host BLS signing key (seed) persisted at init (§12.2). +/// +/// The error names the file and the likely cause, because every caller of this +/// is a serve path that must now REFUSE to run without it: an operator seeing a +/// bare io error would reasonably look for a content problem instead of a +/// missing store identity. +/// +/// The length is validated before the key is derived — `SecretKey::from_seed` +/// asserts `len >= 32` and therefore ABORTS THE PROCESS on a short seed. A +/// zero-length or truncated `signing_key.bin` needs no attacker to occur (an +/// `init` interrupted mid-write leaves exactly that), and a panic is the one +/// failure an operator cannot act on. Matches [`read_signing_seed`], which has +/// always required exactly 32 bytes. pub(crate) fn load_signing_key( ctx: &CliContext, ) -> Result { - let bytes = - fs::read(ctx.dig_dir.join("signing_key.bin")).map_err(|e| CliError::Other(e.into()))?; - Ok(digstore_crypto::bls::SecretKey::from_seed(&bytes)) + let path = ctx.dig_dir.join("signing_key.bin"); + let bytes = fs::read(&path).map_err(|e| { + CliError::Other(anyhow::anyhow!( + "cannot read the host signing key at {} ({e}) — the store may not have been \ + initialized (`dig init`), or the key file was removed or is unreadable", + path.display() + )) + })?; + let seed: [u8; 32] = bytes.as_slice().try_into().map_err(|_| { + CliError::InvalidArgument(format!( + "the host signing key at {} is {} bytes, not a 32-byte seed — the file is \ + truncated or corrupt; re-create the store identity", + path.display(), + bytes.len() + )) + })?; + Ok(digstore_crypto::bls::SecretKey::from_seed(&seed)) } /// Generate a fresh host BLS signing identity: returns the 32-byte seed and the @@ -1844,6 +1881,64 @@ mod tests { assert_eq!(seed.to_vec(), on_disk); } + /// A truncated `signing_key.bin` must be a clean ERROR, not a process abort. + /// + /// `SecretKey::from_seed` is `assert!(seed.len() >= 32)`, so handing it the + /// file bytes unvalidated turns a corrupt file into a panic. Both truncation + /// cases are reachable without an attacker: an `init` interrupted mid-write + /// leaves a short (often zero-length) file. Driving `load_signing_key` — the + /// function the serve paths call — is what makes this load-bearing; asserting + /// on a length check in isolation would survive its removal. + #[test] + fn a_truncated_signing_key_is_an_error_not_a_panic() { + for short in [0usize, 1, 31] { + let (_td, ctx) = ctx(false); + fs::write(ctx.dig_dir.join("signing_key.bin"), vec![0xABu8; short]).unwrap(); + + let err = load_signing_key(&ctx) + .err() + .unwrap_or_else(|| panic!("a {short}-byte key must not be accepted")); + assert!( + matches!(err, CliError::InvalidArgument(_)), + "a {short}-byte key must report a corrupt file, got {err:?}" + ); + } + } + + /// An OVERLONG key file is equally corrupt. `from_seed` accepts `len >= 32`, + /// so without an exact-length check a 64-byte file would silently derive a + /// key from bytes nothing wrote — a different identity than the store's, with + /// no error anywhere. This is the side of the bound a `>= 32` guard misses. + #[test] + fn an_overlong_signing_key_is_rejected_rather_than_silently_truncated() { + let (_td, ctx) = ctx(false); + fs::write(ctx.dig_dir.join("signing_key.bin"), vec![0xABu8; 64]).unwrap(); + + assert!( + matches!(load_signing_key(&ctx), Err(CliError::InvalidArgument(_))), + "a 64-byte key file must be rejected, not truncated to its first 32 bytes" + ); + } + + /// The at-bound case still works: exactly 32 bytes derives the expected key. + /// Without this the two rejection tests above are satisfied by a guard that + /// rejects everything. + #[test] + fn an_exactly_32_byte_signing_key_is_accepted() { + let (_td, ctx) = ctx(false); + let seed = [0x5Au8; 32]; + fs::write(ctx.dig_dir.join("signing_key.bin"), seed).unwrap(); + + let sk = load_signing_key(&ctx).expect("a 32-byte seed is valid"); + assert_eq!( + sk.public_key().to_bytes().0, + digstore_crypto::bls::SecretKey::from_seed(&seed) + .public_key() + .to_bytes() + .0 + ); + } + #[test] fn read_signing_seed_errors_without_store() { let (_td, ctx) = empty_ctx(); diff --git a/crates/digstore-cli/tests/serve_fails_closed.rs b/crates/digstore-cli/tests/serve_fails_closed.rs new file mode 100644 index 0000000..8e4cf83 --- /dev/null +++ b/crates/digstore-cli/tests/serve_fails_closed.rs @@ -0,0 +1,87 @@ +//! The serve path must FAIL CLOSED, not fall back to a world-known default. +//! +//! Two independent fail-open sites lived in `ops::serve` (issue #2553). +//! This file covers the first: a missing host signing key silently became +//! `BlsSecretKey::from_seed(&[42u8; 32])`, a value anyone can reproduce from the +//! source. It is exercised against a REAL store built by the genuine `init` + +//! `add` + `commit` machinery, so the assertions observe the shipped code path +//! rather than a hand-built fixture. +//! +//! The second — the pinned host RNG seed — is asserted in `ops::serve`'s own +//! unit test instead, because neither consumer of that RNG is observable from +//! serve output. + +use digstore_cli::context::CliContext; +use digstore_cli::ops::{serve, store_ops}; +use digstore_core::Urn; + +/// A real committed store: returns its context, the compiled module path, and a +/// URN for the one resource it holds. +struct Fixture { + _td: tempfile::TempDir, + ctx: CliContext, + module_path: std::path::PathBuf, + urn: Urn, +} + +fn committed_store() -> Fixture { + let td = tempfile::tempdir().unwrap(); + let ctx = CliContext::resolve(Some(td.path().to_path_buf()), false, false); + store_ops::init_store(&ctx, false, None, None, None, None, None, None).unwrap(); + + let f = td.path().join("known.txt"); + std::fs::write(&f, b"fail-closed fixture payload 0123456789").unwrap(); + store_ops::add_path(&ctx, &f, Some("known".into())).unwrap(); + + let res = store_ops::commit(&ctx, None, serve::empty_manifest()).unwrap(); + let store_id = ctx.find_store_id().unwrap(); + + Fixture { + _td: td, + ctx, + module_path: res.output_path, + urn: Urn { + chain: "chia".into(), + store_id, + root_hash: None, + resource_key: Some("known".into()), + }, + } +} + +/// CONTROL: with the signing key present the serve path succeeds. Without this +/// the "missing key fails" test below could pass for an unrelated reason (a +/// broken fixture, an uncommitted resource) and prove nothing. +#[test] +fn serving_succeeds_while_the_host_signing_key_is_present() { + let fx = committed_store(); + assert!( + fx.ctx.dig_dir.join("signing_key.bin").exists(), + "init must persist the host signing key" + ); + serve::serve_content_raw(&fx.ctx, &fx.module_path, &fx.urn) + .expect("a store with its signing key serves normally"); +} + +/// FAIL CLOSED: with `signing_key.bin` removed, serving must ERROR rather than +/// silently attest with a world-known key baked into the source. The fallback +/// key is reproducible by anyone reading this repository, so a host using it +/// carries no identity at all — the operator must learn that the key is gone, +/// not be handed a degraded serve that looks like a content problem. +#[test] +fn serving_fails_closed_when_the_host_signing_key_is_missing() { + let fx = committed_store(); + let key_path = fx.ctx.dig_dir.join("signing_key.bin"); + std::fs::remove_file(&key_path).unwrap(); + + let err = serve::serve_content_raw(&fx.ctx, &fx.module_path, &fx.urn) + .expect_err("a host with no signing key must refuse to serve"); + + // Observe the REASON, not merely the failure: the error must name the + // signing key, otherwise this test would also pass on an unrelated break. + let msg = err.to_string(); + assert!( + msg.contains("signing key"), + "error must name the missing signing key, got: {msg}" + ); +} diff --git a/crates/digstore-host/src/runtime.rs b/crates/digstore-host/src/runtime.rs index cb14fad..a46ec1d 100644 --- a/crates/digstore-host/src/runtime.rs +++ b/crates/digstore-host/src/runtime.rs @@ -82,6 +82,13 @@ pub struct HostRuntime { memory: Memory, limits_cfg: ExecutionLimits, _ticker: EpochTicker, + /// Whether this runtime's `host_random_bytes` RNG was pinned to a fixed seed + /// ([`HostDeps::rng_seed`] was `Some`). Recorded so a CALLER can assert what + /// it actually built: the RNG itself is not observable through any export, so + /// without this a test can only inspect the deps struct it hands in — which + /// proves nothing about the deps the production call site constructs. See + /// [`HostRuntime::rng_is_deterministic`]. + rng_seeded: bool, } impl HostRuntime { @@ -124,6 +131,7 @@ impl HostRuntime { let module = Module::new(&engine, module_bytes).map_err(|e| HostError::Wasmtime(e.to_string()))?; + let rng_seeded = deps.rng_seed.is_some(); let rng = match deps.rng_seed { Some(s) => HostRng::from_seed(s), None => HostRng::from_entropy(), @@ -224,9 +232,18 @@ impl HostRuntime { memory, limits_cfg: limits, _ticker: ticker, + rng_seeded, }) } + /// `true` when this runtime's host RNG was pinned to a fixed seed, making + /// every `host_random_bytes` draw reproducible by anyone who can read the + /// seed. Production serve paths MUST build a runtime for which this is + /// `false`; deterministic-fixture tests are the only legitimate `true`. + pub fn rng_is_deterministic(&self) -> bool { + self.rng_seeded + } + /// Set the per-export-call fuel budget. Epoch deadline is added in Task 12. /// NOTE: bounds are armed PER export call (alloc, serve, dealloc each get /// their own budget); the serve flow is not a single combined budget (§18.2). diff --git a/crates/digstore-remote/src/client.rs b/crates/digstore-remote/src/client.rs index bf152b4..61a7783 100644 --- a/crates/digstore-remote/src/client.rs +++ b/crates/digstore-remote/src/client.rs @@ -9,6 +9,24 @@ use digstore_core::{ Bytes32, Bytes96, ContentResponse, Decode, Decoder, Encode, MerkleProof, Tombstone, }; +/// Draw the 32-byte §21.9 per-request nonce from `fill` (the OS CSPRNG in +/// production; a stub in tests). +/// +/// SECURITY — fail closed: a nonce is what makes each signed request unique, so +/// a CSPRNG failure MUST abort the request rather than fall back to a fixed +/// buffer. Signing a constant nonce would make every request byte-identical to +/// the server's replay detector and silently void the protection. The `Result` +/// is what makes the unsafe construction unexpressible: no authed request can be +/// built without real entropy. Matches `identity.rs` and `digstore-chain`'s +/// `seed.rs`, which already propagate the same failure. +fn auth_nonce( + fill: impl FnOnce(&mut [u8]) -> Result<(), getrandom::Error>, +) -> Result<[u8; 32], ClientError> { + let mut nonce = [0u8; 32]; + fill(&mut nonce).map_err(|e| ClientError::Entropy(e.to_string()))?; + Ok(nonce) +} + /// Verify that every chunk in a server-supplied delta actually hashes to the /// content address it is advertised under. Chunks are content-addressed by /// `SHA-256(ciphertext)`, so a server (or MITM) cannot substitute chunk bytes @@ -217,22 +235,41 @@ impl DigClient { req: reqwest::RequestBuilder, method: &str, store_id: &Bytes32, - ) -> reqwest::RequestBuilder { + ) -> Result { + self.authed_with(req, method, store_id, getrandom::getrandom) + } + + /// [`Self::authed`] with the entropy source injected, so a test can drive the + /// REAL header-stamping path under a failing CSPRNG. + /// + /// This exists because `auth_nonce` returning a `Result` is only half the + /// guarantee: the caller must also propagate it. A test that exercises + /// `auth_nonce` alone stays green against `auth_nonce(..).unwrap_or([0u8; + /// 32])` right here — the exact mutation that restores the vulnerability. + /// `authed` is a one-line delegation so there is no second copy of this body + /// for such a mutation to hide in. + fn authed_with( + &self, + req: reqwest::RequestBuilder, + method: &str, + store_id: &Bytes32, + fill: impl FnOnce(&mut [u8]) -> Result<(), getrandom::Error>, + ) -> Result { let Some(identity) = &self.identity else { - return req; + return Ok(req); }; let timestamp = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_secs()) .unwrap_or(0); - let mut nonce = [0u8; 32]; - let _ = getrandom::getrandom(&mut nonce); + let nonce = auth_nonce(fill)?; let msg = digstore_crypto::request_signing_message(method, store_id, timestamp, &nonce); let sig = (identity.sign)(&msg); - req.header("X-Dig-Identity", &identity.pubkey_hex) + Ok(req + .header("X-Dig-Identity", &identity.pubkey_hex) .header("X-Dig-Timestamp", timestamp.to_string()) .header("X-Dig-Nonce", hex::encode(nonce)) - .header("X-Dig-Auth", hex::encode(sig.0)) + .header("X-Dig-Auth", hex::encode(sig.0))) } /// §21.3 fetch: descriptor + root history only. @@ -243,7 +280,7 @@ impl DigClient { self.http.get(self.url(&format!("/stores/{id}"))), "fetch", store_id, - ) + )? .send() .await .map_err(|e| ClientError::Transport(e.to_string()))? @@ -257,7 +294,7 @@ impl DigClient { self.http.get(self.url(&format!("/stores/{id}/roots"))), "roots", store_id, - ) + )? .send() .await .map_err(|e| ClientError::Transport(e.to_string()))? @@ -291,7 +328,7 @@ impl DigClient { self.http.get(self.url(&format!("/stores/{id}/module"))), "module", store_id, - ) + )? .send() .await .map_err(|e| ClientError::Transport(e.to_string()))?; @@ -344,7 +381,7 @@ impl DigClient { .get(self.url(&format!("/stores/{id}/delta?from={from_h}&to={to_h}"))), "delta", store_id, - ) + )? .send() .await .map_err(|e| ClientError::Transport(e.to_string()))?; @@ -367,7 +404,7 @@ impl DigClient { self.http.get(self.url(&format!("/stores/{id}/module"))), "module", store_id, - ); + )?; if let Some(lr) = local_root { req = req.header( reqwest::header::IF_NONE_MATCH, @@ -460,7 +497,7 @@ impl DigClient { .post(self.url(&format!("/stores/{id}/module/upload"))), "push-init", store_id, - ) + )? .header("X-Dig-Signature", &sig_hex) .json(&init_body); if let Some(t) = bearer { @@ -500,7 +537,7 @@ impl DigClient { .put(self.url(&format!("/stores/{id}/module?root={new_root_hex}"))), "push", store_id, - ) + )? .header("X-Dig-Signature", &sig_hex) .header("X-Dig-Upload-Id", &new_root_hex) // §21.4: a node may accept into pending state; the hub ignores this (always @@ -562,7 +599,7 @@ impl DigClient { .post(self.url(&format!("/stores/{id}/module/complete"))), "push-complete", store_id, - ) + )? .header("X-Dig-Signature", &sig_hex) .json(&complete_body); if let Some(t) = bearer { @@ -606,7 +643,7 @@ impl DigClient { self.http.post(self.url(&format!("/stores/{id}/tombstone"))), "tombstone", store_id, - ) + )? .json(&body) .send() .await @@ -634,7 +671,7 @@ impl DigClient { self.http.post(self.url(&format!("/stores/{id}/delta"))), "delta", store_id, - ) + )? .json(&body) .send() .await @@ -1044,4 +1081,76 @@ mod content_tests { Err(ClientError::Decode(_)) )); } + + /// §21.9 FAIL-CLOSED: when the CSPRNG cannot deliver entropy, the nonce is an + /// ERROR — never a usable buffer. Discarding the failure would leave the + /// caller signing an all-zero nonce, which is a constant, so every request + /// would carry the same "unique" value and the replay protection the nonce + /// exists to provide would be silently gone. + #[test] + fn auth_nonce_fails_closed_when_entropy_is_unavailable() { + let err = auth_nonce(|_| Err(getrandom::Error::UNSUPPORTED)) + .expect_err("entropy failure must not yield a nonce"); + assert!(matches!(err, ClientError::Entropy(_)), "got {err:?}"); + } + + /// An identified client whose signer records nothing — enough to reach the + /// nonce draw in `authed_with`. + fn identified_client() -> DigClient { + DigClient::new("http://127.0.0.1:1").with_identity(RequestIdentity { + pubkey_hex: "ab".repeat(48), + sign: Box::new(|_| Bytes96([0u8; 96])), + }) + } + + /// §21.9 FAIL-CLOSED **at the call site**: `authed` must propagate the nonce + /// failure, not merely be able to observe one. + /// + /// `auth_nonce`'s own test proves the helper returns `Err`; it says nothing + /// about what the caller does with it. Replace the draw with + /// `auth_nonce(..).unwrap_or([0u8; 32])` and that test stays green while + /// every request goes back to signing a constant nonce. This test drives the + /// real header-stamping path and fails on exactly that mutation. + #[test] + fn authed_propagates_an_entropy_failure_instead_of_signing_a_constant_nonce() { + let client = identified_client(); + let req = client.http.get("http://127.0.0.1:1/"); + + let err = client + .authed_with(req, "fetch", &Bytes32([7u8; 32]), |_| { + Err(getrandom::Error::UNSUPPORTED) + }) + .expect_err("a request must not be stamped without a real nonce"); + assert!(matches!(err, ClientError::Entropy(_)), "got {err:?}"); + } + + /// The control: with entropy available the SAME path stamps the request. + /// Without this, the test above is satisfied by an `authed` that always + /// errors. + #[test] + fn authed_stamps_the_request_when_entropy_is_available() { + let client = identified_client(); + let req = client.http.get("http://127.0.0.1:1/"); + + let stamped = client.authed_with(req, "fetch", &Bytes32([7u8; 32]), |buf| { + buf.fill(0x11); + Ok(()) + }); + assert!( + stamped.is_ok(), + "entropy available must yield a signed request" + ); + } + + /// The success path still produces a fresh 32-byte nonce from the supplied + /// source (and is not silently zeroed). + #[test] + fn auth_nonce_uses_the_entropy_it_is_given() { + let nonce = auth_nonce(|buf| { + buf.fill(0xA5); + Ok(()) + }) + .expect("entropy available"); + assert_eq!(nonce, [0xA5u8; 32]); + } } diff --git a/crates/digstore-remote/src/error.rs b/crates/digstore-remote/src/error.rs index 22693a3..f6701a4 100644 --- a/crates/digstore-remote/src/error.rs +++ b/crates/digstore-remote/src/error.rs @@ -70,6 +70,11 @@ pub enum ClientError { /// actionable diagnostic; the raw HTML is deliberately NOT surfaced. #[error("remote returned a non-JSON response ({status}): {hint}")] NonJsonResponse { status: u16, hint: String }, + /// The OS CSPRNG could not supply the entropy a §21.9 request nonce needs. + /// The request is REFUSED rather than sent with a predictable nonce, which + /// would silently void the replay protection the nonce exists to provide. + #[error("could not obtain secure randomness for the request nonce: {0}")] + Entropy(String), } #[cfg(test)]