diff --git a/CustomDictionary.xml b/CustomDictionary.xml index e065d3f8c6..aa66c18122 100644 --- a/CustomDictionary.xml +++ b/CustomDictionary.xml @@ -75,6 +75,8 @@ Debounce ScriptHost EventHub + Non + Decryptable diff --git a/src/WebJobs.Script.WebHost/Properties/Resources.Designer.cs b/src/WebJobs.Script.WebHost/Properties/Resources.Designer.cs index 0c06935858..099b20bc2b 100644 --- a/src/WebJobs.Script.WebHost/Properties/Resources.Designer.cs +++ b/src/WebJobs.Script.WebHost/Properties/Resources.Designer.cs @@ -60,6 +60,15 @@ internal Resources() { } } + /// + /// Looks up a localized string similar to Repository has more than {0} non-decryptable secrets backups ({1}).. + /// + internal static string ErrorTooManySecretBackups { + get { + return ResourceManager.GetString("ErrorTooManySecretBackups", resourceCulture); + } + } + /// /// Looks up a localized string similar to { /// "type": "object", @@ -220,6 +229,24 @@ internal static string TraceMasterKeyCreatedOrUpdated { } } + /// + /// Looks up a localized string similar to Non-decryptable function ('{0}') secrets detected. Refreshing secrets.. + /// + internal static string TraceNonDecryptedFunctionSecretRefresh { + get { + return ResourceManager.GetString("TraceNonDecryptedFunctionSecretRefresh", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Non-decryptable host secrets detected. Refreshing secrets.. + /// + internal static string TraceNonDecryptedHostSecretRefresh { + get { + return ResourceManager.GetString("TraceNonDecryptedHostSecretRefresh", resourceCulture); + } + } + /// /// Looks up a localized string similar to {0} secret '{1}' deleted.. /// diff --git a/src/WebJobs.Script.WebHost/Properties/Resources.resx b/src/WebJobs.Script.WebHost/Properties/Resources.resx index a6b168f545..258e3f08de 100644 --- a/src/WebJobs.Script.WebHost/Properties/Resources.resx +++ b/src/WebJobs.Script.WebHost/Properties/Resources.resx @@ -117,6 +117,9 @@ System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + Repository has more than {0} non-decryptable secrets backups ({1}). + { "type": "object", @@ -273,6 +276,12 @@ Master key {0} + + Non-decryptable function ('{0}') secrets detected. Refreshing secrets. + + + Non-decryptable host secrets detected. Refreshing secrets. + {0} secret '{1}' deleted. diff --git a/src/WebJobs.Script.WebHost/Security/KeyManagement/BlobStorageSecretsRepository.cs b/src/WebJobs.Script.WebHost/Security/KeyManagement/BlobStorageSecretsRepository.cs index 9f5c2f1346..25ef0b9c3e 100644 --- a/src/WebJobs.Script.WebHost/Security/KeyManagement/BlobStorageSecretsRepository.cs +++ b/src/WebJobs.Script.WebHost/Security/KeyManagement/BlobStorageSecretsRepository.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Globalization; using System.IO; +using System.Linq; using System.Threading.Tasks; using Microsoft.Azure.WebJobs.Script.IO; using Microsoft.Extensions.Logging; @@ -124,22 +125,48 @@ public async Task WriteAsync(ScriptSecretsType type, string functionName, string } string blobPath = GetSecretsBlobPath(type, functionName); - CloudBlockBlob secretBlob = _blobContainer.GetBlockBlobReference(blobPath); - using (StreamWriter writer = new StreamWriter(await secretBlob.OpenWriteAsync())) - { - await writer.WriteAsync(secretsContent); - } + await WriteToBlobAsync(blobPath, secretsContent); string filePath = GetSecretsSentinelFilePath(type, functionName); await FileUtility.WriteAsync(filePath, DateTime.UtcNow.ToString()); } + public async Task WriteSnapshotAsync(ScriptSecretsType type, string functionName, string secretsContent) + { + if (secretsContent == null) + { + throw new ArgumentNullException(nameof(secretsContent)); + } + + string blobPath = GetSecretsBlobPath(type, functionName); + blobPath = SecretsUtility.GetNonDecryptableName(blobPath); + await WriteToBlobAsync(blobPath, secretsContent); + } + public async Task PurgeOldSecretsAsync(IList currentFunctions, ILogger logger) { // no-op - allow stale secrets to remain await Task.Yield(); } + public async Task GetSecretSnapshots(ScriptSecretsType type, string functionName) + { + // Prefix is secret blob path without extension + string prefix = Path.GetFileNameWithoutExtension(GetSecretsBlobPath(type, functionName)) + $".{ScriptConstants.Snapshot}"; + + BlobResultSegment segmentResult = await _blobContainer.ListBlobsSegmentedAsync(string.Format("{0}/{1}", _secretsBlobPath, prefix.ToLowerInvariant()), null); + return segmentResult.Results.Select(x => x.Uri.ToString()).ToArray(); + } + + private async Task WriteToBlobAsync(string blobPath, string secretsContent) + { + CloudBlockBlob secretBlob = _blobContainer.GetBlockBlobReference(blobPath); + using (StreamWriter writer = new StreamWriter(await secretBlob.OpenWriteAsync())) + { + await writer.WriteAsync(secretsContent); + } + } + private void Dispose(bool disposing) { if (!_disposed) diff --git a/src/WebJobs.Script.WebHost/Security/KeyManagement/FileSystemSecretsRepository.cs b/src/WebJobs.Script.WebHost/Security/KeyManagement/FileSystemSecretsRepository.cs index 9aab4fa7cc..30cad48c91 100644 --- a/src/WebJobs.Script.WebHost/Security/KeyManagement/FileSystemSecretsRepository.cs +++ b/src/WebJobs.Script.WebHost/Security/KeyManagement/FileSystemSecretsRepository.cs @@ -42,11 +42,18 @@ public FileSystemSecretsRepository(string secretsPath) public event EventHandler SecretsChanged; - private string GetSecretsFilePath(ScriptSecretsType secretsType, string functionName = null) + private string GetSecretsFilePath(ScriptSecretsType secretsType, string functionName = null, bool isSnapshot = false) { - return secretsType == ScriptSecretsType.Host + string result = secretsType == ScriptSecretsType.Host ? _hostSecretsPath : GetFunctionSecretsFilePath(functionName); + + if (isSnapshot) + { + result = SecretsUtility.GetNonDecryptableName(result); + } + + return result; } private string GetFunctionSecretsFilePath(string functionName) @@ -120,6 +127,12 @@ public async Task WriteAsync(ScriptSecretsType type, string functionName, string } } + public async Task WriteSnapshotAsync(ScriptSecretsType type, string functionName, string secretsContent) + { + string filePath = GetSecretsFilePath(type, functionName, true); + await FileUtility.WriteAsync(filePath, secretsContent); + } + public async Task PurgeOldSecretsAsync(IList currentFunctions, ILogger logger) { try @@ -132,7 +145,8 @@ public async Task PurgeOldSecretsAsync(IList currentFunctions, ILogger l foreach (var secretFile in secretsDirectory.GetFiles("*.json")) { - if (string.Compare(secretFile.Name, ScriptConstants.HostMetadataFileName, StringComparison.OrdinalIgnoreCase) == 0) + if (string.Compare(secretFile.Name, ScriptConstants.HostMetadataFileName, StringComparison.OrdinalIgnoreCase) == 0 + || secretFile.Name.Contains(ScriptConstants.Snapshot)) { // the secrets directory contains the host secrets file in addition // to function secret files @@ -164,6 +178,13 @@ public async Task PurgeOldSecretsAsync(IList currentFunctions, ILogger l } } + public async Task GetSecretSnapshots(ScriptSecretsType type, string functionName) + { + string prefix = Path.GetFileNameWithoutExtension(GetSecretsFilePath(type, functionName)) + $".{ScriptConstants.Snapshot}*"; + + return await FileUtility.GetFilesAsync(Path.GetDirectoryName(_hostSecretsPath), prefix); + } + private void Dispose(bool disposing) { if (!_disposed) diff --git a/src/WebJobs.Script.WebHost/Security/KeyManagement/ISecretsRepository.cs b/src/WebJobs.Script.WebHost/Security/KeyManagement/ISecretsRepository.cs index 670c733c68..70377249f6 100644 --- a/src/WebJobs.Script.WebHost/Security/KeyManagement/ISecretsRepository.cs +++ b/src/WebJobs.Script.WebHost/Security/KeyManagement/ISecretsRepository.cs @@ -16,6 +16,10 @@ public interface ISecretsRepository Task WriteAsync(ScriptSecretsType type, string functionName, string secretsContent); + Task WriteSnapshotAsync(ScriptSecretsType type, string functionName, string secretsContent); + Task PurgeOldSecretsAsync(IList currentFunctions, ILogger logger); + + Task GetSecretSnapshots(ScriptSecretsType type, string functionName); } } \ No newline at end of file diff --git a/src/WebJobs.Script.WebHost/Security/KeyManagement/SecretManager.cs b/src/WebJobs.Script.WebHost/Security/KeyManagement/SecretManager.cs index 65f7617c27..a51e49387c 100644 --- a/src/WebJobs.Script.WebHost/Security/KeyManagement/SecretManager.cs +++ b/src/WebJobs.Script.WebHost/Security/KeyManagement/SecretManager.cs @@ -74,9 +74,18 @@ public async virtual Task GetHostSecretsAsync() await PersistSecretsAsync(hostSecrets); } - // Host secrets will be in the original persisted state at this point (e.g. encrypted), - // so we read the secrets running them through the appropriate readers - hostSecrets = ReadHostSecrets(hostSecrets); + try + { + // Host secrets will be in the original persisted state at this point (e.g. encrypted), + // so we read the secrets running them through the appropriate readers + hostSecrets = ReadHostSecrets(hostSecrets); + } + catch (CryptographicException) + { + _logger?.LogDebug(Resources.TraceNonDecryptedHostSecretRefresh); + await PersistSecretsAsync(hostSecrets, null, true); + await RefreshSecretsAsync(hostSecrets); + } // If the persistence state of any of our secrets is stale (e.g. the encryption key has been rotated), update // the state and persist the secrets @@ -126,8 +135,18 @@ public async virtual Task> GetFunctionSecretsAsync(s await PersistSecretsAsync(secrets, functionName); } - // Read all secrets, which will run the keys through the appropriate readers - secrets.Keys = secrets.Keys.Select(k => _keyValueConverterFactory.ReadKey(k)).ToList(); + try + { + // Read all secrets, which will run the keys through the appropriate readers + secrets.Keys = secrets.Keys.Select(k => _keyValueConverterFactory.ReadKey(k)).ToList(); + } + catch (CryptographicException) + { + string message = string.Format(Resources.TraceNonDecryptedFunctionSecretRefresh, functionName); + _logger?.LogDebug(message); + await PersistSecretsAsync(secrets, functionName, true); + await RefreshSecretsAsync(secrets, functionName); + } if (secrets.HasStaleKeys) { @@ -358,10 +377,26 @@ private Task RefreshSecretsAsync(T secrets, string keyScope = null) where T : return PersistSecretsAsync(refreshedSecrets, keyScope); } - private Task PersistSecretsAsync(T secrets, string keyScope = null) where T : ScriptSecrets + private async Task PersistSecretsAsync(T secrets, string keyScope = null, bool isNonDecryptable = false) where T : ScriptSecrets { + ScriptSecretsType secretsType = secrets.SecretsType; string secretsContent = ScriptSecretSerializer.SerializeSecrets(secrets); - return _repository.WriteAsync(secrets.SecretsType, keyScope, secretsContent); + if (isNonDecryptable) + { + string[] secretBackups = await _repository.GetSecretSnapshots(secrets.SecretsType, keyScope); + + if (secretBackups.Length >= ScriptConstants.MaximumSecretBackupCount) + { + string message = string.Format(Resources.ErrorTooManySecretBackups, ScriptConstants.MaximumSecretBackupCount, string.IsNullOrEmpty(keyScope) ? "host" : keyScope); + _logger?.LogDebug(message); + throw new InvalidOperationException(message); + } + await _repository.WriteSnapshotAsync(secretsType, keyScope, secretsContent); + } + else + { + await _repository.WriteAsync(secretsType, keyScope, secretsContent); + } } private HostSecrets ReadHostSecrets(HostSecrets hostSecrets) diff --git a/src/WebJobs.Script.WebHost/Security/SecretsUtility.cs b/src/WebJobs.Script.WebHost/Security/SecretsUtility.cs new file mode 100644 index 0000000000..4fe7f0c985 --- /dev/null +++ b/src/WebJobs.Script.WebHost/Security/SecretsUtility.cs @@ -0,0 +1,21 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. + +using System; +using System.IO; + +namespace Microsoft.Azure.WebJobs.Script.WebHost +{ + internal class SecretsUtility + { + public static string GetNonDecryptableName(string secretsPath) + { + string timeStamp = DateTime.UtcNow.ToString("yyyy-MM-dd HH.mm.ss.ffffff"); + if (secretsPath.EndsWith(".json")) + { + secretsPath = secretsPath.Substring(0, secretsPath.Length - 5); + } + return secretsPath + $".{ScriptConstants.Snapshot}.{timeStamp}.json"; + } + } +} \ No newline at end of file diff --git a/src/WebJobs.Script/Extensions/FileUtility.cs b/src/WebJobs.Script/Extensions/FileUtility.cs index cc4fda0911..88c48285f6 100644 --- a/src/WebJobs.Script/Extensions/FileUtility.cs +++ b/src/WebJobs.Script/Extensions/FileUtility.cs @@ -110,6 +110,24 @@ string EnsureTrailingSeparator(string path) return relativePath; } + public static Task GetFilesAsync(string path, string prefix) + { + if (path == null) + { + throw new ArgumentNullException(nameof(path)); + } + + if (prefix == null) + { + throw new ArgumentNullException(nameof(prefix)); + } + + return Task.Run(() => + { + return Directory.GetFiles(path, prefix); + }); + } + public static void CopyDirectory(string sourcePath, string targetPath) { foreach (string dirPath in Directory.GetDirectories(sourcePath, "*", SearchOption.AllDirectories)) diff --git a/src/WebJobs.Script/ScriptConstants.cs b/src/WebJobs.Script/ScriptConstants.cs index 6a1edd5adf..2eb23d0aa5 100644 --- a/src/WebJobs.Script/ScriptConstants.cs +++ b/src/WebJobs.Script/ScriptConstants.cs @@ -70,6 +70,7 @@ public static class ScriptConstants public static readonly ImmutableArray HttpMethods = ImmutableArray.Create("get", "post", "delete", "head", "patch", "put", "options"); public const string HttpMethodConstraintName = "httpMethod"; public static readonly ImmutableArray AssemblyFileTypes = ImmutableArray.Create(".dll", ".exe"); + public const string Snapshot = "snapshot"; public const string Runtime = "runtime"; public const int MaximumHostIdLength = 32; @@ -81,5 +82,6 @@ public static class ScriptConstants public const string PackageReferenceVersionElementName = "Version"; public const int HostTimeoutSeconds = 30; public const int HostPollingIntervalMilliseconds = 25; + public const int MaximumSecretBackupCount = 10; } } diff --git a/test/WebJobs.Script.Tests.Integration/Host/SecretsRepositoryTests.cs b/test/WebJobs.Script.Tests.Integration/Host/SecretsRepositoryTests.cs index de07e89852..5af183a8da 100644 --- a/test/WebJobs.Script.Tests.Integration/Host/SecretsRepositoryTests.cs +++ b/test/WebJobs.Script.Tests.Integration/Host/SecretsRepositoryTests.cs @@ -196,6 +196,32 @@ public async Task FileSystemRepo_PurgeOldSecrets_RemovesOldAndKeepsCurrentSecret } } + [Theory] + [InlineData(SecretsRepositoryType.FileSystem, ScriptSecretsType.Host)] + [InlineData(SecretsRepositoryType.FileSystem, ScriptSecretsType.Function)] + [InlineData(SecretsRepositoryType.BlobStorage, ScriptSecretsType.Host)] + [InlineData(SecretsRepositoryType.BlobStorage, ScriptSecretsType.Function)] + public async Task GetSecretSnapshots_ReturnsExpected(SecretsRepositoryType repositoryType, ScriptSecretsType secretsType) + { + using (var directory = new TempDirectory()) + { + _fixture.TestInitialize(repositoryType, directory.Path); + + string testContent = "test"; + string testFunctionName = secretsType == ScriptSecretsType.Host ? null : "TestFunction"; + + var target = _fixture.GetNewSecretRepository(); + await target.WriteAsync(secretsType, testFunctionName, testContent); + for (int i = 0; i < 5; i++) + { + await target.WriteSnapshotAsync(secretsType, testFunctionName, testContent); + } + string[] files = await target.GetSecretSnapshots(secretsType, testFunctionName); + + Assert.True(files.Length > 0); + } + } + public class Fixture : IDisposable { public Fixture() diff --git a/test/WebJobs.Script.Tests/Security/SecretManagerTests.cs b/test/WebJobs.Script.Tests/Security/SecretManagerTests.cs index 82d2ac292e..804c01092c 100644 --- a/test/WebJobs.Script.Tests/Security/SecretManagerTests.cs +++ b/test/WebJobs.Script.Tests/Security/SecretManagerTests.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Security.Cryptography; using System.Threading.Tasks; using Microsoft.Azure.WebJobs.Script.Config; using Microsoft.Azure.WebJobs.Script.WebHost; @@ -467,6 +468,126 @@ public void Constructor_WithCreateHostSecretsIfMissingSet_CreatesHostSecret() } } + [Fact] + public async Task GetHostSecrets_WhenNonDecryptedHostSecrets_SavesAndRefreshes() + { + using (var directory = new TempDirectory()) + { + string expectedTraceMessage = Resources.TraceNonDecryptedHostSecretRefresh; + string hostSecretsJson = + @"{ + 'masterKey': { + 'name': 'master', + 'value': 'cryptoError', + 'encrypted': true + }, + 'functionKeys': [], + 'systemKeys': [] +}"; + File.WriteAllText(Path.Combine(directory.Path, ScriptConstants.HostMetadataFileName), hostSecretsJson); + Mock mockValueConverterFactory = GetConverterFactoryMock(true, false); + HostSecretsInfo hostSecrets; + ISecretsRepository repository = new FileSystemSecretsRepository(directory.Path); + + using (var secretManager = new SecretManager(repository, mockValueConverterFactory.Object, null)) + { + hostSecrets = await secretManager.GetHostSecretsAsync(); + } + + Assert.NotNull(hostSecrets); + Assert.Equal(hostSecrets.MasterKey, "cryptoError"); + var result = JsonConvert.DeserializeObject(File.ReadAllText(Path.Combine(directory.Path, ScriptConstants.HostMetadataFileName))); + Assert.Equal(result.MasterKey.Value, "!cryptoError"); + Assert.Equal(1, Directory.GetFiles(directory.Path, $"host.{ScriptConstants.Snapshot}*").Length); + } + } + + [Fact] + public async Task GetFunctiontSecrets_WhenNonDecryptedSecrets_SavesAndRefreshes() + { + using (var directory = new TempDirectory()) + { + string functionName = "testfunction"; + string expectedTraceMessage = string.Format(Resources.TraceNonDecryptedFunctionSecretRefresh, functionName); + string functionSecretsJson = + @"{ + 'keys': [ + { + 'name': 'Key1', + 'value': 'cryptoError', + 'encrypted': true + }, + { + 'name': 'Key2', + 'value': '1234', + 'encrypted': false + } + ] +}"; + File.WriteAllText(Path.Combine(directory.Path, functionName + ".json"), functionSecretsJson); + Mock mockValueConverterFactory = GetConverterFactoryMock(true, false); + IDictionary functionSecrets; + ISecretsRepository repository = new FileSystemSecretsRepository(directory.Path); + + using (var secretManager = new SecretManager(repository, mockValueConverterFactory.Object, null)) + { + functionSecrets = await secretManager.GetFunctionSecretsAsync(functionName); + } + + Assert.NotNull(functionSecrets); + Assert.Equal(functionSecrets["Key1"], "cryptoError"); + var result = JsonConvert.DeserializeObject(File.ReadAllText(Path.Combine(directory.Path, functionName + ".json"))); + Assert.Equal(result.GetFunctionKey("Key1", functionName).Value, "!cryptoError"); + Assert.Equal(1, Directory.GetFiles(directory.Path, $"{functionName}.{ScriptConstants.Snapshot}*").Length); + } + } + + [Fact] + public async Task GetHostSecrets_WhenTooManyBackups_ThrowsException() + { + using (var directory = new TempDirectory()) + { + string functionName = "testfunction"; + string expectedTraceMessage = string.Format(Resources.ErrorTooManySecretBackups, ScriptConstants.MaximumSecretBackupCount, functionName); + string functionSecretsJson = + @"{ + 'keys': [ + { + 'name': 'Key1', + 'value': 'FunctionValue1', + 'encrypted': true + }, + { + 'name': 'Key2', + 'value': 'FunctionValue2', + 'encrypted': false + } + ] +}"; + + IDictionary functionSecrets; + ISecretsRepository repository = new FileSystemSecretsRepository(directory.Path); + + using (var secretManager = new SecretManager(_settingsManager, repository, null)) + { + await Assert.ThrowsAsync(async () => + { + for (int i = 0; i < ScriptConstants.MaximumSecretBackupCount + 10; i++) + { + File.WriteAllText(Path.Combine(directory.Path, functionName + ".json"), functionSecretsJson); + functionSecrets = await secretManager.GetFunctionSecretsAsync(functionName); + if (i > ScriptConstants.MaximumSecretBackupCount) + { + await Task.Delay(500); + } + } + }); + } + + Assert.True(Directory.GetFiles(directory.Path, $"{functionName}.{ScriptConstants.Snapshot}*").Length >= ScriptConstants.MaximumSecretBackupCount); + } + } + [Fact] public async Task GetHostSecretsAsync_WaitsForNewSecrets() { @@ -485,10 +606,9 @@ public async Task GetHostSecretsAsync_WaitsForNewSecrets() File.WriteAllText(filePath, hostSecretsJson); HostSecretsInfo hostSecrets = null; - var traceWriter = new TestTraceWriter(TraceLevel.Verbose); ISecretsRepository repository = new FileSystemSecretsRepository(directory.Path); - using (var secretManager = new SecretManager(_settingsManager, repository, traceWriter, null)) + using (var secretManager = new SecretManager(_settingsManager, repository, null)) { await Task.WhenAll( Task.Run(async () => @@ -508,7 +628,7 @@ await Task.WhenAll( Assert.Equal(hostSecrets.MasterKey, "1234"); } - using (var secretManager = new SecretManager(_settingsManager, repository, traceWriter, null)) + using (var secretManager = new SecretManager(_settingsManager, repository, null)) { await Assert.ThrowsAsync(async () => { @@ -556,9 +676,8 @@ public async Task GetFunctionSecretsAsync_WaitsForNewSecrets() File.WriteAllText(filePath, functionSecretsJson); IDictionary functionSecrets = null; - var traceWriter = new TestTraceWriter(TraceLevel.Verbose); ISecretsRepository repository = new FileSystemSecretsRepository(directory.Path); - using (var secretManager = new SecretManager(_settingsManager, repository, traceWriter, null)) + using (var secretManager = new SecretManager(_settingsManager, repository, null)) { await Task.WhenAll( Task.Run(async () => @@ -578,7 +697,7 @@ await Task.WhenAll( Assert.Equal(functionSecrets["Key1"], "FunctionValue1"); } - using (var secretManager = new SecretManager(_settingsManager, repository, traceWriter, null)) + using (var secretManager = new SecretManager(_settingsManager, repository, null)) { await Assert.ThrowsAsync(async () => { @@ -605,7 +724,14 @@ private Mock GetConverterFactoryMock(bool simulateWri { var mockValueReader = new Mock(); mockValueReader.Setup(r => r.ReadValue(It.IsAny())) - .Returns(k => new Key(k.Name, k.Value) { IsStale = setStaleValue ? true : k.IsStale }); + .Returns(k => + { + if (k.Value.StartsWith("cryptoError")) + { + throw new CryptographicException(); + } + return new Key(k.Name, k.Value) { IsStale = setStaleValue ? true : k.IsStale }; + }); var mockValueWriter = new Mock(); mockValueWriter.Setup(r => r.WriteValue(It.IsAny()))