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

[Mimi Zohar <zohar@linux.ibm.com>]

On Thu, 2024-11-07 at 03:55 +0200, Jarkko Sakkinen wrote:
> On Thu Nov 7, 2024 at 3:07 AM EET, Mimi Zohar wrote:
> > On Thu, 2024-11-07 at 02:03 +0200, Jarkko Sakkinen wrote:
> > > On Thu Nov 7, 2024 at 1:52 AM EET, Mimi Zohar wrote:
> > > > On Thu, 2024-11-07 at 01:22 +0200, Jarkko Sakkinen wrote:
> > > > > On Thu Nov 7, 2024 at 12:52 AM EET, James Bottomley wrote:
> > > > > > 
> > > > > > 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.
> > > > > 
> > > > > Ah, right then we need the boot flag, and my earlier comments to the
> > > > > parameter apply. I've never used IMA so I don't actually even know in
> > > > > detail how it is using TPM.
> > > > 
> > > > Huh?  A simple explanation is that IMA-measurement maintains a measurement list,
> > > > similar to the pre-boot event log.  Each IMA-measurement record extends the TPM
> > > > PCR (default PCR 10).
> > > > 
> > > > Assuming IMA is enabled in the kernel, then just add "ima_policy=tcb" or
> > > > "ima_policy=critical_data" on the boot command line.  To view the measurement
> > > > records, cat <securityfs>/integrity/ima/ascii_runtime_measurements.  Normally
> > > > the IMA policy specified on the boot command line is replaced with a finer
> > > > grained custom policy.
> > > 
> > > I'll try to figure out how to test it regularly. And yeah we need the
> > > flag obviously.
> > > 
> > > I have my (CI compatible) framework that I run regularly with upstream
> > > that I've mentioned a few times earlier.
> > > 
> > > https://codeberg.org/jarkko/linux-tpmdd-test
> > > 
> > > How would I would make all files in /etc get to get the checksums, and
> > > how can I generate legit and illegit change to some file in that tree?
> > > 
> > > No need to address how to implement that to my framework, I can figure
> > > that out. I just would love throw something so that any performance
> > > regressions will be catched right at the get go, i.e. before they
> > > end up to the mainline.
> > 
> > Yes, I still need to look at it.  FYI, the IMA policy cannot be defined in terms
> > of pathnames.  For testing, we've been loopback mounting a filesystem and
> > defining policy rules based on the UUID of the filesystem.  If you're using
> > SELinux, then rules can be defined in terms of SELinux labels. There are other
> > methods of identifying files.  Ken's been working on new IMA documentation[1],
> > which can be viewed here
> > https://ima-doc.readthedocs.io/en/latest/ima-concepts.html .
> > 
> > Here are some examples as to how to locally verify the IMA measurement list and
> > the boot aggregate.
> > 
> > 1. To locally verify the IMA measurement list matches TPM PCR-10, use evmctl
> > (ima-evm-utils).  For example,
> > 
> > a. An IMA measurement list without integrity violations
> > (/sys/kernel/security/ima/violations)
> > 
> > evmctl ima_measurement /sys/kernel/security/ima/binary_runtime_measurements
> > 
> > b. An IMA measurement list with integrity violations
> > 
> > evmctl ima_measurement --ignore-violations
> > /sys/kernel/security/ima/binary_runtime_measurements
> > 
> > 2. To locally verify the 'boot_aggregate' record, the first record in the IMA
> > measurement list, use "evmctl ima_boot_aggregate -v" and compare the resulting
> > hash with the one in the boot_aggregate record.
> 
> Thanks! I write an issue based on this to my Codeberg repository, and
> purge it once the time. I'll start by that and later on formalize
> some commits or perhaps IMA specific buildroot config...

Another important test would to be to make sure that IMA doesn't go into "TPM-
bypass" mode, which happens when the TPM initialization is for some reason
delayed.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/security/integrity/ima/ima_init.c#n124
> As far as the patch goes, I thought that I refine the patch myself, and
> save everyone's time and nervers from unnecessary reviews rounds. It
> does not make any radical changes to the approach.

Thanks
> 
> See https://lore.kernel.org/linux-integrity/20241107004708.108667-1-jarkko@kernel.org/
> 
> I cannot take reviewed/tested-by's from any of the authors but if you
> can check that it works for you I can surely send it Linus without
> further tags than three SOB's :-) That said happy to get at least
> tested-by from someone.

Our emails crossed.  I suggested removing the word "encrypted" throughout the
patch, as pcr_extend isn't encrypted, just HMAC'ed.

I'll re-test first thing tomorrow morning. Does the module_param require a value
or is specifying the name on the boot command line enough?

> 
> I'll send a PR to Linus as soon as possible.

Ok
> 
> >   
> > [1] https://github.com/linux-integrity/ima-doc
> > [2] https://github.com/linux-integrity/ima-evm-utils/tree/next-testing/

thanks,

Mimi