Don't use IgnoreRootRevocationUnknown when processing intermediate CA re-key certificates

src/libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs

@@ -157,6 +157,17 @@ internal X509Certificate2 CreateSubordinateCA(
             string subject,
             PublicKey publicKey,
             int? depthLimit = null)
+        {
+            return CreateSubordinateCA(
+                new X500DistinguishedName(subject),
+                publicKey,
+                depthLimit);
+        }
+
+        internal X509Certificate2 CreateSubordinateCA(
+            X500DistinguishedName subject,
+            PublicKey publicKey,
+            int? depthLimit = null)
         {
             return CreateCertificate(
                 subject,
@@ -171,7 +182,22 @@ internal X509Certificate2 CreateSubordinateCA(
                     s_caKeyUsage });
         }
 
-        internal X509Certificate2 CreateEndEntity(string subject, PublicKey publicKey, X509ExtensionCollection extensions)
+        internal X509Certificate2 CreateEndEntity(
+            string subject,
+            PublicKey publicKey,
+            X509ExtensionCollection extensions)
+        {
+            return CreateCertificate(
+                new X500DistinguishedName(subject),
+                publicKey,
+                TimeSpan.FromSeconds(2),
+                extensions);
+        }
+
+        internal X509Certificate2 CreateEndEntity(
+            X500DistinguishedName subject,
+            PublicKey publicKey,
+            X509ExtensionCollection extensions)
         {
             return CreateCertificate(
                 subject,
@@ -254,6 +280,18 @@ private X509Certificate2 CreateCertificate(
             TimeSpan nestingBuffer,
             X509ExtensionCollection extensions,
             bool ocspResponder = false)
+        {
+            X500DistinguishedName name = new(subject);
+
+            return CreateCertificate(name, publicKey, nestingBuffer, extensions, ocspResponder);
+        }
+
+        private X509Certificate2 CreateCertificate(
+            X500DistinguishedName subject,
+            PublicKey publicKey,
+            TimeSpan nestingBuffer,
+            X509ExtensionCollection extensions,
+            bool ocspResponder = false)
         {
             if (_cdpExtension == null && CdpUri != null)
             {
@@ -271,7 +309,7 @@ private X509Certificate2 CreateCertificate(
             }
 
             CertificateRequest request = new CertificateRequest(
-                new X500DistinguishedName(subject),
+                subject,
                 publicKey,
                 HashAlgorithmIfNeeded(_cert.GetKeyAlgorithm()),
                 RSASignaturePadding.Pkcs1);
@@ -807,6 +845,74 @@ internal static void BuildPrivatePki(
             KeyFactory keyFactory = null,
             bool forTls = false,
             X509ExtensionCollection extensions = null)
+        {
+            BuildPrivatePkiCore(
+                pkiOptions,
+                out responder,
+                out rootAuthority,
+                out intermediateAuthorities,
+                out endEntityCert,
+                BuildSubject("A Revocation Test Root", testName, pkiOptions, pkiOptionsInSubject),
+                index => BuildSubject($"A Revocation Test CA {index}", testName, pkiOptions, pkiOptionsInSubject),
+                intermediateAuthorityCount,
+                BuildSubject(subjectName ?? "A Revocation Test Cert", testName, pkiOptions, pkiOptionsInSubject),
+                keyFactoryHashSubjectName: subjectName,
+                testName,
+                registerAuthorities,
+                keyFactory,
+                forTls,
+                extensions);
+        }
+
+        internal static void BuildPrivatePki(
+            PkiOptions pkiOptions,
+            out RevocationResponder responder,
+            X500DistinguishedName rootName,
+            out CertificateAuthority rootAuthority,
+            X500DistinguishedName[] intermediateNames,
+            out CertificateAuthority[] intermediateAuthorities,
+            X500DistinguishedName endEntityName,
+            out X509Certificate2 endEntityCert,
+            string testName = null,
+            bool registerAuthorities = true,
+            KeyFactory keyFactory = null,
+            bool forTls = false,
+            X509ExtensionCollection extensions = null)
+        {
+            BuildPrivatePkiCore(
+                pkiOptions,
+                out responder,
+                out rootAuthority,
+                out intermediateAuthorities,
+                out endEntityCert,
+                rootName,
+                index => intermediateNames[index],
+                intermediateNames.Length,
+                endEntityName: endEntityName,
+                keyFactoryHashSubjectName: null,
+                testName,
+                registerAuthorities,
+                keyFactory,
+                forTls,
+                extensions);
+        }
+
+        private static void BuildPrivatePkiCore(
+            PkiOptions pkiOptions,
+            out RevocationResponder responder,
+            out CertificateAuthority rootAuthority,
+            out CertificateAuthority[] intermediateAuthorities,
+            out X509Certificate2 endEntityCert,
+            X500DistinguishedName rootName,
+            Func<int, X500DistinguishedName> intermediateAuthorityNameFactory,
+            int intermediateAuthorityCount,
+            X500DistinguishedName endEntityName,
+            string keyFactoryHashSubjectName,
+            string testName = null,
+            bool registerAuthorities = true,
+            KeyFactory keyFactory = null,
+            bool forTls = false,
+            X509ExtensionCollection extensions = null)
         {
             bool rootDistributionViaHttp = !pkiOptions.HasFlag(PkiOptions.NoRootCertDistributionUri);
             bool issuerRevocationViaCrl = pkiOptions.HasFlag(PkiOptions.IssuerRevocationViaCrl);
@@ -838,7 +944,7 @@ internal static void BuildPrivatePki(
                     hasher.AppendData(MemoryMarshal.AsBytes(new ReadOnlySpan<PkiOptions>(ref pkiOptions)));
                     hasher.AppendData(MemoryMarshal.AsBytes(new ReadOnlySpan<int>(ref intermediateAuthorityCount)));
                     hasher.AppendData(MemoryMarshal.AsBytes(testName.AsSpan()));
-                    hasher.AppendData(MemoryMarshal.AsBytes(subjectName.AsSpan()));
+                    hasher.AppendData(MemoryMarshal.AsBytes(keyFactoryHashSubjectName.AsSpan()));
 
                     Span<byte> hash = stackalloc byte[256 / 8];
                     int written = hasher.GetCurrentHash(hash);
@@ -854,8 +960,7 @@ internal static void BuildPrivatePki(
             using (KeyHolder rootKey = KeyHolder.CreateKey(keyFactory))
             using (KeyHolder eeKey = KeyHolder.CreateKey(keyFactory))
             {
-                CertificateRequest rootReq = rootKey.CreateRequest(
-                    BuildSubject("A Revocation Test Root", testName, pkiOptions, pkiOptionsInSubject));
+                CertificateRequest rootReq = rootKey.CreateRequest(rootName);
 
                 X509BasicConstraintsExtension caConstraints =
                     new X509BasicConstraintsExtension(true, false, 0, true);
@@ -894,7 +999,7 @@ internal static void BuildPrivatePki(
 
                     {
                         X509Certificate2 intermedPub = issuingAuthority.CreateSubordinateCA(
-                            BuildSubject($"A Revocation Test CA {intermediateIndex}", testName, pkiOptions, pkiOptionsInSubject),
+                            intermediateAuthorityNameFactory(intermediateIndex),
                             intermediateKey.ToPublicKey());
                         intermedCert = intermediateKey.OntoCertificate(intermedPub);
                         intermedPub.Dispose();
@@ -918,7 +1023,7 @@ internal static void BuildPrivatePki(
                 }
 
                 endEntityCert = issuingAuthority.CreateEndEntity(
-                    BuildSubject(subjectName ?? "A Revocation Test Cert", testName, pkiOptions, pkiOptionsInSubject),
+                    endEntityName,
                     eeKey.ToPublicKey(),
                     extensions);
 
@@ -970,7 +1075,7 @@ internal static void BuildPrivatePki(
             intermediateAuthority = intermediateAuthorities.Single();
         }
 
-        private static string BuildSubject(
+        private static X500DistinguishedName BuildSubject(
             string cn,
             string testName,
             PkiOptions pkiOptions,
@@ -979,7 +1084,8 @@ private static string BuildSubject(
             string testNamePart = !string.IsNullOrWhiteSpace(testName) ? $", O=\"{testName}\"" : "";
             string pkiOptionsPart = includePkiOptions ? $", OU=\"{pkiOptions}\"" : "";
 
-            return $"CN=\"{cn}\"" + testNamePart + pkiOptionsPart;
+            string subject = $"CN=\"{cn}\"" + testNamePart + pkiOptionsPart;
+            return new X500DistinguishedName(subject);
         }
 
         private static HashAlgorithmName HashAlgorithmIfNeeded(string publicKeyOid)
@@ -1114,7 +1220,7 @@ internal static KeyHolder CreateKey(KeyFactory factory)
                 return new KeyHolder(factory.CreateKey());
             }
 
-            internal CertificateRequest CreateRequest(string subject)
+            internal CertificateRequest CreateRequest(X500DistinguishedName subject)
             {
                 return _key switch
                 {

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/UnixChainVerifier.cs

@@ -3,30 +3,65 @@
 
 using System.Collections.Generic;
 using System.Diagnostics;
-using Internal.Cryptography;
 
 namespace System.Security.Cryptography.X509Certificates
 {
     internal static class UnixChainVerifier
     {
         public static bool Verify(X509ChainElement[] chainElements, X509VerificationFlags flags)
         {
-            bool isEndEntity = true;
+            int rootIndex = HasPartialChain(chainElements) ? -1 : chainElements.Length - 1;
 
-            foreach (X509ChainElement element in chainElements)
+            for (int i = 0; i < chainElements.Length; i++)
             {
-                if (HasUnsuppressedError(flags, element, isEndEntity))
+                // Element 0 is the end-entity.
+                // If the chain is complete, then match the last element under "root".
+                // Otherwise, the element must be part of the middle of a chain.
+                //
+                // Two fuzzy pieces in this logic:
+                // 1. Non-self-issued trust anchors.  We generally don't support them,
+                //    but they're possible on macOS because of system trust rules.
+                //    This logic will treat them as roots, which is more correct than not.
+                //    (As the trust anchor, you're not expecting someone to say it was revoked,
+                //    but instead you remove it from anchor status.)
+                //
+                // 2. Black-box testing suggests Windows has some special considerations for a
+                //    CA cert that was self-issued by the root (but with a different key and not
+                //    self-signed).  This sort of re-keying is exceptionally rare, so it's not a
+                //    high-priority research effort.  Until then, calling the signed re-key an
+                //    intermediate is more accurate than calling it a root, as it will be checked
+                //    for revocation under the ExcludeRoot policy.
+                X509VerificationFlags suppressionFlag =
+                    i == 0 ? X509VerificationFlags.IgnoreEndRevocationUnknown :
+                    i == rootIndex ? X509VerificationFlags.IgnoreRootRevocationUnknown :
+                    X509VerificationFlags.IgnoreCertificateAuthorityRevocationUnknown;
+
+                if (HasUnsuppressedError(flags, chainElements[i], suppressionFlag))
                 {
                     return false;
                 }
-
-                isEndEntity = false;
             }
 
             return true;
+
+            static bool HasPartialChain(X509ChainElement[] chainElements)
+            {
+                foreach (X509ChainElement element in chainElements)
+                {
+                    foreach (X509ChainStatus status in element.ChainElementStatus)
+                    {
+                        if (status.Status == X509ChainStatusFlags.PartialChain)
+                        {
+                            return true;
+                        }
+                    }
+                }
+
+                return false;
+            }
         }
 
-        private static bool HasUnsuppressedError(X509VerificationFlags flags, X509ChainElement element, bool isEndEntity)
+        private static bool HasUnsuppressedError(X509VerificationFlags flags, X509ChainElement element, X509VerificationFlags revocationSuppressionFlag)
         {
             foreach (X509ChainStatus status in element.ChainElementStatus)
             {
@@ -47,18 +82,7 @@ private static bool HasUnsuppressedError(X509VerificationFlags flags, X509ChainE
 
                 if (status.Status == X509ChainStatusFlags.RevocationStatusUnknown)
                 {
-                    if (isEndEntity)
-                    {
-                        suppressionFlag = X509VerificationFlags.IgnoreEndRevocationUnknown;
-                    }
-                    else if (IsSelfSigned(element.Certificate))
-                    {
-                        suppressionFlag = X509VerificationFlags.IgnoreRootRevocationUnknown;
-                    }
-                    else
-                    {
-                        suppressionFlag = X509VerificationFlags.IgnoreCertificateAuthorityRevocationUnknown;
-                    }
+                    suppressionFlag = revocationSuppressionFlag;
                 }
                 else if (status.Status == X509ChainStatusFlags.OfflineRevocation)
                 {
@@ -83,11 +107,6 @@ private static bool HasUnsuppressedError(X509VerificationFlags flags, X509ChainE
             return false;
         }
 
-        private static bool IsSelfSigned(X509Certificate2 cert)
-        {
-            return cert.SubjectName.RawData.ContentsEqual(cert.IssuerName.RawData);
-        }
-
         private static X509VerificationFlags? GetSuppressionFlag(X509ChainStatusFlags status)
         {
             switch (status)