From d48ad7a971f2bb4b9ece3042156131fe2f856e72 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Tue, 9 Jul 2019 13:42:20 -0700 Subject: [PATCH] DataProtection: Refresh key rings for a short window (#11987) --- .../KeyRingBasedDataProtector.cs | 13 +- .../src/KeyManagement/KeyRingProvider.cs | 47 ++++-- .../KeyRingBasedDataProtectorTests.cs | 159 ++++++++++++++++++ .../KeyManagement/KeyRingProviderTests.cs | 41 +++++ 4 files changed, 244 insertions(+), 16 deletions(-) diff --git a/src/DataProtection/DataProtection/src/KeyManagement/KeyRingBasedDataProtector.cs b/src/DataProtection/DataProtection/src/KeyManagement/KeyRingBasedDataProtector.cs index e0157e66fe..d1857d76d2 100644 --- a/src/DataProtection/DataProtection/src/KeyManagement/KeyRingBasedDataProtector.cs +++ b/src/DataProtection/DataProtection/src/KeyManagement/KeyRingBasedDataProtector.cs @@ -240,8 +240,17 @@ namespace Microsoft.AspNetCore.DataProtection.KeyManagement var requestedEncryptor = currentKeyRing.GetAuthenticatedEncryptorByKeyId(keyIdFromPayload, out keyWasRevoked); if (requestedEncryptor == null) { - _logger.KeyWasNotFoundInTheKeyRingUnprotectOperationCannotProceed(keyIdFromPayload); - throw Error.Common_KeyNotFound(keyIdFromPayload); + if (_keyRingProvider is KeyRingProvider provider && provider.InAutoRefreshWindow()) + { + currentKeyRing = provider.RefreshCurrentKeyRing(); + requestedEncryptor = currentKeyRing.GetAuthenticatedEncryptorByKeyId(keyIdFromPayload, out keyWasRevoked); + } + + if (requestedEncryptor == null) + { + _logger.KeyWasNotFoundInTheKeyRingUnprotectOperationCannotProceed(keyIdFromPayload); + throw Error.Common_KeyNotFound(keyIdFromPayload); + } } // Do we need to notify the caller that he should reprotect the data? diff --git a/src/DataProtection/DataProtection/src/KeyManagement/KeyRingProvider.cs b/src/DataProtection/DataProtection/src/KeyManagement/KeyRingProvider.cs index e407ae62dd..8f3fa1904d 100644 --- a/src/DataProtection/DataProtection/src/KeyManagement/KeyRingProvider.cs +++ b/src/DataProtection/DataProtection/src/KeyManagement/KeyRingProvider.cs @@ -45,11 +45,18 @@ namespace Microsoft.AspNetCore.DataProtection.KeyManagement CacheableKeyRingProvider = this; _defaultKeyResolver = defaultKeyResolver; _logger = loggerFactory.CreateLogger(); + + // We will automatically refresh any unknown keys for 2 minutes see https://github.com/aspnet/AspNetCore/issues/3975 + AutoRefreshWindowEnd = DateTime.UtcNow.AddMinutes(2); } // for testing internal ICacheableKeyRingProvider CacheableKeyRingProvider { get; set; } + internal DateTime AutoRefreshWindowEnd { get; set; } + + internal bool InAutoRefreshWindow() => DateTime.UtcNow < AutoRefreshWindowEnd; + private CacheableKeyRing CreateCacheableKeyRingCore(DateTimeOffset now, IKey keyJustAdded) { // Refresh the list of all keys @@ -142,15 +149,24 @@ namespace Microsoft.AspNetCore.DataProtection.KeyManagement return GetCurrentKeyRingCore(DateTime.UtcNow); } - internal IKeyRing GetCurrentKeyRingCore(DateTime utcNow) + internal IKeyRing RefreshCurrentKeyRing() + { + return GetCurrentKeyRingCore(DateTime.UtcNow, forceRefresh: true); + } + + internal IKeyRing GetCurrentKeyRingCore(DateTime utcNow, bool forceRefresh = false) { Debug.Assert(utcNow.Kind == DateTimeKind.Utc); // Can we return the cached keyring to the caller? - var existingCacheableKeyRing = Volatile.Read(ref _cacheableKeyRing); - if (CacheableKeyRing.IsValid(existingCacheableKeyRing, utcNow)) + CacheableKeyRing existingCacheableKeyRing = null; + if (!forceRefresh) { - return existingCacheableKeyRing.KeyRing; + existingCacheableKeyRing = Volatile.Read(ref _cacheableKeyRing); + if (CacheableKeyRing.IsValid(existingCacheableKeyRing, utcNow)) + { + return existingCacheableKeyRing.KeyRing; + } } // The cached keyring hasn't been created or must be refreshed. We'll allow one thread to @@ -163,18 +179,21 @@ namespace Microsoft.AspNetCore.DataProtection.KeyManagement Monitor.TryEnter(_cacheableKeyRingLockObj, (existingCacheableKeyRing != null) ? 0 : Timeout.Infinite, ref acquiredLock); if (acquiredLock) { - // This thread acquired the critical section and is responsible for updating the - // cached keyring. But first, let's make sure that somebody didn't sneak in before - // us and update the keyring on our behalf. - existingCacheableKeyRing = Volatile.Read(ref _cacheableKeyRing); - if (CacheableKeyRing.IsValid(existingCacheableKeyRing, utcNow)) + if (!forceRefresh) { - return existingCacheableKeyRing.KeyRing; - } + // This thread acquired the critical section and is responsible for updating the + // cached keyring. But first, let's make sure that somebody didn't sneak in before + // us and update the keyring on our behalf. + existingCacheableKeyRing = Volatile.Read(ref _cacheableKeyRing); + if (CacheableKeyRing.IsValid(existingCacheableKeyRing, utcNow)) + { + return existingCacheableKeyRing.KeyRing; + } - if (existingCacheableKeyRing != null) - { - _logger.ExistingCachedKeyRingIsExpired(); + if (existingCacheableKeyRing != null) + { + _logger.ExistingCachedKeyRingIsExpired(); + } } // It's up to us to refresh the cached keyring. diff --git a/src/DataProtection/DataProtection/test/KeyManagement/KeyRingBasedDataProtectorTests.cs b/src/DataProtection/DataProtection/test/KeyManagement/KeyRingBasedDataProtectorTests.cs index d28ea7ff84..97476760ac 100644 --- a/src/DataProtection/DataProtection/test/KeyManagement/KeyRingBasedDataProtectorTests.cs +++ b/src/DataProtection/DataProtection/test/KeyManagement/KeyRingBasedDataProtectorTests.cs @@ -2,17 +2,20 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Globalization; using System.IO; using System.Linq; using System.Net; using System.Reflection; using System.Text; +using System.Threading; using Microsoft.AspNetCore.DataProtection.AuthenticatedEncryption; using Microsoft.AspNetCore.DataProtection.AuthenticatedEncryption.ConfigurationModel; using Microsoft.AspNetCore.DataProtection.KeyManagement.Internal; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; using Moq; using Xunit; @@ -224,6 +227,162 @@ namespace Microsoft.AspNetCore.DataProtection.KeyManagement Assert.Equal(Error.Common_KeyNotFound(notFoundKeyId).Message, ex.Message); } + private static DateTime StringToDateTime(string input) + { + return DateTimeOffset.ParseExact(input, "u", CultureInfo.InvariantCulture).UtcDateTime; + } + + private static KeyRingProvider CreateKeyRingProvider(ICacheableKeyRingProvider cacheableKeyRingProvider) + { + var mockEncryptorFactory = new Mock(); + mockEncryptorFactory.Setup(m => m.CreateEncryptorInstance(It.IsAny())).Returns(new Mock().Object); + var options = new KeyManagementOptions(); + options.AuthenticatedEncryptorFactories.Add(mockEncryptorFactory.Object); + + return new KeyRingProvider( + keyManager: null, + keyManagementOptions: Options.Create(options), + defaultKeyResolver: null, + loggerFactory: NullLoggerFactory.Instance) + { + CacheableKeyRingProvider = cacheableKeyRingProvider + }; + } + + [Fact] + public void Unprotect_KeyNotFound_RefreshOnce_ThrowsKeyNotFound() + { + // Arrange + Guid notFoundKeyId = new Guid("654057ab-2491-4471-a72a-b3b114afda38"); + byte[] protectedData = BuildProtectedDataFromCiphertext( + keyId: notFoundKeyId, + ciphertext: new byte[0]); + + var mockDescriptor = new Mock(); + var mockEncryptorFactory = new Mock(); + mockEncryptorFactory.Setup(o => o.CreateEncryptorInstance(It.IsAny())).Returns(new Mock().Object); + var encryptorFactory = new AuthenticatedEncryptorFactory(NullLoggerFactory.Instance); + + // the keyring has only one key + Key key = new Key(Guid.Empty, DateTimeOffset.Now, DateTimeOffset.Now, DateTimeOffset.Now, mockDescriptor.Object, new[] { mockEncryptorFactory.Object }); + var keyRing = new CacheableKeyRing(CancellationToken.None, DateTimeOffset.MaxValue, key, new[] { key }); + + var keyRingProvider = CreateKeyRingProvider(new TestKeyRingProvider(keyRing)); + + IDataProtector protector = new KeyRingBasedDataProtector( + keyRingProvider: keyRingProvider, + logger: GetLogger(), + originalPurposes: null, + newPurpose: "purpose"); + + // Act & assert + var ex = ExceptionAssert2.ThrowsCryptographicException(() => protector.Unprotect(protectedData)); + Assert.Equal(Error.Common_KeyNotFound(notFoundKeyId).Message, ex.Message); + } + + [Fact] + public void Unprotect_KeyNotFound_WontRefreshOnce_AfterTooLong() + { + // Arrange + Guid notFoundKeyId = new Guid("654057ab-2491-4471-a72a-b3b114afda38"); + byte[] protectedData = BuildProtectedDataFromCiphertext( + keyId: notFoundKeyId, + ciphertext: new byte[0]); + + var mockDescriptor = new Mock(); + var mockEncryptorFactory = new Mock(); + mockEncryptorFactory.Setup(o => o.CreateEncryptorInstance(It.IsAny())).Returns(new Mock().Object); + var encryptorFactory = new AuthenticatedEncryptorFactory(NullLoggerFactory.Instance); + + // the keyring has only one key + Key key = new Key(Guid.Empty, DateTimeOffset.Now, DateTimeOffset.Now, DateTimeOffset.Now, mockDescriptor.Object, new[] { mockEncryptorFactory.Object }); + var keyRing = new CacheableKeyRing(CancellationToken.None, DateTimeOffset.MaxValue, key, new[] { key }); + + // the refresh keyring has the notfound key + Key key2 = new Key(notFoundKeyId, DateTimeOffset.Now, DateTimeOffset.Now, DateTimeOffset.Now, mockDescriptor.Object, new[] { mockEncryptorFactory.Object }); + var keyRing2 = new CacheableKeyRing(CancellationToken.None, DateTimeOffset.MaxValue, key, new[] { key2 }); + + var keyRingProvider = CreateKeyRingProvider(new RefreshTestKeyRingProvider(keyRing, keyRing2)); + keyRingProvider.AutoRefreshWindowEnd = DateTime.UtcNow; + + IDataProtector protector = new KeyRingBasedDataProtector( + keyRingProvider: keyRingProvider, + logger: GetLogger(), + originalPurposes: null, + newPurpose: "purpose"); + + // Act & assert + var ex = ExceptionAssert2.ThrowsCryptographicException(() => protector.Unprotect(protectedData)); + Assert.Equal(Error.Common_KeyNotFound(notFoundKeyId).Message, ex.Message); + } + + [Fact] + public void Unprotect_KeyNotFound_RefreshOnce_CanFindKey() + { + // Arrange + Guid notFoundKeyId = new Guid("654057ab-2491-4471-a72a-b3b114afda38"); + byte[] protectedData = BuildProtectedDataFromCiphertext( + keyId: notFoundKeyId, + ciphertext: new byte[0]); + + var mockDescriptor = new Mock(); + var mockEncryptorFactory = new Mock(); + mockEncryptorFactory.Setup(o => o.CreateEncryptorInstance(It.IsAny())).Returns(new Mock().Object); + var encryptorFactory = new AuthenticatedEncryptorFactory(NullLoggerFactory.Instance); + + // the keyring has only one key + Key key = new Key(Guid.Empty, DateTimeOffset.Now, DateTimeOffset.Now, DateTimeOffset.Now, mockDescriptor.Object, new[] { mockEncryptorFactory.Object }); + var keyRing = new CacheableKeyRing(CancellationToken.None, DateTimeOffset.MaxValue, key, new[] { key }); + + // the refresh keyring has the notfound key + Key key2 = new Key(notFoundKeyId, DateTimeOffset.Now, DateTimeOffset.Now, DateTimeOffset.Now, mockDescriptor.Object, new[] { mockEncryptorFactory.Object }); + var keyRing2 = new CacheableKeyRing(CancellationToken.None, DateTimeOffset.MaxValue, key, new[] { key2 }); + + var keyRingProvider = CreateKeyRingProvider(new RefreshTestKeyRingProvider(keyRing, keyRing2)); + + IDataProtector protector = new KeyRingBasedDataProtector( + keyRingProvider: keyRingProvider, + logger: GetLogger(), + originalPurposes: null, + newPurpose: "purpose"); + + // Act & assert + var result = protector.Unprotect(protectedData); + Assert.Empty(result); + } + + private class TestKeyRingProvider : ICacheableKeyRingProvider + { + private CacheableKeyRing _keyRing; + + public TestKeyRingProvider(CacheableKeyRing keys) => _keyRing = keys; + + public CacheableKeyRing GetCacheableKeyRing(DateTimeOffset now) => _keyRing; + } + + private class RefreshTestKeyRingProvider : ICacheableKeyRingProvider + { + private CacheableKeyRing _keyRing; + private CacheableKeyRing _refreshKeyRing; + private bool _called; + + public RefreshTestKeyRingProvider(CacheableKeyRing keys, CacheableKeyRing refreshKeys) + { + _keyRing = keys; + _refreshKeyRing = refreshKeys; + } + + public CacheableKeyRing GetCacheableKeyRing(DateTimeOffset now) + { + if (!_called) + { + _called = true; + return _keyRing; + } + return _refreshKeyRing; + } + } + [Fact] public void Unprotect_KeyRevoked_RevocationDisallowed_ThrowsKeyRevoked() { diff --git a/src/DataProtection/DataProtection/test/KeyManagement/KeyRingProviderTests.cs b/src/DataProtection/DataProtection/test/KeyManagement/KeyRingProviderTests.cs index 8582ed8359..fd62d6a028 100644 --- a/src/DataProtection/DataProtection/test/KeyManagement/KeyRingProviderTests.cs +++ b/src/DataProtection/DataProtection/test/KeyManagement/KeyRingProviderTests.cs @@ -380,6 +380,47 @@ namespace Microsoft.AspNetCore.DataProtection.KeyManagement mockCacheableKeyRingProvider.Verify(o => o.GetCacheableKeyRing(It.IsAny()), Times.Once); } + [Fact] + public void GetCurrentKeyRing_KeyRingCached_CanForceRefresh() + { + // Arrange + var now = StringToDateTime("2015-03-01 00:00:00Z"); + var expectedKeyRing1 = new Mock().Object; + var expectedKeyRing2 = new Mock().Object; + var mockCacheableKeyRingProvider = new Mock(); + mockCacheableKeyRingProvider + .Setup(o => o.GetCacheableKeyRing(now)) + .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))) + .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))) + .Returns(new CacheableKeyRing( + expirationToken: CancellationToken.None, + expirationTime: StringToDateTime("2015-03-02 00:00:00Z"), + keyRing: expectedKeyRing2)); + + var keyRingProvider = CreateKeyRingProvider(mockCacheableKeyRingProvider.Object); + + // Act + var retVal1 = keyRingProvider.GetCurrentKeyRingCore(now); + var retVal2 = keyRingProvider.GetCurrentKeyRingCore(now + TimeSpan.FromMinutes(1)); + var retVal3 = keyRingProvider.GetCurrentKeyRingCore(now + TimeSpan.FromMinutes(2), forceRefresh: true); + + // Assert - underlying provider should be called twice + Assert.Same(expectedKeyRing1, retVal1); + Assert.Same(expectedKeyRing1, retVal2); + Assert.Same(expectedKeyRing2, retVal3); + mockCacheableKeyRingProvider.Verify(o => o.GetCacheableKeyRing(It.IsAny()), Times.Exactly(2)); + } + [Fact] public void GetCurrentKeyRing_KeyRingCached_AfterExpiration_ClearsCache() {