Re: [PATCH v2 3/7] SecurityPkg/Tcg: Add TcgPpi

["Yao, Jiewen" <jiewen.yao@intel.com>]

Hi Qi
Per the conversation between Bret Barkelew and I, we should install the TCG_PPI *before* any measurement.
Please move it earlier.

Thank you
Yao Jiewen

> -----Original Message-----
> From: Zhang, Qi1 <qi1.zhang@intel.com>
> Sent: Thursday, July 16, 2020 3:44 PM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Chao Zhang <chao.b.zhang@intel.com>; Zhang, Qi1 <qi1.zhang@intel.com>;
> Kumar, Rahul1 <rahul1.kumar@intel.com>
> Subject: [PATCH v2 3/7] SecurityPkg/Tcg: Add TcgPpi
> 
> From: Jiewen Yao <jiewen.yao@intel.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2841
> 
> 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: Jiewen Yao <jiewen.yao@intel.com>
> ---
>  SecurityPkg/Tcg/TcgPei/TcgPei.c   | 61 ++++++++++++++++++++++++++++---
>  SecurityPkg/Tcg/TcgPei/TcgPei.inf |  3 +-
>  2 files changed, 58 insertions(+), 6 deletions(-)
> 
> diff --git a/SecurityPkg/Tcg/TcgPei/TcgPei.c b/SecurityPkg/Tcg/TcgPei/TcgPei.c
> index a9a808c9ec..2533388849 100644
> --- a/SecurityPkg/Tcg/TcgPei/TcgPei.c
> +++ b/SecurityPkg/Tcg/TcgPei/TcgPei.c
> @@ -1,7 +1,7 @@
>  /** @file
> 
>    Initialize TPM device and measure FVs before handing off control to DXE.
> 
> 
> 
> -Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.<BR>
> 
> +Copyright (c) 2005 - 2020, Intel Corporation. All rights reserved.<BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> @@ -17,6 +17,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <Ppi/FirmwareVolume.h>
> 
>  #include <Ppi/EndOfPeiPhase.h>
> 
>  #include <Ppi/FirmwareVolumeInfoMeasurementExcluded.h>
> 
> +#include <Ppi/Tcg.h>
> 
> 
> 
>  #include <Guid/TcgEventHob.h>
> 
>  #include <Guid/MeasuredFvHob.h>
> 
> @@ -51,6 +52,45 @@ EFI_PEI_PPI_DESCRIPTOR  mTpmInitializationDonePpiList
> = {
>    NULL
> 
>  };
> 
> 
> 
> +/**
> 
> +  Do a hash operation on a data buffer, extend a specific TPM PCR with the
> hash result,
> 
> +  and build a GUIDed HOB recording the event which will be passed to the DXE
> phase and
> 
> +  added into the Event Log.
> 
> +
> 
> +  @param[in]      This          Indicates the calling context
> 
> +  @param[in]      Flags         Bitmap providing additional information.
> 
> +  @param[in]      HashData      Physical address of the start of the data buffer
> 
> +                                to be hashed, extended, and logged.
> 
> +  @param[in]      HashDataLen   The length, in bytes, of the buffer referenced by
> HashData.
> 
> +  @param[in]      NewEventHdr   Pointer to a TCG_PCR_EVENT_HDR data
> structure.
> 
> +  @param[in]      NewEventData  Pointer to the new event data.
> 
> +
> 
> +  @retval EFI_SUCCESS           Operation completed successfully.
> 
> +  @retval EFI_OUT_OF_RESOURCES  No enough memory to log the new event.
> 
> +  @retval EFI_DEVICE_ERROR      The command was unsuccessful.
> 
> +
> 
> +**/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +HashLogExtendEvent (
> 
> +  IN      EDKII_TCG_PPI             *This,
> 
> +  IN      UINT64                    Flags,
> 
> +  IN      UINT8                     *HashData,
> 
> +  IN      UINTN                     HashDataLen,
> 
> +  IN      TCG_PCR_EVENT_HDR         *NewEventHdr,
> 
> +  IN      UINT8                     *NewEventData
> 
> +  );
> 
> +
> 
> +EDKII_TCG_PPI mEdkiiTcgPpi = {
> 
> +  HashLogExtendEvent
> 
> +};
> 
> +
> 
> +EFI_PEI_PPI_DESCRIPTOR  mTcgPpiList = {
> 
> +  EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
> 
> +  &gEdkiiTcgPpiGuid,
> 
> +  &mEdkiiTcgPpi
> 
> +};
> 
> +
> 
>  //
> 
>  // Number of firmware blobs to grow by each time we run out of room
> 
>  //
> 
> @@ -243,7 +283,8 @@ TpmCommHashAll (
>    and build a GUIDed HOB recording the event which will be passed to the DXE
> phase and
> 
>    added into the Event Log.
> 
> 
> 
> -  @param[in]      PeiServices   Describes the list of possible PEI Services.
> 
> +  @param[in]      This          Indicates the calling context.
> 
> +  @param[in]      Flags         Bitmap providing additional information.
> 
>    @param[in]      HashData      Physical address of the start of the data buffer
> 
>                                  to be hashed, extended, and logged.
> 
>    @param[in]      HashDataLen   The length, in bytes, of the buffer referenced by
> HashData.
> 
> @@ -256,8 +297,10 @@ TpmCommHashAll (
> 
> 
>  **/
> 
>  EFI_STATUS
> 
> +EFIAPI
> 
>  HashLogExtendEvent (
> 
> -  IN      EFI_PEI_SERVICES          **PeiServices,
> 
> +  IN      EDKII_TCG_PPI             *This,
> 
> +  IN      UINT64                    Flags,
> 
>    IN      UINT8                     *HashData,
> 
>    IN      UINTN                     HashDataLen,
> 
>    IN      TCG_PCR_EVENT_HDR         *NewEventHdr,
> 
> @@ -346,7 +389,8 @@ MeasureCRTMVersion (
>    TcgEventHdr.EventSize = (UINT32) StrSize((CHAR16*)PcdGetPtr
> (PcdFirmwareVersionString));
> 
> 
> 
>    return HashLogExtendEvent (
> 
> -           PeiServices,
> 
> +           &mEdkiiTcgPpi,
> 
> +           0,
> 
>             (UINT8*)PcdGetPtr (PcdFirmwareVersionString),
> 
>             TcgEventHdr.EventSize,
> 
>             &TcgEventHdr,
> 
> @@ -415,7 +459,8 @@ MeasureFvImage (
>    TcgEventHdr.EventSize = sizeof (FvBlob);
> 
> 
> 
>    Status = HashLogExtendEvent (
> 
> -             (EFI_PEI_SERVICES **) GetPeiServicesTablePointer(),
> 
> +             &mEdkiiTcgPpi,
> 
> +             0,
> 
>               (UINT8*) (UINTN) FvBlob.BlobBase,
> 
>               (UINTN) FvBlob.BlobLength,
> 
>               &TcgEventHdr,
> 
> @@ -744,6 +789,12 @@ PeimEntryMP (
>    Status = PeiServicesNotifyPpi (&mNotifyList[0]);
> 
>    ASSERT_EFI_ERROR (Status);
> 
> 
> 
> +  //
> 
> +  // install Tcg Services
> 
> +  //
> 
> +  Status = PeiServicesInstallPpi (&mTcgPpiList);
> 
> +  ASSERT_EFI_ERROR (Status);
> 
> +
> 
>    return Status;
> 
>  }
> 
> 
> 
> diff --git a/SecurityPkg/Tcg/TcgPei/TcgPei.inf
> b/SecurityPkg/Tcg/TcgPei/TcgPei.inf
> index c0bff6e85e..4ab4edd657 100644
> --- a/SecurityPkg/Tcg/TcgPei/TcgPei.inf
> +++ b/SecurityPkg/Tcg/TcgPei/TcgPei.inf
> @@ -4,7 +4,7 @@
>  #  This module will initialize TPM device, measure reported FVs and BIOS version.
> 
>  #  This module may also lock TPM physical presence and
> physicalPresenceLifetimeLock.
> 
>  #
> 
> -# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> 
> +# Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.<BR>
> 
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  #
> 
>  ##
> 
> @@ -67,6 +67,7 @@
>    gPeiTpmInitializedPpiGuid                                           ## SOMETIMES_PRODUCES
> 
>    gPeiTpmInitializationDonePpiGuid                                    ## PRODUCES
> 
>    gEfiEndOfPeiSignalPpiGuid                                           ## SOMETIMES_CONSUMES
> ## NOTIFY
> 
> +  gEdkiiTcgPpiGuid                                                    ## PRODUCES
> 
> 
> 
>  [Pcd]
> 
>    gEfiSecurityPkgTokenSpaceGuid.PcdPhysicalPresenceLifetimeLock       ##
> SOMETIMES_CONSUMES
> 
> --
> 2.26.2.windows.1