Skip to content

Conversation

@vcsjones
Copy link
Member

@vcsjones vcsjones commented Nov 7, 2025

The certificate chain we were using in unit tests has a root certificate that is signed with RSA SHA-1. This signature algorithm is disabled on some linux distributions, and it is starting to get picked up in our CI.

Let's replace the chain with one that does not use RSA+SHA-1 signatures.

On CentOS Stream 10:
CleanShot 2025-11-07 at 12 13 27@2x

Contributes to #120527

Copilot AI review requested due to automatic review settings November 7, 2025 17:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request updates the Microsoft.com SSL certificate test data and associated test cases to use a newer certificate version. The changes reflect an update from the Baltimore CyberTrust Root certificate chain to the DigiCert Global Root G2 certificate chain, with an updated certificate valid from October 2025 to March 2026.

  • Updates the Microsoft.com SSL certificate and its issuer/root certificate chain data
  • Adjusts verification times in tests to fall within the new certificate's validity period
  • Updates hostname test cases to reflect the Subject Alternative Names (SANs) in the new certificate
  • Simplifies platform-specific conditional logic for chain building tests

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
MatchesHostnameTests.cs Updates hostname test cases to match SANs in the new certificate, including new domains like copilot.ai and yarp.dot.net
AuthorityKeyIdentifierTests.cs Updates expected hex values for authority key identifiers to match the new DigiCert-based certificate chain
CollectionTests.cs Updates verification time to December 2025 to fall within new certificate validity period
ChainTests.cs Updates verification times, simplifies platform conditionals, improves expiration test structure with collection expressions, and adds better failure diagnostics
TestData.cs Replaces entire certificate chain data (leaf, issuer, and root certificates) from Baltimore CyberTrust to DigiCert Global Root G2

bartonjs
bartonjs previously approved these changes Nov 7, 2025
@vcsjones vcsjones requested a review from bartonjs November 8, 2025 02:31
@vcsjones vcsjones dismissed bartonjs’s stale review November 8, 2025 02:31

Dismissing because it was approved before some test failures were known and need to re-evaluate the solution.

@vcsjones vcsjones added this to the 11.0.0 milestone Nov 8, 2025
@bartonjs
Copy link
Member

I'm guessing that what's really going on here is that I made a synecdoche mistake about how Windows handles NotValidForUsage. Looking up the root in question, I see that it's actually in my root trust store twice:

image

One of which is valid for all purposes, the other is only valid for certain. The previous test probably only involved a certificate that had no EKU, but did have Windows override limited purposes... so we saw "no" look like it propagated up. The new one is hitting ambiguity, which is either working because it always tries to succeed when it can, or it's random depending on something I don't understand.

@vcsjones
Copy link
Member Author

I see that it's actually in my root trust store twice

Ah good point on the multiple roots.

Some options on how to proceed.

  1. Leave as-is and create an issue for it to look in to.
  2. Find a different chain that is more predictable and change to that. And create an issue.
  3. Something more clever.

@bartonjs
Copy link
Member

I'm good with (1). This is good enough to backport, and doesn't require renaming things 😄

@bartonjs
Copy link
Member

If we want any change here, it'd be to say "fine, on Windows, it can either be NoError on top, or NotValidForUsage", just to show a) we know there's currently variance, but b) it's not across a huge spectrum.

@vcsjones
Copy link
Member Author

/ba-g android timeouts are known.

@vcsjones vcsjones merged commit d594a04 into dotnet:main Nov 13, 2025
78 of 86 checks passed
@vcsjones vcsjones deleted the fix-failing-chain-tests branch November 13, 2025 18:09
@vcsjones
Copy link
Member Author

/backport to release/10.0

@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

@vcsjones backporting to release/10.0 failed, the patch most likely resulted in conflicts. Please backport manually!

git am output
$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Applying: Replace test certificate chain with one that does not have an RSA+SHA-1 root
Applying: Fix tests for Linux and make failures more clear
Applying: Fix the one second offset thing
Applying: Remove allowance for SHA-1 in test since it does not use SHA-1 anymore
Applying: Fix more verification times
error: sha1 information is lacking or useless (src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.cs).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0005 Fix more verification times
Error: The process '/usr/bin/git' failed with exit code 128

Link to workflow output

ViktorHofer pushed a commit that referenced this pull request Nov 19, 2025
…SHA-1 (#121665)

Backport of #121450 to
release/10.0

cc @bartonjs 

## Customer Impact

These are test only changes. These changes react to more environments in
CI rejecting certificate chains that use an RSA+SHA-1 root certificate.

- [ ] Customer reported
- [x] Found internally

## Regression

- [ ] Yes
- [x] No

## Testing

Tests that were failing are now passing in CI.

## Risk

None, test only changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants