public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Min Xu" <min.m.xu@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"kraxel@redhat.com" <kraxel@redhat.com>
Cc: "Aktas, Erdem" <erdemaktas@google.com>,
	James Bottomley <jejb@linux.ibm.com>,
	"Yao, Jiewen" <jiewen.yao@intel.com>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	Michael Roth <michael.roth@amd.com>
Subject: Re: [edk2-devel] [PATCH V1 1/7] OvmfPkg: Add Tdx measurement data structure in WorkArea
Date: Wed, 18 Jan 2023 01:41:15 +0000	[thread overview]
Message-ID: <PH0PR11MB5064B3E67788DFDF533FA684C5C79@PH0PR11MB5064.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20230117112554.opz5cc7edq26raty@sirius.home.kraxel.org>

On January 17, 2023 7:26 PM, Gerd Hoffmann wrote:
> On Tue, Jan 17, 2023 at 03:40:10PM +0800, Min Xu wrote:
> > From: Min M Xu <min.m.xu@intel.com>
> >
> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4243
> >
> > From the perspective of security any external input should be measured
> > and extended to some registers (TPM PCRs or TDX RTMR registers).
> >
> > There are below 2 external input in a Td guest:
> >  - TdHob
> >  - Configuration FV (CFV)
> >
> > TdHob contains the resource information passed from VMM, such as
> > unaccepted memory region. CFV contains the configurations, such as
> > secure boot variables.
> >
> > TdHob and CFV should be measured and extended to RTMRs before they're
> > consumed. TdHob is consumed in the very early stage of boot process.
> > At that moment the memory service is not ready. Cfv is consumed in
> > PlatformPei to initialize the EmuVariableNvStore. To make the
> > implementation simple and clean, these 2 external input are measured
> > and extended to RTMRs in SEC phase.  The measurement values are stored
> > in WorkArea. Then after the Hob service is available, these 2
> > measurement values are retrieved and GuidHobs for these 2 tdx
> > measurements are generated.
> 
> So the measurement is done early and the hashes are stored to create the
> event log entries later, correct?
Yes.
> 
> Why both TdHob and CFV are handled this way?  It should be needed for
> TdHob only, right?  The work area has a fixed size, IMHO we should not store
> data there unless we absolutely have to, and for CFV I don't see the
> justification.
In our first design CFV was measured and extended in PEI phase. Because CFV is consumed in PlatformInitEmuVariableNvStore. 
But then we find a problem. That we must either refactor the HashLibBaseCryptoRouterPei or introduce a new HashLib in PEI phase.
1) If HashLibBaseCryptoRouterPei is to be refactored to support tdx-measurement, then it must detect the tdx-guest in run-time so that it can determine to call Tpm2PcrExtend or call TdxExtendRtmr. 
2) If we import a new HashLib in PEI phase, we are facing another problem, that we have to load either the new HashLib or HashLibBaseCryptoRouterPei in run-time.

Cfv is measured and extended in both OvmfPkgX64 and IntelTdxX64. Our current design reduces the code duplication of measurement, as well as the generation of GuidHob for the measurement. We have the helper function in SEC phase to do the measurement for TdHob, it's easy to measure Cfv as well. From the security perspective, the earlier the Cfv is measured/extended the better.

As to the work-area, now the size of work-area is 4096 bytes. Before this patch TDX uses 4+16 bytes. TDX_MEASUREMENTS_DATA uses 4+48+48=100 bytes. So totally 120 bytes are used. I don't think the size is a problem. And if Cfv is measured in SEC phase, then its measurement value has to be stored in work-area.

Based on above consideration, we finally propose this solution.

Thanks
Min

  reply	other threads:[~2023-01-18  1:41 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-17  7:40 [PATCH V1 0/7] Enable Tdx measurement in OvmfPkgX64 Min Xu
2023-01-17  7:40 ` [PATCH V1 1/7] OvmfPkg: Add Tdx measurement data structure in WorkArea Min Xu
2023-01-17 11:25   ` Gerd Hoffmann
2023-01-18  1:41     ` Min Xu [this message]
2023-01-18  8:04       ` [edk2-devel] " Gerd Hoffmann
2023-01-18  8:09         ` Min Xu
2023-01-17  7:40 ` [PATCH V1 2/7] OvmfPkg/IntelTdx: Add TdxHelperLib Min Xu
2023-01-17  7:40 ` [PATCH V1 3/7] OvmfPkg/PeilessStartupLib: Build GuidHob for Tdx measurements Min Xu
2023-01-17  7:40 ` [PATCH V1 4/7] OvmfPkg/IntelTdx: Update tdx measurement in SEC phase Min Xu
2023-01-17  7:40 ` [PATCH V1 5/7] OvmfPkg: Enable Tdx measurement in OvmfPkgX64 Min Xu
2023-01-17 11:28   ` Gerd Hoffmann
2023-01-17  7:40 ` [PATCH V1 6/7] OvmfPkg/PlatformInitLib: Delete the ProcessTdxHobList() Min Xu
2023-01-17  7:40 ` [PATCH V1 7/7] OvmfPkg/PlatformPei: Build GuidHob for Tdx measurement Min Xu
2023-01-17 11:22 ` [PATCH V1 0/7] Enable Tdx measurement in OvmfPkgX64 Gerd Hoffmann
2023-01-17 13:09   ` Min Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=PH0PR11MB5064B3E67788DFDF533FA684C5C79@PH0PR11MB5064.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox