From 6ab66591cd42b3a5a99f17a160828ace305ed36d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Pasty=C5=99=C3=ADk?= Date: Thu, 6 Nov 2025 14:35:30 +0100 Subject: [PATCH 1/5] chore(core): remove double check when changing pin [no changelog] --- core/src/apps/management/change_pin.py | 4 ++-- storage/storage.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/apps/management/change_pin.py b/core/src/apps/management/change_pin.py index b82a690e05b..08b20908598 100644 --- a/core/src/apps/management/change_pin.py +++ b/core/src/apps/management/change_pin.py @@ -38,8 +38,8 @@ async def change_pin(msg: ChangePin) -> Success: # get old pin curpin, salt = await request_pin_and_sd_salt(TR.pin__enter) - # if changing pin, pre-check the entered pin before getting new pin - if curpin and not msg.remove: + # pre-check the entered pin before getting new pin + if curpin: if not config.check_pin(curpin, salt): await error_pin_invalid() diff --git a/storage/storage.c b/storage/storage.c index ba4f7c66b03..16ca905ef3d 100644 --- a/storage/storage.c +++ b/storage/storage.c @@ -1712,11 +1712,11 @@ secbool storage_change_pin(const uint8_t *oldpin, size_t oldpin_len, mpu_mode_t mpu_mode = mpu_reconfig(MPU_MODE_STORAGE); - ui_progress_init(STORAGE_PIN_OP_CHANGE); + ui_progress_init(STORAGE_PIN_OP_SET); ui_message = (oldpin_len != 0 && newpin_len == 0) ? VERIFYING_PIN_MSG : PROCESSING_MSG; - secbool ret = unlock(oldpin, oldpin_len, old_ext_salt); + secbool ret = storage_is_unlocked(); if (sectrue != ret) { goto end; } From b548d54a5866572d7bf56541b654da1bf5680371 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Pasty=C5=99=C3=ADk?= Date: Mon, 10 Nov 2025 16:34:43 +0100 Subject: [PATCH 2/5] fixup! chore(core): remove double check when changing pin --- core/embed/upymod/modtrezorconfig/modtrezorconfig.c | 1 + core/mocks/generated/trezorconfig.pyi | 3 ++- core/src/apps/management/change_pin.py | 2 +- core/tests/test_trezor.config.py | 6 +----- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/core/embed/upymod/modtrezorconfig/modtrezorconfig.c b/core/embed/upymod/modtrezorconfig/modtrezorconfig.c index cea36e87442..ea78b600d21 100644 --- a/core/embed/upymod/modtrezorconfig/modtrezorconfig.c +++ b/core/embed/upymod/modtrezorconfig/modtrezorconfig.c @@ -159,6 +159,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_0(mod_trezorconfig_get_pin_rem_obj, /// ) -> bool: /// """ /// Change PIN and external salt. Returns True on success, False on failure. +/// Has to be run with unlocked storage. /// """ STATIC mp_obj_t mod_trezorconfig_change_pin(size_t n_args, const mp_obj_t *args) { diff --git a/core/mocks/generated/trezorconfig.pyi b/core/mocks/generated/trezorconfig.pyi index f7ab806f5b0..b273952028b 100644 --- a/core/mocks/generated/trezorconfig.pyi +++ b/core/mocks/generated/trezorconfig.pyi @@ -66,7 +66,8 @@ def change_pin( new_ext_salt: AnyBytes | None, ) -> bool: """ - Change PIN and external salt. Returns True on success, False on failure. + Change PIN and external salt. Returns True on success, False on failure. Has to be + ran with unlocked storage. """ diff --git a/core/src/apps/management/change_pin.py b/core/src/apps/management/change_pin.py index 08b20908598..ff29909dc0f 100644 --- a/core/src/apps/management/change_pin.py +++ b/core/src/apps/management/change_pin.py @@ -38,7 +38,7 @@ async def change_pin(msg: ChangePin) -> Success: # get old pin curpin, salt = await request_pin_and_sd_salt(TR.pin__enter) - # pre-check the entered pin before getting new pin + # check the entered pin before getting new pin if curpin: if not config.check_pin(curpin, salt): await error_pin_invalid() diff --git a/core/tests/test_trezor.config.py b/core/tests/test_trezor.config.py index 4eb1dfc34f9..8b37493ba57 100644 --- a/core/tests/test_trezor.config.py +++ b/core/tests/test_trezor.config.py @@ -102,10 +102,6 @@ def test_change_pin(self): self.assertTrue(config.change_pin(old_pin, new_pin, None, None)) - # Old PIN cannot be used to change the current PIN. - if old_pin != new_pin: - self.assertFalse(config.change_pin(old_pin, "666", None, None)) - # Storage remains unlocked. self.assertEqual(config.get(1, 1), b"value") @@ -139,7 +135,7 @@ def test_change_sd_salt(self): config.wipe() self.assertTrue(config.unlock("", None)) config.set(1, 1, b"value") - self.assertFalse(config.change_pin("", "", salt1, None)) + self.assertFalse(config.unlock("", salt1)) self.assertTrue(config.change_pin("", "000", None, salt1)) self.assertEqual(config.get(1, 1), b"value") From 2b6b6dfa09ad254f9fdcf518f668c720b93c75b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Pasty=C5=99=C3=ADk?= Date: Thu, 13 Nov 2025 16:39:58 +0100 Subject: [PATCH 3/5] chore(core): remove the unused arguments --- core/embed/rust/src/trezorhal/storage.rs | 14 +++------- core/embed/sys/smcall/stm32/smcall_dispatch.c | 12 +++------ core/embed/sys/smcall/stm32/smcall_stubs.c | 7 ++--- .../embed/sys/smcall/stm32/smcall_verifiers.c | 7 ++--- .../embed/sys/smcall/stm32/smcall_verifiers.h | 4 +-- .../sys/syscall/stm32/syscall_dispatch.c | 12 +++------ core/embed/sys/syscall/stm32/syscall_stubs.c | 11 +++----- .../sys/syscall/stm32/syscall_verifiers.c | 7 ++--- .../sys/syscall/stm32/syscall_verifiers.h | 4 +-- .../upymod/modtrezorconfig/modtrezorconfig.c | 26 +++++-------------- core/mocks/generated/trezorconfig.pyi | 6 ++--- core/src/apps/debug/load_device.py | 2 +- core/src/apps/management/change_pin.py | 2 +- .../management/recovery_device/__init__.py | 2 +- .../apps/management/reset_device/__init__.py | 2 +- core/src/apps/management/sd_protect.py | 8 +++--- core/tests/test_trezor.config.py | 8 +++--- storage/storage.c | 9 +++---- storage/storage.h | 4 +-- storage/tests/c/storage.py | 3 --- 20 files changed, 48 insertions(+), 102 deletions(-) diff --git a/core/embed/rust/src/trezorhal/storage.rs b/core/embed/rust/src/trezorhal/storage.rs index f790f2a56b6..f361165ade9 100644 --- a/core/embed/rust/src/trezorhal/storage.rs +++ b/core/embed/rust/src/trezorhal/storage.rs @@ -134,20 +134,12 @@ pub fn unlock(pin: &str, salt: Option<&ExternalSalt>) -> bool { /// Change PIN and/or external salt. /// Returns true if the PIN + salt combination is correct and the change was /// successful. -pub fn change_pin( - old_pin: &str, - new_pin: &str, - old_salt: Option<&ExternalSalt>, - new_salt: Option<&ExternalSalt>, -) -> bool { +pub fn change_pin(new_pin: &str, new_salt: Option<&ExternalSalt>) -> bool { ffi::sectrue == unsafe { ffi::storage_change_pin( - old_pin.as_ptr() as *const _, - old_pin.len(), new_pin.as_ptr() as *const _, new_pin.len(), - old_salt.map(|s| s.as_ptr()).unwrap_or(ptr::null()), new_salt.map(|s| s.as_ptr()).unwrap_or(ptr::null()), ) } @@ -333,8 +325,8 @@ mod tests { assert!(!unlock("1234", None)); - //unlock("", None); - assert!(change_pin("", "1234", None, None)); + unlock("", None); + assert!(change_pin("1234", None)); assert!(has_pin()); lock(); diff --git a/core/embed/sys/smcall/stm32/smcall_dispatch.c b/core/embed/sys/smcall/stm32/smcall_dispatch.c index d02ea5fb31e..b466e92a98d 100644 --- a/core/embed/sys/smcall/stm32/smcall_dispatch.c +++ b/core/embed/sys/smcall/stm32/smcall_dispatch.c @@ -241,14 +241,10 @@ __attribute((no_stack_protector)) void smcall_handler(uint32_t *args, } break; case SMCALL_STORAGE_CHANGE_PIN: { - const uint8_t *oldpin = (const uint8_t *)args[0]; - size_t oldpin_len = args[1]; - const uint8_t *newpin = (const uint8_t *)args[2]; - size_t newpin_len = args[3]; - const uint8_t *old_ext_salt = (const uint8_t *)args[4]; - const uint8_t *new_ext_salt = (const uint8_t *)args[5]; - args[0] = storage_change_pin__verified( - oldpin, oldpin_len, newpin, newpin_len, old_ext_salt, new_ext_salt); + const uint8_t *newpin = (const uint8_t *)args[0]; + size_t newpin_len = args[1]; + const uint8_t *new_ext_salt = (const uint8_t *)args[2]; + args[0] = storage_change_pin__verified(newpin, newpin_len, new_ext_salt); } break; case SMCALL_STORAGE_ENSURE_NOT_WIPE_CODE: { diff --git a/core/embed/sys/smcall/stm32/smcall_stubs.c b/core/embed/sys/smcall/stm32/smcall_stubs.c index 8c56aaff65e..6dd5ae5e064 100644 --- a/core/embed/sys/smcall/stm32/smcall_stubs.c +++ b/core/embed/sys/smcall/stm32/smcall_stubs.c @@ -247,12 +247,9 @@ uint32_t storage_get_pin_rem(void) { return smcall_invoke0(SMCALL_STORAGE_GET_PIN_REM); } -secbool storage_change_pin(const uint8_t *oldpin, size_t oldpin_len, - const uint8_t *newpin, size_t newpin_len, - const uint8_t *old_ext_salt, +secbool storage_change_pin(const uint8_t *newpin, size_t newpin_len, const uint8_t *new_ext_salt) { - return (secbool)smcall_invoke6((uint32_t)oldpin, oldpin_len, (uint32_t)newpin, - newpin_len, (uint32_t)old_ext_salt, + return (secbool)smcall_invoke3((uint32_t)newpin, newpin_len, (uint32_t)new_ext_salt, SMCALL_STORAGE_CHANGE_PIN); } diff --git a/core/embed/sys/smcall/stm32/smcall_verifiers.c b/core/embed/sys/smcall/stm32/smcall_verifiers.c index ffc21bba935..8010a62e5ee 100644 --- a/core/embed/sys/smcall/stm32/smcall_verifiers.c +++ b/core/embed/sys/smcall/stm32/smcall_verifiers.c @@ -279,9 +279,7 @@ secbool storage_unlock__verified(const uint8_t *pin, size_t pin_len, return secfalse; } -secbool storage_change_pin__verified(const uint8_t *oldpin, size_t oldpin_len, - const uint8_t *newpin, size_t newpin_len, - const uint8_t *old_ext_salt, +secbool storage_change_pin__verified(const uint8_t *newpin, size_t newpin_len, const uint8_t *new_ext_salt) { if (!probe_read_access(oldpin, oldpin_len)) { goto access_violation; @@ -299,8 +297,7 @@ secbool storage_change_pin__verified(const uint8_t *oldpin, size_t oldpin_len, goto access_violation; } - return storage_change_pin(oldpin, oldpin_len, newpin, newpin_len, - old_ext_salt, new_ext_salt); + return storage_change_pin(newpin, newpin_len, new_ext_salt); access_violation: apptask_access_violation(); diff --git a/core/embed/sys/smcall/stm32/smcall_verifiers.h b/core/embed/sys/smcall/stm32/smcall_verifiers.h index eca1c646613..b8aeceffa6e 100644 --- a/core/embed/sys/smcall/stm32/smcall_verifiers.h +++ b/core/embed/sys/smcall/stm32/smcall_verifiers.h @@ -87,9 +87,7 @@ void storage_setup__verified(PIN_UI_WAIT_CALLBACK callback); secbool storage_unlock__verified(const uint8_t *pin, size_t pin_len, const uint8_t *ext_salt); -secbool storage_change_pin__verified(const uint8_t *oldpin, size_t oldpin_len, - const uint8_t *newpin, size_t newpin_len, - const uint8_t *old_ext_salt, +secbool storage_change_pin__verified(const uint8_t *newpin, size_t newpin_len, const uint8_t *new_ext_salt); void storage_ensure_not_wipe_code__verified(const uint8_t *pin, size_t pin_len); diff --git a/core/embed/sys/syscall/stm32/syscall_dispatch.c b/core/embed/sys/syscall/stm32/syscall_dispatch.c index 62963537f9b..06600883c78 100644 --- a/core/embed/sys/syscall/stm32/syscall_dispatch.c +++ b/core/embed/sys/syscall/stm32/syscall_dispatch.c @@ -483,14 +483,10 @@ __attribute((no_stack_protector)) void syscall_handler(uint32_t *args, } break; case SYSCALL_STORAGE_CHANGE_PIN: { - const uint8_t *oldpin = (const uint8_t *)args[0]; - size_t oldpin_len = args[1]; - const uint8_t *newpin = (const uint8_t *)args[2]; - size_t newpin_len = args[3]; - const uint8_t *old_ext_salt = (const uint8_t *)args[4]; - const uint8_t *new_ext_salt = (const uint8_t *)args[5]; - args[0] = storage_change_pin__verified( - oldpin, oldpin_len, newpin, newpin_len, old_ext_salt, new_ext_salt); + const uint8_t *newpin = (const uint8_t *)args[0]; + size_t newpin_len = args[1]; + const uint8_t *new_ext_salt = (const uint8_t *)args[2]; + args[0] = storage_change_pin__verified(newpin, newpin_len, new_ext_salt); } break; case SYSCALL_STORAGE_ENSURE_NOT_WIPE_CODE: { diff --git a/core/embed/sys/syscall/stm32/syscall_stubs.c b/core/embed/sys/syscall/stm32/syscall_stubs.c index c19c51c5e91..dcc534c5f62 100644 --- a/core/embed/sys/syscall/stm32/syscall_stubs.c +++ b/core/embed/sys/syscall/stm32/syscall_stubs.c @@ -482,14 +482,11 @@ uint32_t storage_get_pin_rem(void) { return syscall_invoke0(SYSCALL_STORAGE_GET_PIN_REM); } -secbool storage_change_pin(const uint8_t *oldpin, size_t oldpin_len, - const uint8_t *newpin, size_t newpin_len, - const uint8_t *old_ext_salt, +secbool storage_change_pin(const uint8_t *newpin, size_t newpin_len, const uint8_t *new_ext_salt) { - return (secbool)syscall_invoke6( - (uint32_t)oldpin, oldpin_len, (uint32_t)newpin, newpin_len, - (uint32_t)old_ext_salt, (uint32_t)new_ext_salt, - SYSCALL_STORAGE_CHANGE_PIN); + return (secbool)syscall_invoke3((uint32_t)newpin, newpin_len, + (uint32_t)new_ext_salt, + SYSCALL_STORAGE_CHANGE_PIN); } void storage_ensure_not_wipe_code(const uint8_t *pin, size_t pin_len) { diff --git a/core/embed/sys/syscall/stm32/syscall_verifiers.c b/core/embed/sys/syscall/stm32/syscall_verifiers.c index 70857d50df3..9ef029883e3 100644 --- a/core/embed/sys/syscall/stm32/syscall_verifiers.c +++ b/core/embed/sys/syscall/stm32/syscall_verifiers.c @@ -538,9 +538,7 @@ secbool storage_unlock__verified(const uint8_t *pin, size_t pin_len, return secfalse; } -secbool storage_change_pin__verified(const uint8_t *oldpin, size_t oldpin_len, - const uint8_t *newpin, size_t newpin_len, - const uint8_t *old_ext_salt, +secbool storage_change_pin__verified(const uint8_t *newpin, size_t newpin_len, const uint8_t *new_ext_salt) { if (!probe_read_access(oldpin, oldpin_len)) { goto access_violation; @@ -558,8 +556,7 @@ secbool storage_change_pin__verified(const uint8_t *oldpin, size_t oldpin_len, goto access_violation; } - return storage_change_pin(oldpin, oldpin_len, newpin, newpin_len, - old_ext_salt, new_ext_salt); + return storage_change_pin(newpin, newpin_len, new_ext_salt); access_violation: apptask_access_violation(); diff --git a/core/embed/sys/syscall/stm32/syscall_verifiers.h b/core/embed/sys/syscall/stm32/syscall_verifiers.h index 350828b49df..a8523d921c2 100644 --- a/core/embed/sys/syscall/stm32/syscall_verifiers.h +++ b/core/embed/sys/syscall/stm32/syscall_verifiers.h @@ -143,9 +143,7 @@ void storage_setup__verified(PIN_UI_WAIT_CALLBACK callback); secbool storage_unlock__verified(const uint8_t *pin, size_t pin_len, const uint8_t *ext_salt); -secbool storage_change_pin__verified(const uint8_t *oldpin, size_t oldpin_len, - const uint8_t *newpin, size_t newpin_len, - const uint8_t *old_ext_salt, +secbool storage_change_pin__verified(const uint8_t *newpin, size_t newpin_len, const uint8_t *new_ext_salt); void storage_ensure_not_wipe_code__verified(const uint8_t *pin, size_t pin_len); diff --git a/core/embed/upymod/modtrezorconfig/modtrezorconfig.c b/core/embed/upymod/modtrezorconfig/modtrezorconfig.c index ea78b600d21..3286d751768 100644 --- a/core/embed/upymod/modtrezorconfig/modtrezorconfig.c +++ b/core/embed/upymod/modtrezorconfig/modtrezorconfig.c @@ -152,9 +152,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_0(mod_trezorconfig_get_pin_rem_obj, mod_trezorconfig_get_pin_rem); /// def change_pin( -/// oldpin: str, /// newpin: str, -/// old_ext_salt: AnyBytes | None, /// new_ext_salt: AnyBytes | None, /// ) -> bool: /// """ @@ -163,38 +161,26 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_0(mod_trezorconfig_get_pin_rem_obj, /// """ STATIC mp_obj_t mod_trezorconfig_change_pin(size_t n_args, const mp_obj_t *args) { - mp_buffer_info_t oldpin = {0}; - mp_get_buffer_raise(args[0], &oldpin, MP_BUFFER_READ); - mp_buffer_info_t newpin = {0}; - mp_get_buffer_raise(args[1], &newpin, MP_BUFFER_READ); + mp_get_buffer_raise(args[0], &newpin, MP_BUFFER_READ); mp_buffer_info_t ext_salt_b = {0}; - const uint8_t *old_ext_salt = NULL; - if (args[2] != mp_const_none) { - mp_get_buffer_raise(args[2], &ext_salt_b, MP_BUFFER_READ); - if (ext_salt_b.len != EXTERNAL_SALT_SIZE) - mp_raise_msg(&mp_type_ValueError, - MP_ERROR_TEXT("Invalid length of external salt.")); - old_ext_salt = ext_salt_b.buf; - } const uint8_t *new_ext_salt = NULL; - if (args[3] != mp_const_none) { - mp_get_buffer_raise(args[3], &ext_salt_b, MP_BUFFER_READ); + if (args[1] != mp_const_none) { + mp_get_buffer_raise(args[1], &ext_salt_b, MP_BUFFER_READ); if (ext_salt_b.len != EXTERNAL_SALT_SIZE) mp_raise_msg(&mp_type_ValueError, MP_ERROR_TEXT("Invalid length of external salt.")); new_ext_salt = ext_salt_b.buf; } - if (sectrue != storage_change_pin(oldpin.buf, oldpin.len, newpin.buf, - newpin.len, old_ext_salt, new_ext_salt)) { + if (sectrue != storage_change_pin(newpin.buf, newpin.len, new_ext_salt)) { return mp_const_false; } return mp_const_true; } -STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mod_trezorconfig_change_pin_obj, 4, - 4, mod_trezorconfig_change_pin); +STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mod_trezorconfig_change_pin_obj, 2, + 2, mod_trezorconfig_change_pin); /// def ensure_not_wipe_code(pin: str) -> None: /// """ diff --git a/core/mocks/generated/trezorconfig.pyi b/core/mocks/generated/trezorconfig.pyi index b273952028b..6da8cb8e373 100644 --- a/core/mocks/generated/trezorconfig.pyi +++ b/core/mocks/generated/trezorconfig.pyi @@ -60,14 +60,12 @@ def get_pin_rem() -> int: # upymod/modtrezorconfig/modtrezorconfig.c def change_pin( - oldpin: str, newpin: str, - old_ext_salt: AnyBytes | None, new_ext_salt: AnyBytes | None, ) -> bool: """ - Change PIN and external salt. Returns True on success, False on failure. Has to be - ran with unlocked storage. + Change PIN and external salt. Returns True on success, False on failure. + Has to be run with unlocked storage. """ diff --git a/core/src/apps/debug/load_device.py b/core/src/apps/debug/load_device.py index c1da1b94721..3cc6ff94aea 100644 --- a/core/src/apps/debug/load_device.py +++ b/core/src/apps/debug/load_device.py @@ -70,6 +70,6 @@ async def load_device(msg: LoadDevice) -> Success: storage_device.set_passphrase_enabled(bool(msg.passphrase_protection)) storage_device.set_label(msg.label or "") if msg.pin: - config.change_pin("", msg.pin, None, None) + config.change_pin(msg.pin, None) return Success(message="Device loaded") diff --git a/core/src/apps/management/change_pin.py b/core/src/apps/management/change_pin.py index ff29909dc0f..b1fccbe19a3 100644 --- a/core/src/apps/management/change_pin.py +++ b/core/src/apps/management/change_pin.py @@ -50,7 +50,7 @@ async def change_pin(msg: ChangePin) -> Success: newpin = "" # write into storage - if not config.change_pin(curpin, newpin, salt, salt): + if not config.change_pin(newpin, salt): if newpin: await error_pin_matches_wipe_code() else: diff --git a/core/src/apps/management/recovery_device/__init__.py b/core/src/apps/management/recovery_device/__init__.py index e1c3b03ca4f..1a25b3c74ba 100644 --- a/core/src/apps/management/recovery_device/__init__.py +++ b/core/src/apps/management/recovery_device/__init__.py @@ -81,7 +81,7 @@ async def recovery_device(msg: RecoveryDevice) -> Success: # set up pin if requested if msg.pin_protection: newpin = await request_pin_confirm(allow_cancel=False) - config.change_pin("", newpin, None, None) + config.change_pin(newpin, None) storage_device.set_passphrase_enabled(bool(msg.passphrase_protection)) diff --git a/core/src/apps/management/reset_device/__init__.py b/core/src/apps/management/reset_device/__init__.py index ff2fb3dfd89..1074b5e6558 100644 --- a/core/src/apps/management/reset_device/__init__.py +++ b/core/src/apps/management/reset_device/__init__.py @@ -78,7 +78,7 @@ async def reset_device(msg: ResetDevice) -> Success: # request and set new PIN if msg.pin_protection: newpin = await request_pin_confirm() - if not config.change_pin("", newpin, None, None): + if not config.change_pin(newpin, None): raise ProcessError("Failed to set PIN") prev_int_entropy = None diff --git a/core/src/apps/management/sd_protect.py b/core/src/apps/management/sd_protect.py index 74717fea02d..80a47330c0d 100644 --- a/core/src/apps/management/sd_protect.py +++ b/core/src/apps/management/sd_protect.py @@ -75,7 +75,7 @@ async def _sd_protect_enable(msg: SdProtect) -> Success: salt, salt_auth_key, salt_tag = _make_salt() await _set_salt(salt, salt_tag) - if not config.change_pin(pin, pin, None, salt): + if not config.unlock(pin, None): # Wrong PIN. Clean up the prepared salt file. try: storage_sd_salt.remove_sd_salt() @@ -85,6 +85,7 @@ async def _sd_protect_enable(msg: SdProtect) -> Success: # exception, because primarily we need to raise wire.PinInvalid. pass await error_pin_invalid() + config.change_pin(pin, salt) storage_device.set_sd_salt_auth_key(salt_auth_key) @@ -106,7 +107,7 @@ async def _sd_protect_disable(msg: SdProtect) -> Success: pin, salt = await request_pin_and_sd_salt(TR.pin__enter) # Check PIN and remove salt. - if not config.change_pin(pin, pin, salt, None): + if not config.unlock(pin, salt): await error_pin_invalid() storage_device.set_sd_salt_auth_key(None) @@ -141,8 +142,9 @@ async def _sd_protect_refresh(msg: SdProtect) -> Success: new_salt, new_auth_key, new_salt_tag = _make_salt() await _set_salt(new_salt, new_salt_tag, stage=True) - if not config.change_pin(pin, pin, old_salt, new_salt): + if not config.unlock(pin, old_salt): await error_pin_invalid() + config.change_pin(pin, new_salt) storage_device.set_sd_salt_auth_key(new_auth_key) diff --git a/core/tests/test_trezor.config.py b/core/tests/test_trezor.config.py index 8b37493ba57..5723452465d 100644 --- a/core/tests/test_trezor.config.py +++ b/core/tests/test_trezor.config.py @@ -100,7 +100,7 @@ def test_change_pin(self): with self.assertRaises(ValueError): config.set(PINAPP, PINKEY, b"value") - self.assertTrue(config.change_pin(old_pin, new_pin, None, None)) + self.assertTrue(config.change_pin(new_pin, None)) # Storage remains unlocked. self.assertEqual(config.get(1, 1), b"value") @@ -136,7 +136,7 @@ def test_change_sd_salt(self): self.assertTrue(config.unlock("", None)) config.set(1, 1, b"value") self.assertFalse(config.unlock("", salt1)) - self.assertTrue(config.change_pin("", "000", None, salt1)) + self.assertTrue(config.change_pin("000", salt1)) self.assertEqual(config.get(1, 1), b"value") # Disable PIN and change SD salt. @@ -144,7 +144,7 @@ def test_change_sd_salt(self): self.assertFalse(config.unlock("000", None)) self.assertIsNone(config.get(1, 1)) self.assertTrue(config.unlock("000", salt1)) - self.assertTrue(config.change_pin("000", "", salt1, salt2)) + self.assertTrue(config.change_pin("", salt2)) self.assertEqual(config.get(1, 1), b"value") # Disable SD salt. @@ -152,7 +152,7 @@ def test_change_sd_salt(self): self.assertFalse(config.unlock("000", salt2)) self.assertIsNone(config.get(1, 1)) self.assertTrue(config.unlock("", salt2)) - self.assertTrue(config.change_pin("", "", salt2, None)) + self.assertTrue(config.change_pin("", None)) self.assertEqual(config.get(1, 1), b"value") # Check that PIN and SD salt are disabled. diff --git a/storage/storage.c b/storage/storage.c index 16ca905ef3d..cf4dfb51916 100644 --- a/storage/storage.c +++ b/storage/storage.c @@ -1702,19 +1702,16 @@ uint32_t storage_get_pin_rem(void) { return rem_mcu; } -secbool storage_change_pin(const uint8_t *oldpin, size_t oldpin_len, - const uint8_t *newpin, size_t newpin_len, - const uint8_t *old_ext_salt, +secbool storage_change_pin(const uint8_t *newpin, size_t newpin_len, const uint8_t *new_ext_salt) { - if (sectrue != initialized || oldpin == NULL || newpin == NULL) { + if (sectrue != initialized || newpin == NULL) { return secfalse; } mpu_mode_t mpu_mode = mpu_reconfig(MPU_MODE_STORAGE); ui_progress_init(STORAGE_PIN_OP_SET); - ui_message = - (oldpin_len != 0 && newpin_len == 0) ? VERIFYING_PIN_MSG : PROCESSING_MSG; + ui_message = newpin_len == 0 ? VERIFYING_PIN_MSG : PROCESSING_MSG; secbool ret = storage_is_unlocked(); if (sectrue != ret) { diff --git a/storage/storage.h b/storage/storage.h index 6890bf814ba..1e4628c1f8c 100644 --- a/storage/storage.h +++ b/storage/storage.h @@ -94,9 +94,7 @@ secbool storage_unlock(const uint8_t *pin, size_t pin_len, secbool storage_has_pin(void); secbool storage_pin_fails_increase(void); uint32_t storage_get_pin_rem(void); -secbool storage_change_pin(const uint8_t *oldpin, size_t oldpin_len, - const uint8_t *newpin, size_t newpin_len, - const uint8_t *old_ext_salt, +secbool storage_change_pin(const uint8_t *newpin, size_t newpin_len, const uint8_t *new_ext_salt); void storage_ensure_not_wipe_code(const uint8_t *pin, size_t pin_len); secbool storage_has_wipe_code(void); diff --git a/storage/tests/c/storage.py b/storage/tests/c/storage.py index 16c08edcf78..db890a42395 100644 --- a/storage/tests/c/storage.py +++ b/storage/tests/c/storage.py @@ -55,11 +55,8 @@ def change_pin( if new_ext_salt is not None and len(new_ext_salt) != EXTERNAL_SALT_LEN: raise ValueError return sectrue == self.lib.storage_change_pin( - oldpin.encode(), - len(oldpin), newpin.encode(), len(newpin), - old_ext_salt, new_ext_salt, ) From 8d225fc157f64a4693bb0d58d8240594434babb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Pasty=C5=99=C3=ADk?= Date: Fri, 14 Nov 2025 09:31:59 +0100 Subject: [PATCH 4/5] fixup! chore(core): remove the unused arguments --- core/embed/sys/smcall/stm32/smcall_verifiers.c | 8 -------- core/embed/sys/syscall/stm32/syscall_verifiers.c | 8 -------- 2 files changed, 16 deletions(-) diff --git a/core/embed/sys/smcall/stm32/smcall_verifiers.c b/core/embed/sys/smcall/stm32/smcall_verifiers.c index 8010a62e5ee..2450f36cf5b 100644 --- a/core/embed/sys/smcall/stm32/smcall_verifiers.c +++ b/core/embed/sys/smcall/stm32/smcall_verifiers.c @@ -281,18 +281,10 @@ secbool storage_unlock__verified(const uint8_t *pin, size_t pin_len, secbool storage_change_pin__verified(const uint8_t *newpin, size_t newpin_len, const uint8_t *new_ext_salt) { - if (!probe_read_access(oldpin, oldpin_len)) { - goto access_violation; - } - if (!probe_read_access(newpin, newpin_len)) { goto access_violation; } - if (!probe_read_access(old_ext_salt, EXTERNAL_SALT_SIZE)) { - goto access_violation; - } - if (!probe_read_access(new_ext_salt, EXTERNAL_SALT_SIZE)) { goto access_violation; } diff --git a/core/embed/sys/syscall/stm32/syscall_verifiers.c b/core/embed/sys/syscall/stm32/syscall_verifiers.c index 9ef029883e3..207828be42e 100644 --- a/core/embed/sys/syscall/stm32/syscall_verifiers.c +++ b/core/embed/sys/syscall/stm32/syscall_verifiers.c @@ -540,18 +540,10 @@ secbool storage_unlock__verified(const uint8_t *pin, size_t pin_len, secbool storage_change_pin__verified(const uint8_t *newpin, size_t newpin_len, const uint8_t *new_ext_salt) { - if (!probe_read_access(oldpin, oldpin_len)) { - goto access_violation; - } - if (!probe_read_access(newpin, newpin_len)) { goto access_violation; } - if (!probe_read_access(old_ext_salt, EXTERNAL_SALT_SIZE)) { - goto access_violation; - } - if (!probe_read_access(new_ext_salt, EXTERNAL_SALT_SIZE)) { goto access_violation; } From c020ed042c521fa557f3ebe1b8d4556f557e6dc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Pasty=C5=99=C3=ADk?= Date: Fri, 14 Nov 2025 09:57:12 +0100 Subject: [PATCH 5/5] fixup! fixup! chore(core): remove the unused arguments --- legacy/firmware/config.c | 11 +++++------ legacy/firmware/config.h | 2 +- legacy/firmware/protect.c | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/legacy/firmware/config.c b/legacy/firmware/config.c index 0c6c3ff2a26..7aeb7fe1fa1 100644 --- a/legacy/firmware/config.c +++ b/legacy/firmware/config.c @@ -322,8 +322,8 @@ static secbool config_upgrade_v10(void) { if (config.has_pin) { size_t pin_len = MIN(strnlen(config.pin, sizeof(config.pin)), (size_t)MAX_PIN_LEN); - storage_change_pin(PIN_EMPTY, PIN_EMPTY_LEN, (const uint8_t *)config.pin, - pin_len, NULL, NULL); + storage_change_pin((const uint8_t *)config.pin, + pin_len, NULL); } while (pin_wait != 0) { @@ -505,7 +505,7 @@ void config_loadDevice(const LoadDevice *msg) { msg->passphrase_protection); if (msg->has_pin) { - config_changePin("", msg->pin); + config_changePin(msg->pin); } if (msg->mnemonics_count) { @@ -827,11 +827,10 @@ bool config_unlock(const char *pin) { bool config_hasPin(void) { return sectrue == storage_has_pin(); } -bool config_changePin(const char *old_pin, const char *new_pin) { +bool config_changePin(const char *new_pin) { char oldTiny = usbTiny(1); secbool ret = storage_change_pin( - (const uint8_t *)old_pin, strnlen(old_pin, MAX_PIN_LEN), - (const uint8_t *)new_pin, strnlen(new_pin, MAX_PIN_LEN), NULL, NULL); + (const uint8_t *)new_pin, strnlen(new_pin, MAX_PIN_LEN), NULL); usbTiny(oldTiny); #if DEBUG_LINK diff --git a/legacy/firmware/config.h b/legacy/firmware/config.h index fe523ada486..12d9dbe6096 100644 --- a/legacy/firmware/config.h +++ b/legacy/firmware/config.h @@ -139,7 +139,7 @@ bool config_getPin(char *dest, uint16_t dest_size); bool config_unlock(const char *pin); bool config_hasPin(void); -bool config_changePin(const char *old_pin, const char *new_pin); +bool config_changePin(const char *new_pin); bool session_isUnlocked(void); bool config_hasWipeCode(void); diff --git a/legacy/firmware/protect.c b/legacy/firmware/protect.c index 74e6d9d9ea2..388b3a284d8 100644 --- a/legacy/firmware/protect.c +++ b/legacy/firmware/protect.c @@ -289,7 +289,7 @@ bool protectChangePin(bool removal) { } } - bool ret = config_changePin(old_pin, new_pin); + bool ret = config_changePin(new_pin); memzero(old_pin, sizeof(old_pin)); memzero(new_pin, sizeof(new_pin)); if (ret == false) {