[PATCHv2 1/3] common: SCP03 control (enable and provision of keys)

[Jorge Ramirez-Ortiz, Foundries]

On 07/02/21, Simon Glass wrote:
> Hi Jorge,
> 
> On Sat, 6 Feb 2021 at 16:11, Jorge Ramirez-Ortiz <jorge at foundries.io> wrote:
> >
> > This Trusted Application allows enabling and provisioning SCP03 keys
> > on TEE controlled secure element (ie, NXP SE050)
> >
> > For information on SCP03, check the Global Platform HomePage[1]
> > [1] globalplatform.org
> >
> > Signed-off-by: Jorge Ramirez-Ortiz <jorge at foundries.io>
> > ---
> >  common/Kconfig               |  8 ++++++
> >  common/Makefile              |  1 +
> >  common/scp03.c               | 52 ++++++++++++++++++++++++++++++++++++
> >  include/scp03.h              | 19 +++++++++++++
> >  include/tee/optee_ta_scp03.h | 21 +++++++++++++++
> >  5 files changed, 101 insertions(+)
> >  create mode 100644 common/scp03.c
> >  create mode 100644 include/scp03.h
> >  create mode 100644 include/tee/optee_ta_scp03.h
> 
> Reviewed-by: Simon Glass <sjg at chromium.org>
> 
> But please see below
> 
> >
> > diff --git a/common/Kconfig b/common/Kconfig
> > index 2bb3798f80..482f123534 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -588,6 +588,14 @@ config AVB_BUF_SIZE
> >
> >  endif # AVB_VERIFY
> >
> > +config SCP03
> > +       bool "Build SCP03 - Secure Channel Protocol O3 - controls"
> > +       depends on OPTEE || SANDBOX
> > +       depends on TEE
> > +       help
> > +         This option allows U-Boot to enable and or provision SCP03 on an OPTEE
> > +         controlled Secured Element.
> 
> Why would you want to do that? Please expand this a bit

sure

> 
> > +
> >  config SPL_HASH
> >         bool # "Support hashing API (SHA1, SHA256, etc.)"
> >         help
> > diff --git a/common/Makefile b/common/Makefile
> > index daeea67cf2..215b8b26fd 100644
> > --- a/common/Makefile
> > +++ b/common/Makefile
> > @@ -137,3 +137,4 @@ obj-$(CONFIG_CMD_LOADB) += xyzModem.o
> >  obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o
> >
> >  obj-$(CONFIG_AVB_VERIFY) += avb_verify.o
> > +obj-$(CONFIG_SCP03) += scp03.o
> > diff --git a/common/scp03.c b/common/scp03.c
> > new file mode 100644
> > index 0000000000..c655283387
> > --- /dev/null
> > +++ b/common/scp03.c
> > @@ -0,0 +1,52 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * (C) Copyright 2021, Foundries.IO
> > + *
> > + */
> > +
> 
> common.h
> 
> > +#include <scp03.h>
> > +#include <tee.h>
> > +#include <tee/optee_ta_scp03.h>
> > +
> > +static int scp03_enable(bool provision)
> > +{
> > +       const struct tee_optee_ta_uuid uuid = PTA_SCP03_UUID;
> > +       struct tee_open_session_arg session;
> > +       struct tee_invoke_arg invoke;
> > +       struct tee_param param;
> > +       struct udevice *tee = NULL;
> > +
> > +       tee = tee_find_device(tee, NULL, NULL, NULL);
> > +       if (!tee)
> > +               return -ENODEV;
> > +
> > +       memset(&session, 0, sizeof(session));
> > +       tee_optee_ta_uuid_to_octets(session.uuid, &uuid);
> > +       if (tee_open_session(tee, &session, 0, NULL))
> > +               return -ENODEV;
> 
> Should return the actual error from tee_open_session(). You can't
> return -ENODEV as there is a device.

Right but there is not a TA responding to the requests (the actual
handler for the TEE). But ok, will use EINVAL


> 
> > +
> > +       memset(&param, 0, sizeof(param));
> > +       param.attr = TEE_PARAM_ATTR_TYPE_VALUE_INPUT;
> > +       param.u.value.a = provision;
> > +
> > +       memset(&invoke, 0, sizeof(invoke));
> > +       invoke.func = PTA_CMD_ENABLE_SCP03;
> > +       invoke.session = session.session;
> > +
> > +       if (tee_invoke_func(tee, &invoke, 1, &param))
> > +               return -EIO;
> 
> Please return the actual error

These functions done return a valid error (just <0 on error)

Having said that this is probably the most likely error - the i2c chip
(the secure element) not being accessible but I can return something
more generic like EFAULT?

Notice that if this fails 99% of the times will mean that the secure
element has been bricked in the process.

> 
> > +
> > +       tee_close_session(tee, session.session);
> > +
> > +       return 0;
> > +}
> > +
> > +int tee_enable_scp03(void)
> > +{
> > +       return scp03_enable(false);
> > +}
> > +
> > +int tee_provision_scp03(void)
> > +{
> > +       return scp03_enable(true);
> > +}
> > diff --git a/include/scp03.h b/include/scp03.h
> > new file mode 100644
> > index 0000000000..034796ada3
> > --- /dev/null
> > +++ b/include/scp03.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * (C) Copyright 2021, Foundries.IO
> > + *
> > + */
> > +
> > +#ifndef _SCP03_H
> > +#define _SCP03_H
> > +
> > +/*
> > + * Requests to OPTEE to enable or provision the Secure Channel Protocol on its
> > + * Secure Element
> > + *
> > + *  If key provisioning is requested, OPTEE shall generate new SCP03 keys and
> > + *  write them to the Secure Element.
> 
> @return

ok

> 
> > + */
> > +int tee_enable_scp03(void);
> > +int tee_provision_scp03(void);
> > +#endif /* _SCP03_H */
> 
> Regards,
> Simon