fix: remove optionals from wallet set up (#4984)

Description
---
Currently, it is possible to set up a wallet (in creation/recovery/existing mode) without a password. This is not desirable, as in that case, sensitive data might be potentially stored in the database and leak in memory. We address this issue by enforcing passphrase creation.

Motivation and Context
---
For more details see issue #4977.

How Has This Been Tested?
---

applications/tari_console_wallet/src/init/mod.rs

@@ -82,26 +82,26 @@ pub enum WalletBoot {
 pub fn get_or_prompt_password(
     arg_password: Option<SafePassword>,
     config_password: Option<SafePassword>,
-) -> Result<Option<SafePassword>, ExitError> {
-    if arg_password.is_some() {
-        return Ok(arg_password);
+) -> Result<SafePassword, ExitError> {
+    if let Some(passphrase) = arg_password {
+        return Ok(passphrase);
     }
 
     let env = std::env::var_os(TARI_WALLET_PASSWORD);
     if let Some(p) = env {
         let env_password = p
             .into_string()
             .map_err(|_| ExitError::new(ExitCode::IOError, "Failed to convert OsString into String"))?;
-        return Ok(Some(env_password.into()));
+        return Ok(env_password.into());
     }
 
-    if config_password.is_some() {
-        return Ok(config_password);
+    if let Some(passphrase) = config_password {
+        return Ok(passphrase);
     }
 
     let password = prompt_password("Wallet password: ")?;
 
-    Ok(Some(password))
+    Ok(password)
 }
 
 fn prompt_password(prompt: &str) -> Result<SafePassword, ExitError> {
@@ -121,7 +121,7 @@ fn prompt_password(prompt: &str) -> Result<SafePassword, ExitError> {
 /// Allows the user to change the password of the wallet.
 pub async fn change_password(
     config: &ApplicationConfig,
-    arg_password: Option<SafePassword>,
+    arg_password: SafePassword,
     shutdown_signal: ShutdownSignal,
     non_interactive_mode: bool,
 ) -> Result<(), ExitError> {
@@ -134,15 +134,15 @@ pub async fn change_password(
         return Err(ExitError::new(ExitCode::InputError, "Passwords don't match!"));
     }
 
-    wallet
-        .remove_encryption()
-        .await
-        .map_err(|e| ExitError::new(ExitCode::WalletError, e))?;
+    // wallet
+    //     .remove_encryption()
+    //     .await
+    //     .map_err(|e| ExitError::new(ExitCode::WalletError, e))?;
 
-    wallet
-        .apply_encryption(passphrase)
-        .await
-        .map_err(|e| ExitError::new(ExitCode::WalletError, e))?;
+    // wallet
+    //     .apply_encryption(passphrase)
+    //     .await
+    //     .map_err(|e| ExitError::new(ExitCode::WalletError, e))?;
 
     println!("Wallet password changed successfully.");
 
@@ -250,7 +250,7 @@ pub(crate) fn wallet_mode(cli: &Cli, boot_mode: WalletBoot) -> WalletMode {
 #[allow(clippy::too_many_lines)]
 pub async fn init_wallet(
     config: &ApplicationConfig,
-    arg_password: Option<SafePassword>,
+    arg_password: SafePassword,
     seed_words_file_name: Option<PathBuf>,
     recovery_seed: Option<CipherSeed>,
     shutdown_signal: ShutdownSignal,
@@ -269,34 +269,16 @@ pub async fn init_wallet(
 
     debug!(target: LOG_TARGET, "Running Wallet database migrations");
 
-    // test encryption by initializing with no passphrase...
     let db_path = &config.wallet.db_file;
 
-    let result = initialize_sqlite_database_backends(db_path, None, config.wallet.db_connection_pool_size);
-    let (backends, wallet_encrypted) = match result {
-        Ok(backends) => {
-            // wallet is not encrypted
-            (backends, false)
-        },
-        Err(WalletStorageError::NoPasswordError) => {
-            // get supplied or prompt password
-            let passphrase = get_or_prompt_password(arg_password.clone(), config.wallet.password.clone())?;
-            let backends =
-                initialize_sqlite_database_backends(db_path, passphrase, config.wallet.db_connection_pool_size)?;
-            (backends, true)
-        },
-        Err(e) => {
-            return Err(e.into());
-        },
-    };
-    let (wallet_backend, transaction_backend, output_manager_backend, contacts_backend, key_manager_backend) = backends;
+    // wallet should be encrypted from the beginning, so we must require a password to be provided by the user
+    let (wallet_backend, transaction_backend, output_manager_backend, contacts_backend, key_manager_backend) =
+        initialize_sqlite_database_backends(db_path, arg_password, config.wallet.db_connection_pool_size)?;
+
     let wallet_db = WalletDatabase::new(wallet_backend);
     let output_db = OutputManagerDatabase::new(output_manager_backend.clone());
 
-    debug!(
-        target: LOG_TARGET,
-        "Databases Initialized. Wallet encrypted? {}.", wallet_encrypted
-    );
+    debug!(target: LOG_TARGET, "Databases Initialized. Wallet is encrypted.",);
 
     let node_address = match config.wallet.p2p.public_address.clone() {
         Some(addr) => addr,
@@ -362,43 +344,6 @@ pub async fn init_wallet(
             .map_err(|e| ExitError::new(ExitCode::WalletError, format!("Problem writing tor identity. {}", e)))?;
     }
 
-    if !wallet_encrypted {
-        debug!(target: LOG_TARGET, "Wallet is not encrypted.");
-
-        // create using --password arg if supplied and skip seed words confirmation
-        let passphrase = match arg_password {
-            Some(password) => {
-                debug!(target: LOG_TARGET, "Setting password from command line argument.");
-                password
-            },
-            None => {
-                debug!(target: LOG_TARGET, "Prompting for password.");
-                let password = prompt_password("Create wallet password: ")?;
-                let confirmed = prompt_password("Confirm wallet password: ")?;
-
-                if password.reveal() != confirmed.reveal() {
-                    return Err(ExitError::new(ExitCode::InputError, "Passwords don't match!"));
-                }
-
-                password
-            },
-        };
-
-        wallet.apply_encryption(passphrase).await?;
-
-        debug!(target: LOG_TARGET, "Wallet encrypted.");
-
-        if !non_interactive_mode && recovery_seed.is_none() {
-            match confirm_seed_words(&mut wallet) {
-                Ok(()) => {
-                    print!("\x1Bc"); // Clear the screen
-                },
-                Err(error) => {
-                    return Err(error);
-                },
-            };
-        }
-    }
     if let Some(file_name) = seed_words_file_name {
         let seed_words = wallet.get_seed_words(&MnemonicLanguage::English)?.join(" ");
         let _result = fs::write(file_name, seed_words.reveal()).map_err(|e| {
@@ -590,7 +535,7 @@ pub fn tari_splash_screen(heading: &str) {
 
 /// Prompts the user for a new wallet or to recover an existing wallet.
 /// Returns the wallet bootmode indicating if it's a new or existing wallet, or if recovery is required.
-pub(crate) fn boot(cli: &Cli, wallet_config: &WalletConfig) -> Result<WalletBoot, ExitError> {
+fn boot(cli: &Cli, wallet_config: &WalletConfig) -> Result<WalletBoot, ExitError> {
     let wallet_exists = wallet_config.db_file.exists();
 
     // forced recovery
@@ -652,3 +597,37 @@ pub(crate) fn boot(cli: &Cli, wallet_config: &WalletConfig) -> Result<WalletBoot
         }
     }
 }
+
+pub(crate) fn boot_with_password(
+    cli: &Cli,
+    wallet_config: &WalletConfig,
+) -> Result<(WalletBoot, SafePassword), ExitError> {
+    let boot_mode = boot(cli, wallet_config)?;
+
+    if cli.password.is_some() {
+        return Ok((boot_mode, cli.password.clone().unwrap()));
+    }
+    if wallet_config.password.is_some() {
+        return Ok((boot_mode, wallet_config.password.clone().unwrap()));
+    }
+
+    let password = match boot_mode {
+        WalletBoot::New => {
+            debug!(target: LOG_TARGET, "Prompting for password.");
+            let password = prompt_password("Create wallet password: ")?;
+            let confirmed = prompt_password("Confirm wallet password: ")?;
+
+            if password.reveal() != confirmed.reveal() {
+                return Err(ExitError::new(ExitCode::InputError, "Passwords don't match!"));
+            }
+
+            password
+        },
+        WalletBoot::Existing | WalletBoot::Recovery => {
+            debug!(target: LOG_TARGET, "Prompting for password.");
+            prompt_password("Prompt wallet password: ")?
+        },
+    };
+
+    Ok((boot_mode, password))
+}

applications/tari_console_wallet/src/lib.rs

@@ -32,15 +32,7 @@ mod utils;
 mod wallet_modes;
 
 pub use cli::Cli;
-use init::{
-    boot,
-    change_password,
-    get_base_node_peer_config,
-    init_wallet,
-    start_wallet,
-    tari_splash_screen,
-    WalletBoot,
-};
+use init::{change_password, get_base_node_peer_config, init_wallet, start_wallet, tari_splash_screen, WalletBoot};
 use log::*;
 use recovery::{get_seed_from_seed_words, prompt_private_key_from_seed_words};
 use tari_app_utilities::{common_cli_args::CommonCliArgs, consts};
@@ -57,7 +49,7 @@ use tokio::runtime::Runtime;
 use wallet_modes::{command_mode, grpc_mode, recovery_mode, script_mode, tui_mode, WalletMode};
 
 pub use crate::config::ApplicationConfig;
-use crate::init::wallet_mode;
+use crate::init::{boot_with_password, wallet_mode};
 
 pub const LOG_TARGET: &str = "wallet::console_wallet::main";
 
@@ -110,7 +102,7 @@ pub fn run_wallet_with_cli(runtime: Runtime, config: &mut ApplicationConfig, cli
     }
 
     // check for recovery based on existence of wallet file
-    let mut boot_mode = boot(&cli, &config.wallet)?;
+    let (mut boot_mode, password) = boot_with_password(&cli, &config.wallet)?;
 
     let recovery_seed = get_recovery_seed(boot_mode, &cli)?;
 

base_layer/wallet/src/key_manager_service/handle.rs

@@ -22,7 +22,6 @@
 
 use std::sync::Arc;
 
-use chacha20poly1305::XChaCha20Poly1305;
 use tari_common_types::types::PrivateKey;
 use tari_key_manager::cipher_seed::CipherSeed;
 use tokio::sync::RwLock;
@@ -69,14 +68,6 @@ where TBackend: KeyManagerBackend + 'static
             .add_key_manager_branch(branch.into())
     }
 
-    async fn apply_encryption(&self, cipher: XChaCha20Poly1305) -> Result<(), KeyManagerServiceError> {
-        (*self.key_manager_inner).write().await.apply_encryption(cipher)
-    }
-
-    async fn remove_encryption(&self) -> Result<(), KeyManagerServiceError> {
-        (*self.key_manager_inner).write().await.remove_encryption()
-    }
-
     async fn get_next_key<T: Into<String> + Send>(&self, branch: T) -> Result<NextKeyResult, KeyManagerServiceError> {
         (*self.key_manager_inner).read().await.get_next_key(branch.into()).await
     }

base_layer/wallet/src/key_manager_service/interface.rs

@@ -20,7 +20,6 @@
 //  WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
 //  USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-use chacha20poly1305::XChaCha20Poly1305;
 use tari_common_types::types::{PrivateKey, PublicKey};
 use tari_crypto::keys::PublicKey as PublicKeyTrait;
 
@@ -55,13 +54,6 @@ pub trait KeyManagerInterface: Clone + Send + Sync + 'static {
     /// or in the backend, a new branch will be created and tracked the backend, `Ok(AddResult::NewEntry)`.
     async fn add_new_branch<T: Into<String> + Send>(&self, branch: T) -> Result<AddResult, KeyManagerServiceError>;
 
-    /// Encrypts the key manager state using the provided cipher. An error is returned if the state is already
-    /// encrypted.
-    async fn apply_encryption(&self, cipher: XChaCha20Poly1305) -> Result<(), KeyManagerServiceError>;
-
-    /// Decrypts the key manager state using the provided cipher. An error is returned if the state is not encrypted.
-    async fn remove_encryption(&self) -> Result<(), KeyManagerServiceError>;
-
     /// Gets the next key from the branch. This will auto-increment the branch key index by 1
     async fn get_next_key<T: Into<String> + Send>(&self, branch: T) -> Result<NextKeyResult, KeyManagerServiceError>;
 

base_layer/wallet/src/key_manager_service/mock.rs

@@ -20,7 +20,6 @@
 //  WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
 //  USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-use chacha20poly1305::XChaCha20Poly1305;
 use log::*;
 use tari_common_types::types::PrivateKey;
 use tari_key_manager::{cipher_seed::CipherSeed, key_manager::KeyManager};
@@ -154,14 +153,6 @@ impl KeyManagerInterface for KeyManagerMock {
         self.get_key_at_index_mock(branch.into(), index).await
     }
 
-    async fn apply_encryption(&self, _cipher: XChaCha20Poly1305) -> Result<(), KeyManagerServiceError> {
-        unimplemented!("Not supported");
-    }
-
-    async fn remove_encryption(&self) -> Result<(), KeyManagerServiceError> {
-        unimplemented!("Not supported");
-    }
-
     async fn find_key_index<T: Into<String> + Send>(
         &self,
         branch: T,

base_layer/wallet/src/key_manager_service/service.rs

@@ -19,7 +19,6 @@
 //  SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
 //  WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
 //  USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-use chacha20poly1305::XChaCha20Poly1305;
 use futures::lock::Mutex;
 use log::*;
 use tari_common_types::types::PrivateKey;
@@ -110,16 +109,6 @@ where TBackend: KeyManagerBackend + 'static
         Ok(key.k)
     }
 
-    pub fn apply_encryption(&self, cipher: XChaCha20Poly1305) -> Result<(), KeyManagerServiceError> {
-        self.db.apply_encryption(cipher)?;
-        Ok(())
-    }
-
-    pub fn remove_encryption(&self) -> Result<(), KeyManagerServiceError> {
-        self.db.remove_encryption()?;
-        Ok(())
-    }
-
     /// Search the specified branch key manager key chain to find the index of the specified key.
     pub async fn find_key_index(&self, branch: String, key: &PrivateKey) -> Result<u64, KeyManagerServiceError> {
         let km = self

base_layer/wallet/src/key_manager_service/storage/database/backend.rs

@@ -19,7 +19,6 @@
 // SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
 // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
 // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-use chacha20poly1305::XChaCha20Poly1305;
 
 use crate::key_manager_service::{error::KeyManagerStorageError, storage::database::KeyManagerState};
 
@@ -34,8 +33,4 @@ pub trait KeyManagerBackend: Send + Sync + Clone {
     fn increment_key_index(&self, branch: String) -> Result<(), KeyManagerStorageError>;
     /// This method will set the currently stored key index for the key manager.
     fn set_key_index(&self, branch: String, index: u64) -> Result<(), KeyManagerStorageError>;
-    /// Apply encryption to the backend.
-    fn apply_encryption(&self, cipher: XChaCha20Poly1305) -> Result<(), KeyManagerStorageError>;
-    /// Remove encryption from the backend.
-    fn remove_encryption(&self) -> Result<(), KeyManagerStorageError>;
 }

base_layer/wallet/src/key_manager_service/storage/database/mod.rs

@@ -24,7 +24,6 @@ mod backend;
 use std::sync::Arc;
 
 pub use backend::KeyManagerBackend;
-use chacha20poly1305::XChaCha20Poly1305;
 
 use crate::key_manager_service::error::KeyManagerStorageError;
 
@@ -72,15 +71,4 @@ where T: KeyManagerBackend + 'static
     pub fn set_key_index(&self, branch: String, index: u64) -> Result<(), KeyManagerStorageError> {
         self.db.set_key_index(branch, index)
     }
-
-    /// Encrypts the entire key manager with all branches.
-    /// This will only encrypt the index used, as the master seed phrase is not directly stored with the key manager.
-    pub fn apply_encryption(&self, cipher: XChaCha20Poly1305) -> Result<(), KeyManagerStorageError> {
-        self.db.apply_encryption(cipher)
-    }
-
-    /// Decrypts the entire key manager.
-    pub fn remove_encryption(&self) -> Result<(), KeyManagerStorageError> {
-        self.db.remove_encryption()
-    }
 }