From c553dd5e177561cf20f541cc50678791eb30dfb8 Mon Sep 17 00:00:00 2001 From: Avi Date: Wed, 30 Sep 2026 10:50:48 -0500 Subject: [PATCH] feat(vault): KDF upgrade to 64 MiB / t=3 with transparent migration on unlock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New vaults derive their key with Argon2id m=64 MiB, t=3 (OWASP 2024) instead of the RFC 9106 19 MiB / t=2 defaults. The vault header already records per-vault KDF parameters, so unlocking an old vault keeps working unchanged; when it carries exactly the legacy parameters the unlock transparently re-wraps every secret (profile keys, NIP-46 connection secrets, client keys) under a fresh salt + stronger key and reports the upgrade so the caller persists it. Also fixes a real gap surfaced by the new tests: set_password previously re-encrypted only profile keys — NIP-46 connection secrets and client keys stayed plaintext in the JSON after 'encrypt my vault'. All three secret stores now go through one rewrap_secrets() helper shared by set_password and the KDF upgrade path. Wrong passwords never touch the header; the upgrade runs only after the old verifier accepts. Tests: legacy-params upgrade + no-op on current params, wrong-password leaves header untouched, rewrap covers connection secrets/client keys for both upgrade and set_password. --- src/app.rs | 241 ++++++++++++++++++++++++++++++++++++++++++++++---- src/crypto.rs | 15 +++- src/ipc.rs | 7 +- src/main.rs | 12 ++- 4 files changed, 250 insertions(+), 25 deletions(-) diff --git a/src/app.rs b/src/app.rs index 8ee1134..5460e89 100644 --- a/src/app.rs +++ b/src/app.rs @@ -169,8 +169,16 @@ impl App { self.signing_for(&npub).await } - /// Verify a password and keep the derived key in memory for the session. - pub fn unlock(&mut self, password: &str) -> Result<(), AppError> { + /// Verify a password, keep the derived key in memory for the session, + /// and transparently upgrade the vault's KDF if it is still encrypted + /// under the legacy parameters (19 MiB / t=2). + /// + /// Returns `true` when the vault was re-wrapped under the current + /// [`crate::crypto::KDF_M_COST`]/[`crate::crypto::KDF_T_COST`] (the caller + /// should persist the vault), `false` for a plain unlock. A wrong password + /// never touches the vault header: the upgrade only runs after the old + /// verifier has accepted the supplied password. + pub fn unlock(&mut self, password: &str) -> Result { let crypto = self .vault .crypto @@ -183,8 +191,41 @@ impl App { key.zeroize(); return Err(AppError::wrong_password()); } - self.unlock_key = Some(key); - Ok(()) + + let is_legacy = crypto.kdf.m_cost == crypto::LEGACY_KDF_M_COST + && crypto.kdf.t_cost == crypto::LEGACY_KDF_T_COST; + if !is_legacy { + self.unlock_key = Some(key); + return Ok(false); + } + + // Password accepted under the legacy parameters: re-derive with the + // current ones and re-wrap every stored secret under the new key. + let upgraded = { + let salt = crypto::generate_salt()?; + let new_key = crypto::derive_key( + password, + &salt, + crypto::KDF_M_COST, + crypto::KDF_T_COST, + crypto::KDF_P_COST, + )?; + rewrap_secrets(&mut self.vault, Some(&key), &new_key)?; + key.zeroize(); + self.vault.crypto = Some(VaultCrypto { + kdf: KdfParams { + algorithm: "argon2id".to_string(), + salt: B64.encode(salt), + m_cost: crypto::KDF_M_COST, + t_cost: crypto::KDF_T_COST, + p_cost: crypto::KDF_P_COST, + }, + verifier: crypto::make_verifier(&new_key)?, + }); + new_key + }; + self.unlock_key = Some(upgraded); + Ok(true) } /// Drop the derived key, re-locking the vault for the session. @@ -348,25 +389,14 @@ impl App { crypto::KDF_P_COST, )?; - let mut encrypted = Vec::with_capacity(self.vault.profiles.len()); - for profile in &self.vault.profiles { - // Decrypted plaintexts are Zeroizing: each wipes itself once the - // re-encrypted replacement has been produced. - let plaintext = match &previous_key { - Some(key) => crypto::decrypt_secret(key, &profile.secret_key)?, - None => Zeroizing::new(profile.secret_key.clone()), - }; - encrypted.push(StoredProfile { - secret_key: crypto::encrypt_secret(&new_key, &plaintext)?, - ..profile.clone() - }); - } - // Wipe the old vault key now that every profile is re-encrypted. + // Re-wrap every secret store (profiles, connection secrets, client + // keys) so no credential keeps its old wrapping. + rewrap_secrets(&mut self.vault, previous_key.as_ref(), &new_key)?; + // Wipe the old vault key now that everything is re-encrypted. if let Some(mut key) = previous_key { key.zeroize(); } - self.vault.profiles = encrypted; self.vault.crypto = Some(VaultCrypto { kdf: KdfParams { algorithm: "argon2id".to_string(), @@ -467,6 +497,47 @@ fn derive_with(crypto: &VaultCrypto, password: &str) -> Result, + new_key: &VaultKey, +) -> Result<(), AppError> { + let decrypt_one = |blob: &str| -> Result, AppError> { + match old_key { + Some(key) => crypto::decrypt_secret(key, blob), + None => Ok(Zeroizing::new(blob.to_string())), + } + }; + + let mut profiles = Vec::with_capacity(vault.profiles.len()); + for profile in &vault.profiles { + let plaintext = decrypt_one(&profile.secret_key)?; + profiles.push(StoredProfile { + secret_key: crypto::encrypt_secret(new_key, &plaintext)?, + ..profile.clone() + }); + } + vault.profiles = profiles; + + for entry in &mut vault.connection_secrets { + let plaintext = decrypt_one(&entry.secret)?; + entry.secret = crypto::encrypt_secret(new_key, &plaintext)?; + } + for entry in &mut vault.connection_client_keys { + let plaintext = decrypt_one(&entry.secret_hex)?; + entry.secret_hex = crypto::encrypt_secret(new_key, &plaintext)?; + } + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -607,6 +678,138 @@ mod tests { assert_eq!(err.kind(), ErrorKind::Config); } + /// Build a vault encrypted under explicit (legacy) KDF parameters, + /// exactly as an older app version would have written it, and return the + /// matching password. Also stores one connection secret and one client + /// key encrypted under the same legacy key. + fn legacy_kdf_app(m_cost: u32, t_cost: u32) -> App { + use crate::crypto; + let password = "correct horse battery staple"; + let salt = crypto::generate_salt().unwrap(); + let key = crypto::derive_key(password, &salt, m_cost, t_cost, crypto::KDF_P_COST).unwrap(); + let mut app = sample_app(); + let ref_ = crate::signer::VaultRef::new(Some("npub1test".to_string()), "aabb".to_string()); + app.vault.crypto = Some(VaultCrypto { + kdf: KdfParams { + algorithm: "argon2id".to_string(), + salt: B64.encode(salt), + m_cost, + t_cost, + p_cost: crypto::KDF_P_COST, + }, + verifier: crypto::make_verifier(&key).unwrap(), + }); + // crypto is set first so these store ENCRYPTED under the legacy key, + // exactly as a running pre-upgrade app would have written them. + for profile in &mut app.vault.profiles { + profile.secret_key = crypto::encrypt_secret(&key, &profile.secret_key).unwrap(); + } + vault::store_connection_secret(&mut app.vault, Some(&key), &ref_, "pairing-secret") + .unwrap(); + vault::store_connection_client_key( + &mut app.vault, + Some(&key), + &ref_, + "cd".repeat(32).as_str(), + ) + .unwrap(); + app + } + + #[test] + fn unlock_upgrades_legacy_kdf_params() { + let mut app = legacy_kdf_app(crypto::LEGACY_KDF_M_COST, crypto::LEGACY_KDF_T_COST); + let legacy_m = app.vault.crypto.as_ref().unwrap().kdf.m_cost; + assert_ne!(legacy_m, crypto::KDF_M_COST, "test vault must be legacy"); + + let upgraded = app + .unlock("correct horse battery staple") + .expect("legacy vault must unlock"); + assert!(upgraded, "unlock must report a KDF upgrade happened"); + + let kdf = &app.vault.crypto.as_ref().unwrap().kdf; + assert_eq!(kdf.m_cost, crypto::KDF_M_COST); + assert_eq!(kdf.t_cost, crypto::KDF_T_COST); + + // The new key must decrypt every profile secret re-wrapped under it, + // and the new verifier must accept it. + let key = app.vault_key().expect("unlocked"); + assert!(crypto::verify( + key, + &app.vault.crypto.as_ref().unwrap().verifier + )); + let secret = profiles::resolve_secret_key( + &app.vault, + &app.vault.profiles[0].public_key.clone(), + app.vault_key(), + ) + .unwrap(); + assert!(!secret.is_empty()); + + // Unlocking again is a no-op: already at current parameters. + app.lock(); + assert!( + !app.unlock("correct horse battery staple").unwrap(), + "current-params vault must not re-upgrade" + ); + } + + #[test] + fn kdf_upgrade_rewraps_connection_secrets_and_client_keys() { + let mut app = legacy_kdf_app(crypto::LEGACY_KDF_M_COST, crypto::LEGACY_KDF_T_COST); + let ref_ = crate::signer::VaultRef::new(Some("npub1test".to_string()), "aabb".to_string()); + app.unlock("correct horse battery staple").unwrap(); + + let secret = vault::resolve_connection_secret(&app.vault, app.vault_key(), &ref_) + .unwrap() + .expect("connection secret survives the upgrade"); + assert_eq!(secret.as_str(), "pairing-secret"); + let client_key = vault::resolve_connection_client_key(&app.vault, app.vault_key(), &ref_) + .unwrap() + .expect("client key survives the upgrade"); + assert_eq!(client_key.as_str(), "cd".repeat(32)); + } + + #[test] + fn wrong_password_does_not_upgrade_kdf() { + let mut app = legacy_kdf_app(crypto::LEGACY_KDF_M_COST, crypto::LEGACY_KDF_T_COST); + let err = app + .unlock("not the password") + .expect_err("wrong password must fail"); + assert_eq!(err.kind(), ErrorKind::WrongPassword); + let kdf = &app.vault.crypto.as_ref().unwrap().kdf; + assert_eq!(kdf.m_cost, crypto::LEGACY_KDF_M_COST, "header untouched"); + assert!(app.is_locked()); + } + + #[test] + fn set_password_rewraps_connection_secrets_and_client_keys() { + // Plaintext vault with connection credentials: setting a password + // must encrypt EVERY secret, not just profile keys. + let mut app = sample_app(); + let ref_ = crate::signer::VaultRef::new(Some("npub1test".to_string()), "aabb".to_string()); + vault::store_connection_secret(&mut app.vault, None, &ref_, "pairing-secret").unwrap(); + vault::store_connection_client_key(&mut app.vault, None, &ref_, "ef".repeat(32).as_str()) + .unwrap(); + + app.set_password(None, "correct horse battery staple") + .unwrap(); + + let serialized = serde_json::to_string(&app.vault).unwrap(); + assert!( + !serialized.contains("pairing-secret"), + "pairing secret must not survive in plaintext" + ); + assert!( + !serialized.contains(&"ef".repeat(32)), + "client key must not survive in plaintext" + ); + let secret = vault::resolve_connection_secret(&app.vault, app.vault_key(), &ref_) + .unwrap() + .expect("secret still resolvable after encryption"); + assert_eq!(secret.as_str(), "pairing-secret"); + } + #[test] fn unlock_roundtrip_with_wrong_then_right_password() { let mut app = sample_app(); diff --git a/src/crypto.rs b/src/crypto.rs index e58bed4..21f1181 100644 --- a/src/crypto.rs +++ b/src/crypto.rs @@ -22,13 +22,22 @@ pub const SALT_LEN: usize = 16; /// AES-GCM nonce length in bytes. pub const NONCE_LEN: usize = 12; -/// Argon2id memory cost in KiB (RFC 9106 recommendation). -pub const KDF_M_COST: u32 = 19 * 1024; +/// Argon2id memory cost in KiB (64 MiB — OWASP 2024 recommendation). +pub const KDF_M_COST: u32 = 64 * 1024; /// Argon2id time cost (iterations). -pub const KDF_T_COST: u32 = 2; +pub const KDF_T_COST: u32 = 3; /// Argon2id parallelism. pub const KDF_P_COST: u32 = 1; +/// Memory cost used by app versions before the Step 5 upgrade (RFC 9106's +/// 19 MiB / t=2). Vaults carrying exactly these parameters are transparently +/// re-wrapped under [`KDF_M_COST`]/[`KDF_T_COST`] on next unlock; vaults with +/// any OTHER parameters keep them — the header is authoritative, and these +/// constants are only the definition of "legacy". +pub const LEGACY_KDF_M_COST: u32 = 19 * 1024; +/// Time cost used by pre-upgrade versions (see [`LEGACY_KDF_M_COST`]). +pub const LEGACY_KDF_T_COST: u32 = 2; + /// The in-memory key that unlocks an encrypted vault. pub type VaultKey = [u8; KEY_LEN]; diff --git a/src/ipc.rs b/src/ipc.rs index cd6a09a..43de978 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -1024,7 +1024,12 @@ async fn run_with_app(app: &mut App, request: Request) -> Result { - app.unlock(&password)?; + let upgraded = app.unlock(&password)?; + if upgraded { + // The vault was transparently re-wrapped under stronger KDF + // parameters: persist immediately so the upgrade sticks. + app.save_vault()?; + } // Re-initialize signers with unlocked vault if app.signer_mode == SignerMode::Embedded { if let Some(signer) = &app.embedded_signer { diff --git a/src/main.rs b/src/main.rs index 525b731..4bb8530 100644 --- a/src/main.rs +++ b/src/main.rs @@ -483,7 +483,10 @@ fn load_app_with_unlock() -> Result { let mut app = App::load()?; if app.is_locked() { let password = prompt_password("Vault password: ")?; - app.unlock(&password)?; + if app.unlock(&password)? { + // Transparent KDF upgrade: persist the re-wrapped vault. + app.save_vault()?; + } } Ok(app) } @@ -531,7 +534,12 @@ fn cli_unlock() -> Result { return Err(AppError::config("Your vault is not encrypted.")); } let password = prompt_password("Vault password: ")?; - app.unlock(&password)?; + if app.unlock(&password)? { + app.save_vault()?; + return Ok( + "Vault unlocked and upgraded to stronger key-derivation parameters.".to_string(), + ); + } Ok("Vault unlocked.".to_string()) }