[U-Boot] [PATCH v2 11/19] tpm: add TPM2_Clear command support
Simon Glass
sjg at chromium.org
Thu Apr 26 14:40:27 UTC 2018
Hi Miquel,
On 24 April 2018 at 07:17, Miquel Raynal <miquel.raynal at bootlin.com> wrote:
> Hi Simon,
>
> On Fri, 30 Mar 2018 06:42:32 +0800, Simon Glass <sjg at chromium.org>
> wrote:
>
>> Hi Miquel,
>>
>> On 29 March 2018 at 15:43, Miquel Raynal <miquel.raynal at bootlin.com> wrote:
>> > Add support for the TPM2_Clear command.
>> >
>> > Change the command file and the help accordingly.
>> >
>> > Signed-off-by: Miquel Raynal <miquel.raynal at bootlin.com>
>> > ---
>> > cmd/tpm.c | 29 ++++++++++++++++++++++++++---
>> > cmd/tpm_test.c | 6 +++---
>> > include/tpm.h | 21 +++++++++++++++++----
>> > lib/tpm.c | 42 ++++++++++++++++++++++++++++++++++++++----
>> > 4 files changed, 84 insertions(+), 14 deletions(-)
>> >
>> > diff --git a/cmd/tpm.c b/cmd/tpm.c
>> > index fc9ef9d4a3..32921e1a70 100644
>> > --- a/cmd/tpm.c
>> > +++ b/cmd/tpm.c
>> > @@ -454,6 +454,29 @@ static int do_tpm_init(cmd_tbl_t *cmdtp, int flag,
>> > return report_return_code(tpm_init());
>> > }
>> >
>> > +static int do_tpm_force_clear(cmd_tbl_t *cmdtp, int flag,
>> > + int argc, char * const argv[])
>> > +{
>> > + u32 handle = 0;
>> > + const char *pw = (argc < 3) ? NULL : argv[2];
>> > + const ssize_t pw_sz = pw ? strlen(pw) : 0;
>> > +
>> > + if (argc < 2 || argc > 3)
>> > + return CMD_RET_USAGE;
>> > +
>> > + if (pw_sz > TPM2_DIGEST_LENGTH)
>> > + return -EINVAL;
>> > +
>> > + if (!strcasecmp("TPM2_RH_LOCKOUT", argv[1]))
>> > + handle = TPM2_RH_LOCKOUT;
>> > + else if (!strcasecmp("TPM2_RH_PLATFORM", argv[1]))
>> > + handle = TPM2_RH_PLATFORM;
>> > + else
>> > + return CMD_RET_USAGE;
>> > +
>> > + return report_return_code(tpm_force_clear(handle, pw, pw_sz));
>> > +}
>> > +
>> > #define TPM_COMMAND_NO_ARG(cmd) \
>> > static int do_##cmd(cmd_tbl_t *cmdtp, int flag, \
>> > int argc, char * const argv[]) \
>> > @@ -465,7 +488,6 @@ static int do_##cmd(cmd_tbl_t *cmdtp, int flag, \
>> >
>> > TPM_COMMAND_NO_ARG(tpm_self_test_full)
>> > TPM_COMMAND_NO_ARG(tpm_continue_self_test)
>> > -TPM_COMMAND_NO_ARG(tpm_force_clear)
>> > TPM_COMMAND_NO_ARG(tpm_physical_enable)
>> > TPM_COMMAND_NO_ARG(tpm_physical_disable)
>> >
>> > @@ -951,8 +973,9 @@ U_BOOT_CMD(tpm, CONFIG_SYS_MAXARGS, 1, do_tpm,
>> > " physical_set_deactivated 0|1\n"
>> > " - Set deactivated flag.\n"
>> > "Admin Ownership Commands:\n"
>> > -" force_clear\n"
>> > -" - Issue TPM_ForceClear command.\n"
>> > +" force_clear [<type>]\n"
>> > +" - Issue TPM_[Force]Clear command, with <type> one of (TPMv2 only):\n"
>> > +" * TPM2_RH_LOCKOUT, TPM2_RH_PLATFORM.\n"
>> > " tsc_physical_presence flags\n"
>> > " - Set TPM device's Physical Presence flags to <flags>.\n"
>> > "The Capability Commands:\n"
>> > diff --git a/cmd/tpm_test.c b/cmd/tpm_test.c
>> > index 37ad2ff33d..da40dbc423 100644
>> > --- a/cmd/tpm_test.c
>> > +++ b/cmd/tpm_test.c
>> > @@ -176,7 +176,7 @@ static int test_fast_enable(void)
>> > TPM_CHECK(tpm_get_flags(&disable, &deactivated, NULL));
>> > printf("\tdisable is %d, deactivated is %d\n", disable, deactivated);
>> > for (i = 0; i < 2; i++) {
>> > - TPM_CHECK(tpm_force_clear());
>> > + TPM_CHECK(tpm_force_clear(0, NULL, 0));
>> > TPM_CHECK(tpm_get_flags(&disable, &deactivated, NULL));
>> > printf("\tdisable is %d, deactivated is %d\n", disable,
>> > deactivated);
>> > @@ -458,7 +458,7 @@ static int test_write_limit(void)
>> > TPM_CHECK(TlclStartupIfNeeded());
>> > TPM_CHECK(tpm_self_test_full());
>> > TPM_CHECK(tpm_tsc_physical_presence(PRESENCE));
>> > - TPM_CHECK(tpm_force_clear());
>> > + TPM_CHECK(tpm_force_clear(0, NULL, 0));
>> > TPM_CHECK(tpm_physical_enable());
>> > TPM_CHECK(tpm_physical_set_deactivated(0));
>> >
>> > @@ -477,7 +477,7 @@ static int test_write_limit(void)
>> > }
>> >
>> > /* Reset write count */
>> > - TPM_CHECK(tpm_force_clear());
>> > + TPM_CHECK(tpm_force_clear(0, NULL, 0));
>> > TPM_CHECK(tpm_physical_enable());
>> > TPM_CHECK(tpm_physical_set_deactivated(0));
>> >
>> > diff --git a/include/tpm.h b/include/tpm.h
>> > index 38d7cb899d..2f17166662 100644
>> > --- a/include/tpm.h
>> > +++ b/include/tpm.h
>> > @@ -43,13 +43,23 @@ enum tpm_startup_type {
>> > };
>> >
>> > enum tpm2_startup_types {
>> > - TPM2_SU_CLEAR = 0x0000,
>> > - TPM2_SU_STATE = 0x0001,
>> > + TPM2_SU_CLEAR = 0x0000,
>> > + TPM2_SU_STATE = 0x0001,
>> > +};
>> > +
>> > +enum tpm2_handles {
>>
>> Please add comment to enum
>
> Fine, I will document all of them. Just for you to know, these are
> values extracted from the (publicly available) specification, I did
> not change any of them.
>
>>
>> > + TPM2_RH_OWNER = 0x40000001,
>> > + TPM2_RS_PW = 0x40000009,
>> > + TPM2_RH_LOCKOUT = 0x4000000A,
>> > + TPM2_RH_ENDORSEMENT = 0x4000000B,
>> > + TPM2_RH_PLATFORM = 0x4000000C,
>> > };
>> >
>> > enum tpm2_command_codes {
>> > TPM2_CC_STARTUP = 0x0144,
>> > TPM2_CC_SELF_TEST = 0x0143,
>> > + TPM2_CC_CLEAR = 0x0126,
>> > + TPM2_CC_CLEARCONTROL = 0x0127,
>> > TPM2_CC_GET_CAPABILITY = 0x017A,
>> > TPM2_CC_PCR_READ = 0x017E,
>> > TPM2_CC_PCR_EXTEND = 0x0182,
>> > @@ -567,11 +577,14 @@ uint32_t tpm_tsc_physical_presence(uint16_t presence);
>> > uint32_t tpm_read_pubek(void *data, size_t count);
>> >
>> > /**
>> > - * Issue a TPM_ForceClear command.
>> > + * Issue a TPM_ForceClear or a TPM2_Clear command.
>> > *
>> > + * @param handle Handle
>> > + * @param pw Password
>> > + * @param pw_sz Length of the password
>> > * @return return code of the operation
>> > */
>> > -uint32_t tpm_force_clear(void);
>> > +int tpm_force_clear(u32 handle, const char *pw, const ssize_t pw_sz);
>> >
>> > /**
>> > * Issue a TPM_PhysicalEnable command.
>> > diff --git a/lib/tpm.c b/lib/tpm.c
>> > index 3cc2888267..9a46ac09e6 100644
>> > --- a/lib/tpm.c
>> > +++ b/lib/tpm.c
>> > @@ -608,13 +608,47 @@ uint32_t tpm_read_pubek(void *data, size_t count)
>> > return 0;
>> > }
>> >
>> > -uint32_t tpm_force_clear(void)
>> > +int tpm_force_clear(u32 handle, const char *pw, const ssize_t pw_sz)
>> > {
>> > - const uint8_t command[10] = {
>> > - 0x0, 0xc1, 0x0, 0x0, 0x0, 0xa, 0x0, 0x0, 0x0, 0x5d,
>> > + const u8 command_v1[10] = {
>> > + U16_TO_ARRAY(0xc1),
>> > + U32_TO_ARRAY(10),
>> > + U32_TO_ARRAY(0x5d),
>> > };
>> > + u8 command_v2[COMMAND_BUFFER_SIZE] = {
>> > + U16_TO_ARRAY(TPM2_ST_SESSIONS), /* TAG */
>> > + U32_TO_ARRAY(27 + pw_sz), /* Length */
>> > + U32_TO_ARRAY(TPM2_CC_CLEAR), /* Command code */
>>
>> Here we have both v1 and v2 commands. Perhaps we should have a Kconfig
>> option to enable v2 support? Or do you think it is not a big addition
>> in terms of code side?
>
> It is a big addition in terms of code side.
>
>>
>> One way to do this would be to have an inline function which can tell
>> if you are using v2:
>>
>> static inline bool tpm_v2(void) {
>> return IS_ENABLED(CONFIG_TPM_V2) && further things...
>> }
>>
>> static inline bool tpm_v1(void) {
>> return IS_ENABLED(CONFIG_TPM_V1) && further things...
>> }
>
> I like this way of choosing one specification or the other, I could
> make them mutually exclusive. It would prevent the need for a global
> variable (or an additional field in uclass).
>
> Would it be fine to actually rename the file (with a 'tpm1' suffix) and
> create a new one specific for TPMv2 commands ? Only one file would be
> compiled/linked depending on the configuration, avoiding the possibility
> to call a v1 command from a v2 chip and vice versa.
>
> At first I thought a lot of code would be shared but I don't think we
> would add so much by having one function per revision now.
Well you could have two separate files, if there really is no sharing
of commands. But if there are common commands, you should have a
'common' file to implement them, rather than duplicating code.
With the above static inline functions we can support:
- v1 only (no code-size increment)
- v2 only (minimal code size for what will be a common case)
- v1 + v2 (e.g. for sandbox testing)
Regards,
Simon
More information about the U-Boot
mailing list