Merge pull request #53

f6e8dc9 Generate low-S DER signatures (Ross Nicoll)
4073127 Added more unit tests on hash signing. (Ross Nicoll)

Author: petertodd

bitcoin/core/key.py

@@ -21,6 +21,8 @@
 import hashlib
 import sys
 
+import bitcoin.core.script
+
 _ssl = ctypes.cdll.LoadLibrary(ctypes.util.find_library('ssl') or 'libeay32')
 
 # this specifies the curve used with ECDSA.
@@ -36,6 +38,11 @@ def _check_result (val, func, args):
 _ssl.EC_KEY_new_by_curve_name.restype = ctypes.c_void_p
 _ssl.EC_KEY_new_by_curve_name.errcheck = _check_result
 
+# From openssl/ecdsa.h
+class ECDSA_SIG_st(ctypes.Structure):
+     _fields_ = [("r", ctypes.c_void_p),
+                ("s", ctypes.c_void_p)]
+
 class CECKey:
     """Wrapper around OpenSSL's EC_KEY"""
 
@@ -99,7 +106,6 @@ def get_ecdh_key(self, other_pubkey, kdf=lambda k: hashlib.sha256(k).digest()):
         return kdf(r)
 
     def sign(self, hash):
-        # FIXME: need unit tests for below cases
         if not isinstance(hash, bytes):
             raise TypeError('Hash must be bytes instance; got %r' % hash.__class__)
         if len(hash) != 32:
@@ -110,7 +116,39 @@ def sign(self, hash):
         mb_sig = ctypes.create_string_buffer(sig_size0.value)
         result = _ssl.ECDSA_sign(0, hash, len(hash), mb_sig, ctypes.byref(sig_size0), self.k)
         assert 1 == result
-        return mb_sig.raw[:sig_size0.value]
+        if bitcoin.core.script.IsLowDERSignature(mb_sig.raw[:sig_size0.value]):
+            return mb_sig.raw[:sig_size0.value]
+        else:
+            return self.signature_to_low_s(mb_sig.raw[:sig_size0.value])
+
+    def signature_to_low_s(self, sig):
+        der_sig = ECDSA_SIG_st()
+        _ssl.d2i_ECDSA_SIG(ctypes.byref(ctypes.pointer(der_sig)), ctypes.byref(ctypes.c_char_p(sig)), len(sig))
+        group = _ssl.EC_KEY_get0_group(self.k)
+        order = _ssl.BN_new()
+        halforder = _ssl.BN_new()
+        ctx = _ssl.BN_CTX_new()
+        _ssl.EC_GROUP_get_order(group, order, ctx)
+        _ssl.BN_rshift1(halforder, order)
+
+        # Verify that s is over half the order of the curve before we actually subtract anything from it
+        if _ssl.BN_cmp(der_sig.s, halforder) > 0:
+          _ssl.BN_sub(der_sig.s, order, der_sig.s)
+
+        _ssl.BN_free(halforder)
+        _ssl.BN_free(order)
+        _ssl.BN_CTX_free(ctx)
+
+        derlen = _ssl.i2d_ECDSA_SIG(ctypes.pointer(der_sig), 0)
+        if derlen == 0:
+            _ssl.ECDSA_SIG_free(der_sig)
+            return None
+        new_sig = ctypes.create_string_buffer(derlen)
+        _ssl.i2d_ECDSA_SIG(ctypes.pointer(der_sig), ctypes.byref(ctypes.pointer(new_sig)))
+        _ssl.BN_free(der_sig.r)
+        _ssl.BN_free(der_sig.s)
+
+        return new_sig.raw
 
     def verify(self, hash, sig):
         """Verify a DER signature"""

bitcoin/core/script.py

@@ -25,6 +25,7 @@
     _bchr = lambda x: bytes([x])
     _bord = lambda x: x
 
+import array
 import copy
 import struct
 
@@ -797,6 +798,57 @@ def FindAndDelete(script, sig):
         r += script[last_sop_idx:]
     return CScript(r)
 
+def IsLowDERSignature(sig):
+    """
+    Loosely correlates with IsLowDERSignature() from script/interpreter.cpp
+    Verifies that the S value in a DER signature is the lowest possible value.
+    Used by BIP62 malleability fixes.
+    """
+    length_r = sig[3]
+    if isinstance(length_r, str):
+        length_r = int(struct.unpack('B', length_r)[0])
+    length_s = sig[5 + length_r]
+    if isinstance(length_s, str):
+        length_s = int(struct.unpack('B', length_s)[0])
+    s_val = list(struct.unpack(str(length_s) + 'B', sig[6 + length_r:6 + length_r + length_s]))
+
+    # If the S value is above the order of the curve divided by two, its
+    # complement modulo the order could have been used instead, which is
+    # one byte shorter when encoded correctly.
+    max_mod_half_order = [
+      0x7f,0xff,0xff,0xff,0xff,0xff,0xff,0xff,
+      0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,
+      0x5d,0x57,0x6e,0x73,0x57,0xa4,0x50,0x1d,
+      0xdf,0xe9,0x2f,0x46,0x68,0x1b,0x20,0xa0]
+
+    return CompareBigEndian(s_val, [0]) > 0 and \
+      CompareBigEndian(s_val, max_mod_half_order) <= 0
+
+def CompareBigEndian(c1, c2):
+    """
+    Loosely matches CompareBigEndian() from eccryptoverify.cpp
+    Compares two arrays of bytes, and returns a negative value if the first is
+    less than the second, 0 if they're equal, and a positive value if the
+    first is greater than the second.
+    """
+    c1 = list(c1)
+    c2 = list(c2)
+
+    # Adjust starting positions until remaining lengths of the two arrays match
+    while len(c1) > len(c2):
+        if c1.pop(0) > 0:
+            return 1
+    while len(c2) > len(c1):
+        if c2.pop(0) > 0:
+            return -1
+
+    while len(c1) > 0:
+        diff = c1.pop(0) - c2.pop(0)
+        if diff != 0:
+            return diff
+
+    return 0
+
 
 def RawSignatureHash(script, txTo, inIdx, hashtype):
     """Consensus-correct SignatureHash
@@ -1008,4 +1060,5 @@ def SignatureHash(script, txTo, inIdx, hashtype):
         'FindAndDelete',
         'RawSignatureHash',
         'SignatureHash',
+        'IsLowDERSignature',
 )

bitcoin/tests/test_script.py

@@ -426,3 +426,11 @@ def T(redeemScript, expected_hex_bytes):
 
         with self.assertRaises(ValueError):
             CScript([b'a' * 518]).to_p2sh_scriptPubKey()
+
+class Test_IsLowDERSignature(unittest.TestCase):
+    def test_high_s_value(self):
+        sig = x('3046022100820121109528efda8bb20ca28788639e5ba5b365e0a84f8bd85744321e7312c6022100a7c86a21446daa405306fe10d0a9906e37d1a2c6b6fdfaaf6700053058029bbe')
+        self.assertFalse(IsLowDERSignature(sig))
+    def test_low_s_value(self):
+        sig = x('3045022100b135074e08cc93904a1712b2600d3cb01899a5b1cc7498caa4b8585bcf5f27e7022074ab544045285baef0a63f0fb4c95e577dcbf5c969c0bf47c7da8e478909d669')
+        self.assertTrue(IsLowDERSignature(sig))

bitcoin/tests/test_wallet.py

@@ -14,7 +14,7 @@
 import unittest
 
 from bitcoin.core import b2x, x
-from bitcoin.core.script import CScript
+from bitcoin.core.script import CScript, IsLowDERSignature
 from bitcoin.core.key import CPubKey
 from bitcoin.wallet import *
 
@@ -218,7 +218,20 @@ def test_sign(self):
         hash = b'\x00' * 32
         sig = key.sign(hash)
 
-        # FIXME: need better tests than this
+        # Check a valid signature
         self.assertTrue(key.pub.verify(hash, sig))
+        self.assertTrue(IsLowDERSignature(sig))
+
+        # Check invalid hash returns false
         self.assertFalse(key.pub.verify(b'\xFF'*32, sig))
+        # Check invalid signature returns false
         self.assertFalse(key.pub.verify(hash, sig[0:-1] + b'\x00'))
+
+    def test_sign_invalid_hash(self):
+        key = CBitcoinSecret('5KJvsngHeMpm884wtkJNzQGaCErckhHJBGFsvd3VyK5qMZXj3hS')
+        with self.assertRaises(TypeError):
+          sig = key.sign('0' * 32)
+
+        hash = b'\x00' * 32
+        with self.assertRaises(ValueError):
+          sig = key.sign(hash[0:-2])