Fix for Server Challenge Token Security Incident

src/StorageSync/StorageSync/ChangeLog.md

@@ -19,6 +19,8 @@
 -->
 ## Upcoming Release
 
+* Fixes security bug in token acquisition for MI server registration
+
 ## Version 2.5.0
 * Fixed the bug in server registration
 * Improved the error message for Set-AzStorageSyncServiceIdentity cmdlet

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('=', 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,41 @@ 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
+            string systemDrive = Environment.GetEnvironmentVariable("SystemDrive") ?? "C:";
+            string programData = Environment.GetEnvironmentVariable("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.resx

@@ -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>