dotnet / dotnet/aspnetcore

Async APIs for Data Protection key ring access

Open
#66,684 1 comment 0 reactions 0 assignees View on GitHub
area-dataprotection feature-request
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 6h
Merged PRs (30d)
290

Description

### Is there an existing issue for this?

- [x] I have searched the existing issues

### Is your feature request related to a problem? Please describe the problem.

Today, the entire Data Protection stack is synchronous:

```csharp
namespace Microsoft.AspNetCore.DataProtection.KeyManagement.Internal;

public interface IKeyRingProvider
{
IKeyRing GetCurrentKeyRing();
}

namespace Microsoft.AspNetCore.DataProtection;

public interface IDataProtector : IDataProtectionProvider
{
byte[] Protect(byte[] plaintext);
byte[] Unprotect(byte[] protectedData);
}
```
`GetCurrentKeyRing()` is on the hot path of every `Protect` / `Unprotect` call — which means it is on the hot path of every cookie auth check, antiforgery check, identity token validation, and so on. When the cached key ring is valid (the 99.9% case), the call is a single `Volatile.Read` and is essentially free. But when it isn't valid, the call blocks the calling thread — sometimes briefly, sometimes for the full duration of a repository read against the configured `IXmlRepository` (file share, Azure Blob, Azure Key Vault, Redis, EF, etc.).

Because the API contract is synchronous, every caller in the framework above must consume the result synchronously, and all the I/O the implementation might transitively perform must be turned into a synchronous wait somewhere in the chain.

### Concrete problem this causes

Issue #66380 is a direct consequence of this synchronous contract. The .NET 10 `KeyRingProvider` [rewrite](https://github.com/dotnet/aspnetcore/pull/54675) tried to dispatch refresh work onto `TaskScheduler.Default` so callers wouldn't pay the latency, but on cold start the synchronous public API forces the dispatcher and every racing caller to `Task.Wait()` on the dispatched work — pinning every thread‑pool thread on a task that needs a free thread‑pool thread to run.

The point fix in #66683 resolves the immediate bug by running the cold‑start refresh inline on the calling thread (matching the pre‑#54675 behavior). But it's a workaround for a structural limitation: as long as `GetCurrentKeyRing()` is synchronous, every refresh strategy that *doesn't* run on the calling thread has the same deadlock surface in some configuration.

### Describe the solution you'd like

### Proposed addition

Add **non‑breaking** async overloads to the public API surface, leaving the existing sync members in place for backwards compatibility:

```diff
namespace Microsoft.AspNetCore.DataProtection.KeyManagement.Internal;

public interface IKeyRingProvider
{
IKeyRing GetCurrentKeyRing();

+ ValueTask GetCurrentKeyRingAsync(CancellationToken cancellationToken = default);
}

namespace Microsoft.AspNetCore.DataProtection;

public interface IDataProtector : IDataProtectionProvider
{
byte[] Protect(byte[] plaintext);
byte[] Unprotect(byte[] protectedData);

+ ValueTask ProtectAsync(byte[] plaintext, CancellationToken cancellationToken = default);
+ ValueTask UnprotectAsync(byte[] protectedData, CancellationToken cancellationToken = default);
}
```

`ValueTask` is preferred for the hot path because the cached‑ring case is allocation‑free.

### Why async eliminates the starvation class of bugs

`await` returns the calling thread to the pool while the work is in flight, instead of pinning it inside `Task.Wait()`. Concurrent cold‑start callers no longer occupy thread‑pool slots while the refresh runs, so any refresh strategy — inline, dispatched, retried, whatever — is starvation‑safe. The refresh implementation can also legitimately do real async I/O against `IXmlRepository` instead of sync‑over‑async, which today is forced everywhere from file reads to Azure Blob calls.

### Additional context

_No response_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.