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

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@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

Hi Jorge,

On Sat, 6 Feb 2021 at 16:11, Jorge Ramirez-Ortiz <jorge@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@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@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

> +
>  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.

> +
> +       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

> +
> +       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

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

Regards,
Simon

On 07/02/21, Simon Glass wrote:
> Hi Jorge,
> 
> On Sat, 6 Feb 2021 at 16:11, Jorge Ramirez-Ortiz <jorge@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@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@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

Hi Jorge,

On Sun, 7 Feb 2021 at 08:58, Jorge Ramirez-Ortiz, Foundries
<jorge@foundries.io> wrote:
>
> On 07/02/21, Simon Glass wrote:
> > Hi Jorge,
> >
> > On Sat, 6 Feb 2021 at 16:11, Jorge Ramirez-Ortiz <jorge@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@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@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

Then perhaps -ENXIO ?

>
>
> >
> > > +
> > > +       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)

Oh that should probably be fixed at some point.

>
> 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?

In that case -EIO is fine for now. I think of -EFAULT as a memory
fault, although I don't know that we use it much in U-Boot.

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

OK.

 [...]

Regards,
Simon