Hi Min One more thing: This is to measure ACPI table. According to https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmware-profile-specification/, the event type should be EV_POST_CODE, and event data should be "ACPI DATA" in all caps. You can use below pattern: ================= TpmMeasureAndLogData ( 0, EV_POST_CODE, EV_POSTCODE_INFO_ACPI_DATA, ACPI_DATA_LEN, Table, TableSize ); =================
> -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, > Jiewen > Sent: Monday, January 16, 2023 10:28 PM > To: devel@edk2.groups.io; a...@kernel.org > Cc: Xu, Min M <min.m...@intel.com>; Aktas, Erdem > <erdemak...@google.com>; James Bottomley <j...@linux.ibm.com>; Gerd > Hoffmann <kra...@redhat.com>; Tom Lendacky > <thomas.lenda...@amd.com>; Michael Roth <michael.r...@amd.com> > Subject: Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/AcpiPlatformDxe: > Measure ACPI table from QEMU in TDVF > > Each module, which wants to load ACPI table. > > Most codes are from close source platform. > > One example in EDKII SecurityPkg is TPM2. > https://github.com/tianocore/edk2/blob/master/SecurityPkg/Tcg/Tcg2Acpi/ > Tcg2Acpi.c#L690 > > https://github.com/tianocore/edk2/blob/master/SecurityPkg/Tcg/Tcg2Acpi/ > Tcg2Acpi.c#L794 > > Just search "TpmMeasureAndLogData" ... > > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard > > Biesheuvel > > Sent: Monday, January 16, 2023 10:15 PM > > To: devel@edk2.groups.io; Yao, Jiewen <jiewen....@intel.com> > > Cc: Xu, Min M <min.m...@intel.com>; Aktas, Erdem > > <erdemak...@google.com>; James Bottomley <j...@linux.ibm.com>; Gerd > > Hoffmann <kra...@redhat.com>; Tom Lendacky > > <thomas.lenda...@amd.com>; Michael Roth <michael.r...@amd.com> > > Subject: Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/AcpiPlatformDxe: > > Measure ACPI table from QEMU in TDVF > > > > On Mon, 16 Jan 2023 at 14:57, Yao, Jiewen <jiewen....@intel.com> wrote: > > > > > > Do you mean real platform BIOS? > > > > > > > Yes > > > > > The rules are: > > > 1) The ACPI table on the flash loaded by the BIOS is measured *before* > any > > modification and *before* installation. For example, DSDT, SSDT. > > > 2) The ACPI table constructed by the BIOS dynamically does not need to > be > > measured. For example MADT. > > > > > > > OK so which EDK2 component is normally in charge of this? > > > > > > > > > > -----Original Message----- > > > > From: Ard Biesheuvel <a...@kernel.org> > > > > Sent: Monday, January 16, 2023 6:38 PM > > > > To: devel@edk2.groups.io; Yao, Jiewen <jiewen....@intel.com> > > > > Cc: Xu, Min M <min.m...@intel.com>; Aktas, Erdem > > > > <erdemak...@google.com>; James Bottomley <j...@linux.ibm.com>; > > Gerd > > > > Hoffmann <kra...@redhat.com>; Tom Lendacky > > > > <thomas.lenda...@amd.com>; Michael Roth <michael.r...@amd.com> > > > > Subject: Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/AcpiPlatformDxe: > > > > Measure ACPI table from QEMU in TDVF > > > > > > > > Another question: how does this deviate from ordinary measured boot? > > > > How and when are ACPI tables measured on such systems? > > > > > > > > > > > > On Mon, 16 Jan 2023 at 03:42, Yao, Jiewen <jiewen....@intel.com> > > wrote: > > > > > > > > > > Hi > > > > > I have two feedback: > > > > > > > > > > 1) What we need measure is the input from VMM *before* any > > > > modification and *before* installation. > > > > > Please don't use ACPI SDT protocol to get the table *after* > modification. > > > > > > > > > > 2) Why not use TpmMeasureAndLogData() in > > > > > > > https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Include/Li > > > > brary/TpmMeasurementLib.h ? > > > > > Please don't use locate protocol. > > > > > > > > > > Thank you > > > > > Yao, Jiewen > > > > > > > > > > > -----Original Message----- > > > > > > From: Xu, Min M <min.m...@intel.com> > > > > > > Sent: Monday, January 16, 2023 9:57 AM > > > > > > To: devel@edk2.groups.io > > > > > > Cc: Xu, Min M <min.m...@intel.com>; Aktas, Erdem > > > > > > <erdemak...@google.com>; James Bottomley > <j...@linux.ibm.com>; > > > > Yao, > > > > > > Jiewen <jiewen....@intel.com>; Gerd Hoffmann > > <kra...@redhat.com>; > > > > > > Tom Lendacky <thomas.lenda...@amd.com>; Michael Roth > > > > > > <michael.r...@amd.com> > > > > > > Subject: [PATCH V1 1/1] OvmfPkg/AcpiPlatformDxe: Measure ACPI > > table > > > > from > > > > > > QEMU in TDVF > > > > > > > > > > > > From: Min M Xu <min.m...@intel.com> > > > > > > > > > > > > https://bugzilla.tianocore.org/show_bug.cgi?id=4245 > > > > > > > > > > > > The ACPI tables are downloaded from QEMU. QEMU/VMM is > treated > > as > > > > > > un-trusted in a td-guest. From the security perspective they should > > > > > > be measured and extended if possible. So that they can be audited > > > > > > later. The measurement protocol may be not installed. In this case > > > > > > it still returns EFI_SUCCESS. > > > > > > > > > > > > Cc: Erdem Aktas <erdemak...@google.com> > > > > > > Cc: James Bottomley <j...@linux.ibm.com> > > > > > > Cc: Jiewen Yao <jiewen....@intel.com> > > > > > > Cc: Gerd Hoffmann <kra...@redhat.com> > > > > > > Cc: Tom Lendacky <thomas.lenda...@amd.com> > > > > > > Cc: Michael Roth <michael.r...@amd.com> > > > > > > Signed-off-by: Min Xu <min.m...@intel.com> > > > > > > --- > > > > > > OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf | 2 + > > > > > > OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 168 > > > > > > ++++++++++++++++++++ > > > > > > 2 files changed, 170 insertions(+) > > > > > > > > > > > > diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf > > > > > > b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf > > > > > > index 8939dde42549..ae22bab38cf9 100644 > > > > > > --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf > > > > > > +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf > > > > > > @@ -51,6 +51,8 @@ > > > > > > gEfiAcpiTableProtocolGuid # PROTOCOL > > > > ALWAYS_CONSUMED > > > > > > gEfiPciIoProtocolGuid # PROTOCOL > > > > SOMETIMES_CONSUMED > > > > > > gQemuAcpiTableNotifyProtocolGuid # PROTOCOL > PRODUCES > > > > > > + gEfiCcMeasurementProtocolGuid # PROTOCOL > > > > > > SOMETIMES_CONSUMED > > > > > > + gEfiAcpiSdtProtocolGuid # PROTOCOL > > > > > > SOMETIMES_CONSUMED > > > > > > > > > > > > [Guids] > > > > > > gRootBridgesConnectedEventGroupGuid > > > > > > diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c > > > > > > b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c > > > > > > index f0d81d6fd73d..f442850c2e00 100644 > > > > > > --- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c > > > > > > +++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c > > > > > > @@ -18,6 +18,8 @@ > > > > > > #include <Library/QemuFwCfgLib.h> // > > > > > > QemuFwCfgFindFile() > > > > > > #include <Library/QemuFwCfgS3Lib.h> // > > QemuFwCfgS3Enabled() > > > > > > #include <Library/UefiBootServicesTableLib.h> // gBS > > > > > > +#include <Protocol/AcpiSystemDescriptionTable.h> > > > > > > +#include <Protocol/CcMeasurement.h> > > > > > > > > > > > > #include "AcpiPlatform.h" > > > > > > > > > > > > @@ -812,12 +814,168 @@ UndoCmdWritePointer ( > > > > > > )); > > > > > > } > > > > > > > > > > > > +/** > > > > > > + Mesure firmware ACPI table with CcMeasurement Protocol > > > > > > + > > > > > > + @param[in] CcProtocol Pointer to the CcMeasurment Protocol > > > > > > + @param[in] EventData Pointer to the event data. > > > > > > + @param[in] EventSize Size of event data. > > > > > > + @param[in] CfgDataBase Configuration data base address. > > > > > > + @param[in] EventSize Size of configuration data . > > > > > > + @retval EFI_NOT_FOUND Cannot locate protocol. > > > > > > + @retval EFI_OUT_OF_RESOURCES Allocate zero pool failure. > > > > > > + @return Status codes returned by > > > > > > + > > > > > > mTcg2Protocol->HashLogExtendEvent. > > > > > > +**/ > > > > > > +STATIC > > > > > > +EFI_STATUS > > > > > > +EFIAPI > > > > > > +CcMeasureAcpiTable ( > > > > > > + IN EFI_CC_MEASUREMENT_PROTOCOL *CcProtocol, > > > > > > + IN CHAR8 *EventData, > > > > > > + IN UINT32 EventSize, > > > > > > + IN EFI_PHYSICAL_ADDRESS CfgDataBase, > > > > > > + IN UINTN CfgDataLength > > > > > > + ) > > > > > > +{ > > > > > > + EFI_STATUS Status; > > > > > > + EFI_CC_EVENT *CcEvent; > > > > > > + UINT32 MrIndex; > > > > > > + > > > > > > + if (CcProtocol == NULL) { > > > > > > + return EFI_INVALID_PARAMETER; > > > > > > + } > > > > > > + > > > > > > + Status = CcProtocol->MapPcrToMrIndex (CcProtocol, 1, > &MrIndex); > > > > > > + if (EFI_ERROR (Status)) { > > > > > > + return EFI_INVALID_PARAMETER; > > > > > > + } > > > > > > + > > > > > > + CcEvent = AllocateZeroPool (EventSize + sizeof (EFI_CC_EVENT) - > > sizeof > > > > > > (CcEvent->Event)); > > > > > > + if (CcEvent == NULL) { > > > > > > + return EFI_OUT_OF_RESOURCES; > > > > > > + } > > > > > > + > > > > > > + CcEvent->Size = EventSize + sizeof > > > > > > (EFI_CC_EVENT) - > sizeof > > > > > > (CcEvent->Event); > > > > > > + CcEvent->Header.EventType = EV_PLATFORM_CONFIG_FLAGS; > > > > > > + CcEvent->Header.MrIndex = MrIndex; > > > > > > + CcEvent->Header.HeaderSize = sizeof (EFI_CC_EVENT_HEADER); > > > > > > + CcEvent->Header.HeaderVersion = > > EFI_CC_EVENT_HEADER_VERSION; > > > > > > + CopyMem (&CcEvent->Event[0], EventData, EventSize); > > > > > > + > > > > > > + Status = CcProtocol->HashLogExtendEvent ( > > > > > > + CcProtocol, > > > > > > + 0, > > > > > > + CfgDataBase, > > > > > > + CfgDataLength, > > > > > > + CcEvent > > > > > > + ); > > > > > > + > > > > > > + FreePool (CcEvent); > > > > > > + > > > > > > + return Status; > > > > > > +} > > > > > > + > > > > > > // > > > > > > // We'll be saving the keys of installed tables so that we can roll > them > > > > back > > > > > > // in case of failure. 128 tables should be enough for anyone (TM). > > > > > > // > > > > > > #define INSTALLED_TABLES_MAX 128 > > > > > > > > > > > > +/** > > > > > > + * The ACPI tables are downloaded from QEMU. From the security > > > > > > perspective these are > > > > > > + * external inputs and should be measured and extended if > possible. > > So > > > > that > > > > > > they can > > > > > > + * be audited later. The measurement protocol may be not > installed. > > In > > > > this > > > > > > case it > > > > > > + * still returns EFI_SUCCESS. > > > > > > + * > > > > > > + * @param InstalledKey Pointer to an array which contains the keys > > of > > > > the > > > > > > installed ACPI tables > > > > > > + * @param Length Length of the array > > > > > > + * @retval EFI_SUCCESS Successfully measure the ACPI tables > > > > > > + * @retval Others Other errors as indicated > > > > > > + */ > > > > > > +STATIC > > > > > > +EFI_STATUS > > > > > > +MeasureInstalledTablesFromQemu ( > > > > > > + IN UINTN *InstalledKey, > > > > > > + IN INT32 Length > > > > > > + ) > > > > > > +{ > > > > > > + EFI_STATUS Status; > > > > > > + UINTN Index1; > > > > > > + INT32 Index2; > > > > > > + EFI_ACPI_SDT_HEADER *Table; > > > > > > + EFI_ACPI_TABLE_VERSION Version; > > > > > > + UINTN TableKey; > > > > > > + EFI_ACPI_SDT_PROTOCOL *AcpiSdtProtocol; > > > > > > + EFI_CC_MEASUREMENT_PROTOCOL *CcProtocol = NULL; > > > > > > + > > > > > > + Status = gBS->LocateProtocol (&gEfiCcMeasurementProtocolGuid, > > NULL, > > > > > > (VOID **)&CcProtocol); > > > > > > + if (EFI_ERROR (Status)) { > > > > > > + // > > > > > > + // CcMeasurement protocol is not installed. > > > > > > + // > > > > > > + return EFI_SUCCESS; > > > > > > + } > > > > > > + > > > > > > + Status = gBS->LocateProtocol (&gEfiAcpiSdtProtocolGuid, NULL, > > (void > > > > > > **)&AcpiSdtProtocol); > > > > > > + if (EFI_ERROR (Status)) { > > > > > > + DEBUG ((DEBUG_ERROR, "Unable to locate ACPI SDT > protocol.\n")); > > > > > > + return Status; > > > > > > + } > > > > > > + > > > > > > + Index1 = 0; > > > > > > + do { > > > > > > + Status = AcpiSdtProtocol->GetAcpiTable (Index1, &Table, > &Version, > > > > > > &TableKey); > > > > > > + if (EFI_ERROR (Status)) { > > > > > > + if (Status == EFI_NOT_FOUND) { > > > > > > + // > > > > > > + // There is no more ACPI tables found. So we return with > > > > EFI_SUCCESS. > > > > > > + // > > > > > > + Status = EFI_SUCCESS; > > > > > > + } > > > > > > + > > > > > > + break; > > > > > > + } > > > > > > + > > > > > > + for (Index2 = 0; Index2 < Length; Index2++) { > > > > > > + if (TableKey == InstalledKey[Index2]) { > > > > > > + break; > > > > > > + } > > > > > > + } > > > > > > + > > > > > > + if (Index2 < Length) { > > > > > > + Status = CcMeasureAcpiTable ( > > > > > > + CcProtocol, > > > > > > + (CHAR8 *)&Table->Signature, > > > > > > + sizeof (Table->Signature), > > > > > > + (EFI_PHYSICAL_ADDRESS)(UINTN)Table, > > > > > > + Table->Length > > > > > > + ); > > > > > > + if (EFI_ERROR (Status)) { > > > > > > + DEBUG (( > > > > > > + DEBUG_ERROR, > > > > > > + "Measure ACPI table [%-4.4a] with Size = 0x%x failed! > > > > > > Status > > > > = %r\n", > > > > > > + (CONST CHAR8 *)&Table->Signature, > > > > > > + Table->Length, > > > > > > + Status > > > > > > + )); > > > > > > + break; > > > > > > + } else { > > > > > > + DEBUG (( > > > > > > + DEBUG_INFO, > > > > > > + "Measure ACPI table [%-4.4a] with Size = 0x%x\n", > > > > > > + (CONST CHAR8 *)&Table->Signature, > > > > > > + Table->Length > > > > > > + )); > > > > > > + } > > > > > > + } > > > > > > + > > > > > > + Index1++; > > > > > > + } while (TRUE); > > > > > > + > > > > > > + return Status; > > > > > > +} > > > > > > + > > > > > > /** > > > > > > Process a QEMU_LOADER_ADD_POINTER command in order to > see > > if its > > > > > > target byte > > > > > > array is an ACPI table, and if so, install it. > > > > > > @@ -1247,6 +1405,16 @@ InstallQemuFwCfgTables ( > > > > > > } > > > > > > } > > > > > > > > > > > > + // > > > > > > + // Measure the ACPI tables which are downloaded from QEMU > > > > > > + // > > > > > > + if (Installed > 0) { > > > > > > + Status = MeasureInstalledTablesFromQemu (InstalledKey, > > Installed); > > > > > > + if (EFI_ERROR (Status)) { > > > > > > + goto UninstallAcpiTables; > > > > > > + } > > > > > > + } > > > > > > + > > > > > > // > > > > > > // Install a protocol to notify that the ACPI table provided by > > > > > > Qemu > > is > > > > > > // ready. > > > > > > -- > > > > > > 2.29.2.windows.2 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#98623): https://edk2.groups.io/g/devel/message/98623 Mute This Topic: https://groups.io/mt/96299277/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
