sysbuild: Emit warning log when MCUboot uses KMU #19352

MarekPieta · 2024-12-09T15:00:29Z

Using KMU for MCUboot keys requires provisioning the device manually before running west flash. Emit a CMake warning to ensure that user is aware that an additional action is required.

Jira: NCSDK-30742

Using KMU for MCUboot keys requires provisioning the device manually before running `west flash`. Emit a CMake warning to ensure that user is aware that an additional action is required. Jira: NCSDK-30742 Signed-off-by: Marek Pieta <[email protected]>

NordicBuilder · 2024-12-09T15:06:55Z

CI Information

^{To view the history of this post, clich the 'edited' button above}
Build number: 1

Inputs:

Sources:

sdk-nrf: PR head: 7f3742fb38836ff0831ca2b85c866f8904826c78

more details

sdk-nrf:

PR head: 7f3742fb38836ff0831ca2b85c866f8904826c78
merge base: a97ba919e45db56a60938d89ded2cb31920f985e
target head (main): a97ba919e45db56a60938d89ded2cb31920f985e
Diff

Github labels

Enabled	Name	Description
	ci-disabled	Disable the ci execution
	ci-all-test	Run all of ci, no test spec filtering will be done
	ci-force-downstream	Force execution of downstream even if twister fails
	ci-run-twister	Force run twister
	ci-run-zephyr-twister	Force run zephyr twister

List of changed files detected by CI (2)

sysbuild
│  ├── CMakeLists.txt
│  │ Kconfig.mcuboot

Outputs:

Toolchain

Version: b77d8c1312
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:b77d8c1312_912848a074

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

◻️ Toolchain - Skipped: existing toolchain is used
✅ Build twister
❌ Integration tests
- ❌ test-sdk-audio
- ✅ desktop52_verification
- ✅ test-fw-nrfconnect-boot
- ✅ test-fw-nrfconnect-apps
- ❌ test_ble_nrf_config
- ✅ test-fw-nrfconnect-ble_mesh
- ✅ test-fw-nrfconnect-ble_samples
- ❌ test-fw-nrfconnect-chip
- ✅ test-fw-nrfconnect-nfc
- ✅ test-fw-nrfconnect-nrf-iot_libmodem-nrf
- ✅ test-fw-nrfconnect-nrf-iot_serial_lte_modem
- ❌ test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
- ✅ test-fw-nrfconnect-nrf-iot_samples
- ✅ test-fw-nrfconnect-nrf-iot_lwm2m
- ✅ test-fw-nrfconnect-nrf-iot_thingy91
- ✅ test-fw-nrfconnect-nrf_crypto
- ✅ test-fw-nrfconnect-rpc
- ✅ test-fw-nrfconnect-rs
- ✅ test-fw-nrfconnect-fem
- ✅ test-fw-nrfconnect-tfm
- ✅ test-fw-nrfconnect-thread
- ✅ test-fw-nrfconnect-zigbee
- ✅ test-sdk-find-my
- ✅ test-fw-nrfconnect-nrf-iot_mosh
- ✅ test-fw-nrfconnect-nrf-iot_positioning
- ✅ test-sdk-sidewalk
- ✅ test-sdk-wifi
- ✅ test-low-level
- ✅ test-fw-nrfconnect-nrf-iot_nrf_provisioning
- ✅ test-sdk-pmic-samples
- ✅ test-sdk-mcuboot
- ✅ test-sdk-dfu
- ✅ test-fw-nrfconnect-ps
- ✅ test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

NordicBuilder · 2024-12-09T16:12:14Z

Memory footprint analysis revealed the following potential issues

sample.matter.template.release[nrf7002dk/nrf5340/cpuapp]: High ROM usage: 820678[B] - link (cc: @kkasperczyk-no @ArekBalysNordic @markaj-nordic)
applications.sdp.gpio.icbmsg[nrf54l15dk/nrf54l15/cpuflpr]: High ROM usage: 8568[B] - link (cc: @nrfconnect/ncs-ll-ursus)
sample.matter.template.debug[nrf7002dk/nrf5340/cpuapp]: High ROM usage: 911846[B] - link (cc: @kkasperczyk-no @ArekBalysNordic @markaj-nordic)

Note: This message is automatically posted and updated by the CI (latest/sdk-nrf/PR-19352/1)

zycz · 2024-12-09T16:38:50Z

sysbuild/CMakeLists.txt

+        if(NOT SB_CONFIG_MCUBOOT_SIGNATURE_USING_KMU_SKIP_WARNING)
+          message(WARNING "
+          ------------------------------------------------------------------------------
+          --- WARNING: MCUboot uses KMU stored keys for signature verification. Make ---


Add link to provisioning documentation

Also add information that without keys you won't be able to run application and application won't start so that a user could quickly identify this warning as a cause of his potential problem.

nordicjm

I am really not fond of this annoying warning

zycz · 2024-12-10T06:53:17Z

I am really not fond of this annoying warning

Warning would be very helpful. This feature is active for ~1 week and already 2 persons wasted some time trying to figure out why sample is not working

nordicjm · 2024-12-10T07:25:15Z

I am really not fond of this annoying warning

Warning would be very helpful. This feature is active for ~1 week and already 2 persons wasted some time trying to figure out why sample is not working

Here's the 2 flow routes for this:

A user picks a sample direct from NCS without reading any documentation, builds it, flashes it, it does not work
A user has their own project with a team of people working on it, they disable the warning because it's annoying and completely irrelevant for them and they want clean CI build logs. A new user joins the team, builds the code, flashes it, it does not work

This does absolutely nothing for 2, it will only ever do anything for 1 because in a customer use case they're going to turn this off as soon as they know how to use it. On that basis I will not accept this as it is, I will however accept a NCS sample-specific Kconfig that samples/applications can opt-in to (note: opt-in, not opt-out), can be sysbuild/Kconfig.samples and sysbuild/samples.cmake, or alternatively the sysbuild.cmake file in the application can raise the warning

wbober · 2024-12-10T09:25:35Z

I am really not fond of this annoying warning

Warning would be very helpful. This feature is active for ~1 week and already 2 persons wasted some time trying to figure out why sample is not working

Which sample?

nvlsianpu

I see emitting this warning for anything as unwanted. I'm fine with doing that for NCS samples or applications. Don't have strong opinion on opt-in vs opt-out. I think possibility for suppress this by default for given NCS installation might be a very good idea.

MarekPieta · 2024-12-10T10:47:31Z

I am really not fond of this annoying warning

Warning would be very helpful. This feature is active for ~1 week and already 2 persons wasted some time trying to figure out why sample is not working

Which sample?

nRF Desktop application (nrf/applications/nrf_desktop) on nRF54L15 DK uses MCUboot with HW crypto. In this configuration, MCUboot uses KMU to store public keys required for application image validation. The keys need to be provisioned manually by user before running west flash - otherwise MCUboot will be unable to verify application and eventually the application will not be booted (for more details, see #19356)

nordicjm · 2024-12-10T10:53:47Z

I am really not fond of this annoying warning

Warning would be very helpful. This feature is active for ~1 week and already 2 persons wasted some time trying to figure out why sample is not working

Which sample?

nRF Desktop application (nrf/applications/nrf_desktop) on nRF54L15 DK uses MCUboot with HW crypto. In this configuration, MCUboot uses KMU to store public keys required for application image validation. The keys need to be provisioned manually by user before running west flash - otherwise MCUboot will be unable to verify application and eventually the application will not be booted (for more details, see #19356)

Note: which is no different that any non-secure image, which is documented in the readme files for said samples: https://github.com/nrfconnect/sdk-nrf/tree/main/samples/tfm/tfm_psa_template#building-and-running

MarekPieta · 2024-12-10T10:56:37Z

Here's the 2 flow routes for this:

A user picks a sample direct from NCS without reading any documentation, builds it, flashes it, it does not work

A user has their own project with a team of people working on it, they disable the warning because it's annoying and completely irrelevant for them and they want clean CI build logs. A new user joins the team, builds the code, flashes it, it does not work

This does absolutely nothing for 2, it will only ever do anything for 1 because in a customer use case they're going to turn this off as soon as they know how to use it. On that basis I will not accept this as it is, I will however accept a NCS sample-specific Kconfig that samples/applications can opt-in to (note: opt-in, not opt-out), can be sysbuild/Kconfig.samples and sysbuild/samples.cmake, or alternatively the sysbuild.cmake file in the application can raise the warning

I would not insist on introducing a build warning enabled by default. Still, the missing KMU provisioning is related to MCUboot bootloader, so maybe we could introduce a build warning in scope of MCUboot or MCUboot's sysbuild integration in NCS (using an opt-in Kconfig option)? I would expect that other NCS applications may also use KMU for the MCUboot keys soon. A common warning would allow to achieve consistent user experience in scope of nRF Connect SDK.

If you think that the warning is not a proper approach, we could also consider relying only on documentation (e.g. this PR introduces documentation for nRF Desktop application: #19356). Then I would close this PR.

nordicjm · 2024-12-10T11:01:00Z

Here's the 2 flow routes for this:

A user picks a sample direct from NCS without reading any documentation, builds it, flashes it, it does not work

A user has their own project with a team of people working on it, they disable the warning because it's annoying and completely irrelevant for them and they want clean CI build logs. A new user joins the team, builds the code, flashes it, it does not work

This does absolutely nothing for 2, it will only ever do anything for 1 because in a customer use case they're going to turn this off as soon as they know how to use it. On that basis I will not accept this as it is, I will however accept a NCS sample-specific Kconfig that samples/applications can opt-in to (note: opt-in, not opt-out), can be sysbuild/Kconfig.samples and sysbuild/samples.cmake, or alternatively the sysbuild.cmake file in the application can raise the warning

I would not insist on introducing a build warning enabled by default. Still, the missing KMU provisioning is related to MCUboot bootloader, so maybe we could introduce a build warning in scope of MCUboot or MCUboot's sysbuild integration in NCS (using an opt-in Kconfig option)? I would expect that other NCS applications may also use KMU for the MCUboot keys soon. A common warning would allow to achieve consistent user experience in scope of nRF Connect SDK.

If you think that the warning is not a proper approach, we could also consider relying only on documentation (e.g. this PR introduces documentation for nRF Desktop application: #19356). Then I would close this PR.

The warning can be added as per my comment above for NCS samples, not being enabled at an MCUboot or NSIB level, it is expected that if someone enables them in their applications then they should have first read how to use them like with any feature in NCS/zephyr. This Kconfig should be used, opt-in, for samples only, it can display a cmake warning, samples will need to select the Kconfig manually in their Kconfig.sysbuild file. This also means that the warning will transfer when those applications are copied out of tree too, which will remind users about if it they have forgotten and they can remove the selection during development

MarekPieta · 2024-12-11T14:19:54Z

Taking into account comments from @nordicjm and @zycz, I eventually decided to make it an nRF Desktop application-specific CMake warning: #19440

Introducing a sysbuild/Kconfig.samples and sysbuild/samples.cmake only for this small warning may not be worth it. Having an application-specific warning also allows linking to application-specific documentation (as suggested by @zycz).

I close this PR.

MarekPieta added the backport v2.9-branch auto-create a PR with same commits to v2.9-branch label Dec 9, 2024

MarekPieta added this to the 2.9.0 milestone Dec 9, 2024

MarekPieta requested review from nvlsianpu, zycz, de-nordic, alstrzebonski and a team December 9, 2024 15:00

MarekPieta requested a review from a team as a code owner December 9, 2024 15:00

github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Dec 9, 2024

MarekPieta removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Dec 9, 2024

zycz reviewed Dec 9, 2024

View reviewed changes

nordicjm reviewed Dec 10, 2024

View reviewed changes

nvlsianpu reviewed Dec 10, 2024

View reviewed changes

MarekPieta closed this Dec 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sysbuild: Emit warning log when MCUboot uses KMU #19352

sysbuild: Emit warning log when MCUboot uses KMU #19352

MarekPieta commented Dec 9, 2024

NordicBuilder commented Dec 9, 2024 •

edited

Loading

NordicBuilder commented Dec 9, 2024

zycz Dec 9, 2024

nordicjm left a comment

zycz commented Dec 10, 2024 •

edited

Loading

nordicjm commented Dec 10, 2024 •

edited

Loading

wbober commented Dec 10, 2024

nvlsianpu left a comment •

edited

Loading

MarekPieta commented Dec 10, 2024

nordicjm commented Dec 10, 2024

MarekPieta commented Dec 10, 2024

nordicjm commented Dec 10, 2024 •

edited

Loading

MarekPieta commented Dec 11, 2024

sysbuild: Emit warning log when MCUboot uses KMU #19352

sysbuild: Emit warning log when MCUboot uses KMU #19352

Conversation

MarekPieta commented Dec 9, 2024

NordicBuilder commented Dec 9, 2024 • edited Loading

CI Information

Inputs:

Sources:

Github labels

Outputs:

Toolchain

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

NordicBuilder commented Dec 9, 2024

Memory footprint analysis revealed the following potential issues

zycz Dec 9, 2024

Choose a reason for hiding this comment

nordicjm left a comment

Choose a reason for hiding this comment

zycz commented Dec 10, 2024 • edited Loading

nordicjm commented Dec 10, 2024 • edited Loading

wbober commented Dec 10, 2024

nvlsianpu left a comment • edited Loading

Choose a reason for hiding this comment

MarekPieta commented Dec 10, 2024

nordicjm commented Dec 10, 2024

MarekPieta commented Dec 10, 2024

nordicjm commented Dec 10, 2024 • edited Loading

MarekPieta commented Dec 11, 2024

NordicBuilder commented Dec 9, 2024 •

edited

Loading

zycz commented Dec 10, 2024 •

edited

Loading

nordicjm commented Dec 10, 2024 •

edited

Loading

nvlsianpu left a comment •

edited

Loading

nordicjm commented Dec 10, 2024 •

edited

Loading