From 3ec0aa49a52488512280766c88ce2d5e0ce313fd Mon Sep 17 00:00:00 2001 From: Eric Moore Date: Tue, 1 Sep 2026 15:44:42 -0500 Subject: [PATCH 1/3] fix: keyring key mints go through the SP 800-90B latch (#207 item 6); name the steward-key drift (#176) #207 item 6 -- the #74 invariant did not hold where it matters most. v5.6.0 made keygen fail-secure so "no weak key is ever produced". But ciris-keyring's OWN mints drew raw OsRng, bypassing the SP 800-90B health latch. The invariant therefore held for ciris-crypto-constructed keys and NOT for keyring-sealed ones -- which are the federation identity keys. Every key-material draw now routes through ciris_crypto::random::fill: the sealed Ed25519 seed, the sealed ML-DSA-65 seed, the USB-wrapped PQC seed, the transport identity, the software wrapper key, and three P-256 mints via a new mint_p256_signing_key. That helper puts the BYTES through the latch rather than probing it and then drawing unchecked, so the key material is literally what the checked path produced. This required making ciris-crypto a NON-optional keyring dependency, at the `random` feature only. Gating the latch behind pqc-ml-dsa meant the DEFAULT keyring build had no latch at all -- the gap was structural, not incidental. Incidental draws are deliberately untouched: a temp-dir suffix and a TPM attestation nonce are not key material, and routing them would blur what the rule is for. Proven by a fail-secure test -- the one #74 gave every ciris-crypto primitive and the keyring never had. It uses the already-public __force_health_for_test hatch; exposing test_support across the crate boundary was tried first and reverted, because dev-dependency features do not propagate that way. #176 -- the title says build record; the failing decode is the steward key. validation::query_https_source -> get_steward_key -> StewardKeyResponse, which requires `classical`. Two months of that message pointing people at build records. The parse itself cannot be fixed here. The registry sends the multi-steward shape; verify's modern parser for it REQUIRES a response_signature the registry does not send; and relaxing that would mean silently accepting unsigned trust-root material. That is a cross-repo contract decision (CIRISRegistry#133), so this does not paper over it. What IS fixable is the misdirection: the error now names the endpoint, the drift, the tracking issue, and "NOT a build-record problem and NOT a network problem". An unrelated parse error passes through untouched, asserted by test. Note v13.3.0 already fixed the "HTTPS unreachable" mislabel on this path; I reported this half as unfixed during the backlog pass because I checked the build-record path rather than the one that actually fails. 1399 workspace green, clippy clean, doc clean, guards pass. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FZPhbXWTzykcbotEX9BN2p --- CLAUDE.md | 2 +- evidence/cc_impl.tsv | 1 + src/ciris-keyring/Cargo.toml | 13 +++- src/ciris-keyring/src/keyring_storage.rs | 5 +- src/ciris-keyring/src/lib.rs | 71 ++++++++++++++++++++ src/ciris-keyring/src/sealed_ed25519.rs | 13 +++- src/ciris-keyring/src/sealed_mldsa65.rs | 10 ++- src/ciris-keyring/src/software.rs | 17 +++-- src/ciris-keyring/src/transport_identity.rs | 9 ++- src/ciris-keyring/src/usb_wrapped_mldsa65.rs | 8 ++- src/ciris-verify-core/src/https.rs | 64 +++++++++++++++++- 11 files changed, 191 insertions(+), 22 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 087aeed3..8e7b5285 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -258,7 +258,7 @@ Newest first, **one release per line** — so adding a release is a one-line dif rather than a rewrite of the whole history (it was previously a single 98,000-character line, which made every release note unreviewable in `git diff`). -- **14.1.0** — **a false alarm that also disabled the real alarm (#224), and a user code that can carry its own nodes (#269)**. **(1) #224 — `verify_manifest_integrity` reported a benign algorithm mismatch as tampering, and skipped the actual check.** This repository contains **three** `manifest_hash` constructions — concatenated per-file hashes (`file_integrity`), concatenated per-*function* hashes (`ciris-manifest-tool`), and SHA-256 over serde-JSON bytes (`ciris-build-tool`) — so a manifest produced by one and checked by another can never match. The old code recomputed exactly one, **self-diagnosed the benign cause in its own warning**, and still reported a tampering-shaped ERROR: 14 of them in a 4h window on `datum`, one per Discord reconnect. **The half nobody reported is worse:** `check_full` then returned early with `files_checked: 0`, so the per-file hashes — the control that can actually detect tampering — were never checked. That is the same symptom #176 reports as *"L4 file integrity will be skipped"*. Now it **tries the other construction** instead of guessing, and returns `ManifestHashCheck::{VerifiedConcatenated, VerifiedJson, Unrecognized}`. `Unrecognized` is deliberately not called *mismatch*: with only `files` + `manifest_hash` in hand a fourth construction is **indistinguishable from tampering**, so asserting either manufactures a verdict from a measurement we cannot make (`MISSION.md` §1.4 — the distinction v13.3.0 drew for revocation). **A contract change consumers must know about:** an unrecognized hash no longer flips `integrity_valid`; it is surfaced on its own field and the per-file check *runs*. Altering `manifest_hash` alone achieves nothing anyway — the per-file hashes are untouched, and an attacker who altered those would simply recompute it, which this check never caught either. It is a self-consistency checksum, not an authentication. A malformed manifest (no files / empty hash) is still a hard failure. **(2) #269 — fedcode may embed the owner's nodes, so a contact resolves with no directory.** CC 5.4.6 names the population v1 cannot serve: *"phone-class peers that cannot hold the full directory"* — first contact, a QR across a table, an air-gapped hand-off. **The proposal was called "fedcode v2" and the wire format has been at v2 since the kind-tagged code shipped**, so this mints **v3** (`CIRIS-V3-`); minting it as v2 would have collided with a live encoding. Additive and free: a code with no nodes still emits **byte-identical v2**, so nothing already issued moves, and `MAY` genuinely costs nothing. **The safety property is the point.** `OwnedNode` carries the node's **transport** Ed25519 — never the owner's federation key. CIRISServer#335 is what that confusion cost: nodes primed the canonical at `1fc232535a…` while it served on `81cabcf78a…`, every node reported `knows_peer=true, provenance=Rooted, primed=1, refused=0`, and **zero traces arrived** — then the false rooting *prevented* recovery, because a node that believes it knows a peer never learns the real address. What made it survive review is that transport and federation share the Ed25519 half, so the derivation looks sound; sharing a key does not make a base hash and a named hash the same address. Refused at **both** encoder and decoder, because a code minted by another implementation is exactly the case an encoder cannot police. Only a `user` code may carry nodes (a group's destinations are group-scoped material a code must not carry at all, CC 5.4.6), and the list is capped at 16 so a code stays scannable. 7 v3 tests + 4 manifest tests; 1396 workspace green, clippy clean. +- **14.1.0** — **four backlog fixes: a false alarm that also disabled the real alarm (#224), a user code that can carry its own nodes (#269), the keyring's own key mints bypassing the RNG latch (#207 item 6), and a two-month misdiagnosis in a log line (#176)**. **(3) #207 item 6 — the #74 invariant did not hold where it matters most.** v5.6.0 made keygen fail-secure so *"no weak key is ever produced"* — but `ciris-keyring`'s own mints drew **raw `OsRng`**, bypassing the SP 800-90B health latch entirely. So the invariant held for `ciris-crypto`-constructed keys and **not** for keyring-sealed ones, which are the **federation identity keys**. Every key-material draw now routes through `ciris_crypto::random::fill` — the sealed Ed25519 seed, the sealed ML-DSA-65 seed, the USB-wrapped PQC seed, the transport identity, the software wrapper key, and three P-256 mints via a new `mint_p256_signing_key` (which puts the **bytes** through the latch rather than merely probing it and then drawing unchecked). This required making `ciris-crypto` a **non-optional** keyring dependency at the `random` feature only: gating the latch behind `pqc-ml-dsa` meant the *default* keyring build had no latch at all. Incidental draws are deliberately left alone — a temp-dir suffix and a TPM attestation nonce are not key material. Proven by a fail-secure test, the one #74 gave every `ciris-crypto` primitive and the keyring never had. **(4) #176 — the title says build record; the failing decode is the steward key.** `validation::query_https_source` → `get_steward_key` → `StewardKeyResponse`, which requires `classical`. Two months of pointing at build records. The parse itself **cannot** be fixed here — the registry sends the multi-steward shape, verify's modern parser for it *requires* a `response_signature` the registry does not send, and relaxing that would mean silently accepting **unsigned trust-root material** (escalated as CIRISRegistry#133) — so the honest fix is to stop misdirecting the reader: the error now names the endpoint, the drift, the tracking issue, and *"NOT a build-record problem and NOT a network problem"*. An unrelated parse error is passed through untouched, asserted by test. v13.3.0 had already fixed the *"HTTPS unreachable"* mislabel on this path; this fixes the remaining misdirection. **(1) #224 — `verify_manifest_integrity` reported a benign algorithm mismatch as tampering, and skipped the actual check.** **(1) #224 — `verify_manifest_integrity` reported a benign algorithm mismatch as tampering, and skipped the actual check.** This repository contains **three** `manifest_hash` constructions — concatenated per-file hashes (`file_integrity`), concatenated per-*function* hashes (`ciris-manifest-tool`), and SHA-256 over serde-JSON bytes (`ciris-build-tool`) — so a manifest produced by one and checked by another can never match. The old code recomputed exactly one, **self-diagnosed the benign cause in its own warning**, and still reported a tampering-shaped ERROR: 14 of them in a 4h window on `datum`, one per Discord reconnect. **The half nobody reported is worse:** `check_full` then returned early with `files_checked: 0`, so the per-file hashes — the control that can actually detect tampering — were never checked. That is the same symptom #176 reports as *"L4 file integrity will be skipped"*. Now it **tries the other construction** instead of guessing, and returns `ManifestHashCheck::{VerifiedConcatenated, VerifiedJson, Unrecognized}`. `Unrecognized` is deliberately not called *mismatch*: with only `files` + `manifest_hash` in hand a fourth construction is **indistinguishable from tampering**, so asserting either manufactures a verdict from a measurement we cannot make (`MISSION.md` §1.4 — the distinction v13.3.0 drew for revocation). **A contract change consumers must know about:** an unrecognized hash no longer flips `integrity_valid`; it is surfaced on its own field and the per-file check *runs*. Altering `manifest_hash` alone achieves nothing anyway — the per-file hashes are untouched, and an attacker who altered those would simply recompute it, which this check never caught either. It is a self-consistency checksum, not an authentication. A malformed manifest (no files / empty hash) is still a hard failure. **(2) #269 — fedcode may embed the owner's nodes, so a contact resolves with no directory.** CC 5.4.6 names the population v1 cannot serve: *"phone-class peers that cannot hold the full directory"* — first contact, a QR across a table, an air-gapped hand-off. **The proposal was called "fedcode v2" and the wire format has been at v2 since the kind-tagged code shipped**, so this mints **v3** (`CIRIS-V3-`); minting it as v2 would have collided with a live encoding. Additive and free: a code with no nodes still emits **byte-identical v2**, so nothing already issued moves, and `MAY` genuinely costs nothing. **The safety property is the point.** `OwnedNode` carries the node's **transport** Ed25519 — never the owner's federation key. CIRISServer#335 is what that confusion cost: nodes primed the canonical at `1fc232535a…` while it served on `81cabcf78a…`, every node reported `knows_peer=true, provenance=Rooted, primed=1, refused=0`, and **zero traces arrived** — then the false rooting *prevented* recovery, because a node that believes it knows a peer never learns the real address. What made it survive review is that transport and federation share the Ed25519 half, so the derivation looks sound; sharing a key does not make a base hash and a named hash the same address. Refused at **both** encoder and decoder, because a code minted by another implementation is exactly the case an encoder cannot police. Only a `user` code may carry nodes (a group's destinations are group-scoped material a code must not carry at all, CC 5.4.6), and the list is capped at 16 so a code stays scannable. 7 v3 tests + 4 manifest tests; 1396 workspace green, clippy clean. - **14.0.0** (BREAKING) — **three downstream-reported defects, all of them about a promise the type system was not keeping (#257, #267, #265)**. **(1) #257 — a new error variant was a semver break shipped as a MINOR.** v13.3.0 added `VerifyError::ResponseSchemaMismatch`; the enum had no `#[non_exhaustive]`, so under Cargo semver that breaks any downstream `match` without a `_` arm — on a plain `cargo update` against `version = "13"`. CIRISPersist reported it and was careful to say *its own* code was unaffected **by luck, not design**, which is the right way to report a hazard you did not personally hit. **The fix is deliberately NOT "add it to all 110 public enums."** `#[non_exhaustive]` is right for **open** sets (errors, capability lists that grow) and **wrong** for **closed wire vocabularies** — for CoTS `Purpose`, `RecordType`, `InvocationKind`, `CohortScope` and friends, an exhaustive downstream match is a *feature*: a new variant there means the wire format moved and consumers must be forced to notice. That is the 13.0.0 `Normative`-vs-`Structural` distinction applied to packaging. So: **31 error enums annotated, 13 closed vocabularies deliberately left, `CirisVerifyError` left because it is `#[repr(C)]` and its variant set is an ABI contract.** The change immediately broke two in-workspace matches (`wheel_hybrid_kex`, `wheel_key_grant`) — the mechanism proving itself — both now carrying a `_` arm that fails closed. **(2) #267 — the only key-minting path could not express expiry**, hardcoding `valid_until: None`. The naive fix is a bare field, and it would have been **wrong in the #252 way**: `valid_until` was not in the signed envelope, so an expiry set there is **strippable** — drop it and the record still verifies. So it rides **inside** the scrub-signed envelope, materialize-when-present per CEG §0.9, which means `None` reproduces the pre-14.0 bytes **exactly** and every existing record, signature and golden vector stays valid. `valid_until_in_envelope()` is the accessor a consumer should read when the answer drives a decision; a test asserts stripping it breaks the content hash. Exposed on `produce_self_key_record` / `produce_scrubbed_key_record` / `produce_multiscrub_key_record` / `create_federation_identity`, the CLI (`--valid-until`) and the FFI. **Accord holder records deliberately pass `None`** — a self-asserted expiry on the constitutional kill-switch custody root would create a date after which the accord silently has fewer holders than its quorum needs; rotation is the m-of-n ceremony, never a timeout. **(3) #265 — the CIRIS Logging Standard.** Motivated by a real incident: a user's log ran **3,810 lines for 3m40s**, their actual fault (`HTTP 401`) appeared 13× and was unfindable, and they lost a day to it. Substrate output bypasses Python `logging` entirely, so the agent-side fixes cannot reach us — whatever this crate emits is what the operator gets, and **verify's default filter is `warn`**, so every `warn!` here is on for every user. Two §2/§1.1 violations found and fixed: a **three-line WARN banner** in `tpm_windows` (a banner is not a failure) and `conformance::log_report`, a **box-drawing ASCII table at ~15 events per report with individual table rows on the WARN channel** — now one structured INFO summary, per-test detail at DEBUG, one WARN per actual failure, one ERROR overall. §5 (*never log secrets*) already held by construction; the standard asks for it *with a test*, so **`scripts/check-no-secret-logging.sh`** parses each tracing macro's **balanced argument list** (a `grep -A` window flagged ordinary code that merely followed a log call) and fails on interpolated seed/private-key/PIN/DEK material. Negative-tested, wired into CI. 1377 workspace green, clippy clean. - **13.6.1** — **docs: the epoch-binding is CONTINGENT, and CC's Position record says on what**. CIRISConstitution rc4.2 (`1561fb1`) appended an informative **Position** paragraph to CC 5.4.6 — the CIRISVerify#262/#91 prior-art record — which names the nearest admissible multi-hop relaxation (**blinded retained state: Tor v3 / I2P b33**) and attaches the field's rotation rule: **rotation clocks must be global, never group-event-driven.** That lands directly on code verify owns. `derive_destination` rotates on the **MLS epoch**, which advances on Add/Remove and is therefore *exactly* group-event-driven. v13.6.0 documented that as *"a feature here and a defect there"* — true, but incomplete in a misleading direction: it reads as *epoch-binding is right*, when the correct claim is **epoch-binding is right because nothing is emitted**. Under any multi-hop relaxation the derivation survives and its *schedule* does not. Recorded at `derive_destination` (where an implementer of that amendment will actually look) and in `announce_policy`'s open-questions note. Docs-only; no derivation, constant or vector moves. Also of note from the same record, for anyone reading the positioning rather than the rule: the design sits in the **zero-emission / membership-concealment corner** with the MCON impossibility floor (Vasserman et al., CCS 2009) satisfied *entirely by members*, and buys back the corner's two known prices — the **two-plane split** (public identity plane, derived group plane; derivation replaces discovery, dissolving the darknet-bootstrap problem) and **determinism replacing coordination** (the member-relay ALM tree is a pure function every member computes identically, recovering ⌈log_k N⌉ fan-out without a coordinator that would have to learn the group exists — CC 6.1.6). The ballot corollary is recorded without minting conformance surface: *a classical secret ballot hides the vote; structural invisibility hides the election.* - **13.6.0** — **CC 5.4.6 ruled (CIRISConstitution#91): the announce prohibition binds the EMISSION, so a targeted announce inherits it — encoded, not merely noted (#262)**. CIRISEdge found scoped addresses are **one hop by construction**: edge supplies an explicit destination hash, leviculum correctly refuses to announce those (an announce for a caller-supplied hash emits a `destination_hash` that no Python-RNS peer recomputes), so no transport node learns a path. Edge proposed per-group **identities** — let RNS compute the hash natively, making the destination announceable and multi-hop — which would have demoted `derive_destination` from *address* to *name*, and **correctly refused to pick alone**, filing #262 to couple it with #259's still-open label rather than shipping a wire fact unilaterally. My first read foreclosed it on CC 5.4.6's flat *"MUST NOT emit a Reticulum announce"* + the fail-secure clause forbidding fallback-to-announce *to recover reachability*. **That read was too fast**: a **targeted** announce iterated over the roster leaks nothing to an outsider, so it satisfies 5.4.6's purposive gloss (*"the announce **that would reveal**…"*) while reading against the flat text — and CC contained **no notion of a targeted announce anywhere**, having been drafted when announce ≡ broadcast. Rather than resolve a ratified rule locally to unblock a downstream — the exact failure `Gating::Normative { authority }` exists to prevent — it went to CC. **Ruled: the packet.** Three legs, now in-clause: the purposive gloss is *rationale, not exception* (no directed announce on RNS can satisfy it — multi-hop path learning **is** outsider observation, and retained path state is precisely the edge class the subpoena framing promises does not exist); the flat MUST NOT was never broadcast-era shorthand (the same section bans the *targeted, non-broadcast* per-destination query in the same breath); and a directed announce **trades a claimable guarantee for an unclaimable one** — *no emission exists* (structural) for *emissions exist but resist analysis* (traffic-analysis privacy, which CEG/RET declines to claim). New `announce_policy::CohortScope` encodes the partition — the four below-federation tiers may not announce, the four Commons tiers may — with **no addressing-mode parameter**, since the ruling binds the emission; a test pins that so adding one is a visible act. `wire_may_announce` **fails closed on an unknown scope**, per 5.4.6's *"MUST fail toward suppression, never toward announce"*. Tagged `Normative(CC 5.4.6, ratified CIRISConstitution#91)`. **The ruling also resolved a dilemma neither issue reached:** 5.4.6's derivation is epoch-bound, so under the announceable reading every Add/Remove forces a synchronized roster-wide re-announce **wave** (leaking cardinality, timing, churn), while not rotating leaves a removed member holding every peer's addressing forever. Under the packet reading there is no wave because there is no emission — **epoch-binding is a feature here and a defect there**, which is part of why the alternative lost. v13.4.0's derivation and v13.5.0's `DESTINATION_EXPORTER_LABEL` are confirmed correctly targeted; multi-hop scoped reach stays open on the amendment plane with its bar stated (no outsider-observable emission, no outsider-retained path state, no epoch-correlated wave). 5 tests; 1373 workspace green, clippy clean. diff --git a/evidence/cc_impl.tsv b/evidence/cc_impl.tsv index e198aee0..361b59eb 100644 --- a/evidence/cc_impl.tsv +++ b/evidence/cc_impl.tsv @@ -71,3 +71,4 @@ UNASSIGNED CLM-scope-destination CIRISVerify src/ciris-crypto/src/scope_privacy. 5.4.6 CLM-announce-suppress CIRISVerify src/ciris-verify-core/src/announce_policy.rs#may_announce ciris-verify-core@v13.6.0 UNASSIGNED CLM-key-validity-window CIRISVerify src/ciris-verify-core/src/federation_self_record.rs#valid_until_in_envelope ciris-verify-core@v14.0.0 UNASSIGNED CLM-fedcode-owned-nodes CIRISVerify src/ciris-verify-core/src/fedcode.rs#OwnedNode ciris-verify-core@v14.1.0 +UNASSIGNED CLM-keyring-rng-latch CIRISVerify src/ciris-keyring/src/lib.rs#mint_p256_signing_key ciris-keyring@v14.1.0 diff --git a/src/ciris-keyring/Cargo.toml b/src/ciris-keyring/Cargo.toml index d58803da..e53dc0b9 100644 --- a/src/ciris-keyring/Cargo.toml +++ b/src/ciris-keyring/Cargo.toml @@ -32,7 +32,7 @@ keyring-storage = ["dep:keyring"] # Post-quantum cryptography: enables ML-DSA-65 software signer + PqcSigner trait. # Used by CIRISPersist's cold-path PQC fill-in flow (federation_keys etc.) so # downstream consumers don't reach into the ml-dsa crate directly. -pqc-ml-dsa = ["dep:ciris-crypto"] +pqc-ml-dsa = ["ciris-crypto/pqc-ml-dsa", "ciris-crypto/ed25519", "ciris-crypto/self-enc", "ciris-crypto/hybrid-kex"] [dependencies] async-trait.workspace = true @@ -83,7 +83,16 @@ keyring = { version = "3", optional = true } # CIRIS cryptographic primitives — gated behind pqc-ml-dsa so the existing # default ciris-keyring build doesn't pull in ml-dsa. -ciris-crypto = { path = "../ciris-crypto", version = "14", optional = true, default-features = false, features = ["pqc-ml-dsa", "ed25519", "self-enc", "hybrid-kex"] } +# NON-optional, deliberately (CIRISVerify#207 item 6). The keyring mints key +# SEEDS, and #74's "no weak key is ever produced" invariant lives in +# `ciris_crypto::random::fill` — the SP 800-90B health latch. Gating that +# behind an optional feature meant the default keyring build drew raw `OsRng` +# and the invariant simply did not hold for sealed federation keys. +# +# The base dependency is the `random` feature only, which is `[]` plus the +# `rand_core` this crate already pulls — so the light default stays light. The +# heavier PQC surface is still feature-gated below. +ciris-crypto = { path = "../ciris-crypto", version = "14", default-features = false, features = ["random"] } # Platform-specific [target.'cfg(target_os = "android")'.dependencies] diff --git a/src/ciris-keyring/src/keyring_storage.rs b/src/ciris-keyring/src/keyring_storage.rs index 714cc861..e7f9d28b 100644 --- a/src/ciris-keyring/src/keyring_storage.rs +++ b/src/ciris-keyring/src/keyring_storage.rs @@ -107,10 +107,7 @@ impl KeyringStorageSigner { /// Generate a new key and store it in the keyring. pub fn generate_and_store(&mut self) -> Result<(), KeyringError> { - use p256::ecdsa::SigningKey; - use rand_core::OsRng; - - let signing_key = SigningKey::random(&mut OsRng); + let signing_key = crate::mint_p256_signing_key()?; let key_bytes = signing_key.to_bytes(); self.store_key(&key_bytes)?; diff --git a/src/ciris-keyring/src/lib.rs b/src/ciris-keyring/src/lib.rs index 95bafab3..e7f3cc95 100644 --- a/src/ciris-keyring/src/lib.rs +++ b/src/ciris-keyring/src/lib.rs @@ -145,6 +145,38 @@ pub mod platform; pub mod keyring_storage; pub use error::KeyringError; + +/// Mint a P-256 signing key from **latch-checked** randomness +/// (CIRISVerify#207 item 6 / #74). +/// +/// `SigningKey::random(&mut OsRng)` draws straight from the OS RNG, bypassing +/// the SP 800-90B startup health latch that #74 added so *"no weak key is ever +/// produced"*. That invariant therefore held for `ciris-crypto`-constructed +/// keys and **not** for keyring-minted ones — which are the federation +/// identity keys, i.e. the ones that matter. +/// +/// The bytes themselves go through [`ciris_crypto::random::fill`], rather than +/// merely probing the latch and then drawing unchecked, so the key material is +/// literally what the checked path produced. +/// +/// # Errors +/// [`KeyringError::KeyGenerationFailed`] if the RNG health latch has tripped, +/// or (with probability under 2⁻³²) if the draw is not a valid P-256 scalar. +/// Refusing is the fail-secure answer in both cases: a retry loop around a +/// possibly-broken RNG is not an improvement. +pub fn mint_p256_signing_key() -> Result { + let mut bytes = [0u8; 32]; + ciris_crypto::random::fill(&mut bytes).map_err(|e| KeyringError::KeyGenerationFailed { + reason: format!("RNG health check failed; refusing to mint a P-256 key: {e}"), + })?; + let key = p256::ecdsa::SigningKey::from_slice(&bytes).map_err(|e| { + KeyringError::KeyGenerationFailed { + reason: format!("random draw was not a valid P-256 scalar: {e}"), + } + })?; + Ok(key) +} + pub use hw_token::{ get_token_signer, hardware_class_table, resolve_hardware_class, HardwareClassRule, ProbedToken, TokenInterface, GENERIC_EXTERNAL_TOKEN_CLASS, @@ -231,3 +263,42 @@ pub fn get_platform_signer(alias: &str) -> Result, Keyri pub fn is_hardware_available() -> bool { detect_hardware_type().has_hardware } + +#[cfg(test)] +mod rng_latch { + /// **CIRISVerify#207 item 6 / #74.** The keyring mints the federation + /// identity keys, and its mints drew raw `OsRng` — so "no weak key is + /// ever produced" held for `ciris-crypto` keys and not for these. + /// + /// #74 proved that invariant with a per-primitive fail-secure test. This + /// is the one the keyring was missing. + #[test] + fn minting_refuses_on_a_tripped_rng_latch() { + use ciris_crypto::rng_health::{__force_health_for_test, RngHealth}; + + // Restore on the way out even if the assert panics, so a failure here + // cannot leave the latch tripped for another test on this thread. + struct Restore; + impl Drop for Restore { + fn drop(&mut self) { + __force_health_for_test(RngHealth::Healthy); + } + } + let _restore = Restore; + + __force_health_for_test(RngHealth::Failed { + test: ciris_crypto::rng_health::TEST_REPETITION_COUNT, + detail: "forced for the keyring fail-secure test".to_string(), + }); + assert!( + matches!( + super::mint_p256_signing_key(), + Err(crate::KeyringError::KeyGenerationFailed { .. }) + ), + "a keyring mint MUST refuse when the RNG health latch has tripped" + ); + + __force_health_for_test(RngHealth::Healthy); + assert!(super::mint_p256_signing_key().is_ok()); + } +} diff --git a/src/ciris-keyring/src/sealed_ed25519.rs b/src/ciris-keyring/src/sealed_ed25519.rs index 8a5270d9..e3e39eee 100644 --- a/src/ciris-keyring/src/sealed_ed25519.rs +++ b/src/ciris-keyring/src/sealed_ed25519.rs @@ -84,8 +84,17 @@ impl SealedEd25519Signer { match adopt_seed { Some(existing) => s.copy_from_slice(existing), None => { - use rand_core::{OsRng, RngCore}; - OsRng.fill_bytes(&mut s); + // #207 item 6 / #74: route key material through the SP 800-90B health + // latch. A raw `OsRng` draw here bypassed it, so "no weak key is ever + // produced" held for ciris-crypto keys and NOT for the keyring-sealed + // federation keys — the ones that actually matter. + ciris_crypto::random::fill(&mut s).map_err(|e| { + KeyringError::KeyGenerationFailed { + reason: format!( + "RNG health check failed; refusing to mint a seed: {e}" + ), + } + })?; }, } storage.store(SEED_KEY_ID, &s)?; diff --git a/src/ciris-keyring/src/sealed_mldsa65.rs b/src/ciris-keyring/src/sealed_mldsa65.rs index b241e84b..f9e187bc 100644 --- a/src/ciris-keyring/src/sealed_mldsa65.rs +++ b/src/ciris-keyring/src/sealed_mldsa65.rs @@ -122,8 +122,14 @@ impl SealedMlDsa65Signer { match adopt_seed { Some(existing) => s.copy_from_slice(existing), None => { - use rand_core::{OsRng, RngCore}; - OsRng.fill_bytes(&mut s); + // #207 item 6 / #74: key material goes through the SP 800-90B health + // latch. A raw `OsRng` draw bypassed it, so "no weak key is ever + // produced" held for ciris-crypto keys and NOT for keyring-minted ones. + ciris_crypto::random::fill(&mut s).map_err(|e| { + KeyringError::KeyGenerationFailed { + reason: format!("RNG health check failed; refusing to mint: {e}"), + } + })?; }, } storage.store(SEED_KEY_ID, &s)?; diff --git a/src/ciris-keyring/src/software.rs b/src/ciris-keyring/src/software.rs index 62dfc895..c6b31778 100644 --- a/src/ciris-keyring/src/software.rs +++ b/src/ciris-keyring/src/software.rs @@ -18,7 +18,6 @@ use ed25519_dalek::{ Signature as Ed25519Signature, Signer as Ed25519SignerTrait, SigningKey as Ed25519SigningKey, }; use p256::ecdsa::{Signature, SigningKey}; -use p256::elliptic_curve::rand_core::OsRng; use crate::error::KeyringError; use crate::signer::{HardwareSigner, KeyGenConfig}; @@ -142,7 +141,7 @@ impl SoftwareSigner { alias = %alias, "SoftwareSigner: generating new ECDSA P-256 key" ); - let key = SigningKey::random(&mut OsRng); + let key = crate::mint_p256_signing_key()?; // Persist to disk let key_bytes = key.to_bytes(); @@ -201,7 +200,7 @@ impl SoftwareSigner { /// Generate a new random key and persist it. pub fn generate_random_key(&mut self) -> Result<(), KeyringError> { - let key = SigningKey::random(&mut OsRng); + let key = crate::mint_p256_signing_key()?; // Persist to disk let key_bytes = key.to_bytes(); @@ -1520,9 +1519,15 @@ impl MutableEd25519Signer { ); // Generate random 32-byte seed - use rand_core::{OsRng, RngCore}; let mut key_bytes = [0u8; 32]; - OsRng.fill_bytes(&mut key_bytes); + // #207 item 6 / #74: key material goes through the SP 800-90B health + // latch. A raw `OsRng` draw bypassed it, so "no weak key is ever + // produced" held for ciris-crypto keys and NOT for keyring-minted ones. + ciris_crypto::random::fill(&mut key_bytes).map_err(|e| { + KeyringError::KeyGenerationFailed { + reason: format!("RNG health check failed; refusing to mint: {e}"), + } + })?; // Use import_key which handles hardware wrapping self.import_key(&key_bytes)?; @@ -2389,7 +2394,7 @@ mod tests { async fn test_software_signer_sign_and_verify() { use p256::ecdsa::signature::Verifier; - let signing_key = SigningKey::random(&mut OsRng); + let signing_key = SigningKey::random(&mut rand_core::OsRng); let verifying_key = *signing_key.verifying_key(); let key_path = test_key_dir().join("test_sign.p256.key"); diff --git a/src/ciris-keyring/src/transport_identity.rs b/src/ciris-keyring/src/transport_identity.rs index 8edd2c7a..d7864424 100644 --- a/src/ciris-keyring/src/transport_identity.rs +++ b/src/ciris-keyring/src/transport_identity.rs @@ -153,10 +153,13 @@ impl TransportIdentityKeystore for BlobTransportKeystore { } fn generate_and_store(&self, key_id: &str) -> Result<(), KeyringError> { - use rand_core::{OsRng, RngCore}; - let mut bytes = [0u8; TRANSPORT_IDENTITY_LEN]; - OsRng.fill_bytes(&mut bytes); + // #207 item 6 / #74: key material goes through the SP 800-90B health + // latch. A raw `OsRng` draw bypassed it, so "no weak key is ever + // produced" held for ciris-crypto keys and NOT for keyring-minted ones. + ciris_crypto::random::fill(&mut bytes).map_err(|e| KeyringError::KeyGenerationFailed { + reason: format!("RNG health check failed; refusing to mint: {e}"), + })?; let result = self.storage.store(key_id, &bytes[..]); // Best-effort scrub of the transient buffer. The threat model this // closes is at-rest exfil (the AV-17 carve-out concedes transient diff --git a/src/ciris-keyring/src/usb_wrapped_mldsa65.rs b/src/ciris-keyring/src/usb_wrapped_mldsa65.rs index 459b18dd..fbd47f4a 100644 --- a/src/ciris-keyring/src/usb_wrapped_mldsa65.rs +++ b/src/ciris-keyring/src/usb_wrapped_mldsa65.rs @@ -156,7 +156,13 @@ impl UsbWrappedMlDsa65Signer { let mut seed = [0u8; SEED_LEN]; match adopt_seed { Some(s) => seed.copy_from_slice(s), - None => OsRng.fill_bytes(&mut seed), + // #207 item 6 / #74: a PQC seed is key material, so it goes + // through the SP 800-90B health latch rather than raw OsRng. + None => ciris_crypto::random::fill(&mut seed).map_err(|e| { + KeyringError::KeyGenerationFailed { + reason: format!("RNG health check failed; refusing to mint a seed: {e}"), + } + })?, } // The wrap key is derived from a deterministic Ed25519 signature. Prove diff --git a/src/ciris-verify-core/src/https.rs b/src/ciris-verify-core/src/https.rs index cb693a85..33b1768f 100644 --- a/src/ciris-verify-core/src/https.rs +++ b/src/ciris-verify-core/src/https.rs @@ -250,7 +250,7 @@ impl HttpsClient { VerifyError::ResponseSchemaMismatch { url: url.clone(), status: status.as_u16(), - detail: e.to_string(), + detail: diagnose_steward_key_drift(&e.to_string()), } })?; @@ -523,6 +523,36 @@ impl HttpsClient { Ok(response.revision) } } +/// Turn serde's `missing field \`classical\`` into something actionable +/// (CIRISVerify#176). +/// +/// That message has sent people looking for a build-record bug for two months +/// — the issue is even titled that way. The record that fails to parse is the +/// **steward-key** response: `validation::query_https_source` → +/// `get_steward_key` → [`StewardKeyResponse`], which requires `classical`. +/// +/// The registry now sends the multi-steward shape (`stewards[]` + +/// `verification_policy`, FSD-002 §10.2). Verify carries a parser for that in +/// `steward_key.rs` — but it **requires** a `response_signature` the registry +/// does not send, so switching to it would not fix this, and relaxing that +/// requirement would mean silently accepting **unsigned trust-root material**. +/// That is a cross-repo contract decision (CIRISRegistry#133), not a +/// verify-side edit, which is why this names the situation rather than +/// papering over it. +fn diagnose_steward_key_drift(detail: &str) -> String { + if detail.contains("missing field `classical`") || detail.contains("missing field `pqc`") { + format!( + "{detail} — this is the known /v1/steward-key contract drift \ + (CIRISVerify#176, CIRISRegistry#133): the registry sends the \ + multi-steward shape (`stewards[]` + `verification_policy`) while \ + this path parses the single-steward shape. NOT a build-record \ + problem and NOT a network problem; HTTPS consensus degrades to \ + DNS-only until the contract is settled" + ) + } else { + detail.to_string() + } +} /// Query the HTTPS endpoint and return data for consensus validation. /// @@ -594,3 +624,35 @@ mod tests { assert_eq!(client.base_url, "https://verify.ciris.ai"); } } + +#[cfg(test)] +mod steward_key_drift { + /// **CIRISVerify#176.** `missing field \`classical\`` sent people looking + /// for a build-record bug for two months — the issue is titled that way. + /// The failing decode is the STEWARD-KEY response. + /// + /// The parse itself cannot be fixed here: the registry sends a shape whose + /// modern parser requires a `response_signature` the registry does not + /// send, and relaxing that would mean accepting unsigned trust-root + /// material (CIRISRegistry#133). So the honest fix is to stop + /// misdirecting whoever reads the log. + #[test] + fn the_known_drift_is_named_rather_than_left_opaque() { + let out = + super::diagnose_steward_key_drift("missing field `classical` at line 1 column 3963"); + assert!(out.contains("/v1/steward-key"), "{out}"); + assert!(out.contains("CIRISRegistry#133"), "{out}"); + assert!( + out.contains("NOT a build-record problem"), + "the misdirection is the thing being fixed: {out}" + ); + } + + /// An unrelated decode failure is passed through untouched — the guard + /// must not relabel every schema error as this one. + #[test] + fn an_unrelated_parse_error_is_left_alone() { + let raw = "invalid type: string, expected u64 at line 2 column 7"; + assert_eq!(super::diagnose_steward_key_drift(raw), raw); + } +} From d97de99f02ecc8e6967b1a250e31f729bce74f39 Mon Sep 17 00:00:00 2001 From: Eric Moore Date: Tue, 1 Sep 2026 16:21:20 -0500 Subject: [PATCH 2/3] fix(#268 review): the VerifiedJson branch was dead code, plus 10 more Review found 11 across both PRs. One of them meant #224's fix did not work. DEAD CODE -- VerifiedJson could never fire on a real manifest. ciris-build-tool hashes the WRAPPER `{"files": {...}}` (register.rs:601); my branch hashed the bare map. So the "try the other construction" fix -- the entire point of #224 -- would have classified every real manifest as Unrecognized while appearing to work. My test passed because it hashed the same wrong shape: a fixture agreeing with the code instead of the producer, which is the exact failure mode that let the v10.4.0 preimage bug ship. Also fixed: - malformed digests are a hard failure again ("sha256:" alone passed the emptiness check and fell through to Unrecognized). - manifest_hash_check propagates to FileCheckSummary and the CLI JSON; without it an Unrecognized hash reads as an ordinary pass. - the v3 decoder enforces kind == User -- a scanned code is untrusted input and a foreign impl can mint a CRC-valid non-user one. - the fedcode payload is bounded by SIZE (1024 bytes), not only node count; 16 x 255-byte ids is >7KB encoded, which QrCode::new refuses. - owned_nodes appear in `fedcode show` JSON and human output. - FSD-003 documents the v3 wire format normatively. - the keyring RNG latch is INITIALIZED, not just read. Only the FFI called run_startup_health_check, and an uninitialized latch reads healthy -- so a direct keyring consumer could mint having never run it. - the latch test serializes; ciris-crypto's thread-local override is cfg(test) and inactive across the crate boundary. - the steward-key diagnosis requires EVIDENCE of the multi-steward shape. - mobile gets the same diagnosis instead of a generic HttpsError. 1400 workspace green, clippy clean, doc clean, guards + self-test pass. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FZPhbXWTzykcbotEX9BN2p --- FSD/FSD-003_FEDERATION_IDENTITY_CODES.md | 59 +++++++++++++++++++ src/ciris-keyring/src/lib.rs | 46 ++++++++++++++- src/ciris-keyring/src/sealed_ed25519.rs | 1 + src/ciris-keyring/src/sealed_mldsa65.rs | 1 + src/ciris-keyring/src/software.rs | 1 + src/ciris-keyring/src/transport_identity.rs | 1 + src/ciris-keyring/src/usb_wrapped_mldsa65.rs | 13 ++-- src/ciris-verify-core/src/bin/ciris_verify.rs | 22 +++++++ src/ciris-verify-core/src/fedcode.rs | 35 ++++++++++- src/ciris-verify-core/src/https.rs | 49 ++++++++++++--- src/ciris-verify-core/src/mobile_http.rs | 17 +++++- .../src/security/file_integrity.rs | 35 ++++++++--- src/ciris-verify-core/src/unified.rs | 11 ++++ 13 files changed, 266 insertions(+), 25 deletions(-) diff --git a/FSD/FSD-003_FEDERATION_IDENTITY_CODES.md b/FSD/FSD-003_FEDERATION_IDENTITY_CODES.md index f20a6eed..3f9b3556 100644 --- a/FSD/FSD-003_FEDERATION_IDENTITY_CODES.md +++ b/FSD/FSD-003_FEDERATION_IDENTITY_CODES.md @@ -98,6 +98,65 @@ hint(alias_hint) # 0x00 absent, else LP (display name only — NOT signed hint(group_key_id) # 0x00 absent, else LP (family/community only) ``` +## 3A. `fedcode` wire format (v3 — embedded owned nodes) + +**Status:** normative, CIRISVerify#269. A strict **superset of v2**: everything +above, unchanged, plus a trailing node list under a bumped `CIRIS-V3-` prefix. + +**Note the version number.** The proposal was titled "fedcode v2"; the wire +format has been at v2 since the kind-tagged code shipped, so the node-carrying +format is **v3**. Minting it as v2 would have collided with a live encoding. + +### 3A.1 Why it exists + +CC 5.4.6 names the population v2 cannot serve: *"phone-class peers that cannot +hold the full directory."* First contact, a QR across a table, an air-gapped +hand-off, a fresh install — none can derive anything from a `key_id` alone. A +v3 code carries what the directory would have supplied. + +### 3A.2 Binary payload + +```text +version(1) = 0x03 +... all v2 fields, byte-identical, through hint(group_key_id) ... +node_count(1) # 1..=16 +repeated node_count times: + LP(node_key_id) # 1-byte length prefix + UTF-8 bytes, 1..=255 + transport_ed25519(32) # raw — the NODE's TRANSPORT key (see 3A.3) +``` + +### 3A.3 Constraints (all normative; a conforming impl MUST enforce each) + +1. **The embedded key is the node's TRANSPORT Ed25519 — never the owner's + federation key, never the node's federation key.** Deriving a destination + from a federation key yields `sha256(fed)[..16]`, an explicit-hash + destination that categorically **cannot be announced**, so no peer can + self-learn a route to it. CIRISServer#335 is the production record of that + mistake: nodes primed the canonical at `1fc232535a…` while it served on + `81cabcf78a…`, every node reported `knows_peer=true`, and zero traces + arrived — after which the false rooting *prevented* recovery. + **Enforced at BOTH encoder and decoder**, because a code minted by another + implementation is exactly the case an encoder cannot police. A code whose + embedded transport key equals the owner's pubkey MUST be rejected. +2. **Only `kind = user` may embed nodes.** "The owner's nodes" is meaningless + for a node, and a group's destinations are group-scoped material a code MUST + NOT carry at all (CC 5.4.6, ruled in CIRISConstitution#91). Enforced at + encoder and decoder. +3. **`node_count` ≤ 16**, and the **total payload ≤ 1024 bytes** before base32. + The count alone does not bound the code — 16 × 255-byte ids exceeds what a + QR can render, defeating the hand-off the format exists for. +4. **Empty is valid and is the default.** A code with no nodes MUST encode as + **v2, byte-identically**, so nothing already issued moves. Only a non-empty + list emits `CIRIS-V3-`. +5. **Scope:** v3 carries lightnet facts only — federation-scope identity that + already announces publicly and carries no anonymity claim. + +### 3A.4 Compatibility + +v1 and v2 codes decode unchanged. A v3 decoder accepts all three prefixes; a +v2-only decoder rejects `CIRIS-V3-` outright rather than mis-parsing it, since +the prefix differs before any payload byte is read. + Then **CRC-16-CCITT** (poly `0x1021`, init `0xFFFF`) over the payload, appended as 2 bytes **big-endian**. Then **RFC-4648 base32, no padding** (alphabet `A–Z2–7`). Display form: prefix `CIRIS-V2-` + the base32 grouped into **4-char** diff --git a/src/ciris-keyring/src/lib.rs b/src/ciris-keyring/src/lib.rs index e7f3cc95..827b8872 100644 --- a/src/ciris-keyring/src/lib.rs +++ b/src/ciris-keyring/src/lib.rs @@ -146,6 +146,36 @@ pub mod keyring_storage; pub use error::KeyringError; +/// Run the SP 800-90B startup health check if nothing has yet +/// (CIRISVerify#207 item 6). +/// +/// `ciris_crypto::random::fill` only READS the latch, and an uninitialized +/// latch reads as healthy. The only production caller of +/// `run_startup_health_check` is `ciris-verify-ffi` — so a direct +/// `ciris-keyring` consumer (the documented standalone API, or a downstream +/// service that links the keyring without the FFI) could reach a mint having +/// never run the check, and the routing added for #207 would buy nothing. +/// +/// `run_startup_health_check` latches through a `OnceLock`, so calling it here +/// is idempotent and costs one atomic load after the first mint. +/// +/// # Errors +/// [`KeyringError::KeyGenerationFailed`] if the startup test fails — refusing +/// to mint is the fail-secure answer, and the whole point of #74. +pub(crate) fn ensure_rng_health_checked() -> Result<(), KeyringError> { + match ciris_crypto::rng_health::run_startup_health_check() { + ciris_crypto::rng_health::RngHealth::Healthy => Ok(()), + ciris_crypto::rng_health::RngHealth::Failed { test, detail } => { + Err(KeyringError::KeyGenerationFailed { + reason: format!( + "SP 800-90B startup health check FAILED ({test}: {detail}); \ + refusing to mint key material" + ), + }) + }, + } +} + /// Mint a P-256 signing key from **latch-checked** randomness /// (CIRISVerify#207 item 6 / #74). /// @@ -165,6 +195,7 @@ pub use error::KeyringError; /// Refusing is the fail-secure answer in both cases: a retry loop around a /// possibly-broken RNG is not an improvement. pub fn mint_p256_signing_key() -> Result { + ensure_rng_health_checked()?; let mut bytes = [0u8; 32]; ciris_crypto::random::fill(&mut bytes).map_err(|e| KeyringError::KeyGenerationFailed { reason: format!("RNG health check failed; refusing to mint a P-256 key: {e}"), @@ -276,8 +307,19 @@ mod rng_latch { fn minting_refuses_on_a_tripped_rng_latch() { use ciris_crypto::rng_health::{__force_health_for_test, RngHealth}; - // Restore on the way out even if the assert panics, so a failure here - // cannot leave the latch tripped for another test on this thread. + // `ciris-crypto`'s thread-local override is `#[cfg(test)]`, which is + // NOT active when it is compiled as this crate's dependency — so + // `__force_health_for_test` writes the PROCESS-GLOBAL latch here, and + // any parallel keyring test that mints a key would fail spuriously + // (CIRISVerify#268 review). A `Restore` guard bounds the duration but + // does not remove the race. + // + // So this test serializes against every other test that could mint, + // via a module-local mutex. The lock is the mechanism; the guard below + // still restores the latch if the assert panics. + static LATCH: std::sync::Mutex<()> = std::sync::Mutex::new(()); + let _serialize = LATCH.lock().unwrap_or_else(|e| e.into_inner()); + struct Restore; impl Drop for Restore { fn drop(&mut self) { diff --git a/src/ciris-keyring/src/sealed_ed25519.rs b/src/ciris-keyring/src/sealed_ed25519.rs index e3e39eee..f464ea52 100644 --- a/src/ciris-keyring/src/sealed_ed25519.rs +++ b/src/ciris-keyring/src/sealed_ed25519.rs @@ -88,6 +88,7 @@ impl SealedEd25519Signer { // latch. A raw `OsRng` draw here bypassed it, so "no weak key is ever // produced" held for ciris-crypto keys and NOT for the keyring-sealed // federation keys — the ones that actually matter. + crate::ensure_rng_health_checked()?; ciris_crypto::random::fill(&mut s).map_err(|e| { KeyringError::KeyGenerationFailed { reason: format!( diff --git a/src/ciris-keyring/src/sealed_mldsa65.rs b/src/ciris-keyring/src/sealed_mldsa65.rs index f9e187bc..11e9b97e 100644 --- a/src/ciris-keyring/src/sealed_mldsa65.rs +++ b/src/ciris-keyring/src/sealed_mldsa65.rs @@ -125,6 +125,7 @@ impl SealedMlDsa65Signer { // #207 item 6 / #74: key material goes through the SP 800-90B health // latch. A raw `OsRng` draw bypassed it, so "no weak key is ever // produced" held for ciris-crypto keys and NOT for keyring-minted ones. + crate::ensure_rng_health_checked()?; ciris_crypto::random::fill(&mut s).map_err(|e| { KeyringError::KeyGenerationFailed { reason: format!("RNG health check failed; refusing to mint: {e}"), diff --git a/src/ciris-keyring/src/software.rs b/src/ciris-keyring/src/software.rs index c6b31778..be0c1bda 100644 --- a/src/ciris-keyring/src/software.rs +++ b/src/ciris-keyring/src/software.rs @@ -1523,6 +1523,7 @@ impl MutableEd25519Signer { // #207 item 6 / #74: key material goes through the SP 800-90B health // latch. A raw `OsRng` draw bypassed it, so "no weak key is ever // produced" held for ciris-crypto keys and NOT for keyring-minted ones. + crate::ensure_rng_health_checked()?; ciris_crypto::random::fill(&mut key_bytes).map_err(|e| { KeyringError::KeyGenerationFailed { reason: format!("RNG health check failed; refusing to mint: {e}"), diff --git a/src/ciris-keyring/src/transport_identity.rs b/src/ciris-keyring/src/transport_identity.rs index d7864424..02fef138 100644 --- a/src/ciris-keyring/src/transport_identity.rs +++ b/src/ciris-keyring/src/transport_identity.rs @@ -157,6 +157,7 @@ impl TransportIdentityKeystore for BlobTransportKeystore { // #207 item 6 / #74: key material goes through the SP 800-90B health // latch. A raw `OsRng` draw bypassed it, so "no weak key is ever // produced" held for ciris-crypto keys and NOT for keyring-minted ones. + crate::ensure_rng_health_checked()?; ciris_crypto::random::fill(&mut bytes).map_err(|e| KeyringError::KeyGenerationFailed { reason: format!("RNG health check failed; refusing to mint: {e}"), })?; diff --git a/src/ciris-keyring/src/usb_wrapped_mldsa65.rs b/src/ciris-keyring/src/usb_wrapped_mldsa65.rs index fbd47f4a..c326dfaf 100644 --- a/src/ciris-keyring/src/usb_wrapped_mldsa65.rs +++ b/src/ciris-keyring/src/usb_wrapped_mldsa65.rs @@ -158,11 +158,14 @@ impl UsbWrappedMlDsa65Signer { Some(s) => seed.copy_from_slice(s), // #207 item 6 / #74: a PQC seed is key material, so it goes // through the SP 800-90B health latch rather than raw OsRng. - None => ciris_crypto::random::fill(&mut seed).map_err(|e| { - KeyringError::KeyGenerationFailed { - reason: format!("RNG health check failed; refusing to mint a seed: {e}"), - } - })?, + None => { + crate::ensure_rng_health_checked()?; + ciris_crypto::random::fill(&mut seed).map_err(|e| { + KeyringError::KeyGenerationFailed { + reason: format!("RNG health check failed; refusing to mint a seed: {e}"), + } + })?; + }, } // The wrap key is derived from a deterministic Ed25519 signature. Prove diff --git a/src/ciris-verify-core/src/bin/ciris_verify.rs b/src/ciris-verify-core/src/bin/ciris_verify.rs index c3205c82..7a90f79d 100644 --- a/src/ciris-verify-core/src/bin/ciris_verify.rs +++ b/src/ciris-verify-core/src/bin/ciris_verify.rs @@ -2618,6 +2618,14 @@ fn emit_fedcode( "transport_hint": fc.transport_hint, "alias_hint": fc.alias_hint, "group_key_id": fc.group_key_id, + // #269: the embedded nodes ARE the payload of a v3 code — a + // consumer that cannot read them gets the opaque code echoed + // back and none of the directory-free resolution the format + // exists to provide. + "owned_nodes": fc.owned_nodes.iter().map(|n| serde_json::json!({ + "key_id": n.key_id, + "transport_pubkey_ed25519_base64": n.transport_pubkey_ed25519_base64, + })).collect::>(), "code": code, }) ); @@ -2627,6 +2635,20 @@ fn emit_fedcode( if let Some(g) = &fc.group_key_id { println!(" group : {g}"); } + if !fc.owned_nodes.is_empty() { + println!( + " nodes : {} embedded (directory-free resolution)", + fc.owned_nodes.len() + ); + for n in &fc.owned_nodes { + println!( + " - {} (transport {}…)", + n.key_id, + &n.transport_pubkey_ed25519_base64 + [..n.transport_pubkey_ed25519_base64.len().min(12)] + ); + } + } println!(" code : {code}\n"); if !no_qr { if let Some(q) = render_qr_terminal(code) { diff --git a/src/ciris-verify-core/src/fedcode.rs b/src/ciris-verify-core/src/fedcode.rs index f576a85b..06819433 100644 --- a/src/ciris-verify-core/src/fedcode.rs +++ b/src/ciris-verify-core/src/fedcode.rs @@ -63,9 +63,20 @@ const MAX_FIELD_BYTES: usize = 255; /// Cap on embedded nodes in a v3 code (CIRISVerify#269). /// /// A fedcode is meant to be scannable as a QR and readable aloud; an unbounded -/// list makes it neither. 16 is well past any realistic owner's node count and -/// keeps the code inside a comfortable QR density. +/// list makes it neither. 16 is well past any realistic owner's node count. const MAX_OWNED_NODES: usize = 16; + +/// Cap on the encoded payload, in bytes, before base32 (CIRISVerify#269). +/// +/// The node COUNT alone does not bound the code: 16 nodes each carrying a +/// 255-byte key_id plus a 32-byte key is >4.6 KB of payload and >7 KB encoded, +/// which `QrCode::new` refuses — so `encode` would happily mint a code the +/// advertised QR hand-off cannot render. Bounding the count without bounding +/// the size is bounding the wrong thing. +/// +/// 1024 bytes leaves ample headroom for realistic ids at a QR density that +/// still scans from a phone across a table. +const MAX_PAYLOAD_BYTES: usize = 1024; const PUBKEY_RAW_LEN: usize = 32; const KEY_ID_HASH_LEN: usize = 32; const CRC_POLY: u16 = 0x1021; @@ -402,6 +413,13 @@ fn build_payload(fc: &FedCode) -> Result, FedCodeError> { out.extend_from_slice(&tp); } } + if out.len() > MAX_PAYLOAD_BYTES { + return Err(FedCodeError::Malformed(format!( + "encoded payload is {} bytes, over the {MAX_PAYLOAD_BYTES}-byte budget — \ + the code would not render as a scannable QR", + out.len() + ))); + } Ok(out) } @@ -475,6 +493,19 @@ pub fn decode(code: &str) -> Result { // v3 tail: the owner's nodes (CIRISVerify#269). Absent on v1/v2. let owned_nodes = if version == FEDCODE_VERSION_V3 { + // A scanned code is untrusted input, and a foreign implementation can + // mint a CRC-valid v3 payload with any kind byte. The encoder forbids + // non-user codes carrying nodes; the DECODER has to enforce the same + // invariant, or a consumer receives attacker-supplied "owned nodes" + // attributed to a group. Same reasoning as the transport-key refusal + // below: the encoder cannot police codes it did not mint. + if kind != FedKind::User { + return Err(FedCodeError::Malformed(format!( + "v3 code carries owned nodes but kind is `{}`; only a `user` \ + code may embed nodes", + kind.as_str() + ))); + } let count = usize::from(*payload.get(offset).ok_or_else(trunc)?); offset += 1; if count > MAX_OWNED_NODES { diff --git a/src/ciris-verify-core/src/https.rs b/src/ciris-verify-core/src/https.rs index 33b1768f..3180a629 100644 --- a/src/ciris-verify-core/src/https.rs +++ b/src/ciris-verify-core/src/https.rs @@ -239,7 +239,17 @@ impl HttpsClient { }); } - let body = response.json::().await.map_err(|e| { + // Read the body as TEXT first, so a decode failure can be diagnosed + // against what was actually received rather than against the serde + // message alone (#268 review: naming the known drift without evidence + // of the multi-steward shape would misdirect on an unrelated fault). + let text = response.text().await.map_err(|e| { + warn!(url = %url, error = %e, "HTTPS: could not read response body"); + VerifyError::HttpsError { + message: format!("Read body from {url} failed: {e}"), + } + })?; + let body = serde_json::from_str::(&text).map_err(|e| { // The server ANSWERED. Do not report this as unreachable — see // VerifyError::ResponseSchemaMismatch. warn!( @@ -250,7 +260,7 @@ impl HttpsClient { VerifyError::ResponseSchemaMismatch { url: url.clone(), status: status.as_u16(), - detail: diagnose_steward_key_drift(&e.to_string()), + detail: diagnose_steward_key_drift(&e.to_string(), &text), } })?; @@ -539,8 +549,18 @@ impl HttpsClient { /// That is a cross-repo contract decision (CIRISRegistry#133), not a /// verify-side edit, which is why this names the situation rather than /// papering over it. -fn diagnose_steward_key_drift(detail: &str) -> String { - if detail.contains("missing field `classical`") || detail.contains("missing field `pqc`") { +pub(crate) fn diagnose_steward_key_drift(detail: &str, body: &str) -> String { + // Require EVIDENCE of the multi-steward shape, not just a missing field. + // A malformed single-steward response that happens to omit `pqc` would + // otherwise be labelled as this specific drift and point an operator at + // CIRISRegistry#133 and a DNS-only degradation — the wrong incident. The + // whole point of this function is to stop misdirecting people, so it must + // not introduce a new misdirection of its own. + let looks_multi_steward = + body.contains("\"stewards\"") || body.contains("\"verification_policy\""); + if looks_multi_steward + && (detail.contains("missing field `classical`") || detail.contains("missing field `pqc`")) + { format!( "{detail} — this is the known /v1/steward-key contract drift \ (CIRISVerify#176, CIRISRegistry#133): the registry sends the \ @@ -638,8 +658,10 @@ mod steward_key_drift { /// misdirecting whoever reads the log. #[test] fn the_known_drift_is_named_rather_than_left_opaque() { - let out = - super::diagnose_steward_key_drift("missing field `classical` at line 1 column 3963"); + let out = super::diagnose_steward_key_drift( + "missing field `classical` at line 1 column 3963", + r#"{"stewards":[],"verification_policy":{}}"#, + ); assert!(out.contains("/v1/steward-key"), "{out}"); assert!(out.contains("CIRISRegistry#133"), "{out}"); assert!( @@ -648,11 +670,24 @@ mod steward_key_drift { ); } + /// A missing field WITHOUT evidence of the multi-steward shape is passed + /// through untouched. A malformed single-steward response that omits `pqc` + /// must not be blamed on CIRISRegistry#133 — that would send an operator + /// to the wrong incident, which is the failure this whole function exists + /// to fix. + #[test] + fn a_missing_field_without_the_multi_steward_shape_is_not_the_known_drift() { + let raw = "missing field `pqc` at line 1 column 40"; + let out = + super::diagnose_steward_key_drift(raw, r#"{"classical":{"algorithm":"Ed25519"}}"#); + assert_eq!(out, raw, "no evidence of the drift → no claim about it"); + } + /// An unrelated decode failure is passed through untouched — the guard /// must not relabel every schema error as this one. #[test] fn an_unrelated_parse_error_is_left_alone() { let raw = "invalid type: string, expected u64 at line 2 column 7"; - assert_eq!(super::diagnose_steward_key_drift(raw), raw); + assert_eq!(super::diagnose_steward_key_drift(raw, "{}"), raw); } } diff --git a/src/ciris-verify-core/src/mobile_http.rs b/src/ciris-verify-core/src/mobile_http.rs index 49973839..574e0f0c 100644 --- a/src/ciris-verify-core/src/mobile_http.rs +++ b/src/ciris-verify-core/src/mobile_http.rs @@ -159,8 +159,21 @@ pub fn get_json( }, }; - response.into_json().map_err(|e| VerifyError::HttpsError { - message: format!("JSON parse error: {}", e), + // Read as text, then decode — so a schema drift is reported as + // ResponseSchemaMismatch with the same diagnosis the desktop path gives. + // Previously mobile flattened this into a generic HttpsError("JSON parse + // error: ..."), so an Android/iOS operator hitting the SAME registry + // payload got neither the structured error nor the explanation (#268 + // review). The endpoint is identical; the diagnosis should be too. + let text = response + .into_string() + .map_err(|e| VerifyError::HttpsError { + message: format!("Read body from {url} failed: {e}"), + })?; + serde_json::from_str::(&text).map_err(|e| VerifyError::ResponseSchemaMismatch { + url: url.to_string(), + status: 200, + detail: crate::https::diagnose_steward_key_drift(&e.to_string(), &text), }) } diff --git a/src/ciris-verify-core/src/security/file_integrity.rs b/src/ciris-verify-core/src/security/file_integrity.rs index 5cbd69b5..57b966f2 100644 --- a/src/ciris-verify-core/src/security/file_integrity.rs +++ b/src/ciris-verify-core/src/security/file_integrity.rs @@ -93,7 +93,7 @@ pub enum ManifestHashCheck { /// Deserialization default for [`FileIntegrityResult::manifest_hash_check`] /// on payloads produced before CIRISVerify#224 — those carry no such field, /// and "we do not know" is the honest value for them. -fn manifest_hash_check_default() -> ManifestHashCheck { +pub(crate) fn manifest_hash_check_default() -> ManifestHashCheck { ManifestHashCheck::Unrecognized } @@ -243,6 +243,20 @@ fn verify_manifest_integrity(manifest: &FileManifest) -> Option Option for FileCheckSummary { files_failed: r.files_failed, files_missing: r.files_missing, files_unexpected: r.files_unexpected, + manifest_hash_check: r.manifest_hash_check, failure_reason: r.failure_reason, files_found: r.files_found, partial_check: r.partial_check, From cdca819dba760464e3a232f5700461a98d904eaf Mon Sep 17 00:00:00 2001 From: Eric Moore Date: Tue, 1 Sep 2026 16:38:42 -0500 Subject: [PATCH 3/3] fix: an already-failed RNG latch is decisive; drop a mutex that protected nothing CI caught a real tension between two of my own review fixes, via the test I added for one of them. `ensure_rng_health_checked` called `run_startup_health_check`, which is `get_or_init` + `store_state` -- so on an already-`Failed` latch it RE-RAN the test and OVERWROTE the verdict. A mint could then proceed off a fresh pass after the process had already latched a failure, which defeats the latch's whole reason for existing: the verdict is sticky by design. Now an already-failed latch short-circuits before any re-run. My local `cargo test --lib` was green because this only fails under nextest, which CI uses and which gives every test its own PROCESS. That also makes the module-local mutex I added last commit useless -- there is no shared global to serialize -- so it is removed rather than left implying a protection it does not provide. The Restore guard stays, for plain `cargo test`. Verified with `cargo nextest run --workspace` this time, not `--lib`: 1478/1478 passing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FZPhbXWTzykcbotEX9BN2p --- src/ciris-keyring/src/lib.rs | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/ciris-keyring/src/lib.rs b/src/ciris-keyring/src/lib.rs index 827b8872..aa397d62 100644 --- a/src/ciris-keyring/src/lib.rs +++ b/src/ciris-keyring/src/lib.rs @@ -163,6 +163,18 @@ pub use error::KeyringError; /// [`KeyringError::KeyGenerationFailed`] if the startup test fails — refusing /// to mint is the fail-secure answer, and the whole point of #74. pub(crate) fn ensure_rng_health_checked() -> Result<(), KeyringError> { + // An ALREADY-FAILED latch is decisive — do not re-run. + // + // `run_startup_health_check` is `get_or_init` + `store_state`, so calling + // it when the latch is already `Failed` re-runs the test and OVERWRITES + // the verdict. That would let a mint proceed off a fresh pass after the + // process had already latched a failure, which is exactly the latch's + // reason for existing: the verdict is sticky by design. + if ciris_crypto::rng_health::is_rng_failed() { + return Err(KeyringError::KeyGenerationFailed { + reason: "RNG health latch is FAILED; refusing to mint key material".to_string(), + }); + } match ciris_crypto::rng_health::run_startup_health_check() { ciris_crypto::rng_health::RngHealth::Healthy => Ok(()), ciris_crypto::rng_health::RngHealth::Failed { test, detail } => { @@ -309,17 +321,14 @@ mod rng_latch { // `ciris-crypto`'s thread-local override is `#[cfg(test)]`, which is // NOT active when it is compiled as this crate's dependency — so - // `__force_health_for_test` writes the PROCESS-GLOBAL latch here, and - // any parallel keyring test that mints a key would fail spuriously - // (CIRISVerify#268 review). A `Restore` guard bounds the duration but - // does not remove the race. + // `__force_health_for_test` writes the PROCESS-GLOBAL latch. // - // So this test serializes against every other test that could mint, - // via a module-local mutex. The lock is the mechanism; the guard below - // still restores the latch if the assert panics. - static LATCH: std::sync::Mutex<()> = std::sync::Mutex::new(()); - let _serialize = LATCH.lock().unwrap_or_else(|e| e.into_inner()); - + // CI runs `cargo nextest`, which gives every test its own PROCESS, so + // the global is not shared and there is no race to serialize. An + // earlier revision added a module-local mutex for this; it protected + // nothing under nextest and implied a guarantee it did not provide, so + // it is gone. The `Restore` guard stays: under a plain `cargo test` + // this thread must not leave the latch tripped. struct Restore; impl Drop for Restore { fn drop(&mut self) {