Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,17 @@ internal static Exception CreateWrappedException(Exception exception, string hos

static HttpRequestError DeduceError(Exception exception)
{
// TODO: Deduce quic errors from QuicException.TransportErrorCode once https://github.com/dotnet/runtime/issues/87262 is implemented.
if (exception is AuthenticationException)
{
return HttpRequestError.SecureConnectionError;
}

if (exception is SocketException socketException && socketException.SocketErrorCode == SocketError.HostNotFound)
// Resolving a non-existent hostname often leads to EAI_AGAIN/TryAgain on Linux, indicating a non-authoritative failure, eg. timeout.
// Getting EAGAIN/TryAgain from a TCP connect() is not possible on Windows or Mac according to the docs and indicates lack of kernel resources on Linux,
// which should be a very rare error in practice. As a result, mapping SocketError.TryAgain to HttpRequestError.NameResolutionError
// leads to a more reliable distinction between NameResolutionError and ConnectionError.
if (exception is SocketException socketException &&
socketException.SocketErrorCode is SocketError.HostNotFound or SocketError.TryAgain)
Comment on lines +155 to +156
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you wanted to, since you're already using pattern matching, you could make this even more concise:

exception is SocketException { SocketErrorCode: SocketError.HostNotFound or SocketError.TryAgain }

{
return HttpRequestError.NameResolutionError;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4376,10 +4376,8 @@ public async Task NameResolutionError()
};

HttpRequestException ex = await Assert.ThrowsAsync<HttpRequestException>(() => client.SendAsync(message));

// TODO: Some platforms fail to detect NameResolutionError reliably, we should investigate this.
// Also, System.Net.Quic does not report DNS resolution errors yet.
Assert.True(ex.HttpRequestError is HttpRequestError.NameResolutionError or HttpRequestError.ConnectionError);
Assert.Equal(HttpRequestError.NameResolutionError, ex.HttpRequestError);
Assert.IsType<SocketException>(ex.InnerException);
}

[Fact]
Expand Down
47 changes: 15 additions & 32 deletions src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -258,48 +258,31 @@ private async ValueTask FinishConnectAsync(QuicClientConnectionOptions options,
{
throw new ArgumentException(SR.Format(SR.net_quic_unsupported_endpoint_type, options.RemoteEndPoint.GetType()), nameof(options));
}
int addressFamily = QUIC_ADDRESS_FAMILY_UNSPEC;

// RemoteEndPoint is either IPEndPoint or DnsEndPoint containing IPAddress string.
// --> Set the IP directly, no name resolution needed.
if (address is not null)
if (address is null)
{
QuicAddr quicAddress = new IPEndPoint(address, port).ToQuicAddr();
MsQuicHelpers.SetMsQuicParameter(_handle, QUIC_PARAM_CONN_REMOTE_ADDRESS, quicAddress);
}
// RemoteEndPoint is DnsEndPoint containing hostname that is different from requested SNI.
// --> Resolve the hostname and set the IP directly, use requested SNI in ConnectionStart.
else if (host is not null &&
!host.Equals(options.ClientAuthenticationOptions.TargetHost, StringComparison.OrdinalIgnoreCase))
{
IPAddress[] addresses = await Dns.GetHostAddressesAsync(host!, cancellationToken).ConfigureAwait(false);
Debug.Assert(host is not null);

// Given just a ServerName to connect to, msquic would also use the first address after the resolution
// (https://github.com/microsoft/msquic/issues/1181) and it would not return a well-known error code
// for resolution failures we could rely on. By doing the resolution in managed code, we can guarantee
// that a SocketException will surface to the user if the name resolution fails.
IPAddress[] addresses = await Dns.GetHostAddressesAsync(host, cancellationToken).ConfigureAwait(false);
cancellationToken.ThrowIfCancellationRequested();
if (addresses.Length == 0)
{
throw new SocketException((int)SocketError.HostNotFound);
}

QuicAddr quicAddress = new IPEndPoint(addresses[0], port).ToQuicAddr();
MsQuicHelpers.SetMsQuicParameter(_handle, QUIC_PARAM_CONN_REMOTE_ADDRESS, quicAddress);
}
// RemoteEndPoint is DnsEndPoint containing hostname that is the same as the requested SNI.
// --> Let MsQuic resolve the hostname/SNI, give address family hint is specified in DnsEndPoint.
else
{
if (options.RemoteEndPoint.AddressFamily == AddressFamily.InterNetwork)
{
addressFamily = QUIC_ADDRESS_FAMILY_INET;
}
if (options.RemoteEndPoint.AddressFamily == AddressFamily.InterNetworkV6)
{
addressFamily = QUIC_ADDRESS_FAMILY_INET6;
}
address = addresses[0];
}

QuicAddr remoteQuicAddress = new IPEndPoint(address, port).ToQuicAddr();
MsQuicHelpers.SetMsQuicParameter(_handle, QUIC_PARAM_CONN_REMOTE_ADDRESS, remoteQuicAddress);

if (options.LocalEndPoint is not null)
{
QuicAddr quicAddress = options.LocalEndPoint.ToQuicAddr();
MsQuicHelpers.SetMsQuicParameter(_handle, QUIC_PARAM_CONN_LOCAL_ADDRESS, quicAddress);
QuicAddr localQuicAddress = options.LocalEndPoint.ToQuicAddr();
MsQuicHelpers.SetMsQuicParameter(_handle, QUIC_PARAM_CONN_LOCAL_ADDRESS, localQuicAddress);
}

// RFC 6066 forbids IP literals
Expand All @@ -326,7 +309,7 @@ private async ValueTask FinishConnectAsync(QuicClientConnectionOptions options,
ThrowHelper.ThrowIfMsQuicError(MsQuicApi.Api.ConnectionStart(
_handle,
_configuration,
(ushort)addressFamily,
(ushort)remoteQuicAddress.Family,
(sbyte*)targetHostPtr,
(ushort)port),
"ConnectionStart failed");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Net.Sockets;
using System.Security.Cryptography.X509Certificates;
using System.Threading;
Expand All @@ -18,10 +20,12 @@ public sealed class QuicConnectionTests : QuicTestBase

public QuicConnectionTests(ITestOutputHelper output) : base(output) { }

[Fact]
public async Task TestConnect()
[Theory]
[InlineData(false)]
[InlineData(true)]
public async Task TestConnect(bool ipv6)
{
await using QuicListener listener = await CreateQuicListener();
await using QuicListener listener = await CreateQuicListener(ipv6 ? IPAddress.IPv6Loopback : IPAddress.Loopback);

var options = CreateQuicClientOptions(listener.LocalEndPoint);
ValueTask<QuicConnection> connectTask = CreateQuicConnection(options);
Expand All @@ -31,15 +35,27 @@ public async Task TestConnect()
await using QuicConnection serverConnection = acceptTask.Result;
await using QuicConnection clientConnection = connectTask.Result;

Assert.Equal(listener.LocalEndPoint, serverConnection.LocalEndPoint);
Assert.Equal(listener.LocalEndPoint, clientConnection.RemoteEndPoint);
Assert.Equal(clientConnection.LocalEndPoint, serverConnection.RemoteEndPoint);
IgnoreScopeIdIPEndpointComparer endPointComparer = new();
Assert.Equal(listener.LocalEndPoint, serverConnection.LocalEndPoint, endPointComparer);
Assert.Equal(listener.LocalEndPoint, clientConnection.RemoteEndPoint, endPointComparer);
Assert.Equal(clientConnection.LocalEndPoint, serverConnection.RemoteEndPoint, endPointComparer);
Comment on lines +38 to +41
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To improve confidence in my changes, I extended this test to test IPv6. serverConnection.LocalEndPoint has a ScopeId for some reason while listener.LocalEndpoint doesn't. I assume it is asigned after binding, and we can ignore this difference.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scopeID should be only relevant for LLA. I'm wondering if we are deviating from Socket/NetworkStream here because the underlying MSQuic implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible. If we think it's a problem we can track it in a separate issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this was existing condition. We miss it because QuicConnectionTests.TestConnect only tests IPv4. I sync back and I tried it with IPv6 and it also fails:

   Starting:    System.Net.Quic.Functional.Tests (parallel test collections = on, max threads = 4)
      System.Net.Quic.Tests.QuicConnectionTests.TestConnect [STARTING]
      System.Net.Quic.Tests.QuicConnectionTests.TestConnect [FAIL]
        Assert.Equal() Failure
        Expected: [::1]:34850
        Actual:   [::1%1]:34850

I will fix it tomorrow.
(cc: @ManickaP)

Assert.Equal(ApplicationProtocol.ToString(), clientConnection.NegotiatedApplicationProtocol.ToString());
Assert.Equal(ApplicationProtocol.ToString(), serverConnection.NegotiatedApplicationProtocol.ToString());
Assert.Equal(options.ClientAuthenticationOptions.TargetHost, clientConnection.TargetHostName);
Assert.Equal(options.ClientAuthenticationOptions.TargetHost, serverConnection.TargetHostName);
}

private class IgnoreScopeIdIPEndpointComparer : IEqualityComparer<IPEndPoint>
{
public bool Equals(IPEndPoint x, IPEndPoint y)
{
byte[] xBytes = x.Address.GetAddressBytes();
byte[] yBytes = y.Address.GetAddressBytes();
return xBytes.AsSpan().SequenceEqual(yBytes) && x.Port == y.Port;
}
public int GetHashCode([DisallowNull] IPEndPoint obj) => obj.Port;
}

private static async Task<QuicStream> OpenAndUseStreamAsync(QuicConnection c)
{
QuicStream s = await c.OpenOutboundStreamAsync(QuicStreamType.Bidirectional);
Expand Down Expand Up @@ -369,17 +385,18 @@ await Task.WhenAll(
}).WaitAsync(TimeSpan.FromSeconds(5)));
}

[Fact]
[OuterLoop("Uses external servers")]
public async Task ConnectAsync_InvalidName_ThrowsSocketException()
[Theory]
[InlineData(false)]
[InlineData(true)]
public async Task ConnectAsync_InvalidName_ThrowsSocketException(bool sameTargetHost)
{
string name = $"{Guid.NewGuid().ToString("N")}.microsoft.com.";
var options = new QuicClientConnectionOptions()
{
DefaultStreamErrorCode = DefaultStreamErrorCodeClient,
DefaultCloseErrorCode = DefaultCloseErrorCodeClient,
RemoteEndPoint = new DnsEndPoint(name, 10000),
ClientAuthenticationOptions = GetSslClientAuthenticationOptions()
ClientAuthenticationOptions = GetSslClientAuthenticationOptions(sameTargetHost ? name : "localhost")
};

SocketException ex = await Assert.ThrowsAsync<SocketException>(() => QuicConnection.ConnectAsync(options).AsTask());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,13 @@ public SslServerAuthenticationOptions GetSslServerAuthenticationOptions()
};
}

public SslClientAuthenticationOptions GetSslClientAuthenticationOptions()
public SslClientAuthenticationOptions GetSslClientAuthenticationOptions(string targetHost = "localhost")
{
return new SslClientAuthenticationOptions()
{
ApplicationProtocols = new List<SslApplicationProtocol>() { ApplicationProtocol },
RemoteCertificateValidationCallback = RemoteCertificateValidationCallback,
TargetHost = "localhost"
TargetHost = targetHost
};
}

Expand All @@ -140,19 +140,20 @@ internal ValueTask<QuicConnection> CreateQuicConnection(QuicClientConnectionOpti
return QuicConnection.ConnectAsync(clientOptions);
}

internal QuicListenerOptions CreateQuicListenerOptions()
internal QuicListenerOptions CreateQuicListenerOptions(IPAddress address = null)
{
address ??= IPAddress.Loopback;
return new QuicListenerOptions()
{
ListenEndPoint = new IPEndPoint(IPAddress.Loopback, 0),
ListenEndPoint = new IPEndPoint(address, 0),
ApplicationProtocols = new List<SslApplicationProtocol>() { ApplicationProtocol },
ConnectionOptionsCallback = (_, _, _) => ValueTask.FromResult(CreateQuicServerOptions())
};
}

internal ValueTask<QuicListener> CreateQuicListener(int MaxInboundUnidirectionalStreams = 100, int MaxInboundBidirectionalStreams = 100)
internal ValueTask<QuicListener> CreateQuicListener(IPAddress address = null)
{
var options = CreateQuicListenerOptions();
var options = CreateQuicListenerOptions(address);
return CreateQuicListener(options);
}

Expand Down