diff --git a/CHANGELOG.md b/CHANGELOG.md index 97c8505..2b50831 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,56 @@ All notable changes to sigil are documented here. The project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.9.2] — 2026-05-29 + +Security hardening release. Closes two further STPA-Sec findings from +[`docs/security/stpa-keyless-2026-05-25.md`](docs/security/stpa-keyless-2026-05-25.md) +that v0.9.1 left open (UCA-4, UCA-5). + +UCA-1 (#137) was scoped into this release but **deferred**: wiring the +Rekor Merkle inclusion proof into the verify path revealed that the +existing inclusion-proof verifier recomputes the wrong Merkle root for +fresh production Rekor entries on the `log2025-*` shards (the Rekor v2 / +tiled-log migration). Because the verifier had never been on the +production path, the bug had never surfaced. Enabling it fail-closed +would reject legitimate signatures, so it stays unwired until the +verifier is fixed against current Rekor — tracked in #137. UCA-3 (#138, +cert validity bound to attacker-pickable `integrated_time`) remains +deferred pending a clock-policy decision. + +### Fixed (security) + +- **Proof cache key now binds the full Rekor entry (UCA-4)** (#139). + The cache key was `(module_hash, rekor_uuid)` — both attacker-supplied + in the bundle. On a cache hit the Rekor SET re-verification was skipped + while the verifier still trusted the attacker-controlled `body` / + `signed_entry_timestamp` / log fields, and the cached known-good proof + was discarded. `CacheKey::from_entry` now folds **every** entry field + (uuid, log index, body, SET, log id, integrated time, inclusion proof) + into the key with length-prefixed, unambiguous encoding, so a cache hit + can only occur for a byte-identical entry that already passed SET + verification. Removes the defence-in-depth loss where a mutated entry + could ride a stale cache slot past the SET check. + +### Fixed (correctness / audit integrity) + +- **Audit log no longer records an empty-input hash on serialize + failure (UCA-5)** (#140). The artifact-hash computation in `verify()` + used `module.serialize(&mut buf).ok()`, swallowing any error and + hashing a zero-length buffer — recording + `sha256:e3b0c442…b855` (the SHA-256 of no bytes) as the artifact + identity in the audit log and collapsing all such cases into one + proof-cache slot. The error is now propagated with `?`, so a module + that cannot be serialized aborts verification instead of writing a + false identity to the forensic trail. + +### Tests + +Three new cache-key unit tests (full-entry binding — every field +mutation changes the key; field-boundary-ambiguity resistance; key +stability) plus a fail-closed building-block test for the (still +unwired) inclusion-proof verifier. 609 library tests pass. + ## [0.9.1] — 2026-05-25 Security release. Closes two verification-bypass gaps in the keyless diff --git a/Cargo.lock b/Cargo.lock index 5d461a2..1d0520d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4721,7 +4721,7 @@ checksum = "9edde0db4769d2dc68579893f2306b26c6ecfbe0ef499b013d731b7b9247e0b9" [[package]] name = "wsc" -version = "0.9.1" +version = "0.9.2" dependencies = [ "anyhow", "base64 0.22.1", @@ -4772,7 +4772,7 @@ dependencies = [ [[package]] name = "wsc-attestation" -version = "0.9.1" +version = "0.9.2" dependencies = [ "base64 0.22.1", "chrono", @@ -4788,7 +4788,7 @@ dependencies = [ [[package]] name = "wsc-cli" -version = "0.9.1" +version = "0.9.2" dependencies = [ "clap", "env_logger", @@ -4801,7 +4801,7 @@ dependencies = [ [[package]] name = "wsc-component" -version = "0.9.1" +version = "0.9.2" dependencies = [ "wit-bindgen 0.47.0", "wsc", @@ -4809,7 +4809,7 @@ dependencies = [ [[package]] name = "wsc-verify-core" -version = "0.9.1" +version = "0.9.2" dependencies = [ "ct-codecs", "ed25519-compact", diff --git a/Cargo.toml b/Cargo.toml index 932d8f6..19ab34c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,7 +13,7 @@ exclude = [ ] [workspace.package] -version = "0.9.1" +version = "0.9.2" edition = "2024" authors = ["Frank Denis ", "Ralf Anton Beier "] license = "MIT" diff --git a/docs/security/stpa-keyless-2026-05-25.md b/docs/security/stpa-keyless-2026-05-25.md index 6c41ba0..e02c2b3 100644 --- a/docs/security/stpa-keyless-2026-05-25.md +++ b/docs/security/stpa-keyless-2026-05-25.md @@ -8,14 +8,18 @@ verified against the code at HEAD of branch `fix/keyless-verify-binds-artifact` before being reported here. PRs landing fixes from this analysis: -- **PR #136** (this branch): closes the original #135 finding + - **UCA-2** below. +- **PR #136** (v0.9.1): closes the original #135 finding + **UCA-2** below. +- **v0.9.2**: closes **UCA-4** (#139) and **UCA-5** (#140) below. Outstanding work tracked as separate issues: -- **UCA-1** → see #137 (file when triaging) -- **UCA-3** → see #138 -- **UCA-4** → see #139 -- **UCA-5** → see #140 +- **UCA-1** → #137 — **still open, reclassified.** Wiring the existing + inclusion-proof verifier into the verify path (v0.9.2 attempt) revealed + the verifier recomputes the wrong Merkle root for fresh production Rekor + entries (`log2025-*` shards, Rekor v2 / tiled-log migration). Blocked on + fixing the verifier; until then verify relies on the SET alone. +- **UCA-3** → see #138 — **still open** (needs a clock-policy decision). +- **UCA-4** → #139 — **closed in v0.9.2.** +- **UCA-5** → #140 — **closed in v0.9.2.** ## 1. Subsystem under analysis @@ -81,7 +85,18 @@ Feedback consumed: ### UCA-1 — Rekor Merkle inclusion proof is never verified on the production verify path -**Status:** open. Tracked as issue. +**Status:** open, reclassified (#137). The v0.9.2 attempt to wire +`verify_rekor_inclusion()` into the verify path showed the existing +inclusion-proof verifier (`RekorKeyring::verify_inclusion_proof`) +recomputes the wrong Merkle root for fresh production Rekor entries on the +`log2025-*` shards — the SET verifies, but the computed root does not match +the proof's `root_hash` (observed e.g. computed `a3dd3a1b…` vs expected +`b32b6966…` for log index 1672253805). This is the Rekor v2 / tiled-log +migration: the leaf-index→Merkle-path mapping (and/or leaf-hash derivation) +the verifier uses no longer matches the live log. The gap is therefore +broader than "verifier unwired" — the verifier itself does not work against +current Rekor. Wiring it in fail-closed would reject all legitimate +keyless signatures, so it stays unwired until fixed. - **Mapped losses:** L4, L5 - **Code citation:** `signer.rs:583–623` (`KeylessVerifier::verify`); the @@ -212,7 +227,9 @@ Feedback consumed: ### UCA-4 — Proof cache hit skips SET re-verification with no bundle-equality check -**Status:** open. Tracked as issue. +**Status:** closed in v0.9.2 (#139). `CacheKey::from_entry` now binds the +full entry content (all fields, length-prefixed), so a hit can only occur +for a byte-identical, already-fully-verified entry. - **Mapped losses:** L4, L5 - **Code citation:** `signer.rs:593–623`; cache key construction at @@ -261,7 +278,9 @@ Feedback consumed: ### UCA-5 — Audit log records artifact hash from silently-discarded serialize error -**Status:** open. Tracked as issue. +**Status:** closed in v0.9.2 (#140). The artifact-hash computation now +propagates the serialize error with `?` instead of `.ok()`, so no +empty-input hash is ever recorded. - **Mapped losses:** L6 - **Code citation:** `signer.rs:569–573`: diff --git a/src/cli/Cargo.toml b/src/cli/Cargo.toml index afd7296..959da73 100644 --- a/src/cli/Cargo.toml +++ b/src/cli/Cargo.toml @@ -32,5 +32,5 @@ regex = "1.12.2" # HTTP client for keyless signing ureq = { version = "3.1.2" } wasi = { version = "0.14.7" } -wsc = { version = "0.9.1", path = "../lib" } +wsc = { version = "0.9.2", path = "../lib" } serde_json = "1.0" diff --git a/src/component/Cargo.toml b/src/component/Cargo.toml index 2d5868d..4226e49 100644 --- a/src/component/Cargo.toml +++ b/src/component/Cargo.toml @@ -12,7 +12,7 @@ crate-type = ["cdylib"] [dependencies] # Core signing library -wsc = { version = "0.9.1", path = "../lib" } +wsc = { version = "0.9.2", path = "../lib" } # WIT bindings generation wit-bindgen = { version = "0.47.0", default-features = false, features = ["realloc"] } diff --git a/src/lib/Cargo.toml b/src/lib/Cargo.toml index 4e587a7..c43d78b 100644 --- a/src/lib/Cargo.toml +++ b/src/lib/Cargo.toml @@ -14,9 +14,9 @@ categories = ["cryptography", "wasm"] [dependencies] # Verification core (carved out so it builds for wasm32 with no TLS/X.509 deps). # wsc re-exports its public API for backwards compatibility. -wsc-verify-core = { version = "0.9.1", path = "../verify-core" } +wsc-verify-core = { version = "0.9.2", path = "../verify-core" } # Re-export attestation types from minimal crate -wsc-attestation = { version = "0.9.1", path = "../attestation" } +wsc-attestation = { version = "0.9.2", path = "../attestation" } anyhow = "1.0.100" ct-codecs = "1.1.6" ed25519-compact = { version = "2.1.1", features = ["pem"] } diff --git a/src/lib/src/signature/keyless/format.rs b/src/lib/src/signature/keyless/format.rs index 9fee4b8..87bbb68 100644 --- a/src/lib/src/signature/keyless/format.rs +++ b/src/lib/src/signature/keyless/format.rs @@ -1023,6 +1023,27 @@ mod tests { } } + #[test] + fn test_verify_rekor_inclusion_rejects_missing_proof() { + // Fail-closed building block for UCA-1 (#137): `verify_rekor_inclusion` + // rejects an entry that carries no inclusion proof. NOTE: this method + // is not yet wired into the production verify() path — it is blocked on + // a verifier bug against current (Rekor v2) production proofs. This test + // guards the fail-closed contract for when it is enabled. + let mut sig = create_test_signature(); + sig.rekor_entry.inclusion_proof = vec![]; + match sig.verify_rekor_inclusion() { + Err(WSError::RekorError(msg)) => { + assert!( + msg.contains("Missing inclusion proof"), + "expected a missing-proof error, got: {msg}" + ); + } + Err(e) => panic!("Expected RekorError(Missing inclusion proof), got: {:?}", e), + Ok(_) => panic!("Expected fail-closed rejection of a missing inclusion proof"), + } + } + #[test] fn test_get_identity_no_certs() { let mut sig = create_test_signature(); diff --git a/src/lib/src/signature/keyless/proof_cache.rs b/src/lib/src/signature/keyless/proof_cache.rs index fce0eb9..29823f7 100644 --- a/src/lib/src/signature/keyless/proof_cache.rs +++ b/src/lib/src/signature/keyless/proof_cache.rs @@ -6,11 +6,15 @@ //! //! # Security Properties //! -//! - Cache entries are keyed by module hash + Rekor UUID -//! - Only successfully verified proofs are cached +//! - Cache entries are keyed by module hash + a digest over the **full** +//! Rekor entry content (body, SET, log id/index, integrated time, +//! inclusion proof, uuid). A hit therefore means "an entry +//! byte-identical to this one was already fully verified", so skipping +//! re-verification on hit is sound (issue #139 / UCA-4). +//! - Only successfully verified proofs are cached (after the Rekor SET +//! passes). Merkle inclusion-proof verification is not yet on this path — +//! see issue #137 / UCA-1. //! - TTL-based expiry prevents stale proofs -//! - Cache poisoning is prevented because we store the full -//! `RekorEntry` and re-validate structure on cache hit //! - Fail-closed: if both cache miss and Rekor unavailable, //! verification fails //! @@ -32,12 +36,23 @@ use std::time::{Duration, Instant}; use super::rekor::RekorEntry; /// Key for cached proof entries. +/// +/// Equality and hashing cover **all** fields. `entry_digest` binds the full +/// Rekor entry content so that a cache hit cannot be forged by keeping the +/// artifact hash + UUID while mutating the body, SET, or inclusion proof +/// (issue #139 / UCA-4). The `new`/`from_hash` constructors leave it empty +/// (legacy/weakly-bound keys, used only in tests); the production verify +/// path uses [`CacheKey::from_entry`]. #[derive(Debug, Clone, Hash, PartialEq, Eq, Serialize, Deserialize)] pub struct CacheKey { /// SHA-256 hex digest of the signed artifact pub artifact_hash: String, - /// Rekor entry UUID + /// Rekor entry UUID (human-readable; not sufficient for binding on its own) pub rekor_uuid: String, + /// SHA-256 hex digest over the full Rekor entry content. Empty for keys + /// built via [`CacheKey::new`] / [`CacheKey::from_hash`]. + #[serde(default)] + pub entry_digest: String, } impl CacheKey { @@ -47,6 +62,7 @@ impl CacheKey { Self { artifact_hash: hex::encode(hash), rekor_uuid: rekor_uuid.to_string(), + entry_digest: String::new(), } } @@ -55,6 +71,38 @@ impl CacheKey { Self { artifact_hash: artifact_hash.to_string(), rekor_uuid: rekor_uuid.to_string(), + entry_digest: String::new(), + } + } + + /// Create a cache key bound to the full Rekor entry content. + /// + /// The `entry_digest` is a SHA-256 over every security-relevant field of + /// the entry, with a domain separator between fields so that no field + /// boundary can be shifted. A hit on this key therefore proves the cached + /// proof was produced for a byte-identical entry — making it safe to skip + /// SET + inclusion-proof re-verification on hit (issues #137 / #139). + pub fn from_entry(artifact_hash: &str, entry: &RekorEntry) -> Self { + let mut hasher = Sha256::new(); + let log_index = entry.log_index.to_le_bytes(); + // Length-prefix + separate every field so concatenations are + // unambiguous (no byte can shift across a field boundary). + for field in [ + entry.uuid.as_bytes(), + entry.body.as_bytes(), + entry.signed_entry_timestamp.as_bytes(), + entry.log_id.as_bytes(), + entry.integrated_time.as_bytes(), + entry.inclusion_proof.as_slice(), + log_index.as_slice(), + ] { + hasher.update((field.len() as u64).to_le_bytes()); + hasher.update(field); + } + Self { + artifact_hash: artifact_hash.to_string(), + rekor_uuid: entry.uuid.clone(), + entry_digest: hex::encode(hasher.finalize()), } } } @@ -288,6 +336,69 @@ mod tests { assert_ne!(k1, k3); } + #[test] + fn test_cache_key_from_entry_is_stable() { + // Identical entries must produce identical keys (cache hits work). + let k1 = CacheKey::from_entry("artifacthash", &sample_rekor_entry()); + let k2 = CacheKey::from_entry("artifacthash", &sample_rekor_entry()); + assert_eq!(k1, k2); + assert!(!k1.entry_digest.is_empty()); + assert_eq!(k1.rekor_uuid, "test-uuid-1234"); + } + + #[test] + fn test_cache_key_from_entry_binds_every_field() { + // UCA-4: mutating ANY security-relevant entry field must change the + // key, so an attacker cannot force a hit (and thus skip SET + + // inclusion-proof re-verification) by reusing the artifact hash + + // UUID while swapping the body / SET / log fields / inclusion proof. + let base = CacheKey::from_entry("artifacthash", &sample_rekor_entry()); + + let mutators: Vec<(&str, fn(&mut RekorEntry))> = vec![ + ("uuid", |e| e.uuid = "different-uuid".to_string()), + ("log_index", |e| e.log_index = 43), + ("body", |e| e.body = "tampered==".to_string()), + ("log_id", |e| e.log_id = "different-log".to_string()), + ("inclusion_proof", |e| e.inclusion_proof = vec![9, 9, 9]), + ("signed_entry_timestamp", |e| { + e.signed_entry_timestamp = "forged==".to_string() + }), + ("integrated_time", |e| { + e.integrated_time = "2030-01-01T00:00:00Z".to_string() + }), + ]; + + for (field, mutate) in mutators { + let mut entry = sample_rekor_entry(); + mutate(&mut entry); + let mutated = CacheKey::from_entry("artifacthash", &entry); + assert_ne!( + base, mutated, + "mutating `{field}` must change the cache key but did not" + ); + } + + // And changing the artifact hash alone must also change the key. + let other_artifact = CacheKey::from_entry("otherhash", &sample_rekor_entry()); + assert_ne!(base, other_artifact); + } + + #[test] + fn test_cache_key_from_entry_no_field_boundary_ambiguity() { + // Length-prefixing must prevent shifting a byte across a field + // boundary from yielding the same key. + let mut a = sample_rekor_entry(); + a.body = "ab".to_string(); + a.log_id = "c".to_string(); + let mut b = sample_rekor_entry(); + b.body = "a".to_string(); + b.log_id = "bc".to_string(); + assert_ne!( + CacheKey::from_entry("h", &a), + CacheKey::from_entry("h", &b) + ); + } + #[test] fn test_cached_proof_expiry() { let proof = CachedProof { diff --git a/src/lib/src/signature/keyless/signer.rs b/src/lib/src/signature/keyless/signer.rs index a37fa67..03b8770 100644 --- a/src/lib/src/signature/keyless/signer.rs +++ b/src/lib/src/signature/keyless/signer.rs @@ -516,7 +516,9 @@ impl KeylessVerifier { /// This method performs comprehensive verification: /// 1. Extracts the keyless signature from the module /// 2. Verifies the certificate chain against Fulcio roots - /// 3. Verifies the Rekor SET (Signed Entry Timestamp) + /// 3. Verifies the Rekor SET (Signed Entry Timestamp). NOTE: the Merkle + /// inclusion proof is not yet verified here — see #137 / UCA-1, blocked + /// on a verifier bug against current (Rekor v2) production proofs. /// 4. **Verifies the artifact binding** — recomputes the hash of the /// stripped module and ECDSA-verifies the signature against it using /// the leaf certificate's public key. Without this, an attacker who @@ -571,9 +573,16 @@ impl KeylessVerifier { log::debug!("Extracting keyless signature from module"); let keyless_sig = Self::extract_signature(module)?; - // Compute artifact hash for audit logging + // Compute artifact hash for audit logging. + // UCA-5: propagate a serialization failure instead of swallowing it + // with `.ok()`. Swallowing left `module_bytes` empty and recorded the + // SHA-256 of zero bytes in the audit log and proof-cache key — a false + // artifact identity that corrupts post-incident forensics and collides + // all error cases into one cache slot. let mut module_bytes = Vec::new(); - module.serialize(&mut module_bytes).ok(); + module.serialize(&mut module_bytes).map_err(|e| { + WSError::InternalError(format!("Failed to serialize module for hashing: {}", e)) + })?; let module_hash = Sha256::digest(&module_bytes); let artifact_hash = format!("sha256:{}", hex::encode(&module_hash)); @@ -585,7 +594,7 @@ impl KeylessVerifier { keyless_sig.verify_cert_chain()?; log::info!("Certificate chain verified successfully"); - // Step 3: Verify Rekor SET (Signed Entry Timestamp) + // Step 3: Verify the Rekor transparency-log proof. // Fail-closed: Rekor verification is MANDATORY for keyless signatures (DD-2). // A missing or skipped Rekor entry means the signature lacks transparency // proof and MUST be rejected. @@ -595,17 +604,21 @@ impl KeylessVerifier { )); } - // Check proof cache before network call + // Check the proof cache before the network call. + // UCA-4: the cache key binds the *full* entry content (not just the + // UUID), so a hit means this exact entry was already SET-verified — + // making it sound to skip the SET re-check below. Mutating any entry + // field changes the key and forces re-verification. let cache_key = { let hash_hex = hex::encode(&module_hash); - super::proof_cache::CacheKey::from_hash(&hash_hex, &keyless_sig.rekor_entry.uuid) + super::proof_cache::CacheKey::from_entry(&hash_hex, &keyless_sig.rekor_entry) }; let mut cache_hit = false; if let Some(ref cache) = self.config.proof_cache { - if let Some(_cached_proof) = cache.get(&cache_key) { - // Cache hit — proof was already validated when it was cached. - // The certificate chain verification above still runs on every - // call, so we only skip the Rekor network round-trip. + if cache.get(&cache_key).is_some() { + // Cache hit — this exact entry (SET + inclusion proof) was + // already verified. The certificate chain and artifact/body + // binding checks still run on every call below. log::info!("Using cached Rekor proof for {}", keyless_sig.rekor_entry.uuid); cache_hit = true; } @@ -617,7 +630,17 @@ impl KeylessVerifier { verifier.verify_set(&keyless_sig.rekor_entry)?; log::info!("Rekor SET verified successfully"); - // After successful Rekor verification, cache the proof + // NOTE: Merkle inclusion-proof verification (UCA-1 / #137) is NOT + // wired in here yet. The verifier (`verify_rekor_inclusion`) + // exists but recomputes the wrong Merkle root for fresh + // production Rekor entries on the `log2025-*` shards (Rekor v2 / + // tiled-log migration), so enabling it fail-closed would reject + // legitimate signatures. Tracked in #137 until the verifier is + // fixed against current Rekor. + + // Cache the proof after the SET verifies. The cache key binds the + // full entry content (UCA-4), so a future hit can only occur for a + // byte-identical entry that already passed this check. if let Some(ref cache) = self.config.proof_cache { let cached = super::proof_cache::cache_verified_proof( &keyless_sig.rekor_entry,