WIP: ecdsaSignatureParseDer

secp-api/src/main/java/org/bitcoinj/secp/Secp256k1.java

@@ -284,6 +284,13 @@ default SecpPubKey ecPubKeyFromXOnly(SecpXOnlyPubKey xOnlyPubKey){
      */
     SecpResult<EcdsaSignature> ecdsaSignatureParseCompact(byte[] serialized_signature);
 
+    /**
+     * Parse an ECDSA signature in DER format.
+     * @param derSignature DER-format signature bytes
+     * @return signature object
+     */
+    SecpResult<EcdsaSignature> ecdsaSignatureParseDer(byte[] derSignature);
+
     /**
      * Verify an ECDSA signature is valid and low-s.
      * @param sig The signature to verify.

secp-bouncy/src/main/java/org/bitcoinj/secp/bouncy/Bouncy256k1.java

@@ -31,6 +31,12 @@
 import org.bitcoinj.secp.internal.SecpPrivKeyImpl;
 import org.bitcoinj.secp.internal.SecpScalarImpl;
 import org.bitcoinj.secp.internal.SecpXOnlyPubKeyImpl;
+import org.bouncycastle.asn1.ASN1Encoding;
+import org.bouncycastle.asn1.ASN1InputStream;
+import org.bouncycastle.asn1.ASN1Integer;
+import org.bouncycastle.asn1.ASN1Primitive;
+import org.bouncycastle.asn1.ASN1Sequence;
+import org.bouncycastle.asn1.DLSequence;
 import org.bouncycastle.asn1.x9.X9ECParameters;
 import org.bouncycastle.crypto.AsymmetricCipherKeyPair;
 import org.bouncycastle.crypto.digests.SHA256Digest;
@@ -50,7 +56,9 @@
 import org.bouncycastle.math.ec.custom.sec.SecP256K1Curve;
 import org.bouncycastle.math.ec.custom.sec.SecP256K1Point;
 import org.bouncycastle.util.Arrays;
+import org.bouncycastle.util.Properties;
 
+import java.io.IOException;
 import java.math.BigInteger;
 import java.nio.ByteBuffer;
 import java.security.MessageDigest;
@@ -280,6 +288,79 @@ public SecpResult<EcdsaSignature> ecdsaSignatureParseCompact(byte[] serializedSi
         }
     }
 
+    @Override
+    public SecpResult<EcdsaSignature> ecdsaSignatureParseDer(byte[] derSignature) {
+        ASN1Sequence seq;
+        try {
+            // fromByteArray() rejects trailing bytes after the top-level object
+            ASN1Primitive primitive = ASN1Primitive.fromByteArray(derSignature);
+            if (!(primitive instanceof ASN1Sequence)) {
+                return SecpResult.err(0);
+            }
+            seq = (ASN1Sequence) primitive;
+            // BC's parser accepts BER. Re-encoding as DER and comparing rejects the
+            // indefinite-length and non-minimal encodings that strict DER forbids.
+            if (!Arrays.areEqual(derSignature, seq.getEncoded(ASN1Encoding.DER))) {
+                return SecpResult.err(0);
+            }
+        } catch (IOException | IllegalArgumentException e) {
+            // IOException: malformed input or trailing data
+            // IllegalArgumentException: BC rejecting a non-minimally-encoded INTEGER
+            return SecpResult.err(0);
+        }
+        if (seq.size() != 2
+                || !(seq.getObjectAt(0) instanceof ASN1Integer)
+                || !(seq.getObjectAt(1) instanceof ASN1Integer)) {
+            return SecpResult.err(0);
+        }
+        BigInteger r = ((ASN1Integer) seq.getObjectAt(0)).getValue();
+        BigInteger s = ((ASN1Integer) seq.getObjectAt(1)).getValue();
+        if (r.signum() < 0 || s.signum() < 0) {
+            return SecpResult.err(0);   // the C parser rejects a set high bit as negative
+        }
+        return SecpResult.ok(new EcdsaSignatureBc(zeroIfOverflow(r), zeroIfOverflow(s)));
+    }
+
+    /**
+     * Replace an out-of-range scalar with zero, matching {@code secp256k1_der_parse_integer}.
+     * @param scalar a non-negative scalar
+     * @return the scalar, or {@link BigInteger#ZERO} if it is not less than the curve order
+     */
+    private static BigInteger zeroIfOverflow(BigInteger scalar) {
+        return scalar.compareTo(BC_ECDOMAIN_PARAMS.getN()) < 0 ? scalar : BigInteger.ZERO;
+    }
+
+    public SecpResult<EcdsaSignature> ecdsaSignatureParseDerBitcoinJ(byte[] derSignature) {
+        try {
+            // BouncyCastle by default is strict about parsing ASN.1 integers. We relax this check, because some
+            // Bitcoin signatures would not parse.
+            Properties.setThreadOverride("org.bouncycastle.asn1.allow_unsafe_integer", true);
+            final ASN1Primitive seqObj;
+            try (ASN1InputStream decoder = new ASN1InputStream(derSignature)) {
+                seqObj = decoder.readObject();
+            }
+            if (seqObj == null)
+                throw new IllegalArgumentException("Reached past end of ASN.1 stream.");
+            if (!(seqObj instanceof DLSequence))
+                throw new IllegalArgumentException("Read unexpected class: " + seqObj.getClass().getName());
+            final DLSequence seq = (DLSequence) seqObj;
+            ASN1Integer r, s;
+            try {
+                r = (ASN1Integer) seq.getObjectAt(0);
+                s = (ASN1Integer) seq.getObjectAt(1);
+            } catch (ClassCastException e) {
+                throw new IllegalArgumentException(e);
+            }
+            // OpenSSL deviates from the DER spec by interpreting these values as unsigned, though they should not be
+            // Thus, we always use the positive versions. See: http://r6.ca/blog/20111119T211504Z.html
+            return SecpResult.ok(new EcdsaSignatureBc(r.getPositiveValue(), s.getPositiveValue()));
+        } catch (IOException e) {
+            throw new IllegalArgumentException(e);
+        } finally {
+            Properties.removeThreadOverride("org.bouncycastle.asn1.allow_unsafe_integer");
+        }
+    }
+
     @Override
     public SecpResult<Boolean> ecdsaVerify(EcdsaSignature signature, byte[] msg_hash_data, SecpPubKey pubKey) {
         checkArg(msg_hash_data.length == 32, "Message must be 32-byte (hash)");

secp-ffm/src/main/java/org/bitcoinj/secp/ffm/Secp256k1Foreign.java

@@ -43,9 +43,13 @@
 import org.bitcoinj.secp.internal.SecpXOnlyPubKeyImpl;
 
 import java.lang.foreign.Arena;
+import java.lang.foreign.MemoryLayout;
 import java.lang.foreign.MemorySegment;
 import java.lang.foreign.SegmentAllocator;
+import java.lang.foreign.StructLayout;
+import java.lang.foreign.ValueLayout;
 import java.math.BigInteger;
+import java.nio.ByteOrder;
 import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
 import java.util.Arrays;
@@ -434,6 +438,17 @@ public SecpResult<EcdsaSignature> ecdsaSignatureParseCompact(byte[] serialized_s
         }
     }
 
+    public SecpResult<EcdsaSignature> ecdsaSignatureParseDer(byte[] derSignature) {
+        try (Arena ta = Arena.ofConfined()) {
+            MemorySegment sig = secp256k1_ecdsa_signature.allocate(ta);
+            int parseResult = secp256k1_h.secp256k1_ecdsa_signature_parse_der(ctx, sig, ta.allocateFrom(JAVA_BYTE, derSignature), derSignature.length);
+            if (parseResult != OK) return SecpResult.err(parseResult);
+            MemorySegment compactSigSeg = ta.allocate(64);
+            int serializeResult = secp256k1_h.secp256k1_ecdsa_signature_serialize_compact(ctx, compactSigSeg, sig);
+            return SecpResult.checked(serializeResult, () -> new EcdsaSignatureImpl(compactSigSeg.toArray(JAVA_BYTE)));
+        }
+    }
+
     @Override
     public SecpResult<Boolean> ecdsaVerify(EcdsaSignature sig, byte[] msg_hash_data, SecpPubKey pubKey) {
         checkArg(msg_hash_data.length == 32, "Message must be 32-byte (hash)");

secp-integration-test/src/test/java/org/bitcoinj/secp/integration/EcdsaTest.java

@@ -21,7 +21,6 @@
 import org.bitcoinj.secp.EcdsaSignature;
 import org.bitcoinj.secp.bouncy.Bouncy256k1;
 import org.bitcoinj.secp.ffm.Secp256k1Foreign;
-import org.bitcoinj.secp.internal.EcdsaSignatureImpl;
 import org.bitcoinj.secp.internal.UInt256;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -170,4 +169,20 @@ void testParseCompact(Secp256k1 secp) {
         EcdsaSignature signature = secp.ecdsaSignatureParseCompact(sigBytes).get();
         assertArrayEquals(sigBytes, signature.serializeCompact());
     }
+
+    // Test interop with a signature from elsewhere (from a bitcoinj test)
+    @MethodSource("secpImplementations")
+    @ParameterizedTest(name = "Test Ecdsa for {0}")
+    void testInterop(Secp256k1 secp) {
+        SecpPrivKey privKey = secp.ecPrivKeyImport(parseHex("180cb41c7c600be951b5d3d0a7334acc7506173875834f7a6c4c786a28fcbb19"));
+        SecpPubKey pubKey = secp.ecPubKeyCreate(privKey);
+        byte[] sigBytes = parseHex(
+                "3046022100dffbc26774fc841bbe1c1362fd643609c6e42dcb274763476d87af2c0597e89e022100c59e3c13b96b316cae9fa0ab0260612c7a133a6fe2b3445b6bf80b3123bf274d");
+        EcdsaSignature sig = secp.ecdsaSignatureParseDer(sigBytes).get();
+        assertFalse(sig.hasLowS());
+        EcdsaSignature canonicalSig = sig.normalize();
+        assertTrue(canonicalSig.hasLowS());
+        assertFalse(secp.ecdsaVerify(sig, UInt256.ZERO_VALUE, pubKey).get(), "signature with non-canonical-S verified");
+        assertTrue(secp.ecdsaVerify(canonicalSig, UInt256.ZERO_VALUE, pubKey).get(), "signature with non-canonical-S failed to verify");
+    }
 }