Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Commit

Permalink
Validate enodeurl syntax from command line (#403)
Browse files Browse the repository at this point in the history
* Creating EnodeURLProperty and custom converter

* Replacing String for URI when parsing EnodeURL

* Fixing acceptance test bootnode config

* Removing invalid empty bootnode property from docker script

* Validating nodeId in Enode URL

* Adding final to method param
  • Loading branch information
lucassaldanha authored Dec 12, 2018
1 parent 7120c77 commit a4c50e5
Show file tree
Hide file tree
Showing 13 changed files with 389 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -293,10 +293,11 @@ String p2pListenAddress() {
return LOCALHOST + ":" + p2pPort;
}

List<String> bootnodes() {
List<URI> bootnodes() {
return bootnodes
.stream()
.filter(node -> !node.equals(this.enodeUrl()))
.map(URI::create)
.collect(Collectors.toList());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public void startNode(final PantheonNode node) {
}

params.add("--bootnodes");
params.add(String.join(",", node.bootnodes()));
params.add(String.join(",", node.bootnodes().toString()));

if (node.jsonRpcEnabled()) {
params.add("--rpc-enabled");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer;
import tech.pegasys.pantheon.ethereum.p2p.peers.Peer;

import java.net.URI;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -136,14 +137,9 @@ public List<Peer> getBootstrapPeers() {
}

public DiscoveryConfiguration setBootstrapPeers(final Collection<?> bootstrapPeers) {
if (bootstrapPeers.stream().allMatch(String.class::isInstance)) {
if (bootstrapPeers.stream().allMatch(URI.class::isInstance)) {
this.bootstrapPeers =
bootstrapPeers
.stream()
.map(String.class::cast)
.filter(string -> !string.isEmpty())
.map(DefaultPeer::fromURI)
.collect(toList());
bootstrapPeers.stream().map(URI.class::cast).map(DefaultPeer::fromURI).collect(toList());
} else if (bootstrapPeers.stream().allMatch(Peer.class::isInstance)) {
this.bootstrapPeers = bootstrapPeers.stream().map(Peer.class::cast).collect(toList());
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,16 @@
*/
package tech.pegasys.pantheon.ethereum.p2p.config;

import java.net.URI;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;

public class PermissioningConfiguration {
private List<String> nodeWhitelist;
private List<URI> nodeWhitelist;
private boolean nodeWhitelistSet;

public List<String> getNodeWhitelist() {
public List<URI> getNodeWhitelist() {
return nodeWhitelist;
}

Expand All @@ -30,7 +31,7 @@ public static PermissioningConfiguration createDefault() {
return config;
}

public void setNodeWhitelist(final Collection<String> nodeWhitelist) {
public void setNodeWhitelist(final Collection<URI> nodeWhitelist) {
if (nodeWhitelist != null) {
this.nodeWhitelist.addAll(nodeWhitelist);
this.nodeWhitelistSet = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer;
import tech.pegasys.pantheon.ethereum.p2p.peers.Peer;

import java.net.URI;
import java.util.ArrayList;
import java.util.List;

Expand All @@ -27,8 +28,8 @@ public class NodeWhitelistController {
public NodeWhitelistController(final PermissioningConfiguration configuration) {
nodeWhitelist = new ArrayList<>();
if (configuration != null && configuration.getNodeWhitelist() != null) {
for (String urlString : configuration.getNodeWhitelist()) {
nodeWhitelist.add(DefaultPeer.fromURI(urlString));
for (URI uri : configuration.getNodeWhitelist()) {
nodeWhitelist.add(DefaultPeer.fromURI(uri));
}
if (configuration.isNodeWhitelistSet()) {
nodeWhitelistSet = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import static org.assertj.core.api.Assertions.assertThat;

import java.net.URI;
import java.util.Arrays;

import org.junit.Test;
Expand All @@ -29,7 +30,11 @@ public void defaultConfiguration() {

@Test
public void setNodeWhitelist() {
final String[] nodes = {"enode://001@123:4567", "enode://002@123:4567", "enode://003@123:4567"};
final URI[] nodes = {
URI.create("enode://001@123:4567"),
URI.create("enode://002@123:4567"),
URI.create("enode://003@123:4567")
};
final PermissioningConfiguration configuration = PermissioningConfiguration.createDefault();
configuration.setNodeWhitelist(Arrays.asList(nodes));
assertThat(configuration.getNodeWhitelist()).containsExactlyInAnyOrder(nodes);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import tech.pegasys.pantheon.Runner;
import tech.pegasys.pantheon.RunnerBuilder;
import tech.pegasys.pantheon.cli.custom.CorsAllowedOriginsProperty;
import tech.pegasys.pantheon.cli.custom.EnodeToURIPropertyConverter;
import tech.pegasys.pantheon.cli.custom.JsonRPCWhitelistHostsProperty;
import tech.pegasys.pantheon.consensus.clique.jsonrpc.CliqueRpcApis;
import tech.pegasys.pantheon.consensus.ibft.jsonrpc.IbftRpcApis;
Expand All @@ -45,6 +46,7 @@
import java.io.File;
import java.io.IOException;
import java.net.InetAddress;
import java.net.URI;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -170,9 +172,10 @@ public static class RpcApisConversionException extends Exception {
"Comma separated enode URLs for P2P discovery bootstrap. "
+ "Default is a predefined list.",
split = ",",
arity = "1..*"
arity = "1..*",
converter = EnodeToURIPropertyConverter.class
)
private final Collection<String> bootstrapNodes = null;
private final Collection<URI> bootstrapNodes = null;

@Option(
names = {"--max-peers"},
Expand Down Expand Up @@ -401,9 +404,10 @@ private void setRefreshDelay(final Long refreshDelay) {
description =
"Comma separated enode URLs for permissioned networks. You may specify an empty list.",
split = ",",
arity = "0..*"
arity = "0..*",
converter = EnodeToURIPropertyConverter.class
)
private final Collection<String> nodesWhitelist = null;
private final Collection<URI> nodesWhitelist = null;

public PantheonCommand(
final BlockImporter blockImporter,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/*
* Copyright 2018 ConsenSys AG.
*
* 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.
*/
package tech.pegasys.pantheon.cli.custom;

import static com.google.common.base.Preconditions.checkArgument;

import tech.pegasys.pantheon.util.NetworkUtility;

import java.net.URI;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import picocli.CommandLine.ITypeConverter;

public class EnodeToURIPropertyConverter implements ITypeConverter<URI> {

private static final String IP_REPLACE_MARKER = "$$IP_PATTERN$$";
private static final String IPV4_PATTERN =
"(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)(\\.(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)){3}";
private static final String IPV6_PATTERN = "(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}";
private static final String IPV6_COMPACT_PATTERN =
"((?:[0-9A-Fa-f]{1,4}(?::[0-9A-Fa-f]{1,4})*)?)::((?:[0-9A-Fa-f]{1,4}(?::[0-9A-Fa-f]{1,4})*)?)";
private static final String DISCOVERY_PORT_PATTERN = "\\?discport=(?<discovery>\\d+)";
private static final String HEX_STRING_PATTERN = "[0-9a-fA-F]+";

private static final String ENODE_URL_PATTERN =
"enode://(?<nodeId>\\w+)@(?<ip>" + IP_REPLACE_MARKER + "):(?<listening>\\d+)";

@Override
public URI convert(final String value) throws IllegalArgumentException {
checkArgument(
value != null && !value.isEmpty(), "Can't convert null/empty string to EnodeURLProperty.");

final boolean containsDiscoveryPort = value.contains("discport");
final boolean isIPV4 = Pattern.compile(".*" + IPV4_PATTERN + ".*").matcher(value).matches();
final boolean isIPV6 = Pattern.compile(".*" + IPV6_PATTERN + ".*").matcher(value).matches();
final boolean isIPV6Compact =
Pattern.compile(".*" + IPV6_COMPACT_PATTERN + ".*").matcher(value).matches();

String pattern = ENODE_URL_PATTERN;
if (isIPV4) {
pattern = pattern.replace(IP_REPLACE_MARKER, IPV4_PATTERN);
} else if (isIPV6) {
pattern = pattern.replace(IP_REPLACE_MARKER, IPV6_PATTERN);
} else if (isIPV6Compact) {
pattern = pattern.replace(IP_REPLACE_MARKER, IPV6_COMPACT_PATTERN);
} else {
throw new IllegalArgumentException("Invalid enode URL IP format.");
}

if (containsDiscoveryPort) {
pattern += DISCOVERY_PORT_PATTERN;
}
if (isIPV6) {
pattern = pattern.replace(IP_REPLACE_MARKER, IPV6_PATTERN);
} else {
pattern = pattern.replace(IP_REPLACE_MARKER, IPV4_PATTERN);
}

final Matcher matcher = Pattern.compile(pattern).matcher(value);
checkArgument(
matcher.matches(),
"Invalid enode URL syntax. Enode URL should have the following format 'enode://<node_id>@<ip>:<listening_port>[?discport=<discovery_port>]'.");

final String nodeId = getAndValidateNodeId(matcher);
final String ip = matcher.group("ip");
final Integer listeningPort = getAndValidatePort(matcher, "listening");

if (containsDiscoveryPort(value)) {
final Integer discoveryPort = getAndValidatePort(matcher, "discovery");
return URI.create(
String.format("enode://%s@%s:%d?discport=%d", nodeId, ip, listeningPort, discoveryPort));
} else {
return URI.create(String.format("enode://%s@%s:%d", nodeId, ip, listeningPort));
}
}

private String getAndValidateNodeId(final Matcher matcher) {
final String invalidNodeIdErrorMsg =
"Enode URL contains an invalid node ID. Node ID must have 128 characters and shouldn't include the '0x' hex prefix.";
final String nodeId = matcher.group("nodeId");

checkArgument(nodeId.matches(HEX_STRING_PATTERN), invalidNodeIdErrorMsg);
checkArgument(nodeId.length() == 128, invalidNodeIdErrorMsg);

return nodeId;
}

private Integer getAndValidatePort(final Matcher matcher, final String portName) {
int port = Integer.valueOf(matcher.group(portName));
checkArgument(
NetworkUtility.isValidPort(port),
"Invalid " + portName + " port range. Port should be between 0 - 65535");
return port;
}

private boolean containsDiscoveryPort(final String value) {
return value.contains("discport");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.PrintStream;
import java.net.URI;
import java.nio.file.Path;
import java.util.Collection;
import java.util.List;
Expand Down Expand Up @@ -73,6 +74,7 @@ public abstract class CommandTestAbstract {
@Captor ArgumentCaptor<JsonRpcConfiguration> jsonRpcConfigArgumentCaptor;
@Captor ArgumentCaptor<WebSocketConfiguration> wsRpcConfigArgumentCaptor;
@Captor ArgumentCaptor<PermissioningConfiguration> permissioningConfigurationArgumentCaptor;
@Captor ArgumentCaptor<Collection<URI>> uriListArgumentCaptor;

@Before
public void initMocks() throws Exception {
Expand Down
Loading

0 comments on commit a4c50e5

Please sign in to comment.