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

nrf_security: Add key revocation for SICR keys #19710

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

Conversation

Vge0rge
Copy link
Contributor

@Vge0rge Vge0rge commented Dec 26, 2024

Adds key revocation for the platform keys stored in SICR for nRF54H20.

@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 Dec 26, 2024
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Dec 26, 2024

CI Information

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

Inputs:

Sources:

sdk-nrf: PR head: dcd079a863c4adce3246d3ec5f822c634f0bc412

more details

sdk-nrf:

PR head: dcd079a863c4adce3246d3ec5f822c634f0bc412
merge base: cac49ac985664e9357e1c30691ce1091470c528b
target head (main): c361c8ac4ab828a90edb02a59cd982de05f4d818
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
│  │  ├── src
│  │  │  ├── drivers
│  │  │  │  ├── cracen
│  │  │  │  │  ├── cracenpsa
│  │  │  │  │  │  ├── src
│  │  │  │  │  │  │  ├── key_management.c
│  │  │  │  │  │  │  ├── platform_keys
│  │  │  │  │  │  │  │  ├── platform_keys.c
│  │  │  │  │  │  │  │  │ platform_keys.h

Outputs:

Toolchain

Version: 11349092be
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:11349092be_912848a074

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

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister
    • sdk-nrf test count: 1642
  • ❌ 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_libmodem-nrf
    • 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-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

@Vge0rge Vge0rge force-pushed the platform_key_revocation branch from 4fdb848 to caca435 Compare December 30, 2024 15:32
@Vge0rge Vge0rge marked this pull request as ready for review December 30, 2024 15:32
@Vge0rge Vge0rge requested a review from a team as a code owner December 30, 2024 15:32

nrf_mramc_config_set(mramc, &mramc_config_write_enabled);

memcpy(key.sicr.attr_addr, &sicr_attr, sizeof(sicr_attr));
Copy link
Contributor

Choose a reason for hiding this comment

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

Ahem key is a pointer but you use it as if it were not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good catch, thanks! I will update.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated and opened a PR in secdom to run it's CI: 1051

Copy link
Contributor

Choose a reason for hiding this comment

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

I see CI failures, I think you need to make CI green first before I review?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that you need to wait no, the CI failures seem to build some nrfx clock build failures. These should not be relevant to this PR at all.

@Vge0rge Vge0rge force-pushed the platform_key_revocation branch from caca435 to 4ed1318 Compare January 2, 2025 10:56
This refactors how SICR keys are writen into MRAM. This refactors the
code but it should not change any funcionality. The purpose of this
is to make the MRAM writing part reusable so it can be used by the
revocation functionality later.

Signed-off-by: Georgios Vasilakis <[email protected]>
@Vge0rge Vge0rge force-pushed the platform_key_revocation branch from 4ed1318 to 8008df6 Compare January 6, 2025 15:33
@Vge0rge Vge0rge force-pushed the platform_key_revocation branch from 8008df6 to 14c4ee8 Compare January 8, 2025 11:53
Adds support of key revocation using the psa_destroy_key API.
The value 0xfa50 is used in the key type in order to mark an
revoked key.

The return code PSA_ERROR_NOT_PERMITTED is returned for
revoked keys for all the functions in the PSA crypto driver
wrapper. This error code seems OK since it mentions
platform specific policies for not permitted an operation.

Ref: NCSDK-30076

Signed-off-by: Georgios Vasilakis <[email protected]>
@Vge0rge Vge0rge force-pushed the platform_key_revocation branch from 14c4ee8 to dcd079a Compare January 14, 2025 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants