Re: [PATCH v8 22/22] tpm: disable the TPM if NULL name changes

["Jarkko Sakkinen" <jarkko@kernel.org>]

On Mon Apr 29, 2024 at 11:28 PM EEST, 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@HansenPartnership.com>
> ---
>  drivers/char/tpm/tpm-chip.c      |  3 ++
>  drivers/char/tpm/tpm2-sessions.c | 77 +++++++++++++++++++++++++-------
>  drivers/char/tpm/tpm2-space.c    |  3 ++
>  include/linux/tpm.h              |  4 +-
>  4 files changed, 70 insertions(+), 17 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;
>  
> +	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 3c97d3d5e00e..98819c9a0913 100644
> --- a/drivers/char/tpm/tpm2-sessions.c
> +++ b/drivers/char/tpm/tpm2-sessions.c
> @@ -80,6 +80,9 @@
>  /* maximum number of names the TPM must remember for authorization */
>  #define AUTH_MAX_NAMES	3
>  
> +static int tpm2_create_primary(struct tpm_chip *chip, u32 hierarchy,
> +			       u32 *handle, u8 *name);
> +
>  /*
>   * This is the structure that carries all the auth information (like
>   * session handle, nonces, session key and auth) from use to use it is
> @@ -851,6 +854,37 @@ static int tpm2_parse_start_auth_session(struct tpm2_auth *auth,
>  	return 0;
>  }
>  
> +static int tpm2_load_null(struct tpm_chip *chip, u32 *null_key)
> +{
> +	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,
> +			       null_key);
> +	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 programmes 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
> @@ -868,12 +902,9 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
>  	struct tpm_buf buf;
>  	struct tpm2_auth *auth = chip->auth;
>  	int rc;
> -	/* null seed context has no offset, but we must provide one */
> -	unsigned int offset = 0;
> -	u32 nullkey;
> +	u32 null_key;
>  
> -	rc = tpm2_load_context(chip, chip->null_key_context, &offset,
> -			       &nullkey);
> +	rc = tpm2_load_null(chip, &null_key);
>  	if (rc)
>  		goto out;
>  
> @@ -884,7 +915,7 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
>  		goto out;
>  
>  	/* salt key handle */
> -	tpm_buf_append_u32(&buf, nullkey);
> +	tpm_buf_append_u32(&buf, null_key);
>  	/* bind key handle */
>  	tpm_buf_append_u32(&buf, TPM2_RH_NULL);
>  	/* nonce caller */
> @@ -908,7 +939,7 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
>  	tpm_buf_append_u16(&buf, TPM_ALG_SHA256);
>  
>  	rc = tpm_transmit_cmd(chip, &buf, 0, "start auth session");
> -	tpm2_flush_context(chip, nullkey);
> +	tpm2_flush_context(chip, null_key);
>  
>  	if (rc == TPM2_RC_SUCCESS)
>  		rc = tpm2_parse_start_auth_session(auth, &buf);
> @@ -930,19 +961,25 @@ EXPORT_SYMBOL(tpm2_start_auth_session);
>   * @buf:	The response buffer from the chip
>   * @handle:	pointer to be filled in with the return handle of the primary
>   * @hierarchy:	The hierarchy the primary was created for
> + * @name:	pointer to be filled in with the primary key name
>   *
>   * @returns: 0 on success or a positive TPM or negative standard error
>   */
>  static int tpm2_parse_create_primary(struct tpm_chip *chip, struct tpm_buf *buf,
> -				     u32 *handle, u32 hierarchy)
> +				     u32 *handle, u32 hierarchy, u8 *name)
>  {
>  	struct tpm_header *head = (struct tpm_header *)buf->data;
>  	off_t offset_r = TPM_HEADER_SIZE, offset_t;
>  	u16 len = TPM_HEADER_SIZE;
>  	u32 total_len = be32_to_cpu(head->length);
> -	u32 val, param_len;
> +	u32 val, param_len, keyhandle;
> +
> +	keyhandle = tpm_buf_read_u32(buf, &offset_r);
> +	if (handle)
> +		*handle = keyhandle;
> +	else
> +		tpm2_flush_context(chip, keyhandle);
>  
> -	*handle = tpm_buf_read_u32(buf, &offset_r);
>  	param_len = tpm_buf_read_u32(buf, &offset_r);
>  	/*
>  	 * param_len doesn't include the header, but all the other
> @@ -955,9 +992,14 @@ 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);
> @@ -1086,6 +1128,7 @@ static int tpm2_parse_create_primary(struct tpm_chip *chip, struct tpm_buf *buf,
>   * @chip:      the TPM chip to create under
>   * @hierarchy: The hierarchy handle to create under
>   * @handle:    The returned volatile handle on success
> + * @name:      The name of the returned key
>   *
>   * For platforms that might not have a persistent primary, this can be
>   * used to create one quickly on the fly (it uses Elliptic Curve not
> @@ -1097,7 +1140,7 @@ static int tpm2_parse_create_primary(struct tpm_chip *chip, struct tpm_buf *buf,
>   * @returns: 0 on success or positive TPM or negative error.
>   */
>  static int tpm2_create_primary(struct tpm_chip *chip, u32 hierarchy,
> -			       u32 *handle)
> +			       u32 *handle, u8 *name)
>  {
>  	int rc;
>  	struct tpm_buf buf;
> @@ -1187,7 +1230,8 @@ static int tpm2_create_primary(struct tpm_chip *chip, u32 hierarchy,
>  			      "attempting to create NULL primary");
>  
>  	if (rc == TPM2_RC_SUCCESS)
> -		rc = tpm2_parse_create_primary(chip, &buf, handle, hierarchy);
> +		rc = tpm2_parse_create_primary(chip, &buf, handle, hierarchy,
> +					       name);
>  
>  	tpm_buf_destroy(&buf);
>  
> @@ -1199,7 +1243,8 @@ static int tpm2_create_null_primary(struct tpm_chip *chip)
>  	u32 null_key;
>  	int rc;
>  
> -	rc = tpm2_create_primary(chip, TPM2_RH_NULL, &null_key);
> +	rc = tpm2_create_primary(chip, TPM2_RH_NULL, &null_key,
> +				 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 dd4d6a6158c4..c17e4efbb2e5 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -248,6 +248,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,
> @@ -346,6 +347,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)
> @@ -447,7 +449,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;

Bit 6 is set for TPM2_RC_INTEGRITY and thus you needed to
mask it out, as it is set given the handle, right? Just
sanity checking that I understand the change.

>  }
>  
>  #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)

BR, Jarkko