All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv2 1/3] common: SCP03 control (enable and provision of keys)
@ 2021-02-06 23:11 Jorge Ramirez-Ortiz
  2021-02-06 23:11 ` [PATCHv2 2/3] cmd: SCP03: enable and provision command Jorge Ramirez-Ortiz
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Jorge Ramirez-Ortiz @ 2021-02-06 23:11 UTC (permalink / raw)
  To: u-boot

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

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.
+
 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
+ *
+ */
+
+#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;
+
+	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;
+
+	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.
+ */
+int tee_enable_scp03(void);
+int tee_provision_scp03(void);
+#endif /* _SCP03_H */
diff --git a/include/tee/optee_ta_scp03.h b/include/tee/optee_ta_scp03.h
new file mode 100644
index 0000000000..13f9956d98
--- /dev/null
+++ b/include/tee/optee_ta_scp03.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * (C) Copyright 2021, Foundries.IO
+ *
+ */
+#ifndef __TA_SCP03_H
+#define __TA_SCP03_H
+
+#define PTA_SCP03_UUID { 0xbe0e5821, 0xe718, 0x4f77, \
+			{ 0xab, 0x3e, 0x8e, 0x6c, 0x73, 0xa9, 0xc7, 0x35 } }
+
+/*
+ * Enable Secure Channel Protocol functionality (SCP03) on the Secure Element.
+ *   Setting the operation value to something different than NULL will trigger
+ *   the SCP03 provisioning request.
+ *
+ *   in	params[0].a = operation
+ */
+#define PTA_CMD_ENABLE_SCP03	0
+
+#endif /*__TA_SCP03_H*/
-- 
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 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 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(&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

^ permalink raw reply	[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 3/3] drivers: tee: sandbox: secure channel protocol control 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:
>
> 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(-)

No change log, so this seems to be a resend? Anyway, see earlier comments.

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(&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

^ 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(&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@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

end of thread, other threads:[~2021-02-07 16:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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-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:38   ` Simon Glass
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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.