Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 28, 2020
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3502cf3
use empty server name
wfurt Jul 20, 2020
d92c2d4
fix merge
wfurt Jul 20, 2020
7986bee
feedback from review
wfurt Jul 21, 2020
1a370aa
add missing file
wfurt Jul 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Ssl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
13 changes: 5 additions & 8 deletions src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ namespace System.Net.Security
{
public partial class SslStream
{
private static int s_uniqueNameInteger = 123;

private SslAuthenticationOptions? _sslAuthenticationOptions;

private int _nestedAuth;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
}
Expand Down
34 changes: 6 additions & 28 deletions src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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;
};
Expand All @@ -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);
}
}

Expand Down