Validator import password flag (#2228)

## Issue Addressed

#2224

## Proposed Changes

Add a `--password-file` option to the `lighthouse account validator import` command. The flag requires `--reuse-password` and will copy the password over to the `validator_definitions.yml` file. I used #2070 as a guide for validating the password as UTF-8 and stripping newlines.

## Additional Info



Co-authored-by: realbigsean <seananderson33@gmail.com>
This commit is contained in:
realbigsean 2021-03-17 05:09:56 +00:00
parent 87825b2bd2
commit 6a69b20be1
3 changed files with 185 additions and 8 deletions

View File

@ -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<PathBuf> = 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<PathBuf> =
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<ZeroizeString> = 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(_) => {

View File

@ -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();

View File

@ -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<Item = PublicKey>) {
let slashing_db_path = validator_dir.path().join(SLASHING_PROTECTION_FILENAME);