-
Couldn't load subscription status.
- Fork 1.4k
nrf desktop autoprovisioning #23509
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
nrf desktop autoprovisioning #23509
Conversation
CI InformationTo view the history of this post, clich the 'edited' button above Inputs:Sources:sdk-nrf: PR head: d6143a58ee58b5a0b1a013e0999723f208c51d1e more detailssdk-nrf:
Github labels
List of changed files detected by CI (18)Outputs:ToolchainVersion: 8ea1732c3a Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped;
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| For this purpose a set of private and public keys is needed. | |
| For this purpose, a set of private and public keys is needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| In this application, the application image is automatically signed with private key by the |NCS| build system. | |
| In this application, the application image is automatically signed with a private key by the |NCS| build system. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you already mentioned in L1024 that the private key is used to sign the image so you can drop the "with a private key" i think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| To store public key in KMU public key must first be provisioned. | |
| To store the public key in the KMU, it must first be provisioned. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| This ensures that both the firmware and the MCUboot public key are correctly programmed onto the target device using KMU-based key storage. | |
| This ensures that both the firmware and the MCUboot public key are correctly programmed onto the target device using the KMU-based key storage. |
|
You can find the documentation preview for this PR here. Preview links for modified nRF Connect SDK documents: https://ncsdoc.z6.web.core.windows.net/PR-23509/nrf/releases_and_maturity/releases/release-notes-changelog.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you already mentioned in L1024 that the private key is used to sign the image so you can drop the "with a private key" i think
applications/nrf_desktop/sample.yaml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is already in the applications.nrf_desktop.zdebug, is it needed also here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove L05 from here.
It will fail during tests due to the missing log:
"dfu: Secondary image slot is clean"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
applications.nrf_desktop.zdebug is build only - here we run tests with harness, so I intended it. However if we you think we should forfeit those tests we can talk it over.
applications/nrf_desktop/sample.yaml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove L05 from here.
It will fail during tests due to the missing log:
"dfu: Secondary image slot is clean"
applications/nrf_desktop/configuration/nrf54l15dk_nrf54l15_cpuapp/sysbuild.conf
Outdated
Show resolved
Hide resolved
|
Couldn't respond to your comment directly :(
I wanted in first place show what keys there are so user won't confuse them. If you strongly disagree with it please propose alternative :) |
320b6f1 to
d4a9db6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| See the :ref:`ug_nrf54l_developing_provision_kmu` documenta`tion for details. | |
| See the :ref:`ug_nrf54l_developing_provision_kmu` documentation for details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| To use automatic provisioning, enable the ``SB_CONFIG_MCUBOOT_GENERATE_DEFAULT_KMU_KEYFILE`` | |
| To use automatic provisioning, enable the ``SB_CONFIG_MCUBOOT_GENERATE_DEFAULT_KMU_KEYFILE`` Kconfig option. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| The automatic provisioning is only performed if the west flash command is executed with the --erase or --recover flag. | |
| The automatic provisioning is only performed if the west flash command is executed with the ``--erase`` or ``--recover`` flag. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| .. parsed-literal:: | |
| :class: highlight | |
| west flash --recover | |
| .. parsed-literal:: | |
| :class: highlight | |
| west flash --recover |
d4a9db6 to
ab2763b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| To use automatic provisioning, enable the ``SB_CONFIG_MCUBOOT_GENERATE_DEFAULT_KMU_KEYFILE`` Kconfig option. | |
| To use automatic provisioning, enable the :kconfig:option:`SB_CONFIG_MCUBOOT_GENERATE_DEFAULT_KMU_KEYFILE` sysbuild Kconfig option. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rendered docs still do not allow me to click on the option label and navigate to the Kconfig search page. @nordicjm, are you sure that it works?
See.
https://ncsdoc.z6.web.core.windows.net/PR-23509/nrf/applications/nrf_desktop/bootloader_dfu.html
and the SB_CONFIG_MCUBOOT_GENERATE_DEFAULT_KMU_KEYFILE label in the incorrectly rendered ..note::
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it won't be searchable because only non-sysbuild Kconfigs are shown in the search, that's a different issue, however it highlights that it is a Kconfig option
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nordicjm : All the sysbuild configurations are represented in double backticks, unlike the other normal Kconfig options, because these options are not in the Kconfig search and hence don't link. Please see some examples here:
- https://docs.nordicsemi.com/bundle/ncs-latest/page/nrf/app_dev/config_and_build/sysbuild/zephyr_samples_sysbuild.html
- https://docs.nordicsemi.com/bundle/ncs-latest/page/nrf/releases_and_maturity/migration/migration_sysbuild.html#network_core
The earlier representation was correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is used for sysbuild Kconfigs too in zephyr, see https://github.com/zephyrproject-rtos/zephyr/blob/main/doc/releases/release-notes-4.2.rst?plain=1#L261
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@divipillai https://builds.zephyrproject.io/zephyr/pr/93861/docs/kconfig.html#!%5ESB_CONFIG_BOO so let's not continue with the wrong usage of backticks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we plan to implement the same for the Kconfig search in NCS as well in the future - https://docs.nordicsemi.com/bundle/ncs-latest/page/kconfig/index.html?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://ncsdoc.z6.web.core.windows.net/PR-23535/kconfig/index.html#!%5ESB_CONFIG_
feel free to review/approve:
nrfconnect/sdk-zephyr#3106
#23535
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @nordicjm. This was quick. The sysbuild Kconfig options now link as well. Tested a few doc pages and works fine. We will fix the syntax to ":kconfig:option:`SB" for other sysbuild config options in a separate PR. We already have a Jira open by Jan for nRF desktop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix here too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix as above
ab2763b to
8496243
Compare
|
@nordicjm, can we convert documentation mentions of sysbuild Kconfig options in a dedicated PR using the scheme you requested? Currently, @zycz changed a few sysbuild Kconfigs in the nRF Desktop documentation. However, docs still contain hundreds sysbuild Kconfig mentions formatted with the old syntax. It would be good to align this in one go. @zycz, could you create a ticket for that? |
|
We shouldn't be adding new wrong usage of options, the old ones exist because previously :kconfig:option: could only reference non-sysbuild Kconfigs and would error out, but that has been fixed |
|
8496243 to
6261716
Compare
6261716 to
cea2e2b
Compare
d6e92e5 to
da2ae4b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix this reference in a separate pr
da2ae4b to
ced0ca8
Compare
|
Since quarantine was modified, please make sure you are following the process described in Quarantine Process. |
Updated the configurations of the nRF Desktop application for board targets that store the MCUboot verification key in the KMU peripheral. These nRF54L-based targets now use the automatic provisioning feature that makes it possible to perform the KMU provisioning together with the west flash operation. The provisioning is only performed if the ``west flash`` command is executed with the ``--erase`` or ``--recover`` flag. Aligned the sample documentation with this change. JIRA: NCSDK-34240 Signed-off-by: Jan Zyczkowski <[email protected]>
Remove obsolete test_mcuboot_kmu.py used for KMU key provisioning. The pytest file `test_mcuboot_kmu.py`, previously used to provision KMU keys, has been removed. KMU key provisioning is now handled automatically when running the `west flash --recover` command. Additionally, the `sample.yaml` file was updated to reflect this change. JIRA: NCSDK-34240 Signed-off-by: Jan Zyczkowski <[email protected]>
ced0ca8 to
d6143a5
Compare
| The public key that MCUboot uses to validate the application image is securely stored in the hardware Key Management Unit (KMU). | ||
| In this use case, the application image is automatically signed by the |NCS| build system. | ||
| However, the public key is not automatically provisioned to the device when programming the bootloader and the application images using the ``west flash`` command. | ||
| For this purpose, a set of private and public keys is needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enabling hardware crypto does not require the bootloader to use a private/public key. The bootloader uses it regardless of whether HW crypto is used or not.
It may be improved in a follow-up PR. Merging this one at @zycz request
Updated the configurations of the nRF Desktop application for
board targets that store the MCUboot verification key in the KMU
peripheral. These nRF54L-based targets now use the automatic
provisioning feature that makes it possible to perform the KMU
provisioning together with the west flash operation. The provisioning
is only performed if the
west flashcommand is executed with the--eraseor--recoverflag.