From 8e04a1e2a649836c1fcff2434738e1c9a5e90a07 Mon Sep 17 00:00:00 2001 From: EdisonJwa Date: Fri, 15 May 2026 21:25:29 +0800 Subject: [PATCH] fix(storage): file is durable DEK source; keyring is accelerator only Surfaced on the v1.0.0-rc.7 Windows verification round as 'Bridge Error connection failed: storage crypto decrypt aead error'. Root cause: IdentityFileStore::ensure_dek treated the platform keyring as the authoritative store and deleted identity.dek after successfully promoting it. Subsequent launches whose process context could not reach the keyring (Windows SSH session hits ERROR_NO_SUCH_LOGON_SESSION; macOS LaunchAgent contexts hit errSecMissingEntitlement) saw dek_path.exists() = false and generated a fresh DEK, even though the keyring still held the DEK that originally encrypted identity.tskey. Next ChaCha20- Poly1305 AEAD decrypt of the identity blob then failed because the in-process DEK was 32 fresh random bytes, not the bytes that encrypted the stored ciphertext. The bookmark store (which shares the DEK via crypto()) also broke for the same reason. Manual reproduction on the rc.7 build at 100.84.219.45: * Launch via SSH (keyring unreachable) -> file DEK_v1 created, identity.tskey eventually encrypted under DEK_v1. * Launch via RDP (keyring reachable) -> file DEK_v1 promoted to keyring, identity.dek deleted. * Launch via SSH again (keyring unreachable, file gone) -> a fresh DEK_v2 is written to file. identity.tskey still encrypted under DEK_v1. * Next decrypt: DEK_v2 vs identity.tskey ciphertext -> AEAD tag mismatch -> StorageError::Crypto('decrypt: \u2026') -> bubble up as 'storage crypto decrypt aead error'. Fix invariants: * identity.dek (file) is the durable source of truth and is never deleted by ensure_dek. * keyring is opportunistic: we copy the DEK into it for the UX-level convenience of platform-managed secret storage, but its presence/absence does not affect correctness. * ensure_dek on first install writes the DEK to BOTH places. * ensure_dek on subsequent launches: keep using the file DEK; re-copy into the keyring if not present (idempotent). * load_dek prefers the file; only consults the keyring as a legacy-migration fallback for installs that lost their file mirror before this commit landed. No SDD / SAD / SRS contract changes - the file fallback at identity.dek and the in-keyring entry at app.chanora.identity:: identity-dek:: were both already documented behaviours; this commit corrects which one is authoritative. The file lives in app-private storage where the platform sandbox is the access-control authority (this was already called out in the existing open_private comment on non-Unix targets), so retaining the file mirror does not weaken the security posture in any meaningful way relative to the prior keyring-only durable-state design. Verified on Linux: cargo test --workspace 59/0/3 (no regressions from feceacf + 5c413ba). --- crates/chanora_storage/src/lib.rs | 54 ++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 18 deletions(-) diff --git a/crates/chanora_storage/src/lib.rs b/crates/chanora_storage/src/lib.rs index 0ca64d1..596c848 100644 --- a/crates/chanora_storage/src/lib.rs +++ b/crates/chanora_storage/src/lib.rs @@ -253,41 +253,59 @@ impl IdentityFileStore { return Ok(()); } // File-fallback DEK present? Try to migrate into the - // keyring opportunistically (lets users upgrade for free) - // and keep the file as the live source if migration fails. + // keyring opportunistically (lets users upgrade for free). + // We always keep the file around as a stable mirror — + // platform keyring access can flicker (e.g. SSH-launched + // session on Windows hits ERROR_NO_SUCH_LOGON_SESSION even + // though an RDP / console session reached the same store + // fine), and if a transient keyring outage made us + // regenerate the DEK we would silently break decrypt of + // every prior identity.tskey blob (reported on the v1.0.0 + // -rc.7 Windows verification run as "Bridge Error + // connection failed: storage crypto decrypt aead error"). + // The file lives in app-private storage where the platform + // sandbox is the access-control authority, so retaining + // the mirror does not weaken the security posture in any + // meaningful way. if self.dek_path.exists() { if let Ok(key) = read_file_dek(&self.dek_path) { - if self.keyring_save(&key) { - // Best-effort scrub: remove the file copy now - // that the keyring holds the authoritative value. - let _ = fs::remove_file(&self.dek_path); - } + let _ = self.keyring_save(&key); let mut k = key; k.zeroize(); } return Ok(()); } - // Fresh install: generate a new DEK and store it in the - // keyring if we can, else fall back to the file. + // Fresh install: generate a new DEK and persist to BOTH + // the keyring (when reachable) and the file. The file is + // the durable source of truth; the keyring is an optional + // accelerator that platform UX integrates with. let mut key = [0u8; 32]; OsRng.fill_bytes(&mut key); - if !self.keyring_save(&key) { - let mut f = open_private(&self.dek_path)?; - f.write_all(&key) - .map_err(|e| StorageError::Io(format!("write dek: {e}")))?; - f.sync_all() - .map_err(|e| StorageError::Io(format!("sync dek: {e}")))?; - info!(target: "chanora_storage", path = ?self.dek_path, "DEK generated (file fallback)"); - } + let _ = self.keyring_save(&key); + let mut f = open_private(&self.dek_path)?; + f.write_all(&key) + .map_err(|e| StorageError::Io(format!("write dek: {e}")))?; + f.sync_all() + .map_err(|e| StorageError::Io(format!("sync dek: {e}")))?; + info!(target: "chanora_storage", path = ?self.dek_path, "DEK generated (file fallback)"); key.zeroize(); Ok(()) } fn load_dek(&self) -> Result<[u8; 32], StorageError> { + // File is the durable source of truth (see `ensure_dek`). + // Prefer it when present; only consult the keyring as a + // legacy-migration path for installs that lost their file + // mirror before this commit landed. + if self.dek_path.exists() { + return read_file_dek(&self.dek_path); + } if let Some(k) = self.keyring_load()? { return Ok(k); } - read_file_dek(&self.dek_path) + Err(StorageError::Crypto( + "no DEK available (file missing, keyring empty)".into(), + )) } /// Hand out a [`DekCrypto`] anchored on the same DEK that