[API Proposal]: [QUIC] QuicConnection

[ManickaP]

Background and motivation

API design for exposing QuicConnection and related classes to the public.

The API shape is based on the current internal shape of the class with the exception of merging the ConnectAsync into QuicProvider.CreateConnectionAsync.

Related issues:

• QUIC API: Consider passing QuicConnectionOptions to Quic.Connection.ConnectAsync #56009
  Consider passing QuicConnectionOptions to Quic.Connection.ConnectAsync: we don't need to hold them just for the meantime between ctor and ConnectAsync

• [QUIC] Rewrite Open(Uni|Bidi)rectionalStream to leverage async stream opening #67302
  Async stream open: perf need, coming from our benchmarks

• HTTP/3: Create additional connections when maximum active streams is reached #51775
  Multiple connections: same as H/2 requirement

• [API Proposal]: QuicConnection.CancelPendingAcceptStream #60818
  CancelPendingAcceptStream: used only for closing the connection, it's not as hot path as originally thought, I'd prefer not to do it now.

• [API Proposal]: Expose TLS details for QUIC connection #70184
  TLS details for QUIC connection

Discussed Considerations

1. StreamCount/OpenStream parametrize by stream type instead of sets of 2 methods
   --> YES
2. Create with endpoint parameters or put them into options?
   a) inside options in ConnectAsync fits better with Socket/SslStream
   --> YES, also keep them non-nullable and throw if not connected yet
   b) inside ctor allows us to have RemoteEndPoint non-nullable and better aligns with incoming connections (they have both side address, but are not "configured" with options yet)
   --> NO
3. Endpoints are IP endpoints, need to consider on which level we'll do DNS since MsQuic has only crude resolution
   • to able to do what Socket does, means that we need to create MsQuic connection object, try to connect it, let it fail, release it and then repeat for another IP
   • the resolution cannot be done in Create, it would need to be done in ConnectAsync
     --> Properties are IPEndPoint, provided is EndPoint that can also be DnsEndPoint
4. Options in ConnectAsync - we don't need them past the connection establishement moment so putting them into Create doesn't make much sense
   • we might consider collapsing Create + ConnectAsync (==> we might consider making QuicListener Create async as well)
   • we might keep Create parameter less, put everything to options and let ConnectAsync deal with it, this is not consistent with QuicListener though (does it matter or not?)
     --> YES, options in ConnectAsync, Create is ctor replacement
5. Multiple connections scenario: we'll need something like TryOpenStreamAsync + WaitForStreamAsync (not prototyped yet)

API Proposal

namespace System.Net.Quic;

public sealed class QuicConnection : IAsyncDisposable
{
    /// <summary>Returns true if QUIC is supported and can be used, e.g. msquic is present, high enough version of TLS is available etc.</summary>
    public static bool IsSupported { get; }

    /// <summary>Creates new, fully connected connection configured with the provided options.</summary>
    /// <exception cref="PlatformNotSupportedException">When <see cref="IsSupported" /> is <c>false</c>.</exception>
    public static ValueTask<QuicConnection> ConnectAsync(QuicConnectionOptions options, CancellationToken cancellationToken = default);

    /// <summary>Remote endpoint to which the connection is connected.</summary>
    public IPEndPoint RemoteEndPoint { get; }

    /// <summary>Local endpoint to which the connection is bound.</summary>
    public IPEndPoint LocalEndPoint { get; }

    /// <summary>Peer's certificate, available only if the peer provided the certificate.</summary>
    public X509Certificate2? RemoteCertificate { get; }

    /// <summary>Final, negotiated ALPN.</summary>
    public SslApplicationProtocol NegotiatedApplicationProtocol { get; }

    /// <summary>
    /// Create an outbound uni/bidirectional stream.
    /// </summary>
    public ValueTask<QuicStream> OpenOutboundStreamAsync(QuicStreamType type, CancellationToken cancellationToken = default);

    /// <summary>
    /// Accept an inbound stream.
    /// </summary>
    public ValueTask<QuicStream> AcceptInboundStreamAsync(CancellationToken cancellationToken = default);

    /// <summary>
    /// Close the connection and terminate any active streams.
    /// </summary>
    public ValueTask CloseAsync(long errorCode, CancellationToken cancellationToken = default);

    /// <summary>
    /// Silently closes the connection if not closed with CloseAsync beforehand.
    /// </summary>
    public void DisposeAsync();
}

/// <summary>Options for a new connection, the same options are used for incoming and outgoing connections.</summary>
public abstract class QuicConnectionOptions
{
    /// <summary>Prevent user sub-classing.</summary>
    internal QuicConnectionOptions()
    {}

    /// <summary>Limit on the number of bidirectional streams the remote peer connection can create on an open connection.</summary>
    public int MaxInboundBidirectionalStreams { get; set; }

    /// <summary>Limit on the number of unidirectional streams the remote peer connection can create on an open connection.</summary>
    public int MaxInboundUnidirectionalStreams { get; set; }

    /// <summary>Idle timeout for connections, after which the connection will be closed. Zero means using default of the underlying implementation.</summary>
    public TimeSpan IdleTimeout { get; set; } = TimeSpan.Zero;

    /// <summary>Error code used when the stream needs to abort read or write side of the stream internally.</summary>
    public required long DefaultStreamErrorCode { get; set; }
}

/// <summary>Options for a new connection, only used for outbound connections.</summary>
public sealed class QuicClientConnectionOptions : QuicConnectionOptions
{
    /// <summary>SSL options for the outgoing connection.</summary>
    public required SslClientAuthenticationOptions ClientAuthenticationOptions { get; set; }

    /// <summary>The endpoint to connect to.</summary>
    public required EndPoint RemoteEndPoint { get; set; }

    /// <summary>Optional local endpoint from which the connection is to be established.</summary>
    public IPEndPoint? LocalEndPoint { get; set; }
    
    public QuicClientConnectionOptions()
    {
        MaxInboundBidirectionalStreams = 0;
        MaxInboundUnidirectionalStreams = 0;
    }
}

/// <summary>Options for a new connection, only used for incoming connections.</summary>
public sealed class QuicServerConnectionOptions : QuicConnectionOptions
{
    /// <summary>SSL options for the incoming connection</summary>
    public required SslServerAuthenticationOptions ServerAuthenticationOptions { get; set; }
    
    public QuicServerConnectionOptions()
    {
        MaxInboundBidirectionalStreams = 100;
        MaxInboundUnidirectionalStreams = 10;
    }
}

API Usage

Client usage:

var options = new QuicClientConnectionOptions()
{
    RemoteEndPoint = new DnsEndPoint("localhost", 5001),
    DefaultStreamErrorCode = (long)Http3ErrorCode.RequestCancelled,
    ClientAuthenticationOptions = new SslClientAuthenticationOptions()
    {
        ApplicationProtocols = new List<SslApplicationProtocol>() { SslApplicationProtocol.Http3 },
    }
};
await using var connection = await QuicProvider.CreateConnectionAsync(options, cancellationToken);

await using var stream = await connection.OpenStreamAsync(StreamDirection.Bidirectional, cancellationToken);
// Work with stream, open more of them, send and receive data, close them ... https://github.com/dotnet/runtime/issues/69675

// Close will terminate all unclosed streams.
// If not called, the peer side of the connection will have to wait for idle connection timeout.
await connection.CloseAsync((long)Http3ErrorCode.NoError, cancellationToken);

// DisposeAsync called by await using.

Server usage:

// Consider listener from https://github.com/dotnet/runtime/issues/67560:
await using var connection = await listener.AcceptConnectionAsync(cancellationToken);
while (running)
{
    // In case the client closes the connection, Accept will throw appropriate exception.
    await using var stream = await connection.AcceptStreamAsync(cancellationToken);
    // Send and receive data... https://github.com/dotnet/runtime/issues/69675


    // DisposeAsync called by await using.
}
// Close will terminate all unclosed streams. Note that H/3 uses GO_AWAY to negotiate graceful connection shutdown with the client.
await connection.CloseAsync((long)Http3ErrorCode.NoError, cancellationToken);

// DisposeAsync called by await using.

Alternative Designs

• See Discussed Considerations
• As per [API Proposal]: Expose TLS details for QUIC connection #70184, TLS related properties could be combined into a common struct/class or left as individual properties on QuicConnection.

Risks

As I'll state with all QUIC APIs. We might consider making all of these PreviewFeature. Not to deter user from using it, but to give us flexibility to tune the API shape based on customer feedback.
We don't have many users now and we're mostly making these APIs based on what Kestrel needs, our limited experience with System.Net.Quic and my meager experiments with other QUIC implementations.

[ghost]

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

Issue Details

---

Background and motivation

API design for exposing QuicConnection and related classes to the public.

The API shape is based on the current internal shape of the class with the exception of postponing the connection configuration until the ConnectAsync is called (for outbound, inbound are already connected).

Related issues:

• QUIC API: Consider passing QuicConnectionOptions to Quic.Connection.ConnectAsync #56009
  Consider passing QuicConnectionOptions to Quic.Connection.ConnectAsync: we don't need to hold them just for the meantime between ctor and ConnectAsync

• [QUIC] Rewrite Open(Uni|Bidi)rectionalStream to leverage async stream opening #67302
  Async stream open: perf need, coming from our benchmarks

• HTTP/3: Create additional connections when maximum active streams is reached #51775
  Multiple connections: same as H/2 requirement

• [API Proposal]: QuicConnection.CancelPendingAcceptStream #60818
  CancelPendingAcceptStream: used only for closing the connection, it's not as hot path as originally thought, I'd prefer not to do it now.

Discussed Considerations

1. StreamCount/OpenStream parametrize by stream direction instead of sets of 2 methods
   --> YES
2. Create with endpoint parameters or put them into options?
   a) inside options in ConnectAsync fits better with Socket/SslStream
   --> YES, also keep them non-nullable and throw if not connected yet
   b) inside ctor allows us to have RemoteEndPoint non-nullable and better aligns with incoming connections (they have both side address, but are not "configured" with options yet)
   --> NO
3. Endpoints are IP endpoints, need to consider on which level we'll do DNS since MsQuic has only crude resolution
   • to able to do what Socket does, means that we need to create MsQuic connection object, try to connect it, let it fail, release it and then repeat for another IP
   • the resolution cannot be done in Create, it would need to be done in ConnectAsync
     --> Properties are IPEndPoint, provided is EndPoint that can also be DnsEndPoint
4. Options in ConnectAsync - we don't need them past the connection establishement moment so putting them into Create doesn't make much sense
   • we might consider collapsing Create + ConnectAsync (==> we might consider making QuicListener Create async as well)
   • we might keep Create parameter less, put everything to options and let ConnectAsync deal with it, this is not consistent with QuicListener though (does it matter or not?)
     --> YES, options in ConnectAsync, Create is ctor replacement
5. Multiple connections scenario: we'll need something like TryOpenStreamAsync + WaitForStreamAsync (not prototyped yet)

API Proposal

public sealed class QuicConnection : IAsyncDisposable
{
    public static QuicConnection Create();

    /// <summary>Indicates whether the QuicConnection is connected.</summary>
    public bool Connected { get; }

    /// <summary>Remote endpoint to which the connection try to get / is connected. Throws if Connected is false.</summary>
    public IPEndPoint RemoteEndPoint { get; }

    /// <summary>Local endpoint to which the connection will be / is bound. Throws if Connected is false.</summary>
    public IPEndPoint LocalEndPoint { get; }

    /// <summary>Peer's certificate, available only after the connection is fully connected (Connected is true) and the peer provided the certificate.</summary>
    public X509Certificate? RemoteCertificate { get; }

    /// <summary>Final, negotiated ALPN, available only after the connection is fully connected (Connected is true).</summary>
    public SslApplicationProtocol NegotiatedApplicationProtocol { get; }

    /// <summary>
    /// Connects to the remote endpoint.
    /// </summary>
    public ValueTask ConnectAsync(QuicClientConnectionOptions options, CancellationToken cancellationToken = default);

    /// <summary>
    /// Gets the maximum number of uni/bidirectional streams that can be made to the peer.
    /// </summary>
    public int GetAvailableStreamCount(StreamDirection direction);

    /// <summary>
    /// Create an outbound uni/bidirectional stream.
    /// </summary>
    public ValueTask<QuicStream> OpenStreamAsync(StreamDirection direction, CancellationToken cancellationToken = default);

    /// <summary>
    /// Create an outbound bidirectional stream.
    /// </summary>
    public ValueTask<QuicStream> OpenBidirectionalStreamAsync();

    /// <summary>
    /// Accept an incoming stream.
    /// </summary>
    public ValueTask<QuicStream> AcceptStreamAsync(CancellationToken cancellationToken = default);

    /// <summary>
    /// Close the connection and terminate any active streams.
    /// </summary>
    public ValueTask CloseAsync(long errorCode, CancellationToken cancellationToken = default);
}

public enum StreamDirection
{
    Unidirectional,
    Bidirectional
}

/// <summary>Options for a new connection, the same options are used for incoming and outgoing connections.</summary>
public class QuicConnectionOptions
{
    /// <summary>Limit on the number of bidirectional streams the remote peer connection can create on an open connection.</summary>
    public int MaxBidirectionalStreams { get; init; } = 100;

    /// <summary>Limit on the number of unidirectional streams the remote peer connection can create on an open connection.</summary>
    public int MaxUnidirectionalStreams { get; init; } = 100;

    /// <summary>Idle timeout for connections, after which the connection will be closed.</summary>
    public TimeSpan IdleTimeout { get; init; } = TimeSpan.FromMinutes(2);

    // This class will potentially expand with other connection options which we'll deem interesting for user to set.
}

/// <summary>Options for a new connection, only used for outgoing connections.</summary>
public class QuicClientConnectionOptions : QuicConnectionOptions
{
    /// <summary>SSL options for the outgoing connection.</summary>
    [Required]
    public SslClientAuthenticationOptions ClientAuthenticationOptions { get; init; }

    /// <summary>The endpoint to connect to.</summary>
    [Required]
    public EndPoint RemoteEndPoint { get; init; }

    /// <summary>Optional local endpoint from which the connection is to be established.</summary>
    public IPEndPoint? LocalEndPoint { get; init; }
}

API Usage

TODO

Alternative Designs

TODO

Risks

As I'll state with all QUIC APIs. We might consider making all of these PreviewFeature. Not to deter user from using it, but to give us flexibility to tune the API shape based on customer feedback.
We don't have many users now and we're mostly making these APIs based on what Kestrel needs, our limited experience with System.Net.Quic and my meager experiments with other QUIC implementations.

Author:	ManickaP
Assignees:	-
Labels:	api-suggestion, area-System.Net.Quic
Milestone:	-

[nibanks]

Some feedback:

• Instead of bool Connected why not have something like ConnectionState State?
• For RemoteEndPoint and LocalEndPoint ensure that you are prepared for there being multiple endpoints for each. Initially, there will only be one active/primary endpoint; but when multi-pah support is added, that won't be the case. Don't back yourself into a corner and force your API to break in the future.
• Why do you need to expose GetAvailableStreamCount at all? And if you really need to, I don't think it's important enough to be a top-level API.
• For CloseAsync this maps to the MsQuic API ConnectionShutdown I assume. We specifically chose the word "shutdown" to match the TCP socket API shutdown that most closely maps to the same concept. I still feel that's important and should be matched here.
• In the QuicConnectionOptions, why not expose everything already exposed from MsQuic?

cc @thhous-msft

[ManickaP]

* Instead of `bool Connected` why not have something like `ConnectionState State`?

We have the same for Socket: https://docs.microsoft.com/en-us/dotnet/api/system.net.sockets.socket.connected?view=net-6.0. On the other hand, e.g. WebSocket has state: https://docs.microsoft.com/en-us/dotnet/api/system.net.websockets.websocketstate?view=net-6.0.

* For `RemoteEndPoint` and `LocalEndPoint` ensure that you are prepared for there being **multiple** endpoints for each. Initially, there will only be one active/primary endpoint; but when multi-pah support is added, that won't be the case. Don't back yourself into a corner and force your API to break in the future.

I haven't know about this. I have to read the specs and we need to discuss this. Thanks for pointing this out!

* Why do you need to expose `GetAvailableStreamCount` at all? And if you really need to, I don't think it's important enough to be a top-level API.

We were using it for multiple connections support, but we don't need it now and it's an additive API, so, I'll remove it for now.

* For `CloseAsync` this maps to the MsQuic API `ConnectionShutdown` I assume. We specifically chose the word "shutdown" to match the TCP socket API `shutdown` that most closely maps to the same concept. I still feel that's important and should be matched here.

That's true, also Socket has shutdown: https://docs.microsoft.com/en-us/dotnet/api/system.net.sockets.socket.shutdown?view=net-6.0, SslStream as well: https://docs.microsoft.com/en-us/dotnet/api/system.net.security.sslstream.shutdownasync?view=net-6.0#system-net-security-sslstream-shutdownasync. On the other hand, we have a precedent for CloseAsync in web sockets.

* In the `QuicConnectionOptions`, why not expose everything already exposed from MsQuic?

We prefer to keep APIs minimal and add incrementally only when necessary (to reduce amount of code, to prevent mistakes in APIs, etc.). Also we don't want to lock us down only with msquic implementation.

All of these are great comments and I'll discuss them with our team, thanks Nick.

[terrajobst]

Video

• QuicConnection.RemoteCertificate
  • Should be X509Certificate2; nothing in our libraries should ever allocate X509Certificate.
• QuicConnection.OpenStreamAsync
  • Should probably be OpenOutboundStream; otherwise OpenStreamAsync and AcceptStream seem too similar
  • We may want to consider renaming AcceptStreamAsync to AcceptInboundStreamAsync
• Note: we haven't reviewed the other types yet

namespace System.Net.Quic;

public sealed class QuicConnection : IAsyncDisposable
{
    public static bool IsSupported { get; }
    public static ValueTask<QuicConnection> ConnectAsync(QuicConnectionOptions options,
                                                         CancellationToken cancellationToken = default);

    public IPEndPoint RemoteEndPoint { get; }
    public IPEndPoint LocalEndPoint { get; }
    public X509Certificate2? RemoteCertificate { get; }
    public SslApplicationProtocol NegotiatedApplicationProtocol { get; }
    public ValueTask<QuicStream> OpenStreamAsync(QuicStreamType type, CancellationToken cancellationToken = default);
    public ValueTask<QuicStream> AcceptStreamAsync(CancellationToken cancellationToken = default);
    public ValueTask CloseAsync(long errorCode, CancellationToken cancellationToken = default);
    public ValueTask DisposeAsync();
}

[terrajobst]

Video

• QuicConnection
  • What's the lifetime expectation of CloseAsync vs DisposeAsync?
• QuicConnectionOptions
  • Should we have a default values for MaxBidirectionalStreams and MaxUnidirectionalStreams. Consider renaming those to make it clear that this is about accepted/inbound streams, never about outbound.
  • You may want to move the initialization to the derived classes to allow for different defaults for client/server, e.g. 0/0 for client and 100/10 for server.
  • IdleTimeout can we expose the underlying timeout? This way we don't expose our own policy. If we can't, we can also make it nullable and just say "platform specific timeout".
  • Depending on where QuicConnection's CloseAsync/DisposeAsync lands, you may want to add DefaultConnectionErrorCode
• This is going to either use constructors to enforce configuring required values or the new C# required modifier

namespace System.Net.Quic;

public abstract class QuicConnectionOptions
{
    public int MaxBidirectionalStreams { get; set; } = 100;
    public int MaxUnidirectionalStreams { get; set; } = 100;
    public TimeSpan IdleTimeout { get; set; } = TimeSpan.FromMinutes(2);
    public long DefaultStreamErrorCode { get; set; }
    public long DefaultConnectionErrorCode { get; set; }
    public Action<QuicConnection>? EndPointChanged { get; set; }
}
public sealed class QuicClientConnectionOptions : QuicConnectionOptions
{
    public QuicClientConnectionOptions();
    public SslClientAuthenticationOptions ClientAuthenticationOptions { get; set; }
    public EndPoint RemoteEndPoint { get; set; }
    public IPEndPoint? LocalEndPoint { get; set; }
}
public sealed class QuicServerConnectionOptions : QuicConnectionOptions
{
    public QuicServerConnectionOptions();
    public SslServerAuthenticationOptions ServerAuthenticationOptions { get; set; }
}

[ManickaP]

API Review Comments

QuicConnection.RemoteCertificate

• Should be X509Certificate2; nothing in our libraries should ever allocate X509Certificate.
  • Changed in the proposal to X509Certificate2

QuicConnection.OpenStreamAsync

• Should probably be OpenOutboundStream; otherwise OpenStreamAsync and AcceptStream seem too similar
  • Changed in the proposal to OpenOutboundStreamAsync
• We may want to consider renaming AcceptStreamAsync to AcceptInboundStreamAsync
  • Changed in the proposal to AcceptInboundStreamAsync

QuicConnection

• What's the lifetime expectation of CloseAsync vs DisposeAsync?
  • One option is to leave it as is. Not calling CloseAsync would make DisposeAsync close the connection silently, i.e. not informing the peer and letting peer's connection idle out. As a result, await using var connection ... is not enough to gracefully close the connection. However, this is rather advanced API, so documenting this behavior might be enough.
  • Another option is to add DefaultConnectionCloseCode to QuicConnectionOptions which would be used in cases when CloseAsync wasn't called before DisposeAsync. However, that prevents non-graceful closure of the connection in all cases.
  • Leaning towards the first option at the moment, not changing the proposal.

QuicConnectionOptions

• Should we have a default values for MaxBidirectionalStreams and MaxUnidirectionalStreams. Consider renaming those to make it clear that this is about accepted/inbound streams, never about outbound.
  • Changed in the proposal to MaxInboundBidirectionalStreams and MaxInboundUnidirectionalStreams
• You may want to move the initialization to the derived classes to allow for different defaults for client/server, e.g. 0/0 for client and 100/10 for server.
  • Changed in the proposal to 0/0 for client and 100/10 for server
• IdleTimeout can we expose the underlying timeout? This way we don't expose our own policy. If we can't, we can also make it nullable and just say "platform specific timeout".
  • Changed in the proposal to TimeSpan.Zero meaning the underlying implementation default. The same pattern we used for ListenBacklog in [API Proposal]: [QUIC] QuicListener #67560
• Depending on where QuicConnection's CloseAsync/DisposeAsync lands, you may want to add DefaultConnectionErrorCode
  • Answered above.
• This is going to either use constructors to enforce configuring required values or the new C# required modifier
  • Changed in the proposal to use required for mandatory properties. Properties that cannot be enforced by this, e.g. non-empty collections, SslAuthenticationOptions properties, etc., will be validated in ConnectAsync/ListenAsync.

[terrajobst]

Video

• Maybe we should replace required with regular constructors or just forgo all required checking and move it to QuicConnection

namespace System.Net.Quic;

public sealed class QuicConnection : IAsyncDisposable
{
    public static bool IsSupported { get; }
    public IPEndPoint RemoteEndPoint { get; }
    public IPEndPoint LocalEndPoint { get; }
    public X509Certificate2? RemoteCertificate { get; }
    public SslApplicationProtocol NegotiatedApplicationProtocol { get; }
    public static ValueTask<QuicConnection> ConnectAsync(QuicConnectionOptions options, CancellationToken cancellationToken = default);
    public ValueTask<QuicStream> OpenOutboundStreamAsync(QuicStreamType type, CancellationToken cancellationToken = default);
    public ValueTask<QuicStream> AcceptInboundStreamAsync(CancellationToken cancellationToken = default);
    public ValueTask CloseAsync(long errorCode, CancellationToken cancellationToken = default);
    public void DisposeAsync();
}
public abstract class QuicConnectionOptions
{
    public int MaxInboundBidirectionalStreams { get; set; }
    public int MaxInboundUnidirectionalStreams { get; set; }
    public TimeSpan IdleTimeout { get; set; } = TimeSpan.Zero;
    public long DefaultStreamErrorCode { get; set; }
    public long DefaultCloseErrorCode { get; set; }
}
public sealed class QuicClientConnectionOptions : QuicConnectionOptions
{
    public QuicClientConnectionOptions();
    public SslClientAuthenticationOptions ClientAuthenticationOptions { get; set; }
    public EndPoint RemoteEndPoint { get; set; }
    public IPEndPoint? LocalEndPoint { get; set; }
}
public sealed class QuicServerConnectionOptions : QuicConnectionOptions
{
    public QuicServerConnectionOptions();
    public SslServerAuthenticationOptions ServerAuthenticationOptions { get; set; }
}