Normalize X509Chain App/Cert policies across OSes

[bartonjs]

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.

Fixes#31246 (and maybe others)

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[bartonjs]

/azp run runtime-libraries-coreclr outerloop

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

This PR updates X509Chain policy processing so ApplicationPolicy/CertificatePolicy handling (and invalid/undecodable policy-related extensions) is evaluated and reported more consistently across Windows/OpenSSL/Apple/Android, and adjusts/expands tests to match the unified behavior.

Changes:

• Refactors CertificatePolicyChain to compute per-chain-element “encoding” vs “usage” errors and exposes helpers to reuse the same logic across platform chain processors.
• Updates OpenSSL, Apple, and Android chain processors to merge policy/encoding errors into chain + element status consistently (and to detect encoding issues even when no explicit policy filtering is requested).
• Normalizes existing tests’ OS-conditional expectations and adds focused test suites for app-policy vs EKU behavior and corrupt policy-related extensions.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/libraries/System.Security.Cryptography/tests/X509Certificates/RevocationTests/DynamicRevocationTests.cs	Removes OS-conditional expectations for NotValidForUsage at non-leaf levels.
src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs	Refactors/makes helpers reusable by new tests; factors out shared DER encoding for policy extensions.
src/libraries/System.Security.Cryptography/tests/X509Certificates/CorruptPoliciesChainTests.cs	New coverage for corrupt/undecodable policy/EKU-related extensions and expected chain-element status behavior.
src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs	Normalizes expectations around NotValidForUsage across platforms.
src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs	New tests covering certificate policy constraints/mappings plus Application Policies vs EKU interactions.
src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj	Adds the new test files to the test project.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs	Splits policy evaluation into “merge errors” and “process policy”; adds encoding-only validation when no policy filters are requested.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Apple.cs	Switches to shared policy-chain logic and per-element error attribution.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs	Switches to shared policy-chain logic and per-element error attribution.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs	Implements CertificatePolicyChain.Build/CheckEncodingOnly, per-element error vectors, and revised EKU/app-policy semantics.
src/libraries/Common/src/System/Security/Cryptography/Oids.cs	Adds AnyEnhancedKeyUsage constant used in policy evaluation.

[Copilot]

Pull request overview

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

Suppressed comments (2)

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:661

• ErrorVector's scalar bit test uses (1 << index) (an int shift). For indices >= 31 this overflows/sign-extends and will report the wrong bit, so long chains can misattribute policy/encoding errors to the wrong element.

 internal bool this[int index]
{
get
{
if (_vector is null)
{
return (_scalar & (1 << index)) != 0;
}

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:456

• The comment about only checking EKU “for structural validity” when ApplicationCertPolicies is present but corrupt is misleading: this block currently skips EKU entirely whenever ApplicationCertPolicies is present (even if it failed to decode). Either update the comment to match the behavior, or add the intended validation call.

 if (policyData.EnhancedKeyUsage != null)
{
// 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);
}

[Copilot]

Pull request overview

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

Suppressed comments (3)

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:456

• ReadExtendedKeyUsageExtension can throw CryptographicException on malformed EKU. In this code path (policy filtering active and no ApplicationCertPolicies extension), the exception is not caught, which would cause chain building to throw instead of reporting InvalidExtension/InvalidPolicyConstraints via encodingErrors.

 // should stay null.
if (policyData.ApplicationCertPolicies is null)
{
applicationCertPolicies = ReadExtendedKeyUsageExtension(policyData.EnhancedKeyUsage);
}

src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs:18

• The RSATestData using-alias appears to be unused, which can trigger CS8019/IDE0005 in builds that enforce unused usings.

using RSATestData = System.Security.Cryptography.Rsa.Tests.TestData;

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs:424

• InvalidPolicyConstraints is being reported with SR.Chain_NoPolicyMatch, which describes a policy mismatch rather than a malformed/invalid extension. This can make X509ChainStatus.StatusInformation misleading for encoding failures.

 Status = X509ChainStatusFlags.InvalidPolicyConstraints,
// "NoPolicyMatch" says that the policy is "invalid", which works for this one, too.
StatusInformation = SR.Chain_NoPolicyMatch,

[Copilot]

Pull request overview

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

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

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

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs:1147

• Use UtcNow instead of Now for certificate validity timestamps to avoid time zone/DST sensitivity in CI and local runs.

 DateTimeOffset notBefore = DateTimeOffset.Now.AddMinutes(-5);

src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs:235

• Remove trailing whitespace in this initializer (it can cause noisy diffs and violates common formatting expectations).

 X509BasicConstraintsExtension.CreateForCertificateAuthority(), 

[bartonjs]

I've trawled the logs, and I attest, to the best of my ability, that none of the extra-platforms test failures are caused by this change.

(And I'm astounded at how many of them report failure when all tests passed and the runner script reports that it's returning success)

[bartonjs]

/ba-g Many DeadLetters, other failures were investigated.

[bartonjs]

/backport-to release/11.0

[bartonjs]

/backport to release/11.0

[github-actions]

Started backporting to release/11.0 (link to workflow run)

[bartonjs]

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.

Fixes#31246 (and maybe others)

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[bartonjs]

/azp run runtime-libraries-coreclr outerloop

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

This PR updates X509Chain policy processing so ApplicationPolicy/CertificatePolicy handling (and invalid/undecodable policy-related extensions) is evaluated and reported more consistently across Windows/OpenSSL/Apple/Android, and adjusts/expands tests to match the unified behavior.

Changes:

• Refactors CertificatePolicyChain to compute per-chain-element “encoding” vs “usage” errors and exposes helpers to reuse the same logic across platform chain processors.
• Updates OpenSSL, Apple, and Android chain processors to merge policy/encoding errors into chain + element status consistently (and to detect encoding issues even when no explicit policy filtering is requested).
• Normalizes existing tests’ OS-conditional expectations and adds focused test suites for app-policy vs EKU behavior and corrupt policy-related extensions.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/libraries/System.Security.Cryptography/tests/X509Certificates/RevocationTests/DynamicRevocationTests.cs	Removes OS-conditional expectations for NotValidForUsage at non-leaf levels.
src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs	Refactors/makes helpers reusable by new tests; factors out shared DER encoding for policy extensions.
src/libraries/System.Security.Cryptography/tests/X509Certificates/CorruptPoliciesChainTests.cs	New coverage for corrupt/undecodable policy/EKU-related extensions and expected chain-element status behavior.
src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs	Normalizes expectations around NotValidForUsage across platforms.
src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs	New tests covering certificate policy constraints/mappings plus Application Policies vs EKU interactions.
src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj	Adds the new test files to the test project.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs	Splits policy evaluation into “merge errors” and “process policy”; adds encoding-only validation when no policy filters are requested.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Apple.cs	Switches to shared policy-chain logic and per-element error attribution.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs	Switches to shared policy-chain logic and per-element error attribution.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs	Implements CertificatePolicyChain.Build/CheckEncodingOnly, per-element error vectors, and revised EKU/app-policy semantics.
src/libraries/Common/src/System/Security/Cryptography/Oids.cs	Adds AnyEnhancedKeyUsage constant used in policy evaluation.

[Copilot]

Pull request overview

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

Suppressed comments (2)

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:661

• ErrorVector's scalar bit test uses (1 << index) (an int shift). For indices >= 31 this overflows/sign-extends and will report the wrong bit, so long chains can misattribute policy/encoding errors to the wrong element.

 internal bool this[int index]
{
get
{
if (_vector is null)
{
return (_scalar & (1 << index)) != 0;
}

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:456

• The comment about only checking EKU “for structural validity” when ApplicationCertPolicies is present but corrupt is misleading: this block currently skips EKU entirely whenever ApplicationCertPolicies is present (even if it failed to decode). Either update the comment to match the behavior, or add the intended validation call.

 if (policyData.EnhancedKeyUsage != null)
{
// 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);
}

[Copilot]

Pull request overview

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

Suppressed comments (3)

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:456

• ReadExtendedKeyUsageExtension can throw CryptographicException on malformed EKU. In this code path (policy filtering active and no ApplicationCertPolicies extension), the exception is not caught, which would cause chain building to throw instead of reporting InvalidExtension/InvalidPolicyConstraints via encodingErrors.

 // should stay null.
if (policyData.ApplicationCertPolicies is null)
{
applicationCertPolicies = ReadExtendedKeyUsageExtension(policyData.EnhancedKeyUsage);
}

src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs:18

• The RSATestData using-alias appears to be unused, which can trigger CS8019/IDE0005 in builds that enforce unused usings.

using RSATestData = System.Security.Cryptography.Rsa.Tests.TestData;

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs:424

• InvalidPolicyConstraints is being reported with SR.Chain_NoPolicyMatch, which describes a policy mismatch rather than a malformed/invalid extension. This can make X509ChainStatus.StatusInformation misleading for encoding failures.

 Status = X509ChainStatusFlags.InvalidPolicyConstraints,
// "NoPolicyMatch" says that the policy is "invalid", which works for this one, too.
StatusInformation = SR.Chain_NoPolicyMatch,

[Copilot]

Pull request overview

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

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

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

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs:1147

• Use UtcNow instead of Now for certificate validity timestamps to avoid time zone/DST sensitivity in CI and local runs.

 DateTimeOffset notBefore = DateTimeOffset.Now.AddMinutes(-5);

src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs:235

• Remove trailing whitespace in this initializer (it can cause noisy diffs and violates common formatting expectations).

 X509BasicConstraintsExtension.CreateForCertificateAuthority(), 

[bartonjs]

I've trawled the logs, and I attest, to the best of my ability, that none of the extra-platforms test failures are caused by this change.

(And I'm astounded at how many of them report failure when all tests passed and the runner script reports that it's returning success)

[bartonjs]

/ba-g Many DeadLetters, other failures were investigated.

[bartonjs]

/backport-to release/11.0

[bartonjs]

/backport to release/11.0

[github-actions]

Started backporting to release/11.0 (link to workflow run)

[bartonjs]

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.

Fixes#31246 (and maybe others)

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[bartonjs]

/azp run runtime-libraries-coreclr outerloop

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

This PR updates X509Chain policy processing so ApplicationPolicy/CertificatePolicy handling (and invalid/undecodable policy-related extensions) is evaluated and reported more consistently across Windows/OpenSSL/Apple/Android, and adjusts/expands tests to match the unified behavior.

Changes:

• Refactors CertificatePolicyChain to compute per-chain-element “encoding” vs “usage” errors and exposes helpers to reuse the same logic across platform chain processors.
• Updates OpenSSL, Apple, and Android chain processors to merge policy/encoding errors into chain + element status consistently (and to detect encoding issues even when no explicit policy filtering is requested).
• Normalizes existing tests’ OS-conditional expectations and adds focused test suites for app-policy vs EKU behavior and corrupt policy-related extensions.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/libraries/System.Security.Cryptography/tests/X509Certificates/RevocationTests/DynamicRevocationTests.cs	Removes OS-conditional expectations for NotValidForUsage at non-leaf levels.
src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs	Refactors/makes helpers reusable by new tests; factors out shared DER encoding for policy extensions.
src/libraries/System.Security.Cryptography/tests/X509Certificates/CorruptPoliciesChainTests.cs	New coverage for corrupt/undecodable policy/EKU-related extensions and expected chain-element status behavior.
src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs	Normalizes expectations around NotValidForUsage across platforms.
src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs	New tests covering certificate policy constraints/mappings plus Application Policies vs EKU interactions.
src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj	Adds the new test files to the test project.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs	Splits policy evaluation into “merge errors” and “process policy”; adds encoding-only validation when no policy filters are requested.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Apple.cs	Switches to shared policy-chain logic and per-element error attribution.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs	Switches to shared policy-chain logic and per-element error attribution.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs	Implements CertificatePolicyChain.Build/CheckEncodingOnly, per-element error vectors, and revised EKU/app-policy semantics.
src/libraries/Common/src/System/Security/Cryptography/Oids.cs	Adds AnyEnhancedKeyUsage constant used in policy evaluation.

[Copilot]

Pull request overview

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

Suppressed comments (2)

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:661

• ErrorVector's scalar bit test uses (1 << index) (an int shift). For indices >= 31 this overflows/sign-extends and will report the wrong bit, so long chains can misattribute policy/encoding errors to the wrong element.

 internal bool this[int index]
{
get
{
if (_vector is null)
{
return (_scalar & (1 << index)) != 0;
}

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:456

• The comment about only checking EKU “for structural validity” when ApplicationCertPolicies is present but corrupt is misleading: this block currently skips EKU entirely whenever ApplicationCertPolicies is present (even if it failed to decode). Either update the comment to match the behavior, or add the intended validation call.

 if (policyData.EnhancedKeyUsage != null)
{
// 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);
}

[Copilot]

Pull request overview

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

Suppressed comments (3)

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:456

• ReadExtendedKeyUsageExtension can throw CryptographicException on malformed EKU. In this code path (policy filtering active and no ApplicationCertPolicies extension), the exception is not caught, which would cause chain building to throw instead of reporting InvalidExtension/InvalidPolicyConstraints via encodingErrors.

 // should stay null.
if (policyData.ApplicationCertPolicies is null)
{
applicationCertPolicies = ReadExtendedKeyUsageExtension(policyData.EnhancedKeyUsage);
}

src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs:18

• The RSATestData using-alias appears to be unused, which can trigger CS8019/IDE0005 in builds that enforce unused usings.

using RSATestData = System.Security.Cryptography.Rsa.Tests.TestData;

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs:424

• InvalidPolicyConstraints is being reported with SR.Chain_NoPolicyMatch, which describes a policy mismatch rather than a malformed/invalid extension. This can make X509ChainStatus.StatusInformation misleading for encoding failures.

 Status = X509ChainStatusFlags.InvalidPolicyConstraints,
// "NoPolicyMatch" says that the policy is "invalid", which works for this one, too.
StatusInformation = SR.Chain_NoPolicyMatch,

[Copilot]

Pull request overview

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

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

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

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs:1147

• Use UtcNow instead of Now for certificate validity timestamps to avoid time zone/DST sensitivity in CI and local runs.

 DateTimeOffset notBefore = DateTimeOffset.Now.AddMinutes(-5);

src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs:235

• Remove trailing whitespace in this initializer (it can cause noisy diffs and violates common formatting expectations).

 X509BasicConstraintsExtension.CreateForCertificateAuthority(), 

[bartonjs]

I've trawled the logs, and I attest, to the best of my ability, that none of the extra-platforms test failures are caused by this change.

(And I'm astounded at how many of them report failure when all tests passed and the runner script reports that it's returning success)

[bartonjs]

/ba-g Many DeadLetters, other failures were investigated.

[bartonjs]

/backport-to release/11.0

[bartonjs]

/backport to release/11.0

[github-actions]

Started backporting to release/11.0 (link to workflow run)

[bartonjs]

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.

Fixes#31246 (and maybe others)

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[bartonjs]

/azp run runtime-libraries-coreclr outerloop

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

This PR updates X509Chain policy processing so ApplicationPolicy/CertificatePolicy handling (and invalid/undecodable policy-related extensions) is evaluated and reported more consistently across Windows/OpenSSL/Apple/Android, and adjusts/expands tests to match the unified behavior.

Changes:

• Refactors CertificatePolicyChain to compute per-chain-element “encoding” vs “usage” errors and exposes helpers to reuse the same logic across platform chain processors.
• Updates OpenSSL, Apple, and Android chain processors to merge policy/encoding errors into chain + element status consistently (and to detect encoding issues even when no explicit policy filtering is requested).
• Normalizes existing tests’ OS-conditional expectations and adds focused test suites for app-policy vs EKU behavior and corrupt policy-related extensions.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/libraries/System.Security.Cryptography/tests/X509Certificates/RevocationTests/DynamicRevocationTests.cs	Removes OS-conditional expectations for NotValidForUsage at non-leaf levels.
src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs	Refactors/makes helpers reusable by new tests; factors out shared DER encoding for policy extensions.
src/libraries/System.Security.Cryptography/tests/X509Certificates/CorruptPoliciesChainTests.cs	New coverage for corrupt/undecodable policy/EKU-related extensions and expected chain-element status behavior.
src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs	Normalizes expectations around NotValidForUsage across platforms.
src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs	New tests covering certificate policy constraints/mappings plus Application Policies vs EKU interactions.
src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj	Adds the new test files to the test project.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs	Splits policy evaluation into “merge errors” and “process policy”; adds encoding-only validation when no policy filters are requested.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Apple.cs	Switches to shared policy-chain logic and per-element error attribution.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs	Switches to shared policy-chain logic and per-element error attribution.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs	Implements CertificatePolicyChain.Build/CheckEncodingOnly, per-element error vectors, and revised EKU/app-policy semantics.
src/libraries/Common/src/System/Security/Cryptography/Oids.cs	Adds AnyEnhancedKeyUsage constant used in policy evaluation.

[Copilot]

Pull request overview

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

Suppressed comments (2)

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:661

• ErrorVector's scalar bit test uses (1 << index) (an int shift). For indices >= 31 this overflows/sign-extends and will report the wrong bit, so long chains can misattribute policy/encoding errors to the wrong element.

 internal bool this[int index]
{
get
{
if (_vector is null)
{
return (_scalar & (1 << index)) != 0;
}

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:456

• The comment about only checking EKU “for structural validity” when ApplicationCertPolicies is present but corrupt is misleading: this block currently skips EKU entirely whenever ApplicationCertPolicies is present (even if it failed to decode). Either update the comment to match the behavior, or add the intended validation call.

 if (policyData.EnhancedKeyUsage != null)
{
// 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);
}

[Copilot]

Pull request overview

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

Suppressed comments (3)

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:456

• ReadExtendedKeyUsageExtension can throw CryptographicException on malformed EKU. In this code path (policy filtering active and no ApplicationCertPolicies extension), the exception is not caught, which would cause chain building to throw instead of reporting InvalidExtension/InvalidPolicyConstraints via encodingErrors.

 // should stay null.
if (policyData.ApplicationCertPolicies is null)
{
applicationCertPolicies = ReadExtendedKeyUsageExtension(policyData.EnhancedKeyUsage);
}

src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs:18

• The RSATestData using-alias appears to be unused, which can trigger CS8019/IDE0005 in builds that enforce unused usings.

using RSATestData = System.Security.Cryptography.Rsa.Tests.TestData;

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs:424

• InvalidPolicyConstraints is being reported with SR.Chain_NoPolicyMatch, which describes a policy mismatch rather than a malformed/invalid extension. This can make X509ChainStatus.StatusInformation misleading for encoding failures.

 Status = X509ChainStatusFlags.InvalidPolicyConstraints,
// "NoPolicyMatch" says that the policy is "invalid", which works for this one, too.
StatusInformation = SR.Chain_NoPolicyMatch,

[Copilot]

Pull request overview

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

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

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

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs:1147

• Use UtcNow instead of Now for certificate validity timestamps to avoid time zone/DST sensitivity in CI and local runs.

 DateTimeOffset notBefore = DateTimeOffset.Now.AddMinutes(-5);

src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs:235

• Remove trailing whitespace in this initializer (it can cause noisy diffs and violates common formatting expectations).

 X509BasicConstraintsExtension.CreateForCertificateAuthority(), 

[bartonjs]

I've trawled the logs, and I attest, to the best of my ability, that none of the extra-platforms test failures are caused by this change.

(And I'm astounded at how many of them report failure when all tests passed and the runner script reports that it's returning success)

[bartonjs]

/ba-g Many DeadLetters, other failures were investigated.

[bartonjs]

/backport-to release/11.0

[bartonjs]

/backport to release/11.0

[github-actions]

Started backporting to release/11.0 (link to workflow run)

[bartonjs]

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.

Fixes#31246 (and maybe others)

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[bartonjs]

/azp run runtime-libraries-coreclr outerloop

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

This PR updates X509Chain policy processing so ApplicationPolicy/CertificatePolicy handling (and invalid/undecodable policy-related extensions) is evaluated and reported more consistently across Windows/OpenSSL/Apple/Android, and adjusts/expands tests to match the unified behavior.

Changes:

• Refactors CertificatePolicyChain to compute per-chain-element “encoding” vs “usage” errors and exposes helpers to reuse the same logic across platform chain processors.
• Updates OpenSSL, Apple, and Android chain processors to merge policy/encoding errors into chain + element status consistently (and to detect encoding issues even when no explicit policy filtering is requested).
• Normalizes existing tests’ OS-conditional expectations and adds focused test suites for app-policy vs EKU behavior and corrupt policy-related extensions.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/libraries/System.Security.Cryptography/tests/X509Certificates/RevocationTests/DynamicRevocationTests.cs	Removes OS-conditional expectations for NotValidForUsage at non-leaf levels.
src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs	Refactors/makes helpers reusable by new tests; factors out shared DER encoding for policy extensions.
src/libraries/System.Security.Cryptography/tests/X509Certificates/CorruptPoliciesChainTests.cs	New coverage for corrupt/undecodable policy/EKU-related extensions and expected chain-element status behavior.
src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs	Normalizes expectations around NotValidForUsage across platforms.
src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs	New tests covering certificate policy constraints/mappings plus Application Policies vs EKU interactions.
src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj	Adds the new test files to the test project.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs	Splits policy evaluation into “merge errors” and “process policy”; adds encoding-only validation when no policy filters are requested.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Apple.cs	Switches to shared policy-chain logic and per-element error attribution.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs	Switches to shared policy-chain logic and per-element error attribution.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs	Implements CertificatePolicyChain.Build/CheckEncodingOnly, per-element error vectors, and revised EKU/app-policy semantics.
src/libraries/Common/src/System/Security/Cryptography/Oids.cs	Adds AnyEnhancedKeyUsage constant used in policy evaluation.

[Copilot]

Pull request overview

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

Suppressed comments (2)

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:661

• ErrorVector's scalar bit test uses (1 << index) (an int shift). For indices >= 31 this overflows/sign-extends and will report the wrong bit, so long chains can misattribute policy/encoding errors to the wrong element.

 internal bool this[int index]
{
get
{
if (_vector is null)
{
return (_scalar & (1 << index)) != 0;
}

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:456

• The comment about only checking EKU “for structural validity” when ApplicationCertPolicies is present but corrupt is misleading: this block currently skips EKU entirely whenever ApplicationCertPolicies is present (even if it failed to decode). Either update the comment to match the behavior, or add the intended validation call.

 if (policyData.EnhancedKeyUsage != null)
{
// 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);
}

[Copilot]

Pull request overview

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

Suppressed comments (3)

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:456

• ReadExtendedKeyUsageExtension can throw CryptographicException on malformed EKU. In this code path (policy filtering active and no ApplicationCertPolicies extension), the exception is not caught, which would cause chain building to throw instead of reporting InvalidExtension/InvalidPolicyConstraints via encodingErrors.

 // should stay null.
if (policyData.ApplicationCertPolicies is null)
{
applicationCertPolicies = ReadExtendedKeyUsageExtension(policyData.EnhancedKeyUsage);
}

src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs:18

• The RSATestData using-alias appears to be unused, which can trigger CS8019/IDE0005 in builds that enforce unused usings.

using RSATestData = System.Security.Cryptography.Rsa.Tests.TestData;

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs:424

• InvalidPolicyConstraints is being reported with SR.Chain_NoPolicyMatch, which describes a policy mismatch rather than a malformed/invalid extension. This can make X509ChainStatus.StatusInformation misleading for encoding failures.

 Status = X509ChainStatusFlags.InvalidPolicyConstraints,
// "NoPolicyMatch" says that the policy is "invalid", which works for this one, too.
StatusInformation = SR.Chain_NoPolicyMatch,

[Copilot]

Pull request overview

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

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

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

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs:1147

• Use UtcNow instead of Now for certificate validity timestamps to avoid time zone/DST sensitivity in CI and local runs.

 DateTimeOffset notBefore = DateTimeOffset.Now.AddMinutes(-5);

src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs:235

• Remove trailing whitespace in this initializer (it can cause noisy diffs and violates common formatting expectations).

 X509BasicConstraintsExtension.CreateForCertificateAuthority(), 

[bartonjs]

I've trawled the logs, and I attest, to the best of my ability, that none of the extra-platforms test failures are caused by this change.

(And I'm astounded at how many of them report failure when all tests passed and the runner script reports that it's returning success)

[bartonjs]

/ba-g Many DeadLetters, other failures were investigated.

[bartonjs]

/backport-to release/11.0

[bartonjs]

/backport to release/11.0

[github-actions]

Started backporting to release/11.0 (link to workflow run)

[bartonjs]

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.

Fixes#31246 (and maybe others)

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[bartonjs]

/azp run runtime-libraries-coreclr outerloop

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

This PR updates X509Chain policy processing so ApplicationPolicy/CertificatePolicy handling (and invalid/undecodable policy-related extensions) is evaluated and reported more consistently across Windows/OpenSSL/Apple/Android, and adjusts/expands tests to match the unified behavior.

Changes:

• Refactors CertificatePolicyChain to compute per-chain-element “encoding” vs “usage” errors and exposes helpers to reuse the same logic across platform chain processors.
• Updates OpenSSL, Apple, and Android chain processors to merge policy/encoding errors into chain + element status consistently (and to detect encoding issues even when no explicit policy filtering is requested).
• Normalizes existing tests’ OS-conditional expectations and adds focused test suites for app-policy vs EKU behavior and corrupt policy-related extensions.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/libraries/System.Security.Cryptography/tests/X509Certificates/RevocationTests/DynamicRevocationTests.cs	Removes OS-conditional expectations for NotValidForUsage at non-leaf levels.
src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs	Refactors/makes helpers reusable by new tests; factors out shared DER encoding for policy extensions.
src/libraries/System.Security.Cryptography/tests/X509Certificates/CorruptPoliciesChainTests.cs	New coverage for corrupt/undecodable policy/EKU-related extensions and expected chain-element status behavior.
src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs	Normalizes expectations around NotValidForUsage across platforms.
src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs	New tests covering certificate policy constraints/mappings plus Application Policies vs EKU interactions.
src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj	Adds the new test files to the test project.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs	Splits policy evaluation into “merge errors” and “process policy”; adds encoding-only validation when no policy filters are requested.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Apple.cs	Switches to shared policy-chain logic and per-element error attribution.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs	Switches to shared policy-chain logic and per-element error attribution.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs	Implements CertificatePolicyChain.Build/CheckEncodingOnly, per-element error vectors, and revised EKU/app-policy semantics.
src/libraries/Common/src/System/Security/Cryptography/Oids.cs	Adds AnyEnhancedKeyUsage constant used in policy evaluation.

[Copilot]

Pull request overview

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

Suppressed comments (2)

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:661

• ErrorVector's scalar bit test uses (1 << index) (an int shift). For indices >= 31 this overflows/sign-extends and will report the wrong bit, so long chains can misattribute policy/encoding errors to the wrong element.

 internal bool this[int index]
{
get
{
if (_vector is null)
{
return (_scalar & (1 << index)) != 0;
}

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:456

• The comment about only checking EKU “for structural validity” when ApplicationCertPolicies is present but corrupt is misleading: this block currently skips EKU entirely whenever ApplicationCertPolicies is present (even if it failed to decode). Either update the comment to match the behavior, or add the intended validation call.

 if (policyData.EnhancedKeyUsage != null)
{
// 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);
}

[Copilot]

Pull request overview

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

Suppressed comments (3)

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:456

• ReadExtendedKeyUsageExtension can throw CryptographicException on malformed EKU. In this code path (policy filtering active and no ApplicationCertPolicies extension), the exception is not caught, which would cause chain building to throw instead of reporting InvalidExtension/InvalidPolicyConstraints via encodingErrors.

 // should stay null.
if (policyData.ApplicationCertPolicies is null)
{
applicationCertPolicies = ReadExtendedKeyUsageExtension(policyData.EnhancedKeyUsage);
}

src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs:18

• The RSATestData using-alias appears to be unused, which can trigger CS8019/IDE0005 in builds that enforce unused usings.

using RSATestData = System.Security.Cryptography.Rsa.Tests.TestData;

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs:424

• InvalidPolicyConstraints is being reported with SR.Chain_NoPolicyMatch, which describes a policy mismatch rather than a malformed/invalid extension. This can make X509ChainStatus.StatusInformation misleading for encoding failures.

 Status = X509ChainStatusFlags.InvalidPolicyConstraints,
// "NoPolicyMatch" says that the policy is "invalid", which works for this one, too.
StatusInformation = SR.Chain_NoPolicyMatch,

[Copilot]

Pull request overview

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

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

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

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs:1147

• Use UtcNow instead of Now for certificate validity timestamps to avoid time zone/DST sensitivity in CI and local runs.

 DateTimeOffset notBefore = DateTimeOffset.Now.AddMinutes(-5);

src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs:235

• Remove trailing whitespace in this initializer (it can cause noisy diffs and violates common formatting expectations).

 X509BasicConstraintsExtension.CreateForCertificateAuthority(), 

[bartonjs]

I've trawled the logs, and I attest, to the best of my ability, that none of the extra-platforms test failures are caused by this change.

(And I'm astounded at how many of them report failure when all tests passed and the runner script reports that it's returning success)

[bartonjs]

/ba-g Many DeadLetters, other failures were investigated.

[bartonjs]

/backport-to release/11.0

[bartonjs]

/backport to release/11.0

[github-actions]

Started backporting to release/11.0 (link to workflow run)

[bartonjs]

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.

Fixes#31246 (and maybe others)

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[bartonjs]

/azp run runtime-libraries-coreclr outerloop

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

This PR updates X509Chain policy processing so ApplicationPolicy/CertificatePolicy handling (and invalid/undecodable policy-related extensions) is evaluated and reported more consistently across Windows/OpenSSL/Apple/Android, and adjusts/expands tests to match the unified behavior.

Changes:

• Refactors CertificatePolicyChain to compute per-chain-element “encoding” vs “usage” errors and exposes helpers to reuse the same logic across platform chain processors.
• Updates OpenSSL, Apple, and Android chain processors to merge policy/encoding errors into chain + element status consistently (and to detect encoding issues even when no explicit policy filtering is requested).
• Normalizes existing tests’ OS-conditional expectations and adds focused test suites for app-policy vs EKU behavior and corrupt policy-related extensions.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/libraries/System.Security.Cryptography/tests/X509Certificates/RevocationTests/DynamicRevocationTests.cs	Removes OS-conditional expectations for NotValidForUsage at non-leaf levels.
src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs	Refactors/makes helpers reusable by new tests; factors out shared DER encoding for policy extensions.
src/libraries/System.Security.Cryptography/tests/X509Certificates/CorruptPoliciesChainTests.cs	New coverage for corrupt/undecodable policy/EKU-related extensions and expected chain-element status behavior.
src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs	Normalizes expectations around NotValidForUsage across platforms.
src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs	New tests covering certificate policy constraints/mappings plus Application Policies vs EKU interactions.
src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj	Adds the new test files to the test project.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs	Splits policy evaluation into “merge errors” and “process policy”; adds encoding-only validation when no policy filters are requested.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Apple.cs	Switches to shared policy-chain logic and per-element error attribution.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs	Switches to shared policy-chain logic and per-element error attribution.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs	Implements CertificatePolicyChain.Build/CheckEncodingOnly, per-element error vectors, and revised EKU/app-policy semantics.
src/libraries/Common/src/System/Security/Cryptography/Oids.cs	Adds AnyEnhancedKeyUsage constant used in policy evaluation.

[Copilot]

Pull request overview

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

Suppressed comments (2)

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:661

• ErrorVector's scalar bit test uses (1 << index) (an int shift). For indices >= 31 this overflows/sign-extends and will report the wrong bit, so long chains can misattribute policy/encoding errors to the wrong element.

 internal bool this[int index]
{
get
{
if (_vector is null)
{
return (_scalar & (1 << index)) != 0;
}

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:456

• The comment about only checking EKU “for structural validity” when ApplicationCertPolicies is present but corrupt is misleading: this block currently skips EKU entirely whenever ApplicationCertPolicies is present (even if it failed to decode). Either update the comment to match the behavior, or add the intended validation call.

 if (policyData.EnhancedKeyUsage != null)
{
// 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);
}

[Copilot]

Pull request overview

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

Suppressed comments (3)

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:456

• ReadExtendedKeyUsageExtension can throw CryptographicException on malformed EKU. In this code path (policy filtering active and no ApplicationCertPolicies extension), the exception is not caught, which would cause chain building to throw instead of reporting InvalidExtension/InvalidPolicyConstraints via encodingErrors.

 // should stay null.
if (policyData.ApplicationCertPolicies is null)
{
applicationCertPolicies = ReadExtendedKeyUsageExtension(policyData.EnhancedKeyUsage);
}

src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs:18

• The RSATestData using-alias appears to be unused, which can trigger CS8019/IDE0005 in builds that enforce unused usings.

using RSATestData = System.Security.Cryptography.Rsa.Tests.TestData;

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs:424

• InvalidPolicyConstraints is being reported with SR.Chain_NoPolicyMatch, which describes a policy mismatch rather than a malformed/invalid extension. This can make X509ChainStatus.StatusInformation misleading for encoding failures.

 Status = X509ChainStatusFlags.InvalidPolicyConstraints,
// "NoPolicyMatch" says that the policy is "invalid", which works for this one, too.
StatusInformation = SR.Chain_NoPolicyMatch,

[Copilot]

Pull request overview

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

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

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

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs:1147

• Use UtcNow instead of Now for certificate validity timestamps to avoid time zone/DST sensitivity in CI and local runs.

 DateTimeOffset notBefore = DateTimeOffset.Now.AddMinutes(-5);

src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs:235

• Remove trailing whitespace in this initializer (it can cause noisy diffs and violates common formatting expectations).

 X509BasicConstraintsExtension.CreateForCertificateAuthority(), 

[bartonjs]

I've trawled the logs, and I attest, to the best of my ability, that none of the extra-platforms test failures are caused by this change.

(And I'm astounded at how many of them report failure when all tests passed and the runner script reports that it's returning success)

[bartonjs]

/ba-g Many DeadLetters, other failures were investigated.

[bartonjs]

/backport-to release/11.0

[bartonjs]

/backport to release/11.0

[github-actions]

Started backporting to release/11.0 (link to workflow run)

[bartonjs]

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.

Fixes#31246 (and maybe others)

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[bartonjs]

/azp run runtime-libraries-coreclr outerloop

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

This PR updates X509Chain policy processing so ApplicationPolicy/CertificatePolicy handling (and invalid/undecodable policy-related extensions) is evaluated and reported more consistently across Windows/OpenSSL/Apple/Android, and adjusts/expands tests to match the unified behavior.

Changes:

• Refactors CertificatePolicyChain to compute per-chain-element “encoding” vs “usage” errors and exposes helpers to reuse the same logic across platform chain processors.
• Updates OpenSSL, Apple, and Android chain processors to merge policy/encoding errors into chain + element status consistently (and to detect encoding issues even when no explicit policy filtering is requested).
• Normalizes existing tests’ OS-conditional expectations and adds focused test suites for app-policy vs EKU behavior and corrupt policy-related extensions.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/libraries/System.Security.Cryptography/tests/X509Certificates/RevocationTests/DynamicRevocationTests.cs	Removes OS-conditional expectations for NotValidForUsage at non-leaf levels.
src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs	Refactors/makes helpers reusable by new tests; factors out shared DER encoding for policy extensions.
src/libraries/System.Security.Cryptography/tests/X509Certificates/CorruptPoliciesChainTests.cs	New coverage for corrupt/undecodable policy/EKU-related extensions and expected chain-element status behavior.
src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs	Normalizes expectations around NotValidForUsage across platforms.
src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs	New tests covering certificate policy constraints/mappings plus Application Policies vs EKU interactions.
src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj	Adds the new test files to the test project.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs	Splits policy evaluation into “merge errors” and “process policy”; adds encoding-only validation when no policy filters are requested.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Apple.cs	Switches to shared policy-chain logic and per-element error attribution.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs	Switches to shared policy-chain logic and per-element error attribution.
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs	Implements CertificatePolicyChain.Build/CheckEncodingOnly, per-element error vectors, and revised EKU/app-policy semantics.
src/libraries/Common/src/System/Security/Cryptography/Oids.cs	Adds AnyEnhancedKeyUsage constant used in policy evaluation.

[Copilot]

Pull request overview

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

Suppressed comments (2)

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:661

• ErrorVector's scalar bit test uses (1 << index) (an int shift). For indices >= 31 this overflows/sign-extends and will report the wrong bit, so long chains can misattribute policy/encoding errors to the wrong element.

 internal bool this[int index]
{
get
{
if (_vector is null)
{
return (_scalar & (1 << index)) != 0;
}

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:456

• The comment about only checking EKU “for structural validity” when ApplicationCertPolicies is present but corrupt is misleading: this block currently skips EKU entirely whenever ApplicationCertPolicies is present (even if it failed to decode). Either update the comment to match the behavior, or add the intended validation call.

 if (policyData.EnhancedKeyUsage != null)
{
// 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);
}

[Copilot]

Pull request overview

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

Suppressed comments (3)

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePolicy.cs:456

• ReadExtendedKeyUsageExtension can throw CryptographicException on malformed EKU. In this code path (policy filtering active and no ApplicationCertPolicies extension), the exception is not caught, which would cause chain building to throw instead of reporting InvalidExtension/InvalidPolicyConstraints via encodingErrors.

 // should stay null.
if (policyData.ApplicationCertPolicies is null)
{
applicationCertPolicies = ReadExtendedKeyUsageExtension(policyData.EnhancedKeyUsage);
}

src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs:18

• The RSATestData using-alias appears to be unused, which can trigger CS8019/IDE0005 in builds that enforce unused usings.

using RSATestData = System.Security.Cryptography.Rsa.Tests.TestData;

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Android.cs:424

• InvalidPolicyConstraints is being reported with SR.Chain_NoPolicyMatch, which describes a policy mismatch rather than a malformed/invalid extension. This can make X509ChainStatus.StatusInformation misleading for encoding failures.

 Status = X509ChainStatusFlags.InvalidPolicyConstraints,
// "NoPolicyMatch" says that the policy is "invalid", which works for this one, too.
StatusInformation = SR.Chain_NoPolicyMatch,

[Copilot]

Pull request overview

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

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

[bartonjs]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

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

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

src/libraries/System.Security.Cryptography/tests/X509Certificates/DynamicChainTests.cs:1147

• Use UtcNow instead of Now for certificate validity timestamps to avoid time zone/DST sensitivity in CI and local runs.

 DateTimeOffset notBefore = DateTimeOffset.Now.AddMinutes(-5);

src/libraries/System.Security.Cryptography/tests/X509Certificates/AppAndCertPoliciesChainTests.cs:235

• Remove trailing whitespace in this initializer (it can cause noisy diffs and violates common formatting expectations).

 X509BasicConstraintsExtension.CreateForCertificateAuthority(), 

[bartonjs]

I've trawled the logs, and I attest, to the best of my ability, that none of the extra-platforms test failures are caused by this change.

(And I'm astounded at how many of them report failure when all tests passed and the runner script reports that it's returning success)

[bartonjs]

/ba-g Many DeadLetters, other failures were investigated.

[bartonjs]

/backport-to release/11.0

[bartonjs]

/backport to release/11.0

[github-actions]

Started backporting to release/11.0 (link to workflow run)