diff --git a/node-data/Cargo.toml b/node-data/Cargo.toml index 40b8889bda..cc0dedf891 100644 --- a/node-data/Cargo.toml +++ b/node-data/Cargo.toml @@ -27,6 +27,7 @@ bs58 = { workspace = true } tracing = { workspace = true } anyhow = { workspace = true } thiserror = { workspace = true } +zeroize = { workspace = true } # faker feature dependencies fake = { workspace = true, features = ['derive'], optional = true } diff --git a/node-data/src/bls.rs b/node-data/src/bls.rs index 9ae508d452..6922a100e3 100644 --- a/node-data/src/bls.rs +++ b/node-data/src/bls.rs @@ -23,6 +23,7 @@ use serde::{Deserialize, Serialize}; use serde_with::base64::Base64; use serde_with::serde_as; use sha2::{Digest, Sha256}; +use zeroize::Zeroize; pub const PUBLIC_BLS_SIZE: usize = BlsPublicKey::SIZE; @@ -165,7 +166,7 @@ fn read_from_file( ) })?; - let aes_key = hash_sha256(pwd); + let aes_key = hash_sha256(pwd.as_bytes()); let bytes = decrypt(&ciphertext[..], &aes_key) .map_err(|e| anyhow::anyhow!("Invalid consensus keys password {e}"))?; @@ -187,30 +188,32 @@ pub fn save_consensus_keys( filename: &str, pk: &BlsPublicKey, sk: &BlsSecretKey, - pwd: &str, + pwd: &[u8], ) -> Result<(PathBuf, PathBuf), ConsensusKeysError> { let path = path.join(filename); let bytes = pk.to_bytes(); fs::write(path.with_extension("cpk"), bytes)?; - let bls = BlsKeyPair { + let mut bls = BlsKeyPair { public_key_bls: pk.to_bytes().to_vec(), secret_key_bls: sk.to_bytes().to_vec(), }; - let json = serde_json::to_string(&bls)?; - let mut bytes = json.as_bytes().to_vec(); + let mut bytes = serde_json::to_vec(&bls)?; let aes_key = hash_sha256(pwd); - bytes = encrypt(&bytes, &aes_key)?; + let encrypted_bytes = encrypt(&bytes, &aes_key)?; + + fs::write(path.with_extension("keys"), encrypted_bytes)?; - fs::write(path.with_extension("keys"), bytes)?; + bls.secret_key_bls.zeroize(); + bytes.zeroize(); Ok((path.with_extension("keys"), path.with_extension("cpk"))) } -fn hash_sha256(pwd: &str) -> Vec { +fn hash_sha256(pwd: &[u8]) -> Vec { let mut hasher = Sha256::new(); - hasher.update(pwd.as_bytes()); + hasher.update(pwd); hasher.finalize().to_vec() } @@ -277,7 +280,7 @@ mod tests { let pk = BlsPublicKey::from(&sk); let pwd = "password"; - save_consensus_keys(dir.path(), "consensus", &pk, &sk, pwd)?; + save_consensus_keys(dir.path(), "consensus", &pk, &sk, pwd.as_bytes())?; let keys_path = dir.path().join("consensus.keys"); let (loaded_sk, loaded_pk) = load_keys( keys_path diff --git a/rusk-wallet/CHANGELOG.md b/rusk-wallet/CHANGELOG.md index eeabba3214..5c5595db79 100644 --- a/rusk-wallet/CHANGELOG.md +++ b/rusk-wallet/CHANGELOG.md @@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add deploy contract output (display the new contractId) - Add optional deposit to ContractCall [#3650] - Add pagination for transaction history to not pollute the stdout [#3292] +- Add zeroization for passwords and AES keys to prevent data leaks [#3687] ### Changed @@ -99,6 +100,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#3704]: https://github.com/dusk-network/rusk/issues/3704 [#3702]: https://github.com/dusk-network/rusk/issues/3702 [#3700]: https://github.com/dusk-network/rusk/issues/3700 +[#3687]: https://github.com/dusk-network/rusk/issues/3687 [#3650]: https://github.com/dusk-network/rusk/issues/3650 [#3623]: https://github.com/dusk-network/rusk/issues/3623 [#3602]: https://github.com/dusk-network/rusk/issues/3602 diff --git a/rusk-wallet/src/bin/command.rs b/rusk-wallet/src/bin/command.rs index c3bfb77aa7..8e405717d6 100644 --- a/rusk-wallet/src/bin/command.rs +++ b/rusk-wallet/src/bin/command.rs @@ -12,6 +12,7 @@ pub use history::TransactionHistory; #[cfg(all(test, feature = "e2e-test"))] mod tests; +use std::borrow::Borrow; use std::fmt; use std::fs::File; use std::io::Write; @@ -41,6 +42,7 @@ use wallet_core::BalanceInfo; use crate::io::prompt; use crate::prompt::Prompt; use crate::settings::Settings; +use crate::zeroizing_bytes::ZeroizingBytes; use crate::{WalletFile, WalletPath}; /// Commands that can be run against the Dusk wallet @@ -336,8 +338,8 @@ pub(crate) enum Command { name: Option, /// Password for the exported keys [default: env(RUSK_WALLET_PWD)] - #[arg(short, long, env = "RUSK_WALLET_EXPORT_PWD")] - export_pwd: Option, + #[arg(short, long, env = "RUSK_WALLET_EXPORT_PWD", value_parser = clap::value_parser!(ZeroizingBytes))] + export_pwd: Option, }, /// Show current settings @@ -542,7 +544,7 @@ impl Command { let pwd = match export_pwd { Some(pwd) => pwd, None => match settings.password.as_ref() { - Some(p) => p.to_string(), + Some(p) => p.clone(), None => prompt::ask_pwd( "Provide a password for your provisioner keys", )?, @@ -555,7 +557,7 @@ impl Command { profile_idx, &dir, name, - &pwd, + pwd.borrow(), )?; Ok(RunResult::ExportedKeys(pub_key, key_pair)) @@ -786,7 +788,7 @@ impl Command { pub(crate) fn run_create( skip_recovery: bool, seed_file: &Option, - password: &Option, + password: &Option, wallet_path: &WalletPath, prompter: &dyn Prompt, ) -> anyhow::Result> { diff --git a/rusk-wallet/src/bin/command/tests/utils.rs b/rusk-wallet/src/bin/command/tests/utils.rs index 85464880fb..dd3b63f642 100644 --- a/rusk-wallet/src/bin/command/tests/utils.rs +++ b/rusk-wallet/src/bin/command/tests/utils.rs @@ -19,6 +19,7 @@ use url::Url; use super::*; use crate::command::history::TransactionDirection; use crate::settings::{LogLevel, Logging}; +use crate::zeroizing_bytes::ZeroizingBytes; use crate::{connect, status, LogFormat}; #[derive(Default)] @@ -29,8 +30,8 @@ struct FakePrompter { impl Prompt for FakePrompter { fn create_new_password( &self, - ) -> anyhow::Result { - Ok("password".to_string()) + ) -> anyhow::Result { + Ok(ZeroizingBytes::from("password".to_string())) } fn prompt_text(&self, _msg: &str) -> inquire::error::InquireResult { diff --git a/rusk-wallet/src/bin/io/args.rs b/rusk-wallet/src/bin/io/args.rs index 4b4b4f69a5..8aeafcdec1 100644 --- a/rusk-wallet/src/bin/io/args.rs +++ b/rusk-wallet/src/bin/io/args.rs @@ -9,6 +9,7 @@ use std::path::PathBuf; use clap::{arg, Parser}; use crate::settings::{LogFormat, LogLevel}; +use crate::zeroizing_bytes::ZeroizingBytes; use crate::Command; #[derive(Parser, Debug)] @@ -27,8 +28,8 @@ pub(crate) struct WalletArgs { pub network: Option, /// Set the password for wallet's creation - #[arg(long, env = "RUSK_WALLET_PWD")] - pub password: Option, + #[arg(long, env = "RUSK_WALLET_PWD", value_parser = clap::value_parser!(ZeroizingBytes))] + pub password: Option, /// The state server fully qualified URL #[arg(long)] diff --git a/rusk-wallet/src/bin/io/prompt.rs b/rusk-wallet/src/bin/io/prompt.rs index ea3cfc169f..6c19c408a6 100644 --- a/rusk-wallet/src/bin/io/prompt.rs +++ b/rusk-wallet/src/bin/io/prompt.rs @@ -4,6 +4,7 @@ // // Copyright (c) DUSK NETWORK. All rights reserved. +use std::borrow::Borrow; use std::fmt::Display; use std::path::PathBuf; use std::str::FromStr; @@ -35,10 +36,11 @@ use rusk_wallet::{PBKDF2_ROUNDS, SALT_SIZE}; use sha2::{Digest, Sha256}; use crate::command::TransactionHistory; +use crate::zeroizing_bytes::ZeroizingBytes; pub(crate) trait Prompt { /// Prompt the user to enter a password - fn create_new_password(&self) -> InquireResult { + fn create_new_password(&self) -> InquireResult { create_new_password() } @@ -52,17 +54,17 @@ pub(crate) struct Prompter; impl Prompt for Prompter {} -pub(crate) fn ask_pwd(msg: &str) -> Result { +pub(crate) fn ask_pwd(msg: &str) -> Result { let pwd = Password::new(msg) .with_display_toggle_enabled() .without_confirmation() .with_display_mode(PasswordDisplayMode::Masked) .prompt(); - pwd + pwd.map(ZeroizingBytes::from) } -pub(crate) fn create_new_password() -> Result { +pub(crate) fn create_new_password() -> Result { let pwd = Password::new("Password:") .with_display_toggle_enabled() .with_display_mode(PasswordDisplayMode::Hidden) @@ -70,19 +72,18 @@ pub(crate) fn create_new_password() -> Result { .with_custom_confirmation_error_message("The passwords doesn't match") .prompt(); - pwd + pwd.map(ZeroizingBytes::from) } /// Request the user to authenticate with a password and return the derived key pub(crate) fn derive_key_from_password( msg: &str, - password: &Option, + password: &Option, salt: Option<&[u8; SALT_SIZE]>, file_version: DatFileVersion, -) -> anyhow::Result> { +) -> anyhow::Result { let pwd = match password.as_ref() { - Some(p) => p.to_string(), - + Some(p) => p.clone(), None => ask_pwd(msg)?, }; @@ -91,13 +92,13 @@ pub(crate) fn derive_key_from_password( /// Request the user to create a wallet password and return the derived key pub(crate) fn derive_key_from_new_password( - password: &Option, + password: &Option, salt: Option<&[u8; SALT_SIZE]>, file_version: DatFileVersion, prompter: &dyn Prompt, -) -> anyhow::Result> { +) -> anyhow::Result { let pwd = match password.as_ref() { - Some(p) => p.to_string(), + Some(p) => p.clone(), None => prompter.create_new_password()?, }; @@ -155,28 +156,29 @@ pub(crate) fn request_mnemonic_phrase( pub(crate) fn derive_key( file_version: DatFileVersion, - pwd: &str, + pwd: &ZeroizingBytes, salt: Option<&[u8; SALT_SIZE]>, -) -> anyhow::Result> { +) -> anyhow::Result { match file_version { DatFileVersion::RuskBinaryFileFormat(version) => { if version_without_pre_higher(version) >= (0, 0, 2, 0) { let salt = salt .ok_or_else(|| anyhow::anyhow!("Couldn't find the salt"))?; Ok(pbkdf2::pbkdf2_hmac_array::( - pwd.as_bytes(), + pwd.borrow(), salt, PBKDF2_ROUNDS, ) .to_vec()) } else { let mut hasher = Sha256::new(); - hasher.update(pwd.as_bytes()); + hasher.update(Borrow::<[u8]>::borrow(pwd)); Ok(hasher.finalize().to_vec()) } } - _ => Ok(blake3::hash(pwd.as_bytes()).as_bytes().to_vec()), + _ => Ok(blake3::hash(pwd.borrow()).as_bytes().to_vec()), } + .map(ZeroizingBytes::from) } /// Request a directory diff --git a/rusk-wallet/src/bin/main.rs b/rusk-wallet/src/bin/main.rs index 1ee94728cf..7970d3a7cb 100644 --- a/rusk-wallet/src/bin/main.rs +++ b/rusk-wallet/src/bin/main.rs @@ -9,11 +9,14 @@ mod config; mod interactive; mod io; mod settings; +mod zeroizing_bytes; use command::{gen_iv, gen_salt}; pub(crate) use command::{Command, RunResult}; use io::prompt::{ask_pwd, derive_key, Prompter}; +use zeroizing_bytes::ZeroizingBytes; +use std::borrow::Borrow; use std::fs; use std::path::PathBuf; @@ -36,7 +39,7 @@ use io::{prompt, status, WalletArgs}; #[derive(Debug, Clone)] pub(crate) struct WalletFile { path: WalletPath, - aes_key: Vec, + aes_key: ZeroizingBytes, salt: Option<[u8; SALT_SIZE]>, iv: Option<[u8; IV_SIZE]>, } @@ -47,7 +50,7 @@ impl SecureWalletFile for WalletFile { } fn aes_key(&self) -> &[u8] { - &self.aes_key + self.aes_key.borrow() } fn salt(&self) -> Option<&[u8; SALT_SIZE]> { @@ -295,13 +298,11 @@ async fn exec() -> anyhow::Result<()> { let file_version = wallet.get_file_version()?; - let password = &settings.password; - if file_version.is_old() { let salt = gen_salt(); let iv = gen_iv(); let pwd = match password.as_ref() { - Some(p) => p.to_string(), + Some(p) => p.clone(), None => ask_pwd("Updating your wallet data file, please enter your wallet password ")?, }; diff --git a/rusk-wallet/src/bin/settings.rs b/rusk-wallet/src/bin/settings.rs index 496941285f..d90691bf94 100644 --- a/rusk-wallet/src/bin/settings.rs +++ b/rusk-wallet/src/bin/settings.rs @@ -14,6 +14,7 @@ use url::Url; use crate::config::Network; use crate::io::WalletArgs; +use crate::zeroizing_bytes::ZeroizingBytes; #[derive(clap::ValueEnum, Debug, Clone)] pub(crate) enum LogFormat { @@ -55,7 +56,7 @@ pub(crate) struct Settings { pub(crate) logging: Logging, pub(crate) wallet_dir: PathBuf, - pub(crate) password: Option, + pub(crate) password: Option, } pub(crate) struct SettingsBuilder { diff --git a/rusk-wallet/src/bin/zeroizing_bytes.rs b/rusk-wallet/src/bin/zeroizing_bytes.rs new file mode 100644 index 0000000000..5e9b228aa8 --- /dev/null +++ b/rusk-wallet/src/bin/zeroizing_bytes.rs @@ -0,0 +1,90 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. +// +// Copyright (c) DUSK NETWORK. All rights reserved. + +use std::borrow::Borrow; + +use zeroize::Zeroize; + +pub type ZeroizingBytes = ZeroizingVec; + +/// The purpose of this struct is to provide a testable +/// zeroize-on-drop wrapper around a non-reallocating vector - this +/// is to ensure that the underlying data is stored in a single location +/// on the heap and will be zeroized when dropped. +#[derive(Debug, Clone, PartialEq, Eq, Hash, Zeroize)] +pub struct ZeroizingVec(Vec); + +impl Drop for ZeroizingVec { + fn drop(&mut self) { + self.0.zeroize(); + assert!(self.0.is_empty(), "Zeroization failed"); + } +} + +impl From for ZeroizingVec { + fn from(s: String) -> Self { + Self(s.into_bytes()) + } +} + +impl From> for ZeroizingVec { + fn from(v: Vec) -> Self { + Self(v) + } +} + +impl Borrow<[u8]> for ZeroizingVec { + fn borrow(&self) -> &[u8] { + &self.0 + } +} + +#[cfg(test)] +mod tests { + use std::borrow::BorrowMut; + use std::cell::RefCell; + use std::mem::ManuallyDrop; + use std::sync::Arc; + + use super::*; + + struct AppendOnZeroize { + val: i32, + append_to: Arc>>>, + } + + impl Zeroize for AppendOnZeroize { + fn zeroize(&mut self) { + let mut append_to = RefCell::borrow_mut(&self.append_to); + append_to.push(self.val); + } + } + + #[test] + fn test_zeroizing_bytes() { + let vec = Arc::new(RefCell::new(ManuallyDrop::new(vec![]))); + let z_vec = ZeroizingVec(vec![ + AppendOnZeroize { + val: 1, + append_to: vec.clone(), + }, + AppendOnZeroize { + val: 2, + append_to: vec.clone(), + }, + AppendOnZeroize { + val: 3, + append_to: vec.clone(), + }, + ]); + + drop(z_vec); + + let vec = RefCell::take(&vec); + let vec = ManuallyDrop::into_inner(vec); + assert_eq!(vec, vec![1i32, 2, 3]); + } +} diff --git a/rusk-wallet/src/wallet.rs b/rusk-wallet/src/wallet.rs index bd21c373d2..2b6099ab6e 100644 --- a/rusk-wallet/src/wallet.rs +++ b/rusk-wallet/src/wallet.rs @@ -580,20 +580,24 @@ impl Wallet { profile_idx: u8, dir: &Path, filename: Option, - pwd: &str, + pwd: &[u8], ) -> Result<(PathBuf, PathBuf), Error> { // we're expecting a directory here if !dir.is_dir() { return Err(Error::NotDirectory); } - let (pk, sk) = self.provisioner_keys(profile_idx)?; + let (pk, mut sk) = self.provisioner_keys(profile_idx)?; let path = PathBuf::from(dir); let filename = filename.unwrap_or(profile_idx.to_string()); - Ok(node_data::bls::save_consensus_keys( + let keys_paths = node_data::bls::save_consensus_keys( &path, &filename, &pk, &sk, pwd, - )?) + )?; + + sk.zeroize(); + + Ok(keys_paths) } /// Return the index of the address passed, returns an error if the address