Adding WebTransport Handshake to Kestrel

[Daniel-Genkin-MS-2]

This is step one in my implementation of WebTransport support in Kestrel! This PR focuses on the handshake over HTTP/3. The next one will add the Middleware layer changes and expose an API for interacting with it. Eventually, I will also implement the handshake for HTTP/2.

HTTP/3 Spec: https://www.ietf.org/archive/id/draft-ietf-webtrans-http3-01.html

What's changed

• There are 2 new HttpSettingType values (EnableWebTransport and H3Datagram) which are introduced in the spec.
• New HeaderNames.Protocol which encodes the :protocol pseudo-header
• New KestrelServerOptions properties (EnableWebTransport and EnableH3Datagram)
  • The getters and setters are new public API
• Some control logic that prevents malformed connection requests. (the rest will go into the middleware layer)
• Updated the Http3Sample to support WebTransport over HTTP/3 <--- Should I revert this before merging?

Step 1 of 3 to address: #39583

[Daniel-Genkin-MS-2]

Should I keep the changes to the Http3Sample app or should I revert them prior to merging? It is currently the only configured way to use this handshake.

[Daniel-Genkin-MS-2]

Unit tests are currently broken. I am going to address Chris' comments then fix the tests tomorrow

[gfoidl]

Super nit.

[JamesNK]

• There are 2 new HttpSettingType values (EnableWebTransport and H3Datagram) which are introduced in the spec.

What are people's thoughts on these settings being hardcoded into the HTTP/3 layer? It feels like there should be an extension point to receive and send custom settings. I don't think anything needs to change right now as this is private and can be refactored in the future, but maybe in the future?

[Tratcher]

• There are 2 new HttpSettingType values (EnableWebTransport and H3Datagram) which are introduced in the spec.

What are people's thoughts on these settings being hardcoded into the HTTP/3 layer? It feels like there should be an extension point to receive and send custom settings. I don't think anything needs to change right now as this is private and can be refactored in the future, but maybe in the future?

These type of settings seem like they would be difficult to provide via extensions because they indicate that the server has additional HTTP/3 capabilities, so you'd also have to be able to provide that functionality. EnableWebTransport redefines how the server handles CONNECT requests, that's not something that can be added at the application layer.

[halter73]

There's still no logic to reject a "NOT_webtransport" protocol. This test is only passing because the request is getting rejected due to a missing path.

We should use a unique Http3StreamErrorException.Message for each rejection reason and we should always be always using the matchExpectedErrorMessage to verify were rejecting the request for the right reasons.

aspnetcore/src/Servers/Kestrel/shared/test/Http3/Http3InMemory.cs

Lines 576 to 589 in 0d1a2a8

	internal async Task WaitForStreamErrorAsync(Http3ErrorCode protocolError, Action<string> matchExpectedErrorMessage = null, string expectedErrorMessage = null)
	{
	var result = await ReadApplicationInputAsync();
	if (!result.IsCompleted)
	{
	throw new InvalidOperationException("Stream not ended.");
	}
	if ((Http3ErrorCode)Error != protocolError)
	{
	throw new InvalidOperationException($"Expected error code {protocolError}, got {(Http3ErrorCode)Error}.");
	}
	matchExpectedErrorMessage?.Invoke(expectedErrorMessage);
	}

I don't think matchExpectedErrorMessage?.Invoke(expectedErrorMessage) is never going to fail though. 🤣 It looks like we need to update WaitForStreamErrorAsync to check matchExpectedErrorMessage against the abortReason.Message passed into TestStreamContext.Abort(ConnectionAbortedException abortReason) while we're at it.

[halter73]

Or maybe it can look at the LogMessages like Http2TestBase does.

aspnetcore/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs

Lines 1287 to 1290 in 0d1a2a8

	if (expectedErrorMessage != null)
	{
	Assert.Contains(LogMessages, m => m.Exception?.Message.Contains(expectedErrorMessage) ?? false);
	}

[Daniel-Genkin-MS-2]

Ohh, ok thank you for clarifying. I didn't understand what you meant before. I will make sure to fix this!

[Daniel-Genkin-MS-2]

I agree that this logic should be in place, and I originally had it here. However, Chris told me, in an earlier comment, that he wants to keep this chain of if statements completely general and that the middleware should do the checking for webtransport specific conditions. So, I removed it from here.

[halter73]

However, Chris told me, in an earlier comment, that he wants to keep this chain of if statements completely general and that the middleware should do the checking for webtransport specific conditions. So, I removed it from here.

I talked this over with Chris and I agree. Other protocols could use the "extended CONNECT method" defined in https://datatracker.ietf.org/doc/html/rfc8441#section-4, so it should be the middleware verifying the protocol is "webtransport".

I still think we should be verifying the error messages in this test. Sorry that this turned into a little bit of yak shaving fixing the WaitForStreamErrorAsync, but I can help out with that.

[halter73]

Suggested change

_serviceContext.ServerOptions.AllowAlternateSchemes = true;

Since the extended connect method spec doesn't require "https" scheme, let's go back to using the actual "http" scheme in these tests.

[Daniel-Genkin-MS-2]

Doesn't http3 require https though? So, for consistency with the real use cases, shouldn't we try to maximize the use of https?

[halter73]

We discussed this offline. My feeling is unless we're doing something special with the scheme value in this particular test, it should match the other tests as much as possible.

[halter73]

My reading of the "extended CONNECT method" spec is that :scheme and :path are required, but not :authority.

The spec is basically saying that we should treat :authority the way we normally would for non-CONNECT requests, which we're doing.

[Daniel-Genkin-MS-2]

Ok. So, since webtransport requires :authority, this check should be in the Middleware?

[halter73]

Can you submit an API proposal for this? I know there are going to be a bunch more APIs, but it might be good to learn the process by quickly filling out the issue template.

[Tratcher]

I already did as part of my h2 WebSockets PR. This field is duplicated in both.

[halter73]

Is there an API proposal for that yet? The PR doesn't count! I figured approving this one API would be easier than approving the full extended connect server feature we were discussing.

[Daniel-Genkin-MS-2]

@Tratcher have you already filed one for :protocol as part of your websockets PR?
Edit: nevermind, should have refreshed the page sooner.

[Daniel-Genkin-MS-2]

API proposal: #42002

[halter73]

We should wait for #42002 to be api-approved before merging. If this is blocking anything, I expect we can approve it very quickly over email considering it's a small API change.

[Daniel-Genkin-MS-2]

You can set this in the csproj https://docs.microsoft.com/en-us/dotnet/core/extensions/httpclient-http3#httpclient-settings

Ok Cool! I will do this in the dedicated sample app for webtrasport. This one is shared for all http/3 so I think it is best to keep it siloed
Nevermind. Upon offline discussion, I will include this in the csproj lol

[Daniel-Genkin-MS-2]

Oh, I didn't realize that auto-merge was on. I think I must have clicked it accidentally. I still intend to go through the API review process on Monday with this change.

[ghost]

Hi @Daniel-Genkin-MS-2. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

[1someone1]

Hi @Daniel-Genkin-MS-2 , just curious because I had one kesterl WebTransport implementation month ago which add this according to the draft:

// https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-h3-websockets-02#section-5
ENABLE_CONNECT_PROTOCOL = 0x8

Does it mean we actually don't have to set enable connect protocol manually and can get WebTransport worked?
Btw thanks for your great work :)

[Daniel-Genkin-MS-2]

@1someone1 your link is for WebSockets, it's a different spec. For WebTransport it says this:

[RFC8441] defines an extended CONNECT method in Section 4, enabled by the SETTINGS_ENABLE_CONNECT_PROTOCOL parameter. That parameter is only defined for HTTP/2. This document does not create a new multi-purpose parameter to indicate support for extended CONNECT in HTTP/3; instead, the SETTINGS_ENABLE_WEBTRANSPORT setting implies that an endpoint supports extended CONNECT.

From: https://ietf-wg-webtrans.github.io/draft-ietf-webtrans-http3/draft-ietf-webtrans-http3.html#name-extended-connect-in-http-3

[1someone1]

That make sense, thank you!