Fix: Native libraries of secp256k1 and alt bn 128 can be disabled (#2163

) fix disabling of native libraries Signed-off-by: Daniel Lehrner <daniel@io.builders>
hyperledger · Jun 17, 2021 · ec4db5b · ec4db5b
1 parent 413d62a
commit ec4db5b
Show file tree

Hide file tree

Showing 7 changed files with 83 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,8 @@
 - eip-1559 changes: accept transactions which have maxFeePerGas below current baseFee [\#2374](https://github.com/hyperledger/besu/pull/2374)
 - Introduced transitions for IBFT2 block rewards [\#1977](https://github.com/hyperledger/besu/pull/1977) 
 - Change Ethstats's status from experimental feature to stable. [\#2405](https://github.com/hyperledger/besu/pull/2405) 
+- Fixed disabling of native libraries for secp256k1 and altBn128. [\#2163](https://github.com/hyperledger/besu/pull/2163)
+
 
 ### Bug Fixes
 

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@@ -1404,11 +1404,20 @@ public static Optional<Boolean> getColorEnabled() {
   }
 
   private void configureNativeLibs() {
-    if (unstableNativeLibraryOptions.getNativeAltbn128()) {
-      AbstractAltBnPrecompiledContract.enableNative();
+    if (unstableNativeLibraryOptions.getNativeAltbn128()
+        && AbstractAltBnPrecompiledContract.isNative()) {
+      logger.info("Using LibEthPairings native alt bn128");
+    } else {
+      AbstractAltBnPrecompiledContract.disableNative();
+      logger.info("Using the Java implementation of alt bn128");
     }
-    if (unstableNativeLibraryOptions.getNativeSecp256k1()) {
-      SignatureAlgorithmFactory.getInstance().enableNative();
+
+    if (unstableNativeLibraryOptions.getNativeSecp256k1()
+        && SignatureAlgorithmFactory.getInstance().isNative()) {
+      logger.info("Using native secp256k1");
+    } else {
+      SignatureAlgorithmFactory.getInstance().disableNative();
+      logger.info("Using the Java implementation of secp256k1");
     }
   }
 

diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java
@@ -66,6 +66,7 @@
 import org.hyperledger.besu.ethereum.core.Wei;
 import org.hyperledger.besu.ethereum.eth.sync.SyncMode;
 import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration;
+import org.hyperledger.besu.ethereum.mainnet.precompiles.AbstractAltBnPrecompiledContract;
 import org.hyperledger.besu.ethereum.p2p.peers.EnodeURLImpl;
 import org.hyperledger.besu.ethereum.permissioning.LocalPermissioningConfiguration;
 import org.hyperledger.besu.ethereum.permissioning.PermissioningConfiguration;
@@ -4346,4 +4347,36 @@ public void validEcCurveSucceeds() throws IOException {
     parseCommand("--genesis-file", genesisFile.toString());
     assertThat(commandErrorOutput.toString()).isEmpty();
   }
+
+  @Test
+  public void nativeSecp256IsDisabled() {
+    SignatureAlgorithmFactory.resetInstance();
+    parseCommand("--Xsecp256k1-native-enabled", "false");
+
+    verify(mockLogger).info("Using the Java implementation of secp256k1");
+    assertThat(SignatureAlgorithmFactory.getInstance().isNative()).isFalse();
+  }
+
+  @Test
+  public void nativeAltBn128IsDisabled() {
+    // it is necessary to reset it, because the tested variable
+    // is static and it will stay true if it has been set in another test
+    // AbstractAltBnPrecompiledContract.resetNative();
+
+    parseCommand("--Xaltbn128-native-enabled", "false");
+
+    verify(mockLogger).info("Using the Java implementation of alt bn128");
+    assertThat(AbstractAltBnPrecompiledContract.isNative()).isFalse();
+  }
+
+  @Test
+  public void nativeLibrariesAreEnabledByDefault() {
+    parseCommand();
+
+    assertThat(SignatureAlgorithmFactory.getInstance().isNative()).isTrue();
+    verify(mockLogger).info("Using native secp256k1");
+
+    assertThat(AbstractAltBnPrecompiledContract.isNative()).isTrue();
+    verify(mockLogger).info("Using LibEthPairings native alt bn128");
+  }
 }
diff --git a/crypto/src/main/java/org/hyperledger/besu/crypto/SECP256K1.java b/crypto/src/main/java/org/hyperledger/besu/crypto/SECP256K1.java
@@ -44,18 +44,28 @@ public class SECP256K1 extends AbstractSECP256 {
 
   private static final Logger LOG = LogManager.getLogger();
 
-  private boolean useNative = true;
+  private boolean useNative;
 
   public static final String CURVE_NAME = "secp256k1";
 
   public SECP256K1() {
     super(CURVE_NAME, SecP256K1Curve.q);
+
+    // use the native library implementation, if it is available
+    useNative = LibSecp256k1.CONTEXT != null;
+    if (!useNative) {
+      LOG.info("Native secp256k1 not available");
+    }
   }
 
   @Override
-  public void enableNative() {
-    useNative = LibSecp256k1.CONTEXT != null;
-    LOG.info(useNative ? "Using native secp256k1" : "Native secp256k1 requested but not available");
+  public void disableNative() {
+    useNative = false;
+  }
+
+  @Override
+  public boolean isNative() {
+    return useNative;
   }
 
   @Override

diff --git a/crypto/src/main/java/org/hyperledger/besu/crypto/SECP256R1.java b/crypto/src/main/java/org/hyperledger/besu/crypto/SECP256R1.java
@@ -14,22 +14,22 @@
  */
 package org.hyperledger.besu.crypto;
 
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
 import org.bouncycastle.math.ec.custom.sec.SecP256R1Curve;
 
 public class SECP256R1 extends AbstractSECP256 {
 
-  private static final Logger LOG = LogManager.getLogger();
   public static final String CURVE_NAME = "secp256r1";
 
   public SECP256R1() {
     super(CURVE_NAME, SecP256R1Curve.q);
   }
 
   @Override
-  public void enableNative() {
-    LOG.warn("Native secp256r1 requested but not available");
+  public void disableNative() {}
+
+  @Override
+  public boolean isNative() {
+    return false;
   }
 
   @Override

diff --git a/crypto/src/main/java/org/hyperledger/besu/crypto/SignatureAlgorithm.java b/crypto/src/main/java/org/hyperledger/besu/crypto/SignatureAlgorithm.java
@@ -26,7 +26,9 @@ public interface SignatureAlgorithm {
   // needs to be known at compile time otherwise triggers InsecureCryptoUsage error
   String ALGORITHM = "ECDSA";
 
-  void enableNative();
+  void disableNative();
+
+  boolean isNative();
 
   SECPSignature sign(final Bytes32 dataHash, final KeyPair keyPair);
 

diff --git a/...a/org/hyperledger/besu/ethereum/mainnet/precompiles/AbstractAltBnPrecompiledContract.java b/...a/org/hyperledger/besu/ethereum/mainnet/precompiles/AbstractAltBnPrecompiledContract.java
@@ -30,14 +30,16 @@
 public abstract class AbstractAltBnPrecompiledContract extends AbstractPrecompiledContract {
 
   private static final Logger LOG = LogManager.getLogger();
-  static boolean useNative = true;
 
-  public static void enableNative() {
-    useNative = LibEthPairings.ENABLED;
-    LOG.info(
-        useNative
-            ? "Using LibEthPairings native alt bn128"
-            : "Native alt bn128 requested but not available");
+  // use the native library implementation, if it is available
+  static boolean useNative = LibEthPairings.ENABLED;
+
+  public static void disableNative() {
+    useNative = false;
+  }
+
+  public static boolean isNative() {
+    return useNative;
   }
 
   private final byte operationId;
@@ -46,6 +48,10 @@ public static void enableNative() {
       final String name, final GasCalculator gasCalculator, final byte operationId) {
     super(name, gasCalculator);
     this.operationId = operationId;
+
+    if (!LibEthPairings.ENABLED) {
+      LOG.info("Native alt bn128 not available");
+    }
   }
 
   public Bytes computeNative(final Bytes input, final MessageFrame messageFrame) {