* [PATCHv2 2/3] cmd: SCP03: enable and provision command
2021-02-06 23:11 [PATCHv2 1/3] common: SCP03 control (enable and provision of keys) Jorge Ramirez-Ortiz
@ 2021-02-06 23:11 ` Jorge Ramirez-Ortiz
2021-02-07 14:38 ` Simon Glass
2021-02-06 23:11 ` [PATCHv2 3/3] drivers: tee: sandbox: secure channel protocol control Jorge Ramirez-Ortiz
2021-02-07 14:37 ` [PATCHv2 1/3] common: SCP03 control (enable and provision of keys) Simon Glass
2 siblings, 1 reply; 8+ messages in thread
From: Jorge Ramirez-Ortiz @ 2021-02-06 23:11 UTC (permalink / raw)
To: u-boot
Enable and provision the SCP03 keys on a TEE controlled secured elemt
from the U-Boot shell.
Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
---
cmd/Kconfig | 9 ++++++++
cmd/Makefile | 3 +++
cmd/scp03.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 76 insertions(+)
create mode 100644 cmd/scp03.c
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 928a2a0a2d..4f990249b4 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2021,6 +2021,15 @@ config HASH_VERIFY
help
Add -v option to verify data against a hash.
+config CMD_SCP03
+ bool "scp03 - SCP03 enable and rotate/provision operations"
+ depends on SCP03
+ help
+ Enables the SCP03 commands to activate I2C channel encryption and
+ provision the SCP03 keys.
+ scp03 enable
+ scp03 provision
+
config CMD_TPM_V1
bool
diff --git a/cmd/Makefile b/cmd/Makefile
index 176bf925fd..a7017e8452 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -193,6 +193,9 @@ obj-$(CONFIG_CMD_BLOB) += blob.o
# Android Verified Boot 2.0
obj-$(CONFIG_CMD_AVB) += avb.o
+# Foundries.IO SCP03
+obj-$(CONFIG_CMD_SCP03) += scp03.o
+
obj-$(CONFIG_ARM) += arm/
obj-$(CONFIG_RISCV) += riscv/
obj-$(CONFIG_SANDBOX) += sandbox/
diff --git a/cmd/scp03.c b/cmd/scp03.c
new file mode 100644
index 0000000000..07913dbd3e
--- /dev/null
+++ b/cmd/scp03.c
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2021, Foundries.IO
+ *
+ */
+
+#include <common.h>
+#include <command.h>
+#include <env.h>
+#include <scp03.h>
+
+int do_scp03_enable(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+ if (argc != 1)
+ return CMD_RET_USAGE;
+
+ if (tee_enable_scp03())
+ return CMD_RET_FAILURE;
+
+ return CMD_RET_SUCCESS;
+}
+
+int do_scp03_provision(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+ if (argc != 1)
+ return CMD_RET_USAGE;
+
+ if (tee_provision_scp03())
+ return CMD_RET_FAILURE;
+
+ return CMD_RET_SUCCESS;
+}
+
+static struct cmd_tbl cmd_scp03[] = {
+ U_BOOT_CMD_MKENT(enable, 1, 0, do_scp03_enable, "", ""),
+ U_BOOT_CMD_MKENT(provision, 1, 0, do_scp03_provision, "", ""),
+};
+
+static int do_scp03(struct cmd_tbl *cmdtp, int flag, int argc,
+ char * const argv[])
+{
+ struct cmd_tbl *cp;
+
+ cp = find_cmd_tbl(argv[1], cmd_scp03, ARRAY_SIZE(cmd_scp03));
+
+ argc--;
+ argv++;
+
+ if (!cp || argc > cp->maxargs)
+ return CMD_RET_USAGE;
+
+ if (flag == CMD_FLAG_REPEAT)
+ return CMD_RET_FAILURE;
+
+ return cp->cmd(cmdtp, flag, argc, argv);
+}
+
+U_BOOT_CMD(scp03, 2, 0, do_scp03,
+ "Provides a command to enable SCP03 and provision the SCP03 keys\n",
+ "\tenable - enable SCP03\n"
+ "\tprovision - provision SCP03\n"
+);
--
2.30.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCHv2 2/3] cmd: SCP03: enable and provision command
2021-02-06 23:11 ` [PATCHv2 2/3] cmd: SCP03: enable and provision command Jorge Ramirez-Ortiz
@ 2021-02-07 14:38 ` Simon Glass
0 siblings, 0 replies; 8+ messages in thread
From: Simon Glass @ 2021-02-07 14:38 UTC (permalink / raw)
To: u-boot
Hi Jorge,
On Sat, 6 Feb 2021 at 16:11, Jorge Ramirez-Ortiz <jorge@foundries.io> wrote:
>
> Enable and provision the SCP03 keys on a TEE controlled secured elemt
> from the U-Boot shell.
>
> Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
> ---
> cmd/Kconfig | 9 ++++++++
> cmd/Makefile | 3 +++
> cmd/scp03.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 76 insertions(+)
> create mode 100644 cmd/scp03.c
Same comments on this one.
Regards,
Simon
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCHv2 3/3] drivers: tee: sandbox: secure channel protocol control
2021-02-06 23:11 [PATCHv2 1/3] common: SCP03 control (enable and provision of keys) Jorge Ramirez-Ortiz
2021-02-06 23:11 ` [PATCHv2 2/3] cmd: SCP03: enable and provision command Jorge Ramirez-Ortiz
@ 2021-02-06 23:11 ` Jorge Ramirez-Ortiz
2021-02-07 14:38 ` Simon Glass
2021-02-07 14:37 ` [PATCHv2 1/3] common: SCP03 control (enable and provision of keys) Simon Glass
2 siblings, 1 reply; 8+ messages in thread
From: Jorge Ramirez-Ortiz @ 2021-02-06 23:11 UTC (permalink / raw)
To: u-boot
Adds support for SCP03 emulation.
Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
---
drivers/tee/optee/Kconfig | 6 ++++
drivers/tee/sandbox.c | 60 +++++++++++++++++++++++++++++++++++++--
2 files changed, 64 insertions(+), 2 deletions(-)
diff --git a/drivers/tee/optee/Kconfig b/drivers/tee/optee/Kconfig
index d489834df9..b7f704a5e6 100644
--- a/drivers/tee/optee/Kconfig
+++ b/drivers/tee/optee/Kconfig
@@ -22,6 +22,12 @@ config OPTEE_TA_AVB
The TA can support the "avb" subcommands "read_rb", "write"rb"
and "is_unlocked".
+config OPTEE_TA_SCP03
+ bool "Support SCP03 TA"
+ default y
+ help
+ Enables support for the SCP03 Trusted Application (TA) in OP-TEE.
+
endmenu
endif
diff --git a/drivers/tee/sandbox.c b/drivers/tee/sandbox.c
index e1ba027fd6..273c23239a 100644
--- a/drivers/tee/sandbox.c
+++ b/drivers/tee/sandbox.c
@@ -7,6 +7,7 @@
#include <sandboxtee.h>
#include <tee.h>
#include <tee/optee_ta_avb.h>
+#include <tee/optee_ta_scp03.h>
/*
* The sandbox tee driver tries to emulate a generic Trusted Exectution
@@ -32,7 +33,7 @@ struct ta_entry {
struct tee_param *params);
};
-#ifdef CONFIG_OPTEE_TA_AVB
+#if defined(CONFIG_OPTEE_TA_SCP03) || defined(CONFIG_OPTEE_TA_AVB)
static u32 get_attr(uint n, uint num_params, struct tee_param *params)
{
if (n >= num_params)
@@ -44,7 +45,7 @@ static u32 get_attr(uint n, uint num_params, struct tee_param *params)
static u32 check_params(u8 p0, u8 p1, u8 p2, u8 p3, uint num_params,
struct tee_param *params)
{
- u8 p[] = { p0, p1, p2, p3};
+ u8 p[] = { p0, p1, p2, p3 };
uint n;
for (n = 0; n < ARRAY_SIZE(p); n++)
@@ -62,6 +63,55 @@ bad_params:
return TEE_ERROR_BAD_PARAMETERS;
}
+#endif
+
+#ifdef CONFIG_OPTEE_TA_SCP03
+static u32 pta_scp03_open_session(struct udevice *dev, uint num_params,
+ struct tee_param *params)
+{
+ /*
+ * We don't expect additional parameters when opening a session to
+ * this TA.
+ */
+ return check_params(TEE_PARAM_ATTR_TYPE_NONE, TEE_PARAM_ATTR_TYPE_NONE,
+ TEE_PARAM_ATTR_TYPE_NONE, TEE_PARAM_ATTR_TYPE_NONE,
+ num_params, params);
+}
+
+static u32 pta_scp03_invoke_func(struct udevice *dev, u32 func, uint num_params,
+ struct tee_param *params)
+{
+ u32 res;
+ static bool enabled;
+
+ switch (func) {
+ case PTA_CMD_ENABLE_SCP03:
+ res = check_params(TEE_PARAM_ATTR_TYPE_VALUE_INPUT,
+ TEE_PARAM_ATTR_TYPE_NONE,
+ TEE_PARAM_ATTR_TYPE_NONE,
+ TEE_PARAM_ATTR_TYPE_NONE,
+ num_params, params);
+ if (res)
+ return res;
+
+ if (!enabled) {
+ printf("SCP03 enabled\n");
+ enabled = true;
+ } else {
+ printf("SCP03 already enabled, no action\n");
+ }
+
+ if (params[0].u.value.a)
+ printf("SCP03 keys rotated\n");
+
+ return TEE_SUCCESS;
+ default:
+ return TEE_ERROR_NOT_SUPPORTED;
+ }
+}
+#endif
+
+#ifdef CONFIG_OPTEE_TA_AVB
static u32 ta_avb_open_session(struct udevice *dev, uint num_params,
struct tee_param *params)
@@ -223,6 +273,12 @@ static const struct ta_entry ta_entries[] = {
.invoke_func = ta_avb_invoke_func,
},
#endif
+#ifdef CONFIG_OPTEE_TA_SCP03
+ { .uuid = PTA_SCP03_UUID,
+ .open_session = pta_scp03_open_session,
+ .invoke_func = pta_scp03_invoke_func,
+ },
+#endif
};
static void sandbox_tee_get_version(struct udevice *dev,
--
2.30.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCHv2 1/3] common: SCP03 control (enable and provision of keys)
2021-02-06 23:11 [PATCHv2 1/3] common: SCP03 control (enable and provision of keys) Jorge Ramirez-Ortiz
2021-02-06 23:11 ` [PATCHv2 2/3] cmd: SCP03: enable and provision command Jorge Ramirez-Ortiz
2021-02-06 23:11 ` [PATCHv2 3/3] drivers: tee: sandbox: secure channel protocol control Jorge Ramirez-Ortiz
@ 2021-02-07 14:37 ` Simon Glass
2021-02-07 15:58 ` Jorge
2 siblings, 1 reply; 8+ messages in thread
From: Simon Glass @ 2021-02-07 14:37 UTC (permalink / raw)
To: u-boot
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(¶m, 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, ¶m))
> + 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCHv2 1/3] common: SCP03 control (enable and provision of keys)
2021-02-07 14:37 ` [PATCHv2 1/3] common: SCP03 control (enable and provision of keys) Simon Glass
@ 2021-02-07 15:58 ` Jorge
2021-02-07 16:19 ` Simon Glass
0 siblings, 1 reply; 8+ messages in thread
From: Jorge @ 2021-02-07 15:58 UTC (permalink / raw)
To: u-boot
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(¶m, 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, ¶m))
> > + 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCHv2 1/3] common: SCP03 control (enable and provision of keys)
2021-02-07 15:58 ` Jorge
@ 2021-02-07 16:19 ` Simon Glass
0 siblings, 0 replies; 8+ messages in thread
From: Simon Glass @ 2021-02-07 16:19 UTC (permalink / raw)
To: u-boot
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(¶m, 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, ¶m))
> > > + return -EIO;
> >
> > Please return the actual error
>
> These functions done return a valid error (just <0 on error)
Oh that should probably be fixed@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
^ permalink raw reply [flat|nested] 8+ messages in thread