| Message ID | 20170320092830.3040-4-mario.six@gdsys.cc |
|---|---|
| State | Accepted |
| Commit | 3d1df0e363ff13b7aed17f9bb576d045f26c3f74 |
| Delegated to: | Simon Glass |
| Headers | show |
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.
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
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!
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
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(-)