Skip to content

Keyring error when persisting key to registry #60049

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
1 task done
Julien-Marpault opened this issue Jan 27, 2025 · 9 comments
Closed
1 task done

Keyring error when persisting key to registry #60049

Julien-Marpault opened this issue Jan 27, 2025 · 9 comments
Labels
area-dataprotection Includes: DataProtection Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update.

Comments

@Julien-Marpault
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

I customized DataProtection to save key to windows registry.
When doing this I get this exception:

fail: Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingProvider[48]
      An error occurred while reading the key ring.
      System.ArgumentOutOfRangeException: The added or subtracted value results in an un-representable DateTime. (Parameter 't')
         at System.DateTime.ThrowDateArithmetic(Int32 param)
         at System.DateTime.op_Addition(DateTime d, TimeSpan t)
         at System.DateTimeOffset.op_Addition(DateTimeOffset dateTimeOffset, TimeSpan timeSpan)
         at Microsoft.AspNetCore.DataProtection.KeyManagement.DefaultKeyResolver.<>c__DisplayClass6_0.<FindDefaultKey>b__3(IKey key)
         at System.Linq.Enumerable.Any[TSource](IEnumerable`1 source, Func`2 predicate)
         at Microsoft.AspNetCore.DataProtection.KeyManagement.DefaultKeyResolver.FindDefaultKey(DateTimeOffset now, IEnumerable`1 allKeys, IKey& fallbackKey, Boolean& callerShouldGenerateNewKey)
         at Microsoft.AspNetCore.DataProtection.KeyManagement.DefaultKeyResolver.ResolveDefaultKeyPolicy(DateTimeOffset now, IEnumerable`1 allKeys)
         at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingProvider.CreateCacheableKeyRingCore(DateTimeOffset now, IKey keyJustAdded)
         at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingProvider.GetCurrentKeyRingCore(DateTime utcNow, Boolean forceRefresh)

Here's my code:

public static IServiceCollection AddDataProtectionKeysToWindowsRegistry(this IServiceCollection services)
    {
        RegistryKey registreyKey = Registry.CurrentUser.CreateSubKey(@"Software\Test\keys", true);

        services.AddDataProtection()
            .SetApplicationName(DataProtectionKeysOptions.ApplicationName)
            .PersistKeysToRegistry(registreyKey)
            .ProtectKeysWithDpapi();

        return services;
    }

Removing PersistKeysToRegistry() makes things working.
Removing ProtectKeysWithDpapi() doesn't change anything.

Expected Behavior

No response

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version

8.0.404

Anything else?

No response

@ghost ghost added the area-dataprotection Includes: DataProtection label Jan 27, 2025
@martincostello
Copy link
Member

Looks like the exception is coming from here:

where key.ActivationDate <= now + _maxServerToServerClockSkew

Have you re-configured MaxServerClockSkew or how UTC now (e.g. changing the time provider)?

_maxServerToServerClockSkew = KeyManagementOptions.MaxServerClockSkew;

It seems like one of the two (or both) are returning unexpecting values, causing the addition to go wrong.

Getting an idea of what the values going into the addition will help clarify what's going wrong.

@martincostello martincostello added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label Jan 27, 2025
@Julien-Marpault
Copy link
Author

I didn't re-confifured ClockSkew.
I juste tried to launch the app (13:56 UTC+1)
Here's the key:

<key id="2ea6b366-2e71-443b-8377-3d33d947bbf9" version="1">
  <creationDate>2025-01-27T12:56:37.9446723Z</creationDate>
  <activationDate>2025-01-27T13:56:37.8812794+01:00</activationDate>
  <expirationDate>9999-12-31T23:59:59.9999999Z</expirationDate>
  <descriptor deserializerType="Microsoft.AspNetCore.DataProtection.AuthenticatedEncryption.ConfigurationModel.AuthenticatedEncryptorDescriptorDeserializer, Microsoft.AspNetCore.DataProtection, Version=8.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60">
    <descriptor>
      <encryption algorithm="AES_256_CBC" />
      <validation algorithm="HMACSHA256" />
      <encryptedSecret decryptorType="Microsoft.AspNetCore.DataProtection.XmlEncryption.DpapiXmlDecryptor, Microsoft.AspNetCore.DataProtection, Version=8.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60" xmlns="http://schemas.asp.net/2015/03/dataProtection">
        <encryptedKey xmlns="">
          <!-- This key is encrypted with Windows DPAPI. -->
          <value>****</value>
        </encryptedKey>
      </encryptedSecret>
    </descriptor>
  </descriptor>
</key>

@dotnet-policy-service dotnet-policy-service bot added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Jan 27, 2025
@Julien-Marpault
Copy link
Author

Julien-Marpault commented Jan 27, 2025

Spotted the issue
DefaultKeyResolver.FindDefaultKey() Line 105:

callerShouldGenerateNewKey = !allKeys.Any(key =>
   key.ActivationDate <= (preferredDefaultKey.ExpirationDate + _maxServerToServerClockSkew)
   && !key.IsExpired(now + _keyPropagationWindow)
   && !key.IsRevoked);

Precisely it's this part wich is not working:

key.ActivationDate <= (preferredDefaultKey.ExpirationDate + _maxServerToServerClockSkew)

it's equal to :

key.ActivationDate <= (DateTimeOffset.MaxValue + TimeSpan.FromMinutes(5)));

So the value is over DateTimeOffset.MaxValue

@martincostello
Copy link
Member

// Does *any* key in the key ring fulfill the requirement that its activation date is prior
// to the preferred default key's expiration date (allowing for skew) and that it will
// remain valid one propagation cycle from now? If so, the caller doesn't need to add a
// new key.
callerShouldGenerateNewKey = !allKeys.Any(key =>
key.ActivationDate <= (preferredDefaultKey.ExpirationDate + _maxServerToServerClockSkew)
&& !key.IsExpired(now + _keyPropagationWindow)
&& !key.IsRevoked);

Looks like this is possibly only an issue for .NET 8.0.x as .NET 9.0.x and main don't seem to inspect ExpirationDate anymore in that method.

The code was refactored by #54309 and #57186 during the development of .NET 9.

@Julien-Marpault
Copy link
Author

@martincostello I seen this. But unfortunately we are on .Net 8 and I can't upgrade to .Net 9 now.

@martincostello
Copy link
Member

Unless the team think this is serious enough an issue to fix and backport to the release/8.0 branch, I think your only solution is a workaround to not use DateTimeOffset.MaxValue as the expiration date of your keys. I'm sure keys that "only" expire in the year 3000 would probably be fine, for example 😉.

@Julien-Marpault
Copy link
Author

@martincostello It's hardcoded:

IKeyManagerExtensions.GetKey() Line 24:

        if(key == null)
            key = _keyManager.CreateNewKey(DateTimeOffset.Now, DateTimeOffset.MaxValue);

I think a good workaround would be to crete a key before the first call to DataProtection. This way it will bypass the creation of the default key with DateTimeOffset.MaxValue as expiration date.

I'll do the PR after my test

@martincostello
Copy link
Member

Where is that code? I can't find it.

PRs for released versions of .NET have a higher bar to merge so I would suggest getting some input from the ASP.NET Core team first that this is something that has a good chance of being merged or not, to save you spending the time creating a PR that won't get approved.

@Julien-Marpault
Copy link
Author

Julien-Marpault commented Jan 27, 2025

My god, it's some extension code someone made internally to override IKeyManager.GetKey() Method.

I want to cry ....

I will close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dataprotection Includes: DataProtection Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update.
Projects
None yet
Development

No branches or pull requests

2 participants