From 14e2e0dc296c134320049e40116a6a581b45d3de Mon Sep 17 00:00:00 2001 From: wfurt Date: Fri, 3 Feb 2023 22:54:11 -0800 Subject: [PATCH 1/2] don't send server_name when literal IP --- .../src/System.Net.Security.csproj | 7 +++ .../Net/Security/SslAuthenticationOptions.cs | 46 ++++++++++++++++++- .../tests/FunctionalTests/SslStreamSniTest.cs | 25 ++++++++++ 3 files changed, 76 insertions(+), 2 deletions(-) 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 af280c25779ed9..56206142915281 100644 --- a/src/libraries/System.Net.Security/src/System.Net.Security.csproj +++ b/src/libraries/System.Net.Security/src/System.Net.Security.csproj @@ -55,6 +55,13 @@ + + + + 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 6113cecff667cf..e4bd2b32c8992b 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 @@ using System.Collections.Generic; using System.Diagnostics; +using System.Runtime.InteropServices; using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; @@ -10,6 +11,41 @@ namespace System.Net.Security { internal sealed class SslAuthenticationOptions { + // Simplified version of IPAddressParser.Parse to avoid allocations and dependencies. + // It purposely ignores scopeId as we don't really use so we do not need to map it to actual interface id. + private static unsafe bool IsValidAddress(ReadOnlySpan ipSpan) + { + int end = ipSpan.Length; + + if (ipSpan.Contains(':')) + { + // The address is parsed as IPv6 if and only if it contains a colon. This is valid because + // we don't support/parse a port specification at the end of an IPv4 address. + Span numbers = stackalloc ushort[IPAddressParserStatics.IPv6AddressShorts]; + + fixed (char* ipStringPtr = &MemoryMarshal.GetReference(ipSpan)) + { + return IPv6AddressHelper.IsValidStrict(ipStringPtr, 0, ref end); + } + } + else if (char.IsDigit(ipSpan[0])) + { + long tmpAddr; + + fixed (char* ipStringPtr = &MemoryMarshal.GetReference(ipSpan)) + { + tmpAddr = IPv4AddressHelper.ParseNonCanonical(ipStringPtr, 0, ref end, notImplicitFile: true); + } + + if (tmpAddr != IPv4AddressHelper.Invalid && end == ipSpan.Length) + { + return true; + } + } + + return false; + } + internal SslAuthenticationOptions() { TargetHost = string.Empty; @@ -46,10 +82,16 @@ internal void UpdateOptions(SslClientAuthenticationOptions sslClientAuthenticati EncryptionPolicy = sslClientAuthenticationOptions.EncryptionPolicy; IsServer = false; RemoteCertRequired = true; - // RFC 6066 section 3 says to exclude trailing dot from fully qualified DNS hostname - if (sslClientAuthenticationOptions.TargetHost != null) + if (!string.IsNullOrEmpty(sslClientAuthenticationOptions.TargetHost)) { + // RFC 6066 section 3 says to exclude trailing dot from fully qualified DNS hostname TargetHost = sslClientAuthenticationOptions.TargetHost.TrimEnd('.'); + + // RFC 6066 forbids IP literals + if (IsValidAddress(TargetHost)) + { + TargetHost = string.Empty; + } } // Client specific options. diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamSniTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamSniTest.cs index 9d4aa07ff92991..46eefb089d04df 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamSniTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamSniTest.cs @@ -172,6 +172,31 @@ await server.AuthenticateAsServerAsync(options, cts.Token) return true; }); } + [Theory] + [InlineData("127.0.0.1")] + [InlineData("::1")] + [InlineData("2001:11:22::1")] + [InlineData("fe80::9c3a:b64d:6249:1de8%2")] + [InlineData("fe80::9c3a:b64d:6249:1de8")] + public async Task SslStream_IpLiteral_NotSend(string target) + { + (SslStream client, SslStream server) = TestHelper.GetConnectedSslStreams(); + SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions() + { + TargetHost = target, + RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true, + }; + SslServerAuthenticationOptions serverOptions = new SslServerAuthenticationOptions() + { + ServerCertificate = Configuration.Certificates.GetServerCertificate(), + }; + + await TestConfiguration.WhenAllOrAnyFailedWithTimeout( + client.AuthenticateAsClientAsync(clientOptions, default), + server.AuthenticateAsServerAsync(serverOptions, default)); + + Assert.Equal(string.Empty, server.TargetHostName); + } private static Func WithAggregateExceptionUnwrapping(Func a) { From 2e91e530acaa44aa0bae43a371e13a3eb385651c Mon Sep 17 00:00:00 2001 From: wfurt Date: Sat, 4 Feb 2023 10:00:33 -0800 Subject: [PATCH 2/2] fix unit test --- .../tests/UnitTests/System.Net.Security.Unit.Tests.csproj | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libraries/System.Net.Security/tests/UnitTests/System.Net.Security.Unit.Tests.csproj b/src/libraries/System.Net.Security/tests/UnitTests/System.Net.Security.Unit.Tests.csproj index a2fa3f655114c8..2a2655ae1ffbcc 100644 --- a/src/libraries/System.Net.Security/tests/UnitTests/System.Net.Security.Unit.Tests.csproj +++ b/src/libraries/System.Net.Security/tests/UnitTests/System.Net.Security.Unit.Tests.csproj @@ -84,6 +84,13 @@ Link="Common\System\Threading\Tasks\TaskToApm.cs" /> + + + +