Skip to content

Commit a568e04

Browse files
feat: provide password feedback (#5111)
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.
1 parent 8fa076a commit a568e04

File tree

3 files changed

+191
-18
lines changed

3 files changed

+191
-18
lines changed

Cargo.lock

+122-8
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

applications/tari_console_wallet/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ tracing = "0.1.26"
5252
unicode-segmentation = "1.6.0"
5353
unicode-width = "0.1"
5454
zeroize = "1"
55+
zxcvbn = "2"
5556

5657
[dependencies.tari_core]
5758
path = "../../base_layer/core"

applications/tari_console_wallet/src/init/mod.rs

+68-10
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ use tari_wallet::{
5959
WalletConfig,
6060
WalletSqlite,
6161
};
62+
use zxcvbn::zxcvbn;
6263

6364
use crate::{
6465
cli::Cli,
@@ -77,6 +78,40 @@ pub enum WalletBoot {
7778
Recovery,
7879
}
7980

81+
/// Get feedback, if available, for a weak passphrase
82+
fn get_password_feedback(passphrase: &SafePassword) -> Option<Vec<String>> {
83+
std::str::from_utf8(passphrase.reveal())
84+
.ok()
85+
.and_then(|passphrase| zxcvbn(passphrase, &[]).ok())
86+
.and_then(|scored| scored.feedback().to_owned())
87+
.map(|feedback| feedback.suggestions().to_owned())
88+
.map(|suggestion| suggestion.into_iter().map(|item| item.to_string()).collect())
89+
}
90+
91+
// Display password feedback to the user
92+
fn display_password_feedback(passphrase: &SafePassword) {
93+
if passphrase.reveal().is_empty() {
94+
// The passphrase is empty, which the scoring library doesn't handle
95+
println!("However, an empty password puts your wallet at risk against an attacker with access to this device.");
96+
println!("Use this only if you are sure that your device is safe from prying eyes!");
97+
println!();
98+
} else if let Some(feedback) = get_password_feedback(passphrase) {
99+
// The scoring library provided feedback
100+
println!(
101+
"However, the password you chose is weak; a determined attacker with access to your device may be able to \
102+
guess it."
103+
);
104+
println!("You may want to consider changing it to a stronger one.");
105+
println!("Here are some suggestions:");
106+
for suggestion in feedback {
107+
println!("- {}", suggestion);
108+
}
109+
println!();
110+
} else {
111+
// There is no feedback to provide
112+
}
113+
}
114+
80115
/// Gets the password provided by command line argument or environment variable if available.
81116
/// Otherwise prompts for the password to be typed in.
82117
pub fn get_or_prompt_password(
@@ -105,15 +140,7 @@ pub fn get_or_prompt_password(
105140
}
106141

107142
fn prompt_password(prompt: &str) -> Result<SafePassword, ExitError> {
108-
let password = loop {
109-
let pass = prompt_password_stdout(prompt).map_err(|e| ExitError::new(ExitCode::IOError, e))?;
110-
if pass.is_empty() {
111-
println!("Password cannot be empty!");
112-
continue;
113-
} else {
114-
break pass;
115-
}
116-
};
143+
let password = prompt_password_stdout(prompt).map_err(|e| ExitError::new(ExitCode::IOError, e))?;
117144

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

147-
println!("Wallet password changed successfully.");
174+
println!("Passwords match.");
175+
176+
// If the passphrase is weak, let the user know
177+
display_password_feedback(&passphrase);
178+
179+
// TODO: remove this warning when this functionality is added
180+
println!();
181+
println!("WARNING: Password change functionality is not yet completed, so continue to use your existing password!");
148182

149183
Ok(())
150184
}
@@ -625,6 +659,11 @@ pub(crate) fn boot_with_password(
625659
return Err(ExitError::new(ExitCode::InputError, "Passwords don't match!"));
626660
}
627661

662+
println!("Passwords match.");
663+
664+
// If the passphrase is weak, let the user know
665+
display_password_feedback(&password);
666+
628667
password
629668
},
630669
WalletBoot::Existing | WalletBoot::Recovery => {
@@ -635,3 +674,22 @@ pub(crate) fn boot_with_password(
635674

636675
Ok((boot_mode, password))
637676
}
677+
678+
#[cfg(test)]
679+
mod test {
680+
use tari_utilities::SafePassword;
681+
682+
use super::get_password_feedback;
683+
684+
#[test]
685+
fn weak_password() {
686+
let weak_password = SafePassword::from("weak");
687+
assert!(get_password_feedback(&weak_password).is_some());
688+
}
689+
690+
#[test]
691+
fn strong_password() {
692+
let strong_password = SafePassword::from("This is a reasonably strong password!");
693+
assert!(get_password_feedback(&strong_password).is_none());
694+
}
695+
}

0 commit comments

Comments
 (0)