Use the node's configuration to decide if adding a peer with DNS in t…

…he enode is allowed (hyperledger#5584) * Use the node's configuration to decide if adding a peer with DNS in the enode is allowed Signed-off-by: Matthew Whitehead <[email protected]> * Update command test mocks Signed-off-by: Matthew Whitehead <[email protected]> * Update besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java Co-authored-by: Sally MacFarlane <[email protected]> Signed-off-by: Matt Whitehead <[email protected]> * Address PR comments. Add a reference in the change log. Fix failing integration test. Signed-off-by: Matthew Whitehead <[email protected]> * Add the same DNS-config checking logic to admin_removePeer, along with a unit test file for it Signed-off-by: Matthew Whitehead <[email protected]> * Tweak the change log Signed-off-by: Matthew Whitehead <[email protected]> * Add clearer error messages for the cases where enode DNS is disabled (but a hostname one has been specified in the URL) or where DNS name resolution failed Signed-off-by: Matthew Whitehead <[email protected]> * Spotless Java fixes Signed-off-by: Matthew Whitehead <[email protected]> * Fix copyright for new file Co-authored-by: Sally MacFarlane <[email protected]> Signed-off-by: Matt Whitehead <[email protected]> * Address PR comments (mainly copyright & constant renaming) Signed-off-by: Matthew Whitehead <[email protected]> * move changelog entry to 23.4.4 Signed-off-by: Sally MacFarlane <[email protected]> --------- Signed-off-by: Matthew Whitehead <[email protected]> Signed-off-by: Matt Whitehead <[email protected]> Signed-off-by: Sally MacFarlane <[email protected]> Co-authored-by: Matthew Whitehead <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]>
gtebrean · Jun 20, 2023 · 5bff76f · 5bff76f
1 parent 8f9882f
commit 5bff76f
Show file tree

Hide file tree

Showing 22 changed files with 434 additions and 19 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,19 @@
 # Changelog
 
-## 23.4.2
+## 23.4.4
+
+### Breaking Changes
+
+### Additions and Improvements
+
+### Bug Fixes
+- Use the node's configuration to determine if DNS enode URLs are allowed in calls to `admin_addPeer` and `admin_removePeer` [#5584](https://github.com/hyperledger/besu/pull/5584)
+
+### Download Links
+
+---
+
+## 23.4.3
 
 ### Breaking Changes
 - Move blockchain related variables in a dedicated storage, to pave the way to future optimizations [#5471](https://github.com/hyperledger/besu/pull/5471). The migration is performed automatically at startup,
@@ -24,6 +37,10 @@ and in case a rollback is needed, before installing a previous version, the migr
 
 ### Download Links
 
+## 23.4.2
+
+- Was not released (failed burn-in test)
+
 ---
 
 ## 23.4.1

diff --git a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java
@@ -87,6 +87,7 @@
 import org.hyperledger.besu.ethereum.p2p.network.P2PNetwork;
 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.PeerPermissions;
 import org.hyperledger.besu.ethereum.p2p.permissions.PeerPermissionsDenylist;
 import org.hyperledger.besu.ethereum.p2p.rlpx.connections.netty.TLSConfiguration;
@@ -191,6 +192,7 @@ public class RunnerBuilder {
   private JsonRpcIpcConfiguration jsonRpcIpcConfiguration;
   private boolean legacyForkIdEnabled;
   private Optional<Long> rpcMaxLogsRange;
+  private Optional<EnodeDnsConfiguration> enodeDnsConfiguration;
 
   /**
    * Add Vertx.
@@ -584,6 +586,18 @@ public RunnerBuilder rpcMaxLogsRange(final Long rpcMaxLogsRange) {
     return this;
   }
 
+  /**
+   * Add enode DNS configuration
+   *
+   * @param enodeDnsConfiguration the DNS configuration for enodes
+   * @return the runner builder
+   */
+  public RunnerBuilder enodeDnsConfiguration(final EnodeDnsConfiguration enodeDnsConfiguration) {
+    this.enodeDnsConfiguration =
+        enodeDnsConfiguration != null ? Optional.of(enodeDnsConfiguration) : Optional.empty();
+    return this;
+  }
+
   /**
    * Build Runner instance.
    *
@@ -1224,7 +1238,8 @@ private Map<String, JsonRpcMethod> jsonRpcMethods(
                 dataDir,
                 besuController.getProtocolManager().ethContext().getEthPeers(),
                 consensusEngineServer,
-                rpcMaxLogsRange);
+                rpcMaxLogsRange,
+                enodeDnsConfiguration);
     methods.putAll(besuController.getAdditionalJsonRpcMethods(jsonRpcApis));
 
     final var pluginMethods =

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@@ -3103,6 +3103,7 @@ private Runner synchronize(
             .storageProvider(keyValueStorageProvider(keyValueStorageName))
             .rpcEndpointService(rpcEndpointServiceImpl)
             .rpcMaxLogsRange(rpcMaxLogsRange)
+            .enodeDnsConfiguration(getEnodeDnsConfiguration())
             .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
@@ -291,6 +291,7 @@ public void initMocks() throws Exception {
     when(mockRunnerBuilder.rpcEndpointService(any())).thenReturn(mockRunnerBuilder);
     when(mockRunnerBuilder.legacyForkId(anyBoolean())).thenReturn(mockRunnerBuilder);
     when(mockRunnerBuilder.rpcMaxLogsRange(any())).thenReturn(mockRunnerBuilder);
+    when(mockRunnerBuilder.enodeDnsConfiguration(any())).thenReturn(mockRunnerBuilder);
     when(mockRunnerBuilder.build()).thenReturn(mockRunner);
 
     final SignatureAlgorithm signatureAlgorithm = SignatureAlgorithmFactory.getInstance();

diff --git a/...ration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcTestMethodsFactory.java b/...ration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcTestMethodsFactory.java
@@ -184,6 +184,7 @@ public Map<String, JsonRpcMethod> methods() {
             dataDir,
             ethPeers,
             Vertx.vertx(new VertxOptions().setWorkerPoolSize(1)),
+            Optional.empty(),
             Optional.empty());
   }
 }
diff --git a/...rc/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/AdminAddPeer.java b/...rc/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/AdminAddPeer.java
@@ -19,19 +19,23 @@
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
 import org.hyperledger.besu.ethereum.p2p.network.P2PNetwork;
 import org.hyperledger.besu.ethereum.p2p.peers.DefaultPeer;
+import org.hyperledger.besu.ethereum.p2p.peers.EnodeDnsConfiguration;
 import org.hyperledger.besu.ethereum.p2p.peers.EnodeURLImpl;
 import org.hyperledger.besu.ethereum.p2p.peers.Peer;
 import org.hyperledger.besu.plugin.data.EnodeURL;
 
+import java.util.Optional;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 public class AdminAddPeer extends AdminModifyPeer {
 
   private static final Logger LOG = LoggerFactory.getLogger(AdminAddPeer.class);
 
-  public AdminAddPeer(final P2PNetwork peerNetwork) {
-    super(peerNetwork);
+  public AdminAddPeer(
+      final P2PNetwork peerNetwork, final Optional<EnodeDnsConfiguration> enodeDnsConfiguration) {
+    super(peerNetwork, enodeDnsConfiguration);
   }
 
   @Override
@@ -42,7 +46,10 @@ public String getName() {
   @Override
   protected JsonRpcResponse performOperation(final Object id, final String enode) {
     LOG.debug("Adding ({}) to peers", enode);
-    final EnodeURL enodeURL = EnodeURLImpl.fromString(enode);
+    final EnodeURL enodeURL =
+        this.enodeDnsConfiguration.isEmpty()
+            ? EnodeURLImpl.fromString(enode)
+            : EnodeURLImpl.fromString(enode, enodeDnsConfiguration.get());
     final Peer peer = DefaultPeer.fromEnodeURL(enodeURL);
     final boolean addedToNetwork = peerNetwork.addMaintainedConnectionPeer(peer);
     return new JsonRpcSuccessResponse(id, addedToNetwork);

diff --git a/...main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/AdminModifyPeer.java b/...main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/AdminModifyPeer.java
@@ -21,13 +21,19 @@
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse;
 import org.hyperledger.besu.ethereum.p2p.network.P2PNetwork;
 import org.hyperledger.besu.ethereum.p2p.network.exceptions.P2PDisabledException;
+import org.hyperledger.besu.ethereum.p2p.peers.EnodeDnsConfiguration;
+
+import java.util.Optional;
 
 public abstract class AdminModifyPeer implements JsonRpcMethod {
 
   protected final P2PNetwork peerNetwork;
+  protected final Optional<EnodeDnsConfiguration> enodeDnsConfiguration;
 
-  protected AdminModifyPeer(final P2PNetwork peerNetwork) {
+  protected AdminModifyPeer(
+      final P2PNetwork peerNetwork, final Optional<EnodeDnsConfiguration> enodeDnsConfiguration) {
     this.peerNetwork = peerNetwork;
+    this.enodeDnsConfiguration = enodeDnsConfiguration;
   }
 
   @Override
@@ -49,8 +55,16 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
         return new JsonRpcErrorResponse(
             requestContext.getRequest().getId(), JsonRpcError.ENODE_ID_INVALID);
       } else {
-        return new JsonRpcErrorResponse(
-            requestContext.getRequest().getId(), JsonRpcError.PARSE_ERROR);
+        if (e.getMessage().endsWith("Invalid ip address.")) {
+          return new JsonRpcErrorResponse(
+              requestContext.getRequest().getId(), JsonRpcError.DNS_NOT_ENABLED);
+        } else if (e.getMessage().endsWith("dns-update-enabled flag is false.")) {
+          return new JsonRpcErrorResponse(
+              requestContext.getRequest().getId(), JsonRpcError.CANT_RESOLVE_PEER_ENODE_DNS);
+        } else {
+          return new JsonRpcErrorResponse(
+              requestContext.getRequest().getId(), JsonRpcError.PARSE_ERROR);
+        }
       }
     } catch (final P2PDisabledException e) {
       return new JsonRpcErrorResponse(

diff --git a/...main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/AdminRemovePeer.java b/...main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/AdminRemovePeer.java
@@ -19,18 +19,22 @@
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
 import org.hyperledger.besu.ethereum.p2p.network.P2PNetwork;
 import org.hyperledger.besu.ethereum.p2p.peers.DefaultPeer;
+import org.hyperledger.besu.ethereum.p2p.peers.EnodeDnsConfiguration;
 import org.hyperledger.besu.ethereum.p2p.peers.EnodeURLImpl;
 import org.hyperledger.besu.plugin.data.EnodeURL;
 
+import java.util.Optional;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 public class AdminRemovePeer extends AdminModifyPeer {
 
   private static final Logger LOG = LoggerFactory.getLogger(AdminRemovePeer.class);
 
-  public AdminRemovePeer(final P2PNetwork peerNetwork) {
-    super(peerNetwork);
+  public AdminRemovePeer(
+      final P2PNetwork peerNetwork, final Optional<EnodeDnsConfiguration> enodeDnsConfiguration) {
+    super(peerNetwork, enodeDnsConfiguration);
   }
 
   @Override
@@ -41,7 +45,10 @@ public String getName() {
   @Override
   protected JsonRpcResponse performOperation(final Object id, final String enode) {
     LOG.debug("Remove ({}) from peer cache", enode);
-    final EnodeURL enodeURL = EnodeURLImpl.fromString(enode);
+    final EnodeURL enodeURL =
+        this.enodeDnsConfiguration.isEmpty()
+            ? EnodeURLImpl.fromString(enode)
+            : EnodeURLImpl.fromString(enode, enodeDnsConfiguration.get());
     final boolean result =
         peerNetwork.removeMaintainedConnectionPeer(DefaultPeer.fromEnodeURL(enodeURL));
     return new JsonRpcSuccessResponse(id, result);

diff --git a/...c/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/JsonRpcError.java b/...c/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/JsonRpcError.java
@@ -162,6 +162,8 @@ public enum JsonRpcError {
   VALUE_NOT_ZERO(-50100, "We cannot transfer ether in a private transaction yet."),
 
   CANT_CONNECT_TO_LOCAL_PEER(-32100, "Cannot add local node as peer."),
+  CANT_RESOLVE_PEER_ENODE_DNS(-32100, "Cannot resolve enode DNS hostname"),
+  DNS_NOT_ENABLED(-32100, "Enode DNS support is disabled"),
 
   // Invalid input errors
   ENODE_ID_INVALID(

diff --git a/.../src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/AdminJsonRpcMethods.java b/.../src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/AdminJsonRpcMethods.java
@@ -29,11 +29,13 @@
 import org.hyperledger.besu.ethereum.api.query.BlockchainQueries;
 import org.hyperledger.besu.ethereum.eth.manager.EthPeers;
 import org.hyperledger.besu.ethereum.p2p.network.P2PNetwork;
+import org.hyperledger.besu.ethereum.p2p.peers.EnodeDnsConfiguration;
 import org.hyperledger.besu.nat.NatService;
 import org.hyperledger.besu.plugin.BesuPlugin;
 
 import java.math.BigInteger;
 import java.util.Map;
+import java.util.Optional;
 
 public class AdminJsonRpcMethods extends ApiGroupJsonRpcMethods {
 
@@ -45,6 +47,7 @@ public class AdminJsonRpcMethods extends ApiGroupJsonRpcMethods {
   private final NatService natService;
   private final Map<String, BesuPlugin> namedPlugins;
   private final EthPeers ethPeers;
+  private final Optional<EnodeDnsConfiguration> enodeDnsConfiguration;
 
   public AdminJsonRpcMethods(
       final String clientVersion,
@@ -54,7 +57,8 @@ public AdminJsonRpcMethods(
       final BlockchainQueries blockchainQueries,
       final Map<String, BesuPlugin> namedPlugins,
       final NatService natService,
-      final EthPeers ethPeers) {
+      final EthPeers ethPeers,
+      final Optional<EnodeDnsConfiguration> enodeDnsConfiguration) {
     this.clientVersion = clientVersion;
     this.networkId = networkId;
     this.genesisConfigOptions = genesisConfigOptions;
@@ -63,6 +67,7 @@ public AdminJsonRpcMethods(
     this.namedPlugins = namedPlugins;
     this.natService = natService;
     this.ethPeers = ethPeers;
+    this.enodeDnsConfiguration = enodeDnsConfiguration;
   }
 
   @Override
@@ -73,8 +78,8 @@ protected String getApiGroup() {
   @Override
   protected Map<String, JsonRpcMethod> create() {
     return mapOf(
-        new AdminAddPeer(p2pNetwork),
-        new AdminRemovePeer(p2pNetwork),
+        new AdminAddPeer(p2pNetwork, enodeDnsConfiguration),
+        new AdminRemovePeer(p2pNetwork, enodeDnsConfiguration),
         new AdminNodeInfo(
             clientVersion,
             networkId,

diff --git a/...rc/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/JsonRpcMethodsFactory.java b/...rc/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/JsonRpcMethodsFactory.java
@@ -29,6 +29,7 @@
 import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.ethereum.p2p.network.P2PNetwork;
+import org.hyperledger.besu.ethereum.p2p.peers.EnodeDnsConfiguration;
 import org.hyperledger.besu.ethereum.p2p.rlpx.wire.Capability;
 import org.hyperledger.besu.ethereum.permissioning.AccountLocalConfigPermissioningController;
 import org.hyperledger.besu.ethereum.permissioning.NodeLocalConfigPermissioningController;
@@ -76,13 +77,12 @@ public Map<String, JsonRpcMethod> methods(
       final Path dataDir,
       final EthPeers ethPeers,
       final Vertx consensusEngineServer,
-      final Optional<Long> maxLogRange) {
+      final Optional<Long> maxLogRange,
+      final Optional<EnodeDnsConfiguration> enodeDnsConfiguration) {
     final Map<String, JsonRpcMethod> enabled = new HashMap<>();
-
     if (!rpcApis.isEmpty()) {
       final JsonRpcMethod modules = new RpcModules(rpcApis);
       enabled.put(modules.getName(), modules);
-
       final List<JsonRpcMethods> availableApiGroups =
           List.of(
               new AdminJsonRpcMethods(
@@ -93,7 +93,8 @@ public Map<String, JsonRpcMethod> methods(
                   blockchainQueries,
                   namedPlugins,
                   natService,
-                  ethPeers),
+                  ethPeers,
+                  enodeDnsConfiguration),
               new DebugJsonRpcMethods(
                   blockchainQueries,
                   protocolContext,

diff --git a/...c/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/AbstractJsonRpcHttpServiceTest.java b/...c/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/AbstractJsonRpcHttpServiceTest.java
@@ -191,6 +191,7 @@ protected Map<String, JsonRpcMethod> getRpcMethods(
             folder.getRoot().toPath(),
             mock(EthPeers.class),
             syncVertx,
+            Optional.empty(),
             Optional.empty());
   }
 

diff --git a/...t/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceHostAllowlistTest.java b/...t/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceHostAllowlistTest.java
@@ -124,6 +124,7 @@ public void initServerAndClient() throws Exception {
                     folder.getRoot().toPath(),
                     mock(EthPeers.class),
                     vertx,
+                    Optional.empty(),
                     Optional.empty()));
     service = createJsonRpcHttpService();
     service.start().join();

diff --git a/.../src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceLoginTest.java b/.../src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceLoginTest.java
@@ -154,6 +154,7 @@ public static void initServerAndClient() throws Exception {
                     folder.getRoot().toPath(),
                     mock(EthPeers.class),
                     vertx,
+                    Optional.empty(),
                     Optional.empty()));
     service = createJsonRpcHttpService();
     jwtAuth = service.authenticationService.get().getJwtAuthProvider();

diff --git a/...rc/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceRpcApisTest.java b/...rc/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceRpcApisTest.java
@@ -227,6 +227,7 @@ private JsonRpcHttpService createJsonRpcHttpServiceWithRpcApis(final JsonRpcConf
                     folder,
                     mock(EthPeers.class),
                     vertx,
+                    Optional.empty(),
                     Optional.empty()));
     final JsonRpcHttpService jsonRpcHttpService =
         new JsonRpcHttpService(
@@ -335,6 +336,7 @@ private JsonRpcHttpService createJsonRpcHttpService(
                     folder,
                     mock(EthPeers.class),
                     vertx,
+                    Optional.empty(),
                     Optional.empty()));
     final JsonRpcHttpService jsonRpcHttpService =
         new JsonRpcHttpService(

diff --git a/...i/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTestBase.java b/...i/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTestBase.java
@@ -133,6 +133,7 @@ public static void initServerAndClient() throws Exception {
                     folder.getRoot().toPath(),
                     ethPeersMock,
                     vertx,
+                    Optional.empty(),
                     Optional.empty()));
     service = createJsonRpcHttpService(createLimitedJsonRpcConfig());
     service.start().join();

diff --git a/...t/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTlsClientAuthTest.java b/...t/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTlsClientAuthTest.java
@@ -139,6 +139,7 @@ public void initServer() throws Exception {
                     folder.getRoot().toPath(),
                     mock(EthPeers.class),
                     vertx,
+                    Optional.empty(),
                     Optional.empty()));
 
     System.setProperty("javax.net.ssl.trustStore", CLIENT_AS_CA_CERT.getKeyStoreFile().toString());

diff --git a/.../org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTlsMisconfigurationTest.java b/.../org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTlsMisconfigurationTest.java
@@ -127,6 +127,7 @@ public void beforeEach() {
                     tempDir.getRoot(),
                     mock(EthPeers.class),
                     vertx,
+                    Optional.empty(),
                     Optional.empty()));
   }
 

diff --git a/...pi/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTlsTest.java b/...pi/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTlsTest.java
@@ -129,6 +129,7 @@ public void initServer() throws Exception {
                     folder.getRoot().toPath(),
                     mock(EthPeers.class),
                     vertx,
+                    Optional.empty(),
                     Optional.empty()));
     service = createJsonRpcHttpService(createJsonRpcConfig());
     service.start().join();