Detect orphaned KG certs via public key modulus comparison

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}");
             }
         }
+
+        /// <summary>
+        /// Returns <see langword="true"/> 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.
+        /// </summary>
+        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;
+            }
+        }
+
+        /// <summary>
+        /// Returns <see langword="true"/> if the public key exported from <paramref name="containerKey"/>
+        /// matches the public key embedded in <paramref name="cert"/>.
+        /// A mismatch means the container holds different key material than when the cert was issued.
+        /// </summary>
+        /// <remarks>
+        /// Check 3 from the original proposal — comparing the CNG container's
+        /// <c>NCRYPT_LAST_MODIFIED_PROPERTY</c> against the cert's <c>NotBefore</c> — 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.
+        /// </remarks>
+        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;
+            }
+        }
     }
 }

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,6 +33,16 @@ namespace Microsoft.Identity.Client.ManagedIdentity.V2
     internal sealed class WindowsPersistentCertificateCache : IPersistentCertificateCache
     {
         public bool Read(string alias, out CertificateCacheValue value, ILoggerAdapter logger)
+        {
+            return Read(alias, out value, logger, MtlsBindingCache.IsCertKeyOrphaned);
+        }
+
+        // Internal overload for testability: accepts an injectable orphan-check delegate.
+        internal bool Read(
+            string alias,
+            out CertificateCacheValue value,
+            ILoggerAdapter logger,
+            Func<X509Certificate2, ILoggerAdapter, bool> isOrphaned)
         {
             value = default;
 
@@ -56,6 +67,7 @@ public bool Read(string alias, out CertificateCacheValue value, ILoggerAdapter l
                 X509Certificate2 best = null;
                 string bestEndpoint = null;
                 DateTime bestNotAfter = DateTime.MinValue;
+                List<string> orphanedThumbprints = null;
 
                 foreach (var candidate in items)
                 {
@@ -84,6 +96,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<string>();
+                            orphanedThumbprints.Add(candidate.Thumbprint);
+                            continue;
+                        }
+
                         if (candidate.NotAfter > bestNotAfter)
                         {
                             best?.Dispose();
@@ -98,6 +121,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 +160,50 @@ public bool Read(string alias, out CertificateCacheValue value, ILoggerAdapter l
             return false;
         }
 
+        private static void RemoveByThumbprints(string alias, List<string> thumbprints, ILoggerAdapter logger)
+        {
+            try
+            {
+                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] Orphan-removal snapshot via CopyTo failed; falling back to enumeration. Details: " + ex.Message);
+                    items = store.Certificates.Cast<X509Certificate2>().ToArray();
+                }
+
+                int removed = 0;
+                foreach (var cert in items)
+                {
+                    try
+                    {
+                        if (thumbprints.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);
+            }
+        }
+
         public void Write(string alias, X509Certificate2 cert, string endpointBase, ILoggerAdapter logger)
         {
             if (cert == null)

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<ILoggerAdapter>();
+
+            // 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.Read(alias, out _, Logger, (_, __) => true);
+
+                // 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
     }
 }