From 6e2f5ff27d8a152e4f7d0ff7a72d722c92468ea5 Mon Sep 17 00:00:00 2001 From: tychovrahe Date: Mon, 26 Feb 2024 09:49:58 +0100 Subject: [PATCH] fix(core): improve bhk handling on STM32U5 [no changelog] --- core/embed/boardloader/main.c | 44 +++++++++++++++-- core/embed/bootloader/main.c | 4 ++ core/embed/trezorhal/secret.h | 4 ++ core/embed/trezorhal/stm32f4/secret.c | 17 ++++--- core/embed/trezorhal/stm32u5/secret.c | 69 +++++++++++++++++++-------- 5 files changed, 105 insertions(+), 33 deletions(-) diff --git a/core/embed/boardloader/main.c b/core/embed/boardloader/main.c index 675f297307..8cbd3871d9 100644 --- a/core/embed/boardloader/main.c +++ b/core/embed/boardloader/main.c @@ -67,7 +67,7 @@ static const uint8_t * const BOARDLOADER_KEYS[] = { }; #ifdef STM32U5 -void check_bootloader_version(uint8_t bld_version) { +uint8_t get_bootloader_min_version(void) { const uint8_t *counter_addr = flash_area_get_address(&SECRET_AREA, SECRET_MONOTONIC_COUNTER_OFFSET, SECRET_MONOTONIC_COUNTER_LEN); @@ -92,10 +92,12 @@ void check_bootloader_version(uint8_t bld_version) { } } - ensure((bld_version >= counter) * sectrue, "BOOTLOADER DOWNGRADED"); + return counter; +} - if (bld_version > counter) { - for (int i = 0; i < bld_version; i++) { +void write_bootloader_min_version(uint8_t version) { + if (version > get_bootloader_min_version()) { + for (int i = 0; i < version; i++) { uint32_t data[4] = {0}; secret_write((uint8_t *)data, SECRET_MONOTONIC_COUNTER_OFFSET + i * 16, 16); @@ -160,6 +162,12 @@ static uint32_t check_sdcard(void) { return 0; } +#ifdef STM32U5 + if (hdr->monotonic < get_bootloader_min_version()) { + return 0; + } +#endif + return hdr->codelen; } @@ -255,6 +263,8 @@ int main(void) { } trustzone_init_boardloader(); + + secret_ensure_initialized(); #endif #ifdef STM32F4 @@ -280,6 +290,25 @@ int main(void) { #if defined USE_SD_CARD sdcard_init(); +#ifdef STM32U5 + // If the bootloader is being updated from SD card, we need to preserve the + // monotonic counter from the old bootloader. This is in case that the old + // bootloader did not have the chance yet to write its monotonic counter to + // the secret area - which normally happens later in the flow. + const image_header *old_hdr = read_image_header( + (const uint8_t *)BOOTLOADER_START, BOOTLOADER_IMAGE_MAGIC, + flash_area_get_size(&BOOTLOADER_AREA)); + + if ((old_hdr != NULL) && + (sectrue == check_image_header_sig(old_hdr, BOARDLOADER_KEY_M, + BOARDLOADER_KEY_N, + BOARDLOADER_KEYS)) && + (sectrue == + check_image_contents(old_hdr, IMAGE_HEADER_SIZE, &BOOTLOADER_AREA))) { + write_bootloader_min_version(old_hdr->monotonic); + } +#endif + if (check_sdcard()) { return copy_sdcard() == sectrue ? 0 : 3; } @@ -300,7 +329,12 @@ int main(void) { "invalid bootloader hash"); #ifdef STM32U5 - check_bootloader_version(hdr->monotonic); + uint8_t bld_min_version = get_bootloader_min_version(); + ensure((hdr->monotonic >= bld_min_version) * sectrue, + "BOOTLOADER DOWNGRADED"); + // Write the bootloader version to the secret area. + // This includes the version of bootloader potentially updated from SD card. + write_bootloader_min_version(hdr->monotonic); #endif ensure_compatible_settings(); diff --git a/core/embed/bootloader/main.c b/core/embed/bootloader/main.c index 17be369e65..6ad99498f8 100644 --- a/core/embed/bootloader/main.c +++ b/core/embed/bootloader/main.c @@ -430,6 +430,10 @@ int bootloader_main(void) { display_reinit(); +#ifdef STM32U5 + ensure(secret_ensure_initialized(), "secret reinitialized"); +#endif + ui_screen_boot_empty(false); mpu_config_bootloader(); diff --git a/core/embed/trezorhal/secret.h b/core/embed/trezorhal/secret.h index 2b226c3d67..202d5952cb 100644 --- a/core/embed/trezorhal/secret.h +++ b/core/embed/trezorhal/secret.h @@ -21,6 +21,10 @@ secbool secret_read(uint8_t* data, uint32_t offset, uint32_t len); secbool secret_wiped(void); +secbool secret_verify_header(void); + +secbool secret_ensure_initialized(void); + void secret_erase(void); void secret_hide(void); diff --git a/core/embed/trezorhal/stm32f4/secret.c b/core/embed/trezorhal/stm32f4/secret.c index 33e8b66e5b..3e274fe9df 100644 --- a/core/embed/trezorhal/stm32f4/secret.c +++ b/core/embed/trezorhal/stm32f4/secret.c @@ -7,14 +7,17 @@ static secbool bootloader_locked_set = secfalse; static secbool bootloader_locked = secfalse; -static secbool verify_header(void) { - uint8_t header[SECRET_HEADER_LEN] = {0}; +secbool secret_verify_header(void) { + uint8_t header[sizeof(SECRET_HEADER_MAGIC)] = {0}; - memcpy(header, flash_area_get_address(&SECRET_AREA, 0, SECRET_HEADER_LEN), - SECRET_HEADER_LEN); + memcpy(header, + flash_area_get_address(&SECRET_AREA, 0, sizeof(SECRET_HEADER_MAGIC)), + sizeof(SECRET_HEADER_MAGIC)); bootloader_locked = - memcmp(header, SECRET_HEADER_MAGIC, 4) == 0 ? sectrue : secfalse; + memcmp(header, SECRET_HEADER_MAGIC, sizeof(SECRET_HEADER_MAGIC)) == 0 + ? sectrue + : secfalse; bootloader_locked_set = sectrue; return bootloader_locked; } @@ -22,7 +25,7 @@ static secbool verify_header(void) { secbool secret_bootloader_locked(void) { if (bootloader_locked_set != sectrue) { // Set bootloader_locked. - verify_header(); + secret_verify_header(); } return bootloader_locked; @@ -44,7 +47,7 @@ void secret_write(uint8_t* data, uint32_t offset, uint32_t len) { } secbool secret_read(uint8_t* data, uint32_t offset, uint32_t len) { - if (sectrue != verify_header()) { + if (sectrue != secret_verify_header()) { return secfalse; } diff --git a/core/embed/trezorhal/stm32u5/secret.c b/core/embed/trezorhal/stm32u5/secret.c index 7fd3c08e96..9f2efdebfc 100644 --- a/core/embed/trezorhal/stm32u5/secret.c +++ b/core/embed/trezorhal/stm32u5/secret.c @@ -3,24 +3,37 @@ #include #include "common.h" #include "flash.h" +#include "memzero.h" #include "model.h" #include "rng.h" -static secbool bootloader_locked_set = secfalse; static secbool bootloader_locked = secfalse; -static secbool verify_header(void) { - uint8_t header[SECRET_HEADER_LEN] = {0}; +secbool secret_verify_header(void) { + uint8_t header[sizeof(SECRET_HEADER_MAGIC)] = {0}; - memcpy(header, flash_area_get_address(&SECRET_AREA, 0, SECRET_HEADER_LEN), - SECRET_HEADER_LEN); + memcpy(header, + flash_area_get_address(&SECRET_AREA, 0, sizeof(SECRET_HEADER_MAGIC)), + sizeof(SECRET_HEADER_MAGIC)); bootloader_locked = - memcmp(header, SECRET_HEADER_MAGIC, 4) == 0 ? sectrue : secfalse; - bootloader_locked_set = sectrue; + memcmp(header, SECRET_HEADER_MAGIC, sizeof(SECRET_HEADER_MAGIC)) == 0 + ? sectrue + : secfalse; return bootloader_locked; } +secbool secret_ensure_initialized(void) { + if (sectrue != secret_verify_header()) { + ensure(flash_area_erase_bulk(STORAGE_AREAS, STORAGE_AREAS_COUNT, NULL), + "erase storage failed"); + secret_erase(); + secret_write_header(); + return secfalse; + } + return sectrue; +} + secbool secret_bootloader_locked(void) { #ifdef FIRMWARE return TAMP->BKP8R != 0 * sectrue; @@ -46,7 +59,7 @@ void secret_write(uint8_t *data, uint32_t offset, uint32_t len) { } secbool secret_read(uint8_t *data, uint32_t offset, uint32_t len) { - if (sectrue != verify_header()) { + if (sectrue != secret_verify_header()) { return secfalse; } @@ -69,8 +82,27 @@ secbool secret_bhk_locked(void) { sectrue; } +static secbool secret_present(uint32_t offset, uint32_t len) { + uint8_t *optiga_secret = + (uint8_t *)flash_area_get_address(&SECRET_AREA, offset, len); + + int optiga_secret_empty_bytes = 0; + + for (int i = 0; i < len; i++) { + if (optiga_secret[i] == 0xFF) { + optiga_secret_empty_bytes++; + } + } + return sectrue * (optiga_secret_empty_bytes != len); +} + void secret_bhk_provision(void) { uint32_t secret[SECRET_BHK_LEN / sizeof(uint32_t)] = {0}; + + if (sectrue != secret_present(SECRET_BHK_OFFSET, SECRET_BHK_LEN)) { + secret_bhk_regenerate(); + } + secbool ok = secret_read((uint8_t *)secret, SECRET_BHK_OFFSET, SECRET_BHK_LEN); @@ -86,6 +118,8 @@ void secret_bhk_provision(void) { reg1++; } } + + memzero(secret, sizeof(secret)); } void secret_bhk_regenerate(void) { @@ -96,24 +130,16 @@ void secret_bhk_regenerate(void) { for (int j = 0; j < 4; j++) { val[j] = rng_get(); } - ensure(flash_area_write_quadword(&BHK_AREA, i * 4 * sizeof(uint32_t), val), - "Failed regenerating BHK"); + secbool res = + flash_area_write_quadword(&BHK_AREA, i * 4 * sizeof(uint32_t), val); + memzero(val, sizeof(val)); + ensure(res, "Failed regenerating BHK"); } ensure(flash_lock_write(), "Failed regenerating BHK"); } secbool secret_optiga_present(void) { - uint8_t *optiga_secret = (uint8_t *)flash_area_get_address( - &SECRET_AREA, SECRET_OPTIGA_KEY_OFFSET, SECRET_OPTIGA_KEY_LEN); - - int optiga_secret_empty_bytes = 0; - - for (int i = 0; i < SECRET_OPTIGA_KEY_LEN; i++) { - if (optiga_secret[i] == 0xFF) { - optiga_secret_empty_bytes++; - } - } - return sectrue * (optiga_secret_empty_bytes != SECRET_OPTIGA_KEY_LEN); + return secret_present(SECRET_OPTIGA_KEY_OFFSET, SECRET_OPTIGA_KEY_LEN); } void secret_optiga_backup(void) { @@ -133,6 +159,7 @@ void secret_optiga_backup(void) { reg1++; } } + memzero(secret, sizeof(secret)); } secbool secret_optiga_extract(uint8_t *dest) {