Re: [PATCH v2 4/7] SecurityPkg/Tcg2: 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 4/7] SecurityPkg/Tcg2: 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/Tcg2Pei/Tcg2Pei.c   | 100 +++++++++++++++++++++-------
>  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf |   3 +-
>  2 files changed, 78 insertions(+), 25 deletions(-)
> 
> diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> index 19b8e4b318..8114de45c7 100644
> --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> @@ -1,7 +1,7 @@
>  /** @file
> 
>    Initialize TPM2 device and measure FVs before handing off control to DXE.
> 
> 
> 
> -Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
> 
> +Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.<BR>
> 
>  Copyright (c) 2017, Microsoft 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/EndOfPeiPhase.h>
> 
>  #include <Ppi/FirmwareVolumeInfoMeasurementExcluded.h>
> 
>  #include <Ppi/FirmwareVolumeInfoPrehashedFV.h>
> 
> +#include <Ppi/Tcg.h>
> 
> 
> 
>  #include <Guid/TcgEventHob.h>
> 
>  #include <Guid/MeasuredFvHob.h>
> 
> @@ -66,6 +67,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
> 
>  //
> 
> @@ -375,6 +415,7 @@ LogHashEvent (
>    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.
> 
> @@ -388,7 +429,9 @@ LogHashEvent (
> 
> 
>  **/
> 
>  EFI_STATUS
> 
> +EFIAPI
> 
>  HashLogExtendEvent (
> 
> +  IN      EDKII_TCG_PPI             *This,
> 
>    IN      UINT64                    Flags,
> 
>    IN      UINT8                     *HashData,
> 
>    IN      UINTN                     HashDataLen,
> 
> @@ -403,16 +446,23 @@ HashLogExtendEvent (
>      return EFI_DEVICE_ERROR;
> 
>    }
> 
> 
> 
> -  Status = HashAndExtend (
> 
> -             NewEventHdr->PCRIndex,
> 
> -             HashData,
> 
> -             HashDataLen,
> 
> +  if(Flags & EFI_TCG2_PRE_HASH) {
> 
> +    ZeroMem (&DigestList, sizeof(DigestList));
> 
> +    CopyMem(&DigestList, HashData, sizeof(DigestList));
> 
> +    Status = Tpm2PcrExtend(
> 
> +             0,
> 
>               &DigestList
> 
>               );
> 
> +  } else {
> 
> +    Status = HashAndExtend (
> 
> +               NewEventHdr->PCRIndex,
> 
> +               HashData,
> 
> +               HashDataLen,
> 
> +               &DigestList
> 
> +               );
> 
> +  }
> 
>    if (!EFI_ERROR (Status)) {
> 
> -    if ((Flags & EFI_TCG2_EXTEND_ONLY) == 0) {
> 
> -      Status = LogHashEvent (&DigestList, NewEventHdr, NewEventData);
> 
> -    }
> 
> +    Status = LogHashEvent (&DigestList, NewEventHdr, NewEventData);
> 
>    }
> 
> 
> 
>    if (Status == EFI_DEVICE_ERROR) {
> 
> @@ -452,6 +502,7 @@ MeasureCRTMVersion (
>    TcgEventHdr.EventSize = (UINT32) StrSize((CHAR16*)PcdGetPtr
> (PcdFirmwareVersionString));
> 
> 
> 
>    return HashLogExtendEvent (
> 
> +           &mEdkiiTcgPpi,
> 
>             0,
> 
>             (UINT8*)PcdGetPtr (PcdFirmwareVersionString),
> 
>             TcgEventHdr.EventSize,
> 
> @@ -651,27 +702,22 @@ MeasureFvImage (
>      // FV pre-hash algos comply with current TPM hash requirement
> 
>      // Skip hashing step in measure, only extend DigestList to PCR and log event
> 
>      //
> 
> -    Status = Tpm2PcrExtend(
> 
> -               0,
> 
> -               &DigestList
> 
> +    Status = HashLogExtendEvent (
> 
> +               &mEdkiiTcgPpi,
> 
> +               EFI_TCG2_PRE_HASH,
> 
> +               (UINT8*) &DigestList,        // HashData
> 
> +               (UINTN) sizeof(DigestList),  // HashDataLen
> 
> +               &TcgEventHdr,                // EventHdr
> 
> +               EventData                    // EventData
> 
>                 );
> 
> -
> 
> -    if (!EFI_ERROR(Status)) {
> 
> -       Status = LogHashEvent (&DigestList, &TcgEventHdr, EventData);
> 
> -       DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by
> Tcg2Pei starts at: 0x%x\n", FvBase));
> 
> -       DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by
> Tcg2Pei has the size: 0x%x\n", FvLength));
> 
> -    } else if (Status == EFI_DEVICE_ERROR) {
> 
> -      BuildGuidHob (&gTpmErrorHobGuid,0);
> 
> -      REPORT_STATUS_CODE (
> 
> -        EFI_ERROR_CODE | EFI_ERROR_MINOR,
> 
> -        (PcdGet32 (PcdStatusCodeSubClassTpmDevice) |
> EFI_P_EC_INTERFACE_ERROR)
> 
> -        );
> 
> -    }
> 
> +    DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by
> Tcg2Pei starts at: 0x%x\n", FvBase));
> 
> +    DEBUG ((DEBUG_INFO, "The pre-hashed FV which is extended & logged by
> Tcg2Pei has the size: 0x%x\n", FvLength));
> 
>    } else {
> 
>      //
> 
>      // Hash the FV, extend digest to the TPM and log TCG event
> 
>      //
> 
>      Status = HashLogExtendEvent (
> 
> +               &mEdkiiTcgPpi,
> 
>                 0,
> 
>                 (UINT8*) (UINTN) FvBase, // HashData
> 
>                 (UINTN) FvLength,        // HashDataLen
> 
> @@ -866,6 +912,12 @@ PeimEntryMP (
>    Status = PeiServicesNotifyPpi (&mNotifyList[0]);
> 
>    ASSERT_EFI_ERROR (Status);
> 
> 
> 
> +  //
> 
> +  // install Tcg Services
> 
> +  //
> 
> +  Status = PeiServicesInstallPpi (&mTcgPpiList);
> 
> +  ASSERT_EFI_ERROR (Status);
> 
> +
> 
>    return Status;
> 
>  }
> 
> 
> 
> @@ -893,7 +945,7 @@ MeasureSeparatorEventWithError (
>    TcgEvent.PCRIndex  = PCRIndex;
> 
>    TcgEvent.EventType = EV_SEPARATOR;
> 
>    TcgEvent.EventSize = (UINT32)sizeof (EventData);
> 
> -  return HashLogExtendEvent(0,(UINT8 *)&EventData, TcgEvent.EventSize,
> &TcgEvent,(UINT8 *)&EventData);
> 
> +  return HashLogExtendEvent(&mEdkiiTcgPpi, 0, (UINT8 *)&EventData,
> TcgEvent.EventSize, &TcgEvent,(UINT8 *)&EventData);
> 
>  }
> 
> 
> 
>  /**
> 
> diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> index 3d361e8859..f64b29f1ae 100644
> --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> @@ -8,7 +8,7 @@
>  #
> 
>  #  This module will initialize TPM device, measure reported FVs and BIOS version.
> 
>  #
> 
> -# Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
> 
> +# Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.<BR>
> 
>  # Copyright (c) 2017, Microsoft Corporation.  All rights reserved. <BR>
> 
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  #
> 
> @@ -72,6 +72,7 @@
>    gPeiTpmInitializationDonePpiGuid                                     ## PRODUCES
> 
>    gEfiEndOfPeiSignalPpiGuid                                            ## SOMETIMES_CONSUMES
> ## NOTIFY
> 
>    gEdkiiPeiFirmwareVolumeInfoPrehashedFvPpiGuid                        ##
> SOMETIMES_CONSUMES
> 
> +  gEdkiiTcgPpiGuid                                                     ## PRODUCES
> 
> 
> 
>  [Pcd]
> 
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString              ##
> SOMETIMES_CONSUMES
> 
> --
> 2.26.2.windows.1