From 53aa6bed29e7ec06b2f6d94002563431ec23ed7e Mon Sep 17 00:00:00 2001 From: Jordan Mecom Date: Mon, 21 Sep 2026 15:58:10 -0700 Subject: [PATCH] fix(audit): frame hash inputs with TLV Signed-off-by: Jordan Mecom --- Justfile | 1 + crates/buzz-audit/src/entry.rs | 4 +- crates/buzz-audit/src/error.rs | 8 ++ crates/buzz-audit/src/hash.rs | 123 ++++++++++++++++++++---- crates/buzz-audit/src/lib.rs | 4 +- crates/buzz-audit/src/service.rs | 56 ++++++++--- crates/buzz-db/src/runtime/migration.rs | 2 +- migrations/0048_audit_hash_version.sql | 4 + schema/schema.sql | 1 + scripts/run-tests.sh | 3 + 10 files changed, 168 insertions(+), 38 deletions(-) create mode 100644 migrations/0048_audit_hash_version.sql diff --git a/Justfile b/Justfile index 3f5bec95a64..c21035cd750 100644 --- a/Justfile +++ b/Justfile @@ -365,6 +365,7 @@ test-unit: ./scripts/test-ensure-local-relay-key.sh if command -v cargo-nextest &>/dev/null; then cargo nextest run -p buzz-core -p buzz-auth --lib + cargo nextest run -p buzz-audit --lib # buzz-auth NIP-FI verifier doctests. The sealed-authority # `compile_fail` doctests prove the default-feature public API alone # cannot forge the issuer→JWKS authority; nextest does not run diff --git a/crates/buzz-audit/src/entry.rs b/crates/buzz-audit/src/entry.rs index 33b51f8cf3e..a920de4e30d 100644 --- a/crates/buzz-audit/src/entry.rs +++ b/crates/buzz-audit/src/entry.rs @@ -16,10 +16,12 @@ pub struct AuditEntry { pub community_id: Uuid, /// Sequence number, monotonic within `community_id` (starts at 1). pub seq: i64, + /// Hash encoding: 1 for historical concatenation, 2 for TLV. + pub hash_version: i16, /// SHA-256 of this entry's fields including `community_id` and `prev_hash`. pub hash: Vec, /// SHA-256 of the previous entry in *this community's* chain, or `None` for - /// the community's first entry (hashed as [`crate::hash::GENESIS_HASH`]). + /// the community's first entry. pub prev_hash: Option>, /// Action that was performed. pub action: AuditAction, diff --git a/crates/buzz-audit/src/error.rs b/crates/buzz-audit/src/error.rs index b4ffd24d83f..6f28302b4b0 100644 --- a/crates/buzz-audit/src/error.rs +++ b/crates/buzz-audit/src/error.rs @@ -35,6 +35,13 @@ pub enum AuditError { #[error("unknown audit action in database")] UnknownAction, + /// The stored hash encoding version is not supported. + #[error("unsupported audit hash version {version}")] + UnsupportedHashVersion { + /// Unrecognised encoding version. + version: i16, + }, + /// A JSON serialization error occurred (e.g. while canonicalising `detail`). #[error("serialization error: {0}")] Serialization(#[from] serde_json::Error), @@ -69,6 +76,7 @@ mod tests { AuditError::ChainViolation { seq: 7 }, AuditError::HashMismatch { seq: 42 }, AuditError::UnknownAction, + AuditError::UnsupportedHashVersion { version: 99 }, ]; for err in &domain_errors { diff --git a/crates/buzz-audit/src/hash.rs b/crates/buzz-audit/src/hash.rs index 8d6091a00c8..1bbdfe82c62 100644 --- a/crates/buzz-audit/src/hash.rs +++ b/crates/buzz-audit/src/hash.rs @@ -5,9 +5,12 @@ use crate::entry::AuditEntry; use crate::error::AuditError; /// The 32-byte sentinel hashed in place of `prev_hash` for a community's first -/// entry. Stored as `prev_hash = NULL`; hashed as all-zero bytes. +/// entry in the legacy encoding. TLV omits the absent field instead. pub const GENESIS_HASH: [u8; 32] = [0u8; 32]; +/// Encoding used for new entries. Version 1 is retained for historical rows. +pub const CURRENT_HASH_VERSION: i16 = 2; + /// Reduce a timestamp to the precision the audit store round-trips. /// /// `audit_log.created_at` is `TIMESTAMPTZ`, which Postgres keeps at microsecond @@ -23,23 +26,54 @@ pub fn to_storage_precision(created_at: DateTime) -> DateTime { created_at.trunc_subsecs(6) } -/// SHA-256 over the entry's identity, chain, and context fields. -/// -/// Field order is fixed — changing it invalidates all existing chains. The -/// `community_id` is hashed first so chain identity carries the tenant: an entry -/// cannot be lifted out of one community's chain and re-verified inside another. -/// -/// `created_at` is normalized through [`to_storage_precision`] here rather than -/// hashed as given. Write paths truncate before storing so the row matches the -/// in-memory entry, but normalizing again at the single point that consumes the -/// value means no future caller can reintroduce the write/read preimage split -/// by forgetting to. Values already at storage precision are unaffected — -/// truncation is idempotent — so this does not change any digest. +/// SHA-256 over the entry's fields using its stored encoding version. /// -/// `detail` is serialized via [`canonical_json`] (sorted keys) so the hash is -/// stable across machines and Rust versions. A serialization failure is a hard -/// error, never silently hashed as empty. +/// Version 2 uses one-byte tags and big-endian u64 byte lengths. Fields are +/// appended in tag order; absent optional fields are omitted, while present +/// empty values retain their tag and zero length. JSON keys are sorted and +/// timestamps are truncated to storage precision before encoding. pub fn compute_hash(entry: &AuditEntry) -> Result<[u8; 32], AuditError> { + match entry.hash_version { + 1 => compute_legacy_hash(entry), + CURRENT_HASH_VERSION => compute_tlv_hash(entry), + version => Err(AuditError::UnsupportedHashVersion { version }), + } +} + +fn compute_tlv_hash(entry: &AuditEntry) -> Result<[u8; 32], AuditError> { + let mut encoded = b"buzz:audit:v2\0".to_vec(); + append_tlv(&mut encoded, 1, entry.community_id.as_bytes()); + append_tlv(&mut encoded, 2, &entry.seq.to_be_bytes()); + append_tlv( + &mut encoded, + 3, + to_storage_precision(entry.created_at) + .to_rfc3339() + .as_bytes(), + ); + append_tlv(&mut encoded, 4, entry.action.as_str().as_bytes()); + if let Some(pk) = &entry.actor_pubkey { + append_tlv(&mut encoded, 5, pk); + } + if let Some(id) = &entry.object_id { + append_tlv(&mut encoded, 6, id.as_bytes()); + } + append_tlv(&mut encoded, 7, canonical_json(&entry.detail)?.as_bytes()); + if let Some(hash) = &entry.prev_hash { + append_tlv(&mut encoded, 8, hash); + } + Ok(Sha256::digest(encoded).into()) +} + +fn append_tlv(encoded: &mut Vec, tag: u8, value: &[u8]) { + encoded.push(tag); + encoded.extend_from_slice(&(value.len() as u64).to_be_bytes()); + encoded.extend_from_slice(value); +} + +// Preserve historical digests. This encoding retains its original ambiguity; +// only the version 2 writer prevents bytes from moving between fields. +fn compute_legacy_hash(entry: &AuditEntry) -> Result<[u8; 32], AuditError> { let mut hasher = Sha256::new(); // Tenant binding: community_id leads the hash. hasher.update(entry.community_id.as_bytes()); @@ -124,6 +158,7 @@ mod tests { fn sample_entry() -> AuditEntry { AuditEntry { + hash_version: CURRENT_HASH_VERSION, community_id: Uuid::from_u128(1), seq: 1, hash: Vec::new(), @@ -156,6 +191,25 @@ mod tests { assert_eq!(compute_hash(&entry).unwrap().len(), 32); } + #[test] + fn encoding_versions_have_stable_digests() { + let mut entry = sample_entry(); + assert_eq!( + hex::encode(compute_hash(&entry).unwrap()), + "e12c899d941af9794679602ae83ef2ab1c8f029fe380349263a046a93374d580" + ); + entry.hash_version = 1; + assert_eq!( + hex::encode(compute_hash(&entry).unwrap()), + "5fd6c48ddbd39979bd7fcc94357d78b5fcffa0d1443a0cdda997ad7e22f4f2ce" + ); + entry.hash_version = 99; + assert!(matches!( + compute_hash(&entry), + Err(AuditError::UnsupportedHashVersion { version: 99 }) + )); + } + #[test] fn storage_precision_drops_sub_microsecond_digits() { let stored = to_storage_precision(nanosecond_instant()); @@ -254,13 +308,46 @@ mod tests { } #[test] - fn presence_tag_distinguishes_none_from_empty() { - // Some(empty) must not collide with None — the presence tag prevents it. + fn omitted_fields_are_distinct_from_empty_values() { let mut none = sample_entry(); none.actor_pubkey = None; let mut empty = sample_entry(); empty.actor_pubkey = Some(Vec::new()); assert_ne!(compute_hash(&none).unwrap(), compute_hash(&empty).unwrap()); + + none.object_id = None; + empty = none.clone(); + empty.object_id = Some(String::new()); + assert_ne!(compute_hash(&none).unwrap(), compute_hash(&empty).unwrap()); + + empty = none.clone(); + empty.prev_hash = Some(Vec::new()); + assert_ne!(compute_hash(&none).unwrap(), compute_hash(&empty).unwrap()); + empty.prev_hash = Some(vec![0; 32]); + assert_ne!(compute_hash(&none).unwrap(), compute_hash(&empty).unwrap()); + } + + #[test] + fn object_id_and_detail_have_distinct_boundaries() { + let mut a = sample_entry(); + a.object_id = Some("record1".into()); + a.detail = serde_json::json!(23); + let mut b = a.clone(); + b.object_id = Some("record12".into()); + b.detail = serde_json::json!(3); + // The old encoding concatenated both pairs as "record123". + assert_ne!(compute_hash(&a).unwrap(), compute_hash(&b).unwrap()); + } + + #[test] + fn actor_and_object_id_have_distinct_boundaries() { + let mut a = sample_entry(); + a.actor_pubkey = Some(vec![1; 32]); + let mut b = a.clone(); + b.actor_pubkey = Some(vec![1; 31]); + b.object_id = Some(format!("\u{1}{}", a.object_id.as_deref().unwrap())); + // Moving the old presence byte into the object ID preserved its hash. + assert_ne!(compute_hash(&a).unwrap(), compute_hash(&b).unwrap()); } #[test] diff --git a/crates/buzz-audit/src/lib.rs b/crates/buzz-audit/src/lib.rs index 0248a7dfd3f..b172db4a0bd 100644 --- a/crates/buzz-audit/src/lib.rs +++ b/crates/buzz-audit/src/lib.rs @@ -14,8 +14,8 @@ //! advisory lock, so the chain stays consistent across relay processes without one //! global lock serializing (and timing-coupling) every tenant. //! -//! The `audit_log` table is owned by the consolidated `0001` migration — this crate -//! is pure chain logic and ships no DDL. +//! The `audit_log` table is owned by `migrations/` — this crate is pure chain +//! logic and ships no DDL. /// Audit action types recorded in the log. pub mod action; diff --git a/crates/buzz-audit/src/service.rs b/crates/buzz-audit/src/service.rs index 6819fe23ca3..518ae33cec5 100644 --- a/crates/buzz-audit/src/service.rs +++ b/crates/buzz-audit/src/service.rs @@ -11,7 +11,7 @@ use crate::{ action::AuditAction, entry::{AuditEntry, NewAuditEntry}, error::AuditError, - hash::{compute_hash, to_storage_precision}, + hash::{compute_hash, to_storage_precision, CURRENT_HASH_VERSION}, }; /// The `created_at` stamped on a new entry. @@ -119,6 +119,7 @@ impl AuditService { let mut audit_entry = AuditEntry { community_id, seq, + hash_version: CURRENT_HASH_VERSION, hash: Vec::new(), prev_hash, action: entry.action, @@ -135,8 +136,8 @@ impl AuditService { sqlx::query( r#" INSERT INTO audit_log - (community_id, seq, hash, prev_hash, action, actor_pubkey, object_id, detail, created_at) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) + (community_id, seq, hash, prev_hash, action, actor_pubkey, object_id, detail, created_at, hash_version) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) "#, ) .bind(audit_entry.community_id) @@ -148,6 +149,7 @@ impl AuditService { .bind(audit_entry.object_id.as_deref()) .bind(&audit_entry.detail) .bind(audit_entry.created_at) + .bind(audit_entry.hash_version) .execute(&mut *tx) .await?; @@ -175,7 +177,7 @@ impl AuditService { let rows = sqlx::query( r#" SELECT community_id, seq, hash, prev_hash, action, actor_pubkey, - object_id, detail, created_at + object_id, detail, created_at, hash_version FROM audit_log WHERE community_id = $1 AND seq BETWEEN $2 AND $3 ORDER BY seq ASC @@ -231,7 +233,7 @@ impl AuditService { let rows = sqlx::query( r#" SELECT community_id, seq, hash, prev_hash, action, actor_pubkey, - object_id, detail, created_at + object_id, detail, created_at, hash_version FROM audit_log WHERE community_id = $1 AND seq >= $2 ORDER BY seq ASC @@ -258,6 +260,7 @@ fn row_to_audit_entry(row: &sqlx::postgres::PgRow) -> Result("community_id"), seq: row.get("seq"), + hash_version: row.get("hash_version"), hash: row.get("hash"), prev_hash: row.get("prev_hash"), action, @@ -343,6 +346,7 @@ mod postgres_tests { .await .unwrap(); assert_eq!(e.seq, 1, "first entry in a community starts at seq 1"); + assert_eq!(e.hash_version, CURRENT_HASH_VERSION); assert!(e.prev_hash.is_none(), "genesis entry has NULL prev_hash"); assert_eq!(e.hash.len(), 32); assert_eq!(e.community_id, c); @@ -358,10 +362,20 @@ mod postgres_tests { let svc = AuditService::new(pool.clone()); let c = make_community(&pool).await; - let e1 = svc + let mut e1 = svc .log(new_entry(c, AuditAction::EventCreated)) .await .unwrap(); + // Model a historical row, including the migration's version default. + e1.hash_version = 1; + e1.hash = compute_hash(&e1).unwrap().to_vec(); + sqlx::query("UPDATE audit_log SET hash_version = DEFAULT, hash = $1 WHERE community_id = $2 AND seq = $3") + .bind(&e1.hash) + .bind(c) + .bind(e1.seq) + .execute(&pool) + .await + .unwrap(); let e2 = svc .log(new_entry(c, AuditAction::ChannelCreated)) .await @@ -374,6 +388,8 @@ mod postgres_tests { assert_eq!(e1.seq, 1); assert_eq!(e2.seq, 2); assert_eq!(e3.seq, 3); + assert_eq!(e2.hash_version, CURRENT_HASH_VERSION); + assert_eq!(e3.hash_version, CURRENT_HASH_VERSION); assert!(e1.prev_hash.is_none()); assert_eq!(e2.prev_hash.as_deref(), Some(e1.hash.as_slice())); assert_eq!(e3.prev_hash.as_deref(), Some(e2.hash.as_slice())); @@ -381,6 +397,13 @@ mod postgres_tests { .verify_chain(CommunityId::from_uuid(c), 1, 3) .await .unwrap()); + let rows = svc + .get_entries(CommunityId::from_uuid(c), 1, 3) + .await + .unwrap(); + assert_eq!(rows[0].hash_version, 1); + assert_eq!(rows[0].hash, e1.hash); + assert_eq!(rows[1].hash_version, CURRENT_HASH_VERSION); } /// THE isolation property: two communities keep independent chains. Each @@ -460,20 +483,19 @@ mod postgres_tests { svc.log(new_entry(c, AuditAction::EventCreated)) .await .unwrap(); - let e2 = svc - .log(new_entry(c, AuditAction::EventDeleted)) - .await - .unwrap(); + let mut input = new_entry(c, AuditAction::EventDeleted); + input.actor_pubkey = Some(vec![1; 32]); + let e2 = svc.log(input).await.unwrap(); svc.log(new_entry(c, AuditAction::ChannelDeleted)) .await .unwrap(); - // Tamper with e2's stored actor_pubkey. - let tampered: Vec = vec![0xff; 32]; - sqlx::query("UPDATE audit_log SET actor_pubkey = $1 WHERE community_id = $2 AND seq = $3") - .bind(tampered) + // This shift preserved the old concatenated bytes, but changes TLV. + sqlx::query("UPDATE audit_log SET actor_pubkey = $1, object_id = $4 WHERE community_id = $2 AND seq = $3") + .bind(vec![1_u8; 31]) .bind(c) .bind(e2.seq) + .bind(format!("\u{1}{}", e2.object_id.as_deref().unwrap())) .execute(&pool) .await .unwrap(); @@ -503,8 +525,8 @@ mod postgres_tests { // Forge: copy A's seq-1 row's hash into B's chain at seq 1. sqlx::query( - "INSERT INTO audit_log (community_id, seq, hash, prev_hash, action, actor_pubkey, object_id, detail, created_at) - VALUES ($1, 1, $2, NULL, $3, $4, $5, $6, NOW())", + "INSERT INTO audit_log (community_id, seq, hash, prev_hash, action, actor_pubkey, object_id, detail, created_at, hash_version) + VALUES ($1, 1, $2, NULL, $3, $4, $5, $6, $7, $8)", ) .bind(b) .bind(&a1.hash) // A's hash, which was computed over community_id = A @@ -512,6 +534,8 @@ mod postgres_tests { .bind(a1.actor_pubkey.as_deref()) .bind(a1.object_id.as_deref()) .bind(&a1.detail) + .bind(a1.created_at) + .bind(a1.hash_version) .execute(&pool) .await .unwrap(); diff --git a/crates/buzz-db/src/runtime/migration.rs b/crates/buzz-db/src/runtime/migration.rs index 97c619d6edb..6e5e14c2c62 100644 --- a/crates/buzz-db/src/runtime/migration.rs +++ b/crates/buzz-db/src/runtime/migration.rs @@ -703,7 +703,7 @@ mod postgres_tests { let mut migrations: Vec<_> = MIGRATOR.iter().collect(); migrations.sort_by_key(|migration| migration.version); - assert_eq!(migrations.len(), 47); + assert_eq!(migrations.len(), 48); assert_eq!(migrations[0].version, 1); assert_eq!(&*migrations[0].description, "initial schema"); assert!(migrations[0] diff --git a/migrations/0048_audit_hash_version.sql b/migrations/0048_audit_hash_version.sql new file mode 100644 index 00000000000..a98748d89f0 --- /dev/null +++ b/migrations/0048_audit_hash_version.sql @@ -0,0 +1,4 @@ +-- Preserve existing hashes. New writers explicitly select the TLV encoding. +ALTER TABLE audit_log + ADD COLUMN hash_version SMALLINT NOT NULL DEFAULT 1 + CHECK (hash_version IN (1, 2)); diff --git a/schema/schema.sql b/schema/schema.sql index 21d0d2ea8ea..797a83f5d10 100644 --- a/schema/schema.sql +++ b/schema/schema.sql @@ -645,6 +645,7 @@ CREATE TABLE archived_identities ( CREATE TABLE audit_log ( community_id UUID NOT NULL REFERENCES communities(id), seq BIGINT NOT NULL, + hash_version SMALLINT NOT NULL DEFAULT 1 CHECK (hash_version IN (1, 2)), hash BYTEA NOT NULL, prev_hash BYTEA, action VARCHAR(64) NOT NULL, diff --git a/scripts/run-tests.sh b/scripts/run-tests.sh index 8395e9d0580..0f33c9c8760 100755 --- a/scripts/run-tests.sh +++ b/scripts/run-tests.sh @@ -81,6 +81,9 @@ run_unit_tests() { run_test_step "buzz-core tests" \ cargo test -p buzz-core --lib -- --nocapture + run_test_step "buzz-audit tests" \ + cargo test -p buzz-audit --lib -- --nocapture + run_test_step "buzz-auth unit tests" \ cargo test -p buzz-auth --lib -- --nocapture