-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SslStream delayed client certificate negotiation #49346
Comments
Tagging subscribers to this area: @dotnet/ncl, @vcsjones Issue DetailsBackground and MotivationRelated: Http servers like IIS, Http.Sys, HttpListener, etc. can conditionally request a client certificate after receiving the http request. Kestrel does not due to a lack of support in SslStream. Customers ask for this because they only want to require a client certificate for some pages/paths in their application. Alternative strategies have proven too breaking for applications to adopt when moving from IIS to Kestrel. Proposed APInamespace System.Net.Security
{
public class SslStream {
+ public Task<X509Certificate2?> GetClientCertificateAsync(CancellationToken cancellationToken) No reads or writes would be allowed in parallel with this operation in order to avoid corrupting internal state. Should successes and failures be cached? E.g. what happens if you call this API more than once per connection? What if a client cert was provided during the initial handshake? What validation should happen on this certificate? Risks
|
This seems like a 99th percentile perf issue waiting to be discovered -- can you go into more detail on why this behavior is needed? |
CC @nibanks any plans for MsQuic delayed client certs? |
Can you clarify what the ask is here? |
A) It's unclear if the TLS state would allow data to be encrypted or decrypted during a renegotiation since the ciphers could change, I'll defer to Tomas on that. |
Ok. Read a bit more. No, we don't support that TLS extension, nor do we plan to expose that type of functionality in our API. |
@nibanks Are there known blockers or is it a matter of priorities? |
TLS has to support it first. Then expose an API for it. Then we need to understand how it would would with QUIC (if at all; I don't know anything about the mentioned extension) and then we need to figure out how/if we can expose this in our API. And then there is the question of prioritization; which I'd expect this to fall fairly low on the list. |
@nibanks nevermind, Quic (and HTTP/3) also prohibits the use of this extension.
That limits the usefulness of this feature to HTTP/1.x (and non-HTTP protocols). |
It is part of base TLS 1.3 RFC https://tools.ietf.org/html/rfc8446#section-4.6 @nibanks. But as @Tratcher mentioned, it seems to be forbidden by QUICK. This works with with TLS 1.3 - tested with openssl s_client and Windows Insider build. As long as client indicates willingneess via post_handshake_auth. Interestingly, neither Chrome, Safari nor HttpClient sets that. (by default?) Setting RemoteCertificate impacts IsMutuallyAuthenticated. If we re-use (but we don't have to) handshake logic, standard verification logic would apply e.g. we would build a chain and if RemoteCertificateValidationCallback is set, it would be called as well. That may seems redundant but mimics current flow. This generally seems like server feature. So on client, we would either throw or simply return RemoteCertificate. If we return RemoteCertificate, I would do the same on server e.g. if the certificate exist (was provided during handshake) then return it. Otherwise ask for it. This would effectively block option to ask multiple times, perhaps for different certificates. But I don't know if that is need or if IIS supports that. So I would probably go with one shot for simplicity unless there is indication that something else is needed. cc: @blowdart @bartonjs @GrabYourPitchforks for any security concerns. |
Triage: 6.0 until proven otherwise. |
I think this should be feasible with Windows and Linux. I have PoC for Windows with TLS 1.2 & 1.3 and Linux with TLS 1.2. I think this should return existing remote certificate if that exist. e.g. it will not ask for yet another one during existing session and that will also cover cases when called on client. If no remote certificate exist, SslStream will try to use means of given protocol version to obtain one. Since this does not serve in handshake, the certificate will be returned without any validation. It would be up to the caller to do what ever needs to be done with it. That also prevents cases when SslStream tries to do too much and can for example block on resolving certificate chain. |
At most one client certificate negotiation should happen for the lifetime of an SslStream instance, even if a prior call returned null. |
namespace System.Net.Security
{
public partial class SslStream
{
public virtual Task NegotiateClientCertificateAsync(CancellationToken cancellationToken = default) => throw null;
}
} |
Also, just to be clear... if the negotiation fails (i.e. no client cert) then does this throw? Or simply return and RemoteCertificate is still null? |
I don't think it needs to throw if everything end cleanly e.g. negotiation succeeds without certificate. We would throw if negotiation fails for whet ever reason. |
At most one negotiation should happen. Subsequent calls to the API should return the original result (success or failure). |
Should we do validation and call RemoteCertificateValidationCallback on the certificate? Current code does it somewhat automatically on any renegotiation but we can prevent it if we want to. That would give caller more control as the validation can happen if needed outside of the SslStream. |
What would happen if the cert failed validation?
|
If validation fails, the |
But would the stream still be usable or would it be aborted? If NegotiateClientCertificateAsync is canceled then the stream is aborted, correct? Being inconsistent seems concerning. What if validation failures were only reported to RemoteCertificateValidationCallback? Or made available through a parallel property? |
We realized there's a problem with this approach. Kestrel always has a loop running that reads from the SslStream and copies into a Pipe. While we wouldn't want the client to send any data during NegotiateClientCertificateAsync, there's no way to interrupt a pending read in order to call NegotiateClientCertificateAsync. Would it be possible to allow the pending read to sit paused so long as no data was received during the NegotiateClientCertificateAsync call? Or would we need some way to cancel the read without aborting the SslStream? |
The underlying SslStream does not need to be aborted if renegotiation finished (or at least did not fail) even if the And yes, I think we can accommodate the pending read. On Windows the renegotiate code call I have Windows implementation now and I'm working on tests to cover some of the scenarios we discussed. (hoping to get PR up today or early next week) |
We only do this with HTTP/2 which this feature is blocked for anyways right? |
You're right @davidfowl . This would have been a problem back when we had a read loop in Kestrel's HttpsConnectionMiddleware, but I forgot that you changed it to use stream-to-pipe adapter a long time ago when @Tratcher and I were discussing it earlier. |
I'm wondering if there is harm in letting the read working. Seems finicky to build feature on latest Kestrel behavior. |
Nah, just make it fail. |
This is reminder of few things missed in initial #51905
missing tests for 2 & 3. This will be impacted by refactoring unrelated to delayed certificate We should add tests for early certificate with TLS 1.3 - at least on windows. There seems to be strange interaction as TLS 1.3 handshake completion use renegotiation path so as delayed certificate. |
What happens if AllowRenegotiation is set to false and NegotiateClientCertificateAsync is called? Does it no-op, throw, or renegotiate? AllowRenegotiation was added for HTTP/2 to prevent the remote party from initiating a renegotiation. Since APLN could result in the connection using HTTP/1.1 it might be OK to allow the server to initiate a renegotiation even if AllowRenegotiation is disabled. It would be the caller's responsibility to prevent that for HTTP/2. |
I'll need to test it but I think it will work on Linux and fail and throw on Windows. We do copy the provided options so setting them after the stream is negotiated does nothing. There is currently no good way how to enforce the HTTTP/2 requirement AFAIK. |
Should be functional on Windows and Linux with current OpenSSL. 2 remaining issues are tracked separately:
|
Background and Motivation
Related:
dotnet/aspnetcore#23948
#26210
Http servers like IIS, Http.Sys, HttpListener, etc. can conditionally request a client certificate after receiving the http request. Kestrel does not due to a lack of support in SslStream. Customers ask for this because they only want to require a client certificate for some pages/paths in their application.
Alternative strategies have proven too breaking for applications to adopt when moving from IIS to Kestrel.
Proposed API
namespace System.Net.Security { public class SslStream { + public Task<X509Certificate2?> GetClientCertificateAsync(CancellationToken cancellationToken)
No reads or writes would be allowed in parallel with this operation in order to avoid corrupting internal state.
Should successes and failures be cached? E.g. what happens if you call this API more than once per connection? What if a client cert was provided during the initial handshake?
What validation should happen on this certificate?
Risks
The text was updated successfully, but these errors were encountered: