[edk2-devel] [PATCH] Reallocate TPM Active PCRs based on platform support.
Yao, Jiewen
jiewen.yao at intel.com
Mon Aug 9 01:13:38 UTC 2021
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 <rodrigo.gonzalez.del.cueto at intel.com>
> Sent: Thursday, August 5, 2021 7:28 AM
> To: devel at edk2.groups.io
> Cc: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto at intel.com>;
> Wang, Jian J <jian.j.wang at intel.com>; Yao, Jiewen <jiewen.yao at intel.com>
> Subject: [PATCH] Reallocate TPM Active PCRs based on platform support.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3515
>
> 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.cueto at intel.com>
>
> Cc: Jian J Wang <jian.j.wang at intel.com>
> Cc: Jiewen Yao <jiewen.yao at intel.com>
> ---
> 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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78910): https://edk2.groups.io/g/devel/message/78910
Mute This Topic: https://groups.io/mt/84674454/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list