From 1e2cb652f17c77e8f15eee0e082258624a96e396 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Fri, 19 Apr 2024 12:08:56 -0300 Subject: [PATCH 01/20] Initial implementation of net restrict Signed-off-by: Gabriel-Trintinalia --- .../org/hyperledger/besu/RunnerBuilder.java | 15 +++++++- .../org/hyperledger/besu/cli/BesuCommand.java | 9 +++++ .../besu/cli/CommandTestAbstract.java | 1 + .../src/test/resources/everything_config.toml | 1 + .../p2p/config/RlpxConfiguration.java | 12 +++++++ .../netty/NettyConnectionInitializer.java | 36 +++++++++++++++++++ 6 files changed, 73 insertions(+), 1 deletion(-) diff --git a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java index 6379e2c1a64..e8628d0226c 100644 --- a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java @@ -193,6 +193,7 @@ public class RunnerBuilder { private JsonRpcIpcConfiguration jsonRpcIpcConfiguration; private boolean legacyForkIdEnabled; private Optional enodeDnsConfiguration; + private List allowedSubnets = new ArrayList<>(); /** * Add Vertx. @@ -587,6 +588,17 @@ public RunnerBuilder enodeDnsConfiguration(final EnodeDnsConfiguration enodeDnsC return this; } + /** + * Add subnet configuration + * + * @param allowedSubnets the allowedSubnets + * @return the runner builder + */ + public RunnerBuilder allowedSubnets(final List allowedSubnets) { + this.allowedSubnets = allowedSubnets; + return this; + } + /** * Build Runner instance. * @@ -640,7 +652,8 @@ public Runner build() { .setBindHost(p2pListenInterface) .setBindPort(p2pListenPort) .setSupportedProtocols(subProtocols) - .setClientId(BesuInfo.nodeName(identityString)); + .setClientId(BesuInfo.nodeName(identityString)) + .setAllowSubnets(allowedSubnets); networkingConfiguration.setRlpx(rlpxConfiguration).setDiscovery(discoveryConfiguration); final PeerPermissionsDenylist bannedNodes = PeerPermissionsDenylist.create(); diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index 9648122de3b..e833aca6b87 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -513,6 +513,14 @@ private InetAddress autoDiscoverDefaultIP() { return autoDiscoveredDefaultIP; } + + @Option( + names = {"--net-restrict"}, + arity = "1..*", + split = ",", + description = + "Comma-separated list of allowed IP subnets (e.g., '192.168.1.0/24,10.0.0.0/8').") + private List allowedSubnets; } @Option( @@ -2233,6 +2241,7 @@ private Runner synchronize( .storageProvider(keyValueStorageProvider(keyValueStorageName)) .rpcEndpointService(rpcEndpointServiceImpl) .enodeDnsConfiguration(getEnodeDnsConfiguration()) + .allowedSubnets(p2PDiscoveryOptionGroup.allowedSubnets) .build(); addShutdownHook(runner); diff --git a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java index deef4f648ca..3148dbe25f6 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java @@ -354,6 +354,7 @@ public void initMocks() throws Exception { when(mockRunnerBuilder.legacyForkId(anyBoolean())).thenReturn(mockRunnerBuilder); when(mockRunnerBuilder.apiConfiguration(any())).thenReturn(mockRunnerBuilder); when(mockRunnerBuilder.enodeDnsConfiguration(any())).thenReturn(mockRunnerBuilder); + when(mockRunnerBuilder.allowedSubnets(any())).thenReturn(mockRunnerBuilder); when(mockRunnerBuilder.build()).thenReturn(mockRunner); final SignatureAlgorithm signatureAlgorithm = SignatureAlgorithmFactory.getInstance(); diff --git a/besu/src/test/resources/everything_config.toml b/besu/src/test/resources/everything_config.toml index f2b112eccb1..eb5fda8b5fe 100644 --- a/besu/src/test/resources/everything_config.toml +++ b/besu/src/test/resources/everything_config.toml @@ -51,6 +51,7 @@ engine-jwt-disabled=true engine-jwt-secret="/tmp/jwt.hex" required-blocks=["8675309=123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"] discovery-dns-url="enrtree://AM5FCQLWIZX2QFPNJAP7VUERCCRNGRHWZG3YYHIUV7BVDQ5FDPRT2@nodes.example.org" +net-restrict=["none"] # chain network="MAINNET" diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/config/RlpxConfiguration.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/config/RlpxConfiguration.java index 3968971f869..588f267fa53 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/config/RlpxConfiguration.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/config/RlpxConfiguration.java @@ -17,6 +17,7 @@ import org.hyperledger.besu.ethereum.p2p.rlpx.wire.SubProtocol; import org.hyperledger.besu.util.NetworkUtility; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -29,6 +30,8 @@ public class RlpxConfiguration { private int bindPort = 30303; private List supportedProtocols = Collections.emptyList(); + private List allowSubnets = new ArrayList<>(); + public static RlpxConfiguration create() { return new RlpxConfiguration(); } @@ -74,6 +77,15 @@ public RlpxConfiguration setClientId(final String clientId) { return this; } + public RlpxConfiguration setAllowSubnets(final List allowSubnets) { + this.allowSubnets = allowSubnets; + return this; + } + + public List getAllowSubnets() { + return allowSubnets; + } + @Override public boolean equals(final Object o) { if (this == o) { diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java index f386c59a384..5e0fea4b60e 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java @@ -38,6 +38,8 @@ import java.io.IOException; import java.net.InetSocketAddress; import java.security.GeneralSecurityException; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; @@ -45,6 +47,7 @@ import java.util.stream.StreamSupport; import javax.annotation.Nonnull; +import com.google.common.base.Splitter; import io.netty.bootstrap.Bootstrap; import io.netty.bootstrap.ServerBootstrap; import io.netty.channel.Channel; @@ -56,6 +59,9 @@ import io.netty.channel.socket.SocketChannel; import io.netty.channel.socket.nio.NioServerSocketChannel; import io.netty.channel.socket.nio.NioSocketChannel; +import io.netty.handler.ipfilter.IpFilterRuleType; +import io.netty.handler.ipfilter.IpSubnetFilterRule; +import io.netty.handler.ipfilter.RuleBasedIpFilter; import io.netty.util.concurrent.SingleThreadEventExecutor; public class NettyConnectionInitializer @@ -206,6 +212,9 @@ private ChannelInitializer outboundChannelInitializer( return new ChannelInitializer() { @Override protected void initChannel(final SocketChannel ch) throws Exception { + if (!config.getAllowSubnets().isEmpty()) { + ch.pipeline().addFirst(ipRestrictionHandler()); + } ch.pipeline() .addLast( timeoutHandler( @@ -227,6 +236,9 @@ protected void initChannel(final SocketChannel ch) throws Exception { final CompletableFuture connectionFuture = new CompletableFuture<>(); connectionFuture.thenAccept( connection -> connectSubscribers.forEach(c -> c.onConnect(connection))); + if (!config.getAllowSubnets().isEmpty()) { + ch.pipeline().addFirst(ipRestrictionHandler()); + } ch.pipeline() .addLast( timeoutHandler( @@ -277,6 +289,30 @@ private TimeoutHandler timeoutHandler( () -> connectionFuture.completeExceptionally(new TimeoutException(s))); } + private RuleBasedIpFilter ipRestrictionHandler() { + IpSubnetFilterRule[] rules = parseSubnetRules(config.getAllowSubnets()); + return new RuleBasedIpFilter(rules); + } + + private IpSubnetFilterRule[] parseSubnetRules(final List allowedSubnets) { + if (allowedSubnets == null || allowedSubnets.isEmpty()) { + return new IpSubnetFilterRule[0]; // No restrictions + } + List rulesList = new ArrayList<>(); + for (String subnet : allowedSubnets) { + List parts = Splitter.on('/').splitToList(subnet.trim()); + if (parts.size() == 2) { + String ipAddress = parts.get(0); + int cidrPrefix = Integer.parseInt(parts.get(1)); + rulesList.add(new IpSubnetFilterRule(ipAddress, cidrPrefix, IpFilterRuleType.ACCEPT)); + } else { + System.err.println("Invalid subnet format: " + subnet); + } + } + rulesList.add(new IpSubnetFilterRule("0.0.0.0", 0, IpFilterRuleType.REJECT)); + return rulesList.toArray(new IpSubnetFilterRule[0]); + } + void addAdditionalOutboundHandlers(final Channel ch, final Peer peer) throws GeneralSecurityException, IOException {} From 991fc4167517def2385bfbb9f631e5830c9d9ae1 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Mon, 22 Apr 2024 13:56:17 -0300 Subject: [PATCH 02/20] Refactoring and tests Signed-off-by: Gabriel-Trintinalia --- .../netty/IpFilterRuleCreator.java | 86 +++++++++++++++++++ .../netty/NettyConnectionInitializer.java | 29 +------ .../netty/IpFilterRuleCreatorTest.java | 40 +++++++++ 3 files changed, 129 insertions(+), 26 deletions(-) create mode 100644 ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java create mode 100644 ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java new file mode 100644 index 00000000000..0ba2e1eb064 --- /dev/null +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java @@ -0,0 +1,86 @@ +/* + * Copyright Hyperledger Besu Contributors. + * + * 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. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty; + +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.util.ArrayList; +import java.util.List; + +import com.google.common.base.Splitter; +import io.netty.handler.ipfilter.IpFilterRuleType; +import io.netty.handler.ipfilter.IpSubnetFilterRule; +import io.netty.handler.ipfilter.RuleBasedIpFilter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class IpFilterRuleCreator { + private static final Logger LOG = LoggerFactory.getLogger(IpFilterRuleCreator.class); + + /** + * Creates a RuleBasedIpFilter based on a list of allowed subnets. + * + * @param allowedSubnets A list of allowed subnets in CIDR notation. + * @return A RuleBasedIpFilter configured with rules based on the allowed subnets. + */ + public static RuleBasedIpFilter createIpRestrictionHandler(final List allowedSubnets) { + IpSubnetFilterRule[] rules = parseSubnetRules(allowedSubnets); + return new RuleBasedIpFilter(rules); + } + + /** + * Parses a list of allowed subnets into an array of IpSubnetFilterRule objects. + * + * @param allowedSubnets A list of allowed subnets in CIDR notation. + * @return An array of IpSubnetFilterRule objects. + */ + public static IpSubnetFilterRule[] parseSubnetRules(final List allowedSubnets) { + if (allowedSubnets == null || allowedSubnets.isEmpty()) { + return new IpSubnetFilterRule[0]; // No restrictions + } + List rulesList = new ArrayList<>(); + for (String subnet : allowedSubnets) { + try { + IpSubnetFilterRule rule = createRule(subnet.trim()); + rulesList.add(rule); + } catch (IllegalArgumentException | UnknownHostException e) { + LOG.trace("Skipping invalid subnet: {} subnet ({})", subnet, e.getMessage()); + } + } + + // Add a "reject all" rule at the end + rulesList.add(new IpSubnetFilterRule("0.0.0.0", 0, IpFilterRuleType.REJECT)); + return rulesList.toArray(new IpSubnetFilterRule[0]); + } + + public static IpSubnetFilterRule createRule(final String cidr) throws UnknownHostException { + if (cidr == null || !cidr.contains("/")) { + throw new IllegalArgumentException("Invalid CIDR notation: " + cidr); + } + + List parts = Splitter.on('/').splitToList(cidr); + if (parts.size() != 2) { + throw new IllegalArgumentException("Invalid CIDR notation: " + cidr); + } + + String ipAddress = parts.get(0); + int cidrPrefix = Integer.parseInt(parts.get(1)); + + // Validate IP address format + InetAddress.getByName(ipAddress); // This will throw UnknownHostException for invalid IPs + + return new IpSubnetFilterRule(ipAddress, cidrPrefix, IpFilterRuleType.ACCEPT); + } +} diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java index 5e0fea4b60e..623a4cc6c4b 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java @@ -14,6 +14,8 @@ */ package org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty; +import static org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty.IpFilterRuleCreator.createIpRestrictionHandler; + import org.hyperledger.besu.cryptoservices.NodeKey; import org.hyperledger.besu.ethereum.p2p.config.RlpxConfiguration; import org.hyperledger.besu.ethereum.p2p.discovery.DiscoveryPeer; @@ -38,8 +40,6 @@ import java.io.IOException; import java.net.InetSocketAddress; import java.security.GeneralSecurityException; -import java.util.ArrayList; -import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; @@ -47,7 +47,6 @@ import java.util.stream.StreamSupport; import javax.annotation.Nonnull; -import com.google.common.base.Splitter; import io.netty.bootstrap.Bootstrap; import io.netty.bootstrap.ServerBootstrap; import io.netty.channel.Channel; @@ -59,8 +58,6 @@ import io.netty.channel.socket.SocketChannel; import io.netty.channel.socket.nio.NioServerSocketChannel; import io.netty.channel.socket.nio.NioSocketChannel; -import io.netty.handler.ipfilter.IpFilterRuleType; -import io.netty.handler.ipfilter.IpSubnetFilterRule; import io.netty.handler.ipfilter.RuleBasedIpFilter; import io.netty.util.concurrent.SingleThreadEventExecutor; @@ -290,27 +287,7 @@ private TimeoutHandler timeoutHandler( } private RuleBasedIpFilter ipRestrictionHandler() { - IpSubnetFilterRule[] rules = parseSubnetRules(config.getAllowSubnets()); - return new RuleBasedIpFilter(rules); - } - - private IpSubnetFilterRule[] parseSubnetRules(final List allowedSubnets) { - if (allowedSubnets == null || allowedSubnets.isEmpty()) { - return new IpSubnetFilterRule[0]; // No restrictions - } - List rulesList = new ArrayList<>(); - for (String subnet : allowedSubnets) { - List parts = Splitter.on('/').splitToList(subnet.trim()); - if (parts.size() == 2) { - String ipAddress = parts.get(0); - int cidrPrefix = Integer.parseInt(parts.get(1)); - rulesList.add(new IpSubnetFilterRule(ipAddress, cidrPrefix, IpFilterRuleType.ACCEPT)); - } else { - System.err.println("Invalid subnet format: " + subnet); - } - } - rulesList.add(new IpSubnetFilterRule("0.0.0.0", 0, IpFilterRuleType.REJECT)); - return rulesList.toArray(new IpSubnetFilterRule[0]); + return createIpRestrictionHandler(config.getAllowSubnets()); } void addAdditionalOutboundHandlers(final Channel ch, final Peer peer) diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java new file mode 100644 index 00000000000..228ed78fe38 --- /dev/null +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java @@ -0,0 +1,40 @@ +package org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import io.netty.handler.ipfilter.IpFilterRule; +import org.junit.jupiter.api.Test; + +public class IpFilterRuleCreatorTest { + + @Test + void testCreateIpRestrictionHandlerWithValidSubnets() { + List allowedSubnets = Arrays.asList("192.168.1.0/24", "10.0.0.0/8"); + IpFilterRule[] rules = IpFilterRuleCreator.parseSubnetRules(allowedSubnets); + assertEquals(3, rules.length); // 2 accept rules + Reject all only + } + + @Test + void testCreateIpRestrictionHandlerWithInvalidSubnetFormat() { + List allowedSubnets = Collections.singletonList("192.168.1.0"); // Missing CIDR prefix + IpFilterRule[] rules = IpFilterRuleCreator.parseSubnetRules(allowedSubnets); + assertEquals(1, rules.length); // Reject all only + } + + @Test + void testCreateIpRestrictionHandlerWithEmptyList() { + List allowedSubnets = Collections.emptyList(); + IpFilterRule[] rules = IpFilterRuleCreator.parseSubnetRules(allowedSubnets); + assertEquals(0, rules.length); // No rules should be present + } + + @Test + void testCreateIpRestrictionHandlerWithNullList() { + IpFilterRule[] rules = IpFilterRuleCreator.parseSubnetRules(null); + assertEquals(0, rules.length); // No rules should be present + } +} From bce4085ddf0581f0732d7c539ab41d627745c733 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Mon, 22 Apr 2024 18:09:29 -0300 Subject: [PATCH 03/20] Simplify code Signed-off-by: Gabriel-Trintinalia --- .../p2p/rlpx/connections/netty/IpFilterRuleCreator.java | 9 +-------- .../connections/netty/NettyConnectionInitializer.java | 8 ++------ .../rlpx/connections/netty/IpFilterRuleCreatorTest.java | 4 ++-- 3 files changed, 5 insertions(+), 16 deletions(-) diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java index 0ba2e1eb064..ace70e35a8c 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java @@ -14,7 +14,6 @@ */ package org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty; -import java.net.InetAddress; import java.net.UnknownHostException; import java.util.ArrayList; import java.util.List; @@ -37,7 +36,7 @@ public class IpFilterRuleCreator { */ public static RuleBasedIpFilter createIpRestrictionHandler(final List allowedSubnets) { IpSubnetFilterRule[] rules = parseSubnetRules(allowedSubnets); - return new RuleBasedIpFilter(rules); + return new RuleBasedIpFilter(false, rules); } /** @@ -59,9 +58,6 @@ public static IpSubnetFilterRule[] parseSubnetRules(final List allowedSu LOG.trace("Skipping invalid subnet: {} subnet ({})", subnet, e.getMessage()); } } - - // Add a "reject all" rule at the end - rulesList.add(new IpSubnetFilterRule("0.0.0.0", 0, IpFilterRuleType.REJECT)); return rulesList.toArray(new IpSubnetFilterRule[0]); } @@ -78,9 +74,6 @@ public static IpSubnetFilterRule createRule(final String cidr) throws UnknownHos String ipAddress = parts.get(0); int cidrPrefix = Integer.parseInt(parts.get(1)); - // Validate IP address format - InetAddress.getByName(ipAddress); // This will throw UnknownHostException for invalid IPs - return new IpSubnetFilterRule(ipAddress, cidrPrefix, IpFilterRuleType.ACCEPT); } } diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java index 623a4cc6c4b..e6bc3b9bc5e 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java @@ -209,9 +209,7 @@ private ChannelInitializer outboundChannelInitializer( return new ChannelInitializer() { @Override protected void initChannel(final SocketChannel ch) throws Exception { - if (!config.getAllowSubnets().isEmpty()) { - ch.pipeline().addFirst(ipRestrictionHandler()); - } + ch.pipeline().addFirst(ipRestrictionHandler()); ch.pipeline() .addLast( timeoutHandler( @@ -233,9 +231,7 @@ protected void initChannel(final SocketChannel ch) throws Exception { final CompletableFuture connectionFuture = new CompletableFuture<>(); connectionFuture.thenAccept( connection -> connectSubscribers.forEach(c -> c.onConnect(connection))); - if (!config.getAllowSubnets().isEmpty()) { - ch.pipeline().addFirst(ipRestrictionHandler()); - } + ch.pipeline().addFirst(ipRestrictionHandler()); ch.pipeline() .addLast( timeoutHandler( diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java index 228ed78fe38..c7863078f0d 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java @@ -15,14 +15,14 @@ public class IpFilterRuleCreatorTest { void testCreateIpRestrictionHandlerWithValidSubnets() { List allowedSubnets = Arrays.asList("192.168.1.0/24", "10.0.0.0/8"); IpFilterRule[] rules = IpFilterRuleCreator.parseSubnetRules(allowedSubnets); - assertEquals(3, rules.length); // 2 accept rules + Reject all only + assertEquals(2, rules.length); } @Test void testCreateIpRestrictionHandlerWithInvalidSubnetFormat() { List allowedSubnets = Collections.singletonList("192.168.1.0"); // Missing CIDR prefix IpFilterRule[] rules = IpFilterRuleCreator.parseSubnetRules(allowedSubnets); - assertEquals(1, rules.length); // Reject all only + assertEquals(0, rules.length); } @Test From d9b6073e4e7fd7787cb99f84fd12afe96e12463f Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Fri, 31 May 2024 15:05:17 +1000 Subject: [PATCH 04/20] Remove manual parsing of rule filter Signed-off-by: Gabriel-Trintinalia --- .../netty/IpFilterRuleCreator.java | 34 +++++-------------- .../netty/NettyConnectionInitializer.java | 4 +-- .../netty/IpFilterRuleCreatorTest.java | 14 ++++++++ 3 files changed, 25 insertions(+), 27 deletions(-) diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java index ace70e35a8c..7d6c7e638b8 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java @@ -1,5 +1,5 @@ /* - * Copyright Hyperledger Besu Contributors. + * Copyright contributors to Hyperledger Besu. * * 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 @@ -14,11 +14,9 @@ */ package org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty; -import java.net.UnknownHostException; import java.util.ArrayList; import java.util.List; -import com.google.common.base.Splitter; import io.netty.handler.ipfilter.IpFilterRuleType; import io.netty.handler.ipfilter.IpSubnetFilterRule; import io.netty.handler.ipfilter.RuleBasedIpFilter; @@ -34,9 +32,11 @@ public class IpFilterRuleCreator { * @param allowedSubnets A list of allowed subnets in CIDR notation. * @return A RuleBasedIpFilter configured with rules based on the allowed subnets. */ - public static RuleBasedIpFilter createIpRestrictionHandler(final List allowedSubnets) { - IpSubnetFilterRule[] rules = parseSubnetRules(allowedSubnets); - return new RuleBasedIpFilter(false, rules); + public static RuleBasedIpFilter createRuleBasedIpFilter(final List allowedSubnets) { + if (allowedSubnets == null || allowedSubnets.isEmpty()) { + return new RuleBasedIpFilter(true); // No restrictions + } + return new RuleBasedIpFilter(false, parseSubnetRules(allowedSubnets)); } /** @@ -47,33 +47,17 @@ public static RuleBasedIpFilter createIpRestrictionHandler(final List al */ public static IpSubnetFilterRule[] parseSubnetRules(final List allowedSubnets) { if (allowedSubnets == null || allowedSubnets.isEmpty()) { - return new IpSubnetFilterRule[0]; // No restrictions + return new IpSubnetFilterRule[0]; } List rulesList = new ArrayList<>(); for (String subnet : allowedSubnets) { try { - IpSubnetFilterRule rule = createRule(subnet.trim()); + IpSubnetFilterRule rule = new IpSubnetFilterRule(subnet, IpFilterRuleType.ACCEPT); rulesList.add(rule); - } catch (IllegalArgumentException | UnknownHostException e) { + } catch (IllegalArgumentException e) { LOG.trace("Skipping invalid subnet: {} subnet ({})", subnet, e.getMessage()); } } return rulesList.toArray(new IpSubnetFilterRule[0]); } - - public static IpSubnetFilterRule createRule(final String cidr) throws UnknownHostException { - if (cidr == null || !cidr.contains("/")) { - throw new IllegalArgumentException("Invalid CIDR notation: " + cidr); - } - - List parts = Splitter.on('/').splitToList(cidr); - if (parts.size() != 2) { - throw new IllegalArgumentException("Invalid CIDR notation: " + cidr); - } - - String ipAddress = parts.get(0); - int cidrPrefix = Integer.parseInt(parts.get(1)); - - return new IpSubnetFilterRule(ipAddress, cidrPrefix, IpFilterRuleType.ACCEPT); - } } diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java index e6bc3b9bc5e..cc19e75df6e 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java @@ -14,7 +14,7 @@ */ package org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty; -import static org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty.IpFilterRuleCreator.createIpRestrictionHandler; +import static org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty.IpFilterRuleCreator.createRuleBasedIpFilter; import org.hyperledger.besu.cryptoservices.NodeKey; import org.hyperledger.besu.ethereum.p2p.config.RlpxConfiguration; @@ -283,7 +283,7 @@ private TimeoutHandler timeoutHandler( } private RuleBasedIpFilter ipRestrictionHandler() { - return createIpRestrictionHandler(config.getAllowSubnets()); + return createRuleBasedIpFilter(config.getAllowSubnets()); } void addAdditionalOutboundHandlers(final Channel ch, final Peer peer) diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java index c7863078f0d..cd01a98c026 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java @@ -1,3 +1,17 @@ +/* + * Copyright contributors to Hyperledger Besu. + * + * 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. + * + * SPDX-License-Identifier: Apache-2.0 + */ package org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty; import static org.junit.jupiter.api.Assertions.assertEquals; From 6971290f8a7cb5ae672e7d7658bd9d02dbac425e Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Tue, 4 Jun 2024 09:50:56 +1000 Subject: [PATCH 05/20] Refactor to fail at startup instead of runtime Signed-off-by: Gabriel-Trintinalia --- .../org/hyperledger/besu/RunnerBuilder.java | 5 +- .../org/hyperledger/besu/cli/BesuCommand.java | 5 +- .../IpSubnetFilterRuleConverter.java | 46 +++++++++++++++ .../hyperledger/besu/cli/BesuCommandTest.java | 19 +++++++ .../besu/cli/CommandTestAbstract.java | 2 + .../p2p/config/RlpxConfiguration.java | 8 ++- .../netty/IpFilterRuleCreator.java | 24 ++++---- .../netty/IpFilterRuleCreatorTest.java | 56 +++++++++++++++---- 8 files changed, 134 insertions(+), 31 deletions(-) create mode 100644 besu/src/main/java/org/hyperledger/besu/cli/converter/IpSubnetFilterRuleConverter.java diff --git a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java index 20d76942987..edf304490fd 100644 --- a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java @@ -144,6 +144,7 @@ import com.google.common.base.Preconditions; import com.google.common.base.Strings; import graphql.GraphQL; +import io.netty.handler.ipfilter.IpSubnetFilterRule; import io.vertx.core.Vertx; import io.vertx.core.VertxOptions; import org.apache.tuweni.bytes.Bytes; @@ -192,7 +193,7 @@ public class RunnerBuilder { private JsonRpcIpcConfiguration jsonRpcIpcConfiguration; private boolean legacyForkIdEnabled; private Optional enodeDnsConfiguration; - private List allowedSubnets = new ArrayList<>(); + private List allowedSubnets = new ArrayList<>(); /** Instantiates a new Runner builder. */ public RunnerBuilder() {} @@ -596,7 +597,7 @@ public RunnerBuilder enodeDnsConfiguration(final EnodeDnsConfiguration enodeDnsC * @param allowedSubnets the allowedSubnets * @return the runner builder */ - public RunnerBuilder allowedSubnets(final List allowedSubnets) { + public RunnerBuilder allowedSubnets(final List allowedSubnets) { this.allowedSubnets = allowedSubnets; return this; } diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index eb2bd6bf1ab..663c3f09a42 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -41,6 +41,7 @@ import org.hyperledger.besu.cli.config.EthNetworkConfig; import org.hyperledger.besu.cli.config.NetworkName; import org.hyperledger.besu.cli.config.ProfileName; +import org.hyperledger.besu.cli.converter.IpSubnetFilterRuleConverter; import org.hyperledger.besu.cli.converter.MetricCategoryConverter; import org.hyperledger.besu.cli.converter.PercentageConverter; import org.hyperledger.besu.cli.custom.JsonRPCAllowlistHostsProperty; @@ -239,6 +240,7 @@ import com.google.common.base.Strings; import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableMap; +import io.netty.handler.ipfilter.IpSubnetFilterRule; import io.vertx.core.Vertx; import io.vertx.core.VertxOptions; import io.vertx.core.json.DecodeException; @@ -532,9 +534,10 @@ private InetAddress autoDiscoverDefaultIP() { names = {"--net-restrict"}, arity = "1..*", split = ",", + converter = IpSubnetFilterRuleConverter.class, description = "Comma-separated list of allowed IP subnets (e.g., '192.168.1.0/24,10.0.0.0/8').") - private List allowedSubnets; + private List allowedSubnets; } @Option( diff --git a/besu/src/main/java/org/hyperledger/besu/cli/converter/IpSubnetFilterRuleConverter.java b/besu/src/main/java/org/hyperledger/besu/cli/converter/IpSubnetFilterRuleConverter.java new file mode 100644 index 00000000000..693bc5cc605 --- /dev/null +++ b/besu/src/main/java/org/hyperledger/besu/cli/converter/IpSubnetFilterRuleConverter.java @@ -0,0 +1,46 @@ +/* + * Copyright contributors to Hyperledger Besu. + * + * 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. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.cli.converter; + +import org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty.IpFilterRuleCreator; + +import java.util.List; +import java.util.stream.Stream; + +import io.netty.handler.ipfilter.IpSubnetFilterRule; +import picocli.CommandLine; + +public class IpSubnetFilterRuleConverter + implements CommandLine.ITypeConverter> { + /** Default Constructor. */ + public IpSubnetFilterRuleConverter() {} + + /** + * Converts a comma-separated string of IP addresses with CIDR notation into a list of + * IpSubnetFilterRule objects. Each IP address is accepted by the filter rule. + * + * @param value The comma-separated string of IP addresses with CIDR notation. + * @return A list of IpSubnetFilterRule objects, or an empty list if the input is null or blank. + */ + @Override + public List convert(final String value) { + // Check if the input string is null or blank, and return an empty list if true. + if (value == null || value.isBlank()) { + return List.of(); + } + List list = Stream.of(value.split(",")).toList(); + return IpFilterRuleCreator.parseSubnetRules(list); + } +} diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java index 74579a975d2..13c48d06b48 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java @@ -58,6 +58,7 @@ import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.p2p.peers.EnodeURLImpl; +import org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty.IpFilterRuleCreator; import org.hyperledger.besu.ethereum.worldstate.DataStorageConfiguration; import org.hyperledger.besu.evm.precompile.AbstractAltBnPrecompiledContract; import org.hyperledger.besu.evm.precompile.KZGPointEvalPrecompiledContract; @@ -1217,6 +1218,24 @@ public void parsesInvalidFastSyncMinPeersOptionWrongFormatShouldFail() { .contains("Invalid value for option '--fast-sync-min-peers': 'ten' is not an int"); } + @Test + public void netRestrictParsedCorrectly() { + final String subnet = "127.0.0.1/24"; + parseCommand("--net-restrict", subnet); + verify(mockRunnerBuilder).allowedSubnets(subnetsArgumentCaptor.capture()); + var expected = IpFilterRuleCreator.parseSubnetRules(List.of(subnet)); + assertThat(subnetsArgumentCaptor.getValue().get(0).compareTo(expected.get(0))).isEqualTo(0); + } + + @Test + public void netRestrictInvalidShouldFail() { + final String subnet = "127.0.0.1/abc"; + parseCommand("--net-restrict", subnet); + Mockito.verifyNoInteractions(mockRunnerBuilder); + assertThat(commandErrorOutput.toString(UTF_8)) + .contains("Invalid value for option '--net-restrict'"); + } + @Test public void ethStatsOptionIsParsedCorrectly() { final String url = "besu-node:secret@host:443"; diff --git a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java index 86b21715113..2372565912c 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java @@ -113,6 +113,7 @@ import java.util.function.Function; import java.util.function.Supplier; +import io.netty.handler.ipfilter.IpSubnetFilterRule; import io.opentelemetry.api.GlobalOpenTelemetry; import io.vertx.core.Vertx; import io.vertx.core.VertxOptions; @@ -261,6 +262,7 @@ public abstract class CommandTestAbstract { @Captor protected ArgumentCaptor apiConfigurationCaptor; @Captor protected ArgumentCaptor ethstatsOptionsArgumentCaptor; + @Captor protected ArgumentCaptor> subnetsArgumentCaptor; @BeforeEach public void initMocks() throws Exception { diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/config/RlpxConfiguration.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/config/RlpxConfiguration.java index 588f267fa53..c3bb9127c69 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/config/RlpxConfiguration.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/config/RlpxConfiguration.java @@ -23,6 +23,8 @@ import java.util.List; import java.util.Objects; +import io.netty.handler.ipfilter.IpSubnetFilterRule; + public class RlpxConfiguration { public static final float DEFAULT_FRACTION_REMOTE_CONNECTIONS_ALLOWED = 0.6f; private String clientId = "TestClient/1.0.0"; @@ -30,7 +32,7 @@ public class RlpxConfiguration { private int bindPort = 30303; private List supportedProtocols = Collections.emptyList(); - private List allowSubnets = new ArrayList<>(); + private List allowSubnets = new ArrayList<>(); public static RlpxConfiguration create() { return new RlpxConfiguration(); @@ -77,12 +79,12 @@ public RlpxConfiguration setClientId(final String clientId) { return this; } - public RlpxConfiguration setAllowSubnets(final List allowSubnets) { + public RlpxConfiguration setAllowSubnets(final List allowSubnets) { this.allowSubnets = allowSubnets; return this; } - public List getAllowSubnets() { + public List getAllowSubnets() { return allowSubnets; } diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java index 7d6c7e638b8..34851ac5f27 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java @@ -17,14 +17,12 @@ import java.util.ArrayList; import java.util.List; +import com.google.common.annotations.VisibleForTesting; import io.netty.handler.ipfilter.IpFilterRuleType; import io.netty.handler.ipfilter.IpSubnetFilterRule; import io.netty.handler.ipfilter.RuleBasedIpFilter; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public class IpFilterRuleCreator { - private static final Logger LOG = LoggerFactory.getLogger(IpFilterRuleCreator.class); /** * Creates a RuleBasedIpFilter based on a list of allowed subnets. @@ -32,11 +30,12 @@ public class IpFilterRuleCreator { * @param allowedSubnets A list of allowed subnets in CIDR notation. * @return A RuleBasedIpFilter configured with rules based on the allowed subnets. */ - public static RuleBasedIpFilter createRuleBasedIpFilter(final List allowedSubnets) { + public static RuleBasedIpFilter createRuleBasedIpFilter( + final List allowedSubnets) { if (allowedSubnets == null || allowedSubnets.isEmpty()) { return new RuleBasedIpFilter(true); // No restrictions } - return new RuleBasedIpFilter(false, parseSubnetRules(allowedSubnets)); + return new RuleBasedIpFilter(false, allowedSubnets.toArray(new IpSubnetFilterRule[0])); } /** @@ -45,19 +44,16 @@ public static RuleBasedIpFilter createRuleBasedIpFilter(final List allow * @param allowedSubnets A list of allowed subnets in CIDR notation. * @return An array of IpSubnetFilterRule objects. */ - public static IpSubnetFilterRule[] parseSubnetRules(final List allowedSubnets) { + @VisibleForTesting + public static List parseSubnetRules(final List allowedSubnets) { if (allowedSubnets == null || allowedSubnets.isEmpty()) { - return new IpSubnetFilterRule[0]; + return List.of(); } List rulesList = new ArrayList<>(); for (String subnet : allowedSubnets) { - try { - IpSubnetFilterRule rule = new IpSubnetFilterRule(subnet, IpFilterRuleType.ACCEPT); - rulesList.add(rule); - } catch (IllegalArgumentException e) { - LOG.trace("Skipping invalid subnet: {} subnet ({})", subnet, e.getMessage()); - } + IpSubnetFilterRule rule = new IpSubnetFilterRule(subnet, IpFilterRuleType.ACCEPT); + rulesList.add(rule); } - return rulesList.toArray(new IpSubnetFilterRule[0]); + return rulesList; } } diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java index cd01a98c026..1944fd3f0e1 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java @@ -15,12 +15,13 @@ package org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import java.util.Arrays; import java.util.Collections; import java.util.List; -import io.netty.handler.ipfilter.IpFilterRule; +import io.netty.handler.ipfilter.IpSubnetFilterRule; import org.junit.jupiter.api.Test; public class IpFilterRuleCreatorTest { @@ -28,27 +29,60 @@ public class IpFilterRuleCreatorTest { @Test void testCreateIpRestrictionHandlerWithValidSubnets() { List allowedSubnets = Arrays.asList("192.168.1.0/24", "10.0.0.0/8"); - IpFilterRule[] rules = IpFilterRuleCreator.parseSubnetRules(allowedSubnets); - assertEquals(2, rules.length); + List rules = IpFilterRuleCreator.parseSubnetRules(allowedSubnets); + assertEquals(2, rules.size()); } @Test - void testCreateIpRestrictionHandlerWithInvalidSubnetFormat() { - List allowedSubnets = Collections.singletonList("192.168.1.0"); // Missing CIDR prefix - IpFilterRule[] rules = IpFilterRuleCreator.parseSubnetRules(allowedSubnets); - assertEquals(0, rules.length); + void testCreateIpRestrictionHandlerWithInvalidSubnet() { + assertThrows( + IllegalArgumentException.class, + () -> { + List allowedSubnets = Collections.singletonList("abc"); + IpFilterRuleCreator.parseSubnetRules(allowedSubnets); + }); + } + + @Test + void testCreateIpRestrictionHandlerMissingCIDR() { + assertThrows( + IllegalArgumentException.class, + () -> { + List allowedSubnets = Collections.singletonList("192.168.1.0"); + IpFilterRuleCreator.parseSubnetRules(allowedSubnets); + }); + } + + @Test + void testCreateIpRestrictionHandlerBigCIDR() { + assertThrows( + IllegalArgumentException.class, + () -> { + List allowedSubnets = Collections.singletonList("192.168.1.0:25"); + IpFilterRuleCreator.parseSubnetRules(allowedSubnets); + }); + } + + @Test + void testCreateIpRestrictionHandlerWithInvalidCIDR() { + assertThrows( + IllegalArgumentException.class, + () -> { + List allowedSubnets = Collections.singletonList("192.168.1.0/abc"); + IpFilterRuleCreator.parseSubnetRules(allowedSubnets); + }); } @Test void testCreateIpRestrictionHandlerWithEmptyList() { List allowedSubnets = Collections.emptyList(); - IpFilterRule[] rules = IpFilterRuleCreator.parseSubnetRules(allowedSubnets); - assertEquals(0, rules.length); // No rules should be present + List rules = IpFilterRuleCreator.parseSubnetRules(allowedSubnets); + assertEquals(0, rules.size()); // No rules should be present } @Test void testCreateIpRestrictionHandlerWithNullList() { - IpFilterRule[] rules = IpFilterRuleCreator.parseSubnetRules(null); - assertEquals(0, rules.length); // No rules should be present + List rules = IpFilterRuleCreator.parseSubnetRules(null); + assertEquals(0, rules.size()); // No rules should be present } } From 34621163d937b5568be4deea43a6e7a920d8d3b5 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Tue, 4 Jun 2024 10:00:16 +1000 Subject: [PATCH 06/20] Remove redundant utility class Signed-off-by: Gabriel-Trintinalia --- .../IpSubnetFilterRuleConverter.java | 20 ++++++- .../hyperledger/besu/cli/BesuCommandTest.java | 4 +- .../converter/IpFilterRuleConverterTest.java | 19 +++--- .../netty/IpFilterRuleCreator.java | 59 ------------------- .../netty/NettyConnectionInitializer.java | 9 ++- 5 files changed, 35 insertions(+), 76 deletions(-) rename ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java => besu/src/test/java/org/hyperledger/besu/cli/converter/IpFilterRuleConverterTest.java (79%) delete mode 100644 ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java diff --git a/besu/src/main/java/org/hyperledger/besu/cli/converter/IpSubnetFilterRuleConverter.java b/besu/src/main/java/org/hyperledger/besu/cli/converter/IpSubnetFilterRuleConverter.java index 693bc5cc605..a4d12ac91f1 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/converter/IpSubnetFilterRuleConverter.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/converter/IpSubnetFilterRuleConverter.java @@ -14,11 +14,12 @@ */ package org.hyperledger.besu.cli.converter; -import org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty.IpFilterRuleCreator; - +import java.util.ArrayList; import java.util.List; import java.util.stream.Stream; +import com.google.common.annotations.VisibleForTesting; +import io.netty.handler.ipfilter.IpFilterRuleType; import io.netty.handler.ipfilter.IpSubnetFilterRule; import picocli.CommandLine; @@ -41,6 +42,19 @@ public List convert(final String value) { return List.of(); } List list = Stream.of(value.split(",")).toList(); - return IpFilterRuleCreator.parseSubnetRules(list); + return parseSubnetRules(list); + } + + @VisibleForTesting + public static List parseSubnetRules(final List allowedSubnets) { + if (allowedSubnets == null || allowedSubnets.isEmpty()) { + return List.of(); + } + List rulesList = new ArrayList<>(); + for (String subnet : allowedSubnets) { + IpSubnetFilterRule rule = new IpSubnetFilterRule(subnet, IpFilterRuleType.ACCEPT); + rulesList.add(rule); + } + return rulesList; } } diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java index 13c48d06b48..5153271f3c9 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java @@ -25,6 +25,7 @@ import static org.hyperledger.besu.cli.config.NetworkName.MAINNET; import static org.hyperledger.besu.cli.config.NetworkName.MORDOR; import static org.hyperledger.besu.cli.config.NetworkName.SEPOLIA; +import static org.hyperledger.besu.cli.converter.IpSubnetFilterRuleConverter.parseSubnetRules; import static org.hyperledger.besu.ethereum.api.jsonrpc.RpcApis.ENGINE; import static org.hyperledger.besu.ethereum.p2p.config.DefaultDiscoveryConfiguration.MAINNET_BOOTSTRAP_NODES; import static org.hyperledger.besu.ethereum.p2p.config.DefaultDiscoveryConfiguration.MAINNET_DISCOVERY_URL; @@ -58,7 +59,6 @@ import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.p2p.peers.EnodeURLImpl; -import org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty.IpFilterRuleCreator; import org.hyperledger.besu.ethereum.worldstate.DataStorageConfiguration; import org.hyperledger.besu.evm.precompile.AbstractAltBnPrecompiledContract; import org.hyperledger.besu.evm.precompile.KZGPointEvalPrecompiledContract; @@ -1223,7 +1223,7 @@ public void netRestrictParsedCorrectly() { final String subnet = "127.0.0.1/24"; parseCommand("--net-restrict", subnet); verify(mockRunnerBuilder).allowedSubnets(subnetsArgumentCaptor.capture()); - var expected = IpFilterRuleCreator.parseSubnetRules(List.of(subnet)); + var expected = parseSubnetRules(List.of(subnet)); assertThat(subnetsArgumentCaptor.getValue().get(0).compareTo(expected.get(0))).isEqualTo(0); } diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java b/besu/src/test/java/org/hyperledger/besu/cli/converter/IpFilterRuleConverterTest.java similarity index 79% rename from ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java rename to besu/src/test/java/org/hyperledger/besu/cli/converter/IpFilterRuleConverterTest.java index 1944fd3f0e1..1b815dd2089 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreatorTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/converter/IpFilterRuleConverterTest.java @@ -12,8 +12,9 @@ * * SPDX-License-Identifier: Apache-2.0 */ -package org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty; +package org.hyperledger.besu.cli.converter; +import static org.hyperledger.besu.cli.converter.IpSubnetFilterRuleConverter.parseSubnetRules; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -24,12 +25,12 @@ import io.netty.handler.ipfilter.IpSubnetFilterRule; import org.junit.jupiter.api.Test; -public class IpFilterRuleCreatorTest { +public class IpFilterRuleConverterTest { @Test void testCreateIpRestrictionHandlerWithValidSubnets() { List allowedSubnets = Arrays.asList("192.168.1.0/24", "10.0.0.0/8"); - List rules = IpFilterRuleCreator.parseSubnetRules(allowedSubnets); + List rules = parseSubnetRules(allowedSubnets); assertEquals(2, rules.size()); } @@ -39,7 +40,7 @@ void testCreateIpRestrictionHandlerWithInvalidSubnet() { IllegalArgumentException.class, () -> { List allowedSubnets = Collections.singletonList("abc"); - IpFilterRuleCreator.parseSubnetRules(allowedSubnets); + parseSubnetRules(allowedSubnets); }); } @@ -49,7 +50,7 @@ void testCreateIpRestrictionHandlerMissingCIDR() { IllegalArgumentException.class, () -> { List allowedSubnets = Collections.singletonList("192.168.1.0"); - IpFilterRuleCreator.parseSubnetRules(allowedSubnets); + parseSubnetRules(allowedSubnets); }); } @@ -59,7 +60,7 @@ void testCreateIpRestrictionHandlerBigCIDR() { IllegalArgumentException.class, () -> { List allowedSubnets = Collections.singletonList("192.168.1.0:25"); - IpFilterRuleCreator.parseSubnetRules(allowedSubnets); + parseSubnetRules(allowedSubnets); }); } @@ -69,20 +70,20 @@ void testCreateIpRestrictionHandlerWithInvalidCIDR() { IllegalArgumentException.class, () -> { List allowedSubnets = Collections.singletonList("192.168.1.0/abc"); - IpFilterRuleCreator.parseSubnetRules(allowedSubnets); + parseSubnetRules(allowedSubnets); }); } @Test void testCreateIpRestrictionHandlerWithEmptyList() { List allowedSubnets = Collections.emptyList(); - List rules = IpFilterRuleCreator.parseSubnetRules(allowedSubnets); + List rules = parseSubnetRules(allowedSubnets); assertEquals(0, rules.size()); // No rules should be present } @Test void testCreateIpRestrictionHandlerWithNullList() { - List rules = IpFilterRuleCreator.parseSubnetRules(null); + List rules = parseSubnetRules(null); assertEquals(0, rules.size()); // No rules should be present } } diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java deleted file mode 100644 index 34851ac5f27..00000000000 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/IpFilterRuleCreator.java +++ /dev/null @@ -1,59 +0,0 @@ -/* - * Copyright contributors to Hyperledger Besu. - * - * 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. - * - * SPDX-License-Identifier: Apache-2.0 - */ -package org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty; - -import java.util.ArrayList; -import java.util.List; - -import com.google.common.annotations.VisibleForTesting; -import io.netty.handler.ipfilter.IpFilterRuleType; -import io.netty.handler.ipfilter.IpSubnetFilterRule; -import io.netty.handler.ipfilter.RuleBasedIpFilter; - -public class IpFilterRuleCreator { - - /** - * Creates a RuleBasedIpFilter based on a list of allowed subnets. - * - * @param allowedSubnets A list of allowed subnets in CIDR notation. - * @return A RuleBasedIpFilter configured with rules based on the allowed subnets. - */ - public static RuleBasedIpFilter createRuleBasedIpFilter( - final List allowedSubnets) { - if (allowedSubnets == null || allowedSubnets.isEmpty()) { - return new RuleBasedIpFilter(true); // No restrictions - } - return new RuleBasedIpFilter(false, allowedSubnets.toArray(new IpSubnetFilterRule[0])); - } - - /** - * Parses a list of allowed subnets into an array of IpSubnetFilterRule objects. - * - * @param allowedSubnets A list of allowed subnets in CIDR notation. - * @return An array of IpSubnetFilterRule objects. - */ - @VisibleForTesting - public static List parseSubnetRules(final List allowedSubnets) { - if (allowedSubnets == null || allowedSubnets.isEmpty()) { - return List.of(); - } - List rulesList = new ArrayList<>(); - for (String subnet : allowedSubnets) { - IpSubnetFilterRule rule = new IpSubnetFilterRule(subnet, IpFilterRuleType.ACCEPT); - rulesList.add(rule); - } - return rulesList; - } -} diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java index cc19e75df6e..cc9fbd2b24e 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java @@ -14,8 +14,6 @@ */ package org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty; -import static org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty.IpFilterRuleCreator.createRuleBasedIpFilter; - import org.hyperledger.besu.cryptoservices.NodeKey; import org.hyperledger.besu.ethereum.p2p.config.RlpxConfiguration; import org.hyperledger.besu.ethereum.p2p.discovery.DiscoveryPeer; @@ -58,6 +56,7 @@ import io.netty.channel.socket.SocketChannel; import io.netty.channel.socket.nio.NioServerSocketChannel; import io.netty.channel.socket.nio.NioSocketChannel; +import io.netty.handler.ipfilter.IpSubnetFilterRule; import io.netty.handler.ipfilter.RuleBasedIpFilter; import io.netty.util.concurrent.SingleThreadEventExecutor; @@ -283,7 +282,11 @@ private TimeoutHandler timeoutHandler( } private RuleBasedIpFilter ipRestrictionHandler() { - return createRuleBasedIpFilter(config.getAllowSubnets()); + if (config.getAllowSubnets() == null || config.getAllowSubnets().isEmpty()) { + return new RuleBasedIpFilter(true); // No restrictions + } + return new RuleBasedIpFilter( + false, config.getAllowSubnets().toArray(new IpSubnetFilterRule[0])); } void addAdditionalOutboundHandlers(final Channel ch, final Peer peer) From 2adb525d2693f250521b3331a90ed4ce501c3d26 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Wed, 5 Jun 2024 11:49:28 +1000 Subject: [PATCH 07/20] Fix javadoc Signed-off-by: Gabriel-Trintinalia --- .../besu/cli/converter/IpSubnetFilterRuleConverter.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/converter/IpSubnetFilterRuleConverter.java b/besu/src/main/java/org/hyperledger/besu/cli/converter/IpSubnetFilterRuleConverter.java index a4d12ac91f1..0a1301162b0 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/converter/IpSubnetFilterRuleConverter.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/converter/IpSubnetFilterRuleConverter.java @@ -23,6 +23,7 @@ import io.netty.handler.ipfilter.IpSubnetFilterRule; import picocli.CommandLine; +/** The Ip subnet filter converter for CLI options. */ public class IpSubnetFilterRuleConverter implements CommandLine.ITypeConverter> { /** Default Constructor. */ @@ -45,6 +46,13 @@ public List convert(final String value) { return parseSubnetRules(list); } + /** + * Converts a list of IP addresses with CIDR notation into a list of IpSubnetFilterRule objects. + * Each IP address is accepted by the filter rule. + * + * @param allowedSubnets A list of IP addresses with CIDR notation. + * @return A list of IpSubnetFilterRule objects, or an empty list if the input is null or blank. + */ @VisibleForTesting public static List parseSubnetRules(final List allowedSubnets) { if (allowedSubnets == null || allowedSubnets.isEmpty()) { From cabf377dde38ab111a6b26c901c6ff40090dde65 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Wed, 5 Jun 2024 20:14:13 +1000 Subject: [PATCH 08/20] Refactor to control the peer connection in the permission level Signed-off-by: Gabriel-Trintinalia --- besu/build.gradle | 1 + .../org/hyperledger/besu/RunnerBuilder.java | 18 +++-- .../org/hyperledger/besu/cli/BesuCommand.java | 8 +- ...onverter.java => SubnetInfoConverter.java} | 19 +++-- .../hyperledger/besu/cli/BesuCommandTest.java | 4 +- .../besu/cli/CommandTestAbstract.java | 4 +- ...Test.java => SubnetInfoConverterTest.java} | 13 ++-- ethereum/p2p/build.gradle | 1 + .../p2p/config/RlpxConfiguration.java | 14 ---- .../p2p/permissions/PeerPermissionSubnet.java | 78 +++++++++++++++++++ .../netty/NettyConnectionInitializer.java | 12 --- gradle/verification-metadata.xml | 23 ++++++ 12 files changed, 138 insertions(+), 57 deletions(-) rename besu/src/main/java/org/hyperledger/besu/cli/converter/{IpSubnetFilterRuleConverter.java => SubnetInfoConverter.java} (78%) rename besu/src/test/java/org/hyperledger/besu/cli/converter/{IpFilterRuleConverterTest.java => SubnetInfoConverterTest.java} (85%) create mode 100644 ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionSubnet.java diff --git a/besu/build.gradle b/besu/build.gradle index 85c17e7ff35..0b194164612 100644 --- a/besu/build.gradle +++ b/besu/build.gradle @@ -80,6 +80,7 @@ dependencies { implementation 'org.xerial.snappy:snappy-java' implementation 'tech.pegasys:jc-kzg-4844' implementation 'org.rocksdb:rocksdbjni' + implementation 'commons-net:commons-net:3.11.0' runtimeOnly 'org.apache.logging.log4j:log4j-jul' runtimeOnly 'com.splunk.logging:splunk-library-javalogging' diff --git a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java index edf304490fd..1f9c48cccc5 100644 --- a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java @@ -88,6 +88,7 @@ import org.hyperledger.besu.ethereum.p2p.network.ProtocolManager; import org.hyperledger.besu.ethereum.p2p.peers.DefaultPeer; import org.hyperledger.besu.ethereum.p2p.peers.EnodeDnsConfiguration; +import org.hyperledger.besu.ethereum.p2p.permissions.PeerPermissionSubnet; import org.hyperledger.besu.ethereum.p2p.permissions.PeerPermissions; import org.hyperledger.besu.ethereum.p2p.permissions.PeerPermissionsDenylist; import org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty.TLSConfiguration; @@ -144,9 +145,9 @@ import com.google.common.base.Preconditions; import com.google.common.base.Strings; import graphql.GraphQL; -import io.netty.handler.ipfilter.IpSubnetFilterRule; import io.vertx.core.Vertx; import io.vertx.core.VertxOptions; +import org.apache.commons.net.util.SubnetUtils.SubnetInfo; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.units.bigints.UInt256; import org.slf4j.Logger; @@ -193,7 +194,7 @@ public class RunnerBuilder { private JsonRpcIpcConfiguration jsonRpcIpcConfiguration; private boolean legacyForkIdEnabled; private Optional enodeDnsConfiguration; - private List allowedSubnets = new ArrayList<>(); + private List allowedSubnets = new ArrayList<>(); /** Instantiates a new Runner builder. */ public RunnerBuilder() {} @@ -597,7 +598,7 @@ public RunnerBuilder enodeDnsConfiguration(final EnodeDnsConfiguration enodeDnsC * @param allowedSubnets the allowedSubnets * @return the runner builder */ - public RunnerBuilder allowedSubnets(final List allowedSubnets) { + public RunnerBuilder allowedSubnets(final List allowedSubnets) { this.allowedSubnets = allowedSubnets; return this; } @@ -655,13 +656,16 @@ public Runner build() { .setBindHost(p2pListenInterface) .setBindPort(p2pListenPort) .setSupportedProtocols(subProtocols) - .setClientId(BesuInfo.nodeName(identityString)) - .setAllowSubnets(allowedSubnets); + .setClientId(BesuInfo.nodeName(identityString)); networkingConfiguration.setRlpx(rlpxConfiguration).setDiscovery(discoveryConfiguration); final PeerPermissionsDenylist bannedNodes = PeerPermissionsDenylist.create(); bannedNodeIds.forEach(bannedNodes::add); + PeerPermissionSubnet peerPermissionSubnet = new PeerPermissionSubnet(allowedSubnets); + final PeerPermissions defaultPeerPermissions = + PeerPermissions.combine(peerPermissionSubnet, bannedNodes); + final List bootnodes = discoveryConfiguration.getBootnodes(); final Synchronizer synchronizer = besuController.getSynchronizer(); @@ -681,8 +685,8 @@ public Runner build() { final PeerPermissions peerPermissions = nodePermissioningController .map(nodePC -> new PeerPermissionsAdapter(nodePC, bootnodes, context.getBlockchain())) - .map(nodePerms -> PeerPermissions.combine(nodePerms, bannedNodes)) - .orElse(bannedNodes); + .map(nodePerms -> PeerPermissions.combine(nodePerms, defaultPeerPermissions)) + .orElse(defaultPeerPermissions); LOG.info("Detecting NAT service."); final boolean fallbackEnabled = natMethod == NatMethod.AUTO || natMethodFallbackEnabled; diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index 663c3f09a42..c3d4f7d969e 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -41,9 +41,9 @@ import org.hyperledger.besu.cli.config.EthNetworkConfig; import org.hyperledger.besu.cli.config.NetworkName; import org.hyperledger.besu.cli.config.ProfileName; -import org.hyperledger.besu.cli.converter.IpSubnetFilterRuleConverter; import org.hyperledger.besu.cli.converter.MetricCategoryConverter; import org.hyperledger.besu.cli.converter.PercentageConverter; +import org.hyperledger.besu.cli.converter.SubnetInfoConverter; import org.hyperledger.besu.cli.custom.JsonRPCAllowlistHostsProperty; import org.hyperledger.besu.cli.error.BesuExecutionExceptionHandler; import org.hyperledger.besu.cli.error.BesuParameterExceptionHandler; @@ -240,11 +240,11 @@ import com.google.common.base.Strings; import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableMap; -import io.netty.handler.ipfilter.IpSubnetFilterRule; import io.vertx.core.Vertx; import io.vertx.core.VertxOptions; import io.vertx.core.json.DecodeException; import io.vertx.core.metrics.MetricsOptions; +import org.apache.commons.net.util.SubnetUtils.SubnetInfo; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.units.bigints.UInt256; import org.slf4j.Logger; @@ -534,10 +534,10 @@ private InetAddress autoDiscoverDefaultIP() { names = {"--net-restrict"}, arity = "1..*", split = ",", - converter = IpSubnetFilterRuleConverter.class, + converter = SubnetInfoConverter.class, description = "Comma-separated list of allowed IP subnets (e.g., '192.168.1.0/24,10.0.0.0/8').") - private List allowedSubnets; + private List allowedSubnets; } @Option( diff --git a/besu/src/main/java/org/hyperledger/besu/cli/converter/IpSubnetFilterRuleConverter.java b/besu/src/main/java/org/hyperledger/besu/cli/converter/SubnetInfoConverter.java similarity index 78% rename from besu/src/main/java/org/hyperledger/besu/cli/converter/IpSubnetFilterRuleConverter.java rename to besu/src/main/java/org/hyperledger/besu/cli/converter/SubnetInfoConverter.java index 0a1301162b0..492e2f829d7 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/converter/IpSubnetFilterRuleConverter.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/converter/SubnetInfoConverter.java @@ -19,15 +19,14 @@ import java.util.stream.Stream; import com.google.common.annotations.VisibleForTesting; -import io.netty.handler.ipfilter.IpFilterRuleType; -import io.netty.handler.ipfilter.IpSubnetFilterRule; +import org.apache.commons.net.util.SubnetUtils; +import org.apache.commons.net.util.SubnetUtils.SubnetInfo; import picocli.CommandLine; /** The Ip subnet filter converter for CLI options. */ -public class IpSubnetFilterRuleConverter - implements CommandLine.ITypeConverter> { +public class SubnetInfoConverter implements CommandLine.ITypeConverter> { /** Default Constructor. */ - public IpSubnetFilterRuleConverter() {} + public SubnetInfoConverter() {} /** * Converts a comma-separated string of IP addresses with CIDR notation into a list of @@ -37,7 +36,7 @@ public IpSubnetFilterRuleConverter() {} * @return A list of IpSubnetFilterRule objects, or an empty list if the input is null or blank. */ @Override - public List convert(final String value) { + public List convert(final String value) { // Check if the input string is null or blank, and return an empty list if true. if (value == null || value.isBlank()) { return List.of(); @@ -54,14 +53,14 @@ public List convert(final String value) { * @return A list of IpSubnetFilterRule objects, or an empty list if the input is null or blank. */ @VisibleForTesting - public static List parseSubnetRules(final List allowedSubnets) { + public static List parseSubnetRules(final List allowedSubnets) { if (allowedSubnets == null || allowedSubnets.isEmpty()) { return List.of(); } - List rulesList = new ArrayList<>(); + List rulesList = new ArrayList<>(); for (String subnet : allowedSubnets) { - IpSubnetFilterRule rule = new IpSubnetFilterRule(subnet, IpFilterRuleType.ACCEPT); - rulesList.add(rule); + SubnetInfo info = new SubnetUtils(subnet).getInfo(); + rulesList.add(info); } return rulesList; } diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java index 5153271f3c9..2dfa5e3d382 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java @@ -25,7 +25,7 @@ import static org.hyperledger.besu.cli.config.NetworkName.MAINNET; import static org.hyperledger.besu.cli.config.NetworkName.MORDOR; import static org.hyperledger.besu.cli.config.NetworkName.SEPOLIA; -import static org.hyperledger.besu.cli.converter.IpSubnetFilterRuleConverter.parseSubnetRules; +import static org.hyperledger.besu.cli.converter.SubnetInfoConverter.parseSubnetRules; import static org.hyperledger.besu.ethereum.api.jsonrpc.RpcApis.ENGINE; import static org.hyperledger.besu.ethereum.p2p.config.DefaultDiscoveryConfiguration.MAINNET_BOOTSTRAP_NODES; import static org.hyperledger.besu.ethereum.p2p.config.DefaultDiscoveryConfiguration.MAINNET_DISCOVERY_URL; @@ -1224,7 +1224,7 @@ public void netRestrictParsedCorrectly() { parseCommand("--net-restrict", subnet); verify(mockRunnerBuilder).allowedSubnets(subnetsArgumentCaptor.capture()); var expected = parseSubnetRules(List.of(subnet)); - assertThat(subnetsArgumentCaptor.getValue().get(0).compareTo(expected.get(0))).isEqualTo(0); + assertThat(subnetsArgumentCaptor.getValue()).isEqualTo(expected); } @Test diff --git a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java index 2372565912c..4e253d39292 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java @@ -113,11 +113,11 @@ import java.util.function.Function; import java.util.function.Supplier; -import io.netty.handler.ipfilter.IpSubnetFilterRule; import io.opentelemetry.api.GlobalOpenTelemetry; import io.vertx.core.Vertx; import io.vertx.core.VertxOptions; import io.vertx.core.json.JsonObject; +import org.apache.commons.net.util.SubnetUtils.SubnetInfo; import org.apache.commons.text.StringEscapeUtils; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; @@ -262,7 +262,7 @@ public abstract class CommandTestAbstract { @Captor protected ArgumentCaptor apiConfigurationCaptor; @Captor protected ArgumentCaptor ethstatsOptionsArgumentCaptor; - @Captor protected ArgumentCaptor> subnetsArgumentCaptor; + @Captor protected ArgumentCaptor> subnetsArgumentCaptor; @BeforeEach public void initMocks() throws Exception { diff --git a/besu/src/test/java/org/hyperledger/besu/cli/converter/IpFilterRuleConverterTest.java b/besu/src/test/java/org/hyperledger/besu/cli/converter/SubnetInfoConverterTest.java similarity index 85% rename from besu/src/test/java/org/hyperledger/besu/cli/converter/IpFilterRuleConverterTest.java rename to besu/src/test/java/org/hyperledger/besu/cli/converter/SubnetInfoConverterTest.java index 1b815dd2089..6277e9072ca 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/converter/IpFilterRuleConverterTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/converter/SubnetInfoConverterTest.java @@ -14,7 +14,7 @@ */ package org.hyperledger.besu.cli.converter; -import static org.hyperledger.besu.cli.converter.IpSubnetFilterRuleConverter.parseSubnetRules; +import static org.hyperledger.besu.cli.converter.SubnetInfoConverter.parseSubnetRules; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -22,15 +22,16 @@ import java.util.Collections; import java.util.List; -import io.netty.handler.ipfilter.IpSubnetFilterRule; +import org.apache.commons.net.util.SubnetUtils; +import org.apache.commons.net.util.SubnetUtils.SubnetInfo; import org.junit.jupiter.api.Test; -public class IpFilterRuleConverterTest { +public class SubnetInfoConverterTest { @Test void testCreateIpRestrictionHandlerWithValidSubnets() { List allowedSubnets = Arrays.asList("192.168.1.0/24", "10.0.0.0/8"); - List rules = parseSubnetRules(allowedSubnets); + List rules = parseSubnetRules(allowedSubnets); assertEquals(2, rules.size()); } @@ -77,13 +78,13 @@ void testCreateIpRestrictionHandlerWithInvalidCIDR() { @Test void testCreateIpRestrictionHandlerWithEmptyList() { List allowedSubnets = Collections.emptyList(); - List rules = parseSubnetRules(allowedSubnets); + List rules = parseSubnetRules(allowedSubnets); assertEquals(0, rules.size()); // No rules should be present } @Test void testCreateIpRestrictionHandlerWithNullList() { - List rules = parseSubnetRules(null); + List rules = parseSubnetRules(null); assertEquals(0, rules.size()); // No rules should be present } } diff --git a/ethereum/p2p/build.gradle b/ethereum/p2p/build.gradle index 7f56178978d..69ff6e4ac04 100644 --- a/ethereum/p2p/build.gradle +++ b/ethereum/p2p/build.gradle @@ -60,6 +60,7 @@ dependencies { implementation 'org.jetbrains.kotlin:kotlin-stdlib' implementation 'org.owasp.encoder:encoder' implementation 'org.xerial.snappy:snappy-java' + implementation 'commons-net:commons-net:3.11.0' annotationProcessor "org.immutables:value" implementation "org.immutables:value-annotations" diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/config/RlpxConfiguration.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/config/RlpxConfiguration.java index c3bb9127c69..3968971f869 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/config/RlpxConfiguration.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/config/RlpxConfiguration.java @@ -17,14 +17,11 @@ import org.hyperledger.besu.ethereum.p2p.rlpx.wire.SubProtocol; import org.hyperledger.besu.util.NetworkUtility; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Objects; -import io.netty.handler.ipfilter.IpSubnetFilterRule; - public class RlpxConfiguration { public static final float DEFAULT_FRACTION_REMOTE_CONNECTIONS_ALLOWED = 0.6f; private String clientId = "TestClient/1.0.0"; @@ -32,8 +29,6 @@ public class RlpxConfiguration { private int bindPort = 30303; private List supportedProtocols = Collections.emptyList(); - private List allowSubnets = new ArrayList<>(); - public static RlpxConfiguration create() { return new RlpxConfiguration(); } @@ -79,15 +74,6 @@ public RlpxConfiguration setClientId(final String clientId) { return this; } - public RlpxConfiguration setAllowSubnets(final List allowSubnets) { - this.allowSubnets = allowSubnets; - return this; - } - - public List getAllowSubnets() { - return allowSubnets; - } - @Override public boolean equals(final Object o) { if (this == o) { diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionSubnet.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionSubnet.java new file mode 100644 index 00000000000..9562d9aef66 --- /dev/null +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionSubnet.java @@ -0,0 +1,78 @@ +/* + * Copyright contributors to Hyperledger Besu. + * + * 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. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.ethereum.p2p.permissions; + +import org.hyperledger.besu.ethereum.p2p.peers.Peer; + +import java.util.List; + +import org.apache.commons.net.util.SubnetUtils.SubnetInfo; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Manages peer permissions based on IP subnet restrictions. + * + *

This class extends {@link PeerPermissions} to implement access control based on IP subnets. It + * allows for the configuration of permitted subnets and uses these configurations to determine + * whether a peer should be allowed or denied access based on its IP address. + * + *

Note: If no subnets are specified, all peers are considered permitted by default. + * + * @see PeerPermissions + */ +public class PeerPermissionSubnet extends PeerPermissions { + private static final Logger LOG = LoggerFactory.getLogger(PeerPermissionSubnet.class); + + private final List allowedSubnets; + + /** + * Constructs a new {@code PeerPermissionSubnet} instance with specified allowed subnets. + * + * @param allowedSubnets A list of {@link SubnetInfo} objects representing the subnets that are + * allowed to interact with the local node. Cannot be {@code null}. + */ + public PeerPermissionSubnet(final List allowedSubnets) { + this.allowedSubnets = allowedSubnets; + } + + /** + * Determines if a peer is permitted based on the configured subnets. + * + *

This method checks if the remote peer's IP address falls within any of the configured + * allowed subnets. If the peer's IP is within any of the allowed subnets, it is permitted. + * Otherwise, it is denied. + * + * @param localNode This parameter is not used in the current implementation. + * @param remotePeer The remote peer to check. Its IP address is used to determine permission. + * @param action Ignored. If the peer is not allowed in the subnet, all actions are now allowed. + * @return {@code true} if the peer is permitted based on its IP address; {@code false} otherwise. + */ + @Override + public boolean isPermitted(final Peer localNode, final Peer remotePeer, final Action action) { + // If no subnets are specified, all peers are permitted + if (allowedSubnets.isEmpty()) { + return true; + } + String remotePeerHostAddress = remotePeer.getEnodeURL().getIp().getHostAddress(); + for (SubnetInfo subnet : allowedSubnets) { + if (subnet.isInRange(remotePeerHostAddress)) { + return true; + } + } + LOG.info("Peer {} is not allowed in any of the configured subnets.", remotePeerHostAddress); + return false; + } +} diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java index cc9fbd2b24e..f386c59a384 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/NettyConnectionInitializer.java @@ -56,8 +56,6 @@ import io.netty.channel.socket.SocketChannel; import io.netty.channel.socket.nio.NioServerSocketChannel; import io.netty.channel.socket.nio.NioSocketChannel; -import io.netty.handler.ipfilter.IpSubnetFilterRule; -import io.netty.handler.ipfilter.RuleBasedIpFilter; import io.netty.util.concurrent.SingleThreadEventExecutor; public class NettyConnectionInitializer @@ -208,7 +206,6 @@ private ChannelInitializer outboundChannelInitializer( return new ChannelInitializer() { @Override protected void initChannel(final SocketChannel ch) throws Exception { - ch.pipeline().addFirst(ipRestrictionHandler()); ch.pipeline() .addLast( timeoutHandler( @@ -230,7 +227,6 @@ protected void initChannel(final SocketChannel ch) throws Exception { final CompletableFuture connectionFuture = new CompletableFuture<>(); connectionFuture.thenAccept( connection -> connectSubscribers.forEach(c -> c.onConnect(connection))); - ch.pipeline().addFirst(ipRestrictionHandler()); ch.pipeline() .addLast( timeoutHandler( @@ -281,14 +277,6 @@ private TimeoutHandler timeoutHandler( () -> connectionFuture.completeExceptionally(new TimeoutException(s))); } - private RuleBasedIpFilter ipRestrictionHandler() { - if (config.getAllowSubnets() == null || config.getAllowSubnets().isEmpty()) { - return new RuleBasedIpFilter(true); // No restrictions - } - return new RuleBasedIpFilter( - false, config.getAllowSubnets().toArray(new IpSubnetFilterRule[0])); - } - void addAdditionalOutboundHandlers(final Channel ch, final Peer peer) throws GeneralSecurityException, IOException {} diff --git a/gradle/verification-metadata.xml b/gradle/verification-metadata.xml index c6e7701e625..5ee870c5484 100644 --- a/gradle/verification-metadata.xml +++ b/gradle/verification-metadata.xml @@ -1353,6 +1353,14 @@ + + + + + + + + @@ -3007,6 +3015,11 @@ + + + + + @@ -3161,6 +3174,11 @@ + + + + + @@ -5135,6 +5153,11 @@ + + + + + From 9bcfdfdf17be375a5151577f27404025d32616fc Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Wed, 5 Jun 2024 20:22:19 +1000 Subject: [PATCH 09/20] Change javadoc Signed-off-by: Gabriel-Trintinalia --- .../besu/cli/converter/SubnetInfoConverter.java | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/converter/SubnetInfoConverter.java b/besu/src/main/java/org/hyperledger/besu/cli/converter/SubnetInfoConverter.java index 492e2f829d7..5ffeca9e6e2 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/converter/SubnetInfoConverter.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/converter/SubnetInfoConverter.java @@ -23,17 +23,17 @@ import org.apache.commons.net.util.SubnetUtils.SubnetInfo; import picocli.CommandLine; -/** The Ip subnet filter converter for CLI options. */ +/** The SubnetInfo converter for CLI options. */ public class SubnetInfoConverter implements CommandLine.ITypeConverter> { /** Default Constructor. */ public SubnetInfoConverter() {} /** - * Converts a comma-separated string of IP addresses with CIDR notation into a list of - * IpSubnetFilterRule objects. Each IP address is accepted by the filter rule. + * Converts a comma-separated string of IP addresses with CIDR notation into a list of SubnetInfo + * objects * * @param value The comma-separated string of IP addresses with CIDR notation. - * @return A list of IpSubnetFilterRule objects, or an empty list if the input is null or blank. + * @return A list of SubnetInfo objects, or an empty list if the input is null or blank. */ @Override public List convert(final String value) { @@ -45,13 +45,6 @@ public List convert(final String value) { return parseSubnetRules(list); } - /** - * Converts a list of IP addresses with CIDR notation into a list of IpSubnetFilterRule objects. - * Each IP address is accepted by the filter rule. - * - * @param allowedSubnets A list of IP addresses with CIDR notation. - * @return A list of IpSubnetFilterRule objects, or an empty list if the input is null or blank. - */ @VisibleForTesting public static List parseSubnetRules(final List allowedSubnets) { if (allowedSubnets == null || allowedSubnets.isEmpty()) { From 6fb5be0c8a370211544def514cddbdde5e3a82d0 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Wed, 5 Jun 2024 21:20:43 +1000 Subject: [PATCH 10/20] Change javadoc Signed-off-by: Gabriel-Trintinalia --- .../besu/cli/converter/SubnetInfoConverter.java | 11 +++++++++-- .../org/hyperledger/besu/cli/BesuCommandTest.java | 6 ++---- .../org/hyperledger/besu/cli/CommandTestAbstract.java | 2 +- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/converter/SubnetInfoConverter.java b/besu/src/main/java/org/hyperledger/besu/cli/converter/SubnetInfoConverter.java index 5ffeca9e6e2..801c7ded304 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/converter/SubnetInfoConverter.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/converter/SubnetInfoConverter.java @@ -23,14 +23,14 @@ import org.apache.commons.net.util.SubnetUtils.SubnetInfo; import picocli.CommandLine; -/** The SubnetInfo converter for CLI options. */ +/** The Ip subnet filter converter for CLI options. */ public class SubnetInfoConverter implements CommandLine.ITypeConverter> { /** Default Constructor. */ public SubnetInfoConverter() {} /** * Converts a comma-separated string of IP addresses with CIDR notation into a list of SubnetInfo - * objects + * objects. * * @param value The comma-separated string of IP addresses with CIDR notation. * @return A list of SubnetInfo objects, or an empty list if the input is null or blank. @@ -45,6 +45,13 @@ public List convert(final String value) { return parseSubnetRules(list); } + /** + * Converts a list of IP addresses with CIDR notation into a list of SubnetInfo objects. Each IP + * address is accepted by the filter rule. + * + * @param allowedSubnets A list of IP addresses with CIDR notation. + * @return A list of SubnetInfo objects, or an empty list if the input is null or blank. + */ @VisibleForTesting public static List parseSubnetRules(final List allowedSubnets) { if (allowedSubnets == null || allowedSubnets.isEmpty()) { diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java index 2dfa5e3d382..7be82c07215 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java @@ -25,7 +25,6 @@ import static org.hyperledger.besu.cli.config.NetworkName.MAINNET; import static org.hyperledger.besu.cli.config.NetworkName.MORDOR; import static org.hyperledger.besu.cli.config.NetworkName.SEPOLIA; -import static org.hyperledger.besu.cli.converter.SubnetInfoConverter.parseSubnetRules; import static org.hyperledger.besu.ethereum.api.jsonrpc.RpcApis.ENGINE; import static org.hyperledger.besu.ethereum.p2p.config.DefaultDiscoveryConfiguration.MAINNET_BOOTSTRAP_NODES; import static org.hyperledger.besu.ethereum.p2p.config.DefaultDiscoveryConfiguration.MAINNET_DISCOVERY_URL; @@ -1222,9 +1221,8 @@ public void parsesInvalidFastSyncMinPeersOptionWrongFormatShouldFail() { public void netRestrictParsedCorrectly() { final String subnet = "127.0.0.1/24"; parseCommand("--net-restrict", subnet); - verify(mockRunnerBuilder).allowedSubnets(subnetsArgumentCaptor.capture()); - var expected = parseSubnetRules(List.of(subnet)); - assertThat(subnetsArgumentCaptor.getValue()).isEqualTo(expected); + verify(mockRunnerBuilder).allowedSubnets(allowedSubnetsArgumentCaptor.capture()); + assertThat(allowedSubnetsArgumentCaptor.getValue().get(0).getCidrSignature()).isEqualTo(subnet); } @Test diff --git a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java index 4e253d39292..f620c729b68 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java @@ -262,7 +262,7 @@ public abstract class CommandTestAbstract { @Captor protected ArgumentCaptor apiConfigurationCaptor; @Captor protected ArgumentCaptor ethstatsOptionsArgumentCaptor; - @Captor protected ArgumentCaptor> subnetsArgumentCaptor; + @Captor protected ArgumentCaptor> allowedSubnetsArgumentCaptor; @BeforeEach public void initMocks() throws Exception { From 7a7bc3028ce64a87385399ad6fd6c4eed0d78e3d Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Thu, 6 Jun 2024 09:12:40 +1000 Subject: [PATCH 11/20] Change version to versions gradle Signed-off-by: Gabriel-Trintinalia --- besu/build.gradle | 2 +- ethereum/p2p/build.gradle | 2 +- gradle/versions.gradle | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/besu/build.gradle b/besu/build.gradle index 0b194164612..286ca7a30fc 100644 --- a/besu/build.gradle +++ b/besu/build.gradle @@ -80,7 +80,7 @@ dependencies { implementation 'org.xerial.snappy:snappy-java' implementation 'tech.pegasys:jc-kzg-4844' implementation 'org.rocksdb:rocksdbjni' - implementation 'commons-net:commons-net:3.11.0' + implementation 'commons-net:commons-net' runtimeOnly 'org.apache.logging.log4j:log4j-jul' runtimeOnly 'com.splunk.logging:splunk-library-javalogging' diff --git a/ethereum/p2p/build.gradle b/ethereum/p2p/build.gradle index cf827892fa9..2d0331503f5 100644 --- a/ethereum/p2p/build.gradle +++ b/ethereum/p2p/build.gradle @@ -57,7 +57,7 @@ dependencies { implementation 'org.jetbrains.kotlin:kotlin-stdlib' implementation 'org.owasp.encoder:encoder' implementation 'org.xerial.snappy:snappy-java' - implementation 'commons-net:commons-net:3.11.0' + implementation 'commons-net:commons-net' annotationProcessor "org.immutables:value" implementation "org.immutables:value-annotations" diff --git a/gradle/versions.gradle b/gradle/versions.gradle index 4dd279d7e5f..db29cfd589b 100644 --- a/gradle/versions.gradle +++ b/gradle/versions.gradle @@ -134,6 +134,7 @@ dependencyManagement { dependency 'org.apache.commons:commons-lang3:3.14.0' dependency 'org.apache.commons:commons-text:1.11.0' dependency 'org.apache.commons:commons-collections4:4.4' + dependency 'commons-net:commons-net:3.11.0' dependencySet(group: 'org.apache.logging.log4j', version: '2.22.1') { entry 'log4j-api' From 62c690e026b1b55b5d85c7a56c4566b4b33a0d1d Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Thu, 6 Jun 2024 09:54:00 +1000 Subject: [PATCH 12/20] Change CHANGELOG.md Signed-off-by: Gabriel-Trintinalia --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49d43191302..2b0e40144f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ ### Additions and Improvements - Add two counters to DefaultBlockchain in order to be able to calculate TPS and Mgas/s [#7105](https://github.com/hyperledger/besu/pull/7105) - `admin_nodeInfo` JSON/RPC call returns the currently active EVM version [#7127](https://github.com/hyperledger/besu/pull/7127) +- Add Subnet-Based Peer Permissions. [#7168](https://github.com/hyperledger/besu/pull/7168) ### Bug fixes - Make `eth_gasPrice` aware of the base fee market [#7102](https://github.com/hyperledger/besu/pull/7102) From 2ca8d51fbb98e5727e7eb2703eb28a6dc20c9e2f Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Thu, 6 Jun 2024 10:10:54 +1000 Subject: [PATCH 13/20] Add tests Signed-off-by: Gabriel-Trintinalia --- .../converter/SubnetInfoConverterTest.java | 3 +- .../PeerPermissionsSubnetTest.java | 72 +++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionsSubnetTest.java diff --git a/besu/src/test/java/org/hyperledger/besu/cli/converter/SubnetInfoConverterTest.java b/besu/src/test/java/org/hyperledger/besu/cli/converter/SubnetInfoConverterTest.java index 6277e9072ca..f071743dc6b 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/converter/SubnetInfoConverterTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/converter/SubnetInfoConverterTest.java @@ -22,7 +22,6 @@ import java.util.Collections; import java.util.List; -import org.apache.commons.net.util.SubnetUtils; import org.apache.commons.net.util.SubnetUtils.SubnetInfo; import org.junit.jupiter.api.Test; @@ -31,7 +30,7 @@ public class SubnetInfoConverterTest { @Test void testCreateIpRestrictionHandlerWithValidSubnets() { List allowedSubnets = Arrays.asList("192.168.1.0/24", "10.0.0.0/8"); - List rules = parseSubnetRules(allowedSubnets); + List rules = parseSubnetRules(allowedSubnets); assertEquals(2, rules.size()); } diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionsSubnetTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionsSubnetTest.java new file mode 100644 index 00000000000..dfe9b664c12 --- /dev/null +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionsSubnetTest.java @@ -0,0 +1,72 @@ +/* + * Copyright contributors to Hyperledger Besu. + * + * 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. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.ethereum.p2p.permissions; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.hyperledger.besu.ethereum.p2p.peers.DefaultPeer; +import org.hyperledger.besu.ethereum.p2p.peers.EnodeURLImpl; +import org.hyperledger.besu.ethereum.p2p.peers.Peer; +import org.hyperledger.besu.ethereum.p2p.permissions.PeerPermissions.Action; + +import java.util.List; + +import org.apache.commons.net.util.SubnetUtils; +import org.apache.commons.net.util.SubnetUtils.SubnetInfo; +import org.junit.jupiter.api.Test; + +public class PeerPermissionsSubnetTest { + + private final Peer remoteNode = createPeer(); + + @Test + public void peerInSubnetRangeShouldBePermitted() { + String subnet = "127.0.0.0/24"; + List allowedSubnets = List.of(new SubnetUtils(subnet).getInfo()); + PeerPermissionSubnet peerPermissionSubnet = new PeerPermissionSubnet(allowedSubnets); + checkPermissions(peerPermissionSubnet, remoteNode, true); + } + + @Test + public void peerOutSubnetRangeShouldNolBePermitted() { + String subnet = "10.0.0.0/24"; + List allowedSubnets = List.of(new SubnetUtils(subnet).getInfo()); + PeerPermissionSubnet peerPermissionSubnet = new PeerPermissionSubnet(allowedSubnets); + checkPermissions(peerPermissionSubnet, remoteNode, false); + } + + @Test + public void peerShouldBePermittedIfNoSubnets() { + PeerPermissionSubnet peerPermissionSubnet = new PeerPermissionSubnet(List.of()); + checkPermissions(peerPermissionSubnet, remoteNode, true); + } + + private void checkPermissions( + final PeerPermissions peerPermissions, final Peer remotePeer, final boolean expectedResult) { + for (Action action : Action.values()) { + assertThat(peerPermissions.isPermitted(createPeer(), remotePeer, action)) + .isEqualTo(expectedResult); + } + } + + private Peer createPeer() { + return DefaultPeer.fromEnodeURL( + EnodeURLImpl.builder() + .nodeId(Peer.randomId()) + .ipAddress("127.0.0.1") + .discoveryAndListeningPorts(EnodeURLImpl.DEFAULT_LISTENING_PORT) + .build()); + } +} From c575047fabb612da394dd77e4b9878a6992eb515 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Thu, 6 Jun 2024 10:12:24 +1000 Subject: [PATCH 14/20] Change log level Signed-off-by: Gabriel-Trintinalia --- .../besu/ethereum/p2p/permissions/PeerPermissionSubnet.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionSubnet.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionSubnet.java index 9562d9aef66..5eccc639f26 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionSubnet.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionSubnet.java @@ -72,7 +72,7 @@ public boolean isPermitted(final Peer localNode, final Peer remotePeer, final Ac return true; } } - LOG.info("Peer {} is not allowed in any of the configured subnets.", remotePeerHostAddress); + LOG.trace("Peer {} is not allowed in any of the configured subnets.", remotePeerHostAddress); return false; } } From 650537d5a36de6d9eee6be674066b88a60bed6a9 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Thu, 6 Jun 2024 10:34:31 +1000 Subject: [PATCH 15/20] Simplify code Signed-off-by: Gabriel-Trintinalia --- .../cli/converter/SubnetInfoConverter.java | 33 ++++--------------- .../converter/SubnetInfoConverterTest.java | 7 ++-- 2 files changed, 12 insertions(+), 28 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/converter/SubnetInfoConverter.java b/besu/src/main/java/org/hyperledger/besu/cli/converter/SubnetInfoConverter.java index 801c7ded304..08f68ff71d3 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/converter/SubnetInfoConverter.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/converter/SubnetInfoConverter.java @@ -16,14 +16,13 @@ import java.util.ArrayList; import java.util.List; -import java.util.stream.Stream; -import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Splitter; import org.apache.commons.net.util.SubnetUtils; import org.apache.commons.net.util.SubnetUtils.SubnetInfo; import picocli.CommandLine; -/** The Ip subnet filter converter for CLI options. */ +/** The SubnetInfo converter for CLI options. */ public class SubnetInfoConverter implements CommandLine.ITypeConverter> { /** Default Constructor. */ public SubnetInfoConverter() {} @@ -37,31 +36,13 @@ public SubnetInfoConverter() {} */ @Override public List convert(final String value) { - // Check if the input string is null or blank, and return an empty list if true. + List subnetInfo = new ArrayList<>(); if (value == null || value.isBlank()) { - return List.of(); + return subnetInfo; } - List list = Stream.of(value.split(",")).toList(); - return parseSubnetRules(list); - } - - /** - * Converts a list of IP addresses with CIDR notation into a list of SubnetInfo objects. Each IP - * address is accepted by the filter rule. - * - * @param allowedSubnets A list of IP addresses with CIDR notation. - * @return A list of SubnetInfo objects, or an empty list if the input is null or blank. - */ - @VisibleForTesting - public static List parseSubnetRules(final List allowedSubnets) { - if (allowedSubnets == null || allowedSubnets.isEmpty()) { - return List.of(); - } - List rulesList = new ArrayList<>(); - for (String subnet : allowedSubnets) { - SubnetInfo info = new SubnetUtils(subnet).getInfo(); - rulesList.add(info); + for (String subnet : Splitter.on(',').split(value)) { + subnetInfo.add(new SubnetUtils(subnet).getInfo()); } - return rulesList; + return subnetInfo; } } diff --git a/besu/src/test/java/org/hyperledger/besu/cli/converter/SubnetInfoConverterTest.java b/besu/src/test/java/org/hyperledger/besu/cli/converter/SubnetInfoConverterTest.java index f071743dc6b..1939b47570d 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/converter/SubnetInfoConverterTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/converter/SubnetInfoConverterTest.java @@ -14,7 +14,6 @@ */ package org.hyperledger.besu.cli.converter; -import static org.hyperledger.besu.cli.converter.SubnetInfoConverter.parseSubnetRules; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -83,7 +82,11 @@ void testCreateIpRestrictionHandlerWithEmptyList() { @Test void testCreateIpRestrictionHandlerWithNullList() { - List rules = parseSubnetRules(null); + List rules = new SubnetInfoConverter().convert(null); assertEquals(0, rules.size()); // No rules should be present } + + private List parseSubnetRules(final List subnets) { + return new SubnetInfoConverter().convert(String.join(",", subnets)); + } } From e7314e60c76b0b8d8f903c453eed07dc01190e03 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Thu, 6 Jun 2024 11:08:47 +1000 Subject: [PATCH 16/20] Simplify code Signed-off-by: Gabriel-Trintinalia --- .../cli/converter/SubnetInfoConverter.java | 24 ++------ .../hyperledger/besu/cli/BesuCommandTest.java | 11 +++- .../converter/SubnetInfoConverterTest.java | 55 ++++--------------- 3 files changed, 25 insertions(+), 65 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/converter/SubnetInfoConverter.java b/besu/src/main/java/org/hyperledger/besu/cli/converter/SubnetInfoConverter.java index 08f68ff71d3..9ad9db9d14e 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/converter/SubnetInfoConverter.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/converter/SubnetInfoConverter.java @@ -14,35 +14,23 @@ */ package org.hyperledger.besu.cli.converter; -import java.util.ArrayList; -import java.util.List; - -import com.google.common.base.Splitter; import org.apache.commons.net.util.SubnetUtils; import org.apache.commons.net.util.SubnetUtils.SubnetInfo; import picocli.CommandLine; /** The SubnetInfo converter for CLI options. */ -public class SubnetInfoConverter implements CommandLine.ITypeConverter> { +public class SubnetInfoConverter implements CommandLine.ITypeConverter { /** Default Constructor. */ public SubnetInfoConverter() {} /** - * Converts a comma-separated string of IP addresses with CIDR notation into a list of SubnetInfo - * objects. + * Converts an IP addresses with CIDR notation into SubnetInfo * - * @param value The comma-separated string of IP addresses with CIDR notation. - * @return A list of SubnetInfo objects, or an empty list if the input is null or blank. + * @param value The IP addresses with CIDR notation. + * @return the SubnetInfo */ @Override - public List convert(final String value) { - List subnetInfo = new ArrayList<>(); - if (value == null || value.isBlank()) { - return subnetInfo; - } - for (String subnet : Splitter.on(',').split(value)) { - subnetInfo.add(new SubnetUtils(subnet).getInfo()); - } - return subnetInfo; + public SubnetInfo convert(final String value) { + return new SubnetUtils(value).getInfo(); } } diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java index 7be82c07215..d738ed13b7a 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java @@ -1219,10 +1219,15 @@ public void parsesInvalidFastSyncMinPeersOptionWrongFormatShouldFail() { @Test public void netRestrictParsedCorrectly() { - final String subnet = "127.0.0.1/24"; - parseCommand("--net-restrict", subnet); + final String subnet1 = "127.0.0.1/24"; + final String subnet2 = "10.0.0.1/24"; + parseCommand("--net-restrict", String.join(",", subnet1, subnet2)); verify(mockRunnerBuilder).allowedSubnets(allowedSubnetsArgumentCaptor.capture()); - assertThat(allowedSubnetsArgumentCaptor.getValue().get(0).getCidrSignature()).isEqualTo(subnet); + assertThat(allowedSubnetsArgumentCaptor.getValue().size()).isEqualTo(2); + assertThat(allowedSubnetsArgumentCaptor.getValue().get(0).getCidrSignature()) + .isEqualTo(subnet1); + assertThat(allowedSubnetsArgumentCaptor.getValue().get(1).getCidrSignature()) + .isEqualTo(subnet2); } @Test diff --git a/besu/src/test/java/org/hyperledger/besu/cli/converter/SubnetInfoConverterTest.java b/besu/src/test/java/org/hyperledger/besu/cli/converter/SubnetInfoConverterTest.java index 1939b47570d..aaeb4536142 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/converter/SubnetInfoConverterTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/converter/SubnetInfoConverterTest.java @@ -14,13 +14,9 @@ */ package org.hyperledger.besu.cli.converter; -import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; - import org.apache.commons.net.util.SubnetUtils.SubnetInfo; import org.junit.jupiter.api.Test; @@ -28,65 +24,36 @@ public class SubnetInfoConverterTest { @Test void testCreateIpRestrictionHandlerWithValidSubnets() { - List allowedSubnets = Arrays.asList("192.168.1.0/24", "10.0.0.0/8"); - List rules = parseSubnetRules(allowedSubnets); - assertEquals(2, rules.size()); + String subnet = "192.168.1.0/24"; + assertThat(parseSubnetRules(subnet).getCidrSignature()).isEqualTo(subnet); } @Test void testCreateIpRestrictionHandlerWithInvalidSubnet() { - assertThrows( - IllegalArgumentException.class, - () -> { - List allowedSubnets = Collections.singletonList("abc"); - parseSubnetRules(allowedSubnets); - }); + assertThrows(IllegalArgumentException.class, () -> parseSubnetRules("abc")); } @Test void testCreateIpRestrictionHandlerMissingCIDR() { - assertThrows( - IllegalArgumentException.class, - () -> { - List allowedSubnets = Collections.singletonList("192.168.1.0"); - parseSubnetRules(allowedSubnets); - }); + assertThrows(IllegalArgumentException.class, () -> parseSubnetRules("192.168.1.0")); } @Test void testCreateIpRestrictionHandlerBigCIDR() { - assertThrows( - IllegalArgumentException.class, - () -> { - List allowedSubnets = Collections.singletonList("192.168.1.0:25"); - parseSubnetRules(allowedSubnets); - }); + assertThrows(IllegalArgumentException.class, () -> parseSubnetRules("192.168.1.0:25")); } @Test void testCreateIpRestrictionHandlerWithInvalidCIDR() { - assertThrows( - IllegalArgumentException.class, - () -> { - List allowedSubnets = Collections.singletonList("192.168.1.0/abc"); - parseSubnetRules(allowedSubnets); - }); - } - - @Test - void testCreateIpRestrictionHandlerWithEmptyList() { - List allowedSubnets = Collections.emptyList(); - List rules = parseSubnetRules(allowedSubnets); - assertEquals(0, rules.size()); // No rules should be present + assertThrows(IllegalArgumentException.class, () -> parseSubnetRules("192.168.1.0/abc")); } @Test - void testCreateIpRestrictionHandlerWithNullList() { - List rules = new SubnetInfoConverter().convert(null); - assertEquals(0, rules.size()); // No rules should be present + void testCreateIpRestrictionHandlerWithEmptyString() { + assertThrows(IllegalArgumentException.class, () -> parseSubnetRules("")); } - private List parseSubnetRules(final List subnets) { - return new SubnetInfoConverter().convert(String.join(",", subnets)); + private SubnetInfo parseSubnetRules(final String subnet) { + return new SubnetInfoConverter().convert(subnet); } } From d4119a060548ac36dffc8658f242b930f4c75d6a Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Wed, 12 Jun 2024 13:24:47 +1000 Subject: [PATCH 17/20] PR suggestions Signed-off-by: Gabriel-Trintinalia --- .../p2p/permissions/PeerPermissionSubnet.java | 2 +- .../PeerPermissionsSubnetTest.java | 19 ++++++++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionSubnet.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionSubnet.java index 5eccc639f26..e38e6f4f904 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionSubnet.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionSubnet.java @@ -66,7 +66,7 @@ public boolean isPermitted(final Peer localNode, final Peer remotePeer, final Ac if (allowedSubnets.isEmpty()) { return true; } - String remotePeerHostAddress = remotePeer.getEnodeURL().getIp().getHostAddress(); + String remotePeerHostAddress = remotePeer.getEnodeURL().getIpAsString(); for (SubnetInfo subnet : allowedSubnets) { if (subnet.isInRange(remotePeerHostAddress)) { return true; diff --git a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionsSubnetTest.java b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionsSubnetTest.java index dfe9b664c12..185858e658e 100644 --- a/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionsSubnetTest.java +++ b/ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionsSubnetTest.java @@ -33,16 +33,21 @@ public class PeerPermissionsSubnetTest { @Test public void peerInSubnetRangeShouldBePermitted() { - String subnet = "127.0.0.0/24"; - List allowedSubnets = List.of(new SubnetUtils(subnet).getInfo()); + List allowedSubnets = List.of(subnet("127.0.0.0/24")); PeerPermissionSubnet peerPermissionSubnet = new PeerPermissionSubnet(allowedSubnets); checkPermissions(peerPermissionSubnet, remoteNode, true); } @Test - public void peerOutSubnetRangeShouldNolBePermitted() { - String subnet = "10.0.0.0/24"; - List allowedSubnets = List.of(new SubnetUtils(subnet).getInfo()); + public void peerInAtLeastOneSubnetRangeShouldBePermitted() { + List allowedSubnets = List.of(subnet("127.0.0.0/24"), subnet("10.0.0.1/24")); + PeerPermissionSubnet peerPermissionSubnet = new PeerPermissionSubnet(allowedSubnets); + checkPermissions(peerPermissionSubnet, remoteNode, true); + } + + @Test + public void peerOutSubnetRangeShouldNotBePermitted() { + List allowedSubnets = List.of(subnet("10.0.0.0/24")); PeerPermissionSubnet peerPermissionSubnet = new PeerPermissionSubnet(allowedSubnets); checkPermissions(peerPermissionSubnet, remoteNode, false); } @@ -61,6 +66,10 @@ private void checkPermissions( } } + private SubnetInfo subnet(final String subnet) { + return new SubnetUtils(subnet).getInfo(); + } + private Peer createPeer() { return DefaultPeer.fromEnodeURL( EnodeURLImpl.builder() From f2c01c732b927c30c2666872967c30c109febd3e Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Wed, 12 Jun 2024 17:21:19 +1000 Subject: [PATCH 18/20] Add dependency Signed-off-by: Gabriel-Trintinalia --- gradle/verification-metadata.xml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gradle/verification-metadata.xml b/gradle/verification-metadata.xml index 1dd75565280..bc47ac15ee9 100644 --- a/gradle/verification-metadata.xml +++ b/gradle/verification-metadata.xml @@ -5154,6 +5154,9 @@ + + + From 10c928d9cdb880580a0b8afbd2c117fe62be79c5 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Wed, 12 Jun 2024 17:30:11 +1000 Subject: [PATCH 19/20] Fix sha256 Signed-off-by: Gabriel-Trintinalia --- gradle/verification-metadata.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/verification-metadata.xml b/gradle/verification-metadata.xml index bc47ac15ee9..86f65970e61 100644 --- a/gradle/verification-metadata.xml +++ b/gradle/verification-metadata.xml @@ -5155,7 +5155,7 @@ - + From 13edd8d6773f7ba25f4e6b1abeb77ab807942d4c Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Wed, 12 Jun 2024 19:07:27 +1000 Subject: [PATCH 20/20] add extra null check Signed-off-by: Gabriel-Trintinalia --- .../besu/ethereum/p2p/permissions/PeerPermissionSubnet.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionSubnet.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionSubnet.java index e38e6f4f904..720cd4dd609 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionSubnet.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/permissions/PeerPermissionSubnet.java @@ -63,7 +63,7 @@ public PeerPermissionSubnet(final List allowedSubnets) { @Override public boolean isPermitted(final Peer localNode, final Peer remotePeer, final Action action) { // If no subnets are specified, all peers are permitted - if (allowedSubnets.isEmpty()) { + if (allowedSubnets == null || allowedSubnets.isEmpty()) { return true; } String remotePeerHostAddress = remotePeer.getEnodeURL().getIpAsString();