-
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
[HTTP/3] SNI does not use the Host header value #57169
Comments
Tagging subscribers to this area: @dotnet/ncl Issue DetailsHttpClient HTTP/3's implementation does not use HttpRequestMessage.Headers.Host for SNI, unlike prior HTTP versions.
Expected SNI: "testhost" Priority: Medium, common customer requirement, difficult to work around.
|
@wfurt I think there are some limitation in msquic around SNI, do I remember right? |
We should check if HttpClient sets |
Then this might be easy to fix, we might reconsider this if we have spare time. |
Ok. It is actually runtime/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicConnection.cs Lines 573 to 592 in c97cf0c
Unless @nibanks has suggestion I don't know how to connect to given host but set separate SNI. |
For MsQuic, you must pass the SNI you want in the TLS handshake in |
I don't see this fixup happening, I'm getting a blank SNI on the server when an IP is used. That's how I found this issue. |
That is not the point. In this case the host you connecting to is different than the SNI. And It is possible HttpClient converts the IP to string and then it comes to Quc as DnsEndPoint with IP string. |
runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs Line 852 in 6ac0370
So with HttpClient, we would never hit the + if (!string.IsNullOrEmpty(_state.TargetHost) && !dnsHost.Equals(_state.TargetHost, StringComparison.InvariantCultureIgnoreCase) && IPAddress.TryParse(((DnsEndPoint)_remoteEndPoint).Host!, out IPAddress? address))
+ { and I can possibly fix it for cases when URI is set to address and Host header is set to different value. (_state.TargetHost) Alternatively we would need to fix the code around |
I think this is the primary use case, people bypassing DNS. Eventually it should behave the same as HTTP/1.1 to avoid breaking people as they move up. |
Triage: This will likely require API changes in msquic |
The design MsQuic currently has, and how HTTP.sys uses it, is that if you want to target a different IP than what the SNI points to, you'd resolve the name to whatever IP you choose and then set the remote address to that before calling ConnectionStart. We have no way to use two different host names. |
right, I understand this is current limitation. We can do resolution in .NET but then can we give you list of addresses? It seems suboptimal top duplicate any fall-back logic. And make it same with case when the SNI is same. Also it seems like it would be easy to split SNI and hostanke within MsQuic and track them separately - and in common case have them same. |
Triage: msquic only connects to the first IP to which the hostname resolves and doesn't do any fallbacks. Nowadays we have this fallback on the managed layer in Sockets, doing it in S.N.Quic would be equivalent to that. So we should do the resolution on our side, set remote IP in msquic, and SNI separately. No work for msquic in this, just for us. |
HttpClient HTTP/3's implementation does not use HttpRequestMessage.Headers.Host for SNI, unlike prior HTTP versions.
Expected SNI: "testhost"
Actual SNI: "localhost"
Priority: Medium, common customer requirement, difficult to work around.
The text was updated successfully, but these errors were encountered: