From 1e073e1f094969b4fee1794fcd00efdf0b21f069 Mon Sep 17 00:00:00 2001 From: samialabri Date: Sun, 18 Oct 2026 07:00:50 +0900 Subject: [PATCH] kv-wasm: forget the Secret Key on logout and on any failed unlock or login Co-Authored-By: Claude Opus 5.5 --- crates/kv-wasm/README.md | 6 +++-- crates/kv-wasm/src/core.rs | 46 ++++++++++++++++++++++++++------ crates/kv-wasm/src/wasm/tests.rs | 5 ++++ crates/kv-wasm/tests/sync.rs | 11 ++++++++ 4 files changed, 58 insertions(+), 10 deletions(-) diff --git a/crates/kv-wasm/README.md b/crates/kv-wasm/README.md index f6cc7f8..170652c 100644 --- a/crates/kv-wasm/README.md +++ b/crates/kv-wasm/README.md @@ -113,8 +113,10 @@ two replicas overwriting each other. the platform defaults to the OS from the user agent because the server has no `web` platform yet. 4. **Every unlock needs the master password and Secret Key.** There is no Local Unlock Key on the web: nothing secret is ever written to IndexedDB, so a reload, `lock()` or closing the tab - leaves the vault locked. (The Secret Key that setup codes carry is kept in wasm memory while - unlocked and dropped on `lock()`, unlike kv-ffi, which keeps a sealed copy on disk.) Lock on idle and on `visibilitychange`/`pagehide` as the UI sees fit; + leaves the vault locked. The Secret Key that setup codes carry is kept in wasm memory only, set + after an unlock/login proved it and dropped on `lock()`, `logout()` and any failed unlock or + login; no getter exposes it (unlike kv-ffi, which keeps a sealed copy on disk). See + kestrove-spec docs/10 ยง5 "Web vault". Lock on idle and on `visibilitychange`/`pagehide` as the UI sees fit; `close()` drops the worker and every key in it. 5. **Secrets in JS.** Master password, Secret Key and decrypted `ItemDto`s are JS strings the core cannot wipe. Do not keep them in long-lived state, logs or URLs. The core zeroizes its own diff --git a/crates/kv-wasm/src/core.rs b/crates/kv-wasm/src/core.rs index 544ac04..1d58644 100644 --- a/crates/kv-wasm/src/core.rs +++ b/crates/kv-wasm/src/core.rs @@ -79,8 +79,10 @@ struct Unlocked { vault_key: Arc, /// OPAQUE password, kept while unlocked so an expired session can be renewed without prompting. auth_input: Option>, - /// The Secret Key, for setup payloads (Emergency Kit, "Add a device", scan-to-join). - secret_key: Zeroizing<[u8; 16]>, + /// The Secret Key, for setup payloads (Emergency Kit, "Add a device", scan-to-join). Set only + /// once an unlock or login proved it (the keyset unwrapped); dropped on `lock`, `logout` and any + /// failed unlock or login. Never persisted in any form, and no getter exposes it. + secret_key: Option>, } struct Device { @@ -191,12 +193,16 @@ impl WebCore { keyset: Arc::new(keyset), vault_key: Arc::new(vault_key), auth_input: Some(derived.auth_input), - secret_key: Zeroizing::new(*secret_key.expose()), + secret_key: Some(Zeroizing::new(*secret_key.expose())), }); Ok(secret_key.to_display().to_string()) } pub fn unlock(&self, master_password: &str, secret_key: &str) -> Result<()> { + self.unlock_inner(master_password, secret_key).inspect_err(|_| self.forget_secret_key()) + } + + fn unlock_inner(&self, master_password: &str, secret_key: &str) -> Result<()> { let mut inner = self.lock_inner(); let acct = inner.account.clone().ok_or(CoreError::NoAccount)?; let sk = SecretKey::parse(secret_key)?; @@ -208,7 +214,7 @@ impl WebCore { keyset: Arc::new(keyset), vault_key: Arc::new(vault_key), auth_input: Some(derived.auth_input), - secret_key: Zeroizing::new(*sk.expose()), + secret_key: Some(Zeroizing::new(*sk.expose())), }); Ok(()) } @@ -496,7 +502,7 @@ impl WebCore { /// Logs in with the master password and Secret Key. A browser with no account fetches it from the /// server; call `sync_now` afterwards to download the vault. pub async fn login(&self, master_password: &str, secret_key: &str, totp: Option) -> Result { - let sk = SecretKey::parse(secret_key)?; + let sk = SecretKey::parse(secret_key).inspect_err(|_| self.forget_secret_key())?; self.login_inner(master_password, sk, totp, None, None).await } @@ -508,6 +514,18 @@ impl WebCore { totp: Option, ticket: Option, expect_account: Option, + ) -> Result { + let r = self.login_unchecked(master_password, sk, totp, ticket, expect_account).await; + r.inspect_err(|_| self.forget_secret_key()) + } + + async fn login_unchecked( + &self, + master_password: &str, + sk: SecretKey, + totp: Option, + ticket: Option, + expect_account: Option, ) -> Result { let api = self.api()?; let local = self.lock_inner().account.clone(); @@ -561,7 +579,7 @@ impl WebCore { keyset: Arc::new(keyset), vault_key: Arc::new(vault_key), auth_input: Some(derived.auth_input), - secret_key: Zeroizing::new(*sk.expose()), + secret_key: Some(Zeroizing::new(*sk.expose())), }); Ok(LoginResult::Ok) } @@ -625,8 +643,9 @@ impl WebCore { Ok(self.api()?.revoke_device(parse_id(device_id)?).await?) } - /// Ends the server session (local data stays). + /// Ends the server session (local data stays) and forgets the Secret Key. pub async fn logout(&self) -> Result<()> { + self.forget_secret_key(); let api = self.api()?; let r = api.logout().await; self.store.delete_meta(META_SESSION)?; @@ -667,6 +686,11 @@ impl WebCore { /// Web only: the browser talks to its own origin (`connect-src 'self'`), so a setup code for /// another server than the configured one is refused instead of switching servers. pub async fn login_with_setup(&self, setup: &SetupInfo, master_password: &str) -> Result { + let r = self.login_with_setup_inner(setup, master_password).await; + r.inspect_err(|_| self.forget_secret_key()) + } + + async fn login_with_setup_inner(&self, setup: &SetupInfo, master_password: &str) -> Result { let p = from_info(setup)?; let (account, server) = { let inner = self.lock_inner(); @@ -805,6 +829,12 @@ impl WebCore { // ------------------------------------------------------------ internals + fn forget_secret_key(&self) { + if let Some(u) = self.lock_inner().unlocked.as_mut() { + u.secret_key = None; + } + } + fn lock_join(&self) -> std::sync::MutexGuard<'_, Option> { self.join.lock().unwrap_or_else(|p| p.into_inner()) } @@ -817,7 +847,7 @@ impl WebCore { server_url: kv_sync::transport::normalize_base_url(&server.url)?, spki_pins: server.pins, account_id: acct.account_id, - secret_key: SecretKey::from_bytes(*u.secret_key).to_display(), + secret_key: SecretKey::from_bytes(**u.secret_key.as_ref().ok_or(CoreError::Locked)?).to_display(), ticket, }) } diff --git a/crates/kv-wasm/src/wasm/tests.rs b/crates/kv-wasm/src/wasm/tests.rs index a968616..a618be7 100644 --- a/crates/kv-wasm/src/wasm/tests.rs +++ b/crates/kv-wasm/src/wasm/tests.rs @@ -239,6 +239,11 @@ async fn setup_codes_in_the_browser_keep_the_secret_key_in_memory_only() { let text = String::from_utf8_lossy(&value); assert!(!text.contains(&secret_key) && !text.contains(&compact[2..]), "{key} holds the Secret Key"); } + // A failed unlock forgets it; lock drops it with the keys. + assert_eq!(err_code(core.unlock("wrong password!".into(), secret_key.clone())).await, "BadCredentials"); + assert_eq!(err_code(core.setup_payload_for_kit()).await, "Locked"); + ok(core.unlock(PW.into(), secret_key.clone())).await; + assert_eq!(ok(core.setup_payload_for_kit()).await.as_string().as_deref(), Some(kit.as_str())); ok(core.lock()).await; assert_eq!(err_code(core.setup_payload_for_kit()).await, "Locked"); db.close(); diff --git a/crates/kv-wasm/tests/sync.rs b/crates/kv-wasm/tests/sync.rs index 57fb45b..4275858 100644 --- a/crates/kv-wasm/tests/sync.rs +++ b/crates/kv-wasm/tests/sync.rs @@ -247,6 +247,13 @@ async fn web_vault_enrolls_by_ticket_both_ways() { reloaded.unlock(PW, &sk).unwrap(); assert_eq!(reloaded.setup_payload_for_kit().unwrap(), kit); web.unlock(PW, &sk).unwrap(); + // A failed unlock (or login) forgets it until the next one that succeeds. + assert!(matches!(web.unlock("wrong password!!", &sk), Err(CoreError::BadCredentials))); + assert!(matches!(web.setup_payload_for_kit(), Err(CoreError::Locked))); + web.unlock(PW, &sk).unwrap(); + assert!(web.login("wrong password!!", &sk, None).await.is_err()); + assert!(matches!(web.setup_payload_for_kit(), Err(CoreError::Locked))); + web.unlock(PW, &sk).unwrap(); // Single use: another browser with the same QR waits for approval. let (_, third) = new_web(&server); @@ -273,6 +280,10 @@ async fn web_vault_enrolls_by_ticket_both_ways() { let (_, late) = new_web(&server); let setup = kv_wasm::core::parse_setup_payload(&add.payload).unwrap(); assert_eq!(late.login_with_setup(&setup, PW).await.unwrap(), LoginResult::PendingApproval); + + // Logging out forgets the Secret Key too. + web.logout().await.unwrap(); + assert!(matches!(web.setup_payload_for_kit(), Err(CoreError::Locked))); } #[tokio::test]