OP-TEE / optee_os

Trusted side of the TEE
Other
1.6k stars 1.07k forks source link

Bump OP-TEE major version to 4 #6032

Closed jenswi-linaro closed 1 year ago

jenswi-linaro commented 1 year ago

With the OP-TEE 3.x releases we've accumulated a bit of compatibility code. Some of the code has been untested for quite a few releases now. I'd like to propose bumping the major version of OP-TEE to 4 and getting rid of some old stuff.

We should try to avoid breaking things more than necessary so in most cases, TAs should just continue to work. It's only if they are old and depending on compatibility that they may break. In most cases, it should be enough to just recompile the TA. Regardless a 4.0.0 release might be disruptive so we should prepare ahead carefully. Targetting not the next release but the release after the next release should give as much time as possible to sort out eventual problems before the 4.0.0 release.

I'm going to keep track of the changes we'd like to bring into 4.0.0 in my OP-TEE OS repository https://github.com/jenswi-linaro/optee_os, branch wip_4_0_0 (https://github.com/jenswi-linaro/optee_os/tree/wip_4_0_0). Changes that we'd like to discuss or prepare for 4.0.0 can be proposed with a pull request to that branch. Changes on the wip_4_0_0 branch will not automatically make it into an eventual 4.0.0 release, they will still need to be merged via normal pull requests. The purpose of the wip_4_0_0 is to keep track of what's on the table for 4.0.0.

I'll add comments to this issue as I propose changes. Feel free to propose cleanup changes for 4.0.0, either to be discussed here or in a pull request to my wip_4_0_0 branch.

The first out is https://github.com/jenswi-linaro/optee_os/pull/8, please let me know if you think it makes sense.

Edit: I'm doing the same for optee_test using my own optee_test repository https://github.com/jenswi-linaro/optee_test, branch wip_4_0_0 (https://github.com/jenswi-linaro/optee_test/tree/wip_4_0_0).

jenswi-linaro commented 1 year ago

Updating to mbedtls 3.4.0 https://github.com/jenswi-linaro/optee_os/pull/9, some issues remain to be resolved.

clementfaure commented 1 year ago

Hello @jenswi-linaro , Just one suggestion about OPTEE 4.x.x and the embedded OpenSSL lib in optee-test. I remember a quick discussion I had with @jforissier about the embedded OpenSSL lib being outdated : https://github.com/OP-TEE/optee_test/issues/592#issuecomment-1083286593 It could be the opportunity to remove it from optee-test ?

jenswi-linaro commented 1 year ago

@clementfaure, good point, I'll add this to the list.

jenswi-linaro commented 1 year ago

Relaxed location of the TA header in https://github.com/jenswi-linaro/optee_os/pull/10

jenswi-linaro commented 1 year ago

Removing the embedded openssl copy from optee_test in https://github.com/jenswi-linaro/optee_test/pull/2

jenswi-linaro commented 1 year ago

Added Wip/mbedtls 3.4.0 take2, https://github.com/jenswi-linaro/optee_os/pull/11 as an alternative to https://github.com/jenswi-linaro/optee_os/pull/9.

clementfaure commented 1 year ago

Hi @jenswi-linaro Just one question, what is the reason behind having two software crypto libraries today in optee-os? Could optee 4.x be the opportunity to only have mbedtls?

jenswi-linaro commented 1 year ago

I guess the short answer is that someone wanted to include mbedtls really bad so we went along. However, we're now stuck on the 2.x branch due to API changes. When trying to upgrade to 3.x in https://github.com/jenswi-linaro/optee_os/pull/9 a few more issues with the 3.x branch was discovered (see the PR). At the moment it looks like we're going to switch entirely to LTC for OP-TEE core, only using the bignum implementation from mbedtls and we'll upgrade mbedtls for TAs to 3.x.

clementfaure commented 1 year ago

Thanks for the explanation @jenswi-linaro

jenswi-linaro commented 1 year ago

Created https://github.com/OP-TEE/optee_os/pull/6316 to merge upstream.

jenswi-linaro commented 1 year ago

Created https://github.com/OP-TEE/optee_os/pull/6317 to merge upstream.

clementfaure commented 1 year ago

Hi @jenswi-linaro, I know it's too late to add any big change for 4.0 but let's open the discussion: Do we really want to maintain both Makefile and CMake for optee-test ? Currently, Buildroot uses CMake and Yocto uses Makefile.

jenswi-linaro commented 1 year ago

Hi @clementfaure, I'm not sure that relates to 4.0 since the OP-TEE version is mainly about compatibility with TAs. The normal world ABI will always be compatible.

How optee_test is built is quite independent from that so it can happen after 4.0 if needed. I wouldn't mind getting rid of CMake if one is to be picked, but that might be more of a personal preference than a technical argument.

clementfaure commented 1 year ago

I see what you mean. If such change is independent of a major version bump, that's even better. Firstly, I should have asked why we have both CMake and Makefile for optee-test? Maybe I'm missing something?

I'm asking because we realized that CMake and Makefile does not even support the same features. For instance, Makefile supports NIST test vectors, but CMake does not : https://github.com/OP-TEE/optee_test/pull/698

jenswi-linaro commented 1 year ago

The CMake support was added when we started to use buildroot, but buildroot doesn't really require CMake so we could just as well use normal make files. We would save some build time if we could get rid of CMake.

jenswi-linaro commented 1 year ago

Closing this as 4.0.0-rc1 has now been tagged. This issue resulted in the following pull requests: