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.
This patch defines the structure of TDX_MEASUREMENTS_DATA in
SEC_TDX_WORK_AREA to store above 2 tdx measurements. It can be extended
to store more tdx measurements if needed in the future.
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Michael Roth <michael.roth@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
OvmfPkg/Include/WorkArea.h | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/OvmfPkg/Include/WorkArea.h b/OvmfPkg/Include/WorkArea.h
index 6c3702b716f0..b1c7045ce18c 100644
--- a/OvmfPkg/Include/WorkArea.h
+++ b/OvmfPkg/Include/WorkArea.h
@@ -11,6 +11,7 @@
#define __OVMF_WORK_AREA_H__
#include <ConfidentialComputingGuestAttr.h>
+#include <IndustryStandard/Tpm20.h>
//
// Confidential computing work area header definition. Any change
@@ -64,13 +65,27 @@ typedef struct _SEV_WORK_AREA {
SEC_SEV_ES_WORK_AREA SevEsWorkArea;
} SEV_WORK_AREA;
+//
+// Start of TDX Specific WorkArea definition
+//
+
+#define TDX_MEASUREMENT_TDHOB_BITMASK 0x1
+#define TDX_MEASUREMENT_CFVIMG_BITMASK 0x2
+
+typedef struct _TDX_MEASUREMENTS_DATA {
+ UINT32 MeasurementsBitmap;
+ UINT8 TdHobHashValue[SHA384_DIGEST_SIZE];
+ UINT8 CfvImgHashValue[SHA384_DIGEST_SIZE];
+} TDX_MEASUREMENTS_DATA;
+
//
// The TDX work area definition
//
typedef struct _SEC_TDX_WORK_AREA {
- UINT32 PageTableReady;
- UINT32 Gpaw;
- UINT64 HobList;
+ UINT32 PageTableReady;
+ UINT32 Gpaw;
+ UINT64 HobList;
+ TDX_MEASUREMENTS_DATA TdxMeasurementsData;
} SEC_TDX_WORK_AREA;
typedef struct _TDX_WORK_AREA {
@@ -78,6 +93,10 @@ typedef struct _TDX_WORK_AREA {
SEC_TDX_WORK_AREA SecTdxWorkArea;
} TDX_WORK_AREA;
+//
+// End of TDX Specific WorkArea definition
+//
+
typedef union {
CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER Header;
SEV_WORK_AREA SevWorkArea;
--
2.29.2.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98642): https://edk2.groups.io/g/devel/message/98642
Mute This Topic: https://groups.io/mt/96325908/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-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? 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. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#98669): https://edk2.groups.io/g/devel/message/98669 Mute This Topic: https://groups.io/mt/96325908/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#98750): https://edk2.groups.io/g/devel/message/98750 Mute This Topic: https://groups.io/mt/96325908/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Wed, Jan 18, 2023 at 01:41:15AM +0000, Xu, Min M wrote: > On January 17, 2023 7:26 PM, Gerd Hoffmann wrote: > > 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. So, in short, we don't have support for TDX measurements in PEI, so you are doing it in SEC instead. Can you note that in the commit message? thanks, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#98772): https://edk2.groups.io/g/devel/message/98772 Mute This Topic: https://groups.io/mt/96325908/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On January 18, 2023 4:05 PM, Gerd Hoffmann wrote: > On Wed, Jan 18, 2023 at 01:41:15AM +0000, Xu, Min M wrote: > > On January 17, 2023 7:26 PM, Gerd Hoffmann wrote: > > > 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. > > So, in short, we don't have support for TDX measurements in PEI, so you are > doing it in SEC instead. Can you note that in the commit message? Right, this patch-set doesn't support tdx measurement in PEI phase. I will note it in the commit message. Thanks Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#98773): https://edk2.groups.io/g/devel/message/98773 Mute This Topic: https://groups.io/mt/96325908/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.