Re: [PATCH v2 3/9] SecurityPkg/DxeTpmMeasurementLib: Add new API.


Wang, Jian J
 

Qi,

Comments below.

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Qi Zhang
Sent: Thursday, August 06, 2020 8:34 AM
To: devel@edk2.groups.io
Cc: Yao, Jiewen <jiewen.yao@...>; Wang, Jian J <jian.j.wang@...>;
Zhang, Qi1 <qi1.zhang@...>
Subject: [edk2-devel] [PATCH v2 3/9] SecurityPkg/DxeTpmMeasurementLib: Add
new API.

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

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

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

diff --git
a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
index 7d41bc41f9..39448f8ee8 100644
--- a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
+++
b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
@@ -4,7 +4,7 @@
# This library provides TpmMeasureAndLogData() to measure and log data, and

# extend the measurement result into a specific PCR.

#

-# Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>

+# Copyright (c) 2012 - 2020, Intel Corporation. All rights reserved.<BR>

# SPDX-License-Identifier: BSD-2-Clause-Patent

#

##

@@ -26,6 +26,7 @@


[Sources]

DxeTpmMeasurementLib.c

+ EventLogRecord.c



[Packages]

MdePkg/MdePkg.dec

@@ -42,3 +43,6 @@
[Protocols]

gEfiTcgProtocolGuid ## SOMETIMES_CONSUMES

gEfiTcg2ProtocolGuid ## SOMETIMES_CONSUMES

+

+[Pcd]

+ gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision ##
CONSUMES

diff --git a/SecurityPkg/Library/DxeTpmMeasurementLib/EventLogRecord.c
b/SecurityPkg/Library/DxeTpmMeasurementLib/EventLogRecord.c
new file mode 100644
index 0000000000..7b3726e44b
--- /dev/null
+++ b/SecurityPkg/Library/DxeTpmMeasurementLib/EventLogRecord.c
@@ -0,0 +1,218 @@
+/** @file

+ This library is used by other modules to measure data to TPM.

+

+Copyright (c) 2020, Intel Corporation. All rights reserved. <BR>

+SPDX-License-Identifier: BSD-2-Clause-Patent

+

+**/

+

+#include <PiDxe.h>

+

+#include <Library/BaseMemoryLib.h>

+#include <Library/DebugLib.h>

+#include <Library/ReportStatusCodeLib.h>

+#include <Library/HobLib.h>

+#include <Library/PcdLib.h>

+#include <Library/PrintLib.h>

+#include <Library/TpmMeasurementLib.h>

+

+#include <IndustryStandard/UefiTcgPlatform.h>

+

+#pragma pack (1)

+

+#define PLATFORM_FIRMWARE_BLOB_DESC "Fv(XXXXXXXX-XXXX-XXXX-XXXX-
XXXXXXXXXXXX)"

+typedef struct {

+ UINT8 BlobDescriptionSize;

+ UINT8
BlobDescription[sizeof(PLATFORM_FIRMWARE_BLOB_DESC)];

+ EFI_PHYSICAL_ADDRESS BlobBase;

+ UINT64 BlobLength;

+} PLATFORM_FIRMWARE_BLOB2_STRUCT;

+

+#define HANDOFF_TABLE_POINTER_DESC "1234567890ABCDEF"

+typedef struct {

+ UINT8 TableDescriptionSize;

+ UINT8
TableDescription[sizeof(HANDOFF_TABLE_POINTER_DESC)];

+ UINT64 NumberOfTables;

+ EFI_CONFIGURATION_TABLE TableEntry[1];

+} HANDOFF_TABLE_POINTERS2_STRUCT;

+

+#pragma pack ()

+

+/**

+ Get the FvName from the FV header.

+

+ Causion: The FV is untrusted input.

+

+ @param[in] FvBase Base address of FV image.

+ @param[in] FvLength Length of FV image.

+

+ @return FvName pointer

+ @retval NULL FvName is NOT found

+**/

+VOID *

+TpmMeasurementGetFvName (

+ IN EFI_PHYSICAL_ADDRESS FvBase,

+ IN UINT64 FvLength

+ )

+{

+ EFI_FIRMWARE_VOLUME_HEADER *FvHeader;

+ EFI_FIRMWARE_VOLUME_EXT_HEADER *FvExtHeader;

+

+ if (FvBase >= MAX_ADDRESS) {

+ return NULL;

+ }

+ if (FvLength >= MAX_ADDRESS - FvBase) {

+ return NULL;

+ }

+ if (FvLength < sizeof(EFI_FIRMWARE_VOLUME_HEADER)) {

+ return NULL;

+ }

+

+ FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)FvBase;

+ if (FvHeader->Signature != EFI_FVH_SIGNATURE) {

+ return NULL;

+ }

+ if (FvHeader->ExtHeaderOffset < sizeof(EFI_FIRMWARE_VOLUME_HEADER)) {

+ return NULL;

+ }

+ if (FvHeader->ExtHeaderOffset +
sizeof(EFI_FIRMWARE_VOLUME_EXT_HEADER) > FvLength) {

+ return NULL;

+ }

+ FvExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *)(UINTN)(FvBase +
FvHeader->ExtHeaderOffset);

+

+ return &FvExtHeader->FvName;

+}

+

+/**

+ Mesure a FirmwareBlob.
'Mesure' -> 'Measure'


+

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


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


+ @param[in] FirmwareBlobBase Base address of this FirmwareBlob.

+ @param[in] FirmwareBlobLength Size in bytes of this FirmwareBlob.

+

+ @retval EFI_SUCCESS Operation completed successfully.

+ @retval EFI_UNSUPPORTED TPM device not available.

+ @retval EFI_OUT_OF_RESOURCES Out of memory.

+ @retval EFI_DEVICE_ERROR The operation was unsuccessful.

+*/

+EFI_STATUS

+EFIAPI

+MeasureFirmwareBlob (

+ IN UINT32 PcrIndex,

+ IN CHAR8 *Description OPTIONAL,

+ IN EFI_PHYSICAL_ADDRESS FirmwareBlobBase,

+ IN UINT64 FirmwareBlobLength

+ )

+{

+ EFI_PLATFORM_FIRMWARE_BLOB FvBlob;

+ PLATFORM_FIRMWARE_BLOB2_STRUCT FvBlob2;

+ VOID *FvName;

+ UINT32 EventType;

+ VOID *EventLog;

+ UINT32 EventLogSize;

+ EFI_STATUS Status;

+

+ FvName = TpmMeasurementGetFvName (FirmwareBlobBase,
FirmwareBlobLength);

+

+ if (((Description != NULL) || (FvName != NULL)) &&

+ (PcdGet32(PcdTcgPfpMeasurementRevision) >=
TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105)) {

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


+ if (Description != NULL) {

+ AsciiSPrint((CHAR8*)FvBlob2.BlobDescription,
sizeof(FvBlob2.BlobDescription), "%a", Description);

+ } else {

+ AsciiSPrint((CHAR8*)FvBlob2.BlobDescription,
sizeof(FvBlob2.BlobDescription), "Fv(%g)", FvName);

+ }

+

+ FvBlob2.BlobDescriptionSize = sizeof(FvBlob2.BlobDescription);

+ FvBlob2.BlobBase = FirmwareBlobBase;

+ FvBlob2.BlobLength = FirmwareBlobLength;

+

+ EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB2;

+ EventLog = &FvBlob2;

+ EventLogSize = sizeof(FvBlob2);

+ } else {

+ FvBlob.BlobBase = FirmwareBlobBase;

+ FvBlob.BlobLength = FirmwareBlobLength;

+

+ EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB;

+ EventLog = &FvBlob;

+ EventLogSize = sizeof(FvBlob);

+ }

+

+ Status = TpmMeasureAndLogData (

+ PcrIndex,

+ EventType,

+ EventLog,

+ EventLogSize,

+ (VOID*)(UINTN)FirmwareBlobBase,

+ FirmwareBlobLength

+ );

+

+ return Status;

+}

+

+/**

+ Mesure a HandoffTable.
'Mesure' -> 'Measure'


+

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


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


+ @param[in] TableGuid GUID of this HandoffTable.

+ @param[in] TableAddress Base address of this HandoffTable.

+ @param[in] TableLength Size in bytes of this HandoffTable.

+

+ @retval EFI_SUCCESS Operation completed successfully.

+ @retval EFI_UNSUPPORTED TPM device not available.

+ @retval EFI_OUT_OF_RESOURCES Out of memory.

+ @retval EFI_DEVICE_ERROR The operation was unsuccessful.

+*/

+EFI_STATUS

+EFIAPI

+MeasureHandoffTable (

+ IN UINT32 PcrIndex,

+ IN CHAR8 *Description OPTIONAL,

+ IN EFI_GUID *TableGuid,

+ IN VOID *TableAddress,

+ IN UINTN TableLength

+ )

+{

+ EFI_HANDOFF_TABLE_POINTERS HandoffTables;

+ HANDOFF_TABLE_POINTERS2_STRUCT HandoffTables2;

+ UINT32 EventType;

+ VOID *EventLog;

+ UINT32 EventLogSize;

+ EFI_STATUS Status;

+

+ if ((Description != NULL) &&

+ (PcdGet32(PcdTcgPfpMeasurementRevision) >=
TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2_REV_105)) {

+ ZeroMem (&HandoffTables2, sizeof(HandoffTables2));
The same as before. I see no reason to clear the data in advance.


+ AsciiSPrint((CHAR8*)HandoffTables2.TableDescription,
sizeof(HandoffTables2.TableDescription), "%a", Description);

+

+ HandoffTables2.TableDescriptionSize =
sizeof(HandoffTables2.TableDescription);

+ HandoffTables2.NumberOfTables = 1;

+ CopyGuid (&(HandoffTables2.TableEntry[0].VendorGuid), TableGuid);

+ HandoffTables2.TableEntry[0].VendorTable = TableAddress;

+

+ EventType = EV_EFI_HANDOFF_TABLES2;

+ EventLog = &HandoffTables2;

+ EventLogSize = sizeof(HandoffTables2);

+ } else {

+ HandoffTables.NumberOfTables = 1;

+ CopyGuid (&(HandoffTables.TableEntry[0].VendorGuid), TableGuid);

+ HandoffTables.TableEntry[0].VendorTable = TableAddress;

+

+ EventType = EV_EFI_HANDOFF_TABLES;

+ EventLog = &HandoffTables;

+ EventLogSize = sizeof(HandoffTables);

+ }

+

+ Status = TpmMeasureAndLogData (

+ PcrIndex,

+ EventType,

+ EventLog,

+ EventLogSize,

+ TableAddress,

+ TableLength

+ );

+ return Status;

+}

--
2.26.2.windows.1


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

View/Reply Online (#63754): https://edk2.groups.io/g/devel/message/63754
Mute This Topic: https://groups.io/mt/76019584/1768734
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [jian.j.wang@...]
-=-=-=-=-=-=

Join devel@edk2.groups.io to automatically receive all group messages.