[U-Boot] [PATCH 0/3] tpm: Add and fix commands

[U-Boot] [PATCH 0/3] tpm: Add and fix commands

[Mario Six]

This series fixes the tpm flush command, which is currently broken, adds a
command to list resources, and a command to load TPM keys via their parent's
SHA1 hash.

Mario Six (3):
  tpm: Add function to load keys via their parent's SHA1 hash
  cmd: tpm: Fix flush command
  lib: tpm: Add command to list resources

 cmd/tpm.c           | 162 +++++++++++++++++++++++++++++++++++++++++++++++-----
 drivers/tpm/Kconfig |  15 +++++
 include/tpm.h       |  12 ++++
 lib/tpm.c           |  40 +++++++++++++
 4 files changed, 214 insertions(+), 15 deletions(-)

--
2.11.0

[U-Boot] [PATCH 1/3] tpm: Add function to load keys via their parent's SHA1 hash

[Mario Six]

If we want to load a key into a TPM, we need to know the designated parent
key's handle, so that the TPM is able to insert the key at the correct place in
the key hierarchy.

However, if we want to load a key whose designated parent key we also
previously loaded ourselves, we first need to memorize this parent key's handle
(since the handles for the key are chosen at random when they are inserted into
the TPM). If we are, however, unable to do so, for example if the parent key is
loaded into the TPM during production, and its child key during the actual
boot, we must find a different mechanism to identify the parent key.

To solve this problem, we add a function that allows U-Boot to load a key into
the TPM using their designated parent key's SHA1 hash, and the corresponding
auth data.

Signed-off-by: Mario Six <mario.six@gdsys.cc>
---
 cmd/tpm.c           | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/tpm/Kconfig |  8 ++++++++
 include/tpm.h       | 12 ++++++++++++
 lib/tpm.c           | 40 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 109 insertions(+)

diff --git a/cmd/tpm.c b/cmd/tpm.c
index 625fc43d26..91bd20da25 100644
--- a/cmd/tpm.c
+++ b/cmd/tpm.c
@@ -592,6 +592,45 @@ static int do_tpm_oiap(cmd_tbl_t *cmdtp, int flag,
 	return report_return_code(err);
 }
 
+#ifdef CONFIG_TPM_LOAD_KEY_BY_SHA1
+static int do_tpm_load_key_by_sha1(cmd_tbl_t *cmdtp, int flag, int argc, char *
+				   const argv[])
+{
+	uint32_t parent_handle = 0;
+	uint32_t key_len, key_handle, err;
+	uint8_t usage_auth[DIGEST_LENGTH];
+	uint8_t parent_hash[DIGEST_LENGTH];
+	void *key;
+
+	if (argc < 5)
+		return CMD_RET_USAGE;
+
+	parse_byte_string(argv[1], parent_hash, NULL);
+	key = (void *)simple_strtoul(argv[2], NULL, 0);
+	key_len = simple_strtoul(argv[3], NULL, 0);
+	if (strlen(argv[4]) != 2 * DIGEST_LENGTH)
+		return CMD_RET_FAILURE;
+	parse_byte_string(argv[4], usage_auth, NULL);
+
+	err = tpm_find_key_sha1(usage_auth, parent_hash, &parent_handle);
+	if (err) {
+		printf("Could not find matching parent key (err = %d)\n", err);
+		return CMD_RET_FAILURE;
+	}
+
+	printf("Found parent key %08x\n", parent_handle);
+
+	err = tpm_load_key2_oiap(parent_handle, key, key_len, usage_auth,
+				 &key_handle);
+	if (!err) {
+		printf("Key handle is 0x%x\n", key_handle);
+		setenv_hex("key_handle", key_handle);
+	}
+
+	return report_return_code(err);
+}
+#endif /* CONFIG_TPM_LOAD_KEY_BY_SHA1 */
+
 static int do_tpm_load_key2_oiap(cmd_tbl_t *cmdtp, int flag,
 		int argc, char * const argv[])
 {
@@ -756,6 +795,10 @@ static cmd_tbl_t tpm_commands[] = {
 			 do_tpm_end_oiap, "", ""),
 	U_BOOT_CMD_MKENT(load_key2_oiap, 0, 1,
 			 do_tpm_load_key2_oiap, "", ""),
+#ifdef CONFIG_TPM_LOAD_KEY_BY_SHA1
+	U_BOOT_CMD_MKENT(load_key_by_sha1, 0, 1,
+			 do_tpm_load_key_by_sha1, "", ""),
+#endif /* CONFIG_TPM_LOAD_KEY_BY_SHA1 */
 	U_BOOT_CMD_MKENT(get_pub_key_oiap, 0, 1,
 			 do_tpm_get_pub_key_oiap, "", ""),
 #endif /* CONFIG_TPM_AUTH_SESSIONS */
@@ -826,6 +869,12 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
 "    - loads a key data from memory address <key_addr>, <key_len> bytes\n"
 "      into TPM using the parent key <parent_handle> with authorization\n"
 "      <usage_auth> (20 bytes hex string).\n"
+#ifdef CONFIG_TPM_LOAD_KEY_BY_SHA1
+"  load_key_by_sha1 parent_hash key_addr key_len usage_auth\n"
+"    - loads a key data from memory address <key_addr>, <key_len> bytes\n"
+"      into TPM using the parent hash <parent_hash> (20 bytes hex string)\n"
+"      with authorization <usage_auth> (20 bytes hex string).\n"
+#endif /* CONFIG_TPM_LOAD_KEY_BY_SHA1 */
 "  get_pub_key_oiap key_handle usage_auth\n"
 "    - get the public key portion of a loaded key <key_handle> using\n"
 "      authorization <usage auth> (20 bytes hex string)\n"
diff --git a/drivers/tpm/Kconfig b/drivers/tpm/Kconfig
index 3490ee0c3b..a54b6a988a 100644
--- a/drivers/tpm/Kconfig
+++ b/drivers/tpm/Kconfig
@@ -88,4 +88,12 @@ config TPM_FLUSH_RESOURCES
 	help
 	  Enable support to flush specific resources (e.g. keys) from the TPM.
 	  The functionality is available via the 'tpm' command as well.
+
+config TPM_LOAD_KEY_BY_SHA1
+	bool "Enable TPM key loading by SHA1 support"
+	depends on TPM
+	help
+	  Enable support to load keys into the TPM by identifying
+	  their parent via the public key's SHA1 hash.
+	  The functionality is available via the 'tpm' command as well.
 endmenu
diff --git a/include/tpm.h b/include/tpm.h
index 800f29c101..f88388f353 100644
--- a/include/tpm.h
+++ b/include/tpm.h
@@ -639,4 +639,16 @@ uint32_t tpm_get_permissions(uint32_t index, uint32_t *perm);
  */
 uint32_t tpm_flush_specific(uint32_t key_handle, uint32_t resource_type);
 
+#ifdef CONFIG_TPM_LOAD_KEY_BY_SHA1
+/**
+ * Search for a key by usage AuthData and the hash of the parent's pub key.
+ *
+ * @param auth	        Usage auth of the key to search for
+ * @param pubkey_digest	SHA1 hash of the pub key structure of the key
+ * @param[out] handle	The handle of the key (Non-null iff found)
+ * @return 0 if key was found in TPM; != 0 if not.
+ */
+uint32_t tpm_find_key_sha1(const uint8_t auth[20], const uint8_t
+			   pubkey_digest[20], uint32_t *handle);
+#endif /* CONFIG_TPM_LOAD_KEY_BY_SHA1 */
 #endif /* __TPM_H */
diff --git a/lib/tpm.c b/lib/tpm.c
index fb1221472a..cd7f88f220 100644
--- a/lib/tpm.c
+++ b/lib/tpm.c
@@ -996,4 +996,44 @@ uint32_t tpm_get_pub_key_oiap(uint32_t key_handle, const void *usage_auth,
 	return 0;
 }
 
+#ifdef CONFIG_TPM_LOAD_KEY_BY_SHA1
+uint32_t tpm_find_key_sha1(const uint8_t auth[20], const uint8_t
+			   pubkey_digest[20], uint32_t *handle)
+{
+	uint16_t key_count;
+	uint32_t key_handles[10];
+	uint8_t buf[288];
+	uint8_t *ptr;
+	uint32_t err;
+	uint8_t digest[20];
+	size_t buf_len;
+	unsigned int i;
+
+	/* fetch list of already loaded keys in the TPM */
+	err = tpm_get_capability(TPM_CAP_HANDLE, TPM_RT_KEY, buf, sizeof(buf));
+	if (err)
+		return -1;
+	key_count = get_unaligned_be16(buf);
+	ptr = buf + 2;
+	for (i = 0; i < key_count; ++i, ptr += 4)
+		key_handles[i] = get_unaligned_be32(ptr);
+
+	/* now search a(/ the) key which we can access with the given auth */
+	for (i = 0; i < key_count; ++i) {
+		buf_len = sizeof(buf);
+		err = tpm_get_pub_key_oiap(key_handles[i], auth, buf, &buf_len);
+		if (err && err != TPM_AUTHFAIL)
+			return -1;
+		if (err)
+			continue;
+		sha1_csum(buf, buf_len, digest);
+		if (!memcmp(digest, pubkey_digest, 20)) {
+			*handle = key_handles[i];
+			return 0;
+		}
+	}
+	return 1;
+}
+#endif /* CONFIG_TPM_LOAD_KEY_BY_SHA1 */
+
 #endif /* CONFIG_TPM_AUTH_SESSIONS */
-- 
2.11.0

[U-Boot] [PATCH 2/3] cmd: tpm: Fix flush command

[Mario Six]

Commit 7690be35de ("lib: tpm: Add command to flush resources") added a command
to flush resources from a TPM.

However, a previous development version was accidentially used to generate the
patch, resulting in a non-functional command.

This patch fixes the flush command.

Signed-off-by: Mario Six <mario.six@gdsys.cc>
---
 cmd/tpm.c | 37 +++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/cmd/tpm.c b/cmd/tpm.c
index 91bd20da25..e3d26b714c 100644
--- a/cmd/tpm.c
+++ b/cmd/tpm.c
@@ -691,31 +691,36 @@ static int do_tpm_flush(cmd_tbl_t *cmdtp, int flag, int argc,
 {
 	int type = 0;
 
-	if (argc != 2)
+	if (argc != 3)
 		return CMD_RET_USAGE;
 
-	if (strcasecmp(argv[1], "key"))
+	if (!strcasecmp(argv[1], "key"))
 		type = TPM_RT_KEY;
-	else if (strcasecmp(argv[1], "auth"))
+	else if (!strcasecmp(argv[1], "auth"))
 		type = TPM_RT_AUTH;
-	else if (strcasecmp(argv[1], "hash"))
+	else if (!strcasecmp(argv[1], "hash"))
 		type = TPM_RT_HASH;
-	else if (strcasecmp(argv[1], "trans"))
+	else if (!strcasecmp(argv[1], "trans"))
 		type = TPM_RT_TRANS;
-	else if (strcasecmp(argv[1], "context"))
+	else if (!strcasecmp(argv[1], "context"))
 		type = TPM_RT_CONTEXT;
-	else if (strcasecmp(argv[1], "counter"))
+	else if (!strcasecmp(argv[1], "counter"))
 		type = TPM_RT_COUNTER;
-	else if (strcasecmp(argv[1], "delegate"))
+	else if (!strcasecmp(argv[1], "delegate"))
 		type = TPM_RT_DELEGATE;
-	else if (strcasecmp(argv[1], "daa_tpm"))
+	else if (!strcasecmp(argv[1], "daa_tpm"))
 		type = TPM_RT_DAA_TPM;
-	else if (strcasecmp(argv[1], "daa_v0"))
+	else if (!strcasecmp(argv[1], "daa_v0"))
 		type = TPM_RT_DAA_V0;
-	else if (strcasecmp(argv[1], "daa_v1"))
+	else if (!strcasecmp(argv[1], "daa_v1"))
 		type = TPM_RT_DAA_V1;
 
-	if (strcasecmp(argv[2], "all")) {
+	if (!type) {
+		printf("Resource type %s unknown.\n", argv[1]);
+		return -1;
+	}
+
+	if (!strcasecmp(argv[2], "all")) {
 		uint16_t res_count;
 		uint8_t buf[288];
 		uint8_t *ptr;
@@ -725,8 +730,10 @@ static int do_tpm_flush(cmd_tbl_t *cmdtp, int flag, int argc,
 		/* fetch list of already loaded resources in the TPM */
 		err = tpm_get_capability(TPM_CAP_HANDLE, type, buf,
 					 sizeof(buf));
-		if (err)
+		if (err) {
+			printf("tpm_get_capability returned error %d.\n", err);
 			return -1;
+		}
 		res_count = get_unaligned_be16(buf);
 		ptr = buf + 2;
 		for (i = 0; i < res_count; ++i, ptr += 4)
@@ -734,8 +741,10 @@ static int do_tpm_flush(cmd_tbl_t *cmdtp, int flag, int argc,
 	} else {
 		uint32_t handle = simple_strtoul(argv[2], NULL, 0);
 
-		if (!handle)
+		if (!handle) {
+			printf("Illegal resource handle %s\n", argv[2]);
 			return -1;
+		}
 		tpm_flush_specific(cpu_to_be32(handle), type);
 	}
 
-- 
2.11.0

[U-Boot] [PATCH 3/3] lib: tpm: Add command to list resources

[Mario Six]

It is sometimes convenient to know how many and/or which resources are
currently loaded into a TPG, e.g. to test is a flush operation succeeded.

Hence, we add a command that lists the resources of a given type currently
loaded into the TPM.

Signed-off-by: Mario Six <mario.six@gdsys.cc>
---
 cmd/tpm.c           | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 drivers/tpm/Kconfig |  7 +++++
 2 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/cmd/tpm.c b/cmd/tpm.c
index e3d26b714c..0c4bc73ca6 100644
--- a/cmd/tpm.c
+++ b/cmd/tpm.c
@@ -752,6 +752,68 @@ static int do_tpm_flush(cmd_tbl_t *cmdtp, int flag, int argc,
 }
 #endif /* CONFIG_TPM_FLUSH_RESOURCES */
 
+#ifdef CONFIG_TPM_LIST_RESOURCES
+static int do_tpm_list(cmd_tbl_t *cmdtp, int flag, int argc,
+		       char * const argv[])
+{
+	int type = 0;
+	uint16_t res_count;
+	uint8_t buf[288];
+	uint8_t *ptr;
+	int err;
+	uint i;
+
+	if (argc != 2)
+		return CMD_RET_USAGE;
+
+	if (!strcasecmp(argv[1], "key"))
+		type = TPM_RT_KEY;
+	else if (!strcasecmp(argv[1], "auth"))
+		type = TPM_RT_AUTH;
+	else if (!strcasecmp(argv[1], "hash"))
+		type = TPM_RT_HASH;
+	else if (!strcasecmp(argv[1], "trans"))
+		type = TPM_RT_TRANS;
+	else if (!strcasecmp(argv[1], "context"))
+		type = TPM_RT_CONTEXT;
+	else if (!strcasecmp(argv[1], "counter"))
+		type = TPM_RT_COUNTER;
+	else if (!strcasecmp(argv[1], "delegate"))
+		type = TPM_RT_DELEGATE;
+	else if (!strcasecmp(argv[1], "daa_tpm"))
+		type = TPM_RT_DAA_TPM;
+	else if (!strcasecmp(argv[1], "daa_v0"))
+		type = TPM_RT_DAA_V0;
+	else if (!strcasecmp(argv[1], "daa_v1"))
+		type = TPM_RT_DAA_V1;
+
+	if (!type) {
+		printf("Resource type %s unknown.\n", argv[1]);
+		return -1;
+	}
+
+	/* fetch list of already loaded resources in the TPM */
+	err = tpm_get_capability(TPM_CAP_HANDLE, type, buf,
+				 sizeof(buf));
+	if (err) {
+		printf("tpm_get_capability returned error %d.\n", err);
+		return -1;
+	}
+	res_count = get_unaligned_be16(buf);
+	ptr = buf + 2;
+
+	printf("Resources of type %s (%02x):\n", argv[1], type);
+	if (!res_count) {
+		puts("None\n");
+	} else {
+		for (i = 0; i < res_count; ++i, ptr += 4)
+			printf("Index %d: %08x\n", i, get_unaligned_be32(ptr));
+	}
+
+	return 0;
+}
+#endif /* CONFIG_TPM_LIST_RESOURCES */
+
 #define MAKE_TPM_CMD_ENTRY(cmd) \
 	U_BOOT_CMD_MKENT(cmd, 0, 1, do_tpm_ ## cmd, "", "")
 
@@ -815,6 +877,10 @@ static cmd_tbl_t tpm_commands[] = {
 	U_BOOT_CMD_MKENT(flush, 0, 1,
 			 do_tpm_flush, "", ""),
 #endif /* CONFIG_TPM_FLUSH_RESOURCES */
+#ifdef CONFIG_TPM_LIST_RESOURCES
+	U_BOOT_CMD_MKENT(list, 0, 1,
+			 do_tpm_list, "", ""),
+#endif /* CONFIG_TPM_LIST_RESOURCES */
 };
 
 static int do_tpm(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
@@ -864,14 +930,22 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
 "  get_capability cap_area sub_cap addr count\n"
 "    - Read <count> bytes of TPM capability indexed by <cap_area> and\n"
 "      <sub_cap> to memory address <addr>.\n"
-#ifdef CONFIG_TPM_FLUSH_RESOURCES
+#if defined(CONFIG_TPM_FLUSH_RESOURCES) || defined(CONFIG_TPM_LIST_RESOURCES)
 "Resource management functions\n"
+#endif
+#ifdef CONFIG_TPM_FLUSH_RESOURCES
 "  flush resource_type id\n"
 "    - flushes a resource of type <resource_type> (may be one of key, auth,\n"
 "      hash, trans, context, counter, delegate, daa_tpm, daa_v0, daa_v1),\n"
 "      and id <id> from the TPM. Use an <id> of \"all\" to flush all\n"
 "      resources of that type.\n"
 #endif /* CONFIG_TPM_FLUSH_RESOURCES */
+#ifdef CONFIG_TPM_LIST_RESOURCES
+"  list resource_type\n"
+"    - lists resources of type <resource_type> (may be one of key, auth,\n"
+"      hash, trans, context, counter, delegate, daa_tpm, daa_v0, daa_v1),\n"
+"      contained in the TPM.\n"
+#endif /* CONFIG_TPM_LIST_RESOURCES */
 #ifdef CONFIG_TPM_AUTH_SESSIONS
 "Storage functions\n"
 "  loadkey2_oiap parent_handle key_addr key_len usage_auth\n"
diff --git a/drivers/tpm/Kconfig b/drivers/tpm/Kconfig
index a54b6a988a..2a64bc49c3 100644
--- a/drivers/tpm/Kconfig
+++ b/drivers/tpm/Kconfig
@@ -96,4 +96,11 @@ config TPM_LOAD_KEY_BY_SHA1
 	  Enable support to load keys into the TPM by identifying
 	  their parent via the public key's SHA1 hash.
 	  The functionality is available via the 'tpm' command as well.
+
+config TPM_LIST_RESOURCES
+	bool "Enable TPM resource listing support"
+	depends on TPM
+	help
+	  Enable support to list specific resources (e.g. keys) within the TPM.
+	  The functionality is available via the 'tpm' command as well.
 endmenu
-- 
2.11.0

[U-Boot] [PATCH 1/3] tpm: Add function to load keys via their parent's SHA1 hash

[Simon Glass]

On 20 March 2017 at 03:28, Mario Six <mario.six@gdsys.cc> wrote:
> If we want to load a key into a TPM, we need to know the designated parent
> key's handle, so that the TPM is able to insert the key at the correct place in
> the key hierarchy.
>
> However, if we want to load a key whose designated parent key we also
> previously loaded ourselves, we first need to memorize this parent key's handle
> (since the handles for the key are chosen at random when they are inserted into
> the TPM). If we are, however, unable to do so, for example if the parent key is
> loaded into the TPM during production, and its child key during the actual
> boot, we must find a different mechanism to identify the parent key.
>
> To solve this problem, we add a function that allows U-Boot to load a key into
> the TPM using their designated parent key's SHA1 hash, and the corresponding
> auth data.
>
> Signed-off-by: Mario Six <mario.six@gdsys.cc>
> ---
>  cmd/tpm.c           | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/tpm/Kconfig |  8 ++++++++
>  include/tpm.h       | 12 ++++++++++++
>  lib/tpm.c           | 40 ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 109 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

Perhaps you don't need a new Kconfig option? Is that to save code space?

[U-Boot] [PATCH 2/3] cmd: tpm: Fix flush command

[Simon Glass]

On 20 March 2017 at 03:28, Mario Six <mario.six@gdsys.cc> wrote:
> Commit 7690be35de ("lib: tpm: Add command to flush resources") added a command
> to flush resources from a TPM.
>
> However, a previous development version was accidentially used to generate the
> patch, resulting in a non-functional command.
>
> This patch fixes the flush command.
>
> Signed-off-by: Mario Six <mario.six@gdsys.cc>
> ---
>  cmd/tpm.c | 37 +++++++++++++++++++++++--------------
>  1 file changed, 23 insertions(+), 14 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 3/3] lib: tpm: Add command to list resources

[Simon Glass]

On 20 March 2017 at 03:28, Mario Six <mario.six@gdsys.cc> wrote:
> It is sometimes convenient to know how many and/or which resources are
> currently loaded into a TPG, e.g. to test is a flush operation succeeded.
>
> Hence, we add a command that lists the resources of a given type currently
> loaded into the TPM.
>
> Signed-off-by: Mario Six <mario.six@gdsys.cc>
> ---
>  cmd/tpm.c           | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  drivers/tpm/Kconfig |  7 +++++
>  2 files changed, 82 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Again I wonder if we need the CONFIG.

[U-Boot] [PATCH 1/3] tpm: Add function to load keys via their parent's SHA1 hash

[Mario Six]

On Wed, Mar 22, 2017 at 2:05 PM, Simon Glass <sjg@chromium.org> wrote:
> On 20 March 2017 at 03:28, Mario Six <mario.six@gdsys.cc> wrote:
>> If we want to load a key into a TPM, we need to know the designated parent
>> key's handle, so that the TPM is able to insert the key at the correct place in
>> the key hierarchy.
>>
>> However, if we want to load a key whose designated parent key we also
>> previously loaded ourselves, we first need to memorize this parent key's handle
>> (since the handles for the key are chosen at random when they are inserted into
>> the TPM). If we are, however, unable to do so, for example if the parent key is
>> loaded into the TPM during production, and its child key during the actual
>> boot, we must find a different mechanism to identify the parent key.
>>
>> To solve this problem, we add a function that allows U-Boot to load a key into
>> the TPM using their designated parent key's SHA1 hash, and the corresponding
>> auth data.
>>
>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>> ---
>>  cmd/tpm.c           | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/tpm/Kconfig |  8 ++++++++
>>  include/tpm.h       | 12 ++++++++++++
>>  lib/tpm.c           | 40 ++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 109 insertions(+)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Perhaps you don't need a new Kconfig option? Is that to save code space?
>
>

Yes, it's primarily to save code space. I haven't really investigated how much
this option does impact the overall size, but since every recent addition to
the TPM library was guarded with a new Kconfig option, I thought it was prudent
to emulate that.

If you think it's overkill, I can drop the option, and just have it
compiled in by default.

Thanks, and best regards,

Mario

[U-Boot] [PATCH 1/3] tpm: Add function to load keys via their parent's SHA1 hash

[Simon Glass]

Hi Mario,

On 22 March 2017 at 07:20, Mario Six <mario.six@gdsys.cc> wrote:
> On Wed, Mar 22, 2017 at 2:05 PM, Simon Glass <sjg@chromium.org> wrote:
>> On 20 March 2017 at 03:28, Mario Six <mario.six@gdsys.cc> wrote:
>>> If we want to load a key into a TPM, we need to know the designated parent
>>> key's handle, so that the TPM is able to insert the key at the correct place in
>>> the key hierarchy.
>>>
>>> However, if we want to load a key whose designated parent key we also
>>> previously loaded ourselves, we first need to memorize this parent key's handle
>>> (since the handles for the key are chosen at random when they are inserted into
>>> the TPM). If we are, however, unable to do so, for example if the parent key is
>>> loaded into the TPM during production, and its child key during the actual
>>> boot, we must find a different mechanism to identify the parent key.
>>>
>>> To solve this problem, we add a function that allows U-Boot to load a key into
>>> the TPM using their designated parent key's SHA1 hash, and the corresponding
>>> auth data.
>>>
>>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>>> ---
>>>  cmd/tpm.c           | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>  drivers/tpm/Kconfig |  8 ++++++++
>>>  include/tpm.h       | 12 ++++++++++++
>>>  lib/tpm.c           | 40 ++++++++++++++++++++++++++++++++++++++++
>>>  4 files changed, 109 insertions(+)
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> Perhaps you don't need a new Kconfig option? Is that to save code space?
>>
>>
>
> Yes, it's primarily to save code space. I haven't really investigated how much
> this option does impact the overall size, but since every recent addition to
> the TPM library was guarded with a new Kconfig option, I thought it was prudent
> to emulate that.
>
> If you think it's overkill, I can drop the option, and just have it
> compiled in by default.

I think for now it is overkill, and I'm happy to just include the new
functionality always. We have a sandbox tpm emulator - can we use that
to write tests?

Regards,
Simon

[U-Boot] [PATCH 1/3] tpm: Add function to load keys via their parent's SHA1 hash

[Mario Six]

On Wed, Mar 22, 2017 at 2:27 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Mario,
>
> On 22 March 2017 at 07:20, Mario Six <mario.six@gdsys.cc> wrote:
>> On Wed, Mar 22, 2017 at 2:05 PM, Simon Glass <sjg@chromium.org> wrote:
>>> On 20 March 2017 at 03:28, Mario Six <mario.six@gdsys.cc> wrote:
>>>> If we want to load a key into a TPM, we need to know the designated parent
>>>> key's handle, so that the TPM is able to insert the key at the correct place in
>>>> the key hierarchy.
>>>>
>>>> However, if we want to load a key whose designated parent key we also
>>>> previously loaded ourselves, we first need to memorize this parent key's handle
>>>> (since the handles for the key are chosen at random when they are inserted into
>>>> the TPM). If we are, however, unable to do so, for example if the parent key is
>>>> loaded into the TPM during production, and its child key during the actual
>>>> boot, we must find a different mechanism to identify the parent key.
>>>>
>>>> To solve this problem, we add a function that allows U-Boot to load a key into
>>>> the TPM using their designated parent key's SHA1 hash, and the corresponding
>>>> auth data.
>>>>
>>>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>>>> ---
>>>>  cmd/tpm.c           | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  drivers/tpm/Kconfig |  8 ++++++++
>>>>  include/tpm.h       | 12 ++++++++++++
>>>>  lib/tpm.c           | 40 ++++++++++++++++++++++++++++++++++++++++
>>>>  4 files changed, 109 insertions(+)
>>>
>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>
>>> Perhaps you don't need a new Kconfig option? Is that to save code space?
>>>
>>>
>>
>> Yes, it's primarily to save code space. I haven't really investigated how much
>> this option does impact the overall size, but since every recent addition to
>> the TPM library was guarded with a new Kconfig option, I thought it was prudent
>> to emulate that.
>>
>> If you think it's overkill, I can drop the option, and just have it
>> compiled in by default.
>
> I think for now it is overkill, and I'm happy to just include the new
> functionality always. We have a sandbox tpm emulator - can we use that
> to write tests?
>
> Regards,
> Simon
>

OK, no Kconfig option is good as well. :-)

As for tests, I took a quick look at tpm_tis_sandbox.c: Right now, the driver
doesn't support the TPM_LoadKey2 command, which is used to implement the
loading mechanism. And it's decidedly non-trivial to implement it, primarily
for the reason that U-Boot, at the moment, doesn't provide all the needed
cryptographic primitives (the key blob that is loaded in is cryptographically
secured). I know that RSA is implemented, but we would require OAEP padding,
which is not implemented, and we would also need AES-128 in CBC mode. This
could be overcome if we could somehow gain access to the host system's OpenSSL
library from within the sandbox driver. Would that be possible? Then again, it
would be pretty nice if we had working OAEP padding available for FIT image
signing, so it might be worth implementing it.

So, bottom line: I'll look into it, but it will definitely take a while to have
something usable at hand.

Best regards,

Mario

[U-Boot] [PATCH 1/3] tpm: Add function to load keys via their parent's SHA1 hash

[Simon Glass]

Hi Mario,

On 22 March 2017 at 08:07, Mario Six <mario.six@gdsys.cc> wrote:
> On Wed, Mar 22, 2017 at 2:27 PM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Mario,
>>
>> On 22 March 2017 at 07:20, Mario Six <mario.six@gdsys.cc> wrote:
>>> On Wed, Mar 22, 2017 at 2:05 PM, Simon Glass <sjg@chromium.org> wrote:
>>>> On 20 March 2017 at 03:28, Mario Six <mario.six@gdsys.cc> wrote:
>>>>> If we want to load a key into a TPM, we need to know the designated parent
>>>>> key's handle, so that the TPM is able to insert the key at the correct place in
>>>>> the key hierarchy.
>>>>>
>>>>> However, if we want to load a key whose designated parent key we also
>>>>> previously loaded ourselves, we first need to memorize this parent key's handle
>>>>> (since the handles for the key are chosen at random when they are inserted into
>>>>> the TPM). If we are, however, unable to do so, for example if the parent key is
>>>>> loaded into the TPM during production, and its child key during the actual
>>>>> boot, we must find a different mechanism to identify the parent key.
>>>>>
>>>>> To solve this problem, we add a function that allows U-Boot to load a key into
>>>>> the TPM using their designated parent key's SHA1 hash, and the corresponding
>>>>> auth data.
>>>>>
>>>>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>>>>> ---
>>>>>  cmd/tpm.c           | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>  drivers/tpm/Kconfig |  8 ++++++++
>>>>>  include/tpm.h       | 12 ++++++++++++
>>>>>  lib/tpm.c           | 40 ++++++++++++++++++++++++++++++++++++++++
>>>>>  4 files changed, 109 insertions(+)
>>>>
>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>>
>>>> Perhaps you don't need a new Kconfig option? Is that to save code space?
>>>>
>>>>
>>>
>>> Yes, it's primarily to save code space. I haven't really investigated how much
>>> this option does impact the overall size, but since every recent addition to
>>> the TPM library was guarded with a new Kconfig option, I thought it was prudent
>>> to emulate that.
>>>
>>> If you think it's overkill, I can drop the option, and just have it
>>> compiled in by default.
>>
>> I think for now it is overkill, and I'm happy to just include the new
>> functionality always. We have a sandbox tpm emulator - can we use that
>> to write tests?
>>
>> Regards,
>> Simon
>>
>
> OK, no Kconfig option is good as well. :-)
>
> As for tests, I took a quick look at tpm_tis_sandbox.c: Right now, the driver
> doesn't support the TPM_LoadKey2 command, which is used to implement the
> loading mechanism. And it's decidedly non-trivial to implement it, primarily
> for the reason that U-Boot, at the moment, doesn't provide all the needed
> cryptographic primitives (the key blob that is loaded in is cryptographically
> secured). I know that RSA is implemented, but we would require OAEP padding,
> which is not implemented, and we would also need AES-128 in CBC mode. This
> could be overcome if we could somehow gain access to the host system's OpenSSL
> library from within the sandbox driver. Would that be possible? Then again, it
> would be pretty nice if we had working OAEP padding available for FIT image
> signing, so it might be worth implementing it.

Yes you can access OpenSSL - see for example os.c which is built by
the host tools and provides an interface between U-Boot and the C
libraries.

Also bear in mind that one option is to implement a 'fake', where it
appears to do the right thing, but in fact fakes most of its actions,
so that (for example) it doesn't provide any security checks. I'm not
suggesting that, but just pointing out that the primary purpose of
test code in U-Boot is to test U-Boot., so we don't need such faithful
implementations.

>
> So, bottom line: I'll look into it, but it will definitely take a while to have
> something usable at hand.
>
> Best regards,
>
> Mario

Regards,
Simon

[U-Boot] [PATCH 3/3] lib: tpm: Add command to list resources

[Mario Six]

On Wed, Mar 22, 2017 at 2:05 PM, Simon Glass <sjg@chromium.org> wrote:
> On 20 March 2017 at 03:28, Mario Six <mario.six@gdsys.cc> wrote:
>> It is sometimes convenient to know how many and/or which resources are
>> currently loaded into a TPG, e.g. to test is a flush operation succeeded.
>>
>> Hence, we add a command that lists the resources of a given type currently
>> loaded into the TPM.
>>
>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>> ---
>>  cmd/tpm.c           | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  drivers/tpm/Kconfig |  7 +++++
>>  2 files changed, 82 insertions(+), 1 deletion(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Again I wonder if we need the CONFIG.
>

Thanks for the review!

As for the CONFIG option, well, there is the trivial symmetry reason that the
flush command is deactivatable, so this should be too (since they are,
essentially, complementary functions, one view, one deletion).

Also, the list function is really more of a debug tool than a function that
should be in a production environment.

And, the most important reason why I think the CONFIG is justified is this:
should a embedded device with a TPM that's using U-Boot as a boot loader be
subjected to a security evaluation (e.g. Common Criteria), an evaluator might
ask why a function like this, which, essentially has no real purpose aside from
providing debug information, is part of the TOE (especially if the TPM is used
as a fundamental security mechanism in the design). It enables an attacker that
gains access to the U-Boot console to, for example, read the handles of the
keys stored in the TPM, which is already one part of the data needed to access
them. Granted, it's not a huge advantage, but the best answer you can give an
evaluator is always "That's not possible" :-).

So, from a user perspective, I think it's desirable to have to option to
deactivate this function.

Best regards,

Mario

[U-Boot] [PATCH 1/3] tpm: Add function to load keys via their parent's SHA1 hash

[Simon Glass]

On 22 March 2017 at 08:47, Simon Glass <sjg@chromium.org> wrote:
> Hi Mario,
>
> On 22 March 2017 at 08:07, Mario Six <mario.six@gdsys.cc> wrote:
>> On Wed, Mar 22, 2017 at 2:27 PM, Simon Glass <sjg@chromium.org> wrote:
>>> Hi Mario,
>>>
>>> On 22 March 2017 at 07:20, Mario Six <mario.six@gdsys.cc> wrote:
>>>> On Wed, Mar 22, 2017 at 2:05 PM, Simon Glass <sjg@chromium.org> wrote:
>>>>> On 20 March 2017 at 03:28, Mario Six <mario.six@gdsys.cc> wrote:
>>>>>> If we want to load a key into a TPM, we need to know the designated parent
>>>>>> key's handle, so that the TPM is able to insert the key at the correct place in
>>>>>> the key hierarchy.
>>>>>>
>>>>>> However, if we want to load a key whose designated parent key we also
>>>>>> previously loaded ourselves, we first need to memorize this parent key's handle
>>>>>> (since the handles for the key are chosen at random when they are inserted into
>>>>>> the TPM). If we are, however, unable to do so, for example if the parent key is
>>>>>> loaded into the TPM during production, and its child key during the actual
>>>>>> boot, we must find a different mechanism to identify the parent key.
>>>>>>
>>>>>> To solve this problem, we add a function that allows U-Boot to load a key into
>>>>>> the TPM using their designated parent key's SHA1 hash, and the corresponding
>>>>>> auth data.
>>>>>>
>>>>>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>>>>>> ---
>>>>>>  cmd/tpm.c           | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>  drivers/tpm/Kconfig |  8 ++++++++
>>>>>>  include/tpm.h       | 12 ++++++++++++
>>>>>>  lib/tpm.c           | 40 ++++++++++++++++++++++++++++++++++++++++
>>>>>>  4 files changed, 109 insertions(+)
>>>>>
>>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>>>
>>>>> Perhaps you don't need a new Kconfig option? Is that to save code space?
>>>>>
>>>>>
>>>>
>>>> Yes, it's primarily to save code space. I haven't really investigated how much
>>>> this option does impact the overall size, but since every recent addition to
>>>> the TPM library was guarded with a new Kconfig option, I thought it was prudent
>>>> to emulate that.
>>>>
>>>> If you think it's overkill, I can drop the option, and just have it
>>>> compiled in by default.
>>>
>>> I think for now it is overkill, and I'm happy to just include the new
>>> functionality always. We have a sandbox tpm emulator - can we use that
>>> to write tests?
>>>
>>> Regards,
>>> Simon
>>>
>>
>> OK, no Kconfig option is good as well. :-)
>>
>> As for tests, I took a quick look at tpm_tis_sandbox.c: Right now, the driver
>> doesn't support the TPM_LoadKey2 command, which is used to implement the
>> loading mechanism. And it's decidedly non-trivial to implement it, primarily
>> for the reason that U-Boot, at the moment, doesn't provide all the needed
>> cryptographic primitives (the key blob that is loaded in is cryptographically
>> secured). I know that RSA is implemented, but we would require OAEP padding,
>> which is not implemented, and we would also need AES-128 in CBC mode. This
>> could be overcome if we could somehow gain access to the host system's OpenSSL
>> library from within the sandbox driver. Would that be possible? Then again, it
>> would be pretty nice if we had working OAEP padding available for FIT image
>> signing, so it might be worth implementing it.
>
> Yes you can access OpenSSL - see for example os.c which is built by
> the host tools and provides an interface between U-Boot and the C
> libraries.
>
> Also bear in mind that one option is to implement a 'fake', where it
> appears to do the right thing, but in fact fakes most of its actions,
> so that (for example) it doesn't provide any security checks. I'm not
> suggesting that, but just pointing out that the primary purpose of
> test code in U-Boot is to test U-Boot., so we don't need such faithful
> implementations.
>
>>
>> So, bottom line: I'll look into it, but it will definitely take a while to have
>> something usable at hand.
>>

OK, keep it simple!

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH 2/3] cmd: tpm: Fix flush command

[Simon Glass]

On 22 March 2017 at 07:05, Simon Glass <sjg@chromium.org> wrote:
> On 20 March 2017 at 03:28, Mario Six <mario.six@gdsys.cc> wrote:
>> Commit 7690be35de ("lib: tpm: Add command to flush resources") added a command
>> to flush resources from a TPM.
>>
>> However, a previous development version was accidentially used to generate the
>> patch, resulting in a non-functional command.
>>
>> This patch fixes the flush command.
>>
>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>> ---
>>  cmd/tpm.c | 37 +++++++++++++++++++++++--------------
>>  1 file changed, 23 insertions(+), 14 deletions(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH 3/3] lib: tpm: Add command to list resources

[Simon Glass]

On 24 March 2017 at 03:54, Mario Six <mario.six@gdsys.cc> wrote:
> On Wed, Mar 22, 2017 at 2:05 PM, Simon Glass <sjg@chromium.org> wrote:
>> On 20 March 2017 at 03:28, Mario Six <mario.six@gdsys.cc> wrote:
>>> It is sometimes convenient to know how many and/or which resources are
>>> currently loaded into a TPG, e.g. to test is a flush operation succeeded.
>>>
>>> Hence, we add a command that lists the resources of a given type currently
>>> loaded into the TPM.
>>>
>>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>>> ---
>>>  cmd/tpm.c           | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>  drivers/tpm/Kconfig |  7 +++++
>>>  2 files changed, 82 insertions(+), 1 deletion(-)
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> Again I wonder if we need the CONFIG.
>>
>
> Thanks for the review!
>
> As for the CONFIG option, well, there is the trivial symmetry reason that the
> flush command is deactivatable, so this should be too (since they are,
> essentially, complementary functions, one view, one deletion).
>
> Also, the list function is really more of a debug tool than a function that
> should be in a production environment.
>
> And, the most important reason why I think the CONFIG is justified is this:
> should a embedded device with a TPM that's using U-Boot as a boot loader be
> subjected to a security evaluation (e.g. Common Criteria), an evaluator might
> ask why a function like this, which, essentially has no real purpose aside from
> providing debug information, is part of the TOE (especially if the TPM is used
> as a fundamental security mechanism in the design). It enables an attacker that
> gains access to the U-Boot console to, for example, read the handles of the
> keys stored in the TPM, which is already one part of the data needed to access
> them. Granted, it's not a huge advantage, but the best answer you can give an
> evaluator is always "That's not possible" :-).
>
> So, from a user perspective, I think it's desirable to have to option to
> deactivate this function.

OK well I'm OK with it.

>
> Best regards,
>
> Mario

Applied to u-boot-dm, thanks!