MBEDTLS_ECP_C not build when MBEDTLS_USE_PSA_CRYPTO

[nandojve]

Describe the bug
Currently version of TFM/PSA not build when enable MBEDTLS_ECP_C and MBEDTLS_USE_PSA_CRYPTO. I've been testing using main repository and mps2_an521_ns qemu.

To Reproduce
This is a proprietary implementation which get problem when combine MBEDTLS configurations at user-tls.conf.
Steps to reproduce the behavior:

With below configuration Zephyr App build and works properly, the algorithm works as expected and no issues are detected.

#define MBEDTLS_ENTROPY_C

#define MBEDTLS_ECP_C
#define MBEDTLS_ECP_DP_SECP256R1_ENABLED
#define MBEDTLS_ECP_DP_SECP384R1_ENABLED
#define MBEDTLS_ECDSA_C

#define MBEDTLS_X509_CSR_WRITE_C
#define MBEDTLS_X509_CREATE_C
#define MBEDTLS_PEM_WRITE_C
#define MBEDTLS_PEM_PARSE_C
#define MBEDTLS_BASE64_C
#define MBEDTLS_SHA512_C
#define MBEDTLS_SHA384_C
#define MBEDTLS_SHA256_C
#define MBEDTLS_OID_C
#define MBEDTLS_ASN1_WRITE_C
#define MBEDTLS_PK_C
#define MBEDTLS_PK_PARSE_C
#define MBEDTLS_PK_WRITE_C
#define MBEDTLS_MD_C
#define MBEDTLS_BIGNUM_C

When PSA_CRYPTO is enabled, as below, there are buildings errors.

+ #define MBEDTLS_USE_PSA_CRYPTO

#define MBEDTLS_ENTROPY_C

#define MBEDTLS_ECP_C
#define MBEDTLS_ECP_DP_SECP256R1_ENABLED
#define MBEDTLS_ECP_DP_SECP384R1_ENABLED
#define MBEDTLS_ECDSA_C

#define MBEDTLS_X509_CSR_WRITE_C
#define MBEDTLS_X509_CREATE_C
#define MBEDTLS_PEM_WRITE_C
#define MBEDTLS_PEM_PARSE_C
#define MBEDTLS_BASE64_C
#define MBEDTLS_SHA512_C
#define MBEDTLS_SHA384_C
#define MBEDTLS_SHA256_C
#define MBEDTLS_OID_C
#define MBEDTLS_ASN1_WRITE_C
#define MBEDTLS_PK_C
#define MBEDTLS_PK_PARSE_C
#define MBEDTLS_PK_WRITE_C
#define MBEDTLS_MD_C
#define MBEDTLS_BIGNUM_C

[252/263] Linking C executable zephyr/zephyr_pre0.elf
FAILED: zephyr/zephyr_pre0.elf
...
$HOME/zephyr-sdk-0.13.1/arm-zephyr-eabi/bin/../lib/gcc/arm-zephyr-eabi/10.3.0/../../../../arm-zephyr-eabi/bin/ld.bfd: modules/mbedtls/libmodules__mbedtls.a(pk_wrap.c.obj): in function `psa_set_key_type':
$HOME/workspace/elgin/modules/crypto/mbedtls/include/psa/crypto_struct.h:450: undefined reference to `psa_set_key_domain_parameters'
collect2: error: ld returned 1 exit status
ninja: build stopped: subcommand failed.
FATAL ERROR: command exited with status 1: $HOME/.local/bin/cmake --build $HOME/workspace/elgin/daf-firmware/build --target run

Expected behavior
The MBEDTLS_ENTROPY_C should work with PSA_CRYPTO.

Impact
Impossible to use feature.

Environment (please complete the following information):

• OS: Linux
• Toolchain Zephyr SDK 0.13.1, 0.15.0
• Current Main, v3.2.0
• CC: @erwango @theotherjimmy

[microbuilder]

The next release of TF-M enables MbedTLS 3.1.0, which provides better PSA API integration. There are still a number of issues with the 3.0.0 release used in the current TF-M version we reference in Zephyr.

[nandojve]

Thank you @microbuilder ,

I wondering if is there any release schedule in mind?

[microbuilder]

TF-M has three releases a year, and we can only update after a TF-M release, so there should be an upmerge available in the next two months.

[joerchan]

They switched to 2 releases a year, next release is in May.

Version	Feature Freeze	Release
v1.6.0	2022-4-1	2022-4-15

[carlescufi]

@joerchan do you know if mbedTLS 3.1.0 will fix this issue?

[microbuilder]

@joerchan We could do an upmerge to TF-M, which would bring us up to MbedTLS 3.1 on the secure side, but I'm not sure where we are with all the cherry-picking post 1.5.0?

[joerchan]

We could do an upmerge to TF-M, which would bring us up to MbedTLS 3.1 on the secure side, but I'm not sure where we are with all the cherry-picking post 1.5.0?

They should all just be reverted, so this isn't an issue (except fro the lpcxpresso SDK).

do you know if mbedTLS 3.1.0 will fix this issue?

This does not seem to be a MbedTLS issue at all. From what I can tell this is about not using the TF-M PSA headers when TF-M is enabled.
A problem that I have seen before but I'm not 100% sure how to deal with.

[microbuilder]

@joerchan At some point in the near future, we really need to prioritize using TF-M as the PSA backend. I've heard from Arm that MbedTLS 3.1 should now make it possible to do TLS with the TF-M backend via the PSA APIs, but the MbedTLS management is more in @d3zd3z and @ceolin 's purview. Have you tried connecting the two in the Nordic SDK yet? I don't expect this will easily 'just work'.

[joerchan]

Yes, we are looking into that specific problem. From what I know MBedTLS 3.1 still needs additional changes in order to have all crypto functionality under PSA API and in the Crypto partition in TF-M. We have something that works with parts of the crypto still in the non-secure domain.

PS: Somewhat confusing that the project and repository is called MbedTLS when the library for the tls stack is called mbedtls, so note the capitalization in the following explanation.

The MBedTLS project exposes the three libraries:

• mbedtls (TLS library)
• mbedcrypto (Crypto library)
• mbedx509 (Certificates library

When building with TF-M the mbedtls and mbedx509 libraries will be part of the non-secure image, while mbedcrypto will be included in the secure image TF-M, and is exported through the TF-M crypto partition.
In this configuration the PSA crypto headers has to now be taken from the TF-M project, and not the MBedTLS project.

The explanation for the linker error comes from not using the correct set of PSA crypto headers.
The TLS library has now linked against the PSA crypto headers export by the MBedTLS project, and not TF-M project.

The incorrect header file included from mbedtls library modules/crypto/mbedtls/include/psa/crypto_struct.h:

static inline void psa_set_key_type( psa_key_attributes_t *attributes,
                                    psa_key_type_t type )
{
    if( attributes->MBEDTLS_PRIVATE(domain_parameters) == NULL )
    {
        /* Common case: quick path */
        attributes->MBEDTLS_PRIVATE(core).MBEDTLS_PRIVATE(type) = type;
    }
    else
    {
        /* Call the bigger function to free the old domain paramteres.
         * Ignore any errors which may arise due to type requiring
         * non-default domain parameters, since this function can't
         * report errors. */
        (void) psa_set_key_domain_parameters( attributes, type, NULL, 0 );
    }
}

This includes a call to psa_set_key_domain_parameters which is not defined anyway in the non-secure image.
This leads to the error message in linking: undefined reference to 'psa_set_key_domain_parameters'

The correct header to include modules/tee/tf-m/trusted-firmware-m/interface/include/psa/crypto_struct.h:

static inline void psa_set_key_type(psa_key_attributes_t *attributes,
                                    psa_key_type_t type)
{
    attributes->type = type;
}

A possibly workaround would be to make sure that the TF-M include path is first among the includes.
Example:

zephyr_library_include_directories(
    $<TARGET_PROPERTY:tfm,TFM_BINARY_DIR>/install/interface/include
  )

Not sure where exactly place this workaround, need more information on how to reproduce the error.

[d3zd3z]

PS: Somewhat confusing that the project and repository is called MbedTLS when the library for the tls stack is called mbedtls, so note the capitalization in the following explanation.

As I understand, the official name of the project is "mbed TLS" (note in particular the lowercase 'm' and the space). Sometimes, when writing, they will capitalize the 'm' if it occurs in a place where a word would be capitalized in English (such as the beginning of a sentence). The docs somewhat randomly capitalize the 'm' in other contexts (the probably is a result of an uncapitalized name being unusual in English). (mbed has the same issues)

The run together version 'mbedtls' is commonly used for symbols (within the library), and seems to be how the name is commonly rendered into variable names.

[github-actions]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[nandojve]

Hi folks,

Is there any update about this issue since 3.1.0 was released?

[joerchan]

@nandojve As far as I can see the problem is still there. Your reproduce point is a bit vague, would you be able to clarify, perhaps by providing a change to a zephyr sample that would lead to the error?

[d3zd3z]

I have been unable to build with MBEDTLS_USE_PSA_CRYPTO even after the 3.1.0 release. I get a small number of conflicting symbols. There is a bit deeper of a question as to where the operations actually get implemented, and should all of them be handled by the PSA calls into the trusted side.

[github-actions]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[github-actions]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[nandojve]

I updated internal code to latest Zephyr version last week. I hope I can re-test soon and give a proper feedback.

[microbuilder]

cc @mimok ... see the above comment as well.

[trustngotech]

@microbuilder Thank you for the feedback! This is a major improvement in my opinion as i'm planning to use this feature in a commercial demo.

[microbuilder]

@microbuilder Thank you for the feedback! This is a major improvement in my opinion as i'm planning to use this feature in a commercial demo.

It needs more testing, and I'm sure there are problems to sort out, but please raise issues (even better, PRs) in the upstream TF-M or here in Zephyr if you find any issues. Agree it's a long overdue step forward, though.

[github-actions]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[github-actions]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[github-actions]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.