Fix mTLS PoP token cache: key on certificate DER (x5t#S256), not the public key

[gladjohn]

Summary

MSAL keys its mTLS Proof-of-Possession (PoP) token cache on a value derived from the certificate's public key. ESTS/MSS, however, bind and validate the PoP token against the full DER certificate (x5t#S256, RFC 8705). On a same-key certificate renewal (identical public key, new DER), these diverge: MSAL keeps serving a cached PoP token bound to the previous certificate while the renewed certificate is presented on the wire, and the server rejects the call with AADSTS500181.

The fix keys the cache on the certificate DER, so any certificate change — including a same-key renewal — is a cache miss and the token is re-minted against the current certificate.

Symptom

A two-leg mTLS PoP flow fails intermittently with:

AADSTS500181: The TLS certificate provided does not match the certificate in the assertion.
error: CertificateValidationFailedTlsCertMismatch  (HTTP 400)

Both token acquisitions log TokenSource=Cache, yet the downstream mTLS call is rejected. It is transient — it lasts until the stale cached token expires and is re-minted.

Background: the two-leg flow

The caller runs two MSAL acquisitions and then calls a downstream mTLS endpoint:

1. Leg 1 — AcquireTokenForManagedIdentity(...).WithMtlsProofOfPossession() → returns an MSI access token and the binding certificate. (The token is used only as the client assertion for Leg 2; the certificate is presented on the downstream mTLS handshake.)
2. Leg 2 — AcquireTokenForClient(...).WithMtlsProofOfPossession() (client assertion = the Leg 1 token) → returns the PoP token. (This token is sent to the downstream endpoint.)
3. Downstream call — presents Leg 1's certificate on the mTLS wire together with Leg 2's PoP token in the auth header. The server validates that the wire certificate matches the PoP token's cnf.

The two things the server compares come from different caches: the certificate from Leg 1, the token from Leg 2.

Root cause

MSAL's mTLS-PoP cache identity is CoreHelpers.ComputeX5tS256KeyId, which hashes the public key:

var publicKey = certificate.GetPublicKey();
byte[] hash = sha256.ComputeHash(publicKey);   // public-key hash

ClientCredentialRequest.ShouldUseCachedToken uses this KeyId as a "did the certificate change?" gate — if the current certificate's KeyId differs from the cached token's KeyId, it bypasses the cache and re-mints. The intent is correct, but the identity is wrong.

ESTS/MSS bind to the x5t#S256 over the DER certificate — which MSAL already computes correctly for the wire in JsonWebToken (GetCertHash(SHA256) / SHA256(RawData)). On a same-key renewal (cert 1 → cert 2, identical public key, new DER/serial), the public-key hash is unchanged, so the gate concludes "same certificate, reuse" and serves the stale cert-1-bound token.

Walkthrough

T0: MSI cert = cert 1.  Leg 2 PoP token minted, bound to cert 1, cached (KeyId = pubkey-hash(cert 1)).
T1: MSI cert renews same-key -> cert 2.  Leg 1's MSI cache re-mints token+cert together -> cert 2.
T2 (failing call):
    Leg 1 -> cache hit -> cert 2   ............ wire cert = cert 2 (current)
    Leg 2 -> cache hit -> old PoP token ....... cnf       = cert 1 (stale)
            (lookup KeyId pubkey-hash(cert 2) == pubkey-hash(cert 1) -> HIT, not evicted)
    Server: wire cert 2 != token cnf cert 1 -> AADSTS500181

Both legs are cache hits (matching the log). The MSI cache (Leg 1) correctly rotated to cert 2; the CCA cache (Leg 2) failed to notice the same-key rotation and kept serving the cert-1-bound token.

Fix

Derive the cache KeyId from the certificate DER:

- byte[] hash = sha256.ComputeHash(certificate.GetPublicKey());   // public key
+ byte[] hash = sha256.ComputeHash(certificate.RawData);          // DER (x5t#S256)

Now cert 1 and cert 2 have different KeyIds → the ShouldUseCachedToken gate fires → cache bypass → the PoP token is re-minted bound to cert 2 → wire certificate and token cnf match.

Wire-safe: this KeyId is used only as a cache/lookup identity; it is never sent on the wire (the wire x5t#S256 already uses the DER). The only runtime effect is a one-time cache miss when a certificate is renewed.

Alternatives considered and ruled out

• Double delegate-invocation — the assertion delegate returns a self-consistent (assertion, cert) pair; not the cause.
• Proactive-refresh "cert rotates mid-flow" race — AuthenticationResult.BindingCertificate is a plain snapshot property. A background refresh mints a new certificate into RuntimeMtlsBindingCertificate and its own result; it never repoints an already-returned result. Verified by a dedicated test (below).
• Changing the certificate renewal timing (e.g., half-life instead of 24h) — the renewal is the trigger, not the defect; changing it only changes how often a rotation happens (more often, if anything). The defect is the cache identity, fixed here.

Tests

• MtlsPop_SameKeyCertRenewal_MustNotServeStaleCachedTokenAsync — the proving test. Builds two certificates sharing one RSA key pair but differing in DER, mints a PoP token bound to cert A, then presents cert B. Fails on the old code (second acquisition returns TokenSource.Cache — the stale cert-A token) and passes with the fix (TokenSource.IdentityProvider, re-bound to cert B).
• mTLSPop_ProactiveRefresh_RotatesCert_ServedResultStaysConsistentAsync — documents that a cache-hit result served alongside a background proactive refresh that rotates the binding certificate stays a consistent (token, BindingCertificate) pair, and the served BindingCertificate reference is never repointed to the newly-minted certificate (asserted after the rotation completes, by thumbprint and ReferenceEquals).

Full unit suite: 2237 passed, 0 failed.

Changes

• CoreHelpers.ComputeX5tS256KeyId — hash certificate.RawData (DER) instead of GetPublicKey().
• ClientCredentialRequest — cache-match diagnostics updated to reflect DER-based key identity (behavior unchanged; it calls the corrected helper).
• MtlsPopTests — new proving test + ComputeExpectedKeyId helper updated to DER.
• ImdsV2Tests — new proactive-refresh consistency test.
• TestCommon.WithRefreshOn — preserve AdditionalCacheKeyComponents (matching WithExpiresOn), fixing duplicate cache entries for PoP/extended tokens.

Commits

1. Failing proving test (the problem).
2. The DER-key fix.
3. Proactive-refresh consistency test + WithRefreshOn helper fix.

[Copilot]

Pull request overview

This PR fixes an mTLS Proof-of-Possession (PoP) token-cache invalidation bug by aligning MSAL’s cache “certificate identity” with what ESTS/MSS actually binds to (x5t#S256 over the certificate DER), preventing stale PoP tokens from being served after same-key certificate renewals.

Changes:

• Update CoreHelpers.ComputeX5tS256KeyId to hash X509Certificate2.RawData (DER) instead of the public key.
• Adjust CCA cache-match diagnostics to reflect the DER-based identity.
• Add/adjust unit tests to prove the same-key renewal scenario and to document proactive-refresh behavior, plus a helper fix to preserve AdditionalCacheKeyComponents when mutating RefreshOn.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
tests/Microsoft.Identity.Test.Unit/PublicApiTests/MtlsPopTests.cs	Updates KeyId expectation to DER hash and adds a regression test for same-key cert renewal to ensure cache bypass/re-mint.
tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/ImdsV2Tests.cs	Adds a proactive-refresh + cert-rotation invariant test to show served results remain self-consistent during background refresh.
tests/Microsoft.Identity.Test.Common/TestCommon.cs	Fixes WithRefreshOn test helper to preserve AdditionalCacheKeyComponents, avoiding unintended cache-key changes/duplication in tests.
src/client/Microsoft.Identity.Client/Utils/CoreHelpers.cs	Implements DER-based KeyId computation (x5t#S256) for mTLS PoP cache identity.
src/client/Microsoft.Identity.Client/Internal/Requests/ClientCredentialRequest.cs	Updates verbose logging text for the KeyId/certificate cache gate to reflect the DER-based identity.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.