Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions src/embit/liquid/descriptor.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,20 +225,23 @@ def sec(self):

def musig_combine_privs(privs, sort=True):
keys = [
[b"" + prv, secp256k1.ec_pubkey_serialize(secp256k1.ec_pubkey_create(prv))]
[
bytearray(prv),
secp256k1.ec_pubkey_serialize(secp256k1.ec_pubkey_create(prv)),
]
for prv in privs
]
for karr in keys:
if karr[1][0] == 0x03:
secp256k1.ec_privkey_negate(karr[0])
karr[0] = bytearray(secp256k1.ec_privkey_negate(karr[0]))
# x only
karr[1] = karr[1][1:]
if sort:
keys = list(sorted(keys, key=lambda k: k[1]))
secs = [k[1] for k in keys]
ll = sha256(b"".join(secs))
coefs = [
tagged_hash("MuSig coefficient", ll + i.to_bytes(4, "little"))
bytearray(tagged_hash("MuSig coefficient", ll + i.to_bytes(4, "little")))
for i in range(len(keys))
]
# tweak them all
Expand All @@ -249,15 +252,15 @@ def musig_combine_privs(privs, sort=True):
s = secp256k1.ec_privkey_add(s, c)
pub = secp256k1.ec_pubkey_create(s)
if secp256k1.ec_pubkey_serialize(pub)[0] == 0x03:
secp256k1.ec_privkey_negate(s)
s = secp256k1.ec_privkey_negate(s)
return s


def musig_combine_pubs(pubs, sort=True):
keys = [[pub, secp256k1.ec_pubkey_serialize(pub)] for pub in pubs]
keys = [[bytearray(pub), secp256k1.ec_pubkey_serialize(pub)] for pub in pubs]
for karr in keys:
if karr[1][0] == 0x03:
secp256k1.ec_pubkey_negate(karr[0])
karr[0] = bytearray(secp256k1.ec_pubkey_negate(karr[0]))
# x only
karr[1] = karr[1][1:]
if sort:
Expand Down
2 changes: 1 addition & 1 deletion src/embit/liquid/pset.py
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ def reblind(self, nonce, blinding_pubkey=None, extra_message=b""):
blinding_pubkey = blinding_pubkey or self.blinding_pubkey
if not blinding_pubkey:
raise PSBTError("Blinding pubkey required")
pub = secp256k1.ec_pubkey_parse(blinding_pubkey)
pub = bytearray(secp256k1.ec_pubkey_parse(blinding_pubkey))
self.ecdh_pubkey = ec.PrivateKey(nonce).sec()
secp256k1.ec_pubkey_tweak_mul(pub, nonce)
sec = secp256k1.ec_pubkey_serialize(pub)
Expand Down
2 changes: 1 addition & 1 deletion src/embit/liquid/transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def unblind(
raise TransactionError("Invalid value commitment length")
if len(asset_commitment) != 33:
raise TransactionError("Invalid asset commitment length")
pub = secp256k1.ec_pubkey_parse(pubkey)
pub = bytearray(secp256k1.ec_pubkey_parse(pubkey))
secp256k1.ec_pubkey_tweak_mul(pub, blinding_key)
sec = secp256k1.ec_pubkey_serialize(pub)
nonce = hashlib.sha256(hashlib.sha256(sec).digest()).digest()
Expand Down
63 changes: 50 additions & 13 deletions src/embit/util/ctypes_secp256k1.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,19 @@ def _copy(a: bytes) -> bytes:
"""Ugly copy that works everywhere incl micropython"""
if len(a) == 0:
return b""
return a[:1] + a[1:]
return bytes(a[:1] + a[1:])


def _readable_buffer(a):
if isinstance(a, bytearray):
return bytes(a)
return a


def _writable_buffer(a):
if isinstance(a, bytearray):
return (c_char * len(a)).from_buffer(a)
return a


def _platform_binary_ext():
Expand Down Expand Up @@ -562,7 +574,7 @@ def _init(flags=(CONTEXT_SIGN | CONTEXT_VERIFY)):
def context_randomize(seed, context=_secp.ctx):
if len(seed) != 32:
raise ValueError("Seed should be 32 bytes long")
if _secp.secp256k1_context_randomize(context, seed) == 0:
if _secp.secp256k1_context_randomize(context, _readable_buffer(seed)) == 0:
raise RuntimeError("Failed to randomize context")


Expand All @@ -571,7 +583,7 @@ def ec_pubkey_create(secret, context=_secp.ctx):
if len(secret) != 32:
raise ValueError("Private key should be 32 bytes long")
pub = bytes(64)
r = _secp.secp256k1_ec_pubkey_create(context, pub, secret)
r = _secp.secp256k1_ec_pubkey_create(context, pub, _readable_buffer(secret))
if r == 0:
raise ValueError("Invalid private key")
return pub
Expand All @@ -588,7 +600,9 @@ def ec_pubkey_parse(sec, context=_secp.ctx):
if sec[0] != 0x04:
raise ValueError("Uncompressed pubkey should start with 0x04")
pub = bytes(64)
r = _secp.secp256k1_ec_pubkey_parse(context, pub, sec, len(sec))
r = _secp.secp256k1_ec_pubkey_parse(
context, pub, _readable_buffer(sec), len(sec)
)
if r == 0:
raise ValueError("Failed parsing public key")
return pub
Expand All @@ -602,7 +616,9 @@ def ec_pubkey_serialize(pubkey, flag=EC_COMPRESSED, context=_secp.ctx):
raise ValueError("Invalid flag")
sec = bytes(33) if (flag == EC_COMPRESSED) else bytes(65)
sz = c_size_t(len(sec))
r = _secp.secp256k1_ec_pubkey_serialize(context, sec, byref(sz), pubkey, flag)
r = _secp.secp256k1_ec_pubkey_serialize(
context, sec, byref(sz), _readable_buffer(pubkey), flag
)
if r == 0:
raise ValueError("Failed to serialize pubkey")
return sec
Expand Down Expand Up @@ -693,7 +709,7 @@ def ecdsa_sign(msg, secret, nonce_function=None, extra_data=None, context=_secp.
def ec_seckey_verify(secret, context=_secp.ctx):
if len(secret) != 32:
raise ValueError("Secret should be 32 bytes long")
return bool(_secp.secp256k1_ec_seckey_verify(context, secret))
return bool(_secp.secp256k1_ec_seckey_verify(context, _readable_buffer(secret)))


@locked
Expand All @@ -720,7 +736,12 @@ def ec_pubkey_negate(pubkey, context=_secp.ctx):
def ec_privkey_tweak_add(secret, tweak, context=_secp.ctx):
if len(secret) != 32 or len(tweak) != 32:
raise ValueError("Secret and tweak should both be 32 bytes long")
if _secp.secp256k1_ec_privkey_tweak_add(context, secret, tweak) == 0:
if (
_secp.secp256k1_ec_privkey_tweak_add(
context, _writable_buffer(secret), _readable_buffer(tweak)
)
== 0
):
raise ValueError("Failed to tweak the secret")
return None

Expand All @@ -731,7 +752,12 @@ def ec_pubkey_tweak_add(pub, tweak, context=_secp.ctx):
raise ValueError("Public key should be 64 bytes long")
if len(tweak) != 32:
raise ValueError("Tweak should be 32 bytes long")
if _secp.secp256k1_ec_pubkey_tweak_add(context, pub, tweak) == 0:
if (
_secp.secp256k1_ec_pubkey_tweak_add(
context, _writable_buffer(pub), _readable_buffer(tweak)
)
== 0
):
raise ValueError("Failed to tweak the public key")
return None

Expand All @@ -755,7 +781,7 @@ def ec_pubkey_add(pub, tweak, context=_secp.ctx):
if len(tweak) != 32:
raise ValueError("Tweak should be 32 bytes long")
p = _copy(pub)
if _secp.secp256k1_ec_pubkey_tweak_add(context, p, tweak) == 0:
if _secp.secp256k1_ec_pubkey_tweak_add(context, p, _readable_buffer(tweak)) == 0:
raise ValueError("Failed to tweak the public key")
return p

Expand All @@ -764,7 +790,12 @@ def ec_pubkey_add(pub, tweak, context=_secp.ctx):
def ec_privkey_tweak_mul(secret, tweak, context=_secp.ctx):
if len(secret) != 32 or len(tweak) != 32:
raise ValueError("Secret and tweak should both be 32 bytes long")
if _secp.secp256k1_ec_privkey_tweak_mul(context, secret, tweak) == 0:
if (
_secp.secp256k1_ec_privkey_tweak_mul(
context, _writable_buffer(secret), _readable_buffer(tweak)
)
== 0
):
raise ValueError("Failed to tweak the secret")


Expand All @@ -774,15 +805,21 @@ def ec_pubkey_tweak_mul(pub, tweak, context=_secp.ctx):
raise ValueError("Public key should be 64 bytes long")
if len(tweak) != 32:
raise ValueError("Tweak should be 32 bytes long")
if _secp.secp256k1_ec_pubkey_tweak_mul(context, pub, tweak) == 0:
if (
_secp.secp256k1_ec_pubkey_tweak_mul(
context, _writable_buffer(pub), _readable_buffer(tweak)
)
== 0
):
raise ValueError("Failed to tweak the public key")


@locked
def ec_pubkey_combine(*args, context=_secp.ctx):
pub = bytes(64)
pubkeys = (c_char_p * len(args))(*args)
r = _secp.secp256k1_ec_pubkey_combine(context, pub, pubkeys, len(args))
readable_args = [_readable_buffer(arg) for arg in args]
pubkeys = (c_char_p * len(readable_args))(*readable_args)
r = _secp.secp256k1_ec_pubkey_combine(context, pub, pubkeys, len(readable_args))
if r == 0:
raise ValueError("Failed to combine pubkeys")
return pub
Expand Down
98 changes: 69 additions & 29 deletions src/embit/util/py_secp256k1.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,18 +205,52 @@ def ec_pubkey_negate(pubkey, context=None):
return ec_pubkey_parse(bytes([0x05 - sec[0]]) + sec[1:])


def ec_privkey_tweak_add(secret, tweak, context=None):
def _ensure_mutable_key(key):
if not isinstance(key, bytearray):
raise TypeError("Tweaked key must be a bytearray")


def ec_privkey_tweak_add(
secret: bytearray, tweak: bytes | bytearray, context: object | None = None
) -> None:
"""Add tweak to secret in place."""
_ensure_mutable_key(secret)
res = ec_privkey_add(secret, tweak)
for i in range(len(secret)):
secret[i] = res[i]


def ec_pubkey_tweak_add(pub, tweak, context=None):
def ec_pubkey_tweak_add(
pub: bytearray, tweak: bytes | bytearray, context: object | None = None
) -> None:
"""Add tweak to public key in place."""
_ensure_mutable_key(pub)
res = ec_pubkey_add(pub, tweak)
for i in range(len(pub)):
pub[i] = res[i]


def ec_privkey_tweak_mul(
secret: bytearray, tweak: bytes | bytearray, context: object | None = None
) -> None:
"""Multiply secret by tweak in place."""
_ensure_mutable_key(secret)
if len(secret) != 32 or len(tweak) != 32:
raise ValueError("Secret and tweak should both be 32 bytes long")
s = int.from_bytes(secret, "big")
t = int.from_bytes(tweak, "big")
if (
s == 0
or s >= _key.SECP256K1_ORDER
or t == 0
or t >= _key.SECP256K1_ORDER
):
raise ValueError("Failed to tweak the secret")
res = ((s * t) % _key.SECP256K1_ORDER).to_bytes(32, "big")
for i in range(len(secret)):
secret[i] = res[i]


def ec_privkey_add(secret, tweak, context=None):
if len(secret) != 32 or len(tweak) != 32:
raise ValueError("Secret and tweak should both be 32 bytes long")
Expand All @@ -242,33 +276,39 @@ def ec_pubkey_add(pub, tweak, context=None):
return Q[0].to_bytes(32, "little") + Q[1].to_bytes(32, "little")


# def ec_privkey_tweak_mul(secret, tweak, context=None):
# if len(secret)!=32 or len(tweak)!=32:
# raise ValueError("Secret and tweak should both be 32 bytes long")
# s = int.from_bytes(secret, 'big')
# t = int.from_bytes(tweak, 'big')
# if t > _key.SECP256K1_ORDER or s > _key.SECP256K1_ORDER:
# raise ValueError("Failed to tweak the secret")
# r = pow(s, t, _key.SECP256K1_ORDER)
# res = r.to_bytes(32, 'big')
# for i in range(len(secret)):
# secret[i] = res[i]

# def ec_pubkey_tweak_mul(pub, tweak, context=None):
# if len(pub)!=64:
# raise ValueError("Public key should be 64 bytes long")
# if len(tweak)!=32:
# raise ValueError("Tweak should be 32 bytes long")
# if _secp.secp256k1_ec_pubkey_tweak_mul(context, pub, tweak) == 0:
# raise ValueError("Failed to tweak the public key")

# def ec_pubkey_combine(*args, context=None):
# pub = bytes(64)
# pubkeys = (c_char_p * len(args))(*args)
# r = _secp.secp256k1_ec_pubkey_combine(context, pub, pubkeys, len(args))
# if r == 0:
# raise ValueError("Failed to negate pubkey")
# return pub
def ec_pubkey_tweak_mul(
pub: bytearray, tweak: bytes | bytearray, context: object | None = None
) -> None:
"""Multiply public key by tweak in place."""
_ensure_mutable_key(pub)
if len(pub) != 64:
raise ValueError("Public key should be 64 bytes long")
if len(tweak) != 32:
raise ValueError("Tweak should be 32 bytes long")
t = int.from_bytes(tweak, "big")
if t == 0 or t >= _key.SECP256K1_ORDER:
raise ValueError("Failed to tweak the public key")
pubkey = _pubkey_parse(pub)
Q = _key.SECP256K1.affine(_key.SECP256K1.mul([(pubkey.p, t)]))
if Q is None:
raise ValueError("Failed to tweak the public key")
res = Q[0].to_bytes(32, "little") + Q[1].to_bytes(32, "little")
for i in range(len(pub)):
pub[i] = res[i]
Comment on lines +296 to +297

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qlrd, I think this makes sense.

According to PEP 3137:

- bytes is an immutable array of bytes (PyString)
- bytearray is a mutable array of bytes (PyBytes)
- memoryview is a bytes view on another object (PyMemory)

So at runtime this loop may raise a TypeError:

for i in range(len(pub)):
    pub[i] = res[i]

because pub may be a bytes object, which is immutable. In that case the assignment would fail with something like:

TypeError: 'bytes' object does not support item assignment

Looking at the suggested code paths, ec_pubkey_parse() returns a bytes object:

pub = bytes(64)
r = _secp.secp256k1_ec_pubkey_parse(context, pub, sec, len(sec))
if r == 0:
raise ValueError("Failed parsing public key")
return pub

and that value is passed directly to ec_pubkey_tweak_mul():

pub = secp256k1.ec_pubkey_parse(pubkey)
secp256k1.ec_pubkey_tweak_mul(pub, blinding_key)

So this will fail because we are trying to mutate an immutable bytes object.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lucas, thank you for your analysis and explanations. I addressed this issue in my commit 7673240. Your feedback is very welcome.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@miketlk, seems like a nice patch adding _ensure_mutable_key() helper that raises TypeError.

Is way batter then silently just doing pub = bytearray(pub) inside of ec_pubkey_tweak_mul

I also cloned the repo and executed the tests locally and everything seems to be working properly.



def ec_pubkey_combine(*args, context=None):
if len(args) == 0:
raise ValueError("At least one pubkey is required")
acc = (0, 1, 0)
for pub in args:
if len(pub) != 64:
raise ValueError("Public key should be 64 bytes long")
acc = _key.SECP256K1.add(acc, _pubkey_parse(pub).p)
Q = _key.SECP256K1.affine(acc)
if Q is None:
raise ValueError("Failed to combine pubkeys")
return Q[0].to_bytes(32, "little") + Q[1].to_bytes(32, "little")

# schnorrsig

Expand Down
Loading
Loading