From 6985512be73ddea135016398bdf2ff06c7e05c92 Mon Sep 17 00:00:00 2001 From: scgbckbone Date: Thu, 6 Feb 2025 13:34:49 +0100 Subject: [PATCH] msg sign: address format from standard derivation paths if address format not specified (cherry picked from commit 2feb991d96b2011eda6435692f43d35b13565a03) --- releases/Next-ChangeLog.md | 2 ++ shared/auth.py | 23 +++++++++++----- testing/test_msg.py | 54 ++++++++++++++++++++++++++++++++++---- 3 files changed, 68 insertions(+), 11 deletions(-) diff --git a/releases/Next-ChangeLog.md b/releases/Next-ChangeLog.md index facf4fbd..ccf04bf3 100644 --- a/releases/Next-ChangeLog.md +++ b/releases/Next-ChangeLog.md @@ -27,6 +27,8 @@ This lists the new changes that have not yet been published in a normal release. - 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). +- Change: If address format is not provided during msg signing and subpath derivation starts with: + a.) `m/84h/...` p2wpkh address format is implied b.) `m/49h/...` p2sh-p2wpkh is implied. - Bugfix: Sometimes see a struck screen after _Verifying..._ in boot up sequence. On Q, result is blank screen, on Mk4, result is three-dots screen. - Bugfix: Do not allow to enable/disable Seed Vault feature when in temporary seed mode. diff --git a/shared/auth.py b/shared/auth.py index bbea3421..5f89ad04 100644 --- a/shared/auth.py +++ b/shared/auth.py @@ -301,9 +301,20 @@ def validate_text_for_signing(text, only_printable=True): # looks ok return result +def addr_fmt_from_subpath(subpath): + if not subpath: + af = "p2pkh" + elif subpath[:4] == "m/84": + af = "p2wpkh" + elif subpath[:4] == "m/49": + af = "p2sh-p2wpkh" + else: + af = "p2pkh" + return af + def parse_msg_sign_request(data): subpath = "" - addr_fmt = "p2pkh" + addr_fmt = None is_json = False # sparrow compat @@ -314,8 +325,8 @@ def parse_msg_sign_request(data): # subpath will be verified & cleaned later assert msg_line[0][:6] == "ascii:" text = msg_line[0][6:] - return text, subpath, addr_fmt, is_json - except: pass + return text, subpath, addr_fmt_from_subpath(subpath), is_json + except:pass # === try: @@ -337,8 +348,9 @@ def parse_msg_sign_request(data): text, subpath = lines else: text, subpath, addr_fmt = lines - if not addr_fmt: - addr_fmt = "p2pkh" + + if not addr_fmt: + addr_fmt = addr_fmt_from_subpath(subpath) if not subpath: subpath = chains.STD_DERIVATIONS[addr_fmt] @@ -410,7 +422,6 @@ class ApproveMessageSign(UserAuthorizedAction): def sign_msg(text, subpath, addr_fmt): - subpath = cleanup_deriv_path(subpath) UserAuthorizedAction.check_busy() UserAuthorizedAction.active_request = ApproveMessageSign(text, subpath, addr_fmt) # kill any menu stack, and put our thing at the top diff --git a/testing/test_msg.py b/testing/test_msg.py index 7d6c0d78..65765035 100644 --- a/testing/test_msg.py +++ b/testing/test_msg.py @@ -13,6 +13,17 @@ from charcodes import KEY_QR, KEY_NFC from helpers import addr_from_display_format +def addr_fmt_from_subpath(subpath): + if not subpath: + af = AF_CLASSIC + elif subpath[:4] == "m/84": + af = AF_P2WPKH + elif subpath[:4] == "m/49": + af = AF_P2WPKH_P2SH + else: + af = AF_CLASSIC + return af + def default_derivation_by_af(addr_fmt, testnet=True): b44ct = "1" if testnet else "0" if addr_fmt == AF_CLASSIC: @@ -359,7 +370,7 @@ def sign_on_microsd(open_microsd, cap_story, pick_menu_item, goto_home, @pytest.mark.bitcoind # only for testnet and p2pkh @pytest.mark.parametrize("use_json", [True, False]) -@pytest.mark.parametrize('msg', [ 'ab', 'abc def eght', "x"*140, 'a'*240]) +@pytest.mark.parametrize('msg', [ 'ab', 'abc def eght', 'a'*240]) @pytest.mark.parametrize('path', [ "m/84'/0'/22'", None, @@ -388,7 +399,7 @@ def test_sign_msg_microsd_good(sign_on_microsd, msg, path, addr_vs_path, assert 40 <= len(raw) <= 65 if addr_fmt is None: - addr_fmt = AF_CLASSIC + addr_fmt = addr_fmt_from_subpath(path) if not path: path = default_derivation_by_af(addr_fmt, testnet=testnet) @@ -430,7 +441,7 @@ def sign_using_nfc(goto_home, pick_menu_item, nfc_write_text, cap_story, press_s return cap_story() if not addr_fmt: - addr_fmt = AF_CLASSIC + addr_fmt = addr_fmt_from_subpath(subpath) if not subpath: subpath = default_derivation_by_af(addr_fmt, testnet=testnet) @@ -586,7 +597,7 @@ def test_low_R_cases(msg, num_iter, expect, dev, set_seed_words, use_mainnet, @pytest.mark.parametrize("testnet", [True, False]) @pytest.mark.parametrize("use_json", [True, False]) @pytest.mark.parametrize("msg", ["Coldcard Signing Device!", 200 * "a"]) -@pytest.mark.parametrize("path", ["", "m/84'/0'/0'/300/0", "m/0/0/0/0/1/1/1"]) +@pytest.mark.parametrize("path", ["", "m/84h/0h/0h/300/0", "m/0/0/0/0/1/1/1"]) @pytest.mark.parametrize("addr_fmt", [AF_CLASSIC, None, AF_P2WPKH, AF_P2WPKH_P2SH]) def test_nfc_msg_signing(msg, path, addr_fmt, testnet, settings_set, bitcoind, use_json, sign_using_nfc, goto_home): @@ -950,6 +961,8 @@ def test_sign_scanned_text(msg, addr_fmt, acct, goto_home, need_keypress, scan_a {"msg": "msg to be signed via QR"}, {"msg": "msg with some\n\t\n control characters", "addr_fmt": "p2sh-p2wpkh"}, {"msg": 100*"CC", "addr_fmt": "p2wpkh", "subpath": "m/900h/0"}, + {"msg": "This is my address! @twiiter_nick", "subpath": "m/84h/1h/0h/0/0"}, + {"msg": "This is my address! @twiiter_nick", "subpath": "m/49'/0'/5'/1/100"}, ]) @pytest.mark.parametrize("way", ["sd", "nfc", "qr"]) def test_sign_scanned_json(data, way, goto_home, need_keypress, scan_a_qr, @@ -960,7 +973,7 @@ def test_sign_scanned_json(data, way, goto_home, need_keypress, scan_a_qr, goto_home() af = data.get("addr_fmt", None) if not af: - addr_fmt = AF_CLASSIC + addr_fmt = addr_fmt_from_subpath(data.get("subpath", None)) else: addr_fmt = msg_sign_unmap_addr_fmt[af] @@ -983,6 +996,37 @@ def test_sign_scanned_json(data, way, goto_home, need_keypress, scan_a_qr, assert res is True +@pytest.mark.bitcoind +@pytest.mark.parametrize("msg", ["an an an an an an an an", 240*"a"]) +@pytest.mark.parametrize("path", ["m/84h/0", "m/44h/0", "m/49h/0", "m"]) +def test_sparrow_qr_sign_msg(msg, path, skip_if_useless_way, need_keypress, scan_a_qr, cap_story, + verify_msg_sign_story, press_select, msg_sign_export, addr_vs_path, + bitcoind): + skip_if_useless_way("qr") + + tmplt = "signmessage %s ascii:%s" + data = tmplt % (path, msg) + + addr_fmt = addr_fmt_from_subpath(path) + + need_keypress(KEY_QR) + scan_a_qr(data) + time.sleep(1) + title, story = cap_story() + subpath = verify_msg_sign_story(story, msg, path, addr_fmt) + press_select() + + signed_msg = msg_sign_export("qr") + ret_msg, addr, sig = parse_signed_message(signed_msg) + assert ret_msg == msg + # check expected addr was used + addr_vs_path(addr, subpath, addr_fmt) + assert verify_message(addr, sig, ret_msg) is True + if addr_fmt == AF_CLASSIC: + res = bitcoind.rpc.verifymessage(addr, sig, ret_msg) + assert res is True + + @pytest.mark.parametrize("msg", [(50*"a")+"\n\n"+(100*"b"), "Balance replenish 564565456254"]) def test_verify_scanned_signed_msg(msg, scan_a_qr, need_keypress, goto_home, cap_story, skip_if_useless_way):