Skip to content

Commit

Permalink
Use the node's configuration to decide if adding a peer with DNS in t…
Browse files Browse the repository at this point in the history
…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 <matthew1001@gmail.com>

* Update command test mocks

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Update besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java

Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Matt Whitehead <matthew1001@hotmail.com>

* Address PR comments. Add a reference in the change log. Fix failing integration test.

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Add the same DNS-config checking logic to admin_removePeer, along with a unit test file for it

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Tweak the change log

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* 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 <matthew1001@gmail.com>

* Spotless Java fixes

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Fix copyright for new file

Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Matt Whitehead <matthew1001@hotmail.com>

* Address PR comments (mainly copyright & constant renaming)

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* move changelog entry to 23.4.4

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

---------

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Matt Whitehead <matthew1001@hotmail.com>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Co-authored-by: Matthew Whitehead <matthew1001@gmail.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
  • Loading branch information
3 people authored and Chengxuan committed Dec 13, 2023
1 parent 479530d commit 30655e7
Show file tree
Hide file tree
Showing 21 changed files with 416 additions and 18 deletions.
17 changes: 16 additions & 1 deletion besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -191,6 +192,7 @@ public class RunnerBuilder {
private JsonRpcIpcConfiguration jsonRpcIpcConfiguration;
private boolean legacyForkIdEnabled;
private Optional<Long> rpcMaxLogsRange;
private Optional<EnodeDnsConfiguration> enodeDnsConfiguration;

/**
* Add Vertx.
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3095,6 +3095,7 @@ private Runner synchronize(
.storageProvider(keyValueStorageProvider(keyValueStorageName))
.rpcEndpointService(rpcEndpointServiceImpl)
.rpcMaxLogsRange(rpcMaxLogsRange)
.enodeDnsConfiguration(getEnodeDnsConfiguration())
.build();

addShutdownHook(runner);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ public Map<String, JsonRpcMethod> methods() {
dataDir,
ethPeers,
Vertx.vertx(new VertxOptions().setWorkerPoolSize(1)),
Optional.empty(),
Optional.empty());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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,
Expand All @@ -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;
Expand All @@ -63,6 +67,7 @@ public AdminJsonRpcMethods(
this.namedPlugins = namedPlugins;
this.natService = natService;
this.ethPeers = ethPeers;
this.enodeDnsConfiguration = enodeDnsConfiguration;
}

@Override
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand All @@ -93,7 +93,8 @@ public Map<String, JsonRpcMethod> methods(
blockchainQueries,
namedPlugins,
natService,
ethPeers),
ethPeers,
enodeDnsConfiguration),
new DebugJsonRpcMethods(
blockchainQueries,
protocolContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ protected Map<String, JsonRpcMethod> getRpcMethods(
folder.getRoot().toPath(),
mock(EthPeers.class),
syncVertx,
Optional.empty(),
Optional.empty());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ private JsonRpcHttpService createJsonRpcHttpServiceWithRpcApis(final JsonRpcConf
folder,
mock(EthPeers.class),
vertx,
Optional.empty(),
Optional.empty()));
final JsonRpcHttpService jsonRpcHttpService =
new JsonRpcHttpService(
Expand Down Expand Up @@ -335,6 +336,7 @@ private JsonRpcHttpService createJsonRpcHttpService(
folder,
mock(EthPeers.class),
vertx,
Optional.empty(),
Optional.empty()));
final JsonRpcHttpService jsonRpcHttpService =
new JsonRpcHttpService(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ public void beforeEach() {
tempDir.getRoot(),
mock(EthPeers.class),
vertx,
Optional.empty(),
Optional.empty()));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 30655e7

Please sign in to comment.