<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="blue" vlink="purple" style="word-wrap:break-word"> <div class="WordSection1"> OK, Would you please to share the PCD configuration works before and PCD configuration fails now? As well as your DSC file on how to configure the library.<o:p></o:p> <o:p> </o:p> I would like to understand the problem statement from real use case, because the issue description cannot provide useful information to me.<o:p></o:p> <o:p> </o:p> <div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt"> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> From: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@intel.com> Sent: Tuesday, August 10, 2021 2:27 PM To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io Cc: Wang, Jian J <jian.j.wang@intel.com> Subject: Re: [PATCH] Reallocate TPM Active PCRs based on platform support.<o:p></o:p> </div> </div> <o:p> </o:p> <div> Hi Jiewen,<o:p></o:p> </div> <div> <o:p> </o:p> </div> <div> Indeed, this bug has existed for a long time in this code. What recently changed are the TPM configurations we are testing and exposed the issue; this can be reproduced when the BIOS supported algorithms are a strict subset of the PCRs currently active in the TPM.<o:p></o:p> </div> <div> <o:p> </o:p> </div> <div> Now that we are using TPM configurations with support for additional PCR banks (ex. SHA384 and SM3) the bug has been exposed when compiling a BIOS without support for these PCR banks which are active by default in the some of the TPMs.<o:p></o:p> </div> <div> <o:p> </o:p> </div> <div> Regards,<o:p></o:p> </div> <div> -Rodrigo<o:p></o:p> </div> <div> <o:p> </o:p> </div> <div class="MsoNormal" align="center" style="text-align:center"> <hr size="2" width="98%" align="center"> </div> <div id="divRplyFwdMsg"> From: Yao, Jiewen <<a href="mailto:jiewen.yao@intel.com">jiewen.yao@intel.com</a>> Sent: Sunday, August 8, 2021 6:13 PM To: Gonzalez Del Cueto, Rodrigo <<a href="mailto:rodrigo.gonzalez.del.cueto@intel.com">rodrigo.gonzalez.del.cueto@intel.com</a>>; <a href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a> <<a href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a>> Cc: Wang, Jian J <<a href="mailto:jian.j.wang@intel.com">jian.j.wang@intel.com</a>> Subject: RE: [PATCH] Reallocate TPM Active PCRs based on platform support. <o:p></o:p> <div> <o:p></o:p> </div> </div> <div> <div> Hi Rodrigo I don’t understand the problem statement. This code has been there for long time. What is changed recently ? Thank you Yao Jiewen > -----Original Message----- > From: Gonzalez Del Cueto, Rodrigo <<a href="mailto:rodrigo.gonzalez.del.cueto@intel.com">rodrigo.gonzalez.del.cueto@intel.com</a>> > Sent: Thursday, August 5, 2021 7:28 AM > To: <a href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a> > Cc: Gonzalez Del Cueto, Rodrigo <<a href="mailto:rodrigo.gonzalez.del.cueto@intel.com">rodrigo.gonzalez.del.cueto@intel.com</a>>; > Wang, Jian J <<a href="mailto:jian.j.wang@intel.com">jian.j.wang@intel.com</a>>; Yao, Jiewen <<a href="mailto:jiewen.yao@intel.com">jiewen.yao@intel.com</a>> > Subject: [PATCH] Reallocate TPM Active PCRs based on platform support. > > REF: <a href="https://bugzilla.tianocore.org/show_bug.cgi?id=3515">https://bugzilla.tianocore.org/show_bug.cgi?id=3515</a> > > 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 > <<a href="mailto:rodrigo.gonzalez.del.cueto@intel.com">rodrigo.gonzalez.del.cueto@intel.com</a>> > > Cc: Jian J Wang <<a href="mailto:jian.j.wang@intel.com">jian.j.wang@intel.com</a>> > Cc: Jiewen Yao <<a href="mailto:jiewen.yao@intel.com">jiewen.yao@intel.com</a>> > --- > SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.c > | 6 +++++- > SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c | > 6 +++++- > SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 18 > +++++++++++++++++- > SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 1 + > 4 files changed, 28 insertions(+), 3 deletions(-) > > diff --git > a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe. > c > b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe. > c > index 7a0f61efbb..0821159120 100644 > --- > a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe. > c > +++ > b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe. > c > @@ -230,13 +230,17 @@ RegisterHashInterfaceLib ( > { > UINTN Index; > UINT32 HashMask; > + UINT32 Tpm2HashMask; > EFI_STATUS Status; > > // > // Check allow > // > HashMask = Tpm2GetHashMaskFromAlgo (&HashInterface->HashGuid); > - if ((HashMask & PcdGet32 (PcdTpm2HashMask)) == 0) { > + 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 42cb562f67..6ae51dbce4 100644 > --- > a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c > +++ > b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c > @@ -327,13 +327,17 @@ 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) { > + 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 93a8803ff6..5ad6a45cf3 100644 > --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > @@ -262,6 +262,7 @@ SyncPcrAllocationsAndPcrMask ( > { > EFI_STATUS Status; > EFI_TCG2_EVENT_ALGORITHM_BITMAP TpmHashAlgorithmBitmap; > + EFI_TCG2_EVENT_ALGORITHM_BITMAP BiosHashAlgorithmBitmap; > UINT32 TpmActivePcrBanks; > UINT32 NewTpmActivePcrBanks; > UINT32 Tpm2PcrMask; > @@ -273,16 +274,27 @@ SyncPcrAllocationsAndPcrMask ( > // Determine the current TPM support and the Platform PCR mask. > // > Status = Tpm2GetCapabilitySupportedAndActivePcrs > (&TpmHashAlgorithmBitmap, &TpmActivePcrBanks); > + > ASSERT_EFI_ERROR (Status); > + > + DEBUG ((EFI_D_INFO, "Tpm2GetCapabilitySupportedAndActivePcrs - > TpmHashAlgorithmBitmap: 0x%08x\n", TpmHashAlgorithmBitmap)); > + DEBUG ((EFI_D_INFO, "Tpm2GetCapabilitySupportedAndActivePcrs - > TpmActivePcrBanks 0x%08x\n", TpmActivePcrBanks)); > > Tpm2PcrMask = PcdGet32 (PcdTpm2HashMask); > if (Tpm2PcrMask == 0) { > // > // if PcdTPm2HashMask is zero, use ActivePcr setting > // > + DEBUG ((EFI_D_VERBOSE, "Initializing PcdTpm2HashMask to > TpmActivePcrBanks 0x%08x\n", TpmActivePcrBanks)); > PcdSet32S (PcdTpm2HashMask, TpmActivePcrBanks); > + DEBUG ((EFI_D_VERBOSE, "Initializing Tpm2PcrMask to TpmActivePcrBanks > 0x%08x\n", Tpm2PcrMask)); > Tpm2PcrMask = TpmActivePcrBanks; > } > + > + BiosHashAlgorithmBitmap = PcdGet32 (PcdTcg2HashAlgorithmBitmap); > + DEBUG ((EFI_D_INFO, "PcdTcg2HashAlgorithmBitmap 0x%08x\n", > BiosHashAlgorithmBitmap)); > + DEBUG ((EFI_D_INFO, "Tpm2PcrMask 0x%08x\n", Tpm2PcrMask)); // Active > PCR banks from TPM input > + DEBUG ((EFI_D_INFO, "TpmActivePcrBanks & BiosHashAlgorithmBitmap = > 0x%08x\n", NewTpmActivePcrBanks)); > > // > // Find the intersection of Pcd support and TPM support. > @@ -294,9 +306,12 @@ SyncPcrAllocationsAndPcrMask ( > // If there are active PCR banks that are not supported by the Platform mask, > // update the TPM allocations and reboot the machine. > // > - if ((TpmActivePcrBanks & Tpm2PcrMask) != TpmActivePcrBanks) { > + if (((TpmActivePcrBanks & Tpm2PcrMask) != TpmActivePcrBanks) || > + ((TpmActivePcrBanks & BiosHashAlgorithmBitmap) != TpmActivePcrBanks)) { > NewTpmActivePcrBanks = TpmActivePcrBanks & Tpm2PcrMask; > + NewTpmActivePcrBanks &= BiosHashAlgorithmBitmap; > > + DEBUG ((EFI_D_INFO, "NewTpmActivePcrBanks 0x%08x\n", > NewTpmActivePcrBanks)); > DEBUG ((EFI_D_INFO, "%a - Reallocating PCR banks from 0x%X to 0x%X.\n", > __FUNCTION__, TpmActivePcrBanks, NewTpmActivePcrBanks)); > if (NewTpmActivePcrBanks == 0) { > DEBUG ((EFI_D_ERROR, "%a - No viable PCRs active! Please set a less > restrictive value for PcdTpm2HashMask!\n", __FUNCTION__)); > @@ -331,6 +346,7 @@ SyncPcrAllocationsAndPcrMask ( > } > > Status = PcdSet32S (PcdTpm2HashMask, NewTpm2PcrMask); > + DEBUG ((EFI_D_INFO, "Setting 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.31.1.windows.1<o:p></o:p> </div> </div> </div> </div> </body> </html> <div width="1" style="color:white;clear:both">_._,_._,_</div> <hr> Groups.io Links: You receive all messages sent to this group. <a target="_blank" href="https://edk2.groups.io/g/devel/message/79085">View/Reply Online (#79085)</a> | | <a target="_blank" href="https://groups.io/mt/84674454/1813853">Mute This Topic</a> | <a href="https://edk2.groups.io/g/devel/post">New Topic</a> <a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> | <a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> | <a href="https://edk2.groups.io/g/devel/unsub">Unsubscribe</a> [edk2-devel-archive@redhat.com] <div width="1" style="color:white;clear:both">_._,_._,_</div>