Message-ID: <b8140cc8-a56b-40f6-a593-7be49db14c77@intel.com>
Date: Tue, 6 Feb 2024 00:34:59 -0800
From: "Xing, Cedric" <cedric.xing@...el.com>
To: James Bottomley <James.Bottomley@...senPartnership.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 2/3/2024 2:27 AM, James Bottomley wrote:
> On Fri, 2024-02-02 at 23:13 -0800, Kuppuswamy Sathyanarayanan wrote:
>>
>> On 2/2/24 10:03 PM, James Bottomley wrote:
>>> On Fri, 2024-02-02 at 17:07 -0600, Dan Middleton wrote:
>>>> On 2/2/24 12:24 AM, James Bottomley wrote:
>>>>> On Sun, 2024-01-28 at 22:25 +0100, Samuel Ortiz wrote:
>>>>>> All architectures supporting RTMRs expose a similar interface
>>>>>> to
>>>>>> their TVMs: An extension command/call that takes a
>>>>>> measurement
>>>>>> value and an RTMR index to extend it with, and a readback
>>>>>> command
>>>>>> for reading an RTMR value back (taking an RTMR index as an
>>>>>> argument as well). This patch series builds an architecture
>>>>>> agnostic, configfs-based ABI for userspace to extend and read
>>>>>> RTMR values back. It extends the current TSM ops structure
>>>>>> and
>>>>>> each confidential computing architecture can implement this
>>>>>> extension to provide RTMR support.
>>>>> What's the actual use case for this?  At the moment the TPM
>>>>> PCRs
>>>>> only provide a read interface to userspace (via
>>>>> /sys/class/tpm/tpmX/pcr-shaY/Z) and don't have any extension
>>>>> ability becuase nothing in userspace currently extends them.
>>>>>
>>>>> The only current runtime use for TPM PCRs is IMA, which is in-
>>>>> kernel (and which this patch doesn't enable).
>>>>>
>>>>> Without the ability to log, this interface is unusable anyway,
>>>>> but
>>>>> even with that it's not clear that you need the ability
>>>>> separately
>>>>> to extend PCRs because the extension and log entry should be
>>>>> done
>>>>> atomically to prevent the log going out of sync with the PCRs,
>>>>> so
>>>>> it would seem a log first interface would be the correct way of
>>>>> doing this rather than a PCR first one.
>>>>>
>>>>> James
>>>>>
>>>>>
>>>> While we clearly need to cover PCR-like usages, I think
>>>> Confidential
>>>> Computing affords usages that go beyond TPM.
>>> Well, don't get me wrong, I think the ability to create non
>>> repudiable
>>> log entries from userspace is very useful.  However, I think the
>>> proposed ABI is wrong: it should take the log entry (which will
>>> contain
>>> the PCR number and the hash) then do the extension and add it to
>>> the
>>> log so we get the non-repudiable verifiability.  This should work
>>> equally with TPM and RTMR (and anything else).
>>
>> Maybe I misunderstood your comments, but I am not sure why
>> the user ABI needs to change?
> 
> Well, there is no ABI currently, so I'm saying get it right before
> there is one.
> 
>>   I agree that logging after extension is the right approach. But,
>> IMO, it should be owned by the back end TSM vendor drivers. The user
>> ABI should just pass the digest and RTMR index.
> 
> Well, lets wind back to the assumptions about the log.  The current
> convention from IMA and Measured Boot is that the log is managed by the
> kernel.  Given the potential problems with timing and serialization
> (which can cause log mismatches) it would make sense for this ABI also
> to have a kernel backed log (probably a new one from the other two).

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. IMA seems to maintain the measurement list/log by 
itself. Am I right? If so, why do we want logging to be part of TSM here?

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.

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

> James
> 

Powered by blists - more mailing lists