* [PATCH 0/2] Tcg2Dxe: Filter inactive digest in event2 log from PEI HOB @ 2016-11-17 9:29 Star Zeng 2016-11-17 9:29 ` [PATCH 1/2] SecurityPkg TPM2: Make IsHashAlgSupportedInHashAlgorithmMask external Star Zeng 2016-11-17 9:29 ` [PATCH 2/2] SecurityPkg Tcg2Dxe: Filter inactive digest in event2 log from PEI HOB Star Zeng 0 siblings, 2 replies; 7+ messages in thread From: Star Zeng @ 2016-11-17 9:29 UTC (permalink / raw) To: edk2-devel; +Cc: Star Zeng Star Zeng (2): SecurityPkg TPM2: Make IsHashAlgSupportedInHashAlgorithmMask external SecurityPkg Tcg2Dxe: Filter inactive digest in event2 log from PEI HOB SecurityPkg/Include/Library/Tpm2CommandLib.h | 16 ++++++ SecurityPkg/Library/Tpm2CommandLib/Tpm2Help.c | 1 + SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 80 +++++++++++++++++++++++++-- 3 files changed, 93 insertions(+), 4 deletions(-) -- 2.7.0.windows.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] SecurityPkg TPM2: Make IsHashAlgSupportedInHashAlgorithmMask external 2016-11-17 9:29 [PATCH 0/2] Tcg2Dxe: Filter inactive digest in event2 log from PEI HOB Star Zeng @ 2016-11-17 9:29 ` Star Zeng 2016-11-17 12:26 ` Yao, Jiewen 2016-11-17 9:29 ` [PATCH 2/2] SecurityPkg Tcg2Dxe: Filter inactive digest in event2 log from PEI HOB Star Zeng 1 sibling, 1 reply; 7+ messages in thread From: Star Zeng @ 2016-11-17 9:29 UTC (permalink / raw) To: edk2-devel; +Cc: Star Zeng, Jiewen Yao, Chao Zhang Current IsHashAlgSupportedInHashAlgorithmMask is only an internal function, this patch makes it external for coming consumer. 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/Include/Library/Tpm2CommandLib.h | 16 ++++++++++++++++ SecurityPkg/Library/Tpm2CommandLib/Tpm2Help.c | 1 + 2 files changed, 17 insertions(+) diff --git a/SecurityPkg/Include/Library/Tpm2CommandLib.h b/SecurityPkg/Include/Library/Tpm2CommandLib.h index 9a1dd8d8aceb..85a4c65e0263 100644 --- a/SecurityPkg/Include/Library/Tpm2CommandLib.h +++ b/SecurityPkg/Include/Library/Tpm2CommandLib.h @@ -1007,6 +1007,22 @@ GetHashSizeFromAlgo ( ); /** + Return if hash alg is supported in HashAlgorithmMask. + + @param HashAlg Hash algorithm to be checked. + @param HashAlgorithmMask Bitfield of allowed hash algorithms. + + @retval TRUE Hash algorithm is supported. + @retval FALSE Hash algorithm is not supported. +**/ +BOOLEAN +EFIAPI +IsHashAlgSupportedInHashAlgorithmMask( + IN TPMI_ALG_HASH HashAlg, + IN UINT32 HashAlgorithmMask + ); + +/** Copy TPML_DIGEST_VALUES into a buffer @param[in,out] Buffer Buffer to hold TPML_DIGEST_VALUES. diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Help.c b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Help.c index be95fd69b3dd..95d4f7c84ce9 100644 --- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Help.c +++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Help.c @@ -175,6 +175,7 @@ CopyAuthSessionResponse ( @retval FALSE Hash algorithm is not supported. **/ BOOLEAN +EFIAPI IsHashAlgSupportedInHashAlgorithmMask( IN TPMI_ALG_HASH HashAlg, IN UINT32 HashAlgorithmMask -- 2.7.0.windows.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] SecurityPkg TPM2: Make IsHashAlgSupportedInHashAlgorithmMask external 2016-11-17 9:29 ` [PATCH 1/2] SecurityPkg TPM2: Make IsHashAlgSupportedInHashAlgorithmMask external Star Zeng @ 2016-11-17 12:26 ` Yao, Jiewen 0 siblings, 0 replies; 7+ messages in thread From: Yao, Jiewen @ 2016-11-17 12:26 UTC (permalink / raw) To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Zhang, Chao B Reviewed-by: jiewen.yao@intel.com > -----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 1/2] SecurityPkg TPM2: Make > IsHashAlgSupportedInHashAlgorithmMask external > > Current IsHashAlgSupportedInHashAlgorithmMask is only an internal > function, this patch makes it external for coming consumer. > > 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/Include/Library/Tpm2CommandLib.h | 16 > ++++++++++++++++ > SecurityPkg/Library/Tpm2CommandLib/Tpm2Help.c | 1 + > 2 files changed, 17 insertions(+) > > diff --git a/SecurityPkg/Include/Library/Tpm2CommandLib.h > b/SecurityPkg/Include/Library/Tpm2CommandLib.h > index 9a1dd8d8aceb..85a4c65e0263 100644 > --- a/SecurityPkg/Include/Library/Tpm2CommandLib.h > +++ b/SecurityPkg/Include/Library/Tpm2CommandLib.h > @@ -1007,6 +1007,22 @@ GetHashSizeFromAlgo ( > ); > > /** > + Return if hash alg is supported in HashAlgorithmMask. > + > + @param HashAlg Hash algorithm to be checked. > + @param HashAlgorithmMask Bitfield of allowed hash algorithms. > + > + @retval TRUE Hash algorithm is supported. > + @retval FALSE Hash algorithm is not supported. > +**/ > +BOOLEAN > +EFIAPI > +IsHashAlgSupportedInHashAlgorithmMask( > + IN TPMI_ALG_HASH HashAlg, > + IN UINT32 HashAlgorithmMask > + ); > + > +/** > Copy TPML_DIGEST_VALUES into a buffer > > @param[in,out] Buffer Buffer to hold > TPML_DIGEST_VALUES. > diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Help.c > b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Help.c > index be95fd69b3dd..95d4f7c84ce9 100644 > --- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Help.c > +++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Help.c > @@ -175,6 +175,7 @@ CopyAuthSessionResponse ( > @retval FALSE Hash algorithm is not supported. > **/ > BOOLEAN > +EFIAPI > IsHashAlgSupportedInHashAlgorithmMask( > IN TPMI_ALG_HASH HashAlg, > IN UINT32 HashAlgorithmMask > -- > 2.7.0.windows.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] SecurityPkg Tcg2Dxe: Filter inactive digest in event2 log from PEI HOB 2016-11-17 9:29 [PATCH 0/2] Tcg2Dxe: Filter inactive digest in event2 log from PEI HOB Star Zeng 2016-11-17 9:29 ` [PATCH 1/2] SecurityPkg TPM2: Make IsHashAlgSupportedInHashAlgorithmMask external Star Zeng @ 2016-11-17 9:29 ` Star Zeng 2016-11-17 12:19 ` Yao, Jiewen 1 sibling, 1 reply; 7+ messages in thread From: Star Zeng @ 2016-11-17 9:29 UTC (permalink / raw) To: edk2-devel; +Cc: Star Zeng, Jiewen Yao, Chao Zhang 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] SecurityPkg Tcg2Dxe: Filter inactive digest in event2 log from PEI HOB 2016-11-17 9:29 ` [PATCH 2/2] SecurityPkg Tcg2Dxe: Filter inactive digest in event2 log from PEI HOB Star Zeng @ 2016-11-17 12:19 ` Yao, Jiewen 2016-11-18 1:31 ` Zeng, Star 0 siblings, 1 reply; 7+ messages in thread From: Yao, Jiewen @ 2016-11-17 12:19 UTC (permalink / raw) To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Zhang, Chao B 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] SecurityPkg Tcg2Dxe: Filter inactive digest in event2 log from PEI HOB 2016-11-17 12:19 ` Yao, Jiewen @ 2016-11-18 1:31 ` Zeng, Star 2016-11-18 1:40 ` Yao, Jiewen 0 siblings, 1 reply; 7+ messages in thread From: Zeng, Star @ 2016-11-18 1:31 UTC (permalink / raw) To: Yao, Jiewen, edk2-devel@lists.01.org; +Cc: Zhang, Chao B, star.zeng 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 <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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] SecurityPkg Tcg2Dxe: Filter inactive digest in event2 log from PEI HOB 2016-11-18 1:31 ` Zeng, Star @ 2016-11-18 1:40 ` Yao, Jiewen 0 siblings, 0 replies; 7+ messages in thread From: Yao, Jiewen @ 2016-11-18 1:40 UTC (permalink / raw) To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Zhang, Chao B, Yao, Jiewen That's good idea. I like both. :) From: Zeng, Star Sent: Friday, November 18, 2016 9:31 AM To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org Cc: Zhang, Chao B <chao.b.zhang@intel.com>; Zeng, Star <star.zeng@intel.com> Subject: Re: [edk2] [PATCH 2/2] SecurityPkg Tcg2Dxe: Filter inactive digest in event2 log from PEI HOB 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<mailto:edk2-devel@lists.01.org> >> Cc: Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; >> Zhang, Chao B <chao.b.zhang@intel.com<mailto: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<mailto:jiewen.yao@intel.com>> >> Cc: Chao Zhang <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Star Zeng <star.zeng@intel.com<mailto: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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-11-18 1:42 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-11-17 9:29 [PATCH 0/2] Tcg2Dxe: Filter inactive digest in event2 log from PEI HOB Star Zeng 2016-11-17 9:29 ` [PATCH 1/2] SecurityPkg TPM2: Make IsHashAlgSupportedInHashAlgorithmMask external Star Zeng 2016-11-17 12:26 ` Yao, Jiewen 2016-11-17 9:29 ` [PATCH 2/2] SecurityPkg Tcg2Dxe: Filter inactive digest in event2 log from PEI HOB Star Zeng 2016-11-17 12:19 ` Yao, Jiewen 2016-11-18 1:31 ` Zeng, Star 2016-11-18 1:40 ` Yao, Jiewen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox