From ae96529ece2f41cb552d0dc349f2fce3c0866c57 Mon Sep 17 00:00:00 2001 From: Alex Bespalov Date: Mon, 5 Aug 2024 15:14:07 +0300 Subject: [PATCH] Added `NettyDiscoveryBaseHandler` and packet size validation --- .../NettyDiscoveryV5HandlerTests.cs | 3 +- .../Discv5/DiscoveryV5App.cs | 6 ++- .../Discv5/NettyDiscoveryV5Handler.cs | 12 +++--- .../NettyDiscoveryBaseHandler.cs | 41 +++++++++++++++++++ .../NettyDiscoveryHandler.cs | 7 ++-- 5 files changed, 58 insertions(+), 11 deletions(-) create mode 100644 src/Nethermind/Nethermind.Network.Discovery/NettyDiscoveryBaseHandler.cs diff --git a/src/Nethermind/Nethermind.Network.Discovery.Test/NettyDiscoveryV5HandlerTests.cs b/src/Nethermind/Nethermind.Network.Discovery.Test/NettyDiscoveryV5HandlerTests.cs index 8619ae71296..81756692ca5 100644 --- a/src/Nethermind/Nethermind.Network.Discovery.Test/NettyDiscoveryV5HandlerTests.cs +++ b/src/Nethermind/Nethermind.Network.Discovery.Test/NettyDiscoveryV5HandlerTests.cs @@ -12,6 +12,7 @@ using DotNetty.Transport.Channels.Sockets; using FluentAssertions; using Microsoft.Extensions.Logging; +using Nethermind.Logging; using Nethermind.Serialization.Rlp; using NSubstitute; using NUnit.Framework; @@ -29,7 +30,7 @@ public class NettyDiscoveryV5HandlerTests public void Initialize() { _channel = new(); - _handler = new(new LoggerFactory()); + _handler = new(new TestLogManager()); _handler.InitializeChannel(_channel); } diff --git a/src/Nethermind/Nethermind.Network.Discovery/Discv5/DiscoveryV5App.cs b/src/Nethermind/Nethermind.Network.Discovery/Discv5/DiscoveryV5App.cs index 6be89d2ea3d..8453c4cd941 100644 --- a/src/Nethermind/Nethermind.Network.Discovery/Discv5/DiscoveryV5App.cs +++ b/src/Nethermind/Nethermind.Network.Discovery/Discv5/DiscoveryV5App.cs @@ -91,7 +91,11 @@ .. _discoveryDb.GetAllValues().Select(enr => enrFactory.CreateFromBytes(enr, ide .WithTableOptions(new TableOptions(bootstrapEnrs.Select(enr => enr.ToString()).ToArray())) .WithEnrBuilder(enrBuilder) .WithLoggerFactory(new NethermindLoggerFactory(logManager, true)) - .WithServices(NettyDiscoveryV5Handler.Register); + .WithServices(s => + { + s.AddSingleton(logManager); + NettyDiscoveryV5Handler.Register(s); + }); _discv5Protocol = discv5Builder.Build(); _discv5Protocol.NodeAdded += (e) => NodeAddedByDiscovery(e.Record); diff --git a/src/Nethermind/Nethermind.Network.Discovery/Discv5/NettyDiscoveryV5Handler.cs b/src/Nethermind/Nethermind.Network.Discovery/Discv5/NettyDiscoveryV5Handler.cs index 9c7eeda398e..bac2e601243 100644 --- a/src/Nethermind/Nethermind.Network.Discovery/Discv5/NettyDiscoveryV5Handler.cs +++ b/src/Nethermind/Nethermind.Network.Discovery/Discv5/NettyDiscoveryV5Handler.cs @@ -9,7 +9,7 @@ using DotNetty.Transport.Channels.Sockets; using Lantern.Discv5.WireProtocol.Connection; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; +using Nethermind.Logging; using Nethermind.Serialization.Rlp; namespace Nethermind.Network.Discovery; @@ -17,16 +17,16 @@ namespace Nethermind.Network.Discovery; /// /// Adapter, integrating DotNetty externally-managed with Lantern.Discv5 /// -public class NettyDiscoveryV5Handler : SimpleChannelInboundHandler, IUdpConnection +public class NettyDiscoveryV5Handler : NettyDiscoveryBaseHandler, IUdpConnection { - private readonly ILogger _logger; + private readonly ILogger _logger; private readonly Channel _inboundQueue; private IChannel? _nettyChannel; - public NettyDiscoveryV5Handler(ILoggerFactory loggerFactory) + public NettyDiscoveryV5Handler(ILogManager loggerManager) : base(loggerManager) { - _logger = loggerFactory.CreateLogger(); + _logger = loggerManager.GetClassLogger(); _inboundQueue = Channel.CreateUnbounded(); } @@ -50,7 +50,7 @@ public async Task SendAsync(byte[] data, IPEndPoint destination) } catch (SocketException exception) { - _logger.LogError(exception, "Error sending data"); + if (_logger.IsError) _logger.Error($"Error sending data", exception); throw; } } diff --git a/src/Nethermind/Nethermind.Network.Discovery/NettyDiscoveryBaseHandler.cs b/src/Nethermind/Nethermind.Network.Discovery/NettyDiscoveryBaseHandler.cs new file mode 100644 index 00000000000..cef1f12b6f5 --- /dev/null +++ b/src/Nethermind/Nethermind.Network.Discovery/NettyDiscoveryBaseHandler.cs @@ -0,0 +1,41 @@ +// SPDX-FileCopyrightText: 2024 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using DotNetty.Transport.Channels; +using DotNetty.Transport.Channels.Sockets; +using Nethermind.Logging; + +namespace Nethermind.Network.Discovery; + +public abstract class NettyDiscoveryBaseHandler : SimpleChannelInboundHandler +{ + private readonly ILogger _logger; + + // https://github.com/ethereum/devp2p/blob/master/discv4.md#wire-protocol + // https://github.com/ethereum/devp2p/blob/master/discv5/discv5-wire.md#udp-communication + protected const int MaxPacketSize = 1280; + + protected NettyDiscoveryBaseHandler(ILogManager? logManager) + { + _logger = logManager?.GetClassLogger() ?? throw new ArgumentNullException(nameof(logManager)); + } + + public override void ChannelRead(IChannelHandlerContext ctx, object msg) + { + if (msg is DatagramPacket packet && AcceptInboundMessage(packet) && !ValidatePacket(packet)) + return; + + base.ChannelRead(ctx, msg); + } + + protected bool ValidatePacket(DatagramPacket packet) + { + if (packet.Content.ReadableBytes is 0 or > MaxPacketSize) + { + if (_logger.IsWarn) _logger.Warn($"Skipping discovery packet of invalid size: {packet.Content.ReadableBytes}"); + return false; + } + + return true; + } +} diff --git a/src/Nethermind/Nethermind.Network.Discovery/NettyDiscoveryHandler.cs b/src/Nethermind/Nethermind.Network.Discovery/NettyDiscoveryHandler.cs index e966a057119..0df65c97928 100644 --- a/src/Nethermind/Nethermind.Network.Discovery/NettyDiscoveryHandler.cs +++ b/src/Nethermind/Nethermind.Network.Discovery/NettyDiscoveryHandler.cs @@ -11,10 +11,11 @@ using Nethermind.Core.Extensions; using Nethermind.Logging; using Nethermind.Network.Discovery.Messages; +using ILogger = Nethermind.Logging.ILogger; namespace Nethermind.Network.Discovery; -public class NettyDiscoveryHandler : SimpleChannelInboundHandler, IMsgSender +public class NettyDiscoveryHandler : NettyDiscoveryBaseHandler, IMsgSender { private readonly ILogger _logger; private readonly IDiscoveryManager _discoveryManager; @@ -27,7 +28,7 @@ public NettyDiscoveryHandler( IChannel? channel, IMessageSerializationService? msgSerializationService, ITimestamper? timestamper, - ILogManager? logManager) + ILogManager? logManager) : base(logManager) { _logger = logManager?.GetClassLogger() ?? throw new ArgumentNullException(nameof(logManager)); _discoveryManager = discoveryManager ?? throw new ArgumentNullException(nameof(discoveryManager)); @@ -80,7 +81,7 @@ public async Task SendMsg(DiscoveryMsg discoveryMsg) } int size = msgBuffer.ReadableBytes; - if (size > 1280) + if (size > MaxPacketSize) { if (_logger.IsWarn) _logger.Warn($"Attempting to send message larger than 1280 bytes. This is out of spec and may not work for all client. Msg: ${discoveryMsg}"); }