Skip to content

Commit

Permalink
Move p2p host validation to CLI validation (hyperledger#8158)
Browse files Browse the repository at this point in the history
Signed-off-by: Gabriel-Trintinalia <[email protected]>
  • Loading branch information
Gabriel-Trintinalia authored Jan 23, 2025
1 parent 113cf8b commit a1e087d
Show file tree
Hide file tree
Showing 5 changed files with 196 additions and 122 deletions.
24 changes: 10 additions & 14 deletions besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -212,10 +212,8 @@
import java.io.InputStream;
import java.io.InputStreamReader;
import java.math.BigInteger;
import java.net.SocketException;
import java.net.URI;
import java.net.URL;
import java.net.UnknownHostException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.GroupPrincipal;
Expand Down Expand Up @@ -1433,7 +1431,7 @@ private void configureNativeLibs() {

private void validateOptions() {
issueOptionWarnings();
validateP2PInterface(p2PDiscoveryOptions.p2pInterface);
validateP2POptions();
validateMiningParams();
validateNatParams();
validateNetStatsParams();
Expand Down Expand Up @@ -1488,20 +1486,18 @@ private void validateMiningParams() {
commandLine, genesisConfigOptionsSupplier.get(), isMergeEnabled(), logger);
}

private void validateP2POptions() {
p2PDiscoveryOptions.validate(commandLine, getNetworkInterfaceChecker());
}

/**
* Validates P2P interface IP address/host name. Visible for testing.
* Returns a network interface checker that can be used to validate P2P options.
*
* @param p2pInterface IP Address/host name
* @return A {@link P2PDiscoveryOptions.NetworkInterfaceChecker} that checks if a network
* interface is available.
*/
protected void validateP2PInterface(final String p2pInterface) {
final String failMessage = "The provided --p2p-interface is not available: " + p2pInterface;
try {
if (!NetworkUtility.isNetworkInterfaceAvailable(p2pInterface)) {
throw new ParameterException(commandLine, failMessage);
}
} catch (final UnknownHostException | SocketException e) {
throw new ParameterException(commandLine, failMessage, e);
}
protected P2PDiscoveryOptions.NetworkInterfaceChecker getNetworkInterfaceChecker() {
return NetworkUtility::isNetworkInterfaceAvailable;
}

private void validateGraphQlOptions() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,38 @@
import org.hyperledger.besu.util.number.Percentage;

import java.net.InetAddress;
import java.net.SocketException;
import java.net.UnknownHostException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

import com.google.common.net.InetAddresses;
import org.apache.commons.net.util.SubnetUtils;
import org.apache.tuweni.bytes.Bytes;
import picocli.CommandLine;

/** Command line options for configuring P2P discovery on the node. */
public class P2PDiscoveryOptions implements CLIOptions<P2PDiscoveryConfiguration> {

/** Functional interface for checking if a network interface is available. */
@FunctionalInterface
public interface NetworkInterfaceChecker {

/**
* Checks if the provided network interface is available.
*
* @param p2pInterface The network interface to check.
* @return True if the network interface is available, false otherwise.
* @throws UnknownHostException If the host is unknown.
* @throws SocketException If there is an error with the socket.
*/
boolean isNetworkInterfaceAvailable(final String p2pInterface)
throws UnknownHostException, SocketException;
}

/** Default constructor */
public P2PDiscoveryOptions() {}

Expand Down Expand Up @@ -217,6 +236,37 @@ public P2PDiscoveryConfiguration toDomainObject() {
discoveryDnsUrl);
}

/**
* Validates the provided P2P discovery options.
*
* @param commandLine The command line object used for parsing and error reporting.
* @param networkInterfaceChecker The checker used to validate the network interface.
*/
public void validate(
final CommandLine commandLine, final NetworkInterfaceChecker networkInterfaceChecker) {
validateP2PHost(commandLine);
validateP2PInterface(commandLine, networkInterfaceChecker);
}

private void validateP2PInterface(
final CommandLine commandLine, final NetworkInterfaceChecker networkInterfaceChecker) {
final String failMessage = "The provided --p2p-interface is not available: " + p2pInterface;
try {
if (!networkInterfaceChecker.isNetworkInterfaceAvailable(p2pInterface)) {
throw new CommandLine.ParameterException(commandLine, failMessage);
}
} catch (final UnknownHostException | SocketException e) {
throw new CommandLine.ParameterException(commandLine, failMessage, e);
}
}

private void validateP2PHost(final CommandLine commandLine) {
final String failMessage = "The provided --p2p-host is invalid: " + p2pHost;
if (!InetAddresses.isInetAddress(p2pHost)) {
throw new CommandLine.ParameterException(commandLine, failMessage);
}
}

@Override
public List<String> getCLIOptions() {
return CommandLineUtils.getCLIOptions(this, new P2PDiscoveryOptions());
Expand Down
107 changes: 0 additions & 107 deletions besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1011,113 +1011,6 @@ public void callingWithBannedNodeidsOptionWithInvalidValuesMustDisplayError() {
assertThat(commandErrorOutput.toString(UTF_8)).startsWith(expectedErrorOutputStart);
}

@Test
public void p2pHostAndPortOptionsAreRespectedAndNotLeaked() {

final String host = "1.2.3.4";
final int port = 1234;
parseCommand("--p2p-host", host, "--p2p-port", String.valueOf(port));

verify(mockRunnerBuilder).p2pAdvertisedHost(stringArgumentCaptor.capture());
verify(mockRunnerBuilder).p2pListenPort(intArgumentCaptor.capture());
verify(mockRunnerBuilder).metricsConfiguration(metricsConfigArgumentCaptor.capture());
verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture());
verify(mockRunnerBuilder).build();

assertThat(stringArgumentCaptor.getValue()).isEqualTo(host);
assertThat(intArgumentCaptor.getValue()).isEqualTo(port);

assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();

// all other port values remain default
assertThat(metricsConfigArgumentCaptor.getValue().getPort()).isEqualTo(9545);
assertThat(metricsConfigArgumentCaptor.getValue().getPushPort()).isEqualTo(9001);
assertThat(jsonRpcConfigArgumentCaptor.getValue().getPort()).isEqualTo(8545);

// all other host values remain default
final String defaultHost = "127.0.0.1";
assertThat(metricsConfigArgumentCaptor.getValue().getHost()).isEqualTo(defaultHost);
assertThat(metricsConfigArgumentCaptor.getValue().getPushHost()).isEqualTo(defaultHost);
assertThat(jsonRpcConfigArgumentCaptor.getValue().getHost()).isEqualTo(defaultHost);
}

@Test
public void p2pHostAndPortOptionsAreRespectedAndNotLeakedWithMetricsEnabled() {

final String host = "1.2.3.4";
final int port = 1234;
parseCommand("--p2p-host", host, "--p2p-port", String.valueOf(port), "--metrics-enabled");

verify(mockRunnerBuilder).p2pAdvertisedHost(stringArgumentCaptor.capture());
verify(mockRunnerBuilder).p2pListenPort(intArgumentCaptor.capture());
verify(mockRunnerBuilder).metricsConfiguration(metricsConfigArgumentCaptor.capture());
verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture());
verify(mockRunnerBuilder).build();

assertThat(stringArgumentCaptor.getValue()).isEqualTo(host);
assertThat(intArgumentCaptor.getValue()).isEqualTo(port);

assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();

// all other port values remain default
assertThat(metricsConfigArgumentCaptor.getValue().getPort()).isEqualTo(9545);
assertThat(metricsConfigArgumentCaptor.getValue().getPushPort()).isEqualTo(9001);
assertThat(jsonRpcConfigArgumentCaptor.getValue().getPort()).isEqualTo(8545);

// all other host values remain default
final String defaultHost = "127.0.0.1";
assertThat(metricsConfigArgumentCaptor.getValue().getHost()).isEqualTo(defaultHost);
assertThat(metricsConfigArgumentCaptor.getValue().getPushHost()).isEqualTo(defaultHost);
assertThat(jsonRpcConfigArgumentCaptor.getValue().getHost()).isEqualTo(defaultHost);
}

@Test
public void p2pInterfaceOptionIsRespected() {

final String ip = "1.2.3.4";
parseCommand("--p2p-interface", ip);

assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();

verify(mockRunnerBuilder).p2pListenInterface(stringArgumentCaptor.capture());
verify(mockRunnerBuilder).build();

assertThat(stringArgumentCaptor.getValue()).isEqualTo(ip);
}

@Test
public void p2pHostMayBeLocalhost() {

final String host = "localhost";
parseCommand("--p2p-host", host);

verify(mockRunnerBuilder).p2pAdvertisedHost(stringArgumentCaptor.capture());
verify(mockRunnerBuilder).build();

assertThat(stringArgumentCaptor.getValue()).isEqualTo(host);

assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
}

@Test
public void p2pHostMayBeIPv6() {

final String host = "2600:DB8::8545";
parseCommand("--p2p-host", host);

verify(mockRunnerBuilder).p2pAdvertisedHost(stringArgumentCaptor.capture());
verify(mockRunnerBuilder).build();

assertThat(stringArgumentCaptor.getValue()).isEqualTo(host);

assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
}

@Test
public void maxpeersOptionMustBeUsed() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.hyperledger.besu.cli.options.EthstatsOptions;
import org.hyperledger.besu.cli.options.MiningOptions;
import org.hyperledger.besu.cli.options.NetworkingOptions;
import org.hyperledger.besu.cli.options.P2PDiscoveryOptions;
import org.hyperledger.besu.cli.options.SynchronizerOptions;
import org.hyperledger.besu.cli.options.TransactionPoolOptions;
import org.hyperledger.besu.cli.options.storage.DataStorageOptions;
Expand Down Expand Up @@ -568,8 +569,9 @@ public static class TestBesuCommand extends BesuCommand {
}

@Override
protected void validateP2PInterface(final String p2pInterface) {
protected P2PDiscoveryOptions.NetworkInterfaceChecker getNetworkInterfaceChecker() {
// For testing, don't actually query for networking interfaces to validate this option
return (networkInterface) -> true;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
/*
* Copyright contributors to 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.options;

import static java.nio.charset.StandardCharsets.UTF_8;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.verify;

import org.hyperledger.besu.cli.CommandTestAbstract;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.junit.jupiter.MockitoExtension;

@ExtendWith(MockitoExtension.class)
public class P2PDiscoveryOptionsTest extends CommandTestAbstract {

@Test
public void p2pHostAndPortOptionsAreRespectedAndNotLeakedWithMetricsEnabled() {

final String host = "1.2.3.4";
final int port = 1234;
parseCommand("--p2p-host", host, "--p2p-port", String.valueOf(port), "--metrics-enabled");

verify(mockRunnerBuilder).p2pAdvertisedHost(stringArgumentCaptor.capture());
verify(mockRunnerBuilder).p2pListenPort(intArgumentCaptor.capture());
verify(mockRunnerBuilder).metricsConfiguration(metricsConfigArgumentCaptor.capture());
verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture());
verify(mockRunnerBuilder).build();

assertThat(stringArgumentCaptor.getValue()).isEqualTo(host);
assertThat(intArgumentCaptor.getValue()).isEqualTo(port);

assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();

// all other port values remain default
assertThat(metricsConfigArgumentCaptor.getValue().getPort()).isEqualTo(9545);
assertThat(metricsConfigArgumentCaptor.getValue().getPushPort()).isEqualTo(9001);
assertThat(jsonRpcConfigArgumentCaptor.getValue().getPort()).isEqualTo(8545);

// all other host values remain default
final String defaultHost = "127.0.0.1";
assertThat(metricsConfigArgumentCaptor.getValue().getHost()).isEqualTo(defaultHost);
assertThat(metricsConfigArgumentCaptor.getValue().getPushHost()).isEqualTo(defaultHost);
assertThat(jsonRpcConfigArgumentCaptor.getValue().getHost()).isEqualTo(defaultHost);
}

@Test
public void p2pInterfaceOptionIsRespected() {

final String ip = "1.2.3.4";
parseCommand("--p2p-interface", ip);

assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();

verify(mockRunnerBuilder).p2pListenInterface(stringArgumentCaptor.capture());
verify(mockRunnerBuilder).build();

assertThat(stringArgumentCaptor.getValue()).isEqualTo(ip);
}

@ParameterizedTest
@ValueSource(strings = {"localhost", " localhost.localdomain", "invalid-host"})
public void p2pHostMustBeAnIPAddress(final String host) {
parseCommand("--p2p-host", host);

assertThat(commandOutput.toString(UTF_8)).isEmpty();
String errorMessage = "The provided --p2p-host is invalid: " + host;
assertThat(commandErrorOutput.toString(UTF_8)).contains(errorMessage);
}

@Test
public void p2pHostMayBeIPv6() {

final String host = "2600:DB8::8545";
parseCommand("--p2p-host", host);

verify(mockRunnerBuilder).p2pAdvertisedHost(stringArgumentCaptor.capture());
verify(mockRunnerBuilder).build();

assertThat(stringArgumentCaptor.getValue()).isEqualTo(host);

assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
}

@Test
public void p2pHostAndPortOptionsAreRespectedAndNotLeaked() {

final String host = "1.2.3.4";
final int port = 1234;
parseCommand("--p2p-host", host, "--p2p-port", String.valueOf(port));

verify(mockRunnerBuilder).p2pAdvertisedHost(stringArgumentCaptor.capture());
verify(mockRunnerBuilder).p2pListenPort(intArgumentCaptor.capture());
verify(mockRunnerBuilder).metricsConfiguration(metricsConfigArgumentCaptor.capture());
verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture());
verify(mockRunnerBuilder).build();

assertThat(stringArgumentCaptor.getValue()).isEqualTo(host);
assertThat(intArgumentCaptor.getValue()).isEqualTo(port);

assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();

// all other port values remain default
assertThat(metricsConfigArgumentCaptor.getValue().getPort()).isEqualTo(9545);
assertThat(metricsConfigArgumentCaptor.getValue().getPushPort()).isEqualTo(9001);
assertThat(jsonRpcConfigArgumentCaptor.getValue().getPort()).isEqualTo(8545);

// all other host values remain default
final String defaultHost = "127.0.0.1";
assertThat(metricsConfigArgumentCaptor.getValue().getHost()).isEqualTo(defaultHost);
assertThat(metricsConfigArgumentCaptor.getValue().getPushHost()).isEqualTo(defaultHost);
assertThat(jsonRpcConfigArgumentCaptor.getValue().getHost()).isEqualTo(defaultHost);
}
}

0 comments on commit a1e087d

Please sign in to comment.