From 00505e7192adaeacd05a88ab47889e594c9d43a8 Mon Sep 17 00:00:00 2001 From: alix Date: Mon, 4 Nov 2024 17:10:44 +1100 Subject: [PATCH 01/14] what if we just like .... didn't ... specify tls versions tho? --- source/Halibut/Transport/DiscoveryClient.cs | 2 +- source/Halibut/Transport/SecureListener.cs | 2 +- source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs | 1 - source/Halibut/Transport/TcpConnectionFactory.cs | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/source/Halibut/Transport/DiscoveryClient.cs b/source/Halibut/Transport/DiscoveryClient.cs index d95494d2a..8da4aeafd 100644 --- a/source/Halibut/Transport/DiscoveryClient.cs +++ b/source/Halibut/Transport/DiscoveryClient.cs @@ -42,7 +42,7 @@ public async Task DiscoverAsync(ServiceEndPoint serviceEndpoint #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(), SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12, false); + await ssl.AuthenticateAsClientAsync(serviceEndpoint.BaseUri.Host, new X509Certificate2Collection(), false); #else await ssl.AuthenticateAsClientEnforcingTimeout(serviceEndpoint, new X509Certificate2Collection(), cancellationToken); #endif diff --git a/source/Halibut/Transport/SecureListener.cs b/source/Halibut/Transport/SecureListener.cs index aa1f66ad3..3431dcd56 100644 --- a/source/Halibut/Transport/SecureListener.cs +++ b/source/Halibut/Transport/SecureListener.cs @@ -298,7 +298,7 @@ async Task ExecuteRequest(TcpClient client) { log.Write(EventType.SecurityNegotiation, "Performing TLS server handshake"); - await ssl.AuthenticateAsServerAsync(serverCertificate, true, SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12, false).ConfigureAwait(false); + await ssl.AuthenticateAsServerAsync(serverCertificate, true, false).ConfigureAwait(false); log.Write(EventType.SecurityNegotiation, "Secure connection established, client is not yet authenticated, client connected with {0}", ssl.SslProtocol.ToString()); diff --git a/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs b/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs index 67112cc76..b523df36b 100644 --- a/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs +++ b/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs @@ -23,7 +23,6 @@ internal static async Task AuthenticateAsClientEnforcingTimeout( { TargetHost = serviceEndpoint.BaseUri.Host, ClientCertificates = clientCertificates, - EnabledSslProtocols = SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12, CertificateRevocationCheckMode = X509RevocationMode.NoCheck }; diff --git a/source/Halibut/Transport/TcpConnectionFactory.cs b/source/Halibut/Transport/TcpConnectionFactory.cs index 41512bc64..d5f67577e 100644 --- a/source/Halibut/Transport/TcpConnectionFactory.cs +++ b/source/Halibut/Transport/TcpConnectionFactory.cs @@ -47,7 +47,7 @@ public async Task EstablishNewConnectionAsync(ExchangeProtocolBuild #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), SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12, false); + await ssl.AuthenticateAsClientAsync(serviceEndpoint.BaseUri.Host, new X509Certificate2Collection(clientCertificate), false); #else await ssl.AuthenticateAsClientEnforcingTimeout(serviceEndpoint, new X509Certificate2Collection(clientCertificate), cancellationToken); #endif From a7a6ebf6222e26bf8b9478de084a4dd1fc174384 Mon Sep 17 00:00:00 2001 From: alix Date: Mon, 4 Nov 2024 19:02:23 +1100 Subject: [PATCH 02/14] okay that wasnt great what if we added it instead? --- source/Halibut/Transport/DiscoveryClient.cs | 2 +- source/Halibut/Transport/SecureListener.cs | 2 +- source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs | 1 + source/Halibut/Transport/TcpConnectionFactory.cs | 2 +- 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/source/Halibut/Transport/DiscoveryClient.cs b/source/Halibut/Transport/DiscoveryClient.cs index 8da4aeafd..26859865b 100644 --- a/source/Halibut/Transport/DiscoveryClient.cs +++ b/source/Halibut/Transport/DiscoveryClient.cs @@ -42,7 +42,7 @@ public async Task DiscoverAsync(ServiceEndPoint serviceEndpoint #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(), false); + await ssl.AuthenticateAsClientAsync(serviceEndpoint.BaseUri.Host, new X509Certificate2Collection(), SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12 | SslProtocols.Tls13, false); #else await ssl.AuthenticateAsClientEnforcingTimeout(serviceEndpoint, new X509Certificate2Collection(), cancellationToken); #endif diff --git a/source/Halibut/Transport/SecureListener.cs b/source/Halibut/Transport/SecureListener.cs index 3431dcd56..61cad75a5 100644 --- a/source/Halibut/Transport/SecureListener.cs +++ b/source/Halibut/Transport/SecureListener.cs @@ -298,7 +298,7 @@ async Task ExecuteRequest(TcpClient client) { log.Write(EventType.SecurityNegotiation, "Performing TLS server handshake"); - await ssl.AuthenticateAsServerAsync(serverCertificate, true, false).ConfigureAwait(false); + await ssl.AuthenticateAsServerAsync(serverCertificate, true, SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12 | SslProtocols.Tls13, false).ConfigureAwait(false); log.Write(EventType.SecurityNegotiation, "Secure connection established, client is not yet authenticated, client connected with {0}", ssl.SslProtocol.ToString()); diff --git a/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs b/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs index b523df36b..ee1ab3ade 100644 --- a/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs +++ b/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs @@ -23,6 +23,7 @@ internal static async Task AuthenticateAsClientEnforcingTimeout( { TargetHost = serviceEndpoint.BaseUri.Host, ClientCertificates = clientCertificates, + EnabledSslProtocols = SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12 | SslProtocols.Tls13, CertificateRevocationCheckMode = X509RevocationMode.NoCheck }; diff --git a/source/Halibut/Transport/TcpConnectionFactory.cs b/source/Halibut/Transport/TcpConnectionFactory.cs index d5f67577e..0802d06e4 100644 --- a/source/Halibut/Transport/TcpConnectionFactory.cs +++ b/source/Halibut/Transport/TcpConnectionFactory.cs @@ -47,7 +47,7 @@ public async Task EstablishNewConnectionAsync(ExchangeProtocolBuild #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), false); + await ssl.AuthenticateAsClientAsync(serviceEndpoint.BaseUri.Host, new X509Certificate2Collection(clientCertificate), SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12 | SslProtocols.Tls13, false); #else await ssl.AuthenticateAsClientEnforcingTimeout(serviceEndpoint, new X509Certificate2Collection(clientCertificate), cancellationToken); #endif From 7a2d6abecfdf8bcdfbd405080fe734f445237ccc Mon Sep 17 00:00:00 2001 From: alix Date: Mon, 4 Nov 2024 19:31:07 +1100 Subject: [PATCH 03/14] it's weird that the docs say `SslProtocols.SystemDefault` but the code we're depending on says `SslProtocols.None`. Apparently it'll do the right thing - allow the OS to specify which protocol to use --- source/Halibut/Transport/DiscoveryClient.cs | 2 +- source/Halibut/Transport/SecureListener.cs | 2 +- source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs | 4 +++- source/Halibut/Transport/TcpConnectionFactory.cs | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/source/Halibut/Transport/DiscoveryClient.cs b/source/Halibut/Transport/DiscoveryClient.cs index 26859865b..1af4b5f2e 100644 --- a/source/Halibut/Transport/DiscoveryClient.cs +++ b/source/Halibut/Transport/DiscoveryClient.cs @@ -42,7 +42,7 @@ public async Task DiscoverAsync(ServiceEndPoint serviceEndpoint #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(), SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12 | SslProtocols.Tls13, false); + await ssl.AuthenticateAsClientAsync(serviceEndpoint.BaseUri.Host, new X509Certificate2Collection(), SslProtocols.None, false); #else await ssl.AuthenticateAsClientEnforcingTimeout(serviceEndpoint, new X509Certificate2Collection(), cancellationToken); #endif diff --git a/source/Halibut/Transport/SecureListener.cs b/source/Halibut/Transport/SecureListener.cs index 61cad75a5..8d11695a1 100644 --- a/source/Halibut/Transport/SecureListener.cs +++ b/source/Halibut/Transport/SecureListener.cs @@ -298,7 +298,7 @@ async Task ExecuteRequest(TcpClient client) { log.Write(EventType.SecurityNegotiation, "Performing TLS server handshake"); - await ssl.AuthenticateAsServerAsync(serverCertificate, true, SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12 | SslProtocols.Tls13, false).ConfigureAwait(false); + await ssl.AuthenticateAsServerAsync(serverCertificate, true, SslProtocols.None, false).ConfigureAwait(false); log.Write(EventType.SecurityNegotiation, "Secure connection established, client is not yet authenticated, client connected with {0}", ssl.SslProtocol.ToString()); diff --git a/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs b/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs index ee1ab3ade..e297f303a 100644 --- a/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs +++ b/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs @@ -23,7 +23,9 @@ internal static async Task AuthenticateAsClientEnforcingTimeout( { TargetHost = serviceEndpoint.BaseUri.Host, ClientCertificates = clientCertificates, - EnabledSslProtocols = SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12 | SslProtocols.Tls13, + // Despite what you might think, this option allows the OS to decide the SSL version to use and is the recommended approach per the code comments on the version of SslProtocols we're using + // This is current at time of writing. MS docs recommend using SslProtocols.SystemDefault which does not exist on this particular enum, so that's fun. + EnabledSslProtocols = SslProtocols.None, CertificateRevocationCheckMode = X509RevocationMode.NoCheck }; diff --git a/source/Halibut/Transport/TcpConnectionFactory.cs b/source/Halibut/Transport/TcpConnectionFactory.cs index 0802d06e4..f3ad3eee5 100644 --- a/source/Halibut/Transport/TcpConnectionFactory.cs +++ b/source/Halibut/Transport/TcpConnectionFactory.cs @@ -47,7 +47,7 @@ public async Task EstablishNewConnectionAsync(ExchangeProtocolBuild #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), SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12 | SslProtocols.Tls13, false); + await ssl.AuthenticateAsClientAsync(serviceEndpoint.BaseUri.Host, new X509Certificate2Collection(clientCertificate), SslProtocols.None, false); #else await ssl.AuthenticateAsClientEnforcingTimeout(serviceEndpoint, new X509Certificate2Collection(clientCertificate), cancellationToken); #endif From 456d5b51e6fd3b2bbb28141b82f465c7d715f011 Mon Sep 17 00:00:00 2001 From: Geoff Battye Date: Tue, 26 Nov 2024 17:38:55 +1100 Subject: [PATCH 04/14] Restore specification of SSL protocols and add TLS 1.3. --- source/Halibut/Transport/DiscoveryClient.cs | 6 +++++- source/Halibut/Transport/SecureListener.cs | 8 +++++++- .../Transport/Streams/SslStreamExtensionMethods.cs | 2 +- source/Halibut/Transport/TcpConnectionFactory.cs | 6 +++++- 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/source/Halibut/Transport/DiscoveryClient.cs b/source/Halibut/Transport/DiscoveryClient.cs index 1af4b5f2e..79b343adb 100644 --- a/source/Halibut/Transport/DiscoveryClient.cs +++ b/source/Halibut/Transport/DiscoveryClient.cs @@ -42,7 +42,11 @@ public async Task DiscoverAsync(ServiceEndPoint serviceEndpoint #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(), SslProtocols.None, false); + await ssl.AuthenticateAsClientAsync( + serviceEndpoint.BaseUri.Host, + new X509Certificate2Collection(), + SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12 | SslProtocols.Tls13, + false); #else await ssl.AuthenticateAsClientEnforcingTimeout(serviceEndpoint, new X509Certificate2Collection(), cancellationToken); #endif diff --git a/source/Halibut/Transport/SecureListener.cs b/source/Halibut/Transport/SecureListener.cs index 8d11695a1..6105fa903 100644 --- a/source/Halibut/Transport/SecureListener.cs +++ b/source/Halibut/Transport/SecureListener.cs @@ -298,7 +298,13 @@ async Task ExecuteRequest(TcpClient client) { log.Write(EventType.SecurityNegotiation, "Performing TLS server handshake"); - await ssl.AuthenticateAsServerAsync(serverCertificate, true, SslProtocols.None, false).ConfigureAwait(false); + await ssl + .AuthenticateAsServerAsync( + serverCertificate, + true, + SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12 | SslProtocols.Tls13, + false) + .ConfigureAwait(false); log.Write(EventType.SecurityNegotiation, "Secure connection established, client is not yet authenticated, client connected with {0}", ssl.SslProtocol.ToString()); diff --git a/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs b/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs index e297f303a..a2b0db036 100644 --- a/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs +++ b/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs @@ -25,7 +25,7 @@ internal static async Task AuthenticateAsClientEnforcingTimeout( ClientCertificates = clientCertificates, // Despite what you might think, this option allows the OS to decide the SSL version to use and is the recommended approach per the code comments on the version of SslProtocols we're using // This is current at time of writing. MS docs recommend using SslProtocols.SystemDefault which does not exist on this particular enum, so that's fun. - EnabledSslProtocols = SslProtocols.None, + EnabledSslProtocols = SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12 | SslProtocols.Tls13, CertificateRevocationCheckMode = X509RevocationMode.NoCheck }; diff --git a/source/Halibut/Transport/TcpConnectionFactory.cs b/source/Halibut/Transport/TcpConnectionFactory.cs index f3ad3eee5..3a13913a9 100644 --- a/source/Halibut/Transport/TcpConnectionFactory.cs +++ b/source/Halibut/Transport/TcpConnectionFactory.cs @@ -47,7 +47,11 @@ public async Task EstablishNewConnectionAsync(ExchangeProtocolBuild #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), SslProtocols.None, false); + await ssl.AuthenticateAsClientAsync( + serviceEndpoint.BaseUri.Host, + new X509Certificate2Collection(clientCertificate), + SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12 | SslProtocols.Tls13, + false); #else await ssl.AuthenticateAsClientEnforcingTimeout(serviceEndpoint, new X509Certificate2Collection(clientCertificate), cancellationToken); #endif From 7bbf6cbfa1129e81e81ec2062a3d15759f3f62bf Mon Sep 17 00:00:00 2001 From: Geoff Battye Date: Tue, 26 Nov 2024 19:19:01 +1100 Subject: [PATCH 05/14] Remove outdated comment. --- source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs b/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs index a2b0db036..ee1ab3ade 100644 --- a/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs +++ b/source/Halibut/Transport/Streams/SslStreamExtensionMethods.cs @@ -23,8 +23,6 @@ internal static async Task AuthenticateAsClientEnforcingTimeout( { TargetHost = serviceEndpoint.BaseUri.Host, ClientCertificates = clientCertificates, - // Despite what you might think, this option allows the OS to decide the SSL version to use and is the recommended approach per the code comments on the version of SslProtocols we're using - // This is current at time of writing. MS docs recommend using SslProtocols.SystemDefault which does not exist on this particular enum, so that's fun. EnabledSslProtocols = SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12 | SslProtocols.Tls13, CertificateRevocationCheckMode = X509RevocationMode.NoCheck }; From 6335269ab5a253237ab68bf7aa354667e035723d Mon Sep 17 00:00:00 2001 From: Geoff Battye Date: Thu, 28 Nov 2024 22:03:38 +1100 Subject: [PATCH 06/14] Added TLS test. --- source/Halibut.Tests/TlsFixture.cs | 66 ++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 source/Halibut.Tests/TlsFixture.cs diff --git a/source/Halibut.Tests/TlsFixture.cs b/source/Halibut.Tests/TlsFixture.cs new file mode 100644 index 000000000..ffc332f0b --- /dev/null +++ b/source/Halibut.Tests/TlsFixture.cs @@ -0,0 +1,66 @@ +// 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; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Runtime.InteropServices; +using System.Security.Authentication; +using System.Threading.Tasks; +using FluentAssertions; +using Halibut.Exceptions; +using Halibut.Tests.Support; +using Halibut.Tests.Support.TestAttributes; +using Halibut.Tests.Support.TestCases; +using Halibut.Tests.TestServices.Async; +using Halibut.Tests.Util; +using Halibut.TestUtils.Contracts; +using NUnit.Framework; + +namespace Halibut.Tests +{ + public class TlsFixture : BaseTest + { + [Test] + [LatestClientAndLatestServiceTestCases(testNetworkConditions: false)] + public async Task LatestClientAndServiceUseBestAvailableSslProtocol(ClientAndServiceTestCase clientAndServiceTestCase) + { + await using (var clientAndService = await clientAndServiceTestCase.CreateTestCaseBuilder() + .WithStandardServices() + .AsLatestClientAndLatestServiceBuilder() + .RecordingClientLogs(out var clientLogs) + .RecordingServiceLogs(out var serviceLogs) + .Build(CancellationToken)) + { + var echo = clientAndService.CreateAsyncClient(); + await echo.SayHelloAsync("World"); + + var connectionInitiatorLogs = clientAndServiceTestCase.ServiceConnectionType == ServiceConnectionType.Listening + ? clientLogs + : serviceLogs; + + // .NET does not support TLS 1.3 on Mac OS yet. + // https://github.com/dotnet/runtime/issues/1979 + var expectedSslProtocol = RuntimeInformation.IsOSPlatform(OSPlatform.OSX) + ? SslProtocols.Tls12 + : SslProtocols.Tls13; + + connectionInitiatorLogs.Values + .SelectMany(log => log.GetLogs()) + .Should().Contain(logEvent => logEvent.FormattedMessage.Contains($"using protocol {expectedSslProtocol}")); + } + } + } +} \ No newline at end of file From 703b82a9880218926c939b396d88221b0b9bf152 Mon Sep 17 00:00:00 2001 From: Geoff Battye Date: Tue, 3 Dec 2024 22:52:13 +1100 Subject: [PATCH 07/14] Added TLS fallback test. --- ...tClientAndPreviousServiceVersionBuilder.cs | 34 +++++++++++++++++-- ...ClientAndServiceBuilderExtensionMethods.cs | 7 +++- source/Halibut.Tests/TlsFixture.cs | 27 ++++++++++++--- 3 files changed, 61 insertions(+), 7 deletions(-) diff --git a/source/Halibut.Tests/Support/BackwardsCompatibility/LatestClientAndPreviousServiceVersionBuilder.cs b/source/Halibut.Tests/Support/BackwardsCompatibility/LatestClientAndPreviousServiceVersionBuilder.cs index 4d0d041d2..711bd6be0 100644 --- a/source/Halibut.Tests/Support/BackwardsCompatibility/LatestClientAndPreviousServiceVersionBuilder.cs +++ b/source/Halibut.Tests/Support/BackwardsCompatibility/LatestClientAndPreviousServiceVersionBuilder.cs @@ -1,12 +1,15 @@ using System; +using System.Collections.Concurrent; using System.Threading; using System.Threading.Tasks; +using Halibut.Diagnostics; using Halibut.Diagnostics.LogCreators; using Halibut.Logging; using Halibut.TestProxy; using Halibut.Tests.Support.Logging; using Halibut.Transport.Proxy; using Octopus.TestPortForwarder; +using ILog = Halibut.Diagnostics.ILog; namespace Halibut.Tests.Support.BackwardsCompatibility { @@ -21,6 +24,7 @@ public class LatestClientAndPreviousServiceVersionBuilder : IClientAndServiceBui ProxyFactory? proxyFactory; Reference? proxyServiceReference; LogLevel halibutLogLevel = LogLevel.Trace; + ConcurrentDictionary? clientInMemoryLoggers; readonly OldServiceAvailableServices availableServices = new(false, false); LatestClientAndPreviousServiceVersionBuilder(ServiceConnectionType serviceConnectionType, CertAndThumbprint serviceCertAndThumbprint) @@ -146,7 +150,14 @@ async Task IClientAndServiceBuilder.Build(CancellationToken c { return await Build(cancellationToken); } - + + public LatestClientAndPreviousServiceVersionBuilder RecordingClientLogs(out ConcurrentDictionary inMemoryLoggers) + { + inMemoryLoggers = new ConcurrentDictionary(); + this.clientInMemoryLoggers = inMemoryLoggers; + return this; + } + public async Task Build(CancellationToken cancellationToken) { var logger = new SerilogLoggerBuilder().Build().ForContext(); @@ -158,7 +169,7 @@ public async Task Build(CancellationToken cancellationToken) var clientBuilder = new HalibutRuntimeBuilder() .WithServerCertificate(clientCertAndThumbprint.Certificate2) - .WithLogFactory(new TestContextLogCreator("Client", halibutLogLevel).ToCachingLogFactory()) + .WithLogFactory(BuildClientLogger()) .WithHalibutTimeoutsAndLimits(new HalibutTimeoutsAndLimitsForTestsBuilder().Build()); var client = clientBuilder.Build(); @@ -338,5 +349,24 @@ public async ValueTask DisposeAsync() Try.CatchingError(() => cancellationTokenSource.Dispose(), LogError); } } + + ILogFactory BuildClientLogger() + { + if (clientInMemoryLoggers == null) + { + return new TestContextLogCreator("Client", halibutLogLevel).ToCachingLogFactory(); + } + + return new AggregateLogWriterLogCreator( + new TestContextLogCreator("Client", halibutLogLevel), + s => + { + var logger = new InMemoryLogWriter(); + clientInMemoryLoggers[s] = logger; + return new[] {logger}; + } + ) + .ToCachingLogFactory(); + } } } diff --git a/source/Halibut.Tests/Support/ClientAndServiceBuilderExtensionMethods.cs b/source/Halibut.Tests/Support/ClientAndServiceBuilderExtensionMethods.cs index 01c1e1276..def312aca 100644 --- a/source/Halibut.Tests/Support/ClientAndServiceBuilderExtensionMethods.cs +++ b/source/Halibut.Tests/Support/ClientAndServiceBuilderExtensionMethods.cs @@ -10,12 +10,17 @@ public static LatestClientAndLatestServiceBuilder AsLatestClientAndLatestService { return (LatestClientAndLatestServiceBuilder) clientAndServiceBuilder; } - + public static PreviousClientVersionAndLatestServiceBuilder AsPreviousClientVersionAndLatestServiceBuilder(this IClientAndServiceBuilder clientAndServiceBuilder) { return (PreviousClientVersionAndLatestServiceBuilder) clientAndServiceBuilder; } + public static LatestClientAndPreviousServiceVersionBuilder AsLatestClientAndPreviousServiceVersionBuilder(this IClientAndServiceBuilder clientAndServiceBuilder) + { + return (LatestClientAndPreviousServiceVersionBuilder) clientAndServiceBuilder; + } + public static IClientAndServiceBuilder WithAsyncService(this IClientAndServiceBuilder clientAndServiceBuilder, Func implementation) { if (clientAndServiceBuilder is LatestClientAndLatestServiceBuilder) diff --git a/source/Halibut.Tests/TlsFixture.cs b/source/Halibut.Tests/TlsFixture.cs index ffc332f0b..bed047a1c 100644 --- a/source/Halibut.Tests/TlsFixture.cs +++ b/source/Halibut.Tests/TlsFixture.cs @@ -13,19 +13,15 @@ // limitations under the License. using System; -using System.Collections.Generic; -using System.IO; using System.Linq; using System.Runtime.InteropServices; using System.Security.Authentication; using System.Threading.Tasks; using FluentAssertions; -using Halibut.Exceptions; using Halibut.Tests.Support; using Halibut.Tests.Support.TestAttributes; using Halibut.Tests.Support.TestCases; using Halibut.Tests.TestServices.Async; -using Halibut.Tests.Util; using Halibut.TestUtils.Contracts; using NUnit.Framework; @@ -62,5 +58,28 @@ public async Task LatestClientAndServiceUseBestAvailableSslProtocol(ClientAndSer .Should().Contain(logEvent => logEvent.FormattedMessage.Contains($"using protocol {expectedSslProtocol}")); } } + + [Test] + [LatestClientAndPreviousServiceVersionsTestCases(testNetworkConditions: false)] + public async Task LatestClientAndPreviousServiceFallBackOnTls12(ClientAndServiceTestCase clientAndServiceTestCase) + { + await using (var clientAndService = await clientAndServiceTestCase.CreateTestCaseBuilder() + .WithStandardServices() + .AsLatestClientAndPreviousServiceVersionBuilder() + .RecordingClientLogs(out var clientLogs) + .Build(CancellationToken)) + { + var echo = clientAndService.CreateAsyncClient(); + await echo.SayHelloAsync("World"); + + var expectedLogMessage = clientAndServiceTestCase.ServiceConnectionType == ServiceConnectionType.Listening + ? $"using protocol {SslProtocols.Tls12}" + : $"client connected with {SslProtocols.Tls12}"; + + clientLogs.Values + .SelectMany(log => log.GetLogs()) + .Should().Contain(logEvent => logEvent.FormattedMessage.Contains(expectedLogMessage)); + } + } } } \ No newline at end of file From d0a708c98f671f6ef66019d119feced0d8f7a70c Mon Sep 17 00:00:00 2001 From: Geoff Battye Date: Wed, 4 Dec 2024 14:34:34 +1100 Subject: [PATCH 08/14] Log platform info (temp to work out how to detect platform). --- source/Halibut.Tests/TlsFixture.cs | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/source/Halibut.Tests/TlsFixture.cs b/source/Halibut.Tests/TlsFixture.cs index bed047a1c..21dcdabf8 100644 --- a/source/Halibut.Tests/TlsFixture.cs +++ b/source/Halibut.Tests/TlsFixture.cs @@ -1,17 +1,3 @@ -// 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; using System.Linq; using System.Runtime.InteropServices; @@ -47,6 +33,18 @@ public async Task LatestClientAndServiceUseBestAvailableSslProtocol(ClientAndSer ? clientLogs : serviceLogs; + Logger.Information("Platform detection:"); + Logger.Information("Environment.OSVersion.Platform: {EnvironmentOSVersionPlatform}", Environment.OSVersion.Platform); + Logger.Information("Environment.OSVersion.Version: {EnvironmentOSVersionVersion}", Environment.OSVersion.Version); + Logger.Information("Environment.OSVersion.VersionString: {EnvironmentOSVersionVersionString}", Environment.OSVersion.VersionString); + Logger.Information("Environment.OSVersion.ServicePack: {Environment.OSVersion.ServicePack}", Environment.OSVersion.ServicePack); + Logger.Information("RuntimeInformation.OSDescription: {RuntimeInformation.OSDescription}", RuntimeInformation.OSDescription); + Logger.Information("RuntimeInformation.RuntimeIdentifier: {RuntimeInformationRuntimeIdentifier}", RuntimeInformation.RuntimeIdentifier); + Logger.Information("RuntimeInformation.FrameworkDescription: {RuntimeInformation.FrameworkDescription}", RuntimeInformation.FrameworkDescription); + Logger.Information("RuntimeInformation.ProcessArchitecture: {RuntimeInformation.ProcessArchitecture}", RuntimeInformation.ProcessArchitecture); + Logger.Information("RuntimeInformation.OSArchitecture: {RuntimeInformation.OSArchitecture}", RuntimeInformation.OSArchitecture); + Logger.Information("RuntimeInformation.RuntimeIdentifier: {RuntimeInformationRuntimeIdentifier}", RuntimeInformation.RuntimeIdentifier); + // .NET does not support TLS 1.3 on Mac OS yet. // https://github.com/dotnet/runtime/issues/1979 var expectedSslProtocol = RuntimeInformation.IsOSPlatform(OSPlatform.OSX) From e60006a57452d7dede81ec4950987b9a343a1949 Mon Sep 17 00:00:00 2001 From: Geoff Battye Date: Wed, 4 Dec 2024 14:43:52 +1100 Subject: [PATCH 09/14] Fix platform logging. --- source/Halibut.Tests/TlsFixture.cs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/source/Halibut.Tests/TlsFixture.cs b/source/Halibut.Tests/TlsFixture.cs index 21dcdabf8..b8b76e3ab 100644 --- a/source/Halibut.Tests/TlsFixture.cs +++ b/source/Halibut.Tests/TlsFixture.cs @@ -26,6 +26,17 @@ public async Task LatestClientAndServiceUseBestAvailableSslProtocol(ClientAndSer .RecordingServiceLogs(out var serviceLogs) .Build(CancellationToken)) { + Logger.Information("Platform detection:"); + Logger.Information("Environment.OSVersion.Platform: {EnvironmentOSVersionPlatform}", Environment.OSVersion.Platform); + Logger.Information("Environment.OSVersion.Version: {EnvironmentOSVersionVersion}", Environment.OSVersion.Version); + Logger.Information("Environment.OSVersion.VersionString: {EnvironmentOSVersionVersionString}", Environment.OSVersion.VersionString); + Logger.Information("Environment.OSVersion.ServicePack: {EnvironmentOSVersionServicePack}", Environment.OSVersion.ServicePack); + Logger.Information("RuntimeInformation.OSDescription: {RuntimeInformationOSDescription}", RuntimeInformation.OSDescription); + Logger.Information("RuntimeInformation.RuntimeIdentifier: {RuntimeInformationRuntimeIdentifier}", RuntimeInformation.RuntimeIdentifier); + Logger.Information("RuntimeInformation.FrameworkDescription: {RuntimeInformationFrameworkDescription}", RuntimeInformation.FrameworkDescription); + Logger.Information("RuntimeInformation.ProcessArchitecture: {RuntimeInformationProcessArchitecture}", RuntimeInformation.ProcessArchitecture); + Logger.Information("RuntimeInformation.OSArchitecture: {RuntimeInformationOSArchitecture}", RuntimeInformation.OSArchitecture); + var echo = clientAndService.CreateAsyncClient(); await echo.SayHelloAsync("World"); @@ -33,18 +44,6 @@ public async Task LatestClientAndServiceUseBestAvailableSslProtocol(ClientAndSer ? clientLogs : serviceLogs; - Logger.Information("Platform detection:"); - Logger.Information("Environment.OSVersion.Platform: {EnvironmentOSVersionPlatform}", Environment.OSVersion.Platform); - Logger.Information("Environment.OSVersion.Version: {EnvironmentOSVersionVersion}", Environment.OSVersion.Version); - Logger.Information("Environment.OSVersion.VersionString: {EnvironmentOSVersionVersionString}", Environment.OSVersion.VersionString); - Logger.Information("Environment.OSVersion.ServicePack: {Environment.OSVersion.ServicePack}", Environment.OSVersion.ServicePack); - Logger.Information("RuntimeInformation.OSDescription: {RuntimeInformation.OSDescription}", RuntimeInformation.OSDescription); - Logger.Information("RuntimeInformation.RuntimeIdentifier: {RuntimeInformationRuntimeIdentifier}", RuntimeInformation.RuntimeIdentifier); - Logger.Information("RuntimeInformation.FrameworkDescription: {RuntimeInformation.FrameworkDescription}", RuntimeInformation.FrameworkDescription); - Logger.Information("RuntimeInformation.ProcessArchitecture: {RuntimeInformation.ProcessArchitecture}", RuntimeInformation.ProcessArchitecture); - Logger.Information("RuntimeInformation.OSArchitecture: {RuntimeInformation.OSArchitecture}", RuntimeInformation.OSArchitecture); - Logger.Information("RuntimeInformation.RuntimeIdentifier: {RuntimeInformationRuntimeIdentifier}", RuntimeInformation.RuntimeIdentifier); - // .NET does not support TLS 1.3 on Mac OS yet. // https://github.com/dotnet/runtime/issues/1979 var expectedSslProtocol = RuntimeInformation.IsOSPlatform(OSPlatform.OSX) From 8205df88a527290ed809a49ca42097d05a184dd4 Mon Sep 17 00:00:00 2001 From: Geoff Battye Date: Wed, 4 Dec 2024 17:27:09 +1100 Subject: [PATCH 10/14] Remove usage of RuntimeInformation.RuntimeIdentifier which is not available in .NET Framework 4.8. --- source/Halibut.Tests/TlsFixture.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/source/Halibut.Tests/TlsFixture.cs b/source/Halibut.Tests/TlsFixture.cs index b8b76e3ab..fc5b6fcf4 100644 --- a/source/Halibut.Tests/TlsFixture.cs +++ b/source/Halibut.Tests/TlsFixture.cs @@ -32,7 +32,6 @@ public async Task LatestClientAndServiceUseBestAvailableSslProtocol(ClientAndSer Logger.Information("Environment.OSVersion.VersionString: {EnvironmentOSVersionVersionString}", Environment.OSVersion.VersionString); Logger.Information("Environment.OSVersion.ServicePack: {EnvironmentOSVersionServicePack}", Environment.OSVersion.ServicePack); Logger.Information("RuntimeInformation.OSDescription: {RuntimeInformationOSDescription}", RuntimeInformation.OSDescription); - Logger.Information("RuntimeInformation.RuntimeIdentifier: {RuntimeInformationRuntimeIdentifier}", RuntimeInformation.RuntimeIdentifier); Logger.Information("RuntimeInformation.FrameworkDescription: {RuntimeInformationFrameworkDescription}", RuntimeInformation.FrameworkDescription); Logger.Information("RuntimeInformation.ProcessArchitecture: {RuntimeInformationProcessArchitecture}", RuntimeInformation.ProcessArchitecture); Logger.Information("RuntimeInformation.OSArchitecture: {RuntimeInformationOSArchitecture}", RuntimeInformation.OSArchitecture); From 8e1a3ecc6dd196fe9808e1e8594600b71285add0 Mon Sep 17 00:00:00 2001 From: Geoff Battye Date: Fri, 6 Dec 2024 17:43:00 +1100 Subject: [PATCH 11/14] Update expected TLS version per platform in tests. --- source/Halibut.Tests/TlsFixture.cs | 46 +++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/source/Halibut.Tests/TlsFixture.cs b/source/Halibut.Tests/TlsFixture.cs index fc5b6fcf4..960e67b0e 100644 --- a/source/Halibut.Tests/TlsFixture.cs +++ b/source/Halibut.Tests/TlsFixture.cs @@ -43,15 +43,18 @@ public async Task LatestClientAndServiceUseBestAvailableSslProtocol(ClientAndSer ? clientLogs : serviceLogs; - // .NET does not support TLS 1.3 on Mac OS yet. - // https://github.com/dotnet/runtime/issues/1979 - var expectedSslProtocol = RuntimeInformation.IsOSPlatform(OSPlatform.OSX) - ? SslProtocols.Tls12 - : SslProtocols.Tls13; - + var expectedSslProtocol = GetExpectedSslProtocolForTheCurrentPlatform(); + var expectedLogFragment = $"using protocol {expectedSslProtocol}"; + connectionInitiatorLogs.Values .SelectMany(log => log.GetLogs()) - .Should().Contain(logEvent => logEvent.FormattedMessage.Contains($"using protocol {expectedSslProtocol}")); + .Should().Contain( + logEvent => logEvent.FormattedMessage.Contains(expectedLogFragment), + "The OS is \"{{OSDescription}}\", so we expect {{expectedSslProtocol}} to be used, and expect log output to contain \"{expectedLogFragment}\" for {tentacleType} tentacles.", + RuntimeInformation.OSDescription, + expectedSslProtocol, + expectedLogFragment, + clientAndServiceTestCase.ServiceConnectionType); } } @@ -77,5 +80,34 @@ public async Task LatestClientAndPreviousServiceFallBackOnTls12(ClientAndService .Should().Contain(logEvent => logEvent.FormattedMessage.Contains(expectedLogMessage)); } } + + SslProtocols GetExpectedSslProtocolForTheCurrentPlatform() + { + // All linux platforms we test against support TLS 1.3. + if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + return SslProtocols.Tls13; + } + + // We test against old versions of Windows which do not support TLS 1.3. + // TLS 1.3 is supported since Windows Server 2022 which has build number 20348, and Windows 11 which has higher build numbers. + // TLS 1.3 is partially supported in Windows 10, which can have lower build numbers, but we don't test against that so it is ignored here. + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + const int WindowsServer2022OSBuild = 20348; + return Environment.OSVersion.Version.Build >= WindowsServer2022OSBuild + ? SslProtocols.Tls13 + : SslProtocols.Tls12; + } + + // .NET does not support TLS 1.3 on Mac OS yet. + // https://github.com/dotnet/runtime/issues/1979 + if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) + { + return SslProtocols.Tls12; + } + + throw new NotSupportedException($"Unsupported OS platform: {RuntimeInformation.OSDescription}"); + } } } \ No newline at end of file From 4f2cf24a9114400957c29bb3dd6c9880500eedfe Mon Sep 17 00:00:00 2001 From: Geoff Battye Date: Mon, 9 Dec 2024 10:00:59 +1100 Subject: [PATCH 12/14] Disable TLS tests for websocket, as we don't log the TLS version. --- source/Halibut.Tests/TlsFixture.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/Halibut.Tests/TlsFixture.cs b/source/Halibut.Tests/TlsFixture.cs index 960e67b0e..9889f190e 100644 --- a/source/Halibut.Tests/TlsFixture.cs +++ b/source/Halibut.Tests/TlsFixture.cs @@ -16,7 +16,7 @@ namespace Halibut.Tests public class TlsFixture : BaseTest { [Test] - [LatestClientAndLatestServiceTestCases(testNetworkConditions: false)] + [LatestClientAndLatestServiceTestCases(testWebSocket: false, testNetworkConditions: false)] public async Task LatestClientAndServiceUseBestAvailableSslProtocol(ClientAndServiceTestCase clientAndServiceTestCase) { await using (var clientAndService = await clientAndServiceTestCase.CreateTestCaseBuilder() @@ -59,7 +59,7 @@ public async Task LatestClientAndServiceUseBestAvailableSslProtocol(ClientAndSer } [Test] - [LatestClientAndPreviousServiceVersionsTestCases(testNetworkConditions: false)] + [LatestClientAndPreviousServiceVersionsTestCases(testWebSocket: false, testNetworkConditions: false)] public async Task LatestClientAndPreviousServiceFallBackOnTls12(ClientAndServiceTestCase clientAndServiceTestCase) { await using (var clientAndService = await clientAndServiceTestCase.CreateTestCaseBuilder() From cfd152e72a92fa0468e8cabcee0da5a6da65089e Mon Sep 17 00:00:00 2001 From: Geoff Battye Date: Mon, 9 Dec 2024 15:35:07 +1100 Subject: [PATCH 13/14] Fix 'because' formatting error in test. --- source/Halibut.Tests/TlsFixture.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/source/Halibut.Tests/TlsFixture.cs b/source/Halibut.Tests/TlsFixture.cs index 9889f190e..b0bc4e332 100644 --- a/source/Halibut.Tests/TlsFixture.cs +++ b/source/Halibut.Tests/TlsFixture.cs @@ -43,18 +43,14 @@ public async Task LatestClientAndServiceUseBestAvailableSslProtocol(ClientAndSer ? clientLogs : serviceLogs; - var expectedSslProtocol = GetExpectedSslProtocolForTheCurrentPlatform(); + var expectedSslProtocol = SslProtocols.Tls13;// GetExpectedSslProtocolForTheCurrentPlatform(); var expectedLogFragment = $"using protocol {expectedSslProtocol}"; connectionInitiatorLogs.Values .SelectMany(log => log.GetLogs()) .Should().Contain( logEvent => logEvent.FormattedMessage.Contains(expectedLogFragment), - "The OS is \"{{OSDescription}}\", so we expect {{expectedSslProtocol}} to be used, and expect log output to contain \"{expectedLogFragment}\" for {tentacleType} tentacles.", - RuntimeInformation.OSDescription, - expectedSslProtocol, - expectedLogFragment, - clientAndServiceTestCase.ServiceConnectionType); + $"The OS is \"{RuntimeInformation.OSDescription}\", so we expect {expectedSslProtocol} to be used, and expect log output to contain \"{expectedLogFragment}\" for {clientAndServiceTestCase.ServiceConnectionType} tentacles"); } } From 72e33bf41ccca11ddb11388be60a4eae2213feb5 Mon Sep 17 00:00:00 2001 From: Geoff Battye Date: Fri, 3 Jan 2025 12:48:06 +1100 Subject: [PATCH 14/14] Restore calculation of expected TLS version in test. --- source/Halibut.Tests/TlsFixture.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/Halibut.Tests/TlsFixture.cs b/source/Halibut.Tests/TlsFixture.cs index b0bc4e332..b7aff1381 100644 --- a/source/Halibut.Tests/TlsFixture.cs +++ b/source/Halibut.Tests/TlsFixture.cs @@ -43,14 +43,14 @@ public async Task LatestClientAndServiceUseBestAvailableSslProtocol(ClientAndSer ? clientLogs : serviceLogs; - var expectedSslProtocol = SslProtocols.Tls13;// GetExpectedSslProtocolForTheCurrentPlatform(); + var expectedSslProtocol = GetExpectedSslProtocolForTheCurrentPlatform(); var expectedLogFragment = $"using protocol {expectedSslProtocol}"; connectionInitiatorLogs.Values .SelectMany(log => log.GetLogs()) .Should().Contain( logEvent => logEvent.FormattedMessage.Contains(expectedLogFragment), - $"The OS is \"{RuntimeInformation.OSDescription}\", so we expect {expectedSslProtocol} to be used, and expect log output to contain \"{expectedLogFragment}\" for {clientAndServiceTestCase.ServiceConnectionType} tentacles"); + $"the OS is \"{RuntimeInformation.OSDescription}\", so we expect {expectedSslProtocol} to be used, and expect log output to contain \"{expectedLogFragment}\" for {clientAndServiceTestCase.ServiceConnectionType} tentacles"); } }