The Wayback Machine - https://web.archive.org/web/20210528165029/https://github.com/dotnet/aspnetcore/issues/33116
Skip to content
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

Transient failure to refresh a key ring produces a 500 response #33116

Open
mdomashchenko opened this issue May 28, 2021 · 0 comments
Open

Transient failure to refresh a key ring produces a 500 response #33116

mdomashchenko opened this issue May 28, 2021 · 0 comments

Comments

@mdomashchenko
Copy link

@mdomashchenko mdomashchenko commented May 28, 2021

Here's how it happens:

  1. A request comes in, with a cookie. Cookie handler needs to decrypt the cookie, this ultimately results in a call to KeyRingBasedDataProtector.UnprotectCore()
  2. UnprotectCore calls KeyRingProvider.GetCurrentKeyRingCore to get keys. The latter determines that it needs to refresh the key ring from the repository.
  3. The repository experiences some transient failure and throws an exception. At this time the KeyRingProvider does have the key ring available, so it extends its lifetime for 2 minutes and reports that from here
    _logger.ErrorOccurredWhileRefreshingKeyRing(ex);
  4. Now, instead of just returning the old key ring with freshly extended lifetime, KeyRingProvider re-throws the exception. Rationale for that is described here
    // The immediate caller should fail so that they can report the error up the chain. This makes it more likely
    // that an administrator can see the error and react to it as appropriate. The caller can retry the operation
    // and will probably have success as long as they fall within the temporary extension mentioned above.
  5. The problem is, KeyRingBasedDataProtector.UnprotectCore does not retry. If it did, it would get the key ring, because it's lifetime was just extended. But it just doesn't make that second call to GetCurrentKeyRing
    var currentKeyRing = _keyRingProvider.GetCurrentKeyRing();
  6. So, the cookie is not decrypted, authentication handler fails and the error leaks to the caller as a 500 response, which is completely preventable.

Here's a stack trace for your reference

at Microsoft.AspNetCore.DataProtection.KeyManagement.XmlKeyManager.GetAllKeys()
at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingProvider.CreateCacheableKeyRingCore(DateTimeOffset now, IKey keyJustAdded)
at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingProvider.Microsoft.AspNetCore.DataProtection.KeyManagement.Internal.ICacheableKeyRingProvider.GetCacheableKeyRing(DateTimeOffset now)
at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingProvider.GetCurrentKeyRingCore(DateTime utcNow, Boolean forceRefresh)
at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingBasedDataProtector.UnprotectCore(Byte[] protectedData, Boolean allowOperationsOnRevokedKeys, UnprotectStatus& status)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant