Throw if the default key is revoked (#54278)

Under somewhat contrived circumstances, it's possible that the keyring, on determining that no suitable default key is available, will generate a new, immediately-activated key and that that key will also be immediately-revoked.  For example, it's possible to revoke all keys created before a given date and clock skew between servers could result in that being in the future for some servers.

It's also possible that a third-party `IDefaultKeyResolver` could select a revoked key, since the contract doesn't state that it should not.  Having said that, we haven't really hardened against other misbehavior by resolvers, so this isn't terribly compelling.

Still, no harm in throwing - better than using a revoked key to encrypt data.

Author: amcasey

src/DataProtection/DataProtection/src/Error.cs

@@ -91,4 +91,10 @@ public static InvalidOperationException XmlKeyManager_DuplicateKey(Guid keyId)
         var message = string.Format(CultureInfo.CurrentCulture, Resources.XmlKeyManager_DuplicateKey, keyId);
         return new InvalidOperationException(message);
     }
+
+    public static InvalidOperationException KeyRingProvider_DefaultKeyRevoked(Guid id)
+    {
+        var message = string.Format(CultureInfo.CurrentCulture, Resources.KeyRingProvider_DefaultKeyRevoked, id);
+        return new InvalidOperationException(message);
+    }
 }

src/DataProtection/DataProtection/src/KeyManagement/KeyRingProvider.cs

@@ -155,6 +155,13 @@ private CacheableKeyRing CreateCacheableKeyRingCoreStep2(DateTimeOffset now, Can
         // Invariant: our caller ensures that CreateEncryptorInstance succeeded at least once
         Debug.Assert(defaultKey.CreateEncryptor() != null);
 
+        // This can happen if there's a date-based revocation that's in the future (e.g. because of clock skew)
+        if (defaultKey.IsRevoked)
+        {
+            _logger.KeyRingDefaultKeyIsRevoked(defaultKey.KeyId);
+            throw Error.KeyRingProvider_DefaultKeyRevoked(defaultKey.KeyId);
+        }
+
         _logger.UsingKeyAsDefaultKey(defaultKey.KeyId);
 
         var nextAutoRefreshTime = now + GetRefreshPeriodWithJitter(KeyManagementOptions.KeyRingRefreshPeriod);

src/DataProtection/DataProtection/src/LoggingExtensions.cs

@@ -249,4 +249,7 @@ private static bool IsLogLevelEnabledCore([NotNullWhen(true)] ILogger? logger, L
 
     [LoggerMessage(64, LogLevel.Debug, "Not enabling read-only key access because an XML encryptor has been specified", EventName = "NotUsingReadOnlyKeyConfigurationBecauseOfEncryptor")]
     public static partial void NotUsingReadOnlyKeyConfigurationBecauseOfEncryptor(this ILogger logger);
+
+    [LoggerMessage(72, LogLevel.Error, "The key ring's default data protection key {KeyId:B} has been revoked.", EventName = "KeyRingDefaultKeyIsRevoked")]
+    public static partial void KeyRingDefaultKeyIsRevoked(this ILogger logger, Guid keyId);
 }

src/DataProtection/DataProtection/src/Resources.resx

@@ -187,6 +187,9 @@
   <data name="KeyRingProvider_NoDefaultKey_AutoGenerateDisabled" xml:space="preserve">
     <value>The key ring does not contain a valid default protection key. The data protection system cannot create a new key because auto-generation of keys is disabled. For more information go to https://aka.ms/aspnet/dataprotectionwarning</value>
   </data>
+  <data name="KeyRingProvider_DefaultKeyRevoked" xml:space="preserve">
+    <value>The key ring's default data protection key {0:B} has been revoked.</value>
+  </data>
   <data name="LifetimeMustNotBeNegative" xml:space="preserve">
     <value>{0} must not be negative.</value>
   </data>

src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/KeyManagement/KeyRingProviderTests.cs

@@ -145,6 +145,47 @@ public void CreateCacheableKeyRing_GenerationRequired_NoDefaultKey_CreatesNewKey
         Assert.Equal(new[] { "GetCacheExpirationToken", "GetAllKeys", "ResolveDefaultKeyPolicy", "CreateNewKey", "GetCacheExpirationToken", "GetAllKeys", "ResolveDefaultKeyPolicy" }, callSequence);
     }
 
+    [Fact]
+    public void CreateCacheableKeyRing_GenerationRequired_NoDefaultKey_CreatesNewKeyWithImmediateActivation_NewKeyIsRevoked()
+    {
+        // Arrange
+        var callSequence = new List<string>();
+
+        var now = (DateTimeOffset)StringToDateTime("2015-03-01 00:00:00Z");
+        var allKeys1 = Array.Empty<IKey>();
+
+        // This could happen if there were a date-based revocation newer than 2015-03-01
+        var newKey = CreateKey("2015-03-01 00:00:00Z", "2016-03-01 00:00:00Z", isRevoked: true);
+        var allKeys2 = new[] { newKey };
+
+        var keyRingProvider = SetupCreateCacheableKeyRingTestAndCreateKeyManager(
+            callSequence: callSequence,
+            getCacheExpirationTokenReturnValues: new[] { CancellationToken.None, CancellationToken.None },
+            getAllKeysReturnValues: new[] { allKeys1, allKeys2 },
+            createNewKeyCallbacks: new[] {
+                Tuple.Create(now, now + TimeSpan.FromDays(90), newKey)
+            },
+            resolveDefaultKeyPolicyReturnValues: new[]
+            {
+                Tuple.Create(now, (IEnumerable<IKey>)allKeys1, new DefaultKeyResolution()
+                {
+                    DefaultKey = null, // Since there are no keys
+                    ShouldGenerateNewKey = true
+                }),
+                Tuple.Create(now, (IEnumerable<IKey>)allKeys2, new DefaultKeyResolution()
+                {
+                    DefaultKey = null, // Since all keys are revoked
+                    ShouldGenerateNewKey = true
+                })
+            });
+
+        // Act/Assert
+        Assert.Throws<InvalidOperationException>(() => keyRingProvider.GetCacheableKeyRing(now)); // The would-be default key is revoked
+
+        // Still make the usual calls - just throw before creating a keyring
+        Assert.Equal(new[] { "GetCacheExpirationToken", "GetAllKeys", "ResolveDefaultKeyPolicy", "CreateNewKey", "GetCacheExpirationToken", "GetAllKeys", "ResolveDefaultKeyPolicy" }, callSequence);
+    }
+
     [Fact]
     public void CreateCacheableKeyRing_GenerationRequired_NoDefaultKey_CreatesNewKeyWithImmediateActivation_StillNoDefaultKey_ReturnsNewlyCreatedKey()
     {