From 49cd4daab29186139ef18b35340cd3552f36144f Mon Sep 17 00:00:00 2001
From: Scott Sadler <scott@alphanode.co>
Date: Sat, 17 Mar 2018 23:09:14 -0300
Subject: [PATCH] cryptoconditions secp256k1 tests canonical signature

---
 .../src/include/secp256k1/include/secp256k1.h     | 12 ++++++++++++
 .../src/include/secp256k1/src/secp256k1.c         |  8 ++++++++
 src/cryptoconditions/src/secp256k1.c              |  7 ++++++-
 src/cryptoconditions/tests/test_failure_modes.py  | 15 +++++++++++++++
 4 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/src/cryptoconditions/src/include/secp256k1/include/secp256k1.h b/src/cryptoconditions/src/include/secp256k1/include/secp256k1.h
index 06afd4c65..9db5cf3a8 100644
--- a/src/cryptoconditions/src/include/secp256k1/include/secp256k1.h
+++ b/src/cryptoconditions/src/include/secp256k1/include/secp256k1.h
@@ -95,6 +95,18 @@ SECP256K1_WARN_UNUSED_RESULT int secp256k1_ecdsa_verify(
   int pubkeylen
 ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(5);
 
+/** Check that signature is in canonical form
+ *  Returns: 1: In canonical form
+ *           0: Non canonical
+ *          -1: invalid signature
+ * In:       sig:       the signature being verified (cannot be NULL)
+ *           siglen:    the length of the signature
+ */
+SECP256K1_WARN_UNUSED_RESULT int secp256k1_ecdsa_check_canonical_sig(
+  const unsigned char *sig,
+  int siglen
+) SECP256K1_ARG_NONNULL(1);
+
 /** A pointer to a function to deterministically generate a nonce.
  * Returns: 1 if a nonce was successfully generated. 0 will cause signing to fail.
  * In:      msg32:     the 32-byte message hash being verified (will not be NULL)
diff --git a/src/cryptoconditions/src/include/secp256k1/src/secp256k1.c b/src/cryptoconditions/src/include/secp256k1/src/secp256k1.c
index d6192dc4e..48569ad9e 100644
--- a/src/cryptoconditions/src/include/secp256k1/src/secp256k1.c
+++ b/src/cryptoconditions/src/include/secp256k1/src/secp256k1.c
@@ -85,6 +85,14 @@ int secp256k1_ecdsa_verify(const secp256k1_context_t* ctx, const unsigned char *
     return ret;
 }
 
+
+int secp256k1_ecdsa_check_canonical_sig(const unsigned char *sig, int siglen) {
+    secp256k1_ecdsa_sig_t s;
+    if (!secp256k1_ecdsa_sig_parse(&s, sig, siglen)) return -1;
+    return !secp256k1_scalar_is_high(&s.s);
+}
+
+
 static int nonce_function_rfc6979(unsigned char *nonce32, const unsigned char *msg32, const unsigned char *key32, unsigned int counter, const void *data) {
    secp256k1_rfc6979_hmac_sha256_t rng;
    unsigned int i;
diff --git a/src/cryptoconditions/src/secp256k1.c b/src/cryptoconditions/src/secp256k1.c
index 507c9c84f..9e09cabe5 100644
--- a/src/cryptoconditions/src/secp256k1.c
+++ b/src/cryptoconditions/src/secp256k1.c
@@ -77,7 +77,12 @@ int secp256k1Verify(CC *cond, CCVisitor visitor) {
     if (cond->type->typeId != cc_secp256k1Type.typeId) return 1;
     // TODO: test failure mode: empty sig / null pointer
     initVerify();
-    int rc = secp256k1_ecdsa_verify(ec_ctx_verify, visitor.msg, cond->signature, SECP256K1_SIG_SIZE,
+
+    // Test for non canonical S
+    int rc = secp256k1_ecdsa_check_canonical_sig(cond->signature, SECP256K1_SIG_SIZE);
+    if (rc == 1)
+        // Test for correct sig
+        rc = secp256k1_ecdsa_verify(ec_ctx_verify, visitor.msg, cond->signature, SECP256K1_SIG_SIZE,
                 cond->publicKey, SECP256K1_PK_SIZE);
     return rc == 1;
 }
diff --git a/src/cryptoconditions/tests/test_failure_modes.py b/src/cryptoconditions/tests/test_failure_modes.py
index b95c8527e..4e252424f 100644
--- a/src/cryptoconditions/tests/test_failure_modes.py
+++ b/src/cryptoconditions/tests/test_failure_modes.py
@@ -61,3 +61,18 @@ def test_decode_invalid_condition():
 def test_validate_empty_sigs():
     #// TODO: test failure mode: empty sig / null pointer
     pass
+
+
+def test_non_canonical_secp256k1():
+    cond = {
+        "type": "secp256k1-sha-256",
+        "publicKey": "AtXZaTBVNawpp3B5wR1PDdQGYc-W4E6XSl6NfjdO4iWq",
+        # Signature is correct, but non canonical; validation should fail
+        "signature": "nC1v8580C7r2XohL3_rnQ2p7dWiDnFuhF_poGCRfudrDITgwKywgjm5CTdnHAnkK4QskG4nI0KBrActwgzSrbg"
+    }
+    res = jsonRPC('verifyFulfillment', {
+        'fulfillment': jsonRPC('encodeFulfillment', cond)['fulfillment'],
+        'condition': jsonRPC('encodeCondition', cond)['bin'],
+        'message': ''
+        })
+    assert res['valid'] == False