[edk2] [PATCH] SecurityPkg/DxePhysicalPresenceLib: Reject illegal PCR bank allocation

Zhang, Chao B posted 1 patch 6 years, 9 months ago
Failed in applying to current master (apply log)
.../DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.c  | 12 ++++++++++++
1 file changed, 12 insertions(+)
[edk2] [PATCH] SecurityPkg/DxePhysicalPresenceLib: Reject illegal PCR bank allocation
Posted by Zhang, Chao B 6 years, 9 months ago
According to TCG PP1.3 spec, error PCR bank allocation input should be rejected by
Physical Presence. Firmware has to ensure that at least one PCR banks is active.

Cc: Long Qin <qin.long@intel.com>
Cc: Yao Jiewen <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
---
 .../DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.c  | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.c b/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.c
index 5bf95a1..830266b 100644
--- a/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.c
+++ b/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.c
@@ -186,6 +186,18 @@ Tcg2ExecutePhysicalPresence (
     case TCG2_PHYSICAL_PRESENCE_SET_PCR_BANKS:
       Status = Tpm2GetCapabilitySupportedAndActivePcrs (&TpmHashAlgorithmBitmap, &ActivePcrBanks);
       ASSERT_EFI_ERROR (Status);
+
+      //
+      // PP spec requirements:
+      //    Firmware should check that all requested (set) hashing algorithms are supported with respective PCR banks. 
+      //    Firmware has to ensure that at least one PCR banks is active
+      // If not, an error is returned and no action is taken
+      //
+      if (CommandParameter == 0 || (CommandParameter & (~TpmHashAlgorithmBitmap)) != 0) {
+        DEBUG((DEBUG_ERROR, "PCR banks %x to allocate are not supported by TPM. Skip operation\n", CommandParameter));
+        return TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE
+      }
+      DEBUG((DEBUG_ERROR, "zhangchao TpmHashAlgorithmBitmap %x CommandParameter %x\n", TpmHashAlgorithmBitmap, CommandParameter));
       Status = Tpm2PcrAllocateBanks (PlatformAuth, TpmHashAlgorithmBitmap, CommandParameter);
       if (EFI_ERROR (Status)) {
         return TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE;
-- 
1.9.5.msysgit.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] SecurityPkg/DxePhysicalPresenceLib: Reject illegal PCR bank allocation
Posted by Bill Paul 6 years, 9 months ago
Of all the gin joints in all the towns in all the world, Zhang, Chao B had to 
walk into mine at 20:53 on Wednesday 24 January 2018 and say:

> According to TCG PP1.3 spec, error PCR bank allocation input should be
> rejected by Physical Presence. Firmware has to ensure that at least one
> PCR banks is active.
> 
> Cc: Long Qin <qin.long@intel.com>
> Cc: Yao Jiewen <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
> ---
>  .../DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.c  | 12
> ++++++++++++ 1 file changed, 12 insertions(+)
> 
> diff --git
> a/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLi
> b.c
> b/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLi
> b.c index 5bf95a1..830266b 100644
> ---
> a/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLi
> b.c +++
> b/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLi
> b.c @@ -186,6 +186,18 @@ Tcg2ExecutePhysicalPresence (
>      case TCG2_PHYSICAL_PRESENCE_SET_PCR_BANKS:
>        Status = Tpm2GetCapabilitySupportedAndActivePcrs
> (&TpmHashAlgorithmBitmap, &ActivePcrBanks); ASSERT_EFI_ERROR (Status);
> +
> +      //
> +      // PP spec requirements:
> +      //    Firmware should check that all requested (set) hashing
> algorithms are supported with respective PCR banks. +      //    Firmware
> has to ensure that at least one PCR banks is active +      // If not, an
> error is returned and no action is taken
> +      //
> +      if (CommandParameter == 0 || (CommandParameter &
> (~TpmHashAlgorithmBitmap)) != 0) { +        DEBUG((DEBUG_ERROR, "PCR banks
> %x to allocate are not supported by TPM. Skip operation\n",
> CommandParameter)); +        return TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE
> +      }
> +      DEBUG((DEBUG_ERROR, "zhangchao TpmHashAlgorithmBitmap %x

Was it your intention to have the debug error message string identify you by 
name? :)

-Bill

> CommandParameter %x\n", TpmHashAlgorithmBitmap, CommandParameter)); Status
> = Tpm2PcrAllocateBanks (PlatformAuth, TpmHashAlgorithmBitmap,
> CommandParameter); if (EFI_ERROR (Status)) {
>          return TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE;
-- 
=============================================================================
-Bill Paul            (510) 749-2329 | Senior Member of Technical Staff,
                 wpaul@windriver.com | Master of Unix-Fu - Wind River Systems
=============================================================================
   "I put a dollar in a change machine. Nothing changed." - George Carlin
=============================================================================
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel