fix passkey cli argument being printed in plaintext with debug logging (#229)

fixes #228
This commit is contained in:
Carson McManus 2023-06-25 20:23:26 -04:00 committed by GitHub
parent ac7717bde8
commit a062e718d7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 39 additions and 23 deletions

View file

@ -2,6 +2,7 @@ use crate::accountmanager::legacy::SdaManifest;
pub use crate::encryption::EntryEncryptionParams; pub use crate::encryption::EntryEncryptionParams;
use crate::encryption::EntryEncryptor; use crate::encryption::EntryEncryptor;
use log::*; use log::*;
use secrecy::{ExposeSecret, SecretString};
use std::collections::HashMap; use std::collections::HashMap;
use std::fs::File; use std::fs::File;
use std::io::{BufReader, Read, Write}; use std::io::{BufReader, Read, Write};
@ -21,7 +22,7 @@ pub struct AccountManager {
manifest: Manifest, manifest: Manifest,
accounts: HashMap<String, Arc<Mutex<SteamGuardAccount>>>, accounts: HashMap<String, Arc<Mutex<SteamGuardAccount>>>,
folder: String, folder: String,
passkey: Option<String>, passkey: Option<SecretString>,
} }
impl AccountManager { 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. /// 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<String>) { pub fn submit_passkey(&mut self, passkey: Option<SecretString>) {
if let Some(p) = passkey.as_ref() { if let Some(p) = passkey.as_ref() {
if p.is_empty() { if p.expose_secret().is_empty() {
panic!("Encryption passkey cannot be empty"); panic!("Encryption passkey cannot be empty");
} }
} }
@ -199,9 +200,11 @@ impl AccountManager {
); );
let final_buffer: Vec<u8> = match (&self.passkey, entry.encryption.as_ref()) { let final_buffer: Vec<u8> = match (&self.passkey, entry.encryption.as_ref()) {
(Some(passkey), Some(params)) => { (Some(passkey), Some(params)) => crate::encryption::LegacySdaCompatible::encrypt(
crate::encryption::LegacySdaCompatible::encrypt(passkey, params, serialized)? passkey.expose_secret(),
} params,
serialized,
)?,
(None, Some(_)) => { (None, Some(_)) => {
bail!("maFiles are encrypted, but no passkey was provided."); bail!("maFiles are encrypted, but no passkey was provided.");
} }
@ -338,7 +341,7 @@ trait EntryLoader<T> {
fn load( fn load(
&self, &self,
path: &Path, path: &Path,
passkey: Option<&String>, passkey: Option<&SecretString>,
encryption_params: Option<&EntryEncryptionParams>, encryption_params: Option<&EntryEncryptionParams>,
) -> anyhow::Result<T, ManifestAccountLoadError>; ) -> anyhow::Result<T, ManifestAccountLoadError>;
} }
@ -347,7 +350,7 @@ impl EntryLoader<SteamGuardAccount> for ManifestEntry {
fn load( fn load(
&self, &self,
path: &Path, path: &Path,
passkey: Option<&String>, passkey: Option<&SecretString>,
encryption_params: Option<&EntryEncryptionParams>, encryption_params: Option<&EntryEncryptionParams>,
) -> anyhow::Result<SteamGuardAccount, ManifestAccountLoadError> { ) -> anyhow::Result<SteamGuardAccount, ManifestAccountLoadError> {
debug!("loading entry: {:?}", path); debug!("loading entry: {:?}", path);
@ -357,8 +360,11 @@ impl EntryLoader<SteamGuardAccount> for ManifestEntry {
(Some(passkey), Some(params)) => { (Some(passkey), Some(params)) => {
let mut ciphertext: Vec<u8> = vec![]; let mut ciphertext: Vec<u8> = vec![];
reader.read_to_end(&mut ciphertext)?; reader.read_to_end(&mut ciphertext)?;
let plaintext = let plaintext = crate::encryption::LegacySdaCompatible::decrypt(
crate::encryption::LegacySdaCompatible::decrypt(passkey, params, ciphertext)?; passkey.expose_secret(),
params,
ciphertext,
)?;
if plaintext[0] != b'{' && plaintext[plaintext.len() - 1] != b'}' { if plaintext[0] != b'{' && plaintext[plaintext.len() - 1] != b'}' {
return Err(ManifestAccountLoadError::IncorrectPasskey); return Err(ManifestAccountLoadError::IncorrectPasskey);
} }
@ -494,7 +500,7 @@ mod tests {
#[test] #[test]
fn test_should_save_and_load_manifest_encrypted() -> anyhow::Result<()> { 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 tmp_dir = TempDir::new("steamguard-cli-test")?;
let manifest_path = tmp_dir.path().join("manifest.json"); let manifest_path = tmp_dir.path().join("manifest.json");
let mut manager = AccountManager::new(manifest_path.as_path()); let mut manager = AccountManager::new(manifest_path.as_path());
@ -559,7 +565,7 @@ mod tests {
#[test] #[test]
fn test_should_save_and_load_manifest_encrypted_longer() -> anyhow::Result<()> { 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 tmp_dir = TempDir::new("steamguard-cli-test")?;
let manifest_path = tmp_dir.path().join("manifest.json"); let manifest_path = tmp_dir.path().join("manifest.json");
let mut manager = AccountManager::new(manifest_path.as_path()); let mut manager = AccountManager::new(manifest_path.as_path());

View file

@ -72,7 +72,7 @@ impl EntryLoader<SdaAccount> for SdaManifestEntry {
fn load( fn load(
&self, &self,
path: &Path, path: &Path,
passkey: Option<&String>, passkey: Option<&SecretString>,
encryption_params: Option<&EntryEncryptionParams>, encryption_params: Option<&EntryEncryptionParams>,
) -> anyhow::Result<SdaAccount, ManifestAccountLoadError> { ) -> anyhow::Result<SdaAccount, ManifestAccountLoadError> {
debug!("loading entry: {:?}", path); debug!("loading entry: {:?}", path);
@ -82,8 +82,11 @@ impl EntryLoader<SdaAccount> for SdaManifestEntry {
(Some(passkey), Some(params)) => { (Some(passkey), Some(params)) => {
let mut ciphertext: Vec<u8> = vec![]; let mut ciphertext: Vec<u8> = vec![];
reader.read_to_end(&mut ciphertext)?; reader.read_to_end(&mut ciphertext)?;
let plaintext = let plaintext = crate::encryption::LegacySdaCompatible::decrypt(
crate::encryption::LegacySdaCompatible::decrypt(passkey, params, ciphertext)?; passkey.expose_secret(),
params,
ciphertext,
)?;
if plaintext[0] != b'{' && plaintext[plaintext.len() - 1] != b'}' { if plaintext[0] != b'{' && plaintext[plaintext.len() - 1] != b'}' {
return Err(ManifestAccountLoadError::IncorrectPasskey); return Err(ManifestAccountLoadError::IncorrectPasskey);
} }

View file

@ -1,6 +1,7 @@
use std::{fs::File, io::Read, path::Path}; use std::{fs::File, io::Read, path::Path};
use log::debug; use log::debug;
use secrecy::SecretString;
use serde::{de::Error, Deserialize}; use serde::{de::Error, Deserialize};
use steamguard::SteamGuardAccount; use steamguard::SteamGuardAccount;
use thiserror::Error; use thiserror::Error;
@ -13,7 +14,7 @@ use super::{
pub(crate) fn load_and_migrate( pub(crate) fn load_and_migrate(
manifest_path: &Path, manifest_path: &Path,
passkey: Option<&String>, passkey: Option<&SecretString>,
) -> Result<(Manifest, Vec<SteamGuardAccount>), MigrationError> { ) -> Result<(Manifest, Vec<SteamGuardAccount>), MigrationError> {
backup_file(manifest_path)?; backup_file(manifest_path)?;
let parent = manifest_path.parent().unwrap(); let parent = manifest_path.parent().unwrap();
@ -32,7 +33,7 @@ pub(crate) fn load_and_migrate(
fn do_migrate( fn do_migrate(
manifest_path: &Path, manifest_path: &Path,
passkey: Option<&String>, passkey: Option<&SecretString>,
) -> Result<(Manifest, Vec<SteamGuardAccount>), MigrationError> { ) -> Result<(Manifest, Vec<SteamGuardAccount>), MigrationError> {
let mut file = File::open(manifest_path)?; let mut file = File::open(manifest_path)?;
let mut buffer = String::new(); let mut buffer = String::new();
@ -117,7 +118,7 @@ impl MigratingManifest {
pub fn load_all_accounts( pub fn load_all_accounts(
&self, &self,
folder: &Path, folder: &Path,
passkey: Option<&String>, passkey: Option<&SecretString>,
) -> anyhow::Result<Vec<MigratingAccount>> { ) -> anyhow::Result<Vec<MigratingAccount>> {
debug!("loading all accounts for migration"); debug!("loading all accounts for migration");
let accounts = match self { let accounts = match self {
@ -271,7 +272,7 @@ mod tests {
#[derive(Debug)] #[derive(Debug)]
struct Test { struct Test {
manifest: &'static str, manifest: &'static str,
passkey: Option<String>, passkey: Option<SecretString>,
} }
let cases = vec![ let cases = vec![
Test { Test {
@ -280,7 +281,7 @@ mod tests {
}, },
Test { Test {
manifest: "src/fixtures/maFiles/compat/1-account-encrypted/manifest.json", manifest: "src/fixtures/maFiles/compat/1-account-encrypted/manifest.json",
passkey: Some("password".into()), passkey: Some(SecretString::new("password".into())),
}, },
Test { Test {
manifest: "src/fixtures/maFiles/compat/2-account/manifest.json", manifest: "src/fixtures/maFiles/compat/2-account/manifest.json",

View file

@ -2,6 +2,7 @@ use std::sync::{Arc, Mutex};
use clap::{clap_derive::ArgEnum, Parser}; use clap::{clap_derive::ArgEnum, Parser};
use clap_complete::Shell; use clap_complete::Shell;
use secrecy::SecretString;
use std::str::FromStr; use std::str::FromStr;
use steamguard::SteamGuardAccount; use steamguard::SteamGuardAccount;
@ -101,7 +102,7 @@ pub(crate) struct GlobalArgs {
env = "STEAMGUARD_CLI_PASSKEY", env = "STEAMGUARD_CLI_PASSKEY",
help = "Specify your encryption passkey." help = "Specify your encryption passkey."
)] )]
pub passkey: Option<String>, pub passkey: Option<SecretString>,
#[clap(short, long, arg_enum, default_value_t=Verbosity::Info, help = "Set the log level. Be warned, trace is capable of printing sensitive data.")] #[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, pub verbosity: Verbosity,

View file

@ -32,6 +32,7 @@ fn load_accounts_with_prompts(manager: &mut AccountManager) -> anyhow::Result<()
error!("Incorrect passkey"); error!("Incorrect passkey");
} }
let passkey = rpassword::prompt_password_stdout("Enter encryption passkey: ").ok(); let passkey = rpassword::prompt_password_stdout("Enter encryption passkey: ").ok();
let passkey = passkey.map(SecretString::new);
manager.submit_passkey(passkey); manager.submit_passkey(passkey);
} }
Err(e) => { Err(e) => {

View file

@ -27,6 +27,7 @@ impl ManifestCommand for EncryptCommand {
} }
error!("Passkeys do not match, try again."); error!("Passkeys do not match, try again.");
} }
let passkey = passkey.map(SecretString::new);
manager.submit_passkey(passkey); manager.submit_passkey(passkey);
} }
manager.load_accounts()?; manager.load_accounts()?;

View file

@ -1,6 +1,7 @@
extern crate rpassword; extern crate rpassword;
use clap::Parser; use clap::Parser;
use log::*; use log::*;
use secrecy::SecretString;
use std::{ use std::{
io::Write, io::Write,
path::Path, path::Path,
@ -164,7 +165,8 @@ fn run(args: commands::Args) -> anyhow::Result<()> {
if manager.has_passkey() { if manager.has_passkey() {
error!("Incorrect 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); manager.submit_passkey(passkey);
} }
Err(e) => { Err(e) => {
@ -193,7 +195,8 @@ fn run(args: commands::Args) -> anyhow::Result<()> {
if manager.has_passkey() { if manager.has_passkey() {
error!("Incorrect 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); manager.submit_passkey(passkey);
} }
Err(e) => { Err(e) => {