From 043139dff8b24b95f55d671df7c3cc48f1b63541 Mon Sep 17 00:00:00 2001 From: Daniel Lehrner Date: Thu, 17 Jun 2021 00:11:03 +0200 Subject: [PATCH] Fix SECP256R1AcceptanceTest (#2321) * don't set SECP256R1 as signature algorithm instance in the tests, only use it when the nodes are started as their own processes Signed-off-by: Daniel Lehrner * renamed some variables for better readability Signed-off-by: Daniel Lehrner --- .../tests/acceptance/dsl/node/BesuNode.java | 12 ++- .../acceptance/dsl/node/BesuNodeRunner.java | 6 +- .../configuration/BesuNodeConfiguration.java | 10 +- .../BesuNodeConfigurationBuilder.java | 10 +- .../node/configuration/BesuNodeFactory.java | 91 +++++++++++-------- .../acceptance/dsl/privacy/PrivacyNode.java | 3 +- .../crypto/SECP256R1AcceptanceTest.java | 68 +++++++++++--- .../org/hyperledger/besu/crypto/KeyPair.java | 9 ++ .../hyperledger/besu/crypto/KeyPairUtil.java | 27 +++--- 9 files changed, 166 insertions(+), 70 deletions(-) diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNode.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNode.java index b14571f0af4..4c494fd1054 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNode.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNode.java @@ -84,7 +84,7 @@ public class BesuNode implements NodeConfiguration, RunnableNode, AutoCloseable private static final Logger LOG = getLogger(); private final Path homeDirectory; - private final KeyPair keyPair; + private KeyPair keyPair; private final Properties portsProperties = new Properties(); private final Boolean p2pEnabled; private final NetworkingConfiguration networkingConfiguration; @@ -141,7 +141,8 @@ public BesuNode( final List staticNodes, final boolean isDnsEnabled, final Optional privacyParameters, - final List runCommand) + final List runCommand, + final Optional keyPair) throws IOException { this.homeDirectory = dataPath.orElseGet(BesuNode::createTmpDataDirectory); keyfilePath.ifPresent( @@ -152,7 +153,12 @@ public BesuNode( LOG.error("Could not find key file \"{}\" in resources", path); } }); - this.keyPair = KeyPairUtil.loadKeyPair(homeDirectory); + keyPair.ifPresentOrElse( + (existingKeyPair) -> { + this.keyPair = existingKeyPair; + KeyPairUtil.storeKeyFile(existingKeyPair, homeDirectory); + }, + () -> this.keyPair = KeyPairUtil.loadKeyPair(homeDirectory)); this.name = name; this.miningParameters = miningParameters; this.jsonRpcConfiguration = jsonRpcConfiguration; diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNodeRunner.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNodeRunner.java index 89512ddbdb9..fa74644ccbc 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNodeRunner.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNodeRunner.java @@ -26,13 +26,17 @@ public interface BesuNodeRunner { static BesuNodeRunner instance() { - if (Boolean.getBoolean("acctests.runBesuAsProcess")) { + if (isProcessBesuNodeRunner()) { return new ProcessBesuNodeRunner(); } else { return new ThreadBesuNodeRunner(); } } + static boolean isProcessBesuNodeRunner() { + return Boolean.getBoolean("acctests.runBesuAsProcess"); + } + void startNode(BesuNode node); void stopNode(BesuNode node); diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfiguration.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfiguration.java index cea80f15ae6..684d4e2cd48 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfiguration.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfiguration.java @@ -15,6 +15,7 @@ package org.hyperledger.besu.tests.acceptance.dsl.node.configuration; import org.hyperledger.besu.cli.config.NetworkName; +import org.hyperledger.besu.crypto.KeyPair; import org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcConfiguration; import org.hyperledger.besu.ethereum.api.jsonrpc.websocket.WebSocketConfiguration; import org.hyperledger.besu.ethereum.core.MiningParameters; @@ -54,6 +55,7 @@ public class BesuNodeConfiguration { private final Optional privacyParameters; private final List runCommand; private final NetworkName network; + private final Optional keyPair; BesuNodeConfiguration( final String name, @@ -79,7 +81,8 @@ public class BesuNodeConfiguration { final List staticNodes, final boolean isDnsEnabled, final Optional privacyParameters, - final List runCommand) { + final List runCommand, + final Optional keyPair) { this.name = name; this.miningParameters = miningParameters; this.jsonRpcConfiguration = jsonRpcConfiguration; @@ -104,6 +107,7 @@ public class BesuNodeConfiguration { this.isDnsEnabled = isDnsEnabled; this.privacyParameters = privacyParameters; this.runCommand = runCommand; + this.keyPair = keyPair; } public String getName() { @@ -201,4 +205,8 @@ public List getRunCommand() { public NetworkName getNetwork() { return network; } + + public Optional getKeyPair() { + return keyPair; + } } diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfigurationBuilder.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfigurationBuilder.java index 6066a87e0ad..f6840c518f4 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfigurationBuilder.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfigurationBuilder.java @@ -18,6 +18,7 @@ import static java.util.Collections.singletonList; import org.hyperledger.besu.cli.config.NetworkName; +import org.hyperledger.besu.crypto.KeyPair; import org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcConfiguration; import org.hyperledger.besu.ethereum.api.jsonrpc.RpcApis; import org.hyperledger.besu.ethereum.api.jsonrpc.websocket.WebSocketConfiguration; @@ -65,6 +66,7 @@ public class BesuNodeConfigurationBuilder { private boolean isDnsEnabled = false; private Optional privacyParameters = Optional.empty(); private List runCommand = new ArrayList<>(); + private Optional keyPair = Optional.empty(); public BesuNodeConfigurationBuilder() { // Check connections more frequently during acceptance tests to cut down on @@ -295,6 +297,11 @@ public BesuNodeConfigurationBuilder privacyParameters(final PrivacyParameters pr return this; } + public BesuNodeConfigurationBuilder keyPair(final KeyPair keyPair) { + this.keyPair = Optional.of(keyPair); + return this; + } + public BesuNodeConfigurationBuilder run(final String... commands) { this.runCommand = List.of(commands); return this; @@ -325,6 +332,7 @@ public BesuNodeConfiguration build() { staticNodes, isDnsEnabled, privacyParameters, - runCommand); + runCommand, + keyPair); } } diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeFactory.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeFactory.java index e887df10204..6a9810c0e30 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeFactory.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeFactory.java @@ -17,9 +17,7 @@ import static java.util.Arrays.asList; import static java.util.stream.Collectors.toList; -import org.hyperledger.besu.config.GenesisConfigFile; -import org.hyperledger.besu.crypto.SignatureAlgorithmFactory; -import org.hyperledger.besu.crypto.SignatureAlgorithmType; +import org.hyperledger.besu.crypto.KeyPair; import org.hyperledger.besu.enclave.EnclaveFactory; import org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcConfiguration; import org.hyperledger.besu.ethereum.api.jsonrpc.RpcApi; @@ -41,7 +39,7 @@ import java.net.URI; import java.net.URISyntaxException; import java.nio.file.Paths; -import java.util.Collections; +import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.function.Function; @@ -54,8 +52,6 @@ public class BesuNodeFactory { private final NodeConfigurationFactory node = new NodeConfigurationFactory(); public BesuNode create(final BesuNodeConfiguration config) throws IOException { - instantiateSignatureAlgorithmFactory(config); - return new BesuNode( config.getName(), config.getDataPath(), @@ -80,7 +76,8 @@ public BesuNode create(final BesuNodeConfiguration config) throws IOException { config.getStaticNodes(), config.isDnsEnabled(), config.getPrivacyParameters(), - config.getRunCommand()); + config.getRunCommand(), + config.getKeyPair()); } public BesuNode createMinerNode(final String name) throws IOException { @@ -443,6 +440,13 @@ public BesuNode createQbftNodeWithValidators(final String name, final String... public BesuNode createNodeWithStaticNodes(final String name, final List staticNodes) throws IOException { + BesuNodeConfigurationBuilder builder = + createConfigurationBuilderWithStaticNodes(name, staticNodes); + return create(builder.build()); + } + + private BesuNodeConfigurationBuilder createConfigurationBuilderWithStaticNodes( + final String name, final List staticNodes) { final List staticNodesUrls = staticNodes.stream() .map(node -> (RunnableNode) node) @@ -450,46 +454,55 @@ public BesuNode createNodeWithStaticNodes(final String name, final List st .map(URI::toASCIIString) .collect(toList()); - return create( - new BesuNodeConfigurationBuilder() - .name(name) - .jsonRpcEnabled() - .webSocketEnabled() - .discoveryEnabled(false) - .staticNodes(staticNodesUrls) - .bootnodeEligible(false) - .build()); + return new BesuNodeConfigurationBuilder() + .name(name) + .jsonRpcEnabled() + .webSocketEnabled() + .discoveryEnabled(false) + .staticNodes(staticNodesUrls) + .bootnodeEligible(false); } - public BesuNode runCommand(final String command) throws IOException { - return create(new BesuNodeConfigurationBuilder().name("run " + command).run(command).build()); - } + public BesuNode createNodeWithNonDefaultSignatureAlgorithm( + final String name, final String genesisPath, final KeyPair keyPair) throws IOException { + BesuNodeConfigurationBuilder builder = + createNodeConfigurationWithNonDefaultSignatureAlgorithm( + name, genesisPath, keyPair, new ArrayList<>()); + builder.miningEnabled(); - private void instantiateSignatureAlgorithmFactory(final BesuNodeConfiguration config) { - if (SignatureAlgorithmFactory.isInstanceSet()) { - return; - } - - Optional ecCurve = getEcCurveFromGenesisFile(config); - - if (ecCurve.isEmpty()) { - SignatureAlgorithmFactory.setDefaultInstance(); - return; - } + return create(builder.build()); + } - SignatureAlgorithmFactory.setInstance(SignatureAlgorithmType.create(ecCurve.get())); + public BesuNode createNodeWithNonDefaultSignatureAlgorithm( + final String name, + final String genesisPath, + final KeyPair keyPair, + final List staticNodes) + throws IOException { + BesuNodeConfigurationBuilder builder = + createNodeConfigurationWithNonDefaultSignatureAlgorithm( + name, genesisPath, keyPair, staticNodes); + return create(builder.build()); } - private Optional getEcCurveFromGenesisFile(final BesuNodeConfiguration config) { - Optional genesisConfig = - config.getGenesisConfigProvider().create(Collections.emptyList()); + public BesuNodeConfigurationBuilder createNodeConfigurationWithNonDefaultSignatureAlgorithm( + final String name, + final String genesisPath, + final KeyPair keyPair, + final List staticNodes) { + BesuNodeConfigurationBuilder builder = + createConfigurationBuilderWithStaticNodes(name, staticNodes); - if (genesisConfig.isEmpty()) { - return Optional.empty(); - } + final GenesisConfigurationFactory genesis = new GenesisConfigurationFactory(); + final String genesisData = genesis.readGenesisFile(genesisPath); - GenesisConfigFile genesisConfigFile = GenesisConfigFile.fromConfig(genesisConfig.get()); + return builder + .devMode(false) + .genesisConfigProvider((nodes) -> Optional.of(genesisData)) + .keyPair(keyPair); + } - return genesisConfigFile.getConfigOptions().getEcCurve(); + public BesuNode runCommand(final String command) throws IOException { + return create(new BesuNodeConfigurationBuilder().name("run " + command).run(command).build()); } } diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/privacy/PrivacyNode.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/privacy/PrivacyNode.java index d94bf4ed3d9..525fcf67378 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/privacy/PrivacyNode.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/privacy/PrivacyNode.java @@ -118,7 +118,8 @@ public PrivacyNode( Collections.emptyList(), besuConfig.isDnsEnabled(), besuConfig.getPrivacyParameters(), - List.of()); + List.of(), + Optional.empty()); } public void testEnclaveConnection(final List otherNodes) { diff --git a/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/crypto/SECP256R1AcceptanceTest.java b/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/crypto/SECP256R1AcceptanceTest.java index d8cb833db51..353e25ce1a0 100644 --- a/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/crypto/SECP256R1AcceptanceTest.java +++ b/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/crypto/SECP256R1AcceptanceTest.java @@ -16,40 +16,70 @@ package org.hyperledger.besu.tests.acceptance.crypto; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assumptions.assumeThat; +import org.hyperledger.besu.crypto.KeyPair; import org.hyperledger.besu.crypto.SECP256R1; import org.hyperledger.besu.ethereum.core.Hash; import org.hyperledger.besu.tests.acceptance.dsl.AcceptanceTestBase; import org.hyperledger.besu.tests.acceptance.dsl.account.Account; +import org.hyperledger.besu.tests.acceptance.dsl.node.BesuNodeRunner; import org.hyperledger.besu.tests.acceptance.dsl.node.Node; +import org.hyperledger.besu.tests.acceptance.dsl.node.cluster.Cluster; +import org.hyperledger.besu.tests.acceptance.dsl.node.cluster.ClusterConfiguration; +import org.hyperledger.besu.tests.acceptance.dsl.node.cluster.ClusterConfigurationBuilder; +import java.util.List; + +import org.apache.tuweni.bytes.Bytes32; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; public class SECP256R1AcceptanceTest extends AcceptanceTestBase { private Node minerNode; - private Node fullNode; + private Node otherNode; + private Cluster noDiscoveryCluster; + + private static final String GENESIS_FILE = "/crypto/secp256r1.json"; - protected static final String GENESIS_FILE = "/crypto/secp256r1.json"; + private static final String MINER_NODE_PRIVATE_KEY = + "8f2a55949038a9610f50fb23b5883af3b4ecb3c3bb792cbcefbd1542c692be63"; + private static final String OTHER_NODE_PRIVATE_KEY = + "c87509a1c067bbde78beb793e6fa76530b6382a4c0241e5e4a9ec0a0f44dc0d3"; + + private static final SECP256R1 SECP256R1_SIGNATURE_ALGORITHM = new SECP256R1(); @Before public void setUp() throws Exception { - minerNode = besu.createCustomGenesisNode("node1", GENESIS_FILE, true, true); - fullNode = besu.createCustomGenesisNode("node2", GENESIS_FILE, false); - cluster.start(minerNode, fullNode); - } + KeyPair minerNodeKeyPair = createKeyPair(MINER_NODE_PRIVATE_KEY); + KeyPair otherNodeKeyPair = createKeyPair(OTHER_NODE_PRIVATE_KEY); - @Test - @Ignore - public void shouldConnectToOtherPeer() { - minerNode.verify(net.awaitPeerCount(1)); - fullNode.verify(net.awaitPeerCount(1)); + final ClusterConfiguration clusterConfiguration = + new ClusterConfigurationBuilder().awaitPeerDiscovery(false).build(); + noDiscoveryCluster = new Cluster(clusterConfiguration, net); + + minerNode = + besu.createNodeWithNonDefaultSignatureAlgorithm( + "minerNode", GENESIS_FILE, minerNodeKeyPair); + noDiscoveryCluster.start(minerNode); + + otherNode = + besu.createNodeWithNonDefaultSignatureAlgorithm( + "otherNode", GENESIS_FILE, otherNodeKeyPair, List.of(minerNode)); + cluster.addNode(otherNode); } @Test - @Ignore public void transactionShouldBeSuccessful() { + // SignatureAlgorithmFactory.instance is static. When ThreadBesuRunner is used, we cannot change + // the signature algorithm instance to SECP256R1 as it could influence other tests running at + // the same time. So we only execute the test when ProcessBesuNodeRunner is used, as there is + // not conflict because we use separate processes. + assumeThat(BesuNodeRunner.isProcessBesuNodeRunner()).isTrue(); + + minerNode.verify(net.awaitPeerCount(1)); + otherNode.verify(net.awaitPeerCount(1)); + final Account recipient = accounts.createAccount("recipient"); final Hash transactionHash = @@ -57,4 +87,16 @@ public void transactionShouldBeSuccessful() { assertThat(transactionHash).isNotNull(); cluster.verify(recipient.balanceEquals(5)); } + + @Override + public void tearDownAcceptanceTestBase() { + super.tearDownAcceptanceTestBase(); + + noDiscoveryCluster.stop(); + } + + private KeyPair createKeyPair(final String privateKey) { + return SECP256R1_SIGNATURE_ALGORITHM.createKeyPair( + SECP256R1_SIGNATURE_ALGORITHM.createPrivateKey(Bytes32.fromHexString(privateKey))); + } } diff --git a/crypto/src/main/java/org/hyperledger/besu/crypto/KeyPair.java b/crypto/src/main/java/org/hyperledger/besu/crypto/KeyPair.java index 2f5b78938a8..e03242aab62 100644 --- a/crypto/src/main/java/org/hyperledger/besu/crypto/KeyPair.java +++ b/crypto/src/main/java/org/hyperledger/besu/crypto/KeyPair.java @@ -76,6 +76,15 @@ public boolean equals(final Object other) { return this.privateKey.equals(that.privateKey) && this.publicKey.equals(that.publicKey); } + @Override + public String toString() { + return ("KeyPair{privateKey: " + + this.privateKey.toString() + + ", publicKey: " + + this.publicKey.toString() + + "]"); + } + public SECPPrivateKey getPrivateKey() { return privateKey; } diff --git a/crypto/src/main/java/org/hyperledger/besu/crypto/KeyPairUtil.java b/crypto/src/main/java/org/hyperledger/besu/crypto/KeyPairUtil.java index af8b6d6a621..890843d7382 100644 --- a/crypto/src/main/java/org/hyperledger/besu/crypto/KeyPairUtil.java +++ b/crypto/src/main/java/org/hyperledger/besu/crypto/KeyPairUtil.java @@ -70,11 +70,8 @@ public static KeyPair loadKeyPair(final File keyFile) { "Loaded public key {} from {}", key.getPublicKey().toString(), keyFile.getAbsolutePath()); } else { key = SIGNATURE_ALGORITHM.get().generateKeyPair(); - try { - storeKeyPair(key, keyFile); - } catch (IOException e) { - throw new IllegalArgumentException("Cannot store generated private key."); - } + storeKeyFile(key, keyFile.getParentFile().toPath()); + LOG.info( "Generated new public key {} and stored it to {}", key.getPublicKey().toString(), @@ -83,12 +80,20 @@ public static KeyPair loadKeyPair(final File keyFile) { return key; } - public static KeyPair loadKeyPair(final Path homeDirectory) { - return loadKeyPair(getDefaultKeyFile(homeDirectory)); + public static KeyPair loadKeyPair(final Path directory) { + return loadKeyPair(getDefaultKeyFile(directory)); + } + + public static void storeKeyFile(final KeyPair keyPair, final Path homeDirectory) { + try { + storeKeyPair(keyPair, getDefaultKeyFile(homeDirectory)); + } catch (IOException e) { + throw new IllegalArgumentException("Cannot store generated private key."); + } } - public static File getDefaultKeyFile(final Path homeDirectory) { - return homeDirectory.resolve("key").toFile(); + public static File getDefaultKeyFile(final Path directory) { + return directory.resolve("key").toFile(); } public static KeyPair load(final File file) { @@ -107,13 +112,13 @@ static SECPPrivateKey loadPrivateKey(final File file) { } } - static void storeKeyPair(final KeyPair keyKair, final File file) throws IOException { + static void storeKeyPair(final KeyPair keyPair, final File file) throws IOException { final File privateKeyDir = file.getParentFile(); privateKeyDir.mkdirs(); final Path tempPath = Files.createTempFile(privateKeyDir.toPath(), ".tmp", ""); Files.write( tempPath, - keyKair.getPrivateKey().getEncodedBytes().toString().getBytes(StandardCharsets.UTF_8)); + keyPair.getPrivateKey().getEncodedBytes().toString().getBytes(StandardCharsets.UTF_8)); Files.move(tempPath, file.toPath(), REPLACE_EXISTING, ATOMIC_MOVE); } }