From 39b4376b6543ae76a543e8ae7cf12d62d37facc1 Mon Sep 17 00:00:00 2001 From: Andrew Kozlik Date: Fri, 18 Oct 2019 13:53:00 +0200 Subject: [PATCH 1/2] core/sd-protect: If writing to the SD card fails in request_sd_salt(), inform the user and allow them to retry or abort. --- core/src/apps/common/sd_salt.py | 83 ++++++++++++++++----------------- 1 file changed, 41 insertions(+), 42 deletions(-) diff --git a/core/src/apps/common/sd_salt.py b/core/src/apps/common/sd_salt.py index 2b5b784dc..b0e06d69b 100644 --- a/core/src/apps/common/sd_salt.py +++ b/core/src/apps/common/sd_salt.py @@ -30,7 +30,7 @@ async def _wrong_card_dialog(ctx: Optional[wire.Context]) -> None: text.br_half() if SD_CARD_HOT_SWAPPABLE: text.normal("Please insert the", "correct SD card for", "this device.") - btn_confirm = "Retry" + btn_confirm = "Retry" # type: Optional[str] btn_cancel = "Abort" else: text.normal("Please unplug the", "device and insert the", "correct SD card.") @@ -51,7 +51,7 @@ async def _insert_card_dialog(ctx: Optional[wire.Context]) -> None: text.br_half() if SD_CARD_HOT_SWAPPABLE: text.normal("Please insert your", "SD card.") - btn_confirm = "Retry" + btn_confirm = "Retry" # type: Optional[str] btn_cancel = "Abort" else: text.normal("Please unplug the", "device and insert your", "SD card.") @@ -88,6 +88,27 @@ def _get_salt_path(new: bool = False) -> str: return "%s/salt" % _get_device_dir() +def _load_salt(fs: io.FatFS, auth_key: bytes, path: str) -> Optional[bytearray]: + # Load the salt file if it exists. + try: + with fs.open(path, "r") as f: + salt = bytearray(SD_SALT_LEN_BYTES) + stored_tag = bytearray(SD_SALT_AUTH_TAG_LEN_BYTES) + f.read(salt) + f.read(stored_tag) + except OSError: + return None + + # Check the salt's authentication tag. + computed_tag = hmac.new(auth_key, salt, sha256).digest()[ + :SD_SALT_AUTH_TAG_LEN_BYTES + ] + if not consteq(computed_tag, stored_tag): + return None + + return salt + + async def request_sd_salt( ctx: Optional[wire.Context], salt_auth_key: bytes ) -> bytearray: @@ -102,56 +123,34 @@ async def request_sd_salt( try: fs.mount() - - # Load salt if it exists. - salt = None # type: Optional[bytearray] - try: - with fs.open(salt_path, "r") as f: - salt = bytearray(SD_SALT_LEN_BYTES) - salt_tag = bytearray(SD_SALT_AUTH_TAG_LEN_BYTES) - f.read(salt) - f.read(salt_tag) - except OSError: - salt = None - - if salt is not None and consteq( - hmac.new(salt_auth_key, salt, sha256).digest()[ - :SD_SALT_AUTH_TAG_LEN_BYTES - ], - salt_tag, - ): + salt = _load_salt(fs, salt_auth_key, salt_path) + if salt is not None: return salt - # Load salt.new if it exists. - new_salt = None # type: Optional[bytearray] - try: - with fs.open(new_salt_path, "r") as f: - new_salt = bytearray(SD_SALT_LEN_BYTES) - new_salt_tag = bytearray(SD_SALT_AUTH_TAG_LEN_BYTES) - f.read(new_salt) - f.read(new_salt_tag) - except OSError: - new_salt = None - - if new_salt is not None and consteq( - hmac.new(salt_auth_key, new_salt, sha256).digest()[ - :SD_SALT_AUTH_TAG_LEN_BYTES - ], - new_salt_tag, - ): + # Check if there is a new salt. + salt = _load_salt(fs, salt_auth_key, new_salt_path) + if salt is not None: # SD salt regeneration was interrupted earlier. Bring into consistent state. # TODO Possibly overwrite salt file with random data. try: fs.unlink(salt_path) except OSError: pass - fs.rename(new_salt_path, salt_path) - return new_salt + + try: + fs.rename(new_salt_path, salt_path) + except OSError: + error_dialog = _write_failed_dialog(ctx) + else: + return salt + else: + # No valid salt file on this SD card. + error_dialog = _wrong_card_dialog(ctx) finally: fs.unmount() sd.power(False) - await _wrong_card_dialog(ctx) + await error_dialog async def set_sd_salt( @@ -195,7 +194,7 @@ async def commit_sd_salt(ctx: Optional[wire.Context]) -> None: sd = io.SDCard() fs = io.FatFS() if not sd.power(True): - raise IOError + raise OSError try: fs.mount() @@ -216,7 +215,7 @@ async def remove_sd_salt(ctx: Optional[wire.Context]) -> None: sd = io.SDCard() fs = io.FatFS() if not sd.power(True): - raise IOError + raise OSError try: fs.mount() From 60f6ab908767616ed18287d6653dc311ab06af6b Mon Sep 17 00:00:00 2001 From: Andrew Kozlik Date: Fri, 18 Oct 2019 14:36:40 +0200 Subject: [PATCH 2/2] core: Fix mypy warnings. --- core/src/apps/common/confirm.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/apps/common/confirm.py b/core/src/apps/common/confirm.py index 7b701a3c6..06389ffd2 100644 --- a/core/src/apps/common/confirm.py +++ b/core/src/apps/common/confirm.py @@ -8,7 +8,7 @@ if __debug__: from apps.debug import confirm_signal if False: - from typing import Any, Callable + from typing import Any, Callable, Optional from trezor import ui from trezor.ui.confirm import ButtonContent, ButtonStyleType from trezor.ui.loader import LoaderStyleType @@ -18,9 +18,9 @@ async def confirm( ctx: wire.Context, content: ui.Component, code: int = ButtonRequestType.Other, - confirm: ButtonContent = Confirm.DEFAULT_CONFIRM, + confirm: Optional[ButtonContent] = Confirm.DEFAULT_CONFIRM, confirm_style: ButtonStyleType = Confirm.DEFAULT_CONFIRM_STYLE, - cancel: ButtonContent = Confirm.DEFAULT_CANCEL, + cancel: Optional[ButtonContent] = Confirm.DEFAULT_CANCEL, cancel_style: ButtonStyleType = Confirm.DEFAULT_CANCEL_STYLE, major_confirm: bool = False, ) -> bool: