HTTP/3: QuicConnectionListener supports ServerOptionsSelectionCallback

[JamesNK]

ASP.NET Core allows you to configure a callback that creates SslServerAuthenticationOptions when a TLS handshake happens. This is done using the ServerOptionsSelectionCallback delegate.

Example use:

serverOptions.ListenLocalhost(5001, listenOptions =>
{
    listenOptions.Protocols = HttpProtocols.Http3;
    listenOptions.UseHttps((SslStream stream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken) =>
    {
        return ValueTask.FromResult((new SslServerAuthenticationOptions()));
    }, state: null);
});

Does this callback make sense with QUIC? SslStream would be null, but SslClientHelloInfo and state could still be used to customize auth options with QUIC.

[ghost]

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

Issue Details

---

ASP.NET Core allows you to configure a callback that creates SslServerAuthenticationOptions when a TLS handshake happens. This is done using the ServerOptionsSelectionCallback delegate.

Example use:

serverOptions.ListenLocalhost(5001, listenOptions =>
{
    listenOptions.Protocols = HttpProtocols.Http3;
    listenOptions.UseHttps((SslStream stream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken) =>
    {
        return ValueTask.FromResult((new SslServerAuthenticationOptions()));
    }, state: null);
});

Does this callback make sense with QUIC? SslStream would be null, but SslClientHelloInfo and state could still be used to customize auth options with QUIC.

Author:	JamesNK
Assignees:	-
Labels:	area-System.Net.Security, untriaged
Milestone:	-

[ghost]

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

Issue Details

---

ASP.NET Core allows you to configure a callback that creates SslServerAuthenticationOptions when a TLS handshake happens. This is done using the ServerOptionsSelectionCallback delegate.

Example use:

serverOptions.ListenLocalhost(5001, listenOptions =>
{
    listenOptions.Protocols = HttpProtocols.Http3;
    listenOptions.UseHttps((SslStream stream, SslClientHelloInfo clientHelloInfo, object state, CancellationToken cancellationToken) =>
    {
        return ValueTask.FromResult((new SslServerAuthenticationOptions()));
    }, state: null);
});

Does this callback make sense with QUIC? SslStream would be null, but SslClientHelloInfo and state could still be used to customize auth options with QUIC.

Author:	JamesNK
Assignees:	-
Labels:	area-System.Net.Quic, untriaged
Milestone:	-

[karelz]

@wfurt do you have more insights here?
It looks like msquic does not support anything like this ...

[JamesNK]

I think the msquic NEW_CONNECTION event, which is already used by MsQuicListener, has the required info:

runtime/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicListener.cs

Lines 160 to 205 in 31c28fc

	private static unsafe uint NativeCallbackHandler(
	IntPtr listener,
	IntPtr context,
	ref ListenerEvent evt)
	{
	if (evt.Type != QUIC_LISTENER_EVENT.NEW_CONNECTION)
	{
	return MsQuicStatusCodes.InternalError;
	}
	State state = (State)GCHandle.FromIntPtr(context).Target!;
	SafeMsQuicConnectionHandle? connectionHandle = null;
	try
	{
	ref NewConnectionInfo connectionInfo = ref *evt.Data.NewConnection.Info;
	IPEndPoint localEndPoint = MsQuicAddressHelpers.INetToIPEndPoint(ref *(SOCKADDR_INET*)connectionInfo.LocalAddress);
	IPEndPoint remoteEndPoint = MsQuicAddressHelpers.INetToIPEndPoint(ref *(SOCKADDR_INET*)connectionInfo.RemoteAddress);
	connectionHandle = new SafeMsQuicConnectionHandle(evt.Data.NewConnection.Connection);
	uint status = MsQuicApi.Api.ConnectionSetConfigurationDelegate(connectionHandle, state.ConnectionConfiguration);
	QuicExceptionHelpers.ThrowIfFailed(status, "ConnectionSetConfiguration failed.");
	var msQuicConnection = new MsQuicConnection(localEndPoint, remoteEndPoint, connectionHandle);
	msQuicConnection.SetNegotiatedAlpn(connectionInfo.NegotiatedAlpn, connectionInfo.NegotiatedAlpnLength);
	if (!state.AcceptConnectionQueue.Writer.TryWrite(msQuicConnection))
	{
	// This handle will be cleaned up by MsQuic.
	connectionHandle.SetHandleAsInvalid();
	msQuicConnection.Dispose();
	return MsQuicStatusCodes.InternalError;
	}
	return MsQuicStatusCodes.Success;
	}
	catch (Exception ex)
	{
	// This handle will be cleaned up by MsQuic by returning InternalError.
	connectionHandle?.SetHandleAsInvalid();
	state.AcceptConnectionQueue.Writer.TryComplete(ex);
	return MsQuicStatusCodes.InternalError;
	}
	}

SslStream would be null, but the server name on SslClientHelloInfo is available from the event's NewConnectionInfo argument.

I wonder if QuicListenerOptions.SslServerAuthenticationOptions should be changed to a ServerOptionsSelectionCallback delegate that is run in the NEW_CONNECTION event. It seems better to have one way to supply the options to the listener, rather than an options property and callback property on QuicListenerOptions. If someone wants static options then they can always return the same result from the callback.

[wfurt]

I'm not sure @karelz. I think this is one of the missing parts. This will be likely more problematic IMHO as QUIC closely integrates with TLS ClientHello. When NEW_CONNECTION fires, it may be too late to too early.
I think it is good to keep it open for tracking

cc: @ManickaP

[scalablecory]

Triage: @wfurt to work with @nibanks to figure out if this works.

[adityamandaleeka]

We think this is important to get done in .NET 7.

[wfurt]

There are several issue @adityamandaleeka. MsQuic needs ALPN before starting the Listener. The other part is API shape. This is more then property and the Listener concept is very different from single SslStream. There you can also pass state and cancellation but neither fist to the Listener. If we work out the API the details should be doable IMHO.

[adityamandaleeka]

If we work out the API the details should be doable IMHO.

Sounds good. For context, I left that message when we were triaging and prioritizing the issues in Future opened by us (aspnet).

[karelz]

@adityamandaleeka we do not distinguish between Future and 7.0 yet. Once 6.0 is wrapped up, we will retriage and start making the distinction.
Doing it now would be just distraction from our main goal - ship high quality 6.0.

[ManickaP]

MsQuic needs ALPN to start the listener, but nothing else from SslOptions.
The security configuration is done on the connection instance when NEW_CONNECTION event arrives with SNI information. If the SslOptions do not have certificate but have cert selection delegate, we'll invoke it with the new connection.
So functionally this should be covered. What's open is the shape of the API.
Is having all SslOptions in listener ctor problematic even with working cert selection callback? Do you have any preferences/requirements on the shape of the API?

cc @JamesNK

[wfurt]

Can we fix this? I see no fundamental reason why QUIC would need ALPN upfront.
cc: @nibanks.

[ManickaP]

The ServerOptionsSelectionCallback is callback used SslStream.AuthenticateAsServerAsync to get the SslServerAuthenticationOptions instead of providing them statically in listener ctor.

This is about functional parity with SslStream that's used directly in Kestrel for HTTP/1|2.

The callback: ValueTask<SslServerAuthenticationOptions> ServerOptionsSelectionCallback(SslStream stream, SslClientHelloInfo clientHelloInfo, object? state, CancellationToken cancellationToken);
Obviously we don't have an SslStream in QUIC, we'd need to use QuicConnection --> define a new delegate.

I assume this is something that would Kestrel use somewhere here: https://source.dot.net/#Microsoft.AspNetCore.Server.Kestrel.Core/Middleware/HttpsConnectionMiddleware.cs,507
We'd need to shuffle the code a bit around here:

runtime/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicListener.cs

Lines 280 to 286 in 5906521

	connectionHandle = new SafeMsQuicConnectionHandle(evt.Data.NewConnection.Connection);
	uint status = MsQuicApi.Api.ConnectionSetConfigurationDelegate(connectionHandle, connectionConfiguration);
	if (MsQuicStatusHelper.SuccessfulStatusCode(status))
	{
	msQuicConnection = new MsQuicConnection(localEndPoint, remoteEndPoint, state, connectionHandle, state.AuthenticationOptions.ClientCertificateRequired, state.AuthenticationOptions.CertificateRevocationCheckMode, state.AuthenticationOptions.RemoteCertificateValidationCallback);
	msQuicConnection.SetNegotiatedAlpn(connectionInfo.NegotiatedAlpn, connectionInfo.NegotiatedAlpnLength);

but it's doable.

@JamesNK @Tratcher considering we cannot reuse the existing delegate (due to SslStream) any thoughts on priority (how critical it is) and any other thoughts on the callback?

[Tratcher]

Being able to configure the SslServerAuthenticationOptions per connection with SNI input is a blocking requirement, though some properties are more important than others like ServerCertificate, ClientCertificateRequired, RemoteCertificateValidationCallback.

We want to use the same kestrel configuration HTTP 1, 2, and 3. The main delegate in Kestrel we want to make work is:
public Func<TlsHandshakeCallbackContext, ValueTask<SslServerAuthenticationOptions>> OnConnection
https://github.com/dotnet/aspnetcore/blob/e24ff5418aa4d065338de6e74188235bd32c5d14/src/Servers/Kestrel/Core/src/TlsHandshakeCallbackOptions.cs#L19
https://github.com/dotnet/aspnetcore/blob/e24ff5418aa4d065338de6e74188235bd32c5d14/src/Servers/Kestrel/Core/src/TlsHandshakeCallbackContext.cs
Today the TlsHandshakeCallbackContext properties match ServerOptionsSelectionCallback, but we can add others that are quic specific like QuicConnection. It's ok if some of them like SslStream can't be populated.

If you need a new delegate to replace ServerOptionsSelectionCallback that's ok, so long as we can adapt TlsHandshakeCallbackContext over both of them.

[ManickaP]

Note that we have a working server certificate selection callback based on SNI (called for each new connection open). We're just lacking dynamic selection of all the other properties on SslServerAuthenticationOptions. Does that change anything from your perspective? If not, I'll proceed with the new callback API suggestion.

[Tratcher]

No, we need more than just the server cert. The client cert options are almost as important.

[JamesNK]

but it's doable.
@JamesNK @Tratcher considering we cannot reuse the existing delegate (due to SslStream) any thoughts on priority (how critical it is) and any other thoughts on the callback?

A way to reuse the existing delegate would be to make the SslStream parameter nullable, and simply not passing a value for HTTP/3.

@Tratcher Correct me if I'm wrong here but the most important parameter in this callback is the server name on SslClientHelloInfo. Have you seen SslStream used to build SslServerAuthenticationOptions?

If we make SslStream nullable and someone is using it then that will error if HTTP/3 starts happening on the endpoint. It would be good to understand if that is likely to impact anyone.

[Tratcher]

I don't think any of the SslStream properties are even populated at this stage, I don't know why it's passed to the callback.

Yes the ServerName is the most important. The sub protocols (ALPN) are useful. The other thing we get asked for is the local and remote IPs and ports. That's why we had to create our own callback and flow the ConnectionContext.

If we make SslStream nullable and someone is using it then that will error if HTTP/3 starts happening on the endpoint. It would be good to understand if that is likely to impact anyone.

That's how we're making some of the HTTP/3 scenarios work in 6, by invoking the callback ourselves with a null stream or connection context.

[ManickaP]

For a reference, the callback API issue #37933

• I think that the ClientHello object was intended to contain all the relevant properties and could be extended, now it only has SNI and SslProtocols.
• SslStream parameter is basically a sender, it would be nice to have it as an object as we do with many other callbacks. But that's certainly a breaking change and a new callback, nullability change is probably not.

[wfurt]

Conceptually, SslStream does not have visibility to transport since it depends on provided stream. I was thinking about ciphers or ALPN's but we would need some list/array and would need to allocate more. Since there was no specific ask I was not sure if it is worth of the trouble since the basic info seem sufficient. (so far)

Now, that callback may be fixable. ServerCertificateSelectionCallback for example has object sender and that is annoying since it is not clear what the object really is. And that was motivation for SslStream in ServerOptionsSelectionCallback so it is clear what you getting upfront without probing. We can probably create similar callback that gives you QuicConnection instead.

[ManickaP]

Closing as covered by #67560