Skip to content

DNM [nrf fromtree] mbedtls: align crypto_sizes.h to the Mbed TLS one #184

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

Closed
wants to merge 46 commits into from

Conversation

frkv
Copy link
Contributor

@frkv frkv commented Feb 4, 2025

crypto_sizes.h is no more aligned to the Mbed TLS version and it misses some #define that make some test to fail at build time in Zephyr. This commit fixes this disparities.

Note: this commit can be ignored in future TF-M repo updates assuming that this fix will be already included in the upstream version of TF-M.

Signed-off-by: Valerio Setti vsetti@baylibre.com
Signed-off-by: Frank Audun Kvamtrø frank.kvamtro@nordicsemi.no
(Cherry-picked from commit bceac6c)

DNM: Used for testing

SebastianBoe and others added 30 commits December 23, 2024 10:43
…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.

Signed-off-by: Sebastian Bøe <sebastian.boe@nordicsemi.no>
Change-Id: Ife60d1e76adffeb62f5ad32e0a85da8cfa467203
(cherry picked from commit b60bdb6)
…tances

Add driver function.

Signed-off-by: Sebastian Bøe <sebastian.boe@nordicsemi.no>
Change-Id: Ib1e442a54d599c4e42e74903d49920f24e9d8ec9
(cherry picked from commit 5d8b824)
…ecure

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

Change-Id: I2489defaf6deb89beba7447ba079ea3e5afebca5
Signed-off-by: Markus Rekdal <markus.rekdal@nordicsemi.no>
(cherry picked from commit c670a6a)
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 <georgios.vasilakis@nordicsemi.no>
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ø <vidar.lillebo@nordicsemi.no>
Change-Id: Ifa7fe00e511a6071b2b5c455df84b8e4f0535c84
(cherry picked from commit dc77905)
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 <markus.lassila@nordicsemi.no>
(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 <seppo.takalo@nordicsemi.no>
(cherry picked from commit ca03e40)
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 <seppo.takalo@nordicsemi.no>
(cherry picked from commit 539dd89)
Port spu_peripheral_config to also support the new API.

Signed-off-by: Sebastian Bøe <sebastian.boe@nordicsemi.no>
Change-Id: I1763874ce74ad39cbf0ef256ef8edc669038d226
(cherry picked from commit 3f49abf)
Deactivation of STSAFEA, which is used to deactivate the flag
MBEDTLS_PSA_CRYPTO_SE_C and the latter causes a problem of structure
alignment /*psa_key_attributes_s*/ in the crypto_struct.h file
between the 2 service protected storage(PS) and crypto

Change-Id: I8312e0a92030d2bd205222c9beb81bc4089c6be6
Signed-off-by: Ahmad EL JOUAID <ahmad.eljouaid@st.com>
(cherry picked from commit 7045675)
…tion

The whole of the SRAM was configured unprivileged on this platform, so
the memory protection required for isolation level 2 was not present.

This patch changes the S_DATA_START to S_DATA_LIMIT MPU region to be
configured for privileged access only. It also reorders the MPU regions
so that the App RoT sub-region overlapping S_DATA has a higher region
number and so takes priority in the operation of the Armv6-M MPU.

Signed-off-by: Jamie Fox <jamie.fox@arm.com>
Change-Id: Icdf169f92f7a47b27ea38dac4098e3205af7f5af
(cherry picked from commit 66596b4)
These scripts help the user to compile the TF-M project
on all the STM platforms. Picking it up as platform specific improvement which is acceptable in LTS designed branches)

Signed-off-by: Ahmad EL JOUAID <ahmad.eljouaid@st.com>
Change-Id: Id9fe7c8c048b4919e2ec199a251b0ecec5e1c1aa
(cherry picked from commit 6737911b041db2c2f37f4e827af29cf36129fe4b)
(cherry picked from commit 6a54ec8)
Update the CMake checkout dependency and re-align the
headers to the ones available in Mbed TLS 3.6.1.

Signed-off-by: Antonio de Angelis <antonio.deangelis@arm.com>
Change-Id: I681df1f2662c55b7aaf7eed2642b7ce3eeae8192
(cherry picked from commit 2a59580)
…ciation fails

After the connection is successfully allocated, if the parameters
association fails, then the connection needs to be released.
This is only required for STATELESS connections for the reason that
for stateful connections those are taken care of as part of the
psa_close sequence.

Signed-off-by: Nicola Mazzucato <nicola.mazzucato@arm.com>
Change-Id: Ic0674098b7780a4e83b21fe93c5ed83ff5a2e8d1
(cherry picked from commit 417063d)
If the requested_size from the scratch allocator is
greater than 0xfffffffc, the align macro overflows
without failing allocation thus allowing out-of-bounds
writes in the Crypto partition memory.

Signed-off-by: Antonio de Angelis <antonio.deangelis@arm.com>
Change-Id: Ic218fea8238ecd3e8d146586d2c413386870d580
(cherry picked from commit fc289ce)
Several places in the Crypto service directly write
without checking at least pointer validity (i.e. not
NULL) or out-of-bound access in the scratch buffer
writes. These mostly would lead to crashes, i.e. a
Denial-of-Service attack for which TF-M does not
protect against, but reduce the potential for more
serious attacks by protecting those writes anyway.

Signed-off-by: Antonio de Angelis <antonio.deangelis@arm.com>
Change-Id: I4d60cca04162f15abd96a8c3fbe3683042b7b55d
(cherry picked from commit 8d506f5)
Assert on the validity of the input buffer only if the
input_length is different than 0. Calls with non-NULL
input and input_length == 0 are fairly normal, especially
when used only for authentication purposes.

Signed-off-by: Antonio de Angelis <antonio.deangelis@arm.com>
Change-Id: I733341179dcbd04c3862050b1105474dd7322e77
(cherry picked from commit 75bbe3f)
A few checks are missing from the mailbox message parameters:
 - NULL checks on vector pointers
 - maximum number of input vectors

Add such checks and move the related copy to local vectors
in a separate function.

Note that proper validation of the psa vectors on whether the
given addresses are valid is performed in the subsequent
'spm_associate_call_params' when processing
the call.

Signed-off-by: Nicola Mazzucato <nicola.mazzucato@arm.com>
Change-Id: Ife09a48ca9d8547ada3ac099cc1eb2b0c9cf3f17
(cherry picked from commit 25f2408)
…ecks failure

If the validation of the vector parameters fails, the outvec are written
back regardless.
This may cause an out-of-bound write from the address that was previously
stored in original_out_vec and the length that could go passed the local
out_vec.

Note that this fix requires:
`tfm_spe_mailbox: Validate vectors from NSPE`

Prior to this change and the one above, it is possible to craft a couple
of mailbox messages to first write in vectors[1].in_vec a target value,
then a second message with:
 - a out_vec.len to go past out_vec[0], 6 for example
 - a target address for a PSA-ROT private storage, `ps_crypto_iv_buf`
   for example.

Signed-off-by: Nicola Mazzucato <nicola.mazzucato@arm.com>
Change-Id: Iadff8d6ba8160c1b757e6a1a9622473781b2027c
(cherry picked from commit 5ae0a02)
…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 <nicola.mazzucato@arm.com>
Change-Id: Ieb72bbe046e4d909aab4728902fa5da61ab9bf0c
(cherry picked from commit a691e2f)
Minor tidy-up to use local in_vec and out_vec in local_copy_vects.

Signed-off-by: Nicola Mazzucato <nicola.mazzucato@arm.com>
Suggested-by: Chris Brand <chris.brand@cypress.com>
Change-Id: I7179d668e42b27a1d18ccf727008cc47e549a7ef
(cherry picked from commit 64b6ea5)
…ound access

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

Change-Id: Ie642abf61bd4789cc5d51ba66efe2e852b6659fa
Signed-off-by: Bohdan Hunko <Bohdan.Hunko@infineon.com>
(cherry picked from commit 13f69b3)
…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 <antonio.deangelis@arm.com>
Change-Id: Ia267cad1a248b29d96efdf5f5acfcf92b743de97
(cherry picked from commit 9098fa8)
…re proceeding

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

Signed-off-by: Mikolai Gütschow <mikolai.guetschow@tu-dresden.de>

Change-Id: Ifdc75aa1d2a0c71c8fbce5917375216388f55f68
(cherry picked from commit d1d4e2a)
in/out vectors can be NULL as long as size is 0.

Change-Id: Ie4c03b01224260001600b94aa22886f6d8cd62e7
Signed-off-by: Bohdan Hunko <Bohdan.Hunko@infineon.com>
(cherry picked from commit 44a0a82)
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 <antonio.deangelis@arm.com>
Change-Id: I056831f7fcd74d9c45010aa1d79ad10418c1f1f3
(cherry picked from commit bb6f711)
PCD memory area used with nRF53 to be locked with TF-M,
instead of bootloader.

Change-Id: Ie9058cac2236ed1c4e179c740a4b903b5e676c23
Signed-off-by: Markus Lassila <markus.lassila@nordicsemi.no>
(cherry picked from commit 5d2562c)
Fix warning induced by missing include.

Change-Id: I27a429dfbc8f1c2c926da2089bffd7e81363276a
Signed-off-by: Markus Lassila <markus.lassila@nordicsemi.no>
(cherry picked from commit 21ff86a)
Fixes a missing ifdef in the spu.c which broke building
TF-M.

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
Change-Id: Ie6129882127d5119f491c8f6be2bd0d4486d668f
(cherry picked from commit e8d8675)
Add missing capacity in tfm_ps_get_info calls.

Change-Id: I37432d204ee87971915471dce9b3a2ebcce057e2
Signed-off-by: Markus Lassila <markus.lassila@nordicsemi.no>
(cherry picked from commit fafe163)
Vge0rge and others added 16 commits December 23, 2024 10:48
…sync repos

This is a big change with two goals:
1) Add initial support for building nRF54L15 with upstream TF-M
2) Align the Nordic platform code between the upstream TF-M and
   Nordics TF-M fork

This change does NOT add support for nRF54L15 in upstream TF-M yet,
it only adds building support at the moment. More effort is needed
to allow running upstream TF-M in this platform. Most of the
configuration files for nRF54L15 are plain copies from the nRF5340
with light modifications to allow building.

This change brings an updated version of the nrfx library as well,
since it is needed to provide definitions for the nRF54L15.

Change-Id: I7543296f2ba839c5dd886fbc1231a5fedc23fd8f
Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
(cherry picked from commit fe48e05d32bee79bd61e990c78b1367b52a7d955)
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 <sebastian.boe@nordicsemi.no>
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
…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 <sebastian.boe@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
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 <markus.swarowsky@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
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ø <vidar.lillebo@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
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 <sebastian.boe@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
Version check depends on upstream's tagging scheme which differs
from NCS's

Signed-off-by: Vidar Lillebø <vidar.lillebo@nordicsemi.no>
-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ø <frank.kvamtro@nordicsemi.no>
Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
-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ø <frank.kvamtro@nordicsemi.no>
-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ø <frank.kvamtro@nordicsemi.no>
…r sharing

-Will be squashed in a different commit which was the version that
 worked before

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
…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 <joakim.andersson@nordicsemi.no>
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Signed-off-by: Sebastian Bøe <sebastian.boe@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
Adjust CRYPTO_HW_ACCELERATOR build scripts to also support
nrf_security.

Signed-off-by: Sebastian Bøe <sebastian.boe@nordicsemi.no>
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
(cherry picked from commit c136210)
(cherry picked from commit 3834117)
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
(cherry picked from commit 2bdad64)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Change-Id: Ied8e378ef55fe398ea4e45f65b3c270e9e9cd030
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
(cherry picked from commit 5903966)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
(cherry picked from commit a3a03e5)
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
crypto_sizes.h is no more aligned to the Mbed TLS version and
it misses some #define that make some test to fail at build
time in Zephyr. This commit fixes this disparities.

Note: this commit can be ignored in future TF-M repo updates
assuming that this fix will be already included in the upstream
version of TF-M.

Signed-off-by: Valerio Setti <vsetti@baylibre.com>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
(Cherry-picked from commit bceac6c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.