mirror of
https://github.com/ruvnet/RuView
synced 2026-08-01 19:01:42 +00:00
harden(cog-ha-matter crypto): domain-separate witness signing + verify_strict (signing chain otherwise sound — P2 crypto core verified) (#1080)
* fix(cog-ha-matter): domain-separate witness signing chain + verify_strict (ADR-116 §2.2) Crypto review of the SHA-256 + Ed25519 witness chain that ADR-262 P2 reuses. The sibling wifi-densepose-engine bug class (unframed concatenation of operator-influenceable strings into a signed digest) is ABSENT here — canonical_bytes already length-prefixes kind/payload. Two real hardening gaps fixed: - CHM-WIT-01: add a versioned domain-separation tag (WITNESS_DOMAIN_TAG = b"cog-ha-matter/witness-event/v1\0") to canonical_bytes so the witness SHA-256 preimage / Ed25519 message cannot be replayed as a message for another signing context that shares key infrastructure (notably the manifest binary_signature). Completes the engine review's "domain-tag + length-prefix" rule. Witness bytes change by design (prior on-disk hashes/sigs invalidated); no in-repo crate consumes these bytes programmatically. - CHM-WIT-02: verify_signature uses VerifyingKey::verify_strict (rejects non-canonical encodings + small-order keys) for the audit-uniqueness property. Key stays caller-pinned (not read from the event). Pinned by fails-on-old tests: canonical_bytes_is_domain_separated, canonical_bytes_starts_with_domain_tag_then_prev_hash, witness_preimage_cannot_collide_with_a_bare_manifest_digest, signature_commits_to_domain_tag_not_bare_fields; key-pinning guarded by verify_uses_strict_path_and_pins_caller_key. cog-ha-matter 64 -> 68 tests, 0 failed. Co-Authored-By: claude-flow <ruv@ruv.net> * docs(cog-ha-matter): record ADR-116 crypto review findings (CHM-WIT-01/02) CHANGELOG [Unreleased] Security entry + ADR-116 §4.1 review notes: engine-class signed-digest collision confirmed ABSENT (length-prefixing already correct), domain-separation tag added, verify_strict hardening, and the clean dimensions (verify-before-trust, key-handling, determinism, fail-closed parsing) with byte-layout evidence. Co-Authored-By: claude-flow <ruv@ruv.net>
This commit is contained in:
@@ -102,19 +102,43 @@ pub struct WitnessEvent {
|
||||
pub this_hash: WitnessHash,
|
||||
}
|
||||
|
||||
/// Domain-separation tag prefixing every witness canonical message.
|
||||
///
|
||||
/// This is the *domain tag* half of the "domain-tag + length-prefix"
|
||||
/// rule for any hashed/signed message whose fields are
|
||||
/// operator-influenceable. The witness chain already length-prefixes
|
||||
/// `kind` and `payload` (preventing intra-protocol concatenation
|
||||
/// forgery); the tag adds cross-protocol separation so a SHA-256
|
||||
/// preimage / Ed25519 message produced here can never be re-interpreted
|
||||
/// as a message from another signing context that shares key
|
||||
/// infrastructure — notably ADR-116's *manifest* `binary_signature`
|
||||
/// (Ed25519 over `binary_sha256`), which ADR-262 P2 reuses this exact
|
||||
/// chain for. A signature is only ever valid for the one domain whose
|
||||
/// tag it commits to.
|
||||
///
|
||||
/// The trailing NUL terminates the version string so a future
|
||||
/// migration (Blake3, extra fields, Merkle tier) bumps the tag instead
|
||||
/// of silently colliding with v1 bundles.
|
||||
pub const WITNESS_DOMAIN_TAG: &[u8] = b"cog-ha-matter/witness-event/v1\x00";
|
||||
|
||||
/// Compute the canonical-bytes form an event is hashed over.
|
||||
///
|
||||
/// The format is intentionally simple and length-prefixed so a
|
||||
/// future migration can be staged with a `version` byte in front
|
||||
/// without ambiguity:
|
||||
/// The format is domain-tagged and length-prefixed:
|
||||
///
|
||||
/// ```text
|
||||
/// prev_hash[32] | seq:u64-be | ts:u64-be | kind_len:u32-be | kind | payload_len:u32-be | payload
|
||||
/// DOMAIN_TAG | prev_hash[32] | seq:u64-be | ts:u64-be
|
||||
/// | kind_len:u32-be | kind | payload_len:u32-be | payload
|
||||
/// ```
|
||||
///
|
||||
/// Length-prefixing prevents the classic "concatenation forgery"
|
||||
/// attack where `"abc" + "def"` and `"ab" + "cdef"` would hash the
|
||||
/// same.
|
||||
/// * The leading [`WITNESS_DOMAIN_TAG`] gives cross-protocol
|
||||
/// separation: bytes signed/hashed here cannot be replayed as a
|
||||
/// message for another Ed25519 context in the same trust chain
|
||||
/// (e.g. the manifest `binary_signature`). It also carries a format
|
||||
/// version for staged migrations.
|
||||
/// * Length-prefixing `kind` and `payload` prevents the classic
|
||||
/// "concatenation forgery" where `"abc" + "def"` and `"ab" + "cdef"`
|
||||
/// would hash the same. The fixed-width `prev_hash`/`seq`/`ts`
|
||||
/// fields are self-delimiting.
|
||||
pub fn canonical_bytes(
|
||||
prev_hash: WitnessHash,
|
||||
seq: u64,
|
||||
@@ -123,7 +147,10 @@ pub fn canonical_bytes(
|
||||
payload: &[u8],
|
||||
) -> Vec<u8> {
|
||||
let kind_bytes = kind.as_bytes();
|
||||
let mut out = Vec::with_capacity(32 + 8 + 8 + 4 + kind_bytes.len() + 4 + payload.len());
|
||||
let mut out = Vec::with_capacity(
|
||||
WITNESS_DOMAIN_TAG.len() + 32 + 8 + 8 + 4 + kind_bytes.len() + 4 + payload.len(),
|
||||
);
|
||||
out.extend_from_slice(WITNESS_DOMAIN_TAG);
|
||||
out.extend_from_slice(&prev_hash.0);
|
||||
out.extend_from_slice(&seq.to_be_bytes());
|
||||
out.extend_from_slice(×tamp_unix_s.to_be_bytes());
|
||||
@@ -466,11 +493,51 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn canonical_bytes_starts_with_prev_hash() {
|
||||
fn canonical_bytes_starts_with_domain_tag_then_prev_hash() {
|
||||
// Locks the on-wire format. A future migration that flips
|
||||
// field order must bump a version byte and update this test.
|
||||
// field order must bump the domain tag and update this test.
|
||||
let bytes = canonical_bytes(WitnessHash([7u8; 32]), 1, 2, "k", b"p");
|
||||
assert_eq!(&bytes[..32], &[7u8; 32]);
|
||||
let tag = WITNESS_DOMAIN_TAG.len();
|
||||
assert_eq!(&bytes[..tag], WITNESS_DOMAIN_TAG);
|
||||
assert_eq!(&bytes[tag..tag + 32], &[7u8; 32]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn canonical_bytes_is_domain_separated() {
|
||||
// Cross-protocol separation: the witness preimage must begin
|
||||
// with the domain tag so its SHA-256 / Ed25519 message can
|
||||
// never be reinterpreted as a message from another signing
|
||||
// context that shares key infrastructure (e.g. the manifest
|
||||
// `binary_signature` over `binary_sha256`). Fails on the old
|
||||
// un-tagged encoding, which began directly with `prev_hash`.
|
||||
let bytes = canonical_bytes(WitnessHash::GENESIS, 0, 0, "k", b"p");
|
||||
assert!(
|
||||
bytes.starts_with(WITNESS_DOMAIN_TAG),
|
||||
"canonical message is not domain-separated"
|
||||
);
|
||||
// The tag is versioned and NUL-terminated.
|
||||
assert!(WITNESS_DOMAIN_TAG.ends_with(b"\x00"));
|
||||
assert!(WITNESS_DOMAIN_TAG.windows(2).any(|w| w == b"v1"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn witness_preimage_cannot_collide_with_a_bare_manifest_digest() {
|
||||
// The manifest `binary_signature` signs a bare 64-byte
|
||||
// SHA-256 hex string. A witness preimage must never *equal*
|
||||
// such a string, even if an operator crafted kind/payload to
|
||||
// try — the domain tag (33 bytes) + fixed 48-byte prefix make
|
||||
// the witness message structurally longer and tag-distinct.
|
||||
// Fails on the old encoding only if it could ever produce a
|
||||
// 64-byte all-hex message; the tag makes the impossibility
|
||||
// explicit and regression-guarded.
|
||||
let manifest_digest_msg = "a".repeat(64); // 64 ASCII hex bytes
|
||||
let witness = canonical_bytes(WitnessHash::GENESIS, 0, 0, "", b"");
|
||||
assert_ne!(witness.as_slice(), manifest_digest_msg.as_bytes());
|
||||
assert!(
|
||||
witness.len() > manifest_digest_msg.len(),
|
||||
"domain tag must make witness preimage structurally distinct"
|
||||
);
|
||||
assert!(!witness.starts_with(b"aaaa"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -36,7 +36,7 @@
|
||||
//! key store (separate concern). Tests use a fixed-bytes seed for
|
||||
//! determinism — never check in real Seed keys here.
|
||||
|
||||
use ed25519_dalek::{Signature, Signer, SigningKey, Verifier, VerifyingKey};
|
||||
use ed25519_dalek::{Signature, Signer, SigningKey, VerifyingKey};
|
||||
|
||||
use crate::witness::{canonical_bytes, WitnessEvent};
|
||||
|
||||
@@ -58,6 +58,16 @@ pub fn sign_event(event: &WitnessEvent, key: &SigningKey) -> Signature {
|
||||
/// Verify an Ed25519 signature against a witness event using the
|
||||
/// Seed's public key. `Ok(())` iff the signature is valid for the
|
||||
/// event's canonical bytes under this key.
|
||||
///
|
||||
/// Uses `verify_strict` (not the permissive `Verifier::verify`) on
|
||||
/// purpose: for a tamper-evident *audit* chain the signature is the
|
||||
/// attestation, so non-canonical encodings and small-order public
|
||||
/// keys must be rejected. `verify_strict` enforces RFC 8032's
|
||||
/// stricter checks, giving the "one canonical signature per event"
|
||||
/// property an auditor relies on when comparing or deduplicating
|
||||
/// signed witness records. The public key is caller-pinned (the
|
||||
/// Seed's known verifying key) — never parsed from the event — so a
|
||||
/// forged event carrying its own key cannot self-verify.
|
||||
pub fn verify_signature(
|
||||
event: &WitnessEvent,
|
||||
signature: &Signature,
|
||||
@@ -71,7 +81,7 @@ pub fn verify_signature(
|
||||
&event.payload,
|
||||
);
|
||||
public_key
|
||||
.verify(&bytes, signature)
|
||||
.verify_strict(&bytes, signature)
|
||||
.map_err(|_| SignatureVerifyError::Invalid)
|
||||
}
|
||||
|
||||
@@ -140,6 +150,58 @@ mod tests {
|
||||
verify_signature(&event, &sig, &public).expect("clean signature verifies");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn signature_commits_to_domain_tag_not_bare_fields() {
|
||||
// The signature is over the domain-tagged canonical bytes. A
|
||||
// signature produced over the *un-tagged* concatenation of the
|
||||
// same fields must NOT verify — proving cross-protocol
|
||||
// separation reaches the signature layer, not just the hash.
|
||||
// Fails on the old encoding where the signed message began
|
||||
// directly with `prev_hash` (no tag).
|
||||
use ed25519_dalek::Signer;
|
||||
let key = fixed_key();
|
||||
let public = key.verifying_key();
|
||||
let event = fresh_event();
|
||||
|
||||
// Hand-build the OLD (un-tagged) preimage and sign it.
|
||||
let mut untagged = Vec::new();
|
||||
untagged.extend_from_slice(&event.prev_hash.0);
|
||||
untagged.extend_from_slice(&event.seq.to_be_bytes());
|
||||
untagged.extend_from_slice(&event.timestamp_unix_s.to_be_bytes());
|
||||
untagged.extend_from_slice(&(event.kind.len() as u32).to_be_bytes());
|
||||
untagged.extend_from_slice(event.kind.as_bytes());
|
||||
untagged.extend_from_slice(&(event.payload.len() as u32).to_be_bytes());
|
||||
untagged.extend_from_slice(&event.payload);
|
||||
let old_sig = key.sign(&untagged);
|
||||
|
||||
// The current verifier (which uses the domain-tagged message)
|
||||
// must reject a signature made over the un-tagged bytes.
|
||||
let err = verify_signature(&event, &old_sig, &public).unwrap_err();
|
||||
assert_eq!(err, SignatureVerifyError::Invalid);
|
||||
|
||||
// Sanity: the proper signature still verifies.
|
||||
let good = sign_event(&event, &key);
|
||||
verify_signature(&event, &good, &public).expect("tagged signature verifies");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn verify_uses_strict_path_and_pins_caller_key() {
|
||||
// Regression guard: verification must run through the strict
|
||||
// path against a CALLER-supplied key. A wrong key fails; the
|
||||
// event never carries its own verifying key, so a forged event
|
||||
// cannot self-attest. (verify_strict additionally rejects
|
||||
// non-canonical / small-order encodings.)
|
||||
let key = fixed_key();
|
||||
let wrong = SigningKey::from_bytes(b"another-wrong-key-another-wrong-");
|
||||
let event = fresh_event();
|
||||
let sig = sign_event(&event, &key);
|
||||
verify_signature(&event, &sig, &key.verifying_key()).expect("right key verifies");
|
||||
assert_eq!(
|
||||
verify_signature(&event, &sig, &wrong.verifying_key()).unwrap_err(),
|
||||
SignatureVerifyError::Invalid
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn verify_rejects_signature_under_wrong_key() {
|
||||
let key = fixed_key();
|
||||
|
||||
Reference in New Issue
Block a user