From fd117a0c9f6f33b0833309749490e93e8439504c Mon Sep 17 00:00:00 2001 From: Martin Milata Date: Tue, 21 Jul 2020 16:39:36 +0200 Subject: [PATCH] core: raise error on auto-lock value out of range --- core/src/apps/management/apply_settings.py | 10 ++-- core/src/trezor/strings.py | 19 ++++++++ core/tests/test_trezor.strings.py | 21 +++++++++ tests/device_tests/test_autolock.py | 54 +++++++++++++--------- tests/ui_tests/fixtures.json | 7 ++- 5 files changed, 83 insertions(+), 28 deletions(-) diff --git a/core/src/apps/management/apply_settings.py b/core/src/apps/management/apply_settings.py index 434315be9..485d29697 100644 --- a/core/src/apps/management/apply_settings.py +++ b/core/src/apps/management/apply_settings.py @@ -2,6 +2,7 @@ import storage.device from trezor import ui, wire, workflow from trezor.messages import ButtonRequestType from trezor.messages.Success import Success +from trezor.strings import format_duration_ms from trezor.ui.text import Text from apps.base import lock_device @@ -44,9 +45,10 @@ async def apply_settings(ctx: wire.Context, msg: ApplySettings): await require_confirm_change_display_rotation(ctx, msg.display_rotation) if msg.auto_lock_delay_ms is not None: - msg.auto_lock_delay_ms = max( - msg.auto_lock_delay_ms, storage.device.AUTOLOCK_DELAY_MINIMUM - ) + if msg.auto_lock_delay_ms < storage.device.AUTOLOCK_DELAY_MINIMUM: + raise wire.ProcessError("Auto-lock delay too short") + if msg.auto_lock_delay_ms > storage.device.AUTOLOCK_DELAY_MAXIMUM: + raise wire.ProcessError("Auto-lock delay too long") await require_confirm_change_autolock_delay(ctx, msg.auto_lock_delay_ms) storage.device.load_settings( @@ -120,5 +122,5 @@ async def require_confirm_change_display_rotation(ctx, rotation): async def require_confirm_change_autolock_delay(ctx, delay_ms): text = Text("Auto-lock delay", ui.ICON_CONFIG, new_lines=False) text.normal("Do you really want to", "auto-lock your device", "after") - text.bold("{} seconds?".format(delay_ms // 1000)) + text.bold("{}?".format(format_duration_ms(delay_ms))) await require_confirm(ctx, text, ButtonRequestType.ProtectCall) diff --git a/core/src/trezor/strings.py b/core/src/trezor/strings.py index 976f21335..48de22705 100644 --- a/core/src/trezor/strings.py +++ b/core/src/trezor/strings.py @@ -45,3 +45,22 @@ def format_plural(string: str, count: int, plural: str) -> str: plural = plural + "s" return string.format(count=count, plural=plural) + + +def format_duration_ms(milliseconds: int) -> str: + """ + Returns human-friendly representation of a duration. Truncates all decimals. + """ + units = ( + ("hour", 60 * 60 * 1000), + ("minute", 60 * 1000), + ("second", 1000), + ) + for unit, divisor in units: + if milliseconds >= divisor: + break + else: + unit = "millisecond" + divisor = 1 + + return format_plural("{count} {plural}", milliseconds // divisor, unit) diff --git a/core/tests/test_trezor.strings.py b/core/tests/test_trezor.strings.py index e6bf1d3f1..34ad1deb0 100644 --- a/core/tests/test_trezor.strings.py +++ b/core/tests/test_trezor.strings.py @@ -32,6 +32,27 @@ class TestStrings(unittest.TestCase): with self.assertRaises(ValueError): strings.format_plural("Hello", 1, "share") + def test_format_duration_ms(self): + VECTORS = [ + (0, "0 milliseconds"), + (1, "1 millisecond"), + (999, "999 milliseconds"), + (1000, "1 second"), + (2345, "2 seconds"), + (59999, "59 seconds"), + (60 * 1000, "1 minute"), + (119 * 1000, "1 minute"), + (120 * 1000, "2 minutes"), + (59 * 60 * 1000, "59 minutes"), + (60 * 60 * 1000, "1 hour"), + (119 * 60 * 1000, "1 hour"), + (3 * 60 * 60 * 1000, "3 hours"), + (48 * 60 * 60 * 1000, "48 hours"), + ] + + for v in VECTORS: + self.assertEqual(strings.format_duration_ms(v[0]), v[1]) + if __name__ == '__main__': unittest.main() diff --git a/tests/device_tests/test_autolock.py b/tests/device_tests/test_autolock.py index 64c12339e..422fa7ca4 100644 --- a/tests/device_tests/test_autolock.py +++ b/tests/device_tests/test_autolock.py @@ -19,6 +19,7 @@ import time import pytest from trezorlib import device, messages +from trezorlib.exceptions import TrezorFailure from ..common import TEST_ADDRESS_N, get_test_address @@ -65,31 +66,38 @@ def test_apply_auto_lock_delay(client): get_test_address(client) -def test_apply_minimal_auto_lock_delay(client): - """ - Verify that the delay is not below the minimal auto-lock delay (10 secs) - otherwise the device may auto-lock before any user interaction. - """ - set_autolock_delay(client, 1 * 1000) - - time.sleep(0.1) # sleep less than auto-lock delay - with client: - # No PIN protection is required. - client.set_expected_responses([messages.Address()]) - get_test_address(client) - - # sleep more than specified auto-lock delay (1s) but less than minimal allowed (10s) - time.sleep(3) - with client: - # No PIN protection is required. - client.set_expected_responses([messages.Address()]) - get_test_address(client) - - time.sleep(10.1) # sleep more than the minimal auto-lock delay +@pytest.mark.parametrize( + "seconds", + [ + 10, # 10 seconds, minimum + 60, # 1 minute + 123, # 2 minutes + 3601, # 1 hour + 7227, # 2 hours + 536870, # 149 hours, maximum + ], +) +def test_apply_auto_lock_delay_valid(client, seconds): + set_autolock_delay(client, seconds * 1000) + + +@pytest.mark.skip_ui +@pytest.mark.parametrize( + "seconds", [0, 1, 9, 536871, 2 ** 22], +) +def test_apply_auto_lock_delay_out_of_range(client, seconds): with client: client.use_pin_sequence([PIN4]) - client.set_expected_responses([pin_request(client), messages.Address()]) - get_test_address(client) + client.set_expected_responses( + [ + pin_request(client), + messages.Failure(code=messages.FailureType.ProcessError), + ] + ) + + delay = seconds * 1000 + with pytest.raises(TrezorFailure): + device.apply_settings(client, auto_lock_delay_ms=delay) @pytest.mark.skip_t1 diff --git a/tests/ui_tests/fixtures.json b/tests/ui_tests/fixtures.json index 742b2f1eb..118c62fcf 100644 --- a/tests/ui_tests/fixtures.json +++ b/tests/ui_tests/fixtures.json @@ -1,6 +1,11 @@ { "test_autolock.py::test_apply_auto_lock_delay": "1997527e85989f3ca5719f93cd76bcfb8f125fb96ef3025073b13fd4de7a5fa2", -"test_autolock.py::test_apply_minimal_auto_lock_delay": "adc5da99fcc8b4023d6989fa59f69c3918e70165bdeb693a887364e2009ee2fa", +"test_autolock.py::test_apply_auto_lock_delay_valid[10]": "d03aca645ebd8a44c8a1bb24a275e31441245c3211f7c82365b2f432370b05bc", +"test_autolock.py::test_apply_auto_lock_delay_valid[123]": "7588a745923a732cd8715ab76f95b7abb9c2caf043075ec1a73e31b2453743a1", +"test_autolock.py::test_apply_auto_lock_delay_valid[3601]": "904323c7f7a27393c4a192680340d571a2a4899f0300da4d0e439f55f32768e8", +"test_autolock.py::test_apply_auto_lock_delay_valid[536870]": "ff8b1d62a60d581504779cb7bb3dcf7882e960914bac4981d52fa714880c3d1e", +"test_autolock.py::test_apply_auto_lock_delay_valid[60]": "02f813f809bec7b303998fe288f02bfa4cd1a30990c0dc071ad51ff86f2739e6", +"test_autolock.py::test_apply_auto_lock_delay_valid[7227]": "41bca947f7834baa9968cbb164b70005aee4dbf23586187ab3bef336ff42a422", "test_autolock.py::test_autolock_cancels_ui": "eedc6196565bf6d53bc9c3f8984acc2bb91d2d71e57f3a28f8afbe35b02fb4dc", "test_basic.py-test_device_id_different": "bc6acd0386b9d009e6550519917d6e08632b3badde0b0cf04c95abe5f773038a", "test_basic.py-test_device_id_same": "5a80508a71a9ef64f94762b07636f90e464832f0f4a3102af8fa1a8c69e94586",