On Tue Feb 13, 2024 at 7:13 PM EET, James Bottomley wrote:
> Update tpm2_load_context() to return -EINVAL on integrity failures and
> use this as a signal when loading the NULL context that something
> might be wrong. If the signal fails, check the name of the NULL
> primary against the one stored in the chip data and if there is a
> mismatch disable the TPM because it is likely to have suffered a reset
> attack.
>
> Signed-off-by: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
> ---
> drivers/char/tpm/tpm-chip.c | 3 ++
> drivers/char/tpm/tpm2-sessions.c | 65 ++++++++++++++++++++++++++------
> drivers/char/tpm/tpm2-space.c | 3 ++
> include/linux/tpm.h | 4 +-
> 4 files changed, 62 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index d93937326b2e..854546000c92 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -158,6 +158,9 @@ int tpm_try_get_ops(struct tpm_chip *chip)
> {
> int rc = -EIO;
>
Inline comment here as a reminder of the purpose of the check:
> + if (chip->flags & TPM_CHIP_FLAG_DISABLE)
> + return rc;
> +
> get_device(&chip->dev);
>
> down_read(&chip->ops_sem);
> diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
> index 9d6da0c9652f..60a1a8746563 100644
> --- a/drivers/char/tpm/tpm2-sessions.c
> +++ b/drivers/char/tpm/tpm2-sessions.c
> @@ -87,6 +87,9 @@
> #define AES_KEYBITS (AES_KEYBYTES*8)
> #define AUTH_MAX_NAMES 3
>
> +static int tpm2_create_primary(struct tpm_chip *chip, u32 hierarchy,
> + u32 *handle, u8 *name);
Should be ordered in the way that this declaration is not required.
> +
> /*
> * This is the structure that carries all the auth information (like
> * session handle, nonces, session key and auth) from use to use it is
> @@ -847,6 +850,37 @@ static int tpm2_parse_start_auth_session(struct tpm2_auth *auth,
> return 0;
> }
>
> +static int tpm2_load_null(struct tpm_chip *chip, u32 *nullkey)
> +{
> + int rc;
> + unsigned int offset = 0; /* dummy offset for null seed context */
> + u8 name[SHA256_DIGEST_SIZE + 2];
> +
> + rc = tpm2_load_context(chip, chip->null_key_context, &offset,
> + nullkey);
> + if (rc != -EINVAL)
> + return rc;
> +
> + /* an integrity failure may mean the TPM has been reset */
> + dev_err(&chip->dev, "NULL key integrity failure!\n");
> + /* check the null name against what we know */
> + tpm2_create_primary(chip, TPM2_RH_NULL, NULL, name);
> + if (memcmp(name, chip->null_key_name, sizeof(name)) == 0)
> + /* name unchanged, assume transient integrity failure */
> + return rc;
> + /*
> + * Fatal TPM failure: the NULL seed has actually changed, so
> + * the TPM must have been illegally reset. All in-kernel TPM
> + * operations will fail because the NULL primary can't be
> + * loaded to salt the sessions, but disable the TPM anyway so
> + * userspace programms can't be compromised by it.
> + */
> + dev_err(&chip->dev, "NULL name has changed, disabling TPM due to interference\n");
> + chip->flags |= TPM_CHIP_FLAG_DISABLE;
> +
> + return rc;
> +}
> +
> /**
> * tpm2_start_auth_session() - create a HMAC authentication session with the TPM
> * @chip: the TPM chip structure to create the session with
> @@ -864,11 +898,9 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
> struct tpm_buf buf;
> struct tpm2_auth *auth = chip->auth;
> int rc;
> - unsigned int offset = 0; /* dummy offset for null seed context */
> u32 nullkey;
>
> - rc = tpm2_load_context(chip, chip->null_key_context, &offset,
> - &nullkey);
> + rc = tpm2_load_null(chip, &nullkey);
> if (rc)
> goto out;
>
> @@ -919,15 +951,19 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
> EXPORT_SYMBOL(tpm2_start_auth_session);
>
> static int tpm2_parse_create_primary(struct tpm_chip *chip, struct tpm_buf *buf,
> - u32 *nullkey)
> + u32 *handle, u8 *name)
Right sorry for my earlier comment this obviously cannot
tpm2_create_primary :-)
So tpm2_read_primary is probably in the ballpark given
that other more primitive functions are also "read".
> {
> struct tpm_header *head = (struct tpm_header *)buf->data;
> off_t offset_r = TPM_HEADER_SIZE, offset_t;
> u16 len = TPM_HEADER_SIZE;
> u32 tot_len = be32_to_cpu(head->length);
> - u32 val, parm_len;
> + u32 val, parm_len, keyhandle;
> + keyhandle = tpm_buf_read_u32(buf, &offset_r);
> + if (handle)
> + *handle = keyhandle;
> + else
> + tpm2_flush_context(chip, keyhandle);
>
> - *nullkey = tpm_buf_read_u32(buf, &offset_r);
> parm_len = tpm_buf_read_u32(buf, &offset_r);
> /*
> * parm_len doesn't include the header, but all the other
> @@ -940,9 +976,12 @@ static int tpm2_parse_create_primary(struct tpm_chip *chip, struct tpm_buf *buf,
> return -EINVAL;
> len = tpm_buf_read_u16(buf, &offset_r);
> offset_t = offset_r;
> - /* now we have the public area, compute the name of the object */
> - put_unaligned_be16(TPM_ALG_SHA256, chip->null_key_name);
> - sha256(&buf->data[offset_r], len, chip->null_key_name + 2);
> + if (name) {
> + /* now we have the public area, compute the name of
> + * the object */
> + put_unaligned_be16(TPM_ALG_SHA256, name);
> + sha256(&buf->data[offset_r], len, name + 2);
> + }
>
> /* validate the public key */
> val = tpm_buf_read_u16(buf, &offset_t);
> @@ -1054,7 +1093,8 @@ static int tpm2_parse_create_primary(struct tpm_chip *chip, struct tpm_buf *buf,
> return 0;
> }
>
> -static int tpm2_create_primary(struct tpm_chip *chip, u32 hierarchy, u32 *handle)
> +static int tpm2_create_primary(struct tpm_chip *chip, u32 hierarchy,
> + u32 *handle, u8 *name)
> {
> int rc;
> struct tpm_buf buf;
> @@ -1133,7 +1173,7 @@ static int tpm2_create_primary(struct tpm_chip *chip, u32 hierarchy, u32 *handle
> "attempting to create NULL primary");
>
> if (rc == TPM2_RC_SUCCESS)
> - rc = tpm2_parse_create_primary(chip, &buf, handle);
> + rc = tpm2_parse_create_primary(chip, &buf, handle, name);
>
> tpm_buf_destroy(&buf);
>
> @@ -1145,7 +1185,8 @@ static int tpm2_create_null_primary(struct tpm_chip *chip)
> u32 nullkey;
> int rc;
>
> - rc = tpm2_create_primary(chip, TPM2_RH_NULL, &nullkey);
> + rc = tpm2_create_primary(chip, TPM2_RH_NULL, &nullkey,
> + chip->null_key_name);
>
> if (rc == TPM2_RC_SUCCESS) {
> unsigned int offset = 0; /* dummy offset for null key context */
> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> index 24479a81c23c..4892d491da8d 100644
> --- a/drivers/char/tpm/tpm2-space.c
> +++ b/drivers/char/tpm/tpm2-space.c
> @@ -105,6 +105,9 @@ int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
> *handle = 0;
> tpm_buf_destroy(&tbuf);
> return -ENOENT;
> + } else if (tpm2_rc_value(rc) == TPM2_RC_INTEGRITY) {
> + tpm_buf_destroy(&tbuf);
> + return -EINVAL;
> } else if (rc > 0) {
> dev_warn(&chip->dev, "%s: failed with a TPM error 0x%04X\n",
> __func__, rc);
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 9c608fac8935..4474dabfb69d 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -244,6 +244,7 @@ enum tpm2_return_codes {
> TPM2_RC_SUCCESS = 0x0000,
> TPM2_RC_HASH = 0x0083, /* RC_FMT1 */
> TPM2_RC_HANDLE = 0x008B,
> + TPM2_RC_INTEGRITY = 0x009F,
> TPM2_RC_INITIALIZE = 0x0100, /* RC_VER1 */
> TPM2_RC_FAILURE = 0x0101,
> TPM2_RC_DISABLED = 0x0120,
> @@ -342,6 +343,7 @@ enum tpm_chip_flags {
> TPM_CHIP_FLAG_FIRMWARE_UPGRADE = BIT(7),
> TPM_CHIP_FLAG_SUSPENDED = BIT(8),
> TPM_CHIP_FLAG_HWRNG_DISABLED = BIT(9),
> + TPM_CHIP_FLAG_DISABLE = BIT(10),
> };
>
> #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
> @@ -428,7 +430,7 @@ static inline bool tpm_is_firmware_upgrade(struct tpm_chip *chip)
>
> static inline u32 tpm2_rc_value(u32 rc)
> {
> - return (rc & BIT(7)) ? rc & 0xff : rc;
> + return (rc & BIT(7)) ? rc & 0xbf : rc;
What is this change?
> }
>
> #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
BR, Jarkko
