From 1801dd1a349d01a02cdbb4486c7901fc042467fa Mon Sep 17 00:00:00 2001 From: realbigsean Date: Wed, 23 Sep 2020 01:19:54 +0000 Subject: [PATCH] Interactive account passwords (#1623) ## Issue Addressed #1437 ## Proposed Changes - Make the `--wallet-password` flag optional and creates an interactive prompt if not provided. - Make the `--wallet-name` flag optional and creates an interactive prompt if not provided. - Add a minimum password requirement of a 12 character length. - Update the `--stdin-passwords` flag to `--stdin-inputs` because we have non-password user inputs ## Additional Info --- account_manager/src/common.rs | 25 ++++- account_manager/src/validator/create.rs | 55 ++++++++--- account_manager/src/validator/import.rs | 12 +-- account_manager/src/validator/recover.rs | 12 +-- account_manager/src/wallet/create.rs | 112 ++++++++++++++++++----- account_manager/src/wallet/recover.rs | 19 ++-- book/src/become-a-validator-source.md | 8 +- common/account_utils/src/lib.rs | 45 ++++++++- lighthouse/tests/account_manager.rs | 2 +- 9 files changed, 221 insertions(+), 69 deletions(-) diff --git a/account_manager/src/common.rs b/account_manager/src/common.rs index d7ab3f3ef..030092036 100644 --- a/account_manager/src/common.rs +++ b/account_manager/src/common.rs @@ -1,5 +1,5 @@ use account_utils::PlainText; -use account_utils::{read_mnemonic_from_user, strip_off_newlines}; +use account_utils::{read_input_from_user, strip_off_newlines}; use clap::ArgMatches; use eth2_wallet::bip39::{Language, Mnemonic}; use std::fs; @@ -10,6 +10,7 @@ use std::thread::sleep; use std::time::Duration; pub const MNEMONIC_PROMPT: &str = "Enter the mnemonic phrase:"; +pub const WALLET_NAME_PROMPT: &str = "Enter wallet name:"; pub fn ensure_dir_exists>(path: P) -> Result<(), String> { let path = path.as_ref(); @@ -29,9 +30,11 @@ pub fn base_wallet_dir(matches: &ArgMatches, arg: &'static str) -> Result, - stdin_password: bool, + stdin_inputs: bool, ) -> Result { let mnemonic = match mnemonic_path { Some(path) => fs::read(&path) @@ -51,7 +54,7 @@ pub fn read_mnemonic_from_cli( eprintln!(""); eprintln!("{}", MNEMONIC_PROMPT); - let mnemonic = read_mnemonic_from_user(stdin_password)?; + let mnemonic = read_input_from_user(stdin_inputs)?; match Mnemonic::from_phrase(mnemonic.as_str(), Language::English) { Ok(mnemonic_m) => { @@ -68,3 +71,19 @@ pub fn read_mnemonic_from_cli( }; Ok(mnemonic) } + +/// Reads in a wallet name from the user. If the `--wallet-name` flag is provided, use it. Otherwise +/// read from an interactive prompt using tty unless the `--stdin-inputs` flag is provided. +pub fn read_wallet_name_from_cli( + wallet_name: Option, + stdin_inputs: bool, +) -> Result { + match wallet_name { + Some(name) => Ok(name), + None => { + eprintln!("{}", WALLET_NAME_PROMPT); + + read_input_from_user(stdin_inputs) + } + } +} diff --git a/account_manager/src/validator/create.rs b/account_manager/src/validator/create.rs index c708e8396..948942978 100644 --- a/account_manager/src/validator/create.rs +++ b/account_manager/src/validator/create.rs @@ -1,8 +1,11 @@ +use crate::common::read_wallet_name_from_cli; +use crate::wallet::create::STDIN_INPUTS_FLAG; use crate::{common::ensure_dir_exists, SECRETS_DIR_FLAG, VALIDATOR_DIR_FLAG}; -use account_utils::{random_password, strip_off_newlines, validator_definitions}; +use account_utils::{ + random_password, read_password_from_user, strip_off_newlines, validator_definitions, PlainText, +}; use clap::{App, Arg, ArgMatches}; use environment::Environment; -use eth2_wallet::PlainText; use eth2_wallet_manager::WalletManager; use std::ffi::OsStr; use std::fs; @@ -18,6 +21,7 @@ pub const DEPOSIT_GWEI_FLAG: &str = "deposit-gwei"; pub const STORE_WITHDRAW_FLAG: &str = "store-withdrawal-keystore"; pub const COUNT_FLAG: &str = "count"; pub const AT_MOST_FLAG: &str = "at-most"; +pub const WALLET_PASSWORD_PROMPT: &str = "Enter your wallet's password:"; pub fn cli_app<'a, 'b>() -> App<'a, 'b> { App::new(CMD) @@ -30,16 +34,14 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .long(WALLET_NAME_FLAG) .value_name("WALLET_NAME") .help("Use the wallet identified by this name") - .takes_value(true) - .required(true), + .takes_value(true), ) .arg( Arg::with_name(WALLET_PASSWORD_FLAG) .long(WALLET_PASSWORD_FLAG) .value_name("WALLET_PASSWORD_PATH") .help("A path to a file containing the password which will unlock the wallet.") - .takes_value(true) - .required(true), + .takes_value(true), ) .arg( Arg::with_name(VALIDATOR_DIR_FLAG) @@ -99,6 +101,11 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .conflicts_with("count") .takes_value(true), ) + .arg( + Arg::with_name(STDIN_INPUTS_FLAG) + .long(STDIN_INPUTS_FLAG) + .help("If present, read all user inputs from stdin instead of tty."), + ) } pub fn cli_run( @@ -108,8 +115,9 @@ pub fn cli_run( ) -> Result<(), String> { let spec = env.core_context().eth2_config.spec; - let name: String = clap_utils::parse_required(matches, WALLET_NAME_FLAG)?; - let wallet_password_path: PathBuf = clap_utils::parse_required(matches, WALLET_PASSWORD_FLAG)?; + let name: Option = clap_utils::parse_optional(matches, WALLET_NAME_FLAG)?; + let stdin_inputs = matches.is_present(STDIN_INPUTS_FLAG); + let validator_dir = clap_utils::parse_path_with_default_in_home_dir( matches, VALIDATOR_DIR_FLAG, @@ -151,15 +159,17 @@ pub fn cli_run( return Ok(()); } - let wallet_password = fs::read(&wallet_password_path) - .map_err(|e| format!("Unable to read {:?}: {:?}", wallet_password_path, e)) - .map(|bytes| PlainText::from(strip_off_newlines(bytes)))?; + let wallet_password_path: Option = + clap_utils::parse_optional(matches, WALLET_PASSWORD_FLAG)?; + + let wallet_name = read_wallet_name_from_cli(name, stdin_inputs)?; + let wallet_password = read_wallet_password_from_cli(wallet_password_path, stdin_inputs)?; let mgr = WalletManager::open(&wallet_base_dir) .map_err(|e| format!("Unable to open --{}: {:?}", BASE_DIR_FLAG, e))?; let mut wallet = mgr - .wallet_by_name(&name) + .wallet_by_name(&wallet_name) .map_err(|e| format!("Unable to open wallet: {:?}", e))?; for i in 0..n { @@ -204,3 +214,24 @@ fn existing_validator_count>(validator_dir: P) -> Result, + stdin_inputs: bool, +) -> Result { + match password_file_path { + Some(path) => fs::read(&path) + .map_err(|e| format!("Unable to read {:?}: {:?}", path, e)) + .map(|bytes| strip_off_newlines(bytes).into()), + None => { + eprintln!(""); + eprintln!("{}", WALLET_PASSWORD_PROMPT); + let password = + PlainText::from(read_password_from_user(stdin_inputs)?.as_ref().to_vec()); + Ok(password) + } + } +} diff --git a/account_manager/src/validator/import.rs b/account_manager/src/validator/import.rs index 4f780179f..5216b3d9c 100644 --- a/account_manager/src/validator/import.rs +++ b/account_manager/src/validator/import.rs @@ -1,3 +1,4 @@ +use crate::wallet::create::STDIN_INPUTS_FLAG; use crate::{common::ensure_dir_exists, VALIDATOR_DIR_FLAG}; use account_utils::{ eth2_keystore::Keystore, @@ -17,7 +18,6 @@ use std::time::Duration; pub const CMD: &str = "import"; pub const KEYSTORE_FLAG: &str = "keystore"; pub const DIR_FLAG: &str = "directory"; -pub const STDIN_PASSWORD_FLAG: &str = "stdin-passwords"; pub const REUSE_PASSWORD_FLAG: &str = "reuse-password"; pub const PASSWORD_PROMPT: &str = "Enter the keystore password, or press enter to omit it:"; @@ -66,9 +66,9 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .takes_value(true), ) .arg( - Arg::with_name(STDIN_PASSWORD_FLAG) - .long(STDIN_PASSWORD_FLAG) - .help("If present, read passwords from stdin instead of tty."), + Arg::with_name(STDIN_INPUTS_FLAG) + .long(STDIN_INPUTS_FLAG) + .help("If present, read all user inputs from stdin instead of tty."), ) .arg( Arg::with_name(REUSE_PASSWORD_FLAG) @@ -85,7 +85,7 @@ pub fn cli_run(matches: &ArgMatches) -> Result<(), String> { VALIDATOR_DIR_FLAG, PathBuf::new().join(".lighthouse").join("validators"), )?; - let stdin_password = matches.is_present(STDIN_PASSWORD_FLAG); + let stdin_inputs = matches.is_present(STDIN_INPUTS_FLAG); let reuse_password = matches.is_present(REUSE_PASSWORD_FLAG); ensure_dir_exists(&validator_dir)?; @@ -153,7 +153,7 @@ pub fn cli_run(matches: &ArgMatches) -> Result<(), String> { eprintln!(""); eprintln!("{}", PASSWORD_PROMPT); - let password = read_password_from_user(stdin_password)?; + let password = read_password_from_user(stdin_inputs)?; if password.as_ref().is_empty() { eprintln!("Continuing without password."); diff --git a/account_manager/src/validator/recover.rs b/account_manager/src/validator/recover.rs index bc9624099..376c21645 100644 --- a/account_manager/src/validator/recover.rs +++ b/account_manager/src/validator/recover.rs @@ -1,7 +1,7 @@ use super::create::STORE_WITHDRAW_FLAG; -use super::import::STDIN_PASSWORD_FLAG; use crate::common::{ensure_dir_exists, read_mnemonic_from_cli}; use crate::validator::create::COUNT_FLAG; +use crate::wallet::create::STDIN_INPUTS_FLAG; use crate::{SECRETS_DIR_FLAG, VALIDATOR_DIR_FLAG}; use account_utils::eth2_keystore::{keypair_from_secret, Keystore, KeystoreBuilder}; use account_utils::random_password; @@ -78,9 +78,9 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { ), ) .arg( - Arg::with_name(STDIN_PASSWORD_FLAG) - .long(STDIN_PASSWORD_FLAG) - .help("If present, read passwords from stdin instead of tty."), + Arg::with_name(STDIN_INPUTS_FLAG) + .long(STDIN_INPUTS_FLAG) + .help("If present, read all user inputs from stdin instead of tty."), ) } @@ -98,7 +98,7 @@ pub fn cli_run(matches: &ArgMatches) -> Result<(), String> { let first_index: u32 = clap_utils::parse_required(matches, FIRST_INDEX_FLAG)?; let count: u32 = clap_utils::parse_required(matches, COUNT_FLAG)?; let mnemonic_path: Option = clap_utils::parse_optional(matches, MNEMONIC_FLAG)?; - let stdin_password = matches.is_present(STDIN_PASSWORD_FLAG); + let stdin_inputs = matches.is_present(STDIN_INPUTS_FLAG); ensure_dir_exists(&validator_dir)?; ensure_dir_exists(&secrets_dir)?; @@ -107,7 +107,7 @@ pub fn cli_run(matches: &ArgMatches) -> Result<(), String> { eprintln!("WARNING: KEY RECOVERY CAN LEAD TO DUPLICATING VALIDATORS KEYS, WHICH CAN LEAD TO SLASHING."); eprintln!(""); - let mnemonic = read_mnemonic_from_cli(mnemonic_path, stdin_password)?; + let mnemonic = read_mnemonic_from_cli(mnemonic_path, stdin_inputs)?; let seed = Seed::new(&mnemonic, ""); diff --git a/account_manager/src/wallet/create.rs b/account_manager/src/wallet/create.rs index 0bf9905e2..04d141b48 100644 --- a/account_manager/src/wallet/create.rs +++ b/account_manager/src/wallet/create.rs @@ -1,5 +1,8 @@ +use crate::common::read_wallet_name_from_cli; use crate::BASE_DIR_FLAG; -use account_utils::{random_password, strip_off_newlines}; +use account_utils::{ + is_password_sufficiently_complex, random_password, read_password_from_user, strip_off_newlines, +}; use clap::{App, Arg, ArgMatches}; use eth2_wallet::{ bip39::{Language, Mnemonic, MnemonicType}, @@ -7,7 +10,8 @@ use eth2_wallet::{ }; use eth2_wallet_manager::{LockedWallet, WalletManager, WalletType}; use std::ffi::OsStr; -use std::fs::{self, File}; +use std::fs; +use std::fs::File; use std::io::prelude::*; use std::os::unix::fs::PermissionsExt; use std::path::{Path, PathBuf}; @@ -18,6 +22,10 @@ pub const NAME_FLAG: &str = "name"; pub const PASSWORD_FLAG: &str = "password-file"; pub const TYPE_FLAG: &str = "type"; pub const MNEMONIC_FLAG: &str = "mnemonic-output-path"; +pub const STDIN_INPUTS_FLAG: &str = "stdin-inputs"; +pub const NEW_WALLET_PASSWORD_PROMPT: &str = + "Enter a password for your new wallet that is at least 12 characters long:"; +pub const RETYPE_PASSWORD_PROMPT: &str = "Please re-enter your wallet's new password:"; pub fn cli_app<'a, 'b>() -> App<'a, 'b> { App::new(CMD) @@ -30,8 +38,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { "The wallet will be created with this name. It is not allowed to \ create two wallets with the same name for the same --base-dir.", ) - .takes_value(true) - .required(true), + .takes_value(true), ) .arg( Arg::with_name(PASSWORD_FLAG) @@ -43,8 +50,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { saved at that path. To avoid confusion, if the file does not already \ exist it must include a '.pass' suffix.", ) - .takes_value(true) - .required(true), + .takes_value(true), ) .arg( Arg::with_name(TYPE_FLAG) @@ -67,6 +73,11 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { ) .takes_value(true) ) + .arg( + Arg::with_name(STDIN_INPUTS_FLAG) + .long(STDIN_INPUTS_FLAG) + .help("If present, read all user inputs from stdin instead of tty."), + ) } pub fn cli_run(matches: &ArgMatches, base_dir: PathBuf) -> Result<(), String> { @@ -113,9 +124,10 @@ pub fn create_wallet_from_mnemonic( base_dir: &Path, mnemonic: &Mnemonic, ) -> Result { - let name: String = clap_utils::parse_required(matches, NAME_FLAG)?; - let wallet_password_path: PathBuf = clap_utils::parse_required(matches, PASSWORD_FLAG)?; + let name: Option = clap_utils::parse_optional(matches, NAME_FLAG)?; + let wallet_password_path: Option = clap_utils::parse_optional(matches, PASSWORD_FLAG)?; let type_field: String = clap_utils::parse_required(matches, TYPE_FLAG)?; + let stdin_inputs = matches.is_present(STDIN_INPUTS_FLAG); let wallet_type = match type_field.as_ref() { HD_TYPE => WalletType::Hd, @@ -125,31 +137,81 @@ pub fn create_wallet_from_mnemonic( let mgr = WalletManager::open(&base_dir) .map_err(|e| format!("Unable to open --{}: {:?}", BASE_DIR_FLAG, e))?; - // Create a random password if the file does not exist. - if !wallet_password_path.exists() { - // To prevent users from accidentally supplying their password to the PASSWORD_FLAG and - // create a file with that name, we require that the password has a .pass suffix. - if wallet_password_path.extension() != Some(&OsStr::new("pass")) { - return Err(format!( - "Only creates a password file if that file ends in .pass: {:?}", - wallet_password_path - )); + let wallet_password: PlainText = match wallet_password_path { + Some(path) => { + // Create a random password if the file does not exist. + if !path.exists() { + // To prevent users from accidentally supplying their password to the PASSWORD_FLAG and + // create a file with that name, we require that the password has a .pass suffix. + if path.extension() != Some(&OsStr::new("pass")) { + return Err(format!( + "Only creates a password file if that file ends in .pass: {:?}", + path + )); + } + + create_with_600_perms(&path, random_password().as_bytes()) + .map_err(|e| format!("Unable to write to {:?}: {:?}", path, e))?; + } + read_new_wallet_password_from_cli(Some(path), stdin_inputs)? } + None => read_new_wallet_password_from_cli(None, stdin_inputs)?, + }; - create_with_600_perms(&wallet_password_path, random_password().as_bytes()) - .map_err(|e| format!("Unable to write to {:?}: {:?}", wallet_password_path, e))?; - } - - let wallet_password = fs::read(&wallet_password_path) - .map_err(|e| format!("Unable to read {:?}: {:?}", wallet_password_path, e)) - .map(|bytes| PlainText::from(strip_off_newlines(bytes)))?; + let wallet_name = read_wallet_name_from_cli(name, stdin_inputs)?; let wallet = mgr - .create_wallet(name, wallet_type, &mnemonic, wallet_password.as_bytes()) + .create_wallet( + wallet_name, + wallet_type, + &mnemonic, + wallet_password.as_bytes(), + ) .map_err(|e| format!("Unable to create wallet: {:?}", e))?; Ok(wallet) } +/// Used when a user is creating a new wallet. Read in a wallet password from a file if the password file +/// path is provided. Otherwise, read from an interactive prompt using tty unless the `--stdin-inputs` +/// flag is provided. This verifies the password complexity and verifies the password is correctly re-entered. +pub fn read_new_wallet_password_from_cli( + password_file_path: Option, + stdin_inputs: bool, +) -> Result { + match password_file_path { + Some(path) => { + let password: PlainText = fs::read(&path) + .map_err(|e| format!("Unable to read {:?}: {:?}", path, e)) + .map(|bytes| strip_off_newlines(bytes).into())?; + + // Ensure the password meets the minimum requirements. + is_password_sufficiently_complex(password.as_bytes())?; + Ok(password) + } + None => loop { + eprintln!(""); + eprintln!("{}", NEW_WALLET_PASSWORD_PROMPT); + let password = + PlainText::from(read_password_from_user(stdin_inputs)?.as_ref().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()); + if retyped_password == password { + break Ok(password); + } else { + eprintln!("Passwords do not match."); + } + } + Err(message) => eprintln!("{}", message), + } + }, + } +} + /// Creates a file with `600 (-rw-------)` permissions. pub fn create_with_600_perms>(path: P, bytes: &[u8]) -> Result<(), String> { let path = path.as_ref(); diff --git a/account_manager/src/wallet/recover.rs b/account_manager/src/wallet/recover.rs index 9e96de60d..2240323c2 100644 --- a/account_manager/src/wallet/recover.rs +++ b/account_manager/src/wallet/recover.rs @@ -1,12 +1,11 @@ use crate::common::read_mnemonic_from_cli; -use crate::wallet::create::create_wallet_from_mnemonic; +use crate::wallet::create::{create_wallet_from_mnemonic, STDIN_INPUTS_FLAG}; use crate::wallet::create::{HD_TYPE, NAME_FLAG, PASSWORD_FLAG, TYPE_FLAG}; use clap::{App, Arg, ArgMatches}; use std::path::PathBuf; pub const CMD: &str = "recover"; pub const MNEMONIC_FLAG: &str = "mnemonic-path"; -pub const STDIN_PASSWORD_FLAG: &str = "stdin-passwords"; pub fn cli_app<'a, 'b>() -> App<'a, 'b> { App::new(CMD) @@ -19,8 +18,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { "The wallet will be created with this name. It is not allowed to \ create two wallets with the same name for the same --base-dir.", ) - .takes_value(true) - .required(true), + .takes_value(true), ) .arg( Arg::with_name(PASSWORD_FLAG) @@ -33,8 +31,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { saved at that path. To avoid confusion, if the file does not already \ exist it must include a '.pass' suffix.", ) - .takes_value(true) - .required(true), + .takes_value(true), ) .arg( Arg::with_name(MNEMONIC_FLAG) @@ -56,21 +53,21 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .default_value(HD_TYPE), ) .arg( - Arg::with_name(STDIN_PASSWORD_FLAG) - .long(STDIN_PASSWORD_FLAG) - .help("If present, read passwords from stdin instead of tty."), + Arg::with_name(STDIN_INPUTS_FLAG) + .long(STDIN_INPUTS_FLAG) + .help("If present, read all user inputs from stdin instead of tty."), ) } pub fn cli_run(matches: &ArgMatches, wallet_base_dir: PathBuf) -> Result<(), String> { let mnemonic_path: Option = clap_utils::parse_optional(matches, MNEMONIC_FLAG)?; - let stdin_password = matches.is_present(STDIN_PASSWORD_FLAG); + let stdin_inputs = matches.is_present(STDIN_INPUTS_FLAG); eprintln!(""); eprintln!("WARNING: KEY RECOVERY CAN LEAD TO DUPLICATING VALIDATORS KEYS, WHICH CAN LEAD TO SLASHING."); eprintln!(""); - let mnemonic = read_mnemonic_from_cli(mnemonic_path, stdin_password)?; + let mnemonic = read_mnemonic_from_cli(mnemonic_path, stdin_inputs)?; let wallet = create_wallet_from_mnemonic(matches, &wallet_base_dir.as_path(), &mnemonic) .map_err(|e| format!("Unable to create wallet: {:?}", e))?; diff --git a/book/src/become-a-validator-source.md b/book/src/become-a-validator-source.md index adab86ce3..ce44d0c5f 100644 --- a/book/src/become-a-validator-source.md +++ b/book/src/become-a-validator-source.md @@ -88,10 +88,10 @@ validator](./validator-create.md). A two-step example follows: Create a wallet with: ```bash -lighthouse --testnet medalla account wallet create --name my-validators --password-file my-validators.pass +lighthouse --testnet medalla account wallet create ``` -The output will look like this: +You will be prompted for a wallet name and a password. The output will look like this: ``` Your wallet's 12-word BIP-39 mnemonic is: @@ -124,10 +124,10 @@ used to restore your validator if there is a data loss. Create a validator from the wallet with: ```bash -lighthouse --testnet medalla account validator create --wallet-name my-validators --wallet-password my-validators.pass --count 1 +lighthouse --testnet medalla account validator create --count 1 ``` -The output will look like this: +Enter your wallet's name and password when prompted. The output will look like this: ```bash 1/1 0x80f3dce8d6745a725d8442c9bc3ca0852e772394b898c95c134b94979ebb0af6f898d5c5f65b71be6889185c486918a7 diff --git a/common/account_utils/src/lib.rs b/common/account_utils/src/lib.rs index 8af026641..77351a7b9 100644 --- a/common/account_utils/src/lib.rs +++ b/common/account_utils/src/lib.rs @@ -17,6 +17,8 @@ pub mod validator_definitions; pub use eth2_keystore; pub use eth2_wallet::PlainText; +/// The minimum number of characters required for a wallet password. +pub const MINIMUM_PASSWORD_LEN: usize = 12; /// The `Alphanumeric` crate only generates a-z, A-Z, 0-9, therefore it has a range of 62 /// characters. /// @@ -108,7 +110,7 @@ pub fn read_password_from_user(use_stdin: bool) -> Result } /// Reads a mnemonic phrase from TTY or stdin if `use_stdin == true`. -pub fn read_mnemonic_from_user(use_stdin: bool) -> Result { +pub fn read_input_from_user(use_stdin: bool) -> Result { let mut input = String::new(); if use_stdin { io::stdin() @@ -121,9 +123,33 @@ pub fn read_mnemonic_from_user(use_stdin: bool) -> Result { .read_line(&mut input) .map_err(|e| format!("Error reading from tty: {}", e))?; } + trim_newline(&mut input); Ok(input) } +fn trim_newline(s: &mut String) { + if s.ends_with('\n') { + s.pop(); + if s.ends_with('\r') { + s.pop(); + } + } +} + +/// Takes a string password and checks that it meets minimum requirements. +/// +/// The current minimum password requirement is a 12 character length character length. +pub fn is_password_sufficiently_complex(password: &[u8]) -> Result<(), String> { + if password.len() >= MINIMUM_PASSWORD_LEN { + Ok(()) + } else { + Err(format!( + "Please use at least {} characters for your password.", + MINIMUM_PASSWORD_LEN + )) + } +} + /// Provides a new-type wrapper around `String` that is zeroized on `Drop`. /// /// Useful for ensuring that password memory is zeroed-out on drop. @@ -146,6 +172,7 @@ impl AsRef<[u8]> for ZeroizeString { #[cfg(test)] mod test { + use super::is_password_sufficiently_complex; use super::strip_off_newlines; #[test] @@ -181,4 +208,20 @@ mod test { expected ); } + + #[test] + fn test_password_over_min_length() { + is_password_sufficiently_complex(b"TestPasswordLong").unwrap(); + } + + #[test] + fn test_password_exactly_min_length() { + is_password_sufficiently_complex(b"TestPassword").unwrap(); + } + + #[test] + #[should_panic] + fn test_password_too_short() { + is_password_sufficiently_complex(b"TestPass").unwrap(); + } } diff --git a/lighthouse/tests/account_manager.rs b/lighthouse/tests/account_manager.rs index 6652c1ec6..f5c473034 100644 --- a/lighthouse/tests/account_manager.rs +++ b/lighthouse/tests/account_manager.rs @@ -408,7 +408,7 @@ fn validator_import_launchpad() { File::create(src_dir.path().join(NOT_KEYSTORE_NAME)).unwrap(); let mut child = validator_import_cmd() - .arg(format!("--{}", import::STDIN_PASSWORD_FLAG)) // Using tty does not work well with tests. + .arg(format!("--{}", STDIN_INPUTS_FLAG)) // Using tty does not work well with tests. .arg(format!("--{}", import::DIR_FLAG)) .arg(src_dir.path().as_os_str()) .arg(format!("--{}", VALIDATOR_DIR_FLAG))