From 1019ad8453db22a69c87fd66453a12c77d274fe7 Mon Sep 17 00:00:00 2001 From: Tomas Weinfurt Date: Wed, 8 Feb 2023 12:14:31 -0800 Subject: [PATCH] don't send server_name when literal IP (#81631) * don't send server_name when literal IP * fix unit test --- .../src/System.Net.Security.csproj | 7 +++ .../Net/Security/SslAuthenticationOptions.cs | 46 ++++++++++++++++++- .../tests/FunctionalTests/SslStreamSniTest.cs | 25 ++++++++++ .../System.Net.Security.Unit.Tests.csproj | 7 +++ 4 files changed, 83 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 ca5f00eea13ede..bc44d7a699b23a 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 261b356472bcf1..047bf5c87d5f5d 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; @@ -47,10 +83,16 @@ internal void UpdateOptions(SslClientAuthenticationOptions sslClientAuthenticati IsServer = false; RemoteCertRequired = true; CertificateContext = sslClientAuthenticationOptions.ClientCertificateContext; - // 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 944ab95699cce6..ced88b12ebce46 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) { 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" /> + + + +