Merge pull request #154 from dyc3/more-memory-protection

Mark all sensitive fields in SteamGuardAccount as secret

Cargo.lock

@@ -1857,6 +1857,7 @@ dependencies = [
  "standback",
  "thiserror",
  "uuid",
+ "zeroize",
 ]
 
 [[package]]

src/accountmanager.rs

@@ -7,7 +7,7 @@ use std::fs::File;
 use std::io::{BufReader, Read, Write};
 use std::path::Path;
 use std::sync::{Arc, Mutex};
-use steamguard::SteamGuardAccount;
+use steamguard::{ExposeSecret, SteamGuardAccount};
 use thiserror::Error;
 
 #[derive(Debug, Serialize, Deserialize)]
@@ -169,7 +169,10 @@ impl Manifest {
 
 	pub fn add_account(&mut self, account: SteamGuardAccount) {
 		debug!("adding account to manifest: {}", account.account_name);
-		let steamid = account.session.as_ref().map_or(0, |s| s.steam_id);
+		let steamid = account
+			.session
+			.as_ref()
+			.map_or(0, |s| s.expose_secret().steam_id);
 		self.entries.push(ManifestEntry {
 			filename: format!("{}.maFile", &account.account_name),
 			steam_id: steamid,
@@ -372,6 +375,7 @@ impl From<std::io::Error> for ManifestAccountLoadError {
 #[cfg(test)]
 mod tests {
 	use super::*;
+	use steamguard::ExposeSecret;
 	use tempdir::TempDir;
 
 	#[test]
@@ -390,7 +394,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 +423,8 @@ mod tests {
 				.get_account(&account_name)?
 				.lock()
 				.unwrap()
-				.revocation_code,
+				.revocation_code
+				.expose_secret(),
 			"R12345"
 		);
 		assert_eq!(
@@ -443,7 +448,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 +484,8 @@ mod tests {
 				.get_account(&account_name)?
 				.lock()
 				.unwrap()
-				.revocation_code,
+				.revocation_code
+				.expose_secret(),
 			"R12345"
 		);
 		assert_eq!(
@@ -504,12 +510,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 +545,8 @@ mod tests {
 				.get_account(&account_name)?
 				.lock()
 				.unwrap()
-				.revocation_code,
+				.revocation_code
+				.expose_secret(),
 			"R12345"
 		);
 		assert_eq!(
@@ -564,7 +571,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 +610,8 @@ mod tests {
 				.get_account(&account_name)?
 				.lock()
 				.unwrap()
-				.revocation_code,
+				.revocation_code
+				.expose_secret(),
 			"R12345"
 		);
 		assert_eq!(
@@ -674,7 +682,14 @@ mod tests {
 		let account = manifest.get_account(&account_name)?;
 		assert_eq!(account_name, account.lock().unwrap().account_name);
 		assert_eq!(
-			account.lock().unwrap().session.as_ref().unwrap().web_cookie,
+			account
+				.lock()
+				.unwrap()
+				.session
+				.as_ref()
+				.unwrap()
+				.expose_secret()
+				.web_cookie,
 			None
 		);
 		Ok(())
@@ -690,18 +705,38 @@ 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().session.as_ref().unwrap().steam_id,
+			account.lock().unwrap().revocation_code.expose_secret(),
+			"R12345"
+		);
+		assert_eq!(
+			account
+				.lock()
+				.unwrap()
+				.session
+				.as_ref()
+				.unwrap()
+				.expose_secret()
+				.steam_id,
 			1234
 		);
 
 		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().session.as_ref().unwrap().steam_id,
+			account.lock().unwrap().revocation_code.expose_secret(),
+			"R56789"
+		);
+		assert_eq!(
+			account
+				.lock()
+				.unwrap()
+				.session
+				.as_ref()
+				.unwrap()
+				.expose_secret()
+				.steam_id,
 			5678
 		);
 		Ok(())

src/main.rs

@@ -7,8 +7,8 @@ use std::{
 	sync::{Arc, Mutex},
 };
 use steamguard::{
-	steamapi, AccountLinkError, AccountLinker, Confirmation, FinalizeLinkError, LoginError,
+	steamapi, AccountLinkError, AccountLinker, Confirmation, ExposeSecret, FinalizeLinkError,
-	SteamGuardAccount, UserLogin,
+	LoginError, SteamGuardAccount, UserLogin,
 };
 
 use crate::accountmanager::ManifestAccountLoadError;
@@ -227,7 +227,7 @@ fn do_login(account: &mut SteamGuardAccount) -> anyhow::Result<()> {
 	} else {
 		debug!("password is empty");
 	}
-	account.session = Some(do_login_impl(
+	account.set_session(do_login_impl(
 		account.account_name.clone(),
 		password,
 		Some(account),
@@ -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(());

steamguard/Cargo.toml

@@ -29,3 +29,4 @@ scraper = "0.12.0"
 maplit = "1.0.2"
 thiserror = "1.0.26"
 secrecy = { version = "0.8", features = ["serde"] }
+zeroize = "^1.4.3"

steamguard/src/accountlinker.rs

@@ -27,7 +27,7 @@ impl AccountLinker {
 			finalized: false,
 			sent_confirmation_email: false,
 			session: session.clone(),
-			client: SteamApiClient::new(Some(session)),
+			client: SteamApiClient::new(Some(secrecy::Secret::new(session))),
 		};
 	}
 

steamguard/src/lib.rs

@@ -11,6 +11,7 @@ use reqwest::{
 	Url,
 };
 use scraper::{Html, Selector};
+pub use secrecy::{ExposeSecret, SecretString};
 use serde::{Deserialize, Serialize};
 use std::{collections::HashMap, convert::TryInto};
 use steamapi::SteamApiClient;
@@ -24,6 +25,7 @@ extern crate maplit;
 
 mod accountlinker;
 mod confirmation;
+mod secret_string;
 pub mod steamapi;
 pub mod token;
 mod userlogin;
@@ -43,17 +45,21 @@ extern crate hmacsha1;
 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<steamapi::Session>,
+	pub session: Option<secrecy::Secret<steamapi::Session>>,
 }
 
 fn build_time_bytes(time: i64) -> [u8; 8] {
@@ -75,32 +81,36 @@ 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,
 		};
 	}
 
+	pub fn set_session(&mut self, session: steamapi::Session) {
+		self.session = Some(session.into());
+	}
+
 	pub fn generate_code(&self, time: i64) -> String {
 		return self.shared_secret.generate_code(time);
 	}
 
 	fn get_confirmation_query_params(&self, tag: &str) -> HashMap<&str, String> {
-		let session = self.session.clone().unwrap();
+		let session = self.session.as_ref().unwrap().expose_secret();
 		let time = steamapi::get_server_time();
 		let mut params = HashMap::new();
 		params.insert("p", self.device_id.clone());
 		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"));
@@ -111,13 +121,12 @@ impl SteamGuardAccount {
 	fn build_cookie_jar(&self) -> reqwest::cookie::Jar {
 		let url = "https://steamcommunity.com".parse::<Url>().unwrap();
 		let cookies = reqwest::cookie::Jar::default();
-		let session = self.session.clone().unwrap();
+		let session = self.session.as_ref().unwrap().expose_secret();
-		let session_id = session.session_id;
 		cookies.add_cookie_str("mobileClientVersion=0 (2.1.3)", &url);
 		cookies.add_cookie_str("mobileClient=android", &url);
 		cookies.add_cookie_str("Steam_Language=english", &url);
 		cookies.add_cookie_str("dob=", &url);
-		cookies.add_cookie_str(format!("sessionid={}", session_id).as_str(), &url);
+		cookies.add_cookie_str(format!("sessionid={}", session.session_id).as_str(), &url);
 		cookies.add_cookie_str(format!("steamid={}", session.steam_id).as_str(), &url);
 		cookies.add_cookie_str(format!("steamLogin={}", session.steam_login).as_str(), &url);
 		cookies.add_cookie_str(
@@ -226,12 +235,13 @@ impl SteamGuardAccount {
 	/// Returns whether or not the operation was successful.
 	pub fn remove_authenticator(&self, revocation_code: Option<String>) -> anyhow::Result<bool> {
 		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 =
+		let resp = client.remove_authenticator(
-			client.remove_authenticator(revocation_code.unwrap_or(self.revocation_code.clone()))?;
+			revocation_code.unwrap_or(self.revocation_code.expose_secret().to_owned()),
+		)?;
 		Ok(resp.success)
 	}
 }

steamguard/src/secret_string.rs

@@ -0,0 +1,53 @@
+use secrecy::{ExposeSecret, SecretString};
+use serde::{Deserialize, Deserializer, Serialize, Serializer};
+
+/// Helper to allow serializing a [secrecy::SecretString] as a [String]
+pub(crate) fn serialize<S>(secret_string: &SecretString, serializer: S) -> Result<S::Ok, S::Error>
+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<secrecy::SecretString, D::Error>
+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");
+	}
+}

steamguard/src/steamapi.rs

@@ -8,11 +8,13 @@ use reqwest::{
 	header::{HeaderMap, HeaderName, HeaderValue, SET_COOKIE},
 	Url,
 };
+use secrecy::{CloneableSecret, DebugSecret, ExposeSecret, SerializableSecret};
 use serde::{Deserialize, Deserializer, Serialize};
 use serde_json::Value;
 use std::iter::FromIterator;
 use std::str::FromStr;
 use std::time::{SystemTime, UNIX_EPOCH};
+use zeroize::Zeroize;
 
 lazy_static! {
 	static ref STEAM_COOKIE_URL: Url = "https://steamcommunity.com".parse::<Url>().unwrap();
@@ -90,7 +92,8 @@ pub struct OAuthData {
 	webcookie: String,
 }
 
-#[derive(Debug, Clone, Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize, Zeroize)]
+#[zeroize(drop)]
 pub struct Session {
 	#[serde(rename = "SessionID")]
 	pub session_id: String,
@@ -106,6 +109,10 @@ pub struct Session {
 	pub steam_id: u64,
 }
 
+impl SerializableSecret for Session {}
+impl CloneableSecret for Session {}
+impl DebugSecret for Session {}
+
 pub fn get_server_time() -> i64 {
 	let client = reqwest::blocking::Client::new();
 	let resp = client
@@ -124,11 +131,11 @@ pub fn get_server_time() -> i64 {
 pub struct SteamApiClient {
 	cookies: reqwest::cookie::Jar,
 	client: reqwest::blocking::Client,
-	pub session: Option<Session>,
+	pub session: Option<secrecy::Secret<Session>>,
 }
 
 impl SteamApiClient {
-	pub fn new(session: Option<Session>) -> SteamApiClient {
+	pub fn new(session: Option<secrecy::Secret<Session>>) -> SteamApiClient {
 		SteamApiClient {
 			cookies: reqwest::cookie::Jar::default(),
 			client: reqwest::blocking::ClientBuilder::new()
@@ -195,7 +202,7 @@ impl SteamApiClient {
 			.add_cookie_str("Steam_Language=english", &STEAM_COOKIE_URL);
 		if let Some(session) = &self.session {
 			self.cookies.add_cookie_str(
-				format!("sessionid={}", session.session_id).as_str(),
+				format!("sessionid={}", session.expose_secret().session_id).as_str(),
 				&STEAM_COOKIE_URL,
 			);
 		}
@@ -270,7 +277,7 @@ impl SteamApiClient {
 		let login_resp: LoginResponse = serde_json::from_str(text.as_str())?;
 
 		if let Some(oauth) = &login_resp.oauth {
-			self.session = Some(self.build_session(&oauth));
+			self.session = Some(secrecy::Secret::new(self.build_session(&oauth)));
 		}
 
 		return Ok(login_resp);
@@ -295,7 +302,7 @@ impl SteamApiClient {
 					wgtoken_secure: params.token_secure,
 					webcookie: params.webcookie,
 				};
-				self.session = Some(self.build_session(&oauth));
+				self.session = Some(secrecy::Secret::new(self.build_session(&oauth)));
 				return Ok(oauth);
 			}
 			(None, None) => {
@@ -319,7 +326,7 @@ impl SteamApiClient {
 		let mut params = hashmap! {
 			"op" => op,
 			"arg" => arg,
-			"sessionid" => self.session.as_ref().unwrap().session_id.as_str(),
+			"sessionid" => self.session.as_ref().unwrap().expose_secret().session_id.as_str(),
 		};
 		if op == "check_sms_code" {
 			params.insert("checkfortos", "0");
@@ -365,7 +372,7 @@ impl SteamApiClient {
 		let params = hashmap! {
 			"op" => op,
 			"input" => input,
-			"sessionid" => self.session.as_ref().unwrap().session_id.as_str(),
+			"sessionid" => self.session.as_ref().unwrap().expose_secret().session_id.as_str(),
 		};
 
 		let resp = self
@@ -421,8 +428,8 @@ impl SteamApiClient {
 	) -> anyhow::Result<AddAuthenticatorResponse> {
 		ensure!(matches!(self.session, Some(_)));
 		let params = hashmap! {
-			"access_token" => self.session.as_ref().unwrap().token.clone(),
+			"access_token" => self.session.as_ref().unwrap().expose_secret().token.clone(),
-			"steamid" => self.session.as_ref().unwrap().steam_id.to_string(),
+			"steamid" => self.session.as_ref().unwrap().expose_secret().steam_id.to_string(),
 			"authenticator_type" => "1".into(),
 			"device_identifier" => device_id,
 			"sms_phone_id" => "1".into(),
@@ -454,8 +461,8 @@ impl SteamApiClient {
 	) -> anyhow::Result<FinalizeAddAuthenticatorResponse> {
 		ensure!(matches!(self.session, Some(_)));
 		let params = hashmap! {
-			"steamid" => self.session.as_ref().unwrap().steam_id.to_string(),
+			"steamid" => self.session.as_ref().unwrap().expose_secret().steam_id.to_string(),
-			"access_token" => self.session.as_ref().unwrap().token.clone(),
+			"access_token" => self.session.as_ref().unwrap().expose_secret().token.clone(),
 			"activation_code" => sms_code,
 			"authenticator_code" => code_2fa,
 			"authenticator_time" => time_2fa.to_string(),
@@ -485,10 +492,10 @@ impl SteamApiClient {
 		revocation_code: String,
 	) -> anyhow::Result<RemoveAuthenticatorResponse> {
 		let params = hashmap! {
-			"steamid" => self.session.as_ref().unwrap().steam_id.to_string(),
+			"steamid" => self.session.as_ref().unwrap().expose_secret().steam_id.to_string(),
 			"steamguard_scheme" => "2".into(),
 			"revocation_code" => revocation_code,
-			"access_token" => self.session.as_ref().unwrap().token.to_string(),
+			"access_token" => self.session.as_ref().unwrap().expose_secret().token.to_string(),
 		};
 
 		let resp = self
@@ -612,13 +619,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(),
+			revocation_code: self.revocation_code.into(),
-			uri: self.uri.clone(),
+			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(),
+			identity_secret: self.identity_secret.into(),
-			secret_1: self.secret_1.clone(),
+			secret_1: self.secret_1.into(),
 			fully_enrolled: false,
 			device_id: "".into(),
 			session: None,

steamguard/src/userlogin.rs

@@ -1,6 +1,7 @@
 use crate::steamapi::{LoginResponse, RsaResponse, Session, SteamApiClient};
 use log::*;
 use rsa::{PublicKey, RsaPublicKey};
+use secrecy::ExposeSecret;
 use std::time::{SystemTime, UNIX_EPOCH};
 
 #[derive(Debug)]
@@ -154,7 +155,13 @@ impl UserLogin {
 			self.client.transfer_login(login_resp)?;
 		}
 
-		return Ok(self.client.session.clone().unwrap());
+		return Ok(self
+			.client
+			.session
+			.as_ref()
+			.unwrap()
+			.expose_secret()
+			.to_owned());
 	}
 }