Add SslStream.ServerName and other properties from SslClientAuthenticationOptions

[ChristofferGersen]

I would like to use SocketsHttpHandler instead of HttpClientHandler, because of the additional timeout options. In my current implementation I use HttpClientHandler.ServerCertificateCustomValidationCallback to implement custom SSL certificate validation. Inside the callback I use HttpRequestMessage.RequestUri.Host to see if the user has configured a trusted certificate for the specified host. If the certificate is not trusted, then I add the certificate to HttpRequestMessage.Properties, so that the requester/caller can show the certificate to the user. When I try to re-implement this using SocketsHttpHandler.SslOptions.RemoteCertificateValidationCallback, then I get an SslStream instead of a HttpResponseMessage. As a result I do not know for which host I am verifying the certificate and I cannot add the certificate to the request properties.

As a workaround I use reflection to get HttpClientHandler._socketsHttpHandler, so that I can configure the timeout options of SocketsHttpHandler, while still using HttpClientHandler. Of course I would prefer not having to resort to using reflection and exploiting implementation details.

Another way to allow this is to expose System.Net.Http.ConnectHelper.CertificateCallbackMapper, which is what HttpClientHandler.ServerCertificateCustomValidationCallback uses internally.

[stephentoub]

Thanks, @ChristofferGersen.

There are two pieces here:

1. Getting the host name in the callback.
2. Storing something into the HttpRequestMessage

For (2), this would only impact the HttpRequestMessage that actually caused the connection to be established; other requests that reused the same connection would not cause the callback to be invoked and thus would not have their Properties configured. How does that fit into your scenario?

For (1), I think a cleaner approach would be to ensure that SslStream exposes the information needed for such situations. @davidsh, is there any way to get that info from the SslStream instance? If not, have we considered adding it?

I'd like us to try hard to avoid adding a duplicated callback as you suggest. I understand why you want it, and it is a bit unfortunate that the request message isn't made available in the callback. It was done this way so that the same SslOptions bag could be used that's generally used with SslStream, so that all of those options can be used to configure it, now and in the future with any other properties/configuration added there. We could of course add the other variety directly to SocketsHttpHandler rather than having you set the callback on SocketsHttpHandler.SslOptions, but then that ends up being a bit confusing, that some SSL configuration is in the SslOptions and some not, and that there are two different variants of the same callback that do almost exactly the same thing.

[ChristofferGersen]

For (1) I agree that having the host name available on the SslStream would be nicer. Having duplicate callbacks is indeed confusing. I also understand that the API was made to be more in line with SSL connections in general for code re-use. I was focusing a bit too much on the way it used to work with HttpClientHandler.

For (2) the idea was that the callback was called again for a second request, because the connection from the first request is not maintained, because of the certificate validation failure. Although, now I think more about it, that probably would not have worked if two requests would have been made at roughly the same time, and both would immediately try to share a single connection. As such it is probably a good idea for me to leave HttpRequestMessage alone. I could just as well store the last certificate that failed validation for each host elsewhere, and lookup the invalid certificate when I get the exception. Thanks for noticing a concurrency issue in my code, which I didn't even realize existed.

[davidsh]

For (1), I think a cleaner approach would be to ensure that SslStream exposes the information needed for such situations. @davidsh, is there any way to get that info from the SslStream instance? If not, have we considered adding it?

What additional information are you asking to be "exposed"? Is it the "host name"? If so, wouldn't that already be exposed in the SNI information of SslStream.

I would like to use SocketsHttpHandler instead of HttpClientHandler, because of the additional timeout options.

HttpClientHandler is really the class that we encourage all developers to use. It is the only class in .NET Standard. Perhaps the additional properties such as "timeouts" should be added to that. Then you wouldn't have to directly create/use SocketsHttpHandler.

[stephentoub]

Is it the "host name"?

Yes

If so, wouldn't that already be exposed in the SNI information of SslStream.

Where? I must be missing it. Given an SslStream, from where do I get the host name?

[davidsh]

Where? I must be missing it. Given an SslStream, from where do I get the host name?

I might have been thinking of the server-side SslStream API where we have a callback for received SNI (hostname) information. So, yes, it's possible we don't have a way to determine what host (SNI) was sent during TLS protocol exchange.

[stephentoub]

So, yes, it's possible we don't have a way to determine what host (SNI) was sent during TLS protocol exchange.

Thanks. Any reason not to consider adding that?

[davidsh]

Thanks. Any reason not to consider adding that?

I see can the value of having the host (SNI) name being sent by client-side APIs as useful information. I just don't see exactly where we would provide that without creating another callback.

[stephentoub]

I just don't see exactly where we would provide that without creating another callback.

Could it / should it not just be stored onto the SslStream instance as field, exposed via a property, that's set and available for read if/once the information is available? Similar to what we do with information like what hash algorithm, cipher algorithm, key exchange algorithm, etc. was established?

[davidsh]

Could it / should it not just be stored onto the SslStream instance as field, exposed via a property, that's set and available for read if/once the information is available? Similar to what we do with information like what hash algorithm, cipher algorithm, key exchange algorithm, etc. was established?

Yes, that is a possibility.

I look forward to seeing an API proposal for this.

[karelz]

Triage: We agree to expose RequestUri.Host (SNI) on SslStream (assuming it is not there).
We need API proposal. The name should be likely something like ServerName.

This is similar / overlapping with dotnet/corefx#41622 - we should keep that in mind (it asks to allow changes to SslStream options in the callback).

[wfurt]

We store SslClientAuthenticationOptions.TargetHost internally but it is not exposed publicly.
(all the other overloaded are converted to SslClientAuthenticationOptions under the cover)
On server, that property is used to hold name requested by client (if any)
That may possibly be used past the callback for diagnostic or in cases when certificate is specified without delegate (like wildcard cert matching subdomains and all sites).

How about adding this @stephentoub @davidsh

public string TargetName { get; };

[stephentoub]

That's effectively what I had in mind. Could it be HostName?