Skip to content

Commit

Permalink
feat: provide password feedback (#5111)
Browse files Browse the repository at this point in the history
Description
---
Adds wallet password complexity feedback. Allows empty passwords. Adds a warning indicating that password changing functionality is [not yet implemented](#5003). Adds tests.

Closes [issue 5101](#5101).

Motivation and Context
---
The only check on a wallet password is that it not be empty. This introduces two issues:
- The user has no feedback on the practical strength of their password.
- The user may specifically wish not to set a password for fail-available backups.

This PR uses the [zxcvbn](https://crates.io/crates/zxcvbn) password complexity library to score a password and provide actionable feedback to the user. When the user enters a new password or changes their password, feedback is displayed if applicable. This is informational; the user is free to ignore the feedback if they wish.

Further, the user is now allowed to set an empty password, which may be desired for backups that must fail available. A warning is displayed if this happens.

Finally, a warning message is displayed during the password changing process to indicate that this functionality is incomplete.

How Has This Been Tested?
---
Existing CI passes. New tests pass. Tested manually for new wallets.
  • Loading branch information
AaronFeickert authored Jan 18, 2023
1 parent 8fa076a commit a568e04
Show file tree
Hide file tree
Showing 3 changed files with 191 additions and 18 deletions.
130 changes: 122 additions & 8 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions applications/tari_console_wallet/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ tracing = "0.1.26"
unicode-segmentation = "1.6.0"
unicode-width = "0.1"
zeroize = "1"
zxcvbn = "2"

[dependencies.tari_core]
path = "../../base_layer/core"
Expand Down
78 changes: 68 additions & 10 deletions applications/tari_console_wallet/src/init/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ use tari_wallet::{
WalletConfig,
WalletSqlite,
};
use zxcvbn::zxcvbn;

use crate::{
cli::Cli,
Expand All @@ -77,6 +78,40 @@ pub enum WalletBoot {
Recovery,
}

/// Get feedback, if available, for a weak passphrase
fn get_password_feedback(passphrase: &SafePassword) -> Option<Vec<String>> {
std::str::from_utf8(passphrase.reveal())
.ok()
.and_then(|passphrase| zxcvbn(passphrase, &[]).ok())
.and_then(|scored| scored.feedback().to_owned())
.map(|feedback| feedback.suggestions().to_owned())
.map(|suggestion| suggestion.into_iter().map(|item| item.to_string()).collect())
}

// Display password feedback to the user
fn display_password_feedback(passphrase: &SafePassword) {
if passphrase.reveal().is_empty() {
// The passphrase is empty, which the scoring library doesn't handle
println!("However, an empty password puts your wallet at risk against an attacker with access to this device.");
println!("Use this only if you are sure that your device is safe from prying eyes!");
println!();
} else if let Some(feedback) = get_password_feedback(passphrase) {
// The scoring library provided feedback
println!(
"However, the password you chose is weak; a determined attacker with access to your device may be able to \
guess it."
);
println!("You may want to consider changing it to a stronger one.");
println!("Here are some suggestions:");
for suggestion in feedback {
println!("- {}", suggestion);
}
println!();
} else {
// There is no feedback to provide
}
}

/// 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(
Expand Down Expand Up @@ -105,15 +140,7 @@ pub fn get_or_prompt_password(
}

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() {
println!("Password cannot be empty!");
continue;
} else {
break pass;
}
};
let password = prompt_password_stdout(prompt).map_err(|e| ExitError::new(ExitCode::IOError, e))?;

Ok(SafePassword::from(password))
}
Expand Down Expand Up @@ -144,7 +171,14 @@ pub async fn change_password(
// .await
// .map_err(|e| ExitError::new(ExitCode::WalletError, e))?;

println!("Wallet password changed successfully.");
println!("Passwords match.");

// If the passphrase is weak, let the user know
display_password_feedback(&passphrase);

// TODO: remove this warning when this functionality is added
println!();
println!("WARNING: Password change functionality is not yet completed, so continue to use your existing password!");

Ok(())
}
Expand Down Expand Up @@ -625,6 +659,11 @@ pub(crate) fn boot_with_password(
return Err(ExitError::new(ExitCode::InputError, "Passwords don't match!"));
}

println!("Passwords match.");

// If the passphrase is weak, let the user know
display_password_feedback(&password);

password
},
WalletBoot::Existing | WalletBoot::Recovery => {
Expand All @@ -635,3 +674,22 @@ pub(crate) fn boot_with_password(

Ok((boot_mode, password))
}

#[cfg(test)]
mod test {
use tari_utilities::SafePassword;

use super::get_password_feedback;

#[test]
fn weak_password() {
let weak_password = SafePassword::from("weak");
assert!(get_password_feedback(&weak_password).is_some());
}

#[test]
fn strong_password() {
let strong_password = SafePassword::from("This is a reasonably strong password!");
assert!(get_password_feedback(&strong_password).is_none());
}
}

0 comments on commit a568e04

Please sign in to comment.