public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH V4 0/3] Introduce CcMeasurementProtocol into EDK2
@ 2021-11-02  2:50 Min Xu
  2021-11-02  2:50 ` [PATCH V4 1/3] MdePkg: Introduce CcMeasurementProtocol for CC Guest firmware Min Xu
                   ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Min Xu @ 2021-11-02  2:50 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Michael D Kinney, Liming Gao, Zhiguang Liu, Jiewen Yao,
	Jian J Wang, Ken Lu, Sami Mujawar, Gerd Hoffmann

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3625

If Confidential Computing (Cc) firmware supports measurement and an
event is created, CC-Guest firmware is designed to report the event
log with the same data structure in TCG-Platform-Firmware-Profile
specification with EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 format.

The CC-Guest firmware supports measurement. It is designed to produce
EFI_CC_MEASUREMENT_PROTOCOL with new GUID
EFI_CC_MEASUREMENT_PROTOCOL_GUID to report event log and provides
hash capability.

Patch #1:
Introduce the CC Measurement Protocol definition into MdePkg.

Patch #2:
Update DxeTpm2MeasureBootLib to support CC based measure boot.

Patch #3:
Update DxeTpmMeasurementLib to support CC based measurement.

Code is at https://github.com/mxu9/edk2/tree/td_protocol.v4

v4 changes:
 - Rename TeeMeasurementProtocol to CcMeasurementProtocol based
   on the discussion in below links:
   https://edk2.groups.io/g/devel/message/82876
   https://edk2.groups.io/g/devel/message/82999
   https://edk2.groups.io/g/devel/message/83000
   With this protocol, CC based measure boot is supported.
   TD based measure boot is one of the CC based measure boot.
 - The spec will be updated according to the changes later.
 - TdProtocol.h is deleted. Its content is merged into CcMeasurement.h.
 - Add gEfiCcFinalEventsTableGuid definition in MdePkg.dec
 - Update the description in DxeTpm2MeasureBootLib.inf
   and DxeTpmMeasurementLib.inf

v3 changes:
 - Rename TdProtocol to TeeMeasurementProtocol which is a neutral name.
   With this protocol, TEE based measure boot is supported.
   TD based measure boot is one of the TEE based measure boot.
 - The spec will be updated according to the changes later.
 - Fix errors in DxeTpm2MeasureBootLib.

v2 changes:
 - TD based measure boot is implemented in DxeTpm2MeasureBootLib.
   This minimize the code changes.
 - TD based measurement is added. It is implemented in
   DxeTpmMeasurementLib.
 - Fix the typo in comments.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ken Lu <ken.lu@intel.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>

Min Xu (3):
  MdePkg: Introduce CcMeasurementProtocol for CC Guest firmware
  SecurityPkg: Support CcMeasurementProtocol in DxeTpm2MeasureBootLib
  SecurityPkg: Support CcMeasurementProtocol in DxeTpmMeasurementLib

 MdePkg/Include/Protocol/CcMeasurement.h       | 305 +++++++++++++++
 MdePkg/MdePkg.dec                             |   4 +
 .../DxeTpm2MeasureBootLib.c                   | 366 ++++++++++++++----
 .../DxeTpm2MeasureBootLib.inf                 |   3 +-
 .../DxeTpmMeasurementLib.c                    |  91 ++++-
 .../DxeTpmMeasurementLib.inf                  |   9 +-
 6 files changed, 700 insertions(+), 78 deletions(-)
 create mode 100644 MdePkg/Include/Protocol/CcMeasurement.h

-- 
2.29.2.windows.2


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

* [PATCH V4 1/3] MdePkg: Introduce CcMeasurementProtocol for CC Guest firmware
  2021-11-02  2:50 [PATCH V4 0/3] Introduce CcMeasurementProtocol into EDK2 Min Xu
@ 2021-11-02  2:50 ` Min Xu
  2021-11-02  6:24   ` Yao, Jiewen
                     ` (2 more replies)
  2021-11-02  2:50 ` [PATCH V4 2/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpm2MeasureBootLib Min Xu
  2021-11-02  2:50 ` [PATCH V4 3/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpmMeasurementLib Min Xu
  2 siblings, 3 replies; 23+ messages in thread
From: Min Xu @ 2021-11-02  2:50 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Michael D Kinney, Liming Gao, Zhiguang Liu, Jiewen Yao,
	Jian J Wang, Ken Lu, Sami Mujawar, Gerd Hoffmann

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3625

CC guest is a Confidential Computing guest. If CC Guest firmware
supports measurement and an event is created, CC Guest firmware
is designed to report the event log with the same data structure
in TCG-Platform-Firmware-Profile specification with
EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 format.

The CC Guest firmware supports measurement. It is designed to
produce EFI_CC_MEASUREMENT_PROTOCOL with new GUID
EFI_CC_MEASUREMENT_PROTOCOL_GUID to report event log and provides
hash capability.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ken Lu <ken.lu@intel.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 MdePkg/Include/Protocol/CcMeasurement.h | 305 ++++++++++++++++++++++++
 MdePkg/MdePkg.dec                       |   4 +
 2 files changed, 309 insertions(+)
 create mode 100644 MdePkg/Include/Protocol/CcMeasurement.h

diff --git a/MdePkg/Include/Protocol/CcMeasurement.h b/MdePkg/Include/Protocol/CcMeasurement.h
new file mode 100644
index 000000000000..eaedbfffdb6a
--- /dev/null
+++ b/MdePkg/Include/Protocol/CcMeasurement.h
@@ -0,0 +1,305 @@
+/** @file
+  If CC Guest firmware supports measurement and an event is created,
+  CC Guest firmware is designed to report the event log with the same
+  data structure in TCG-Platform-Firmware-Profile specification with
+  EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 format.
+
+  The CC Guest firmware supports measurement, the CC Guest Firmware is
+  designed to produce EFI_CC_MEASUREMENT_PROTOCOL with new GUID
+  EFI_CC_MEASUREMENT_PROTOCOL_GUID to report event log and provides hash
+  capability.
+
+Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef CC_MEASUREMENT_PROTOCOL_H_
+#define CC_MEASUREMENT_PROTOCOL_H_
+
+#include <IndustryStandard/UefiTcgPlatform.h>
+
+#define EFI_CC_MEASUREMENT_PROTOCOL_GUID  \
+  { 0x96751a3d, 0x72f4, 0x41a6, { 0xa7, 0x94, 0xed, 0x5d, 0x0e, 0x67, 0xae, 0x6b }}
+extern EFI_GUID gEfiCcMeasurementProtocolGuid;
+
+typedef struct _EFI_CC_MEASUREMENT_PROTOCOL EFI_CC_MEASUREMENT_PROTOCOL;
+
+typedef struct {
+  UINT8 Major;
+  UINT8 Minor;
+} EFI_CC_VERSION;
+
+//
+// EFI_CC Type/SubType definition
+//
+#define EFI_CC_TYPE_NONE   0
+#define EFI_CC_TYPE_SEV    1
+#define EFI_CC_TYPE_TDX    2
+
+typedef struct {
+  UINT8          Type;
+  UINT8          SubType;
+} EFI_CC_TYPE;
+
+typedef UINT32                      EFI_CC_EVENT_LOG_BITMAP;
+typedef UINT32                      EFI_CC_EVENT_LOG_FORMAT;
+typedef UINT32                      EFI_CC_EVENT_ALGORITHM_BITMAP;
+typedef UINT32                      EFI_CC_MR_INDEX;
+
+//
+// Intel TDX measure register index
+//
+#define TDX_MR_INDEX_MRTD  0
+#define TDX_MR_INDEX_RTMR0 1
+#define TDX_MR_INDEX_RTMR1 2
+#define TDX_MR_INDEX_RTMR2 3
+#define TDX_MR_INDEX_RTMR3 4
+
+
+#define EFI_CC_EVENT_LOG_FORMAT_TCG_2   0x00000002
+#define EFI_CC_BOOT_HASH_ALG_SHA384     0x00000004
+
+//
+// This bit is shall be set when an event shall be extended but not logged.
+//
+#define EFI_CC_FLAG_EXTEND_ONLY       0x0000000000000001
+//
+// This bit shall be set when the intent is to measure a PE/COFF image.
+//
+#define EFI_CC_FLAG_PE_COFF_IMAGE     0x0000000000000010
+
+#pragma pack (1)
+
+#define EFI_CC_EVENT_HEADER_VERSION   1
+
+typedef struct {
+  //
+  // Size of the event header itself (sizeof(EFI_CC_EVENT_HEADER)).
+  //
+  UINT32            HeaderSize;
+  //
+  // Header version. For this version of this specification, the value shall be 1.
+  //
+  UINT16            HeaderVersion;
+  //
+  // Index of the MR (measurement register) that shall be extended.
+  //
+  EFI_CC_MR_INDEX   MrIndex;
+  //
+  // Type of the event that shall be extended (and optionally logged).
+  //
+  UINT32            EventType;
+} EFI_CC_EVENT_HEADER;
+
+typedef struct {
+  //
+  // Total size of the event including the Size component, the header and the Event data.
+  //
+  UINT32                  Size;
+  EFI_CC_EVENT_HEADER     Header;
+  UINT8                   Event[1];
+} EFI_CC_EVENT;
+
+#pragma pack()
+
+
+typedef struct {
+  //
+  // Allocated size of the structure
+  //
+  UINT8                            Size;
+  //
+  // Version of the EFI_CC_BOOT_SERVICE_CAPABILITY structure itself.
+  // For this version of the protocol, the Major version shall be set to 1
+  // and the Minor version shall be set to 0.
+  //
+  EFI_CC_VERSION                   StructureVersion;
+  //
+  // Version of the EFI CC Measurement protocol.
+  // For this version of the protocol, the Major version shall be set to 1
+  // and the Minor version shall be set to 0.
+  //
+  EFI_CC_VERSION                   ProtocolVersion;
+  //
+  // Supported hash algorithms
+  //
+  EFI_CC_EVENT_ALGORITHM_BITMAP   HashAlgorithmBitmap;
+  //
+  // Bitmap of supported event log formats
+  //
+  EFI_CC_EVENT_LOG_BITMAP          SupportedEventLogs;
+
+  //
+  // Indicates the CC type
+  //
+  EFI_CC_TYPE                      CcType;
+} EFI_CC_BOOT_SERVICE_CAPABILITY;
+
+/**
+  The EFI_CC_MEASUREMENT_PROTOCOL GetCapability function call provides protocol
+  capability information and state information.
+
+  @param[in]      This               Indicates the calling context
+  @param[in, out] ProtocolCapability The caller allocates memory for a EFI_CC_BOOT_SERVICE_CAPABILITY
+                                     structure and sets the size field to the size of the structure allocated.
+                                     The callee fills in the fields with the EFI CC BOOT Service capability
+                                     information and the current CC information.
+
+  @retval EFI_SUCCESS            Operation completed successfully.
+  @retval EFI_DEVICE_ERROR       The command was unsuccessful.
+                                 The ProtocolCapability variable will not be populated.
+  @retval EFI_INVALID_PARAMETER  One or more of the parameters are incorrect.
+                                 The ProtocolCapability variable will not be populated.
+  @retval EFI_BUFFER_TOO_SMALL   The ProtocolCapability variable is too small to hold the full response.
+                                 It will be partially populated (required Size field will be set).
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EFI_CC_GET_CAPABILITY) (
+  IN     EFI_CC_MEASUREMENT_PROTOCOL       *This,
+  IN OUT EFI_CC_BOOT_SERVICE_CAPABILITY    *ProtocolCapability
+  );
+
+/**
+  The EFI_CC_MEASUREMENT_PROTOCOL Get Event Log function call allows a caller to
+  retrieve the address of a given event log and its last entry.
+
+  @param[in]  This               Indicates the calling context
+  @param[in]  EventLogFormat     The type of the event log for which the information is requested.
+  @param[out] EventLogLocation   A pointer to the memory address of the event log.
+  @param[out] EventLogLastEntry  If the Event Log contains more than one entry, this is a pointer to the
+                                 address of the start of the last entry in the event log in memory.
+  @param[out] EventLogTruncated  If the Event Log is missing at least one entry because an event would
+                                 have exceeded the area allocated for events, this value is set to TRUE.
+                                 Otherwise, the value will be FALSE and the Event Log will be complete.
+
+  @retval EFI_SUCCESS            Operation completed successfully.
+  @retval EFI_INVALID_PARAMETER  One or more of the parameters are incorrect
+                                 (e.g. asking for an event log whose format is not supported).
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EFI_CC_GET_EVENT_LOG) (
+  IN  EFI_CC_MEASUREMENT_PROTOCOL     *This,
+  IN  EFI_CC_EVENT_LOG_FORMAT         EventLogFormat,
+  OUT EFI_PHYSICAL_ADDRESS            *EventLogLocation,
+  OUT EFI_PHYSICAL_ADDRESS            *EventLogLastEntry,
+  OUT BOOLEAN                         *EventLogTruncated
+  );
+
+/**
+  The EFI_CC_MEASUREMENT_PROTOCOL HashLogExtendEvent function call provides
+  callers with an opportunity to extend and optionally log events without requiring
+  knowledge of actual CC commands.
+  The extend operation will occur even if this function cannot create an event
+  log entry (e.g. due to the event log being full).
+
+  @param[in]  This               Indicates the calling context
+  @param[in]  Flags              Bitmap providing additional information.
+  @param[in]  DataToHash         Physical address of the start of the data buffer to be hashed.
+  @param[in]  DataToHashLen      The length in bytes of the buffer referenced by DataToHash.
+  @param[in]  EfiCcEvent        Pointer to data buffer containing information about the event.
+
+  @retval EFI_SUCCESS            Operation completed successfully.
+  @retval EFI_DEVICE_ERROR       The command was unsuccessful.
+  @retval EFI_VOLUME_FULL        The extend operation occurred, but the event could not be written to one or more event logs.
+  @retval EFI_INVALID_PARAMETER  One or more of the parameters are incorrect.
+  @retval EFI_UNSUPPORTED        The PE/COFF image type is not supported.
+**/
+typedef
+EFI_STATUS
+(EFIAPI * EFI_CC_HASH_LOG_EXTEND_EVENT) (
+  IN EFI_CC_MEASUREMENT_PROTOCOL    *This,
+  IN UINT64                         Flags,
+  IN EFI_PHYSICAL_ADDRESS           DataToHash,
+  IN UINT64                         DataToHashLen,
+  IN EFI_CC_EVENT                   *EfiCcEvent
+  );
+
+/**
+  The EFI_CC_MEASUREMENT_PROTOCOL MapPcrToMrIndex function call provides callers
+  the info on TPM PCR <-> CC MR mapping information.
+
+  @param[in]  This               Indicates the calling context
+  @param[in]  PcrIndex           TPM PCR index.
+  @param[out] MrIndex            CC MR index.
+
+  @retval EFI_SUCCESS            The MrIndex is returned.
+  @retval EFI_INVALID_PARAMETER  The MrIndex is NULL.
+  @retval EFI_UNSUPPORTED        The PcrIndex is invalid.
+**/
+typedef
+EFI_STATUS
+(EFIAPI * EFI_CC_MAP_PCR_TO_MR_INDEX) (
+  IN  EFI_CC_MEASUREMENT_PROTOCOL    *This,
+  IN  TCG_PCRINDEX                   PcrIndex,
+  OUT EFI_CC_MR_INDEX                *MrIndex
+  );
+
+struct _EFI_CC_MEASUREMENT_PROTOCOL {
+  EFI_CC_GET_CAPABILITY                     GetCapability;
+  EFI_CC_GET_EVENT_LOG                      GetEventLog;
+  EFI_CC_HASH_LOG_EXTEND_EVENT              HashLogExtendEvent;
+  EFI_CC_MAP_PCR_TO_MR_INDEX                MapPcrToMrIndex;
+};
+
+//
+// CC event log
+//
+
+#pragma pack(1)
+
+//
+// Crypto Agile Log Entry Format.
+// It is similar with TCG_PCR_EVENT2 except the field of MrIndex and PCRIndex.
+//
+typedef struct {
+  EFI_CC_MR_INDEX     MrIndex;
+  UINT32              EventType;
+  TPML_DIGEST_VALUES  Digests;
+  UINT32              EventSize;
+  UINT8               Event[1];
+} CC_EVENT;
+
+//
+// EFI CC Event Header
+// It is similar with TCG_PCR_EVENT2_HDR except the field of MrIndex and PCRIndex
+//
+typedef struct {
+  EFI_CC_MR_INDEX     MrIndex;
+  UINT32              EventType;
+  TPML_DIGEST_VALUES  Digests;
+  UINT32              EventSize;
+} CC_EVENT_HDR;
+
+#pragma pack()
+
+//
+// Log entries after Get Event Log service
+//
+
+#define EFI_CC_FINAL_EVENTS_TABLE_VERSION  1
+
+typedef struct {
+  //
+  // The version of this structure. It shall be set to 1.
+  //
+  UINT64                  Version;
+  //
+  // Number of events recorded after invocation of GetEventLog API
+  //
+  UINT64                  NumberOfEvents;
+  //
+  // List of events of type CC_EVENT.
+  //
+  //CC_EVENT              Event[1];
+} EFI_CC_FINAL_EVENTS_TABLE;
+
+
+#define EFI_CC_FINAL_EVENTS_TABLE_GUID \
+  {0xdd4a4648, 0x2de7, 0x4665, {0x96, 0x4d, 0x21, 0xd9, 0xef, 0x5f, 0xb4, 0x46}}
+
+extern EFI_GUID gEfiCcFinalEventsTableGuid;
+
+#endif
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 8b18415b107a..8e19be662ada 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -1011,6 +1011,10 @@
   ## Include/Protocol/PcdInfo.h
   gGetPcdInfoProtocolGuid        = { 0x5be40f57, 0xfa68, 0x4610, { 0xbb, 0xbf, 0xe9, 0xc5, 0xfc, 0xda, 0xd3, 0x65 } }
 
+  ## Include/Protocol/CcMeasurement.h
+  gEfiCcMeasurementProtocolGuid  = { 0x96751a3d, 0x72f4, 0x41a6, { 0xa7, 0x94, 0xed, 0x5d, 0x0e, 0x67, 0xae, 0x6b }}
+  gEfiCcFinalEventsTableGuid     = { 0xdd4a4648, 0x2de7, 0x4665, { 0x96, 0x4d, 0x21, 0xd9, 0xef, 0x5f, 0xb4, 0x46 }}
+
   #
   # Protocols defined in PI1.0.
   #
-- 
2.29.2.windows.2


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

* [PATCH V4 2/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpm2MeasureBootLib
  2021-11-02  2:50 [PATCH V4 0/3] Introduce CcMeasurementProtocol into EDK2 Min Xu
  2021-11-02  2:50 ` [PATCH V4 1/3] MdePkg: Introduce CcMeasurementProtocol for CC Guest firmware Min Xu
@ 2021-11-02  2:50 ` Min Xu
  2021-11-02  6:24   ` Yao, Jiewen
  2021-11-02  9:43   ` Sami Mujawar
  2021-11-02  2:50 ` [PATCH V4 3/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpmMeasurementLib Min Xu
  2 siblings, 2 replies; 23+ messages in thread
From: Min Xu @ 2021-11-02  2:50 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Michael D Kinney, Liming Gao, Zhiguang Liu, Jiewen Yao,
	Jian J Wang, Sami Mujawar, Gerd Hoffmann

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3625

DxeTpm2MeasureBootLib supports TPM2 based measure boot. After
CcMeasurementProtocol is introduced, CC based measure boot needs to
be supported in DxeTpm2MeasureBootLib as well.

There are 2 major changes in this commit.

1. MEASURE_BOOT_PROTOCOLS is defined to store the instances of TCG2
protocol and TEE protocol. In the DxeTpm2MeasureBootHandler above 2
measure boot protocol instances will be located. Then the located
protocol instances will be called to do the measure boot.

2. CcEvent is similar to Tcg2Event except the MrIndex and PcrIndex.
CreateCcEventFromTcg2Event is used to create the CcEvent based on the
Tcg2Event.

Above 2 changes make the minimize changes to the existing code.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 .../DxeTpm2MeasureBootLib.c                   | 366 ++++++++++++++----
 .../DxeTpm2MeasureBootLib.inf                 |   3 +-
 2 files changed, 299 insertions(+), 70 deletions(-)

diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
index 92eac715800f..af889b6ed3ed 100644
--- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
+++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
@@ -1,5 +1,6 @@
 /** @file
-  The library instance provides security service of TPM2 measure boot.
+  The library instance provides security service of TPM2 measure boot and
+  Confidential Computing (CC) measure boot.
 
   Caution: This file requires additional review when modified.
   This library will have external input - PE/COFF image and GPT partition.
@@ -41,6 +42,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/PeCoffLib.h>
 #include <Library/SecurityManagementLib.h>
 #include <Library/HobLib.h>
+#include <Protocol/CcMeasurement.h>
+
+typedef struct {
+  EFI_TCG2_PROTOCOL             *Tcg2Protocol;
+  EFI_CC_MEASUREMENT_PROTOCOL   *CcProtocol;
+} MEASURE_BOOT_PROTOCOLS;
 
 //
 // Flag to check GPT partition. It only need be measured once.
@@ -55,6 +62,62 @@ UINTN                             mTcg2ImageSize;
 EFI_HANDLE                        mTcg2CacheMeasuredHandle  = NULL;
 MEASURED_HOB_DATA                 *mTcg2MeasuredHobData     = NULL;
 
+/**
+  Create CcEvent from Tcg2Event.
+
+  CcEvent is similar to Tcg2Event except the MrIndex.
+
+  @param  CcProtocol  Pointer to the located Cc Measurement protocol instance.
+  @param  Tcg2Event   Pointer to the Tcg2Event.
+  @param  EventSize   Size of the Event.
+  @param  EfiCcEvent  The created CcEvent
+
+  @retval EFI_SUCCESS           Successfully create the CcEvent
+  @retval EFI_INVALID_PARAMETER The input parameter is invalid
+  @retval EFI_UNSUPPORTED       The input PCRIndex cannot be mapped to Cc MR
+  @retval EFI_OUT_OF_RESOURCES  Out of resource
+**/
+EFI_STATUS
+CreateCcEventFromTcg2Event (
+  IN  EFI_CC_MEASUREMENT_PROTOCOL   *CcProtocol,
+  IN  EFI_TCG2_EVENT                *Tcg2Event,
+  IN  UINT32                        EventSize,
+  IN OUT EFI_CC_EVENT               **EfiCcEvent
+  )
+{
+  UINT32            MrIndex;
+  EFI_STATUS        Status;
+  EFI_CC_EVENT      *CcEvent;
+
+  if (Tcg2Event == NULL || CcProtocol == NULL || EfiCcEvent == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  *EfiCcEvent = NULL;
+
+  Status = CcProtocol->MapPcrToMrIndex (CcProtocol, Tcg2Event->Header.PCRIndex, &MrIndex);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Cannot map PcrIndex(%d) to MrIndex\n", Tcg2Event->Header.PCRIndex));
+    return Status;
+  }
+
+  CcEvent = (EFI_CC_EVENT *)AllocateZeroPool (Tcg2Event->Size);
+  if (CcEvent == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  CcEvent->Size                 = Tcg2Event->Size;
+  CcEvent->Header.HeaderSize    = Tcg2Event->Header.HeaderSize;
+  CcEvent->Header.HeaderVersion = Tcg2Event->Header.HeaderVersion;
+  CcEvent->Header.MrIndex       = MrIndex;
+  CcEvent->Header.EventType     = Tcg2Event->Header.EventType;
+  CopyMem (CcEvent->Event, Tcg2Event->Event, EventSize);
+
+  *EfiCcEvent = CcEvent;
+
+  return EFI_SUCCESS;
+}
+
 /**
   Reads contents of a PE/COFF image in memory buffer.
 
@@ -109,7 +172,7 @@ DxeTpm2MeasureBootLibImageRead (
   Caution: This function may receive untrusted input.
   The GPT partition table is external input, so this function should parse partition data carefully.
 
-  @param Tcg2Protocol            Pointer to the located TCG2 protocol instance.
+  @param MeasureBootProtocols    Pointer to the located MeasureBoot protocol instances (i.e. TCG2/Td protocol).
   @param GptHandle               Handle that GPT partition was installed.
 
   @retval EFI_SUCCESS            Successfully measure GPT table.
@@ -121,8 +184,8 @@ DxeTpm2MeasureBootLibImageRead (
 EFI_STATUS
 EFIAPI
 Tcg2MeasureGptTable (
-  IN  EFI_TCG2_PROTOCOL  *Tcg2Protocol,
-  IN  EFI_HANDLE         GptHandle
+  IN  MEASURE_BOOT_PROTOCOLS  *MeasureBootProtocols,
+  IN  EFI_HANDLE              GptHandle
   )
 {
   EFI_STATUS                        Status;
@@ -134,13 +197,29 @@ Tcg2MeasureGptTable (
   UINTN                             NumberOfPartition;
   UINT32                            Index;
   EFI_TCG2_EVENT                    *Tcg2Event;
+  EFI_CC_EVENT                      *CcEvent;
   EFI_GPT_DATA                      *GptData;
   UINT32                            EventSize;
+  EFI_TCG2_PROTOCOL                 *Tcg2Protocol;
+  EFI_CC_MEASUREMENT_PROTOCOL       *CcProtocol;
 
   if (mTcg2MeasureGptCount > 0) {
     return EFI_SUCCESS;
   }
 
+  PrimaryHeader = NULL;
+  EntryPtr      = NULL;
+  CcEvent       = NULL;
+  Tcg2Event     = NULL;
+
+  Tcg2Protocol  = MeasureBootProtocols->Tcg2Protocol;
+  CcProtocol    = MeasureBootProtocols->CcProtocol;
+
+  if (Tcg2Protocol == NULL && CcProtocol == NULL) {
+    ASSERT (FALSE);
+    return EFI_UNSUPPORTED;
+  }
+
   Status = gBS->HandleProtocol (GptHandle, &gEfiBlockIoProtocolGuid, (VOID**)&BlockIo);
   if (EFI_ERROR (Status)) {
     return EFI_UNSUPPORTED;
@@ -149,6 +228,7 @@ Tcg2MeasureGptTable (
   if (EFI_ERROR (Status)) {
     return EFI_UNSUPPORTED;
   }
+
   //
   // Read the EFI Partition Table Header
   //
@@ -156,6 +236,7 @@ Tcg2MeasureGptTable (
   if (PrimaryHeader == NULL) {
     return EFI_OUT_OF_RESOURCES;
   }
+
   Status = DiskIo->ReadDisk (
                      DiskIo,
                      BlockIo->Media->MediaId,
@@ -164,10 +245,20 @@ Tcg2MeasureGptTable (
                      (UINT8 *)PrimaryHeader
                      );
   if (EFI_ERROR (Status)) {
-    DEBUG ((EFI_D_ERROR, "Failed to Read Partition Table Header!\n"));
+    DEBUG ((DEBUG_ERROR, "Failed to Read Partition Table Header!\n"));
     FreePool (PrimaryHeader);
     return EFI_DEVICE_ERROR;
   }
+
+  //
+  // PrimaryHeader->SizeOfPartitionEntry should not be zero
+  //
+  if (PrimaryHeader->SizeOfPartitionEntry == 0) {
+    DEBUG ((DEBUG_ERROR, "SizeOfPartitionEntry should not be zero!\n"));
+    FreePool (PrimaryHeader);
+    return EFI_BAD_BUFFER_SIZE;
+  }
+
   //
   // Read the partition entry.
   //
@@ -202,15 +293,14 @@ Tcg2MeasureGptTable (
   }
 
   //
-  // Prepare Data for Measurement
+  // Prepare Data for Measurement (CcProtocol and Tcg2Protocol)
   //
   EventSize = (UINT32)(sizeof (EFI_GPT_DATA) - sizeof (GptData->Partitions)
                         + NumberOfPartition * PrimaryHeader->SizeOfPartitionEntry);
   Tcg2Event = (EFI_TCG2_EVENT *) AllocateZeroPool (EventSize + sizeof (EFI_TCG2_EVENT) - sizeof(Tcg2Event->Event));
   if (Tcg2Event == NULL) {
-    FreePool (PrimaryHeader);
-    FreePool (EntryPtr);
-    return EFI_OUT_OF_RESOURCES;
+    Status = EFI_OUT_OF_RESOURCES;
+    goto Exit;
   }
 
   Tcg2Event->Size = EventSize + sizeof (EFI_TCG2_EVENT) - sizeof(Tcg2Event->Event);
@@ -243,22 +333,57 @@ Tcg2MeasureGptTable (
   }
 
   //
-  // Measure the GPT data
+  // Measure the GPT data by Tcg2Protocol
   //
-  Status = Tcg2Protocol->HashLogExtendEvent (
-             Tcg2Protocol,
-             0,
-             (EFI_PHYSICAL_ADDRESS) (UINTN) (VOID *) GptData,
-             (UINT64) EventSize,
-             Tcg2Event
-             );
-  if (!EFI_ERROR (Status)) {
-    mTcg2MeasureGptCount++;
-  }
-
-  FreePool (PrimaryHeader);
-  FreePool (EntryPtr);
-  FreePool (Tcg2Event);
+  if (Tcg2Protocol != NULL) {
+    Status = Tcg2Protocol->HashLogExtendEvent (
+               Tcg2Protocol,
+               0,
+               (EFI_PHYSICAL_ADDRESS) (UINTN) (VOID *) GptData,
+               (UINT64) EventSize,
+               Tcg2Event
+               );
+    if (!EFI_ERROR (Status)) {
+      mTcg2MeasureGptCount++;
+    }
+    DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - Tcg2 MeasureGptTable - %r\n", Status));
+
+  } else if (CcProtocol != NULL) {
+
+    //
+    // Measure the GPT data by TdProtocol
+    //
+    Status = CreateCcEventFromTcg2Event (CcProtocol, Tcg2Event, EventSize, &CcEvent);
+    if (EFI_ERROR (Status)) {
+      goto Exit;
+    }
+
+    Status = CcProtocol->HashLogExtendEvent (
+               CcProtocol,
+               0,
+               (EFI_PHYSICAL_ADDRESS) (UINTN) (VOID *) GptData,
+               (UINT64) EventSize,
+               CcEvent
+               );
+    if (!EFI_ERROR (Status)) {
+      mTcg2MeasureGptCount++;
+    }
+    DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - Cc MeasureGptTable - %r\n", Status));
+  }
+
+Exit:
+  if (PrimaryHeader != NULL) {
+    FreePool (PrimaryHeader);
+  }
+  if (EntryPtr != NULL) {
+    FreePool (EntryPtr);
+  }
+  if (Tcg2Event != NULL) {
+    FreePool (Tcg2Event);
+  }
+  if (CcEvent != NULL) {
+    FreePool (CcEvent);
+  }
 
   return Status;
 }
@@ -271,12 +396,12 @@ Tcg2MeasureGptTable (
   PE/COFF image is external input, so this function will validate its data structure
   within this image buffer before use.
 
-  @param[in] Tcg2Protocol   Pointer to the located TCG2 protocol instance.
-  @param[in] ImageAddress   Start address of image buffer.
-  @param[in] ImageSize      Image size
-  @param[in] LinkTimeBase   Address that the image is loaded into memory.
-  @param[in] ImageType      Image subsystem type.
-  @param[in] FilePath       File path is corresponding to the input image.
+  @param[in] MeasureBootProtocols   Pointer to the located MeasureBoot protocol instances.
+  @param[in] ImageAddress           Start address of image buffer.
+  @param[in] ImageSize              Image size
+  @param[in] LinkTimeBase           Address that the image is loaded into memory.
+  @param[in] ImageType              Image subsystem type.
+  @param[in] FilePath               File path is corresponding to the input image.
 
   @retval EFI_SUCCESS            Successfully measure image.
   @retval EFI_OUT_OF_RESOURCES   No enough resource to measure image.
@@ -287,7 +412,7 @@ Tcg2MeasureGptTable (
 EFI_STATUS
 EFIAPI
 Tcg2MeasurePeImage (
-  IN  EFI_TCG2_PROTOCOL         *Tcg2Protocol,
+  IN  MEASURE_BOOT_PROTOCOLS    *MeasureBootProtocols,
   IN  EFI_PHYSICAL_ADDRESS      ImageAddress,
   IN  UINTN                     ImageSize,
   IN  UINTN                     LinkTimeBase,
@@ -300,9 +425,22 @@ Tcg2MeasurePeImage (
   EFI_IMAGE_LOAD_EVENT              *ImageLoad;
   UINT32                            FilePathSize;
   UINT32                            EventSize;
+  EFI_CC_EVENT                      *CcEvent;
+  EFI_CC_MEASUREMENT_PROTOCOL       *CcProtocol;
+  EFI_TCG2_PROTOCOL                 *Tcg2Protocol;
 
   Status        = EFI_UNSUPPORTED;
   ImageLoad     = NULL;
+  CcEvent       = NULL;
+
+  Tcg2Protocol  = MeasureBootProtocols->Tcg2Protocol;
+  CcProtocol    = MeasureBootProtocols->CcProtocol;
+
+  if (Tcg2Protocol == NULL && CcProtocol == NULL) {
+    ASSERT (FALSE);
+    return EFI_UNSUPPORTED;
+  }
+
   FilePathSize  = (UINT32) GetDevicePathSize (FilePath);
 
   //
@@ -334,7 +472,7 @@ Tcg2MeasurePeImage (
       break;
     default:
       DEBUG ((
-        EFI_D_ERROR,
+        DEBUG_ERROR,
         "Tcg2MeasurePeImage: Unknown subsystem type %d",
         ImageType
         ));
@@ -352,28 +490,125 @@ Tcg2MeasurePeImage (
   //
   // Log the PE data
   //
-  Status = Tcg2Protocol->HashLogExtendEvent (
-             Tcg2Protocol,
-             PE_COFF_IMAGE,
-             ImageAddress,
-             ImageSize,
-             Tcg2Event
-             );
-  if (Status == EFI_VOLUME_FULL) {
-    //
-    // Volume full here means the image is hashed and its result is extended to PCR.
-    // But the event log can't be saved since log area is full.
-    // Just return EFI_SUCCESS in order not to block the image load.
-    //
-    Status = EFI_SUCCESS;
+  if (Tcg2Protocol != NULL) {
+    Status = Tcg2Protocol->HashLogExtendEvent (
+               Tcg2Protocol,
+               PE_COFF_IMAGE,
+               ImageAddress,
+               ImageSize,
+               Tcg2Event
+               );
+    if (Status == EFI_VOLUME_FULL) {
+      //
+      // Volume full here means the image is hashed and its result is extended to PCR.
+      // But the event log can't be saved since log area is full.
+      // Just return EFI_SUCCESS in order not to block the image load.
+      //
+      Status = EFI_SUCCESS;
+    }
+    DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - Tcg2 MeasurePeImage - %r\n", Status));
+
+  } else if (CcProtocol != NULL) {
+
+    Status = CreateCcEventFromTcg2Event (CcProtocol, Tcg2Event, EventSize, &CcEvent);
+    if (EFI_ERROR (Status)) {
+      goto Finish;
+    }
+
+    Status = CcProtocol->HashLogExtendEvent (
+               CcProtocol,
+               PE_COFF_IMAGE,
+               ImageAddress,
+               ImageSize,
+               CcEvent
+               );
+    if (Status == EFI_VOLUME_FULL) {
+      //
+      // Volume full here means the image is hashed and its result is extended to PCR.
+      // But the event log can't be saved since log area is full.
+      // Just return EFI_SUCCESS in order not to block the image load.
+      //
+      Status = EFI_SUCCESS;
+    }
+    DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - Cc MeasurePeImage - %r\n", Status));
   }
 
 Finish:
-  FreePool (Tcg2Event);
+  if (Tcg2Event != NULL) {
+    FreePool (Tcg2Event);
+  }
+
+  if (CcEvent != NULL) {
+    FreePool (CcEvent);
+  }
 
   return Status;
 }
 
+/**
+  Get the measure boot protocols.
+
+  There are 2 measure boot, TCG2 protocol based and Cc measurement protocol based.
+
+  @param  MeasureBootProtocols  Pointer to the located measure boot protocol instances.
+
+  @retval EFI_SUCCESS           Sucessfully locate the measure boot protocol instances (at least one instance).
+  @retval EFI_UNSUPPORTED       Measure boot is not supported.
+**/
+EFI_STATUS
+EFIAPI
+GetMeasureBootProtocols (
+  MEASURE_BOOT_PROTOCOLS    *MeasureBootProtocols
+  )
+{
+  EFI_STATUS                          Status;
+  EFI_TCG2_PROTOCOL                   *Tcg2Protocol;
+  EFI_CC_MEASUREMENT_PROTOCOL         *CcProtocol;
+  EFI_TCG2_BOOT_SERVICE_CAPABILITY    Tcg2ProtocolCapability;
+  EFI_CC_BOOT_SERVICE_CAPABILITY      CcProtocolCapability;
+
+  CcProtocol = NULL;
+  Status = gBS->LocateProtocol (&gEfiCcMeasurementProtocolGuid, NULL, (VOID **) &CcProtocol);
+  if (EFI_ERROR (Status)) {
+    //
+    // Cc Measurement protocol is not installed.
+    //
+    DEBUG ((DEBUG_VERBOSE, "CcMeasurementProtocol is not installed. - %r\n", Status));
+  } else {
+    ZeroMem (&CcProtocolCapability, sizeof (CcProtocolCapability));
+    CcProtocolCapability.Size = sizeof (CcProtocolCapability);
+    Status = CcProtocol->GetCapability (CcProtocol, &CcProtocolCapability);
+    if (EFI_ERROR (Status) || CcProtocolCapability.CcType.Type == EFI_CC_TYPE_NONE) {
+      DEBUG ((DEBUG_ERROR, " CcProtocol->GetCapability returns : %x, %r\n", CcProtocolCapability.CcType.Type, Status));
+      CcProtocol = NULL;
+    }
+  }
+
+  Tcg2Protocol = NULL;
+  Status = gBS->LocateProtocol (&gEfiTcg2ProtocolGuid, NULL, (VOID **) &Tcg2Protocol);
+  if (EFI_ERROR (Status)) {
+    //
+    // Tcg2 protocol is not installed. So, TPM2 is not present.
+    //
+    DEBUG ((DEBUG_VERBOSE, "Tcg2Protocol is not installed. - %r\n", Status));
+  } else {
+    Tcg2ProtocolCapability.Size = (UINT8) sizeof (Tcg2ProtocolCapability);
+    Status = Tcg2Protocol->GetCapability (Tcg2Protocol, &Tcg2ProtocolCapability);
+    if (EFI_ERROR (Status) || (!Tcg2ProtocolCapability.TPMPresentFlag)) {
+      //
+      // TPM device doesn't work or activate.
+      //
+      DEBUG ((DEBUG_ERROR, "TPMPresentFlag=FALSE %r\n", Status));
+      Tcg2Protocol = NULL;
+    }
+  }
+
+  MeasureBootProtocols->Tcg2Protocol = Tcg2Protocol;
+  MeasureBootProtocols->CcProtocol   = CcProtocol;
+
+  return (Tcg2Protocol == NULL && CcProtocol == NULL) ? EFI_UNSUPPORTED: EFI_SUCCESS;
+}
+
 /**
   The security handler is used to abstract platform-specific policy
   from the DXE core response to an attempt to use a file that returns a
@@ -422,9 +657,8 @@ DxeTpm2MeasureBootHandler (
   IN  BOOLEAN                          BootPolicy
   )
 {
-  EFI_TCG2_PROTOCOL                   *Tcg2Protocol;
+  MEASURE_BOOT_PROTOCOLS              MeasureBootProtocols;
   EFI_STATUS                          Status;
-  EFI_TCG2_BOOT_SERVICE_CAPABILITY    ProtocolCapability;
   EFI_DEVICE_PATH_PROTOCOL            *DevicePathNode;
   EFI_DEVICE_PATH_PROTOCOL            *OrigDevicePathNode;
   EFI_HANDLE                          Handle;
@@ -435,28 +669,23 @@ DxeTpm2MeasureBootHandler (
   EFI_PHYSICAL_ADDRESS                FvAddress;
   UINT32                              Index;
 
-  Status = gBS->LocateProtocol (&gEfiTcg2ProtocolGuid, NULL, (VOID **) &Tcg2Protocol);
+  MeasureBootProtocols.Tcg2Protocol = NULL;
+  MeasureBootProtocols.CcProtocol   = NULL;
+
+  Status = GetMeasureBootProtocols(&MeasureBootProtocols);
+
   if (EFI_ERROR (Status)) {
     //
-    // Tcg2 protocol is not installed. So, TPM2 is not present.
+    // None of Measured boot protocols (Tcg2, Cc) is installed.
     // Don't do any measurement, and directly return EFI_SUCCESS.
     //
-    DEBUG ((EFI_D_VERBOSE, "DxeTpm2MeasureBootHandler - Tcg2 - %r\n", Status));
+    DEBUG ((DEBUG_INFO, "None of Tcg2Protocol/CcMeasurementProtocol is installed.\n"));
     return EFI_SUCCESS;
   }
 
-  ProtocolCapability.Size = (UINT8) sizeof (ProtocolCapability);
-  Status = Tcg2Protocol->GetCapability (
-                           Tcg2Protocol,
-                           &ProtocolCapability
-                           );
-  if (EFI_ERROR (Status) || (!ProtocolCapability.TPMPresentFlag)) {
-    //
-    // TPM device doesn't work or activate.
-    //
-    DEBUG ((EFI_D_ERROR, "DxeTpm2MeasureBootHandler (%r) - TPMPresentFlag - %x\n", Status, ProtocolCapability.TPMPresentFlag));
-    return EFI_SUCCESS;
-  }
+  DEBUG ((DEBUG_INFO, "Tcg2Protocol = %p, TdProtocol = %p\n",
+                      MeasureBootProtocols.Tcg2Protocol,
+                      MeasureBootProtocols.CcProtocol));
 
   //
   // Copy File Device Path
@@ -502,8 +731,8 @@ DxeTpm2MeasureBootHandler (
             //
             // Measure GPT disk.
             //
-            Status = Tcg2MeasureGptTable (Tcg2Protocol, Handle);
-            DEBUG ((EFI_D_INFO, "DxeTpm2MeasureBootHandler - Tcg2MeasureGptTable - %r\n", Status));
+            Status = Tcg2MeasureGptTable (&MeasureBootProtocols, Handle);
+
             if (!EFI_ERROR (Status)) {
               //
               // GPT disk check done.
@@ -647,14 +876,13 @@ DxeTpm2MeasureBootHandler (
     // Measure PE image into TPM log.
     //
     Status = Tcg2MeasurePeImage (
-               Tcg2Protocol,
+               &MeasureBootProtocols,
                (EFI_PHYSICAL_ADDRESS) (UINTN) FileBuffer,
                FileSize,
                (UINTN) ImageContext.ImageAddress,
                ImageContext.ImageType,
                DevicePathNode
                );
-    DEBUG ((EFI_D_INFO, "DxeTpm2MeasureBootHandler - Tcg2MeasurePeImage - %r\n", Status));
   }
 
   //
@@ -665,7 +893,7 @@ Finish:
     FreePool (OrigDevicePathNode);
   }
 
-  DEBUG ((EFI_D_INFO, "DxeTpm2MeasureBootHandler - %r\n", Status));
+  DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - %r\n", Status));
 
   return Status;
 }
diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
index 2506abbe7c8b..6dca79a20c93 100644
--- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
+++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
@@ -1,5 +1,5 @@
 ## @file
-#  Provides security service for TPM 2.0 measured boot
+#  Provides security service for TPM 2.0 measured boot and Confidential Computing measure boot.
 #
 #  Spec Compliance Info:
 #    "TCG PC Client Platform Firmware Profile Specification for TPM Family 2.0 Level 00 Revision 1.03 v51"
@@ -61,6 +61,7 @@
 
 [Protocols]
   gEfiTcg2ProtocolGuid                  ## SOMETIMES_CONSUMES
+  gEfiCcMeasurementProtocolGuid         ## SOMETIMES_CONSUMES
   gEfiFirmwareVolumeBlockProtocolGuid   ## SOMETIMES_CONSUMES
   gEfiBlockIoProtocolGuid               ## SOMETIMES_CONSUMES
   gEfiDiskIoProtocolGuid                ## SOMETIMES_CONSUMES
-- 
2.29.2.windows.2


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

* [PATCH V4 3/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpmMeasurementLib
  2021-11-02  2:50 [PATCH V4 0/3] Introduce CcMeasurementProtocol into EDK2 Min Xu
  2021-11-02  2:50 ` [PATCH V4 1/3] MdePkg: Introduce CcMeasurementProtocol for CC Guest firmware Min Xu
  2021-11-02  2:50 ` [PATCH V4 2/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpm2MeasureBootLib Min Xu
@ 2021-11-02  2:50 ` Min Xu
  2021-11-02  6:24   ` Yao, Jiewen
  2021-11-02  9:45   ` Sami Mujawar
  2 siblings, 2 replies; 23+ messages in thread
From: Min Xu @ 2021-11-02  2:50 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Michael D Kinney, Liming Gao, Zhiguang Liu, Jiewen Yao,
	Jian J Wang, Sami Mujawar, Gerd Hoffmann

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3625

DxeTpmMeasurementLib supports TPM based measurement in DXE phase.
After CcMeasurementProtocol is introduced, CC based measurement needs
to be supported in DxeTpmMeasurementLib as well.

In TpmMeasureAndLogData, CC based measurement will be first called.
If it failed, TPM based measurement will be called sequentially.
Currently there is an assumption that CC based measurement and
TPM based measurement won't be exist at the same time.If the
assumption is not true in the future, we will revisit here then.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 .../DxeTpmMeasurementLib.c                    | 91 ++++++++++++++++++-
 .../DxeTpmMeasurementLib.inf                  |  9 +-
 2 files changed, 92 insertions(+), 8 deletions(-)

diff --git a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
index 061136ee7860..2ddb9033a0d5 100644
--- a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
+++ b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
@@ -1,5 +1,6 @@
 /** @file
-  This library is used by other modules to measure data to TPM.
+  This library is used by other modules to measure data to TPM and Confidential
+  Computing (CC) measure registers.
 
 Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved. <BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -19,8 +20,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include <Guid/Acpi.h>
 #include <IndustryStandard/Acpi.h>
-
-
+#include <Protocol/CcMeasurement.h>
 
 /**
   Tpm12 measure and log data, and extend the measurement result into a specific PCR.
@@ -149,6 +149,73 @@ Tpm20MeasureAndLogData (
   return Status;
 }
 
+/**
+  Cc measure and log data, and extend the measurement result into a
+  specific CC MR.
+
+  @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
+
+  @retval EFI_SUCCESS           Operation completed successfully.
+  @retval EFI_UNSUPPORTED       Tdx device not available.
+  @retval EFI_OUT_OF_RESOURCES  Out of memory.
+  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
+**/
+EFI_STATUS
+EFIAPI
+CcMeasureAndLogData (
+  IN UINT32             PcrIndex,
+  IN UINT32             EventType,
+  IN VOID               *EventLog,
+  IN UINT32             LogLen,
+  IN VOID               *HashData,
+  IN UINT64             HashDataLen
+  )
+{
+  EFI_STATUS                    Status;
+  EFI_CC_MEASUREMENT_PROTOCOL  *CcProtocol;
+  EFI_CC_EVENT                 *EfiCcEvent;
+  UINT32                        MrIndex;
+
+  Status = gBS->LocateProtocol (&gEfiCcMeasurementProtocolGuid, NULL, (VOID **) &CcProtocol);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Status = CcProtocol->MapPcrToMrIndex (CcProtocol, PcrIndex, &MrIndex);
+  if (EFI_ERROR (Status)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  EfiCcEvent = (EFI_CC_EVENT *) AllocateZeroPool (LogLen + sizeof (EFI_CC_EVENT));
+  if(EfiCcEvent == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  EfiCcEvent->Size = (UINT32) LogLen + sizeof (EFI_CC_EVENT) - sizeof (EfiCcEvent->Event);
+  EfiCcEvent->Header.HeaderSize    = sizeof (EFI_CC_EVENT_HEADER);
+  EfiCcEvent->Header.HeaderVersion = EFI_CC_EVENT_HEADER_VERSION;
+  EfiCcEvent->Header.MrIndex       = MrIndex;
+  EfiCcEvent->Header.EventType     = EventType;
+  CopyMem (&EfiCcEvent->Event[0], EventLog, LogLen);
+
+  Status = CcProtocol->HashLogExtendEvent (
+                           CcProtocol,
+                           0,
+                           (EFI_PHYSICAL_ADDRESS) (UINTN) HashData,
+                           HashDataLen,
+                           EfiCcEvent
+                           );
+  FreePool (EfiCcEvent);
+
+  return Status;
+}
+
+
 /**
   Tpm measure and log data, and extend the measurement result into a specific PCR.
 
@@ -178,9 +245,9 @@ TpmMeasureAndLogData (
   EFI_STATUS  Status;
 
   //
-  // Try to measure using Tpm20 protocol
+  // Try to measure using Cc measurement protocol
   //
-  Status = Tpm20MeasureAndLogData(
+  Status = CcMeasureAndLogData (
              PcrIndex,
              EventType,
              EventLog,
@@ -189,6 +256,20 @@ TpmMeasureAndLogData (
              HashDataLen
              );
 
+  if (EFI_ERROR (Status)) {
+    //
+    // Try to measure using Tpm20 protocol
+    //
+    Status = Tpm20MeasureAndLogData(
+               PcrIndex,
+               EventType,
+               EventLog,
+               LogLen,
+               HashData,
+               HashDataLen
+               );
+  }
+
   if (EFI_ERROR (Status)) {
     //
     // Try to measure using Tpm1.2 protocol
diff --git a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
index 7d41bc41f95d..3af3d4e33b25 100644
--- a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
+++ b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
@@ -1,5 +1,7 @@
 ## @file
-#  Provides TPM measurement functions for TPM1.2 and TPM 2.0
+#  Provides below measurement functions:
+#    1. TPM measurement functions for TPM1.2 and TPM 2.0
+#    2. Confidential Computing (CC) measurement functions
 #
 #  This library provides TpmMeasureAndLogData() to measure and log data, and
 #  extend the measurement result into a specific PCR.
@@ -40,5 +42,6 @@
   UefiBootServicesTableLib
 
 [Protocols]
-  gEfiTcgProtocolGuid           ## SOMETIMES_CONSUMES
-  gEfiTcg2ProtocolGuid          ## SOMETIMES_CONSUMES
+  gEfiTcgProtocolGuid               ## SOMETIMES_CONSUMES
+  gEfiTcg2ProtocolGuid              ## SOMETIMES_CONSUMES
+  gEfiCcMeasurementProtocolGuid     ## SOMETIMES_CONSUMES
-- 
2.29.2.windows.2


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

* Re: [PATCH V4 1/3] MdePkg: Introduce CcMeasurementProtocol for CC Guest firmware
  2021-11-02  2:50 ` [PATCH V4 1/3] MdePkg: Introduce CcMeasurementProtocol for CC Guest firmware Min Xu
@ 2021-11-02  6:24   ` Yao, Jiewen
  2021-11-02  9:41   ` Sami Mujawar
  2021-11-04  5:51   ` 回复: " gaoliming
  2 siblings, 0 replies; 23+ messages in thread
From: Yao, Jiewen @ 2021-11-02  6:24 UTC (permalink / raw)
  To: Xu, Min M, devel@edk2.groups.io
  Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang, Wang, Jian J,
	Lu, Ken, Sami Mujawar, Gerd Hoffmann

Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Tuesday, November 2, 2021 10:51 AM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu,
> Zhiguang <zhiguang.liu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>; Lu, Ken <ken.lu@intel.com>; Sami
> Mujawar <sami.mujawar@arm.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [PATCH V4 1/3] MdePkg: Introduce CcMeasurementProtocol for CC
> Guest firmware
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3625
> 
> CC guest is a Confidential Computing guest. If CC Guest firmware
> supports measurement and an event is created, CC Guest firmware
> is designed to report the event log with the same data structure
> in TCG-Platform-Firmware-Profile specification with
> EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 format.
> 
> The CC Guest firmware supports measurement. It is designed to
> produce EFI_CC_MEASUREMENT_PROTOCOL with new GUID
> EFI_CC_MEASUREMENT_PROTOCOL_GUID to report event log and provides
> hash capability.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Ken Lu <ken.lu@intel.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
>  MdePkg/Include/Protocol/CcMeasurement.h | 305
> ++++++++++++++++++++++++
>  MdePkg/MdePkg.dec                       |   4 +
>  2 files changed, 309 insertions(+)
>  create mode 100644 MdePkg/Include/Protocol/CcMeasurement.h
> 
> diff --git a/MdePkg/Include/Protocol/CcMeasurement.h
> b/MdePkg/Include/Protocol/CcMeasurement.h
> new file mode 100644
> index 000000000000..eaedbfffdb6a
> --- /dev/null
> +++ b/MdePkg/Include/Protocol/CcMeasurement.h
> @@ -0,0 +1,305 @@
> +/** @file
> +  If CC Guest firmware supports measurement and an event is created,
> +  CC Guest firmware is designed to report the event log with the same
> +  data structure in TCG-Platform-Firmware-Profile specification with
> +  EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 format.
> +
> +  The CC Guest firmware supports measurement, the CC Guest Firmware is
> +  designed to produce EFI_CC_MEASUREMENT_PROTOCOL with new GUID
> +  EFI_CC_MEASUREMENT_PROTOCOL_GUID to report event log and provides
> hash
> +  capability.
> +
> +Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef CC_MEASUREMENT_PROTOCOL_H_
> +#define CC_MEASUREMENT_PROTOCOL_H_
> +
> +#include <IndustryStandard/UefiTcgPlatform.h>
> +
> +#define EFI_CC_MEASUREMENT_PROTOCOL_GUID  \
> +  { 0x96751a3d, 0x72f4, 0x41a6, { 0xa7, 0x94, 0xed, 0x5d, 0x0e, 0x67, 0xae,
> 0x6b }}
> +extern EFI_GUID gEfiCcMeasurementProtocolGuid;
> +
> +typedef struct _EFI_CC_MEASUREMENT_PROTOCOL
> EFI_CC_MEASUREMENT_PROTOCOL;
> +
> +typedef struct {
> +  UINT8 Major;
> +  UINT8 Minor;
> +} EFI_CC_VERSION;
> +
> +//
> +// EFI_CC Type/SubType definition
> +//
> +#define EFI_CC_TYPE_NONE   0
> +#define EFI_CC_TYPE_SEV    1
> +#define EFI_CC_TYPE_TDX    2
> +
> +typedef struct {
> +  UINT8          Type;
> +  UINT8          SubType;
> +} EFI_CC_TYPE;
> +
> +typedef UINT32                      EFI_CC_EVENT_LOG_BITMAP;
> +typedef UINT32                      EFI_CC_EVENT_LOG_FORMAT;
> +typedef UINT32                      EFI_CC_EVENT_ALGORITHM_BITMAP;
> +typedef UINT32                      EFI_CC_MR_INDEX;
> +
> +//
> +// Intel TDX measure register index
> +//
> +#define TDX_MR_INDEX_MRTD  0
> +#define TDX_MR_INDEX_RTMR0 1
> +#define TDX_MR_INDEX_RTMR1 2
> +#define TDX_MR_INDEX_RTMR2 3
> +#define TDX_MR_INDEX_RTMR3 4
> +
> +
> +#define EFI_CC_EVENT_LOG_FORMAT_TCG_2   0x00000002
> +#define EFI_CC_BOOT_HASH_ALG_SHA384     0x00000004
> +
> +//
> +// This bit is shall be set when an event shall be extended but not logged.
> +//
> +#define EFI_CC_FLAG_EXTEND_ONLY       0x0000000000000001
> +//
> +// This bit shall be set when the intent is to measure a PE/COFF image.
> +//
> +#define EFI_CC_FLAG_PE_COFF_IMAGE     0x0000000000000010
> +
> +#pragma pack (1)
> +
> +#define EFI_CC_EVENT_HEADER_VERSION   1
> +
> +typedef struct {
> +  //
> +  // Size of the event header itself (sizeof(EFI_CC_EVENT_HEADER)).
> +  //
> +  UINT32            HeaderSize;
> +  //
> +  // Header version. For this version of this specification, the value shall be 1.
> +  //
> +  UINT16            HeaderVersion;
> +  //
> +  // Index of the MR (measurement register) that shall be extended.
> +  //
> +  EFI_CC_MR_INDEX   MrIndex;
> +  //
> +  // Type of the event that shall be extended (and optionally logged).
> +  //
> +  UINT32            EventType;
> +} EFI_CC_EVENT_HEADER;
> +
> +typedef struct {
> +  //
> +  // Total size of the event including the Size component, the header and the
> Event data.
> +  //
> +  UINT32                  Size;
> +  EFI_CC_EVENT_HEADER     Header;
> +  UINT8                   Event[1];
> +} EFI_CC_EVENT;
> +
> +#pragma pack()
> +
> +
> +typedef struct {
> +  //
> +  // Allocated size of the structure
> +  //
> +  UINT8                            Size;
> +  //
> +  // Version of the EFI_CC_BOOT_SERVICE_CAPABILITY structure itself.
> +  // For this version of the protocol, the Major version shall be set to 1
> +  // and the Minor version shall be set to 0.
> +  //
> +  EFI_CC_VERSION                   StructureVersion;
> +  //
> +  // Version of the EFI CC Measurement protocol.
> +  // For this version of the protocol, the Major version shall be set to 1
> +  // and the Minor version shall be set to 0.
> +  //
> +  EFI_CC_VERSION                   ProtocolVersion;
> +  //
> +  // Supported hash algorithms
> +  //
> +  EFI_CC_EVENT_ALGORITHM_BITMAP   HashAlgorithmBitmap;
> +  //
> +  // Bitmap of supported event log formats
> +  //
> +  EFI_CC_EVENT_LOG_BITMAP          SupportedEventLogs;
> +
> +  //
> +  // Indicates the CC type
> +  //
> +  EFI_CC_TYPE                      CcType;
> +} EFI_CC_BOOT_SERVICE_CAPABILITY;
> +
> +/**
> +  The EFI_CC_MEASUREMENT_PROTOCOL GetCapability function call provides
> protocol
> +  capability information and state information.
> +
> +  @param[in]      This               Indicates the calling context
> +  @param[in, out] ProtocolCapability The caller allocates memory for a
> EFI_CC_BOOT_SERVICE_CAPABILITY
> +                                     structure and sets the size field to the size of the structure
> allocated.
> +                                     The callee fills in the fields with the EFI CC BOOT Service
> capability
> +                                     information and the current CC information.
> +
> +  @retval EFI_SUCCESS            Operation completed successfully.
> +  @retval EFI_DEVICE_ERROR       The command was unsuccessful.
> +                                 The ProtocolCapability variable will not be populated.
> +  @retval EFI_INVALID_PARAMETER  One or more of the parameters are
> incorrect.
> +                                 The ProtocolCapability variable will not be populated.
> +  @retval EFI_BUFFER_TOO_SMALL   The ProtocolCapability variable is too
> small to hold the full response.
> +                                 It will be partially populated (required Size field will be set).
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EFI_CC_GET_CAPABILITY) (
> +  IN     EFI_CC_MEASUREMENT_PROTOCOL       *This,
> +  IN OUT EFI_CC_BOOT_SERVICE_CAPABILITY    *ProtocolCapability
> +  );
> +
> +/**
> +  The EFI_CC_MEASUREMENT_PROTOCOL Get Event Log function call allows a
> caller to
> +  retrieve the address of a given event log and its last entry.
> +
> +  @param[in]  This               Indicates the calling context
> +  @param[in]  EventLogFormat     The type of the event log for which the
> information is requested.
> +  @param[out] EventLogLocation   A pointer to the memory address of the
> event log.
> +  @param[out] EventLogLastEntry  If the Event Log contains more than one
> entry, this is a pointer to the
> +                                 address of the start of the last entry in the event log in
> memory.
> +  @param[out] EventLogTruncated  If the Event Log is missing at least one
> entry because an event would
> +                                 have exceeded the area allocated for events, this value is set
> to TRUE.
> +                                 Otherwise, the value will be FALSE and the Event Log will be
> complete.
> +
> +  @retval EFI_SUCCESS            Operation completed successfully.
> +  @retval EFI_INVALID_PARAMETER  One or more of the parameters are
> incorrect
> +                                 (e.g. asking for an event log whose format is not supported).
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EFI_CC_GET_EVENT_LOG) (
> +  IN  EFI_CC_MEASUREMENT_PROTOCOL     *This,
> +  IN  EFI_CC_EVENT_LOG_FORMAT         EventLogFormat,
> +  OUT EFI_PHYSICAL_ADDRESS            *EventLogLocation,
> +  OUT EFI_PHYSICAL_ADDRESS            *EventLogLastEntry,
> +  OUT BOOLEAN                         *EventLogTruncated
> +  );
> +
> +/**
> +  The EFI_CC_MEASUREMENT_PROTOCOL HashLogExtendEvent function call
> provides
> +  callers with an opportunity to extend and optionally log events without
> requiring
> +  knowledge of actual CC commands.
> +  The extend operation will occur even if this function cannot create an event
> +  log entry (e.g. due to the event log being full).
> +
> +  @param[in]  This               Indicates the calling context
> +  @param[in]  Flags              Bitmap providing additional information.
> +  @param[in]  DataToHash         Physical address of the start of the data buffer
> to be hashed.
> +  @param[in]  DataToHashLen      The length in bytes of the buffer referenced
> by DataToHash.
> +  @param[in]  EfiCcEvent        Pointer to data buffer containing information
> about the event.
> +
> +  @retval EFI_SUCCESS            Operation completed successfully.
> +  @retval EFI_DEVICE_ERROR       The command was unsuccessful.
> +  @retval EFI_VOLUME_FULL        The extend operation occurred, but the event
> could not be written to one or more event logs.
> +  @retval EFI_INVALID_PARAMETER  One or more of the parameters are
> incorrect.
> +  @retval EFI_UNSUPPORTED        The PE/COFF image type is not supported.
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI * EFI_CC_HASH_LOG_EXTEND_EVENT) (
> +  IN EFI_CC_MEASUREMENT_PROTOCOL    *This,
> +  IN UINT64                         Flags,
> +  IN EFI_PHYSICAL_ADDRESS           DataToHash,
> +  IN UINT64                         DataToHashLen,
> +  IN EFI_CC_EVENT                   *EfiCcEvent
> +  );
> +
> +/**
> +  The EFI_CC_MEASUREMENT_PROTOCOL MapPcrToMrIndex function call
> provides callers
> +  the info on TPM PCR <-> CC MR mapping information.
> +
> +  @param[in]  This               Indicates the calling context
> +  @param[in]  PcrIndex           TPM PCR index.
> +  @param[out] MrIndex            CC MR index.
> +
> +  @retval EFI_SUCCESS            The MrIndex is returned.
> +  @retval EFI_INVALID_PARAMETER  The MrIndex is NULL.
> +  @retval EFI_UNSUPPORTED        The PcrIndex is invalid.
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI * EFI_CC_MAP_PCR_TO_MR_INDEX) (
> +  IN  EFI_CC_MEASUREMENT_PROTOCOL    *This,
> +  IN  TCG_PCRINDEX                   PcrIndex,
> +  OUT EFI_CC_MR_INDEX                *MrIndex
> +  );
> +
> +struct _EFI_CC_MEASUREMENT_PROTOCOL {
> +  EFI_CC_GET_CAPABILITY                     GetCapability;
> +  EFI_CC_GET_EVENT_LOG                      GetEventLog;
> +  EFI_CC_HASH_LOG_EXTEND_EVENT              HashLogExtendEvent;
> +  EFI_CC_MAP_PCR_TO_MR_INDEX                MapPcrToMrIndex;
> +};
> +
> +//
> +// CC event log
> +//
> +
> +#pragma pack(1)
> +
> +//
> +// Crypto Agile Log Entry Format.
> +// It is similar with TCG_PCR_EVENT2 except the field of MrIndex and PCRIndex.
> +//
> +typedef struct {
> +  EFI_CC_MR_INDEX     MrIndex;
> +  UINT32              EventType;
> +  TPML_DIGEST_VALUES  Digests;
> +  UINT32              EventSize;
> +  UINT8               Event[1];
> +} CC_EVENT;
> +
> +//
> +// EFI CC Event Header
> +// It is similar with TCG_PCR_EVENT2_HDR except the field of MrIndex and
> PCRIndex
> +//
> +typedef struct {
> +  EFI_CC_MR_INDEX     MrIndex;
> +  UINT32              EventType;
> +  TPML_DIGEST_VALUES  Digests;
> +  UINT32              EventSize;
> +} CC_EVENT_HDR;
> +
> +#pragma pack()
> +
> +//
> +// Log entries after Get Event Log service
> +//
> +
> +#define EFI_CC_FINAL_EVENTS_TABLE_VERSION  1
> +
> +typedef struct {
> +  //
> +  // The version of this structure. It shall be set to 1.
> +  //
> +  UINT64                  Version;
> +  //
> +  // Number of events recorded after invocation of GetEventLog API
> +  //
> +  UINT64                  NumberOfEvents;
> +  //
> +  // List of events of type CC_EVENT.
> +  //
> +  //CC_EVENT              Event[1];
> +} EFI_CC_FINAL_EVENTS_TABLE;
> +
> +
> +#define EFI_CC_FINAL_EVENTS_TABLE_GUID \
> +  {0xdd4a4648, 0x2de7, 0x4665, {0x96, 0x4d, 0x21, 0xd9, 0xef, 0x5f, 0xb4,
> 0x46}}
> +
> +extern EFI_GUID gEfiCcFinalEventsTableGuid;
> +
> +#endif
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> index 8b18415b107a..8e19be662ada 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -1011,6 +1011,10 @@
>    ## Include/Protocol/PcdInfo.h
>    gGetPcdInfoProtocolGuid        = { 0x5be40f57, 0xfa68, 0x4610, { 0xbb, 0xbf,
> 0xe9, 0xc5, 0xfc, 0xda, 0xd3, 0x65 } }
> 
> +  ## Include/Protocol/CcMeasurement.h
> +  gEfiCcMeasurementProtocolGuid  = { 0x96751a3d, 0x72f4, 0x41a6, { 0xa7,
> 0x94, 0xed, 0x5d, 0x0e, 0x67, 0xae, 0x6b }}
> +  gEfiCcFinalEventsTableGuid     = { 0xdd4a4648, 0x2de7, 0x4665, { 0x96, 0x4d,
> 0x21, 0xd9, 0xef, 0x5f, 0xb4, 0x46 }}
> +
>    #
>    # Protocols defined in PI1.0.
>    #
> --
> 2.29.2.windows.2


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

* Re: [PATCH V4 2/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpm2MeasureBootLib
  2021-11-02  2:50 ` [PATCH V4 2/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpm2MeasureBootLib Min Xu
@ 2021-11-02  6:24   ` Yao, Jiewen
  2021-11-03  2:59     ` Min Xu
  2021-11-02  9:43   ` Sami Mujawar
  1 sibling, 1 reply; 23+ messages in thread
From: Yao, Jiewen @ 2021-11-02  6:24 UTC (permalink / raw)
  To: Xu, Min M, devel@edk2.groups.io
  Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang, Wang, Jian J,
	Sami Mujawar, Gerd Hoffmann

May I know which platform you have run the test?

I think we need cover both TD and TPM in real platform.

> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Tuesday, November 2, 2021 10:51 AM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu,
> Zhiguang <zhiguang.liu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>; Sami Mujawar
> <sami.mujawar@arm.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [PATCH V4 2/3] SecurityPkg: Support CcMeasurementProtocol in
> DxeTpm2MeasureBootLib
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3625
> 
> DxeTpm2MeasureBootLib supports TPM2 based measure boot. After
> CcMeasurementProtocol is introduced, CC based measure boot needs to
> be supported in DxeTpm2MeasureBootLib as well.
> 
> There are 2 major changes in this commit.
> 
> 1. MEASURE_BOOT_PROTOCOLS is defined to store the instances of TCG2
> protocol and TEE protocol. In the DxeTpm2MeasureBootHandler above 2
> measure boot protocol instances will be located. Then the located
> protocol instances will be called to do the measure boot.
> 
> 2. CcEvent is similar to Tcg2Event except the MrIndex and PcrIndex.
> CreateCcEventFromTcg2Event is used to create the CcEvent based on the
> Tcg2Event.
> 
> Above 2 changes make the minimize changes to the existing code.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
>  .../DxeTpm2MeasureBootLib.c                   | 366 ++++++++++++++----
>  .../DxeTpm2MeasureBootLib.inf                 |   3 +-
>  2 files changed, 299 insertions(+), 70 deletions(-)
> 
> diff --git
> a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
> b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
> index 92eac715800f..af889b6ed3ed 100644
> --- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
> +++
> b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
> @@ -1,5 +1,6 @@
>  /** @file
> -  The library instance provides security service of TPM2 measure boot.
> +  The library instance provides security service of TPM2 measure boot and
> +  Confidential Computing (CC) measure boot.
> 
>    Caution: This file requires additional review when modified.
>    This library will have external input - PE/COFF image and GPT partition.
> @@ -41,6 +42,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <Library/PeCoffLib.h>
>  #include <Library/SecurityManagementLib.h>
>  #include <Library/HobLib.h>
> +#include <Protocol/CcMeasurement.h>
> +
> +typedef struct {
> +  EFI_TCG2_PROTOCOL             *Tcg2Protocol;
> +  EFI_CC_MEASUREMENT_PROTOCOL   *CcProtocol;
> +} MEASURE_BOOT_PROTOCOLS;
> 
>  //
>  // Flag to check GPT partition. It only need be measured once.
> @@ -55,6 +62,62 @@ UINTN                             mTcg2ImageSize;
>  EFI_HANDLE                        mTcg2CacheMeasuredHandle  = NULL;
>  MEASURED_HOB_DATA                 *mTcg2MeasuredHobData     = NULL;
> 
> +/**
> +  Create CcEvent from Tcg2Event.
> +
> +  CcEvent is similar to Tcg2Event except the MrIndex.
> +
> +  @param  CcProtocol  Pointer to the located Cc Measurement protocol
> instance.
> +  @param  Tcg2Event   Pointer to the Tcg2Event.
> +  @param  EventSize   Size of the Event.
> +  @param  EfiCcEvent  The created CcEvent
> +
> +  @retval EFI_SUCCESS           Successfully create the CcEvent
> +  @retval EFI_INVALID_PARAMETER The input parameter is invalid
> +  @retval EFI_UNSUPPORTED       The input PCRIndex cannot be mapped to Cc
> MR
> +  @retval EFI_OUT_OF_RESOURCES  Out of resource
> +**/
> +EFI_STATUS
> +CreateCcEventFromTcg2Event (
> +  IN  EFI_CC_MEASUREMENT_PROTOCOL   *CcProtocol,
> +  IN  EFI_TCG2_EVENT                *Tcg2Event,
> +  IN  UINT32                        EventSize,
> +  IN OUT EFI_CC_EVENT               **EfiCcEvent
> +  )
> +{
> +  UINT32            MrIndex;
> +  EFI_STATUS        Status;
> +  EFI_CC_EVENT      *CcEvent;
> +
> +  if (Tcg2Event == NULL || CcProtocol == NULL || EfiCcEvent == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  *EfiCcEvent = NULL;
> +
> +  Status = CcProtocol->MapPcrToMrIndex (CcProtocol, Tcg2Event-
> >Header.PCRIndex, &MrIndex);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Cannot map PcrIndex(%d) to MrIndex\n",
> Tcg2Event->Header.PCRIndex));
> +    return Status;
> +  }
> +
> +  CcEvent = (EFI_CC_EVENT *)AllocateZeroPool (Tcg2Event->Size);
> +  if (CcEvent == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  CcEvent->Size                 = Tcg2Event->Size;
> +  CcEvent->Header.HeaderSize    = Tcg2Event->Header.HeaderSize;
> +  CcEvent->Header.HeaderVersion = Tcg2Event->Header.HeaderVersion;
> +  CcEvent->Header.MrIndex       = MrIndex;
> +  CcEvent->Header.EventType     = Tcg2Event->Header.EventType;
> +  CopyMem (CcEvent->Event, Tcg2Event->Event, EventSize);
> +
> +  *EfiCcEvent = CcEvent;
> +
> +  return EFI_SUCCESS;
> +}
> +
>  /**
>    Reads contents of a PE/COFF image in memory buffer.
> 
> @@ -109,7 +172,7 @@ DxeTpm2MeasureBootLibImageRead (
>    Caution: This function may receive untrusted input.
>    The GPT partition table is external input, so this function should parse partition
> data carefully.
> 
> -  @param Tcg2Protocol            Pointer to the located TCG2 protocol instance.
> +  @param MeasureBootProtocols    Pointer to the located MeasureBoot
> protocol instances (i.e. TCG2/Td protocol).
>    @param GptHandle               Handle that GPT partition was installed.
> 
>    @retval EFI_SUCCESS            Successfully measure GPT table.
> @@ -121,8 +184,8 @@ DxeTpm2MeasureBootLibImageRead (
>  EFI_STATUS
>  EFIAPI
>  Tcg2MeasureGptTable (
> -  IN  EFI_TCG2_PROTOCOL  *Tcg2Protocol,
> -  IN  EFI_HANDLE         GptHandle
> +  IN  MEASURE_BOOT_PROTOCOLS  *MeasureBootProtocols,
> +  IN  EFI_HANDLE              GptHandle
>    )
>  {
>    EFI_STATUS                        Status;
> @@ -134,13 +197,29 @@ Tcg2MeasureGptTable (
>    UINTN                             NumberOfPartition;
>    UINT32                            Index;
>    EFI_TCG2_EVENT                    *Tcg2Event;
> +  EFI_CC_EVENT                      *CcEvent;
>    EFI_GPT_DATA                      *GptData;
>    UINT32                            EventSize;
> +  EFI_TCG2_PROTOCOL                 *Tcg2Protocol;
> +  EFI_CC_MEASUREMENT_PROTOCOL       *CcProtocol;
> 
>    if (mTcg2MeasureGptCount > 0) {
>      return EFI_SUCCESS;
>    }
> 
> +  PrimaryHeader = NULL;
> +  EntryPtr      = NULL;
> +  CcEvent       = NULL;
> +  Tcg2Event     = NULL;
> +
> +  Tcg2Protocol  = MeasureBootProtocols->Tcg2Protocol;
> +  CcProtocol    = MeasureBootProtocols->CcProtocol;
> +
> +  if (Tcg2Protocol == NULL && CcProtocol == NULL) {
> +    ASSERT (FALSE);
> +    return EFI_UNSUPPORTED;
> +  }
> +
>    Status = gBS->HandleProtocol (GptHandle, &gEfiBlockIoProtocolGuid,
> (VOID**)&BlockIo);
>    if (EFI_ERROR (Status)) {
>      return EFI_UNSUPPORTED;
> @@ -149,6 +228,7 @@ Tcg2MeasureGptTable (
>    if (EFI_ERROR (Status)) {
>      return EFI_UNSUPPORTED;
>    }
> +
>    //
>    // Read the EFI Partition Table Header
>    //
> @@ -156,6 +236,7 @@ Tcg2MeasureGptTable (
>    if (PrimaryHeader == NULL) {
>      return EFI_OUT_OF_RESOURCES;
>    }
> +
>    Status = DiskIo->ReadDisk (
>                       DiskIo,
>                       BlockIo->Media->MediaId,
> @@ -164,10 +245,20 @@ Tcg2MeasureGptTable (
>                       (UINT8 *)PrimaryHeader
>                       );
>    if (EFI_ERROR (Status)) {
> -    DEBUG ((EFI_D_ERROR, "Failed to Read Partition Table Header!\n"));
> +    DEBUG ((DEBUG_ERROR, "Failed to Read Partition Table Header!\n"));
>      FreePool (PrimaryHeader);
>      return EFI_DEVICE_ERROR;
>    }
> +
> +  //
> +  // PrimaryHeader->SizeOfPartitionEntry should not be zero
> +  //
> +  if (PrimaryHeader->SizeOfPartitionEntry == 0) {
> +    DEBUG ((DEBUG_ERROR, "SizeOfPartitionEntry should not be zero!\n"));
> +    FreePool (PrimaryHeader);
> +    return EFI_BAD_BUFFER_SIZE;
> +  }
> +
>    //
>    // Read the partition entry.
>    //
> @@ -202,15 +293,14 @@ Tcg2MeasureGptTable (
>    }
> 
>    //
> -  // Prepare Data for Measurement
> +  // Prepare Data for Measurement (CcProtocol and Tcg2Protocol)
>    //
>    EventSize = (UINT32)(sizeof (EFI_GPT_DATA) - sizeof (GptData->Partitions)
>                          + NumberOfPartition * PrimaryHeader->SizeOfPartitionEntry);
>    Tcg2Event = (EFI_TCG2_EVENT *) AllocateZeroPool (EventSize + sizeof
> (EFI_TCG2_EVENT) - sizeof(Tcg2Event->Event));
>    if (Tcg2Event == NULL) {
> -    FreePool (PrimaryHeader);
> -    FreePool (EntryPtr);
> -    return EFI_OUT_OF_RESOURCES;
> +    Status = EFI_OUT_OF_RESOURCES;
> +    goto Exit;
>    }
> 
>    Tcg2Event->Size = EventSize + sizeof (EFI_TCG2_EVENT) - sizeof(Tcg2Event-
> >Event);
> @@ -243,22 +333,57 @@ Tcg2MeasureGptTable (
>    }
> 
>    //
> -  // Measure the GPT data
> +  // Measure the GPT data by Tcg2Protocol
>    //
> -  Status = Tcg2Protocol->HashLogExtendEvent (
> -             Tcg2Protocol,
> -             0,
> -             (EFI_PHYSICAL_ADDRESS) (UINTN) (VOID *) GptData,
> -             (UINT64) EventSize,
> -             Tcg2Event
> -             );
> -  if (!EFI_ERROR (Status)) {
> -    mTcg2MeasureGptCount++;
> -  }
> -
> -  FreePool (PrimaryHeader);
> -  FreePool (EntryPtr);
> -  FreePool (Tcg2Event);
> +  if (Tcg2Protocol != NULL) {
> +    Status = Tcg2Protocol->HashLogExtendEvent (
> +               Tcg2Protocol,
> +               0,
> +               (EFI_PHYSICAL_ADDRESS) (UINTN) (VOID *) GptData,
> +               (UINT64) EventSize,
> +               Tcg2Event
> +               );
> +    if (!EFI_ERROR (Status)) {
> +      mTcg2MeasureGptCount++;
> +    }
> +    DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - Tcg2
> MeasureGptTable - %r\n", Status));
> +
> +  } else if (CcProtocol != NULL) {
> +
> +    //
> +    // Measure the GPT data by TdProtocol
> +    //
> +    Status = CreateCcEventFromTcg2Event (CcProtocol, Tcg2Event, EventSize,
> &CcEvent);
> +    if (EFI_ERROR (Status)) {
> +      goto Exit;
> +    }
> +
> +    Status = CcProtocol->HashLogExtendEvent (
> +               CcProtocol,
> +               0,
> +               (EFI_PHYSICAL_ADDRESS) (UINTN) (VOID *) GptData,
> +               (UINT64) EventSize,
> +               CcEvent
> +               );
> +    if (!EFI_ERROR (Status)) {
> +      mTcg2MeasureGptCount++;
> +    }
> +    DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - Cc
> MeasureGptTable - %r\n", Status));
> +  }
> +
> +Exit:
> +  if (PrimaryHeader != NULL) {
> +    FreePool (PrimaryHeader);
> +  }
> +  if (EntryPtr != NULL) {
> +    FreePool (EntryPtr);
> +  }
> +  if (Tcg2Event != NULL) {
> +    FreePool (Tcg2Event);
> +  }
> +  if (CcEvent != NULL) {
> +    FreePool (CcEvent);
> +  }
> 
>    return Status;
>  }
> @@ -271,12 +396,12 @@ Tcg2MeasureGptTable (
>    PE/COFF image is external input, so this function will validate its data structure
>    within this image buffer before use.
> 
> -  @param[in] Tcg2Protocol   Pointer to the located TCG2 protocol instance.
> -  @param[in] ImageAddress   Start address of image buffer.
> -  @param[in] ImageSize      Image size
> -  @param[in] LinkTimeBase   Address that the image is loaded into memory.
> -  @param[in] ImageType      Image subsystem type.
> -  @param[in] FilePath       File path is corresponding to the input image.
> +  @param[in] MeasureBootProtocols   Pointer to the located MeasureBoot
> protocol instances.
> +  @param[in] ImageAddress           Start address of image buffer.
> +  @param[in] ImageSize              Image size
> +  @param[in] LinkTimeBase           Address that the image is loaded into memory.
> +  @param[in] ImageType              Image subsystem type.
> +  @param[in] FilePath               File path is corresponding to the input image.
> 
>    @retval EFI_SUCCESS            Successfully measure image.
>    @retval EFI_OUT_OF_RESOURCES   No enough resource to measure image.
> @@ -287,7 +412,7 @@ Tcg2MeasureGptTable (
>  EFI_STATUS
>  EFIAPI
>  Tcg2MeasurePeImage (
> -  IN  EFI_TCG2_PROTOCOL         *Tcg2Protocol,
> +  IN  MEASURE_BOOT_PROTOCOLS    *MeasureBootProtocols,
>    IN  EFI_PHYSICAL_ADDRESS      ImageAddress,
>    IN  UINTN                     ImageSize,
>    IN  UINTN                     LinkTimeBase,
> @@ -300,9 +425,22 @@ Tcg2MeasurePeImage (
>    EFI_IMAGE_LOAD_EVENT              *ImageLoad;
>    UINT32                            FilePathSize;
>    UINT32                            EventSize;
> +  EFI_CC_EVENT                      *CcEvent;
> +  EFI_CC_MEASUREMENT_PROTOCOL       *CcProtocol;
> +  EFI_TCG2_PROTOCOL                 *Tcg2Protocol;
> 
>    Status        = EFI_UNSUPPORTED;
>    ImageLoad     = NULL;
> +  CcEvent       = NULL;
> +
> +  Tcg2Protocol  = MeasureBootProtocols->Tcg2Protocol;
> +  CcProtocol    = MeasureBootProtocols->CcProtocol;
> +
> +  if (Tcg2Protocol == NULL && CcProtocol == NULL) {
> +    ASSERT (FALSE);
> +    return EFI_UNSUPPORTED;
> +  }
> +
>    FilePathSize  = (UINT32) GetDevicePathSize (FilePath);
> 
>    //
> @@ -334,7 +472,7 @@ Tcg2MeasurePeImage (
>        break;
>      default:
>        DEBUG ((
> -        EFI_D_ERROR,
> +        DEBUG_ERROR,
>          "Tcg2MeasurePeImage: Unknown subsystem type %d",
>          ImageType
>          ));
> @@ -352,28 +490,125 @@ Tcg2MeasurePeImage (
>    //
>    // Log the PE data
>    //
> -  Status = Tcg2Protocol->HashLogExtendEvent (
> -             Tcg2Protocol,
> -             PE_COFF_IMAGE,
> -             ImageAddress,
> -             ImageSize,
> -             Tcg2Event
> -             );
> -  if (Status == EFI_VOLUME_FULL) {
> -    //
> -    // Volume full here means the image is hashed and its result is extended to
> PCR.
> -    // But the event log can't be saved since log area is full.
> -    // Just return EFI_SUCCESS in order not to block the image load.
> -    //
> -    Status = EFI_SUCCESS;
> +  if (Tcg2Protocol != NULL) {
> +    Status = Tcg2Protocol->HashLogExtendEvent (
> +               Tcg2Protocol,
> +               PE_COFF_IMAGE,
> +               ImageAddress,
> +               ImageSize,
> +               Tcg2Event
> +               );
> +    if (Status == EFI_VOLUME_FULL) {
> +      //
> +      // Volume full here means the image is hashed and its result is extended to
> PCR.
> +      // But the event log can't be saved since log area is full.
> +      // Just return EFI_SUCCESS in order not to block the image load.
> +      //
> +      Status = EFI_SUCCESS;
> +    }
> +    DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - Tcg2
> MeasurePeImage - %r\n", Status));
> +
> +  } else if (CcProtocol != NULL) {
> +
> +    Status = CreateCcEventFromTcg2Event (CcProtocol, Tcg2Event, EventSize,
> &CcEvent);
> +    if (EFI_ERROR (Status)) {
> +      goto Finish;
> +    }
> +
> +    Status = CcProtocol->HashLogExtendEvent (
> +               CcProtocol,
> +               PE_COFF_IMAGE,
> +               ImageAddress,
> +               ImageSize,
> +               CcEvent
> +               );
> +    if (Status == EFI_VOLUME_FULL) {
> +      //
> +      // Volume full here means the image is hashed and its result is extended to
> PCR.
> +      // But the event log can't be saved since log area is full.
> +      // Just return EFI_SUCCESS in order not to block the image load.
> +      //
> +      Status = EFI_SUCCESS;
> +    }
> +    DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - Cc
> MeasurePeImage - %r\n", Status));
>    }
> 
>  Finish:
> -  FreePool (Tcg2Event);
> +  if (Tcg2Event != NULL) {
> +    FreePool (Tcg2Event);
> +  }
> +
> +  if (CcEvent != NULL) {
> +    FreePool (CcEvent);
> +  }
> 
>    return Status;
>  }
> 
> +/**
> +  Get the measure boot protocols.
> +
> +  There are 2 measure boot, TCG2 protocol based and Cc measurement
> protocol based.
> +
> +  @param  MeasureBootProtocols  Pointer to the located measure boot
> protocol instances.
> +
> +  @retval EFI_SUCCESS           Sucessfully locate the measure boot protocol
> instances (at least one instance).
> +  @retval EFI_UNSUPPORTED       Measure boot is not supported.
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetMeasureBootProtocols (
> +  MEASURE_BOOT_PROTOCOLS    *MeasureBootProtocols
> +  )
> +{
> +  EFI_STATUS                          Status;
> +  EFI_TCG2_PROTOCOL                   *Tcg2Protocol;
> +  EFI_CC_MEASUREMENT_PROTOCOL         *CcProtocol;
> +  EFI_TCG2_BOOT_SERVICE_CAPABILITY    Tcg2ProtocolCapability;
> +  EFI_CC_BOOT_SERVICE_CAPABILITY      CcProtocolCapability;
> +
> +  CcProtocol = NULL;
> +  Status = gBS->LocateProtocol (&gEfiCcMeasurementProtocolGuid, NULL,
> (VOID **) &CcProtocol);
> +  if (EFI_ERROR (Status)) {
> +    //
> +    // Cc Measurement protocol is not installed.
> +    //
> +    DEBUG ((DEBUG_VERBOSE, "CcMeasurementProtocol is not installed. -
>  %r\n", Status));
> +  } else {
> +    ZeroMem (&CcProtocolCapability, sizeof (CcProtocolCapability));
> +    CcProtocolCapability.Size = sizeof (CcProtocolCapability);
> +    Status = CcProtocol->GetCapability (CcProtocol, &CcProtocolCapability);
> +    if (EFI_ERROR (Status) || CcProtocolCapability.CcType.Type ==
> EFI_CC_TYPE_NONE) {
> +      DEBUG ((DEBUG_ERROR, " CcProtocol->GetCapability returns : %x, %r\n",
> CcProtocolCapability.CcType.Type, Status));
> +      CcProtocol = NULL;
> +    }
> +  }
> +
> +  Tcg2Protocol = NULL;
> +  Status = gBS->LocateProtocol (&gEfiTcg2ProtocolGuid, NULL, (VOID **)
> &Tcg2Protocol);
> +  if (EFI_ERROR (Status)) {
> +    //
> +    // Tcg2 protocol is not installed. So, TPM2 is not present.
> +    //
> +    DEBUG ((DEBUG_VERBOSE, "Tcg2Protocol is not installed. - %r\n", Status));
> +  } else {
> +    Tcg2ProtocolCapability.Size = (UINT8) sizeof (Tcg2ProtocolCapability);
> +    Status = Tcg2Protocol->GetCapability (Tcg2Protocol,
> &Tcg2ProtocolCapability);
> +    if (EFI_ERROR (Status) || (!Tcg2ProtocolCapability.TPMPresentFlag)) {
> +      //
> +      // TPM device doesn't work or activate.
> +      //
> +      DEBUG ((DEBUG_ERROR, "TPMPresentFlag=FALSE %r\n", Status));
> +      Tcg2Protocol = NULL;
> +    }
> +  }
> +
> +  MeasureBootProtocols->Tcg2Protocol = Tcg2Protocol;
> +  MeasureBootProtocols->CcProtocol   = CcProtocol;
> +
> +  return (Tcg2Protocol == NULL && CcProtocol == NULL) ? EFI_UNSUPPORTED:
> EFI_SUCCESS;
> +}
> +
>  /**
>    The security handler is used to abstract platform-specific policy
>    from the DXE core response to an attempt to use a file that returns a
> @@ -422,9 +657,8 @@ DxeTpm2MeasureBootHandler (
>    IN  BOOLEAN                          BootPolicy
>    )
>  {
> -  EFI_TCG2_PROTOCOL                   *Tcg2Protocol;
> +  MEASURE_BOOT_PROTOCOLS              MeasureBootProtocols;
>    EFI_STATUS                          Status;
> -  EFI_TCG2_BOOT_SERVICE_CAPABILITY    ProtocolCapability;
>    EFI_DEVICE_PATH_PROTOCOL            *DevicePathNode;
>    EFI_DEVICE_PATH_PROTOCOL            *OrigDevicePathNode;
>    EFI_HANDLE                          Handle;
> @@ -435,28 +669,23 @@ DxeTpm2MeasureBootHandler (
>    EFI_PHYSICAL_ADDRESS                FvAddress;
>    UINT32                              Index;
> 
> -  Status = gBS->LocateProtocol (&gEfiTcg2ProtocolGuid, NULL, (VOID **)
> &Tcg2Protocol);
> +  MeasureBootProtocols.Tcg2Protocol = NULL;
> +  MeasureBootProtocols.CcProtocol   = NULL;
> +
> +  Status = GetMeasureBootProtocols(&MeasureBootProtocols);
> +
>    if (EFI_ERROR (Status)) {
>      //
> -    // Tcg2 protocol is not installed. So, TPM2 is not present.
> +    // None of Measured boot protocols (Tcg2, Cc) is installed.
>      // Don't do any measurement, and directly return EFI_SUCCESS.
>      //
> -    DEBUG ((EFI_D_VERBOSE, "DxeTpm2MeasureBootHandler - Tcg2 - %r\n",
> Status));
> +    DEBUG ((DEBUG_INFO, "None of Tcg2Protocol/CcMeasurementProtocol is
> installed.\n"));
>      return EFI_SUCCESS;
>    }
> 
> -  ProtocolCapability.Size = (UINT8) sizeof (ProtocolCapability);
> -  Status = Tcg2Protocol->GetCapability (
> -                           Tcg2Protocol,
> -                           &ProtocolCapability
> -                           );
> -  if (EFI_ERROR (Status) || (!ProtocolCapability.TPMPresentFlag)) {
> -    //
> -    // TPM device doesn't work or activate.
> -    //
> -    DEBUG ((EFI_D_ERROR, "DxeTpm2MeasureBootHandler (%r) -
> TPMPresentFlag - %x\n", Status, ProtocolCapability.TPMPresentFlag));
> -    return EFI_SUCCESS;
> -  }
> +  DEBUG ((DEBUG_INFO, "Tcg2Protocol = %p, TdProtocol = %p\n",
> +                      MeasureBootProtocols.Tcg2Protocol,
> +                      MeasureBootProtocols.CcProtocol));
> 
>    //
>    // Copy File Device Path
> @@ -502,8 +731,8 @@ DxeTpm2MeasureBootHandler (
>              //
>              // Measure GPT disk.
>              //
> -            Status = Tcg2MeasureGptTable (Tcg2Protocol, Handle);
> -            DEBUG ((EFI_D_INFO, "DxeTpm2MeasureBootHandler -
> Tcg2MeasureGptTable - %r\n", Status));
> +            Status = Tcg2MeasureGptTable (&MeasureBootProtocols, Handle);
> +
>              if (!EFI_ERROR (Status)) {
>                //
>                // GPT disk check done.
> @@ -647,14 +876,13 @@ DxeTpm2MeasureBootHandler (
>      // Measure PE image into TPM log.
>      //
>      Status = Tcg2MeasurePeImage (
> -               Tcg2Protocol,
> +               &MeasureBootProtocols,
>                 (EFI_PHYSICAL_ADDRESS) (UINTN) FileBuffer,
>                 FileSize,
>                 (UINTN) ImageContext.ImageAddress,
>                 ImageContext.ImageType,
>                 DevicePathNode
>                 );
> -    DEBUG ((EFI_D_INFO, "DxeTpm2MeasureBootHandler -
> Tcg2MeasurePeImage - %r\n", Status));
>    }
> 
>    //
> @@ -665,7 +893,7 @@ Finish:
>      FreePool (OrigDevicePathNode);
>    }
> 
> -  DEBUG ((EFI_D_INFO, "DxeTpm2MeasureBootHandler - %r\n", Status));
> +  DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - %r\n", Status));
> 
>    return Status;
>  }
> diff --git
> a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
> b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
> index 2506abbe7c8b..6dca79a20c93 100644
> ---
> a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
> +++
> b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
> @@ -1,5 +1,5 @@
>  ## @file
> -#  Provides security service for TPM 2.0 measured boot
> +#  Provides security service for TPM 2.0 measured boot and Confidential
> Computing measure boot.
>  #
>  #  Spec Compliance Info:
>  #    "TCG PC Client Platform Firmware Profile Specification for TPM Family 2.0
> Level 00 Revision 1.03 v51"
> @@ -61,6 +61,7 @@
> 
>  [Protocols]
>    gEfiTcg2ProtocolGuid                  ## SOMETIMES_CONSUMES
> +  gEfiCcMeasurementProtocolGuid         ## SOMETIMES_CONSUMES
>    gEfiFirmwareVolumeBlockProtocolGuid   ## SOMETIMES_CONSUMES
>    gEfiBlockIoProtocolGuid               ## SOMETIMES_CONSUMES
>    gEfiDiskIoProtocolGuid                ## SOMETIMES_CONSUMES
> --
> 2.29.2.windows.2


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

* Re: [PATCH V4 3/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpmMeasurementLib
  2021-11-02  2:50 ` [PATCH V4 3/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpmMeasurementLib Min Xu
@ 2021-11-02  6:24   ` Yao, Jiewen
  2021-11-03  3:01     ` Min Xu
  2021-11-02  9:45   ` Sami Mujawar
  1 sibling, 1 reply; 23+ messages in thread
From: Yao, Jiewen @ 2021-11-02  6:24 UTC (permalink / raw)
  To: Xu, Min M, devel@edk2.groups.io
  Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang, Wang, Jian J,
	Sami Mujawar, Gerd Hoffmann

May I know which platform you have run the test?

I think we need cover both TD and TPM in real platform.

> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Tuesday, November 2, 2021 10:51 AM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu,
> Zhiguang <zhiguang.liu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>; Sami Mujawar
> <sami.mujawar@arm.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [PATCH V4 3/3] SecurityPkg: Support CcMeasurementProtocol in
> DxeTpmMeasurementLib
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3625
> 
> DxeTpmMeasurementLib supports TPM based measurement in DXE phase.
> After CcMeasurementProtocol is introduced, CC based measurement needs
> to be supported in DxeTpmMeasurementLib as well.
> 
> In TpmMeasureAndLogData, CC based measurement will be first called.
> If it failed, TPM based measurement will be called sequentially.
> Currently there is an assumption that CC based measurement and
> TPM based measurement won't be exist at the same time.If the
> assumption is not true in the future, we will revisit here then.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
>  .../DxeTpmMeasurementLib.c                    | 91 ++++++++++++++++++-
>  .../DxeTpmMeasurementLib.inf                  |  9 +-
>  2 files changed, 92 insertions(+), 8 deletions(-)
> 
> diff --git
> a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
> b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
> index 061136ee7860..2ddb9033a0d5 100644
> --- a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
> +++ b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
> @@ -1,5 +1,6 @@
>  /** @file
> -  This library is used by other modules to measure data to TPM.
> +  This library is used by other modules to measure data to TPM and Confidential
> +  Computing (CC) measure registers.
> 
>  Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved. <BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -19,8 +20,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  #include <Guid/Acpi.h>
>  #include <IndustryStandard/Acpi.h>
> -
> -
> +#include <Protocol/CcMeasurement.h>
> 
>  /**
>    Tpm12 measure and log data, and extend the measurement result into a
> specific PCR.
> @@ -149,6 +149,73 @@ Tpm20MeasureAndLogData (
>    return Status;
>  }
> 
> +/**
> +  Cc measure and log data, and extend the measurement result into a
> +  specific CC MR.
> +
> +  @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
> +
> +  @retval EFI_SUCCESS           Operation completed successfully.
> +  @retval EFI_UNSUPPORTED       Tdx device not available.
> +  @retval EFI_OUT_OF_RESOURCES  Out of memory.
> +  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
> +**/
> +EFI_STATUS
> +EFIAPI
> +CcMeasureAndLogData (
> +  IN UINT32             PcrIndex,
> +  IN UINT32             EventType,
> +  IN VOID               *EventLog,
> +  IN UINT32             LogLen,
> +  IN VOID               *HashData,
> +  IN UINT64             HashDataLen
> +  )
> +{
> +  EFI_STATUS                    Status;
> +  EFI_CC_MEASUREMENT_PROTOCOL  *CcProtocol;
> +  EFI_CC_EVENT                 *EfiCcEvent;
> +  UINT32                        MrIndex;
> +
> +  Status = gBS->LocateProtocol (&gEfiCcMeasurementProtocolGuid, NULL,
> (VOID **) &CcProtocol);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = CcProtocol->MapPcrToMrIndex (CcProtocol, PcrIndex, &MrIndex);
> +  if (EFI_ERROR (Status)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  EfiCcEvent = (EFI_CC_EVENT *) AllocateZeroPool (LogLen + sizeof
> (EFI_CC_EVENT));
> +  if(EfiCcEvent == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  EfiCcEvent->Size = (UINT32) LogLen + sizeof (EFI_CC_EVENT) - sizeof
> (EfiCcEvent->Event);
> +  EfiCcEvent->Header.HeaderSize    = sizeof (EFI_CC_EVENT_HEADER);
> +  EfiCcEvent->Header.HeaderVersion = EFI_CC_EVENT_HEADER_VERSION;
> +  EfiCcEvent->Header.MrIndex       = MrIndex;
> +  EfiCcEvent->Header.EventType     = EventType;
> +  CopyMem (&EfiCcEvent->Event[0], EventLog, LogLen);
> +
> +  Status = CcProtocol->HashLogExtendEvent (
> +                           CcProtocol,
> +                           0,
> +                           (EFI_PHYSICAL_ADDRESS) (UINTN) HashData,
> +                           HashDataLen,
> +                           EfiCcEvent
> +                           );
> +  FreePool (EfiCcEvent);
> +
> +  return Status;
> +}
> +
> +
>  /**
>    Tpm measure and log data, and extend the measurement result into a specific
> PCR.
> 
> @@ -178,9 +245,9 @@ TpmMeasureAndLogData (
>    EFI_STATUS  Status;
> 
>    //
> -  // Try to measure using Tpm20 protocol
> +  // Try to measure using Cc measurement protocol
>    //
> -  Status = Tpm20MeasureAndLogData(
> +  Status = CcMeasureAndLogData (
>               PcrIndex,
>               EventType,
>               EventLog,
> @@ -189,6 +256,20 @@ TpmMeasureAndLogData (
>               HashDataLen
>               );
> 
> +  if (EFI_ERROR (Status)) {
> +    //
> +    // Try to measure using Tpm20 protocol
> +    //
> +    Status = Tpm20MeasureAndLogData(
> +               PcrIndex,
> +               EventType,
> +               EventLog,
> +               LogLen,
> +               HashData,
> +               HashDataLen
> +               );
> +  }
> +
>    if (EFI_ERROR (Status)) {
>      //
>      // Try to measure using Tpm1.2 protocol
> diff --git
> a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> index 7d41bc41f95d..3af3d4e33b25 100644
> --- a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> +++
> b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> @@ -1,5 +1,7 @@
>  ## @file
> -#  Provides TPM measurement functions for TPM1.2 and TPM 2.0
> +#  Provides below measurement functions:
> +#    1. TPM measurement functions for TPM1.2 and TPM 2.0
> +#    2. Confidential Computing (CC) measurement functions
>  #
>  #  This library provides TpmMeasureAndLogData() to measure and log data, and
>  #  extend the measurement result into a specific PCR.
> @@ -40,5 +42,6 @@
>    UefiBootServicesTableLib
> 
>  [Protocols]
> -  gEfiTcgProtocolGuid           ## SOMETIMES_CONSUMES
> -  gEfiTcg2ProtocolGuid          ## SOMETIMES_CONSUMES
> +  gEfiTcgProtocolGuid               ## SOMETIMES_CONSUMES
> +  gEfiTcg2ProtocolGuid              ## SOMETIMES_CONSUMES
> +  gEfiCcMeasurementProtocolGuid     ## SOMETIMES_CONSUMES
> --
> 2.29.2.windows.2


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

* Re: [PATCH V4 1/3] MdePkg: Introduce CcMeasurementProtocol for CC Guest firmware
  2021-11-02  2:50 ` [PATCH V4 1/3] MdePkg: Introduce CcMeasurementProtocol for CC Guest firmware Min Xu
  2021-11-02  6:24   ` Yao, Jiewen
@ 2021-11-02  9:41   ` Sami Mujawar
  2021-11-04  5:51   ` 回复: " gaoliming
  2 siblings, 0 replies; 23+ messages in thread
From: Sami Mujawar @ 2021-11-02  9:41 UTC (permalink / raw)
  To: Min Xu, devel
  Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Jiewen Yao,
	Jian J Wang, Ken Lu, Gerd Hoffmann, nd

Hi Min,

Thank you for this patch.

These changes look good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 02/11/2021 02:50 AM, Min Xu wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3625
>
> CC guest is a Confidential Computing guest. If CC Guest firmware
> supports measurement and an event is created, CC Guest firmware
> is designed to report the event log with the same data structure
> in TCG-Platform-Firmware-Profile specification with
> EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 format.
>
> The CC Guest firmware supports measurement. It is designed to
> produce EFI_CC_MEASUREMENT_PROTOCOL with new GUID
> EFI_CC_MEASUREMENT_PROTOCOL_GUID to report event log and provides
> hash capability.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Ken Lu <ken.lu@intel.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
>   MdePkg/Include/Protocol/CcMeasurement.h | 305 ++++++++++++++++++++++++
>   MdePkg/MdePkg.dec                       |   4 +
>   2 files changed, 309 insertions(+)
>   create mode 100644 MdePkg/Include/Protocol/CcMeasurement.h
>
> diff --git a/MdePkg/Include/Protocol/CcMeasurement.h b/MdePkg/Include/Protocol/CcMeasurement.h
> new file mode 100644
> index 000000000000..eaedbfffdb6a
> --- /dev/null
> +++ b/MdePkg/Include/Protocol/CcMeasurement.h
> @@ -0,0 +1,305 @@
> +/** @file
> +  If CC Guest firmware supports measurement and an event is created,
> +  CC Guest firmware is designed to report the event log with the same
> +  data structure in TCG-Platform-Firmware-Profile specification with
> +  EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 format.
> +
> +  The CC Guest firmware supports measurement, the CC Guest Firmware is
> +  designed to produce EFI_CC_MEASUREMENT_PROTOCOL with new GUID
> +  EFI_CC_MEASUREMENT_PROTOCOL_GUID to report event log and provides hash
> +  capability.
> +
> +Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef CC_MEASUREMENT_PROTOCOL_H_
> +#define CC_MEASUREMENT_PROTOCOL_H_
> +
> +#include <IndustryStandard/UefiTcgPlatform.h>
> +
> +#define EFI_CC_MEASUREMENT_PROTOCOL_GUID  \
> +  { 0x96751a3d, 0x72f4, 0x41a6, { 0xa7, 0x94, 0xed, 0x5d, 0x0e, 0x67, 0xae, 0x6b }}
> +extern EFI_GUID gEfiCcMeasurementProtocolGuid;
> +
> +typedef struct _EFI_CC_MEASUREMENT_PROTOCOL EFI_CC_MEASUREMENT_PROTOCOL;
> +
> +typedef struct {
> +  UINT8 Major;
> +  UINT8 Minor;
> +} EFI_CC_VERSION;
> +
> +//
> +// EFI_CC Type/SubType definition
> +//
> +#define EFI_CC_TYPE_NONE   0
> +#define EFI_CC_TYPE_SEV    1
> +#define EFI_CC_TYPE_TDX    2
> +
> +typedef struct {
> +  UINT8          Type;
> +  UINT8          SubType;
> +} EFI_CC_TYPE;
> +
> +typedef UINT32                      EFI_CC_EVENT_LOG_BITMAP;
> +typedef UINT32                      EFI_CC_EVENT_LOG_FORMAT;
> +typedef UINT32                      EFI_CC_EVENT_ALGORITHM_BITMAP;
> +typedef UINT32                      EFI_CC_MR_INDEX;
> +
> +//
> +// Intel TDX measure register index
> +//
> +#define TDX_MR_INDEX_MRTD  0
> +#define TDX_MR_INDEX_RTMR0 1
> +#define TDX_MR_INDEX_RTMR1 2
> +#define TDX_MR_INDEX_RTMR2 3
> +#define TDX_MR_INDEX_RTMR3 4
> +
> +
> +#define EFI_CC_EVENT_LOG_FORMAT_TCG_2   0x00000002
> +#define EFI_CC_BOOT_HASH_ALG_SHA384     0x00000004
> +
> +//
> +// This bit is shall be set when an event shall be extended but not logged.
> +//
> +#define EFI_CC_FLAG_EXTEND_ONLY       0x0000000000000001
> +//
> +// This bit shall be set when the intent is to measure a PE/COFF image.
> +//
> +#define EFI_CC_FLAG_PE_COFF_IMAGE     0x0000000000000010
> +
> +#pragma pack (1)
> +
> +#define EFI_CC_EVENT_HEADER_VERSION   1
> +
> +typedef struct {
> +  //
> +  // Size of the event header itself (sizeof(EFI_CC_EVENT_HEADER)).
> +  //
> +  UINT32            HeaderSize;
> +  //
> +  // Header version. For this version of this specification, the value shall be 1.
> +  //
> +  UINT16            HeaderVersion;
> +  //
> +  // Index of the MR (measurement register) that shall be extended.
> +  //
> +  EFI_CC_MR_INDEX   MrIndex;
> +  //
> +  // Type of the event that shall be extended (and optionally logged).
> +  //
> +  UINT32            EventType;
> +} EFI_CC_EVENT_HEADER;
> +
> +typedef struct {
> +  //
> +  // Total size of the event including the Size component, the header and the Event data.
> +  //
> +  UINT32                  Size;
> +  EFI_CC_EVENT_HEADER     Header;
> +  UINT8                   Event[1];
> +} EFI_CC_EVENT;
> +
> +#pragma pack()
> +
> +
> +typedef struct {
> +  //
> +  // Allocated size of the structure
> +  //
> +  UINT8                            Size;
> +  //
> +  // Version of the EFI_CC_BOOT_SERVICE_CAPABILITY structure itself.
> +  // For this version of the protocol, the Major version shall be set to 1
> +  // and the Minor version shall be set to 0.
> +  //
> +  EFI_CC_VERSION                   StructureVersion;
> +  //
> +  // Version of the EFI CC Measurement protocol.
> +  // For this version of the protocol, the Major version shall be set to 1
> +  // and the Minor version shall be set to 0.
> +  //
> +  EFI_CC_VERSION                   ProtocolVersion;
> +  //
> +  // Supported hash algorithms
> +  //
> +  EFI_CC_EVENT_ALGORITHM_BITMAP   HashAlgorithmBitmap;
> +  //
> +  // Bitmap of supported event log formats
> +  //
> +  EFI_CC_EVENT_LOG_BITMAP          SupportedEventLogs;
> +
> +  //
> +  // Indicates the CC type
> +  //
> +  EFI_CC_TYPE                      CcType;
> +} EFI_CC_BOOT_SERVICE_CAPABILITY;
> +
> +/**
> +  The EFI_CC_MEASUREMENT_PROTOCOL GetCapability function call provides protocol
> +  capability information and state information.
> +
> +  @param[in]      This               Indicates the calling context
> +  @param[in, out] ProtocolCapability The caller allocates memory for a EFI_CC_BOOT_SERVICE_CAPABILITY
> +                                     structure and sets the size field to the size of the structure allocated.
> +                                     The callee fills in the fields with the EFI CC BOOT Service capability
> +                                     information and the current CC information.
> +
> +  @retval EFI_SUCCESS            Operation completed successfully.
> +  @retval EFI_DEVICE_ERROR       The command was unsuccessful.
> +                                 The ProtocolCapability variable will not be populated.
> +  @retval EFI_INVALID_PARAMETER  One or more of the parameters are incorrect.
> +                                 The ProtocolCapability variable will not be populated.
> +  @retval EFI_BUFFER_TOO_SMALL   The ProtocolCapability variable is too small to hold the full response.
> +                                 It will be partially populated (required Size field will be set).
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EFI_CC_GET_CAPABILITY) (
> +  IN     EFI_CC_MEASUREMENT_PROTOCOL       *This,
> +  IN OUT EFI_CC_BOOT_SERVICE_CAPABILITY    *ProtocolCapability
> +  );
> +
> +/**
> +  The EFI_CC_MEASUREMENT_PROTOCOL Get Event Log function call allows a caller to
> +  retrieve the address of a given event log and its last entry.
> +
> +  @param[in]  This               Indicates the calling context
> +  @param[in]  EventLogFormat     The type of the event log for which the information is requested.
> +  @param[out] EventLogLocation   A pointer to the memory address of the event log.
> +  @param[out] EventLogLastEntry  If the Event Log contains more than one entry, this is a pointer to the
> +                                 address of the start of the last entry in the event log in memory.
> +  @param[out] EventLogTruncated  If the Event Log is missing at least one entry because an event would
> +                                 have exceeded the area allocated for events, this value is set to TRUE.
> +                                 Otherwise, the value will be FALSE and the Event Log will be complete.
> +
> +  @retval EFI_SUCCESS            Operation completed successfully.
> +  @retval EFI_INVALID_PARAMETER  One or more of the parameters are incorrect
> +                                 (e.g. asking for an event log whose format is not supported).
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EFI_CC_GET_EVENT_LOG) (
> +  IN  EFI_CC_MEASUREMENT_PROTOCOL     *This,
> +  IN  EFI_CC_EVENT_LOG_FORMAT         EventLogFormat,
> +  OUT EFI_PHYSICAL_ADDRESS            *EventLogLocation,
> +  OUT EFI_PHYSICAL_ADDRESS            *EventLogLastEntry,
> +  OUT BOOLEAN                         *EventLogTruncated
> +  );
> +
> +/**
> +  The EFI_CC_MEASUREMENT_PROTOCOL HashLogExtendEvent function call provides
> +  callers with an opportunity to extend and optionally log events without requiring
> +  knowledge of actual CC commands.
> +  The extend operation will occur even if this function cannot create an event
> +  log entry (e.g. due to the event log being full).
> +
> +  @param[in]  This               Indicates the calling context
> +  @param[in]  Flags              Bitmap providing additional information.
> +  @param[in]  DataToHash         Physical address of the start of the data buffer to be hashed.
> +  @param[in]  DataToHashLen      The length in bytes of the buffer referenced by DataToHash.
> +  @param[in]  EfiCcEvent        Pointer to data buffer containing information about the event.
> +
> +  @retval EFI_SUCCESS            Operation completed successfully.
> +  @retval EFI_DEVICE_ERROR       The command was unsuccessful.
> +  @retval EFI_VOLUME_FULL        The extend operation occurred, but the event could not be written to one or more event logs.
> +  @retval EFI_INVALID_PARAMETER  One or more of the parameters are incorrect.
> +  @retval EFI_UNSUPPORTED        The PE/COFF image type is not supported.
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI * EFI_CC_HASH_LOG_EXTEND_EVENT) (
> +  IN EFI_CC_MEASUREMENT_PROTOCOL    *This,
> +  IN UINT64                         Flags,
> +  IN EFI_PHYSICAL_ADDRESS           DataToHash,
> +  IN UINT64                         DataToHashLen,
> +  IN EFI_CC_EVENT                   *EfiCcEvent
> +  );
> +
> +/**
> +  The EFI_CC_MEASUREMENT_PROTOCOL MapPcrToMrIndex function call provides callers
> +  the info on TPM PCR <-> CC MR mapping information.
> +
> +  @param[in]  This               Indicates the calling context
> +  @param[in]  PcrIndex           TPM PCR index.
> +  @param[out] MrIndex            CC MR index.
> +
> +  @retval EFI_SUCCESS            The MrIndex is returned.
> +  @retval EFI_INVALID_PARAMETER  The MrIndex is NULL.
> +  @retval EFI_UNSUPPORTED        The PcrIndex is invalid.
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI * EFI_CC_MAP_PCR_TO_MR_INDEX) (
> +  IN  EFI_CC_MEASUREMENT_PROTOCOL    *This,
> +  IN  TCG_PCRINDEX                   PcrIndex,
> +  OUT EFI_CC_MR_INDEX                *MrIndex
> +  );
> +
> +struct _EFI_CC_MEASUREMENT_PROTOCOL {
> +  EFI_CC_GET_CAPABILITY                     GetCapability;
> +  EFI_CC_GET_EVENT_LOG                      GetEventLog;
> +  EFI_CC_HASH_LOG_EXTEND_EVENT              HashLogExtendEvent;
> +  EFI_CC_MAP_PCR_TO_MR_INDEX                MapPcrToMrIndex;
> +};
> +
> +//
> +// CC event log
> +//
> +
> +#pragma pack(1)
> +
> +//
> +// Crypto Agile Log Entry Format.
> +// It is similar with TCG_PCR_EVENT2 except the field of MrIndex and PCRIndex.
> +//
> +typedef struct {
> +  EFI_CC_MR_INDEX     MrIndex;
> +  UINT32              EventType;
> +  TPML_DIGEST_VALUES  Digests;
> +  UINT32              EventSize;
> +  UINT8               Event[1];
> +} CC_EVENT;
> +
> +//
> +// EFI CC Event Header
> +// It is similar with TCG_PCR_EVENT2_HDR except the field of MrIndex and PCRIndex
> +//
> +typedef struct {
> +  EFI_CC_MR_INDEX     MrIndex;
> +  UINT32              EventType;
> +  TPML_DIGEST_VALUES  Digests;
> +  UINT32              EventSize;
> +} CC_EVENT_HDR;
> +
> +#pragma pack()
> +
> +//
> +// Log entries after Get Event Log service
> +//
> +
> +#define EFI_CC_FINAL_EVENTS_TABLE_VERSION  1
> +
> +typedef struct {
> +  //
> +  // The version of this structure. It shall be set to 1.
> +  //
> +  UINT64                  Version;
> +  //
> +  // Number of events recorded after invocation of GetEventLog API
> +  //
> +  UINT64                  NumberOfEvents;
> +  //
> +  // List of events of type CC_EVENT.
> +  //
> +  //CC_EVENT              Event[1];
> +} EFI_CC_FINAL_EVENTS_TABLE;
> +
> +
> +#define EFI_CC_FINAL_EVENTS_TABLE_GUID \
> +  {0xdd4a4648, 0x2de7, 0x4665, {0x96, 0x4d, 0x21, 0xd9, 0xef, 0x5f, 0xb4, 0x46}}
> +
> +extern EFI_GUID gEfiCcFinalEventsTableGuid;
> +
> +#endif
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> index 8b18415b107a..8e19be662ada 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -1011,6 +1011,10 @@
>     ## Include/Protocol/PcdInfo.h
>     gGetPcdInfoProtocolGuid        = { 0x5be40f57, 0xfa68, 0x4610, { 0xbb, 0xbf, 0xe9, 0xc5, 0xfc, 0xda, 0xd3, 0x65 } }
>   
> +  ## Include/Protocol/CcMeasurement.h
> +  gEfiCcMeasurementProtocolGuid  = { 0x96751a3d, 0x72f4, 0x41a6, { 0xa7, 0x94, 0xed, 0x5d, 0x0e, 0x67, 0xae, 0x6b }}
> +  gEfiCcFinalEventsTableGuid     = { 0xdd4a4648, 0x2de7, 0x4665, { 0x96, 0x4d, 0x21, 0xd9, 0xef, 0x5f, 0xb4, 0x46 }}
> +
>     #
>     # Protocols defined in PI1.0.
>     #


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

* Re: [PATCH V4 2/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpm2MeasureBootLib
  2021-11-02  2:50 ` [PATCH V4 2/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpm2MeasureBootLib Min Xu
  2021-11-02  6:24   ` Yao, Jiewen
@ 2021-11-02  9:43   ` Sami Mujawar
  2021-11-05  2:12     ` [edk2-devel] " Min Xu
  1 sibling, 1 reply; 23+ messages in thread
From: Sami Mujawar @ 2021-11-02  9:43 UTC (permalink / raw)
  To: Min Xu, devel
  Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Jiewen Yao,
	Jian J Wang, Gerd Hoffmann, nd

[-- Attachment #1: Type: text/plain, Size: 23262 bytes --]

Hi Min,

Thank you for this patch.

Please find my feedback inline marked [SAMI].

Regards,

Sami Mujawar


On 02/11/2021 02:50 AM, Min Xu wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3625
>
> DxeTpm2MeasureBootLib supports TPM2 based measure boot. After
> CcMeasurementProtocol is introduced, CC based measure boot needs to
> be supported in DxeTpm2MeasureBootLib as well.
>
> There are 2 major changes in this commit.
>
> 1. MEASURE_BOOT_PROTOCOLS is defined to store the instances of TCG2
> protocol and TEE protocol. In the DxeTpm2MeasureBootHandler above 2
> measure boot protocol instances will be located. Then the located
> protocol instances will be called to do the measure boot.
>
> 2. CcEvent is similar to Tcg2Event except the MrIndex and PcrIndex.
> CreateCcEventFromTcg2Event is used to create the CcEvent based on the
> Tcg2Event.
>
> Above 2 changes make the minimize changes to the existing code.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
>   .../DxeTpm2MeasureBootLib.c                   | 366 ++++++++++++++----
>   .../DxeTpm2MeasureBootLib.inf                 |   3 +-
>   2 files changed, 299 insertions(+), 70 deletions(-)
>
> diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
> index 92eac715800f..af889b6ed3ed 100644
> --- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
> +++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
> @@ -1,5 +1,6 @@
>   /** @file
> -  The library instance provides security service of TPM2 measure boot.
> +  The library instance provides security service of TPM2 measure boot and
> +  Confidential Computing (CC) measure boot.
>   
>     Caution: This file requires additional review when modified.
>     This library will have external input - PE/COFF image and GPT partition.
> @@ -41,6 +42,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>   #include <Library/PeCoffLib.h>
>   #include <Library/SecurityManagementLib.h>
>   #include <Library/HobLib.h>
> +#include <Protocol/CcMeasurement.h>
> +
> +typedef struct {
> +  EFI_TCG2_PROTOCOL             *Tcg2Protocol;
> +  EFI_CC_MEASUREMENT_PROTOCOL   *CcProtocol;
> +} MEASURE_BOOT_PROTOCOLS;
>   
>   //
>   // Flag to check GPT partition. It only need be measured once.
> @@ -55,6 +62,62 @@ UINTN                             mTcg2ImageSize;
>   EFI_HANDLE                        mTcg2CacheMeasuredHandle  = NULL;
>   MEASURED_HOB_DATA                 *mTcg2MeasuredHobData     = NULL;
>   
> +/**
> +  Create CcEvent from Tcg2Event.
> +
> +  CcEvent is similar to Tcg2Event except the MrIndex.
> +
> +  @param  CcProtocol  Pointer to the located Cc Measurement protocol instance.
> +  @param  Tcg2Event   Pointer to the Tcg2Event.
> +  @param  EventSize   Size of the Event.
> +  @param  EfiCcEvent  The created CcEvent
> +
> +  @retval EFI_SUCCESS           Successfully create the CcEvent
> +  @retval EFI_INVALID_PARAMETER The input parameter is invalid
> +  @retval EFI_UNSUPPORTED       The input PCRIndex cannot be mapped to Cc MR
> +  @retval EFI_OUT_OF_RESOURCES  Out of resource
> +**/
> +EFI_STATUS
[SAMI] Is EFIAPI needed here?
> +CreateCcEventFromTcg2Event (
> +  IN  EFI_CC_MEASUREMENT_PROTOCOL   *CcProtocol,
> +  IN  EFI_TCG2_EVENT                *Tcg2Event,
> +  IN  UINT32                        EventSize,
> +  IN OUT EFI_CC_EVENT               **EfiCcEvent
> +  )
> +{
> +  UINT32            MrIndex;
[SAMI] I think it may be good to use the typedef for the measurment 
register index here i.e.EFI_CC_MR_INDEX.
> +  EFI_STATUS        Status;
> +  EFI_CC_EVENT      *CcEvent;
> +
> +  if (Tcg2Event == NULL || CcProtocol == NULL || EfiCcEvent == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  *EfiCcEvent = NULL;
> +
> +  Status = CcProtocol->MapPcrToMrIndex (CcProtocol, Tcg2Event->Header.PCRIndex, &MrIndex);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Cannot map PcrIndex(%d) to MrIndex\n", Tcg2Event->Header.PCRIndex));
> +    return Status;
> +  }
> +
> +  CcEvent = (EFI_CC_EVENT *)AllocateZeroPool (Tcg2Event->Size);
> +  if (CcEvent == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  CcEvent->Size                 = Tcg2Event->Size;
> +  CcEvent->Header.HeaderSize    = Tcg2Event->Header.HeaderSize;
> +  CcEvent->Header.HeaderVersion = Tcg2Event->Header.HeaderVersion;
> +  CcEvent->Header.MrIndex       = MrIndex;
> +  CcEvent->Header.EventType     = Tcg2Event->Header.EventType;
> +  CopyMem (CcEvent->Event, Tcg2Event->Event, EventSize);
> +
> +  *EfiCcEvent = CcEvent;
> +
> +  return EFI_SUCCESS;
> +}
> +
>   /**
>     Reads contents of a PE/COFF image in memory buffer.
>   
> @@ -109,7 +172,7 @@ DxeTpm2MeasureBootLibImageRead (
>     Caution: This function may receive untrusted input.
>     The GPT partition table is external input, so this function should parse partition data carefully.
>   
> -  @param Tcg2Protocol            Pointer to the located TCG2 protocol instance.
> +  @param MeasureBootProtocols    Pointer to the located MeasureBoot protocol instances (i.e. TCG2/Td protocol).
>     @param GptHandle               Handle that GPT partition was installed.
>   
>     @retval EFI_SUCCESS            Successfully measure GPT table.
> @@ -121,8 +184,8 @@ DxeTpm2MeasureBootLibImageRead (
>   EFI_STATUS
>   EFIAPI
>   Tcg2MeasureGptTable (
> -  IN  EFI_TCG2_PROTOCOL  *Tcg2Protocol,
> -  IN  EFI_HANDLE         GptHandle
> +  IN  MEASURE_BOOT_PROTOCOLS  *MeasureBootProtocols,
> +  IN  EFI_HANDLE              GptHandle
>     )
>   {
>     EFI_STATUS                        Status;
> @@ -134,13 +197,29 @@ Tcg2MeasureGptTable (
>     UINTN                             NumberOfPartition;
>     UINT32                            Index;
>     EFI_TCG2_EVENT                    *Tcg2Event;
> +  EFI_CC_EVENT                      *CcEvent;
>     EFI_GPT_DATA                      *GptData;
>     UINT32                            EventSize;
> +  EFI_TCG2_PROTOCOL                 *Tcg2Protocol;
> +  EFI_CC_MEASUREMENT_PROTOCOL       *CcProtocol;
>   
>     if (mTcg2MeasureGptCount > 0) {
>       return EFI_SUCCESS;
>     }
>   
> +  PrimaryHeader = NULL;
> +  EntryPtr      = NULL;
> +  CcEvent       = NULL;
> +  Tcg2Event     = NULL;
> +
> +  Tcg2Protocol  = MeasureBootProtocols->Tcg2Protocol;
> +  CcProtocol    = MeasureBootProtocols->CcProtocol;
> +
> +  if (Tcg2Protocol == NULL && CcProtocol == NULL) {
> +    ASSERT (FALSE);
> +    return EFI_UNSUPPORTED;
> +  }
> +
>     Status = gBS->HandleProtocol (GptHandle, &gEfiBlockIoProtocolGuid, (VOID**)&BlockIo);
>     if (EFI_ERROR (Status)) {
>       return EFI_UNSUPPORTED;
> @@ -149,6 +228,7 @@ Tcg2MeasureGptTable (
>     if (EFI_ERROR (Status)) {
>       return EFI_UNSUPPORTED;
>     }
> +
>     //
>     // Read the EFI Partition Table Header
>     //
> @@ -156,6 +236,7 @@ Tcg2MeasureGptTable (
>     if (PrimaryHeader == NULL) {
>       return EFI_OUT_OF_RESOURCES;
>     }
> +
>     Status = DiskIo->ReadDisk (
>                        DiskIo,
>                        BlockIo->Media->MediaId,
> @@ -164,10 +245,20 @@ Tcg2MeasureGptTable (
>                        (UINT8 *)PrimaryHeader
>                        );
>     if (EFI_ERROR (Status)) {
> -    DEBUG ((EFI_D_ERROR, "Failed to Read Partition Table Header!\n"));
> +    DEBUG ((DEBUG_ERROR, "Failed to Read Partition Table Header!\n"));
>       FreePool (PrimaryHeader);
>       return EFI_DEVICE_ERROR;
>     }
> +
> +  //
> +  // PrimaryHeader->SizeOfPartitionEntry should not be zero
> +  //
> +  if (PrimaryHeader->SizeOfPartitionEntry == 0) {
> +    DEBUG ((DEBUG_ERROR, "SizeOfPartitionEntry should not be zero!\n"));
> +    FreePool (PrimaryHeader);
> +    return EFI_BAD_BUFFER_SIZE;
> +  }
> +
>     //
>     // Read the partition entry.
>     //
> @@ -202,15 +293,14 @@ Tcg2MeasureGptTable (
>     }
>   
>     //
> -  // Prepare Data for Measurement
> +  // Prepare Data for Measurement (CcProtocol and Tcg2Protocol)
>     //
>     EventSize = (UINT32)(sizeof (EFI_GPT_DATA) - sizeof (GptData->Partitions)
>                           + NumberOfPartition * PrimaryHeader->SizeOfPartitionEntry);
>     Tcg2Event = (EFI_TCG2_EVENT *) AllocateZeroPool (EventSize + sizeof (EFI_TCG2_EVENT) - sizeof(Tcg2Event->Event));
>     if (Tcg2Event == NULL) {
> -    FreePool (PrimaryHeader);
> -    FreePool (EntryPtr);
> -    return EFI_OUT_OF_RESOURCES;
> +    Status = EFI_OUT_OF_RESOURCES;
> +    goto Exit;
>     }
>   
>     Tcg2Event->Size = EventSize + sizeof (EFI_TCG2_EVENT) - sizeof(Tcg2Event->Event);
> @@ -243,22 +333,57 @@ Tcg2MeasureGptTable (
>     }
>   
>     //
> -  // Measure the GPT data
> +  // Measure the GPT data by Tcg2Protocol
>     //
> -  Status = Tcg2Protocol->HashLogExtendEvent (
> -             Tcg2Protocol,
> -             0,
> -             (EFI_PHYSICAL_ADDRESS) (UINTN) (VOID *) GptData,
> -             (UINT64) EventSize,
> -             Tcg2Event
> -             );
> -  if (!EFI_ERROR (Status)) {
> -    mTcg2MeasureGptCount++;
> -  }
> -
> -  FreePool (PrimaryHeader);
> -  FreePool (EntryPtr);
> -  FreePool (Tcg2Event);
> +  if (Tcg2Protocol != NULL) {
> +    Status = Tcg2Protocol->HashLogExtendEvent (
> +               Tcg2Protocol,
> +               0,
> +               (EFI_PHYSICAL_ADDRESS) (UINTN) (VOID *) GptData,
> +               (UINT64) EventSize,
> +               Tcg2Event
> +               );
> +    if (!EFI_ERROR (Status)) {
> +      mTcg2MeasureGptCount++;
> +    }
> +    DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - Tcg2 MeasureGptTable - %r\n", Status));
> +
> +  } else if (CcProtocol != NULL) {
[SAMI] Please see my comment in patch 3/3 about the behaviour if both 
the TCG2 and CC measurement protocols are installed.
> +
> +    //
> +    // Measure the GPT data by TdProtocol
> +    //
> +    Status = CreateCcEventFromTcg2Event (CcProtocol, Tcg2Event, EventSize, &CcEvent);
> +    if (EFI_ERROR (Status)) {
> +      goto Exit;
> +    }
> +
> +    Status = CcProtocol->HashLogExtendEvent (
> +               CcProtocol,
> +               0,
> +               (EFI_PHYSICAL_ADDRESS) (UINTN) (VOID *) GptData,
> +               (UINT64) EventSize,
> +               CcEvent
> +               );
> +    if (!EFI_ERROR (Status)) {
> +      mTcg2MeasureGptCount++;
> +    }
> +    DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - Cc MeasureGptTable - %r\n", Status));
> +  }
> +
> +Exit:
> +  if (PrimaryHeader != NULL) {
> +    FreePool (PrimaryHeader);
> +  }
> +  if (EntryPtr != NULL) {
> +    FreePool (EntryPtr);
> +  }
> +  if (Tcg2Event != NULL) {
> +    FreePool (Tcg2Event);
> +  }
> +  if (CcEvent != NULL) {
> +    FreePool (CcEvent);
> +  }
>   
>     return Status;
>   }
> @@ -271,12 +396,12 @@ Tcg2MeasureGptTable (
>     PE/COFF image is external input, so this function will validate its data structure
>     within this image buffer before use.
>   
> -  @param[in] Tcg2Protocol   Pointer to the located TCG2 protocol instance.
> -  @param[in] ImageAddress   Start address of image buffer.
> -  @param[in] ImageSize      Image size
> -  @param[in] LinkTimeBase   Address that the image is loaded into memory.
> -  @param[in] ImageType      Image subsystem type.
> -  @param[in] FilePath       File path is corresponding to the input image.
> +  @param[in] MeasureBootProtocols   Pointer to the located MeasureBoot protocol instances.
> +  @param[in] ImageAddress           Start address of image buffer.
> +  @param[in] ImageSize              Image size
> +  @param[in] LinkTimeBase           Address that the image is loaded into memory.
> +  @param[in] ImageType              Image subsystem type.
> +  @param[in] FilePath               File path is corresponding to the input image.
>   
>     @retval EFI_SUCCESS            Successfully measure image.
>     @retval EFI_OUT_OF_RESOURCES   No enough resource to measure image.
> @@ -287,7 +412,7 @@ Tcg2MeasureGptTable (
>   EFI_STATUS
>   EFIAPI
>   Tcg2MeasurePeImage (
> -  IN  EFI_TCG2_PROTOCOL         *Tcg2Protocol,
> +  IN  MEASURE_BOOT_PROTOCOLS    *MeasureBootProtocols,
>     IN  EFI_PHYSICAL_ADDRESS      ImageAddress,
>     IN  UINTN                     ImageSize,
>     IN  UINTN                     LinkTimeBase,
> @@ -300,9 +425,22 @@ Tcg2MeasurePeImage (
>     EFI_IMAGE_LOAD_EVENT              *ImageLoad;
>     UINT32                            FilePathSize;
>     UINT32                            EventSize;
> +  EFI_CC_EVENT                      *CcEvent;
> +  EFI_CC_MEASUREMENT_PROTOCOL       *CcProtocol;
> +  EFI_TCG2_PROTOCOL                 *Tcg2Protocol;
>   
>     Status        = EFI_UNSUPPORTED;
>     ImageLoad     = NULL;
> +  CcEvent       = NULL;
> +
> +  Tcg2Protocol  = MeasureBootProtocols->Tcg2Protocol;
> +  CcProtocol    = MeasureBootProtocols->CcProtocol;
> +
> +  if (Tcg2Protocol == NULL && CcProtocol == NULL) {
> +    ASSERT (FALSE);
> +    return EFI_UNSUPPORTED;
> +  }
> +
>     FilePathSize  = (UINT32) GetDevicePathSize (FilePath);
>   
>     //
> @@ -334,7 +472,7 @@ Tcg2MeasurePeImage (
>         break;
>       default:
>         DEBUG ((
> -        EFI_D_ERROR,
> +        DEBUG_ERROR,
>           "Tcg2MeasurePeImage: Unknown subsystem type %d",
>           ImageType
>           ));
> @@ -352,28 +490,125 @@ Tcg2MeasurePeImage (
>     //
>     // Log the PE data
>     //
> -  Status = Tcg2Protocol->HashLogExtendEvent (
> -             Tcg2Protocol,
> -             PE_COFF_IMAGE,
> -             ImageAddress,
> -             ImageSize,
> -             Tcg2Event
> -             );
> -  if (Status == EFI_VOLUME_FULL) {
> -    //
> -    // Volume full here means the image is hashed and its result is extended to PCR.
> -    // But the event log can't be saved since log area is full.
> -    // Just return EFI_SUCCESS in order not to block the image load.
> -    //
> -    Status = EFI_SUCCESS;
> +  if (Tcg2Protocol != NULL) {
> +    Status = Tcg2Protocol->HashLogExtendEvent (
> +               Tcg2Protocol,
> +               PE_COFF_IMAGE,
> +               ImageAddress,
> +               ImageSize,
> +               Tcg2Event
> +               );
> +    if (Status == EFI_VOLUME_FULL) {
> +      //
> +      // Volume full here means the image is hashed and its result is extended to PCR.
> +      // But the event log can't be saved since log area is full.
> +      // Just return EFI_SUCCESS in order not to block the image load.
> +      //
> +      Status = EFI_SUCCESS;
> +    }
> +    DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - Tcg2 MeasurePeImage - %r\n", Status));
> +
> +  } else if (CcProtocol != NULL) {
> +
> +    Status = CreateCcEventFromTcg2Event (CcProtocol, Tcg2Event, EventSize, &CcEvent);
> +    if (EFI_ERROR (Status)) {
> +      goto Finish;
> +    }
> +
> +    Status = CcProtocol->HashLogExtendEvent (
> +               CcProtocol,
> +               PE_COFF_IMAGE,
> +               ImageAddress,
> +               ImageSize,
> +               CcEvent
> +               );
> +    if (Status == EFI_VOLUME_FULL) {
> +      //
> +      // Volume full here means the image is hashed and its result is extended to PCR.
> +      // But the event log can't be saved since log area is full.
> +      // Just return EFI_SUCCESS in order not to block the image load.
> +      //
> +      Status = EFI_SUCCESS;
> +    }
> +    DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - Cc MeasurePeImage - %r\n", Status));
>     }
>   
>   Finish:
> -  FreePool (Tcg2Event);
> +  if (Tcg2Event != NULL) {
> +    FreePool (Tcg2Event);
> +  }
> +
> +  if (CcEvent != NULL) {
> +    FreePool (CcEvent);
> +  }
>   
>     return Status;
>   }
>   
> +/**
> +  Get the measure boot protocols.
> +
> +  There are 2 measure boot, TCG2 protocol based and Cc measurement protocol based.
> +
> +  @param  MeasureBootProtocols  Pointer to the located measure boot protocol instances.
> +
> +  @retval EFI_SUCCESS           Sucessfully locate the measure boot protocol instances (at least one instance).
> +  @retval EFI_UNSUPPORTED       Measure boot is not supported.
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetMeasureBootProtocols (
> +  MEASURE_BOOT_PROTOCOLS    *MeasureBootProtocols
> +  )
> +{
> +  EFI_STATUS                          Status;
> +  EFI_TCG2_PROTOCOL                   *Tcg2Protocol;
> +  EFI_CC_MEASUREMENT_PROTOCOL         *CcProtocol;
> +  EFI_TCG2_BOOT_SERVICE_CAPABILITY    Tcg2ProtocolCapability;
> +  EFI_CC_BOOT_SERVICE_CAPABILITY      CcProtocolCapability;
> +
> +  CcProtocol = NULL;
> +  Status = gBS->LocateProtocol (&gEfiCcMeasurementProtocolGuid, NULL, (VOID **) &CcProtocol);
> +  if (EFI_ERROR (Status)) {
> +    //
> +    // Cc Measurement protocol is not installed.
> +    //
> +    DEBUG ((DEBUG_VERBOSE, "CcMeasurementProtocol is not installed. - %r\n", Status));
> +  } else {
> +    ZeroMem (&CcProtocolCapability, sizeof (CcProtocolCapability));
> +    CcProtocolCapability.Size = sizeof (CcProtocolCapability);
> +    Status = CcProtocol->GetCapability (CcProtocol, &CcProtocolCapability);
> +    if (EFI_ERROR (Status) || CcProtocolCapability.CcType.Type == EFI_CC_TYPE_NONE) {
> +      DEBUG ((DEBUG_ERROR, " CcProtocol->GetCapability returns : %x, %r\n", CcProtocolCapability.CcType.Type, Status));
> +      CcProtocol = NULL;
> +    }
> +  }
> +
> +  Tcg2Protocol = NULL;
> +  Status = gBS->LocateProtocol (&gEfiTcg2ProtocolGuid, NULL, (VOID **) &Tcg2Protocol);
> +  if (EFI_ERROR (Status)) {
> +    //
> +    // Tcg2 protocol is not installed. So, TPM2 is not present.
> +    //
> +    DEBUG ((DEBUG_VERBOSE, "Tcg2Protocol is not installed. - %r\n", Status));
> +  } else {
> +    Tcg2ProtocolCapability.Size = (UINT8) sizeof (Tcg2ProtocolCapability);
> +    Status = Tcg2Protocol->GetCapability (Tcg2Protocol, &Tcg2ProtocolCapability);
> +    if (EFI_ERROR (Status) || (!Tcg2ProtocolCapability.TPMPresentFlag)) {
> +      //
> +      // TPM device doesn't work or activate.
> +      //
> +      DEBUG ((DEBUG_ERROR, "TPMPresentFlag=FALSE %r\n", Status));
> +      Tcg2Protocol = NULL;
> +    }
> +  }
> +
> +  MeasureBootProtocols->Tcg2Protocol = Tcg2Protocol;
> +  MeasureBootProtocols->CcProtocol   = CcProtocol;
> +
> +  return (Tcg2Protocol == NULL && CcProtocol == NULL) ? EFI_UNSUPPORTED: EFI_SUCCESS;
> +}
> +
>   /**
>     The security handler is used to abstract platform-specific policy
>     from the DXE core response to an attempt to use a file that returns a
> @@ -422,9 +657,8 @@ DxeTpm2MeasureBootHandler (
>     IN  BOOLEAN                          BootPolicy
>     )
>   {
> -  EFI_TCG2_PROTOCOL                   *Tcg2Protocol;
> +  MEASURE_BOOT_PROTOCOLS              MeasureBootProtocols;
>     EFI_STATUS                          Status;
> -  EFI_TCG2_BOOT_SERVICE_CAPABILITY    ProtocolCapability;
>     EFI_DEVICE_PATH_PROTOCOL            *DevicePathNode;
>     EFI_DEVICE_PATH_PROTOCOL            *OrigDevicePathNode;
>     EFI_HANDLE                          Handle;
> @@ -435,28 +669,23 @@ DxeTpm2MeasureBootHandler (
>     EFI_PHYSICAL_ADDRESS                FvAddress;
>     UINT32                              Index;
>   
> -  Status = gBS->LocateProtocol (&gEfiTcg2ProtocolGuid, NULL, (VOID **) &Tcg2Protocol);
> +  MeasureBootProtocols.Tcg2Protocol = NULL;
> +  MeasureBootProtocols.CcProtocol   = NULL;
> +
> +  Status = GetMeasureBootProtocols(&MeasureBootProtocols);
> +
>     if (EFI_ERROR (Status)) {
>       //
> -    // Tcg2 protocol is not installed. So, TPM2 is not present.
> +    // None of Measured boot protocols (Tcg2, Cc) is installed.
>       // Don't do any measurement, and directly return EFI_SUCCESS.
>       //
> -    DEBUG ((EFI_D_VERBOSE, "DxeTpm2MeasureBootHandler - Tcg2 - %r\n", Status));
> +    DEBUG ((DEBUG_INFO, "None of Tcg2Protocol/CcMeasurementProtocol is installed.\n"));
>       return EFI_SUCCESS;
>     }
>   
> -  ProtocolCapability.Size = (UINT8) sizeof (ProtocolCapability);
> -  Status = Tcg2Protocol->GetCapability (
> -                           Tcg2Protocol,
> -                           &ProtocolCapability
> -                           );
> -  if (EFI_ERROR (Status) || (!ProtocolCapability.TPMPresentFlag)) {
> -    //
> -    // TPM device doesn't work or activate.
> -    //
> -    DEBUG ((EFI_D_ERROR, "DxeTpm2MeasureBootHandler (%r) - TPMPresentFlag - %x\n", Status, ProtocolCapability.TPMPresentFlag));
> -    return EFI_SUCCESS;
> -  }
> +  DEBUG ((DEBUG_INFO, "Tcg2Protocol = %p, TdProtocol = %p\n",
> +                      MeasureBootProtocols.Tcg2Protocol,
> +                      MeasureBootProtocols.CcProtocol));
>   
>     //
>     // Copy File Device Path
> @@ -502,8 +731,8 @@ DxeTpm2MeasureBootHandler (
>               //
>               // Measure GPT disk.
>               //
> -            Status = Tcg2MeasureGptTable (Tcg2Protocol, Handle);
> -            DEBUG ((EFI_D_INFO, "DxeTpm2MeasureBootHandler - Tcg2MeasureGptTable - %r\n", Status));
> +            Status = Tcg2MeasureGptTable (&MeasureBootProtocols, Handle);
> +
>               if (!EFI_ERROR (Status)) {
>                 //
>                 // GPT disk check done.
> @@ -647,14 +876,13 @@ DxeTpm2MeasureBootHandler (
>       // Measure PE image into TPM log.
>       //
>       Status = Tcg2MeasurePeImage (
> -               Tcg2Protocol,
> +               &MeasureBootProtocols,
>                  (EFI_PHYSICAL_ADDRESS) (UINTN) FileBuffer,
>                  FileSize,
>                  (UINTN) ImageContext.ImageAddress,
>                  ImageContext.ImageType,
>                  DevicePathNode
>                  );
> -    DEBUG ((EFI_D_INFO, "DxeTpm2MeasureBootHandler - Tcg2MeasurePeImage - %r\n", Status));
>     }
>   
>     //
> @@ -665,7 +893,7 @@ Finish:
>       FreePool (OrigDevicePathNode);
>     }
>   
> -  DEBUG ((EFI_D_INFO, "DxeTpm2MeasureBootHandler - %r\n", Status));
> +  DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - %r\n", Status));
>   
>     return Status;
>   }
> diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
> index 2506abbe7c8b..6dca79a20c93 100644
> --- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
> +++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
> @@ -1,5 +1,5 @@
>   ## @file
> -#  Provides security service for TPM 2.0 measured boot
> +#  Provides security service for TPM 2.0 measured boot and Confidential Computing measure boot.
>   #
>   #  Spec Compliance Info:
>   #    "TCG PC Client Platform Firmware Profile Specification for TPM Family 2.0 Level 00 Revision 1.03 v51"
> @@ -61,6 +61,7 @@
>   
>   [Protocols]
>     gEfiTcg2ProtocolGuid                  ## SOMETIMES_CONSUMES
> +  gEfiCcMeasurementProtocolGuid         ## SOMETIMES_CONSUMES
>     gEfiFirmwareVolumeBlockProtocolGuid   ## SOMETIMES_CONSUMES
>     gEfiBlockIoProtocolGuid               ## SOMETIMES_CONSUMES
>     gEfiDiskIoProtocolGuid                ## SOMETIMES_CONSUMES


[-- Attachment #2: Type: text/html, Size: 24049 bytes --]

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

* Re: [PATCH V4 3/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpmMeasurementLib
  2021-11-02  2:50 ` [PATCH V4 3/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpmMeasurementLib Min Xu
  2021-11-02  6:24   ` Yao, Jiewen
@ 2021-11-02  9:45   ` Sami Mujawar
  2021-11-04  8:20     ` Gerd Hoffmann
  2021-11-05  2:15     ` Min Xu
  1 sibling, 2 replies; 23+ messages in thread
From: Sami Mujawar @ 2021-11-02  9:45 UTC (permalink / raw)
  To: Min Xu, devel
  Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Jiewen Yao,
	Jian J Wang, Gerd Hoffmann, nd

[-- Attachment #1: Type: text/plain, Size: 7945 bytes --]

Hi Min,

Thank you for this patch.

Please find my feedback inline marked [SAMI].

Regards,

Sami Mujawar


On 02/11/2021 02:50 AM, Min Xu wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3625
>
> DxeTpmMeasurementLib supports TPM based measurement in DXE phase.
> After CcMeasurementProtocol is introduced, CC based measurement needs
> to be supported in DxeTpmMeasurementLib as well.
>
> In TpmMeasureAndLogData, CC based measurement will be first called.
> If it failed, TPM based measurement will be called sequentially.
> Currently there is an assumption that CC based measurement and
> TPM based measurement won't be exist at the same time.If the
> assumption is not true in the future, we will revisit here then.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
>   .../DxeTpmMeasurementLib.c                    | 91 ++++++++++++++++++-
>   .../DxeTpmMeasurementLib.inf                  |  9 +-
>   2 files changed, 92 insertions(+), 8 deletions(-)
>
> diff --git a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
> index 061136ee7860..2ddb9033a0d5 100644
> --- a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
> +++ b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
> @@ -1,5 +1,6 @@
>   /** @file
> -  This library is used by other modules to measure data to TPM.
> +  This library is used by other modules to measure data to TPM and Confidential
> +  Computing (CC) measure registers.
>   
>   Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved. <BR>
>   SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -19,8 +20,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>   
>   #include <Guid/Acpi.h>
>   #include <IndustryStandard/Acpi.h>
> -
> -
> +#include <Protocol/CcMeasurement.h>
>   
>   /**
>     Tpm12 measure and log data, and extend the measurement result into a specific PCR.
> @@ -149,6 +149,73 @@ Tpm20MeasureAndLogData (
>     return Status;
>   }
>   
> +/**
> +  Cc measure and log data, and extend the measurement result into a
> +  specific CC MR.
> +
> +  @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
> +
> +  @retval EFI_SUCCESS           Operation completed successfully.
> +  @retval EFI_UNSUPPORTED       Tdx device not available.
> +  @retval EFI_OUT_OF_RESOURCES  Out of memory.
> +  @retval EFI_DEVICE_ERROR      The operation was unsuccessful.
> +**/
> +EFI_STATUS
> +EFIAPI
> +CcMeasureAndLogData (
> +  IN UINT32             PcrIndex,
> +  IN UINT32             EventType,
> +  IN VOID               *EventLog,
> +  IN UINT32             LogLen,
> +  IN VOID               *HashData,
> +  IN UINT64             HashDataLen
> +  )
> +{
> +  EFI_STATUS                    Status;
> +  EFI_CC_MEASUREMENT_PROTOCOL  *CcProtocol;
> +  EFI_CC_EVENT                 *EfiCcEvent;
> +  UINT32                        MrIndex;
[SAMI] Same comment as in patch 2/3. Is it possible to use the typedef 
for the measurment register index here, please?
> +
> +  Status = gBS->LocateProtocol (&gEfiCcMeasurementProtocolGuid, NULL, (VOID **) &CcProtocol);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = CcProtocol->MapPcrToMrIndex (CcProtocol, PcrIndex, &MrIndex);
> +  if (EFI_ERROR (Status)) {
> +    return EFI_INVALID_PARAMETER;
[SAMI] Is it possible to return the error code returned by 
CcProtocol->MapPcrToMrIndex(), please?
> +  }
> +
> +  EfiCcEvent = (EFI_CC_EVENT *) AllocateZeroPool (LogLen + sizeof (EFI_CC_EVENT));
> +  if(EfiCcEvent == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  EfiCcEvent->Size = (UINT32) LogLen + sizeof (EFI_CC_EVENT) - sizeof (EfiCcEvent->Event);
> +  EfiCcEvent->Header.HeaderSize    = sizeof (EFI_CC_EVENT_HEADER);
> +  EfiCcEvent->Header.HeaderVersion = EFI_CC_EVENT_HEADER_VERSION;
> +  EfiCcEvent->Header.MrIndex       = MrIndex;
> +  EfiCcEvent->Header.EventType     = EventType;
> +  CopyMem (&EfiCcEvent->Event[0], EventLog, LogLen);
> +
> +  Status = CcProtocol->HashLogExtendEvent (
> +                           CcProtocol,
> +                           0,
> +                           (EFI_PHYSICAL_ADDRESS) (UINTN) HashData,
> +                           HashDataLen,
> +                           EfiCcEvent
> +                           );
> +  FreePool (EfiCcEvent);
> +
> +  return Status;
> +}
> +
> +
>   /**
>     Tpm measure and log data, and extend the measurement result into a specific PCR.
>   
> @@ -178,9 +245,9 @@ TpmMeasureAndLogData (
>     EFI_STATUS  Status;
>   
>     //
> -  // Try to measure using Tpm20 protocol
> +  // Try to measure using Cc measurement protocol
>     //
> -  Status = Tpm20MeasureAndLogData(
> +  Status = CcMeasureAndLogData (
>                PcrIndex,
>                EventType,
>                EventLog,
> @@ -189,6 +256,20 @@ TpmMeasureAndLogData (
>                HashDataLen
>                );
>   
> +  if (EFI_ERROR (Status)) {
> +    //
> +    // Try to measure using Tpm20 protocol
> +    //
> +    Status = Tpm20MeasureAndLogData(
> +               PcrIndex,
> +               EventType,
> +               EventLog,
> +               LogLen,
> +               HashData,
> +               HashDataLen
> +               );
> +  }
[SAMI] Apologies, I missed this in my previous review. I think the 
behaviour if both the TCG2 and CC measurement protocols are installed
would be inconsistent between DxeTpmMeasurementLib and 
DxeTpm2MeasureBootLib. The main difference being in the later, the
TCG2 protocol takes precedence for extending the measurement.
I think it would be good to modify DxeTpm2MeasureBootLib so that the CC 
measurement protocol is used if both protocols are installed. What do 
you think?

There is another subtle difference in this patch. Consider the case 
where LocateProtocol(gEfiCcMeasurementProtocolGuid, ... ) succeeds but 
subsequently
MapPcrToMrIndex() or HashLogExtendEvent() fail then 
Tpm20MeasureAndLogData() will be called. This is not the case with 
DxeTpm2MeasureBootLib.
I think it would be good to have a consistent behaviour.
  [/SAMI]
> +
>     if (EFI_ERROR (Status)) {
>       //
>       // Try to measure using Tpm1.2 protocol
> diff --git a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> index 7d41bc41f95d..3af3d4e33b25 100644
> --- a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> +++ b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> @@ -1,5 +1,7 @@
>   ## @file
> -#  Provides TPM measurement functions for TPM1.2 and TPM 2.0
> +#  Provides below measurement functions:
> +#    1. TPM measurement functions for TPM1.2 and TPM 2.0
> +#    2. Confidential Computing (CC) measurement functions
>   #
>   #  This library provides TpmMeasureAndLogData() to measure and log data, and
>   #  extend the measurement result into a specific PCR.
> @@ -40,5 +42,6 @@
>     UefiBootServicesTableLib
>   
>   [Protocols]
> -  gEfiTcgProtocolGuid           ## SOMETIMES_CONSUMES
> -  gEfiTcg2ProtocolGuid          ## SOMETIMES_CONSUMES
> +  gEfiTcgProtocolGuid               ## SOMETIMES_CONSUMES
> +  gEfiTcg2ProtocolGuid              ## SOMETIMES_CONSUMES
> +  gEfiCcMeasurementProtocolGuid     ## SOMETIMES_CONSUMES


[-- Attachment #2: Type: text/html, Size: 9462 bytes --]

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

* Re: [PATCH V4 2/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpm2MeasureBootLib
  2021-11-02  6:24   ` Yao, Jiewen
@ 2021-11-03  2:59     ` Min Xu
  0 siblings, 0 replies; 23+ messages in thread
From: Min Xu @ 2021-11-03  2:59 UTC (permalink / raw)
  To: Yao, Jiewen, devel@edk2.groups.io
  Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang, Wang, Jian J,
	Sami Mujawar, Gerd Hoffmann

On November 2, 2021 2:25 PM, Jiewen Yao wrote:
> May I know which platform you have run the test?
> 
> I think we need cover both TD and TPM in real platform.
> 
I have run the test in Intel's internal hardware platform (covering both TD and TPM).
The test all pass.

Thanks
Min

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

* Re: [PATCH V4 3/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpmMeasurementLib
  2021-11-02  6:24   ` Yao, Jiewen
@ 2021-11-03  3:01     ` Min Xu
  0 siblings, 0 replies; 23+ messages in thread
From: Min Xu @ 2021-11-03  3:01 UTC (permalink / raw)
  To: Yao, Jiewen, devel@edk2.groups.io
  Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang, Wang, Jian J,
	Sami Mujawar, Gerd Hoffmann

On November 2, 2021 2:25 PM, Jiewen Yao wrote:
> May I know which platform you have run the test?
> 
> I think we need cover both TD and TPM in real platform.
> 
I have run the test in Intel's internal hardware platform (covering both TD and TPM).
The test all pass.

Thanks
Min


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

* 回复: [PATCH V4 1/3] MdePkg: Introduce CcMeasurementProtocol for CC Guest firmware
  2021-11-02  2:50 ` [PATCH V4 1/3] MdePkg: Introduce CcMeasurementProtocol for CC Guest firmware Min Xu
  2021-11-02  6:24   ` Yao, Jiewen
  2021-11-02  9:41   ` Sami Mujawar
@ 2021-11-04  5:51   ` gaoliming
  2021-11-04 12:35     ` [edk2-devel] " Min Xu
  2 siblings, 1 reply; 23+ messages in thread
From: gaoliming @ 2021-11-04  5:51 UTC (permalink / raw)
  To: 'Min Xu', devel
  Cc: 'Michael D Kinney', 'Zhiguang Liu',
	'Jiewen Yao', 'Jian J Wang', 'Ken Lu',
	'Sami Mujawar', 'Gerd Hoffmann'

Min:
  I have one minor comment. gEfiCcFinalEventsTableGuid may be placed into
[Guids] section instead of [Protocols] section.

Thanks
Liming
> -----邮件原件-----
> 发件人: Min Xu <min.m.xu@intel.com>
> 发送时间: 2021年11月2日 10:51
> 收件人: devel@edk2.groups.io
> 抄送: Min Xu <min.m.xu@intel.com>; Michael D Kinney
> <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
> Zhiguang Liu <zhiguang.liu@intel.com>; Jiewen Yao <jiewen.yao@intel.com>;
> Jian J Wang <jian.j.wang@intel.com>; Ken Lu <ken.lu@intel.com>; Sami
> Mujawar <sami.mujawar@arm.com>; Gerd Hoffmann <kraxel@redhat.com>
> 主题: [PATCH V4 1/3] MdePkg: Introduce CcMeasurementProtocol for CC
> Guest firmware
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3625
> 
> CC guest is a Confidential Computing guest. If CC Guest firmware
> supports measurement and an event is created, CC Guest firmware
> is designed to report the event log with the same data structure
> in TCG-Platform-Firmware-Profile specification with
> EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 format.
> 
> The CC Guest firmware supports measurement. It is designed to
> produce EFI_CC_MEASUREMENT_PROTOCOL with new GUID
> EFI_CC_MEASUREMENT_PROTOCOL_GUID to report event log and provides
> hash capability.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Ken Lu <ken.lu@intel.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
>  MdePkg/Include/Protocol/CcMeasurement.h | 305
> ++++++++++++++++++++++++
>  MdePkg/MdePkg.dec                       |   4 +
>  2 files changed, 309 insertions(+)
>  create mode 100644 MdePkg/Include/Protocol/CcMeasurement.h
> 
> diff --git a/MdePkg/Include/Protocol/CcMeasurement.h
> b/MdePkg/Include/Protocol/CcMeasurement.h
> new file mode 100644
> index 000000000000..eaedbfffdb6a
> --- /dev/null
> +++ b/MdePkg/Include/Protocol/CcMeasurement.h
> @@ -0,0 +1,305 @@
> +/** @file
> +  If CC Guest firmware supports measurement and an event is created,
> +  CC Guest firmware is designed to report the event log with the same
> +  data structure in TCG-Platform-Firmware-Profile specification with
> +  EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 format.
> +
> +  The CC Guest firmware supports measurement, the CC Guest Firmware is
> +  designed to produce EFI_CC_MEASUREMENT_PROTOCOL with new GUID
> +  EFI_CC_MEASUREMENT_PROTOCOL_GUID to report event log and
> provides hash
> +  capability.
> +
> +Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef CC_MEASUREMENT_PROTOCOL_H_
> +#define CC_MEASUREMENT_PROTOCOL_H_
> +
> +#include <IndustryStandard/UefiTcgPlatform.h>
> +
> +#define EFI_CC_MEASUREMENT_PROTOCOL_GUID  \
> +  { 0x96751a3d, 0x72f4, 0x41a6, { 0xa7, 0x94, 0xed, 0x5d, 0x0e, 0x67,
0xae,
> 0x6b }}
> +extern EFI_GUID gEfiCcMeasurementProtocolGuid;
> +
> +typedef struct _EFI_CC_MEASUREMENT_PROTOCOL
> EFI_CC_MEASUREMENT_PROTOCOL;
> +
> +typedef struct {
> +  UINT8 Major;
> +  UINT8 Minor;
> +} EFI_CC_VERSION;
> +
> +//
> +// EFI_CC Type/SubType definition
> +//
> +#define EFI_CC_TYPE_NONE   0
> +#define EFI_CC_TYPE_SEV    1
> +#define EFI_CC_TYPE_TDX    2
> +
> +typedef struct {
> +  UINT8          Type;
> +  UINT8          SubType;
> +} EFI_CC_TYPE;
> +
> +typedef UINT32                      EFI_CC_EVENT_LOG_BITMAP;
> +typedef UINT32                      EFI_CC_EVENT_LOG_FORMAT;
> +typedef UINT32
> EFI_CC_EVENT_ALGORITHM_BITMAP;
> +typedef UINT32                      EFI_CC_MR_INDEX;
> +
> +//
> +// Intel TDX measure register index
> +//
> +#define TDX_MR_INDEX_MRTD  0
> +#define TDX_MR_INDEX_RTMR0 1
> +#define TDX_MR_INDEX_RTMR1 2
> +#define TDX_MR_INDEX_RTMR2 3
> +#define TDX_MR_INDEX_RTMR3 4
> +
> +
> +#define EFI_CC_EVENT_LOG_FORMAT_TCG_2   0x00000002
> +#define EFI_CC_BOOT_HASH_ALG_SHA384     0x00000004
> +
> +//
> +// This bit is shall be set when an event shall be extended but not
logged.
> +//
> +#define EFI_CC_FLAG_EXTEND_ONLY       0x0000000000000001
> +//
> +// This bit shall be set when the intent is to measure a PE/COFF image.
> +//
> +#define EFI_CC_FLAG_PE_COFF_IMAGE     0x0000000000000010
> +
> +#pragma pack (1)
> +
> +#define EFI_CC_EVENT_HEADER_VERSION   1
> +
> +typedef struct {
> +  //
> +  // Size of the event header itself (sizeof(EFI_CC_EVENT_HEADER)).
> +  //
> +  UINT32            HeaderSize;
> +  //
> +  // Header version. For this version of this specification, the value
shall be
> 1.
> +  //
> +  UINT16            HeaderVersion;
> +  //
> +  // Index of the MR (measurement register) that shall be extended.
> +  //
> +  EFI_CC_MR_INDEX   MrIndex;
> +  //
> +  // Type of the event that shall be extended (and optionally logged).
> +  //
> +  UINT32            EventType;
> +} EFI_CC_EVENT_HEADER;
> +
> +typedef struct {
> +  //
> +  // Total size of the event including the Size component, the header and
the
> Event data.
> +  //
> +  UINT32                  Size;
> +  EFI_CC_EVENT_HEADER     Header;
> +  UINT8                   Event[1];
> +} EFI_CC_EVENT;
> +
> +#pragma pack()
> +
> +
> +typedef struct {
> +  //
> +  // Allocated size of the structure
> +  //
> +  UINT8                            Size;
> +  //
> +  // Version of the EFI_CC_BOOT_SERVICE_CAPABILITY structure itself.
> +  // For this version of the protocol, the Major version shall be set to
1
> +  // and the Minor version shall be set to 0.
> +  //
> +  EFI_CC_VERSION                   StructureVersion;
> +  //
> +  // Version of the EFI CC Measurement protocol.
> +  // For this version of the protocol, the Major version shall be set to
1
> +  // and the Minor version shall be set to 0.
> +  //
> +  EFI_CC_VERSION                   ProtocolVersion;
> +  //
> +  // Supported hash algorithms
> +  //
> +  EFI_CC_EVENT_ALGORITHM_BITMAP   HashAlgorithmBitmap;
> +  //
> +  // Bitmap of supported event log formats
> +  //
> +  EFI_CC_EVENT_LOG_BITMAP          SupportedEventLogs;
> +
> +  //
> +  // Indicates the CC type
> +  //
> +  EFI_CC_TYPE                      CcType;
> +} EFI_CC_BOOT_SERVICE_CAPABILITY;
> +
> +/**
> +  The EFI_CC_MEASUREMENT_PROTOCOL GetCapability function call
> provides protocol
> +  capability information and state information.
> +
> +  @param[in]      This               Indicates the calling context
> +  @param[in, out] ProtocolCapability The caller allocates memory for a
> EFI_CC_BOOT_SERVICE_CAPABILITY
> +                                     structure and sets the size
> field to the size of the structure allocated.
> +                                     The callee fills in the fields
> with the EFI CC BOOT Service capability
> +                                     information and the current
> CC information.
> +
> +  @retval EFI_SUCCESS            Operation completed successfully.
> +  @retval EFI_DEVICE_ERROR       The command was unsuccessful.
> +                                 The ProtocolCapability variable will
> not be populated.
> +  @retval EFI_INVALID_PARAMETER  One or more of the parameters are
> incorrect.
> +                                 The ProtocolCapability variable will
> not be populated.
> +  @retval EFI_BUFFER_TOO_SMALL   The ProtocolCapability variable is
> too small to hold the full response.
> +                                 It will be partially populated
> (required Size field will be set).
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EFI_CC_GET_CAPABILITY) (
> +  IN     EFI_CC_MEASUREMENT_PROTOCOL       *This,
> +  IN OUT EFI_CC_BOOT_SERVICE_CAPABILITY    *ProtocolCapability
> +  );
> +
> +/**
> +  The EFI_CC_MEASUREMENT_PROTOCOL Get Event Log function call
> allows a caller to
> +  retrieve the address of a given event log and its last entry.
> +
> +  @param[in]  This               Indicates the calling context
> +  @param[in]  EventLogFormat     The type of the event log for which
> the information is requested.
> +  @param[out] EventLogLocation   A pointer to the memory address of
> the event log.
> +  @param[out] EventLogLastEntry  If the Event Log contains more than
> one entry, this is a pointer to the
> +                                 address of the start of the last
> entry in the event log in memory.
> +  @param[out] EventLogTruncated  If the Event Log is missing at least one
> entry because an event would
> +                                 have exceeded the area allocated
> for events, this value is set to TRUE.
> +                                 Otherwise, the value will be FALSE
> and the Event Log will be complete.
> +
> +  @retval EFI_SUCCESS            Operation completed successfully.
> +  @retval EFI_INVALID_PARAMETER  One or more of the parameters are
> incorrect
> +                                 (e.g. asking for an event log whose
> format is not supported).
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EFI_CC_GET_EVENT_LOG) (
> +  IN  EFI_CC_MEASUREMENT_PROTOCOL     *This,
> +  IN  EFI_CC_EVENT_LOG_FORMAT         EventLogFormat,
> +  OUT EFI_PHYSICAL_ADDRESS            *EventLogLocation,
> +  OUT EFI_PHYSICAL_ADDRESS            *EventLogLastEntry,
> +  OUT BOOLEAN                         *EventLogTruncated
> +  );
> +
> +/**
> +  The EFI_CC_MEASUREMENT_PROTOCOL HashLogExtendEvent function
> call provides
> +  callers with an opportunity to extend and optionally log events without
> requiring
> +  knowledge of actual CC commands.
> +  The extend operation will occur even if this function cannot create an
> event
> +  log entry (e.g. due to the event log being full).
> +
> +  @param[in]  This               Indicates the calling context
> +  @param[in]  Flags              Bitmap providing additional
> information.
> +  @param[in]  DataToHash         Physical address of the start of the
> data buffer to be hashed.
> +  @param[in]  DataToHashLen      The length in bytes of the buffer
> referenced by DataToHash.
> +  @param[in]  EfiCcEvent        Pointer to data buffer containing
> information about the event.
> +
> +  @retval EFI_SUCCESS            Operation completed successfully.
> +  @retval EFI_DEVICE_ERROR       The command was unsuccessful.
> +  @retval EFI_VOLUME_FULL        The extend operation occurred, but
> the event could not be written to one or more event logs.
> +  @retval EFI_INVALID_PARAMETER  One or more of the parameters are
> incorrect.
> +  @retval EFI_UNSUPPORTED        The PE/COFF image type is not
> supported.
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI * EFI_CC_HASH_LOG_EXTEND_EVENT) (
> +  IN EFI_CC_MEASUREMENT_PROTOCOL    *This,
> +  IN UINT64                         Flags,
> +  IN EFI_PHYSICAL_ADDRESS           DataToHash,
> +  IN UINT64                         DataToHashLen,
> +  IN EFI_CC_EVENT                   *EfiCcEvent
> +  );
> +
> +/**
> +  The EFI_CC_MEASUREMENT_PROTOCOL MapPcrToMrIndex function call
> provides callers
> +  the info on TPM PCR <-> CC MR mapping information.
> +
> +  @param[in]  This               Indicates the calling context
> +  @param[in]  PcrIndex           TPM PCR index.
> +  @param[out] MrIndex            CC MR index.
> +
> +  @retval EFI_SUCCESS            The MrIndex is returned.
> +  @retval EFI_INVALID_PARAMETER  The MrIndex is NULL.
> +  @retval EFI_UNSUPPORTED        The PcrIndex is invalid.
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI * EFI_CC_MAP_PCR_TO_MR_INDEX) (
> +  IN  EFI_CC_MEASUREMENT_PROTOCOL    *This,
> +  IN  TCG_PCRINDEX                   PcrIndex,
> +  OUT EFI_CC_MR_INDEX                *MrIndex
> +  );
> +
> +struct _EFI_CC_MEASUREMENT_PROTOCOL {
> +  EFI_CC_GET_CAPABILITY                     GetCapability;
> +  EFI_CC_GET_EVENT_LOG                      GetEventLog;
> +  EFI_CC_HASH_LOG_EXTEND_EVENT
> HashLogExtendEvent;
> +  EFI_CC_MAP_PCR_TO_MR_INDEX                MapPcrToMrIndex;
> +};
> +
> +//
> +// CC event log
> +//
> +
> +#pragma pack(1)
> +
> +//
> +// Crypto Agile Log Entry Format.
> +// It is similar with TCG_PCR_EVENT2 except the field of MrIndex and
> PCRIndex.
> +//
> +typedef struct {
> +  EFI_CC_MR_INDEX     MrIndex;
> +  UINT32              EventType;
> +  TPML_DIGEST_VALUES  Digests;
> +  UINT32              EventSize;
> +  UINT8               Event[1];
> +} CC_EVENT;
> +
> +//
> +// EFI CC Event Header
> +// It is similar with TCG_PCR_EVENT2_HDR except the field of MrIndex and
> PCRIndex
> +//
> +typedef struct {
> +  EFI_CC_MR_INDEX     MrIndex;
> +  UINT32              EventType;
> +  TPML_DIGEST_VALUES  Digests;
> +  UINT32              EventSize;
> +} CC_EVENT_HDR;
> +
> +#pragma pack()
> +
> +//
> +// Log entries after Get Event Log service
> +//
> +
> +#define EFI_CC_FINAL_EVENTS_TABLE_VERSION  1
> +
> +typedef struct {
> +  //
> +  // The version of this structure. It shall be set to 1.
> +  //
> +  UINT64                  Version;
> +  //
> +  // Number of events recorded after invocation of GetEventLog API
> +  //
> +  UINT64                  NumberOfEvents;
> +  //
> +  // List of events of type CC_EVENT.
> +  //
> +  //CC_EVENT              Event[1];
> +} EFI_CC_FINAL_EVENTS_TABLE;
> +
> +
> +#define EFI_CC_FINAL_EVENTS_TABLE_GUID \
> +  {0xdd4a4648, 0x2de7, 0x4665, {0x96, 0x4d, 0x21, 0xd9, 0xef, 0x5f, 0xb4,
> 0x46}}
> +
> +extern EFI_GUID gEfiCcFinalEventsTableGuid;
> +
> +#endif
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> index 8b18415b107a..8e19be662ada 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -1011,6 +1011,10 @@
>    ## Include/Protocol/PcdInfo.h
>    gGetPcdInfoProtocolGuid        = { 0x5be40f57, 0xfa68, 0x4610, { 0xbb,
> 0xbf, 0xe9, 0xc5, 0xfc, 0xda, 0xd3, 0x65 } }
> 
> +  ## Include/Protocol/CcMeasurement.h
> +  gEfiCcMeasurementProtocolGuid  = { 0x96751a3d, 0x72f4, 0x41a6,
> { 0xa7, 0x94, 0xed, 0x5d, 0x0e, 0x67, 0xae, 0x6b }}
> +  gEfiCcFinalEventsTableGuid     = { 0xdd4a4648, 0x2de7, 0x4665, { 0x96,
> 0x4d, 0x21, 0xd9, 0xef, 0x5f, 0xb4, 0x46 }}
> +
>    #
>    # Protocols defined in PI1.0.
>    #
> --
> 2.29.2.windows.2




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

* Re: [PATCH V4 3/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpmMeasurementLib
  2021-11-02  9:45   ` Sami Mujawar
@ 2021-11-04  8:20     ` Gerd Hoffmann
  2021-11-04 13:35       ` [edk2-devel] " Min Xu
  2021-11-05  2:15     ` Min Xu
  1 sibling, 1 reply; 23+ messages in thread
From: Gerd Hoffmann @ 2021-11-04  8:20 UTC (permalink / raw)
  To: Sami Mujawar
  Cc: Min Xu, devel, Michael D Kinney, Liming Gao, Zhiguang Liu,
	Jiewen Yao, Jian J Wang, nd

  Hi,

> [SAMI] Apologies, I missed this in my previous review. I think the behaviour
> if both the TCG2 and CC measurement protocols are installed
> would be inconsistent between DxeTpmMeasurementLib and
> DxeTpm2MeasureBootLib. The main difference being in the later, the
> TCG2 protocol takes precedence for extending the measurement.

Yes, we should have consistent behavior in both cases.

> I think it would be good to modify DxeTpm2MeasureBootLib so that the CC
> measurement protocol is used if both protocols are installed. What do you
> think?

Does it makes sense to use both protocols?

take care,
  Gerd


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

* Re: [edk2-devel] 回复: [PATCH V4 1/3] MdePkg: Introduce CcMeasurementProtocol for CC Guest firmware
  2021-11-04  5:51   ` 回复: " gaoliming
@ 2021-11-04 12:35     ` Min Xu
  2021-11-05  5:20       ` 回复: " gaoliming
  0 siblings, 1 reply; 23+ messages in thread
From: Min Xu @ 2021-11-04 12:35 UTC (permalink / raw)
  To: devel@edk2.groups.io, gaoliming@byosoft.com.cn
  Cc: Kinney, Michael D, Liu, Zhiguang, Yao, Jiewen, Wang, Jian J,
	Lu, Ken, 'Sami Mujawar', 'Gerd Hoffmann'

On November 4, 2021 1:51 PM, Gao, Liming wrote:
> Min:
>   I have one minor comment. gEfiCcFinalEventsTableGuid may be placed into
> [Guids] section instead of [Protocols] section.
> 
Hi, Liming
I follow the definition of gEfiTcg2ProtocolGuid and gEfiTcg2FinalEventsTableGuid. See https://github.com/tianocore/edk2/blob/master/MdePkg/MdePkg.dec#L1590-L1592
Actually gEfiCcMeasurementProtocolGuid and gEfiCcFinalEventsTableGuid are the counterpart protocol/guid definition in Confidential Computing measure boot.
I am not sure if there is some other consideration that gEfiTcg2ProtocolGuid and gEfiTcg2FinalEventsTableGuid are defined in the section of [Protocols].

Thanks
Min

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

* Re: [edk2-devel] [PATCH V4 3/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpmMeasurementLib
  2021-11-04  8:20     ` Gerd Hoffmann
@ 2021-11-04 13:35       ` Min Xu
  2021-11-04 13:49         ` Min Xu
  0 siblings, 1 reply; 23+ messages in thread
From: Min Xu @ 2021-11-04 13:35 UTC (permalink / raw)
  To: devel@edk2.groups.io, kraxel@redhat.com, Sami Mujawar
  Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang, Yao, Jiewen,
	Wang, Jian J, nd

On November 4, 2021 4:21 PM, Gerd Hoffmann wrote:
>   Hi,
> 
> > [SAMI] Apologies, I missed this in my previous review. I think the
> > behaviour if both the TCG2 and CC measurement protocols are installed
> > would be inconsistent between DxeTpmMeasurementLib and
> > DxeTpm2MeasureBootLib. The main difference being in the later, the
> > TCG2 protocol takes precedence for extending the measurement.
> 
> Yes, we should have consistent behavior in both cases.
In DxeTpmMeasurementLib, Cc measurement protocol is used as the first try. If it fails, then it try to measure with TCG2 / TCG protocol in turn.
In DxeTpm2MeasureBootLib, TCG2 protocol is used the as the first try. If it fails, CC measurement protocol is tried in turn.
Yes, this is inconsistent. I will update DxeTpm2MeasureBootLib to try Cc measurement protocol first, then try TCG2 protocol if Cc measurement protocol fails. In this way, only one protocol will be called to do the measurement. But TCG2 protocol is the first try, CC measurement protocol is the second try.

> 
> > I think it would be good to modify DxeTpm2MeasureBootLib so that the
> > CC measurement protocol is used if both protocols are installed. What
> > do you think?
> 
> Does it makes sense to use both protocols?
Agree with Gerd. I don't think we should use both protocols to do the measurement. 
My suggestion is that, first try CC protocol, if it fails, then try TCG2 protocol. Just as I explained above.

Sami, what's your thought?

Thanks
Min

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

* Re: [edk2-devel] [PATCH V4 3/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpmMeasurementLib
  2021-11-04 13:35       ` [edk2-devel] " Min Xu
@ 2021-11-04 13:49         ` Min Xu
  2021-11-04 14:18           ` Sami Mujawar
  0 siblings, 1 reply; 23+ messages in thread
From: Min Xu @ 2021-11-04 13:49 UTC (permalink / raw)
  To: devel@edk2.groups.io, kraxel@redhat.com, Sami Mujawar
  Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang, Yao, Jiewen,
	Wang, Jian J, nd

On November 4, 2021 9:35 PM, Xu Min wrote:
> On November 4, 2021 4:21 PM, Gerd Hoffmann wrote:
> >   Hi,
> >
> > > [SAMI] Apologies, I missed this in my previous review. I think the
> > > behaviour if both the TCG2 and CC measurement protocols are
> > > installed would be inconsistent between DxeTpmMeasurementLib and
> > > DxeTpm2MeasureBootLib. The main difference being in the later, the
> > > TCG2 protocol takes precedence for extending the measurement.
> >
> > Yes, we should have consistent behavior in both cases.
> In DxeTpmMeasurementLib, Cc measurement protocol is used as the first try. If
> it fails, then it try to measure with TCG2 / TCG protocol in turn.
> In DxeTpm2MeasureBootLib, TCG2 protocol is used the as the first try. If it fails,
> CC measurement protocol is tried in turn.
> Yes, this is inconsistent. I will update DxeTpm2MeasureBootLib to try Cc
> measurement protocol first, then try TCG2 protocol if Cc measurement protocol
> fails. In this way, only one protocol will be called to do the measurement. But
> TCG2 protocol is the first try, CC measurement protocol is the second try.
> 
> >
> > > I think it would be good to modify DxeTpm2MeasureBootLib so that the
> > > CC measurement protocol is used if both protocols are installed.
> > > What do you think?
> >
> > Does it makes sense to use both protocols?
> Agree with Gerd. I don't think we should use both protocols to do the
> measurement.
> My suggestion is that, first try CC protocol, if it fails, then try TCG2 protocol. Just
> as I explained above.

Another option will be that:
In DxeTpmMeasurementLib the pseudo would look like:
If (CC Protocol is installed) {
  Status = CcMeasureAndLogData (...)
} else {  // below is the original code
 Status = Tpm20MeasureAndLogData (...)    
 If (EFI_ERROR (Status)) { 
    Status = Tpm12MeasureAndLogData (...) 
 }
}

In DxeTpm2MeasureBootLib, the pseudo would look like:
If (CC Protocol is installed) {
    Status = DoCcMeasureBoot(...)
} else if (TCG2 protocol is installed) {
    Status = DoTcg2MeasureBoot(...)
}

Sami & Gerd
What's your thougth?

Thanks
Min

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

* Re: [edk2-devel] [PATCH V4 3/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpmMeasurementLib
  2021-11-04 13:49         ` Min Xu
@ 2021-11-04 14:18           ` Sami Mujawar
  2021-11-04 14:25             ` Yao, Jiewen
  0 siblings, 1 reply; 23+ messages in thread
From: Sami Mujawar @ 2021-11-04 14:18 UTC (permalink / raw)
  To: Xu, Min M, devel@edk2.groups.io, kraxel@redhat.com
  Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang, Yao, Jiewen,
	Wang, Jian J, nd

Hi Min,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar


On 04/11/2021 01:49 PM, Xu, Min M wrote:
> On November 4, 2021 9:35 PM, Xu Min wrote:
>> On November 4, 2021 4:21 PM, Gerd Hoffmann wrote:
>>>    Hi,
>>>
>>>> [SAMI] Apologies, I missed this in my previous review. I think the
>>>> behaviour if both the TCG2 and CC measurement protocols are
>>>> installed would be inconsistent between DxeTpmMeasurementLib and
>>>> DxeTpm2MeasureBootLib. The main difference being in the later, the
>>>> TCG2 protocol takes precedence for extending the measurement.
>>> Yes, we should have consistent behavior in both cases.
>> In DxeTpmMeasurementLib, Cc measurement protocol is used as the first try. If
>> it fails, then it try to measure with TCG2 / TCG protocol in turn.
>> In DxeTpm2MeasureBootLib, TCG2 protocol is used the as the first try. If it fails,
>> CC measurement protocol is tried in turn.
>> Yes, this is inconsistent. I will update DxeTpm2MeasureBootLib to try Cc
>> measurement protocol first, then try TCG2 protocol if Cc measurement protocol
>> fails. In this way, only one protocol will be called to do the measurement. But
>> TCG2 protocol is the first try, CC measurement protocol is the second try.
>>
>>>> I think it would be good to modify DxeTpm2MeasureBootLib so that the
>>>> CC measurement protocol is used if both protocols are installed.
>>>> What do you think?
>>> Does it makes sense to use both protocols?
>> Agree with Gerd. I don't think we should use both protocols to do the
>> measurement.
>> My suggestion is that, first try CC protocol, if it fails, then try TCG2 protocol. Just
>> as I explained above.
> Another option will be that:
> In DxeTpmMeasurementLib the pseudo would look like:
> If (CC Protocol is installed) {
>    Status = CcMeasureAndLogData (...)
> } else {  // below is the original code
>   Status = Tpm20MeasureAndLogData (...)
>   If (EFI_ERROR (Status)) {
>      Status = Tpm12MeasureAndLogData (...)
>   }
> }
>
> In DxeTpm2MeasureBootLib, the pseudo would look like:
> If (CC Protocol is installed) {
>      Status = DoCcMeasureBoot(...)
> } else if (TCG2 protocol is installed) {
>      Status = DoTcg2MeasureBoot(...)
> }
[SAMI] Your pseudo code looks good to me. It makes the measurement logic 
much clearer.
  Also, I am not aware if there is a use-case for both the CC Protocol 
and the TCG2 protocols to be installed at the same time.
[/SAMI]
> Sami & Gerd
> What's your thougth?
>
> Thanks
> Min


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

* Re: [edk2-devel] [PATCH V4 3/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpmMeasurementLib
  2021-11-04 14:18           ` Sami Mujawar
@ 2021-11-04 14:25             ` Yao, Jiewen
  0 siblings, 0 replies; 23+ messages in thread
From: Yao, Jiewen @ 2021-11-04 14:25 UTC (permalink / raw)
  To: Sami Mujawar, Xu, Min M, devel@edk2.groups.io, kraxel@redhat.com
  Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang, Wang, Jian J, nd

I believe a platform should have only one RTS/RTR.

Only one of (virtual)TPM1.2, (virtual)TPM2.0 and CC MR exists. Then only one TCG_SERVICE_PROTOCOL, TCG2_PROTOCOL, CC_MEASUREMENT_PROTOCOL is exposed.

In the case that, a vTPM is present to emulate the CC MR, then a TDVF should only expose TCG2_PROTOCOL. Otherwise, there will be confusing on the final event log.

Thank you
Yao Jiewen

> -----Original Message-----
> From: Sami Mujawar <sami.mujawar@arm.com>
> Sent: Thursday, November 4, 2021 10:18 PM
> To: Xu, Min M <min.m.xu@intel.com>; devel@edk2.groups.io;
> kraxel@redhat.com
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; nd
> <nd@arm.com>
> Subject: Re: [edk2-devel] [PATCH V4 3/3] SecurityPkg: Support
> CcMeasurementProtocol in DxeTpmMeasurementLib
> 
> Hi Min,
> 
> Please find my response inline marked [SAMI].
> 
> Regards,
> 
> Sami Mujawar
> 
> 
> On 04/11/2021 01:49 PM, Xu, Min M wrote:
> > On November 4, 2021 9:35 PM, Xu Min wrote:
> >> On November 4, 2021 4:21 PM, Gerd Hoffmann wrote:
> >>>    Hi,
> >>>
> >>>> [SAMI] Apologies, I missed this in my previous review. I think the
> >>>> behaviour if both the TCG2 and CC measurement protocols are
> >>>> installed would be inconsistent between DxeTpmMeasurementLib and
> >>>> DxeTpm2MeasureBootLib. The main difference being in the later, the
> >>>> TCG2 protocol takes precedence for extending the measurement.
> >>> Yes, we should have consistent behavior in both cases.
> >> In DxeTpmMeasurementLib, Cc measurement protocol is used as the first try.
> If
> >> it fails, then it try to measure with TCG2 / TCG protocol in turn.
> >> In DxeTpm2MeasureBootLib, TCG2 protocol is used the as the first try. If it
> fails,
> >> CC measurement protocol is tried in turn.
> >> Yes, this is inconsistent. I will update DxeTpm2MeasureBootLib to try Cc
> >> measurement protocol first, then try TCG2 protocol if Cc measurement
> protocol
> >> fails. In this way, only one protocol will be called to do the measurement. But
> >> TCG2 protocol is the first try, CC measurement protocol is the second try.
> >>
> >>>> I think it would be good to modify DxeTpm2MeasureBootLib so that the
> >>>> CC measurement protocol is used if both protocols are installed.
> >>>> What do you think?
> >>> Does it makes sense to use both protocols?
> >> Agree with Gerd. I don't think we should use both protocols to do the
> >> measurement.
> >> My suggestion is that, first try CC protocol, if it fails, then try TCG2 protocol.
> Just
> >> as I explained above.
> > Another option will be that:
> > In DxeTpmMeasurementLib the pseudo would look like:
> > If (CC Protocol is installed) {
> >    Status = CcMeasureAndLogData (...)
> > } else {  // below is the original code
> >   Status = Tpm20MeasureAndLogData (...)
> >   If (EFI_ERROR (Status)) {
> >      Status = Tpm12MeasureAndLogData (...)
> >   }
> > }
> >
> > In DxeTpm2MeasureBootLib, the pseudo would look like:
> > If (CC Protocol is installed) {
> >      Status = DoCcMeasureBoot(...)
> > } else if (TCG2 protocol is installed) {
> >      Status = DoTcg2MeasureBoot(...)
> > }
> [SAMI] Your pseudo code looks good to me. It makes the measurement logic
> much clearer.
>   Also, I am not aware if there is a use-case for both the CC Protocol
> and the TCG2 protocols to be installed at the same time.
> [/SAMI]
> > Sami & Gerd
> > What's your thougth?
> >
> > Thanks
> > Min


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

* Re: [edk2-devel] [PATCH V4 2/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpm2MeasureBootLib
  2021-11-02  9:43   ` Sami Mujawar
@ 2021-11-05  2:12     ` Min Xu
  0 siblings, 0 replies; 23+ messages in thread
From: Min Xu @ 2021-11-05  2:12 UTC (permalink / raw)
  To: devel@edk2.groups.io, sami.mujawar@arm.com
  Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang, Yao, Jiewen,
	Wang, Jian J, Gerd Hoffmann, nd

[-- Attachment #1: Type: text/plain, Size: 2497 bytes --]

Hi, Sami
Please see my comments inline.

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sami Mujawar
Sent: Tuesday, November 2, 2021 5:43 PM
To: Xu, Min M <min.m.xu@intel.com>; devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH V4 2/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpm2MeasureBootLib


+/**

+  Create CcEvent from Tcg2Event.

+

+  CcEvent is similar to Tcg2Event except the MrIndex.

+

+  @param  CcProtocol  Pointer to the located Cc Measurement protocol instance.

+  @param  Tcg2Event   Pointer to the Tcg2Event.

+  @param  EventSize   Size of the Event.

+  @param  EfiCcEvent  The created CcEvent

+

+  @retval EFI_SUCCESS           Successfully create the CcEvent

+  @retval EFI_INVALID_PARAMETER The input parameter is invalid

+  @retval EFI_UNSUPPORTED       The input PCRIndex cannot be mapped to Cc MR

+  @retval EFI_OUT_OF_RESOURCES  Out of resource

+**/

+EFI_STATUS
[SAMI] Is EFIAPI needed here?
[Min] EFIAPI is not needed here. From the EDKII C Coding Standards Spec (https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/56_declarations_and_types)
“The EFIAPI modifier must be used for all UEFI defined API functions, as well as for any function that takes a variable number of arguments. All protocol functions as well as public functions exposed by drivers must also be declared EFIAPI. This establishes a common calling convention for functions that could be referenced by other code that has potentially been built using a different compiler, with a different native calling convention”
CreateCcEventFromTcg2Event is only called internally and it will not be exposed outside. So EFIAPI is not needed.



+CreateCcEventFromTcg2Event (

+  IN  EFI_CC_MEASUREMENT_PROTOCOL   *CcProtocol,

+  IN  EFI_TCG2_EVENT                *Tcg2Event,

+  IN  UINT32                        EventSize,

+  IN OUT EFI_CC_EVENT               **EfiCcEvent

+  )

+{

+  UINT32            MrIndex;
[SAMI] I think it may be good to use the typedef for the measurment register index here i.e. EFI_CC_MR_INDEX.
[Min] Thanks for reminder. It will be fixed in the next version.

Thanks
Min_._,_._,_

[-- Attachment #2: Type: text/html, Size: 7273 bytes --]

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

* Re: [edk2-devel] [PATCH V4 3/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpmMeasurementLib
  2021-11-02  9:45   ` Sami Mujawar
  2021-11-04  8:20     ` Gerd Hoffmann
@ 2021-11-05  2:15     ` Min Xu
  1 sibling, 0 replies; 23+ messages in thread
From: Min Xu @ 2021-11-05  2:15 UTC (permalink / raw)
  To: devel@edk2.groups.io, sami.mujawar@arm.com
  Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang, Yao, Jiewen,
	Wang, Jian J, Gerd Hoffmann, nd

[-- Attachment #1: Type: text/plain, Size: 1197 bytes --]

Hi, Sami
Please see my comments inline.

+**/

+EFI_STATUS

+EFIAPI

+CcMeasureAndLogData (

+  IN UINT32             PcrIndex,

+  IN UINT32             EventType,

+  IN VOID               *EventLog,

+  IN UINT32             LogLen,

+  IN VOID               *HashData,

+  IN UINT64             HashDataLen

+  )

+{

+  EFI_STATUS                    Status;

+  EFI_CC_MEASUREMENT_PROTOCOL  *CcProtocol;

+  EFI_CC_EVENT                 *EfiCcEvent;

+  UINT32                        MrIndex;
[SAMI] Same comment as in patch 2/3. Is it possible to use the typedef for the measurment register index here, please?
[Min] Thanks for reminder. It will be fixed.



+

+  Status = gBS->LocateProtocol (&gEfiCcMeasurementProtocolGuid, NULL, (VOID **) &CcProtocol);

+  if (EFI_ERROR (Status)) {

+    return Status;

+  }

+

+  Status = CcProtocol->MapPcrToMrIndex (CcProtocol, PcrIndex, &MrIndex);

+  if (EFI_ERROR (Status)) {

+    return EFI_INVALID_PARAMETER;
[SAMI] Is it possible to return the error code returned by  CcProtocol->MapPcrToMrIndex(), please?
[Min] Sure. It will be updated in the next version.

Thanks
Min_._,_._,_

[-- Attachment #2: Type: text/html, Size: 5323 bytes --]

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

* 回复: [edk2-devel] 回复: [PATCH V4 1/3] MdePkg: Introduce CcMeasurementProtocol for CC Guest firmware
  2021-11-04 12:35     ` [edk2-devel] " Min Xu
@ 2021-11-05  5:20       ` gaoliming
  2021-11-05  6:22         ` Min Xu
  0 siblings, 1 reply; 23+ messages in thread
From: gaoliming @ 2021-11-05  5:20 UTC (permalink / raw)
  To: devel, min.m.xu
  Cc: 'Kinney, Michael D', 'Liu, Zhiguang',
	'Yao, Jiewen', 'Wang, Jian J', 'Lu, Ken',
	'Sami Mujawar', 'Gerd Hoffmann'

Min:

> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Min Xu
> 发送时间: 2021年11月4日 20:35
> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
> 抄送: Kinney, Michael D <michael.d.kinney@intel.com>; Liu, Zhiguang
> <zhiguang.liu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Lu, Ken <ken.lu@intel.com>; 'Sami Mujawar'
> <sami.mujawar@arm.com>; 'Gerd Hoffmann' <kraxel@redhat.com>
> 主题: Re: [edk2-devel] 回复: [PATCH V4 1/3] MdePkg: Introduce
> CcMeasurementProtocol for CC Guest firmware
> 
> On November 4, 2021 1:51 PM, Gao, Liming wrote:
> > Min:
> >   I have one minor comment. gEfiCcFinalEventsTableGuid may be placed
> into
> > [Guids] section instead of [Protocols] section.
> >
> Hi, Liming
> I follow the definition of gEfiTcg2ProtocolGuid and
> gEfiTcg2FinalEventsTableGuid. See
> https://github.com/tianocore/edk2/blob/master/MdePkg/MdePkg.dec#L159
> 0-L1592
> Actually gEfiCcMeasurementProtocolGuid and gEfiCcFinalEventsTableGuid are
> the counterpart protocol/guid definition in Confidential Computing measure
> boot.
> I am not sure if there is some other consideration that
gEfiTcg2ProtocolGuid
> and gEfiTcg2FinalEventsTableGuid are defined in the section of
[Protocols].
I find gEfiTcg2FinalEventsTableGuid is used for configuration table. This is
one Guid usage. 
It should be placed into [Guids] section. 

You can see hash protocol and hash algorithm guid in MdePkg. 
gEfiHashProtocolGuid is defined in [Protocols] section, and
gEfiHashAlgorithmMD5Guid
is defined in [Guids] section. They are both from
MdePkg/Include/Protocol/Hash.h.

So, I suggest to follow the guid usage to define this Guid into the
different section.

Thanks
Liming
> 
> Thanks
> Min
> 
> 
> 
> 




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

* Re: [edk2-devel] 回复: [PATCH V4 1/3] MdePkg: Introduce CcMeasurementProtocol for CC Guest firmware
  2021-11-05  5:20       ` 回复: " gaoliming
@ 2021-11-05  6:22         ` Min Xu
  0 siblings, 0 replies; 23+ messages in thread
From: Min Xu @ 2021-11-05  6:22 UTC (permalink / raw)
  To: gaoliming, devel@edk2.groups.io
  Cc: Kinney, Michael D, Liu, Zhiguang, Yao, Jiewen, Wang, Jian J,
	Lu, Ken, 'Sami Mujawar', 'Gerd Hoffmann'

On November 5, 2021 1:20 PM, Gao Liming wrote:
> > On November 4, 2021 1:51 PM, Gao, Liming wrote:
> > > Min:
> > >   I have one minor comment. gEfiCcFinalEventsTableGuid may be placed
> > into
> > > [Guids] section instead of [Protocols] section.
> > >
> > Hi, Liming
> > I follow the definition of gEfiTcg2ProtocolGuid and
> > gEfiTcg2FinalEventsTableGuid. See
> > https://github.com/tianocore/edk2/blob/master/MdePkg/MdePkg.dec#L159
> > 0-L1592
> > Actually gEfiCcMeasurementProtocolGuid and gEfiCcFinalEventsTableGuid
> > are the counterpart protocol/guid definition in Confidential Computing
> > measure boot.
> > I am not sure if there is some other consideration that gEfiTcg2ProtocolGuid
> > and gEfiTcg2FinalEventsTableGuid are defined in the section of [Protocols].
> I find gEfiTcg2FinalEventsTableGuid is used for configuration table. This is one
> Guid usage.
> It should be placed into [Guids] section.
> 
> You can see hash protocol and hash algorithm guid in MdePkg.
> gEfiHashProtocolGuid is defined in [Protocols] section, and
> gEfiHashAlgorithmMD5Guid is defined in [Guids] section. They are both from
> MdePkg/Include/Protocol/Hash.h.
> 
> So, I suggest to follow the guid usage to define this Guid into the different
> section.
> 
Thanks for the reminder. I agree with you that gEfiTcg2FinalEventsTableGuid should go to [Guids] section. 
It will be fixed in the next version.

Thanks
Min

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

end of thread, other threads:[~2021-11-05  6:22 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-02  2:50 [PATCH V4 0/3] Introduce CcMeasurementProtocol into EDK2 Min Xu
2021-11-02  2:50 ` [PATCH V4 1/3] MdePkg: Introduce CcMeasurementProtocol for CC Guest firmware Min Xu
2021-11-02  6:24   ` Yao, Jiewen
2021-11-02  9:41   ` Sami Mujawar
2021-11-04  5:51   ` 回复: " gaoliming
2021-11-04 12:35     ` [edk2-devel] " Min Xu
2021-11-05  5:20       ` 回复: " gaoliming
2021-11-05  6:22         ` Min Xu
2021-11-02  2:50 ` [PATCH V4 2/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpm2MeasureBootLib Min Xu
2021-11-02  6:24   ` Yao, Jiewen
2021-11-03  2:59     ` Min Xu
2021-11-02  9:43   ` Sami Mujawar
2021-11-05  2:12     ` [edk2-devel] " Min Xu
2021-11-02  2:50 ` [PATCH V4 3/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpmMeasurementLib Min Xu
2021-11-02  6:24   ` Yao, Jiewen
2021-11-03  3:01     ` Min Xu
2021-11-02  9:45   ` Sami Mujawar
2021-11-04  8:20     ` Gerd Hoffmann
2021-11-04 13:35       ` [edk2-devel] " Min Xu
2021-11-04 13:49         ` Min Xu
2021-11-04 14:18           ` Sami Mujawar
2021-11-04 14:25             ` Yao, Jiewen
2021-11-05  2:15     ` Min Xu

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