diff --git a/releases/Next-ChangeLog.md b/releases/Next-ChangeLog.md index c1f66a42..df7dd7db 100644 --- a/releases/Next-ChangeLog.md +++ b/releases/Next-ChangeLog.md @@ -60,6 +60,7 @@ This lists the new changes that have not yet been published in a normal release. ## 1.4.xQ - 2065-04-xx - New Feature: Secure Notes & Passwords UX groups +- New Feature: Apply Secure Note text, or Secure Note password as BIP-39 passphrase - Bugfix: Teleporting a multisig PSBT file (without signing it first) sent stale data instead of the selected file - Bugfix: Fix export UX message after teleport PSBT import & sign - Bugfix: BIP-21 QR `amount` rendered with wrong decimal scaling on the Payment Address screen (e.g. `amount=1.1` was shown as `1.00000001 BTC`) diff --git a/shared/auth.py b/shared/auth.py index 639aed61..6618324f 100644 --- a/shared/auth.py +++ b/shared/auth.py @@ -131,7 +131,7 @@ Press %s to continue, otherwise %s to cancel.''' % (OK, X) class ApproveMessageSign(UserAuthorizedAction): def __init__(self, text, subpath, addr_fmt, approved_cb=None, - msg_sign_request=None, only_printable=True, privkey=None): + msg_sign_request=None, allow_tab_nl=False, privkey=None): super().__init__() is_json = False @@ -141,7 +141,7 @@ class ApproveMessageSign(UserAuthorizedAction): text, subpath, addr_fmt, is_json = parse_msg_sign_request(msg_sign_request) self.text = validate_text_for_signing( - text, only_printable=not is_json and only_printable + text, allow_tab_nl=is_json and allow_tab_nl ) self.subpath = cleanup_deriv_path(subpath) self.addr_fmt = chains.parse_addr_fmt_str(addr_fmt) @@ -203,7 +203,7 @@ def sign_msg(text, subpath, addr_fmt): async def approve_msg_sign(text, subpath, addr_fmt, approved_cb=None, msg_sign_request=None, kill_menu=False, - only_printable=True, privkey=None): + allow_tab_nl=False, privkey=None): # Ask user if they want to sign some short text message. UserAuthorizedAction.cleanup() @@ -213,7 +213,7 @@ async def approve_msg_sign(text, subpath, addr_fmt, approved_cb=None, text, subpath, addr_fmt, approved_cb=approved_cb, msg_sign_request=msg_sign_request, - only_printable=only_printable, + allow_tab_nl=allow_tab_nl, privkey=privkey ) diff --git a/shared/msgsign.py b/shared/msgsign.py index 23ba61f0..27d6ff2f 100644 --- a/shared/msgsign.py +++ b/shared/msgsign.py @@ -262,13 +262,13 @@ def write_sig_file(content_list, derive=None, addr_fmt=AF_CLASSIC, pk=None, sig_ return sig_nice -def validate_text_for_signing(text, only_printable=True): +def validate_text_for_signing(text, allow_tab_nl=False): # Check for some UX/UI traps in the message itself. # - messages must be short and ascii only. Our charset is limited # - too many spaces, leading/trailing can be an issue # MSG_MAX_SPACES = 4 # impt. compared to -=- positioning text = str(text, "ascii") # handle memoryview coming from USB - result = to_ascii_printable(text, only_printable=only_printable) + result = to_ascii_printable(text, allow_tab_nl=allow_tab_nl) length = len(result) assert length >= 2, "msg too short (min. 2)" @@ -416,7 +416,7 @@ async def ux_sign_msg(txt, approved_cb=None, kill_menu=True): if subpath is None: return await approve_msg_sign(text, subpath, af, approved_cb=approved_cb, - kill_menu=kill_menu, only_printable=False) + kill_menu=kill_menu, allow_tab_nl=True) # pick address format rv = [ diff --git a/shared/notes.py b/shared/notes.py index bee03c36..5b46d1ec 100644 --- a/shared/notes.py +++ b/shared/notes.py @@ -14,7 +14,7 @@ from public_constants import MSG_SIGNING_MAX_LENGTH from charcodes import KEY_QR, KEY_NFC, KEY_CANCEL from charcodes import KEY_F1, KEY_F2, KEY_F3, KEY_F4, KEY_F5, KEY_F6 from lcd_display import CHARS_W -from utils import problem_file_line, url_unquote, wipe_if_deltamode +from utils import problem_file_line, url_unquote, wipe_if_deltamode, is_printable # title, username and such are limited that they fit on the one line both in # text entry (W-2) and also in menu display (W-3) @@ -426,6 +426,23 @@ class NoteContentBase: return MenuItem("Sign Note Text", f=self.sign_txt_msg, arg=self.misc, predicate=2 <= len(self.misc) <= MSG_SIGNING_MAX_LENGTH) + @staticmethod + def is_b39pass_applicable(data, read_only): + from seed import MAX_PASS_LEN + from ccc import sssp_spending_policy + if read_only and not sssp_spending_policy('okeys'): + return False + return (len(data) <= MAX_PASS_LEN) and is_printable(data) and settings.get("words", True) + + async def apply_as_b39_pass(self, a, b, item): + data, readonly = item.arg + # rstrip just trailing whitespaces/tabs/newlines + data = data.rstrip() + # do not allow any more tabs/newlines + assert self.is_b39pass_applicable(data, readonly) + from seed import apply_pass_value + await apply_pass_value(data) + class NoteGroupMenu(MenuSystem): def __init__(self, group, readonly=False): @@ -520,6 +537,12 @@ class PasswordContent(NoteContentBase): ShortcutItem(KEY_NFC, f=self.share_nfc, arg=self.type_label), ] + # if password is less than MAX_PASS_LEN and only consist of printable ASCII characters + # and current seed (master or tmp) is word based - offer to apply pwd text as BIP-39 passphrase + if self.is_b39pass_applicable(self.password, readonly): + rv += [MenuItem('Apply as BIP-39 Passphrase', + f=self.apply_as_b39_pass, arg=(self.password, readonly))] + return rv async def make_menu(self, a, b, item): @@ -647,6 +670,7 @@ class NoteContent(NoteContentBase): async def _make_menu(self, readonly=False): # Details and actions for this Note + rv = [ MenuItem('"%s"' % self.title, f=self.view), MenuItem('View Note', f=self.view), @@ -657,11 +681,19 @@ class NoteContent(NoteContentBase): MenuItem('Delete', f=self.delete), MenuItem('Export', f=self.export), ] + rv += [ self.sign_misc_menu_item(), ShortcutItem(KEY_QR, f=self.view_qr_menu, arg="misc"), ShortcutItem(KEY_NFC, f=self.share_nfc, arg='misc'), ] + + # if misc is less than MAX_PASS_LEN and only consist of printable ASCII characters + # and current seed (master or tmp) is word based - offer to apply note text as BIP-39 passphrase + if self.is_b39pass_applicable(self.misc, readonly): + rv += [MenuItem('Apply as BIP-39 Passphrase', + f=self.apply_as_b39_pass, arg=(self.misc, readonly))] + return rv async def make_menu(self, a, b, item): diff --git a/shared/psbt.py b/shared/psbt.py index d7c8d0b6..b5c19f50 100644 --- a/shared/psbt.py +++ b/shared/psbt.py @@ -1532,7 +1532,7 @@ class psbtObject(psbtProxy): self.por322 = bool(self.por322_msg) if self.por322: - if not all(ord(ch) < 128 for ch in self.por322_msg): + if len(self.por322_msg) != len(self.por322_msg.encode()): self.warnings.append(( "Message", "Message contains non-ASCII characters that may not be readable on this screen." diff --git a/shared/seed.py b/shared/seed.py index bbca691a..e63feb84 100644 --- a/shared/seed.py +++ b/shared/seed.py @@ -33,6 +33,9 @@ from ucollections import namedtuple # seed words lengths we support: 24=>256 bits, and recommended VALID_LENGTHS = (24, 18, 12) +# maximum length for BIP-39 passphrase +MAX_PASS_LEN = 100 + # bit flag that means "also include bare prefix as a valid word" _PREFIX_MARKER = const(1<<26) @@ -1235,10 +1238,10 @@ the passphrase as well, it's okay to put them together.) There is no way for \ the Coldcard to know if your entry is correct, and if you have it wrong, \ you will be looking at an empty wallet. -Limitations: 100 characters max length, ASCII characters 32-126 (0x20-0x7e) only. +Limitations: %d characters max length, ASCII characters 32-126 (0x20-0x7e) only. %s to continue or press (2) to hide this message forever. -''' % (howto if not version.has_qwerty else '', OK) +''' % (howto if not version.has_qwerty else '', MAX_PASS_LEN, OK) ch = await ux_show_story(msg, escape='2') if ch == '2': @@ -1248,8 +1251,8 @@ Limitations: 100 characters max length, ASCII characters 32-126 (0x20-0x7e) only if version.has_qwerty and not PassphraseSaver.has_file(): # no need for any menus if Q and no card present - pp = await ux_input_text('', prompt="Your BIP-39 Passphrase", - b39_complete=True, scan_ok=True, max_len=100) + pp = await ux_input_text('', prompt="Your BIP-39 Passphrase", b39_complete=True, + scan_ok=True, max_len=MAX_PASS_LEN) if not pp: return await apply_pass_value(pp) @@ -1259,7 +1262,7 @@ Limitations: 100 characters max length, ASCII characters 32-126 (0x20-0x7e) only class PassphraseMenu(MenuSystem): - # Collect up to 100 chars as a BIP-39 passphrase + # Collect up to MAX_PASS_LEN chars as a BIP-39 passphrase # singleton (cls level) vars done_cb = None @@ -1348,7 +1351,7 @@ class PassphraseMenu(MenuSystem): async def view_edit_phrase(cls, *a): # let them control each character pw = await ux_input_text(cls.pp_sofar, prompt="Your BIP-39 Passphrase", - b39_complete=True, scan_ok=True, max_len=100) + b39_complete=True, scan_ok=True, max_len=MAX_PASS_LEN) if pw is not None: cls.pp_sofar = pw cls.check_length() @@ -1359,8 +1362,8 @@ class PassphraseMenu(MenuSystem): @classmethod def check_length(cls): - # enforce a limit of 100 chars - cls.pp_sofar = cls.pp_sofar[0:100] + # enforce a limit of MAX_PASS_LEN chars + cls.pp_sofar = cls.pp_sofar[0:MAX_PASS_LEN] @classmethod async def add_text(cls, _1, _2, item): diff --git a/shared/utils.py b/shared/utils.py index 780535ef..1f2a4c0b 100644 --- a/shared/utils.py +++ b/shared/utils.py @@ -193,20 +193,28 @@ def str2xfp(txt): # Inverse of xfp2str return ustruct.unpack(' 126: + return False + return True -def to_ascii_printable(s, only_printable=True): +def to_ascii_printable(s, allow_tab_nl=False): try: # s must be a string! - # in relaxed mode allow \n and \t; reject other C0 controls / DEL - extra = b'' if only_printable else b'\t\n' - for o in s.encode('ascii'): - assert 32 <= o <= 126 or (o in extra) + assert len(s) == len(s.encode()) + if not allow_tab_nl: + assert is_printable(s) + else: + for ch in s: + o = ord(ch) + assert 32 <= o <= 126 or o == 9 or o == 10 return s except: - err = "must be ascii printable" + ("" if only_printable else ", tab, or newline") + err = "must be ascii printable" + (", tab, or newline" if allow_tab_nl else "") raise AssertionError(err) - def problem_file_line(exc): # return a string of just the filename.py and line number where # an exception occurred. Best used on AssertionError. diff --git a/testing/test_hobble.py b/testing/test_hobble.py index e3e8acf3..19264c3c 100644 --- a/testing/test_hobble.py +++ b/testing/test_hobble.py @@ -139,8 +139,8 @@ def test_menu_contents(set_hobble, pick_menu_item, cap_menu, en_okeys, en_notes, assert set(m) == fm_expect, "File Mgmt menu wrong" -def test_h_notes(only_q1, set_hobble, pick_menu_item, cap_menu, settings_set, need_some_notes, - sim_exec, settings_remove): +def test_h_notes(only_q1, set_hobble, pick_menu_item, cap_menu, settings_set, + need_some_notes, sim_exec, settings_remove, press_cancel): ''' * load a secure note/pw; check readonly once hobbled * cannot export @@ -161,6 +161,12 @@ def test_h_notes(only_q1, set_hobble, pick_menu_item, cap_menu, settings_set, ne m = cap_menu() assert m == [ '"Title Here"', 'View Note', 'Sign Note Text' ] + set_hobble(True, {'notes', 'okeys'}) + + pick_menu_item('Secure Notes & Passwords') + pick_menu_item('1: Title Here') + assert cap_menu() == ['"Title Here"', 'View Note', 'Sign Note Text', 'Apply as BIP-39 Passphrase'] + # clear notes, should not be offered settings_remove('notes') settings_remove('secnap') diff --git a/testing/test_notes.py b/testing/test_notes.py index 3fd5c473..df61060a 100644 --- a/testing/test_notes.py +++ b/testing/test_notes.py @@ -5,8 +5,11 @@ import pytest, time, json, random, os, pdb from helpers import prandom from charcodes import * -from constants import AF_CLASSIC, AF_P2WPKH_P2SH, AF_P2WPKH +from constants import AF_CLASSIC, AF_P2WPKH_P2SH, AF_P2WPKH, simulator_fixed_words from bbqr import split_qrs +from ckcc.protocol import CCProtocolPacker +from bip32 import BIP32Node +from mnemonic import Mnemonic # All tests in this file are exclusively meant for Q @@ -988,4 +991,146 @@ def test_sign_misc_length(length, settings_set, cap_menu, goto_notes, pick_menu_item(f"2: AB") assert "Sign Note Text" not in cap_menu() + +@pytest.mark.parametrize("pw", [ + "My secret BIP-39 passphrase!!", + "a" * 100, + "secret\n\t", # newline+tab will be stripped + "secret1 ", # space will be stripped + # below, not allowed + "a" * 101, # too long + "aaaaaaa\nbbbbbbbbb", # non-printable ASCII +]) +@pytest.mark.parametrize("sv", [True, False]) # Seed Vault +@pytest.mark.parametrize("pwd", [True, False]) # whether note or password +def test_bip39_passphrase_from_note(dev, need_some_notes, settings_set, goto_notes, pick_menu_item, + cap_story, press_select, cap_menu, reset_seed_words, pw, sv, pwd, + seed_vault_enable, need_keypress, settings_remove): + reset_seed_words() + + settings_remove("seeds") # clear + seed_vault_enable(enable=sv) + + settings_set('notes', []) # clear + title = "A1" + if pwd: + settings_set('notes', [ + {'misc': "some\nrandom\nnote", + 'password': pw, + 'site': 'https://a.com', + 'title': title, + 'user': 'AAA'} + ]) + mi = "Apply as BIP-39 Passphrase" + else: + need_some_notes(title=title, body=pw) + mi = "Apply as BIP-39 Passphrase" + + goto_notes() + pick_menu_item(f"1: {title}") + time.sleep(.1) + + if len(pw) > 100 or "\n" in pw: + # not allowed - must be ASCII 32-127 and length <= 100 + assert mi not in cap_menu() + return # done + + pick_menu_item(mi) + + # firmware rstrips any note before using it + pw = pw.rstrip() + # what it should be + seed = Mnemonic.to_seed(simulator_fixed_words, passphrase=pw) + expect = BIP32Node.from_master_secret(seed) + + time.sleep(.1) + title, story = cap_story() + title_xfp = title[1:-1] + + assert "created by adding passphrase to master seed [0F056943]" in story + assert expect.fingerprint().hex().upper() == title_xfp + + press_select() + time.sleep(.2) + + if sv: + title, story = cap_story() + assert "Press (1) to store temporary seed into Seed Vault" in story + time.sleep(.1) + need_keypress("1") # store it + time.sleep(.1) + title, story = cap_story() + assert "Saved to Seed Vault" in story + assert title_xfp in story + press_select() + + assert title_xfp in cap_menu()[0] + + xpub = dev.send_recv(CCProtocolPacker.get_xpub("m"), timeout=None) + got = BIP32Node.from_wallet_key(xpub) + assert got.sec() == expect.sec() + + +@pytest.mark.parametrize("words", [True, False]) +@pytest.mark.parametrize("pwd", [True, False]) +def test_b39_from_note_eph_seed(words, pwd, generate_ephemeral_words, set_bip39_pw, settings_remove, + reset_seed_words, settings_set, need_some_notes, goto_notes, + pick_menu_item, cap_menu, cap_story, press_select, dev): + reset_seed_words() + settings_remove("seeds") + settings_remove("seedvault") + if words: + e_seed_words = generate_ephemeral_words(num_words=12, seed_vault=False) + e_seed_words = " ".join(e_seed_words) + else: + set_bip39_pw('bdfhjkds', seed_vault=False, reset=False) + + # enabling notes & pwds in temporary settings + settings_set('notes', []) # clear + title = "A1" + pw = "abcdefg" # allowed + if pwd: + settings_set('notes', [ + {'misc': "some\nrandom\nnote", + 'password': pw, + 'site': 'https://a.com', + 'title': title, + 'user': 'AAA'} + ]) + mi = "Apply as BIP-39 Passphrase" + else: + need_some_notes(title=title, body=pw) + mi = "Apply as BIP-39 Passphrase" + + goto_notes() + pick_menu_item(f"1: {title}") + time.sleep(.1) + + if not words: + # no way to apply passphrase on secret that is not word-based + assert mi not in cap_menu() + return # done + + pick_menu_item(mi) + + # what it should be + e_xfp = BIP32Node.from_master_secret(Mnemonic.to_seed(e_seed_words)).fingerprint().hex().upper() + seed = Mnemonic.to_seed(e_seed_words, passphrase=pw) + expect = BIP32Node.from_master_secret(seed) + + time.sleep(.1) + title, story = cap_story() + title_xfp = title[1:-1] + + assert f"created by adding passphrase to current active temporary seed [{e_xfp}]" in story + assert expect.fingerprint().hex().upper() == title_xfp + + press_select() + time.sleep(.2) + + assert title_xfp in cap_menu()[0] + xpub = dev.send_recv(CCProtocolPacker.get_xpub("m"), timeout=None) + got = BIP32Node.from_wallet_key(xpub) + assert got.sec() == expect.sec() + # EOF