public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 0/9] Need add a FSP binary measurement
@ 2020-08-06  0:33 Qi Zhang
  2020-08-06  0:33 ` [PATCH v2 1/9] MdeModulePkg/TpmMeasurementLib: Add new API to TpmMeasurmentLib Qi Zhang
                   ` (10 more replies)
  0 siblings, 11 replies; 23+ messages in thread
From: Qi Zhang @ 2020-08-06  0:33 UTC (permalink / raw)
  To: devel
  Cc: Qi Zhang, Jiewen Yao, Jian J Wang, Hao A Wu, Chasel Chiu,
	Nate DeSimone, Star Zeng

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376

The EDKII BIOS calls FSP API in FSP Wrapper Pkg.
This FSP code need to be measured into TPM.

We need add a generic module in FSP Wrapper Pkg code to measure:
1) FSP-T, FSP-M, FSP-S in API mode.
2) FSP-T in Dispatch-mode. The FSP-M and FSP-S will be reported
   as standard FV and they will be measured by TCG-PEI.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>

Jiewen Yao (8):
  MdeModulePkg/TpmMeasurementLib: Add new API to TpmMeasurmentLib.
  MdeModulePkg/NullTpmMeasurementLib: Add new API.
  SecurityPkg/DxeTpmMeasurementLib: Add new API.
  SecurityPkg/PeiTpmMeasurementLib: Add new API.
  IntelFsp2WrapperPkg/FspMeasurementLib: Add header file.
  IntelFsp2WrapperPkg/FspMeasurementLib: Add BaseFspMeasurementLib.
  IntelFsp2WraperPkg/Fsp{m|s}WrapperPeim: Add FspBin measurement.
  IntelFsp2Wrapper/dsc: Add FspTpmMeasurementLib and
    PcdFspMeasurementConfig.

Qi Zhang (1):
  SecurityPkg/Tcg2: handle PRE HASH and LOG ONLY

 .../FspmWrapperPeim/FspmWrapperPeim.c         |  90 ++++-
 .../FspmWrapperPeim/FspmWrapperPeim.inf       |  20 +-
 .../FspsWrapperPeim/FspsWrapperPeim.c         |  85 ++++-
 .../FspsWrapperPeim/FspsWrapperPeim.inf       |  27 +-
 .../Include/Library/FspMeasurementLib.h       |  39 ++
 IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec   |  17 +
 IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc   |   5 +-
 .../BaseFspMeasurementLib.inf                 |  54 +++
 .../BaseFspMeasurementLib/FspMeasurementLib.c | 349 ++++++++++++++++++
 .../Include/Library/TpmMeasurementLib.h       |  48 ++-
 .../TpmMeasurementLibNull.c                   |  61 ++-
 .../TpmMeasurementLibNull.inf                 |   6 +-
 SecurityPkg/Include/Ppi/Tcg.h                 |   5 +
 .../DxeTpmMeasurementLib.inf                  |   6 +-
 .../DxeTpmMeasurementLib/EventLogRecord.c     | 218 +++++++++++
 .../PeiTpmMeasurementLib/EventLogRecord.c     | 218 +++++++++++
 .../PeiTpmMeasurementLib.inf                  |   4 +
 SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c             |  12 +-
 18 files changed, 1233 insertions(+), 31 deletions(-)
 create mode 100644 IntelFsp2WrapperPkg/Include/Library/FspMeasurementLib.h
 create mode 100644 IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurementLib.inf
 create mode 100644 IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c
 create mode 100644 SecurityPkg/Library/DxeTpmMeasurementLib/EventLogRecord.c
 create mode 100644 SecurityPkg/Library/PeiTpmMeasurementLib/EventLogRecord.c

-- 
2.26.2.windows.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 1/9] MdeModulePkg/TpmMeasurementLib: Add new API to TpmMeasurmentLib.
  2020-08-06  0:33 [PATCH v2 0/9] Need add a FSP binary measurement Qi Zhang
@ 2020-08-06  0:33 ` Qi Zhang
  2020-08-12  1:30   ` Wang, Jian J
  2020-08-06  0:33 ` [PATCH v2 2/9] MdeModulePkg/NullTpmMeasurementLib: Add new API Qi Zhang
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Qi Zhang @ 2020-08-06  0:33 UTC (permalink / raw)
  To: devel; +Cc: Jiewen Yao, Jian J Wang, Hao A Wu, Qi Zhang

From: Jiewen Yao <jiewen.yao@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
 .../Include/Library/TpmMeasurementLib.h       | 48 ++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Include/Library/TpmMeasurementLib.h b/MdeModulePkg/Include/Library/TpmMeasurementLib.h
index ddf6723f03..5a0f97d208 100644
--- a/MdeModulePkg/Include/Library/TpmMeasurementLib.h
+++ b/MdeModulePkg/Include/Library/TpmMeasurementLib.h
@@ -1,7 +1,7 @@
 /** @file
   This library is used by other modules to measure data to TPM.
 
-Copyright (c) 2012, Intel Corporation. All rights reserved. <BR>
+Copyright (c) 2012 - 2020, Intel Corporation. All rights reserved. <BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -35,4 +35,50 @@ TpmMeasureAndLogData (
   IN UINT64             HashDataLen
   );
 
+/**
+  Mesure a FirmwareBlob.
+
+  @param[in]  PcrIndex                PCR Index.
+  @param[in]  Descrption              Description for this FirmwareBlob.
+  @param[in]  FirmwareBlobBase        Base address of this FirmwareBlob.
+  @param[in]  FirmwareBlobLength      Size in bytes of this FirmwareBlob.
+
+  @retval EFI_SUCCESS           Operation completed successfully.
+  @retval EFI_UNSUPPORTED       TPM device not available.
+  @retval EFI_OUT_OF_RESOURCES  Out of memory.
+  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
+*/
+EFI_STATUS
+EFIAPI
+MeasureFirmwareBlob (
+  IN UINT32                         PcrIndex,
+  IN CHAR8                          *Description OPTIONAL,
+  IN EFI_PHYSICAL_ADDRESS           FirmwareBlobBase,
+  IN UINT64                         FirmwareBlobLength
+  );
+
+/**
+  Mesure a HandoffTable.
+
+  @param[in]  PcrIndex                PcrIndex of the measurment.
+  @param[in]  Descrption              Description for this HandoffTable.
+  @param[in]  TableGuid               GUID of this HandoffTable.
+  @param[in]  TableAddress            Base address of this HandoffTable.
+  @param[in]  TableLength             Size in bytes of this HandoffTable.
+
+  @retval EFI_SUCCESS           Operation completed successfully.
+  @retval EFI_UNSUPPORTED       TPM device not available.
+  @retval EFI_OUT_OF_RESOURCES  Out of memory.
+  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
+*/
+EFI_STATUS
+EFIAPI
+MeasureHandoffTable (
+  IN UINT32                         PcrIndex,
+  IN CHAR8                          *Description OPTIONAL,
+  IN EFI_GUID                       *TableGuid,
+  IN VOID                           *TableAddress,
+  IN UINTN                          TableLength
+  );
+
 #endif
-- 
2.26.2.windows.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 2/9] MdeModulePkg/NullTpmMeasurementLib: Add new API.
  2020-08-06  0:33 [PATCH v2 0/9] Need add a FSP binary measurement Qi Zhang
  2020-08-06  0:33 ` [PATCH v2 1/9] MdeModulePkg/TpmMeasurementLib: Add new API to TpmMeasurmentLib Qi Zhang
@ 2020-08-06  0:33 ` Qi Zhang
  2020-08-12  1:35   ` Wang, Jian J
  2020-08-06  0:33 ` [PATCH v2 3/9] SecurityPkg/DxeTpmMeasurementLib: " Qi Zhang
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Qi Zhang @ 2020-08-06  0:33 UTC (permalink / raw)
  To: devel; +Cc: Jiewen Yao, Jian J Wang, Hao A Wu, Qi Zhang

From: Jiewen Yao <jiewen.yao@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
 .../TpmMeasurementLibNull.c                   | 61 ++++++++++++++++++-
 .../TpmMeasurementLibNull.inf                 |  6 +-
 2 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c
index b9c5b68de8..2ce38d8258 100644
--- a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c
+++ b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c
@@ -1,11 +1,13 @@
 /** @file
   This library is used by other modules to measure data to TPM.
 
-Copyright (c) 2015, Intel Corporation. All rights reserved. <BR>
+Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved. <BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
+#include <Uefi.h>
+
 /**
   Tpm measure and log data, and extend the measurement result into a specific PCR.
 
@@ -37,3 +39,60 @@ TpmMeasureAndLogData (
   //
   return EFI_SUCCESS;
 }
+
+/**
+  Mesure a FirmwareBlob.
+
+  @param[in]  Descrption              Description for this FirmwareBlob.
+  @param[in]  FirmwareBlobBase        Base address of this FirmwareBlob.
+  @param[in]  FirmwareBlobLength      Size in bytes of this FirmwareBlob.
+
+  @retval EFI_SUCCESS           Operation completed successfully.
+  @retval EFI_UNSUPPORTED       TPM device not available.
+  @retval EFI_OUT_OF_RESOURCES  Out of memory.
+  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
+*/
+EFI_STATUS
+EFIAPI
+MeasureFirmwareBlob (
+  IN UINT32                         PcrIndex,
+  IN CHAR8                          *Description OPTIONAL,
+  IN EFI_PHYSICAL_ADDRESS           FirmwareBlobBase,
+  IN UINT64                         FirmwareBlobLength
+  )
+{
+  //
+  // Do nothing, just return EFI_SUCCESS.
+  //
+  return EFI_SUCCESS;
+}
+
+/**
+  Mesure a HandoffTable.
+
+  @param[in]  PcrIndex                PcrIndex of the measurment.
+  @param[in]  Descrption              Description for this HandoffTable.
+  @param[in]  TableGuid               GUID of this HandoffTable.
+  @param[in]  TableAddress            Base address of this HandoffTable.
+  @param[in]  TableLength             Size in bytes of this HandoffTable.
+
+  @retval EFI_SUCCESS           Operation completed successfully.
+  @retval EFI_UNSUPPORTED       TPM device not available.
+  @retval EFI_OUT_OF_RESOURCES  Out of memory.
+  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
+*/
+EFI_STATUS
+EFIAPI
+MeasureHandoffTable (
+  IN UINT32                         PcrIndex,
+  IN CHAR8                          *Description OPTIONAL,
+  IN EFI_GUID                       *TableGuid,
+  IN VOID                           *TableAddress,
+  IN UINTN                          TableLength
+  )
+{
+  //
+  // Do nothing, just return EFI_SUCCESS.
+  //
+  return EFI_SUCCESS;
+}
diff --git a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
index 61abcfa2ec..1db2c0d6a7 100644
--- a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
+++ b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
@@ -1,7 +1,7 @@
 ## @file
 #  Provides NULL TPM measurement function.
 #
-# Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.<BR>
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -10,9 +10,9 @@
   INF_VERSION                    = 0x00010005
   BASE_NAME                      = TpmMeasurementLibNull
   FILE_GUID                      = 6DFD6E9F-9278-48D8-8F45-B6CFF2C2B69C
-  MODULE_TYPE                    = UEFI_DRIVER
+  MODULE_TYPE                    = BASE
   VERSION_STRING                 = 1.0
-  LIBRARY_CLASS                  = TpmMeasurementLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
+  LIBRARY_CLASS                  = TpmMeasurementLib
   MODULE_UNI_FILE                = TpmMeasurementLibNull.uni
 
 #
-- 
2.26.2.windows.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 3/9] SecurityPkg/DxeTpmMeasurementLib: Add new API.
  2020-08-06  0:33 [PATCH v2 0/9] Need add a FSP binary measurement Qi Zhang
  2020-08-06  0:33 ` [PATCH v2 1/9] MdeModulePkg/TpmMeasurementLib: Add new API to TpmMeasurmentLib Qi Zhang
  2020-08-06  0:33 ` [PATCH v2 2/9] MdeModulePkg/NullTpmMeasurementLib: Add new API Qi Zhang
@ 2020-08-06  0:33 ` Qi Zhang
  2020-08-12  2:12   ` [edk2-devel] " Wang, Jian J
  2020-08-06  0:33 ` [PATCH v2 4/9] SecurityPkg/PeiTpmMeasurementLib: " Qi Zhang
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Qi Zhang @ 2020-08-06  0:33 UTC (permalink / raw)
  To: devel; +Cc: Jiewen Yao, Jian J Wang, Qi Zhang

From: Jiewen Yao <jiewen.yao@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
 .../DxeTpmMeasurementLib.inf                  |   6 +-
 .../DxeTpmMeasurementLib/EventLogRecord.c     | 218 ++++++++++++++++++
 2 files changed, 223 insertions(+), 1 deletion(-)
 create mode 100644 SecurityPkg/Library/DxeTpmMeasurementLib/EventLogRecord.c

diff --git a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
index 7d41bc41f9..39448f8ee8 100644
--- a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
+++ b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
@@ -4,7 +4,7 @@
 #  This library provides TpmMeasureAndLogData() to measure and log data, and
 #  extend the measurement result into a specific PCR.
 #
-# Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2012 - 2020, Intel Corporation. All rights reserved.<BR>
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -26,6 +26,7 @@
 
 [Sources]
   DxeTpmMeasurementLib.c
+  EventLogRecord.c
 
 [Packages]
   MdePkg/MdePkg.dec
@@ -42,3 +43,6 @@
 [Protocols]
   gEfiTcgProtocolGuid           ## SOMETIMES_CONSUMES
   gEfiTcg2ProtocolGuid          ## SOMETIMES_CONSUMES
+
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision          ## CONSUMES
diff --git a/SecurityPkg/Library/DxeTpmMeasurementLib/EventLogRecord.c b/SecurityPkg/Library/DxeTpmMeasurementLib/EventLogRecord.c
new file mode 100644
index 0000000000..7b3726e44b
--- /dev/null
+++ b/SecurityPkg/Library/DxeTpmMeasurementLib/EventLogRecord.c
@@ -0,0 +1,218 @@
+/** @file
+  This library is used by other modules to measure data to TPM.
+
+Copyright (c) 2020, Intel Corporation. All rights reserved. <BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <PiDxe.h>
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/ReportStatusCodeLib.h>
+#include <Library/HobLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PrintLib.h>
+#include <Library/TpmMeasurementLib.h>
+
+#include <IndustryStandard/UefiTcgPlatform.h>
+
+#pragma pack (1)
+
+#define PLATFORM_FIRMWARE_BLOB_DESC "Fv(XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX)"
+typedef struct {
+  UINT8                             BlobDescriptionSize;
+  UINT8                             BlobDescription[sizeof(PLATFORM_FIRMWARE_BLOB_DESC)];
+  EFI_PHYSICAL_ADDRESS              BlobBase;
+  UINT64                            BlobLength;
+} PLATFORM_FIRMWARE_BLOB2_STRUCT;
+
+#define HANDOFF_TABLE_POINTER_DESC  "1234567890ABCDEF"
+typedef struct {
+  UINT8                             TableDescriptionSize;
+  UINT8                             TableDescription[sizeof(HANDOFF_TABLE_POINTER_DESC)];
+  UINT64                            NumberOfTables;
+  EFI_CONFIGURATION_TABLE           TableEntry[1];
+} HANDOFF_TABLE_POINTERS2_STRUCT;
+
+#pragma pack ()
+
+/**
+  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 *
+TpmMeasurementGetFvName (
+  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->Signature != EFI_FVH_SIGNATURE) {
+    return NULL;
+  }
+  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;
+}
+
+/**
+  Mesure a FirmwareBlob.
+
+  @param[in]  PcrIndex                PcrIndex of the measurment.
+  @param[in]  Descrption              Description for this FirmwareBlob.
+  @param[in]  FirmwareBlobBase        Base address of this FirmwareBlob.
+  @param[in]  FirmwareBlobLength      Size in bytes of this FirmwareBlob.
+
+  @retval EFI_SUCCESS           Operation completed successfully.
+  @retval EFI_UNSUPPORTED       TPM device not available.
+  @retval EFI_OUT_OF_RESOURCES  Out of memory.
+  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
+*/
+EFI_STATUS
+EFIAPI
+MeasureFirmwareBlob (
+  IN UINT32                         PcrIndex,
+  IN CHAR8                          *Description OPTIONAL,
+  IN EFI_PHYSICAL_ADDRESS           FirmwareBlobBase,
+  IN UINT64                         FirmwareBlobLength
+  )
+{
+  EFI_PLATFORM_FIRMWARE_BLOB        FvBlob;
+  PLATFORM_FIRMWARE_BLOB2_STRUCT    FvBlob2;
+  VOID                              *FvName;
+  UINT32                            EventType;
+  VOID                              *EventLog;
+  UINT32                            EventLogSize;
+  EFI_STATUS                        Status;
+
+  FvName = TpmMeasurementGetFvName (FirmwareBlobBase, FirmwareBlobLength);
+
+  if (((Description != NULL) || (FvName != NULL)) &&
+      (PcdGet32(PcdTcgPfpMeasurementRevision) >= TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105)) {
+    ZeroMem (&FvBlob2, sizeof(FvBlob2));
+    if (Description != NULL) {
+      AsciiSPrint((CHAR8*)FvBlob2.BlobDescription, sizeof(FvBlob2.BlobDescription), "%a", Description);
+    } else {
+      AsciiSPrint((CHAR8*)FvBlob2.BlobDescription, sizeof(FvBlob2.BlobDescription), "Fv(%g)", FvName);
+    }
+
+    FvBlob2.BlobDescriptionSize = sizeof(FvBlob2.BlobDescription);
+    FvBlob2.BlobBase = FirmwareBlobBase;
+    FvBlob2.BlobLength = FirmwareBlobLength;
+
+    EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB2;
+    EventLog = &FvBlob2;
+    EventLogSize = sizeof(FvBlob2);
+  } else {
+    FvBlob.BlobBase = FirmwareBlobBase;
+    FvBlob.BlobLength = FirmwareBlobLength;
+
+    EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB;
+    EventLog = &FvBlob;
+    EventLogSize = sizeof(FvBlob);
+  }
+
+  Status = TpmMeasureAndLogData (
+             PcrIndex,
+             EventType,
+             EventLog,
+             EventLogSize,
+             (VOID*)(UINTN)FirmwareBlobBase,
+             FirmwareBlobLength
+             );
+
+  return Status;
+}
+
+/**
+  Mesure a HandoffTable.
+
+  @param[in]  PcrIndex                PcrIndex of the measurment.
+  @param[in]  Descrption              Description for this HandoffTable.
+  @param[in]  TableGuid               GUID of this HandoffTable.
+  @param[in]  TableAddress            Base address of this HandoffTable.
+  @param[in]  TableLength             Size in bytes of this HandoffTable.
+
+  @retval EFI_SUCCESS           Operation completed successfully.
+  @retval EFI_UNSUPPORTED       TPM device not available.
+  @retval EFI_OUT_OF_RESOURCES  Out of memory.
+  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
+*/
+EFI_STATUS
+EFIAPI
+MeasureHandoffTable (
+  IN UINT32                         PcrIndex,
+  IN CHAR8                          *Description OPTIONAL,
+  IN EFI_GUID                       *TableGuid,
+  IN VOID                           *TableAddress,
+  IN UINTN                          TableLength
+  )
+{
+  EFI_HANDOFF_TABLE_POINTERS        HandoffTables;
+  HANDOFF_TABLE_POINTERS2_STRUCT    HandoffTables2;
+  UINT32                            EventType;
+  VOID                              *EventLog;
+  UINT32                            EventLogSize;
+  EFI_STATUS                        Status;
+
+  if ((Description != NULL) &&
+      (PcdGet32(PcdTcgPfpMeasurementRevision) >= TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105)) {
+    ZeroMem (&HandoffTables2, sizeof(HandoffTables2));
+    AsciiSPrint((CHAR8*)HandoffTables2.TableDescription, sizeof(HandoffTables2.TableDescription), "%a", Description);
+
+    HandoffTables2.TableDescriptionSize = sizeof(HandoffTables2.TableDescription);
+    HandoffTables2.NumberOfTables = 1;
+    CopyGuid (&(HandoffTables2.TableEntry[0].VendorGuid), TableGuid);
+    HandoffTables2.TableEntry[0].VendorTable = TableAddress;
+
+    EventType = EV_EFI_HANDOFF_TABLES2;
+    EventLog = &HandoffTables2;
+    EventLogSize = sizeof(HandoffTables2);
+  } else {
+    HandoffTables.NumberOfTables = 1;
+    CopyGuid (&(HandoffTables.TableEntry[0].VendorGuid), TableGuid);
+    HandoffTables.TableEntry[0].VendorTable = TableAddress;
+
+    EventType = EV_EFI_HANDOFF_TABLES;
+    EventLog = &HandoffTables;
+    EventLogSize = sizeof(HandoffTables);
+  }
+
+  Status = TpmMeasureAndLogData (
+             PcrIndex,
+             EventType,
+             EventLog,
+             EventLogSize,
+             TableAddress,
+             TableLength
+             );
+  return Status;
+}
-- 
2.26.2.windows.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 4/9] SecurityPkg/PeiTpmMeasurementLib: Add new API.
  2020-08-06  0:33 [PATCH v2 0/9] Need add a FSP binary measurement Qi Zhang
                   ` (2 preceding siblings ...)
  2020-08-06  0:33 ` [PATCH v2 3/9] SecurityPkg/DxeTpmMeasurementLib: " Qi Zhang
@ 2020-08-06  0:33 ` Qi Zhang
  2020-08-12  2:14   ` Wang, Jian J
  2020-08-06  0:33 ` [PATCH v2 5/9] IntelFsp2WrapperPkg/FspMeasurementLib: Add header file Qi Zhang
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Qi Zhang @ 2020-08-06  0:33 UTC (permalink / raw)
  To: devel; +Cc: Jiewen Yao, Jian J Wang, Qi Zhang

From: Jiewen Yao <jiewen.yao@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
 .../PeiTpmMeasurementLib/EventLogRecord.c     | 218 ++++++++++++++++++
 .../PeiTpmMeasurementLib.inf                  |   4 +
 2 files changed, 222 insertions(+)
 create mode 100644 SecurityPkg/Library/PeiTpmMeasurementLib/EventLogRecord.c

diff --git a/SecurityPkg/Library/PeiTpmMeasurementLib/EventLogRecord.c b/SecurityPkg/Library/PeiTpmMeasurementLib/EventLogRecord.c
new file mode 100644
index 0000000000..cececdf7b2
--- /dev/null
+++ b/SecurityPkg/Library/PeiTpmMeasurementLib/EventLogRecord.c
@@ -0,0 +1,218 @@
+/** @file
+  This library is used by other modules to measure data to TPM.
+
+Copyright (c) 2020, Intel Corporation. All rights reserved. <BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <PiPei.h>
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/ReportStatusCodeLib.h>
+#include <Library/HobLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PrintLib.h>
+#include <Library/TpmMeasurementLib.h>
+
+#include <IndustryStandard/UefiTcgPlatform.h>
+
+#pragma pack (1)
+
+#define PLATFORM_FIRMWARE_BLOB_DESC "Fv(XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX)"
+typedef struct {
+  UINT8                             BlobDescriptionSize;
+  UINT8                             BlobDescription[sizeof(PLATFORM_FIRMWARE_BLOB_DESC)];
+  EFI_PHYSICAL_ADDRESS              BlobBase;
+  UINT64                            BlobLength;
+} PLATFORM_FIRMWARE_BLOB2_STRUCT;
+
+#define HANDOFF_TABLE_POINTER_DESC  "1234567890ABCDEF"
+typedef struct {
+  UINT8                             TableDescriptionSize;
+  UINT8                             TableDescription[sizeof(HANDOFF_TABLE_POINTER_DESC)];
+  UINT64                            NumberOfTables;
+  EFI_CONFIGURATION_TABLE           TableEntry[1];
+} HANDOFF_TABLE_POINTERS2_STRUCT;
+
+#pragma pack ()
+
+/**
+  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 *
+TpmMeasurementGetFvName (
+  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->Signature != EFI_FVH_SIGNATURE) {
+    return NULL;
+  }
+  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;
+}
+
+/**
+  Mesure a FirmwareBlob.
+
+  @param[in]  PcrIndex                PcrIndex of the measurment.
+  @param[in]  Descrption              Description for this FirmwareBlob.
+  @param[in]  FirmwareBlobBase        Base address of this FirmwareBlob.
+  @param[in]  FirmwareBlobLength      Size in bytes of this FirmwareBlob.
+
+  @retval EFI_SUCCESS           Operation completed successfully.
+  @retval EFI_UNSUPPORTED       TPM device not available.
+  @retval EFI_OUT_OF_RESOURCES  Out of memory.
+  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
+*/
+EFI_STATUS
+EFIAPI
+MeasureFirmwareBlob (
+  IN UINT32                         PcrIndex,
+  IN CHAR8                          *Description OPTIONAL,
+  IN EFI_PHYSICAL_ADDRESS           FirmwareBlobBase,
+  IN UINT64                         FirmwareBlobLength
+  )
+{
+  EFI_PLATFORM_FIRMWARE_BLOB        FvBlob;
+  PLATFORM_FIRMWARE_BLOB2_STRUCT    FvBlob2;
+  VOID                              *FvName;
+  UINT32                            EventType;
+  VOID                              *EventLog;
+  UINT32                            EventLogSize;
+  EFI_STATUS                        Status;
+
+  FvName = TpmMeasurementGetFvName (FirmwareBlobBase, FirmwareBlobLength);
+
+  if (((Description != NULL) || (FvName != NULL)) &&
+      (PcdGet32(PcdTcgPfpMeasurementRevision) >= TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105)) {
+    ZeroMem (&FvBlob2, sizeof(FvBlob2));
+    if (Description != NULL) {
+      AsciiSPrint((CHAR8*)FvBlob2.BlobDescription, sizeof(FvBlob2.BlobDescription), "%a", Description);
+    } else {
+      AsciiSPrint((CHAR8*)FvBlob2.BlobDescription, sizeof(FvBlob2.BlobDescription), "Fv(%g)", FvName);
+    }
+
+    FvBlob2.BlobDescriptionSize = sizeof(FvBlob2.BlobDescription);
+    FvBlob2.BlobBase = FirmwareBlobBase;
+    FvBlob2.BlobLength = FirmwareBlobLength;
+
+    EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB2;
+    EventLog = &FvBlob2;
+    EventLogSize = sizeof(FvBlob2);
+  } else {
+    FvBlob.BlobBase = FirmwareBlobBase;
+    FvBlob.BlobLength = FirmwareBlobLength;
+
+    EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB;
+    EventLog = &FvBlob;
+    EventLogSize = sizeof(FvBlob);
+  }
+
+  Status = TpmMeasureAndLogData (
+             PcrIndex,
+             EventType,
+             EventLog,
+             EventLogSize,
+             (VOID*)(UINTN)FirmwareBlobBase,
+             FirmwareBlobLength
+             );
+
+  return Status;
+}
+
+/**
+  Mesure a HandoffTable.
+
+  @param[in]  PcrIndex                PcrIndex of the measurment.
+  @param[in]  Descrption              Description for this HandoffTable.
+  @param[in]  TableGuid               GUID of this HandoffTable.
+  @param[in]  TableAddress            Base address of this HandoffTable.
+  @param[in]  TableLength             Size in bytes of this HandoffTable.
+
+  @retval EFI_SUCCESS           Operation completed successfully.
+  @retval EFI_UNSUPPORTED       TPM device not available.
+  @retval EFI_OUT_OF_RESOURCES  Out of memory.
+  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
+*/
+EFI_STATUS
+EFIAPI
+MeasureHandoffTable (
+  IN UINT32                         PcrIndex,
+  IN CHAR8                          *Description OPTIONAL,
+  IN EFI_GUID                       *TableGuid,
+  IN VOID                           *TableAddress,
+  IN UINTN                          TableLength
+  )
+{
+  EFI_HANDOFF_TABLE_POINTERS        HandoffTables;
+  HANDOFF_TABLE_POINTERS2_STRUCT    HandoffTables2;
+  UINT32                            EventType;
+  VOID                              *EventLog;
+  UINT32                            EventLogSize;
+  EFI_STATUS                        Status;
+
+  if ((Description != NULL) &&
+      (PcdGet32(PcdTcgPfpMeasurementRevision) >= TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105)) {
+    ZeroMem (&HandoffTables2, sizeof(HandoffTables2));
+    AsciiSPrint((CHAR8*)HandoffTables2.TableDescription, sizeof(HandoffTables2.TableDescription), "%a", Description);
+
+    HandoffTables2.TableDescriptionSize = sizeof(HandoffTables2.TableDescription);
+    HandoffTables2.NumberOfTables = 1;
+    CopyGuid (&(HandoffTables2.TableEntry[0].VendorGuid), TableGuid);
+    HandoffTables2.TableEntry[0].VendorTable = TableAddress;
+
+    EventType = EV_EFI_HANDOFF_TABLES2;
+    EventLog = &HandoffTables2;
+    EventLogSize = sizeof(HandoffTables2);
+  } else {
+    HandoffTables.NumberOfTables = 1;
+    CopyGuid (&(HandoffTables.TableEntry[0].VendorGuid), TableGuid);
+    HandoffTables.TableEntry[0].VendorTable = TableAddress;
+
+    EventType = EV_EFI_HANDOFF_TABLES;
+    EventLog = &HandoffTables;
+    EventLogSize = sizeof(HandoffTables);
+  }
+
+  Status = TpmMeasureAndLogData (
+             PcrIndex,
+             EventType,
+             EventLog,
+             EventLogSize,
+             TableAddress,
+             TableLength
+             );
+  return Status;
+}
diff --git a/SecurityPkg/Library/PeiTpmMeasurementLib/PeiTpmMeasurementLib.inf b/SecurityPkg/Library/PeiTpmMeasurementLib/PeiTpmMeasurementLib.inf
index 6625d0fd01..489353af2e 100644
--- a/SecurityPkg/Library/PeiTpmMeasurementLib/PeiTpmMeasurementLib.inf
+++ b/SecurityPkg/Library/PeiTpmMeasurementLib/PeiTpmMeasurementLib.inf
@@ -26,6 +26,7 @@
 
 [Sources]
   PeiTpmMeasurementLib.c
+  EventLogRecord.c
 
 [Packages]
   MdePkg/MdePkg.dec
@@ -45,6 +46,9 @@
 [Ppis]
   gEdkiiTcgPpiGuid                                                     ## CONSUMES
 
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision          ## CONSUMES
+
 [Depex]
   gEfiPeiMasterBootModePpiGuid AND
   gEfiTpmDeviceSelectedGuid
-- 
2.26.2.windows.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 5/9] IntelFsp2WrapperPkg/FspMeasurementLib: Add header file.
  2020-08-06  0:33 [PATCH v2 0/9] Need add a FSP binary measurement Qi Zhang
                   ` (3 preceding siblings ...)
  2020-08-06  0:33 ` [PATCH v2 4/9] SecurityPkg/PeiTpmMeasurementLib: " Qi Zhang
@ 2020-08-06  0:33 ` Qi Zhang
  2020-08-12  2:15   ` [edk2-devel] " Wang, Jian J
  2020-08-06  0:33 ` [PATCH v2 6/9] IntelFsp2WrapperPkg/FspMeasurementLib: Add BaseFspMeasurementLib Qi Zhang
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Qi Zhang @ 2020-08-06  0:33 UTC (permalink / raw)
  To: devel; +Cc: Jiewen Yao, Chasel Chiu, Nate DeSimone, Star Zeng, Qi Zhang

From: Jiewen Yao <jiewen.yao@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
 .../Include/Library/FspMeasurementLib.h       | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 IntelFsp2WrapperPkg/Include/Library/FspMeasurementLib.h

diff --git a/IntelFsp2WrapperPkg/Include/Library/FspMeasurementLib.h b/IntelFsp2WrapperPkg/Include/Library/FspMeasurementLib.h
new file mode 100644
index 0000000000..4ab40420ad
--- /dev/null
+++ b/IntelFsp2WrapperPkg/Include/Library/FspMeasurementLib.h
@@ -0,0 +1,39 @@
+/** @file
+  This library is used by FSP modules to measure data to TPM.
+
+Copyright (c) 2020, Intel Corporation. All rights reserved. <BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _FSP_MEASUREMENT_LIB_H_
+#define _FSP_MEASUREMENT_LIB_H_
+
+#define FSP_MEASURE_FSP       BIT0
+#define FSP_MEASURE_FSPT      BIT1
+#define FSP_MEASURE_FSPM      BIT2
+#define FSP_MEASURE_FSPS      BIT3
+#define FSP_MEASURE_FSPUPD    BIT31
+
+/**
+  Mesure a FSP FirmwareBlob.
+
+  @param[in]  PcrIndex                PCR Index.
+  @param[in]  Descrption              Description for this FirmwareBlob.
+  @param[in]  FirmwareBlobBase        Base address of this FirmwareBlob.
+  @param[in]  FirmwareBlobLength      Size in bytes of this FirmwareBlob.
+
+  @retval EFI_SUCCESS           Operation completed successfully.
+  @retval EFI_UNSUPPORTED       TPM device not available.
+  @retval EFI_OUT_OF_RESOURCES  Out of memory.
+  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
+*/
+EFI_STATUS
+EFIAPI
+MeasureFspFirmwareBlob (
+  IN UINT32                         PcrIndex,
+  IN CHAR8                          *Description OPTIONAL,
+  IN EFI_PHYSICAL_ADDRESS           FirmwareBlobBase,
+  IN UINT64                         FirmwareBlobLength
+  );
+#endif
-- 
2.26.2.windows.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 6/9] IntelFsp2WrapperPkg/FspMeasurementLib: Add BaseFspMeasurementLib.
  2020-08-06  0:33 [PATCH v2 0/9] Need add a FSP binary measurement Qi Zhang
                   ` (4 preceding siblings ...)
  2020-08-06  0:33 ` [PATCH v2 5/9] IntelFsp2WrapperPkg/FspMeasurementLib: Add header file Qi Zhang
@ 2020-08-06  0:33 ` Qi Zhang
  2020-08-06  1:09   ` Chiu, Chasel
  2020-08-12  2:48   ` [edk2-devel] " Wang, Jian J
  2020-08-06  0:33 ` [PATCH v2 7/9] IntelFsp2WraperPkg/Fsp{m|s}WrapperPeim: Add FspBin measurement Qi Zhang
                   ` (4 subsequent siblings)
  10 siblings, 2 replies; 23+ messages in thread
From: Qi Zhang @ 2020-08-06  0:33 UTC (permalink / raw)
  To: devel; +Cc: Jiewen Yao, Chasel Chiu, Nate DeSimone, Star Zeng, Qi Zhang

From: Jiewen Yao <jiewen.yao@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
 .../BaseFspMeasurementLib.inf                 |  54 +++
 .../BaseFspMeasurementLib/FspMeasurementLib.c | 349 ++++++++++++++++++
 2 files changed, 403 insertions(+)
 create mode 100644 IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurementLib.inf
 create mode 100644 IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c

diff --git a/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurementLib.inf b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurementLib.inf
new file mode 100644
index 0000000000..d30168117d
--- /dev/null
+++ b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurementLib.inf
@@ -0,0 +1,54 @@
+## @file
+#  Provides FSP measurement functions.
+#
+#  This library provides MeasureFspFirmwareBlob() to measure FSP binary.
+#
+# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = FspMeasurementLib
+  FILE_GUID                      = 9A62C49D-C45A-4322-9F3C-45958DF0056B
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = FspMeasurementLib
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64
+#
+
+[Sources]
+  FspMeasurementLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  SecurityPkg/SecurityPkg.dec
+  IntelFsp2Pkg/IntelFsp2Pkg.dec
+  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  DebugLib
+  PrintLib
+  PcdLib
+  PeiServicesLib
+  PeiServicesTablePointerLib
+  FspWrapperApiLib
+  TpmMeasurementLib
+  HashLib
+
+[Ppis]
+  gEdkiiTcgPpiGuid                                                   ## CONSUMES
+
+[Pcd]
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig            ## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress                 ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision        ## CONSUMES
+
diff --git a/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c
new file mode 100644
index 0000000000..316570cd2c
--- /dev/null
+++ b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c
@@ -0,0 +1,349 @@
+/** @file
+  This library is used by FSP modules to measure data to TPM.
+
+Copyright (c) 2020, Intel Corporation. All rights reserved. <BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <PiPei.h>
+#include <Uefi.h>
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/PeiServicesLib.h>
+#include <Library/PeiServicesTablePointerLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PrintLib.h>
+#include <Library/DebugLib.h>
+#include <Library/FspWrapperApiLib.h>
+#include <Library/TpmMeasurementLib.h>
+#include <Library/FspMeasurementLib.h>
+#include <Library/HashLib.h>
+
+#include <Ppi/Tcg.h>
+#include <IndustryStandard/UefiTcgPlatform.h>
+
+#pragma pack (1)
+
+#define PLATFORM_FIRMWARE_BLOB_DESC "Fv(XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX)"
+typedef struct {
+  UINT8                             BlobDescriptionSize;
+  UINT8                             BlobDescription[sizeof(PLATFORM_FIRMWARE_BLOB_DESC)];
+  EFI_PHYSICAL_ADDRESS              BlobBase;
+  UINT64                            BlobLength;
+} PLATFORM_FIRMWARE_BLOB2_STRUCT;
+
+#define HANDOFF_TABLE_POINTER_DESC  "1234567890ABCDEF"
+typedef struct {
+  UINT8                             TableDescriptionSize;
+  UINT8                             TableDescription[sizeof(HANDOFF_TABLE_POINTER_DESC)];
+  UINT64                            NumberOfTables;
+  EFI_CONFIGURATION_TABLE           TableEntry[1];
+} HANDOFF_TABLE_POINTERS2_STRUCT;
+
+#pragma pack ()
+
+/**
+  Tpm measure and log data, and extend the measurement result into a specific PCR.
+
+  @param[in]  PcrIndex         PCR Index.
+  @param[in]  EventType        Event type.
+  @param[in]  EventLog         Measurement event log.
+  @param[in]  LogLen           Event log length in bytes.
+  @param[in]  HashData         The start of the data buffer to be hashed, extended.
+  @param[in]  HashDataLen      The length, in bytes, of the buffer referenced by HashData
+  @param[in]  Flags            Bitmap providing additional information.
+
+  @retval EFI_SUCCESS           Operation completed successfully.
+  @retval EFI_UNSUPPORTED       TPM device not available.
+  @retval EFI_OUT_OF_RESOURCES  Out of memory.
+  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
+**/
+EFI_STATUS
+EFIAPI
+TpmMeasureAndLogDataWithFlags (
+  IN UINT32             PcrIndex,
+  IN UINT32             EventType,
+  IN VOID               *EventLog,
+  IN UINT32             LogLen,
+  IN VOID               *HashData,
+  IN UINT64             HashDataLen,
+  IN UINT64             Flags
+  )
+{
+  EFI_STATUS                Status;
+  EDKII_TCG_PPI             *TcgPpi;
+  TCG_PCR_EVENT_HDR         TcgEventHdr;
+
+  Status = PeiServicesLocatePpi(
+             &gEdkiiTcgPpiGuid,
+             0,
+             NULL,
+             (VOID**)&TcgPpi
+             );
+  if (EFI_ERROR(Status)) {
+    return Status;
+  }
+
+  TcgEventHdr.PCRIndex  = PcrIndex;
+  TcgEventHdr.EventType = EventType;
+  TcgEventHdr.EventSize = LogLen;
+
+  Status = TcgPpi->HashLogExtendEvent (
+                     TcgPpi,
+                     Flags,
+                     HashData,
+                     (UINTN)HashDataLen,
+                     &TcgEventHdr,
+                     EventLog
+                     );
+  return Status;
+}
+
+/**
+  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
+**/
+STATIC
+VOID *
+TpmMeasurementGetFvName (
+  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->Signature != EFI_FVH_SIGNATURE) {
+    return NULL;
+  }
+  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;
+}
+
+/**
+  Mesure a FSP FirmwareBlob.
+
+  @param[in]  Descrption              Description for this FirmwareBlob.
+  @param[in]  FirmwareBlobBase        Base address of this FirmwareBlob.
+  @param[in]  FirmwareBlobLength      Size in bytes of this FirmwareBlob.
+  @param[in]  CfgRegionOffset         Configuration region offset in bytes.
+  @param[in]  CfgRegionSize           Configuration region in bytes.
+
+  @retval EFI_SUCCESS           Operation completed successfully.
+  @retval EFI_UNSUPPORTED       TPM device not available.
+  @retval EFI_OUT_OF_RESOURCES  Out of memory.
+  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
+*/
+STATIC
+EFI_STATUS
+EFIAPI
+MeasureFspFirmwareBlobWithCfg (
+  IN CHAR8                          *Description OPTIONAL,
+  IN EFI_PHYSICAL_ADDRESS           FirmwareBlobBase,
+  IN UINT64                         FirmwareBlobLength,
+  IN UINT32                         CfgRegionOffset,
+  IN UINT32                         CfgRegionSize
+  )
+{
+  EFI_PLATFORM_FIRMWARE_BLOB        FvBlob, UPDBlob;
+  PLATFORM_FIRMWARE_BLOB2_STRUCT    FvBlob2, UPDBlob2;
+  VOID                              *FvName;
+  UINT32                            FvEventType;
+  VOID                              *FvEventLog, *UPDEventLog;
+  UINT32                            FvEventLogSize, UPDEventLogSize;
+  EFI_STATUS                        Status;
+  HASH_HANDLE                       HashHandle;
+  UINT8                             *HashBase;
+  UINTN                             HashSize;
+  TPML_DIGEST_VALUES                DigestList;
+
+  FvName = TpmMeasurementGetFvName (FirmwareBlobBase, FirmwareBlobLength);
+
+  if (((Description != NULL) || (FvName != NULL)) &&
+      (PcdGet32(PcdTcgPfpMeasurementRevision) >= TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105)) {
+    ZeroMem (&FvBlob2, sizeof(FvBlob2));
+    ZeroMem (&UPDBlob2, sizeof(UPDBlob2));
+    if (Description != NULL) {
+      AsciiSPrint((CHAR8*)FvBlob2.BlobDescription, sizeof(FvBlob2.BlobDescription), "%a", Description);
+      AsciiSPrint((CHAR8*)UPDBlob2.BlobDescription, sizeof(UPDBlob2.BlobDescription), "%aUDP", Description);
+     } else {
+      AsciiSPrint((CHAR8*)FvBlob2.BlobDescription, sizeof(FvBlob2.BlobDescription), "Fv(%g)", FvName);
+      AsciiSPrint((CHAR8*)UPDBlob2.BlobDescription, sizeof(UPDBlob2.BlobDescription), "(%g)UDP", FvName);
+    }
+
+    FvBlob2.BlobDescriptionSize = sizeof(FvBlob2.BlobDescription);
+    FvBlob2.BlobBase = FirmwareBlobBase;
+    FvBlob2.BlobLength = FirmwareBlobLength;
+    FvEventType = EV_EFI_PLATFORM_FIRMWARE_BLOB2;
+    FvEventLog = &FvBlob2;
+    FvEventLogSize = sizeof(FvBlob2);
+
+    UPDBlob2.BlobDescriptionSize = sizeof(UPDBlob2.BlobDescription);
+    UPDBlob2.BlobBase = CfgRegionOffset;
+    UPDBlob2.BlobLength = CfgRegionSize;
+    UPDEventLog = &UPDBlob2;
+    UPDEventLogSize = sizeof(UPDBlob2);
+  } else {
+    FvBlob.BlobBase = FirmwareBlobBase;
+    FvBlob.BlobLength = FirmwareBlobLength;
+    FvEventType = EV_EFI_PLATFORM_FIRMWARE_BLOB;
+    FvEventLog = &FvBlob;
+    FvEventLogSize = sizeof(FvBlob);
+
+    UPDBlob.BlobBase = CfgRegionOffset;
+    UPDBlob.BlobLength = CfgRegionSize;
+    UPDEventLog = &UPDBlob;
+    UPDEventLogSize = sizeof(UPDBlob);
+  }
+
+  // Initialize a SHA hash context.
+  Status = HashStart (&HashHandle);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "HashStart failed - %r\n", Status));
+    return Status;
+  }
+
+  // Hash FSP binary before UDP
+  HashBase = (UINT8 *) (UINTN) FirmwareBlobBase;
+  HashSize = (UINTN) CfgRegionOffset;
+  Status = HashUpdate (HashHandle, HashBase, HashSize);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "HashUpdate failed - %r\n", Status));
+    return Status;
+  }
+
+  // Hash FSP binary after UDP
+  HashBase = (UINT8 *) (UINTN) FirmwareBlobBase + CfgRegionOffset + CfgRegionSize;
+  HashSize = (UINTN)(FirmwareBlobLength - CfgRegionOffset - CfgRegionSize);
+  Status = HashUpdate (HashHandle, HashBase, HashSize);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "HashUpdate failed - %r\n", Status));
+    return Status;
+  }
+
+  // Finalize the SHA hash.
+  Status = HashCompleteAndExtend (HashHandle, 0, NULL, 0, &DigestList);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "HashCompleteAndExtend failed - %r\n", Status));
+    return Status;
+  }
+
+  Status = TpmMeasureAndLogDataWithFlags (
+             0,
+             FvEventType,
+             FvEventLog,
+             FvEventLogSize,
+             (UINT8 *) &DigestList,
+             (UINTN) sizeof(DigestList),
+             EDKII_TCG_PRE_HASH_LOG_ONLY
+             );
+
+  Status = TpmMeasureAndLogData (
+             1,
+             EV_PLATFORM_CONFIG_FLAGS,
+             UPDEventLog,
+             UPDEventLogSize,
+             (UINT8 *) (UINTN) FirmwareBlobBase + CfgRegionOffset,
+             CfgRegionSize
+             );
+
+  return Status;
+}
+
+FSP_INFO_HEADER *
+EFIAPI
+mFspFindFspHeader (
+  IN EFI_PHYSICAL_ADDRESS  FlashFvFspBase
+  )
+{
+  UINT8 *CheckPointer;
+
+  CheckPointer = (UINT8 *) (UINTN) FlashFvFspBase;
+
+  if (((EFI_FIRMWARE_VOLUME_HEADER *)CheckPointer)->Signature != EFI_FVH_SIGNATURE) {
+    return NULL;
+  }
+
+  if (((EFI_FIRMWARE_VOLUME_HEADER *)CheckPointer)->ExtHeaderOffset != 0) {
+    CheckPointer = CheckPointer + ((EFI_FIRMWARE_VOLUME_HEADER *)CheckPointer)->ExtHeaderOffset;
+    CheckPointer = CheckPointer + ((EFI_FIRMWARE_VOLUME_EXT_HEADER *)CheckPointer)->ExtHeaderSize;
+    CheckPointer = (UINT8 *) ALIGN_POINTER (CheckPointer, 8);
+  } else {
+    CheckPointer = CheckPointer + ((EFI_FIRMWARE_VOLUME_HEADER *)CheckPointer)->HeaderLength;
+  }
+
+
+  CheckPointer = CheckPointer + sizeof (EFI_FFS_FILE_HEADER);
+
+  if (((EFI_RAW_SECTION *)CheckPointer)->Type != EFI_SECTION_RAW) {
+    return NULL;
+  }
+
+  CheckPointer = CheckPointer + sizeof (EFI_RAW_SECTION);
+
+  return (FSP_INFO_HEADER *)CheckPointer;
+}
+/**
+  Mesure a FSP FirmwareBlob.
+
+  @param[in]  PcrIndex                PCR Index.
+  @param[in]  Descrption              Description for this FirmwareBlob.
+  @param[in]  FirmwareBlobBase        Base address of this FirmwareBlob.
+  @param[in]  FirmwareBlobLength      Size in bytes of this FirmwareBlob.
+
+  @retval EFI_SUCCESS           Operation completed successfully.
+  @retval EFI_UNSUPPORTED       TPM device not available.
+  @retval EFI_OUT_OF_RESOURCES  Out of memory.
+  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
+*/
+EFI_STATUS
+EFIAPI
+MeasureFspFirmwareBlob (
+  IN UINT32                         PcrIndex,
+  IN CHAR8                          *Description OPTIONAL,
+  IN EFI_PHYSICAL_ADDRESS           FirmwareBlobBase,
+  IN UINT64                         FirmwareBlobLength
+  )
+{
+  UINT32           FspMeasureMask;
+  FSP_INFO_HEADER  *FspHeaderPtr;
+
+  FspMeasureMask = PcdGet32 (PcdFspMeasurementConfig);
+  if (FspMeasureMask & FSP_MEASURE_FSPUPD) {
+    FspHeaderPtr = (FSP_INFO_HEADER *) mFspFindFspHeader (FirmwareBlobBase);
+    if (FspHeaderPtr == NULL) {
+      return MeasureFirmwareBlob (PcrIndex, Description, FirmwareBlobBase, FirmwareBlobLength);;
+    }
+    return MeasureFspFirmwareBlobWithCfg(Description, FirmwareBlobBase, FirmwareBlobLength,
+                                         FspHeaderPtr->CfgRegionOffset, FspHeaderPtr->CfgRegionSize);
+  } else {
+    return MeasureFirmwareBlob (PcrIndex, Description, FirmwareBlobBase, FirmwareBlobLength);
+  }
+}
+
-- 
2.26.2.windows.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 7/9] IntelFsp2WraperPkg/Fsp{m|s}WrapperPeim: Add FspBin measurement.
  2020-08-06  0:33 [PATCH v2 0/9] Need add a FSP binary measurement Qi Zhang
                   ` (5 preceding siblings ...)
  2020-08-06  0:33 ` [PATCH v2 6/9] IntelFsp2WrapperPkg/FspMeasurementLib: Add BaseFspMeasurementLib Qi Zhang
@ 2020-08-06  0:33 ` Qi Zhang
  2020-08-06  0:33 ` [PATCH v2 8/9] IntelFsp2Wrapper/dsc: Add FspTpmMeasurementLib and PcdFspMeasurementConfig Qi Zhang
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Qi Zhang @ 2020-08-06  0:33 UTC (permalink / raw)
  To: devel; +Cc: Jiewen Yao, Chasel Chiu, Nate DeSimone, Star Zeng, Qi Zhang

From: Jiewen Yao <jiewen.yao@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
 .../FspmWrapperPeim/FspmWrapperPeim.c         | 90 ++++++++++++++++++-
 .../FspmWrapperPeim/FspmWrapperPeim.inf       | 20 +++--
 .../FspsWrapperPeim/FspsWrapperPeim.c         | 85 +++++++++++++++++-
 .../FspsWrapperPeim/FspsWrapperPeim.inf       | 27 +++---
 4 files changed, 203 insertions(+), 19 deletions(-)

diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
index 265b77ed60..f1bff46baa 100644
--- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
+++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c
@@ -3,7 +3,7 @@
   register TemporaryRamDonePpi to call TempRamExit API, and register MemoryDiscoveredPpi
   notify to call FspSiliconInit API.
 
-  Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -25,11 +25,14 @@
 #include <Library/FspWrapperPlatformLib.h>
 #include <Library/FspWrapperHobProcessLib.h>
 #include <Library/FspWrapperApiLib.h>
+#include <Library/FspMeasurementLib.h>
 
 #include <Ppi/FspSiliconInitDone.h>
 #include <Ppi/EndOfPeiPhase.h>
 #include <Ppi/MemoryDiscovered.h>
 #include <Ppi/SecPlatformInformation.h>
+#include <Ppi/Tcg.h>
+#include <Ppi/FirmwareVolumeInfoMeasurementExcluded.h>
 #include <Library/FspWrapperApiTestLib.h>
 #include <FspEas.h>
 #include <FspStatusCode.h>
@@ -147,7 +150,21 @@ FspmWrapperInit (
   VOID
   )
 {
-  EFI_STATUS           Status;
+  EFI_STATUS                                            Status;
+  EFI_PEI_FIRMWARE_VOLUME_INFO_MEASUREMENT_EXCLUDED_PPI *MeasurementExcludedFvPpi;
+  EFI_PEI_PPI_DESCRIPTOR                                *MeasurementExcludedPpiList;
+
+  MeasurementExcludedFvPpi = AllocatePool (sizeof(*MeasurementExcludedFvPpi));
+  ASSERT(MeasurementExcludedFvPpi != NULL);
+  MeasurementExcludedFvPpi->Count = 1;
+  MeasurementExcludedFvPpi->Fv[0].FvBase = PcdGet32 (PcdFspmBaseAddress);
+  MeasurementExcludedFvPpi->Fv[0].FvLength = ((EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) PcdGet32 (PcdFspmBaseAddress))->FvLength;
+
+  MeasurementExcludedPpiList = AllocatePool (sizeof(*MeasurementExcludedPpiList));
+  ASSERT(MeasurementExcludedPpiList != NULL);
+  MeasurementExcludedPpiList->Flags = EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
+  MeasurementExcludedPpiList->Guid  = &gEfiPeiFirmwareVolumeInfoMeasurementExcludedPpiGuid;
+  MeasurementExcludedPpiList->Ppi   = MeasurementExcludedFvPpi;
 
   Status = EFI_SUCCESS;
 
@@ -155,6 +172,9 @@ FspmWrapperInit (
     Status = PeiFspMemoryInit ();
     ASSERT_EFI_ERROR (Status);
   } else {
+    Status = PeiServicesInstallPpi (MeasurementExcludedPpiList);
+    ASSERT_EFI_ERROR (Status);
+
     PeiServicesInstallFvInfoPpi (
       NULL,
       (VOID *)(UINTN) PcdGet32 (PcdFspmBaseAddress),
@@ -167,6 +187,67 @@ FspmWrapperInit (
   return Status;
 }
 
+/**
+  This function is called after TCG installed PPI.
+
+  @param[in] PeiServices    Pointer to PEI Services Table.
+  @param[in] NotifyDesc     Pointer to the descriptor for the Notification event that
+                            caused this function to execute.
+  @param[in] Ppi            Pointer to the PPI data associated with this function.
+
+  @retval EFI_STATUS        Always return EFI_SUCCESS
+**/
+EFI_STATUS
+EFIAPI
+TcgPpiNotify (
+  IN EFI_PEI_SERVICES          **PeiServices,
+  IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDesc,
+  IN VOID                      *Ppi
+  );
+
+EFI_PEI_NOTIFY_DESCRIPTOR mTcgPpiNotifyDesc = {
+  (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+  &gEdkiiTcgPpiGuid,
+  TcgPpiNotify
+};
+
+/**
+  This function is called after TCG installed PPI.
+
+  @param[in] PeiServices    Pointer to PEI Services Table.
+  @param[in] NotifyDesc     Pointer to the descriptor for the Notification event that
+                            caused this function to execute.
+  @param[in] Ppi            Pointer to the PPI data associated with this function.
+
+  @retval EFI_STATUS        Always return EFI_SUCCESS
+**/
+EFI_STATUS
+EFIAPI
+TcgPpiNotify (
+  IN EFI_PEI_SERVICES          **PeiServices,
+  IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDesc,
+  IN VOID                      *Ppi
+  )
+{
+  UINT32                    FspMeasureMask;
+
+  DEBUG ((DEBUG_INFO, "TcgPpiNotify FSPM\n"));
+
+  FspMeasureMask = PcdGet32 (PcdFspMeasurementConfig);
+  if (FspMeasureMask & FSP_MEASURE_FSP) {
+    if (FspMeasureMask & FSP_MEASURE_FSPT) {
+      MeasureFspFirmwareBlob (0, "FSPT", PcdGet32(PcdFsptBaseAddress),
+                              (UINT32)((EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) PcdGet32 (PcdFsptBaseAddress))->FvLength);
+    }
+    if (FspMeasureMask & FSP_MEASURE_FSPM) {
+      MeasureFspFirmwareBlob (0, "FSPM", PcdGet32(PcdFspmBaseAddress),
+                              (UINT32)((EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) PcdGet32 (PcdFspmBaseAddress))->FvLength);
+    }
+  }
+
+  return EFI_SUCCESS;
+}
+
 /**
   This is the entrypoint of PEIM
 
@@ -182,8 +263,13 @@ FspmWrapperPeimEntryPoint (
   IN CONST EFI_PEI_SERVICES     **PeiServices
   )
 {
+  EFI_STATUS  Status;
+
   DEBUG((DEBUG_INFO, "FspmWrapperPeimEntryPoint\n"));
 
+  Status = PeiServicesNotifyPpi (&mTcgPpiNotifyDesc);
+  ASSERT_EFI_ERROR (Status);
+
   FspmWrapperInit ();
 
   return EFI_SUCCESS;
diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
index dce7ef3d0b..c3578397b6 100644
--- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
+++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
@@ -6,7 +6,7 @@
 # register TemporaryRamDonePpi to call TempRamExit API, and register MemoryDiscoveredPpi
 # notify to call FspSiliconInit API.
 #
-#  Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -44,17 +44,22 @@
   TimerLib
   FspWrapperApiLib
   FspWrapperApiTestLib
+  FspMeasurementLib
 
 [Packages]
   MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
+  SecurityPkg/SecurityPkg.dec
   IntelFsp2Pkg/IntelFsp2Pkg.dec
   IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
 
 [Pcd]
-  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress     ## CONSUMES
-  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress  ## CONSUMES
-  gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection    ## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress       ## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress    ## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFsptBaseAddress       ## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ## CONSUMES
 
 [Sources]
   FspmWrapperPeim.c
@@ -63,5 +68,10 @@
   gFspHobGuid                           ## PRODUCES ## HOB
   gFspApiPerformanceGuid                ## SOMETIMES_CONSUMES ## GUID
 
+[Ppis]
+  gEdkiiTcgPpiGuid                                       ## NOTIFY
+  gEfiPeiFirmwareVolumeInfoMeasurementExcludedPpiGuid    ## PRODUCES
+
 [Depex]
-  gEfiPeiMasterBootModePpiGuid
+  gEfiPeiMasterBootModePpiGuid AND
+  gPeiTpmInitializationDonePpiGuid
diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
index b20f0805a0..6d023b75ef 100644
--- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
+++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c
@@ -3,7 +3,7 @@
   register TemporaryRamDonePpi to call TempRamExit API, and register MemoryDiscoveredPpi
   notify to call FspSiliconInit API.
 
-  Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -24,12 +24,15 @@
 #include <Library/TimerLib.h>
 #include <Library/PerformanceLib.h>
 #include <Library/FspWrapperApiLib.h>
+#include <Library/FspMeasurementLib.h>
 
 #include <Ppi/FspSiliconInitDone.h>
 #include <Ppi/EndOfPeiPhase.h>
 #include <Ppi/MemoryDiscovered.h>
 #include <Ppi/TemporaryRamDone.h>
 #include <Ppi/SecPlatformInformation.h>
+#include <Ppi/Tcg.h>
+#include <Ppi/FirmwareVolumeInfoMeasurementExcluded.h>
 #include <Library/FspWrapperApiTestLib.h>
 #include <FspEas.h>
 #include <FspStatusCode.h>
@@ -379,7 +382,25 @@ FspsWrapperInitDispatchMode (
   VOID
   )
 {
-  EFI_STATUS           Status;
+  EFI_STATUS                                            Status;
+  EFI_PEI_FIRMWARE_VOLUME_INFO_MEASUREMENT_EXCLUDED_PPI *MeasurementExcludedFvPpi;
+  EFI_PEI_PPI_DESCRIPTOR                                *MeasurementExcludedPpiList;
+
+  MeasurementExcludedFvPpi = AllocatePool (sizeof(*MeasurementExcludedFvPpi));
+  ASSERT(MeasurementExcludedFvPpi != NULL);
+  MeasurementExcludedFvPpi->Count = 1;
+  MeasurementExcludedFvPpi->Fv[0].FvBase = PcdGet32 (PcdFspsBaseAddress);
+  MeasurementExcludedFvPpi->Fv[0].FvLength = ((EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) PcdGet32 (PcdFspsBaseAddress))->FvLength;
+
+  MeasurementExcludedPpiList = AllocatePool (sizeof(*MeasurementExcludedPpiList));
+  ASSERT(MeasurementExcludedPpiList != NULL);
+  MeasurementExcludedPpiList->Flags = EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
+  MeasurementExcludedPpiList->Guid  = &gEfiPeiFirmwareVolumeInfoMeasurementExcludedPpiGuid;
+  MeasurementExcludedPpiList->Ppi   = MeasurementExcludedFvPpi;
+
+  Status = PeiServicesInstallPpi (MeasurementExcludedPpiList);
+  ASSERT_EFI_ERROR (Status);
+
   //
   // FSP-S Wrapper running in Dispatch mode and reports FSP-S FV to PEI dispatcher.
   //
@@ -398,6 +419,61 @@ FspsWrapperInitDispatchMode (
   return Status;
 }
 
+/**
+  This function is called after TCG installed PPI.
+
+  @param[in] PeiServices    Pointer to PEI Services Table.
+  @param[in] NotifyDesc     Pointer to the descriptor for the Notification event that
+                            caused this function to execute.
+  @param[in] Ppi            Pointer to the PPI data associated with this function.
+
+  @retval EFI_STATUS        Always return EFI_SUCCESS
+**/
+EFI_STATUS
+EFIAPI
+TcgPpiNotify (
+  IN EFI_PEI_SERVICES          **PeiServices,
+  IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDesc,
+  IN VOID                      *Ppi
+  );
+
+EFI_PEI_NOTIFY_DESCRIPTOR mTcgPpiNotifyDesc = {
+  (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+  &gEdkiiTcgPpiGuid,
+  TcgPpiNotify
+};
+
+/**
+  This function is called after TCG installed PPI.
+
+  @param[in] PeiServices    Pointer to PEI Services Table.
+  @param[in] NotifyDesc     Pointer to the descriptor for the Notification event that
+                            caused this function to execute.
+  @param[in] Ppi            Pointer to the PPI data associated with this function.
+
+  @retval EFI_STATUS        Always return EFI_SUCCESS
+**/
+EFI_STATUS
+EFIAPI
+TcgPpiNotify (
+  IN EFI_PEI_SERVICES          **PeiServices,
+  IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDesc,
+  IN VOID                      *Ppi
+  )
+{
+  UINT32                    FspMeasureMask;
+
+  DEBUG ((DEBUG_INFO, "TcgPpiNotify FSPS\n"));
+
+  FspMeasureMask = PcdGet32 (PcdFspMeasurementConfig);
+  if ((FspMeasureMask & FSP_MEASURE_FSP) && (FspMeasureMask & FSP_MEASURE_FSPS)) {
+    MeasureFspFirmwareBlob (0, "FSPS", PcdGet32(PcdFspsBaseAddress),
+                            (UINT32)((EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) PcdGet32 (PcdFspsBaseAddress))->FvLength);
+  }
+
+  return EFI_SUCCESS;
+}
+
 /**
   This is the entrypoint of PEIM.
 
@@ -413,8 +489,13 @@ FspsWrapperPeimEntryPoint (
   IN CONST EFI_PEI_SERVICES     **PeiServices
   )
 {
+  EFI_STATUS  Status;
+
   DEBUG ((DEBUG_INFO, "FspsWrapperPeimEntryPoint\n"));
 
+  Status = PeiServicesNotifyPpi (&mTcgPpiNotifyDesc);
+  ASSERT_EFI_ERROR (Status);
+
   if (PcdGet8 (PcdFspModeSelection) == 1) {
     FspsWrapperInitApiMode ();
   } else {
diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
index 7da92991c8..884514747f 100644
--- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
+++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
@@ -6,7 +6,7 @@
 # register TemporaryRamDonePpi to call TempRamExit API, and register MemoryDiscoveredPpi
 # notify to call FspSiliconInit API.
 #
-#  Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -44,24 +44,30 @@
   PerformanceLib
   FspWrapperApiLib
   FspWrapperApiTestLib
+  FspMeasurementLib
 
 [Packages]
   MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
+  SecurityPkg/SecurityPkg.dec
   IntelFsp2Pkg/IntelFsp2Pkg.dec
   IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
 
 [Ppis]
-  gTopOfTemporaryRamPpiGuid             ## PRODUCES
-  gFspSiliconInitDonePpiGuid            ## PRODUCES
-  gEfiEndOfPeiSignalPpiGuid             ## PRODUCES
-  gEfiTemporaryRamDonePpiGuid           ## PRODUCES
-  gEfiPeiMemoryDiscoveredPpiGuid        ## NOTIFY
+  gTopOfTemporaryRamPpiGuid                              ## PRODUCES
+  gFspSiliconInitDonePpiGuid                             ## PRODUCES
+  gEfiEndOfPeiSignalPpiGuid                              ## PRODUCES
+  gEfiTemporaryRamDonePpiGuid                            ## PRODUCES
+  gEfiPeiMemoryDiscoveredPpiGuid                         ## NOTIFY
+  gEdkiiTcgPpiGuid                                       ## NOTIFY
+  gEfiPeiFirmwareVolumeInfoMeasurementExcludedPpiGuid    ## PRODUCES
 
 [Pcd]
-  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress     ## CONSUMES
-  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress  ## CONSUMES
-  gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection    ## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsBaseAddress       ## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspsUpdDataAddress    ## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection      ## CONSUMES
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig  ## CONSUMES
 
 [Guids]
   gFspHobGuid                           ## CONSUMES ## HOB
@@ -71,4 +77,5 @@
   FspsWrapperPeim.c
 
 [Depex]
-  gEfiPeiMemoryDiscoveredPpiGuid
+  gEfiPeiMemoryDiscoveredPpiGuid AND
+  gPeiTpmInitializationDonePpiGuid
-- 
2.26.2.windows.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 8/9] IntelFsp2Wrapper/dsc: Add FspTpmMeasurementLib and PcdFspMeasurementConfig.
  2020-08-06  0:33 [PATCH v2 0/9] Need add a FSP binary measurement Qi Zhang
                   ` (6 preceding siblings ...)
  2020-08-06  0:33 ` [PATCH v2 7/9] IntelFsp2WraperPkg/Fsp{m|s}WrapperPeim: Add FspBin measurement Qi Zhang
@ 2020-08-06  0:33 ` Qi Zhang
  2020-08-06  0:33 ` [PATCH v2 9/9] SecurityPkg/Tcg2: handle PRE HASH and LOG ONLY Qi Zhang
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Qi Zhang @ 2020-08-06  0:33 UTC (permalink / raw)
  To: devel; +Cc: Jiewen Yao, Chasel Chiu, Nate DeSimone, Star Zeng, Qi Zhang

From: Jiewen Yao <jiewen.yao@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
 IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec | 17 +++++++++++++++++
 IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc |  5 ++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
index faf2be621c..4bd3250571 100644
--- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
+++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
@@ -92,6 +92,23 @@
   #
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection|0x00000001|UINT8|0x4000000A
 
+  ## This PCD decides how FSP is measured
+  # 1) The BootGuard ACM may already measured the FSP component, such as FSPT/FSPM.
+  # We need a flag (PCD) to indicate if there is need to do such FSP measurement or NOT.
+  # 2) The FSP binary includes FSP code and FSP UPD region. The UPD region is considered
+  # as configuration block, and it may be updated by OEM by design.
+  # This flag (PCD) is to indicate if we need isolate the the UPD region from the FSP code region.
+  # BIT0: Need measure FSP. (for FSP1.x) - reserved in FSP2.
+  # BIT1: Need measure FSPT. (for FSP 2.x)
+  # BIT2: Need measure FSPM. (for FSP 2.x)
+  # BIT3: Need measure FSPS. (for FSP 2.x)
+  # BIT4~30: reserved.
+  # BIT31: Need isolate UPD region measurement.
+    #0: measure FSP[T|M|S] as one binary in one record (PCR0).
+    #1: measure FSP UPD region in one record (PCR1), the FSP code without UPD in another record (PCR0).
+  #
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig|0x0000000F|UINT32|0x4000000B
+
 [PcdsFixedAtBuild, PcdsPatchableInModule,PcdsDynamic,PcdsDynamicEx]
   #
   ## These are the base address of FSP-M/S
diff --git a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc
index cb4f69285d..5c0d509be4 100644
--- a/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc
+++ b/IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc
@@ -1,7 +1,7 @@
 ## @file
 # Provides drivers and definitions to support fsp in EDKII bios.
 #
-# Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -45,6 +45,7 @@
   # FSP Wrapper Lib
   FspWrapperApiLib|IntelFsp2WrapperPkg/Library/BaseFspWrapperApiLib/BaseFspWrapperApiLib.inf
   FspWrapperApiTestLib|IntelFsp2WrapperPkg/Library/BaseFspWrapperApiTestLibNull/BaseFspWrapperApiTestLibNull.inf
+  FspMeasurementLib|IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurementLib.inf
 
   # FSP platform sample
   FspWrapperPlatformLib|IntelFsp2WrapperPkg/Library/BaseFspWrapperPlatformLibSample/BaseFspWrapperPlatformLibSample.inf
@@ -57,6 +58,7 @@
   PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
   MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
   HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
+  TpmMeasurementLib|SecurityPkg/Library/PeiTpmMeasurementLib/PeiTpmMeasurementLib.inf
 
 [LibraryClasses.common.DXE_DRIVER]
   UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
@@ -73,6 +75,7 @@
   IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/SecFspWrapperPlatformSecLibSample.inf
   IntelFsp2WrapperPkg/Library/PeiFspWrapperHobProcessLibSample/PeiFspWrapperHobProcessLibSample.inf
   IntelFsp2WrapperPkg/Library/PeiFspWrapperApiTestLib/PeiFspWrapperApiTestLib.inf
+  IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurementLib.inf
 
   IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf
   IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
-- 
2.26.2.windows.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 9/9] SecurityPkg/Tcg2: handle PRE HASH and LOG ONLY
  2020-08-06  0:33 [PATCH v2 0/9] Need add a FSP binary measurement Qi Zhang
                   ` (7 preceding siblings ...)
  2020-08-06  0:33 ` [PATCH v2 8/9] IntelFsp2Wrapper/dsc: Add FspTpmMeasurementLib and PcdFspMeasurementConfig Qi Zhang
@ 2020-08-06  0:33 ` Qi Zhang
  2020-08-11  0:19   ` [edk2-devel] " Liming Gao
  2020-08-12  2:56   ` Wang, Jian J
  2020-08-11  2:00 ` [PATCH v2 0/9] Need add a FSP binary measurement Yao, Jiewen
  2020-08-11 16:25 ` Wang, Jian J
  10 siblings, 2 replies; 23+ messages in thread
From: Qi Zhang @ 2020-08-06  0:33 UTC (permalink / raw)
  To: devel; +Cc: Qi Zhang, Jiewen Yao, Jian J Wang, Rahul Kumar

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Qi Zhang <qi1.zhang@intel.com>
---
 SecurityPkg/Include/Ppi/Tcg.h     |  5 +++++
 SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 12 +++++++-----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/SecurityPkg/Include/Ppi/Tcg.h b/SecurityPkg/Include/Ppi/Tcg.h
index 0e943f2465..22f47f9817 100644
--- a/SecurityPkg/Include/Ppi/Tcg.h
+++ b/SecurityPkg/Include/Ppi/Tcg.h
@@ -18,6 +18,11 @@ typedef struct _EDKII_TCG_PPI EDKII_TCG_PPI;
 //
 #define EDKII_TCG_PRE_HASH  0x0000000000000001
 
+//
+// This bit is shall be set when HashData is the pre-hash digest and log only.
+//
+#define EDKII_TCG_PRE_HASH_LOG_ONLY  0x0000000000000002
+
 /**
   Tpm measure and log data, and extend the measurement result into a specific PCR.
 
diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
index 246968bb7f..b56b03746c 100644
--- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
+++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
@@ -453,13 +453,15 @@ HashLogExtendEvent (
     return EFI_DEVICE_ERROR;
   }
 
-  if(Flags & EDKII_TCG_PRE_HASH) {
+  if ((Flags & EDKII_TCG_PRE_HASH) || (Flags & EDKII_TCG_PRE_HASH_LOG_ONLY)) {
     ZeroMem (&DigestList, sizeof(DigestList));
     CopyMem (&DigestList, HashData, sizeof(DigestList));
-    Status = Tpm2PcrExtend (
-             0,
-             &DigestList
-             );
+    if (Flags & EDKII_TCG_PRE_HASH) {
+      Status = Tpm2PcrExtend (
+               NewEventHdr->PCRIndex,
+               &DigestList
+               );
+    }
   } else {
     Status = HashAndExtend (
                NewEventHdr->PCRIndex,
-- 
2.26.2.windows.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 6/9] IntelFsp2WrapperPkg/FspMeasurementLib: Add BaseFspMeasurementLib.
  2020-08-06  0:33 ` [PATCH v2 6/9] IntelFsp2WrapperPkg/FspMeasurementLib: Add BaseFspMeasurementLib Qi Zhang
@ 2020-08-06  1:09   ` Chiu, Chasel
  2020-08-12  2:48   ` [edk2-devel] " Wang, Jian J
  1 sibling, 0 replies; 23+ messages in thread
From: Chiu, Chasel @ 2020-08-06  1:09 UTC (permalink / raw)
  To: Zhang, Qi1, devel@edk2.groups.io
  Cc: Yao, Jiewen, Desimone, Nathaniel L, Zeng, Star


Hi Qi,

Please see my comments below inline.

Thanks,
Chasel


> -----Original Message-----
> From: Zhang, Qi1 <qi1.zhang@intel.com>
> Sent: Thursday, August 6, 2020 8:34 AM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Chiu, Chasel
> <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; Zhang,
> Qi1 <qi1.zhang@intel.com>
> Subject: [PATCH v2 6/9] IntelFsp2WrapperPkg/FspMeasurementLib: Add
> BaseFspMeasurementLib.
> 
> From: Jiewen Yao <jiewen.yao@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Qi Zhang <qi1.zhang@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
>  .../BaseFspMeasurementLib.inf                 |  54 +++
>  .../BaseFspMeasurementLib/FspMeasurementLib.c | 349
> ++++++++++++++++++
>  2 files changed, 403 insertions(+)
>  create mode 100644
> IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasuremen
> tLib.inf
>  create mode 100644
> IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.
> c
> 
> diff --git
> a/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurem
> entLib.inf
> b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurem
> entLib.inf
> new file mode 100644
> index 0000000000..d30168117d
> --- /dev/null
> +++
> b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurem
> entLib.inf
> @@ -0,0 +1,54 @@
> +## @file
> 
> +#  Provides FSP measurement functions.
> 
> +#
> 
> +#  This library provides MeasureFspFirmwareBlob() to measure FSP binary.
> 
> +#
> 
> +# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> 
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +#
> 
> +##
> 
> +
> 
> +[Defines]
> 
> +  INF_VERSION                    = 0x00010005
> 
> +  BASE_NAME                      = FspMeasurementLib
> 
> +  FILE_GUID                      =
> 9A62C49D-C45A-4322-9F3C-45958DF0056B
> 
> +  MODULE_TYPE                    = BASE
> 
> +  VERSION_STRING                 = 1.0
> 
> +  LIBRARY_CLASS                  = FspMeasurementLib
> 
> +
> 
> +#
> 
> +# The following information is for reference only and not required by the
> build tools.
> 
> +#
> 
> +#  VALID_ARCHITECTURES           = IA32 X64
> 
> +#
> 
> +
> 
> +[Sources]
> 
> +  FspMeasurementLib.c
> 
> +
> 
> +[Packages]
> 
> +  MdePkg/MdePkg.dec
> 
> +  MdeModulePkg/MdeModulePkg.dec
> 
> +  SecurityPkg/SecurityPkg.dec
> 
> +  IntelFsp2Pkg/IntelFsp2Pkg.dec
> 
> +  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> 
> +
> 
> +[LibraryClasses]
> 
> +  BaseLib
> 
> +  BaseMemoryLib
> 
> +  DebugLib
> 
> +  PrintLib
> 
> +  PcdLib
> 
> +  PeiServicesLib
> 
> +  PeiServicesTablePointerLib
> 
> +  FspWrapperApiLib
> 
> +  TpmMeasurementLib
> 
> +  HashLib
> 
> +
> 
> +[Ppis]
> 
> +  gEdkiiTcgPpiGuid
> ## CONSUMES
> 
> +
> 
> +[Pcd]
> 
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig
> ## CONSUMES
> 
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress
> ## CONSUMES
> 
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision
> ## CONSUMES
> 
> +
> 
> diff --git
> a/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLi
> b.c
> b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLi
> b.c
> new file mode 100644
> index 0000000000..316570cd2c
> --- /dev/null
> +++
> b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLi
> b.c
> @@ -0,0 +1,349 @@
> +/** @file
> 
> +  This library is used by FSP modules to measure data to TPM.
> 
> +
> 
> +Copyright (c) 2020, Intel Corporation. All rights reserved. <BR>
> 
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +
> 
> +**/
> 
> +
> 
> +#include <PiPei.h>
> 
> +#include <Uefi.h>
> 
> +
> 
> +#include <Library/BaseMemoryLib.h>
> 
> +#include <Library/PeiServicesLib.h>
> 
> +#include <Library/PeiServicesTablePointerLib.h>
> 
> +#include <Library/PcdLib.h>
> 
> +#include <Library/PrintLib.h>
> 
> +#include <Library/DebugLib.h>
> 
> +#include <Library/FspWrapperApiLib.h>
> 
> +#include <Library/TpmMeasurementLib.h>
> 
> +#include <Library/FspMeasurementLib.h>
> 
> +#include <Library/HashLib.h>
> 
> +
> 
> +#include <Ppi/Tcg.h>
> 
> +#include <IndustryStandard/UefiTcgPlatform.h>
> 
> +
> 
> +#pragma pack (1)
> 
> +
> 
> +#define PLATFORM_FIRMWARE_BLOB_DESC
> "Fv(XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX)"
> 
> +typedef struct {
> 
> +  UINT8                             BlobDescriptionSize;
> 
> +  UINT8
> BlobDescription[sizeof(PLATFORM_FIRMWARE_BLOB_DESC)];
> 
> +  EFI_PHYSICAL_ADDRESS              BlobBase;
> 
> +  UINT64                            BlobLength;
> 
> +} PLATFORM_FIRMWARE_BLOB2_STRUCT;
> 
> +
> 
> +#define HANDOFF_TABLE_POINTER_DESC  "1234567890ABCDEF"
> 
> +typedef struct {
> 
> +  UINT8                             TableDescriptionSize;
> 
> +  UINT8
> TableDescription[sizeof(HANDOFF_TABLE_POINTER_DESC)];
> 
> +  UINT64                            NumberOfTables;
> 
> +  EFI_CONFIGURATION_TABLE           TableEntry[1];
> 
> +} HANDOFF_TABLE_POINTERS2_STRUCT;
> 
> +
> 
> +#pragma pack ()
> 
> +
> 
> +/**
> 
> +  Tpm measure and log data, and extend the measurement result into a
> specific PCR.
> 
> +
> 
> +  @param[in]  PcrIndex         PCR Index.
> 
> +  @param[in]  EventType        Event type.
> 
> +  @param[in]  EventLog         Measurement event log.
> 
> +  @param[in]  LogLen           Event log length in bytes.
> 
> +  @param[in]  HashData         The start of the data buffer to be
> hashed, extended.
> 
> +  @param[in]  HashDataLen      The length, in bytes, of the buffer
> referenced by HashData
> 
> +  @param[in]  Flags            Bitmap providing additional
> information.
> 
> +
> 
> +  @retval EFI_SUCCESS           Operation completed successfully.
> 
> +  @retval EFI_UNSUPPORTED       TPM device not available.
> 
> +  @retval EFI_OUT_OF_RESOURCES  Out of memory.
> 
> +  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
> 
> +**/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +TpmMeasureAndLogDataWithFlags (
> 
> +  IN UINT32             PcrIndex,
> 
> +  IN UINT32             EventType,
> 
> +  IN VOID               *EventLog,
> 
> +  IN UINT32             LogLen,
> 
> +  IN VOID               *HashData,
> 
> +  IN UINT64             HashDataLen,
> 
> +  IN UINT64             Flags
> 
> +  )
> 
> +{
> 
> +  EFI_STATUS                Status;
> 
> +  EDKII_TCG_PPI             *TcgPpi;
> 
> +  TCG_PCR_EVENT_HDR         TcgEventHdr;
> 
> +
> 
> +  Status = PeiServicesLocatePpi(
> 
> +             &gEdkiiTcgPpiGuid,
> 
> +             0,
> 
> +             NULL,
> 
> +             (VOID**)&TcgPpi
> 
> +             );
> 
> +  if (EFI_ERROR(Status)) {
> 
> +    return Status;
> 
> +  }
> 
> +
> 
> +  TcgEventHdr.PCRIndex  = PcrIndex;
> 
> +  TcgEventHdr.EventType = EventType;
> 
> +  TcgEventHdr.EventSize = LogLen;
> 
> +
> 
> +  Status = TcgPpi->HashLogExtendEvent (
> 
> +                     TcgPpi,
> 
> +                     Flags,
> 
> +                     HashData,
> 
> +                     (UINTN)HashDataLen,
> 
> +                     &TcgEventHdr,
> 
> +                     EventLog
> 
> +                     );
> 
> +  return Status;
> 
> +}
> 
> +
> 
> +/**
> 
> +  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
> 
> +**/
> 
> +STATIC
> 
> +VOID *
> 
> +TpmMeasurementGetFvName (
> 
> +  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->Signature != EFI_FVH_SIGNATURE) {
> 
> +    return NULL;
> 
> +  }
> 
> +  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;
> 
> +}
> 
> +
> 
> +/**
> 
> +  Mesure a FSP FirmwareBlob.
> 
> +
> 
> +  @param[in]  Descrption              Description for this
> FirmwareBlob.
> 
> +  @param[in]  FirmwareBlobBase        Base address of this
> FirmwareBlob.
> 
> +  @param[in]  FirmwareBlobLength      Size in bytes of this
> FirmwareBlob.
> 
> +  @param[in]  CfgRegionOffset         Configuration region offset in
> bytes.
> 
> +  @param[in]  CfgRegionSize           Configuration region in bytes.
> 
> +
> 
> +  @retval EFI_SUCCESS           Operation completed successfully.
> 
> +  @retval EFI_UNSUPPORTED       TPM device not available.
> 
> +  @retval EFI_OUT_OF_RESOURCES  Out of memory.
> 
> +  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
> 
> +*/
> 
> +STATIC
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +MeasureFspFirmwareBlobWithCfg (
> 
> +  IN CHAR8                          *Description OPTIONAL,
> 
> +  IN EFI_PHYSICAL_ADDRESS           FirmwareBlobBase,
> 
> +  IN UINT64                         FirmwareBlobLength,
> 
> +  IN UINT32                         CfgRegionOffset,
> 
> +  IN UINT32                         CfgRegionSize
> 
> +  )
> 
> +{
> 
> +  EFI_PLATFORM_FIRMWARE_BLOB        FvBlob, UPDBlob;
> 
> +  PLATFORM_FIRMWARE_BLOB2_STRUCT    FvBlob2, UPDBlob2;
> 
> +  VOID                              *FvName;
> 
> +  UINT32                            FvEventType;
> 
> +  VOID                              *FvEventLog, *UPDEventLog;
> 
> +  UINT32                            FvEventLogSize,
> UPDEventLogSize;
> 
> +  EFI_STATUS                        Status;
> 
> +  HASH_HANDLE                       HashHandle;
> 
> +  UINT8                             *HashBase;
> 
> +  UINTN                             HashSize;
> 
> +  TPML_DIGEST_VALUES                DigestList;
> 
> +
> 
> +  FvName = TpmMeasurementGetFvName (FirmwareBlobBase,
> FirmwareBlobLength);
> 
> +
> 
> +  if (((Description != NULL) || (FvName != NULL)) &&
> 
> +      (PcdGet32(PcdTcgPfpMeasurementRevision) >=
> TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105)) {
> 
> +    ZeroMem (&FvBlob2, sizeof(FvBlob2));
> 
> +    ZeroMem (&UPDBlob2, sizeof(UPDBlob2));
> 
> +    if (Description != NULL) {
> 
> +      AsciiSPrint((CHAR8*)FvBlob2.BlobDescription,
> sizeof(FvBlob2.BlobDescription), "%a", Description);
> 
> +      AsciiSPrint((CHAR8*)UPDBlob2.BlobDescription,
> sizeof(UPDBlob2.BlobDescription), "%aUDP", Description);
> 
> +     } else {
> 
> +      AsciiSPrint((CHAR8*)FvBlob2.BlobDescription,
> sizeof(FvBlob2.BlobDescription), "Fv(%g)", FvName);
> 
> +      AsciiSPrint((CHAR8*)UPDBlob2.BlobDescription,
> sizeof(UPDBlob2.BlobDescription), "(%g)UDP", FvName);
> 
> +    }
> 
> +
> 
> +    FvBlob2.BlobDescriptionSize = sizeof(FvBlob2.BlobDescription);
> 
> +    FvBlob2.BlobBase = FirmwareBlobBase;
> 
> +    FvBlob2.BlobLength = FirmwareBlobLength;
> 
> +    FvEventType = EV_EFI_PLATFORM_FIRMWARE_BLOB2;
> 
> +    FvEventLog = &FvBlob2;
> 
> +    FvEventLogSize = sizeof(FvBlob2);
> 
> +
> 
> +    UPDBlob2.BlobDescriptionSize = sizeof(UPDBlob2.BlobDescription);
> 
> +    UPDBlob2.BlobBase = CfgRegionOffset;
> 
> +    UPDBlob2.BlobLength = CfgRegionSize;
> 
> +    UPDEventLog = &UPDBlob2;
> 
> +    UPDEventLogSize = sizeof(UPDBlob2);
> 
> +  } else {
> 
> +    FvBlob.BlobBase = FirmwareBlobBase;
> 
> +    FvBlob.BlobLength = FirmwareBlobLength;
> 
> +    FvEventType = EV_EFI_PLATFORM_FIRMWARE_BLOB;
> 
> +    FvEventLog = &FvBlob;
> 
> +    FvEventLogSize = sizeof(FvBlob);
> 
> +
> 
> +    UPDBlob.BlobBase = CfgRegionOffset;
> 
> +    UPDBlob.BlobLength = CfgRegionSize;
> 
> +    UPDEventLog = &UPDBlob;
> 
> +    UPDEventLogSize = sizeof(UPDBlob);
> 
> +  }
> 
> +
> 
> +  // Initialize a SHA hash context.
> 
> +  Status = HashStart (&HashHandle);
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    DEBUG ((DEBUG_ERROR, "HashStart failed - %r\n", Status));
> 
> +    return Status;
> 
> +  }
> 
> +
> 
> +  // Hash FSP binary before UDP
> 
> +  HashBase = (UINT8 *) (UINTN) FirmwareBlobBase;
> 
> +  HashSize = (UINTN) CfgRegionOffset;
> 
> +  Status = HashUpdate (HashHandle, HashBase, HashSize);
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    DEBUG ((DEBUG_ERROR, "HashUpdate failed - %r\n", Status));
> 
> +    return Status;
> 
> +  }
> 
> +
> 
> +  // Hash FSP binary after UDP
> 
> +  HashBase = (UINT8 *) (UINTN) FirmwareBlobBase + CfgRegionOffset +
> CfgRegionSize;
> 
> +  HashSize = (UINTN)(FirmwareBlobLength - CfgRegionOffset -
> CfgRegionSize);
> 
> +  Status = HashUpdate (HashHandle, HashBase, HashSize);
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    DEBUG ((DEBUG_ERROR, "HashUpdate failed - %r\n", Status));
> 
> +    return Status;
> 
> +  }
> 
> +
> 
> +  // Finalize the SHA hash.
> 
> +  Status = HashCompleteAndExtend (HashHandle, 0, NULL, 0, &DigestList);
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    DEBUG ((DEBUG_ERROR, "HashCompleteAndExtend failed - %r\n",
> Status));
> 
> +    return Status;
> 
> +  }
> 
> +
> 
> +  Status = TpmMeasureAndLogDataWithFlags (
> 
> +             0,
> 
> +             FvEventType,
> 
> +             FvEventLog,
> 
> +             FvEventLogSize,
> 
> +             (UINT8 *) &DigestList,
> 
> +             (UINTN) sizeof(DigestList),
> 
> +             EDKII_TCG_PRE_HASH_LOG_ONLY
> 
> +             );
> 
> +
> 
> +  Status = TpmMeasureAndLogData (
> 
> +             1,
> 
> +             EV_PLATFORM_CONFIG_FLAGS,
> 
> +             UPDEventLog,
> 
> +             UPDEventLogSize,
> 
> +             (UINT8 *) (UINTN) FirmwareBlobBase + CfgRegionOffset,
> 
> +             CfgRegionSize
> 
> +             );
> 
> +
> 
> +  return Status;
> 
> +}
> 
> +
> 
> +FSP_INFO_HEADER *
> 
> +EFIAPI
> 
> +mFspFindFspHeader (

Can we use existing function from same package? FspWrapperApiLib.c->FspFindFspHeader ()

> 
> +  IN EFI_PHYSICAL_ADDRESS  FlashFvFspBase
> 
> +  )
> 
> +{
> 
> +  UINT8 *CheckPointer;
> 
> +
> 
> +  CheckPointer = (UINT8 *) (UINTN) FlashFvFspBase;
> 
> +
> 
> +  if (((EFI_FIRMWARE_VOLUME_HEADER *)CheckPointer)->Signature !=
> EFI_FVH_SIGNATURE) {
> 
> +    return NULL;
> 
> +  }
> 
> +
> 
> +  if (((EFI_FIRMWARE_VOLUME_HEADER
> *)CheckPointer)->ExtHeaderOffset != 0) {
> 
> +    CheckPointer = CheckPointer + ((EFI_FIRMWARE_VOLUME_HEADER
> *)CheckPointer)->ExtHeaderOffset;
> 
> +    CheckPointer = CheckPointer +
> ((EFI_FIRMWARE_VOLUME_EXT_HEADER *)CheckPointer)->ExtHeaderSize;
> 
> +    CheckPointer = (UINT8 *) ALIGN_POINTER (CheckPointer, 8);
> 
> +  } else {
> 
> +    CheckPointer = CheckPointer + ((EFI_FIRMWARE_VOLUME_HEADER
> *)CheckPointer)->HeaderLength;
> 
> +  }
> 
> +
> 
> +
> 
> +  CheckPointer = CheckPointer + sizeof (EFI_FFS_FILE_HEADER);
> 
> +
> 
> +  if (((EFI_RAW_SECTION *)CheckPointer)->Type != EFI_SECTION_RAW) {
> 
> +    return NULL;
> 
> +  }
> 
> +
> 
> +  CheckPointer = CheckPointer + sizeof (EFI_RAW_SECTION);
> 
> +
> 
> +  return (FSP_INFO_HEADER *)CheckPointer;
> 
> +}
> 
> +/**
> 
> +  Mesure a FSP FirmwareBlob.
> 
> +
> 
> +  @param[in]  PcrIndex                PCR Index.
> 
> +  @param[in]  Descrption              Description for this
> FirmwareBlob.
> 
> +  @param[in]  FirmwareBlobBase        Base address of this
> FirmwareBlob.
> 
> +  @param[in]  FirmwareBlobLength      Size in bytes of this
> FirmwareBlob.
> 
> +
> 
> +  @retval EFI_SUCCESS           Operation completed successfully.
> 
> +  @retval EFI_UNSUPPORTED       TPM device not available.
> 
> +  @retval EFI_OUT_OF_RESOURCES  Out of memory.
> 
> +  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
> 
> +*/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +MeasureFspFirmwareBlob (
> 
> +  IN UINT32                         PcrIndex,
> 
> +  IN CHAR8                          *Description OPTIONAL,
> 
> +  IN EFI_PHYSICAL_ADDRESS           FirmwareBlobBase,
> 
> +  IN UINT64                         FirmwareBlobLength
> 
> +  )
> 
> +{
> 
> +  UINT32           FspMeasureMask;
> 
> +  FSP_INFO_HEADER  *FspHeaderPtr;
> 
> +
> 
> +  FspMeasureMask = PcdGet32 (PcdFspMeasurementConfig);
> 
> +  if (FspMeasureMask & FSP_MEASURE_FSPUPD) {
> 
> +    FspHeaderPtr = (FSP_INFO_HEADER *) mFspFindFspHeader
> (FirmwareBlobBase);
> 
> +    if (FspHeaderPtr == NULL) {
> 
> +      return MeasureFirmwareBlob (PcrIndex, Description,
> FirmwareBlobBase, FirmwareBlobLength);;

Double ";;"

> 
> +    }
> 
> +    return MeasureFspFirmwareBlobWithCfg(Description,
> FirmwareBlobBase, FirmwareBlobLength,
> 
> +
> FspHeaderPtr->CfgRegionOffset, FspHeaderPtr->CfgRegionSize);
> 
> +  } else {
> 
> +    return MeasureFirmwareBlob (PcrIndex, Description,
> FirmwareBlobBase, FirmwareBlobLength);
> 
> +  }
> 
> +}
> 
> +
> 
> --
> 2.26.2.windows.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [edk2-devel] [PATCH v2 9/9] SecurityPkg/Tcg2: handle PRE HASH and LOG ONLY
  2020-08-06  0:33 ` [PATCH v2 9/9] SecurityPkg/Tcg2: handle PRE HASH and LOG ONLY Qi Zhang
@ 2020-08-11  0:19   ` Liming Gao
  2020-08-11  0:53     ` Qi Zhang
  2020-08-12  2:56   ` Wang, Jian J
  1 sibling, 1 reply; 23+ messages in thread
From: Liming Gao @ 2020-08-11  0:19 UTC (permalink / raw)
  To: devel@edk2.groups.io, Zhang, Qi1; +Cc: Yao, Jiewen, Wang, Jian J, Kumar, Rahul1

Qi:
  I run ECC plugin (https://edk2.groups.io/g/devel/message/63271) for this patch set. It reports below issues. Can you help update the patches to fix them?

EFI coding style error
  *Error code: 3002
  *Non-Boolean comparisons should use a compare operator (==, !=, >, < >=, <=)
  *file: D:\AllPkg\edk2\SecurityPkg\Tcg\Tcg2Pei\Tcg2Pei.c
  *Line number: 456
  *Predicate Expression: (Flags & EDKII_TCG_PRE_HASH 
EFI coding style error
  *Error code: 3002
  *Non-Boolean comparisons should use a compare operator (==, !=, >, < >=, <=)
  *file: D:\AllPkg\edk2\SecurityPkg\Tcg\Tcg2Pei\Tcg2Pei.c
  *Line number: 456
  *Predicate Expression: Flags & EDKII_TCG_PRE_HASH_LOG_ONLY 
EFI coding style error
  *Error code: 3002
  *Non-Boolean comparisons should use a compare operator (==, !=, >, < >=, <=)
  *file: D:\AllPkg\edk2\SecurityPkg\Tcg\Tcg2Pei\Tcg2Pei.c
  *Line number: 459
  *Predicate Expression: Flags & EDKII_TCG_PRE_HASH 
EFI coding style error
  *Error code: 4002
  *Function header doesn't exist
  *file: D:\AllPkg\edk2\IntelFsp2WrapperPkg\Library\BaseFspMeasurementLib\FspMeasurementLib.c
  *Line number: 279
  *Function [mFspFindFspHeader] has NO comment immediately preceding it. 
EFI coding style error
  *Error code: 8005
  *Variable name does not follow the rules: 1. First character should be upper case 2. Must contain lower case characters 3. No white space characters 4. Global variable name must start with a 'g'
  *file: D:\AllPkg\edk2\IntelFsp2WrapperPkg\Library\BaseFspMeasurementLib\FspMeasurementLib.c
  *Line number: 178
  *The variable name [*UPDEventLo] does not follow the rules
EFI coding style error
  *Error code: 8006
  *Function name does not follow the rules: 1. First character should be upper case 2. Must contain lower case characters 3. No white space characters
  *file: D:\AllPkg\edk2\IntelFsp2WrapperPkg\Library\BaseFspMeasurementLib\FspMeasurementLib.c
  *Line number: 279
  *The function name [mFspFindFspHeader] does not follow the rules
EFI coding style error
  *Error code: 9002
  *The function headers should follow Doxygen special documentation blocks in section 2.3.5
  *file: D:\AllPkg\edk2\IntelFsp2WrapperPkg\Library\BaseFspMeasurementLib\FspMeasurementLib.c
  *Line number: 149
  *Comment does NOT have tail **/ 
EFI coding style error
  *Error code: 9002
  *The function headers should follow Doxygen special documentation blocks in section 2.3.5
  *file: D:\AllPkg\edk2\IntelFsp2WrapperPkg\Library\BaseFspMeasurementLib\FspMeasurementLib.c
  *Line number: 312
  *Comment does NOT have tail **/ 
EFI coding style error
  *Error code: 9002
  *The function headers should follow Doxygen special documentation blocks in section 2.3.5
  *file: D:\AllPkg\edk2\SecurityPkg\Library\PeiTpmMeasurementLib\EventLogRecord.c
  *Line number: 86
  *Comment does NOT have tail **/ 
EFI coding style error
  *Error code: 9002
  *The function headers should follow Doxygen special documentation blocks in section 2.3.5
  *file: D:\AllPkg\edk2\SecurityPkg\Library\PeiTpmMeasurementLib\EventLogRecord.c
  *Line number: 155
  *Comment does NOT have tail **/ 
EFI coding style error
  *Error code: 9002
  *The function headers should follow Doxygen special documentation blocks in section 2.3.5
  *file: D:\AllPkg\edk2\SecurityPkg\Library\DxeTpmMeasurementLib\EventLogRecord.c
  *Line number: 86
  *Comment does NOT have tail **/ 
EFI coding style error
  *Error code: 9002
  *The function headers should follow Doxygen special documentation blocks in section 2.3.5
  *file: D:\AllPkg\edk2\SecurityPkg\Library\DxeTpmMeasurementLib\EventLogRecord.c
  *Line number: 155
  *Comment does NOT have tail **/ 
EFI coding style error
  *Error code: 9002
  *The function headers should follow Doxygen special documentation blocks in section 2.3.5
  *file: D:\AllPkg\edk2\MdeModulePkg\Library\TpmMeasurementLibNull\TpmMeasurementLibNull.c
  *Line number: 43
  *Comment does NOT have tail **/ 
EFI coding style error
  *Error code: 9002
  *The function headers should follow Doxygen special documentation blocks in section 2.3.5
  *file: D:\AllPkg\edk2\MdeModulePkg\Library\TpmMeasurementLibNull\TpmMeasurementLibNull.c
  *Line number: 43
  *in Comment, <@param[in] Descrption> does NOT consistent with parameter name PcrIndex 
EFI coding style error
  *Error code: 9002
  *The function headers should follow Doxygen special documentation blocks in section 2.3.5
  *file: D:\AllPkg\edk2\MdeModulePkg\Library\TpmMeasurementLibNull\TpmMeasurementLibNull.c
  *Line number: 43
  *in Comment, <@param[in] FirmwareBlobBase> does NOT consistent with parameter name Description 
EFI coding style error
  *Error code: 9002
  *The function headers should follow Doxygen special documentation blocks in section 2.3.5
  *file: D:\AllPkg\edk2\MdeModulePkg\Library\TpmMeasurementLibNull\TpmMeasurementLibNull.c
  *Line number: 43
  *in Comment, <@param[in] FirmwareBlobLength> does NOT consistent with parameter name FirmwareBlobBase 
EFI coding style error
  *Error code: 9002
  *The function headers should follow Doxygen special documentation blocks in section 2.3.5
  *file: D:\AllPkg\edk2\MdeModulePkg\Library\TpmMeasurementLibNull\TpmMeasurementLibNull.c
  *Line number: 43
  *in Comment, <@retval EFI_SUCCESS> does NOT consistent with parameter name FirmwareBlobLength 
EFI coding style error
  *Error code: 9002
  *The function headers should follow Doxygen special documentation blocks in section 2.3.5
  *file: D:\AllPkg\edk2\MdeModulePkg\Library\TpmMeasurementLibNull\TpmMeasurementLibNull.c
  *Line number: 70
  *Comment does NOT have tail **/

Thanks
Liming
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Qi Zhang
Sent: 2020年8月6日 8:34
To: devel@edk2.groups.io
Cc: Zhang, Qi1 <qi1.zhang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
Subject: [edk2-devel] [PATCH v2 9/9] SecurityPkg/Tcg2: handle PRE HASH and LOG ONLY

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Qi Zhang <qi1.zhang@intel.com>
---
 SecurityPkg/Include/Ppi/Tcg.h     |  5 +++++
 SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 12 +++++++-----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/SecurityPkg/Include/Ppi/Tcg.h b/SecurityPkg/Include/Ppi/Tcg.h index 0e943f2465..22f47f9817 100644
--- a/SecurityPkg/Include/Ppi/Tcg.h
+++ b/SecurityPkg/Include/Ppi/Tcg.h
@@ -18,6 +18,11 @@ typedef struct _EDKII_TCG_PPI EDKII_TCG_PPI;
 // #define EDKII_TCG_PRE_HASH  0x0000000000000001 +//+// This bit is shall be set when HashData is the pre-hash digest and log only.+//+#define EDKII_TCG_PRE_HASH_LOG_ONLY  0x0000000000000002+ /**   Tpm measure and log data, and extend the measurement result into a specific PCR. diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
index 246968bb7f..b56b03746c 100644
--- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
+++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
@@ -453,13 +453,15 @@ HashLogExtendEvent (
     return EFI_DEVICE_ERROR;   } -  if(Flags & EDKII_TCG_PRE_HASH) {+  if ((Flags & EDKII_TCG_PRE_HASH) || (Flags & EDKII_TCG_PRE_HASH_LOG_ONLY)) {     ZeroMem (&DigestList, sizeof(DigestList));     CopyMem (&DigestList, HashData, sizeof(DigestList));-    Status = Tpm2PcrExtend (-             0,-             &DigestList-             );+    if (Flags & EDKII_TCG_PRE_HASH) {+      Status = Tpm2PcrExtend (+               NewEventHdr->PCRIndex,+               &DigestList+               );+    }   } else {     Status = HashAndExtend (                NewEventHdr->PCRIndex,-- 
2.26.2.windows.1


-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#63760): https://edk2.groups.io/g/devel/message/63760
Mute This Topic: https://groups.io/mt/76019593/1759384
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [liming.gao@intel.com] -=-=-=-=-=-=


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [edk2-devel] [PATCH v2 9/9] SecurityPkg/Tcg2: handle PRE HASH and LOG ONLY
  2020-08-11  0:19   ` [edk2-devel] " Liming Gao
@ 2020-08-11  0:53     ` Qi Zhang
  2020-08-11  1:20       ` Liming Gao
  0 siblings, 1 reply; 23+ messages in thread
From: Qi Zhang @ 2020-08-11  0:53 UTC (permalink / raw)
  To: Gao, Liming, devel@edk2.groups.io
  Cc: Yao, Jiewen, Wang, Jian J, Kumar, Rahul1

Hi, Liming

Thanks for your comments! Is there any wiki of how to run ECC plugin?

BRs
Qi Zhang

> -----Original Message-----
> From: Gao, Liming <liming.gao@intel.com>
> Sent: Tuesday, August 11, 2020 8:19 AM
> To: devel@edk2.groups.io; Zhang, Qi1 <qi1.zhang@intel.com>
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
> Subject: RE: [edk2-devel] [PATCH v2 9/9] SecurityPkg/Tcg2: handle PRE HASH
> and LOG ONLY
> 
> Qi:
>   I run ECC plugin (https://edk2.groups.io/g/devel/message/63271) for this
> patch set. It reports below issues. Can you help update the patches to fix them?
> 
> EFI coding style error
>   *Error code: 3002
>   *Non-Boolean comparisons should use a compare operator (==, !=, >, < >=, <=)
>   *file: D:\AllPkg\edk2\SecurityPkg\Tcg\Tcg2Pei\Tcg2Pei.c
>   *Line number: 456
>   *Predicate Expression: (Flags & EDKII_TCG_PRE_HASH EFI coding style error
>   *Error code: 3002
>   *Non-Boolean comparisons should use a compare operator (==, !=, >, < >=, <=)
>   *file: D:\AllPkg\edk2\SecurityPkg\Tcg\Tcg2Pei\Tcg2Pei.c
>   *Line number: 456
>   *Predicate Expression: Flags & EDKII_TCG_PRE_HASH_LOG_ONLY EFI coding
> style error
>   *Error code: 3002
>   *Non-Boolean comparisons should use a compare operator (==, !=, >, < >=, <=)
>   *file: D:\AllPkg\edk2\SecurityPkg\Tcg\Tcg2Pei\Tcg2Pei.c
>   *Line number: 459
>   *Predicate Expression: Flags & EDKII_TCG_PRE_HASH EFI coding style error
>   *Error code: 4002
>   *Function header doesn't exist
>   *file:
> D:\AllPkg\edk2\IntelFsp2WrapperPkg\Library\BaseFspMeasurementLib\FspMe
> asurementLib.c
>   *Line number: 279
>   *Function [mFspFindFspHeader] has NO comment immediately preceding it.
> EFI coding style error
>   *Error code: 8005
>   *Variable name does not follow the rules: 1. First character should be upper
> case 2. Must contain lower case characters 3. No white space characters 4.
> Global variable name must start with a 'g'
>   *file:
> D:\AllPkg\edk2\IntelFsp2WrapperPkg\Library\BaseFspMeasurementLib\FspMe
> asurementLib.c
>   *Line number: 178
>   *The variable name [*UPDEventLo] does not follow the rules EFI coding style
> error
>   *Error code: 8006
>   *Function name does not follow the rules: 1. First character should be upper
> case 2. Must contain lower case characters 3. No white space characters
>   *file:
> D:\AllPkg\edk2\IntelFsp2WrapperPkg\Library\BaseFspMeasurementLib\FspMe
> asurementLib.c
>   *Line number: 279
>   *The function name [mFspFindFspHeader] does not follow the rules EFI coding
> style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation blocks in
> section 2.3.5
>   *file:
> D:\AllPkg\edk2\IntelFsp2WrapperPkg\Library\BaseFspMeasurementLib\FspMe
> asurementLib.c
>   *Line number: 149
>   *Comment does NOT have tail **/
> EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation blocks in
> section 2.3.5
>   *file:
> D:\AllPkg\edk2\IntelFsp2WrapperPkg\Library\BaseFspMeasurementLib\FspMe
> asurementLib.c
>   *Line number: 312
>   *Comment does NOT have tail **/
> EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation blocks in
> section 2.3.5
>   *file:
> D:\AllPkg\edk2\SecurityPkg\Library\PeiTpmMeasurementLib\EventLogRecord.c
>   *Line number: 86
>   *Comment does NOT have tail **/
> EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation blocks in
> section 2.3.5
>   *file:
> D:\AllPkg\edk2\SecurityPkg\Library\PeiTpmMeasurementLib\EventLogRecord.c
>   *Line number: 155
>   *Comment does NOT have tail **/
> EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation blocks in
> section 2.3.5
>   *file:
> D:\AllPkg\edk2\SecurityPkg\Library\DxeTpmMeasurementLib\EventLogRecord.
> c
>   *Line number: 86
>   *Comment does NOT have tail **/
> EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation blocks in
> section 2.3.5
>   *file:
> D:\AllPkg\edk2\SecurityPkg\Library\DxeTpmMeasurementLib\EventLogRecord.
> c
>   *Line number: 155
>   *Comment does NOT have tail **/
> EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation blocks in
> section 2.3.5
>   *file:
> D:\AllPkg\edk2\MdeModulePkg\Library\TpmMeasurementLibNull\TpmMeasur
> ementLibNull.c
>   *Line number: 43
>   *Comment does NOT have tail **/
> EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation blocks in
> section 2.3.5
>   *file:
> D:\AllPkg\edk2\MdeModulePkg\Library\TpmMeasurementLibNull\TpmMeasur
> ementLibNull.c
>   *Line number: 43
>   *in Comment, <@param[in] Descrption> does NOT consistent with parameter
> name PcrIndex EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation blocks in
> section 2.3.5
>   *file:
> D:\AllPkg\edk2\MdeModulePkg\Library\TpmMeasurementLibNull\TpmMeasur
> ementLibNull.c
>   *Line number: 43
>   *in Comment, <@param[in] FirmwareBlobBase> does NOT consistent with
> parameter name Description EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation blocks in
> section 2.3.5
>   *file:
> D:\AllPkg\edk2\MdeModulePkg\Library\TpmMeasurementLibNull\TpmMeasur
> ementLibNull.c
>   *Line number: 43
>   *in Comment, <@param[in] FirmwareBlobLength> does NOT consistent with
> parameter name FirmwareBlobBase EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation blocks in
> section 2.3.5
>   *file:
> D:\AllPkg\edk2\MdeModulePkg\Library\TpmMeasurementLibNull\TpmMeasur
> ementLibNull.c
>   *Line number: 43
>   *in Comment, <@retval EFI_SUCCESS> does NOT consistent with parameter
> name FirmwareBlobLength EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation blocks in
> section 2.3.5
>   *file:
> D:\AllPkg\edk2\MdeModulePkg\Library\TpmMeasurementLibNull\TpmMeasur
> ementLibNull.c
>   *Line number: 70
>   *Comment does NOT have tail **/
> 
> Thanks
> Liming
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Qi Zhang
> Sent: 2020年8月6日 8:34
> To: devel@edk2.groups.io
> Cc: Zhang, Qi1 <qi1.zhang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>; Kumar, Rahul1
> <rahul1.kumar@intel.com>
> Subject: [edk2-devel] [PATCH v2 9/9] SecurityPkg/Tcg2: handle PRE HASH and
> LOG ONLY
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Qi Zhang <qi1.zhang@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Qi Zhang <qi1.zhang@intel.com>
> ---
>  SecurityPkg/Include/Ppi/Tcg.h     |  5 +++++
>  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 12 +++++++-----
>  2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/SecurityPkg/Include/Ppi/Tcg.h b/SecurityPkg/Include/Ppi/Tcg.h index
> 0e943f2465..22f47f9817 100644
> --- a/SecurityPkg/Include/Ppi/Tcg.h
> +++ b/SecurityPkg/Include/Ppi/Tcg.h
> @@ -18,6 +18,11 @@ typedef struct _EDKII_TCG_PPI EDKII_TCG_PPI;
>  // #define EDKII_TCG_PRE_HASH  0x0000000000000001 +//+// This bit is shall
> be set when HashData is the pre-hash digest and log only.+//+#define
> EDKII_TCG_PRE_HASH_LOG_ONLY  0x0000000000000002+ /**   Tpm measure
> and log data, and extend the measurement result into a specific PCR. diff --git
> a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> index 246968bb7f..b56b03746c 100644
> --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> @@ -453,13 +453,15 @@ HashLogExtendEvent (
>      return EFI_DEVICE_ERROR;   } -  if(Flags & EDKII_TCG_PRE_HASH) {+  if
> ((Flags & EDKII_TCG_PRE_HASH) || (Flags & EDKII_TCG_PRE_HASH_LOG_ONLY))
> {     ZeroMem (&DigestList, sizeof(DigestList));     CopyMem (&DigestList,
> HashData, sizeof(DigestList));-    Status = Tpm2PcrExtend (-             0,-
> &DigestList-             );+    if (Flags & EDKII_TCG_PRE_HASH) {+      Status =
> Tpm2PcrExtend (+               NewEventHdr->PCRIndex,+
> &DigestList+               );+    }   } else {     Status = HashAndExtend
> (                NewEventHdr->PCRIndex,--
> 2.26.2.windows.1
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> 
> View/Reply Online (#63760): https://edk2.groups.io/g/devel/message/63760
> Mute This Topic: https://groups.io/mt/76019593/1759384
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub  [liming.gao@intel.com] -=-
> =-=-=-=-=


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [edk2-devel] [PATCH v2 9/9] SecurityPkg/Tcg2: handle PRE HASH and LOG ONLY
  2020-08-11  0:53     ` Qi Zhang
@ 2020-08-11  1:20       ` Liming Gao
  0 siblings, 0 replies; 23+ messages in thread
From: Liming Gao @ 2020-08-11  1:20 UTC (permalink / raw)
  To: Zhang, Qi1, devel@edk2.groups.io
  Cc: Yao, Jiewen, Wang, Jian J, Kumar, Rahul1, Gao, Liming

Qi:
  EccPlugin will be enabled in open CI. Then, ECC result can be checked in open CI result. Now, I use the standalone EccCheck.py from https://github.com/shenglei10/edk2/tree/ecc_script to check the patch set. 

Thanks
Liming
-----Original Message-----
From: Zhang, Qi1 <qi1.zhang@intel.com> 
Sent: 2020年8月11日 8:54
To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io
Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
Subject: RE: [edk2-devel] [PATCH v2 9/9] SecurityPkg/Tcg2: handle PRE HASH and LOG ONLY

Hi, Liming

Thanks for your comments! Is there any wiki of how to run ECC plugin?

BRs
Qi Zhang

> -----Original Message-----
> From: Gao, Liming <liming.gao@intel.com>
> Sent: Tuesday, August 11, 2020 8:19 AM
> To: devel@edk2.groups.io; Zhang, Qi1 <qi1.zhang@intel.com>
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J 
> <jian.j.wang@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
> Subject: RE: [edk2-devel] [PATCH v2 9/9] SecurityPkg/Tcg2: handle PRE 
> HASH and LOG ONLY
> 
> Qi:
>   I run ECC plugin (https://edk2.groups.io/g/devel/message/63271) for 
> this patch set. It reports below issues. Can you help update the patches to fix them?
> 
> EFI coding style error
>   *Error code: 3002
>   *Non-Boolean comparisons should use a compare operator (==, !=, >, < >=, <=)
>   *file: D:\AllPkg\edk2\SecurityPkg\Tcg\Tcg2Pei\Tcg2Pei.c
>   *Line number: 456
>   *Predicate Expression: (Flags & EDKII_TCG_PRE_HASH EFI coding style error
>   *Error code: 3002
>   *Non-Boolean comparisons should use a compare operator (==, !=, >, < >=, <=)
>   *file: D:\AllPkg\edk2\SecurityPkg\Tcg\Tcg2Pei\Tcg2Pei.c
>   *Line number: 456
>   *Predicate Expression: Flags & EDKII_TCG_PRE_HASH_LOG_ONLY EFI 
> coding style error
>   *Error code: 3002
>   *Non-Boolean comparisons should use a compare operator (==, !=, >, < >=, <=)
>   *file: D:\AllPkg\edk2\SecurityPkg\Tcg\Tcg2Pei\Tcg2Pei.c
>   *Line number: 459
>   *Predicate Expression: Flags & EDKII_TCG_PRE_HASH EFI coding style error
>   *Error code: 4002
>   *Function header doesn't exist
>   *file:
> D:\AllPkg\edk2\IntelFsp2WrapperPkg\Library\BaseFspMeasurementLib\FspMe
> asurementLib.c
>   *Line number: 279
>   *Function [mFspFindFspHeader] has NO comment immediately preceding it.
> EFI coding style error
>   *Error code: 8005
>   *Variable name does not follow the rules: 1. First character should 
> be upper case 2. Must contain lower case characters 3. No white space characters 4.
> Global variable name must start with a 'g'
>   *file:
> D:\AllPkg\edk2\IntelFsp2WrapperPkg\Library\BaseFspMeasurementLib\FspMe
> asurementLib.c
>   *Line number: 178
>   *The variable name [*UPDEventLo] does not follow the rules EFI 
> coding style error
>   *Error code: 8006
>   *Function name does not follow the rules: 1. First character should 
> be upper case 2. Must contain lower case characters 3. No white space characters
>   *file:
> D:\AllPkg\edk2\IntelFsp2WrapperPkg\Library\BaseFspMeasurementLib\FspMe
> asurementLib.c
>   *Line number: 279
>   *The function name [mFspFindFspHeader] does not follow the rules EFI 
> coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation 
> blocks in section 2.3.5
>   *file:
> D:\AllPkg\edk2\IntelFsp2WrapperPkg\Library\BaseFspMeasurementLib\FspMe
> asurementLib.c
>   *Line number: 149
>   *Comment does NOT have tail **/
> EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation 
> blocks in section 2.3.5
>   *file:
> D:\AllPkg\edk2\IntelFsp2WrapperPkg\Library\BaseFspMeasurementLib\FspMe
> asurementLib.c
>   *Line number: 312
>   *Comment does NOT have tail **/
> EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation 
> blocks in section 2.3.5
>   *file:
> D:\AllPkg\edk2\SecurityPkg\Library\PeiTpmMeasurementLib\EventLogRecord.c
>   *Line number: 86
>   *Comment does NOT have tail **/
> EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation 
> blocks in section 2.3.5
>   *file:
> D:\AllPkg\edk2\SecurityPkg\Library\PeiTpmMeasurementLib\EventLogRecord.c
>   *Line number: 155
>   *Comment does NOT have tail **/
> EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation 
> blocks in section 2.3.5
>   *file:
> D:\AllPkg\edk2\SecurityPkg\Library\DxeTpmMeasurementLib\EventLogRecord.
> c
>   *Line number: 86
>   *Comment does NOT have tail **/
> EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation 
> blocks in section 2.3.5
>   *file:
> D:\AllPkg\edk2\SecurityPkg\Library\DxeTpmMeasurementLib\EventLogRecord.
> c
>   *Line number: 155
>   *Comment does NOT have tail **/
> EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation 
> blocks in section 2.3.5
>   *file:
> D:\AllPkg\edk2\MdeModulePkg\Library\TpmMeasurementLibNull\TpmMeasur
> ementLibNull.c
>   *Line number: 43
>   *Comment does NOT have tail **/
> EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation 
> blocks in section 2.3.5
>   *file:
> D:\AllPkg\edk2\MdeModulePkg\Library\TpmMeasurementLibNull\TpmMeasur
> ementLibNull.c
>   *Line number: 43
>   *in Comment, <@param[in] Descrption> does NOT consistent with 
> parameter name PcrIndex EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation 
> blocks in section 2.3.5
>   *file:
> D:\AllPkg\edk2\MdeModulePkg\Library\TpmMeasurementLibNull\TpmMeasur
> ementLibNull.c
>   *Line number: 43
>   *in Comment, <@param[in] FirmwareBlobBase> does NOT consistent with 
> parameter name Description EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation 
> blocks in section 2.3.5
>   *file:
> D:\AllPkg\edk2\MdeModulePkg\Library\TpmMeasurementLibNull\TpmMeasur
> ementLibNull.c
>   *Line number: 43
>   *in Comment, <@param[in] FirmwareBlobLength> does NOT consistent 
> with parameter name FirmwareBlobBase EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation 
> blocks in section 2.3.5
>   *file:
> D:\AllPkg\edk2\MdeModulePkg\Library\TpmMeasurementLibNull\TpmMeasur
> ementLibNull.c
>   *Line number: 43
>   *in Comment, <@retval EFI_SUCCESS> does NOT consistent with 
> parameter name FirmwareBlobLength EFI coding style error
>   *Error code: 9002
>   *The function headers should follow Doxygen special documentation 
> blocks in section 2.3.5
>   *file:
> D:\AllPkg\edk2\MdeModulePkg\Library\TpmMeasurementLibNull\TpmMeasur
> ementLibNull.c
>   *Line number: 70
>   *Comment does NOT have tail **/
> 
> Thanks
> Liming
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Qi 
> Zhang
> Sent: 2020年8月6日 8:34
> To: devel@edk2.groups.io
> Cc: Zhang, Qi1 <qi1.zhang@intel.com>; Yao, Jiewen 
> <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Kumar, 
> Rahul1 <rahul1.kumar@intel.com>
> Subject: [edk2-devel] [PATCH v2 9/9] SecurityPkg/Tcg2: handle PRE HASH 
> and LOG ONLY
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Qi Zhang <qi1.zhang@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Qi Zhang <qi1.zhang@intel.com>
> ---
>  SecurityPkg/Include/Ppi/Tcg.h     |  5 +++++
>  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 12 +++++++-----
>  2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/SecurityPkg/Include/Ppi/Tcg.h 
> b/SecurityPkg/Include/Ppi/Tcg.h index
> 0e943f2465..22f47f9817 100644
> --- a/SecurityPkg/Include/Ppi/Tcg.h
> +++ b/SecurityPkg/Include/Ppi/Tcg.h
> @@ -18,6 +18,11 @@ typedef struct _EDKII_TCG_PPI EDKII_TCG_PPI;  // 
> #define EDKII_TCG_PRE_HASH  0x0000000000000001 +//+// This bit is 
> shall be set when HashData is the pre-hash digest and log only.+//+#define
> EDKII_TCG_PRE_HASH_LOG_ONLY  0x0000000000000002+ /**   Tpm measure
> and log data, and extend the measurement result into a specific PCR. 
> diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c 
> b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> index 246968bb7f..b56b03746c 100644
> --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> @@ -453,13 +453,15 @@ HashLogExtendEvent (
>      return EFI_DEVICE_ERROR;   } -  if(Flags & EDKII_TCG_PRE_HASH) {+  if
> ((Flags & EDKII_TCG_PRE_HASH) || (Flags & EDKII_TCG_PRE_HASH_LOG_ONLY))
> {     ZeroMem (&DigestList, sizeof(DigestList));     CopyMem (&DigestList,
> HashData, sizeof(DigestList));-    Status = Tpm2PcrExtend (-             0,-
> &DigestList-             );+    if (Flags & EDKII_TCG_PRE_HASH) {+      Status =
> Tpm2PcrExtend (+               NewEventHdr->PCRIndex,+
> &DigestList+               );+    }   } else {     Status = HashAndExtend
> (                NewEventHdr->PCRIndex,--
> 2.26.2.windows.1
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> 
> View/Reply Online (#63760): 
> https://edk2.groups.io/g/devel/message/63760
> Mute This Topic: https://groups.io/mt/76019593/1759384
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub  
> [liming.gao@intel.com] -=- =-=-=-=-=


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 0/9] Need add a FSP binary measurement
  2020-08-06  0:33 [PATCH v2 0/9] Need add a FSP binary measurement Qi Zhang
                   ` (8 preceding siblings ...)
  2020-08-06  0:33 ` [PATCH v2 9/9] SecurityPkg/Tcg2: handle PRE HASH and LOG ONLY Qi Zhang
@ 2020-08-11  2:00 ` Yao, Jiewen
  2020-08-11 16:25 ` Wang, Jian J
  10 siblings, 0 replies; 23+ messages in thread
From: Yao, Jiewen @ 2020-08-11  2:00 UTC (permalink / raw)
  To: Zhang, Qi1, devel@edk2.groups.io
  Cc: Wang, Jian J, Wu, Hao A, Chiu, Chasel, Desimone, Nathaniel L,
	Zeng, Star

Hi Qi
Thanks for the update.

1) Since this is a new feature, a platform may already measure FSP binary in some ways, I recommend we change the default policy to: gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig|0x00000000.

2) We should not check FSP_MEASURE_FSP in IntelFsp2WrappePkg, because it is reserved bit. Below code should be removed.
if (FspMeasureMask & FSP_MEASURE_FSP) {

3) I think " CheckPointer = (UINT8 *) ALIGN_POINTER (CheckPointer, 8);" should also be present in "else" branch below.

  if (((EFI_FIRMWARE_VOLUME_HEADER *)CheckPointer)->ExtHeaderOffset != 0) {
    CheckPointer = CheckPointer + ((EFI_FIRMWARE_VOLUME_HEADER *)CheckPointer)->ExtHeaderOffset;
    CheckPointer = CheckPointer + ((EFI_FIRMWARE_VOLUME_EXT_HEADER *)CheckPointer)->ExtHeaderSize;
    CheckPointer = (UINT8 *) ALIGN_POINTER (CheckPointer, 8);
  } else {
    CheckPointer = CheckPointer + ((EFI_FIRMWARE_VOLUME_HEADER *)CheckPointer)->HeaderLength;
  }

4) Below logic can be simplified to:

  if (FspMeasureMask & FSP_MEASURE_FSPUPD) {
    FspHeaderPtr = (FSP_INFO_HEADER *) mFspFindFspHeader (FirmwareBlobBase);
    if (FspHeaderPtr == NULL) {
      return MeasureFirmwareBlob (PcrIndex, Description, FirmwareBlobBase, FirmwareBlobLength);;
    }
    return MeasureFspFirmwareBlobWithCfg(Description, FirmwareBlobBase, FirmwareBlobLength,
                                         FspHeaderPtr->CfgRegionOffset, FspHeaderPtr->CfgRegionSize);
  } else {
    return MeasureFirmwareBlob (PcrIndex, Description, FirmwareBlobBase, FirmwareBlobLength);
  }

To:

  if (FspMeasureMask & FSP_MEASURE_FSPUPD) {
    FspHeaderPtr = (FSP_INFO_HEADER *) mFspFindFspHeader (FirmwareBlobBase);
    if (FspHeaderPtr != NULL) {
      return MeasureFspFirmwareBlobWithCfg(Description, FirmwareBlobBase, FirmwareBlobLength,
                                         FspHeaderPtr->CfgRegionOffset, FspHeaderPtr->CfgRegionSize);
    }
  }

  return MeasureFirmwareBlob (PcrIndex, Description, FirmwareBlobBase, FirmwareBlobLength);


Thank you
Yao Jiewen



> -----Original Message-----
> From: Zhang, Qi1 <qi1.zhang@intel.com>
> Sent: Thursday, August 6, 2020 8:34 AM
> To: devel@edk2.groups.io
> Cc: Zhang, Qi1 <qi1.zhang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Chiu,
> Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [PATCH v2 0/9] Need add a FSP binary measurement
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376
> 
> The EDKII BIOS calls FSP API in FSP Wrapper Pkg.
> This FSP code need to be measured into TPM.
> 
> We need add a generic module in FSP Wrapper Pkg code to measure:
> 1) FSP-T, FSP-M, FSP-S in API mode.
> 2) FSP-T in Dispatch-mode. The FSP-M and FSP-S will be reported
>    as standard FV and they will be measured by TCG-PEI.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Qi Zhang <qi1.zhang@intel.com>
> 
> Jiewen Yao (8):
>   MdeModulePkg/TpmMeasurementLib: Add new API to TpmMeasurmentLib.
>   MdeModulePkg/NullTpmMeasurementLib: Add new API.
>   SecurityPkg/DxeTpmMeasurementLib: Add new API.
>   SecurityPkg/PeiTpmMeasurementLib: Add new API.
>   IntelFsp2WrapperPkg/FspMeasurementLib: Add header file.
>   IntelFsp2WrapperPkg/FspMeasurementLib: Add BaseFspMeasurementLib.
>   IntelFsp2WraperPkg/Fsp{m|s}WrapperPeim: Add FspBin measurement.
>   IntelFsp2Wrapper/dsc: Add FspTpmMeasurementLib and
>     PcdFspMeasurementConfig.
> 
> Qi Zhang (1):
>   SecurityPkg/Tcg2: handle PRE HASH and LOG ONLY
> 
>  .../FspmWrapperPeim/FspmWrapperPeim.c         |  90 ++++-
>  .../FspmWrapperPeim/FspmWrapperPeim.inf       |  20 +-
>  .../FspsWrapperPeim/FspsWrapperPeim.c         |  85 ++++-
>  .../FspsWrapperPeim/FspsWrapperPeim.inf       |  27 +-
>  .../Include/Library/FspMeasurementLib.h       |  39 ++
>  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec   |  17 +
>  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc   |   5 +-
>  .../BaseFspMeasurementLib.inf                 |  54 +++
>  .../BaseFspMeasurementLib/FspMeasurementLib.c | 349 ++++++++++++++++++
>  .../Include/Library/TpmMeasurementLib.h       |  48 ++-
>  .../TpmMeasurementLibNull.c                   |  61 ++-
>  .../TpmMeasurementLibNull.inf                 |   6 +-
>  SecurityPkg/Include/Ppi/Tcg.h                 |   5 +
>  .../DxeTpmMeasurementLib.inf                  |   6 +-
>  .../DxeTpmMeasurementLib/EventLogRecord.c     | 218 +++++++++++
>  .../PeiTpmMeasurementLib/EventLogRecord.c     | 218 +++++++++++
>  .../PeiTpmMeasurementLib.inf                  |   4 +
>  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c             |  12 +-
>  18 files changed, 1233 insertions(+), 31 deletions(-)
>  create mode 100644
> IntelFsp2WrapperPkg/Include/Library/FspMeasurementLib.h
>  create mode 100644
> IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurementLi
> b.inf
>  create mode 100644
> IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c
>  create mode 100644
> SecurityPkg/Library/DxeTpmMeasurementLib/EventLogRecord.c
>  create mode 100644
> SecurityPkg/Library/PeiTpmMeasurementLib/EventLogRecord.c
> 
> --
> 2.26.2.windows.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 0/9] Need add a FSP binary measurement
  2020-08-06  0:33 [PATCH v2 0/9] Need add a FSP binary measurement Qi Zhang
                   ` (9 preceding siblings ...)
  2020-08-11  2:00 ` [PATCH v2 0/9] Need add a FSP binary measurement Yao, Jiewen
@ 2020-08-11 16:25 ` Wang, Jian J
  10 siblings, 0 replies; 23+ messages in thread
From: Wang, Jian J @ 2020-08-11 16:25 UTC (permalink / raw)
  To: Zhang, Qi1, devel@edk2.groups.io
  Cc: Yao, Jiewen, Wu, Hao A, Chiu, Chasel, Desimone, Nathaniel L,
	Zeng, Star

Hi Qi,

Two common comments here. More specific comments will be given separately
in each patch email later.

c1. SecurityPkg/Library/DxeTpmMeasurementLib/EventLogRecord.c and 
      SecurityPkg/Library/PeiTpmMeasurementLib/EventLogRecord.c are almost
      the same. Consider consolidating the code in some way, like a shared lib
     or shared folder.
c2. TpmMeasurementGetFvName() or similar is duplicated in at least four places:
      DxeTpmMeasurementLib, PeiTpmMeasurementLib, BaseFspMeasurementLib
      and Tcg2Pei. Consider consolidate the code.


Regards,
Jian

> -----Original Message-----
> From: Zhang, Qi1 <qi1.zhang@intel.com>
> Sent: Thursday, August 06, 2020 8:34 AM
> To: devel@edk2.groups.io
> Cc: Zhang, Qi1 <qi1.zhang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Chiu,
> Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [PATCH v2 0/9] Need add a FSP binary measurement
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376
> 
> The EDKII BIOS calls FSP API in FSP Wrapper Pkg.
> This FSP code need to be measured into TPM.
> 
> We need add a generic module in FSP Wrapper Pkg code to measure:
> 1) FSP-T, FSP-M, FSP-S in API mode.
> 2) FSP-T in Dispatch-mode. The FSP-M and FSP-S will be reported
>    as standard FV and they will be measured by TCG-PEI.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Qi Zhang <qi1.zhang@intel.com>
> 
> Jiewen Yao (8):
>   MdeModulePkg/TpmMeasurementLib: Add new API to TpmMeasurmentLib.
>   MdeModulePkg/NullTpmMeasurementLib: Add new API.
>   SecurityPkg/DxeTpmMeasurementLib: Add new API.
>   SecurityPkg/PeiTpmMeasurementLib: Add new API.
>   IntelFsp2WrapperPkg/FspMeasurementLib: Add header file.
>   IntelFsp2WrapperPkg/FspMeasurementLib: Add BaseFspMeasurementLib.
>   IntelFsp2WraperPkg/Fsp{m|s}WrapperPeim: Add FspBin measurement.
>   IntelFsp2Wrapper/dsc: Add FspTpmMeasurementLib and
>     PcdFspMeasurementConfig.
> 
> Qi Zhang (1):
>   SecurityPkg/Tcg2: handle PRE HASH and LOG ONLY
> 
>  .../FspmWrapperPeim/FspmWrapperPeim.c         |  90 ++++-
>  .../FspmWrapperPeim/FspmWrapperPeim.inf       |  20 +-
>  .../FspsWrapperPeim/FspsWrapperPeim.c         |  85 ++++-
>  .../FspsWrapperPeim/FspsWrapperPeim.inf       |  27 +-
>  .../Include/Library/FspMeasurementLib.h       |  39 ++
>  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec   |  17 +
>  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc   |   5 +-
>  .../BaseFspMeasurementLib.inf                 |  54 +++
>  .../BaseFspMeasurementLib/FspMeasurementLib.c | 349 ++++++++++++++++++
>  .../Include/Library/TpmMeasurementLib.h       |  48 ++-
>  .../TpmMeasurementLibNull.c                   |  61 ++-
>  .../TpmMeasurementLibNull.inf                 |   6 +-
>  SecurityPkg/Include/Ppi/Tcg.h                 |   5 +
>  .../DxeTpmMeasurementLib.inf                  |   6 +-
>  .../DxeTpmMeasurementLib/EventLogRecord.c     | 218 +++++++++++
>  .../PeiTpmMeasurementLib/EventLogRecord.c     | 218 +++++++++++
>  .../PeiTpmMeasurementLib.inf                  |   4 +
>  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c             |  12 +-
>  18 files changed, 1233 insertions(+), 31 deletions(-)
>  create mode 100644
> IntelFsp2WrapperPkg/Include/Library/FspMeasurementLib.h
>  create mode 100644
> IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurementLi
> b.inf
>  create mode 100644
> IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c
>  create mode 100644
> SecurityPkg/Library/DxeTpmMeasurementLib/EventLogRecord.c
>  create mode 100644
> SecurityPkg/Library/PeiTpmMeasurementLib/EventLogRecord.c
> 
> --
> 2.26.2.windows.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 1/9] MdeModulePkg/TpmMeasurementLib: Add new API to TpmMeasurmentLib.
  2020-08-06  0:33 ` [PATCH v2 1/9] MdeModulePkg/TpmMeasurementLib: Add new API to TpmMeasurmentLib Qi Zhang
@ 2020-08-12  1:30   ` Wang, Jian J
  0 siblings, 0 replies; 23+ messages in thread
From: Wang, Jian J @ 2020-08-12  1:30 UTC (permalink / raw)
  To: Zhang, Qi1, devel@edk2.groups.io; +Cc: Yao, Jiewen, Wu, Hao A

Hi Qi,

Some typos. See inline comments below.

> -----Original Message-----
> From: Zhang, Qi1 <qi1.zhang@intel.com>
> Sent: Thursday, August 06, 2020 8:34 AM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Wu, Hao A <hao.a.wu@intel.com>; Zhang, Qi1 <qi1.zhang@intel.com>
> Subject: [PATCH v2 1/9] MdeModulePkg/TpmMeasurementLib: Add new API to
> TpmMeasurmentLib.
> 
> From: Jiewen Yao <jiewen.yao@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Qi Zhang <qi1.zhang@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
>  .../Include/Library/TpmMeasurementLib.h       | 48 ++++++++++++++++++-
>  1 file changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Include/Library/TpmMeasurementLib.h
> b/MdeModulePkg/Include/Library/TpmMeasurementLib.h
> index ddf6723f03..5a0f97d208 100644
> --- a/MdeModulePkg/Include/Library/TpmMeasurementLib.h
> +++ b/MdeModulePkg/Include/Library/TpmMeasurementLib.h
> @@ -1,7 +1,7 @@
>  /** @file
> 
>    This library is used by other modules to measure data to TPM.
> 
> 
> 
> -Copyright (c) 2012, Intel Corporation. All rights reserved. <BR>
> 
> +Copyright (c) 2012 - 2020, Intel Corporation. All rights reserved. <BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> @@ -35,4 +35,50 @@ TpmMeasureAndLogData (
>    IN UINT64             HashDataLen
> 
>    );
> 
> 
> 
> +/**
> 
> +  Mesure a FirmwareBlob.

'Mesure' -> 'Measure'

> 
> +
> 
> +  @param[in]  PcrIndex                PCR Index.
> 
> +  @param[in]  Descrption              Description for this FirmwareBlob.

'Descrption' -> 'Description'

> 
> +  @param[in]  FirmwareBlobBase        Base address of this FirmwareBlob.
> 
> +  @param[in]  FirmwareBlobLength      Size in bytes of this FirmwareBlob.
> 
> +
> 
> +  @retval EFI_SUCCESS           Operation completed successfully.
> 
> +  @retval EFI_UNSUPPORTED       TPM device not available.
> 
> +  @retval EFI_OUT_OF_RESOURCES  Out of memory.
> 
> +  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
> 
> +*/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +MeasureFirmwareBlob (
> 
> +  IN UINT32                         PcrIndex,
> 
> +  IN CHAR8                          *Description OPTIONAL,
> 
> +  IN EFI_PHYSICAL_ADDRESS           FirmwareBlobBase,
> 
> +  IN UINT64                         FirmwareBlobLength
> 
> +  );
> 
> +
> 
> +/**
> 
> +  Mesure a HandoffTable.

'Mesure' -> 'Measure'

> 
> +
> 
> +  @param[in]  PcrIndex                PcrIndex of the measurment.

'measurment' -> 'measurement'

> 
> +  @param[in]  Descrption              Description for this HandoffTable.

'Descrption' -> 'Description'

> 
> +  @param[in]  TableGuid               GUID of this HandoffTable.
> 
> +  @param[in]  TableAddress            Base address of this HandoffTable.
> 
> +  @param[in]  TableLength             Size in bytes of this HandoffTable.
> 
> +
> 
> +  @retval EFI_SUCCESS           Operation completed successfully.
> 
> +  @retval EFI_UNSUPPORTED       TPM device not available.
> 
> +  @retval EFI_OUT_OF_RESOURCES  Out of memory.
> 
> +  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
> 
> +*/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +MeasureHandoffTable (
> 
> +  IN UINT32                         PcrIndex,
> 
> +  IN CHAR8                          *Description OPTIONAL,
> 
> +  IN EFI_GUID                       *TableGuid,
> 
> +  IN VOID                           *TableAddress,
> 
> +  IN UINTN                          TableLength
> 
> +  );
> 
> +
> 
>  #endif
> 
> --
> 2.26.2.windows.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 2/9] MdeModulePkg/NullTpmMeasurementLib: Add new API.
  2020-08-06  0:33 ` [PATCH v2 2/9] MdeModulePkg/NullTpmMeasurementLib: Add new API Qi Zhang
@ 2020-08-12  1:35   ` Wang, Jian J
  0 siblings, 0 replies; 23+ messages in thread
From: Wang, Jian J @ 2020-08-12  1:35 UTC (permalink / raw)
  To: Zhang, Qi1, devel@edk2.groups.io; +Cc: Yao, Jiewen, Wu, Hao A

Qi,

Some typos. See inline comments below.

> -----Original Message-----
> From: Zhang, Qi1 <qi1.zhang@intel.com>
> Sent: Thursday, August 06, 2020 8:34 AM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Wu, Hao A <hao.a.wu@intel.com>; Zhang, Qi1 <qi1.zhang@intel.com>
> Subject: [PATCH v2 2/9] MdeModulePkg/NullTpmMeasurementLib: Add new API.
> 
> From: Jiewen Yao <jiewen.yao@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Qi Zhang <qi1.zhang@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
>  .../TpmMeasurementLibNull.c                   | 61 ++++++++++++++++++-
>  .../TpmMeasurementLibNull.inf                 |  6 +-
>  2 files changed, 63 insertions(+), 4 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c
> b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c
> index b9c5b68de8..2ce38d8258 100644
> ---
> a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c
> +++
> b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c
> @@ -1,11 +1,13 @@
>  /** @file
> 
>    This library is used by other modules to measure data to TPM.
> 
> 
> 
> -Copyright (c) 2015, Intel Corporation. All rights reserved. <BR>
> 
> +Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved. <BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> 
> 
> +#include <Uefi.h>
> 
> +
> 
>  /**
> 
>    Tpm measure and log data, and extend the measurement result into a specific
> PCR.
> 
> 
> 
> @@ -37,3 +39,60 @@ TpmMeasureAndLogData (
>    //
> 
>    return EFI_SUCCESS;
> 
>  }
> 
> +
> 
> +/**
> 
> +  Mesure a FirmwareBlob.

'Mesure' -> 'Measure'

> 
> +
> 
> +  @param[in]  Descrption              Description for this FirmwareBlob.

'Descrption' -> 'Description'

> 
> +  @param[in]  FirmwareBlobBase        Base address of this FirmwareBlob.
> 
> +  @param[in]  FirmwareBlobLength      Size in bytes of this FirmwareBlob.
> 
> +
> 
> +  @retval EFI_SUCCESS           Operation completed successfully.
> 
> +  @retval EFI_UNSUPPORTED       TPM device not available.
> 
> +  @retval EFI_OUT_OF_RESOURCES  Out of memory.
> 
> +  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
> 
> +*/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +MeasureFirmwareBlob (
> 
> +  IN UINT32                         PcrIndex,
> 
> +  IN CHAR8                          *Description OPTIONAL,
> 
> +  IN EFI_PHYSICAL_ADDRESS           FirmwareBlobBase,
> 
> +  IN UINT64                         FirmwareBlobLength
> 
> +  )
> 
> +{
> 
> +  //
> 
> +  // Do nothing, just return EFI_SUCCESS.
> 
> +  //
> 
> +  return EFI_SUCCESS;
> 
> +}
> 
> +
> 
> +/**
> 
> +  Mesure a HandoffTable.

'Mesure' -> 'Measure'

> 
> +
> 
> +  @param[in]  PcrIndex                PcrIndex of the measurment.

'measurment' -> 'measurement'

> 
> +  @param[in]  Descrption              Description for this HandoffTable.

'Descrption' -> 'Description'

> 
> +  @param[in]  TableGuid               GUID of this HandoffTable.
> 
> +  @param[in]  TableAddress            Base address of this HandoffTable.
> 
> +  @param[in]  TableLength             Size in bytes of this HandoffTable.
> 
> +
> 
> +  @retval EFI_SUCCESS           Operation completed successfully.
> 
> +  @retval EFI_UNSUPPORTED       TPM device not available.
> 
> +  @retval EFI_OUT_OF_RESOURCES  Out of memory.
> 
> +  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
> 
> +*/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +MeasureHandoffTable (
> 
> +  IN UINT32                         PcrIndex,
> 
> +  IN CHAR8                          *Description OPTIONAL,
> 
> +  IN EFI_GUID                       *TableGuid,
> 
> +  IN VOID                           *TableAddress,
> 
> +  IN UINTN                          TableLength
> 
> +  )
> 
> +{
> 
> +  //
> 
> +  // Do nothing, just return EFI_SUCCESS.
> 
> +  //
> 
> +  return EFI_SUCCESS;
> 
> +}
> 
> diff --git
> a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.in
> f
> b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.in
> f
> index 61abcfa2ec..1db2c0d6a7 100644
> ---
> a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.in
> f
> +++
> b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.in
> f
> @@ -1,7 +1,7 @@
>  ## @file
> 
>  #  Provides NULL TPM measurement function.
> 
>  #
> 
> -# Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
> 
> +# Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.<BR>
> 
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  #
> 
>  ##
> 
> @@ -10,9 +10,9 @@
>    INF_VERSION                    = 0x00010005
> 
>    BASE_NAME                      = TpmMeasurementLibNull
> 
>    FILE_GUID                      = 6DFD6E9F-9278-48D8-8F45-B6CFF2C2B69C
> 
> -  MODULE_TYPE                    = UEFI_DRIVER
> 
> +  MODULE_TYPE                    = BASE
> 
>    VERSION_STRING                 = 1.0
> 
> -  LIBRARY_CLASS                  = TpmMeasurementLib|DXE_DRIVER
> DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
> 
> +  LIBRARY_CLASS                  = TpmMeasurementLib
> 
>    MODULE_UNI_FILE                = TpmMeasurementLibNull.uni
> 
> 
> 
>  #
> 
> --
> 2.26.2.windows.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [edk2-devel] [PATCH v2 3/9] SecurityPkg/DxeTpmMeasurementLib: Add new API.
  2020-08-06  0:33 ` [PATCH v2 3/9] SecurityPkg/DxeTpmMeasurementLib: " Qi Zhang
@ 2020-08-12  2:12   ` Wang, Jian J
  0 siblings, 0 replies; 23+ messages in thread
From: Wang, Jian J @ 2020-08-12  2:12 UTC (permalink / raw)
  To: devel@edk2.groups.io, Zhang, Qi1; +Cc: Yao, Jiewen

Qi,

Comments below.

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Qi Zhang
> Sent: Thursday, August 06, 2020 8:34 AM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Zhang, Qi1 <qi1.zhang@intel.com>
> Subject: [edk2-devel] [PATCH v2 3/9] SecurityPkg/DxeTpmMeasurementLib: Add
> new API.
> 
> From: Jiewen Yao <jiewen.yao@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Qi Zhang <qi1.zhang@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
>  .../DxeTpmMeasurementLib.inf                  |   6 +-
>  .../DxeTpmMeasurementLib/EventLogRecord.c     | 218 ++++++++++++++++++
>  2 files changed, 223 insertions(+), 1 deletion(-)
>  create mode 100644
> SecurityPkg/Library/DxeTpmMeasurementLib/EventLogRecord.c
> 
> diff --git
> a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> index 7d41bc41f9..39448f8ee8 100644
> --- a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> +++
> b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> @@ -4,7 +4,7 @@
>  #  This library provides TpmMeasureAndLogData() to measure and log data, and
> 
>  #  extend the measurement result into a specific PCR.
> 
>  #
> 
> -# Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
> 
> +# Copyright (c) 2012 - 2020, Intel Corporation. All rights reserved.<BR>
> 
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  #
> 
>  ##
> 
> @@ -26,6 +26,7 @@
> 
> 
>  [Sources]
> 
>    DxeTpmMeasurementLib.c
> 
> +  EventLogRecord.c
> 
> 
> 
>  [Packages]
> 
>    MdePkg/MdePkg.dec
> 
> @@ -42,3 +43,6 @@
>  [Protocols]
> 
>    gEfiTcgProtocolGuid           ## SOMETIMES_CONSUMES
> 
>    gEfiTcg2ProtocolGuid          ## SOMETIMES_CONSUMES
> 
> +
> 
> +[Pcd]
> 
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision          ##
> CONSUMES
> 
> diff --git a/SecurityPkg/Library/DxeTpmMeasurementLib/EventLogRecord.c
> b/SecurityPkg/Library/DxeTpmMeasurementLib/EventLogRecord.c
> new file mode 100644
> index 0000000000..7b3726e44b
> --- /dev/null
> +++ b/SecurityPkg/Library/DxeTpmMeasurementLib/EventLogRecord.c
> @@ -0,0 +1,218 @@
> +/** @file
> 
> +  This library is used by other modules to measure data to TPM.
> 
> +
> 
> +Copyright (c) 2020, Intel Corporation. All rights reserved. <BR>
> 
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +
> 
> +**/
> 
> +
> 
> +#include <PiDxe.h>
> 
> +
> 
> +#include <Library/BaseMemoryLib.h>
> 
> +#include <Library/DebugLib.h>
> 
> +#include <Library/ReportStatusCodeLib.h>
> 
> +#include <Library/HobLib.h>
> 
> +#include <Library/PcdLib.h>
> 
> +#include <Library/PrintLib.h>
> 
> +#include <Library/TpmMeasurementLib.h>
> 
> +
> 
> +#include <IndustryStandard/UefiTcgPlatform.h>
> 
> +
> 
> +#pragma pack (1)
> 
> +
> 
> +#define PLATFORM_FIRMWARE_BLOB_DESC "Fv(XXXXXXXX-XXXX-XXXX-XXXX-
> XXXXXXXXXXXX)"
> 
> +typedef struct {
> 
> +  UINT8                             BlobDescriptionSize;
> 
> +  UINT8
> BlobDescription[sizeof(PLATFORM_FIRMWARE_BLOB_DESC)];
> 
> +  EFI_PHYSICAL_ADDRESS              BlobBase;
> 
> +  UINT64                            BlobLength;
> 
> +} PLATFORM_FIRMWARE_BLOB2_STRUCT;
> 
> +
> 
> +#define HANDOFF_TABLE_POINTER_DESC  "1234567890ABCDEF"
> 
> +typedef struct {
> 
> +  UINT8                             TableDescriptionSize;
> 
> +  UINT8
> TableDescription[sizeof(HANDOFF_TABLE_POINTER_DESC)];
> 
> +  UINT64                            NumberOfTables;
> 
> +  EFI_CONFIGURATION_TABLE           TableEntry[1];
> 
> +} HANDOFF_TABLE_POINTERS2_STRUCT;
> 
> +
> 
> +#pragma pack ()
> 
> +
> 
> +/**
> 
> +  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 *
> 
> +TpmMeasurementGetFvName (
> 
> +  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->Signature != EFI_FVH_SIGNATURE) {
> 
> +    return NULL;
> 
> +  }
> 
> +  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;
> 
> +}
> 
> +
> 
> +/**
> 
> +  Mesure a FirmwareBlob.

'Mesure' -> 'Measure'

> 
> +
> 
> +  @param[in]  PcrIndex                PcrIndex of the measurment.

'measurment' -> 'measurement'

> 
> +  @param[in]  Descrption              Description for this FirmwareBlob.

'Descrption' -> 'Description'

> 
> +  @param[in]  FirmwareBlobBase        Base address of this FirmwareBlob.
> 
> +  @param[in]  FirmwareBlobLength      Size in bytes of this FirmwareBlob.
> 
> +
> 
> +  @retval EFI_SUCCESS           Operation completed successfully.
> 
> +  @retval EFI_UNSUPPORTED       TPM device not available.
> 
> +  @retval EFI_OUT_OF_RESOURCES  Out of memory.
> 
> +  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
> 
> +*/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +MeasureFirmwareBlob (
> 
> +  IN UINT32                         PcrIndex,
> 
> +  IN CHAR8                          *Description OPTIONAL,
> 
> +  IN EFI_PHYSICAL_ADDRESS           FirmwareBlobBase,
> 
> +  IN UINT64                         FirmwareBlobLength
> 
> +  )
> 
> +{
> 
> +  EFI_PLATFORM_FIRMWARE_BLOB        FvBlob;
> 
> +  PLATFORM_FIRMWARE_BLOB2_STRUCT    FvBlob2;
> 
> +  VOID                              *FvName;
> 
> +  UINT32                            EventType;
> 
> +  VOID                              *EventLog;
> 
> +  UINT32                            EventLogSize;
> 
> +  EFI_STATUS                        Status;
> 
> +
> 
> +  FvName = TpmMeasurementGetFvName (FirmwareBlobBase,
> FirmwareBlobLength);
> 
> +
> 
> +  if (((Description != NULL) || (FvName != NULL)) &&
> 
> +      (PcdGet32(PcdTcgPfpMeasurementRevision) >=
> TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105)) {
> 
> +    ZeroMem (&FvBlob2, sizeof(FvBlob2));

It looks that clear the data structure is not necessary. Code below
will fill all fields in it. According to description of AsciiSPrint(), it also
produces NULL-terminated string. I see no reason to clear it in
advance.

> 
> +    if (Description != NULL) {
> 
> +      AsciiSPrint((CHAR8*)FvBlob2.BlobDescription,
> sizeof(FvBlob2.BlobDescription), "%a", Description);
> 
> +    } else {
> 
> +      AsciiSPrint((CHAR8*)FvBlob2.BlobDescription,
> sizeof(FvBlob2.BlobDescription), "Fv(%g)", FvName);
> 
> +    }
> 
> +
> 
> +    FvBlob2.BlobDescriptionSize = sizeof(FvBlob2.BlobDescription);
> 
> +    FvBlob2.BlobBase = FirmwareBlobBase;
> 
> +    FvBlob2.BlobLength = FirmwareBlobLength;
> 
> +
> 
> +    EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB2;
> 
> +    EventLog = &FvBlob2;
> 
> +    EventLogSize = sizeof(FvBlob2);
> 
> +  } else {
> 
> +    FvBlob.BlobBase = FirmwareBlobBase;
> 
> +    FvBlob.BlobLength = FirmwareBlobLength;
> 
> +
> 
> +    EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB;
> 
> +    EventLog = &FvBlob;
> 
> +    EventLogSize = sizeof(FvBlob);
> 
> +  }
> 
> +
> 
> +  Status = TpmMeasureAndLogData (
> 
> +             PcrIndex,
> 
> +             EventType,
> 
> +             EventLog,
> 
> +             EventLogSize,
> 
> +             (VOID*)(UINTN)FirmwareBlobBase,
> 
> +             FirmwareBlobLength
> 
> +             );
> 
> +
> 
> +  return Status;
> 
> +}
> 
> +
> 
> +/**
> 
> +  Mesure a HandoffTable.

'Mesure' -> 'Measure'

> 
> +
> 
> +  @param[in]  PcrIndex                PcrIndex of the measurment.

'measurment' -> 'measurement'

> 
> +  @param[in]  Descrption              Description for this HandoffTable.

'Descrption' -> 'Description'

> 
> +  @param[in]  TableGuid               GUID of this HandoffTable.
> 
> +  @param[in]  TableAddress            Base address of this HandoffTable.
> 
> +  @param[in]  TableLength             Size in bytes of this HandoffTable.
> 
> +
> 
> +  @retval EFI_SUCCESS           Operation completed successfully.
> 
> +  @retval EFI_UNSUPPORTED       TPM device not available.
> 
> +  @retval EFI_OUT_OF_RESOURCES  Out of memory.
> 
> +  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
> 
> +*/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +MeasureHandoffTable (
> 
> +  IN UINT32                         PcrIndex,
> 
> +  IN CHAR8                          *Description OPTIONAL,
> 
> +  IN EFI_GUID                       *TableGuid,
> 
> +  IN VOID                           *TableAddress,
> 
> +  IN UINTN                          TableLength
> 
> +  )
> 
> +{
> 
> +  EFI_HANDOFF_TABLE_POINTERS        HandoffTables;
> 
> +  HANDOFF_TABLE_POINTERS2_STRUCT    HandoffTables2;
> 
> +  UINT32                            EventType;
> 
> +  VOID                              *EventLog;
> 
> +  UINT32                            EventLogSize;
> 
> +  EFI_STATUS                        Status;
> 
> +
> 
> +  if ((Description != NULL) &&
> 
> +      (PcdGet32(PcdTcgPfpMeasurementRevision) >=
> TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105)) {
> 
> +    ZeroMem (&HandoffTables2, sizeof(HandoffTables2));

The same as before. I see no reason to clear the data in advance.

> 
> +    AsciiSPrint((CHAR8*)HandoffTables2.TableDescription,
> sizeof(HandoffTables2.TableDescription), "%a", Description);
> 
> +
> 
> +    HandoffTables2.TableDescriptionSize =
> sizeof(HandoffTables2.TableDescription);
> 
> +    HandoffTables2.NumberOfTables = 1;
> 
> +    CopyGuid (&(HandoffTables2.TableEntry[0].VendorGuid), TableGuid);
> 
> +    HandoffTables2.TableEntry[0].VendorTable = TableAddress;
> 
> +
> 
> +    EventType = EV_EFI_HANDOFF_TABLES2;
> 
> +    EventLog = &HandoffTables2;
> 
> +    EventLogSize = sizeof(HandoffTables2);
> 
> +  } else {
> 
> +    HandoffTables.NumberOfTables = 1;
> 
> +    CopyGuid (&(HandoffTables.TableEntry[0].VendorGuid), TableGuid);
> 
> +    HandoffTables.TableEntry[0].VendorTable = TableAddress;
> 
> +
> 
> +    EventType = EV_EFI_HANDOFF_TABLES;
> 
> +    EventLog = &HandoffTables;
> 
> +    EventLogSize = sizeof(HandoffTables);
> 
> +  }
> 
> +
> 
> +  Status = TpmMeasureAndLogData (
> 
> +             PcrIndex,
> 
> +             EventType,
> 
> +             EventLog,
> 
> +             EventLogSize,
> 
> +             TableAddress,
> 
> +             TableLength
> 
> +             );
> 
> +  return Status;
> 
> +}
> 
> --
> 2.26.2.windows.1
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> 
> View/Reply Online (#63754): https://edk2.groups.io/g/devel/message/63754
> Mute This Topic: https://groups.io/mt/76019584/1768734
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub  [jian.j.wang@intel.com]
> -=-=-=-=-=-=


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 4/9] SecurityPkg/PeiTpmMeasurementLib: Add new API.
  2020-08-06  0:33 ` [PATCH v2 4/9] SecurityPkg/PeiTpmMeasurementLib: " Qi Zhang
@ 2020-08-12  2:14   ` Wang, Jian J
  0 siblings, 0 replies; 23+ messages in thread
From: Wang, Jian J @ 2020-08-12  2:14 UTC (permalink / raw)
  To: Zhang, Qi1, devel@edk2.groups.io; +Cc: Yao, Jiewen

Qi,

This patch is similar to patch 3. Please refer to comments in that one.

Regards,
Jian

> -----Original Message-----
> From: Zhang, Qi1 <qi1.zhang@intel.com>
> Sent: Thursday, August 06, 2020 8:34 AM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Zhang, Qi1 <qi1.zhang@intel.com>
> Subject: [PATCH v2 4/9] SecurityPkg/PeiTpmMeasurementLib: Add new API.
> 
> From: Jiewen Yao <jiewen.yao@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Qi Zhang <qi1.zhang@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
>  .../PeiTpmMeasurementLib/EventLogRecord.c     | 218 ++++++++++++++++++
>  .../PeiTpmMeasurementLib.inf                  |   4 +
>  2 files changed, 222 insertions(+)
>  create mode 100644
> SecurityPkg/Library/PeiTpmMeasurementLib/EventLogRecord.c
> 
> diff --git a/SecurityPkg/Library/PeiTpmMeasurementLib/EventLogRecord.c
> b/SecurityPkg/Library/PeiTpmMeasurementLib/EventLogRecord.c
> new file mode 100644
> index 0000000000..cececdf7b2
> --- /dev/null
> +++ b/SecurityPkg/Library/PeiTpmMeasurementLib/EventLogRecord.c
> @@ -0,0 +1,218 @@
> +/** @file
> 
> +  This library is used by other modules to measure data to TPM.
> 
> +
> 
> +Copyright (c) 2020, Intel Corporation. All rights reserved. <BR>
> 
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +
> 
> +**/
> 
> +
> 
> +#include <PiPei.h>
> 
> +
> 
> +#include <Library/BaseMemoryLib.h>
> 
> +#include <Library/DebugLib.h>
> 
> +#include <Library/ReportStatusCodeLib.h>
> 
> +#include <Library/HobLib.h>
> 
> +#include <Library/PcdLib.h>
> 
> +#include <Library/PrintLib.h>
> 
> +#include <Library/TpmMeasurementLib.h>
> 
> +
> 
> +#include <IndustryStandard/UefiTcgPlatform.h>
> 
> +
> 
> +#pragma pack (1)
> 
> +
> 
> +#define PLATFORM_FIRMWARE_BLOB_DESC "Fv(XXXXXXXX-XXXX-XXXX-XXXX-
> XXXXXXXXXXXX)"
> 
> +typedef struct {
> 
> +  UINT8                             BlobDescriptionSize;
> 
> +  UINT8
> BlobDescription[sizeof(PLATFORM_FIRMWARE_BLOB_DESC)];
> 
> +  EFI_PHYSICAL_ADDRESS              BlobBase;
> 
> +  UINT64                            BlobLength;
> 
> +} PLATFORM_FIRMWARE_BLOB2_STRUCT;
> 
> +
> 
> +#define HANDOFF_TABLE_POINTER_DESC  "1234567890ABCDEF"
> 
> +typedef struct {
> 
> +  UINT8                             TableDescriptionSize;
> 
> +  UINT8
> TableDescription[sizeof(HANDOFF_TABLE_POINTER_DESC)];
> 
> +  UINT64                            NumberOfTables;
> 
> +  EFI_CONFIGURATION_TABLE           TableEntry[1];
> 
> +} HANDOFF_TABLE_POINTERS2_STRUCT;
> 
> +
> 
> +#pragma pack ()
> 
> +
> 
> +/**
> 
> +  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 *
> 
> +TpmMeasurementGetFvName (
> 
> +  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->Signature != EFI_FVH_SIGNATURE) {
> 
> +    return NULL;
> 
> +  }
> 
> +  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;
> 
> +}
> 
> +
> 
> +/**
> 
> +  Mesure a FirmwareBlob.
> 
> +
> 
> +  @param[in]  PcrIndex                PcrIndex of the measurment.
> 
> +  @param[in]  Descrption              Description for this FirmwareBlob.
> 
> +  @param[in]  FirmwareBlobBase        Base address of this FirmwareBlob.
> 
> +  @param[in]  FirmwareBlobLength      Size in bytes of this FirmwareBlob.
> 
> +
> 
> +  @retval EFI_SUCCESS           Operation completed successfully.
> 
> +  @retval EFI_UNSUPPORTED       TPM device not available.
> 
> +  @retval EFI_OUT_OF_RESOURCES  Out of memory.
> 
> +  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
> 
> +*/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +MeasureFirmwareBlob (
> 
> +  IN UINT32                         PcrIndex,
> 
> +  IN CHAR8                          *Description OPTIONAL,
> 
> +  IN EFI_PHYSICAL_ADDRESS           FirmwareBlobBase,
> 
> +  IN UINT64                         FirmwareBlobLength
> 
> +  )
> 
> +{
> 
> +  EFI_PLATFORM_FIRMWARE_BLOB        FvBlob;
> 
> +  PLATFORM_FIRMWARE_BLOB2_STRUCT    FvBlob2;
> 
> +  VOID                              *FvName;
> 
> +  UINT32                            EventType;
> 
> +  VOID                              *EventLog;
> 
> +  UINT32                            EventLogSize;
> 
> +  EFI_STATUS                        Status;
> 
> +
> 
> +  FvName = TpmMeasurementGetFvName (FirmwareBlobBase,
> FirmwareBlobLength);
> 
> +
> 
> +  if (((Description != NULL) || (FvName != NULL)) &&
> 
> +      (PcdGet32(PcdTcgPfpMeasurementRevision) >=
> TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105)) {
> 
> +    ZeroMem (&FvBlob2, sizeof(FvBlob2));
> 
> +    if (Description != NULL) {
> 
> +      AsciiSPrint((CHAR8*)FvBlob2.BlobDescription,
> sizeof(FvBlob2.BlobDescription), "%a", Description);
> 
> +    } else {
> 
> +      AsciiSPrint((CHAR8*)FvBlob2.BlobDescription,
> sizeof(FvBlob2.BlobDescription), "Fv(%g)", FvName);
> 
> +    }
> 
> +
> 
> +    FvBlob2.BlobDescriptionSize = sizeof(FvBlob2.BlobDescription);
> 
> +    FvBlob2.BlobBase = FirmwareBlobBase;
> 
> +    FvBlob2.BlobLength = FirmwareBlobLength;
> 
> +
> 
> +    EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB2;
> 
> +    EventLog = &FvBlob2;
> 
> +    EventLogSize = sizeof(FvBlob2);
> 
> +  } else {
> 
> +    FvBlob.BlobBase = FirmwareBlobBase;
> 
> +    FvBlob.BlobLength = FirmwareBlobLength;
> 
> +
> 
> +    EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB;
> 
> +    EventLog = &FvBlob;
> 
> +    EventLogSize = sizeof(FvBlob);
> 
> +  }
> 
> +
> 
> +  Status = TpmMeasureAndLogData (
> 
> +             PcrIndex,
> 
> +             EventType,
> 
> +             EventLog,
> 
> +             EventLogSize,
> 
> +             (VOID*)(UINTN)FirmwareBlobBase,
> 
> +             FirmwareBlobLength
> 
> +             );
> 
> +
> 
> +  return Status;
> 
> +}
> 
> +
> 
> +/**
> 
> +  Mesure a HandoffTable.
> 
> +
> 
> +  @param[in]  PcrIndex                PcrIndex of the measurment.
> 
> +  @param[in]  Descrption              Description for this HandoffTable.
> 
> +  @param[in]  TableGuid               GUID of this HandoffTable.
> 
> +  @param[in]  TableAddress            Base address of this HandoffTable.
> 
> +  @param[in]  TableLength             Size in bytes of this HandoffTable.
> 
> +
> 
> +  @retval EFI_SUCCESS           Operation completed successfully.
> 
> +  @retval EFI_UNSUPPORTED       TPM device not available.
> 
> +  @retval EFI_OUT_OF_RESOURCES  Out of memory.
> 
> +  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
> 
> +*/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +MeasureHandoffTable (
> 
> +  IN UINT32                         PcrIndex,
> 
> +  IN CHAR8                          *Description OPTIONAL,
> 
> +  IN EFI_GUID                       *TableGuid,
> 
> +  IN VOID                           *TableAddress,
> 
> +  IN UINTN                          TableLength
> 
> +  )
> 
> +{
> 
> +  EFI_HANDOFF_TABLE_POINTERS        HandoffTables;
> 
> +  HANDOFF_TABLE_POINTERS2_STRUCT    HandoffTables2;
> 
> +  UINT32                            EventType;
> 
> +  VOID                              *EventLog;
> 
> +  UINT32                            EventLogSize;
> 
> +  EFI_STATUS                        Status;
> 
> +
> 
> +  if ((Description != NULL) &&
> 
> +      (PcdGet32(PcdTcgPfpMeasurementRevision) >=
> TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105)) {
> 
> +    ZeroMem (&HandoffTables2, sizeof(HandoffTables2));
> 
> +    AsciiSPrint((CHAR8*)HandoffTables2.TableDescription,
> sizeof(HandoffTables2.TableDescription), "%a", Description);
> 
> +
> 
> +    HandoffTables2.TableDescriptionSize =
> sizeof(HandoffTables2.TableDescription);
> 
> +    HandoffTables2.NumberOfTables = 1;
> 
> +    CopyGuid (&(HandoffTables2.TableEntry[0].VendorGuid), TableGuid);
> 
> +    HandoffTables2.TableEntry[0].VendorTable = TableAddress;
> 
> +
> 
> +    EventType = EV_EFI_HANDOFF_TABLES2;
> 
> +    EventLog = &HandoffTables2;
> 
> +    EventLogSize = sizeof(HandoffTables2);
> 
> +  } else {
> 
> +    HandoffTables.NumberOfTables = 1;
> 
> +    CopyGuid (&(HandoffTables.TableEntry[0].VendorGuid), TableGuid);
> 
> +    HandoffTables.TableEntry[0].VendorTable = TableAddress;
> 
> +
> 
> +    EventType = EV_EFI_HANDOFF_TABLES;
> 
> +    EventLog = &HandoffTables;
> 
> +    EventLogSize = sizeof(HandoffTables);
> 
> +  }
> 
> +
> 
> +  Status = TpmMeasureAndLogData (
> 
> +             PcrIndex,
> 
> +             EventType,
> 
> +             EventLog,
> 
> +             EventLogSize,
> 
> +             TableAddress,
> 
> +             TableLength
> 
> +             );
> 
> +  return Status;
> 
> +}
> 
> diff --git
> a/SecurityPkg/Library/PeiTpmMeasurementLib/PeiTpmMeasurementLib.inf
> b/SecurityPkg/Library/PeiTpmMeasurementLib/PeiTpmMeasurementLib.inf
> index 6625d0fd01..489353af2e 100644
> --- a/SecurityPkg/Library/PeiTpmMeasurementLib/PeiTpmMeasurementLib.inf
> +++ b/SecurityPkg/Library/PeiTpmMeasurementLib/PeiTpmMeasurementLib.inf
> @@ -26,6 +26,7 @@
> 
> 
>  [Sources]
> 
>    PeiTpmMeasurementLib.c
> 
> +  EventLogRecord.c
> 
> 
> 
>  [Packages]
> 
>    MdePkg/MdePkg.dec
> 
> @@ -45,6 +46,9 @@
>  [Ppis]
> 
>    gEdkiiTcgPpiGuid                                                     ## CONSUMES
> 
> 
> 
> +[Pcd]
> 
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision          ##
> CONSUMES
> 
> +
> 
>  [Depex]
> 
>    gEfiPeiMasterBootModePpiGuid AND
> 
>    gEfiTpmDeviceSelectedGuid
> 
> --
> 2.26.2.windows.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [edk2-devel] [PATCH v2 5/9] IntelFsp2WrapperPkg/FspMeasurementLib: Add header file.
  2020-08-06  0:33 ` [PATCH v2 5/9] IntelFsp2WrapperPkg/FspMeasurementLib: Add header file Qi Zhang
@ 2020-08-12  2:15   ` Wang, Jian J
  0 siblings, 0 replies; 23+ messages in thread
From: Wang, Jian J @ 2020-08-12  2:15 UTC (permalink / raw)
  To: devel@edk2.groups.io, Zhang, Qi1
  Cc: Yao, Jiewen, Chiu, Chasel, Desimone, Nathaniel L, Zeng, Star

Qi,

Some typos below.


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Qi Zhang
> Sent: Thursday, August 06, 2020 8:34 AM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>;
> Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Zhang, Qi1 <qi1.zhang@intel.com>
> Subject: [edk2-devel] [PATCH v2 5/9] IntelFsp2WrapperPkg/FspMeasurementLib:
> Add header file.
> 
> From: Jiewen Yao <jiewen.yao@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Qi Zhang <qi1.zhang@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
>  .../Include/Library/FspMeasurementLib.h       | 39 +++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644
> IntelFsp2WrapperPkg/Include/Library/FspMeasurementLib.h
> 
> diff --git a/IntelFsp2WrapperPkg/Include/Library/FspMeasurementLib.h
> b/IntelFsp2WrapperPkg/Include/Library/FspMeasurementLib.h
> new file mode 100644
> index 0000000000..4ab40420ad
> --- /dev/null
> +++ b/IntelFsp2WrapperPkg/Include/Library/FspMeasurementLib.h
> @@ -0,0 +1,39 @@
> +/** @file
> 
> +  This library is used by FSP modules to measure data to TPM.
> 
> +
> 
> +Copyright (c) 2020, Intel Corporation. All rights reserved. <BR>
> 
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +
> 
> +**/
> 
> +
> 
> +#ifndef _FSP_MEASUREMENT_LIB_H_
> 
> +#define _FSP_MEASUREMENT_LIB_H_
> 
> +
> 
> +#define FSP_MEASURE_FSP       BIT0
> 
> +#define FSP_MEASURE_FSPT      BIT1
> 
> +#define FSP_MEASURE_FSPM      BIT2
> 
> +#define FSP_MEASURE_FSPS      BIT3
> 
> +#define FSP_MEASURE_FSPUPD    BIT31
> 
> +
> 
> +/**
> 
> +  Mesure a FSP FirmwareBlob.

'Mesure' -> 'Measure'

> 
> +
> 
> +  @param[in]  PcrIndex                PCR Index.
> 
> +  @param[in]  Descrption              Description for this FirmwareBlob.

'Descrption' -> 'Description'

> 
> +  @param[in]  FirmwareBlobBase        Base address of this FirmwareBlob.
> 
> +  @param[in]  FirmwareBlobLength      Size in bytes of this FirmwareBlob.
> 
> +
> 
> +  @retval EFI_SUCCESS           Operation completed successfully.
> 
> +  @retval EFI_UNSUPPORTED       TPM device not available.
> 
> +  @retval EFI_OUT_OF_RESOURCES  Out of memory.
> 
> +  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
> 
> +*/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +MeasureFspFirmwareBlob (
> 
> +  IN UINT32                         PcrIndex,
> 
> +  IN CHAR8                          *Description OPTIONAL,
> 
> +  IN EFI_PHYSICAL_ADDRESS           FirmwareBlobBase,
> 
> +  IN UINT64                         FirmwareBlobLength
> 
> +  );
> 
> +#endif
> 
> --
> 2.26.2.windows.1
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> 
> View/Reply Online (#63756): https://edk2.groups.io/g/devel/message/63756
> Mute This Topic: https://groups.io/mt/76019586/1768734
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub  [jian.j.wang@intel.com]
> -=-=-=-=-=-=


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [edk2-devel] [PATCH v2 6/9] IntelFsp2WrapperPkg/FspMeasurementLib: Add BaseFspMeasurementLib.
  2020-08-06  0:33 ` [PATCH v2 6/9] IntelFsp2WrapperPkg/FspMeasurementLib: Add BaseFspMeasurementLib Qi Zhang
  2020-08-06  1:09   ` Chiu, Chasel
@ 2020-08-12  2:48   ` Wang, Jian J
  1 sibling, 0 replies; 23+ messages in thread
From: Wang, Jian J @ 2020-08-12  2:48 UTC (permalink / raw)
  To: devel@edk2.groups.io, Zhang, Qi1
  Cc: Yao, Jiewen, Chiu, Chasel, Desimone, Nathaniel L, Zeng, Star

Qi,

Comments below.

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Qi Zhang
> Sent: Thursday, August 06, 2020 8:34 AM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>;
> Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Zhang, Qi1 <qi1.zhang@intel.com>
> Subject: [edk2-devel] [PATCH v2 6/9] IntelFsp2WrapperPkg/FspMeasurementLib:
> Add BaseFspMeasurementLib.
> 
> From: Jiewen Yao <jiewen.yao@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Qi Zhang <qi1.zhang@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
>  .../BaseFspMeasurementLib.inf                 |  54 +++
>  .../BaseFspMeasurementLib/FspMeasurementLib.c | 349 ++++++++++++++++++
>  2 files changed, 403 insertions(+)
>  create mode 100644
> IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurementLi
> b.inf
>  create mode 100644
> IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c
> 
> diff --git
> a/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurement
> Lib.inf
> b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurement
> Lib.inf
> new file mode 100644
> index 0000000000..d30168117d
> --- /dev/null
> +++
> b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/BaseFspMeasurement
> Lib.inf
> @@ -0,0 +1,54 @@
> +## @file
> 
> +#  Provides FSP measurement functions.
> 
> +#
> 
> +#  This library provides MeasureFspFirmwareBlob() to measure FSP binary.
> 
> +#
> 
> +# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> 
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +#
> 
> +##
> 
> +
> 
> +[Defines]
> 
> +  INF_VERSION                    = 0x00010005
> 
> +  BASE_NAME                      = FspMeasurementLib
> 
> +  FILE_GUID                      = 9A62C49D-C45A-4322-9F3C-45958DF0056B
> 
> +  MODULE_TYPE                    = BASE
> 
> +  VERSION_STRING                 = 1.0
> 
> +  LIBRARY_CLASS                  = FspMeasurementLib
> 
> +
> 
> +#
> 
> +# The following information is for reference only and not required by the build
> tools.
> 
> +#
> 
> +#  VALID_ARCHITECTURES           = IA32 X64
> 
> +#
> 
> +
> 
> +[Sources]
> 
> +  FspMeasurementLib.c
> 
> +
> 
> +[Packages]
> 
> +  MdePkg/MdePkg.dec
> 
> +  MdeModulePkg/MdeModulePkg.dec
> 
> +  SecurityPkg/SecurityPkg.dec
> 
> +  IntelFsp2Pkg/IntelFsp2Pkg.dec
> 
> +  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec
> 
> +
> 
> +[LibraryClasses]
> 
> +  BaseLib
> 
> +  BaseMemoryLib
> 
> +  DebugLib
> 
> +  PrintLib
> 
> +  PcdLib
> 
> +  PeiServicesLib
> 
> +  PeiServicesTablePointerLib
> 
> +  FspWrapperApiLib
> 
> +  TpmMeasurementLib
> 
> +  HashLib
> 
> +
> 
> +[Ppis]
> 
> +  gEdkiiTcgPpiGuid                                                   ## CONSUMES
> 
> +
> 
> +[Pcd]
> 
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig            ##
> CONSUMES
> 
> +  gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress                 ##
> CONSUMES
> 
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision        ##
> CONSUMES
> 
> +
> 
> diff --git
> a/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c
> b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c
> new file mode 100644
> index 0000000000..316570cd2c
> --- /dev/null
> +++
> b/IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/FspMeasurementLib.c
> @@ -0,0 +1,349 @@
> +/** @file
> 
> +  This library is used by FSP modules to measure data to TPM.
> 
> +
> 
> +Copyright (c) 2020, Intel Corporation. All rights reserved. <BR>
> 
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +
> 
> +**/
> 
> +
> 
> +#include <PiPei.h>
> 
> +#include <Uefi.h>
> 
> +
> 
> +#include <Library/BaseMemoryLib.h>
> 
> +#include <Library/PeiServicesLib.h>
> 
> +#include <Library/PeiServicesTablePointerLib.h>
> 
> +#include <Library/PcdLib.h>
> 
> +#include <Library/PrintLib.h>
> 
> +#include <Library/DebugLib.h>
> 
> +#include <Library/FspWrapperApiLib.h>
> 
> +#include <Library/TpmMeasurementLib.h>
> 
> +#include <Library/FspMeasurementLib.h>
> 
> +#include <Library/HashLib.h>
> 
> +
> 
> +#include <Ppi/Tcg.h>
> 
> +#include <IndustryStandard/UefiTcgPlatform.h>
> 
> +
> 
> +#pragma pack (1)
> 
> +
> 
> +#define PLATFORM_FIRMWARE_BLOB_DESC "Fv(XXXXXXXX-XXXX-XXXX-XXXX-
> XXXXXXXXXXXX)"
> 
> +typedef struct {
> 
> +  UINT8                             BlobDescriptionSize;
> 
> +  UINT8
> BlobDescription[sizeof(PLATFORM_FIRMWARE_BLOB_DESC)];
> 
> +  EFI_PHYSICAL_ADDRESS              BlobBase;
> 
> +  UINT64                            BlobLength;
> 
> +} PLATFORM_FIRMWARE_BLOB2_STRUCT;
> 
> +
> 
> +#define HANDOFF_TABLE_POINTER_DESC  "1234567890ABCDEF"
> 
> +typedef struct {
> 
> +  UINT8                             TableDescriptionSize;
> 
> +  UINT8
> TableDescription[sizeof(HANDOFF_TABLE_POINTER_DESC)];
> 
> +  UINT64                            NumberOfTables;
> 
> +  EFI_CONFIGURATION_TABLE           TableEntry[1];
> 
> +} HANDOFF_TABLE_POINTERS2_STRUCT;
> 
> +
> 
> +#pragma pack ()
> 

Above definitions have been defined the same in EventLogRecord.c in patch3&4.
Suggest put them in a common header file.

> +
> 
> +/**
> 
> +  Tpm measure and log data, and extend the measurement result into a specific
> PCR.
> 
> +
> 
> +  @param[in]  PcrIndex         PCR Index.
> 
> +  @param[in]  EventType        Event type.
> 
> +  @param[in]  EventLog         Measurement event log.
> 
> +  @param[in]  LogLen           Event log length in bytes.
> 
> +  @param[in]  HashData         The start of the data buffer to be hashed,
> extended.
> 
> +  @param[in]  HashDataLen      The length, in bytes, of the buffer referenced by
> HashData
> 
> +  @param[in]  Flags            Bitmap providing additional information.
> 
> +
> 
> +  @retval EFI_SUCCESS           Operation completed successfully.
> 
> +  @retval EFI_UNSUPPORTED       TPM device not available.
> 
> +  @retval EFI_OUT_OF_RESOURCES  Out of memory.
> 
> +  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
> 
> +**/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +TpmMeasureAndLogDataWithFlags (
> 
> +  IN UINT32             PcrIndex,
> 
> +  IN UINT32             EventType,
> 
> +  IN VOID               *EventLog,
> 
> +  IN UINT32             LogLen,
> 
> +  IN VOID               *HashData,
> 
> +  IN UINT64             HashDataLen,
> 
> +  IN UINT64             Flags
> 
> +  )
> 
> +{
> 
> +  EFI_STATUS                Status;
> 
> +  EDKII_TCG_PPI             *TcgPpi;
> 
> +  TCG_PCR_EVENT_HDR         TcgEventHdr;
> 
> +
> 
> +  Status = PeiServicesLocatePpi(
> 
> +             &gEdkiiTcgPpiGuid,
> 
> +             0,
> 
> +             NULL,
> 
> +             (VOID**)&TcgPpi
> 
> +             );
> 
> +  if (EFI_ERROR(Status)) {
> 
> +    return Status;
> 
> +  }
> 
> +
> 
> +  TcgEventHdr.PCRIndex  = PcrIndex;
> 
> +  TcgEventHdr.EventType = EventType;
> 
> +  TcgEventHdr.EventSize = LogLen;
> 
> +
> 
> +  Status = TcgPpi->HashLogExtendEvent (
> 
> +                     TcgPpi,
> 
> +                     Flags,
> 
> +                     HashData,
> 
> +                     (UINTN)HashDataLen,
> 
> +                     &TcgEventHdr,
> 
> +                     EventLog
> 
> +                     );
> 
> +  return Status;
> 
> +}
> 
> +
> 
> +/**
> 
> +  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
> 
> +**/
> 
> +STATIC
> 
> +VOID *
> 
> +TpmMeasurementGetFvName (
> 
> +  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->Signature != EFI_FVH_SIGNATURE) {
> 
> +    return NULL;
> 
> +  }
> 
> +  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;
> 
> +}
> 
> +
> 
> +/**
> 
> +  Mesure a FSP FirmwareBlob.

'Mesure' -> 'Measure'

> 
> +
> 
> +  @param[in]  Descrption              Description for this FirmwareBlob.

'Descrption' -> 'Description'

> 
> +  @param[in]  FirmwareBlobBase        Base address of this FirmwareBlob.
> 
> +  @param[in]  FirmwareBlobLength      Size in bytes of this FirmwareBlob.
> 
> +  @param[in]  CfgRegionOffset         Configuration region offset in bytes.
> 
> +  @param[in]  CfgRegionSize           Configuration region in bytes.
> 
> +
> 
> +  @retval EFI_SUCCESS           Operation completed successfully.
> 
> +  @retval EFI_UNSUPPORTED       TPM device not available.
> 
> +  @retval EFI_OUT_OF_RESOURCES  Out of memory.
> 
> +  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
> 
> +*/
> 
> +STATIC
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +MeasureFspFirmwareBlobWithCfg (
> 
> +  IN CHAR8                          *Description OPTIONAL,
> 
> +  IN EFI_PHYSICAL_ADDRESS           FirmwareBlobBase,
> 
> +  IN UINT64                         FirmwareBlobLength,
> 
> +  IN UINT32                         CfgRegionOffset,
> 
> +  IN UINT32                         CfgRegionSize
> 
> +  )
> 
> +{
> 
> +  EFI_PLATFORM_FIRMWARE_BLOB        FvBlob, UPDBlob;
> 
> +  PLATFORM_FIRMWARE_BLOB2_STRUCT    FvBlob2, UPDBlob2;
> 
> +  VOID                              *FvName;
> 
> +  UINT32                            FvEventType;
> 
> +  VOID                              *FvEventLog, *UPDEventLog;
> 
> +  UINT32                            FvEventLogSize, UPDEventLogSize;
> 
> +  EFI_STATUS                        Status;
> 
> +  HASH_HANDLE                       HashHandle;
> 
> +  UINT8                             *HashBase;
> 
> +  UINTN                             HashSize;
> 
> +  TPML_DIGEST_VALUES                DigestList;
> 
> +
> 
> +  FvName = TpmMeasurementGetFvName (FirmwareBlobBase,
> FirmwareBlobLength);
> 
> +
> 
> +  if (((Description != NULL) || (FvName != NULL)) &&
> 
> +      (PcdGet32(PcdTcgPfpMeasurementRevision) >=
> TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105)) {
> 
> +    ZeroMem (&FvBlob2, sizeof(FvBlob2));
> 
> +    ZeroMem (&UPDBlob2, sizeof(UPDBlob2));
> 

The same as patch3&4, I think it's not necessary to clear the data
in advance.

> +    if (Description != NULL) {
> 
> +      AsciiSPrint((CHAR8*)FvBlob2.BlobDescription,
> sizeof(FvBlob2.BlobDescription), "%a", Description);
> 
> +      AsciiSPrint((CHAR8*)UPDBlob2.BlobDescription,
> sizeof(UPDBlob2.BlobDescription), "%aUDP", Description);
> 
> +     } else {
> 
> +      AsciiSPrint((CHAR8*)FvBlob2.BlobDescription,
> sizeof(FvBlob2.BlobDescription), "Fv(%g)", FvName);
> 
> +      AsciiSPrint((CHAR8*)UPDBlob2.BlobDescription,
> sizeof(UPDBlob2.BlobDescription), "(%g)UDP", FvName);
> 
> +    }
> 
> +
> 
> +    FvBlob2.BlobDescriptionSize = sizeof(FvBlob2.BlobDescription);
> 
> +    FvBlob2.BlobBase = FirmwareBlobBase;
> 
> +    FvBlob2.BlobLength = FirmwareBlobLength;
> 
> +    FvEventType = EV_EFI_PLATFORM_FIRMWARE_BLOB2;
> 
> +    FvEventLog = &FvBlob2;
> 
> +    FvEventLogSize = sizeof(FvBlob2);
> 
> +
> 
> +    UPDBlob2.BlobDescriptionSize = sizeof(UPDBlob2.BlobDescription);
> 
> +    UPDBlob2.BlobBase = CfgRegionOffset;
> 
> +    UPDBlob2.BlobLength = CfgRegionSize;
> 
> +    UPDEventLog = &UPDBlob2;
> 
> +    UPDEventLogSize = sizeof(UPDBlob2);
> 
> +  } else {
> 
> +    FvBlob.BlobBase = FirmwareBlobBase;
> 
> +    FvBlob.BlobLength = FirmwareBlobLength;
> 
> +    FvEventType = EV_EFI_PLATFORM_FIRMWARE_BLOB;
> 
> +    FvEventLog = &FvBlob;
> 
> +    FvEventLogSize = sizeof(FvBlob);
> 
> +
> 
> +    UPDBlob.BlobBase = CfgRegionOffset;
> 
> +    UPDBlob.BlobLength = CfgRegionSize;
> 
> +    UPDEventLog = &UPDBlob;
> 
> +    UPDEventLogSize = sizeof(UPDBlob);
> 
> +  }
> 
> +
> 
> +  // Initialize a SHA hash context.
> 
> +  Status = HashStart (&HashHandle);
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    DEBUG ((DEBUG_ERROR, "HashStart failed - %r\n", Status));
> 
> +    return Status;
> 
> +  }
> 
> +
> 
> +  // Hash FSP binary before UDP
> 
> +  HashBase = (UINT8 *) (UINTN) FirmwareBlobBase;
> 
> +  HashSize = (UINTN) CfgRegionOffset;
> 
> +  Status = HashUpdate (HashHandle, HashBase, HashSize);
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    DEBUG ((DEBUG_ERROR, "HashUpdate failed - %r\n", Status));
> 
> +    return Status;
> 
> +  }
> 
> +
> 
> +  // Hash FSP binary after UDP
> 
> +  HashBase = (UINT8 *) (UINTN) FirmwareBlobBase + CfgRegionOffset +
> CfgRegionSize;
> 
> +  HashSize = (UINTN)(FirmwareBlobLength - CfgRegionOffset - CfgRegionSize);
> 
> +  Status = HashUpdate (HashHandle, HashBase, HashSize);
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    DEBUG ((DEBUG_ERROR, "HashUpdate failed - %r\n", Status));
> 
> +    return Status;
> 
> +  }
> 
> +
> 
> +  // Finalize the SHA hash.
> 
> +  Status = HashCompleteAndExtend (HashHandle, 0, NULL, 0, &DigestList);
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    DEBUG ((DEBUG_ERROR, "HashCompleteAndExtend failed - %r\n", Status));
> 
> +    return Status;
> 
> +  }
> 
> +
> 
> +  Status = TpmMeasureAndLogDataWithFlags (
> 
> +             0,
> 
> +             FvEventType,
> 
> +             FvEventLog,
> 
> +             FvEventLogSize,
> 
> +             (UINT8 *) &DigestList,
> 
> +             (UINTN) sizeof(DigestList),
> 
> +             EDKII_TCG_PRE_HASH_LOG_ONLY
> 
> +             );
> 
> +
> 
> +  Status = TpmMeasureAndLogData (
> 
> +             1,
> 
> +             EV_PLATFORM_CONFIG_FLAGS,
> 
> +             UPDEventLog,
> 
> +             UPDEventLogSize,
> 
> +             (UINT8 *) (UINTN) FirmwareBlobBase + CfgRegionOffset,
> 
> +             CfgRegionSize
> 
> +             );
> 
> +
> 
> +  return Status;
> 
> +}
> 
> +
> 
> +FSP_INFO_HEADER *
> 
> +EFIAPI
> 
> +mFspFindFspHeader (
> 
> +  IN EFI_PHYSICAL_ADDRESS  FlashFvFspBase
> 
> +  )
> 
> +{
> 
> +  UINT8 *CheckPointer;
> 
> +
> 
> +  CheckPointer = (UINT8 *) (UINTN) FlashFvFspBase;
> 
> +
> 
> +  if (((EFI_FIRMWARE_VOLUME_HEADER *)CheckPointer)->Signature !=
> EFI_FVH_SIGNATURE) {
> 
> +    return NULL;
> 
> +  }
> 
> +
> 
> +  if (((EFI_FIRMWARE_VOLUME_HEADER *)CheckPointer)->ExtHeaderOffset !=
> 0) {
> 
> +    CheckPointer = CheckPointer + ((EFI_FIRMWARE_VOLUME_HEADER
> *)CheckPointer)->ExtHeaderOffset;
> 
> +    CheckPointer = CheckPointer + ((EFI_FIRMWARE_VOLUME_EXT_HEADER
> *)CheckPointer)->ExtHeaderSize;
> 
> +    CheckPointer = (UINT8 *) ALIGN_POINTER (CheckPointer, 8);
> 
> +  } else {
> 
> +    CheckPointer = CheckPointer + ((EFI_FIRMWARE_VOLUME_HEADER
> *)CheckPointer)->HeaderLength;
> 
> +  }
> 
> +
> 
> +
> 
> +  CheckPointer = CheckPointer + sizeof (EFI_FFS_FILE_HEADER);
> 
> +
> 
> +  if (((EFI_RAW_SECTION *)CheckPointer)->Type != EFI_SECTION_RAW) {
> 
> +    return NULL;
> 
> +  }
> 
> +
> 
> +  CheckPointer = CheckPointer + sizeof (EFI_RAW_SECTION);
> 
> +
> 
> +  return (FSP_INFO_HEADER *)CheckPointer;
> 
> +}
> 
> +/**
> 
> +  Mesure a FSP FirmwareBlob.

'Mesure' -> ' Measure'

> 
> +
> 
> +  @param[in]  PcrIndex                PCR Index.
> 
> +  @param[in]  Descrption              Description for this FirmwareBlob.
> 
> +  @param[in]  FirmwareBlobBase        Base address of this FirmwareBlob.
> 
> +  @param[in]  FirmwareBlobLength      Size in bytes of this FirmwareBlob.
> 
> +
> 
> +  @retval EFI_SUCCESS           Operation completed successfully.
> 
> +  @retval EFI_UNSUPPORTED       TPM device not available.
> 
> +  @retval EFI_OUT_OF_RESOURCES  Out of memory.
> 
> +  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
> 
> +*/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +MeasureFspFirmwareBlob (
> 
> +  IN UINT32                         PcrIndex,
> 
> +  IN CHAR8                          *Description OPTIONAL,
> 
> +  IN EFI_PHYSICAL_ADDRESS           FirmwareBlobBase,
> 
> +  IN UINT64                         FirmwareBlobLength
> 
> +  )
> 
> +{
> 
> +  UINT32           FspMeasureMask;
> 
> +  FSP_INFO_HEADER  *FspHeaderPtr;
> 
> +
> 
> +  FspMeasureMask = PcdGet32 (PcdFspMeasurementConfig);
> 
> +  if (FspMeasureMask & FSP_MEASURE_FSPUPD) {
> 
> +    FspHeaderPtr = (FSP_INFO_HEADER *) mFspFindFspHeader
> (FirmwareBlobBase);
> 
> +    if (FspHeaderPtr == NULL) {
> 
> +      return MeasureFirmwareBlob (PcrIndex, Description, FirmwareBlobBase,
> FirmwareBlobLength);;
> 
> +    }
> 
> +    return MeasureFspFirmwareBlobWithCfg(Description, FirmwareBlobBase,
> FirmwareBlobLength,
> 
> +                                         FspHeaderPtr->CfgRegionOffset, FspHeaderPtr-
> >CfgRegionSize);
> 
> +  } else {
> 
> +    return MeasureFirmwareBlob (PcrIndex, Description, FirmwareBlobBase,
> FirmwareBlobLength);
> 
> +  }
> 
> +}
> 
> +
> 
> --
> 2.26.2.windows.1
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> 
> View/Reply Online (#63757): https://edk2.groups.io/g/devel/message/63757
> Mute This Topic: https://groups.io/mt/76019588/1768734
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub  [jian.j.wang@intel.com]
> -=-=-=-=-=-=


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 9/9] SecurityPkg/Tcg2: handle PRE HASH and LOG ONLY
  2020-08-06  0:33 ` [PATCH v2 9/9] SecurityPkg/Tcg2: handle PRE HASH and LOG ONLY Qi Zhang
  2020-08-11  0:19   ` [edk2-devel] " Liming Gao
@ 2020-08-12  2:56   ` Wang, Jian J
  1 sibling, 0 replies; 23+ messages in thread
From: Wang, Jian J @ 2020-08-12  2:56 UTC (permalink / raw)
  To: Zhang, Qi1, devel@edk2.groups.io; +Cc: Yao, Jiewen, Kumar, Rahul1



Reviewed-by: Jian J Wang <jian.j.wang@intel.com>

Regards,
Jian

> -----Original Message-----
> From: Zhang, Qi1 <qi1.zhang@intel.com>
> Sent: Thursday, August 06, 2020 8:34 AM
> To: devel@edk2.groups.io
> Cc: Zhang, Qi1 <qi1.zhang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
> Subject: [PATCH v2 9/9] SecurityPkg/Tcg2: handle PRE HASH and LOG ONLY
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2376
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Qi Zhang <qi1.zhang@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Qi Zhang <qi1.zhang@intel.com>
> ---
>  SecurityPkg/Include/Ppi/Tcg.h     |  5 +++++
>  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 12 +++++++-----
>  2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/SecurityPkg/Include/Ppi/Tcg.h b/SecurityPkg/Include/Ppi/Tcg.h
> index 0e943f2465..22f47f9817 100644
> --- a/SecurityPkg/Include/Ppi/Tcg.h
> +++ b/SecurityPkg/Include/Ppi/Tcg.h
> @@ -18,6 +18,11 @@ typedef struct _EDKII_TCG_PPI EDKII_TCG_PPI;
>  //
> 
>  #define EDKII_TCG_PRE_HASH  0x0000000000000001
> 
> 
> 
> +//
> 
> +// This bit is shall be set when HashData is the pre-hash digest and log only.
> 
> +//
> 
> +#define EDKII_TCG_PRE_HASH_LOG_ONLY  0x0000000000000002
> 
> +
> 
>  /**
> 
>    Tpm measure and log data, and extend the measurement result into a specific
> PCR.
> 
> 
> 
> diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> index 246968bb7f..b56b03746c 100644
> --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> @@ -453,13 +453,15 @@ HashLogExtendEvent (
>      return EFI_DEVICE_ERROR;
> 
>    }
> 
> 
> 
> -  if(Flags & EDKII_TCG_PRE_HASH) {
> 
> +  if ((Flags & EDKII_TCG_PRE_HASH) || (Flags &
> EDKII_TCG_PRE_HASH_LOG_ONLY)) {
> 
>      ZeroMem (&DigestList, sizeof(DigestList));
> 
>      CopyMem (&DigestList, HashData, sizeof(DigestList));
> 
> -    Status = Tpm2PcrExtend (
> 
> -             0,
> 
> -             &DigestList
> 
> -             );
> 
> +    if (Flags & EDKII_TCG_PRE_HASH) {
> 
> +      Status = Tpm2PcrExtend (
> 
> +               NewEventHdr->PCRIndex,
> 
> +               &DigestList
> 
> +               );
> 
> +    }
> 
>    } else {
> 
>      Status = HashAndExtend (
> 
>                 NewEventHdr->PCRIndex,
> 
> --
> 2.26.2.windows.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2020-08-12  2:56 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-06  0:33 [PATCH v2 0/9] Need add a FSP binary measurement Qi Zhang
2020-08-06  0:33 ` [PATCH v2 1/9] MdeModulePkg/TpmMeasurementLib: Add new API to TpmMeasurmentLib Qi Zhang
2020-08-12  1:30   ` Wang, Jian J
2020-08-06  0:33 ` [PATCH v2 2/9] MdeModulePkg/NullTpmMeasurementLib: Add new API Qi Zhang
2020-08-12  1:35   ` Wang, Jian J
2020-08-06  0:33 ` [PATCH v2 3/9] SecurityPkg/DxeTpmMeasurementLib: " Qi Zhang
2020-08-12  2:12   ` [edk2-devel] " Wang, Jian J
2020-08-06  0:33 ` [PATCH v2 4/9] SecurityPkg/PeiTpmMeasurementLib: " Qi Zhang
2020-08-12  2:14   ` Wang, Jian J
2020-08-06  0:33 ` [PATCH v2 5/9] IntelFsp2WrapperPkg/FspMeasurementLib: Add header file Qi Zhang
2020-08-12  2:15   ` [edk2-devel] " Wang, Jian J
2020-08-06  0:33 ` [PATCH v2 6/9] IntelFsp2WrapperPkg/FspMeasurementLib: Add BaseFspMeasurementLib Qi Zhang
2020-08-06  1:09   ` Chiu, Chasel
2020-08-12  2:48   ` [edk2-devel] " Wang, Jian J
2020-08-06  0:33 ` [PATCH v2 7/9] IntelFsp2WraperPkg/Fsp{m|s}WrapperPeim: Add FspBin measurement Qi Zhang
2020-08-06  0:33 ` [PATCH v2 8/9] IntelFsp2Wrapper/dsc: Add FspTpmMeasurementLib and PcdFspMeasurementConfig Qi Zhang
2020-08-06  0:33 ` [PATCH v2 9/9] SecurityPkg/Tcg2: handle PRE HASH and LOG ONLY Qi Zhang
2020-08-11  0:19   ` [edk2-devel] " Liming Gao
2020-08-11  0:53     ` Qi Zhang
2020-08-11  1:20       ` Liming Gao
2020-08-12  2:56   ` Wang, Jian J
2020-08-11  2:00 ` [PATCH v2 0/9] Need add a FSP binary measurement Yao, Jiewen
2020-08-11 16:25 ` Wang, Jian J

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox