If a key is undecryptable, rename it, log it, regnerate it . Fixes #2072

CustomDictionary.xml

@@ -75,6 +75,8 @@
       <Word>Debounce</Word>
       <Word>ScriptHost</Word>
       <Word>EventHub</Word>
+      <Word>Non</Word>
+      <Word>Decryptable</Word>
 	</Recognized>
     <Deprecated/>
     <Compound>

src/WebJobs.Script.WebHost/Properties/Resources.resx

@@ -117,6 +117,9 @@
   <resheader name="writer">
     <value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
   </resheader>
+  <data name="ErrorTooManySecretBackups" xml:space="preserve">
+    <value>Repository has more than {0} non-decryptable secrets backups ({1}).</value>
+  </data>
   <data name="FunctionSecretsSchemaV0" xml:space="preserve">
     <value>{
     "type": "object",
@@ -273,6 +276,12 @@
   <data name="TraceMasterKeyCreatedOrUpdated" xml:space="preserve">
     <value>Master key {0}</value>
   </data>
+  <data name="TraceNonDecryptedFunctionSecretRefresh" xml:space="preserve">
+    <value>Non-decryptable function ('{0}') secrets detected. Refreshing secrets.</value>
+  </data>
+  <data name="TraceNonDecryptedHostSecretRefresh" xml:space="preserve">
+    <value>Non-decryptable host secrets detected. Refreshing secrets.</value>
+  </data>
   <data name="TraceSecretDeleted" xml:space="preserve">
     <value>{0} secret '{1}' deleted.</value>
   </data>

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<string> currentFunctions, ILogger logger)
         {
             // no-op - allow stale secrets to remain
             await Task.Yield();
         }
 
+        public async Task<string[]> 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)

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

@@ -42,11 +42,18 @@ public FileSystemSecretsRepository(string secretsPath)
 
         public event EventHandler<SecretsChangedEventArgs> 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<string> currentFunctions, ILogger logger)
         {
             try
@@ -132,7 +145,8 @@ public async Task PurgeOldSecretsAsync(IList<string> 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<string> currentFunctions, ILogger l
             }
         }
 
+        public async Task<string[]> 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)

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<string> currentFunctions, ILogger logger);
+
+        Task<string[]> GetSecretSnapshots(ScriptSecretsType type, string functionName);
     }
 }

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

@@ -74,9 +74,18 @@ public async virtual Task<HostSecretsInfo> 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<IDictionary<string, string>> 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>(T secrets, string keyScope = null) where T :
             return PersistSecretsAsync(refreshedSecrets, keyScope);
         }
 
-        private Task PersistSecretsAsync<T>(T secrets, string keyScope = null) where T : ScriptSecrets
+        private async Task PersistSecretsAsync<T>(T secrets, string keyScope = null, bool isNonDecryptable = false) where T : ScriptSecrets
         {
+            ScriptSecretsType secretsType = secrets.SecretsType;
             string secretsContent = ScriptSecretSerializer.SerializeSecrets<T>(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)

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";
+        }
+    }
+}

src/WebJobs.Script/Extensions/FileUtility.cs

@@ -110,6 +110,24 @@ string EnsureTrailingSeparator(string path)
             return relativePath;
         }
 
+        public static Task<string[]> 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))

src/WebJobs.Script/ScriptConstants.cs

@@ -70,6 +70,7 @@ public static class ScriptConstants
         public static readonly ImmutableArray<string> HttpMethods = ImmutableArray.Create("get", "post", "delete", "head", "patch", "put", "options");
         public const string HttpMethodConstraintName = "httpMethod";
         public static readonly ImmutableArray<string> 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;
     }
 }

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()