bugfix: PSBT global XPUBs validation

This commit is contained in:
scgbckbone 2025-12-27 02:40:15 +01:00 committed by doc-hex
parent 413a91cf39
commit 9e0e187e49
5 changed files with 153 additions and 9 deletions

View File

@ -16,6 +16,7 @@ This lists the changes in the most recent EDGE firmware, for each hardware platf
- New Feature: Support for v3 transactions
- New Feature: Send keystrokes with all derived BIP-85 secrets
- Enhancement: CCC allow to reset block height
- Bugfix: PSBT global XPUBs validation when signing with specific wallet
# Mk4 Specific Changes

View File

@ -1412,8 +1412,6 @@ class psbtObject(psbtProxy):
# Lookup correct wallet based on xpubs in globals
# - only happens if they volunteered this 'extra' data
# - do not assume multisig
assert not self.active_miniscript
has_mine = 0
parsed_xpubs = []
for k,v in self.xpubs:
@ -1428,6 +1426,13 @@ class psbtObject(psbtProxy):
if not has_mine:
raise FatalPSBTIssue('My XFP not involved')
if self.active_miniscript:
# user is going via wallet->Sign PSBT
# check XPUBs are correct
if not self.active_miniscript.disable_checks:
self.active_miniscript.validate_psbt_xpubs(parsed_xpubs)
return
# don't want to guess M if not needed, but we need it
af, M, N = self.guess_M_of_N()
if not N:
@ -1452,11 +1457,6 @@ class psbtObject(psbtProxy):
if wal:
# exact match (by xfp+deriv set) .. normal case
self.active_miniscript = wal
# now proper check should follow - matching actual master pubkeys
# but is it needed?, we just matched the wallet
# and are going to use our own data for verification anyway
if not self.active_miniscript.disable_checks:
self.active_miniscript.validate_psbt_xpubs(parsed_xpubs)
else:
trust_mode = MiniScriptWallet.get_trust_policy()

View File

@ -582,14 +582,16 @@ class MiniScriptWallet(WalletABC):
return cls.from_descriptor_obj(name, desc_obj)
def validate_psbt_xpubs(self, psbt_xpubs):
# validate via set equality on string representation of the key(s)
# using __hash__ of the key object ignores origin derivation
keys = set()
for ek, xfp_pth in psbt_xpubs:
key = Key.from_psbt_xpub(ek, xfp_pth)
key.validate(settings.get('xfp', 0), self.disable_checks)
keys.add(key)
keys.add(key.to_string(external=False, internal=False))
if not self.disable_checks:
assert set(self.to_descriptor().keys) == keys
assert set(self.keys_info) == keys, "PSBT xpubs mismatch"
def ux_unique_name_msg(self, name=None):
return ("%s wallet with name '%s' already exists. All wallets MUST"

View File

@ -9,6 +9,7 @@ from psbt import BasicPSBT
from charcodes import KEY_QR, KEY_RIGHT, KEY_CANCEL, KEY_DELETE
from bbqr import split_qrs
from bip32 import BIP32Node
from helpers import str_to_path
H = "50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0" # BIP-0341
@ -3164,6 +3165,93 @@ def test_same_key_set_miniscript(get_cc_key, bitcoin_core_signer, create_core_wa
assert title == 'PSBT Signed'
@pytest.mark.bitcoind
@pytest.mark.parametrize("orig_der", [False, True])
def test_specific_wallet_signing_xpubs(orig_der, get_cc_key, bitcoin_core_signer, create_core_wallet,
offer_minsc_import, press_select, bitcoind, start_sign,
cap_story, end_sign, clear_miniscript, goto_home):
goto_home()
clear_miniscript()
msc = "wsh(or_d(pk(@D),and_v(v:multi(2,@A,@B,@C),older(65535))))"
ak = get_cc_key("m/48h/1h/0h/2h")
bs, bk = bitcoin_core_signer("bb")
cs, ck = bitcoin_core_signer("cc")
ds, dk = bitcoin_core_signer("dd")
bk = bk.replace("/0/*", "/<0;1>/*")
ck = ck.replace("/0/*", "/<0;1>/*")
dk = dk.replace("/0/*", "/<0;1>/*")
if not orig_der:
bk = bk.split("]")[-1]
ck = ck.split("]")[-1]
dk = dk.split("]")[-1]
msc = msc.replace("@A", ak)
msc = msc.replace("@B", bk)
msc = msc.replace("@C", ck)
msc = msc.replace("@D", dk)
title, story = offer_minsc_import(json.dumps(dict(name="msc", desc=msc)))
assert "msc" in story
assert "Create new miniscript wallet?" in story
press_select()
wo = create_core_wallet("msc", "bech32")
psbt = wo.walletcreatefundedpsbt([], [{bitcoind.supply_wallet.getnewaddress(): 1.0}],
0, {"fee_rate": 2})["psbt"]
po = BasicPSBT().parse(base64.b64decode(psbt))
for ke in [bk, ck, dk, ak]:
if "]" in ke:
a, b = ke.split("]")
ext_key = b.split("/")[0]
der = a[1:]
xfp_str = der.split("/")[0]
der = der.replace(xfp_str, "m")
path_lst = str_to_path(der)
n = BIP32Node.from_wallet_key(ext_key)
po.xpubs.append(
(
n.node.serialize_public(),
bytes.fromhex(xfp_str) + struct.pack(f'<{"I"*len(path_lst)}', *path_lst)
)
)
else:
ext_key = ke.split("/")[0]
n = BIP32Node.from_wallet_key(ext_key)
po.xpubs.append((n.node.serialize_public(), n.fingerprint()))
# success case
start_sign(po.as_bytes(), miniscript="msc")
end_sign(accept=True)
item = po.xpubs[0]
# wrong key
key_wrong = item[0][:-1] + b"\x10"
po.xpubs[0] = (key_wrong, item[1])
start_sign(po.as_bytes(), miniscript="msc")
title, story = cap_story()
assert "Failure" in title
if orig_der:
assert "PSBT xpubs mismatch" in story
if orig_der:
# wrong derivation path
# do not check if we only have xfp as derivation, because blinded keys allowed
pth_wrong = item[1][:-1] + b"\x10"
po.xpubs[0] = (item[0], pth_wrong)
start_sign(po.as_bytes(), miniscript="msc")
title, story = cap_story()
assert "Failure" in title
assert "PSBT xpubs mismatch" in story
@pytest.mark.parametrize("desc", CHANGE_BASED_DESCS)
@pytest.mark.parametrize("way", ["usb", "sd", "vdisk", "nfc", "qr"])
def test_bip388_policies(desc, way, offer_minsc_import, press_select, pick_menu_item, goto_home,

View File

@ -3481,4 +3481,57 @@ def test_af_matching_convoluted_case(af, psbt_v2, clear_miniscript, fake_ms_txn,
assert len(po.inputs[0].part_sigs) == 0 # considered not ours
assert len(po.inputs[1].part_sigs) == 1 # signature added
@pytest.mark.parametrize("fail", [0, 1, 2])
@pytest.mark.parametrize("der", ["m", "m/48h/1h/0h/2h"])
def test_specific_wallet_signing_xpubs(der, fail, clear_miniscript, import_ms_wallet, fake_ms_txn,
try_sign, try_sign_microsd, start_sign, end_sign, cap_story):
M, N = 2, 3
addr_fmt = "p2wsh"
clear_miniscript()
def path_mapper(idx):
kk = str_to_path(der)
return kk + [0,0]
def include_xpubs(idx, xfp, m, sk):
kk = str_to_path(der)
bp = pack('<%dI' % (der.count("/") + 1), xfp, *kk)
return sk.node.serialize_public(), bp
name = "ms01"
keys = import_ms_wallet(M, N, name=name, accept=True, addr_fmt=addr_fmt, do_import=True, common=der)
psbt = fake_ms_txn(2, 1, M, keys, inp_addr_fmt=addr_fmt, incl_xpubs=include_xpubs,
outstyles=[addr_fmt], change_outputs=[0], netcode="XRT", path_mapper=path_mapper)
if fail:
po = BasicPSBT().parse(psbt)
item = po.xpubs[0]
if fail == 1:
# wrong key
key_wrong = item[0][:-1] + b"\x10"
po.xpubs[0] = (key_wrong, item[1])
elif fail == 2:
# wrong derivation path
pth_wrong = item[1][:-1] + b"\x10"
po.xpubs[0] = (item[0], pth_wrong)
psbt = po.as_bytes()
start_sign(psbt, miniscript=name)
if fail:
title, story = cap_story()
assert "Failure" in title
if der == "m":
# more thorough checks on root keys
# error messages differ, but always failure
pass
else:
assert "PSBT xpubs mismatch" in story
else:
end_sign(accept=True)
# EOF