From 4f7165308933deb4655878304f77af6ede11e235 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 3 Sep 2022 11:52:46 +0800 Subject: [PATCH 1/6] Fix auth exception stopping connection listener --- .../src/Internal/QuicConnectionListener.cs | 48 ++++++++++++------- .../Transport.Quic/src/Internal/QuicLog.cs | 3 ++ .../test/QuicConnectionListenerTests.cs | 43 ++++++++++++++++- .../Transport.Quic/test/QuicTestHelpers.cs | 12 +++-- 4 files changed, 83 insertions(+), 23 deletions(-) diff --git a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs index 68eb810f13c7..b491f0304c84 100644 --- a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs +++ b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs @@ -6,6 +6,7 @@ using System.Net.Quic; using System.Net.Security; using System.Runtime.CompilerServices; +using System.Security.Authentication; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Http.Features; using Microsoft.Extensions.Logging; @@ -145,31 +146,42 @@ public async ValueTask CreateListenerAsync() throw new InvalidOperationException($"The listener needs to be initialized by calling {nameof(CreateListenerAsync)}."); } - try + while (!cancellationToken.IsCancellationRequested) { - var quicConnection = await _listener.AcceptConnectionAsync(cancellationToken); + try + { + var quicConnection = await _listener.AcceptConnectionAsync(cancellationToken); + + if (!_pendingConnections.TryGetValue(quicConnection, out var connectionContext)) + { + throw new InvalidOperationException("Couldn't find ConnectionContext for QuicConnection."); + } + else + { + _pendingConnections.Remove(quicConnection); + } + + // Verify the connection context was created and set correctly. + Debug.Assert(connectionContext != null); + Debug.Assert(connectionContext.GetInnerConnection() == quicConnection); - if (!_pendingConnections.TryGetValue(quicConnection, out var connectionContext)) + QuicLog.AcceptedConnection(_log, connectionContext); + + return connectionContext; + } + catch (QuicException ex) when (ex.QuicError == QuicError.OperationAborted) { - throw new InvalidOperationException("Couldn't find ConnectionContext for QuicConnection."); + QuicLog.ConnectionListenerAborted(_log, ex); + return null; } - else + catch (AuthenticationException ex) { - _pendingConnections.Remove(quicConnection); + // If the client rejects the connection because of an invalid cert then AcceptAsync throws. + // This is a recoverable error and we don't want to stop accepting connections because of this. + QuicLog.ConnectionListenerAcceptConnectionFailed(_log, ex); } - - // Verify the connection context was created and set correctly. - Debug.Assert(connectionContext != null); - Debug.Assert(connectionContext.GetInnerConnection() == quicConnection); - - QuicLog.AcceptedConnection(_log, connectionContext); - - return connectionContext; - } - catch (QuicException ex) when (ex.QuicError == QuicError.OperationAborted) - { - QuicLog.ConnectionListenerAborted(_log, ex); } + return null; } diff --git a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicLog.cs b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicLog.cs index d58061344f24..1aad5a655390 100644 --- a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicLog.cs +++ b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicLog.cs @@ -232,6 +232,9 @@ public static void StreamTimeoutWrite(ILogger logger, QuicStreamContext streamCo } } + [LoggerMessage(24, LogLevel.Debug, "QUIC listener accept connection failed.", EventName = "ConnectionListenerAcceptConnectionFailed")] + public static partial void ConnectionListenerAcceptConnectionFailed(ILogger logger, Exception exception); + private static StreamType GetStreamType(QuicStreamContext streamContext) => streamContext.CanRead && streamContext.CanWrite ? StreamType.Bidirectional diff --git a/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs b/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs index 4416e80c470f..c00c104dd02c 100644 --- a/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs +++ b/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs @@ -13,6 +13,7 @@ using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.Testing; +using Microsoft.Extensions.Logging; using Xunit; namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Quic.Tests; @@ -51,7 +52,7 @@ public async Task AcceptAsync_ClientCreatesConnection_ServerAccepts() await using var clientConnection = await QuicConnection.ConnectAsync(options); // Assert - await using var serverConnection = await acceptTask.DefaultTimeout(); + var serverConnection = await acceptTask.DefaultTimeout(); Assert.False(serverConnection.ConnectionClosed.IsCancellationRequested); await serverConnection.DisposeAsync().AsTask().DefaultTimeout(); @@ -60,6 +61,46 @@ public async Task AcceptAsync_ClientCreatesConnection_ServerAccepts() Assert.False(serverConnection.ConnectionClosed.IsCancellationRequested); } + [ConditionalFact] + [MsQuicSupported] + public async Task AcceptAsync_ClientCreatesInvalidConnection_ServerContinuesToAccept() + { + await using var connectionListener = await QuicTestHelpers.CreateConnectionListenerFactory(LoggerFactory); + + // Act & Assert 1 + Logger.LogInformation("Client creating successful connection 1"); + var acceptTask1 = connectionListener.AcceptAndAddFeatureAsync().DefaultTimeout(); + await using var clientConnection1 = await QuicConnection.ConnectAsync( + QuicTestHelpers.CreateClientConnectionOptions(connectionListener.EndPoint)); + + var serverConnection1 = await acceptTask1.DefaultTimeout(); + Assert.False(serverConnection1.ConnectionClosed.IsCancellationRequested); + await serverConnection1.DisposeAsync().AsTask().DefaultTimeout(); + + // Act & Assert 2 + var serverFailureLogTask = WaitForLogMessage(m => m.EventId.Name == "ConnectionListenerAcceptConnectionFailed"); + + Logger.LogInformation("Client creating unsuccessful connection 2"); + var acceptTask2 = connectionListener.AcceptAndAddFeatureAsync().DefaultTimeout(); + var ex = await ExceptionAssert.ThrowsAsync(async () => + { + await QuicConnection.ConnectAsync( + QuicTestHelpers.CreateClientConnectionOptions(connectionListener.EndPoint, ignoreInvalidCertificate: false)); + }, "The remote certificate is invalid because of errors in the certificate chain: RemoteCertificateChainErrors"); + + Assert.False(acceptTask2.IsCompleted, "Accept doesn't return for failed client connection."); + await serverFailureLogTask.DefaultTimeout(); + + // Act & Assert 3 + Logger.LogInformation("Client creating successful connection 3"); + await using var clientConnection2 = await QuicConnection.ConnectAsync( + QuicTestHelpers.CreateClientConnectionOptions(connectionListener.EndPoint)); + + var serverConnection2 = await acceptTask2.DefaultTimeout(); + Assert.False(serverConnection2.ConnectionClosed.IsCancellationRequested); + await serverConnection2.DisposeAsync().AsTask().DefaultTimeout(); + } + [ConditionalFact] [MsQuicSupported] [OSSkipCondition(OperatingSystems.Linux | OperatingSystems.MacOSX)] diff --git a/src/Servers/Kestrel/Transport.Quic/test/QuicTestHelpers.cs b/src/Servers/Kestrel/Transport.Quic/test/QuicTestHelpers.cs index 3b26da87cb05..4fa223873d5a 100644 --- a/src/Servers/Kestrel/Transport.Quic/test/QuicTestHelpers.cs +++ b/src/Servers/Kestrel/Transport.Quic/test/QuicTestHelpers.cs @@ -116,9 +116,9 @@ private static bool RemoteCertificateValidationCallback(object sender, X509Certi return true; } - public static QuicClientConnectionOptions CreateClientConnectionOptions(EndPoint remoteEndPoint) + public static QuicClientConnectionOptions CreateClientConnectionOptions(EndPoint remoteEndPoint, bool? ignoreInvalidCertificate = null) { - return new QuicClientConnectionOptions + var options = new QuicClientConnectionOptions { MaxInboundBidirectionalStreams = 200, MaxInboundUnidirectionalStreams = 200, @@ -128,12 +128,16 @@ public static QuicClientConnectionOptions CreateClientConnectionOptions(EndPoint ApplicationProtocols = new List { SslApplicationProtocol.Http3 - }, - RemoteCertificateValidationCallback = RemoteCertificateValidationCallback + } }, DefaultStreamErrorCode = 0, DefaultCloseErrorCode = 0, }; + if (ignoreInvalidCertificate ?? true) + { + options.ClientAuthenticationOptions.RemoteCertificateValidationCallback = RemoteCertificateValidationCallback; + } + return options; } public static async Task CreateAndCompleteBidirectionalStreamGracefully(QuicConnection clientConnection, MultiplexedConnectionContext serverConnection, ILogger logger) From 01c3b2038d01ee5ac20e4d49fed22efa9e409274 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 3 Sep 2022 13:12:47 +0800 Subject: [PATCH 2/6] Update --- .../Transport.Quic/src/Internal/QuicConnectionListener.cs | 2 +- src/Servers/Kestrel/Transport.Quic/src/Internal/QuicLog.cs | 4 ++-- .../Transport.Quic/test/QuicConnectionListenerTests.cs | 5 +++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs index b491f0304c84..7bf29af98ad5 100644 --- a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs +++ b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs @@ -178,7 +178,7 @@ public async ValueTask CreateListenerAsync() { // If the client rejects the connection because of an invalid cert then AcceptAsync throws. // This is a recoverable error and we don't want to stop accepting connections because of this. - QuicLog.ConnectionListenerAcceptConnectionFailed(_log, ex); + QuicLog.ConnectionListenerHandshakeFailed(_log, ex); } } diff --git a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicLog.cs b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicLog.cs index 1aad5a655390..64572a427253 100644 --- a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicLog.cs +++ b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicLog.cs @@ -232,8 +232,8 @@ public static void StreamTimeoutWrite(ILogger logger, QuicStreamContext streamCo } } - [LoggerMessage(24, LogLevel.Debug, "QUIC listener accept connection failed.", EventName = "ConnectionListenerAcceptConnectionFailed")] - public static partial void ConnectionListenerAcceptConnectionFailed(ILogger logger, Exception exception); + [LoggerMessage(24, LogLevel.Debug, "QUIC listener connection handshake failed.", EventName = "ConnectionListenerHandshakeFailed")] + public static partial void ConnectionListenerHandshakeFailed(ILogger logger, Exception exception); private static StreamType GetStreamType(QuicStreamContext streamContext) => streamContext.CanRead && streamContext.CanWrite diff --git a/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs b/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs index c00c104dd02c..e95b7daff7a4 100644 --- a/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs +++ b/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs @@ -78,7 +78,7 @@ public async Task AcceptAsync_ClientCreatesInvalidConnection_ServerContinuesToAc await serverConnection1.DisposeAsync().AsTask().DefaultTimeout(); // Act & Assert 2 - var serverFailureLogTask = WaitForLogMessage(m => m.EventId.Name == "ConnectionListenerAcceptConnectionFailed"); + var serverFailureLogTask = WaitForLogMessage(m => m.EventId.Name == "ConnectionListenerHandshakeFailed"); Logger.LogInformation("Client creating unsuccessful connection 2"); var acceptTask2 = connectionListener.AcceptAndAddFeatureAsync().DefaultTimeout(); @@ -89,7 +89,8 @@ await QuicConnection.ConnectAsync( }, "The remote certificate is invalid because of errors in the certificate chain: RemoteCertificateChainErrors"); Assert.False(acceptTask2.IsCompleted, "Accept doesn't return for failed client connection."); - await serverFailureLogTask.DefaultTimeout(); + var serverFailureLog = await serverFailureLogTask.DefaultTimeout(); + Assert.NotNull(serverFailureLog.Exception); // Act & Assert 3 Logger.LogInformation("Client creating successful connection 3"); From cf45afaeaa4c2e6362d962f4cf72bb2b02678145 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Mon, 5 Sep 2022 20:31:25 +0800 Subject: [PATCH 3/6] Update --- .../src/Internal/QuicConnectionListener.cs | 16 +++- .../Transport.Quic/src/Internal/QuicLog.cs | 4 +- .../test/QuicConnectionListenerTests.cs | 89 ++++++++++++++++++- 3 files changed, 99 insertions(+), 10 deletions(-) diff --git a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs index 7bf29af98ad5..9fcffa80da25 100644 --- a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs +++ b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs @@ -171,14 +171,22 @@ public async ValueTask CreateListenerAsync() } catch (QuicException ex) when (ex.QuicError == QuicError.OperationAborted) { + // OperationAborted is reported when an accept is in-progress and the listener is unbind/disposed. QuicLog.ConnectionListenerAborted(_log, ex); return null; } - catch (AuthenticationException ex) + catch (ObjectDisposedException ex) { - // If the client rejects the connection because of an invalid cert then AcceptAsync throws. - // This is a recoverable error and we don't want to stop accepting connections because of this. - QuicLog.ConnectionListenerHandshakeFailed(_log, ex); + // ObjectDisposedException is reported when an accept is started after the listener is unbind/disposed. + QuicLog.ConnectionListenerAborted(_log, ex); + return null; + } + catch (Exception ex) + { + // If the client rejects the connection because of an invalid cert then AcceptConnectionAsync throws. + // An error thrown inside ConnectionOptionsCallback can also throw from AcceptConnectionAsync. + // These are recoverable errors and we don't want to stop accepting connections. + QuicLog.ConnectionListenerConnectionFailed(_log, ex); } } diff --git a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicLog.cs b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicLog.cs index 64572a427253..8319d34464f9 100644 --- a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicLog.cs +++ b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicLog.cs @@ -232,8 +232,8 @@ public static void StreamTimeoutWrite(ILogger logger, QuicStreamContext streamCo } } - [LoggerMessage(24, LogLevel.Debug, "QUIC listener connection handshake failed.", EventName = "ConnectionListenerHandshakeFailed")] - public static partial void ConnectionListenerHandshakeFailed(ILogger logger, Exception exception); + [LoggerMessage(24, LogLevel.Debug, "QUIC listener connection failed.", EventName = "ConnectionListenerConnectionFailed")] + public static partial void ConnectionListenerConnectionFailed(ILogger logger, Exception exception); private static StreamType GetStreamType(QuicStreamContext streamContext) => streamContext.CanRead && streamContext.CanWrite diff --git a/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs b/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs index e95b7daff7a4..bd44cad85cd2 100644 --- a/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs +++ b/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs @@ -25,7 +25,7 @@ public class QuicConnectionListenerTests : TestApplicationErrorLoggerLoggedTest [ConditionalFact] [MsQuicSupported] - public async Task AcceptAsync_AfterUnbind_Error() + public async Task AcceptAsync_AfterUnbind_ReturnNull() { // Arrange await using var connectionListener = await QuicTestHelpers.CreateConnectionListenerFactory(LoggerFactory); @@ -34,7 +34,7 @@ public async Task AcceptAsync_AfterUnbind_Error() await connectionListener.UnbindAsync().DefaultTimeout(); // Assert - await Assert.ThrowsAsync(() => connectionListener.AcceptAndAddFeatureAsync().AsTask()).DefaultTimeout(); + Assert.Null(await connectionListener.AcceptAndAddFeatureAsync().DefaultTimeout()); } [ConditionalFact] @@ -78,7 +78,7 @@ public async Task AcceptAsync_ClientCreatesInvalidConnection_ServerContinuesToAc await serverConnection1.DisposeAsync().AsTask().DefaultTimeout(); // Act & Assert 2 - var serverFailureLogTask = WaitForLogMessage(m => m.EventId.Name == "ConnectionListenerHandshakeFailed"); + var serverFailureLogTask = WaitForLogMessage(m => m.EventId.Name == "ConnectionListenerConnectionFailed"); Logger.LogInformation("Client creating unsuccessful connection 2"); var acceptTask2 = connectionListener.AcceptAndAddFeatureAsync().DefaultTimeout(); @@ -187,6 +187,87 @@ public async Task AcceptAsync_NoCertificateOrApplicationProtocol_Log() Assert.Contains(LogMessages, m => m.EventId.Name == "ConnectionListenerApplicationProtocolsNotSpecified"); } + [ConditionalFact] + [MsQuicSupported] + public async Task AcceptAsync_UnbindAfterCall_CleanExitAndLog() + { + // Arrange + await using var connectionListener = await QuicTestHelpers.CreateConnectionListenerFactory(LoggerFactory); + + // Act + var acceptTask = connectionListener.AcceptAndAddFeatureAsync(); + + await connectionListener.UnbindAsync().DefaultTimeout(); + + // Assert + Assert.Null(await acceptTask.AsTask().DefaultTimeout()); + + Assert.Contains(LogMessages, m => m.EventId.Name == "ConnectionListenerAborted"); + } + + [ConditionalFact] + [MsQuicSupported] + public async Task AcceptAsync_DisposeAfterCall_CleanExitAndLog() + { + // Arrange + await using var connectionListener = await QuicTestHelpers.CreateConnectionListenerFactory(LoggerFactory); + + // Act + var acceptTask = connectionListener.AcceptAndAddFeatureAsync(); + + await connectionListener.DisposeAsync().DefaultTimeout(); + + // Assert + Assert.Null(await acceptTask.AsTask().DefaultTimeout()); + + Assert.Contains(LogMessages, m => m.EventId.Name == "ConnectionListenerAborted"); + } + + [ConditionalFact] + [MsQuicSupported] + public async Task AcceptAsync_ErrorFromServerCallback_CleanExitAndLog() + { + // Arrange + var throwErrorInCallback = true; + await using var connectionListener = await QuicTestHelpers.CreateConnectionListenerFactory( + new TlsConnectionCallbackOptions + { + ApplicationProtocols = new List { SslApplicationProtocol.Http3 }, + OnConnection = (context, cancellationToken) => + { + if (throwErrorInCallback) + { + throwErrorInCallback = false; + throw new Exception("An error!"); + } + + var options = new SslServerAuthenticationOptions(); + options.ServerCertificate = TestResources.GetTestCertificate(); + return ValueTask.FromResult(options); + } + }, + LoggerFactory); + + // Act + var acceptTask = connectionListener.AcceptAndAddFeatureAsync(); + + var options = QuicTestHelpers.CreateClientConnectionOptions(connectionListener.EndPoint); + + var ex = await Assert.ThrowsAsync(() => QuicConnection.ConnectAsync(options).AsTask()).DefaultTimeout(); + Assert.Equal("Authentication failed because the remote party sent a TLS alert: 'UserCanceled'.", ex.Message); + + // Assert + Assert.False(acceptTask.IsCompleted); + + var clientConnection = await QuicConnection.ConnectAsync(options).DefaultTimeout(); + var serverConnection = await acceptTask.DefaultTimeout(); + + //await Assert.ThrowsAsync(() => QuicConnection.ConnectAsync(options).AsTask()); + //Assert.Null(await acceptTask.AsTask().DefaultTimeout()); + + //Assert.Contains(LogMessages, m => m.EventId.Name == "ConnectionListenerAborted"); + } + [ConditionalFact] [MsQuicSupported] public async Task BindAsync_ListenersSharePort_ThrowAddressInUse() @@ -308,8 +389,8 @@ public async Task AcceptAsync_NoCertificateCallback_RemovedFromPendingConnection syncPoint.Continue(); - await Assert.ThrowsAsync(() => acceptTask.AsTask()).DefaultTimeout(); await Assert.ThrowsAsync(() => clientConnectionTask.AsTask()).DefaultTimeout(); + Assert.False(acceptTask.IsCompleted); // Assert for (var i = 0; i < 20; i++) From d6e9fcd1f268ac65c7e461a25f26bf946d494c11 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Mon, 5 Sep 2022 20:34:30 +0800 Subject: [PATCH 4/6] Update --- .../Transport.Quic/src/Internal/QuicConnectionListener.cs | 2 +- src/Servers/Kestrel/Transport.Quic/src/Internal/QuicLog.cs | 4 ++-- .../Transport.Quic/test/QuicConnectionListenerTests.cs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs index 9fcffa80da25..a4d9d109e8d9 100644 --- a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs +++ b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs @@ -186,7 +186,7 @@ public async ValueTask CreateListenerAsync() // If the client rejects the connection because of an invalid cert then AcceptConnectionAsync throws. // An error thrown inside ConnectionOptionsCallback can also throw from AcceptConnectionAsync. // These are recoverable errors and we don't want to stop accepting connections. - QuicLog.ConnectionListenerConnectionFailed(_log, ex); + QuicLog.ConnectionListenerAcceptConnectionFailed(_log, ex); } } diff --git a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicLog.cs b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicLog.cs index 8319d34464f9..7077f79ca6fb 100644 --- a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicLog.cs +++ b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicLog.cs @@ -232,8 +232,8 @@ public static void StreamTimeoutWrite(ILogger logger, QuicStreamContext streamCo } } - [LoggerMessage(24, LogLevel.Debug, "QUIC listener connection failed.", EventName = "ConnectionListenerConnectionFailed")] - public static partial void ConnectionListenerConnectionFailed(ILogger logger, Exception exception); + [LoggerMessage(24, LogLevel.Debug, "QUIC listener connection failed.", EventName = "ConnectionListenerAcceptConnectionFailed")] + public static partial void ConnectionListenerAcceptConnectionFailed(ILogger logger, Exception exception); private static StreamType GetStreamType(QuicStreamContext streamContext) => streamContext.CanRead && streamContext.CanWrite diff --git a/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs b/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs index bd44cad85cd2..f88706902206 100644 --- a/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs +++ b/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs @@ -78,7 +78,7 @@ public async Task AcceptAsync_ClientCreatesInvalidConnection_ServerContinuesToAc await serverConnection1.DisposeAsync().AsTask().DefaultTimeout(); // Act & Assert 2 - var serverFailureLogTask = WaitForLogMessage(m => m.EventId.Name == "ConnectionListenerConnectionFailed"); + var serverFailureLogTask = WaitForLogMessage(m => m.EventId.Name == "ConnectionListenerAcceptConnectionFailed"); Logger.LogInformation("Client creating unsuccessful connection 2"); var acceptTask2 = connectionListener.AcceptAndAddFeatureAsync().DefaultTimeout(); From 7274e64d38ea9a4531a20713f8ddcba6cf73bb18 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 6 Sep 2022 07:21:30 +0800 Subject: [PATCH 5/6] Fix build --- .../src/Internal/QuicConnectionListener.cs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs index a4d9d109e8d9..11866cb40405 100644 --- a/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs +++ b/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs @@ -6,7 +6,6 @@ using System.Net.Quic; using System.Net.Security; using System.Runtime.CompilerServices; -using System.Security.Authentication; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Http.Features; using Microsoft.Extensions.Logging; @@ -76,10 +75,7 @@ public QuicConnectionListener( var serverAuthenticationOptions = await _tlsConnectionCallbackOptions.OnConnection(context, cancellationToken); // If the callback didn't set protocols then use the listener's list of protocols. - if (serverAuthenticationOptions.ApplicationProtocols == null) - { - serverAuthenticationOptions.ApplicationProtocols = _tlsConnectionCallbackOptions.ApplicationProtocols; - } + serverAuthenticationOptions.ApplicationProtocols ??= _tlsConnectionCallbackOptions.ApplicationProtocols; // If the SslServerAuthenticationOptions doesn't have a cert or protocols then the // QUIC connection will fail and the client receives an unhelpful message. @@ -193,10 +189,7 @@ public async ValueTask CreateListenerAsync() return null; } - public async ValueTask UnbindAsync(CancellationToken cancellationToken = default) - { - await DisposeAsync(); - } + public ValueTask UnbindAsync(CancellationToken cancellationToken = default) => DisposeAsync(); public async ValueTask DisposeAsync() { From 539196b8ad0146d1914caff47883bdfc7eaf6203 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 7 Sep 2022 08:15:57 +0800 Subject: [PATCH 6/6] PR feedback --- .../test/QuicConnectionListenerTests.cs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs b/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs index f88706902206..b89615a9133e 100644 --- a/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs +++ b/src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs @@ -82,11 +82,12 @@ public async Task AcceptAsync_ClientCreatesInvalidConnection_ServerContinuesToAc Logger.LogInformation("Client creating unsuccessful connection 2"); var acceptTask2 = connectionListener.AcceptAndAddFeatureAsync().DefaultTimeout(); - var ex = await ExceptionAssert.ThrowsAsync(async () => + var ex = await Assert.ThrowsAsync(async () => { await QuicConnection.ConnectAsync( QuicTestHelpers.CreateClientConnectionOptions(connectionListener.EndPoint, ignoreInvalidCertificate: false)); - }, "The remote certificate is invalid because of errors in the certificate chain: RemoteCertificateChainErrors"); + }); + Assert.Contains("RemoteCertificateChainErrors", ex.Message); Assert.False(acceptTask2.IsCompleted, "Accept doesn't return for failed client connection."); var serverFailureLog = await serverFailureLogTask.DefaultTimeout(); @@ -257,15 +258,13 @@ public async Task AcceptAsync_ErrorFromServerCallback_CleanExitAndLog() Assert.Equal("Authentication failed because the remote party sent a TLS alert: 'UserCanceled'.", ex.Message); // Assert - Assert.False(acceptTask.IsCompleted); - - var clientConnection = await QuicConnection.ConnectAsync(options).DefaultTimeout(); - var serverConnection = await acceptTask.DefaultTimeout(); + Assert.False(acceptTask.IsCompleted, "Still waiting for non-errored connection."); - //await Assert.ThrowsAsync(() => QuicConnection.ConnectAsync(options).AsTask()); - //Assert.Null(await acceptTask.AsTask().DefaultTimeout()); + await using var clientConnection = await QuicConnection.ConnectAsync(options).DefaultTimeout(); + await using var serverConnection = await acceptTask.DefaultTimeout(); - //Assert.Contains(LogMessages, m => m.EventId.Name == "ConnectionListenerAborted"); + Assert.NotNull(serverConnection); + Assert.NotNull(clientConnection); } [ConditionalFact]