SECP256R1 BC now using non-deterministic variant (#2501)

* SECP256R1 BC now using non-deterministic variant

Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>

crypto/src/main/java/org/hyperledger/besu/crypto/AbstractSECP256.java

@@ -32,17 +32,17 @@ import org.bouncycastle.asn1.sec.SECNamedCurves;
 import org.bouncycastle.asn1.x9.X9ECParameters;
 import org.bouncycastle.asn1.x9.X9IntegerConverter;
 import org.bouncycastle.crypto.agreement.ECDHBasicAgreement;
-import org.bouncycastle.crypto.digests.SHA256Digest;
 import org.bouncycastle.crypto.params.ECDomainParameters;
 import org.bouncycastle.crypto.params.ECPrivateKeyParameters;
 import org.bouncycastle.crypto.params.ECPublicKeyParameters;
+import org.bouncycastle.crypto.signers.DSAKCalculator;
 import org.bouncycastle.crypto.signers.ECDSASigner;
-import org.bouncycastle.crypto.signers.HMacDSAKCalculator;
 import org.bouncycastle.jce.provider.BouncyCastleProvider;
 import org.bouncycastle.math.ec.ECAlgorithms;
 import org.bouncycastle.math.ec.ECPoint;
 
 public abstract class AbstractSECP256 implements SignatureAlgorithm {
+
   protected static final int PRIVATE_KEY_BYTE_LENGTH = 32;
   protected static final int PUBLIC_KEY_BYTE_LENGTH = 64;
   protected static final int SIGNATURE_BYTE_LENGTH = 65;
@@ -215,6 +215,8 @@ public abstract class AbstractSECP256 implements SignatureAlgorithm {
     return PROVIDER;
   }
 
+  public abstract DSAKCalculator getKCalculator();
+
   // Decompress a compressed public key (x co-ord and low-bit of y-coord).
   protected ECPoint decompressKey(final BigInteger xBN, final boolean yBit) {
     final X9IntegerConverter x9 = new X9IntegerConverter();
@@ -305,7 +307,7 @@ public abstract class AbstractSECP256 implements SignatureAlgorithm {
 
   @Override
   public SECPSignature sign(final Bytes32 dataHash, final KeyPair keyPair) {
-    final ECDSASigner signer = new ECDSASigner(new HMacDSAKCalculator(new SHA256Digest()));
+    final ECDSASigner signer = new ECDSASigner(getKCalculator());
 
     final ECPrivateKeyParameters privKey =
         new ECPrivateKeyParameters(

crypto/src/main/java/org/hyperledger/besu/crypto/SECP256K1.java

@@ -30,6 +30,9 @@ import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.tuweni.bytes.Bytes;
 import org.apache.tuweni.bytes.Bytes32;
+import org.bouncycastle.crypto.digests.SHA256Digest;
+import org.bouncycastle.crypto.signers.DSAKCalculator;
+import org.bouncycastle.crypto.signers.HMacDSAKCalculator;
 import org.bouncycastle.math.ec.custom.sec.SecP256K1Curve;
 
 /*
@@ -68,6 +71,19 @@ public class SECP256K1 extends AbstractSECP256 {
     return useNative;
   }
 
+  /**
+   * SECP256K1 is using the deterministic implementation of K calculation (defined on RFC 6979
+   * section 3.2)
+   *
+   * @return an instance of HMacDSAKCalculator
+   * @see <a href="https://datatracker.ietf.org/doc/html/rfc6979#section-3.2">RFC 6979 Section
+   *     3.2</a>
+   */
+  @Override
+  public DSAKCalculator getKCalculator() {
+    return new HMacDSAKCalculator(new SHA256Digest());
+  }
+
   @Override
   public SECPSignature sign(final Bytes32 dataHash, final KeyPair keyPair) {
     if (useNative) {

crypto/src/main/java/org/hyperledger/besu/crypto/SECP256R1.java

@@ -14,6 +14,8 @@
  */
 package org.hyperledger.besu.crypto;
 
+import org.bouncycastle.crypto.signers.DSAKCalculator;
+import org.bouncycastle.crypto.signers.RandomDSAKCalculator;
 import org.bouncycastle.math.ec.custom.sec.SecP256R1Curve;
 
 public class SECP256R1 extends AbstractSECP256 {
@@ -32,6 +34,16 @@ public class SECP256R1 extends AbstractSECP256 {
     return false;
   }
 
+  /**
+   * SECP256R1 is using the non-deterministic implementation of K calculation (standard)
+   *
+   * @return an instance of RandomDSAKCalculator
+   */
+  @Override
+  public DSAKCalculator getKCalculator() {
+    return new RandomDSAKCalculator();
+  }
+
   @Override
   public String getCurveName() {
     return CURVE_NAME;

crypto/src/test/java/org/hyperledger/besu/crypto/SECP256R1Test.java

@@ -76,26 +76,7 @@ public class SECP256R1Test {
   }
 
   @Test
-  public void signatureGeneration() {
-    final SECPPrivateKey privateKey =
-        secp256R1.createPrivateKey(
-            new BigInteger("909753034398cf9371b88871c0a8b3051f1bb55d4f28d3d7261abe7d32adcdde", 16));
-    final KeyPair keyPair = secp256R1.createKeyPair(privateKey);
-
-    final Bytes data = Bytes.wrap("This is an example of a signed message.".getBytes(UTF_8));
-    final Bytes32 dataHash = keccak256(data);
-    final SECPSignature expectedSignature =
-        secp256R1.createSignature(
-            new BigInteger("6ae3ac096d1b69ab1e18a721689cc40f2710ab25c35a4f465b8384c470e7079b", 16),
-            new BigInteger("28a39d61a8812005312b552e022afd6fa3db323754f48033c87f4acf6e9960e6", 16),
-            (byte) 1);
-
-    final SECPSignature actualSignature = secp256R1.sign(dataHash, keyPair);
-    assertThat(actualSignature).isEqualTo(expectedSignature);
-  }
-
-  @Test
-  public void signatureVerification() {
+  public void signatureGenerationAndVerification() {
     final SECPPrivateKey privateKey =
         secp256R1.createPrivateKey(
             new BigInteger("a7e8b16ad7ffa26fce80be2b0e00008018aadf1b16dea4ecc913b8c1c4f18531", 16));
@@ -105,6 +86,7 @@ public class SECP256R1Test {
     final Bytes32 dataHash = keccak256(data);
 
     final SECPSignature signature = secp256R1.sign(dataHash, keyPair);
+
     assertThat(secp256R1.verify(data, signature, keyPair.getPublicKey(), Hash::keccak256)).isTrue();
   }