diff --git a/account_manager/src/validator/import.rs b/account_manager/src/validator/import.rs index 12c84363b..79d939e38 100644 --- a/account_manager/src/validator/import.rs +++ b/account_manager/src/validator/import.rs @@ -1,4 +1,4 @@ -use crate::wallet::create::STDIN_INPUTS_FLAG; +use crate::wallet::create::{PASSWORD_FLAG, STDIN_INPUTS_FLAG}; use account_utils::{ eth2_keystore::Keystore, read_password_from_user, @@ -65,6 +65,19 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .long(REUSE_PASSWORD_FLAG) .help("If present, the same password will be used for all imported keystores."), ) + .arg( + Arg::with_name(PASSWORD_FLAG) + .long(PASSWORD_FLAG) + .value_name("KEYSTORE_PASSWORD_PATH") + .requires(REUSE_PASSWORD_FLAG) + .help( + "The path to the file containing the password which will unlock all \ + keystores being imported. This flag must be used with `--reuse-password`. \ + The password will be copied to the `validator_definitions.yml` file, so after \ + import we strongly recommend you delete the file at KEYSTORE_PASSWORD_PATH.", + ) + .takes_value(true), + ) } pub fn cli_run(matches: &ArgMatches, validator_dir: PathBuf) -> Result<(), String> { @@ -72,6 +85,8 @@ pub fn cli_run(matches: &ArgMatches, validator_dir: PathBuf) -> Result<(), Strin let keystores_dir: Option = clap_utils::parse_optional(matches, DIR_FLAG)?; let stdin_inputs = matches.is_present(STDIN_INPUTS_FLAG); let reuse_password = matches.is_present(REUSE_PASSWORD_FLAG); + let keystore_password_path: Option = + clap_utils::parse_optional(matches, PASSWORD_FLAG)?; let mut defs = ValidatorDefinitions::open_or_create(&validator_dir) .map_err(|e| format!("Unable to open {}: {:?}", CONFIG_FILENAME, e))?; @@ -131,6 +146,7 @@ pub fn cli_run(matches: &ArgMatches, validator_dir: PathBuf) -> Result<(), Strin // 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; + for src_keystore in &keystore_paths { let keystore = Keystore::from_json_file(src_keystore) .map_err(|e| format!("Unable to read keystore JSON {:?}: {:?}", src_keystore, e))?; @@ -155,13 +171,23 @@ pub fn cli_run(matches: &ArgMatches, validator_dir: PathBuf) -> Result<(), Strin eprintln!(); eprintln!("{}", PASSWORD_PROMPT); - let password = read_password_from_user(stdin_inputs)?; - - if password.as_ref().is_empty() { - eprintln!("Continuing without password."); - sleep(Duration::from_secs(1)); // Provides nicer UX. - break None; - } + let password = match keystore_password_path.as_ref() { + Some(path) => { + let password_from_file: ZeroizeString = fs::read_to_string(&path) + .map_err(|e| format!("Unable to read {:?}: {:?}", path, e))? + .into(); + password_from_file.without_newlines() + } + None => { + let password_from_user = read_password_from_user(stdin_inputs)?; + if password_from_user.as_ref().is_empty() { + eprintln!("Continuing without password."); + sleep(Duration::from_secs(1)); // Provides nicer UX. + break None; + } + password_from_user + } + }; match keystore.decrypt_keypair(password.as_ref()) { Ok(_) => { diff --git a/common/account_utils/src/lib.rs b/common/account_utils/src/lib.rs index 152b6ba46..1ec15fa4e 100644 --- a/common/account_utils/src/lib.rs +++ b/common/account_utils/src/lib.rs @@ -188,6 +188,12 @@ 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(|c| c == '\r' || c == '\n').into(); + Self(stripped_string) + } } impl AsRef<[u8]> for ZeroizeString { @@ -200,6 +206,54 @@ impl AsRef<[u8]> for ZeroizeString { 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/lighthouse/tests/account_manager.rs b/lighthouse/tests/account_manager.rs index baa2ccda3..74b5a5ba8 100644 --- a/lighthouse/tests/account_manager.rs +++ b/lighthouse/tests/account_manager.rs @@ -497,6 +497,103 @@ fn validator_import_launchpad() { ); } +#[test] +fn validator_import_launchpad_password_file() { + const PASSWORD: &str = "cats"; + const PASSWORD_FILE_NAME: &str = "pw_is_cats.txt"; + const KEYSTORE_NAME: &str = "keystore-m_12381_3600_0_0_0-1595406747.json"; + const NOT_KEYSTORE_NAME: &str = "keystore-m_12381_3600_0_0-1595406747.json"; + + let src_dir = tempdir().unwrap(); + let dst_dir = tempdir().unwrap(); + + let keypair = Keypair::random(); + let keystore = KeystoreBuilder::new(&keypair, PASSWORD.as_bytes(), "".into()) + .unwrap() + .build() + .unwrap(); + + let dst_keystore_dir = dst_dir.path().join(format!("0x{}", keystore.pubkey())); + + // Create a keystore in the src dir. + File::create(src_dir.path().join(KEYSTORE_NAME)) + .map(|mut file| keystore.to_json_writer(&mut file).unwrap()) + .unwrap(); + + // Create a not-keystore file in the src dir. + File::create(src_dir.path().join(NOT_KEYSTORE_NAME)).unwrap(); + + // Create a password file in the src dir. + File::create(src_dir.path().join(PASSWORD_FILE_NAME)) + .map(|mut file| file.write(PASSWORD.as_ref())) + .unwrap() + .unwrap(); + + let mut child = validator_cmd() + .arg(format!("--{}", VALIDATOR_DIR_FLAG)) + .arg(dst_dir.path().as_os_str()) + .arg(IMPORT_CMD) + .arg(format!("--{}", import::DIR_FLAG)) + .arg(src_dir.path().as_os_str()) + .arg(format!("--{}", import::REUSE_PASSWORD_FLAG)) + .arg(format!("--{}", PASSWORD_FLAG)) + .arg(src_dir.path().join(PASSWORD_FILE_NAME).as_os_str()) + .spawn() + .unwrap(); + + child.wait().unwrap(); + + assert!( + src_dir.path().join(KEYSTORE_NAME).exists(), + "keystore should not be removed from src dir" + ); + assert!( + src_dir.path().join(NOT_KEYSTORE_NAME).exists(), + "not-keystore should not be removed from src dir." + ); + assert!( + src_dir.path().join(PASSWORD_FILE_NAME).exists(), + "password file should not be removed from src dir." + ); + + let voting_keystore_path = dst_keystore_dir.join(KEYSTORE_NAME); + + assert!( + voting_keystore_path.exists(), + "keystore should be present in dst dir" + ); + assert!( + !dst_dir.path().join(NOT_KEYSTORE_NAME).exists(), + "not-keystore should not be present in dst dir" + ); + assert!( + !dst_dir.path().join(PASSWORD_FILE_NAME).exists(), + "password file should not be present in dst dir" + ); + + // Validator should be registered with slashing protection. + check_slashing_protection(&dst_dir, std::iter::once(keystore.public_key().unwrap())); + + let defs = ValidatorDefinitions::open(&dst_dir).unwrap(); + + let expected_def = ValidatorDefinition { + enabled: true, + description: "".into(), + voting_public_key: keystore.public_key().unwrap(), + graffiti: None, + signing_definition: SigningDefinition::LocalKeystore { + voting_keystore_path, + voting_keystore_password_path: None, + voting_keystore_password: Some(ZeroizeString::from(PASSWORD.to_string())), + }, + }; + + assert!( + defs.as_slice() == &[expected_def], + "validator defs file should be accurate" + ); +} + /// Check that all of the given pubkeys have been registered with slashing protection. fn check_slashing_protection(validator_dir: &TempDir, pubkeys: impl Iterator) { let slashing_db_path = validator_dir.path().join(SLASHING_PROTECTION_FILENAME);