Message-ID: <Zmj2Wg_YqdebN3xO@nuoska>
Date: Wed, 12 Jun 2024 10:14:02 +0900
From: Mikko Rapeli <mikko.rapeli@...aro.org>
To: Sumit Garg <sumit.garg@...aro.org>
Cc: Jens Wiklander <jens.wiklander@...aro.org>,
	Manuel Traut <manut@...ka.net>, linux-kernel@...r.kernel.org,
	linux-mmc@...r.kernel.org, op-tee@...ts.trustedfirmware.org,
	Shyam Saini <shyamsaini@...ux.microsoft.com>,
	Ulf Hansson <ulf.hansson@...aro.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Jerome Forissier <jerome.forissier@...aro.org>,
	Ilias Apalodimas <ilias.apalodimas@...aro.org>,
	Bart Van Assche <bvanassche@....org>,
	Randy Dunlap <rdunlap@...radead.org>,
	Ard Biesheuvel <ardb@...nel.org>, Arnd Bergmann <arnd@...db.de>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Peter Huewe <peterhuewe@....de>,
	Jarkko Sakkinen <jarkko@...nel.org>, Jason Gunthorpe <jgg@...pe.ca>,
	linux-integrity@...r.kernel.org
Subject: Re: [PATCH v7 4/4] optee: probe RPMB device using RPMB subsystem

Hi,

Adding TPM maintainers and linux-integrity since discussion relates to firmware TPM
driver tpm_ftpm_tee

On Tue, Jun 11, 2024 at 04:13:21PM +0530, Sumit Garg wrote:
> On Tue, 11 Jun 2024 at 08:32, Mikko Rapeli <mikko.rapeli@...aro.org> wrote:
> >
> > Hi,
> >
> > On Mon, Jun 10, 2024 at 02:52:31PM +0200, Jens Wiklander wrote:
> > > Hi Manuel,
> > >
> > > On Mon, Jun 3, 2024 at 11:10 AM Manuel Traut <manut@...ka.net> wrote:
> > > >
> > > > On 14:13 Mon 27 May     , Jens Wiklander wrote:
> > > > > --- a/drivers/tee/optee/ffa_abi.c
> > > > > +++ b/drivers/tee/optee/ffa_abi.c
> > > > > @@ -7,6 +7,7 @@
> > > > >
> > > > >  #include <linux/arm_ffa.h>
> > > > >  #include <linux/errno.h>
> > > > > +#include <linux/rpmb.h>
> > > > >  #include <linux/scatterlist.h>
> > > > >  #include <linux/sched.h>
> > > > >  #include <linux/slab.h>
> > > > > @@ -903,6 +904,10 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
> > > > >       optee->ffa.bottom_half_value = U32_MAX;
> > > > >       optee->rpc_param_count = rpc_param_count;
> > > > >
> > > > > +     if (IS_REACHABLE(CONFIG_RPMB) &&
> > > > > +         (sec_caps & OPTEE_FFA_SEC_CAP_RPMB_PROBE))
> > > > > +             optee->in_kernel_rpmb_routing = true;
> > > >
> > > > The SEC_CAP_RPMB_PROBE flag seems to be missing in optee_os at the moment.
> > > > If I remove this check here, the series works for me.
> > >
> > > You're right, I missed pushing those flags to optee_os. I've pushed them now.
> >
> > Thanks! Tested with optee 4.1 and your patches from
> > https://github.com/jenswi-linaro/optee_os/commits/rpmb_probe_v7/
> > in Trusted Substrate uefi firmware
> > ( https://gitlab.com/Linaro/trustedsubstrate/meta-ts/ )
> > and this series and a bunch of dependencies backported to
> > our Trusted Reference Stack
> > ( https://trs.readthedocs.io/en/latest/ )
> > 6.6.29 kernel on rockpi4b (rk3399 ARM64 SoC) with secure boot and
> > the optee side fTPM TA device used to create an encrypted rootfs with
> > systemd. Kernel side RPMB routing is in use and works for the TPM use cases.
> >
> 
> Glad to see that you can get fTPM to work without tee-supplicant after
> this patch-set.

Sorry but the fTPM TA only works with tee-supplicant in userspace. It's needed
for RPC setup. For RPMB it is not needed or used with these patches applied.

> > Full boot and test log (with unrelated test failures)
> > https://ledge.validation.linaro.org/scheduler/job/88692
> >
> > root@...-qemuarm64:~# cat /sys/class/tee/tee0/rpmb_routing_model
> > ...
> > kernel
> 
> So coming back to the real question, do we really need this new
> rpmb_routing_model ABI? Did systemd still need it with no
> tee-supplicant dependency? IMHO, a user-space ABI requires use-case
> justification otherwise it's just going to add on maintenance burden.

Currently it is not needed, because tee-supplicant is still required to
setup RPC with fTPM. If the RPC setup were also done in kernel directly and
tee-supplicant need is removed, then this kind of ABI is important so that
userspace init knows if it needs to queue startup of tee-supplicant or not.

On a related note, the kernel tpm_ftpm_tee driver for fTPM TA really has
a hard dependency to tee-supplicant in userspace. If tee-supplicant is stopped,
restarted etc without unloading the kernel module (or otherwise disabling the TPM device),
then all TPM device actions done without tee-supplicant running will fail ane keep
failing until next reboot. The kernel driver is not crashing but all functionality
breaks.

The availability of tpm_ftpm_tee should be tied much harder to the tee-supplicant
running in userspace, e.g. optee should be in charge to start and bring tpm_ftpm_tee down
based on tee-supplicant userspace daemon availability. Or the needed tee-supplicant code
should be moved to kernel side. Currently systemd side init scripts have issues switching
from initrd to main rootfs since they need to disable tpm_ftpm_tee driver, built in or a module,
before shutting down tee-supplicant. A suspend or other inactive state in the ftpm driver
needs to be triggered, which AFAIK is not currently possible, at least from userspace
(I'd happy be proven wrong here).

An alternative for tpm_fptm_tee driver is to use optee APIs so that the calls
wait for tee-supplicant in userspace if needed:

--- a/drivers/char/tpm/tpm_ftpm_tee.c
+++ b/drivers/char/tpm/tpm_ftpm_tee.c
@@ -237,6 +237,9 @@ static int ftpm_tee_probe(struct device *dev)
                return PTR_ERR(pvt_data->ctx);
        }
 
+       /* wait for tee-supplicant in userspace, fTPM TA really depends on it */
+       pvt_data->ctx->supp_nowait = false;
+
        /* Open a session with fTPM TA */
        memset(&sess_arg, 0, sizeof(sess_arg));
        export_uuid(sess_arg.uuid, &ftpm_ta_uuid);

This works pretty well for the tee-supplicant initrd to main rootfs switch but currently
breaks for example reboot (just hangs), and Jens doesn't approve of this as a
real solution.

So as an alternative, userspace needs to be very careful in initrd and rootfs
to start tee-supplicant earlier than loading tpm_ftpm_tee driver which can
only be supported as module and removed before shutting down tee-supplicant.
In other use cases, TPM drivers are only supported if driver is built into
the kernel (or ACPI table entry for a TPM device exists) which I'm trying
to change with

[PATCH] efi: expose TPM event log to userspace via sysfs

https://lore.kernel.org/lkml/20240422112711.362779-1-mikko.rapeli@linaro.org/

where userspace init can check if it should wait longer for the tpm device to appear,
e.g. let udev load optee etc drivers which eventually start also tee-supplicant and
thus load tpm_ftpm_tee driver (fTPM TA enumration is tied to tee-supplicant in userspace
https://git.yoctoproject.org/meta-arm/tree/meta-arm/recipes-security/optee-ftpm/optee-ftpm/0001-add-enum-to-ta-flags.patch )

Cheers,

-Mikko

Powered by blists - more mailing lists