kv-wasm: forget the Secret Key on logout and on any failed unlock or login
Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -79,8 +79,10 @@ struct Unlocked {
|
||||
vault_key: Arc<SymKey>,
|
||||
/// OPAQUE password, kept while unlocked so an expired session can be renewed without prompting.
|
||||
auth_input: Option<Zeroizing<[u8; 32]>>,
|
||||
/// 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<Zeroizing<[u8; 16]>>,
|
||||
}
|
||||
|
||||
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<String>) -> Result<LoginResult> {
|
||||
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<String>,
|
||||
ticket: Option<String>,
|
||||
expect_account: Option<Uuid>,
|
||||
) -> Result<LoginResult> {
|
||||
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<String>,
|
||||
ticket: Option<String>,
|
||||
expect_account: Option<Uuid>,
|
||||
) -> Result<LoginResult> {
|
||||
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<LoginResult> {
|
||||
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<LoginResult> {
|
||||
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<JoinState>> {
|
||||
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,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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]
|
||||
|
||||
Reference in New Issue
Block a user