PIE-1663: Ignore discport during startup whitelist validation (#1625)

* Ignore discport during startup whitelist validation
* Fixing SyncStatusNodePermissioningProvider

Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
pull/2/head
Lucas Saldanha 5 years ago committed by GitHub
parent de36aed05e
commit 4528956d07
  1. 29
      ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/node/NodePermissioningController.java
  2. 2
      ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/node/provider/SyncStatusNodePermissioningProvider.java
  3. 22
      ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/node/provider/SyncStatusNodePermissioningProviderTest.java
  4. 36
      pantheon/src/main/java/tech/pegasys/pantheon/util/PermissioningConfigurationValidator.java
  5. 32
      pantheon/src/test/java/tech/pegasys/pantheon/util/LocalPermissioningConfigurationValidatorTest.java

@ -41,11 +41,15 @@ public class NodePermissioningController {
}
public boolean isPermitted(final EnodeURL sourceEnode, final EnodeURL destinationEnode) {
LOG.trace("Checking node permission: {} -> {}", sourceEnode, destinationEnode);
LOG.trace("Node permissioning: Checking {} -> {}", sourceEnode, destinationEnode);
if (syncStatusNodePermissioningProvider
.map(p -> !p.hasReachedSync() && p.isPermitted(sourceEnode, destinationEnode))
.orElse(false)) {
LOG.trace(
"Node permissioning - Sync Status: Permitted {} -> {}", sourceEnode, destinationEnode);
return true;
}
@ -54,19 +58,40 @@ public class NodePermissioningController {
p -> p.isPermitted(sourceEnode, destinationEnode));
if (insufficientPeerPermission.isPresent()) {
return insufficientPeerPermission.get();
final Boolean permitted = insufficientPeerPermission.get();
LOG.trace(
"Node permissioning - Insufficient Peers: {} {} -> {}",
permitted ? "Permitted" : "Rejected",
sourceEnode,
destinationEnode);
return permitted;
}
if (syncStatusNodePermissioningProvider.isPresent()
&& !syncStatusNodePermissioningProvider.get().isPermitted(sourceEnode, destinationEnode)) {
LOG.trace(
"Node permissioning - Sync Status: Rejected {} -> {}", sourceEnode, destinationEnode);
return false;
} else {
for (final NodePermissioningProvider provider : providers) {
if (!provider.isPermitted(sourceEnode, destinationEnode)) {
LOG.trace(
"Node permissioning - {}: Rejected {} -> {}",
provider.getClass().getSimpleName(),
sourceEnode,
destinationEnode);
return false;
}
}
}
LOG.trace("Node permissioning: Permitted {} -> {}", sourceEnode, destinationEnode);
return true;
}

@ -103,7 +103,7 @@ public class SyncStatusNodePermissioningProvider implements NodePermissioningPro
return true;
} else {
checkCounter.inc();
if (fixedNodes.contains(destinationEnode)) {
if (fixedNodes.stream().anyMatch(p -> EnodeURL.sameListeningEndpoint(p, destinationEnode))) {
checkCounterPermitted.inc();
return true;
} else {

@ -31,6 +31,7 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.function.IntSupplier;
import com.google.common.collect.Lists;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@ -187,4 +188,25 @@ public class SyncStatusNodePermissioningProviderTest {
verify(checkPermittedCounter, times(0)).inc();
verify(checkUnpermittedCounter, times(0)).inc();
}
@Test
public void syncStatusPermissioningCheckShouldIgnoreEnodeURLDiscoveryPort() {
syncStatusListener.onSyncStatus(new SyncStatus(0, 1, 2));
assertThat(provider.hasReachedSync()).isFalse();
final EnodeURL bootnode =
EnodeURL.fromString(
"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.3:5678");
final EnodeURL enodeWithDiscoveryPort =
EnodeURL.fromString(
"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.3:5678?discport=30303");
final SyncStatusNodePermissioningProvider provider =
new SyncStatusNodePermissioningProvider(
synchronizer, Lists.newArrayList(bootnode), metricsSystem);
boolean isPermitted = provider.isPermitted(enode1, enodeWithDiscoveryPort);
assertThat(isPermitted).isTrue();
}
}

@ -15,7 +15,7 @@ package tech.pegasys.pantheon.util;
import tech.pegasys.pantheon.ethereum.permissioning.LocalPermissioningConfiguration;
import java.net.URI;
import java.util.ArrayList;
import java.net.URISyntaxException;
import java.util.Collection;
import java.util.List;
import java.util.stream.Collectors;
@ -26,16 +26,38 @@ public class PermissioningConfigurationValidator {
final Collection<URI> nodeURIs,
final LocalPermissioningConfiguration permissioningConfiguration)
throws Exception {
List<URI> nodesNotInWhitelist = new ArrayList<>();
if (permissioningConfiguration.isNodeWhitelistEnabled() && nodeURIs != null) {
nodesNotInWhitelist =
final List<URI> whitelistNodesWithoutQueryParam =
permissioningConfiguration.getNodeWhitelist().stream()
.map(PermissioningConfigurationValidator::removeQueryFromURI)
.collect(Collectors.toList());
final List<URI> nodeURIsNotInWhitelist =
nodeURIs.stream()
.filter(enode -> !permissioningConfiguration.getNodeWhitelist().contains(enode))
.map(PermissioningConfigurationValidator::removeQueryFromURI)
.filter(uri -> !whitelistNodesWithoutQueryParam.contains(uri))
.collect(Collectors.toList());
}
if (!nodesNotInWhitelist.isEmpty()) {
if (!nodeURIsNotInWhitelist.isEmpty()) {
throw new Exception(
"Specified node(s) not in nodes-whitelist " + enodesAsStrings(nodesNotInWhitelist));
"Specified node(s) not in nodes-whitelist " + enodesAsStrings(nodeURIsNotInWhitelist));
}
}
}
private static URI removeQueryFromURI(final URI uri) {
try {
return new URI(
uri.getScheme(),
uri.getUserInfo(),
uri.getHost(),
uri.getPort(),
uri.getPath(),
null,
uri.getFragment());
} catch (URISyntaxException e) {
throw new IllegalArgumentException(e);
}
}

@ -28,6 +28,7 @@ import java.nio.file.Path;
import java.util.List;
import java.util.stream.Collectors;
import com.google.common.collect.Lists;
import com.google.common.io.Resources;
import org.junit.Test;
@ -89,4 +90,35 @@ public class LocalPermissioningConfigurationValidatorTest {
"enode://94c15d1b9e2fe7ce56e458b9a3b672ef11894ddedd0c6f247e0f1d3487f52b66208fb4aeb8179fce6e3a749ea93ed147c37976d67af557508d199d9594c35f09@192.81.208.223:30303");
}
}
@Test
public void nodeWhitelistCheckShouldIgnoreDiscoveryPortParam() throws Exception {
final URL configFile = this.getClass().getResource(PERMISSIONING_CONFIG);
final Path toml = Files.createTempFile("toml", "");
toml.toFile().deleteOnExit();
Files.write(toml, Resources.toByteArray(configFile));
final LocalPermissioningConfiguration permissioningConfiguration =
PermissioningConfigurationBuilder.permissioningConfiguration(
true, toml.toAbsolutePath().toString(), true, toml.toAbsolutePath().toString());
// This node is defined in the PERMISSIONING_CONFIG file without the discovery port
final URI enodeURL =
URI.create(
"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.9:4567?discport=30303");
// In an URI comparison the URLs should not match
boolean isInWhitelist = permissioningConfiguration.getNodeWhitelist().contains(enodeURL);
assertThat(isInWhitelist).isFalse();
// However, for the whitelist validation, we should ignore the discovery port and don't throw an
// error
try {
PermissioningConfigurationValidator.areAllNodesAreInWhitelist(
Lists.newArrayList(enodeURL), permissioningConfiguration);
} catch (Exception e) {
fail(
"Exception not expected. Validation of nodes in whitelist should ignore the optional discovery port param.");
}
}
}

Loading…
Cancel
Save