Certificate validation cache key is not stable across application event callbacks

[rokonec]

Summary

CertificateValidationCache derives the scheme component of its cache key from mutable per-request state that is read at call time. CertificateAuthenticationHandler establishes that state before reading the cache but does not re-establish it before writing, and between those two points it awaits application-supplied event callbacks. The key used to store a result is therefore not guaranteed to identify the scheme that produced it.

What is wrong

The invariant introduced by #66673 is that a result stored in the certificate validation cache is keyed by the authentication scheme that produced it, so one scheme's cached result is never returned for another scheme's lookup. That invariant is established on the read path but not on the write path.

• The scheme name reaches the cache out of band, through the single-slot per-request dictionary entry HttpContext.Items[CertificateAuthenticationHandler.CertificateSchemeCacheKeyItem], rather than as a parameter. ICertificateValidationCache.Get and Put accept only (HttpContext, X509Certificate2).
• CertificateValidationCache.ComputeKey reads that slot at call time, so the key is late-bound on both the read and the write.
• CertificateAuthenticationHandler.HandleAuthenticateAsync writes the slot, reads the cache, then awaits Events.CertificateValidated and — on the non-exception failure path — Events.AuthenticationFailed, before writing the cache. The scheme is never captured into a local and never re-asserted, and no try/finally restores the slot.
• Handler instances, and the AuthenticationHandler._authenticateTask idempotence guard, are scoped per (request × scheme name). The Items slot is scoped per request and shared by every certificate scheme. A per-scheme guard cannot protect a per-request single-slot channel, so any code that runs a second certificate scheme's handler inside those callbacks leaves the slot holding a different scheme name for the remainder of the request.

Stated as a property: the key derivation is stable across the read but not across the write, so the store operation can associate a result with a scheme other than the one that produced it.

Two secondary observations worth addressing in the same work item:

• AddCertificateCache registers ICertificateValidationCache as an application-wide singleton. Neither CertificateAuthenticationOptions nor CertificateValidationCacheOptions (configured unnamed) offers a per-scheme opt-out, so an application that registers two certificate schemes necessarily shares one key space. There is no supported configuration that avoids it.
• The ICertificateValidationCache contract exposes no scheme parameter. A third-party implementation either depends on an internal const it cannot reference, or keys on the certificate alone — which does not satisfy the isolation invariant at all. The Include scheme in certificate cache for Authentication middleware #66673 fix lives in the in-box implementation rather than in the contract.

Why it matters (defense in depth)

• Correctness, independent of any actor: a scheme can observe a cached result it did not produce, skipping its own chain build, revocation check, and CertificateValidated callback. Claims minted by one scheme's callback can surface under another scheme's identity.
• AuthenticateResult.Clone() preserves Ticket.AuthenticationScheme, and nothing downstream compares it against the requested scheme, so the inconsistency is neither detected nor corrected later in the pipeline. ClaimsIdentity.AuthenticationType is the same constant for every certificate scheme, so it cannot distinguish them either.
• The write is unconditional for success, failure, and no-result outcomes, so a misattributed entry can suppress a scheme's own validation in either direction, for the cache lifetime.
• Hardening value: this closes the remaining gap in the scheme-isolation boundary that Include scheme in certificate cache for Authentication middleware #66673 set out to establish, and makes that boundary hold regardless of what application code does inside the event callbacks.

Affected code

• src/Security/Authentication/Certificate/src/CertificateAuthenticationHandler.cs:16 — CertificateSchemeCacheKeyItem, the out-of-band channel; internal const, so applications cannot participate in maintaining it.
• src/Security/Authentication/Certificate/src/CertificateAuthenticationHandler.cs:70-78 — sole write of the slot, immediately followed by Get. Note the write also occurs on the cache-hit return path.
• src/Security/Authentication/Certificate/src/CertificateAuthenticationHandler.cs:80-90 — the two awaited application callbacks that sit between the write and the read-back.
• src/Security/Authentication/Certificate/src/CertificateAuthenticationHandler.cs:92 — Put, whose key is recomputed from the slot.
• src/Security/Authentication/Certificate/src/CertificateValidationCache.cs:40-48 — Get.
• src/Security/Authentication/Certificate/src/CertificateValidationCache.cs:56-75 — Put, unconditional for all result kinds.
• src/Security/Authentication/Certificate/src/CertificateValidationCache.cs:77-85 — ComputeKey, the late-bound read of the slot.
• src/Security/Authentication/Certificate/src/ICertificateValidationCache.cs — contract with no scheme parameter.
• src/Security/Authentication/test/CertificateTests.cs:938-1060 — VerifyCacheIsIsolatedAcrossSchemes and VerifyCacheNoOpsWithoutSchemeInHttpContextItems; both exercise sequential requests only, so the re-entrant path is uncovered.

Recommended fix

Capture Scheme.Name into a local before any await, and re-establish the slot immediately before Put so the key is derived from state the handler controls rather than from state application code may have changed. This is a minimal, non-breaking change confined to one method.

protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
{
    // You only get client certificates over HTTPS
    if (!Context.Request.IsHttps)
    {
        Logger.NotHttps();
        return AuthenticateResult.NoResult();
    }

    try
    {
        var clientCertificate = await Context.Connection.GetClientCertificateAsync();

        // This should never be the case, as cert authentication happens long before ASP.NET kicks in.
        if (clientCertificate == null)
        {
            Logger.NoCertificate();
            return AuthenticateResult.NoResult();
        }

        // Event callbacks may run another scheme's handler, which overwrites the shared marker.
        var schemeName = Scheme.Name;

        if (_cache != null)
        {
            Context.Items[CertificateSchemeCacheKeyItem] = schemeName;
            var cacheHit = _cache.Get(Context, clientCertificate);
            if (cacheHit != null)
            {
                return cacheHit;
            }
        }

        var result = await ValidateCertificateAsync(clientCertificate);

        // Invoke the failed handler if validation failed, before updating the cache
        if (result.Failure != null)
        {
            var authenticationFailedContext = await HandleFailureAsync(result.Failure);
            if (authenticationFailedContext.Result != null)
            {
                result = authenticationFailedContext.Result;
            }
        }

        if (_cache != null)
        {
            Context.Items[CertificateSchemeCacheKeyItem] = schemeName;
            _cache.Put(Context, clientCertificate, result);
        }

        return result;
    }
    catch (Exception ex)
    {
        var authenticationFailedContext = await HandleFailureAsync(ex);
        if (authenticationFailedContext.Result != null)
        {
            return authenticationFailedContext.Result;
        }

        throw;
    }
}

The catch path needs no change: it has two terminal exits and no fall-through to Put, so no cache write occurs when an exception propagates.

Alternatives considered

• Add a scheme parameter to ICertificateValidationCache. Structurally correct — it removes the out-of-band channel entirely and lets third-party implementations honour the invariant. Rejected for servicing because ICertificateValidationCache is public, so this is a breaking change. Worth considering for main as a follow-up, optionally as a default interface method that forwards to the existing overload.
• try/finally around the callbacks to restore the slot. Equivalent for this defect and additionally leaves the slot consistent for the rest of the pipeline, but broader in scope than the property being fixed, and the slot has no defined meaning outside the handler.
• Move the channel to AsyncLocal. Heavier, and the value is genuinely request-scoped, so it would trade one implicit channel for another without removing the late binding.
• Document the constraint instead of fixing it. Not viable: the slot is an internal const on an internal sealed type, so applications cannot save or restore it, and no per-scheme cache opt-out exists. The framework is the only component able to maintain the invariant.

Compatibility, migration, versioning

• No public API change; ICertificateValidationCache, CertificateValidationCache, CertificateAuthenticationOptions, and CertificateValidationCacheOptions are untouched.
• No cache format or key format change, so no invalidation or migration is required. Existing entries remain valid.
• Behaviour changes only where the slot would previously have been observed in a mutated state; all single-scheme and sequential multi-scheme behaviour is unchanged.
• Applies wherever Include scheme in certificate cache for Authentication middleware #66673 shipped: main plus the servicing branches carrying the scheme-keyed cache. Branches predating Include scheme in certificate cache for Authentication middleware #66673 key on the certificate alone and need that change first.
• Consider adding remarks to ICertificateValidationCache stating that implementations must treat the certificate alone as an insufficient key when more than one certificate scheme is registered.

Acceptance criteria

• A result produced by a given scheme is retrievable only under that scheme's key, regardless of what application code does inside CertificateValidated or AuthenticationFailed.
• Regression test: a scheme whose CertificateValidated callback triggers authentication for a second certificate scheme registered against the same cache. Assert the outer scheme's result is stored under the outer scheme's key, and that a later lookup by the second scheme does not observe it.
• Equivalent coverage for the AuthenticationFailed callback on the non-exception failure path.
• Test asserting no cache write occurs when an exception propagates out of a callback.
• Existing VerifyCacheIsIsolatedAcrossSchemes and VerifyCacheNoOpsWithoutSchemeInHttpContextItems continue to pass unchanged.
• Decision recorded on whether ICertificateValidationCache gains a scheme parameter in main, and whether the interface documentation is updated for third-party implementers.

[github-actions]

Triage Summary

Area: area-auth (issue concerns CertificateAuthenticationHandler and CertificateValidationCache in src/Security/Authentication/Certificate/)
Type: Bug (describes a scheme-isolation invariant, introduced by #66673, that holds on the cache read path but not on the write path)

Potential Duplicates

• None found

Generated by Issue Triage Agent for dotnet/aspnetcore for #69273 · copilot · auto · 45 AIC · ⌖ 7.55 AIC · ⊞ 31.8K · ◷