From ffe07f2ca6231d9f9bb06f132ee0ea65294b26e8 Mon Sep 17 00:00:00 2001 From: matejcik Date: Wed, 5 Jun 2024 17:41:40 +0200 Subject: [PATCH] feat(core): improve repeated backup * allow upgrading from Single to Basic * do not skip confirmation screen when sending BackupDevice from Suite --- core/src/apps/management/backup_device.py | 99 ++++++++++++++----- .../management/recovery_device/homescreen.py | 21 +--- core/src/storage/device.py | 4 + tests/common.py | 3 + tests/device_tests/test_repeated_backup.py | 49 ++++++++- tests/input_flows.py | 33 ++++++- 6 files changed, 161 insertions(+), 48 deletions(-) diff --git a/core/src/apps/management/backup_device.py b/core/src/apps/management/backup_device.py index 243e50de02..0a6c461190 100644 --- a/core/src/apps/management/backup_device.py +++ b/core/src/apps/management/backup_device.py @@ -1,63 +1,108 @@ from typing import TYPE_CHECKING +import storage.device as storage_device from trezor.enums import BackupType if TYPE_CHECKING: + from typing import Sequence + from trezor.messages import BackupDevice, Success -BAK_T_BIP39 = BackupType.Bip39 # global_import_cache - - -async def backup_device(msg: BackupDevice) -> Success: - import storage.device as storage_device - from trezor import wire - from trezor.messages import Success +async def perform_backup( + is_repeated_backup: bool, + group_threshold: int | None = None, + groups: Sequence[tuple[int, int]] = (), +) -> None: + from trezor import TR + from trezor.enums import ButtonRequestType + from trezor.ui.layouts import confirm_action + from trezor.utils import ensure from apps.common import backup, backup_types, mnemonic from .reset_device import backup_seed, backup_slip39_custom, layout + # Ask the user to confirm backup. The user can still escape here. + if is_repeated_backup: + await confirm_action( + "confirm_repeated_backup", + TR.recovery__title_unlock_repeated_backup, + description=TR.recovery__unlock_repeated_backup, + br_code=ButtonRequestType.ProtectCall, + verb=TR.recovery__unlock_repeated_backup_verb, + ) + + mnemonic_secret = mnemonic.get_secret() + ensure(mnemonic_secret is not None) # checked at run-time + assert mnemonic_secret is not None # checked at type-check time + backup_type = mnemonic.get_type() + + # upgrade Single to Basic if necessary + if is_repeated_backup and backup_type == BackupType.Slip39_Single_Extendable: + # TODO upgrade to Advanced if appropriate + backup_type = BackupType.Slip39_Basic_Extendable + storage_device.set_backup_type(backup_type) + + # set unfinished flag -- if the process gets interrupted, the unfinished flag stays + if not is_repeated_backup: + storage_device.set_unfinished_backup(True) + + # Deactivate repeated backup, set backed up flag, before showing anything to the + # user. If anything bad happens from now on, the backup counts as "already done". + backup.deactivate_repeated_backup() + storage_device.set_backed_up() + + if group_threshold is not None: + # Parameters provided from host side. + assert backup_types.is_slip39_backup_type(backup_type) + extendable = backup_types.is_extendable_backup_type(backup_type) + # Run the backup process directly. + await backup_slip39_custom(mnemonic_secret, group_threshold, groups, extendable) + else: + # No parameters provided, allow the user to configure them on screen. + await backup_seed(backup_type, mnemonic_secret) + + # If the backup was successful, clear the unfinished flag and show success. + + # (NOTE that if the user manages to enable repeated backup while unfinished flag is + # set, the unfinished flag is cleared here. That is the correct thing to do -- the + # user _has_ finished the backup because they were able to unlock the repeated + # backup -- and now they finished another one.) + storage_device.set_unfinished_backup(False) + await layout.show_backup_success() + + +async def backup_device(msg: BackupDevice) -> Success: + from trezor import wire + from trezor.messages import Success + + from apps.common import backup, mnemonic + # do this early before we show any UI # the homescreen will clear the flag right after its own UI is gone repeated_backup_enabled = backup.repeated_backup_enabled() + is_repeated_backup = repeated_backup_enabled and not storage_device.needs_backup() if not storage_device.is_initialized(): raise wire.NotInitialized("Device is not initialized") if not storage_device.needs_backup() and not repeated_backup_enabled: raise wire.ProcessError("Seed already backed up") - mnemonic_secret, backup_type = mnemonic.get() - if mnemonic_secret is None: - raise RuntimeError - group_threshold = msg.group_threshold groups = [(g.member_threshold, g.member_count) for g in msg.groups] + # validate host-side SLIP39 parameters if group_threshold is not None: if group_threshold < 1: raise wire.DataError("group_threshold must be a positive integer") if len(groups) < group_threshold: raise wire.DataError("Not enough groups provided for group_threshold") - if backup_type == BAK_T_BIP39: + if mnemonic.is_bip39(): raise wire.ProcessError("Expected SLIP39 backup") elif len(groups) > 0: raise wire.DataError("group_threshold is missing") - if not repeated_backup_enabled: - storage_device.set_unfinished_backup(True) - - backup.deactivate_repeated_backup() - storage_device.set_backed_up() - - if group_threshold is not None: - extendable = backup_types.is_extendable_backup_type(backup_type) - await backup_slip39_custom(mnemonic_secret, group_threshold, groups, extendable) - else: - await backup_seed(backup_type, mnemonic_secret) - - storage_device.set_unfinished_backup(False) - - await layout.show_backup_success() + await perform_backup(is_repeated_backup, group_threshold, groups) return Success(message="Seed successfully backed up") diff --git a/core/src/apps/management/recovery_device/homescreen.py b/core/src/apps/management/recovery_device/homescreen.py index 3455ccab77..717a62cdc6 100644 --- a/core/src/apps/management/recovery_device/homescreen.py +++ b/core/src/apps/management/recovery_device/homescreen.py @@ -52,11 +52,10 @@ async def recovery_process() -> Success: async def _continue_repeated_backup() -> None: - from trezor.enums import ButtonRequestType, MessageType - from trezor.ui.layouts import confirm_action + from trezor.enums import MessageType - from apps.common import backup, mnemonic - from apps.management.reset_device import backup_seed + from apps.common import backup + from apps.management.backup_device import perform_backup wire.AVOID_RESTARTING_FOR = ( MessageType.Initialize, @@ -65,19 +64,7 @@ async def _continue_repeated_backup() -> None: ) try: - await confirm_action( - "confirm_repeated_backup", - TR.recovery__title_unlock_repeated_backup, - description=TR.recovery__unlock_repeated_backup, - br_code=ButtonRequestType.ProtectCall, - verb=TR.recovery__unlock_repeated_backup_verb, - ) - - mnemonic_secret, backup_type = mnemonic.get() - if mnemonic_secret is None: - raise RuntimeError - - await backup_seed(backup_type, mnemonic_secret) + await perform_backup(is_repeated_backup=True) finally: backup.deactivate_repeated_backup() diff --git a/core/src/storage/device.py b/core/src/storage/device.py index eef8e648c3..1b84f1bfdc 100644 --- a/core/src/storage/device.py +++ b/core/src/storage/device.py @@ -146,6 +146,10 @@ def get_backup_type() -> BackupType: return backup_type +def set_backup_type(backup_type: BackupType) -> None: + common.set_uint8(_NAMESPACE, _BACKUP_TYPE, backup_type) + + def is_passphrase_enabled() -> bool: return common.get_bool(_NAMESPACE, _USE_PASSPHRASE) diff --git a/tests/common.py b/tests/common.py index 6502faf591..b270f46e33 100644 --- a/tests/common.py +++ b/tests/common.py @@ -67,6 +67,9 @@ MNEMONIC_SLIP39_ADVANCED_33 = [ ] MNEMONIC_SLIP39_CUSTOM_1of1 = ["tolerate flexible academic academic average dwarf square home promise aspect temple cluster roster forward hand unfair tenant emperor ceramic element forget perfect knit adapt review usual formal receiver typical pleasure duke yield party"] MNEMONIC_SLIP39_CUSTOM_SECRET = "3439316237393562383066633231636364663436366330666263393863386663" + +MNEMONIC_SLIP39_SINGLE_EXT_20 = ["academic again academic academic academic academic academic academic academic academic academic academic academic academic academic academic academic pecan provide remember"] + # External entropy mocked as received from trezorlib. EXTERNAL_ENTROPY = b"zlutoucky kun upel divoke ody" * 2 # fmt: on diff --git a/tests/device_tests/test_repeated_backup.py b/tests/device_tests/test_repeated_backup.py index d236a78145..87c0c7f8f3 100644 --- a/tests/device_tests/test_repeated_backup.py +++ b/tests/device_tests/test_repeated_backup.py @@ -21,7 +21,12 @@ from trezorlib import device, messages from trezorlib.debuglink import TrezorClientDebugLink as Client from trezorlib.exceptions import Cancelled, TrezorFailure -from ..common import TEST_ADDRESS_N, WITH_MOCK_URANDOM, MNEMONIC_SLIP39_BASIC_20_3of6 +from ..common import ( + MNEMONIC_SLIP39_SINGLE_EXT_20, + TEST_ADDRESS_N, + WITH_MOCK_URANDOM, + MNEMONIC_SLIP39_BASIC_20_3of6, +) from ..input_flows import InputFlowSlip39BasicBackup, InputFlowSlip39BasicRecoveryDryRun @@ -65,7 +70,7 @@ def test_repeated_backup(client: Client): # we can now perform another backup with client: - IF = InputFlowSlip39BasicBackup(client, False) + IF = InputFlowSlip39BasicBackup(client, False, repeated=True) client.set_input_flow(IF.get()) device.backup(client) @@ -78,6 +83,46 @@ def test_repeated_backup(client: Client): device.backup(client) +@pytest.mark.setup_client(mnemonic=MNEMONIC_SLIP39_SINGLE_EXT_20) +@pytest.mark.skip_t1b1 +@WITH_MOCK_URANDOM +def test_repeated_backup_upgrade_single(client: Client): + assert ( + client.features.backup_availability == messages.BackupAvailability.NotAvailable + ) + assert client.features.recovery_status == messages.RecoveryStatus.Nothing + assert client.features.backup_type == messages.BackupType.Slip39_Single_Extendable + + # unlock repeated backup by entering the single share + with client: + IF = InputFlowSlip39BasicRecoveryDryRun( + client, MNEMONIC_SLIP39_SINGLE_EXT_20, unlock_repeated_backup=True + ) + client.set_input_flow(IF.get()) + ret = device.recover(client, type=messages.RecoveryType.UnlockRepeatedBackup) + assert ret == messages.Success(message="Backup unlocked") + assert ( + client.features.backup_availability == messages.BackupAvailability.Available + ) + assert client.features.recovery_status == messages.RecoveryStatus.Backup + + # we can now perform another backup + with client: + IF = InputFlowSlip39BasicBackup(client, False, repeated=True) + client.set_input_flow(IF.get()) + device.backup(client) + + # backup type was upgraded: + assert client.features.backup_type == messages.BackupType.Slip39_Basic_Extendable + # the backup feature is locked again... + assert ( + client.features.backup_availability == messages.BackupAvailability.NotAvailable + ) + assert client.features.recovery_status == messages.RecoveryStatus.Nothing + with pytest.raises(TrezorFailure, match=r".*Seed already backed up"): + device.backup(client) + + @pytest.mark.setup_client(needs_backup=True, mnemonic=MNEMONIC_SLIP39_BASIC_20_3of6) @pytest.mark.skip_t1b1 @WITH_MOCK_URANDOM diff --git a/tests/input_flows.py b/tests/input_flows.py index 275a6442c0..73c2832cb4 100644 --- a/tests/input_flows.py +++ b/tests/input_flows.py @@ -1421,12 +1421,18 @@ def load_N_shares( class InputFlowSlip39BasicBackup(InputFlowBase): - def __init__(self, client: Client, click_info: bool): + def __init__(self, client: Client, click_info: bool, repeated: bool = False): super().__init__(client) self.mnemonics: list[str] = [] self.click_info = click_info + self.repeated = repeated def input_flow_tt(self) -> BRGeneratorType: + if self.repeated: + # intro confirmation screen + yield + self.debug.press_yes() + yield # 1. Backup intro self.debug.press_yes() yield # 2. Checklist @@ -1454,6 +1460,11 @@ class InputFlowSlip39BasicBackup(InputFlowBase): self.debug.press_yes() def input_flow_tr(self) -> BRGeneratorType: + if self.repeated: + # intro confirmation screen + yield + self.debug.press_yes() + yield # 1. Backup intro self.debug.press_yes() yield # 2. Checklist @@ -1481,6 +1492,11 @@ class InputFlowSlip39BasicBackup(InputFlowBase): self.debug.press_yes() def input_flow_t3t1(self) -> BRGeneratorType: + if self.repeated: + # intro confirmation screen + yield + self.debug.press_yes() + yield # 1. Backup intro self.debug.wait_layout() self.debug.swipe_up() @@ -1587,12 +1603,17 @@ class InputFlowSlip39BasicResetRecovery(InputFlowBase): class InputFlowSlip39CustomBackup(InputFlowBase): - def __init__(self, client: Client, share_count: int): + def __init__(self, client: Client, share_count: int, repeated: bool = False): super().__init__(client) self.mnemonics: list[str] = [] self.share_count = share_count + self.repeated = repeated def input_flow_tt(self) -> BRGeneratorType: + if self.repeated: + yield + self.debug.press_yes() + if self.share_count > 1: yield # Checklist self.debug.press_yes() @@ -1611,6 +1632,10 @@ class InputFlowSlip39CustomBackup(InputFlowBase): self.debug.press_yes() def input_flow_tr(self) -> BRGeneratorType: + if self.repeated: + yield + self.debug.press_yes() + if self.share_count > 1: yield # Checklist self.debug.press_yes() @@ -1629,6 +1654,10 @@ class InputFlowSlip39CustomBackup(InputFlowBase): self.debug.press_yes() def input_flow_t3t1(self) -> BRGeneratorType: + if self.repeated: + yield + self.debug.press_yes() + if self.share_count > 1: yield # Checklist self.debug.press_yes()