diff --git a/Justfile b/Justfile index 00c4fecc2f9..bf6a9126296 100644 --- a/Justfile +++ b/Justfile @@ -365,6 +365,8 @@ 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 + cargo nextest run -p git-sign-nostr # 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-agent/README.md b/crates/buzz-agent/README.md index 6032dd11392..c6147e15592 100644 --- a/crates/buzz-agent/README.md +++ b/crates/buzz-agent/README.md @@ -383,3 +383,9 @@ Test strategy is **real subprocess, no mocks**: - **Fake LLM** — `tests/fake_llm.rs` and the helpers in `tests/regressions.rs` spin up a real `tokio::net::TcpListener` on port 0, parse `Content-Length`, and return scripted JSON. No HTTP mocking library. - **Fake MCP server** — `tests/bin/fake_mcp.rs` is a separate binary controlled by env vars: `FAKE_MCP_HANG_INIT`, `FAKE_MCP_TOOL_DELAY`, `FAKE_MCP_SPAWN_GRANDCHILD`, etc. Each fault path is a real process being abused. - **Regression tests are the changelog.** Each `#[test]` in `regressions.rs` is named for the bug it locks down: `assistant_text_preserved_across_prompts`, `cancel_leaves_history_valid_for_next_prompt`, `mcp_init_timeout_kills_child`, `oversize_line_kills_connection`. Read them in order to learn the protocol's failure modes. + +OAuth cache filenames now use a versioned JSON encoding of the discovery URL, +client ID, and scopes array. Existing caches from the delimiter-joined format +are left in place but are not loaded: their tokens cannot be unambiguously +associated with a configuration. Sign in once after upgrading; headless agents +need a populated new cache or their configured static bearer token. diff --git a/crates/buzz-agent/src/auth.rs b/crates/buzz-agent/src/auth.rs index 0ae34318c27..2fa8c724548 100644 --- a/crates/buzz-agent/src/auth.rs +++ b/crates/buzz-agent/src/auth.rs @@ -6,9 +6,9 @@ //! and refresh logic; the [`Llm`] just asks for a bearer per request. //! //! The PKCE engine implements RFC 6749 + RFC 7636 with on-disk token -//! caching keyed by `sha256(discovery_url|client_id|scopes)`. It's the -//! same shape goose uses for Databricks, but we own the wire format and -//! cache directory so the two are independently upgradable. +//! caching keyed by a versioned JSON tuple of discovery URL, client ID, and +//! the scopes array. Buzz owns the wire format and cache directory, so it +//! can upgrade them independently of other OAuth clients. //! //! First-use (cache empty) requires a browser: the engine opens //! `authorization_endpoint` in `webbrowser`, listens on `127.0.0.1:0`, @@ -1377,21 +1377,29 @@ fn default_oauth_cache_root() -> Result { } fn cache_path_for(cfg: &PkceOAuthConfig) -> Result { - let mut h = sha2::Sha256::new(); - h.update(cfg.discovery_url.as_bytes()); - h.update(b"|"); - h.update(cfg.client_id.as_bytes()); - h.update(b"|"); - h.update(cfg.scopes.join(",").as_bytes()); - let hash = hex::encode(h.finalize()); + // JSON preserves field boundaries and scope element boundaries, including + // delimiters inside values. Do not fall back to the old joined cache key: + // a token there could belong to a different OAuth configuration. + let identity = serde_json::to_vec(&( + "buzz:oauth-cache:v2", + &cfg.discovery_url, + &cfg.client_id, + &cfg.scopes, + )) + .map_err(|e| AgentError::Llm(format!("oauth cache identity: {e}")))?; + let hash = hex::encode(sha2::Sha256::digest(identity)); let dir = match &cfg.cache_dir_override { Some(p) => p.join(&cfg.cache_namespace), None => default_oauth_cache_root()?.join(&cfg.cache_namespace), }; - Ok(dir.join(format!("{hash}.json"))) + Ok(dir.join(format!("v2-{hash}.json"))) } +#[cfg(test)] +#[path = "auth_cache_identity_tests.rs"] +mod cache_identity_tests; + /// Append `ext` as an extra extension onto `base` (e.g. `.json` → /// `.json.lock`). Keeps the lock and cooldown sidecars in the same /// per-key directory as the cache, so they inherit its `$HOME` override and diff --git a/crates/buzz-agent/src/auth_cache_identity_tests.rs b/crates/buzz-agent/src/auth_cache_identity_tests.rs new file mode 100644 index 00000000000..ef3740df7af --- /dev/null +++ b/crates/buzz-agent/src/auth_cache_identity_tests.rs @@ -0,0 +1,104 @@ +use super::*; + +fn config(dir: &Path) -> PkceOAuthConfig { + PkceOAuthConfig { + discovery_url: "https://example.com/.well-known".into(), + client_id: "client".into(), + scopes: vec!["read".into(), "write".into()], + cache_namespace: "test".into(), + cache_dir_override: Some(dir.to_owned()), + } +} + +fn legacy_identity(cfg: &PkceOAuthConfig) -> String { + format!( + "{}|{}|{}", + cfg.discovery_url, + cfg.client_id, + cfg.scopes.join(",") + ) +} + +#[test] +fn cache_keys_preserve_field_and_scope_boundaries() { + let dir = tempfile::tempdir().unwrap(); + let first = config(dir.path()); + let mut scope_alias = first.clone(); + scope_alias.scopes = vec!["read,write".into()]; + let mut url_alias = first.clone(); + url_alias.discovery_url.push_str("|client"); + url_alias.client_id = "other".into(); + let mut client_alias = first.clone(); + client_alias.client_id = "client|other".into(); + let mut empty_array = first.clone(); + empty_array.scopes.clear(); + let mut empty_element = empty_array.clone(); + empty_element.scopes.push(String::new()); + let mut client_scope = first.clone(); + client_scope.client_id.push_str("|read"); + client_scope.scopes = vec!["write".into()]; + let mut scope_client = first.clone(); + scope_client.scopes = vec!["read|write".into()]; + + for (a, b) in [ + (first, scope_alias), + (url_alias, client_alias), + (empty_array, empty_element), + (client_scope, scope_client), + ] { + assert_eq!(legacy_identity(&a), legacy_identity(&b)); + assert_ne!(cache_path_for(&a).unwrap(), cache_path_for(&b).unwrap()); + } +} + +#[tokio::test] +async fn colliding_configs_do_not_share_tokens_or_coordination_files() { + let dir = tempfile::tempdir().unwrap(); + let cfg = config(dir.path()); + let source = PkceOAuthTokenSource::new(cfg.clone()).unwrap(); + source + .save( + &mut *source.state.lock().await, + CachedToken { + access_token: "only-for-two-scopes".into(), + refresh_token: Some("only-refresh-two-scopes".into()), + expires_at: None, + }, + ) + .unwrap(); + let same = PkceOAuthTokenSource::new(cfg.clone()).unwrap(); + assert_eq!( + same.state.lock().await.as_ref().unwrap().access_token, + "only-for-two-scopes" + ); + let mut other = cfg.clone(); + other.scopes = vec!["read,write".into()]; + assert_eq!(legacy_identity(&cfg), legacy_identity(&other)); + let alias = PkceOAuthTokenSource::new(other).unwrap(); + assert!(alias.state.lock().await.is_none()); + for suffix in ["lock", "cooldown", "attempt"] { + assert_ne!( + append_ext(&source.cache_path, suffix), + append_ext(&alias.cache_path, suffix) + ); + } +} + +#[tokio::test] +async fn legacy_cache_is_not_imported() { + let dir = tempfile::tempdir().unwrap(); + let cfg = config(dir.path()); + let namespace = dir.path().join(&cfg.cache_namespace); + fs::create_dir_all(&namespace).unwrap(); + let hash = hex::encode(sha2::Sha256::digest(legacy_identity(&cfg))); + let old = namespace.join(format!("{hash}.json")); + fs::write( + &old, + br#"{"access_token":"ambiguous","refresh_token":"ambiguous","expires_at":null}"#, + ) + .unwrap(); + let source = PkceOAuthTokenSource::new(cfg).unwrap(); + assert!(source.state.lock().await.is_none()); + assert!(!source.cache_path.exists()); + assert!(old.exists()); +} diff --git a/crates/buzz-agent/tests/databricks_auth_coordinator.rs b/crates/buzz-agent/tests/databricks_auth_coordinator.rs index 0937cc87c1d..b4f08206bbf 100644 --- a/crates/buzz-agent/tests/databricks_auth_coordinator.rs +++ b/crates/buzz-agent/tests/databricks_auth_coordinator.rs @@ -543,16 +543,17 @@ fn future_secs() -> u64 { fn cache_file_path(cfg: &PkceOAuthConfig, cache_dir: &std::path::Path) -> std::path::PathBuf { use sha2::Digest; - let mut h = sha2::Sha256::new(); - h.update(cfg.discovery_url.as_bytes()); - h.update(b"|"); - h.update(cfg.client_id.as_bytes()); - h.update(b"|"); - h.update(cfg.scopes.join(",").as_bytes()); - let hash = hex::encode(h.finalize()); + let identity = serde_json::to_vec(&( + "buzz:oauth-cache:v2", + &cfg.discovery_url, + &cfg.client_id, + &cfg.scopes, + )) + .unwrap(); + let hash = hex::encode(sha2::Sha256::digest(identity)); cache_dir .join(&cfg.cache_namespace) - .join(format!("{hash}.json")) + .join(format!("v2-{hash}.json")) } /// The cross-process attempt sidecar path for a config, matching the diff --git a/crates/buzz-agent/tests/databricks_oauth.rs b/crates/buzz-agent/tests/databricks_oauth.rs index ac2b9578626..dfa6bae53dc 100644 --- a/crates/buzz-agent/tests/databricks_oauth.rs +++ b/crates/buzz-agent/tests/databricks_oauth.rs @@ -81,19 +81,20 @@ async fn spawn_oidc() -> (String, Arc) { (base, counter) } -/// Cache key construction matches the auth module: sha256(discovery|client|scopes). +/// Seed the version 2 cache used by the auth module. fn cache_path_for(cache_dir: &std::path::Path, cfg: &PkceOAuthConfig) -> std::path::PathBuf { use sha2::Digest; - let mut h = sha2::Sha256::new(); - h.update(cfg.discovery_url.as_bytes()); - h.update(b"|"); - h.update(cfg.client_id.as_bytes()); - h.update(b"|"); - h.update(cfg.scopes.join(",").as_bytes()); - let hash = hex::encode(h.finalize()); + let identity = serde_json::to_vec(&( + "buzz:oauth-cache:v2", + &cfg.discovery_url, + &cfg.client_id, + &cfg.scopes, + )) + .unwrap(); + let hash = hex::encode(sha2::Sha256::digest(identity)); cache_dir .join(&cfg.cache_namespace) - .join(format!("{hash}.json")) + .join(format!("v2-{hash}.json")) } /// Write a token file the engine should pick up on construction. @@ -1088,18 +1089,19 @@ fn databricks_oauth_cache_path(home: &std::path::Path, host: &str) -> std::path: "{}/oidc/.well-known/oauth-authorization-server", host.trim_end_matches('/') ); - let mut hasher = Sha256::new(); - hasher.update(discovery_url.as_bytes()); - hasher.update(b"|"); - hasher.update(b"databricks-cli"); - hasher.update(b"|"); - hasher.update(b"all-apis,offline_access"); - let hash = hex::encode(hasher.finalize()); + let identity = serde_json::to_vec(&( + "buzz:oauth-cache:v2", + &discovery_url, + "databricks-cli", + ["all-apis", "offline_access"], + )) + .unwrap(); + let hash = hex::encode(Sha256::digest(identity)); home.join(".config") .join("buzz-agent") .join("oauth") .join("databricks") - .join(format!("{hash}.json")) + .join(format!("v2-{hash}.json")) } fn write_cached_oauth_token(home: &std::path::Path, host: &str, access_token: &str) { diff --git a/crates/buzz-audit/README.md b/crates/buzz-audit/README.md new file mode 100644 index 00000000000..c6d744ea91a --- /dev/null +++ b/crates/buzz-audit/README.md @@ -0,0 +1,31 @@ +# Audit hash encodings + +New entries use `hash_version = 2`. The SHA-256 input starts with +`buzz:audit:v2\0`, then fields in this order: community UUID bytes, signed +64-bit big-endian sequence, storage-precision RFC3339 timestamp, action, +optional actor bytes, optional object ID, canonical JSON detail, optional +previous hash. Field tags are the single bytes 1 through 8. Each required +field carries its tag, unsigned 64-bit big-endian byte length, and bytes. +Optional fields carry their tag and a zero/one presence byte; present values +then carry their length and bytes. Public keys and previous hashes must be +32 bytes when present. + +Apply migration 0045 (or the desired schema) and upgrade verifiers before +deploying the new writer. Older verifiers cannot validate version 2 rows. +The column defaults to 1 so existing rows and old writers retain their actual +encoding during a rolling deployment. New writers explicitly select 2 and +link to the previous row's unchanged hash. Do not backfill the version or +recompute historical hashes. + +Legacy verification accepts absent object IDs, canonical hyphenated UUIDs, +and 64-character lowercase hex identifiers. It rejects other identifiers and +invalid cryptographic field widths because the old concatenation cannot +reliably separate those values. Historical event and media writers used these +accepted forms. A legacy entry with a nonconforming identifier needs operator +inspection; changing its identifier or version is not a repair of its proof. +Legacy genesis entries must have no previous hash, and subsequent entries +must have one. + +These checks prevent alternate interpretations accepted by this verifier. +They cannot prove that a legacy row was never altered before this upgrade. +Preserve any independently trusted chain checkpoints when rolling forward. diff --git a/crates/buzz-audit/src/entry.rs b/crates/buzz-audit/src/entry.rs index 33b51f8cf3e..060a222567d 100644 --- a/crates/buzz-audit/src/entry.rs +++ b/crates/buzz-audit/src/entry.rs @@ -12,6 +12,8 @@ use crate::action::AuditAction; /// community*. The chain is independent per tenant. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct AuditEntry { + /// Encoding used to compute `hash`; legacy rows use 1, new writes use 2. + pub hash_version: i16, /// Server-resolved community this entry belongs to. Leads the primary key. pub community_id: Uuid, /// Sequence number, monotonic within `community_id` (starts at 1). @@ -19,7 +21,7 @@ pub struct AuditEntry { /// 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. Version 2 explicitly encodes its absence. 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..2b90767032b 100644 --- a/crates/buzz-audit/src/error.rs +++ b/crates/buzz-audit/src/error.rs @@ -35,6 +35,22 @@ pub enum AuditError { #[error("unknown audit action in database")] UnknownAction, + /// The entry uses an encoding this verifier does not support. + #[error("unsupported audit hash version {version}")] + UnsupportedHashVersion { + /// Version stored with the entry. + version: i16, + }, + + /// A field violates the encoding's structural requirements. + #[error("invalid audit entry at seq {seq}: {field}")] + InvalidField { + /// Per-community sequence number. + seq: i64, + /// Static field description, never the untrusted field value. + field: &'static str, + }, + /// A JSON serialization error occurred (e.g. while canonicalising `detail`). #[error("serialization error: {0}")] Serialization(#[from] serde_json::Error), diff --git a/crates/buzz-audit/src/hash.rs b/crates/buzz-audit/src/hash.rs index 8d6091a00c8..b6fb547b697 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. Version 2 encodes `None` explicitly. pub const GENESIS_HASH: [u8; 32] = [0u8; 32]; +/// Version written by the audit service. Older rows retain their original hash. +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,90 @@ 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 a versioned encoding of the entry, excluding `hash` itself. /// -/// `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 encodes each field with a tag and a big-endian u64 byte length; +/// optional fields additionally encode a presence byte. JSON keys are sorted +/// and timestamps use storage precision. Legacy hashes are accepted only for +/// fixed-width cryptographic fields and unambiguous object identifiers. pub fn compute_hash(entry: &AuditEntry) -> Result<[u8; 32], AuditError> { + for (field, value) in [ + ("actor_pubkey must be 32 bytes", &entry.actor_pubkey), + ("prev_hash must be 32 bytes", &entry.prev_hash), + ] { + if value.as_ref().is_some_and(|bytes| bytes.len() != 32) { + return Err(AuditError::InvalidField { + seq: entry.seq, + field, + }); + } + } + match entry.hash_version { + 1 => compute_legacy_hash(entry), + CURRENT_HASH_VERSION => { + let mut hasher = Sha256::new(); + hasher.update(b"buzz:audit:v2\0"); + hash_field(&mut hasher, 1, entry.community_id.as_bytes()); + hash_field(&mut hasher, 2, &entry.seq.to_be_bytes()); + hash_field( + &mut hasher, + 3, + to_storage_precision(entry.created_at) + .to_rfc3339() + .as_bytes(), + ); + hash_field(&mut hasher, 4, entry.action.as_str().as_bytes()); + hash_optional(&mut hasher, 5, entry.actor_pubkey.as_deref()); + hash_optional( + &mut hasher, + 6, + entry.object_id.as_deref().map(str::as_bytes), + ); + hash_field(&mut hasher, 7, canonical_json(&entry.detail)?.as_bytes()); + hash_optional(&mut hasher, 8, entry.prev_hash.as_deref()); + Ok(hasher.finalize().into()) + } + version => Err(AuditError::UnsupportedHashVersion { version }), + } +} + +fn hash_field(hasher: &mut Sha256, tag: u8, bytes: &[u8]) { + hasher.update([tag]); + hasher.update((bytes.len() as u64).to_be_bytes()); + hasher.update(bytes); +} + +fn hash_optional(hasher: &mut Sha256, tag: u8, bytes: Option<&[u8]>) { + hasher.update([tag, u8::from(bytes.is_some())]); + if let Some(bytes) = bytes { + hasher.update((bytes.len() as u64).to_be_bytes()); + hasher.update(bytes); + } +} + +fn compute_legacy_hash(entry: &AuditEntry) -> Result<[u8; 32], AuditError> { + if entry.seq < 1 || (entry.seq == 1) != entry.prev_hash.is_none() { + return Err(AuditError::InvalidField { + seq: entry.seq, + field: "legacy prev_hash must be absent only at seq 1", + }); + } + // Legacy producers used event/blob hashes or channel UUIDs. Their encodings + // are prefix-free: a canonical UUID has a '-' at byte 8, unlike hex hashes. + // Arbitrary identifiers cannot be safely separated from the following JSON. + if entry.object_id.as_deref().is_some_and(|id| { + let hash = id.len() == 64 + && id + .bytes() + .all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b)); + let uuid = uuid::Uuid::parse_str(id).is_ok_and(|value| value.to_string() == id); + !hash && !uuid + }) { + return Err(AuditError::InvalidField { + seq: entry.seq, + field: "legacy object_id must be a canonical UUID or 64-character lowercase hex", + }); + } let mut hasher = Sha256::new(); // Tenant binding: community_id leads the hash. hasher.update(entry.community_id.as_bytes()); @@ -72,6 +142,10 @@ pub fn compute_hash(entry: &AuditEntry) -> Result<[u8; 32], AuditError> { Ok(hasher.finalize().into()) } +#[cfg(test)] +#[path = "hash_encoding_tests.rs"] +mod encoding_tests; + /// Serialize a JSON value with sorted object keys for deterministic output. /// /// Propagates any scalar serialization error rather than substituting a @@ -124,6 +198,7 @@ mod tests { fn sample_entry() -> AuditEntry { AuditEntry { + hash_version: CURRENT_HASH_VERSION, community_id: Uuid::from_u128(1), seq: 1, hash: Vec::new(), @@ -255,11 +330,10 @@ mod tests { #[test] fn presence_tag_distinguishes_none_from_empty() { - // Some(empty) must not collide with None — the presence tag prevents it. let mut none = sample_entry(); - none.actor_pubkey = None; + none.object_id = None; let mut empty = sample_entry(); - empty.actor_pubkey = Some(Vec::new()); + empty.object_id = Some(String::new()); assert_ne!(compute_hash(&none).unwrap(), compute_hash(&empty).unwrap()); } diff --git a/crates/buzz-audit/src/hash_encoding_tests.rs b/crates/buzz-audit/src/hash_encoding_tests.rs new file mode 100644 index 00000000000..fca0ecf3a07 --- /dev/null +++ b/crates/buzz-audit/src/hash_encoding_tests.rs @@ -0,0 +1,129 @@ +use super::*; +use crate::AuditAction; + +fn entry() -> AuditEntry { + AuditEntry { + hash_version: CURRENT_HASH_VERSION, + community_id: uuid::Uuid::from_u128(1), + seq: 1, + hash: Vec::new(), + prev_hash: None, + action: AuditAction::EventCreated, + actor_pubkey: Some(vec![0xab; 32]), + object_id: Some("a".repeat(64)), + detail: serde_json::json!({"event_kind": 9, "channel_id": null}), + created_at: "2026-01-01T00:00:00Z".parse().unwrap(), + } +} + +#[test] +fn legacy_digest_is_preserved() { + let mut legacy = entry(); + legacy.hash_version = 1; + // Independently computed using the original concatenation format. + assert_eq!( + hex::encode(compute_hash(&legacy).unwrap()), + "9a14ea40b48dbd85d321cf214c360ae93937d5c146a2a49586847a4317a32345" + ); +} + +#[test] +fn moving_digits_between_object_and_detail_changes_hash() { + let mut first = entry(); + first.object_id = Some("record1".into()); + first.detail = serde_json::json!(23); + let mut second = first.clone(); + second.object_id = Some("record12".into()); + second.detail = serde_json::json!(3); + assert_ne!( + compute_hash(&first).unwrap(), + compute_hash(&second).unwrap() + ); + + // Reinterpreting either row as the old encoding must not resurrect the + // original collision: legacy identifiers have a restricted grammar. + first.hash_version = 1; + second.hash_version = 1; + assert!(compute_hash(&first).is_err()); + assert!(compute_hash(&second).is_err()); +} + +#[test] +fn actor_cannot_absorb_an_object_presence_byte() { + for version in [1, CURRENT_HASH_VERSION] { + let mut original = entry(); + original.hash_version = version; + original.actor_pubkey.as_mut().unwrap()[31] = 1; + assert!(compute_hash(&original).is_ok()); + let mut altered = original.clone(); + altered.actor_pubkey.as_mut().unwrap().pop(); + altered.object_id = Some(format!("\u{1}{}", original.object_id.unwrap())); + assert!(matches!( + compute_hash(&altered), + Err(AuditError::InvalidField { .. }) + )); + } +} + +#[test] +fn optional_values_and_version_are_bound() { + let original = entry(); + let mut changed = original.clone(); + changed.prev_hash = Some(GENESIS_HASH.to_vec()); + assert_ne!( + compute_hash(&original).unwrap(), + compute_hash(&changed).unwrap() + ); + changed.hash_version = 1; + assert!(compute_hash(&changed).is_err()); + changed = original.clone(); + changed.hash_version = 1; + assert_ne!( + compute_hash(&original).unwrap(), + compute_hash(&changed).unwrap() + ); + changed.hash_version = 3; + assert!(matches!( + compute_hash(&changed), + Err(AuditError::UnsupportedHashVersion { version: 3 }) + )); +} + +#[test] +fn legacy_identifiers_have_unambiguous_boundaries() { + let mut row = entry(); + row.hash_version = 1; + for id in [ + Some("a".repeat(64)), + Some(uuid::Uuid::nil().to_string()), + None, + ] { + row.object_id = id; + assert!(compute_hash(&row).is_ok()); + } + for id in [ + "".to_string(), + "a".repeat(63), + "A".repeat(64), + "record1".into(), + ] { + row.object_id = Some(id); + assert!(compute_hash(&row).is_err()); + } +} + +#[test] +fn stored_hash_and_actor_widths_are_checked() { + for version in [1, CURRENT_HASH_VERSION] { + for length in [0, 31, 33] { + let mut row = entry(); + row.hash_version = version; + row.actor_pubkey = Some(vec![1; length]); + assert!(compute_hash(&row).is_err()); + row.actor_pubkey = None; + row.seq = 2; + row.prev_hash = Some(vec![1; length]); + assert!(compute_hash(&row).is_err()); + } + } +} diff --git a/crates/buzz-audit/src/service.rs b/crates/buzz-audit/src/service.rs index 6819fe23ca3..144c134385e 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. @@ -117,6 +117,7 @@ impl AuditService { let created_at: DateTime = log_timestamp(); let mut audit_entry = AuditEntry { + hash_version: CURRENT_HASH_VERSION, community_id, seq, hash: Vec::new(), @@ -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?; @@ -174,7 +176,7 @@ impl AuditService { ) -> Result { let rows = sqlx::query( r#" - SELECT community_id, seq, hash, prev_hash, action, actor_pubkey, + SELECT hash_version, community_id, seq, hash, prev_hash, action, actor_pubkey, object_id, detail, created_at FROM audit_log WHERE community_id = $1 AND seq BETWEEN $2 AND $3 @@ -230,7 +232,7 @@ impl AuditService { ) -> Result, AuditError> { let rows = sqlx::query( r#" - SELECT community_id, seq, hash, prev_hash, action, actor_pubkey, + SELECT hash_version, community_id, seq, hash, prev_hash, action, actor_pubkey, object_id, detail, created_at FROM audit_log WHERE community_id = $1 AND seq >= $2 @@ -256,6 +258,7 @@ fn row_to_audit_entry(row: &sqlx::postgres::PgRow) -> Result("community_id"), seq: row.get("seq"), hash: row.get("hash"), @@ -345,6 +348,7 @@ mod postgres_tests { assert_eq!(e.seq, 1, "first entry in a community starts at seq 1"); assert!(e.prev_hash.is_none(), "genesis entry has NULL prev_hash"); assert_eq!(e.hash.len(), 32); + assert_eq!(e.hash_version, CURRENT_HASH_VERSION); assert_eq!(e.community_id, c); } @@ -503,8 +507,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, NOW(), 2)", ) .bind(b) .bind(&a1.hash) // A's hash, which was computed over community_id = A @@ -537,4 +541,46 @@ mod postgres_tests { .await .unwrap()); } + + #[tokio::test] + #[ignore = "requires Postgres"] + async fn legacy_chain_continues_with_framed_entries_and_rejects_boundary_moves() { + let pool = test_pool().await.expect("test Postgres must be available"); + let svc = AuditService::new(pool.clone()); + let community = make_community(&pool).await; + let scope = CommunityId::from_uuid(community); + let mut legacy = new_entry(community, AuditAction::EventCreated); + legacy.object_id = Some("a".repeat(64)); + let mut row = svc.log(legacy).await.unwrap(); + row.hash_version = 1; + row.hash = compute_hash(&row).unwrap().to_vec(); + sqlx::query( + "UPDATE audit_log SET hash_version = 1, hash = $1 WHERE community_id = $2 AND seq = 1", + ) + .bind(&row.hash) + .bind(community) + .execute(&pool) + .await + .unwrap(); + + let mut input = new_entry(community, AuditAction::EventCreated); + input.object_id = Some("record1".into()); + input.detail = serde_json::json!(23); + let next = svc.log(input).await.unwrap(); + assert_eq!(next.hash_version, CURRENT_HASH_VERSION); + assert_eq!(next.prev_hash, Some(row.hash)); + assert!(svc.verify_chain(scope, 1, 2).await.unwrap()); + let rows = svc.get_entries(scope, 1, 2).await.unwrap(); + assert_eq!( + rows.iter().map(|e| e.hash_version).collect::>(), + [1, 2] + ); + + sqlx::query("UPDATE audit_log SET object_id = 'record12', detail = '3'::jsonb WHERE community_id = $1 AND seq = 2") + .bind(community).execute(&pool).await.unwrap(); + assert!(matches!( + svc.verify_chain(scope, 1, 2).await, + Err(AuditError::HashMismatch { seq: 2 }) + )); + } } diff --git a/crates/buzz-db/src/runtime/migration.rs b/crates/buzz-db/src/runtime/migration.rs index 59015125042..7bf04edbdab 100644 --- a/crates/buzz-db/src/runtime/migration.rs +++ b/crates/buzz-db/src/runtime/migration.rs @@ -702,7 +702,8 @@ mod postgres_tests { let mut migrations: Vec<_> = MIGRATOR.iter().collect(); migrations.sort_by_key(|migration| migration.version); - assert_eq!(migrations.len(), 44); + assert_eq!(migrations.len(), 45); + assert_eq!(migrations[44].version, 45); assert_eq!(migrations[0].version, 1); assert_eq!(&*migrations[0].description, "initial schema"); assert!(migrations[0] @@ -2665,6 +2666,51 @@ mod postgres_tests { .expect("drop late-table fixtures"); } + #[tokio::test] + #[ignore = "requires Postgres"] + async fn migration_0045_preserves_legacy_hashes_and_writer_defaults() { + let pool = PgPool::connect(&crate::test_support::database_url()) + .await + .unwrap(); + reset_public_schema(&pool).await; + run_migrations_through(&pool, 44).await.unwrap(); + let community = uuid::Uuid::new_v4(); + sqlx::query("INSERT INTO communities (id, host) VALUES ($1, $2)") + .bind(community) + .bind(format!("audit-version-{community}.example")) + .execute(&pool) + .await + .unwrap(); + sqlx::query("INSERT INTO audit_log (community_id, seq, hash, action, detail) VALUES ($1, 1, $2, 'event_created', '{}')") + .bind(community).bind(vec![0xab_u8; 32]).execute(&pool).await.unwrap(); + run_migrations_through(&pool, 45).await.unwrap(); + let (version, hash): (i16, Vec) = sqlx::query_as( + "SELECT hash_version, hash FROM audit_log WHERE community_id = $1 AND seq = 1", + ) + .bind(community) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(version, 1); + assert_eq!(hash, vec![0xab_u8; 32]); + let old_writer_version: i16 = sqlx::query_scalar("INSERT INTO audit_log (community_id, seq, hash, action, detail) VALUES ($1, 2, $2, 'event_created', '{}') RETURNING hash_version") + .bind(community).bind(vec![0xcd_u8; 32]).fetch_one(&pool).await.unwrap(); + assert_eq!(old_writer_version, 1); + for version in [2_i16, 3] { + let result = sqlx::query("INSERT INTO audit_log (community_id, seq, hash, action, detail, hash_version) VALUES ($1, $2, $3, 'event_created', '{}', $4)") + .bind(community).bind(i64::from(version) + 1).bind(vec![0xef_u8; 32]).bind(version).execute(&pool).await; + if version == 2 { + result.unwrap(); + } else { + assert!(result + .unwrap_err() + .as_database_error() + .unwrap() + .is_check_violation()); + } + } + } + /// Verify migration 0044 applies cleanly against a DB that has rows in /// the NIP-FI 0041+0042 tables. The immutability guards (no_delete, /// no_truncate) are enforced via triggers; DROP TABLE bypasses them and diff --git a/crates/buzz-db/src/store/deletion.rs b/crates/buzz-db/src/store/deletion.rs index 0e184e00d88..4de618e0602 100644 --- a/crates/buzz-db/src/store/deletion.rs +++ b/crates/buzz-db/src/store/deletion.rs @@ -442,7 +442,8 @@ type TaxonomySweepRow = ( /// The executor's prefix enumeration and the destructive freeze's chunk /// validation both fold entries through this, so "the chunk rows are exactly /// the frozen enumeration" reduces to digest equality. Each entry is hashed -/// with a trailing newline so concatenation cannot alias two streams. +/// with a trailing newline; embedded newlines are rejected so concatenation +/// cannot alias two streams, including legacy manifests with raw keys. pub struct KeyStreamDigest { hasher: Sha256, last: Option, @@ -485,6 +486,11 @@ impl KeyStreamDigest { /// require strictly ascending serialized entries. Digest equality still /// binds the exact stream that was listed and chunked. pub fn fold_unordered(&mut self, key: &str) -> Result<()> { + if key.contains('\n') { + return Err(DbError::DeletionSafety( + "storage manifest entry contains the stream newline delimiter".to_string(), + )); + } self.hasher.update(key.as_bytes()); self.hasher.update(b"\n"); self.last = Some(key.to_owned()); @@ -3308,6 +3314,47 @@ mod tests { assert!(duplicate.fold("a").is_err()); } + #[test] + fn key_stream_rejects_newlines_without_changing_state() { + for ordered in [true, false] { + let mut digest = KeyStreamDigest::new(); + digest.fold("a").unwrap(); + let error = if ordered { + digest.fold("z\nz") + } else { + digest.fold_unordered("z\nz") + }; + assert!(error.is_err()); + // A failed fold must neither hash bytes, increment the count, nor + // advance `last` and reject the next otherwise valid key. + digest.fold("b").unwrap(); + assert_eq!(digest.finish(), (hex::encode(Sha256::digest(b"a\nb\n")), 2)); + } + } + + #[test] + fn legacy_chunks_reject_equal_count_newline_collision() { + let first = vec!["_meta/c/a".to_string(), "_meta/c/b\n_meta/c/c".to_string()]; + let second = vec!["_meta/c/a\n_meta/c/b".to_string(), "_meta/c/c".to_string()]; + let old_bytes = |keys: &[String]| format!("{}\n", keys.join("\n")); + assert_eq!(old_bytes(&first), old_bytes(&second)); + assert_eq!(first.len(), second.len()); + let mut manifest = storage_manifest(); + manifest.version = 4; + manifest.prefixes[0].object_count = 2; + manifest.prefixes[0].keys_digest = hex::encode(Sha256::digest(old_bytes(&first))); + for keys in [first, second] { + assert!(keys[0] < keys[1]); + let result = validate_manifest_key_chunks( + &manifest, + &[(0, "_meta/c/".to_string(), sqlx::types::Json(keys))], + ); + assert!( + matches!(result, Err(DbError::DeletionSafety(message)) if message.contains("newline delimiter")) + ); + } + } + #[test] fn manifest_key_chunks_must_hash_to_the_frozen_summaries() { let keys = vec!["_meta/c/1".to_string(), "_meta/c/2".to_string()]; diff --git a/crates/git-sign-nostr/README.md b/crates/git-sign-nostr/README.md index 908682fd7fe..fefd1bc0cfb 100644 --- a/crates/git-sign-nostr/README.md +++ b/crates/git-sign-nostr/README.md @@ -44,3 +44,16 @@ Git invokes this program as a signing/verification backend: payload from stdin, verifies signature from file, status lines to fd 1 (stdout) See [NIP-GS](../../docs/nips/NIP-GS.md) for the full specification. + +### Signature encoding upgrade + +New signatures use NIP-GS version 2, with an attestation presence byte and +length-prefixed fields. Upgrade verifiers before enabling the new signer; +older binaries reject version 2. Existing version 1 commit and tag signatures +remain verifiable. Legacy signatures over arbitrary payloads are rejected +because their optional attestation can otherwise be moved into the payload +without changing the signed bytes. See [NIP-GS](../../docs/nips/NIP-GS.md#signing-hash). + +Legacy verification assumes the original signer received a Git object. It +cannot recover the intended attestation boundary of historical version 1 +signatures produced over arbitrary bytes. diff --git a/crates/git-sign-nostr/src/lib.rs b/crates/git-sign-nostr/src/lib.rs index d316711200b..f389de8d74d 100644 --- a/crates/git-sign-nostr/src/lib.rs +++ b/crates/git-sign-nostr/src/lib.rs @@ -109,7 +109,7 @@ impl Drop for KeypairGuard { } } -const DOMAIN_SEPARATOR: &str = "nostr:git:v1:"; +const SIGNATURE_VERSION: u64 = 2; const ARMOR_BEGIN: &str = "-----BEGIN SIGNED MESSAGE-----"; const ARMOR_END: &str = "-----END SIGNED MESSAGE-----"; @@ -883,7 +883,63 @@ fn read_keyfile_secure(path: &str) -> Result, Error> Ok(trimmed) } -/// Compute the NIP-GS signing hash. +/// Bind the timestamp, attestation presence, and each variable-length field. +fn compute_signing_hash( + timestamp: u64, + oa: Option<&(String, String, String)>, + payload: &[u8], +) -> [u8; 32] { + let mut engine = Sha256Hash::engine(); + engine.input(b"nostr:git:v2:"); + engine.input(×tamp.to_be_bytes()); + engine.input(&[u8::from(oa.is_some())]); + if let Some((owner, conditions, sig)) = oa { + for field in [owner, conditions, sig] { + engine.input(&(field.len() as u64).to_be_bytes()); + engine.input(field.as_bytes()); + } + } + engine.input(&(payload.len() as u64).to_be_bytes()); + engine.input(payload); + Sha256Hash::from_engine(engine).to_byte_array() +} + +fn verification_hash(envelope: &Envelope, payload: &[u8]) -> Result<[u8; 32], String> { + match envelope.version { + 1 => { + // A legacy attestation starts with a hex public key. Requiring a + // Git object header makes it impossible to move that attestation + // into an unattested payload while keeping the same signed bytes. + let first_line = payload.split(|b| *b == b'\n').next().unwrap_or_default(); + let oid = first_line + .strip_prefix(b"tree ") + .or_else(|| first_line.strip_prefix(b"object ")); + if !payload.contains(&b'\n') + || !oid.is_some_and(|oid| { + matches!(oid.len(), 40 | 64) + && oid + .iter() + .all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(b)) + }) + { + return Err("legacy signatures require a Git commit or tag header".into()); + } + Ok(compute_legacy_signing_hash( + envelope.t, + envelope.oa.as_ref(), + payload, + )) + } + SIGNATURE_VERSION => Ok(compute_signing_hash( + envelope.t, + envelope.oa.as_ref(), + payload, + )), + version => Err(format!("unsupported version: {version}")), + } +} + +/// Compute the legacy NIP-GS signing hash. /// /// ```text /// hash = SHA-256("nostr:git:v1:" || decimal(t) || ":" || oa_binding || payload) @@ -892,13 +948,13 @@ fn read_keyfile_secure(path: &str) -> Result, Error> /// Where `oa_binding` is: /// - If oa present: `oa[0] || ":" || oa[1] || ":" || oa[2] || ":"` /// - If oa absent: empty (zero bytes) -fn compute_signing_hash( +fn compute_legacy_signing_hash( timestamp: u64, oa: Option<&(String, String, String)>, payload: &[u8], ) -> [u8; 32] { let mut engine = Sha256Hash::engine(); - engine.input(DOMAIN_SEPARATOR.as_bytes()); + engine.input(b"nostr:git:v1:"); engine.input(timestamp.to_string().as_bytes()); engine.input(b":"); @@ -921,15 +977,21 @@ fn compute_signing_hash( /// is `v, pk, sig, t[, oa]`, no whitespace, no trailing commas. We use /// `format!` rather than serde to guarantee this exact byte layout — serde's /// serialization order depends on the `Map` implementation and feature flags. -fn build_envelope(pk: &str, sig: &str, t: u64, oa: Option<&(String, String, String)>) -> String { +fn build_envelope( + version: u64, + pk: &str, + sig: &str, + t: u64, + oa: Option<&(String, String, String)>, +) -> String { match oa { Some((owner, conditions, owner_sig)) => { format!( - r#"{{"v":1,"pk":"{pk}","sig":"{sig}","t":{t},"oa":["{owner}","{conditions}","{owner_sig}"]}}"# + r#"{{"v":{version},"pk":"{pk}","sig":"{sig}","t":{t},"oa":["{owner}","{conditions}","{owner_sig}"]}}"# ) } None => { - format!(r#"{{"v":1,"pk":"{pk}","sig":"{sig}","t":{t}}}"#) + format!(r#"{{"v":{version},"pk":"{pk}","sig":"{sig}","t":{t}}}"#) } } } @@ -1059,7 +1121,7 @@ fn do_sign(key_id: &str, status: &mut StatusWriter) -> Result<(), Error> { drop(keypair); // Build envelope and armor - let json = build_envelope(&pk_hex, &sig_hex, t, oa.as_ref()); + let json = build_envelope(SIGNATURE_VERSION, &pk_hex, &sig_hex, t, oa.as_ref()); let armored = armor(json.as_bytes()); // Write signature to stdout — errors are fatal because git reads @@ -1174,6 +1236,7 @@ fn do_verify(sig_file: &str, status: &mut StatusWriter) -> Result<(), Error> { // Canonical JSON reconstruction check — ensures no field reordering or // extra whitespace was present in the original. let reconstructed = build_envelope( + envelope.version, &envelope.pk, &envelope.sig, envelope.t, @@ -1203,7 +1266,13 @@ fn do_verify(sig_file: &str, status: &mut StatusWriter) -> Result<(), Error> { })?; // Compute signing hash - let hash = compute_signing_hash(envelope.t, envelope.oa.as_ref(), &payload); + let hash = verification_hash(&envelope, &payload).map_err(|msg| { + write_errsig(status, Some(&envelope.pk)); + Error::VerifyFailed { + pk: Some(envelope.pk.clone()), + msg, + } + })?; let message = Message::from_digest(hash); // Parse signature @@ -1336,6 +1405,7 @@ fn do_verify(sig_file: &str, status: &mut StatusWriter) -> Result<(), Error> { #[derive(Debug)] struct Envelope { + version: u64, pk: String, sig: String, t: u64, @@ -1348,21 +1418,21 @@ fn parse_envelope(json_str: &str) -> Result { let obj = val.as_object().ok_or("JSON must be an object")?; - // Reject unknown keys — v=1 envelope allows only: v, pk, sig, t, oa + // Both supported versions allow only: v, pk, sig, t, oa let allowed = ["v", "pk", "sig", "t", "oa"]; for key in obj.keys() { if !allowed.contains(&key.as_str()) { - return Err(format!("unknown key in v=1 envelope: {key:?}")); + return Err(format!("unknown key in envelope: {key:?}")); } } - // v (required, must be 1) + // v (required; never fall back to an older hash on verification failure) let v = obj .get("v") .ok_or("missing required field: v")? .as_u64() .ok_or("v must be an integer")?; - if v != 1 { + if !matches!(v, 1 | SIGNATURE_VERSION) { return Err(format!("unsupported version: {v}")); } @@ -1421,6 +1491,7 @@ fn parse_envelope(json_str: &str) -> Result { // Validate oa[0] is a valid BIP-340 x-only public key (not just hex) PublicKey::from_hex(owner) + .and_then(|pk| pk.xonly()) .map_err(|e| format!("oa[0] is not a valid BIP-340 public key: {e}"))?; // Self-attestation is meaningless — owner must differ from signer @@ -1438,6 +1509,7 @@ fn parse_envelope(json_str: &str) -> Result { }; Ok(Envelope { + version: v, pk: pk.to_string(), sig: sig.to_string(), t, @@ -1808,9 +1880,13 @@ Initial commit" #[test] fn test_signing_hash_matches_spec() { // From NIP-GS spec: SHA-256 of preimage with t=1700000000, no oa - let hash = compute_signing_hash(1700000000, None, &test_payload()); + let hash = compute_legacy_signing_hash(1700000000, None, &test_payload()); let expected = "a11a32173aa35125aaefaad8854f2eda5a144268a4a355905c841f79ff44aa18"; assert_eq!(hex::encode(hash), expected); + assert_eq!( + hex::encode(compute_signing_hash(1700000000, None, &test_payload())), + "0f3a88b1c5eb1a13bcbcc30d33bfc95589182b7b7b7a548f40427e8271335fbd" + ); } #[test] @@ -1821,17 +1897,27 @@ Initial commit" "".to_string(), "54b97dfd2b7d61c1bc1b5facab9d12a991fe0ac3dcb9044b3176f63bebb6f67340eb0ad866f2d5568b78b58ba234ee9f490f8c41e64a949c200315801520ed25".to_string(), ); - let hash = compute_signing_hash(1700000000, Some(&oa), &test_payload()); + let hash = compute_legacy_signing_hash(1700000000, Some(&oa), &test_payload()); let expected = "b61f1658836a4f63a2d2f5d621014a064435dde0765dd9c1dc79c9530fe879f0"; assert_eq!(hex::encode(hash), expected); + assert_eq!( + hex::encode(compute_signing_hash(1700000000, Some(&oa), &test_payload())), + "80d4e5e24736147be9ed7c89e122e96eaef83df09ec3725b8d813f3073ea2f71" + ); } #[test] fn test_canonical_json_no_oa() { - let json = build_envelope(TEST_PK, &"a".repeat(128), 1700000000, None); + let json = build_envelope( + SIGNATURE_VERSION, + TEST_PK, + &"a".repeat(128), + 1700000000, + None, + ); // Must be compact (no whitespace), field order: v, pk, sig, t assert!(!json.contains(' ')); - assert!(json.starts_with(r#"{"v":1,"pk":""#)); + assert!(json.starts_with(r#"{"v":2,"pk":""#)); assert!(json.contains(r#","t":1700000000}"#)); assert!(!json.contains("oa")); } @@ -1843,7 +1929,13 @@ Initial commit" "".to_string(), "b".repeat(128), ); - let json = build_envelope(TEST_PK, &"a".repeat(128), 1700000000, Some(&oa)); + let json = build_envelope( + SIGNATURE_VERSION, + TEST_PK, + &"a".repeat(128), + 1700000000, + Some(&oa), + ); // Field order: v, pk, sig, t, oa assert!(json.contains(r#","oa":["#)); let v_pos = json.find(r#""v""#).unwrap(); @@ -2018,7 +2110,7 @@ Initial commit" let message = Message::from_digest(hash); let sig = SECP256K1.sign_schnorr(&message, &keypair); let sig_hex = hex::encode(sig.serialize()); - let json = build_envelope(&pk_hex, &sig_hex, t, None); + let json = build_envelope(SIGNATURE_VERSION, &pk_hex, &sig_hex, t, None); armor(json.as_bytes()) } @@ -2031,6 +2123,7 @@ Initial commit" let json_str = std::str::from_utf8(&decoded).map_err(|e| format!("utf8: {e}"))?; let envelope = parse_envelope(json_str)?; let reconstructed = build_envelope( + envelope.version, &envelope.pk, &envelope.sig, envelope.t, @@ -2040,7 +2133,7 @@ Initial commit" return Err("non-canonical JSON".to_string()); } let pk = PublicKey::from_hex(&envelope.pk).map_err(|e| format!("invalid pk: {e}"))?; - let hash = compute_signing_hash(envelope.t, envelope.oa.as_ref(), payload); + let hash = verification_hash(&envelope, payload)?; let message = Message::from_digest(hash); let sig_bytes = hex::decode(&envelope.sig).map_err(|_| "bad sig hex")?; let sig = Signature::from_slice(&sig_bytes).map_err(|_| "bad sig")?; @@ -2372,9 +2465,9 @@ Initial commit" } #[test] - fn test_envelope_rejects_v_not_1() { + fn test_envelope_rejects_unsupported_version() { let json = format!( - r#"{{"v":2,"pk":"{pk}","sig":"{sig}","t":1700000000}}"#, + r#"{{"v":3,"pk":"{pk}","sig":"{sig}","t":1700000000}}"#, pk = valid_pk(), sig = valid_sig(), ); @@ -2434,7 +2527,7 @@ Initial commit" fn test_canonical_json_roundtrip() { let json = valid_envelope_json(); let env = parse_envelope(&json).unwrap(); - let rebuilt = build_envelope(&env.pk, &env.sig, env.t, env.oa.as_ref()); + let rebuilt = build_envelope(env.version, &env.pk, &env.sig, env.t, env.oa.as_ref()); assert_eq!(rebuilt.as_bytes(), json.as_bytes()); } diff --git a/crates/git-sign-nostr/tests/hash_encoding.rs b/crates/git-sign-nostr/tests/hash_encoding.rs new file mode 100644 index 00000000000..9778b47bf84 --- /dev/null +++ b/crates/git-sign-nostr/tests/hash_encoding.rs @@ -0,0 +1,221 @@ +#![cfg(unix)] + +use std::{ + fs, + io::Write, + path::PathBuf, + process::{Command, Output, Stdio}, +}; + +use base64::{engine::general_purpose::STANDARD, Engine}; +use nostr::{ + hashes::{sha256, Hash}, + secp256k1::{Keypair, Message}, + SECP256K1, +}; +use serde_json::Value; + +const SECRET: &str = "0000000000000000000000000000000000000000000000000000000000000003"; +const SIGNER: &str = "f9308a019258c31049344f85f89d5229b531c845836f99b08601f113bce036f9"; +const OWNER: &str = "79be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798"; +const OA_SIG: &str = "54b97dfd2b7d61c1bc1b5facab9d12a991fe0ac3dcb9044b3176f63bebb6f67340eb0ad866f2d5568b78b58ba234ee9f490f8c41e64a949c200315801520ed25"; +const PAYLOAD: &[u8] = b"tree 4b825dc642cb6eb9a060e54bf899d69f7cb46101\nauthor Test 1700000000 +0000\ncommitter Test 1700000000 +0000\n\nTest\n"; + +struct Harness(PathBuf); + +impl Harness { + fn new() -> Self { + let path = std::env::temp_dir().join(format!( + "buzz-git-encoding-{}", + nostr::Keys::generate().public_key().to_hex() + )); + fs::create_dir(&path).unwrap(); + Self(path) + } + + fn command(&self, program: &str) -> Command { + let mut cmd = Command::new(program); + cmd.current_dir(&self.0) + .env_remove("NOSTR_PRIVATE_KEY") + .env_remove("BUZZ_PRIVATE_KEY") + .env_remove("BUZZ_AUTH_TAG") + .env_remove("GIT_DIR") + .env_remove("GIT_WORK_TREE") + .env("GIT_CONFIG_NOSYSTEM", "1") + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_COUNT", "0"); + cmd + } + + fn verify(&self, json: &str, payload: &[u8]) -> Output { + let sigfile = self.0.join("sig"); + fs::write( + &sigfile, + format!( + "-----BEGIN SIGNED MESSAGE-----\n{}\n-----END SIGNED MESSAGE-----\n", + STANDARD.encode(json) + ), + ) + .unwrap(); + pipe( + self.command(env!("CARGO_BIN_EXE_git-sign-nostr")) + .args(["--status-fd=1", "--verify"]) + .arg(sigfile) + .arg("-"), + payload, + ) + } + + fn sign(&self, payload: &[u8]) -> String { + let output = pipe( + self.command(env!("CARGO_BIN_EXE_git-sign-nostr")) + .args(["--status-fd=2", "-bsau", SIGNER]) + .env("NOSTR_PRIVATE_KEY", SECRET) + .env( + "BUZZ_AUTH_TAG", + serde_json::json!(["auth", OWNER, "", OA_SIG]).to_string(), + ), + payload, + ); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let armor = String::from_utf8(output.stdout).unwrap(); + String::from_utf8(STANDARD.decode(armor.lines().nth(1).unwrap()).unwrap()).unwrap() + } +} + +impl Drop for Harness { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } +} + +fn pipe(cmd: &mut Command, payload: &[u8]) -> Output { + let mut child = cmd + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + child.stdin.take().unwrap().write_all(payload).unwrap(); + child.wait_with_output().unwrap() +} + +fn envelope(version: u64, sig: &str, t: u64, oa: bool) -> String { + let binding = if oa { + format!(r#","oa":["{OWNER}","","{OA_SIG}"]"#) + } else { + String::new() + }; + format!(r#"{{"v":{version},"pk":"{SIGNER}","sig":"{sig}","t":{t}{binding}}}"#) +} + +fn legacy_signature(payload: &[u8], oa: bool) -> String { + let binding = if oa { + format!("{OWNER}::{OA_SIG}:") + } else { + String::new() + }; + let preimage = [ + format!("nostr:git:v1:1700000000:{binding}").as_bytes(), + payload, + ] + .concat(); + let digest = sha256::Hash::hash(&preimage).to_byte_array(); + let key = Keypair::from_seckey_str(SECP256K1, SECRET).unwrap(); + let sig = SECP256K1.sign_schnorr(&Message::from_digest(digest), &key); + envelope(1, &sig.to_string(), 1700000000, oa) +} + +#[test] +fn attestation_cannot_move_into_payload_or_downgrade_version() { + let h = Harness::new(); + let json = h.sign(PAYLOAD); + let parsed: Value = serde_json::from_str(&json).unwrap(); + assert_eq!(parsed["v"], 2); + assert!(h.verify(&json, PAYLOAD).status.success()); + let sig = parsed["sig"].as_str().unwrap(); + let t = parsed["t"].as_u64().unwrap(); + let moved = [format!("{OWNER}::{OA_SIG}:").as_bytes(), PAYLOAD].concat(); + for (altered, payload) in [ + (envelope(2, sig, t, false), moved.as_slice()), + (envelope(2, sig, t, false), PAYLOAD), + (envelope(1, sig, t, true), PAYLOAD), + (envelope(3, sig, t, true), PAYLOAD), + ] { + let output = h.verify(&altered, payload); + assert!(!output.status.success()); + assert!(!String::from_utf8_lossy(&output.stdout).contains("GOODSIG")); + } +} + +#[test] +fn legacy_commits_and_tags_verify_but_prefixed_payloads_do_not() { + let h = Harness::new(); + for header in ["tree", "object"] { + for width in [40, 64] { + let payload = format!("{header} {}\n\nlegacy\n", "a".repeat(width)); + for oa in [false, true] { + assert!(h + .verify( + &legacy_signature(payload.as_bytes(), oa), + payload.as_bytes() + ) + .status + .success()); + } + } + } + let json = legacy_signature(PAYLOAD, true); + assert!(h.verify(&json, PAYLOAD).status.success()); + let parsed: Value = serde_json::from_str(&json).unwrap(); + let stripped = envelope(1, parsed["sig"].as_str().unwrap(), 1700000000, false); + let moved = [format!("{OWNER}::{OA_SIG}:").as_bytes(), PAYLOAD].concat(); + let output = h.verify(&stripped, &moved); + assert!(!output.status.success()); + assert!(String::from_utf8_lossy(&output.stderr).contains("legacy signatures require")); +} + +#[test] +fn git_creates_and_verifies_a_commit_and_tag() { + let h = Harness::new(); + for args in [ + vec!["init", "-q"], + vec!["config", "user.name", "Test"], + vec!["config", "user.email", "test@example.com"], + vec!["config", "user.signingkey", SIGNER], + vec!["config", "gpg.format", "x509"], + vec![ + "config", + "gpg.x509.program", + env!("CARGO_BIN_EXE_git-sign-nostr"), + ], + vec![ + "-c", + "core.hooksPath=/dev/null", + "commit", + "--allow-empty", + "-S", + "-m", + "Test", + ], + vec!["verify-commit", "HEAD"], + vec!["tag", "-s", "v-test", "-m", "Test tag"], + vec!["verify-tag", "v-test"], + ] { + let output = h + .command("git") + .args(&args) + .env("NOSTR_PRIVATE_KEY", SECRET) + .output() + .unwrap(); + assert!( + output.status.success(), + "{args:?}: {}", + String::from_utf8_lossy(&output.stderr) + ); + } +} diff --git a/docs/nips/NIP-GS.md b/docs/nips/NIP-GS.md index 7ef37bcc276..359fe0f91f7 100644 --- a/docs/nips/NIP-GS.md +++ b/docs/nips/NIP-GS.md @@ -114,7 +114,7 @@ The base64 content decodes to a JSON object: ```json { - "v": 1, + "v": 2, "pk": "", "sig": "", "t": , @@ -124,7 +124,7 @@ The base64 content decodes to a JSON object: | Field | Type | Required | Constraints | Description | |-------|---------|----------|-------------|-------------| -| `v` | integer | MUST | MUST be `1` | Schema version. | +| `v` | integer | MUST | `2` for new signatures; `1` for legacy verification | Hash encoding version. | | `pk` | string | MUST | Exactly 64 lowercase hex characters. MUST be a valid BIP-340 x-only public key (i.e., the x-coordinate of a point on the secp256k1 curve). | Signer's public key. | | `sig` | string | MUST | Exactly 128 lowercase hex characters. | BIP-340 Schnorr signature over the git object. | | `t` | integer | MUST | MUST be in the range 0 to 4294967295. MUST NOT be negative, a float, or a string. | Claimed unix timestamp (seconds) of the signing event. See Security Considerations for implications of signer-controlled timestamps. | @@ -142,8 +142,8 @@ JSON parsing rules: - Duplicate keys: verifiers MUST reject the signature. Implementations SHOULD use a JSON parser configured to fault on duplicate keys, or verify key uniqueness before parsing. -- For `v=1`, the only permitted keys are `v`, `pk`, `sig`, `t`, and `oa`. - Any other key MUST cause rejection. Future versions (`v=2`, etc.) define +- For `v=1` and `v=2`, the only permitted keys are `v`, `pk`, `sig`, `t`, and `oa`. + Any other key MUST cause rejection. Future versions (`v=3`, etc.) define their own field sets. This prevents unsigned extension fields from being injected into the envelope. - The total decoded JSON MUST NOT exceed 2048 bytes (the `oa` field adds @@ -152,52 +152,55 @@ JSON parsing rules: ### Signing Hash -All envelope metadata (`t`, `oa`) is included in the hash preimage so that it -is cryptographically bound to the signature. Tampering with any field -invalidates the signature. +New signatures MUST use version 2. Each variable-length value is framed as +`field(x) = u64be(byte_length(x)) || x`. Strings use UTF-8; lengths count bytes. +The timestamp uses eight unsigned big-endian bytes. + +``` +hash = SHA-256("nostr:git:v2:" || u64be(t) || oa_binding || field(payload_bytes)) +``` + +If `oa` is absent, `oa_binding` is the single byte `0x00`. If it is present, +`oa_binding` is `0x01 || field(oa[0]) || field(oa[1]) || field(oa[2])`. +The public keys and signatures remain lowercase hex strings as in the envelope. +The presence byte distinguishes absence from an attestation, including one with +empty conditions. The lengths prevent bytes moving between fields or into the +payload. The version-specific domain separator prevents signature reuse across +versions and other Nostr protocols. + +#### Legacy version 1 verification -Given a git object payload (the bytes git pipes to stdin), a signing timestamp -`t`, and an optional owner attestation: +Version 1 used: ``` -hash = SHA-256( "nostr:git:v1:" || decimal(t) || ":" || oa_binding || payload_bytes ) +hash = SHA-256("nostr:git:v1:" || decimal(t) || ":" || oa_binding || payload_bytes) ``` -Where: -- `"nostr:git:v1:"` is the domain separator: exactly 13 bytes of UTF-8 - (`6e6f7374723a6769743a76313a`). -- `decimal(t)` is the ASCII decimal encoding of `t` with no leading zeroes - (except `0` itself). Example: `1700000000`. -- `":"` is a single colon byte (`3a`), separating the timestamp from the next - field. -- `oa_binding` is: - - If `oa` is present: `oa[0] || ":" || oa[1] || ":" || oa[2] || ":"` (the - three `oa` array elements concatenated with colon separators, followed by a - trailing colon). All elements are their exact string values (hex pubkey, - conditions string which may be empty, hex signature). - - If `oa` is absent: empty (zero bytes). The colon after `decimal(t)` is - immediately followed by `payload_bytes`. -- `payload_bytes` is the raw bytes git pipes to stdin. - -**Important:** Because the `oa` data is included in the signing hash, stripping -or modifying the `oa` field invalidates the NIP-GS `sig`. This is intentional — -the signature envelope is immutable once signed. - -The domain separator prevents cross-protocol signature reuse: -- NIP-01 event signatures sign `SHA-256(serialized_event)` — different preimage. -- NIP-98 HTTP auth signatures sign a kind:27235 event — different preimage. -- NIP-OA attestations sign `SHA-256("nostr:agent-auth:" || ...)` — different - domain separator. +Here `oa_binding` is empty when absent, or +`oa[0] || ":" || oa[1] || ":" || oa[2] || ":"` when present. This encoding +is ambiguous for arbitrary payloads: removing `oa` and prepending its binding +to the payload preserves the hash. + +Verifiers MAY accept legacy signatures only if the payload starts with +`tree \n` (commit) or `object \n` (tag), where `` is exactly +40 or 64 lowercase hex characters. These headers cannot begin with an +attestation's hex public key, so the two interpretations cannot both verify. +All envelope and attestation structural checks still apply. Other legacy +payloads MUST be rejected. Verifiers MUST select the hash by `v`, and MUST NOT +retry another version after failure. Existing valid commit and tag signatures +retain their original digests; version 1 verifiers need an upgrade to read new +version 2 signatures. + +Legacy acceptance assumes the original payload came from Git. The version 1 +signature alone cannot prove which interpretation an arbitrary-byte signer +intended before this upgrade. Version 2 binds that distinction explicitly. ### Signing Procedure 1. Record the current unix timestamp as `t`. 2. Read the git object payload from stdin. If the payload exceeds 100 MB, exit with code 1 and a diagnostic on stderr. MUST NOT write to stdout. -3. Compute the signing hash per the Signing Hash section: - `hash = SHA-256("nostr:git:v1:" || decimal(t) || ":" || oa_binding || payload)`. - If including `oa`, the `oa_binding` is `oa[0] || ":" || oa[1] || ":" || oa[2] || ":"`. - If not including `oa`, the `oa_binding` is empty (zero bytes). +3. Compute the version 2 signing hash per the Signing Hash section. 4. Produce a BIP-340 Schnorr signature over `hash` using the signer's secret key. Implementations MUST use a cryptographically secure nonce per BIP-340 §4. Implementations SHOULD use auxiliary randomness (BIP-340 §4 default @@ -206,8 +209,8 @@ The domain separator prevents cross-protocol signature reuse: reproducible test vectors. 5. Construct the JSON object with compact serialization (no whitespace). Field order MUST be `v`, `pk`, `sig`, `t`, then `oa` if present. - Example without `oa`: `{"v":1,"pk":"","sig":"","t":}` - Example with `oa`: `{"v":1,"pk":"","sig":"","t":,"oa":["","",""]}` + Example without `oa`: `{"v":2,"pk":"","sig":"","t":}` + Example with `oa`: `{"v":2,"pk":"","sig":"","t":,"oa":["","",""]}` 6. Base64-encode the JSON bytes (standard alphabet, with padding). 7. Write to stdout: ``` @@ -240,17 +243,14 @@ The domain separator prevents cross-protocol signature reuse: any given set of field values. 3. Validate all fields per the constraints table. If any field is invalid or missing, write `ERRSIG` (see below) and exit with code 1. -4. If `v` is not `1`, write `ERRSIG` and exit with code 1. +4. If `v` is neither `1` nor `2`, write `ERRSIG` and exit with code 1. 5. Validate that `pk` is a valid BIP-340 x-only public key (not just hex — the value must be the x-coordinate of a point on secp256k1, i.e., `lift_x(pk)` must succeed per BIP-340 §5.3.2). 6. Read the git object payload from stdin. If the payload exceeds 100 MB, write `ERRSIG` to the status fd and exit with code 1. -7. Compute the signing hash per the Signing Hash section. If the `oa` field is - present and structurally valid (array of 3 strings), include the oa_binding: - `hash = SHA-256("nostr:git:v1:" || decimal(t) || ":" || oa[0] || ":" || oa[1] || ":" || oa[2] || ":" || payload)`. - If `oa` is absent: - `hash = SHA-256("nostr:git:v1:" || decimal(t) || ":" || payload)`. +7. Select the signing hash by `v` per the Signing Hash section. For version 1, + enforce the legacy payload header restriction before computing the hash. 8. Verify the BIP-340 Schnorr signature `sig` over `hash` against public key `pk`. 9. If verification fails, write to the status fd: @@ -523,6 +523,14 @@ absence of `SIG_CREATED` as a signing failure. ## Test Vectors +The detailed signatures below are retained as legacy version 1 vectors. For +version 2, the same payload and timestamp (`1700000000`) produce these hashes: + +- Without `oa`: `0f3a88b1c5eb1a13bcbcc30d33bfc95589182b7b7b7a548f40427e8271335fbd` +- With the owner attestation below: `80d4e5e24736147be9ed7c89e122e96eaef83df09ec3725b8d813f3073ea2f71` + +### Legacy version 1 vectors + ### Test Key ``` @@ -723,7 +731,7 @@ Implementations MUST handle the following: ### Domain Separation -The `nostr:git:v1:` prefix in the hash preimage ensures that a signature over a +The version-specific `nostr:git:v2:` prefix in the hash preimage ensures that a signature over a git object cannot be replayed in another context. The timestamp is included in the preimage so that `t` is cryptographically bound — tampering with `t` invalidates the signature. diff --git a/migrations/0045_audit_hash_version.sql b/migrations/0045_audit_hash_version.sql new file mode 100644 index 00000000000..e4206d345e2 --- /dev/null +++ b/migrations/0045_audit_hash_version.sql @@ -0,0 +1,5 @@ +-- Existing rows and older writers retain version 1. New writers explicitly +-- select version 2; no historical hashes or links are rewritten. +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 09508125622..9fe8525bb20 100644 --- a/schema/schema.sql +++ b/schema/schema.sql @@ -643,6 +643,7 @@ CREATE TABLE archived_identities ( -- (Lane Audit/Dawn builds the chain logic; Lane 0 fixes the scoped schema.) CREATE TABLE audit_log ( + hash_version SMALLINT NOT NULL DEFAULT 1 CHECK (hash_version IN (1, 2)), community_id UUID NOT NULL REFERENCES communities(id), seq BIGINT NOT NULL, hash BYTEA NOT NULL, diff --git a/scripts/run-tests.sh b/scripts/run-tests.sh index 6a14fe0cfee..7c3579bcfcd 100755 --- a/scripts/run-tests.sh +++ b/scripts/run-tests.sh @@ -81,6 +81,12 @@ 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 "git-sign-nostr tests" \ + cargo test -p git-sign-nostr -- --nocapture + run_test_step "buzz-auth unit tests" \ cargo test -p buzz-auth --lib -- --nocapture