feat(core/sdbackup): improve recovery

- recovery does not need filesystem
- so far not possible to use SD with shamir
- export first backup block as a C constant
- WIP

core/embed/extmod/modtrezorio/modtrezorio-fatfs.h

@@ -21,6 +21,7 @@
 #include "py/mperrno.h"
 #include "py/obj.h"
 #include "py/objstr.h"
+#include "stdio.h"
 
 // clang-format off
 #include "ff.h"
@@ -559,10 +560,10 @@ STATIC mp_obj_t mod_trezorio_fatfs_mkfs(size_t n_args, const mp_obj_t *args) {
 
   // create partition
   if (n_args > 0 && args[0] == mp_const_true) {
-    // for SD card backup: we make a small partition and keep the rest
+    // for SD card backup: we make a small FAT32 partition and keep the rest
-    // unallocated
+    // unallocated, FatFS allows smallest size as 0xFFF5 + 550. Windows needs
-    // TODO: seems like not big enough for Windows (problem detected popup)
+    // two more clusters not to complain. MAX_FAT16 + 1 + 551
-    const int n_clusters = 0xFFF5 + 1 + 549;  // MAX_FAT16 + 1 + overhead
+    const int n_clusters = 0xFFF5 + 552;
     make_partition(n_clusters);
   } else {
     // for other use (SD salt): make the partition over the whole space.
@@ -589,14 +590,14 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mod_trezorio_fatfs_mkfs_obj, 0, 1,
 ///     """
 STATIC mp_obj_t mod_trezorio_fatfs_get_capacity() {
   FATFS_ONLY_MOUNTED;
-  /* printf("csize: %d\n", fs_instance.csize); */
+  printf("csize: %d\n", fs_instance.csize);
-  /* printf("fatent: %d\n", fs_instance.n_fatent); */
+  printf("fatent: %d\n", fs_instance.n_fatent);
-  /* printf("free clusters: %d\n", fs_instance.free_clst); */
+  printf("free clusters: %d\n", fs_instance.free_clst);
-  /* printf("volbase: %d\n", fs_instance.volbase); */
+  printf("volbase: %d\n", fs_instance.volbase);
-  /* printf("fatbase: %d\n", fs_instance.fatbase); */
+  printf("fatbase: %d\n", fs_instance.fatbase);
-  /* printf("dirbase: %d\n", fs_instance.dirbase); */
+  printf("dirbase: %d\n", fs_instance.dirbase);
-  /* printf("database: %d\n", fs_instance.database); */
+  printf("database: %d\n", fs_instance.database);
-  /* printf("winsect: %d\n", fs_instance.winsect); */
+  printf("winsect: %d\n", fs_instance.winsect);
   // total number of clusters in the filesystem
   DWORD total_clusters = fs_instance.n_fatent - 2;
   // size of each cluster in bytes

core/embed/extmod/modtrezorio/modtrezorio-sdcard.h

@@ -25,6 +25,7 @@
 /// package: trezorio.sdcard
 
 /// BLOCK_SIZE: int  # size of SD card block
+/// BACKUP_BLOCK_START: int  # first sector for SD seed backup
 
 /// def is_present() -> bool:
 ///     """
@@ -78,7 +79,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_0(mod_trezorio_sdcard_capacity_obj,
 ///     """
 ///     Reads blocks starting with block_num from the SD card into buf.
 ///     Number of bytes read is length of buf rounded down to multiply of
-///     SDCARD_BLOCK_SIZE. 
+///     SDCARD_BLOCK_SIZE.
 ///     """
 STATIC mp_obj_t mod_trezorio_sdcard_read(mp_obj_t block_num, mp_obj_t buf) {
   uint32_t block = trezor_obj_get_uint(block_num);
@@ -97,7 +98,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_2(mod_trezorio_sdcard_read_obj,
 ///     """
 ///     Writes blocks starting with block_num from buf to the SD card.
 ///     Number of bytes written is length of buf rounded down to multiply of
-///     SDCARD_BLOCK_SIZE. 
+///     SDCARD_BLOCK_SIZE.
 ///     """
 STATIC mp_obj_t mod_trezorio_sdcard_write(mp_obj_t block_num, mp_obj_t buf) {
   uint32_t block = trezor_obj_get_uint(block_num);
@@ -124,6 +125,7 @@ STATIC const mp_rom_map_elem_t mod_trezorio_sdcard_globals_table[] = {
     {MP_ROM_QSTR(MP_QSTR_capacity),
      MP_ROM_PTR(&mod_trezorio_sdcard_capacity_obj)},
     {MP_ROM_QSTR(MP_QSTR_BLOCK_SIZE), MP_ROM_INT(SDCARD_BLOCK_SIZE)},
+    {MP_ROM_QSTR(MP_QSTR_BACKUP_BLOCK_START), MP_ROM_INT(SDCARD_BACKUP_BLOCK_START)},
     {MP_ROM_QSTR(MP_QSTR_read), MP_ROM_PTR(&mod_trezorio_sdcard_read_obj)},
     {MP_ROM_QSTR(MP_QSTR_write), MP_ROM_PTR(&mod_trezorio_sdcard_write_obj)},
 };

core/embed/trezorhal/sdcard.h

@@ -50,6 +50,9 @@
 
 // this is a fixed size and should not be changed
 #define SDCARD_BLOCK_SIZE (512)
+// fixed offset for SD seed backup:
+// Maximal size for FAT16 + overhead + start offset (65525 + 552 + 63)
+#define SDCARD_BACKUP_BLOCK_START (66140)
 
 void sdcard_init(void);
 secbool __wur sdcard_power_on(void);

core/mocks/generated/trezorio/sdcard.pyi

@@ -1,5 +1,6 @@
 from typing import *
 BLOCK_SIZE: int  # size of SD card block
+BACKUP_BLOCK_START: int  # first sector for SD seed backup
 
 
 # extmod/modtrezorio/modtrezorio-sdcard.h
@@ -37,7 +38,7 @@ def read(block_num: int, buf: bytearray) -> None:
     """
     Reads blocks starting with block_num from the SD card into buf.
     Number of bytes read is length of buf rounded down to multiply of
-    SDCARD_BLOCK_SIZE. 
+    SDCARD_BLOCK_SIZE.
     """
 
 
@@ -46,5 +47,5 @@ def write(block_num: int, buf: bytes) -> None:
     """
     Writes blocks starting with block_num from buf to the SD card.
     Number of bytes written is length of buf rounded down to multiply of
-    SDCARD_BLOCK_SIZE. 
+    SDCARD_BLOCK_SIZE.
     """

core/src/apps/management/recovery_device/__init__.py

@@ -24,7 +24,6 @@ async def recovery_device(msg: RecoveryDevice) -> Success:
     from trezor.ui.layouts import (
         confirm_action,
         confirm_reset_device,
-        choose_backup_medium,
     )
 
     from apps.common.request_pin import (

core/src/apps/management/recovery_device/homescreen.py

@@ -41,39 +41,57 @@ async def recovery_process() -> Success:
         raise wire.ActionCancelled
 
 
+async def _choose_backup_medium() -> str:
+    from trezor import utils
+
+    if utils.USE_SD_CARD:
+        from apps.management.sd_backup import bip39_choose_backup_medium
+
+        # ask the user for backup type (words/SD card)
+        backup_medium: str = await bip39_choose_backup_medium(recovery=True)
+    else:
+        backup_medium: str = "words"
+    return backup_medium
+
+
 async def _continue_recovery_process() -> Success:
     from trezor import utils
     from trezor.errors import MnemonicError
-    from trezor.ui.layouts import choose_backup_medium
+
+    if utils.USE_SD_CARD:
+        from apps.management.sd_backup import (
+            sdcard_recover_seed,
+        )
 
     # gather the current recovery state from storage
     dry_run = storage_recovery.is_dry_run()
     word_count, backup_type = recover.load_slip39_state()
 
-    # ask the user for backup type (words/SD card)
+    # Both word_count and backup_type are derived from the same data. Both will be
-    backup_medium: str = await choose_backup_medium(recovery=True)
+    # either set or unset. We use 'backup_type is None' to detect status of both.
-    if backup_medium == "sdcard":
+    # The following variable indicates that we are (re)starting the first recovery step,
-        from apps.management.sd_backup import sdcard_recover_seed
+    # which includes word count selection.
+    is_first_step = backup_type is None
 
-        words = await sdcard_recover_seed()
+    if not is_first_step:
-        if not words:
+        assert word_count is not None
-            raise wire.ProcessError("SD card backup could not be recovered.")
+        # If we continue recovery, show starting screen with word count immediately.
-        secret, backup_type = await _process_words(words)
+        await _request_share_first_screen(word_count)
-    else:
-        # Both word_count and backup_type are derived from the same data. Both will be
-        # either set or unset. We use 'backup_type is None' to detect status of both.
-        # The following variable indicates that we are (re)starting the first recovery step,
-        # which includes word count selection.
-        is_first_step = backup_type is None
 
-        if not is_first_step:
+    secret = None
-            assert word_count is not None
+    while secret is None:
-            # If we continue recovery, show starting screen with word count immediately.
+        if is_first_step:
-            await _request_share_first_screen(word_count)
+            backup_medium: str = await _choose_backup_medium()
-
+            if backup_medium == "sdcard":
-        secret = None
+                # attempt to recover words from sd card
-        while secret is None:
+                words = await sdcard_recover_seed()
-            if is_first_step:
+                if words is None:
+                    continue
+                word_count = len(words.split())
+                if word_count not in (12, 24):
+                    await show_warning("Shamir not yet supported for SD")
+                    raise wire.ProcessError("Attempt to recover Shamir from SD card.")
+            else:
                 # If we are starting recovery, ask for word count first...
                 # _request_word_count
                 # For TT, just continuing straight to word count keyboard
@@ -85,23 +103,24 @@ async def _continue_recovery_process() -> Success:
                 word_count = await layout.request_word_count(dry_run)
                 # ...and only then show the starting screen with word count.
                 await _request_share_first_screen(word_count)
-            assert word_count is not None
+        assert word_count is not None
 
+        if backup_medium == "words":
             # ask for mnemonic words one by one
             words = await layout.request_mnemonic(word_count, backup_type)
 
-            # if they were invalid or some checks failed we continue and request them again
+        # if they were invalid or some checks failed we continue and request them again
-            if not words:
+        if not words:
-                continue
+            continue
 
-            try:
+        try:
-                secret, backup_type = await _process_words(words)
+            secret, backup_type = await _process_words(words)
-                # If _process_words succeeded, we now have both backup_type (from
+            # If _process_words succeeded, we now have both backup_type (from
-                # its result) and word_count (from _request_word_count earlier), which means
+            # its result) and word_count (from _request_word_count earlier), which means
-                # that the first step is complete.
+            # that the first step is complete.
-                is_first_step = False
+            is_first_step = False
-            except MnemonicError:
+        except MnemonicError:
-                await layout.show_invalid_mnemonic(word_count)
+            await layout.show_invalid_mnemonic(word_count)
 
     assert backup_type is not None
     if dry_run:

core/src/apps/management/reset_device/__init__.py

@@ -215,18 +215,26 @@ def _compute_secret_from_entropy(
 
 async def _backup_bip39_sdcard(mnemonic: str) -> None:
     from apps.management.sd_backup import sdcard_backup_seed
+
     backup_success: bool = await sdcard_backup_seed(mnemonic)
     if not backup_success:
         raise ProcessError("SD Card backup could not be verified.")
 
 
 async def backup_seed(backup_type: BackupType, mnemonic_secret: bytes) -> None:
+    from trezor import utils
+
     if backup_type == BAK_T_SLIP39_BASIC:
         await _backup_slip39_basic(mnemonic_secret)
     elif backup_type == BAK_T_SLIP39_ADVANCED:
         await _backup_slip39_advanced(mnemonic_secret)
     else:
-        backup_medium: str = await layout.bip39_choose_backup_medium()
+        if utils.USE_SD_CARD:
+            from apps.management.sd_backup import bip39_choose_backup_medium
+
+            backup_medium: str = await bip39_choose_backup_medium()
+        else:
+            backup_medium: str = "words"
         if backup_medium == "sdcard":
             await _backup_bip39_sdcard(mnemonic_secret)
         elif backup_medium == "words":

core/src/apps/management/reset_device/layout.py

@@ -158,12 +158,6 @@ async def show_backup_success() -> None:
 # BIP39
 # ===
 
-async def bip39_choose_backup_medium() -> str:
-    # TODO this will be general, not only for BIP39
-    from trezor.ui.layouts import choose_backup_medium
-
-    return await choose_backup_medium()
-
 
 async def bip39_show_and_confirm_mnemonic(mnemonic: str) -> None:
     # warn user about mnemonic safety

core/src/apps/management/sd_backup.py

@@ -5,6 +5,12 @@ from storage.sd_seed_backup import store_seed_on_sdcard, recover_seed_from_sdcar
 if utils.USE_SD_CARD:
     fatfs = io.fatfs  # global_import_cache
 
+async def bip39_choose_backup_medium(recovery: bool = False) -> str:
+    # TODO this will be general, not only for BIP39
+    from trezor.ui.layouts import choose_backup_medium
+
+    return await choose_backup_medium(recovery)
+
 
 async def sdcard_backup_seed(mnemonic_secret: bytes) -> bool:
     from apps.common.sdcard import ensure_sdcard

core/src/storage/sd_seed_backup.py

@@ -3,20 +3,19 @@ from typing import TYPE_CHECKING
 
 import storage.device
 from trezor import io, utils
-from trezor.sdcard import with_filesystem
+from trezor.sdcard import with_filesystem, with_sdcard
 from trezorcrypto import crc
 
 if utils.USE_SD_CARD:
     fatfs = io.fatfs  # global_import_cache
     sdcard = io.sdcard  # global_import_cache
     SDCARD_BLOCK_SIZE_B = sdcard.BLOCK_SIZE  # global_import_cache
-    SDBACKUP_BLOCK_START = 66_138
+    SDBACKUP_BLOCK_START = sdcard.BACKUP_BLOCK_START  # global_import_cache
     SDBACKUP_BLOCK_OFFSET = 130  # TODO arbitrary for now
-    SDBACKUP_MAGIC = b"TRZS"
+    SDBACKUP_N_WRITINGS = 100  # TODO decide between offset/writings
+    SDBACKUP_MAGIC = b"TRZM"
     SDBACKUP_VERSION = b"00"
 
-# TODO with_filesystem can be just with_sdcard, unnecessary to mount FS for recovery
-
 
 @with_filesystem
 def store_seed_on_sdcard(mnemonic_secret: bytes) -> bool:
@@ -29,12 +28,11 @@ def store_seed_on_sdcard(mnemonic_secret: bytes) -> bool:
         return False
 
 
-@with_filesystem
+@with_sdcard
 def recover_seed_from_sdcard() -> str | None:
     return _read_seed_unalloc()
 
 
-@with_filesystem
 def _verify_backup(mnemonic_secret: bytes) -> bool:
     mnemonic_read_plain = _read_seed_plain_text()
     mnemonic_read_unalloc = _read_seed_unalloc()
@@ -49,15 +47,12 @@ def _verify_backup(mnemonic_secret: bytes) -> bool:
     )
 
 
-@with_filesystem
 def _write_seed_unalloc(mnemonic_secret: bytes) -> None:
     block_to_write = _encode_mnemonic_to_backup_block(mnemonic_secret)
     for block_idx in _storage_blocks_gen():
-        # print(f"block_idx: {block_idx}, writing: {block_to_write[10:10+4]}")
         sdcard.write(block_idx, block_to_write)
 
 
-@with_filesystem
 def _read_seed_unalloc() -> str | None:
     block_buffer = bytearray(SDCARD_BLOCK_SIZE_B)
     for block_idx in _storage_blocks_gen():
@@ -66,28 +61,45 @@ def _read_seed_unalloc() -> str | None:
             mnemonic_read = _decode_mnemonic_from_backup_block(block_buffer)
             if mnemonic_read is not None:
                 break
-        except fatfs.FatFSError:
+        except Exception:
             return None
-    # print(f"_read_seed_unalloc: block_read: {block_read}")
+    if mnemonic_read is None:
+        return None
     mnemonic_read_decoded = mnemonic_read.decode("utf-8").rstrip("\x00")
-    # print(f"_read_seed_unalloc: mnemonic_read_decoded: {mnemonic_read_decoded}")
     return mnemonic_read_decoded
 
 
 def _storage_blocks_gen() -> Generator:
+    return _storage_blocks_gen_by_n()
+
+
+def _storage_blocks_gen_by_offset() -> Generator:
     cap = sdcard.capacity()
     if cap == 0:
         raise ProcessError
     BLOCK_END = cap // SDCARD_BLOCK_SIZE_B - 1
     return range(SDBACKUP_BLOCK_START, BLOCK_END, SDBACKUP_BLOCK_OFFSET)
 
+
+def _storage_blocks_gen_by_n() -> Generator:
+    cap = sdcard.capacity()
+    if cap == 0:
+        raise ProcessError
+    BLOCK_END = cap // SDCARD_BLOCK_SIZE_B - 1
+    return (
+        SDBACKUP_BLOCK_START
+        + n * (BLOCK_END - SDBACKUP_BLOCK_START) // (SDBACKUP_N_WRITINGS - 1)
+        for n in range(SDBACKUP_N_WRITINGS)
+    )
+
+
 """
 Backup Memory Block Layout:
 +----------------------+----------------------+----------------------+-------------------------------+
 | SDBACKUP_MAGIC (4B) | SDBACKUP_VERSION (2B)| SEED_LENGTH (4B)      | MNEMONIC (variable length)    |
 +----------------------+----------------------+----------------------+-------------------------------+
-| CHECKSUM (4B)                                                              | Padding (variable)         |
+| CHECKSUM (4B)                                                         | Padding (variable)         |
-+----------------------------------------------------------------------------+----------------------------+
++-----------------------------------------------------------------------+----------------------------+
 
 - SDBACKUP_MAGIC: 4 bytes magic number identifying the backup block
 - SDBACKUP_VERSION: 2 bytes representing the version of the backup format
@@ -147,13 +159,11 @@ def _decode_mnemonic_from_backup_block(block: bytes) -> bytes | None:
         return None
 
 
-@with_filesystem
 def _write_readme() -> None:
     with fatfs.open("README.txt", "w") as f:
         f.write("This is a Trezor backup SD card.")
 
 
-@with_filesystem
 def _write_seed_plain_text(mnemonic_secret: bytes) -> None:
     # TODO to be removed, just for testing purposes
     fatfs.mkdir("/trezor", True)
@@ -161,7 +171,6 @@ def _write_seed_plain_text(mnemonic_secret: bytes) -> None:
         f.write(mnemonic_secret)
 
 
-@with_filesystem
 def _read_seed_plain_text() -> str | None:
     # TODO to be removed, just for testing purposes
     mnemonic_read = bytearray(SDCARD_BLOCK_SIZE_B)
@@ -170,5 +179,4 @@ def _read_seed_plain_text() -> str | None:
             f.read(mnemonic_read)
     except fatfs.FatFSError:
         return None
-    # print(f"_read_seed_plain_text: mnemonic_read: {mnemonic_read}")
     return mnemonic_read.decode("utf-8").rstrip("\x00")

core/src/trezor/sdcard.py

@@ -26,6 +26,11 @@ if TYPE_CHECKING:
     R = TypeVar("R")
 
 
+def is_trz_card() -> bool:
+    sdcard.capacity()
+    pass
+
+
 class FilesystemWrapper:
     _INSTANCE: "FilesystemWrapper" | None = None
 
@@ -78,3 +83,11 @@ def with_filesystem(func: Callable[P, R]) -> Callable[P, R]:
             return func(*args, **kwargs)
 
     return wrapped_func
+
+
+def with_sdcard(func: Callable[P, R]) -> Callable[P, R]:
+    def wrapped_func(*args: P.args, **kwargs: P.kwargs) -> R:
+        with filesystem(mounted=False):
+            return func(*args, **kwargs)
+
+    return wrapped_func

core/tests/test_storage.sd_seed_backup.py

@@ -1,11 +1,11 @@
 from common import *
 
-from trezorio import sdcard, fatfs
 from storage.sd_seed_backup import *
+from trezor import io, sdcard
 
 
 class TestStorageSdSeedBackup(unittest.TestCase):
-    # TODO add more tests, also with partly damaged backup
+    # TODO add more tests, also for repairing the backup card
 
     def setUp(self):
         self.mnemonic = (
@@ -16,14 +16,44 @@ class TestStorageSdSeedBackup(unittest.TestCase):
         # with self.assertRaises(fatfs.FatFSError):
         #     store_seed_on_sdcard(self.mnemonic.encode("utf-8"))
 
-        sdcard.power_on()
+        io.sdcard.power_on()
-        fatfs.mkfs(True)
+        io.fatfs.mkfs(True)
+        io.fatfs.mount()
+
         success = store_seed_on_sdcard(self.mnemonic.encode("utf-8"))
         self.assertTrue(success)
 
         restored = recover_seed_from_sdcard()
         self.assertEqual(self.mnemonic, restored)
 
+        io.fatfs.unmount()
+        io.sdcard.power_off()
+
+    def test_backup_partlywipe_restore(self):
+        with sdcard.filesystem(mounted=True):
+            success = store_seed_on_sdcard(self.mnemonic.encode("utf-8"))
+            self.assertTrue(success)
+
+        # wipe half of the card, restore must succeed
+        block_buffer = bytearray(SDCARD_BLOCK_SIZE_B)
+        with sdcard.filesystem(mounted=False):
+            for block_num in range((io.sdcard.capacity() // 2) // io.sdcard.BLOCK_SIZE):
+                io.sdcard.write(block_num, block_buffer)
+
+        with sdcard.filesystem(mounted=False):
+            restored = recover_seed_from_sdcard()
+            self.assertEqual(self.mnemonic, restored)
+
+
+        # remove everything, restore fails
+        with sdcard.filesystem(mounted=False):
+            for block_num in range(io.sdcard.capacity() // io.sdcard.BLOCK_SIZE):
+                io.sdcard.write(block_num, block_buffer)
+
+        with sdcard.filesystem(mounted=False):
+            restored = recover_seed_from_sdcard()
+            self.assertEqual(None, restored)
+
 
 if __name__ == "__main__":
     unittest.main()