Fix encoding of ECDSA signatures and public keys

evm/ecc/backends/coincurve.py

@@ -1,26 +1,15 @@
 from .base import BaseECCBackend
 
 from evm.utils.ecdsa import (
+    decode_signature,
     encode_signature,
 )
 
-from evm.utils.numeric import (
-    big_endian_to_int,
-    safe_ord
-)
-
-from evm.constants import (
-    NULL_BYTE,
-)
-
 from evm.utils.keccak import (
     keccak,
 )
 
-from evm.utils.secp256k1 import (
-    decode_public_key,
-    encode_raw_public_key,
-)
+from evm.utils.secp256k1 import decode_public_key
 
 
 class CoinCurveECCBackend(BaseECCBackend):
@@ -40,42 +29,20 @@ def ecdsa_sign(self, msg, private_key):
 
     def ecdsa_raw_sign(self, msg_hash, private_key):
         signature = self.keys.PrivateKey(private_key).sign_recoverable(msg_hash, hasher=None)
-        v = safe_ord(signature[64]) + 27
-        r = big_endian_to_int(signature[0:32])
-        s = big_endian_to_int(signature[32:64])
-        return v, r, s
+        return decode_signature(signature)
 
     def ecdsa_verify(self, msg, signature, public_key):
-        signature = signature[1:] + NULL_BYTE
-        signature = self.__recoverable_to_normal(signature)
-        return self.keys.PublicKey(public_key).verify(signature, msg, hasher=keccak)
+        return self.ecdsa_recover(msg, signature) == public_key
 
     def ecdsa_raw_verify(self, msg_hash, vrs, raw_public_key):
-        v, r, s = vrs
-        signature = encode_signature(v, r, s)[1:] + NULL_BYTE
-        signature = self.__recoverable_to_normal(signature)
-        public_key = encode_raw_public_key(raw_public_key)
-        return self.keys.PublicKey(public_key).verify(signature, msg_hash, hasher=None)
+        return self.ecdsa_raw_recover(msg_hash, vrs) == raw_public_key
 
     def ecdsa_recover(self, msg, signature):
-        signature = signature[1:] + NULL_BYTE
-        return self.keys.PublicKey.from_signature_and_message(signature,
-                                                              msg,
-                                                              hasher=keccak
-                                                              ).format(compressed=False)
+        return self.keys.PublicKey.from_signature_and_message(
+            signature, msg, hasher=keccak).format(compressed=False)[1:]
 
     def ecdsa_raw_recover(self, msg_hash, vrs):
-        v, r, s = vrs
-        signature = encode_signature(v, r, s)[1:] + NULL_BYTE
-        raw_public_key = self.keys.PublicKey.from_signature_and_message(signature,
-                                                                        msg_hash,
-                                                                        hasher=None
-                                                                        ).format(compressed=False)
-        return decode_public_key(raw_public_key)
-
-    def __recoverable_to_normal(self, signature):
-        return self.ecdsa.cdata_to_der(
-            self.ecdsa.recoverable_convert(
-                self.ecdsa.deserialize_recoverable(signature)
-            )
-        )
+        signature = encode_signature(*vrs)
+        public_key = self.keys.PublicKey.from_signature_and_message(
+            signature, msg_hash, hasher=None).format(compressed=False)[1:]
+        return decode_public_key(public_key)

evm/utils/address.py

@@ -27,4 +27,8 @@ def private_key_to_address(private_key):
 
 
 def public_key_to_address(public_key):
-    return keccak(public_key[1:])[-20:]
+    if len(public_key) != 64:
+        raise ValueError(
+            "Unexpected public key format: {}. Public keys must be 64 bytes long and must not "
+            "include the fixed \x04 prefix".format(public_key))
+    return keccak(public_key)[-20:]

evm/utils/ecdsa.py

@@ -49,11 +49,11 @@ class BadSignature(ValueError):
 
 
 def encode_signature(v, r, s):
-    vb = int_to_byte(v)
+    vb = int_to_byte(v - 27)
     rb = pad32(int_to_big_endian(r))
     sb = pad32(int_to_big_endian(s))
 
-    return b''.join((vb, rb, sb))
+    return b''.join((rb, sb, vb))
 
 
 def decode_signature(signature):
@@ -64,13 +64,9 @@ def decode_signature(signature):
             "signature must be exactly 65 bytes in length: got {0}".format(len(signature))
         )
 
-    rb = signature[1:33]
-    sb = signature[33:65]
-
-    v = signature[0]
-    r = big_endian_to_int(rb)
-    s = big_endian_to_int(sb)
-
+    r = big_endian_to_int(signature[0:32])
+    s = big_endian_to_int(signature[32:64])
+    v = signature[64] + 27
     return v, r, s
 
 

evm/utils/secp256k1.py

@@ -19,15 +19,18 @@
 
 
 def decode_public_key(public_key):
-    left = big_endian_to_int(public_key[1:33])
-    right = big_endian_to_int(public_key[33:65])
+    if len(public_key) != 64:
+        raise ValueError(
+            "Unexpected public key format: {}. Public keys must be 64 bytes long and must not "
+            "include the fixed \x04 prefix".format(public_key))
+    left = big_endian_to_int(public_key[0:32])
+    right = big_endian_to_int(public_key[32:64])
     return left, right
 
 
 def encode_raw_public_key(raw_public_key):
     left, right = raw_public_key
     return b''.join((
-        b'\x04',
         pad32(int_to_big_endian(left)),
         pad32(int_to_big_endian(right)),
     ))

tests/core/ecdsa-utils/test_ecdsa_utils.py

@@ -1,82 +1,34 @@
-from evm.utils.keccak import (
-    keccak,
-)
+import pytest
+
+from evm.utils.keccak import keccak
 from evm.utils.secp256k1 import (
     decode_public_key,
     encode_raw_public_key,
 )
 from evm.utils.ecdsa import (
     encode_signature,
     decode_signature,
-    ecdsa_sign,
-    ecdsa_raw_sign,
-    ecdsa_verify,
-    ecdsa_raw_verify,
     ecdsa_verify_address,
-    ecdsa_recover,
-    ecdsa_raw_recover,
-)
-
-
-PRIVATE_KEY = (
-    b'E\xa9\x15\xe4\xd0`\x14\x9e\xb46Y`\xe6\xa7\xa4_3C\x93\t0a\x11k\x19~2@\x06_\xf2\xd8'
 )
-PUBLIC_KEY = (
-    b'\x04:QAvFo\xa8\x15\xedH\x1f\xfa\xd0\x91\x10\xa2\xd3D\xf6\xc9\xb7\x8c\x1d\x14\xaf\xc3Q\xc3\xa5\x1b\xe3=\x80r\xe7y9\xdc\x03\xbaDy\x07y\xb7\xa1\x02\x5b\xaf0\x03\xf6s$0\xe2\x0c\xd9\xb7m\x953\x91\xb3'  # noqa: E501
+from tests.ecdsa_fixtures import (
+    MSG,
+    SECRETS,
 )
-RAW_PUBLIC_KEY = decode_public_key(PUBLIC_KEY)
-ADDRESS = (
-    b'\xa9OSt\xfc\xe5\xed\xbc\x8e*\x86\x97\xc1S1g~n\xbf\x0b'
-)
-
-MSG = b'my message'
-MSG_HASH = b'#tpO\xbbmDaqK\xcb\xab\xebj\x16\x0c"E\x9ex\x1b\x08\\\x83lI\x08JG\x0e\xd6\xa4'
-
-V = 27
-R = 54060028713369731575288880898058519584012347418583874062392262086259746767623
-S = 41474707565615897636207177895621376369577110960831782659442889110043833138559
-
-
-assert keccak(MSG) == MSG_HASH
-
-
-assert encode_raw_public_key(decode_public_key(PUBLIC_KEY)) == PUBLIC_KEY
-
-
-def test_raw_signing():
-    v, r, s = ecdsa_raw_sign(MSG_HASH, PRIVATE_KEY)
-    assert ecdsa_raw_verify(MSG_HASH, (v, r, s), RAW_PUBLIC_KEY)
-
-
-def test_raw_recover():
-    raw_public_key = ecdsa_raw_recover(MSG_HASH, (V, R, S))
-    recovered_public_key = encode_raw_public_key(raw_public_key)
-    assert recovered_public_key == PUBLIC_KEY
-
-
-def test_raw_verify():
-    assert ecdsa_raw_verify(MSG_HASH, (V, R, S), RAW_PUBLIC_KEY)
-
-
-def test_signature_encoding_and_decoding():
-    signature = encode_signature(V, R, S)
-    v, r, s, = decode_signature(signature)
-    assert v == V
-    assert r == R
-    assert s == S
 
 
-def test_signing_and_verifying_with_public_key():
-    signature = ecdsa_sign(MSG, PRIVATE_KEY)
-    assert ecdsa_verify(MSG, signature, PUBLIC_KEY)
+@pytest.mark.parametrize("label, d", sorted(SECRETS.items()))
+def test_encode_decode_raw_public_key(label, d):
+    assert encode_raw_public_key(decode_public_key(d['pubkey'])) == d['pubkey']
 
 
-def test_signing_and_verifying_with_address():
-    signature = ecdsa_sign(MSG, PRIVATE_KEY)
-    assert ecdsa_verify_address(MSG, signature, ADDRESS)
+@pytest.mark.parametrize("label, d", sorted(SECRETS.items()))
+def test_signature_encoding_and_decoding(label, d):
+    v, r, s, = decode_signature(d['sig'])
+    assert (v, r, s) == d['raw_sig']
+    assert encode_signature(v, r, s) == d['sig']
 
 
-def test_recovering_public_key():
-    signature = ecdsa_sign(MSG, PRIVATE_KEY)
-    recovered_public_key = ecdsa_recover(MSG, signature)
-    assert recovered_public_key == PUBLIC_KEY
+@pytest.mark.parametrize("label, d", sorted(SECRETS.items()))
+def test_verify_address(label, d):
+    addr = keccak(d['pubkey'])[-20:]
+    assert ecdsa_verify_address(MSG, d['sig'], addr)

tests/core/secp256k1-utils/test_public_key_conversion.py

@@ -4,16 +4,10 @@
     private_key_to_public_key,
 )
 
+from tests.ecdsa_fixtures import SECRETS
 
-@pytest.mark.parametrize(
-    'private_key,expected',
-    (
-        (
-            b"\xe9\x87=y\xc6\xd8}\xc0\xfbjWxc3\x89\xf4E2\x130=\xa6\x1f \xbdg\xfc#:\xa32b",
-            b"\x04X\x8d *\xfc\xc1\xeeJ\xb5%LxG\xec%\xb9\xa15\xbb\xda\x0f+\xc6\x9e\xe1\xa7\x14t\x9f\xd7}\xc9\xf8\x8f\xf2\xa0\r~u-D\xcb\xe1n\x1e\xbc\xf0\x89\x0bv\xec|x\x88a\t\xde\xe7l\xcf\xc8DT$",  # noqa: E501
-        ),
-    ),
-)
-def test_private_key_to_public_key(private_key, expected):
-    actual = private_key_to_public_key(private_key)
-    assert actual == expected
+
+@pytest.mark.parametrize('label, d', sorted(SECRETS.items()))
+def test_private_key_to_public_key(label, d):
+    actual = private_key_to_public_key(d['privkey'])
+    assert actual == d['pubkey']