From 535a4e010cf4d8c2e2dbf9a1b6873150657efe3e Mon Sep 17 00:00:00 2001 From: Christian <6939810+chkr1011@users.noreply.github.com> Date: Mon, 20 Oct 2025 14:33:50 +0200 Subject: [PATCH] Add validations of variable byte integers (#2214) * Add validations and add docs * Update release notes * Fix build --- Samples/Server/Server_Intercepting_Samples.cs | 29 ++++++ .../Options/MqttServerOptionsBuilder.cs | 92 +++++++++++-------- .../MqttServerTcpEndpointBaseOptions.cs | 49 ++++++---- .../MqttProtocolViolationException_Tests.cs | 24 +++++ .../MqttProtocolViolationException.cs | 13 ++- Source/MQTTnet/Formatter/MqttBufferWriter.cs | 6 +- .../MQTTnet/MqttApplicationMessageBuilder.cs | 7 +- Source/ReleaseNotes.md | 1 + 8 files changed, 157 insertions(+), 64 deletions(-) create mode 100644 Source/MQTTnet.Tests/Exceptions/MqttProtocolViolationException_Tests.cs diff --git a/Samples/Server/Server_Intercepting_Samples.cs b/Samples/Server/Server_Intercepting_Samples.cs index 68c5c8ba..dfd39563 100644 --- a/Samples/Server/Server_Intercepting_Samples.cs +++ b/Samples/Server/Server_Intercepting_Samples.cs @@ -9,6 +9,7 @@ // ReSharper disable MemberCanBeMadeStatic.Local using MQTTnet.Internal; +using MQTTnet.Protocol; using MQTTnet.Server; namespace MQTTnet.Samples.Server; @@ -42,4 +43,32 @@ public static class Server_Intercepting_Samples Console.ReadLine(); await mqttServer.StopAsync(); } + + public static async Task Intercept_Subscription() + { + /* + * This sample starts a simple MQTT server which does not allow to subscribe to '#' + * Start the server and try to subscribe to '#' with a client of choice. + */ + + var mqttServerFactory = new MqttServerFactory(); + var mqttServerOptions = new MqttServerOptionsBuilder().WithDefaultEndpoint().Build(); + + using var mqttServer = mqttServerFactory.CreateMqttServer(mqttServerOptions); + mqttServer.InterceptingSubscriptionAsync += args => + { + if (args.TopicFilter.Topic.Equals("#", StringComparison.Ordinal)) + { + args.Response.ReasonCode = MqttSubscribeReasonCode.NotAuthorized; + } + + return CompletedTask.Instance; + }; + + await mqttServer.StartAsync(); + + Console.WriteLine("Press Enter to exit."); + Console.ReadLine(); + await mqttServer.StopAsync(); + } } \ No newline at end of file diff --git a/Source/MQTTnet.Server/Options/MqttServerOptionsBuilder.cs b/Source/MQTTnet.Server/Options/MqttServerOptionsBuilder.cs index 52035890..2cbc524e 100644 --- a/Source/MQTTnet.Server/Options/MqttServerOptionsBuilder.cs +++ b/Source/MQTTnet.Server/Options/MqttServerOptionsBuilder.cs @@ -5,8 +5,8 @@ using System.Net; using System.Net.Security; using System.Security.Authentication; -using MQTTnet.Certificates; using System.Security.Cryptography.X509Certificates; +using MQTTnet.Certificates; // ReSharper disable UnusedMember.Global namespace MQTTnet.Server; @@ -47,12 +47,22 @@ public class MqttServerOptionsBuilder return this; } + /// + /// Sets the IPv4 network address to bind. + /// Defaults to any available IPv4 address of the machine. + /// Set this to `IPAddress.None` to disable the IPv4 support entirely. + /// public MqttServerOptionsBuilder WithDefaultEndpointBoundIPAddress(IPAddress value) { _options.DefaultEndpointOptions.BoundInterNetworkAddress = value ?? IPAddress.Any; return this; } + /// + /// Sets the IPv6 network address to bind. + /// Defaults to any available IPv6 address of the machine. + /// Set this to `IPAddress.None` to disable the IPv6 support entirely. + /// public MqttServerOptionsBuilder WithDefaultEndpointBoundIPV6Address(IPAddress value) { _options.DefaultEndpointOptions.BoundInterNetworkV6Address = value ?? IPAddress.Any; @@ -77,12 +87,22 @@ public class MqttServerOptionsBuilder return this; } + /// + /// Sets the IPv4 network address to bind. + /// Defaults to any available IPv4 address of the machine. + /// Set this to `IPAddress.None` to disable the IPv4 support entirely. + /// public MqttServerOptionsBuilder WithEncryptedEndpointBoundIPAddress(IPAddress value) { _options.TlsEndpointOptions.BoundInterNetworkAddress = value; return this; } + /// + /// Sets the IPv6 network address to bind. + /// Defaults to any available IPv6 address of the machine. + /// Set this to `IPAddress.None` to disable the IPv6 support entirely. + /// public MqttServerOptionsBuilder WithEncryptedEndpointBoundIPV6Address(IPAddress value) { _options.TlsEndpointOptions.BoundInterNetworkV6Address = value; @@ -95,6 +115,35 @@ public class MqttServerOptionsBuilder return this; } + public MqttServerOptionsBuilder WithEncryptionCertificate(byte[] value, IMqttServerCertificateCredentials credentials = null) + { + ArgumentNullException.ThrowIfNull(value); + + _options.TlsEndpointOptions.CertificateProvider = new BlobCertificateProvider(value) + { + Password = credentials?.Password + }; + + return this; + } + + public MqttServerOptionsBuilder WithEncryptionCertificate(X509Certificate2 certificate) + { + ArgumentNullException.ThrowIfNull(certificate); + + _options.TlsEndpointOptions.CertificateProvider = new X509CertificateProvider(certificate); + return this; + } + + public MqttServerOptionsBuilder WithEncryptionCertificate(ICertificateProvider certificateProvider) + { + ArgumentNullException.ThrowIfNull(certificateProvider); + + _options.TlsEndpointOptions.CertificateProvider = certificateProvider; + + return this; + } + public MqttServerOptionsBuilder WithEncryptionSslProtocol(SslProtocols value) { _options.TlsEndpointOptions.SslProtocol = value; @@ -114,12 +163,6 @@ public class MqttServerOptionsBuilder return this; } - public MqttServerOptionsBuilder WithPendingMessagesOverflowStrategy(MqttPendingMessagesOverflowStrategy value) - { - _options.PendingMessagesOverflowStrategy = value; - return this; - } - public MqttServerOptionsBuilder WithoutDefaultEndpoint() { _options.DefaultEndpointOptions.IsEnabled = false; @@ -144,6 +187,12 @@ public class MqttServerOptionsBuilder return this; } + public MqttServerOptionsBuilder WithPendingMessagesOverflowStrategy(MqttPendingMessagesOverflowStrategy value) + { + _options.PendingMessagesOverflowStrategy = value; + return this; + } + public MqttServerOptionsBuilder WithPersistentSessions(bool value = true) { _options.EnablePersistentSessions = value; @@ -182,33 +231,4 @@ public class MqttServerOptionsBuilder _options.TlsEndpointOptions.ReuseAddress = true; return this; } - - public MqttServerOptionsBuilder WithEncryptionCertificate(byte[] value, IMqttServerCertificateCredentials credentials = null) - { - ArgumentNullException.ThrowIfNull(value); - - _options.TlsEndpointOptions.CertificateProvider = new BlobCertificateProvider(value) - { - Password = credentials?.Password - }; - - return this; - } - - public MqttServerOptionsBuilder WithEncryptionCertificate(X509Certificate2 certificate) - { - ArgumentNullException.ThrowIfNull(certificate); - - _options.TlsEndpointOptions.CertificateProvider = new X509CertificateProvider(certificate); - return this; - } - - public MqttServerOptionsBuilder WithEncryptionCertificate(ICertificateProvider certificateProvider) - { - ArgumentNullException.ThrowIfNull(certificateProvider); - - _options.TlsEndpointOptions.CertificateProvider = certificateProvider; - - return this; - } } \ No newline at end of file diff --git a/Source/MQTTnet.Server/Options/MqttServerTcpEndpointBaseOptions.cs b/Source/MQTTnet.Server/Options/MqttServerTcpEndpointBaseOptions.cs index bdd95315..e97b3119 100644 --- a/Source/MQTTnet.Server/Options/MqttServerTcpEndpointBaseOptions.cs +++ b/Source/MQTTnet.Server/Options/MqttServerTcpEndpointBaseOptions.cs @@ -9,13 +9,30 @@ namespace MQTTnet.Server; public abstract class MqttServerTcpEndpointBaseOptions { - public bool IsEnabled { get; set; } + /// + /// Usually the MQTT packets can be sent partially. This is done by using multiple TCP packets + /// or WebSocket frames etc. Unfortunately not all clients do support this feature and + /// will close the connection when receiving such packets. If such clients are connecting to this + /// server the flag must be set to _false_. + /// + public bool AllowPacketFragmentation { get; set; } = true; - public int Port { get; set; } + /// + /// Gets or sets the IPv4 network address to bind. + /// Defaults to any available IPv4 address of the machine. + /// Set this to `IPAddress.None` to disable the IPv4 support entirely. + /// + public IPAddress BoundInterNetworkAddress { get; set; } = IPAddress.Any; + + /// + /// Gets or sets the IPv6 network address to bind. + /// Defaults to any available IPv6 address of the machine. + /// Set this to `IPAddress.None` to disable the IPv6 support entirely. + /// + public IPAddress BoundInterNetworkV6Address { get; set; } = IPAddress.IPv6Any; public int ConnectionBacklog { get; set; } = 100; - - public bool NoDelay { get; set; } = true; + public bool IsEnabled { get; set; } /// /// Gets or sets whether the sockets keep alive feature should be used. @@ -23,13 +40,16 @@ public abstract class MqttServerTcpEndpointBaseOptions /// public bool? KeepAlive { get; set; } + public LingerOption LingerState { get; set; } = new(true, 0); + + public bool NoDelay { get; set; } = true; + + public int Port { get; set; } + /// - /// Usually the MQTT packets can be send partially. This is done by using multiple TCP packets - /// or WebSocket frames etc. Unfortunately not all clients do support this feature and - /// will close the connection when receiving such packets. If such clients are connecting to this - /// server the flag must be set to _false_. + /// This requires admin permissions on Linux. /// - public bool AllowPacketFragmentation { get; set; } = true; + public bool ReuseAddress { get; set; } /// /// Gets or sets the TCP keep alive interval. @@ -48,15 +68,4 @@ public abstract class MqttServerTcpEndpointBaseOptions /// The value _null_ indicates that the OS and framework defaults should be used. /// public int? TcpKeepAliveTime { get; set; } - - public LingerOption LingerState { get; set; } = new LingerOption(true, 0); - - public IPAddress BoundInterNetworkAddress { get; set; } = IPAddress.Any; - - public IPAddress BoundInterNetworkV6Address { get; set; } = IPAddress.IPv6Any; - - /// - /// This requires admin permissions on Linux. - /// - public bool ReuseAddress { get; set; } } \ No newline at end of file diff --git a/Source/MQTTnet.Tests/Exceptions/MqttProtocolViolationException_Tests.cs b/Source/MQTTnet.Tests/Exceptions/MqttProtocolViolationException_Tests.cs new file mode 100644 index 00000000..34e59725 --- /dev/null +++ b/Source/MQTTnet.Tests/Exceptions/MqttProtocolViolationException_Tests.cs @@ -0,0 +1,24 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using Microsoft.VisualStudio.TestTools.UnitTesting; +using MQTTnet.Exceptions; + +namespace MQTTnet.Tests.Exceptions; + +[TestClass] +public class MqttProtocolViolationException_Tests +{ + [TestMethod] + public void No_For_Max_Large_Variable_Byte_Integer() + { + MqttProtocolViolationException.ThrowIfVariableByteIntegerExceedsLimit(MqttProtocolViolationException.VariableByteIntegerMaxValue); + } + + [TestMethod] + public void Throw_Exception_For_Too_Large_Variable_Byte_Integer() + { + Assert.ThrowsExactly(() => MqttProtocolViolationException.ThrowIfVariableByteIntegerExceedsLimit(1726790899)); + } +} \ No newline at end of file diff --git a/Source/MQTTnet/Exceptions/MqttProtocolViolationException.cs b/Source/MQTTnet/Exceptions/MqttProtocolViolationException.cs index f373d643..96b738be 100644 --- a/Source/MQTTnet/Exceptions/MqttProtocolViolationException.cs +++ b/Source/MQTTnet/Exceptions/MqttProtocolViolationException.cs @@ -6,4 +6,15 @@ using System; namespace MQTTnet.Exceptions; -public class MqttProtocolViolationException(string message) : Exception(message); \ No newline at end of file +public class MqttProtocolViolationException(string message) : Exception(message) +{ + public const uint VariableByteIntegerMaxValue = 268435455; + + public static void ThrowIfVariableByteIntegerExceedsLimit(uint value) + { + if (value > VariableByteIntegerMaxValue) + { + throw new MqttProtocolViolationException($"The value {value} is too large for a variable byte integer ({VariableByteIntegerMaxValue})."); + } + } +} \ No newline at end of file diff --git a/Source/MQTTnet/Formatter/MqttBufferWriter.cs b/Source/MQTTnet/Formatter/MqttBufferWriter.cs index abaca7da..d685847c 100644 --- a/Source/MQTTnet/Formatter/MqttBufferWriter.cs +++ b/Source/MQTTnet/Formatter/MqttBufferWriter.cs @@ -19,7 +19,6 @@ namespace MQTTnet.Formatter; /// public sealed class MqttBufferWriter { - const uint VariableByteIntegerMaxValue = 268435455; const int EncodedStringMaxLength = 65535; readonly int _maxBufferSize; @@ -223,10 +222,7 @@ public sealed class MqttBufferWriter return; } - if (value > VariableByteIntegerMaxValue) - { - throw new MqttProtocolViolationException($"The specified value ({value}) is too large for a variable byte integer."); - } + MqttProtocolViolationException.ThrowIfVariableByteIntegerExceedsLimit(value); var size = 0; var x = value; diff --git a/Source/MQTTnet/MqttApplicationMessageBuilder.cs b/Source/MQTTnet/MqttApplicationMessageBuilder.cs index 08a2a4a7..ce5d7a53 100644 --- a/Source/MQTTnet/MqttApplicationMessageBuilder.cs +++ b/Source/MQTTnet/MqttApplicationMessageBuilder.cs @@ -21,7 +21,6 @@ public sealed class MqttApplicationMessageBuilder string _contentType; byte[] _correlationData; uint _messageExpiryInterval; - MqttPayloadFormatIndicator _payloadFormatIndicator; ReadOnlySequence _payload; MqttQualityOfServiceLevel _qualityOfServiceLevel = MqttQualityOfServiceLevel.AtMostOnce; @@ -84,6 +83,8 @@ public sealed class MqttApplicationMessageBuilder /// public MqttApplicationMessageBuilder WithMessageExpiryInterval(uint messageExpiryInterval) { + // No validation required because this is a 4 byte integer! + _messageExpiryInterval = messageExpiryInterval; return this; } @@ -228,9 +229,11 @@ public sealed class MqttApplicationMessageBuilder /// public MqttApplicationMessageBuilder WithSubscriptionIdentifier(uint subscriptionIdentifier) { + MqttProtocolViolationException.ThrowIfVariableByteIntegerExceedsLimit(subscriptionIdentifier); + if (_subscriptionIdentifiers == null) { - _subscriptionIdentifiers = new List(); + _subscriptionIdentifiers = []; } _subscriptionIdentifiers.Add(subscriptionIdentifier); diff --git a/Source/ReleaseNotes.md b/Source/ReleaseNotes.md index d18fa9dc..c539dbe6 100644 --- a/Source/ReleaseNotes.md +++ b/Source/ReleaseNotes.md @@ -1,5 +1,6 @@ * Core: Used new language features across the entire library * Core: Performance improvements +* Core: Added validations for variable byte integers which do not match the .NET uint perfectly * Server: Improved performance of retained messages when no event handler is attached (#2093, thanks to @zhaowgit) * Server: The event `InterceptingClientEnqueue` is now also called for retained messages (BREAKING CHANGE!) * Server: The local end point is now also exposed in the channel adapter (#2179)