On Fri, 8 Mar 2024 at 10:38, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Ard
On Fri, 8 Mar 2024 at 10:58, Ard Biesheuvel ardb+git@google.com wrote:
From: Ard Biesheuvel ardb@kernel.org
Our efi_tcg2_tagged_event is not defined in the EFI spec, but it is not a local invention either: it was taken from the TCG PC Client spec, where it is called TCG_PCClientTaggedEvent.
This spec also contains some guidance on how to populate it, which is not being followed closely at the moment; the event size should cover the TCG_PCClientTaggedEvent and its payload only, but it currently covers the preceding efi_tcg2_event too, and this may result in trailing garbage being measured into the TPM.
I think there's a confusion here and the current code we have is correct. The EFI TCG spec [0] says that the tdEFI_TCG2_EVENT size is: "Total size of the event including the Size component, the header and the Event data." which obviously contradicts the definition of the tagged event in the PC client spec. But given the fact that TCG_PCClientTaggedEvent has its own size field I think we should use what we already have.
[0] https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specificat... page 33
Fair enough.
It is rather disappointing that the TCG2 specs contradict each other, but a quick inspection of the EDK2 implementation shows that it follows your interpretation.
For example, in Tcg2HashLogExtendEvent() [SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c], we have a check
if (Event->Size < Event->Header.HeaderSize + sizeof (UINT32)) { return EFI_INVALID_PARAMETER; }
which ensures that the event size covers at least the EFI_TCG2_EVENT, which obviously implies that 'size' is expected to include it.
So please disregard this series