Message-ID: <fe1722c3618a8216cb53b8fd3f1b7cbb6fdff5a0.camel@HansenPartnership.com>
Date: Tue, 06 Feb 2024 08:57:45 +0000
From: James Bottomley <James.Bottomley@...senPartnership.com>
To: "Xing, Cedric" <cedric.xing@...el.com>, Kuppuswamy Sathyanarayanan
	 <sathyanarayanan.kuppuswamy@...ux.intel.com>, Dan Middleton
	 <dan.middleton@...ux.intel.com>, Samuel Ortiz <sameo@...osinc.com>, Dan
	Williams <dan.j.williams@...el.com>
Cc: Qinkun Bao <qinkun@...gle.com>, "Yao, Jiewen" <jiewen.yao@...el.com>, 
 Dionna Amalie Glaze <dionnaglaze@...gle.com>, biao.lu@...el.com,
 linux-coco@...ts.linux.dev,  linux-integrity@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI

On Tue, 2024-02-06 at 00:34 -0800, Xing, Cedric wrote:
[...]
> I'm not familiar with existing TPM code. Per 
> https://elixir.free-electrons.com/linux/latest/source/drivers/char/tpm/tpm-interface.c#L314
> ,
> tpm_pcr_extend() doesn't seem to take/log the actual event, but only 
> extends the PCR.

That's the low level code we build on.  The TPM doesn't maintain a log
at all, just the measuring entity.

>  IMA seems to maintain the measurement list/log by itself.

It does, yes.

>  Am I right? If so, why do we want logging to be part of TSM
> here?

Well, as I said above: without a log you have a combinatoric explosion
of events that lead to the PCR value.

> For measured boots, I think UEFI BIOS has already maintained a log so
> what's needed here is just to expose the log somewhere in sysfs.
> IMHO, I don't think logging is even necessary because everything in
> the boot flow is static, hence a relying party can simply compare
> measurement registers against known good values without looking at
> any log. But please correct me if I have missed anything.

Without the log the UEFI boot flow is way too brittle because
measurements aren't actually static and without knowing what happened
you can't reproduce the PCR value.  It was actually the earliest
insight from the keylime project that it couldn't just define state by
PCR values and had to parse the log instead.

> > If you have a kernel backed log, the ABI for extending it should be
> > where you get the PCR extensions from, that way nothing can go
> > wrong. An API to extend the PCRs separately will only cause pain
> > for people who get it wrong (and lead to ordering issues if more
> > than one thing wants to add to the log, which they will do because
> > neither the TPM nor the RTMRs have enough registers to do one per
> > process that wants to use it if this becomes popular).
> > 
> There's an easy way to solve the synchronization problem in user mode
> by applying flock() on the log file - i.e., a process can extend a 
> measurement register only when holding an exclusive lock on the 
> corresponding log file.

Which would be where exactly? and owned by whom?

>  A possible drawback is it'd allow a malicious
> process to starve all other processes by holding the lock forever, or
> to mess up the log file content intentionally. But that shouldn't be
> a practical problem because the existence of such malicious processes
> would have rendered the CVM untrustworthy anyway - i.e., should the
> CVM still be able to generate a valid attestation, that would only
> lead to a distrust decision by any sane relying party.
> 
> IMHO, if something can be easily solved in user mode, probably it 
> shouldn't be solved in kernel mode.

There isn't really anything more complex about an interface that takes
a log entry, and does the record an extend, than an interface which
takes a PCR extension value.  So best practice would say that you
should create the ABI that you can't get wrong (log and record) rather
than creating one that causes additional problems for userspace.

James

Powered by blists - more mailing lists