Fix for Server Challenge Token Security Incident (#28177)

Co-authored-by: Copilot <[email protected]>

Author: stefong99

src/StorageSync/StorageSync/ChangeLog.md

@@ -18,6 +18,7 @@
         - Additional information about change #1
 -->
 ## Upcoming Release
+* Fixed security bug in token acquisition for MI server registration
 
 ## Version 2.5.0
 * Fixed the bug in server registration

src/StorageSync/StorageSync/Interop/ManagedIdentity/ServerManagedIdentityUtils.cs

@@ -272,9 +272,9 @@ public static async Task<string> GetChallengeToken(string requestUri, HttpClient
                             null);
                     }
 
-                    var headerValue = authenticateHeaderValues.FirstOrDefault();
+                    var wwwHeader = authenticateHeaderValues.FirstOrDefault();
 
-                    if (string.IsNullOrEmpty(headerValue) || !headerValue.Contains('='))
+                    if (string.IsNullOrEmpty(wwwHeader) || !wwwHeader.Contains('='))
                     {
                         throw new ServerManagedIdentityTokenException(
                             ManagedIdentityErrorCodes.ServerManagedIdentityTokenChallengeFailed,
@@ -283,31 +283,17 @@ public static async Task<string> GetChallengeToken(string requestUri, HttpClient
                     }
 
                     // Value in the header is: "Basic realm=<secret file path>"
-                    var secretFilePath = headerValue.Split('=')[1];
+                    var secretFilePath = wwwHeader.Split(new char[] { '=' }, StringSplitOptions.RemoveEmptyEntries).LastOrDefault();
 
-                    var expectedSecretFileLocation = Environment.GetEnvironmentVariable("ProgramData") + HybridSecretFileDirectory;
-
-                    // Validate the secret file path received is from the expected predefined directory and is of expected .key file extension.
-                    // This ensures we are not redirected by some malicious process listening on localhost:40342 into a bad secret file.
-                    if (string.IsNullOrEmpty(secretFilePath) ||
-                        !secretFilePath.Contains(expectedSecretFileLocation) ||
-                        !secretFilePath.Contains(".key"))
-                    {
-                        throw new ServerManagedIdentityTokenException(
-                            ManagedIdentityErrorCodes.ServerManagedIdentityTokenChallengeFailed,
-                            StorageSyncResources.AgentMI_InvalidSecretFileError,
-                            null);
-                    }
-
-                    if (File.Exists(secretFilePath))
+                    if (IsSecretFilePathValid(secretFilePath))
                     {
                         challengeToken = File.ReadAllText(secretFilePath);
                     }
                     else
                     {
                         throw new ServerManagedIdentityTokenException(
                             ManagedIdentityErrorCodes.ServerManagedIdentityTokenChallengeFailed,
-                            StorageSyncResources.AgentMI_MissingSecretFilePathOnServerError,
+                            StorageSyncResources.AgentMI_InvalidSecretFileError,
                             null);
                     }
                 }
@@ -349,6 +335,59 @@ public static async Task<string> GetChallengeToken(string requestUri, HttpClient
             return challengeToken;
         }
 
+        /// <summary>
+        /// Validate the secret file path received is from the expected predefined directory and is of expected .key file extension.
+        /// This ensures we are not redirected by some malicious process listening on localhost:40342 into a bad secret file.
+        /// </summary>
+        /// <param name="secretFilePath"></param>
+        /// <returns></returns>
+        private static bool IsSecretFilePathValid(string secretFilePath)
+        {
+            // Check if the secret file path is null or empty
+            if (string.IsNullOrEmpty(secretFilePath))
+            {
+                return false;
+            }
+
+            // Normalize the path to prevent path traversal attacks
+            string normalizedTokenLocation = Path.GetFullPath(secretFilePath);
+
+            string allowedFolder;
+
+            // Expected form: %ProgramData%\AzureConnectedMachineAgent\Tokens\<guid>.key
+            var programData = Environment.GetEnvironmentVariable("ProgramData");
+            
+            if (string.IsNullOrEmpty(programData))
+            {
+                // If ProgramData is not found, try to manually construct it using SystemDrive
+                var systemDrive = Environment.GetEnvironmentVariable("SystemDrive");
+
+                if (string.IsNullOrEmpty(systemDrive))
+                {
+                    throw new ServerManagedIdentityTokenException(
+                        ManagedIdentityErrorCodes.ServerManagedIdentityTokenChallengeFailed,
+                        StorageSyncResources.AgentMI_ProgramDataNotFoundError,
+                        null);
+                }
+                else
+                {
+                    programData = Path.Combine(systemDrive, "ProgramData");
+                }
+            }
+
+            allowedFolder = Path.GetFullPath(Path.Combine(programData, "AzureConnectedMachineAgent", "Tokens"));
+
+            // Ensure the secret file is within the allowed tokens folder, exists, and ends with .key
+            if (!normalizedTokenLocation.StartsWith(allowedFolder + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase) ||
+                !File.Exists(normalizedTokenLocation) ||
+                !Path.GetFileName(normalizedTokenLocation).EndsWith(".key", StringComparison.OrdinalIgnoreCase))
+            {
+                return false;
+            }
+
+            return true;
+        }
+
         /// <summary>
         /// Gets the Server Type from the the StorageSync registry path. Default to <see cref="LocalServerType.HybridServer"/>
         /// Not using ServerManagedIdentityProvider.GetServerType because it does not necessarily do a direct registry key read. 

src/StorageSync/StorageSync/Properties/StorageSyncResources.Designer.cs

@@ -294,9 +294,6 @@
   <data name="AgentMI_InvalidSecretFileError" xml:space="preserve">
     <value>Secret file is invalid. It is either empty, not in the expected directory, or not of file type .key.</value>
   </data>
-  <data name="AgentMI_MissingSecretFilePathOnServerError" xml:space="preserve">
-    <value>Secret file path does not exist on the server.</value>
-  </data>
   <data name="AgentMI_MissingSystemAssignedIdentityError" xml:space="preserve">
     <value>No system-assigned Managed Identity was found for this resource.</value>
   </data>
@@ -327,4 +324,7 @@
   <data name="AgentMI_ArcServerNotEnabled" xml:space="preserve">
     <value>&gt;This hybrid server is not Azure Arc-enabled. Please ensure that the Azure Connected Machine agent is installed and the server is connected to Azure Arc.</value>
   </data>
+  <data name="AgentMI_ProgramDataNotFoundError" xml:space="preserve">
+    <value>GetEnvironmentVariable failed to find 'ProgramData'</value>
+  </data>
 </root>