* [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event support.
@ 2019-12-31 6:44 Yao, Jiewen
2019-12-31 6:44 ` [PATCH 1/6] SecurityPkg/Guid: Add TCG 800-155 event GUID definition Yao, Jiewen
` (7 more replies)
0 siblings, 8 replies; 21+ messages in thread
From: Yao, Jiewen @ 2019-12-31 6:44 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Chao Zhang
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
This patch series adds TCG PFP rev 105 and 800-155 event support.
TCG published Platform Firmware Profile spec revision 105
(https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_PFP_r1p05_05_3feb20.pdf)
and Firmware Integrity Measurement spec
(https://trustedcomputinggroup.org/wp-content/uploads/TCG_PC_Client-FIM_v1r24_3feb20.pdf)
2 major impact to the BIOS:
1) Need add 800-155 event at the beginning of the TCG2 event log.
2) Need use EV_EFI_PLATFORM_FIRMWARE_BLOB2 to replace EV_EFI_PLATFORM_FIRMWARE_BLOB
and use EV_EFI_HANDOFF_TABLES2 to replace EV_EFI_HANDOFF_TABLES.
TCG2 DXE update: Add 800-155 event handling.
TCG2 PEI update: Use EV_EFI_PLATFORM_FIRMWARE_BLOB2.
Smbios Update: Use EV_EFI_HANDOFF_TABLES2.
This patch series is tested on EmulatorPkg with TPM2 simulator.
(https://github.com/jyao1/edk2/tree/feature_tpm_emulator).
The new entries - 800-155 event, EV_EFI_PLATFORM_FIRMWARE_BLOB2, EV_EFI_HANDOFF_TABLES2,
can be dumpped with UEFI Tcg2DumpLog tool.
(https://github.com/jyao1/EdkiiShellTool/tree/master/EdkiiShellToolPkg/Tcg2DumpLog)
NOTE: To support the full 800-155 requirement in TCG, a platform need report the
platform specific 800-155 event.
If the platform chooses to report in PEI, it can refer to the example at
https://github.com/jyao1/edk2/tree/feature_tpm_emulator/EmulatorPkg/Tpm2/Platform800155EventPei.
If the platform chooses to report in DXE, it can refer to the example at
https://github.com/jyao1/edk2/tree/feature_tpm_emulator/EmulatorPkg/Tpm2/Platform800155EventDxe.
Multiple 800-155 events are support.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
Jiewen Yao (6):
SecurityPkg/Guid: Add TCG 800-155 event GUID definition.
SecurityPkg/Tcg2Dxe: Add Tcg2Dxe to support 800-155 event.
MdeModulePkg/Smbios: Done measure Smbios multiple times.
MdeModulePkg/dec: add PcdTcgPfpMeasurementRevision PCD
MdeModulePkg/Smbios: Add TCG PFP rev 105 support.
SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
MdeModulePkg/MdeModulePkg.dec | 8 +
.../SmbiosMeasurementDxe.c | 39 ++++-
.../SmbiosMeasurementDxe.inf | 3 +
SecurityPkg/Include/Guid/TcgEventHob.h | 11 ++
SecurityPkg/SecurityPkg.dec | 4 +
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 157 ++++++++++++++----
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf | 1 +
SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 91 +++++++++-
SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 2 +
9 files changed, 273 insertions(+), 43 deletions(-)
--
2.19.2.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/6] SecurityPkg/Guid: Add TCG 800-155 event GUID definition.
2019-12-31 6:44 [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event support Yao, Jiewen
@ 2019-12-31 6:44 ` Yao, Jiewen
2020-01-06 3:22 ` Wang, Jian J
2019-12-31 6:44 ` [PATCH 2/6] SecurityPkg/Tcg2Dxe: Add Tcg2Dxe to support 800-155 event Yao, Jiewen
` (6 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Yao, Jiewen @ 2019-12-31 6:44 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Chao Zhang
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
The PEIM can produce the 800-155 event and the event
will be recorded to TCG event log by the TCG2 DXE.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
SecurityPkg/Include/Guid/TcgEventHob.h | 11 +++++++++++
SecurityPkg/SecurityPkg.dec | 4 ++++
2 files changed, 15 insertions(+)
diff --git a/SecurityPkg/Include/Guid/TcgEventHob.h b/SecurityPkg/Include/Guid/TcgEventHob.h
index eef3f92abd..97e40b47d0 100644
--- a/SecurityPkg/Include/Guid/TcgEventHob.h
+++ b/SecurityPkg/Include/Guid/TcgEventHob.h
@@ -49,4 +49,15 @@ extern EFI_GUID gTpmErrorHobGuid;
extern EFI_GUID gTpm2StartupLocalityHobGuid;
+///
+/// The Global ID of a GUIDed HOB used to record TCG 800-155 PlatformId Event.
+/// HOB payload is the whole TCG_Sp800_155_PlatformId_Event2 according to TCG 800-155 PlatformId Event.
+///
+#define EFI_TCG_800_155_PLATFORM_ID_EVENT_HOB_GUID \
+ { \
+ 0xe2c3bc69, 0x615c, 0x4b5b, { 0x8e, 0x5c, 0xa0, 0x33, 0xa9, 0xc2, 0x5e, 0xd6 } \
+ }
+
+extern EFI_GUID gTcg800155PlatformIdEventHobGuid;
+
#endif
diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
index cac36caf0a..5335cc5397 100644
--- a/SecurityPkg/SecurityPkg.dec
+++ b/SecurityPkg/SecurityPkg.dec
@@ -121,6 +121,10 @@
## Include/Guid/TcgEventHob.h
gTpm2StartupLocalityHobGuid = { 0x397b0c9, 0x22e8, 0x459e, { 0xa4, 0xff, 0x99, 0xbc, 0x65, 0x27, 0x9, 0x29 }}
+ ## HOB GUID used to record TCG 800-155 PlatformId Event
+ ## Include/Guid/TcgEventHob.h
+ gTcg800155PlatformIdEventHobGuid = { 0xe2c3bc69, 0x615c, 0x4b5b, { 0x8e, 0x5c, 0xa0, 0x33, 0xa9, 0xc2, 0x5e, 0xd6 }}
+
## HOB GUID used to pass all PEI measured FV info to DXE Driver.
# Include/Guid/MeasuredFvHob.h
gMeasuredFvHobGuid = { 0xb2360b42, 0x7173, 0x420a, { 0x86, 0x96, 0x46, 0xca, 0x6b, 0xab, 0x10, 0x60 }}
--
2.19.2.windows.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/6] SecurityPkg/Tcg2Dxe: Add Tcg2Dxe to support 800-155 event.
2019-12-31 6:44 [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event support Yao, Jiewen
2019-12-31 6:44 ` [PATCH 1/6] SecurityPkg/Guid: Add TCG 800-155 event GUID definition Yao, Jiewen
@ 2019-12-31 6:44 ` Yao, Jiewen
2020-01-06 5:59 ` [edk2-devel] " Wang, Jian J
2019-12-31 6:44 ` [PATCH 3/6] MdeModulePkg/Smbios: Done measure Smbios multiple times Yao, Jiewen
` (5 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Yao, Jiewen @ 2019-12-31 6:44 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Chao Zhang
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
The TCG2 DXE supports to parse the 800-155 event GUID from PEI
and puts to the beginning of the TCG2 event.
The TCG2 DXE also supports a DXE driver produces 800-155 event
and let TCG2 DXE driver record.
The 800-155 is a NO-ACTION event which does not need extend
anything to TPM2. The TCG2 DXE also supports that.
Multiple 800-155 events are supported. All of them will be put
to the beginning of the TCG2 event, just after the SpecId event.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 157 +++++++++++++++++++++++-----
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf | 1 +
2 files changed, 129 insertions(+), 29 deletions(-)
diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
index 3cd16c2fa3..b185b56703 100644
--- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
+++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
@@ -75,6 +75,7 @@ typedef struct {
UINT8 *LastEvent;
BOOLEAN EventLogStarted;
BOOLEAN EventLogTruncated;
+ UINTN Next800155EventOffset;
} TCG_EVENT_LOG_AREA_STRUCT;
typedef struct _TCG_DXE_DATA {
@@ -771,16 +772,42 @@ Tcg2GetEventLog (
return EFI_SUCCESS;
}
+/*
+ Return if this is a Tcg800155PlatformIdEvent.
+
+ @param[in] NewEventHdr Pointer to a TCG_PCR_EVENT_HDR/TCG_PCR_EVENT_EX data structure.
+ @param[in] NewEventHdrSize New event header size.
+ @param[in] NewEventData Pointer to the new event data.
+ @param[in] NewEventSize New event data size.
+
+ @retval TRUE This is a Tcg800155PlatformIdEvent.
+ @retval FALSE This is NOT a Tcg800155PlatformIdEvent.
+
+*/
+BOOLEAN
+Is800155Event (
+ IN VOID *NewEventHdr,
+ IN UINT32 NewEventHdrSize,
+ IN UINT8 *NewEventData,
+ IN UINT32 NewEventSize
+ )
+{
+ if ((((TCG_PCR_EVENT2_HDR *)NewEventHdr)->EventType == EV_NO_ACTION) &&
+ (NewEventSize >= sizeof(TCG_Sp800_155_PlatformId_Event2)) &&
+ (CompareMem (NewEventData, TCG_Sp800_155_PlatformId_Event2_SIGNATURE, sizeof(TCG_Sp800_155_PlatformId_Event2_SIGNATURE) - 1) == 0)) {
+ return TRUE;
+ }
+ return FALSE;
+}
+
/**
Add a new entry to the Event Log.
- @param[in, out] EventLogPtr Pointer to the Event Log data.
- @param[in, out] LogSize Size of the Event Log.
- @param[in] MaxSize Maximum size of the Event Log.
- @param[in] NewEventHdr Pointer to a TCG_PCR_EVENT_HDR/TCG_PCR_EVENT_EX data structure.
- @param[in] NewEventHdrSize New event header size.
- @param[in] NewEventData Pointer to the new event data.
- @param[in] NewEventSize New event data size.
+ @param[in, out] EventLogAreaStruct The event log area data structure
+ @param[in] NewEventHdr Pointer to a TCG_PCR_EVENT_HDR/TCG_PCR_EVENT_EX data structure.
+ @param[in] NewEventHdrSize New event header size.
+ @param[in] NewEventData Pointer to the new event data.
+ @param[in] NewEventSize New event data size.
@retval EFI_SUCCESS The new event log entry was added.
@retval EFI_OUT_OF_RESOURCES No enough memory to log the new event.
@@ -788,9 +815,7 @@ Tcg2GetEventLog (
**/
EFI_STATUS
TcgCommLogEvent (
- IN OUT UINT8 **EventLogPtr,
- IN OUT UINTN *LogSize,
- IN UINTN MaxSize,
+ IN OUT TCG_EVENT_LOG_AREA_STRUCT *EventLogAreaStruct,
IN VOID *NewEventHdr,
IN UINT32 NewEventHdrSize,
IN UINT8 *NewEventData,
@@ -798,6 +823,7 @@ TcgCommLogEvent (
)
{
UINTN NewLogSize;
+ BOOLEAN Record800155Event;
if (NewEventSize > MAX_ADDRESS - NewEventHdrSize) {
return EFI_OUT_OF_RESOURCES;
@@ -805,23 +831,55 @@ TcgCommLogEvent (
NewLogSize = NewEventHdrSize + NewEventSize;
- if (NewLogSize > MAX_ADDRESS - *LogSize) {
+ if (NewLogSize > MAX_ADDRESS - EventLogAreaStruct->EventLogSize) {
return EFI_OUT_OF_RESOURCES;
}
- if (NewLogSize + *LogSize > MaxSize) {
- DEBUG ((EFI_D_INFO, " MaxSize - 0x%x\n", MaxSize));
- DEBUG ((EFI_D_INFO, " NewLogSize - 0x%x\n", NewLogSize));
- DEBUG ((EFI_D_INFO, " LogSize - 0x%x\n", *LogSize));
- DEBUG ((EFI_D_INFO, "TcgCommLogEvent - %r\n", EFI_OUT_OF_RESOURCES));
+ if (NewLogSize + EventLogAreaStruct->EventLogSize > EventLogAreaStruct->Laml) {
+ DEBUG ((DEBUG_INFO, " Laml - 0x%x\n", EventLogAreaStruct->Laml));
+ DEBUG ((DEBUG_INFO, " NewLogSize - 0x%x\n", NewLogSize));
+ DEBUG ((DEBUG_INFO, " LogSize - 0x%x\n", EventLogAreaStruct->EventLogSize));
+ DEBUG ((DEBUG_INFO, "TcgCommLogEvent - %r\n", EFI_OUT_OF_RESOURCES));
return EFI_OUT_OF_RESOURCES;
}
- *EventLogPtr += *LogSize;
- *LogSize += NewLogSize;
- CopyMem (*EventLogPtr, NewEventHdr, NewEventHdrSize);
+ //
+ // Check 800-155 event
+ // Record to 800-155 event offset only.
+ // If the offset is 0, no need to record.
+ //
+ Record800155Event = Is800155Event (NewEventHdr, NewEventHdrSize, NewEventData, NewEventSize);
+ if (Record800155Event) {
+ if (EventLogAreaStruct->Next800155EventOffset != 0) {
+ CopyMem (
+ (UINT8 *)(UINTN)EventLogAreaStruct->Lasa + EventLogAreaStruct->Next800155EventOffset + NewLogSize,
+ (UINT8 *)(UINTN)EventLogAreaStruct->Lasa + EventLogAreaStruct->Next800155EventOffset,
+ EventLogAreaStruct->EventLogSize - EventLogAreaStruct->Next800155EventOffset
+ );
+
+ CopyMem (
+ (UINT8 *)(UINTN)EventLogAreaStruct->Lasa + EventLogAreaStruct->Next800155EventOffset,
+ NewEventHdr,
+ NewEventHdrSize
+ );
+ CopyMem (
+ (UINT8 *)(UINTN)EventLogAreaStruct->Lasa + EventLogAreaStruct->Next800155EventOffset + NewEventHdrSize,
+ NewEventData,
+ NewEventSize
+ );
+
+ EventLogAreaStruct->Next800155EventOffset += NewLogSize;
+ EventLogAreaStruct->LastEvent += NewLogSize;
+ EventLogAreaStruct->EventLogSize += NewLogSize;
+ }
+ return EFI_SUCCESS;
+ }
+
+ EventLogAreaStruct->LastEvent = (UINT8 *)(UINTN)EventLogAreaStruct->Lasa + EventLogAreaStruct->EventLogSize;
+ EventLogAreaStruct->EventLogSize += NewLogSize;
+ CopyMem (EventLogAreaStruct->LastEvent, NewEventHdr, NewEventHdrSize);
CopyMem (
- *EventLogPtr + NewEventHdrSize,
+ EventLogAreaStruct->LastEvent + NewEventHdrSize,
NewEventData,
NewEventSize
);
@@ -873,11 +931,8 @@ TcgDxeLogEvent (
return EFI_VOLUME_FULL;
}
- EventLogAreaStruct->LastEvent = (UINT8*)(UINTN)EventLogAreaStruct->Lasa;
Status = TcgCommLogEvent (
- &EventLogAreaStruct->LastEvent,
- &EventLogAreaStruct->EventLogSize,
- (UINTN)EventLogAreaStruct->Laml,
+ EventLogAreaStruct,
NewEventHdr,
NewEventHdrSize,
NewEventData,
@@ -907,11 +962,8 @@ TcgDxeLogEvent (
return EFI_VOLUME_FULL;
}
- EventLogAreaStruct->LastEvent = (UINT8*)(UINTN)EventLogAreaStruct->Lasa;
Status = TcgCommLogEvent (
- &EventLogAreaStruct->LastEvent,
- &EventLogAreaStruct->EventLogSize,
- (UINTN)EventLogAreaStruct->Laml,
+ EventLogAreaStruct,
NewEventHdr,
NewEventHdrSize,
NewEventData,
@@ -1138,11 +1190,25 @@ TcgDxeHashLogExtendEvent (
{
EFI_STATUS Status;
TPML_DIGEST_VALUES DigestList;
+ TCG_PCR_EVENT2_HDR NoActionEvent;
if (!mTcgDxeData.BsCap.TPMPresentFlag) {
return EFI_DEVICE_ERROR;
}
+ if (NewEventHdr->EventType == EV_NO_ACTION) {
+ //
+ // Do not do TPM extend for EV_NO_ACTION
+ //
+ Status = EFI_SUCCESS;
+ InitNoActionEvent (&NoActionEvent, NewEventHdr->EventSize);
+ if ((Flags & EFI_TCG2_EXTEND_ONLY) == 0) {
+ Status = TcgDxeLogHashEvent (&(NoActionEvent.Digests), NewEventHdr, NewEventData);
+ }
+
+ return Status;
+ }
+
Status = HashAndExtend (
NewEventHdr->PCRIndex,
HashData,
@@ -1202,7 +1268,13 @@ Tcg2HashLogExtendEvent (
DEBUG ((DEBUG_VERBOSE, "Tcg2HashLogExtendEvent ...\n"));
- if ((This == NULL) || (DataToHash == 0) || (Event == NULL)) {
+ if ((This == NULL) || (Event == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+ //
+ // Do not check hash data size for EV_NO_ACTION event.
+ //
+ if ((Event->Header.EventType != EV_NO_ACTION) && (DataToHash == 0)) {
return EFI_INVALID_PARAMETER;
}
@@ -1487,6 +1559,7 @@ SetupEventLog (
}
mTcgDxeData.EventLogAreaStruct[Index].Lasa = Lasa;
mTcgDxeData.EventLogAreaStruct[Index].Laml = PcdGet32 (PcdTcgLogAreaMinLen);
+ mTcgDxeData.EventLogAreaStruct[Index].Next800155EventOffset = 0;
if ((PcdGet8(PcdTpm2AcpiTableRev) >= 4) ||
(mTcg2EventInfo[Index].LogFormat == EFI_TCG2_EVENT_LOG_FORMAT_TCG_2)) {
@@ -1577,6 +1650,30 @@ SetupEventLog (
(UINT8 *)TcgEfiSpecIdEventStruct,
SpecIdEvent.EventSize
);
+ //
+ // record the offset at the end of 800-155 event.
+ // the future 800-155 event can be inserted here.
+ //
+ mTcgDxeData.EventLogAreaStruct[Index].Next800155EventOffset = mTcgDxeData.EventLogAreaStruct[Index].EventLogSize;
+
+ //
+ // Tcg800155PlatformIdEvent. Event format is TCG_PCR_EVENT2
+ //
+ GuidHob.Guid = GetFirstGuidHob (&gTcg800155PlatformIdEventHobGuid);
+ while (GuidHob.Guid != NULL) {
+ InitNoActionEvent(&NoActionEvent, GET_GUID_HOB_DATA_SIZE (GuidHob.Guid));
+
+ Status = TcgDxeLogEvent (
+ mTcg2EventInfo[Index].LogFormat,
+ &NoActionEvent,
+ sizeof(NoActionEvent.PCRIndex) + sizeof(NoActionEvent.EventType) + GetDigestListBinSize (&NoActionEvent.Digests) + sizeof(NoActionEvent.EventSize),
+ GET_GUID_HOB_DATA (GuidHob.Guid),
+ GET_GUID_HOB_DATA_SIZE (GuidHob.Guid)
+ );
+
+ GuidHob.Guid = GET_NEXT_HOB (GuidHob);
+ GuidHob.Guid = GetNextGuidHob (&gTcg800155PlatformIdEventHobGuid, GuidHob.Guid);
+ }
//
// EfiStartupLocalityEvent. Event format is TCG_PCR_EVENT2
@@ -1643,6 +1740,7 @@ SetupEventLog (
mTcgDxeData.FinalEventLogAreaStruct[Index].LastEvent = (VOID *)(UINTN)mTcgDxeData.FinalEventLogAreaStruct[Index].Lasa;
mTcgDxeData.FinalEventLogAreaStruct[Index].EventLogStarted = FALSE;
mTcgDxeData.FinalEventLogAreaStruct[Index].EventLogTruncated = FALSE;
+ mTcgDxeData.FinalEventLogAreaStruct[Index].Next800155EventOffset = 0;
//
// Install to configuration table for EFI_TCG2_EVENT_LOG_FORMAT_TCG_2
@@ -1663,6 +1761,7 @@ SetupEventLog (
mTcgDxeData.FinalEventLogAreaStruct[Index].LastEvent = 0;
mTcgDxeData.FinalEventLogAreaStruct[Index].EventLogStarted = FALSE;
mTcgDxeData.FinalEventLogAreaStruct[Index].EventLogTruncated = FALSE;
+ mTcgDxeData.FinalEventLogAreaStruct[Index].Next800155EventOffset = 0;
}
}
}
diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
index 0127a31e97..576cf80d06 100644
--- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
+++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
@@ -85,6 +85,7 @@
gTcgEvent2EntryHobGuid ## SOMETIMES_CONSUMES ## HOB
gTpm2StartupLocalityHobGuid ## SOMETIMES_CONSUMES ## HOB
+ gTcg800155PlatformIdEventHobGuid ## SOMETIMES_CONSUMES ## HOB
[Protocols]
gEfiTcg2ProtocolGuid ## PRODUCES
--
2.19.2.windows.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/6] MdeModulePkg/Smbios: Done measure Smbios multiple times.
2019-12-31 6:44 [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event support Yao, Jiewen
2019-12-31 6:44 ` [PATCH 1/6] SecurityPkg/Guid: Add TCG 800-155 event GUID definition Yao, Jiewen
2019-12-31 6:44 ` [PATCH 2/6] SecurityPkg/Tcg2Dxe: Add Tcg2Dxe to support 800-155 event Yao, Jiewen
@ 2019-12-31 6:44 ` Yao, Jiewen
2020-01-02 11:01 ` Zeng, Star
2019-12-31 6:44 ` [PATCH 4/6] MdeModulePkg/dec: add PcdTcgPfpMeasurementRevision PCD Yao, Jiewen
` (4 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Yao, Jiewen @ 2019-12-31 6:44 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Hao A Wu, Dandan Bi, Star Zeng
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
In current implementation, the SMBIOS table is measured multiple
time in every readytoboot event.
This causes Smbios Table record appears multiple time in the TCG event log
and confuses people.
This issue makes it hard to implement 800-155 reference measurement.
This patch closes the event to make sure Smbios is measured only once.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
.../Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
index 7b5d473146..5ec2aca095 100644
--- a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
+++ b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
@@ -577,8 +577,8 @@ MeasureSmbiosTable (
TableAddress, // HashData
TableLength // HashDataLen
);
- if (EFI_ERROR (Status)) {
- return ;
+ if (!EFI_ERROR (Status)) {
+ gBS->CloseEvent (Event) ;
}
}
--
2.19.2.windows.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/6] MdeModulePkg/dec: add PcdTcgPfpMeasurementRevision PCD
2019-12-31 6:44 [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event support Yao, Jiewen
` (2 preceding siblings ...)
2019-12-31 6:44 ` [PATCH 3/6] MdeModulePkg/Smbios: Done measure Smbios multiple times Yao, Jiewen
@ 2019-12-31 6:44 ` Yao, Jiewen
2020-01-06 3:13 ` Wang, Jian J
2019-12-31 6:44 ` [PATCH 5/6] MdeModulePkg/Smbios: Add TCG PFP rev 105 support Yao, Jiewen
` (3 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Yao, Jiewen @ 2019-12-31 6:44 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Hao A Wu, Chao Zhang
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
This PCD is to control the TCG PFP spec revision.
The PFP 105 added new event type to support NIST SP800-155,
and deprecated old event type.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdeModulePkg/MdeModulePkg.dec | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 41b9e70a1a..f75a74af25 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -2003,6 +2003,14 @@
# @Prompt Capsule On Disk relocation device path.
gEfiMdeModulePkgTokenSpaceGuid.PcdCodRelocationDevPath|{0xFF}|VOID*|0x0000002f
+ ## Indicates which TCG Platform Firmware Profile revision the EDKII firmware follows.
+ # The revision number is defined in MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
+ # 0: This is for compatiblity support.
+ # 105: This is the first revision to support 800-155 is related event, such as
+ # EV_EFI_PLATFORM_FIRMWARE_BLOB2 and EV_EFI_HANDOFF_TABLES2.
+ # @Prompt TCG Platform Firmware Profile revision.
+ gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision|0|UINT32|0x00010077
+
[PcdsPatchableInModule]
## Specify memory size with page number for PEI code when
# Loading Module at Fixed Address feature is enabled.
--
2.19.2.windows.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 5/6] MdeModulePkg/Smbios: Add TCG PFP rev 105 support.
2019-12-31 6:44 [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event support Yao, Jiewen
` (3 preceding siblings ...)
2019-12-31 6:44 ` [PATCH 4/6] MdeModulePkg/dec: add PcdTcgPfpMeasurementRevision PCD Yao, Jiewen
@ 2019-12-31 6:44 ` Yao, Jiewen
2020-01-02 11:09 ` Zeng, Star
2019-12-31 6:44 ` [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP " Yao, Jiewen
` (2 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Yao, Jiewen @ 2019-12-31 6:44 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Hao A Wu, Dandan Bi, Star Zeng
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
Report EV_EFI_HANDOFF_TABLES2 if the platform chooses PFP >= 105.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
.../SmbiosMeasurementDxe.c | 35 +++++++++++++++++--
.../SmbiosMeasurementDxe.inf | 3 ++
2 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
index 5ec2aca095..a5839c09f1 100644
--- a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
+++ b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c
@@ -108,6 +108,18 @@ SMBIOS_FILTER_STRUCT mSmbiosFilterStandardTableBlackList[] = {
EFI_SMBIOS_PROTOCOL *mSmbios;
UINTN mMaxLen;
+#pragma pack (1)
+
+#define SMBIOS_HANDOFF_TABLE_DESC "SmbiosTable"
+typedef struct {
+ UINT8 TableDescriptionSize;
+ UINT8 TableDescription[sizeof(SMBIOS_HANDOFF_TABLE_DESC)];
+ UINT64 NumberOfTables;
+ EFI_CONFIGURATION_TABLE TableEntry[1];
+} SMBIOS_HANDOFF_TABLE_POINTERS2;
+
+#pragma pack ()
+
/**
This function dump raw data.
@@ -460,6 +472,10 @@ MeasureSmbiosTable (
{
EFI_STATUS Status;
EFI_HANDOFF_TABLE_POINTERS HandoffTables;
+ SMBIOS_HANDOFF_TABLE_POINTERS2 SmbiosHandoffTables2;
+ UINT32 EventType;
+ VOID *EventLog;
+ UINT32 EventLogSize;
SMBIOS_TABLE_ENTRY_POINT *SmbiosTable;
SMBIOS_TABLE_3_0_ENTRY_POINT *Smbios3Table;
VOID *SmbiosTableAddress;
@@ -569,11 +585,24 @@ MeasureSmbiosTable (
CopyGuid (&(HandoffTables.TableEntry[0].VendorGuid), &gEfiSmbiosTableGuid);
HandoffTables.TableEntry[0].VendorTable = SmbiosTable;
}
+ EventType = EV_EFI_HANDOFF_TABLES;
+ EventLog = &HandoffTables;
+ EventLogSize = sizeof (HandoffTables);
+
+ if (PcdGet32(PcdTcgPfpMeasurementRevision) >= TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105) {
+ SmbiosHandoffTables2.TableDescriptionSize = sizeof(SmbiosHandoffTables2.TableDescription);
+ CopyMem (SmbiosHandoffTables2.TableDescription, SMBIOS_HANDOFF_TABLE_DESC, sizeof(SmbiosHandoffTables2.TableDescription));
+ SmbiosHandoffTables2.NumberOfTables = HandoffTables.NumberOfTables;
+ CopyMem (&(SmbiosHandoffTables2.TableEntry[0]), &(HandoffTables.TableEntry[0]), sizeof(SmbiosHandoffTables2.TableEntry[0]));
+ EventType = EV_EFI_HANDOFF_TABLES2;
+ EventLog = &SmbiosHandoffTables2;
+ EventLogSize = sizeof (SmbiosHandoffTables2);
+ }
Status = TpmMeasureAndLogData (
1, // PCRIndex
- EV_EFI_HANDOFF_TABLES, // EventType
- &HandoffTables, // EventLog
- sizeof (HandoffTables), // LogLen
+ EventType, // EventType
+ EventLog, // EventLog
+ EventLogSize, // LogLen
TableAddress, // HashData
TableLength // HashDataLen
);
diff --git a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.inf b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.inf
index a074044c84..81d3655dc7 100644
--- a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.inf
+++ b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.inf
@@ -57,6 +57,9 @@
gEfiSmbiosTableGuid ## SOMETIMES_CONSUMES ## SystemTable
gEfiSmbios3TableGuid ## SOMETIMES_CONSUMES ## SystemTable
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision ## CONSUMES
+
[Depex]
gEfiSmbiosProtocolGuid
--
2.19.2.windows.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
2019-12-31 6:44 [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event support Yao, Jiewen
` (4 preceding siblings ...)
2019-12-31 6:44 ` [PATCH 5/6] MdeModulePkg/Smbios: Add TCG PFP rev 105 support Yao, Jiewen
@ 2019-12-31 6:44 ` Yao, Jiewen
2020-01-06 5:33 ` Wang, Jian J
2020-01-02 0:11 ` [edk2-devel] [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event support Liming Gao
2020-01-06 6:11 ` Wang, Jian J
7 siblings, 1 reply; 21+ messages in thread
From: Yao, Jiewen @ 2019-12-31 6:44 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Chao Zhang
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
Use EV_EFI_PLATFORM_FIRMWARE_BLOB2 if the TCG PFP revision is >= 105.
Use FvName as the description for the FV.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 91 ++++++++++++++++++++++++++---
SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 2 +
2 files changed, 84 insertions(+), 9 deletions(-)
diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
index 1565d4e402..7d99c7906a 100644
--- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
+++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
@@ -37,6 +37,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/MemoryAllocationLib.h>
#include <Library/ReportStatusCodeLib.h>
#include <Library/ResetSystemLib.h>
+#include <Library/PrintLib.h>
#define PERF_ID_TCG2_PEI 0x3080
@@ -78,6 +79,18 @@ EFI_PLATFORM_FIRMWARE_BLOB *mMeasuredChildFvInfo;
UINT32 mMeasuredMaxChildFvIndex = 0;
UINT32 mMeasuredChildFvIndex = 0;
+#pragma pack (1)
+
+#define FV_HANDOFF_TABLE_DESC "Fv(XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX)"
+typedef struct {
+ UINT8 BlobDescriptionSize;
+ UINT8 BlobDescription[sizeof(FV_HANDOFF_TABLE_DESC)];
+ EFI_PHYSICAL_ADDRESS BlobBase;
+ UINT64 BlobLength;
+} FV_HANDOFF_TABLE_POINTERS2;
+
+#pragma pack ()
+
/**
Measure and record the Firmware Volume Information once FvInfoPPI install.
@@ -447,6 +460,48 @@ MeasureCRTMVersion (
);
}
+/*
+ Get the FvName from the FV header.
+
+ Causion: The FV is untrusted input.
+
+ @param[in] FvBase Base address of FV image.
+ @param[in] FvLength Length of FV image.
+
+ @return FvName pointer
+ @retval NULL FvName is NOT found
+*/
+VOID *
+GetFvName (
+ IN EFI_PHYSICAL_ADDRESS FvBase,
+ IN UINT64 FvLength
+ )
+{
+ EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
+ EFI_FIRMWARE_VOLUME_EXT_HEADER *FvExtHeader;
+
+ if (FvBase >= MAX_ADDRESS) {
+ return NULL;
+ }
+ if (FvLength >= MAX_ADDRESS - FvBase) {
+ return NULL;
+ }
+ if (FvLength < sizeof(EFI_FIRMWARE_VOLUME_HEADER)) {
+ return NULL;
+ }
+
+ FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)FvBase;
+ if (FvHeader->ExtHeaderOffset < sizeof(EFI_FIRMWARE_VOLUME_HEADER)) {
+ return NULL;
+ }
+ if (FvHeader->ExtHeaderOffset + sizeof(EFI_FIRMWARE_VOLUME_EXT_HEADER) > FvLength) {
+ return NULL;
+ }
+ FvExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *)(UINTN)(FvBase + FvHeader->ExtHeaderOffset);
+
+ return &FvExtHeader->FvName;
+}
+
/**
Measure FV image.
Add it into the measured FV list after the FV is measured successfully.
@@ -469,6 +524,9 @@ MeasureFvImage (
UINT32 Index;
EFI_STATUS Status;
EFI_PLATFORM_FIRMWARE_BLOB FvBlob;
+ FV_HANDOFF_TABLE_POINTERS2 FvBlob2;
+ VOID *EventData;
+ VOID *FvName;
TCG_PCR_EVENT_HDR TcgEventHdr;
UINT32 Instance;
UINT32 Tpm2HashMask;
@@ -571,6 +629,21 @@ MeasureFvImage (
TcgEventHdr.PCRIndex = 0;
TcgEventHdr.EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB;
TcgEventHdr.EventSize = sizeof (FvBlob);
+ EventData = &FvBlob;
+
+ if (PcdGet32(PcdTcgPfpMeasurementRevision) >= TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105) {
+ FvBlob2.BlobDescriptionSize = sizeof(FvBlob2.BlobDescription);
+ CopyMem (FvBlob2.BlobDescription, FV_HANDOFF_TABLE_DESC, sizeof(FvBlob2.BlobDescription));
+ FvName = GetFvName (FvBase, FvLength);
+ if (FvName != NULL) {
+ AsciiSPrint ((CHAR8 *)FvBlob2.BlobDescription, sizeof(FvBlob2.BlobDescription), "Fv(%g)", FvName);
+ }
+ FvBlob2.BlobBase = FvBlob.BlobBase;
+ FvBlob2.BlobLength = FvBlob.BlobLength;
+ TcgEventHdr.EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB2;
+ TcgEventHdr.EventSize = sizeof (FvBlob2);
+ EventData = &FvBlob2;
+ }
if (Tpm2HashMask == 0) {
//
@@ -583,9 +656,9 @@ MeasureFvImage (
);
if (!EFI_ERROR(Status)) {
- Status = LogHashEvent (&DigestList, &TcgEventHdr, (UINT8*) &FvBlob);
- DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by Tcg2Pei starts at: 0x%x\n", FvBlob.BlobBase));
- DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by Tcg2Pei has the size: 0x%x\n", FvBlob.BlobLength));
+ Status = LogHashEvent (&DigestList, &TcgEventHdr, EventData);
+ DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by Tcg2Pei starts at: 0x%x\n", FvBase));
+ DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by Tcg2Pei has the size: 0x%x\n", FvLength));
} else if (Status == EFI_DEVICE_ERROR) {
BuildGuidHob (&gTpmErrorHobGuid,0);
REPORT_STATUS_CODE (
@@ -599,13 +672,13 @@ MeasureFvImage (
//
Status = HashLogExtendEvent (
0,
- (UINT8*) (UINTN) FvBlob.BlobBase,
- (UINTN) FvBlob.BlobLength,
- &TcgEventHdr,
- (UINT8*) &FvBlob
+ (UINT8*) (UINTN) FvBase, // HashData
+ (UINTN) FvLength, // HashDataLen
+ &TcgEventHdr, // EventHdr
+ EventData // EventData
);
- DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei starts at: 0x%x\n", FvBlob.BlobBase));
- DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei has the size: 0x%x\n", FvBlob.BlobLength));
+ DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei starts at: 0x%x\n", FvBase));
+ DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei has the size: 0x%x\n", FvLength));
}
if (EFI_ERROR(Status)) {
diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
index 30f985b6ea..3d361e8859 100644
--- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
+++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
@@ -54,6 +54,7 @@
MemoryAllocationLib
ReportStatusCodeLib
ResetSystemLib
+ PrintLib
[Guids]
gTcgEventEntryHobGuid ## PRODUCES ## HOB
@@ -74,6 +75,7 @@
[Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision ## CONSUMES
gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid ## CONSUMES
gEfiSecurityPkgTokenSpaceGuid.PcdTpm2InitializationPolicy ## CONSUMES
gEfiSecurityPkgTokenSpaceGuid.PcdTpm2SelfTestPolicy ## SOMETIMES_CONSUMES
--
2.19.2.windows.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event support.
2019-12-31 6:44 [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event support Yao, Jiewen
` (5 preceding siblings ...)
2019-12-31 6:44 ` [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP " Yao, Jiewen
@ 2020-01-02 0:11 ` Liming Gao
2020-01-02 0:39 ` Yao, Jiewen
2020-01-06 6:11 ` Wang, Jian J
7 siblings, 1 reply; 21+ messages in thread
From: Liming Gao @ 2020-01-02 0:11 UTC (permalink / raw)
To: devel@edk2.groups.io, Yao, Jiewen; +Cc: Wang, Jian J, Zhang, Chao B
Jiewen:
Seemly, this is a new feature. Dose it need to catch 2020 Q1 stable tag? If yes, I will add it into the feature planning.
Thanks
Liming
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
Sent: 2019年12月31日 14:44
To: devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>
Subject: [edk2-devel] [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event support.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
This patch series adds TCG PFP rev 105 and 800-155 event support.
TCG published Platform Firmware Profile spec revision 105
(https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_PFP_r1p05_05_3feb20.pdf)
and Firmware Integrity Measurement spec
(https://trustedcomputinggroup.org/wp-content/uploads/TCG_PC_Client-FIM_v1r24_3feb20.pdf)
2 major impact to the BIOS:
1) Need add 800-155 event at the beginning of the TCG2 event log.
2) Need use EV_EFI_PLATFORM_FIRMWARE_BLOB2 to replace EV_EFI_PLATFORM_FIRMWARE_BLOB and use EV_EFI_HANDOFF_TABLES2 to replace EV_EFI_HANDOFF_TABLES.
TCG2 DXE update: Add 800-155 event handling.
TCG2 PEI update: Use EV_EFI_PLATFORM_FIRMWARE_BLOB2.
Smbios Update: Use EV_EFI_HANDOFF_TABLES2.
This patch series is tested on EmulatorPkg with TPM2 simulator.
(https://github.com/jyao1/edk2/tree/feature_tpm_emulator).
The new entries - 800-155 event, EV_EFI_PLATFORM_FIRMWARE_BLOB2, EV_EFI_HANDOFF_TABLES2, can be dumpped with UEFI Tcg2DumpLog tool.
(https://github.com/jyao1/EdkiiShellTool/tree/master/EdkiiShellToolPkg/Tcg2DumpLog)
NOTE: To support the full 800-155 requirement in TCG, a platform need report the platform specific 800-155 event.
If the platform chooses to report in PEI, it can refer to the example at https://github.com/jyao1/edk2/tree/feature_tpm_emulator/EmulatorPkg/Tpm2/Platform800155EventPei.
If the platform chooses to report in DXE, it can refer to the example at https://github.com/jyao1/edk2/tree/feature_tpm_emulator/EmulatorPkg/Tpm2/Platform800155EventDxe.
Multiple 800-155 events are support.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
Jiewen Yao (6):
SecurityPkg/Guid: Add TCG 800-155 event GUID definition.
SecurityPkg/Tcg2Dxe: Add Tcg2Dxe to support 800-155 event.
MdeModulePkg/Smbios: Done measure Smbios multiple times.
MdeModulePkg/dec: add PcdTcgPfpMeasurementRevision PCD
MdeModulePkg/Smbios: Add TCG PFP rev 105 support.
SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
MdeModulePkg/MdeModulePkg.dec | 8 +
.../SmbiosMeasurementDxe.c | 39 ++++-
.../SmbiosMeasurementDxe.inf | 3 +
SecurityPkg/Include/Guid/TcgEventHob.h | 11 ++
SecurityPkg/SecurityPkg.dec | 4 +
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 157 ++++++++++++++----
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf | 1 +
SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 91 +++++++++-
SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 2 +
9 files changed, 273 insertions(+), 43 deletions(-)
--
2.19.2.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event support.
2020-01-02 0:11 ` [edk2-devel] [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event support Liming Gao
@ 2020-01-02 0:39 ` Yao, Jiewen
0 siblings, 0 replies; 21+ messages in thread
From: Yao, Jiewen @ 2020-01-02 0:39 UTC (permalink / raw)
To: Gao, Liming, devel@edk2.groups.io; +Cc: Wang, Jian J, Zhang, Chao B
Yes. I add it to https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning.
Thank you
Yao Jiewen
> -----Original Message-----
> From: Gao, Liming <liming.gao@intel.com>
> Sent: Thursday, January 2, 2020 8:12 AM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Zhang, Chao B
> <chao.b.zhang@intel.com>
> Subject: RE: [edk2-devel] [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155
> event support.
>
> Jiewen:
> Seemly, this is a new feature. Dose it need to catch 2020 Q1 stable tag? If yes, I
> will add it into the feature planning.
>
> Thanks
> Liming
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
> Sent: 2019年12月31日 14:44
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Zhang, Chao B
> <chao.b.zhang@intel.com>
> Subject: [edk2-devel] [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event
> support.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
>
> This patch series adds TCG PFP rev 105 and 800-155 event support.
>
> TCG published Platform Firmware Profile spec revision 105
> (https://trustedcomputinggroup.org/wp-
> content/uploads/TCG_PCClient_PFP_r1p05_05_3feb20.pdf)
> and Firmware Integrity Measurement spec
> (https://trustedcomputinggroup.org/wp-content/uploads/TCG_PC_Client-
> FIM_v1r24_3feb20.pdf)
>
> 2 major impact to the BIOS:
>
> 1) Need add 800-155 event at the beginning of the TCG2 event log.
>
> 2) Need use EV_EFI_PLATFORM_FIRMWARE_BLOB2 to replace
> EV_EFI_PLATFORM_FIRMWARE_BLOB and use EV_EFI_HANDOFF_TABLES2 to
> replace EV_EFI_HANDOFF_TABLES.
>
> TCG2 DXE update: Add 800-155 event handling.
> TCG2 PEI update: Use EV_EFI_PLATFORM_FIRMWARE_BLOB2.
> Smbios Update: Use EV_EFI_HANDOFF_TABLES2.
>
> This patch series is tested on EmulatorPkg with TPM2 simulator.
> (https://github.com/jyao1/edk2/tree/feature_tpm_emulator).
>
> The new entries - 800-155 event, EV_EFI_PLATFORM_FIRMWARE_BLOB2,
> EV_EFI_HANDOFF_TABLES2, can be dumpped with UEFI Tcg2DumpLog tool.
> (https://github.com/jyao1/EdkiiShellTool/tree/master/EdkiiShellToolPkg/Tcg2D
> umpLog)
>
> NOTE: To support the full 800-155 requirement in TCG, a platform need report
> the platform specific 800-155 event.
>
> If the platform chooses to report in PEI, it can refer to the example at
> https://github.com/jyao1/edk2/tree/feature_tpm_emulator/EmulatorPkg/Tpm2
> /Platform800155EventPei.
>
> If the platform chooses to report in DXE, it can refer to the example at
> https://github.com/jyao1/edk2/tree/feature_tpm_emulator/EmulatorPkg/Tpm2
> /Platform800155EventDxe.
>
> Multiple 800-155 events are support.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
>
>
>
> Jiewen Yao (6):
> SecurityPkg/Guid: Add TCG 800-155 event GUID definition.
> SecurityPkg/Tcg2Dxe: Add Tcg2Dxe to support 800-155 event.
> MdeModulePkg/Smbios: Done measure Smbios multiple times.
> MdeModulePkg/dec: add PcdTcgPfpMeasurementRevision PCD
> MdeModulePkg/Smbios: Add TCG PFP rev 105 support.
> SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
>
> MdeModulePkg/MdeModulePkg.dec | 8 +
> .../SmbiosMeasurementDxe.c | 39 ++++-
> .../SmbiosMeasurementDxe.inf | 3 +
> SecurityPkg/Include/Guid/TcgEventHob.h | 11 ++
> SecurityPkg/SecurityPkg.dec | 4 +
> SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 157 ++++++++++++++----
> SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf | 1 +
> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 91 +++++++++-
> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 2 +
> 9 files changed, 273 insertions(+), 43 deletions(-)
>
> --
> 2.19.2.windows.1
>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/6] MdeModulePkg/Smbios: Done measure Smbios multiple times.
2019-12-31 6:44 ` [PATCH 3/6] MdeModulePkg/Smbios: Done measure Smbios multiple times Yao, Jiewen
@ 2020-01-02 11:01 ` Zeng, Star
0 siblings, 0 replies; 21+ messages in thread
From: Zeng, Star @ 2020-01-02 11:01 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io
Cc: Wang, Jian J, Wu, Hao A, Bi, Dandan, Zeng, Star
Reviewed-by: Star Zeng <star.zeng@intel.com>
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Tuesday, December 31, 2019 2:44 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Bi, Dandan <dandan.bi@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [PATCH 3/6] MdeModulePkg/Smbios: Done measure Smbios
> multiple times.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
>
> In current implementation, the SMBIOS table is measured multiple time in
> every readytoboot event.
>
> This causes Smbios Table record appears multiple time in the TCG event log
> and confuses people.
>
> This issue makes it hard to implement 800-155 reference measurement.
>
> This patch closes the event to make sure Smbios is measured only once.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
> .../Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c | 4 ++-
> -
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git
> a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasuremen
> tDxe.c
> b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasureme
> ntDxe.c
> index 7b5d473146..5ec2aca095 100644
> ---
> a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasuremen
> tDxe.c
> +++
> b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasureme
> ntDxe.c
> @@ -577,8 +577,8 @@ MeasureSmbiosTable (
> TableAddress, // HashData
> TableLength // HashDataLen
> );
> - if (EFI_ERROR (Status)) {
> - return ;
> + if (!EFI_ERROR (Status)) {
> + gBS->CloseEvent (Event) ;
> }
> }
>
> --
> 2.19.2.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/6] MdeModulePkg/Smbios: Add TCG PFP rev 105 support.
2019-12-31 6:44 ` [PATCH 5/6] MdeModulePkg/Smbios: Add TCG PFP rev 105 support Yao, Jiewen
@ 2020-01-02 11:09 ` Zeng, Star
2020-01-02 14:16 ` Yao, Jiewen
0 siblings, 1 reply; 21+ messages in thread
From: Zeng, Star @ 2020-01-02 11:09 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io
Cc: Wang, Jian J, Wu, Hao A, Bi, Dandan, Zeng, Star
Minor comments.
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Tuesday, December 31, 2019 2:44 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Bi, Dandan <dandan.bi@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [PATCH 5/6] MdeModulePkg/Smbios: Add TCG PFP rev 105 support.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
>
> Report EV_EFI_HANDOFF_TABLES2 if the platform chooses PFP >= 105.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
> .../SmbiosMeasurementDxe.c | 35 +++++++++++++++++--
> .../SmbiosMeasurementDxe.inf | 3 ++
> 2 files changed, 35 insertions(+), 3 deletions(-)
>
> diff --git
> a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasuremen
> tDxe.c
> b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasureme
> ntDxe.c
> index 5ec2aca095..a5839c09f1 100644
> ---
> a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasuremen
> tDxe.c
> +++
> b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasureme
> ntDxe.c
> @@ -108,6 +108,18 @@ SMBIOS_FILTER_STRUCT
> mSmbiosFilterStandardTableBlackList[] = { EFI_SMBIOS_PROTOCOL
> *mSmbios;
> UINTN mMaxLen;
>
> +#pragma pack (1)
> +
> +#define SMBIOS_HANDOFF_TABLE_DESC "SmbiosTable"
> +typedef struct {
> + UINT8 TableDescriptionSize;
> + UINT8
> TableDescription[sizeof(SMBIOS_HANDOFF_TABLE_DESC)];
> + UINT64 NumberOfTables;
> + EFI_CONFIGURATION_TABLE TableEntry[1];
> +} SMBIOS_HANDOFF_TABLE_POINTERS2;
Curious about that there is no standard structure defined in public header instead of defining it internal here?
> +
> +#pragma pack ()
> +
> /**
>
> This function dump raw data.
> @@ -460,6 +472,10 @@ MeasureSmbiosTable ( {
> EFI_STATUS Status;
> EFI_HANDOFF_TABLE_POINTERS HandoffTables;
> + SMBIOS_HANDOFF_TABLE_POINTERS2 SmbiosHandoffTables2;
> + UINT32 EventType;
> + VOID *EventLog;
> + UINT32 EventLogSize;
> SMBIOS_TABLE_ENTRY_POINT *SmbiosTable;
> SMBIOS_TABLE_3_0_ENTRY_POINT *Smbios3Table;
> VOID *SmbiosTableAddress;
> @@ -569,11 +585,24 @@ MeasureSmbiosTable (
> CopyGuid (&(HandoffTables.TableEntry[0].VendorGuid),
> &gEfiSmbiosTableGuid);
> HandoffTables.TableEntry[0].VendorTable = SmbiosTable;
> }
> + EventType = EV_EFI_HANDOFF_TABLES;
> + EventLog = &HandoffTables;
> + EventLogSize = sizeof (HandoffTables);
How about making them into the else condition in the if condition below?
> +
> + if (PcdGet32(PcdTcgPfpMeasurementRevision) >=
> TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105) {
> + SmbiosHandoffTables2.TableDescriptionSize =
> sizeof(SmbiosHandoffTables2.TableDescription);
> + CopyMem (SmbiosHandoffTables2.TableDescription,
> SMBIOS_HANDOFF_TABLE_DESC,
> sizeof(SmbiosHandoffTables2.TableDescription));
> + SmbiosHandoffTables2.NumberOfTables =
> HandoffTables.NumberOfTables;
> + CopyMem (&(SmbiosHandoffTables2.TableEntry[0]),
> &(HandoffTables.TableEntry[0]),
> sizeof(SmbiosHandoffTables2.TableEntry[0]));
> + EventType = EV_EFI_HANDOFF_TABLES2;
> + EventLog = &SmbiosHandoffTables2;
> + EventLogSize = sizeof (SmbiosHandoffTables2);
> + }
> Status = TpmMeasureAndLogData (
> 1, // PCRIndex
> - EV_EFI_HANDOFF_TABLES, // EventType
> - &HandoffTables, // EventLog
> - sizeof (HandoffTables), // LogLen
> + EventType, // EventType
> + EventLog, // EventLog
> + EventLogSize, // LogLen
> TableAddress, // HashData
> TableLength // HashDataLen
> );
> diff --git
> a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasuremen
> tDxe.inf
> b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasureme
> ntDxe.inf
> index a074044c84..81d3655dc7 100644
> ---
> a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasuremen
> tDxe.inf
> +++
> b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasureme
> ntDxe.i
> +++ nf
> @@ -57,6 +57,9 @@
> gEfiSmbiosTableGuid ## SOMETIMES_CONSUMES ##
> SystemTable
> gEfiSmbios3TableGuid ## SOMETIMES_CONSUMES ##
> SystemTable
>
> +[Pcd]
> + gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision
> ## CONSUMES
To be explicit, how about adding PcdLib in inf and PcdLib.h in c?
Thanks,
Star
> +
> [Depex]
> gEfiSmbiosProtocolGuid
>
> --
> 2.19.2.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/6] MdeModulePkg/Smbios: Add TCG PFP rev 105 support.
2020-01-02 11:09 ` Zeng, Star
@ 2020-01-02 14:16 ` Yao, Jiewen
2020-01-03 0:54 ` Zeng, Star
0 siblings, 1 reply; 21+ messages in thread
From: Yao, Jiewen @ 2020-01-02 14:16 UTC (permalink / raw)
To: Zeng, Star, devel@edk2.groups.io; +Cc: Wang, Jian J, Wu, Hao A, Bi, Dandan
Below:
> -----Original Message-----
> From: Zeng, Star <star.zeng@intel.com>
> Sent: Thursday, January 2, 2020 7:09 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Bi, Dandan <dandan.bi@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [PATCH 5/6] MdeModulePkg/Smbios: Add TCG PFP rev 105 support.
>
> Minor comments.
>
> > -----Original Message-----
> > From: Yao, Jiewen
> > Sent: Tuesday, December 31, 2019 2:44 PM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> > Bi, Dandan <dandan.bi@intel.com>; Zeng, Star <star.zeng@intel.com>
> > Subject: [PATCH 5/6] MdeModulePkg/Smbios: Add TCG PFP rev 105 support.
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
> >
> > Report EV_EFI_HANDOFF_TABLES2 if the platform chooses PFP >= 105.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Dandan Bi <dandan.bi@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> > ---
> > .../SmbiosMeasurementDxe.c | 35 +++++++++++++++++--
> > .../SmbiosMeasurementDxe.inf | 3 ++
> > 2 files changed, 35 insertions(+), 3 deletions(-)
> >
> > diff --git
> > a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasuremen
> > tDxe.c
> > b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasureme
> > ntDxe.c
> > index 5ec2aca095..a5839c09f1 100644
> > ---
> > a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasuremen
> > tDxe.c
> > +++
> > b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasureme
> > ntDxe.c
> > @@ -108,6 +108,18 @@ SMBIOS_FILTER_STRUCT
> > mSmbiosFilterStandardTableBlackList[] = { EFI_SMBIOS_PROTOCOL
> > *mSmbios;
> > UINTN mMaxLen;
> >
> > +#pragma pack (1)
> > +
> > +#define SMBIOS_HANDOFF_TABLE_DESC "SmbiosTable"
> > +typedef struct {
> > + UINT8 TableDescriptionSize;
> > + UINT8
> > TableDescription[sizeof(SMBIOS_HANDOFF_TABLE_DESC)];
> > + UINT64 NumberOfTables;
> > + EFI_CONFIGURATION_TABLE TableEntry[1];
> > +} SMBIOS_HANDOFF_TABLE_POINTERS2;
>
> Curious about that there is no standard structure defined in public header
> instead of defining it internal here?
[Jiewen] Right. The first byte is the size of the following description.
The table owner may define different description for the table.
>
> > +
> > +#pragma pack ()
> > +
> > /**
> >
> > This function dump raw data.
> > @@ -460,6 +472,10 @@ MeasureSmbiosTable ( {
> > EFI_STATUS Status;
> > EFI_HANDOFF_TABLE_POINTERS HandoffTables;
> > + SMBIOS_HANDOFF_TABLE_POINTERS2 SmbiosHandoffTables2;
> > + UINT32 EventType;
> > + VOID *EventLog;
> > + UINT32 EventLogSize;
> > SMBIOS_TABLE_ENTRY_POINT *SmbiosTable;
> > SMBIOS_TABLE_3_0_ENTRY_POINT *Smbios3Table;
> > VOID *SmbiosTableAddress;
> > @@ -569,11 +585,24 @@ MeasureSmbiosTable (
> > CopyGuid (&(HandoffTables.TableEntry[0].VendorGuid),
> > &gEfiSmbiosTableGuid);
> > HandoffTables.TableEntry[0].VendorTable = SmbiosTable;
> > }
> > + EventType = EV_EFI_HANDOFF_TABLES;
> > + EventLog = &HandoffTables;
> > + EventLogSize = sizeof (HandoffTables);
>
> How about making them into the else condition in the if condition below?
[Jiewen] My purpose is that all the rev 105 content are scoped in following if.
For rev 0, people can just see the above logic.
It is more straight forward than putting partial of logic into else.
>
> > +
> > + if (PcdGet32(PcdTcgPfpMeasurementRevision) >=
> > TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105) {
> > + SmbiosHandoffTables2.TableDescriptionSize =
> > sizeof(SmbiosHandoffTables2.TableDescription);
> > + CopyMem (SmbiosHandoffTables2.TableDescription,
> > SMBIOS_HANDOFF_TABLE_DESC,
> > sizeof(SmbiosHandoffTables2.TableDescription));
> > + SmbiosHandoffTables2.NumberOfTables =
> > HandoffTables.NumberOfTables;
> > + CopyMem (&(SmbiosHandoffTables2.TableEntry[0]),
> > &(HandoffTables.TableEntry[0]),
> > sizeof(SmbiosHandoffTables2.TableEntry[0]));
> > + EventType = EV_EFI_HANDOFF_TABLES2;
> > + EventLog = &SmbiosHandoffTables2;
> > + EventLogSize = sizeof (SmbiosHandoffTables2);
> > + }
> > Status = TpmMeasureAndLogData (
> > 1, // PCRIndex
> > - EV_EFI_HANDOFF_TABLES, // EventType
> > - &HandoffTables, // EventLog
> > - sizeof (HandoffTables), // LogLen
> > + EventType, // EventType
> > + EventLog, // EventLog
> > + EventLogSize, // LogLen
> > TableAddress, // HashData
> > TableLength // HashDataLen
> > );
> > diff --git
> > a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasuremen
> > tDxe.inf
> > b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasureme
> > ntDxe.inf
> > index a074044c84..81d3655dc7 100644
> > ---
> > a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasuremen
> > tDxe.inf
> > +++
> > b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasureme
> > ntDxe.i
> > +++ nf
> > @@ -57,6 +57,9 @@
> > gEfiSmbiosTableGuid ## SOMETIMES_CONSUMES ##
> > SystemTable
> > gEfiSmbios3TableGuid ## SOMETIMES_CONSUMES ##
> > SystemTable
> >
> > +[Pcd]
> > + gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision
> > ## CONSUMES
>
> To be explicit, how about adding PcdLib in inf and PcdLib.h in c?
[Jiewen] Agree. I will add them.
>
>
> Thanks,
> Star
>
> > +
> > [Depex]
> > gEfiSmbiosProtocolGuid
> >
> > --
> > 2.19.2.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/6] MdeModulePkg/Smbios: Add TCG PFP rev 105 support.
2020-01-02 14:16 ` Yao, Jiewen
@ 2020-01-03 0:54 ` Zeng, Star
0 siblings, 0 replies; 21+ messages in thread
From: Zeng, Star @ 2020-01-03 0:54 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io
Cc: Wang, Jian J, Wu, Hao A, Bi, Dandan, Zeng, Star
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Thursday, January 2, 2020 10:16 PM
> To: Zeng, Star <star.zeng@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Bi, Dandan <dandan.bi@intel.com>
> Subject: RE: [PATCH 5/6] MdeModulePkg/Smbios: Add TCG PFP rev 105
> support.
>
> Below:
>
> > -----Original Message-----
> > From: Zeng, Star <star.zeng@intel.com>
> > Sent: Thursday, January 2, 2020 7:09 PM
> > To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > <hao.a.wu@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Zeng, Star
> > <star.zeng@intel.com>
> > Subject: RE: [PATCH 5/6] MdeModulePkg/Smbios: Add TCG PFP rev 105
> support.
> >
> > Minor comments.
> >
> > > -----Original Message-----
> > > From: Yao, Jiewen
> > > Sent: Tuesday, December 31, 2019 2:44 PM
> > > To: devel@edk2.groups.io
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > > <hao.a.wu@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Zeng, Star
> > > <star.zeng@intel.com>
> > > Subject: [PATCH 5/6] MdeModulePkg/Smbios: Add TCG PFP rev 105
> support.
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
> > >
> > > Report EV_EFI_HANDOFF_TABLES2 if the platform chooses PFP >= 105.
> > >
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > Cc: Dandan Bi <dandan.bi@intel.com>
> > > Cc: Star Zeng <star.zeng@intel.com>
> > > Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> > > ---
> > > .../SmbiosMeasurementDxe.c | 35 +++++++++++++++++--
> > > .../SmbiosMeasurementDxe.inf | 3 ++
> > > 2 files changed, 35 insertions(+), 3 deletions(-)
> > >
> > > diff --git
> > >
> a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasuremen
> > > tDxe.c
> > >
> b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasureme
> > > ntDxe.c
> > > index 5ec2aca095..a5839c09f1 100644
> > > ---
> > >
> a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasuremen
> > > tDxe.c
> > > +++
> > >
> b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasureme
> > > ntDxe.c
> > > @@ -108,6 +108,18 @@ SMBIOS_FILTER_STRUCT
> > > mSmbiosFilterStandardTableBlackList[] = { EFI_SMBIOS_PROTOCOL
> > > *mSmbios;
> > > UINTN mMaxLen;
> > >
> > > +#pragma pack (1)
> > > +
> > > +#define SMBIOS_HANDOFF_TABLE_DESC "SmbiosTable"
> > > +typedef struct {
> > > + UINT8 TableDescriptionSize;
> > > + UINT8
> > > TableDescription[sizeof(SMBIOS_HANDOFF_TABLE_DESC)];
> > > + UINT64 NumberOfTables;
> > > + EFI_CONFIGURATION_TABLE TableEntry[1];
> > > +} SMBIOS_HANDOFF_TABLE_POINTERS2;
> >
> > Curious about that there is no standard structure defined in public
> > header instead of defining it internal here?
> [Jiewen] Right. The first byte is the size of the following description.
> The table owner may define different description for the table.
>
> >
> > > +
> > > +#pragma pack ()
> > > +
> > > /**
> > >
> > > This function dump raw data.
> > > @@ -460,6 +472,10 @@ MeasureSmbiosTable ( {
> > > EFI_STATUS Status;
> > > EFI_HANDOFF_TABLE_POINTERS HandoffTables;
> > > + SMBIOS_HANDOFF_TABLE_POINTERS2 SmbiosHandoffTables2;
> > > + UINT32 EventType;
> > > + VOID *EventLog;
> > > + UINT32 EventLogSize;
> > > SMBIOS_TABLE_ENTRY_POINT *SmbiosTable;
> > > SMBIOS_TABLE_3_0_ENTRY_POINT *Smbios3Table;
> > > VOID *SmbiosTableAddress;
> > > @@ -569,11 +585,24 @@ MeasureSmbiosTable (
> > > CopyGuid (&(HandoffTables.TableEntry[0].VendorGuid),
> > > &gEfiSmbiosTableGuid);
> > > HandoffTables.TableEntry[0].VendorTable = SmbiosTable;
> > > }
> > > + EventType = EV_EFI_HANDOFF_TABLES;
> > > + EventLog = &HandoffTables;
> > > + EventLogSize = sizeof (HandoffTables);
> >
> > How about making them into the else condition in the if condition below?
> [Jiewen] My purpose is that all the rev 105 content are scoped in following if.
> For rev 0, people can just see the above logic.
> It is more straight forward than putting partial of logic into else.
>
> >
> > > +
> > > + if (PcdGet32(PcdTcgPfpMeasurementRevision) >=
> > > TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105) {
> > > + SmbiosHandoffTables2.TableDescriptionSize =
> > > sizeof(SmbiosHandoffTables2.TableDescription);
> > > + CopyMem (SmbiosHandoffTables2.TableDescription,
> > > SMBIOS_HANDOFF_TABLE_DESC,
> > > sizeof(SmbiosHandoffTables2.TableDescription));
> > > + SmbiosHandoffTables2.NumberOfTables =
> > > HandoffTables.NumberOfTables;
> > > + CopyMem (&(SmbiosHandoffTables2.TableEntry[0]),
> > > &(HandoffTables.TableEntry[0]),
> > > sizeof(SmbiosHandoffTables2.TableEntry[0]));
> > > + EventType = EV_EFI_HANDOFF_TABLES2;
> > > + EventLog = &SmbiosHandoffTables2;
> > > + EventLogSize = sizeof (SmbiosHandoffTables2);
> > > + }
> > > Status = TpmMeasureAndLogData (
> > > 1, // PCRIndex
> > > - EV_EFI_HANDOFF_TABLES, // EventType
> > > - &HandoffTables, // EventLog
> > > - sizeof (HandoffTables), // LogLen
> > > + EventType, // EventType
> > > + EventLog, // EventLog
> > > + EventLogSize, // LogLen
> > > TableAddress, // HashData
> > > TableLength // HashDataLen
> > > );
> > > diff --git
> > >
> a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasuremen
> > > tDxe.inf
> > >
> b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasureme
> > > ntDxe.inf
> > > index a074044c84..81d3655dc7 100644
> > > ---
> > >
> a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasuremen
> > > tDxe.inf
> > > +++
> > >
> b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasureme
> > > ntDxe.i
> > > +++ nf
> > > @@ -57,6 +57,9 @@
> > > gEfiSmbiosTableGuid ## SOMETIMES_CONSUMES ##
> > > SystemTable
> > > gEfiSmbios3TableGuid ## SOMETIMES_CONSUMES ##
> > > SystemTable
> > >
> > > +[Pcd]
> > > + gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision
> > > ## CONSUMES
> >
> > To be explicit, how about adding PcdLib in inf and PcdLib.h in c?
> [Jiewen] Agree. I will add them.
Reviewed-by: Star Zeng <star.zeng@intel.com>
No need send new patch series just for this, you can do them when doing pull-request.
Star
>
>
> >
> >
> > Thanks,
> > Star
> >
> > > +
> > > [Depex]
> > > gEfiSmbiosProtocolGuid
> > >
> > > --
> > > 2.19.2.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/6] MdeModulePkg/dec: add PcdTcgPfpMeasurementRevision PCD
2019-12-31 6:44 ` [PATCH 4/6] MdeModulePkg/dec: add PcdTcgPfpMeasurementRevision PCD Yao, Jiewen
@ 2020-01-06 3:13 ` Wang, Jian J
0 siblings, 0 replies; 21+ messages in thread
From: Wang, Jian J @ 2020-01-06 3:13 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io; +Cc: Wu, Hao A, Zhang, Chao B
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Regards,
Jian
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Tuesday, December 31, 2019 2:44 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Zhang, Chao B <chao.b.zhang@intel.com>
> Subject: [PATCH 4/6] MdeModulePkg/dec: add PcdTcgPfpMeasurementRevision
> PCD
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
>
> This PCD is to control the TCG PFP spec revision.
>
> The PFP 105 added new event type to support NIST SP800-155,
> and deprecated old event type.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
> MdeModulePkg/MdeModulePkg.dec | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec
> index 41b9e70a1a..f75a74af25 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -2003,6 +2003,14 @@
> # @Prompt Capsule On Disk relocation device path.
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdCodRelocationDevPath|{0xFF}|VOID*|
> 0x0000002f
>
> + ## Indicates which TCG Platform Firmware Profile revision the EDKII firmware
> follows.
> + # The revision number is defined in
> MdePkg/Include/IndustryStandard/UefiTcgPlatform.h
> + # 0: This is for compatiblity support.
> + # 105: This is the first revision to support 800-155 is related event, such as
> + # EV_EFI_PLATFORM_FIRMWARE_BLOB2 and EV_EFI_HANDOFF_TABLES2.
> + # @Prompt TCG Platform Firmware Profile revision.
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision|0|UINT3
> 2|0x00010077
> +
> [PcdsPatchableInModule]
> ## Specify memory size with page number for PEI code when
> # Loading Module at Fixed Address feature is enabled.
> --
> 2.19.2.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/6] SecurityPkg/Guid: Add TCG 800-155 event GUID definition.
2019-12-31 6:44 ` [PATCH 1/6] SecurityPkg/Guid: Add TCG 800-155 event GUID definition Yao, Jiewen
@ 2020-01-06 3:22 ` Wang, Jian J
0 siblings, 0 replies; 21+ messages in thread
From: Wang, Jian J @ 2020-01-06 3:22 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io; +Cc: Zhang, Chao B
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Regards,
Jian
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Tuesday, December 31, 2019 2:44 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Zhang, Chao B
> <chao.b.zhang@intel.com>
> Subject: [PATCH 1/6] SecurityPkg/Guid: Add TCG 800-155 event GUID definition.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
>
> The PEIM can produce the 800-155 event and the event
> will be recorded to TCG event log by the TCG2 DXE.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
> SecurityPkg/Include/Guid/TcgEventHob.h | 11 +++++++++++
> SecurityPkg/SecurityPkg.dec | 4 ++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/SecurityPkg/Include/Guid/TcgEventHob.h
> b/SecurityPkg/Include/Guid/TcgEventHob.h
> index eef3f92abd..97e40b47d0 100644
> --- a/SecurityPkg/Include/Guid/TcgEventHob.h
> +++ b/SecurityPkg/Include/Guid/TcgEventHob.h
> @@ -49,4 +49,15 @@ extern EFI_GUID gTpmErrorHobGuid;
>
> extern EFI_GUID gTpm2StartupLocalityHobGuid;
>
> +///
> +/// The Global ID of a GUIDed HOB used to record TCG 800-155 PlatformId
> Event.
> +/// HOB payload is the whole TCG_Sp800_155_PlatformId_Event2 according to
> TCG 800-155 PlatformId Event.
> +///
> +#define EFI_TCG_800_155_PLATFORM_ID_EVENT_HOB_GUID \
> + { \
> + 0xe2c3bc69, 0x615c, 0x4b5b, { 0x8e, 0x5c, 0xa0, 0x33, 0xa9, 0xc2, 0x5e,
> 0xd6 } \
> + }
> +
> +extern EFI_GUID gTcg800155PlatformIdEventHobGuid;
> +
> #endif
> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> index cac36caf0a..5335cc5397 100644
> --- a/SecurityPkg/SecurityPkg.dec
> +++ b/SecurityPkg/SecurityPkg.dec
> @@ -121,6 +121,10 @@
> ## Include/Guid/TcgEventHob.h
> gTpm2StartupLocalityHobGuid = { 0x397b0c9, 0x22e8, 0x459e, { 0xa4, 0xff,
> 0x99, 0xbc, 0x65, 0x27, 0x9, 0x29 }}
>
> + ## HOB GUID used to record TCG 800-155 PlatformId Event
> + ## Include/Guid/TcgEventHob.h
> + gTcg800155PlatformIdEventHobGuid = { 0xe2c3bc69, 0x615c, 0x4b5b, { 0x8e,
> 0x5c, 0xa0, 0x33, 0xa9, 0xc2, 0x5e, 0xd6 }}
> +
> ## HOB GUID used to pass all PEI measured FV info to DXE Driver.
> # Include/Guid/MeasuredFvHob.h
> gMeasuredFvHobGuid = { 0xb2360b42, 0x7173, 0x420a, { 0x86, 0x96,
> 0x46, 0xca, 0x6b, 0xab, 0x10, 0x60 }}
> --
> 2.19.2.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
2019-12-31 6:44 ` [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP " Yao, Jiewen
@ 2020-01-06 5:33 ` Wang, Jian J
2020-01-06 5:53 ` Yao, Jiewen
0 siblings, 1 reply; 21+ messages in thread
From: Wang, Jian J @ 2020-01-06 5:33 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io; +Cc: Zhang, Chao B
Jiewen,
Just one minor comment. With it addressed,
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Tuesday, December 31, 2019 2:44 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Zhang, Chao B
> <chao.b.zhang@intel.com>
> Subject: [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
>
> Use EV_EFI_PLATFORM_FIRMWARE_BLOB2 if the TCG PFP revision is >= 105.
> Use FvName as the description for the FV.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 91 ++++++++++++++++++++++++++---
> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 2 +
> 2 files changed, 84 insertions(+), 9 deletions(-)
>
> diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> index 1565d4e402..7d99c7906a 100644
> --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> @@ -37,6 +37,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Library/MemoryAllocationLib.h>
> #include <Library/ReportStatusCodeLib.h>
> #include <Library/ResetSystemLib.h>
> +#include <Library/PrintLib.h>
>
> #define PERF_ID_TCG2_PEI 0x3080
>
> @@ -78,6 +79,18 @@ EFI_PLATFORM_FIRMWARE_BLOB
> *mMeasuredChildFvInfo;
> UINT32 mMeasuredMaxChildFvIndex = 0;
> UINT32 mMeasuredChildFvIndex = 0;
>
> +#pragma pack (1)
> +
> +#define FV_HANDOFF_TABLE_DESC "Fv(XXXXXXXX-XXXX-XXXX-XXXX-
> XXXXXXXXXXXX)"
> +typedef struct {
> + UINT8 BlobDescriptionSize;
> + UINT8 BlobDescription[sizeof(FV_HANDOFF_TABLE_DESC)];
> + EFI_PHYSICAL_ADDRESS BlobBase;
> + UINT64 BlobLength;
> +} FV_HANDOFF_TABLE_POINTERS2;
> +
> +#pragma pack ()
> +
> /**
> Measure and record the Firmware Volume Information once FvInfoPPI install.
>
> @@ -447,6 +460,48 @@ MeasureCRTMVersion (
> );
> }
>
> +/*
> + Get the FvName from the FV header.
> +
> + Causion: The FV is untrusted input.
> +
> + @param[in] FvBase Base address of FV image.
> + @param[in] FvLength Length of FV image.
> +
> + @return FvName pointer
> + @retval NULL FvName is NOT found
> +*/
> +VOID *
> +GetFvName (
> + IN EFI_PHYSICAL_ADDRESS FvBase,
> + IN UINT64 FvLength
> + )
> +{
> + EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
> + EFI_FIRMWARE_VOLUME_EXT_HEADER *FvExtHeader;
> +
> + if (FvBase >= MAX_ADDRESS) {
> + return NULL;
> + }
> + if (FvLength >= MAX_ADDRESS - FvBase) {
> + return NULL;
> + }
> + if (FvLength < sizeof(EFI_FIRMWARE_VOLUME_HEADER)) {
> + return NULL;
> + }
> +
> + FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)FvBase;
> + if (FvHeader->ExtHeaderOffset < sizeof(EFI_FIRMWARE_VOLUME_HEADER)) {
> + return NULL;
> + }
> + if (FvHeader->ExtHeaderOffset +
> sizeof(EFI_FIRMWARE_VOLUME_EXT_HEADER) > FvLength) {
> + return NULL;
> + }
> + FvExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *)(UINTN)(FvBase +
> FvHeader->ExtHeaderOffset);
> +
> + return &FvExtHeader->FvName;
> +}
> +
> /**
> Measure FV image.
> Add it into the measured FV list after the FV is measured successfully.
> @@ -469,6 +524,9 @@ MeasureFvImage (
> UINT32 Index;
> EFI_STATUS Status;
> EFI_PLATFORM_FIRMWARE_BLOB FvBlob;
> + FV_HANDOFF_TABLE_POINTERS2 FvBlob2;
> + VOID *EventData;
> + VOID *FvName;
> TCG_PCR_EVENT_HDR TcgEventHdr;
> UINT32 Instance;
> UINT32 Tpm2HashMask;
> @@ -571,6 +629,21 @@ MeasureFvImage (
> TcgEventHdr.PCRIndex = 0;
> TcgEventHdr.EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB;
> TcgEventHdr.EventSize = sizeof (FvBlob);
> + EventData = &FvBlob;
> +
I'd suggest to put above code in 'else' block to make code easier to read,
i.e. FvBlob for revision less than 105 and FvBlob2 for later ones.
Regards,
Jian
> + if (PcdGet32(PcdTcgPfpMeasurementRevision) >=
> TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105) {
> + FvBlob2.BlobDescriptionSize = sizeof(FvBlob2.BlobDescription);
> + CopyMem (FvBlob2.BlobDescription, FV_HANDOFF_TABLE_DESC,
> sizeof(FvBlob2.BlobDescription));
> + FvName = GetFvName (FvBase, FvLength);
> + if (FvName != NULL) {
> + AsciiSPrint ((CHAR8 *)FvBlob2.BlobDescription,
> sizeof(FvBlob2.BlobDescription), "Fv(%g)", FvName);
> + }
> + FvBlob2.BlobBase = FvBlob.BlobBase;
> + FvBlob2.BlobLength = FvBlob.BlobLength;
> + TcgEventHdr.EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB2;
> + TcgEventHdr.EventSize = sizeof (FvBlob2);
> + EventData = &FvBlob2;
> + }
>
> if (Tpm2HashMask == 0) {
> //
> @@ -583,9 +656,9 @@ MeasureFvImage (
> );
>
> if (!EFI_ERROR(Status)) {
> - Status = LogHashEvent (&DigestList, &TcgEventHdr, (UINT8*) &FvBlob);
> - DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by
> Tcg2Pei starts at: 0x%x\n", FvBlob.BlobBase));
> - DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by
> Tcg2Pei has the size: 0x%x\n", FvBlob.BlobLength));
> + Status = LogHashEvent (&DigestList, &TcgEventHdr, EventData);
> + DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by
> Tcg2Pei starts at: 0x%x\n", FvBase));
> + DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by
> Tcg2Pei has the size: 0x%x\n", FvLength));
> } else if (Status == EFI_DEVICE_ERROR) {
> BuildGuidHob (&gTpmErrorHobGuid,0);
> REPORT_STATUS_CODE (
> @@ -599,13 +672,13 @@ MeasureFvImage (
> //
> Status = HashLogExtendEvent (
> 0,
> - (UINT8*) (UINTN) FvBlob.BlobBase,
> - (UINTN) FvBlob.BlobLength,
> - &TcgEventHdr,
> - (UINT8*) &FvBlob
> + (UINT8*) (UINTN) FvBase, // HashData
> + (UINTN) FvLength, // HashDataLen
> + &TcgEventHdr, // EventHdr
> + EventData // EventData
> );
> - DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei starts at:
> 0x%x\n", FvBlob.BlobBase));
> - DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei has the size:
> 0x%x\n", FvBlob.BlobLength));
> + DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei starts at:
> 0x%x\n", FvBase));
> + DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei has the size:
> 0x%x\n", FvLength));
> }
>
> if (EFI_ERROR(Status)) {
> diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> index 30f985b6ea..3d361e8859 100644
> --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> @@ -54,6 +54,7 @@
> MemoryAllocationLib
> ReportStatusCodeLib
> ResetSystemLib
> + PrintLib
>
> [Guids]
> gTcgEventEntryHobGuid ## PRODUCES ##
> HOB
> @@ -74,6 +75,7 @@
>
> [Pcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString ##
> SOMETIMES_CONSUMES
> + gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision ##
> CONSUMES
> gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid ##
> CONSUMES
> gEfiSecurityPkgTokenSpaceGuid.PcdTpm2InitializationPolicy ##
> CONSUMES
> gEfiSecurityPkgTokenSpaceGuid.PcdTpm2SelfTestPolicy ##
> SOMETIMES_CONSUMES
> --
> 2.19.2.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
2020-01-06 5:33 ` Wang, Jian J
@ 2020-01-06 5:53 ` Yao, Jiewen
2020-01-06 5:57 ` Wang, Jian J
0 siblings, 1 reply; 21+ messages in thread
From: Yao, Jiewen @ 2020-01-06 5:53 UTC (permalink / raw)
To: Wang, Jian J, devel@edk2.groups.io; +Cc: Zhang, Chao B
Hi Jian
I purposely put the line there, because I want to group all the rev 0 code together and rev 105 code together in if statement. I don't want to move that particular line to else statement.
Thank you
Yao Jiewen
> -----Original Message-----
> From: Wang, Jian J <jian.j.wang@intel.com>
> Sent: Monday, January 6, 2020 1:33 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: Zhang, Chao B <chao.b.zhang@intel.com>
> Subject: RE: [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
>
> Jiewen,
>
> Just one minor comment. With it addressed,
>
> Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
>
>
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen.yao@intel.com>
> > Sent: Tuesday, December 31, 2019 2:44 PM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Zhang, Chao B
> > <chao.b.zhang@intel.com>
> > Subject: [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
> >
> > Use EV_EFI_PLATFORM_FIRMWARE_BLOB2 if the TCG PFP revision is >= 105.
> > Use FvName as the description for the FV.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Chao Zhang <chao.b.zhang@intel.com>
> > Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> > ---
> > SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 91 ++++++++++++++++++++++++++---
> > SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 2 +
> > 2 files changed, 84 insertions(+), 9 deletions(-)
> >
> > diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> > b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> > index 1565d4e402..7d99c7906a 100644
> > --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> > +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> > @@ -37,6 +37,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > #include <Library/MemoryAllocationLib.h>
> > #include <Library/ReportStatusCodeLib.h>
> > #include <Library/ResetSystemLib.h>
> > +#include <Library/PrintLib.h>
> >
> > #define PERF_ID_TCG2_PEI 0x3080
> >
> > @@ -78,6 +79,18 @@ EFI_PLATFORM_FIRMWARE_BLOB
> > *mMeasuredChildFvInfo;
> > UINT32 mMeasuredMaxChildFvIndex = 0;
> > UINT32 mMeasuredChildFvIndex = 0;
> >
> > +#pragma pack (1)
> > +
> > +#define FV_HANDOFF_TABLE_DESC "Fv(XXXXXXXX-XXXX-XXXX-XXXX-
> > XXXXXXXXXXXX)"
> > +typedef struct {
> > + UINT8 BlobDescriptionSize;
> > + UINT8 BlobDescription[sizeof(FV_HANDOFF_TABLE_DESC)];
> > + EFI_PHYSICAL_ADDRESS BlobBase;
> > + UINT64 BlobLength;
> > +} FV_HANDOFF_TABLE_POINTERS2;
> > +
> > +#pragma pack ()
> > +
> > /**
> > Measure and record the Firmware Volume Information once FvInfoPPI install.
> >
> > @@ -447,6 +460,48 @@ MeasureCRTMVersion (
> > );
> > }
> >
> > +/*
> > + Get the FvName from the FV header.
> > +
> > + Causion: The FV is untrusted input.
> > +
> > + @param[in] FvBase Base address of FV image.
> > + @param[in] FvLength Length of FV image.
> > +
> > + @return FvName pointer
> > + @retval NULL FvName is NOT found
> > +*/
> > +VOID *
> > +GetFvName (
> > + IN EFI_PHYSICAL_ADDRESS FvBase,
> > + IN UINT64 FvLength
> > + )
> > +{
> > + EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
> > + EFI_FIRMWARE_VOLUME_EXT_HEADER *FvExtHeader;
> > +
> > + if (FvBase >= MAX_ADDRESS) {
> > + return NULL;
> > + }
> > + if (FvLength >= MAX_ADDRESS - FvBase) {
> > + return NULL;
> > + }
> > + if (FvLength < sizeof(EFI_FIRMWARE_VOLUME_HEADER)) {
> > + return NULL;
> > + }
> > +
> > + FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)FvBase;
> > + if (FvHeader->ExtHeaderOffset < sizeof(EFI_FIRMWARE_VOLUME_HEADER))
> {
> > + return NULL;
> > + }
> > + if (FvHeader->ExtHeaderOffset +
> > sizeof(EFI_FIRMWARE_VOLUME_EXT_HEADER) > FvLength) {
> > + return NULL;
> > + }
> > + FvExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *)(UINTN)(FvBase +
> > FvHeader->ExtHeaderOffset);
> > +
> > + return &FvExtHeader->FvName;
> > +}
> > +
> > /**
> > Measure FV image.
> > Add it into the measured FV list after the FV is measured successfully.
> > @@ -469,6 +524,9 @@ MeasureFvImage (
> > UINT32 Index;
> > EFI_STATUS Status;
> > EFI_PLATFORM_FIRMWARE_BLOB FvBlob;
> > + FV_HANDOFF_TABLE_POINTERS2 FvBlob2;
> > + VOID *EventData;
> > + VOID *FvName;
> > TCG_PCR_EVENT_HDR TcgEventHdr;
> > UINT32 Instance;
> > UINT32 Tpm2HashMask;
> > @@ -571,6 +629,21 @@ MeasureFvImage (
> > TcgEventHdr.PCRIndex = 0;
> > TcgEventHdr.EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB;
> > TcgEventHdr.EventSize = sizeof (FvBlob);
> > + EventData = &FvBlob;
> > +
>
> I'd suggest to put above code in 'else' block to make code easier to read,
> i.e. FvBlob for revision less than 105 and FvBlob2 for later ones.
>
> Regards,
> Jian
>
> > + if (PcdGet32(PcdTcgPfpMeasurementRevision) >=
> > TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105) {
> > + FvBlob2.BlobDescriptionSize = sizeof(FvBlob2.BlobDescription);
> > + CopyMem (FvBlob2.BlobDescription, FV_HANDOFF_TABLE_DESC,
> > sizeof(FvBlob2.BlobDescription));
> > + FvName = GetFvName (FvBase, FvLength);
> > + if (FvName != NULL) {
> > + AsciiSPrint ((CHAR8 *)FvBlob2.BlobDescription,
> > sizeof(FvBlob2.BlobDescription), "Fv(%g)", FvName);
> > + }
> > + FvBlob2.BlobBase = FvBlob.BlobBase;
> > + FvBlob2.BlobLength = FvBlob.BlobLength;
> > + TcgEventHdr.EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB2;
> > + TcgEventHdr.EventSize = sizeof (FvBlob2);
> > + EventData = &FvBlob2;
> > + }
> >
> > if (Tpm2HashMask == 0) {
> > //
> > @@ -583,9 +656,9 @@ MeasureFvImage (
> > );
> >
> > if (!EFI_ERROR(Status)) {
> > - Status = LogHashEvent (&DigestList, &TcgEventHdr, (UINT8*) &FvBlob);
> > - DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged
> by
> > Tcg2Pei starts at: 0x%x\n", FvBlob.BlobBase));
> > - DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged
> by
> > Tcg2Pei has the size: 0x%x\n", FvBlob.BlobLength));
> > + Status = LogHashEvent (&DigestList, &TcgEventHdr, EventData);
> > + DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged
> by
> > Tcg2Pei starts at: 0x%x\n", FvBase));
> > + DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged
> by
> > Tcg2Pei has the size: 0x%x\n", FvLength));
> > } else if (Status == EFI_DEVICE_ERROR) {
> > BuildGuidHob (&gTpmErrorHobGuid,0);
> > REPORT_STATUS_CODE (
> > @@ -599,13 +672,13 @@ MeasureFvImage (
> > //
> > Status = HashLogExtendEvent (
> > 0,
> > - (UINT8*) (UINTN) FvBlob.BlobBase,
> > - (UINTN) FvBlob.BlobLength,
> > - &TcgEventHdr,
> > - (UINT8*) &FvBlob
> > + (UINT8*) (UINTN) FvBase, // HashData
> > + (UINTN) FvLength, // HashDataLen
> > + &TcgEventHdr, // EventHdr
> > + EventData // EventData
> > );
> > - DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei starts at:
> > 0x%x\n", FvBlob.BlobBase));
> > - DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei has the size:
> > 0x%x\n", FvBlob.BlobLength));
> > + DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei starts at:
> > 0x%x\n", FvBase));
> > + DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei has the size:
> > 0x%x\n", FvLength));
> > }
> >
> > if (EFI_ERROR(Status)) {
> > diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> > b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> > index 30f985b6ea..3d361e8859 100644
> > --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> > +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> > @@ -54,6 +54,7 @@
> > MemoryAllocationLib
> > ReportStatusCodeLib
> > ResetSystemLib
> > + PrintLib
> >
> > [Guids]
> > gTcgEventEntryHobGuid ## PRODUCES ##
> > HOB
> > @@ -74,6 +75,7 @@
> >
> > [Pcd]
> > gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString ##
> > SOMETIMES_CONSUMES
> > + gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision
> ##
> > CONSUMES
> > gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid ##
> > CONSUMES
> > gEfiSecurityPkgTokenSpaceGuid.PcdTpm2InitializationPolicy ##
> > CONSUMES
> > gEfiSecurityPkgTokenSpaceGuid.PcdTpm2SelfTestPolicy ##
> > SOMETIMES_CONSUMES
> > --
> > 2.19.2.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
2020-01-06 5:53 ` Yao, Jiewen
@ 2020-01-06 5:57 ` Wang, Jian J
2020-01-06 6:00 ` Yao, Jiewen
0 siblings, 1 reply; 21+ messages in thread
From: Wang, Jian J @ 2020-01-06 5:57 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io; +Cc: Zhang, Chao B
I mean putting all rev0 code in 'else' not just one line. For rev105, your patch
will initialize FvBlob but never use it at all. It will confuse readers sometimes.
Regards,
Jian
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Monday, January 06, 2020 1:53 PM
> To: Wang, Jian J <jian.j.wang@intel.com>; devel@edk2.groups.io
> Cc: Zhang, Chao B <chao.b.zhang@intel.com>
> Subject: RE: [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
>
> Hi Jian
> I purposely put the line there, because I want to group all the rev 0 code
> together and rev 105 code together in if statement. I don't want to move that
> particular line to else statement.
>
> Thank you
> Yao Jiewen
>
> > -----Original Message-----
> > From: Wang, Jian J <jian.j.wang@intel.com>
> > Sent: Monday, January 6, 2020 1:33 PM
> > To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> > Cc: Zhang, Chao B <chao.b.zhang@intel.com>
> > Subject: RE: [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
> >
> > Jiewen,
> >
> > Just one minor comment. With it addressed,
> >
> > Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
> >
> >
> > > -----Original Message-----
> > > From: Yao, Jiewen <jiewen.yao@intel.com>
> > > Sent: Tuesday, December 31, 2019 2:44 PM
> > > To: devel@edk2.groups.io
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Zhang, Chao B
> > > <chao.b.zhang@intel.com>
> > > Subject: [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
> > >
> > > Use EV_EFI_PLATFORM_FIRMWARE_BLOB2 if the TCG PFP revision is >= 105.
> > > Use FvName as the description for the FV.
> > >
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Chao Zhang <chao.b.zhang@intel.com>
> > > Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> > > ---
> > > SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 91 ++++++++++++++++++++++++++---
> > > SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 2 +
> > > 2 files changed, 84 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> > > b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> > > index 1565d4e402..7d99c7906a 100644
> > > --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> > > +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> > > @@ -37,6 +37,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > > #include <Library/MemoryAllocationLib.h>
> > > #include <Library/ReportStatusCodeLib.h>
> > > #include <Library/ResetSystemLib.h>
> > > +#include <Library/PrintLib.h>
> > >
> > > #define PERF_ID_TCG2_PEI 0x3080
> > >
> > > @@ -78,6 +79,18 @@ EFI_PLATFORM_FIRMWARE_BLOB
> > > *mMeasuredChildFvInfo;
> > > UINT32 mMeasuredMaxChildFvIndex = 0;
> > > UINT32 mMeasuredChildFvIndex = 0;
> > >
> > > +#pragma pack (1)
> > > +
> > > +#define FV_HANDOFF_TABLE_DESC "Fv(XXXXXXXX-XXXX-XXXX-XXXX-
> > > XXXXXXXXXXXX)"
> > > +typedef struct {
> > > + UINT8 BlobDescriptionSize;
> > > + UINT8 BlobDescription[sizeof(FV_HANDOFF_TABLE_DESC)];
> > > + EFI_PHYSICAL_ADDRESS BlobBase;
> > > + UINT64 BlobLength;
> > > +} FV_HANDOFF_TABLE_POINTERS2;
> > > +
> > > +#pragma pack ()
> > > +
> > > /**
> > > Measure and record the Firmware Volume Information once FvInfoPPI
> install.
> > >
> > > @@ -447,6 +460,48 @@ MeasureCRTMVersion (
> > > );
> > > }
> > >
> > > +/*
> > > + Get the FvName from the FV header.
> > > +
> > > + Causion: The FV is untrusted input.
> > > +
> > > + @param[in] FvBase Base address of FV image.
> > > + @param[in] FvLength Length of FV image.
> > > +
> > > + @return FvName pointer
> > > + @retval NULL FvName is NOT found
> > > +*/
> > > +VOID *
> > > +GetFvName (
> > > + IN EFI_PHYSICAL_ADDRESS FvBase,
> > > + IN UINT64 FvLength
> > > + )
> > > +{
> > > + EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
> > > + EFI_FIRMWARE_VOLUME_EXT_HEADER *FvExtHeader;
> > > +
> > > + if (FvBase >= MAX_ADDRESS) {
> > > + return NULL;
> > > + }
> > > + if (FvLength >= MAX_ADDRESS - FvBase) {
> > > + return NULL;
> > > + }
> > > + if (FvLength < sizeof(EFI_FIRMWARE_VOLUME_HEADER)) {
> > > + return NULL;
> > > + }
> > > +
> > > + FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)FvBase;
> > > + if (FvHeader->ExtHeaderOffset <
> sizeof(EFI_FIRMWARE_VOLUME_HEADER))
> > {
> > > + return NULL;
> > > + }
> > > + if (FvHeader->ExtHeaderOffset +
> > > sizeof(EFI_FIRMWARE_VOLUME_EXT_HEADER) > FvLength) {
> > > + return NULL;
> > > + }
> > > + FvExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *)(UINTN)(FvBase
> +
> > > FvHeader->ExtHeaderOffset);
> > > +
> > > + return &FvExtHeader->FvName;
> > > +}
> > > +
> > > /**
> > > Measure FV image.
> > > Add it into the measured FV list after the FV is measured successfully.
> > > @@ -469,6 +524,9 @@ MeasureFvImage (
> > > UINT32 Index;
> > > EFI_STATUS Status;
> > > EFI_PLATFORM_FIRMWARE_BLOB FvBlob;
> > > + FV_HANDOFF_TABLE_POINTERS2 FvBlob2;
> > > + VOID *EventData;
> > > + VOID *FvName;
> > > TCG_PCR_EVENT_HDR TcgEventHdr;
> > > UINT32 Instance;
> > > UINT32 Tpm2HashMask;
> > > @@ -571,6 +629,21 @@ MeasureFvImage (
> > > TcgEventHdr.PCRIndex = 0;
> > > TcgEventHdr.EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB;
> > > TcgEventHdr.EventSize = sizeof (FvBlob);
> > > + EventData = &FvBlob;
> > > +
> >
> > I'd suggest to put above code in 'else' block to make code easier to read,
> > i.e. FvBlob for revision less than 105 and FvBlob2 for later ones.
> >
> > Regards,
> > Jian
> >
> > > + if (PcdGet32(PcdTcgPfpMeasurementRevision) >=
> > > TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105) {
> > > + FvBlob2.BlobDescriptionSize = sizeof(FvBlob2.BlobDescription);
> > > + CopyMem (FvBlob2.BlobDescription, FV_HANDOFF_TABLE_DESC,
> > > sizeof(FvBlob2.BlobDescription));
> > > + FvName = GetFvName (FvBase, FvLength);
> > > + if (FvName != NULL) {
> > > + AsciiSPrint ((CHAR8 *)FvBlob2.BlobDescription,
> > > sizeof(FvBlob2.BlobDescription), "Fv(%g)", FvName);
> > > + }
> > > + FvBlob2.BlobBase = FvBlob.BlobBase;
> > > + FvBlob2.BlobLength = FvBlob.BlobLength;
> > > + TcgEventHdr.EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB2;
> > > + TcgEventHdr.EventSize = sizeof (FvBlob2);
> > > + EventData = &FvBlob2;
> > > + }
> > >
> > > if (Tpm2HashMask == 0) {
> > > //
> > > @@ -583,9 +656,9 @@ MeasureFvImage (
> > > );
> > >
> > > if (!EFI_ERROR(Status)) {
> > > - Status = LogHashEvent (&DigestList, &TcgEventHdr, (UINT8*) &FvBlob);
> > > - DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged
> > by
> > > Tcg2Pei starts at: 0x%x\n", FvBlob.BlobBase));
> > > - DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged
> > by
> > > Tcg2Pei has the size: 0x%x\n", FvBlob.BlobLength));
> > > + Status = LogHashEvent (&DigestList, &TcgEventHdr, EventData);
> > > + DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged
> > by
> > > Tcg2Pei starts at: 0x%x\n", FvBase));
> > > + DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged
> > by
> > > Tcg2Pei has the size: 0x%x\n", FvLength));
> > > } else if (Status == EFI_DEVICE_ERROR) {
> > > BuildGuidHob (&gTpmErrorHobGuid,0);
> > > REPORT_STATUS_CODE (
> > > @@ -599,13 +672,13 @@ MeasureFvImage (
> > > //
> > > Status = HashLogExtendEvent (
> > > 0,
> > > - (UINT8*) (UINTN) FvBlob.BlobBase,
> > > - (UINTN) FvBlob.BlobLength,
> > > - &TcgEventHdr,
> > > - (UINT8*) &FvBlob
> > > + (UINT8*) (UINTN) FvBase, // HashData
> > > + (UINTN) FvLength, // HashDataLen
> > > + &TcgEventHdr, // EventHdr
> > > + EventData // EventData
> > > );
> > > - DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei starts at:
> > > 0x%x\n", FvBlob.BlobBase));
> > > - DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei has the size:
> > > 0x%x\n", FvBlob.BlobLength));
> > > + DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei starts at:
> > > 0x%x\n", FvBase));
> > > + DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei has the
> size:
> > > 0x%x\n", FvLength));
> > > }
> > >
> > > if (EFI_ERROR(Status)) {
> > > diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> > > b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> > > index 30f985b6ea..3d361e8859 100644
> > > --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> > > +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> > > @@ -54,6 +54,7 @@
> > > MemoryAllocationLib
> > > ReportStatusCodeLib
> > > ResetSystemLib
> > > + PrintLib
> > >
> > > [Guids]
> > > gTcgEventEntryHobGuid ## PRODUCES ##
> > > HOB
> > > @@ -74,6 +75,7 @@
> > >
> > > [Pcd]
> > > gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString ##
> > > SOMETIMES_CONSUMES
> > > + gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision
> > ##
> > > CONSUMES
> > > gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid ##
> > > CONSUMES
> > > gEfiSecurityPkgTokenSpaceGuid.PcdTpm2InitializationPolicy ##
> > > CONSUMES
> > > gEfiSecurityPkgTokenSpaceGuid.PcdTpm2SelfTestPolicy ##
> > > SOMETIMES_CONSUMES
> > > --
> > > 2.19.2.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH 2/6] SecurityPkg/Tcg2Dxe: Add Tcg2Dxe to support 800-155 event.
2019-12-31 6:44 ` [PATCH 2/6] SecurityPkg/Tcg2Dxe: Add Tcg2Dxe to support 800-155 event Yao, Jiewen
@ 2020-01-06 5:59 ` Wang, Jian J
0 siblings, 0 replies; 21+ messages in thread
From: Wang, Jian J @ 2020-01-06 5:59 UTC (permalink / raw)
To: devel@edk2.groups.io, Yao, Jiewen; +Cc: Zhang, Chao B
Jiewen,
Just two coding style comments below. With it addressed,
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
> Sent: Tuesday, December 31, 2019 2:44 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Zhang, Chao B
> <chao.b.zhang@intel.com>
> Subject: [edk2-devel] [PATCH 2/6] SecurityPkg/Tcg2Dxe: Add Tcg2Dxe to
> support 800-155 event.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
>
> The TCG2 DXE supports to parse the 800-155 event GUID from PEI
> and puts to the beginning of the TCG2 event.
>
> The TCG2 DXE also supports a DXE driver produces 800-155 event
> and let TCG2 DXE driver record.
>
> The 800-155 is a NO-ACTION event which does not need extend
> anything to TPM2. The TCG2 DXE also supports that.
>
> Multiple 800-155 events are supported. All of them will be put
> to the beginning of the TCG2 event, just after the SpecId event.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
> SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 157 +++++++++++++++++++++++-----
> SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf | 1 +
> 2 files changed, 129 insertions(+), 29 deletions(-)
>
> diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> index 3cd16c2fa3..b185b56703 100644
> --- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> +++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> @@ -75,6 +75,7 @@ typedef struct {
> UINT8 *LastEvent;
> BOOLEAN EventLogStarted;
> BOOLEAN EventLogTruncated;
> + UINTN Next800155EventOffset;
> } TCG_EVENT_LOG_AREA_STRUCT;
>
> typedef struct _TCG_DXE_DATA {
> @@ -771,16 +772,42 @@ Tcg2GetEventLog (
> return EFI_SUCCESS;
> }
>
> +/*
> + Return if this is a Tcg800155PlatformIdEvent.
> +
> + @param[in] NewEventHdr Pointer to a
> TCG_PCR_EVENT_HDR/TCG_PCR_EVENT_EX data structure.
> + @param[in] NewEventHdrSize New event header size.
> + @param[in] NewEventData Pointer to the new event data.
> + @param[in] NewEventSize New event data size.
> +
> + @retval TRUE This is a Tcg800155PlatformIdEvent.
> + @retval FALSE This is NOT a Tcg800155PlatformIdEvent.
> +
> +*/
> +BOOLEAN
> +Is800155Event (
> + IN VOID *NewEventHdr,
> + IN UINT32 NewEventHdrSize,
> + IN UINT8 *NewEventData,
> + IN UINT32 NewEventSize
> + )
> +{
> + if ((((TCG_PCR_EVENT2_HDR *)NewEventHdr)->EventType == EV_NO_ACTION)
> &&
> + (NewEventSize >= sizeof(TCG_Sp800_155_PlatformId_Event2)) &&
> + (CompareMem (NewEventData,
> TCG_Sp800_155_PlatformId_Event2_SIGNATURE,
> sizeof(TCG_Sp800_155_PlatformId_Event2_SIGNATURE) - 1) == 0)) {
The above line is too long to read in one screen. You could wrap the last
parameter of CompareMem in new line or add a new local variable to
keep the size.
> + return TRUE;
> + }
> + return FALSE;
> +}
> +
> /**
> Add a new entry to the Event Log.
>
> - @param[in, out] EventLogPtr Pointer to the Event Log data.
> - @param[in, out] LogSize Size of the Event Log.
> - @param[in] MaxSize Maximum size of the Event Log.
> - @param[in] NewEventHdr Pointer to a
> TCG_PCR_EVENT_HDR/TCG_PCR_EVENT_EX data structure.
> - @param[in] NewEventHdrSize New event header size.
> - @param[in] NewEventData Pointer to the new event data.
> - @param[in] NewEventSize New event data size.
> + @param[in, out] EventLogAreaStruct The event log area data structure
> + @param[in] NewEventHdr Pointer to a
> TCG_PCR_EVENT_HDR/TCG_PCR_EVENT_EX data structure.
> + @param[in] NewEventHdrSize New event header size.
> + @param[in] NewEventData Pointer to the new event data.
> + @param[in] NewEventSize New event data size.
>
> @retval EFI_SUCCESS The new event log entry was added.
> @retval EFI_OUT_OF_RESOURCES No enough memory to log the new event.
> @@ -788,9 +815,7 @@ Tcg2GetEventLog (
> **/
> EFI_STATUS
> TcgCommLogEvent (
> - IN OUT UINT8 **EventLogPtr,
> - IN OUT UINTN *LogSize,
> - IN UINTN MaxSize,
> + IN OUT TCG_EVENT_LOG_AREA_STRUCT *EventLogAreaStruct,
> IN VOID *NewEventHdr,
> IN UINT32 NewEventHdrSize,
> IN UINT8 *NewEventData,
> @@ -798,6 +823,7 @@ TcgCommLogEvent (
> )
> {
> UINTN NewLogSize;
> + BOOLEAN Record800155Event;
>
> if (NewEventSize > MAX_ADDRESS - NewEventHdrSize) {
> return EFI_OUT_OF_RESOURCES;
> @@ -805,23 +831,55 @@ TcgCommLogEvent (
>
> NewLogSize = NewEventHdrSize + NewEventSize;
>
> - if (NewLogSize > MAX_ADDRESS - *LogSize) {
> + if (NewLogSize > MAX_ADDRESS - EventLogAreaStruct->EventLogSize) {
> return EFI_OUT_OF_RESOURCES;
> }
>
> - if (NewLogSize + *LogSize > MaxSize) {
> - DEBUG ((EFI_D_INFO, " MaxSize - 0x%x\n", MaxSize));
> - DEBUG ((EFI_D_INFO, " NewLogSize - 0x%x\n", NewLogSize));
> - DEBUG ((EFI_D_INFO, " LogSize - 0x%x\n", *LogSize));
> - DEBUG ((EFI_D_INFO, "TcgCommLogEvent - %r\n",
> EFI_OUT_OF_RESOURCES));
> + if (NewLogSize + EventLogAreaStruct->EventLogSize > EventLogAreaStruct-
> >Laml) {
> + DEBUG ((DEBUG_INFO, " Laml - 0x%x\n", EventLogAreaStruct->Laml));
> + DEBUG ((DEBUG_INFO, " NewLogSize - 0x%x\n", NewLogSize));
> + DEBUG ((DEBUG_INFO, " LogSize - 0x%x\n", EventLogAreaStruct-
> >EventLogSize));
> + DEBUG ((DEBUG_INFO, "TcgCommLogEvent - %r\n",
> EFI_OUT_OF_RESOURCES));
> return EFI_OUT_OF_RESOURCES;
> }
>
> - *EventLogPtr += *LogSize;
> - *LogSize += NewLogSize;
> - CopyMem (*EventLogPtr, NewEventHdr, NewEventHdrSize);
> + //
> + // Check 800-155 event
> + // Record to 800-155 event offset only.
> + // If the offset is 0, no need to record.
> + //
> + Record800155Event = Is800155Event (NewEventHdr, NewEventHdrSize,
> NewEventData, NewEventSize);
> + if (Record800155Event) {
> + if (EventLogAreaStruct->Next800155EventOffset != 0) {
> + CopyMem (
> + (UINT8 *)(UINTN)EventLogAreaStruct->Lasa + EventLogAreaStruct-
> >Next800155EventOffset + NewLogSize,
> + (UINT8 *)(UINTN)EventLogAreaStruct->Lasa + EventLogAreaStruct-
> >Next800155EventOffset,
> + EventLogAreaStruct->EventLogSize - EventLogAreaStruct-
> >Next800155EventOffset
> + );
> +
> + CopyMem (
> + (UINT8 *)(UINTN)EventLogAreaStruct->Lasa + EventLogAreaStruct-
> >Next800155EventOffset,
> + NewEventHdr,
> + NewEventHdrSize
> + );
> + CopyMem (
> + (UINT8 *)(UINTN)EventLogAreaStruct->Lasa + EventLogAreaStruct-
> >Next800155EventOffset + NewEventHdrSize,
> + NewEventData,
> + NewEventSize
> + );
> +
> + EventLogAreaStruct->Next800155EventOffset += NewLogSize;
> + EventLogAreaStruct->LastEvent += NewLogSize;
> + EventLogAreaStruct->EventLogSize += NewLogSize;
> + }
> + return EFI_SUCCESS;
> + }
> +
> + EventLogAreaStruct->LastEvent = (UINT8 *)(UINTN)EventLogAreaStruct->Lasa
> + EventLogAreaStruct->EventLogSize;
> + EventLogAreaStruct->EventLogSize += NewLogSize;
> + CopyMem (EventLogAreaStruct->LastEvent, NewEventHdr, NewEventHdrSize);
> CopyMem (
> - *EventLogPtr + NewEventHdrSize,
> + EventLogAreaStruct->LastEvent + NewEventHdrSize,
> NewEventData,
> NewEventSize
> );
> @@ -873,11 +931,8 @@ TcgDxeLogEvent (
> return EFI_VOLUME_FULL;
> }
>
> - EventLogAreaStruct->LastEvent = (UINT8*)(UINTN)EventLogAreaStruct->Lasa;
> Status = TcgCommLogEvent (
> - &EventLogAreaStruct->LastEvent,
> - &EventLogAreaStruct->EventLogSize,
> - (UINTN)EventLogAreaStruct->Laml,
> + EventLogAreaStruct,
> NewEventHdr,
> NewEventHdrSize,
> NewEventData,
> @@ -907,11 +962,8 @@ TcgDxeLogEvent (
> return EFI_VOLUME_FULL;
> }
>
> - EventLogAreaStruct->LastEvent = (UINT8*)(UINTN)EventLogAreaStruct-
> >Lasa;
> Status = TcgCommLogEvent (
> - &EventLogAreaStruct->LastEvent,
> - &EventLogAreaStruct->EventLogSize,
> - (UINTN)EventLogAreaStruct->Laml,
> + EventLogAreaStruct,
> NewEventHdr,
> NewEventHdrSize,
> NewEventData,
> @@ -1138,11 +1190,25 @@ TcgDxeHashLogExtendEvent (
> {
> EFI_STATUS Status;
> TPML_DIGEST_VALUES DigestList;
> + TCG_PCR_EVENT2_HDR NoActionEvent;
>
> if (!mTcgDxeData.BsCap.TPMPresentFlag) {
> return EFI_DEVICE_ERROR;
> }
>
> + if (NewEventHdr->EventType == EV_NO_ACTION) {
> + //
> + // Do not do TPM extend for EV_NO_ACTION
> + //
> + Status = EFI_SUCCESS;
> + InitNoActionEvent (&NoActionEvent, NewEventHdr->EventSize);
> + if ((Flags & EFI_TCG2_EXTEND_ONLY) == 0) {
> + Status = TcgDxeLogHashEvent (&(NoActionEvent.Digests), NewEventHdr,
> NewEventData);
> + }
> +
> + return Status;
> + }
> +
> Status = HashAndExtend (
> NewEventHdr->PCRIndex,
> HashData,
> @@ -1202,7 +1268,13 @@ Tcg2HashLogExtendEvent (
>
> DEBUG ((DEBUG_VERBOSE, "Tcg2HashLogExtendEvent ...\n"));
>
> - if ((This == NULL) || (DataToHash == 0) || (Event == NULL)) {
> + if ((This == NULL) || (Event == NULL)) {
> + return EFI_INVALID_PARAMETER;
> + }
> + //
> + // Do not check hash data size for EV_NO_ACTION event.
> + //
> + if ((Event->Header.EventType != EV_NO_ACTION) && (DataToHash == 0)) {
> return EFI_INVALID_PARAMETER;
> }
>
> @@ -1487,6 +1559,7 @@ SetupEventLog (
> }
> mTcgDxeData.EventLogAreaStruct[Index].Lasa = Lasa;
> mTcgDxeData.EventLogAreaStruct[Index].Laml = PcdGet32
> (PcdTcgLogAreaMinLen);
> + mTcgDxeData.EventLogAreaStruct[Index].Next800155EventOffset = 0;
>
> if ((PcdGet8(PcdTpm2AcpiTableRev) >= 4) ||
> (mTcg2EventInfo[Index].LogFormat ==
> EFI_TCG2_EVENT_LOG_FORMAT_TCG_2)) {
> @@ -1577,6 +1650,30 @@ SetupEventLog (
> (UINT8 *)TcgEfiSpecIdEventStruct,
> SpecIdEvent.EventSize
> );
> + //
> + // record the offset at the end of 800-155 event.
> + // the future 800-155 event can be inserted here.
> + //
> + mTcgDxeData.EventLogAreaStruct[Index].Next800155EventOffset =
> mTcgDxeData.EventLogAreaStruct[Index].EventLogSize;
> +
The above line is too long to read in one screen. Suggest to wrap after '='.
Regards,
Jian
> + //
> + // Tcg800155PlatformIdEvent. Event format is TCG_PCR_EVENT2
> + //
> + GuidHob.Guid = GetFirstGuidHob (&gTcg800155PlatformIdEventHobGuid);
> + while (GuidHob.Guid != NULL) {
> + InitNoActionEvent(&NoActionEvent, GET_GUID_HOB_DATA_SIZE
> (GuidHob.Guid));
> +
> + Status = TcgDxeLogEvent (
> + mTcg2EventInfo[Index].LogFormat,
> + &NoActionEvent,
> + sizeof(NoActionEvent.PCRIndex) + sizeof(NoActionEvent.EventType)
> + GetDigestListBinSize (&NoActionEvent.Digests) +
> sizeof(NoActionEvent.EventSize),
The above line is too long to read in one screen. Suggest to wrap around second '+'.
> + GET_GUID_HOB_DATA (GuidHob.Guid),
> + GET_GUID_HOB_DATA_SIZE (GuidHob.Guid)
> + );
> +
> + GuidHob.Guid = GET_NEXT_HOB (GuidHob);
> + GuidHob.Guid = GetNextGuidHob (&gTcg800155PlatformIdEventHobGuid,
> GuidHob.Guid);
> + }
>
> //
> // EfiStartupLocalityEvent. Event format is TCG_PCR_EVENT2
> @@ -1643,6 +1740,7 @@ SetupEventLog (
> mTcgDxeData.FinalEventLogAreaStruct[Index].LastEvent = (VOID
> *)(UINTN)mTcgDxeData.FinalEventLogAreaStruct[Index].Lasa;
> mTcgDxeData.FinalEventLogAreaStruct[Index].EventLogStarted = FALSE;
> mTcgDxeData.FinalEventLogAreaStruct[Index].EventLogTruncated = FALSE;
> + mTcgDxeData.FinalEventLogAreaStruct[Index].Next800155EventOffset = 0;
>
> //
> // Install to configuration table for
> EFI_TCG2_EVENT_LOG_FORMAT_TCG_2
> @@ -1663,6 +1761,7 @@ SetupEventLog (
> mTcgDxeData.FinalEventLogAreaStruct[Index].LastEvent = 0;
> mTcgDxeData.FinalEventLogAreaStruct[Index].EventLogStarted = FALSE;
> mTcgDxeData.FinalEventLogAreaStruct[Index].EventLogTruncated = FALSE;
> + mTcgDxeData.FinalEventLogAreaStruct[Index].Next800155EventOffset = 0;
> }
> }
> }
> diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
> b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
> index 0127a31e97..576cf80d06 100644
> --- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
> +++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
> @@ -85,6 +85,7 @@
>
> gTcgEvent2EntryHobGuid ## SOMETIMES_CONSUMES ## HOB
> gTpm2StartupLocalityHobGuid ## SOMETIMES_CONSUMES ##
> HOB
> + gTcg800155PlatformIdEventHobGuid ## SOMETIMES_CONSUMES
> ## HOB
>
> [Protocols]
> gEfiTcg2ProtocolGuid ## PRODUCES
> --
> 2.19.2.windows.1
>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
2020-01-06 5:57 ` Wang, Jian J
@ 2020-01-06 6:00 ` Yao, Jiewen
0 siblings, 0 replies; 21+ messages in thread
From: Yao, Jiewen @ 2020-01-06 6:00 UTC (permalink / raw)
To: Wang, Jian J, devel@edk2.groups.io; +Cc: Zhang, Chao B
I see. Will do that.
> -----Original Message-----
> From: Wang, Jian J <jian.j.wang@intel.com>
> Sent: Monday, January 6, 2020 1:58 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: Zhang, Chao B <chao.b.zhang@intel.com>
> Subject: RE: [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
>
> I mean putting all rev0 code in 'else' not just one line. For rev105, your patch
> will initialize FvBlob but never use it at all. It will confuse readers sometimes.
>
> Regards,
> Jian
>
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen.yao@intel.com>
> > Sent: Monday, January 06, 2020 1:53 PM
> > To: Wang, Jian J <jian.j.wang@intel.com>; devel@edk2.groups.io
> > Cc: Zhang, Chao B <chao.b.zhang@intel.com>
> > Subject: RE: [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
> >
> > Hi Jian
> > I purposely put the line there, because I want to group all the rev 0 code
> > together and rev 105 code together in if statement. I don't want to move that
> > particular line to else statement.
> >
> > Thank you
> > Yao Jiewen
> >
> > > -----Original Message-----
> > > From: Wang, Jian J <jian.j.wang@intel.com>
> > > Sent: Monday, January 6, 2020 1:33 PM
> > > To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> > > Cc: Zhang, Chao B <chao.b.zhang@intel.com>
> > > Subject: RE: [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
> > >
> > > Jiewen,
> > >
> > > Just one minor comment. With it addressed,
> > >
> > > Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
> > >
> > >
> > > > -----Original Message-----
> > > > From: Yao, Jiewen <jiewen.yao@intel.com>
> > > > Sent: Tuesday, December 31, 2019 2:44 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Zhang, Chao B
> > > > <chao.b.zhang@intel.com>
> > > > Subject: [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
> > > >
> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
> > > >
> > > > Use EV_EFI_PLATFORM_FIRMWARE_BLOB2 if the TCG PFP revision is >=
> 105.
> > > > Use FvName as the description for the FV.
> > > >
> > > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > > Cc: Chao Zhang <chao.b.zhang@intel.com>
> > > > Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> > > > ---
> > > > SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 91 ++++++++++++++++++++++++++-
> --
> > > > SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 2 +
> > > > 2 files changed, 84 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> > > > b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> > > > index 1565d4e402..7d99c7906a 100644
> > > > --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> > > > +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> > > > @@ -37,6 +37,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > #include <Library/MemoryAllocationLib.h>
> > > > #include <Library/ReportStatusCodeLib.h>
> > > > #include <Library/ResetSystemLib.h>
> > > > +#include <Library/PrintLib.h>
> > > >
> > > > #define PERF_ID_TCG2_PEI 0x3080
> > > >
> > > > @@ -78,6 +79,18 @@ EFI_PLATFORM_FIRMWARE_BLOB
> > > > *mMeasuredChildFvInfo;
> > > > UINT32 mMeasuredMaxChildFvIndex = 0;
> > > > UINT32 mMeasuredChildFvIndex = 0;
> > > >
> > > > +#pragma pack (1)
> > > > +
> > > > +#define FV_HANDOFF_TABLE_DESC "Fv(XXXXXXXX-XXXX-XXXX-XXXX-
> > > > XXXXXXXXXXXX)"
> > > > +typedef struct {
> > > > + UINT8 BlobDescriptionSize;
> > > > + UINT8
> BlobDescription[sizeof(FV_HANDOFF_TABLE_DESC)];
> > > > + EFI_PHYSICAL_ADDRESS BlobBase;
> > > > + UINT64 BlobLength;
> > > > +} FV_HANDOFF_TABLE_POINTERS2;
> > > > +
> > > > +#pragma pack ()
> > > > +
> > > > /**
> > > > Measure and record the Firmware Volume Information once FvInfoPPI
> > install.
> > > >
> > > > @@ -447,6 +460,48 @@ MeasureCRTMVersion (
> > > > );
> > > > }
> > > >
> > > > +/*
> > > > + Get the FvName from the FV header.
> > > > +
> > > > + Causion: The FV is untrusted input.
> > > > +
> > > > + @param[in] FvBase Base address of FV image.
> > > > + @param[in] FvLength Length of FV image.
> > > > +
> > > > + @return FvName pointer
> > > > + @retval NULL FvName is NOT found
> > > > +*/
> > > > +VOID *
> > > > +GetFvName (
> > > > + IN EFI_PHYSICAL_ADDRESS FvBase,
> > > > + IN UINT64 FvLength
> > > > + )
> > > > +{
> > > > + EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
> > > > + EFI_FIRMWARE_VOLUME_EXT_HEADER *FvExtHeader;
> > > > +
> > > > + if (FvBase >= MAX_ADDRESS) {
> > > > + return NULL;
> > > > + }
> > > > + if (FvLength >= MAX_ADDRESS - FvBase) {
> > > > + return NULL;
> > > > + }
> > > > + if (FvLength < sizeof(EFI_FIRMWARE_VOLUME_HEADER)) {
> > > > + return NULL;
> > > > + }
> > > > +
> > > > + FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)FvBase;
> > > > + if (FvHeader->ExtHeaderOffset <
> > sizeof(EFI_FIRMWARE_VOLUME_HEADER))
> > > {
> > > > + return NULL;
> > > > + }
> > > > + if (FvHeader->ExtHeaderOffset +
> > > > sizeof(EFI_FIRMWARE_VOLUME_EXT_HEADER) > FvLength) {
> > > > + return NULL;
> > > > + }
> > > > + FvExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER
> *)(UINTN)(FvBase
> > +
> > > > FvHeader->ExtHeaderOffset);
> > > > +
> > > > + return &FvExtHeader->FvName;
> > > > +}
> > > > +
> > > > /**
> > > > Measure FV image.
> > > > Add it into the measured FV list after the FV is measured successfully.
> > > > @@ -469,6 +524,9 @@ MeasureFvImage (
> > > > UINT32 Index;
> > > > EFI_STATUS Status;
> > > > EFI_PLATFORM_FIRMWARE_BLOB FvBlob;
> > > > + FV_HANDOFF_TABLE_POINTERS2 FvBlob2;
> > > > + VOID *EventData;
> > > > + VOID *FvName;
> > > > TCG_PCR_EVENT_HDR TcgEventHdr;
> > > > UINT32 Instance;
> > > > UINT32 Tpm2HashMask;
> > > > @@ -571,6 +629,21 @@ MeasureFvImage (
> > > > TcgEventHdr.PCRIndex = 0;
> > > > TcgEventHdr.EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB;
> > > > TcgEventHdr.EventSize = sizeof (FvBlob);
> > > > + EventData = &FvBlob;
> > > > +
> > >
> > > I'd suggest to put above code in 'else' block to make code easier to read,
> > > i.e. FvBlob for revision less than 105 and FvBlob2 for later ones.
> > >
> > > Regards,
> > > Jian
> > >
> > > > + if (PcdGet32(PcdTcgPfpMeasurementRevision) >=
> > > > TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105) {
> > > > + FvBlob2.BlobDescriptionSize = sizeof(FvBlob2.BlobDescription);
> > > > + CopyMem (FvBlob2.BlobDescription, FV_HANDOFF_TABLE_DESC,
> > > > sizeof(FvBlob2.BlobDescription));
> > > > + FvName = GetFvName (FvBase, FvLength);
> > > > + if (FvName != NULL) {
> > > > + AsciiSPrint ((CHAR8 *)FvBlob2.BlobDescription,
> > > > sizeof(FvBlob2.BlobDescription), "Fv(%g)", FvName);
> > > > + }
> > > > + FvBlob2.BlobBase = FvBlob.BlobBase;
> > > > + FvBlob2.BlobLength = FvBlob.BlobLength;
> > > > + TcgEventHdr.EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB2;
> > > > + TcgEventHdr.EventSize = sizeof (FvBlob2);
> > > > + EventData = &FvBlob2;
> > > > + }
> > > >
> > > > if (Tpm2HashMask == 0) {
> > > > //
> > > > @@ -583,9 +656,9 @@ MeasureFvImage (
> > > > );
> > > >
> > > > if (!EFI_ERROR(Status)) {
> > > > - Status = LogHashEvent (&DigestList, &TcgEventHdr, (UINT8*)
> &FvBlob);
> > > > - DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended &
> logged
> > > by
> > > > Tcg2Pei starts at: 0x%x\n", FvBlob.BlobBase));
> > > > - DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended &
> logged
> > > by
> > > > Tcg2Pei has the size: 0x%x\n", FvBlob.BlobLength));
> > > > + Status = LogHashEvent (&DigestList, &TcgEventHdr, EventData);
> > > > + DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended &
> logged
> > > by
> > > > Tcg2Pei starts at: 0x%x\n", FvBase));
> > > > + DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended &
> logged
> > > by
> > > > Tcg2Pei has the size: 0x%x\n", FvLength));
> > > > } else if (Status == EFI_DEVICE_ERROR) {
> > > > BuildGuidHob (&gTpmErrorHobGuid,0);
> > > > REPORT_STATUS_CODE (
> > > > @@ -599,13 +672,13 @@ MeasureFvImage (
> > > > //
> > > > Status = HashLogExtendEvent (
> > > > 0,
> > > > - (UINT8*) (UINTN) FvBlob.BlobBase,
> > > > - (UINTN) FvBlob.BlobLength,
> > > > - &TcgEventHdr,
> > > > - (UINT8*) &FvBlob
> > > > + (UINT8*) (UINTN) FvBase, // HashData
> > > > + (UINTN) FvLength, // HashDataLen
> > > > + &TcgEventHdr, // EventHdr
> > > > + EventData // EventData
> > > > );
> > > > - DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei starts at:
> > > > 0x%x\n", FvBlob.BlobBase));
> > > > - DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei has the
> size:
> > > > 0x%x\n", FvBlob.BlobLength));
> > > > + DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei starts at:
> > > > 0x%x\n", FvBase));
> > > > + DEBUG ((DEBUG_INFO, "The FV which is measured by Tcg2Pei has the
> > size:
> > > > 0x%x\n", FvLength));
> > > > }
> > > >
> > > > if (EFI_ERROR(Status)) {
> > > > diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> > > > b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> > > > index 30f985b6ea..3d361e8859 100644
> > > > --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> > > > +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> > > > @@ -54,6 +54,7 @@
> > > > MemoryAllocationLib
> > > > ReportStatusCodeLib
> > > > ResetSystemLib
> > > > + PrintLib
> > > >
> > > > [Guids]
> > > > gTcgEventEntryHobGuid ## PRODUCES
> ##
> > > > HOB
> > > > @@ -74,6 +75,7 @@
> > > >
> > > > [Pcd]
> > > > gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString ##
> > > > SOMETIMES_CONSUMES
> > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision
> > > ##
> > > > CONSUMES
> > > > gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid ##
> > > > CONSUMES
> > > > gEfiSecurityPkgTokenSpaceGuid.PcdTpm2InitializationPolicy ##
> > > > CONSUMES
> > > > gEfiSecurityPkgTokenSpaceGuid.PcdTpm2SelfTestPolicy ##
> > > > SOMETIMES_CONSUMES
> > > > --
> > > > 2.19.2.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event support.
2019-12-31 6:44 [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event support Yao, Jiewen
` (6 preceding siblings ...)
2020-01-02 0:11 ` [edk2-devel] [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event support Liming Gao
@ 2020-01-06 6:11 ` Wang, Jian J
7 siblings, 0 replies; 21+ messages in thread
From: Wang, Jian J @ 2020-01-06 6:11 UTC (permalink / raw)
To: devel@edk2.groups.io, Yao, Jiewen; +Cc: Zhang, Chao B
Jiewen,
Did you do tests on real platforms? I remember there're regressions caused by TCG
driver changes made months ago.
Regards,
Jian
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
> Sent: Tuesday, December 31, 2019 2:44 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Zhang, Chao B
> <chao.b.zhang@intel.com>
> Subject: [edk2-devel] [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event
> support.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2439
>
> This patch series adds TCG PFP rev 105 and 800-155 event support.
>
> TCG published Platform Firmware Profile spec revision 105
> (https://trustedcomputinggroup.org/wp-
> content/uploads/TCG_PCClient_PFP_r1p05_05_3feb20.pdf)
> and Firmware Integrity Measurement spec
> (https://trustedcomputinggroup.org/wp-content/uploads/TCG_PC_Client-
> FIM_v1r24_3feb20.pdf)
>
> 2 major impact to the BIOS:
>
> 1) Need add 800-155 event at the beginning of the TCG2 event log.
>
> 2) Need use EV_EFI_PLATFORM_FIRMWARE_BLOB2 to replace
> EV_EFI_PLATFORM_FIRMWARE_BLOB
> and use EV_EFI_HANDOFF_TABLES2 to replace EV_EFI_HANDOFF_TABLES.
>
> TCG2 DXE update: Add 800-155 event handling.
> TCG2 PEI update: Use EV_EFI_PLATFORM_FIRMWARE_BLOB2.
> Smbios Update: Use EV_EFI_HANDOFF_TABLES2.
>
> This patch series is tested on EmulatorPkg with TPM2 simulator.
> (https://github.com/jyao1/edk2/tree/feature_tpm_emulator).
>
> The new entries - 800-155 event, EV_EFI_PLATFORM_FIRMWARE_BLOB2,
> EV_EFI_HANDOFF_TABLES2,
> can be dumpped with UEFI Tcg2DumpLog tool.
> (https://github.com/jyao1/EdkiiShellTool/tree/master/EdkiiShellToolPkg/Tcg2D
> umpLog)
>
> NOTE: To support the full 800-155 requirement in TCG, a platform need report
> the
> platform specific 800-155 event.
>
> If the platform chooses to report in PEI, it can refer to the example at
> https://github.com/jyao1/edk2/tree/feature_tpm_emulator/EmulatorPkg/Tpm2
> /Platform800155EventPei.
>
> If the platform chooses to report in DXE, it can refer to the example at
> https://github.com/jyao1/edk2/tree/feature_tpm_emulator/EmulatorPkg/Tpm2
> /Platform800155EventDxe.
>
> Multiple 800-155 events are support.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
>
>
>
> Jiewen Yao (6):
> SecurityPkg/Guid: Add TCG 800-155 event GUID definition.
> SecurityPkg/Tcg2Dxe: Add Tcg2Dxe to support 800-155 event.
> MdeModulePkg/Smbios: Done measure Smbios multiple times.
> MdeModulePkg/dec: add PcdTcgPfpMeasurementRevision PCD
> MdeModulePkg/Smbios: Add TCG PFP rev 105 support.
> SecurityPkg/Tcg2Pei: Add TCG PFP 105 support.
>
> MdeModulePkg/MdeModulePkg.dec | 8 +
> .../SmbiosMeasurementDxe.c | 39 ++++-
> .../SmbiosMeasurementDxe.inf | 3 +
> SecurityPkg/Include/Guid/TcgEventHob.h | 11 ++
> SecurityPkg/SecurityPkg.dec | 4 +
> SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 157 ++++++++++++++----
> SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf | 1 +
> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 91 +++++++++-
> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 2 +
> 9 files changed, 273 insertions(+), 43 deletions(-)
>
> --
> 2.19.2.windows.1
>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2020-01-06 6:12 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-12-31 6:44 [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event support Yao, Jiewen
2019-12-31 6:44 ` [PATCH 1/6] SecurityPkg/Guid: Add TCG 800-155 event GUID definition Yao, Jiewen
2020-01-06 3:22 ` Wang, Jian J
2019-12-31 6:44 ` [PATCH 2/6] SecurityPkg/Tcg2Dxe: Add Tcg2Dxe to support 800-155 event Yao, Jiewen
2020-01-06 5:59 ` [edk2-devel] " Wang, Jian J
2019-12-31 6:44 ` [PATCH 3/6] MdeModulePkg/Smbios: Done measure Smbios multiple times Yao, Jiewen
2020-01-02 11:01 ` Zeng, Star
2019-12-31 6:44 ` [PATCH 4/6] MdeModulePkg/dec: add PcdTcgPfpMeasurementRevision PCD Yao, Jiewen
2020-01-06 3:13 ` Wang, Jian J
2019-12-31 6:44 ` [PATCH 5/6] MdeModulePkg/Smbios: Add TCG PFP rev 105 support Yao, Jiewen
2020-01-02 11:09 ` Zeng, Star
2020-01-02 14:16 ` Yao, Jiewen
2020-01-03 0:54 ` Zeng, Star
2019-12-31 6:44 ` [PATCH 6/6] SecurityPkg/Tcg2Pei: Add TCG PFP " Yao, Jiewen
2020-01-06 5:33 ` Wang, Jian J
2020-01-06 5:53 ` Yao, Jiewen
2020-01-06 5:57 ` Wang, Jian J
2020-01-06 6:00 ` Yao, Jiewen
2020-01-02 0:11 ` [edk2-devel] [PATCH 0/6] TCG: Add TCG PFP rev 105 and 800-155 event support Liming Gao
2020-01-02 0:39 ` Yao, Jiewen
2020-01-06 6:11 ` Wang, Jian J
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox