Date: Mon, 22 Jan 2024 13:58:42 -0800
From: "Xing, Cedric" <cedric.xing@...el.com>
To: Dan Williams <dan.j.williams@...el.com>, "Yao, Jiewen"
	<jiewen.yao@...el.com>, Qinkun Bao <qinkun@...gle.com>, Samuel Ortiz
	<sameo@...osinc.com>, "Lu, Ken" <ken.lu@...el.com>
CC: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>,
	"linux-coco@...ts.linux.dev" <linux-coco@...ts.linux.dev>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH v1 3/4] tsm: Allow for mapping RTMRs to TCG TPM PCRs



On 1/22/2024 12:10 PM, Dan Williams wrote:
> Yao, Jiewen wrote:
>> Comment below:
>>
>>> -----Original Message-----
>>> From: Qinkun Bao <qinkun@...gle.com>
>>> Sent: Monday, January 22, 2024 10:13 AM
>>> To: Samuel Ortiz <sameo@...osinc.com>; Yao, Jiewen <jiewen.yao@...el.com>;
>>> Lu, Ken <ken.lu@...el.com>
>>> Cc: Kuppuswamy Sathyanarayanan
>>> <sathyanarayanan.kuppuswamy@...ux.intel.com>; Williams, Dan J
>>> <dan.j.williams@...el.com>; linux-coco@...ts.linux.dev; linux-
>>> kernel@...r.kernel.org
>>> Subject: Re: [RFC PATCH v1 3/4] tsm: Allow for mapping RTMRs to TCG TPM PCRs
>>>
>>>
>>>
>>>> On Jan 21, 2024, at 8:31 AM, Samuel Ortiz <sameo@...osinc.com> wrote:
>>>>
>>>> On Tue, Jan 16, 2024 at 07:35:30PM -0800, Kuppuswamy Sathyanarayanan
>>> wrote:
>>>>>
>>>>> On 1/16/24 5:24 PM, Dan Williams wrote:
>>>>>> Kuppuswamy Sathyanarayanan wrote:
>>>>>>> On 1/14/24 2:35 PM, Samuel Ortiz wrote:
>>>>>>>> Many user space and internal kernel subsystems (e.g. the Linux IMA)
>>>>>>>> expect a Root of Trust for Storage (RTS) that allows for extending
>>>>>>>> and reading measurement registers that are compatible with the TCG TPM
>>>>>>>> PCRs layout, e.g. a TPM. In order to allow those components to
>>>>>>>> alternatively use a platform TSM as their RTS, a TVM could map the
>>>>>>>> available RTMRs to one or more TCG TPM PCRs. Once configured, those
>>> PCR
>>>>>>>> to RTMR mappings give the kernel TSM layer all the necessary information
>>>>>>>> to be a RTS for e.g. the Linux IMA or any other components that expects
>>>>>>>> a TCG compliant TPM PCRs layout.
>>>>>>>>
>>>>>>>> TPM PCR mappings are configured through configfs:
>>>>>>>>
>>>>>>>> // Create and configure 2 RTMRs
>>>>>>>> mkdir /sys/kernel/config/tsm/rtmrs/rtmr0
>>>>>>>> mkdir /sys/kernel/config/tsm/rtmrs/rtmr1
>>>>>>>> echo 0 > /sys/kernel/config/tsm/rtmrs/rtmr0/index
>>>>>>>> echo 1 > /sys/kernel/config/tsm/rtmrs/rtmr1/index
>>>>>>>>
>>>>>>>> // Map RTMR 0 to PCRs 4, 5, 6, 7 and 8
>>>>>>>> echo 4-8 > /sys/kernel/config/tsm/rtmrs/rtmr0/tcg_map
>>>>>>>>
>>>>>>>> // Map RTMR 1 to PCRs 16, 17 and 18
>>>>>>>> echo 16-18 > /sys/kernel/config/tsm/rtmrs/rtmr1/tcg_map
>>>>>>> Any information on how this mapping will be used by TPM or IMA ?
>>>>>>>
>>>>>>> RTMR to PCR mapping is fixed by design, right? If yes, why allow
>>>>>>> user to configure it. We can let vendor drivers to configure it, right?
>>>>>> I assume the "vendor driver", that publishes the RTMR to the tsm-core,
>>>>>> has no idea whether they will be used for PCR emulation, or not. The TPM
>>>>>> proxy layer sitting on top of this would know the mapping of which RTMRs
>>>>>> are recording a transcript of which PCR extend events.
>>>>>
>>>>> My thinking is, since this mapping is ARCH-specific information
>>>>> and fixed by design, it makes more sense to hide this detail in the
>>>>> vendor driver than letting userspace configure it. If we allow users to
>>>>> configure it, there is a chance for incorrect mapping.
>>>>
>>>> I think I agree with the fact that letting users configure that mapping
>>>> may be error prone. But I'm not sure this is an architecture specific
>>>> mapping, but rather a platform specific one. I'd expect the guest firmware
>>>> to provide it through e.g. the MapPcrToMrIndex EFI CC protocol.
>>>>
>>>> So I agree I should remove the user interface for setting that mapping,
>>>> and pass it from the provider capabilities instead. It is then up to the
>>>> provider to choose how it'd build that information (hard coded, from
>>>> EFI, etc).
>>>
>>> The UEFI specification has defined the mapping relationship between the
>>> TDX RTMR and TPM PCRs (See
>>> https://uefi.org/specs/UEFI/2.10/38_Confidential_Computing.html#intel-trust-
>>> domain-extension). The current RTMR implementation in the boot loader
>>> is “hooked” in the implementation for the TPM.
>>>
>>> When the bootloader needs to extend the PCR value, it calls
>>> `map_pcr_to_mr_index`  to retrieve the corresponding RTMR index and
>>> then extends the RTMR. Considering this behavior, I don’t think we should
>>> allow users to configure the mappings between the PCR and RTMR. (See
>>> https://github.com/rhboot/shim/pull/485/files).
>>>
>>> Add Jiewen (owner of the RTMR changes in the firmware) and Ken (
>>> owner of the RTMR changes in the boot loader) for the visibility.
>>
>> I think the mapping should be static and determined by the hardware architecture.
>>
>> Allowing user to configure the mapping just adds complexity and
>> confusing. For example, the user must understand clearly on what is
>> Intel-TDX/AMD-SEV/ARM-CCA/RISCV-CoVE, how many registers they have,
>> what is the best way to map it.
>>
>> It also adds complexity to the verifier. For example, the verifier
>> must understand how a user configure the mapping, then get the
>> expected measurement register value.
> 
> I agree with this, what I have a problem with is that this:
> 
> https://uefi.org/specs/UEFI/2.10/38_Confidential_Computing.html#intel-trust-domain-extension
> 
> ...is vendor specific, especially when the kernel enabling is being
> targeted as cross-vendor.
> 

I have the same concern.

> So, yes, the mapping should be allowed to specified by the low-level
> driver, but at the same time every vendor should not reinvent their own
> enumeration method when we have EFI for that.
> 

Given PCR->RTMR mapping is static, I just wonder why it needs to be kept 
in kernel. Given that PCRs can never be 1:1 mapped to RTMRs, and that 
TDX quotes are never TPM quotes, applications used to extend PCRs would 
have to be changed/recompiled. Then wouldn't it suffice to define the 
mappings as macros in an architecture specific header file?

Powered by blists - more mailing lists