Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bootutil: Store image encrypted in scratch area #1952

Merged
merged 5 commits into from
Jul 15, 2024

Conversation

taltenbach
Copy link
Contributor

@taltenbach taltenbach commented May 2, 2024

Description

As discussed on #1942, when swap using scratch is used, the image is currently decrypted while copying from the secondary slot to the scratch area. This means even when BOOT_SWAP_SAVE_ENCTLV is enabled, the scratch area has to reside in the MCU's internal flash memory.

However, being able to place the scratch area in an external flash memory could be valuable since:

  • It would enable to have more space available in internal flash memory, which could be used e.g. the increase the size of the primary slot(s).
  • There is typically more space available in external flash memory, which would make possible to have a larger scratch area, reducing flash wear.
  • The external flash memory might have higher endurance than the internal flash memory, increasing the number of possible updates.

This PR proposes changes to perform the decryption while copying from the scratch area to the secondary slot instead, meaning the scratch area does not contain plaintext image data anymore.

The reason why this wasn't performed that way in the first place is probably that it introduces an issue when the device is reset after having erased the first sector of the secondary slot but before having copied it from the scratch area to the primary slot. Indeed, for being able to properly decrypt an image, MCUboot needs the header of that image (cf. boot_copy_region). This header is located at the beginning of the secondary slot until the first sector of the secondary slot is processed. At this time:

  1. The secondary image's header is copied to scratch area.
  2. The secondary image's header header is erased in the secondary slot and replaced by the header of the primary image.
  3. The primary image's header is erased in the primary slot and replaced by the header of the secondary image.

If a reset occurs after step 1, the secondary image's header might not be located anymore in the secondary slot but is still needed when using encrypted scratch since the first segment of the image has to be decrypted during step 3. However, when using swap using scratch, MCUboot is currently always looking for the primary and secondary image's headers in respectively the primary and secondary slots, regardless of the current boot status (cf. boot_read_image_header). Therefore, this MR also modifies the boot_read_image_header routine to properly load the image headers, according to the current progress of the upgrade.

Tests

  • Simulator OK with enc-x25519, enc-x25519 multiimage, enc-x25519 overwrite-only, enc-x25519 ram-load, enc-x25519 swap-move and direct-xip.
  • Update and revert on an STM32F413-based device, with two encrypted images and scratch area in external flash memory.

@utzig
Copy link
Member

utzig commented Jul 10, 2024

Please rebase on latest main, I'll try to review and test tomorrow or Friday.

For swap using scratch, the boot_read_image_header routine, responsible
for reading the image headers, was always looking for the primary and
secondary image's headers at the beginning of respectively the primary
and secondary slots, regardless of the current boot status.

This means if during a swap-scratch upgrade a reset happens after the
sector containing the image header in the primary or secondary slot has
been erased, invalid image headers were read since at that time the
location of the headers has changed.

Currently, this doesn't seem to cause any issue because the swap-scratch
algorithm is implemented in such a way the content of the headers is no
more necessary when the headers are erased. However, to be able to
decrypt the secondary image when copied to the primary slot instead of
when copied to the scratch area, properly reading the secondary image's
header is required even after it has been erased from the secondary
slot.

To that end, the boot_read_image_header is modified to determine from
the boot status the current location of the image headers and to always
read the actual header, no matter the current state of the upgrade
process.

Signed-off-by: Thomas Altenbach <thomas.altenbach@legrand.com>
Currently, when swap using scratch is used with encrypted images,
MCUboot is decrypting the images during the copy from the secondary slot
to the scratch area. This means the scratch area contains plaintext
image data and therefore that the scratch area must be placed in the
MCU's internal flash memory. This commit makes the necessary changes to
perform the decryption when copying from the scratch area to the primary
slot instead, making possible to place the scratch area in an external
flash memory since the scratch area is now encrypted.

Note that BOOT_SWAP_SAVE_ENCTLV must be enabled if the scratch area is
placed in external flash memory.

Signed-off-by: Thomas Altenbach <thomas.altenbach@legrand.com>
When using swap using scratch, the decryption now happens when copying
from the scratch area to the primary slot, which means the image is
stored encrypted in the scratch area. This commit updates the
documentation accordingly.

Signed-off-by: Thomas Altenbach <thomas.altenbach@legrand.com>
When 'direct-xip' or 'ram-load' features were enabled,
CONFIG_BOOT_SWAP_USING_SCRATCH and MCUBOOT_SWAP_USING_SCRATCH were
defined even though swap using scratch wasn't used. This commit fixes
the issue.

Signed-off-by: Thomas Altenbach <thomas.altenbach@legrand.com>
@taltenbach
Copy link
Contributor Author

Thanks @utzig, rebased on main

@utzig
Copy link
Member

utzig commented Jul 14, 2024

Also please add a release notes snippet as described here: https://docs.mcuboot.com/SubmittingPatches.html#release-notes

Add release note snippet regarding the change made to the swap with
scratch algorithm to avoid having plaintext firmware data stored in the
scratch area.

Signed-off-by: Thomas Altenbach <thomas.altenbach@legrand.com>
@taltenbach
Copy link
Contributor Author

@utzig Release note snippet added

Copy link
Member

@utzig utzig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@utzig utzig merged commit c4b89ba into mcu-tools:main Jul 15, 2024
58 checks passed
@utzig
Copy link
Member

utzig commented Jul 15, 2024

@taltenbach Thanks for the work and patience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants