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

Not ready for review: [nrf noup] Allow TF-M without ITS build #181

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

Conversation

frkv
Copy link
Contributor

@frkv frkv commented Nov 22, 2024

Allowing TF-M to be enabled without PSA ITS support (NOUP as it breaks Firmware Framework rules built in
manifest-based generation of partitions)

SebastianBoe and others added 30 commits August 16, 2024 09:24
…e base addr

Refactor spu_peripheral_config to use base addresses instead of IDs as
future platforms will need the base address to identify which spu
instance to use.

(Cherry picked from commit b60bdb6)

Signed-off-by: Sebastian Bøe <[email protected]>
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
…tances

Add a function to return the SPU instance that can be used to configure
the peripheral at a given base address.

Signed-off-by: Sebastian Bøe <[email protected]>
Change-Id: Ib1e442a54d599c4e42e74903d49920f24e9d8ec9
(Cherry picked from commit 5d8b824)
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
…ecure

Dont configure the volatile memory controller as a non-secure peripheral

(cherry picked from commit c670a6a)

Change-Id: I2489defaf6deb89beba7447ba079ea3e5afebca5
Signed-off-by: Markus Rekdal <[email protected]>
There are some hardware registers in Nordic platforms
which are mapped as secure only. In order to allow the
non-secure application to control these registers I added
here a secure service which allows 32-bit writes to secure
mapped memory. The writes are only allowed on  addresses and
masks defined in a header list. It is also possible to
provide an allowed_values list in order to further limit
the accepted values.

Renamed:  tfm_read_ranges.h -> tfm_platform_user_memory_ranges.h
since now it can be used for both reads and writes.

The list in the current platforms is empty and might be populated
later.

Signed-off-by: Georgios Vasilakis <[email protected]>
Change-Id: Ifa31ba73ec07b216a7e987653255fcc6e9d3989c
(cherry picked from commit 57b3342)
The check for whether file should be encrypted, and be fully written
missed some PS usage.

Signed-off-by: Vidar Lillebø <[email protected]>
Change-Id: Ifa7fe00e511a6071b2b5c455df84b8e4f0535c84
(Cherry picked from commit dc77905)
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
NRF_APPROTECT and NRF_SECURE_APPROTECT
to take precedence over other mechanisms when configuring
debugging for TF-M.

For nRF53 and nRF91x1 the actual locking of firmware is done
elsewhere. This further locks the UICR.

nRF9160 supports only hardware APPROTECT. This will lock the
APPROTECT / SECUREAPPROTECT in the next boot, when the above
settings are configured.

Change-Id: I5e304be0f8a34c0016488d9ec09929bbcb38481f
Signed-off-by: Markus Lassila <[email protected]>
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
(Cherry picked from commit 734a51d)
On certain nRF plaforms, like nRF9160, reading UICR registers
might need special handling, which is already implemented in
nrfx_nvmc_uicr_word_read() so use that, instead on memcpy().

For more information, see nRF9160 Errata 7.

Change-Id: Iea9d0bf4184decd5650b4d4b620fbef0c64a55f6
Signed-off-by: Seppo Takalo <[email protected]>
(cherry picked from commit ca03e40)
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
The anomaly only appears on nRF91 platforms and some
platforms do not have NVMC so the header cannot be
included.

Change-Id: I02c73c9a752599ca9be9320dc19f390aea0f767a
Signed-off-by: Seppo Takalo <[email protected]>
(cherry picked from commit 539dd89)
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
Port spu_peripheral_config to also support the new API.

Signed-off-by: Sebastian Bøe <[email protected]>
Change-Id: I1763874ce74ad39cbf0ef256ef8edc669038d226
(Cherry-picked from the commit 3f49abf)
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
Adjust CRYPTO_HW_ACCELERATOR build scripts to also support
nrf_security.

Signed-off-by: Sebastian Bøe <[email protected]>
Signed-off-by: Joakim Andersson <[email protected]>
(cherry picked from commit c136210)
(cherry picked from commit 3834117)
Signed-off-by: Joakim Andersson <[email protected]>
(cherry picked from commit 2bdad64)
Signed-off-by: Markus Swarowsky <[email protected]>
Change-Id: Ied8e378ef55fe398ea4e45f65b3c270e9e9cd030
Signed-off-by: Markus Swarowsky <[email protected]>
(cherry picked from commit 5903966)
Signed-off-by: Markus Swarowsky <[email protected]>
(cherry picked from commit a3a03e5)
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
The MDK for nRF9120 used in the nRF9161 target doesn't define the Secure FPU
as it doesn't exist, but for other platforms like the 9160 it has a dummy
define, with an UNUSED field in the type.
The long plan is to get this fixed in the MDK but until then, to make
the nrfxlib 3.1.0 update possible this tempfix is applied.

 Ref: NCSDK-23046

Signed-off-by: Markus Swarowsky <[email protected]>
Change-Id: I44042ee9aada99c59a5930440306bb6c40ae4880
(cherry picked from commit 6ad9c58)
Signed-off-by: Markus Swarowsky <[email protected]>
(cherry picked from commit a489e9f)
Signed-off-by: Markus Swarowsky <[email protected]>
…nce.

Add an option to send the log output from the secure firmware on a
UART instance that would be shared with the non-secure application.

This option is added where the number of UART instances is limited
and the application only cares about the receiving the TF-M log
on fatal errors.

To allow this option to be enabled the log is disabled in the boot
process before the non-secure application is started.
It is enabled again when an unrecoverable exception has occurred in
the secure firmware.

Here is an abandoned upstream PR (with some of the fixes):
https://review.trustedfirmware.org/c/TF-M/trusted-firmware-m/+/25905

Note: This has removed any information about cherry-picked items
as this is not valid since it is combining efforts form multiple
commits

Ref: NCSDK-18595
Ref: NCSDK-28740

Signed-off-by: Joakim Andersson <[email protected]>
Signed-off-by: Markus Swarowsky <[email protected]>
Signed-off-by: Sebastian Bøe <[email protected]>
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
The MBEDTLS_PSA_CRYPTO_CONFIG_FILE gets already defined in the
mbedtls_common target and is included in the nrf-config.h file.
TF-M adds the compile definition again, causing a redefined warning when
building

We may want to refactor this to align better with upstream project

Ref: NCSDK-28740

Signed-off-by: Markus Swarowsky <[email protected]>
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
TF-M checks if p256-m is available during build time using
MBEDCRYPTO_PATH which is set to the TF-M repo to use custom
Mbed TLS cmake configurations, but this means the script can not be
found. But as Mbed TLS software crypto is not used anyway we can
hardcode p256-m to be disabled.

Ref: NCSDK-28740

Signed-off-by: Sebastian Bøe <[email protected]>
Signed-off-by: Markus Swarowsky <[email protected]>
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
…nifest

This commit is [nrf noup] because I would like to user-test this for a
few months in case of unintended side-effects before upstreaming.

In the TF-M build scripts we run the manifest tool twice, first from
CMake and then from ninja.

It is bad practice to configure CMake projects like this. Instead, if
configuration from CMake is necessary, one should configure from CMake
only, and then re-run CMake when necessary, not just the command.

This organization has been causing problems for our users as they have
been required to rebuild TF-M twice.

This is due to this scenario playing out:

CMake generates config_impl.cmake by invoking the manifest tool at
Configure time.

CMake generates build.ninja.

Ninja generates config_impl.cmake by invoking the manifest tool at
build time.

When the user then invokes ninja a second time config_impl.cmake will
be newer than build.ninja. But CMake is supposed to be includ'ing
config_impl.cmake, so build.ninja is now considered out-of-date
wrt. config_impl.cmake.

ninja therefore invokes CMake again, and then ninja afterwards.

Ref: NCSDK-28740

Signed-off-by: Sebastian Bøe <[email protected]>
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
This is noup commit as upstream TF-M relies on the mbed TLS PSA Core
hat does not support the PAKE API's according to 1.2 at the moment.
Once this exists then this can be up streamed, or removed if TF-M adds
it themself.

Added PAKE API support accoding the PSA crypto spec 1.2

Ref: NCSDK-22416
Ref: NCSDK-28740

Signed-off-by: Markus Swarowsky <[email protected]>
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
Include autoconf.h from target_cfg.c so we can configure the TF-M
image based on the non-secure image's Kconfig.

Signed-off-by: Sebastian Bøe <[email protected]>
Change-Id: I2212f2ec3428f16618334c5583b0e641aa30ea08
Allows custom key-loader to be used for the PSA core and allows
configuring CMAC KDF usage for PS.

noup-reason: PSA_ALG_SP800_108_COUNTER_CMAC is not available in upstream.
After testing and verifying the solution (determining if we need further
changes) we should try to upstream this.

Ref: NCSDK-28740

Signed-off-by: Vidar Lillebø <[email protected]>
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
This commit is a noup because we want an NCS specific error message.

Detect wrong headers being included. See comment for details.

Ref: NCSDK-28740

Signed-off-by: Sebastian Bøe <[email protected]>
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
For Secure only builds on 53 there exists the Kconfig
CONFIG_SOC_ENABLE_LFXO to define if the XL1 and XL2 pin should be
configured to used for the LFXO oscillator. TF-M should have the same
behavior, to enable the possibility to use these pins for something else

[nrf noup] as we don't have the NCS Kconfigs available in upstream TF-M.
The CONFIG_ prefixed Kconfigs is made available in the noup commit:

Ref: NCSDK-20678
Ref: NCSDK-28740

Signed-off-by: Markus Swarowsky <[email protected]>
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
Version check depends on upstream's tagging scheme which differs
from NCS's

Signed-off-by: Vidar Lillebø <[email protected]>
…RT0 instance

Add support for selecting which UART instance to use as the secure UART
instance. The supported options are UART0 and UART1.

Add support for the secure UART instance being shared with the non-secure
application.
The UART instance is configured as non-secure after it has been
uninitialized, and configured as secure when it is initialized again
on a fatal error.

Note: device-specific target_cfg.h was provided here, which has been
dropped from the commit

Fixup: The spu_peripheral_config_(non_)secure calls takes the
ID of the peripheral as the argument and not the register
address.

Ref: NCSDK-18595
Ref: NCSDK-28740

Signed-off-by: Joakim Andersson <[email protected]>
(cherry picked from commit b2346e8)
Signed-off-by: Joakim Andersson <[email protected]>
(cherry picked from commit 97224b0)
Signed-off-by: Markus Swarowsky <[email protected]>
Change-Id: I2da826ec4817143ece52baeceaab14999f0d2d96
Signed-off-by: Markus Swarowsky <[email protected]>
(cherry picked from commit d2a1b89)
Signed-off-by: Markus Swarowsky <[email protected]>
Signed-off-by: Georgios Vasilakis <[email protected]>
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
Add support for nRF54L

Signed-off-by: Sebastian Bøe <[email protected]>
Signed-off-by: Joakim Andersson <[email protected]>
Signed-off-by: Markus Swarowsky <[email protected]>
Signed-off-by: Georgios Vasilakis <[email protected]>
Signed-off-by: Vidar Lillebø <[email protected]>
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
-This commit adds support for externally built PSA core in TF-M
 by checking for the CMake variable (cached) PSA_CRYPTO_EXTERNAL_CORE.
 By setting this define, then a platform-target file called
 external_core.cmake as well as external_core_install.cmake is called
 to allow for the following:
 - Early include of necessary replacement include folders
 - Support for using generated configuration files for TF-M build
-This commit also tries to make psa_crypto_config and
 psa_crypto_library_config linked in first to ensure that certain
 folders are included as early as possible in the build

Signed-off-by: Frank Audun Kvamtrø <[email protected]>
-This changes includes from autoconf.h to zephyr/autoconf.h as the former
 has been deprecated

Signed-off-by: Frank Audun Kvamtrø <[email protected]>
This commit will be reworked

Signed-off-by: Frank Audun Kvamtrø <[email protected]>
-The macro ARRAY_LENGTH is defined without checking if there is already
 a definition. This commit can be reverted once the proposed fix
 is handled upstream
-This fixes ARRAY_LENGTH in s_io_sorage_tests.c

Signed-off-by: Frank Audun Kvamtrø <[email protected]>
-The upstream code is using peripheral-ids, but is lacking
 the ability to resolve SPU entries for the peripheral.
 This WIP commit sets it back to the way it is in sdk-trusted-firmware-m
 prior to TF-M 2.1.0

Signed-off-by: Frank Audun Kvamtrø <[email protected]>
-This adds MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS and
 PSA_CRYPTO_DRIVER_TFM_BUILTIN_KEY to tfm_psa_rot_partition_crypto

Signed-off-by: Frank Audun Kvamtrø <[email protected]>
-Hopefully fixes TF-M shared UART issues

Signed-off-by: Frank Audun Kvamtrø <[email protected]>
nicola-mazzucato-arm and others added 28 commits October 18, 2024 12:18
…x message

Security Advisory TFMV-8 is documented:
"Unchecked user-supplied pointer via mailbox messages may cause write
of arbitrary address".

Please check the advisory document for further details.

Signed-off-by: Nicola Mazzucato <[email protected]>
Change-Id: Ieb72bbe046e4d909aab4728902fa5da61ab9bf0c
(cherry picked from commit a691e2f)
(cherry picked from commit 15afe61)
Signed-off-by: Tomi Fontanilles <[email protected]>
Minor tidy-up to use local in_vec and out_vec in local_copy_vects.

Signed-off-by: Nicola Mazzucato <[email protected]>
Suggested-by: Chris Brand <[email protected]>
Change-Id: I7179d668e42b27a1d18ccf727008cc47e549a7ef
(cherry picked from commit 64b6ea5)
(cherry picked from commit a2cead6)
Signed-off-by: Tomi Fontanilles <[email protected]>
…ound access

Fix some checks, add some more missed checks.
With that, add missing brackets.

Change-Id: Ie642abf61bd4789cc5d51ba66efe2e852b6659fa
Signed-off-by: Bohdan Hunko <[email protected]>
(cherry picked from commit 62b1300)
Signed-off-by: Tomi Fontanilles <[email protected]>
…n Mbed TLS config

The TF-M Crypto service is configured by default not to enable the
memory mapped IOVEC, hence keep the MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS
on to avoid unnecessary copying of parameters back and forth.

Signed-off-by: Antonio de Angelis <[email protected]>
Change-Id: Ia267cad1a248b29d96efdf5f5acfcf92b743de97
(cherry picked from commit 89b9c48)
Signed-off-by: Tomi Fontanilles <[email protected]>
…re proceeding

nRF52840's CRYPTOCELL implementation of cc310 was dead-locking otherwise on the first PKA operation

Signed-off-by: Mikolai Gütschow <[email protected]>

Change-Id: Ifdc75aa1d2a0c71c8fbce5917375216388f55f68
(cherry picked from commit 974bc10)
Signed-off-by: Tomi Fontanilles <[email protected]>
in/out vectors can be NULL as long as size is 0.

Change-Id: Ie4c03b01224260001600b94aa22886f6d8cd62e7
Signed-off-by: Bohdan Hunko <[email protected]>
(cherry picked from commit 7da71fd)
Signed-off-by: Tomi Fontanilles <[email protected]>
Some integration decide to enforce ABI compatibility
between the client interfaces and the crypto service
interfaces for PSA Crypto API. In this case the structures
have the same layout hence make sure that the service
performs the appropriate checks on parameters. Enable this
through the CRYPTO_LIBRARY_ABI_COMPAT option during
TF-M Crypto service build.

Signed-off-by: Antonio de Angelis <[email protected]>
Change-Id: I056831f7fcd74d9c45010aa1d79ad10418c1f1f3
(cherry picked from commit 471c127)
Signed-off-by: Tomi Fontanilles <[email protected]>
PCD memory area used with nRF53 to be locked with TF-M,
instead of bootloader.

Change-Id: Ie9058cac2236ed1c4e179c740a4b903b5e676c23
Signed-off-by: Markus Lassila <[email protected]>
(cherry picked from commit 5d2562c)
Fix warning induced by missing include.

Change-Id: I27a429dfbc8f1c2c926da2089bffd7e81363276a
Signed-off-by: Markus Lassila <[email protected]>
(cherry picked from commit 21ff86a)
Add support for nRF54L

Change-Id: I3df897232195a4f113db5a934a8da2504720db38
Signed-off-by: Sebastian Bøe <[email protected]>
Signed-off-by: Joakim Andersson <[email protected]>
Signed-off-by: Markus Swarowsky <[email protected]>
Signed-off-by: Georgios Vasilakis <[email protected]>
Signed-off-by: Vidar Lillebø <[email protected]>
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
(cherry picked from commit e0b5686)
Signed-off-by: Markus Lassila <[email protected]>
NRF_APPROTECT and NRF_SECURE_APPROTECT
to take precedence over other mechanisms when configuring
debugging for TF-M.

For nRF53 and nRF91x1 the actual locking of firmware is done
elsewhere. This further locks the UICR.

nRF9160 supports only hardware APPROTECT. This will lock the
APPROTECT / SECUREAPPROTECT in the next boot, when the above
settings are configured.

Change-Id: I5e304be0f8a34c0016488d9ec09929bbcb38481f
Signed-off-by: Markus Lassila <[email protected]>
Signed-off-by: Frank Audun Kvamtrø <[email protected]>
(cherry picked from commit 9573717)
Signed-off-by: Markus Lassila <[email protected]>
This commit will be reworked

Signed-off-by: Frank Audun Kvamtrø <[email protected]>
(cherry picked from commit ccb7244)
Signed-off-by: Markus Lassila <[email protected]>
-The upstream code is using peripheral-ids, but is lacking
 the ability to resolve SPU entries for the peripheral.
 This WIP commit sets it back to the way it is in sdk-trusted-firmware-m
 prior to TF-M 2.1.0

Signed-off-by: Frank Audun Kvamtrø <[email protected]>
(cherry picked from commit 6d6229a)
Signed-off-by: Markus Lassila <[email protected]>
-This changes includes from autoconf.h to zephyr/autoconf.h as the former
 has been deprecated

Signed-off-by: Frank Audun Kvamtrø <[email protected]>
(cherry picked from commit abc2127)
Signed-off-by: Markus Lassila <[email protected]>
And remove support for `nrf54l15pdk` at the same time.
Occurences of `NRF54L15_ENGA_XXAA` are replaced by `NRF54L15_XXAA`.

Files from the `nrf54l15dk_nrf54l15_cpuapp` directory that are unused
are deleted.
One of them, `tfm_peripherals_config.h`, even had a blatant syntax
issue (`#endiff`).

Signed-off-by: Tomi Fontanilles <[email protected]>
(cherry picked from commit d762004)
Signed-off-by: Markus Lassila <[email protected]>
Do not attempt to lock the UICR.APPROTECT or UICR.SECUREAPPROTECT
for nRF54L15. These registers do not exist.

TF-M provisioning with nRF54L15 will still result in compilation
failure. As it should at this point.

Signed-off-by: Markus Lassila <[email protected]>
Not needed.
Speeds up asset writing time and improves RRAM lifetime.

Signed-off-by: Tomi Fontanilles <[email protected]>
…ze setting

CONFIG_NRF_RRAM_WRITE_BUFFER_SIZE is not passed to TF-M, so it cannot
be used.
Moreover, it seems that it's not even defined on the NS image when
using TF-M.

As a bonus, do not run the COMMIT_WRITEBUF task when using unbuffered writing.

Signed-off-by: Tomi Fontanilles <[email protected]>
Temporarily set RRAMC.CONFIG.WRITEBUFSIZE to 0 to use unbuffered mode
when writing to RRAM in TF-M.
This is done to reduce the interrupt latency increases provoked when
writing to RRAM.
Do not set it permanently so that it remains
CONFIG_NRF_RRAM_WRITE_BUFFER_SIZE for when the NS image code writes
to RRAM.

Signed-off-by: Tomi Fontanilles <[email protected]>
Add missing capacity in tfm_ps_get_info calls.

Change-Id: I37432d204ee87971915471dce9b3a2ebcce057e2
Signed-off-by: Markus Lassila <[email protected]>
(cherry picked from commit fafe163)
-This commit breaks with rules in Firmware Framework for Trusted
 Firmware-M with regards to stating that ITS is a mandatory feature.
 This can be done when PSA crypto + drivers can provide support for
 persistent keys without relying on MBEDTLS_PSA_CRYPTO_STORAGE_C being
 set when building the PSA core. This is only possible when PSA crypto
 driver provides support for persistent key (e.g. using CRACEN and KMU)
-Made TFM_INTERNAL_TRUSTED_STORAGE_SERVICE a weak_dependency to
 fix build-issues when PSA ITS is not enabled (tfm_crypto.yaml)
-Added weak_dependencies as a non_ffm_attribute for the PSA crypto
 partition (tfm_manifest_list.yaml)
-
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.