[v2] tpm: Opt-in in disable PCR integrity protection

Commit Message

Jarkko Sakkinen Nov. 7, 2024, 9:51 a.m. UTC

The initial HMAC session feature added TPM bus encryption and/or integrity
protection to various in-kernel TPM operations. This can cause performance
bottlenecks with IMA, as it heavily utilizes PCR extend operations.

In order to mitigate this performance issue, introduce a kernel
command-line parameter to the TPM driver for disabling the integrity
protection for PCR extension.

Cc: James Bottomley <[e-mail]>
Link: https://lore.kernel.org/linux-integrity/[e-mail]/
Fixes: 6519fea6fd37 ("tpm: add hmac checks to tpm2_pcr_extend()")
Co-developed-by: Roberto Sassu <[e-mail]>
Signed-off-by: Roberto Sassu <[e-mail]>
Co-developed-by: Mimi Zohar <[e-mail]>
Signed-off-by: Mimi Zohar <[e-mail]>
Signed-off-by: Jarkko Sakkinen <[e-mail]>
---
v2:
- Move tpm_buf_append_handle() to the correct file, remove spurious
  parameter (name), include error on TPM2B and add documentation.
  Keep the declaration in linux/tpm.h despite not exported as it
  is easiest to maintain tpm_buf_* in a single header.
- Rename kernel command-line option as "disable_pcr_integrity_protection",
  as Mimi pointed out it does not carry SA_ENCRYPT flag.
v1:
- Derived from the earlier RFC patch with a different parameter scope,
  cleaner commit message and some other tweaks. I decided to create
  something because I did not noticed any progress. Note only compile
  tested as I wanted to get something quickly out.
---
 .../admin-guide/kernel-parameters.txt         | 10 ++++
 drivers/char/tpm/tpm-buf.c                    | 20 ++++++++
 drivers/char/tpm/tpm2-cmd.c                   | 30 ++++++++---
 drivers/char/tpm/tpm2-sessions.c              | 51 ++++++++++---------
 include/linux/tpm.h                           |  3 ++
 5 files changed, 83 insertions(+), 31 deletions(-)

Comments

James Bottomley Nov. 7, 2024, 1:20 p.m. UTC | #1

On Thu, 2024-11-07 at 11:51 +0200, Jarkko Sakkinen wrote:
[...]
> +void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf,
> +                        u8 attributes, u8 *passphrase, int
> passphrase_len)
> +{
> +       /* offset tells us where the sessions area begins */
> +       int offset = buf->handles * 4 + TPM_HEADER_SIZE;
> +       u32 len = 9 + passphrase_len;
> +
> +       if (tpm_buf_length(buf) != offset) {
> +               /* not the first session so update the existing
> length */
> +               len += get_unaligned_be32(&buf->data[offset]);
> +               put_unaligned_be32(len, &buf->data[offset]);
> +       } else {
> +               tpm_buf_append_u32(buf, len);
> +       }
> +       /* auth handle */
> +       tpm_buf_append_u32(buf, TPM2_RS_PW);
> +       /* nonce */
> +       tpm_buf_append_u16(buf, 0);
> +       /* attributes */
> +       tpm_buf_append_u8(buf, 0);
> +       /* passphrase */
> +       tpm_buf_append_u16(buf, passphrase_len);
> +       tpm_buf_append(buf, passphrase, passphrase_len);
> +}
> +

The rest of the code looks fine, but if you're going to extract this as
a separate function instead of doing the open coded struct
tpm2_null_auth that was there originally, you should probably extract
and use the tpm2_buf_append_auth() function in trusted_tpm2.c

James

Mimi Zohar Nov. 7, 2024, 1:44 p.m. UTC | #2

On Thu, 2024-11-07 at 11:51 +0200, Jarkko Sakkinen wrote:
> The initial HMAC session feature added TPM bus encryption and/or integrity
> protection to various in-kernel TPM operations. This can cause performance
> bottlenecks with IMA, as it heavily utilizes PCR extend operations.
> 
> In order to mitigate this performance issue, introduce a kernel
> command-line parameter to the TPM driver for disabling the integrity
> protection for PCR extension.
> 
> Cc: James Bottomley <[e-mail]>
> Link: https://lore.kernel.org/linux-integrity/[e-mail]/
> Fixes: 6519fea6fd37 ("tpm: add hmac checks to tpm2_pcr_extend()")
> Co-developed-by: Roberto Sassu <[e-mail]>
> Signed-off-by: Roberto Sassu <[e-mail]>
> Co-developed-by: Mimi Zohar <[e-mail]>
> Signed-off-by: Mimi Zohar <[e-mail]>
> Signed-off-by: Jarkko Sakkinen <[e-mail]>
> ---
> v2:
> - Move tpm_buf_append_handle() to the correct file, remove spurious
>   parameter (name), include error on TPM2B and add documentation.
>   Keep the declaration in linux/tpm.h despite not exported as it
>   is easiest to maintain tpm_buf_* in a single header.
> - Rename kernel command-line option as "disable_pcr_integrity_protection",
>   as Mimi pointed out it does not carry SA_ENCRYPT flag.
> v1:
> - Derived from the earlier RFC patch with a different parameter scope,
>   cleaner commit message and some other tweaks. I decided to create
>   something because I did not noticed any progress. Note only compile
>   tested as I wanted to get something quickly out.
> ---
>  .../admin-guide/kernel-parameters.txt         | 10 ++++
>  drivers/char/tpm/tpm-buf.c                    | 20 ++++++++
>  drivers/char/tpm/tpm2-cmd.c                   | 30 ++++++++---
>  drivers/char/tpm/tpm2-sessions.c              | 51 ++++++++++---------
>  include/linux/tpm.h                           |  3 ++
>  5 files changed, 83 insertions(+), 31 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 1518343bbe22..9fc406b20a74 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -6727,6 +6727,16 @@
>  	torture.verbose_sleep_duration= [KNL]
>  			Duration of each verbose-printk() sleep in jiffies.
>  
> +	tpm.disable_pcr_integrity_protection= [HW,TPM]
> +			Do not protect PCR registers from unintended physical
> +			access, or interposers in the bus by the means of
> +			having an encrypted and integrity protected session

"encrypted" isn't needed here.

> +			wrapped around TPM2_PCR_Extend command. Consider this
> +			in a situation where TPM is heavily utilized by
> +			IMA, thus protection causing a major performance hit,
> +			and the space where machines are deployed is by other
> +			means guarded.
> +
>  	tpm_suspend_pcr=[HW,TPM]
>  			Format: integer pcr id
>  			Specify that at suspend time, the tpm driver
> diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
> index cad0048bcc3c..e49a19fea3bd 100644
> --- a/drivers/char/tpm/tpm-buf.c
> +++ b/drivers/char/tpm/tpm-buf.c
> @@ -146,6 +146,26 @@ void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
>  }
>  EXPORT_SYMBOL_GPL(tpm_buf_append_u32);
>  
> +/**
> + * tpm_buf_append_handle() - Add a handle
> + * @chip:	&tpm_chip instance
> + * @buf:	&tpm_buf instance
> + * @handle:	a TPM object handle
> + *
> + * Add a handle to the buffer, and increase the count tracking the number of
> + * handles in the command buffer. Works only for command buffers.
> + */
> +void tpm_buf_append_handle(struct tpm_chip *chip, struct tpm_buf *buf, u32 handle)
> +{
> +	if (buf->flags & TPM_BUF_TPM2B) {
> +		dev_err(&chip->dev, "Invalid buffer type (TPM2B)\n");
> +		return;
> +	}
> +
> +	tpm_buf_append_u32(buf, handle);
> +	buf->handles++;
> +}
> +
>  /**
>   * tpm_buf_read() - Read from a TPM buffer
>   * @buf:	&tpm_buf instance
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 1e856259219e..cc443bcf15e8 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -14,6 +14,10 @@
>  #include "tpm.h"
>  #include <crypto/hash_info.h>
>  
> +static bool disable_pcr_integrity_protection;
> +module_param(disable_pcr_integrity_protection, bool, 0444);
> +MODULE_PARM_DESC(disable_pcr_integrity_protection, "Disable TPM2_PCR_Extend encryption");

I like the name 'disable_pcr_integrity_protection.  However, the name and
description doesn't match.  Replace 'encryption' with 'integrity protection'.

> +
>  static struct tpm2_hash tpm2_hash_map[] = {
>  	{HASH_ALGO_SHA1, TPM_ALG_SHA1},
>  	{HASH_ALGO_SHA256, TPM_ALG_SHA256},
> @@ -232,18 +236,26 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
>  	int rc;
>  	int i;
>  
> -	rc = tpm2_start_auth_session(chip);
> -	if (rc)
> -		return rc;
> +	if (!disable_pcr_integrity_protection) {
> +		rc = tpm2_start_auth_session(chip);
> +		if (rc)
> +			return rc;
> +	}
>  
>  	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
>  	if (rc) {
> -		tpm2_end_auth_session(chip);
> +		if (!disable_pcr_integrity_protection)
> +			tpm2_end_auth_session(chip);
>  		return rc;
>  	}
>  
> -	tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
> -	tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> +	if (!disable_pcr_integrity_protection) {
> +		tpm_buf_append_name(chip, &buf, pcr_idx);

tpm_buf_append_name() parameters didn't change.  Don't remove the 'name' field
here.


> +		tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> +	} else {
> +		tpm_buf_append_handle(chip, &buf, pcr_idx);

Or here.

> +		tpm_buf_append_auth(chip, &buf, 0, NULL, 0);
> +	}
>  
>  	tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
>  
> 

Mimi

kernel test robot Nov. 7, 2024, 1:46 p.m. UTC | #3

Hi Jarkko,

kernel test robot noticed the following build errors:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.12-rc6 next-20241106]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jarkko-Sakkinen/tpm-Opt-in-in-disable-PCR-integrity-protection/20241107-175515
base:   char-misc/char-misc-testing
patch link:    https://lore.kernel.org/r/20241107095138.78209-1-jarkko%40kernel.org
patch subject: [PATCH v2] tpm: Opt-in in disable PCR integrity protection
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20241107/[e-mail]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241107/[e-mail]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[e-mail]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[e-mail]/

All errors (new ones prefixed by >>):

   drivers/char/tpm/tpm2-cmd.c: In function 'tpm2_pcr_extend':
>> drivers/char/tpm/tpm2-cmd.c:253:17: error: too few arguments to function 'tpm_buf_append_name'
     253 |                 tpm_buf_append_name(chip, &buf, pcr_idx);
         |                 ^~~~~~~~~~~~~~~~~~~
   In file included from drivers/char/tpm/tpm.h:27,
                    from drivers/char/tpm/tpm2-cmd.c:14:
   include/linux/tpm.h:504:6: note: declared here
     504 | void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
         |      ^~~~~~~~~~~~~~~~~~~


vim +/tpm_buf_append_name +253 drivers/char/tpm/tpm2-cmd.c

   222	
   223	/**
   224	 * tpm2_pcr_extend() - extend a PCR value
   225	 *
   226	 * @chip:	TPM chip to use.
   227	 * @pcr_idx:	index of the PCR.
   228	 * @digests:	list of pcr banks and corresponding digest values to extend.
   229	 *
   230	 * Return: Same as with tpm_transmit_cmd.
   231	 */
   232	int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
   233			    struct tpm_digest *digests)
   234	{
   235		struct tpm_buf buf;
   236		int rc;
   237		int i;
   238	
   239		if (!disable_pcr_integrity_protection) {
   240			rc = tpm2_start_auth_session(chip);
   241			if (rc)
   242				return rc;
   243		}
   244	
   245		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
   246		if (rc) {
   247			if (!disable_pcr_integrity_protection)
   248				tpm2_end_auth_session(chip);
   249			return rc;
   250		}
   251	
   252		if (!disable_pcr_integrity_protection) {
 > 253			tpm_buf_append_name(chip, &buf, pcr_idx);
   254			tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
   255		} else {
   256			tpm_buf_append_handle(chip, &buf, pcr_idx);
   257			tpm_buf_append_auth(chip, &buf, 0, NULL, 0);
   258		}
   259	
   260		tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
   261	
   262		for (i = 0; i < chip->nr_allocated_banks; i++) {
   263			tpm_buf_append_u16(&buf, digests[i].alg_id);
   264			tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest,
   265				       chip->allocated_banks[i].digest_size);
   266		}
   267	
   268		if (!disable_pcr_integrity_protection)
   269			tpm_buf_fill_hmac_session(chip, &buf);
   270		rc = tpm_transmit_cmd(chip, &buf, 0, "attempting extend a PCR value");
   271		if (!disable_pcr_integrity_protection)
   272			rc = tpm_buf_check_hmac_response(chip, &buf, rc);
   273	
   274		tpm_buf_destroy(&buf);
   275	
   276		return rc;
   277	}
   278

Jarkko Sakkinen Nov. 7, 2024, 1:47 p.m. UTC | #4

On Thu Nov 7, 2024 at 3:44 PM EET, Mimi Zohar wrote:
> > +	tpm.disable_pcr_integrity_protection= [HW,TPM]
> > +			Do not protect PCR registers from unintended physical
> > +			access, or interposers in the bus by the means of
> > +			having an encrypted and integrity protected session
>
> "encrypted" isn't needed here.

fixing

>
> > +			wrapped around TPM2_PCR_Extend command. Consider this
> > +			in a situation where TPM is heavily utilized by
> > +			IMA, thus protection causing a major performance hit,
> > +			and the space where machines are deployed is by other
> > +			means guarded.
> > +
> >  	tpm_suspend_pcr=[HW,TPM]
> >  			Format: integer pcr id
> >  			Specify that at suspend time, the tpm driver
> > diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
> > index cad0048bcc3c..e49a19fea3bd 100644
> > --- a/drivers/char/tpm/tpm-buf.c
> > +++ b/drivers/char/tpm/tpm-buf.c
> > @@ -146,6 +146,26 @@ void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
> >  }
> >  EXPORT_SYMBOL_GPL(tpm_buf_append_u32);
> >  
> > +/**
> > + * tpm_buf_append_handle() - Add a handle
> > + * @chip:	&tpm_chip instance
> > + * @buf:	&tpm_buf instance
> > + * @handle:	a TPM object handle
> > + *
> > + * Add a handle to the buffer, and increase the count tracking the number of
> > + * handles in the command buffer. Works only for command buffers.
> > + */
> > +void tpm_buf_append_handle(struct tpm_chip *chip, struct tpm_buf *buf, u32 handle)
> > +{
> > +	if (buf->flags & TPM_BUF_TPM2B) {
> > +		dev_err(&chip->dev, "Invalid buffer type (TPM2B)\n");
> > +		return;
> > +	}
> > +
> > +	tpm_buf_append_u32(buf, handle);
> > +	buf->handles++;
> > +}
> > +
> >  /**
> >   * tpm_buf_read() - Read from a TPM buffer
> >   * @buf:	&tpm_buf instance
> > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > index 1e856259219e..cc443bcf15e8 100644
> > --- a/drivers/char/tpm/tpm2-cmd.c
> > +++ b/drivers/char/tpm/tpm2-cmd.c
> > @@ -14,6 +14,10 @@
> >  #include "tpm.h"
> >  #include <crypto/hash_info.h>
> >  
> > +static bool disable_pcr_integrity_protection;
> > +module_param(disable_pcr_integrity_protection, bool, 0444);
> > +MODULE_PARM_DESC(disable_pcr_integrity_protection, "Disable TPM2_PCR_Extend encryption");
>
> I like the name 'disable_pcr_integrity_protection.  However, the name and
> description doesn't match.  Replace 'encryption' with 'integrity protection'.

Weird, I changed that. I don't know  how it ended up being like that.

>
> > +
> >  static struct tpm2_hash tpm2_hash_map[] = {
> >  	{HASH_ALGO_SHA1, TPM_ALG_SHA1},
> >  	{HASH_ALGO_SHA256, TPM_ALG_SHA256},
> > @@ -232,18 +236,26 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> >  	int rc;
> >  	int i;
> >  
> > -	rc = tpm2_start_auth_session(chip);
> > -	if (rc)
> > -		return rc;
> > +	if (!disable_pcr_integrity_protection) {
> > +		rc = tpm2_start_auth_session(chip);
> > +		if (rc)
> > +			return rc;
> > +	}
> >  
> >  	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
> >  	if (rc) {
> > -		tpm2_end_auth_session(chip);
> > +		if (!disable_pcr_integrity_protection)
> > +			tpm2_end_auth_session(chip);
> >  		return rc;
> >  	}
> >  
> > -	tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
> > -	tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> > +	if (!disable_pcr_integrity_protection) {
> > +		tpm_buf_append_name(chip, &buf, pcr_idx);
>
> tpm_buf_append_name() parameters didn't change.  Don't remove the 'name' field
> here.

Hmm... weird I'll check this. Maybe I had something left to staging...

>
>
> > +		tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> > +	} else {
> > +		tpm_buf_append_handle(chip, &buf, pcr_idx);
>

> Or here.

Here I think it is appropriate

>
> > +		tpm_buf_append_auth(chip, &buf, 0, NULL, 0);
> > +	}
> >  
> >  	tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
> >  
> > 
>
> Mimi


BR, Jarkko

Jarkko Sakkinen Nov. 7, 2024, 1:49 p.m. UTC | #5

On Thu Nov 7, 2024 at 3:20 PM EET, James Bottomley wrote:
> On Thu, 2024-11-07 at 11:51 +0200, Jarkko Sakkinen wrote:
> [...]
> > +void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf,
> > +                        u8 attributes, u8 *passphrase, int
> > passphrase_len)
> > +{
> > +       /* offset tells us where the sessions area begins */
> > +       int offset = buf->handles * 4 + TPM_HEADER_SIZE;
> > +       u32 len = 9 + passphrase_len;
> > +
> > +       if (tpm_buf_length(buf) != offset) {
> > +               /* not the first session so update the existing
> > length */
> > +               len += get_unaligned_be32(&buf->data[offset]);
> > +               put_unaligned_be32(len, &buf->data[offset]);
> > +       } else {
> > +               tpm_buf_append_u32(buf, len);
> > +       }
> > +       /* auth handle */
> > +       tpm_buf_append_u32(buf, TPM2_RS_PW);
> > +       /* nonce */
> > +       tpm_buf_append_u16(buf, 0);
> > +       /* attributes */
> > +       tpm_buf_append_u8(buf, 0);
> > +       /* passphrase */
> > +       tpm_buf_append_u16(buf, passphrase_len);
> > +       tpm_buf_append(buf, passphrase, passphrase_len);
> > +}
> > +
>
> The rest of the code looks fine, but if you're going to extract this as
> a separate function instead of doing the open coded struct
> tpm2_null_auth that was there originally, you should probably extract
> and use the tpm2_buf_append_auth() function in trusted_tpm2.c

So this was straight up from Mimi's original patch :-)

Hmm... was there duplicate use for this in the patch? I'll check this.

>
> James

BR, Jarkko

James Bottomley Nov. 7, 2024, 1:52 p.m. UTC | #6

On Thu, 2024-11-07 at 15:49 +0200, Jarkko Sakkinen wrote:
> On Thu Nov 7, 2024 at 3:20 PM EET, James Bottomley wrote:
> > On Thu, 2024-11-07 at 11:51 +0200, Jarkko Sakkinen wrote:
> > [...]
> > > +void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf
> > > *buf,
> > > +                        u8 attributes, u8 *passphrase, int
> > > passphrase_len)
> > > +{
> > > +       /* offset tells us where the sessions area begins */
> > > +       int offset = buf->handles * 4 + TPM_HEADER_SIZE;
> > > +       u32 len = 9 + passphrase_len;
> > > +
> > > +       if (tpm_buf_length(buf) != offset) {
> > > +               /* not the first session so update the existing
> > > length */
> > > +               len += get_unaligned_be32(&buf->data[offset]);
> > > +               put_unaligned_be32(len, &buf->data[offset]);
> > > +       } else {
> > > +               tpm_buf_append_u32(buf, len);
> > > +       }
> > > +       /* auth handle */
> > > +       tpm_buf_append_u32(buf, TPM2_RS_PW);
> > > +       /* nonce */
> > > +       tpm_buf_append_u16(buf, 0);
> > > +       /* attributes */
> > > +       tpm_buf_append_u8(buf, 0);
> > > +       /* passphrase */
> > > +       tpm_buf_append_u16(buf, passphrase_len);
> > > +       tpm_buf_append(buf, passphrase, passphrase_len);
> > > +}
> > > +
> > 
> > The rest of the code looks fine, but if you're going to extract
> > this as a separate function instead of doing the open coded struct
> > tpm2_null_auth that was there originally, you should probably
> > extract and use the tpm2_buf_append_auth() function in
> > trusted_tpm2.c
> 
> So this was straight up from Mimi's original patch :-)

Yes, I had the same comment prepped for that too.

> Hmm... was there duplicate use for this in the patch? I'll check
> this.

The original open coded the empty auth append with struct
tpm2_null_auth since it's the only user.  However, since we do have
another user in trusted keys, it might make sense to consolidate.

James

Mimi Zohar Nov. 7, 2024, 2 p.m. UTC | #7

On Thu, 2024-11-07 at 15:47 +0200, Jarkko Sakkinen wrote:
> On Thu Nov 7, 2024 at 3:44 PM EET, Mimi Zohar wrote:
> > > 
> > > @@ -232,18 +236,26 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> > >  	int rc;
> > >  	int i;
> > >  
> > > -	rc = tpm2_start_auth_session(chip);
> > > -	if (rc)
> > > -		return rc;
> > > +	if (!disable_pcr_integrity_protection) {
> > > +		rc = tpm2_start_auth_session(chip);
> > > +		if (rc)
> > > +			return rc;
> > > +	}
> > >  
> > >  	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
> > >  	if (rc) {
> > > -		tpm2_end_auth_session(chip);
> > > +		if (!disable_pcr_integrity_protection)
> > > +			tpm2_end_auth_session(chip);
> > >  		return rc;
> > >  	}
> > >  
> > > -	tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
> > > -	tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> > > +	if (!disable_pcr_integrity_protection) {
> > > +		tpm_buf_append_name(chip, &buf, pcr_idx);
> > 
> > tpm_buf_append_name() parameters didn't change.  Don't remove the 'name' field
> > here.
> 
> Hmm... weird I'll check this. Maybe I had something left to staging...
> 
> > 
> > 
> > > +		tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> > > +	} else {
> > > +		tpm_buf_append_handle(chip, &buf, pcr_idx);
> > 
> 
> > Or here.
> 
> Here I think it is appropriate

Agreed

> 
> > 
> > > +		tpm_buf_append_auth(chip, &buf, 0, NULL, 0);
> > > +	}
> > >  
> > >  	tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
> > >

Jarkko Sakkinen Nov. 7, 2024, 4:45 p.m. UTC | #8

On Thu Nov 7, 2024 at 4:00 PM EET, Mimi Zohar wrote:
> On Thu, 2024-11-07 at 15:47 +0200, Jarkko Sakkinen wrote:
> > On Thu Nov 7, 2024 at 3:44 PM EET, Mimi Zohar wrote:
> > > > 
> > > > @@ -232,18 +236,26 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> > > >  	int rc;
> > > >  	int i;
> > > >  
> > > > -	rc = tpm2_start_auth_session(chip);
> > > > -	if (rc)
> > > > -		return rc;
> > > > +	if (!disable_pcr_integrity_protection) {
> > > > +		rc = tpm2_start_auth_session(chip);
> > > > +		if (rc)
> > > > +			return rc;
> > > > +	}
> > > >  
> > > >  	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
> > > >  	if (rc) {
> > > > -		tpm2_end_auth_session(chip);
> > > > +		if (!disable_pcr_integrity_protection)
> > > > +			tpm2_end_auth_session(chip);
> > > >  		return rc;
> > > >  	}
> > > >  
> > > > -	tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
> > > > -	tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> > > > +	if (!disable_pcr_integrity_protection) {
> > > > +		tpm_buf_append_name(chip, &buf, pcr_idx);
> > > 
> > > tpm_buf_append_name() parameters didn't change.  Don't remove the 'name' field
> > > here.
> > 
> > Hmm... weird I'll check this. Maybe I had something left to staging...

Yes! This was correct in my clone but not in the patch.

Clearly a sign that I wait until next week before sending a new version
:-)


> > 
> > > 
> > > 
> > > > +		tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
> > > > +	} else {
> > > > +		tpm_buf_append_handle(chip, &buf, pcr_idx);
> > > 
> > 
> > > Or here.
> > 
> > Here I think it is appropriate
>
> Agreed

Great

BR, Jarkko

kernel test robot Nov. 7, 2024, 8:43 p.m. UTC | #9

Hi Jarkko,

kernel test robot noticed the following build errors:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.12-rc6 next-20241107]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jarkko-Sakkinen/tpm-Opt-in-in-disable-PCR-integrity-protection/20241107-175515
base:   char-misc/char-misc-testing
patch link:    https://lore.kernel.org/r/20241107095138.78209-1-jarkko%40kernel.org
patch subject: [PATCH v2] tpm: Opt-in in disable PCR integrity protection
config: arm-randconfig-003-20241108 (https://download.01.org/0day-ci/archive/20241108/[e-mail]/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241108/[e-mail]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[e-mail]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[e-mail]/

All errors (new ones prefixed by >>):

   In file included from drivers/char/tpm/tpm2-cmd.c:14:
   In file included from drivers/char/tpm/tpm.h:28:
   include/linux/tpm_eventlog.h:167:6: warning: variable 'mapping_size' set but not used [-Wunused-but-set-variable]
           int mapping_size;
               ^
>> drivers/char/tpm/tpm2-cmd.c:253:42: error: too few arguments to function call, expected 4, have 3
                   tpm_buf_append_name(chip, &buf, pcr_idx);
                   ~~~~~~~~~~~~~~~~~~~                    ^
   include/linux/tpm.h:504:6: note: 'tpm_buf_append_name' declared here
   void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
        ^
   1 warning and 1 error generated.


vim +253 drivers/char/tpm/tpm2-cmd.c

   222	
   223	/**
   224	 * tpm2_pcr_extend() - extend a PCR value
   225	 *
   226	 * @chip:	TPM chip to use.
   227	 * @pcr_idx:	index of the PCR.
   228	 * @digests:	list of pcr banks and corresponding digest values to extend.
   229	 *
   230	 * Return: Same as with tpm_transmit_cmd.
   231	 */
   232	int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
   233			    struct tpm_digest *digests)
   234	{
   235		struct tpm_buf buf;
   236		int rc;
   237		int i;
   238	
   239		if (!disable_pcr_integrity_protection) {
   240			rc = tpm2_start_auth_session(chip);
   241			if (rc)
   242				return rc;
   243		}
   244	
   245		rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
   246		if (rc) {
   247			if (!disable_pcr_integrity_protection)
   248				tpm2_end_auth_session(chip);
   249			return rc;
   250		}
   251	
   252		if (!disable_pcr_integrity_protection) {
 > 253			tpm_buf_append_name(chip, &buf, pcr_idx);
   254			tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
   255		} else {
   256			tpm_buf_append_handle(chip, &buf, pcr_idx);
   257			tpm_buf_append_auth(chip, &buf, 0, NULL, 0);
   258		}
   259	
   260		tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
   261	
   262		for (i = 0; i < chip->nr_allocated_banks; i++) {
   263			tpm_buf_append_u16(&buf, digests[i].alg_id);
   264			tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest,
   265				       chip->allocated_banks[i].digest_size);
   266		}
   267	
   268		if (!disable_pcr_integrity_protection)
   269			tpm_buf_fill_hmac_session(chip, &buf);
   270		rc = tpm_transmit_cmd(chip, &buf, 0, "attempting extend a PCR value");
   271		if (!disable_pcr_integrity_protection)
   272			rc = tpm_buf_check_hmac_response(chip, &buf, rc);
   273	
   274		tpm_buf_destroy(&buf);
   275	
   276		return rc;
   277	}
   278

Mimi Zohar Nov. 11, 2024, 7:53 p.m. UTC | #10

On Thu, 2024-11-07 at 08:52 -0500, James Bottomley wrote:
> On Thu, 2024-11-07 at 15:49 +0200, Jarkko Sakkinen wrote:
> > On Thu Nov 7, 2024 at 3:20 PM EET, James Bottomley wrote:
> > > On Thu, 2024-11-07 at 11:51 +0200, Jarkko Sakkinen wrote:
> > > [...]
> > > > +void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf
> > > > *buf,
> > > > +                        u8 attributes, u8 *passphrase, int
> > > > passphrase_len)
> > > > +{
> > > > +       /* offset tells us where the sessions area begins */
> > > > +       int offset = buf->handles * 4 + TPM_HEADER_SIZE;
> > > > +       u32 len = 9 + passphrase_len;
> > > > +
> > > > +       if (tpm_buf_length(buf) != offset) {
> > > > +               /* not the first session so update the existing
> > > > length */
> > > > +               len += get_unaligned_be32(&buf->data[offset]);
> > > > +               put_unaligned_be32(len, &buf->data[offset]);
> > > > +       } else {
> > > > +               tpm_buf_append_u32(buf, len);
> > > > +       }
> > > > +       /* auth handle */
> > > > +       tpm_buf_append_u32(buf, TPM2_RS_PW);
> > > > +       /* nonce */
> > > > +       tpm_buf_append_u16(buf, 0);
> > > > +       /* attributes */
> > > > +       tpm_buf_append_u8(buf, 0);
> > > > +       /* passphrase */
> > > > +       tpm_buf_append_u16(buf, passphrase_len);
> > > > +       tpm_buf_append(buf, passphrase, passphrase_len);
> > > > +}
> > > > +
> > > 
> > > The rest of the code looks fine, but if you're going to extract
> > > this as a separate function instead of doing the open coded struct
> > > tpm2_null_auth that was there originally, you should probably
> > > extract and use the tpm2_buf_append_auth() function in
> > > trusted_tpm2.c
> > 
> > So this was straight up from Mimi's original patch :-)
> 
> Yes, I had the same comment prepped for that too.

But it wasn't sent ...
> 
> > Hmm... was there duplicate use for this in the patch? I'll check
> > this.
> 
> The original open coded the empty auth append with struct
> tpm2_null_auth since it's the only user.  However, since we do have
> another user in trusted keys, it might make sense to consolidate.

Instead of delaying the current patch from being upstreamed, perhaps this change
can be deferred?

thanks,

Mimi

James Bottomley Nov. 11, 2024, 8:12 p.m. UTC | #11

On Mon, 2024-11-11 at 14:53 -0500, Mimi Zohar wrote:
> On Thu, 2024-11-07 at 08:52 -0500, James Bottomley wrote:
> > On Thu, 2024-11-07 at 15:49 +0200, Jarkko Sakkinen wrote:
> > > On Thu Nov 7, 2024 at 3:20 PM EET, James Bottomley wrote:
> > > > On Thu, 2024-11-07 at 11:51 +0200, Jarkko Sakkinen wrote:
> > > > [...]
> > > > > +void tpm_buf_append_auth(struct tpm_chip *chip, struct
> > > > > tpm_buf
> > > > > *buf,
> > > > > +                        u8 attributes, u8 *passphrase, int
> > > > > passphrase_len)
> > > > > +{
> > > > > +       /* offset tells us where the sessions area begins */
> > > > > +       int offset = buf->handles * 4 + TPM_HEADER_SIZE;
> > > > > +       u32 len = 9 + passphrase_len;
> > > > > +
> > > > > +       if (tpm_buf_length(buf) != offset) {
> > > > > +               /* not the first session so update the
> > > > > existing
> > > > > length */
> > > > > +               len += get_unaligned_be32(&buf-
> > > > > >data[offset]);
> > > > > +               put_unaligned_be32(len, &buf->data[offset]);
> > > > > +       } else {
> > > > > +               tpm_buf_append_u32(buf, len);
> > > > > +       }
> > > > > +       /* auth handle */
> > > > > +       tpm_buf_append_u32(buf, TPM2_RS_PW);
> > > > > +       /* nonce */
> > > > > +       tpm_buf_append_u16(buf, 0);
> > > > > +       /* attributes */
> > > > > +       tpm_buf_append_u8(buf, 0);
> > > > > +       /* passphrase */
> > > > > +       tpm_buf_append_u16(buf, passphrase_len);
> > > > > +       tpm_buf_append(buf, passphrase, passphrase_len);
> > > > > +}
> > > > > +
> > > > 
> > > > The rest of the code looks fine, but if you're going to extract
> > > > this as a separate function instead of doing the open coded
> > > > struct
> > > > tpm2_null_auth that was there originally, you should probably
> > > > extract and use the tpm2_buf_append_auth() function in
> > > > trusted_tpm2.c
> > > 
> > > So this was straight up from Mimi's original patch :-)
> > 
> > Yes, I had the same comment prepped for that too.
> 
> But it wasn't sent ...

no.

> > 
> > > Hmm... was there duplicate use for this in the patch? I'll check
> > > this.
> > 
> > The original open coded the empty auth append with struct
> > tpm2_null_auth since it's the only user.  However, since we do have
> > another user in trusted keys, it might make sense to consolidate.
> 
> Instead of delaying the current patch from being upstreamed, perhaps
> this change can be deferred?

I don't see why not; it was the introduction of the new function rather
than going back to the struct tpm2_null_auth_area of the original that
caught my attention.

James

Jarkko Sakkinen Nov. 12, 2024, 5:57 p.m. UTC | #12

On Mon Nov 11, 2024 at 9:53 PM EET, Mimi Zohar wrote:
> > The original open coded the empty auth append with struct
> > tpm2_null_auth since it's the only user.  However, since we do have
> > another user in trusted keys, it might make sense to consolidate.
>
> Instead of delaying the current patch from being upstreamed, perhaps this change
> can be deferred?

Yes.

BR, Jarkko

Mimi Zohar Nov. 12, 2024, 8 p.m. UTC | #13

On Tue, 2024-11-12 at 19:57 +0200, Jarkko Sakkinen wrote:
> On Mon Nov 11, 2024 at 9:53 PM EET, Mimi Zohar wrote:
> > > The original open coded the empty auth append with struct
> > > tpm2_null_auth since it's the only user.  However, since we do have
> > > another user in trusted keys, it might make sense to consolidate.
> > 
> > Instead of delaying the current patch from being upstreamed, perhaps this change
> > can be deferred?
> 
> Yes.

Thanks.  As soon as you repost the patch, I'll re-test.

Mimi

Patch

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 1518343bbe22..9fc406b20a74 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6727,6 +6727,16 @@ 
 	torture.verbose_sleep_duration= [KNL]
 			Duration of each verbose-printk() sleep in jiffies.
 
+	tpm.disable_pcr_integrity_protection= [HW,TPM]
+			Do not protect PCR registers from unintended physical
+			access, or interposers in the bus by the means of
+			having an encrypted and integrity protected session
+			wrapped around TPM2_PCR_Extend command. Consider this
+			in a situation where TPM is heavily utilized by
+			IMA, thus protection causing a major performance hit,
+			and the space where machines are deployed is by other
+			means guarded.
+
 	tpm_suspend_pcr=[HW,TPM]
 			Format: integer pcr id
 			Specify that at suspend time, the tpm driver
diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
index cad0048bcc3c..e49a19fea3bd 100644
--- a/drivers/char/tpm/tpm-buf.c
+++ b/drivers/char/tpm/tpm-buf.c
@@ -146,6 +146,26 @@  void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
 }
 EXPORT_SYMBOL_GPL(tpm_buf_append_u32);
 
+/**
+ * tpm_buf_append_handle() - Add a handle
+ * @chip:	&tpm_chip instance
+ * @buf:	&tpm_buf instance
+ * @handle:	a TPM object handle
+ *
+ * Add a handle to the buffer, and increase the count tracking the number of
+ * handles in the command buffer. Works only for command buffers.
+ */
+void tpm_buf_append_handle(struct tpm_chip *chip, struct tpm_buf *buf, u32 handle)
+{
+	if (buf->flags & TPM_BUF_TPM2B) {
+		dev_err(&chip->dev, "Invalid buffer type (TPM2B)\n");
+		return;
+	}
+
+	tpm_buf_append_u32(buf, handle);
+	buf->handles++;
+}
+
 /**
  * tpm_buf_read() - Read from a TPM buffer
  * @buf:	&tpm_buf instance
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 1e856259219e..cc443bcf15e8 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -14,6 +14,10 @@ 
 #include "tpm.h"
 #include <crypto/hash_info.h>
 
+static bool disable_pcr_integrity_protection;
+module_param(disable_pcr_integrity_protection, bool, 0444);
+MODULE_PARM_DESC(disable_pcr_integrity_protection, "Disable TPM2_PCR_Extend encryption");
+
 static struct tpm2_hash tpm2_hash_map[] = {
 	{HASH_ALGO_SHA1, TPM_ALG_SHA1},
 	{HASH_ALGO_SHA256, TPM_ALG_SHA256},
@@ -232,18 +236,26 @@  int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 	int rc;
 	int i;
 
-	rc = tpm2_start_auth_session(chip);
-	if (rc)
-		return rc;
+	if (!disable_pcr_integrity_protection) {
+		rc = tpm2_start_auth_session(chip);
+		if (rc)
+			return rc;
+	}
 
 	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
 	if (rc) {
-		tpm2_end_auth_session(chip);
+		if (!disable_pcr_integrity_protection)
+			tpm2_end_auth_session(chip);
 		return rc;
 	}
 
-	tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
-	tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
+	if (!disable_pcr_integrity_protection) {
+		tpm_buf_append_name(chip, &buf, pcr_idx);
+		tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
+	} else {
+		tpm_buf_append_handle(chip, &buf, pcr_idx);
+		tpm_buf_append_auth(chip, &buf, 0, NULL, 0);
+	}
 
 	tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
 
@@ -253,9 +265,11 @@  int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 			       chip->allocated_banks[i].digest_size);
 	}
 
-	tpm_buf_fill_hmac_session(chip, &buf);
+	if (!disable_pcr_integrity_protection)
+		tpm_buf_fill_hmac_session(chip, &buf);
 	rc = tpm_transmit_cmd(chip, &buf, 0, "attempting extend a PCR value");
-	rc = tpm_buf_check_hmac_response(chip, &buf, rc);
+	if (!disable_pcr_integrity_protection)
+		rc = tpm_buf_check_hmac_response(chip, &buf, rc);
 
 	tpm_buf_destroy(&buf);
 
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 42df980168b6..a7c1b162251b 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -237,9 +237,7 @@  void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
 #endif
 
 	if (!tpm2_chip_auth(chip)) {
-		tpm_buf_append_u32(buf, handle);
-		/* count the number of handles in the upper bits of flags */
-		buf->handles++;
+		tpm_buf_append_handle(chip, buf, handle);
 		return;
 	}
 
@@ -272,6 +270,31 @@  void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
 }
 EXPORT_SYMBOL_GPL(tpm_buf_append_name);
 
+void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf,
+			 u8 attributes, u8 *passphrase, int passphrase_len)
+{
+	/* offset tells us where the sessions area begins */
+	int offset = buf->handles * 4 + TPM_HEADER_SIZE;
+	u32 len = 9 + passphrase_len;
+
+	if (tpm_buf_length(buf) != offset) {
+		/* not the first session so update the existing length */
+		len += get_unaligned_be32(&buf->data[offset]);
+		put_unaligned_be32(len, &buf->data[offset]);
+	} else {
+		tpm_buf_append_u32(buf, len);
+	}
+	/* auth handle */
+	tpm_buf_append_u32(buf, TPM2_RS_PW);
+	/* nonce */
+	tpm_buf_append_u16(buf, 0);
+	/* attributes */
+	tpm_buf_append_u8(buf, 0);
+	/* passphrase */
+	tpm_buf_append_u16(buf, passphrase_len);
+	tpm_buf_append(buf, passphrase, passphrase_len);
+}
+
 /**
  * tpm_buf_append_hmac_session() - Append a TPM session element
  * @chip: the TPM chip structure
@@ -309,26 +332,8 @@  void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
 #endif
 
 	if (!tpm2_chip_auth(chip)) {
-		/* offset tells us where the sessions area begins */
-		int offset = buf->handles * 4 + TPM_HEADER_SIZE;
-		u32 len = 9 + passphrase_len;
-
-		if (tpm_buf_length(buf) != offset) {
-			/* not the first session so update the existing length */
-			len += get_unaligned_be32(&buf->data[offset]);
-			put_unaligned_be32(len, &buf->data[offset]);
-		} else {
-			tpm_buf_append_u32(buf, len);
-		}
-		/* auth handle */
-		tpm_buf_append_u32(buf, TPM2_RS_PW);
-		/* nonce */
-		tpm_buf_append_u16(buf, 0);
-		/* attributes */
-		tpm_buf_append_u8(buf, 0);
-		/* passphrase */
-		tpm_buf_append_u16(buf, passphrase_len);
-		tpm_buf_append(buf, passphrase, passphrase_len);
+		tpm_buf_append_auth(chip, buf, attributes, passphrase,
+				    passphrase_len);
 		return;
 	}
 
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 587b96b4418e..20a40ade8030 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -421,6 +421,7 @@  void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value);
 u8 tpm_buf_read_u8(struct tpm_buf *buf, off_t *offset);
 u16 tpm_buf_read_u16(struct tpm_buf *buf, off_t *offset);
 u32 tpm_buf_read_u32(struct tpm_buf *buf, off_t *offset);
+void tpm_buf_append_handle(struct tpm_chip *chip, struct tpm_buf *buf, u32 handle);
 
 /*
  * Check if TPM device is in the firmware upgrade mode.
@@ -505,6 +506,8 @@  void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
 void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
 				 u8 attributes, u8 *passphrase,
 				 int passphraselen);
+void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf,
+			 u8 attributes, u8 *passphrase, int passphraselen);
 static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip,
 						   struct tpm_buf *buf,
 						   u8 attributes,