Skip to content
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

[Question] Custom the server name of SocketsHttpHandler #45144

Closed
dylech30th opened this issue Nov 24, 2020 · 22 comments
Closed

[Question] Custom the server name of SocketsHttpHandler #45144

dylech30th opened this issue Nov 24, 2020 · 22 comments
Assignees
Labels
area-System.Net.Http question Answer questions and provide assistance, not an issue with source code or documentation.
Milestone

Comments

@dylech30th
Copy link

dylech30th commented Nov 24, 2020

Since the WinHttpHandler has been abandoned in .NET 5(at least I cannot find WinHttpHandler anymore in HttpClientHandler's source code),the SocketsHttpHandler seems becomes the only possible choice, but it has a default behavior that I cannot set the server name directly or disable it, it uses either the host of the request URI, or your specified Host header, my requirement is, I want to keep the URI host as server name (or use custom one) while I can specify the Host header

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Net.Http untriaged New issue has not been triaged by the area owner labels Nov 24, 2020
@ghost
Copy link

ghost commented Nov 24, 2020

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Since the WinHttpHandler has been abandoned in .NET 5(at least I cannot find WinHttpHandler anymore in HttpClientHandler's source code),the SocketsHttpHandler seems becomes the only possible choice, but it has a default behavior that I cannot set the server name directly or disable it, it uses either the host of the request URI, or your specified Host header, but my requirement is, I want to keep the URI host as server name (or use custom one) while I can specify the Host header

Author: dylech30th
Assignees: -
Labels:

area-System.Net.Http, untriaged

Milestone: -

@ManickaP
Copy link
Member

WinHttpHandler hasn't been abandoned completely, it's just been removed from the HttpClient dll, but it's still present in its own NuGet package: https://www.nuget.org/packagesSystem.Net.Http.WinHttpHandler/. You can still use it with .NET 5.

@dylech30th
Copy link
Author

Do we have a way to satisfy what I want without WinHttpHandler? After all, I found that WinHttpHandler's debug message is quite Illegible compare to SocketsHttpHandler

@ManickaP
Copy link
Member

Could you please post repro of what you're trying to achieve? Either your original WinHttpHandler one or what you got so far with SocketsHttpHandler and what you're exactly missing?

@dylech30th
Copy link
Author

public class DelegatedHttpClientHandler : HttpClientHandler
    {
        private readonly bool directConnect;
        private readonly IHttpRequestInterceptor? interceptor;

        public DelegatedHttpClientHandler(IHttpRequestInterceptor? interceptor = null, bool directConnect = true)
        {
            this.directConnect = directConnect;
            this.interceptor = interceptor;
            ServerCertificateCustomValidationCallback = DangerousAcceptAnyServerCertificateValidator;
            SslProtocols = SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12;
        }

        protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
        {
            interceptor?.Intercept(request);

            if (directConnect)
            {
                var host = request.RequestUri?.DnsSafeHost;
                var isSslSession = request.RequestUri?.ToString().StartsWith("https://");

                request.RequestUri = new Uri($"{(isSslSession is true ? "https://" : "http://")}{Dns.GetSourceIpAddresses()[0]}{request.RequestUri?.PathAndQuery}");
                request.Headers.Host = host;
            }

            return await base.SendAsync(request, cancellationToken);
        }
    }

where IHttpRequestInterceptor is a custom interface given by the following definition:

public interface IHttpRequestInterceptor
    {
        void Intercept(HttpRequestMessage message);
    }

and I need to prevent the server name to be the same as the Host header(in other words, the real hostname)

@scalablecory
Copy link
Contributor

scalablecory commented Nov 24, 2020

SocketsHttpHandler has no way to specify SNI separate from the URI and Host header.

Can you give any info on what scenarios you would use this in? This isn't clear to me from your example.

@scalablecory scalablecory added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration question Answer questions and provide assistance, not an issue with source code or documentation. and removed untriaged New issue has not been triaged by the area owner labels Nov 24, 2020
@samsosa
Copy link

samsosa commented Nov 24, 2020

I use this:

string? SocketsHttpHandler.SslOptions.TargetHost

@dylech30th
Copy link
Author

dylech30th commented Nov 24, 2020

SocketsHttpHandler has no way to specify SNI separate from the URI and Host header.

Can you give any info on what scenarios you would use this in? This isn't clear to me from your example.

Well, this scenario is rather special, I need to access IP directly to bypass the firewall (which will sniff the connection through both URI host and server name) but on the other hand, I also need to specify the Host header to tell the server which host I‘m trying to access, and I don't want to let the hostname appears in the server name because its sent by cleartext, which will make it too vulnerable to be sniffed by the firewall, and that's what I'm trying to prevent

@dylech30th
Copy link
Author

I use this:

string? SocketsHttpHandler.SslOptions.TargetHost

seems that it's only used by certificate validation according to the documentation, and my requirement is to find a way to set the server name during client hello

@samsosa
Copy link

samsosa commented Nov 24, 2020

I've never really checked this, but I believe it successfully influences the SNI in my case.

It is also noticed in the source code of the Runtime accordingly:

// Set TargetHost for SNI
sslOptions.TargetHost = sslHostName;

Or

string punyCode = string.IsNullOrEmpty(sslAuthenticationOptions.TargetHost) ? string.Empty : s_idnMapping.GetAscii(sslAuthenticationOptions.TargetHost!);
// Similar to windows behavior, set SNI on openssl by default for client context, ignore errors.
if (!Ssl.SslSetTlsExtHostName(context, punyCode))

which calls OpenSSL's SSL_set_tlsext_host_name() and that is according to documentation:

SSL_set_tlsext_host_name() sets the server name indication ClientHello extension to contain the value name. The type of server name indication extension is set to TLSEXT_NAMETYPE_host_name (defined in RFC3546).

@dylech30th
Copy link
Author

dylech30th commented Nov 24, 2020

Through SocketsHttpHandler::SetupHandlerChain we can found that a request process depends on HttpConnectionPoolManager

private HttpMessageHandlerStage SetupHandlerChain()
{
// Clone the settings to get a relatively consistent view that won't change after this point.
// (This isn't entirely complete, as some of the collections it contains aren't currently deeply cloned.)
HttpConnectionSettings settings = _settings.CloneAndNormalize();
HttpConnectionPoolManager poolManager = new HttpConnectionPoolManager(settings);
HttpMessageHandlerStage handler;
if (settings._credentials == null)
{
handler = new HttpConnectionHandler(poolManager);
}
else
{
handler = new HttpAuthenticatedConnectionHandler(poolManager);
}

and the requests are processed by HttpConnectionPoolManager::SendAsync in HttpAuthenticatedConnectonHandler:
internal override ValueTask<HttpResponseMessage> SendAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken)
{
return _poolManager.SendAsync(request, async, doRequestAuth: true, cancellationToken);
}

which invokes HttpConnectionPoolManager::SendAsyncCore
public ValueTask<HttpResponseMessage> SendAsync(HttpRequestMessage request, bool async, bool doRequestAuth, CancellationToken cancellationToken)
{
if (_proxy == null)
{
return SendAsyncCore(request, null, async, doRequestAuth, isProxyConnect: false, cancellationToken);
}

and step into its body, it is clearly that HttpConnectionPoolManager::SendAsyncCore calls HttpConnectionPoolManager::GetConnectionKey to get a HttpConnectionKey and pass it to a HttpConnectionPool, then invokes HttpConnectionPool::SendAsync:
public ValueTask<HttpResponseMessage> SendAsyncCore(HttpRequestMessage request, Uri? proxyUri, bool async, bool doRequestAuth, bool isProxyConnect, CancellationToken cancellationToken)
{
HttpConnectionKey key = GetConnectionKey(request, proxyUri, isProxyConnect);
HttpConnectionPool? pool;
while (!_pools.TryGetValue(key, out pool))
{
pool = new HttpConnectionPool(this, key.Kind, key.Host, key.Port, key.SslHostName, key.ProxyUri, _maxConnectionsPerServer);
if (_cleaningTimer == null)
{
// There's no cleaning timer, which means we're not adding connections into pools, but we still need
// the pool object for this request. We don't need or want to add the pool to the pools, though,
// since we don't want it to sit there forever, which it would without the cleaning timer.
break;
}
if (_pools.TryAdd(key, pool))
{
// We need to ensure the cleanup timer is running if it isn't
// already now that we added a new connection pool.
lock (SyncObj)
{
if (!_timerIsRunning)
{
SetCleaningTimer(_cleanPoolTimeout);
}
}
break;
}
// We created a pool and tried to add it to our pools, but some other thread got there before us.
// We don't need to Dispose the pool, as that's only needed when it contains connections
// that need to be closed.
}
return pool.SendAsync(request, async, doRequestAuth, cancellationToken);

It is noticed that the key.SslHostName is passed into HttpConnectionPool::ctor, and if we look furture into GetConnectionKey, there is a snippet about how to acquire the SslHostName:
string? sslHostName = null;
if (HttpUtilities.IsSupportedSecureScheme(uri.Scheme))
{
string? hostHeader = request.Headers.Host;
if (hostHeader != null)
{
sslHostName = ParseHostNameFromHeader(hostHeader);
}
else
{
// No explicit Host header. Use host from uri.
sslHostName = uri.IdnHost;
}
}

which precisely implies what was aforementioned "it uses either the host of the request URI, or your specified Host header", we can find its application here:
if (sslHostName != null)
{
_sslOptionsHttp11 = ConstructSslOptions(poolManager, sslHostName);
_sslOptionsHttp11.ApplicationProtocols = null;
if (_http2Enabled)
{
_sslOptionsHttp2 = ConstructSslOptions(poolManager, sslHostName);
_sslOptionsHttp2.ApplicationProtocols = s_http2ApplicationProtocols;
_sslOptionsHttp2Only = ConstructSslOptions(poolManager, sslHostName);
_sslOptionsHttp2Only.ApplicationProtocols = s_http2OnlyApplicationProtocols;
// Note:
// The HTTP/2 specification states:
// "A deployment of HTTP/2 over TLS 1.2 MUST disable renegotiation.
// An endpoint MUST treat a TLS renegotiation as a connection error (Section 5.4.1)
// of type PROTOCOL_ERROR."
// which suggests we should do:
// _sslOptionsHttp2.AllowRenegotiation = false;
// However, if AllowRenegotiation is set to false, that will also prevent
// renegotation if the server denies the HTTP/2 request and causes a
// downgrade to HTTP/1.1, and the current APIs don't provide a mechanism
// by which AllowRenegotiation could be set back to true in that case.
// For now, if an HTTP/2 server erroneously issues a renegotiation, we'll
// allow it.
Debug.Assert(hostHeader != null);
_http2EncodedAuthorityHostHeader = HPackEncoder.EncodeLiteralHeaderFieldWithoutIndexingToAllocatedArray(H2StaticTable.Authority, hostHeader);
_http3EncodedAuthorityHostHeader = QPackEncoder.EncodeLiteralHeaderFieldWithStaticNameReferenceToArray(H3StaticTable.Authority, hostHeader);
}
if (_http3Enabled)
{
_sslOptionsHttp3 = ConstructSslOptions(poolManager, sslHostName);
_sslOptionsHttp3.ApplicationProtocols = s_http3ApplicationProtocols;
}
}

HttpConnectionPool constructs several SslOptions using our sslHostName and ConstructSslOptions, and the later one is what your example points to:
private static SslClientAuthenticationOptions ConstructSslOptions(HttpConnectionPoolManager poolManager, string sslHostName)
{
Debug.Assert(sslHostName != null);
SslClientAuthenticationOptions sslOptions = poolManager.Settings._sslOptions?.ShallowClone() ?? new SslClientAuthenticationOptions();
// Set TargetHost for SNI
sslOptions.TargetHost = sslHostName;
// Windows 7 and Windows 2008 R2 support TLS 1.1 and 1.2, but for legacy reasons by default those protocols
// are not enabled when a developer elects to use the system default. However, in .NET Core 2.0 and earlier,
// HttpClientHandler would enable them, due to being a wrapper for WinHTTP, which enabled them. Both for
// compatibility and because we prefer those higher protocols whenever possible, SocketsHttpHandler also
// pretends they're part of the default when running on Win7/2008R2.
if (s_isWindows7Or2008R2 && sslOptions.EnabledSslProtocols == SslProtocols.None)
{
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(poolManager, $"Win7OrWin2K8R2 platform, Changing default TLS protocols to {SecurityProtocol.DefaultSecurityProtocols}");
}
sslOptions.EnabledSslProtocols = SecurityProtocol.DefaultSecurityProtocols;
}
return sslOptions;
}

According to my tracking steps above, I think change SslOptions.TargetHost won't work because no matter what value you set to it, it will always be modified at last in ConstructSslOptions unless you can intercept the request after ConstructSslOptions's invocation, btw, I'd actually prefer to "turn off server name completely during this request"

@samsosa
Copy link

samsosa commented Nov 24, 2020

Yup, indeed. The TargetHost setting in the SocketsHttpHandler seems to be completely ignored.

@samsosa
Copy link

samsosa commented Nov 24, 2020

I would say that this could be a bug.

Because SslStream.AuthenticateAsClientAsync(targetHost: String.Empty); switches off the SNI.

Accordingly there should be the distinction:

SocketsHttpHandler.SslOptions.TargetHost = null;  // Like now, from Host-Header or from Url-Host
SocketsHttpHandler.SslOptions.TargetHost = String.Empty;  // Disable SNI
SocketsHttpHandler.SslOptions.TargetHost = "SomeDnsOrIpEndPoint";  // Set the SslStream targetHost to SocketsHttpHandler.SslOptions.TargetHost

@dylech30th
Copy link
Author

I would say that this could be a bug.
Because SslStream.AuthenticateAsClientAsync(targetHost: String.Empty); switches off the SNI.
Accordingly there should be the distinction:

SocketsHttpHandler.SslOptions.TargetHost = null;  // Like now, from Host-Header or from Url-Host
SocketsHttpHandler.SslOptions.TargetHost = String.Empty;  // Disable SNI
SocketsHttpHandler.SslOptions.TargetHost = "SomeDnsOrIpEndPoint";  // Set the SslStream targetHost to SocketsHttpHandler.SslOptions.TargetHost

It seems like they just completely prevented this in higher abstraction layers such as SocketsHttpHandler, I think this might cause two problems: 1. If I have such a requirement like this seems I must implement the underlying by myself which would be complicated and it's somehow unworthy to write a network layer just for a little functionality(not to mention that it should be easy, just add an if branch to check if the TargetHost is already present). 2. There would be a semantic issue because TargetHost is said to be used for certificate validating in docs and it does give us a set accessor but if you specify it in SocketsHttpHandler it might not work as you want because its assignment is completely ignored

@samsosa
Copy link

samsosa commented Nov 25, 2020

I think the Dotnet team just forgot to include it here:

string? sslHostName = null;
if (HttpUtilities.IsSupportedSecureScheme(uri.Scheme))
{
string? hostHeader = request.Headers.Host;
if (hostHeader != null)
{
sslHostName = ParseHostNameFromHeader(hostHeader);
}
else
{
// No explicit Host header. Use host from uri.
sslHostName = uri.IdnHost;
}
}

@karelz karelz added untriaged New issue has not been triaged by the area owner and removed needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration labels Jan 5, 2021
@karelz
Copy link
Member

karelz commented Jan 21, 2021

Triage: @wfurt to look at it and provide recommendation. We may need more info on the use case.

@huoyaoyuan
Copy link
Member

The use case is non-standard that the server refuses SNI.
This maybe somehow unsafe. The client hardcodes the server ip and fingerprint. It doesn't ask for DNS.

This can be useful in some internal network.

@dylech30th
Copy link
Author

The current workaround is rather more unsafe, I have to hook into runtime libraries and modify the runtime behaviors such as return value. IMO this should be considered as a design fault

@wfurt
Copy link
Member

wfurt commented Jan 22, 2021

I think the SocketHttpHandler was never intended for tricks like this. This is conceptually similar to #29149 where once in a while somebody wants to do something special.

The caveat here is that the SslOptions is set on handler and the TargetHost needs to be set for each server. So you would effectively need to create new handler for each site you need to visit - pattern we try to discourage when possible as that is very inefficient . Or at least serialize access if you choose to mutate the options. That may not be concern for the special needs.

Conceptually, this can be also fixed with #46849 / #42455. (so as #29149)

If you are desperate @dylech30th , for 5.0 you can construct `http://a.b.c.d/' URL and use ConnectCallback to return SslStream. That would avoid all the TLS logic in HttpClient and give you complete control.

@huoyaoyuan
Copy link
Member

@wfurt Thanks for your suggestion. I've built a working sample!

    class DirectConnectHandler : HttpMessageHandler
    {
        private readonly HttpMessageInvoker _handler = new(new SocketsHttpHandler
        {
            ConnectCallback = async (context, ct) =>
            {
                var ip = CustomizeIP(context.DnsEndPoint.Host); // Get IP here

                var socket = new Socket(SocketType.Stream, ProtocolType.Tcp);
                await socket.ConnectAsync(IPAddress.Parse(ip), 443, cancellationToken: ct);
                var networkStream = new NetworkStream(socket, true);
                var sslStream = new SslStream(networkStream, false,
                    delegate { return true; });

                await sslStream.AuthenticateAsClientAsync(""); // whatever you want, "" means no SNI
                return sslStream;
            }
        });

        protected override void Dispose(bool disposing)
        {
            base.Dispose(disposing);
            _handler.Dispose();
        }

        protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
        {
            string? newUri = request.RequestUri!.ToString().Replace("https", "http", StringComparison.Ordinal);
            request.RequestUri = new(newUri);
            return _handler.SendAsync(request, cancellationToken);
        }
    }

@wfurt
Copy link
Member

wfurt commented Jan 25, 2021

good to know @huoyaoyuan. #46849 is still coming to 6.0 but now you don't have to wait for it :) It may give you more elegant solution at some point.

@karelz
Copy link
Member

karelz commented Jan 28, 2021

Triage: There is workaround and #42455 should solve this in 6.0. Closing this one as answered.

@karelz karelz closed this as completed Jan 28, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Feb 27, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@karelz karelz removed the untriaged New issue has not been triaged by the area owner label Oct 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Http question Answer questions and provide assistance, not an issue with source code or documentation.
Projects
None yet
Development

No branches or pull requests

8 participants