On Mon, Feb 28, 2022 at 05:11:19PM -0700, Simon Glass wrote:
> This feature is used for measured boot. It is not currently supported in
> the TPM drivers, but add it to the API so that code which expects it can
> signal its request.
>
> Signed-off-by: Simon Glass <s...@chromium.org>
> ---
>
> cmd/tpm-v1.c | 3 ++-
> cmd/tpm_test.c | 5 +++--
> include/tpm_api.h | 8 +++++---
> lib/tpm-v2.c | 2 ++
> lib/tpm_api.c | 14 ++++++++++----
> 5 files changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/cmd/tpm-v1.c b/cmd/tpm-v1.c
> index bf238a9f2e..0869b70775 100644
> --- a/cmd/tpm-v1.c
> +++ b/cmd/tpm-v1.c
> @@ -131,7 +131,8 @@ static int do_tpm_extend(struct cmd_tbl *cmdtp, int flag,
> int argc,
> return CMD_RET_FAILURE;
> }
>
> - rc = tpm_pcr_extend(dev, index, in_digest, out_digest);
> + rc = tpm_pcr_extend(dev, index, in_digest, sizeof(in_digest),
> + out_digest, "test");
Where is the output value of an extended PCR needed in measured boot?
IMHO this out_digest seems pointless. I'd be happier if we just completely
removed it and make the v2 variant look like v1 more.
> if (!rc) {
> puts("PCR value after execution of the command:\n");
> print_byte_string(out_digest, sizeof(out_digest));
> diff --git a/cmd/tpm_test.c b/cmd/tpm_test.c
> index a3ccb12f53..b35eae81dc 100644
> --- a/cmd/tpm_test.c
> +++ b/cmd/tpm_test.c
> @@ -91,7 +91,8 @@ static int test_early_extend(struct udevice *dev)
> tpm_init(dev);
> TPM_CHECK(tpm_startup(dev, TPM_ST_CLEAR));
> TPM_CHECK(tpm_continue_self_test(dev));
> - TPM_CHECK(tpm_pcr_extend(dev, 1, value_in, value_out));
> + TPM_CHECK(tpm_pcr_extend(dev, 1, value_in, sizeof(value_in), value_out,
> + "test"));
> printf("done\n");
> return 0;
> }
> @@ -438,7 +439,7 @@ static int test_timing(struct udevice *dev)
> 100);
> TTPM_CHECK(tpm_nv_read_value(dev, INDEX0, (uint8_t *)&x, sizeof(x)),
> 100);
> - TTPM_CHECK(tpm_pcr_extend(dev, 0, in, out), 200);
> + TTPM_CHECK(tpm_pcr_extend(dev, 0, in, sizeof(in), out, "test"), 200);
> TTPM_CHECK(tpm_set_global_lock(dev), 50);
> TTPM_CHECK(tpm_tsc_physical_presence(dev, PHYS_PRESENCE), 100);
> printf("done\n");
> diff --git a/include/tpm_api.h b/include/tpm_api.h
> index 11aa14eb79..3c8e48bc25 100644
> --- a/include/tpm_api.h
> +++ b/include/tpm_api.h
> @@ -81,14 +81,16 @@ u32 tpm_nv_write_value(struct udevice *dev, u32 index,
> const void *data,
> *
> * @param dev TPM device
> * @param index index of the PCR
> - * @param in_digest 160-bit value representing the event to be
> + * @param in_digest 160/256-bit value representing the event to be
> * recorded
> - * @param out_digest 160-bit PCR value after execution of the
> + * @param size size of digest in bytes
> + * @param out_digest 160/256-bit PCR value after execution of the
> * command
> + * @param name additional info about where the digest comes
> from
> * Return: return code of the operation
> */
> u32 tpm_pcr_extend(struct udevice *dev, u32 index, const void *in_digest,
> - void *out_digest);
> + uint size, void *out_digest, const char *name);
>
> /**
> * Issue a TPM_PCRRead command.
> diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
> index 1bf627853a..6058f2e1e4 100644
> --- a/lib/tpm-v2.c
> +++ b/lib/tpm-v2.c
> @@ -157,6 +157,8 @@ u32 tpm2_pcr_extend(struct udevice *dev, u32 index, u32
> algorithm,
> };
> int ret;
>
> + if (!digest)
> + return -EINVAL;
> /*
> * Fill the command structure starting from the first buffer:
> * - the digest
> diff --git a/lib/tpm_api.c b/lib/tpm_api.c
> index 4ac4612c81..a8d3731d3a 100644
> --- a/lib/tpm_api.c
> +++ b/lib/tpm_api.c
> @@ -140,15 +140,21 @@ u32 tpm_write_lock(struct udevice *dev, u32 index)
> }
>
> u32 tpm_pcr_extend(struct udevice *dev, u32 index, const void *in_digest,
> - void *out_digest)
> + uint size, void *out_digest, const char *name)
> {
> - if (tpm_is_v1(dev))
> + if (tpm_is_v1(dev)) {
> + if (size != PCR_DIGEST_LENGTH || !out_digest)
> + return -EINVAL;
> return tpm1_extend(dev, index, in_digest, out_digest);
> - else if (tpm_is_v2(dev))
> + } else if (tpm_is_v2(dev)) {
> + if (size != TPM2_SHA256_DIGEST_SIZE)
> + return -EINVAL;
Why are we limiting this? This is supposed to be dictated by the PCR bank
configuration of each hardware
> return tpm2_pcr_extend(dev, index, TPM2_ALG_SHA256, in_digest,
> TPM2_DIGEST_LEN);
> - else
> + /* @name is ignored as we do not support measured boot */
> + } else {
> return -ENOSYS;
> + }
> }
>
> u32 tpm_pcr_read(struct udevice *dev, u32 index, void *data, size_t count)
> --
> 2.35.1.574.g5d30c73bfb-goog
>
Thanks
/Ilias
