Received-SPF: pass (google.com: domain of linux-kernel+bounces-32465-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Mon, 22 Jan 2024 08:49:10 +0100
From: Samuel Ortiz <sameo@rivosinc.com>
To: "Yao, Jiewen" <jiewen.yao@intel.com>
Cc: Qinkun Bao <qinkun@google.com>, "Lu, Ken" <ken.lu@intel.com>,
	Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>,
	"Williams, Dan J" <dan.j.williams@intel.com>,
	"linux-coco@lists.linux.dev" <linux-coco@lists.linux.dev>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH v1 3/4] tsm: Allow for mapping RTMRs to TCG TPM PCRs
Message-ID: <Za4d9iS/lpEOEzpl@vermeer>
References: <20240114223532.290550-1-sameo@rivosinc.com>
 <20240114223532.290550-4-sameo@rivosinc.com>
 <1bbf8d3e-aa94-48c7-a1e4-76f9eefc4af7@linux.intel.com>
 <65a72c305291f_3b8e29484@dwillia2-xfh.jf.intel.com.notmuch>
 <5539c533-37b2-4b12-a5c5-056881cf8e3c@linux.intel.com>
 <Za1G9I+tYuIL9ser@vermeer>
 <CF3D8DE1-AD47-4A77-B8BD-8A12A6F7E9DB@google.com>
 <MW4PR11MB5872F46A2089C8C2D8EF7A008C752@MW4PR11MB5872.namprd11.prod.outlook.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <MW4PR11MB5872F46A2089C8C2D8EF7A008C752@MW4PR11MB5872.namprd11.prod.outlook.com>

Hi Jiewen,

On Mon, Jan 22, 2024 at 02:23:02AM +0000, Yao, Jiewen wrote:
> Comment below:
> 
> > -----Original Message-----
> > From: Qinkun Bao <qinkun@google.com>
> > Sent: Monday, January 22, 2024 10:13 AM
> > To: Samuel Ortiz <sameo@rivosinc.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> > Lu, Ken <ken.lu@intel.com>
> > Cc: Kuppuswamy Sathyanarayanan
> > <sathyanarayanan.kuppuswamy@linux.intel.com>; Williams, Dan J
> > <dan.j.williams@intel.com>; linux-coco@lists.linux.dev; linux-
> > kernel@vger.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@rivosinc.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 believe that hiding detail is a better way to avoid those complexity, and make it easy to use.

I agree.

> Do we have some real use cases that a user MUST configure the mapping?

Not that I know of, and I will remove that userspace interface in v2 of this patchset.

Cheers,
Samuel.