diff --git a/releases/Next-ChangeLog.md b/releases/Next-ChangeLog.md index 310b80a5..d7f3ee09 100644 --- a/releases/Next-ChangeLog.md +++ b/releases/Next-ChangeLog.md @@ -26,6 +26,12 @@ This lists the new changes that have not yet been published in a normal release. # Mk4 Specific Changes ## 5.4.1 - 2024-??-?? +- 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). + + +## 5.4.? - 2024-??-?? - Enhancement: Export single sig descriptor with simple QR. diff --git a/shared/auth.py b/shared/auth.py index 8bf24ad5..fe0ae699 100644 --- a/shared/auth.py +++ b/shared/auth.py @@ -16,7 +16,7 @@ from ux import ux_aborted, ux_show_story, abort_and_goto, ux_dramatic_pause, ux_ from ux import show_qr_code, OK, X from usb import CCBusyError from utils import HexWriter, xfp2str, problem_file_line, cleanup_deriv_path -from utils import B2A, parse_addr_fmt_str, to_ascii_printable +from utils import B2A, parse_addr_fmt_str, to_ascii_printable, parse_msg_sign_request from psbt import psbtObject, FatalPSBTIssue, FraudulentChangeOutput from files import CardSlot from exceptions import HSMDenied @@ -303,8 +303,13 @@ def validate_text_for_signing(text): return result class ApproveMessageSign(UserAuthorizedAction): - def __init__(self, text, subpath, addr_fmt, approved_cb=None): + def __init__(self, text, subpath, addr_fmt, approved_cb=None, + msg_sign_request=None): super().__init__() + + if msg_sign_request: + text, subpath, addr_fmt = parse_msg_sign_request(msg_sign_request) + self.text = validate_text_for_signing(text) self.subpath = cleanup_deriv_path(subpath) self.addr_fmt = parse_addr_fmt_str(addr_fmt) @@ -362,25 +367,8 @@ async def sign_txt_file(filename): # sign a one-line text file found on a MicroSD card # - not yet clear how to do address types other than 'classic' from files import CardSlot, CardMissingError - from ux import the_ux - UserAuthorizedAction.cleanup() - - # copy message into memory - with CardSlot() as card: - with card.open(filename, 'rt') as fd: - text = fd.readline().strip() - subpath = fd.readline().strip() - addr_fmt = fd.readline().strip() - - if not subpath: - # default: top of wallet. - subpath = 'm' - - if not addr_fmt: - addr_fmt = AF_CLASSIC - async def done(signature, address, text): # complete. write out result from glob import dis @@ -442,9 +430,19 @@ async def sign_txt_file(filename): msg = "Created new file:\n\n%s" % out_fn await ux_show_story(msg, title='File Signed') + UserAuthorizedAction.cleanup() UserAuthorizedAction.check_busy() + + # copy message into memory + with CardSlot() as card: + with card.open(filename, 'rt') as fd: + res = fd.read() + try: - UserAuthorizedAction.active_request = ApproveMessageSign(text, subpath, addr_fmt, approved_cb=done) + UserAuthorizedAction.active_request = ApproveMessageSign( + None, None, None, approved_cb=done, + msg_sign_request=res + ) # do not kill the menu stack! the_ux.push(UserAuthorizedAction.active_request) except AssertionError as exc: @@ -1039,7 +1037,7 @@ class ApproveTransaction(UserAuthorizedAction): msg.write("\n") - # if we didn't already show all outputs, then give user a chance to + # if we didn't already show all outputs, then give user a chance to # view them individually return needs_txn_explorer @@ -1396,7 +1394,7 @@ class ShowAddressBase(UserAuthorizedAction): else: # finish the Wait... - dis.progress_bar_show(1) + dis.progress_bar_show(1) if self.restore_menu: self.pop_menu() diff --git a/shared/nfc.py b/shared/nfc.py index 7a876a7b..3df377ef 100644 --- a/shared/nfc.py +++ b/shared/nfc.py @@ -667,7 +667,7 @@ class NFCHandler: if len(m) < 70: return m = m.decode() - + # multi( catches both multi( and sortedmulti( if 'pub' in m or "multi(" in m: return m @@ -742,7 +742,7 @@ class NFCHandler: from ux import the_ux UserAuthorizedAction.cleanup() - + def f(m): m = m.decode() split_msg = m.split("\n") @@ -754,21 +754,11 @@ class NFCHandler: if not winner: return - if len(winner) == 1: - text = winner[0] - subpath = "m" - addr_fmt = AF_CLASSIC - elif len(winner) == 2: - text, subpath = winner - addr_fmt = AF_CLASSIC # maybe default to native segwit? - else: - # len(winner) == 3 - text, subpath, addr_fmt = winner - UserAuthorizedAction.check_busy(ApproveMessageSign) try: UserAuthorizedAction.active_request = ApproveMessageSign( - text, subpath, addr_fmt, approved_cb=self.msg_sign_done + None, None, None, approved_cb=self.msg_sign_done, + msg_sign_request=winner ) the_ux.push(UserAuthorizedAction.active_request) except AssertionError as exc: @@ -784,7 +774,7 @@ class NFCHandler: async def verify_sig_nfc(self): from auth import verify_armored_signed_msg - + f = lambda x: x.decode().strip() if b"SIGNED MESSAGE" in x else None winner = await self._nfc_reader(f, 'Unable to find signed message.') @@ -794,7 +784,7 @@ class NFCHandler: async def verify_address_nfc(self): # Get an address or complete bip-21 url even and search it... slow. from utils import decode_bip21_text - + def f(m): m = m.decode() what, vals = decode_bip21_text(m) @@ -814,7 +804,7 @@ class NFCHandler: async def read_tapsigner_b64_backup(self): f = lambda x: a2b_base64(x.decode()) if 150 <= len(x) <= 280 else None return await self._nfc_reader(f, 'Unable to find base64 encoded TAPSIGNER backup.') - + async def _nfc_reader(self, func, fail_msg): data = await self.start_nfc_rx() if not data: return diff --git a/shared/utils.py b/shared/utils.py index 15d548bc..a2761141 100644 --- a/shared/utils.py +++ b/shared/utils.py @@ -11,6 +11,13 @@ from public_constants import AF_CLASSIC, AF_P2WPKH, AF_P2WPKH_P2SH B2A = lambda x: str(b2a_hex(x), 'ascii') +STD_DERIVATIONS = { + "p2pkh": "m/44h/{chain}h/0h/0/0", + "p2sh-p2wpkh": "m/49h/{chain}h/0h/0/0", + "p2wpkh-p2sh": "m/49h/{chain}h/0h/0/0", + "p2wpkh": "m/84h/{chain}h/0h/0/0", +} + try: from font_iosevka import FontIosevka DOUBLE_WIDE = FontIosevka.DOUBLE_WIDE @@ -416,7 +423,7 @@ def check_firmware_hdr(hdr, binary_size): ok = (hw_compat & MK_4_OK) elif hw_label == 'q1': ok = (hw_compat & MK_Q1_OK) - + if not ok: return "That firmware doesn't support this version of Coldcard hardware (%s)."%hw_label @@ -701,4 +708,28 @@ def decode_bip21_text(got): def encode_seed_qr(words): return ''.join('%04d' % bip39.get_word_index(w) for w in words) +def parse_msg_sign_request(data): + lines = data.split("\n") + assert len(lines) >= 1, "min 1 line" + assert len(lines) <= 3, "max 3 lines" + + subpath = "" + addr_fmt = "p2pkh" + if len(lines) == 1: + text = lines[0] + elif len(lines) == 2: + text, subpath = lines + else: + text, subpath, addr_fmt = lines + if not addr_fmt: + addr_fmt = "p2pkh" + + if not subpath: + subpath = STD_DERIVATIONS[addr_fmt] + subpath = subpath.format( + chain=chains.current_chain().b44_cointype + ) + + return text, subpath, addr_fmt + # EOF diff --git a/testing/test_msg.py b/testing/test_msg.py index 8540f6ad..d6a3f058 100644 --- a/testing/test_msg.py +++ b/testing/test_msg.py @@ -11,6 +11,20 @@ from ckcc_protocol.constants import * from constants import addr_fmt_names, msg_sign_unmap_addr_fmt +def default_derivation_by_af(addr_fmt, testnet=True): + b44ct = "1" if testnet else "0" + if addr_fmt == AF_CLASSIC: + path = "m/44h/{chain}h/0h/0/0" + elif addr_fmt == AF_P2WPKH_P2SH: + path = "m/49h/{chain}h/0h/0/0" + elif addr_fmt == AF_P2WPKH: + path = "m/84h/{chain}h/0h/0/0" + else: + assert False, "unsupported address format" + + return path.format(chain=b44ct) + + @pytest.mark.parametrize('msg', [ 'aZ', 'hello', 'abc def eght', "x"*140, 'a'*240]) @pytest.mark.parametrize('path', [ 'm', "m/1/2", "m/1'/100'", 'm/23h/22h']) @pytest.mark.parametrize('addr_fmt', [ AF_CLASSIC, AF_P2WPKH, AF_P2WPKH_P2SH ]) @@ -82,7 +96,7 @@ def sign_on_microsd(open_microsd, cap_story, pick_menu_item, goto_home, # sign a file on the microSD card - def doit(msg, subpath=None, addr_fmt=None, expect_fail=False): + def doit(msg, subpath="", addr_fmt=None, expect_fail=False, testnet=True): fname = 't-msgsign.txt' result_fname = 't-msgsign-signed.txt' @@ -92,10 +106,10 @@ def sign_on_microsd(open_microsd, cap_story, pick_menu_item, goto_home, with open_microsd(fname, 'wt') as sd: sd.write(msg + '\n') - if subpath is not None: - sd.write(subpath + '\n') - if addr_fmt is not None: - sd.write(addr_fmt_names[addr_fmt] + '\n') + if subpath or addr_fmt: + sd.write((subpath or "") + '\n') + if addr_fmt is not None: + sd.write(addr_fmt_names[addr_fmt]) goto_home() pick_menu_item('Advanced/Tools') @@ -120,7 +134,9 @@ def sign_on_microsd(open_microsd, cap_story, pick_menu_item, goto_home, assert msg in story assert 'Using the key associated' in story if not subpath: - assert 'm =>' in story + assert 'm =>' not in story + pth = default_derivation_by_af(addr_fmt or AF_CLASSIC, testnet) + assert pth in story else: x_subpath = subpath.lower().replace("'", "h") assert ('%s =>' % x_subpath) in story @@ -148,6 +164,7 @@ def sign_on_microsd(open_microsd, cap_story, pick_menu_item, goto_home, return doit +@pytest.mark.bitcoind # only for testnet and p2pkh @pytest.mark.parametrize('msg', [ 'ab', 'hello', 'abc def eght', "x"*140, 'a'*240]) @pytest.mark.parametrize('path', [ "m/84'/0'/22'", @@ -163,24 +180,29 @@ def sign_on_microsd(open_microsd, cap_story, pick_menu_item, goto_home, AF_CLASSIC, AF_P2WPKH_P2SH, ]) -def test_sign_msg_microsd_good(sign_on_microsd, msg, path, addr_vs_path, addr_fmt): - - if (path is None) and (addr_fmt is not None): - # must give path if addr fmt is to be specified - return +@pytest.mark.parametrize("testnet", [True, False]) +def test_sign_msg_microsd_good(sign_on_microsd, msg, path, addr_vs_path, + addr_fmt, testnet, settings_set, bitcoind): + settings_set("chain", "XTN" if testnet else "BTC") # cases we expect to work - sig, addr = sign_on_microsd(msg, path, addr_fmt) + sig, addr = sign_on_microsd(msg, path, addr_fmt, testnet=testnet) raw = b64decode(sig) assert 40 <= len(raw) <= 65 - if path is None: - path = 'm' + if addr_fmt is None: + addr_fmt = AF_CLASSIC + + if not path: + path = default_derivation_by_af(addr_fmt, testnet=testnet) # check expected addr was used - addr_vs_path(addr, path, addr_fmt) + addr_vs_path(addr, path, addr_fmt, testnet=testnet) assert verify_message(addr, sig, msg) is True + if addr_fmt == AF_CLASSIC and testnet: + res = bitcoind.rpc.verifymessage(addr, sig, msg) + assert res is True @pytest.fixture @@ -210,7 +232,8 @@ def sign_using_nfc(goto_home, pick_menu_item, nfc_write_text, cap_story): ('testêtest', "must be ascii printable", 0), ]) @pytest.mark.parametrize('transport', ['sd', 'usb', 'nfc']) -def test_sign_msg_fails(dev, sign_on_microsd, msg, concern, no_file, transport, sign_using_nfc, path='m/12/34'): +def test_sign_msg_fails(dev, sign_on_microsd, msg, concern, no_file, + transport, sign_using_nfc, path='m/12/34'): if transport == 'usb': with pytest.raises(CCProtoError) as ee: @@ -229,7 +252,7 @@ def test_sign_msg_fails(dev, sign_on_microsd, msg, concern, no_file, transport, assert ("No suitable files found" in str(e)) or story == 'NO-FILE' return elif transport == 'nfc': - title, story = sign_using_nfc(msg, expect_fail=True) + title, story = sign_using_nfc(msg+"\n"+path, expect_fail=True) assert title == 'ERROR' or "Problem" in story else: raise ValueError(transport) @@ -301,11 +324,16 @@ def test_nfc_msg_signing_invalid(body, goto_home, pick_menu_item, nfc_write_text title, story = cap_story() assert title == 'ERROR' or "Problem" in story + +@pytest.mark.bitcoind # only for testnet and p2pkh +@pytest.mark.parametrize("testnet", [True, False]) @pytest.mark.parametrize("msg", ["coinkite", "Coldcard Signing Device!", 200 * "a"]) @pytest.mark.parametrize("path", ["", "m/84'/0'/0'/300/0", "m/800h/0h", "m/0/0/0/0/1/1/1"]) @pytest.mark.parametrize("str_addr_fmt", ["p2pkh", "", "p2wpkh", "p2wpkh-p2sh", "p2sh-p2wpkh"]) def test_nfc_msg_signing(msg, path, str_addr_fmt, nfc_write_text, nfc_read_text, pick_menu_item, - goto_home, cap_story, press_select, press_cancel, addr_vs_path, OK): + goto_home, cap_story, press_select, press_cancel, addr_vs_path, OK, + testnet, settings_set, bitcoind): + settings_set("chain", "XTN" if testnet else "BTC") for _ in range(5): # need to wait for ApproveMessageSign to be popped from ux stack @@ -325,6 +353,9 @@ def test_nfc_msg_signing(msg, path, str_addr_fmt, nfc_write_text, nfc_read_text, addr_fmt = AF_CLASSIC body = "\n".join([msg, path]) + if not path: + path = default_derivation_by_af(addr_fmt, testnet=testnet) + nfc_write_text(body) time.sleep(0.5) _, story = cap_story() @@ -339,7 +370,7 @@ def test_nfc_msg_signing(msg, path, str_addr_fmt, nfc_write_text, nfc_read_text, press_select() # exit NFC animation pmsg, addr, sig = parse_signed_message(signed_msg) assert pmsg == msg - addr_vs_path(addr, path, addr_fmt) + addr_vs_path(addr, path, addr_fmt, testnet=testnet) assert verify_message(addr, sig, msg) is True time.sleep(0.5) _, story = cap_story() @@ -349,9 +380,12 @@ def test_nfc_msg_signing(msg, path, str_addr_fmt, nfc_write_text, nfc_read_text, assert signed_msg == signed_msg_again press_cancel() # exit NFC animation press_cancel() # do not want to share again + if addr_fmt == AF_CLASSIC and testnet: + res = bitcoind.rpc.verifymessage(addr, sig, msg) + assert res is True @pytest.fixture -def verify_armored_signature(pick_menu_item, nfc_write_text, press_select, +def verify_armored_signature(pick_menu_item, nfc_write_text, cap_story, goto_home): def doit(way, fname=None, signed_msg=None): goto_home()