From 07c80d2ca14b54f2c41c81fbcbaa8b7e4b3e42ef Mon Sep 17 00:00:00 2001 From: SomberNight Date: Mon, 27 May 2024 17:12:33 +0000 Subject: [PATCH] ecc: ecdsa_verify to enforce low-S rule The low-S rule for ecdsa signatures is mandated by Bitcoin Core policy/standardness (though not by consensus). If we get signatures from untrusted sources, we should mandate they obey the policy rules. (e.g. from LN peers) Note that we normalize the signatures in the sig format conversion methods (DER <-> (r,s) <-> sig64). The BOLTs treat high-S signatures as invalid, and this changes our behaviour to that. (previously we would silently normalize the S value) see https://github.com/bitcoin/bitcoin/pull/6769 see https://github.com/lightning/bolts/pull/807 --- electrum/ecc.py | 14 +++++++++++--- tests/test_ecc.py | 24 ++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/electrum/ecc.py b/electrum/ecc.py index 882a31f9a..4c86cfd8d 100644 --- a/electrum/ecc.py +++ b/electrum/ecc.py @@ -342,7 +342,13 @@ class ECPubkey(object): # 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 @@ class ECPubkey(object): 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: diff --git a/tests/test_ecc.py b/tests/test_ecc.py index 8197a8449..d9f0c129d 100644 --- a/tests/test_ecc.py +++ b/tests/test_ecc.py @@ -116,3 +116,27 @@ class TestSchnorr(ElectrumTestCase): 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))