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

Message ID 20170320092830.3040-4-mario.six@gdsys.cc
State Accepted
Commit 3d1df0e363ff13b7aed17f9bb576d045f26c3f74
Delegated to: Simon Glass
Headers show

Commit Message

Mario Six March 20, 2017, 9:28 a.m.
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(-)

Comments

Simon Glass March 22, 2017, 1:05 p.m. | #1
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.
Mario Six March 24, 2017, 9:54 a.m. | #2
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
Simon Glass March 27, 2017, 2:27 a.m. | #3
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!

Patch

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