On Thu, 11 May 2023 at 10:36, Etienne Carriere <etienne.carriere@xxxxxxxxxx> wrote: > > Dearl all, > > Typo in my previous post! > > On Thu, 11 May 2023 at 06:47, Etienne Carriere > <etienne.carriere@xxxxxxxxxx> wrote: > > > > Hello Jarkko, > > > > On Thu, 11 May 2023 at 00:12, Jarkko Sakkinen <jarkko@xxxxxxxxxx> wrote: > > > > > > On Fri May 5, 2023 at 9:43 PM EEST, Etienne Carriere wrote: > > > > Changes fTPM TEE driver to open the TEE session with REE kernel login > > > > identifier rather than public login. This is needed in case fTPM service > > > > it denied to user land application and restricted to kernel operating > > > > system services only. > > > > > > > > Signed-off-by: Etienne Carriere <etienne.carriere@xxxxxxxxxx> > > > > > > > > > Can you bring up a little context here? > > > > > > What is REE login? > > > Does it break backwards compatibility to switch? > > > What kind of scenario we are talking about? What does it mean in plain > > > English when fTPM service is denied. > > > What is fTPM service? > > > > By fTPM service I meant the services exposed by fTPM through its > > OP-TEE interface, that are the commands a client can invoke in fTPM, > > see [1]. > > > > Regarding backward compatibility, this change is backward compatible > > as far as the OP-TEE entity this driver communicates with is of > > revision 3.9.0 or above. > > I understand this case should be addressed in some way. > > > > In current implementation, fTPM can be invoked by Linux kernel drivers > > (through Linux kernel tee API as tpm_ftpm_tee currently does) as well > > as by userland application (through TEE client library API [2]). > > This change makes tpm_ftpm_tee to invoke fTPM interface using a client > > identifier stating it is the Linux kernel that invokes it, not a > > userland application. fTPM implementation does not check the client > > identity when a client opens a session toward it. Therefore using a > > public identifier (TEE_IOCTL_LOGIN_PUBLIC) or the OS privilege > > identifier (TEE_IOCTL_LOGIN_REE_KERNEL) does not matter, as far as > > OP-TEE supports these IDs. The former is native to OP-TEE initial UAPI > > [3], the latter was introduced in OP-TEE 3.9.0 [4] and Linux kernel > > v5.8 [5]. > > > > That said, this change does fix an existing issue in fTPM integration. > > Typo, sorry, I meant > "That said, this change does **NOT** fix an existing issue in fTPM integration." > > BR, > Etienne > > > The fTPM entity currently only accepts a single session opened towards > > it. This is enforced as fTPM sets property TA_FLAG_SINGLE_INSTANCE and > > does not set property TA_FLAG_MULTI_SESSION [6]. > > Linux kernel tpm_ftpm_tee driver currently opens a session to fTPM at > > probe time and releases it at remove time so once the driver is > > successfully probed, no userland application can use TEE userland > > client API to open another session and communicate with fTPM. How about if the fTPM TEE kernel driver is built as a module and removed at runtime by a malicious user-space client? -Sumit > > > > [1] https://github.com/microsoft/ms-tpm-20-ref/blob/main/Samples/ARM32-FirmwareTPM/optee_ta/fTPM/fTPM.c#L456 > > [2] https://github.com/OP-TEE/optee_client > > [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=967c9cca2cc50569efc65945325c173cecba83bd > > [4] https://github.com/OP-TEE/optee_os/commit/78f462f646e7c037bea13aa6282c81f255922a4f > > [5] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=104edb94cc4b3101bab33161cd861de13e85610b > > [6] https://github.com/microsoft/ms-tpm-20-ref/blob/main/Samples/ARM32-FirmwareTPM/optee_ta/fTPM/user_ta_header_defines.h#L47 > > > > Regards, > > Etienne > > > > > > > > > --- > > > > drivers/char/tpm/tpm_ftpm_tee.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/char/tpm/tpm_ftpm_tee.c b/drivers/char/tpm/tpm_ftpm_tee.c > > > > index 528f35b14fb6..6d32e260af43 100644 > > > > --- a/drivers/char/tpm/tpm_ftpm_tee.c > > > > +++ b/drivers/char/tpm/tpm_ftpm_tee.c > > > > @@ -241,7 +241,7 @@ static int ftpm_tee_probe(struct device *dev) > > > > /* Open a session with fTPM TA */ > > > > memset(&sess_arg, 0, sizeof(sess_arg)); > > > > export_uuid(sess_arg.uuid, &ftpm_ta_uuid); > > > > - sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC; > > > > + sess_arg.clnt_login = TEE_IOCTL_LOGIN_REE_KERNEL; > > > > sess_arg.num_params = 0; > > > > > > > > rc = tee_client_open_session(pvt_data->ctx, &sess_arg, NULL); > > > > -- > > > > 2.25.1 > > > > > > BR, Jarkko
