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

secure_storage: fix tests #20710

Merged
merged 8 commits into from
Mar 24, 2025
Merged

Conversation

tomi-font
Copy link
Contributor

No description provided.

@tomi-font tomi-font requested review from a team as code owners March 3, 2025 15:37
@github-actions github-actions bot added manifest changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Mar 3, 2025
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Mar 3, 2025

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

Name Old Revision New Revision Diff
mbedtls nrfconnect/sdk-mbedtls@v3.6.2-ncs2 nrfconnect/sdk-mbedtls@f109c9b (main) nrfconnect/sdk-mbedtls@v3.6.2-ncs2..f109c9ba
zephyr nrfconnect/sdk-zephyr@c9113a8 nrfconnect/sdk-zephyr@bd1cf27 (main) nrfconnect/sdk-zephyr@c9113a87..bd1cf27b

All manifest checks OK

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

@NordicBuilder
Copy link
Contributor

NordicBuilder commented Mar 3, 2025

CI Information

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

Inputs:

Sources:

sdk-nrf: PR head: 14b68f51d52d41b518da1f7b178065c0a7069e25
mbedtls: PR head: f109c9bac0bdb9699854e88a9c14772cbbdffb4f
zephyr: PR head: bd1cf27b22f1003d58ecf4af81961722e1eb9949

more details

sdk-nrf:

PR head: 14b68f51d52d41b518da1f7b178065c0a7069e25
merge base: 4bd5e19d4f0d5c24b5e58e5ddb99c328f37bd490
target head (main): 648c06e61f46d5f2f56ff94db222762c663fd519
Diff

mbedtls:

PR head: f109c9bac0bdb9699854e88a9c14772cbbdffb4f
merge base: 98603a8c91660beac00e0ee1d76198fb7c4ed29b
Diff

zephyr:

PR head: bd1cf27b22f1003d58ecf4af81961722e1eb9949
merge base: c9113a87822e123bd287568c957d92b35502e5d1
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 (52)
modules
│  ├── crypto
│  │  ├── mbedtls
│  │  │  ├── include
│  │  │  │  ├── mbedtls
│  │  │  │  │  ├── config_adjust_legacy_crypto.h
│  │  │  │  │  │ md.h
│  │  │  ├── tests
│  │  │  │  ├── suites
│  │  │  │  │  │ test_suite_md.function
│  ├── trusted-firmware-m
│  │  │ Kconfig
scripts
│  │ quarantine_zephyr.yaml
subsys
│  ├── nrf_security
│  │  ├── Kconfig.psa.nordic
│  │  ├── src
│  │  │  ├── core
│  │  │  │  ├── nrf_oberon
│  │  │  │  │  │ CMakeLists.txt
│  │  │  ├── drivers
│  │  │  │  ├── cracen
│  │  │  │  │  ├── cracenpsa
│  │  │  │  │  │  ├── src
│  │  │  │  │  │  │  │ aead.c
west.yml
zephyr
│  ├── boards
│  │  ├── nordic
│  │  │  ├── nrf54l09pdk
│  │  │  │  │ nrf54l09pdk_nrf54l09-common.dtsi
│  ├── cmake
│  │  ├── modules
│  │  │  │ extensions.cmake
│  ├── doc
│  │  ├── connectivity
│  │  │  ├── bluetooth
│  │  │  │  ├── api
│  │  │  │  │  │ gatt.rst
│  │  ├── releases
│  │  │  │ release-notes-4.2.rst
│  ├── drivers
│  │  ├── bluetooth
│  │  │  ├── hci
│  │  │  │  ├── hci_nxp.c
│  │  │  │  │ ipc.c
│  ├── include
│  │  ├── zephyr
│  │  │  ├── bluetooth
│  │  │  │  ├── bluetooth.h
│  │  │  │  ├── gatt.h
│  │  │  │  │ hci_types.h
│  ├── modules
│  │  ├── mbedtls
│  │  │  ├── Kconfig.tls-generic
│  │  │  ├── configs
│  │  │  │  │ config-tls-generic.h
│  ├── samples
│  │  ├── psa
│  │  │  ├── its
│  │  │  │  ├── overlay-secure_storage.conf
│  │  │  │  ├── overlay-tfm.conf
│  │  │  │  ├── prj.conf
│  │  │  │  │ sample.yaml
│  │  │  ├── persistent_key
│  │  │  │  ├── overlay-secure_storage.conf
│  │  │  │  ├── overlay-tfm.conf
│  │  │  │  ├── prj.conf
│  │  │  │  │ sample.yaml
│  ├── subsys
│  │  ├── bluetooth
│  │  │  ├── Kconfig
│  │  │  ├── Kconfig.adv
│  │  │  ├── audio
│  │  │  │  │ bap_broadcast_assistant.c
│  │  │  ├── host
│  │  │  │  ├── adv.c
│  │  │  │  ├── att.c
│  │  │  │  ├── ecc.c
│  │  │  │  ├── gatt.c
│  │  │  │  ├── hci_core.c
│  │  │  │  ├── hci_core.h
│  │  │  │  │ smp.c
│  │  ├── secure_storage
│  │  │  │ Kconfig
│  ├── tests
│  │  ├── bsim
│  │  │  ├── bluetooth
│  │  │  │  ├── host
│  │  │  │  │  ├── gatt
│  │  │  │  │  │  ├── ccc_store
│  │  │  │  │  │  │  ├── src
│  │  │  │  │  │  │  │  │ peripheral.c
│  │  │  │  │  │  ├── general
│  │  │  │  │  │  │  ├── src
│  │  │  │  │  │  │  │  │ gatt_client_test.c
│  │  │  │  │  ├── security
│  │  │  │  │  │  ├── ccc_update
│  │  │  │  │  │  │  ├── src
│  │  │  │  │  │  │  │  │ peripheral.c
│  │  ├── crypto
│  │  │  ├── secp256r1
│  │  │  │  │ mbedtls.conf
│  │  ├── subsys
│  │  │  ├── secure_storage
│  │  │  │  ├── psa
│  │  │  │  │  ├── crypto
│  │  │  │  │  │  ├── overlay-secure_storage.conf
│  │  │  │  │  │  ├── overlay-tfm.conf
│  │  │  │  │  │  ├── prj.conf
│  │  │  │  │  │  ├── src
│  │  │  │  │  │  │  │ main.c
│  │  │  │  │  │  │ testcase.yaml
│  │  │  │  │  ├── its
│  │  │  │  │  │  ├── overlay-secure_storage.conf
│  │  │  │  │  │  ├── overlay-tfm.conf
│  │  │  │  │  │  ├── overlay-transform_default.conf
│  │  │  │  │  │  │ prj.conf

Outputs:

Toolchain

Version: 4ffa2202d5
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:4ffa2202d5_e579f9fbe6

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

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister - Skipped: Skipping Build & Test as it succeeded in a previous run: 19
  • ✅ Integration tests
    • ✅ test-sdk-audio - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ desktop52_verification - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-boot - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-ble_samples - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-chip - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nfc - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nrf-iot_cloud - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nrf-iot_thingy91 - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-nrf_crypto - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-rs - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-fem - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-tfm - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-thread - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-sdk-find-my - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-low-level - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-sdk-mcuboot - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-sdk-dfu - Skipped: Job was skipped as it succeeded in a previous run
    • ⚠️ test-fw-nrfconnect-fw-update
    • ⚠️ test_ble_nrf_config
Disabled integration tests
    • doc-internal
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • 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_zephyr_lwm2m
    • test-fw-nrfconnect-proprietary_esb
    • test-fw-nrfconnect-ps
    • test-fw-nrfconnect-rpc
    • test-fw-nrfconnect-zigbee
    • test-sdk-pmic-samples
    • test-sdk-wifi
    • test-secdom-samples-public

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

Copy link

github-actions bot commented Mar 3, 2025

You can find the documentation preview for this PR here.

@tomi-font tomi-font force-pushed the secure_storage_fix_tests branch 2 times, most recently from aece2e9 to 85fc86d Compare March 4, 2025 08:00
Copy link
Contributor

@katgiadla katgiadla left a comment

Choose a reason for hiding this comment

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

works on-device. LGTM

@tomi-font tomi-font force-pushed the secure_storage_fix_tests branch from 48e20b3 to 15afba8 Compare March 17, 2025 14:11
Copy link

Since quarantine was modified, please make sure you are following the process described in Quarantine Process.

@tomi-font tomi-font force-pushed the secure_storage_fix_tests branch from 15afba8 to 03acad4 Compare March 17, 2025 14:14
@tomi-font
Copy link
Contributor Author

Rebased and updated the Zephyr/Mbed TLS PRs.

@tomi-font tomi-font force-pushed the secure_storage_fix_tests branch 2 times, most recently from 8deea05 to a0da5f9 Compare March 18, 2025 08:43
@tomi-font
Copy link
Contributor Author

@nrfconnect/ncs-test-leads @nrfconnect/ncs-aegir please review (CI failure unrelated)

@nordic-piks
Copy link
Contributor

@nrfconnect/ncs-test-leads @nrfconnect/ncs-aegir please review (CI failure unrelated)

Indeed, find-my CI failure is from different reason (will be fixed soon)

@tomi-font tomi-font force-pushed the secure_storage_fix_tests branch from a0da5f9 to 4545f9b Compare March 20, 2025 11:03
@tomi-font tomi-font force-pushed the secure_storage_fix_tests branch from 4545f9b to 0a62db2 Compare March 20, 2025 11:09
Copy link
Contributor

@frkv frkv left a comment

Choose a reason for hiding this comment

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

This and dependent PRs LGTM (pending updated shas)

@tomi-font
Copy link
Contributor Author

@nrfconnect/ncs-test-leads please review

1 similar comment
@tomi-font
Copy link
Contributor Author

@nrfconnect/ncs-test-leads please review

Lower the minimum value allowed from 512 to 256 as the secure_storage
tests use 256 and this is still quite a reasonable minimum value
to have.

Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
@tomi-font tomi-font force-pushed the secure_storage_fix_tests branch from 0a62db2 to 5d59c98 Compare March 24, 2025 07:52
@tomi-font tomi-font removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Mar 24, 2025
@NordicBuilder NordicBuilder removed the DNM label Mar 24, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Pull in changes made in Zephyr to fix secure storage tests.

Update the sdk-zephyr revision to the latest main to also
pull in a fix to a Zephyr test that would make CI fail.

Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
Pull in the PR that was originally created to
fix the block_cipher configuration in Mbed TLS.

The fix ended up being done in sdk-nrf so the
Mbed TLS PR turned into a cleanup of some
noups we have in there, without intended
functional change.

Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
Remove the quarantine as thoses tests are now fixed.

Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
psa_aead_encrypt() would fail, returning PSA_ERROR_BUFFER_TOO_SMALL,
because ciphertext_length wouldn't be assigned and have a random value.

Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
Define BUILDING_MBEDTLS_CRYPTO when building source files of the Oberon
PSA Crypto core.
This is done to be aligned with Mbed TLS in Zephyr.

BUILDING_MBEDTLS_CRYPTO is used by the secure storage subsystem
to identify that the ITS calls are coming from PSA Crypto.
So it needs to be defined in all the PSA Crypto implementations.

Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
With the latest changes the remaining test scenario that used to not be
filtered out is now overflowing.
This is not something we want to support anyway so quarantine all
secure_storage tests on that board target.

Also, replace the other quarantined secure_storage scenarios by the same
regexp to make sure to encompass them all.

Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
Auto-enable PSA_WANT_ALG_ECB_NO_PADDING when MBEDTLS_BUILTIN and CCM or
GCM is enabled and !MBEDTLS_FORCE_LEGACY_CIPHER.
ECB is needed by the block_cipher module in Mbed TLS when
MBEDTLS_BLOCK_CIPHER_SOME_PSA.
Because of a noup we have to enable that in our Mbed TLS fork, ECB ends
up needing to be enabled when using Mbed TLS' PSA API to encrypt/decrypt
data with CCM/GCM because the code goes to block_cipher.
Rather than having to enable that dependency everywhere it's needed,
auto-enable it in those conditions.

Also, move the PSA_WANT_ALG Kconfig options that were separate from
the others.

Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
@tomi-font tomi-font force-pushed the secure_storage_fix_tests branch from 5d59c98 to 14b68f5 Compare March 24, 2025 09:15
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Mar 24, 2025
@tomi-font tomi-font removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Mar 24, 2025
@nordicjm nordicjm merged commit c33743c into nrfconnect:main Mar 24, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants