tls: Upgrade to Mbed TLS v2.25.0 #14652
Conversation
ciarmcom
added
the
release-type: patch
|
@Patater, thank you for your changes. |
Jenkins CI Test : ❌ FAILEDBuild Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
|
@Patater We need to backport a number of definitions to TF-M's PSA headers. I encountered the same issue when trying to compile for CYTFM_064B0S2_4343W and ARM_MUSCA_S1. To reproduce the issue, either use Mbed CLI 1, or add Alternatively, should we force use the Mbed TLS version of all PSA headers and remove ones from TF-M? |
| ret = mbedtls_ecp_gen_key( grp_id, &ecp, | ||
| mbedtls_ctr_drbg_random, |
There was a problem hiding this comment.
For Mbed PSA targets (e.g. K64F), we get undefined symbol error for mbedtls_ecp_gen_key().
This call here is inside #if defined(MBEDTLS_PSA_BUILTIN_KEY_TYPE_ECC_KEY_PAIR), which is enabled by the newly added connectivity/mbedtls/include/mbedtls/config_psa.h if MBEDTLS_ECP_C is defined.
The definition of mbedtls_ecp_gen_key() in connectivity/mbedtls/source/rsa.c requiresMBEDTLS_GENPRIME. But our script unsets it:
I wonder if we should enable MBEDTLS_GENPRIME globally (if current targets support it), or enable it for Mbed PSA targets, or disable MBEDTLS_PSA_BUILTIN_KEY_TYPE_ECC_KEY_PAIR to avoid the call?
There was a problem hiding this comment.
It might be interesting to see whether mbedtls_ecp_gen_key() was called prior to the update. Though because we had something before doesn't automatically mean it was the most ideal.
There was a problem hiding this comment.
We'd like to avoid enabling GENPRIME_C if possible, but still be able to use PSA to import RSA keys or sign with them (at least verify would be good). I'll dig to see if this is a permitted configuration.
| @@ -0,0 +1,993 @@ | |||
| /* | |||
There was a problem hiding this comment.
This file needs to be added to platform/FEATURE_EXPERIMENTAL_API/FEATURE_PSA/TARGET_MBED_PSA_SRV/CMakeLists.txt.
Patater
force-pushed
the
upgrade-mbedtls-2.25.0
branch
from
4a0fc4a to
db005d0
Compare
| #define MBEDTLS_PSA_BUILTIN_ALG_RSA_PSS 1 | ||
| #define PSA_WANT_ALG_RSA_PSS 1 | ||
| #endif /* MBEDTLS_PKCS1_V21 */ | ||
| #define MBEDTLS_PSA_BUILTIN_KEY_TYPE_RSA_KEY_PAIR 1 |
There was a problem hiding this comment.
This part of the configuration seems to be causing linker errors with mbed-os-example-crypto, as it is used to replace MBEDTLS_RSA_C and MBEDTLS_GENPRIME in ifdefs, but it only requires RSA_C to be enabled here. An example where it relies on MBEDTLS_GENPRIME (for mbedtls_rsa_gen_key):
Wrapping this line with an
#ifdef MBEDTLS_GENPRIME seems to result in the same behavior of the example as before this upgrade was introduced.
There was a problem hiding this comment.
I came to the same conclusion and raised Mbed-TLS/mbedtls#4513
We can workaround this issue until the fix is available upstream.
Until we have a fix for Mbed-TLS/mbedtls#4512, we need to patch the fix during import time. Otherwise, we run into linker errors when PSA attempts to use RSA key generation, which we've excluded. This patch is extracted from Mbed-TLS/mbedtls#4513
Patater
force-pushed
the
upgrade-mbedtls-2.25.0
branch
from
db005d0 to
f275a83
Compare
There was a problem hiding this comment.
@Patater Thanks, the fix looks good to me.
@Patater We need to backport a number of definitions to TF-M's PSA headers. I encountered the same issue when trying to compile for CYTFM_064B0S2_4343W and ARM_MUSCA_S1.
To reproduce the issue, either use Mbed CLI 1, or add
mbed-mbedtlsto an application'sCMakeLists.txtif usingmbed-tools. Here's a quick fix: LDong-Arm@cfe2c48 (I haven't polished the commit message yet.)Alternatively, should we force use the Mbed TLS version of all PSA headers and remove ones from TF-M?
We still need to address this for TF-M targets. They are similar to the mbedtls_ecc_group_to_psa() issue we worked around previously, but now we have multiple definitions to handle instead of just one...
In order for Mbed TLS to use the PSA Crypto API, definitions of `MBEDTLS_SVC_KEY_ID_INIT`, `mbedtls_svc_key_id_t` and `mbedtls_svc_key_id_is_null()` need to be present but are not provided by the PSA headers from TF-M. To solve this issue, this commit copies those definitions from Mbed TLS's original `psa/crypto_types.h` and `psa/crypto_values.h` into a separate `mbedtls_svc_key_id.h` for TF-M PSA.
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`.
Jenkins CI Test : ❌ FAILEDBuild Number: 3 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
|
connectivity-netsocket-tests-tests-network-interface failed on NUCLEO_F767ZI: I happen to have this target at home, will try it locally tomorrow. |
|
I just double checked by running the failing test on CI boards, from my machine. Two of the three NUCLEO_F767ZI labelled with @ARMmbed/mbed-os-maintainers This needs to be fixed before we rerun CI on this and any other PRs. |
|
CI Started |
Jenkins CI Test : ✔️ SUCCESSBuild Number: 4 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
|
@Patater it would be nice if there was a bullet point summary of what this version of TLS actually brings (as this will go into the release notes for the next release). Also if there is any impact or any migrations required...... |
|
Added link to the Mbed TLS v2.25.0 release notes in the description |
mbedmain
added
release-version: 6.12.0
Release-pending
and removed
release-type: patch
Summary of changes
Upgrade to Mbed TLS v2.25.0
Please refer to the Mbed TLS v2.25.0 release notes at https://github.com/ARMmbed/mbedtls/releases/tag/v2.25.0 for more information about what comes with this release.
Impact of changes
None
Migration actions required
None
Documentation
None
Pull request type
Test results
Reviewers