From 90b8178085dfa2e8059bf60fd4f032a7fd7a85ee Mon Sep 17 00:00:00 2001 From: Michael Peng Date: Mon, 21 Mar 2022 14:42:44 -0700 Subject: [PATCH] Make null and unsupported Key Vault reference formats non-blocking (#2991) * Make null and unsupported Key Vault references non-blocking * Changed variable names * Altered warning message * Corrected regular expressions and altered warning message * Remove redundant matching * Changed regular expression * Add unit tests and address comments * Fix unit test for KeyVaultReferenceManager * Added support for other key vault reference syntax * Added nested object to test case * Update test/Azure.Functions.Cli.Tests/KeyVaultReferencesManagerTests.cs Co-authored-by: Eric Jizba * Update test/Azure.Functions.Cli.Tests/KeyVaultReferencesManagerTests.cs Co-authored-by: Eric Jizba * Update src/Azure.Functions.Cli/Common/KeyVaultReferencesManager.cs Co-authored-by: Eric Jizba * Update src/Azure.Functions.Cli/Common/KeyVaultReferencesManager.cs Co-authored-by: Eric Jizba Co-authored-by: Eric Jizba --- .../Common/KeyVaultReferencesManager.cs | 99 ++++++++++++----- .../KeyVaultReferencesManagerTests.cs | 105 ++++++++++++++++++ 2 files changed, 177 insertions(+), 27 deletions(-) create mode 100644 test/Azure.Functions.Cli.Tests/KeyVaultReferencesManagerTests.cs diff --git a/src/Azure.Functions.Cli/Common/KeyVaultReferencesManager.cs b/src/Azure.Functions.Cli/Common/KeyVaultReferencesManager.cs index bfb637cd1..3275f1880 100644 --- a/src/Azure.Functions.Cli/Common/KeyVaultReferencesManager.cs +++ b/src/Azure.Functions.Cli/Common/KeyVaultReferencesManager.cs @@ -3,17 +3,18 @@ using System.Collections.Generic; using System.Linq; using System.Text.RegularExpressions; +using Colors.Net; using Azure.Core; using Azure.Identity; using Azure.Security.KeyVault.Secrets; +using static Azure.Functions.Cli.Common.OutputTheme; namespace Azure.Functions.Cli.Common { class KeyVaultReferencesManager { private const string vaultUriSuffix = "vault.azure.net"; - private static readonly Regex PrimaryKeyVaultReferenceRegex = new Regex(@"^@Microsoft.KeyVault\(SecretUri=(?[\S^/]+)/(?[\S^/]+)/(?[\S^/]+)/(?[\S^/]+)\)$", RegexOptions.Compiled); - private static readonly Regex SecondaryKeyVaultReferenceRegex = new Regex(@"^@Microsoft.KeyVault\(VaultName=(?[\S^;]+);SecretName=(?[\S^;]+)\)$", RegexOptions.Compiled); + private static readonly Regex BasicKeyVaultReferenceRegex = new Regex(@"^@Microsoft\.KeyVault\((?.*)\)$", RegexOptions.Compiled); private readonly ConcurrentDictionary clients = new ConcurrentDictionary(); private readonly TokenCredential credential = new DefaultAzureCredential(); @@ -21,17 +22,26 @@ public void ResolveKeyVaultReferences(IDictionary settings) { foreach (var key in settings.Keys.ToList()) { - var keyVaultValue = GetSecretValue(settings[key]); - if (keyVaultValue != null) + try { - settings[key] = keyVaultValue; + var keyVaultValue = GetSecretValue(key, settings[key]); + if (keyVaultValue != null) + { + settings[key] = keyVaultValue; + } + } + catch + { + // Do not block StartHostAction if secret cannot be resolved: instead, skip it + // and attempt to resolve other secrets + continue; } } } - private string GetSecretValue(string value) + private string GetSecretValue(string key, string value) { - var result = ParseSecret(value); + var result = ParseSecret(key, value); if (result != null) { @@ -43,43 +53,78 @@ private string GetSecretValue(string value) return null; } - private ParseSecretResult ParseSecret(string value) + internal ParseSecretResult ParseSecret(string key, string value) { - try + // If the value is null, then we return nothing, as the subsequent call to + // UpdateEnvironmentVariables(settings) will log to the user that the setting + // is skipped. We check here, because Regex.Match throws when supplied with a + // null value. + if (value == null) { - return ParsePrimaryRegexSecret(value) ?? ParseSecondaryRegexSecret(value); + return null; } - catch + // Determine if the secret value is attempting to use a key vault reference + var keyVaultReferenceMatch = BasicKeyVaultReferenceRegex.Match(value); + if (keyVaultReferenceMatch.Success) { - throw new FormatException($"Key Vault Reference format invalid: {value}"); + var referenceString = keyVaultReferenceMatch.Groups["ReferenceString"].Value; + ParseSecretResult result = null; + try + { + result = ParseVaultReference(referenceString); + } + catch + { + // ignore and show warning below + } + + // If we detect that a key vault reference was attempted, but did not match any of + // the supported formats, we write a warning to the console. + if (result == null) + { + ColoredConsole.WriteLine(WarningColor($"Unable to parse the Key Vault reference for setting: {key}")); + } + return result; } + return null; } - private ParseSecretResult ParsePrimaryRegexSecret(string value) + internal ParseSecretResult ParseVaultReference(string vaultReference) { - var match = PrimaryKeyVaultReferenceRegex.Match(value); - if (match.Success) + var secretUriString = GetValueFromVaultReference("SecretUri", vaultReference); + if (!string.IsNullOrEmpty(secretUriString)) + { + var secretUri = new Uri(secretUriString); + var secretIdentifier = new KeyVaultSecretIdentifier(secretUri); + return new ParseSecretResult + { + Uri = secretIdentifier.VaultUri, + Name = secretIdentifier.Name, + Version = secretIdentifier.Version + }; + } + var vaultName = GetValueFromVaultReference("VaultName", vaultReference); + var secretName = GetValueFromVaultReference("SecretName", vaultReference); + var version = GetValueFromVaultReference("SecretVersion", vaultReference); + if (!string.IsNullOrEmpty(vaultName) && !string.IsNullOrEmpty(secretName)) { return new ParseSecretResult { - Uri = new Uri(match.Groups["VaultUri"].Value), - Name = match.Groups["SecretName"].Value, - Version = match.Groups["Version"].Value + Uri = new Uri($"https://{vaultName}.{vaultUriSuffix}"), + Name = secretName, + Version = version }; } return null; } - private ParseSecretResult ParseSecondaryRegexSecret(string value) + internal string GetValueFromVaultReference(string key, string vaultReference) { - var altMatch = SecondaryKeyVaultReferenceRegex.Match(value); - if (altMatch.Success) + var regex = new Regex(key + "=(?[^;]+)(;|$)"); + var match = regex.Match(vaultReference); + if (match.Success) { - return new ParseSecretResult - { - Uri = new Uri($"https://{altMatch.Groups["VaultName"]}.{vaultUriSuffix}"), - Name = altMatch.Groups["SecretName"].Value - }; + return match.Groups["Value"].Value; } return null; } @@ -89,7 +134,7 @@ private SecretClient GetSecretClient(Uri vaultUri) return clients.GetOrAdd(vaultUri.ToString(), _ => new SecretClient(vaultUri, credential)); } - private class ParseSecretResult + internal class ParseSecretResult { public Uri Uri { get; set; } public string Name { get; set; } diff --git a/test/Azure.Functions.Cli.Tests/KeyVaultReferencesManagerTests.cs b/test/Azure.Functions.Cli.Tests/KeyVaultReferencesManagerTests.cs new file mode 100644 index 000000000..a53ac38bf --- /dev/null +++ b/test/Azure.Functions.Cli.Tests/KeyVaultReferencesManagerTests.cs @@ -0,0 +1,105 @@ +using System; +using System.Collections.Generic; +using System.Text; +using Colors.Net; +using FluentAssertions; +using NSubstitute; +using Xunit; + +using Azure.Functions.Cli.Common; + +namespace Azure.Functions.Cli.Tests +{ + public class KeyVaultReferencesManagerTests + { + private IDictionary _settings; + + private readonly KeyVaultReferencesManager _keyVaultReferencesManager = new KeyVaultReferencesManager(); + + [Theory] + [InlineData("", null)] + [InlineData("testKey", null)] + [InlineData("testKey", "")] + [InlineData("testKey", "testValue")] + [InlineData("testKey", "testValue: {\"nestedKey\": \"nestedValue\"}")] + public void ResolveKeyVaultReferencesDoesNotThrow(string key, string value) + { + _settings = new Dictionary + { + { Constants.AzureWebJobsStorage, "UseDevelopmentStorage=true" }, + }; + _settings.Add(key, value); + Exception exception = null; + try + { + _keyVaultReferencesManager.ResolveKeyVaultReferences(_settings); + } + catch (Exception e) + { + exception = e; + } + exception.Should().BeNull(); + } + + [Theory] + [InlineData("test", null, false)] + [InlineData("test", "@Microsoft.KeyVault()", true)] + [InlineData("test", "@Microsoft.KeyVault(string)", true)] + [InlineData("test", "@Microsoft.KeyVault(SecretUri=bad uri)", true)] + [InlineData("test", "@Microsoft.KeyVault(VaultName=vault;)", true)] // missing secret name + [InlineData("test", "@Microsoft.KeyVault(SecretName=vault;)", true)] // missing vault name + [InlineData("test", "@Microsoft-KeyVault()", false)] // hyphen instead of dot + // Attempted Key Vault references are seen as those matching the regular expression + // "^@Microsoft.KeyVault(.*)$". + public void ParseSecretEmitsWarningWithUnsuccessullyMatchedKeyVaultReferences(string key, string value, bool attemptedKeyVaultReference) + { + var output = new StringBuilder(); + var console = Substitute.For(); + console.WriteLine(Arg.Do(o => output.AppendLine(o?.ToString()))).Returns(console); + console.Write(Arg.Do(o => output.Append(o.ToString()))).Returns(console); + ColoredConsole.Out = console; + ColoredConsole.Error = console; + + _keyVaultReferencesManager.ParseSecret(key, value); + var outputString = output.ToString(); + if (attemptedKeyVaultReference) + { + outputString.Should().Contain($"Unable to parse the Key Vault reference for setting: {key}"); + outputString.Should().NotContain(value); + } + else + { + outputString.Should().BeEmpty(); + } + + } + + // See https://docs.microsoft.com/en-us/azure/app-service/app-service-key-vault-references + // for more detail on supported key vault reference syntax. + [Theory] + [InlineData("SecretUri=https://sampleurl/secrets/mysecret/version", true, "https://sampleurl/", "mysecret", "version")] + [InlineData("SecretUri=https://sampleurl/secrets/mysecret/version;", true, "https://sampleurl/", "mysecret", "version")] // with semicolon at the end + [InlineData("SecretUri=https://sampleurl/secrets/mysecret/", true, "https://sampleurl/", "mysecret", null)] + [InlineData("VaultName=sampleVault;SecretName=mysecret", true, "https://samplevault.vault.azure.net/", "mysecret", null)] + [InlineData("VaultName=sampleVault;SecretName=mysecret;", true, "https://samplevault.vault.azure.net/", "mysecret", null)] // with semicolon at the end + [InlineData("VaultName=sampleVault;SecretName=mysecret;SecretVersion=secretVersion", true, "https://samplevault.vault.azure.net/", "mysecret", "secretVersion")] + [InlineData("SecretName=mysecret;VaultName=sampleVault;SecretVersion=secretVersion", true, "https://samplevault.vault.azure.net/", "mysecret", "secretVersion")] // different order + public void ParseVaultReferenceMatchesFieldsAppropriately( + string vaultReference, + bool shouldMatch, + string expectedVaultUri = null, + string expectedSecretName = null, + string expectedVersion = null) + { + var matchResult = _keyVaultReferencesManager.ParseVaultReference(vaultReference); + + Assert.True(!((matchResult != null) ^ shouldMatch)); + if (shouldMatch) + { + Assert.Equal(matchResult.Uri.ToString(), expectedVaultUri); + Assert.Equal(matchResult.Name, expectedSecretName); + Assert.Equal(matchResult.Version, expectedVersion); + } + } + } +} \ No newline at end of file