From 8415f51a7c1da28ca11378868a5ef7541a6036e1 Mon Sep 17 00:00:00 2001 From: obrusvit Date: Tue, 16 Jan 2024 15:21:44 +0100 Subject: [PATCH] feat(core/sdbackup): verify random seed blocks --- core/src/storage/sd_seed_backup.py | 36 +++++++++++-------- .../test_recovery_bip39_sdcard.py | 1 - .../test_reset_recovery_bip39_sdcard.py | 1 - tests/input_flows.py | 4 ++- 4 files changed, 25 insertions(+), 17 deletions(-) diff --git a/core/src/storage/sd_seed_backup.py b/core/src/storage/sd_seed_backup.py index 056f162bb..0c91b27da 100644 --- a/core/src/storage/sd_seed_backup.py +++ b/core/src/storage/sd_seed_backup.py @@ -19,6 +19,8 @@ if utils.USE_SD_CARD: SDCARD_BLOCK_SIZE_B = sdcard.BLOCK_SIZE # global_import_cache SDBACKUP_BLOCK_START = sdcard.BACKUP_BLOCK_START # global_import_cache SDBACKUP_N_WRITINGS = 100 # TODO arbitrary for now + SDBACKUP_N_VERIFY = 10 + assert SDBACKUP_N_WRITINGS > SDBACKUP_N_VERIFY SDBACKUP_MAGIC = b"TRZM" SDBACKUP_VERSION = 0 @@ -29,7 +31,7 @@ class BackupMedium(IntEnum): @with_filesystem -def store_seed_on_sdcard(mnemonic_secret: bytes, backup_type: BackupType): +def store_seed_on_sdcard(mnemonic_secret: bytes, backup_type: BackupType) -> None: _write_seed_unalloc(mnemonic_secret, backup_type) if _verify_backup(mnemonic_secret, backup_type): _write_readme() @@ -49,10 +51,19 @@ def is_backup_present() -> bool: def _verify_backup(mnemonic_secret: bytes, backup_type: BackupType) -> bool: - decoded_mnemonic, decoded_backup_type = _read_seed_unalloc() - if decoded_mnemonic is None or decoded_backup_type is None: - return False - return decoded_mnemonic == mnemonic_secret and decoded_backup_type == backup_type + from trezor.crypto import random + + block_buffer = bytearray(SDCARD_BLOCK_SIZE_B) + all_backup_blocks = list(_storage_blocks_gen()) + for _ in range(SDBACKUP_N_VERIFY): + block_idx = random.uniform(len(all_backup_blocks)) + sdcard.read(all_backup_blocks[block_idx], block_buffer) + decoded_mnemonic, decoded_backup_type = _decode_backup_block(block_buffer) + if decoded_mnemonic is None or decoded_backup_type is None: + return False + if decoded_mnemonic != mnemonic_secret or decoded_backup_type != backup_type: + return False + return True def _write_seed_unalloc(mnemonic_secret: bytes, backup_type: BackupType) -> None: @@ -63,18 +74,15 @@ def _write_seed_unalloc(mnemonic_secret: bytes, backup_type: BackupType) -> None def _read_seed_unalloc() -> tuple[bytes | None, BackupType | None]: block_buffer = bytearray(SDCARD_BLOCK_SIZE_B) - restored_block = None + (decoded_mnemonic, decoded_backup_type) = (None, None) for block_idx in _storage_blocks_gen(): try: sdcard.read(block_idx, block_buffer) - restored_block = _decode_backup_block(block_buffer) - if restored_block is not None: + decoded_mnemonic, decoded_backup_type = _decode_backup_block(block_buffer) + if (decoded_mnemonic, decoded_backup_type) != (None, None): break except Exception: return (None, None) - if restored_block is None: - return (None, None) - decoded_mnemonic, decoded_backup_type = restored_block return (decoded_mnemonic, decoded_backup_type) @@ -130,14 +138,14 @@ def _encode_backup_block(mnemonic: bytes, backup_type: BackupType) -> bytes: return bytes(ret) -def _decode_backup_block(block: bytes) -> tuple[bytes, BackupType] | None: +def _decode_backup_block(block: bytes) -> tuple[bytes | None, BackupType | None]: from trezor.enums import BackupType assert len(block) == SDCARD_BLOCK_SIZE_B try: r = utils.BufferReader(block) if r.read_memoryview(MAGIC_LEN) != SDBACKUP_MAGIC: - return None + return (None, None) r.read_memoryview(VERSION_LEN) # skip the version for now backup_type = int.from_bytes(r.read_memoryview(BACKUPTYPE_LEN), "big") seed_len = int.from_bytes(r.read_memoryview(SEEDLEN_LEN), "big") @@ -156,7 +164,7 @@ def _decode_backup_block(block: bytes) -> tuple[bytes, BackupType] | None: ): return (mnemonic, backup_type) else: - return None + return (None, None) except (ValueError, EOFError): raise DataError("Trying to decode invalid SD card block.") diff --git a/tests/device_tests/reset_recovery/test_recovery_bip39_sdcard.py b/tests/device_tests/reset_recovery/test_recovery_bip39_sdcard.py index e4bb5c13a..75ff727cc 100644 --- a/tests/device_tests/reset_recovery/test_recovery_bip39_sdcard.py +++ b/tests/device_tests/reset_recovery/test_recovery_bip39_sdcard.py @@ -2,7 +2,6 @@ import pytest from trezorlib import device, messages from trezorlib.debuglink import TrezorClientDebugLink as Client -from trezorlib.messages import BackupType from ...common import MNEMONIC12 from ...input_flows import InputFlowBip39RecoverySdCard diff --git a/tests/device_tests/reset_recovery/test_reset_recovery_bip39_sdcard.py b/tests/device_tests/reset_recovery/test_reset_recovery_bip39_sdcard.py index 0d913b6ac..0f1e29047 100644 --- a/tests/device_tests/reset_recovery/test_reset_recovery_bip39_sdcard.py +++ b/tests/device_tests/reset_recovery/test_reset_recovery_bip39_sdcard.py @@ -49,7 +49,6 @@ def reset(client: Client, strength: int = 128, skip_backup: bool = False) -> Non assert client.features.passphrase_protection is False - def recover(client: Client): with client: IF = InputFlowBip39RecoverySdCard(client) diff --git a/tests/input_flows.py b/tests/input_flows.py index 5bf276454..6d97ca833 100644 --- a/tests/input_flows.py +++ b/tests/input_flows.py @@ -1578,7 +1578,9 @@ class InputFlowSlip39BasicRecovery(InputFlowBase): class InputFlowSlip39BasicRecoverySdCard(InputFlowBase): - def __init__(self, client: Client, sdcard_numbers: list[int], pin: str | None = None): + def __init__( + self, client: Client, sdcard_numbers: list[int], pin: str | None = None + ): super().__init__(client) self.sdcard_numbers = sdcard_numbers self.pin = pin