zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.48k stars 6.41k forks source link

Integration of the OP-TEE client open header files with needed definitions #61031

Open oleksiimoisieiev opened 1 year ago

oleksiimoisieiev commented 1 year ago

Origin

Integration of the OP-TEE open headers in terms of the PR which introducing OP-TEE driver to Zephyr.

Purpose

OP_TEE is a Trusted Execution Environment (TEE) which was designed to run Trusted Applications (TAs) secured from the Rich Execution Environment (REE) using ARM TrustZone technology. See Docs for details. The proposed changes are introducing OP-TEE driver and TEE subsystem to communicate with OP-TEE from Zephyr REE.

Mode of integration

The following headers should be integrated into the Zephyr main tree:

  1. tee.h - Generic API to TEE subsystem that should be implemented by OP-TEE driver origin: optee_client revision: 3.18.0 place in Zephyr main tree: include/zephyr/drivers
  2. optee_msg.h - defines the OP-TEE message protocol to communicate with OP-TEE instance running in Secure World origin: optee_os revision: 3.18.0 place in Zephyr main tree: drivers/tee/optee
  3. optee_rpc_cmd.h - RPC defines and structure to communicate with Secure World. origin: optee_os revision: 3.18.0 place in Zephyr main tree: drivers/tee/optee
  4. optee_smc.h - defines SMC interface between Normal World and Secure World origin: optee_os revision: 3.18.0 place in Zephyr main tree: drivers/tee/optee

Maintainership

Those headers are maintained by Linaro (according to the license information from the header License comment). But they had to be changed to match Zephyr code style and to be compatible with existing Zephyr definitions and the Device Driver model. That's why I propose myself as a maintainer candidate. These external sources were added in terms of the new feature, which should have a maintainer.

Pull Request

The following PR Introduces TEE subsystem support and OP-TEE OS driver to Zephyr.

Description

Global Platform provides the Technology of the Trusted Execution Environment (TEE) see link for the details. TEE is the secure area on the main processor that ensures that all sensitive data is stored, processed and protected in an isolated and trusted environment. TEE offers save execution of the authorized security software (Trusted Applications - TAs) and protects device and TA assets. GlobalPlatform defines the following security features:

  1. Isolation from Rich OS;
  2. Isolation from Other TAs;
  3. App management control;
  4. Identifications and binding;
  5. Trusted Storage;
  6. Trusted Access to the peripherals;
  7. Cryptography.

OP-TEE OS is an implementation of TEE using ARM TrustZone technology: link. It provides OS running on the secure privileged layer, set of secure user space libraries for TAs and API for REE to manipulate with TAs via SMC. Zephyr OP-TEE driver implements SMC connection to the OP-TEE OS and implements TEE Internal Core API .

This allows using main TEE features, mentioned above. optee_client library, ported as Zephyr Module (will be posted after merging OP-TEE driver) link implements TEE Client API with tee-supplicant support, which should provide TAs from the user-space and manage Secure Storage.

Changes may be tested by using ported xtest: zephyr-optee-test

TEE allows Zephyr application to use TAs functionality, store secure keys and support crypto standards, such as PKCS11. OP-TEE also provides Key generation functionality and storage. TA's could be provided by Zephyr application or to be compiled inside OP-TEE.

Dependencies

OP-TEE driver, according to About page should be compatible with TEE Internal Core API v1.3.1 and TEE Client API v1.0. So OP-TEE OS and OP-TEE client library should be compatible with the same API revisions.

There is no direct dependency for Zephyr. It provides API abstraction for the Client library and has SMC interface to the OP-TEE OS.

Revision

Implementation was based on revision 3.18.0 of optee_os and optee_client. Please see Mode of integration section for the details.

License

All headers are licensed under: BSD-2-Clause

github-actions[bot] commented 1 year ago

Hi @oleksiimoisieiev! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

galak commented 1 year ago

@carlescufi any guide on this being in tree vs treating this as module like we do for CMSIS headers.

sjg20 commented 1 year ago

It seem quite small, so I think this is better to be in-tree.

carlescufi commented 1 year ago

@oleksiimoisieiev this is the outcome of the TSC discussion:

Integration of the OP-TEE client open header files with needed definitions [Decide] First with BSD-2-Clause OSI approved Take it by the license review process to introduce a license that hasn’t previously been introduced to Zephyr? // See https://docs.zephyrproject.org/latest/contribute/external.html TSC can vote to approve; then pass through the Board for review/approval Why in-tree versus being in a module like the CMSIS headers? (see comment) CMSIS was put in a module as it’s already a repo itself. It’s also shared by a lot of the HALs Discussion conclusion: Would be good if the person who proposed the PR replied with additional motivation/justification around approach // Continue with review + feedback

Can you please provide a bit more motivation/justification around the approach you took? i.e. why in-tree vs as a module

carlescufi commented 1 year ago

@carlescufi any guide on this being in tree vs treating this as module like we do for CMSIS headers.

My personal opinion is that this belongs in-tree because of the limited number of files, just like we did with Xen. But there are no real rules, it's left to the implementor and reviewers to decide.

oleksiimoisieiev commented 12 months ago

@oleksiimoisieiev this is the outcome of the TSC discussion:

Integration of the OP-TEE client open header files with needed definitions [Decide] First with BSD-2-Clause OSI approved Take it by the license review process to introduce a license that hasn’t previously been introduced to Zephyr? // See https://docs.zephyrproject.org/latest/contribute/external.html TSC can vote to approve; then pass through the Board for review/approval Why in-tree versus being in a module like the CMSIS headers? (see comment) CMSIS was put in a module as it’s already a repo itself. It’s also shared by a lot of the HALs Discussion conclusion: Would be good if the person who proposed the PR replied with additional motivation/justification around approach // Continue with review + feedback

Can you please provide a bit more motivation/justification around the approach you took? i.e. why in-tree vs as a module

Hello,

My motivation for selecting these headers was to include only the necessary definitions for OP-TEE communication, without incorporating any actual functionality. OP-TEE does not provide any standard module that can be connected to Zephyr as an external module and offer these headers as part of the main OP-TEE repository. As you probably noticed, "tee.h" was sourced from the optee-client repository, while the others were obtained from optee-os, and they represent different communication interfaces (Driver -> OP-TEE and External user -> Driver). I don't believe it's a good idea to place them in the same module. If I am correct, then I would need to create two modules with inner and outer interfaces. This approach seems overly complex to me.

It's a common practice to include these headers in-tree; for example, the same approach was used in the Linux Kernel.

Another point to note is that these headers are licensed under the BSD-2 License, which is compatible with the Apache-2 License.

Please let me know if you need any further assistance or have additional questions!

Best regards, Oleksii.

carlescufi commented 10 months ago

@oleksiimoisieiev this will be brought up in the next TSC. See https://github.com/zephyrproject-rtos/zephyr/wiki/Technical-Steering-Committee-%28TSC%29 if you would like to join.

oleksiimoisieiev commented 10 months ago

@oleksiimoisieiev this will be brought up in the next TSC. See https://github.com/zephyrproject-rtos/zephyr/wiki/Technical-Steering-Committee-%28TSC%29 if you would like to join.

Hi @carlescufi,

Thank you for the link. I'll be in vacation next week, but I'll try to join.

carlescufi commented 10 months ago

@oleksiimoisieiev this will be brought up in the next TSC. See https://github.com/zephyrproject-rtos/zephyr/wiki/Technical-Steering-Committee-%28TSC%29 if you would like to join.

Hi @carlescufi,

Thank you for the link. I'll be in vacation next week, but I'll try to join.

Sorry @oleksiimoisieiev, the TSC was cancelled this week. It will have to wait until next Wednesday (Nov. 1st).

ifyall commented 10 months ago

Given that the files needed to integrate OP-TEE support in to Zephyr require modifications that are specific to Zephyr, it makes sense to me that they are in tree.

github-actions[bot] commented 8 months ago

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.

oleksiimoisieiev commented 8 months ago

Hi @nashif, @carlescufi. I see github-bot have added Stale label to this issue. Is there any actions that should be done from my side?

github-actions[bot] commented 6 months ago

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.

carlescufi commented 3 months ago

Approved by the TSC:

image

github-actions[bot] commented 1 month ago

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.

ithinuel commented 1 month ago

https://github.com/zephyrproject-rtos/zephyr/pull/60680 has been merged. Should this issue be close as completed?