diff --git a/Cargo.lock b/Cargo.lock index f366f24c1f8..d65459cd7ff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -36,6 +36,7 @@ dependencies = [ "tokio", "types", "validator_dir", + "zeroize", ] [[package]] @@ -2561,8 +2562,6 @@ dependencies = [ name = "eth2" version = "0.1.0" dependencies = [ - "account_utils", - "bytes", "derivative", "enr", "eth2_keystore", @@ -2572,7 +2571,6 @@ dependencies = [ "futures", "futures-util", "libp2p-identity", - "libsecp256k1", "mediatype", "multiaddr", "pretty_reqwest_error", @@ -2580,7 +2578,6 @@ dependencies = [ "proto_array", "psutil", "reqwest", - "ring 0.16.20", "sensitive_url", "serde", "serde_json", @@ -2588,6 +2585,7 @@ dependencies = [ "ssz_types", "tokio", "types", + "zeroize", ] [[package]] @@ -4434,6 +4432,7 @@ dependencies = [ "url", "validator_dir", "validator_metrics", + "zeroize", ] [[package]] @@ -5288,6 +5287,7 @@ dependencies = [ "validator_client", "validator_dir", "validator_manager", + "zeroize", ] [[package]] @@ -9586,6 +9586,7 @@ dependencies = [ "validator_store", "warp", "warp_utils", + "zeroize", ] [[package]] @@ -9629,6 +9630,7 @@ dependencies = [ "tree_hash", "types", "validator_http_api", + "zeroize", ] [[package]] @@ -10564,6 +10566,7 @@ version = "1.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ced3678a2879b30306d323f4542626697a464a97c0a07c9aebf7ebca65cd4dde" dependencies = [ + "serde", "zeroize_derive", ] diff --git a/Cargo.toml b/Cargo.toml index 0be462754e0..9e921190b8f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -201,7 +201,7 @@ tree_hash_derive = "0.8" url = "2" uuid = { version = "0.8", features = ["serde", "v4"] } warp = { version = "0.3.7", default-features = false, features = ["tls"] } -zeroize = { version = "1", features = ["zeroize_derive"] } +zeroize = { version = "1", features = ["zeroize_derive", "serde"] } zip = "0.6" # Local crates. diff --git a/account_manager/Cargo.toml b/account_manager/Cargo.toml index 7f2fa05a888..48230bb2812 100644 --- a/account_manager/Cargo.toml +++ b/account_manager/Cargo.toml @@ -27,6 +27,7 @@ safe_arith = { workspace = true } slot_clock = { workspace = true } filesystem = { workspace = true } sensitive_url = { workspace = true } +zeroize = { workspace = true } [dev-dependencies] tempfile = { workspace = true } diff --git a/account_manager/src/validator/create.rs b/account_manager/src/validator/create.rs index ec5af1e2ece..73e0ad54d47 100644 --- a/account_manager/src/validator/create.rs +++ b/account_manager/src/validator/create.rs @@ -294,7 +294,7 @@ pub fn read_wallet_password_from_cli( eprintln!(); eprintln!("{}", WALLET_PASSWORD_PROMPT); let password = - PlainText::from(read_password_from_user(stdin_inputs)?.as_ref().to_vec()); + PlainText::from(read_password_from_user(stdin_inputs)?.as_bytes().to_vec()); Ok(password) } } diff --git a/account_manager/src/validator/import.rs b/account_manager/src/validator/import.rs index 19ab5ad60ac..4d2353b5534 100644 --- a/account_manager/src/validator/import.rs +++ b/account_manager/src/validator/import.rs @@ -7,7 +7,7 @@ use account_utils::{ recursively_find_voting_keystores, PasswordStorage, ValidatorDefinition, ValidatorDefinitions, CONFIG_FILENAME, }, - ZeroizeString, STDIN_INPUTS_FLAG, + STDIN_INPUTS_FLAG, }; use clap::{Arg, ArgAction, ArgMatches, Command}; use clap_utils::FLAG_HEADER; @@ -16,6 +16,7 @@ use std::fs; use std::path::PathBuf; use std::thread::sleep; use std::time::Duration; +use zeroize::Zeroizing; pub const CMD: &str = "import"; pub const KEYSTORE_FLAG: &str = "keystore"; @@ -148,7 +149,7 @@ pub fn cli_run(matches: &ArgMatches, validator_dir: PathBuf) -> Result<(), Strin // Skip keystores that already exist, but exit early if any operation fails. // Reuses the same password for all keystores if the `REUSE_PASSWORD_FLAG` flag is set. let mut num_imported_keystores = 0; - let mut previous_password: Option = None; + let mut previous_password: Option> = None; for src_keystore in &keystore_paths { let keystore = Keystore::from_json_file(src_keystore) @@ -182,14 +183,17 @@ pub fn cli_run(matches: &ArgMatches, validator_dir: PathBuf) -> Result<(), Strin let password = match keystore_password_path.as_ref() { Some(path) => { - let password_from_file: ZeroizeString = fs::read_to_string(path) + let password_from_file: Zeroizing = fs::read_to_string(path) .map_err(|e| format!("Unable to read {:?}: {:?}", path, e))? .into(); - password_from_file.without_newlines() + password_from_file + .trim_end_matches(['\r', '\n']) + .to_string() + .into() } None => { let password_from_user = read_password_from_user(stdin_inputs)?; - if password_from_user.as_ref().is_empty() { + if password_from_user.is_empty() { eprintln!("Continuing without password."); sleep(Duration::from_secs(1)); // Provides nicer UX. break None; @@ -314,7 +318,7 @@ pub fn cli_run(matches: &ArgMatches, validator_dir: PathBuf) -> Result<(), Strin /// Otherwise, returns the keystore error. fn check_password_on_keystore( keystore: &Keystore, - password: &ZeroizeString, + password: &Zeroizing, ) -> Result { match keystore.decrypt_keypair(password.as_ref()) { Ok(_) => { diff --git a/account_manager/src/wallet/create.rs b/account_manager/src/wallet/create.rs index b22007050fd..6369646929a 100644 --- a/account_manager/src/wallet/create.rs +++ b/account_manager/src/wallet/create.rs @@ -226,14 +226,14 @@ pub fn read_new_wallet_password_from_cli( eprintln!(); eprintln!("{}", NEW_WALLET_PASSWORD_PROMPT); let password = - PlainText::from(read_password_from_user(stdin_inputs)?.as_ref().to_vec()); + PlainText::from(read_password_from_user(stdin_inputs)?.as_bytes().to_vec()); // Ensure the password meets the minimum requirements. match is_password_sufficiently_complex(password.as_bytes()) { Ok(_) => { eprintln!("{}", RETYPE_PASSWORD_PROMPT); let retyped_password = - PlainText::from(read_password_from_user(stdin_inputs)?.as_ref().to_vec()); + PlainText::from(read_password_from_user(stdin_inputs)?.as_bytes().to_vec()); if retyped_password == password { break Ok(password); } else { diff --git a/common/account_utils/src/lib.rs b/common/account_utils/src/lib.rs index c1fa621abb1..0f576efb3ab 100644 --- a/common/account_utils/src/lib.rs +++ b/common/account_utils/src/lib.rs @@ -8,18 +8,14 @@ use eth2_wallet::{ }; use filesystem::{create_with_600_perms, Error as FsError}; use rand::{distributions::Alphanumeric, Rng}; -use serde::{Deserialize, Serialize}; +use std::fs::{self, File}; use std::io; use std::io::prelude::*; use std::path::{Path, PathBuf}; use std::str::from_utf8; use std::thread::sleep; use std::time::Duration; -use std::{ - fs::{self, File}, - str::FromStr, -}; -use zeroize::Zeroize; +use zeroize::Zeroizing; pub mod validator_definitions; @@ -69,8 +65,8 @@ pub fn read_password>(path: P) -> Result { fs::read(path).map(strip_off_newlines).map(Into::into) } -/// Reads a password file into a `ZeroizeString` struct, with new-lines removed. -pub fn read_password_string>(path: P) -> Result { +/// Reads a password file into a `Zeroizing` struct, with new-lines removed. +pub fn read_password_string>(path: P) -> Result, String> { fs::read(path) .map_err(|e| format!("Error opening file: {:?}", e)) .map(strip_off_newlines) @@ -112,8 +108,8 @@ pub fn random_password() -> PlainText { random_password_raw_string().into_bytes().into() } -/// Generates a random alphanumeric password of length `DEFAULT_PASSWORD_LEN` as `ZeroizeString`. -pub fn random_password_string() -> ZeroizeString { +/// Generates a random alphanumeric password of length `DEFAULT_PASSWORD_LEN` as `Zeroizing`. +pub fn random_password_string() -> Zeroizing { random_password_raw_string().into() } @@ -141,7 +137,7 @@ pub fn strip_off_newlines(mut bytes: Vec) -> Vec { } /// Reads a password from TTY or stdin if `use_stdin == true`. -pub fn read_password_from_user(use_stdin: bool) -> Result { +pub fn read_password_from_user(use_stdin: bool) -> Result, String> { let result = if use_stdin { rpassword::prompt_password_stderr("") .map_err(|e| format!("Error reading from stdin: {}", e)) @@ -150,7 +146,7 @@ pub fn read_password_from_user(use_stdin: bool) -> Result .map_err(|e| format!("Error reading from tty: {}", e)) }; - result.map(ZeroizeString::from) + result.map(Zeroizing::from) } /// Reads a mnemonic phrase from TTY or stdin if `use_stdin == true`. @@ -210,46 +206,6 @@ pub fn mnemonic_from_phrase(phrase: &str) -> Result { Mnemonic::from_phrase(phrase, Language::English).map_err(|e| e.to_string()) } -/// Provides a new-type wrapper around `String` that is zeroized on `Drop`. -/// -/// Useful for ensuring that password memory is zeroed-out on drop. -#[derive(Clone, PartialEq, Serialize, Deserialize, Zeroize)] -#[zeroize(drop)] -#[serde(transparent)] -pub struct ZeroizeString(String); - -impl FromStr for ZeroizeString { - type Err = String; - - fn from_str(s: &str) -> Result { - Ok(Self(s.to_owned())) - } -} - -impl From for ZeroizeString { - fn from(s: String) -> Self { - Self(s) - } -} - -impl ZeroizeString { - pub fn as_str(&self) -> &str { - &self.0 - } - - /// Remove any number of newline or carriage returns from the end of a vector of bytes. - pub fn without_newlines(&self) -> ZeroizeString { - let stripped_string = self.0.trim_end_matches(['\r', '\n']).into(); - Self(stripped_string) - } -} - -impl AsRef<[u8]> for ZeroizeString { - fn as_ref(&self) -> &[u8] { - self.0.as_bytes() - } -} - pub fn read_mnemonic_from_cli( mnemonic_path: Option, stdin_inputs: bool, @@ -294,54 +250,6 @@ pub fn read_mnemonic_from_cli( mod test { use super::*; - #[test] - fn test_zeroize_strip_off() { - let expected = "hello world"; - - assert_eq!( - ZeroizeString::from("hello world\n".to_string()) - .without_newlines() - .as_str(), - expected - ); - assert_eq!( - ZeroizeString::from("hello world\n\n\n\n".to_string()) - .without_newlines() - .as_str(), - expected - ); - assert_eq!( - ZeroizeString::from("hello world\r".to_string()) - .without_newlines() - .as_str(), - expected - ); - assert_eq!( - ZeroizeString::from("hello world\r\r\r\r\r".to_string()) - .without_newlines() - .as_str(), - expected - ); - assert_eq!( - ZeroizeString::from("hello world\r\n".to_string()) - .without_newlines() - .as_str(), - expected - ); - assert_eq!( - ZeroizeString::from("hello world\r\n\r\n".to_string()) - .without_newlines() - .as_str(), - expected - ); - assert_eq!( - ZeroizeString::from("hello world".to_string()) - .without_newlines() - .as_str(), - expected - ); - } - #[test] fn test_strip_off() { let expected = b"hello world".to_vec(); diff --git a/common/account_utils/src/validator_definitions.rs b/common/account_utils/src/validator_definitions.rs index f228ce5fdfa..a4850fc1c63 100644 --- a/common/account_utils/src/validator_definitions.rs +++ b/common/account_utils/src/validator_definitions.rs @@ -3,9 +3,7 @@ //! Serves as the source-of-truth of which validators this validator client should attempt (or not //! attempt) to load into the `crate::intialized_validators::InitializedValidators` struct. -use crate::{ - default_keystore_password_path, read_password_string, write_file_via_temporary, ZeroizeString, -}; +use crate::{default_keystore_password_path, read_password_string, write_file_via_temporary}; use directory::ensure_dir_exists; use eth2_keystore::Keystore; use regex::Regex; @@ -17,6 +15,7 @@ use std::io; use std::path::{Path, PathBuf}; use types::{graffiti::GraffitiString, Address, PublicKey}; use validator_dir::VOTING_KEYSTORE_FILE; +use zeroize::Zeroizing; /// The file name for the serialized `ValidatorDefinitions` struct. pub const CONFIG_FILENAME: &str = "validator_definitions.yml"; @@ -52,7 +51,7 @@ pub enum Error { /// Defines how a password for a validator keystore will be persisted. pub enum PasswordStorage { /// Store the password in the `validator_definitions.yml` file. - ValidatorDefinitions(ZeroizeString), + ValidatorDefinitions(Zeroizing), /// Store the password in a separate, dedicated file (likely in the "secrets" directory). File(PathBuf), /// Don't store the password at all. @@ -93,7 +92,7 @@ pub enum SigningDefinition { #[serde(skip_serializing_if = "Option::is_none")] voting_keystore_password_path: Option, #[serde(skip_serializing_if = "Option::is_none")] - voting_keystore_password: Option, + voting_keystore_password: Option>, }, /// A validator that defers to a Web3Signer HTTP server for signing. /// @@ -107,7 +106,7 @@ impl SigningDefinition { matches!(self, SigningDefinition::LocalKeystore { .. }) } - pub fn voting_keystore_password(&self) -> Result, Error> { + pub fn voting_keystore_password(&self) -> Result>, Error> { match self { SigningDefinition::LocalKeystore { voting_keystore_password: Some(password), diff --git a/common/eth2/Cargo.toml b/common/eth2/Cargo.toml index 5a3aa83ad98..3b9a34d13be 100644 --- a/common/eth2/Cargo.toml +++ b/common/eth2/Cargo.toml @@ -16,10 +16,7 @@ libp2p-identity = { version = "0.2", features = ["peerid"] } proto_array = { workspace = true } ethereum_serde_utils = { workspace = true } eth2_keystore = { workspace = true } -libsecp256k1 = { workspace = true } -ring = { workspace = true } -bytes = { workspace = true } -account_utils = { workspace = true } +zeroize = { workspace = true } sensitive_url = { workspace = true } ethereum_ssz = { workspace = true } ethereum_ssz_derive = { workspace = true } diff --git a/common/eth2/src/lighthouse_vc/http_client.rs b/common/eth2/src/lighthouse_vc/http_client.rs index 67fe77a3157..1d1abcac791 100644 --- a/common/eth2/src/lighthouse_vc/http_client.rs +++ b/common/eth2/src/lighthouse_vc/http_client.rs @@ -1,6 +1,5 @@ use super::types::*; use crate::Error; -use account_utils::ZeroizeString; use reqwest::{ header::{HeaderMap, HeaderValue}, IntoUrl, @@ -14,6 +13,7 @@ use std::path::Path; pub use reqwest; pub use reqwest::{Response, StatusCode, Url}; use types::graffiti::GraffitiString; +use zeroize::Zeroizing; /// A wrapper around `reqwest::Client` which provides convenience methods for interfacing with a /// Lighthouse Validator Client HTTP server (`validator_client/src/http_api`). @@ -21,7 +21,7 @@ use types::graffiti::GraffitiString; pub struct ValidatorClientHttpClient { client: reqwest::Client, server: SensitiveUrl, - api_token: Option, + api_token: Option>, authorization_header: AuthorizationHeader, } @@ -79,18 +79,18 @@ impl ValidatorClientHttpClient { } /// Get a reference to this client's API token, if any. - pub fn api_token(&self) -> Option<&ZeroizeString> { + pub fn api_token(&self) -> Option<&Zeroizing> { self.api_token.as_ref() } /// Read an API token from the specified `path`, stripping any trailing whitespace. - pub fn load_api_token_from_file(path: &Path) -> Result { + pub fn load_api_token_from_file(path: &Path) -> Result, Error> { let token = fs::read_to_string(path).map_err(|e| Error::TokenReadError(path.into(), e))?; - Ok(ZeroizeString::from(token.trim_end().to_string())) + Ok(token.trim_end().to_string().into()) } /// Add an authentication token to use when making requests. - pub fn add_auth_token(&mut self, token: ZeroizeString) -> Result<(), Error> { + pub fn add_auth_token(&mut self, token: Zeroizing) -> Result<(), Error> { self.api_token = Some(token); self.authorization_header = AuthorizationHeader::Bearer; diff --git a/common/eth2/src/lighthouse_vc/std_types.rs b/common/eth2/src/lighthouse_vc/std_types.rs index ee05c298399..ae192312bdb 100644 --- a/common/eth2/src/lighthouse_vc/std_types.rs +++ b/common/eth2/src/lighthouse_vc/std_types.rs @@ -1,7 +1,7 @@ -use account_utils::ZeroizeString; use eth2_keystore::Keystore; use serde::{Deserialize, Serialize}; use types::{Address, Graffiti, PublicKeyBytes}; +use zeroize::Zeroizing; pub use slashing_protection::interchange::Interchange; @@ -41,7 +41,7 @@ pub struct SingleKeystoreResponse { #[serde(deny_unknown_fields)] pub struct ImportKeystoresRequest { pub keystores: Vec, - pub passwords: Vec, + pub passwords: Vec>, pub slashing_protection: Option, } diff --git a/common/eth2/src/lighthouse_vc/types.rs b/common/eth2/src/lighthouse_vc/types.rs index 1921549bcb5..d7d5a00df51 100644 --- a/common/eth2/src/lighthouse_vc/types.rs +++ b/common/eth2/src/lighthouse_vc/types.rs @@ -1,13 +1,12 @@ -use account_utils::ZeroizeString; +pub use crate::lighthouse::Health; +pub use crate::lighthouse_vc::std_types::*; +pub use crate::types::{GenericResponse, VersionData}; use eth2_keystore::Keystore; use graffiti::GraffitiString; use serde::{Deserialize, Serialize}; use std::path::PathBuf; - -pub use crate::lighthouse::Health; -pub use crate::lighthouse_vc::std_types::*; -pub use crate::types::{GenericResponse, VersionData}; pub use types::*; +use zeroize::Zeroizing; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct ValidatorData { @@ -44,7 +43,7 @@ pub struct ValidatorRequest { #[derive(Clone, PartialEq, Serialize, Deserialize)] pub struct CreateValidatorsMnemonicRequest { - pub mnemonic: ZeroizeString, + pub mnemonic: Zeroizing, #[serde(with = "serde_utils::quoted_u32")] pub key_derivation_path_offset: u32, pub validators: Vec, @@ -74,7 +73,7 @@ pub struct CreatedValidator { #[derive(Clone, PartialEq, Serialize, Deserialize)] pub struct PostValidatorsResponseData { - pub mnemonic: ZeroizeString, + pub mnemonic: Zeroizing, pub validators: Vec, } @@ -102,7 +101,7 @@ pub struct ValidatorPatchRequest { #[derive(Clone, PartialEq, Serialize, Deserialize)] pub struct KeystoreValidatorsPostRequest { - pub password: ZeroizeString, + pub password: Zeroizing, pub enable: bool, pub keystore: Keystore, #[serde(default)] @@ -191,7 +190,7 @@ pub struct SingleExportKeystoresResponse { #[serde(skip_serializing_if = "Option::is_none")] pub validating_keystore: Option, #[serde(skip_serializing_if = "Option::is_none")] - pub validating_keystore_password: Option, + pub validating_keystore_password: Option>, } #[derive(Serialize, Deserialize, Debug)] diff --git a/crypto/eth2_keystore/src/keystore.rs b/crypto/eth2_keystore/src/keystore.rs index 304ea3ecd6f..16a979cf63a 100644 --- a/crypto/eth2_keystore/src/keystore.rs +++ b/crypto/eth2_keystore/src/keystore.rs @@ -26,7 +26,7 @@ use std::io::{Read, Write}; use std::path::Path; use std::str; use unicode_normalization::UnicodeNormalization; -use zeroize::Zeroize; +use zeroize::Zeroizing; /// The byte-length of a BLS secret key. const SECRET_KEY_LEN: usize = 32; @@ -60,45 +60,6 @@ pub const HASH_SIZE: usize = 32; /// The default iteraction count, `c`, for PBKDF2. pub const DEFAULT_PBKDF2_C: u32 = 262_144; -/// Provides a new-type wrapper around `String` that is zeroized on `Drop`. -/// -/// Useful for ensuring that password memory is zeroed-out on drop. -#[derive(Clone, PartialEq, Serialize, Deserialize, Zeroize)] -#[zeroize(drop)] -#[serde(transparent)] -struct ZeroizeString(String); - -impl From for ZeroizeString { - fn from(s: String) -> Self { - Self(s) - } -} - -impl AsRef<[u8]> for ZeroizeString { - fn as_ref(&self) -> &[u8] { - self.0.as_bytes() - } -} - -impl std::ops::Deref for ZeroizeString { - type Target = String; - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -impl std::ops::DerefMut for ZeroizeString { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.0 - } -} - -impl FromIterator for ZeroizeString { - fn from_iter>(iter: T) -> Self { - ZeroizeString(String::from_iter(iter)) - } -} - #[derive(Debug, PartialEq)] pub enum Error { InvalidSecretKeyLen { len: usize, expected: usize }, @@ -451,11 +412,12 @@ fn is_control_character(c: char) -> bool { /// Takes a slice of bytes and returns a NFKD normalized string representation. /// /// Returns an error if the bytes are not valid utf8. -fn normalize(bytes: &[u8]) -> Result { +fn normalize(bytes: &[u8]) -> Result, Error> { Ok(str::from_utf8(bytes) .map_err(|_| Error::InvalidPasswordBytes)? .nfkd() - .collect::()) + .collect::() + .into()) } /// Generates a checksum to indicate that the `derived_key` is associated with the diff --git a/lighthouse/Cargo.toml b/lighthouse/Cargo.toml index 329519fb54f..1fd9e3dac87 100644 --- a/lighthouse/Cargo.toml +++ b/lighthouse/Cargo.toml @@ -73,6 +73,7 @@ eth2 = { workspace = true } beacon_processor = { workspace = true } beacon_node_fallback = { workspace = true } initialized_validators = { workspace = true } +zeroize = { workspace = true } [[test]] diff --git a/lighthouse/tests/account_manager.rs b/lighthouse/tests/account_manager.rs index 4d155937140..c7153f48ef5 100644 --- a/lighthouse/tests/account_manager.rs +++ b/lighthouse/tests/account_manager.rs @@ -15,7 +15,7 @@ use account_manager::{ use account_utils::{ eth2_keystore::KeystoreBuilder, validator_definitions::{SigningDefinition, ValidatorDefinition, ValidatorDefinitions}, - ZeroizeString, STDIN_INPUTS_FLAG, + STDIN_INPUTS_FLAG, }; use slashing_protection::{SlashingDatabase, SLASHING_PROTECTION_FILENAME}; use std::env; @@ -27,6 +27,7 @@ use std::str::from_utf8; use tempfile::{tempdir, TempDir}; use types::{Keypair, PublicKey}; use validator_dir::ValidatorDir; +use zeroize::Zeroizing; /// Returns the `lighthouse account` command. fn account_cmd() -> Command { @@ -498,7 +499,7 @@ fn validator_import_launchpad() { signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path, voting_keystore_password_path: None, - voting_keystore_password: Some(ZeroizeString::from(PASSWORD.to_string())), + voting_keystore_password: Some(Zeroizing::from(PASSWORD.to_string())), }, }; @@ -650,7 +651,7 @@ fn validator_import_launchpad_no_password_then_add_password() { signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path: dst_keystore_dir.join(KEYSTORE_NAME), voting_keystore_password_path: None, - voting_keystore_password: Some(ZeroizeString::from(PASSWORD.to_string())), + voting_keystore_password: Some(Zeroizing::from(PASSWORD.to_string())), }, }; @@ -753,7 +754,7 @@ fn validator_import_launchpad_password_file() { signing_definition: SigningDefinition::LocalKeystore { voting_keystore_path, voting_keystore_password_path: None, - voting_keystore_password: Some(ZeroizeString::from(PASSWORD.to_string())), + voting_keystore_password: Some(Zeroizing::from(PASSWORD.to_string())), }, }; diff --git a/validator_client/http_api/Cargo.toml b/validator_client/http_api/Cargo.toml index b83acdc782a..18e0604ad51 100644 --- a/validator_client/http_api/Cargo.toml +++ b/validator_client/http_api/Cargo.toml @@ -43,6 +43,7 @@ validator_services = { workspace = true } url = { workspace = true } warp_utils = { workspace = true } warp = { workspace = true } +zeroize = { workspace = true } [dev-dependencies] itertools = { workspace = true } diff --git a/validator_client/http_api/src/create_validator.rs b/validator_client/http_api/src/create_validator.rs index dfd092e8b46..f90a1057a43 100644 --- a/validator_client/http_api/src/create_validator.rs +++ b/validator_client/http_api/src/create_validator.rs @@ -2,7 +2,7 @@ use account_utils::validator_definitions::{PasswordStorage, ValidatorDefinition} use account_utils::{ eth2_keystore::Keystore, eth2_wallet::{bip39::Mnemonic, WalletBuilder}, - random_mnemonic, random_password, ZeroizeString, + random_mnemonic, random_password, }; use eth2::lighthouse_vc::types::{self as api_types}; use slot_clock::SlotClock; @@ -11,6 +11,7 @@ use types::ChainSpec; use types::EthSpec; use validator_dir::{keystore_password_path, Builder as ValidatorDirBuilder}; use validator_store::ValidatorStore; +use zeroize::Zeroizing; /// Create some validator EIP-2335 keystores and store them on disk. Then, enroll the validators in /// this validator client. @@ -59,7 +60,7 @@ pub async fn create_validators_mnemonic, T: 'static + SlotClock, for request in validator_requests { let voting_password = random_password(); let withdrawal_password = random_password(); - let voting_password_string = ZeroizeString::from( + let voting_password_string = Zeroizing::from( String::from_utf8(voting_password.as_bytes().to_vec()).map_err(|e| { warp_utils::reject::custom_server_error(format!( "locally generated password is not utf8: {:?}", @@ -199,7 +200,7 @@ pub async fn create_validators_web3signer( pub fn get_voting_password_storage( secrets_dir: &Option, voting_keystore: &Keystore, - voting_password_string: &ZeroizeString, + voting_password_string: &Zeroizing, ) -> Result { if let Some(secrets_dir) = &secrets_dir { let password_path = keystore_password_path(secrets_dir, voting_keystore); diff --git a/validator_client/http_api/src/keystores.rs b/validator_client/http_api/src/keystores.rs index 5822c89cb8a..fd6b4fdae51 100644 --- a/validator_client/http_api/src/keystores.rs +++ b/validator_client/http_api/src/keystores.rs @@ -1,5 +1,5 @@ //! Implementation of the standard keystore management API. -use account_utils::{validator_definitions::PasswordStorage, ZeroizeString}; +use account_utils::validator_definitions::PasswordStorage; use eth2::lighthouse_vc::{ std_types::{ DeleteKeystoreStatus, DeleteKeystoresRequest, DeleteKeystoresResponse, @@ -22,6 +22,7 @@ use validator_dir::{keystore_password_path, Builder as ValidatorDirBuilder}; use validator_store::ValidatorStore; use warp::Rejection; use warp_utils::reject::{custom_bad_request, custom_server_error}; +use zeroize::Zeroizing; pub fn list( validator_store: Arc>, @@ -167,7 +168,7 @@ pub fn import( fn import_single_keystore( keystore: Keystore, - password: ZeroizeString, + password: Zeroizing, validator_dir_path: PathBuf, secrets_dir: Option, validator_store: &ValidatorStore, diff --git a/validator_client/http_api/src/test_utils.rs b/validator_client/http_api/src/test_utils.rs index 931c4ea08ed..d033fdbf2d9 100644 --- a/validator_client/http_api/src/test_utils.rs +++ b/validator_client/http_api/src/test_utils.rs @@ -2,7 +2,6 @@ use crate::{ApiSecret, Config as HttpConfig, Context}; use account_utils::validator_definitions::ValidatorDefinitions; use account_utils::{ eth2_wallet::WalletBuilder, mnemonic_from_phrase, random_mnemonic, random_password, - ZeroizeString, }; use deposit_contract::decode_eth1_tx_data; use doppelganger_service::DoppelgangerService; @@ -28,6 +27,7 @@ use task_executor::test_utils::TestRuntime; use tempfile::{tempdir, TempDir}; use tokio::sync::oneshot; use validator_store::{Config as ValidatorStoreConfig, ValidatorStore}; +use zeroize::Zeroizing; pub const PASSWORD_BYTES: &[u8] = &[42, 50, 37]; pub const TEST_DEFAULT_FEE_RECIPIENT: Address = Address::repeat_byte(42); @@ -321,7 +321,7 @@ impl ApiTester { .collect::>(); let (response, mnemonic) = if s.specify_mnemonic { - let mnemonic = ZeroizeString::from(random_mnemonic().phrase().to_string()); + let mnemonic = Zeroizing::from(random_mnemonic().phrase().to_string()); let request = CreateValidatorsMnemonicRequest { mnemonic: mnemonic.clone(), key_derivation_path_offset: s.key_derivation_path_offset, diff --git a/validator_client/http_api/src/tests.rs b/validator_client/http_api/src/tests.rs index 76a6952153b..262bb64e69d 100644 --- a/validator_client/http_api/src/tests.rs +++ b/validator_client/http_api/src/tests.rs @@ -9,7 +9,7 @@ use initialized_validators::{Config as InitializedValidatorsConfig, InitializedV use crate::{ApiSecret, Config as HttpConfig, Context}; use account_utils::{ eth2_wallet::WalletBuilder, mnemonic_from_phrase, random_mnemonic, random_password, - random_password_string, validator_definitions::ValidatorDefinitions, ZeroizeString, + random_password_string, validator_definitions::ValidatorDefinitions, }; use deposit_contract::decode_eth1_tx_data; use eth2::{ @@ -33,6 +33,7 @@ use task_executor::test_utils::TestRuntime; use tempfile::{tempdir, TempDir}; use types::graffiti::GraffitiString; use validator_store::{Config as ValidatorStoreConfig, ValidatorStore}; +use zeroize::Zeroizing; const PASSWORD_BYTES: &[u8] = &[42, 50, 37]; pub const TEST_DEFAULT_FEE_RECIPIENT: Address = Address::repeat_byte(42); @@ -282,7 +283,7 @@ impl ApiTester { .collect::>(); let (response, mnemonic) = if s.specify_mnemonic { - let mnemonic = ZeroizeString::from(random_mnemonic().phrase().to_string()); + let mnemonic = Zeroizing::from(random_mnemonic().phrase().to_string()); let request = CreateValidatorsMnemonicRequest { mnemonic: mnemonic.clone(), key_derivation_path_offset: s.key_derivation_path_offset, diff --git a/validator_client/http_api/src/tests/keystores.rs b/validator_client/http_api/src/tests/keystores.rs index f3f6de548bf..2dde087a7fd 100644 --- a/validator_client/http_api/src/tests/keystores.rs +++ b/validator_client/http_api/src/tests/keystores.rs @@ -14,8 +14,9 @@ use std::{collections::HashMap, path::Path}; use tokio::runtime::Handle; use types::{attestation::AttestationBase, Address}; use validator_store::DEFAULT_GAS_LIMIT; +use zeroize::Zeroizing; -fn new_keystore(password: ZeroizeString) -> Keystore { +fn new_keystore(password: Zeroizing) -> Keystore { let keypair = Keypair::random(); Keystore( KeystoreBuilder::new(&keypair, password.as_ref(), String::new()) diff --git a/validator_client/initialized_validators/Cargo.toml b/validator_client/initialized_validators/Cargo.toml index 426cb303f6e..9c7a3f19d60 100644 --- a/validator_client/initialized_validators/Cargo.toml +++ b/validator_client/initialized_validators/Cargo.toml @@ -24,3 +24,4 @@ tokio = { workspace = true } bincode = { workspace = true } filesystem = { workspace = true } validator_metrics = { workspace = true } +zeroize = { workspace = true } diff --git a/validator_client/initialized_validators/src/lib.rs b/validator_client/initialized_validators/src/lib.rs index 0b36dbd62cf..bd64091dae4 100644 --- a/validator_client/initialized_validators/src/lib.rs +++ b/validator_client/initialized_validators/src/lib.rs @@ -14,7 +14,6 @@ use account_utils::{ self, SigningDefinition, ValidatorDefinition, ValidatorDefinitions, Web3SignerDefinition, CONFIG_FILENAME, }, - ZeroizeString, }; use eth2_keystore::Keystore; use lockfile::{Lockfile, LockfileError}; @@ -34,6 +33,7 @@ use types::graffiti::GraffitiString; use types::{Address, Graffiti, Keypair, PublicKey, PublicKeyBytes}; use url::{ParseError, Url}; use validator_dir::Builder as ValidatorDirBuilder; +use zeroize::Zeroizing; use key_cache::KeyCache; @@ -74,7 +74,7 @@ pub enum OnDecryptFailure { pub struct KeystoreAndPassword { pub keystore: Keystore, - pub password: Option, + pub password: Option>, } #[derive(Debug)] @@ -262,7 +262,7 @@ impl InitializedValidator { // If the password is supplied, use it and ignore the path // (if supplied). (_, Some(password)) => ( - password.as_ref().to_vec().into(), + password.as_bytes().to_vec().into(), keystore .decrypt_keypair(password.as_ref()) .map_err(Error::UnableToDecryptKeystore)?, @@ -282,7 +282,7 @@ impl InitializedValidator { &keystore, &keystore_path, )?; - (password.as_ref().to_vec().into(), keypair) + (password.as_bytes().to_vec().into(), keypair) } }, ) @@ -455,7 +455,7 @@ fn build_web3_signer_client( fn unlock_keystore_via_stdin_password( keystore: &Keystore, keystore_path: &Path, -) -> Result<(ZeroizeString, Keypair), Error> { +) -> Result<(Zeroizing, Keypair), Error> { eprintln!(); eprintln!( "The {} file does not contain either of the following fields for {:?}:", @@ -1172,14 +1172,14 @@ impl InitializedValidators { voting_keystore_path, } => { let pw = if let Some(p) = voting_keystore_password { - p.as_ref().to_vec().into() + p.as_bytes().to_vec().into() } else if let Some(path) = voting_keystore_password_path { read_password(path).map_err(Error::UnableToReadVotingKeystorePassword)? } else { let keystore = open_keystore(voting_keystore_path)?; unlock_keystore_via_stdin_password(&keystore, voting_keystore_path)? .0 - .as_ref() + .as_bytes() .to_vec() .into() }; @@ -1425,7 +1425,7 @@ impl InitializedValidators { /// This should only be used for testing, it's rather destructive. pub fn delete_passwords_from_validator_definitions( &mut self, - ) -> Result, Error> { + ) -> Result>, Error> { let mut passwords = HashMap::default(); for def in self.definitions.as_mut_slice() { diff --git a/validator_manager/Cargo.toml b/validator_manager/Cargo.toml index 4f367b8f5b1..36df2568410 100644 --- a/validator_manager/Cargo.toml +++ b/validator_manager/Cargo.toml @@ -21,6 +21,7 @@ eth2 = { workspace = true } hex = { workspace = true } tokio = { workspace = true } derivative = { workspace = true } +zeroize = { workspace = true } [dev-dependencies] tempfile = { workspace = true } diff --git a/validator_manager/src/common.rs b/validator_manager/src/common.rs index 4a35791b322..cc4157990fd 100644 --- a/validator_manager/src/common.rs +++ b/validator_manager/src/common.rs @@ -1,5 +1,5 @@ +use account_utils::strip_off_newlines; pub use account_utils::STDIN_INPUTS_FLAG; -use account_utils::{strip_off_newlines, ZeroizeString}; use eth2::lighthouse_vc::std_types::{InterchangeJsonStr, KeystoreJsonStr}; use eth2::{ lighthouse_vc::{ @@ -14,6 +14,7 @@ use std::fs; use std::path::{Path, PathBuf}; use tree_hash::TreeHash; use types::*; +use zeroize::Zeroizing; pub const IGNORE_DUPLICATES_FLAG: &str = "ignore-duplicates"; pub const COUNT_FLAG: &str = "count"; @@ -41,7 +42,7 @@ pub enum UploadError { #[derive(Clone, Serialize, Deserialize)] pub struct ValidatorSpecification { pub voting_keystore: KeystoreJsonStr, - pub voting_keystore_password: ZeroizeString, + pub voting_keystore_password: Zeroizing, pub slashing_protection: Option, pub fee_recipient: Option
, pub gas_limit: Option, diff --git a/validator_manager/src/import_validators.rs b/validator_manager/src/import_validators.rs index 2a819a2a645..2e8821f0db9 100644 --- a/validator_manager/src/import_validators.rs +++ b/validator_manager/src/import_validators.rs @@ -1,6 +1,6 @@ use super::common::*; use crate::DumpConfig; -use account_utils::{eth2_keystore::Keystore, ZeroizeString}; +use account_utils::eth2_keystore::Keystore; use clap::{Arg, ArgAction, ArgMatches, Command}; use clap_utils::FLAG_HEADER; use derivative::Derivative; @@ -10,6 +10,7 @@ use serde::{Deserialize, Serialize}; use std::fs; use std::path::PathBuf; use types::Address; +use zeroize::Zeroizing; pub const CMD: &str = "import"; pub const VALIDATORS_FILE_FLAG: &str = "validators-file"; @@ -167,7 +168,7 @@ pub struct ImportConfig { pub vc_token_path: PathBuf, pub ignore_duplicates: bool, #[derivative(Debug = "ignore")] - pub password: Option, + pub password: Option>, pub fee_recipient: Option
, pub gas_limit: Option, pub builder_proposals: Option, @@ -184,7 +185,7 @@ impl ImportConfig { vc_url: clap_utils::parse_required(matches, VC_URL_FLAG)?, vc_token_path: clap_utils::parse_required(matches, VC_TOKEN_FLAG)?, ignore_duplicates: matches.get_flag(IGNORE_DUPLICATES_FLAG), - password: clap_utils::parse_optional(matches, PASSWORD)?, + password: clap_utils::parse_optional(matches, PASSWORD)?.map(Zeroizing::new), fee_recipient: clap_utils::parse_optional(matches, FEE_RECIPIENT)?, gas_limit: clap_utils::parse_optional(matches, GAS_LIMIT)?, builder_proposals: clap_utils::parse_optional(matches, BUILDER_PROPOSALS)?, @@ -382,10 +383,7 @@ async fn run<'a>(config: ImportConfig) -> Result<(), String> { pub mod tests { use super::*; use crate::create_validators::tests::TestBuilder as CreateTestBuilder; - use std::{ - fs::{self, File}, - str::FromStr, - }; + use std::fs::{self, File}; use tempfile::{tempdir, TempDir}; use validator_http_api::{test_utils::ApiTester, Config as HttpConfig}; @@ -419,7 +417,7 @@ pub mod tests { vc_url: vc.url.clone(), vc_token_path, ignore_duplicates: false, - password: Some(ZeroizeString::from_str("password").unwrap()), + password: Some(Zeroizing::new("password".into())), fee_recipient: None, builder_boost_factor: None, gas_limit: None, diff --git a/validator_manager/src/move_validators.rs b/validator_manager/src/move_validators.rs index 807a147ca1a..c039728e6f8 100644 --- a/validator_manager/src/move_validators.rs +++ b/validator_manager/src/move_validators.rs @@ -1,6 +1,6 @@ use super::common::*; use crate::DumpConfig; -use account_utils::{read_password_from_user, ZeroizeString}; +use account_utils::read_password_from_user; use clap::{Arg, ArgAction, ArgMatches, Command}; use eth2::{ lighthouse_vc::{ @@ -19,6 +19,7 @@ use std::str::FromStr; use std::time::Duration; use tokio::time::sleep; use types::{Address, PublicKeyBytes}; +use zeroize::Zeroizing; pub const MOVE_DIR_NAME: &str = "lighthouse-validator-move"; pub const VALIDATOR_SPECIFICATION_FILE: &str = "validator-specification.json"; @@ -48,7 +49,7 @@ pub enum PasswordSource { } impl PasswordSource { - fn read_password(&mut self, pubkey: &PublicKeyBytes) -> Result { + fn read_password(&mut self, pubkey: &PublicKeyBytes) -> Result, String> { match self { PasswordSource::Interactive { stdin_inputs } => { eprintln!("Please enter a password for keystore {:?}:", pubkey);