[v1] SecurityPkg/PhysicalPresenceLib: Reject illegal PCR bank allocation

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

Posted by Zhang, Chao B 6 years, 11 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..5ece8e5 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;
+      }
+
       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/PhysicalPresenceLib: Reject illegal PCR bank allocation

Posted by Long, Qin 6 years, 11 months ago

Reviewed-by: Long Qin <qin.long@intel.com>


Best Regards & Thanks,
LONG, Qin

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Zhang, Chao B
Sent: Monday, January 15, 2018 3:29 PM
To: edk2-devel@lists.01.org
Cc: Yao, Jiewen <jiewen.yao@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>; Long, Qin <qin.long@intel.com>
Subject: [edk2] [PATCH] SecurityPkg/PhysicalPresenceLib: Reject illegal PCR bank allocation

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..5ece8e5 100644
--- a/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.c
+++ b/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPres
+++ enceLib.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;
+      }
+
       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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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

Posted by Yao, Jiewen 6 years, 11 months ago

Reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: Zhang, Chao B
> Sent: Monday, January 15, 2018 3:29 PM
> To: edk2-devel@lists.01.org
> Cc: Long, Qin <qin.long@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Zhang, Chao B <chao.b.zhang@intel.com>
> Subject: [PATCH] SecurityPkg/PhysicalPresenceLib: Reject illegal PCR bank
> allocation
> 
> 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..5ece8e5 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;
> +      }
> +
>        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