Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
6 changes: 3 additions & 3 deletions src/Servers/Kestrel/Core/test/TlsListenerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public async Task RunTlsClientHelloCallbackTest_WithPendingCancellation()
await writer.WriteAsync(new byte[2] { 0x03, 0x01 });
cts.Cancel();

await Assert.ThrowsAsync<OperationCanceledException>(async () => await listenerTask);
await VerifyThrowsAnyAsync(() => listenerTask, typeof(OperationCanceledException), typeof(TaskCanceledException));
Copy link
Member

Choose a reason for hiding this comment

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

Normally we just use Assert.ThrowsAnyAsync<OperationCanceledException>(...); for this

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks, i did not see that API before. refactored

Assert.False(tlsClientHelloCallbackInvoked);
}

Expand Down Expand Up @@ -158,8 +158,8 @@ public async Task RunTlsClientHelloCallbackTest_DeterministicallyReads()
Assert.Equal(5, readResult.Buffer.Length);

// ensuring that we have read limited number of times
Assert.True(reader.ReadAsyncCounter is >= 2 && reader.ReadAsyncCounter is <= 4,
$"Expected ReadAsync() to happen about 2-4 times. Actually happened {reader.ReadAsyncCounter} times.");
Assert.True(reader.ReadAsyncCounter is >= 2 && reader.ReadAsyncCounter is <= 5,
$"Expected ReadAsync() to happen about 2-5 times. Actually happened {reader.ReadAsyncCounter} times.");
}

private async Task RunTlsClientHelloCallbackTest_WithMultipleSegments(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
using Xunit.Sdk;

namespace InMemory.FunctionalTests;

Expand Down Expand Up @@ -66,4 +67,60 @@ await sslStream.AuthenticateAsClientAsync(new SslClientAuthenticationOptions

Assert.True(tlsClientHelloCallbackInvoked);
}

[Fact]
public async Task TlsClientHelloBytesCallback_PreCanceledToken()
Copy link
Member

Choose a reason for hiding this comment

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

There still isn't a test using the HandshakeTimeout

Copy link
Member Author

Choose a reason for hiding this comment

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

added

{
var tlsClientHelloCallbackInvoked = false;

var testContext = new TestServiceContext(LoggerFactory);
await using (var server = new TestServer(context => Task.CompletedTask,
testContext,
listenOptions =>
{
listenOptions.UseHttps(_x509Certificate2, httpsOptions =>
{
httpsOptions.TlsClientHelloBytesCallback = (connection, clientHelloBytes) =>
{
Logger.LogDebug("[Received TlsClientHelloBytesCallback] Connection: {0}; TLS client hello buffer: {1}", connection.ConnectionId, clientHelloBytes.Length);
tlsClientHelloCallbackInvoked = true;
Assert.True(clientHelloBytes.Length > 32);
Assert.NotNull(connection);
};
});
}))
{
using (var connection = server.CreateConnection())
{
using (var sslStream = new SslStream(connection.Stream, false, (sender, cert, chain, errors) => true, null))
Copy link
Member

Choose a reason for hiding this comment

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

I don't think these tests should actually send any bytes over the connection. That creates a race where the timeout doesn't actually occur by the time the tls client hello is received and parsed.

Copy link
Member Author

Choose a reason for hiding this comment

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

HttpsConnectionMiddleware does catch the exception, so i am only seeing if the connection is closed after the timeout. I changed to

await connection.TransportConnection.Input.WriteAsync(new byte[] { 0x16 });
var readResult = await connection.TransportConnection.Output.ReadAsync();

// HttpsConnectionMiddleware catches the exception, so we can only check the effects of the timeout here
Assert.True(readResult.IsCompleted);

{
var cancellationTokenSource = new CancellationTokenSource(TimeSpan.FromMilliseconds(1));
var token = cancellationTokenSource.Token;

try
{
await sslStream.AuthenticateAsClientAsync(new SslClientAuthenticationOptions
{
TargetHost = "localhost",
EnabledSslProtocols = SslProtocols.None
}, token);

var request = Encoding.ASCII.GetBytes("GET / HTTP/1.1\r\nHost:\r\n\r\n");
await sslStream.WriteAsync(request, 0, request.Length, token);
await sslStream.ReadAsync(new Memory<byte>(new byte[1024]), token);
}
catch (Exception ex) when (ex is OperationCanceledException or TaskCanceledException)
{
// expected
}
catch (Exception ex)
{
ThrowsException.ForIncorrectExceptionType(typeof(OperationCanceledException), ex);
}
}
}
}

Assert.False(tlsClientHelloCallbackInvoked);
}
}
Loading