fix handling of client cert on macOS #73574
Conversation
|
Tagging subscribers to this area: @dotnet/ncl, @vcsjones Issue DetailsWe did not build X509Chain when using specific client or server certificate (e.g. Without context). This got missed by the existing test. Because of #48207 the prerequisites would never be satisfied and the test would always skip on macOS. To fix both the test is no longer conditional. Hopefully it will be stable enough in CI. fixes #66537
|
src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.OSX.cs
Show resolved
Hide resolved
| { | ||
| _output.WriteLine($" Status: {status.Status}: {status.StatusInformation}"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Logging items from chain.ChainStatus may also be useful, in case there's an overall code it presented that didn't map to any individual element
There was a problem hiding this comment.
CN=SslStream_ClientCertificate_SendsChain, O=SslStream_ClientCertificate_SendsChain length 1
Status: PartialChain: One or more certificates required to validate this certificate cannot be found.
ChainStatus: PartialChain One or more certificates required to validate this certificate cannot be found..
it seems like it cannot find the part certs.
That are in store dump AFAIK
CN=A Revocation Test CA 0, O=SslStream_ClientCertificate_SendsChain
CN=A Revocation Test Root, O=SslStream_ClientCertificate_SendsChain
There was a problem hiding this comment.
Huh. I certainly would have expected the default keychain to be involved in a chain build; but I guess not. We probably don't have tests like this in the X509 suite because we don't like messing with system state.
Since there's not a flow where the client can provide their own chain context, you might just need to have this test leave the responder running, so the intermediate can be fetched from AIA. (Overload GenerateCertificates to out the responder instead of disposing it)
There was a problem hiding this comment.
I did many runs on my local machines and it works fine. But there is something unusual about the CI machine ;(
I was thinking about making the test optional again but we lost coverage before and it hid valid issue.
And I can try the AIA. I did not like it for the servers as it make some of the test less predicable but I can try it for client.
There was a problem hiding this comment.
I did many runs on my local machines and it works fine. But there is something unusual about the CI machine ;(
Now that sounds familiar :). IIRC it doesn't have a UI session, just an SSH session, and that makes some of the keychain systems behave oddly.
|
I did not figure out how to pass the test in CI. It runs locally and I have reasonable confidence the product change is right. |
ghost
locked as resolved and limited conversation to collaborators
We did not build X509Chain when using specific client or server certificate (e.g. Without context).
We used to call X509Chain.Build inside of PAL but we don't any more. So the intermediate CA certificates would not be sent even if available in certificate stores.
This got missed by the existing test. Because of #48207 the prerequisites would never be satisfied and the test would always skip on macOS.
Also macOS can provide full chain (when certs are stored in Key Chain) even if the client did not sent everything.
To fix both the test is no longer conditional. Hopefully it will be stable enough in CI.
fixes #66537
contributes to #73295