diff --git a/source/Halibut/Transport/DiscoveryClient.cs b/source/Halibut/Transport/DiscoveryClient.cs index 075361118..297cb7e79 100644 --- a/source/Halibut/Transport/DiscoveryClient.cs +++ b/source/Halibut/Transport/DiscoveryClient.cs @@ -27,6 +27,8 @@ public async Task DiscoverAsync(ServiceEndPoint serviceEndpoint try { var log = logs.ForEndpoint(serviceEndpoint.BaseUri); + var streamClientAuthentication = new SslStreamClientAuthentication(serviceEndpoint.BaseUri.Host, new X509Certificate2Collection(), SslConfiguration.SupportedProtocols); + using (var client = await TcpConnectionFactory.CreateConnectedTcpClientAsync(serviceEndpoint, halibutTimeoutsAndLimits, streamFactory, log, cancellationToken)) { #if !NETFRAMEWORK @@ -39,17 +41,8 @@ public async Task DiscoverAsync(ServiceEndPoint serviceEndpoint #endif using (var ssl = new SslStream(networkTimeoutStream, false, ValidateCertificate)) { -#if NETFRAMEWORK - // TODO: ASYNC ME UP! - // AuthenticateAsClientAsync in .NET 4.8 does not support cancellation tokens. So `cancellationToken` is not respected here. - await ssl.AuthenticateAsClientAsync( - serviceEndpoint.BaseUri.Host, - new X509Certificate2Collection(), - SslConfiguration.SupportedProtocols, - false); -#else - await ssl.AuthenticateAsClientEnforcingTimeout(serviceEndpoint, new X509Certificate2Collection(), cancellationToken); -#endif + await streamClientAuthentication.AuthenticateAsClientAsync(ssl, cancellationToken); + await ssl.WriteAsync(HelloLine, 0, HelloLine.Length, cancellationToken); await ssl.FlushAsync(cancellationToken); diff --git a/source/Halibut/Transport/SecureListener.cs b/source/Halibut/Transport/SecureListener.cs index 59f666f84..73ba8b5c0 100644 --- a/source/Halibut/Transport/SecureListener.cs +++ b/source/Halibut/Transport/SecureListener.cs @@ -52,6 +52,7 @@ public class SecureListener : IAsyncDisposable TcpListener listener; Thread? backgroundThread; Task? backgroundTask; + readonly SslStreamServerAuthentication streamServerAuthentication; #pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable. public SecureListener( @@ -81,6 +82,13 @@ public SecureListener( this.halibutTimeoutsAndLimits = halibutTimeoutsAndLimits; this.streamFactory = streamFactory; this.connectionsObserver = connectionsObserver; + + this.streamServerAuthentication = new SslStreamServerAuthentication( + serverCertificate, + clientCertificateRequired: true, + SslConfiguration.SupportedProtocols, + checkCertificateRevocation: false); + this.cts = new CancellationTokenSource(); this.cancellationToken = cts.Token; @@ -301,14 +309,8 @@ async Task ExecuteRequest(TcpClient client) { log.Write(EventType.SecurityNegotiation, "Performing TLS server handshake"); - await ssl - .AuthenticateAsServerAsync( - serverCertificate, - true, - SslConfiguration.SupportedProtocols, - false) - .ConfigureAwait(false); - + await streamServerAuthentication.AuthenticateAsServerAsync(ssl, cancellationToken).ConfigureAwait(false); + log.Write(EventType.SecurityNegotiation, "Secure connection established, client is not yet authenticated, client connected with {0}", ssl.SslProtocol.ToString()); var req = await ReadInitialRequest(ssl); @@ -527,4 +529,4 @@ public async ValueTask DisposeAsync() log?.Write(EventType.ListenerStopped, "Listener stopped"); } } -} \ No newline at end of file +} diff --git a/source/Halibut/Transport/SslStreamClientAuthentication.cs b/source/Halibut/Transport/SslStreamClientAuthentication.cs new file mode 100644 index 000000000..7edd5e0d7 --- /dev/null +++ b/source/Halibut/Transport/SslStreamClientAuthentication.cs @@ -0,0 +1,74 @@ +// Copyright 2012-2013 Octopus Deploy Pty. Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +using System.Net.Security; +using System.Security.Authentication; +using System.Security.Cryptography.X509Certificates; +using System.Threading; +using System.Threading.Tasks; + +namespace Halibut.Transport +{ + // In .NET 8, SslClientAuthenticationOptions gained an SslStreamCertificateContext which deals with client certificates. + // In .NET 9, we saw signal that these contexts could leak the native OpenSSL context on linux when acting as a server. + // https://github.com/dotnet/runtime/issues/110803#issuecomment-2553658071 + // + // It was not mentioned, but logically it holds that if the Server context could leak, so could the Client one. + // SslClientAuthenticationOptions doesn't even exist in NetFramework, so this class provides a wrapper, enabling + // other code to use the same API for both .NET 4.8 and .NET 8+. + class SslStreamClientAuthentication + { + readonly string targetHost; + readonly X509Certificate2Collection clientCertificates; + readonly SslProtocols enabledSslProtocols; + +#if !NETFRAMEWORK + readonly SslClientAuthenticationOptions clientAuthenticationOptions; +#endif + + public SslStreamClientAuthentication(string targetHost, X509Certificate2Collection clientCertificates, SslProtocols enabledSslProtocols) + { + this.targetHost = targetHost; + this.clientCertificates = clientCertificates; + this.enabledSslProtocols = enabledSslProtocols; + +#if !NETFRAMEWORK + clientAuthenticationOptions = new SslClientAuthenticationOptions + { + TargetHost = targetHost, + ClientCertificates = clientCertificates, + EnabledSslProtocols = SslConfiguration.SupportedProtocols, + CertificateRevocationCheckMode = X509RevocationMode.NoCheck, + }; +#endif + } + + public async Task AuthenticateAsClientAsync(SslStream ssl, CancellationToken cancellationToken) + { +#if NETFRAMEWORK + // AuthenticateAsClientAsync in .NET 4.8 does not support cancellation tokens. So `cancellationToken` is not respected here. + await ssl.AuthenticateAsClientAsync( + targetHost, + clientCertificates, + enabledSslProtocols, + false); +#else + using var timeoutCts = new CancellationTokenSource(ssl.ReadTimeout); + using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(timeoutCts.Token, cancellationToken); + + await ssl.AuthenticateAsClientAsync(clientAuthenticationOptions, linkedCts.Token); +#endif + } + } +} \ No newline at end of file diff --git a/source/Halibut/Transport/SslStreamServerAuthentication.cs b/source/Halibut/Transport/SslStreamServerAuthentication.cs new file mode 100644 index 000000000..649cba084 --- /dev/null +++ b/source/Halibut/Transport/SslStreamServerAuthentication.cs @@ -0,0 +1,68 @@ +// Copyright 2012-2013 Octopus Deploy Pty. Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +using System.Net.Security; +using System.Security.Authentication; +using System.Security.Cryptography.X509Certificates; +using System.Threading; +using System.Threading.Tasks; + +namespace Halibut.Transport +{ + // In .NET 9+, we need to share a SslServerAuthenticationOptions and the underlying SslStreamCertificateContext + // between connections to resolve a memory leak. + // https://github.com/dotnet/runtime/issues/110803#issuecomment-2553658071 + // However these types don't exist in NetFramework. This facade + // allows SecureListener to do the right thing for each platform without a bunch of #if's + class SslStreamServerAuthentication + { + readonly X509Certificate2 certificate; + readonly bool clientCertificateRequired; + readonly SslProtocols enabledSslProtocols; + readonly bool checkCertificateRevocation; + +#if !NETFRAMEWORK + readonly SslServerAuthenticationOptions serverAuthenticationOptions; +#endif + + public SslStreamServerAuthentication(X509Certificate2 certificate, bool clientCertificateRequired, SslProtocols enabledSslProtocols, bool checkCertificateRevocation) + { + this.certificate = certificate; + this.clientCertificateRequired = clientCertificateRequired; + this.enabledSslProtocols = enabledSslProtocols; + this.checkCertificateRevocation = checkCertificateRevocation; + +#if !NETFRAMEWORK + serverAuthenticationOptions = new SslServerAuthenticationOptions + { + ServerCertificate = certificate, + ClientCertificateRequired = clientCertificateRequired, + EnabledSslProtocols = enabledSslProtocols, + CertificateRevocationCheckMode = checkCertificateRevocation ? X509RevocationMode.Online : X509RevocationMode.NoCheck, + EncryptionPolicy = EncryptionPolicy.RequireEncryption + }; +#endif + } + + public Task AuthenticateAsServerAsync(SslStream ssl, CancellationToken cancellationToken) + { +#if NETFRAMEWORK + // NetFramework doesn't support cancellation here + return ssl.AuthenticateAsServerAsync(certificate, clientCertificateRequired, enabledSslProtocols, checkCertificateRevocation); +#else + return ssl.AuthenticateAsServerAsync(serverAuthenticationOptions, cancellationToken); +#endif + } + } +} \ No newline at end of file diff --git a/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs b/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs deleted file mode 100644 index 46cb8fce1..000000000 --- a/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs +++ /dev/null @@ -1,34 +0,0 @@ -using System; -using System.Net.Security; -using System.Security.Authentication; -using System.Security.Cryptography.X509Certificates; -using System.Threading; -using System.Threading.Tasks; - -namespace Halibut.Transport.Streams -{ - static class SslStreamExtensionMethods - { -#if !NETFRAMEWORK - internal static async Task AuthenticateAsClientEnforcingTimeout( - this SslStream ssl, - ServiceEndPoint serviceEndpoint, - X509Certificate2Collection clientCertificates, - CancellationToken cancellationToken) - { - using var timeoutCts = new CancellationTokenSource(ssl.ReadTimeout); - using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(timeoutCts.Token, cancellationToken); - - var options = new SslClientAuthenticationOptions - { - TargetHost = serviceEndpoint.BaseUri.Host, - ClientCertificates = clientCertificates, - EnabledSslProtocols = SslConfiguration.SupportedProtocols, - CertificateRevocationCheckMode = X509RevocationMode.NoCheck - }; - - await ssl.AuthenticateAsClientAsync(options, linkedCts.Token); - } -#endif - } -} \ No newline at end of file diff --git a/source/Halibut/Transport/TcpConnectionFactory.cs b/source/Halibut/Transport/TcpConnectionFactory.cs index 2931297a5..793085d45 100644 --- a/source/Halibut/Transport/TcpConnectionFactory.cs +++ b/source/Halibut/Transport/TcpConnectionFactory.cs @@ -1,7 +1,7 @@ using System; +using System.Collections.Generic; using System.Net.Security; using System.Net.Sockets; -using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; using System.Text; using System.Threading; @@ -11,6 +11,8 @@ using Halibut.Transport.Proxy; using Halibut.Transport.Streams; +using ClientAuthStreamKey = System.ValueTuple; + namespace Halibut.Transport { public class TcpConnectionFactory : IConnectionFactory @@ -21,13 +23,17 @@ public class TcpConnectionFactory : IConnectionFactory readonly HalibutTimeoutsAndLimits halibutTimeoutsAndLimits; readonly IStreamFactory streamFactory; + // Note: We never clear this cache; If Octopus is connecting out to tentacles, will do so to the same tentacles repeatedly. + // In the uncommon case that a tentacle is deleted or changes its certificate, we will "leak" a small amount of memory but this is acceptable. + readonly Dictionary authContextCache = new(); + public TcpConnectionFactory(X509Certificate2 clientCertificate, HalibutTimeoutsAndLimits halibutTimeoutsAndLimits, IStreamFactory streamFactory) { this.clientCertificate = clientCertificate; this.halibutTimeoutsAndLimits = halibutTimeoutsAndLimits; this.streamFactory = streamFactory; } - + public async Task EstablishNewConnectionAsync(ExchangeProtocolBuilder exchangeProtocolBuilder, ServiceEndPoint serviceEndpoint, ILog log, CancellationToken cancellationToken) { log.Write(EventType.OpeningNewConnection, $"Opening a new connection to {serviceEndpoint.BaseUri}"); @@ -35,26 +41,27 @@ public async Task EstablishNewConnectionAsync(ExchangeProtocolBuild var certificateValidator = new ClientCertificateValidator(serviceEndpoint); var client = await CreateConnectedTcpClientAsync(serviceEndpoint, halibutTimeoutsAndLimits, streamFactory, log, cancellationToken); log.Write(EventType.Diagnostic, $"Connection established to {client.Client.RemoteEndPoint} for {serviceEndpoint.BaseUri}"); - + var networkTimeoutStream = streamFactory.CreateStream(client); client.ConfigureTcpOptions(halibutTimeoutsAndLimits); + SslStreamClientAuthentication? streamClientAuthentication; + lock (authContextCache) + { + var key = new ClientAuthStreamKey(serviceEndpoint.BaseUri.Host, clientCertificate.Thumbprint); + if (!authContextCache.TryGetValue(key, out streamClientAuthentication)) + { + streamClientAuthentication = new SslStreamClientAuthentication(serviceEndpoint.BaseUri.Host, new X509Certificate2Collection(clientCertificate), SslConfiguration.SupportedProtocols); + authContextCache[key] = streamClientAuthentication; + } + } + var ssl = new SslStream(networkTimeoutStream, false, certificateValidator.Validate, UserCertificateSelectionCallback); log.Write(EventType.SecurityNegotiation, "Performing TLS handshake"); -#if NETFRAMEWORK - // TODO: ASYNC ME UP! - // AuthenticateAsClientAsync in .NET 4.8 does not support cancellation tokens. So `cancellationToken` is not respected here. - await ssl.AuthenticateAsClientAsync( - serviceEndpoint.BaseUri.Host, - new X509Certificate2Collection(clientCertificate), - SslConfiguration.SupportedProtocols, - false); -#else - await ssl.AuthenticateAsClientEnforcingTimeout(serviceEndpoint, new X509Certificate2Collection(clientCertificate), cancellationToken); -#endif + await streamClientAuthentication.AuthenticateAsClientAsync(ssl, cancellationToken); await ssl.WriteAsync(MxLine, 0, MxLine.Length, cancellationToken); await ssl.FlushAsync(cancellationToken); @@ -63,7 +70,7 @@ await ssl.AuthenticateAsClientAsync( return new SecureConnection(client, ssl, exchangeProtocolBuilder, halibutTimeoutsAndLimits, log); } - + internal static async Task CreateConnectedTcpClientAsync(ServiceEndPoint endPoint, HalibutTimeoutsAndLimits halibutTimeoutsAndLimits, IStreamFactory streamFactory, ILog log, CancellationToken cancellationToken) { TcpClient client; @@ -75,12 +82,13 @@ internal static async Task CreateConnectedTcpClientAsync(ServiceEndPo else { log.Write(EventType.Diagnostic, "Creating a proxy client"); - + client = await new ProxyClientFactory(streamFactory) .CreateProxyClient(log, endPoint.Proxy) .WithTcpClientFactory(() => CreateTcpClientAsync(halibutTimeoutsAndLimits)) .CreateConnectionAsync(endPoint.BaseUri.Host, endPoint.BaseUri.Port, endPoint.TcpClientConnectTimeout, cancellationToken); } + return client; } @@ -92,7 +100,7 @@ internal static TcpClient CreateTcpClientAsync(HalibutTimeoutsAndLimits halibutT return CreateTcpClientAsync(addressFamily, halibutTimeoutsAndLimits); } - + internal static TcpClient CreateTcpClientAsync(AddressFamily addressFamily, HalibutTimeoutsAndLimits halibutTimeoutsAndLimits) { var client = new TcpClient(addressFamily) @@ -105,6 +113,7 @@ internal static TcpClient CreateTcpClientAsync(AddressFamily addressFamily, Hali { client.Client.DualMode = true; } + return client; } @@ -113,4 +122,4 @@ X509Certificate UserCertificateSelectionCallback(object sender, string targetHos return clientCertificate; } } -} +} \ No newline at end of file