DRAFT: ESP32-XX hardware flash encryption issue when updating images - "bootutil layer" solution #3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
One of includes is not available when bypassing ASN1 encoding as mbedTLS is no longer enabled for compilation. Discovered with zephyr, but common for other platforms, after recent changes in CMakeLists.txt. Signed-off-by: Dominik Ermel <[email protected]>
Commit removes files needed for ASN1 parsing from compilation, when ASN1 bypass is enabled. Signed-off-by: Dominik Ermel <[email protected]>
Make selection of MBEDTLS_ASN1_PARSE_C, in BOOT_ED25519_MBEDTLS, depending on ASN1 parsing being enabled. Signed-off-by: Dominik Ermel <[email protected]>
ED25519 with mbedTLS has not been linking due to missing SHA512, which is internally required by ED25519 implementation. Signed-off-by: Dominik Ermel <[email protected]>
Fixes directly accessing an element of this object with one of the helper functions Signed-off-by: Jamie McCrae <[email protected]>
All of ED25519 backends allow SHA512, together with SHA512. The ED25519 internally requires SHA512 for calculations, but image may be hashed with any SHA algorithm. The PSA has also been missing selecting of any SHA as allowed. Signed-off-by: Dominik Ermel <[email protected]>
Removes lines that have never done anything because this is already the default Signed-off-by: Jamie McCrae <[email protected]>
The commit adds two MCUboot configuration options: - MCUBOOT_SUPPORT_DEV_WITHOUT_ERASE - MCUBOOT_SUPPORT_DEV_WITH_ERASE - MCUBOOT_MINIMAL_SCRAMBLE The first one should be enabled to support devices that do not require erase. When such devices are used in system then MCUboot will avoid erasing such device, which is not needed by hardware, and will just write data to it. This allows to both improve device lifetime and reduce time of operations like swap. The second option is just bringing a configuration option for already existing support for deviceses with erase. The third option allows to reduce amount of removed data. When enabled, MCUboot will remove enough of data, depending on the purpose of the removal, to just fulfill the purpose; for example if removal of data is done to make image unrecognizable for MCUboot, with this option, it will only remove header. Signed-off-by: Dominik Ermel <[email protected]>
Add Kconfig options:
- CONFIG_MCUBOOT_STORAGE_WITHOUT_ERASE that enables MCUboot
configuration MCUBOOT_SUPPORT_DEV_WITHOUT_ERASE
- CONFIG_MCUBOOT_STORAGE_WITH_ERASE that enables MCUboot
configuration MCUBOOT_SUPPORT_DEV_WITH_ERASE
- CONFIG_MCUBOOT_STORAGE_MINIMAL_SCRAMBLE that enables MCUboot
configuration MCUBOOT_MINIMAL_SCRAMBLE
Adds implementation of flash_area_erase_required, which is required when
MCUBOOT_STORAGE_DEV_WITHOUT_ERASE is enabled.
Signed-off-by: Dominik Ermel <[email protected]>
The intention of bs_custom_storage_erase is to remove data from device; to support devices that do not require erase, without calling erase, so that devices that do not implement such functions could work, the flash_area_erase has been replaced with flash_area_flatten. Signed-off-by: Dominik Ermel <[email protected]>
By default enable all other systems to work with devices that require erase prior to write. Signed-off-by: Dominik Ermel <[email protected]>
Accidentally added check for size of blen against hash length, in bootutil_verify, was doubling check done in bootutli_verify_sig and prevented pure signature from working. Signed-off-by: Dominik Ermel <[email protected]>
MCUBOOT_USE_PSA_CRYPTO should be set by CONFIG_BOOT_USE_PSA_CRYPTO instead of CONFIG_MBEDTLS_PSA_CRYPTO_CLIENT. Signed-off-by: Dominik Ermel <[email protected]>
Fixes an issue where the variable might not be set and be empty, and would still be included which would cause a compiler include empty file error Signed-off-by: Jamie McCrae <[email protected]>
This resolves a warning when building with `-Wsign-compare`. `struct flash_area.fa_size` is declared as `size_t` in the Zephyr source tree (in `include/zephyr/storage/flash_map.h`). Signed-off-by: Samuel Coleman <[email protected]>
Fixes tags for pygments Signed-off-by: Jamie McCrae <[email protected]>
ECDSA signatures are encoded as ASN.1 and the size of the ASN.1 representation can vary depending on the value of the two integers the signature is composed of. This means that when ECDSA is used, the size of the TLV area is not always equal to the size that was estimated by the simulator when attempting to determine the maximum image size. Indeed, the estimate gives the maximum possible size of the TLV area and depending on its actual size, the generated images might be in fact a bit smaller than expected. This is not a big issue but adds a bit of randomness in the simulation and make difficult to generate precisely oversized images when desired for example. This commit ensures an ECDSA signature with the largest possible size is always used, making the size of the corresponding TLV entry constant in the simulator. Signed-off-by: Thomas Altenbach <[email protected]>
To generate oversized, the simulator needs to know the maximum image size. To obtain such size, the size of the TLV area is estimated and when using ECDSA, the actual size of the TLV area in the generated image was not always equal to the estimated size. This required to add a bit more data than what should be necessary when creating oversized images, to ensure the generated images will actually be oversized in most cases. Thanks to the previous commit, this is no more necessary and it is now possible to reliably generate oversized images with the smallest size. Signed-off-by: Thomas Altenbach <[email protected]>
For the overwrite-only upgrade strategy, the trailer size computed by the simulator and used to determine the maximum image size was not correct. This commit fixes the issue. Having an underestimated trailer size was causing the 'oversized_secondary_slot' to fail since the previous commit, because the oversized images are now generated to have the smallest possible size. Signed-off-by: Thomas Altenbach <[email protected]>
Since 1b2fc09, many places now reuse the flash area pointer from the bootloader state. Unfortunately, some RAM load usage (on single loader or runtime-source sample) didn't set up the flash area pointer on the bootloader state, so they were broken. This patch fixes that by adding the flash area pointer to the created bootloader states - directly or via a new parameter to boot_load_image_from_flash_to_sram(). Signed-off-by: Ederson de Souza <[email protected]>
This will encompass both CONFIG_BOOT_RAM_LOAD and CONFIG_SINGLE_APPLICATION_SLOT_RAM_LOAD, which, at this point, should follow the same code path - load image to RAM. Signed-off-by: Ederson de Souza <[email protected]>
Add default configuration for mcx_n9xx_evk. Signed-off-by: Derek Snell <[email protected]>
Fixes an issue whereby another module might have declared this by undefining it if it's already set Signed-off-by: Jamie McCrae <[email protected]>
Added basic adaptations needed for introducing nrf54h20dk board support in the future. Signed-off-by: Michal Kozikowski <[email protected]>
Signed-off-by: Guillaume G. <[email protected]>
Clang wrongly throws a warning, which will be treated as an error in twister builds, add pointless workaround to set variable that is already set by the hook function to avoid this Signed-off-by: Jamie McCrae <[email protected]>
Code implied that WATCHDOG_INTERVAL will enable watchdog in bootloader however it never did hal_watchdog_init sets up some watchdog data but for most mcu is does not start watchdog. Now hal_watchdog_enable() is called when WATCHDOG_INTERVAL is set to non zero as git history suggested . Signed-off-by: Jerzy Kasenberg <[email protected]>
Now flag defining config file for MbedTLS is global (see apache/mynewt-core#3394), so we do not have to include the same flag in bootutil package. This also enables MBEDTLS_CIPHER_MODE_CTR in boot_serial test package, as it is used in unit tests and it is disabled by default. Signed-off-by: Michal Gorecki <[email protected]>
Before the PR mcu-tools#2206 fixing issues in the swap-scratch logic when the trailer is larger than the last sector was merged, the PR mcu-tools#2114 was reverted (see mcu-tools#2211). This changes made by mcu-tools#2211 were then reapplied (see mcu-tools#2216) after mcu-tools#2206 was merged, leading to some conflicts: a call to swap_erase_trailer_sectors was moved by mcu-tools#2206 but the old call to that function was added back by mcu-tools#2216 and a new call to swap_erase_trailer_sectors was added by mcu-tools#2206 but not converted to a call to swap_scramble_trailer_sectors by mcu-tools#2216. Signed-off-by: Thomas Altenbach <[email protected]>
app_max_sectors() is always off by one in its calculation. This corrects that calculation as well as the warning which should only be comparing the two slot sector counts and checking to see if they are different by one sector for the swap sector. The size of the application is not relevant in that warning for optimal sector distribution. Additionally app_max_size() suffers similarly in that it is not taking into account the fact that the secondary slot does also contain a trailer. Finally makes a minor addition to the design document which further clarifies the primary and secondary partition size differences. Signed-off-by: Robert Wolff <[email protected]>
The function has been provided to support Zephyr, but is not needed anymore. It does not even compile, actually. Signed-off-by: Dominik Ermel <[email protected]>
Remove redundant application size calculations in favor of a swap-specific function, implemented inside swap_<type>.c. In this way, slot sizes use the same restrictions as image validation. Signed-off-by: Tomasz Chyrowicz <[email protected]>
Applies zephyr coding style Signed-off-by: Jamie McCrae <[email protected]>
Makes this sample path listed in the zephyr module file Signed-off-by: Jamie McCrae <[email protected]>
Option to put execution in infinite loop. Meant to be used for debug. Signed-off-by: Mateusz Michalek <[email protected]>
Add noreturn attribute the fih_panic_loop() and add a terminal
endless loop in the function to prevent build warnings like
the below one found when build MCUBOOT in TF-M:
.../arm-none-eabi/include/assert.h: In function '__assert_func':
.../trusted-firmware-m/platform/ext/common/tfm_assert.c:22:1: warning: 'noreturn' function does return
22 | }
| ^
Signed-off-by: Etienne Carriere <[email protected]>
Removes support for bootloader-led0, as this has been replaced with mcuboot-led0 Signed-off-by: Jamie McCrae <[email protected]>
These are not needed Signed-off-by: Jamie McCrae <[email protected]>
Add Cortex-R support by using ifdefs to change the vector table, remove CORTEX_M only code and do an explicit bx instruction to switch from Thumb to ARM mode on exit of the MCUboot application. Signed-off-by: Mika Braunschweig <[email protected]>
Add code to cleanup the Cortex-R processor state to the reset configuration and disable + acknowledge all interrupts before entering the booted application. Signed-off-by: Mika Braunschweig <[email protected]>
Add notes about the additions made in order to support Cortex-R booting. Signed-off-by: Mika Braunschweig <[email protected]>
The nrf54h20 power domains are forced on at boot, no need to enable them or their drivers unless they are to be managed further within the bootloader. Signed-off-by: Bjarki Arge Andreasen <[email protected]>
To prevent breakage when the default mode changes to swap using offset, any board that is current set up to have more sectors in slot0 has been excluded from the move Signed-off-by: Jamie McCrae <[email protected]>
Defaults to the new swap using offset mode, excluding stm32 as these devices are already aligned to swap using move with extra sectors in slot0 Signed-off-by: Jamie McCrae <[email protected]>
Commit introduces BOOT_SOMETHING_USES_SHA<256,384,512> Kconfig options that can be used to control what algorithms should be compiled in with crypto backends. Signed-off-by: Dominik Ermel <[email protected]>
Adds Kconfig option CONFIG_BOOT_ECDSA_PSA that allows to switch ECDSA to PSA backend. Signed-off-by: Artur Hadasz <[email protected]>
Allow to depend on a specific slot while specifying the version number. This functionality is useful when the Direct XIP mode is used and the booting process of other images is done by the next stage, not the MCUboot itself. Signed-off-by: Tomasz Chyrowicz <[email protected]>
Add a configuration file for the nRF52840 dongle bare variant. Signed-off-by: Tomasz Chyrowicz <[email protected]>
This MCUboot configuration option turns off matching of public key hash, taken from image TLV, against built in public key. Such verification is not needed when there is only one key built in as the signature verification will reject image signed with unknown key anyway. Enabling the option allows to slightly reduce MCUboot binary size by removing the code that does the key matching. Boot time improvement is not really significant. Signed-off-by: Dominik Ermel <[email protected]>
Add Zephyr support for MCUBOOT_BYPASS_KEY_MATCH Signed-off-by: Dominik Ermel <[email protected]>
Align the project security policy with the Trusted Firmware security policy at: https://www.trustedfirmware.org/.well-known/security.txt Signed-off-by: Dan Handley <[email protected]>
Not needed in that unit. Signed-off-by: Dominik Ermel <[email protected]>
The commit uses typedef to define common name for key exchange in order to reduce number of local definitions and #ifdef in code. Signed-off-by: Dominik Ermel <[email protected]>
Allows to reuse size_t variables. Signed-off-by: Dominik Ermel <[email protected]>
Add a possibility to read the current value of the hardware security counter from the application. Signed-off-by: Tomasz Chyrowicz <[email protected]>
…sif Port Signed-off-by: Almir Okato <[email protected]>
Signed-off-by: Almir Okato <[email protected]>
Signed-off-by: Almir Okato <[email protected]>
MCUboot's state machine relies on erased valued data (e.g. 0xFF) readed from this erased region that could be not written before, however if the flash device has hardware flash encryption and its flash read operation always decrypts what is being read from flash, thus a region that was erased would not be read as what MCUboot expected (after erasing, the region physically contains 0xFF, but once reading it, flash controller decrypts 0xFF to something else). So this configuration force the erased value into the region after the erasing the trailer regions, and also make an erase operation before writing trailers. Signed-off-by: Almir Okato <[email protected]>
almir-okato
force-pushed
the
fix_esp_mcuboot_flash_enc
branch
from
5f92c79 to
3822a5c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
bootutil: add MCUBOOT_FLASH_HAS_HW_ENCRYPTION config-condition
MCUboot's state machine relies on erased valued data (e.g. 0xFF) readed from this erased region that could be not written before, however if the flash device has hardware flash encryption and its flash read operation always decrypts what is being read from flash, thus a region that was erased would not be read as what MCUboot expected (after erasing, the region physically contains 0xFF, but once reading it, flash controller decrypts 0xFF to something else).
So this configuration force the erased value into the region after the erasing the trailer regions, and also make an erase operation before writing trailers.
Use Zephyr branch from this PR:
zephyrproject-rtos/zephyr#90442
Use hal_espressif branch from this PR:
zephyrproject-rtos/hal_espressif#446
Prepare and enable Flash Encryption on ESP32-S3 (the conf files are already modified on the used branches):
Building the sample application on Zephyr:
west build -b esp32s3_devkitm/esp32s3/procpu -p -- -DEXTRA_CONF_FILE="overlay-bt.conf"build/zephyr/zephyr.signed.confirmed.bin) can be used for testing the scenario in which an image is already confirmedBuilding and flashing MCUboot:
(MCUBOOT_DIR should be the directory from Zephyr environment:
<ZEPHYRPROJECT_DIR>/bootloader/mcuboot)