fix(keyless): bind proof-cache key to full entry + propagate audit-hash error (v0.9.2)

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

Cargo.toml

@@ -13,7 +13,7 @@ exclude = [
 ]
 
 [workspace.package]
-version = "0.9.1"
+version = "0.9.2"
 edition = "2024"
 authors = ["Frank Denis <github@pureftpd.org>", "Ralf Anton Beier <ralf_beier@me.com>"]
 license = "MIT"

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`:

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"

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"] }

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"] }

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();

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 {