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

[BUG] Azure.Extensions.AspNetCore.DataProtection.Keys Heavy load causes blocked threads #40174

Closed
kimbell opened this issue Nov 22, 2023 · 5 comments
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Extensions ASP.NET Core extensions needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@kimbell
Copy link

kimbell commented Nov 22, 2023

Library name and version

Azure.Extensions.AspNetCore.DataProtection.Keys 1.2.2

Describe the bug

We are using this together with ASP.NET Cookie authentication. One of our applications receives very many requests. When we restarted this application, it stopped responding. We took a memory dump, and it showed that all threads were waiting on

return Task.Run(() => DecryptAsync(encryptedElement)).GetAwaiter().GetResult();

https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/master/AsyncGuidance.md#avoid-using-taskresult-and-taskwait

Expected behavior

It should handle heavy load during startup.

Actual behavior

All threads were blocked.

Reproduction Steps

NA

Environment

.NET 7

@github-actions github-actions bot added Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Extensions ASP.NET Core extensions needs-team-triage Workflow: This issue needs the team to triage. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Nov 22, 2023
@jsquire jsquire added needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team and removed needs-team-triage Workflow: This issue needs the team to triage. labels Nov 22, 2023
@jsquire
Copy link
Member

jsquire commented Nov 22, 2023

@AlexanderSher: Would you please take a look?

@jsquire
Copy link
Member

jsquire commented Nov 22, 2023

Thank you for your feedback. Tagging and routing to the team member best able to assist.

@kimbell
Copy link
Author

kimbell commented Nov 23, 2023

Our system is built up of many internal applications that uses web components for client side UI composition. One of these applications provide UI used by all of the others, so it receives a lot of traffic. All requests are authenticated using cookies.

Yesterday we got a new key in our key-ring, then we restarted this central application. The default key would then be the new one, but all of our web browsers had a cookie created with a previous key. I don't know the internal details of the data protection system, but if it only decrypts XML when it receives a cookie encrypted with a key it hasn't got decrypted XML for yet, all these old cookies would result in a lot of decryption requests.

One workaround we have been thinking about is to trigger all these decryption request during startup; then everything will be in place by the time we can handle requests. I've added this to our startup process

var km = serviceProvider.GetRequiredService<IKeyManager>();
var logger = serviceProvider.GetRequiredService<ILogger<Something>>();

foreach (var k in km.GetAllKeys())
{
    _ = k.Descriptor;
    logger.LogDebug("Successfully accessed descriptor for key {KeyId}", k.KeyId);
}

If I set a breakpoint on the Descriptor line, things crash due to some cross thread issue; the stacktrace includes the AzureKeyVaultXmlDecryptor

After adding this, I have noticed a change in the startup logs.

Found key {239f27f2-798a-469c-9fd7-7686a682680a}.
Found key {3a14bf9d-8ef7-42b5-b540-aff774e527ce}.
Found key {76a010a4-1485-47f1-b617-9b4ef25dd796}.
Found key {069f3ab7-b847-4ed2-a910-8ddf748ef33a}.
Found key {46559c28-bd65-4507-a7a0-15f23ba19376}.
Successfully accessed descriptor for key 239f27f2-798a-469c-9fd7-7686a682680a
Successfully accessed descriptor for key 3a14bf9d-8ef7-42b5-b540-aff774e527ce
Successfully accessed descriptor for key 76a010a4-1485-47f1-b617-9b4ef25dd796
Successfully accessed descriptor for key 069f3ab7-b847-4ed2-a910-8ddf748ef33a
Successfully accessed descriptor for key 46559c28-bd65-4507-a7a0-15f23ba19376
Found key {239f27f2-798a-469c-9fd7-7686a682680a}.
Found key {3a14bf9d-8ef7-42b5-b540-aff774e527ce}.
Found key {76a010a4-1485-47f1-b617-9b4ef25dd796}.
Found key {069f3ab7-b847-4ed2-a910-8ddf748ef33a}.
Found key {46559c28-bd65-4507-a7a0-15f23ba19376}.

Before adding the code, I only get one set of 'Found key' (issued by Microsoft.AspNetCore.DataProtection.KeyManagement.XmlKeyManager)

@AlexanderSher
Copy link
Contributor

There are several places where sync over async is used. I'm going to update those to use proper sync API that should help with task-related part of the issue, so then we can look into the amount of decryption requests.

@AlexanderSher
Copy link
Contributor

#40914

@github-actions github-actions bot locked and limited conversation to collaborators Apr 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Extensions ASP.NET Core extensions needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
Development

No branches or pull requests

3 participants