Re: [PATCH 2/2] SecurityPkg Tcg2Dxe: Filter inactive digest in event2 log from PEI HOB

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

Hi Star
1) I am a little confused on below description.
+  @param[in,out] Buffer             Buffer to hold TPML_DIGEST_VALUES.

I think the buffer is the *TPML_DIGEST_VALUES compact binary buffer*, instead of *TPML_DIGEST_VALUES*, right?
I suggest we describe it clearly.

2) I think the FILTER is great to report ERROR if the TCG event hob producer makes mistake.
Do you think it is worthy to add more stronger check that: All required TCG event log are reported ?



> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, November 17, 2016 5:29 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 2/2] SecurityPkg Tcg2Dxe: Filter inactive digest in event2 log
> from PEI HOB
> 
> Cc: Jiewen Yao <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>
> ---
>  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 80
> +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 76 insertions(+), 4 deletions(-)
> 
> diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> index db8d662f80dc..d5a32307db6e 100644
> --- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> +++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> @@ -898,6 +898,56 @@ GetDigestListBinSize (
>  }
> 
>  /**
> +  Copy TPML_DIGEST_VALUES compact binary into a buffer
> +
> +  @param[in,out] Buffer             Buffer to hold
> TPML_DIGEST_VALUES.
> +  @param[in]     DigestListBin      TPML_DIGEST_VALUES compact
> binary buffer.
> +  @param[in]     HashAlgorithmMask  HASH bits corresponding to the
> desired digests to copy.
> +
> +  @return The end of buffer to hold TPML_DIGEST_VALUES.
> +**/
> +VOID *
> +CopyDigestListBinToBuffer (
> +  IN OUT VOID                       *Buffer,
> +  IN VOID                           *DigestListBin,
> +  IN UINT32                         HashAlgorithmMask
> +  )
> +{
> +  UINTN         Index;
> +  UINT16        DigestSize;
> +  UINT32        Count;
> +  TPMI_ALG_HASH HashAlg;
> +  UINT32        DigestListCount;
> +  UINT32        *DigestListCountPtr;
> +
> +  DigestListCountPtr = (UINT32 *) Buffer;
> +  DigestListCount = 0;
> +
> +  Count = ReadUnaligned32 (DigestListBin);
> +  Buffer = (UINT8 *)Buffer + sizeof(Count);
> +  DigestListBin = (UINT8 *)DigestListBin + sizeof(Count);
> +  for (Index = 0; Index < Count; Index++) {
> +    HashAlg = ReadUnaligned16 (DigestListBin);
> +    DigestListBin = (UINT8 *)DigestListBin + sizeof(HashAlg);
> +    DigestSize = GetHashSizeFromAlgo (HashAlg);
> +
> +    if (IsHashAlgSupportedInHashAlgorithmMask(HashAlg,
> HashAlgorithmMask)) {
> +      CopyMem (Buffer, &HashAlg, sizeof(HashAlg));
> +      Buffer = (UINT8 *)Buffer + sizeof(HashAlg);
> +      CopyMem (Buffer, DigestListBin, DigestSize);
> +      Buffer = (UINT8 *)Buffer + DigestSize;
> +      DigestListCount++;
> +    } else {
> +      DEBUG ((EFI_D_ERROR, "WARNING: CopyDigestListBinToBuffer
> Event log has HashAlg unsupported by PCR bank (0x%x)\n", HashAlg));
> +    }
> +    DigestListBin = (UINT8 *)DigestListBin + DigestSize;
> +  }
> +  WriteUnaligned32 (DigestListCountPtr, DigestListCount);
> +
> +  return Buffer;
> +}
> +
> +/**
>    Add a new entry to the Event Log.
> 
>    @param[in]     DigestList    A list of digest.
> @@ -1317,8 +1367,12 @@ SetupEventLog (
>    EFI_PEI_HOB_POINTERS            GuidHob;
>    EFI_PHYSICAL_ADDRESS            Lasa;
>    UINTN                           Index;
> +  VOID                            *DigestListBin;
> +  TPML_DIGEST_VALUES              TempDigestListBin;
>    UINT32                          DigestListBinSize;
> +  UINT8                           *Event;
>    UINT32                          EventSize;
> +  UINT32                          *EventSizePtr;
>    TCG_EfiSpecIDEventStruct        *TcgEfiSpecIdEventStruct;
>    UINT8
> TempBuf[sizeof(TCG_EfiSpecIDEventStruct) + sizeof(UINT32) +
> (HASH_COUNT * sizeof(TCG_EfiSpecIdEventAlgorithmSize)) + sizeof(UINT8)];
>    TCG_PCR_EVENT_HDR               FirstPcrEvent;
> @@ -1497,7 +1551,8 @@ SetupEventLog (
>        Status = EFI_SUCCESS;
>        while (!EFI_ERROR (Status) &&
>               (GuidHob.Raw = GetNextGuidHob
> (mTcg2EventInfo[Index].EventGuid, GuidHob.Raw)) != NULL) {
> -        TcgEvent    = GET_GUID_HOB_DATA (GuidHob.Guid);
> +        TcgEvent    = AllocateCopyPool (GET_GUID_HOB_DATA_SIZE
> (GuidHob.Guid), GET_GUID_HOB_DATA (GuidHob.Guid));
> +        ASSERT (TcgEvent != NULL);
>          GuidHob.Raw = GET_NEXT_HOB (GuidHob);
>          switch (mTcg2EventInfo[Index].LogFormat) {
>          case EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2:
> @@ -1510,17 +1565,34 @@ SetupEventLog (
>                       );
>            break;
>          case EFI_TCG2_EVENT_LOG_FORMAT_TCG_2:
> -          DigestListBinSize = GetDigestListBinSize ((UINT8 *)TcgEvent +
> sizeof(TCG_PCRINDEX) + sizeof(TCG_EVENTTYPE));
> -          CopyMem (&EventSize, (UINT8 *)TcgEvent +
> sizeof(TCG_PCRINDEX) + sizeof(TCG_EVENTTYPE) + DigestListBinSize,
> sizeof(UINT32));
> +          DigestListBin = (UINT8 *)TcgEvent + sizeof(TCG_PCRINDEX) +
> sizeof(TCG_EVENTTYPE);
> +          DigestListBinSize = GetDigestListBinSize (DigestListBin);
> +          //
> +          // Save event size.
> +          //
> +          CopyMem (&EventSize, (UINT8 *)DigestListBin +
> DigestListBinSize, sizeof(UINT32));
> +          Event = (UINT8 *)DigestListBin + DigestListBinSize +
> sizeof(UINT32);
> +          //
> +          // Filter inactive digest in the event2 log from PEI HOB.
> +          //
> +          CopyMem (&TempDigestListBin, DigestListBin,
> GetDigestListBinSize (DigestListBin));
> +          EventSizePtr = CopyDigestListBinToBuffer (DigestListBin,
> &TempDigestListBin, mTcgDxeData.BsCap.ActivePcrBanks);
> +          //
> +          // Restore event size.
> +          //
> +          CopyMem (EventSizePtr, &EventSize, sizeof(UINT32));
> +          DigestListBinSize = GetDigestListBinSize (DigestListBin);
> +
>            Status = TcgDxeLogEvent (
>                       mTcg2EventInfo[Index].LogFormat,
>                       TcgEvent,
>                       sizeof(TCG_PCRINDEX) + sizeof(TCG_EVENTTYPE)
> + DigestListBinSize + sizeof(UINT32),
> -                     (UINT8 *)TcgEvent + sizeof(TCG_PCRINDEX) +
> sizeof(TCG_EVENTTYPE) + DigestListBinSize + sizeof(UINT32),
> +                     Event,
>                       EventSize
>                       );
>            break;
>          }
> +        FreePool (TcgEvent);
>        }
>      }
>    }
> --
> 2.7.0.windows.1