On Fri, 8 Mar 2024 at 11:43, Ard Biesheuvel ardb@kernel.org wrote:
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,
Indeed
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.
FWIW the same principle applies in the u-boot implementation as well. As far as this series is concerned, keeping the TCG_PCClientTaggedEvent will make reading the code & spec in parallel easier, but I don't have any strong opinions -- I am fine with both
Cheers /Ilias
So please disregard this series