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

[Nicolai Stange]

James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> writes:

> 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.

Thanks a lot for the explanation, it makes a lot of sense now. The above
was assumption based, along the lines of "the boot_aggregate gets
measured into the IMA PCR, therefore committing to its value must serve
some purpose and the extended value should probably be genuine".

I would like to make it clear that my main motivation for this patch
wasn't about the IMA-measured boot_aggregate integrity, but more about
not getting blamed for (silently) breaking the null_key auth based
protection guarantees provided for IMA's PCR extends with the last patch
in this series (which would skip some extends conditioned on what's
previously been read). FWIW, it's been agreed upon to split this series
in batches, with the first one extending to up to [9/13] only, so this
patch here wouldn't be part of that.

>> +		/*
>> +		 * 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.

Noted, I didn't realize there's a lazy flushing scheme in place.

Thanks!

Nicolai

>
> 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.
>

-- 
SUSE Software Solutions Germany GmbH, Frankenstraße 146, 90461 Nürnberg, Germany
GF: Ivo Totev, Andrew McDonald, Werner Knoblich
(HRB 36809, AG Nürnberg)