use empty server name is client did not specify one (#39671)

* use empty server name * fix merge * feedback from review * add missing file
dotnet · Jul 28, 2020 · 25c222b · 25c222b
1 parent 065bf96
commit 25c222b
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 38 deletions.
diff --git a/...libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Ssl.cs b/...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/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))

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
@@ -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,10 +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)
@@ -420,12 +414,15 @@ private async ValueTask<ProtocolToken> ReceiveBlobAsync<TIOAdapter>(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/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,19 +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;
-                        if (useEmptyName)
-                        {
-                            Assert.Equal('?', stream.TargetHostName[0]);
-                        }
-                        else
-                        {
-                            Assert.Equal(tagetName, stream.TargetHostName);
-                        }
+                        Assert.Equal(targetName, stream.TargetHostName);
 
                         return true;
                     };
@@ -240,31 +233,16 @@ 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(targetName, stream.TargetHostName);
 
                         return certificate;
                     };
 
                 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(targetName, client.TargetHostName);
+                Assert.Equal(targetName, server.TargetHostName);
             }
         }