Public commit record
70/100 · Adequate
What the developer wrote
Authored by matejcik
feat(core): introduce lock_manager
it eats some functions from apps.base, and introduces an asynchronous
notification mechanism for suspending via `notify_suspend()`
✓ Descriptive subject✓ Uses a recognizable type or scope✓ Provides detailed explanatory context
What changed, and why it matters
This commit is a code reorganization: it moves device locking, unlocking, homescreen selection, and suspend-related logic out of apps/base.py into a new module called apps/common/lock_manager.py. It also changes how the power button triggers suspend, using a new asynchronous notification mechanism (notify_suspend) instead of immediately locking the device. There is no direct evidence in the diff that this fixes a security vulnerability; it appears to be a feature/refactoring change.
Recommended action
Treat this as a normal feature/refactoring commit rather than an urgent security patch. Reviewers should verify that the new asynchronous suspend path preserves the existing security invariant that the device locks before suspend, and that the removal of the immediate Shutdown() in the power button handler does not create a window where sensitive UI remains interactive after suspend is requested. The TODO comments indicate the implementation may still be evolving, so additional commits should be monitored.
Security signals we found
01
Refactoring of security-critical locking/unlocking code into a new module
02
Change in power button handling from synchronous lock+shutdown to asynchronous suspend notification
03
Introduction of asynchronous mailbox-based suspend signaling (notify_suspend, _power_handler)
04
Addition of TODO comments suggesting incomplete design decisions ('TODO: should we suspend the device here?', 'TODO: hide this inside lock_manager')
05
No explicit security bug fix or vulnerability description in commit message or diff
Evidence from the diff
The commit introduces apps/common/lock_manager.py, which centralizes previously scattered functions: set_homescreen, lock_device, unlock_device, lock_device_if_unlocked, reload_settings_from_storage, _pinlock_filter, and power-management suspend helpers. The power button handler in trezor/ui/init.py no longer directly calls lock_device_if_unlocked and raises Shutdown; instead it calls notify_suspend(), which signals a mailbox-scheduled task to lock and suspend asynchronously. Several modules are updated to import these functions from lock_manager instead of apps.base. The commit message frames this as a feature (‘introduce lock_manager’) and a new notification mechanism.
Changed components
core/src/apps/common/lock_manager.pycore/src/apps/base.pycore/src/apps/homescreen/__init__.pycore/src/apps/management/apply_settings.pycore/src/apps/management/wipe_device.pycore/src/apps/webauthn/fido2.pycore/src/boot.pycore/src/session.pycore/src/trezor/ui/__init__.pycore/src/trezor/ui/layouts/homescreen.pycore/embed/upymod/qstrdefsport.hInspect captured patch +217 / −160
diff --git a/core/embed/upymod/qstrdefsport.h b/core/embed/upymod/qstrdefsport.h
index d720a19d..69ec0ef7 100644
--- a/core/embed/upymod/qstrdefsport.h
+++ b/core/embed/upymod/qstrdefsport.h
@@ -108,6 +108,7 @@ Q(apps.common.coins)
Q(apps.common.definitions)
Q(apps.common.definitions_constants)
Q(apps.common.keychain)
+Q(apps.common.lock_manager)
Q(apps.common.passphrase)
Q(apps.common.paths)
Q(apps.common.payment_request)
@@ -232,6 +233,7 @@ Q(keychain)
Q(layout)
Q(layouts)
Q(list_names)
+Q(lock_manager)
Q(log)
Q(log_benchmark)
Q(loop)
diff --git a/core/src/apps/base.py b/core/src/apps/base.py
index 50bd88d6..0490f5b6 100644
--- a/core/src/apps/base.py
+++ b/core/src/apps/base.py
@@ -7,14 +7,10 @@ from trezor.enums import HomescreenFormat, MessageType
from trezor.messages import Success, UnlockPath
from trezor.ui.layouts import confirm_action
from trezor.wire import context
-from trezor.wire.message_handler import filters, remove_filter
+from trezor.wire.message_handler import filters
from . import workflow_handlers
-
-if utils.USE_POWER_MANAGER:
- from trezor import io
- from trezor.power_management.autodim import autodim_display
- from trezor.power_management.suspend import suspend_device
+from .common import lock_manager
if TYPE_CHECKING:
from typing import NoReturn
@@ -32,7 +28,6 @@ if TYPE_CHECKING:
Ping,
SetBusy,
)
- from trezor.wire import Handler, Msg
if utils.USE_THP:
from trezor.messages import (
@@ -273,7 +268,7 @@ if utils.USE_THP:
channel_ctx=channel, session_id=session_id
)
try:
- await unlock_device()
+ await lock_manager.unlock_device()
await derive_and_store_roots(new_session, message)
except DataError as e:
return Failure(code=FailureType.DataError, message=e.message)
@@ -407,7 +402,7 @@ async def handle_Cancel(msg: Cancel) -> NoReturn:
async def handle_LockDevice(msg: LockDevice) -> Success:
- lock_device()
+ lock_manager.lock_device()
return Success()
@@ -422,7 +417,7 @@ async def handle_SetBusy(msg: SetBusy) -> Success:
context.cache_set_int(APP_COMMON_BUSY_DEADLINE_MS, deadline)
else:
context.cache_delete(APP_COMMON_BUSY_DEADLINE_MS)
- set_homescreen()
+ lock_manager.set_homescreen()
workflow.close_others()
return Success()
@@ -521,137 +516,6 @@ async def handle_CancelAuthorization(msg: CancelAuthorization) -> protobuf.Messa
return Success(message="Authorization cancelled")
-def set_homescreen() -> None:
- import storage.recovery as storage_recovery
-
- from apps.common import backup
-
- set_default = workflow.set_default # local_cache_attribute
-
- if context.cache_is_set(APP_COMMON_BUSY_DEADLINE_MS):
- from apps.homescreen import busyscreen
-
- set_default(busyscreen)
-
- elif not config.is_unlocked():
- from apps.homescreen import lockscreen
-
- set_default(lockscreen)
-
- elif _SCREENSAVER_IS_ON:
- from apps.homescreen import screensaver
-
- set_default(screensaver, restart=True)
-
- elif storage_recovery.is_in_progress() or backup.repeated_backup_enabled():
- from apps.management.recovery_device.homescreen import recovery_homescreen
-
- set_default(recovery_homescreen)
-
- else:
- from apps.homescreen import homescreen
-
- set_default(homescreen)
-
-
-def lock_device(interrupt_workflow: bool = True) -> None:
- if config.has_pin():
- config.lock()
- filters.append(_pinlock_filter)
- set_homescreen()
- if interrupt_workflow:
- workflow.close_others()
-
-
-def lock_device_if_unlocked() -> None:
- from apps.common.request_pin import can_lock_device
-
- if not utils.USE_BACKLIGHT and not can_lock_device():
- # on OLED devices without PIN, trigger screensaver
- global _SCREENSAVER_IS_ON
-
- _SCREENSAVER_IS_ON = True
- set_homescreen()
-
- elif config.is_unlocked():
- lock_device(interrupt_workflow=workflow.autolock_interrupts_workflow)
-
- if utils.USE_POWER_MANAGER and not utils.EMULATOR:
- # FIXME: suspend not implemented on emulator
- wakeup_flag = suspend_device()
- handle_wakeup_from_suspend(wakeup_flag)
-
-
-if utils.USE_POWER_MANAGER:
-
- def lock_device_if_unlocked_on_battery() -> None:
- """Lock the device if it is unlocked and running on battery or wireless charger."""
- if not io.pm.is_usb_connected():
- lock_device_if_unlocked()
-
- def handle_wakeup_from_suspend(wakeup_flag: int) -> None:
- """Handle wakeup from suspend."""
- from trezor.ui import CURRENT_LAYOUT
-
- if wakeup_flag == io.pm.WAKEUP_FLAG_BUTTON:
- if CURRENT_LAYOUT is not None:
- CURRENT_LAYOUT.layout.request_complete_repaint()
- # TODO: handle PWR
-
-
-async def unlock_device() -> None:
- """Ensure the device is in unlocked state.
-
- If the storage is locked, attempt to unlock it. Reset the homescreen and the wire
- handler.
- """
- from apps.common.request_pin import verify_user_pin
-
- global _SCREENSAVER_IS_ON
-
- if not config.is_unlocked():
- # verify_user_pin will raise if the PIN was invalid
- await verify_user_pin()
-
- _SCREENSAVER_IS_ON = False
- set_homescreen()
- remove_filter(_pinlock_filter)
-
-
-def _pinlock_filter(msg_type: int, prev_handler: Handler[Msg]) -> Handler[Msg]:
- if msg_type in workflow.ALLOW_WHILE_LOCKED:
- return prev_handler
-
- async def wrapper(msg: Msg) -> protobuf.MessageType:
- await unlock_device()
- return await prev_handler(msg)
-
- return wrapper
-
-
-# this function is also called when handling ApplySettings
-def reload_settings_from_storage() -> None:
- from trezor import ui
-
- workflow.idle_timer.set(
- storage_device.get_autolock_delay_ms(), lock_device_if_unlocked
- )
-
- if utils.USE_POWER_MANAGER:
- # autodim setting is not from storage but keeping it here for simplicity
- workflow.idle_timer.set(30_000, autodim_display)
- workflow.idle_timer.set(
- storage_device.get_autolock_delay_battery_ms(),
- lock_device_if_unlocked_on_battery,
- )
- wire.message_handler.EXPERIMENTAL_ENABLED = (
- storage_device.get_experimental_features()
- )
- if ui.display.orientation() != storage_device.get_rotation():
- ui.backlight_fade(ui.BacklightLevels.DIM)
- ui.display.orientation(storage_device.get_rotation())
-
-
def boot() -> None:
from apps.common import backup
@@ -676,9 +540,10 @@ def boot() -> None:
]:
workflow_handlers.register(msg_type, handler)
- reload_settings_from_storage()
+ lock_manager.reload_settings_from_storage()
if backup.repeated_backup_enabled():
backup.activate_repeated_backup()
if not config.is_unlocked():
# pinlocked handler should always be the last one
- filters.append(_pinlock_filter)
+ # TODO: hide this inside lock_manager
+ filters.append(lock_manager._pinlock_filter)
diff --git a/core/src/apps/common/lock_manager.py b/core/src/apps/common/lock_manager.py
new file mode 100644
index 00000000..9ec6dab2
--- /dev/null
+++ b/core/src/apps/common/lock_manager.py
@@ -0,0 +1,195 @@
+from typing import TYPE_CHECKING
+
+import storage.device as storage_device
+from storage.cache_common import APP_COMMON_BUSY_DEADLINE_MS
+from trezor import config, utils, wire, workflow
+from trezor.wire import context
+from trezor.wire.message_handler import filters, remove_filter
+
+if utils.USE_POWER_MANAGER:
+ from trezor import io
+ from trezor.power_management.autodim import autodim_display
+ from trezor.power_management.suspend import suspend_device
+
+if TYPE_CHECKING:
+ from trezor import protobuf
+ from trezor.wire import Handler, Msg
+
+_SCREENSAVER_IS_ON = False
+
+
+if not utils.USE_POWER_MANAGER:
+
+ def notify_suspend() -> None:
+ pass
+
+else:
+ from trezor import loop
+
+ _SHOULD_SUSPEND = False
+ _notify_power_button: loop.mailbox[None] = loop.mailbox()
+
+ def _prepare_suspend() -> None:
+ """Signal that the device should be suspended by the default task.
+
+ Sets a suspend homescreen for next time the default task is invoked."""
+ if not utils.EMULATOR:
+ # FIXME: suspend not implemented on emulator
+ _SHOULD_SUSPEND = True
+ set_homescreen()
+
+ def notify_suspend() -> None:
+ """Signal that the the device should be suspended in the next cycle.
+
+ Notifies an asynchronous task to perform the suspend in a separate thread.
+ """
+ _notify_power_button.put(None)
+
+ async def _power_handler() -> None:
+ """Handler for the notify_suspend signal."""
+ while True:
+ await _notify_power_button
+ lock_device_if_unlocked()
+
+ async def _suspend_and_resume() -> None:
+ """Default task that suspends the device and invokes resumption.
+
+ Must be async (or more precisely a generator) so that we can schedule it
+ via set_default."""
+ from trezor.ui import CURRENT_LAYOUT
+
+ wakeup_flag = suspend_device()
+
+ if wakeup_flag == io.pm.WAKEUP_FLAG_BUTTON:
+ if CURRENT_LAYOUT is not None:
+ CURRENT_LAYOUT.layout.request_complete_repaint()
+
+ _SHOULD_SUSPEND = False
+ set_homescreen()
+
+ def lock_device_if_unlocked_on_battery() -> None:
+ """Lock the device if it is unlocked and running on battery or wireless charger."""
+ if not io.pm.is_usb_connected():
+ lock_device_if_unlocked()
+
+
+def set_homescreen() -> None:
+ import storage.recovery as storage_recovery
+
+ from apps.common import backup
+
+ set_default = workflow.set_default # local_cache_attribute
+
+ if utils.USE_POWER_MANAGER and _SHOULD_SUSPEND:
+ set_default(_suspend_and_resume)
+
+ elif context.cache_is_set(APP_COMMON_BUSY_DEADLINE_MS):
+ from apps.homescreen import busyscreen
+
+ set_default(busyscreen)
+
+ elif not config.is_unlocked():
+ from apps.homescreen import lockscreen
+
+ set_default(lockscreen)
+
+ elif _SCREENSAVER_IS_ON:
+ from apps.homescreen import screensaver
+
+ set_default(screensaver, restart=True)
+
+ elif storage_recovery.is_in_progress() or backup.repeated_backup_enabled():
+ from apps.management.recovery_device.homescreen import recovery_homescreen
+
+ set_default(recovery_homescreen)
+
+ else:
+ from apps.homescreen import homescreen
+
+ set_default(homescreen)
+
+
+def lock_device(interrupt_workflow: bool = True) -> None:
+ if config.has_pin():
+ config.lock()
+ filters.append(_pinlock_filter)
+ set_homescreen()
+ if interrupt_workflow:
+ workflow.close_others()
+ # TODO: should we suspend the device here?
+
+
+def lock_device_if_unlocked() -> None:
+ from apps.common.request_pin import can_lock_device
+
+ if not utils.USE_BACKLIGHT and not can_lock_device():
+ # on OLED devices without PIN, trigger screensaver
+ global _SCREENSAVER_IS_ON
+
+ _SCREENSAVER_IS_ON = True
+ set_homescreen()
+
+ elif config.is_unlocked():
+ lock_device(interrupt_workflow=workflow.autolock_interrupts_workflow)
+
+ if utils.USE_POWER_MANAGER:
+ _prepare_suspend()
+
+
+async def unlock_device() -> None:
+ """Ensure the device is in unlocked state.
+
+ If the storage is locked, attempt to unlock it. Reset the homescreen and the wire
+ handler.
+ """
+ from apps.common.request_pin import verify_user_pin
+
+ global _SCREENSAVER_IS_ON
+
+ if not config.is_unlocked():
+ # verify_user_pin will raise if the PIN was invalid
+ await verify_user_pin()
+
+ _SCREENSAVER_IS_ON = False
+ set_homescreen()
+ remove_filter(_pinlock_filter)
+
+
+def _pinlock_filter(msg_type: int, prev_handler: Handler[Msg]) -> Handler[Msg]:
+ if msg_type in workflow.ALLOW_WHILE_LOCKED:
+ return prev_handler
+
+ async def wrapper(msg: Msg) -> protobuf.MessageType:
+ await unlock_device()
+ return await prev_handler(msg)
+
+ return wrapper
+
+
+# this function is also called when handling ApplySettings
+def reload_settings_from_storage() -> None:
+ from trezor import ui
+
+ workflow.idle_timer.set(
+ storage_device.get_autolock_delay_ms(), lock_device_if_unlocked
+ )
+
+ if utils.USE_POWER_MANAGER:
+ # autodim setting is not from storage but keeping it here for simplicity
+ workflow.idle_timer.set(30_000, autodim_display)
+ workflow.idle_timer.set(
+ storage_device.get_autolock_delay_battery_ms(),
+ lock_device_if_unlocked_on_battery,
+ )
+ wire.message_handler.EXPERIMENTAL_ENABLED = (
+ storage_device.get_experimental_features()
+ )
+ if ui.display.orientation() != storage_device.get_rotation():
+ ui.backlight_fade(ui.BacklightLevels.DIM)
+ ui.display.orientation(storage_device.get_rotation())
+
+
+def boot() -> None:
+ set_homescreen()
+ if utils.USE_POWER_MANAGER:
+ loop.schedule(_power_handler())
diff --git a/core/src/apps/homescreen/__init__.py b/core/src/apps/homescreen/__init__.py
index a20341fd..9d785bb5 100644
--- a/core/src/apps/homescreen/__init__.py
+++ b/core/src/apps/homescreen/__init__.py
@@ -1,15 +1,15 @@
from typing import Coroutine
import storage
-import storage.cache
import storage.device
import trezorui_api
from trezor import config, utils, wire
from trezor.enums import MessageType
from trezor.ui.layouts.homescreen import Busyscreen, Homescreen, Lockscreen
-from apps.base import busy_expiry_ms, lock_device
+from apps.base import busy_expiry_ms
from apps.common.authorization import is_set_any_session
+from apps.common.lock_manager import lock_device
async def busyscreen() -> None:
@@ -71,7 +71,7 @@ async def homescreen() -> None:
async def _lockscreen(screensaver: bool = False) -> None:
- from apps.base import unlock_device
+ from apps.common.lock_manager import unlock_device
from apps.common.request_pin import can_lock_device
# Only show the lockscreen UI if the device can in fact be locked, or if it is
diff --git a/core/src/apps/management/apply_settings.py b/core/src/apps/management/apply_settings.py
index 0d9f2ccd..61b3ff34 100644
--- a/core/src/apps/management/apply_settings.py
+++ b/core/src/apps/management/apply_settings.py
@@ -51,8 +51,8 @@ async def apply_settings(msg: ApplySettings) -> Success:
from trezor.messages import Success
from trezor.wire import NotInitialized, ProcessError
- from apps.base import reload_settings_from_storage
from apps.common import safety_checks
+ from apps.common.lock_manager import reload_settings_from_storage
if not storage_device.is_initialized():
raise NotInitialized("Device is not initialized")
diff --git a/core/src/apps/management/wipe_device.py b/core/src/apps/management/wipe_device.py
index bcde9c8f..1ac19c6f 100644
--- a/core/src/apps/management/wipe_device.py
+++ b/core/src/apps/management/wipe_device.py
@@ -20,7 +20,7 @@ async def wipe_device(msg: WipeDevice) -> NoReturn:
from trezor.pin import render_empty_loader
from trezor.ui.layouts import confirm_action
- from apps.base import reload_settings_from_storage
+ from apps.common.lock_manager import reload_settings_from_storage
await confirm_action(
"confirm_wipe",
diff --git a/core/src/apps/webauthn/fido2.py b/core/src/apps/webauthn/fido2.py
index 864878fe..4343b0d4 100644
--- a/core/src/apps/webauthn/fido2.py
+++ b/core/src/apps/webauthn/fido2.py
@@ -11,8 +11,8 @@ from trezor.crypto.curve import nist256p1
from trezor.ui import Layout
from trezor.ui.layouts import error_popup
-from apps.base import set_homescreen
from apps.common import cbor
+from apps.common.lock_manager import set_homescreen
from . import common
from .credential import Credential, Fido2Credential
diff --git a/core/src/boot.py b/core/src/boot.py
index 9d1eedfe..c458f66b 100644
--- a/core/src/boot.py
+++ b/core/src/boot.py
@@ -26,7 +26,7 @@ if utils.USE_OPTIGA:
if utils.USE_POWER_MANAGER:
from trezor import workflow
from trezor.power_management.autodim import autodim_display
- from apps.base import lock_device_if_unlocked_on_battery
+ from apps.common.lock_manager import lock_device_if_unlocked_on_battery
# have to use "==" over "in (list)" so that it can be statically replaced
# with the correct value during the build process
diff --git a/core/src/session.py b/core/src/session.py
index deeb7ede..e45b7338 100644
--- a/core/src/session.py
+++ b/core/src/session.py
@@ -2,6 +2,7 @@
from trezor import log, loop, utils, wire, workflow
import apps.base
+from apps.common import lock_manager
import usb
apps.base.boot()
@@ -17,7 +18,7 @@ if __debug__:
apps.debug.boot()
# run main event loop and specify which screen is the default
-apps.base.set_homescreen()
+lock_manager.boot()
workflow.start_default()
if utils.USE_BLE:
diff --git a/core/src/trezor/ui/__init__.py b/core/src/trezor/ui/__init__.py
index 43f9fae4..3a8b6acb 100644
--- a/core/src/trezor/ui/__init__.py
+++ b/core/src/trezor/ui/__init__.py
@@ -17,7 +17,6 @@ from trezorui_api import (
)
if utils.USE_POWER_MANAGER:
- from trezor import config
from trezor.power_management.autodim import autodim_clear
if TYPE_CHECKING:
@@ -106,14 +105,9 @@ if utils.USE_POWER_MANAGER:
def _handle_power_button_press() -> None:
"""Handle power button press event during firmware operation."""
- from apps.base import lock_device_if_unlocked
+ from apps.common.lock_manager import notify_suspend
- will_close_workflow = config.has_pin() and workflow.autolock_interrupts_workflow
- lock_device_if_unlocked()
-
- if will_close_workflow:
- # prevent further layout interaction
- raise Shutdown()
+ notify_suspend()
class Layout(Generic[T]):
diff --git a/core/src/trezor/ui/layouts/homescreen.py b/core/src/trezor/ui/layouts/homescreen.py
index 7639dd82..72f092a5 100644
--- a/core/src/trezor/ui/layouts/homescreen.py
+++ b/core/src/trezor/ui/layouts/homescreen.py
@@ -145,7 +145,7 @@ class Busyscreen(HomescreenBase):
async def get_result(self) -> Any:
from trezor.wire import context
- from apps.base import set_homescreen
+ from apps.common.lock_manager import set_homescreen
# Handle timeout.
result = await super().get_result()
Risk score
Our methodology →Why this scored 27/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.