From 33e9611ffc6b38e32e8838649c32e5046b94ea25 Mon Sep 17 00:00:00 2001 From: grdddj Date: Wed, 16 Mar 2022 16:24:17 +0100 Subject: [PATCH] WIP - safety check levels --- core/embed/rust/librust_qstr.h | 2 + .../rust/src/storagedevice/storage_device.rs | 65 +++++++++++++++---- core/mocks/generated/trezorstoragedevice.pyi | 15 +++++ core/src/apps/common/safety_checks.py | 12 ++-- core/src/storage/device.py | 24 +++---- 5 files changed, 89 insertions(+), 29 deletions(-) diff --git a/core/embed/rust/librust_qstr.h b/core/embed/rust/librust_qstr.h index 140b883ece..906be03a3e 100644 --- a/core/embed/rust/librust_qstr.h +++ b/core/embed/rust/librust_qstr.h @@ -49,6 +49,8 @@ static void _librust_qstrs(void) { MP_QSTR_set_autolock_delay_ms; MP_QSTR_get_flags; MP_QSTR_set_flags; + MP_QSTR_get_safety_check_level; + MP_QSTR_set_safety_check_level; MP_QSTR_set_timer_fn; MP_QSTR_touch_event; diff --git a/core/embed/rust/src/storagedevice/storage_device.rs b/core/embed/rust/src/storagedevice/storage_device.rs index ddabfbdbc8..219beb987c 100644 --- a/core/embed/rust/src/storagedevice/storage_device.rs +++ b/core/embed/rust/src/storagedevice/storage_device.rs @@ -12,6 +12,7 @@ use core::convert::{TryFrom, TryInto}; // MISSING FUNCTIONALITY: // - returning strings into python +// - raising custom exceptions into python const FLAG_PUBLIC: u8 = 0x80; @@ -55,6 +56,11 @@ const INITIALIZED: u8 = 0x13; const _SAFETY_CHECK_LEVEL: u8 = 0x14; const _EXPERIMENTAL_FEATURES: u8 = 0x15; +const SAFETY_CHECK_LEVEL_STRICT: u8 = 0; +const SAFETY_CHECK_LEVEL_PROMPT: u8 = 0; +const _DEFAULT_SAFETY_CHECK_LEVEL: u8 = SAFETY_CHECK_LEVEL_STRICT; +const SAFETY_CHECK_LEVELS: [u8; 2] = [SAFETY_CHECK_LEVEL_STRICT, SAFETY_CHECK_LEVEL_PROMPT]; + // TODO: somehow determine the DEBUG_MODE value const DEBUG_MODE: bool = true; const AUTOLOCK_DELAY_DEFAULT: u32 = 10 * 60 * 1000; // 10 minutes @@ -124,13 +130,7 @@ extern "C" fn storagedevice_is_initialized() -> Obj { extern "C" fn storagedevice_get_rotation() -> Obj { let block = || { let key = _get_appkey(_ROTATION, true); - // TODO: could create something like - // storagedevice_storage_get_u16_with_default(key, 0) - let result = storagedevice_storage_get_u16(key); - match result { - Some(num) => Ok(num.into()), - None => Ok(0u16.into()), - } + Ok(storagedevice_storage_get_u16(key).unwrap_or(0).into()) }; unsafe { util::try_or_raise(block) } } @@ -187,7 +187,6 @@ extern "C" fn storagedevice_get_mnemonic_secret() -> Obj { let block = || { let key = _get_appkey(_MNEMONIC_SECRET, false); let result = &storagedevice_storage_get(key) as &[u8]; - // TODO: find out how to return None result.try_into() }; unsafe { util::try_or_raise(block) } @@ -371,10 +370,7 @@ extern "C" fn storagedevice_set_autolock_delay_ms(delay_ms: Obj) -> Obj { extern "C" fn storagedevice_get_flags() -> Obj { let block = || { let key = _get_appkey(_FLAGS, false); - match storagedevice_storage_get_u32(key) { - Some(flag) => flag.try_into(), - None => 0.try_into(), - } + storagedevice_storage_get_u32(key).unwrap_or(0).try_into() }; unsafe { util::try_or_raise(block) } } @@ -394,6 +390,37 @@ extern "C" fn storagedevice_set_flags(flags: Obj) -> Obj { unsafe { util::try_or_raise(block) } } +extern "C" fn storagedevice_get_safety_check_level() -> Obj { + let block = || { + let key = _get_appkey(_SAFETY_CHECK_LEVEL, false); + let level = storagedevice_storage_get_u8(key).unwrap_or(0); + + let level = if !SAFETY_CHECK_LEVELS.contains(&level) { + _DEFAULT_SAFETY_CHECK_LEVEL + } else { + level + }; + + Ok(level.into()) + }; + unsafe { util::try_or_raise(block) } +} + +extern "C" fn storagedevice_set_safety_check_level(level: Obj) -> Obj { + let block = || { + let level = u8::try_from(level)?; + + // TODO: how to raise a micropython exception? + if !SAFETY_CHECK_LEVELS.contains(&level) { + // return Error::ValueError(cstr!("Not valid safety level")); + } + + let key = _get_appkey(_SAFETY_CHECK_LEVEL, false); + Ok(storagedevice_storage_set_u8(key, level).into()) + }; + unsafe { util::try_or_raise(block) } +} + pub fn storagedevice_storage_get(key: u16) -> Vec { let mut buf: [u8; MAX_LEN] = [0; MAX_LEN]; let mut len: u16 = 0; @@ -528,6 +555,8 @@ pub static mp_module_trezorstoragedevice: Module = obj_module! { // TODO: should we return None or True in case of set_xxx? + /// StorageSafetyCheckLevel = Literal[0, 1] + /// def is_version_stored() -> bool: /// """Whether version is in storage.""" Qstr::MP_QSTR_is_version_stored => obj_fn_0!(storagedevice_is_version_stored).as_obj(), @@ -653,6 +682,18 @@ pub static mp_module_trezorstoragedevice: Module = obj_module! { /// def set_flags(flags: int) -> bool: /// """Set flags.""" Qstr::MP_QSTR_set_flags => obj_fn_1!(storagedevice_set_flags).as_obj(), + + /// def get_safety_check_level() -> StorageSafetyCheckLevel: + /// """Get safety check level. + /// Do not use this function directly, see apps.common.safety_checks instead. + /// """ + Qstr::MP_QSTR_get_safety_check_level => obj_fn_0!(storagedevice_get_safety_check_level).as_obj(), + + /// def set_safety_check_level(level: StorageSafetyCheckLevel) -> bool: + /// """Set safety check level. + /// Do not use this function directly, see apps.common.safety_checks instead. + /// """ + Qstr::MP_QSTR_set_safety_check_level => obj_fn_1!(storagedevice_set_safety_check_level).as_obj(), }; #[cfg(test)] diff --git a/core/mocks/generated/trezorstoragedevice.pyi b/core/mocks/generated/trezorstoragedevice.pyi index 7519732b8d..8fbaa9b1b0 100644 --- a/core/mocks/generated/trezorstoragedevice.pyi +++ b/core/mocks/generated/trezorstoragedevice.pyi @@ -1,4 +1,5 @@ from typing import * +StorageSafetyCheckLevel = Literal[0, 1] # rust/src/storagedevice/storage_device.rs @@ -152,3 +153,17 @@ def get_flags() -> int: # rust/src/storagedevice/storage_device.rs def set_flags(flags: int) -> bool: """Set flags.""" + + +# rust/src/storagedevice/storage_device.rs +def get_safety_check_level() -> StorageSafetyCheckLevel: + """Get safety check level. + Do not use this function directly, see apps.common.safety_checks instead. + """ + + +# rust/src/storagedevice/storage_device.rs +def set_safety_check_level(level: StorageSafetyCheckLevel) -> bool: + """Set safety check level. + Do not use this function directly, see apps.common.safety_checks instead. + """ diff --git a/core/src/apps/common/safety_checks.py b/core/src/apps/common/safety_checks.py index bff8d54ca4..5a4a25e994 100644 --- a/core/src/apps/common/safety_checks.py +++ b/core/src/apps/common/safety_checks.py @@ -1,7 +1,9 @@ import storage.cache -import storage.device from storage.cache import APP_COMMON_SAFETY_CHECKS_TEMPORARY + +# TODO: export these constants also in storagedevice? from storage.device import SAFETY_CHECK_LEVEL_PROMPT, SAFETY_CHECK_LEVEL_STRICT +from trezor import storagedevice from trezor.enums import SafetyCheckLevel @@ -13,7 +15,7 @@ def read_setting() -> SafetyCheckLevel: if temporary_safety_check_level: return int.from_bytes(temporary_safety_check_level, "big") # type: ignore [int-into-enum] else: - stored = storage.device.safety_check_level() + stored = storagedevice.get_safety_check_level() if stored == SAFETY_CHECK_LEVEL_STRICT: return SafetyCheckLevel.Strict elif stored == SAFETY_CHECK_LEVEL_PROMPT: @@ -28,12 +30,12 @@ def apply_setting(level: SafetyCheckLevel) -> None: """ if level == SafetyCheckLevel.Strict: storage.cache.delete(APP_COMMON_SAFETY_CHECKS_TEMPORARY) - storage.device.set_safety_check_level(SAFETY_CHECK_LEVEL_STRICT) + storagedevice.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) + storagedevice.set_safety_check_level(SAFETY_CHECK_LEVEL_PROMPT) elif level == SafetyCheckLevel.PromptTemporarily: - storage.device.set_safety_check_level(SAFETY_CHECK_LEVEL_STRICT) + storagedevice.set_safety_check_level(SAFETY_CHECK_LEVEL_STRICT) storage.cache.set(APP_COMMON_SAFETY_CHECKS_TEMPORARY, level.to_bytes(1, "big")) else: raise ValueError("Unknown SafetyCheckLevel") diff --git a/core/src/storage/device.py b/core/src/storage/device.py index dfbb88710d..20fb7cef14 100644 --- a/core/src/storage/device.py +++ b/core/src/storage/device.py @@ -306,20 +306,20 @@ def set_sd_salt_auth_key(auth_key: bytes | None) -> None: return common.delete(_NAMESPACE, _SD_SALT_AUTH_KEY, public=True) -# 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 [int-into-enum] +# # 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 -# 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) +# # 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) @storage.cache.stored(storage.cache.STORAGE_DEVICE_EXPERIMENTAL_FEATURES)