Merge pull request #9070 from SomberNight/202405_ecc_ecdsa_low_s

ecc: ecdsa_verify to enforce low-S rule

Author: SomberNight

electrum/ecc.py

@@ -342,7 +342,13 @@ def ecdsa_verify_recoverable(self, sig65: bytes, msg32: bytes) -> bool:
         # check message
         return self.ecdsa_verify(sig65[1:], msg32)
 
-    def ecdsa_verify(self, sig64: bytes, msg32: bytes) -> bool:
+    def ecdsa_verify(
+        self,
+        sig64: bytes,
+        msg32: bytes,
+        *,
+        enforce_low_s: bool = True,  # policy/standardness rule
+    ) -> bool:
         assert_bytes(sig64)
         if len(sig64) != 64:
             return False
@@ -353,7 +359,8 @@ def ecdsa_verify(self, sig64: bytes, msg32: bytes) -> bool:
         ret = _libsecp256k1.secp256k1_ecdsa_signature_parse_compact(_libsecp256k1.ctx, sig, sig64)
         if 1 != ret:
             return False
-        ret = _libsecp256k1.secp256k1_ecdsa_signature_normalize(_libsecp256k1.ctx, sig, sig)
+        if not enforce_low_s:
+            ret = _libsecp256k1.secp256k1_ecdsa_signature_normalize(_libsecp256k1.ctx, sig, sig)
 
         pubkey = self._to_libsecp256k1_pubkey_ptr()
         if 1 != _libsecp256k1.secp256k1_ecdsa_verify(_libsecp256k1.ctx, sig, msg32, pubkey):
@@ -439,7 +446,8 @@ def verify_usermessage_with_address(address: str, sig65: bytes, message: bytes,
     else:
         return False
     # check message
-    return public_key.ecdsa_verify(sig65[1:], h)
+    # note: `$ bitcoin-cli verifymessage` does NOT enforce the low-S rule for ecdsa sigs
+    return public_key.ecdsa_verify(sig65[1:], h, enforce_low_s=False)
 
 
 def is_secret_within_curve_range(secret: Union[int, bytes]) -> bool:
@@ -566,6 +574,8 @@ def schnorr_sign(self, msg32: bytes, *, aux_rand32: bytes = None) -> bytes:
         return sig64
 
     def ecdsa_sign_recoverable(self, msg32: bytes, *, is_compressed: bool) -> bytes:
+        assert len(msg32) == 32, len(msg32)
+
         def bruteforce_recid(sig64: bytes):
             for recid in range(4):
                 sig65 = construct_ecdsa_sig65(sig64, recid, is_compressed=is_compressed)

tests/test_bitcoin.py

@@ -246,6 +246,15 @@ def test_signmessage_legacy_address(self):
         self.assertFalse(ecc.verify_usermessage_with_address(addr1, b'wrong', msg1))
         self.assertFalse(ecc.verify_usermessage_with_address(addr1, sig2, msg1))
 
+    def test_signmessage_low_s(self):
+        """`$ bitcoin-cli verifymessage` does NOT enforce the low-S rule for ecdsa sigs. This tests we do the same."""
+        addr = "15hETetDmcXm1mM4sEf7U2KXC9hDHFMSzz"
+        sig_low_s = b'Hzsu0U/THAsPz/MSuXGBKSULz2dTfmrg1NsAhFp+wH5aKfmX4Db7ExLGa7FGn0m6Mf43KsbEOWpvUUUBTM3Uusw='
+        sig_high_s = b'IDsu0U/THAsPz/MSuXGBKSULz2dTfmrg1NsAhFp+wH5a1gZoH8kE7O05lE65YLZFzLx3sh/rDzXMbo1dQAJhhnU='
+        msg = b'Chancellor on brink of second bailout for banks'
+        self.assertTrue(ecc.verify_usermessage_with_address(address=addr, sig65=base64.b64decode(sig_low_s), message=msg))
+        self.assertTrue(ecc.verify_usermessage_with_address(address=addr, sig65=base64.b64decode(sig_high_s), message=msg))
+
     def test_signmessage_segwit_witness_v0_address(self):
         msg = b'Electrum'
         # p2wpkh-p2sh

tests/test_ecc.py

@@ -116,3 +116,27 @@ def bip340_tagged_hash__from_libsecp(tag: bytes, msg: bytes) -> bytes:
         for tag, msg in data:
             self.assertEqual(bip340_tagged_hash__from_libsecp(tag, msg),
                              ecc.bip340_tagged_hash(tag, msg))
+
+
+class TestEcdsa(ElectrumTestCase):
+
+    def test_verify_enforces_low_s(self):
+        # privkey = ecc.ECPrivkey(bytes.fromhex("d473e2ec218dca8e3508798f01cdfde0135fc79d95526b12e3537fe57e479ac1"))
+        # r, low_s = privkey.ecdsa_sign(msg32, sigencode=lambda x, y: (x,y))
+        # pubkey = ecc.ECPubkey(privkey.get_public_key_bytes())
+        pubkey = ecc.ECPubkey(bytes.fromhex("03befe4f7c92eaed73fb8eddac28c6191c87c6a3546bf8dc09643e1e10bc6f5ab0"))
+        msg32 = sha256("hello there")
+        r = 29658118546717807188148256874354333643324863178937517286987684851194094232509
+        # low-S
+        low_s = 9695211969150896589566136599751503273246834163278279637071703776634378000266
+        sig64_low_s = (
+            int.to_bytes(r, length=32, byteorder="big") +
+            int.to_bytes(low_s, length=32, byteorder="big"))
+        self.assertTrue(pubkey.ecdsa_verify(sig64_low_s, msg32))
+        # high-S
+        high_s = ecc.CURVE_ORDER - low_s
+        sig64_high_s = (
+            int.to_bytes(r, length=32, byteorder="big") +
+            int.to_bytes(high_s, length=32, byteorder="big"))
+        self.assertFalse(pubkey.ecdsa_verify(sig64_high_s, msg32))
+        self.assertTrue(pubkey.ecdsa_verify(sig64_high_s, msg32, enforce_low_s=False))