Skip to content

Commit c64e6c4

Browse files
mathewcmathewc
authored
Adding HIS Enforcement feature flags and metrics logging (#9872)
1 parent 1c0a2ef commit c64e6c4

File tree

14 files changed

+774
-84
lines changed

14 files changed

+774
-84
lines changed

‎src/WebJobs.Script.WebHost/Controllers/KeysController.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@ namespace Microsoft.Azure.WebJobs.Script.WebHost.Controllers
2323
[ResourceContainsSecrets]
2424
public class KeysController : Controller
2525
{
26-
private const string MasterKeyName = "_master";
27-
2826
private static readonly Lazy<Dictionary<string, string>> EmptyKeys = new Lazy<Dictionary<string, string>>(() => new Dictionary<string, string>());
2927
private readonly ISecretManagerProvider _secretManagerProvider;
3028
private readonly ILogger _logger;
@@ -187,7 +185,7 @@ private async Task<IActionResult> AddOrUpdateSecretAsync(string keyName, string
187185
}
188186

189187
KeyOperationResult operationResult;
190-
if (secretsType == ScriptSecretsType.Host && string.Equals(keyName, MasterKeyName, StringComparison.OrdinalIgnoreCase))
188+
if (secretsType == ScriptSecretsType.Host && string.Equals(keyName, ScriptConstants.MasterKeyName, StringComparison.OrdinalIgnoreCase))
191189
{
192190
operationResult = await _secretManagerProvider.Current.SetMasterKeyAsync(value);
193191
}
@@ -216,6 +214,8 @@ private async Task<IActionResult> AddOrUpdateSecretAsync(string keyName, string
216214
return NotFound();
217215
case OperationResult.Conflict:
218216
return StatusCode(StatusCodes.Status409Conflict);
217+
case OperationResult.BadRequest:
218+
return StatusCode(StatusCodes.Status400BadRequest);
219219
default:
220220
return StatusCode(StatusCodes.Status500InternalServerError);
221221
}
@@ -237,7 +237,7 @@ private async Task<Dictionary<string, string>> GetHostSecretsByScope(string secr
237237
{
238238
keys = new Dictionary<string, string>(keys)
239239
{
240-
{ MasterKeyName, hostSecrets.MasterKey }
240+
{ ScriptConstants.MasterKeyName, hostSecrets.MasterKey }
241241
};
242242
}
243243

@@ -276,7 +276,7 @@ private async Task<IActionResult> DeleteFunctionSecretAsync(string keyName, stri
276276

277277
internal bool IsBuiltInSystemKeyName(string keyName)
278278
{
279-
if (keyName.Equals(MasterKeyName, StringComparison.OrdinalIgnoreCase))
279+
if (keyName.Equals(ScriptConstants.MasterKeyName, StringComparison.OrdinalIgnoreCase))
280280
{
281281
return true;
282282
}

‎src/WebJobs.Script.WebHost/Diagnostics/Extensions/ScriptHostServiceLoggerExtension.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ public static void ScriptHostServiceRestartCanceledByRuntime(this ILogger logger
333333
public static void LogHostInitializationSettings(this ILogger logger, string originalFunctionWorkerRuntime, string functionWorkerRuntime,
334334
string originalFunctionWorkerRuntimeVersion, string functionsWorkerRuntimeVersion, string functionExtensionVersion, string hostDirectory,
335335
bool inStandbyMode, bool hasBeenSpecialized, bool usePlaceholderDotNetIsolated, string websiteSku, string featureFlags,
336-
IDictionary<string, string> hostingConfig)
336+
IDictionary<string, string> hostingConfig, string hisMode)
337337
{
338338
// This is a dump of values for telemetry right now, but eventually we will refactor this
339339
// into a proper Options object
@@ -350,7 +350,8 @@ public static void LogHostInitializationSettings(this ILogger logger, string ori
350350
UsePlaceholderDotNetIsolated = usePlaceholderDotNetIsolated,
351351
WebSiteSku = websiteSku,
352352
FeatureFlags = featureFlags,
353-
HostingConfig = hostingConfig
353+
HostingConfig = hostingConfig,
354+
HISMode = hisMode
354355
};
355356

356357
var options = new JsonSerializerOptions { WriteIndented = true };

‎src/WebJobs.Script.WebHost/OperationResult.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ public enum OperationResult
1010
Created,
1111
Updated,
1212
NotFound,
13-
Conflict
13+
Conflict,
14+
BadRequest
1415
}
1516
}

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

Lines changed: 9 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: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,9 @@
283283
<data name="HostSpecializationTrace" xml:space="preserve">
284284
<value>Starting host specialization</value>
285285
</data>
286+
<data name="NonHISSecret" xml:space="preserve">
287+
<value>A non highly-identifiable secret has been loaded by the application: (KeyType:{0}, KeyName:{1}, FunctionName:{2}).</value>
288+
</data>
286289
<data name="TraceAddOrUpdateFunctionSecret" xml:space="preserve">
287290
<value>{0} secret '{1}' for '{2}' {3}.</value>
288291
</data>

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

Lines changed: 154 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,13 @@
1111
using System.Text;
1212
using System.Threading;
1313
using System.Threading.Tasks;
14-
using DryIoc;
15-
using Microsoft.Azure.Web.DataProtection;
1614
using Microsoft.Azure.WebJobs.Extensions.Http;
15+
using Microsoft.Azure.WebJobs.Script.Config;
1716
using Microsoft.Azure.WebJobs.Script.Diagnostics;
1817
using Microsoft.Azure.WebJobs.Script.WebHost.Properties;
1918
using Microsoft.Azure.WebJobs.Script.WebHost.Security;
20-
using Microsoft.Extensions.Hosting;
2119
using Microsoft.Extensions.Logging;
20+
using static Microsoft.Azure.WebJobs.Script.WebHost.Models.FunctionAppSecrets;
2221
using DataProtectionConstants = Microsoft.Azure.Web.DataProtection.Constants;
2322

2423
namespace Microsoft.Azure.WebJobs.Script.WebHost
@@ -30,6 +29,9 @@ public class SecretManager : IDisposable, ISecretManager
3029
private readonly ISecretsRepository _repository;
3130
private readonly HostNameProvider _hostNameProvider;
3231
private readonly StartupContextProvider _startupContextProvider;
32+
private readonly Lazy<bool> _strictHISFeatureEnabled = new Lazy<bool>(() => FeatureFlags.IsEnabled(ScriptConstants.FeatureFlagStrictHISModeEnabled));
33+
private readonly Lazy<bool> _strictHISWarnFeatureEnabled = new Lazy<bool>(() => FeatureFlags.IsEnabled(ScriptConstants.FeatureFlagStrictHISModeWarn));
34+
private readonly HashSet<string> _invalidNonHISKeys = new HashSet<string>();
3335
private ConcurrentDictionary<string, IDictionary<string, string>> _functionSecrets;
3436
private ConcurrentDictionary<string, (string, AuthorizationLevel)> _authorizationCache = new ConcurrentDictionary<string, (string, AuthorizationLevel)>(StringComparer.OrdinalIgnoreCase);
3537
private HostSecretsInfo _hostSecrets;
@@ -137,11 +139,17 @@ public async virtual Task<HostSecretsInfo> GetHostSecretsAsync()
137139
await RefreshSecretsAsync(hostSecrets);
138140
}
139141

142+
// before caching any secrets, validate them
143+
string masterKeyValue = hostSecrets.MasterKey.Value;
144+
var functionKeys = hostSecrets.FunctionKeys.ToDictionary(p => p.Name, p => p.Value);
145+
var systemKeys = hostSecrets.SystemKeys.ToDictionary(p => p.Name, p => p.Value);
146+
ValidateHostSecrets(masterKeyValue, functionKeys, systemKeys);
147+
140148
_hostSecrets = new HostSecretsInfo
141149
{
142-
MasterKey = hostSecrets.MasterKey.Value,
143-
FunctionKeys = hostSecrets.FunctionKeys.ToDictionary(s => s.Name, s => s.Value),
144-
SystemKeys = hostSecrets.SystemKeys.ToDictionary(s => s.Name, s => s.Value)
150+
MasterKey = masterKeyValue,
151+
FunctionKeys = functionKeys,
152+
SystemKeys = systemKeys
145153
};
146154
}
147155
finally
@@ -219,7 +227,10 @@ public async virtual Task<IDictionary<string, string>> GetFunctionSecretsAsync(s
219227
await RefreshSecretsAsync(secrets, functionName);
220228
}
221229

230+
// before caching any secrets, validate them
222231
var result = secrets.Keys.ToDictionary(s => s.Name, s => s.Value);
232+
ValidateSecrets(result, SecretGenerator.FunctionKeySeed, functionName);
233+
223234
functionSecrets = _functionSecrets.AddOrUpdate(functionName, result, (n, r) => result);
224235
}
225236
finally
@@ -290,6 +301,15 @@ public async Task<KeyOperationResult> SetMasterKeyAsync(string value = null)
290301
}
291302
else
292303
{
304+
if (_strictHISFeatureEnabled.Value)
305+
{
306+
// if an explicit value has been provided and strict HIS mode is enabled, validate the secret
307+
if (!SecretGenerator.TryValidateSecret(value, SecretGenerator.MasterKeySeed))
308+
{
309+
return new KeyOperationResult(value, OperationResult.BadRequest);
310+
}
311+
}
312+
293313
// Use the provided secret
294314
masterKey = value;
295315
result = OperationResult.Updated;
@@ -329,11 +349,57 @@ public async Task<bool> DeleteSecretAsync(string secretName, string keyScope, Sc
329349
}
330350
}
331351

352+
internal static ulong GetKeySeed(ScriptSecretsType secretsType, string keyScope)
353+
{
354+
if (secretsType == ScriptSecretsType.Function)
355+
{
356+
return SecretGenerator.FunctionKeySeed;
357+
}
358+
else if (secretsType == ScriptSecretsType.Host)
359+
{
360+
if (string.Equals(keyScope, HostKeyScopes.FunctionKeys, StringComparison.OrdinalIgnoreCase))
361+
{
362+
return SecretGenerator.FunctionKeySeed;
363+
}
364+
else if (string.Equals(keyScope, HostKeyScopes.SystemKeys, StringComparison.OrdinalIgnoreCase))
365+
{
366+
return SecretGenerator.SystemKeySeed;
367+
}
368+
}
369+
370+
return 0;
371+
}
372+
373+
public static string GetKeyType(ulong seed)
374+
{
375+
switch (seed)
376+
{
377+
case SecretGenerator.MasterKeySeed:
378+
return "Master";
379+
case SecretGenerator.SystemKeySeed:
380+
return "System";
381+
case SecretGenerator.FunctionKeySeed:
382+
return "Function";
383+
default:
384+
return "Unknown";
385+
}
386+
}
387+
332388
private async Task<KeyOperationResult> AddOrUpdateSecretAsync(ScriptSecretsType secretsType, string keyScope,
333389
string secretName, string secret, Func<ScriptSecrets> secretsFactory)
334390
{
335391
OperationResult result = OperationResult.NotFound;
336392

393+
if (!string.IsNullOrEmpty(secret) && _strictHISFeatureEnabled.Value)
394+
{
395+
// if an explicit value has been provided and strict HIS mode is enabled, validate the secret
396+
ulong seed = GetKeySeed(secretsType, keyScope);
397+
if (seed > 0 && !SecretGenerator.TryValidateSecret(secret, seed))
398+
{
399+
return new KeyOperationResult(secret, OperationResult.BadRequest);
400+
}
401+
}
402+
337403
secret ??= SecretGenerator.GenerateFunctionKeyValue();
338404

339405
await ModifyFunctionSecretsAsync(secretsType, keyScope, secrets =>
@@ -400,8 +466,7 @@ private async Task ModifyFunctionSecretsAsync(ScriptSecretsType secretsType, str
400466
}
401467
}
402468

403-
private Task<FunctionSecrets> LoadFunctionSecretsAsync(string functionName)
404-
=> LoadSecretsAsync<FunctionSecrets>(functionName);
469+
private Task<FunctionSecrets> LoadFunctionSecretsAsync(string functionName) => LoadSecretsAsync<FunctionSecrets>(functionName);
405470

406471
private async Task<T> LoadSecretsAsync<T>(string keyScope = null) where T : ScriptSecrets
407472
{
@@ -419,6 +484,22 @@ private async Task<ScriptSecrets> LoadSecretsAsync(ScriptSecretsType type, strin
419484

420485
public async Task<(string KeyName, AuthorizationLevel Level)> GetAuthorizationLevelOrNullAsync(string keyValue, string functionName = null)
421486
{
487+
// local helper function to get auth level and also enforce HIS validation
488+
async Task<(string KeyName, AuthorizationLevel Level)> GetAuthorizationLevelAndValidateAsync(string keyValue, string functionName)
489+
{
490+
var result = await GetAuthorizationLevelAsync(this, keyValue, functionName);
491+
492+
if (result.Level != AuthorizationLevel.Anonymous &&
493+
_strictHISFeatureEnabled.Value && _invalidNonHISKeys.Contains(keyValue))
494+
{
495+
// HIS strict mode is enabled and the matched key is invalid
496+
// Such keys cannot grant access.
497+
return (null, AuthorizationLevel.Anonymous);
498+
}
499+
500+
return result;
501+
}
502+
422503
if (keyValue != null)
423504
{
424505
string cacheKey = $"{keyValue}{functionName}";
@@ -432,7 +513,7 @@ private async Task<ScriptSecrets> LoadSecretsAsync(ScriptSecretsType type, strin
432513
// cause the secrets to be loaded into cache - we want to know if they were cached BEFORE this check.
433514
bool secretsCached = _hostSecrets != null || _functionSecrets.Any();
434515

435-
var result = await GetAuthorizationLevelAsync(this, keyValue, functionName);
516+
var result = await GetAuthorizationLevelAndValidateAsync(keyValue, functionName);
436517
if (result.Level != AuthorizationLevel.Anonymous)
437518
{
438519
// key match
@@ -448,9 +529,10 @@ private async Task<ScriptSecrets> LoadSecretsAsync(ScriptSecretsType type, strin
448529
{
449530
_hostSecrets = null;
450531
_functionSecrets.Clear();
532+
_invalidNonHISKeys.Clear();
451533
_lastCacheResetTime = DateTime.UtcNow;
452534

453-
return await GetAuthorizationLevelAsync(this, keyValue, functionName);
535+
return await GetAuthorizationLevelAndValidateAsync(keyValue, functionName);
454536
}
455537
}
456538
}
@@ -514,6 +596,47 @@ private static ScriptSecretsType GetSecretsType<T>() where T : ScriptSecrets
514596
: ScriptSecretsType.Function;
515597
}
516598

599+
private void ValidateHostSecrets(string masterKey, IDictionary<string, string> functionKeys, IDictionary<string, string> systemKeys)
600+
{
601+
ValidateSecret(ScriptConstants.MasterKeyName, masterKey, SecretGenerator.MasterKeySeed);
602+
ValidateSecrets(functionKeys, SecretGenerator.FunctionKeySeed);
603+
ValidateSecrets(systemKeys, SecretGenerator.SystemKeySeed);
604+
}
605+
606+
private void ValidateSecret(string keyName, string keyValue, ulong seed, string functionName = null)
607+
{
608+
if (SecretGenerator.TryValidateSecret(keyValue, seed))
609+
{
610+
_metricsLogger.LogEvent(MetricEventNames.IdentifiableSecretLoaded, functionName);
611+
}
612+
else
613+
{
614+
_metricsLogger.LogEvent(MetricEventNames.NonIdentifiableSecretLoaded, functionName);
615+
616+
if (_strictHISFeatureEnabled.Value || _strictHISWarnFeatureEnabled.Value)
617+
{
618+
// if either HIS mode is enabled log the appropriate diagnostic event
619+
// and track the secret as invalid
620+
string message = string.Format(Resources.NonHISSecret, GetKeyType(seed), keyName, functionName);
621+
LogLevel level = _strictHISFeatureEnabled.Value ? LogLevel.Error : LogLevel.Warning;
622+
_logger.LogDiagnosticEvent(level, 0, DiagnosticEventConstants.NonHISSecretLoaded, message, DiagnosticEventConstants.NonHISSecretLoadedHelpLink, exception: null);
623+
624+
_invalidNonHISKeys.Add(keyValue);
625+
}
626+
}
627+
}
628+
629+
private void ValidateSecrets(IDictionary<string, string> keys, ulong seed, string functionName = null)
630+
{
631+
if (keys != null)
632+
{
633+
foreach (var key in keys)
634+
{
635+
ValidateSecret(key.Key, key.Value, seed, functionName);
636+
}
637+
}
638+
}
639+
517640
private HostSecrets GenerateHostSecrets()
518641
{
519642
return new HostSecrets
@@ -661,6 +784,7 @@ private void ClearCacheOnChange(ScriptSecretsType secretsType, string functionNa
661784
// clear the cached secrets if they exist
662785
// they'll be reloaded on demand next time
663786
_authorizationCache.Clear();
787+
664788
if (secretsType == ScriptSecretsType.Host && _hostSecrets != null)
665789
{
666790
_logger.LogInformation("Host keys change detected. Clearing cache.");
@@ -686,6 +810,7 @@ public void ClearCache()
686810
_authorizationCache.Clear();
687811
_hostSecrets = null;
688812
_functionSecrets.Clear();
813+
_invalidNonHISKeys.Clear();
689814
}
690815

691816
private async Task<string> AnalyzeSnapshots(string[] secretBackups)
@@ -745,11 +870,26 @@ private string GetFunctionName(string keyScope, ScriptSecretsType secretsType)
745870
private void InitializeCache()
746871
{
747872
var cachedFunctionSecrets = _startupContextProvider.GetFunctionSecretsOrNull();
748-
_functionSecrets = cachedFunctionSecrets != null ?
749-
new ConcurrentDictionary<string, IDictionary<string, string>>(cachedFunctionSecrets, StringComparer.OrdinalIgnoreCase) :
750-
new ConcurrentDictionary<string, IDictionary<string, string>>(StringComparer.OrdinalIgnoreCase);
873+
if (cachedFunctionSecrets != null)
874+
{
875+
foreach (var functionKeys in cachedFunctionSecrets)
876+
{
877+
ValidateSecrets(functionKeys.Value, SecretGenerator.FunctionKeySeed, functionKeys.Key);
878+
}
751879

752-
_hostSecrets = _startupContextProvider.GetHostSecretsOrNull();
880+
_functionSecrets = new ConcurrentDictionary<string, IDictionary<string, string>>(cachedFunctionSecrets, StringComparer.OrdinalIgnoreCase);
881+
}
882+
else
883+
{
884+
_functionSecrets = new ConcurrentDictionary<string, IDictionary<string, string>>(StringComparer.OrdinalIgnoreCase);
885+
}
886+
887+
var hostSecrets = _startupContextProvider.GetHostSecretsOrNull();
888+
if (hostSecrets != null)
889+
{
890+
ValidateHostSecrets(hostSecrets.MasterKey, hostSecrets.FunctionKeys, hostSecrets.SystemKeys);
891+
_hostSecrets = hostSecrets;
892+
}
753893
}
754894

755895
private string GetEncryptionKeysHashes()

0 commit comments

Comments
 (0)