Skip to content

bootloader: mcuboot: Changes needed to support AES256 #93809

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

bootloader: mcuboot: Changes needed to support AES256 #93809

wants to merge 1 commit into from
+56 −4

Conversation

ahasztag
Copy link
Contributor

This commit adds changes which are necessary to support the AES256 encryption algorithm in mcuboot.

@ahasztag
Copy link
Contributor Author

ahasztag commented Jul 29, 2025 •
edited
Loading

The complaince here fails as it needs changes from mcu-tools/mcuboot#2406

@nordicjm Should I open some PR to https://github.com/zephyrproject-rtos/mcuboot or should I simply wait until the mcu-tools/mcuboot PR is merged and then the changes are imported to the zephyr mcuboot?

@nordicjm
Copy link
Contributor

The complaince here fails as it needs changes from mcu-tools/mcuboot#2406

@nordicjm Should I open some PR to https://github.com/zephyrproject-rtos/mcuboot or should I simply wait until the mcu-tools/mcuboot PR is merged and then the changes are imported to the zephyr mcuboot?

Add a commit (before referencing the Kconfigs) which updates west.yml with:

diff --git a/west.yml b/west.yml
index 42952106a1c..bddcea727fd 100644
--- a/west.yml
+++ b/west.yml
@@ -23,6 +23,8 @@ manifest:
       url-base: https://github.com/zephyrproject-rtos
     - name: babblesim
       url-base: https://github.com/BabbleSim
+    - name: mcu-tools
+      url-base: https://github.com/mcu-tools
 
   group-filter: [-babblesim, -optional]
 
@@ -310,8 +312,9 @@ manifest:
       groups:
         - crypto
     - name: mcuboot
-      revision: 461e060e8687c9a75074effcc61d7f68c5112cfd
+      revision: pull/2406/head
       path: bootloader/mcuboot
+      remote: mcu-tools
       groups:
         - bootloader
     - name: mipi-sys-t

and see if it passes, then it can be updated properly later once merged and brought into the zephyr fork

@ahasztag ahasztag force-pushed the upstream_mcuboot_aes256_support branch 2 times, most recently from 801002a to 2ba96aa Compare July 29, 2025 11:13
@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 29, 2025 •
edited
Loading

The following west manifest projects have changed revision in this Pull Request:

Name Old Revision New Revision Diff

✅ All manifest checks OK

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@github-actions github-actions bot added manifest manifest-mcuboot DNM (manifest) This PR should not be merged (controlled by action-manifest) labels Jul 29, 2025
@ahasztag ahasztag force-pushed the upstream_mcuboot_aes256_support branch from 2ba96aa to f6e3466 Compare July 29, 2025 13:25
nordicjm
nordicjm reviewed Jul 31, 2025
View reviewed changes
share/sysbuild/image_configurations/MAIN_image_default.cmake Outdated
set_config_bool(${ZCMAKE_APPLICATION} CONFIG_MCUBOOT_ENCRYPTION_ALG_AES_256 y)
endif()
endif()

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@ahasztag ahasztag force-pushed the upstream_mcuboot_aes256_support branch from f6e3466 to 48896d4 Compare August 1, 2025 06:58
@ahasztag ahasztag requested a review from nordicjm August 1, 2025 06:58
nordicjm
nordicjm reviewed Aug 1, 2025
View reviewed changes
share/sysbuild/images/bootloader/CMakeLists.txt Outdated
Comment on lines 19 to 26
if(SB_CONFIG_BOOT_ENCRYPTION)
set_config_string(${image} CONFIG_BOOT_ENCRYPTION_KEY_FILE "${SB_CONFIG_BOOT_ENCRYPTION_KEY_FILE}")
if(SB_CONFIG_BOOT_ENCRYPTION_ALG_AES_128)
set_config_bool(${image} CONFIG_BOOT_ENCRYPT_ALG_AES_128 y)
elseif(SB_CONFIG_BOOT_ENCRYPTION_ALG_AES_256)
set_config_bool(${image} CONFIG_BOOT_ENCRYPT_ALG_AES_256 y)
endif()
endif()
Copy link
Contributor

Choose a reason for hiding this comment

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

yes these lines would go:

Suggested change
if(SB_CONFIG_BOOT_ENCRYPTION)
set_config_string(${image} CONFIG_BOOT_ENCRYPTION_KEY_FILE "${SB_CONFIG_BOOT_ENCRYPTION_KEY_FILE}")
if(SB_CONFIG_BOOT_ENCRYPTION_ALG_AES_128)
set_config_bool(${image} CONFIG_BOOT_ENCRYPT_ALG_AES_128 y)
elseif(SB_CONFIG_BOOT_ENCRYPTION_ALG_AES_256)
set_config_bool(${image} CONFIG_BOOT_ENCRYPT_ALG_AES_256 y)
endif()
endif()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, done

@ahasztag ahasztag force-pushed the upstream_mcuboot_aes256_support branch from 48896d4 to a7a31d3 Compare August 1, 2025 07:12
de-nordic
de-nordic previously approved these changes Aug 1, 2025
View reviewed changes
Copy link
Contributor

@de-nordic de-nordic left a comment

Choose a reason for hiding this comment

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

Looks OK, will have to wait for MCUboot sync.

west.yml Outdated
@@ -310,8 +312,9 @@ manifest:
groups:
- crypto
- name: mcuboot
revision: 461e060e8687c9a75074effcc61d7f68c5112cfd
revision: pull/2406/head
Copy link
Contributor

Choose a reason for hiding this comment

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

You can already change the revision to the sha, as your pr has been merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to proper revision

@ahasztag ahasztag force-pushed the upstream_mcuboot_aes256_support branch from a7a31d3 to 4707ce0 Compare August 4, 2025 10:21
@github-actions github-actions bot removed the DNM (manifest) This PR should not be merged (controlled by action-manifest) label Aug 4, 2025
nordicjm
nordicjm previously approved these changes Aug 6, 2025
View reviewed changes
@ahasztag ahasztag requested a review from de-nordic August 6, 2025 11:52
de-nordic
de-nordic previously approved these changes Aug 6, 2025
View reviewed changes
@ahasztag
Copy link
Contributor Author

ahasztag commented Aug 6, 2025

Ready to be merged!

nvlsianpu
nvlsianpu approved these changes Aug 6, 2025
View reviewed changes
Copy link
Contributor

@nvlsianpu nvlsianpu left a comment

Choose a reason for hiding this comment

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

just 2 nits

@ahasztag
bootloader: mcuboot: Changes needed to support AES256
9038c78 
This commit adds changes which are necessary to support
the AES256 encryption algorithm in mcuboot.

Signed-off-by: Artur Hadasz <[email protected]>
@ahasztag ahasztag dismissed stale reviews from de-nordic and nordicjm via 9038c78 August 7, 2025 07:57
@ahasztag ahasztag force-pushed the upstream_mcuboot_aes256_support branch from 4707ce0 to 9038c78 Compare August 7, 2025 07:57
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Aug 7, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@carlescufi carlescufi requested a review from nordicjm August 11, 2025 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nvlsianpu nvlsianpu nvlsianpu approved these changes

@nordicjm nordicjm nordicjm approved these changes

@de-nordic de-nordic de-nordic approved these changes

@57300 57300 Awaiting requested review from 57300

@d3zd3z d3zd3z Awaiting requested review from d3zd3z

@jeremybettis jeremybettis Awaiting requested review from jeremybettis

@nashif nashif Awaiting requested review from nashif

@tejlmand tejlmand Awaiting requested review from tejlmand

Assignees

@tejlmand tejlmand

Labels
area: Build System area: MCUBoot area: Sysbuild
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ahasztag @nordicjm @nvlsianpu @de-nordic @tejlmand @zephyrbot