From 52c7c1147e94651091ace53b43d11d13ed043f0f Mon Sep 17 00:00:00 2001 From: Mickey Scherrer <5324300+iicky@users.noreply.github.com> Date: Thu, 23 Apr 2026 00:23:25 -0400 Subject: [PATCH 1/2] zeroize decrypted secrets in memory --- Cargo.lock | 1 + Cargo.toml | 1 + src/crypto.rs | 24 ++++-- src/export.rs | 184 ++++++++++++++++++++++++---------------------- src/lib.rs | 86 +++++++++++++++------- src/main.rs | 89 +++++++++++----------- src/python.rs | 9 ++- src/recipients.rs | 6 +- src/recovery.rs | 31 ++++---- src/scan.rs | 25 +++++-- src/secrets.rs | 40 +++++----- src/testutil.rs | 7 ++ src/types.rs | 8 +- 13 files changed, 299 insertions(+), 212 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0b0492a..c78a716 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1405,6 +1405,7 @@ dependencies = [ "tempfile", "ureq", "walkdir", + "zeroize", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index ff4b115..4911d2a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,6 +43,7 @@ clap_complete = "4.6.1" fs2 = "0.4.3" pyo3 = { version = "0.28", features = ["extension-module"], optional = true } walkdir = "2.5.0" +zeroize = "1" [target.'cfg(unix)'.dependencies] libc = "0.2" diff --git a/src/crypto.rs b/src/crypto.rs index 118a0c1..4e64470 100644 --- a/src/crypto.rs +++ b/src/crypto.rs @@ -1,4 +1,5 @@ use std::io::{Read, Write}; +use zeroize::Zeroizing; /// Errors that can occur during crypto operations. #[derive(Debug)] @@ -156,11 +157,18 @@ pub fn encrypt(plaintext: &[u8], recipients: &[MurkRecipient]) -> Result } /// Decrypt ciphertext using an identity (age or SSH key). -pub fn decrypt(ciphertext: &[u8], identity: &MurkIdentity) -> Result, CryptoError> { +/// +/// Returns the plaintext wrapped in `Zeroizing>` so the buffer is +/// cleared when dropped. Defense-in-depth against plaintext lingering in +/// freed heap memory. +pub fn decrypt( + ciphertext: &[u8], + identity: &MurkIdentity, +) -> Result>, CryptoError> { let decryptor = age::Decryptor::new_buffered(ciphertext) .map_err(|e| CryptoError::Decrypt(e.to_string()))?; - let mut plaintext = vec![]; + let mut plaintext = Zeroizing::new(vec![]); let mut reader = decryptor .decrypt(std::iter::once(identity.as_dyn())) .map_err(|e| CryptoError::Decrypt(e.to_string()))?; @@ -194,7 +202,7 @@ mod tests { let ciphertext = encrypt(plaintext, &[recipient]).unwrap(); let decrypted = decrypt(&ciphertext, &identity).unwrap(); - assert_eq!(decrypted, plaintext); + assert_eq!(&decrypted[..], plaintext); } #[test] @@ -213,8 +221,8 @@ mod tests { // Both recipients can decrypt let id_a = parse_identity(&secret_a).unwrap(); let id_b = parse_identity(&secret_b).unwrap(); - assert_eq!(decrypt(&ciphertext, &id_a).unwrap(), plaintext); - assert_eq!(decrypt(&ciphertext, &id_b).unwrap(), plaintext); + assert_eq!(&decrypt(&ciphertext, &id_a).unwrap()[..], plaintext); + assert_eq!(&decrypt(&ciphertext, &id_b).unwrap()[..], plaintext); } #[test] @@ -311,7 +319,7 @@ mod tests { let plaintext = b"ssh secrets"; let ciphertext = encrypt(plaintext, &[recipient]).unwrap(); let decrypted = decrypt(&ciphertext, &id).unwrap(); - assert_eq!(decrypted, plaintext); + assert_eq!(&decrypted[..], plaintext); } #[test] @@ -334,7 +342,7 @@ mod tests { // Both can decrypt. let age_id = parse_identity(&age_secret).unwrap(); - assert_eq!(decrypt(&ciphertext, &age_id).unwrap(), plaintext); - assert_eq!(decrypt(&ciphertext, &ssh_id).unwrap(), plaintext); + assert_eq!(&decrypt(&ciphertext, &age_id).unwrap()[..], plaintext); + assert_eq!(&decrypt(&ciphertext, &ssh_id).unwrap()[..], plaintext); } } diff --git a/src/export.rs b/src/export.rs index a2debf9..740998b 100644 --- a/src/export.rs +++ b/src/export.rs @@ -2,17 +2,26 @@ use std::collections::{BTreeMap, HashMap}; +use zeroize::Zeroizing; + use crate::types; /// Merge scoped overrides over shared values and filter by tag. /// Returns raw (unescaped) values suitable for env var injection. +/// +/// Values are wrapped in `Zeroizing` so plaintext is cleared from memory +/// when the returned map is dropped. pub fn resolve_secrets( vault: &types::Vault, murk: &types::Murk, pubkey: &str, tags: &[String], -) -> BTreeMap { - let mut values = murk.values.clone(); +) -> BTreeMap> { + let mut values = murk + .values + .iter() + .map(|(k, v)| (k.clone(), v.clone())) + .collect::>>(); // Apply scoped overrides. for (key, scoped_map) in &murk.scoped { @@ -36,14 +45,14 @@ pub fn resolve_secrets( }; let mut result = BTreeMap::new(); - for (k, v) in &values { + for (k, v) in values { if allowed_keys .as_ref() .is_some_and(|a| !a.contains(k.as_str())) { continue; } - result.insert(k.clone(), v.clone()); + result.insert(k, v); } result } @@ -55,10 +64,10 @@ pub fn export_secrets( murk: &types::Murk, pubkey: &str, tags: &[String], -) -> BTreeMap { +) -> BTreeMap> { resolve_secrets(vault, murk, pubkey, tags) .into_iter() - .map(|(k, v)| (k, v.replace('\'', "'\\''"))) + .map(|(k, v)| (k, Zeroizing::new(v.replace('\'', "'\\''")))) .collect() } @@ -69,14 +78,15 @@ pub fn export_secrets( pub fn decrypt_vault_values( vault: &types::Vault, identity: &crate::crypto::MurkIdentity, -) -> HashMap { +) -> HashMap> { let pubkey = identity.pubkey_string().unwrap_or_default(); let mut values = HashMap::new(); for (key, entry) in &vault.secrets { // Decrypt shared value. if !entry.shared.is_empty() && let Ok(value) = crate::decrypt_value(&entry.shared, identity).and_then(|pt| { - String::from_utf8(pt).map_err(|e| crate::error::MurkError::Secret(e.to_string())) + crate::plaintext_bytes_to_zeroizing_string(&pt) + .map_err(|e| crate::error::MurkError::Secret(e.to_string())) }) { values.insert(key.clone(), value); @@ -84,7 +94,8 @@ pub fn decrypt_vault_values( // Scoped override takes priority. if let Some(encoded) = entry.scoped.get(&pubkey) && let Ok(value) = crate::decrypt_value(encoded, identity).and_then(|pt| { - String::from_utf8(pt).map_err(|e| crate::error::MurkError::Secret(e.to_string())) + crate::plaintext_bytes_to_zeroizing_string(&pt) + .map_err(|e| crate::error::MurkError::Secret(e.to_string())) }) { values.insert(key.clone(), value); @@ -100,7 +111,7 @@ pub fn decrypt_vault_values( pub fn parse_and_decrypt_values( vault_contents: &str, identity: &crate::crypto::MurkIdentity, -) -> Result, String> { +) -> Result>, String> { let vault = crate::vault::parse(vault_contents).map_err(|e| e.to_string())?; Ok(decrypt_vault_values(&vault, identity)) } @@ -114,18 +125,22 @@ pub enum DiffKind { } /// A single entry in a secret diff. +/// +/// `old_value` and `new_value` are held in `Zeroizing` so plaintext is cleared +/// when the entry is dropped. Formatting/printing callers should take care not +/// to retain their own unzeroed copies. #[derive(Debug)] pub struct DiffEntry { pub key: String, pub kind: DiffKind, - pub old_value: Option, - pub new_value: Option, + pub old_value: Option>, + pub new_value: Option>, } /// Compare two sets of secret values and return the differences. pub fn diff_secrets( - old: &HashMap, - new: &HashMap, + old: &HashMap>, + new: &HashMap>, ) -> Vec { let mut all_keys: Vec<&str> = old .keys() @@ -151,7 +166,7 @@ pub fn diff_secrets( old_value: Some(v.clone()), new_value: None, }), - (Some(old_v), Some(new_v)) if old_v != new_v => entries.push(DiffEntry { + (Some(old_v), Some(new_v)) if **old_v != **new_v => entries.push(DiffEntry { key: key.into(), kind: DiffKind::Changed, old_value: Some(old_v.clone()), @@ -175,23 +190,14 @@ pub fn format_diff_lines(entries: &[DiffEntry], show_values: bool) -> Vec "~", }; if show_values { + let old = entry.old_value.as_ref().map_or("", |v| v.as_str()); + let new = entry.new_value.as_ref().map_or("", |v| v.as_str()); match entry.kind { - DiffKind::Added => format!( - "{symbol} {} = {}", - entry.key, - entry.new_value.as_deref().unwrap_or("") - ), - DiffKind::Removed => format!( - "{symbol} {} = {}", - entry.key, - entry.old_value.as_deref().unwrap_or("") - ), - DiffKind::Changed => format!( - "{symbol} {} {} → {}", - entry.key, - entry.old_value.as_deref().unwrap_or(""), - entry.new_value.as_deref().unwrap_or("") - ), + DiffKind::Added => format!("{symbol} {} = {}", entry.key, new), + DiffKind::Removed => format!("{symbol} {} = {}", entry.key, old), + DiffKind::Changed => { + format!("{symbol} {} {} → {}", entry.key, old, new) + } } } else { format!("{symbol} {}", entry.key) @@ -220,11 +226,11 @@ mod tests { ); let mut murk = empty_murk(); - murk.values.insert("FOO".into(), "bar".into()); + murk.values.insert("FOO".into(), secret("bar")); let exports = export_secrets(&vault, &murk, "age1pk", &[]); assert_eq!(exports.len(), 1); - assert_eq!(exports["FOO"], "bar"); + assert_eq!(exports["FOO"].as_str(), "bar"); } #[test] @@ -241,13 +247,13 @@ mod tests { ); let mut murk = empty_murk(); - murk.values.insert("KEY".into(), "shared".into()); + murk.values.insert("KEY".into(), secret("shared")); let mut scoped = HashMap::new(); - scoped.insert("age1pk".into(), "override".into()); + scoped.insert("age1pk".into(), secret("override")); murk.scoped.insert("KEY".into(), scoped); let exports = export_secrets(&vault, &murk, "age1pk", &[]); - assert_eq!(exports["KEY"], "override"); + assert_eq!(exports["KEY"].as_str(), "override"); } #[test] @@ -273,12 +279,12 @@ mod tests { ); let mut murk = empty_murk(); - murk.values.insert("A".into(), "val_a".into()); - murk.values.insert("B".into(), "val_b".into()); + murk.values.insert("A".into(), secret("val_a")); + murk.values.insert("B".into(), secret("val_b")); let exports = export_secrets(&vault, &murk, "age1pk", &["db".into()]); assert_eq!(exports.len(), 1); - assert_eq!(exports["A"], "val_a"); + assert_eq!(exports["A"].as_str(), "val_a"); } #[test] @@ -295,15 +301,15 @@ mod tests { ); let mut murk = empty_murk(); - murk.values.insert("KEY".into(), "it's a test".into()); + murk.values.insert("KEY".into(), secret("it's a test")); let exports = export_secrets(&vault, &murk, "age1pk", &[]); - assert_eq!(exports["KEY"], "it'\\''s a test"); + assert_eq!(exports["KEY"].as_str(), "it'\\''s a test"); } #[test] fn diff_secrets_no_changes() { - let old = HashMap::from([("K".into(), "V".into())]); + let old = HashMap::from([("K".into(), secret("V"))]); let new = old.clone(); assert!(diff_secrets(&old, &new).is_empty()); } @@ -311,46 +317,52 @@ mod tests { #[test] fn diff_secrets_added() { let old = HashMap::new(); - let new = HashMap::from([("KEY".into(), "val".into())]); + let new = HashMap::from([("KEY".into(), secret("val"))]); let entries = diff_secrets(&old, &new); assert_eq!(entries.len(), 1); assert_eq!(entries[0].kind, DiffKind::Added); assert_eq!(entries[0].key, "KEY"); - assert_eq!(entries[0].new_value.as_deref(), Some("val")); + assert_eq!(entries[0].new_value.as_deref(), Some(&String::from("val"))); } #[test] fn diff_secrets_removed() { - let old = HashMap::from([("KEY".into(), "val".into())]); + let old = HashMap::from([("KEY".into(), secret("val"))]); let new = HashMap::new(); let entries = diff_secrets(&old, &new); assert_eq!(entries.len(), 1); assert_eq!(entries[0].kind, DiffKind::Removed); - assert_eq!(entries[0].old_value.as_deref(), Some("val")); + assert_eq!(entries[0].old_value.as_deref(), Some(&String::from("val"))); } #[test] fn diff_secrets_changed() { - let old = HashMap::from([("KEY".into(), "old_val".into())]); - let new = HashMap::from([("KEY".into(), "new_val".into())]); + let old = HashMap::from([("KEY".into(), secret("old_val"))]); + let new = HashMap::from([("KEY".into(), secret("new_val"))]); let entries = diff_secrets(&old, &new); assert_eq!(entries.len(), 1); assert_eq!(entries[0].kind, DiffKind::Changed); - assert_eq!(entries[0].old_value.as_deref(), Some("old_val")); - assert_eq!(entries[0].new_value.as_deref(), Some("new_val")); + assert_eq!( + entries[0].old_value.as_deref(), + Some(&String::from("old_val")) + ); + assert_eq!( + entries[0].new_value.as_deref(), + Some(&String::from("new_val")) + ); } #[test] fn diff_secrets_mixed() { let old = HashMap::from([ - ("KEEP".into(), "same".into()), - ("REMOVE".into(), "gone".into()), - ("CHANGE".into(), "old".into()), + ("KEEP".into(), secret("same")), + ("REMOVE".into(), secret("gone")), + ("CHANGE".into(), secret("old")), ]); let new = HashMap::from([ - ("KEEP".into(), "same".into()), - ("ADD".into(), "new".into()), - ("CHANGE".into(), "new".into()), + ("KEEP".into(), secret("same")), + ("ADD".into(), secret("new")), + ("CHANGE".into(), secret("new")), ]); let entries = diff_secrets(&old, &new); assert_eq!(entries.len(), 3); @@ -365,9 +377,9 @@ mod tests { fn diff_secrets_sorted_by_key() { let old = HashMap::new(); let new = HashMap::from([ - ("Z".into(), "z".into()), - ("A".into(), "a".into()), - ("M".into(), "m".into()), + ("Z".into(), secret("z")), + ("A".into(), secret("a")), + ("M".into(), secret("m")), ]); let entries = diff_secrets(&old, &new); let keys: Vec<&str> = entries.iter().map(|e| e.key.as_str()).collect(); @@ -383,19 +395,19 @@ mod tests { key: "NEW_KEY".into(), kind: DiffKind::Added, old_value: None, - new_value: Some("secret".into()), + new_value: Some(secret("secret")), }, DiffEntry { key: "OLD_KEY".into(), kind: DiffKind::Removed, - old_value: Some("old".into()), + old_value: Some(secret("old")), new_value: None, }, DiffEntry { key: "MOD_KEY".into(), kind: DiffKind::Changed, - old_value: Some("v1".into()), - new_value: Some("v2".into()), + old_value: Some(secret("v1")), + new_value: Some(secret("v2")), }, ]; let lines = format_diff_lines(&entries, false); @@ -409,13 +421,13 @@ mod tests { key: "KEY".into(), kind: DiffKind::Added, old_value: None, - new_value: Some("new_val".into()), + new_value: Some(secret("new_val")), }, DiffEntry { key: "KEY2".into(), kind: DiffKind::Changed, - old_value: Some("old".into()), - new_value: Some("new".into()), + old_value: Some(secret("old")), + new_value: Some(secret("new")), }, ]; let lines = format_diff_lines(&entries, true); @@ -445,11 +457,11 @@ mod tests { ); let mut murk = empty_murk(); - murk.values.insert("FOO".into(), "bar".into()); + murk.values.insert("FOO".into(), secret("bar")); let resolved = resolve_secrets(&vault, &murk, "age1pk", &[]); assert_eq!(resolved.len(), 1); - assert_eq!(resolved["FOO"], "bar"); + assert_eq!(resolved["FOO"].as_str(), "bar"); } #[test] @@ -466,10 +478,10 @@ mod tests { ); let mut murk = empty_murk(); - murk.values.insert("KEY".into(), "it's a test".into()); + murk.values.insert("KEY".into(), secret("it's a test")); let resolved = resolve_secrets(&vault, &murk, "age1pk", &[]); - assert_eq!(resolved["KEY"], "it's a test"); + assert_eq!(resolved["KEY"].as_str(), "it's a test"); } #[test] @@ -486,13 +498,13 @@ mod tests { ); let mut murk = empty_murk(); - murk.values.insert("KEY".into(), "shared".into()); + murk.values.insert("KEY".into(), secret("shared")); let mut scoped = HashMap::new(); - scoped.insert("age1pk".into(), "override".into()); + scoped.insert("age1pk".into(), secret("override")); murk.scoped.insert("KEY".into(), scoped); let resolved = resolve_secrets(&vault, &murk, "age1pk", &[]); - assert_eq!(resolved["KEY"], "override"); + assert_eq!(resolved["KEY"].as_str(), "override"); } #[test] @@ -518,12 +530,12 @@ mod tests { ); let mut murk = empty_murk(); - murk.values.insert("A".into(), "val_a".into()); - murk.values.insert("B".into(), "val_b".into()); + murk.values.insert("A".into(), secret("val_a")); + murk.values.insert("B".into(), secret("val_b")); let resolved = resolve_secrets(&vault, &murk, "age1pk", &["db".into()]); assert_eq!(resolved.len(), 1); - assert_eq!(resolved["A"], "val_a"); + assert_eq!(resolved["A"].as_str(), "val_a"); } #[test] @@ -551,12 +563,12 @@ mod tests { let mut murk = empty_murk(); // Only REAL has a value, ORPHAN does not. - murk.values.insert("REAL".into(), "real_val".into()); + murk.values.insert("REAL".into(), secret("real_val")); let resolved = resolve_secrets(&vault, &murk, "age1pk", &["db".into()]); // ORPHAN should not appear since it has no value. assert_eq!(resolved.len(), 1); - assert_eq!(resolved["REAL"], "real_val"); + assert_eq!(resolved["REAL"].as_str(), "real_val"); assert!(!resolved.contains_key("ORPHAN")); } @@ -575,19 +587,19 @@ mod tests { ); let mut murk = empty_murk(); - murk.values.insert("KEY".into(), "shared".into()); + murk.values.insert("KEY".into(), secret("shared")); // Scoped override for a pubkey NOT in vault.recipients. let mut scoped = HashMap::new(); - scoped.insert("age1outsider".into(), "outsider_val".into()); + scoped.insert("age1outsider".into(), secret("outsider_val")); murk.scoped.insert("KEY".into(), scoped); // The outsider's override should still be applied (resolve doesn't gate on recipient list). let resolved = resolve_secrets(&vault, &murk, "age1outsider", &[]); - assert_eq!(resolved["KEY"], "outsider_val"); + assert_eq!(resolved["KEY"].as_str(), "outsider_val"); // Alice gets the shared value since she has no scoped override. let resolved_alice = resolve_secrets(&vault, &murk, "age1alice", &[]); - assert_eq!(resolved_alice["KEY"], "shared"); + assert_eq!(resolved_alice["KEY"].as_str(), "shared"); } // ── New edge-case tests ── @@ -625,8 +637,8 @@ mod tests { let values = crate::export::decrypt_vault_values(&vault, &identity); assert_eq!(values.len(), 2); - assert_eq!(values["KEY1"], "val1"); - assert_eq!(values["KEY2"], "val2"); + assert_eq!(values["KEY1"].as_str(), "val1"); + assert_eq!(values["KEY2"].as_str(), "val2"); } #[test] @@ -695,8 +707,8 @@ mod tests { let json = serde_json::to_string(&vault).unwrap(); let values = parse_and_decrypt_values(&json, &identity).unwrap(); assert_eq!(values.len(), 2); - assert_eq!(values["KEY1"], "val1"); - assert_eq!(values["KEY2"], "val2"); + assert_eq!(values["KEY1"].as_str(), "val1"); + assert_eq!(values["KEY2"].as_str(), "val2"); } #[test] diff --git a/src/lib.rs b/src/lib.rs index 28345c9..5895ea6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -77,6 +77,7 @@ pub fn is_valid_key_name(key: &str) -> bool { use age::secrecy::ExposeSecret; use base64::{Engine, engine::general_purpose::STANDARD as BASE64}; +use zeroize::Zeroizing; // Re-export polymorphic types for consumers. pub use crypto::{MurkIdentity, MurkRecipient}; @@ -110,13 +111,30 @@ pub fn encrypt_value( } /// Decrypt a base64-encoded ciphertext and return plaintext bytes. -pub fn decrypt_value(encoded: &str, identity: &crypto::MurkIdentity) -> Result, MurkError> { +/// +/// The returned buffer is zeroized on drop. +pub fn decrypt_value( + encoded: &str, + identity: &crypto::MurkIdentity, +) -> Result>, MurkError> { let ciphertext = BASE64.decode(encoded).map_err(|e| { MurkError::Crypto(crypto::CryptoError::Decrypt(format!("invalid base64: {e}"))) })?; Ok(crypto::decrypt(&ciphertext, identity)?) } +/// Validate decrypted bytes as UTF-8 and return a zeroizing `String`. +/// +/// The returned `String` and the input `&[u8]` are both zeroized when dropped +/// (assuming the caller holds the bytes inside a `Zeroizing`), so plaintext +/// never escapes to a non-zeroed buffer. +pub(crate) fn plaintext_bytes_to_zeroizing_string( + bytes: &[u8], +) -> Result, std::str::Utf8Error> { + let s = std::str::from_utf8(bytes)?; + Ok(Zeroizing::new(s.to_owned())) +} + /// Read a vault file from disk. /// /// This is a thin wrapper around `vault::read` for a convenient string-path API. @@ -225,7 +243,7 @@ pub fn decrypt_vault( }; // Decrypt shared values (skip scoped-only entries with empty shared ciphertext). - let mut values = HashMap::new(); + let mut values: HashMap> = HashMap::new(); for (key, entry) in &vault.secrets { if entry.shared.is_empty() { continue; @@ -235,21 +253,23 @@ pub fn decrypt_vault( "you are not a recipient of this vault. Run `murk circle` to check, or ask a recipient to authorize you".into() )) })?; - let value = String::from_utf8(plaintext) + let value = plaintext_bytes_to_zeroizing_string(&plaintext) .map_err(|e| MurkError::Secret(format!("invalid UTF-8 in secret {key}: {e}")))?; values.insert(key.clone(), value); } // Decrypt our scoped (mote) overrides. - let mut scoped = HashMap::new(); + let mut scoped: HashMap>> = HashMap::new(); for (key, entry) in &vault.secrets { if let Some(encoded) = entry.scoped.get(&pubkey) - && let Ok(value) = decrypt_value(encoded, identity) - .and_then(|pt| String::from_utf8(pt).map_err(|e| MurkError::Secret(e.to_string()))) + && let Ok(value) = decrypt_value(encoded, identity).and_then(|pt| { + plaintext_bytes_to_zeroizing_string(&pt) + .map_err(|e| MurkError::Secret(e.to_string())) + }) { scoped .entry(key.clone()) - .or_insert_with(HashMap::new) + .or_default() .insert(pubkey.clone(), value); } } @@ -692,7 +712,7 @@ mod tests { let encoded = encrypt_value(b"hello world", &[recipient]).unwrap(); let decrypted = decrypt_value(&encoded, &identity).unwrap(); - assert_eq!(decrypted, b"hello world"); + assert_eq!(&decrypted[..], b"hello world"); } #[test] @@ -716,8 +736,14 @@ mod tests { // Both can decrypt. let id_a = make_identity(&secret_a); let id_b = make_identity(&secret_b); - assert_eq!(decrypt_value(&encoded, &id_a).unwrap(), b"shared secret"); - assert_eq!(decrypt_value(&encoded, &id_b).unwrap(), b"shared secret"); + assert_eq!( + &decrypt_value(&encoded, &id_a).unwrap()[..], + b"shared secret" + ); + assert_eq!( + &decrypt_value(&encoded, &id_b).unwrap()[..], + b"shared secret" + ); } #[test] @@ -836,7 +862,7 @@ mod tests { let mut recipients_map = HashMap::new(); recipients_map.insert(pubkey.clone(), "alice".into()); let original = types::Murk { - values: HashMap::from([("KEY1".into(), "original".into())]), + values: HashMap::from([("KEY1".into(), crate::testutil::secret("original"))]), recipients: recipients_map.clone(), scoped: HashMap::new(), legacy_mac: false, @@ -849,13 +875,15 @@ mod tests { assert_eq!(vault.secrets["KEY1"].shared, shared); let mut changed = current.clone(); - changed.values.insert("KEY1".into(), "modified".into()); + changed + .values + .insert("KEY1".into(), crate::testutil::secret("modified")); save_vault(path.to_str().unwrap(), &mut vault, &original, &changed).unwrap(); assert_ne!(vault.secrets["KEY1"].shared, shared); let decrypted = decrypt_value(&vault.secrets["KEY1"].shared, &identity).unwrap(); - assert_eq!(decrypted, b"modified"); + assert_eq!(&decrypted[..], b"modified"); fs::remove_dir_all(&dir).unwrap(); } @@ -891,7 +919,7 @@ mod tests { let mut recipients_map = HashMap::new(); recipients_map.insert(pubkey.clone(), "alice".into()); let original = types::Murk { - values: HashMap::from([("KEY1".into(), "val1".into())]), + values: HashMap::from([("KEY1".into(), crate::testutil::secret("val1"))]), recipients: recipients_map.clone(), scoped: HashMap::new(), legacy_mac: false, @@ -899,7 +927,9 @@ mod tests { }; let mut current = original.clone(); - current.values.insert("KEY2".into(), "val2".into()); + current + .values + .insert("KEY2".into(), crate::testutil::secret("val2")); save_vault(path.to_str().unwrap(), &mut vault, &original, ¤t).unwrap(); @@ -947,8 +977,8 @@ mod tests { recipients_map.insert(pubkey.clone(), "alice".into()); let original = types::Murk { values: HashMap::from([ - ("KEY1".into(), "val1".into()), - ("KEY2".into(), "val2".into()), + ("KEY1".into(), crate::testutil::secret("val1")), + ("KEY2".into(), crate::testutil::secret("val2")), ]), recipients: recipients_map.clone(), scoped: HashMap::new(), @@ -999,7 +1029,7 @@ mod tests { let mut recipients_map = HashMap::new(); recipients_map.insert(pubkey1.clone(), "alice".into()); let original = types::Murk { - values: HashMap::from([("KEY1".into(), "val1".into())]), + values: HashMap::from([("KEY1".into(), crate::testutil::secret("val1"))]), recipients: recipients_map, scoped: HashMap::new(), legacy_mac: false, @@ -1010,7 +1040,7 @@ mod tests { current_recipients.insert(pubkey1.clone(), "alice".into()); current_recipients.insert(pubkey2.clone(), "bob".into()); let current = types::Murk { - values: HashMap::from([("KEY1".into(), "val1".into())]), + values: HashMap::from([("KEY1".into(), crate::testutil::secret("val1"))]), recipients: current_recipients, scoped: HashMap::new(), legacy_mac: false, @@ -1023,7 +1053,7 @@ mod tests { let identity1 = make_identity(&secret1); let decrypted = decrypt_value(&vault.secrets["KEY1"].shared, &identity1).unwrap(); - assert_eq!(decrypted, b"val1"); + assert_eq!(&decrypted[..], b"val1"); fs::remove_dir_all(&dir).unwrap(); } @@ -1060,7 +1090,7 @@ mod tests { let mut recipients_map = HashMap::new(); recipients_map.insert(pubkey.clone(), "alice".into()); let original = types::Murk { - values: HashMap::from([("KEY1".into(), "shared_val".into())]), + values: HashMap::from([("KEY1".into(), crate::testutil::secret("shared_val"))]), recipients: recipients_map.clone(), scoped: HashMap::new(), legacy_mac: false, @@ -1070,14 +1100,14 @@ mod tests { // Add a scoped override. let mut current = original.clone(); let mut key_scoped = HashMap::new(); - key_scoped.insert(pubkey.clone(), "my_override".into()); + key_scoped.insert(pubkey.clone(), crate::testutil::secret("my_override")); current.scoped.insert("KEY1".into(), key_scoped); save_vault(path.to_str().unwrap(), &mut vault, &original, ¤t).unwrap(); assert!(vault.secrets["KEY1"].scoped.contains_key(&pubkey)); let scoped_val = decrypt_value(&vault.secrets["KEY1"].scoped[&pubkey], &identity).unwrap(); - assert_eq!(scoped_val, b"my_override"); + assert_eq!(&scoped_val[..], b"my_override"); // Now remove the scoped override. let original_with_scoped = current.clone(); @@ -1132,7 +1162,7 @@ mod tests { let mut recipients_map = HashMap::new(); recipients_map.insert(pubkey.clone(), "alice".into()); let original = types::Murk { - values: HashMap::from([("KEY1".into(), "val1".into())]), + values: HashMap::from([("KEY1".into(), crate::testutil::secret("val1"))]), recipients: recipients_map, scoped: HashMap::new(), legacy_mac: false, @@ -1197,7 +1227,7 @@ mod tests { let mut recipients_map = HashMap::new(); recipients_map.insert(pubkey.clone(), "alice".into()); let original = types::Murk { - values: HashMap::from([("KEY1".into(), "val1".into())]), + values: HashMap::from([("KEY1".into(), crate::testutil::secret("val1"))]), recipients: recipients_map, scoped: HashMap::new(), legacy_mac: false, @@ -1214,7 +1244,7 @@ mod tests { assert!(result.is_ok()); let (_, murk, _) = result.unwrap(); - assert_eq!(murk.values["KEY1"], "val1"); + assert_eq!(murk.values["KEY1"].as_str(), "val1"); fs::remove_dir_all(&dir).unwrap(); } @@ -1255,7 +1285,7 @@ mod tests { let mut recipients_map = HashMap::new(); recipients_map.insert(other_pubkey.clone(), "other".into()); let original = types::Murk { - values: HashMap::from([("KEY1".into(), "val1".into())]), + values: HashMap::from([("KEY1".into(), crate::testutil::secret("val1"))]), recipients: recipients_map, scoped: HashMap::new(), legacy_mac: false, @@ -1369,7 +1399,7 @@ mod tests { let mut recipients_map = HashMap::new(); recipients_map.insert(pubkey.clone(), "alice".into()); let original = types::Murk { - values: HashMap::from([("KEY1".into(), "val1".into())]), + values: HashMap::from([("KEY1".into(), crate::testutil::secret("val1"))]), recipients: recipients_map, scoped: HashMap::new(), legacy_mac: false, diff --git a/src/main.rs b/src/main.rs index 8129faf..cf07c0b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -410,8 +410,9 @@ fn prompt(label: &str, default: Option<&str>) -> String { } /// Generate a BIP39 keypair, write key to ~/.config/murk/keys/, reference in .env. -/// Returns (secret_key, pubkey). -fn generate_and_write_key(vault_name: &str) -> (String, String) { +/// Returns (secret_key, pubkey). Secret key is wrapped in `Zeroizing` so the +/// plaintext clears when the caller drops it. +fn generate_and_write_key(vault_name: &str) -> (zeroize::Zeroizing, String) { eprintln!("{} generating keypair...", "◆".magenta()); let (phrase, secret_key, pubkey) = try_or_die(recovery::generate()); @@ -446,7 +447,7 @@ fn generate_and_write_key(vault_name: &str) -> (String, String) { .yellow() .bold() ); - eprintln!(" {}", phrase.bold()); + eprintln!(" {}", phrase.as_str().bold()); eprintln!(); eprintln!( " {}", @@ -946,9 +947,10 @@ fn cmd_export(tags: &[String], json: bool, vault_path: &str) { if json { let raw = murk_cli::resolve_secrets(&vault, &murk, &pubkey, tags); + // serde_json copies into its own owned String, so zeroization ends here. let map: serde_json::Map = raw .iter() - .map(|(k, v)| (k.clone(), serde_json::Value::String(v.clone()))) + .map(|(k, v)| (k.clone(), serde_json::Value::String(v.to_string()))) .collect(); println!("{}", serde_json::to_string_pretty(&map).unwrap()); } else { @@ -958,7 +960,7 @@ fn cmd_export(tags: &[String], json: bool, vault_path: &str) { eprintln!("{} skipping unsafe key name: {}", "⚠".yellow(), k.bold()); continue; } - println!("export {k}='{escaped}'"); + println!("export {k}='{}'", escaped.as_str()); } } } @@ -993,11 +995,11 @@ fn cmd_edit(key: Option<&str>, scoped: bool, vault_path: &str) { k, if scoped { " (scoped)" } else { "" } ), - vec![(k.to_string(), value)], + vec![(k.to_string(), value)] as Vec<(String, zeroize::Zeroizing)>, ) } else { // All keys: KEY=VALUE format. - let mut entries: Vec<(String, String)> = if scoped { + let mut entries: Vec<(String, zeroize::Zeroizing)> = if scoped { current .scoped .iter() @@ -1027,11 +1029,11 @@ fn cmd_edit(key: Option<&str>, scoped: bool, vault_path: &str) { let single_key = key.is_some(); let buffer = if single_key { - format!("{}{}", header, entries[0].1) + format!("{}{}", header, entries[0].1.as_str()) } else { let mut buf = header; for (k, v) in &entries { - buf.push_str(&format!("{k}={v}\n")); + buf.push_str(&format!("{k}={}\n", v.as_str())); } buf }; @@ -1102,25 +1104,26 @@ fn cmd_edit(key: Option<&str>, scoped: bool, vault_path: &str) { return; } - let old_value = if scoped { + let old_value: Option> = if scoped { current.scoped.get(k).and_then(|m| m.get(&pubkey)).cloned() } else { current.values.get(k).cloned() }; - if old_value.as_deref() == Some(new_value) { + if old_value.as_ref().map(|v| v.as_str()) == Some(new_value) { eprintln!("{} no changes", "◆".magenta()); return; } if scoped { - current - .scoped - .entry(k.into()) - .or_default() - .insert(pubkey.clone(), new_value.to_string()); + current.scoped.entry(k.into()).or_default().insert( + pubkey.clone(), + zeroize::Zeroizing::new(new_value.to_string()), + ); } else { - current.values.insert(k.into(), new_value.to_string()); + current + .values + .insert(k.into(), zeroize::Zeroizing::new(new_value.to_string())); } save_vault(vault_path, &mut vault, &original, ¤t); @@ -1158,7 +1161,8 @@ fn cmd_edit(key: Option<&str>, scoped: bool, vault_path: &str) { } // Compute diff. - let old_entries: std::collections::BTreeMap = entries.into_iter().collect(); + let old_entries: std::collections::BTreeMap> = + entries.into_iter().collect(); let mut added = 0usize; let mut updated = 0usize; let mut removed = 0usize; @@ -1166,16 +1170,18 @@ fn cmd_edit(key: Option<&str>, scoped: bool, vault_path: &str) { // Add or update. for (k, v) in &new_entries { match old_entries.get(k) { - Some(old_v) if old_v == v => {} // Unchanged. + Some(old_v) if old_v.as_str() == v.as_str() => {} // Unchanged. Some(_) => { if scoped { current .scoped .entry(k.clone()) .or_default() - .insert(pubkey.clone(), v.clone()); + .insert(pubkey.clone(), zeroize::Zeroizing::new(v.clone())); } else { - current.values.insert(k.clone(), v.clone()); + current + .values + .insert(k.clone(), zeroize::Zeroizing::new(v.clone())); } updated += 1; } @@ -1185,9 +1191,11 @@ fn cmd_edit(key: Option<&str>, scoped: bool, vault_path: &str) { .scoped .entry(k.clone()) .or_default() - .insert(pubkey.clone(), v.clone()); + .insert(pubkey.clone(), zeroize::Zeroizing::new(v.clone())); } else { - current.values.insert(k.clone(), v.clone()); + current + .values + .insert(k.clone(), zeroize::Zeroizing::new(v.clone())); } // Ensure schema entry exists for new keys. vault @@ -1435,7 +1443,7 @@ fn cmd_diff(git_ref: &str, show_values: bool, json: bool, vault_path: &str) { .output() .unwrap_or_else(|e| die(&format_args!("running git: {e}"), 1)); - let old_values: HashMap = if output.status.success() { + let old_values: HashMap> = if output.status.success() { let old_contents = String::from_utf8_lossy(&output.stdout); match murk_cli::parse_and_decrypt_values(&old_contents, &identity) { Ok(values) => { @@ -1455,25 +1463,26 @@ fn cmd_diff(git_ref: &str, show_values: bool, json: bool, vault_path: &str) { Err(e) => die(&format_args!("parsing vault at {git_ref}: {e}"), 1), } } else { - HashMap::new() + HashMap::>::new() }; let pubkey = identity.pubkey_string().unwrap_or_else(|e| die(&e, 1)); - let current_values: HashMap = + let current_values: HashMap> = murk_cli::resolve_secrets(&_vault, ¤t_murk, &pubkey, &[]) .into_iter() .collect(); let entries = murk_cli::diff_secrets(&old_values, ¤t_values); if json { + // serde_json copies into its own owned String; zeroization ends at this boundary. let list: Vec = entries .iter() .map(|e| { serde_json::json!({ "key": e.key, "kind": format!("{:?}", e.kind).to_lowercase(), - "old_value": e.old_value, - "new_value": e.new_value, + "old_value": e.old_value.as_ref().map(|v| v.as_str()), + "new_value": e.new_value.as_ref().map(|v| v.as_str()), }) }) .collect(); @@ -1487,27 +1496,19 @@ fn cmd_diff(git_ref: &str, show_values: bool, json: bool, vault_path: &str) { } for entry in &entries { + let old = entry.old_value.as_ref().map_or("", |v| v.as_str()); + let new = entry.new_value.as_ref().map_or("", |v| v.as_str()); match entry.kind { DiffKind::Added => { if show_values { - println!( - "{} {} = {}", - "+".magenta().bold(), - entry.key.bold(), - entry.new_value.as_deref().unwrap_or("") - ); + println!("{} {} = {}", "+".magenta().bold(), entry.key.bold(), new); } else { println!("{} {}", "+".magenta().bold(), entry.key.bold()); } } DiffKind::Removed => { if show_values { - println!( - "{} {} = {}", - "-".red().bold(), - entry.key.bold(), - entry.old_value.as_deref().unwrap_or("") - ); + println!("{} {} = {}", "-".red().bold(), entry.key.bold(), old); } else { println!("{} {}", "-".red().bold(), entry.key.bold()); } @@ -1518,9 +1519,9 @@ fn cmd_diff(git_ref: &str, show_values: bool, json: bool, vault_path: &str) { "{} {} {} {} {}", "~".yellow().bold(), entry.key.bold(), - entry.old_value.as_deref().unwrap_or(""), + old, "→".dimmed(), - entry.new_value.as_deref().unwrap_or("") + new ); } else { println!("{} {}", "~".yellow().bold(), entry.key.bold()); @@ -1897,7 +1898,7 @@ fn cmd_restore() { die(&"recovery phrase is required", 1); } - println!("{}", try_or_die(recovery::recover(&phrase))); + println!("{}", try_or_die(recovery::recover(&phrase)).as_str()); } fn cmd_recover() { @@ -1915,7 +1916,7 @@ fn cmd_recover() { println!( "{}", - try_or_die(recovery::phrase_from_key(secret_key.expose_secret())) + try_or_die(recovery::phrase_from_key(secret_key.expose_secret())).as_str() ); } diff --git a/src/python.rs b/src/python.rs index 6fad14a..edc1966 100644 --- a/src/python.rs +++ b/src/python.rs @@ -28,6 +28,9 @@ struct Vault { impl Vault { /// Get a single decrypted secret value. /// Returns the scoped override if one exists, otherwise the shared value. + /// + /// The returned `String` is a plain Python-owned copy — once it crosses + /// the FFI boundary the plaintext is outside murk's zeroization. fn get(&self, key: &str) -> Option { if let Some(value) = self .decrypted @@ -35,15 +38,17 @@ impl Vault { .get(key) .and_then(|m| m.get(&self.pubkey)) { - return Some(value.clone()); + return Some(value.to_string()); } - self.decrypted.values.get(key).cloned() + self.decrypted.values.get(key).map(|v| v.to_string()) } /// Export all secrets as a dict. Scoped values override shared values. fn export(&self) -> HashMap { + // Python dicts own plain Strings — zeroization ends at the FFI boundary. export::resolve_secrets(&self.inner, &self.decrypted, &self.pubkey, &[]) .into_iter() + .map(|(k, v)| (k, v.to_string())) .collect() } diff --git a/src/recipients.rs b/src/recipients.rs index cf3628b..4a5565a 100644 --- a/src/recipients.rs +++ b/src/recipients.rs @@ -420,7 +420,7 @@ mod tests { ); let mut murk = empty_murk(); let mut scoped = HashMap::new(); - scoped.insert(pk2.clone(), "scoped_val".into()); + scoped.insert(pk2.clone(), secret("scoped_val")); murk.scoped.insert("KEY".into(), scoped); revoke_recipient(&mut vault, &mut murk, &pk2).unwrap(); @@ -470,10 +470,10 @@ mod tests { ); let mut murk = empty_murk(); murk.scoped - .insert("DB_URL".into(), HashMap::from([(pk2.clone(), "v".into())])); + .insert("DB_URL".into(), HashMap::from([(pk2.clone(), secret("v"))])); murk.scoped.insert( "API_KEY".into(), - HashMap::from([(pk2.clone(), "v2".into())]), + HashMap::from([(pk2.clone(), secret("v2"))]), ); let result = revoke_recipient(&mut vault, &mut murk, &pk2).unwrap(); diff --git a/src/recovery.rs b/src/recovery.rs index 96a324e..10209a2 100644 --- a/src/recovery.rs +++ b/src/recovery.rs @@ -1,4 +1,5 @@ use bech32::{Bech32, Hrp}; +use zeroize::Zeroizing; /// Errors that can occur during recovery phrase operations. #[derive(Debug)] @@ -26,12 +27,15 @@ const AGE_SECRET_KEY_HRP: Hrp = Hrp::parse_unchecked("age-secret-key-"); /// 24 BIP39 words encode 256 bits (32 bytes) — exactly the size of an /// age x25519 secret key. The mnemonic is a direct encoding of the key /// bytes with no derivation step. Same words, same key, always. -pub fn generate() -> Result<(String, String, String), RecoveryError> { - let entropy: [u8; 32] = rand::random(); - let mnemonic = - bip39::Mnemonic::from_entropy(&entropy).map_err(|e| RecoveryError::Bip39(e.to_string()))?; +/// +/// The mnemonic and secret key are returned in `Zeroizing` wrappers so the +/// plaintext is cleared from memory when dropped. +pub fn generate() -> Result<(Zeroizing, Zeroizing, String), RecoveryError> { + let entropy = Zeroizing::new([0u8; 32].map(|_| rand::random::())); + let mnemonic = bip39::Mnemonic::from_entropy(&*entropy) + .map_err(|e| RecoveryError::Bip39(e.to_string()))?; - let secret_key = bytes_to_age_key(&entropy)?; + let secret_key = bytes_to_age_key(&*entropy)?; let identity = crate::crypto::parse_identity(&secret_key) .map_err(|e| RecoveryError::InvalidKey(e.to_string()))?; @@ -39,38 +43,39 @@ pub fn generate() -> Result<(String, String, String), RecoveryError> { .pubkey_string() .map_err(|e| RecoveryError::InvalidKey(e.to_string()))?; - Ok((mnemonic.to_string(), secret_key, pubkey)) + Ok((Zeroizing::new(mnemonic.to_string()), secret_key, pubkey)) } /// Re-derive the BIP39 24-word mnemonic from an existing MURK_KEY. /// Decodes the Bech32 key back to raw bytes, then encodes as a mnemonic. -pub fn phrase_from_key(secret_key: &str) -> Result { +pub fn phrase_from_key(secret_key: &str) -> Result, RecoveryError> { // age keys are uppercase; bech32 decoding requires lowercase. - let lowercase = secret_key.to_lowercase(); + let lowercase = Zeroizing::new(secret_key.to_lowercase()); let (_, key_bytes) = bech32::decode(&lowercase).map_err(|e| RecoveryError::InvalidKey(e.to_string()))?; + let key_bytes = Zeroizing::new(key_bytes); let mnemonic = bip39::Mnemonic::from_entropy(&key_bytes) .map_err(|e| RecoveryError::Bip39(e.to_string()))?; - Ok(mnemonic.to_string()) + Ok(Zeroizing::new(mnemonic.to_string())) } /// Recover an age secret key from a BIP39 24-word mnemonic phrase. /// Returns the same MURK_KEY that was originally generated. -pub fn recover(phrase: &str) -> Result { +pub fn recover(phrase: &str) -> Result, RecoveryError> { let mnemonic = bip39::Mnemonic::parse_in_normalized(bip39::Language::English, phrase) .map_err(|e| RecoveryError::Bip39(e.to_string()))?; - let entropy = mnemonic.to_entropy(); + let entropy = Zeroizing::new(mnemonic.to_entropy()); bytes_to_age_key(&entropy) } /// Bech32-encode raw key bytes as an AGE-SECRET-KEY-1... string. /// This matches exactly how the age crate encodes keys internally. -fn bytes_to_age_key(key_bytes: &[u8]) -> Result { +fn bytes_to_age_key(key_bytes: &[u8]) -> Result, RecoveryError> { let encoded = bech32::encode::(AGE_SECRET_KEY_HRP, key_bytes) .map_err(|e| RecoveryError::InvalidKey(e.to_string()))?; - let key_str = encoded.to_uppercase(); + let key_str = Zeroizing::new(encoded.to_uppercase()); // Validate by round-tripping through the age crate. crate::crypto::parse_identity(&key_str) diff --git a/src/scan.rs b/src/scan.rs index caec5ed..9391edb 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -2,6 +2,8 @@ use std::collections::BTreeMap; +use zeroize::Zeroizing; + /// A single scan finding: a secret key was found in a file. #[derive(Debug)] pub struct ScanFinding { @@ -18,7 +20,7 @@ pub struct ScanFinding { /// `min_length` are skipped to reduce false positives. pub fn scan_for_leaks( paths: &[&str], - secrets: &BTreeMap, + secrets: &BTreeMap>, min_length: usize, ) -> Vec { let mut findings = Vec::new(); @@ -82,7 +84,10 @@ mod tests { .unwrap(); let mut secrets = BTreeMap::new(); - secrets.insert("DB_PASSWORD".into(), "supersecretvalue123".into()); + secrets.insert( + "DB_PASSWORD".into(), + Zeroizing::new("supersecretvalue123".to_string()), + ); let findings = scan_for_leaks(&[dir.path().to_str().unwrap()], &secrets, 8); assert_eq!(findings.len(), 1); @@ -95,7 +100,7 @@ mod tests { std::fs::write(dir.path().join("file.txt"), "abc").unwrap(); let mut secrets = BTreeMap::new(); - secrets.insert("SHORT".into(), "abc".into()); + secrets.insert("SHORT".into(), Zeroizing::new("abc".to_string())); let findings = scan_for_leaks(&[dir.path().to_str().unwrap()], &secrets, 8); assert!(findings.is_empty()); @@ -107,7 +112,10 @@ mod tests { std::fs::write(dir.path().join("test.murk"), "supersecretvalue123").unwrap(); let mut secrets = BTreeMap::new(); - secrets.insert("KEY".into(), "supersecretvalue123".into()); + secrets.insert( + "KEY".into(), + Zeroizing::new("supersecretvalue123".to_string()), + ); let findings = scan_for_leaks(&[dir.path().to_str().unwrap()], &secrets, 8); assert!(findings.is_empty()); @@ -121,7 +129,10 @@ mod tests { std::fs::write(hidden.join("leaked.txt"), "supersecretvalue123").unwrap(); let mut secrets = BTreeMap::new(); - secrets.insert("KEY".into(), "supersecretvalue123".into()); + secrets.insert( + "KEY".into(), + Zeroizing::new("supersecretvalue123".to_string()), + ); let findings = scan_for_leaks(&[dir.path().to_str().unwrap()], &secrets, 8); assert!(findings.is_empty()); @@ -147,8 +158,8 @@ mod tests { .unwrap(); let mut secrets = BTreeMap::new(); - secrets.insert("K1".into(), "secretvalue1".into()); - secrets.insert("K2".into(), "secretvalue2".into()); + secrets.insert("K1".into(), Zeroizing::new("secretvalue1".to_string())); + secrets.insert("K2".into(), Zeroizing::new("secretvalue2".to_string())); let findings = scan_for_leaks(&[dir.path().to_str().unwrap()], &secrets, 8); assert_eq!(findings.len(), 2); diff --git a/src/secrets.rs b/src/secrets.rs index 19fbde2..fee59d6 100644 --- a/src/secrets.rs +++ b/src/secrets.rs @@ -1,5 +1,7 @@ //! Secret CRUD operations on the in-memory `Murk` state. +use zeroize::Zeroizing; + use crate::{crypto, now_utc, types}; /// Add or update a secret in the working state. @@ -20,9 +22,10 @@ pub fn add_secret( murk.scoped .entry(key.into()) .or_default() - .insert(pubkey, value.into()); + .insert(pubkey, Zeroizing::new(value.to_owned())); } else { - murk.values.insert(key.into(), value.into()); + murk.values + .insert(key.into(), Zeroizing::new(value.to_owned())); } let is_new = !vault.schema.contains_key(key); @@ -68,7 +71,7 @@ pub fn get_secret<'a>(murk: &'a types::Murk, key: &str, pubkey: &str) -> Option< if let Some(value) = murk.scoped.get(key).and_then(|m| m.get(pubkey)) { return Some(value.as_str()); } - murk.values.get(key).map(String::as_str) + murk.values.get(key).map(|v| v.as_str()) } /// Return key names from the vault schema, optionally filtered by tags. @@ -93,7 +96,8 @@ pub fn import_secrets( let now = now_utc(); let mut imported = Vec::new(); for (key, value) in pairs { - murk.values.insert(key.clone(), value.clone()); + murk.values + .insert(key.clone(), Zeroizing::new(value.clone())); if let Some(entry) = vault.schema.get_mut(key.as_str()) { entry.updated = Some(now.clone()); @@ -169,7 +173,7 @@ mod tests { ); assert!(needs_hint); - assert_eq!(murk.values["KEY"], "value"); + assert_eq!(murk.values["KEY"].as_str(), "value"); assert!(vault.schema.contains_key("KEY")); assert!(vault.schema["KEY"].description.is_empty()); } @@ -215,7 +219,7 @@ mod tests { ); assert!(!murk.values.contains_key("KEY")); - assert_eq!(murk.scoped["KEY"][&pubkey], "scoped_val"); + assert_eq!(murk.scoped["KEY"][&pubkey].as_str(), "scoped_val"); } #[test] @@ -288,9 +292,9 @@ mod tests { }, ); let mut murk = empty_murk(); - murk.values.insert("KEY".into(), "val".into()); + murk.values.insert("KEY".into(), secret("val")); let mut scoped = HashMap::new(); - scoped.insert("age1pk".into(), "scoped_val".into()); + scoped.insert("age1pk".into(), secret("scoped_val")); murk.scoped.insert("KEY".into(), scoped); remove_secret(&mut vault, &mut murk, "KEY"); @@ -303,7 +307,7 @@ mod tests { #[test] fn get_secret_shared_value() { let mut murk = empty_murk(); - murk.values.insert("KEY".into(), "shared_val".into()); + murk.values.insert("KEY".into(), secret("shared_val")); assert_eq!(get_secret(&murk, "KEY", "age1pk"), Some("shared_val")); } @@ -311,9 +315,9 @@ mod tests { #[test] fn get_secret_scoped_overrides_shared() { let mut murk = empty_murk(); - murk.values.insert("KEY".into(), "shared_val".into()); + murk.values.insert("KEY".into(), secret("shared_val")); let mut scoped = HashMap::new(); - scoped.insert("age1pk".into(), "scoped_val".into()); + scoped.insert("age1pk".into(), secret("scoped_val")); murk.scoped.insert("KEY".into(), scoped); assert_eq!(get_secret(&murk, "KEY", "age1pk"), Some("scoped_val")); @@ -476,7 +480,7 @@ mod tests { &[], &identity, ); - assert_eq!(murk.values["KEY"], "shared_val"); + assert_eq!(murk.values["KEY"].as_str(), "shared_val"); add_secret( &mut vault, @@ -489,8 +493,8 @@ mod tests { &identity, ); // Shared value still exists, scoped override added. - assert_eq!(murk.values["KEY"], "shared_val"); - assert_eq!(murk.scoped["KEY"][&pubkey], "scoped_val"); + assert_eq!(murk.values["KEY"].as_str(), "shared_val"); + assert_eq!(murk.scoped["KEY"][&pubkey].as_str(), "scoped_val"); } #[test] @@ -510,7 +514,7 @@ mod tests { &[], &identity, ); - assert_eq!(murk.values["KEY"], ""); + assert_eq!(murk.values["KEY"].as_str(), ""); } #[test] @@ -525,8 +529,8 @@ mod tests { let imported = import_secrets(&mut vault, &mut murk, &pairs); assert_eq!(imported, vec!["KEY1", "KEY2"]); - assert_eq!(murk.values["KEY1"], "val1"); - assert_eq!(murk.values["KEY2"], "val2"); + assert_eq!(murk.values["KEY1"].as_str(), "val1"); + assert_eq!(murk.values["KEY2"].as_str(), "val2"); assert!(vault.schema.contains_key("KEY1")); assert!(vault.schema.contains_key("KEY2")); } @@ -548,7 +552,7 @@ mod tests { let pairs = vec![("KEY1".into(), "new_val".into())]; import_secrets(&mut vault, &mut murk, &pairs); - assert_eq!(murk.values["KEY1"], "new_val"); + assert_eq!(murk.values["KEY1"].as_str(), "new_val"); assert_eq!(vault.schema["KEY1"].description, "existing desc"); } diff --git a/src/testutil.rs b/src/testutil.rs index 929cdc5..da98944 100644 --- a/src/testutil.rs +++ b/src/testutil.rs @@ -3,6 +3,8 @@ use std::collections::{BTreeMap, HashMap}; use std::sync::Mutex; +use zeroize::Zeroizing; + /// Process-global lock for tests that mutate env vars (MURK_KEY, MURK_KEY_FILE). pub static ENV_LOCK: Mutex<()> = Mutex::new(()); @@ -50,3 +52,8 @@ pub fn empty_murk() -> types::Murk { github_pins: HashMap::new(), } } + +/// Wrap a string in `Zeroizing` for test inserts into `Murk.values` / `Murk.scoped`. +pub fn secret(s: &str) -> Zeroizing { + Zeroizing::new(s.to_string()) +} diff --git a/src/types.rs b/src/types.rs index 095ae5f..1e9ba80 100644 --- a/src/types.rs +++ b/src/types.rs @@ -1,6 +1,7 @@ use std::collections::{BTreeMap, HashMap}; use serde::{Deserialize, Serialize}; +use zeroize::Zeroizing; /// Current vault format version. pub const VAULT_VERSION: &str = "2.0"; @@ -78,13 +79,14 @@ pub struct Meta { #[derive(Debug, Clone)] pub struct Murk { - /// Decrypted shared values. - pub values: HashMap, + /// Decrypted shared values. Wrapped in `Zeroizing` so plaintext is cleared + /// from memory when the `Murk` is dropped. + pub values: HashMap>, /// Pubkey → display name (from meta). pub recipients: HashMap, /// Scoped overrides: key → { pubkey → decrypted value }. /// Only contains entries decryptable by the current identity. - pub scoped: HashMap>, + pub scoped: HashMap>>, /// True if the vault uses a legacy unkeyed MAC (sha256/sha256v2). pub legacy_mac: bool, /// Pinned GitHub key fingerprints (carried from meta). From 8facc2a0ea0e1307bd9aaf29ec709360c0c1ae91 Mon Sep 17 00:00:00 2001 From: Mickey Scherrer <5324300+iicky@users.noreply.github.com> Date: Thu, 23 Apr 2026 08:36:24 -0400 Subject: [PATCH 2/2] bump rustls-webpki to 0.103.13 for RUSTSEC advisory --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c78a716..b09ac17 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2063,9 +2063,9 @@ dependencies = [ [[package]] name = "rustls-webpki" -version = "0.103.12" +version = "0.103.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8279bb85272c9f10811ae6a6c547ff594d6a7f3c6c6b02ee9726d1d0dcfcdd06" +checksum = "61c429a8649f110dddef65e2a5ad240f747e85f7758a6bccc7e5777bd33f756e" dependencies = [ "ring", "rustls-pki-types",