From 209e39c62d5fbf2fdb083fdc3e288b506b17d227 Mon Sep 17 00:00:00 2001 From: Transaction Science Date: Sat, 17 Oct 2026 09:35:49 -0400 Subject: [PATCH] op-screening: the audit log can outlive the process that wrote it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The crate header claims every `Screener::screen` call is recorded into a chained, signed audit log "so that regulators can inspect the screening history of any payment attempt after the fact". The log could not do that. Its whole public surface was `new`, `verifying_key`, `entries`, `record` and `verify`: no save, no load, and no way to supply an existing key. The only constructor mints a fresh keypair from `OsRng`, and every call site in the repo is `AuditLog::new()`. So the trail died with the process, twice over. The rows were gone, and the key was gone — which is the worse half: on the next run `new()` mints a new keypair, so the verifying key a regulator ingested out-of-band no longer matches anything, and rows an operator had dumped to disk by hand could never be verified by any process, because `verify` only ever checked against the key the running log happened to own. "After the fact" was the one time the log could not be read. The same crate already persists its other long-lived state — `SanctionsIndex::save_snapshot` / `load_snapshot`, CBOR via ciborium — so this follows that precedent rather than inventing one: - `AuditLog::with_signing_key` signs with a key the operator already holds, instead of rotating it on every start. - `save_entries` / `load_entries` persist the rows. The signing key is deliberately **not** written: a private key does not belong in a file beside the rows it signed, and this crate ships `compliance/hsm-kms-guidance.md` saying where it does belong. - `resume(signing_key, entries)` continues the chain onto its real head, so the next row's `prev_hash` and `seq` follow the pre-restart rows rather than starting over. It verifies them first: resuming onto rows this key did not write would build a log that fails verification later, at the regulator, over a chain nobody can repair — so it fails here instead. - `verify_entries(entries, verifying_key)` is a free function, which is the regulator's path: rows plus the published public key, with no signing key and no live log. `AuditLog::verify` delegates to it, so the two invariants have one implementation rather than two that can drift. Additive throughout — `new`, `Default` and every existing call site are unchanged. Tests: a restart writes two rows, loads them back, resumes and appends a third, and the whole chain verifies with `seq` continuing at 2; a regulator verifies dumped rows from the public key alone, and both invariants still bite on the detached rows (an edited row breaks its chain hash at index 0, a removed row breaks the chain); resuming under the wrong key is refused; and `a_fresh_log_cannot_verify_another_runs_rows` pins the limit of `new()` that motivated all of it. Co-Authored-By: Claude Opus 5 --- openpay/crates/op-screening/src/audit.rs | 293 ++++++++++++++++++++--- openpay/crates/op-screening/src/lib.rs | 2 +- 2 files changed, 265 insertions(+), 30 deletions(-) diff --git a/openpay/crates/op-screening/src/audit.rs b/openpay/crates/op-screening/src/audit.rs index 7dda92d3f..22120bf2d 100644 --- a/openpay/crates/op-screening/src/audit.rs +++ b/openpay/crates/op-screening/src/audit.rs @@ -14,6 +14,26 @@ //! hash breaks). //! //! [`AuditLog::verify`] checks both. +//! +//! ## Surviving a restart +//! +//! Both invariants are about a log a regulator reads *later*, so the log has +//! to outlive the process that wrote it. [`AuditLog::new`] mints a fresh +//! keypair, which is right for a one-shot run and wrong for anything +//! inspected after the fact: the verifying key the regulator ingested +//! out-of-band no longer matches, and rows written before the restart cannot +//! be verified by any process. +//! +//! So: [`AuditLog::with_signing_key`] signs with an operator-held key, +//! [`AuditLog::save_entries`] / [`AuditLog::load_entries`] persist the rows +//! (never the key — that belongs where `compliance/hsm-kms-guidance.md` says), +//! [`AuditLog::resume`] continues the chain onto its real head, and +//! [`verify_entries`] verifies a detached run of rows from the published +//! verifying key alone, with no signing key and no live log. + +use std::fs::File; +use std::io::{BufReader, BufWriter}; +use std::path::Path; use chrono::{DateTime, Utc}; use ed25519_dalek::{Signature, Signer, SigningKey, Verifier, VerifyingKey}; @@ -75,6 +95,76 @@ impl AuditLog { Self { signing_key, verifying_key, entries: Vec::new() } } + /// Build a log that signs with a key the operator already holds, so the + /// chain can be continued after a restart. + /// + /// [`Self::new`] mints a fresh keypair every time it is called, which + /// silently rotates the key a regulator was given. Hand the long-lived key + /// in here instead; keep it where `compliance/hsm-kms-guidance.md` says to + /// keep it. + #[must_use] + pub fn with_signing_key(signing_key: SigningKey) -> Self { + let verifying_key = signing_key.verifying_key(); + Self { + signing_key, + verifying_key, + entries: Vec::new(), + } + } + + /// Continue an existing chain: the same signing key, plus the rows written + /// before the restart so the next row hashes onto the real head rather + /// than onto an empty one. + /// + /// # Errors + /// [`Error::AuditChainBroken`] or [`Error::AuditSignatureInvalid`] if the + /// rows do not verify under this key. Resuming onto rows this key did not + /// write would build a log that fails verification later — at the + /// regulator, over a chain nobody can repair — so it fails here instead. + pub fn resume(signing_key: SigningKey, entries: Vec) -> Result { + let verifying_key = signing_key.verifying_key(); + verify_entries(&entries, &verifying_key)?; + Ok(Self { + signing_key, + verifying_key, + entries, + }) + } + + /// Write the rows to a CBOR file, as + /// [`SanctionsIndex::save_snapshot`](crate::SanctionsIndex::save_snapshot) + /// does for the index. + /// + /// The signing key is deliberately **not** written: a private key does not + /// belong in a file beside the rows it signed. Pair this with + /// [`Self::resume`] and a key from the operator's own store. + /// + /// # Errors + /// [`Error::Io`] if the file cannot be written, [`Error::Cbor`] on an + /// encoding failure. + pub fn save_entries(&self, path: &Path) -> Result<()> { + let file = File::create(path)?; + let writer = BufWriter::new(file); + ciborium::into_writer(&self.entries, writer).map_err(|e| Error::Cbor(e.to_string()))?; + Ok(()) + } + + /// Read rows back from a file written by [`Self::save_entries`]. + /// + /// Returns the rows unverified — pass them to [`Self::resume`] to continue + /// signing, or to [`verify_entries`] to check them against a published key. + /// + /// # Errors + /// [`Error::Io`] if the file cannot be read, [`Error::Cbor`] on a decoding + /// failure. + pub fn load_entries(path: &Path) -> Result> { + let file = File::open(path)?; + let reader = BufReader::new(file); + let entries: Vec = + ciborium::from_reader(reader).map_err(|e| Error::Cbor(e.to_string()))?; + Ok(entries) + } + /// Public verifying key (regulators ingest this out-of-band). #[must_use] pub fn verifying_key(&self) -> VerifyingKey { @@ -156,39 +246,59 @@ impl AuditLog { /// Recompute every entry's chain hash and verify every signature. /// /// Returns `Ok(())` only if the entire log is intact. + /// + /// # Errors + /// See [`verify_entries`], which this delegates to. pub fn verify(&self) -> Result<()> { - let mut prev_hash = String::new(); - for (i, e) in self.entries.iter().enumerate() { - let body = serde_json::to_string(&AuditBody { - seq: e.seq, - at: e.at, - query: &e.query, - decision: &e.decision, - hit_count: e.hit_count, - top_hit: e.top_hit.clone(), - id: e.id.clone(), - })?; - let mut hasher = blake3::Hasher::new(); - hasher.update(prev_hash.as_bytes()); - hasher.update(body.as_bytes()); - let recomputed = hex(hasher.finalize().as_bytes()); - if recomputed != e.chain_hash { - return Err(Error::AuditChainBroken(i)); - } - let sig_bytes = unhex(&e.signature).ok_or(Error::AuditSignatureInvalid)?; - let sig_array: [u8; 64] = sig_bytes - .try_into() - .map_err(|_| Error::AuditSignatureInvalid)?; - let sig = Signature::from_bytes(&sig_array); - self.verifying_key - .verify(e.chain_hash.as_bytes(), &sig) - .map_err(|_| Error::AuditSignatureInvalid)?; - prev_hash = e.chain_hash.clone(); - } - Ok(()) + verify_entries(&self.entries, &self.verifying_key) } } +/// Verify a detached run of rows against a published verifying key. +/// +/// This is the regulator's path. The crate header's claim is that "regulators +/// can inspect the screening history of any payment attempt after the fact", +/// which requires verification to be possible from the rows plus the public +/// key alone — no signing key, no live [`AuditLog`], no process that is still +/// running. [`AuditLog::verify`] delegates here so there is one +/// implementation of the two invariants rather than two that can drift. +/// +/// # Errors +/// [`Error::AuditChainBroken`] with the index of the first row whose chain +/// hash does not recompute, or [`Error::AuditSignatureInvalid`] if a signature +/// is malformed or does not verify under `verifying_key`. +pub fn verify_entries(entries: &[AuditEntry], verifying_key: &VerifyingKey) -> Result<()> { + let mut prev_hash = String::new(); + for (i, e) in entries.iter().enumerate() { + let body = serde_json::to_string(&AuditBody { + seq: e.seq, + at: e.at, + query: &e.query, + decision: &e.decision, + hit_count: e.hit_count, + top_hit: e.top_hit.clone(), + id: e.id.clone(), + })?; + let mut hasher = blake3::Hasher::new(); + hasher.update(prev_hash.as_bytes()); + hasher.update(body.as_bytes()); + let recomputed = hex(hasher.finalize().as_bytes()); + if recomputed != e.chain_hash { + return Err(Error::AuditChainBroken(i)); + } + let sig_bytes = unhex(&e.signature).ok_or(Error::AuditSignatureInvalid)?; + let sig_array: [u8; 64] = sig_bytes + .try_into() + .map_err(|_| Error::AuditSignatureInvalid)?; + let sig = Signature::from_bytes(&sig_array); + verifying_key + .verify(e.chain_hash.as_bytes(), &sig) + .map_err(|_| Error::AuditSignatureInvalid)?; + prev_hash = e.chain_hash.clone(); + } + Ok(()) +} + impl Default for AuditLog { fn default() -> Self { Self::new() @@ -305,4 +415,129 @@ mod tests { other => panic!("wrong err: {other:?}"), } } + + // ---- surviving a restart ---------------------------------------- + + fn key(seed: u8) -> SigningKey { + SigningKey::from_bytes(&[seed; 32]) + } + + fn tempdir() -> std::path::PathBuf { + let mut p = std::env::temp_dir(); + let uniq = format!( + "op-screening-audit-{}", + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or(0) + ); + p.push(uniq); + std::fs::create_dir_all(&p).expect("mkdir"); + p + } + + /// The restart the crate header's claim depends on: two rows written, the + /// process ends, the rows come back and the chain continues onto its real + /// head rather than onto an empty one. + #[test] + fn a_restart_continues_the_chain() { + let dir = tempdir(); + let path = dir.join("audit.cbor"); + + let mut before = AuditLog::with_signing_key(key(1)); + before.record("Alpha Corp", &[], "clear").expect("row 0"); + before.record("Beta Ltd", &[], "review").expect("row 1"); + before.save_entries(&path).expect("save"); + let head = before.entries().last().expect("two rows").chain_hash.clone(); + drop(before); + + let rows = AuditLog::load_entries(&path).expect("load"); + assert_eq!(rows.len(), 2, "both pre-restart rows came back"); + let mut after = AuditLog::resume(key(1), rows).expect("the rows verify under this key"); + after.record("Gamma GmbH", &[], "clear").expect("row 2"); + + let all = after.entries(); + assert_eq!(all.len(), 3); + assert_eq!(all[2].seq, 2, "the sequence continues, it does not restart at 0"); + assert_ne!( + all[2].chain_hash, head, + "the new row is its own link, not a repeat of the old head" + ); + after.verify().expect("the whole chain verifies across the restart"); + } + + /// The regulator's path: rows on disk plus the published verifying key, no + /// signing key and no live log. That is what "inspect the screening + /// history after the fact" has to mean. + #[test] + fn a_regulator_verifies_dumped_rows_with_only_the_public_key() { + let dir = tempdir(); + let path = dir.join("audit.cbor"); + + let mut log = AuditLog::with_signing_key(key(2)); + log.record("Delta SA", &[], "clear").expect("row"); + log.record("Epsilon BV", &[], "hit").expect("row"); + log.save_entries(&path).expect("save"); + let published = log.verifying_key(); + drop(log); + + let rows = AuditLog::load_entries(&path).expect("load"); + verify_entries(&rows, &published).expect("the dumped rows verify on their own"); + + // Both invariants still bite on the detached rows. + let mut edited = rows.clone(); + edited[0].query = "Not Delta SA".into(); + assert!( + matches!( + verify_entries(&edited, &published), + Err(Error::AuditChainBroken(0)) + ), + "an edited row breaks its chain hash" + ); + + let mut dropped = rows.clone(); + dropped.remove(0); + assert!( + verify_entries(&dropped, &published).is_err(), + "a removed row breaks the chain" + ); + } + + /// Resuming onto rows a different key signed fails here, rather than + /// building a log that fails later at the regulator over a chain nobody + /// can repair. + #[test] + fn resuming_with_the_wrong_key_is_refused() { + let mut log = AuditLog::with_signing_key(key(3)); + log.record("Zeta Oy", &[], "clear").expect("row"); + let rows = log.entries().to_vec(); + + assert!( + AuditLog::resume(key(4), rows.clone()).is_err(), + "the rows do not verify under this key" + ); + AuditLog::resume(key(3), rows).expect("the right key resumes"); + } + + /// Why `with_signing_key` exists. `new()` mints a fresh keypair, so a + /// second run cannot verify the first run's rows at all — pinned here so + /// it reads as a known limit of `new()` rather than a surprise found by a + /// regulator. + #[test] + fn a_fresh_log_cannot_verify_another_runs_rows() { + let mut first = AuditLog::new(); + first.record("Eta AB", &[], "clear").expect("row"); + let rows = first.entries().to_vec(); + + let second = AuditLog::new(); + assert_ne!( + first.verifying_key().to_bytes(), + second.verifying_key().to_bytes(), + "each new() mints its own key" + ); + assert!( + verify_entries(&rows, &second.verifying_key()).is_err(), + "so the previous run's rows are unverifiable under the new key" + ); + } } diff --git a/openpay/crates/op-screening/src/lib.rs b/openpay/crates/op-screening/src/lib.rs index f56ebe4a0..01935a53b 100644 --- a/openpay/crates/op-screening/src/lib.rs +++ b/openpay/crates/op-screening/src/lib.rs @@ -96,7 +96,7 @@ pub mod screener; pub mod storage; pub mod updater; -pub use audit::{AuditEntry, AuditLog, AuditVerifyError}; +pub use audit::{AuditEntry, AuditLog, AuditVerifyError, verify_entries}; pub use bsa::{CtrHelper, SarHelper, Transaction, Trigger, TriggerKind}; pub use error::{Error, Result}; pub use lists::{ -- 2.54.0 (Apple Git-157)