diff --git a/Cargo.lock b/Cargo.lock index 4182cee1..b5d44ca9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2816,6 +2816,7 @@ dependencies = [ "toml 0.8.8", "totp-rs", "whoami", + "zeroize", ] [[package]] @@ -2837,6 +2838,7 @@ dependencies = [ "terminal_size", "toml 0.8.8", "unic-langid", + "zeroize", ] [[package]] @@ -4209,3 +4211,23 @@ dependencies = [ "quote", "syn 2.0.48", ] + +[[package]] +name = "zeroize" +version = "1.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "525b4ec142c6b68a2d10f01f7bbf6755599ca3f81ea53b8431b7dd348f5fdb2d" +dependencies = [ + "zeroize_derive", +] + +[[package]] +name = "zeroize_derive" +version = "1.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce36e65b0d2999d2aafac989fb249189a141aee1f53c612c1f37d72631959f69" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.48", +] diff --git a/Cargo.toml b/Cargo.toml index 90c7884f..a91eacd6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,7 @@ base64 = "0.21.4" sha2 = "0.10.8" sha1 = "0.10.6" hmac = "0.12.1" +zeroize = { version = "1.7.0", features = ["zeroize_derive", "alloc"] } [dependencies.config] version = "0.11.0" diff --git a/cursive/Cargo.toml b/cursive/Cargo.toml index 6d6d2dd7..b7e90067 100644 --- a/cursive/Cargo.toml +++ b/cursive/Cargo.toml @@ -21,6 +21,7 @@ lazy_static = "1.4.0" toml = "0.8.2" terminal_size = "0.3.0" hex = "0.4.3" +zeroize = { version = "1.7.0", features = ["zeroize_derive", "alloc"] } [dependencies.config] version = "0.11.0" diff --git a/cursive/src/helpers.rs b/cursive/src/helpers.rs index 7267291b..a0500897 100644 --- a/cursive/src/helpers.rs +++ b/cursive/src/helpers.rs @@ -52,7 +52,7 @@ pub fn errorbox(ui: &mut Cursive, err: &pass::Error) { } /// Copies content to the clipboard. -pub fn set_clipboard(content: String) -> Result<()> { +pub fn set_clipboard(content: &String) -> Result<()> { Ok(CLIPBOARD.lock().unwrap().set_text(content)?) } diff --git a/cursive/src/main.rs b/cursive/src/main.rs index 4205b881..6a4750f5 100644 --- a/cursive/src/main.rs +++ b/cursive/src/main.rs @@ -50,6 +50,7 @@ mod helpers; mod wizard; use lazy_static::lazy_static; +use zeroize::Zeroize; use crate::helpers::{get_value_from_input, is_checkbox_checked, is_radio_button_selected}; @@ -141,7 +142,9 @@ fn copy(ui: &mut Cursive, store: PasswordStoreType) { return; } if let Err(err) = || -> pass::Result<()> { - helpers::set_clipboard(sel.unwrap().secret(&*store.lock()?.lock()?)?)?; + let mut secret: String = sel.unwrap().secret(&*store.lock()?.lock()?)?; + helpers::set_clipboard(&secret)?; + secret.zeroize(); Ok(()) }() { helpers::errorbox(ui, &err); @@ -150,7 +153,7 @@ fn copy(ui: &mut Cursive, store: PasswordStoreType) { thread::spawn(|| { thread::sleep(time::Duration::from_secs(40)); - helpers::set_clipboard(String::new()).unwrap(); + helpers::set_clipboard(&String::new()).unwrap(); }); ui.call_on_name("status_bar", |l: &mut TextView| { l.set_content(CATALOG.gettext("Copied password to copy buffer for 40 seconds")); @@ -167,7 +170,10 @@ fn copy_first_line(ui: &mut Cursive, store: PasswordStoreType) { return; } if let Err(err) = || -> pass::Result<()> { - helpers::set_clipboard(sel.unwrap().secret(&*store.lock()?.lock()?)?)?; + //Wait, isn't this supposed to be a call to password()? + let mut secret = sel.unwrap().secret(&*store.lock()?.lock()?)?; + helpers::set_clipboard(&secret)?; + secret.zeroize(); Ok(()) }() { helpers::errorbox(ui, &err); @@ -176,7 +182,7 @@ fn copy_first_line(ui: &mut Cursive, store: PasswordStoreType) { thread::spawn(|| { thread::sleep(time::Duration::from_secs(40)); - helpers::set_clipboard(String::new()).unwrap(); + helpers::set_clipboard(&String::new()).unwrap(); }); ui.call_on_name("status_bar", |l: &mut TextView| { l.set_content(CATALOG.gettext("Copied password to copy buffer for 40 seconds")); @@ -193,7 +199,9 @@ fn copy_mfa(ui: &mut Cursive, store: PasswordStoreType) { return; } if let Err(err) = || -> pass::Result<()> { - helpers::set_clipboard(sel.unwrap().mfa(&*store.lock()?.lock()?)?)?; + let mut secret = sel.unwrap().mfa(&*store.lock()?.lock()?)?; + helpers::set_clipboard(&secret)?; + secret.zeroize(); Ok(()) }() { helpers::errorbox(ui, &err); @@ -218,7 +226,7 @@ fn copy_name(ui: &mut Cursive) { if let Err(err) = || -> pass::Result<()> { let name = sel.name.split('/').next_back(); - helpers::set_clipboard(name.unwrap_or("").to_string())?; + helpers::set_clipboard(&name.unwrap_or("").to_string())?; Ok(()) }() { helpers::errorbox(ui, &err); @@ -395,7 +403,7 @@ fn open(ui: &mut Cursive, store: PasswordStoreType) -> Result<()> { let password_entry = password_entry_opt.unwrap(); - let password = { + let mut password = { match password_entry.secret(&*store.lock()?.lock()?) { Ok(p) => p, Err(err) => { @@ -404,35 +412,40 @@ fn open(ui: &mut Cursive, store: PasswordStoreType) -> Result<()> { } } }; - let d = Dialog::around(TextArea::new().content(password).with_name("editbox")) + let d = Dialog::around(TextArea::new().content(&password).with_name("editbox")) .button(CATALOG.gettext("Save"), move |s| { - let new_password = s + let mut new_secret = s .call_on_name("editbox", |e: &mut TextArea| e.get_content().to_string()) .unwrap(); - if new_password.contains("otpauth://") { + if new_secret.contains("otpauth://") { let store = store.clone(); let d = Dialog::around(TextView::new(CATALOG.gettext("It seems like you are trying to save a TOTP code to the password store. This will reduce your 2FA solution to just 1FA, do you want to proceed?"))) .button(CATALOG.gettext("Save"), move |s| { - do_password_save(s, &new_password, store.clone(), true); + let mut confirmed_new_secret = s + .call_on_name("editbox", |e: &mut TextArea| e.get_content().to_string()) + .unwrap(); + do_password_save(s, &confirmed_new_secret, store.clone(), true); + confirmed_new_secret.zeroize(); }) .dismiss_button(CATALOG.gettext("Close")); let ev = OnEventView::new(d).on_event(Key::Esc, |s| { s.pop_layer(); }); - s.add_layer(ev); } else { - do_password_save(s, &new_password, store.clone(), false); + do_password_save(s, &new_secret, store.clone(), false); }; + new_secret.zeroize(); }) .button(CATALOG.gettext("Generate"), move |s| { - let new_password = ripasso::words::generate_password(6); + let mut new_password = ripasso::words::generate_password(6); s.call_on_name("editbox", |e: &mut TextArea| { - e.set_content(new_password); + e.set_content(&new_password); }); + new_password.zeroize(); }) .dismiss_button(CATALOG.gettext("Close")); @@ -441,7 +454,7 @@ fn open(ui: &mut Cursive, store: PasswordStoreType) -> Result<()> { }); ui.add_layer(ev); - + password.zeroize(); Ok(()) } diff --git a/cursive/src/tests/main.rs b/cursive/src/tests/main.rs index 1a78ae77..787d54bf 100644 --- a/cursive/src/tests/main.rs +++ b/cursive/src/tests/main.rs @@ -193,6 +193,8 @@ fn substr_overlong() { #[test] fn create_label_basic() { + // TODO: Fix this test so that time zones don't mess with it. + let p = pass::PasswordEntry::new( &PathBuf::from("/tmp/"), &PathBuf::from("file.gpg"), diff --git a/src/crypto.rs b/src/crypto.rs index 0bffc2b4..fd477553 100644 --- a/src/crypto.rs +++ b/src/crypto.rs @@ -26,6 +26,7 @@ use sequoia_openpgp::{ types::{RevocationStatus, SymmetricAlgorithm}, Cert, KeyHandle, }; +use zeroize::Zeroize; pub use crate::error::{Error, Result}; use crate::{ @@ -227,7 +228,9 @@ impl Crypto for GpgMe { let mut ctx = gpgme::Context::from_protocol(gpgme::Protocol::OpenPgp)?; let mut output = Vec::new(); ctx.decrypt(ciphertext, &mut output)?; - Ok(String::from_utf8(output)?) + let result = String::from_utf8(output.to_vec())?; + output.zeroize(); + Ok(result) } fn encrypt_string(&self, plaintext: &str, recipients: &[Recipient]) -> Result> { @@ -249,7 +252,6 @@ impl Crypto for GpgMe { &mut output, gpgme::EncryptFlags::NO_COMPRESS, )?; - Ok(output) } @@ -756,8 +758,9 @@ impl Crypto for Sequoia { // Decrypt the data. std::io::copy(&mut decryptor, &mut sink).unwrap(); - - Ok(std::str::from_utf8(&sink).unwrap().to_owned()) + let result = std::str::from_utf8(&sink).unwrap().to_owned(); + sink.zeroize(); + Ok(result) } else { // Make a helper that that feeds the recipient's secret key to the // decryptor. @@ -776,8 +779,9 @@ impl Crypto for Sequoia { // Decrypt the data. std::io::copy(&mut decryptor, &mut sink)?; - - Ok(std::str::from_utf8(&sink)?.to_owned()) + let result = std::str::from_utf8(&sink)?.to_owned(); + sink.zeroize(); + Ok(result) } } diff --git a/src/pass.rs b/src/pass.rs index c34789aa..b9cae17f 100644 --- a/src/pass.rs +++ b/src/pass.rs @@ -26,6 +26,7 @@ use std::{ use chrono::prelude::*; use totp_rs::TOTP; +use zeroize::Zeroize; use crate::{ crypto::{Crypto, CryptoImpl, GpgMe, Sequoia, VerificationError}, @@ -692,7 +693,9 @@ impl PasswordStore { fn reencrypt_all_password_entries(&self) -> Result<()> { let mut names: Vec = Vec::new(); for entry in self.all_passwords()? { - entry.update_internal(&entry.secret(self)?, self)?; + let mut secret = entry.secret(self)?; + entry.update_internal(&secret, self)?; + secret.zeroize(); names.push(append_extension(PathBuf::from(&entry.name), ".gpg")); } names.push(PathBuf::from(".gpg-id")); @@ -785,12 +788,13 @@ impl PasswordStore { fs::create_dir_all(&new_path_dir)?; let mut file = std::fs::File::create(&new_path)?; - let secret = self.crypto.decrypt_string(&std::fs::read(&old_path)?)?; + let mut secret = self.crypto.decrypt_string(&std::fs::read(&old_path)?)?; let new_recipients = Recipient::all_recipients( &self.recipients_file_for_dir(&new_path)?, self.crypto.as_ref(), )?; file.write_all(&self.crypto.encrypt_string(&secret, &new_recipients)?)?; + secret.zeroize(); std::fs::remove_file(&old_path)?; if self.repo().is_ok() { @@ -1014,14 +1018,17 @@ impl PasswordEntry { /// # Errors /// Returns an `Err` if the decryption fails pub fn password(&self, store: &PasswordStore) -> Result { - Ok(self.secret(store)?.split('\n').take(1).collect()) + let mut secret = self.secret(store)?; + let password: String = secret.split('\n').take(1).collect(); + secret.zeroize(); + Ok(password) } /// decrypts and returns a TOTP code if the entry contains a otpauth:// url /// # Errors /// Returns an `Err` if the code generation fails pub fn mfa(&self, store: &PasswordStore) -> Result { - let secret = self.secret(store)?; + let mut secret = self.secret(store)?; if let Some(start_pos) = secret.find("otpauth://") { let end_pos = { @@ -1035,12 +1042,15 @@ impl PasswordEntry { end_pos }; let totp = TOTP::from_url(&secret[start_pos..end_pos])?; + secret.zeroize(); Ok(totp.generate_current()?) } else { + secret.zeroize(); Err(Error::Generic("No otpauth:// url in secret")) } } + /// All calls to this function must be followed by secret.zeroize() fn update_internal(&self, secret: &str, store: &PasswordStore) -> Result<()> { if !store.valid_gpg_signing_keys.is_empty() { store.verify_gpg_id_files()?; @@ -1058,8 +1068,9 @@ impl PasswordEntry { /// is supplied. /// # Errors /// Returns an `Err` if the update fails. - pub fn update(&self, secret: String, store: &PasswordStore) -> Result<()> { + pub fn update(&self, mut secret: String, store: &PasswordStore) -> Result<()> { self.update_internal(&secret, store)?; + secret.zeroize(); if store.repo().is_err() { return Ok(()); diff --git a/src/tests/pass.rs b/src/tests/pass.rs index e082dcce..8e94c029 100644 --- a/src/tests/pass.rs +++ b/src/tests/pass.rs @@ -1184,9 +1184,10 @@ fn decrypt_password_multiline() -> Result<()> { user_home: None, }; - let res = pe.password(&store).unwrap(); + let mut res = pe.password(&store).unwrap(); assert_eq!("row one", res); + res.zeroize(); Ok(()) }