From deabfc299d70a8578a55f19216623fe06e90cd0a Mon Sep 17 00:00:00 2001 From: Carson McManus Date: Sun, 19 Jun 2022 14:09:08 -0400 Subject: [PATCH] protect top level secrets in SteamGuardAccount except for session --- src/accountmanager.rs | 25 +++++++++--------- src/main.rs | 6 ++--- steamguard/src/lib.rs | 29 ++++++++++++-------- steamguard/src/secret_string.rs | 47 +++++++++++++++++++++++++++++++++ steamguard/src/steamapi.rs | 8 +++--- 5 files changed, 85 insertions(+), 30 deletions(-) create mode 100644 steamguard/src/secret_string.rs diff --git a/src/accountmanager.rs b/src/accountmanager.rs index 73dfb60..18a24d4 100644 --- a/src/accountmanager.rs +++ b/src/accountmanager.rs @@ -372,7 +372,8 @@ impl From for ManifestAccountLoadError { #[cfg(test)] mod tests { use super::*; - use tempdir::TempDir; + use steamguard::ExposeSecret; +use tempdir::TempDir; #[test] fn test_should_save_new_manifest() { @@ -390,7 +391,7 @@ mod tests { let mut manifest = Manifest::new(manifest_path.as_path()); let mut account = SteamGuardAccount::new(); account.account_name = "asdf1234".into(); - account.revocation_code = "R12345".into(); + account.revocation_code = String::from("R12345").into(); account.shared_secret = steamguard::token::TwoFactorSecret::parse_shared_secret( "zvIayp3JPvtvX/QGHqsqKBk/44s=".into(), )?; @@ -419,7 +420,7 @@ mod tests { .get_account(&account_name)? .lock() .unwrap() - .revocation_code, + .revocation_code.expose_secret(), "R12345" ); assert_eq!( @@ -443,7 +444,7 @@ mod tests { let mut manifest = Manifest::new(manifest_path.as_path()); let mut account = SteamGuardAccount::new(); account.account_name = "asdf1234".into(); - account.revocation_code = "R12345".into(); + account.revocation_code = String::from("R12345").into(); account.shared_secret = steamguard::token::TwoFactorSecret::parse_shared_secret( "zvIayp3JPvtvX/QGHqsqKBk/44s=".into(), )?; @@ -479,7 +480,7 @@ mod tests { .get_account(&account_name)? .lock() .unwrap() - .revocation_code, + .revocation_code.expose_secret(), "R12345" ); assert_eq!( @@ -504,12 +505,12 @@ mod tests { let mut manifest = Manifest::new(manifest_path.as_path()); let mut account = SteamGuardAccount::new(); account.account_name = "asdf1234".into(); - account.revocation_code = "R12345".into(); + account.revocation_code = String::from("R12345").into(); account.shared_secret = steamguard::token::TwoFactorSecret::parse_shared_secret( "zvIayp3JPvtvX/QGHqsqKBk/44s=".into(), ) .unwrap(); - account.uri = "otpauth://;laksdjf;lkasdjf;lkasdj;flkasdjlkf;asjdlkfjslk;adjfl;kasdjf;lksdjflk;asjd;lfajs;ldkfjaslk;djf;lsakdjf;lksdj".into(); + account.uri = String::from("otpauth://;laksdjf;lkasdjf;lkasdj;flkasdjlkf;asjdlkfjslk;adjfl;kasdjf;lksdjflk;asjd;lfajs;ldkfjaslk;djf;lsakdjf;lksdj").into(); account.token_gid = "asdf1234".into(); manifest.add_account(account); manifest.submit_passkey(passkey.clone()); @@ -539,7 +540,7 @@ mod tests { .get_account(&account_name)? .lock() .unwrap() - .revocation_code, + .revocation_code.expose_secret(), "R12345" ); assert_eq!( @@ -564,7 +565,7 @@ mod tests { let mut manifest = Manifest::new(manifest_path.as_path()); let mut account = SteamGuardAccount::new(); account.account_name = "asdf1234".into(); - account.revocation_code = "R12345".into(); + account.revocation_code = String::from("R12345").into(); account.shared_secret = steamguard::token::TwoFactorSecret::parse_shared_secret( "zvIayp3JPvtvX/QGHqsqKBk/44s=".into(), ) @@ -603,7 +604,7 @@ mod tests { .get_account(&account_name)? .lock() .unwrap() - .revocation_code, + .revocation_code.expose_secret(), "R12345" ); assert_eq!( @@ -690,7 +691,7 @@ mod tests { let account_name = manifest.entries[0].account_name.clone(); let account = manifest.get_account(&account_name)?; assert_eq!(account_name, account.lock().unwrap().account_name); - assert_eq!(account.lock().unwrap().revocation_code, "R12345"); + assert_eq!(account.lock().unwrap().revocation_code.expose_secret(), "R12345"); assert_eq!( account.lock().unwrap().session.as_ref().unwrap().steam_id, 1234 @@ -699,7 +700,7 @@ mod tests { let account_name = manifest.entries[1].account_name.clone(); let account = manifest.get_account(&account_name)?; assert_eq!(account_name, account.lock().unwrap().account_name); - assert_eq!(account.lock().unwrap().revocation_code, "R56789"); + assert_eq!(account.lock().unwrap().revocation_code.expose_secret(), "R56789"); assert_eq!( account.lock().unwrap().session.as_ref().unwrap().steam_id, 5678 diff --git a/src/main.rs b/src/main.rs index d65f3d5..0153403 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,7 +8,7 @@ use std::{ }; use steamguard::{ steamapi, AccountLinkError, AccountLinker, Confirmation, FinalizeLinkError, LoginError, - SteamGuardAccount, UserLogin, + SteamGuardAccount, UserLogin, ExposeSecret, }; use crate::accountmanager::ManifestAccountLoadError; @@ -391,7 +391,7 @@ fn do_subcmd_setup( let account_arc = manifest.get_account(&account_name).unwrap(); let mut account = account_arc.lock().unwrap(); - println!("Authenticator has not yet been linked. Before continuing with finalization, please take the time to write down your revocation code: {}", account.revocation_code); + println!("Authenticator has not yet been linked. Before continuing with finalization, please take the time to write down your revocation code: {}", account.revocation_code.expose_secret()); tui::pause(); debug!("attempting link finalization"); @@ -430,7 +430,7 @@ fn do_subcmd_setup( println!( "Authenticator has been finalized. Please actually write down your revocation code: {}", - account.revocation_code + account.revocation_code.expose_secret() ); return Ok(()); diff --git a/steamguard/src/lib.rs b/steamguard/src/lib.rs index f1f3a38..5457741 100644 --- a/steamguard/src/lib.rs +++ b/steamguard/src/lib.rs @@ -13,6 +13,7 @@ use reqwest::{ use scraper::{Html, Selector}; use serde::{Deserialize, Serialize}; use std::{collections::HashMap, convert::TryInto}; +pub use secrecy::{ExposeSecret, SecretString}; use steamapi::SteamApiClient; pub use userlogin::{LoginError, UserLogin}; #[macro_use] @@ -27,6 +28,7 @@ mod confirmation; pub mod steamapi; pub mod token; mod userlogin; +mod secret_string; // const STEAMAPI_BASE: String = "https://api.steampowered.com"; // const COMMUNITY_BASE: String = "https://steamcommunity.com"; @@ -39,19 +41,24 @@ extern crate base64; extern crate cookie; extern crate hmacsha1; + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct SteamGuardAccount { pub account_name: String, pub serial_number: String, - pub revocation_code: String, + #[serde(with = "secret_string")] + pub revocation_code: SecretString, pub shared_secret: TwoFactorSecret, pub token_gid: String, - pub identity_secret: String, + #[serde(with = "secret_string")] + pub identity_secret: SecretString, pub server_time: u64, - pub uri: String, + #[serde(with = "secret_string")] + pub uri: SecretString, pub fully_enrolled: bool, pub device_id: String, - pub secret_1: String, + #[serde(with = "secret_string")] + pub secret_1: SecretString, #[serde(default, rename = "Session")] pub session: Option, } @@ -75,15 +82,15 @@ impl SteamGuardAccount { return SteamGuardAccount { account_name: String::from(""), serial_number: String::from(""), - revocation_code: String::from(""), + revocation_code: String::from("").into(), shared_secret: TwoFactorSecret::new(), token_gid: String::from(""), - identity_secret: String::from(""), + identity_secret: String::from("").into(), server_time: 0, - uri: String::from(""), + uri: String::from("").into(), fully_enrolled: false, device_id: String::from(""), - secret_1: "".into(), + secret_1: String::from("").into(), session: Option::None, }; } @@ -100,7 +107,7 @@ impl SteamGuardAccount { params.insert("a", session.steam_id.to_string()); params.insert( "k", - generate_confirmation_hash_for_time(time, tag, &self.identity_secret), + generate_confirmation_hash_for_time(time, tag, &self.identity_secret.expose_secret()), ); params.insert("t", time.to_string()); params.insert("m", String::from("android")); @@ -226,12 +233,12 @@ impl SteamGuardAccount { /// Returns whether or not the operation was successful. pub fn remove_authenticator(&self, revocation_code: Option) -> anyhow::Result { ensure!( - matches!(revocation_code, Some(_)) || !self.revocation_code.is_empty(), + matches!(revocation_code, Some(_)) || !self.revocation_code.expose_secret().is_empty(), "Revocation code not provided." ); let client: SteamApiClient = SteamApiClient::new(self.session.clone()); let resp = - client.remove_authenticator(revocation_code.unwrap_or(self.revocation_code.clone()))?; + client.remove_authenticator(revocation_code.unwrap_or(self.revocation_code.expose_secret().to_owned()))?; Ok(resp.success) } } diff --git a/steamguard/src/secret_string.rs b/steamguard/src/secret_string.rs new file mode 100644 index 0000000..b409f8f --- /dev/null +++ b/steamguard/src/secret_string.rs @@ -0,0 +1,47 @@ +use serde::{Deserialize, Serialize, Serializer, Deserializer}; +use secrecy::{SecretString, ExposeSecret}; + +/// Helper to allow serializing a [secrecy::SecretString] as a [String] +pub(crate) fn serialize(secret_string: &SecretString, serializer: S) -> Result where S: Serializer { + serializer.serialize_str(secret_string.expose_secret()) +} + +/// Helper to allow deserializing a [String] as a [secrecy::SecretString] +pub(crate) fn deserialize<'de, D>(d: D) -> Result where D: Deserializer<'de> { + let s = String::deserialize(d)?; + Ok(SecretString::new(s)) +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_secret_string_round_trip() { + #[derive(Serialize, Deserialize)] + struct Foo { + #[serde(with = "super")] + secret: SecretString, + } + + let foo = Foo { + secret: String::from("hello").into(), + }; + + let s = serde_json::to_string(&foo).unwrap(); + let foo2: Foo = serde_json::from_str(&s).unwrap(); + assert_eq!(foo.secret.expose_secret(), foo2.secret.expose_secret()); + } + + #[test] + fn test_secret_string_deserialize() { + #[derive(Serialize, Deserialize)] + struct Foo { + #[serde(with = "super")] + secret: SecretString, + } + + let foo: Foo = serde_json::from_str("{\"secret\": \"hello\"}").unwrap(); + assert_eq!(foo.secret.expose_secret(), "hello"); + } +} diff --git a/steamguard/src/steamapi.rs b/steamguard/src/steamapi.rs index d57d961..af98e19 100644 --- a/steamguard/src/steamapi.rs +++ b/steamguard/src/steamapi.rs @@ -612,13 +612,13 @@ impl AddAuthenticatorResponse { SteamGuardAccount { shared_secret: TwoFactorSecret::parse_shared_secret(self.shared_secret).unwrap(), serial_number: self.serial_number.clone(), - revocation_code: self.revocation_code.clone(), - uri: self.uri.clone(), + revocation_code: self.revocation_code.into(), + uri: self.uri.into(), server_time: self.server_time, account_name: self.account_name.clone(), token_gid: self.token_gid.clone(), - identity_secret: self.identity_secret.clone(), - secret_1: self.secret_1.clone(), + identity_secret: self.identity_secret.into(), + secret_1: self.secret_1.into(), fully_enrolled: false, device_id: "".into(), session: None,