[edk2-devel] [PATCH V1 1/1] OvmfPkg/AcpiPlatformDxe: Measure ACPI table from QEMU in TDVF
Yao, Jiewen
jiewen.yao at intel.com
Mon Jan 16 14:27:48 UTC 2023
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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Ard
> Biesheuvel
> Sent: Monday, January 16, 2023 10:15 PM
> To: devel at edk2.groups.io; Yao, Jiewen <jiewen.yao at intel.com>
> Cc: Xu, Min M <min.m.xu at intel.com>; Aktas, Erdem
> <erdemaktas at google.com>; James Bottomley <jejb at linux.ibm.com>; Gerd
> Hoffmann <kraxel at redhat.com>; Tom Lendacky
> <thomas.lendacky at amd.com>; Michael Roth <michael.roth at 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.yao at 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 <ardb at kernel.org>
> > > Sent: Monday, January 16, 2023 6:38 PM
> > > To: devel at edk2.groups.io; Yao, Jiewen <jiewen.yao at intel.com>
> > > Cc: Xu, Min M <min.m.xu at intel.com>; Aktas, Erdem
> > > <erdemaktas at google.com>; James Bottomley <jejb at linux.ibm.com>;
> Gerd
> > > Hoffmann <kraxel at redhat.com>; Tom Lendacky
> > > <thomas.lendacky at amd.com>; Michael Roth <michael.roth at 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.yao at 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.xu at intel.com>
> > > > > Sent: Monday, January 16, 2023 9:57 AM
> > > > > To: devel at edk2.groups.io
> > > > > Cc: Xu, Min M <min.m.xu at intel.com>; Aktas, Erdem
> > > > > <erdemaktas at google.com>; James Bottomley <jejb at linux.ibm.com>;
> > > Yao,
> > > > > Jiewen <jiewen.yao at intel.com>; Gerd Hoffmann
> <kraxel at redhat.com>;
> > > > > Tom Lendacky <thomas.lendacky at amd.com>; Michael Roth
> > > > > <michael.roth at amd.com>
> > > > > Subject: [PATCH V1 1/1] OvmfPkg/AcpiPlatformDxe: Measure ACPI
> table
> > > from
> > > > > QEMU in TDVF
> > > > >
> > > > > From: Min M Xu <min.m.xu at 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 <erdemaktas at google.com>
> > > > > Cc: James Bottomley <jejb at linux.ibm.com>
> > > > > Cc: Jiewen Yao <jiewen.yao at intel.com>
> > > > > Cc: Gerd Hoffmann <kraxel at redhat.com>
> > > > > Cc: Tom Lendacky <thomas.lendacky at amd.com>
> > > > > Cc: Michael Roth <michael.roth at amd.com>
> > > > > Signed-off-by: Min Xu <min.m.xu at 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 (#98574): https://edk2.groups.io/g/devel/message/98574
Mute This Topic: https://groups.io/mt/96299277/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list