* [PATCH] SecurityPkg HashLibRouter: Avoid incorrect PcdTcg2HashAlgorithmBitmap
@ 2017-01-24 10:06 Star Zeng
2017-01-26 2:29 ` Yao, Jiewen
2017-01-26 5:14 ` Zhang, Chao B
0 siblings, 2 replies; 3+ messages in thread
From: Star Zeng @ 2017-01-24 10:06 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng, Yao Jiewen, Chao Zhang
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=244
Currently, when software HashLib (HashLibBaseCryptoRouter) and related
HashInstanceLib instances are used, PcdTcg2HashAlgorithmBitmap is
expected to be configured to 0 in platform dsc.
But PcdTcg2HashAlgorithmBitmap has default value 0xFFFFFFFF in
SecurityPkg.dec, and some platforms forget to configure it to 0 or
still configure it to 0xFFFFFFFF in platform dsc, that will make final
PcdTcg2HashAlgorithmBitmap value incorrect.
This patch is to add CONSTRUCTOR in HashLib (HashLibBaseCryptoRouter)
and PcdTcg2HashAlgorithmBitmap will be set to 0 in the CONSTRUCTOR.
Current HASH_LIB_PEI_ROUTER_GUID HOB created in
HashLibBaseCryptoRouterPei is shared between modules that links
HashLibBaseCryptoRouterPei.
To avoid mutual interference, separated HASH_LIB_PEI_ROUTER_GUID HOBs
with gEfiCallerIdGuid Identifier will be created for those modules.
This patch is also to add check in HashLib (HashLibBaseCryptoRouter)
for the mismatch of supported HashMask between modules that may link
different HashInstanceLib instances, warning will be reported if
mismatch is found.
Cc: Yao Jiewen <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
.../HashLibBaseCryptoRouterDxe.c | 80 ++++++++-
.../HashLibBaseCryptoRouterDxe.inf | 3 +-
.../HashLibBaseCryptoRouterPei.c | 190 +++++++++++++++++----
.../HashLibBaseCryptoRouterPei.inf | 8 +-
SecurityPkg/SecurityPkg.dec | 4 +
SecurityPkg/SecurityPkg.uni | 8 +-
6 files changed, 254 insertions(+), 39 deletions(-)
diff --git a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.c b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.c
index 3250c3a01a0c..4775cfee2d7a 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.c
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.c
@@ -3,7 +3,7 @@
hash handler registerd, such as SHA1, SHA256.
Platform can use PcdTpm2HashMask to mask some hash engines.
-Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved. <BR>
+Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved. <BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -28,6 +28,30 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
HASH_INTERFACE mHashInterface[HASH_COUNT] = {{{0}, NULL, NULL, NULL}};
UINTN mHashInterfaceCount = 0;
+UINT32 mSupportedHashMaskLast = 0;
+UINT32 mSupportedHashMaskCurrent = 0;
+
+/**
+ Check mismatch of supported HashMask between modules
+ that may link different HashInstanceLib instances.
+
+**/
+VOID
+CheckSupportedHashMaskMismatch (
+ VOID
+ )
+{
+ if (mSupportedHashMaskCurrent != mSupportedHashMaskLast) {
+ DEBUG ((
+ DEBUG_WARN,
+ "WARNING: There is mismatch of supported HashMask (0x%x - 0x%x) between modules\n",
+ mSupportedHashMaskCurrent,
+ mSupportedHashMaskLast
+ ));
+ DEBUG ((DEBUG_WARN, "that are linking different HashInstanceLib instances!\n"));
+ }
+}
+
/**
Start hash sequence.
@@ -50,6 +74,8 @@ HashStart (
return EFI_UNSUPPORTED;
}
+ CheckSupportedHashMaskMismatch ();
+
HashCtx = AllocatePool (sizeof(*HashCtx) * mHashInterfaceCount);
ASSERT (HashCtx != NULL);
@@ -90,6 +116,8 @@ HashUpdate (
return EFI_UNSUPPORTED;
}
+ CheckSupportedHashMaskMismatch ();
+
HashCtx = (HASH_HANDLE *)HashHandle;
for (Index = 0; Index < mHashInterfaceCount; Index++) {
@@ -133,6 +161,8 @@ HashCompleteAndExtend (
return EFI_UNSUPPORTED;
}
+ CheckSupportedHashMaskMismatch ();
+
HashCtx = (HASH_HANDLE *)HashHandle;
ZeroMem (DigestList, sizeof(*DigestList));
@@ -180,6 +210,8 @@ HashAndExtend (
return EFI_UNSUPPORTED;
}
+ CheckSupportedHashMaskMismatch ();
+
HashStart (&HashHandle);
HashUpdate (HashHandle, DataToHash, DataToHashLen);
Status = HashCompleteAndExtend (HashHandle, PcrIndex, NULL, 0, DigestList);
@@ -204,7 +236,6 @@ RegisterHashInterfaceLib (
{
UINTN Index;
UINT32 HashMask;
- UINT32 BiosSupportedHashMask;
EFI_STATUS Status;
//
@@ -218,21 +249,58 @@ RegisterHashInterfaceLib (
if (mHashInterfaceCount >= sizeof(mHashInterface)/sizeof(mHashInterface[0])) {
return EFI_OUT_OF_RESOURCES;
}
- BiosSupportedHashMask = PcdGet32 (PcdTcg2HashAlgorithmBitmap);
- Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, BiosSupportedHashMask | HashMask);
- ASSERT_EFI_ERROR (Status);
//
// Check duplication
//
for (Index = 0; Index < mHashInterfaceCount; Index++) {
if (CompareGuid (&mHashInterface[Index].HashGuid, &HashInterface->HashGuid)) {
+ DEBUG ((DEBUG_ERROR, "Hash Interface (%g) has been registered\n"));
return EFI_ALREADY_STARTED;
}
}
+ //
+ // Record hash algorithm bitmap of CURRENT module which consumes HashLib.
+ //
+ mSupportedHashMaskCurrent = PcdGet32 (PcdTcg2HashAlgorithmBitmap) | HashMask;
+ Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, mSupportedHashMaskCurrent);
+ ASSERT_EFI_ERROR (Status);
+
CopyMem (&mHashInterface[mHashInterfaceCount], HashInterface, sizeof(*HashInterface));
mHashInterfaceCount ++;
return EFI_SUCCESS;
-}
\ No newline at end of file
+}
+
+/**
+ The constructor function of HashLibBaseCryptoRouterDxe.
+
+ @param ImageHandle The firmware allocated handle for the EFI image.
+ @param SystemTable A pointer to the EFI System Table.
+
+ @retval EFI_SUCCESS The constructor executed correctly.
+
+**/
+EFI_STATUS
+EFIAPI
+HashLibBaseCryptoRouterDxeConstructor (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ //
+ // Record hash algorithm bitmap of LAST module which also consumes HashLib.
+ //
+ mSupportedHashMaskLast = PcdGet32 (PcdTcg2HashAlgorithmBitmap);
+
+ //
+ // Set PcdTcg2HashAlgorithmBitmap to 0 in CONSTRUCTOR for CURRENT module.
+ //
+ Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, 0);
+ ASSERT_EFI_ERROR (Status);
+
+ return EFI_SUCCESS;
+}
diff --git a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.inf b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.inf
index c4e40d47d998..6e660d4f14f8 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.inf
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.inf
@@ -5,7 +5,7 @@
# hash handler registered, such as SHA1, SHA256. Platform can use PcdTpm2HashMask to
# mask some hash engines.
#
-# Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.<BR>
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
# which accompanies this distribution. The full text of the license may be found at
@@ -23,6 +23,7 @@ [Defines]
MODULE_TYPE = DXE_DRIVER
VERSION_STRING = 1.0
LIBRARY_CLASS = HashLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
+ CONSTRUCTOR = HashLibBaseCryptoRouterDxeConstructor
#
# The following information is for reference only and not required by the build tools.
diff --git a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
index a4fc0c6595d8..3ed56f337942 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
@@ -3,7 +3,7 @@
hash handler registerd, such as SHA1, SHA256.
Platform can use PcdTpm2HashMask to mask some hash engines.
-Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved. <BR>
+Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved. <BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -23,6 +23,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Library/PcdLib.h>
#include <Library/HobLib.h>
#include <Library/HashLib.h>
+#include <Guid/ZeroGuid.h>
#include "HashLibBaseCryptoRouterCommon.h"
@@ -32,27 +33,95 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
EFI_GUID mHashLibPeiRouterGuid = HASH_LIB_PEI_ROUTER_GUID;
typedef struct {
+ //
+ // If gZeroGuid, SupportedHashMask is 0 for FIRST module which consumes HashLib
+ // or the hash algorithm bitmap of LAST module which consumes HashLib.
+ // HashInterfaceCount and HashInterface are all 0.
+ // If gEfiCallerIdGuid, HashInterfaceCount, HashInterface and SupportedHashMask
+ // are the hash interface information of CURRENT module which consumes HashLib.
+ //
+ EFI_GUID Identifier;
UINTN HashInterfaceCount;
HASH_INTERFACE HashInterface[HASH_COUNT];
+ UINT32 SupportedHashMask;
} HASH_INTERFACE_HOB;
/**
- This function get hash interface.
+ This function gets hash interface hob.
+
+ @param Identifier Identifier to get hash interface hob.
+
+ @retval hash interface hob.
+**/
+HASH_INTERFACE_HOB *
+InternalGetHashInterfaceHob (
+ EFI_GUID *Identifier
+ )
+{
+ EFI_PEI_HOB_POINTERS Hob;
+ HASH_INTERFACE_HOB *HashInterfaceHob;
+
+ Hob.Raw = GetFirstGuidHob (&mHashLibPeiRouterGuid);
+ while (Hob.Raw != NULL) {
+ HashInterfaceHob = GET_GUID_HOB_DATA (Hob);
+ if (CompareGuid (&HashInterfaceHob->Identifier, Identifier)) {
+ //
+ // Found the matched one.
+ //
+ return HashInterfaceHob;
+ }
+ Hob.Raw = GET_NEXT_HOB (Hob);
+ Hob.Raw = GetNextGuidHob (&mHashLibPeiRouterGuid, Hob.Raw);
+ }
+ return NULL;
+}
+
+/**
+ This function creates hash interface hob.
- @retval hash interface.
+ @param Identifier Identifier to create hash interface hob.
+
+ @retval hash interface hob.
**/
HASH_INTERFACE_HOB *
-InternalGetHashInterface (
- VOID
+InternalCreateHashInterfaceHob (
+ EFI_GUID *Identifier
)
{
- EFI_HOB_GUID_TYPE *Hob;
+ HASH_INTERFACE_HOB LocalHashInterfaceHob;
+
+ ZeroMem (&LocalHashInterfaceHob, sizeof(LocalHashInterfaceHob));
+ CopyGuid (&LocalHashInterfaceHob.Identifier, Identifier);
+ return BuildGuidDataHob (&mHashLibPeiRouterGuid, &LocalHashInterfaceHob, sizeof(LocalHashInterfaceHob));
+}
- Hob = GetFirstGuidHob (&mHashLibPeiRouterGuid);
- if (Hob == NULL) {
- return NULL;
+/**
+ Check mismatch of supported HashMask between modules
+ that may link different HashInstanceLib instances.
+
+ @param HashInterfaceHobCurrent Pointer to hash interface hob for CURRENT module.
+
+**/
+VOID
+CheckSupportedHashMaskMismatch (
+ IN HASH_INTERFACE_HOB *HashInterfaceHobCurrent
+ )
+{
+ HASH_INTERFACE_HOB *HashInterfaceHobLast;
+
+ HashInterfaceHobLast = InternalGetHashInterfaceHob (&gZeroGuid);
+ ASSERT (HashInterfaceHobLast != NULL);
+
+ if ((HashInterfaceHobLast->SupportedHashMask != 0) &&
+ (HashInterfaceHobCurrent->SupportedHashMask != HashInterfaceHobLast->SupportedHashMask)) {
+ DEBUG ((
+ DEBUG_WARN,
+ "WARNING: There is mismatch of supported HashMask (0x%x - 0x%x) between modules\n",
+ HashInterfaceHobCurrent->SupportedHashMask,
+ HashInterfaceHobLast->SupportedHashMask
+ ));
+ DEBUG ((DEBUG_WARN, "that are linking different HashInstanceLib instances!\n"));
}
- return (HASH_INTERFACE_HOB *)(Hob + 1);
}
/**
@@ -74,7 +143,7 @@ HashStart (
UINTN Index;
UINT32 HashMask;
- HashInterfaceHob = InternalGetHashInterface ();
+ HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
if (HashInterfaceHob == NULL) {
return EFI_UNSUPPORTED;
}
@@ -83,6 +152,8 @@ HashStart (
return EFI_UNSUPPORTED;
}
+ CheckSupportedHashMaskMismatch (HashInterfaceHob);
+
HashCtx = AllocatePool (sizeof(*HashCtx) * HashInterfaceHob->HashInterfaceCount);
ASSERT (HashCtx != NULL);
@@ -120,7 +191,7 @@ HashUpdate (
UINTN Index;
UINT32 HashMask;
- HashInterfaceHob = InternalGetHashInterface ();
+ HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
if (HashInterfaceHob == NULL) {
return EFI_UNSUPPORTED;
}
@@ -129,6 +200,8 @@ HashUpdate (
return EFI_UNSUPPORTED;
}
+ CheckSupportedHashMaskMismatch (HashInterfaceHob);
+
HashCtx = (HASH_HANDLE *)HashHandle;
for (Index = 0; Index < HashInterfaceHob->HashInterfaceCount; Index++) {
@@ -169,7 +242,7 @@ HashCompleteAndExtend (
EFI_STATUS Status;
UINT32 HashMask;
- HashInterfaceHob = InternalGetHashInterface ();
+ HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
if (HashInterfaceHob == NULL) {
return EFI_UNSUPPORTED;
}
@@ -178,6 +251,8 @@ HashCompleteAndExtend (
return EFI_UNSUPPORTED;
}
+ CheckSupportedHashMaskMismatch (HashInterfaceHob);
+
HashCtx = (HASH_HANDLE *)HashHandle;
ZeroMem (DigestList, sizeof(*DigestList));
@@ -222,7 +297,7 @@ HashAndExtend (
HASH_HANDLE HashHandle;
EFI_STATUS Status;
- HashInterfaceHob = InternalGetHashInterface ();
+ HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
if (HashInterfaceHob == NULL) {
return EFI_UNSUPPORTED;
}
@@ -231,6 +306,8 @@ HashAndExtend (
return EFI_UNSUPPORTED;
}
+ CheckSupportedHashMaskMismatch (HashInterfaceHob);
+
HashStart (&HashHandle);
HashUpdate (HashHandle, DataToHash, DataToHashLen);
Status = HashCompleteAndExtend (HashHandle, PcrIndex, NULL, 0, DigestList);
@@ -255,9 +332,7 @@ RegisterHashInterfaceLib (
{
UINTN Index;
HASH_INTERFACE_HOB *HashInterfaceHob;
- HASH_INTERFACE_HOB LocalHashInterfaceHob;
UINT32 HashMask;
- UINT32 BiosSupportedHashMask;
EFI_STATUS Status;
//
@@ -268,10 +343,9 @@ RegisterHashInterfaceLib (
return EFI_UNSUPPORTED;
}
- HashInterfaceHob = InternalGetHashInterface ();
+ HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
if (HashInterfaceHob == NULL) {
- ZeroMem (&LocalHashInterfaceHob, sizeof(LocalHashInterfaceHob));
- HashInterfaceHob = BuildGuidDataHob (&mHashLibPeiRouterGuid, &LocalHashInterfaceHob, sizeof(LocalHashInterfaceHob));
+ HashInterfaceHob = InternalCreateHashInterfaceHob (&gEfiCallerIdGuid);
if (HashInterfaceHob == NULL) {
return EFI_OUT_OF_RESOURCES;
}
@@ -280,26 +354,84 @@ RegisterHashInterfaceLib (
if (HashInterfaceHob->HashInterfaceCount >= HASH_COUNT) {
return EFI_OUT_OF_RESOURCES;
}
- BiosSupportedHashMask = PcdGet32 (PcdTcg2HashAlgorithmBitmap);
- Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, BiosSupportedHashMask | HashMask);
- ASSERT_EFI_ERROR (Status);
//
// Check duplication
//
for (Index = 0; Index < HashInterfaceHob->HashInterfaceCount; Index++) {
if (CompareGuid (&HashInterfaceHob->HashInterface[Index].HashGuid, &HashInterface->HashGuid)) {
- //
- // In PEI phase, there will be shadow driver dispatched again.
- //
- DEBUG ((EFI_D_INFO, "RegisterHashInterfaceLib - Override\n"));
- CopyMem (&HashInterfaceHob->HashInterface[Index], HashInterface, sizeof(*HashInterface));
- return EFI_SUCCESS;
+ DEBUG ((DEBUG_ERROR, "Hash Interface (%g) has been registered\n"));
+ return EFI_ALREADY_STARTED;
}
}
+ //
+ // Record hash algorithm bitmap of CURRENT module which consumes HashLib.
+ //
+ HashInterfaceHob->SupportedHashMask = PcdGet32 (PcdTcg2HashAlgorithmBitmap) | HashMask;
+ Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, HashInterfaceHob->SupportedHashMask);
+ ASSERT_EFI_ERROR (Status);
+
CopyMem (&HashInterfaceHob->HashInterface[HashInterfaceHob->HashInterfaceCount], HashInterface, sizeof(*HashInterface));
HashInterfaceHob->HashInterfaceCount ++;
return EFI_SUCCESS;
-}
\ No newline at end of file
+}
+
+/**
+ The constructor function of HashLibBaseCryptoRouterPei.
+
+ @param FileHandle The handle of FFS header the loaded driver.
+ @param PeiServices The pointer to the PEI services.
+
+ @retval EFI_SUCCESS The constructor executes successfully.
+ @retval EFI_OUT_OF_RESOURCES There is no enough resource for the constructor.
+
+**/
+EFI_STATUS
+EFIAPI
+HashLibBaseCryptoRouterPeiConstructor (
+ IN EFI_PEI_FILE_HANDLE FileHandle,
+ IN CONST EFI_PEI_SERVICES **PeiServices
+ )
+{
+ EFI_STATUS Status;
+ HASH_INTERFACE_HOB *HashInterfaceHob;
+
+ HashInterfaceHob = InternalGetHashInterfaceHob (&gZeroGuid);
+ if (HashInterfaceHob == NULL) {
+ //
+ // No HOB with gZeroGuid Identifier has been created,
+ // this is FIRST module which consumes HashLib.
+ // Create the HOB with gZeroGuid Identifier.
+ //
+ HashInterfaceHob = InternalCreateHashInterfaceHob (&gZeroGuid);
+ if (HashInterfaceHob == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+ } else {
+ //
+ // Record hash algorithm bitmap of LAST module which also consumes HashLib.
+ //
+ HashInterfaceHob->SupportedHashMask = PcdGet32 (PcdTcg2HashAlgorithmBitmap);
+ }
+
+ HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
+ if (HashInterfaceHob != NULL) {
+ //
+ // In PEI phase, some modules may call RegisterForShadow and will be
+ // shadowed and executed again after memory is discovered.
+ // This is the second execution of this module, clear the hash interface
+ // information registered at its first execution.
+ //
+ ZeroMem (&HashInterfaceHob->HashInterface, sizeof (*HashInterfaceHob) - sizeof (EFI_GUID));
+ }
+
+ //
+ // Set PcdTcg2HashAlgorithmBitmap to 0 in CONSTRUCTOR for CURRENT module.
+ //
+ Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, 0);
+ ASSERT_EFI_ERROR (Status);
+
+ return EFI_SUCCESS;
+}
diff --git a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf
index f5ca5d4635d8..eebf90e2ef40 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf
@@ -5,7 +5,7 @@
# hash handler registered, such as SHA1, SHA256. Platform can use PcdTpm2HashMask to
# mask some hash engines.
#
-# Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.<BR>
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
# which accompanies this distribution. The full text of the license may be found at
@@ -23,6 +23,7 @@ [Defines]
MODULE_TYPE = PEIM
VERSION_STRING = 1.0
LIBRARY_CLASS = HashLib|PEIM
+ CONSTRUCTOR = HashLibBaseCryptoRouterPeiConstructor
#
# The following information is for reference only and not required by the build tools.
@@ -38,6 +39,7 @@ [Sources]
[Packages]
MdePkg/MdePkg.dec
SecurityPkg/SecurityPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
[LibraryClasses]
BaseLib
@@ -48,6 +50,10 @@ [LibraryClasses]
PcdLib
HobLib
+[Guids]
+ ## CONSUMES ## GUID
+ gZeroGuid
+
[Pcd]
gEfiSecurityPkgTokenSpaceGuid.PcdTpm2HashMask ## CONSUMES
## SOMETIMES_CONSUMES
diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
index 0c64d25b0cbc..88154611ae9c 100644
--- a/SecurityPkg/SecurityPkg.dec
+++ b/SecurityPkg/SecurityPkg.dec
@@ -458,6 +458,10 @@ [PcdsDynamic, PcdsDynamicEx]
## This PCD indicated final BIOS supported Hash mask.
# Bios may choose to register a subset of PcdTpm2HashMask.
# So this PCD is final value of how many hash algo is extended to PCR.
+ # If software HashLib(HashLibBaseCryptoRouter) solution is chosen, this PCD
+ # has no need to be configured in platform dsc and will be set to correct
+ # value by the HashLib instance according to the HashInstanceLib instances
+ # linked, and the value of this PCD should be got in module entrypoint.
# @Prompt Hash Algorithm bitmap.
gEfiSecurityPkgTokenSpaceGuid.PcdTcg2HashAlgorithmBitmap|0xFFFFFFFF|UINT32|0x00010016
diff --git a/SecurityPkg/SecurityPkg.uni b/SecurityPkg/SecurityPkg.uni
index 17d36c0d6014..126351624056 100644
--- a/SecurityPkg/SecurityPkg.uni
+++ b/SecurityPkg/SecurityPkg.uni
@@ -191,7 +191,11 @@
#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTcg2HashAlgorithmBitmap_HELP #language en-US "This PCD indicated final BIOS supported Hash mask.\n"
"Bios may choose to register a subset of PcdTpm2HashMask.\n"
- "So this PCD is final value of how many hash algo is extended to PCR."
+ "So this PCD is final value of how many hash algo is extended to PCR.\n"
+ "If software HashLib(HashLibBaseCryptoRouter) solution is chosen, this PCD\n"
+ "has no need to be configured in platform dsc and will be set to correct\n"
+ "value by the HashLib instance according to the HashInstanceLib instances\n"
+ "linked, and the value of this PCD should be got in module entrypoint."
#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTcgLogAreaMinLen_PROMPT #language en-US "Minimum length(in bytes) of the system preboot TCG event log area(LAML)."
@@ -233,4 +237,4 @@
#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTpm2AcpiTableRev_HELP #language en-US "This PCD defines initial revision of TPM2 ACPI table\n"
"To support configuring from setup page, this PCD can be DynamicHii type and map to a setup option.<BR>\n"
"For example, map to TCG2_VERSION.Tpm2AcpiTableRev to be configured by Tcg2ConfigDxe driver.<BR>\n"
- "gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L\"TCG2_VERSION\"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS<BR>"
\ No newline at end of file
+ "gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L\"TCG2_VERSION\"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS<BR>"
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] SecurityPkg HashLibRouter: Avoid incorrect PcdTcg2HashAlgorithmBitmap
2017-01-24 10:06 [PATCH] SecurityPkg HashLibRouter: Avoid incorrect PcdTcg2HashAlgorithmBitmap Star Zeng
@ 2017-01-26 2:29 ` Yao, Jiewen
2017-01-26 5:14 ` Zhang, Chao B
1 sibling, 0 replies; 3+ messages in thread
From: Yao, Jiewen @ 2017-01-26 2:29 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Zhang, Chao B
Good feature to catch mis-configuration.
Reviewed-by: Jiewen.yao@intel.com
> -----Original Message-----
> From: Zeng, Star
> Sent: Tuesday, January 24, 2017 6:07 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Zhang, Chao B <chao.b.zhang@intel.com>
> Subject: [PATCH] SecurityPkg HashLibRouter: Avoid incorrect
> PcdTcg2HashAlgorithmBitmap
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=244
>
> Currently, when software HashLib (HashLibBaseCryptoRouter) and related
> HashInstanceLib instances are used, PcdTcg2HashAlgorithmBitmap is
> expected to be configured to 0 in platform dsc.
> But PcdTcg2HashAlgorithmBitmap has default value 0xFFFFFFFF in
> SecurityPkg.dec, and some platforms forget to configure it to 0 or
> still configure it to 0xFFFFFFFF in platform dsc, that will make final
> PcdTcg2HashAlgorithmBitmap value incorrect.
>
> This patch is to add CONSTRUCTOR in HashLib (HashLibBaseCryptoRouter)
> and PcdTcg2HashAlgorithmBitmap will be set to 0 in the CONSTRUCTOR.
>
> Current HASH_LIB_PEI_ROUTER_GUID HOB created in
> HashLibBaseCryptoRouterPei is shared between modules that links
> HashLibBaseCryptoRouterPei.
> To avoid mutual interference, separated HASH_LIB_PEI_ROUTER_GUID HOBs
> with gEfiCallerIdGuid Identifier will be created for those modules.
>
> This patch is also to add check in HashLib (HashLibBaseCryptoRouter)
> for the mismatch of supported HashMask between modules that may link
> different HashInstanceLib instances, warning will be reported if
> mismatch is found.
>
> Cc: Yao Jiewen <jiewen.yao@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> .../HashLibBaseCryptoRouterDxe.c | 80 ++++++++-
> .../HashLibBaseCryptoRouterDxe.inf | 3 +-
> .../HashLibBaseCryptoRouterPei.c | 190
> +++++++++++++++++----
> .../HashLibBaseCryptoRouterPei.inf | 8 +-
> SecurityPkg/SecurityPkg.dec | 4 +
> SecurityPkg/SecurityPkg.uni | 8 +-
> 6 files changed, 254 insertions(+), 39 deletions(-)
>
> diff --git
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.
> c
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.
> c
> index 3250c3a01a0c..4775cfee2d7a 100644
> ---
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.
> c
> +++
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.
> c
> @@ -3,7 +3,7 @@
> hash handler registerd, such as SHA1, SHA256.
> Platform can use PcdTpm2HashMask to mask some hash engines.
>
> -Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved. <BR>
> +Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved. <BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> which accompanies this distribution. The full text of the license may be found
> at
> @@ -28,6 +28,30 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
> HASH_INTERFACE mHashInterface[HASH_COUNT] = {{{0}, NULL, NULL,
> NULL}};
> UINTN mHashInterfaceCount = 0;
>
> +UINT32 mSupportedHashMaskLast = 0;
> +UINT32 mSupportedHashMaskCurrent = 0;
> +
> +/**
> + Check mismatch of supported HashMask between modules
> + that may link different HashInstanceLib instances.
> +
> +**/
> +VOID
> +CheckSupportedHashMaskMismatch (
> + VOID
> + )
> +{
> + if (mSupportedHashMaskCurrent != mSupportedHashMaskLast) {
> + DEBUG ((
> + DEBUG_WARN,
> + "WARNING: There is mismatch of supported HashMask (0x%x - 0x%x)
> between modules\n",
> + mSupportedHashMaskCurrent,
> + mSupportedHashMaskLast
> + ));
> + DEBUG ((DEBUG_WARN, "that are linking different HashInstanceLib
> instances!\n"));
> + }
> +}
> +
> /**
> Start hash sequence.
>
> @@ -50,6 +74,8 @@ HashStart (
> return EFI_UNSUPPORTED;
> }
>
> + CheckSupportedHashMaskMismatch ();
> +
> HashCtx = AllocatePool (sizeof(*HashCtx) * mHashInterfaceCount);
> ASSERT (HashCtx != NULL);
>
> @@ -90,6 +116,8 @@ HashUpdate (
> return EFI_UNSUPPORTED;
> }
>
> + CheckSupportedHashMaskMismatch ();
> +
> HashCtx = (HASH_HANDLE *)HashHandle;
>
> for (Index = 0; Index < mHashInterfaceCount; Index++) {
> @@ -133,6 +161,8 @@ HashCompleteAndExtend (
> return EFI_UNSUPPORTED;
> }
>
> + CheckSupportedHashMaskMismatch ();
> +
> HashCtx = (HASH_HANDLE *)HashHandle;
> ZeroMem (DigestList, sizeof(*DigestList));
>
> @@ -180,6 +210,8 @@ HashAndExtend (
> return EFI_UNSUPPORTED;
> }
>
> + CheckSupportedHashMaskMismatch ();
> +
> HashStart (&HashHandle);
> HashUpdate (HashHandle, DataToHash, DataToHashLen);
> Status = HashCompleteAndExtend (HashHandle, PcrIndex, NULL, 0,
> DigestList);
> @@ -204,7 +236,6 @@ RegisterHashInterfaceLib (
> {
> UINTN Index;
> UINT32 HashMask;
> - UINT32 BiosSupportedHashMask;
> EFI_STATUS Status;
>
> //
> @@ -218,21 +249,58 @@ RegisterHashInterfaceLib (
> if (mHashInterfaceCount >=
> sizeof(mHashInterface)/sizeof(mHashInterface[0])) {
> return EFI_OUT_OF_RESOURCES;
> }
> - BiosSupportedHashMask = PcdGet32 (PcdTcg2HashAlgorithmBitmap);
> - Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, BiosSupportedHashMask |
> HashMask);
> - ASSERT_EFI_ERROR (Status);
>
> //
> // Check duplication
> //
> for (Index = 0; Index < mHashInterfaceCount; Index++) {
> if (CompareGuid (&mHashInterface[Index].HashGuid,
> &HashInterface->HashGuid)) {
> + DEBUG ((DEBUG_ERROR, "Hash Interface (%g) has been registered\n"));
> return EFI_ALREADY_STARTED;
> }
> }
>
> + //
> + // Record hash algorithm bitmap of CURRENT module which consumes
> HashLib.
> + //
> + mSupportedHashMaskCurrent = PcdGet32 (PcdTcg2HashAlgorithmBitmap) |
> HashMask;
> + Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap,
> mSupportedHashMaskCurrent);
> + ASSERT_EFI_ERROR (Status);
> +
> CopyMem (&mHashInterface[mHashInterfaceCount], HashInterface,
> sizeof(*HashInterface));
> mHashInterfaceCount ++;
>
> return EFI_SUCCESS;
> -}
> \ No newline at end of file
> +}
> +
> +/**
> + The constructor function of HashLibBaseCryptoRouterDxe.
> +
> + @param ImageHandle The firmware allocated handle for the EFI image.
> + @param SystemTable A pointer to the EFI System Table.
> +
> + @retval EFI_SUCCESS The constructor executed correctly.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +HashLibBaseCryptoRouterDxeConstructor (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + EFI_STATUS Status;
> +
> + //
> + // Record hash algorithm bitmap of LAST module which also consumes
> HashLib.
> + //
> + mSupportedHashMaskLast = PcdGet32 (PcdTcg2HashAlgorithmBitmap);
> +
> + //
> + // Set PcdTcg2HashAlgorithmBitmap to 0 in CONSTRUCTOR for CURRENT
> module.
> + //
> + Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, 0);
> + ASSERT_EFI_ERROR (Status);
> +
> + return EFI_SUCCESS;
> +}
> diff --git
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.i
> nf
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.i
> nf
> index c4e40d47d998..6e660d4f14f8 100644
> ---
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.i
> nf
> +++
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.i
> nf
> @@ -5,7 +5,7 @@
> # hash handler registered, such as SHA1, SHA256. Platform can use
> PcdTpm2HashMask to
> # mask some hash engines.
> #
> -# Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.<BR>
> # This program and the accompanying materials
> # are licensed and made available under the terms and conditions of the BSD
> License
> # which accompanies this distribution. The full text of the license may be found
> at
> @@ -23,6 +23,7 @@ [Defines]
> MODULE_TYPE = DXE_DRIVER
> VERSION_STRING = 1.0
> LIBRARY_CLASS = HashLib|DXE_DRIVER
> DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER
> UEFI_APPLICATION UEFI_DRIVER
> + CONSTRUCTOR =
> HashLibBaseCryptoRouterDxeConstructor
>
> #
> # The following information is for reference only and not required by the build
> tools.
> diff --git
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> index a4fc0c6595d8..3ed56f337942 100644
> ---
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> +++
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> @@ -3,7 +3,7 @@
> hash handler registerd, such as SHA1, SHA256.
> Platform can use PcdTpm2HashMask to mask some hash engines.
>
> -Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved. <BR>
> +Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved. <BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> which accompanies this distribution. The full text of the license may be found
> at
> @@ -23,6 +23,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
> #include <Library/PcdLib.h>
> #include <Library/HobLib.h>
> #include <Library/HashLib.h>
> +#include <Guid/ZeroGuid.h>
>
> #include "HashLibBaseCryptoRouterCommon.h"
>
> @@ -32,27 +33,95 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
> EFI_GUID mHashLibPeiRouterGuid = HASH_LIB_PEI_ROUTER_GUID;
>
> typedef struct {
> + //
> + // If gZeroGuid, SupportedHashMask is 0 for FIRST module which consumes
> HashLib
> + // or the hash algorithm bitmap of LAST module which consumes HashLib.
> + // HashInterfaceCount and HashInterface are all 0.
> + // If gEfiCallerIdGuid, HashInterfaceCount, HashInterface and
> SupportedHashMask
> + // are the hash interface information of CURRENT module which consumes
> HashLib.
> + //
> + EFI_GUID Identifier;
> UINTN HashInterfaceCount;
> HASH_INTERFACE HashInterface[HASH_COUNT];
> + UINT32 SupportedHashMask;
> } HASH_INTERFACE_HOB;
>
> /**
> - This function get hash interface.
> + This function gets hash interface hob.
> +
> + @param Identifier Identifier to get hash interface hob.
> +
> + @retval hash interface hob.
> +**/
> +HASH_INTERFACE_HOB *
> +InternalGetHashInterfaceHob (
> + EFI_GUID *Identifier
> + )
> +{
> + EFI_PEI_HOB_POINTERS Hob;
> + HASH_INTERFACE_HOB *HashInterfaceHob;
> +
> + Hob.Raw = GetFirstGuidHob (&mHashLibPeiRouterGuid);
> + while (Hob.Raw != NULL) {
> + HashInterfaceHob = GET_GUID_HOB_DATA (Hob);
> + if (CompareGuid (&HashInterfaceHob->Identifier, Identifier)) {
> + //
> + // Found the matched one.
> + //
> + return HashInterfaceHob;
> + }
> + Hob.Raw = GET_NEXT_HOB (Hob);
> + Hob.Raw = GetNextGuidHob (&mHashLibPeiRouterGuid, Hob.Raw);
> + }
> + return NULL;
> +}
> +
> +/**
> + This function creates hash interface hob.
>
> - @retval hash interface.
> + @param Identifier Identifier to create hash interface hob.
> +
> + @retval hash interface hob.
> **/
> HASH_INTERFACE_HOB *
> -InternalGetHashInterface (
> - VOID
> +InternalCreateHashInterfaceHob (
> + EFI_GUID *Identifier
> )
> {
> - EFI_HOB_GUID_TYPE *Hob;
> + HASH_INTERFACE_HOB LocalHashInterfaceHob;
> +
> + ZeroMem (&LocalHashInterfaceHob, sizeof(LocalHashInterfaceHob));
> + CopyGuid (&LocalHashInterfaceHob.Identifier, Identifier);
> + return BuildGuidDataHob (&mHashLibPeiRouterGuid,
> &LocalHashInterfaceHob, sizeof(LocalHashInterfaceHob));
> +}
>
> - Hob = GetFirstGuidHob (&mHashLibPeiRouterGuid);
> - if (Hob == NULL) {
> - return NULL;
> +/**
> + Check mismatch of supported HashMask between modules
> + that may link different HashInstanceLib instances.
> +
> + @param HashInterfaceHobCurrent Pointer to hash interface hob for
> CURRENT module.
> +
> +**/
> +VOID
> +CheckSupportedHashMaskMismatch (
> + IN HASH_INTERFACE_HOB *HashInterfaceHobCurrent
> + )
> +{
> + HASH_INTERFACE_HOB *HashInterfaceHobLast;
> +
> + HashInterfaceHobLast = InternalGetHashInterfaceHob (&gZeroGuid);
> + ASSERT (HashInterfaceHobLast != NULL);
> +
> + if ((HashInterfaceHobLast->SupportedHashMask != 0) &&
> + (HashInterfaceHobCurrent->SupportedHashMask !=
> HashInterfaceHobLast->SupportedHashMask)) {
> + DEBUG ((
> + DEBUG_WARN,
> + "WARNING: There is mismatch of supported HashMask (0x%x - 0x%x)
> between modules\n",
> + HashInterfaceHobCurrent->SupportedHashMask,
> + HashInterfaceHobLast->SupportedHashMask
> + ));
> + DEBUG ((DEBUG_WARN, "that are linking different HashInstanceLib
> instances!\n"));
> }
> - return (HASH_INTERFACE_HOB *)(Hob + 1);
> }
>
> /**
> @@ -74,7 +143,7 @@ HashStart (
> UINTN Index;
> UINT32 HashMask;
>
> - HashInterfaceHob = InternalGetHashInterface ();
> + HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
> if (HashInterfaceHob == NULL) {
> return EFI_UNSUPPORTED;
> }
> @@ -83,6 +152,8 @@ HashStart (
> return EFI_UNSUPPORTED;
> }
>
> + CheckSupportedHashMaskMismatch (HashInterfaceHob);
> +
> HashCtx = AllocatePool (sizeof(*HashCtx) *
> HashInterfaceHob->HashInterfaceCount);
> ASSERT (HashCtx != NULL);
>
> @@ -120,7 +191,7 @@ HashUpdate (
> UINTN Index;
> UINT32 HashMask;
>
> - HashInterfaceHob = InternalGetHashInterface ();
> + HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
> if (HashInterfaceHob == NULL) {
> return EFI_UNSUPPORTED;
> }
> @@ -129,6 +200,8 @@ HashUpdate (
> return EFI_UNSUPPORTED;
> }
>
> + CheckSupportedHashMaskMismatch (HashInterfaceHob);
> +
> HashCtx = (HASH_HANDLE *)HashHandle;
>
> for (Index = 0; Index < HashInterfaceHob->HashInterfaceCount; Index++) {
> @@ -169,7 +242,7 @@ HashCompleteAndExtend (
> EFI_STATUS Status;
> UINT32 HashMask;
>
> - HashInterfaceHob = InternalGetHashInterface ();
> + HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
> if (HashInterfaceHob == NULL) {
> return EFI_UNSUPPORTED;
> }
> @@ -178,6 +251,8 @@ HashCompleteAndExtend (
> return EFI_UNSUPPORTED;
> }
>
> + CheckSupportedHashMaskMismatch (HashInterfaceHob);
> +
> HashCtx = (HASH_HANDLE *)HashHandle;
> ZeroMem (DigestList, sizeof(*DigestList));
>
> @@ -222,7 +297,7 @@ HashAndExtend (
> HASH_HANDLE HashHandle;
> EFI_STATUS Status;
>
> - HashInterfaceHob = InternalGetHashInterface ();
> + HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
> if (HashInterfaceHob == NULL) {
> return EFI_UNSUPPORTED;
> }
> @@ -231,6 +306,8 @@ HashAndExtend (
> return EFI_UNSUPPORTED;
> }
>
> + CheckSupportedHashMaskMismatch (HashInterfaceHob);
> +
> HashStart (&HashHandle);
> HashUpdate (HashHandle, DataToHash, DataToHashLen);
> Status = HashCompleteAndExtend (HashHandle, PcrIndex, NULL, 0,
> DigestList);
> @@ -255,9 +332,7 @@ RegisterHashInterfaceLib (
> {
> UINTN Index;
> HASH_INTERFACE_HOB *HashInterfaceHob;
> - HASH_INTERFACE_HOB LocalHashInterfaceHob;
> UINT32 HashMask;
> - UINT32 BiosSupportedHashMask;
> EFI_STATUS Status;
>
> //
> @@ -268,10 +343,9 @@ RegisterHashInterfaceLib (
> return EFI_UNSUPPORTED;
> }
>
> - HashInterfaceHob = InternalGetHashInterface ();
> + HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
> if (HashInterfaceHob == NULL) {
> - ZeroMem (&LocalHashInterfaceHob, sizeof(LocalHashInterfaceHob));
> - HashInterfaceHob = BuildGuidDataHob (&mHashLibPeiRouterGuid,
> &LocalHashInterfaceHob, sizeof(LocalHashInterfaceHob));
> + HashInterfaceHob = InternalCreateHashInterfaceHob (&gEfiCallerIdGuid);
> if (HashInterfaceHob == NULL) {
> return EFI_OUT_OF_RESOURCES;
> }
> @@ -280,26 +354,84 @@ RegisterHashInterfaceLib (
> if (HashInterfaceHob->HashInterfaceCount >= HASH_COUNT) {
> return EFI_OUT_OF_RESOURCES;
> }
> - BiosSupportedHashMask = PcdGet32 (PcdTcg2HashAlgorithmBitmap);
> - Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, BiosSupportedHashMask |
> HashMask);
> - ASSERT_EFI_ERROR (Status);
>
> //
> // Check duplication
> //
> for (Index = 0; Index < HashInterfaceHob->HashInterfaceCount; Index++) {
> if (CompareGuid (&HashInterfaceHob->HashInterface[Index].HashGuid,
> &HashInterface->HashGuid)) {
> - //
> - // In PEI phase, there will be shadow driver dispatched again.
> - //
> - DEBUG ((EFI_D_INFO, "RegisterHashInterfaceLib - Override\n"));
> - CopyMem (&HashInterfaceHob->HashInterface[Index], HashInterface,
> sizeof(*HashInterface));
> - return EFI_SUCCESS;
> + DEBUG ((DEBUG_ERROR, "Hash Interface (%g) has been registered\n"));
> + return EFI_ALREADY_STARTED;
> }
> }
>
> + //
> + // Record hash algorithm bitmap of CURRENT module which consumes
> HashLib.
> + //
> + HashInterfaceHob->SupportedHashMask = PcdGet32
> (PcdTcg2HashAlgorithmBitmap) | HashMask;
> + Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap,
> HashInterfaceHob->SupportedHashMask);
> + ASSERT_EFI_ERROR (Status);
> +
> CopyMem
> (&HashInterfaceHob->HashInterface[HashInterfaceHob->HashInterfaceCount],
> HashInterface, sizeof(*HashInterface));
> HashInterfaceHob->HashInterfaceCount ++;
>
> return EFI_SUCCESS;
> -}
> \ No newline at end of file
> +}
> +
> +/**
> + The constructor function of HashLibBaseCryptoRouterPei.
> +
> + @param FileHandle The handle of FFS header the loaded driver.
> + @param PeiServices The pointer to the PEI services.
> +
> + @retval EFI_SUCCESS The constructor executes successfully.
> + @retval EFI_OUT_OF_RESOURCES There is no enough resource for the
> constructor.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +HashLibBaseCryptoRouterPeiConstructor (
> + IN EFI_PEI_FILE_HANDLE FileHandle,
> + IN CONST EFI_PEI_SERVICES **PeiServices
> + )
> +{
> + EFI_STATUS Status;
> + HASH_INTERFACE_HOB *HashInterfaceHob;
> +
> + HashInterfaceHob = InternalGetHashInterfaceHob (&gZeroGuid);
> + if (HashInterfaceHob == NULL) {
> + //
> + // No HOB with gZeroGuid Identifier has been created,
> + // this is FIRST module which consumes HashLib.
> + // Create the HOB with gZeroGuid Identifier.
> + //
> + HashInterfaceHob = InternalCreateHashInterfaceHob (&gZeroGuid);
> + if (HashInterfaceHob == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> + } else {
> + //
> + // Record hash algorithm bitmap of LAST module which also consumes
> HashLib.
> + //
> + HashInterfaceHob->SupportedHashMask = PcdGet32
> (PcdTcg2HashAlgorithmBitmap);
> + }
> +
> + HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
> + if (HashInterfaceHob != NULL) {
> + //
> + // In PEI phase, some modules may call RegisterForShadow and will be
> + // shadowed and executed again after memory is discovered.
> + // This is the second execution of this module, clear the hash interface
> + // information registered at its first execution.
> + //
> + ZeroMem (&HashInterfaceHob->HashInterface, sizeof (*HashInterfaceHob)
> - sizeof (EFI_GUID));
> + }
> +
> + //
> + // Set PcdTcg2HashAlgorithmBitmap to 0 in CONSTRUCTOR for CURRENT
> module.
> + //
> + Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, 0);
> + ASSERT_EFI_ERROR (Status);
> +
> + return EFI_SUCCESS;
> +}
> diff --git
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.i
> nf
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.i
> nf
> index f5ca5d4635d8..eebf90e2ef40 100644
> ---
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.i
> nf
> +++
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.i
> nf
> @@ -5,7 +5,7 @@
> # hash handler registered, such as SHA1, SHA256. Platform can use
> PcdTpm2HashMask to
> # mask some hash engines.
> #
> -# Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.<BR>
> # This program and the accompanying materials
> # are licensed and made available under the terms and conditions of the BSD
> License
> # which accompanies this distribution. The full text of the license may be found
> at
> @@ -23,6 +23,7 @@ [Defines]
> MODULE_TYPE = PEIM
> VERSION_STRING = 1.0
> LIBRARY_CLASS = HashLib|PEIM
> + CONSTRUCTOR =
> HashLibBaseCryptoRouterPeiConstructor
>
> #
> # The following information is for reference only and not required by the build
> tools.
> @@ -38,6 +39,7 @@ [Sources]
> [Packages]
> MdePkg/MdePkg.dec
> SecurityPkg/SecurityPkg.dec
> + MdeModulePkg/MdeModulePkg.dec
>
> [LibraryClasses]
> BaseLib
> @@ -48,6 +50,10 @@ [LibraryClasses]
> PcdLib
> HobLib
>
> +[Guids]
> + ## CONSUMES ## GUID
> + gZeroGuid
> +
> [Pcd]
> gEfiSecurityPkgTokenSpaceGuid.PcdTpm2HashMask ##
> CONSUMES
> ## SOMETIMES_CONSUMES
> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> index 0c64d25b0cbc..88154611ae9c 100644
> --- a/SecurityPkg/SecurityPkg.dec
> +++ b/SecurityPkg/SecurityPkg.dec
> @@ -458,6 +458,10 @@ [PcdsDynamic, PcdsDynamicEx]
> ## This PCD indicated final BIOS supported Hash mask.
> # Bios may choose to register a subset of PcdTpm2HashMask.
> # So this PCD is final value of how many hash algo is extended to PCR.
> + # If software HashLib(HashLibBaseCryptoRouter) solution is chosen, this PCD
> + # has no need to be configured in platform dsc and will be set to correct
> + # value by the HashLib instance according to the HashInstanceLib instances
> + # linked, and the value of this PCD should be got in module entrypoint.
> # @Prompt Hash Algorithm bitmap.
>
> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2HashAlgorithmBitmap|0xFFFFFFFF|UINT
> 32|0x00010016
>
> diff --git a/SecurityPkg/SecurityPkg.uni b/SecurityPkg/SecurityPkg.uni
> index 17d36c0d6014..126351624056 100644
> --- a/SecurityPkg/SecurityPkg.uni
> +++ b/SecurityPkg/SecurityPkg.uni
> @@ -191,7 +191,11 @@
>
> #string
> STR_gEfiSecurityPkgTokenSpaceGuid_PcdTcg2HashAlgorithmBitmap_HELP
> #language en-US "This PCD indicated final BIOS supported Hash mask.\n"
>
> "Bios may choose to register a subset of PcdTpm2HashMask.\n"
> -
> "So this PCD is final value of how many hash algo is extended to PCR."
> +
> "So this PCD is final value of how many hash algo is extended to PCR.\n"
> +
> "If software HashLib(HashLibBaseCryptoRouter) solution is chosen, this PCD\n"
> +
> "has no need to be configured in platform dsc and will be set to correct\n"
> +
> "value by the HashLib instance according to the HashInstanceLib instances\n"
> +
> "linked, and the value of this PCD should be got in module entrypoint."
>
> #string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTcgLogAreaMinLen_PROMPT
> #language en-US "Minimum length(in bytes) of the system preboot TCG event
> log area(LAML)."
>
> @@ -233,4 +237,4 @@
> #string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTpm2AcpiTableRev_HELP
> #language en-US "This PCD defines initial revision of TPM2 ACPI table\n"
>
> "To support configuring from setup page, this PCD can be DynamicHii type and
> map to a setup option.<BR>\n"
>
> "For example, map to TCG2_VERSION.Tpm2AcpiTableRev to be configured by
> Tcg2ConfigDxe driver.<BR>\n"
> -
> "gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L\"TCG2_VERSION\"|gT
> cg2ConfigFormSetGuid|0x8|3|NV,BS<BR>"
> \ No newline at end of file
> +
> "gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L\"TCG2_VERSION\"|gT
> cg2ConfigFormSetGuid|0x8|3|NV,BS<BR>"
> --
> 2.7.0.windows.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] SecurityPkg HashLibRouter: Avoid incorrect PcdTcg2HashAlgorithmBitmap
2017-01-24 10:06 [PATCH] SecurityPkg HashLibRouter: Avoid incorrect PcdTcg2HashAlgorithmBitmap Star Zeng
2017-01-26 2:29 ` Yao, Jiewen
@ 2017-01-26 5:14 ` Zhang, Chao B
1 sibling, 0 replies; 3+ messages in thread
From: Zhang, Chao B @ 2017-01-26 5:14 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Yao, Jiewen
Star:
The patch is good to me. Reviewed-by: Chao Zhang <chao.b.zhang@intel.com>
-----Original Message-----
From: Zeng, Star
Sent: Tuesday, January 24, 2017 6:07 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>
Subject: [PATCH] SecurityPkg HashLibRouter: Avoid incorrect PcdTcg2HashAlgorithmBitmap
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=244
Currently, when software HashLib (HashLibBaseCryptoRouter) and related HashInstanceLib instances are used, PcdTcg2HashAlgorithmBitmap is expected to be configured to 0 in platform dsc.
But PcdTcg2HashAlgorithmBitmap has default value 0xFFFFFFFF in SecurityPkg.dec, and some platforms forget to configure it to 0 or still configure it to 0xFFFFFFFF in platform dsc, that will make final PcdTcg2HashAlgorithmBitmap value incorrect.
This patch is to add CONSTRUCTOR in HashLib (HashLibBaseCryptoRouter) and PcdTcg2HashAlgorithmBitmap will be set to 0 in the CONSTRUCTOR.
Current HASH_LIB_PEI_ROUTER_GUID HOB created in HashLibBaseCryptoRouterPei is shared between modules that links HashLibBaseCryptoRouterPei.
To avoid mutual interference, separated HASH_LIB_PEI_ROUTER_GUID HOBs with gEfiCallerIdGuid Identifier will be created for those modules.
This patch is also to add check in HashLib (HashLibBaseCryptoRouter) for the mismatch of supported HashMask between modules that may link different HashInstanceLib instances, warning will be reported if mismatch is found.
Cc: Yao Jiewen <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
.../HashLibBaseCryptoRouterDxe.c | 80 ++++++++-
.../HashLibBaseCryptoRouterDxe.inf | 3 +-
.../HashLibBaseCryptoRouterPei.c | 190 +++++++++++++++++----
.../HashLibBaseCryptoRouterPei.inf | 8 +-
SecurityPkg/SecurityPkg.dec | 4 +
SecurityPkg/SecurityPkg.uni | 8 +-
6 files changed, 254 insertions(+), 39 deletions(-)
diff --git a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.c b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.c
index 3250c3a01a0c..4775cfee2d7a 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.c
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRoute
+++ rDxe.c
@@ -3,7 +3,7 @@
hash handler registerd, such as SHA1, SHA256.
Platform can use PcdTpm2HashMask to mask some hash engines.
-Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved. <BR>
+Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved. <BR>
This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at @@ -28,6 +28,30 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
HASH_INTERFACE mHashInterface[HASH_COUNT] = {{{0}, NULL, NULL, NULL}};
UINTN mHashInterfaceCount = 0;
+UINT32 mSupportedHashMaskLast = 0;
+UINT32 mSupportedHashMaskCurrent = 0;
+
+/**
+ Check mismatch of supported HashMask between modules
+ that may link different HashInstanceLib instances.
+
+**/
+VOID
+CheckSupportedHashMaskMismatch (
+ VOID
+ )
+{
+ if (mSupportedHashMaskCurrent != mSupportedHashMaskLast) {
+ DEBUG ((
+ DEBUG_WARN,
+ "WARNING: There is mismatch of supported HashMask (0x%x - 0x%x) between modules\n",
+ mSupportedHashMaskCurrent,
+ mSupportedHashMaskLast
+ ));
+ DEBUG ((DEBUG_WARN, "that are linking different HashInstanceLib
+instances!\n"));
+ }
+}
+
/**
Start hash sequence.
@@ -50,6 +74,8 @@ HashStart (
return EFI_UNSUPPORTED;
}
+ CheckSupportedHashMaskMismatch ();
+
HashCtx = AllocatePool (sizeof(*HashCtx) * mHashInterfaceCount);
ASSERT (HashCtx != NULL);
@@ -90,6 +116,8 @@ HashUpdate (
return EFI_UNSUPPORTED;
}
+ CheckSupportedHashMaskMismatch ();
+
HashCtx = (HASH_HANDLE *)HashHandle;
for (Index = 0; Index < mHashInterfaceCount; Index++) { @@ -133,6 +161,8 @@ HashCompleteAndExtend (
return EFI_UNSUPPORTED;
}
+ CheckSupportedHashMaskMismatch ();
+
HashCtx = (HASH_HANDLE *)HashHandle;
ZeroMem (DigestList, sizeof(*DigestList));
@@ -180,6 +210,8 @@ HashAndExtend (
return EFI_UNSUPPORTED;
}
+ CheckSupportedHashMaskMismatch ();
+
HashStart (&HashHandle);
HashUpdate (HashHandle, DataToHash, DataToHashLen);
Status = HashCompleteAndExtend (HashHandle, PcrIndex, NULL, 0, DigestList); @@ -204,7 +236,6 @@ RegisterHashInterfaceLib ( {
UINTN Index;
UINT32 HashMask;
- UINT32 BiosSupportedHashMask;
EFI_STATUS Status;
//
@@ -218,21 +249,58 @@ RegisterHashInterfaceLib (
if (mHashInterfaceCount >= sizeof(mHashInterface)/sizeof(mHashInterface[0])) {
return EFI_OUT_OF_RESOURCES;
}
- BiosSupportedHashMask = PcdGet32 (PcdTcg2HashAlgorithmBitmap);
- Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, BiosSupportedHashMask | HashMask);
- ASSERT_EFI_ERROR (Status);
//
// Check duplication
//
for (Index = 0; Index < mHashInterfaceCount; Index++) {
if (CompareGuid (&mHashInterface[Index].HashGuid, &HashInterface->HashGuid)) {
+ DEBUG ((DEBUG_ERROR, "Hash Interface (%g) has been
+ registered\n"));
return EFI_ALREADY_STARTED;
}
}
+ //
+ // Record hash algorithm bitmap of CURRENT module which consumes HashLib.
+ //
+ mSupportedHashMaskCurrent = PcdGet32 (PcdTcg2HashAlgorithmBitmap) |
+ HashMask; Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap,
+ mSupportedHashMaskCurrent); ASSERT_EFI_ERROR (Status);
+
CopyMem (&mHashInterface[mHashInterfaceCount], HashInterface, sizeof(*HashInterface));
mHashInterfaceCount ++;
return EFI_SUCCESS;
-}
\ No newline at end of file
+}
+
+/**
+ The constructor function of HashLibBaseCryptoRouterDxe.
+
+ @param ImageHandle The firmware allocated handle for the EFI image.
+ @param SystemTable A pointer to the EFI System Table.
+
+ @retval EFI_SUCCESS The constructor executed correctly.
+
+**/
+EFI_STATUS
+EFIAPI
+HashLibBaseCryptoRouterDxeConstructor (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ //
+ // Record hash algorithm bitmap of LAST module which also consumes HashLib.
+ //
+ mSupportedHashMaskLast = PcdGet32 (PcdTcg2HashAlgorithmBitmap);
+
+ //
+ // Set PcdTcg2HashAlgorithmBitmap to 0 in CONSTRUCTOR for CURRENT module.
+ //
+ Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, 0); ASSERT_EFI_ERROR
+ (Status);
+
+ return EFI_SUCCESS;
+}
diff --git a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.inf b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.inf
index c4e40d47d998..6e660d4f14f8 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.inf
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRoute
+++ rDxe.inf
@@ -5,7 +5,7 @@
# hash handler registered, such as SHA1, SHA256. Platform can use PcdTpm2HashMask to # mask some hash engines.
#
-# Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2013 - 2017, Intel Corporation. All rights
+reserved.<BR>
# This program and the accompanying materials # are licensed and made available under the terms and conditions of the BSD License # which accompanies this distribution. The full text of the license may be found at @@ -23,6 +23,7 @@ [Defines]
MODULE_TYPE = DXE_DRIVER
VERSION_STRING = 1.0
LIBRARY_CLASS = HashLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
+ CONSTRUCTOR = HashLibBaseCryptoRouterDxeConstructor
#
# The following information is for reference only and not required by the build tools.
diff --git a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
index a4fc0c6595d8..3ed56f337942 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRoute
+++ rPei.c
@@ -3,7 +3,7 @@
hash handler registerd, such as SHA1, SHA256.
Platform can use PcdTpm2HashMask to mask some hash engines.
-Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved. <BR>
+Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved. <BR>
This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at @@ -23,6 +23,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Library/PcdLib.h>
#include <Library/HobLib.h>
#include <Library/HashLib.h>
+#include <Guid/ZeroGuid.h>
#include "HashLibBaseCryptoRouterCommon.h"
@@ -32,27 +33,95 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
EFI_GUID mHashLibPeiRouterGuid = HASH_LIB_PEI_ROUTER_GUID;
typedef struct {
+ //
+ // If gZeroGuid, SupportedHashMask is 0 for FIRST module which consumes HashLib
+ // or the hash algorithm bitmap of LAST module which consumes HashLib.
+ // HashInterfaceCount and HashInterface are all 0.
+ // If gEfiCallerIdGuid, HashInterfaceCount, HashInterface and SupportedHashMask
+ // are the hash interface information of CURRENT module which consumes HashLib.
+ //
+ EFI_GUID Identifier;
UINTN HashInterfaceCount;
HASH_INTERFACE HashInterface[HASH_COUNT];
+ UINT32 SupportedHashMask;
} HASH_INTERFACE_HOB;
/**
- This function get hash interface.
+ This function gets hash interface hob.
+
+ @param Identifier Identifier to get hash interface hob.
+
+ @retval hash interface hob.
+**/
+HASH_INTERFACE_HOB *
+InternalGetHashInterfaceHob (
+ EFI_GUID *Identifier
+ )
+{
+ EFI_PEI_HOB_POINTERS Hob;
+ HASH_INTERFACE_HOB *HashInterfaceHob;
+
+ Hob.Raw = GetFirstGuidHob (&mHashLibPeiRouterGuid);
+ while (Hob.Raw != NULL) {
+ HashInterfaceHob = GET_GUID_HOB_DATA (Hob);
+ if (CompareGuid (&HashInterfaceHob->Identifier, Identifier)) {
+ //
+ // Found the matched one.
+ //
+ return HashInterfaceHob;
+ }
+ Hob.Raw = GET_NEXT_HOB (Hob);
+ Hob.Raw = GetNextGuidHob (&mHashLibPeiRouterGuid, Hob.Raw);
+ }
+ return NULL;
+}
+
+/**
+ This function creates hash interface hob.
- @retval hash interface.
+ @param Identifier Identifier to create hash interface hob.
+
+ @retval hash interface hob.
**/
HASH_INTERFACE_HOB *
-InternalGetHashInterface (
- VOID
+InternalCreateHashInterfaceHob (
+ EFI_GUID *Identifier
)
{
- EFI_HOB_GUID_TYPE *Hob;
+ HASH_INTERFACE_HOB LocalHashInterfaceHob;
+
+ ZeroMem (&LocalHashInterfaceHob, sizeof(LocalHashInterfaceHob));
+ CopyGuid (&LocalHashInterfaceHob.Identifier, Identifier);
+ return BuildGuidDataHob (&mHashLibPeiRouterGuid,
+&LocalHashInterfaceHob, sizeof(LocalHashInterfaceHob)); }
- Hob = GetFirstGuidHob (&mHashLibPeiRouterGuid);
- if (Hob == NULL) {
- return NULL;
+/**
+ Check mismatch of supported HashMask between modules
+ that may link different HashInstanceLib instances.
+
+ @param HashInterfaceHobCurrent Pointer to hash interface hob for CURRENT module.
+
+**/
+VOID
+CheckSupportedHashMaskMismatch (
+ IN HASH_INTERFACE_HOB *HashInterfaceHobCurrent
+ )
+{
+ HASH_INTERFACE_HOB *HashInterfaceHobLast;
+
+ HashInterfaceHobLast = InternalGetHashInterfaceHob (&gZeroGuid);
+ ASSERT (HashInterfaceHobLast != NULL);
+
+ if ((HashInterfaceHobLast->SupportedHashMask != 0) &&
+ (HashInterfaceHobCurrent->SupportedHashMask != HashInterfaceHobLast->SupportedHashMask)) {
+ DEBUG ((
+ DEBUG_WARN,
+ "WARNING: There is mismatch of supported HashMask (0x%x - 0x%x) between modules\n",
+ HashInterfaceHobCurrent->SupportedHashMask,
+ HashInterfaceHobLast->SupportedHashMask
+ ));
+ DEBUG ((DEBUG_WARN, "that are linking different HashInstanceLib
+ instances!\n"));
}
- return (HASH_INTERFACE_HOB *)(Hob + 1); }
/**
@@ -74,7 +143,7 @@ HashStart (
UINTN Index;
UINT32 HashMask;
- HashInterfaceHob = InternalGetHashInterface ();
+ HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
if (HashInterfaceHob == NULL) {
return EFI_UNSUPPORTED;
}
@@ -83,6 +152,8 @@ HashStart (
return EFI_UNSUPPORTED;
}
+ CheckSupportedHashMaskMismatch (HashInterfaceHob);
+
HashCtx = AllocatePool (sizeof(*HashCtx) * HashInterfaceHob->HashInterfaceCount);
ASSERT (HashCtx != NULL);
@@ -120,7 +191,7 @@ HashUpdate (
UINTN Index;
UINT32 HashMask;
- HashInterfaceHob = InternalGetHashInterface ();
+ HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
if (HashInterfaceHob == NULL) {
return EFI_UNSUPPORTED;
}
@@ -129,6 +200,8 @@ HashUpdate (
return EFI_UNSUPPORTED;
}
+ CheckSupportedHashMaskMismatch (HashInterfaceHob);
+
HashCtx = (HASH_HANDLE *)HashHandle;
for (Index = 0; Index < HashInterfaceHob->HashInterfaceCount; Index++) { @@ -169,7 +242,7 @@ HashCompleteAndExtend (
EFI_STATUS Status;
UINT32 HashMask;
- HashInterfaceHob = InternalGetHashInterface ();
+ HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
if (HashInterfaceHob == NULL) {
return EFI_UNSUPPORTED;
}
@@ -178,6 +251,8 @@ HashCompleteAndExtend (
return EFI_UNSUPPORTED;
}
+ CheckSupportedHashMaskMismatch (HashInterfaceHob);
+
HashCtx = (HASH_HANDLE *)HashHandle;
ZeroMem (DigestList, sizeof(*DigestList));
@@ -222,7 +297,7 @@ HashAndExtend (
HASH_HANDLE HashHandle;
EFI_STATUS Status;
- HashInterfaceHob = InternalGetHashInterface ();
+ HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
if (HashInterfaceHob == NULL) {
return EFI_UNSUPPORTED;
}
@@ -231,6 +306,8 @@ HashAndExtend (
return EFI_UNSUPPORTED;
}
+ CheckSupportedHashMaskMismatch (HashInterfaceHob);
+
HashStart (&HashHandle);
HashUpdate (HashHandle, DataToHash, DataToHashLen);
Status = HashCompleteAndExtend (HashHandle, PcrIndex, NULL, 0, DigestList); @@ -255,9 +332,7 @@ RegisterHashInterfaceLib ( {
UINTN Index;
HASH_INTERFACE_HOB *HashInterfaceHob;
- HASH_INTERFACE_HOB LocalHashInterfaceHob;
UINT32 HashMask;
- UINT32 BiosSupportedHashMask;
EFI_STATUS Status;
//
@@ -268,10 +343,9 @@ RegisterHashInterfaceLib (
return EFI_UNSUPPORTED;
}
- HashInterfaceHob = InternalGetHashInterface ();
+ HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
if (HashInterfaceHob == NULL) {
- ZeroMem (&LocalHashInterfaceHob, sizeof(LocalHashInterfaceHob));
- HashInterfaceHob = BuildGuidDataHob (&mHashLibPeiRouterGuid, &LocalHashInterfaceHob, sizeof(LocalHashInterfaceHob));
+ HashInterfaceHob = InternalCreateHashInterfaceHob
+ (&gEfiCallerIdGuid);
if (HashInterfaceHob == NULL) {
return EFI_OUT_OF_RESOURCES;
}
@@ -280,26 +354,84 @@ RegisterHashInterfaceLib (
if (HashInterfaceHob->HashInterfaceCount >= HASH_COUNT) {
return EFI_OUT_OF_RESOURCES;
}
- BiosSupportedHashMask = PcdGet32 (PcdTcg2HashAlgorithmBitmap);
- Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, BiosSupportedHashMask | HashMask);
- ASSERT_EFI_ERROR (Status);
//
// Check duplication
//
for (Index = 0; Index < HashInterfaceHob->HashInterfaceCount; Index++) {
if (CompareGuid (&HashInterfaceHob->HashInterface[Index].HashGuid, &HashInterface->HashGuid)) {
- //
- // In PEI phase, there will be shadow driver dispatched again.
- //
- DEBUG ((EFI_D_INFO, "RegisterHashInterfaceLib - Override\n"));
- CopyMem (&HashInterfaceHob->HashInterface[Index], HashInterface, sizeof(*HashInterface));
- return EFI_SUCCESS;
+ DEBUG ((DEBUG_ERROR, "Hash Interface (%g) has been registered\n"));
+ return EFI_ALREADY_STARTED;
}
}
+ //
+ // Record hash algorithm bitmap of CURRENT module which consumes HashLib.
+ //
+ HashInterfaceHob->SupportedHashMask = PcdGet32
+ (PcdTcg2HashAlgorithmBitmap) | HashMask; Status = PcdSet32S
+ (PcdTcg2HashAlgorithmBitmap, HashInterfaceHob->SupportedHashMask);
+ ASSERT_EFI_ERROR (Status);
+
CopyMem (&HashInterfaceHob->HashInterface[HashInterfaceHob->HashInterfaceCount], HashInterface, sizeof(*HashInterface));
HashInterfaceHob->HashInterfaceCount ++;
return EFI_SUCCESS;
-}
\ No newline at end of file
+}
+
+/**
+ The constructor function of HashLibBaseCryptoRouterPei.
+
+ @param FileHandle The handle of FFS header the loaded driver.
+ @param PeiServices The pointer to the PEI services.
+
+ @retval EFI_SUCCESS The constructor executes successfully.
+ @retval EFI_OUT_OF_RESOURCES There is no enough resource for the constructor.
+
+**/
+EFI_STATUS
+EFIAPI
+HashLibBaseCryptoRouterPeiConstructor (
+ IN EFI_PEI_FILE_HANDLE FileHandle,
+ IN CONST EFI_PEI_SERVICES **PeiServices
+ )
+{
+ EFI_STATUS Status;
+ HASH_INTERFACE_HOB *HashInterfaceHob;
+
+ HashInterfaceHob = InternalGetHashInterfaceHob (&gZeroGuid); if
+ (HashInterfaceHob == NULL) {
+ //
+ // No HOB with gZeroGuid Identifier has been created,
+ // this is FIRST module which consumes HashLib.
+ // Create the HOB with gZeroGuid Identifier.
+ //
+ HashInterfaceHob = InternalCreateHashInterfaceHob (&gZeroGuid);
+ if (HashInterfaceHob == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+ } else {
+ //
+ // Record hash algorithm bitmap of LAST module which also consumes HashLib.
+ //
+ HashInterfaceHob->SupportedHashMask = PcdGet32
+ (PcdTcg2HashAlgorithmBitmap); }
+
+ HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
+ if (HashInterfaceHob != NULL) {
+ //
+ // In PEI phase, some modules may call RegisterForShadow and will be
+ // shadowed and executed again after memory is discovered.
+ // This is the second execution of this module, clear the hash interface
+ // information registered at its first execution.
+ //
+ ZeroMem (&HashInterfaceHob->HashInterface, sizeof
+ (*HashInterfaceHob) - sizeof (EFI_GUID)); }
+
+ //
+ // Set PcdTcg2HashAlgorithmBitmap to 0 in CONSTRUCTOR for CURRENT module.
+ //
+ Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, 0); ASSERT_EFI_ERROR
+ (Status);
+
+ return EFI_SUCCESS;
+}
diff --git a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf
index f5ca5d4635d8..eebf90e2ef40 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRoute
+++ rPei.inf
@@ -5,7 +5,7 @@
# hash handler registered, such as SHA1, SHA256. Platform can use PcdTpm2HashMask to # mask some hash engines.
#
-# Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2013 - 2017, Intel Corporation. All rights
+reserved.<BR>
# This program and the accompanying materials # are licensed and made available under the terms and conditions of the BSD License # which accompanies this distribution. The full text of the license may be found at @@ -23,6 +23,7 @@ [Defines]
MODULE_TYPE = PEIM
VERSION_STRING = 1.0
LIBRARY_CLASS = HashLib|PEIM
+ CONSTRUCTOR = HashLibBaseCryptoRouterPeiConstructor
#
# The following information is for reference only and not required by the build tools.
@@ -38,6 +39,7 @@ [Sources]
[Packages]
MdePkg/MdePkg.dec
SecurityPkg/SecurityPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
[LibraryClasses]
BaseLib
@@ -48,6 +50,10 @@ [LibraryClasses]
PcdLib
HobLib
+[Guids]
+ ## CONSUMES ## GUID
+ gZeroGuid
+
[Pcd]
gEfiSecurityPkgTokenSpaceGuid.PcdTpm2HashMask ## CONSUMES
## SOMETIMES_CONSUMES
diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec index 0c64d25b0cbc..88154611ae9c 100644
--- a/SecurityPkg/SecurityPkg.dec
+++ b/SecurityPkg/SecurityPkg.dec
@@ -458,6 +458,10 @@ [PcdsDynamic, PcdsDynamicEx]
## This PCD indicated final BIOS supported Hash mask.
# Bios may choose to register a subset of PcdTpm2HashMask.
# So this PCD is final value of how many hash algo is extended to PCR.
+ # If software HashLib(HashLibBaseCryptoRouter) solution is chosen,
+ this PCD # has no need to be configured in platform dsc and will be
+ set to correct # value by the HashLib instance according to the
+ HashInstanceLib instances # linked, and the value of this PCD should be got in module entrypoint.
# @Prompt Hash Algorithm bitmap.
gEfiSecurityPkgTokenSpaceGuid.PcdTcg2HashAlgorithmBitmap|0xFFFFFFFF|UINT32|0x00010016
diff --git a/SecurityPkg/SecurityPkg.uni b/SecurityPkg/SecurityPkg.uni index 17d36c0d6014..126351624056 100644
--- a/SecurityPkg/SecurityPkg.uni
+++ b/SecurityPkg/SecurityPkg.uni
@@ -191,7 +191,11 @@
#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTcg2HashAlgorithmBitmap_HELP #language en-US "This PCD indicated final BIOS supported Hash mask.\n"
"Bios may choose to register a subset of PcdTpm2HashMask.\n"
- "So this PCD is final value of how many hash algo is extended to PCR."
+ "So this PCD is final value of how many hash algo is extended to PCR.\n"
+ "If software HashLib(HashLibBaseCryptoRouter) solution is chosen, this PCD\n"
+ "has no need to be configured in platform dsc and will be set to correct\n"
+ "value by the HashLib instance according to the HashInstanceLib instances\n"
+ "linked, and the value of this PCD should be got in module entrypoint."
#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTcgLogAreaMinLen_PROMPT #language en-US "Minimum length(in bytes) of the system preboot TCG event log area(LAML)."
@@ -233,4 +237,4 @@
#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTpm2AcpiTableRev_HELP #language en-US "This PCD defines initial revision of TPM2 ACPI table\n"
"To support configuring from setup page, this PCD can be DynamicHii type and map to a setup option.<BR>\n"
"For example, map to TCG2_VERSION.Tpm2AcpiTableRev to be configured by Tcg2ConfigDxe driver.<BR>\n"
- "gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L\"TCG2_VERSION\"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS<BR>"
\ No newline at end of file
+ "gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L\"TCG2_VERSION\"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS<BR>"
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-01-26 5:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-24 10:06 [PATCH] SecurityPkg HashLibRouter: Avoid incorrect PcdTcg2HashAlgorithmBitmap Star Zeng
2017-01-26 2:29 ` Yao, Jiewen
2017-01-26 5:14 ` Zhang, Chao B
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox