From 4754fd8e345ec90d2d25240584055df9d35e5bde Mon Sep 17 00:00:00 2001 From: cepetr Date: Mon, 14 Jul 2025 14:36:41 +0200 Subject: [PATCH] refactor(core): reconfigure mpu directly in the storage [no changelog] --- core/embed/sys/smcall/stm32/smcall_dispatch.c | 19 ---- .../sys/syscall/stm32/syscall_dispatch.c | 19 ---- legacy/sys/mpu.h | 32 ++++++ storage/storage.c | 98 ++++++++++++++++--- storage/tests/c/sys/mpu.h | 32 ++++++ 5 files changed, 146 insertions(+), 54 deletions(-) create mode 100644 legacy/sys/mpu.h create mode 100644 storage/tests/c/sys/mpu.h diff --git a/core/embed/sys/smcall/stm32/smcall_dispatch.c b/core/embed/sys/smcall/stm32/smcall_dispatch.c index 6bc83cebdb..7d4799d4cf 100644 --- a/core/embed/sys/smcall/stm32/smcall_dispatch.c +++ b/core/embed/sys/smcall/stm32/smcall_dispatch.c @@ -27,7 +27,6 @@ #include #include #include -#include #include #include #include @@ -197,22 +196,18 @@ __attribute((no_stack_protector)) void smcall_handler(uint32_t *args, PIN_UI_WAIT_CALLBACK callback = (PIN_UI_WAIT_CALLBACK)args[0]; const uint8_t *salt = (const uint8_t *)args[1]; uint16_t salt_len = args[2]; - mpu_reconfig(MPU_MODE_STORAGE); storage_init__verified(callback, salt, salt_len); } break; case SMCALL_STORAGE_WIPE: { - mpu_reconfig(MPU_MODE_STORAGE); storage_wipe(); } break; case SMCALL_STORAGE_IS_UNLOCKED: { - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_is_unlocked(); } break; case SMCALL_STORAGE_LOCK: { - mpu_reconfig(MPU_MODE_STORAGE); storage_lock(); } break; @@ -220,22 +215,18 @@ __attribute((no_stack_protector)) void smcall_handler(uint32_t *args, const uint8_t *pin = (const uint8_t *)args[0]; size_t pin_len = args[1]; const uint8_t *ext_salt = (const uint8_t *)args[2]; - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_unlock__verified(pin, pin_len, ext_salt); } break; case SMCALL_STORAGE_HAS_PIN: { - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_has_pin(); } break; case SMCALL_STORAGE_PIN_FAILS_INCREASE: { - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_pin_fails_increase(); } break; case SMCALL_STORAGE_GET_PIN_REM: { - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_get_pin_rem(); } break; @@ -246,7 +237,6 @@ __attribute((no_stack_protector)) void smcall_handler(uint32_t *args, 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]; - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_change_pin__verified( oldpin, oldpin_len, newpin, newpin_len, old_ext_salt, new_ext_salt); } break; @@ -254,12 +244,10 @@ __attribute((no_stack_protector)) void smcall_handler(uint32_t *args, case SMCALL_STORAGE_ENSURE_NOT_WIPE_CODE: { const uint8_t *pin = (const uint8_t *)args[0]; size_t pin_len = args[1]; - mpu_reconfig(MPU_MODE_STORAGE); storage_ensure_not_wipe_code__verified(pin, pin_len); } break; case SMCALL_STORAGE_HAS_WIPE_CODE: { - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_has_wipe_code(); } break; @@ -269,14 +257,12 @@ __attribute((no_stack_protector)) void smcall_handler(uint32_t *args, const uint8_t *ext_salt = (const uint8_t *)args[2]; const uint8_t *wipe_code = (const uint8_t *)args[3]; size_t wipe_code_len = args[4]; - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_change_wipe_code__verified(pin, pin_len, ext_salt, wipe_code, wipe_code_len); } break; case SMCALL_STORAGE_HAS: { uint16_t key = (uint16_t)args[0]; - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_has(key); } break; @@ -285,7 +271,6 @@ __attribute((no_stack_protector)) void smcall_handler(uint32_t *args, void *val = (void *)args[1]; uint16_t max_len = (uint16_t)args[2]; uint16_t *len = (uint16_t *)args[3]; - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_get__verified(key, val, max_len, len); } break; @@ -293,27 +278,23 @@ __attribute((no_stack_protector)) void smcall_handler(uint32_t *args, uint16_t key = (uint16_t)args[0]; const void *val = (const void *)args[1]; uint16_t len = (uint16_t)args[2]; - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_set__verified(key, val, len); } break; case SMCALL_STORAGE_DELETE: { uint16_t key = (uint16_t)args[0]; - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_delete(key); } break; case SMCALL_STORAGE_SET_COUNTER: { uint16_t key = (uint16_t)args[0]; uint32_t count = args[1]; - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_set_counter(key, count); } break; case SMCALL_STORAGE_NEXT_COUNTER: { uint16_t key = (uint16_t)args[0]; uint32_t *count = (uint32_t *)args[1]; - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_next_counter__verified(key, count); } break; diff --git a/core/embed/sys/syscall/stm32/syscall_dispatch.c b/core/embed/sys/syscall/stm32/syscall_dispatch.c index 63f475c3e5..272bae117d 100644 --- a/core/embed/sys/syscall/stm32/syscall_dispatch.c +++ b/core/embed/sys/syscall/stm32/syscall_dispatch.c @@ -32,7 +32,6 @@ #include #include #include -#include #include #include #include @@ -553,22 +552,18 @@ __attribute((no_stack_protector)) void syscall_handler(uint32_t *args, storage_init_callback = (PIN_UI_WAIT_CALLBACK)args[0]; const uint8_t *salt = (const uint8_t *)args[1]; uint16_t salt_len = args[2]; - mpu_reconfig(MPU_MODE_STORAGE); storage_init__verified(storage_init_callback_wrapper, salt, salt_len); } break; case SYSCALL_STORAGE_WIPE: { - mpu_reconfig(MPU_MODE_STORAGE); storage_wipe(); } break; case SYSCALL_STORAGE_IS_UNLOCKED: { - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_is_unlocked(); } break; case SYSCALL_STORAGE_LOCK: { - mpu_reconfig(MPU_MODE_STORAGE); storage_lock(); } break; @@ -576,22 +571,18 @@ __attribute((no_stack_protector)) void syscall_handler(uint32_t *args, const uint8_t *pin = (const uint8_t *)args[0]; size_t pin_len = args[1]; const uint8_t *ext_salt = (const uint8_t *)args[2]; - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_unlock__verified(pin, pin_len, ext_salt); } break; case SYSCALL_STORAGE_HAS_PIN: { - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_has_pin(); } break; case SYSCALL_STORAGE_PIN_FAILS_INCREASE: { - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_pin_fails_increase(); } break; case SYSCALL_STORAGE_GET_PIN_REM: { - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_get_pin_rem(); } break; @@ -602,7 +593,6 @@ __attribute((no_stack_protector)) void syscall_handler(uint32_t *args, 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]; - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_change_pin__verified( oldpin, oldpin_len, newpin, newpin_len, old_ext_salt, new_ext_salt); } break; @@ -610,12 +600,10 @@ __attribute((no_stack_protector)) void syscall_handler(uint32_t *args, case SYSCALL_STORAGE_ENSURE_NOT_WIPE_CODE: { const uint8_t *pin = (const uint8_t *)args[0]; size_t pin_len = args[1]; - mpu_reconfig(MPU_MODE_STORAGE); storage_ensure_not_wipe_code__verified(pin, pin_len); } break; case SYSCALL_STORAGE_HAS_WIPE_CODE: { - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_has_wipe_code(); } break; @@ -625,14 +613,12 @@ __attribute((no_stack_protector)) void syscall_handler(uint32_t *args, const uint8_t *ext_salt = (const uint8_t *)args[2]; const uint8_t *wipe_code = (const uint8_t *)args[3]; size_t wipe_code_len = args[4]; - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_change_wipe_code__verified(pin, pin_len, ext_salt, wipe_code, wipe_code_len); } break; case SYSCALL_STORAGE_HAS: { uint16_t key = (uint16_t)args[0]; - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_has(key); } break; @@ -641,7 +627,6 @@ __attribute((no_stack_protector)) void syscall_handler(uint32_t *args, void *val = (void *)args[1]; uint16_t max_len = (uint16_t)args[2]; uint16_t *len = (uint16_t *)args[3]; - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_get__verified(key, val, max_len, len); } break; @@ -649,27 +634,23 @@ __attribute((no_stack_protector)) void syscall_handler(uint32_t *args, uint16_t key = (uint16_t)args[0]; const void *val = (const void *)args[1]; uint16_t len = (uint16_t)args[2]; - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_set__verified(key, val, len); } break; case SYSCALL_STORAGE_DELETE: { uint16_t key = (uint16_t)args[0]; - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_delete(key); } break; case SYSCALL_STORAGE_SET_COUNTER: { uint16_t key = (uint16_t)args[0]; uint32_t count = args[1]; - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_set_counter(key, count); } break; case SYSCALL_STORAGE_NEXT_COUNTER: { uint16_t key = (uint16_t)args[0]; uint32_t *count = (uint32_t *)args[1]; - mpu_reconfig(MPU_MODE_STORAGE); args[0] = storage_next_counter__verified(key, count); } break; diff --git a/legacy/sys/mpu.h b/legacy/sys/mpu.h new file mode 100644 index 0000000000..7fc2d5fd6a --- /dev/null +++ b/legacy/sys/mpu.h @@ -0,0 +1,32 @@ +/* + * This file is part of the Trezor project, https://trezor.io/ + * + * Copyright (c) SatoshiLabs + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#pragma once + +// Empty implementation of mpu.h + +typedef int mpu_mode_t; + +#define MPU_MODE_STORAGE 0 + +#define mpu_reconfig(mode) (mode) +#define mpu_restore(mode) \ + do { \ + (void)mode; \ + } while (0) diff --git a/storage/storage.c b/storage/storage.c index 84f2a47570..68e11e1950 100644 --- a/storage/storage.c +++ b/storage/storage.c @@ -20,6 +20,8 @@ #include #include +#include + #include "chacha20poly1305/rfc7539.h" #include "common.h" #include "hmac.h" @@ -811,6 +813,8 @@ static void init_wiped_storage(void) { void storage_init(PIN_UI_WAIT_CALLBACK callback, const uint8_t *salt, const uint16_t salt_len) { + mpu_mode_t mpu_mode = mpu_reconfig(MPU_MODE_STORAGE); + initialized = secfalse; unlocked = secfalse; memzero(cached_keys, sizeof(cached_keys)); @@ -833,13 +837,19 @@ void storage_init(PIN_UI_WAIT_CALLBACK callback, const uint8_t *salt, if (secfalse == norcow_get(EDEK_PVC_KEY, &val, &len)) { init_wiped_storage(); } + + mpu_restore(mpu_mode); } secbool storage_pin_fails_increase(void) { if (sectrue != initialized) { return secfalse; } - return pin_fails_increase(); + + mpu_mode_t mpu_mode = mpu_reconfig(MPU_MODE_STORAGE); + secbool ret = pin_fails_increase(); + mpu_restore(mpu_mode); + return ret; } secbool storage_is_unlocked(void) { @@ -1126,7 +1136,10 @@ secbool storage_unlock(const uint8_t *pin, size_t pin_len, ui_message = VERIFYING_PIN_MSG; } + mpu_mode_t mpu_mode = mpu_reconfig(MPU_MODE_STORAGE); secbool ret = unlock(pin, pin_len, ext_salt); + mpu_restore(mpu_mode); + ui_progress_finish(); return ret; } @@ -1199,27 +1212,36 @@ secbool storage_get(const uint16_t key, void *val_dest, const uint16_t max_len, return secfalse; } + mpu_mode_t mpu_mode = mpu_reconfig(MPU_MODE_STORAGE); + secbool ret = secfalse; + // If the top bit of APP is set, then the value is not encrypted and can be // read from a locked device. if ((app & FLAG_PUBLIC) != 0) { const void *val_stored = NULL; if (sectrue != norcow_get(key, &val_stored, len)) { - return secfalse; + goto end; } if (val_dest == NULL) { - return sectrue; + ret = sectrue; + goto end; } if (*len > max_len) { - return secfalse; + goto end; } memcpy(val_dest, val_stored, *len); - return sectrue; + ret = sectrue; + goto end; } else { if (sectrue != unlocked) { - return secfalse; + goto end; } - return storage_get_encrypted(key, val_dest, max_len, len); + ret = storage_get_encrypted(key, val_dest, max_len, len); } + +end: + mpu_restore(mpu_mode); + return ret; } /* @@ -1284,12 +1306,16 @@ secbool storage_set(const uint16_t key, const void *val, const uint16_t len) { return secfalse; } + mpu_mode_t mpu_mode = mpu_reconfig(MPU_MODE_STORAGE); + secbool ret = secfalse; if ((app & FLAG_PUBLIC) != 0) { ret = norcow_set(key, val, len); } else { ret = storage_set_encrypted(key, val, len); } + + mpu_restore(mpu_mode); return ret; } @@ -1305,10 +1331,12 @@ secbool storage_delete(const uint16_t key) { return secfalse; } + mpu_mode_t mpu_mode = mpu_reconfig(MPU_MODE_STORAGE); secbool ret = norcow_delete(key); if (sectrue == ret) { ret = auth_update(key); } + mpu_restore(mpu_mode); return ret; } @@ -1327,7 +1355,11 @@ secbool storage_set_counter(const uint16_t key, const uint32_t count) { return secfalse; } - return norcow_set_counter(key, count); + mpu_mode_t mpu_mode = mpu_reconfig(MPU_MODE_STORAGE); + secbool ret = norcow_set_counter(key, count); + mpu_restore(mpu_mode); + + return ret; } secbool storage_next_counter(const uint16_t key, uint32_t *count) { @@ -1347,7 +1379,11 @@ secbool storage_next_counter(const uint16_t key, uint32_t *count) { return secfalse; } - return norcow_next_counter(key, count); + mpu_mode_t mpu_mode = mpu_reconfig(MPU_MODE_STORAGE); + secbool ret = norcow_next_counter(key, count); + mpu_restore(mpu_mode); + + return ret; } secbool storage_has_pin(void) { @@ -1355,13 +1391,24 @@ secbool storage_has_pin(void) { return secfalse; } + secbool ret = secfalse; + mpu_mode_t mpu_mode = mpu_reconfig(MPU_MODE_STORAGE); + const void *val = NULL; uint16_t len = 0; - if (sectrue != norcow_get(PIN_NOT_SET_KEY, &val, &len) || - (len > 0 && *(uint8_t *)val != FALSE_BYTE)) { - return secfalse; + if (sectrue != norcow_get(PIN_NOT_SET_KEY, &val, &len)) { + goto end; } - return sectrue; + + if (len > 0 && *(uint8_t *)val != FALSE_BYTE) { + goto end; + } + + ret = sectrue; + +end: + mpu_restore(mpu_mode); + return ret; } uint32_t storage_get_pin_rem(void) { @@ -1369,12 +1416,15 @@ uint32_t storage_get_pin_rem(void) { return 0; } + uint32_t rem_mcu = 0; + mpu_mode_t mpu_mode = mpu_reconfig(MPU_MODE_STORAGE); + uint32_t ctr_mcu = 0; if (sectrue != pin_get_fails(&ctr_mcu)) { - return 0; + goto end; } - uint32_t rem_mcu = PIN_MAX_TRIES - ctr_mcu; + rem_mcu = PIN_MAX_TRIES - ctr_mcu; #if USE_OPTIGA // Synchronize counters in case they diverged. @@ -1403,6 +1453,8 @@ uint32_t storage_get_pin_rem(void) { } #endif +end: + mpu_restore(mpu_mode); return rem_mcu; } @@ -1418,6 +1470,8 @@ secbool storage_change_pin(const uint8_t *oldpin, size_t oldpin_len, ui_message = (oldpin_len != 0 && newpin_len == 0) ? VERIFYING_PIN_MSG : PROCESSING_MSG; + mpu_mode_t mpu_mode = mpu_reconfig(MPU_MODE_STORAGE); + secbool ret = unlock(oldpin, oldpin_len, old_ext_salt); if (sectrue != ret) { goto end; @@ -1432,11 +1486,13 @@ secbool storage_change_pin(const uint8_t *oldpin, size_t oldpin_len, ret = set_pin(newpin, newpin_len, new_ext_salt); end: + mpu_restore(mpu_mode); ui_progress_finish(); return ret; } void storage_ensure_not_wipe_code(const uint8_t *pin, size_t pin_len) { + mpu_mode_t mpu_mode = mpu_reconfig(MPU_MODE_STORAGE); #if NORCOW_MIN_VERSION <= 2 // If we are unlocking the storage during upgrade from version 2 or lower, // then encode the PIN to the old format. @@ -1452,6 +1508,7 @@ void storage_ensure_not_wipe_code(const uint8_t *pin, size_t pin_len) { #if NORCOW_MIN_VERSION <= 2 memzero(&legacy_pin, sizeof(legacy_pin)); #endif + mpu_restore(mpu_mode); } secbool storage_has_wipe_code(void) { @@ -1459,7 +1516,11 @@ secbool storage_has_wipe_code(void) { return secfalse; } - return is_not_wipe_code(WIPE_CODE_EMPTY, WIPE_CODE_EMPTY_LEN); + mpu_mode_t mpu_mode = mpu_reconfig(MPU_MODE_STORAGE); + secbool ret = is_not_wipe_code(WIPE_CODE_EMPTY, WIPE_CODE_EMPTY_LEN); + mpu_restore(mpu_mode); + + return ret; } secbool storage_change_wipe_code(const uint8_t *pin, size_t pin_len, @@ -1476,6 +1537,8 @@ secbool storage_change_wipe_code(const uint8_t *pin, size_t pin_len, ui_message = (pin_len != 0 && wipe_code_len == 0) ? VERIFYING_PIN_MSG : PROCESSING_MSG; + mpu_mode_t mpu_mode = mpu_reconfig(MPU_MODE_STORAGE); + secbool ret = unlock(pin, pin_len, ext_salt); if (sectrue != ret) { goto end; @@ -1484,16 +1547,19 @@ secbool storage_change_wipe_code(const uint8_t *pin, size_t pin_len, ret = set_wipe_code(wipe_code, wipe_code_len); end: + mpu_restore(mpu_mode); ui_progress_finish(); return ret; } void storage_wipe(void) { + mpu_mode_t mpu_mode = mpu_reconfig(MPU_MODE_STORAGE); norcow_wipe(); norcow_active_version = NORCOW_VERSION; memzero(authentication_sum, sizeof(authentication_sum)); memzero(cached_keys, sizeof(cached_keys)); init_wiped_storage(); + mpu_restore(mpu_mode); } static void __handle_fault(const char *msg, const char *file, int line) { diff --git a/storage/tests/c/sys/mpu.h b/storage/tests/c/sys/mpu.h new file mode 100644 index 0000000000..69068dd8d6 --- /dev/null +++ b/storage/tests/c/sys/mpu.h @@ -0,0 +1,32 @@ +/* + * This file is part of the Trezor project, https://trezor.io/ + * + * Copyright (c) SatoshiLabs + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#pragma once + +// Empty implementation of mpu.h for the storage tests + +typedef int mpu_mode_t; + +#define MPU_MODE_STORAGE 0 + +#define mpu_reconfig(mode) (mode) +#define mpu_restore(mode) \ + do { \ + (void)mode; \ + } while (0)