option to show/export full multisig addresses; do not return to home menu after setting unsort_ms

This commit is contained in:
scgbckbone 2025-01-28 11:42:22 +01:00 committed by doc-hex
parent 70d303af78
commit 9b597592bc
7 changed files with 127 additions and 45 deletions

View File

@ -18,6 +18,7 @@ This lists the new changes that have not yet been published in a normal release.
- Enhancement: Catch more DeltaMode cases in XOR path.
Thanks to [@dmonakhov](https://github.com/dmonakhov))
- Enhancement: BKPW override (for "developers")
- Enhancement: Add option to show/export full multisg addresses. Enable in `Settings->Multisig Wallets->Full Address View`.
- Change: If derivation path is omitted during message signing, default is used
based on address format (`m/44h/0h/0h/0/0` for p2pkh, and `m/84h/0h/0h/0/0` for p2wpkh).
Default is no longer root (m).

View File

@ -15,7 +15,6 @@ from uhashlib import sha256
from ubinascii import hexlify as b2a_hex
from glob import settings
from auth import write_sig_file
from utils import censor_address
from charcodes import KEY_QR, KEY_NFC, KEY_PAGE_UP, KEY_PAGE_DOWN, KEY_HOME, KEY_LEFT, KEY_RIGHT
from charcodes import KEY_CANCEL
@ -30,6 +29,16 @@ def truncate_address(addr):
# tons of space on Q1
return addr[0:12] + '' + addr[-12:]
def censor_address(addr):
# We don't like to show the user full multisig addresses because we cannot be certain
# they could actually be signed. And yet, don't blank too many
# spots or else an attacker could grind out a suitable replacement.
# 3 chars in the middle hidden by default
# censoring can be disabled by msas setting
if settings.get("msas", 0):
return addr
return addr[0:12] + '___' + addr[12+3:]
class KeypathMenu(MenuSystem):
def __init__(self, path=None, nl=0):
self.prefix = None
@ -187,7 +196,7 @@ class AddressListMenu(MenuSystem):
deriv = path.format(account=self.account_num, change=0, idx=self.start)
node = sv.derive_path(deriv, register=False)
address = chain.address(node, addr_fmt)
choices.append( (truncate_address(address), path, addr_fmt) )
choices.append((truncate_address(address), path, addr_fmt))
dis.progress_sofar(len(choices), len(chains.CommonDerivations))
@ -291,20 +300,21 @@ Press (3) if you really understand and accept these risks.
dis.fullscreen('Wait...')
if ms_wallet:
# IMPORTANT safety feature: never show complete address
# IMPORTANT safety feature: do not show complete address unless user opt-in
# but show enough they can verify addrs shown elsewhere.
# - makes a redeem script
# - converts into addr
# - assumes 0/0 is first address.
for idx, addr, paths, script in ms_wallet.yield_addresses(start, n, change):
addrs.append(censor_address(addr))
addr = censor_address(addr)
addrs.append(addr)
if idx == 0 and ms_wallet.N <= 4:
msg += '\n'.join(paths) + '\n =>\n'
else:
msg += '⋯/%d/%d =>\n' % (change, idx)
msg += truncate_address(addr) + '\n\n'
msg += addr + '\n\n'
dis.progress_sofar(idx-start+1, n)
else:

View File

@ -10,7 +10,7 @@ from ux import import_export_prompt, ux_enter_bip32_index, show_qr_code, ux_ente
from files import CardSlot, CardMissingError, needs_microsd
from descriptor import MultisigDescriptor, multisig_descriptor_template
from public_constants import AF_P2SH, AF_P2WSH_P2SH, AF_P2WSH, AFC_SCRIPT, MAX_SIGNERS
from menu import MenuSystem, MenuItem, NonDefaultMenuItem
from menu import MenuSystem, MenuItem, NonDefaultMenuItem, start_chooser, ToggleMenuItem
from opcodes import OP_CHECKMULTISIG
from exceptions import FatalPSBTIssue
from glob import settings
@ -1238,7 +1238,6 @@ def disable_checks_chooser():
return int(MultisigWallet.disable_checks), ch, xset
async def disable_checks_menu(*a):
from menu import start_chooser
if not MultisigWallet.disable_checks:
ch = await ux_show_story('''\
@ -1271,7 +1270,6 @@ def psbt_xpubs_policy_chooser():
async def trust_psbt_menu(*a):
# show a story then go into chooser
from menu import start_chooser
ch = await ux_show_story('''\
This setting controls what the Coldcard does \
@ -1296,18 +1294,16 @@ exists, otherwise 'Verify'.''')
if ch == 'x': return
start_chooser(psbt_xpubs_policy_chooser)
def unsorted_ms_chooser():
ch = ['Do Not Allow', 'Allow']
def unsort_ms_chooser():
def xset(idx, text):
settings.set('unsort_ms', idx)
from actions import goto_top_menu
goto_top_menu()
if idx:
settings.set('unsort_ms', idx)
else:
settings.remove_key('unsort_ms')
return settings.get('unsort_ms', 0), ch, xset
return settings.get('unsort_ms', 0), ['Do Not Allow', 'Allow'], xset
async def unsorted_ms_menu(*a):
from menu import start_chooser
if not settings.get("unsort_ms", None):
ch = await ux_show_story(
@ -1335,7 +1331,7 @@ async def unsorted_ms_menu(*a):
)
return
start_chooser(unsorted_ms_chooser)
start_chooser(unsort_ms_chooser)
class MultisigMenu(MenuSystem):
@ -1360,6 +1356,11 @@ class MultisigMenu(MenuSystem):
rv.append(MenuItem('Create Airgapped', f=create_ms_step1))
rv.append(MenuItem('Trust PSBT?', f=trust_psbt_menu))
rv.append(MenuItem('Skip Checks?', f=disable_checks_menu))
rv.append(ToggleMenuItem('Full %s View' % ("Address" if version.has_qwerty else "Addr"),
'msas', ["Hide Chars", "Show Full"], story=(
"With this setting ON, full multisig addresses are shown."
" This should not discourage you to cross-verify multisig addresses"
" with your coordinator software.")))
rv.append(NonDefaultMenuItem('Unsorted Multisig' if version.has_qwerty else "Unsorted Multi",
'unsort_ms',
f=unsorted_ms_menu))

View File

@ -64,6 +64,7 @@ from utils import call_later_ms
# ptxurl = (str) URL for PushTx feature, clear to disable feature
# hmx = (bool) Force display of current XFP in home menu, even w/o tmp seed active
# unsort_ms = (bool) Allow unsorted multisig with BIP-67 disabled
# msas = multisig address show (do not censor multisig addresses)
# Stored w/ key=00 for access before login
# _skip_pin = hard code a PIN value (dangerous, only for debug)
@ -85,7 +86,7 @@ from utils import call_later_ms
# keep these settings only if unspecified on the other end
KEEP_IF_BLANK_SETTINGS = ["wa", "sighshchk", "emu", "rz", "b39skip",
"axskip", "del", "pms", "idle_to", "batt_to",
"bright"]
"bright", "msas"]
SEEDVAULT_FIELDS = ['seeds', 'seedvault', 'xfp', 'words', "bkpw"]

View File

@ -589,12 +589,6 @@ def datetime_to_str(dt, fmt="%d-%02d-%02d %02d:%02d:%02d"):
dts = fmt % (y, mo, d, h, mi, s)
return dts + " UTC"
def censor_address(addr):
# We don't like to show the user multisig addresses because we cannot be certain
# they are valid and could actually be signed. And yet, dont blank too many
# spots or else an attacker could grind out a suitable replacement.
return addr[0:12] + '___' + addr[12+3:]
def txid_from_fname(fname):
if len(fname) >= 64:
txid = fname[:64]

View File

@ -936,8 +936,9 @@ def reset_seed_words(sim_exec, sim_execfile, simulator):
@pytest.fixture()
def settings_set(sim_exec):
def doit(key, val):
x = sim_exec("settings.set('%s', %r)" % (key, val))
def doit(key, val, prelogin=False):
source = "from nvstore import SettingsObject;SettingsObject.prelogin()" if prelogin else "settings"
x = sim_exec("%s.set('%s', %r)" % (source, key, val))
assert x == ''
return doit
@ -945,8 +946,9 @@ def settings_set(sim_exec):
@pytest.fixture()
def settings_get(sim_exec):
def doit(key, def_val=None):
cmd = f"RV.write(repr(settings.get('{key}', {def_val!r})))"
def doit(key, def_val=None, prelogin=False):
source = "from nvstore import SettingsObject;SettingsObject.prelogin()" if prelogin else "settings"
cmd = f"RV.write(repr({source}.get('{key}', {def_val!r})))"
resp = sim_exec(cmd)
assert 'Traceback' not in resp, resp
return eval(resp)

View File

@ -15,7 +15,7 @@ from ckcc.protocol import CCProtocolPacker, MAX_TXN_LEN
from pprint import pprint
from base64 import b64encode, b64decode
from base58 import encode_base58_checksum
from helpers import B2A, fake_dest_addr, xfp2str, detruncate_address
from helpers import B2A, fake_dest_addr, xfp2str
from helpers import path_to_str, str_to_path, slip132undo, swab32, hash160
from struct import unpack, pack
from constants import *
@ -1052,7 +1052,7 @@ def test_import_dup_safe(N, clear_ms, make_multisig, offer_ms_import,
menu = cap_menu()
assert f'{M}/{N}: {name}' in menu
# depending if NFC enabled or not, and if Q (has QR)
assert (len(menu) - num_wallets) in [6, 7, 8]
assert (len(menu) - num_wallets) in [7,8,9]
title, story = offer_ms_import(make_named('xxx-orig'))
assert 'Create new multisig wallet' in story
@ -1266,7 +1266,7 @@ def make_myself_wallet(dev, set_bip39_pw, offer_ms_import, press_select, clear_m
title, story = offer_ms_import(config)
#print(story)
# dont care if update or create; accept it.
# don't care if update or create; accept it.
time.sleep(.1)
press_select()
@ -2150,6 +2150,7 @@ def test_danger_warning(request, descriptor, clear_ms, import_ms_wallet, cap_sto
else:
assert 'WARNING' not in story
@pytest.mark.parametrize('msas', [True, False])
@pytest.mark.parametrize('change', [True, False])
@pytest.mark.parametrize('desc', ["multi", "sortedmulti"])
@pytest.mark.parametrize('start_idx', [1000, MAX_BIP32_IDX, 0])
@ -2158,12 +2159,13 @@ def test_danger_warning(request, descriptor, clear_ms, import_ms_wallet, cap_sto
def test_ms_addr_explorer(change, M_N, addr_fmt, start_idx, clear_ms, cap_menu,
need_keypress, goto_home, pick_menu_item, cap_story,
import_ms_wallet, make_multisig, settings_set,
enter_number, set_addr_exp_start_idx, desc):
enter_number, set_addr_exp_start_idx, desc, msas):
clear_ms()
M, N = M_N
wal_name = f"ax{M}-{N}-{addr_fmt}"
settings_set("aei", True if start_idx else False)
settings_set("msas", 1 if msas else 0)
dd = {
AF_P2WSH: ("m/48h/1h/0h/2h/{idx}", 'p2wsh'),
@ -2240,9 +2242,12 @@ def test_ms_addr_explorer(change, M_N, addr_fmt, start_idx, clear_ms, cap_menu,
assert int(subpath.split('/')[-1]) == idx
#print('../0/%s => \n %s' % (idx, B2A(script)))
start, end = detruncate_address(addr)
assert expect.startswith(start)
assert expect.endswith(end)
if msas:
assert addr == expect
else:
start, end = addr.strip().split('___')
assert expect.startswith(start)
assert expect.endswith(end)
def test_dup_ms_wallet_bug(goto_home, pick_menu_item, press_select, import_ms_wallet,
@ -2322,11 +2327,12 @@ def test_bitcoind_ms_address(change, M_N, addr_fmt, clear_ms, goto_home, need_ke
pick_menu_item, cap_menu, cap_story, make_multisig, import_ms_wallet,
microsd_path, bitcoind_d_wallet_w_sk, use_regtest, load_export, way,
is_q1, press_select, start_idx, settings_set, set_addr_exp_start_idx,
desc):
desc, garbage_collector, virtdisk_path):
use_regtest()
clear_ms()
bitcoind = bitcoind_d_wallet_w_sk
M, N = M_N
path_f = microsd_path if way == "sd" else virtdisk_path
# whether to import as descriptor or old school to CC
descriptor = random.choice([True, False])
bip67 = True
@ -2335,6 +2341,9 @@ def test_bitcoind_ms_address(change, M_N, addr_fmt, clear_ms, goto_home, need_ke
descriptor = True
settings_set("aei", True if start_idx else False)
# adding this as parameter doubles the time this runs
msas = random.getrandbits(1)
settings_set("msas", 1 if msas else 0)
wal_name = f"ax{M}-{N}-{addr_fmt}"
@ -2377,7 +2386,12 @@ def test_bitcoind_ms_address(change, M_N, addr_fmt, clear_ms, goto_home, need_ke
assert "change addresses." not in story
assert "(0)" not in story
contents = load_export(way, label="Address summary", is_json=False, sig_check=False)
if way != "nfc":
contents, exp_fname = load_export(way, label="Address summary", is_json=False,
sig_check=False, ret_fname=True)
garbage_collector.append(path_f(exp_fname))
else:
contents = load_export(way, label="Address summary", is_json=False, sig_check=False)
addr_cont = contents.strip()
goto_home()
pick_menu_item('Settings')
@ -2385,7 +2399,12 @@ def test_bitcoind_ms_address(change, M_N, addr_fmt, clear_ms, goto_home, need_ke
press_select() # only one enrolled multisig - choose it
pick_menu_item('Descriptors')
pick_menu_item("Bitcoin Core")
contents = load_export(way, label="Bitcoin Core multisig setup", is_json=False, sig_check=False)
if way != "nfc":
contents, exp_fname = load_export(way, label="Bitcoin Core multisig setup", is_json=False,
sig_check=False, ret_fname=True)
garbage_collector.append(path_f(exp_fname))
else:
contents = load_export(way, label="Bitcoin Core multisig setup", is_json=False, sig_check=False)
text = contents.replace("importdescriptors ", "").strip()
# remove junk
r1 = text.find("[")
@ -2421,19 +2440,24 @@ def test_bitcoind_ms_address(change, M_N, addr_fmt, clear_ms, goto_home, need_ke
bitcoind_addrs = bitcoind.deriveaddresses(desc_export, addr_range)
for idx, cc_item in enumerate(cc_addrs):
cc_item = cc_item.split(",")
partial_address = cc_item[part_addr_index]
_start, _end = partial_address.split("___")
if way != "nfc":
_start, _end = _start[1:], _end[:-1]
assert bitcoind_addrs[idx].startswith(_start)
assert bitcoind_addrs[idx].endswith(_end)
if msas:
addr = cc_item[part_addr_index]
if way != "nfc":
addr = addr[1:-1]
assert bitcoind_addrs[idx] == addr
else:
partial_address = cc_item[part_addr_index]
_start, _end = partial_address.split("___")
if way != "nfc":
_start, _end = _start[1:], _end[:-1]
assert bitcoind_addrs[idx].startswith(_start)
assert bitcoind_addrs[idx].endswith(_end)
@pytest.mark.bitcoind
def test_legacy_multisig_witness_utxo_in_psbt(bitcoind, use_regtest, clear_ms, microsd_wipe, goto_home, need_keypress,
pick_menu_item, cap_story, load_export, microsd_path, cap_menu, try_sign,
is_q1, press_select):
use_regtest()
clear_ms()
microsd_wipe()
@ -3384,4 +3408,53 @@ def test_json_import_failures(err, config, offer_ms_import):
offer_ms_import(json.dumps(config))
assert err in e.value.args[0]
def test_msas_enable_disable(import_ms_wallet, pick_menu_item, cap_story, goto_home, is_q1,
settings_set, need_keypress, press_select):
goto_home()
settings_set("msas", 0, prelogin=True) # default
name = "msas_test"
import_ms_wallet(2,3,"p2wsh", accept=True, name=name)
goto_home()
pick_menu_item("Address Explorer")
need_keypress("4") # confirm msg
pick_menu_item(name) # ms wallet
time.sleep(.1)
_, story = cap_story()
assert "___" in story
goto_home()
pick_menu_item("Settings")
pick_menu_item("Multisig Wallets")
pick_menu_item("Full %s View" % ("Address" if is_q1 else "Addr"))
time.sleep(.1)
_, story = cap_story()
assert "full multisig addresses are shown" in story
press_select()
time.sleep(.1)
pick_menu_item("Show Full")
goto_home()
pick_menu_item("Address Explorer")
need_keypress("4") # confirm msg
pick_menu_item(name) # ms wallet
time.sleep(.1)
_, story = cap_story()
assert "___" not in story
goto_home()
pick_menu_item("Settings")
pick_menu_item("Multisig Wallets")
pick_menu_item("Full %s View" % ("Address" if is_q1 else "Addr"))
# now enabled - so no story
pick_menu_item("Hide Chars")
goto_home()
pick_menu_item("Address Explorer")
need_keypress("4") # confirm msg
pick_menu_item(name) # ms wallet
time.sleep(.1)
_, story = cap_story()
assert "___" in story
# EOF