-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Update TF-M support to v1.2 #14354
Update TF-M support to v1.2 #14354
Conversation
hash_wrappers.c is specific to Mbed OS, moving it into platform as its original directory is for imported Mbed TLS source only.
Note: Now we need to export common.h to the include path, because this header is now also needed by PSA Crypto service.
Files have been automatically imported by setting MBED_TLS_RELEASE to mbedtls-2.24.0 in connectivity/mbedtls/tools/importer/Makefile and running `make` in that directory.
Rather than maintaining a specific `TARGET_TFM_V1_x`, its better to use more generic name `TARGET_TFM_LATEST` to avoid confusion on the latest TFM version supported by Mbed OS * Rename the folder from `TARGET_TFM_V1_1` to `TARGET_TFM_LATEST` * Update the CmakeLists.txt * Change the name of the MUSCA targets to maintain uniformity with TF-M v1.2 * Update target.json for PSA_V8_M to use `TFM_LATEST`
These files have been imported/copied from: * ARMmbed/trusted-firmware-m * ARMmbed/tf-m-tests These are generic files, which are required for TF-M v1.2 integration with Mbed OS for PSA_V8M and PSA_DUAL_CORE targets.
The PSA headers imported from TF-M does not contain a declaration of mbedtls_ecc_group_to_psa(), which is expected by pk.c from Mbed TLS. This leads to an "undefined symbol" error when using the ARM toolchain to compile an application for a TF-M target.
* Partition files are synced with TF-M v1.2 * To have uniformity with TF-M v1.2, rename the following: ** image_macros_preprocessed_ns.c to `signing_layout_ns.c` ** image_macros_preprocessed_s.c to `signing_layout_s.c` * `MCUBOOT_IMAGE_NUMBER` is set to 2 by default for TF-M v1.2, therefore it is necessary that Mbed OS compiles the right macros for when linking and using the partition files ** Workaround ** The `region_defs.h` has an explicit definition of `BL2`, even though it is already defined in target.json for `ARM_MUSCA_B1`. This is because of Mbed CLI 1, as it can't seem to use the right macro when linking the files for Mbed OS application when using the ARMCLANG toolchain.
* Remove the old `mcuboot.bin` * Add `bl2.bin` * Update TF-M secure binaries GCC_ARM toolchain used.
The script changes are required with respect to TF-M v1.2 integration for this target. The imgtool.py is been replaced with `wrapper.py` which uses click command to run the signing algorithm. The version `-v` and dependencies `-d` have been updated to resolve upgrade issues from TF-M v1.1 --> v1.2
* Partition files are synced with TF-M v1.2 * To have uniformity with TF-M v1.2, rename the following: ** image_macros_preprocessed_ns.c to `signing_layout_ns.c` ** image_macros_preprocessed_s.c to `signing_layout_s.c` * `MCUBOOT_IMAGE_NUMBER` is set to 2 by default for TF-M v1.2, therefore it is necessary that Mbed OS compiles the right macros for when linking and using the partition files.
* Remove the old `mcuboot.bin` * Add `bl2.bin` * Update TF-M secure binaries GCC_ARM toolchain used.
The script changes are required with respect to TF-M v1.2 integration for this target. The imgtool.py is been replaced with `wrapper.py` which uses click command to run the signing algorithm. The version `-v` and dependencies `-d` have been updated to resolve upgrade issues from TF-M v1.1 --> v1.2
ARM_MUSCA_A1 is not supported since Mbed OS 6.0 Refer: #13165 Therefore remove files from kv_config and TF-M post binary hook script.
Raise an exception if there was an issue with handling any command when signing the binaries for MUSCA targets.
This files are imported and part of: mcu-tools/mcuboot@a8e12da Issue has been raised mcu-tools/mcuboot#930 As these scripts are going to be part of release its important to have licensing information.
Files containing signing layouts are parsed by the post build hook for signing purpose only.
The vector table needs to be copied from ROM to RAM, in order for us to set IRQ handlers at run time. The address in RAM is defined by `NVIC_RAM_VECTOR_ADDRESS` in `cmsis_nvic.h`, but its inclusion was missing from Musca S1's `cmsis.h` and consequently the vector table was not copied. On most targets this results in a memory access error when we set vectors. But Musca S1's ROM is in its MRAM (which can be accessed like any RAMs), and this causes the ROM image to be modified with no error/warning. On the next boot, MCUboot fails the image integrity check. This commit adds the missing include, in the same spirit as 01dd997.
Add `psa_set_key_enrollment_algorithm()` and `psa_get_key_enrollment_algorithm()` for TF-M targets. Note: This is deprecated and for backward compatibility only. Setting an enrollment algorithm is not recommended, because using the same key with different algorithms can allow some attacks based on arithmetic relations between different computations made with the same key, or can escalate harmless side channels into exploitable ones. Use this function only if it is necessary to support a protocol for which it has been verified that the usage of the key with multiple algorithms is safe.
Setting/getting key enrollment algorithm is not recommended and not part of the vanilla PSA or TF-M. For now keep the API just for backward compatibility with existing projects, and this commit adds deprecation warnings.
In targets.json, ARM_MUSCA_B1 and ARM_MUSCA_S1 have alias target names suffixed with `_NS`. They are identical to targets without `_NS` and exist purely for compatibility with the old naming convention we had. The CI builds them as separate targets and uses extra resources. As we are upgrading Musca targets to TF-M v1.2, it's time to clean up the aliases.
The directory `mbed-tls-lib` previously in `.gitignore` no longer exists. Instead, we can simply ignore the entire TARGET_IGNORE.
The Mbed TLS importer accidentally imports Makefile and .gitignore from Mbed TLS's `library/` directory. This commit restricts the pattern to .h and .c files only, and removes the unnecessary files.
Mbed TLS 2.24.0 has added a new function mbedtls_ecp_write_key() which is the reverse of the existing mbedtls_ecp_read_key(). This function should be platform agnostic, but needs to be copied into Cypress's hardware-accelerated ECP driver as part of the updated API.
…ET_TFM/TARGET_TFM_LATEST "
Mbed TLS 2.24.0 requires a few new macros and an inline function in the PSA Crypto header. This PR adds them to make sure the TF-M v1.0 target (specifically CYTFM_064B0S2_4343W) continues to compile with the new Mbed TLS. Note: Support for older versions of TF-M than v1.2 will be dropped, so existing TF-M targets should migrate to TF-M v1.2 as soon as possible.
@Patater
Is it possible to modify the rules to not check |
@ARMmbed/mbed-b-tools |
@@ -144,7 +144,7 @@ | |||
], | |||
"extra_labels": [ | |||
"TFM", | |||
"TFM_V1_1", | |||
"TFM_LATEST", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that using a generic name like TFM_LATEST
simplifies the maintenance but it is less explicit than before. Since you explained in your documentation PR how to map TFM_LATEST
to the actual TFM release that is supported, this should be fine.
"Travis CI - Branch" passed, so the Travis coverage is already complete, even though "Travis CI - Pull Request" did not finish due to the frozen tools check. |
Jenkins CI Test : ✔️ SUCCESSBuild Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
@0xc0170 @adbridge The Core team have done the review. Now only a maintainer's review/approval is left. @ARMmbed/team-cypress @ifyall |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 files permission change, otherwise LGTM
@@ -3,6 +3,8 @@ | |||
# Copyright 2017 Linaro Limited | |||
# Copyright (c) 2017-2019, Arm Limited. | |||
# | |||
# SPDX-License-Identifier: Apache-2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
100644 → 100755 - can you revert the permission change for this and file below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
755 (executable bit) is the latest permission in the upstream project: https://github.com/ARMmbed/trusted-firmware-m/tree/mbed-tfm-1.2/bl2/ext/mcuboot/scripts
and those files are imported. Can we keep them as they are?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, interesting they changed it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea it was changed between TF-M v1.1 -> v1.2
Summary of changes
This PR brings in all the changes to update trusted-firmware-m support from v1.1 to v1.2:
psa_set_key_enrollment_algorithm()
- deprecated, included for backward-compatibility only, to be removed from future TF-M updatesJust as before this update
The following have been removed:
<TARGET>_NS
aliases for ARM_MUSCA_B1 and ARM_MUSCA_S1Not yet included in this PR, to be added in the very near future:
Impact of changes
See the PR description for targets that are impacted.
Migration actions required
From the perspective of Mbed OS support, applications for ARM_MUSCA_B1 and ARM_MUSCA_S1 should continue to work as before.
A board running the TF-M v1.1 firmware should be able to update to the v1.2 firmware without resetting data (verified on Musca targets), though downgrading is not possible.
Any references to
ARM_MUSCA_B1_NS
andARM_MUSCA_S1_NS
as target names should be changed to have_NS
removed from the names.Documentation
The updated steps to build and test TF-M v1.2 are in
README.md
in mbed-os-tf-m-regression-tests (on thetf-m-v1.2-integration
branch currently, to be merged after this PR).The porting documentation is updated by ARMmbed/mbed-os-5-docs#1416, to be merged after this PR. On the next Mbed OS release, the update will become visible on https://os.mbed.com/docs/mbed-os/latest/porting/porting-security.html
Pull request type
Test results
Nightly CI passes for the tf-m-v1.2-integration branch of mbed-os-tf-m-regression-tests
Compilation tests of mbed-os-example-psa for other PSA targets including Mbed PSA targets (e.g. K64F) and the TF-M v1.0 target CYTFM_064B0S2_4343W.
Reviewers
@evedon @jainvikas8 @ARMmbed/mbed-os-security