Skip to content

Commit

Permalink
fix: use SafePassword struct instead of String for passwords
Browse files Browse the repository at this point in the history
  • Loading branch information
therustmonk committed Jul 28, 2022
1 parent 3c74064 commit d6359d3
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 40 deletions.
7 changes: 5 additions & 2 deletions applications/tari_console_wallet/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ use clap::{Args, Parser, Subcommand};
use tari_app_utilities::{common_cli_args::CommonCliArgs, utilities::UniPublicKey};
use tari_comms::multiaddr::Multiaddr;
use tari_core::transactions::{tari_amount, tari_amount::MicroTari};
use tari_utilities::hex::{Hex, HexError};
use tari_utilities::{
hex::{Hex, HexError},
SafePassword,
};

const DEFAULT_NETWORK: &str = "dibbler";

Expand All @@ -45,7 +48,7 @@ pub(crate) struct Cli {
/// command line, since it's visible using `ps ax` from anywhere on the system, so always use the env var where
/// possible.
#[clap(long, env = "TARI_WALLET_PASSWORD", hide_env_values = true)]
pub password: Option<String>,
pub password: Option<SafePassword>,
/// Change the password for the console wallet
#[clap(long, alias = "update-password")]
pub change_password: bool,
Expand Down
24 changes: 12 additions & 12 deletions applications/tari_console_wallet/src/init/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ use tari_crypto::keys::PublicKey;
use tari_key_manager::{cipher_seed::CipherSeed, mnemonic::MnemonicLanguage};
use tari_p2p::{initialization::CommsInitializationError, peer_seeds::SeedPeer, TransportType};
use tari_shutdown::ShutdownSignal;
use tari_utilities::SafePassword;
use tari_wallet::{
error::{WalletError, WalletStorageError},
output_manager_service::storage::database::OutputManagerDatabase,
Expand Down Expand Up @@ -72,20 +73,19 @@ pub enum WalletBoot {
/// Gets the password provided by command line argument or environment variable if available.
/// Otherwise prompts for the password to be typed in.
pub fn get_or_prompt_password(
arg_password: Option<String>,
config_password: Option<String>,
) -> Result<Option<String>, ExitError> {
arg_password: Option<SafePassword>,
config_password: Option<SafePassword>,
) -> Result<Option<SafePassword>, ExitError> {
if arg_password.is_some() {
return Ok(arg_password);
}

let env = std::env::var_os(TARI_WALLET_PASSWORD);
if let Some(p) = env {
let env_password = Some(
p.into_string()
.map_err(|_| ExitError::new(ExitCode::IOError, "Failed to convert OsString into String"))?,
);
return Ok(env_password);
let env_password = p
.into_string()
.map_err(|_| ExitError::new(ExitCode::IOError, "Failed to convert OsString into String"))?;
return Ok(Some(env_password.into()));
}

if config_password.is_some() {
Expand All @@ -97,7 +97,7 @@ pub fn get_or_prompt_password(
Ok(Some(password))
}

fn prompt_password(prompt: &str) -> Result<String, ExitError> {
fn prompt_password(prompt: &str) -> Result<SafePassword, ExitError> {
let password = loop {
let pass = prompt_password_stdout(prompt).map_err(|e| ExitError::new(ExitCode::IOError, e))?;
if pass.is_empty() {
Expand All @@ -108,13 +108,13 @@ fn prompt_password(prompt: &str) -> Result<String, ExitError> {
}
};

Ok(password)
Ok(SafePassword::from(password))
}

/// Allows the user to change the password of the wallet.
pub async fn change_password(
config: &ApplicationConfig,
arg_password: Option<String>,
arg_password: Option<SafePassword>,
shutdown_signal: ShutdownSignal,
) -> Result<(), ExitError> {
let mut wallet = init_wallet(config, arg_password, None, None, shutdown_signal).await?;
Expand Down Expand Up @@ -221,7 +221,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<String>,
arg_password: Option<SafePassword>,
seed_words_file_name: Option<PathBuf>,
recovery_seed: Option<CipherSeed>,
shutdown_signal: ShutdownSignal,
Expand Down
3 changes: 2 additions & 1 deletion base_layer/wallet/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use tari_common::{
};
use tari_comms::multiaddr::Multiaddr;
use tari_p2p::P2pConfig;
use tari_utilities::SafePassword;

use crate::{
base_node_service::config::BaseNodeServiceConfig,
Expand Down Expand Up @@ -72,7 +73,7 @@ pub struct WalletConfig {
/// The main wallet db sqlite database backend connection pool size for concurrent reads
pub db_connection_pool_size: usize,
/// The main wallet password
pub password: Option<String>, // TODO: Make clear on drop
pub password: Option<SafePassword>,
/// The auto ping interval to use for contacts liveness data
#[serde(with = "serializers::seconds")]
pub contacts_auto_ping_interval: Duration,
Expand Down
5 changes: 3 additions & 2 deletions base_layer/wallet/src/storage/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use tari_comms::{
tor::TorIdentity,
};
use tari_key_manager::cipher_seed::CipherSeed;
use tari_utilities::SafePassword;

use crate::{error::WalletStorageError, utxo_scanner_service::service::ScannedBlock};

Expand All @@ -46,7 +47,7 @@ pub trait WalletBackend: Send + Sync + Clone {
/// Modify the state the of the backend with a write operation
fn write(&self, op: WriteOperation) -> Result<Option<DbValue>, WalletStorageError>;
/// Apply encryption to the backend.
fn apply_encryption(&self, passphrase: String) -> Result<Aes256Gcm, WalletStorageError>;
fn apply_encryption(&self, passphrase: SafePassword) -> Result<Aes256Gcm, WalletStorageError>;
/// Remove encryption from the backend.
fn remove_encryption(&self) -> Result<(), WalletStorageError>;

Expand Down Expand Up @@ -276,7 +277,7 @@ where T: WalletBackend + 'static
Ok(())
}

pub async fn apply_encryption(&self, passphrase: String) -> Result<Aes256Gcm, WalletStorageError> {
pub async fn apply_encryption(&self, passphrase: SafePassword) -> Result<Aes256Gcm, WalletStorageError> {
let db_clone = self.db.clone();
tokio::task::spawn_blocking(move || db_clone.apply_encryption(passphrase))
.await
Expand Down
21 changes: 11 additions & 10 deletions base_layer/wallet/src/storage/sqlite_db/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ use tari_key_manager::cipher_seed::CipherSeed;
use tari_utilities::{
hex::{from_hex, Hex},
message_format::MessageFormat,
SafePassword,
};
use tokio::time::Instant;

Expand All @@ -72,7 +73,7 @@ pub struct WalletSqliteDatabase {
impl WalletSqliteDatabase {
pub fn new(
database_connection: WalletDbConnection,
passphrase: Option<String>,
passphrase: Option<SafePassword>,
) -> Result<Self, WalletStorageError> {
let cipher = check_db_encryption_status(&database_connection, passphrase)?;

Expand Down Expand Up @@ -383,7 +384,7 @@ impl WalletBackend for WalletSqliteDatabase {
}
}

fn apply_encryption(&self, passphrase: String) -> Result<Aes256Gcm, WalletStorageError> {
fn apply_encryption(&self, passphrase: SafePassword) -> Result<Aes256Gcm, WalletStorageError> {
let mut current_cipher = acquire_write_lock!(self.cipher);
if current_cipher.is_some() {
return Err(WalletStorageError::AlreadyEncrypted);
Expand All @@ -404,13 +405,13 @@ impl WalletBackend for WalletSqliteDatabase {
let passphrase_salt = SaltString::generate(&mut OsRng);

let passphrase_hash = argon2
.hash_password_simple(passphrase.as_bytes(), &passphrase_salt)
.hash_password_simple(passphrase.reveal(), &passphrase_salt)
.map_err(|e| WalletStorageError::AeadError(e.to_string()))?
.to_string();
let encryption_salt = SaltString::generate(&mut OsRng);

let derived_encryption_key = argon2
.hash_password_simple(passphrase.as_bytes(), encryption_salt.as_str())
.hash_password_simple(passphrase.reveal(), encryption_salt.as_str())
.map_err(|e| WalletStorageError::AeadError(e.to_string()))?
.hash
.ok_or_else(|| WalletStorageError::AeadError("Problem generating encryption key hash".to_string()))?;
Expand Down Expand Up @@ -560,7 +561,7 @@ impl WalletBackend for WalletSqliteDatabase {
/// Master Public Key that is stored in the db
fn check_db_encryption_status(
database_connection: &WalletDbConnection,
passphrase: Option<String>,
passphrase: Option<SafePassword>,
) -> Result<Option<Aes256Gcm>, WalletStorageError> {
let start = Instant::now();
let conn = database_connection.get_pooled_connection()?;
Expand All @@ -581,13 +582,13 @@ fn check_db_encryption_status(
let argon2 = Argon2::default();
let stored_hash =
PasswordHash::new(&db_passphrase_hash).map_err(|e| WalletStorageError::AeadError(e.to_string()))?;
if let Err(e) = argon2.verify_password(passphrase.as_bytes(), &stored_hash) {
if let Err(e) = argon2.verify_password(passphrase.reveal(), &stored_hash) {
error!(target: LOG_TARGET, "Incorrect passphrase ({})", e);
return Err(WalletStorageError::InvalidPassphrase);
}

let derived_encryption_key = argon2
.hash_password_simple(passphrase.as_bytes(), encryption_salt.as_str())
.hash_password_simple(passphrase.reveal(), encryption_salt.as_str())
.map_err(|e| WalletStorageError::AeadError(e.to_string()))?
.hash
.ok_or_else(|| WalletStorageError::AeadError("Problem generating encryption key hash".to_string()))?;
Expand Down Expand Up @@ -770,7 +771,7 @@ impl Encryptable<Aes256Gcm> for ClientKeyValueSql {
mod test {
use tari_key_manager::cipher_seed::CipherSeed;
use tari_test_utils::random::string;
use tari_utilities::hex::Hex;
use tari_utilities::{hex::Hex, SafePassword};
use tempfile::tempdir;

use crate::storage::{
Expand Down Expand Up @@ -826,7 +827,7 @@ mod test {
let db_folder = db_tempdir.path().to_str().unwrap().to_string();
let connection = run_migration_and_create_sqlite_connection(&format!("{}{}", db_folder, db_name), 16).unwrap();

let passphrase = "an example very very secret key.".to_string();
let passphrase = SafePassword::from("an example very very secret key.".to_string());

assert!(WalletSqliteDatabase::new(connection.clone(), Some(passphrase.clone())).is_err());

Expand Down Expand Up @@ -879,7 +880,7 @@ mod test {
};
assert_eq!(seed, read_seed1);

let passphrase = "an example very very secret key.".to_string();
let passphrase = "an example very very secret key.".to_string().into();
db.apply_encryption(passphrase).unwrap();
let read_seed2 = match db.fetch(&DbKey::MasterSeed).unwrap().unwrap() {
DbValue::MasterSeed(sk) => sk,
Expand Down
3 changes: 2 additions & 1 deletion base_layer/wallet/src/storage/sqlite_utilities/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use std::{fs::File, path::Path, time::Duration};
use fs2::FileExt;
use log::*;
use tari_common_sqlite::sqlite_connection_pool::SqliteConnectionPool;
use tari_utilities::SafePassword;
pub use wallet_db_connection::WalletDbConnection;

use crate::{
Expand Down Expand Up @@ -125,7 +126,7 @@ pub fn acquire_exclusive_file_lock(db_path: &Path) -> Result<File, WalletStorage

pub fn initialize_sqlite_database_backends<P: AsRef<Path>>(
db_path: P,
passphrase: Option<String>,
passphrase: Option<SafePassword>,
sqlite_pool_size: usize,
) -> Result<
(
Expand Down
3 changes: 2 additions & 1 deletion base_layer/wallet/src/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ use tari_p2p::{
use tari_script::{script, ExecutionStack, TariScript};
use tari_service_framework::StackBuilder;
use tari_shutdown::ShutdownSignal;
use tari_utilities::SafePassword;

use crate::{
assets::{infrastructure::initializer::AssetManagerServiceInitializer, AssetManagerHandle},
Expand Down Expand Up @@ -685,7 +686,7 @@ where

/// Apply encryption to all the Wallet db backends. The Wallet backend will test if the db's are already encrypted
/// in which case this will fail.
pub async fn apply_encryption(&mut self, passphrase: String) -> Result<(), WalletError> {
pub async fn apply_encryption(&mut self, passphrase: SafePassword) -> Result<(), WalletError> {
debug!(target: LOG_TARGET, "Applying wallet encryption.");
let cipher = self.db.apply_encryption(passphrase).await?;
self.output_manager_service.apply_encryption(cipher.clone()).await?;
Expand Down
14 changes: 7 additions & 7 deletions base_layer/wallet/tests/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ use tari_p2p::{
use tari_script::{inputs, script};
use tari_shutdown::{Shutdown, ShutdownSignal};
use tari_test_utils::{collect_recv, random};
use tari_utilities::Hashable;
use tari_utilities::{Hashable, SafePassword};
use tari_wallet::{
contacts_service::{
handle::ContactsLivenessEvent,
Expand Down Expand Up @@ -114,7 +114,7 @@ async fn create_wallet(
database_name: &str,
factories: CryptoFactories,
shutdown_signal: ShutdownSignal,
passphrase: Option<String>,
passphrase: Option<SafePassword>,
recovery_seed: Option<CipherSeed>,
) -> Result<WalletSqlite, WalletError> {
const NETWORK: Network = Network::LocalNet;
Expand Down Expand Up @@ -316,14 +316,14 @@ async fn test_wallet() {
let current_wallet_path = alice_db_tempdir.path().join("alice_db").with_extension("sqlite3");

alice_wallet
.apply_encryption("It's turtles all the way down".to_string())
.apply_encryption("It's turtles all the way down".to_string().into())
.await
.unwrap();

// Second encryption should fail
#[allow(clippy::match_wild_err_arm)]
match alice_wallet
.apply_encryption("It's turtles all the way down".to_string())
.apply_encryption("It's turtles all the way down".to_string().into())
.await
{
Ok(_) => panic!("Should not be able to encrypt twice"),
Expand All @@ -342,15 +342,15 @@ async fn test_wallet() {
panic!("Should not be able to instantiate encrypted wallet without cipher");
}

let result = WalletSqliteDatabase::new(connection.clone(), Some("wrong passphrase".to_string()));
let result = WalletSqliteDatabase::new(connection.clone(), Some("wrong passphrase".to_string().into()));

if let Err(err) = result {
assert!(matches!(err, WalletStorageError::InvalidPassphrase));
} else {
panic!("Should not be able to instantiate encrypted wallet without cipher");
}

let db = WalletSqliteDatabase::new(connection, Some("It's turtles all the way down".to_string()))
let db = WalletSqliteDatabase::new(connection, Some("It's turtles all the way down".to_string().into()))
.expect("Should be able to instantiate db with cipher");
drop(db);

Expand All @@ -360,7 +360,7 @@ async fn test_wallet() {
"alice_db",
factories.clone(),
shutdown_a.to_signal(),
Some("It's turtles all the way down".to_string()),
Some("It's turtles all the way down".to_string().into()),
None,
)
.await
Expand Down
8 changes: 4 additions & 4 deletions base_layer/wallet_ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ use tari_p2p::{
};
use tari_script::{inputs, script};
use tari_shutdown::Shutdown;
use tari_utilities::{hex, hex::Hex};
use tari_utilities::{hex, hex::Hex, SafePassword};
use tari_wallet::{
connectivity_service::WalletConnectivityInterface,
contacts_service::storage::database::Contact,
Expand Down Expand Up @@ -4256,7 +4256,7 @@ pub unsafe extern "C" fn wallet_create(
.to_str()
.expect("A non-null passphrase should be able to be converted to string")
.to_owned();
Some(pf)
Some(SafePassword::from(pf))
};

let network = if network_str.is_null() {
Expand Down Expand Up @@ -6792,8 +6792,8 @@ pub unsafe extern "C" fn wallet_apply_encryption(

let pf = CStr::from_ptr(passphrase)
.to_str()
.expect("A non-null passphrase should be able to be converted to string")
.to_owned();
.map(|s| SafePassword::from(s.to_owned()))
.expect("A non-null passphrase should be able to be converted to string");

if let Err(e) = (*wallet).runtime.block_on((*wallet).wallet.apply_encryption(pf)) {
error = LibWalletError::from(e).code;
Expand Down

0 comments on commit d6359d3

Please sign in to comment.