Improve string encryption

src/BizHawk.Client.EmuHawk/RetroAchievements/RCheevos.Login.cs

@@ -59,7 +59,7 @@ namespace BizHawk.Client.EmuHawk
 		{
 			var config = _getConfig();
 			Username = config.RAUsername;
-			ApiToken = SensitiveStrings.DecryptString(config.RAToken);
+			ApiToken = SecretStrings.DecryptString(config.RAToken);
 
 			if (LoggedIn)
 			{
@@ -67,7 +67,7 @@ namespace BizHawk.Client.EmuHawk
 				if (DoLogin(Username, apiToken: ApiToken))
 				{
 					config.RAUsername = Username;
-					config.RAToken = SensitiveStrings.EncryptString(ApiToken);
+					config.RAToken = SecretStrings.EncryptString(ApiToken);
 					PlaySound(_loginSound);
 					return;
 				}
@@ -77,7 +77,7 @@ namespace BizHawk.Client.EmuHawk
 			loginForm.ShowDialog();
 
 			config.RAUsername = Username;
-			config.RAToken = SensitiveStrings.EncryptString(ApiToken);
+			config.RAToken = SecretStrings.EncryptString(ApiToken);
 
 			if (LoggedIn)
 			{

src/BizHawk.Common/BizHawk.Common.csproj

@@ -8,6 +8,7 @@
 	</PropertyGroup>
 	<ItemGroup>
 		<PackageReference Include="Microsoft.Bcl.HashCode" />
+		<PackageReference Include="Microsoft.Win32.Registry" />
 		<PackageReference Include="PolySharp" />
 		<PackageReference Include="System.ComponentModel.Annotations" />
 		<PackageReference Include="System.Memory" />

src/BizHawk.Common/SecretStrings.cs

@@ -0,0 +1,100 @@
+using System.IO;
+using System.Security.Cryptography;
+using System.Text;
+
+using Microsoft.Win32;
+
+namespace BizHawk.Common
+{
+	/// <summary>
+	/// Goals
+	/// 1. Allows for encrypting/decrypting strings, suitable for any secret info (e.g. api tokens) to be placed in a config file
+	/// 2. Encryption is unique to a particular machine, so another person with the string can't decrypt it
+	/// 3. Uses very unique GUIDs to create an encryption key, along with Environment.MachineName (if as at least a fallback)
+	/// 4. Generally, this should be sufficient to protect any string, assuming the attack does not have access to the target machine (no way to protect the strings at that point)
+	/// 5. In the case Environment.MachineName is the only thing available, the protection is low for a skilled attack, but sufficient for low grade attacks.
+	/// (Probably should just keep these strings outside of the main config file, but still protect them as to prevent secrets being present as plaintext)
+	/// </summary>
+	public static class SecretStrings
+	{
+		private static readonly Aes _aes;
+
+		/// <summary>
+		/// Obtains a machine GUID
+		/// This is unique for a given machine and does not change based on the network config or hardware changes
+		/// It should be considered a secret, something never intended to be exposed in the network
+		/// (Note: Implementation matches Chromium's BrowserDMTokenStorage::Delegate::InitClientId)
+		/// </summary>
+		/// <returns>machine guid</returns>
+		private static string GetMachineGuid()
+		{
+			try
+			{
+				if (OSTailoredCode.IsUnixHost)
+				{
+					// see https://www.freedesktop.org/software/systemd/man/latest/machine-id.html
+					return File.ReadAllText("/etc/machine-id");
+				}
+
+				// windows can use HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Cryptography\MachineGuid
+				using var subKey = RegistryKey
+					.OpenBaseKey(RegistryHive.LocalMachine, RegistryView.Registry64)
+					.OpenSubKey(@"SOFTWARE\Microsoft\Cryptography");
+				return subKey!.GetValue("MachineGuid")!.ToString();
+			}
+			catch
+			{
+				// bad case mentioned in 5.
+				// hopefully never happens in practice
+				return string.Empty;
+			}
+		}
+
+		static SecretStrings()
+		{
+			_aes = Aes.Create();
+			_aes.Key = SHA256Checksum.Compute(Encoding.UTF8.GetBytes($"{GetMachineGuid()}/{Environment.MachineName}"));
+			_aes.IV = new byte[_aes.BlockSize / 8];
+			_aes.Mode = CipherMode.CBC;
+			_aes.Padding = PaddingMode.PKCS7;
+		}
+
+		public static string EncryptString(string secretString)
+		{
+			var bytes = Encoding.UTF8.GetBytes(secretString);
+			using var ms = new MemoryStream();
+			using var encryptor = _aes.CreateEncryptor();
+			using (var cryptoStream = new CryptoStream(ms, encryptor, CryptoStreamMode.Write))
+			{
+				cryptoStream.Write(bytes, 0, bytes.Length);
+			}
+
+			return Convert.ToBase64String(ms.ToArray());
+		}
+
+		public static string DecryptString(string encryptedString)
+		{
+			try
+			{
+				if (string.IsNullOrEmpty(encryptedString))
+				{
+					return string.Empty;
+				}
+
+				var bytes = Convert.FromBase64String(encryptedString);
+				using var ms = new MemoryStream();
+				using var decryptor = _aes.CreateDecryptor();
+				using (var cryptoStream = new CryptoStream(ms, decryptor, CryptoStreamMode.Write))
+				{
+					cryptoStream.Write(bytes, 0, bytes.Length);
+				}
+
+				return Encoding.UTF8.GetString(ms.ToArray());
+			}
+			catch
+			{
+				return string.Empty;
+			}
+		}
+	}
+}

src/BizHawk.Common/SensitiveStrings.cs

@@ -1,68 +0,0 @@
-using System.IO;
-using System.Security.Cryptography;
-using System.Text;
-
-namespace BizHawk.Common
-{
-	/// <summary>
-	/// Goals
-	/// 1. Allows for encrypting/decrypting strings, suitable for any sensitive info (e.g. api tokens) to be placed in a config file
-	/// 2. Encryption is unique to a particular machine, so another person with the string can't decrypt it
-	/// 3. ASSUMES ANY ATTACK HAS NO KNOWLEDGE OF THE MACHINE NAME
-	/// 4. As such, DOES NOT protect against attacks which have access to said machine or within the same LAN
-	/// (This is mainly to mitigate sensitive info leaks in the case the user posted the config publicly, unredacted)
-	/// (For anyone who's generally security minded, don't trust this to keep your tokens safe)
-	/// </summary>
-	public static class SensitiveStrings
-	{
-		private static readonly Aes _aes;
-
-		static SensitiveStrings()
-		{
-			_aes = Aes.Create();
-			// lame, but this seems to be the best method to do this...
-			_aes.Key = SHA256Checksum.Compute(Encoding.UTF8.GetBytes(Environment.MachineName));
-			_aes.IV = new byte[_aes.BlockSize / 8];
-			_aes.Mode = CipherMode.CBC;
-			_aes.Padding = PaddingMode.PKCS7;
-		}
-
-		public static string EncryptString(string sensitiveString)
-		{
-			var bytes = Encoding.UTF8.GetBytes(sensitiveString);
-			using var ms = new MemoryStream();
-			using var encryptor = _aes.CreateEncryptor();
-			using (var cryptoStream = new CryptoStream(ms, encryptor, CryptoStreamMode.Write))
-			{
-				cryptoStream.Write(bytes, 0, bytes.Length);
-			}
-
-			return Convert.ToBase64String(ms.ToArray());
-		}
-
-		public static string DecryptString(string encryptedString)
-		{
-			try
-			{
-				if (string.IsNullOrEmpty(encryptedString))
-				{
-					return string.Empty;
-				}
-
-				var bytes = Convert.FromBase64String(encryptedString);
-				using var ms = new MemoryStream();
-				using var decryptor = _aes.CreateDecryptor();
-				using (var cryptoStream = new CryptoStream(ms, decryptor, CryptoStreamMode.Write))
-				{
-					cryptoStream.Write(bytes, 0, bytes.Length);
-				}
-
-				return Encoding.UTF8.GetString(ms.ToArray());
-			}
-			catch
-			{
-				return string.Empty;
-			}
-		}
-	}
-}