Merge branch 'origin/aleks/wallet-zeroize' (#1443)

* origin/aleks/wallet-zeroize:
  [ci] wasm checksums update
  Add changelog
  fix: zeroize passphrases

.changelog/unreleased/improvements/1425-wallet-zeroize.md

@@ -0,0 +1,2 @@
+- Zeroizes memory containing passphrases in wallet.
+  ([\#1425](https://github.com/anoma/namada/issues/1425))

apps/Cargo.toml

@@ -135,7 +135,6 @@ tendermint-config = {version = "0.23.6", optional = true}
 tendermint-proto = {version = "0.23.6", optional = true}
 tendermint-rpc = {version = "0.23.6", features = ["http-client", "websocket-client"], optional = true}
 thiserror = "1.0.38"
-tiny-bip39 = "0.8.2"
 tokio = {version = "1.8.2", features = ["full"]}
 toml = "0.5.8"
 tonic = "0.8.3"

apps/src/lib/wallet/mod.rs

@@ -6,7 +6,7 @@ use std::io::{self, Write};
 use std::path::{Path, PathBuf};
 use std::{env, fs};
 
-use bip39::{Language, Mnemonic};
+use namada::bip39::{Language, Mnemonic};
 pub use namada::ledger::wallet::alias::Alias;
 use namada::ledger::wallet::{
     ConfirmationResponse, FindKeyError, GenRestoreKeyError, Wallet, WalletUtils,
@@ -27,27 +27,32 @@ impl WalletUtils for CliWalletUtils {
     type Rng = OsRng;
     type Storage = PathBuf;
 
-    fn read_decryption_password() -> String {
+    fn read_decryption_password() -> Zeroizing<String> {
         match env::var("NAMADA_WALLET_PASSWORD_FILE") {
-            Ok(path) => fs::read_to_string(path)
-                .expect("Something went wrong reading the file"),
+            Ok(path) => Zeroizing::new(
+                fs::read_to_string(path)
+                    .expect("Something went wrong reading the file"),
+            ),
             Err(_) => match env::var("NAMADA_WALLET_PASSWORD") {
-                Ok(password) => password,
+                Ok(password) => Zeroizing::new(password),
                 Err(_) => {
                     let prompt = "Enter your decryption password: ";
                     rpassword::read_password_from_tty(Some(prompt))
+                        .map(Zeroizing::new)
                         .expect("Failed reading password from tty.")
                 }
             },
         }
     }
 
-    fn read_encryption_password() -> String {
+    fn read_encryption_password() -> Zeroizing<String> {
         let pwd = match env::var("NAMADA_WALLET_PASSWORD_FILE") {
-            Ok(path) => fs::read_to_string(path)
-                .expect("Something went wrong reading the file"),
+            Ok(path) => Zeroizing::new(
+                fs::read_to_string(path)
+                    .expect("Something went wrong reading the file"),
+            ),
             Err(_) => match env::var("NAMADA_WALLET_PASSWORD") {
-                Ok(password) => password,
+                Ok(password) => Zeroizing::new(password),
                 Err(_) => {
                     let prompt = "Enter your encryption password: ";
                     read_and_confirm_passphrase_tty(prompt).unwrap_or_else(
@@ -62,7 +67,7 @@ impl WalletUtils for CliWalletUtils {
                 }
             },
         };
-        if pwd.is_empty() {
+        if pwd.as_str().is_empty() {
             eprintln!("Password cannot be empty");
             eprintln!("Action cancelled, no changes persisted.");
             cli::safe_exit(1)
@@ -85,12 +90,12 @@ impl WalletUtils for CliWalletUtils {
             .map_err(|_| GenRestoreKeyError::MnemonicInputError)
     }
 
-    fn read_mnemonic_passphrase(confirm: bool) -> String {
+    fn read_mnemonic_passphrase(confirm: bool) -> Zeroizing<String> {
         let prompt = "Enter BIP39 passphrase (empty for none): ";
         let result = if confirm {
             read_and_confirm_passphrase_tty(prompt)
         } else {
-            rpassword::read_password_from_tty(Some(prompt))
+            rpassword::read_password_from_tty(Some(prompt)).map(Zeroizing::new)
         };
         result.unwrap_or_else(|e| {
             eprintln!("{}", e);
@@ -152,19 +157,21 @@ where
     print!("{} ", request);
     std::io::stdout().flush()?;
 
-    let mut response = String::new();
+    let mut response = Zeroizing::default();
     std::io::stdin().read_line(&mut response)?;
-    Ok(Zeroizing::new(response))
+    Ok(response)
 }
 
 pub fn read_and_confirm_passphrase_tty(
     prompt: &str,
-) -> Result<String, std::io::Error> {
-    let passphrase = rpassword::read_password_from_tty(Some(prompt))?;
+) -> Result<Zeroizing<String>, std::io::Error> {
+    let passphrase =
+        rpassword::read_password_from_tty(Some(prompt)).map(Zeroizing::new)?;
     if !passphrase.is_empty() {
         let confirmed = rpassword::read_password_from_tty(Some(
             "Enter same passphrase again: ",
-        ))?;
+        ))
+        .map(Zeroizing::new)?;
         if confirmed != passphrase {
             return Err(std::io::Error::new(
                 std::io::ErrorKind::InvalidInput,
@@ -261,7 +268,7 @@ pub fn load_or_new_from_genesis(
 /// confirmation if read from stdin.
 pub fn read_and_confirm_encryption_password(
     unsafe_dont_encrypt: bool,
-) -> Option<String> {
+) -> Option<Zeroizing<String>> {
     if unsafe_dont_encrypt {
         println!("Warning: The keypair will NOT be encrypted.");
         None
@@ -272,7 +279,7 @@ pub fn read_and_confirm_encryption_password(
 
 #[cfg(test)]
 mod tests {
-    use bip39::MnemonicType;
+    use namada::bip39::MnemonicType;
     use namada::ledger::wallet::WalletUtils;
     use rand_core;
 

apps/src/lib/wallet/pre_genesis.rs

@@ -8,6 +8,7 @@ use namada::ledger::wallet::pre_genesis::{
 };
 use namada::ledger::wallet::{gen_key_to_store, WalletUtils};
 use namada::types::key::SchemeType;
+use zeroize::Zeroizing;
 
 use crate::wallet::store::gen_validator_keys;
 use crate::wallet::{read_and_confirm_encryption_password, CliWalletUtils};
@@ -97,17 +98,20 @@ pub fn load(store_dir: &Path) -> Result<ValidatorWallet, ReadError> {
 
 /// Generate a new [`ValidatorWallet`] with required pre-genesis keys. Will
 /// prompt for password when `!unsafe_dont_encrypt`.
-fn gen(scheme: SchemeType, password: Option<String>) -> ValidatorWallet {
-    let (account_key, account_sk) = gen_key_to_store(scheme, &password);
+fn gen(
+    scheme: SchemeType,
+    password: Option<Zeroizing<String>>,
+) -> ValidatorWallet {
+    let (account_key, account_sk) = gen_key_to_store(scheme, password.clone());
     let (consensus_key, consensus_sk) = gen_key_to_store(
         // Note that TM only allows ed25519 for consensus key
         SchemeType::Ed25519,
-        &password,
+        password.clone(),
     );
     let (tendermint_node_key, tendermint_node_sk) = gen_key_to_store(
         // Note that TM only allows ed25519 for node IDs
         SchemeType::Ed25519,
-        &password,
+        password,
     );
     let validator_keys = gen_validator_keys(None, scheme);
     let store = ValidatorStore {

shared/Cargo.toml

@@ -127,7 +127,7 @@ serde = {version = "1.0.125", features = ["derive"]}
 serde_json = "1.0.62"
 sha2 = "0.9.3"
 slip10_ed25519 = "0.1.3"
-tiny-bip39 = "0.8.2"
+tiny-bip39 = {git = "https://github.com/anoma/tiny-bip39.git", rev = "bf0f6d8713589b83af7a917366ec31f5275c0e57"}
 tiny-hderive = "0.3.0"
 # We switch off "blake2b" because it cannot be compiled to wasm
 tempfile = {version = "3.2.0", optional = true}

shared/src/ledger/args.rs

@@ -2,6 +2,7 @@
 use namada_core::types::chain::ChainId;
 use namada_core::types::time::DateTimeUtc;
 use rust_decimal::Decimal;
+use zeroize::Zeroizing;
 
 use crate::ibc::core::ics24_host::identifier::{ChannelId, PortId};
 use crate::types::address::Address;
@@ -403,7 +404,7 @@ pub struct Tx<C: NamadaTypes = SdkTypes> {
     /// Path to the TX WASM code file to reveal PK
     pub tx_reveal_code_path: C::Data,
     /// Password to decrypt key
-    pub password: Option<String>,
+    pub password: Option<Zeroizing<String>>,
 }
 
 /// MASP add key or address arguments

shared/src/ledger/signing.rs

@@ -4,6 +4,7 @@ use namada_core::ledger::parameters::storage as parameter_storage;
 use namada_core::types::address::{Address, ImplicitAddress};
 use namada_core::types::token::{self, Amount};
 use namada_core::types::transaction::MIN_FEE;
+use zeroize::Zeroizing;
 
 use crate::ledger::rpc::TxBroadcastData;
 use crate::ledger::tx::Error;
@@ -25,7 +26,7 @@ pub async fn find_keypair<
     client: &C,
     wallet: &mut Wallet<U>,
     addr: &Address,
-    password: Option<String>,
+    password: Option<Zeroizing<String>>,
 ) -> Result<common::SecretKey, Error> {
     match addr {
         Address::Established(_) => {

shared/src/ledger/wallet/keys.rs

@@ -9,6 +9,7 @@ use data_encoding::HEXLOWER;
 use orion::{aead, kdf};
 use serde::{Deserialize, Serialize};
 use thiserror::Error;
+use zeroize::Zeroizing;
 
 use crate::ledger::wallet::WalletUtils;
 
@@ -152,7 +153,7 @@ where
     /// Construct a keypair for storage. If no password is provided, the keypair
     /// will be stored raw without encryption. Returns the key for storing and a
     /// reference-counting point to the raw key.
-    pub fn new(keypair: T, password: Option<String>) -> (Self, T) {
+    pub fn new(keypair: T, password: Option<Zeroizing<String>>) -> (Self, T) {
         match password {
             Some(password) => (
                 Self::Encrypted(EncryptedKeypair::new(&keypair, password)),
@@ -168,7 +169,7 @@ where
     pub fn get<U: WalletUtils>(
         &self,
         decrypt: bool,
-        password: Option<String>,
+        password: Option<Zeroizing<String>>,
     ) -> Result<T, DecryptionError> {
         match self {
             StoredKeypair::Encrypted(encrypted_keypair) => {
@@ -196,9 +197,9 @@ where
 
 impl<T: BorshSerialize + BorshDeserialize> EncryptedKeypair<T> {
     /// Encrypt a keypair and store it with its salt.
-    pub fn new(keypair: &T, password: String) -> Self {
+    pub fn new(keypair: &T, password: Zeroizing<String>) -> Self {
         let salt = encryption_salt();
-        let encryption_key = encryption_key(&salt, password);
+        let encryption_key = encryption_key(&salt, &password);
 
         let data = keypair
             .try_to_vec()
@@ -213,14 +214,17 @@ impl<T: BorshSerialize + BorshDeserialize> EncryptedKeypair<T> {
     }
 
     /// Decrypt an encrypted keypair
-    pub fn decrypt(&self, password: String) -> Result<T, DecryptionError> {
+    pub fn decrypt(
+        &self,
+        password: Zeroizing<String>,
+    ) -> Result<T, DecryptionError> {
         let salt_len = encryption_salt().len();
         let (raw_salt, cipher) = self.0.split_at(salt_len);
 
         let salt = kdf::Salt::from_slice(raw_salt)
             .map_err(|_| DecryptionError::BadSalt)?;
 
-        let encryption_key = encryption_key(&salt, password);
+        let encryption_key = encryption_key(&salt, &password);
 
         let decrypted_data = aead::open(&encryption_key, cipher)
             .map_err(|_| DecryptionError::DecryptionError)?;
@@ -236,7 +240,7 @@ fn encryption_salt() -> kdf::Salt {
 }
 
 /// Make encryption secret key from a password.
-fn encryption_key(salt: &kdf::Salt, password: String) -> kdf::SecretKey {
+fn encryption_key(salt: &kdf::Salt, password: &str) -> kdf::SecretKey {
     kdf::Password::from_slice(password.as_bytes())
         .and_then(|password| kdf::derive_key(&password, salt, 3, 1 << 17, 32))
         .expect("Generation of encryption secret key shouldn't fail")