public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Yao, Jiewen" <jiewen.yao@intel.com>
To: "Zeng, Star" <star.zeng@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Zhang, Chao B" <chao.b.zhang@intel.com>
Subject: Re: [PATCH] SecurityPkg HashLibRouter: Avoid incorrect PcdTcg2HashAlgorithmBitmap
Date: Thu, 26 Jan 2017 02:29:16 +0000	[thread overview]
Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503A8E5AD6@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <1485252410-17656-1-git-send-email-star.zeng@intel.com>

Good feature to catch mis-configuration.

Reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: Zeng, Star
> Sent: Tuesday, January 24, 2017 6:07 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Zhang, Chao B <chao.b.zhang@intel.com>
> Subject: [PATCH] SecurityPkg HashLibRouter: Avoid incorrect
> PcdTcg2HashAlgorithmBitmap
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=244
> 
> Currently, when software HashLib (HashLibBaseCryptoRouter) and related
> HashInstanceLib instances are used, PcdTcg2HashAlgorithmBitmap is
> expected to be configured to 0 in platform dsc.
> But PcdTcg2HashAlgorithmBitmap has default value 0xFFFFFFFF in
> SecurityPkg.dec, and some platforms forget to configure it to 0 or
> still configure it to 0xFFFFFFFF in platform dsc, that will make final
> PcdTcg2HashAlgorithmBitmap value incorrect.
> 
> This patch is to add CONSTRUCTOR in HashLib (HashLibBaseCryptoRouter)
> and PcdTcg2HashAlgorithmBitmap will be set to 0 in the CONSTRUCTOR.
> 
> Current HASH_LIB_PEI_ROUTER_GUID HOB created in
> HashLibBaseCryptoRouterPei is shared between modules that links
> HashLibBaseCryptoRouterPei.
> To avoid mutual interference, separated HASH_LIB_PEI_ROUTER_GUID HOBs
> with gEfiCallerIdGuid Identifier will be created for those modules.
> 
> This patch is also to add check in HashLib (HashLibBaseCryptoRouter)
> for the mismatch of supported HashMask between modules that may link
> different HashInstanceLib instances, warning will be reported if
> mismatch is found.
> 
> Cc: Yao Jiewen <jiewen.yao@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  .../HashLibBaseCryptoRouterDxe.c                   |  80 ++++++++-
>  .../HashLibBaseCryptoRouterDxe.inf                 |   3 +-
>  .../HashLibBaseCryptoRouterPei.c                   | 190
> +++++++++++++++++----
>  .../HashLibBaseCryptoRouterPei.inf                 |   8 +-
>  SecurityPkg/SecurityPkg.dec                        |   4 +
>  SecurityPkg/SecurityPkg.uni                        |   8 +-
>  6 files changed, 254 insertions(+), 39 deletions(-)
> 
> diff --git
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.
> c
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.
> c
> index 3250c3a01a0c..4775cfee2d7a 100644
> ---
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.
> c
> +++
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.
> c
> @@ -3,7 +3,7 @@
>    hash handler registerd, such as SHA1, SHA256.
>    Platform can use PcdTpm2HashMask to mask some hash engines.
> 
> -Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved. <BR>
> +Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved. <BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be found
> at
> @@ -28,6 +28,30 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  HASH_INTERFACE   mHashInterface[HASH_COUNT] = {{{0}, NULL, NULL,
> NULL}};
>  UINTN            mHashInterfaceCount = 0;
> 
> +UINT32           mSupportedHashMaskLast = 0;
> +UINT32           mSupportedHashMaskCurrent = 0;
> +
> +/**
> +  Check mismatch of supported HashMask between modules
> +  that may link different HashInstanceLib instances.
> +
> +**/
> +VOID
> +CheckSupportedHashMaskMismatch (
> +  VOID
> +  )
> +{
> +  if (mSupportedHashMaskCurrent != mSupportedHashMaskLast) {
> +    DEBUG ((
> +      DEBUG_WARN,
> +      "WARNING: There is mismatch of supported HashMask (0x%x - 0x%x)
> between modules\n",
> +      mSupportedHashMaskCurrent,
> +      mSupportedHashMaskLast
> +      ));
> +    DEBUG ((DEBUG_WARN, "that are linking different HashInstanceLib
> instances!\n"));
> +  }
> +}
> +
>  /**
>    Start hash sequence.
> 
> @@ -50,6 +74,8 @@ HashStart (
>      return EFI_UNSUPPORTED;
>    }
> 
> +  CheckSupportedHashMaskMismatch ();
> +
>    HashCtx = AllocatePool (sizeof(*HashCtx) * mHashInterfaceCount);
>    ASSERT (HashCtx != NULL);
> 
> @@ -90,6 +116,8 @@ HashUpdate (
>      return EFI_UNSUPPORTED;
>    }
> 
> +  CheckSupportedHashMaskMismatch ();
> +
>    HashCtx = (HASH_HANDLE *)HashHandle;
> 
>    for (Index = 0; Index < mHashInterfaceCount; Index++) {
> @@ -133,6 +161,8 @@ HashCompleteAndExtend (
>      return EFI_UNSUPPORTED;
>    }
> 
> +  CheckSupportedHashMaskMismatch ();
> +
>    HashCtx = (HASH_HANDLE *)HashHandle;
>    ZeroMem (DigestList, sizeof(*DigestList));
> 
> @@ -180,6 +210,8 @@ HashAndExtend (
>      return EFI_UNSUPPORTED;
>    }
> 
> +  CheckSupportedHashMaskMismatch ();
> +
>    HashStart (&HashHandle);
>    HashUpdate (HashHandle, DataToHash, DataToHashLen);
>    Status = HashCompleteAndExtend (HashHandle, PcrIndex, NULL, 0,
> DigestList);
> @@ -204,7 +236,6 @@ RegisterHashInterfaceLib (
>  {
>    UINTN              Index;
>    UINT32             HashMask;
> -  UINT32             BiosSupportedHashMask;
>    EFI_STATUS         Status;
> 
>    //
> @@ -218,21 +249,58 @@ RegisterHashInterfaceLib (
>    if (mHashInterfaceCount >=
> sizeof(mHashInterface)/sizeof(mHashInterface[0])) {
>      return EFI_OUT_OF_RESOURCES;
>    }
> -  BiosSupportedHashMask = PcdGet32 (PcdTcg2HashAlgorithmBitmap);
> -  Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, BiosSupportedHashMask |
> HashMask);
> -  ASSERT_EFI_ERROR (Status);
> 
>    //
>    // Check duplication
>    //
>    for (Index = 0; Index < mHashInterfaceCount; Index++) {
>      if (CompareGuid (&mHashInterface[Index].HashGuid,
> &HashInterface->HashGuid)) {
> +      DEBUG ((DEBUG_ERROR, "Hash Interface (%g) has been registered\n"));
>        return EFI_ALREADY_STARTED;
>      }
>    }
> 
> +  //
> +  // Record hash algorithm bitmap of CURRENT module which consumes
> HashLib.
> +  //
> +  mSupportedHashMaskCurrent = PcdGet32 (PcdTcg2HashAlgorithmBitmap) |
> HashMask;
> +  Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap,
> mSupportedHashMaskCurrent);
> +  ASSERT_EFI_ERROR (Status);
> +
>    CopyMem (&mHashInterface[mHashInterfaceCount], HashInterface,
> sizeof(*HashInterface));
>    mHashInterfaceCount ++;
> 
>    return EFI_SUCCESS;
> -}
> \ No newline at end of file
> +}
> +
> +/**
> +  The constructor function of HashLibBaseCryptoRouterDxe.
> +
> +  @param  ImageHandle   The firmware allocated handle for the EFI image.
> +  @param  SystemTable   A pointer to the EFI System Table.
> +
> +  @retval EFI_SUCCESS   The constructor executed correctly.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +HashLibBaseCryptoRouterDxeConstructor (
> +  IN EFI_HANDLE        ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  EFI_STATUS    Status;
> +
> +  //
> +  // Record hash algorithm bitmap of LAST module which also consumes
> HashLib.
> +  //
> +  mSupportedHashMaskLast = PcdGet32 (PcdTcg2HashAlgorithmBitmap);
> +
> +  //
> +  // Set PcdTcg2HashAlgorithmBitmap to 0 in CONSTRUCTOR for CURRENT
> module.
> +  //
> +  Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, 0);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return EFI_SUCCESS;
> +}
> diff --git
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.i
> nf
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.i
> nf
> index c4e40d47d998..6e660d4f14f8 100644
> ---
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.i
> nf
> +++
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.i
> nf
> @@ -5,7 +5,7 @@
>  #  hash handler registered, such as SHA1, SHA256. Platform can use
> PcdTpm2HashMask to
>  #  mask some hash engines.
>  #
> -# Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.<BR>
>  # This program and the accompanying materials
>  # are licensed and made available under the terms and conditions of the BSD
> License
>  # which accompanies this distribution. The full text of the license may be found
> at
> @@ -23,6 +23,7 @@ [Defines]
>    MODULE_TYPE                    = DXE_DRIVER
>    VERSION_STRING                 = 1.0
>    LIBRARY_CLASS                  = HashLib|DXE_DRIVER
> DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER
> UEFI_APPLICATION UEFI_DRIVER
> +  CONSTRUCTOR                    =
> HashLibBaseCryptoRouterDxeConstructor
> 
>  #
>  # The following information is for reference only and not required by the build
> tools.
> diff --git
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> index a4fc0c6595d8..3ed56f337942 100644
> ---
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> +++
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> @@ -3,7 +3,7 @@
>    hash handler registerd, such as SHA1, SHA256.
>    Platform can use PcdTpm2HashMask to mask some hash engines.
> 
> -Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved. <BR>
> +Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved. <BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be found
> at
> @@ -23,6 +23,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
>  #include <Library/PcdLib.h>
>  #include <Library/HobLib.h>
>  #include <Library/HashLib.h>
> +#include <Guid/ZeroGuid.h>
> 
>  #include "HashLibBaseCryptoRouterCommon.h"
> 
> @@ -32,27 +33,95 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  EFI_GUID mHashLibPeiRouterGuid = HASH_LIB_PEI_ROUTER_GUID;
> 
>  typedef struct {
> +  //
> +  // If gZeroGuid, SupportedHashMask is 0 for FIRST module which consumes
> HashLib
> +  //   or the hash algorithm bitmap of LAST module which consumes HashLib.
> +  //   HashInterfaceCount and HashInterface are all 0.
> +  // If gEfiCallerIdGuid, HashInterfaceCount, HashInterface and
> SupportedHashMask
> +  //   are the hash interface information of CURRENT module which consumes
> HashLib.
> +  //
> +  EFI_GUID         Identifier;
>    UINTN            HashInterfaceCount;
>    HASH_INTERFACE   HashInterface[HASH_COUNT];
> +  UINT32           SupportedHashMask;
>  } HASH_INTERFACE_HOB;
> 
>  /**
> -  This function get hash interface.
> +  This function gets hash interface hob.
> +
> +  @param Identifier    Identifier to get hash interface hob.
> +
> +  @retval hash interface hob.
> +**/
> +HASH_INTERFACE_HOB *
> +InternalGetHashInterfaceHob (
> +  EFI_GUID      *Identifier
> +  )
> +{
> +  EFI_PEI_HOB_POINTERS  Hob;
> +  HASH_INTERFACE_HOB    *HashInterfaceHob;
> +
> +  Hob.Raw = GetFirstGuidHob (&mHashLibPeiRouterGuid);
> +  while (Hob.Raw != NULL) {
> +    HashInterfaceHob = GET_GUID_HOB_DATA (Hob);
> +    if (CompareGuid (&HashInterfaceHob->Identifier, Identifier)) {
> +      //
> +      // Found the matched one.
> +      //
> +      return HashInterfaceHob;
> +    }
> +    Hob.Raw = GET_NEXT_HOB (Hob);
> +    Hob.Raw = GetNextGuidHob (&mHashLibPeiRouterGuid, Hob.Raw);
> +  }
> +  return NULL;
> +}
> +
> +/**
> +  This function creates hash interface hob.
> 
> -  @retval hash interface.
> +  @param Identifier    Identifier to create hash interface hob.
> +
> +  @retval hash interface hob.
>  **/
>  HASH_INTERFACE_HOB *
> -InternalGetHashInterface (
> -  VOID
> +InternalCreateHashInterfaceHob (
> +  EFI_GUID      *Identifier
>    )
>  {
> -  EFI_HOB_GUID_TYPE *Hob;
> +  HASH_INTERFACE_HOB LocalHashInterfaceHob;
> +
> +  ZeroMem (&LocalHashInterfaceHob, sizeof(LocalHashInterfaceHob));
> +  CopyGuid (&LocalHashInterfaceHob.Identifier, Identifier);
> +  return BuildGuidDataHob (&mHashLibPeiRouterGuid,
> &LocalHashInterfaceHob, sizeof(LocalHashInterfaceHob));
> +}
> 
> -  Hob = GetFirstGuidHob (&mHashLibPeiRouterGuid);
> -  if (Hob == NULL) {
> -    return NULL;
> +/**
> +  Check mismatch of supported HashMask between modules
> +  that may link different HashInstanceLib instances.
> +
> +  @param HashInterfaceHobCurrent    Pointer to hash interface hob for
> CURRENT module.
> +
> +**/
> +VOID
> +CheckSupportedHashMaskMismatch (
> +  IN HASH_INTERFACE_HOB *HashInterfaceHobCurrent
> +  )
> +{
> +  HASH_INTERFACE_HOB    *HashInterfaceHobLast;
> +
> +  HashInterfaceHobLast = InternalGetHashInterfaceHob (&gZeroGuid);
> +  ASSERT (HashInterfaceHobLast != NULL);
> +
> +  if ((HashInterfaceHobLast->SupportedHashMask != 0) &&
> +      (HashInterfaceHobCurrent->SupportedHashMask !=
> HashInterfaceHobLast->SupportedHashMask)) {
> +    DEBUG ((
> +      DEBUG_WARN,
> +      "WARNING: There is mismatch of supported HashMask (0x%x - 0x%x)
> between modules\n",
> +      HashInterfaceHobCurrent->SupportedHashMask,
> +      HashInterfaceHobLast->SupportedHashMask
> +      ));
> +    DEBUG ((DEBUG_WARN, "that are linking different HashInstanceLib
> instances!\n"));
>    }
> -  return (HASH_INTERFACE_HOB *)(Hob + 1);
>  }
> 
>  /**
> @@ -74,7 +143,7 @@ HashStart (
>    UINTN              Index;
>    UINT32             HashMask;
> 
> -  HashInterfaceHob = InternalGetHashInterface ();
> +  HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
>    if (HashInterfaceHob == NULL) {
>      return EFI_UNSUPPORTED;
>    }
> @@ -83,6 +152,8 @@ HashStart (
>      return EFI_UNSUPPORTED;
>    }
> 
> +  CheckSupportedHashMaskMismatch (HashInterfaceHob);
> +
>    HashCtx = AllocatePool (sizeof(*HashCtx) *
> HashInterfaceHob->HashInterfaceCount);
>    ASSERT (HashCtx != NULL);
> 
> @@ -120,7 +191,7 @@ HashUpdate (
>    UINTN              Index;
>    UINT32             HashMask;
> 
> -  HashInterfaceHob = InternalGetHashInterface ();
> +  HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
>    if (HashInterfaceHob == NULL) {
>      return EFI_UNSUPPORTED;
>    }
> @@ -129,6 +200,8 @@ HashUpdate (
>      return EFI_UNSUPPORTED;
>    }
> 
> +  CheckSupportedHashMaskMismatch (HashInterfaceHob);
> +
>    HashCtx = (HASH_HANDLE *)HashHandle;
> 
>    for (Index = 0; Index < HashInterfaceHob->HashInterfaceCount; Index++) {
> @@ -169,7 +242,7 @@ HashCompleteAndExtend (
>    EFI_STATUS         Status;
>    UINT32             HashMask;
> 
> -  HashInterfaceHob = InternalGetHashInterface ();
> +  HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
>    if (HashInterfaceHob == NULL) {
>      return EFI_UNSUPPORTED;
>    }
> @@ -178,6 +251,8 @@ HashCompleteAndExtend (
>      return EFI_UNSUPPORTED;
>    }
> 
> +  CheckSupportedHashMaskMismatch (HashInterfaceHob);
> +
>    HashCtx = (HASH_HANDLE *)HashHandle;
>    ZeroMem (DigestList, sizeof(*DigestList));
> 
> @@ -222,7 +297,7 @@ HashAndExtend (
>    HASH_HANDLE        HashHandle;
>    EFI_STATUS         Status;
> 
> -  HashInterfaceHob = InternalGetHashInterface ();
> +  HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
>    if (HashInterfaceHob == NULL) {
>      return EFI_UNSUPPORTED;
>    }
> @@ -231,6 +306,8 @@ HashAndExtend (
>      return EFI_UNSUPPORTED;
>    }
> 
> +  CheckSupportedHashMaskMismatch (HashInterfaceHob);
> +
>    HashStart (&HashHandle);
>    HashUpdate (HashHandle, DataToHash, DataToHashLen);
>    Status = HashCompleteAndExtend (HashHandle, PcrIndex, NULL, 0,
> DigestList);
> @@ -255,9 +332,7 @@ RegisterHashInterfaceLib (
>  {
>    UINTN              Index;
>    HASH_INTERFACE_HOB *HashInterfaceHob;
> -  HASH_INTERFACE_HOB LocalHashInterfaceHob;
>    UINT32             HashMask;
> -  UINT32             BiosSupportedHashMask;
>    EFI_STATUS         Status;
> 
>    //
> @@ -268,10 +343,9 @@ RegisterHashInterfaceLib (
>      return EFI_UNSUPPORTED;
>    }
> 
> -  HashInterfaceHob = InternalGetHashInterface ();
> +  HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
>    if (HashInterfaceHob == NULL) {
> -    ZeroMem (&LocalHashInterfaceHob, sizeof(LocalHashInterfaceHob));
> -    HashInterfaceHob = BuildGuidDataHob (&mHashLibPeiRouterGuid,
> &LocalHashInterfaceHob, sizeof(LocalHashInterfaceHob));
> +    HashInterfaceHob = InternalCreateHashInterfaceHob (&gEfiCallerIdGuid);
>      if (HashInterfaceHob == NULL) {
>        return EFI_OUT_OF_RESOURCES;
>      }
> @@ -280,26 +354,84 @@ RegisterHashInterfaceLib (
>    if (HashInterfaceHob->HashInterfaceCount >= HASH_COUNT) {
>      return EFI_OUT_OF_RESOURCES;
>    }
> -  BiosSupportedHashMask = PcdGet32 (PcdTcg2HashAlgorithmBitmap);
> -  Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, BiosSupportedHashMask |
> HashMask);
> -  ASSERT_EFI_ERROR (Status);
> 
>    //
>    // Check duplication
>    //
>    for (Index = 0; Index < HashInterfaceHob->HashInterfaceCount; Index++) {
>      if (CompareGuid (&HashInterfaceHob->HashInterface[Index].HashGuid,
> &HashInterface->HashGuid)) {
> -      //
> -      // In PEI phase, there will be shadow driver dispatched again.
> -      //
> -      DEBUG ((EFI_D_INFO, "RegisterHashInterfaceLib - Override\n"));
> -      CopyMem (&HashInterfaceHob->HashInterface[Index], HashInterface,
> sizeof(*HashInterface));
> -      return EFI_SUCCESS;
> +      DEBUG ((DEBUG_ERROR, "Hash Interface (%g) has been registered\n"));
> +      return EFI_ALREADY_STARTED;
>      }
>    }
> 
> +  //
> +  // Record hash algorithm bitmap of CURRENT module which consumes
> HashLib.
> +  //
> +  HashInterfaceHob->SupportedHashMask = PcdGet32
> (PcdTcg2HashAlgorithmBitmap) | HashMask;
> +  Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap,
> HashInterfaceHob->SupportedHashMask);
> +  ASSERT_EFI_ERROR (Status);
> +
>    CopyMem
> (&HashInterfaceHob->HashInterface[HashInterfaceHob->HashInterfaceCount],
> HashInterface, sizeof(*HashInterface));
>    HashInterfaceHob->HashInterfaceCount ++;
> 
>    return EFI_SUCCESS;
> -}
> \ No newline at end of file
> +}
> +
> +/**
> +  The constructor function of HashLibBaseCryptoRouterPei.
> +
> +  @param  FileHandle   The handle of FFS header the loaded driver.
> +  @param  PeiServices  The pointer to the PEI services.
> +
> +  @retval EFI_SUCCESS           The constructor executes successfully.
> +  @retval EFI_OUT_OF_RESOURCES  There is no enough resource for the
> constructor.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +HashLibBaseCryptoRouterPeiConstructor (
> +  IN EFI_PEI_FILE_HANDLE        FileHandle,
> +  IN CONST EFI_PEI_SERVICES     **PeiServices
> +  )
> +{
> +  EFI_STATUS            Status;
> +  HASH_INTERFACE_HOB    *HashInterfaceHob;
> +
> +  HashInterfaceHob = InternalGetHashInterfaceHob (&gZeroGuid);
> +  if (HashInterfaceHob == NULL) {
> +    //
> +    // No HOB with gZeroGuid Identifier has been created,
> +    // this is FIRST module which consumes HashLib.
> +    // Create the HOB with gZeroGuid Identifier.
> +    //
> +    HashInterfaceHob = InternalCreateHashInterfaceHob (&gZeroGuid);
> +    if (HashInterfaceHob == NULL) {
> +      return EFI_OUT_OF_RESOURCES;
> +    }
> +  } else {
> +    //
> +    // Record hash algorithm bitmap of LAST module which also consumes
> HashLib.
> +    //
> +    HashInterfaceHob->SupportedHashMask = PcdGet32
> (PcdTcg2HashAlgorithmBitmap);
> +  }
> +
> +  HashInterfaceHob = InternalGetHashInterfaceHob (&gEfiCallerIdGuid);
> +  if (HashInterfaceHob != NULL) {
> +    //
> +    // In PEI phase, some modules may call RegisterForShadow and will be
> +    // shadowed and executed again after memory is discovered.
> +    // This is the second execution of this module, clear the hash interface
> +    // information registered at its first execution.
> +    //
> +    ZeroMem (&HashInterfaceHob->HashInterface, sizeof (*HashInterfaceHob)
> - sizeof (EFI_GUID));
> +  }
> +
> +  //
> +  // Set PcdTcg2HashAlgorithmBitmap to 0 in CONSTRUCTOR for CURRENT
> module.
> +  //
> +  Status = PcdSet32S (PcdTcg2HashAlgorithmBitmap, 0);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return EFI_SUCCESS;
> +}
> diff --git
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.i
> nf
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.i
> nf
> index f5ca5d4635d8..eebf90e2ef40 100644
> ---
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.i
> nf
> +++
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.i
> nf
> @@ -5,7 +5,7 @@
>  #  hash handler registered, such as SHA1, SHA256. Platform can use
> PcdTpm2HashMask to
>  #  mask some hash engines.
>  #
> -# Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.<BR>
>  # This program and the accompanying materials
>  # are licensed and made available under the terms and conditions of the BSD
> License
>  # which accompanies this distribution. The full text of the license may be found
> at
> @@ -23,6 +23,7 @@ [Defines]
>    MODULE_TYPE                    = PEIM
>    VERSION_STRING                 = 1.0
>    LIBRARY_CLASS                  = HashLib|PEIM
> +  CONSTRUCTOR                    =
> HashLibBaseCryptoRouterPeiConstructor
> 
>  #
>  # The following information is for reference only and not required by the build
> tools.
> @@ -38,6 +39,7 @@ [Sources]
>  [Packages]
>    MdePkg/MdePkg.dec
>    SecurityPkg/SecurityPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> 
>  [LibraryClasses]
>    BaseLib
> @@ -48,6 +50,10 @@ [LibraryClasses]
>    PcdLib
>    HobLib
> 
> +[Guids]
> +  ## CONSUMES   ## GUID
> +  gZeroGuid
> +
>  [Pcd]
>    gEfiSecurityPkgTokenSpaceGuid.PcdTpm2HashMask             ##
> CONSUMES
>    ## SOMETIMES_CONSUMES
> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> index 0c64d25b0cbc..88154611ae9c 100644
> --- a/SecurityPkg/SecurityPkg.dec
> +++ b/SecurityPkg/SecurityPkg.dec
> @@ -458,6 +458,10 @@ [PcdsDynamic, PcdsDynamicEx]
>    ## This PCD indicated final BIOS supported Hash mask.
>    #    Bios may choose to register a subset of PcdTpm2HashMask.
>    #    So this PCD is final value of how many hash algo is extended to PCR.
> +  # If software HashLib(HashLibBaseCryptoRouter) solution is chosen, this PCD
> +  # has no need to be configured in platform dsc and will be set to correct
> +  # value by the HashLib instance according to the HashInstanceLib instances
> +  # linked, and the value of this PCD should be got in module entrypoint.
>    # @Prompt Hash Algorithm bitmap.
> 
> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2HashAlgorithmBitmap|0xFFFFFFFF|UINT
> 32|0x00010016
> 
> diff --git a/SecurityPkg/SecurityPkg.uni b/SecurityPkg/SecurityPkg.uni
> index 17d36c0d6014..126351624056 100644
> --- a/SecurityPkg/SecurityPkg.uni
> +++ b/SecurityPkg/SecurityPkg.uni
> @@ -191,7 +191,11 @@
> 
>  #string
> STR_gEfiSecurityPkgTokenSpaceGuid_PcdTcg2HashAlgorithmBitmap_HELP
> #language en-US "This PCD indicated final BIOS supported Hash mask.\n"
> 
> "Bios may choose to register a subset of PcdTpm2HashMask.\n"
> -
> "So this PCD is final value of how many hash algo is extended to PCR."
> +
> "So this PCD is final value of how many hash algo is extended to PCR.\n"
> +
> "If software HashLib(HashLibBaseCryptoRouter) solution is chosen, this PCD\n"
> +
> "has no need to be configured in platform dsc and will be set to correct\n"
> +
> "value by the HashLib instance according to the HashInstanceLib instances\n"
> +
> "linked, and the value of this PCD should be got in module entrypoint."
> 
>  #string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTcgLogAreaMinLen_PROMPT
> #language en-US "Minimum length(in bytes) of the system preboot TCG event
> log area(LAML)."
> 
> @@ -233,4 +237,4 @@
>  #string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTpm2AcpiTableRev_HELP
> #language en-US "This PCD defines initial revision of TPM2 ACPI table\n"
> 
> "To support configuring from setup page, this PCD can be DynamicHii type and
> map to a setup option.<BR>\n"
> 
> "For example, map to TCG2_VERSION.Tpm2AcpiTableRev to be configured by
> Tcg2ConfigDxe driver.<BR>\n"
> -
> "gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L\"TCG2_VERSION\"|gT
> cg2ConfigFormSetGuid|0x8|3|NV,BS<BR>"
> \ No newline at end of file
> +
> "gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L\"TCG2_VERSION\"|gT
> cg2ConfigFormSetGuid|0x8|3|NV,BS<BR>"
> --
> 2.7.0.windows.1



  reply	other threads:[~2017-01-26  2:29 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-24 10:06 [PATCH] SecurityPkg HashLibRouter: Avoid incorrect PcdTcg2HashAlgorithmBitmap Star Zeng
2017-01-26  2:29 ` Yao, Jiewen [this message]
2017-01-26  5:14 ` Zhang, Chao B

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=74D8A39837DF1E4DA445A8C0B3885C503A8E5AD6@shsmsx102.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox