KeyVaultSecretsRepository comparisons are now case insensitive (#9644)

* Ignore case on KeyVaultSecrets repository lookups and comparisons

* Adding test to check Secret Provider reads Host Secrets properly from KeyVault

* Testing WriteAsync to ensure we're not deleting secrets that should be kept

* Adding tests for Reading and Writing Function Secrets

* Adding log messages for set and delete keys.

* Added release notes

Author: cjaliaga

release_notes.md

@@ -25,4 +25,6 @@
 - Update Node.js Worker Version to [3.9.0](https://github.com/Azure/azure-functions-nodejs-worker/releases/tag/v3.9.0)
 - Upgrading dependent packages to latest versions. #9646
    - Azure.Identity (1.10.0 to 1.10.3)
-   - Azure.Core (1.34.0 to 1.35.0)
+   - Azure.Core (1.34.0 to 1.35.0)
+- Bug fix: Comparisons in the Azure Key Vault Secrets Repository are now case insensitive (#9644)
+  - This fixes a bug where keys could be automatically recreated if they had been manually added to Key Vault with all lowercase secret names

src/WebJobs.Script.WebHost/Diagnostics/Extensions/ScriptHostServiceSecurityExtension.cs

@@ -15,9 +15,31 @@ public static class ScriptHostServiceSecurityExtension
                 new EventId(600, nameof(BlobStorageSecretRepoError)),
                 "There was an error performing a {operation} operation on the Blob Storage Secret Repository.");
 
+        private static readonly Action<ILogger, string, Exception> _keyVaultSecretRepoSetKey =
+            LoggerMessage.Define<string>(
+                LogLevel.Debug,
+                new EventId(601, nameof(KeyVaultSecretRepoSetKey)),
+                "Setting key '{key}' on the KeyVault Secret Repository.");
+
+        private static readonly Action<ILogger, string, Exception> _keyVaultSecretRepoDeleteKey =
+            LoggerMessage.Define<string>(
+                LogLevel.Debug,
+                new EventId(602, nameof(KeyVaultSecretRepoDeleteKey)),
+                "Deleting key '{key}' on the KeyVault Secret Repository.");
+
         public static void BlobStorageSecretRepoError(this ILogger logger, string operation, Exception exception)
         {
             _blobStorageSecretRepoError(logger, operation, exception);
         }
+
+        public static void KeyVaultSecretRepoSetKey(this ILogger logger, string key)
+        {
+            _keyVaultSecretRepoSetKey(logger, key, null);
+        }
+
+        public static void KeyVaultSecretRepoDeleteKey(this ILogger logger, string key)
+        {
+            _keyVaultSecretRepoDeleteKey(logger, key, null);
+        }
     }
 }

src/WebJobs.Script.WebHost/Security/KeyManagement/KeyVaultSecretsRepository.cs

@@ -9,6 +9,7 @@
 using Azure.Core;
 using Azure.Identity;
 using Azure.Security.KeyVault.Secrets;
+using Microsoft.Azure.WebJobs.Script.WebHost.Diagnostics.Extensions;
 using Microsoft.Extensions.Logging;
 
 namespace Microsoft.Azure.WebJobs.Script.WebHost
@@ -50,6 +51,12 @@ public KeyVaultSecretsRepository(string secretsSentinelFilePath, string vaultUri
             _environment = environment ?? throw new ArgumentNullException(nameof(environment));
         }
 
+        // For testing
+        internal KeyVaultSecretsRepository(SecretClient secretClient, string secretsSentinelFilePath, ILogger logger, IEnvironment environment) : base(secretsSentinelFilePath, logger, environment)
+        {
+            _secretClient = new Lazy<SecretClient>(() => secretClient);
+        }
+
         public override bool IsEncryptionSupported
         {
             get
@@ -75,11 +82,12 @@ public override async Task WriteAsync(ScriptSecretsType type, string functionNam
             List<Task> deleteTasks = new List<Task>();
             string prefix = (type == ScriptSecretsType.Host) ? HostPrefix : FunctionPrefix + Normalize(functionName);
 
-            foreach (SecretProperties item in await FindSecrets(secretsPages, x => x.Name.StartsWith(prefix)))
+            foreach (SecretProperties item in await FindSecrets(secretsPages, x => x.Name.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)))
             {
                 // Delete only keys which no longer exist in passed-in secrets
                 if (!dictionary.Keys.Contains(item.Name))
                 {
+                    Logger?.KeyVaultSecretRepoDeleteKey(item.Name);
                     deleteTasks.Add(_secretClient.Value.StartDeleteSecretAsync(item.Name));
                 }
             }
@@ -93,6 +101,7 @@ public override async Task WriteAsync(ScriptSecretsType type, string functionNam
             List<Task> setTasks = new List<Task>();
             foreach (string key in dictionary.Keys)
             {
+                Logger?.KeyVaultSecretRepoSetKey(key);
                 setTasks.Add(_secretClient.Value.SetSecretAsync(key, dictionary[key]));
             }
             await Task.WhenAll(setTasks);
@@ -125,7 +134,7 @@ private async Task<ScriptSecrets> ReadHostSecrets()
             List<Task<Response<KeyVaultSecret>>> tasks = new List<Task<Response<KeyVaultSecret>>>();
 
             // Add master key task
-            List<SecretProperties> masterItems = await FindSecrets(secretsPages, x => x.Name.StartsWith(MasterKey));
+            List<SecretProperties> masterItems = await FindSecrets(secretsPages, x => x.Name.StartsWith(MasterKey, StringComparison.OrdinalIgnoreCase));
             if (masterItems.Count > 0)
             {
                 tasks.Add(_secretClient.Value.GetSecretAsync(masterItems[0].Name));
@@ -136,13 +145,13 @@ private async Task<ScriptSecrets> ReadHostSecrets()
             }
 
             // Add functionKey tasks
-            foreach (SecretProperties item in await FindSecrets(secretsPages, x => x.Name.StartsWith(FunctionKeyPrefix)))
+            foreach (SecretProperties item in await FindSecrets(secretsPages, x => x.Name.StartsWith(FunctionKeyPrefix, StringComparison.OrdinalIgnoreCase)))
             {
                 tasks.Add(_secretClient.Value.GetSecretAsync(item.Name));
             }
 
             // Add systemKey tasks
-            foreach (SecretProperties item in await FindSecrets(secretsPages, x => x.Name.StartsWith(SystemKeyPrefix)))
+            foreach (SecretProperties item in await FindSecrets(secretsPages, x => x.Name.StartsWith(SystemKeyPrefix, StringComparison.OrdinalIgnoreCase)))
             {
                 tasks.Add(_secretClient.Value.GetSecretAsync(item.Name));
             }
@@ -158,15 +167,15 @@ private async Task<ScriptSecrets> ReadHostSecrets()
             foreach (Task<Response<KeyVaultSecret>> task in tasks)
             {
                 KeyVaultSecret item = task.Result;
-                if (item.Name.StartsWith(MasterKey))
+                if (item.Name.StartsWith(MasterKey, StringComparison.OrdinalIgnoreCase))
                 {
                     hostSecrets.MasterKey = KeyVaultSecretToKey(item, MasterKey);
                 }
-                else if (item.Name.StartsWith(FunctionKeyPrefix))
+                else if (item.Name.StartsWith(FunctionKeyPrefix, StringComparison.OrdinalIgnoreCase))
                 {
                     hostSecrets.FunctionKeys.Add(KeyVaultSecretToKey(item, FunctionKeyPrefix));
                 }
-                else if (item.Name.StartsWith(SystemKeyPrefix))
+                else if (item.Name.StartsWith(SystemKeyPrefix, StringComparison.OrdinalIgnoreCase))
                 {
                     hostSecrets.SystemKeys.Add(KeyVaultSecretToKey(item, SystemKeyPrefix));
                 }
@@ -234,7 +243,7 @@ public static async Task<List<SecretProperties>> FindSecrets(AsyncPageable<Secre
 
         public static Dictionary<string, string> GetDictionaryFromScriptSecrets(ScriptSecrets secrets, string functionName)
         {
-            Dictionary<string, string> dic = new Dictionary<string, string>();
+            Dictionary<string, string> dic = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
             HostSecrets hostSecrets = secrets as HostSecrets;
             FunctionSecrets functionSecrets = secrets as FunctionSecrets;
             if (hostSecrets != null)