Re: [RFC PATCH v1 3/4] tsm: Allow for mapping RTMRs to TCG TPM PCRs

[Xing, Cedric]

On 1/22/2024 12:10 PM, Dan Williams wrote:
> Yao, Jiewen wrote:
> > Comment below:
> >
> > > -----Original Message-----
> > > From: Qinkun Bao <qinkun@xxxxxxxxxx>
> > > Sent: Monday, January 22, 2024 10:13 AM
> > > To: Samuel Ortiz <sameo@xxxxxxxxxxxx>; Yao, Jiewen <jiewen.yao@xxxxxxxxx>;
> > > Lu, Ken <ken.lu@xxxxxxxxx>
> > > Cc: Kuppuswamy Sathyanarayanan
> > > <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>; Williams, Dan J
> > > <dan.j.williams@xxxxxxxxx>; linux-coco@xxxxxxxxxxxxxxx; linux-
> > > kernel@xxxxxxxxxxxxxxx
> > > 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@xxxxxxxxxxxx> 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?