MbedTLS TLS 1.3 PSA dependency

[tomvaneyck]

I'm trying to implement the Global Platform TLS Sockets API using MbedTLS. I want to limit the implementation to TLS 1.3¹ for now.

As the features that enable TLS 1.3 require the use of functions defined by/in PSA [1], at least in MbedTLS v3.4.0, I added MBEDTLS_PSA_CRYPTO_Cto the configuration header.

However, and this is the issue, the files implementing these PSA functions have been removed while importing MbedTLS in commit dfafe50. This seems to have been the default action since one of the first imports of MbedTLS. Was this done just to remove unnecessary code, or was there a more fundamental reason? In other words, would the inclusion of those files pose issues for the rest of OP-TEE OS?

As an aside, I would like to submit a pull request once I've got a semi-decent version. I wonder, is there still some interest in adding such a wrapper to OP-TEE OS? Thanks!

[1] TLS 1.3 support in MbedTLS v3.4.0

Footnotes

1. This seems to be allowed by the API specification (v1.0.3, section C.3). ↩

[jforissier]

Hi @tomvaneyck,

The PSA functions were removed only for simplicity and ease of maintenance and because there were not strictly needed. However your use case seems totally valid. I believe the right thing to do would be to upgrade MbedTLS as we do from time to time, but this time keep the PSA API add it to the build. The process is described at https://optee.readthedocs.io/en/latest/building/gits/optee_os.html#import-branches. I can create the import branch ; then would you mind doing the rest and opening a pull request against that import branch?
I noticed the latest release is v3.6.0, would that fit your purpose?

[tomvaneyck]

Hello @jforissier

Thanks for the response! I'm fine with going through the import process. v3.6.0 should be good as well.

[tomvaneyck]

I've started the import process on branch import/mbedtls-3.6.0 on my fork. Currently it is able to compile.

There were quite some changes between v3.4 and v3.6 in for example bignum.c, so I had to adjust some commits from the previous import process.
It would be great if someone that knows more about these things could check what I did.

[jforissier]

@tomvaneyck thanks for taking care of this! After a quick glance I would say the series looks good, however it did not pass my testing:

jerome@builder:~/work/optee_repo_qemu_v8/build$ make -j$(nproc) CFG_CRYPTOLIB_NAME=mbedtls CFG_CRYPTOLIB_DIR=lib/libmbedtls XTEST_ARGS="regression_1004" check
jerome@builder:~/work/optee_repo_qemu_v8$ cat out/bin/serial0.log | ./optee_os/scripts/symbolize.py -d optee_os/out/arm/core
[...]
Test ID: regression_1004
Run test suite with level=0

TEE test application started over default TEE instance
######################################################
#
# regression+pkcs11
#
######################################################

* regression_1004 Test User Crypt TA
D/TC:? 0 tee_ta_init_pseudo_ta_session:297 Lookup pseudo TA cb3e5ba0-adf1-11e0-998b-0002a5d5c51b
D/TC:? 0 ldelf_load_ldelf:110 ldelf load address 0x40007000
D/LD:  ldelf:142 Loading TS cb3e5ba0-adf1-11e0-998b-0002a5d5c51b
D/TC:? 0 ldelf_syscall_open_bin:163 Lookup user TA ELF cb3e5ba0-adf1-11e0-998b-0002a5d5c51b (early TA)
D/TC:? 0 ldelf_syscall_open_bin:167 res=0xffff0008
D/TC:? 0 ldelf_syscall_open_bin:163 Lookup user TA ELF cb3e5ba0-adf1-11e0-998b-0002a5d5c51b (Secure Storage TA)
I/TC: WARNING (insecure configuration): Failed to get monotonic counter for REE FS, using 0
I/TC: WARNING (insecure configuration): Failed to commit dirh counter 2
D/TC:? 0 ldelf_syscall_open_bin:167 res=0xffff0008
D/TC:? 0 ldelf_syscall_open_bin:163 Lookup user TA ELF cb3e5ba0-adf1-11e0-998b-0002a5d5c51b (REE)
E/TC:0 0
E/TC:0 0 Core data-abort at address 0x108 (translation fault)
E/TC:0 0  esr 0x96000006  ttbr0 0x400000e1d4000   ttbr1 0x00000000   cidr 0x0
E/TC:0 0  cpu #0          cpsr 0x80000004
E/TC:0 0  x0  0000000000000100 x1  0000000000000009
E/TC:0 0  x2  00000000bc3129b4 x3  0000000000000020
E/TC:0 0  x4  00000000bc3129d4 x5  0000000000000100
E/TC:0 0  x6  00000000bc2a2550 x7  000000000001c240
E/TC:0 0  x8  00000000bc316fa0 x9  00000000bc2f64d0
E/TC:0 0  x10 0000000000000000 x11 0000000000000000
E/TC:0 0  x12 0000000000000000 x13 0000000040006f80
E/TC:0 0  x14 0000000000000000 x15 0000000000000000
E/TC:0 0  x16 00000000bc32ab98 x17 0000000000000000
E/TC:0 0  x18 0000000000000000 x19 0000000000000100
E/TC:0 0  x20 0000000000000000 x21 0000000000000009
E/TC:0 0  x22 0000000000000020 x23 00000000bc3129b4
E/TC:0 0  x24 00000000bc3129d4 x25 0000000000000100
E/TC:0 0  x26 00000000bc3129d4 x27 00000000bc32aa10
E/TC:0 0  x28 00000000bc32a9d0 x29 00000000bc32a900
E/TC:0 0  x30 00000000bc2a2588 elr 00000000bc2a2e20
E/TC:0 0  sp_el0 00000000bc32a900
E/TC:0 0 TEE load address @ 0xbc252000
E/TC:0 0 Call stack:
E/TC:0 0  0xbc2a2e20 mbedtls_rsa_get_bitlen at optee_os/lib/libmbedtls/mbedtls/library/rsa.c:1026
E/TC:0 0  0xbc291c14 sw_crypto_acipher_rsassa_verify at optee_os/lib/libmbedtls/core/rsa.c:795
E/TC:0 0  0xbc25ea44 shdr_verify_signature at optee_os/core/crypto/signed_hdr.c:97
E/TC:0 0  0xbc26e100 ree_fs_ta_open at optee_os/core/kernel/ree_fs_ta.c:271
E/TC:0 0  0xbc267854 ldelf_syscall_open_bin at optee_os/core/kernel/ldelf_syscalls.c:166
E/TC:0 0  0xbc2579c4 scall_do_call at optee_os/core/arch/arm/kernel/arch_scall_a64.S:140
E/TC:0 0  0xbc257690 thread_scall_handler at optee_os/core/arch/arm/kernel/thread.c:1138
E/TC:0 0  0xbc256340 el0_svc at optee_os/core/arch/arm/kernel/thread_a64.S:850
E/TC:0 0 Panic 'unhandled pageable abort' at core/arch/arm/kernel/abort.c:582 <abort_handler>
E/TC:0 0 TEE load address @ 0xbc252000
E/TC:0 0 Call stack:
E/TC:0 0  0xbc25a264 print_kernel_stack at optee_os/core/arch/arm/kernel/unwind_arm64.c:89
E/TC:0 0  0xbc2685f4 __do_panic at optee_os/core/kernel/panic.c:73
E/TC:0 0  0xbc2591b8 abort_handler at optee_os/core/arch/arm/kernel/abort.c:582
E/TC:0 0  0xbc256470 el1_sync_abort at optee_os/core/arch/arm/kernel/thread_a64.S:964
I/TC: Halting CPU 1

How did you test on your side? Any idea what's wrong before I dig further in the code?

[tomvaneyck]

I quickly tested it with something I was doing on the side, did not come to thourough testing yet. I'll take a look tomorrow and let you know what I find!

[tomvaneyck]

Sorry for the delay.

I've fixed the above test, but I had to increase the thread stack size to prevent stack overflows in that same and some other tests. I don't know if this is to be expected or not.
Additionally, MbedTLS originally set the minimum rsa key size at 128 bits, but now provides a config value with a default of 1024 bits. I reset that value back to 128 to make sure regression_4007 succeeds.

[tomvaneyck]

All tests succeed, except for pkcs11_1021 and pkcs11_1026, which also don't succeed on the master branch. @jforissier, I think I'm ready for creating a pull request.

[jforissier]

All tests succeed, except for pkcs11_1021 and pkcs11_1026, which also don't succeed on the master branch. @jforissier, I think I'm ready for creating a pull request.

@tomvaneyck that's good, please do so. I have created branch import/mbedtls-3.6.0 so please use that as the target branch for your pull request. Thanks!

[tomvaneyck]

@tomvaneyck that's good, please do so. I have created branch import/mbedtls-3.6.0 so please use that as the target branch for your pull request. Thanks!

I think the branch name is wrong, it seems to be called import/mbedtls-2.6.0 😅.

[jforissier]

@tomvaneyck that's good, please do so. I have created branch import/mbedtls-3.6.0 so please use that as the target branch for your pull request. Thanks!

I think the branch name is wrong, it seems to be called import/mbedtls-2.6.0 😅.

Oops! Fixed 😉

[github-actions]

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