From 6fdee9e0db738817c6f457c3597f84460cff3cd0 Mon Sep 17 00:00:00 2001 From: Christian <6939810+chkr1011@users.noreply.github.com> Date: Wed, 6 Sep 2023 20:08:35 +0200 Subject: [PATCH] Fix wrong TLS usage (#1833) * Fix wrong usage of TLS options * Fix public broker tests * Update ReleaseNotes.md --- .github/workflows/ReleaseNotes.md | 1 + .../WebSocket4NetMqttChannel.cs | 6 ++--- Source/MQTTnet.TestApp/PublicBrokerTest.cs | 22 +++++++++++-------- .../Options/MqttClientOptionsBuilder.cs | 7 +++++- .../Implementations/MqttWebSocketChannel.cs | 6 ++--- 5 files changed, 26 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ReleaseNotes.md b/.github/workflows/ReleaseNotes.md index a3c59fc92..f1aa2d52b 100644 --- a/.github/workflows/ReleaseNotes.md +++ b/.github/workflows/ReleaseNotes.md @@ -1 +1,2 @@ +* [Client] Fixed wrong TLS options handling (#1830). * [Client] Fixed NullReferenceExeption when performing a Ping when the client is not connected (#1831). diff --git a/Source/MQTTnet.Extensions.WebSocket4Net/WebSocket4NetMqttChannel.cs b/Source/MQTTnet.Extensions.WebSocket4Net/WebSocket4NetMqttChannel.cs index b70735943..56f7f01f3 100644 --- a/Source/MQTTnet.Extensions.WebSocket4Net/WebSocket4NetMqttChannel.cs +++ b/Source/MQTTnet.Extensions.WebSocket4Net/WebSocket4NetMqttChannel.cs @@ -47,13 +47,13 @@ public Task ConnectAsync(CancellationToken cancellationToken) var uri = _webSocketOptions.Uri; if (!uri.StartsWith("ws://", StringComparison.OrdinalIgnoreCase) && !uri.StartsWith("wss://", StringComparison.OrdinalIgnoreCase)) { - if (_webSocketOptions.TlsOptions?.UseTls == false) + if (_webSocketOptions.TlsOptions?.UseTls == true) { - uri = "ws://" + uri; + uri = "wss://" + uri; } else { - uri = "wss://" + uri; + uri = "ws://" + uri; } } diff --git a/Source/MQTTnet.TestApp/PublicBrokerTest.cs b/Source/MQTTnet.TestApp/PublicBrokerTest.cs index 1659f2610..b43b00ef7 100644 --- a/Source/MQTTnet.TestApp/PublicBrokerTest.cs +++ b/Source/MQTTnet.TestApp/PublicBrokerTest.cs @@ -25,7 +25,9 @@ public static async Task RunAsync() UseTls = true, SslProtocol = SslProtocols.Tls13, // Don't use this in production code. This handler simply allows any invalid certificate to work. - CertificateValidationHandler = w => true + AllowUntrustedCertificates = true, + IgnoreCertificateChainErrors = true, + CertificateValidationHandler = _ => true }; #endif // Also defining TLS12 for servers that don't seem no to support TLS13. @@ -34,7 +36,9 @@ public static async Task RunAsync() UseTls = true, SslProtocol = SslProtocols.Tls12, // Don't use this in production code. This handler simply allows any invalid certificate to work. - CertificateValidationHandler = w => true + AllowUntrustedCertificates = true, + IgnoreCertificateChainErrors = true, + CertificateValidationHandler = _ => true }; // mqtt.eclipseprojects.io @@ -97,10 +101,10 @@ await ExecuteTestAsync( "test.mosquitto.org WS TLS12", new MqttClientOptionsBuilder().WithWebSocketServer(o => o.WithUri("test.mosquitto.org:8081/mqtt")).WithProtocolVersion(MqttProtocolVersion.V311).WithTlsOptions(unsafeTls12).Build()); - // await ExecuteTestAsync( - // "test.mosquitto.org WS TLS12 (WebSocket4Net)", - // new MqttClientOptionsBuilder().WithWebSocketServer("test.mosquitto.org:8081/mqtt").WithProtocolVersion(MqttProtocolVersion.V311).WithTls(unsafeTls12).Build(), - // true); + await ExecuteTestAsync( + "test.mosquitto.org WS TLS12 (WebSocket4Net)", + new MqttClientOptionsBuilder().WithWebSocketServer(o => o.WithUri("test.mosquitto.org:8081/mqtt")).WithProtocolVersion(MqttProtocolVersion.V311).WithTlsOptions(unsafeTls12).Build(), + true); // broker.emqx.io await ExecuteTestAsync( @@ -150,7 +154,6 @@ await ExecuteTestAsync( true); // mqtt.swifitch.cz: Does not seem to operate any more - // cloudmqtt.com: Cannot test because it does not offer a free plan any more. Write("Finished.", ConsoleColor.White); @@ -197,9 +200,10 @@ static async Task ExecuteTestAsync(string name, MqttClientOptions options, bool Write("[OK]\n", ConsoleColor.Green); } - catch (Exception e) + catch (Exception exception) { - Write("[FAILED] " + e.Message + "\n", ConsoleColor.Red); + Write("[FAILED]" + Environment.NewLine, ConsoleColor.Red); + Write(exception + Environment.NewLine, ConsoleColor.Red); } } diff --git a/Source/MQTTnet/Client/Options/MqttClientOptionsBuilder.cs b/Source/MQTTnet/Client/Options/MqttClientOptionsBuilder.cs index 768d87541..65d4f6d3a 100644 --- a/Source/MQTTnet/Client/Options/MqttClientOptionsBuilder.cs +++ b/Source/MQTTnet/Client/Options/MqttClientOptionsBuilder.cs @@ -32,7 +32,12 @@ public MqttClientOptions Build() throw new InvalidOperationException("A channel must be set."); } - var tlsOptions = _tlsOptions; + // The user can specify the TCP options with already configured TLS options + // or start with TLS settings not knowing which transport will be used (depending + // on the order of called methods from the builder). + // The builder prefers the explicitly set TLS options! + var tlsOptions = _tlsOptions ?? _tcpOptions?.TlsOptions; + if (_tlsParameters != null) { if (_tlsParameters?.UseTls == true) diff --git a/Source/MQTTnet/Implementations/MqttWebSocketChannel.cs b/Source/MQTTnet/Implementations/MqttWebSocketChannel.cs index fdddcf6e0..a9f2b92ec 100644 --- a/Source/MQTTnet/Implementations/MqttWebSocketChannel.cs +++ b/Source/MQTTnet/Implementations/MqttWebSocketChannel.cs @@ -46,13 +46,13 @@ public async Task ConnectAsync(CancellationToken cancellationToken) var uri = _options.Uri; if (!uri.StartsWith("ws://", StringComparison.OrdinalIgnoreCase) && !uri.StartsWith("wss://", StringComparison.OrdinalIgnoreCase)) { - if (_options.TlsOptions?.UseTls == false) + if (_options.TlsOptions?.UseTls == true) { - uri = "ws://" + uri; + uri = "wss://" + uri; } else { - uri = "wss://" + uri; + uri = "ws://" + uri; } }