Re: [RFC PATCH] tpm: Allow the TPM2 pcr_extend HMAC capability to be disabled on boot

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

On Thu, 2024-11-07 at 00:26 +0200, Jarkko Sakkinen wrote:
> On Tue Oct 15, 2024 at 10:39 PM EEST, Mimi Zohar wrote:
> > The initial TPM2 HMAC session capability added HMAC authentication
> > to each and every TPM communication making the pcr_extend
> > performance abysmal for HW TPMs. Further, the new
> > CONFIG_TCG_TPM2_HMAC option was configured by default on x86_64.
> > 
> > The decision to use the TPM2 HMAC session capability feature
> > doesn't differentiate between the critical encrypted and the non-
> > encrypted communication, but when configured is required for all
> > TPM communication.
> > 
> > In addition, the reason to HMAC the tpm2_pcr_extend() as provided
> > in commit 6519fea6fd37 ("tpm: add hmac checks to
> > tpm2_pcr_extend()") was to protect tpm2_pcr_extend() when used by
> > "trusted keys" to lock the PCR.  However, locking the PCR is
> > currently limited to TPM 1.2.
> > 
> > We can revert the commit which adds the HMAC sessions for
> > tpm2_pcr_extend, allow just the TPM2 pcr_extend HMAC capability to
> > be disabled on boot for better IMA performance, or define a generic
> > boot command line option to disable HMAC in general.  This patch
> > allows disabling the HMAC for just the TPM2_pcr_extend.
> > 
> > Fixes: 6519fea6fd37 ("tpm: add hmac checks to tpm2_pcr_extend()")
> > Co-developed-by: Roberto Sassu <roberto.sassu@huawei.com>
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> 
> I have alternative proposal that hit me today.
> 
> First an observation: I think this issue shows that we also stress
> beyond limits desktop configurations with encrypted bus, even tho it
> is
> not in the same way visible. This affects bunch of things, including
> e.g. power consumption. Not a lot but best possible situation would
> be
> if callers could be served without any additional stress.
> 
> A second observation is in [1]: 
> 
> "It is recommended that a TPM implement the RNG in a manner that
> would
> allow it to return RNG octets such that, as long as the value of
> bytesRequested is not greater than the maximum digest size, the
> frequency of bytesRequested being more than the number of octets
> available is an infrequent occurrence."
> 
> I think from this we can derive a fair assumption that with any
> possible
> TPM2 chip we can pull a 32 byte value within a single transcation
> (i.e.
> matching SHA256 digest size).
> 
> So based on these facts I think this might be a sweet spot in making
> a
> compromise between performance and security:
> 
> 1. Generate a 32 byte seed every N iterations (calls of
>    tpm2_get_random(). Store it to chip->random_seed.
> 2. In-between iterations use PRNG to generate the values
>    starting form chip->random_seed.
> 
> I think N could be fairly large without causing any major difference
> (even when analyzed through numerical error analysis) between calling
> TPM2_GetRandom for each and every iteration. And this way bus
> encryption
> never has to be disabled.
> 
> I'd see this as win-win approach.
> 
> PS. I have no idea what kind of PRNG's kernel provides (never used
> such).
> 
> [1] 16.1.TPM2_GetRandom
>    
> https://trustedcomputinggroup.org/wp-content/uploads/TPM-2.0-1.83-Part-3-Commands.pdf

I'm a bit confused here.  It's TPM2_PCR_Extend we have the trouble with
(as Mimi says in her email that you quoted) not TPM2_GetRandom.

The random number generator reseed occurs in a kernel thread that fires
about once a minute, so it doesn't show up in really any of the boot
timings.  Plus even with sessions added, what there now isn't a
significant overhead even to the running kernel given it's asynchronous
and called infrequently.

James