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

childimage/NSIB: Remove non-functional environmental key support #17088

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nvlsianpu
Copy link
Contributor

@nvlsianpu nvlsianpu commented Aug 29, 2024

Removes non-workable possibility for configuring signing key using
either environmental or command-line variable (SB_SIGNING_KEY_FILE).
This has never worked.

ref.: NCSDK-28124

similar change was done for the sysbuild (#16154)

@github-actions github-actions bot added the doc-required PR must not be merged without tech writer approval. label Aug 29, 2024
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Aug 29, 2024

CI Information

To view the history of this post, clich the 'edited' button above
Build number: 4

Inputs:

Sources:

sdk-nrf: PR head: 13f07f07ca0c6473537c6c5964e458e5a9b3404a

more details

sdk-nrf:

PR head: 13f07f07ca0c6473537c6c5964e458e5a9b3404a
merge base: 2797c9dd88d99fac3c45a5ef4c7fc1670c9d1f59
target head (main): dea1c7f59a4557273dbc1dd8729e22fbcb7c6ddc
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 (3)
doc
│  ├── nrf
│  │  ├── releases_and_maturity
│  │  │  ├── releases
│  │  │  │  │ release-notes-changelog.rst
subsys
│  ├── bootloader
│  │  ├── Kconfig
│  │  ├── cmake
│  │  │  │ debug_keys.cmake

Outputs:

Toolchain

Version: 2aae60c2f9
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:2aae60c2f9_81ed5a52d6

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

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

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

@NordicBuilder
Copy link
Contributor

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

@nvlsianpu
Copy link
Contributor Author

CI passed after re-run.

@@ -45,6 +45,12 @@ Build and configuration system
.. note::
This has security implications and may allow secrets to be leaked to the non-secure application in RAM.

* Removed non-working support for setting the NSIB signing key using either environmental or command line variable ``SB_SIGNING_KEY_FILE`` along with child_image.
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
* Removed non-working support for setting the NSIB signing key using either environmental or command line variable ``SB_SIGNING_KEY_FILE`` along with child_image.
* Removed the non-working support for configuring the NSIB signing key through the environmental or command line variable (``SB_SIGNING_KEY_FILE``), along with child image.

Comment on lines 50 to 51
..note::
This never been working. Use any Kconfig method for configuring the signing key.
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
..note::
This never been working. Use any Kconfig method for configuring the signing key.
..note::
This feature has never been functional.
To configure the signing key, use any available Kconfig method.

@nvlsianpu nvlsianpu force-pushed the drop_env_signing_key_childimage branch from 2f238ba to 1a3a284 Compare September 12, 2024 15:05
Removes non-workable possibility for configuring signing key using
either environmental or command-line variable (SB_SIGNING_KEY_FILE).
This has never worked.

ref.: NCSDK-28124

Signed-off-by: Andrzej Puzdrowski <[email protected]>
@nvlsianpu nvlsianpu force-pushed the drop_env_signing_key_childimage branch from 1a3a284 to d52dc07 Compare September 13, 2024 08:38
Added note on removal of the configuration method.

Signed-off-by: Andrzej Puzdrowski <[email protected]>
@nvlsianpu nvlsianpu force-pushed the drop_env_signing_key_childimage branch from d52dc07 to 13f07f0 Compare September 13, 2024 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-required PR must not be merged without tech writer approval.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants