From a062e718d79204a91ad489d8d247e4685ba4b0fb Mon Sep 17 00:00:00 2001 From: Carson McManus Date: Sun, 25 Jun 2023 20:23:26 -0400 Subject: [PATCH] fix passkey cli argument being printed in plaintext with debug logging (#229) fixes #228 --- src/accountmanager.rs | 30 ++++++++++++++++++------------ src/accountmanager/legacy.rs | 9 ++++++--- src/accountmanager/migrate.rs | 11 ++++++----- src/commands.rs | 3 ++- src/commands/decrypt.rs | 1 + src/commands/encrypt.rs | 1 + src/main.rs | 7 +++++-- 7 files changed, 39 insertions(+), 23 deletions(-) diff --git a/src/accountmanager.rs b/src/accountmanager.rs index 608f2b6..f35cd1c 100644 --- a/src/accountmanager.rs +++ b/src/accountmanager.rs @@ -2,6 +2,7 @@ use crate::accountmanager::legacy::SdaManifest; pub use crate::encryption::EntryEncryptionParams; use crate::encryption::EntryEncryptor; use log::*; +use secrecy::{ExposeSecret, SecretString}; use std::collections::HashMap; use std::fs::File; use std::io::{BufReader, Read, Write}; @@ -21,7 +22,7 @@ pub struct AccountManager { manifest: Manifest, accounts: HashMap>>, folder: String, - passkey: Option, + passkey: Option, } impl AccountManager { @@ -73,9 +74,9 @@ impl AccountManager { } /// Tells the manager to keep track of the encryption passkey, and use it for encryption when loading or saving accounts. - pub fn submit_passkey(&mut self, passkey: Option) { + pub fn submit_passkey(&mut self, passkey: Option) { if let Some(p) = passkey.as_ref() { - if p.is_empty() { + if p.expose_secret().is_empty() { panic!("Encryption passkey cannot be empty"); } } @@ -199,9 +200,11 @@ impl AccountManager { ); let final_buffer: Vec = match (&self.passkey, entry.encryption.as_ref()) { - (Some(passkey), Some(params)) => { - crate::encryption::LegacySdaCompatible::encrypt(passkey, params, serialized)? - } + (Some(passkey), Some(params)) => crate::encryption::LegacySdaCompatible::encrypt( + passkey.expose_secret(), + params, + serialized, + )?, (None, Some(_)) => { bail!("maFiles are encrypted, but no passkey was provided."); } @@ -338,7 +341,7 @@ trait EntryLoader { fn load( &self, path: &Path, - passkey: Option<&String>, + passkey: Option<&SecretString>, encryption_params: Option<&EntryEncryptionParams>, ) -> anyhow::Result; } @@ -347,7 +350,7 @@ impl EntryLoader for ManifestEntry { fn load( &self, path: &Path, - passkey: Option<&String>, + passkey: Option<&SecretString>, encryption_params: Option<&EntryEncryptionParams>, ) -> anyhow::Result { debug!("loading entry: {:?}", path); @@ -357,8 +360,11 @@ impl EntryLoader for ManifestEntry { (Some(passkey), Some(params)) => { let mut ciphertext: Vec = vec![]; reader.read_to_end(&mut ciphertext)?; - let plaintext = - crate::encryption::LegacySdaCompatible::decrypt(passkey, params, ciphertext)?; + let plaintext = crate::encryption::LegacySdaCompatible::decrypt( + passkey.expose_secret(), + params, + ciphertext, + )?; if plaintext[0] != b'{' && plaintext[plaintext.len() - 1] != b'}' { return Err(ManifestAccountLoadError::IncorrectPasskey); } @@ -494,7 +500,7 @@ mod tests { #[test] fn test_should_save_and_load_manifest_encrypted() -> anyhow::Result<()> { - let passkey = Some("password".into()); + let passkey = Some(SecretString::new("password".into())); let tmp_dir = TempDir::new("steamguard-cli-test")?; let manifest_path = tmp_dir.path().join("manifest.json"); let mut manager = AccountManager::new(manifest_path.as_path()); @@ -559,7 +565,7 @@ mod tests { #[test] fn test_should_save_and_load_manifest_encrypted_longer() -> anyhow::Result<()> { - let passkey = Some("password".into()); + let passkey = Some(SecretString::new("password".into())); let tmp_dir = TempDir::new("steamguard-cli-test")?; let manifest_path = tmp_dir.path().join("manifest.json"); let mut manager = AccountManager::new(manifest_path.as_path()); diff --git a/src/accountmanager/legacy.rs b/src/accountmanager/legacy.rs index cf0c9f2..1a957b5 100644 --- a/src/accountmanager/legacy.rs +++ b/src/accountmanager/legacy.rs @@ -72,7 +72,7 @@ impl EntryLoader for SdaManifestEntry { fn load( &self, path: &Path, - passkey: Option<&String>, + passkey: Option<&SecretString>, encryption_params: Option<&EntryEncryptionParams>, ) -> anyhow::Result { debug!("loading entry: {:?}", path); @@ -82,8 +82,11 @@ impl EntryLoader for SdaManifestEntry { (Some(passkey), Some(params)) => { let mut ciphertext: Vec = vec![]; reader.read_to_end(&mut ciphertext)?; - let plaintext = - crate::encryption::LegacySdaCompatible::decrypt(passkey, params, ciphertext)?; + let plaintext = crate::encryption::LegacySdaCompatible::decrypt( + passkey.expose_secret(), + params, + ciphertext, + )?; if plaintext[0] != b'{' && plaintext[plaintext.len() - 1] != b'}' { return Err(ManifestAccountLoadError::IncorrectPasskey); } diff --git a/src/accountmanager/migrate.rs b/src/accountmanager/migrate.rs index 36fca31..3abaf07 100644 --- a/src/accountmanager/migrate.rs +++ b/src/accountmanager/migrate.rs @@ -1,6 +1,7 @@ use std::{fs::File, io::Read, path::Path}; use log::debug; +use secrecy::SecretString; use serde::{de::Error, Deserialize}; use steamguard::SteamGuardAccount; use thiserror::Error; @@ -13,7 +14,7 @@ use super::{ pub(crate) fn load_and_migrate( manifest_path: &Path, - passkey: Option<&String>, + passkey: Option<&SecretString>, ) -> Result<(Manifest, Vec), MigrationError> { backup_file(manifest_path)?; let parent = manifest_path.parent().unwrap(); @@ -32,7 +33,7 @@ pub(crate) fn load_and_migrate( fn do_migrate( manifest_path: &Path, - passkey: Option<&String>, + passkey: Option<&SecretString>, ) -> Result<(Manifest, Vec), MigrationError> { let mut file = File::open(manifest_path)?; let mut buffer = String::new(); @@ -117,7 +118,7 @@ impl MigratingManifest { pub fn load_all_accounts( &self, folder: &Path, - passkey: Option<&String>, + passkey: Option<&SecretString>, ) -> anyhow::Result> { debug!("loading all accounts for migration"); let accounts = match self { @@ -271,7 +272,7 @@ mod tests { #[derive(Debug)] struct Test { manifest: &'static str, - passkey: Option, + passkey: Option, } let cases = vec![ Test { @@ -280,7 +281,7 @@ mod tests { }, Test { manifest: "src/fixtures/maFiles/compat/1-account-encrypted/manifest.json", - passkey: Some("password".into()), + passkey: Some(SecretString::new("password".into())), }, Test { manifest: "src/fixtures/maFiles/compat/2-account/manifest.json", diff --git a/src/commands.rs b/src/commands.rs index cf39f8b..fa76653 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -2,6 +2,7 @@ use std::sync::{Arc, Mutex}; use clap::{clap_derive::ArgEnum, Parser}; use clap_complete::Shell; +use secrecy::SecretString; use std::str::FromStr; use steamguard::SteamGuardAccount; @@ -101,7 +102,7 @@ pub(crate) struct GlobalArgs { env = "STEAMGUARD_CLI_PASSKEY", help = "Specify your encryption passkey." )] - pub passkey: Option, + pub passkey: Option, #[clap(short, long, arg_enum, default_value_t=Verbosity::Info, help = "Set the log level. Be warned, trace is capable of printing sensitive data.")] pub verbosity: Verbosity, diff --git a/src/commands/decrypt.rs b/src/commands/decrypt.rs index 7f847fb..cdbf93c 100644 --- a/src/commands/decrypt.rs +++ b/src/commands/decrypt.rs @@ -32,6 +32,7 @@ fn load_accounts_with_prompts(manager: &mut AccountManager) -> anyhow::Result<() error!("Incorrect passkey"); } let passkey = rpassword::prompt_password_stdout("Enter encryption passkey: ").ok(); + let passkey = passkey.map(SecretString::new); manager.submit_passkey(passkey); } Err(e) => { diff --git a/src/commands/encrypt.rs b/src/commands/encrypt.rs index b475db6..e4ed4e3 100644 --- a/src/commands/encrypt.rs +++ b/src/commands/encrypt.rs @@ -27,6 +27,7 @@ impl ManifestCommand for EncryptCommand { } error!("Passkeys do not match, try again."); } + let passkey = passkey.map(SecretString::new); manager.submit_passkey(passkey); } manager.load_accounts()?; diff --git a/src/main.rs b/src/main.rs index 9d2727a..39ef85c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,6 +1,7 @@ extern crate rpassword; use clap::Parser; use log::*; +use secrecy::SecretString; use std::{ io::Write, path::Path, @@ -164,7 +165,8 @@ fn run(args: commands::Args) -> anyhow::Result<()> { if manager.has_passkey() { error!("Incorrect passkey"); } - passkey = rpassword::prompt_password_stderr("Enter encryption passkey: ").ok(); + let raw = rpassword::prompt_password_stdout("Enter encryption passkey: ")?; + passkey = Some(SecretString::new(raw)); manager.submit_passkey(passkey); } Err(e) => { @@ -193,7 +195,8 @@ fn run(args: commands::Args) -> anyhow::Result<()> { if manager.has_passkey() { error!("Incorrect passkey"); } - passkey = rpassword::prompt_password_stdout("Enter encryption passkey: ").ok(); + let raw = rpassword::prompt_password_stdout("Enter encryption passkey: ")?; + passkey = Some(SecretString::new(raw)); manager.submit_passkey(passkey); } Err(e) => {