From b56d66ab51e9ffc7dd4ab322d62325e650f583c8 Mon Sep 17 00:00:00 2001 From: Tomas Weinfurt Date: Fri, 3 Jul 2020 09:46:15 -0700 Subject: [PATCH 1/8] add ServerOptionsSelectionCallback to SslStream --- .../ref/System.Net.Security.cs | 9 ++ .../src/System.Net.Security.csproj | 1 + .../Net/Security/SslAuthenticationOptions.cs | 38 +++++++- .../Net/Security/SslStream.Implementation.cs | 22 ++++- .../src/System/Net/Security/SslStream.cs | 8 ++ .../ServerAsyncAuthenticateTest.cs | 89 +++++++++++++++++++ .../tests/FunctionalTests/TestHelper.cs | 6 ++ 7 files changed, 170 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 6c202736fc62d..e28d00b586e41 100644 --- a/src/libraries/System.Net.Security/ref/System.Net.Security.cs +++ b/src/libraries/System.Net.Security/ref/System.Net.Security.cs @@ -105,8 +105,16 @@ public enum ProtectionLevel Sign = 1, EncryptAndSign = 2, } + public readonly struct SslClientHelloInfo + { + public string ServerName { get; } + public System.Security.Authentication.SslProtocols SslProtocols { get; } + internal SslClientHelloInfo(string serverName, System.Security.Authentication.SslProtocols sslProtocol) { throw null; } + } public delegate bool RemoteCertificateValidationCallback(object sender, System.Security.Cryptography.X509Certificates.X509Certificate? certificate, System.Security.Cryptography.X509Certificates.X509Chain? chain, System.Net.Security.SslPolicyErrors sslPolicyErrors); public delegate System.Security.Cryptography.X509Certificates.X509Certificate ServerCertificateSelectionCallback(object sender, string? hostName); + public delegate System.Threading.Tasks.ValueTask ServerOptionsSelectionCallback(SslStream stream, SslClientHelloInfo clientHelloInfo, object? state, System.Threading.CancellationToken cancellationToken); + public readonly partial struct SslApplicationProtocol : System.IEquatable { private readonly object _dummy; @@ -237,6 +245,7 @@ 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 System.Threading.Tasks.Task AuthenticateAsServerAsync( ServerOptionsSelectionCallback optionCallback, object? state, System.Threading.CancellationToken cancellationToken = default) { throw null; } } [System.CLSCompliantAttribute(false)] public enum TlsCipherSuite : ushort diff --git a/src/libraries/System.Net.Security/src/System.Net.Security.csproj b/src/libraries/System.Net.Security/src/System.Net.Security.csproj index 7827fa79c8a19..ec0da39cf9456 100644 --- a/src/libraries/System.Net.Security/src/System.Net.Security.csproj +++ b/src/libraries/System.Net.Security/src/System.Net.Security.csproj @@ -23,6 +23,7 @@ + diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs index faf3ac340e27a..31bb4fec70507 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs @@ -81,6 +81,40 @@ internal SslAuthenticationOptions(SslServerAuthenticationOptions sslServerAuthen } } + internal SslAuthenticationOptions(ServerOptionsSelectionCallback optionCallback, object? state) + { + CheckCertName = false; + TargetHost = string.Empty; + IsServer = true; + UserState = state; + ServerOptionDelegate = optionCallback; + } + + internal void UpdateOptions(SslServerAuthenticationOptions sslServerAuthenticationOptions) + { + AllowRenegotiation = sslServerAuthenticationOptions.AllowRenegotiation; + ApplicationProtocols = sslServerAuthenticationOptions.ApplicationProtocols; + EnabledSslProtocols = FilterOutIncompatibleSslProtocols(sslServerAuthenticationOptions.EnabledSslProtocols); + EncryptionPolicy = sslServerAuthenticationOptions.EncryptionPolicy; + RemoteCertRequired = sslServerAuthenticationOptions.ClientCertificateRequired; + CipherSuitesPolicy = sslServerAuthenticationOptions.CipherSuitesPolicy; + CertificateRevocationCheckMode = sslServerAuthenticationOptions.CertificateRevocationCheckMode; + if (sslServerAuthenticationOptions.ServerCertificateContext != null) + { + CertificateContext = sslServerAuthenticationOptions.ServerCertificateContext; + } + else if (sslServerAuthenticationOptions.ServerCertificate != null) + { + X509Certificate2? certificateWithKey = sslServerAuthenticationOptions.ServerCertificate as X509Certificate2; + + if (certificateWithKey != null && certificateWithKey.HasPrivateKey) + { + // given cert is X509Certificate2 with key. We can use it directly. + CertificateContext = SslStreamCertificateContext.Create(certificateWithKey); + } + } + } + private static SslProtocols FilterOutIncompatibleSslProtocols(SslProtocols protocols) { if (protocols.HasFlag(SslProtocols.Tls12) || protocols.HasFlag(SslProtocols.Tls13)) @@ -99,7 +133,7 @@ private static SslProtocols FilterOutIncompatibleSslProtocols(SslProtocols proto internal bool AllowRenegotiation { get; set; } internal string TargetHost { get; set; } internal X509CertificateCollection? ClientCertificates { get; set; } - internal List? ApplicationProtocols { get; } + internal List? ApplicationProtocols { get; set; } internal bool IsServer { get; set; } internal SslStreamCertificateContext? CertificateContext { get; set; } internal SslProtocols EnabledSslProtocols { get; set; } @@ -111,5 +145,7 @@ private static SslProtocols FilterOutIncompatibleSslProtocols(SslProtocols proto internal LocalCertSelectionCallback? CertSelectionDelegate { get; set; } internal ServerCertSelectionCallback? ServerCertSelectionDelegate { get; set; } internal CipherSuitesPolicy? CipherSuitesPolicy { get; set; } + internal object? UserState { get; set; } + internal ServerOptionsSelectionCallback? ServerOptionDelegate { get; set; } } } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs index 42e8a7c7c85ac..8f1293abebaae 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs @@ -36,7 +36,7 @@ private enum Framing private TlsFrameHelper.TlsFrameInfo _lastFrame; - private readonly object _handshakeLock = new object(); + private object _handshakeLock => _sslAuthenticationOptions!; private volatile TaskCompletionSource? _handshakeWaiter; private const int FrameOverhead = 32; @@ -409,7 +409,8 @@ private async ValueTask ReceiveBlobAsync(TIOAdapter a else if (_lastFrame.Header.Type == TlsContentType.Handshake) { if ((_handshakeBuffer.ActiveReadOnlySpan[TlsFrameHelper.HeaderSize] == (byte)TlsHandshakeType.ClientHello && - _sslAuthenticationOptions!.ServerCertSelectionDelegate != null) || + (_sslAuthenticationOptions!.ServerCertSelectionDelegate != null || + _sslAuthenticationOptions!.ServerOptionDelegate != null)) || NetEventSource.IsEnabled) { TlsFrameHelper.ProcessingOptions options = NetEventSource.IsEnabled ? @@ -426,6 +427,23 @@ private async ValueTask ReceiveBlobAsync(TIOAdapter a { // SNI if it exist. Even if we could not parse the hello, we can fall-back to default certificate. _sslAuthenticationOptions!.TargetHost = _lastFrame.TargetName; + + if (_sslAuthenticationOptions.ServerOptionDelegate != null) + { + ValueTask t = + _sslAuthenticationOptions.ServerOptionDelegate(this, new SslClientHelloInfo(_lastFrame.TargetName, _lastFrame.SupportedVersions), + _sslAuthenticationOptions.UserState, adapter.CancellationToken); + + if (t.IsCompletedSuccessfully) + { + _sslAuthenticationOptions.UpdateOptions(t.Result); + } + else + { + SslServerAuthenticationOptions userOptions = await t.ConfigureAwait(false); + _sslAuthenticationOptions.UpdateOptions(userOptions); + } + } } if (NetEventSource.IsEnabled) 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 11612d3300cdb..bbd745a03d3ab 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 @@ -35,6 +35,8 @@ public enum EncryptionPolicy public delegate X509Certificate ServerCertificateSelectionCallback(object sender, string? hostName); + public delegate ValueTask ServerOptionsSelectionCallback(SslStream stream, SslClientHelloInfo clientHelloInfo, object? state, CancellationToken cancellationToken); + // Internal versions of the above delegates. internal delegate bool RemoteCertValidationCallback(string? host, X509Certificate2? certificate, X509Chain? chain, SslPolicyErrors sslPolicyErrors); internal delegate X509Certificate LocalCertSelectionCallback(string targetHost, X509CertificateCollection localCertificates, X509Certificate2? remoteCertificate, string[] acceptableIssuers); @@ -454,6 +456,12 @@ private Task AuthenticateAsServerApm(SslServerAuthenticationOptions sslServerAut return ProcessAuthentication(true, true, cancellationToken)!; } + public Task AuthenticateAsServerAsync( ServerOptionsSelectionCallback optionCallback, object? state, CancellationToken cancellationToken = default) + { + ValidateCreateContext(new SslAuthenticationOptions(optionCallback, state)); + return ProcessAuthentication(true, false, cancellationToken)!; + } + public virtual Task ShutdownAsync() { ThrowIfExceptionalOrNotAuthenticatedOrShutdown(); diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs index b3375ba8d311e..c07a91861ffb4 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.ComponentModel; +using System.IO; using System.Net.Sockets; using System.Net.Test.Common; using System.Security.Authentication; @@ -70,6 +71,94 @@ public async Task ServerAsyncAuthenticate_AllClientVsIndividualServerSupportedPr await ServerAsyncSslHelper(SslProtocolSupport.SupportedSslProtocols, serverProtocol); } + [Fact] + public async Task ServerAsyncAuthenticate_SimpleSniOptions_Success() + { + var state = new Object(); + var serverOptions = new SslServerAuthenticationOptions() { ServerCertificate = _serverCertificate }; + var clientOptions = new SslClientAuthenticationOptions() { TargetHost = _serverCertificate.GetNameInfo(X509NameType.SimpleName, false) }; + clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true; + + (SslStream client, SslStream server) = TestHelper.GetConnectedSslStreams(); + using (client) + using (server) + { + Task t1 = client.AuthenticateAsClientAsync(clientOptions, default); + Task t2 = server.AuthenticateAsServerAsync( + (stream, clientHelloInfo, userState, cancellationToken) => + { + Assert.Equal(server, stream); + Assert.Equal(clientOptions.TargetHost, clientHelloInfo.ServerName); + Assert.True(object.ReferenceEquals(state, userState)); + return new ValueTask(serverOptions); + }, + state, default); + + await TestConfiguration.WhenAllOrAnyFailedWithTimeout(t1, t2); + } + } + + private async Task FailedTask() + { + await Task.Delay(10); + throw new InvalidOperationException("foo"); + } + + private async Task OptionsTask(SslServerAuthenticationOptions value) + { + await Task.Delay(10); + return value; + } + + [Fact] + public async Task ServerAsyncAuthenticate_AsyncOptions_Success() + { + var state = new Object(); + var serverOptions = new SslServerAuthenticationOptions() { ServerCertificate = _serverCertificate }; + var clientOptions = new SslClientAuthenticationOptions() { TargetHost = _serverCertificate.GetNameInfo(X509NameType.SimpleName, false) }; + clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true; + + (SslStream client, SslStream server) = TestHelper.GetConnectedSslStreams(); + using (client) + using (server) + { + Task t1 = client.AuthenticateAsClientAsync(clientOptions, default); + Task t2 = server.AuthenticateAsServerAsync( + (stream, clientHelloInfo, userState, cancellationToken) => + { + Assert.Equal(server, stream); + Assert.Equal(clientOptions.TargetHost, clientHelloInfo.ServerName); + Assert.True(object.ReferenceEquals(state, userState)); + return new ValueTask(OptionsTask(serverOptions)); + }, + state, default); + + await TestConfiguration.WhenAllOrAnyFailedWithTimeout(t1, t2); + } + } + + [Fact] + public async Task ServerAsyncAuthenticate_FailingOptionCallback_Throws() + { + var serverOptions = new SslServerAuthenticationOptions() { ServerCertificate = _serverCertificate }; + var clientOptions = new SslClientAuthenticationOptions() { TargetHost = _serverCertificate.GetNameInfo(X509NameType.SimpleName, false) }; + clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true; + + (SslStream client, SslStream server) = TestHelper.GetConnectedSslStreams(); + using (client) + using (server) + { + Task t1 = client.AuthenticateAsClientAsync(clientOptions, default); + Task t2 = server.AuthenticateAsServerAsync( + (stream, clientHelloInfo, userState, cancellationToken) => + { + return new ValueTask(FailedTask()); + }, + null, default); + await Assert.ThrowsAsync(() => t2); + } + } + public static IEnumerable ProtocolMismatchData() { if (PlatformDetection.SupportsSsl3) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs index 5d95d0826ec62..0d8b73d81351d 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/TestHelper.cs @@ -14,6 +14,12 @@ namespace System.Net.Security.Tests { public static class TestHelper { + public static (SslStream ClientStream, SslStream ServerStream) GetConnectedSslStreams() + { + (Stream clientStream, Stream serverStream) = GetConnectedStreams(); + return (new SslStream(clientStream), new SslStream(serverStream)); + } + public static (Stream ClientStream, Stream ServerStream) GetConnectedStreams() { if (Capability.SecurityForceSocketStreams()) From 920548510ae10f1b4cd39ad635b3b057bfc546a2 Mon Sep 17 00:00:00 2001 From: wfurt Date: Fri, 3 Jul 2020 09:59:23 -0700 Subject: [PATCH 2/8] remove System.IO --- .../tests/FunctionalTests/ServerAsyncAuthenticateTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs index c07a91861ffb4..503b54878cce6 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.ComponentModel; -using System.IO; using System.Net.Sockets; using System.Net.Test.Common; using System.Security.Authentication; From 3b9586919291567ddb1949cf95a0f2b5d2273b30 Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 6 Jul 2020 19:23:51 -0700 Subject: [PATCH 3/8] add missing file --- .../System/Net/Security/SslClientHelloInfo.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 src/libraries/System.Net.Security/src/System/Net/Security/SslClientHelloInfo.cs diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslClientHelloInfo.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslClientHelloInfo.cs new file mode 100644 index 0000000000000..a4316f0c27faa --- /dev/null +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslClientHelloInfo.cs @@ -0,0 +1,20 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Security.Authentication; + +namespace System.Net.Security +{ + public readonly struct SslClientHelloInfo + { + public readonly string ServerName { get; } + public readonly SslProtocols SslProtocols { get; } + + internal SslClientHelloInfo(string serverName, SslProtocols sslProtocols) + { + ServerName = serverName; + SslProtocols = sslProtocols; + } + } +} From 804e0ffe460085287737b350a86e501671765e9d Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 8 Jul 2020 13:35:33 -0700 Subject: [PATCH 4/8] more tests --- .../src/System/Net/Security/SslStream.cs | 2 +- .../ServerAsyncAuthenticateTest.cs | 71 ++++++++++++++++++- 2 files changed, 69 insertions(+), 4 deletions(-) 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 bbd745a03d3ab..3703556c47fa1 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 @@ -456,7 +456,7 @@ private Task AuthenticateAsServerApm(SslServerAuthenticationOptions sslServerAut return ProcessAuthentication(true, true, cancellationToken)!; } - public Task AuthenticateAsServerAsync( ServerOptionsSelectionCallback optionCallback, object? state, CancellationToken cancellationToken = default) + public Task AuthenticateAsServerAsync(ServerOptionsSelectionCallback optionCallback, object? state, CancellationToken cancellationToken = default) { ValidateCreateContext(new SslAuthenticationOptions(optionCallback, state)); return ProcessAuthentication(true, false, cancellationToken)!; diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs index 503b54878cce6..eb1ebfb614b8e 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs @@ -97,6 +97,35 @@ public async Task ServerAsyncAuthenticate_SimpleSniOptions_Success() } } + [Theory] + [InlineData(SslProtocols.Tls11)] + [InlineData(SslProtocols.Tls12)] + public async Task ServerAsyncAuthenticate_SniSetVersion_Success(SslProtocols version) + { + var serverOptions = new SslServerAuthenticationOptions() { ServerCertificate = _serverCertificate, EnabledSslProtocols = version }; + var clientOptions = new SslClientAuthenticationOptions() { TargetHost = _serverCertificate.GetNameInfo(X509NameType.SimpleName, false) }; + clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true; + + (SslStream client, SslStream server) = TestHelper.GetConnectedSslStreams(); + using (client) + using (server) + { + Task t1 = client.AuthenticateAsClientAsync(clientOptions, default); + Task t2 = server.AuthenticateAsServerAsync( + (stream, clientHelloInfo, userState, cancellationToken) => + { + Assert.Equal(server, stream); + Assert.Equal(clientOptions.TargetHost, clientHelloInfo.ServerName); + return new ValueTask(serverOptions); + }, + null, default); + + await TestConfiguration.WhenAllOrAnyFailedWithTimeout(t1, t2); + // Verify that the SNI callback can impact version. + Assert.Equal(version, client.SslProtocol); + } + } + private async Task FailedTask() { await Task.Delay(10); @@ -136,8 +165,10 @@ public async Task ServerAsyncAuthenticate_AsyncOptions_Success() } } - [Fact] - public async Task ServerAsyncAuthenticate_FailingOptionCallback_Throws() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ServerAsyncAuthenticate_FailingOptionCallback_Throws(bool useAsync) { var serverOptions = new SslServerAuthenticationOptions() { ServerCertificate = _serverCertificate }; var clientOptions = new SslClientAuthenticationOptions() { TargetHost = _serverCertificate.GetNameInfo(X509NameType.SimpleName, false) }; @@ -151,13 +182,47 @@ public async Task ServerAsyncAuthenticate_FailingOptionCallback_Throws() Task t2 = server.AuthenticateAsServerAsync( (stream, clientHelloInfo, userState, cancellationToken) => { - return new ValueTask(FailedTask()); + if (useAsync) + { + return new ValueTask(FailedTask()); + } + + throw new InvalidOperationException("foo"); }, null, default); await Assert.ThrowsAsync(() => t2); } } + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ServerAsyncAuthenticate_NoCertificate_Throws(bool useAsync) + { + var serverOptions = new SslServerAuthenticationOptions(); + var clientOptions = new SslClientAuthenticationOptions() { TargetHost = _serverCertificate.GetNameInfo(X509NameType.SimpleName, false) }; + clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true; + + (SslStream client, SslStream server) = TestHelper.GetConnectedSslStreams(); + using (client) + using (server) + { + Task t1 = client.AuthenticateAsClientAsync(clientOptions, default); + Task t2 = server.AuthenticateAsServerAsync( + (stream, clientHelloInfo, userState, cancellationToken) => + { + if (useAsync) + { + return new ValueTask(serverOptions); + } + + return new ValueTask(OptionsTask(serverOptions)); + }, + null, default); + await Assert.ThrowsAsync(() => t2); + } + } + public static IEnumerable ProtocolMismatchData() { if (PlatformDetection.SupportsSsl3) From fbfedd826f80788db7a37886da88a688d18f4c29 Mon Sep 17 00:00:00 2001 From: wfurt Date: Fri, 10 Jul 2020 13:45:36 -0700 Subject: [PATCH 5/8] feedback from review --- .../ref/System.Net.Security.cs | 3 +-- .../Net/Security/SslAuthenticationOptions.cs | 5 ++--- .../Net/Security/SslStream.Implementation.cs | 17 ++++------------- .../src/System/Net/Security/SslStream.cs | 6 +++--- .../ServerAsyncAuthenticateTest.cs | 4 ++-- 5 files changed, 12 insertions(+), 23 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 20e61ad4eb860..3ef7ad8ab16fd 100644 --- a/src/libraries/System.Net.Security/ref/System.Net.Security.cs +++ b/src/libraries/System.Net.Security/ref/System.Net.Security.cs @@ -113,7 +113,6 @@ public readonly struct SslClientHelloInfo public delegate bool RemoteCertificateValidationCallback(object sender, System.Security.Cryptography.X509Certificates.X509Certificate? certificate, System.Security.Cryptography.X509Certificates.X509Chain? chain, System.Net.Security.SslPolicyErrors sslPolicyErrors); public delegate System.Security.Cryptography.X509Certificates.X509Certificate ServerCertificateSelectionCallback(object sender, string? hostName); public delegate System.Threading.Tasks.ValueTask ServerOptionsSelectionCallback(SslStream stream, SslClientHelloInfo clientHelloInfo, object? state, System.Threading.CancellationToken cancellationToken); - public readonly partial struct SslApplicationProtocol : System.IEquatable { private readonly object _dummy; @@ -244,7 +243,7 @@ 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 System.Threading.Tasks.Task AuthenticateAsServerAsync( ServerOptionsSelectionCallback optionCallback, object? state, System.Threading.CancellationToken cancellationToken = default) { throw null; } + public System.Threading.Tasks.Task AuthenticateAsServerAsync(ServerOptionsSelectionCallback optionsCallback, object? state, System.Threading.CancellationToken cancellationToken = default) { throw null; } } [System.CLSCompliantAttribute(false)] public enum TlsCipherSuite : ushort diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs index 67c19c6899b16..579795aafa0b3 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs @@ -104,9 +104,8 @@ internal void UpdateOptions(SslServerAuthenticationOptions sslServerAuthenticati } else if (sslServerAuthenticationOptions.ServerCertificate != null) { - X509Certificate2? certificateWithKey = sslServerAuthenticationOptions.ServerCertificate as X509Certificate2; - - if (certificateWithKey != null && certificateWithKey.HasPrivateKey) + if (sslServerAuthenticationOptions.ServerCertificate is X509Certificate2 certificateWithKey && + certificateWithKey.HasPrivateKey) { // given cert is X509Certificate2 with key. We can use it directly. CertificateContext = SslStreamCertificateContext.Create(certificateWithKey); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs index 31350afe608fc..9631edaa2b612 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs @@ -429,19 +429,10 @@ private async ValueTask ReceiveBlobAsync(TIOAdapter a if (_sslAuthenticationOptions.ServerOptionDelegate != null) { - ValueTask t = - _sslAuthenticationOptions.ServerOptionDelegate(this, new SslClientHelloInfo(_lastFrame.TargetName, _lastFrame.SupportedVersions), - _sslAuthenticationOptions.UserState, adapter.CancellationToken); - - if (t.IsCompletedSuccessfully) - { - _sslAuthenticationOptions.UpdateOptions(t.Result); - } - else - { - SslServerAuthenticationOptions userOptions = await t.ConfigureAwait(false); - _sslAuthenticationOptions.UpdateOptions(userOptions); - } + SslServerAuthenticationOptions userOptions = + await _sslAuthenticationOptions.ServerOptionDelegate(this, new SslClientHelloInfo(_lastFrame.TargetName, _lastFrame.SupportedVersions), + _sslAuthenticationOptions.UserState, adapter.CancellationToken).ConfigureAwait(false); + _sslAuthenticationOptions.UpdateOptions(userOptions); } } 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 eaba72de8be22..f7165d6c1ae4b 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 @@ -455,10 +455,10 @@ private Task AuthenticateAsServerApm(SslServerAuthenticationOptions sslServerAut return ProcessAuthentication(true, true, cancellationToken)!; } - public Task AuthenticateAsServerAsync(ServerOptionsSelectionCallback optionCallback, object? state, CancellationToken cancellationToken = default) + public Task AuthenticateAsServerAsync(ServerOptionsSelectionCallback optionsCallback, object? state, CancellationToken cancellationToken = default) { - ValidateCreateContext(new SslAuthenticationOptions(optionCallback, state)); - return ProcessAuthentication(true, false, cancellationToken)!; + ValidateCreateContext(new SslAuthenticationOptions(optionsCallback, state)); + return ProcessAuthentication(isAsync: true, isApm: false, cancellationToken)!; } public virtual Task ShutdownAsync() diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs index 76f884bf75742..b5c1b18453f4b 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs @@ -127,13 +127,13 @@ public async Task ServerAsyncAuthenticate_SniSetVersion_Success(SslProtocols ver private async Task FailedTask() { - await Task.Delay(10); + await Task.Yield(); throw new InvalidOperationException("foo"); } private async Task OptionsTask(SslServerAuthenticationOptions value) { - await Task.Delay(10); + await Task.Yield(); return value; } From 6cff50fc3bd026f2f79c82145188e544c48379ee Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 13 Jul 2020 11:26:10 -0700 Subject: [PATCH 6/8] feedback from review --- .../Net/Security/SslAuthenticationOptions.cs | 15 +++++------ .../System/Net/Security/SslClientHelloInfo.cs | 3 +++ .../ServerAsyncAuthenticateTest.cs | 27 ++++++++++--------- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs index 60b80e18d1a2b..e229ddd0e72e2 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs @@ -102,14 +102,11 @@ internal void UpdateOptions(SslServerAuthenticationOptions sslServerAuthenticati { CertificateContext = sslServerAuthenticationOptions.ServerCertificateContext; } - else if (sslServerAuthenticationOptions.ServerCertificate != null) - { - if (sslServerAuthenticationOptions.ServerCertificate is X509Certificate2 certificateWithKey && + else if (sslServerAuthenticationOptions.ServerCertificate is X509Certificate2 certificateWithKey && certificateWithKey.HasPrivateKey) - { - // given cert is X509Certificate2 with key. We can use it directly. - CertificateContext = SslStreamCertificateContext.Create(certificateWithKey); - } + { + // given cert is X509Certificate2 with key. We can use it directly. + CertificateContext = SslStreamCertificateContext.Create(certificateWithKey); } } @@ -143,7 +140,7 @@ private static SslProtocols FilterOutIncompatibleSslProtocols(SslProtocols proto internal LocalCertSelectionCallback? CertSelectionDelegate { get; set; } internal ServerCertSelectionCallback? ServerCertSelectionDelegate { get; set; } internal CipherSuitesPolicy? CipherSuitesPolicy { get; set; } - internal object? UserState { get; set; } - internal ServerOptionsSelectionCallback? ServerOptionDelegate { get; set; } + internal object? UserState { get; } + internal ServerOptionsSelectionCallback? ServerOptionDelegate { get; } } } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslClientHelloInfo.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslClientHelloInfo.cs index a4316f0c27faa..16a19935c206f 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslClientHelloInfo.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslClientHelloInfo.cs @@ -6,6 +6,9 @@ namespace System.Net.Security { + /// + /// This struct contains information from received TLS Client Hello frame. + /// public readonly struct SslClientHelloInfo { public readonly string ServerName { get; } diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs index b5c1b18453f4b..fc01763a90306 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs @@ -7,6 +7,7 @@ using System.Net.Test.Common; using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; +using System.Threading; using System.Threading.Tasks; using Xunit; @@ -72,7 +73,7 @@ public async Task ServerAsyncAuthenticate_AllClientVsIndividualServerSupportedPr [Fact] public async Task ServerAsyncAuthenticate_SimpleSniOptions_Success() { - var state = new Object(); + var state = new object(); var serverOptions = new SslServerAuthenticationOptions() { ServerCertificate = _serverCertificate }; var clientOptions = new SslClientAuthenticationOptions() { TargetHost = _serverCertificate.GetNameInfo(X509NameType.SimpleName, false) }; clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true; @@ -81,7 +82,7 @@ public async Task ServerAsyncAuthenticate_SimpleSniOptions_Success() using (client) using (server) { - Task t1 = client.AuthenticateAsClientAsync(clientOptions, default); + Task t1 = client.AuthenticateAsClientAsync(clientOptions, CancellationToken.None); Task t2 = server.AuthenticateAsServerAsync( (stream, clientHelloInfo, userState, cancellationToken) => { @@ -90,7 +91,7 @@ public async Task ServerAsyncAuthenticate_SimpleSniOptions_Success() Assert.True(object.ReferenceEquals(state, userState)); return new ValueTask(serverOptions); }, - state, default); + state, CancellationToken.None); await TestConfiguration.WhenAllOrAnyFailedWithTimeout(t1, t2); } @@ -102,14 +103,14 @@ public async Task ServerAsyncAuthenticate_SimpleSniOptions_Success() public async Task ServerAsyncAuthenticate_SniSetVersion_Success(SslProtocols version) { var serverOptions = new SslServerAuthenticationOptions() { ServerCertificate = _serverCertificate, EnabledSslProtocols = version }; - var clientOptions = new SslClientAuthenticationOptions() { TargetHost = _serverCertificate.GetNameInfo(X509NameType.SimpleName, false) }; + var clientOptions = new SslClientAuthenticationOptions() { TargetHost = _serverCertificate.GetNameInfo(X509NameType.SimpleName, forIssuer: false) }; clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true; (SslStream client, SslStream server) = TestHelper.GetConnectedSslStreams(); using (client) using (server) { - Task t1 = client.AuthenticateAsClientAsync(clientOptions, default); + Task t1 = client.AuthenticateAsClientAsync(clientOptions, CancellationToken.None); Task t2 = server.AuthenticateAsServerAsync( (stream, clientHelloInfo, userState, cancellationToken) => { @@ -117,7 +118,7 @@ public async Task ServerAsyncAuthenticate_SniSetVersion_Success(SslProtocols ver Assert.Equal(clientOptions.TargetHost, clientHelloInfo.ServerName); return new ValueTask(serverOptions); }, - null, default); + null, CancellationToken.None); await TestConfiguration.WhenAllOrAnyFailedWithTimeout(t1, t2); // Verify that the SNI callback can impact version. @@ -140,7 +141,7 @@ private async Task OptionsTask(SslServerAuthenti [Fact] public async Task ServerAsyncAuthenticate_AsyncOptions_Success() { - var state = new Object(); + var state = new object(); var serverOptions = new SslServerAuthenticationOptions() { ServerCertificate = _serverCertificate }; var clientOptions = new SslClientAuthenticationOptions() { TargetHost = _serverCertificate.GetNameInfo(X509NameType.SimpleName, false) }; clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true; @@ -149,7 +150,7 @@ public async Task ServerAsyncAuthenticate_AsyncOptions_Success() using (client) using (server) { - Task t1 = client.AuthenticateAsClientAsync(clientOptions, default); + Task t1 = client.AuthenticateAsClientAsync(clientOptions, CancellationToken.None); Task t2 = server.AuthenticateAsServerAsync( (stream, clientHelloInfo, userState, cancellationToken) => { @@ -158,7 +159,7 @@ public async Task ServerAsyncAuthenticate_AsyncOptions_Success() Assert.True(object.ReferenceEquals(state, userState)); return new ValueTask(OptionsTask(serverOptions)); }, - state, default); + state, CancellationToken.None); await TestConfiguration.WhenAllOrAnyFailedWithTimeout(t1, t2); } @@ -177,7 +178,7 @@ public async Task ServerAsyncAuthenticate_FailingOptionCallback_Throws(bool useA using (client) using (server) { - Task t1 = client.AuthenticateAsClientAsync(clientOptions, default); + Task t1 = client.AuthenticateAsClientAsync(clientOptions, CancellationToken.None); Task t2 = server.AuthenticateAsServerAsync( (stream, clientHelloInfo, userState, cancellationToken) => { @@ -188,7 +189,7 @@ public async Task ServerAsyncAuthenticate_FailingOptionCallback_Throws(bool useA throw new InvalidOperationException("foo"); }, - null, default); + null, CancellationToken.None); await Assert.ThrowsAsync(() => t2); } } @@ -206,7 +207,7 @@ public async Task ServerAsyncAuthenticate_NoCertificate_Throws(bool useAsync) using (client) using (server) { - Task t1 = client.AuthenticateAsClientAsync(clientOptions, default); + Task t1 = client.AuthenticateAsClientAsync(clientOptions, CancellationToken.None); Task t2 = server.AuthenticateAsServerAsync( (stream, clientHelloInfo, userState, cancellationToken) => { @@ -217,7 +218,7 @@ public async Task ServerAsyncAuthenticate_NoCertificate_Throws(bool useAsync) return new ValueTask(OptionsTask(serverOptions)); }, - null, default); + null, CancellationToken.None); await Assert.ThrowsAsync(() => t2); } } From 738b84939a232c653c7601a6bdf193528ca34210 Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 13 Jul 2020 21:03:55 -0700 Subject: [PATCH 7/8] skip SniSetVersion on win7 --- .../tests/FunctionalTests/ServerAsyncAuthenticateTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs index fc01763a90306..6ec74f6b81629 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs @@ -97,7 +97,7 @@ public async Task ServerAsyncAuthenticate_SimpleSniOptions_Success() } } - [Theory] + [ConditionalTheory(nameof(IsNotWindows7))] [InlineData(SslProtocols.Tls11)] [InlineData(SslProtocols.Tls12)] public async Task ServerAsyncAuthenticate_SniSetVersion_Success(SslProtocols version) From 6a2974ea8c0696c06695181f7859031aea0e4463 Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 13 Jul 2020 22:54:21 -0700 Subject: [PATCH 8/8] fix IsNotWindows7 --- .../tests/FunctionalTests/ServerAsyncAuthenticateTest.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs index 6ec74f6b81629..1518cfc3fa545 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs @@ -23,6 +23,8 @@ public class ServerAsyncAuthenticateTest : IDisposable private readonly ITestOutputHelper _logVerbose; private readonly X509Certificate2 _serverCertificate; + public static bool IsNotWindows7 => !PlatformDetection.IsWindows7; + public ServerAsyncAuthenticateTest(ITestOutputHelper output) { _log = output;