Skip to content

Commit 294212a

Browse files
alrodalrod
committed
If a key is undecryptable, rename it, log it, regnerate it . Fixes #2072 
1 parent dc66ddf commit 294212a

File tree

13 files changed

+350
-16
lines changed

13 files changed

+350
-16
lines changed

‎CustomDictionary.xml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@
7575
<Word>Debounce</Word>
7676
<Word>ScriptHost</Word>
7777
<Word>EventHub</Word>
78+
<Word>Non</Word>
79+
<Word>Decryptable</Word>
7880
</Recognized>
7981
<Deprecated/>
8082
<Compound>

‎src/WebJobs.Script.WebHost/Properties/Resources.Designer.cs‎

Lines changed: 27 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,9 @@
117117
<resheader name="writer">
118118
<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
119119
</resheader>
120+
<data name="ErrorTooManySecretBackups" xml:space="preserve">
121+
<value>Repository has more than {0} non-decryptable secrets backups ({1}).</value>
122+
</data>
120123
<data name="FunctionSecretsSchemaV0" xml:space="preserve">
121124
<value>{
122125
"type": "object",
@@ -289,6 +292,12 @@
289292
<data name="TraceMasterKeyCreatedOrUpdated" xml:space="preserve">
290293
<value>Master key {0}</value>
291294
</data>
295+
<data name="TraceNonDecryptedFunctionSecretRefresh" xml:space="preserve">
296+
<value>Non-decryptable function ('{0}') secrets detected. Refreshing secrets.</value>
297+
</data>
298+
<data name="TraceNonDecryptedHostSecretRefresh" xml:space="preserve">
299+
<value>Non-decryptable host secrets detected. Refreshing secrets.</value>
300+
</data>
292301
<data name="TraceSecretDeleted" xml:space="preserve">
293302
<value>{0} secret '{1}' deleted.</value>
294303
</data>

‎src/WebJobs.Script.WebHost/Security/BlobStorageSecretsRepository.cs‎

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Collections.Generic;
66
using System.Globalization;
77
using System.IO;
8+
using System.Linq;
89
using System.Threading.Tasks;
910
using Microsoft.Azure.WebJobs.Host;
1011
using Microsoft.Azure.WebJobs.Script.IO;
@@ -123,22 +124,48 @@ public async Task WriteAsync(ScriptSecretsType type, string functionName, string
123124
}
124125

125126
string blobPath = GetSecretsBlobPath(type, functionName);
126-
CloudBlockBlob secretBlob = _blobContainer.GetBlockBlobReference(blobPath);
127-
using (StreamWriter writer = new StreamWriter(await secretBlob.OpenWriteAsync()))
128-
{
129-
await writer.WriteAsync(secretsContent);
130-
}
127+
await WriteToBlobAsync(blobPath, secretsContent);
131128

132129
string filePath = GetSecretsSentinelFilePath(type, functionName);
133130
await FileUtility.WriteAsync(filePath, DateTime.UtcNow.ToString());
134131
}
135132

133+
public async Task WriteSnapshotAsync(ScriptSecretsType type, string functionName, string secretsContent)
134+
{
135+
if (secretsContent == null)
136+
{
137+
throw new ArgumentNullException(nameof(secretsContent));
138+
}
139+
140+
string blobPath = GetSecretsBlobPath(type, functionName);
141+
blobPath = SecretsUtility.GetNonDecryptableName(blobPath);
142+
await WriteToBlobAsync(blobPath, secretsContent);
143+
}
144+
136145
public async Task PurgeOldSecretsAsync(IList<string> currentFunctions, TraceWriter traceWriter, ILogger logger)
137146
{
138147
// no-op - allow stale secrets to remain
139148
await Task.Yield();
140149
}
141150

151+
public async Task<string[]> GetSecretSnapshots(ScriptSecretsType type, string functionName)
152+
{
153+
// Prefix is secret blob path without extension
154+
string prefix = Path.GetFileNameWithoutExtension(GetSecretsBlobPath(type, functionName)) + $".{ScriptConstants.Snapshot}";
155+
156+
BlobResultSegment segmentResult = await _blobContainer.ListBlobsSegmentedAsync(string.Format("{0}/{1}", _secretsBlobPath, prefix.ToLowerInvariant()), null);
157+
return segmentResult.Results.Select(x => x.Uri.ToString()).ToArray();
158+
}
159+
160+
private async Task WriteToBlobAsync(string blobPath, string secretsContent)
161+
{
162+
CloudBlockBlob secretBlob = _blobContainer.GetBlockBlobReference(blobPath);
163+
using (StreamWriter writer = new StreamWriter(await secretBlob.OpenWriteAsync()))
164+
{
165+
await writer.WriteAsync(secretsContent);
166+
}
167+
}
168+
142169
private void Dispose(bool disposing)
143170
{
144171
if (!_disposed)

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

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,18 @@ public FileSystemSecretsRepository(string secretsPath)
4343

4444
public event EventHandler<SecretsChangedEventArgs> SecretsChanged;
4545

46-
private string GetSecretsFilePath(ScriptSecretsType secretsType, string functionName = null)
46+
private string GetSecretsFilePath(ScriptSecretsType secretsType, string functionName = null, bool isSnapshot = false)
4747
{
48-
return secretsType == ScriptSecretsType.Host
48+
string result = secretsType == ScriptSecretsType.Host
4949
? _hostSecretsPath
5050
: GetFunctionSecretsFilePath(functionName);
51+
52+
if (isSnapshot)
53+
{
54+
result = SecretsUtility.GetNonDecryptableName(result);
55+
}
56+
57+
return result;
5158
}
5259

5360
private string GetFunctionSecretsFilePath(string functionName)
@@ -121,6 +128,12 @@ public async Task WriteAsync(ScriptSecretsType type, string functionName, string
121128
}
122129
}
123130

131+
public async Task WriteSnapshotAsync(ScriptSecretsType type, string functionName, string secretsContent)
132+
{
133+
string filePath = GetSecretsFilePath(type, functionName, true);
134+
await FileUtility.WriteAsync(filePath, secretsContent);
135+
}
136+
124137
public async Task PurgeOldSecretsAsync(IList<string> currentFunctions, TraceWriter traceWriter, ILogger logger)
125138
{
126139
try
@@ -133,7 +146,8 @@ public async Task PurgeOldSecretsAsync(IList<string> currentFunctions, TraceWrit
133146

134147
foreach (var secretFile in secretsDirectory.GetFiles("*.json"))
135148
{
136-
if (string.Compare(secretFile.Name, ScriptConstants.HostMetadataFileName, StringComparison.OrdinalIgnoreCase) == 0)
149+
if (string.Compare(secretFile.Name, ScriptConstants.HostMetadataFileName, StringComparison.OrdinalIgnoreCase) == 0
150+
|| secretFile.Name.Contains(ScriptConstants.Snapshot))
137151
{
138152
// the secrets directory contains the host secrets file in addition
139153
// to function secret files
@@ -167,6 +181,13 @@ public async Task PurgeOldSecretsAsync(IList<string> currentFunctions, TraceWrit
167181
}
168182
}
169183

184+
public async Task<string[]> GetSecretSnapshots(ScriptSecretsType type, string functionName)
185+
{
186+
string prefix = Path.GetFileNameWithoutExtension(GetSecretsFilePath(type, functionName)) + $".{ScriptConstants.Snapshot}*";
187+
188+
return await FileUtility.GetFilesAsync(Path.GetDirectoryName(_hostSecretsPath), prefix);
189+
}
190+
170191
private void Dispose(bool disposing)
171192
{
172193
if (!_disposed)

‎src/WebJobs.Script.WebHost/Security/ISecretsRepository.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ public interface ISecretsRepository
1717

1818
Task WriteAsync(ScriptSecretsType type, string functionName, string secretsContent);
1919

20+
Task WriteSnapshotAsync(ScriptSecretsType type, string functionName, string secretsContent);
21+
2022
Task PurgeOldSecretsAsync(IList<string> currentFunctions, TraceWriter traceWriter, ILogger logger);
23+
24+
Task<string[]> GetSecretSnapshots(ScriptSecretsType type, string functionName);
2125
}
2226
}

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

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,19 @@ public async virtual Task<HostSecretsInfo> GetHostSecretsAsync()
7878
await PersistSecretsAsync(hostSecrets);
7979
}
8080

81-
// Host secrets will be in the original persisted state at this point (e.g. encrypted),
82-
// so we read the secrets running them through the appropriate readers
83-
hostSecrets = ReadHostSecrets(hostSecrets);
81+
try
82+
{
83+
// Host secrets will be in the original persisted state at this point (e.g. encrypted),
84+
// so we read the secrets running them through the appropriate readers
85+
hostSecrets = ReadHostSecrets(hostSecrets);
86+
}
87+
catch (CryptographicException)
88+
{
89+
_traceWriter.Verbose(Resources.TraceNonDecryptedHostSecretRefresh);
90+
_logger?.LogDebug(Resources.TraceNonDecryptedHostSecretRefresh);
91+
await PersistSecretsAsync(hostSecrets, null, true);
92+
await RefreshSecretsAsync(hostSecrets);
93+
}
8494

8595
// If the persistence state of any of our secrets is stale (e.g. the encryption key has been rotated), update
8696
// the state and persist the secrets
@@ -132,8 +142,19 @@ public async virtual Task<IDictionary<string, string>> GetFunctionSecretsAsync(s
132142
await PersistSecretsAsync(secrets, functionName);
133143
}
134144

135-
// Read all secrets, which will run the keys through the appropriate readers
136-
secrets.Keys = secrets.Keys.Select(k => _keyValueConverterFactory.ReadKey(k)).ToList();
145+
try
146+
{
147+
// Read all secrets, which will run the keys through the appropriate readers
148+
secrets.Keys = secrets.Keys.Select(k => _keyValueConverterFactory.ReadKey(k)).ToList();
149+
}
150+
catch (CryptographicException)
151+
{
152+
string message = string.Format(Resources.TraceNonDecryptedFunctionSecretRefresh, functionName);
153+
_traceWriter.Verbose(message);
154+
_logger?.LogDebug(message);
155+
await PersistSecretsAsync(secrets, functionName, true);
156+
await RefreshSecretsAsync(secrets, functionName);
157+
}
137158

138159
if (secrets.HasStaleKeys)
139160
{
@@ -372,10 +393,27 @@ private Task RefreshSecretsAsync<T>(T secrets, string keyScope = null) where T :
372393
return PersistSecretsAsync(refreshedSecrets, keyScope);
373394
}
374395

375-
private Task PersistSecretsAsync<T>(T secrets, string keyScope = null) where T : ScriptSecrets
396+
private async Task PersistSecretsAsync<T>(T secrets, string keyScope = null, bool isNonDecryptable = false) where T : ScriptSecrets
376397
{
398+
ScriptSecretsType secretsType = secrets.SecretsType;
377399
string secretsContent = ScriptSecretSerializer.SerializeSecrets<T>(secrets);
378-
return _repository.WriteAsync(secrets.SecretsType, keyScope, secretsContent);
400+
if (isNonDecryptable)
401+
{
402+
string[] secretBackups = await _repository.GetSecretSnapshots(secrets.SecretsType, keyScope);
403+
404+
if (secretBackups.Length >= ScriptConstants.MaximumSecretBackupCount)
405+
{
406+
string message = string.Format(Resources.ErrorTooManySecretBackups, ScriptConstants.MaximumSecretBackupCount, string.IsNullOrEmpty(keyScope) ? "host" : keyScope);
407+
_traceWriter.Verbose(message);
408+
_logger?.LogDebug(message);
409+
throw new InvalidOperationException(message);
410+
}
411+
await _repository.WriteSnapshotAsync(secretsType, keyScope, secretsContent);
412+
}
413+
else
414+
{
415+
await _repository.WriteAsync(secretsType, keyScope, secretsContent);
416+
}
379417
}
380418

381419
private HostSecrets ReadHostSecrets(HostSecrets hostSecrets)

‎src/WebJobs.Script.WebHost/Security/SecretsUtility.cs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the MIT License. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.IO;
6+
7+
namespace Microsoft.Azure.WebJobs.Script.WebHost
8+
{
9+
internal class SecretsUtility
10+
{
11+
public static string GetNonDecryptableName(string secretsPath)
12+
{
13+
string timeStamp = DateTime.UtcNow.ToString("yyyy-MM-dd HH.mm.ss.ffffff");
14+
if (secretsPath.EndsWith(".json"))
15+
{
16+
secretsPath = secretsPath.Substring(0, secretsPath.Length - 5);
17+
}
18+
return secretsPath + $".{ScriptConstants.Snapshot}.{timeStamp}.json";
19+
}
20+
}
21+
}

‎src/WebJobs.Script.WebHost/WebJobs.Script.WebHost.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,7 @@
440440
<Compile Include="Extensions\HttpRouteCollectionExtensions.cs" />
441441
<Compile Include="Extensions\HttpRouteFactoryExtensions.cs" />
442442
<Compile Include="Filters\RequiresRunningHostAttribute.cs" />
443+
<Compile Include="Security\SecretsUtility.cs" />
443444
<Compile Include="StandbyManager.cs" />
444445
<Compile Include="Controllers\AdminController.cs" />
445446
<Compile Include="Controllers\FunctionsController.cs" />

‎src/WebJobs.Script/Extensions/FileUtility.cs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,5 +109,23 @@ string EnsureTrailingSeparator(string path)
109109

110110
return relativePath;
111111
}
112+
113+
public static Task<string[]> GetFilesAsync(string path, string prefix)
114+
{
115+
if (path == null)
116+
{
117+
throw new ArgumentNullException(nameof(path));
118+
}
119+
120+
if (prefix == null)
121+
{
122+
throw new ArgumentNullException(nameof(prefix));
123+
}
124+
125+
return Task.Run(() =>
126+
{
127+
return Directory.GetFiles(path, prefix);
128+
});
129+
}
112130
}
113131
}

0 commit comments

Comments
 (0)