nrf_security: Remove redefined Kconfigs #20107

nordicjm · 2025-01-29T09:09:45Z

Removes Kconfigs that are redefined because they are defined in an out of tree repo to prevent hiding problems if these symbols are renamed or removed

NordicBuilder · 2025-01-29T09:12:22Z

CI Information

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

Inputs:

Sources:

sdk-nrf: PR head: 05d05459bcfd05b8cd75a8603e88dcb948075116

more details

sdk-nrf:

PR head: 05d05459bcfd05b8cd75a8603e88dcb948075116
merge base: 5561499561f6b0a1a03790155ece7b7c65dce003
target head (main): 5561499561f6b0a1a03790155ece7b7c65dce003
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)

subsys
│  ├── nrf_security
│  │  ├── Kconfig
│  │  ├── src
│  │  │  ├── drivers
│  │  │  │  ├── Kconfig
│  │  │  │  ├── cracen
│  │  │  │  │  │ psa_driver.Kconfig

Outputs:

Toolchain

Version: 342151af73
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:342151af73_912848a074

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

◻️ Toolchain - Skipped: existing toolchain is used
✅ Build twister
- sdk-nrf test count: 1609
❌ Integration tests
- ❌ test-fw-nrfconnect-chip
- ✅ test-fw-nrfconnect-nrf_crypto
- ✅ test-fw-nrfconnect-tfm
- ❌ test-sdk-find-my
- ✅ test-sdk-sidewalk
- ✅ test-sdk-dfu
- ⚠️ test-fw-nrfconnect-nrf-iot_cloud

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-boot
- test-fw-nrfconnect-fem
- test-fw-nrfconnect-nfc
- test-fw-nrfconnect-nrf-iot_lwm2m
- test-fw-nrfconnect-nrf-iot_mosh
- 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-ps
- test-fw-nrfconnect-rpc
- test-fw-nrfconnect-rs
- test-fw-nrfconnect-thread
- test-fw-nrfconnect-zigbee
- test-low-level
- test-sdk-audio
- test-sdk-mcuboot
- test-sdk-pmic-samples
- test-sdk-wifi
- test-secdom-samples-public

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

tomi-font

Makes sense to me, pending green sdk-secdom CI.

NordicBuilder · 2025-01-29T09:26:08Z

You can find the documentation preview for this PR at this link.

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

Vge0rge · 2025-01-29T13:34:20Z

subsys/nrf_security/src/drivers/cracen/psa_driver.Kconfig

@@ -1852,12 +1852,14 @@ config PSA_NEED_CRACEN_KMU_ENCRYPTED_KEYS
 		   PSA_NEED_CRACEN_KEY_DERIVATION_DRIVER && \
 		   PSA_NEED_CRACEN_KMU_DRIVER

+config SUPPORT_PSA_NEED_CRACEN_PLATFORM_KEYS


This file has only configurations which has the PSA_NEED prefix, please keep the prefix as is and use another name.

psa need support? How does that make sense? This matches the global format we use in ncs:

./doc/nrf/app_dev/config_and_build/sysbuild/sysbuild_images.rst: config SUPPORT_NETCORE_ABC ./doc/nrf/app_dev/config_and_build/sysbuild/sysbuild_images.rst: config SUPPORT_NETCORE_ABC ./samples/cellular/http_update/modem_delta_update/Kconfig:config SUPPORTED_BASE_VERSION ./samples/wifi/radio_test/multi_domain/Kconfig.sysbuild:config SUPPORT_NETCORE_PERIPHERAL_RADIO_TEST ./subsys/nrf_security/src/drivers/cracen/psa_driver.Kconfig:config SUPPORT_PSA_NEED_CRACEN_PLATFORM_KEYS ./subsys/suit/mci/Kconfig:config SUPPORT_SUIT_MCI_IMPL_SDFW ./subsys/suit/storage/Kconfig:config SUPPORT_SUIT_STORAGE_LAYOUT_SOC ./subsys/suit/storage/Kconfig:config SUPPORT_SUIT_STORAGE_LAYOUT_TEST ./subsys/suit/validator/Kconfig:config SUPPORT_SUIT_VALIDATOR_IMPL_SDFW ./sysbuild/Kconfig.appcore:config SUPPORT_APPCORE ./sysbuild/Kconfig.appcore:config SUPPORT_APPCORE_EMPTY ./sysbuild/Kconfig.appcore:config SUPPORT_APPCORE_REMOTE_SHELL ./sysbuild/Kconfig.cracen:config SUPPORT_CRACEN ./sysbuild/Kconfig.netcore:config SUPPORT_NETCORE ./sysbuild/Kconfig.netcore:config SUPPORT_NETCORE_EMPTY ./sysbuild/Kconfig.netcore:config SUPPORT_NETCORE_HCI_IPC ./sysbuild/Kconfig.netcore:config SUPPORT_NETCORE_RPC_HOST ./sysbuild/Kconfig.netcore:config SUPPORT_NETCORE_802154_RPMSG ./sysbuild/Kconfig.netcore:config SUPPORT_NETCORE_IPC_RADIO ./sysbuild/Kconfig.pprcore:config SUPPORT_PPRCORE ./sysbuild/Kconfig.xip:config SUPPORT_QSPI_XIP ./tests/subsys/suit/tests/sdfw/Kconfig:config SUPPORT_SUIT_STORAGE_LAYOUT_TEST

ping @Vge0rge

Don't come up with a new naming convention in relation to Mbed TLS.

There is nothing in TF-M, Mbed TLS and/or Oberon-psa-crypto that has SUPPORT_

yeah, the issue here is that the PSA_NEED is already a naming convention and we have too many of them already. Since the Oberon PSA core doesn't use the SUPPORT naming convention I am hesitant to accept that for this file.

Don't come up with a new naming convention in relation to Mbed TLS.

There is nothing in TF-M, Mbed TLS and/or Oberon-psa-crypto that has SUPPORT_

This is not to do with mbedtls, this is to do with Kconfig symbol support in build system

One way to help solve some of these SOC-tie-ins is to create a HAS_HW_CRACEN configuration (as well as a HAS_HW_CRACEN_KMU for instance). This would only be available e.g. inside the H20 sec domain.

This is somewhat similar to how we signal that CC3XX is available:

sdk-nrf/subsys/nrf_security/src/drivers/Kconfig

Line 20 in c4c374f

depends on HAS_HW_NRF_CC3XX

HAS_HW_NRF_CC3XX

The HAS will work but it will not allow you to differentiate between L15 and H20. If your goal is to just have that as conditionally set what you can do is to create a Kconfig symbol here:
https://github.com/nrfconnect/sdk-nrf/blob/main/subsys/nrf_security/src/drivers/Kconfig

You can add a promptless:
config PSA_WANT_PLATFORM_KEYS

Then you can use this both enable the:
MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS
and the
PSA_NEED_CRACEN_PLATFORM_KEYS

If you need platform keys you need to have BOTH enabled.

This is not to do with mbedtls, this is to do with Kconfig symbol support in build system

There are 480+ entries in this file and they are all following the pattern config PSA_NEED_

You are also touching MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS which is an Mbed TLS configuration

SUPPORT_ doesn't make sense in the context

tests/subsys/suit/storage_nrf54h20/CMakeLists.txt

nordicjm · 2025-02-04T07:05:12Z

ping @Vge0rge @robertstypa @ahasztag @kszromek-nordic @tomchy for reviews, this is holding other teams up, would like to get it merged today

Removes Kconfigs that are redefined because they are defined in an out of tree repo to prevent hiding problems if these symbols are renamed or removed Signed-off-by: Jamie McCrae <[email protected]>

nordicjm requested review from a team as code owners January 29, 2025 09:09

github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Jan 29, 2025

tomi-font reviewed Jan 29, 2025

View reviewed changes

57300 approved these changes Jan 29, 2025

View reviewed changes

Vge0rge reviewed Jan 29, 2025

View reviewed changes

tomchy requested changes Jan 29, 2025

View reviewed changes

tests/subsys/suit/storage_nrf54h20/CMakeLists.txt Outdated Show resolved Hide resolved

tomchy requested review from robertstypa, ahasztag and kszromek-nordic January 29, 2025 15:29

nordicjm requested a review from tomchy January 30, 2025 13:40

nordicjm force-pushed the fixcpusecleak branch from 962b847 to 5ef0426 Compare January 31, 2025 13:11

tomchy approved these changes Feb 4, 2025

View reviewed changes

carlescufi requested review from Vge0rge and tomi-font February 4, 2025 09:30

frkv self-requested a review February 4, 2025 09:37

nrf_security: Remove redefined Kconfigs

05d0545

Removes Kconfigs that are redefined because they are defined in an out of tree repo to prevent hiding problems if these symbols are renamed or removed Signed-off-by: Jamie McCrae <[email protected]>

nordicjm force-pushed the fixcpusecleak branch from 5ef0426 to 05d0545 Compare February 4, 2025 13:11

Vge0rge approved these changes Feb 4, 2025

View reviewed changes

frkv approved these changes Feb 4, 2025

View reviewed changes

carlescufi merged commit e77774f into nrfconnect:main Feb 4, 2025
11 of 13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nrf_security: Remove redefined Kconfigs #20107

nrf_security: Remove redefined Kconfigs #20107

nordicjm commented Jan 29, 2025

NordicBuilder commented Jan 29, 2025 •

edited

Loading

tomi-font left a comment

NordicBuilder commented Jan 29, 2025

Vge0rge Jan 29, 2025

nordicjm Jan 29, 2025

nordicjm Feb 3, 2025

frkv Feb 4, 2025 •

edited

Loading

Vge0rge Feb 4, 2025

nordicjm Feb 4, 2025

frkv Feb 4, 2025

Vge0rge Feb 4, 2025

frkv Feb 4, 2025

nordicjm commented Feb 4, 2025

nrf_security: Remove redefined Kconfigs #20107

nrf_security: Remove redefined Kconfigs #20107

Conversation

nordicjm commented Jan 29, 2025

NordicBuilder commented Jan 29, 2025 • edited Loading

CI Information

Inputs:

Sources:

Github labels

Outputs:

Toolchain

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

tomi-font left a comment

Choose a reason for hiding this comment

NordicBuilder commented Jan 29, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frkv Feb 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nordicjm commented Feb 4, 2025

NordicBuilder commented Jan 29, 2025 •

edited

Loading

frkv Feb 4, 2025 •

edited

Loading