From 1cb84841048963ed50b589bf7bb4d97f21789666 Mon Sep 17 00:00:00 2001 From: Tomas Weinfurt Date: Sun, 21 Jun 2020 12:58:24 -0700 Subject: [PATCH 1/3] add TargetHostName to SslStream --- .../ref/System.Net.Security.cs | 1 + .../Net/Security/SslAuthenticationOptions.cs | 4 +- .../src/System/Net/Security/SslStream.cs | 8 +++ .../SslStreamNetworkStreamTest.cs | 64 +++++++++++++++++++ 4 files changed, 75 insertions(+), 2 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 d657bb1fd59d79..4b2cb8f6feee7c 100644 --- a/src/libraries/System.Net.Security/ref/System.Net.Security.cs +++ b/src/libraries/System.Net.Security/ref/System.Net.Security.cs @@ -184,6 +184,7 @@ public partial class SslStream : System.Net.Security.AuthenticatedStream public override int ReadTimeout { get { throw null; } set { } } public virtual System.Security.Cryptography.X509Certificates.X509Certificate? RemoteCertificate { get { throw null; } } public virtual System.Security.Authentication.SslProtocols SslProtocol { get { throw null; } } + public string TargetHostName { 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) { } 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 4895d91c2e3967..2a36723fdce0c8 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 @@ -21,7 +21,7 @@ internal SslAuthenticationOptions(SslClientAuthenticationOptions sslClientAuthen EncryptionPolicy = sslClientAuthenticationOptions.EncryptionPolicy; IsServer = false; RemoteCertRequired = true; - TargetHost = sslClientAuthenticationOptions.TargetHost; + TargetHost = sslClientAuthenticationOptions.TargetHost!; // Client specific options. CertSelectionDelegate = localCallback; @@ -68,7 +68,7 @@ private static SslProtocols FilterOutIncompatibleSslProtocols(SslProtocols proto } internal bool AllowRenegotiation { get; set; } - internal string? TargetHost { get; set; } + internal string TargetHost { get; set; } internal X509CertificateCollection? ClientCertificates { get; set; } internal List? ApplicationProtocols { get; } internal bool IsServer { get; set; } 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 61011fde371c8e..c31e7762bdc271 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 @@ -655,6 +655,14 @@ public virtual int KeyExchangeStrength } } + public string TargetHostName + { + get + { + return _sslAuthenticationOptions != null ? _sslAuthenticationOptions.TargetHost : string.Empty; + } + } + // // Stream contract implementation. // diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs index 8aaf5439430471..adae5dd2afd479 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs @@ -193,6 +193,70 @@ public async Task SslStream_NestedAuth_Throws() } } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task SslStreag_TargetHostName_Succeeds(bool useEmptyName) + { + string tagetName = useEmptyName ? string.Empty : Guid.NewGuid().ToString("N"); + + (Stream clientStream, Stream serverStream) = TestHelper.GetConnectedStreams(); + using (clientStream) + using (serverStream) + using (var client = new SslStream(clientStream)) + using (var server = new SslStream(serverStream)) + using (X509Certificate2 certificate = Configuration.Certificates.GetServerCertificate()) + { + SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions() { TargetHost = tagetName }; + clientOptions.RemoteCertificateValidationCallback = + (sender, certificate, chain, sslPolicyErrors) => + { + SslStream stream = (SslStream)sender; + if (useEmptyName) + { + Assert.Equal('?', stream.TargetHostName[0]); + } + else + { + Assert.Equal(tagetName, stream.TargetHostName); + } + + return true; + }; + + SslServerAuthenticationOptions serverOptions = new SslServerAuthenticationOptions(); + serverOptions.ServerCertificateSelectionCallback = + (sender, name) => + { + SslStream stream = (SslStream)sender; + if (useEmptyName) + { + Assert.Equal('?', stream.TargetHostName[0]); + } + else + { + Assert.Equal(tagetName, 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); + } + } + } + private static bool ValidateServerCertificate( object sender, X509Certificate retrievedServerPublicCertificate, From 17a8f4ef379219c6469fa0c1ab424d98a5edddd8 Mon Sep 17 00:00:00 2001 From: Tomas Weinfurt Date: Mon, 22 Jun 2020 10:06:15 -0700 Subject: [PATCH 2/3] fix unit tests --- .../tests/UnitTests/Fakes/FakeSslStream.Implementation.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libraries/System.Net.Security/tests/UnitTests/Fakes/FakeSslStream.Implementation.cs b/src/libraries/System.Net.Security/tests/UnitTests/Fakes/FakeSslStream.Implementation.cs index ccd294beb54ddd..7a2c556170e55e 100644 --- a/src/libraries/System.Net.Security/tests/UnitTests/Fakes/FakeSslStream.Implementation.cs +++ b/src/libraries/System.Net.Security/tests/UnitTests/Fakes/FakeSslStream.Implementation.cs @@ -11,6 +11,13 @@ namespace System.Net.Security { public partial class SslStream { + private class FakeOptions + { + public string TargetHost; + } + + private FakeOptions? _sslAuthenticationOptions; + private void ValidateCreateContext(SslClientAuthenticationOptions sslClientAuthenticationOptions, RemoteCertValidationCallback remoteCallback, LocalCertSelectionCallback? localCallback) { // Without setting (or using) these members you will get a build exception in the unit test project. @@ -35,6 +42,7 @@ private void ValidateParameters(byte[] buffer, int offset, int count) private void ValidateCreateContext(SslAuthenticationOptions sslAuthenticationOptions) { + _sslAuthenticationOptions = new FakeOptions() { TargetHost = sslAuthenticationOptions.TargetHost }; } private ValueTask WriteAsyncInternal(TWriteAdapter writeAdapter, ReadOnlyMemory buffer) From 06103aa7d5ca5cd1ad8357c2f1cb811c717a70f4 Mon Sep 17 00:00:00 2001 From: Tomas Weinfurt Date: Wed, 24 Jun 2020 13:14:42 -0700 Subject: [PATCH 3/3] feedback from review --- .../src/System/Net/Security/SslAuthenticationOptions.cs | 3 +++ .../tests/FunctionalTests/SslStreamNetworkStreamTest.cs | 6 +++++- 2 files changed, 8 insertions(+), 1 deletion(-) 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 2a36723fdce0c8..d4366c973277b8 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 @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System.Collections.Generic; +using System.Diagnostics; using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; @@ -12,6 +13,8 @@ internal class SslAuthenticationOptions { internal SslAuthenticationOptions(SslClientAuthenticationOptions sslClientAuthenticationOptions, RemoteCertValidationCallback remoteCallback, LocalCertSelectionCallback? localCallback) { + Debug.Assert(sslClientAuthenticationOptions.TargetHost != null); + // Common options. AllowRenegotiation = sslClientAuthenticationOptions.AllowRenegotiation; ApplicationProtocols = sslClientAuthenticationOptions.ApplicationProtocols; diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs index adae5dd2afd479..e68ed5a3d4f650 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs @@ -196,7 +196,7 @@ public async Task SslStream_NestedAuth_Throws() [Theory] [InlineData(false)] [InlineData(true)] - public async Task SslStreag_TargetHostName_Succeeds(bool useEmptyName) + public async Task SslStream_TargetHostName_Succeeds(bool useEmptyName) { string tagetName = useEmptyName ? string.Empty : Guid.NewGuid().ToString("N"); @@ -207,6 +207,10 @@ public async Task SslStreag_TargetHostName_Succeeds(bool useEmptyName) using (var server = new SslStream(serverStream)) using (X509Certificate2 certificate = Configuration.Certificates.GetServerCertificate()) { + // It should be empty before handshake. + Assert.Equal(string.Empty, client.TargetHostName); + Assert.Equal(string.Empty, server.TargetHostName); + SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions() { TargetHost = tagetName }; clientOptions.RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) =>