From 9e944b4dd74b154e28d5783a62072270f38a1723 Mon Sep 17 00:00:00 2001 From: Jeremy Barton Date: Wed, 22 Jul 2026 14:28:27 -0700 Subject: [PATCH] Normalize X509Chain App/Cert policies across OSes The original policy handling code was written before contextual trust, which made it hard to write good tests. Now that we have more tests, unify the behaviors as best we can, even across invalidly encoded extensions. --- .../src/System/Security/Cryptography/Oids.cs | 1 + .../X509Certificates/CertificatePolicy.cs | 363 +++++++++++- .../X509Certificates/ChainPal.Android.cs | 106 +++- .../X509Certificates/ChainPal.Apple.cs | 67 ++- .../OpenSslX509ChainProcessor.cs | 143 +++-- .../System.Security.Cryptography.Tests.csproj | 2 + .../AppAndCertPoliciesChainTests.cs | 545 ++++++++++++++++++ .../tests/X509Certificates/ChainTests.cs | 9 - .../CorruptPoliciesChainTests.cs | 485 ++++++++++++++++ .../X509Certificates/DynamicChainTests.cs | 349 +---------- .../RevocationTests/DynamicRevocationTests.cs | 40 +- 11 files changed, 1637 insertions(+), 473 deletions(-) create mode 100644 src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs create mode 100644 src/libraries/System.Security.Cryptography/tests/X509Certificates/CorruptPoliciesChainTests.cs diff --git a/src/libraries/Common/src/System/Security/Cryptography/Oids.cs b/src/libraries/Common/src/System/Security/Cryptography/Oids.cs index 5abab4ef7eeea7..3e3681e50058f9 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/Oids.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/Oids.cs @@ -220,6 +220,7 @@ internal static partial class Oids internal const string AuthorityKeyIdentifier = "2.5.29.35"; internal const string CertPolicyConstraints = "2.5.29.36"; internal const string EnhancedKeyUsage = "2.5.29.37"; + internal const string AnyEnhancedKeyUsage = "2.5.29.37.0"; internal const string InhibitAnyPolicyExtension = "2.5.29.54"; // RFC3161 Timestamping diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs index e212f561ebddd5..29207c52f2fde1 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Diagnostics; using System.Formats.Asn1; using System.Security.Cryptography.X509Certificates.Asn1; @@ -36,11 +37,119 @@ internal sealed class CertificatePolicyChain private readonly CertificatePolicy[] _policies; private bool _failAllCertificatePolicies; - public CertificatePolicyChain(List chain) + private CertificatePolicyChain(int count) { - _policies = new CertificatePolicy[chain.Count]; + _policies = new CertificatePolicy[count]; + } + + internal static CertificatePolicyChain Build( + IEnumerable chain, + int chainLength, + bool isPartialChain, + ref ErrorVector extensionErrors) + { + CertificatePolicyChain policies = new CertificatePolicyChain(chainLength); + bool corruptDeclaredPolicies = false; + bool ignored = false; + ref bool detector = ref corruptDeclaredPolicies; + + int rootDepth = isPartialChain ? -1 : chainLength - 1; + int i = 0; + + foreach (X509Certificate2 cert in chain) + { + // Windows ignores declared policcy corruption on the root cert. + if (i == rootDepth) + { + detector = ref ignored; + } + + policies._policies[i] = ReadPolicy(cert, out bool error, ref detector); - ReadPolicies(chain); + if (error) + { + if (extensionErrors.Uninitialized) + { + extensionErrors = new ErrorVector(chainLength); + } + + extensionErrors.Set(i); + } + + i++; + } + + policies.ApplyRestrictions(); + policies._failAllCertificatePolicies |= corruptDeclaredPolicies; + + Debug.Assert(i == chainLength); + return policies; + } + + internal static ErrorVector CheckEncodingOnly(IEnumerable chain, int chainLength) + { + ErrorVector vector = new ErrorVector(chainLength); + int i = 0; + + foreach (X509Certificate2 cert in chain) + { + PolicyData policyData = cert.Pal.GetPolicyData(); + + try + { + if (policyData.ApplicationCertPolicies != null) + { + CheckCertPolicyExtension(policyData.ApplicationCertPolicies); + } + + if (policyData.CertPolicies != null) + { + CheckCertPolicyExtension(policyData.CertPolicies); + } + + if (policyData.CertPolicyMappings != null) + { + CheckCertPolicyMappingsExtension(policyData.CertPolicyMappings); + } + + if (policyData.CertPolicyConstraints != null) + { + _ = PolicyConstraintsAsn.Decode(policyData.CertPolicyConstraints, AsnEncodingRules.DER); + } + + if (policyData.EnhancedKeyUsage != null) + { + CheckExtendedKeyUsageExtension(policyData.EnhancedKeyUsage); + } + + if (policyData.InhibitAnyPolicyExtension != null) + { + // Structural read returning a value type + _ = ReadInhibitAnyPolicyExtension(policyData.InhibitAnyPolicyExtension); + } + } + catch (AsnContentException) + { + vector.Set(i); + } + catch (CryptographicException) + { + vector.Set(i); + } + + i++; + } + + Debug.Assert(i == chainLength); + return vector; + } + + internal void MatchCertificatePolicies(OidCollection policyOids, ref ErrorVector usageErrors) + { + foreach (Oid oid in policyOids) + { + MatchCertificatePolicies(oid, ref usageErrors); + } } internal bool MatchesCertificatePolicies(OidCollection policyOids) @@ -56,6 +165,19 @@ internal bool MatchesCertificatePolicies(OidCollection policyOids) return true; } + internal void MatchCertificatePolicies(Oid policyOid, ref ErrorVector usageErrors) + { + if (!MatchesCertificatePolicies(policyOid)) + { + if (usageErrors.Uninitialized) + { + usageErrors = new ErrorVector(_policies.Length); + } + + usageErrors.Set(0); + } + } + internal bool MatchesCertificatePolicies(Oid policyOid) { if (_failAllCertificatePolicies) @@ -107,6 +229,14 @@ internal bool MatchesCertificatePolicies(Oid policyOid) return true; } + internal void MatchApplicationPolicies(OidCollection policyOids, ref ErrorVector usageErrors) + { + foreach (Oid oid in policyOids) + { + MatchApplicationPolicies(oid, ref usageErrors); + } + } + internal bool MatchesApplicationPolicies(OidCollection policyOids) { foreach (Oid oid in policyOids) @@ -120,6 +250,42 @@ internal bool MatchesApplicationPolicies(OidCollection policyOids) return true; } + private void MatchApplicationPolicies(Oid policyOid, ref ErrorVector usageErrors) + { + string oidToCheck = policyOid.Value!; + bool invalid = false; + + for (int i = 1; i <= _policies.Length; i++) + { + // The loop variable (i) matches the definition in RFC 3280, + // section 6.1.3. In that description i=1 is the root CA, and n + // is the EE/leaf certificate. In our chain object 0 is the EE cert + // and _policies.Length-1 is the root cert. So we will index things as + // _policies.Length - i (because i is 1 indexed). + int dataIdx = _policies.Length - i; + CertificatePolicy policy = _policies[dataIdx]; + + // NotValidForUsage can be inherited from the parent. + if (!invalid) + { + if (!policy.AllowsAnyApplicationPolicy && policy.DeclaredApplicationPolicies is not null) + { + invalid = !policy.DeclaredApplicationPolicies.Contains(oidToCheck); + } + } + + if (invalid) + { + if (usageErrors.Uninitialized) + { + usageErrors = new ErrorVector(_policies.Length); + } + + usageErrors.Set(dataIdx); + } + } + } + internal bool MatchesApplicationPolicies(Oid policyOid) { string oidToCheck = policyOid.Value!; @@ -153,25 +319,20 @@ internal bool MatchesApplicationPolicies(Oid policyOid) return true; } - private void ReadPolicies(List chain) + private void ApplyRestrictions() { - for (int i = 0; i < chain.Count; i++) - { - _policies[i] = ReadPolicy(chain[i]); - } - - int explicitPolicyDepth = chain.Count; + int explicitPolicyDepth = _policies.Length; int inhibitAnyPolicyDepth = explicitPolicyDepth; int inhibitPolicyMappingDepth = explicitPolicyDepth; - for (int i = 1; i <= chain.Count; i++) + for (int i = 1; i <= _policies.Length; i++) { // The loop variable (i) matches the definition in RFC 3280, // section 6.1.3. In that description i=1 is the root CA, and n // is the EE/leaf certificate. In our chain object 0 is the EE cert - // and chain.Count-1 is the root cert. So we will index things as - // chain.Count - i (because i is 1 indexed). - int dataIdx = chain.Count - i; + // and _policies.Length-1 is for the root cert. So we will index things as + // _policies.Length - i (because i is 1 indexed). + int dataIdx = _policies.Length - i; CertificatePolicy policy = _policies[dataIdx]; @@ -223,66 +384,122 @@ private static void ApplyRestriction(ref int restriction, int? policyRestriction } } - private static CertificatePolicy ReadPolicy(X509Certificate2 cert) + private static CertificatePolicy ReadPolicy(X509Certificate2 cert, out bool error, ref bool corruptDeclaredPolicies) { // If no ApplicationCertPolicies extension is provided then it uses the EKU // OIDS. HashSet? applicationCertPolicies = null; - HashSet? ekus = null; CertificatePolicy policy = new CertificatePolicy(); + error = false; PolicyData policyData = cert.Pal.GetPolicyData(); if (policyData.ApplicationCertPolicies != null) { - applicationCertPolicies = ReadCertPolicyExtension(policyData.ApplicationCertPolicies); + try + { + applicationCertPolicies = ReadCertPolicyExtension(policyData.ApplicationCertPolicies); + } + catch (CryptographicException) + { + error = true; + } } if (policyData.CertPolicies != null) { - policy.DeclaredCertificatePolicies = ReadCertPolicyExtension(policyData.CertPolicies); + try + { + policy.DeclaredCertificatePolicies = ReadCertPolicyExtension(policyData.CertPolicies); + } + catch (CryptographicException) + { + corruptDeclaredPolicies = true; + error = true; + } } if (policyData.CertPolicyMappings != null) { - policy.PolicyMapping = ReadCertPolicyMappingsExtension(policyData.CertPolicyMappings); + try + { + policy.PolicyMapping = ReadCertPolicyMappingsExtension(policyData.CertPolicyMappings); + } + catch (CryptographicException) + { + error = true; + } } if (policyData.CertPolicyConstraints != null) { - ReadCertPolicyConstraintsExtension(policyData.CertPolicyConstraints, policy); + try + { + ReadCertPolicyConstraintsExtension(policyData.CertPolicyConstraints, policy); + } + catch (CryptographicException) + { + error = true; + } } - if (policyData.EnhancedKeyUsage != null && applicationCertPolicies == null) + if (policyData.EnhancedKeyUsage != null) { - // No reason to do this if the applicationCertPolicies was already read - ekus = ReadExtendedKeyUsageExtension(policyData.EnhancedKeyUsage); + try + { + // If policyData.ApplicationCertPolicies is present, but corrupt, applicationCertPolicies + // should stay null, we'll only check EKU for structural validity. + if (policyData.ApplicationCertPolicies is null) + { + applicationCertPolicies = ReadExtendedKeyUsageExtension(policyData.EnhancedKeyUsage); + } + else + { + // Check can throw either CryptographicException or AsnContentException, so catch both. + CheckExtendedKeyUsageExtension(policyData.EnhancedKeyUsage); + } + } + catch (AsnContentException) + { + error = true; + } + catch (CryptographicException) + { + error = true; + } } if (policyData.InhibitAnyPolicyExtension != null) { - policy.InhibitAnyDepth = ReadInhibitAnyPolicyExtension(policyData.InhibitAnyPolicyExtension); + try + { + policy.InhibitAnyDepth = ReadInhibitAnyPolicyExtension(policyData.InhibitAnyPolicyExtension); + } + catch (CryptographicException) + { + error = true; + } } - policy.DeclaredApplicationPolicies = applicationCertPolicies ?? ekus; + policy.DeclaredApplicationPolicies = applicationCertPolicies; policy.ImplicitAnyApplicationPolicy = policy.DeclaredApplicationPolicies == null; policy.ImplicitAnyCertificatePolicy = policy.DeclaredCertificatePolicies == null; - policy.SpecifiedAnyApplicationPolicy = CheckExplicitAnyPolicy(policy.DeclaredApplicationPolicies); - policy.SpecifiedAnyCertificatePolicy = CheckExplicitAnyPolicy(policy.DeclaredCertificatePolicies); + policy.SpecifiedAnyApplicationPolicy = CheckExplicitAnyPolicy(policy.DeclaredApplicationPolicies, Oids.AnyEnhancedKeyUsage); + policy.SpecifiedAnyCertificatePolicy = CheckExplicitAnyPolicy(policy.DeclaredCertificatePolicies, Oids.AnyCertPolicy); return policy; } - private static bool CheckExplicitAnyPolicy(ISet? declaredPolicies) + private static bool CheckExplicitAnyPolicy(ISet? declaredPolicies, string anyPolicyOid) { if (declaredPolicies == null) { return false; } - return declaredPolicies.Remove(Oids.AnyCertPolicy); + return declaredPolicies.Remove(anyPolicyOid); } private static int ReadInhibitAnyPolicyExtension(byte[] rawData) @@ -311,6 +528,26 @@ private static void ReadCertPolicyConstraintsExtension(byte[] rawData, Certifica policy.InhibitMappingDepth = constraints.InhibitMappingDepth; } + private static void CheckExtendedKeyUsageExtension(byte[] rawData) + { + ValueAsnReader reader = new ValueAsnReader(rawData, AsnEncodingRules.DER); + ValueAsnReader sequenceReader = reader.ReadSequence(); + reader.ThrowIfNotEmpty(); + + //OidCollection usages + while (sequenceReader.HasData) + { + // OBJECT IDENTIFIER only has a primitive encoding, so != is fine, + // doesn't need to be HasSameClassAndValue + if (sequenceReader.PeekTag() != Asn1Tag.ObjectIdentifier) + { + throw new CryptographicException(SR.Cryptography_Der_Invalid_Encoding); + } + + sequenceReader.ReadEncodedValue(); + } + } + private static HashSet ReadExtendedKeyUsageExtension(byte[] rawData) { HashSet oids = new HashSet(); @@ -335,6 +572,18 @@ private static HashSet ReadExtendedKeyUsageExtension(byte[] rawData) return oids; } + private static void CheckCertPolicyExtension(byte[] rawData) + { + ValueAsnReader reader = new ValueAsnReader(rawData, AsnEncodingRules.DER); + ValueAsnReader sequenceReader = reader.ReadSequence(); + reader.ThrowIfNotEmpty(); + + while (sequenceReader.HasData) + { + PolicyInformationAsn.Decode(ref sequenceReader, rawData, out _); + } + } + internal static HashSet ReadCertPolicyExtension(byte[] rawData) { try @@ -364,6 +613,18 @@ internal static HashSet ReadCertPolicyExtension(byte[] rawData) } } + private static void CheckCertPolicyMappingsExtension(byte[] rawData) + { + ValueAsnReader reader = new ValueAsnReader(rawData, AsnEncodingRules.DER); + ValueAsnReader sequenceReader = reader.ReadSequence(); + reader.ThrowIfNotEmpty(); + + while (sequenceReader.HasData) + { + CertificatePolicyMappingAsn.Decode(ref sequenceReader, out _); + } + } + private static List ReadCertPolicyMappingsExtension(byte[] rawData) { try @@ -386,5 +647,49 @@ private static List ReadCertPolicyMappingsExtension throw new CryptographicException(SR.Cryptography_Der_Invalid_Encoding, e); } } + + internal struct ErrorVector + { + private nint _scalar; + private System.Collections.BitArray? _vector; + + internal ErrorVector(int length) + { + if (length > 8 * sizeof(nint)) + { + _vector = new System.Collections.BitArray(length); + } + } + + internal bool Uninitialized => _vector is null && _scalar == 0; + internal bool Any => _scalar != 0; + + internal bool this[int index] + { + get + { + if (_vector is null) + { + return (_scalar & (1 << index)) != 0; + } + + return _vector[index]; + } + } + + internal void Set(int index) + { + if (_vector is null) + { + _scalar |= (1 << index); + } + else + { + // Make scalar non-zero so IsEmpty returns false. + _scalar = 1; + _vector[index] = true; + } + } + } } } diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs index 4e75aa66429f12..a0e14cce343f5b 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs @@ -284,16 +284,7 @@ internal void Evaluate( AddStatusFromIndexToEndCertificate(statuses.Length - 1, ref revocationUnknownStatus, statuses, overallStatus); } - if (!IsPolicyMatch(certs, applicationPolicy, certificatePolicy)) - { - // Assign NotValidForUsage to everything - X509ChainStatus policyFailStatus = new X509ChainStatus - { - Status = X509ChainStatusFlags.NotValidForUsage, - StatusInformation = SR.Chain_NoPolicyMatch, - }; - AddStatusFromIndexToEndCertificate(statuses.Length - 1, ref policyFailStatus, statuses, overallStatus); - } + CheckPolicies(certs, applicationPolicy, certificatePolicy, statuses, overallStatus); X509ChainElement[] elements = new X509ChainElement[certs.Length]; for (int i = 0; i < certs.Length; i++) @@ -365,30 +356,101 @@ private static X509ChainStatus ValidationErrorToChainStatus(Interop.AndroidCrypt }; } - private static bool IsPolicyMatch( + private static void CheckPolicies( X509Certificate2[] certs, OidCollection? applicationPolicy, - OidCollection? certificatePolicy) + OidCollection? certificatePolicy, + List[] statuses, + List overallStatus) { bool hasApplicationPolicy = applicationPolicy != null && applicationPolicy.Count > 0; bool hasCertificatePolicy = certificatePolicy != null && certificatePolicy.Count > 0; + CertificatePolicyChain.ErrorVector encodingErrors = default; + CertificatePolicyChain.ErrorVector usageErrors = default; + + if (hasApplicationPolicy || hasCertificatePolicy) + { + bool isPartialChain = false; + Debug.Assert(statuses.Length == certs.Length); + + List? lastStatus = statuses[^1]; - if (!hasApplicationPolicy && !hasCertificatePolicy) - return true; + if (lastStatus is not null) + { + foreach (X509ChainStatus status in lastStatus) + { + if (status.Status == X509ChainStatusFlags.PartialChain) + { + isPartialChain = true; + break; + } + } + } + + CertificatePolicyChain policyChain = CertificatePolicyChain.Build( + certs, + certs.Length, + isPartialChain, + ref encodingErrors); + + if (certificatePolicy is not null) + { + policyChain.MatchCertificatePolicies(certificatePolicy, ref usageErrors); + } - List certsToRead = new List(certs); - CertificatePolicyChain policyChain = new CertificatePolicyChain(certsToRead); - if (hasCertificatePolicy && !policyChain.MatchesCertificatePolicies(certificatePolicy!)) + if (applicationPolicy is not null) + { + policyChain.MatchApplicationPolicies(applicationPolicy, ref usageErrors); + } + } + else { - return false; + encodingErrors = CertificatePolicyChain.CheckEncodingOnly(certs, certs.Length); } - if (hasApplicationPolicy && !policyChain.MatchesApplicationPolicies(applicationPolicy!)) + if (encodingErrors.Any || usageErrors.Any) { - return false; - } + X509ChainStatus notValidForUsage = new X509ChainStatus + { + Status = X509ChainStatusFlags.NotValidForUsage, + StatusInformation = SR.Chain_NoPolicyMatch, + }; + + X509ChainStatus invalidPolicyConstraints = new X509ChainStatus + { + Status = X509ChainStatusFlags.InvalidPolicyConstraints, + // "NoPolicyMatch" sais that the policy is "invalid", which works for this one, too. + StatusInformation = SR.Chain_NoPolicyMatch, + }; + + X509ChainStatus invalidExtension = new X509ChainStatus + { + Status = X509ChainStatusFlags.InvalidExtension, + StatusInformation = SR.Cryptography_Der_Invalid_Encoding, + }; + + for (int i = 0; i < certs.Length; i++) + { + if (encodingErrors[i]) + { + statuses[i] ??= new List(); + + AddUniqueStatus(statuses[i], ref invalidPolicyConstraints); + AddUniqueStatus(overallStatus, ref invalidPolicyConstraints); + + AddUniqueStatus(statuses[i], ref invalidExtension); + AddUniqueStatus(overallStatus, ref invalidExtension); + } + + if (usageErrors[i]) + { + statuses[i] ??= new List(); - return true; + AddUniqueStatus(statuses[i], ref notValidForUsage); + AddUniqueStatus(overallStatus, ref notValidForUsage); + } + } + } } } } diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Apple.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Apple.cs index da6a16cf5cc9e4..9f1f080fa94b86 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Apple.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Apple.cs @@ -255,16 +255,7 @@ internal void Execute( (X509Certificate2, int)[] elements = ParseResults(_chainHandle!, _revocationMode); Debug.Assert(elements.Length > 0); - if (!IsPolicyMatch(elements, applicationPolicy, certificatePolicy)) - { - for (int i = 0; i < elements.Length; i++) - { - (X509Certificate2, int) currentValue = elements[i]; - - elements[i] = (currentValue.Item1, currentValue.Item2 | (int)X509ChainStatusFlags.NotValidForUsage); - } - } - + CheckPolicies(elements, applicationPolicy, certificatePolicy); FixupRevocationStatus(elements, revocationFlag); BuildAndSetProperties(elements); } @@ -305,40 +296,68 @@ private static (X509Certificate2, int)[] ParseResults( return elements; } - private static bool IsPolicyMatch( + private static void CheckPolicies( (X509Certificate2, int)[] elements, OidCollection? applicationPolicy, OidCollection? certificatePolicy) { + CertificatePolicyChain.ErrorVector encodingErrors = default; + CertificatePolicyChain.ErrorVector usageErrors = default; + if (applicationPolicy?.Count > 0 || certificatePolicy?.Count > 0) { - List certsToRead = new List(); + CertificatePolicyChain policyChain = CertificatePolicyChain.Build( + ElementsToCerts(elements), + elements.Length, + isPartialChain: (elements[^1].Item2 & (int)X509ChainStatusFlags.PartialChain) != 0, + ref encodingErrors); - for (int i = 0; i < elements.Length; i++) + if (certificatePolicy is not null) { - certsToRead.Add(elements[i].Item1); + policyChain.MatchCertificatePolicies(certificatePolicy, ref usageErrors); } - CertificatePolicyChain policyChain = new CertificatePolicyChain(certsToRead); + if (applicationPolicy is not null) + { + policyChain.MatchApplicationPolicies(applicationPolicy, ref usageErrors); + } + } + else + { + encodingErrors = CertificatePolicyChain.CheckEncodingOnly( + ElementsToCerts(elements), + elements.Length); + } - if (certificatePolicy?.Count > 0) + if (encodingErrors.Any || usageErrors.Any) + { + for (int i = 0; i < elements.Length; i++) { - if (!policyChain.MatchesCertificatePolicies(certificatePolicy)) + ref (X509Certificate2, int) currentValue = ref elements[i]; + + if (encodingErrors[i]) { - return false; + const X509ChainStatusFlags EncodingErrorFlags = + X509ChainStatusFlags.InvalidPolicyConstraints | + X509ChainStatusFlags.InvalidExtension; + + currentValue.Item2 |= (int)EncodingErrorFlags; } - } - if (applicationPolicy?.Count > 0) - { - if (!policyChain.MatchesApplicationPolicies(applicationPolicy)) + if (usageErrors[i]) { - return false; + currentValue.Item2 |= (int)X509ChainStatusFlags.NotValidForUsage; } } } - return true; + static IEnumerable ElementsToCerts((X509Certificate2, int)[] elements) + { + foreach ((X509Certificate2 cert, _) in elements) + { + yield return cert; + } + } } private void BuildAndSetProperties((X509Certificate2, int)[] elementTuples) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs index 13c437617b5a36..64bc09871b4ba4 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs @@ -716,6 +716,18 @@ internal void Finish(OidCollection? applicationPolicy, OidCollection? certificat { ProcessPolicy(elements, ref overallStatus, applicationPolicy, certificatePolicy); } + else + { + CertificatePolicyChain.ErrorVector errors = CertificatePolicyChain.CheckEncodingOnly( + ElementsToCerts(elements), + elements.Length); + + if (errors.Any) + { + overallStatus ??= new List(); + MergePolicyErrors(elements, errors, usageErrors: default, overallStatus); + } + } ChainStatus = overallStatus?.ToArray() ?? Array.Empty(); ChainElements = elements; @@ -897,62 +909,66 @@ private X509ChainElement[] BuildChainElements( return elements; } - private static void ProcessPolicy( + private static void MergePolicyErrors( X509ChainElement[] elements, - ref List? overallStatus, - OidCollection? applicationPolicy, - OidCollection? certificatePolicy) + CertificatePolicyChain.ErrorVector extensionErrors, + CertificatePolicyChain.ErrorVector usageErrors, + List overallStatus) { - List certsToRead = new List(); - - foreach (X509ChainElement element in elements) + X509ChainStatus policyConstr = new X509ChainStatus { - certsToRead.Add(element.Certificate); - } - - CertificatePolicyChain policyChain = new CertificatePolicyChain(certsToRead); + Status = X509ChainStatusFlags.InvalidPolicyConstraints, + StatusInformation = GetErrorString(X509VerifyStatusCodeUniversal.X509_V_ERR_INVALID_POLICY_EXTENSION), + }; - bool failsPolicyChecks = false; + X509ChainStatus badExt = new X509ChainStatus + { + Status = X509ChainStatusFlags.InvalidExtension, + StatusInformation = GetErrorString(X509VerifyStatusCodeUniversal.X509_V_ERR_INVALID_EXTENSION), + }; - if (certificatePolicy != null) + X509ChainStatus badUsage = new X509ChainStatus { - if (!policyChain.MatchesCertificatePolicies(certificatePolicy)) - { - failsPolicyChecks = true; - } - } + Status = X509ChainStatusFlags.NotValidForUsage, + StatusInformation = SR.Chain_NoPolicyMatch, + }; - if (applicationPolicy != null) + if (extensionErrors.Any) { - if (!policyChain.MatchesApplicationPolicies(applicationPolicy)) - { - failsPolicyChecks = true; - } + AddUniqueStatus(overallStatus, ref policyConstr); + AddUniqueStatus(overallStatus, ref badExt); } - if (failsPolicyChecks) + if (usageErrors.Any) { - overallStatus ??= new List(); - - X509ChainStatus chainStatus = new X509ChainStatus - { - Status = X509ChainStatusFlags.NotValidForUsage, - StatusInformation = SR.Chain_NoPolicyMatch, - }; + AddUniqueStatus(overallStatus, ref badUsage); + } - AddUniqueStatus(overallStatus, ref chainStatus); + // No individual element can have seen more errors than the chain overall, + // so avoid regrowth of the list. + List elementStatus = new List(overallStatus.Count); - // No individual element can have seen more errors than the chain overall, - // so avoid regrowth of the list. - var elementStatus = new List(overallStatus.Count); + for (int i = 0; i < elements.Length; i++) + { + bool ext = extensionErrors[i]; + bool usage = usageErrors[i]; - for (int i = 0; i < elements.Length; i++) + if (ext || usage) { X509ChainElement element = elements[i]; elementStatus.Clear(); elementStatus.AddRange(element.ChainElementStatus); - AddUniqueStatus(elementStatus, ref chainStatus); + if (ext) + { + AddUniqueStatus(elementStatus, ref policyConstr); + AddUniqueStatus(elementStatus, ref badExt); + } + + if (usage) + { + AddUniqueStatus(elementStatus, ref badUsage); + } elements[i] = new X509ChainElement( element.Certificate, @@ -962,6 +978,51 @@ private static void ProcessPolicy( } } + private static void ProcessPolicy( + X509ChainElement[] elements, + ref List? overallStatus, + OidCollection? applicationPolicy, + OidCollection? certificatePolicy) + { + bool isPartialChain = false; + X509ChainElement lastElement = elements[^1]; + Debug.Assert(lastElement.ChainElementStatus is not null); + + foreach (X509ChainStatus status in lastElement.ChainElementStatus) + { + if (status.Status == X509ChainStatusFlags.PartialChain) + { + isPartialChain = true; + break; + } + } + + CertificatePolicyChain.ErrorVector usageErrors = default; + CertificatePolicyChain.ErrorVector encodingErrors = default; + + CertificatePolicyChain policyChain = CertificatePolicyChain.Build( + ElementsToCerts(elements), + elements.Length, + isPartialChain, + ref encodingErrors); + + if (certificatePolicy is not null) + { + policyChain.MatchCertificatePolicies(certificatePolicy, ref usageErrors); + } + + if (applicationPolicy is not null) + { + policyChain.MatchApplicationPolicies(applicationPolicy, ref usageErrors); + } + + if (usageErrors.Any || encodingErrors.Any) + { + overallStatus ??= new List(); + MergePolicyErrors(elements, encodingErrors, usageErrors, overallStatus); + } + } + private static void AddElementStatus( ErrorCollection errorCodes, List elementStatus, @@ -1307,6 +1368,14 @@ private static string GetErrorString(Interop.Crypto.X509VerifyStatusCode code) Interop.Crypto.GetX509VerifyCertErrorString); } + private static IEnumerable ElementsToCerts(X509ChainElement[] elements) + { + foreach (X509ChainElement element in elements) + { + yield return element.Certificate; + } + } + private sealed class WorkingChain : IDisposable { // OpenSSL 1.0 sets a "signature valid, don't check again" if we OK the signature error diff --git a/src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj b/src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj index ab5f16dfb45d9d..0321a91966ebdb 100644 --- a/src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj +++ b/src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj @@ -638,6 +638,7 @@ + @@ -646,6 +647,7 @@ + diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs new file mode 100644 index 00000000000000..0e1e9b24cae778 --- /dev/null +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs @@ -0,0 +1,545 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Diagnostics; +using Xunit; + +namespace System.Security.Cryptography.X509Certificates.Tests +{ + public static class AppAndCertPoliciesChainTests + { + private const string ApplicationCertPoliciesOid = "1.3.6.1.4.1.311.21.10"; + + [Theory] + [MemberData(nameof(ChainPolicyMemberData))] + [ActiveIssue("https://github.com/dotnet/runtime/issues/128890", TestPlatforms.Android)] + public static void CertificatePolicyTest( + ChainPolicyTestCase testCase) + { + using (testCase) + { + DynamicChainTests.TestChain4( + testCase.Root, + testCase.HighIntermediate, + testCase.LowIntermediate, + testCase.EndEntity, + DynamicChainTests.PlatformPolicyConstraints(testCase.ExpectedFlags), + testCase.ConfigureCallback); + } + } + + public static IEnumerable ChainPolicyMemberData() + { + foreach (ChainPolicyTestCase testCase in TestCases()) + { + yield return new object[] { testCase }; + } + + static IEnumerable TestCases() + { + // Use the same keys for all chains, just to keep the total time low. + // There are enough cases here that keygen shows up in clock time. + RSA[] keys = [RSA.Create(2048), RSA.Create(2048), RSA.Create(2048), RSA.Create(2048)]; + + // No chain.Policy.CertificatePolicy checks, EE uses the mapped policy identifier, + // intermediate requires that the EE certs have a policy extension. + // + // Despite the intermediate specifying a mapping when it's forbidden (inhibit<2), + // Everything is reported valid, because the EE cert policy C doesn't require the mapping. + for (int rootInhibitMapping = 0; rootInhibitMapping <= 2; rootInhibitMapping++) + { + yield return ChainPolicyTestCase.Build( + $"NoPolicyCheck/EEUsesMappedPolicyAndExtra/PolicyRequired/RootInhibit={rootInhibitMapping}", + keys, + rootInhibitMapping, + 0, + [ChainPolicyTestCase.PolicyB, ChainPolicyTestCase.PolicyC], + [], + X509ChainStatusFlags.NoError); + } + + // No chain.Policy.CertificatePolicy checks, EE uses the mapped policy identifier, + // intermediate requires that the EE certs have a policy extension. + // + // The EE policy is required, and the only policy in the EE cert is the mapped one, + // but that is forbidden by the intermediate's inhibit=<2, so it's an + // Issuance-Chain-Policy violation. + for (int rootInhibitMapping = 0; rootInhibitMapping <= 2; rootInhibitMapping++) + { + // Windows seems to be the only OS capable of reporting this error. + + yield return ChainPolicyTestCase.Build( + $"NoPolicyCheck/EEUsesMappedPolicyOnly/PolicyRequired/RootInhibit={rootInhibitMapping}", + keys, + rootInhibitMapping, + 0, + [ChainPolicyTestCase.PolicyB], + [], + rootInhibitMapping < 2 && OperatingSystem.IsWindows() ? + X509ChainStatusFlags.NoIssuanceChainPolicy : + X509ChainStatusFlags.NoError); + } + + // No chain.Policy.CertificatePolicy checks, EE uses the mapped policy identifier, + // intermediate does not require that the EE certs have a policy extension. + // + // Even though the intermediate has a disallowed mapping when inhibit=<2, + // since the EE isn't _required_ to have a policy, nothing was required to + // traverse the map. + for (int rootInhibitMapping = 0; rootInhibitMapping <= 2; rootInhibitMapping++) + { + yield return ChainPolicyTestCase.Build( + $"NoPolicyCheck/EEUsesMappedPolicyOnly/PolicyOptional/RootInhibit={rootInhibitMapping}", + keys, + rootInhibitMapping, + -1, + [ChainPolicyTestCase.PolicyB], + [], + X509ChainStatusFlags.NoError); + } + + // EE uses the mapped policy identifier, + // intermediate requires that the EE certs have a policy extension. + // Require that the EE cert is valid for only policy C (no mapping required). + // + // Despite the intermediate specifying a mapping when it's forbidden (inhibit=<2), + // Everything is reported valid, because the EE cert policy C doesn't require the mapping. + for (int rootInhibitMapping = 0; rootInhibitMapping <= 2; rootInhibitMapping++) + { + yield return ChainPolicyTestCase.Build( + $"CheckExtraPolicy/EEUsesMappedPolicyAndExtra/PolicyRequired/RootInhibit={rootInhibitMapping}", + keys, + rootInhibitMapping, + 0, + [ChainPolicyTestCase.PolicyB, ChainPolicyTestCase.PolicyC], + [ChainPolicyTestCase.PolicyC], + X509ChainStatusFlags.NoError); + } + + // EE uses the mapped policy identifier, + // intermediate does not require that the EE certs have a policy extension. + // Require that the EE cert is valid for only policy C (no mapping required). + // + // Despite the intermediate specifying a mapping when it's forbidden (inhibit=<2), + // Everything is reported valid, because the EE cert policy C doesn't require the mapping. + for (int rootInhibitMapping = 0; rootInhibitMapping <= 2; rootInhibitMapping++) + { + yield return ChainPolicyTestCase.Build( + $"CheckExtraPolicy/EEUsesMappedPolicyAndExtra/PolicyOptional/RootInhibit={rootInhibitMapping}", + keys, + rootInhibitMapping, + -1, + [ChainPolicyTestCase.PolicyB, ChainPolicyTestCase.PolicyC], + [ChainPolicyTestCase.PolicyC], + X509ChainStatusFlags.NoError); + } + + // EE uses the mapped policy identifier, + // intermediate requires that the EE certs have a policy extension. + // Require that the EE cert is valid for only policy A (which it calls B). + // + // Since this requires traversing the mapping, it's NotValidForUsage whenever + // the mapping was disallowed (inhibit<2). + for (int rootInhibitMapping = 0; rootInhibitMapping <= 2; rootInhibitMapping++) + { + yield return ChainPolicyTestCase.Build( + $"CheckMappedPolicy/EEUsesMappedPolicyAndExtra/PolicyRequired/RootInhibit={rootInhibitMapping}", + keys, + rootInhibitMapping, + 0, + [ChainPolicyTestCase.PolicyB, ChainPolicyTestCase.PolicyC], + [ChainPolicyTestCase.PolicyA], + rootInhibitMapping < 2 ? + X509ChainStatusFlags.NotValidForUsage : + X509ChainStatusFlags.NoError); + } + + foreach (RSA key in keys) + { + key.Dispose(); + } + } + } + + public sealed class ChainPolicyTestCase : IDisposable + { + internal const string PolicyA = "0.1.2.3"; + internal const string PolicyB = "1.2.3.4"; + internal const string PolicyC = "2.3.4.5"; + + private string _name; + private string[] _eePoliciesToCheck; + + internal X509ChainStatusFlags ExpectedFlags { get; private set; } + internal X509Certificate2 Root { get; private set; } + internal X509Certificate2 HighIntermediate { get; private set; } + internal X509Certificate2 LowIntermediate { get; private set; } + internal X509Certificate2 EndEntity { get; private set; } + + private ChainPolicyTestCase() + { + } + + public Action ConfigureCallback + { + get + { + if (_eePoliciesToCheck.Length == 0) + { + return null; + } + + return policy => + { + foreach (string policyOid in _eePoliciesToCheck) + { + policy.CertificatePolicy.Add(new Oid(policyOid, null)); + } + }; + } + } + + public void Dispose() + { + Root?.Dispose(); + HighIntermediate?.Dispose(); + LowIntermediate?.Dispose(); + EndEntity?.Dispose(); + } + + internal static ChainPolicyTestCase Build( + string name, + RSA[] keys, + int rootInhibitMapping, + int intermediateRequireExplicit, + string[] eePolicies, + string[] eePoliciesToCheck, + X509ChainStatusFlags expectedFlags) + { + X509Extension[] rootExtensions = new[] + { + X509BasicConstraintsExtension.CreateForCertificateAuthority(), + MaybePolicyConstraints(inhibitPolicyMappingSkipCerts: rootInhibitMapping), + }; + + X509Extension[] highImedExtensions = new[] + { + X509BasicConstraintsExtension.CreateForCertificateAuthority(), + // The "any" policy. + DynamicChainTests.BuildPolicyByIdentifiers("2.5.29.32.0"), + }; + + X509Extension[] lowImedExtensions = new[] + { + X509BasicConstraintsExtension.CreateForCertificateAuthority(), + MaybePolicyConstraints(requireExplicitPolicySkipCerts: intermediateRequireExplicit), + DynamicChainTests.BuildPolicyByIdentifiers(PolicyA, PolicyC), + DynamicChainTests.BuildPolicyMappings((PolicyA, PolicyB)), + }; + + X509Extension[] endEntityExtensions = new[] + { + X509BasicConstraintsExtension.CreateForEndEntity(), + MaybePolicies(eePolicies), + }; + + X509Certificate2[] certs = new X509Certificate2[4]; + + TestDataGenerator.MakeTestChain( + keys, + certs, + endEntityExtensions, + [lowImedExtensions, highImedExtensions], + rootExtensions, + name); + + return new ChainPolicyTestCase + { + _name = name, + EndEntity = certs[0], + LowIntermediate = certs[1], + HighIntermediate = certs[2], + Root = certs[3], + _eePoliciesToCheck = eePoliciesToCheck, + ExpectedFlags = expectedFlags, + }; + + static X509Extension MaybePolicies(string[] policyOids) + { + if (policyOids.Length == 0) + { + return null; + } + + return DynamicChainTests.BuildPolicyByIdentifiers(policyOids); + } + + static X509Extension MaybePolicyConstraints( + int requireExplicitPolicySkipCerts = -1, + int inhibitPolicyMappingSkipCerts = -1) + { + if (inhibitPolicyMappingSkipCerts >= 0 && requireExplicitPolicySkipCerts >= 0) + { + return DynamicChainTests.BuildPolicyConstraints(inhibitPolicyMappingSkipCerts, requireExplicitPolicySkipCerts); + } + + if (inhibitPolicyMappingSkipCerts >= 0) + { + return DynamicChainTests.BuildPolicyConstraints(inhibitPolicyMappingSkipCerts: inhibitPolicyMappingSkipCerts); + } + + if (requireExplicitPolicySkipCerts >= 0) + { + return DynamicChainTests.BuildPolicyConstraints(requireExplicitPolicySkipCerts: requireExplicitPolicySkipCerts); + } + + return null; + } + } + + public override string ToString() + { + return _name; + } + } + + // Explores how the Microsoft Application Policies extension (szOID_APPLICATION_CERT_POLICIES, + // 1.3.6.1.4.1.311.21.10) interacts with the standard EKU (2.5.29.37) extension when filtering + // a chain via X509ChainPolicy.ApplicationPolicy. Summary of the intended behavior: + // * Application Policies absent -> EKU governs (with anyEKU 2.5.29.37.0 as the wildcard). + // * Application Policies present -> it is authoritative; EKU is ignored. Its only wildcard + // is anyExtendedKeyUsage (2.5.29.37.0); anyPolicy (2.5.29.32.0) matches nothing. + // * Application Policies present but empty -> authoritative empty set (matches nothing). + // * Application Policies present but undecodable -> the chain is invalid outright, + // regardless of EKU, criticality, or whether any application policy was requested. + [Theory] + [MemberData(nameof(ApplicationPolicyVsEkuMemberData))] + public static void VerifyApplicationPolicyVsEku(AppPolicyEkuCase testCase) + { + X509Certificate2 rootCert = testCase.Root; + X509Certificate2 intermediateCert = testCase.Intermediate; + + CertificateRequest request = new CertificateRequest( + "CN=App Policy vs EKU Test End-Entity", + s_endEntityKey, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1); + + request.CertificateExtensions.Add(X509BasicConstraintsExtension.CreateForEndEntity()); + + if (testCase.EkuOids is not null) + { + OidCollection oids = new OidCollection(); + + foreach (string oid in testCase.EkuOids) + { + oids.Add(new Oid(oid, null)); + } + + request.CertificateExtensions.Add(new X509EnhancedKeyUsageExtension(oids, critical: false)); + } + + if (testCase.ApplicationPolicyValue is not null) + { + request.CertificateExtensions.Add( + new X509Extension( + ApplicationCertPoliciesOid, + testCase.ApplicationPolicyValue, + testCase.ApplicationPolicyCritical)); + } + + DateTimeOffset notBefore = DateTimeOffset.UtcNow.AddDays(-1); + DateTimeOffset notAfter = notBefore.AddDays(30); + + using (X509Certificate2 endEntityCert = + request.Create(intermediateCert, notBefore, notAfter, CreateTestSerial())) + { + DynamicChainTests.TestChain3( + rootCert, + intermediateCert, + endEntityCert, + testCase.ExpectedFlags, + testCase.RequestedApplicationPolicyOid is null + ? null + : policy => policy.ApplicationPolicy.Add(new Oid(testCase.RequestedApplicationPolicyOid, null))); + } + } + + // The end-entity subject key is irrelevant to what these cases exercise (the intermediate signs + // the end-entity cert), so a single fixed key is imported once and reused for every case. + private static readonly RSA s_endEntityKey = CreateEndEntityKey(); + + private static RSA CreateEndEntityKey() + { + RSA rsa = RSA.Create(); + return rsa; + } + + // A single shared root + issuing intermediate is generated once for the whole VerifyApplicationPolicyVsEku + // theory. Only the end-entity certificate differs between cases, so it is (re)issued in the test body. + private static readonly Lazy<(X509Certificate2 Root, X509Certificate2 Intermediate)> s_appPolicyIssuers = + new Lazy<(X509Certificate2, X509Certificate2)>(CreateAppPolicyIssuers); + + private static (X509Certificate2 Root, X509Certificate2 Intermediate) CreateAppPolicyIssuers() + { + DateTimeOffset now = DateTimeOffset.UtcNow; + + using (RSA rootKey = RSA.Create(2048)) + using (RSA intermediateKey = RSA.Create(2048)) + { + + CertificateRequest rootRequest = new CertificateRequest( + "CN=App Policy vs EKU Test Root", + rootKey, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1); + + rootRequest.CertificateExtensions.Add(X509BasicConstraintsExtension.CreateForCertificateAuthority()); + rootRequest.CertificateExtensions.Add( + new X509KeyUsageExtension( + X509KeyUsageFlags.KeyCertSign | X509KeyUsageFlags.CrlSign, + critical: false)); + + X509Certificate2 root = rootRequest.CreateSelfSigned(now.AddDays(-45), now.AddDays(365)); + + CertificateRequest intermediateRequest = new CertificateRequest( + "CN=App Policy vs EKU Test Intermediate", + intermediateKey, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1); + + intermediateRequest.CertificateExtensions.Add(X509BasicConstraintsExtension.CreateForCertificateAuthority()); + intermediateRequest.CertificateExtensions.Add( + new X509KeyUsageExtension( + X509KeyUsageFlags.KeyCertSign | X509KeyUsageFlags.CrlSign, + critical: false)); + + X509Certificate2 intermediate; + + using (X509Certificate2 intermediatePublic = + intermediateRequest.Create(root, now.AddDays(-40), now.AddDays(180), CreateTestSerial())) + { + intermediate = intermediatePublic.CopyWithPrivateKey(intermediateKey); + } + + return (root, intermediate); + } + } + + private static byte[] CreateTestSerial() + { + byte[] serial = new byte[8]; + RandomNumberGenerator.Fill(serial); + + // Keep the high bit clear so the serial encodes as a positive INTEGER. + serial[0] &= 0x7F; + + if (serial[0] == 0) + { + serial[0] = 1; + } + + return serial; + } + + public static IEnumerable ApplicationPolicyVsEkuMemberData() + { + const string ServerAuth = "1.3.6.1.5.5.7.3.2"; + const string ClientAuth = "1.3.6.1.5.5.7.3.1"; + const string TimeStamp = "1.3.6.1.5.5.7.3.8"; // RFC 3161, used only as a companion value + const string AnyEku = "2.5.29.37.0"; // anyExtendedKeyUsage + const string AnyPolicy = "2.5.29.32.0"; // anyPolicy (certificate policies) + + const X509ChainStatusFlags Ok = X509ChainStatusFlags.NoError; + const X509ChainStatusFlags Usage = X509ChainStatusFlags.NotValidForUsage; + const X509ChainStatusFlags BadExt = + X509ChainStatusFlags.InvalidExtension | X509ChainStatusFlags.InvalidPolicyConstraints; + + // Well-formed Application Policies extension value carrying the given usage OIDs. + static byte[] EncPol(params string[] oids) => DynamicChainTests.EncodeCertificatePoliciesValue(oids); + + AppPolicyEkuCase[] cases = + { + // Baseline: EKU only (sanity, including TLS Server Auth). + new AppPolicyEkuCase("no restrictions; req=Server", null, null, false, ServerAuth, Ok), + new AppPolicyEkuCase("EKU=Server; req=Server", new[] { ServerAuth }, null, false, ServerAuth, Ok), + new AppPolicyEkuCase("EKU=Server; req=Client", new[] { ServerAuth }, null, false, ClientAuth, Usage), + new AppPolicyEkuCase("EKU=Client; req=Server", new[] { ClientAuth }, null, false, ServerAuth, Usage), + new AppPolicyEkuCase("EKU=Client; req=none", new[] { ClientAuth }, null, false, null, Ok), + new AppPolicyEkuCase("EKU=anyEKU; req=Server", new[] { AnyEku }, null, false, ServerAuth, Ok), + new AppPolicyEkuCase("EKU=anyEKU; req=Client", new[] { AnyEku }, null, false, ClientAuth, Ok), + new AppPolicyEkuCase("EKU=anyEKU,TS; req=Client", new[] { AnyEku, TimeStamp }, null, false, ClientAuth, Ok), + new AppPolicyEkuCase("EKU=anyPolicy(32.0); req=Server", new[] { AnyPolicy }, null, false, ServerAuth, Usage), + + // Application Policies only (no EKU): behaves like the same EKU. + new AppPolicyEkuCase("AppPol=Server; req=Server", null, EncPol(ServerAuth), false, ServerAuth, Ok), + new AppPolicyEkuCase("AppPol=Server; req=Client", null, EncPol(ServerAuth), false, ClientAuth, Usage), + new AppPolicyEkuCase("AppPol=anyEKU(37.0); req=Server", null, EncPol(AnyEku), false, ServerAuth, Ok), + new AppPolicyEkuCase("AppPol=anyEKU(37.0); req=Client", null, EncPol(AnyEku), false, ClientAuth, Ok), + new AppPolicyEkuCase("AppPol=anyEKU(37.0),TS; req=Server", null, EncPol(AnyEku, TimeStamp), false, ServerAuth, Ok), + new AppPolicyEkuCase("AppPol=anyPolicy(32.0); req=Server", null, EncPol(AnyPolicy), false, ServerAuth, Usage), + new AppPolicyEkuCase("AppPol=anyPolicy(32.0); req=Client", null, EncPol(AnyPolicy), false, ClientAuth, Usage), + new AppPolicyEkuCase("AppPol=TS,anyPolicy(32.0); req=Server", null, EncPol(TimeStamp, AnyPolicy), false, ServerAuth, Usage), + + // Conflicts: Application Policies overrides EKU entirely. + new AppPolicyEkuCase("EKU=Server AppPol=Client; req=Server", new[] { ServerAuth }, EncPol(ClientAuth), false, ServerAuth, Usage), + new AppPolicyEkuCase("EKU=Server AppPol=Client; req=Client", new[] { ServerAuth }, EncPol(ClientAuth), false, ClientAuth, Ok), + new AppPolicyEkuCase("EKU=Client AppPol=Server; req=Server", new[] { ClientAuth }, EncPol(ServerAuth), false, ServerAuth, Ok), + new AppPolicyEkuCase("EKU=Client AppPol=Server; req=Client", new[] { ClientAuth }, EncPol(ServerAuth), false, ClientAuth, Usage), + new AppPolicyEkuCase("EKU=anyEKU AppPol=Client; req=Server", new[] { AnyEku }, EncPol(ClientAuth), false, ServerAuth, Usage), + new AppPolicyEkuCase("EKU=Client AppPol=anyEKU(37.0),TS; req=Server", new[] { ClientAuth }, EncPol(AnyEku, TimeStamp), false, ServerAuth, Ok), + new AppPolicyEkuCase("EKU=Client AppPol=anyEKU(37.0),TS; req=Client", new[] { ClientAuth }, EncPol(AnyEku, TimeStamp), false, ClientAuth, Ok), + + // Well-formed but empty Application Policies: authoritative empty set (matches nothing). + new AppPolicyEkuCase("AppPol=empty EKU=Server; req=Server", new[] { ServerAuth }, EncPol(), false, ServerAuth, Usage), + new AppPolicyEkuCase("AppPol=empty; req=none", null, EncPol(), false, null, Ok), + + // Undecodable Application Policies: hard failure regardless of EKU / criticality / requested usage. + new AppPolicyEkuCase("AppPol=NULL(05 00); req=none", null, new byte[] { 0x05, 0x00 }, false, null, BadExt), + new AppPolicyEkuCase("AppPol=NULL(05 00) EKU=Server; req=Server", new[] { ServerAuth }, new byte[] { 0x05, 0x00 }, false, ServerAuth, BadExt), + new AppPolicyEkuCase("AppPol=NULL(05 00) critical EKU=Server; req=Server", new[] { ServerAuth }, new byte[] { 0x05, 0x00 }, true, ServerAuth, BadExt), + new AppPolicyEkuCase("AppPol=badInner EKU=Server; req=Server", new[] { ServerAuth }, new byte[] { 0x30, 0x03, 0x02, 0x01, 0x2A }, false, ServerAuth, BadExt), + new AppPolicyEkuCase("AppPol=truncated EKU=Server; req=Server", new[] { ServerAuth }, new byte[] { 0x30, 0x82, 0x7F, 0xFF }, false, ServerAuth, BadExt), + }; + + foreach (AppPolicyEkuCase testCase in cases) + { + (testCase.Root, testCase.Intermediate) = s_appPolicyIssuers.Value; + yield return new object[] { testCase }; + } + } + + public sealed class AppPolicyEkuCase + { + public string Name { get; } + public string[] EkuOids { get; } + public byte[] ApplicationPolicyValue { get; } + public bool ApplicationPolicyCritical { get; } + public string RequestedApplicationPolicyOid { get; } + public X509ChainStatusFlags ExpectedFlags { get; } + + // Shared across all cases; assigned by the member-data generator. + public X509Certificate2 Root { get; set; } + public X509Certificate2 Intermediate { get; set; } + + public AppPolicyEkuCase( + string name, + string[] ekuOids, + byte[] applicationPolicyValue, + bool applicationPolicyCritical, + string requestedApplicationPolicyOid, + X509ChainStatusFlags expectedFlags) + { + Name = name; + EkuOids = ekuOids; + ApplicationPolicyValue = applicationPolicyValue; + ApplicationPolicyCritical = applicationPolicyCritical; + RequestedApplicationPolicyOid = requestedApplicationPolicyOid; + ExpectedFlags = expectedFlags; + } + + public override string ToString() => Name; + } + } +} diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs index 74a3351622e899..62e5b9bb371cb9 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs @@ -647,15 +647,6 @@ public static void BuildChain_FailOnlyApplicationPolicy() holder.Chain.ChainElements[1].ChainElementStatus.Aggregate( X509ChainStatusFlags.NoError, (a, status) => a | status.Status)); - - if (!PlatformDetection.IsWindows) - { - Assert.Equal( - X509ChainStatusFlags.NotValidForUsage, - holder.Chain.ChainElements[2].ChainElementStatus.Aggregate( - X509ChainStatusFlags.NoError, - (a, status) => a | status.Status)); - } } } diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/CorruptPoliciesChainTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CorruptPoliciesChainTests.cs new file mode 100644 index 00000000000000..99783d1a540c76 --- /dev/null +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CorruptPoliciesChainTests.cs @@ -0,0 +1,485 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.CompilerServices; +using Xunit; +using Xunit.Sdk; + +namespace System.Security.Cryptography.X509Certificates.Tests +{ + public static class CorruptPoliciesChainTests + { + private const string ApplicationCertPoliciesOid = "1.3.6.1.4.1.311.21.10"; + private const string CabfDvOid = "2.23.140.1.2.1"; + private const string TlsClientAuthOid = "1.3.6.1.5.5.7.3.2"; + private const string TlsServerAuthOid = "1.3.6.1.5.5.7.3.1"; + private const string UnofficialMappedPolicyOid = "1.0.0.127"; + + private static readonly X509Extension s_unmappedPolicyExtension = + DynamicChainTests.BuildPolicyByIdentifiers(CabfDvOid); + + private static readonly X509Extension s_policyMapping = + DynamicChainTests.BuildPolicyMappings((CabfDvOid, UnofficialMappedPolicyOid)); + + private static readonly X509Extension s_mappedPolicyExtension = + DynamicChainTests.BuildPolicyByIdentifiers(UnofficialMappedPolicyOid); + + private static readonly X509Extension s_applicationPolicyExtension = + new X509Extension( + ApplicationCertPoliciesOid, + DynamicChainTests.EncodeCertificatePoliciesValue(TlsClientAuthOid), + critical: false); + + private static readonly X509Extension s_ekuExtension = + new X509EnhancedKeyUsageExtension( + new OidCollection { new Oid(TlsClientAuthOid, null) }, + critical: false); + + private static readonly X509Extension s_caTrue = + X509BasicConstraintsExtension.CreateForCertificateAuthority(); + + private static readonly X509Extension s_caFalse = + X509BasicConstraintsExtension.CreateForEndEntity(); + + private static readonly X509Extension s_corruptPolicies = + new X509Extension(s_unmappedPolicyExtension.Oid, [0x05], critical: false); + + private static readonly X509Extension s_corruptMapping = + new X509Extension(s_policyMapping.Oid, [0x04, 0x13], critical: false); + + private static readonly X509Extension s_corruptApplicationPolicy = + new X509Extension(s_applicationPolicyExtension.Oid, [0x01, 0x00], critical: false); + + private static readonly X509Extension s_corruptEku = + new X509Extension(s_ekuExtension.Oid, [0x30, 0x11], critical: false); + + private static readonly RSA[] s_keys = + { + RSA.Create(), + RSA.Create(), + RSA.Create(), + RSA.Create(), + RSA.Create(), + }; + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(4)] + public static void CorruptCertificatePolicy(int level) + { + // Corruption at the root is fine for usage, but it will still + // generate an InvalidExtension error. + bool notValidForUsage = level != 4; + + RunCase( + corruptCertificatePolicy: level, + corruptApplicationPolicy: -1, + checkCertificatePolicy: true, + checkApplicationPolicy: false, + leafNotValidForUsage: notValidForUsage); + } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(4)] + public static void CorruptApplicationPolicy_WithEku(int level) + { + // When the ApplicationPolicy is corrupt, it seems to treat + // the element as valid for all usages (but is still + // scoped by the issuers) + const bool notValidForUsage = false; + + RunCase( + corruptApplicationPolicy: level, + checkApplicationPolicy: true, + leafNotValidForUsage: notValidForUsage, + omitEku: false); + } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(4)] + public static void CorruptApplicationPolicy_NoEku(int level) + { + // When the ApplicationPolicy is corrupt, it seems to treat + // the element as valid for all usages (but is still + // scoped by the issuers) + const bool notValidForUsage = false; + + RunCase( + corruptApplicationPolicy: level, + checkApplicationPolicy: true, + leafNotValidForUsage: notValidForUsage, + omitEku: true); + } + + [Theory] + [InlineData(0, false)] + [InlineData(0, true)] + [InlineData(1, false)] + [InlineData(1, true)] + [InlineData(2, false)] + [InlineData(2, true)] + [InlineData(3, false)] + [InlineData(3, true)] + [InlineData(4, false)] + [InlineData(4, true)] + public static void CorruptApplicationPolicy_CheckExtraUsage(int level, bool withEku) + { + // When the ApplicationPolicy is corrupt, it seems to treat + // the element as valid for all usages (but is still + // scoped by the issuers), so by checking for a + + RunCase( + corruptApplicationPolicy: level, + checkApplicationPolicy: true, + omitEku: !withEku, + checkUnrelatedAppPolicy: true); + } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(4)] + public static void CorruptEku_WithAppPol(int level) + { + // AppPol always wins over EKU, so usage is valid. + const bool notValidForUsage = false; + + RunCase( + corruptEku: level, + checkApplicationPolicy: true, + leafNotValidForUsage: notValidForUsage, + omitApplicationPolicy: false); + } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(4)] + public static void CorruptEku_NoAppPol(int level) + { + // When the ApplicationPolicy is corrupt, it seems to treat + // the element as valid for all usages (but is still + // scoped by the issuers) + const bool notValidForUsage = false; + + RunCase( + corruptEku: level, + checkApplicationPolicy: true, + leafNotValidForUsage: notValidForUsage, + omitApplicationPolicy: true); + } + + [Theory] + [InlineData(0, false)] + [InlineData(0, true)] + [InlineData(1, false)] + [InlineData(1, true)] + [InlineData(2, false)] + [InlineData(2, true)] + [InlineData(3, false)] + [InlineData(3, true)] + [InlineData(4, false)] + [InlineData(4, true)] + public static void CorruptEku_CheckExtraUsage(int level, bool withAppPol) + { + // When the ApplicationPolicy is corrupt, it seems to treat + // the element as valid for all usages (but is still + // scoped by the issuers) + + RunCase( + corruptEku: level, + checkApplicationPolicy: true, + omitApplicationPolicy: !withAppPol, + checkUnrelatedAppPolicy: true); + } + + [Theory] + [InlineData(4, 3)] + [InlineData(3, 3)] + [InlineData(2, 3)] + [InlineData(1, 3)] + [InlineData(0, 3)] + [InlineData(2, 1)] + [InlineData(1, 1)] + [InlineData(0, 1)] + public static void CorruptPolicyWithMapping(int policyLevel, int mappingLevel) + { + bool notValidForUsage = policyLevel != 4; + + RunCase( + corruptCertificatePolicy: policyLevel, + mappingLevel: mappingLevel, + checkCertificatePolicy: true, + leafNotValidForUsage: notValidForUsage); + } + + [Theory] + [InlineData(3)] + [InlineData(2)] + [InlineData(1)] + public static void PolicyWithCorruptMapping(int mappingLevel) + { + // The driver for the test issues certs below the mapping level + // as unmapped, so this checks that the corrupt mapping counts as + // an empty mapping for usage purposes. + + RunCase( + mappingLevel: mappingLevel, + checkCertificatePolicy: true, + corruptMapping: true); + } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(4)] + public static void CorruptCertificatePolicyNoChecks(int level) + { + RunCase(corruptCertificatePolicy: level); + } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(4)] + public static void CorruptApplicationPolicyNoChecks(int level) + { + RunCase(corruptApplicationPolicy: level); + } + + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(4)] + public static void CorruptEkuNoChecks(int level) + { + RunCase(corruptEku: level); + } + + [Theory] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + public static void CorruptMappingNoChecks(int level) + { + RunCase(corruptMapping: true, mappingLevel: level); + } + + private static void RunCase( + int corruptCertificatePolicy = -1, + int corruptApplicationPolicy = -1, + int corruptEku = -1, + int mappingLevel = -1, + bool corruptMapping = false, + bool checkCertificatePolicy = false, + bool checkApplicationPolicy = false, + bool checkUnrelatedAppPolicy = false, + bool leafNotValidForUsage = false, + bool omitEku = false, + bool omitApplicationPolicy = false, + [CallerMemberName] string testName = null) + { + X509Certificate2[] certs = new X509Certificate2[5]; + + X509Extension appPolicy = omitApplicationPolicy ? null : s_applicationPolicyExtension; + X509Extension eku = omitEku ? null : s_ekuExtension; + + try + { + lock (s_keys) + { + TestDataGenerator.MakeTestChain( + s_keys, + certs, + endEntityExtensions: Extensions(0), + intermediateExtensions: [ + Extensions(1), + Extensions(2), + Extensions(3), + ], + rootExtensions: Extensions(4), + $"{testName}/{corruptCertificatePolicy}/{corruptApplicationPolicy}/{corruptEku}"); + } + + X509Extension[] Extensions(int level) + { + X509Extension policyExt = + level == corruptCertificatePolicy ? s_corruptPolicies : + corruptMapping ? s_unmappedPolicyExtension : + level < mappingLevel ? s_mappedPolicyExtension : s_unmappedPolicyExtension; + + X509Extension mapping = level == mappingLevel ? + corruptMapping ? s_corruptMapping : s_policyMapping : + null; + + X509Extension actualAppPolicy = + appPolicy is null ? null : + level == corruptApplicationPolicy ? s_corruptApplicationPolicy : appPolicy; + + X509Extension actualEku = + eku is null ? null : + level == corruptEku ? s_corruptEku : eku; + + return new X509Extension[] + { + level == 0 ? s_caFalse : s_caTrue, + actualAppPolicy, + actualEku, + policyExt, + mapping, + }; + } + + string errors = ""; + + using (ChainHolder holder = new ChainHolder()) + { + X509Chain chain = holder.Chain; + chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; + chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; + chain.ChainPolicy.CustomTrustStore.Add(certs[4]); + chain.ChainPolicy.ExtraStore.Add(certs[3]); + chain.ChainPolicy.ExtraStore.Add(certs[2]); + chain.ChainPolicy.ExtraStore.Add(certs[1]); + + if (checkCertificatePolicy) + { + chain.ChainPolicy.CertificatePolicy.Add(new Oid(CabfDvOid, null)); + } + + if (checkApplicationPolicy) + { + chain.ChainPolicy.ApplicationPolicy.Add(new Oid(TlsClientAuthOid, null)); + } + + if (checkUnrelatedAppPolicy) + { + chain.ChainPolicy.ApplicationPolicy.Add(new Oid(TlsServerAuthOid, null)); + } + + bool isValid = chain.Build(certs[0]); + X509ChainStatusFlags aggregateFlags = X509ChainStatusFlags.NoError; + int expectedLength = certs.Length; + bool detectCorruptEku = corruptEku >= 0 && omitApplicationPolicy; + + if (PlatformDetection.IsOpenSslSupported && corruptEku >= 0) + { + // The OpenSSL chain engine will stop processing the chain when it hits a corrupt EKU, + // so the chain length is shorter than expected. + expectedLength = int.Max(1, corruptEku); + detectCorruptEku = true; + } + + Assert.Equal(expectedLength, chain.ChainElements.Count); + + for (int i = expectedLength - 1; i >= 0; i--) + { + X509ChainStatusFlags expectedStatus = X509ChainStatusFlags.NoError; + + if (i == expectedLength - 1 && expectedLength < certs.Length) + { + expectedStatus |= X509ChainStatusFlags.PartialChain; + } + + if (corruptCertificatePolicy == i || + corruptApplicationPolicy == i || + (detectCorruptEku && corruptEku == i) || + (corruptMapping && mappingLevel == i)) + { + expectedStatus |= + X509ChainStatusFlags.InvalidExtension | + X509ChainStatusFlags.InvalidPolicyConstraints; + } + + if (i == 0 && leafNotValidForUsage) + { + expectedStatus |= X509ChainStatusFlags.NotValidForUsage; + } + + if (checkUnrelatedAppPolicy) + { + if (i < expectedLength - 1) + { + expectedStatus |= X509ChainStatusFlags.NotValidForUsage; + } + else if (corruptApplicationPolicy == i) + { + // If only the root has a corrupt application policy, + // then it is valid for all usages, so no error. + // But all lower CAs have scoped usages, so they still + // trigger NotValidForUsage along with the end-entity. + } + else if (omitApplicationPolicy && corruptEku == i) + { + // If only the root has a corrupt EKU, + // then it is valid for all usages, so no error. + // But all lower CAs have scoped usages, so they still + // trigger NotValidForUsage along with the end-entity. + } + else + { + // The root will still be scoped, so expect an error. + expectedStatus |= X509ChainStatusFlags.NotValidForUsage; + } + } + + aggregateFlags |= expectedStatus; + X509ChainStatusFlags actual = chain.ChainElements[i].AllStatusFlags(); + + if (expectedStatus != actual) + { + errors += $"Element {i}: Expected [{expectedStatus}], Actual [{actual}]{Environment.NewLine}"; + } + } + + if (aggregateFlags != chain.AllStatusFlags()) + { + errors += $"Aggregate: Expected [{aggregateFlags}], Actual [{chain.AllStatusFlags()}]{Environment.NewLine}"; + } + + if (errors.Length > 0) + { + throw new XunitException(errors); + } + + if (aggregateFlags == 0) + { + AssertExtensions.TrueExpression(isValid, "chain.Build(certs[0])"); + } + else + { + AssertExtensions.FalseExpression(isValid, "chain.Build(certs[0])"); + } + } + } + finally + { + foreach (X509Certificate2 cert in certs) + { + cert?.Dispose(); + } + } + } + } +} diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs index fa3d06d984d9fc..3f75ff21ff0e05 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs @@ -14,6 +14,8 @@ using Test.Cryptography; using Xunit; +using RSATestData = System.Security.Cryptography.Rsa.Tests.TestData; + namespace System.Security.Cryptography.X509Certificates.Tests { [SkipOnPlatform(TestPlatforms.Browser, "Browser doesn't support X.509 certificates")] @@ -1044,322 +1046,6 @@ public static void PolicyConstraints_Mapped() } } - [Theory] - [MemberData(nameof(ChainPolicyMemberData))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/128890", TestPlatforms.Android)] - public static void CertificatePolicyTest( - ChainPolicyTestCase testCase) - { - using (testCase) - { - TestChain4( - testCase.Root, - testCase.HighIntermediate, - testCase.LowIntermediate, - testCase.EndEntity, - PlatformPolicyConstraints(testCase.ExpectedFlags), - testCase.ConfigureCallback); - } - } - - public static IEnumerable ChainPolicyMemberData() - { - foreach (ChainPolicyTestCase testCase in TestCases()) - { - yield return new object[] { testCase }; - } - - static IEnumerable TestCases() - { - // Use the same keys for all chains, just to keep the total time low. - // There are enough cases here that keygen shows up in clock time. - RSA[] keys = [ RSA.Create(2048), RSA.Create(2048), RSA.Create(2048), RSA.Create(2048) ]; - - // These test cases only show in results by their test case number, - // because describing them in words would be very verbose. - // - // Skipped (unyielded) cases need to reserve their numbers so they're - // the same on all platforms. - // - // Ideally, new cases are added with higher values, so that test history - // isn't comparing apples and oranges. - - int caseId = 0; - - // No chain.Policy.CertificatePolicy checks, EE uses the mapped policy identifier, - // intermediate requires that the EE certs have a policy extension. - // - // Despite the intermediate specifying a mapping when it's forbidden (inhibit<2), - // Everything is reported valid, because the EE cert policy C doesn't require the mapping. - for (int rootInhibitMapping = 0; rootInhibitMapping <= 2; rootInhibitMapping++) - { - yield return ChainPolicyTestCase.Build( - caseId++, - keys, - rootInhibitMapping, - 0, - [ChainPolicyTestCase.PolicyB, ChainPolicyTestCase.PolicyC], - [], - X509ChainStatusFlags.NoError); - } - - Debug.Assert(caseId == 3); - - // No chain.Policy.CertificatePolicy checks, EE uses the mapped policy identifier, - // intermediate requires that the EE certs have a policy extension. - // - // The EE policy is required, and the only policy in the EE cert is the mapped one, - // but that is forbidden by the intermediate's inhibit=<2, so it's an - // Issuance-Chain-Policy violation. - for (int rootInhibitMapping = 0; rootInhibitMapping <= 2; rootInhibitMapping++) - { - // Windows seems to be the only OS capable of reporting this error. - - yield return ChainPolicyTestCase.Build( - caseId++, - keys, - rootInhibitMapping, - 0, - [ChainPolicyTestCase.PolicyB], - [], - rootInhibitMapping < 2 && OperatingSystem.IsWindows() ? - X509ChainStatusFlags.NoIssuanceChainPolicy : - X509ChainStatusFlags.NoError); - } - - Debug.Assert(caseId == 6); - - // No chain.Policy.CertificatePolicy checks, EE uses the mapped policy identifier, - // intermediate does not require that the EE certs have a policy extension. - // - // Even though the intermediate has a disallowed mapping when inhibit=<2, - // since the EE isn't _required_ to have a policy, nothing was required to - // traverse the map. - for (int rootInhibitMapping = 0; rootInhibitMapping <= 2; rootInhibitMapping++) - { - yield return ChainPolicyTestCase.Build( - caseId++, - keys, - rootInhibitMapping, - -1, - [ChainPolicyTestCase.PolicyB], - [], - X509ChainStatusFlags.NoError); - } - - Debug.Assert(caseId == 9); - - // EE uses the mapped policy identifier, - // intermediate requires that the EE certs have a policy extension. - // Require that the EE cert is valid for only policy C (no mapping required). - // - // Despite the intermediate specifying a mapping when it's forbidden (inhibit=<2), - // Everything is reported valid, because the EE cert policy C doesn't require the mapping. - for (int rootInhibitMapping = 0; rootInhibitMapping <= 2; rootInhibitMapping++) - { - yield return ChainPolicyTestCase.Build( - caseId++, - keys, - rootInhibitMapping, - 0, - [ChainPolicyTestCase.PolicyB, ChainPolicyTestCase.PolicyC], - [ChainPolicyTestCase.PolicyC], - X509ChainStatusFlags.NoError); - } - - Debug.Assert(caseId == 12); - - // EE uses the mapped policy identifier, - // intermediate does not require that the EE certs have a policy extension. - // Require that the EE cert is valid for only policy C (no mapping required). - // - // Despite the intermediate specifying a mapping when it's forbidden (inhibit=<2), - // Everything is reported valid, because the EE cert policy C doesn't require the mapping. - for (int rootInhibitMapping = 0; rootInhibitMapping <= 2; rootInhibitMapping++) - { - yield return ChainPolicyTestCase.Build( - caseId++, - keys, - rootInhibitMapping, - -1, - [ChainPolicyTestCase.PolicyB, ChainPolicyTestCase.PolicyC], - [ChainPolicyTestCase.PolicyC], - X509ChainStatusFlags.NoError); - } - - Debug.Assert(caseId == 15); - - // EE uses the mapped policy identifier, - // intermediate requires that the EE certs have a policy extension. - // Require that the EE cert is valid for only policy A (which it calls C). - // - // Since this requires traversing the mapping, it's NotValidForUsage whenever - // the mapping was disallowed (inhibit<2). - for (int rootInhibitMapping = 0; rootInhibitMapping <= 2; rootInhibitMapping++) - { - yield return ChainPolicyTestCase.Build( - caseId++, - keys, - rootInhibitMapping, - 0, - [ChainPolicyTestCase.PolicyB, ChainPolicyTestCase.PolicyC], - [ChainPolicyTestCase.PolicyA], - rootInhibitMapping < 2 ? - X509ChainStatusFlags.NotValidForUsage : - X509ChainStatusFlags.NoError); - } - - Debug.Assert(caseId == 18); - - foreach (RSA key in keys) - { - key.Dispose(); - } - } - } - - public sealed class ChainPolicyTestCase : IDisposable - { - internal const string PolicyA = "0.1.2.3"; - internal const string PolicyB = "1.2.3.4"; - internal const string PolicyC = "2.3.4.5"; - - private int _number; - private string[] _eePoliciesToCheck; - - internal X509ChainStatusFlags ExpectedFlags { get; private set; } - internal X509Certificate2 Root { get; private set; } - internal X509Certificate2 HighIntermediate { get; private set; } - internal X509Certificate2 LowIntermediate { get; private set; } - internal X509Certificate2 EndEntity { get; private set; } - - private ChainPolicyTestCase() - { - } - - public Action ConfigureCallback - { - get - { - if (_eePoliciesToCheck.Length == 0) - { - return null; - } - - return policy => - { - foreach (string policyOid in _eePoliciesToCheck) - { - policy.CertificatePolicy.Add(new Oid(policyOid, null)); - } - }; - } - } - - public void Dispose() - { - Root?.Dispose(); - HighIntermediate?.Dispose(); - LowIntermediate?.Dispose(); - EndEntity?.Dispose(); - } - - internal static ChainPolicyTestCase Build( - int number, - RSA[] keys, - int rootInhibitMapping, - int intermediateRequireExplicit, - string[] eePolicies, - string[] eePoliciesToCheck, - X509ChainStatusFlags expectedFlags) - { - X509Extension[] rootExtensions = new[] - { - BasicConstraintsCA, - MaybePolicyConstraints(inhibitPolicyMappingSkipCerts: rootInhibitMapping), - }; - - X509Extension[] highImedExtensions = new[] - { - BasicConstraintsCA, - // The "any" policy. - BuildPolicyByIdentifiers("2.5.29.32.0"), - }; - - X509Extension[] lowImedExtensions = new[] - { - BasicConstraintsCA, - MaybePolicyConstraints(requireExplicitPolicySkipCerts: intermediateRequireExplicit), - BuildPolicyByIdentifiers(PolicyA, PolicyC), - BuildPolicyMappings((PolicyA, PolicyB)), - }; - - X509Extension[] endEntityExtensions = new[] - { - BasicConstraintsEndEntity, - MaybePolicies(eePolicies), - }; - - X509Certificate2[] certs = new X509Certificate2[4]; - - TestDataGenerator.MakeTestChain( - keys, - certs, - endEntityExtensions, - [lowImedExtensions, highImedExtensions], - rootExtensions, - $"{nameof(ChainPolicyTestCase)}-{number}"); - - return new ChainPolicyTestCase - { - _number = number, - EndEntity = certs[0], - LowIntermediate = certs[1], - HighIntermediate = certs[2], - Root = certs[3], - _eePoliciesToCheck = eePoliciesToCheck, - ExpectedFlags = expectedFlags, - }; - - static X509Extension MaybePolicies(string[] policyOids) - { - if (policyOids.Length == 0) - { - return null; - } - - return BuildPolicyByIdentifiers(policyOids); - } - - static X509Extension MaybePolicyConstraints( - int requireExplicitPolicySkipCerts = -1, - int inhibitPolicyMappingSkipCerts = -1) - { - if (inhibitPolicyMappingSkipCerts >= 0 && requireExplicitPolicySkipCerts >= 0) - { - return BuildPolicyConstraints(inhibitPolicyMappingSkipCerts, requireExplicitPolicySkipCerts); - } - - if (inhibitPolicyMappingSkipCerts >= 0) - { - return BuildPolicyConstraints(inhibitPolicyMappingSkipCerts: inhibitPolicyMappingSkipCerts); - } - - if (requireExplicitPolicySkipCerts >= 0) - { - return BuildPolicyConstraints(requireExplicitPolicySkipCerts: requireExplicitPolicySkipCerts); - } - - return null; - } - } - - public override string ToString() - { - return $"CaseID {_number}"; - } - } - public enum BuildChainWithNotSignatureValidTest : int { TrustedRoot, @@ -1496,7 +1182,7 @@ private static X509ChainStatusFlags PlatformNameConstraints(X509ChainStatusFlags return flags; } - private static X509ChainStatusFlags PlatformPolicyConstraints(X509ChainStatusFlags flags) + internal static X509ChainStatusFlags PlatformPolicyConstraints(X509ChainStatusFlags flags) { if (PlatformDetection.UsesAppleCrypto) { @@ -1580,7 +1266,7 @@ private static X509Certificate2 TamperSignature(X509Certificate2 input) return new X509Certificate2(cert); } - private static X509Extension BuildPolicyConstraints( + internal static X509Extension BuildPolicyConstraints( int? requireExplicitPolicySkipCerts = null, int? inhibitPolicyMappingSkipCerts = null) { @@ -1611,7 +1297,7 @@ private static X509Extension BuildPolicyConstraints( return new X509Extension("2.5.29.36", writer.Encode(), critical: true); } - private static X509Extension BuildPolicyByIdentifiers(params string[] policyOids) + internal static X509Extension BuildPolicyByIdentifiers(params string[] policyOids) { // id-ce-certificatePolicies OBJECT IDENTIFIER ::= { id-ce 32 } @@ -1625,6 +1311,15 @@ private static X509Extension BuildPolicyByIdentifiers(params string[] policyOids // PolicyQualifierInfo OPTIONAL } // CertPolicyId ::= OBJECT IDENTIFIER + return new X509Extension("2.5.29.32", EncodeCertificatePoliciesValue(policyOids), critical: false); + } + + // Produces the DER value shared by the RFC 5280 certificatePolicies (2.5.29.32) extension + // and the Microsoft szOID_APPLICATION_CERT_POLICIES (1.3.6.1.4.1.311.21.10) extension, which + // are structurally identical: SEQUENCE OF PolicyInformation, PolicyInformation ::= SEQUENCE { + // policyIdentifier OBJECT IDENTIFIER, policyQualifiers ... OPTIONAL }. + internal static byte[] EncodeCertificatePoliciesValue(params string[] policyOids) + { AsnWriter writer = new AsnWriter(AsnEncodingRules.DER); using (writer.PushSequence()) //CertificatePolicies @@ -1638,10 +1333,10 @@ private static X509Extension BuildPolicyByIdentifiers(params string[] policyOids } } - return new X509Extension("2.5.29.32", writer.Encode(), critical: false); + return writer.Encode(); } - private static X509Extension BuildPolicyMappings( + internal static X509Extension BuildPolicyMappings( params (string IssuerDomainPolicy, string SubjectDomainPolicy)[] policyMappings) { // PolicyMappings ::= SEQUENCE SIZE (1..MAX) OF SEQUENCE { @@ -1666,7 +1361,7 @@ private static X509Extension BuildPolicyMappings( return new X509Extension("2.5.29.33", writer.Encode(), critical: true); } - private static void TestChain4( + internal static void TestChain4( X509Certificate2 rootCertificate, X509Certificate2 highIntermediateCertificate, X509Certificate2 lowIntermediateCertificate, @@ -1697,12 +1392,13 @@ private static void TestChain4( } } - private static void TestChain3( + internal static void TestChain3( X509Certificate2 rootCertificate, X509Certificate2 intermediateCertificate, X509Certificate2 endEntityCertificate, X509ChainStatusFlags expectedFlags = X509ChainStatusFlags.NoError, - Action configurePolicy = null) + Action configurePolicy = null, + Action extraVerify = null) { using (ChainHolder chainHolder = new ChainHolder()) { @@ -1723,6 +1419,11 @@ private static void TestChain3( Assert.True( actualFlags.HasFlag(expectedFlags), $"Expected Flags: \"{expectedFlags}\"; Actual Flags: \"{actualFlags}\""); + + if (extraVerify is not null) + { + extraVerify(chain); + } } } } diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/RevocationTests/DynamicRevocationTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/RevocationTests/DynamicRevocationTests.cs index 015166f8f776c4..c5b87f2e7532fb 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/RevocationTests/DynamicRevocationTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/RevocationTests/DynamicRevocationTests.cs @@ -573,7 +573,6 @@ public static void RevokeIntermediate_PolicyErrors_NotTimeValid(bool policyError } X509ChainStatusFlags leafProblems = X509ChainStatusFlags.NoError; - X509ChainStatusFlags issuerExtraProblems = X509ChainStatusFlags.NoError; if (notTimeValid) { @@ -589,21 +588,14 @@ public static void RevokeIntermediate_PolicyErrors_NotTimeValid(bool policyError { chain.ChainPolicy.ApplicationPolicy.Add(s_tlsServerOid); leafProblems |= X509ChainStatusFlags.NotValidForUsage; - - // [ActiveIssue("https://github.com/dotnet/runtime/issues/31246")] - // Linux reports this code at more levels than Windows does. - if (OperatingSystem.IsLinux()) - { - issuerExtraProblems |= X509ChainStatusFlags.NotValidForUsage; - } } bool chainBuilt = chain.Build(endEntity); AssertChainStatus( chain, - rootStatus: issuerExtraProblems, - issrStatus: issuerExtraProblems | X509ChainStatusFlags.Revoked, + rootStatus: X509ChainStatusFlags.NoError, + issrStatus: X509ChainStatusFlags.NoError | X509ChainStatusFlags.Revoked, leafStatus: leafProblems | ThisOsRevocationStatusUnknown); Assert.False(chainBuilt, "Chain built with ExcludeRoot."); @@ -615,8 +607,8 @@ public static void RevokeIntermediate_PolicyErrors_NotTimeValid(bool policyError AssertChainStatus( chain, - rootStatus: issuerExtraProblems, - issrStatus: issuerExtraProblems, + rootStatus: X509ChainStatusFlags.NoError, + issrStatus: X509ChainStatusFlags.NoError, leafStatus: leafProblems); Assert.False(chainBuilt, "Chain built with EndCertificateOnly (no ignore flags)"); @@ -630,8 +622,8 @@ public static void RevokeIntermediate_PolicyErrors_NotTimeValid(bool policyError AssertChainStatus( chain, - rootStatus: issuerExtraProblems, - issrStatus: issuerExtraProblems, + rootStatus: X509ChainStatusFlags.NoError, + issrStatus: X509ChainStatusFlags.NoError, leafStatus: leafProblems); Assert.True(chainBuilt, "Chain built with EndCertificateOnly (with ignore flags)"); @@ -654,7 +646,6 @@ public static void RevokeEndEntity_PolicyErrors_NotTimeValid(bool policyErrors, intermediate.Revoke(endEntity, now); X509ChainStatusFlags leafProblems = X509ChainStatusFlags.NoError; - X509ChainStatusFlags issuerExtraProblems = X509ChainStatusFlags.NoError; if (notTimeValid) { @@ -670,21 +661,14 @@ public static void RevokeEndEntity_PolicyErrors_NotTimeValid(bool policyErrors, { chain.ChainPolicy.ApplicationPolicy.Add(s_tlsServerOid); leafProblems |= X509ChainStatusFlags.NotValidForUsage; - - // [ActiveIssue("https://github.com/dotnet/runtime/issues/31246")] - // Linux reports this code at more levels than Windows does. - if (!OperatingSystem.IsWindows()) - { - issuerExtraProblems |= X509ChainStatusFlags.NotValidForUsage; - } } bool chainBuilt = chain.Build(endEntity); AssertChainStatus( chain, - rootStatus: issuerExtraProblems, - issrStatus: issuerExtraProblems, + rootStatus: X509ChainStatusFlags.NoError, + issrStatus: X509ChainStatusFlags.NoError, leafStatus: leafProblems | X509ChainStatusFlags.Revoked); Assert.False(chainBuilt, "Chain built with ExcludeRoot."); @@ -696,8 +680,8 @@ public static void RevokeEndEntity_PolicyErrors_NotTimeValid(bool policyErrors, AssertChainStatus( chain, - rootStatus: issuerExtraProblems, - issrStatus: issuerExtraProblems, + rootStatus: X509ChainStatusFlags.NoError, + issrStatus: X509ChainStatusFlags.NoError, leafStatus: leafProblems | X509ChainStatusFlags.Revoked); Assert.False(chainBuilt, "Chain built with EndCertificateOnly (no ignore flags)"); @@ -711,8 +695,8 @@ public static void RevokeEndEntity_PolicyErrors_NotTimeValid(bool policyErrors, AssertChainStatus( chain, - rootStatus: issuerExtraProblems, - issrStatus: issuerExtraProblems, + rootStatus: X509ChainStatusFlags.NoError, + issrStatus: X509ChainStatusFlags.NoError, leafStatus: leafProblems | X509ChainStatusFlags.Revoked); Assert.False(chainBuilt, "Chain built with EndCertificateOnly (with ignore flags)");