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.

Sign up for GitHub

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

Jump to bottom

tf-m: Add Attestation support for nRF54L15 #19040

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+69 −6
Open

tf-m: Add Attestation support for nRF54L15 #19040

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion modules/trusted-firmware-m/Kconfig.tfm.defconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ config TFM_PARTITION_INITIAL_ATTESTATION
select PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_EXPORT
select PSA_WANT_ALG_ECDSA
select PSA_WANT_ECC_SECP_R1_256
select SECURE_BOOT_STORAGE
select SECURE_BOOT_STORAGE if TRUSTED_EXECUTION_SECURE

config TFM_PARTITION_PROTECTED_STORAGE
bool
Expand Down
2 changes: 1 addition & 1 deletion modules/trusted-firmware-m/tfm_boards/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ if (${TFM_PARTITION_CRYPTO})
tfm_sprt
)

if (${TFM_PARTITION_INITIAL_ATTESTATION})
if ((${TFM_PARTITION_INITIAL_ATTESTATION}) AND CONFIG_IDENTITY_KEY)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will not work for other nRF SoCs.
Would it be better to do "if 5340 or 9160 or etc etc"?
Or to define an TFM_IDENTITY_KEY and then set that inside tfm_boards for each of the boards that use this?

Copy link
Contributor

Choose a reason for hiding this comment

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

To me it makes sense to use a more generic Kconfig option rather than explicitly naming the affected SOCs. The list will likely change over time, so it's nice to not have a bunch of if SOC_X [...] all over.

target_sources(platform_s
PRIVATE
${ZEPHYR_NRF_MODULE_DIR}/lib/identity_key/identity_key.c
Expand Down
39 changes: 37 additions & 2 deletions modules/trusted-firmware-m/tfm_boards/common/attest_hal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,29 @@

#include <stddef.h>
#include <stdint.h>
#include <psa/error.h>
#include <psa/crypto.h>
#include "tfm_attest_hal.h"
#include "tfm_plat_boot_seed.h"
#include "tfm_plat_device_id.h"
#include "tfm_plat_otp.h"
#include <nrf_cc3xx_platform.h>
#include "tfm_strnlen.h"
#include "nrf_provisioning.h"
#include <nrfx_nvmc.h>
#include <bl_storage.h>

#ifdef CONFIG_NRFX_NVMC
#include <nrfx_nvmc.h>
#endif
#ifdef CONFIG_HAS_HW_NRF_CC3XX
#include <nrf_cc3xx_platform.h>
#endif

#if defined(CONFIG_CRACEN_HW_PRESENT)
static bool boot_seed_set = false;
static uint8_t boot_seed[BOOT_SEED_SIZE];
#endif


static enum tfm_security_lifecycle_t map_bl_storage_lcs_to_tfm_slc(enum lcs lcs)
{
switch (lcs) {
Expand Down Expand Up @@ -122,8 +135,11 @@ enum tfm_plat_err_t tfm_attest_hal_get_profile_definition(uint32_t *size, uint8_

enum tfm_plat_err_t tfm_plat_get_boot_seed(uint32_t size, uint8_t *buf)
{
#if defined(CONFIG_HAS_HW_NRF_CC3XX)
int nrf_err;

_Static_assert(NRF_CC3XX_PLATFORM_TFM_BOOT_SEED_SIZE == BOOT_SEED_SIZE,
"NRF_CC3XX_PLATFORM_TFM_BOOT_SEED_SIZE must match BOOT_SEED_SIZE");
if (size != NRF_CC3XX_PLATFORM_TFM_BOOT_SEED_SIZE) {
return TFM_PLAT_ERR_INVALID_INPUT;
}
Expand All @@ -132,7 +148,26 @@ enum tfm_plat_err_t tfm_plat_get_boot_seed(uint32_t size, uint8_t *buf)
if (nrf_err != NRF_CC3XX_PLATFORM_SUCCESS) {
return TFM_PLAT_ERR_SYSTEM_ERR;
}
#elif defined(CONFIG_CRACEN_HW_PRESENT)
psa_status_t psa_err = PSA_SUCCESS;

if (size != BOOT_SEED_SIZE) {
return TFM_PLAT_ERR_INVALID_INPUT;
}

if (!boot_seed_set) {
psa_err = psa_generate_random(boot_seed, sizeof(boot_seed));

if (psa_err != PSA_SUCCESS) {
return TFM_PLAT_ERR_SYSTEM_ERR;
}

boot_seed_set = true;
}
memcpy(buf, boot_seed, sizeof(uint8_t) * size);
#else
#error "No crypto hardware to generate boot seed available."
#endif
hellesvik-nordic marked this conversation as resolved.
Show resolved Hide resolved
return TFM_PLAT_ERR_SUCCESS;
}

Expand Down
6 changes: 6 additions & 0 deletions modules/trusted-firmware-m/tfm_boards/nrf54l15_cpuapp/config.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,9 @@ include(${PLATFORM_PATH}/common/${NRF_SOC_VARIANT}/config.cmake)

# Override PS_CRYPTO_KDF_ALG
set(PS_CRYPTO_KDF_ALG PSA_ALG_SP800_108_COUNTER_CMAC CACHE STRING "KDF Algorithm to use")

set(CONFIG_NRFX_RRAMC ON CACHE BOOL "Enable nrfx drivers for RRAMC")
add_compile_definitions(CONFIG_NRFX_RRAMC)
Comment on lines +17 to +18
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, we are already using a very minimal homemade RRAMC driver in TF-M. What are you doing this for exactly?
(Also the add_compile_definitions() line seems a bit weird, why is it needed?)

Copy link
Contributor Author

@hellesvik-nordic hellesvik-nordic Nov 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

CONFIG_NRFX_RRAMC is used in attest_hal.c, which is used in both Zephyr and TF-M. For example here.
If I remove add_compile_definitions(), I get errors such as "CONFIG_NRFX_RRAMC" not defined. I don't know if this is the correct way to add it, I just copied it from cpuarch.cmake.
Do you think I should look for alternative ways to get this config included?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahem you are linking to add_compile_definitions(__NRF_TFM__) in cpuarch.cmake?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah ups. Edited to change file name.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm okay, you are still linking to the example of __NRF_TFM__, I thought you'd have an example with CONFIG_NRFX_RRAMC. (After grepping I see there is none).
I am not sure, but it seems like this add_compile_definitions() could be unconditionally adding CONFIG_NRFX_RRAMC as a define? Or does it somehow get its value from the set() line above? Do you know how it works?
I'm wondering why don't we do this via Kconfig, as this is a Kconfig option.
This doesn't seem like a natural (or proper) way to do that, but I'm not sure.
What is this needed for exactly? Where do you get errors from?


# Override attestation to sign message instead of hash, because CRACEN drivers can not sign a hash.
set(ATTEST_SIGN_MESSAGE ON CACHE BOOL "Sign message instead of hash")
3 changes: 3 additions & 0 deletions modules/trusted-firmware-m/tfm_config.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@
/* The stack size of the Initial Attestation Secure Partition */
#cmakedefine ATTEST_STACK_SIZE @ATTEST_STACK_SIZE@

/* Sign message instead of message hash */
#cmakedefine01 ATTEST_SIGN_MESSAGE

/* Set the initial attestation token profile */
/* The TF-M config_base.h configuration will do
* #define ATTEST_TOKEN_PROFILE_PSA_IOT_1 1, if non of the token profiles are
Expand Down
13 changes: 13 additions & 0 deletions samples/tfm/tfm_psa_template/boards/nrf54l15dk_nrf54l15_cpuapp_ns.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
CONFIG_TFM_NRF_PROVISIONING=n
CONFIG_TFM_DUMMY_PROVISIONING=y
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 should enable nrf_provisioning


CONFIG_SPI_NOR=n
CONFIG_TFM_EXCEPTION_INFO_DUMP=y
CONFIG_TFM_CMAKE_BUILD_TYPE_DEBUG=y
CONFIG_TFM_SPM_LOG_LEVEL_DEBUG=y
CONFIG_RESET_ON_FATAL_ERROR=n
CONFIG_PM_PARTITION_SIZE_TFM=0x50800
# CONFIG_PSA_WANT_ALG_ECDSA_ANY=y
CONFIG_DEBUG=y
CONFIG_DEBUG_THREAD_INFO=y
CONFIG_DEBUG_OPTIMIZATIONS=y
6 changes: 6 additions & 0 deletions samples/tfm/tfm_psa_template/sysbuild/mcuboot/boards/nrf54l15dk_nrf54l15_cpuapp.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
CONFIG_PM_PARTITION_SIZE_MCUBOOT=0xb800
CONFIG_SPI_NOR=n
CONFIG_BOOT_MAX_IMG_SECTORS=256

# FPROTECT is set in NSIB instead
CONFIG_FPROTECT=n
2 changes: 1 addition & 1 deletion subsys/bootloader/bl_storage/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@

config SECURE_BOOT_STORAGE
bool "Library for accessing the bootloader storage"
select NRFX_RRAMC if SOC_SERIES_NRF54LX
select NRFX_RRAMC if SOC_SERIES_NRF54LX && !TRUSTED_EXECUTION_NONSECURE
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this still needed as you guarded select SECURE_BOOT_STORAGE with if !TRUSTED_EXECUTION_NONSECURE?
(I don't know about this so I can't tell whether or not it makes sense to have this here. Maybe even the whole SECURE_BOOT_STORAGE Kconfig option should depend on !TRUSTED_EXECUTION_NONSECURE?)

2 changes: 1 addition & 1 deletion west.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ manifest:
- name: trusted-firmware-m
repo-path: sdk-trusted-firmware-m
path: modules/tee/tf-m/trusted-firmware-m
revision: 82e7763eba112a350d58dd52dc39f340a291ffd0
revision: pull/180/head
- name: psa-arch-tests
repo-path: sdk-psa-arch-tests
path: modules/tee/tf-m/psa-arch-tests
Expand Down