fix(audit): frame hash inputs with TLV (#7492)

Hash new audit entries from a TLV buffer built with one-byte field tags,
big-endian u64 byte lengths, and raw values. Absent optional fields are
omitted; present empty values retain their tag and zero length. This
prevents bytes from moving between fields without restricting object IDs
or inferring boundaries from field contents.

Store an explicit `hash_version`. Migration 0048 labels existing rows as
version 1, and new writers use version 2. Historical hashes are
unchanged and mixed-version chains verify. The legacy encoding retains
its original ambiguity; TLV protects new entries. Apply the migration
before deploying the updated writers and verifiers.

Tests cover both boundary collisions, absent versus empty fields, stable
digests, unknown versions, mixed-version chains, and stored-row
tampering. All 16 audit and 128 database unit tests passed. The six
PostgreSQL audit tests passed against both migrated and fresh schemas,
and a populated-row migration check preserved the historical hash. Full
`just ci` passed.

Related audit fixes: #4216 and #4249; issue #4173.

Signed-off-by: Jordan Mecom <jm@squareup.com>
This commit is contained in:
Jordan Mecom
2026-09-22 11:19:11 -07:00
committed by GitHub
parent a929532a04
commit 7881447c19
10 changed files with 168 additions and 38 deletions

View File

@@ -365,6 +365,7 @@ test-unit:
./scripts/test-ensure-local-relay-key.sh ./scripts/test-ensure-local-relay-key.sh
if command -v cargo-nextest &>/dev/null; then if command -v cargo-nextest &>/dev/null; then
cargo nextest run -p buzz-core -p buzz-auth --lib 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 # buzz-auth NIP-FI verifier doctests. The sealed-authority
# `compile_fail` doctests prove the default-feature public API alone # `compile_fail` doctests prove the default-feature public API alone
# cannot forge the issuer→JWKS authority; nextest does not run # cannot forge the issuer→JWKS authority; nextest does not run

View File

@@ -16,10 +16,12 @@ pub struct AuditEntry {
pub community_id: Uuid, pub community_id: Uuid,
/// Sequence number, monotonic within `community_id` (starts at 1). /// Sequence number, monotonic within `community_id` (starts at 1).
pub seq: i64, 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`. /// SHA-256 of this entry's fields including `community_id` and `prev_hash`.
pub hash: Vec<u8>, pub hash: Vec<u8>,
/// SHA-256 of the previous entry in *this community's* chain, or `None` for /// 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<Vec<u8>>, pub prev_hash: Option<Vec<u8>>,
/// Action that was performed. /// Action that was performed.
pub action: AuditAction, pub action: AuditAction,

View File

@@ -35,6 +35,13 @@ pub enum AuditError {
#[error("unknown audit action in database")] #[error("unknown audit action in database")]
UnknownAction, 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`). /// A JSON serialization error occurred (e.g. while canonicalising `detail`).
#[error("serialization error: {0}")] #[error("serialization error: {0}")]
Serialization(#[from] serde_json::Error), Serialization(#[from] serde_json::Error),
@@ -69,6 +76,7 @@ mod tests {
AuditError::ChainViolation { seq: 7 }, AuditError::ChainViolation { seq: 7 },
AuditError::HashMismatch { seq: 42 }, AuditError::HashMismatch { seq: 42 },
AuditError::UnknownAction, AuditError::UnknownAction,
AuditError::UnsupportedHashVersion { version: 99 },
]; ];
for err in &domain_errors { for err in &domain_errors {

View File

@@ -5,9 +5,12 @@ use crate::entry::AuditEntry;
use crate::error::AuditError; use crate::error::AuditError;
/// The 32-byte sentinel hashed in place of `prev_hash` for a community's first /// 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]; 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. /// Reduce a timestamp to the precision the audit store round-trips.
/// ///
/// `audit_log.created_at` is `TIMESTAMPTZ`, which Postgres keeps at microsecond /// `audit_log.created_at` is `TIMESTAMPTZ`, which Postgres keeps at microsecond
@@ -23,23 +26,54 @@ pub fn to_storage_precision(created_at: DateTime<Utc>) -> DateTime<Utc> {
created_at.trunc_subsecs(6) created_at.trunc_subsecs(6)
} }
/// SHA-256 over the entry's identity, chain, and context fields. /// SHA-256 over the entry's fields using its stored encoding version.
/// ///
/// Field order is fixed — changing it invalidates all existing chains. The /// Version 2 uses one-byte tags and big-endian u64 byte lengths. Fields are
/// `community_id` is hashed first so chain identity carries the tenant: an entry /// appended in tag order; absent optional fields are omitted, while present
/// cannot be lifted out of one community's chain and re-verified inside another. /// empty values retain their tag and zero length. JSON keys are sorted and
/// /// timestamps are truncated to storage precision before encoding.
/// `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.
///
/// `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.
pub fn compute_hash(entry: &AuditEntry) -> Result<[u8; 32], AuditError> { 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<u8>, 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(); let mut hasher = Sha256::new();
// Tenant binding: community_id leads the hash. // Tenant binding: community_id leads the hash.
hasher.update(entry.community_id.as_bytes()); hasher.update(entry.community_id.as_bytes());
@@ -124,6 +158,7 @@ mod tests {
fn sample_entry() -> AuditEntry { fn sample_entry() -> AuditEntry {
AuditEntry { AuditEntry {
hash_version: CURRENT_HASH_VERSION,
community_id: Uuid::from_u128(1), community_id: Uuid::from_u128(1),
seq: 1, seq: 1,
hash: Vec::new(), hash: Vec::new(),
@@ -156,6 +191,25 @@ mod tests {
assert_eq!(compute_hash(&entry).unwrap().len(), 32); 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] #[test]
fn storage_precision_drops_sub_microsecond_digits() { fn storage_precision_drops_sub_microsecond_digits() {
let stored = to_storage_precision(nanosecond_instant()); let stored = to_storage_precision(nanosecond_instant());
@@ -254,13 +308,46 @@ mod tests {
} }
#[test] #[test]
fn presence_tag_distinguishes_none_from_empty() { fn omitted_fields_are_distinct_from_empty_values() {
// Some(empty) must not collide with None — the presence tag prevents it.
let mut none = sample_entry(); let mut none = sample_entry();
none.actor_pubkey = None; none.actor_pubkey = None;
let mut empty = sample_entry(); let mut empty = sample_entry();
empty.actor_pubkey = Some(Vec::new()); empty.actor_pubkey = Some(Vec::new());
assert_ne!(compute_hash(&none).unwrap(), compute_hash(&empty).unwrap()); 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] #[test]

View File

@@ -14,8 +14,8 @@
//! advisory lock, so the chain stays consistent across relay processes without one //! advisory lock, so the chain stays consistent across relay processes without one
//! global lock serializing (and timing-coupling) every tenant. //! global lock serializing (and timing-coupling) every tenant.
//! //!
//! The `audit_log` table is owned by the consolidated `0001` migration — this crate //! The `audit_log` table is owned by `migrations/` — this crate is pure chain
//! is pure chain logic and ships no DDL. //! logic and ships no DDL.
/// Audit action types recorded in the log. /// Audit action types recorded in the log.
pub mod action; pub mod action;

View File

@@ -11,7 +11,7 @@ use crate::{
action::AuditAction, action::AuditAction,
entry::{AuditEntry, NewAuditEntry}, entry::{AuditEntry, NewAuditEntry},
error::AuditError, 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. /// The `created_at` stamped on a new entry.
@@ -119,6 +119,7 @@ impl AuditService {
let mut audit_entry = AuditEntry { let mut audit_entry = AuditEntry {
community_id, community_id,
seq, seq,
hash_version: CURRENT_HASH_VERSION,
hash: Vec::new(), hash: Vec::new(),
prev_hash, prev_hash,
action: entry.action, action: entry.action,
@@ -135,8 +136,8 @@ impl AuditService {
sqlx::query( sqlx::query(
r#" r#"
INSERT INTO audit_log INSERT INTO audit_log
(community_id, seq, hash, prev_hash, action, actor_pubkey, object_id, detail, created_at) (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) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)
"#, "#,
) )
.bind(audit_entry.community_id) .bind(audit_entry.community_id)
@@ -148,6 +149,7 @@ impl AuditService {
.bind(audit_entry.object_id.as_deref()) .bind(audit_entry.object_id.as_deref())
.bind(&audit_entry.detail) .bind(&audit_entry.detail)
.bind(audit_entry.created_at) .bind(audit_entry.created_at)
.bind(audit_entry.hash_version)
.execute(&mut *tx) .execute(&mut *tx)
.await?; .await?;
@@ -175,7 +177,7 @@ impl AuditService {
let rows = sqlx::query( let rows = sqlx::query(
r#" r#"
SELECT community_id, seq, hash, prev_hash, action, actor_pubkey, 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 FROM audit_log
WHERE community_id = $1 AND seq BETWEEN $2 AND $3 WHERE community_id = $1 AND seq BETWEEN $2 AND $3
ORDER BY seq ASC ORDER BY seq ASC
@@ -231,7 +233,7 @@ impl AuditService {
let rows = sqlx::query( let rows = sqlx::query(
r#" r#"
SELECT community_id, seq, hash, prev_hash, action, actor_pubkey, 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 FROM audit_log
WHERE community_id = $1 AND seq >= $2 WHERE community_id = $1 AND seq >= $2
ORDER BY seq ASC ORDER BY seq ASC
@@ -258,6 +260,7 @@ fn row_to_audit_entry(row: &sqlx::postgres::PgRow) -> Result<AuditEntry, AuditEr
Ok(AuditEntry { Ok(AuditEntry {
community_id: row.get::<Uuid, _>("community_id"), community_id: row.get::<Uuid, _>("community_id"),
seq: row.get("seq"), seq: row.get("seq"),
hash_version: row.get("hash_version"),
hash: row.get("hash"), hash: row.get("hash"),
prev_hash: row.get("prev_hash"), prev_hash: row.get("prev_hash"),
action, action,
@@ -343,6 +346,7 @@ mod postgres_tests {
.await .await
.unwrap(); .unwrap();
assert_eq!(e.seq, 1, "first entry in a community starts at seq 1"); 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!(e.prev_hash.is_none(), "genesis entry has NULL prev_hash");
assert_eq!(e.hash.len(), 32); assert_eq!(e.hash.len(), 32);
assert_eq!(e.community_id, c); assert_eq!(e.community_id, c);
@@ -358,10 +362,20 @@ mod postgres_tests {
let svc = AuditService::new(pool.clone()); let svc = AuditService::new(pool.clone());
let c = make_community(&pool).await; let c = make_community(&pool).await;
let e1 = svc let mut e1 = svc
.log(new_entry(c, AuditAction::EventCreated)) .log(new_entry(c, AuditAction::EventCreated))
.await .await
.unwrap(); .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 let e2 = svc
.log(new_entry(c, AuditAction::ChannelCreated)) .log(new_entry(c, AuditAction::ChannelCreated))
.await .await
@@ -374,6 +388,8 @@ mod postgres_tests {
assert_eq!(e1.seq, 1); assert_eq!(e1.seq, 1);
assert_eq!(e2.seq, 2); assert_eq!(e2.seq, 2);
assert_eq!(e3.seq, 3); 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!(e1.prev_hash.is_none());
assert_eq!(e2.prev_hash.as_deref(), Some(e1.hash.as_slice())); assert_eq!(e2.prev_hash.as_deref(), Some(e1.hash.as_slice()));
assert_eq!(e3.prev_hash.as_deref(), Some(e2.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) .verify_chain(CommunityId::from_uuid(c), 1, 3)
.await .await
.unwrap()); .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 /// THE isolation property: two communities keep independent chains. Each
@@ -460,20 +483,19 @@ mod postgres_tests {
svc.log(new_entry(c, AuditAction::EventCreated)) svc.log(new_entry(c, AuditAction::EventCreated))
.await .await
.unwrap(); .unwrap();
let e2 = svc let mut input = new_entry(c, AuditAction::EventDeleted);
.log(new_entry(c, AuditAction::EventDeleted)) input.actor_pubkey = Some(vec![1; 32]);
.await let e2 = svc.log(input).await.unwrap();
.unwrap();
svc.log(new_entry(c, AuditAction::ChannelDeleted)) svc.log(new_entry(c, AuditAction::ChannelDeleted))
.await .await
.unwrap(); .unwrap();
// Tamper with e2's stored actor_pubkey. // This shift preserved the old concatenated bytes, but changes TLV.
let tampered: Vec<u8> = vec![0xff; 32]; sqlx::query("UPDATE audit_log SET actor_pubkey = $1, object_id = $4 WHERE community_id = $2 AND seq = $3")
sqlx::query("UPDATE audit_log SET actor_pubkey = $1 WHERE community_id = $2 AND seq = $3") .bind(vec![1_u8; 31])
.bind(tampered)
.bind(c) .bind(c)
.bind(e2.seq) .bind(e2.seq)
.bind(format!("\u{1}{}", e2.object_id.as_deref().unwrap()))
.execute(&pool) .execute(&pool)
.await .await
.unwrap(); .unwrap();
@@ -503,8 +525,8 @@ mod postgres_tests {
// Forge: copy A's seq-1 row's hash into B's chain at seq 1. // Forge: copy A's seq-1 row's hash into B's chain at seq 1.
sqlx::query( sqlx::query(
"INSERT INTO audit_log (community_id, seq, hash, prev_hash, action, actor_pubkey, object_id, detail, created_at) "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, NOW())", VALUES ($1, 1, $2, NULL, $3, $4, $5, $6, $7, $8)",
) )
.bind(b) .bind(b)
.bind(&a1.hash) // A's hash, which was computed over community_id = A .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.actor_pubkey.as_deref())
.bind(a1.object_id.as_deref()) .bind(a1.object_id.as_deref())
.bind(&a1.detail) .bind(&a1.detail)
.bind(a1.created_at)
.bind(a1.hash_version)
.execute(&pool) .execute(&pool)
.await .await
.unwrap(); .unwrap();

View File

@@ -703,7 +703,7 @@ mod postgres_tests {
let mut migrations: Vec<_> = MIGRATOR.iter().collect(); let mut migrations: Vec<_> = MIGRATOR.iter().collect();
migrations.sort_by_key(|migration| migration.version); 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].version, 1);
assert_eq!(&*migrations[0].description, "initial schema"); assert_eq!(&*migrations[0].description, "initial schema");
assert!(migrations[0] assert!(migrations[0]

View File

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

View File

@@ -645,6 +645,7 @@ CREATE TABLE archived_identities (
CREATE TABLE audit_log ( CREATE TABLE audit_log (
community_id UUID NOT NULL REFERENCES communities(id), community_id UUID NOT NULL REFERENCES communities(id),
seq BIGINT NOT NULL, seq BIGINT NOT NULL,
hash_version SMALLINT NOT NULL DEFAULT 1 CHECK (hash_version IN (1, 2)),
hash BYTEA NOT NULL, hash BYTEA NOT NULL,
prev_hash BYTEA, prev_hash BYTEA,
action VARCHAR(64) NOT NULL, action VARCHAR(64) NOT NULL,

View File

@@ -81,6 +81,9 @@ run_unit_tests() {
run_test_step "buzz-core tests" \ run_test_step "buzz-core tests" \
cargo test -p buzz-core --lib -- --nocapture 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" \ run_test_step "buzz-auth unit tests" \
cargo test -p buzz-auth --lib -- --nocapture cargo test -p buzz-auth --lib -- --nocapture