Re: [RFC PATCH v2 10/13] tpm: authenticate tpm2_pcr_read()

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Sun, 2025-03-23 at 15:09 +0100, Nicolai Stange wrote:
> PCR reads aren't currently authenticated even with
> CONFIG_TCG_TPM2_HMAC=y yet.

The reason being TPM2_PCR_Read can only support an audit session, so it
has even more overhead than the usual HMAC session for something you
don't care about and because no-one relies on plain reads anyway,
relying entities use quotes.

> It is probably desirable though, as e.g. IMA does some PCR reads to
> form the cumulative boot digest subsequently extended into PCR 10 (an
> operation which *is* authenticated).

Could you elaborate on what security properties this adds?  I can't see
any form of attack that could be done by altering the boot aggregate:
either the relying party cares, in which case it will quote the boot
log and arrive at its own value, or it doesn't, in which case the value
in the log is superfluous.

> +		/*
> +		 * Exclusivity is not needed, but set in the
> response.
> +		 * Set it here too, so that the HMAC verification
> +		 * won't fail.
> +		 */
> +		tpm_buf_append_hmac_session(chip, &buf,
> TPM2_SA_AUDIT
> +					    |
> TPM2_SA_AUDIT_EXCLUSIVE,
> +					    NULL, 0);

Exclusivity here requires no other command be unaudited between the
session starting and now.  That means that with the lazy flush scheme
you have a reasonable chance of this being violated and triggering an
error on the command.

Additionally, the response will only have the exclusive flag set if the
above condition (no other unaudited command since session start) is
true, which it might not be.  The problem you're having is that
tpm2_auth_check_hmac_response() uses the command session flags to
calculate the rpHash, which is a useful short cut because for non-audit
sessions they're always the same.  If you want to use audit sessions,
you have to teach it to dig the response session flags out of the
header and use them instead.

Regards,

James