diff --git a/src/client/Microsoft.Identity.Client/ManagedIdentity/KeyProviders/WindowsCngKeyOperations.cs b/src/client/Microsoft.Identity.Client/ManagedIdentity/KeyProviders/WindowsCngKeyOperations.cs index d672689d43..2763781382 100644 --- a/src/client/Microsoft.Identity.Client/ManagedIdentity/KeyProviders/WindowsCngKeyOperations.cs +++ b/src/client/Microsoft.Identity.Client/ManagedIdentity/KeyProviders/WindowsCngKeyOperations.cs @@ -3,6 +3,7 @@ using System; using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; using Microsoft.Identity.Client.Core; using Microsoft.Identity.Client.Internal; @@ -29,6 +30,12 @@ internal static class WindowsCngKeyOperations private const string KeyGuardVirtualIsoProperty = "Virtual Iso"; private const string VbsNotAvailable = "VBS key isolation is not available"; + // Issuer used by IMDSv2 mTLS PoP binding certificates. Matched as a case-insensitive + // substring against the certificate's Issuer DN, so any cert in CurrentUser\My issued + // by IMDSv2 can be wiped when we mint a fresh KeyGuard key (the previously persisted + // certs are bound to the now-replaced key by name and would fail the mTLS handshake). + internal const string ManagedIdentityIssuerCnFragment = "managedidentitysnissuer.login.microsoft.com"; + // KeyGuard + per-boot flags private const CngKeyCreationOptions NCryptUseVirtualIsolationFlag = (CngKeyCreationOptions)0x00020000; private const CngKeyCreationOptions NCryptUsePerBootKeyFlag = (CngKeyCreationOptions)0x00040000; @@ -66,16 +73,75 @@ public static bool TryGetOrCreateKeyGuard(ILoggerAdapter logger, out RSA rsa) CngKey key; try { + logger?.Info(() => $"[MI][WinKeyProvider] Attempting to open existing KeyGuard key. " + + $"Provider='{SoftwareKspName}', KeyName='{KeyGuardKeyName}', Scope=UserKey, Silent=true."); + key = CngKey.Open( KeyGuardKeyName, new CngProvider(SoftwareKspName), CngKeyOpenOptions.UserKey | CngKeyOpenOptions.Silent); + + logger?.Info(() => $"[MI][WinKeyProvider] CngKey.Open succeeded for '{KeyGuardKeyName}'. " + + "Running liveness sign probe to detect stale per-boot key material " + + "(metadata file can survive a reboot while the VBS-isolated key material is destroyed)."); + + // Liveness probe: per-boot KeyGuard keys (NCryptUsePerBootKeyFlag) leave a stale + // metadata file on disk after reboot. CngKey.Open returns a handle, but the actual + // VBS-protected key material is gone, so the first real sign operation fails. + // Detect this here so we can recreate cleanly instead of failing later in the + // mTLS handshake or signing path. + if (!CanSign(key, logger)) + { + logger?.Info(() => "[MI][WinKeyProvider] KeyGuard liveness sign probe FAILED. " + + "Treating handle as stale (likely post-reboot per-boot key reaped). " + + "Disposing stale handle and recreating fresh KeyGuard key."); + key.Dispose(); + key = CreateFresh(logger); + + if (key == null) + { + logger?.Info(() => "[MI][WinKeyProvider] CreateFresh returned null after failed liveness probe " + + "(VBS unavailable). KeyGuard path will be skipped."); + } + else + { + logger?.Info(() => "[MI][WinKeyProvider] Fresh KeyGuard key created successfully after stale handle replacement. " + + "Purging persisted IMDSv2 mTLS binding certificates that were bound to the replaced key."); + + // The new KeyGuard key reuses the container name 'KeyGuardRSAKey', but its + // public/private pair is different from the one any persisted cert was issued + // against. Wipe all certs in CurrentUser\My issued by IMDSv2 so the next request + // mints fresh instead of failing the mTLS handshake. + PurgeManagedIdentityCertificates(logger); + } + } + else + { + logger?.Info(() => "[MI][WinKeyProvider] KeyGuard liveness sign probe PASSED. Reusing existing handle."); + } } - catch (CryptographicException) + catch (CryptographicException openEx) { // Not found -> create fresh (helper may return null if VBS unavailable) - logger?.Info(() => "[MI][WinKeyProvider] CredentialGuard key not found; creating fresh."); + logger?.Info(() => $"[MI][WinKeyProvider] CngKey.Open threw CryptographicException for '{KeyGuardKeyName}'. " + + $"HR=0x{openEx.HResult:X8}, Message='{openEx.Message}'. " + + "Treating as 'key not found' and creating fresh."); key = CreateFresh(logger); + + if (key == null) + { + logger?.Info(() => "[MI][WinKeyProvider] CreateFresh returned null after Open failure (VBS unavailable)."); + } + else + { + logger?.Info(() => "[MI][WinKeyProvider] Fresh KeyGuard key created successfully after Open failure. " + + "Purging persisted IMDSv2 mTLS binding certificates that were bound to the replaced key."); + + // Same rationale as the probe-failed branch: any persisted IMDSv2 cert in + // CurrentUser\My is bound to the previous KeyGuard key and will fail the mTLS + // handshake. Wipe them so the next request mints fresh. + PurgeManagedIdentityCertificates(logger); + } } // If VBS is unavailable, CreateFresh() returns null. Bail out cleanly. @@ -277,6 +343,178 @@ public static bool IsKeyGuardProtected(CngKey key) return val?.Length > 0 && val[0] != 0; } + /// + /// Performs a small RSA sign operation against the supplied CNG key to verify the + /// underlying key material is actually usable. + /// + /// The CNG key handle returned from . + /// Logger for diagnostic output. + /// + /// if the key signs successfully; otherwise . + /// + /// + /// + /// KeyGuard keys created with NCryptUsePerBootKeyFlag have their VBS-isolated + /// key material destroyed on every reboot, but the on-disk metadata file produced by the + /// Microsoft Software KSP often survives. As a result, + /// can return a handle that looks valid (correct algorithm, "Virtual Iso" property still set) + /// but whose first real cryptographic operation throws. + /// + /// + /// Probing with a one-byte sign here surfaces that condition cheaply (~1-3 ms for RSA-2048) + /// on the cold-start path. Subsequent calls reuse the cached key in + /// WindowsManagedIdentityKeyProvider, so the probe runs at most once per process. + /// + /// + private static bool CanSign(CngKey key, ILoggerAdapter logger) + { + try + { + logger?.Verbose(() => "[MI][WinKeyProvider] Liveness probe: attempting RSA-SHA256 sign of 1-byte payload."); + + using (var rsa = new RSACng(key)) + { + _ = rsa.SignData( + new byte[] { 0 }, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1); + } + + logger?.Verbose(() => "[MI][WinKeyProvider] Liveness probe: sign succeeded; key material is live."); + return true; + } + catch (CryptographicException ex) + { + logger?.Info(() => $"[MI][WinKeyProvider] Liveness probe: sign threw CryptographicException. " + + $"HR=0x{ex.HResult:X8}, Message='{ex.Message}'. Key handle is stale."); + return false; + } + catch (Exception ex) + { + logger?.Info(() => $"[MI][WinKeyProvider] Liveness probe: sign threw unexpected exception. " + + $"{ex.GetType().Name}: '{ex.Message}'. Treating as stale."); + return false; + } + } + + /// + /// Deletes every certificate in the CurrentUser\My store whose issuer matches the + /// IMDSv2 mTLS PoP binding-certificate issuer. + /// + /// Logger for diagnostic output. + /// + /// + /// IMDSv2 binding certificates are issued by + /// CN=managedidentitysnissuer.login.microsoft.com and stored in the user's personal + /// store. They reference the private key by KSP container name (KeyGuardRSAKey), + /// not by key material. When the KeyGuard key is re-minted (post-reboot, or after a failed + /// liveness probe), the new key reuses the same container name but with different + /// public/private parameters — leaving the persisted certs bound to a key that no longer + /// matches them, which then fails the mTLS handshake. + /// + /// + /// Purging the store at the moment we mint a fresh KeyGuard key eliminates the + /// failed-handshake + retry round trip that the SChannel-error catch in + /// ImdsV2ManagedIdentitySource.AuthenticateAsync would otherwise have to recover from. + /// + /// + /// All store I/O is best-effort and non-throwing. + /// + /// + internal static void PurgeManagedIdentityCertificates(ILoggerAdapter logger) + { + int removed = 0; + int inspected = 0; + + try + { + logger?.Info(() => + $"[MI][WinKeyProvider] PurgeManagedIdentityCertificates: opening CurrentUser\\My to remove " + + $"certs whose Issuer contains '{ManagedIdentityIssuerCnFragment}'."); + + using (var store = new X509Store(StoreName.My, StoreLocation.CurrentUser)) + { + store.Open(OpenFlags.ReadWrite); + + // Snapshot to avoid 'collection modified during enumeration' provider quirks. + var snapshot = new X509Certificate2[store.Certificates.Count]; + try + { + store.Certificates.CopyTo(snapshot, 0); + } + catch (Exception copyEx) + { + logger?.Info(() => + $"[MI][WinKeyProvider] PurgeManagedIdentityCertificates: store snapshot via CopyTo failed " + + $"({copyEx.GetType().Name}: {copyEx.Message}). Falling back to enumeration."); + + int i = 0; + snapshot = new X509Certificate2[store.Certificates.Count]; + foreach (X509Certificate2 c in store.Certificates) + { + snapshot[i++] = c; + } + } + + foreach (X509Certificate2 candidate in snapshot) + { + if (candidate is null) + { + // Defensive: snapshot slot may be null if the store enumeration + // yielded fewer items than Certificates.Count reported (TOCTOU). + continue; + } + + try + { + inspected++; + + string issuer = candidate.Issuer ?? string.Empty; + if (issuer.IndexOf(ManagedIdentityIssuerCnFragment, StringComparison.OrdinalIgnoreCase) < 0) + { + continue; + } + + string thumb = candidate.Thumbprint; + DateTime notAfter = candidate.NotAfter; + + try + { + store.Remove(candidate); + removed++; + logger?.Info(() => + $"[MI][WinKeyProvider] PurgeManagedIdentityCertificates: removed cert. " + + $"Thumbprint={thumb}, NotAfter={notAfter:O}, Issuer='{issuer}'."); + } + catch (Exception removeEx) + { + logger?.Info(() => + $"[MI][WinKeyProvider] PurgeManagedIdentityCertificates: failed to remove cert " + + $"Thumbprint={thumb}. {removeEx.GetType().Name}: '{removeEx.Message}'."); + } + } + finally + { + candidate.Dispose(); + } + } + } + } + catch (Exception ex) + { + logger?.Info(() => + $"[MI][WinKeyProvider] PurgeManagedIdentityCertificates: store access failed. " + + $"{ex.GetType().Name}: '{ex.Message}'. Removed={removed}, Inspected={inspected}."); + return; + } + + int removedFinal = removed; + int inspectedFinal = inspected; + logger?.Info(() => + $"[MI][WinKeyProvider] PurgeManagedIdentityCertificates: complete. " + + $"Removed={removedFinal}, Inspected={inspectedFinal}."); + } + /// /// Determines whether a cryptographic exception indicates that VBS is unavailable. /// diff --git a/src/client/Microsoft.Identity.Client/ManagedIdentity/V2/MtlsCertificateCache.cs b/src/client/Microsoft.Identity.Client/ManagedIdentity/V2/MtlsCertificateCache.cs index 538c0fd249..f7f128389c 100644 --- a/src/client/Microsoft.Identity.Client/ManagedIdentity/V2/MtlsCertificateCache.cs +++ b/src/client/Microsoft.Identity.Client/ManagedIdentity/V2/MtlsCertificateCache.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Concurrent; +using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using System.Threading; using System.Threading.Tasks; @@ -185,5 +186,87 @@ public void RemoveBadCert(string cacheKey, ILoggerAdapter logger) logger?.Verbose(() => $"[PersistentCert] Error removing from persistent cache: {ex.Message}"); } } + + /// + /// Returns if the cert's embedded public key does not match the + /// public key currently in the associated CNG container, indicating the container was + /// regenerated (e.g. by KeyGuard on reboot) while the cert on disk still references the + /// old key material. + /// + internal static bool IsCertKeyOrphaned(X509Certificate2 cert, ILoggerAdapter logger) + { + if (cert is null) + return true; + + try + { + using var rsaKey = cert.GetRSAPrivateKey(); + if (rsaKey is null) + { + // GetRSAPrivateKey() returns null for non-RSA certs (e.g. ECDSA) AND for RSA + // certs where the private key is inaccessible. Distinguish the two cases: + // if the cert has an RSA public key, the private key should be present but isn't + // → the cert is unusable. If there is no RSA public key, it's a non-RSA cert + // that we can't check → accept on faith. + using var pubKey = cert.GetRSAPublicKey(); + return pubKey is not null; // RSA cert + inaccessible private key = orphaned + } + + if (rsaKey is not RSACng rsaCng) + { + // Non-CNG RSA key (e.g. software CSP) — cannot perform KG container check; accept. + return false; + } + + return !PublicKeyMatchesCert(rsaCng, cert, logger); + } + catch (CryptographicException ex) + { + logger?.Verbose(() => + $"[PersistentCert] Cannot load private key for orphan check: {ex.Message}. Treating cert as unusable."); + return true; + } + } + + /// + /// Returns if the public key exported from + /// matches the public key embedded in . + /// A mismatch means the container holds different key material than when the cert was issued. + /// + /// + /// Check 3 from the original proposal — comparing the CNG container's + /// NCRYPT_LAST_MODIFIED_PROPERTY against the cert's NotBefore — is + /// intentionally omitted. Both Check 3 and this modulus comparison detect the same event: + /// KeyGuard regenerating the key in the container post-reboot. This check is definitive: + /// two independently generated RSA keys sharing a modulus is computationally infeasible, + /// so a mismatch conclusively means the container was regenerated. Check 3 is a heuristic + /// with a known false-negative window (a reboot occurring within one minute of cert + /// issuance), and adds no coverage that this check does not already provide. + /// + internal static bool PublicKeyMatchesCert(RSACng containerKey, X509Certificate2 cert, ILoggerAdapter logger) + { + try + { + var containerParams = containerKey.ExportParameters(includePrivateParameters: false); + using var certPubKey = cert.GetRSAPublicKey(); + if (certPubKey is null) + return false; + + var certParams = certPubKey.ExportParameters(includePrivateParameters: false); + + return containerParams.Modulus is not null + && certParams.Modulus is not null + && containerParams.Modulus.AsSpan().SequenceEqual(certParams.Modulus) + && containerParams.Exponent is not null + && certParams.Exponent is not null + && containerParams.Exponent.AsSpan().SequenceEqual(certParams.Exponent); + } + catch (CryptographicException ex) + { + logger?.Verbose(() => + $"[PersistentCert] Public key export failed during orphan check: {ex.Message}. Treating cert as orphaned."); + return false; + } + } } } diff --git a/src/client/Microsoft.Identity.Client/ManagedIdentity/V2/WindowsPersistentCertificateCache.cs b/src/client/Microsoft.Identity.Client/ManagedIdentity/V2/WindowsPersistentCertificateCache.cs index 2f1ddff3cb..a9b96d65ff 100644 --- a/src/client/Microsoft.Identity.Client/ManagedIdentity/V2/WindowsPersistentCertificateCache.cs +++ b/src/client/Microsoft.Identity.Client/ManagedIdentity/V2/WindowsPersistentCertificateCache.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Collections.Generic; using System.Linq; using System.Security.Cryptography.X509Certificates; using Microsoft.Identity.Client.Core; @@ -32,30 +33,36 @@ namespace Microsoft.Identity.Client.ManagedIdentity.V2 internal sealed class WindowsPersistentCertificateCache : IPersistentCertificateCache { public bool Read(string alias, out CertificateCacheValue value, ILoggerAdapter logger) + { + return TryRead(alias, logger, MtlsBindingCache.IsCertKeyOrphaned, out value); + } + + // Overload for testability: accepts an injectable orphan-check delegate. + public bool TryRead( + string alias, + ILoggerAdapter logger, + Func isOrphaned, + out CertificateCacheValue value) { value = default; + if (isOrphaned is null) + { + throw new ArgumentNullException(nameof(isOrphaned)); + } + try { using var store = new X509Store(StoreName.My, StoreLocation.CurrentUser); store.Open(OpenFlags.OpenExistingOnly | OpenFlags.ReadOnly); // Snapshot first to avoid provider quirks ("collection modified" during enumeration). - X509Certificate2[] items; - try - { - items = new X509Certificate2[store.Certificates.Count]; - store.Certificates.CopyTo(items, 0); - } - catch (Exception ex) - { - logger.Verbose(() => "[PersistentCert] Store snapshot via CopyTo failed; falling back to enumeration. Details: " + ex.Message); - items = store.Certificates.Cast().ToArray(); - } + var items = SnapshotCertificates(store, logger); X509Certificate2 best = null; string bestEndpoint = null; DateTime bestNotAfter = DateTime.MinValue; + List orphanedThumbprints = null; foreach (var candidate in items) { @@ -84,6 +91,17 @@ public bool Read(string alias, out CertificateCacheValue value, ILoggerAdapter l continue; } + // Skip certs whose CNG container key no longer matches the cert's public key. + // This detects orphaned certs left on disk after a reboot regenerates the KG per-boot key. + // Collect thumbprints for post-loop removal so the store is only opened once for writes. + if (isOrphaned(candidate, logger)) + { + logger.Verbose(() => "[PersistentCert] Candidate skipped: CNG container key does not match cert public key (orphaned post-reboot)."); + orphanedThumbprints ??= new List(); + orphanedThumbprints.Add(candidate.Thumbprint); + continue; + } + if (candidate.NotAfter > bestNotAfter) { best?.Dispose(); @@ -98,6 +116,12 @@ public bool Read(string alias, out CertificateCacheValue value, ILoggerAdapter l } } + // Remove any orphaned certs discovered during the scan. + if (orphanedThumbprints is { Count: > 0 }) + { + RemoveByThumbprints(alias, orphanedThumbprints, logger); + } + if (best != null) { // CN (GUID) → canonical client_id @@ -131,6 +155,49 @@ public bool Read(string alias, out CertificateCacheValue value, ILoggerAdapter l return false; } + private static void RemoveByThumbprints(string alias, List thumbprints, ILoggerAdapter logger) + { + var thumbprintSet = new HashSet(thumbprints, StringComparer.OrdinalIgnoreCase); + + InterprocessLock.TryWithAliasLock( + alias, + timeout: TimeSpan.FromMilliseconds(300), + action: () => + { + try + { + using var store = new X509Store(StoreName.My, StoreLocation.CurrentUser); + store.Open(OpenFlags.ReadWrite); + + var items = SnapshotCertificates(store, logger); + int removed = 0; + + foreach (var cert in items) + { + try + { + if (thumbprintSet.Contains(cert.Thumbprint)) + { + store.Remove(cert); + removed++; + } + } + finally + { + cert.Dispose(); + } + } + + logger.Verbose(() => $"[PersistentCert] Removed {removed} orphaned cert(s) for alias '{alias}'."); + } + catch (Exception ex) + { + logger.Verbose(() => "[PersistentCert] Orphan removal failed (best-effort): " + ex.Message); + } + }, + logVerbose: s => logger.Verbose(() => s)); + } + public void Write(string alias, X509Certificate2 cert, string endpointBase, ILoggerAdapter logger) { if (cert == null) @@ -168,17 +235,7 @@ public void Write(string alias, X509Certificate2 cert, string endpointBase, ILog // Skip write if a newer/equal, non-expired binding for this alias already exists. DateTime newestForAliasUtc = DateTime.MinValue; - X509Certificate2[] present; - try - { - present = new X509Certificate2[store.Certificates.Count]; - store.Certificates.CopyTo(present, 0); - } - catch (Exception ex) - { - logger.Verbose(() => "[PersistentCert] Store snapshot via CopyTo failed; falling back to enumeration. Details: " + ex.Message); - present = store.Certificates.Cast().ToArray(); - } + var present = SnapshotCertificates(store, logger); foreach (var existing in present) { @@ -283,18 +340,7 @@ public void DeleteAllForAlias(string alias, ILoggerAdapter logger) using var store = new X509Store(StoreName.My, StoreLocation.CurrentUser); store.Open(OpenFlags.ReadWrite); - X509Certificate2[] items; - try - { - items = new X509Certificate2[store.Certificates.Count]; - store.Certificates.CopyTo(items, 0); - } - catch (Exception ex) - { - logger.Verbose(() => "[PersistentCert] Store snapshot via CopyTo failed; falling back to enumeration. Details: " + ex.Message); - items = store.Certificates.Cast().ToArray(); - } - + var items = SnapshotCertificates(store, logger); int removed = 0; foreach (var existing in items) @@ -326,28 +372,35 @@ public void DeleteAllForAlias(string alias, ILoggerAdapter logger) } /// - /// Deletes only certificates that are actually expired (NotAfter < nowUtc), - /// scoped to the given alias (cache key) via FriendlyName. + /// Safely snapshots all certificates in into an array. + /// Falls back to LINQ enumeration if CopyTo throws (some providers don't support it). /// - private static void PruneExpiredForAlias( - X509Store store, - string aliasCacheKey, - DateTime nowUtc, - ILoggerAdapter logger) + private static X509Certificate2[] SnapshotCertificates(X509Store store, ILoggerAdapter logger) { - X509Certificate2[] items; try { - items = new X509Certificate2[store.Certificates.Count]; - // Safe snapshot for providers that throw if removing while iterating + var items = new X509Certificate2[store.Certificates.Count]; store.Certificates.CopyTo(items, 0); + return items; } catch (Exception ex) { - logger.Verbose(() => "[PersistentCert] Prune snapshot via CopyTo failed; falling back to enumeration. Details: " + ex.Message); - items = store.Certificates.Cast().ToArray(); + logger.Verbose(() => "[PersistentCert] Store snapshot via CopyTo failed; falling back to enumeration. Details: " + ex.Message); + return store.Certificates.Cast().ToArray(); } + } + /// + /// Deletes only certificates that are actually expired (NotAfter < nowUtc), + /// scoped to the given alias (cache key) via FriendlyName. + /// + private static void PruneExpiredForAlias( + X509Store store, + string aliasCacheKey, + DateTime nowUtc, + ILoggerAdapter logger) + { + var items = SnapshotCertificates(store, logger); int removed = 0; foreach (var existing in items) diff --git a/tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/PersistentCertificateStoreUnitTests.cs b/tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/PersistentCertificateStoreUnitTests.cs index 4777a88a10..12d43e053f 100644 --- a/tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/PersistentCertificateStoreUnitTests.cs +++ b/tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/PersistentCertificateStoreUnitTests.cs @@ -843,5 +843,136 @@ private static bool WaitForAliasCount(string alias, int expected, int retries = } #endregion + + #region IsCertKeyOrphaned tests + + [TestMethod] + public void IsCertKeyOrphaned_ReturnsTrue_For_NullCert() + { + // Arrange (no setup needed) + + // Act + bool result = MtlsBindingCache.IsCertKeyOrphaned(null, null); + + // Assert + Assert.IsTrue(result, "Null cert should be treated as orphaned."); + } + + [TestMethod] + public void IsCertKeyOrphaned_ReturnsFalse_For_ValidCert() + { + WindowsOnly(); + + // Arrange - cert whose private key in the CNG container matches the cert's embedded public key + using var cert = CreateSelfSignedCert(TimeSpan.FromDays(14), "CN=ValidCertOrphanTest"); + var logger = Substitute.For(); + + // Act + bool result = MtlsBindingCache.IsCertKeyOrphaned(cert, logger); + + // Assert + Assert.IsFalse(result, "A cert whose private key matches its embedded public key should not be considered orphaned."); + } + + [TestMethod] + public void IsCertKeyOrphaned_ReturnsTrue_For_RsaCert_WithNoPrivateKey() + { + // Arrange - public-only RSA cert (no private key associated) + using var rsa = RSA.Create(2048); + var req = new System.Security.Cryptography.X509Certificates.CertificateRequest( + new X500DistinguishedName("CN=NoPrivKeyTest"), + rsa, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1); + // Create a cert without associating the private key (CopyWithPrivateKey not called) + using var cert = req.CreateSelfSigned(DateTimeOffset.UtcNow.AddMinutes(-1), DateTimeOffset.UtcNow.AddDays(14)); + using var publicOnlyCert = new X509Certificate2(cert.Export(X509ContentType.Cert)); + + // Act — public-only RSA cert: GetRSAPrivateKey() returns null, GetRSAPublicKey() succeeds + bool result = MtlsBindingCache.IsCertKeyOrphaned(publicOnlyCert, null); + + // Assert + Assert.IsTrue(result, "An RSA cert with no accessible private key should be treated as orphaned."); + } + + [TestMethod] + public void PublicKeyMatchesCert_ReturnsTrue_When_KeyMatchesCert() + { + WindowsOnly(); + + // Arrange - cert created with key1; pass key1 as the container key + using var key1 = new RSACng(2048); + + var req = new System.Security.Cryptography.X509Certificates.CertificateRequest( + new X500DistinguishedName("CN=KeyMatchTest"), + key1, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1); + using var cert = req.CreateSelfSigned(DateTimeOffset.UtcNow.AddMinutes(-1), DateTimeOffset.UtcNow.AddDays(14)); + + // Act + bool result = MtlsBindingCache.PublicKeyMatchesCert(key1, cert, null); + + // Assert + Assert.IsTrue(result, "The key used to create the cert should match the cert's embedded public key."); + } + + [TestMethod] + public void PublicKeyMatchesCert_ReturnsFalse_When_ModulusMismatch() + { + WindowsOnly(); + + // Arrange - cert created with key1, but we pass key2 as the container key + // (simulates post-reboot KG regeneration: same container, new key material) + using var key1 = new RSACng(2048); + using var key2 = new RSACng(2048); + + var req = new System.Security.Cryptography.X509Certificates.CertificateRequest( + new X500DistinguishedName("CN=KeyMismatchTest"), + key1, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1); + using var cert = req.CreateSelfSigned(DateTimeOffset.UtcNow.AddMinutes(-1), DateTimeOffset.UtcNow.AddDays(14)); + + // Act + bool result = MtlsBindingCache.PublicKeyMatchesCert(key2, cert, null); + + // Assert + Assert.IsFalse(result, "A different key than the one used to create the cert should produce a modulus mismatch."); + } + + [TestMethod] + public void Read_RemovesOrphanedCert_FromStore() + { + WindowsOnly(); + + // Arrange + var alias = "alias-orphan-remove-" + Guid.NewGuid().ToString("N"); + var ep = "https://fake_mtls/orphan"; + var guid = Guid.NewGuid().ToString("D"); + + try + { + using var cert = CreateSelfSignedWithKey("CN=" + guid, TimeSpan.FromDays(14)); + _cache.Write(alias, cert, ep, Logger); + + // Verify cert is in the store + Assert.AreEqual(1, CountAliasInStore(alias), "Cert should be in store after Write."); + + // Act — use the injectable overload: treat every cert as orphaned + var concreteCache = (WindowsPersistentCertificateCache)_cache; + bool readResult = concreteCache.TryRead(alias, Logger, (_, __) => true, out _); + + // Assert + Assert.IsFalse(readResult, "Read should return false when all candidates are orphaned."); + Assert.IsTrue(WaitForAliasCount(alias, 0), "Orphaned cert should have been removed from the store."); + } + finally + { + RemoveAliasFromStore(alias); + } + } + + #endregion } } diff --git a/tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/WindowsCngKeyOperationsPurgeUnitTests.cs b/tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/WindowsCngKeyOperationsPurgeUnitTests.cs new file mode 100644 index 0000000000..93340f9978 --- /dev/null +++ b/tests/Microsoft.Identity.Test.Unit/ManagedIdentityTests/WindowsCngKeyOperationsPurgeUnitTests.cs @@ -0,0 +1,359 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Linq; +using System.Runtime.InteropServices; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using Microsoft.Identity.Client.Core; +using Microsoft.Identity.Client.ManagedIdentity.KeyProviders; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using NSubstitute; + +namespace Microsoft.Identity.Test.Unit.ManagedIdentityTests +{ + /// + /// Tests for . + /// The purge sweeps CurrentUser\My and removes every certificate whose issuer + /// contains . + /// It runs after a fresh KeyGuard key is minted so that persisted IMDSv2 binding + /// certs (which are bound by container name to the now-replaced key) are not left + /// behind to fail the next mTLS handshake. + /// + [TestClass] + public class WindowsCngKeyOperationsPurgeUnitTests + { + // Discriminator we plant in each test cert's Subject so cleanup can find leftovers + // from a failed run without touching unrelated certs in the developer's store. + private const string TestSubjectDiscriminatorPrefix = "MSAL-Purge-Test-"; + + private static bool IsWindows => RuntimeInformation.IsOSPlatform(OSPlatform.Windows); + + private static ILoggerAdapter Logger => Substitute.For(); + + [TestInitialize] + public void Init() + { + // Reuse the existing broad sweep so prior test runs don't leak state. + if (ImdsV2TestStoreCleaner.IsWindows) + { + ImdsV2TestStoreCleaner.RemoveAllTestArtifacts(); + } + + // Also remove any leftover purge-test certs from a previous failed run. + RemoveAllPurgeTestArtifacts(); + } + + [TestCleanup] + public void Cleanup() + { + RemoveAllPurgeTestArtifacts(); + } + + private static void WindowsOnly() + { + if (!IsWindows) + { + Assert.Inconclusive("Windows-only"); + } + } + + [TestMethod] + public void PurgeManagedIdentityCertificates_RemovesCertWithMatchingIssuer() + { + WindowsOnly(); + + // Arrange + string discriminator = TestSubjectDiscriminatorPrefix + Guid.NewGuid().ToString("N"); + string subject = + "CN=" + WindowsCngKeyOperations.ManagedIdentityIssuerCnFragment + + ", OU=" + discriminator; + + string plantedThumbprint; + using (var cert = CreateSelfSignedWithKey(subject, TimeSpan.FromDays(2))) + { + plantedThumbprint = cert.Thumbprint; + AddToCurrentUserMyStore(cert); + } + + Assert.IsTrue( + IsInCurrentUserMyStore(plantedThumbprint), + "Test setup precondition: planted cert must be present in CurrentUser\\My before purge."); + + // Act + WindowsCngKeyOperations.PurgeManagedIdentityCertificates(Logger); + + // Assert + Assert.IsFalse( + IsInCurrentUserMyStore(plantedThumbprint), + "Purge should remove certs whose Issuer contains the managed identity issuer CN."); + } + + [TestMethod] + public void PurgeManagedIdentityCertificates_LeavesCertWithNonMatchingIssuer() + { + WindowsOnly(); + + // Arrange + string discriminator = TestSubjectDiscriminatorPrefix + Guid.NewGuid().ToString("N"); + // Subject/Issuer that does NOT contain the managed identity issuer fragment. + string subject = "CN=unrelated.example.test, OU=" + discriminator; + + string plantedThumbprint; + using (var cert = CreateSelfSignedWithKey(subject, TimeSpan.FromDays(2))) + { + plantedThumbprint = cert.Thumbprint; + AddToCurrentUserMyStore(cert); + } + + Assert.IsTrue( + IsInCurrentUserMyStore(plantedThumbprint), + "Test setup precondition: planted cert must be present in CurrentUser\\My before purge."); + + try + { + // Act + WindowsCngKeyOperations.PurgeManagedIdentityCertificates(Logger); + + // Assert + Assert.IsTrue( + IsInCurrentUserMyStore(plantedThumbprint), + "Purge must not remove certs whose Issuer does not contain the managed identity issuer CN."); + } + finally + { + RemoveByThumbprintFromCurrentUserMyStore(plantedThumbprint); + } + } + + [TestMethod] + public void PurgeManagedIdentityCertificates_MatchIsCaseInsensitive() + { + WindowsOnly(); + + // Arrange + string discriminator = TestSubjectDiscriminatorPrefix + Guid.NewGuid().ToString("N"); + // Uppercase the issuer fragment to ensure the match is OrdinalIgnoreCase. + string subject = + "CN=" + WindowsCngKeyOperations.ManagedIdentityIssuerCnFragment.ToUpperInvariant() + + ", OU=" + discriminator; + + string plantedThumbprint; + using (var cert = CreateSelfSignedWithKey(subject, TimeSpan.FromDays(2))) + { + plantedThumbprint = cert.Thumbprint; + AddToCurrentUserMyStore(cert); + } + + Assert.IsTrue( + IsInCurrentUserMyStore(plantedThumbprint), + "Test setup precondition: planted cert must be present in CurrentUser\\My before purge."); + + // Act + WindowsCngKeyOperations.PurgeManagedIdentityCertificates(Logger); + + // Assert + Assert.IsFalse( + IsInCurrentUserMyStore(plantedThumbprint), + "Purge issuer match should be case-insensitive."); + } + + [TestMethod] + public void PurgeManagedIdentityCertificates_OnlyRemovesMatching_LeavesOtherCertsAlone() + { + WindowsOnly(); + + // Arrange: plant one matching and one non-matching cert + string matchDiscriminator = TestSubjectDiscriminatorPrefix + Guid.NewGuid().ToString("N"); + string nonMatchDiscriminator = TestSubjectDiscriminatorPrefix + Guid.NewGuid().ToString("N"); + + string matchingSubject = + "CN=" + WindowsCngKeyOperations.ManagedIdentityIssuerCnFragment + + ", OU=" + matchDiscriminator; + string nonMatchingSubject = "CN=unrelated.example.test, OU=" + nonMatchDiscriminator; + + string matchingThumb; + string nonMatchingThumb; + + using (var matching = CreateSelfSignedWithKey(matchingSubject, TimeSpan.FromDays(2))) + using (var nonMatching = CreateSelfSignedWithKey(nonMatchingSubject, TimeSpan.FromDays(2))) + { + matchingThumb = matching.Thumbprint; + nonMatchingThumb = nonMatching.Thumbprint; + + AddToCurrentUserMyStore(matching); + AddToCurrentUserMyStore(nonMatching); + } + + Assert.IsTrue(IsInCurrentUserMyStore(matchingThumb), "Matching cert must be planted."); + Assert.IsTrue(IsInCurrentUserMyStore(nonMatchingThumb), "Non-matching cert must be planted."); + + try + { + // Act + WindowsCngKeyOperations.PurgeManagedIdentityCertificates(Logger); + + // Assert + Assert.IsFalse(IsInCurrentUserMyStore(matchingThumb), "Matching cert should be purged."); + Assert.IsTrue(IsInCurrentUserMyStore(nonMatchingThumb), "Non-matching cert should survive."); + } + finally + { + RemoveByThumbprintFromCurrentUserMyStore(nonMatchingThumb); + } + } + + // ---------------- helpers ---------------- + + /// + /// Creates a self-signed RSA cert with a persistable private key. + /// Mirrors the pattern used by PersistentCertificateStoreUnitTests.CreateSelfSignedWithKey. + /// + private static X509Certificate2 CreateSelfSignedWithKey(string subject, TimeSpan lifetime) + { + using var rsa = RSA.Create(2048); + + var req = new CertificateRequest( + new X500DistinguishedName(subject), + rsa, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1); + + DateTimeOffset notBefore = DateTimeOffset.UtcNow.AddMinutes(-2); + DateTimeOffset notAfter = notBefore.Add(lifetime); + + using var ephemeral = req.CreateSelfSigned(notBefore, notAfter); + + // Re-import as PFX so the private key is persisted and the store will accept it. + var pfx = ephemeral.Export(X509ContentType.Pfx, ""); + return new X509Certificate2( + pfx, + "", + X509KeyStorageFlags.Exportable | X509KeyStorageFlags.PersistKeySet); + } + + private static void AddToCurrentUserMyStore(X509Certificate2 cert) + { + using var store = new X509Store(StoreName.My, StoreLocation.CurrentUser); + store.Open(OpenFlags.ReadWrite); + store.Add(cert); + } + + private static bool IsInCurrentUserMyStore(string thumbprint) + { + using var store = new X509Store(StoreName.My, StoreLocation.CurrentUser); + store.Open(OpenFlags.OpenExistingOnly | OpenFlags.ReadOnly); + + foreach (X509Certificate2 c in store.Certificates) + { + try + { + if (string.Equals(c.Thumbprint, thumbprint, StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + finally + { + c.Dispose(); + } + } + + return false; + } + + private static void RemoveByThumbprintFromCurrentUserMyStore(string thumbprint) + { + try + { + using var store = new X509Store(StoreName.My, StoreLocation.CurrentUser); + store.Open(OpenFlags.ReadWrite); + + X509Certificate2[] snapshot; + try + { + snapshot = new X509Certificate2[store.Certificates.Count]; + store.Certificates.CopyTo(snapshot, 0); + } + catch + { + snapshot = store.Certificates.Cast().ToArray(); + } + + foreach (var c in snapshot) + { + try + { + if (string.Equals(c.Thumbprint, thumbprint, StringComparison.OrdinalIgnoreCase)) + { + try + { store.Remove(c); } + catch { /* best-effort */ } + } + } + finally + { + c.Dispose(); + } + } + } + catch + { + // best-effort cleanup + } + } + + /// + /// Removes any leftover purge-test certificates from CurrentUser\My. + /// Matches our unique Subject OU discriminator to avoid touching unrelated certs. + /// Best-effort, no-throw. + /// + private static void RemoveAllPurgeTestArtifacts() + { + if (!IsWindows) + { + return; + } + + try + { + using var store = new X509Store(StoreName.My, StoreLocation.CurrentUser); + store.Open(OpenFlags.ReadWrite); + + X509Certificate2[] snapshot; + try + { + snapshot = new X509Certificate2[store.Certificates.Count]; + store.Certificates.CopyTo(snapshot, 0); + } + catch + { + snapshot = store.Certificates.Cast().ToArray(); + } + + foreach (var c in snapshot) + { + try + { + string subject = c.Subject ?? string.Empty; + if (subject.IndexOf(TestSubjectDiscriminatorPrefix, StringComparison.Ordinal) >= 0) + { + try + { store.Remove(c); } + catch { /* best-effort */ } + } + } + finally + { + c.Dispose(); + } + } + } + catch + { + // best-effort + } + } + } +}