From 3502cf35e0dffb06a3eb815813e4d76d5ebd88de Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 20 Jul 2020 16:50:03 -0700 Subject: [PATCH 1/4] use empty server name --- .../Interop.Ssl.cs | 2 +- .../Net/Security/SslStream.Implementation.cs | 11 +++++-- .../ServerAsyncAuthenticateTest.cs | 2 ++ .../SslStreamNetworkStreamTest.cs | 30 +++---------------- 4 files changed, 15 insertions(+), 30 deletions(-) diff --git a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Ssl.cs b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Ssl.cs index 4401dec463c544..577ee8126829ae 100644 --- a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Ssl.cs @@ -384,7 +384,7 @@ public static bool SslCheckHostnameMatch(SafeSslHandle handle, string hostName, // this code could be removed. // // It was verified as supporting case invariant match as of 10.12.1 (Sierra). - string matchName = s_idnMapping.GetAscii(hostName); + string matchName = string.IsNullOrEmpty(hostName) ? string.Empty : s_idnMapping.GetAscii(hostName); using (SafeCFDateHandle cfNotBefore = CoreFoundation.CFDateCreate(notBefore)) using (SafeCreateHandle cfHostname = CoreFoundation.CFStringCreateWithCString(matchName)) 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 afda3eb1de5e75..748aafbdb8b141 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 @@ -15,7 +15,7 @@ namespace System.Net.Security { public partial class SslStream { - private static int s_uniqueNameInteger = 123; +// private static int s_uniqueNameInteger = 123; private SslAuthenticationOptions? _sslAuthenticationOptions; @@ -66,10 +66,12 @@ private void ValidateCreateContext(SslClientAuthenticationOptions sslClientAuthe try { _sslAuthenticationOptions = new SslAuthenticationOptions(sslClientAuthenticationOptions, remoteCallback, localCallback); + /* if (_sslAuthenticationOptions.TargetHost!.Length == 0) { _sslAuthenticationOptions.TargetHost = "?" + Interlocked.Increment(ref s_uniqueNameInteger).ToString(NumberFormatInfo.InvariantInfo); } + */ _context = new SecureChannel(_sslAuthenticationOptions, this); } catch (Win32Exception e) @@ -420,12 +422,15 @@ private async ValueTask ReceiveBlobAsync(TIOAdapter a if (_lastFrame.HandshakeType == TlsHandshakeType.ClientHello) { // SNI if it exist. Even if we could not parse the hello, we can fall-back to default certificate. - _sslAuthenticationOptions!.TargetHost = _lastFrame.TargetName; + if (_lastFrame.TargetName != null) + { + _sslAuthenticationOptions!.TargetHost = _lastFrame.TargetName; + } if (_sslAuthenticationOptions.ServerOptionDelegate != null) { SslServerAuthenticationOptions userOptions = - await _sslAuthenticationOptions.ServerOptionDelegate(this, new SslClientHelloInfo(_lastFrame.TargetName, _lastFrame.SupportedVersions), + await _sslAuthenticationOptions.ServerOptionDelegate(this, new SslClientHelloInfo(_sslAuthenticationOptions!.TargetHost, _lastFrame.SupportedVersions), _sslAuthenticationOptions.UserState, adapter.CancellationToken).ConfigureAwait(false); _sslAuthenticationOptions.UpdateOptions(userOptions); } diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs index 6695fc966ae5ab..b05a918ee98975 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; diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs index fffc62a5a36ee2..586f9c61023af7 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs @@ -223,14 +223,7 @@ public async Task SslStream_TargetHostName_Succeeds(bool useEmptyName) (sender, certificate, chain, sslPolicyErrors) => { SslStream stream = (SslStream)sender; - if (useEmptyName) - { - Assert.Equal('?', stream.TargetHostName[0]); - } - else - { - Assert.Equal(tagetName, stream.TargetHostName); - } + Assert.Equal(tagetName, stream.TargetHostName); return true; }; @@ -240,14 +233,7 @@ public async Task SslStream_TargetHostName_Succeeds(bool useEmptyName) (sender, name) => { SslStream stream = (SslStream)sender; - if (useEmptyName) - { - Assert.Equal('?', stream.TargetHostName[0]); - } - else - { - Assert.Equal(tagetName, stream.TargetHostName); - } + Assert.Equal(tagetName, stream.TargetHostName); return certificate; }; @@ -255,16 +241,8 @@ public async Task SslStream_TargetHostName_Succeeds(bool useEmptyName) await TestConfiguration.WhenAllOrAnyFailedWithTimeout( client.AuthenticateAsClientAsync(clientOptions), server.AuthenticateAsServerAsync(serverOptions)); - if (useEmptyName) - { - Assert.Equal('?', client.TargetHostName[0]); - Assert.Equal('?', server.TargetHostName[0]); - } - else - { - Assert.Equal(tagetName, client.TargetHostName); - Assert.Equal(tagetName, server.TargetHostName); - } + Assert.Equal(tagetName, client.TargetHostName); + Assert.Equal(tagetName, server.TargetHostName); } } From d92c2d434cd98b92833e9cd8c46b658dd6839316 Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 20 Jul 2020 16:53:28 -0700 Subject: [PATCH 2/4] fix merge --- .../src/System/Net/Security/SslStream.Implementation.cs | 8 -------- .../tests/FunctionalTests/ServerAsyncAuthenticateTest.cs | 2 -- 2 files changed, 10 deletions(-) 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 748aafbdb8b141..849bbd66e33b33 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 @@ -15,8 +15,6 @@ namespace System.Net.Security { public partial class SslStream { -// private static int s_uniqueNameInteger = 123; - private SslAuthenticationOptions? _sslAuthenticationOptions; private int _nestedAuth; @@ -66,12 +64,6 @@ private void ValidateCreateContext(SslClientAuthenticationOptions sslClientAuthe try { _sslAuthenticationOptions = new SslAuthenticationOptions(sslClientAuthenticationOptions, remoteCallback, localCallback); - /* - if (_sslAuthenticationOptions.TargetHost!.Length == 0) - { - _sslAuthenticationOptions.TargetHost = "?" + Interlocked.Increment(ref s_uniqueNameInteger).ToString(NumberFormatInfo.InvariantInfo); - } - */ _context = new SecureChannel(_sslAuthenticationOptions, this); } catch (Win32Exception e) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs index b05a918ee98975..6695fc966ae5ab 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs @@ -23,8 +23,6 @@ public class ServerAsyncAuthenticateTest : IDisposable private readonly ITestOutputHelper _logVerbose; private readonly X509Certificate2 _serverCertificate; - public static bool IsNotWindows7 => !PlatformDetection.IsWindows7; - public ServerAsyncAuthenticateTest(ITestOutputHelper output) { _log = output; From 7986bee5b5a068289f0cc6e3dd37fc8cadca212b Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 20 Jul 2020 17:36:32 -0700 Subject: [PATCH 3/4] feedback from review --- .../System/Net/Security/SslStream.Implementation.cs | 2 +- .../FunctionalTests/SslStreamNetworkStreamTest.cs | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) 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 849bbd66e33b33..5f3c5cdff31eb8 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 @@ -422,7 +422,7 @@ private async ValueTask ReceiveBlobAsync(TIOAdapter a if (_sslAuthenticationOptions.ServerOptionDelegate != null) { SslServerAuthenticationOptions userOptions = - await _sslAuthenticationOptions.ServerOptionDelegate(this, new SslClientHelloInfo(_sslAuthenticationOptions!.TargetHost, _lastFrame.SupportedVersions), + await _sslAuthenticationOptions.ServerOptionDelegate(this, new SslClientHelloInfo(_sslAuthenticationOptions.TargetHost, _lastFrame.SupportedVersions), _sslAuthenticationOptions.UserState, adapter.CancellationToken).ConfigureAwait(false); _sslAuthenticationOptions.UpdateOptions(userOptions); } diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs index 586f9c61023af7..46345ba49d4262 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs @@ -205,7 +205,7 @@ public async Task SslStream_NestedAuth_Throws() [InlineData(true)] public async Task SslStream_TargetHostName_Succeeds(bool useEmptyName) { - string tagetName = useEmptyName ? string.Empty : Guid.NewGuid().ToString("N"); + string targetName = useEmptyName ? string.Empty : Guid.NewGuid().ToString("N"); (Stream clientStream, Stream serverStream) = TestHelper.GetConnectedStreams(); using (clientStream) @@ -218,12 +218,12 @@ public async Task SslStream_TargetHostName_Succeeds(bool useEmptyName) Assert.Equal(string.Empty, client.TargetHostName); Assert.Equal(string.Empty, server.TargetHostName); - SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions() { TargetHost = tagetName }; + SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions() { TargetHost = targetName }; clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => { SslStream stream = (SslStream)sender; - Assert.Equal(tagetName, stream.TargetHostName); + Assert.Equal(targetName, stream.TargetHostName); return true; }; @@ -233,7 +233,7 @@ public async Task SslStream_TargetHostName_Succeeds(bool useEmptyName) (sender, name) => { SslStream stream = (SslStream)sender; - Assert.Equal(tagetName, stream.TargetHostName); + Assert.Equal(targetName, stream.TargetHostName); return certificate; }; @@ -241,8 +241,8 @@ public async Task SslStream_TargetHostName_Succeeds(bool useEmptyName) await TestConfiguration.WhenAllOrAnyFailedWithTimeout( client.AuthenticateAsClientAsync(clientOptions), server.AuthenticateAsServerAsync(serverOptions)); - Assert.Equal(tagetName, client.TargetHostName); - Assert.Equal(tagetName, server.TargetHostName); + Assert.Equal(targetName, client.TargetHostName); + Assert.Equal(targetName, server.TargetHostName); } } From 1a370aaedec3d3c75b2eb8366756252b41aeeb5b Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 22 Jul 2020 12:41:56 -0700 Subject: [PATCH 4/4] add missing file --- .../Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 35f2309c34963c..4a8695f5d3d6fe 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -189,7 +189,7 @@ internal static SafeSslHandle AllocateSslContext(SslProtocols protocols, SafeX50 if (!sslAuthenticationOptions.IsServer) { // The IdnMapping converts unicode input into the IDNA punycode sequence. - string punyCode = s_idnMapping.GetAscii(sslAuthenticationOptions.TargetHost!); + string punyCode = string.IsNullOrEmpty(sslAuthenticationOptions.TargetHost) ? string.Empty : s_idnMapping.GetAscii(sslAuthenticationOptions.TargetHost!); // Similar to windows behavior, set SNI on openssl by default for client context, ignore errors. if (!Ssl.SslSetTlsExtHostName(context, punyCode))