aspnetcore icon indicating copy to clipboard operation
aspnetcore copied to clipboard

Make HttpProtocols configuration parsing consistent with SslProtocols

Open Eagle3386 opened this issue 1 year ago • 17 comments

[Edit]

Discussion Summary: it's really not intuitive how to specify multiple protocols if there isn't an existing enum member for the combination (e.g. Http1AndHttp2 exists, but Http2AndHttp3 doesn't). It turns out you can you comma-separation - "Http2, Http3", but that's hard to discover and quite different from how it looks in code (which uses |). SslProtocols appears to support an actual JSON list ([amcasey] I'm not 100% sure the meaning is equivalent), but it's not clear why the two would be inconsistent. We should think about how to make them both consistent and intuitive.

The actual code change will likely be quite simple (Help Wanted?), but we probably need to think through what we actually want here.

Also, it might be time to introduce "TlsProtocols", given how deprecated SSL is.

[Original]

Is there an existing issue for this?

  • [X] I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

Currently, there are 2 enums to limit allowed HTTP protocols in Kestrel: https://github.com/dotnet/aspnetcore/blob/c1b76241bf4043e3e54edbb236963c503eec18e0/src/Servers/Kestrel/Core/src/HttpProtocols.cs#L30 https://github.com/dotnet/aspnetcore/blob/c1b76241bf4043e3e54edbb236963c503eec18e0/src/Servers/Kestrel/Core/src/HttpProtocols.cs#L40

For those of us already on the gRPC "train", HTTP/2 is the lowest possible version for us. But as soon as we're going for HTTP/3, we've got to set the latter, Http1AndHttp2AndHttp3, which feels rather strange.

Additionally, I happen to have a scenario where a non-gRPC app & its services are supposed to explicitly exclude clients without at least HTTP/2 support - which is rather cumbersome to achieve, given how easy it would be with an added Http2AndHttp3 value.

Describe the solution you'd like

I'd like to have said Http2AndHttp3 added to the HttpProtocols enum.

Additional context

None I can think of right now, but I'd happily provide such on request.

Eagle3386 avatar Sep 26 '24 08:09 Eagle3386

I wouldn't say that enum is our finest work. I believe the point of the "And" members you mentioned was to function as "All" values at the times they were added. Giving a name to each combination of flags wasn't really a goal because it would quickly become unwieldy (even more than the enum already has).

Is there a reason you can't set HttpProtocols.Http2 | HttpProtocols.Http3?

amcasey avatar Oct 04 '24 00:10 amcasey

@amcasey I do understand that & while there's always room for improvement (and refactoring 😉), I simply didn't even try your suggestion as Visual Studio would tell you this: {79E0B62F-2361-4A07-AF7A-E652DDE72B36}

But I tried it just now & it seems to work - at least, there's no warning or even error. However, due to my app being Blazor WASM standalone & talking via gRPC with my BFF backend, explicitly enforcing HTTP/3-only on both ends seems to be partly ignored/overridden, because there are still HTTP/2 calls being executed.. 😅

Eagle3386 avatar Oct 08 '24 09:10 Eagle3386

Ah, I had assumed you were configuring the protocols in code - are you actually doing it in JSON?

Regarding enforcement, AFAIK, that property only tells the server which protocols to listen for (though, as noted above, I don't actually know which setting you're changing). I think your (new?) question is how to force gRPC to use HTTP/3 between your frontend and your backend?

amcasey avatar Oct 08 '24 17:10 amcasey

Yes, on the Kestrel side of life, it's JSON-only. 😉 It all started mostly due to different certificate paths on our different environments, but now it's also due to convenience (compile once, configure anytime) - here's the base for any environment:

// … left out for brevity…
  "Kestrel": {
    "EndpointDefaults": {
      // TODO: Remove below after https://github.com/dotnet/aspnetcore/blob/main/src/Servers/Kestrel/Core/src/ListenOptions.cs#L19
      //       includes HTTP/3 to enable it by default.
      "Protocols": "Http2 | Http3", // gRPC ensures HTTP/2 as minimum by itself.
      "SslProtocols": [ "Tls12", "Tls13" ]
    }
  },
// … left out for brevity…

On the client side, it's simply

// … left out for brevity…
channel.DisposeHttpClient = true;
channel.HttpHandler       = new GrpcWebHandler(GrpcWebMode.GrpcWeb, new HttpClientHandler());
channel.HttpVersionPolicy = HttpVersionPolicy.RequestVersionOrHigher;
// … left out for brevity…

because additionally setting channel.HttpVersion = new(3, 0); changes pretty much nothing: especially HTTP OPTIONS requests are still done via HTTP/2 - which brings me to confirming your last question.

Yes, I'd love to see the possibility of enforcing HTTP/3. IMHO, if that breaks gRPC(-Web) in Blazor, Kestrel or ASP.NET in general, that shouldn't be prevented by an "enforced downgrade" to HTTP/2 as lowest HttpVersion & instead should throw exception when something inside ASP.NET/Blazor/Kestrel suddenly can't play along.

However, I'm willing to accept the current state regarding that, because I got the impression that this is on the roadmap for "to be fixed in .NET 9/10" & not "someday in 203x".. 😅😉

Eagle3386 avatar Oct 09 '24 15:10 Eagle3386

I would guess that string is just running through Enum.Parse, so I'd be pretty surprised if it handled | correctly. Personally, I think I'd rather improve parsing support than add a complete set of enum members.

Is this what you're looking for in terms of limiting versions? https://github.com/grpc/grpc-dotnet/pull/2514

Edit: I think this is the parsing code. https://github.com/dotnet/aspnetcore/blob/f14f99eb634dc5a3ce8d5e11c7522577e5473e90/src/Servers/Kestrel/Core/src/Internal/ConfigurationReader.cs#L176

amcasey avatar Oct 09 '24 19:10 amcasey

Ok, thought there might be some magic or at least dragons deep down below, making it "somehow" working. Thanks for pointing it out! 👍🏻

I'm rather looking for reverting James' limitation as I'm both, in full control of client / services & deliberately aiming for explicit exclusion of older browsers/platforms from access.

Eagle3386 avatar Oct 09 '24 19:10 Eagle3386

I'm rather looking for reverting James' limitation as I'm both, in full control of client / services & deliberately aiming for explicit exclusion of older browsers/platforms from access.

You'd have to ask @JamesNK about that.

amcasey avatar Oct 09 '24 19:10 amcasey

Well, I already did: https://github.com/grpc/grpc-dotnet/issues/2546#issuecomment-2379008144

But the outcome suggested to me that Kestrel would benefit from an Http2AndHttp3 enum value - which in turn caused me to open this issue.. 😅

Besides, as he wrote in the other issue, explicitly setting HttpVersion to 3.0 isn't fully respected - there are still calls done with HTTP/2 (mostly HTTP OPTIONS prefetch requests as I understand it).

Eagle3386 avatar Oct 09 '24 19:10 Eagle3386

@Eagle3386 I've just learned that you can use commas to separate enum values. Do you want to give that a shot and see if you can specify HTTP/2 and HTTP/3?

amcasey avatar Oct 14 '24 17:10 amcasey

@amcasey Nice! Where did you get that bit from?

Regarding me, giving a shot by using:

  "Kestrel": {
    "EndpointDefaults": {
      "Protocols": [ "Http2", "Http3" ], // Adapted from SslProtocols below
      "SslProtocols": [ "Tls12", "Tls13" ]
    }

I must report failure - but using:

  "Kestrel": {
    "EndpointDefaults": {
      "Protocols": "Http2,Http3", // This looks just weird & wrong, but let's try anyway 🥳
      "SslProtocols": [ "Tls12", "Tls13" ]
    }

these 2 VS output log lines appeared out of nowhere:

Microsoft.AspNetCore.Server.Kestrel.Transport.Quic: Debug: QUIC listener starting with configured endpoint 127.0.0.1:4430.
Microsoft.AspNetCore.Server.Kestrel.Transport.Quic: Debug: QUIC listener starting with configured endpoint [::1]:4430.

Does that count for you? If so, could either your commit still get merged or, better yet, the parsing be adapted to how SslProtocols are parsed, please? Because using Http2 | Http3 seems way more natural to me than "Http2,Http3" possibly ever could - but having both properties set up the same way, i.e., "<Name>": [ "Value1", "Value2", … ] seems to be the best approach as both properties specify Kestrel's protocol handling.

N.B.: This might even be used to deprecate SslProtocol in favor of TlsProtocols as property name, especially since all SSL versions are forbidden nowadays & even TLS 1.0 is already marked deprecated.. 😉

Eagle3386 avatar Oct 14 '24 18:10 Eagle3386

https://github.com/dotnet/aspnetcore/pull/58328#issuecomment-2403604287

I agree that commas look wrong, but I understand why we (aspnetcore) would be reluctant to diverge from the usual enum parsing behavior of dotnet. FWIW, I think it's tolerant of whitespace.

As for accepting a list, that seems sensible but might be hard to prioritize.

amcasey avatar Oct 14 '24 18:10 amcasey

Thanks for the link, explanation & hint at using a whitespace. It's a pleasure working with you, Andrew! 👏🏻

Could we agree on using commas as a workaround until list is allowed for Protocols, too? And if so, how to achieve the highest success rate: you or me, new issue or something else (please name it)? 🤔

Eagle3386 avatar Oct 14 '24 19:10 Eagle3386

Could we agree on using commas as a workaround until list is allowed for Protocols, too? And if so, how to achieve the highest success rate: you or me, new issue or something else (please name it)? 🤔

I think these were separate questions.

If commas are working for you, then it seems like a good workaround. I agree that depending on implementation details like that isn't ideal (plus the syntax is ugly), but we'll have to maintain it now for back-compat, so I wouldn't worry too much.

I think the second question was about how to get support for a list. With your permission, I'll re-title this issue and add some notes about where the discussion ended up and then we can use it to collect support for the change.

If you still have questions/requests about strictly limiting the protocol, I think we can move those to a separate issue. (I can't quite tell from your earlier conversation whether it belongs here or in the grpc repo.)

amcasey avatar Oct 14 '24 19:10 amcasey

Alright.

Permission granted, too.

Strict protocol limitation isn't a problem here (especially not after having said workaround), neither in Kestrel nor even gRPC. However, according to @JamesNK (see my linked comment above, he said: "HTTP/3 is only supported on full .NET (not WebAssembly).") it's a limitation of Blazor WASM, hence ASP.NET Core, which can't do HTTP/3 in WASM.

If there's already an issue for that, please let me know, so I can subscribe & vote. I'd love to see HTTP/3 in Blazor WASM - the sooner, the better. 😍

Eagle3386 avatar Oct 14 '24 19:10 Eagle3386

"HTTP/3 is only supported on full .NET (not WebAssembly).") it's a limitation of Blazor WASM, hence ASP.NET Core, which can't do HTTP/3 in WASM.

@MackinnonBuck Do you happen to know whether that's already tracked somewhere?

amcasey avatar Oct 14 '24 19:10 amcasey

With Blazor WASM, it's up to the browser to decide what protocol to use when making HTTP requests. It can't be controlled from .NET.

JamesNK avatar Oct 15 '24 00:10 JamesNK

@JamesNK How come Steve Sanderson had a WASI (prototype) SDK for even creating your own WASM-based HTTP client then?

I thought, the whole purpose of WASM is that you get the security of a sandbox while dropping several (platform) constraints & retain "near native" performance - which, for me at least, includes "<whatever does the network calls>" being able to tell Chrome/Edge/Firefox: "Hey buddy, we'll do HTTP/3 from here on".

Eagle3386 avatar Oct 15 '24 16:10 Eagle3386

@JamesNK How come Steve Sanderson had a WASI (prototype) SDK for even creating your own WASM-based HTTP client then?

I thought, the whole purpose of WASM is that you get the security of a sandbox while dropping several (platform) constraints & retain "near native" performance - which, for me at least, includes "<whatever does the network calls>" being able to tell Chrome/Edge/Firefox: "Hey buddy, we'll do HTTP/3 from here on".

Think of the Wasm sandbox as a highly constrained VM that has no access to the outside world other that imported methods that the host implements. In the case of a browser host all I/O is ultimately implemented through the JavaScript apis including network calls.

lewing avatar Jan 22 '25 01:01 lewing