diff --git a/src/DataProtection/DataProtection/src/KeyManagement/Internal/CacheableKeyRing.cs b/src/DataProtection/DataProtection/src/KeyManagement/Internal/CacheableKeyRing.cs index c6e551fa7aff..9913d5735cfd 100644 --- a/src/DataProtection/DataProtection/src/KeyManagement/Internal/CacheableKeyRing.cs +++ b/src/DataProtection/DataProtection/src/KeyManagement/Internal/CacheableKeyRing.cs @@ -21,16 +21,24 @@ internal CacheableKeyRing(CancellationToken expirationToken, DateTimeOffset expi } internal CacheableKeyRing(CancellationToken expirationToken, DateTimeOffset expirationTime, IKeyRing keyRing) + : this(expirationToken, expirationTime, hasShortRefreshPeriod: false, keyRing) + { + } + + internal CacheableKeyRing(CancellationToken expirationToken, DateTimeOffset expirationTime, bool hasShortRefreshPeriod, IKeyRing keyRing) { _expirationToken = expirationToken; ExpirationTimeUtc = expirationTime.UtcDateTime; KeyRing = keyRing; + HasShortRefreshPeriod = hasShortRefreshPeriod; } internal DateTime ExpirationTimeUtc { get; } internal IKeyRing KeyRing { get; } + internal bool HasShortRefreshPeriod { get; } + internal static bool IsValid([NotNullWhen(true)] CacheableKeyRing? keyRing, DateTime utcNow) { return keyRing != null @@ -46,6 +54,6 @@ internal static bool IsValid([NotNullWhen(true)] CacheableKeyRing? keyRing, Date internal CacheableKeyRing WithTemporaryExtendedLifetime(DateTimeOffset now) { var extension = TimeSpan.FromMinutes(2); - return new CacheableKeyRing(CancellationToken.None, now + extension, KeyRing); + return new CacheableKeyRing(CancellationToken.None, now + extension, HasShortRefreshPeriod, KeyRing); } } diff --git a/src/DataProtection/DataProtection/src/KeyManagement/Internal/ICacheableKeyRingProvider.cs b/src/DataProtection/DataProtection/src/KeyManagement/Internal/ICacheableKeyRingProvider.cs index 59532404586c..585610f4fa03 100644 --- a/src/DataProtection/DataProtection/src/KeyManagement/Internal/ICacheableKeyRingProvider.cs +++ b/src/DataProtection/DataProtection/src/KeyManagement/Internal/ICacheableKeyRingProvider.cs @@ -10,6 +10,13 @@ namespace Microsoft.AspNetCore.DataProtection.KeyManagement.Internal; /// This API supports infrastructure and is not intended to be used /// directly from your code. This API may change or be removed in future releases. /// +/// +/// This interface is retained for the benefit of consumers who were casting +/// (also pseudo-internal) to so that they could invoke +/// . That method returns an object with no public properties and doesn't +/// update the state of the , but it does trigger calls to the backing +/// , which may have observable effects. It is no longer used as a test hook. +/// public interface ICacheableKeyRingProvider { /// diff --git a/src/DataProtection/DataProtection/src/KeyManagement/Internal/IInternalCacheableKeyRingProvider.cs b/src/DataProtection/DataProtection/src/KeyManagement/Internal/IInternalCacheableKeyRingProvider.cs new file mode 100644 index 000000000000..13920c731eec --- /dev/null +++ b/src/DataProtection/DataProtection/src/KeyManagement/Internal/IInternalCacheableKeyRingProvider.cs @@ -0,0 +1,24 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Diagnostics.CodeAnalysis; + +namespace Microsoft.AspNetCore.DataProtection.KeyManagement.Internal; + +/// +/// This API supports infrastructure and is not intended to be used +/// directly from your code. This API may change or be removed in future releases. +/// +/// +/// Replaces as a test hook for validating +/// . +/// +internal interface IInternalCacheableKeyRingProvider +{ + /// + /// This API supports infrastructure and is not intended to be used + /// directly from your code. This API may change or be removed in future releases. + /// + CacheableKeyRing GetCacheableKeyRing(DateTimeOffset now, bool allowShortRefreshPeriod = true); +} diff --git a/src/DataProtection/DataProtection/src/KeyManagement/KeyManagementOptions.cs b/src/DataProtection/DataProtection/src/KeyManagement/KeyManagementOptions.cs index 0806ee2df9d7..72f2bfe1a868 100644 --- a/src/DataProtection/DataProtection/src/KeyManagement/KeyManagementOptions.cs +++ b/src/DataProtection/DataProtection/src/KeyManagement/KeyManagementOptions.cs @@ -76,6 +76,20 @@ internal static TimeSpan KeyRingRefreshPeriod } } + /// + /// If a key is required and none is available, one will be generated and immediately + /// activated. Immediately-activated keys are problematic because they may be used + /// before they have propagated to other app instances. What's more, in a scenario + /// where one instance needs to generate an immediately-activated key, it's likely that + /// other instances will need to do the same. A given instance has no control over when + /// other instances pick up its immediately-activated keys, but it can do its best to + /// pick up theirs. Therefore, every time it generates an immediately-activated key, + /// it assumes other instances may have as well and pre-emptively schedules a refresh + /// of its own key ring cache. This property controls how long after an immediately- + /// activated key is generated the key ring cache will be refreshed. + /// + internal static TimeSpan ShortKeyRingRefreshPeriod { get; set; } = TimeSpan.FromSeconds(15); + /// /// Specifies the maximum clock skew allowed between servers when reading /// keys from the key ring. The key ring may use a key which has not yet diff --git a/src/DataProtection/DataProtection/src/KeyManagement/KeyRingProvider.cs b/src/DataProtection/DataProtection/src/KeyManagement/KeyRingProvider.cs index b2b704a27bb4..b0406976a17e 100644 --- a/src/DataProtection/DataProtection/src/KeyManagement/KeyRingProvider.cs +++ b/src/DataProtection/DataProtection/src/KeyManagement/KeyRingProvider.cs @@ -14,7 +14,7 @@ namespace Microsoft.AspNetCore.DataProtection.KeyManagement; -internal sealed class KeyRingProvider : ICacheableKeyRingProvider, IKeyRingProvider +internal sealed class KeyRingProvider : ICacheableKeyRingProvider, IInternalCacheableKeyRingProvider, IKeyRingProvider { private CacheableKeyRing? _cacheableKeyRing; private readonly object _cacheableKeyRingLockObj = new object(); @@ -55,13 +55,16 @@ public KeyRingProvider( } // for testing - internal ICacheableKeyRingProvider CacheableKeyRingProvider { get; set; } + internal CacheableKeyRing? CacheableKeyRing => Volatile.Read(ref _cacheableKeyRing); + + // for testing + internal IInternalCacheableKeyRingProvider CacheableKeyRingProvider { get; set; } internal DateTime AutoRefreshWindowEnd { get; set; } internal bool InAutoRefreshWindow() => DateTime.UtcNow < AutoRefreshWindowEnd; - private CacheableKeyRing CreateCacheableKeyRingCore(DateTimeOffset now, IKey? keyJustAdded) + private CacheableKeyRing CreateCacheableKeyRingCore(DateTimeOffset now, bool allowShortRefreshPeriod, IKey? keyJustAdded) { // Refresh the list of all keys var cacheExpirationToken = _keyManager.GetCacheExpirationToken(); @@ -80,7 +83,7 @@ private CacheableKeyRing CreateCacheableKeyRingCore(DateTimeOffset now, IKey? ke if (keyJustAdded != null) { var keyToUse = defaultKey ?? defaultKeyPolicy.FallbackKey ?? keyJustAdded; - return CreateCacheableKeyRingCoreStep2(now, cacheExpirationToken, keyToUse, allKeys); + return CreateCacheableKeyRingCoreStep2(now, cacheExpirationToken, keyToUse, keyJustAdded, allowShortRefreshPeriod, allKeys); } // Determine whether we need to generate a new key @@ -107,7 +110,7 @@ private CacheableKeyRing CreateCacheableKeyRingCore(DateTimeOffset now, IKey? ke if (!shouldGenerateNewKey) { CryptoUtil.Assert(defaultKey != null, "Expected to see a default key."); - return CreateCacheableKeyRingCoreStep2(now, cacheExpirationToken, defaultKey, allKeys); + return CreateCacheableKeyRingCoreStep2(now, cacheExpirationToken, defaultKey, keyJustAdded, allowShortRefreshPeriod, allKeys); } _logger.PolicyResolutionStatesThatANewKeyShouldBeAddedToTheKeyRing(); @@ -127,7 +130,7 @@ private CacheableKeyRing CreateCacheableKeyRingCore(DateTimeOffset now, IKey? ke else { _logger.UsingFallbackKeyWithExpirationAsDefaultKey(keyToUse.KeyId, keyToUse.ExpirationDate); - return CreateCacheableKeyRingCoreStep2(now, cacheExpirationToken, keyToUse, allKeys); + return CreateCacheableKeyRingCoreStep2(now, cacheExpirationToken, keyToUse, keyJustAdded, allowShortRefreshPeriod, allKeys); } } @@ -139,7 +142,7 @@ private CacheableKeyRing CreateCacheableKeyRingCore(DateTimeOffset now, IKey? ke // The case where there's no default key is the easiest scenario, since it // means that we need to create a new key with immediate activation. var newKey = _keyManager.CreateNewKey(activationDate: now, expirationDate: now + _newKeyLifetime); - return CreateCacheableKeyRingCore(now, keyJustAdded: newKey); // recursively call + return CreateCacheableKeyRingCore(now, allowShortRefreshPeriod, keyJustAdded: newKey); // recursively call } else { @@ -147,11 +150,11 @@ private CacheableKeyRing CreateCacheableKeyRingCore(DateTimeOffset now, IKey? ke // expiration of the default key. The new key lifetime is measured from the creation // date (now), not the activation date. var newKey = _keyManager.CreateNewKey(activationDate: defaultKey.ExpirationDate, expirationDate: now + _newKeyLifetime); - return CreateCacheableKeyRingCore(now, keyJustAdded: newKey); // recursively call + return CreateCacheableKeyRingCore(now, allowShortRefreshPeriod, keyJustAdded: newKey); // recursively call } } - private CacheableKeyRing CreateCacheableKeyRingCoreStep2(DateTimeOffset now, CancellationToken cacheExpirationToken, IKey defaultKey, IEnumerable allKeys) + private CacheableKeyRing CreateCacheableKeyRingCoreStep2(DateTimeOffset now, CancellationToken cacheExpirationToken, IKey defaultKey, IKey? generatedKey, bool allowShortRefreshPeriod, IEnumerable allKeys) { Debug.Assert(defaultKey != null); @@ -167,7 +170,25 @@ private CacheableKeyRing CreateCacheableKeyRingCoreStep2(DateTimeOffset now, Can _logger.UsingKeyAsDefaultKey(defaultKey.KeyId); - var nextAutoRefreshTime = now + GetRefreshPeriodWithJitter(KeyManagementOptions.KeyRingRefreshPeriod); + // If we're in a scenario where we think other app instances may be generating keys that will be + // activated before they are propagated, we'll use a shorter refresh period to ensure that we pick + // up those keys so that we can select from the same pool of candidates as the other instances. + // + // These not-yet-propagated keys are usually immediately-activated, but an app restarted after a period + // of downtime may discover that it has a valid, but soon-to-be-expired key. The replacement will not + // be immediately-activated, but may be activated before it has propagated. + var useShortRefreshPeriod = allowShortRefreshPeriod && + (generatedKey is not null + // Either we had to generate a key and it will be activated before it has time to propagate + // (in which case, other instances may have done the same) + ? (generatedKey.ActivationDate < now + KeyManagementOptions.KeyPropagationWindow) // No clock skew on a key we generated + // Or we selected a key that has yet to propagate (presumably, from another instance) + : (defaultKey.CreationDate > now - KeyManagementOptions.KeyPropagationWindow)); + + var nextAutoRefreshTime = now + GetRefreshPeriodWithJitter( + useShortRefreshPeriod + ? KeyManagementOptions.ShortKeyRingRefreshPeriod + : KeyManagementOptions.KeyRingRefreshPeriod); // The cached keyring should expire at the earliest of (default key expiration, next auto-refresh time). // Since the refresh period and safety window are not user-settable, we can guarantee that there's at @@ -179,8 +200,8 @@ private CacheableKeyRing CreateCacheableKeyRingCoreStep2(DateTimeOffset now, Can return new CacheableKeyRing( expirationToken: cacheExpirationToken, expirationTime: (defaultKey.ExpirationDate <= now) ? nextAutoRefreshTime : Min(defaultKey.ExpirationDate, nextAutoRefreshTime), - defaultKey: defaultKey, - allKeys: allKeys); + hasShortRefreshPeriod: useShortRefreshPeriod, + keyRing: new KeyRing(defaultKey, allKeys)); } public IKeyRing GetCurrentKeyRing() @@ -241,7 +262,7 @@ internal IKeyRing GetCurrentKeyRingCore(DateTime utcNow, bool forceRefresh = fal try { - newCacheableKeyRing = CacheableKeyRingProvider.GetCacheableKeyRing(utcNow); + newCacheableKeyRing = CacheableKeyRingProvider.GetCacheableKeyRing(utcNow, allowShortRefreshPeriod: existingCacheableKeyRing?.HasShortRefreshPeriod != true); } catch (Exception ex) { @@ -311,9 +332,18 @@ private static DateTimeOffset Min(DateTimeOffset a, DateTimeOffset b) return (a < b) ? a : b; } + // This is only here to support external consumers of our internal test hook. + // The object returned has no public properties, so we could probably return a dummy + // without doing any work, but maybe they're calling it for side-effects? CacheableKeyRing ICacheableKeyRingProvider.GetCacheableKeyRing(DateTimeOffset now) { // the entry point allows one recursive call - return CreateCacheableKeyRingCore(now, keyJustAdded: null); + return CreateCacheableKeyRingCore(now, allowShortRefreshPeriod: false, keyJustAdded: null); + } + + CacheableKeyRing IInternalCacheableKeyRingProvider.GetCacheableKeyRing(DateTimeOffset now, bool allowShortRefreshPeriod) + { + // the entry point allows one recursive call + return CreateCacheableKeyRingCore(now, allowShortRefreshPeriod, keyJustAdded: null); } } diff --git a/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/KeyManagement/KeyRingBasedDataProtectorTests.cs b/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/KeyManagement/KeyRingBasedDataProtectorTests.cs index f955c9e27f41..f3eebdab4ece 100644 --- a/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/KeyManagement/KeyRingBasedDataProtectorTests.cs +++ b/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/KeyManagement/KeyRingBasedDataProtectorTests.cs @@ -226,7 +226,7 @@ private static DateTime StringToDateTime(string input) return DateTimeOffset.ParseExact(input, "u", CultureInfo.InvariantCulture).UtcDateTime; } - private static KeyRingProvider CreateKeyRingProvider(ICacheableKeyRingProvider cacheableKeyRingProvider) + private static KeyRingProvider CreateKeyRingProvider(IInternalCacheableKeyRingProvider cacheableKeyRingProvider) { var mockEncryptorFactory = new Mock(); mockEncryptorFactory.Setup(m => m.CreateEncryptorInstance(It.IsAny())).Returns(new Mock().Object); @@ -345,16 +345,16 @@ public void Unprotect_KeyNotFound_RefreshOnce_CanFindKey() Assert.Empty(result); } - private class TestKeyRingProvider : ICacheableKeyRingProvider + private class TestKeyRingProvider : IInternalCacheableKeyRingProvider { private CacheableKeyRing _keyRing; public TestKeyRingProvider(CacheableKeyRing keys) => _keyRing = keys; - public CacheableKeyRing GetCacheableKeyRing(DateTimeOffset now) => _keyRing; + public CacheableKeyRing GetCacheableKeyRing(DateTimeOffset now, bool _allowShortRefreshPeriod) => _keyRing; } - private class RefreshTestKeyRingProvider : ICacheableKeyRingProvider + private class RefreshTestKeyRingProvider : IInternalCacheableKeyRingProvider { private CacheableKeyRing _keyRing; private CacheableKeyRing _refreshKeyRing; @@ -366,7 +366,7 @@ public RefreshTestKeyRingProvider(CacheableKeyRing keys, CacheableKeyRing refres _refreshKeyRing = refreshKeys; } - public CacheableKeyRing GetCacheableKeyRing(DateTimeOffset now) + public CacheableKeyRing GetCacheableKeyRing(DateTimeOffset now, bool _allowShortRefreshPeriod) { if (!_called) { diff --git a/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/KeyManagement/KeyRingProviderTests.cs b/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/KeyManagement/KeyRingProviderTests.cs index 243bdcd443af..94dee450e474 100644 --- a/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/KeyManagement/KeyRingProviderTests.cs +++ b/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/KeyManagement/KeyRingProviderTests.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Concurrent; using System.Globalization; using Microsoft.AspNetCore.DataProtection.AuthenticatedEncryption; using Microsoft.AspNetCore.DataProtection.AuthenticatedEncryption.ConfigurationModel; @@ -225,7 +226,7 @@ public void CreateCacheableKeyRing_GenerationRequired_NoDefaultKey_CreatesNewKey // Assert Assert.Equal(newlyCreatedKey.KeyId, cacheableKeyRing.KeyRing.DefaultKeyId); - AssertWithinJitterRange(cacheableKeyRing.ExpirationTimeUtc, now); + AssertWithinJitterRange(cacheableKeyRing.ExpirationTimeUtc, now, isImmediatelyActivated: true); Assert.True(CacheableKeyRing.IsValid(cacheableKeyRing, now)); expirationCts1.Cancel(); Assert.True(CacheableKeyRing.IsValid(cacheableKeyRing, now)); @@ -575,9 +576,9 @@ public void GetCurrentKeyRing_NoKeyRingCached_CachesAndReturns() // Arrange var now = StringToDateTime("2015-03-01 00:00:00Z"); var expectedKeyRing = new Mock().Object; - var mockCacheableKeyRingProvider = new Mock(); + var mockCacheableKeyRingProvider = new Mock(); mockCacheableKeyRingProvider - .Setup(o => o.GetCacheableKeyRing(now)) + .Setup(o => o.GetCacheableKeyRing(now, true)) .Returns(new CacheableKeyRing( expirationToken: CancellationToken.None, expirationTime: StringToDateTime("2015-03-02 00:00:00Z"), @@ -592,7 +593,7 @@ public void GetCurrentKeyRing_NoKeyRingCached_CachesAndReturns() // Assert - underlying provider only should have been called once Assert.Same(expectedKeyRing, retVal1); Assert.Same(expectedKeyRing, retVal2); - mockCacheableKeyRingProvider.Verify(o => o.GetCacheableKeyRing(It.IsAny()), Times.Once); + mockCacheableKeyRingProvider.Verify(o => o.GetCacheableKeyRing(It.IsAny(), It.IsAny()), Times.Once); } [Fact] @@ -602,21 +603,21 @@ public void GetCurrentKeyRing_KeyRingCached_CanForceRefresh() var now = StringToDateTime("2015-03-01 00:00:00Z"); var expectedKeyRing1 = new Mock().Object; var expectedKeyRing2 = new Mock().Object; - var mockCacheableKeyRingProvider = new Mock(); + var mockCacheableKeyRingProvider = new Mock(); mockCacheableKeyRingProvider - .Setup(o => o.GetCacheableKeyRing(now)) + .Setup(o => o.GetCacheableKeyRing(now, true)) .Returns(new CacheableKeyRing( expirationToken: CancellationToken.None, expirationTime: StringToDateTime("2015-03-01 00:30:00Z"), // expire in half an hour keyRing: expectedKeyRing1)); mockCacheableKeyRingProvider - .Setup(o => o.GetCacheableKeyRing(now + TimeSpan.FromMinutes(1))) + .Setup(o => o.GetCacheableKeyRing(now + TimeSpan.FromMinutes(1), true)) .Returns(new CacheableKeyRing( expirationToken: CancellationToken.None, expirationTime: StringToDateTime("2015-03-01 00:30:00Z"), // expire in half an hour keyRing: expectedKeyRing1)); mockCacheableKeyRingProvider - .Setup(o => o.GetCacheableKeyRing(now + TimeSpan.FromMinutes(2))) + .Setup(o => o.GetCacheableKeyRing(now + TimeSpan.FromMinutes(2), true)) .Returns(new CacheableKeyRing( expirationToken: CancellationToken.None, expirationTime: StringToDateTime("2015-03-02 00:00:00Z"), @@ -633,7 +634,7 @@ public void GetCurrentKeyRing_KeyRingCached_CanForceRefresh() Assert.Same(expectedKeyRing1, retVal1); Assert.Same(expectedKeyRing1, retVal2); Assert.Same(expectedKeyRing2, retVal3); - mockCacheableKeyRingProvider.Verify(o => o.GetCacheableKeyRing(It.IsAny()), Times.Exactly(2)); + mockCacheableKeyRingProvider.Verify(o => o.GetCacheableKeyRing(It.IsAny(), It.IsAny()), Times.Exactly(2)); } [Fact] @@ -643,15 +644,15 @@ public void GetCurrentKeyRing_KeyRingCached_AfterExpiration_ClearsCache() var now = StringToDateTime("2015-03-01 00:00:00Z"); var expectedKeyRing1 = new Mock().Object; var expectedKeyRing2 = new Mock().Object; - var mockCacheableKeyRingProvider = new Mock(); + var mockCacheableKeyRingProvider = new Mock(); mockCacheableKeyRingProvider - .Setup(o => o.GetCacheableKeyRing(now)) + .Setup(o => o.GetCacheableKeyRing(now, true)) .Returns(new CacheableKeyRing( expirationToken: CancellationToken.None, expirationTime: StringToDateTime("2015-03-01 00:30:00Z"), // expire in half an hour keyRing: expectedKeyRing1)); mockCacheableKeyRingProvider - .Setup(o => o.GetCacheableKeyRing(now + TimeSpan.FromHours(1))) + .Setup(o => o.GetCacheableKeyRing(now + TimeSpan.FromHours(1), true)) .Returns(new CacheableKeyRing( expirationToken: CancellationToken.None, expirationTime: StringToDateTime("2015-03-02 00:00:00Z"), @@ -666,7 +667,7 @@ public void GetCurrentKeyRing_KeyRingCached_AfterExpiration_ClearsCache() // Assert - underlying provider only should have been called once Assert.Same(expectedKeyRing1, retVal1); Assert.Same(expectedKeyRing2, retVal2); - mockCacheableKeyRingProvider.Verify(o => o.GetCacheableKeyRing(It.IsAny()), Times.Exactly(2)); + mockCacheableKeyRingProvider.Verify(o => o.GetCacheableKeyRing(It.IsAny(), It.IsAny()), Times.Exactly(2)); } [Fact] @@ -675,7 +676,7 @@ public void GetCurrentKeyRing_NoExistingKeyRing_HoldsAllThreadsUntilKeyRingCreat // Arrange var now = StringToDateTime("2015-03-01 00:00:00Z"); var expectedKeyRing = new Mock().Object; - var mockCacheableKeyRingProvider = new Mock(); + var mockCacheableKeyRingProvider = new Mock(); var keyRingProvider = CreateKeyRingProvider(mockCacheableKeyRingProvider.Object); // This test spawns a background thread which calls GetCurrentKeyRing then waits @@ -691,7 +692,7 @@ public void GetCurrentKeyRing_NoExistingKeyRing_HoldsAllThreadsUntilKeyRingCreat var backgroundGetKeyRingTask = Task.Run(() => { mockCacheableKeyRingProvider - .Setup(o => o.GetCacheableKeyRing(now)) + .Setup(o => o.GetCacheableKeyRing(now, true)) .Returns(() => { mreBackgroundThreadHasCalledGetCurrentKeyRing.Set(); @@ -715,7 +716,7 @@ public void GetCurrentKeyRing_NoExistingKeyRing_HoldsAllThreadsUntilKeyRingCreat // Assert - underlying provider only should have been called once Assert.Same(expectedKeyRing, foregroundRetVal); Assert.Same(expectedKeyRing, backgroundRetVal); - mockCacheableKeyRingProvider.Verify(o => o.GetCacheableKeyRing(It.IsAny()), Times.Once); + mockCacheableKeyRingProvider.Verify(o => o.GetCacheableKeyRing(It.IsAny(), It.IsAny()), Times.Once); } [Fact] @@ -726,7 +727,7 @@ public void GetCurrentKeyRing_WithExpiredExistingKeyRing_AllowsOneThreadToUpdate var originalKeyRingTime = StringToDateTime("2015-03-01 00:00:00Z"); var updatedKeyRing = new Mock().Object; var updatedKeyRingTime = StringToDateTime("2015-03-02 00:00:00Z"); - var mockCacheableKeyRingProvider = new Mock(); + var mockCacheableKeyRingProvider = new Mock(); var keyRingProvider = CreateKeyRingProvider(mockCacheableKeyRingProvider.Object); // In this test, the foreground thread acquires the critial section in GetCurrentKeyRing, @@ -736,10 +737,10 @@ public void GetCurrentKeyRing_WithExpiredExistingKeyRing_AllowsOneThreadToUpdate TimeSpan testTimeout = TimeSpan.FromSeconds(10); IKeyRing keyRingReturnedToBackgroundThread = null; - mockCacheableKeyRingProvider.Setup(o => o.GetCacheableKeyRing(originalKeyRingTime)) + mockCacheableKeyRingProvider.Setup(o => o.GetCacheableKeyRing(originalKeyRingTime, true)) .Returns(new CacheableKeyRing(CancellationToken.None, StringToDateTime("2015-03-02 00:00:00Z"), originalKeyRing)); - mockCacheableKeyRingProvider.Setup(o => o.GetCacheableKeyRing(updatedKeyRingTime)) - .Returns(dto => + mockCacheableKeyRingProvider.Setup(o => o.GetCacheableKeyRing(updatedKeyRingTime, true)) + .Returns((DateTimeOffset dto, bool allowShortRefreshPeriod) => { // at this point we're inside the critical section - spawn the background thread now var backgroundGetKeyRingTask = Task.Run(() => @@ -755,7 +756,7 @@ public void GetCurrentKeyRing_WithExpiredExistingKeyRing_AllowsOneThreadToUpdate Assert.Same(originalKeyRing, keyRingProvider.GetCurrentKeyRingCore(originalKeyRingTime)); Assert.Same(updatedKeyRing, keyRingProvider.GetCurrentKeyRingCore(updatedKeyRingTime)); Assert.Same(originalKeyRing, keyRingReturnedToBackgroundThread); - mockCacheableKeyRingProvider.Verify(o => o.GetCacheableKeyRing(updatedKeyRingTime), Times.Once); + mockCacheableKeyRingProvider.Verify(o => o.GetCacheableKeyRing(updatedKeyRingTime, It.IsAny()), Times.Once); } [Fact] @@ -763,16 +764,16 @@ public void GetCurrentKeyRing_WithExpiredExistingKeyRing_UpdateFails_ThrowsButCa { // Arrange var cts = new CancellationTokenSource(); - var mockCacheableKeyRingProvider = new Mock(); + var mockCacheableKeyRingProvider = new Mock(); var originalKeyRing = new Mock().Object; var originalKeyRingTime = StringToDateTime("2015-03-01 00:00:00Z"); - mockCacheableKeyRingProvider.Setup(o => o.GetCacheableKeyRing(originalKeyRingTime)) + mockCacheableKeyRingProvider.Setup(o => o.GetCacheableKeyRing(originalKeyRingTime, true)) .Returns(new CacheableKeyRing(cts.Token, StringToDateTime("2015-03-02 00:00:00Z"), originalKeyRing)); var throwKeyRingTime = StringToDateTime("2015-03-01 12:00:00Z"); - mockCacheableKeyRingProvider.Setup(o => o.GetCacheableKeyRing(throwKeyRingTime)).Throws(new Exception("How exceptional.")); + mockCacheableKeyRingProvider.Setup(o => o.GetCacheableKeyRing(throwKeyRingTime, true)).Throws(new Exception("How exceptional.")); var updatedKeyRing = new Mock().Object; var updatedKeyRingTime = StringToDateTime("2015-03-01 12:02:00Z"); - mockCacheableKeyRingProvider.Setup(o => o.GetCacheableKeyRing(updatedKeyRingTime)) + mockCacheableKeyRingProvider.Setup(o => o.GetCacheableKeyRing(updatedKeyRingTime, true)) .Returns(new CacheableKeyRing(CancellationToken.None, StringToDateTime("2015-03-02 00:00:00Z"), updatedKeyRing)); var keyRingProvider = CreateKeyRingProvider(mockCacheableKeyRingProvider.Object); @@ -782,12 +783,193 @@ public void GetCurrentKeyRing_WithExpiredExistingKeyRing_UpdateFails_ThrowsButCa ExceptionAssert.Throws(() => keyRingProvider.GetCurrentKeyRingCore(throwKeyRingTime), "How exceptional."); Assert.Same(originalKeyRing, keyRingProvider.GetCurrentKeyRingCore(throwKeyRingTime)); Assert.Same(updatedKeyRing, keyRingProvider.GetCurrentKeyRingCore(updatedKeyRingTime)); - mockCacheableKeyRingProvider.Verify(o => o.GetCacheableKeyRing(originalKeyRingTime), Times.Once); - mockCacheableKeyRingProvider.Verify(o => o.GetCacheableKeyRing(throwKeyRingTime), Times.Once); - mockCacheableKeyRingProvider.Verify(o => o.GetCacheableKeyRing(updatedKeyRingTime), Times.Once); + mockCacheableKeyRingProvider.Verify(o => o.GetCacheableKeyRing(originalKeyRingTime, It.IsAny()), Times.Once); + mockCacheableKeyRingProvider.Verify(o => o.GetCacheableKeyRing(throwKeyRingTime, It.IsAny()), Times.Once); + mockCacheableKeyRingProvider.Verify(o => o.GetCacheableKeyRing(updatedKeyRingTime, It.IsAny()), Times.Once); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void RefreshWhenDefaultKeyIsNearExpiration_KeyGenerated(bool hasPropagated) + { + // This test validates that a short refresh can be used in cases where the default key is not immediately-activated + + var now = StringToDateTime("2015-03-01 00:00:00Z"); + var maxRefreshTime = now + (hasPropagated ? KeyManagementOptions.KeyRingRefreshPeriod : KeyManagementOptions.ShortKeyRingRefreshPeriod); + + var key = CreateKey( + creationDate: now - KeyManagementOptions.KeyPropagationWindow + (hasPropagated ? -1 : +1) * TimeSpan.FromHours(1), // May have propagated + activationDate: now - TimeSpan.FromHours(1), // Has been activated + expirationDate: now + TimeSpan.FromHours(1)); // Will expire before a replacement can be propagated + + var keyManagementOptions = Options.Create(null); // Default options are fine + var keyManager = (IKeyManager)new InMemoryKeyManager(now); // Semi-realistic key manager + + var defaultKeyResolver = new Mock(MockBehavior.Strict); + defaultKeyResolver + .Setup(o => o.ResolveDefaultKeyPolicy(now, It.IsAny>())) + .Returns(new DefaultKeyResolution() + { + DefaultKey = key, + ShouldGenerateNewKey = false + }); + defaultKeyResolver + .Setup(o => o.ResolveDefaultKeyPolicy(key.ExpirationDate, It.IsAny>())) + .Returns(new DefaultKeyResolution() + { + ShouldGenerateNewKey = true + }); + + IInternalCacheableKeyRingProvider provider = new KeyRingProvider(keyManager, keyManagementOptions, defaultKeyResolver.Object); + var keyRing = provider.GetCacheableKeyRing(now); + Assert.InRange(keyRing.ExpirationTimeUtc, now, maxRefreshTime); // Actual range is based on jitter - this lower bound is loose + + // The generation of this new key, which won't have time to propagate before it's activated, is why we need to refresh + var newKey = Assert.Single(keyManager.GetAllKeys().Where(k => !ReferenceEquals(k, key))); + Assert.Equal(key.ExpirationDate, newKey.ActivationDate); + } + + [Fact] + public void RefreshWhenDefaultKeyIsNearExpiration_KeyReceived() + { + // This test validates that a short refresh can be used in cases where the default key is not immediately-activated + + var now = StringToDateTime("2015-03-01 00:00:00Z"); + var maxRefreshTime = now + KeyManagementOptions.ShortKeyRingRefreshPeriod; + + var keyManagementOptions = Options.Create(null); // Default options are fine + var keyManager = (IKeyManager)new InMemoryKeyManager(now); // Semi-realistic key manager + var key = keyManager.CreateNewKey(now, now + TimeSpan.FromDays(90)); // Pre-populated key + Assert.Equal(key.CreationDate, key.ActivationDate); // Immediately-activated + + var defaultKeyResolver = new Mock(MockBehavior.Strict); + defaultKeyResolver + .Setup(o => o.ResolveDefaultKeyPolicy(now, It.IsAny>())) + .Returns(new DefaultKeyResolution() + { + DefaultKey = key, + ShouldGenerateNewKey = false + }); + + IInternalCacheableKeyRingProvider provider = new KeyRingProvider(keyManager, keyManagementOptions, defaultKeyResolver.Object); + var keyRing = provider.GetCacheableKeyRing(now); + Assert.InRange(keyRing.ExpirationTimeUtc, now, maxRefreshTime); // Actual range is based on jitter - this lower bound is loose + + // No new key was generated + Assert.Single(keyManager.GetAllKeys()); } - private static ICacheableKeyRingProvider SetupCreateCacheableKeyRingTestAndCreateKeyManager( + [Fact] + public async Task MultipleInstancesGenerateImmediatelyActivatedKeys() + { + const int taskCount = 10; + var now = StringToDateTime("2015-03-01 00:00:00Z"); + var maxRefreshTime = now + KeyManagementOptions.ShortKeyRingRefreshPeriod; + + // Multiple instances wouldn't really share a key manager, but it's a reasonable simulation of shared storage + var keyManager = new InMemoryKeyManager(now); + var keyManagementOptions = new Mock>(); + var defaultKeyResolver = new DefaultKeyResolver(); + + var tasks1 = new Task>[taskCount]; + for (var i = 0; i < taskCount; i++) + { + tasks1[i] = Task.Run(() => + { + IInternalCacheableKeyRingProvider provider = new KeyRingProvider(keyManager, keyManagementOptions.Object, defaultKeyResolver); + var keyRing = provider.GetCacheableKeyRing(now); + return (provider, keyRing); + }); + } + var tuples1 = await Task.WhenAll(tasks1); + Assert.All(tuples1, tuple => Assert.InRange(tuple.Item2.ExpirationTimeUtc, now, maxRefreshTime)); // Actual range is based on jitter - this lower bound is loose + + var tasks2 = new Task>[taskCount]; + for (var t = 0; t < taskCount; t++) + { + var i = t; + tasks2[i] = Task.Run(() => + { + var (provider, keyRing) = tuples1[i]; + var newKeyRing = provider.GetCacheableKeyRing(keyRing.ExpirationTimeUtc, allowShortRefreshPeriod: !keyRing.HasShortRefreshPeriod); + return (provider, newKeyRing); + }); + } + var tuples2 = await Task.WhenAll(tasks2); + + var keyId = tuples2[0].Item2.KeyRing.DefaultKeyId; + Assert.All(tuples2, tuple => Assert.Equal(keyId, tuple.Item2.KeyRing.DefaultKeyId)); // They should all be the same + Assert.All(tuples2, tuple => Assert.InRange(tuple.Item2.ExpirationTimeUtc, now.AddHours(1), now.AddHours(25))); // This range is loose - just want it not to be a short refresh + } + + [Fact] + public void NoBackToBackShortRefreshPeriods() + { + var now = StringToDateTime("2015-03-01 00:00:00Z"); + var maxRefreshTime = now + KeyManagementOptions.ShortKeyRingRefreshPeriod; + + // Multiple instances wouldn't really share a key manager, but it's a reasonable simulation of shared storage + var keyManager = new InMemoryKeyManager(now); + var keyManagementOptions = new Mock>(); + var defaultKeyResolver = new DefaultKeyResolver(); + + var provider = new KeyRingProvider(keyManager, keyManagementOptions.Object, defaultKeyResolver); + provider.GetCurrentKeyRingCore(now); + var cacheableKeyRing1 = provider.CacheableKeyRing!; + Assert.True(cacheableKeyRing1.HasShortRefreshPeriod); // First key has to be immediately activated + + provider.GetCurrentKeyRingCore(cacheableKeyRing1.ExpirationTimeUtc); + var cacheableKeyRing2 = provider.CacheableKeyRing!; + Assert.Equal(cacheableKeyRing1.KeyRing.DefaultKeyId, cacheableKeyRing2.KeyRing.DefaultKeyId); // Default key hasn't changed and is still immediately-activated + Assert.False(cacheableKeyRing2.HasShortRefreshPeriod); // But we don't loop and use a short refresh period again + + // Descriptive. rather than normative - nothing prevents the following refresh period from being short again + provider.GetCurrentKeyRingCore(cacheableKeyRing2.ExpirationTimeUtc); + var cacheableKeyRing3 = provider.CacheableKeyRing!; + Assert.Equal(cacheableKeyRing1.KeyRing.DefaultKeyId, cacheableKeyRing3.KeyRing.DefaultKeyId); // Default key hasn't changed and is still immediately-activated + Assert.True(cacheableKeyRing3.HasShortRefreshPeriod); + } + + private sealed class InMemoryKeyManager : IKeyManager + { + private readonly ConcurrentBag _keys = new(); + private readonly DateTimeOffset _now; + + public InMemoryKeyManager(DateTimeOffset now) + { + _now = now; + } + + IKey IKeyManager.CreateNewKey(DateTimeOffset activationDate, DateTimeOffset expirationDate) + { + var newKey = CreateKey(creationDate: _now, activationDate, expirationDate); + _keys.Add(newKey); + return newKey; + } + + IReadOnlyCollection IKeyManager.GetAllKeys() + { + return _keys.ToArray(); + } + + CancellationToken IKeyManager.GetCacheExpirationToken() + { + return CancellationToken.None; // This is not a valid implementation, but it's good enough for testing + } + + void IKeyManager.RevokeAllKeys(DateTimeOffset revocationDate, string reason) + { + throw new NotImplementedException(); + } + + void IKeyManager.RevokeKey(Guid keyId, string reason) + { + throw new NotImplementedException(); + } + } + + private static IInternalCacheableKeyRingProvider SetupCreateCacheableKeyRingTestAndCreateKeyManager( IList callSequence, IEnumerable getCacheExpirationTokenReturnValues, IEnumerable> getAllKeysReturnValues, @@ -844,7 +1026,7 @@ private static ICacheableKeyRingProvider SetupCreateCacheableKeyRingTestAndCreat return CreateKeyRingProvider(mockKeyManager.Object, mockDefaultKeyResolver.Object, keyManagementOptions); } - private static KeyRingProvider CreateKeyRingProvider(ICacheableKeyRingProvider cacheableKeyRingProvider) + private static KeyRingProvider CreateKeyRingProvider(IInternalCacheableKeyRingProvider cacheableKeyRingProvider) { var mockEncryptorFactory = new Mock(); mockEncryptorFactory.Setup(m => m.CreateEncryptorInstance(It.IsAny())).Returns(new Mock().Object); @@ -861,7 +1043,7 @@ private static KeyRingProvider CreateKeyRingProvider(ICacheableKeyRingProvider c }; } - private static ICacheableKeyRingProvider CreateKeyRingProvider(IKeyManager keyManager, IDefaultKeyResolver defaultKeyResolver, KeyManagementOptions keyManagementOptions = null) + private static IInternalCacheableKeyRingProvider CreateKeyRingProvider(IKeyManager keyManager, IDefaultKeyResolver defaultKeyResolver, KeyManagementOptions keyManagementOptions = null) { var mockEncryptorFactory = new Mock(); mockEncryptorFactory.Setup(m => m.CreateEncryptorInstance(It.IsAny())).Returns(new Mock().Object); @@ -875,10 +1057,14 @@ private static ICacheableKeyRingProvider CreateKeyRingProvider(IKeyManager keyMa loggerFactory: NullLoggerFactory.Instance); } - private static void AssertWithinJitterRange(DateTimeOffset actual, DateTimeOffset now) + private static void AssertWithinJitterRange(DateTimeOffset actual, DateTimeOffset now, bool isImmediatelyActivated = false) { + var period = isImmediatelyActivated + ? KeyManagementOptions.ShortKeyRingRefreshPeriod + : KeyManagementOptions.KeyRingRefreshPeriod; + // The jitter can cause the actual value to fall in the range [now + 80% of refresh period, now + 100% of refresh period) - Assert.InRange(actual, now + TimeSpan.FromHours(24 * 0.8), now + TimeSpan.FromHours(24)); + Assert.InRange(actual, now + (period * 0.8), now + period); } private static DateTime StringToDateTime(string input) @@ -903,9 +1089,16 @@ private static IKey CreateKey(string activationDate, string expirationDate, bool } private static IKey CreateKey(DateTimeOffset activationDate, DateTimeOffset expirationDate, bool isRevoked = false) + { + // For tests that don't care about creation time, just assume the key has always existed + return CreateKey(creationDate: default, activationDate, expirationDate, isRevoked); + } + + private static IKey CreateKey(DateTimeOffset creationDate, DateTimeOffset activationDate, DateTimeOffset expirationDate, bool isRevoked = false) { var mockKey = new Mock(); mockKey.Setup(o => o.KeyId).Returns(Guid.NewGuid()); + mockKey.Setup(o => o.CreationDate).Returns(creationDate); mockKey.Setup(o => o.ActivationDate).Returns(activationDate); mockKey.Setup(o => o.ExpirationDate).Returns(expirationDate); mockKey.Setup(o => o.IsRevoked).Returns(isRevoked);