Update to TF-M v1.4.0 #15050
Merged
Update to TF-M v1.4.0 #15050
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ciarmcom
added
the
release-type: patch
|
@rwalton-arm, thank you for your changes. |
ciarmcom
added
needs: review
devices: arm
component: tools
devices: nuvoton
component: security
component: core
stale
|
This pull request has automatically been marked as stale because it has had no recent activity. @ARMmbed/mbed-os-maintainers, please complete review of the changes to move the PR forward. Thank you for your contributions. |
Closed
rwalton-arm
force-pushed
the
TF-Mv1.4.0
branch
3 times, most recently
from
58aa9d7 to
8b97701
Compare
rwalton-arm
force-pushed
the
TF-Mv1.4.0
branch
from
8b97701 to
ce0ffa1
Compare
LDong-Arm
previously requested changes
Sep 13, 2021
There was a problem hiding this comment.
@rwalton-arm Thanks, it looks good.
In #14977 I forgot to mention we need to ensure Mbed TLS compatibility. The way to check this is build mbed-os-example-tls for those three targets. So far I can think of the following that need to be done:
- Cherry-pick compatibility patches in imported PSA headers. There are four patches in the link, but only two of them got overridden by the import scripts (032fe4a and e5230c9) so those two need to be cherry picked. (We need those patches because TF-M maintains its own PSA headers rather than using the ones from Mbed TLS, and headers from the two repos are always inconsistent no matter what revisions we check out...)
- mbedtls/psa_util.h from Mbed TLS 2.25 (which we have in Mbed OS) contains support for older algorithms (e.g. MD2, MD4, etc.), which have been dropped from the latest TF-M and Mbed TLS (see TF-M 1.4 commit and Mbed TLS 3.0 PR). Those algorithms are not enabled in Mbed OS (see connectivity/mbedtls/include/mbedtls/config.h) so we're okay. But some configs in mbed-os-example-tls do have MD4 enabled and cause a build error, shall we disable it in the example? @Patater
-
PSA_ALG_AEAD_WITH_TAG_LENGTHhas been replaced byPSA_ALG_AEAD_WITH_SHORTENED_TAGin the latest PSA (see commit) but Mbed TLS 2.25 uses the old one. Adding back the old macro for compatibility fixes the problem.
I only tried to build the "benchmark" example in mbed-os-example-tls on Musca S1 and not anything else. Please make sure to try all of them on those targets & toolchains, and we might spot more things to fix!
MD4 is rather insecure. Now that mbedtls have deprecated the obsolete algorithms, I don't see a reason to try and continue supporting them for the example. IMO we should just disable them in the example. |
mergify
bot
dismissed
LDong-Arm’s stale review
Pull request has been modified.
LDong-Arm
reviewed
Sep 14, 2021
rwalton-arm
force-pushed
the
TF-Mv1.4.0
branch
2 times, most recently
from
a5b3d14 to
13ee303
Compare
tfm_ns_interface.c is intended to be overriden by clients to support different targets. We copy this file from upstream into the mbed-os platform library. We also have a specific "strong" overridden version for the NU_M2354 target, which is located in its target library. Previously the implementations in the platform library were decorated with __attribute__(weak), and we provided a strong definition for the NU_M2354 target. This worked fine because of weak linking, the linker will pick the first "strong" definition and use that, avoiding any ODR violations. However, upstream have removed __attribute__(weak) from the function definitions, which caused multiply defined symbol errors when trying to build the NU_M2354 target. To work around the above issue, we remove the common definition in the platform library; instead we copy the file to the Musca B1 and Musca S1 target libaries. This means the appropriate tfm_ns_interface.c is only included in the build when compiling for the specific target which uses it.
The directory structure upstream has changed. Now ARM_MUSCA board support has been moved under an "arm" subdirectory. Update targets.json with the new locations.
Include mbedtls_ecc_group_to_psa.h from crypto_extra.h so that clients of PSA within Mbed OS do not need to behave differently depending on which PSA implementation they are using. This solution is not ideal as it makes it more difficult to update the TF-M-provided psa/crypto_extra.h. We'll have to see what other options we have for including additional headers based on the Mbed OS configuration.
We have added definitions that are needed by Mbed TLS's PSK key exchange but missing from TF-M's PSA to `mbedtls_svc_key_id.h`. To pick up those definitions, TF-M's `psa/crypto_values.h' needs to include `mbedtls_svc_key_id.h`.
PSA_ALG_AEAD_WITH_TAG_LENGTH has been replaced with PSA_ALG_AEAD_WITH_SHORTENED_TAG upstream. We could just update psa_util.h to use the new macro, but we still have some targets that only support older versions of PSA, so we reinstate the removed macro.
We worked around an issue with mbed-cli1 not recognising the BL2 macro from targets.json by adding patched versions of region_defs.h and flash_layout.h for ARM_MUSCA targets. In the patched headers we defined the BL2 macro to ensure it can be picked up by the ARM scatter files that include the headers. The current solution is not robust, because it means that the aforementioned headers easily become out of date. A workaround of defining the macros in the scatter files which need them was suggested in ARMmbed#14762 This commit applies the suggested changes to the ARM_MUSCA scatter files.
Import the latest partition headers from upstream. We no longer need to patch the headers to define the BL2 macro as we now define it in the scatter files for the MUSCA targets.
rwalton-arm
force-pushed
the
TF-Mv1.4.0
branch
from
13ee303 to
a43103f
Compare
mergify
bot
dismissed
LDong-Arm’s stale review
Pull request has been modified.
Jenkins CI Test : ✔️ SUCCESSBuild Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
|
Started CI after the force push |
Jenkins CI Test : ✔️ SUCCESSBuild Number: 3 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
Patater
approved these changes
Sep 15, 2021
Merged
mbedmain
removed
release-type: patch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes
Bring the latest changes from TF-M v1.4.0 into mbed-os.
Update the Nuvoton python scripts to bump the hard coded TF-M version.
Update locations of ARM_MUSCA target support in targets.json.
Impact of changes
Migration actions required
Documentation
Pull request type
Test results
Reviewers