From ead23b2b3aeea215d45d474ee68bfb05fabfac78 Mon Sep 17 00:00:00 2001 From: willamhou Date: Wed, 24 Jun 2026 11:22:27 +0800 Subject: [PATCH] =?UTF-8?q?fix(security):=20audit=20polish=20=E2=80=94=20t?= =?UTF-8?q?ransport=20from=20base=5Furl,=20encrypted=20key,=20rotated=20cr?= =?UTF-8?q?ed-ref=20(M4/M6/L2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-G of the security-hardening plan. - M4: the receipt's Action.transport is derived from the base_url scheme (http vs https) instead of always claiming https. - M6: the audit signing key is encrypted at rest by default. AuditConfig carries a passphrase; the CLI requires RELAIS_AUDIT_PASSPHRASE and allows an unencrypted key only under RELAIS_AUDIT_ALLOW_UNENCRYPTED_KEY=1. - L2: credential_ref is bound to a salted, non-reversible fingerprint of the credential secrets, so rotating a credential yields a new opaque ref (empty for a no-credential call). The salt is local-only. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/cli/src/commands/audit.rs | 6 ++- crates/cli/src/commands/mod.rs | 28 ++++++++++ crates/core/src/audit/envelope.rs | 37 ++++++++++++- crates/core/src/audit/key.rs | 86 ++++++++++++++++++++++++++++--- crates/core/src/audit/mod.rs | 14 +++-- crates/core/src/router.rs | 1 + 6 files changed, 159 insertions(+), 13 deletions(-) diff --git a/crates/cli/src/commands/audit.rs b/crates/cli/src/commands/audit.rs index d2da58e..27d9c35 100644 --- a/crates/cli/src/commands/audit.rs +++ b/crates/cli/src/commands/audit.rs @@ -26,7 +26,8 @@ pub async fn run(action: AuditAction) -> Result<()> { let owner = owner .or_else(|| std::env::var("RELAIS_AUDIT_OWNER").ok()) .unwrap_or_else(|| "relais".into()); - let key = AuditKey::load_or_init(&dir, &owner, None) + let passphrase = super::audit_passphrase()?; + let key = AuditKey::load_or_init(&dir, &owner, passphrase.as_deref()) .map_err(|e| anyhow::anyhow!(e.to_string()))?; println!("audit key ready under {}", dir.join("keys").display()); println!("owner: {owner}"); @@ -38,7 +39,8 @@ pub async fn run(action: AuditAction) -> Result<()> { Ok(()) } AuditAction::Pubkey => { - let key = AuditKey::load_or_init(&dir, "relais", None) + let passphrase = super::audit_passphrase()?; + let key = AuditKey::load_or_init(&dir, "relais", passphrase.as_deref()) .map_err(|e| anyhow::anyhow!(e.to_string()))?; println!("{}", key.pubkey); Ok(()) diff --git a/crates/cli/src/commands/mod.rs b/crates/cli/src/commands/mod.rs index c39ea8d..b2cadf7 100644 --- a/crates/cli/src/commands/mod.rs +++ b/crates/cli/src/commands/mod.rs @@ -9,6 +9,32 @@ pub mod vault; use relais_core::router::Router; +/// Resolve the audit signing-key passphrase (M6). Encrypted at rest by default via +/// `RELAIS_AUDIT_PASSPHRASE`; an unencrypted key is allowed only under +/// `RELAIS_AUDIT_ALLOW_UNENCRYPTED_KEY=1|true` (dev). +#[cfg(feature = "audit")] +pub fn audit_passphrase() -> anyhow::Result> { + if let Ok(p) = std::env::var("RELAIS_AUDIT_PASSPHRASE") { + if !p.is_empty() { + return Ok(Some(p)); + } + } + let allow_unencrypted = matches!( + std::env::var("RELAIS_AUDIT_ALLOW_UNENCRYPTED_KEY").as_deref(), + Ok("1") | Ok("true") + ); + if allow_unencrypted { + tracing::warn!( + "RELAIS_AUDIT_ALLOW_UNENCRYPTED_KEY is set — the audit signing key is stored UNENCRYPTED (DEV ONLY)" + ); + return Ok(None); + } + anyhow::bail!( + "RELAIS_AUDIT_PASSPHRASE is not set; refusing to store the audit signing key unencrypted. \ + Set a passphrase, or for local dev only: RELAIS_AUDIT_ALLOW_UNENCRYPTED_KEY=1" + ) +} + /// The signet/audit directory: `RELAIS_SIGNET_DIR` or `~/.relais/signet`. #[cfg(feature = "audit")] pub fn audit_dir() -> anyhow::Result { @@ -47,12 +73,14 @@ fn attach_audit(router: Router) -> anyhow::Result { }; let dir = audit_dir()?; let owner = std::env::var("RELAIS_AUDIT_OWNER").unwrap_or_else(|_| "relais".into()); + let passphrase = audit_passphrase()?; match AuditSink::new(AuditConfig { dir, owner, mode, capacity: 1024, ack_timeout: std::time::Duration::from_secs(5), + passphrase, }) { Ok(sink) => Ok(router.with_audit(sink)), Err(e) => match mode { diff --git a/crates/core/src/audit/envelope.rs b/crates/core/src/audit/envelope.rs index 8cbb0e2..151183a 100644 --- a/crates/core/src/audit/envelope.rs +++ b/crates/core/src/audit/envelope.rs @@ -84,7 +84,7 @@ pub fn build_action( params: request, params_hash: String::new(), target: base_url.to_string(), - transport: "https".to_string(), + transport: scheme_of(base_url).to_string(), session, call_id: Some(call_id), response_hash: None, @@ -93,6 +93,16 @@ pub fn build_action( } } +/// The transport scheme of `base_url` (`https`/`http`), so the receipt reflects the +/// real transport security rather than always claiming https (M4). +fn scheme_of(base_url: &str) -> &'static str { + if base_url.to_ascii_lowercase().starts_with("https://") { + "https" + } else { + "http" + } +} + /// Stable short identifier for an `AdapterError` variant (the `kind` in a failure /// envelope). fn error_kind(e: &AdapterError) -> &'static str { @@ -173,6 +183,31 @@ mod tests { assert!(a.params_hash.is_empty()); } + #[test] + fn transport_reflects_base_url_scheme() { + let r = Redactor::new(); + let ctx = ctx_with_token("t"); + let req = build_request(&ctx, &meta(), &r, &[]); + let http = build_action( + &ctx, + req.clone(), + "http://h:8501", + None, + "t".into(), + "c".into(), + ); + assert_eq!(http.transport, "http"); + let https = build_action( + &ctx, + req, + "https://api.example", + None, + "t".into(), + "c".into(), + ); + assert_eq!(https.transport, "https"); + } + #[test] fn response_envelope_ok_and_err() { let r = Redactor::new(); diff --git a/crates/core/src/audit/key.rs b/crates/core/src/audit/key.rs index e3042bb..18fe62f 100644 --- a/crates/core/src/audit/key.rs +++ b/crates/core/src/audit/key.rs @@ -78,17 +78,25 @@ impl AuditKey { /// What an opaque `credential_ref` resolves to, locally. This map is **never** /// serialized into a receipt/sidecar and is omitted from exports. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct CredBinding { pub site: String, + /// Non-reversible, salted fingerprint of the credential secrets, so a rotated + /// credential gets a *different* opaque ref (L2). Empty when no credential. + #[serde(default)] + pub cred_fp: String, } /// Persists `kref_… -> CredBinding` at `dir/credential_refs.json`. Minting is -/// idempotent per binding, so a credential keeps one stable opaque ref. +/// idempotent per binding, so a given credential keeps one stable opaque ref. #[derive(Debug, Default, Serialize, Deserialize)] pub struct CredRefStore { #[serde(skip)] path: PathBuf, + /// Local-only salt for credential fingerprints (never exported), so a fingerprint + /// isn't brute-forceable across hosts. + #[serde(default)] + salt: Vec, map: HashMap, } @@ -104,9 +112,29 @@ impl CredRefStore { CredRefStore::default() }; store.path = path; + if store.salt.is_empty() { + let s: [u8; 16] = rand::random(); + store.salt = s.to_vec(); + } Ok(store) } + /// Non-reversible, salted fingerprint of the credential secrets (L2). A rotated + /// credential (different secrets) yields a different fingerprint, hence a new ref. + pub fn fingerprint(&self, secrets: &[String]) -> String { + if secrets.is_empty() { + return String::new(); // no credential → empty fingerprint + } + use sha2::{Digest, Sha256}; + let mut h = Sha256::new(); + h.update(&self.salt); + for s in secrets { + h.update(s.as_bytes()); + h.update([0u8]); // length-independent separator + } + hex::encode(&h.finalize()[..8]) // 16 hex chars + } + /// Return the existing opaque ref for `binding`, or mint, persist and return a /// new one. Idempotent per binding. pub fn mint(&mut self, binding: CredBinding) -> Result { @@ -182,11 +210,22 @@ mod tests { fn mint_is_idempotent_per_binding_and_persists() { let dir = tempfile::tempdir().unwrap(); let mut store = CredRefStore::load(dir.path()).unwrap(); - let a1 = store.mint(CredBinding { site: "scs".into() }).unwrap(); - let a2 = store.mint(CredBinding { site: "scs".into() }).unwrap(); + let a1 = store + .mint(CredBinding { + site: "scs".into(), + ..Default::default() + }) + .unwrap(); + let a2 = store + .mint(CredBinding { + site: "scs".into(), + ..Default::default() + }) + .unwrap(); let b = store .mint(CredBinding { site: "github".into(), + ..Default::default() }) .unwrap(); assert_eq!(a1, a2, "same binding → same ref"); @@ -197,8 +236,38 @@ mod tests { let reloaded = CredRefStore::load(dir.path()).unwrap(); assert_eq!( reloaded.map.get(&a1), - Some(&CredBinding { site: "scs".into() }) + Some(&CredBinding { + site: "scs".into(), + ..Default::default() + }) + ); + } + + #[test] + fn fingerprint_rotates_the_ref() { + let dir = tempfile::tempdir().unwrap(); + let mut store = CredRefStore::load(dir.path()).unwrap(); + let fp1 = store.fingerprint(&["tok-v1".into()]); + let fp2 = store.fingerprint(&["tok-v2".into()]); + assert_ne!(fp1, fp2, "different secrets → different fingerprint"); + assert_eq!( + fp1, + store.fingerprint(&["tok-v1".into()]), + "stable per secret" ); + let r1 = store + .mint(CredBinding { + site: "s".into(), + cred_fp: fp1, + }) + .unwrap(); + let r2 = store + .mint(CredBinding { + site: "s".into(), + cred_fp: fp2, + }) + .unwrap(); + assert_ne!(r1, r2, "a rotated credential gets a new opaque ref"); } #[cfg(unix)] @@ -207,7 +276,12 @@ mod tests { use std::os::unix::fs::PermissionsExt; let dir = tempfile::tempdir().unwrap(); let mut store = CredRefStore::load(dir.path()).unwrap(); - store.mint(CredBinding { site: "scs".into() }).unwrap(); + store + .mint(CredBinding { + site: "scs".into(), + ..Default::default() + }) + .unwrap(); let mode = std::fs::metadata(dir.path().join("credential_refs.json")) .unwrap() .permissions() diff --git a/crates/core/src/audit/mod.rs b/crates/core/src/audit/mod.rs index 8dd5c6d..9b33eb9 100644 --- a/crates/core/src/audit/mod.rs +++ b/crates/core/src/audit/mod.rs @@ -71,6 +71,9 @@ pub struct AuditConfig { pub mode: AuditMode, pub capacity: usize, pub ack_timeout: Duration, + /// Passphrase to encrypt the signing key at rest (Argon2id + XChaCha20-Poly1305). + /// `None` stores it unencrypted (dev only) — see M6 / the CLI gate. + pub passphrase: Option, } /// The audit sink: owns the single writer task, the opaque credential-ref store and @@ -87,7 +90,7 @@ impl AuditSink { /// Build a sink: load-or-init the gateway key, load the credential-ref store and /// spawn the single writer task. Must be called within a tokio runtime. pub fn new(cfg: AuditConfig) -> Result { - let key = AuditKey::load_or_init(&cfg.dir, &cfg.owner, None)?; + let key = AuditKey::load_or_init(&cfg.dir, &cfg.owner, cfg.passphrase.as_deref())?; let credrefs = CredRefStore::load(&cfg.dir)?; let writer = spawn_writer(cfg.dir.clone(), key, cfg.capacity); Ok(Self { @@ -117,19 +120,22 @@ impl AuditSink { t0: DateTime, t1: DateTime, ) -> Result, AuditError> { + let secrets = secret_values_of(&ctx.credentials); + // Mint the opaque credential ref in a tight sync scope; never hold the guard - // across an await. + // across an await. Bind it to a salted fingerprint of the credential so a + // rotation produces a new ref (L2). let credential_ref = { let mut store = self .credrefs .lock() .map_err(|_| AuditError::Io("credential-ref store lock poisoned".into()))?; + let cred_fp = store.fingerprint(&secrets); store.mint(CredBinding { site: ctx.site.clone(), + cred_fp, })? }; - - let secrets = secret_values_of(&ctx.credentials); let meta = AuditMeta { auth_injection: describe_injection(&ctx.credentials), credential_ref, diff --git a/crates/core/src/router.rs b/crates/core/src/router.rs index f5b36c6..5442182 100644 --- a/crates/core/src/router.rs +++ b/crates/core/src/router.rs @@ -123,6 +123,7 @@ mod audit_tests { mode, capacity: 16, ack_timeout: Duration::from_secs(5), + passphrase: None, }) .unwrap() }