From f19cbfa718fd61b96fc9426c420a65ae7cf68fcd Mon Sep 17 00:00:00 2001 From: scgbckbone Date: Wed, 27 Mar 2024 17:12:42 +0100 Subject: [PATCH] bugfix: b39pw on tmp when SE secret not set --- releases/QChangeLog.md | 3 ++- shared/actions.py | 15 +++++++++++---- shared/auth.py | 2 +- shared/flow.py | 31 +++++++++++++++++++++---------- shared/multisig.py | 2 +- shared/nvstore.py | 4 ++-- shared/pincodes.py | 12 ++++-------- shared/seed.py | 2 +- shared/stash.py | 2 +- shared/usb.py | 2 ++ testing/test_ux.py | 32 +++++++++++++++++++++----------- 11 files changed, 67 insertions(+), 40 deletions(-) diff --git a/releases/QChangeLog.md b/releases/QChangeLog.md index 19d15e6a..f0daa9f1 100644 --- a/releases/QChangeLog.md +++ b/releases/QChangeLog.md @@ -117,4 +117,5 @@ - Tweak: Default idle timeout when on battery, was reduced to 10 minutes from 30. - Tweak: Cursor movements wrap around if menu is longer than screen height. - Bugfix: fixed `Type Passwords` a.k.a emulated keystrokes -- Tweak: Force default HW settings (USB,NFC,VDisk OFF) after clone/backup restore \ No newline at end of file +- Tweak: Force default HW settings (USB,NFC,VDisk OFF) after clone/backup restore +- Bugfix: Yikes when using BIP39 passphrase with temporary seed without master seed set diff --git a/shared/actions.py b/shared/actions.py index e209c2bc..163c526d 100644 --- a/shared/actions.py +++ b/shared/actions.py @@ -955,7 +955,7 @@ def make_top_menu(): else: assert pa.is_successful(), "nonblank but wrong pin" - if not pa.is_secret_blank(): + if pa.has_secrets(): _cls = NormalSystem[:] if pa.tmp_value: active_xfp = settings.get("xfp", 0) @@ -1486,14 +1486,21 @@ async def list_files(*A): await needs_microsd() return + from pincodes import pa + digest = chk.digest() basename = fn.rsplit('/', 1)[-1] msg_base = 'SHA256(%s)\n\n%s\n\nPress ' % (basename, B2A(digest)) - msg_sign = '(4) to sign file digest and export detached signature' + escape = "6" + if pa.has_secrets(): + msg_sign = '(4) to sign file digest and export detached signature, ' + escape += "4" + else: + msg_sign = "" msg_delete = '(6) to delete.' - msg = msg_base + msg_sign + ", press " + msg_delete + msg = msg_base + msg_sign + msg_delete while True: - ch = await ux_show_story(msg, escape='46') + ch = await ux_show_story(msg, escape=escape) if ch == "x": break if ch in '46': with CardSlot() as card: diff --git a/shared/auth.py b/shared/auth.py index d3b64582..2fcd6f25 100644 --- a/shared/auth.py +++ b/shared/auth.py @@ -1214,7 +1214,7 @@ class NewPassphrase(UserAuthorizedAction): escape += "1" msg += "Press (1) to add passphrase to currently active temporary seed. " - if settings.master_get("words", True): + if not pa.is_secret_blank() and settings.master_get("words", True): escape += "y" + KEY_ENTER msg += "Press OK to add passphrase to master seed. " diff --git a/shared/flow.py b/shared/flow.py index 7c183e56..d247e777 100644 --- a/shared/flow.py +++ b/shared/flow.py @@ -56,10 +56,20 @@ else: # Predicates # -def has_secrets(): +def has_se_secrets(): + # SE secret check, return False if only tmp secret is set from pincodes import pa return not pa.is_secret_blank() +def has_pin(): + from pincodes import pa + return not pa.is_blank() + +def has_secrets(): + # Secret is loaded, may be from SE or tmp + from pincodes import pa + return pa.has_secrets() + def nfc_enabled(): from glob import NFC return bool(NFC) @@ -101,7 +111,7 @@ with the Coldcard.''', LoginPrefsMenu = [ # xxxxxxxxxxxxxxxx MenuItem('Change Main PIN', f=main_pin_changer), - NonDefaultMenuItem('Trick PINs', 'tp', menu=TrickPinMenu.make_menu), + NonDefaultMenuItem('Trick PINs', 'tp', menu=TrickPinMenu.make_menu, predicate=se2_and_real_secret), NonDefaultMenuItem('Set Nickname', 'nick', prelogin=True, f=pick_nickname), NonDefaultMenuItem('Scramble Keys', 'rngk', prelogin=True, f=pick_scramble, default_value=0), NonDefaultMenuItem('Kill Key', 'kbtn', prelogin=True, f=pick_killkey), @@ -202,8 +212,6 @@ DevelopersMenu = [ AdvancedVirginMenu = [ # No PIN, no secrets yet (factory fresh) # xxxxxxxxxxxxxxxx MenuItem("View Identity", f=view_ident), - MenuItem("Temporary Seed", menu=make_ephemeral_seed_menu), - MenuItem('Upgrade Firmware', menu=UpgradeMenu, predicate=is_not_tmp), MenuItem('Paper Wallets', f=make_paper_wallet, predicate=lambda: make_paper_wallet), MenuItem('Perform Selftest', f=start_selftest), MenuItem('Secure Logout', f=logout_now, predicate=lambda: not version.has_battery), @@ -242,7 +250,7 @@ SeedXORMenu = [ SeedFunctionsMenu = [ MenuItem('View Seed Words', f=view_seed_words), # text is a little wrong sometimes, rare MenuItem('Seed XOR', menu=SeedXORMenu), - MenuItem("Destroy Seed", f=clear_seed), + MenuItem("Destroy Seed", f=clear_seed, predicate=se2_and_real_secret), MenuItem('Lock Down Seed', f=convert_ephemeral_to_master), MenuItem('Export SeedQR', f=export_seedqr, predicate=lambda: settings.get('words', True)), @@ -260,13 +268,14 @@ DangerZoneMenu = [ "WARNING: Seed Vault is encrypted (AES-256-CTR) by your seed," " but not held directly inside secure elements. Backups are required" " after any change to vault! Recommended for experiments or temporary use."), - predicate=has_secrets), + predicate=has_se_secrets), MenuItem('Perform Selftest', f=start_selftest), # little harmful MenuItem("Set High-Water", f=set_highwater), MenuItem('Wipe HSM Policy', f=wipe_hsm_policy, predicate=hsm_policy_available), MenuItem('Clear OV cache', f=wipe_ovc), MenuItem("Clear Address cache", f=wipe_address_cache), - ToggleMenuItem("Sighash Checks", "sighshchk", ["Default: Block", "Warn"], invert=True, + ToggleMenuItem("Sighash Checks", "sighshchk", ["Default: Block", "Warn"], + invert=True, story='''\ If you disable sighash flag restrictions, and ignore the \ warnings, funds can be stolen by specially crafted PSBT or MitM. @@ -315,9 +324,11 @@ AdvancedNormalMenu = [ MenuItem("Temporary Seed", menu=make_ephemeral_seed_menu), MenuItem('Paper Wallets', f=make_paper_wallet, predicate=lambda: make_paper_wallet), ToggleMenuItem('Enable HSM', 'hsmcmd', ['Default Off', 'Enable'], - story="Enable HSM? Enables all user management commands, and other HSM-only USB commands. \ -By default these commands are disabled.", predicate=hsm_feature), - MenuItem('User Management', menu=make_users_menu, predicate=hsm_feature), + story=("Enable HSM? Enables all user management commands, and other HSM-only USB commands. " + "By default these commands are disabled."), + predicate=lambda: hsm_feature() and se2_and_real_secret()), + MenuItem('User Management', menu=make_users_menu, + predicate=lambda: hsm_feature() and se2_and_real_secret()), MenuItem('NFC Tools', predicate=nfc_enabled, menu=NFCToolsMenu, shortcut=KEY_NFC), MenuItem("Danger Zone", menu=DangerZoneMenu, shortcut='z'), ] diff --git a/shared/multisig.py b/shared/multisig.py index ee019956..b3acaff0 100644 --- a/shared/multisig.py +++ b/shared/multisig.py @@ -1273,7 +1273,7 @@ async def make_multisig_menu(*a): # list of all multisig wallets, and high-level settings/actions from pincodes import pa - if pa.is_secret_blank(): + if pa.has_secrets(): await ux_show_story("You must have wallet seed before creating multisig wallets.") return diff --git a/shared/nvstore.py b/shared/nvstore.py index 8817ae20..47297825 100644 --- a/shared/nvstore.py +++ b/shared/nvstore.py @@ -103,7 +103,7 @@ class SettingsObject: self.is_dirty = 0 self.my_pos = None - self.nvram_key = nvram_key or b'\0'*32 + self.nvram_key = nvram_key or bytes(32) self.current = self.default_values() @classmethod @@ -144,7 +144,7 @@ class SettingsObject: mine = False if not new_secret: - if not pa.is_successful() or pa.is_secret_blank(): + if not pa.is_successful() or not pa.has_secrets(): # simple fixed key allows us to store a few things when logged out key = bytes(32) else: diff --git a/shared/pincodes.py b/shared/pincodes.py index 2caf91df..00275f93 100644 --- a/shared/pincodes.py +++ b/shared/pincodes.py @@ -323,6 +323,9 @@ class PinAttempt: assert self.is_successful() return bool(self.state_flags & PA_ZERO_SECRET) + def has_secrets(self): + return not self.is_secret_blank() or self.tmp_value + # Mk1/2/3 concepts, not used in Mk4 def has_duress_pin(self): return bool(self.state_flags & PA_HAS_DURESS) @@ -469,11 +472,7 @@ class PinAttempt: settings.merge_previous_active(old_values) except stash.ZeroSecretException: - # secret is zero - using ephemeral secrets in CC - # with blank secure element - settings.nvram_key = b'\0'*32 - settings.load() - self.state_flags |= PA_ZERO_SECRET + settings.return_to_master_seed() def tmp_secret(self, encoded, chain=None, bip39pw=''): # Use indicated secret and stop using the SE; operate like this until reboot @@ -504,9 +503,6 @@ class PinAttempt: self.tmp_value = val - # We're no longer blank. hard to say about duress secret and stuff tho - self.state_flags = PA_SUCCESSFUL - # Copies system settings to new encrypted-key value, calculates # XFP, XPUB and saves into that, and starts using them. self.new_main_secret(self.tmp_value, chain=chain, bip39pw=bip39pw, diff --git a/shared/seed.py b/shared/seed.py index 24ae3af5..222db661 100644 --- a/shared/seed.py +++ b/shared/seed.py @@ -424,7 +424,7 @@ async def add_seed_to_vault(encoded, meta=None): # seed vault disabled return if pa.is_secret_blank(): - # do not save anything if no secrets yet + # do not save anything if no SE secret yet return # do not offer to store secrets that are already in vault diff --git a/shared/stash.py b/shared/stash.py index 12cf56a4..bfb75bb3 100644 --- a/shared/stash.py +++ b/shared/stash.py @@ -186,7 +186,7 @@ class SensitiveValues: # - but that's real slow, so avoid if possible from pincodes import pa - if pa.is_secret_blank(): + if not pa.has_secrets(): raise ZeroSecretException self.deltamode = pa.is_deltamode() diff --git a/shared/usb.py b/shared/usb.py index 80c8a39e..2f5a793e 100644 --- a/shared/usb.py +++ b/shared/usb.py @@ -739,6 +739,7 @@ class USBHandler: from glob import dis, hsm_active from utils import check_firmware_hdr from sigheader import FW_HEADER_OFFSET, FW_HEADER_SIZE, FW_HEADER_MAGIC + from pincodes import pa # maintain a running SHA256 over what's received if offset == 0: @@ -777,6 +778,7 @@ class USBHandler: if prob: raise ValueError(prob) self.is_fw_upgrade = bytes(hdr) + assert not pa.tmp_value, "tmp" if is_trailer and self.is_fw_upgrade: # expect the trailer to exactly match the original one diff --git a/testing/test_ux.py b/testing/test_ux.py index 5387309f..8231b094 100644 --- a/testing/test_ux.py +++ b/testing/test_ux.py @@ -867,10 +867,16 @@ def test_chain_changes_settings_xpub(pick_menu_item, goto_home, cap_story, extended_key = story.split("\n\n")[5] assert extended_key.startswith("tpub") +@pytest.mark.parametrize("clear", [1, 0]) @pytest.mark.parametrize("f_len", [50, 500, 5000]) def test_sign_file_from_list_files(f_len, goto_home, cap_story, pick_menu_item, need_keypress, microsd_path, cap_menu, verify_detached_signature_file, - press_select): + press_select, clear, unit_test, reset_seed_words): + if clear: + unit_test('devtest/clear_seed.py') + else: + reset_seed_words() + fname = "test_sign_listed.pdf" signame = "test_sign_listed.sig" fpath = microsd_path(fname) @@ -889,17 +895,21 @@ def test_sign_file_from_list_files(f_len, goto_home, cap_story, pick_menu_item, _, story = cap_story() assert f"SHA256({fname})" in story assert digest in story - assert "(4) to sign file digest and export detached signature" in story - assert "(6) to delete" in story - need_keypress("4") - time.sleep(0.1) - _, story = cap_story() - assert f"Signature file {signame} written" in story - press_select() - verify_detached_signature_file([fname], signame, "sd", AF_CLASSIC) - _, story = cap_story() - assert "(4) to sign file digest and export detached signature" not in story + if clear: + assert "(4) to sign file digest and export detached signature" not in story + else: + assert "(4) to sign file digest and export detached signature" in story + need_keypress("4") + time.sleep(0.1) + _, story = cap_story() + assert f"Signature file {signame} written" in story + need_keypress("y") + verify_detached_signature_file([fname], signame, "sd", AF_CLASSIC) + _, story = cap_story() + assert "(4) to sign file digest and export detached signature" not in story + assert "(6) to delete" in story + need_keypress("6") menu = cap_menu() assert "List Files" in menu