Re: [PATCH v2 05/10] security: keys: trusted: Verify creation data

Kees Cook
2022-09-20T23:07:07Z

On Tue, Aug 23, 2022 at 03:25:21PM -0700, Evan Green wrote:
> If a loaded key contains creation data, ask the TPM to verify that
> creation data. This allows users like encrypted hibernate to know that
> the loaded and parsed creation data has not been tampered with.
>
> Partially-sourced-from: Matthew Garrett <mjg59@google.com>
> Signed-off-by: Evan Green <evgreen@chromium.org>
>
> ---
> Source material for this change is at:
> https://patchwork.kernel.org/project/linux-pm/patch/20210220013255.1083202-9-matthewgarrett@google.com/
>
> Changes in v2:
> - Adjust hash len by 2 due to new ASN.1 storage, and add underflow
> check.
>
> include/linux/tpm.h | 1 +
> security/keys/trusted-keys/trusted_tpm2.c | 77 ++++++++++++++++++++++-
> 2 files changed, 77 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 8320cbac6f4009..438f8bc0a50582 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -224,6 +224,7 @@ enum tpm2_command_codes {
> TPM2_CC_SELF_TEST = 0x0143,
> TPM2_CC_STARTUP = 0x0144,
> TPM2_CC_SHUTDOWN = 0x0145,
> + TPM2_CC_CERTIFYCREATION = 0x014A,
> TPM2_CC_NV_READ = 0x014E,
> TPM2_CC_CREATE = 0x0153,
> TPM2_CC_LOAD = 0x0157,
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> index 1d1470b880ca01..f81c6578c7f783 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -691,6 +691,74 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> return rc;
> }
>
> +/**
> + * tpm2_certify_creation() - execute a TPM2_CertifyCreation command
> + *
> + * @chip: TPM chip to use
> + * @payload: the key data in clear and encrypted form
> + * @blob_handle: the loaded TPM handle of the key
> + *
> + * Return: 0 on success
> + * -EINVAL on tpm error status
> + * < 0 error from tpm_send or tpm_buf_init
> + */
> +static int tpm2_certify_creation(struct tpm_chip *chip,
> + struct trusted_key_payload *payload,
> + u32 blob_handle)
> +{
> + struct tpm_header *head;
> + struct tpm_buf buf;
> + int rc;
> +
> + rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CERTIFYCREATION);
> + if (rc)
> + return rc;
> +
> + /* Use TPM_RH_NULL for signHandle */
> + tpm_buf_append_u32(&buf, 0x40000007);
> +
> + /* Object handle */
> + tpm_buf_append_u32(&buf, blob_handle);
> +
> + /* Auth */
> + tpm_buf_append_u32(&buf, 9);
> + tpm_buf_append_u32(&buf, TPM2_RS_PW);
> + tpm_buf_append_u16(&buf, 0);
> + tpm_buf_append_u8(&buf, 0);
> + tpm_buf_append_u16(&buf, 0);
> +
> + /* Qualifying data */
> + tpm_buf_append_u16(&buf, 0);
> +
> + /* Creation data hash */
> + if (payload->creation_hash_len < 2) {
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + tpm_buf_append_u16(&buf, payload->creation_hash_len - 2);
> + tpm_buf_append(&buf, payload->creation_hash + 2,
> + payload->creation_hash_len - 2);
> +
> + /* signature scheme */
> + tpm_buf_append_u16(&buf, TPM_ALG_NULL);
> +
> + /* creation ticket */
> + tpm_buf_append(&buf, payload->tk, payload->tk_len);
> +
> + rc = tpm_transmit_cmd(chip, &buf, 6, "certifying creation data");
> + if (rc)
> + goto out;
> +
> + head = (struct tpm_header *)buf.data;
> +
> + if (head->return_code != 0)
> + rc = -EINVAL;

Do you have a reference to this TPM command spec? I have a dim memory of
some of these commands having success/failure listed separately from
other things in the reply. Is that true here? (i.e. is the return_code
only about "yes I replied" and there is a missing "but the answer is no"
check?)

> +out:
> + tpm_buf_destroy(&buf);
> + return rc;
> +}
> +
> /**
> * tpm2_unseal_trusted() - unseal the payload of a trusted key
> *
> @@ -716,8 +784,15 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
> goto out;
>
> rc = tpm2_unseal_cmd(chip, payload, options, blob_handle);
> - tpm2_flush_context(chip, blob_handle);
> + if (rc)
> + goto flush;
> +
> + if (payload->creation_len)
> + rc = tpm2_certify_creation(chip, payload, blob_handle);
>
> +
> +flush:
> + tpm2_flush_context(chip, blob_handle);
> out:
> tpm_put_ops(chip);
>
> --
> 2.31.0
>

Otherwise looks good to me. :)

--
Kees Cook