From caf8cfa85e07482c2bda41797e0eeb21b0d2c8c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Wed, 6 May 2020 11:02:42 +0200 Subject: [PATCH 1/6] Added API --- src/libraries/System.Net.Security/ref/System.Net.Security.cs | 5 ++++- .../System.Net.Security/src/System/Net/Security/SslStream.cs | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Net.Security/ref/System.Net.Security.cs b/src/libraries/System.Net.Security/ref/System.Net.Security.cs index c022960def9c77..7a9f10e265d1c7 100644 --- a/src/libraries/System.Net.Security/ref/System.Net.Security.cs +++ b/src/libraries/System.Net.Security/ref/System.Net.Security.cs @@ -182,6 +182,7 @@ public partial class SslStream : System.Net.Security.AuthenticatedStream public virtual System.Security.Authentication.SslProtocols SslProtocol { get { throw null; } } public System.Net.TransportContext TransportContext { get { throw null; } } public override int WriteTimeout { get { throw null; } set { } } + public void AuthenticateAsClient(System.Net.Security.SslClientAuthenticationOptions sslClientAuthenticationOptions) { } public virtual void AuthenticateAsClient(string targetHost) { } public virtual void AuthenticateAsClient(string targetHost, System.Security.Cryptography.X509Certificates.X509CertificateCollection? clientCertificates, bool checkCertificateRevocation) { } public virtual void AuthenticateAsClient(string targetHost, System.Security.Cryptography.X509Certificates.X509CertificateCollection? clientCertificates, System.Security.Authentication.SslProtocols enabledSslProtocols, bool checkCertificateRevocation) { } @@ -189,6 +190,7 @@ public virtual void AuthenticateAsClient(string targetHost, System.Security.Cryp public virtual System.Threading.Tasks.Task AuthenticateAsClientAsync(string targetHost) { throw null; } public virtual System.Threading.Tasks.Task AuthenticateAsClientAsync(string targetHost, System.Security.Cryptography.X509Certificates.X509CertificateCollection? clientCertificates, bool checkCertificateRevocation) { throw null; } public virtual System.Threading.Tasks.Task AuthenticateAsClientAsync(string targetHost, System.Security.Cryptography.X509Certificates.X509CertificateCollection? clientCertificates, System.Security.Authentication.SslProtocols enabledSslProtocols, bool checkCertificateRevocation) { throw null; } + public void AuthenticateAsServer(System.Net.Security.SslServerAuthenticationOptions sslServerAuthenticationOptions) { } public virtual void AuthenticateAsServer(System.Security.Cryptography.X509Certificates.X509Certificate serverCertificate) { } public virtual void AuthenticateAsServer(System.Security.Cryptography.X509Certificates.X509Certificate serverCertificate, bool clientCertificateRequired, bool checkCertificateRevocation) { } public virtual void AuthenticateAsServer(System.Security.Cryptography.X509Certificates.X509Certificate serverCertificate, bool clientCertificateRequired, System.Security.Authentication.SslProtocols enabledSslProtocols, bool checkCertificateRevocation) { } @@ -210,6 +212,7 @@ public virtual void EndAuthenticateAsClient(System.IAsyncResult asyncResult) { } public virtual void EndAuthenticateAsServer(System.IAsyncResult asyncResult) { } public override int EndRead(System.IAsyncResult asyncResult) { throw null; } public override void EndWrite(System.IAsyncResult asyncResult) { } + ~SslStream() { } public override void Flush() { } public override System.Threading.Tasks.Task FlushAsync(System.Threading.CancellationToken cancellationToken) { throw null; } public override int Read(byte[] buffer, int offset, int count) { throw null; } @@ -222,7 +225,7 @@ public override void SetLength(long value) { } public void Write(byte[] buffer) { } public override void Write(byte[] buffer, int offset, int count) { } public override System.Threading.Tasks.Task WriteAsync(byte[] buffer, int offset, int count, System.Threading.CancellationToken cancellationToken) { throw null; } - public override System.Threading.Tasks.ValueTask WriteAsync(System.ReadOnlyMemory buffer, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } + public override System.Threading.Tasks.ValueTask WriteAsync(System.ReadOnlyMemory buffer, System.Threading.CancellationToken cancellationToken) { throw null; } } [System.CLSCompliantAttribute(false)] public enum TlsCipherSuite : ushort diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs index 73f4417dacc215..c48ac8eedffda2 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs @@ -304,7 +304,7 @@ public virtual void AuthenticateAsClient(string targetHost, X509CertificateColle AuthenticateAsClient(options); } - private void AuthenticateAsClient(SslClientAuthenticationOptions sslClientAuthenticationOptions) + public void AuthenticateAsClient(SslClientAuthenticationOptions sslClientAuthenticationOptions) { SetAndVerifyValidationCallback(sslClientAuthenticationOptions.RemoteCertificateValidationCallback); SetAndVerifySelectionCallback(sslClientAuthenticationOptions.LocalCertificateSelectionCallback); @@ -337,7 +337,7 @@ public virtual void AuthenticateAsServer(X509Certificate serverCertificate, bool AuthenticateAsServer(options); } - private void AuthenticateAsServer(SslServerAuthenticationOptions sslServerAuthenticationOptions) + public void AuthenticateAsServer(SslServerAuthenticationOptions sslServerAuthenticationOptions) { SetAndVerifyValidationCallback(sslServerAuthenticationOptions.RemoteCertificateValidationCallback); From d2ba99181875c66487c715f6aa1e0d189dd8334a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Wed, 6 May 2020 17:19:30 +0200 Subject: [PATCH 2/6] Tests --- .../SslAuthenticationOptionsTest.cs | 12 ++-- .../SslStreamAllowRenegotiationTests.cs | 16 ++++-- .../FunctionalTests/SslStreamAlpnTests.cs | 42 ++++++++++---- .../FunctionalTests/SslStreamExtensions.cs | 23 ++++++++ .../SslStreamStreamToStreamTest.cs | 55 ++++++++++++++----- .../System.Net.Security.Tests.csproj | 1 + 6 files changed, 112 insertions(+), 37 deletions(-) create mode 100644 src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamExtensions.cs diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslAuthenticationOptionsTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslAuthenticationOptionsTest.cs index ada4a652b6e776..222eade9647189 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslAuthenticationOptionsTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslAuthenticationOptionsTest.cs @@ -16,8 +16,10 @@ namespace System.Net.Security.Tests public class SslClientAuthenticationOptionsTest { - [Fact] - public async Task ClientOptions_ServerOptions_NotMutatedDuringAuthentication() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ClientOptions_ServerOptions_NotMutatedDuringAuthentication(bool async) { using (X509Certificate2 clientCert = Configuration.Certificates.GetClientCertificate()) using (X509Certificate2 serverCert = Configuration.Certificates.GetServerCertificate()) @@ -74,9 +76,9 @@ public async Task ClientOptions_ServerOptions_NotMutatedDuringAuthentication() }; // Authenticate - Task clientTask = client.AuthenticateAsClientAsync(clientOptions, default); - Task serverTask = server.AuthenticateAsServerAsync(serverOptions, default); - await new[] { clientTask, serverTask }.WhenAllOrAnyFailed(); + Task clientTask = client.AuthenticateAsClientAsync(clientOptions, async); + Task serverTask = server.AuthenticateAsServerAsync(serverOptions, async); + await new[] {clientTask, serverTask}.WhenAllOrAnyFailed(); // Validate that client options are unchanged Assert.Equal(clientAllowRenegotiation, clientOptions.AllowRenegotiation); diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAllowRenegotiationTests.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAllowRenegotiationTests.cs index 1895d118ea6f24..e6777e47b93ccf 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAllowRenegotiationTests.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAllowRenegotiationTests.cs @@ -19,9 +19,11 @@ namespace System.Net.Security.Tests public class SslStreamAllowRenegotiationTests { - [Fact] + [Theory] + [InlineData(true)] + [InlineData(false)] [OuterLoop] // Test hits external azure server. - public async Task SslStream_AllowRenegotiation_True_Succeeds() + public async Task SslStream_AllowRenegotiation_True_Succeeds(bool async) { int validationCount = 0; @@ -49,7 +51,7 @@ public async Task SslStream_AllowRenegotiation_True_Succeeds() }; // Perform handshake to establish secure connection. - await ssl.AuthenticateAsClientAsync(options, CancellationToken.None); + await ssl.AuthenticateAsClientAsync(options, async); Assert.True(ssl.IsAuthenticated); Assert.True(ssl.IsEncrypted); @@ -67,9 +69,11 @@ public async Task SslStream_AllowRenegotiation_True_Succeeds() } } - [Fact] + [Theory] + [InlineData(true)] + [InlineData(false)] [OuterLoop] // Test hits external azure server. - public async Task SslStream_AllowRenegotiation_False_Throws() + public async Task SslStream_AllowRenegotiation_False_Throws(bool async) { Socket s = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); await s.ConnectAsync(Configuration.Security.TlsRenegotiationServer, 443); @@ -89,7 +93,7 @@ public async Task SslStream_AllowRenegotiation_False_Throws() }; // Perform handshake to establish secure connection. - await ssl.AuthenticateAsClientAsync(options, CancellationToken.None); + await ssl.AuthenticateAsClientAsync(options, async); Assert.True(ssl.IsAuthenticated); Assert.True(ssl.IsEncrypted); diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAlpnTests.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAlpnTests.cs index de5212a93fc4c6..3a6d31f4430018 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAlpnTests.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAlpnTests.cs @@ -21,19 +21,21 @@ namespace System.Net.Security.Tests { using Configuration = System.Net.Test.Common.Configuration; - public class SslStreamAlpnTests + public abstract class SslStreamAlpnTestBase { private static bool BackendSupportsAlpn => PlatformDetection.SupportsAlpn; private static bool ClientSupportsAlpn => PlatformDetection.SupportsClientAlpn; readonly ITestOutputHelper _output; public static readonly object[][] Http2Servers = Configuration.Http.Http2Servers; - public SslStreamAlpnTests(ITestOutputHelper output) + // Whether AuthenticateAs(Client/Server) or AuthenticateAs(Client/Server)Async will be called + public abstract bool AuthenticateAsync { get; } + + protected SslStreamAlpnTestBase(ITestOutputHelper output) { _output = output; } - private async Task DoHandshakeWithOptions(SslStream clientSslStream, SslStream serverSslStream, SslClientAuthenticationOptions clientOptions, SslServerAuthenticationOptions serverOptions) { using (X509Certificate2 certificate = Configuration.Certificates.GetServerCertificate()) @@ -42,8 +44,8 @@ private async Task DoHandshakeWithOptions(SslStream clientSslStream, SslStream s clientOptions.TargetHost = certificate.GetNameInfo(X509NameType.SimpleName, false); serverOptions.ServerCertificate = certificate; - Task t1 = clientSslStream.AuthenticateAsClientAsync(clientOptions, CancellationToken.None); - Task t2 = serverSslStream.AuthenticateAsServerAsync(serverOptions, CancellationToken.None); + Task t1 = clientSslStream.AuthenticateAsClientAsync(clientOptions, AuthenticateAsync); + Task t2 = serverSslStream.AuthenticateAsServerAsync(serverOptions, AuthenticateAsync); await TestConfiguration.WhenAllOrAnyFailedWithTimeout(t1, t2); } @@ -95,8 +97,8 @@ public async Task SslStream_StreamToStream_DuplicateOptions_Throws() serverOptions.ServerCertificate = certificate; serverOptions.RemoteCertificateValidationCallback = AllowAnyServerCertificate; - Task t1 = Assert.ThrowsAsync(() => client.AuthenticateAsClientAsync(clientOptions, CancellationToken.None)); - Task t2 = Assert.ThrowsAsync(() => server.AuthenticateAsServerAsync(serverOptions, CancellationToken.None)); + Task t1 = Assert.ThrowsAsync(() => client.AuthenticateAsClientAsync(clientOptions, AuthenticateAsync)); + Task t2 = Assert.ThrowsAsync(() => server.AuthenticateAsServerAsync(serverOptions, AuthenticateAsync)); await TestConfiguration.WhenAllOrAnyFailedWithTimeout(t1, t2); } @@ -163,11 +165,11 @@ public async Task SslStream_StreamToStream_Alpn_NonMatchingProtocols_Fail() { // schannel sends alert on ALPN failure, openssl does not. Task t1 = Assert.ThrowsAsync(TestConfiguration.SupportsAlpnAlerts ? typeof(AuthenticationException) : typeof(IOException), () => - clientStream.AuthenticateAsClientAsync(clientOptions, CancellationToken.None)); + clientStream.AuthenticateAsClientAsync(clientOptions, AuthenticateAsync)); try { - await serverStream.AuthenticateAsServerAsync(serverOptions, CancellationToken.None); + await serverStream.AuthenticateAsServerAsync(serverOptions, AuthenticateAsync); Assert.True(false, "AuthenticationException was not thrown."); } catch (AuthenticationException) { server.Dispose(); } @@ -176,8 +178,8 @@ public async Task SslStream_StreamToStream_Alpn_NonMatchingProtocols_Fail() } else { - Task t1 = clientStream.AuthenticateAsClientAsync(clientOptions, CancellationToken.None); - Task t2 = serverStream.AuthenticateAsServerAsync(serverOptions, CancellationToken.None); + Task t1 = clientStream.AuthenticateAsClientAsync(clientOptions, AuthenticateAsync); + Task t2 = serverStream.AuthenticateAsServerAsync(serverOptions, AuthenticateAsync); await TestConfiguration.WhenAllOrAnyFailedWithTimeout(t1, t2); @@ -211,7 +213,7 @@ public async Task SslStream_Http2_Alpn_Success(Uri server) TargetHost = server.Host }; - await clientStream.AuthenticateAsClientAsync(clientOptions, CancellationToken.None); + await clientStream.AuthenticateAsClientAsync(clientOptions, AuthenticateAsync); Assert.Equal("h2", clientStream.NegotiatedApplicationProtocol.ToString()); } } @@ -246,4 +248,20 @@ public static IEnumerable Alpn_TestData() } } } + + public sealed class SslStreamAlpnTest_Async : SslStreamAlpnTestBase + { + public override bool AuthenticateAsync => true; + + public SslStreamAlpnTest_Async(ITestOutputHelper output) + : base (output) { } + } + + public sealed class SslStreamAlpnTest_Sync : SslStreamAlpnTestBase + { + public override bool AuthenticateAsync => false; + + public SslStreamAlpnTest_Sync(ITestOutputHelper output) + : base(output) { } + } } diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamExtensions.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamExtensions.cs new file mode 100644 index 00000000000000..09fde394f11e2d --- /dev/null +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamExtensions.cs @@ -0,0 +1,23 @@ +using System.Threading; +using System.Threading.Tasks; + +namespace System.Net.Security.Tests +{ + internal static class SslStreamExtensions + { + public static Task AuthenticateAsClientAsync(this SslStream stream, SslClientAuthenticationOptions clientOptions, + bool async, CancellationToken cancellationToken = default) + { + return async + ? stream.AuthenticateAsClientAsync(clientOptions, cancellationToken) + : Task.Run(() => stream.AuthenticateAsClient(clientOptions), cancellationToken); + } + public static Task AuthenticateAsServerAsync(this SslStream stream, SslServerAuthenticationOptions serverOptions, + bool async, CancellationToken cancellationToken = default) + { + return async + ? stream.AuthenticateAsServerAsync(serverOptions, cancellationToken) + : Task.Run(() => stream.AuthenticateAsServer(serverOptions), cancellationToken); + } + } +} \ No newline at end of file diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamStreamToStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamStreamToStreamTest.cs index 93baf83fd156fe..d98e914190033e 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamStreamToStreamTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamStreamToStreamTest.cs @@ -395,7 +395,7 @@ public async Task SslStream_StreamToStream_WriteAsync_ReadByte_Success() [Fact] public async Task SslStream_StreamToStream_WriteAsync_ReadAsync_Pending_Success() { - if (this is SslStreamStreamToStreamTest_Sync) + if (this is SslStreamStreamToStreamTest_SyncBase) { // This test assumes operations complete asynchronously. return; @@ -488,7 +488,7 @@ public async Task SslStream_ConcurrentBidirectionalReadsWrites_Success() [Fact] public async Task SslStream_StreamToStream_Dispose_Throws() { - if (this is SslStreamStreamToStreamTest_Sync) + if (this is SslStreamStreamToStreamTest_SyncBase) { // This test assumes operations complete asynchronously. return; @@ -821,19 +821,8 @@ protected override Task WriteAsync(Stream stream, byte[] buffer, int offset, int Task.Factory.FromAsync(stream.BeginWrite, stream.EndWrite, buffer, offset, count, null); } - public sealed class SslStreamStreamToStreamTest_Sync : SslStreamStreamToStreamTest + public abstract class SslStreamStreamToStreamTest_SyncBase : SslStreamStreamToStreamTest { - protected override async Task DoHandshake(SslStream clientSslStream, SslStream serverSslStream, X509Certificate serverCertificate = null, X509Certificate clientCertificate = null) - { - X509CertificateCollection clientCerts = clientCertificate != null ? new X509CertificateCollection() { clientCertificate } : null; - await WithServerCertificate(serverCertificate, async (certificate, name) => - { - Task t1 = Task.Run(() => clientSslStream.AuthenticateAsClient(name, clientCerts, SslProtocols.None, checkCertificateRevocation: false)); - Task t2 = Task.Run(() => serverSslStream.AuthenticateAsServer(certificate, clientCertificateRequired: clientCertificate != null, checkCertificateRevocation: false)); - await TestConfiguration.WhenAllOrAnyFailedWithTimeout(t1, t2); - }); - } - protected override Task ReadAsync(Stream stream, byte[] buffer, int offset, int count, CancellationToken cancellationToken) { if (cancellationToken.IsCancellationRequested) @@ -870,6 +859,44 @@ protected override Task WriteAsync(Stream stream, byte[] buffer, int offset, int } } + public sealed class SslStreamStreamToStreamTest_SyncParameters : SslStreamStreamToStreamTest_SyncBase + { + protected override async Task DoHandshake(SslStream clientSslStream, SslStream serverSslStream, X509Certificate serverCertificate = null, X509Certificate clientCertificate = null) + { + X509CertificateCollection clientCerts = clientCertificate != null ? new X509CertificateCollection() { clientCertificate } : null; + await WithServerCertificate(serverCertificate, async (certificate, name) => + { + Task t1 = Task.Run(() => clientSslStream.AuthenticateAsClient(name, clientCerts, SslProtocols.None, checkCertificateRevocation: false)); + Task t2 = Task.Run(() => serverSslStream.AuthenticateAsServer(certificate, clientCertificateRequired: clientCertificate != null, checkCertificateRevocation: false)); + await TestConfiguration.WhenAllOrAnyFailedWithTimeout(t1, t2); + }); + } + } + + public sealed class SslStreamStreamToStreamTest_SyncSslOptions : SslStreamStreamToStreamTest_SyncBase + { + protected override async Task DoHandshake(SslStream clientSslStream, SslStream serverSslStream, X509Certificate serverCertificate = null, X509Certificate clientCertificate = null) + { + X509CertificateCollection clientCerts = clientCertificate != null ? new X509CertificateCollection() { clientCertificate } : null; + await WithServerCertificate(serverCertificate, async (certificate, name) => + { + SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions + { + TargetHost = name, + ClientCertificates = clientCerts, + EnabledSslProtocols = SslProtocols.None, + }; + SslServerAuthenticationOptions serverOptions = new SslServerAuthenticationOptions() + { + ServerCertificate = certificate, ClientCertificateRequired = clientCertificate != null, + }; + Task t1 = Task.Run(() => clientSslStream.AuthenticateAsClient(clientOptions)); + Task t2 = Task.Run(() => serverSslStream.AuthenticateAsServer(serverOptions)); + await TestConfiguration.WhenAllOrAnyFailedWithTimeout(t1, t2); + }); + } + } + public sealed class SslStreamStreamToStreamTest_MemoryAsync : SslStreamStreamToStreamTest_CancelableReadWriteAsync { protected override async Task DoHandshake(SslStream clientSslStream, SslStream serverSslStream, X509Certificate serverCertificate = null, X509Certificate clientCertificate = null) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj b/src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj index 4ad947504b2c1c..94026f1078c10d 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj @@ -9,6 +9,7 @@ + From 58f8c9daaad9e511a2a899c17e94555eeef8552b Mon Sep 17 00:00:00 2001 From: ManickaP Date: Tue, 12 May 2020 11:26:57 +0200 Subject: [PATCH 3/6] Fixed missing default in ref, removed wrong ct usage. --- .../System.Net.Security/ref/System.Net.Security.cs | 2 +- .../src/System/Net/Security/SslStream.cs | 2 +- .../tests/FunctionalTests/SslStreamExtensions.cs | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Net.Security/ref/System.Net.Security.cs b/src/libraries/System.Net.Security/ref/System.Net.Security.cs index 7a9f10e265d1c7..ff6cee15760f38 100644 --- a/src/libraries/System.Net.Security/ref/System.Net.Security.cs +++ b/src/libraries/System.Net.Security/ref/System.Net.Security.cs @@ -225,7 +225,7 @@ public override void SetLength(long value) { } public void Write(byte[] buffer) { } public override void Write(byte[] buffer, int offset, int count) { } public override System.Threading.Tasks.Task WriteAsync(byte[] buffer, int offset, int count, System.Threading.CancellationToken cancellationToken) { throw null; } - public override System.Threading.Tasks.ValueTask WriteAsync(System.ReadOnlyMemory buffer, System.Threading.CancellationToken cancellationToken) { throw null; } + public override System.Threading.Tasks.ValueTask WriteAsync(System.ReadOnlyMemory buffer, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } } [System.CLSCompliantAttribute(false)] public enum TlsCipherSuite : ushort diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs index c48ac8eedffda2..0db0bc5ea28350 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs @@ -792,7 +792,7 @@ public override Task WriteAsync(byte[] buffer, int offset, int count, Cancellati return WriteAsync(new ReadOnlyMemory(buffer, offset, count), cancellationToken).AsTask(); } - public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken) + public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken = default) { ThrowIfExceptionalOrNotAuthenticated(); AsyncSslIOAdapter writeAdapter = new AsyncSslIOAdapter(this, cancellationToken); diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamExtensions.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamExtensions.cs index 09fde394f11e2d..5220301a6b779e 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamExtensions.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamExtensions.cs @@ -10,14 +10,14 @@ public static Task AuthenticateAsClientAsync(this SslStream stream, SslClientAut { return async ? stream.AuthenticateAsClientAsync(clientOptions, cancellationToken) - : Task.Run(() => stream.AuthenticateAsClient(clientOptions), cancellationToken); + : Task.Run(() => stream.AuthenticateAsClient(clientOptions)); } public static Task AuthenticateAsServerAsync(this SslStream stream, SslServerAuthenticationOptions serverOptions, bool async, CancellationToken cancellationToken = default) { return async ? stream.AuthenticateAsServerAsync(serverOptions, cancellationToken) - : Task.Run(() => stream.AuthenticateAsServer(serverOptions), cancellationToken); + : Task.Run(() => stream.AuthenticateAsServer(serverOptions)); } } -} \ No newline at end of file +} From 22a2d580002bb69a65ae6127008cecc50df638de Mon Sep 17 00:00:00 2001 From: ManickaP Date: Tue, 12 May 2020 11:49:27 +0200 Subject: [PATCH 4/6] Null checks. --- .../src/System/Net/Security/SslStream.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs index 0db0bc5ea28350..79c16bf043ed4b 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs @@ -306,6 +306,11 @@ public virtual void AuthenticateAsClient(string targetHost, X509CertificateColle public void AuthenticateAsClient(SslClientAuthenticationOptions sslClientAuthenticationOptions) { + if (sslClientAuthenticationOptions == null) + { + throw new ArgumentNullException(nameof(sslClientAuthenticationOptions)); + } + SetAndVerifyValidationCallback(sslClientAuthenticationOptions.RemoteCertificateValidationCallback); SetAndVerifySelectionCallback(sslClientAuthenticationOptions.LocalCertificateSelectionCallback); @@ -339,6 +344,11 @@ public virtual void AuthenticateAsServer(X509Certificate serverCertificate, bool public void AuthenticateAsServer(SslServerAuthenticationOptions sslServerAuthenticationOptions) { + if (sslServerAuthenticationOptions == null) + { + throw new ArgumentNullException(nameof(sslServerAuthenticationOptions)); + } + SetAndVerifyValidationCallback(sslServerAuthenticationOptions.RemoteCertificateValidationCallback); ValidateCreateContext(CreateAuthenticationOptions(sslServerAuthenticationOptions)); @@ -367,6 +377,11 @@ public virtual Task AuthenticateAsClientAsync(string targetHost, X509Certificate public Task AuthenticateAsClientAsync(SslClientAuthenticationOptions sslClientAuthenticationOptions, CancellationToken cancellationToken = default) { + if (sslClientAuthenticationOptions == null) + { + throw new ArgumentNullException(nameof(sslClientAuthenticationOptions)); + } + SetAndVerifyValidationCallback(sslClientAuthenticationOptions.RemoteCertificateValidationCallback); SetAndVerifySelectionCallback(sslClientAuthenticationOptions.LocalCertificateSelectionCallback); @@ -417,6 +432,11 @@ public virtual Task AuthenticateAsServerAsync(X509Certificate serverCertificate, public Task AuthenticateAsServerAsync(SslServerAuthenticationOptions sslServerAuthenticationOptions, CancellationToken cancellationToken = default) { + if (sslServerAuthenticationOptions == null) + { + throw new ArgumentNullException(nameof(sslServerAuthenticationOptions)); + } + SetAndVerifyValidationCallback(sslServerAuthenticationOptions.RemoteCertificateValidationCallback); ValidateCreateContext(CreateAuthenticationOptions(sslServerAuthenticationOptions)); From c0dbb64e8b86be118689f35d76a2563e693fd541 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Tue, 12 May 2020 16:28:31 +0200 Subject: [PATCH 5/6] Added tests for stream dispose before handshake finishes. --- .../SslStreamStreamToStreamTest.cs | 66 ++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamStreamToStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamStreamToStreamTest.cs index d98e914190033e..9fb2ac9fefbd6b 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamStreamToStreamTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamStreamToStreamTest.cs @@ -511,7 +511,7 @@ public async Task SslStream_StreamToStream_Dispose_Throws() await WriteAsync(serverSslStream, new byte[] { 1 }, 0, 1) .TimeoutAfter(TestConfiguration.PassingTestTimeoutMilliseconds); - // Shouldn't throw, the context is diposed now. + // Shouldn't throw, the context is disposed now. // Since the server read task is in progress, the read buffer is not returned to ArrayPool. serverSslStream.Dispose(); @@ -857,6 +857,70 @@ protected override Task WriteAsync(Stream stream, byte[] buffer, int offset, int return Task.FromException(e); } } + + [Fact] + public async Task SslStream_StreamToStream_Handshake_DisposeClient_Throws() + { + VirtualNetwork network = new VirtualNetwork(); + + var clientStream = new VirtualNetworkStream(network, isServer: false); + using (var serverStream = new VirtualNetworkStream(network, isServer: true)) + using (var clientSslStream = new SslStream(clientStream, false, AllowAnyServerCertificate)) + using (var serverSslStream = new SslStream(serverStream)) + { + clientStream.Dispose(); + + await Assert.ThrowsAsync(() => DoHandshake(clientSslStream, serverSslStream)); + } + } + + [Fact] + public async Task SslStream_StreamToStream_Handshake_DisposeServer_Throws() + { + VirtualNetwork network = new VirtualNetwork(); + + var serverStream = new VirtualNetworkStream(network, isServer: true); + using (var clientStream = new VirtualNetworkStream(network, isServer: false)) + using (var clientSslStream = new SslStream(clientStream, false, AllowAnyServerCertificate)) + using (var serverSslStream = new SslStream(serverStream)) + { + serverStream.Dispose(); + + await Assert.ThrowsAsync(() => DoHandshake(clientSslStream, serverSslStream)); + } + } + + [Fact] + public async Task SslStream_StreamToStream_Handshake_DisposeClientSsl_Throws() + { + VirtualNetwork network = new VirtualNetwork() { DisableConnectionBreaking = true }; + + using (var clientStream = new VirtualNetworkStream(network, isServer: false)) + using (var serverStream = new VirtualNetworkStream(network, isServer: true)) + using (var serverSslStream = new SslStream(serverStream)) + { + var clientSslStream = new SslStream(clientStream, false, AllowAnyServerCertificate); + clientSslStream.Dispose(); + + await Assert.ThrowsAsync(() => DoHandshake(clientSslStream, serverSslStream)); + } + } + + [Fact] + public async Task SslStream_StreamToStream_Handshake_DisposeServerSsl_Throws() + { + VirtualNetwork network = new VirtualNetwork() {DisableConnectionBreaking = true}; + + using (var clientStream = new VirtualNetworkStream(network, isServer: false)) + using (var serverStream = new VirtualNetworkStream(network, isServer: true)) + using (var clientSslStream = new SslStream(clientStream, false, AllowAnyServerCertificate)) + { + var serverSslStream = new SslStream(serverStream); + serverSslStream.Dispose(); + + await Assert.ThrowsAsync(() => DoHandshake(clientSslStream, serverSslStream)); + } + } } public sealed class SslStreamStreamToStreamTest_SyncParameters : SslStreamStreamToStreamTest_SyncBase From abe6595fd34d6453f527f2dc950fa9198e8ad9d4 Mon Sep 17 00:00:00 2001 From: ManickaP Date: Fri, 15 May 2020 15:07:29 +0200 Subject: [PATCH 6/6] Made clearer that test extension is called instead of an API. --- .../SslAuthenticationOptionsTest.cs | 24 ++++++++++----- .../SslStreamAllowRenegotiationTests.cs | 30 ++++++++++++------- .../FunctionalTests/SslStreamAlpnTests.cs | 24 +++++++-------- .../FunctionalTests/SslStreamExtensions.cs | 10 ++++--- 4 files changed, 54 insertions(+), 34 deletions(-) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslAuthenticationOptionsTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslAuthenticationOptionsTest.cs index 222eade9647189..af9e02ad03ed60 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslAuthenticationOptionsTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslAuthenticationOptionsTest.cs @@ -14,12 +14,12 @@ namespace System.Net.Security.Tests { using Configuration = System.Net.Test.Common.Configuration; - public class SslClientAuthenticationOptionsTest + public abstract class SslClientAuthenticationOptionsTestBase { - [Theory] - [InlineData(true)] - [InlineData(false)] - public async Task ClientOptions_ServerOptions_NotMutatedDuringAuthentication(bool async) + protected abstract bool TestAuthenticateAsync { get; } + + [Fact] + public async Task ClientOptions_ServerOptions_NotMutatedDuringAuthentication() { using (X509Certificate2 clientCert = Configuration.Certificates.GetClientCertificate()) using (X509Certificate2 serverCert = Configuration.Certificates.GetServerCertificate()) @@ -76,8 +76,8 @@ public async Task ClientOptions_ServerOptions_NotMutatedDuringAuthentication(boo }; // Authenticate - Task clientTask = client.AuthenticateAsClientAsync(clientOptions, async); - Task serverTask = server.AuthenticateAsServerAsync(serverOptions, async); + Task clientTask = client.AuthenticateAsClientAsync(TestAuthenticateAsync, clientOptions); + Task serverTask = server.AuthenticateAsServerAsync(TestAuthenticateAsync, serverOptions); await new[] {clientTask, serverTask}.WhenAllOrAnyFailed(); // Validate that client options are unchanged @@ -107,4 +107,14 @@ public async Task ClientOptions_ServerOptions_NotMutatedDuringAuthentication(boo } } } + + public sealed class SslClientAuthenticationOptionsTestBase_Sync : SslClientAuthenticationOptionsTestBase + { + protected override bool TestAuthenticateAsync => false; + } + + public sealed class SslClientAuthenticationOptionsTestBase_Async : SslClientAuthenticationOptionsTestBase + { + protected override bool TestAuthenticateAsync => true; + } } diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAllowRenegotiationTests.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAllowRenegotiationTests.cs index e6777e47b93ccf..b4147844b9060e 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAllowRenegotiationTests.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAllowRenegotiationTests.cs @@ -17,13 +17,13 @@ namespace System.Net.Security.Tests { using Configuration = System.Net.Test.Common.Configuration; - public class SslStreamAllowRenegotiationTests + public abstract class SslStreamAllowRenegotiationTestsBase { - [Theory] - [InlineData(true)] - [InlineData(false)] + protected abstract bool TestAuthenticateAsync { get; } + + [Fact] [OuterLoop] // Test hits external azure server. - public async Task SslStream_AllowRenegotiation_True_Succeeds(bool async) + public async Task SslStream_AllowRenegotiation_True_Succeeds() { int validationCount = 0; @@ -51,7 +51,7 @@ public async Task SslStream_AllowRenegotiation_True_Succeeds(bool async) }; // Perform handshake to establish secure connection. - await ssl.AuthenticateAsClientAsync(options, async); + await ssl.AuthenticateAsClientAsync(TestAuthenticateAsync, options); Assert.True(ssl.IsAuthenticated); Assert.True(ssl.IsEncrypted); @@ -69,11 +69,9 @@ public async Task SslStream_AllowRenegotiation_True_Succeeds(bool async) } } - [Theory] - [InlineData(true)] - [InlineData(false)] + [Fact] [OuterLoop] // Test hits external azure server. - public async Task SslStream_AllowRenegotiation_False_Throws(bool async) + public async Task SslStream_AllowRenegotiation_False_Throws() { Socket s = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); await s.ConnectAsync(Configuration.Security.TlsRenegotiationServer, 443); @@ -93,7 +91,7 @@ public async Task SslStream_AllowRenegotiation_False_Throws(bool async) }; // Perform handshake to establish secure connection. - await ssl.AuthenticateAsClientAsync(options, async); + await ssl.AuthenticateAsClientAsync(TestAuthenticateAsync, options); Assert.True(ssl.IsAuthenticated); Assert.True(ssl.IsEncrypted); @@ -107,4 +105,14 @@ public async Task SslStream_AllowRenegotiation_False_Throws(bool async) } } } + + public sealed class SslStreamAllowRenegotiationTests_Sync : SslStreamAllowRenegotiationTestsBase + { + protected override bool TestAuthenticateAsync => false; + } + + public sealed class SslStreamAllowRenegotiationTests_Async : SslStreamAllowRenegotiationTestsBase + { + protected override bool TestAuthenticateAsync => true; + } } diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAlpnTests.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAlpnTests.cs index 3a6d31f4430018..c5284653585b78 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAlpnTests.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamAlpnTests.cs @@ -29,7 +29,7 @@ public abstract class SslStreamAlpnTestBase public static readonly object[][] Http2Servers = Configuration.Http.Http2Servers; // Whether AuthenticateAs(Client/Server) or AuthenticateAs(Client/Server)Async will be called - public abstract bool AuthenticateAsync { get; } + public abstract bool TestAuthenticateAsync { get; } protected SslStreamAlpnTestBase(ITestOutputHelper output) { @@ -44,8 +44,8 @@ private async Task DoHandshakeWithOptions(SslStream clientSslStream, SslStream s clientOptions.TargetHost = certificate.GetNameInfo(X509NameType.SimpleName, false); serverOptions.ServerCertificate = certificate; - Task t1 = clientSslStream.AuthenticateAsClientAsync(clientOptions, AuthenticateAsync); - Task t2 = serverSslStream.AuthenticateAsServerAsync(serverOptions, AuthenticateAsync); + Task t1 = clientSslStream.AuthenticateAsClientAsync(TestAuthenticateAsync, clientOptions); + Task t2 = serverSslStream.AuthenticateAsServerAsync(TestAuthenticateAsync, serverOptions); await TestConfiguration.WhenAllOrAnyFailedWithTimeout(t1, t2); } @@ -97,8 +97,8 @@ public async Task SslStream_StreamToStream_DuplicateOptions_Throws() serverOptions.ServerCertificate = certificate; serverOptions.RemoteCertificateValidationCallback = AllowAnyServerCertificate; - Task t1 = Assert.ThrowsAsync(() => client.AuthenticateAsClientAsync(clientOptions, AuthenticateAsync)); - Task t2 = Assert.ThrowsAsync(() => server.AuthenticateAsServerAsync(serverOptions, AuthenticateAsync)); + Task t1 = Assert.ThrowsAsync(() => client.AuthenticateAsClientAsync(TestAuthenticateAsync, clientOptions)); + Task t2 = Assert.ThrowsAsync(() => server.AuthenticateAsServerAsync(TestAuthenticateAsync, serverOptions)); await TestConfiguration.WhenAllOrAnyFailedWithTimeout(t1, t2); } @@ -165,11 +165,11 @@ public async Task SslStream_StreamToStream_Alpn_NonMatchingProtocols_Fail() { // schannel sends alert on ALPN failure, openssl does not. Task t1 = Assert.ThrowsAsync(TestConfiguration.SupportsAlpnAlerts ? typeof(AuthenticationException) : typeof(IOException), () => - clientStream.AuthenticateAsClientAsync(clientOptions, AuthenticateAsync)); + clientStream.AuthenticateAsClientAsync(TestAuthenticateAsync, clientOptions)); try { - await serverStream.AuthenticateAsServerAsync(serverOptions, AuthenticateAsync); + await serverStream.AuthenticateAsServerAsync(TestAuthenticateAsync, serverOptions); Assert.True(false, "AuthenticationException was not thrown."); } catch (AuthenticationException) { server.Dispose(); } @@ -178,8 +178,8 @@ public async Task SslStream_StreamToStream_Alpn_NonMatchingProtocols_Fail() } else { - Task t1 = clientStream.AuthenticateAsClientAsync(clientOptions, AuthenticateAsync); - Task t2 = serverStream.AuthenticateAsServerAsync(serverOptions, AuthenticateAsync); + Task t1 = clientStream.AuthenticateAsClientAsync(TestAuthenticateAsync, clientOptions); + Task t2 = serverStream.AuthenticateAsServerAsync(TestAuthenticateAsync, serverOptions); await TestConfiguration.WhenAllOrAnyFailedWithTimeout(t1, t2); @@ -213,7 +213,7 @@ public async Task SslStream_Http2_Alpn_Success(Uri server) TargetHost = server.Host }; - await clientStream.AuthenticateAsClientAsync(clientOptions, AuthenticateAsync); + await clientStream.AuthenticateAsClientAsync(TestAuthenticateAsync, clientOptions); Assert.Equal("h2", clientStream.NegotiatedApplicationProtocol.ToString()); } } @@ -251,7 +251,7 @@ public static IEnumerable Alpn_TestData() public sealed class SslStreamAlpnTest_Async : SslStreamAlpnTestBase { - public override bool AuthenticateAsync => true; + public override bool TestAuthenticateAsync => true; public SslStreamAlpnTest_Async(ITestOutputHelper output) : base (output) { } @@ -259,7 +259,7 @@ public SslStreamAlpnTest_Async(ITestOutputHelper output) public sealed class SslStreamAlpnTest_Sync : SslStreamAlpnTestBase { - public override bool AuthenticateAsync => false; + public override bool TestAuthenticateAsync => false; public SslStreamAlpnTest_Sync(ITestOutputHelper output) : base(output) { } diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamExtensions.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamExtensions.cs index 5220301a6b779e..e7e423ee89580c 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamExtensions.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamExtensions.cs @@ -5,15 +5,17 @@ namespace System.Net.Security.Tests { internal static class SslStreamExtensions { - public static Task AuthenticateAsClientAsync(this SslStream stream, SslClientAuthenticationOptions clientOptions, - bool async, CancellationToken cancellationToken = default) + public static Task AuthenticateAsClientAsync(this SslStream stream, + bool async, SslClientAuthenticationOptions clientOptions, + CancellationToken cancellationToken = default) { return async ? stream.AuthenticateAsClientAsync(clientOptions, cancellationToken) : Task.Run(() => stream.AuthenticateAsClient(clientOptions)); } - public static Task AuthenticateAsServerAsync(this SslStream stream, SslServerAuthenticationOptions serverOptions, - bool async, CancellationToken cancellationToken = default) + public static Task AuthenticateAsServerAsync(this SslStream stream, + bool async, SslServerAuthenticationOptions serverOptions, + CancellationToken cancellationToken = default) { return async ? stream.AuthenticateAsServerAsync(serverOptions, cancellationToken)