Fixed elliptic curve configuration in genesis file (hyperledger#2066)

* refactored code to always set an instance in SignatureAlgorithmFactory in BesuCommand Signed-off-by: Daniel Lehrner <daniel@io.builders>
noirgif · Mar 24, 2021 · 17cf72b · 17cf72b
1 parent 7c8633a
commit 17cf72b
Show file tree

Hide file tree

Showing 10 changed files with 305 additions and 3 deletions.
diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@@ -85,6 +85,7 @@
 import org.hyperledger.besu.crypto.KeyPairUtil;
 import org.hyperledger.besu.crypto.NodeKey;
 import org.hyperledger.besu.crypto.SignatureAlgorithmFactory;
+import org.hyperledger.besu.crypto.SignatureAlgorithmType;
 import org.hyperledger.besu.enclave.EnclaveFactory;
 import org.hyperledger.besu.enclave.GoQuorumEnclave;
 import org.hyperledger.besu.ethereum.api.ApiConfiguration;
@@ -1568,6 +1569,7 @@ private BesuCommand configure() throws Exception {
     metricsConfiguration = metricsConfiguration();
 
     logger.info("Security Module: {}", securityModuleName);
+    instantiateSignatureAlgorithmFactory();
     return this;
   }
 
@@ -2628,4 +2630,33 @@ public int getDatabaseVersion() {
           .getDatabaseVersion();
     }
   }
+
+  private void instantiateSignatureAlgorithmFactory() {
+    if (SignatureAlgorithmFactory.isInstanceSet()) {
+      return;
+    }
+
+    Optional<String> ecCurve = getEcCurveFromGenesisFile();
+
+    if (ecCurve.isEmpty()) {
+      SignatureAlgorithmFactory.setDefaultInstance();
+      return;
+    }
+
+    try {
+      SignatureAlgorithmFactory.setInstance(SignatureAlgorithmType.create(ecCurve.get()));
+    } catch (IllegalArgumentException e) {
+      throw new CommandLine.InitializationException(e.getMessage());
+    }
+  }
+
+  private Optional<String> getEcCurveFromGenesisFile() {
+    if (genesisFile == null) {
+      return Optional.empty();
+    }
+
+    GenesisConfigOptions options = readGenesisConfigOptions();
+
+    return options.getEcCurve();
+  }
 }
diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java
@@ -50,6 +50,7 @@
 import org.hyperledger.besu.config.GenesisConfigFile;
 import org.hyperledger.besu.config.experimental.ExperimentalEIPs;
 import org.hyperledger.besu.controller.TargetingGasLimitCalculator;
+import org.hyperledger.besu.crypto.SignatureAlgorithmFactory;
 import org.hyperledger.besu.ethereum.api.graphql.GraphQLConfiguration;
 import org.hyperledger.besu.ethereum.api.handlers.TimeoutOptions;
 import org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcConfiguration;
@@ -132,6 +133,10 @@ public class BesuCommandTest extends CommandTestAbstract {
               new JsonObject().put("isquorum", true).put("chainId", GENESIS_CONFIG_TEST_CHAINID));
   private static final JsonObject INVALID_GENESIS_QUORUM_INTEROP_ENABLED_MAINNET =
       (new JsonObject()).put("config", new JsonObject().put("isquorum", true));
+  private static final JsonObject INVALID_GENESIS_EC_CURVE =
+      (new JsonObject()).put("config", new JsonObject().put("ecCurve", "abcd"));
+  private static final JsonObject VALID_GENESIS_EC_CURVE =
+      (new JsonObject()).put("config", new JsonObject().put("ecCurve", "secp256k1"));
   private static final String ENCLAVE_PUBLIC_KEY_PATH =
       BesuCommand.class.getResource("/orion_publickey.pub").getPath();
 
@@ -4191,4 +4196,26 @@ public void staticNodesFileOptionValidParamenter() throws IOException {
     assertThat(commandOutput.toString()).isEmpty();
     assertThat(commandErrorOutput.toString()).isEmpty();
   }
+
+  @Test
+  public void invalidEcCurveFailsWithErrorMessage() throws IOException {
+    SignatureAlgorithmFactory.resetInstance();
+    final Path genesisFile = createFakeGenesisFile(INVALID_GENESIS_EC_CURVE);
+
+    parseCommand("--genesis-file", genesisFile.toString());
+    assertThat(commandOutput.toString()).isEmpty();
+    assertThat(commandErrorOutput.toString())
+        .contains(
+            "Invalid genesis file configuration. "
+                + "Elliptic curve (ecCurve) abcd is not in the list of valid elliptic curves [secp256k1]");
+  }
+
+  @Test
+  public void validEcCurveSucceeds() throws IOException {
+    SignatureAlgorithmFactory.resetInstance();
+    final Path genesisFile = createFakeGenesisFile(VALID_GENESIS_EC_CURVE);
+
+    parseCommand("--genesis-file", genesisFile.toString());
+    assertThat(commandErrorOutput.toString()).isEmpty();
+  }
 }
diff --git a/config/src/main/java/org/hyperledger/besu/config/GenesisConfigOptions.java b/config/src/main/java/org/hyperledger/besu/config/GenesisConfigOptions.java
@@ -230,4 +230,11 @@ public interface GenesisConfigOptions {
    * @return the PoW algorithm in use.
    */
   PowAlgorithm getPowAlgorithm();
+
+  /**
+   * The elliptic curve which should be used in SignatureAlgorithm.
+   *
+   * @return the name of the elliptic curve.
+   */
+  Optional<String> getEcCurve();
 }
diff --git a/config/src/main/java/org/hyperledger/besu/config/JsonGenesisConfigOptions.java b/config/src/main/java/org/hyperledger/besu/config/JsonGenesisConfigOptions.java
@@ -42,8 +42,9 @@ public class JsonGenesisConfigOptions implements GenesisConfigOptions {
   private static final String IBFT2_CONFIG_KEY = "ibft2";
   private static final String QBFT_CONFIG_KEY = "qbft";
   private static final String CLIQUE_CONFIG_KEY = "clique";
-
+  private static final String EC_CURVE_CONFIG_KEY = "eccurve";
   private static final String TRANSITIONS_CONFIG_KEY = "transitions";
+
   private final ObjectNode configRoot;
   private final Map<String, String> configOverrides = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
   private final TransitionsConfigOptions transitions;
@@ -346,6 +347,11 @@ public PowAlgorithm getPowAlgorithm() {
         : isKeccak256() ? PowAlgorithm.KECCAK256 : PowAlgorithm.UNSUPPORTED;
   }
 
+  @Override
+  public Optional<String> getEcCurve() {
+    return JsonUtil.getString(configRoot, EC_CURVE_CONFIG_KEY);
+  }
+
   @Override
   public Map<String, Object> asMap() {
     final ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder();

diff --git a/config/src/main/java/org/hyperledger/besu/config/StubGenesisConfigOptions.java b/config/src/main/java/org/hyperledger/besu/config/StubGenesisConfigOptions.java
@@ -54,6 +54,7 @@ public class StubGenesisConfigOptions implements GenesisConfigOptions {
   private OptionalInt contractSizeLimit = OptionalInt.empty();
   private OptionalInt stackSizeLimit = OptionalInt.empty();
   private final OptionalLong ecip1017EraRounds = OptionalLong.empty();
+  private Optional<String> ecCurve = Optional.empty();
 
   @Override
   public String getConsensusEngine() {
@@ -322,6 +323,11 @@ public PowAlgorithm getPowAlgorithm() {
         : isKeccak256() ? PowAlgorithm.KECCAK256 : PowAlgorithm.UNSUPPORTED;
   }
 
+  @Override
+  public Optional<String> getEcCurve() {
+    return ecCurve;
+  }
+
   @Override
   public List<Long> getForks() {
     return Collections.emptyList();
@@ -441,4 +447,9 @@ public StubGenesisConfigOptions stackSizeLimit(final int stackSizeLimit) {
     this.stackSizeLimit = OptionalInt.of(stackSizeLimit);
     return this;
   }
+
+  public StubGenesisConfigOptions ecCurve(final Optional<String> ecCurve) {
+    this.ecCurve = ecCurve;
+    return this;
+  }
 }
diff --git a/config/src/test/java/org/hyperledger/besu/config/JsonGenesisConfigOptionsTest.java b/config/src/test/java/org/hyperledger/besu/config/JsonGenesisConfigOptionsTest.java
@@ -197,4 +197,26 @@ public void ibftBlockRewardAsDecimalNumberCorrectlyDecodes() {
 
     assertThat(configOptions.getBftConfigOptions().getBlockRewardWei()).isEqualTo(12);
   }
+
+  @Test
+  public void configWithoutEcCurveReturnsEmptyOptional() {
+    final ObjectNode configNode = loadCompleteDataSet();
+
+    final JsonGenesisConfigOptions configOptions =
+        JsonGenesisConfigOptions.fromJsonObject(configNode);
+
+    assertThat(configOptions.getEcCurve().isEmpty()).isTrue();
+  }
+
+  @Test
+  public void configWithEcCurveIsCorrectlySet() {
+    final ObjectNode configNode = loadCompleteDataSet();
+    configNode.put("eccurve", "secp256k1");
+
+    final JsonGenesisConfigOptions configOptions =
+        JsonGenesisConfigOptions.fromJsonObject(configNode);
+
+    assertThat(configOptions.getEcCurve().isPresent()).isTrue();
+    assertThat(configOptions.getEcCurve().get()).isEqualTo("secp256k1");
+  }
 }
diff --git a/crypto/src/main/java/org/hyperledger/besu/crypto/SignatureAlgorithmFactory.java b/crypto/src/main/java/org/hyperledger/besu/crypto/SignatureAlgorithmFactory.java
@@ -14,11 +14,45 @@
  */
 package org.hyperledger.besu.crypto;
 
+import com.google.common.annotations.VisibleForTesting;
+
 public class SignatureAlgorithmFactory {
+  private static SignatureAlgorithm instance = null;
+
+  private SignatureAlgorithmFactory() {}
+
+  public static void setDefaultInstance() {
+    instance = SignatureAlgorithmType.createDefault().getInstance();
+  }
+
+  public static void setInstance(final SignatureAlgorithmType signatureAlgorithmType)
+      throws IllegalStateException {
+    if (instance != null) {
+      throw new IllegalStateException(
+          "Instance of SignatureAlgorithmFactory can only be set once.");
+    }
 
-  private static final SignatureAlgorithm instance = new SECP256K1();
+    instance = signatureAlgorithmType.getInstance();
+  }
 
+  /**
+   * getInstance will always return a valid SignatureAlgorithm and never null. This is necessary in
+   * the unit tests be able to use the factory without having to call setInstance first.
+   *
+   * @return SignatureAlgorithm
+   */
   public static SignatureAlgorithm getInstance() {
-    return instance;
+    return instance != null
+        ? instance
+        : SignatureAlgorithmType.DEFAULT_SIGNATURE_ALGORITHM_TYPE.get();
+  }
+
+  public static boolean isInstanceSet() {
+    return instance != null;
+  }
+
+  @VisibleForTesting
+  public static void resetInstance() {
+    instance = null;
   }
 }
diff --git a/crypto/src/main/java/org/hyperledger/besu/crypto/SignatureAlgorithmType.java b/crypto/src/main/java/org/hyperledger/besu/crypto/SignatureAlgorithmType.java
@@ -0,0 +1,80 @@
+/*
+ * Copyright 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.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.crypto;
+
+import java.util.Iterator;
+import java.util.Map;
+import java.util.function.Supplier;
+
+public class SignatureAlgorithmType {
+
+  private static final Map<String, Supplier<SignatureAlgorithm>> SUPPORTED_ALGORITHMS =
+      Map.of("secp256k1", SECP256K1::new);
+
+  public static final Supplier<SignatureAlgorithm> DEFAULT_SIGNATURE_ALGORITHM_TYPE =
+      SUPPORTED_ALGORITHMS.get("secp256k1");
+
+  private final Supplier<SignatureAlgorithm> instantiator;
+
+  private SignatureAlgorithmType(final Supplier<SignatureAlgorithm> instantiator) {
+    this.instantiator = instantiator;
+  }
+
+  public static SignatureAlgorithmType create(final String ecCurve)
+      throws IllegalArgumentException {
+    if (!isValidType(ecCurve)) {
+      throw new IllegalArgumentException(
+          new StringBuilder()
+              .append("Invalid genesis file configuration. Elliptic curve (ecCurve) ")
+              .append(ecCurve)
+              .append(" is not in the list of valid elliptic curves ")
+              .append(getEcCurvesListAsString())
+              .toString());
+    }
+
+    return new SignatureAlgorithmType(SUPPORTED_ALGORITHMS.get(ecCurve));
+  }
+
+  public static SignatureAlgorithmType createDefault() {
+    return new SignatureAlgorithmType(DEFAULT_SIGNATURE_ALGORITHM_TYPE);
+  }
+
+  public SignatureAlgorithm getInstance() {
+    return instantiator.get();
+  }
+
+  public static boolean isValidType(final String ecCurve) {
+    return SUPPORTED_ALGORITHMS.containsKey(ecCurve);
+  }
+
+  private static String getEcCurvesListAsString() {
+    Iterator<Map.Entry<String, Supplier<SignatureAlgorithm>>> it =
+        SUPPORTED_ALGORITHMS.entrySet().iterator();
+
+    StringBuilder ecCurveListBuilder = new StringBuilder();
+    ecCurveListBuilder.append("[");
+
+    while (it.hasNext()) {
+      ecCurveListBuilder.append(it.next().getKey());
+
+      if (it.hasNext()) {
+        ecCurveListBuilder.append(", ");
+      }
+    }
+    ecCurveListBuilder.append("]");
+
+    return ecCurveListBuilder.toString();
+  }
+}
diff --git a/crypto/src/test/java/org/hyperledger/besu/crypto/SignatureAlgorithmFactoryTest.java b/crypto/src/test/java/org/hyperledger/besu/crypto/SignatureAlgorithmFactoryTest.java
@@ -0,0 +1,46 @@
+/*
+ * Copyright 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.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.crypto;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+
+import org.junit.Test;
+
+public class SignatureAlgorithmFactoryTest {
+
+  @Test
+  public void shouldReturnSECP256K1InstanceByDefault() {
+    SignatureAlgorithm signatureAlgorithm = SignatureAlgorithmFactory.getInstance();
+    assertThat(signatureAlgorithm.getClass().getSimpleName())
+        .isEqualTo(SECP256K1.class.getSimpleName());
+  }
+
+  @Test
+  public void shouldReturnSECP256K1InstanceWhenSet() {
+    SignatureAlgorithmFactory.setInstance(SignatureAlgorithmType.create("secp256k1"));
+
+    SignatureAlgorithm signatureAlgorithm = SignatureAlgorithmFactory.getInstance();
+    assertThat(signatureAlgorithm.getClass().getSimpleName())
+        .isEqualTo(SECP256K1.class.getSimpleName());
+  }
+
+  @Test(expected = RuntimeException.class)
+  public void shouldThrowExceptionWhenSetMoreThanOnce() {
+    SignatureAlgorithmFactory.setInstance(SignatureAlgorithmType.create("secp256k1"));
+    assertThat(SignatureAlgorithmFactory.isInstanceSet()).isTrue();
+
+    SignatureAlgorithmFactory.setInstance(SignatureAlgorithmType.create("secp256k1"));
+  }
+}