Re: [PATCH v5 9/9] SecurityPkg/TcgPei: Use Migrated FV Info Hob for calculating hash (CVE-2019-11098)

["Wang, Jian J" <jian.j.wang@intel.com>]

Reviewed-by: Jian J Wang <jian.j.wang@intel.com>

Regards,
Jian

> -----Original Message-----
> From: Jiang, Guomin <guomin.jiang@intel.com>
> Sent: Thursday, July 09, 2020 9:57 AM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Zhang, Chao B <chao.b.zhang@intel.com>; Zhang, Qi1 <qi1.zhang@intel.com>;
> Kumar, Rahul1 <rahul1.kumar@intel.com>
> Subject: [PATCH v5 9/9] SecurityPkg/TcgPei: Use Migrated FV Info Hob for
> calculating hash (CVE-2019-11098)
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
> 
> When we allocate pool to save rebased the PEIMs, the address will change
> randomly, therefore the hash will change and result PCR0 change as well.
> To avoid this, we save the raw PEIMs and use it to calculate hash.
> The TcgPei calculate the hash and it use the Migrated FV Info.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Cc: Qi Zhang <qi1.zhang@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
> ---
>  SecurityPkg/Tcg/TcgPei/TcgPei.inf |  1 +
>  SecurityPkg/Tcg/TcgPei/TcgPei.c   | 29 +++++++++++++++++++++++++++--
>  2 files changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/SecurityPkg/Tcg/TcgPei/TcgPei.inf
> b/SecurityPkg/Tcg/TcgPei/TcgPei.inf
> index c0bff6e85e9d..6d1951f8ed65 100644
> --- a/SecurityPkg/Tcg/TcgPei/TcgPei.inf
> +++ b/SecurityPkg/Tcg/TcgPei/TcgPei.inf
> @@ -58,6 +58,7 @@ [Guids]
>    gTpmErrorHobGuid                                                    ## SOMETIMES_PRODUCES
> ## HOB
>    gMeasuredFvHobGuid                                                  ## PRODUCES               ##
> HOB
>    gEfiTpmDeviceInstanceTpm12Guid                                      ## PRODUCES
> ## GUID       # TPM device identifier
> +  gEdkiiMigratedFvInfoGuid                                            ##
> SOMETIMES_CONSUMES     ## HOB
> 
>  [Ppis]
>    gPeiLockPhysicalPresencePpiGuid                                     ##
> SOMETIMES_CONSUMES     ## NOTIFY
> diff --git a/SecurityPkg/Tcg/TcgPei/TcgPei.c b/SecurityPkg/Tcg/TcgPei/TcgPei.c
> index a9a808c9ecf3..9701bfe8715b 100644
> --- a/SecurityPkg/Tcg/TcgPei/TcgPei.c
> +++ b/SecurityPkg/Tcg/TcgPei/TcgPei.c
> @@ -21,6 +21,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <Guid/TcgEventHob.h>
>  #include <Guid/MeasuredFvHob.h>
>  #include <Guid/TpmInstance.h>
> +#include <Guid/MigratedFvInfo.h>
> 
>  #include <Library/DebugLib.h>
>  #include <Library/BaseMemoryLib.h>
> @@ -378,6 +379,10 @@ MeasureFvImage (
>    EFI_STATUS                        Status;
>    EFI_PLATFORM_FIRMWARE_BLOB        FvBlob;
>    TCG_PCR_EVENT_HDR                 TcgEventHdr;
> +  EFI_PHYSICAL_ADDRESS              FvOrgBase;
> +  EFI_PHYSICAL_ADDRESS              FvDataBase;
> +  EFI_PEI_HOB_POINTERS              Hob;
> +  EDKII_MIGRATED_FV_INFO            *MigratedFvInfo;
> 
>    //
>    // Check if it is in Excluded FV list
> @@ -401,10 +406,30 @@ MeasureFvImage (
>      }
>    }
> 
> +  //
> +  // Search the matched migration FV info
> +  //
> +  FvOrgBase  = FvBase;
> +  FvDataBase = FvBase;
> +  Hob.Raw  = GetFirstGuidHob (&gEdkiiMigratedFvInfoGuid);
> +  while (Hob.Raw != NULL) {
> +    MigratedFvInfo = GET_GUID_HOB_DATA (Hob);
> +    if ((MigratedFvInfo->FvNewBase == (UINT32) FvBase) && (MigratedFvInfo-
> >FvLength == (UINT32) FvLength)) {
> +      //
> +      // Found the migrated FV info
> +      //
> +      FvOrgBase  = (EFI_PHYSICAL_ADDRESS) (UINTN) MigratedFvInfo-
> >FvOrgBase;
> +      FvDataBase = (EFI_PHYSICAL_ADDRESS) (UINTN) MigratedFvInfo-
> >FvDataBase;
> +      break;
> +    }
> +    Hob.Raw = GET_NEXT_HOB (Hob);
> +    Hob.Raw = GetNextGuidHob (&gEdkiiMigratedFvInfoGuid, Hob.Raw);
> +  }
> +
>    //
>    // Measure and record the FV to the TPM
>    //
> -  FvBlob.BlobBase   = FvBase;
> +  FvBlob.BlobBase   = FvOrgBase;
>    FvBlob.BlobLength = FvLength;
> 
>    DEBUG ((DEBUG_INFO, "The FV which is measured by TcgPei starts at: 0x%x\n",
> FvBlob.BlobBase));
> @@ -416,7 +441,7 @@ MeasureFvImage (
> 
>    Status = HashLogExtendEvent (
>               (EFI_PEI_SERVICES **) GetPeiServicesTablePointer(),
> -             (UINT8*) (UINTN) FvBlob.BlobBase,
> +             (UINT8*) (UINTN) FvDataBase,
>               (UINTN) FvBlob.BlobLength,
>               &TcgEventHdr,
>               (UINT8*) &FvBlob
> --
> 2.25.1.windows.1