Fix coincurve backend signature validation

eth_keys/backends/coincurve.py

@@ -22,6 +22,9 @@
 from eth_keys.utils import (
     der,
 )
+from eth_keys.utils.numeric import (
+    coerce_low_s,
+)
 
 from .base import BaseECCBackend
 
@@ -75,7 +78,9 @@ def ecdsa_verify(self,
                      msg_hash: bytes,
                      signature: BaseSignature,
                      public_key: PublicKey) -> bool:
-        der_encoded_signature = der.two_int_sequence_encoder(signature.r, signature.s)
+        # coincurve rejects signatures with a high s, so convert to the equivalent low s form
+        low_s = coerce_low_s(signature.s)
+        der_encoded_signature = der.two_int_sequence_encoder(signature.r, low_s)
         coincurve_public_key = self.keys.PublicKey(b"\x04" + public_key.to_bytes())
         return coincurve_public_key.verify(
             der_encoded_signature,

eth_keys/utils/numeric.py

@@ -1,2 +1,16 @@
+from eth_keys.constants import (
+    SECPK1_N,
+)
+
+
 def int_to_byte(value: int) -> bytes:
     return bytes([value])
+
+
+def coerce_low_s(value: int) -> int:
+    """Coerce the s component of an ECDSA signature into its low-s form.
+
+    See https://bitcoin.stackexchange.com/questions/83408/in-ecdsa-why-is-r-%E2%88%92s-mod-n-complementary-to-r-s  # noqa: W501
+    or https://github.com/ethereum/EIPs/blob/master/EIPS/eip-2.md.
+    """
+    return min(value, -value % SECPK1_N)

tests/backends/test_backends.py

@@ -9,13 +9,20 @@
 from eth_keys import KeyAPI
 from eth_keys.backends import CoinCurveECCBackend
 from eth_keys.backends import NativeECCBackend
+from eth_keys.constants import (
+    SECPK1_N,
+)
+from eth_keys.utils.numeric import (
+    coerce_low_s,
+)
 
 from eth_utils import (
     keccak,
 )
 
 from strategies import (
     private_key_st,
+    message_hash_st,
 )
 
 
@@ -54,6 +61,9 @@ def test_ecdsa_sign(key_api, key_fixture):
 def test_ecdsa_sign_non_recoverable(key_api, key_fixture):
     private_key = key_api.PrivateKey(key_fixture['privkey'])
     signature = key_api.ecdsa_sign_non_recoverable(MSGHASH, private_key)
+    non_recoverable_signature = key_api.ecdsa_sign_non_recoverable(MSGHASH, private_key)
+    assert non_recoverable_signature.r == signature.r
+    assert non_recoverable_signature.s == signature.s
 
     assert key_api.ecdsa_verify(MSGHASH, signature, private_key.public_key)
 
@@ -98,3 +108,17 @@ def test_compress_decompress_inversion(key_api, private_key_bytes):
     compressed_bytes = original.to_compressed_bytes()
     decompressed = key_api.PublicKey.from_compressed_bytes(compressed_bytes)
     assert decompressed == original
+
+
+@given(
+    private_key_bytes=private_key_st,
+    message_hash=message_hash_st,
+)
+def test_signatures_with_high_s(key_api, private_key_bytes, message_hash):
+    private_key = key_api.PrivateKey(private_key_bytes)
+    low_s_signature = private_key.sign_msg_hash(message_hash)
+    assert coerce_low_s(low_s_signature.s) == low_s_signature.s
+    high_s = -low_s_signature.s % SECPK1_N
+    assert coerce_low_s(high_s) == low_s_signature.s
+    high_s_signature = key_api.Signature(vrs=(low_s_signature.v, low_s_signature.r, high_s))
+    assert key_api.ecdsa_verify(message_hash, high_s_signature, private_key.public_key)

tests/backends/test_native_backend_against_coincurve.py

@@ -104,11 +104,13 @@ def test_native_to_coincurve_recover(private_key_bytes,
     else:
         assert False, "invariant"
 
-    pk_a = backend_a.PrivateKey(private_key_bytes)
-    signature_a = backend_a.ecdsa_sign(message_hash, pk_a)
+    private_key_a = backend_a.PrivateKey(private_key_bytes)
+    public_key_a = private_key_a.public_key
+    signature_a = backend_a.ecdsa_sign(message_hash, private_key_a)
 
     public_key_b = backend_b.ecdsa_recover(message_hash, signature_a)
-    assert public_key_b == pk_a.public_key
+    assert public_key_b == public_key_a
+    assert backend_b.ecdsa_verify(message_hash, signature_a, public_key_b)
 
 
 @given(