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
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_NONSECURE
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
select SECURE_BOOT_STORAGE if !TRUSTED_EXECUTION_NONSECURE
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
25 changes: 22 additions & 3 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 @@ -12,12 +12,22 @@
#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>


Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

#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 volatile uint8_t boot_seed_set = 0;

Check failure on line 27 in modules/trusted-firmware-m/tfm_boards/common/attest_hal.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

INITIALISED_STATIC 

modules/trusted-firmware-m/tfm_boards/common/attest_hal.c:27  do not initialise statics to 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
static volatile uint8_t boot_seed_set = 0;
static volatile bool boot_seed_set = 0;

static volatile uint8_t boot_seed[32];
#endif

static enum tfm_security_lifecycle_t map_bl_storage_lcs_to_tfm_slc(enum lcs lcs)
{
switch (lcs) {
Expand Down Expand Up @@ -122,6 +132,7 @@

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;

if (size != NRF_CC3XX_PLATFORM_TFM_BOOT_SEED_SIZE) {
Expand All @@ -132,7 +143,15 @@
if (nrf_err != NRF_CC3XX_PLATFORM_SUCCESS) {
return TFM_PLAT_ERR_SYSTEM_ERR;
}

#elif defined(CONFIG_PSA_NEED_CRACEN_KMU_DRIVER)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use the same macro when defining and using the boot_seed variables.

if (!boot_seed_set) {
psa_generate_random(boot_seed, 32);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
psa_generate_random(boot_seed, 32);
psa_generate_random(boot_seed, sizeof(boot_seed));

boot_seed_set = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
boot_seed_set = 1;
boot_seed_set = true;

}
memcpy(buf, boot_seed, sizeof(uint8_t) * 32);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
memcpy(buf, boot_seed, sizeof(uint8_t) * 32);
memcpy(buf, boot_seed, size);

maybe?

#else
return TFM_PLAT_ERR_SYSTEM_ERR;
#endif
Copy link
Contributor Author

Choose a reason for hiding this comment

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

My gut feeling says there must be a better way to do this. @frkv do you have any thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Build failure. #error

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?)


# 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
Loading