* [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
@ 2024-01-11 18:16 Doug Flick via groups.io
2024-01-11 18:16 ` [edk2-devel] [PATCH 1/6] SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4117 - CVE 2022-36763 Doug Flick via groups.io
` (8 more replies)
0 siblings, 9 replies; 21+ messages in thread
From: Doug Flick via groups.io @ 2024-01-11 18:16 UTC (permalink / raw)
To: devel; +Cc: Douglas Flick [MSFT], Jiewen Yao
This patch series include the combined / merged security patches
(as seperate commits) for TCBZ4117 (CVE-2022-36763) and TCBZ4118
(CVE-2022-36764) for DxeTpm2MeasureBootLib and DxeTpmMeasureBootLib.
These patches have already been reviewed by SecurityPkg Maintainer
(Jiewen) on GHSA.
This patch series (specifically TCBZ4117) supersedes TCBZ2168.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Douglas Flick [MSFT] (6):
SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4117 - CVE
2022-36763
SecurityPkg: DxeTpmMeasureBootLib: SECURITY PATCH 4117 - CVE
2022-36763
SecurityPkg: : Adding CVE 2022-36763 to SecurityFixes.yaml
SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4118 - CVE
2022-36764
SecurityPkg: DxeTpmMeasureBootLib: SECURITY PATCH 4118 - CVE
2022-36764
SecurityPkg: : Adding CVE 2022-36764 to SecurityFixes.yaml
SecurityPkg/Test/SecurityPkgHostTest.dsc | 2 +
.../DxeTpm2MeasureBootLib.inf | 4 +-
...Tpm2MeasureBootLibSanitizationTestHost.inf | 28 ++
.../DxeTpmMeasureBootLib.inf | 4 +-
...eTpmMeasureBootLibSanitizationTestHost.inf | 28 ++
.../DxeTpm2MeasureBootLibSanitization.h | 139 +++++++
.../DxeTpmMeasureBootLibSanitization.h | 137 +++++++
.../DxeTpm2MeasureBootLib.c | 87 ++--
.../DxeTpm2MeasureBootLibSanitization.c | 319 +++++++++++++++
.../DxeTpm2MeasureBootLibSanitizationTest.c | 345 ++++++++++++++++
.../DxeTpmMeasureBootLib.c | 53 ++-
.../DxeTpmMeasureBootLibSanitization.c | 285 +++++++++++++
.../DxeTpmMeasureBootLibSanitizationTest.c | 387 ++++++++++++++++++
SecurityPkg/SecurityFixes.yaml | 36 ++
SecurityPkg/SecurityPkg.ci.yaml | 2 +
15 files changed, 1801 insertions(+), 55 deletions(-)
create mode 100644 SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTestHost.inf
create mode 100644 SecurityPkg/Library/DxeTpmMeasureBootLib/InternalUnitTest/DxeTpmMeasureBootLibSanitizationTestHost.inf
create mode 100644 SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.h
create mode 100644 SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.h
create mode 100644 SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.c
create mode 100644 SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTest.c
create mode 100644 SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.c
create mode 100644 SecurityPkg/Library/DxeTpmMeasureBootLib/InternalUnitTest/DxeTpmMeasureBootLibSanitizationTest.c
create mode 100644 SecurityPkg/SecurityFixes.yaml
--
2.43.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113756): https://edk2.groups.io/g/devel/message/113756
Mute This Topic: https://groups.io/mt/103675434/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 21+ messages in thread
* [edk2-devel] [PATCH 1/6] SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4117 - CVE 2022-36763
2024-01-11 18:16 [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118 Doug Flick via groups.io
@ 2024-01-11 18:16 ` Doug Flick via groups.io
2024-01-11 18:16 ` [edk2-devel] [PATCH 2/6] SecurityPkg: DxeTpmMeasureBootLib: " Doug Flick via groups.io
` (7 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Doug Flick via groups.io @ 2024-01-11 18:16 UTC (permalink / raw)
To: devel; +Cc: Douglas Flick [MSFT], Jiewen Yao
This commit contains the patch files and tests for DxeTpm2MeasureBootLib
CVE 2022-36763.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com>
---
SecurityPkg/Test/SecurityPkgHostTest.dsc | 1 +
.../DxeTpm2MeasureBootLib.inf | 4 +-
...Tpm2MeasureBootLibSanitizationTestHost.inf | 28 ++
.../DxeTpm2MeasureBootLibSanitization.h | 113 +++++++
.../DxeTpm2MeasureBootLib.c | 75 +++--
.../DxeTpm2MeasureBootLibSanitization.c | 275 ++++++++++++++++
.../DxeTpm2MeasureBootLibSanitizationTest.c | 303 ++++++++++++++++++
SecurityPkg/SecurityPkg.ci.yaml | 1 +
8 files changed, 767 insertions(+), 33 deletions(-)
create mode 100644 SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTestHost.inf
create mode 100644 SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.h
create mode 100644 SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.c
create mode 100644 SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTest.c
diff --git a/SecurityPkg/Test/SecurityPkgHostTest.dsc b/SecurityPkg/Test/SecurityPkgHostTest.dsc
index ad5b4fc350ea..788c1ab6fec6 100644
--- a/SecurityPkg/Test/SecurityPkgHostTest.dsc
+++ b/SecurityPkg/Test/SecurityPkgHostTest.dsc
@@ -26,6 +26,7 @@ [Components]
SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockPlatformPKProtectionLib.inf
SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiLib.inf
SecurityPkg/Test/Mock/Library/GoogleTest/MockPlatformPKProtectionLib/MockPlatformPKProtectionLib.inf
+ SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTestHost.inf
#
# Build SecurityPkg HOST_APPLICATION Tests
diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
index 6dca79a20c93..28995f438de6 100644
--- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
+++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
@@ -37,6 +37,8 @@ [Defines]
[Sources]
DxeTpm2MeasureBootLib.c
+ DxeTpm2MeasureBootLibSanitization.c
+ DxeTpm2MeasureBootLibSanitization.h
[Packages]
MdePkg/MdePkg.dec
@@ -46,6 +48,7 @@ [Packages]
[LibraryClasses]
BaseMemoryLib
+ SafeIntLib
DebugLib
MemoryAllocationLib
DevicePathLib
@@ -65,4 +68,3 @@ [Protocols]
gEfiFirmwareVolumeBlockProtocolGuid ## SOMETIMES_CONSUMES
gEfiBlockIoProtocolGuid ## SOMETIMES_CONSUMES
gEfiDiskIoProtocolGuid ## SOMETIMES_CONSUMES
-
diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTestHost.inf b/SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTestHost.inf
new file mode 100644
index 000000000000..2999aa2a44e0
--- /dev/null
+++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTestHost.inf
@@ -0,0 +1,28 @@
+## @file
+# This file builds the unit tests for DxeTpm2MeasureBootLib
+#
+# Copyright (C) Microsoft Corporation.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+ INF_VERSION = 0x00010006
+ BASE_NAME = DxeTpm2MeasuredBootLibTest
+ FILE_GUID = 144d757f-d423-484e-9309-a23695fad5bd
+ MODULE_TYPE = HOST_APPLICATION
+ VERSION_STRING = 1.0
+ ENTRY_POINT = main
+
+[Sources]
+ DxeTpm2MeasureBootLibSanitizationTest.c
+ ../DxeTpm2MeasureBootLibSanitization.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ BaseLib
+ DebugLib
+ UnitTestLib
+ PrintLib
+ SafeIntLib
diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.h b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.h
new file mode 100644
index 000000000000..048b73898744
--- /dev/null
+++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.h
@@ -0,0 +1,113 @@
+/** @file
+ This file includes the function prototypes for the sanitization functions.
+
+ These are those functions:
+
+ DxeTpm2MeasureBootLibImageRead() function will make sure the PE/COFF image content
+ read is within the image buffer.
+
+ Tcg2MeasureGptTable() function will receive untrusted GPT partition table, and parse
+ partition data carefully.
+
+ Copyright (c) Microsoft Corporation.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef DXE_TPM2_MEASURE_BOOT_LIB_SANITATION_
+#define DXE_TPM2_MEASURE_BOOT_LIB_SANITATION_
+
+#include <Uefi.h>
+#include <Uefi/UefiSpec.h>
+#include <Protocol/BlockIo.h>
+#include <IndustryStandard/UefiTcgPlatform.h>
+#include <Protocol/Tcg2Protocol.h>
+
+/**
+ This function will validate the EFI_PARTITION_TABLE_HEADER structure is safe to parse
+ However this function will not attempt to verify the validity of the GPT partition
+ It will check the following:
+ - Signature
+ - Revision
+ - AlternateLBA
+ - FirstUsableLBA
+ - LastUsableLBA
+ - PartitionEntryLBA
+ - NumberOfPartitionEntries
+ - SizeOfPartitionEntry
+ - BlockIo
+
+ @param[in] PrimaryHeader
+ Pointer to the EFI_PARTITION_TABLE_HEADER structure.
+
+ @param[in] BlockIo
+ Pointer to the EFI_BLOCK_IO_PROTOCOL structure.
+
+ @retval EFI_SUCCESS
+ The EFI_PARTITION_TABLE_HEADER structure is valid.
+
+ @retval EFI_INVALID_PARAMETER
+ The EFI_PARTITION_TABLE_HEADER structure is invalid.
+**/
+EFI_STATUS
+EFIAPI
+SanitizeEfiPartitionTableHeader (
+ IN CONST EFI_PARTITION_TABLE_HEADER *PrimaryHeader,
+ IN CONST EFI_BLOCK_IO_PROTOCOL *BlockIo
+ );
+
+/**
+ This function will validate that the allocation size from the primary header is sane
+ It will check the following:
+ - AllocationSize does not overflow
+
+ @param[in] PrimaryHeader
+ Pointer to the EFI_PARTITION_TABLE_HEADER structure.
+
+ @param[out] AllocationSize
+ Pointer to the allocation size.
+
+ @retval EFI_SUCCESS
+ The allocation size is valid.
+
+ @retval EFI_OUT_OF_RESOURCES
+ The allocation size is invalid.
+**/
+EFI_STATUS
+EFIAPI
+SanitizePrimaryHeaderAllocationSize (
+ IN CONST EFI_PARTITION_TABLE_HEADER *PrimaryHeader,
+ OUT UINT32 *AllocationSize
+ );
+
+/**
+ This function will validate that the Gpt Event Size calculated from the primary header is sane
+ It will check the following:
+ - EventSize does not overflow
+
+ Important: This function includes the entire length of the allocated space, including
+ (sizeof (EFI_TCG2_EVENT) - sizeof (Tcg2Event->Event)) . When hashing the buffer allocated with this
+ size, the caller must subtract the size of the (sizeof (EFI_TCG2_EVENT) - sizeof (Tcg2Event->Event))
+ from the size of the buffer before hashing.
+
+ @param[in] PrimaryHeader - Pointer to the EFI_PARTITION_TABLE_HEADER structure.
+ @param[in] NumberOfPartition - Number of partitions.
+ @param[out] EventSize - Pointer to the event size.
+
+ @retval EFI_SUCCESS
+ The event size is valid.
+
+ @retval EFI_OUT_OF_RESOURCES
+ Overflow would have occurred.
+
+ @retval EFI_INVALID_PARAMETER
+ One of the passed parameters was invalid.
+**/
+EFI_STATUS
+SanitizePrimaryHeaderGptEventSize (
+ IN CONST EFI_PARTITION_TABLE_HEADER *PrimaryHeader,
+ IN UINTN NumberOfPartition,
+ OUT UINT32 *EventSize
+ );
+
+#endif // DXE_TPM2_MEASURE_BOOT_LIB_SANITATION_
diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
index 36a256a7af50..0475103d6ef8 100644
--- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
+++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
@@ -20,6 +20,8 @@ Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
+Copyright (c) Microsoft Corporation.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
**/
#include <PiDxe.h>
@@ -44,6 +46,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/HobLib.h>
#include <Protocol/CcMeasurement.h>
+#include "DxeTpm2MeasureBootLibSanitization.h"
+
typedef struct {
EFI_TCG2_PROTOCOL *Tcg2Protocol;
EFI_CC_MEASUREMENT_PROTOCOL *CcProtocol;
@@ -144,10 +148,11 @@ Tcg2MeasureGptTable (
EFI_TCG2_EVENT *Tcg2Event;
EFI_CC_EVENT *CcEvent;
EFI_GPT_DATA *GptData;
- UINT32 EventSize;
+ UINT32 TcgEventSize;
EFI_TCG2_PROTOCOL *Tcg2Protocol;
EFI_CC_MEASUREMENT_PROTOCOL *CcProtocol;
EFI_CC_MR_INDEX MrIndex;
+ UINT32 AllocSize;
if (mTcg2MeasureGptCount > 0) {
return EFI_SUCCESS;
@@ -195,25 +200,22 @@ Tcg2MeasureGptTable (
BlockIo->Media->BlockSize,
(UINT8 *)PrimaryHeader
);
- if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "Failed to Read Partition Table Header!\n"));
+ if (EFI_ERROR (Status) || EFI_ERROR (SanitizeEfiPartitionTableHeader (PrimaryHeader, BlockIo))) {
+ DEBUG ((DEBUG_ERROR, "Failed to read Partition Table Header or invalid Partition Table Header!\n"));
FreePool (PrimaryHeader);
return EFI_DEVICE_ERROR;
}
- //
- // PrimaryHeader->SizeOfPartitionEntry should not be zero
- //
- if (PrimaryHeader->SizeOfPartitionEntry == 0) {
- DEBUG ((DEBUG_ERROR, "SizeOfPartitionEntry should not be zero!\n"));
- FreePool (PrimaryHeader);
- return EFI_BAD_BUFFER_SIZE;
- }
-
//
// Read the partition entry.
//
- EntryPtr = (UINT8 *)AllocatePool (PrimaryHeader->NumberOfPartitionEntries * PrimaryHeader->SizeOfPartitionEntry);
+ Status = SanitizePrimaryHeaderAllocationSize (PrimaryHeader, &AllocSize);
+ if (EFI_ERROR (Status)) {
+ FreePool (PrimaryHeader);
+ return EFI_BAD_BUFFER_SIZE;
+ }
+
+ EntryPtr = (UINT8 *)AllocatePool (AllocSize);
if (EntryPtr == NULL) {
FreePool (PrimaryHeader);
return EFI_OUT_OF_RESOURCES;
@@ -223,7 +225,7 @@ Tcg2MeasureGptTable (
DiskIo,
BlockIo->Media->MediaId,
MultU64x32 (PrimaryHeader->PartitionEntryLBA, BlockIo->Media->BlockSize),
- PrimaryHeader->NumberOfPartitionEntries * PrimaryHeader->SizeOfPartitionEntry,
+ AllocSize,
EntryPtr
);
if (EFI_ERROR (Status)) {
@@ -248,16 +250,21 @@ Tcg2MeasureGptTable (
//
// Prepare Data for Measurement (CcProtocol and Tcg2Protocol)
//
- EventSize = (UINT32)(sizeof (EFI_GPT_DATA) - sizeof (GptData->Partitions)
- + NumberOfPartition * PrimaryHeader->SizeOfPartitionEntry);
- EventPtr = (UINT8 *)AllocateZeroPool (EventSize + sizeof (EFI_TCG2_EVENT) - sizeof (Tcg2Event->Event));
+ Status = SanitizePrimaryHeaderGptEventSize (PrimaryHeader, NumberOfPartition, &TcgEventSize);
+ if (EFI_ERROR (Status)) {
+ FreePool (PrimaryHeader);
+ FreePool (EntryPtr);
+ return EFI_DEVICE_ERROR;
+ }
+
+ EventPtr = (UINT8 *)AllocateZeroPool (TcgEventSize);
if (EventPtr == NULL) {
Status = EFI_OUT_OF_RESOURCES;
goto Exit;
}
Tcg2Event = (EFI_TCG2_EVENT *)EventPtr;
- Tcg2Event->Size = EventSize + sizeof (EFI_TCG2_EVENT) - sizeof (Tcg2Event->Event);
+ Tcg2Event->Size = TcgEventSize;
Tcg2Event->Header.HeaderSize = sizeof (EFI_TCG2_EVENT_HEADER);
Tcg2Event->Header.HeaderVersion = EFI_TCG2_EVENT_HEADER_VERSION;
Tcg2Event->Header.PCRIndex = 5;
@@ -310,7 +317,7 @@ Tcg2MeasureGptTable (
CcProtocol,
0,
(EFI_PHYSICAL_ADDRESS)(UINTN)(VOID *)GptData,
- (UINT64)EventSize,
+ (UINT64)TcgEventSize - OFFSET_OF (EFI_TCG2_EVENT, Event),
CcEvent
);
if (!EFI_ERROR (Status)) {
@@ -326,7 +333,7 @@ Tcg2MeasureGptTable (
Tcg2Protocol,
0,
(EFI_PHYSICAL_ADDRESS)(UINTN)(VOID *)GptData,
- (UINT64)EventSize,
+ (UINT64)TcgEventSize - OFFSET_OF (EFI_TCG2_EVENT, Event),
Tcg2Event
);
if (!EFI_ERROR (Status)) {
@@ -443,11 +450,13 @@ Tcg2MeasurePeImage (
Tcg2Event->Header.PCRIndex = 2;
break;
default:
- DEBUG ((
- DEBUG_ERROR,
- "Tcg2MeasurePeImage: Unknown subsystem type %d",
- ImageType
- ));
+ DEBUG (
+ (
+ DEBUG_ERROR,
+ "Tcg2MeasurePeImage: Unknown subsystem type %d",
+ ImageType
+ )
+ );
goto Finish;
}
@@ -515,7 +524,7 @@ Finish:
@param MeasureBootProtocols Pointer to the located measure boot protocol instances.
- @retval EFI_SUCCESS Sucessfully locate the measure boot protocol instances (at least one instance).
+ @retval EFI_SUCCESS Successfully locate the measure boot protocol instances (at least one instance).
@retval EFI_UNSUPPORTED Measure boot is not supported.
**/
EFI_STATUS
@@ -646,12 +655,14 @@ DxeTpm2MeasureBootHandler (
return EFI_SUCCESS;
}
- DEBUG ((
- DEBUG_INFO,
- "Tcg2Protocol = %p, CcMeasurementProtocol = %p\n",
- MeasureBootProtocols.Tcg2Protocol,
- MeasureBootProtocols.CcProtocol
- ));
+ DEBUG (
+ (
+ DEBUG_INFO,
+ "Tcg2Protocol = %p, CcMeasurementProtocol = %p\n",
+ MeasureBootProtocols.Tcg2Protocol,
+ MeasureBootProtocols.CcProtocol
+ )
+ );
//
// Copy File Device Path
diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.c b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.c
new file mode 100644
index 000000000000..e2309655d384
--- /dev/null
+++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.c
@@ -0,0 +1,275 @@
+/** @file
+ The library instance provides security service of TPM2 measure boot and
+ Confidential Computing (CC) measure boot.
+
+ Caution: This file requires additional review when modified.
+ This library will have external input - PE/COFF image and GPT partition.
+ This external input must be validated carefully to avoid security issue like
+ buffer overflow, integer overflow.
+
+ This file will pull out the validation logic from the following functions, in an
+ attempt to validate the untrusted input in the form of unit tests
+
+ These are those functions:
+
+ DxeTpm2MeasureBootLibImageRead() function will make sure the PE/COFF image content
+ read is within the image buffer.
+
+ Tcg2MeasureGptTable() function will receive untrusted GPT partition table, and parse
+ partition data carefully.
+
+ Copyright (c) Microsoft Corporation.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+#include <Uefi.h>
+#include <Uefi/UefiSpec.h>
+#include <Library/SafeIntLib.h>
+#include <Library/UefiLib.h>
+#include <Library/DebugLib.h>
+#include <Library/BaseLib.h>
+#include <IndustryStandard/UefiTcgPlatform.h>
+#include <Protocol/BlockIo.h>
+#include <Library/MemoryAllocationLib.h>
+
+#include "DxeTpm2MeasureBootLibSanitization.h"
+
+#define GPT_HEADER_REVISION_V1 0x00010000
+
+/**
+ This function will validate the EFI_PARTITION_TABLE_HEADER structure is safe to parse
+ However this function will not attempt to verify the validity of the GPT partition
+ It will check the following:
+ - Signature
+ - Revision
+ - AlternateLBA
+ - FirstUsableLBA
+ - LastUsableLBA
+ - PartitionEntryLBA
+ - NumberOfPartitionEntries
+ - SizeOfPartitionEntry
+ - BlockIo
+
+ @param[in] PrimaryHeader
+ Pointer to the EFI_PARTITION_TABLE_HEADER structure.
+
+ @param[in] BlockIo
+ Pointer to the EFI_BLOCK_IO_PROTOCOL structure.
+
+ @retval EFI_SUCCESS
+ The EFI_PARTITION_TABLE_HEADER structure is valid.
+
+ @retval EFI_INVALID_PARAMETER
+ The EFI_PARTITION_TABLE_HEADER structure is invalid.
+**/
+EFI_STATUS
+EFIAPI
+SanitizeEfiPartitionTableHeader (
+ IN CONST EFI_PARTITION_TABLE_HEADER *PrimaryHeader,
+ IN CONST EFI_BLOCK_IO_PROTOCOL *BlockIo
+ )
+{
+ //
+ // Verify that the input parameters are safe to use
+ //
+ if (PrimaryHeader == NULL) {
+ DEBUG ((DEBUG_ERROR, "Invalid Partition Table Header!\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if ((BlockIo == NULL) || (BlockIo->Media == NULL)) {
+ DEBUG ((DEBUG_ERROR, "Invalid BlockIo!\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // The signature must be EFI_PTAB_HEADER_ID ("EFI PART" in ASCII)
+ //
+ if (PrimaryHeader->Header.Signature != EFI_PTAB_HEADER_ID) {
+ DEBUG ((DEBUG_ERROR, "Invalid Partition Table Header!\n"));
+ return EFI_DEVICE_ERROR;
+ }
+
+ //
+ // The version must be GPT_HEADER_REVISION_V1 (0x00010000)
+ //
+ if (PrimaryHeader->Header.Revision != GPT_HEADER_REVISION_V1) {
+ DEBUG ((DEBUG_ERROR, "Invalid Partition Table Header Revision!\n"));
+ return EFI_DEVICE_ERROR;
+ }
+
+ //
+ // The HeaderSize must be greater than or equal to 92 and must be less than or equal to the logical block size
+ //
+ if ((PrimaryHeader->Header.HeaderSize < sizeof (EFI_PARTITION_TABLE_HEADER)) || (PrimaryHeader->Header.HeaderSize > BlockIo->Media->BlockSize)) {
+ DEBUG ((DEBUG_ERROR, "Invalid Partition Table Header HeaderSize!\n"));
+ return EFI_DEVICE_ERROR;
+ }
+
+ //
+ // The partition entries should all be before the first usable block
+ //
+ if (PrimaryHeader->FirstUsableLBA <= PrimaryHeader->PartitionEntryLBA) {
+ DEBUG ((DEBUG_ERROR, "GPT PartitionEntryLBA is not less than FirstUsableLBA!\n"));
+ return EFI_DEVICE_ERROR;
+ }
+
+ //
+ // Check that the PartitionEntryLBA greater than the Max LBA
+ // This will be used later for multiplication
+ //
+ if (PrimaryHeader->PartitionEntryLBA > DivU64x32 (MAX_UINT64, BlockIo->Media->BlockSize)) {
+ DEBUG ((DEBUG_ERROR, "Invalid Partition Table Header PartitionEntryLBA!\n"));
+ return EFI_DEVICE_ERROR;
+ }
+
+ //
+ // Check that the number of partition entries is greater than zero
+ //
+ if (PrimaryHeader->NumberOfPartitionEntries == 0) {
+ DEBUG ((DEBUG_ERROR, "Invalid Partition Table Header NumberOfPartitionEntries!\n"));
+ return EFI_DEVICE_ERROR;
+ }
+
+ //
+ // SizeOfPartitionEntry must be 128, 256, 512... improper size may lead to accessing uninitialized memory
+ //
+ if ((PrimaryHeader->SizeOfPartitionEntry < 128) || ((PrimaryHeader->SizeOfPartitionEntry & (PrimaryHeader->SizeOfPartitionEntry - 1)) != 0)) {
+ DEBUG ((DEBUG_ERROR, "SizeOfPartitionEntry shall be set to a value of 128 x 2^n where n is an integer greater than or equal to zero (e.g., 128, 256, 512, etc.)!\n"));
+ return EFI_DEVICE_ERROR;
+ }
+
+ //
+ // This check is to prevent overflow when calculating the allocation size for the partition entries
+ // This check will be used later for multiplication
+ //
+ if (PrimaryHeader->NumberOfPartitionEntries > DivU64x32 (MAX_UINT64, PrimaryHeader->SizeOfPartitionEntry)) {
+ DEBUG ((DEBUG_ERROR, "Invalid Partition Table Header NumberOfPartitionEntries!\n"));
+ return EFI_DEVICE_ERROR;
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
+ This function will validate that the allocation size from the primary header is sane
+ It will check the following:
+ - AllocationSize does not overflow
+
+ @param[in] PrimaryHeader
+ Pointer to the EFI_PARTITION_TABLE_HEADER structure.
+
+ @param[out] AllocationSize
+ Pointer to the allocation size.
+
+ @retval EFI_SUCCESS
+ The allocation size is valid.
+
+ @retval EFI_OUT_OF_RESOURCES
+ The allocation size is invalid.
+**/
+EFI_STATUS
+EFIAPI
+SanitizePrimaryHeaderAllocationSize (
+ IN CONST EFI_PARTITION_TABLE_HEADER *PrimaryHeader,
+ OUT UINT32 *AllocationSize
+ )
+{
+ EFI_STATUS Status;
+
+ if (PrimaryHeader == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (AllocationSize == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // Replacing logic:
+ // PrimaryHeader->NumberOfPartitionEntries * PrimaryHeader->SizeOfPartitionEntry;
+ //
+ Status = SafeUint32Mult (PrimaryHeader->NumberOfPartitionEntries, PrimaryHeader->SizeOfPartitionEntry, AllocationSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Allocation Size would have overflowed!\n"));
+ return EFI_BAD_BUFFER_SIZE;
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
+ This function will validate that the Gpt Event Size calculated from the primary header is sane
+ It will check the following:
+ - EventSize does not overflow
+
+ Important: This function includes the entire length of the allocated space, including
+ (sizeof (EFI_TCG2_EVENT) - sizeof (Tcg2Event->Event)) . When hashing the buffer allocated with this
+ size, the caller must subtract the size of the (sizeof (EFI_TCG2_EVENT) - sizeof (Tcg2Event->Event))
+ from the size of the buffer before hashing.
+
+ @param[in] PrimaryHeader - Pointer to the EFI_PARTITION_TABLE_HEADER structure.
+ @param[in] NumberOfPartition - Number of partitions.
+ @param[out] EventSize - Pointer to the event size.
+
+ @retval EFI_SUCCESS
+ The event size is valid.
+
+ @retval EFI_OUT_OF_RESOURCES
+ Overflow would have occurred.
+
+ @retval EFI_INVALID_PARAMETER
+ One of the passed parameters was invalid.
+**/
+EFI_STATUS
+SanitizePrimaryHeaderGptEventSize (
+ IN CONST EFI_PARTITION_TABLE_HEADER *PrimaryHeader,
+ IN UINTN NumberOfPartition,
+ OUT UINT32 *EventSize
+ )
+{
+ EFI_STATUS Status;
+ UINT32 SafeNumberOfPartitions;
+
+ if (PrimaryHeader == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (EventSize == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // We shouldn't even attempt to perform the multiplication if the number of partitions is greater than the maximum value of UINT32
+ //
+ Status = SafeUintnToUint32 (NumberOfPartition, &SafeNumberOfPartitions);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "NumberOfPartition would have overflowed!\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // Replacing logic:
+ // (UINT32)(sizeof (EFI_GPT_DATA) - sizeof (GptData->Partitions) + NumberOfPartition * PrimaryHeader.SizeOfPartitionEntry);
+ //
+ Status = SafeUint32Mult (SafeNumberOfPartitions, PrimaryHeader->SizeOfPartitionEntry, EventSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Event Size would have overflowed!\n"));
+ return EFI_BAD_BUFFER_SIZE;
+ }
+
+ //
+ // Replacing logic:
+ // *EventSize + sizeof (EFI_TCG2_EVENT) - sizeof (Tcg2Event->Event);
+ //
+ Status = SafeUint32Add (
+ OFFSET_OF (EFI_TCG2_EVENT, Event) + OFFSET_OF (EFI_GPT_DATA, Partitions),
+ *EventSize,
+ EventSize
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Event Size would have overflowed because of GPTData!\n"));
+ return EFI_BAD_BUFFER_SIZE;
+ }
+
+ return EFI_SUCCESS;
+}
diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTest.c b/SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTest.c
new file mode 100644
index 000000000000..3eb9763e3c91
--- /dev/null
+++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTest.c
@@ -0,0 +1,303 @@
+/** @file
+ This file includes the unit test cases for the DxeTpm2MeasureBootLibSanitizationTest.c.
+
+ Copyright (c) Microsoft Corporation.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Uefi.h>
+#include <Library/UefiLib.h>
+#include <Library/DebugLib.h>
+#include <Library/UnitTestLib.h>
+#include <Protocol/BlockIo.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <IndustryStandard/UefiTcgPlatform.h>
+#include <Protocol/Tcg2Protocol.h>
+
+#include "../DxeTpm2MeasureBootLibSanitization.h"
+
+#define UNIT_TEST_NAME "DxeTpm2MeasureBootLibSanitizationTest"
+#define UNIT_TEST_VERSION "1.0"
+
+#define DEFAULT_PRIMARY_TABLE_HEADER_REVISION 0x00010000
+#define DEFAULT_PRIMARY_TABLE_HEADER_NUMBER_OF_PARTITION_ENTRIES 1
+#define DEFAULT_PRIMARY_TABLE_HEADER_SIZE_OF_PARTITION_ENTRY 128
+
+/**
+ This function tests the SanitizeEfiPartitionTableHeader function.
+ It's intent is to test that a malicious EFI_PARTITION_TABLE_HEADER
+ structure will not cause undefined or unexpected behavior.
+
+ In general the TPM should still be able to measure the data, but
+ be the header should be sanitized to prevent any unexpected behavior.
+
+ @param[in] Context The unit test context.
+
+ @retval UNIT_TEST_PASSED The test passed.
+ @retval UNIT_TEST_ERROR_TEST_FAILED The test failed.
+**/
+UNIT_TEST_STATUS
+EFIAPI
+TestSanitizeEfiPartitionTableHeader (
+ IN UNIT_TEST_CONTEXT Context
+ )
+{
+ EFI_STATUS Status;
+ EFI_PARTITION_TABLE_HEADER PrimaryHeader;
+ EFI_BLOCK_IO_PROTOCOL BlockIo;
+ EFI_BLOCK_IO_MEDIA BlockMedia;
+
+ // Generate EFI_BLOCK_IO_MEDIA test data
+ BlockMedia.MediaId = 1;
+ BlockMedia.RemovableMedia = FALSE;
+ BlockMedia.MediaPresent = TRUE;
+ BlockMedia.LogicalPartition = FALSE;
+ BlockMedia.ReadOnly = FALSE;
+ BlockMedia.WriteCaching = FALSE;
+ BlockMedia.BlockSize = 512;
+ BlockMedia.IoAlign = 1;
+ BlockMedia.LastBlock = 0;
+
+ // Generate EFI_BLOCK_IO_PROTOCOL test data
+ BlockIo.Revision = 1;
+ BlockIo.Media = &BlockMedia;
+ BlockIo.Reset = NULL;
+ BlockIo.ReadBlocks = NULL;
+ BlockIo.WriteBlocks = NULL;
+ BlockIo.FlushBlocks = NULL;
+
+ // Geneate EFI_PARTITION_TABLE_HEADER test data
+ PrimaryHeader.Header.Signature = EFI_PTAB_HEADER_ID;
+ PrimaryHeader.Header.Revision = DEFAULT_PRIMARY_TABLE_HEADER_REVISION;
+ PrimaryHeader.Header.HeaderSize = sizeof (EFI_PARTITION_TABLE_HEADER);
+ PrimaryHeader.MyLBA = 1;
+ PrimaryHeader.AlternateLBA = 2;
+ PrimaryHeader.FirstUsableLBA = 3;
+ PrimaryHeader.LastUsableLBA = 4;
+ PrimaryHeader.PartitionEntryLBA = 5;
+ PrimaryHeader.NumberOfPartitionEntries = DEFAULT_PRIMARY_TABLE_HEADER_NUMBER_OF_PARTITION_ENTRIES;
+ PrimaryHeader.SizeOfPartitionEntry = DEFAULT_PRIMARY_TABLE_HEADER_SIZE_OF_PARTITION_ENTRY;
+ PrimaryHeader.PartitionEntryArrayCRC32 = 0; // Purposely invalid
+
+ // Calculate the CRC32 of the PrimaryHeader
+ PrimaryHeader.Header.CRC32 = CalculateCrc32 ((UINT8 *)&PrimaryHeader, PrimaryHeader.Header.HeaderSize);
+
+ // Test that a normal PrimaryHeader passes validation
+ Status = SanitizeEfiPartitionTableHeader (&PrimaryHeader, &BlockIo);
+ UT_ASSERT_NOT_EFI_ERROR (Status);
+
+ // Test that when number of partition entries is 0, the function returns EFI_DEVICE_ERROR
+ // Should print "Invalid Partition Table Header NumberOfPartitionEntries!""
+ PrimaryHeader.NumberOfPartitionEntries = 0;
+ Status = SanitizeEfiPartitionTableHeader (&PrimaryHeader, &BlockIo);
+ UT_ASSERT_EQUAL (Status, EFI_DEVICE_ERROR);
+ PrimaryHeader.NumberOfPartitionEntries = DEFAULT_PRIMARY_TABLE_HEADER_SIZE_OF_PARTITION_ENTRY;
+
+ // Test that when the header size is too small, the function returns EFI_DEVICE_ERROR
+ // Should print "Invalid Partition Table Header Size!"
+ PrimaryHeader.Header.HeaderSize = 0;
+ Status = SanitizeEfiPartitionTableHeader (&PrimaryHeader, &BlockIo);
+ UT_ASSERT_EQUAL (Status, EFI_DEVICE_ERROR);
+ PrimaryHeader.Header.HeaderSize = sizeof (EFI_PARTITION_TABLE_HEADER);
+
+ // Test that when the SizeOfPartitionEntry is too small, the function returns EFI_DEVICE_ERROR
+ // should print: "SizeOfPartitionEntry shall be set to a value of 128 x 2^n where n is an integer greater than or equal to zero (e.g., 128, 256, 512, etc.)!"
+ PrimaryHeader.SizeOfPartitionEntry = 1;
+ Status = SanitizeEfiPartitionTableHeader (&PrimaryHeader, &BlockIo);
+ UT_ASSERT_EQUAL (Status, EFI_DEVICE_ERROR);
+
+ DEBUG ((DEBUG_INFO, "%a: Test passed\n", __func__));
+
+ return UNIT_TEST_PASSED;
+}
+
+/**
+ This function tests the SanitizePrimaryHeaderAllocationSize function.
+ It's intent is to test that the untrusted input from a EFI_PARTITION_TABLE_HEADER
+ structure will not cause an overflow when calculating the allocation size.
+
+ @param[in] Context The unit test context.
+
+ @retval UNIT_TEST_PASSED The test passed.
+ @retval UNIT_TEST_ERROR_TEST_FAILED The test failed.
+**/
+UNIT_TEST_STATUS
+EFIAPI
+TestSanitizePrimaryHeaderAllocationSize (
+ IN UNIT_TEST_CONTEXT Context
+ )
+{
+ UINT32 AllocationSize;
+
+ EFI_STATUS Status;
+ EFI_PARTITION_TABLE_HEADER PrimaryHeader;
+
+ // Test that a normal PrimaryHeader passes validation
+ PrimaryHeader.NumberOfPartitionEntries = 5;
+ PrimaryHeader.SizeOfPartitionEntry = DEFAULT_PRIMARY_TABLE_HEADER_SIZE_OF_PARTITION_ENTRY;
+
+ Status = SanitizePrimaryHeaderAllocationSize (&PrimaryHeader, &AllocationSize);
+ UT_ASSERT_NOT_EFI_ERROR (Status);
+
+ // Test that the allocation size is correct compared to the existing logic
+ UT_ASSERT_EQUAL (AllocationSize, PrimaryHeader.NumberOfPartitionEntries * PrimaryHeader.SizeOfPartitionEntry);
+
+ // Test that an overflow is detected
+ PrimaryHeader.NumberOfPartitionEntries = MAX_UINT32;
+ PrimaryHeader.SizeOfPartitionEntry = 5;
+ Status = SanitizePrimaryHeaderAllocationSize (&PrimaryHeader, &AllocationSize);
+ UT_ASSERT_EQUAL (Status, EFI_BAD_BUFFER_SIZE);
+
+ // Test the inverse
+ PrimaryHeader.NumberOfPartitionEntries = 5;
+ PrimaryHeader.SizeOfPartitionEntry = MAX_UINT32;
+ Status = SanitizePrimaryHeaderAllocationSize (&PrimaryHeader, &AllocationSize);
+ UT_ASSERT_EQUAL (Status, EFI_BAD_BUFFER_SIZE);
+
+ // Test the worst case scenario
+ PrimaryHeader.NumberOfPartitionEntries = MAX_UINT32;
+ PrimaryHeader.SizeOfPartitionEntry = MAX_UINT32;
+ Status = SanitizePrimaryHeaderAllocationSize (&PrimaryHeader, &AllocationSize);
+ UT_ASSERT_EQUAL (Status, EFI_BAD_BUFFER_SIZE);
+
+ DEBUG ((DEBUG_INFO, "%a: Test passed\n", __func__));
+
+ return UNIT_TEST_PASSED;
+}
+
+/**
+ This function tests the SanitizePrimaryHeaderGptEventSize function.
+ It's intent is to test that the untrusted input from a EFI_GPT_DATA structure
+ will not cause an overflow when calculating the event size.
+
+ @param[in] Context The unit test context.
+
+ @retval UNIT_TEST_PASSED The test passed.
+ @retval UNIT_TEST_ERROR_TEST_FAILED The test failed.
+**/
+UNIT_TEST_STATUS
+EFIAPI
+TestSanitizePrimaryHeaderGptEventSize (
+ IN UNIT_TEST_CONTEXT Context
+ )
+{
+ UINT32 EventSize;
+ UINT32 ExistingLogicEventSize;
+ EFI_STATUS Status;
+ EFI_PARTITION_TABLE_HEADER PrimaryHeader;
+ UINTN NumberOfPartition;
+ EFI_GPT_DATA *GptData;
+ EFI_TCG2_EVENT *Tcg2Event;
+
+ Tcg2Event = NULL;
+ GptData = NULL;
+
+ // Test that a normal PrimaryHeader passes validation
+ PrimaryHeader.NumberOfPartitionEntries = 5;
+ PrimaryHeader.SizeOfPartitionEntry = DEFAULT_PRIMARY_TABLE_HEADER_SIZE_OF_PARTITION_ENTRY;
+
+ // set the number of partitions
+ NumberOfPartition = 13;
+
+ // that the primary event size is correct
+ Status = SanitizePrimaryHeaderGptEventSize (&PrimaryHeader, NumberOfPartition, &EventSize);
+ UT_ASSERT_NOT_EFI_ERROR (Status);
+
+ // Calculate the existing logic event size
+ ExistingLogicEventSize = (UINT32)(OFFSET_OF (EFI_TCG2_EVENT, Event) + OFFSET_OF (EFI_GPT_DATA, Partitions)
+ + NumberOfPartition * PrimaryHeader.SizeOfPartitionEntry);
+
+ // Check that the event size is correct
+ UT_ASSERT_EQUAL (EventSize, ExistingLogicEventSize);
+
+ // Tests that the primary event size may not overflow
+ Status = SanitizePrimaryHeaderGptEventSize (&PrimaryHeader, MAX_UINT32, &EventSize);
+ UT_ASSERT_EQUAL (Status, EFI_BAD_BUFFER_SIZE);
+
+ // Test that the size of partition entries may not overflow
+ PrimaryHeader.SizeOfPartitionEntry = MAX_UINT32;
+ Status = SanitizePrimaryHeaderGptEventSize (&PrimaryHeader, NumberOfPartition, &EventSize);
+ UT_ASSERT_EQUAL (Status, EFI_BAD_BUFFER_SIZE);
+
+ DEBUG ((DEBUG_INFO, "%a: Test passed\n", __func__));
+
+ return UNIT_TEST_PASSED;
+}
+
+// *--------------------------------------------------------------------*
+// * Unit Test Code Main Function
+// *--------------------------------------------------------------------*
+
+/**
+ This function acts as the entry point for the unit tests.
+
+ @retval UNIT_TEST_PASSED The test passed.
+ @retval UNIT_TEST_ERROR_TEST_FAILED The test failed.
+ @retval others The test failed.
+**/
+EFI_STATUS
+EFIAPI
+UefiTestMain (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+ UNIT_TEST_FRAMEWORK_HANDLE Framework;
+ UNIT_TEST_SUITE_HANDLE Tcg2MeasureBootLibValidationTestSuite;
+
+ Framework = NULL;
+
+ DEBUG ((DEBUG_INFO, "%a: TestMain() - Start\n", UNIT_TEST_NAME));
+
+ Status = InitUnitTestFramework (&Framework, UNIT_TEST_NAME, gEfiCallerBaseName, UNIT_TEST_VERSION);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: Failed in InitUnitTestFramework. Status = %r\n", UNIT_TEST_NAME, Status));
+ goto EXIT;
+ }
+
+ Status = CreateUnitTestSuite (&Tcg2MeasureBootLibValidationTestSuite, Framework, "Tcg2MeasureBootLibValidationTestSuite", "Common.Tcg2MeasureBootLibValidation", NULL, NULL);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%s: Failed in CreateUnitTestSuite for Tcg2MeasureBootLibValidationTestSuite\n", UNIT_TEST_NAME));
+ Status = EFI_OUT_OF_RESOURCES;
+ goto EXIT;
+ }
+
+ // -----------Suite---------------------------------Description----------------------------Class----------------------------------Test Function------------------------Pre---Clean-Context
+ AddTestCase (Tcg2MeasureBootLibValidationTestSuite, "Tests Validating EFI Partition Table", "Common.Tcg2MeasureBootLibValidation", TestSanitizeEfiPartitionTableHeader, NULL, NULL, NULL);
+ AddTestCase (Tcg2MeasureBootLibValidationTestSuite, "Tests Primary header gpt event checks for overflow", "Common.Tcg2MeasureBootLibValidation", TestSanitizePrimaryHeaderAllocationSize, NULL, NULL, NULL);
+ AddTestCase (Tcg2MeasureBootLibValidationTestSuite, "Tests Primary header allocation size checks for overflow", "Common.Tcg2MeasureBootLibValidation", TestSanitizePrimaryHeaderGptEventSize, NULL, NULL, NULL);
+
+ Status = RunAllTestSuites (Framework);
+
+EXIT:
+ if (Framework != NULL) {
+ FreeUnitTestFramework (Framework);
+ }
+
+ DEBUG ((DEBUG_INFO, "%a: TestMain() - End\n", UNIT_TEST_NAME));
+ return Status;
+}
+
+///
+/// Avoid ECC error for function name that starts with lower case letter
+///
+#define DxeTpm2MeasureBootLibUnitTestMain main
+
+/**
+ Standard POSIX C entry point for host based unit test execution.
+
+ @param[in] Argc Number of arguments
+ @param[in] Argv Array of pointers to arguments
+
+ @retval 0 Success
+ @retval other Error
+**/
+INT32
+DxeTpm2MeasureBootLibUnitTestMain (
+ IN INT32 Argc,
+ IN CHAR8 *Argv[]
+ )
+{
+ return (INT32)UefiTestMain ();
+}
diff --git a/SecurityPkg/SecurityPkg.ci.yaml b/SecurityPkg/SecurityPkg.ci.yaml
index 3f03762bd6f9..24389531afaa 100644
--- a/SecurityPkg/SecurityPkg.ci.yaml
+++ b/SecurityPkg/SecurityPkg.ci.yaml
@@ -16,6 +16,7 @@
## ]
"ExceptionList": [
"8005", "gRT",
+ "8001", "DxeTpm2MeasureBootLibUnitTestMain",
],
## Both file path and directory path are accepted.
"IgnoreFiles": [
--
2.43.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113757): https://edk2.groups.io/g/devel/message/113757
Mute This Topic: https://groups.io/mt/103689720/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [edk2-devel] [PATCH 2/6] SecurityPkg: DxeTpmMeasureBootLib: SECURITY PATCH 4117 - CVE 2022-36763
2024-01-11 18:16 [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118 Doug Flick via groups.io
2024-01-11 18:16 ` [edk2-devel] [PATCH 1/6] SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4117 - CVE 2022-36763 Doug Flick via groups.io
@ 2024-01-11 18:16 ` Doug Flick via groups.io
2024-01-11 18:16 ` [edk2-devel] [PATCH 3/6] SecurityPkg: : Adding CVE 2022-36763 to SecurityFixes.yaml Doug Flick via groups.io
` (6 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Doug Flick via groups.io @ 2024-01-11 18:16 UTC (permalink / raw)
To: devel; +Cc: Douglas Flick [MSFT], Jiewen Yao
This commit contains the patch files and tests for DxeTpmMeasureBootLib
CVE 2022-36763.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com>
---
SecurityPkg/Test/SecurityPkgHostTest.dsc | 1 +
.../DxeTpmMeasureBootLib.inf | 4 +-
...eTpmMeasureBootLibSanitizationTestHost.inf | 28 ++
.../DxeTpmMeasureBootLibSanitization.h | 114 +++++++
.../DxeTpmMeasureBootLib.c | 40 ++-
.../DxeTpmMeasureBootLibSanitization.c | 241 ++++++++++++++
.../DxeTpmMeasureBootLibSanitizationTest.c | 301 ++++++++++++++++++
SecurityPkg/SecurityPkg.ci.yaml | 1 +
8 files changed, 716 insertions(+), 14 deletions(-)
create mode 100644 SecurityPkg/Library/DxeTpmMeasureBootLib/InternalUnitTest/DxeTpmMeasureBootLibSanitizationTestHost.inf
create mode 100644 SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.h
create mode 100644 SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.c
create mode 100644 SecurityPkg/Library/DxeTpmMeasureBootLib/InternalUnitTest/DxeTpmMeasureBootLibSanitizationTest.c
diff --git a/SecurityPkg/Test/SecurityPkgHostTest.dsc b/SecurityPkg/Test/SecurityPkgHostTest.dsc
index 788c1ab6fec6..1655e573eae4 100644
--- a/SecurityPkg/Test/SecurityPkgHostTest.dsc
+++ b/SecurityPkg/Test/SecurityPkgHostTest.dsc
@@ -27,6 +27,7 @@ [Components]
SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiLib.inf
SecurityPkg/Test/Mock/Library/GoogleTest/MockPlatformPKProtectionLib/MockPlatformPKProtectionLib.inf
SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTestHost.inf
+ SecurityPkg/Library/DxeTpmMeasureBootLib/InternalUnitTest/DxeTpmMeasureBootLibSanitizationTestHost.inf
#
# Build SecurityPkg HOST_APPLICATION Tests
diff --git a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.inf b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.inf
index ebab6f7c1e1e..414c654d156a 100644
--- a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.inf
+++ b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.inf
@@ -32,6 +32,8 @@ [Defines]
[Sources]
DxeTpmMeasureBootLib.c
+ DxeTpmMeasureBootLibSanitization.c
+ DxeTpmMeasureBootLibSanitization.h
[Packages]
MdePkg/MdePkg.dec
@@ -41,6 +43,7 @@ [Packages]
[LibraryClasses]
BaseMemoryLib
+ SafeIntLib
DebugLib
MemoryAllocationLib
DevicePathLib
@@ -59,4 +62,3 @@ [Protocols]
gEfiFirmwareVolumeBlockProtocolGuid ## SOMETIMES_CONSUMES
gEfiBlockIoProtocolGuid ## SOMETIMES_CONSUMES
gEfiDiskIoProtocolGuid ## SOMETIMES_CONSUMES
-
diff --git a/SecurityPkg/Library/DxeTpmMeasureBootLib/InternalUnitTest/DxeTpmMeasureBootLibSanitizationTestHost.inf b/SecurityPkg/Library/DxeTpmMeasureBootLib/InternalUnitTest/DxeTpmMeasureBootLibSanitizationTestHost.inf
new file mode 100644
index 000000000000..47b0811b00bc
--- /dev/null
+++ b/SecurityPkg/Library/DxeTpmMeasureBootLib/InternalUnitTest/DxeTpmMeasureBootLibSanitizationTestHost.inf
@@ -0,0 +1,28 @@
+## @file
+# This file builds the unit tests for DxeTpmMeasureBootLib
+#
+# Copyright (C) Microsoft Corporation.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+ INF_VERSION = 0x00010006
+ BASE_NAME = DxeTpmMeasuredBootLibTest
+ FILE_GUID = eb01bc38-309c-4d3e-967e-9f078c90772f
+ MODULE_TYPE = HOST_APPLICATION
+ VERSION_STRING = 1.0
+ ENTRY_POINT = main
+
+[Sources]
+ DxeTpmMeasureBootLibSanitizationTest.c
+ ../DxeTpmMeasureBootLibSanitization.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ BaseLib
+ DebugLib
+ UnitTestLib
+ PrintLib
+ SafeIntLib
diff --git a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.h b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.h
new file mode 100644
index 000000000000..0d9d00c281f6
--- /dev/null
+++ b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.h
@@ -0,0 +1,114 @@
+/** @file
+ This file includes the function prototypes for the sanitization functions.
+
+ These are those functions:
+
+ DxeTpmMeasureBootLibImageRead() function will make sure the PE/COFF image content
+ read is within the image buffer.
+
+ TcgMeasurePeImage() function will accept untrusted PE/COFF image and validate its
+ data structure within this image buffer before use.
+
+ TcgMeasureGptTable() function will receive untrusted GPT partition table, and parse
+ partition data carefully.
+
+ Copyright (c) Microsoft Corporation.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef DXE_TPM_MEASURE_BOOT_LIB_VALIDATION_
+#define DXE_TPM_MEASURE_BOOT_LIB_VALIDATION_
+
+#include <Uefi.h>
+#include <Uefi/UefiSpec.h>
+#include <Protocol/BlockIo.h>
+#include <IndustryStandard/UefiTcgPlatform.h>
+
+/**
+ This function will validate the EFI_PARTITION_TABLE_HEADER structure is safe to parse
+ However this function will not attempt to verify the validity of the GPT partition
+ It will check the following:
+ - Signature
+ - Revision
+ - AlternateLBA
+ - FirstUsableLBA
+ - LastUsableLBA
+ - PartitionEntryLBA
+ - NumberOfPartitionEntries
+ - SizeOfPartitionEntry
+ - BlockIo
+
+ @param[in] PrimaryHeader
+ Pointer to the EFI_PARTITION_TABLE_HEADER structure.
+
+ @param[in] BlockIo
+ Pointer to the EFI_BLOCK_IO_PROTOCOL structure.
+
+ @retval EFI_SUCCESS
+ The EFI_PARTITION_TABLE_HEADER structure is valid.
+
+ @retval EFI_INVALID_PARAMETER
+ The EFI_PARTITION_TABLE_HEADER structure is invalid.
+**/
+EFI_STATUS
+EFIAPI
+SanitizeEfiPartitionTableHeader (
+ IN CONST EFI_PARTITION_TABLE_HEADER *PrimaryHeader,
+ IN CONST EFI_BLOCK_IO_PROTOCOL *BlockIo
+ );
+
+/**
+ This function will validate that the allocation size from the primary header is sane
+ It will check the following:
+ - AllocationSize does not overflow
+
+ @param[in] PrimaryHeader
+ Pointer to the EFI_PARTITION_TABLE_HEADER structure.
+
+ @param[out] AllocationSize
+ Pointer to the allocation size.
+
+ @retval EFI_SUCCESS
+ The allocation size is valid.
+
+ @retval EFI_OUT_OF_RESOURCES
+ The allocation size is invalid.
+**/
+EFI_STATUS
+EFIAPI
+SanitizePrimaryHeaderAllocationSize (
+ IN CONST EFI_PARTITION_TABLE_HEADER *PrimaryHeader,
+ OUT UINT32 *AllocationSize
+ );
+
+/**
+ This function will validate that the Gpt Event Size calculated from the primary header is sane
+ It will check the following:
+ - EventSize does not overflow
+
+ Important: This function includes the entire length of the allocated space, including the
+ TCG_PCR_EVENT_HDR. When hashing the buffer allocated with this size, the caller must subtract
+ the size of the TCG_PCR_EVENT_HDR from the size of the buffer before hashing.
+
+ @param[in] PrimaryHeader - Pointer to the EFI_PARTITION_TABLE_HEADER structure.
+ @param[in] NumberOfPartition - Number of partitions.
+ @param[out] EventSize - Pointer to the event size.
+
+ @retval EFI_SUCCESS
+ The event size is valid.
+
+ @retval EFI_OUT_OF_RESOURCES
+ Overflow would have occurred.
+
+ @retval EFI_INVALID_PARAMETER
+ One of the passed parameters was invalid.
+**/
+EFI_STATUS
+SanitizePrimaryHeaderGptEventSize (
+ IN CONST EFI_PARTITION_TABLE_HEADER *PrimaryHeader,
+ IN UINTN NumberOfPartition,
+ OUT UINT32 *EventSize
+ );
+
+#endif // DXE_TPM_MEASURE_BOOT_LIB_VALIDATION_
diff --git a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
index 220393dd2beb..669ab1913440 100644
--- a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
+++ b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
@@ -18,6 +18,8 @@
Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
+Copyright (c) Microsoft Corporation.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
**/
#include <PiDxe.h>
@@ -40,6 +42,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/SecurityManagementLib.h>
#include <Library/HobLib.h>
+#include "DxeTpmMeasureBootLibSanitization.h"
+
//
// Flag to check GPT partition. It only need be measured once.
//
@@ -136,6 +140,9 @@ TcgMeasureGptTable (
UINT32 EventSize;
UINT32 EventNumber;
EFI_PHYSICAL_ADDRESS EventLogLastEntry;
+ UINT32 AllocSize;
+
+ GptData = NULL;
if (mMeasureGptCount > 0) {
return EFI_SUCCESS;
@@ -166,8 +173,8 @@ TcgMeasureGptTable (
BlockIo->Media->BlockSize,
(UINT8 *)PrimaryHeader
);
- if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "Failed to Read Partition Table Header!\n"));
+ if (EFI_ERROR (Status) || EFI_ERROR (SanitizeEfiPartitionTableHeader (PrimaryHeader, BlockIo))) {
+ DEBUG ((DEBUG_ERROR, "Failed to read Partition Table Header or invalid Partition Table Header!\n"));
FreePool (PrimaryHeader);
return EFI_DEVICE_ERROR;
}
@@ -175,7 +182,13 @@ TcgMeasureGptTable (
//
// Read the partition entry.
//
- EntryPtr = (UINT8 *)AllocatePool (PrimaryHeader->NumberOfPartitionEntries * PrimaryHeader->SizeOfPartitionEntry);
+ Status = SanitizePrimaryHeaderAllocationSize (PrimaryHeader, &AllocSize);
+ if (EFI_ERROR (Status)) {
+ FreePool (PrimaryHeader);
+ return EFI_DEVICE_ERROR;
+ }
+
+ EntryPtr = (UINT8 *)AllocatePool (AllocSize);
if (EntryPtr == NULL) {
FreePool (PrimaryHeader);
return EFI_OUT_OF_RESOURCES;
@@ -185,7 +198,7 @@ TcgMeasureGptTable (
DiskIo,
BlockIo->Media->MediaId,
MultU64x32 (PrimaryHeader->PartitionEntryLBA, BlockIo->Media->BlockSize),
- PrimaryHeader->NumberOfPartitionEntries * PrimaryHeader->SizeOfPartitionEntry,
+ AllocSize,
EntryPtr
);
if (EFI_ERROR (Status)) {
@@ -210,9 +223,8 @@ TcgMeasureGptTable (
//
// Prepare Data for Measurement
//
- EventSize = (UINT32)(sizeof (EFI_GPT_DATA) - sizeof (GptData->Partitions)
- + NumberOfPartition * PrimaryHeader->SizeOfPartitionEntry);
- TcgEvent = (TCG_PCR_EVENT *)AllocateZeroPool (EventSize + sizeof (TCG_PCR_EVENT_HDR));
+ Status = SanitizePrimaryHeaderGptEventSize (PrimaryHeader, NumberOfPartition, &EventSize);
+ TcgEvent = (TCG_PCR_EVENT *)AllocateZeroPool (EventSize);
if (TcgEvent == NULL) {
FreePool (PrimaryHeader);
FreePool (EntryPtr);
@@ -221,7 +233,7 @@ TcgMeasureGptTable (
TcgEvent->PCRIndex = 5;
TcgEvent->EventType = EV_EFI_GPT_EVENT;
- TcgEvent->EventSize = EventSize;
+ TcgEvent->EventSize = EventSize - sizeof (TCG_PCR_EVENT_HDR);
GptData = (EFI_GPT_DATA *)TcgEvent->Event;
//
@@ -361,11 +373,13 @@ TcgMeasurePeImage (
TcgEvent->PCRIndex = 2;
break;
default:
- DEBUG ((
- DEBUG_ERROR,
- "TcgMeasurePeImage: Unknown subsystem type %d",
- ImageType
- ));
+ DEBUG (
+ (
+ DEBUG_ERROR,
+ "TcgMeasurePeImage: Unknown subsystem type %d",
+ ImageType
+ )
+ );
goto Finish;
}
diff --git a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.c b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.c
new file mode 100644
index 000000000000..a3fa46f5e632
--- /dev/null
+++ b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.c
@@ -0,0 +1,241 @@
+/** @file
+ The library instance provides security service of TPM2 measure boot and
+ Confidential Computing (CC) measure boot.
+
+ Caution: This file requires additional review when modified.
+ This library will have external input - PE/COFF image and GPT partition.
+ This external input must be validated carefully to avoid security issue like
+ buffer overflow, integer overflow.
+
+ This file will pull out the validation logic from the following functions, in an
+ attempt to validate the untrusted input in the form of unit tests
+
+ These are those functions:
+
+ DxeTpmMeasureBootLibImageRead() function will make sure the PE/COFF image content
+ read is within the image buffer.
+
+ Tcg2MeasureGptTable() function will receive untrusted GPT partition table, and parse
+ partition data carefully.
+
+ Copyright (c) Microsoft Corporation.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+#include <Uefi.h>
+#include <Uefi/UefiSpec.h>
+#include <Library/SafeIntLib.h>
+#include <Library/UefiLib.h>
+#include <Library/DebugLib.h>
+#include <Library/BaseLib.h>
+#include <IndustryStandard/UefiTcgPlatform.h>
+#include <Protocol/BlockIo.h>
+#include <Library/MemoryAllocationLib.h>
+
+#include "DxeTpmMeasureBootLibSanitization.h"
+
+#define GPT_HEADER_REVISION_V1 0x00010000
+
+/**
+ This function will validate the EFI_PARTITION_TABLE_HEADER structure is safe to parse
+ However this function will not attempt to verify the validity of the GPT partition
+ It will check the following:
+ - Signature
+ - Revision
+ - AlternateLBA
+ - FirstUsableLBA
+ - LastUsableLBA
+ - PartitionEntryLBA
+ - NumberOfPartitionEntries
+ - SizeOfPartitionEntry
+ - BlockIo
+
+ @param[in] PrimaryHeader
+ Pointer to the EFI_PARTITION_TABLE_HEADER structure.
+
+ @param[in] BlockIo
+ Pointer to the EFI_BLOCK_IO_PROTOCOL structure.
+
+ @retval EFI_SUCCESS
+ The EFI_PARTITION_TABLE_HEADER structure is valid.
+
+ @retval EFI_INVALID_PARAMETER
+ The EFI_PARTITION_TABLE_HEADER structure is invalid.
+**/
+EFI_STATUS
+EFIAPI
+SanitizeEfiPartitionTableHeader (
+ IN CONST EFI_PARTITION_TABLE_HEADER *PrimaryHeader,
+ IN CONST EFI_BLOCK_IO_PROTOCOL *BlockIo
+ )
+{
+ // Verify that the input parameters are safe to use
+ if (PrimaryHeader == NULL) {
+ DEBUG ((DEBUG_ERROR, "Invalid Partition Table Header!\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if ((BlockIo == NULL) || (BlockIo->Media == NULL)) {
+ DEBUG ((DEBUG_ERROR, "Invalid BlockIo!\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ // The signature must be EFI_PTAB_HEADER_ID ("EFI PART" in ASCII)
+ if (PrimaryHeader->Header.Signature != EFI_PTAB_HEADER_ID) {
+ DEBUG ((DEBUG_ERROR, "Invalid Partition Table Header!\n"));
+ return EFI_DEVICE_ERROR;
+ }
+
+ // The version must be GPT_HEADER_REVISION_V1 (0x00010000)
+ if (PrimaryHeader->Header.Revision != GPT_HEADER_REVISION_V1) {
+ DEBUG ((DEBUG_ERROR, "Invalid Partition Table Header Revision!\n"));
+ return EFI_DEVICE_ERROR;
+ }
+
+ // The HeaderSize must be greater than or equal to 92 and must be less than or equal to the logical block size
+ if ((PrimaryHeader->Header.HeaderSize < sizeof (EFI_PARTITION_TABLE_HEADER)) || (PrimaryHeader->Header.HeaderSize > BlockIo->Media->BlockSize)) {
+ DEBUG ((DEBUG_ERROR, "Invalid Partition Table Header HeaderSize!\n"));
+ return EFI_DEVICE_ERROR;
+ }
+
+ // check that the PartitionEntryLBA greater than the Max LBA
+ // This will be used later for multiplication
+ if (PrimaryHeader->PartitionEntryLBA > DivU64x32 (MAX_UINT64, BlockIo->Media->BlockSize)) {
+ DEBUG ((DEBUG_ERROR, "Invalid Partition Table Header PartitionEntryLBA!\n"));
+ return EFI_DEVICE_ERROR;
+ }
+
+ // Check that the number of partition entries is greater than zero
+ if (PrimaryHeader->NumberOfPartitionEntries == 0) {
+ DEBUG ((DEBUG_ERROR, "Invalid Partition Table Header NumberOfPartitionEntries!\n"));
+ return EFI_DEVICE_ERROR;
+ }
+
+ // SizeOfPartitionEntry must be 128, 256, 512... improper size may lead to accessing uninitialized memory
+ if ((PrimaryHeader->SizeOfPartitionEntry < 128) || ((PrimaryHeader->SizeOfPartitionEntry & (PrimaryHeader->SizeOfPartitionEntry - 1)) != 0)) {
+ DEBUG ((DEBUG_ERROR, "SizeOfPartitionEntry shall be set to a value of 128 x 2^n where n is an integer greater than or equal to zero (e.g., 128, 256, 512, etc.)!\n"));
+ return EFI_DEVICE_ERROR;
+ }
+
+ // This check is to prevent overflow when calculating the allocation size for the partition entries
+ // This check will be used later for multiplication
+ if (PrimaryHeader->NumberOfPartitionEntries > DivU64x32 (MAX_UINT64, PrimaryHeader->SizeOfPartitionEntry)) {
+ DEBUG ((DEBUG_ERROR, "Invalid Partition Table Header NumberOfPartitionEntries!\n"));
+ return EFI_DEVICE_ERROR;
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
+ This function will validate that the allocation size from the primary header is sane
+ It will check the following:
+ - AllocationSize does not overflow
+
+ @param[in] PrimaryHeader
+ Pointer to the EFI_PARTITION_TABLE_HEADER structure.
+
+ @param[out] AllocationSize
+ Pointer to the allocation size.
+
+ @retval EFI_SUCCESS
+ The allocation size is valid.
+
+ @retval EFI_OUT_OF_RESOURCES
+ The allocation size is invalid.
+**/
+EFI_STATUS
+EFIAPI
+SanitizePrimaryHeaderAllocationSize (
+ IN CONST EFI_PARTITION_TABLE_HEADER *PrimaryHeader,
+ OUT UINT32 *AllocationSize
+ )
+{
+ EFI_STATUS Status;
+
+ if (PrimaryHeader == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (AllocationSize == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ // Replacing logic:
+ // PrimaryHeader->NumberOfPartitionEntries * PrimaryHeader->SizeOfPartitionEntry;
+ Status = SafeUint32Mult (PrimaryHeader->NumberOfPartitionEntries, PrimaryHeader->SizeOfPartitionEntry, AllocationSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Allocation Size would have overflowed!\n"));
+ return EFI_BAD_BUFFER_SIZE;
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
+ This function will validate that the Gpt Event Size calculated from the primary header is sane
+ It will check the following:
+ - EventSize does not overflow
+
+ Important: This function includes the entire length of the allocated space, including the
+ TCG_PCR_EVENT_HDR. When hashing the buffer allocated with this size, the caller must subtract
+ the size of the TCG_PCR_EVENT_HDR from the size of the buffer before hashing.
+
+ @param[in] PrimaryHeader - Pointer to the EFI_PARTITION_TABLE_HEADER structure.
+ @param[in] NumberOfPartition - Number of partitions.
+ @param[out] EventSize - Pointer to the event size.
+
+ @retval EFI_SUCCESS
+ The event size is valid.
+
+ @retval EFI_OUT_OF_RESOURCES
+ Overflow would have occurred.
+
+ @retval EFI_INVALID_PARAMETER
+ One of the passed parameters was invalid.
+**/
+EFI_STATUS
+SanitizePrimaryHeaderGptEventSize (
+ IN CONST EFI_PARTITION_TABLE_HEADER *PrimaryHeader,
+ IN UINTN NumberOfPartition,
+ OUT UINT32 *EventSize
+ )
+{
+ EFI_STATUS Status;
+ UINT32 SafeNumberOfPartitions;
+
+ if (PrimaryHeader == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (EventSize == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ // We shouldn't even attempt to perform the multiplication if the number of partitions is greater than the maximum value of UINT32
+ Status = SafeUintnToUint32 (NumberOfPartition, &SafeNumberOfPartitions);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "NumberOfPartition would have overflowed!\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ // Replacing logic:
+ // (UINT32)(sizeof (EFI_GPT_DATA) - sizeof (GptData->Partitions) + NumberOfPartition * PrimaryHeader.SizeOfPartitionEntry + sizeof (TCG_PCR_EVENT_HDR));
+ Status = SafeUint32Mult (SafeNumberOfPartitions, PrimaryHeader->SizeOfPartitionEntry, EventSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Event Size would have overflowed!\n"));
+ return EFI_BAD_BUFFER_SIZE;
+ }
+
+ Status = SafeUint32Add (
+ sizeof (TCG_PCR_EVENT_HDR) +
+ OFFSET_OF (EFI_GPT_DATA, Partitions),
+ *EventSize,
+ EventSize
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Event Size would have overflowed because of GPTData!\n"));
+ return EFI_BAD_BUFFER_SIZE;
+ }
+
+ return EFI_SUCCESS;
+}
diff --git a/SecurityPkg/Library/DxeTpmMeasureBootLib/InternalUnitTest/DxeTpmMeasureBootLibSanitizationTest.c b/SecurityPkg/Library/DxeTpmMeasureBootLib/InternalUnitTest/DxeTpmMeasureBootLibSanitizationTest.c
new file mode 100644
index 000000000000..eeb928cdb0aa
--- /dev/null
+++ b/SecurityPkg/Library/DxeTpmMeasureBootLib/InternalUnitTest/DxeTpmMeasureBootLibSanitizationTest.c
@@ -0,0 +1,301 @@
+/** @file
+This file includes the unit test cases for the DxeTpmMeasureBootLibSanitizationTest.c.
+
+Copyright (c) Microsoft Corporation.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Uefi.h>
+#include <Library/UefiLib.h>
+#include <Library/DebugLib.h>
+#include <Library/UnitTestLib.h>
+#include <Protocol/BlockIo.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <IndustryStandard/UefiTcgPlatform.h>
+
+#include "../DxeTpmMeasureBootLibSanitization.h"
+
+#define UNIT_TEST_NAME "DxeTpmMeasureBootLibSanitizationTest"
+#define UNIT_TEST_VERSION "1.0"
+
+#define DEFAULT_PRIMARY_TABLE_HEADER_REVISION 0x00010000
+#define DEFAULT_PRIMARY_TABLE_HEADER_NUMBER_OF_PARTITION_ENTRIES 1
+#define DEFAULT_PRIMARY_TABLE_HEADER_SIZE_OF_PARTITION_ENTRY 128
+
+/**
+ This function tests the SanitizeEfiPartitionTableHeader function.
+ It's intent is to test that a malicious EFI_PARTITION_TABLE_HEADER
+ structure will not cause undefined or unexpected behavior.
+
+ In general the TPM should still be able to measure the data, but
+ be the header should be sanitized to prevent any unexpected behavior.
+
+ @param[in] Context The unit test context.
+
+ @retval UNIT_TEST_PASSED The test passed.
+ @retval UNIT_TEST_ERROR_TEST_FAILED The test failed.
+**/
+UNIT_TEST_STATUS
+EFIAPI
+TestSanitizeEfiPartitionTableHeader (
+ IN UNIT_TEST_CONTEXT Context
+ )
+{
+ EFI_STATUS Status;
+ EFI_PARTITION_TABLE_HEADER PrimaryHeader;
+ EFI_BLOCK_IO_PROTOCOL BlockIo;
+ EFI_BLOCK_IO_MEDIA BlockMedia;
+
+ // Generate EFI_BLOCK_IO_MEDIA test data
+ BlockMedia.MediaId = 1;
+ BlockMedia.RemovableMedia = FALSE;
+ BlockMedia.MediaPresent = TRUE;
+ BlockMedia.LogicalPartition = FALSE;
+ BlockMedia.ReadOnly = FALSE;
+ BlockMedia.WriteCaching = FALSE;
+ BlockMedia.BlockSize = 512;
+ BlockMedia.IoAlign = 1;
+ BlockMedia.LastBlock = 0;
+
+ // Generate EFI_BLOCK_IO_PROTOCOL test data
+ BlockIo.Revision = 1;
+ BlockIo.Media = &BlockMedia;
+ BlockIo.Reset = NULL;
+ BlockIo.ReadBlocks = NULL;
+ BlockIo.WriteBlocks = NULL;
+ BlockIo.FlushBlocks = NULL;
+
+ // Geneate EFI_PARTITION_TABLE_HEADER test data
+ PrimaryHeader.Header.Signature = EFI_PTAB_HEADER_ID;
+ PrimaryHeader.Header.Revision = DEFAULT_PRIMARY_TABLE_HEADER_REVISION;
+ PrimaryHeader.Header.HeaderSize = sizeof (EFI_PARTITION_TABLE_HEADER);
+ PrimaryHeader.MyLBA = 1;
+ PrimaryHeader.AlternateLBA = 2;
+ PrimaryHeader.FirstUsableLBA = 3;
+ PrimaryHeader.LastUsableLBA = 4;
+ PrimaryHeader.PartitionEntryLBA = 5;
+ PrimaryHeader.NumberOfPartitionEntries = DEFAULT_PRIMARY_TABLE_HEADER_NUMBER_OF_PARTITION_ENTRIES;
+ PrimaryHeader.SizeOfPartitionEntry = DEFAULT_PRIMARY_TABLE_HEADER_SIZE_OF_PARTITION_ENTRY;
+ PrimaryHeader.PartitionEntryArrayCRC32 = 0; // Purposely invalid
+
+ // Calculate the CRC32 of the PrimaryHeader
+ PrimaryHeader.Header.CRC32 = CalculateCrc32 ((UINT8 *)&PrimaryHeader, PrimaryHeader.Header.HeaderSize);
+
+ // Test that a normal PrimaryHeader passes validation
+ Status = SanitizeEfiPartitionTableHeader (&PrimaryHeader, &BlockIo);
+ UT_ASSERT_NOT_EFI_ERROR (Status);
+
+ // Test that when number of partition entries is 0, the function returns EFI_DEVICE_ERROR
+ // Should print "Invalid Partition Table Header NumberOfPartitionEntries!""
+ PrimaryHeader.NumberOfPartitionEntries = 0;
+ Status = SanitizeEfiPartitionTableHeader (&PrimaryHeader, &BlockIo);
+ UT_ASSERT_EQUAL (Status, EFI_DEVICE_ERROR);
+ PrimaryHeader.NumberOfPartitionEntries = DEFAULT_PRIMARY_TABLE_HEADER_SIZE_OF_PARTITION_ENTRY;
+
+ // Test that when the header size is too small, the function returns EFI_DEVICE_ERROR
+ // Should print "Invalid Partition Table Header Size!"
+ PrimaryHeader.Header.HeaderSize = 0;
+ Status = SanitizeEfiPartitionTableHeader (&PrimaryHeader, &BlockIo);
+ UT_ASSERT_EQUAL (Status, EFI_DEVICE_ERROR);
+ PrimaryHeader.Header.HeaderSize = sizeof (EFI_PARTITION_TABLE_HEADER);
+
+ // Test that when the SizeOfPartitionEntry is too small, the function returns EFI_DEVICE_ERROR
+ // should print: "SizeOfPartitionEntry shall be set to a value of 128 x 2^n where n is an integer greater than or equal to zero (e.g., 128, 256, 512, etc.)!"
+ PrimaryHeader.SizeOfPartitionEntry = 1;
+ Status = SanitizeEfiPartitionTableHeader (&PrimaryHeader, &BlockIo);
+ UT_ASSERT_EQUAL (Status, EFI_DEVICE_ERROR);
+
+ DEBUG ((DEBUG_INFO, "%a: Test passed\n", __func__));
+
+ return UNIT_TEST_PASSED;
+}
+
+/**
+ This function tests the SanitizePrimaryHeaderAllocationSize function.
+ It's intent is to test that the untrusted input from a EFI_PARTITION_TABLE_HEADER
+ structure will not cause an overflow when calculating the allocation size.
+
+ @param[in] Context The unit test context.
+
+ @retval UNIT_TEST_PASSED The test passed.
+ @retval UNIT_TEST_ERROR_TEST_FAILED The test failed.
+**/
+UNIT_TEST_STATUS
+EFIAPI
+TestSanitizePrimaryHeaderAllocationSize (
+ IN UNIT_TEST_CONTEXT Context
+ )
+{
+ UINT32 AllocationSize;
+
+ EFI_STATUS Status;
+ EFI_PARTITION_TABLE_HEADER PrimaryHeader;
+
+ // Test that a normal PrimaryHeader passes validation
+ PrimaryHeader.NumberOfPartitionEntries = 5;
+ PrimaryHeader.SizeOfPartitionEntry = DEFAULT_PRIMARY_TABLE_HEADER_SIZE_OF_PARTITION_ENTRY;
+
+ Status = SanitizePrimaryHeaderAllocationSize (&PrimaryHeader, &AllocationSize);
+ UT_ASSERT_NOT_EFI_ERROR (Status);
+
+ // Test that the allocation size is correct compared to the existing logic
+ UT_ASSERT_EQUAL (AllocationSize, PrimaryHeader.NumberOfPartitionEntries * PrimaryHeader.SizeOfPartitionEntry);
+
+ // Test that an overflow is detected
+ PrimaryHeader.NumberOfPartitionEntries = MAX_UINT32;
+ PrimaryHeader.SizeOfPartitionEntry = 5;
+ Status = SanitizePrimaryHeaderAllocationSize (&PrimaryHeader, &AllocationSize);
+ UT_ASSERT_EQUAL (Status, EFI_BAD_BUFFER_SIZE);
+
+ // Test the inverse
+ PrimaryHeader.NumberOfPartitionEntries = 5;
+ PrimaryHeader.SizeOfPartitionEntry = MAX_UINT32;
+ Status = SanitizePrimaryHeaderAllocationSize (&PrimaryHeader, &AllocationSize);
+ UT_ASSERT_EQUAL (Status, EFI_BAD_BUFFER_SIZE);
+
+ // Test the worst case scenario
+ PrimaryHeader.NumberOfPartitionEntries = MAX_UINT32;
+ PrimaryHeader.SizeOfPartitionEntry = MAX_UINT32;
+ Status = SanitizePrimaryHeaderAllocationSize (&PrimaryHeader, &AllocationSize);
+ UT_ASSERT_EQUAL (Status, EFI_BAD_BUFFER_SIZE);
+
+ DEBUG ((DEBUG_INFO, "%a: Test passed\n", __func__));
+
+ return UNIT_TEST_PASSED;
+}
+
+/**
+ This function tests the SanitizePrimaryHeaderGptEventSize function.
+ It's intent is to test that the untrusted input from a EFI_GPT_DATA structure
+ will not cause an overflow when calculating the event size.
+
+ @param[in] Context The unit test context.
+
+ @retval UNIT_TEST_PASSED The test passed.
+ @retval UNIT_TEST_ERROR_TEST_FAILED The test failed.
+**/
+UNIT_TEST_STATUS
+EFIAPI
+TestSanitizePrimaryHeaderGptEventSize (
+ IN UNIT_TEST_CONTEXT Context
+ )
+{
+ UINT32 EventSize;
+ UINT32 ExistingLogicEventSize;
+ EFI_STATUS Status;
+ EFI_PARTITION_TABLE_HEADER PrimaryHeader;
+ UINTN NumberOfPartition;
+ EFI_GPT_DATA *GptData;
+
+ GptData = NULL;
+
+ // Test that a normal PrimaryHeader passes validation
+ PrimaryHeader.NumberOfPartitionEntries = 5;
+ PrimaryHeader.SizeOfPartitionEntry = DEFAULT_PRIMARY_TABLE_HEADER_SIZE_OF_PARTITION_ENTRY;
+
+ // set the number of partitions
+ NumberOfPartition = 13;
+
+ // that the primary event size is correct
+ Status = SanitizePrimaryHeaderGptEventSize (&PrimaryHeader, NumberOfPartition, &EventSize);
+ UT_ASSERT_NOT_EFI_ERROR (Status);
+
+ // Calculate the existing logic event size
+ ExistingLogicEventSize = (UINT32)(sizeof (TCG_PCR_EVENT_HDR) + OFFSET_OF (EFI_GPT_DATA, Partitions)
+ + NumberOfPartition * PrimaryHeader.SizeOfPartitionEntry);
+
+ // Check that the event size is correct
+ UT_ASSERT_EQUAL (EventSize, ExistingLogicEventSize);
+
+ // Tests that the primary event size may not overflow
+ Status = SanitizePrimaryHeaderGptEventSize (&PrimaryHeader, MAX_UINT32, &EventSize);
+ UT_ASSERT_EQUAL (Status, EFI_BAD_BUFFER_SIZE);
+
+ // Test that the size of partition entries may not overflow
+ PrimaryHeader.SizeOfPartitionEntry = MAX_UINT32;
+ Status = SanitizePrimaryHeaderGptEventSize (&PrimaryHeader, NumberOfPartition, &EventSize);
+ UT_ASSERT_EQUAL (Status, EFI_BAD_BUFFER_SIZE);
+
+ DEBUG ((DEBUG_INFO, "%a: Test passed\n", __func__));
+
+ return UNIT_TEST_PASSED;
+}
+
+// *--------------------------------------------------------------------*
+// * Unit Test Code Main Function
+// *--------------------------------------------------------------------*
+
+/**
+ This function acts as the entry point for the unit tests.
+
+ @param argc - The number of command line arguments
+ @param argv - The command line arguments
+
+ @return int - The status of the test
+**/
+EFI_STATUS
+EFIAPI
+UefiTestMain (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+ UNIT_TEST_FRAMEWORK_HANDLE Framework;
+ UNIT_TEST_SUITE_HANDLE TcgMeasureBootLibValidationTestSuite;
+
+ Framework = NULL;
+
+ DEBUG ((DEBUG_INFO, "%a: TestMain() - Start\n", UNIT_TEST_NAME));
+
+ Status = InitUnitTestFramework (&Framework, UNIT_TEST_NAME, gEfiCallerBaseName, UNIT_TEST_VERSION);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: Failed in InitUnitTestFramework. Status = %r\n", UNIT_TEST_NAME, Status));
+ goto EXIT;
+ }
+
+ Status = CreateUnitTestSuite (&TcgMeasureBootLibValidationTestSuite, Framework, "TcgMeasureBootLibValidationTestSuite", "Common.TcgMeasureBootLibValidation", NULL, NULL);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%s: Failed in CreateUnitTestSuite for TcgMeasureBootLibValidationTestSuite\n", UNIT_TEST_NAME));
+ Status = EFI_OUT_OF_RESOURCES;
+ goto EXIT;
+ }
+
+ // -----------Suite---------------------------------Description----------------------------Class----------------------------------Test Function------------------------Pre---Clean-Context
+ AddTestCase (TcgMeasureBootLibValidationTestSuite, "Tests Validating EFI Partition Table", "Common.TcgMeasureBootLibValidation", TestSanitizeEfiPartitionTableHeader, NULL, NULL, NULL);
+ AddTestCase (TcgMeasureBootLibValidationTestSuite, "Tests Primary header gpt event checks for overflow", "Common.TcgMeasureBootLibValidation", TestSanitizePrimaryHeaderAllocationSize, NULL, NULL, NULL);
+ AddTestCase (TcgMeasureBootLibValidationTestSuite, "Tests Primary header allocation size checks for overflow", "Common.TcgMeasureBootLibValidation", TestSanitizePrimaryHeaderGptEventSize, NULL, NULL, NULL);
+
+ Status = RunAllTestSuites (Framework);
+
+EXIT:
+ if (Framework != NULL) {
+ FreeUnitTestFramework (Framework);
+ }
+
+ DEBUG ((DEBUG_INFO, "%a: TestMain() - End\n", UNIT_TEST_NAME));
+ return Status;
+}
+
+///
+/// Avoid ECC error for function name that starts with lower case letter
+///
+#define DxeTpmMeasureBootLibUnitTestMain main
+
+/**
+ Standard POSIX C entry point for host based unit test execution.
+
+ @param[in] Argc Number of arguments
+ @param[in] Argv Array of pointers to arguments
+
+ @retval 0 Success
+ @retval other Error
+**/
+INT32
+DxeTpmMeasureBootLibUnitTestMain (
+ IN INT32 Argc,
+ IN CHAR8 *Argv[]
+ )
+{
+ return (INT32)UefiTestMain ();
+}
diff --git a/SecurityPkg/SecurityPkg.ci.yaml b/SecurityPkg/SecurityPkg.ci.yaml
index 24389531afaa..53e5b1fd8e69 100644
--- a/SecurityPkg/SecurityPkg.ci.yaml
+++ b/SecurityPkg/SecurityPkg.ci.yaml
@@ -17,6 +17,7 @@
"ExceptionList": [
"8005", "gRT",
"8001", "DxeTpm2MeasureBootLibUnitTestMain",
+ "8001", "DxeTpmMeasureBootLibUnitTestMain"
],
## Both file path and directory path are accepted.
"IgnoreFiles": [
--
2.43.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113758): https://edk2.groups.io/g/devel/message/113758
Mute This Topic: https://groups.io/mt/103689721/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [edk2-devel] [PATCH 3/6] SecurityPkg: : Adding CVE 2022-36763 to SecurityFixes.yaml
2024-01-11 18:16 [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118 Doug Flick via groups.io
2024-01-11 18:16 ` [edk2-devel] [PATCH 1/6] SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4117 - CVE 2022-36763 Doug Flick via groups.io
2024-01-11 18:16 ` [edk2-devel] [PATCH 2/6] SecurityPkg: DxeTpmMeasureBootLib: " Doug Flick via groups.io
@ 2024-01-11 18:16 ` Doug Flick via groups.io
2024-01-11 18:16 ` [edk2-devel] [PATCH 4/6] SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4118 - CVE 2022-36764 Doug Flick via groups.io
` (5 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Doug Flick via groups.io @ 2024-01-11 18:16 UTC (permalink / raw)
To: devel; +Cc: Douglas Flick [MSFT], Jiewen Yao
This creates / adds a security file that tracks the security fixes
found in this package and can be used to find the fixes that were
applied.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com>
---
SecurityPkg/SecurityFixes.yaml | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 SecurityPkg/SecurityFixes.yaml
diff --git a/SecurityPkg/SecurityFixes.yaml b/SecurityPkg/SecurityFixes.yaml
new file mode 100644
index 000000000000..f9e3e7be7453
--- /dev/null
+++ b/SecurityPkg/SecurityFixes.yaml
@@ -0,0 +1,22 @@
+## @file
+# Security Fixes for SecurityPkg
+#
+# Copyright (c) Microsoft Corporation
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+CVE_2022_36763:
+ commit_titles:
+ - "SecurityPkg: DxeTpm2Measurement: SECURITY PATCH 4117 - CVE 2022-36763"
+ - "SecurityPkg: DxeTpmMeasurement: SECURITY PATCH 4117 - CVE 2022-36763"
+ - "SecurityPkg: : Adding CVE 2022-36763 to SecurityFixes.yaml"
+ cve: CVE-2022-36763
+ date_reported: 2022-10-25 11:31 UTC
+ description: (CVE-2022-36763) - Heap Buffer Overflow in Tcg2MeasureGptTable()
+ note: This patch is related to and supersedes TCBZ2168
+ files_impacted:
+ - Library\DxeTpm2MeasureBootLib\DxeTpm2MeasureBootLib.c
+ - Library\DxeTpmMeasureBootLib\DxeTpmMeasureBootLib.c
+ links:
+ - https://bugzilla.tianocore.org/show_bug.cgi?id=4117
+ - https://bugzilla.tianocore.org/show_bug.cgi?id=2168
+ - https://bugzilla.tianocore.org/show_bug.cgi?id=1990
--
2.43.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113759): https://edk2.groups.io/g/devel/message/113759
Mute This Topic: https://groups.io/mt/103689722/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [edk2-devel] [PATCH 4/6] SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4118 - CVE 2022-36764
2024-01-11 18:16 [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118 Doug Flick via groups.io
` (2 preceding siblings ...)
2024-01-11 18:16 ` [edk2-devel] [PATCH 3/6] SecurityPkg: : Adding CVE 2022-36763 to SecurityFixes.yaml Doug Flick via groups.io
@ 2024-01-11 18:16 ` Doug Flick via groups.io
2024-01-11 18:16 ` [edk2-devel] [PATCH 5/6] SecurityPkg: DxeTpmMeasureBootLib: " Doug Flick via groups.io
` (4 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Doug Flick via groups.io @ 2024-01-11 18:16 UTC (permalink / raw)
To: devel; +Cc: Douglas Flick [MSFT], Jiewen Yao
This commit contains the patch files and tests for DxeTpm2MeasureBootLib
CVE 2022-36764.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com>
---
.../DxeTpm2MeasureBootLibSanitization.h | 28 ++++++++-
.../DxeTpm2MeasureBootLib.c | 12 ++--
.../DxeTpm2MeasureBootLibSanitization.c | 46 +++++++++++++-
.../DxeTpm2MeasureBootLibSanitizationTest.c | 60 ++++++++++++++++---
4 files changed, 131 insertions(+), 15 deletions(-)
diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.h b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.h
index 048b73898744..8f72ba42401f 100644
--- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.h
+++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.h
@@ -9,6 +9,9 @@
Tcg2MeasureGptTable() function will receive untrusted GPT partition table, and parse
partition data carefully.
+ Tcg2MeasurePeImage() function will accept untrusted PE/COFF image and validate its
+ data structure within this image buffer before use.
+
Copyright (c) Microsoft Corporation.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -110,4 +113,27 @@ SanitizePrimaryHeaderGptEventSize (
OUT UINT32 *EventSize
);
-#endif // DXE_TPM2_MEASURE_BOOT_LIB_SANITATION_
+/**
+ This function will validate that the PeImage Event Size from the loaded image is sane
+ It will check the following:
+ - EventSize does not overflow
+
+ @param[in] FilePathSize - Size of the file path.
+ @param[out] EventSize - Pointer to the event size.
+
+ @retval EFI_SUCCESS
+ The event size is valid.
+
+ @retval EFI_OUT_OF_RESOURCES
+ Overflow would have occurred.
+
+ @retval EFI_INVALID_PARAMETER
+ One of the passed parameters was invalid.
+**/
+EFI_STATUS
+SanitizePeImageEventSize (
+ IN UINT32 FilePathSize,
+ OUT UINT32 *EventSize
+ );
+
+#endif // DXE_TPM2_MEASURE_BOOT_LIB_VALIDATION_
diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
index 0475103d6ef8..714cc8e03e80 100644
--- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
+++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
@@ -378,7 +378,6 @@ Exit:
@retval EFI_OUT_OF_RESOURCES No enough resource to measure image.
@retval EFI_UNSUPPORTED ImageType is unsupported or PE image is mal-format.
@retval other error value
-
**/
EFI_STATUS
EFIAPI
@@ -405,6 +404,7 @@ Tcg2MeasurePeImage (
Status = EFI_UNSUPPORTED;
ImageLoad = NULL;
EventPtr = NULL;
+ Tcg2Event = NULL;
Tcg2Protocol = MeasureBootProtocols->Tcg2Protocol;
CcProtocol = MeasureBootProtocols->CcProtocol;
@@ -420,18 +420,22 @@ Tcg2MeasurePeImage (
}
FilePathSize = (UINT32)GetDevicePathSize (FilePath);
+ Status = SanitizePeImageEventSize (FilePathSize, &EventSize);
+ if (EFI_ERROR (Status)) {
+ return EFI_UNSUPPORTED;
+ }
//
// Determine destination PCR by BootPolicy
//
- EventSize = sizeof (*ImageLoad) - sizeof (ImageLoad->DevicePath) + FilePathSize;
- EventPtr = AllocateZeroPool (EventSize + sizeof (EFI_TCG2_EVENT) - sizeof (Tcg2Event->Event));
+ // from a malicious GPT disk partition
+ EventPtr = AllocateZeroPool (EventSize);
if (EventPtr == NULL) {
return EFI_OUT_OF_RESOURCES;
}
Tcg2Event = (EFI_TCG2_EVENT *)EventPtr;
- Tcg2Event->Size = EventSize + sizeof (EFI_TCG2_EVENT) - sizeof (Tcg2Event->Event);
+ Tcg2Event->Size = EventSize;
Tcg2Event->Header.HeaderSize = sizeof (EFI_TCG2_EVENT_HEADER);
Tcg2Event->Header.HeaderVersion = EFI_TCG2_EVENT_HEADER_VERSION;
ImageLoad = (EFI_IMAGE_LOAD_EVENT *)Tcg2Event->Event;
diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.c b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.c
index e2309655d384..2a4d52c6d5cf 100644
--- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.c
+++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.c
@@ -151,7 +151,7 @@ SanitizeEfiPartitionTableHeader (
}
/**
- This function will validate that the allocation size from the primary header is sane
+ This function will validate that the allocation size from the primary header is sane
It will check the following:
- AllocationSize does not overflow
@@ -273,3 +273,47 @@ SanitizePrimaryHeaderGptEventSize (
return EFI_SUCCESS;
}
+
+/**
+ This function will validate that the PeImage Event Size from the loaded image is sane
+ It will check the following:
+ - EventSize does not overflow
+
+ @param[in] FilePathSize - Size of the file path.
+ @param[out] EventSize - Pointer to the event size.
+
+ @retval EFI_SUCCESS
+ The event size is valid.
+
+ @retval EFI_OUT_OF_RESOURCES
+ Overflow would have occurred.
+
+ @retval EFI_INVALID_PARAMETER
+ One of the passed parameters was invalid.
+**/
+EFI_STATUS
+SanitizePeImageEventSize (
+ IN UINT32 FilePathSize,
+ OUT UINT32 *EventSize
+ )
+{
+ EFI_STATUS Status;
+
+ // Replacing logic:
+ // sizeof (*ImageLoad) - sizeof (ImageLoad->DevicePath) + FilePathSize;
+ Status = SafeUint32Add (OFFSET_OF (EFI_IMAGE_LOAD_EVENT, DevicePath), FilePathSize, EventSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "EventSize would overflow!\n"));
+ return EFI_BAD_BUFFER_SIZE;
+ }
+
+ // Replacing logic:
+ // EventSize + sizeof (EFI_TCG2_EVENT) - sizeof (Tcg2Event->Event)
+ Status = SafeUint32Add (*EventSize, OFFSET_OF (EFI_TCG2_EVENT, Event), EventSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "EventSize would overflow!\n"));
+ return EFI_BAD_BUFFER_SIZE;
+ }
+
+ return EFI_SUCCESS;
+}
diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTest.c b/SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTest.c
index 3eb9763e3c91..820e99aeb9b4 100644
--- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTest.c
+++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2MeasureBootLibSanitizationTest.c
@@ -72,10 +72,10 @@ TestSanitizeEfiPartitionTableHeader (
PrimaryHeader.Header.Revision = DEFAULT_PRIMARY_TABLE_HEADER_REVISION;
PrimaryHeader.Header.HeaderSize = sizeof (EFI_PARTITION_TABLE_HEADER);
PrimaryHeader.MyLBA = 1;
- PrimaryHeader.AlternateLBA = 2;
- PrimaryHeader.FirstUsableLBA = 3;
- PrimaryHeader.LastUsableLBA = 4;
- PrimaryHeader.PartitionEntryLBA = 5;
+ PrimaryHeader.PartitionEntryLBA = 2;
+ PrimaryHeader.AlternateLBA = 3;
+ PrimaryHeader.FirstUsableLBA = 4;
+ PrimaryHeader.LastUsableLBA = 5;
PrimaryHeader.NumberOfPartitionEntries = DEFAULT_PRIMARY_TABLE_HEADER_NUMBER_OF_PARTITION_ENTRIES;
PrimaryHeader.SizeOfPartitionEntry = DEFAULT_PRIMARY_TABLE_HEADER_SIZE_OF_PARTITION_ENTRY;
PrimaryHeader.PartitionEntryArrayCRC32 = 0; // Purposely invalid
@@ -187,11 +187,6 @@ TestSanitizePrimaryHeaderGptEventSize (
EFI_STATUS Status;
EFI_PARTITION_TABLE_HEADER PrimaryHeader;
UINTN NumberOfPartition;
- EFI_GPT_DATA *GptData;
- EFI_TCG2_EVENT *Tcg2Event;
-
- Tcg2Event = NULL;
- GptData = NULL;
// Test that a normal PrimaryHeader passes validation
PrimaryHeader.NumberOfPartitionEntries = 5;
@@ -225,6 +220,52 @@ TestSanitizePrimaryHeaderGptEventSize (
return UNIT_TEST_PASSED;
}
+/**
+ This function tests the SanitizePeImageEventSize function.
+ It's intent is to test that the untrusted input from a file path when generating a
+ EFI_IMAGE_LOAD_EVENT structure will not cause an overflow when calculating
+ the event size when allocating space
+
+ @param[in] Context The unit test context.
+
+ @retval UNIT_TEST_PASSED The test passed.
+ @retval UNIT_TEST_ERROR_TEST_FAILED The test failed.
+**/
+UNIT_TEST_STATUS
+EFIAPI
+TestSanitizePeImageEventSize (
+ IN UNIT_TEST_CONTEXT Context
+ )
+{
+ UINT32 EventSize;
+ UINTN ExistingLogicEventSize;
+ UINT32 FilePathSize;
+ EFI_STATUS Status;
+
+ FilePathSize = 255;
+
+ // Test that a normal PE image passes validation
+ Status = SanitizePeImageEventSize (FilePathSize, &EventSize);
+ UT_ASSERT_EQUAL (Status, EFI_SUCCESS);
+
+ // Test that the event size is correct compared to the existing logic
+ ExistingLogicEventSize = OFFSET_OF (EFI_IMAGE_LOAD_EVENT, DevicePath) + FilePathSize;
+ ExistingLogicEventSize += OFFSET_OF (EFI_TCG2_EVENT, Event);
+
+ if (EventSize != ExistingLogicEventSize) {
+ UT_LOG_ERROR ("SanitizePeImageEventSize returned an incorrect event size. Expected %u, got %u\n", ExistingLogicEventSize, EventSize);
+ return UNIT_TEST_ERROR_TEST_FAILED;
+ }
+
+ // Test that the event size may not overflow
+ Status = SanitizePeImageEventSize (MAX_UINT32, &EventSize);
+ UT_ASSERT_EQUAL (Status, EFI_BAD_BUFFER_SIZE);
+
+ DEBUG ((DEBUG_INFO, "%a: Test passed\n", __func__));
+
+ return UNIT_TEST_PASSED;
+}
+
// *--------------------------------------------------------------------*
// * Unit Test Code Main Function
// *--------------------------------------------------------------------*
@@ -267,6 +308,7 @@ UefiTestMain (
AddTestCase (Tcg2MeasureBootLibValidationTestSuite, "Tests Validating EFI Partition Table", "Common.Tcg2MeasureBootLibValidation", TestSanitizeEfiPartitionTableHeader, NULL, NULL, NULL);
AddTestCase (Tcg2MeasureBootLibValidationTestSuite, "Tests Primary header gpt event checks for overflow", "Common.Tcg2MeasureBootLibValidation", TestSanitizePrimaryHeaderAllocationSize, NULL, NULL, NULL);
AddTestCase (Tcg2MeasureBootLibValidationTestSuite, "Tests Primary header allocation size checks for overflow", "Common.Tcg2MeasureBootLibValidation", TestSanitizePrimaryHeaderGptEventSize, NULL, NULL, NULL);
+ AddTestCase (Tcg2MeasureBootLibValidationTestSuite, "Tests PE Image and FileSize checks for overflow", "Common.Tcg2MeasureBootLibValidation", TestSanitizePeImageEventSize, NULL, NULL, NULL);
Status = RunAllTestSuites (Framework);
--
2.43.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113760): https://edk2.groups.io/g/devel/message/113760
Mute This Topic: https://groups.io/mt/103689723/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [edk2-devel] [PATCH 5/6] SecurityPkg: DxeTpmMeasureBootLib: SECURITY PATCH 4118 - CVE 2022-36764
2024-01-11 18:16 [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118 Doug Flick via groups.io
` (3 preceding siblings ...)
2024-01-11 18:16 ` [edk2-devel] [PATCH 4/6] SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4118 - CVE 2022-36764 Doug Flick via groups.io
@ 2024-01-11 18:16 ` Doug Flick via groups.io
2024-01-11 18:16 ` [edk2-devel] [PATCH 6/6] SecurityPkg: : Adding CVE 2022-36764 to SecurityFixes.yaml Doug Flick via groups.io
` (3 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Doug Flick via groups.io @ 2024-01-11 18:16 UTC (permalink / raw)
To: devel; +Cc: Douglas Flick [MSFT], Jiewen Yao
This commit contains the patch files and tests for DxeTpmMeasureBootLib
CVE 2022-36764.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com>
---
.../DxeTpmMeasureBootLibSanitization.h | 23 +++++
.../DxeTpmMeasureBootLib.c | 13 ++-
.../DxeTpmMeasureBootLibSanitization.c | 44 +++++++++
.../DxeTpmMeasureBootLibSanitizationTest.c | 98 +++++++++++++++++--
4 files changed, 168 insertions(+), 10 deletions(-)
diff --git a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.h b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.h
index 0d9d00c281f6..2248495813b5 100644
--- a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.h
+++ b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.h
@@ -111,4 +111,27 @@ SanitizePrimaryHeaderGptEventSize (
OUT UINT32 *EventSize
);
+/**
+ This function will validate that the PeImage Event Size from the loaded image is sane
+ It will check the following:
+ - EventSize does not overflow
+
+ @param[in] FilePathSize - Size of the file path.
+ @param[out] EventSize - Pointer to the event size.
+
+ @retval EFI_SUCCESS
+ The event size is valid.
+
+ @retval EFI_OUT_OF_RESOURCES
+ Overflow would have occurred.
+
+ @retval EFI_INVALID_PARAMETER
+ One of the passed parameters was invalid.
+**/
+EFI_STATUS
+SanitizePeImageEventSize (
+ IN UINT32 FilePathSize,
+ OUT UINT32 *EventSize
+ );
+
#endif // DXE_TPM_MEASURE_BOOT_LIB_VALIDATION_
diff --git a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
index 669ab1913440..a9fc440a091e 100644
--- a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
+++ b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
@@ -17,6 +17,7 @@
Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
+Copyright (c) Microsoft Corporation.<BR>
Copyright (c) Microsoft Corporation.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -345,18 +346,22 @@ TcgMeasurePeImage (
ImageLoad = NULL;
SectionHeader = NULL;
Sha1Ctx = NULL;
+ TcgEvent = NULL;
FilePathSize = (UINT32)GetDevicePathSize (FilePath);
- //
// Determine destination PCR by BootPolicy
//
- EventSize = sizeof (*ImageLoad) - sizeof (ImageLoad->DevicePath) + FilePathSize;
- TcgEvent = AllocateZeroPool (EventSize + sizeof (TCG_PCR_EVENT));
+ Status = SanitizePeImageEventSize (FilePathSize, &EventSize);
+ if (EFI_ERROR (Status)) {
+ return EFI_UNSUPPORTED;
+ }
+
+ TcgEvent = AllocateZeroPool (EventSize);
if (TcgEvent == NULL) {
return EFI_OUT_OF_RESOURCES;
}
- TcgEvent->EventSize = EventSize;
+ TcgEvent->EventSize = EventSize - sizeof (TCG_PCR_EVENT_HDR);
ImageLoad = (EFI_IMAGE_LOAD_EVENT *)TcgEvent->Event;
switch (ImageType) {
diff --git a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.c b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.c
index a3fa46f5e632..c989851cec2d 100644
--- a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.c
+++ b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitization.c
@@ -239,3 +239,47 @@ SanitizePrimaryHeaderGptEventSize (
return EFI_SUCCESS;
}
+
+/**
+ This function will validate that the PeImage Event Size from the loaded image is sane
+ It will check the following:
+ - EventSize does not overflow
+
+ @param[in] FilePathSize - Size of the file path.
+ @param[out] EventSize - Pointer to the event size.
+
+ @retval EFI_SUCCESS
+ The event size is valid.
+
+ @retval EFI_OUT_OF_RESOURCES
+ Overflow would have occurred.
+
+ @retval EFI_INVALID_PARAMETER
+ One of the passed parameters was invalid.
+**/
+EFI_STATUS
+SanitizePeImageEventSize (
+ IN UINT32 FilePathSize,
+ OUT UINT32 *EventSize
+ )
+{
+ EFI_STATUS Status;
+
+ // Replacing logic:
+ // sizeof (*ImageLoad) - sizeof (ImageLoad->DevicePath) + FilePathSize;
+ Status = SafeUint32Add (OFFSET_OF (EFI_IMAGE_LOAD_EVENT, DevicePath), FilePathSize, EventSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "EventSize would overflow!\n"));
+ return EFI_BAD_BUFFER_SIZE;
+ }
+
+ // Replacing logic:
+ // EventSize + sizeof (TCG_PCR_EVENT_HDR)
+ Status = SafeUint32Add (*EventSize, sizeof (TCG_PCR_EVENT_HDR), EventSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "EventSize would overflow!\n"));
+ return EFI_BAD_BUFFER_SIZE;
+ }
+
+ return EFI_SUCCESS;
+}
diff --git a/SecurityPkg/Library/DxeTpmMeasureBootLib/InternalUnitTest/DxeTpmMeasureBootLibSanitizationTest.c b/SecurityPkg/Library/DxeTpmMeasureBootLib/InternalUnitTest/DxeTpmMeasureBootLibSanitizationTest.c
index eeb928cdb0aa..c41498be4521 100644
--- a/SecurityPkg/Library/DxeTpmMeasureBootLib/InternalUnitTest/DxeTpmMeasureBootLibSanitizationTest.c
+++ b/SecurityPkg/Library/DxeTpmMeasureBootLib/InternalUnitTest/DxeTpmMeasureBootLibSanitizationTest.c
@@ -1,8 +1,8 @@
/** @file
-This file includes the unit test cases for the DxeTpmMeasureBootLibSanitizationTest.c.
+ This file includes the unit test cases for the DxeTpmMeasureBootLibSanitizationTest.c.
-Copyright (c) Microsoft Corporation.<BR>
-SPDX-License-Identifier: BSD-2-Clause-Patent
+ Copyright (c) Microsoft Corporation.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
**/
#include <Uefi.h>
@@ -186,9 +186,6 @@ TestSanitizePrimaryHeaderGptEventSize (
EFI_STATUS Status;
EFI_PARTITION_TABLE_HEADER PrimaryHeader;
UINTN NumberOfPartition;
- EFI_GPT_DATA *GptData;
-
- GptData = NULL;
// Test that a normal PrimaryHeader passes validation
PrimaryHeader.NumberOfPartitionEntries = 5;
@@ -222,6 +219,94 @@ TestSanitizePrimaryHeaderGptEventSize (
return UNIT_TEST_PASSED;
}
+/**
+ This function tests the SanitizePeImageEventSize function.
+ It's intent is to test that the untrusted input from a file path for an
+ EFI_IMAGE_LOAD_EVENT structure will not cause an overflow when calculating
+ the event size when allocating space.
+
+ @param[in] Context The unit test context.
+
+ @retval UNIT_TEST_PASSED The test passed.
+ @retval UNIT_TEST_ERROR_TEST_FAILED The test failed.
+**/
+UNIT_TEST_STATUS
+EFIAPI
+TestSanitizePeImageEventSize (
+ IN UNIT_TEST_CONTEXT Context
+ )
+{
+ UINT32 EventSize;
+ UINTN ExistingLogicEventSize;
+ UINT32 FilePathSize;
+ EFI_STATUS Status;
+ EFI_DEVICE_PATH_PROTOCOL DevicePath;
+ EFI_IMAGE_LOAD_EVENT *ImageLoadEvent;
+ UNIT_TEST_STATUS TestStatus;
+
+ TestStatus = UNIT_TEST_ERROR_TEST_FAILED;
+
+ // Generate EFI_DEVICE_PATH_PROTOCOL test data
+ DevicePath.Type = 0;
+ DevicePath.SubType = 0;
+ DevicePath.Length[0] = 0;
+ DevicePath.Length[1] = 0;
+
+ // Generate EFI_IMAGE_LOAD_EVENT test data
+ ImageLoadEvent = AllocateZeroPool (sizeof (EFI_IMAGE_LOAD_EVENT) + sizeof (EFI_DEVICE_PATH_PROTOCOL));
+ if (ImageLoadEvent == NULL) {
+ DEBUG ((DEBUG_ERROR, "%a: AllocateZeroPool failed\n", __func__));
+ goto Exit;
+ }
+
+ // Populate EFI_IMAGE_LOAD_EVENT54 test data
+ ImageLoadEvent->ImageLocationInMemory = (EFI_PHYSICAL_ADDRESS)0x12345678;
+ ImageLoadEvent->ImageLengthInMemory = 0x1000;
+ ImageLoadEvent->ImageLinkTimeAddress = (UINTN)ImageLoadEvent;
+ ImageLoadEvent->LengthOfDevicePath = sizeof (EFI_DEVICE_PATH_PROTOCOL);
+ CopyMem (ImageLoadEvent->DevicePath, &DevicePath, sizeof (EFI_DEVICE_PATH_PROTOCOL));
+
+ FilePathSize = 255;
+
+ // Test that a normal PE image passes validation
+ Status = SanitizePeImageEventSize (FilePathSize, &EventSize);
+ if (EFI_ERROR (Status)) {
+ UT_LOG_ERROR ("SanitizePeImageEventSize failed with %r\n", Status);
+ goto Exit;
+ }
+
+ // Test that the event size is correct compared to the existing logic
+ ExistingLogicEventSize = OFFSET_OF (EFI_IMAGE_LOAD_EVENT, DevicePath) + FilePathSize;
+ ExistingLogicEventSize += sizeof (TCG_PCR_EVENT_HDR);
+
+ if (EventSize != ExistingLogicEventSize) {
+ UT_LOG_ERROR ("SanitizePeImageEventSize returned an incorrect event size. Expected %u, got %u\n", ExistingLogicEventSize, EventSize);
+ goto Exit;
+ }
+
+ // Test that the event size may not overflow
+ Status = SanitizePeImageEventSize (MAX_UINT32, &EventSize);
+ if (Status != EFI_BAD_BUFFER_SIZE) {
+ UT_LOG_ERROR ("SanitizePeImageEventSize succeded when it was supposed to fail with %r\n", Status);
+ goto Exit;
+ }
+
+ TestStatus = UNIT_TEST_PASSED;
+Exit:
+
+ if (ImageLoadEvent != NULL) {
+ FreePool (ImageLoadEvent);
+ }
+
+ if (TestStatus == UNIT_TEST_ERROR_TEST_FAILED) {
+ DEBUG ((DEBUG_ERROR, "%a: Test failed\n", __func__));
+ } else {
+ DEBUG ((DEBUG_INFO, "%a: Test passed\n", __func__));
+ }
+
+ return TestStatus;
+}
+
// *--------------------------------------------------------------------*
// * Unit Test Code Main Function
// *--------------------------------------------------------------------*
@@ -265,6 +350,7 @@ UefiTestMain (
AddTestCase (TcgMeasureBootLibValidationTestSuite, "Tests Validating EFI Partition Table", "Common.TcgMeasureBootLibValidation", TestSanitizeEfiPartitionTableHeader, NULL, NULL, NULL);
AddTestCase (TcgMeasureBootLibValidationTestSuite, "Tests Primary header gpt event checks for overflow", "Common.TcgMeasureBootLibValidation", TestSanitizePrimaryHeaderAllocationSize, NULL, NULL, NULL);
AddTestCase (TcgMeasureBootLibValidationTestSuite, "Tests Primary header allocation size checks for overflow", "Common.TcgMeasureBootLibValidation", TestSanitizePrimaryHeaderGptEventSize, NULL, NULL, NULL);
+ AddTestCase (TcgMeasureBootLibValidationTestSuite, "Tests PE Image and FileSize checks for overflow", "Common.TcgMeasureBootLibValidation", TestSanitizePeImageEventSize, NULL, NULL, NULL);
Status = RunAllTestSuites (Framework);
--
2.43.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113761): https://edk2.groups.io/g/devel/message/113761
Mute This Topic: https://groups.io/mt/103689724/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [edk2-devel] [PATCH 6/6] SecurityPkg: : Adding CVE 2022-36764 to SecurityFixes.yaml
2024-01-11 18:16 [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118 Doug Flick via groups.io
` (4 preceding siblings ...)
2024-01-11 18:16 ` [edk2-devel] [PATCH 5/6] SecurityPkg: DxeTpmMeasureBootLib: " Doug Flick via groups.io
@ 2024-01-11 18:16 ` Doug Flick via groups.io
2024-01-12 1:50 ` [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118 Yao, Jiewen
` (2 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Doug Flick via groups.io @ 2024-01-11 18:16 UTC (permalink / raw)
To: devel; +Cc: Douglas Flick [MSFT], Jiewen Yao
This creates / adds a security file that tracks the security fixes
found in this package and can be used to find the fixes that were
applied.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com>
---
SecurityPkg/SecurityFixes.yaml | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/SecurityPkg/SecurityFixes.yaml b/SecurityPkg/SecurityFixes.yaml
index f9e3e7be7453..833fb827a96c 100644
--- a/SecurityPkg/SecurityFixes.yaml
+++ b/SecurityPkg/SecurityFixes.yaml
@@ -20,3 +20,17 @@ CVE_2022_36763:
- https://bugzilla.tianocore.org/show_bug.cgi?id=4117
- https://bugzilla.tianocore.org/show_bug.cgi?id=2168
- https://bugzilla.tianocore.org/show_bug.cgi?id=1990
+CVE_2022_36764:
+ commit_titles:
+ - "SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4118 - CVE 2022-36764"
+ - "SecurityPkg: DxeTpmMeasureBootLib: SECURITY PATCH 4118 - CVE 2022-36764"
+ - "SecurityPkg: : Adding CVE 2022-36764 to SecurityFixes.yaml"
+ cve: CVE-2022-36764
+ date_reported: 2022-10-25 12:23 UTC
+ description: Heap Buffer Overflow in Tcg2MeasurePeImage()
+ note:
+ files_impacted:
+ - Library\DxeTpm2MeasureBootLib\DxeTpm2MeasureBootLib.c
+ - Library\DxeTpmMeasureBootLib\DxeTpmMeasureBootLib.c
+ links:
+ - https://bugzilla.tianocore.org/show_bug.cgi?id=4118
--
2.43.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113762): https://edk2.groups.io/g/devel/message/113762
Mute This Topic: https://groups.io/mt/103689725/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
2024-01-11 18:16 [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118 Doug Flick via groups.io
` (5 preceding siblings ...)
2024-01-11 18:16 ` [edk2-devel] [PATCH 6/6] SecurityPkg: : Adding CVE 2022-36764 to SecurityFixes.yaml Doug Flick via groups.io
@ 2024-01-12 1:50 ` Yao, Jiewen
2024-01-16 7:59 ` Yao, Jiewen
2024-01-16 12:00 ` Gerd Hoffmann
8 siblings, 0 replies; 21+ messages in thread
From: Yao, Jiewen @ 2024-01-12 1:50 UTC (permalink / raw)
To: Douglas Flick [MSFT], devel@edk2.groups.io; +Cc: Yao, Jiewen
Hi Doug
Thanks for the fix.
Please remember to CC all SecurityPkg maintainer and reviewer.
I will merge after several days to see if there is any additional feedback from the community.
Thank you
Yao, Jiewen
> -----Original Message-----
> From: Douglas Flick [MSFT] <doug.edk2@gmail.com>
> Sent: Friday, January 12, 2024 2:16 AM
> To: devel@edk2.groups.io
> Cc: Douglas Flick [MSFT] <doug.edk2@gmail.com>; Yao, Jiewen
> <jiewen.yao@intel.com>
> Subject: [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
>
> This patch series include the combined / merged security patches
> (as seperate commits) for TCBZ4117 (CVE-2022-36763) and TCBZ4118
> (CVE-2022-36764) for DxeTpm2MeasureBootLib and DxeTpmMeasureBootLib.
> These patches have already been reviewed by SecurityPkg Maintainer
> (Jiewen) on GHSA.
>
> This patch series (specifically TCBZ4117) supersedes TCBZ2168.
>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
>
> Douglas Flick [MSFT] (6):
> SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4117 - CVE
> 2022-36763
> SecurityPkg: DxeTpmMeasureBootLib: SECURITY PATCH 4117 - CVE
> 2022-36763
> SecurityPkg: : Adding CVE 2022-36763 to SecurityFixes.yaml
> SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4118 - CVE
> 2022-36764
> SecurityPkg: DxeTpmMeasureBootLib: SECURITY PATCH 4118 - CVE
> 2022-36764
> SecurityPkg: : Adding CVE 2022-36764 to SecurityFixes.yaml
>
> SecurityPkg/Test/SecurityPkgHostTest.dsc | 2 +
> .../DxeTpm2MeasureBootLib.inf | 4 +-
> ...Tpm2MeasureBootLibSanitizationTestHost.inf | 28 ++
> .../DxeTpmMeasureBootLib.inf | 4 +-
> ...eTpmMeasureBootLibSanitizationTestHost.inf | 28 ++
> .../DxeTpm2MeasureBootLibSanitization.h | 139 +++++++
> .../DxeTpmMeasureBootLibSanitization.h | 137 +++++++
> .../DxeTpm2MeasureBootLib.c | 87 ++--
> .../DxeTpm2MeasureBootLibSanitization.c | 319 +++++++++++++++
> .../DxeTpm2MeasureBootLibSanitizationTest.c | 345 ++++++++++++++++
> .../DxeTpmMeasureBootLib.c | 53 ++-
> .../DxeTpmMeasureBootLibSanitization.c | 285 +++++++++++++
> .../DxeTpmMeasureBootLibSanitizationTest.c | 387 ++++++++++++++++++
> SecurityPkg/SecurityFixes.yaml | 36 ++
> SecurityPkg/SecurityPkg.ci.yaml | 2 +
> 15 files changed, 1801 insertions(+), 55 deletions(-)
> create mode 100644
> SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2Measur
> eBootLibSanitizationTestHost.inf
> create mode 100644
> SecurityPkg/Library/DxeTpmMeasureBootLib/InternalUnitTest/DxeTpmMeasureB
> ootLibSanitizationTestHost.inf
> create mode 100644
> SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitiza
> tion.h
> create mode 100644
> SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitizatio
> n.h
> create mode 100644
> SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitiza
> tion.c
> create mode 100644
> SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2Measur
> eBootLibSanitizationTest.c
> create mode 100644
> SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitizatio
> n.c
> create mode 100644
> SecurityPkg/Library/DxeTpmMeasureBootLib/InternalUnitTest/DxeTpmMeasureB
> ootLibSanitizationTest.c
> create mode 100644 SecurityPkg/SecurityFixes.yaml
>
> --
> 2.43.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113637): https://edk2.groups.io/g/devel/message/113637
Mute This Topic: https://groups.io/mt/103675434/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
2024-01-11 18:16 [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118 Doug Flick via groups.io
` (6 preceding siblings ...)
2024-01-12 1:50 ` [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118 Yao, Jiewen
@ 2024-01-16 7:59 ` Yao, Jiewen
2024-01-16 12:00 ` Gerd Hoffmann
8 siblings, 0 replies; 21+ messages in thread
From: Yao, Jiewen @ 2024-01-16 7:59 UTC (permalink / raw)
To: Douglas Flick [MSFT], devel@edk2.groups.io
Merged https://github.com/tianocore/edk2/pull/5264
> -----Original Message-----
> From: Douglas Flick [MSFT] <doug.edk2@gmail.com>
> Sent: Friday, January 12, 2024 2:16 AM
> To: devel@edk2.groups.io
> Cc: Douglas Flick [MSFT] <doug.edk2@gmail.com>; Yao, Jiewen
> <jiewen.yao@intel.com>
> Subject: [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
>
> This patch series include the combined / merged security patches
> (as seperate commits) for TCBZ4117 (CVE-2022-36763) and TCBZ4118
> (CVE-2022-36764) for DxeTpm2MeasureBootLib and DxeTpmMeasureBootLib.
> These patches have already been reviewed by SecurityPkg Maintainer
> (Jiewen) on GHSA.
>
> This patch series (specifically TCBZ4117) supersedes TCBZ2168.
>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
>
> Douglas Flick [MSFT] (6):
> SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4117 - CVE
> 2022-36763
> SecurityPkg: DxeTpmMeasureBootLib: SECURITY PATCH 4117 - CVE
> 2022-36763
> SecurityPkg: : Adding CVE 2022-36763 to SecurityFixes.yaml
> SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4118 - CVE
> 2022-36764
> SecurityPkg: DxeTpmMeasureBootLib: SECURITY PATCH 4118 - CVE
> 2022-36764
> SecurityPkg: : Adding CVE 2022-36764 to SecurityFixes.yaml
>
> SecurityPkg/Test/SecurityPkgHostTest.dsc | 2 +
> .../DxeTpm2MeasureBootLib.inf | 4 +-
> ...Tpm2MeasureBootLibSanitizationTestHost.inf | 28 ++
> .../DxeTpmMeasureBootLib.inf | 4 +-
> ...eTpmMeasureBootLibSanitizationTestHost.inf | 28 ++
> .../DxeTpm2MeasureBootLibSanitization.h | 139 +++++++
> .../DxeTpmMeasureBootLibSanitization.h | 137 +++++++
> .../DxeTpm2MeasureBootLib.c | 87 ++--
> .../DxeTpm2MeasureBootLibSanitization.c | 319 +++++++++++++++
> .../DxeTpm2MeasureBootLibSanitizationTest.c | 345 ++++++++++++++++
> .../DxeTpmMeasureBootLib.c | 53 ++-
> .../DxeTpmMeasureBootLibSanitization.c | 285 +++++++++++++
> .../DxeTpmMeasureBootLibSanitizationTest.c | 387 ++++++++++++++++++
> SecurityPkg/SecurityFixes.yaml | 36 ++
> SecurityPkg/SecurityPkg.ci.yaml | 2 +
> 15 files changed, 1801 insertions(+), 55 deletions(-)
> create mode 100644
> SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2Measur
> eBootLibSanitizationTestHost.inf
> create mode 100644
> SecurityPkg/Library/DxeTpmMeasureBootLib/InternalUnitTest/DxeTpmMeasureB
> ootLibSanitizationTestHost.inf
> create mode 100644
> SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitiza
> tion.h
> create mode 100644
> SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitizatio
> n.h
> create mode 100644
> SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitiza
> tion.c
> create mode 100644
> SecurityPkg/Library/DxeTpm2MeasureBootLib/InternalUnitTest/DxeTpm2Measur
> eBootLibSanitizationTest.c
> create mode 100644
> SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLibSanitizatio
> n.c
> create mode 100644
> SecurityPkg/Library/DxeTpmMeasureBootLib/InternalUnitTest/DxeTpmMeasureB
> ootLibSanitizationTest.c
> create mode 100644 SecurityPkg/SecurityFixes.yaml
>
> --
> 2.43.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113878): https://edk2.groups.io/g/devel/message/113878
Mute This Topic: https://groups.io/mt/103675434/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
2024-01-11 18:16 [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118 Doug Flick via groups.io
` (7 preceding siblings ...)
2024-01-16 7:59 ` Yao, Jiewen
@ 2024-01-16 12:00 ` Gerd Hoffmann
2024-01-16 13:30 ` Yao, Jiewen
8 siblings, 1 reply; 21+ messages in thread
From: Gerd Hoffmann @ 2024-01-16 12:00 UTC (permalink / raw)
To: devel, dougflick; +Cc: Douglas Flick [MSFT], Jiewen Yao
On Thu, Jan 11, 2024 at 10:16:00AM -0800, Doug Flick via groups.io wrote:
> This patch series include the combined / merged security patches
> (as seperate commits) for TCBZ4117 (CVE-2022-36763) and TCBZ4118
> (CVE-2022-36764) for DxeTpm2MeasureBootLib and DxeTpmMeasureBootLib.
> These patches have already been reviewed by SecurityPkg Maintainer
> (Jiewen) on GHSA.
This patch series breaks ovmf build (duplicate symbols) in case both
TPM2 and TPM1 support are enabled (-D TPM2_ENABLE=TRUE
-DTPM1_ENABLE=TRUE). Compiling with TPM2 only (-D TPM2_ENABLE=TRUE
-DTPM1_ENABLE=FALSE) works fine.
I see two options to deal with the problem:
(1) Rename the Sanitize* functions in the TPM2 version of the library
to carry a '2' somewhere in the function name, simliar to all other
TPM2 functions, to avoid the name clash.
(2) Remove TPM1 support from the edk2 code base. The relevance of
TPM 1.2 support should be close to zero given that the TPM 2.0
specification was released almost a decade ago ...
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113889): https://edk2.groups.io/g/devel/message/113889
Mute This Topic: https://groups.io/mt/103675434/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
2024-01-16 12:00 ` Gerd Hoffmann
@ 2024-01-16 13:30 ` Yao, Jiewen
2024-01-16 14:34 ` Gerd Hoffmann
0 siblings, 1 reply; 21+ messages in thread
From: Yao, Jiewen @ 2024-01-16 13:30 UTC (permalink / raw)
To: Gerd Hoffmann, devel@edk2.groups.io, dougflick@microsoft.com
Cc: Douglas Flick [MSFT]
Gerd
I have merged this patch set today.
I am fine to remove TPM1.2 in OVMF because of the known security limitation.
Thank you
Yao, Jiewen
> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Tuesday, January 16, 2024 8:01 PM
> To: devel@edk2.groups.io; dougflick@microsoft.com
> Cc: Douglas Flick [MSFT] <doug.edk2@gmail.com>; Yao, Jiewen
> <jiewen.yao@intel.com>
> Subject: Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
>
> On Thu, Jan 11, 2024 at 10:16:00AM -0800, Doug Flick via groups.io wrote:
> > This patch series include the combined / merged security patches
> > (as seperate commits) for TCBZ4117 (CVE-2022-36763) and TCBZ4118
> > (CVE-2022-36764) for DxeTpm2MeasureBootLib and DxeTpmMeasureBootLib.
> > These patches have already been reviewed by SecurityPkg Maintainer
> > (Jiewen) on GHSA.
>
> This patch series breaks ovmf build (duplicate symbols) in case both
> TPM2 and TPM1 support are enabled (-D TPM2_ENABLE=TRUE
> -DTPM1_ENABLE=TRUE). Compiling with TPM2 only (-D TPM2_ENABLE=TRUE
> -DTPM1_ENABLE=FALSE) works fine.
>
> I see two options to deal with the problem:
>
> (1) Rename the Sanitize* functions in the TPM2 version of the library
> to carry a '2' somewhere in the function name, simliar to all other
> TPM2 functions, to avoid the name clash.
> (2) Remove TPM1 support from the edk2 code base. The relevance of
> TPM 1.2 support should be close to zero given that the TPM 2.0
> specification was released almost a decade ago ...
>
> take care,
> Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113898): https://edk2.groups.io/g/devel/message/113898
Mute This Topic: https://groups.io/mt/103675434/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
2024-01-16 13:30 ` Yao, Jiewen
@ 2024-01-16 14:34 ` Gerd Hoffmann
2024-01-16 14:38 ` Yao, Jiewen
0 siblings, 1 reply; 21+ messages in thread
From: Gerd Hoffmann @ 2024-01-16 14:34 UTC (permalink / raw)
To: devel, jiewen.yao; +Cc: dougflick@microsoft.com, Douglas Flick [MSFT]
On Tue, Jan 16, 2024 at 01:30:43PM +0000, Yao, Jiewen wrote:
> Gerd
> I have merged this patch set today.
>
> I am fine to remove TPM1.2 in OVMF because of the known security limitation.
I was thinking about the complete edk2 code base not only OVMF.
But I can surely start with OVMF. Maybe it is the only platform
affected because on physical hardware you usually know whenever
TPM 1.2 or TPM 2.0 is present so there is no need to include both.
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113903): https://edk2.groups.io/g/devel/message/113903
Mute This Topic: https://groups.io/mt/103675434/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
2024-01-16 14:34 ` Gerd Hoffmann
@ 2024-01-16 14:38 ` Yao, Jiewen
2024-01-17 7:21 ` Li, Yi
0 siblings, 1 reply; 21+ messages in thread
From: Yao, Jiewen @ 2024-01-16 14:38 UTC (permalink / raw)
To: Gerd Hoffmann, devel@edk2.groups.io
Cc: dougflick@microsoft.com, Douglas Flick [MSFT]
Sure. Let's start from OVMF.
We have leaf enough time for feedback, but I see no comment from other people.
> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Tuesday, January 16, 2024 10:35 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> Cc: dougflick@microsoft.com; Douglas Flick [MSFT] <doug.edk2@gmail.com>
> Subject: Re: Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 &
> TCBZ4118
>
> On Tue, Jan 16, 2024 at 01:30:43PM +0000, Yao, Jiewen wrote:
> > Gerd
> > I have merged this patch set today.
> >
> > I am fine to remove TPM1.2 in OVMF because of the known security limitation.
>
> I was thinking about the complete edk2 code base not only OVMF.
>
> But I can surely start with OVMF. Maybe it is the only platform
> affected because on physical hardware you usually know whenever
> TPM 1.2 or TPM 2.0 is present so there is no need to include both.
>
> take care,
> Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113904): https://edk2.groups.io/g/devel/message/113904
Mute This Topic: https://groups.io/mt/103675434/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
2024-01-16 14:38 ` Yao, Jiewen
@ 2024-01-17 7:21 ` Li, Yi
2024-01-17 8:08 ` Yao, Jiewen
0 siblings, 1 reply; 21+ messages in thread
From: Li, Yi @ 2024-01-17 7:21 UTC (permalink / raw)
To: devel@edk2.groups.io, Yao, Jiewen, Gerd Hoffmann
Cc: dougflick@microsoft.com, Douglas Flick [MSFT]
Hi Jiewen,
All EDK2 PR CI builds of OvmfPkg are broken due to this issue.
Maybe we didn't have enough time to wait feedback and should fix the CI issue first.
Regards,
Yi
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
Sent: Tuesday, January 16, 2024 10:38 PM
To: Gerd Hoffmann <kraxel@redhat.com>; devel@edk2.groups.io
Cc: dougflick@microsoft.com; Douglas Flick [MSFT] <doug.edk2@gmail.com>
Subject: Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
Sure. Let's start from OVMF.
We have leaf enough time for feedback, but I see no comment from other people.
> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Tuesday, January 16, 2024 10:35 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> Cc: dougflick@microsoft.com; Douglas Flick [MSFT]
> <doug.edk2@gmail.com>
> Subject: Re: Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 &
> TCBZ4118
>
> On Tue, Jan 16, 2024 at 01:30:43PM +0000, Yao, Jiewen wrote:
> > Gerd
> > I have merged this patch set today.
> >
> > I am fine to remove TPM1.2 in OVMF because of the known security limitation.
>
> I was thinking about the complete edk2 code base not only OVMF.
>
> But I can surely start with OVMF. Maybe it is the only platform
> affected because on physical hardware you usually know whenever TPM
> 1.2 or TPM 2.0 is present so there is no need to include both.
>
> take care,
> Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113933): https://edk2.groups.io/g/devel/message/113933
Mute This Topic: https://groups.io/mt/103675434/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
2024-01-17 7:21 ` Li, Yi
@ 2024-01-17 8:08 ` Yao, Jiewen
2024-01-17 8:15 ` Li, Yi
0 siblings, 1 reply; 21+ messages in thread
From: Yao, Jiewen @ 2024-01-17 8:08 UTC (permalink / raw)
To: Li, Yi1, devel@edk2.groups.io, Gerd Hoffmann
Cc: dougflick@microsoft.com, Douglas Flick [MSFT]
Please check https://github.com/tianocore/edk2/pull/5264. It is merged after pass CI.
May I know where you see PR CI builds are broken?
Thank you
Yao, Jiewen
> -----Original Message-----
> From: Li, Yi1 <yi1.li@intel.com>
> Sent: Wednesday, January 17, 2024 3:21 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>
> Cc: dougflick@microsoft.com; Douglas Flick [MSFT] <doug.edk2@gmail.com>
> Subject: RE: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
>
> Hi Jiewen,
>
> All EDK2 PR CI builds of OvmfPkg are broken due to this issue.
> Maybe we didn't have enough time to wait feedback and should fix the CI issue
> first.
>
> Regards,
> Yi
>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
> Sent: Tuesday, January 16, 2024 10:38 PM
> To: Gerd Hoffmann <kraxel@redhat.com>; devel@edk2.groups.io
> Cc: dougflick@microsoft.com; Douglas Flick [MSFT] <doug.edk2@gmail.com>
> Subject: Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
>
> Sure. Let's start from OVMF.
>
> We have leaf enough time for feedback, but I see no comment from other people.
>
>
> > -----Original Message-----
> > From: Gerd Hoffmann <kraxel@redhat.com>
> > Sent: Tuesday, January 16, 2024 10:35 PM
> > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > Cc: dougflick@microsoft.com; Douglas Flick [MSFT]
> > <doug.edk2@gmail.com>
> > Subject: Re: Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 &
> > TCBZ4118
> >
> > On Tue, Jan 16, 2024 at 01:30:43PM +0000, Yao, Jiewen wrote:
> > > Gerd
> > > I have merged this patch set today.
> > >
> > > I am fine to remove TPM1.2 in OVMF because of the known security
> limitation.
> >
> > I was thinking about the complete edk2 code base not only OVMF.
> >
> > But I can surely start with OVMF. Maybe it is the only platform
> > affected because on physical hardware you usually know whenever TPM
> > 1.2 or TPM 2.0 is present so there is no need to include both.
> >
> > take care,
> > Gerd
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113935): https://edk2.groups.io/g/devel/message/113935
Mute This Topic: https://groups.io/mt/103675434/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
2024-01-17 8:08 ` Yao, Jiewen
@ 2024-01-17 8:15 ` Li, Yi
2024-01-17 8:23 ` Yao, Jiewen
0 siblings, 1 reply; 21+ messages in thread
From: Li, Yi @ 2024-01-17 8:15 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io, Gerd Hoffmann
Cc: dougflick@microsoft.com, Douglas Flick [MSFT]
Hi Jiewen,
Sounds strange, but new PRs in today all broken due to this issue, e.g.:
https://github.com/tianocore/edk2/pull/5210
https://github.com/tianocore/edk2/pull/5268
I checked build log, it matched the description from Gerd:
https://dev.azure.com/tianocore/11ea4a10-ac9f-4e5f-8b13-7def1f19d478/_apis/build/builds/114097/logs/350
2024-01-17T04:09:52.5996237Z INFO - /usr/bin/ld: DxeTpm2MeasureBootLibSanitization.obj (symbol from plugin): in function `SanitizeEfiPartitionTableHeader':
2024-01-17T04:09:52.6010570Z INFO - (.text+0x0): multiple definition of `SanitizeEfiPartitionTableHeader'; DxeTpmMeasureBootLibSanitization.obj (symbol from plugin):(.text+0x0): first defined here
2024-01-17T04:09:52.6020435Z INFO - /usr/bin/ld: DxeTpm2MeasureBootLibSanitization.obj (symbol from plugin): in function `SanitizeEfiPartitionTableHeader':
2024-01-17T04:09:52.6030987Z INFO - (.text+0x0): multiple definition of `SanitizePrimaryHeaderAllocationSize'; DxeTpmMeasureBootLibSanitization.obj (symbol from plugin):(.text+0x0): first defined here
2024-01-17T04:09:52.6040167Z INFO - /usr/bin/ld: DxeTpm2MeasureBootLibSanitization.obj (symbol from plugin): in function `SanitizeEfiPartitionTableHeader':
2024-01-17T04:09:52.6050625Z INFO - (.text+0x0): multiple definition of `SanitizePrimaryHeaderGptEventSize'; DxeTpmMeasureBootLibSanitization.obj (symbol from plugin):(.text+0x0): first defined here
2024-01-17T04:09:52.6061966Z INFO - /usr/bin/ld: DxeTpm2MeasureBootLibSanitization.obj (symbol from plugin): in function `SanitizeEfiPartitionTableHeader':
2024-01-17T04:09:52.6072661Z INFO - (.text+0x0): multiple definition of `SanitizePeImageEventSize'; DxeTpmMeasureBootLibSanitization.obj (symbol from plugin):(.text+0x0): first defined here
2024-01-17T04:10:12.9532147Z INFO - build.py...
2024-01-17T04:10:12.9593220Z INFO - : error 7000: Failed to execute command
2024-01-17T04:10:23.2054653Z INFO - build.py...
2024-01-17T04:10:23.2055014Z INFO - : error F002: Failed to build module
2024-01-17T04:10:23.2055379Z INFO - /__w/1/s/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf [X64, GCC5, DEBUG]
-----Original Message-----
From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Wednesday, January 17, 2024 4:09 PM
To: Li, Yi1 <yi1.li@intel.com>; devel@edk2.groups.io; Gerd Hoffmann <kraxel@redhat.com>
Cc: dougflick@microsoft.com; Douglas Flick [MSFT] <doug.edk2@gmail.com>
Subject: RE: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
Please check https://github.com/tianocore/edk2/pull/5264. It is merged after pass CI.
May I know where you see PR CI builds are broken?
Thank you
Yao, Jiewen
> -----Original Message-----
> From: Li, Yi1 <yi1.li@intel.com>
> Sent: Wednesday, January 17, 2024 3:21 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>; Gerd
> Hoffmann <kraxel@redhat.com>
> Cc: dougflick@microsoft.com; Douglas Flick [MSFT]
> <doug.edk2@gmail.com>
> Subject: RE: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 &
> TCBZ4118
>
> Hi Jiewen,
>
> All EDK2 PR CI builds of OvmfPkg are broken due to this issue.
> Maybe we didn't have enough time to wait feedback and should fix the
> CI issue first.
>
> Regards,
> Yi
>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao,
> Jiewen
> Sent: Tuesday, January 16, 2024 10:38 PM
> To: Gerd Hoffmann <kraxel@redhat.com>; devel@edk2.groups.io
> Cc: dougflick@microsoft.com; Douglas Flick [MSFT]
> <doug.edk2@gmail.com>
> Subject: Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 &
> TCBZ4118
>
> Sure. Let's start from OVMF.
>
> We have leaf enough time for feedback, but I see no comment from other people.
>
>
> > -----Original Message-----
> > From: Gerd Hoffmann <kraxel@redhat.com>
> > Sent: Tuesday, January 16, 2024 10:35 PM
> > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > Cc: dougflick@microsoft.com; Douglas Flick [MSFT]
> > <doug.edk2@gmail.com>
> > Subject: Re: Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117
> > &
> > TCBZ4118
> >
> > On Tue, Jan 16, 2024 at 01:30:43PM +0000, Yao, Jiewen wrote:
> > > Gerd
> > > I have merged this patch set today.
> > >
> > > I am fine to remove TPM1.2 in OVMF because of the known security
> limitation.
> >
> > I was thinking about the complete edk2 code base not only OVMF.
> >
> > But I can surely start with OVMF. Maybe it is the only platform
> > affected because on physical hardware you usually know whenever TPM
> > 1.2 or TPM 2.0 is present so there is no need to include both.
> >
> > take care,
> > Gerd
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113937): https://edk2.groups.io/g/devel/message/113937
Mute This Topic: https://groups.io/mt/103675434/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
2024-01-17 8:15 ` Li, Yi
@ 2024-01-17 8:23 ` Yao, Jiewen
2024-01-17 14:05 ` Gerd Hoffmann
2024-01-17 21:04 ` Doug Flick via groups.io
0 siblings, 2 replies; 21+ messages in thread
From: Yao, Jiewen @ 2024-01-17 8:23 UTC (permalink / raw)
To: Li, Yi1, devel@edk2.groups.io, Gerd Hoffmann,
dougflick@microsoft.com, Douglas Flick [MSFT]
That is weird.
It seems we need to merge Gerd's patch soon - https://github.com/tianocore/edk2/pull/5265 to unblock CI.
Hi Gerd
Would you please confirm what test you have done for removing TPM1.2?
Does TPM2.0 in OvmfPkg still work?
Hi Doug
I cannot tell why CI passed before but failed now.
But it does seems a big issue now. Would you please propose a patch to resolve it? Just rename the symbol.
Thank you
Yao, Jiewen
> -----Original Message-----
> From: Li, Yi1 <yi1.li@intel.com>
> Sent: Wednesday, January 17, 2024 4:15 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Gerd Hoffmann
> <kraxel@redhat.com>
> Cc: dougflick@microsoft.com; Douglas Flick [MSFT] <doug.edk2@gmail.com>
> Subject: RE: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
>
> Hi Jiewen,
>
> Sounds strange, but new PRs in today all broken due to this issue, e.g.:
> https://github.com/tianocore/edk2/pull/5210
> https://github.com/tianocore/edk2/pull/5268
>
>
> I checked build log, it matched the description from Gerd:
> https://dev.azure.com/tianocore/11ea4a10-ac9f-4e5f-8b13-
> 7def1f19d478/_apis/build/builds/114097/logs/350
> 2024-01-17T04:09:52.5996237Z INFO - /usr/bin/ld:
> DxeTpm2MeasureBootLibSanitization.obj (symbol from plugin): in function
> `SanitizeEfiPartitionTableHeader':
> 2024-01-17T04:09:52.6010570Z INFO - (.text+0x0): multiple definition of
> `SanitizeEfiPartitionTableHeader'; DxeTpmMeasureBootLibSanitization.obj
> (symbol from plugin):(.text+0x0): first defined here
> 2024-01-17T04:09:52.6020435Z INFO - /usr/bin/ld:
> DxeTpm2MeasureBootLibSanitization.obj (symbol from plugin): in function
> `SanitizeEfiPartitionTableHeader':
> 2024-01-17T04:09:52.6030987Z INFO - (.text+0x0): multiple definition of
> `SanitizePrimaryHeaderAllocationSize'; DxeTpmMeasureBootLibSanitization.obj
> (symbol from plugin):(.text+0x0): first defined here
> 2024-01-17T04:09:52.6040167Z INFO - /usr/bin/ld:
> DxeTpm2MeasureBootLibSanitization.obj (symbol from plugin): in function
> `SanitizeEfiPartitionTableHeader':
> 2024-01-17T04:09:52.6050625Z INFO - (.text+0x0): multiple definition of
> `SanitizePrimaryHeaderGptEventSize'; DxeTpmMeasureBootLibSanitization.obj
> (symbol from plugin):(.text+0x0): first defined here
> 2024-01-17T04:09:52.6061966Z INFO - /usr/bin/ld:
> DxeTpm2MeasureBootLibSanitization.obj (symbol from plugin): in function
> `SanitizeEfiPartitionTableHeader':
> 2024-01-17T04:09:52.6072661Z INFO - (.text+0x0): multiple definition of
> `SanitizePeImageEventSize'; DxeTpmMeasureBootLibSanitization.obj (symbol
> from plugin):(.text+0x0): first defined here
> 2024-01-17T04:10:12.9532147Z INFO - build.py...
> 2024-01-17T04:10:12.9593220Z INFO - : error 7000: Failed to execute command
> 2024-01-17T04:10:23.2054653Z INFO - build.py...
> 2024-01-17T04:10:23.2055014Z INFO - : error F002: Failed to build module
> 2024-01-17T04:10:23.2055379Z INFO -
> /__w/1/s/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.i
> nf [X64, GCC5, DEBUG]
>
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Wednesday, January 17, 2024 4:09 PM
> To: Li, Yi1 <yi1.li@intel.com>; devel@edk2.groups.io; Gerd Hoffmann
> <kraxel@redhat.com>
> Cc: dougflick@microsoft.com; Douglas Flick [MSFT] <doug.edk2@gmail.com>
> Subject: RE: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
>
> Please check https://github.com/tianocore/edk2/pull/5264. It is merged after
> pass CI.
>
> May I know where you see PR CI builds are broken?
>
> Thank you
> Yao, Jiewen
>
> > -----Original Message-----
> > From: Li, Yi1 <yi1.li@intel.com>
> > Sent: Wednesday, January 17, 2024 3:21 PM
> > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>; Gerd
> > Hoffmann <kraxel@redhat.com>
> > Cc: dougflick@microsoft.com; Douglas Flick [MSFT]
> > <doug.edk2@gmail.com>
> > Subject: RE: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 &
> > TCBZ4118
> >
> > Hi Jiewen,
> >
> > All EDK2 PR CI builds of OvmfPkg are broken due to this issue.
> > Maybe we didn't have enough time to wait feedback and should fix the
> > CI issue first.
> >
> > Regards,
> > Yi
> >
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao,
> > Jiewen
> > Sent: Tuesday, January 16, 2024 10:38 PM
> > To: Gerd Hoffmann <kraxel@redhat.com>; devel@edk2.groups.io
> > Cc: dougflick@microsoft.com; Douglas Flick [MSFT]
> > <doug.edk2@gmail.com>
> > Subject: Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 &
> > TCBZ4118
> >
> > Sure. Let's start from OVMF.
> >
> > We have leaf enough time for feedback, but I see no comment from other
> people.
> >
> >
> > > -----Original Message-----
> > > From: Gerd Hoffmann <kraxel@redhat.com>
> > > Sent: Tuesday, January 16, 2024 10:35 PM
> > > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > > Cc: dougflick@microsoft.com; Douglas Flick [MSFT]
> > > <doug.edk2@gmail.com>
> > > Subject: Re: Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117
> > > &
> > > TCBZ4118
> > >
> > > On Tue, Jan 16, 2024 at 01:30:43PM +0000, Yao, Jiewen wrote:
> > > > Gerd
> > > > I have merged this patch set today.
> > > >
> > > > I am fine to remove TPM1.2 in OVMF because of the known security
> > limitation.
> > >
> > > I was thinking about the complete edk2 code base not only OVMF.
> > >
> > > But I can surely start with OVMF. Maybe it is the only platform
> > > affected because on physical hardware you usually know whenever TPM
> > > 1.2 or TPM 2.0 is present so there is no need to include both.
> > >
> > > take care,
> > > Gerd
> >
> >
> >
> >
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113938): https://edk2.groups.io/g/devel/message/113938
Mute This Topic: https://groups.io/mt/103675434/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
2024-01-17 8:23 ` Yao, Jiewen
@ 2024-01-17 14:05 ` Gerd Hoffmann
2024-01-17 14:12 ` Yao, Jiewen
2024-01-17 21:04 ` Doug Flick via groups.io
1 sibling, 1 reply; 21+ messages in thread
From: Gerd Hoffmann @ 2024-01-17 14:05 UTC (permalink / raw)
To: devel, jiewen.yao; +Cc: Li, Yi1, dougflick@microsoft.com, Douglas Flick [MSFT]
On Wed, Jan 17, 2024 at 08:23:19AM +0000, Yao, Jiewen wrote:
> That is weird.
> It seems we need to merge Gerd's patch soon - https://github.com/tianocore/edk2/pull/5265 to unblock CI.
>
> Hi Gerd
> Would you please confirm what test you have done for removing TPM1.2?
> Does TPM2.0 in OvmfPkg still work?
For RHEL we build OVMF with TPM1_ENABLE=FALSE for quite a while without
seeing any problems, removing the TPM1_ENABLE option altogether should
give in identical results. I have to admit that I didn't actually test
that though.
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113949): https://edk2.groups.io/g/devel/message/113949
Mute This Topic: https://groups.io/mt/103675434/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
2024-01-17 14:05 ` Gerd Hoffmann
@ 2024-01-17 14:12 ` Yao, Jiewen
0 siblings, 0 replies; 21+ messages in thread
From: Yao, Jiewen @ 2024-01-17 14:12 UTC (permalink / raw)
To: Gerd Hoffmann, devel@edk2.groups.io, Marc-André Lureau
Cc: Li, Yi1, dougflick@microsoft.com, Douglas Flick [MSFT]
Hi Marc
I notice you are reviewer for TPM module in OvmfPkg.
Would you please help to test the TPM2.0 feature with patch from Gerd?
Thank you
Yao, Jiewen
> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Wednesday, January 17, 2024 10:06 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Li, Yi1 <yi1.li@intel.com>; dougflick@microsoft.com; Douglas Flick [MSFT]
> <doug.edk2@gmail.com>
> Subject: Re: Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 &
> TCBZ4118
>
> On Wed, Jan 17, 2024 at 08:23:19AM +0000, Yao, Jiewen wrote:
> > That is weird.
> > It seems we need to merge Gerd's patch soon -
> https://github.com/tianocore/edk2/pull/5265 to unblock CI.
> >
> > Hi Gerd
> > Would you please confirm what test you have done for removing TPM1.2?
> > Does TPM2.0 in OvmfPkg still work?
>
> For RHEL we build OVMF with TPM1_ENABLE=FALSE for quite a while without
> seeing any problems, removing the TPM1_ENABLE option altogether should
> give in identical results. I have to admit that I didn't actually test
> that though.
>
> take care,
> Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113952): https://edk2.groups.io/g/devel/message/113952
Mute This Topic: https://groups.io/mt/103675434/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
2024-01-17 8:23 ` Yao, Jiewen
2024-01-17 14:05 ` Gerd Hoffmann
@ 2024-01-17 21:04 ` Doug Flick via groups.io
2024-01-17 22:49 ` Doug Flick via groups.io
1 sibling, 1 reply; 21+ messages in thread
From: Doug Flick via groups.io @ 2024-01-17 21:04 UTC (permalink / raw)
To: Yao, Jiewen, devel
[-- Attachment #1: Type: text/plain, Size: 465 bytes --]
I'll propose a patch to correct this. Building against Ovmf now to confirm it corrects the issue.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113958): https://edk2.groups.io/g/devel/message/113958
Mute This Topic: https://groups.io/mt/103675434/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #2: Type: text/html, Size: 884 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118
2024-01-17 21:04 ` Doug Flick via groups.io
@ 2024-01-17 22:49 ` Doug Flick via groups.io
0 siblings, 0 replies; 21+ messages in thread
From: Doug Flick via groups.io @ 2024-01-17 22:49 UTC (permalink / raw)
To: Doug Flick, devel
[-- Attachment #1: Type: text/plain, Size: 434 bytes --]
Linking this here
https://edk2.groups.io/g/devel/message/113966
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113970): https://edk2.groups.io/g/devel/message/113970
Mute This Topic: https://groups.io/mt/103675434/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #2: Type: text/html, Size: 936 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2024-01-17 22:49 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-11 18:16 [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118 Doug Flick via groups.io
2024-01-11 18:16 ` [edk2-devel] [PATCH 1/6] SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4117 - CVE 2022-36763 Doug Flick via groups.io
2024-01-11 18:16 ` [edk2-devel] [PATCH 2/6] SecurityPkg: DxeTpmMeasureBootLib: " Doug Flick via groups.io
2024-01-11 18:16 ` [edk2-devel] [PATCH 3/6] SecurityPkg: : Adding CVE 2022-36763 to SecurityFixes.yaml Doug Flick via groups.io
2024-01-11 18:16 ` [edk2-devel] [PATCH 4/6] SecurityPkg: DxeTpm2MeasureBootLib: SECURITY PATCH 4118 - CVE 2022-36764 Doug Flick via groups.io
2024-01-11 18:16 ` [edk2-devel] [PATCH 5/6] SecurityPkg: DxeTpmMeasureBootLib: " Doug Flick via groups.io
2024-01-11 18:16 ` [edk2-devel] [PATCH 6/6] SecurityPkg: : Adding CVE 2022-36764 to SecurityFixes.yaml Doug Flick via groups.io
2024-01-12 1:50 ` [edk2-devel] [PATCH 0/6] SECURITY PATCHES TCBZ4117 & TCBZ4118 Yao, Jiewen
2024-01-16 7:59 ` Yao, Jiewen
2024-01-16 12:00 ` Gerd Hoffmann
2024-01-16 13:30 ` Yao, Jiewen
2024-01-16 14:34 ` Gerd Hoffmann
2024-01-16 14:38 ` Yao, Jiewen
2024-01-17 7:21 ` Li, Yi
2024-01-17 8:08 ` Yao, Jiewen
2024-01-17 8:15 ` Li, Yi
2024-01-17 8:23 ` Yao, Jiewen
2024-01-17 14:05 ` Gerd Hoffmann
2024-01-17 14:12 ` Yao, Jiewen
2024-01-17 21:04 ` Doug Flick via groups.io
2024-01-17 22:49 ` Doug Flick via groups.io
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox