[release/3.1] Handle additional chain statuses for X509Chain on macOS #42914
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.
|
@bartonjs I took a stab at writing a summary from the template I've seen used elsewhere (intentionally omitting risk). Feel free to junk it if isn't helpful / correct. |
|
Nah, looked good to me; but since Risk is a required section I went ahead and added it :) |
|
@bartonjs yeah I left risk off not because I didn't think there was any, just that I didn't know what criteria you use for determining it. |
|
You might add to customer impact a note about whether it's customer reported - ie how commonly is this encountered. Presumably it's most relevant to someone porting code from Windows/Linux? |
|
@bartonjs what were your expectations around porting to 2.1? Seems doable, but I did notice we also didn't port this change in to 2.1, either: dotnet/runtime@7073999#diff-7455c509ff333133d8128fece0747213 More specifically, 2.1 has no handling for "UnparseableExtension" at all, not just the link above for the behavior change. |
|
@danmosemsft thanks for the suggestion, I updated it to make a note of that. |
|
Tactics asked- under what principles are we bringing this for servicing?
Is it all 1 or also 2? Also, any data on why this has not come up before - this code is not that new - is it a macOS update that added these codes? Or just someone finally ran into it? In the case of #42875 (which we did take) it was an macOS update -- which is a much clearer case for servicing because we do not want to deter anyone from a supported OS update. Maybe #35590 also. Would it be lower risk for the servicing release to handle only the original customer case? |
|
@danmosemsft @bartonjs Apologies in advance if this is too long winded. tl;dr:
It's both. But,, this exception was in code further up in the .NET stack that was using this functionality - the customer was not using
These statuses fall in to three buckets:
1 and 2 are where we are throwing an exception when the chain is invalid. Most customers are likely using certificate chains that don't exhibit problems. 3 is a case where we are blocking a "good" path, but one that shouldn't be hit often, for now.
I don't have a good feel for how y'all evaluate "risk", but I generally think the risk is very low here. There is arguably non-zero risk by not accepting some of these since there is potential for some event to cause these statuses to occur more frequently (see below). If something causes these statuses to happen frequently (again, hypothetical) then you'd have to react quickly to them. DetailsFor point one above, there are two statuses the customer was hitting "BasicConstraintsCA", and "BasicConstraintsPathLen". They are both new in macOS 10.14. They are new-ish. However as said, they are a case where a certificate is being used in a way they aren't supposed to be used in the first place. "BlackListedLeaf" and "BlackListedKey" have been in macOS for quite a long time. That said, a customer is not very likely to hit them at the time of writing. Apple can via its internal update mechanism put a certificate or a whole intermediate on a disallow list. This doesn't appear to have happened very recently, so most cases of such certificates have probably disappeared from usage of the internet because other software has blocked them. However, if Apple were to suddenly do a mass ban of a Certificate Authority (like DigiNotar back in 2011) and people were connecting to websites or using certificates from this said-banned CA, they would start encountering "BlackListedKey" frequently. "CriticalExtensions" this just seems like one most people won't run in to too much; critical extensions aren't use too often, and where they are, they are used for things that are well understood and have existed in X509 for a long time, like Basic Constraints or Key Usage. "WeakSignature" is where a certificate is signed with MD5 (or weaker). It has been known that MD5 shouldn't be used for a long time, so I suspect certificates are unlikely to use it. The use of MD5 is banned by public CAs, and even most Enterprise CAs (internally trusted) have moved away from it. However, in theory, Apple could move SHA1 on to this list of weak algorithms, which I think is justifiable in the future. But unlike MD5, SHA1 is still used in some scenarios, in my experience. |
Functionality. X509Chain isn't supposed to throw exceptions, it's supposed to return status codes saying what went wrong. But we have a loose interpretation of Apple's chain validator on macOS, so any time we see something we don't know how to handle we throw an exception (on the basis that it could be that a code we already handled got renamed, and now we're lying about a status you actually care about). The original support was based on the strings used in 10.13, with whatever things I could come up with tests for. Since then we've just been reactive to new codes (largely ignored); but the most recent customer report was for the very reason we throw: An existing code got renamed (split in two... which we re-unify back to one, because that's what our API can express it as). @vcsjones then went the extra mile to see what other codes are in the list that we can coax out of the chain engine so we can see how Windows handles the same thing. I'm definitely supportive of servicing it, and the intent here is to get "caught up" to where the OS is now, so we don't have to do as much reactive servicing. I also think we should take it to 2.1 (as well as any other codes in that waterfall that didn't get ported) just for improved stability for 2.1 on newer macOS versions. But I'll also accept the "don't touch it unless there's an active complaint" from someone on that version stance, of course. (In the case of the Basic Constraints violation here, it came from a user on 3.1; so this /is/ servicing a version with an active customer issue) |
leecow
added
Servicing-approved
|
This was approved after some discussion. We don't generally use servicing to 'continue to improve the product' (too much churn) but in this case this is driven at least in part by external factors (accommodating changes to the platform). We also care about customer reports, and diagnosability. We did not approve 2.1 -- we are holding a higher bar there as although both are LTS, 3.1 is also current. We can close that PR, and reopen should there be an issue raised through official support channels, etc that warrants another look at 2.1. Please let @Anipik merge when the branch is open. |
Anipik
added
the
* NO MERGE *
Anipik
removed
the
* NO MERGE *
6 participants
Ports the following PRs from runtime in to the release/3.1 branch.
This omits the unit tests from 35347 because the tests depend on new functionality not present in the release/3.1 branch.
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 3.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.