[edk2-devel] [PATCH] Reallocate TPM Active PCRs based on platform support.

Rodrigo Gonzalez del Cueto posted 1 patch 2 years, 5 months ago
Failed in applying to current master (apply log)
SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.c |  6 +++++-
SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c |  6 +++++-
SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c                                        | 67 ++++++++++++++++++++++++++++++++++++++++++-------------------------
SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf                                      |  1 +
4 files changed, 53 insertions(+), 27 deletions(-)
[edk2-devel] [PATCH] Reallocate TPM Active PCRs based on platform support.
Posted by Rodrigo Gonzalez del Cueto 2 years, 5 months ago
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3515

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.cueto@intel.com>

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
---
 SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.c |  6 +++++-
 SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c |  6 +++++-
 SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c                                        | 67 ++++++++++++++++++++++++++++++++++++++++++-------------------------
 SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf                                      |  1 +
 4 files changed, 53 insertions(+), 27 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..582b9377e5 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
 
@@ -253,7 +253,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 (
@@ -262,52 +262,68 @@ SyncPcrAllocationsAndPcrMask (
 {
   EFI_STATUS                        Status;
   EFI_TCG2_EVENT_ALGORITHM_BITMAP   TpmHashAlgorithmBitmap;
+  EFI_TCG2_EVENT_ALGORITHM_BITMAP   BiosHashAlgorithmBitmap;
   UINT32                            TpmActivePcrBanks;
   UINT32                            NewTpmActivePcrBanks;
   UINT32                            Tpm2PcrMask;
   UINT32                            NewTpm2PcrMask;
 
-  DEBUG ((EFI_D_ERROR, "SyncPcrAllocationsAndPcrMask!\n"));
+  DEBUG ((DEBUG_ERROR, "SyncPcrAllocationsAndPcrMask!\n"));
 
   //
   // Determine the current TPM support and the Platform PCR mask.
   //
   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;
   }
+  DEBUG ((DEBUG_INFO, "Tpm2PcrMask 0x%08x\n", Tpm2PcrMask));
 
   //
-  // 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.
-  //
-
-  //
-  // 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;
-
-    DEBUG ((EFI_D_INFO, "%a - Reallocating PCR banks from 0x%X to 0x%X.\n", __FUNCTION__, TpmActivePcrBanks, NewTpmActivePcrBanks));
+  // * 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 ((EFI_D_ERROR, "%a - No viable PCRs active! Please set a less restrictive value for PcdTpm2HashMask!\n", __FUNCTION__));
+      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)) {
         //
         // We can't do much here, but we hope that this doesn't happen.
         //
-        DEBUG ((EFI_D_ERROR, "%a - Failed to reallocate PCRs!\n", __FUNCTION__));
+        DEBUG ((DEBUG_ERROR, "%a - Failed to reallocate PCRs!\n", __FUNCTION__));
         ASSERT_EFI_ERROR (Status);
       }
       //
@@ -324,13 +340,14 @@ SyncPcrAllocationsAndPcrMask (
   if ((Tpm2PcrMask & TpmHashAlgorithmBitmap) != Tpm2PcrMask) {
     NewTpm2PcrMask = Tpm2PcrMask & TpmHashAlgorithmBitmap;
 
-    DEBUG ((EFI_D_INFO, "%a - Updating PcdTpm2HashMask from 0x%X to 0x%X.\n", __FUNCTION__, Tpm2PcrMask, NewTpm2PcrMask));
+    DEBUG ((DEBUG_ERROR, "%a - Updating PcdTpm2HashMask from 0x%X to 0x%X.\n", __FUNCTION__, Tpm2PcrMask, NewTpm2PcrMask));
     if (NewTpm2PcrMask == 0) {
-      DEBUG ((EFI_D_ERROR, "%a - No viable PCRs supported! Please set a less restrictive value for PcdTpm2HashMask!\n", __FUNCTION__));
+      DEBUG ((DEBUG_ERROR, "%a - No viable PCRs supported! Please set a less restrictive value for PcdTpm2HashMask!\n", __FUNCTION__));
       ASSERT (FALSE);
     }
 
     Status = PcdSet32S (PcdTpm2HashMask, NewTpm2PcrMask);
+    DEBUG ((DEBUG_ERROR, "Set PcdTpm2Hash Mask to 0x%08x\n", NewTpm2PcrMask));
     ASSERT_EFI_ERROR (Status);
   }
 }
@@ -365,7 +382,7 @@ LogHashEvent (
   RetStatus = EFI_SUCCESS;
   for (Index = 0; Index < sizeof(mTcg2EventInfo)/sizeof(mTcg2EventInfo[0]); Index++) {
     if ((SupportedEventLogs & mTcg2EventInfo[Index].LogFormat) != 0) {
-      DEBUG ((EFI_D_INFO, "  LogFormat - 0x%08x\n", mTcg2EventInfo[Index].LogFormat));
+      DEBUG ((DEBUG_INFO, "  LogFormat - 0x%08x\n", mTcg2EventInfo[Index].LogFormat));
       switch (mTcg2EventInfo[Index].LogFormat) {
       case EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2:
         Status = GetDigestFromDigestList (TPM_ALG_SHA1, DigestList, &NewEventHdr->Digest);
@@ -476,7 +493,7 @@ HashLogExtendEvent (
   }
 
   if (Status == EFI_DEVICE_ERROR) {
-    DEBUG ((EFI_D_ERROR, "HashLogExtendEvent - %r. Disable TPM.\n", Status));
+    DEBUG ((DEBUG_ERROR, "HashLogExtendEvent - %r. Disable TPM.\n", Status));
     BuildGuidHob (&gTpmErrorHobGuid,0);
     REPORT_STATUS_CODE (
       EFI_ERROR_CODE | EFI_ERROR_MINOR,
@@ -1011,7 +1028,7 @@ PeimEntryMA (
   }
 
   if (GetFirstGuidHob (&gTpmErrorHobGuid) != NULL) {
-    DEBUG ((EFI_D_ERROR, "TPM2 error!\n"));
+    DEBUG ((DEBUG_ERROR, "TPM2 error!\n"));
     return EFI_DEVICE_ERROR;
   }
 
@@ -1075,7 +1092,7 @@ PeimEntryMA (
       for (PcrIndex = 0; PcrIndex < 8; PcrIndex++) {
         Status = MeasureSeparatorEventWithError (PcrIndex);
         if (EFI_ERROR (Status)) {
-          DEBUG ((EFI_D_ERROR, "Separator Event with Error not Measured. Error!\n"));
+          DEBUG ((DEBUG_ERROR, "Separator Event with Error not Measured. Error!\n"));
         }
       }
     }
@@ -1106,7 +1123,7 @@ PeimEntryMA (
 
 Done:
   if (EFI_ERROR (Status)) {
-    DEBUG ((EFI_D_ERROR, "TPM2 error! Build Hob\n"));
+    DEBUG ((DEBUG_ERROR, "TPM2 error! Build Hob\n"));
     BuildGuidHob (&gTpmErrorHobGuid,0);
     REPORT_STATUS_CODE (
       EFI_ERROR_CODE | EFI_ERROR_MINOR,
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.33.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83355): https://edk2.groups.io/g/devel/message/83355
Mute This Topic: https://groups.io/mt/84674454/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH] Reallocate TPM Active PCRs based on platform support.
Posted by Yao, Jiewen 2 years, 5 months ago
Would you please confirm if you have run CI and got a PASS result?


> -----Original Message-----
> From: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@intel.com>
> Sent: Friday, November 5, 2021 2:07 AM
> To: devel@edk2.groups.io
> Cc: Gonzalez Del Cueto, Rodrigo <rodrigo.gonzalez.del.cueto@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH] Reallocate TPM Active PCRs based on platform support.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3515
> 
> 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.cueto@intel.com>
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> ---
>  SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.c
> |  6 +++++-
>  SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c |
> 6 +++++-
>  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c                                        | 67
> ++++++++++++++++++++++++++++++++++++++++++-------------------------
>  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf                                      |  1 +
>  4 files changed, 53 insertions(+), 27 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..582b9377e5 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
> 
> @@ -253,7 +253,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 (
> @@ -262,52 +262,68 @@ SyncPcrAllocationsAndPcrMask (
>  {
>    EFI_STATUS                        Status;
>    EFI_TCG2_EVENT_ALGORITHM_BITMAP   TpmHashAlgorithmBitmap;
> +  EFI_TCG2_EVENT_ALGORITHM_BITMAP   BiosHashAlgorithmBitmap;
>    UINT32                            TpmActivePcrBanks;
>    UINT32                            NewTpmActivePcrBanks;
>    UINT32                            Tpm2PcrMask;
>    UINT32                            NewTpm2PcrMask;
> 
> -  DEBUG ((EFI_D_ERROR, "SyncPcrAllocationsAndPcrMask!\n"));
> +  DEBUG ((DEBUG_ERROR, "SyncPcrAllocationsAndPcrMask!\n"));
> 
>    //
>    // Determine the current TPM support and the Platform PCR mask.
>    //
>    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;
>    }
> +  DEBUG ((DEBUG_INFO, "Tpm2PcrMask 0x%08x\n", Tpm2PcrMask));
> 
>    //
> -  // 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.
> -  //
> -
> -  //
> -  // 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;
> -
> -    DEBUG ((EFI_D_INFO, "%a - Reallocating PCR banks from 0x%X to 0x%X.\n",
> __FUNCTION__, TpmActivePcrBanks, NewTpmActivePcrBanks));
> +  // * 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 ((EFI_D_ERROR, "%a - No viable PCRs active! Please set a less
> restrictive value for PcdTpm2HashMask!\n", __FUNCTION__));
> +      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)) {
>          //
>          // We can't do much here, but we hope that this doesn't happen.
>          //
> -        DEBUG ((EFI_D_ERROR, "%a - Failed to reallocate PCRs!\n",
> __FUNCTION__));
> +        DEBUG ((DEBUG_ERROR, "%a - Failed to reallocate PCRs!\n",
> __FUNCTION__));
>          ASSERT_EFI_ERROR (Status);
>        }
>        //
> @@ -324,13 +340,14 @@ SyncPcrAllocationsAndPcrMask (
>    if ((Tpm2PcrMask & TpmHashAlgorithmBitmap) != Tpm2PcrMask) {
>      NewTpm2PcrMask = Tpm2PcrMask & TpmHashAlgorithmBitmap;
> 
> -    DEBUG ((EFI_D_INFO, "%a - Updating PcdTpm2HashMask from 0x%X to
> 0x%X.\n", __FUNCTION__, Tpm2PcrMask, NewTpm2PcrMask));
> +    DEBUG ((DEBUG_ERROR, "%a - Updating PcdTpm2HashMask from 0x%X to
> 0x%X.\n", __FUNCTION__, Tpm2PcrMask, NewTpm2PcrMask));
>      if (NewTpm2PcrMask == 0) {
> -      DEBUG ((EFI_D_ERROR, "%a - No viable PCRs supported! Please set a less
> restrictive value for PcdTpm2HashMask!\n", __FUNCTION__));
> +      DEBUG ((DEBUG_ERROR, "%a - No viable PCRs supported! Please set a less
> restrictive value for PcdTpm2HashMask!\n", __FUNCTION__));
>        ASSERT (FALSE);
>      }
> 
>      Status = PcdSet32S (PcdTpm2HashMask, NewTpm2PcrMask);
> +    DEBUG ((DEBUG_ERROR, "Set PcdTpm2Hash Mask to 0x%08x\n",
> NewTpm2PcrMask));
>      ASSERT_EFI_ERROR (Status);
>    }
>  }
> @@ -365,7 +382,7 @@ LogHashEvent (
>    RetStatus = EFI_SUCCESS;
>    for (Index = 0; Index < sizeof(mTcg2EventInfo)/sizeof(mTcg2EventInfo[0]);
> Index++) {
>      if ((SupportedEventLogs & mTcg2EventInfo[Index].LogFormat) != 0) {
> -      DEBUG ((EFI_D_INFO, "  LogFormat - 0x%08x\n",
> mTcg2EventInfo[Index].LogFormat));
> +      DEBUG ((DEBUG_INFO, "  LogFormat - 0x%08x\n",
> mTcg2EventInfo[Index].LogFormat));
>        switch (mTcg2EventInfo[Index].LogFormat) {
>        case EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2:
>          Status = GetDigestFromDigestList (TPM_ALG_SHA1, DigestList,
> &NewEventHdr->Digest);
> @@ -476,7 +493,7 @@ HashLogExtendEvent (
>    }
> 
>    if (Status == EFI_DEVICE_ERROR) {
> -    DEBUG ((EFI_D_ERROR, "HashLogExtendEvent - %r. Disable TPM.\n", Status));
> +    DEBUG ((DEBUG_ERROR, "HashLogExtendEvent - %r. Disable TPM.\n",
> Status));
>      BuildGuidHob (&gTpmErrorHobGuid,0);
>      REPORT_STATUS_CODE (
>        EFI_ERROR_CODE | EFI_ERROR_MINOR,
> @@ -1011,7 +1028,7 @@ PeimEntryMA (
>    }
> 
>    if (GetFirstGuidHob (&gTpmErrorHobGuid) != NULL) {
> -    DEBUG ((EFI_D_ERROR, "TPM2 error!\n"));
> +    DEBUG ((DEBUG_ERROR, "TPM2 error!\n"));
>      return EFI_DEVICE_ERROR;
>    }
> 
> @@ -1075,7 +1092,7 @@ PeimEntryMA (
>        for (PcrIndex = 0; PcrIndex < 8; PcrIndex++) {
>          Status = MeasureSeparatorEventWithError (PcrIndex);
>          if (EFI_ERROR (Status)) {
> -          DEBUG ((EFI_D_ERROR, "Separator Event with Error not Measured.
> Error!\n"));
> +          DEBUG ((DEBUG_ERROR, "Separator Event with Error not Measured.
> Error!\n"));
>          }
>        }
>      }
> @@ -1106,7 +1123,7 @@ PeimEntryMA (
> 
>  Done:
>    if (EFI_ERROR (Status)) {
> -    DEBUG ((EFI_D_ERROR, "TPM2 error! Build Hob\n"));
> +    DEBUG ((DEBUG_ERROR, "TPM2 error! Build Hob\n"));
>      BuildGuidHob (&gTpmErrorHobGuid,0);
>      REPORT_STATUS_CODE (
>        EFI_ERROR_CODE | EFI_ERROR_MINOR,
> 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.33.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83367): https://edk2.groups.io/g/devel/message/83367
Mute This Topic: https://groups.io/mt/84674454/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-