From f2743707b854141c6fd960891548f0d3db15ccb2 Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Tue, 23 Jan 2024 14:11:14 +0530 Subject: [PATCH 1/2] fix(ota): additional checks for secure version in anti-rollback case Some additional checks related to secure version of the application in anti-rollback case have been added to avoid any attempts to boot lower security version but valid application (e.g., passive partition image). - Read secure_version under sha256 protection - First check has been added in the bootloader to ensure correct secure version after application verification and loading stage. This check happens before setting up the flash cache mapping and handling over the final control to application. This check ensures that application was not swapped (e.g., to lower security version but valid image) just before the load stage in bootloader. - Second check has been added in the application startup code to ensure that currently booting app has higher security version than the one programmed in the eFuse for anti-rollback scenario. This will ensure that only the legit application boots-up on the device for anti-rollback case. --- .../include/esp_image_format.h | 1 + .../bootloader_support/src/esp_image_format.c | 78 +++++++++++++++++-- .../esp_app_format/include/esp_app_desc.h | 1 + components/esp_system/startup.c | 5 ++ .../sdkconfig.ci.anti_rollback | 2 +- 5 files changed, 78 insertions(+), 9 deletions(-) diff --git a/components/bootloader_support/include/esp_image_format.h b/components/bootloader_support/include/esp_image_format.h index 20545f5d7f64..ce93d292f66e 100644 --- a/components/bootloader_support/include/esp_image_format.h +++ b/components/bootloader_support/include/esp_image_format.h @@ -33,6 +33,7 @@ typedef struct { uint32_t segment_data[ESP_IMAGE_MAX_SEGMENTS]; /* Data offsets for each segment */ uint32_t image_len; /* Length of image on flash, in bytes */ uint8_t image_digest[32]; /* appended SHA-256 digest */ + uint32_t secure_version; /* secure version for anti-rollback, it is covered by sha256 (set if CONFIG_BOOTLOADER_APP_ANTI_ROLLBACK=y) */ } esp_image_metadata_t; typedef enum { diff --git a/components/bootloader_support/src/esp_image_format.c b/components/bootloader_support/src/esp_image_format.c index 17ee0e8a1686..324440f3da52 100644 --- a/components/bootloader_support/src/esp_image_format.c +++ b/components/bootloader_support/src/esp_image_format.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -19,6 +19,8 @@ #include "bootloader_util.h" #include "bootloader_common.h" #include "esp_rom_sys.h" +#include "esp_efuse.h" +#include "esp_app_desc.h" #include "bootloader_memory_utils.h" #include "soc/soc_caps.h" #if CONFIG_IDF_TARGET_ESP32 @@ -81,10 +83,10 @@ static bool should_map(uint32_t load_addr); static esp_err_t process_segments(esp_image_metadata_t *data, bool silent, bool do_load, bootloader_sha256_handle_t sha_handle, uint32_t *checksum); /* Load or verify a segment */ -static esp_err_t process_segment(int index, uint32_t flash_addr, esp_image_segment_header_t *header, bool silent, bool do_load, bootloader_sha256_handle_t sha_handle, uint32_t *checksum); +static esp_err_t process_segment(int index, uint32_t flash_addr, esp_image_segment_header_t *header, bool silent, bool do_load, bootloader_sha256_handle_t sha_handle, uint32_t *checksum, esp_image_metadata_t *metadata); /* split segment and verify if data_len is too long */ -static esp_err_t process_segment_data(intptr_t load_addr, uint32_t data_addr, uint32_t data_len, bool do_load, bootloader_sha256_handle_t sha_handle, uint32_t *checksum); +static esp_err_t process_segment_data(int segment, intptr_t load_addr, uint32_t data_addr, uint32_t data_len, bool do_load, bootloader_sha256_handle_t sha_handle, uint32_t *checksum, esp_image_metadata_t *metadata); /* Verify the main image header */ static esp_err_t verify_image_header(uint32_t src_addr, const esp_image_header_t *image, bool silent); @@ -229,6 +231,21 @@ static esp_err_t image_load(esp_image_load_mode_t mode, const esp_partition_pos_ } } } + +#if CONFIG_BOOTLOADER_APP_ANTI_ROLLBACK + /* For anti-rollback case, reconfirm security version of the application to prevent FI attacks */ + bool sec_ver = false; + if (do_load) { + sec_ver = esp_efuse_check_secure_version(data->secure_version); + if (!sec_ver) { + err = ESP_FAIL; + goto err; + } + } + /* Ensure that the security version check passes for image loading scenario */ + ESP_FAULT_ASSERT(!do_load || sec_ver == true); +#endif // CONFIG_BOOTLOADER_APP_ANTI_ROLLBACK + #endif // BOOTLOADER_BUILD // Success! @@ -504,8 +521,8 @@ static esp_err_t process_segments(esp_image_metadata_t *data, bool silent, bool uint32_t next_addr = start_segments; for (int i = 0; i < data->image.segment_count; i++) { esp_image_segment_header_t *header = &data->segments[i]; - ESP_LOGV(TAG, "loading segment header %d at offset 0x%x", i, next_addr); - CHECK_ERR(process_segment(i, next_addr, header, silent, do_load, sha_handle, checksum)); + ESP_LOGV(TAG, "loading segment header %d at offset 0x%"PRIx32, i, next_addr); + CHECK_ERR(process_segment(i, next_addr, header, silent, do_load, sha_handle, checksum, data)); next_addr += sizeof(esp_image_segment_header_t); data->segment_data[i] = next_addr; next_addr += header->data_len; @@ -526,7 +543,7 @@ static esp_err_t process_segments(esp_image_metadata_t *data, bool silent, bool return err; } -static esp_err_t process_segment(int index, uint32_t flash_addr, esp_image_segment_header_t *header, bool silent, bool do_load, bootloader_sha256_handle_t sha_handle, uint32_t *checksum) +static esp_err_t process_segment(int index, uint32_t flash_addr, esp_image_segment_header_t *header, bool silent, bool do_load, bootloader_sha256_handle_t sha_handle, uint32_t *checksum, esp_image_metadata_t *metadata) { esp_err_t err; @@ -584,7 +601,7 @@ static esp_err_t process_segment(int index, uint32_t flash_addr, esp_image_segme uint32_t offset_page = ((data_addr & MMAP_ALIGNED_MASK) != 0) ? 1 : 0; /* Data we could map in case we are not aligned to PAGE boundary is one page size lesser. */ data_len = MIN(data_len_remain, ((free_page_count - offset_page) * SPI_FLASH_MMU_PAGE_SIZE)); - CHECK_ERR(process_segment_data(load_addr, data_addr, data_len, do_load, sha_handle, checksum)); + CHECK_ERR(process_segment_data(index, load_addr, data_addr, data_len, do_load, sha_handle, checksum, metadata)); data_addr += data_len; data_len_remain -= data_len; } @@ -599,7 +616,42 @@ static esp_err_t process_segment(int index, uint32_t flash_addr, esp_image_segme return err; } -static esp_err_t process_segment_data(intptr_t load_addr, uint32_t data_addr, uint32_t data_len, bool do_load, bootloader_sha256_handle_t sha_handle, uint32_t *checksum) +#if CONFIG_BOOTLOADER_APP_ANTI_ROLLBACK +/* The __attribute__((optimize("O0"))) is used to disable optimizations for this function, + * preventing the compiler from potentially optimizing data_buffer and reading data directly from src. + * This is crucial as we want to read from Flash only once, ensuring the integrity of the data. + */ +__attribute__((optimize("O0"))) +static size_t process_esp_app_desc_data(const uint32_t *src, bootloader_sha256_handle_t sha_handle, uint32_t *checksum, esp_image_metadata_t *metadata) +{ + /* Using data_buffer here helps to securely read secure_version + * (for anti-rollback) from esp_app_desc_t, preventing FI attack. + * We read data from Flash into this buffer, which is covered by sha256. + * Therefore, if the flash is under attackers control and contents are modified + * the sha256 comparison will fail. + * + * The esp_app_desc_t structure is located in DROM and is always in segment #0. + * + * esp_app_desc_t is always at #0 segment (index==0). + * secure_version field of esp_app_desc_t is located at #2 word (w_i==1). + */ + uint32_t data_buffer[2]; + memcpy(data_buffer, src, sizeof(data_buffer)); + assert(data_buffer[0] == ESP_APP_DESC_MAGIC_WORD); + metadata->secure_version = data_buffer[1]; + if (checksum != NULL) { + *checksum ^= data_buffer[0] ^ data_buffer[1]; + } + if (sha_handle != NULL) { + bootloader_sha256_data(sha_handle, data_buffer, sizeof(data_buffer)); + } + ESP_FAULT_ASSERT(memcmp(data_buffer, src, sizeof(data_buffer)) == 0); + ESP_FAULT_ASSERT(memcmp(&metadata->secure_version, &src[1], sizeof(uint32_t)) == 0); + return sizeof(data_buffer); +} +#endif // CONFIG_BOOTLOADER_APP_ANTI_ROLLBACK + +static esp_err_t process_segment_data(int segment, intptr_t load_addr, uint32_t data_addr, uint32_t data_len, bool do_load, bootloader_sha256_handle_t sha_handle, uint32_t *checksum, esp_image_metadata_t *metadata) { // If we are not loading, and the checksum is empty, skip processing this // segment for data @@ -636,6 +688,16 @@ static esp_err_t process_segment_data(intptr_t load_addr, uint32_t data_addr, ui const uint32_t *src = data; +#if CONFIG_BOOTLOADER_APP_ANTI_ROLLBACK + if (segment == 0) { + // The esp_app_desc_t structure is located in DROM and is always in segment #0. + size_t len = process_esp_app_desc_data(src, sha_handle, checksum, metadata); + data_len -= len; + src += len / 4; + // In BOOTLOADER_BUILD, for DROM (segment #0) we do not load it into dest (only map it), do_load = false. + } +#endif // CONFIG_BOOTLOADER_APP_ANTI_ROLLBACK + for (size_t i = 0; i < data_len; i += 4) { int w_i = i / 4; // Word index uint32_t w = src[w_i]; diff --git a/components/esp_app_format/include/esp_app_desc.h b/components/esp_app_format/include/esp_app_desc.h index fd7582be7925..0a64c5c530ed 100644 --- a/components/esp_app_format/include/esp_app_desc.h +++ b/components/esp_app_format/include/esp_app_desc.h @@ -37,6 +37,7 @@ typedef struct { /** @cond */ ESP_STATIC_ASSERT(sizeof(esp_app_desc_t) == 256, "esp_app_desc_t should be 256 bytes"); +ESP_STATIC_ASSERT(offsetof(esp_app_desc_t, secure_version) == 4, "secure_version field must be at 4 offset"); /** @endcond */ /** diff --git a/components/esp_system/startup.c b/components/esp_system/startup.c index 9960ee8ebbff..0de6410c0934 100644 --- a/components/esp_system/startup.c +++ b/components/esp_system/startup.c @@ -353,6 +353,11 @@ static void do_core_init(void) #endif #endif +#if CONFIG_BOOTLOADER_APP_ANTI_ROLLBACK + // For anti-rollback case, recheck security version before we boot-up the current application + assert(esp_efuse_check_secure_version(esp_app_get_description()->secure_version) == true && "Incorrect secure version of app"); +#endif + #ifdef CONFIG_SECURE_FLASH_ENC_ENABLED esp_flash_encryption_init_checks(); #endif diff --git a/tools/test_apps/system/bootloader_sections/sdkconfig.ci.anti_rollback b/tools/test_apps/system/bootloader_sections/sdkconfig.ci.anti_rollback index 346883f5a474..468731442ea1 100644 --- a/tools/test_apps/system/bootloader_sections/sdkconfig.ci.anti_rollback +++ b/tools/test_apps/system/bootloader_sections/sdkconfig.ci.anti_rollback @@ -3,4 +3,4 @@ CONFIG_BOOTLOADER_APP_ANTI_ROLLBACK=y CONFIG_ESPTOOLPY_FLASHSIZE_4MB=y CONFIG_PARTITION_TABLE_CUSTOM=y CONFIG_PARTITION_TABLE_CUSTOM_FILENAME="partitions_example.csv" -CONFIG_PARTITION_TABLE_OFFSET=0x9000 +CONFIG_PARTITION_TABLE_OFFSET=0xA000 From 1620858985829c864e8b624870384ed5be2e0ec9 Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Mon, 19 Feb 2024 12:26:08 +0530 Subject: [PATCH 2/2] fix(bootloader_support): check the secure version only for app image Secure version in the image header is only available for the application image. However, for certain security workflows, bootloader verifies itself (own image) and hence the secure version check during that must be avoided. Regression introduced in recent commit-id: 3305cb4d Tested that both secure boot and flash-enc workflows work correctly with the anti-rollback scenario. --- components/bootloader_support/src/esp_image_format.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/components/bootloader_support/src/esp_image_format.c b/components/bootloader_support/src/esp_image_format.c index 324440f3da52..bd83ebd3d6c9 100644 --- a/components/bootloader_support/src/esp_image_format.c +++ b/components/bootloader_support/src/esp_image_format.c @@ -684,12 +684,16 @@ static esp_err_t process_segment_data(int segment, intptr_t load_addr, uint32_t #endif } uint32_t *dest = (uint32_t *)load_addr; -#endif +#endif // BOOTLOADER_BUILD const uint32_t *src = data; #if CONFIG_BOOTLOADER_APP_ANTI_ROLLBACK - if (segment == 0) { + // Case I: Bootloader verifying application + // Case II: Bootloader verifying bootloader + // Anti-rollback check should handle only Case I from above. + if (segment == 0 && metadata->start_addr != ESP_BOOTLOADER_OFFSET) { + ESP_LOGD(TAG, "additional anti-rollback check 0x%"PRIx32, data_addr); // The esp_app_desc_t structure is located in DROM and is always in segment #0. size_t len = process_esp_app_desc_data(src, sha_handle, checksum, metadata); data_len -= len;