diff --git a/core/CHANGELOG.md b/core/CHANGELOG.md index a4ed2630b..bf071462a 100644 --- a/core/CHANGELOG.md +++ b/core/CHANGELOG.md @@ -8,8 +8,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Added - CoinJoin preauthorization and signing flow. [#1053] +- Value of the `safety-checks` setting to the `Features` message. [#1193] ### Changed +- The `safety-checks` setting gained new possible value `PromptTemporarily` which overrides safety checks until device reboot. [#1133] ### Deprecated @@ -305,10 +307,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). [#1115]: https://github.com/trezor/trezor-firmware/issues/1115 [#1118]: https://github.com/trezor/trezor-firmware/issues/1118 [#1126]: https://github.com/trezor/trezor-firmware/issues/1126 +[#1133]: https://github.com/trezor/trezor-firmware/issues/1133 [#1139]: https://github.com/trezor/trezor-firmware/issues/1139 [#1159]: https://github.com/trezor/trezor-firmware/issues/1159 [#1165]: https://github.com/trezor/trezor-firmware/pull/1165 [#1173]: https://github.com/trezor/trezor-firmware/pull/1173 [#1188]: https://github.com/trezor/trezor-firmware/issues/1188 [#1190]: https://github.com/trezor/trezor-firmware/issues/1190 +[#1193]: https://github.com/trezor/trezor-firmware/issues/1193 [#1246]: https://github.com/trezor/trezor-firmware/issues/1246 diff --git a/core/src/apps/base.py b/core/src/apps/base.py index 0995f7403..6b7ffedf3 100644 --- a/core/src/apps/base.py +++ b/core/src/apps/base.py @@ -7,10 +7,9 @@ from trezor import config, sdcard, utils, wire, workflow from trezor.messages import Capability, MessageType from trezor.messages.Features import Features from trezor.messages.PreauthorizedRequest import PreauthorizedRequest -from trezor.messages.SafetyCheckLevel import Prompt, Strict from trezor.messages.Success import Success -from apps.common import mnemonic +from apps.common import mnemonic, safety_checks from apps.common.request_pin import verify_user_pin if False: @@ -93,7 +92,7 @@ def get_features() -> Features: f.sd_protection = storage.sd_salt.is_enabled() f.wipe_code_protection = config.has_wipe_code() f.passphrase_always_on_device = storage.device.get_passphrase_always_on_device() - f.safety_checks = Prompt if storage.device.unsafe_prompts_allowed() else Strict + f.safety_checks = safety_checks.read_setting() return f diff --git a/core/src/apps/bitcoin/sign_tx/approvers.py b/core/src/apps/bitcoin/sign_tx/approvers.py index 13bb313ad..e97a6d8b6 100644 --- a/core/src/apps/bitcoin/sign_tx/approvers.py +++ b/core/src/apps/bitcoin/sign_tx/approvers.py @@ -1,12 +1,11 @@ from micropython import const -from storage import device from trezor import wire from trezor.messages.SignTx import SignTx from trezor.messages.TxInputType import TxInputType from trezor.messages.TxOutputType import TxOutputType -from apps.common import coininfo +from apps.common import coininfo, safety_checks from .. import addresses from ..authorization import FEE_PER_ANONYMITY_DECIMALS @@ -100,7 +99,7 @@ class BasicApprover(Approver): # fee > (coin.maxfee per byte * tx size) if fee > fee_threshold: - if fee > 10 * fee_threshold and not device.unsafe_prompts_allowed(): + if fee > 10 * fee_threshold and safety_checks.is_strict(): raise wire.DataError("The fee is unexpectedly large") await helpers.confirm_feeoverthreshold(fee, self.coin) if self.change_count > self.MAX_SILENT_CHANGE_COUNT: diff --git a/core/src/apps/common/keychain.py b/core/src/apps/common/keychain.py index 23ebe7a8d..6fc7cbec3 100644 --- a/core/src/apps/common/keychain.py +++ b/core/src/apps/common/keychain.py @@ -1,8 +1,7 @@ -from storage import device from trezor import wire from trezor.crypto import bip32 -from . import HARDENED, paths +from . import HARDENED, paths, safety_checks from .seed import Slip21Node, get_seed if False: @@ -105,7 +104,7 @@ class Keychain: if "ed25519" in self.curve and not paths.path_is_hardened(path): raise wire.DataError("Non-hardened paths unsupported on Ed25519") - if device.unsafe_prompts_allowed(): + if not safety_checks.is_strict(): return if any(ns == path[: len(ns)] for ns in self.namespaces): @@ -136,7 +135,7 @@ class Keychain: ) def derive_slip21(self, path: paths.Slip21Path) -> Slip21Node: - if not device.unsafe_prompts_allowed() and not any( + if safety_checks.is_strict() and not any( ns == path[: len(ns)] for ns in self.slip21_namespaces ): raise FORBIDDEN_KEY_PATH diff --git a/core/src/apps/common/safety_checks.py b/core/src/apps/common/safety_checks.py new file mode 100644 index 000000000..396ab0a46 --- /dev/null +++ b/core/src/apps/common/safety_checks.py @@ -0,0 +1,52 @@ +import storage.cache +import storage.device +from storage.cache import APP_COMMON_SAFETY_CHECKS_TEMPORARY +from storage.device import SAFETY_CHECK_LEVEL_PROMPT, SAFETY_CHECK_LEVEL_STRICT +from trezor.messages import SafetyCheckLevel + +if False: + from typing import Optional + from trezor.messages.ApplySettings import EnumTypeSafetyCheckLevel + + +def read_setting() -> EnumTypeSafetyCheckLevel: + """ + Returns the effective safety check level. + """ + temporary_safety_check_level = storage.cache.get( + APP_COMMON_SAFETY_CHECKS_TEMPORARY + ) # type: Optional[EnumTypeSafetyCheckLevel] + if temporary_safety_check_level is not None: + return temporary_safety_check_level + else: + stored = storage.device.safety_check_level() + if stored == SAFETY_CHECK_LEVEL_STRICT: + return SafetyCheckLevel.Strict + elif stored == SAFETY_CHECK_LEVEL_PROMPT: + return SafetyCheckLevel.PromptAlways + else: + raise ValueError("Unknown SafetyCheckLevel") + + +def apply_setting(level: EnumTypeSafetyCheckLevel) -> None: + """ + Changes the safety level settings. + """ + if level == SafetyCheckLevel.Strict: + storage.cache.delete(APP_COMMON_SAFETY_CHECKS_TEMPORARY) + storage.device.set_safety_check_level(SAFETY_CHECK_LEVEL_STRICT) + elif level == SafetyCheckLevel.PromptAlways: + storage.cache.delete(APP_COMMON_SAFETY_CHECKS_TEMPORARY) + storage.device.set_safety_check_level(SAFETY_CHECK_LEVEL_PROMPT) + elif level == SafetyCheckLevel.PromptTemporarily: + storage.device.set_safety_check_level(SAFETY_CHECK_LEVEL_STRICT) + storage.cache.set(APP_COMMON_SAFETY_CHECKS_TEMPORARY, level) + else: + raise ValueError("Unknown SafetyCheckLevel") + + +def is_strict() -> bool: + """ + Shorthand for checking whether the effective level is Strict. + """ + return read_setting() == SafetyCheckLevel.Strict diff --git a/core/src/apps/management/apply_settings.py b/core/src/apps/management/apply_settings.py index c40af29f0..c24cf9672 100644 --- a/core/src/apps/management/apply_settings.py +++ b/core/src/apps/management/apply_settings.py @@ -6,6 +6,7 @@ from trezor.strings import format_duration_ms from trezor.ui.text import Text from apps.base import lock_device +from apps.common import safety_checks from apps.common.confirm import require_confirm, require_hold_to_confirm if False: @@ -83,9 +84,7 @@ async def apply_settings(ctx: wire.Context, msg: ApplySettings): if msg.safety_checks is not None: await require_confirm_safety_checks(ctx, msg.safety_checks) - storage.device.set_unsafe_prompts_allowed( - msg.safety_checks == SafetyCheckLevel.Prompt - ) + safety_checks.apply_setting(msg.safety_checks) if msg.display_rotation is not None: await require_confirm_change_display_rotation(ctx, msg.display_rotation) @@ -154,18 +153,32 @@ async def require_confirm_change_autolock_delay(ctx, delay_ms): await require_confirm(ctx, text, ButtonRequestType.ProtectCall) -async def require_confirm_safety_checks(ctx, level: EnumTypeSafetyCheckLevel) -> None: - if level == SafetyCheckLevel.Prompt: - text = Text("Unsafe prompts", ui.ICON_WIPE) +async def require_confirm_safety_checks(ctx, level: EnumTypeSafetyCheckLevel,) -> None: + if level == SafetyCheckLevel.PromptAlways: + text = Text("Safety override", ui.ICON_CONFIG) text.normal( - "Trezor will allow you to", "confirm actions which", "might be dangerous." + "Trezor will allow you to", + "approve some actions", + "which might be unsafe.", ) text.br_half() - text.bold("Allow unsafe prompts?") + text.bold("Are you sure?") + await require_hold_to_confirm(ctx, text, ButtonRequestType.ProtectCall) + elif level == SafetyCheckLevel.PromptTemporarily: + text = Text("Safety override", ui.ICON_CONFIG) + text.normal( + "Trezor will temporarily", + "allow you to approve", + "some actions which", + "might be unsafe.", + ) + text.bold("Are you sure?") await require_hold_to_confirm(ctx, text, ButtonRequestType.ProtectCall) elif level == SafetyCheckLevel.Strict: - text = Text("Unsafe prompts", ui.ICON_CONFIG) - text.normal("Do you really want to", "disable unsafe prompts?") + text = Text("Safety checks", ui.ICON_CONFIG) + text.normal( + "Do you really want to", "enforce strict safety", "checks (recommended)?" + ) await require_confirm(ctx, text, ButtonRequestType.ProtectCall) else: raise ValueError # enum value out of range diff --git a/core/src/storage/cache.py b/core/src/storage/cache.py index 30ec0330b..baceaaebf 100644 --- a/core/src/storage/cache.py +++ b/core/src/storage/cache.py @@ -15,6 +15,7 @@ APP_BASE_AUTHORIZATION = 3 # Keys that are valid across sessions APP_COMMON_SEED_WITHOUT_PASSPHRASE = 1 | _SESSIONLESS_FLAG +APP_COMMON_SAFETY_CHECKS_TEMPORARY = 2 | _SESSIONLESS_FLAG _active_session_id = None # type: Optional[bytes] diff --git a/core/src/storage/device.py b/core/src/storage/device.py index 05478d168..87c624e8b 100644 --- a/core/src/storage/device.py +++ b/core/src/storage/device.py @@ -8,6 +8,7 @@ from trezor.messages import BackupType if False: from trezor.messages.ResetDevice import EnumTypeBackupType from typing import Optional + from typing_extensions import Literal # Namespace: _NAMESPACE = common.APP_DEVICE @@ -34,9 +35,15 @@ _SLIP39_IDENTIFIER = const(0x10) # bool _SLIP39_ITERATION_EXPONENT = const(0x11) # int _SD_SALT_AUTH_KEY = const(0x12) # bytes INITIALIZED = const(0x13) # bool (0x01 or empty) -_UNSAFE_PROMPTS_ALLOWED = const(0x14) # bool (0x01 or empty) +_SAFETY_CHECK_LEVEL = const(0x14) # int _DEFAULT_BACKUP_TYPE = BackupType.Bip39 + +SAFETY_CHECK_LEVEL_STRICT = const(0) # type: Literal[0] +SAFETY_CHECK_LEVEL_PROMPT = const(1) # type: Literal[1] +_DEFAULT_SAFETY_CHECK_LEVEL = SAFETY_CHECK_LEVEL_STRICT +if False: + StorageSafetyCheckLevel = Literal[0, 1] # fmt: on HOMESCREEN_MAXSIZE = 16384 @@ -283,9 +290,17 @@ def set_sd_salt_auth_key(auth_key: Optional[bytes]) -> None: return common.delete(_NAMESPACE, _SD_SALT_AUTH_KEY, public=True) -def unsafe_prompts_allowed() -> bool: - return common.get_bool(_NAMESPACE, _UNSAFE_PROMPTS_ALLOWED) +# do not use this function directly, see apps.common.safety_checks instead +def safety_check_level() -> StorageSafetyCheckLevel: + level = common.get_uint8(_NAMESPACE, _SAFETY_CHECK_LEVEL) + if level not in (SAFETY_CHECK_LEVEL_STRICT, SAFETY_CHECK_LEVEL_PROMPT): + return _DEFAULT_SAFETY_CHECK_LEVEL + else: + return level # type: ignore -def set_unsafe_prompts_allowed(allowed: bool) -> None: - common.set_bool(_NAMESPACE, _UNSAFE_PROMPTS_ALLOWED, allowed) +# do not use this function directly, see apps.common.safety_checks instead +def set_safety_check_level(level: StorageSafetyCheckLevel) -> None: + if level not in (SAFETY_CHECK_LEVEL_STRICT, SAFETY_CHECK_LEVEL_PROMPT): + raise ValueError + common.set_uint8(_NAMESPACE, _SAFETY_CHECK_LEVEL, level) diff --git a/core/tests/test_apps.common.keychain.py b/core/tests/test_apps.common.keychain.py index dce92a155..4288dfe49 100644 --- a/core/tests/test_apps.common.keychain.py +++ b/core/tests/test_apps.common.keychain.py @@ -4,11 +4,12 @@ from mock_storage import mock_storage from storage import cache import storage.device -from apps.common import HARDENED +from apps.common import HARDENED, safety_checks from apps.common.paths import path_is_hardened from apps.common.keychain import LRUCache, Keychain, with_slip44_keychain, get_keychain from trezor import wire from trezor.crypto import bip39 +from trezor.messages import SafetyCheckLevel class TestKeychain(unittest.TestCase): @@ -38,9 +39,11 @@ class TestKeychain(unittest.TestCase): keychain.verify_path(f) # turn off restrictions - storage.device.set_unsafe_prompts_allowed(True) + safety_checks.apply_setting(SafetyCheckLevel.PromptTemporarily) for path in correct + fails: keychain.verify_path(path) + # turn on restrictions + safety_checks.apply_setting(SafetyCheckLevel.Strict) def test_verify_path_special_ed25519(self): n = [[44 | HARDENED, 134 | HARDENED]] diff --git a/tests/device_tests/test_msg_applysettings.py b/tests/device_tests/test_msg_applysettings.py index 7858328b1..adab21120 100644 --- a/tests/device_tests/test_msg_applysettings.py +++ b/tests/device_tests/test_msg_applysettings.py @@ -159,7 +159,8 @@ class TestMsgApplysettings: @pytest.mark.skip_t1 @pytest.mark.setup_client(pin=None) def test_safety_checks(self, client): - BAD_ADDRESS = parse_path("m/0") + def get_bad_address(): + btc.get_address(client, "Bitcoin", parse_path("m/0")) assert client.features.safety_checks == messages.SafetyCheckLevel.Strict @@ -167,21 +168,21 @@ class TestMsgApplysettings: exceptions.TrezorFailure, match="Forbidden key path" ), client: client.set_expected_responses([messages.Failure()]) - btc.get_address(client, "Bitcoin", BAD_ADDRESS) + get_bad_address() with client: client.set_expected_responses(EXPECTED_RESPONSES_NOPIN) device.apply_settings( - client, safety_checks=messages.SafetyCheckLevel.Prompt + client, safety_checks=messages.SafetyCheckLevel.PromptAlways ) - assert client.features.safety_checks == messages.SafetyCheckLevel.Prompt + assert client.features.safety_checks == messages.SafetyCheckLevel.PromptAlways with client: client.set_expected_responses( [messages.ButtonRequest(), messages.Address()] ) - btc.get_address(client, "Bitcoin", BAD_ADDRESS) + get_bad_address() with client: client.set_expected_responses(EXPECTED_RESPONSES_NOPIN) @@ -195,4 +196,20 @@ class TestMsgApplysettings: exceptions.TrezorFailure, match="Forbidden key path" ), client: client.set_expected_responses([messages.Failure()]) - btc.get_address(client, "Bitcoin", BAD_ADDRESS) + get_bad_address() + + with client: + client.set_expected_responses(EXPECTED_RESPONSES_NOPIN) + device.apply_settings( + client, safety_checks=messages.SafetyCheckLevel.PromptTemporarily + ) + + assert ( + client.features.safety_checks == messages.SafetyCheckLevel.PromptTemporarily + ) + + with client: + client.set_expected_responses( + [messages.ButtonRequest(), messages.Address()] + ) + get_bad_address() diff --git a/tests/device_tests/test_msg_signtx.py b/tests/device_tests/test_msg_signtx.py index 1a621014c..e2b0d099e 100644 --- a/tests/device_tests/test_msg_signtx.py +++ b/tests/device_tests/test_msg_signtx.py @@ -653,8 +653,10 @@ class TestMsgSigntx: with pytest.raises(TrezorFailure, match="fee is unexpectedly large"): btc.sign_tx(client, "Bitcoin", [inp1], [out1], prev_txes=TX_CACHE_MAINNET) - # set SafetyCheckLevel to Prompt and try again - device.apply_settings(client, safety_checks=messages.SafetyCheckLevel.Prompt) + # set SafetyCheckLevel to PromptTemporarily and try again + device.apply_settings( + client, safety_checks=messages.SafetyCheckLevel.PromptTemporarily + ) with client: finished = False diff --git a/tests/persistence_tests/test_safety_checks.py b/tests/persistence_tests/test_safety_checks.py new file mode 100644 index 000000000..a5fdb59b7 --- /dev/null +++ b/tests/persistence_tests/test_safety_checks.py @@ -0,0 +1,41 @@ +import pytest + +from trezorlib import debuglink, device +from trezorlib.messages import SafetyCheckLevel + +from ..common import MNEMONIC12 +from ..emulators import EmulatorWrapper +from ..upgrade_tests import core_only + + +@pytest.fixture +def emulator(): + with EmulatorWrapper("core") as emu: + yield emu + + +@core_only +@pytest.mark.parametrize( + "set_level,after_level", + [ + (SafetyCheckLevel.Strict, SafetyCheckLevel.Strict), + (SafetyCheckLevel.PromptTemporarily, SafetyCheckLevel.Strict), + (SafetyCheckLevel.PromptAlways, SafetyCheckLevel.PromptAlways), + ], +) +def test_safety_checks_level_after_reboot(emulator, set_level, after_level): + device.wipe(emulator.client) + debuglink.load_device( + emulator.client, + mnemonic=MNEMONIC12, + pin="", + passphrase_protection=False, + label="SAFETYLEVEL", + ) + + device.apply_settings(emulator.client, safety_checks=set_level) + assert emulator.client.features.safety_checks == set_level + + emulator.restart() + + assert emulator.client.features.safety_checks == after_level diff --git a/tests/ui_tests/fixtures.json b/tests/ui_tests/fixtures.json index 779261b5b..1617bf751 100644 --- a/tests/ui_tests/fixtures.json +++ b/tests/ui_tests/fixtures.json @@ -54,7 +54,7 @@ "test_msg_applysettings.py-test_apply_settings": "2cc8bf660f3be815d19a4bf1265936162a58386fbe632ca4be01541245b79134", "test_msg_applysettings.py-test_apply_settings_passphrase": "5c1ed9a0be3d14475102d447da0b5d51bbb6dfaaeceff5ea9179064609db7870", "test_msg_applysettings.py-test_apply_settings_passphrase_on_device": "3e6527e227bdde54f51bc9c417b176d0d87fdb6c40c4761368f50eb201b4beed", -"test_msg_applysettings.py-test_safety_checks": "19bd500c3b791d51bbd1140085f306a838194593697529263f362acb0b1ab445", +"test_msg_applysettings.py-test_safety_checks": "4eb00e8d3bce08e800f3524f9a03960865c9725a08df0ebe57853602cd84b6a5", "test_msg_authorize_coinjoin.py::test_cancel_authorization": "d8a608beb6165f5667cc44dcff6bdc17ebb4638ddd3bd09e7f0e1e75d1e21135", "test_msg_authorize_coinjoin.py::test_no_anonymity": "fd09da284b650e893990b95047b63a35b6b695fc5301d595f17a6d2cf9d90bcb", "test_msg_authorize_coinjoin.py::test_sign_tx": "2838d4062333c241b6bbef7e680ec8a5764fe7bcaa41419e4141e146d3586a5d", @@ -266,7 +266,7 @@ "test_msg_signtx.py-test_attack_change_outputs": "4872e0db49b2c66f2f033d055abc086520cdd667ffe48ead0ad5ed0f4452af1a", "test_msg_signtx.py-test_attack_modify_change_address": "cfd5c83510c044c456622298138e222aee135a6df607bb6e5603228535f0762f", "test_msg_signtx.py-test_change_on_main_chain_allowed": "cfd5c83510c044c456622298138e222aee135a6df607bb6e5603228535f0762f", -"test_msg_signtx.py-test_fee_high_hardfail": "b450a59808fb20cbd01d34e8d24bf1a5814e9b2a10109710240c617b68e247b6", +"test_msg_signtx.py-test_fee_high_hardfail": "0b0e6938ae67017f876ad56995d5f7131ab5ceee2877b0e3dafcd50b2bf6d4cf", "test_msg_signtx.py-test_fee_high_warning": "8cb3b31dce25fa36cd5c8322c71611dc7bc9d2290579ffd88dd67d21058bde04", "test_msg_signtx.py-test_lock_time[1-4294967295]": "d805244ea557c3695101a6f79f13045f22bc16d5608744e0321eab7f3a98d8b0", "test_msg_signtx.py-test_lock_time[499999999-4294967294]": "23a154e7b40680161bb099cfc6702d75909c222056867515647123573eef1716",