diff --git a/api/src/owner.rs b/api/src/owner.rs index 7ca6b5d..1296db8 100644 --- a/api/src/owner.rs +++ b/api/src/owner.rs @@ -1726,8 +1726,10 @@ where /// * `name`: Reserved for future use, use `None` for the time being. /// * `mnemonic`: If present, restore the wallet seed from the given mnemonic instead of creating /// a new random seed. - /// * `mnemonic_length`: Desired length of mnemonic in bytes (16 or 32, either 12 or 24 words). - /// Use 0 if mnemonic isn't being used. + /// * `mnemonic_length`: Entropy length in bytes: 16, 20, 24, 28 or 32 + /// (12, 15, 18, 21 or 24 words). Ignored when `mnemonic` is provided. + /// For a new random seed, 0 is also accepted and defaults to 32 bytes. + /// All other values return an error. /// * `password`: The password used to encrypt/decrypt the `wallet.seed` file /// /// # Returns diff --git a/impls/src/error.rs b/impls/src/error.rs index 4169e68..a22cb2a 100644 --- a/impls/src/error.rs +++ b/impls/src/error.rs @@ -75,8 +75,8 @@ pub enum Error { Encryption, /// BIP 39 word list - #[error("BIP39 Mnemonic (word list) Error")] - Mnemonic, + #[error("BIP39 Mnemonic (word list) Error: {}", _0)] + Mnemonic(String), /// Command line argument error #[error("{}", _0)] diff --git a/impls/src/lifecycle/default.rs b/impls/src/lifecycle/default.rs index 7514a30..ca11687 100644 --- a/impls/src/lifecycle/default.rs +++ b/impls/src/lifecycle/default.rs @@ -182,7 +182,7 @@ where &mut self, _name: Option<&str>, mnemonic: Option, - mnemonic_length: usize, + entropy_size: usize, password: ZeroingString, test_mode: bool, ) -> Result<(), Error> { @@ -196,16 +196,15 @@ where return Err(Error::WalletSeedExists(msg)); } } + let entropy_size = if entropy_size == 0 { 32 } else { entropy_size }; WalletSeed::init_file( &data_dir_name, - mnemonic_length, + entropy_size, mnemonic.clone(), password, test_mode, ) - .map_err(|_| { - Error::Lifecycle("Error creating wallet seed (is mnemonic valid?)".to_owned()) - })?; + .map_err(|e| Error::Lifecycle(format!("Error creating wallet seed: {}", e)))?; info!("Wallet seed file created"); let mut wallet: WalletBackend = match WalletBackend::new(&data_dir_name, self.node_client.clone()) { @@ -248,6 +247,9 @@ where let wallet_seed = WalletSeed::from_file(&data_dir_name, password).map_err(|_| { Error::Lifecycle("Error opening wallet (is password correct?)".to_owned()) })?; + if wallet_seed.is_empty() { + warn!("This wallet has an empty seed and its funds are unsafe. Move them to a new wallet."); + } let keychain = wallet_seed .derive_keychain(global::is_testnet()) .map_err(|_| Error::Lifecycle("Error deriving keychain".to_owned()))?; @@ -286,7 +288,7 @@ where .map_err(|_| Error::Lifecycle("Error opening wallet seed file".into()))?; let res = wallet_seed .to_mnemonic() - .map_err(|_| Error::Lifecycle("Error recovering wallet seed".into()))?; + .map_err(|e| Error::Lifecycle(format!("Error recovering wallet seed: {}. This wallet has no valid recovery phrase and its funds may be unsafe. Move them to a new wallet.", e)))?; Ok(ZeroingString::from(res)) } @@ -382,3 +384,97 @@ where } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::keychain::ExtKeychain; + use crate::lifecycle::seed::{EncryptedWalletSeed, SEED_FILE}; + use crate::node_clients::HTTPNodeClient; + use std::fs::File; + use std::io::Write; + use std::time::Duration; + + fn provider(test_dir: &str) -> DefaultLCProvider { + let client = + HTTPNodeClient::new("http://127.0.0.1:1", None, Duration::from_secs(1)).unwrap(); + let mut provider = DefaultLCProvider::new(client); + provider.set_top_level_directory(test_dir).unwrap(); + provider + } + + #[test] + fn default_recovery_phrase() { + let test_dir = "test_output/default_recovery_phrase"; + let _ = fs::remove_dir_all(test_dir); + global::set_local_chain_type(global::ChainTypes::AutomatedTesting); + let mut provider = provider(test_dir); + let password = ZeroingString::from("test"); + + provider + .create_wallet(None, None, 0, password.clone(), false) + .unwrap(); + let mnemonic = provider.get_mnemonic(None, password).unwrap(); + + assert_eq!(mnemonic.split_whitespace().count(), 24); + fs::remove_dir_all(test_dir).unwrap(); + } + + #[test] + fn mnemonic_error_redacted() { + let test_dir = "test_output/mnemonic_error_redacted"; + let _ = fs::remove_dir_all(test_dir); + let mut provider = provider(test_dir); + let input = ZeroingString::from("INVALID_INPUT_MARKER ".repeat(12)); + let error = provider + .create_wallet(None, Some(input), 32, ZeroingString::from("test"), false) + .unwrap_err() + .to_string(); + + assert!(error.contains("invalid bip39 word")); + assert!(!error.contains("INVALID_INPUT_MARKER")); + assert!(!provider.wallet_exists(None).unwrap()); + fs::remove_dir_all(test_dir).unwrap(); + } + + #[test] + fn missing_recovery_phrase() { + let test_dir = "test_output/missing_recovery_phrase"; + let _ = fs::remove_dir_all(test_dir); + global::set_local_chain_type(global::ChainTypes::AutomatedTesting); + let mut provider = provider(test_dir); + let password = ZeroingString::from("test"); + let data_dir = PathBuf::from(test_dir).join(GRIN_WALLET_DIR); + fs::create_dir_all(data_dir.clone()).unwrap(); + + let seed_file_path = &format!( + "{}{}{}", + fmt_path(data_dir.to_str().unwrap().to_string()), + MAIN_SEPARATOR, + SEED_FILE, + ); + + let enc_seed = EncryptedWalletSeed::from_seed( + &WalletSeed::from_bytes(vec![].as_slice()), + password.clone(), + ) + .unwrap(); + let enc_seed_json = serde_json::to_string_pretty(&enc_seed).unwrap(); + let mut file = File::create(seed_file_path).unwrap(); + file.write_all(&enc_seed_json.as_bytes()).unwrap(); + + let error = match provider.get_mnemonic(None, password.clone()) { + Ok(_) => panic!("expected missing recovery phrase error"), + Err(error) => error, + }; + + assert_eq!( + error, + Error::Lifecycle("Error recovering wallet seed: BIP39 Mnemonic (word list) Error: invalid mnemonic/entropy length 0. This wallet has no valid recovery phrase and its funds may be unsafe. Move them to a new wallet.".into()) + ); + provider.open_wallet(None, password, false, false).unwrap(); + provider.close_wallet(None).unwrap(); + assert_eq!(fs::read_to_string(seed_file_path).unwrap(), enc_seed_json); + fs::remove_dir_all(test_dir).unwrap(); + } +} diff --git a/impls/src/lifecycle/seed.rs b/impls/src/lifecycle/seed.rs index 5dbbd6c..16f924a 100644 --- a/impls/src/lifecycle/seed.rs +++ b/impls/src/lifecycle/seed.rs @@ -39,11 +39,18 @@ impl WalletSeed { WalletSeed(bytes.to_vec()) } + pub(super) fn is_empty(&self) -> bool { + self.0.is_empty() + } + pub fn from_mnemonic(word_list: util::ZeroingString) -> Result { let res = mnemonic::to_entropy(&word_list); match res { Ok(s) => Ok(WalletSeed::from_bytes(&s)), - Err(_) => Err(Error::Mnemonic.into()), + Err(mnemonic::Error::BadWord(_)) => { + Err(Error::Mnemonic("invalid bip39 word".to_owned())) + } + Err(e) => Err(Error::Mnemonic(format!("{}", e))), } } @@ -61,7 +68,7 @@ impl WalletSeed { let result = mnemonic::from_entropy(&self.0); match result { Ok(r) => Ok(r), - Err(_) => Err(Error::Mnemonic.into()), + Err(e) => Err(Error::Mnemonic(format!("{}", e))), } } @@ -76,14 +83,14 @@ impl WalletSeed { } pub fn init_new( - seed_length: usize, + entropy_size: usize, test_mode: bool, password: Option, - ) -> WalletSeed { + ) -> Result { let mut seed: Vec = vec![]; let mut rng = thread_rng(); if !test_mode { - for _ in 0..seed_length { + for _ in 0..entropy_size { seed.push(rng.gen()); } } else { @@ -93,7 +100,10 @@ impl WalletSeed { .as_bytes() .to_vec(); } - WalletSeed(seed) + let _ = mnemonic::from_entropy(seed.as_slice()) + .map(util::ZeroingString::from) + .map_err(|e| Error::Mnemonic(format!("{}", e)))?; + Ok(WalletSeed(seed)) } pub fn seed_file_exists(data_file_dir: &str) -> Result { @@ -168,7 +178,7 @@ impl WalletSeed { pub fn init_file( data_file_dir: &str, - seed_length: usize, + entropy_size: usize, recovery_phrase: Option, password: util::ZeroingString, test_mode: bool, @@ -193,7 +203,7 @@ impl WalletSeed { let seed = match recovery_phrase { Some(p) => WalletSeed::from_mnemonic(p)?, - None => WalletSeed::init_new(seed_length, test_mode, Some(password.clone())), + None => WalletSeed::init_new(entropy_size, test_mode, Some(password.clone()))?, }; let enc_seed = EncryptedWalletSeed::from_seed(&seed, password)?; @@ -277,7 +287,7 @@ impl EncryptedWalletSeed { let password = password.as_bytes(); let mut key = [0; 32]; pbkdf2::derive( - ring::pbkdf2::PBKDF2_HMAC_SHA512, + pbkdf2::PBKDF2_HMAC_SHA512, NonZeroU32::new(100).unwrap(), &salt, password, @@ -325,7 +335,7 @@ impl EncryptedWalletSeed { let password = password.as_bytes(); let mut key = [0; 32]; pbkdf2::derive( - ring::pbkdf2::PBKDF2_HMAC_SHA512, + pbkdf2::PBKDF2_HMAC_SHA512, NonZeroU32::new(100).unwrap(), &salt, password, @@ -359,8 +369,12 @@ mod tests { use crate::util::ZeroingString; #[test] fn wallet_seed_encrypt() { + // Wrong entropy size. + let wrong_entropy_size = WalletSeed::init_new(31, false, None).err(); + assert!(wrong_entropy_size.is_some()); + let password = ZeroingString::from("passwoid"); - let wallet_seed = WalletSeed::init_new(32, false, None); + let wallet_seed = WalletSeed::init_new(32, false, None).unwrap(); let mut enc_wallet_seed = EncryptedWalletSeed::from_seed(&wallet_seed, password.clone()).unwrap(); println!("EWS: {:?}", enc_wallet_seed); diff --git a/libwallet/src/types.rs b/libwallet/src/types.rs index ae86f1b..0aec740 100644 --- a/libwallet/src/types.rs +++ b/libwallet/src/types.rs @@ -81,7 +81,7 @@ where &mut self, name: Option<&str>, mnemonic: Option, - mnemonic_length: usize, + entropy_size: usize, password: ZeroingString, test_mode: bool, ) -> Result<(), Error>;