diff --git a/releases/ChangeLog.md b/releases/ChangeLog.md index c6dd4405..3238b18b 100644 --- a/releases/ChangeLog.md +++ b/releases/ChangeLog.md @@ -1,3 +1,8 @@ +## 3.0.7 - Jan TBD, 2020 +- Enhancement: when signing a text file from MicroSD card, if you specify a derivation + path that starts with `m/84'/...` indicating that you are following BIP84 for + segwit addresses, the resulting signature will be formated as P2WPKH in Bech32. + ## 3.0.6 - Dec 19, 2019 - Security Bugfix: Fixed a multisig PSBT-tampering issue, that could allow a MitM to diff --git a/shared/auth.py b/shared/auth.py index 57ae03bd..5f3da19c 100644 --- a/shared/auth.py +++ b/shared/auth.py @@ -6,7 +6,7 @@ # import stash, ure, tcc, ux, chains, sys, gc, uio from public_constants import MAX_TXN_LEN, MSG_SIGNING_MAX_LENGTH, SUPPORTED_ADDR_FORMATS -from public_constants import AFC_SCRIPT, AF_CLASSIC +from public_constants import AFC_SCRIPT, AF_CLASSIC, AF_P2WPKH from sffile import SFFile from ux import ux_aborted, ux_show_story, abort_and_goto, ux_dramatic_pause, ux_clear_keys from usb import CCBusyError @@ -234,6 +234,7 @@ def sign_txt_file(filename): global active_request UserAuthorizedAction.cleanup() + addr_fmt = AF_CLASSIC # copy message into memory with CardSlot() as card: @@ -248,6 +249,11 @@ def sign_txt_file(filename): except: await ux_show_story("Second line of file, if included, must specify a subkey path, like: m/44'/0/0") return + + # if they are following BIP84 recommended derivation scheme, + # then they probably would prefer a segwit/bech32 formatted address + if subpath.startswith("m/84'/"): + addr_fmt = AF_P2WPKH else: # default: top of wallet. @@ -322,7 +328,7 @@ def sign_txt_file(filename): global active_request UserAuthorizedAction.check_busy() - active_request = ApproveMessageSign(text, subpath, AF_CLASSIC, approved_cb=done) + active_request = ApproveMessageSign(text, subpath, addr_fmt, approved_cb=done) # do not kill the menu stack! from ux import the_ux diff --git a/testing/conftest.py b/testing/conftest.py index d444a9e7..8013791e 100644 --- a/testing/conftest.py +++ b/testing/conftest.py @@ -167,12 +167,19 @@ def addr_vs_path(master_xpub): from ckcc_protocol.constants import AF_P2WPKH_P2SH, AF_P2SH, AF_P2WSH, AF_P2WSH_P2SH from bech32 import bech32_decode, convertbits from pycoin.encoding import a2b_hashed_base58, hash160 + from pycoin.key.BIP32Node import PublicPrivateMismatchError from hashlib import sha256 def doit(given_addr, path=None, addr_fmt=None, script=None): if not script: - mk = BIP32Node.from_wallet_key(master_xpub) - sk = mk.subkey_for_path(path[2:]) + try: + # prefer using xpub if we can + mk = BIP32Node.from_wallet_key(master_xpub) + sk = mk.subkey_for_path(path[2:]) + except PublicPrivateMismatchError: + mk = BIP32Node.from_wallet_key(simulator_fixed_xprv) + sk = mk.subkey_for_path(path[2:]) + if addr_fmt == AF_CLASSIC: # easy @@ -217,6 +224,8 @@ def addr_vs_path(master_xpub): else: raise ValueError(addr_fmt) + return sk if not script else None + return doit diff --git a/testing/pytest.ini b/testing/pytest.ini index e813584d..42eeebf7 100644 --- a/testing/pytest.ini +++ b/testing/pytest.ini @@ -1,3 +1,5 @@ [pytest] addopts = -vvx #addopts = -vv +markers = + bitcoind: indicated local bitcoind (testnet) will be needed diff --git a/testing/test_addr.py b/testing/test_addr.py index a1389a5b..b2cd3ca5 100644 --- a/testing/test_addr.py +++ b/testing/test_addr.py @@ -16,9 +16,8 @@ def test_show_addr_usb(dev, need_keypress, addr_vs_path, path, addr_fmt): need_keypress('y') - if "'" not in path: - # check expected addr was used - addr_vs_path(addr, path, addr_fmt) + # check expected addr was used + addr_vs_path(addr, path, addr_fmt) @pytest.mark.parametrize('path', [ 'm', "m/1/2", "m/1'/100'"]) @pytest.mark.parametrize('addr_fmt', [ AF_CLASSIC, AF_P2WPKH, AF_P2WPKH_P2SH ]) @@ -32,9 +31,8 @@ def test_show_addr_displayed(dev, need_keypress, addr_vs_path, path, addr_fmt, c #need_keypress('x') - if "'" not in path: - # check expected addr was used - addr_vs_path(addr, path, addr_fmt) + # check expected addr was used + addr_vs_path(addr, path, addr_fmt) print('addr_fmt = 0x%x' % addr_fmt) diff --git a/testing/test_msg.py b/testing/test_msg.py index c4d918a7..44567ea2 100644 --- a/testing/test_msg.py +++ b/testing/test_msg.py @@ -9,11 +9,12 @@ from pycoin.contrib.msg_signing import verify_message from base64 import b64encode, b64decode from ckcc_protocol.protocol import CCProtocolPacker, CCProtoError, CCUserRefused from ckcc_protocol.constants import * +from constants import simulator_fixed_xprv -@pytest.mark.parametrize('msg', [ 'a', 'hello', 'abc def eght', "x"*140, 'a'*240]) +@pytest.mark.parametrize('msg', [ 'aB', 'hello', 'abc def eght', "x"*140, 'a'*240]) @pytest.mark.parametrize('path', [ 'm', "m/1/2", "m/1'/100'", 'm/23H/22p']) @pytest.mark.parametrize('addr_fmt', [ AF_CLASSIC, AF_P2WPKH, AF_P2WPKH_P2SH ]) -def test_sign_msg_good(dev, need_keypress, master_xpub, msg, path, addr_fmt, addr_vs_path): +def test_sign_msg_good(dev, need_keypress, msg, path, addr_fmt, addr_vs_path): msg = msg.encode('ascii') dev.send_recv(CCProtocolPacker.sign_message(msg, path, addr_fmt=addr_fmt), timeout=None) @@ -40,18 +41,12 @@ def test_sign_msg_good(dev, need_keypress, master_xpub, msg, path, addr_fmt, add assert '1' in addr return - if "'" not in path and 'p' not in path: - # check expected addr was used - mk = BIP32Node.from_wallet_key(master_xpub) - sk = mk.subkey_for_path(path[2:]) - - addr_vs_path(addr, path, addr_fmt) + # check expected addr was used + sk = addr_vs_path(addr, path, addr_fmt) - # verify signature - assert verify_message(sk, sig, message=msg.decode('ascii')) == True - else: - # just verify signature - assert verify_message(addr, sig, message=msg.decode('ascii')) == True + # verify signature + assert verify_message(sk, sig, message=msg.decode('ascii')) == True + assert verify_message(addr, sig, message=msg.decode('ascii')) == True def test_sign_msg_refused(dev, need_keypress, msg=b'testing 123', path='m'): @@ -166,8 +161,15 @@ def sign_on_microsd(open_microsd, cap_story, pick_menu_item, goto_home, need_key return doit @pytest.mark.parametrize('msg', [ 'ab', 'hello', 'abc def eght', "x"*140, 'a'*240]) -@pytest.mark.parametrize('path', [ None, 'm', "m/1/2", "m/1'/100'", 'm/23H/22p']) -def test_sign_msg_microsd_good(sign_on_microsd, master_xpub, msg, path, addr_vs_path, addr_fmt=AF_CLASSIC): +@pytest.mark.parametrize('path,addr_fmt', [ + ( "m/84p/0'/22p", AF_P2WPKH), + (None, AF_CLASSIC), + ( 'm', AF_CLASSIC), + ( "m/1/2", AF_CLASSIC), + ( "m/1'/100'", AF_CLASSIC), + ( 'm/23H/22p', AF_CLASSIC), + ]) +def test_sign_msg_microsd_good(sign_on_microsd, msg, path, addr_vs_path, addr_fmt): # cases we expect to work sig, addr = sign_on_microsd(msg, path) @@ -186,18 +188,14 @@ def test_sign_msg_microsd_good(sign_on_microsd, master_xpub, msg, path, addr_vs_ if path is None: path = 'm' - if "'" not in path and 'p' not in path: - # check expected addr was used - mk = BIP32Node.from_wallet_key(master_xpub) - sk = mk.subkey_for_path(path[2:]) + # check expected addr was used + sk = addr_vs_path(addr, path, addr_fmt) - addr_vs_path(addr, path, addr_fmt) - - # verify signature - assert verify_message(sk, sig, message=msg) == True - else: - # just verify signature - assert verify_message(addr, sig, message=msg) == True + if addr_fmt == AF_P2WPKH: + assert addr.startswith('tb1q') + + # verify signature + assert verify_message(sk, sig, message=msg) == True @pytest.mark.parametrize('msg,concern,no_file', [ ('', 'too short', 0), # zero length not supported diff --git a/testing/test_multisig.py b/testing/test_multisig.py index 463570c5..ef2b58b3 100644 --- a/testing/test_multisig.py +++ b/testing/test_multisig.py @@ -318,6 +318,7 @@ def test_ms_show_addr(dev, cap_story, need_keypress, addr_vs_path, bitcoind_p2sh assert ('/_/%d/0/0' % i) in story need_keypress('y') + # check expected addr was generated based on my math addr_vs_path(got_addr, addr_fmt=addr_fmt, script=scr) @@ -1014,7 +1015,7 @@ def fake_ms_txn(): @pytest.mark.parametrize('transport', [ 'sd' ]) @pytest.mark.parametrize('out_style', ADDR_STYLES_MS) @pytest.mark.parametrize('has_change', [ True, False]) -def test_ms_sign_simple(num_ins, dev, addr_fmt, clear_ms, incl_xpubs, import_ms_wallet, addr_vs_path, fake_ms_txn, try_sign, try_sign_microsd, transport, out_style, has_change, M=1, N=3): +def test_ms_sign_simple(num_ins, dev, addr_fmt, clear_ms, incl_xpubs, import_ms_wallet, fake_ms_txn, try_sign, try_sign_microsd, transport, out_style, has_change, M=1, N=3): num_outs = num_ins-1