From e83d5485dba2a881660ef35f655154a4a857e5c2 Mon Sep 17 00:00:00 2001 From: matejcik Date: Thu, 22 Oct 2020 14:58:02 +0200 Subject: [PATCH] fix(core): make sure run-time settings are reset after wipe (fixes #1322) --- core/CHANGELOG.md | 2 ++ core/src/apps/management/apply_settings.py | 12 ++++++--- core/src/apps/management/wipe_device.py | 3 +++ tests/device_tests/test_msg_wipedevice.py | 30 +++++++++++++++++++++- tests/ui_tests/fixtures.json | 1 + 5 files changed, 43 insertions(+), 5 deletions(-) diff --git a/core/CHANGELOG.md b/core/CHANGELOG.md index 01d6db42bae..274e3429e92 100644 --- a/core/CHANGELOG.md +++ b/core/CHANGELOG.md @@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Fixed - Path warning is not shown on `GetAddress(show_display=False)` call [#1206] +- Settings are also erased from RAM when device is wiped [#1322] ### Security @@ -326,3 +327,4 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). [#1193]: https://github.com/trezor/trezor-firmware/issues/1193 [#1206]: https://github.com/trezor/trezor-firmware/issues/1206 [#1246]: https://github.com/trezor/trezor-firmware/issues/1246 +[#1322]: https://github.com/trezor/trezor-firmware/issues/1322 diff --git a/core/src/apps/management/apply_settings.py b/core/src/apps/management/apply_settings.py index 1e83985dbca..015094e4293 100644 --- a/core/src/apps/management/apply_settings.py +++ b/core/src/apps/management/apply_settings.py @@ -80,8 +80,6 @@ async def apply_settings(ctx: wire.Context, msg: ApplySettings): raise wire.ProcessError("Auto-lock delay too long") await require_confirm_change_autolock_delay(ctx, msg.auto_lock_delay_ms) storage.device.set_autolock_delay_ms(msg.auto_lock_delay_ms) - # use the value that was stored, not the one that was supplied by the user - workflow.idle_timer.set(storage.device.get_autolock_delay_ms(), lock_device) if msg.safety_checks is not None: await require_confirm_safety_checks(ctx, msg.safety_checks) @@ -90,16 +88,22 @@ async def apply_settings(ctx: wire.Context, msg: ApplySettings): if msg.display_rotation is not None: await require_confirm_change_display_rotation(ctx, msg.display_rotation) storage.device.set_rotation(msg.display_rotation) - ui.display.orientation(storage.device.get_rotation()) if msg.experimental_features is not None: await require_confirm_experimental_features(ctx, msg.experimental_features) storage.device.set_experimental_features(msg.experimental_features) - wire.experimental_enabled = msg.experimental_features + + reload_settings_from_storage() return Success(message="Settings applied") +def reload_settings_from_storage() -> None: + workflow.idle_timer.set(storage.device.get_autolock_delay_ms(), lock_device) + ui.display.orientation(storage.device.get_rotation()) + wire.experimental_enabled = storage.device.get_experimental_features() + + async def require_confirm_change_homescreen(ctx): text = Text("Set homescreen", ui.ICON_CONFIG) text.normal("Do you really want to", "change the homescreen", "image?") diff --git a/core/src/apps/management/wipe_device.py b/core/src/apps/management/wipe_device.py index 2240b7f5eb5..498ffb5c19b 100644 --- a/core/src/apps/management/wipe_device.py +++ b/core/src/apps/management/wipe_device.py @@ -8,6 +8,8 @@ from apps.common.confirm import require_hold_to_confirm +from .apply_settings import reload_settings_from_storage + async def wipe_device(ctx, msg): text = Text("Wipe device", ui.ICON_WIPE, ui.RED) @@ -22,5 +24,6 @@ async def wipe_device(ctx, msg): ) storage.wipe() + reload_settings_from_storage() return Success(message="Device wiped") diff --git a/tests/device_tests/test_msg_wipedevice.py b/tests/device_tests/test_msg_wipedevice.py index dd157d49cc2..9427d4fb58a 100644 --- a/tests/device_tests/test_msg_wipedevice.py +++ b/tests/device_tests/test_msg_wipedevice.py @@ -14,9 +14,15 @@ # You should have received a copy of the License along with this library. # If not, see . +import time + import pytest -from trezorlib import device +from trezorlib import device, messages + +from ..common import get_test_address + +PIN4 = "1234" @pytest.mark.setup_client(passphrase=True) @@ -32,3 +38,25 @@ def test_wipe_device(client): assert client.features.label is None assert client.features.passphrase_protection is False assert client.get_device_id() != device_id + + +@pytest.mark.setup_client(pin=PIN4) +def test_autolock_not_retained(client): + with client: + client.use_pin_sequence([PIN4]) + device.apply_settings(client, auto_lock_delay_ms=10_000) + + assert client.features.auto_lock_delay_ms == 10_000 + + device.wipe(client) + assert client.features.auto_lock_delay_ms > 10_000 + + with client: + client.use_pin_sequence([PIN4, PIN4]) + device.reset(client, skip_backup=True, pin_protection=True) + + time.sleep(10.1) + with client: + # after sleeping for the pre-wipe autolock amount, Trezor must still be unlocked + client.set_expected_responses([messages.Address]) + get_test_address(client) diff --git a/tests/ui_tests/fixtures.json b/tests/ui_tests/fixtures.json index 8d7f5285c4f..2025bed0e52 100644 --- a/tests/ui_tests/fixtures.json +++ b/tests/ui_tests/fixtures.json @@ -411,6 +411,7 @@ "test_msg_verifymessage_segwit_native.py-test_message_verify": "818b8c38b040ac45e1525ca2bc90507092b57380b220503ad11f7c1e03a5ee62", "test_msg_verifymessage_segwit_native.py-test_verify_utf": "f89d64e7ca25daa7ad180294d45d01d40e4265679dd327c5a23579a261622d24", "test_msg_webauthn.py::test_add_remove": "a9cdefeb089f197427257e097d07179b23de4fcad4ee91af0191ed767f80577c", +"test_msg_wipedevice.py::test_autolock_not_retained": "b519a86b80151e5c1a4849fc5d6e8e6c8f2953c80befee8dd05f7caec254b464", "test_msg_wipedevice.py::test_wipe_device": "bc6acd0386b9d009e6550519917d6e08632b3badde0b0cf04c95abe5f773038a", "test_multisig.py-test_15_of_15": "9d1799a199b45785ac69ae6af715c251b10aebb60f981c9b73d78e53e1a91374", "test_multisig.py-test_2_of_3": "2be92556edf4ff8eed340d535f379ee6915eae34fef25d669ce865848e7b4705",