[edk2-devel] [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
Wang, Jian J
jian.j.wang at intel.com
Mon Jan 6 05:33:16 UTC 2020
Jiewen,
Just one minor comment. With it addressed,
Reviewed-by: Jian J Wang <jian.j.wang at intel.com>
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao at intel.com>
> Sent: Tuesday, December 31, 2019 2:44 PM
> To: devel at edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang at intel.com>; Zhang, Chao B
> <chao.b.zhang at intel.com>
> Subject: [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
>
> Use EV_EFI_PLATFORM_FIRMWARE_BLOB2 if the TCG PFP revision is >= 105.
> Use FvName as the description for the FV.
>
> Cc: Jian J Wang <jian.j.wang at intel.com>
> Cc: Chao Zhang <chao.b.zhang at intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao at intel.com>
> ---
> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 91 ++++++++++++++++++++++++++---
> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 2 +
> 2 files changed, 84 insertions(+), 9 deletions(-)
>
> diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> index 1565d4e402..7d99c7906a 100644
> --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> @@ -37,6 +37,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Library/MemoryAllocationLib.h>
> #include <Library/ReportStatusCodeLib.h>
> #include <Library/ResetSystemLib.h>
> +#include <Library/PrintLib.h>
>
> #define PERF_ID_TCG2_PEI 0x3080
>
> @@ -78,6 +79,18 @@ EFI_PLATFORM_FIRMWARE_BLOB
> *mMeasuredChildFvInfo;
> UINT32 mMeasuredMaxChildFvIndex = 0;
> UINT32 mMeasuredChildFvIndex = 0;
>
> +#pragma pack (1)
> +
> +#define FV_HANDOFF_TABLE_DESC "Fv(XXXXXXXX-XXXX-XXXX-XXXX-
> XXXXXXXXXXXX)"
> +typedef struct {
> + UINT8 BlobDescriptionSize;
> + UINT8 BlobDescription[sizeof(FV_HANDOFF_TABLE_DESC)];
> + EFI_PHYSICAL_ADDRESS BlobBase;
> + UINT64 BlobLength;
> +} FV_HANDOFF_TABLE_POINTERS2;
> +
> +#pragma pack ()
> +
> /**
> Measure and record the Firmware Volume Information once FvInfoPPI install.
>
> @@ -447,6 +460,48 @@ MeasureCRTMVersion (
> );
> }
>
> +/*
> + Get the FvName from the FV header.
> +
> + Causion: The FV is untrusted input.
> +
> + @param[in] FvBase Base address of FV image.
> + @param[in] FvLength Length of FV image.
> +
> + @return FvName pointer
> + @retval NULL FvName is NOT found
> +*/
> +VOID *
> +GetFvName (
> + IN EFI_PHYSICAL_ADDRESS FvBase,
> + IN UINT64 FvLength
> + )
> +{
> + EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
> + EFI_FIRMWARE_VOLUME_EXT_HEADER *FvExtHeader;
> +
> + if (FvBase >= MAX_ADDRESS) {
> + return NULL;
> + }
> + if (FvLength >= MAX_ADDRESS - FvBase) {
> + return NULL;
> + }
> + if (FvLength < sizeof(EFI_FIRMWARE_VOLUME_HEADER)) {
> + return NULL;
> + }
> +
> + FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)FvBase;
> + if (FvHeader->ExtHeaderOffset < sizeof(EFI_FIRMWARE_VOLUME_HEADER)) {
> + return NULL;
> + }
> + if (FvHeader->ExtHeaderOffset +
> sizeof(EFI_FIRMWARE_VOLUME_EXT_HEADER) > FvLength) {
> + return NULL;
> + }
> + FvExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *)(UINTN)(FvBase +
> FvHeader->ExtHeaderOffset);
> +
> + return &FvExtHeader->FvName;
> +}
> +
> /**
> Measure FV image.
> Add it into the measured FV list after the FV is measured successfully.
> @@ -469,6 +524,9 @@ MeasureFvImage (
> UINT32 Index;
> EFI_STATUS Status;
> EFI_PLATFORM_FIRMWARE_BLOB FvBlob;
> + FV_HANDOFF_TABLE_POINTERS2 FvBlob2;
> + VOID *EventData;
> + VOID *FvName;
> TCG_PCR_EVENT_HDR TcgEventHdr;
> UINT32 Instance;
> UINT32 Tpm2HashMask;
> @@ -571,6 +629,21 @@ MeasureFvImage (
> TcgEventHdr.PCRIndex = 0;
> TcgEventHdr.EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB;
> TcgEventHdr.EventSize = sizeof (FvBlob);
> + EventData = &FvBlob;
> +
I'd suggest to put above code in 'else' block to make code easier to read,
i.e. FvBlob for revision less than 105 and FvBlob2 for later ones.
Regards,
Jian
> + if (PcdGet32(PcdTcgPfpMeasurementRevision) >=
> TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105) {
> + FvBlob2.BlobDescriptionSize = sizeof(FvBlob2.BlobDescription);
> + CopyMem (FvBlob2.BlobDescription, FV_HANDOFF_TABLE_DESC,
> sizeof(FvBlob2.BlobDescription));
> + FvName = GetFvName (FvBase, FvLength);
> + if (FvName != NULL) {
> + AsciiSPrint ((CHAR8 *)FvBlob2.BlobDescription,
> sizeof(FvBlob2.BlobDescription), "Fv(%g)", FvName);
> + }
> + FvBlob2.BlobBase = FvBlob.BlobBase;
> + FvBlob2.BlobLength = FvBlob.BlobLength;
> + TcgEventHdr.EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB2;
> + TcgEventHdr.EventSize = sizeof (FvBlob2);
> + EventData = &FvBlob2;
> + }
>
> if (Tpm2HashMask == 0) {
> //
> @@ -583,9 +656,9 @@ MeasureFvImage (
> );
>
> if (!EFI_ERROR(Status)) {
> - Status = LogHashEvent (&DigestList, &TcgEventHdr, (UINT8*) &FvBlob);
> - DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by
> Tcg2Pei starts at: 0x%x\n", FvBlob.BlobBase));
> - DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by
> Tcg2Pei has the size: 0x%x\n", FvBlob.BlobLength));
> + Status = LogHashEvent (&DigestList, &TcgEventHdr, EventData);
> + DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by
> Tcg2Pei starts at: 0x%x\n", FvBase));
> + DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by
> Tcg2Pei has the size: 0x%x\n", FvLength));
> } else if (Status == EFI_DEVICE_ERROR) {
> BuildGuidHob (&gTpmErrorHobGuid,0);
> REPORT_STATUS_CODE (
> @@ -599,13 +672,13 @@ MeasureFvImage (
> //
> Status = HashLogExtendEvent (
> 0,
> - (UINT8*) (UINTN) FvBlob.BlobBase,
> - (UINTN) FvBlob.BlobLength,
> - &TcgEventHdr,
> - (UINT8*) &FvBlob
> + (UINT8*) (UINTN) FvBase, // HashData
> + (UINTN) FvLength, // HashDataLen
> + &TcgEventHdr, // EventHdr
> + EventData // EventData
> );
> - DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei starts at:
> 0x%x\n", FvBlob.BlobBase));
> - DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei has the size:
> 0x%x\n", FvBlob.BlobLength));
> + DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei starts at:
> 0x%x\n", FvBase));
> + DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei has the size:
> 0x%x\n", FvLength));
> }
>
> if (EFI_ERROR(Status)) {
> diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> index 30f985b6ea..3d361e8859 100644
> --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> @@ -54,6 +54,7 @@
> MemoryAllocationLib
> ReportStatusCodeLib
> ResetSystemLib
> + PrintLib
>
> [Guids]
> gTcgEventEntryHobGuid ## PRODUCES ##
> HOB
> @@ -74,6 +75,7 @@
>
> [Pcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString ##
> SOMETIMES_CONSUMES
> + gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision ##
> CONSUMES
> gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid ##
> CONSUMES
> gEfiSecurityPkgTokenSpaceGuid.PcdTpm2InitializationPolicy ##
> CONSUMES
> gEfiSecurityPkgTokenSpaceGuid.PcdTpm2SelfTestPolicy ##
> SOMETIMES_CONSUMES
> --
> 2.19.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52874): https://edk2.groups.io/g/devel/message/52874
Mute This Topic: https://groups.io/mt/69344973/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