Pushed 195f0119731dbc4b93b4d485998dac3bbf8629a3 > -----Original Message----- > From: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cu...@intel.com> > Sent: Friday, December 17, 2021 10:48 AM > To: devel@edk2.groups.io > Cc: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cu...@intel.com>; > Wang, Jian J <jian.j.w...@intel.com>; Yao, Jiewen <jiewen....@intel.com> > Subject: [PATCH] SecurityPkg: Reallocate TPM Active PCRs based on platform > support > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3515 > > In V4: Fixed patch format and uncrustify cleanup > > In V3: Cleaned up comments, debug prints and updated patch to use the > new debug ENUM definitions. > > - Replaced EFI_D_INFO with DEBUG_INFO. > - Replaced EFI_D_VERBOSE with DEBUG_VERBOSE. > > In V2: Add case to RegisterHashInterfaceLib logic > > RegisterHashInterfaceLib needs to correctly handle registering the HashLib > instance supported algorithm bitmap when PcdTpm2HashMask is set to zero. > > The current implementation of SyncPcrAllocationsAndPcrMask() triggers > PCR bank reallocation only based on the intersection between > TpmActivePcrBanks and PcdTpm2HashMask. > > When the software HashLibBaseCryptoRouter solution is used, no PCR bank > reallocation is occurring based on the supported hashing algorithms > registered by the HashLib instances. > > Need to have an additional check for the intersection between the > TpmActivePcrBanks and the PcdTcg2HashAlgorithmBitmap populated by the > HashLib instances present on the platform's BIOS. > > Signed-off-by: Rodrigo Gonzalez del Cueto > <rodrigo.gonzalez.del.cu...@intel.com> > Cc: Jian J Wang <jian.j.w...@intel.com> > Cc: Jiewen Yao <jiewen....@intel.com> > --- > SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.c > | 11 ++++++++--- > SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c | > 11 ++++++++--- > SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 43 > +++++++++++++++++++++++++++++++------------ > SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | > 1 + > 4 files changed, 48 insertions(+), 18 deletions(-) > > diff --git > a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe. > c > b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe. > c > index 59639d0538..ee8fe6e06e 100644 > --- > a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe. > c > +++ > b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe. > c > @@ -3,7 +3,7 @@ > hash handler registered, such as SHA1, SHA256. > Platform can use PcdTpm2HashMask to mask some hash engines. > > -Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved. <BR> > +Copyright (c) 2013 - 2021, Intel Corporation. All rights reserved. <BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -234,13 +234,18 @@ RegisterHashInterfaceLib ( > { > UINTN Index; > UINT32 HashMask; > + UINT32 Tpm2HashMask; > EFI_STATUS Status; > > // > // Check allow > // > - HashMask = Tpm2GetHashMaskFromAlgo (&HashInterface->HashGuid); > - if ((HashMask & PcdGet32 (PcdTpm2HashMask)) == 0) { > + HashMask = Tpm2GetHashMaskFromAlgo (&HashInterface->HashGuid); > + Tpm2HashMask = PcdGet32 (PcdTpm2HashMask); > + > + if ((Tpm2HashMask != 0) && > + ((HashMask & Tpm2HashMask) == 0)) > + { > return EFI_UNSUPPORTED; > } > > diff --git > a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c > b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c > index e21103d371..eeb424b6c3 100644 > --- > a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c > +++ > b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c > @@ -3,7 +3,7 @@ > hash handler registered, such as SHA1, SHA256. > Platform can use PcdTpm2HashMask to mask some hash engines. > > -Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved. <BR> > +Copyright (c) 2013 - 2021, Intel Corporation. All rights reserved. <BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -330,13 +330,18 @@ RegisterHashInterfaceLib ( > UINTN Index; > HASH_INTERFACE_HOB *HashInterfaceHob; > UINT32 HashMask; > + UINT32 Tpm2HashMask; > EFI_STATUS Status; > > // > // Check allow > // > - HashMask = Tpm2GetHashMaskFromAlgo (&HashInterface->HashGuid); > - if ((HashMask & PcdGet32 (PcdTpm2HashMask)) == 0) { > + HashMask = Tpm2GetHashMaskFromAlgo (&HashInterface->HashGuid); > + Tpm2HashMask = PcdGet32 (PcdTpm2HashMask); > + > + if ((Tpm2HashMask != 0) && > + ((HashMask & Tpm2HashMask) == 0)) > + { > return EFI_UNSUPPORTED; > } > > diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > index a97a4e7f2d..0da89b795e 100644 > --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > @@ -1,7 +1,7 @@ > /** @file > Initialize TPM2 device and measure FVs before handing off control to DXE. > > -Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR> > Copyright (c) 2017, Microsoft Corporation. All rights reserved. <BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > @@ -252,7 +252,7 @@ EndofPeiSignalNotifyCallBack ( > > /** > Make sure that the current PCR allocations, the TPM supported PCRs, > - and the PcdTpm2HashMask are all in agreement. > + PcdTcg2HashAlgorithmBitmap and the PcdTpm2HashMask are all in > agreement. > **/ > VOID > SyncPcrAllocationsAndPcrMask ( > @@ -261,6 +261,7 @@ SyncPcrAllocationsAndPcrMask ( > { > EFI_STATUS Status; > EFI_TCG2_EVENT_ALGORITHM_BITMAP TpmHashAlgorithmBitmap; > + EFI_TCG2_EVENT_ALGORITHM_BITMAP BiosHashAlgorithmBitmap; > UINT32 TpmActivePcrBanks; > UINT32 NewTpmActivePcrBanks; > UINT32 Tpm2PcrMask; > @@ -274,33 +275,50 @@ SyncPcrAllocationsAndPcrMask ( > Status = Tpm2GetCapabilitySupportedAndActivePcrs > (&TpmHashAlgorithmBitmap, &TpmActivePcrBanks); > ASSERT_EFI_ERROR (Status); > > + DEBUG ((DEBUG_INFO, "Tpm2GetCapabilitySupportedAndActivePcrs - > TpmHashAlgorithmBitmap: 0x%08x\n", TpmHashAlgorithmBitmap)); > + DEBUG ((DEBUG_INFO, "Tpm2GetCapabilitySupportedAndActivePcrs - > TpmActivePcrBanks 0x%08x\n", TpmActivePcrBanks)); > + > Tpm2PcrMask = PcdGet32 (PcdTpm2HashMask); > if (Tpm2PcrMask == 0) { > // > - // if PcdTPm2HashMask is zero, use ActivePcr setting > + // If PcdTpm2HashMask is zero, use ActivePcr setting. > + // Only when PcdTpm2HashMask is initialized to 0, will it be updated to > current Active Pcrs. > // > PcdSet32S (PcdTpm2HashMask, TpmActivePcrBanks); > Tpm2PcrMask = TpmActivePcrBanks; > } > > - // > - // Find the intersection of Pcd support and TPM support. > - // If banks are missing from the TPM support that are in the PCD, update > the > PCD. > - // If banks are missing from the PCD that are active in the TPM, > reallocate the > banks and reboot. > - // > + DEBUG ((DEBUG_INFO, "Tpm2PcrMask 0x%08x\n", Tpm2PcrMask)); > > // > - // If there are active PCR banks that are not supported by the Platform > mask, > - // update the TPM allocations and reboot the machine. > + // The Active PCRs in the TPM need to be a strict subset of the hashing > algorithms supported by BIOS. > // > - if ((TpmActivePcrBanks & Tpm2PcrMask) != TpmActivePcrBanks) { > - NewTpmActivePcrBanks = TpmActivePcrBanks & Tpm2PcrMask; > + // * Find the intersection of Pcd support and TPM active PCRs. If banks are > missing from the TPM support > + // that are in the PCD, update the PCD. > + // * Find intersection of TPM Active PCRs and BIOS supported algorithms. If > there are active PCR banks > + // that are not supported by the platform, update the TPM allocations and > reboot. > + // Note: When the HashLibBaseCryptoRouter solution is used, the hash > algorithm support from BIOS is reported > + // by Tcg2HashAlgorithmBitmap, which is populated by HashLib > instances > at runtime. > + BiosHashAlgorithmBitmap = PcdGet32 (PcdTcg2HashAlgorithmBitmap); > + DEBUG ((DEBUG_INFO, "Tcg2HashAlgorithmBitmap: 0x%08x\n", > BiosHashAlgorithmBitmap)); > + > + if (((TpmActivePcrBanks & Tpm2PcrMask) != TpmActivePcrBanks) || > + ((TpmActivePcrBanks & BiosHashAlgorithmBitmap) != TpmActivePcrBanks)) > + { > + DEBUG ((DEBUG_INFO, "TpmActivePcrBanks & Tpm2PcrMask = 0x%08x\n", > (TpmActivePcrBanks & Tpm2PcrMask))); > + DEBUG ((DEBUG_INFO, "TpmActivePcrBanks & BiosHashAlgorithmBitmap = > 0x%08x\n", (TpmActivePcrBanks & BiosHashAlgorithmBitmap))); > + NewTpmActivePcrBanks = TpmActivePcrBanks; > + NewTpmActivePcrBanks &= Tpm2PcrMask; > + NewTpmActivePcrBanks &= BiosHashAlgorithmBitmap; > + DEBUG ((DEBUG_INFO, "NewTpmActivePcrBanks 0x%08x\n", > NewTpmActivePcrBanks)); > > DEBUG ((DEBUG_INFO, "%a - Reallocating PCR banks from 0x%X to 0x%X.\n", > __FUNCTION__, TpmActivePcrBanks, NewTpmActivePcrBanks)); > + > if (NewTpmActivePcrBanks == 0) { > DEBUG ((DEBUG_ERROR, "%a - No viable PCRs active! Please set a less > restrictive value for PcdTpm2HashMask!\n", __FUNCTION__)); > ASSERT (FALSE); > } else { > + DEBUG ((DEBUG_ERROR, "Tpm2PcrAllocateBanks > (TpmHashAlgorithmBitmap: 0x%08x, NewTpmActivePcrBanks: 0x%08x)\n", > TpmHashAlgorithmBitmap, NewTpmActivePcrBanks)); > Status = Tpm2PcrAllocateBanks (NULL, (UINT32)TpmHashAlgorithmBitmap, > NewTpmActivePcrBanks); > if (EFI_ERROR (Status)) { > // > @@ -331,6 +349,7 @@ SyncPcrAllocationsAndPcrMask ( > } > > Status = PcdSet32S (PcdTpm2HashMask, NewTpm2PcrMask); > + DEBUG ((DEBUG_ERROR, "Set PcdTpm2Hash Mask to 0x%08x\n", > NewTpm2PcrMask)); > ASSERT_EFI_ERROR (Status); > } > } > diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf > b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf > index 06c26a2904..17ad116126 100644 > --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf > +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf > @@ -86,6 +86,7 @@ > ## SOMETIMES_CONSUMES > ## SOMETIMES_PRODUCES > gEfiSecurityPkgTokenSpaceGuid.PcdTpm2HashMask > + gEfiSecurityPkgTokenSpaceGuid.PcdTcg2HashAlgorithmBitmap > ## > CONSUMES > > [Depex] > gEfiPeiMasterBootModePpiGuid AND > -- > 2.26.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#85072): https://edk2.groups.io/g/devel/message/85072 Mute This Topic: https://groups.io/mt/87782228/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
