From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 98FD581E95 for ; Thu, 17 Nov 2016 17:31:36 -0800 (PST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP; 17 Nov 2016 17:31:41 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,655,1473145200"; d="scan'208";a="1086867912" Received: from shzintpr01.sh.intel.com (HELO [10.253.24.26]) ([10.239.4.80]) by fmsmga002.fm.intel.com with ESMTP; 17 Nov 2016 17:31:39 -0800 To: "Yao, Jiewen" , "edk2-devel@lists.01.org" References: <1479374969-63472-1-git-send-email-star.zeng@intel.com> <1479374969-63472-3-git-send-email-star.zeng@intel.com> <74D8A39837DF1E4DA445A8C0B3885C50386D6929@shsmsx102.ccr.corp.intel.com> Cc: "Zhang, Chao B" , star.zeng@intel.com From: "Zeng, Star" Message-ID: Date: Fri, 18 Nov 2016 09:31:09 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C50386D6929@shsmsx102.ccr.corp.intel.com> Subject: Re: [PATCH 2/2] SecurityPkg Tcg2Dxe: Filter inactive digest in event2 log from PEI HOB X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Nov 2016 01:31:36 -0000 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit On 2016/11/17 20:19, Yao, Jiewen wrote: > 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. Oh, yes. The description was just copied from CopyDigestListToBuffer () of Tpm2CommandLib, do you think we also need to update the description for CopyDigestListToBuffer (). > > 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 ? Yes, It can be done by adding a parameter to CopyDigestListBinToBuffer () and compare if HashAlgorithmMaskCopied equals to HashAlgorithmMask to know if all required TCG evente log are reported. VOID * CopyDigestListBinToBuffer ( IN OUT VOID *Buffer, IN VOID *DigestListBin, IN UINT32 HashAlgorithmMask, * OUT UINT32 *HashAlgorithmMaskCopied* ) Thanks, Star > > > >> -----Original Message----- >> From: Zeng, Star >> Sent: Thursday, November 17, 2016 5:29 PM >> To: edk2-devel@lists.01.org >> Cc: Zeng, Star ; Yao, Jiewen ; >> Zhang, Chao B >> Subject: [PATCH 2/2] SecurityPkg Tcg2Dxe: Filter inactive digest in event2 log >> from PEI HOB >> >> Cc: Jiewen Yao >> Cc: Chao Zhang >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Star Zeng >> --- >> 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