dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15k stars 4.67k forks source link

Add support for custom ALPN list in HttpClient #29149

Open JonathanPCGuy opened 5 years ago

JonathanPCGuy commented 5 years ago

In creating a HttpClient, one can pass in a SocketsHttpHandler. In SocketsHttpHandler an instance of SslOptions can be passed in. One of the options in SslOptions is the alpn list (SslApplicationProtocol), set via the ApplicationProtocols property.

Thus we have a setup like this:

var sslOptions = new SslClientAuthenticationOptions();
sslOptions.ApplicationProtocols = new List<System.Net.Security.SslApplicationProtocol>
{
    new SslApplicationProtocol("some-alpn-string")
};
SocketsHttpHandler socketsHttpHandler = new SocketsHttpHandler
{
    SslOptions = sslOptions
};
HttpClient httpClient = new HttpClient(socketsHttpHandler);

However when making the http call, the ALPN value(s) are never sent as part of ClientHello. Looking at wireshark traces confirms the absence of ALPN values.

In contrast if we use a raw TcpClient to make the connection, we do see ALPN as part of ClientHello.

I think that due to how http functionality is designed, its ALPN functionality is currently setup to focus on http 1.1 and http 2, or have none at all. It would be nice however if a developer, should they have a need to, have the ability to override this list for a specific purpose while still using http as the underlying high level protocol.

To me, an enhancement would be like adding a field to like SocketsHttpHandler a property like "UseSslApplicationProtocolList" to tell the library to use the alpn list passed in and to ignore the built in ones. But the field can go elsewhere as appropriate.

To keep things simple, we can have the underlying http protocol assumed to be http1.1, unless there is a way we can say that this ALPN string should use http2 or http1.1 underneath.

karelz commented 5 years ago

To all my knowledge, this should just work. AFAIK we do not distinguish between known and custom ALPN values. If you change the code about to Http2 or other known value, does it work?

If you are sure this is a bug, we will need a full working repro to test.

Which version do you run on?

JonathanPCGuy commented 5 years ago

I've used .net core 2.1 all the way to the latest 3.0 preview. Currently using 2.2.

Just retried and put in Http2 in the protocol list. No ALPN sent up as part TLS negotiation.

Below is a code sample that shows the issue. (Just create a new .net core 2.2 console app and replace the code with what's below)

using System;
using System.Net.Http;
using System.Net.Security;
using System.Collections.Generic;

namespace CoreTest
{
    class Program
    {
        static void Main(string[] args)
        {
            var sslOptions = new SslClientAuthenticationOptions();

            sslOptions.ApplicationProtocols = new List<System.Net.Security.SslApplicationProtocol>
            {
                new SslApplicationProtocol("some-protocol")
            };

            SocketsHttpHandler socketsHttpHandler = new SocketsHttpHandler
            {
                SslOptions = sslOptions
            };

            HttpClient httpClient = new HttpClient(socketsHttpHandler);
            var testing = new HttpClient();
            string postData = "Test";
            HttpContent content = new StringContent(postData);

            // endpoint does not matter, just need something reachable/to connect to show the ClientHello message
            // use ip address to make it easier to find in wireshark
            string requesturi = @"https://www.microsoft.com";
            try
            {
                var response = httpClient.PostAsync(requesturi, content).Result.Content.ReadAsStringAsync().Result;
                Console.WriteLine(response);
            }
            catch(Exception ex)
            {
                Console.WriteLine(ex);
            }
        }
    }
}

I would have expected some-protocol to show up in ClientHello under application_layer_protocol_negotiation (when the connection attempt is viewed in wireshark). But it does not show up.

Perhaps there's a flag or setting that I'm missing?

karelz commented 5 years ago

This is suspicious ... @wfurt can you please take a look?

wfurt commented 5 years ago

what OS are you on @JonathanPCGuy ? Note that even if you use 3.0 SDK it matters what do you target. You get latest bits only if you target netcoreapp3.0

JonathanPCGuy commented 5 years ago

@wfurt windows 10, 1703.

yea when I tried 3.0 I explicitly set my project to target .net core 3.0. I know this because when I opened my test project in vc 2019 release it wouldn't compile as 3.0 preview isn't apparently compatible with vc 2019 release version (only the preview version)

wfurt commented 5 years ago

I was able to reproduce it. The reason why it does not work is here: https://github.com/dotnet/corefx/blob/master/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs#L148

Provided value is lost and it is later used to construct SslStream. When I removed that line, everything works as expected. Since default for ApplicationProtocols is null, it would work nicely.

However I'm not sure about HTTP/2. We need ALPN to establish HTTP/2 session. But maybe it is OK if we use provided value and caller is responsible for proper values. Any thoughts on this @davidsh @stephentoub @geoffkizer

JonathanPCGuy commented 5 years ago

Just wanted to check my understanding.

If the following is true:

end result: client and server are talking with http2

Is that true? If so then the custom ALPN list should work without breaking anything else. Of course people should be aware of this before setting the value.

The only issue I can see with suddenly respecting this value is if there are other applications out there that have set that ALPN list for whatever reason, but I don't think that should be an issue.

I think that is what you're asking of the others @wfurt ?

If we can get support in for custom ALPN on http that would be awesome, as there are some applications out there that take advantage of ALPN for various reasons.

wfurt commented 5 years ago

It will fallback to HTTP/1.x if the server chooses "custom-http2" Since that is something HttpClinet does not recognize it will assume H/2 is not available. In general, client provides list in preferred order and server pick first one it can support. So you cannot pick custom and H/2.

JonathanPCGuy commented 5 years ago

just following up on this... i'm hoping to have this for http/1 stuff (http/2 is outside my needed scope anyways). it's kind of a niche edge case but there are a few APIs out there that take advantage of ALPN over HTTP, so having this level of control right out of the box would be awesome. i know it may not be a high priority but i would like to know if it would never be done.

@wfurt @davidsh @stephentoub @geoffkizer

karelz commented 5 years ago

We should re-assess after shipping 3.0. I don't have enough insights to tell never / maybe one day.

geoffkizer commented 5 years ago

there are a few APIs out there that take advantage of ALPN over HTTP

Can you point me to these? I'd like to understand the impact here better.

It seems really weird to me that an application would want to control the ALPN list here. We need to check the negotiated protocol to determine whether to proceed with HTTP/1.1 or HTTP2. If we get something unexpected back as the negotiated protocol, it's not clear how to proceed.

JonathanPCGuy commented 5 years ago

main example:

https://docs.aws.amazon.com/iot/latest/developerguide/protocols.html (see note about port 443 and HTTPS)

They're doing this to allow non-client cert and client cert authentication to both go over the same port from the same host. There is a dedicated port number that doesn't require ALPN, but they offer the same service over port 443 (with ALPN) for scenarios where not all port numbers may be accessible from a client (like a firewall or something).

@geoffkizer

if we wanted to keep it simple and not change anything else we could just assume HTTP/1.1 if custom ALPN was set (that's how I think it would work today if the ALPN list was not ignored from my understanding). If we wanted more control and put it on the developer then hypothetically there would need to be some sort of parameter to say "hey i'm really trying to make a HTTP/1.1 or HTTP/2 connection, try to do so regardless of the result of the ALPN negotiation".

geoffkizer commented 5 years ago

Interesting, thanks. Seems kind of wacky to me.

if we wanted to keep it simple and not change anything else we could just assume HTTP/1.1 if custom ALPN was set (that's how I think it would work today if the ALPN list was not ignored from my understanding).

Yeah, we'd need to do something like this. I think we'd probably do something like: (1) If the user provides their own ALPN values, don't override them (2) If the negotiated protocol is HTTP2, do HTTP2, otherwise do HTTP/1.1.

wfurt commented 5 years ago

This should be easy to do @geoffkizer. We should be able to preserve existing logic for downgrade and make it consistent.

JonathanPCGuy commented 5 years ago

fwiw @geoffkizer i guess it would be possible for a scenario where one would want to be able to have custom ALPN (like to multiplex port 443 to allow TLS client auth and non-client auth scenarios), but communicate with HTTP/2, but since i don't know of any example / real world usage of that yet i do not think there is a need to even look at supporting such a scenario.

karelz commented 4 years ago

Triage: We need to decide what happens if dev sets HTTP/2 version AND uses custom ALPN list (without "h2"). We can either use only custom ALPN list, or use only "h2" and ignore custom ALPN list, or add "h2" into the custom ALPN list to be sent out.

JonathanPCGuy commented 4 years ago

@karelz ideally for me i think the ALPN list is ultimately up to the developer and they should be aware of the consequences of specifying http2 and a custom ALPN. i think though if people go out of their way to specify a custom alpn list they have a pretty good reason to do so, and we should probably respect that as long as it doesn't break normal operations for everyone else.

going back to 3rd party APIs, I could see someone down the road using ALPN with http2 to offer a way to multiplex different authentication scenarios, but at the same time that's kind of like engineering for a very small possibility.

another good reason to possibly do this is if we engineer in the ability to accept in different ALPN lists, this could set the framework for any future versions of HTTP that could require a different version of ALPN, and could make it easier to add/drop support for such connections

stephentoub commented 4 years ago

@JamesNK was just asking about this in https://github.com/dotnet/corefx/issues/41701. Changing our logic here to respect the ALPN list if provided would allow, for example, gRPC to prohibit downgrading to HTTP/1.1.

JamesNK commented 4 years ago

When I looked into this yesterday I came across a potential equivalent API on Java's OkHttp library: https://square.github.io/okhttp/3.x/okhttp/okhttp3/OkHttpClient.Builder.html#protocols-java.util.List-

Interestingly "HTTP/2 only" is only supported with h2c prior-knowledge mode. When using ALPN you must have HTTP/1 along with HTTP/2. I don't know if that is a technical limitation or a compromise in their API for backwards compatibility.

krwq commented 4 years ago

Per other issue seems the options don't get propagated to the SslStream