tee-supplicant: add udev rule and systemd service file #391

mikkorapeli-linaro · 2024-10-03T06:51:31Z

tee-supplicant startup with systemd init based
is non-trivial. Add needed udev rule and systemd
service files here so that distros can co-operate maintaining them.

Files are from meta-arm https://git.yoctoproject.org/meta-arm at commit 7cce43e632daa8650f683ac726f9124681b302a4 with license MIT and authors:

Peter Griffin [email protected]
Joshua Watt [email protected]
Javier Tia [email protected]
Mikko Rapeli [email protected]

The udev rule starts tee-supplicant once optee has been detected via /dev/teepriv[0-9]* device file. The startup expects to find teeclnt system group on the running host. systemd service starts before tpm2.target (new in systemd 256) which starts in initramfs too. This covers firmware TPM TA usecases, and possibly others which are started before main rootfs is mounted. For stopping tee-supplicant, the ftpm kernel modules are removed and only then the main process stopped to avoid fTPM breakage. These workarounds may be removed once RPMB kernel and optee patches
without tee-supplicant are merged.

Cc: Peter Griffin [email protected]
Cc: Joshua Watt [email protected]
Cc: Javier Tia [email protected]

mikkorapeli-linaro · 2024-10-03T07:05:01Z

Trying to change the licenses from MIT to BSD-2-Clause used in optee_client. Asking permission from copyright owners over email.

jforissier

Good initiative! Regarding the MIT license, it's not a problem since it is compatible with BSD-2-Clause, but of course things are even simpler if the files are re-licensed by their authors.
One comment below. Thanks!

tee-supplicant/optee-udev.rules

mikkorapeli-linaro · 2024-10-04T13:53:06Z

Trying to change the licenses from MIT to BSD-2-Clause used in optee_client. Asking permission from copyright owners over email.

Done, changes to BSD-2-Clause

mikkorapeli-linaro · 2024-10-04T13:55:24Z

I went a bit further with CMake standard variable use now. I also think distros compiling optee_client should switch from Makefiles to CMake. Maybe bugs with patches to Fedora, Debian and Ubuntu should be filed....

jforissier

Nice cleanup!

For "tee-supplicant: add udev rule and systemd service file" please apply:
Acked-by: Jerome Forissier <[email protected]>
For "libteec/CMakeLists.txt: remove CFG_TEE_CLIENT_LOAD_PATH comment":
Reviewed-by: Jerome Forissier <[email protected]>

ricardosalveti · 2024-10-04T15:19:18Z

tee-supplicant/CMakeLists.txt

@@ -113,3 +118,7 @@ endif()
 # Install targets
 ################################################################################
 install(TARGETS ${PROJECT_NAME} RUNTIME DESTINATION ${CMAKE_INSTALL_SBINDIR})
+configure_file([email protected] [email protected] @ONLY)
+install(FILES ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/[email protected] DESTINATION ${CMAKE_INSTALL_LIBDIR}/systemd/system)


Should we have a conditional for systemd support? Otherwise the service job and the rules are going to be installed even for users that are not using systemd.

I thought about this but then the current sd-notify changes are also always enabled so I did not add a flag.

Users and packagers of optee_client can also ignore and/or remove some of the files installed. Similar situation should be with documentation, man pages etc.

ricardosalveti · 2024-10-04T15:21:33Z

tee-supplicant/[email protected]

+Group=@CFG_TEE_SUPPL_GROUP@
+EnvironmentFile=-@CMAKE_INSTALL_SYSCONFDIR@/default/tee-supplicant
+ExecStart=@CMAKE_INSTALL_PREFIX@/@CMAKE_INSTALL_SBINDIR@/tee-supplicant $OPTARGS
+ExecStop=-/bin/sh -c "/sbin/modprobe -v -r tpm_ftpm_tee ; /bin/kill $MAINPID"


Probably good to add a comment here (not only in the commit message) explaining why removing the module is needed.

Ok I can do that. Sadly the full story is rather complicated so one liner will not fully explain it.

ricardosalveti · 2024-10-04T15:23:23Z

tee-supplicant/CMakeLists.txt

-set(CFG_TEE_CLIENT_LOAD_PATH "/lib" CACHE STRING "Colon-separated list of paths where to look for TAs (see also --ta-dir)")
-set(CFG_TEE_FS_PARENT_PATH "/data/tee" CACHE STRING "Location of TEE filesystem (secure storage)")
+set(CFG_TEE_CLIENT_LOAD_PATH "${CMAKE_INSTALL_LIBDIR}" CACHE STRING "Colon-separated list of paths where to look for TAs (see also --ta-dir)")
+set(CFG_TEE_FS_PARENT_PATH "${CMAKE_INSTALL_LOCALSTATEDIR}/lib/tee" CACHE STRING "Location of TEE filesystem (secure storage)")


Not an issue necessarily, but this will change the default value of CFG_TEE_FS_PARENT_PATH.

Oh yes, sorry about it. Commit message is already too long...

But CFG_TEE_FS_PARENT_PATH is now changed from /data/tee to (most likely) /var/lib/tee.

At least the yocto side packaging in meta-arm was already setting CFG_TEE_FS_PARENT_PATH to this patch so there the effect is zero. Elsewhere the change does happen. I'm not sure how useful it is in GNU and FHS style systems to use /data/tee path. Is this same default hard coded in other optee components? TA devkit maybe?

I can mention this in the long commit message but it may be lost there. Should this be documented elsewhere too?

jforissier · 2024-10-04T15:47:40Z

Oops! Tests failed on my side using the build.git setup (QEMUv8). I believe the paths should be made absolute by prefixing them with ${CMAKE_INSTALL_PREFIX}/ but then I believe some hardcoded paths need to be changed in build.git too.

mikkorapeli-linaro · 2024-10-07T06:45:31Z

Oops! Tests failed on my side using the build.git setup (QEMUv8). I believe the paths should be made absolute by prefixing them with ${CMAKE_INSTALL_PREFIX}/ but then I believe some hardcoded paths need to be changed in build.git too.

Can you show the failure output? I could also run the tests locally.

I have been testing this change in meta-arm qemuarm64-secureboot machine. I removed the yocto side packaging workarounds to these issues. I will submit those changes to meta-arm once this patch is ok for you in optee upstream.

/data/tee is not FHS compatible path. Use /var/lib/tee instead. Related to optee_client side CMake change to use standard CMake install and runtime paths: OP-TEE/optee_client#391 Signed-off-by: Mikko Rapeli <[email protected]>

jforissier · 2024-10-07T07:48:34Z

Oops! Tests failed on my side using the build.git setup (QEMUv8). I believe the paths should be made absolute by prefixing them with ${CMAKE_INSTALL_PREFIX}/ but then I believe some hardcoded paths need to be changed in build.git too.

Can you show the failure output? I could also run the tests locally.

Here is how to reproduce the issue(s):

You may run the commands in the OP-TEE CI container to avoid installing the dependencies
$ docker run -it --rm jforissier/optee_os_ci:qemu_check

$ mkdir -p /tmp/optee
$ cd /tmp/optee
$ repo init -u https://github.com/OP-TEE/manifest.git -m qemu_v8.xml
$ repo sync -j10
$ cd optee_client
$ git fetch github pull/391/head
$ git checkout FETCH_HEAD
$ cd ../build
$ make -j3 toolchains
$ make -j$(nproc) check

* This fails with:
*
* Starting QEMU... done, guest is booted.
* Running: xtest ...
* ##### regression_1004 FAIL
* make: *** [Makefile:593: check] Error 1

* serial0.log is the output of the non-secure console, serial1.log is the secure one 
vi ../out/bin/serial0.log 

* To debug, start two consoles in separate windows
$ build/soc_term.py 54320  # NS console
$ build/soc_term.py 54321  # S console

* Then run:
$ make -j$(nproc) run-only   # Replace 'run-only' with 'run' to rebuild
(qemu) c

* Back to the NS console
buildroot login: root
# xtest 1004

* To debug what's happening in tee-supplicant
# ps -ef | grep supp
   96 tee      /usr/sbin/tee-supplicant -d /dev/teepriv0
# kill 96
# strace -f -o /tmp/supp.log tee-supplicant&
# xtest 1004
# vi /tmp/supp.log

I have been testing this change in meta-arm qemuarm64-secureboot machine. I removed the yocto side packaging workarounds to these issues. I will submit those changes to meta-arm once this patch is ok for you in optee upstream.

/data/tee is not FHS compatible path. Use /var/lib/tee instead. Related to optee_client side CMake change to use standard CMake install and runtime paths: OP-TEE/optee_client#391 Reviewed-by: Jerome Forissier <[email protected]> Signed-off-by: Mikko Rapeli <[email protected]>

tee-supplicant startup with systemd init based is non-trivial. Add sample udev rule and systemd service files here so that distros can co-operate maintaining them. Files are from meta-arm https://git.yoctoproject.org/meta-arm at commit 7cce43e632daa8650f683ac726f9124681b302a4 with license MIT and authors: Peter Griffin <[email protected]> Joshua Watt <[email protected]> Javier Tia <[email protected]> Mikko Rapeli <[email protected]> With permission from the authors, files can be relicensed to BSD-2-Clause like rest of optee client repo. The config files expect to find tee and teepriv system groups and teesuppl user and group (part of teepriv group) for running tee-supplicant. Additionally state directory /var/lib/tee must be owned by teesuppl user and group with no rights to other users. The groups and user can be changed via CMake variables: CFG_TEE_GROUP CFG_TEEPRIV_GROUP CFG_TEE_SUPPL_USER CFG_TEE_SUPPL_GROUP Change storage path from /data to /var/lib and use standard CMake variables also for constructing install paths which can be override to change the defaults: CMAKE_INSTALL_PREFIX, e.g. / CMAKE_INSTALL_LIBDIR, e.g. /usr/lib CMAKE_INSTALL_LOCALSTATEDIR /var Once these are setup, udev will start tee-supplicant in initramfs or rootfs with teesuppl user and group when /dev/teepriv device appears. The systemd service starts before tpm2.target (new in systemd 256) which starts early in initramfs and in main rootfs. This covers firmware TPM TA usecases for main rootfs encryption. When stopping tee-supplicant, the ftpm kernel modules are removed and only then the main process stopped to avoid fTPM breakage. These workarounds may be removed once RPMB kernel and optee patches without tee-supplicant are merged (Linux kernel >= 6.12-rc1, optee_os latest master or >= 4.4). Tested on yocto meta-arm setup which runs fTPM and optee-test/xtest under qemuarm64: $ git clone https://git.yoctoproject.org/meta-arm $ cd meta-arm $ SSTATE_DIR=$HOME/sstate DL_DIR=$HOME/download kas build \ ci/qemuarm64-secureboot.yml:ci/poky-altcfg.yml:ci/testimage.yml Compiled image can be manually started to qemu serial console with: $ SSTATE_DIR=$HOME/sstate DL_DIR=$HOME/download kas shell \ ci/qemuarm64-secureboot.yml:ci/poky-altcfg.yml:ci/testimage.yml $ runqemu slirp nographic meta-arm maintainers run these tests as part of their CI. Note that if the tee-supplicant state directory /var/lib/tee can not be accessed due permissions or other problems, then tee-supplicant startup with systemd still works. Only optee-test/xtest will be failing and fTPM kernel drivers fail to load with error messages. Cc: Peter Griffin <[email protected]> Cc: Joshua Watt <[email protected]> Cc: Javier Tia <[email protected]> Acked-by: Jerome Forissier <[email protected]> Signed-off-by: Mikko Rapeli <[email protected]>

It's not needed now that both use CMAKE_INSTALL_LIBDIR as path. If users want different paths, then they need to set CMAKE_INSTALL_LIBDIR to their liking when compiling. Acked-by: Jerome Forissier <[email protected]> Signed-off-by: Mikko Rapeli <[email protected]>

mikkorapeli-linaro · 2024-10-08T12:53:48Z

Thanks, I ran optee build tests and needed changes are in OP-TEE/build#783

I leave TA loading to /lib for now. Various repos would need to be changed for that and I don't see a big need for it.

jforissier · 2024-10-08T13:29:46Z

Thanks, I ran optee build tests and needed changes are in OP-TEE/build#783

Works for me too. Thanks for looking into this.

I leave TA loading to /lib for now. Various repos would need to be change for that and I don't need a big need for it.

OK

/data/tee is not FHS compatible path. Use /var/lib/tee instead. Related to optee_client side CMake change to use standard CMake install and runtime paths: OP-TEE/optee_client#391 Reviewed-by: Jerome Forissier <[email protected]> Acked-by: Jens Wiklander <[email protected]> Signed-off-by: Mikko Rapeli <[email protected]>

jforissier reviewed Oct 3, 2024

View reviewed changes

tee-supplicant/optee-udev.rules Outdated Show resolved Hide resolved

mikkorapeli-linaro force-pushed the systemd_support branch from 44e6a63 to 648b5be Compare October 4, 2024 13:49

jforissier reviewed Oct 4, 2024

View reviewed changes

mikkorapeli-linaro force-pushed the systemd_support branch from 648b5be to cc53dfc Compare October 4, 2024 14:36

ricardosalveti reviewed Oct 4, 2024

View reviewed changes

mikkorapeli-linaro mentioned this pull request Oct 7, 2024

common.mk post-build.sh S30optee: change /data to /var/lib OP-TEE/build#783

Closed

mikkorapeli-linaro added 2 commits October 8, 2024 12:46

mikkorapeli-linaro force-pushed the systemd_support branch from cc53dfc to bfd3869 Compare October 8, 2024 12:29

jforissier merged commit d221676 into OP-TEE:master Oct 10, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tee-supplicant: add udev rule and systemd service file #391

tee-supplicant: add udev rule and systemd service file #391

mikkorapeli-linaro commented Oct 3, 2024

mikkorapeli-linaro commented Oct 3, 2024

jforissier left a comment

mikkorapeli-linaro commented Oct 4, 2024

mikkorapeli-linaro commented Oct 4, 2024

jforissier left a comment

ricardosalveti Oct 4, 2024

mikkorapeli-linaro Oct 7, 2024

ricardosalveti Oct 4, 2024

mikkorapeli-linaro Oct 7, 2024

ricardosalveti Oct 4, 2024

mikkorapeli-linaro Oct 7, 2024

jforissier commented Oct 4, 2024

mikkorapeli-linaro commented Oct 7, 2024

jforissier commented Oct 7, 2024 •

edited

Loading

mikkorapeli-linaro commented Oct 8, 2024 •

edited

Loading

jforissier commented Oct 8, 2024

tee-supplicant: add udev rule and systemd service file #391

tee-supplicant: add udev rule and systemd service file #391

Conversation

mikkorapeli-linaro commented Oct 3, 2024

mikkorapeli-linaro commented Oct 3, 2024

jforissier left a comment

Choose a reason for hiding this comment

mikkorapeli-linaro commented Oct 4, 2024

mikkorapeli-linaro commented Oct 4, 2024

jforissier left a comment

Choose a reason for hiding this comment

ricardosalveti Oct 4, 2024

Choose a reason for hiding this comment

mikkorapeli-linaro Oct 7, 2024

Choose a reason for hiding this comment

ricardosalveti Oct 4, 2024

Choose a reason for hiding this comment

mikkorapeli-linaro Oct 7, 2024

Choose a reason for hiding this comment

ricardosalveti Oct 4, 2024

Choose a reason for hiding this comment

mikkorapeli-linaro Oct 7, 2024

Choose a reason for hiding this comment

jforissier commented Oct 4, 2024

mikkorapeli-linaro commented Oct 7, 2024

jforissier commented Oct 7, 2024 • edited Loading

mikkorapeli-linaro commented Oct 8, 2024 • edited Loading

jforissier commented Oct 8, 2024

jforissier commented Oct 7, 2024 •

edited

Loading

mikkorapeli-linaro commented Oct 8, 2024 •

edited

Loading