Initial support for SslStreamCertificateContext #38364
Conversation
|
Tagging subscribers to this area: @dotnet/ncl |
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
Nothing should ever use (e.g. the new multi-pem load methods are only on X509Certificate2Collection) |
src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs
Show resolved
Hide resolved
src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs
Outdated
Show resolved
Hide resolved
| intermediates[i] = chain.ChainElements[i + 1].Certificate; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
| } | |
| // Dispose the copy of the target cert. | |
| chain.ChainElements[0].Certificate.Dispose(); | |
| // Dispose the last cert, if we didn't include it. | |
| for (int i = count + 1; i < chain.ChainElements.Count; i++) | |
| { | |
| chain.ChainElements[i].Certificate.Dispose(); | |
| } | |
| } |
There was a problem hiding this comment.
if the chain is not complete, should se sent out as much as we can? e.g. the leave as well?
There was a problem hiding this comment.
I guess bad terminology. I meant certificate closest to the root if root is missing. e.g. is certificate c is signed by i1 -> i2 -> i3 -> root and root is missing, send cert + i1 + i2 + i3.
There was a problem hiding this comment.
Yeah, if the last cert isn't self-signed, then all the certs after the end-entity should be sent; that's why I checked for PartialChain (it's returned when the last cert that the builder could find wasn't self-issued).
| throw new AuthenticationException(SR.net_ssl_io_no_server_cert); | ||
| } | ||
|
|
||
| CertificateContext = new SslStreamCertificateContext(certificateWithKey); |
There was a problem hiding this comment.
| CertificateContext = new SslStreamCertificateContext(certificateWithKey); | |
| CertificateContext = SslStreamCertificateContext.Create(certificateWithKey, null); |
There was a problem hiding this comment.
I added internal Create() to be more consistent but allowing to have SslStreamCertificateContext without building full chain to support legacy behavior on platform where we do not build chain but leave OS to do it.
ghost
locked as resolved and limited conversation to collaborators
This adds part of the new API surface approved in #37933. Internally, I switched SslStream to use CertificateContext instead of ServerCertificate and I did minimal fixes in OpenSSL pal to get that through. MacOS and Windows will follow.
So as more tests once #38182 is in. I will us that structure to create more test scenarios for partial chains.
contribues to #35844
SslStreamCertificateContext.Create() was approved with X509Certificate2Collection and I'm wondering if X509CertificateCollection would be sufficient @bartonjs since we do not need keys for the intermediate certificates.