Received-SPF: pass (google.com: domain of linux-kernel+bounces-54500-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <b8140cc8-a56b-40f6-a593-7be49db14c77@intel.com>
Date: Tue, 6 Feb 2024 00:34:59 -0800
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC PATCH v2 0/4] tsm: Runtime measurement registers ABI
To: James Bottomley <James.Bottomley@HansenPartnership.com>, "Kuppuswamy
 Sathyanarayanan" <sathyanarayanan.kuppuswamy@linux.intel.com>, Dan Middleton
	<dan.middleton@linux.intel.com>, Samuel Ortiz <sameo@rivosinc.com>, "Dan
 Williams" <dan.j.williams@intel.com>
CC: Qinkun Bao <qinkun@google.com>, "Yao, Jiewen" <jiewen.yao@intel.com>,
	Dionna Amalie Glaze <dionnaglaze@google.com>, <biao.lu@intel.com>,
	<linux-coco@lists.linux.dev>, <linux-integrity@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
References: <20240128212532.2754325-1-sameo@rivosinc.com>
 <c17a31e4fb30f5f9d4a337e5bd8d54cc6f99eef7.camel@HansenPartnership.com>
 <a255bc36-2438-41b7-b304-bcf7a6628bef@linux.intel.com>
 <42e14f74d3819c95fdb97cd2e9b2829dcb1b1563.camel@HansenPartnership.com>
 <1557f98a-3d52-4a02-992b-4401c7c85dd7@linux.intel.com>
 <85b7a4a679eada1d17b311bf004c2d9e18ab5cd3.camel@HansenPartnership.com>
Content-Language: en-US
From: "Xing, Cedric" <cedric.xing@intel.com>
In-Reply-To: <85b7a4a679eada1d17b311bf004c2d9e18ab5cd3.camel@HansenPartnership.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?NmdjK2gwbVJkSENCOStXQTNxWmNJck9OZ2pBaHl0L2UyQ3BpTVY4OEFvOVdQ?=
 =?utf-8?B?SGZTaUgzZWM4TEdIMGlqbVB2WmJTUHFmSUJRRDBQRWUxNHBjUmtQS1FBcy9h?=
 =?utf-8?B?REJjaVFkakE5eXlzNmYxYXdNazNqNkFrdVRucGVMR1hraUs3bW93QXlSeVRD?=
 =?utf-8?B?RnIrTzRiMzE5OEkxdDJ5c0w0bTYvZWVMb3IyNWNibVpReHhwMzg4VjA5TWsw?=
 =?utf-8?B?UWs0ckFzQThGZHNJRWR6clg5L20vaEowWGEzamNHZVNabjdZaktmbk1Cc29x?=
 =?utf-8?B?RVNCQTZERStNczNaeWNiczJwMi9ueDZkWk1kazlMZ2JjRmxHSllKR1ppVnM1?=
 =?utf-8?B?TTNBOTcrSmx4aFQ2Q1ZHVGZKUklOVVhhVFp3aTBRSjZqYjl2cVl5NVUybHgw?=
 =?utf-8?B?WWNGL3ZTN3A0cWR3bEhxbWg4UVFnNDlrLytQRktRcHVmbnUwMTE1QnJNZUl3?=
 =?utf-8?B?Q3lEUWtwNzBBSU5IZkdEaFBNcTNPSVJocC9XNkFSeWROdDMxNzE0bFd5YTZS?=
 =?utf-8?B?M3g2TkRvTE9FYzdqdkVhazhsWkRzMUxROHdqMFRlb1hpcTJiTzlaTVA3NzJ0?=
 =?utf-8?B?SG1Yb2hmRzRBdElraWpmLytEckdNMUtYZ3kwM2p4TXZlRmF0SlRTZ0VsQ3gy?=
 =?utf-8?B?MFNqRndhQkZWaHhJL0wzZTJMcFZmYUgvVjN1WGh2N0NJTWYwYWdXV3dsb1Jr?=
 =?utf-8?B?TzNQRkQyQTBaS2c5THFWVzhiRC9VZk1BNGNPOExseWJneGNLSkdCV0VQbmxk?=
 =?utf-8?B?dHo1cmtiTE5kS1NWY1VyZG1ENVV1R2JpQ0FlYnZ0NWVkKytnWlZuTWxxNHFE?=
 =?utf-8?B?ZUJLaEtpSE5GV1V3U0drbUdZUUI1ak9Dcm9YaTUwejByNlhvVnJXYUQyV3hw?=
 =?utf-8?B?bGd5a0wrb0k2d0d0RmxCSnVFY1N6cFE4a3duUjV6OFhwSHNLSDY0cUVINmxP?=
 =?utf-8?B?alp2TXkxSEFEMmc1eERKUWtlWHJZVThLcmRWc1dNaWpIbFZzdFFCSlJuRHZV?=
 =?utf-8?B?OUw3dGhnaXkreHRNdWNCaHhDQmlMMDNkOFVlOU5uVG9uY09ETXFLS1RjZjlh?=
 =?utf-8?B?NUxta1habkpXZmRrRlZlcEZER3pLci8xalJlZGdwWHNIcWNQWlQwODJDRjNL?=
 =?utf-8?B?K1BUdU56NGduOTVxRno1RURVcVpFV3JjTkkxczAyNGhxN0Vqb2NhMnkxRDdh?=
 =?utf-8?B?TnhWekNGcEIvQkxwbHNIRW15U0N0YnJXQVBNRi9Sa29zSlhaZE5kMU1SZVpJ?=
 =?utf-8?B?M0lNbU42Z3ZMckc4V0hBR09tUkpqODVIeDI2ZTdvV1Y2SFhlRURaZWhsQ2FS?=
 =?utf-8?B?cnFxWkVFN3Uzc1l6UTFxWkJnaHdEd215U0NVd3ZFOVBNZkV4SEpzeng3TUll?=
 =?utf-8?B?ck40MWJzNmZOQU1NSnc3dW4xYXFHZUpweXNrQU56NGYydDZuNk9Ya1MvZFdG?=
 =?utf-8?B?MTRDS2pCWmVoTkZnZFJCOHhvaURjN2xGcVBvS2lPVGtoU0hEaVl2MDVjTDJG?=
 =?utf-8?B?TTU2OXVxdis2TG1XSHZicDNneDZUa2NHR1BEQTcxOWxmSUJIcXd6d1ltcWJB?=
 =?utf-8?B?MlBFKzRZVThHVzk5MStnYTJXMndXekhsNzUxYXl4ektzZXliQ1RUaDgzQVZo?=
 =?utf-8?B?MWlpaFRQVHBWUTFEQjlWTG5FMGcvK3hkamJKWTJhdVlxWERIdkpPTXQvbHJH?=
 =?utf-8?B?QnFTaUNRY2NsTHdFVC9GL1BrS21FTVYvS1liVHJaaU1wbGRrK0k1d3BkM1p0?=
 =?utf-8?B?MW8yV1kvTStmNkpUdnVMS0o4YktiNmNRTUpjem9Vd0hXTHhKWUJNWWFEd0Fa?=
 =?utf-8?B?bnFaVkJMdzRMRWUySFA1Smw1anRiZUtISDFsQzZCTGhLMjg4UExCWForU2xX?=
 =?utf-8?B?alByZkNLanhIMDJpYXRtQjdBcmEyb2RlZVJwQ2w2c01nbGxsQnJEMXVsaVY4?=
 =?utf-8?B?NXRMSTN0V2hwdTJIb3UvNFNWUWlYSHVkMEhscVU3QXJhMmhidTBJdTQ0TnZC?=
 =?utf-8?B?TGVZVjNGSXRFOU5idnVsWjdUZmxkVTc2b0FPNDB5UG11R0ZZcVFQQ1BpK25r?=
 =?utf-8?B?SDBrck8zZ052d1FKNzQ5QkJGYkhUaFNpN2VXa3orK0VDc1lkUkJEQzhLUDJO?=
 =?utf-8?Q?GOh1yU6rMYwUHXUIo8YT5+SSM?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 67908461-9e04-4c6e-8097-08dc26ee8347
X-MS-Exchange-CrossTenant-AuthSource: DS7PR11MB6269.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Feb 2024 08:35:02.7964
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: bnVYdprOvUuVemnWdfksZBA8V9mMhPGRCBFcf1+rUla3Y6nOm3R82+gxk5UVM6PRLgum+F1VIXmZ8dxYmFYSgA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5535
X-OriginatorOrg: intel.com

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
>