nrf_security: Add key policy for the Cracen builtin keys #25458

Vge0rge · 2025-11-06T12:21:27Z

Add logic which allows to specifically define who can use the builtin
keys (IKG and KMU one) for Cracen when used with TF-M.

This includes a list of specifically allowed users for the builtin keys
and any other access is denied.

This is only relevant for a TF-M enabled application since that is currently
the only way to encode ownership in the key attributes.

Ref: NCSDK-35736

Copilot

Pull Request Overview

This PR adds access control policies for Cracen builtin keys (IKG and KMU) when used with TF-M, restricting key access to specifically allowed users based on owner ID encoded in key attributes.

Key changes:

Implements a policy framework with user-to-key-slot mappings for both IKG and KMU builtin keys
Adds permission checks across all key management operations to enforce the policy
Provides no-op implementation when TF-M is not enabled

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
cracen_builtin_key_policy.c	New file implementing policy enforcement logic with allowlists for IKG and KMU keys
cracen_psa_builtin_key_policy.h	New header defining policy data structures and the permission check API
key_management.c	Adds permission checks to all key management functions
cracen_psa.h	Removes unused TF-M header include
cracenpsa.cmake	Adds new policy source file to build (duplicated entry)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

subsys/nrf_security/src/drivers/cracen/cracenpsa/cracenpsa.cmake

subsys/nrf_security/src/drivers/cracen/cracenpsa/src/cracen_builtin_key_policy.c

NordicBuilder · 2025-11-06T12:28:38Z

CI Information

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

Inputs:

Sources:

sdk-nrf: PR head: 59a9b82de74b9aeb8922a35cf2b5ace49b76ac25

more details

sdk-nrf:

PR head: 59a9b82de74b9aeb8922a35cf2b5ace49b76ac25
merge base: 4a5e29492e78ad8ae5da7e0ad26ec59db2afa25d
target head (main): 9e14b6e4b8036bb723223fb91d1454811bfc1ef4
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 (5)

subsys
│  ├── nrf_security
│  │  ├── src
│  │  │  ├── drivers
│  │  │  │  ├── cracen
│  │  │  │  │  ├── cracenpsa
│  │  │  │  │  │  ├── cracenpsa.cmake
│  │  │  │  │  │  ├── include
│  │  │  │  │  │  │  ├── cracen_psa.h
│  │  │  │  │  │  │  │ cracen_psa_builtin_key_policy.h
│  │  │  │  │  │  ├── src
│  │  │  │  │  │  │  ├── cracen_builtin_key_policy.c
│  │  │  │  │  │  │  │ key_management.c

Outputs:

Toolchain

Version: df3cc9d822
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:df3cc9d822_e595b21c39

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

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

Disabled integration tests

- test-fw-nrfconnect-nrf_lrcs_positioning
- desktop52_verification
- 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_libmodem-nrf
- test-fw-nrfconnect-nrf-iot_lwm2m
- test-fw-nrfconnect-nrf-iot_samples
- test-fw-nrfconnect-nrf-iot_thingy91
- test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
- test-fw-nrfconnect-ps-main
- test-fw-nrfconnect-rpc
- test-fw-nrfconnect-thread-main
- test-low-level
- test-sdk-audio
- test-sdk-wifi
- test-secdom-samples-public

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

Copilot

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Add logic which allows to specifically define who can use the builtin keys (IKG and KMU one) for Cracen when used with TF-M. This includes a list of specifically allowed users for the builtin keys and any other access is denied. This is only relevant for a TF-M enabled application since that is currently the only way to encode ownership in the key attributes. Ref: NCSDK-35736 Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>

Copilot

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-06T14:22:14Z

subsys/nrf_security/src/drivers/cracen/cracenpsa/src/cracen_builtin_key_policy.c

+	    PSA_KEY_LOCATION_CRACEN) {
+
+		slot_id = MBEDTLS_SVC_KEY_ID_GET_KEY_ID(psa_get_key_id(attributes));
+		if (!cracen_builtin_ikg_user_allowed(owner, slot_id, attributes)) {


The logic here always returns false if the IKG check fails, but should return true if the check passes to allow the key operation. The early return on line 117 prevents the function from returning true when access is allowed for IKG keys. This should be: if (cracen_builtin_ikg_user_allowed(owner, slot_id, attributes)) { return true; }

Suggested change

if (!cracen_builtin_ikg_user_allowed(owner, slot_id, attributes)) {

if (cracen_builtin_ikg_user_allowed(owner, slot_id, attributes)) {

return true;

} else {

Copilot · 2025-11-06T14:22:14Z

subsys/nrf_security/src/drivers/cracen/cracenpsa/src/cracen_builtin_key_policy.c

+		slot_id = CRACEN_PSA_GET_KMU_SLOT(
+			MBEDTLS_SVC_KEY_ID_GET_KEY_ID(psa_get_key_id(attributes)));
+		if (!cracen_builtin_kmu_user_allowed(owner, slot_id, attributes)) {
+			return false;
+		}


The logic here always returns false if the KMU check fails, but should return true if the check passes. The early return on line 128 prevents the function from returning true when access is allowed for KMU keys. This should be: if (cracen_builtin_kmu_user_allowed(owner, slot_id, attributes)) { return true; }

The tfm_builtin_key_loader.h defines the symbol: TFM_BUILTIN_KEY_LOADER_KEY_LOCATION which is also defined in the tfm_builtin_key_ids.h when Cracen is enabled. To avoid this remove including the default key loader and only keep the Cracen one. Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>

tomi-font · 2025-11-07T10:35:13Z

subsys/nrf_security/src/drivers/cracen/cracenpsa/src/cracen_builtin_key_policy.c

+#include "psa_manifest/pid.h"
+#include "tfm_builtin_key_ids.h"


use angle brackets

tomi-font · 2025-11-07T10:38:49Z

subsys/nrf_security/src/drivers/cracen/cracenpsa/include/cracen_psa_builtin_key_policy.h

+#ifndef CRACEN_BUILTIN_KEY_POLICY_H
+#define CRACEN_BUILTIN_KEY_POLICY_H


header guard mismatch

tomi-font · 2025-11-07T10:39:05Z

subsys/nrf_security/src/drivers/cracen/cracenpsa/src/cracen_builtin_key_policy.c

Don't call this source file cracen to follow convention in this directory?

tomi-font · 2025-11-07T10:41:10Z

subsys/nrf_security/src/drivers/cracen/cracenpsa/include/cracen_psa_builtin_key_policy.h

+#ifndef CRACEN_BUILTIN_KEY_POLICY_H
+#define CRACEN_BUILTIN_KEY_POLICY_H
+
+#include "common.h"


Suggested change

#include "common.h"

#include <common.h>

tomi-font · 2025-11-07T10:46:38Z

subsys/nrf_security/src/drivers/cracen/cracenpsa/include/cracen_psa_builtin_key_policy.h

+#ifndef CRACEN_BUILTIN_KEY_POLICY_H
+#define CRACEN_BUILTIN_KEY_POLICY_H
+
+#include "common.h"


What is this even including?

tomi-font · 2025-11-07T10:55:02Z

subsys/nrf_security/src/drivers/cracen/cracenpsa/include/cracen_psa_builtin_key_policy.h

+	mbedtls_key_owner_id_t user;
+	psa_drv_slot_number_t key_slot_start;
+	psa_drv_slot_number_t key_slot_end;
+	cracen_kmu_entry_type_t kmu_entry_type;


It seems to me this field is not actually useful and we could just do with key_slot_start and key_slot_end alone? For the KMU_ENTRY_SLOT_SINGLE case you would just have key_slot_start == key_slot_end.

tomi-font · 2025-11-07T10:58:07Z

subsys/nrf_security/src/drivers/cracen/cracenpsa/src/cracen_builtin_key_policy.c

+	mbedtls_key_owner_id_t owner = MBEDTLS_SVC_KEY_ID_GET_OWNER_ID(psa_get_key_id(attributes));
+	psa_drv_slot_number_t slot_id;


We could also have a variable which holds MBEDTLS_SVC_KEY_ID_GET_KEY_ID(psa_get_key_id(attributes)) to avoid repetition.

tomi-font · 2025-11-07T10:58:34Z

subsys/nrf_security/src/drivers/cracen/cracenpsa/src/cracen_builtin_key_policy.c

+	mbedtls_key_owner_id_t owner = MBEDTLS_SVC_KEY_ID_GET_OWNER_ID(psa_get_key_id(attributes));
+	psa_drv_slot_number_t slot_id;
+
+	if (PSA_KEY_LIFETIME_GET_LOCATION(psa_get_key_lifetime(attributes)) ==


Use a switch instead?

tomi-font · 2025-11-07T10:59:32Z

subsys/nrf_security/src/drivers/cracen/cracenpsa/src/cracen_builtin_key_policy.c

+		if (!cracen_builtin_ikg_user_allowed(owner, slot_id, attributes)) {
+			return false;
+		}


How about just

Suggested change

if (!cracen_builtin_ikg_user_allowed(owner, slot_id, attributes)) {

return false;

}

return cracen_builtin_ikg_user_allowed(owner, slot_id, attributes)

?

tomi-font · 2025-11-07T11:01:15Z

subsys/nrf_security/src/drivers/cracen/cracenpsa/src/cracen_builtin_key_policy.c

+		if (g_builtin_ikg_policy[i].user == owner &&
+		    g_builtin_ikg_policy[i].key_slot == slot_number) {


Terminology is not aligned (user/owner, key_slot/slot_number). Maybe align?

Copilot AI review requested due to automatic review settings November 6, 2025 12:21

Vge0rge requested review from a team as code owners November 6, 2025 12:21

NordicBuilder added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Nov 6, 2025

Copilot AI reviewed Nov 6, 2025

View reviewed changes

Copilot AI review requested due to automatic review settings November 6, 2025 14:19

Copilot AI reviewed Nov 6, 2025

View reviewed changes

Copilot AI review requested due to automatic review settings November 6, 2025 14:21

Copilot AI reviewed Nov 6, 2025

View reviewed changes

Vge0rge force-pushed the protect_builtin_keys branch from 1849194 to 59a9b82 Compare November 6, 2025 14:23

nordicjm approved these changes Nov 7, 2025

View reviewed changes

tomi-font reviewed Nov 7, 2025

View reviewed changes

-		if (!cracen_builtin_ikg_user_allowed(owner, slot_id, attributes)) {
+		if (cracen_builtin_ikg_user_allowed(owner, slot_id, attributes)) {
+			return true;
+		} else {

		#include "psa_manifest/pid.h"
		#include "tfm_builtin_key_ids.h"

		#ifndef CRACEN_BUILTIN_KEY_POLICY_H
		#define CRACEN_BUILTIN_KEY_POLICY_H

		mbedtls_key_owner_id_t owner = MBEDTLS_SVC_KEY_ID_GET_OWNER_ID(psa_get_key_id(attributes));
		psa_drv_slot_number_t slot_id;

		if (g_builtin_ikg_policy[i].user == owner &&
		g_builtin_ikg_policy[i].key_slot == slot_number) {

nrf_security: Add key policy for the Cracen builtin keys #25458

Are you sure you want to change the base?

nrf_security: Add key policy for the Cracen builtin keys #25458

Conversation

Vge0rge commented Nov 6, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NordicBuilder commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI Information

Inputs:

Sources:

Github labels

Outputs:

Toolchain

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

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

NordicBuilder commented Nov 6, 2025 •

edited

Loading