This repository was archived by the owner on Jan 23, 2023. It is now read-only.
[release/2.1] Handle additional chain statuses for X509Chain on macOS#42916
Closed
vcsjones wants to merge 4 commits intodotnet:release/2.1from
Closed
[release/2.1] Handle additional chain statuses for X509Chain on macOS#42916vcsjones wants to merge 4 commits intodotnet:release/2.1from
vcsjones wants to merge 4 commits intodotnet:release/2.1from
Conversation
macOS can return additional chain status strings for a certificate that was issued by a certificate that violated its basic constraints. If a leaf certificate is issued from a certificate with CA:FALSE, the strings BasicConstraintsCA and BasicConstraintsPathLen are reported. We map these the same for BasicConstraints.
MacOS returns a different status string for certificates that are in a special database that are explicitly distrusted. Windows has similar behavior, which reports the certificates as PAL_X509ChainExplicitDistrust. This makes macOS do the same instead of throwing an exception. Linux does not appear to have any special distrusting for these certificates.
* Support unknown critical extensions on macOS. If a certificate contains an unprocessable critical extension in a certificate, map the "CriticalExtensions" status to HasNotSupportedCriticalExtension instead of throwing an exception. * Ignore WeakSignature chain status on macOS. X509Chain on Windows will not check for modern signatures, so we will let macOS do the same thing.
Member
Author
Member
|
@bartonjs which do you prefer? I guess I'd assume the fuller one. |
Member
|
@danmosemsft Yeah, I'd advocate just doing the full catch-up and have the same translation table for all three main branches. |
bartonjs
approved these changes
May 2, 2020
Member
|
Ok |
danmoseley
added
Servicing-consider
Member
|
See note in #42914 If new impact emerges, we can take another look. It would likely involve official support tickets. |
Member
|
Thanks @vcsjones for working on these. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports the following PRs in to the release/2.1 branch.
UnparseableExtensionstatus code when building X509Chain on … #40117This omits the unit tests from 35347 because the tests depend on new functionality not present in the release/2.1 branch.
This omits the unit test changes from 40117 because that test does not exist in release/2.1
Description
X509Chain.Buildcan receive unknown status strings from the underlying platform, macOS, when building an X509 chain. An unknown status string causes a CryptographicException to be thrown due to the missing mappings.The fix is to correctly handle these additional statuses, mapping them such that the behavior matches what Windows and Linux do.
Customer Impact
Initially reported by a customer in dotnet/runtime#35238. Customers that attempt to build an
X509Chainon macOS with a certificate that causes one of the unknown statuses to be triggered will receive aCryptographicExceptioninstead of theX509ChainStatusFlagswhich other platforms correctly report. This may cause compatibility issues as developers are porting from other platforms to macOS.Regression
No.
Testing
Contains unit tests for all but the basic constraints scenario due to missing functionality in release 2.1 to aid testing. All are tested in dotnet/runtime.
Risk
Low. All of the new codes have been encountered in testing and measured against Windows for cross-platform consistency. The existing tests ensure that the change isn't accidentally doing subtle remaps of established values.