diff --git a/Cargo.lock b/Cargo.lock index 76b7e21..b13925c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -797,6 +797,7 @@ dependencies = [ "serde_json", "tokio", "uuid", + "zeroize", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index d920b4a..b046b6e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,4 +15,5 @@ argon2 = "0.5" aes-gcm = "0.10" base64 = "0.22" getrandom = "0.2" +zeroize = "1" rpassword = "7" diff --git a/src/app.rs b/src/app.rs index 5e41df7..43a5a0b 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1,6 +1,7 @@ use base64::engine::general_purpose::STANDARD as B64; use base64::Engine; use serde::Serialize; +use zeroize::{Zeroize, Zeroizing}; use crate::crypto::{self, VaultKey}; use crate::errors::AppError; @@ -69,8 +70,11 @@ impl App { .crypto .as_ref() .ok_or_else(|| AppError::config("Your vault is not encrypted."))?; - let key = derive_with(crypto, password)?; + let mut key = derive_with(crypto, password)?; if !crypto::verify(&key, &crypto.verifier) { + // A throwaway derived key from a wrong password is wiped, not + // left to linger on the heap. + key.zeroize(); return Err(AppError::wrong_password()); } self.unlock_key = Some(key); @@ -79,7 +83,9 @@ impl App { /// Drop the derived key, re-locking the vault for the session. pub fn lock(&mut self) { - self.unlock_key = None; + if let Some(mut key) = self.unlock_key.take() { + key.zeroize(); + } } /// Protect the vault with `new_password`, re-encrypting every stored key. @@ -99,8 +105,9 @@ impl App { let current = current_password.ok_or_else(|| { AppError::config("Enter your current password to change the vault password.") })?; - let key = derive_with(crypto, current)?; + let mut key = derive_with(crypto, current)?; if !crypto::verify(&key, &crypto.verifier) { + key.zeroize(); return Err(AppError::wrong_password()); } Some(key) @@ -119,15 +126,21 @@ impl App { 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 => profile.secret_key.clone(), + 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. + if let Some(mut key) = previous_key { + key.zeroize(); + } self.vault.profiles = encrypted; self.vault.crypto = Some(VaultCrypto { @@ -140,6 +153,9 @@ impl App { }, verifier: crypto::make_verifier(&new_key)?, }); + if let Some(mut old) = self.unlock_key.take() { + old.zeroize(); + } self.unlock_key = Some(new_key); Ok(()) } @@ -151,22 +167,26 @@ impl App { .crypto .as_ref() .ok_or_else(|| AppError::config("Your vault is not encrypted."))?; - let key = derive_with(crypto, current_password)?; + let mut key = derive_with(crypto, current_password)?; if !crypto::verify(&key, &crypto.verifier) { + key.zeroize(); return Err(AppError::wrong_password()); } + // This feature intentionally restores plaintext-at-rest storage, so + // the decrypted value moves into the vault; the Zeroizing wrapper's + // own buffer content is transferred rather than copied. let mut plain = Vec::with_capacity(self.vault.profiles.len()); for profile in &self.vault.profiles { - let plaintext = crypto::decrypt_secret(&key, &profile.secret_key)?; + let mut plaintext = crypto::decrypt_secret(&key, &profile.secret_key)?; plain.push(StoredProfile { - secret_key: plaintext, + secret_key: std::mem::take(&mut *plaintext), ..profile.clone() }); } self.vault.profiles = plain; self.vault.crypto = None; - self.unlock_key = None; + self.lock(); Ok(()) } @@ -186,6 +206,14 @@ impl App { } } +/// Wipe the in-memory vault key when the application state goes away, so the +/// key does not outlive its owner in unscrubbed heap memory. +impl Drop for App { + fn drop(&mut self) { + self.lock(); + } +} + /// Reject weak new passwords with a clear, early message. fn validate_new_password(password: &str) -> Result<(), AppError> { if password.len() < MIN_PASSWORD_LEN { diff --git a/src/crypto.rs b/src/crypto.rs index c2b72e9..e58bed4 100644 --- a/src/crypto.rs +++ b/src/crypto.rs @@ -11,6 +11,7 @@ use argon2::{Algorithm, Argon2, Params, Version}; use base64::engine::general_purpose::STANDARD as B64; use base64::Engine; use getrandom::getrandom; +use zeroize::{Zeroize, Zeroizing}; use crate::errors::AppError; @@ -56,9 +57,12 @@ pub fn derive_key( .map_err(|e| AppError::internal(format!("Invalid Argon2 parameters: {e}")))?; let argon2 = Argon2::new(Algorithm::Argon2id, Version::V0x13, params); let mut key = [0u8; KEY_LEN]; - argon2 - .hash_password_into(password.as_bytes(), salt, &mut key) - .map_err(|e| AppError::internal(format!("Could not derive a vault key: {e}")))?; + if let Err(e) = argon2.hash_password_into(password.as_bytes(), salt, &mut key) { + key.zeroize(); + return Err(AppError::internal(format!( + "Could not derive a vault key: {e}" + ))); + } Ok(key) } @@ -88,10 +92,19 @@ pub fn encrypt_secret(key: &VaultKey, plaintext_hex: &str) -> Result Result { +/// +/// The returned string is [`Zeroizing`]: its memory is shredded the moment it +/// goes out of scope, so transient decrypted keys do not linger on the heap. +pub fn decrypt_secret(key: &VaultKey, encoded: &str) -> Result, AppError> { let plain = decrypt(key, encoded)?; - String::from_utf8(plain) - .map_err(|e| AppError::internal(format!("A decrypted key was not valid text: {e}"))) + match String::from_utf8(plain) { + Ok(text) => Ok(Zeroizing::new(text)), + Err(e) => { + let mut bytes = e.into_bytes(); + bytes.zeroize(); + Err(AppError::internal("A decrypted key was not valid text.")) + } + } } /// AES-256-GCM encrypt, returning nonce || ciphertext || tag. @@ -148,7 +161,7 @@ mod tests { let secret = "00".repeat(32); let blob = encrypt_secret(&key, &secret).unwrap(); assert_ne!(blob, secret); - assert_eq!(decrypt_secret(&key, &blob).unwrap(), secret); + assert_eq!(*decrypt_secret(&key, &blob).unwrap(), secret); } #[test] diff --git a/src/profiles.rs b/src/profiles.rs index 9047899..627aa03 100644 --- a/src/profiles.rs +++ b/src/profiles.rs @@ -1,5 +1,6 @@ use nostr_sdk::prelude::*; use serde::Serialize; +use zeroize::Zeroizing; use crate::crypto::VaultKey; use crate::errors::AppError; @@ -43,7 +44,9 @@ pub fn create_profile( let keys = Keys::generate(); - let secret_hex = ::hex::encode(keys.secret_key().to_secret_bytes()); + // Wiped from memory when this scope ends; only the (possibly encrypted) + // stored copy survives. + let secret_hex = Zeroizing::new(::hex::encode(keys.secret_key().to_secret_bytes())); let public_key = keys .public_key() .to_bech32() @@ -52,7 +55,7 @@ pub fn create_profile( let stored_secret = match &vault.crypto { Some(_) => crate::crypto::encrypt_secret(key.expect("guarded above"), &secret_hex)?, - None => secret_hex, + None => secret_hex.to_string(), }; let profile = StoredProfile { @@ -146,18 +149,21 @@ pub fn active_secret_key(vault: &Vault) -> Result<&str, AppError> { /// Return the plaintext hex secret key for a profile, decrypting it when the /// vault is password-protected. When the vault is encrypted, `key` must be the /// unlocked vault key; otherwise the operation is rejected as locked. +/// +/// The value is [`Zeroizing`]: it shreds itself when the caller drops it, so +/// transient copies of secret keys do not linger on the heap. pub fn resolve_secret_key( vault: &Vault, npub: &str, key: Option<&VaultKey>, -) -> Result { +) -> Result, AppError> { let stored = find_secret_key(vault, npub)?; match &vault.crypto { Some(_) => { let key = key.ok_or_else(AppError::vault_locked)?; crate::crypto::decrypt_secret(key, stored) } - None => Ok(stored.to_string()), + None => Ok(Zeroizing::new(stored.to_string())), } } @@ -165,7 +171,7 @@ pub fn resolve_secret_key( pub fn resolve_active_secret_key( vault: &Vault, key: Option<&VaultKey>, -) -> Result { +) -> Result, AppError> { let npub = vault .active_profile .as_deref() @@ -183,11 +189,16 @@ pub fn reveal_secret_key( npub: &str, key: Option<&VaultKey>, ) -> Result { - let hex = resolve_secret_key(vault, npub, key)?; - let nsec = parse_secret_key(&hex)? + let resolved = resolve_secret_key(vault, npub, key)?; + let nsec = parse_secret_key(&resolved)? .to_bech32() .map_err(|e| AppError::internal(format!("Could not encode the secret key: {e}")))?; - Ok(RevealedKey { hex, nsec }) + // The display copy is intentionally handed to the caller (UI/CLI); the + // internal `resolved` buffer wipes itself on return. + Ok(RevealedKey { + hex: resolved.to_string(), + nsec, + }) } /// Parse and validate a stored hex-encoded secret key.