Use native SECP256K1 and SECP256R1 for signature normalization (hyper…

…ledger#2683) Use native SECP256K1 and SEPC256R1 for signature normalization Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
noirgif · Aug 27, 2021 · cbcf525 · cbcf525
1 parent b5113e7
commit cbcf525
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 5 deletions.
diff --git a/crypto/src/main/java/org/hyperledger/besu/crypto/SECP256K1.java b/crypto/src/main/java/org/hyperledger/besu/crypto/SECP256K1.java
@@ -21,6 +21,7 @@
 import org.hyperledger.besu.nativelib.secp256k1.LibSecp256k1.secp256k1_ecdsa_signature;
 import org.hyperledger.besu.nativelib.secp256k1.LibSecp256k1.secp256k1_pubkey;
 
+import java.math.BigInteger;
 import java.nio.ByteBuffer;
 import java.util.Optional;
 
@@ -117,7 +118,12 @@ public boolean verify(final Bytes data, final SECPSignature signature, final SEC
   public Optional<SECPPublicKey> recoverPublicKeyFromSignature(
       final Bytes32 dataHash, final SECPSignature signature) {
     if (useNative) {
-      return recoverFromSignatureNative(dataHash, signature);
+      Optional<SECPPublicKey> result = recoverFromSignatureNative(dataHash, signature);
+      if (result.isEmpty()) {
+        throw new IllegalArgumentException("Could not recover public key");
+      } else {
+        return result;
+      }
     } else {
       return super.recoverPublicKeyFromSignature(dataHash, signature);
     }
@@ -184,6 +190,18 @@ private boolean verifyNative(
         != 0;
   }
 
+  @Override
+  protected BigInteger recoverFromSignature(
+      final int recId, final BigInteger r, final BigInteger s, final Bytes32 dataHash) {
+    if (useNative) {
+      return recoverFromSignatureNative(dataHash, new SECPSignature(r, s, (byte) recId))
+          .map(key -> new BigInteger(1, key.getEncoded()))
+          .orElse(null);
+    } else {
+      return super.recoverFromSignature(recId, r, s, dataHash);
+    }
+  }
+
   private Optional<SECPPublicKey> recoverFromSignatureNative(
       final Bytes32 dataHash, final SECPSignature signature) {
 
@@ -205,7 +223,7 @@ private Optional<SECPPublicKey> recoverFromSignatureNative(
     if (LibSecp256k1.secp256k1_ecdsa_recover(
             LibSecp256k1.CONTEXT, newPubKey, parsedSignature, dataHash.toArrayUnsafe())
         == 0) {
-      throw new IllegalArgumentException("Could not recover public key");
+      return Optional.empty();
     }
 
     // parse the key

diff --git a/crypto/src/main/java/org/hyperledger/besu/crypto/SECP256R1.java b/crypto/src/main/java/org/hyperledger/besu/crypto/SECP256R1.java
@@ -102,6 +102,18 @@ private SECPSignature signNative(final Bytes32 dataHash, final KeyPair keyPair)
         nativeSignature.getV());
   }
 
+  @Override
+  protected BigInteger recoverFromSignature(
+      final int recId, final BigInteger r, final BigInteger s, final Bytes32 dataHash) {
+    if (useNative) {
+      return recoverPublicKeyFromSignatureNative(dataHash, new SECPSignature(r, s, (byte) recId))
+          .map(key -> new BigInteger(1, key.getEncoded()))
+          .orElse(null);
+    } else {
+      return super.recoverFromSignature(recId, r, s, dataHash);
+    }
+  }
+
   private Optional<SECPPublicKey> recoverPublicKeyFromSignatureNative(
       final Bytes32 dataHash, final SECPSignature signature) {
     byte[] recoveredKey;

diff --git a/crypto/src/test/java/org/hyperledger/besu/crypto/SECP256R1Test.java b/crypto/src/test/java/org/hyperledger/besu/crypto/SECP256R1Test.java
@@ -142,19 +142,24 @@ public void recoverPublicKeyFromSignature() {
   }
 
   @Test
-  public void signatureGenerationAndVerification() {
+  public void signatureGenerationVerificationAndPubKeyRecovery() {
     signTestVectors.forEach(
         signTestVector -> {
           final SECPPrivateKey privateKey =
               secp256R1.createPrivateKey(new BigInteger(signTestVector.getPrivateKey(), 16));
-          final SECPPublicKey publicKey =
-              secp256R1.createPublicKey(new BigInteger(signTestVector.getPublicKey(), 16));
+          final BigInteger publicKeyBigInt = new BigInteger(signTestVector.getPublicKey(), 16);
+          final SECPPublicKey publicKey = secp256R1.createPublicKey(publicKeyBigInt);
           final KeyPair keyPair = secp256R1.createKeyPair(privateKey);
 
           final Bytes32 dataHash = keccak256(Bytes.wrap(signTestVector.getData().getBytes(UTF_8)));
 
           final SECPSignature signature = secp256R1.sign(dataHash, keyPair);
           assertThat(secp256R1.verify(dataHash, signature, publicKey)).isTrue();
+
+          final BigInteger recoveredPubKeyBigInt =
+              secp256R1.recoverFromSignature(
+                  signature.getRecId(), signature.getR(), signature.getS(), dataHash);
+          assertThat(recoveredPubKeyBigInt).isEqualTo(recoveredPubKeyBigInt);
         });
   }