Re: [PATCH 1/5] efi_loader: increase eventlog buffer size

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

On Thu, Jul 15, 2021 at 02:09:57PM +0900, Masahisa Kojima wrote:
> Hi Simon, Ilias,
> 
> On Wed, 14 Jul 2021 at 23:50, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Masahisa,
> >
> > On Mon, 12 Jul 2021 at 02:40, Masahisa Kojima
> > <masahisa.kojima@linaro.org> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Sun, 11 Jul 2021 at 09:01, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Masahisa,
> > > >
> > > > On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:
> > > > >
> > > > > On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > On 7/7/21 3:36 PM, Masahisa Kojima wrote:
> > > > > > > This is a preperation to add eventlog support
> > > > > > > described in TCG PC Client PFP spec.
> > > > > > >
> > > > > > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > > > > > > ---
> > > > > > >   lib/efi_loader/Kconfig | 2 +-
> > > > > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > > > > > > index b2ab48a048..a87bf3cc98 100644
> > > > > > > --- a/lib/efi_loader/Kconfig
> > > > > > > +++ b/lib/efi_loader/Kconfig
> > > > > > > @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL
> > > > > > >   config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE
> > > > > > >       int "EFI_TCG2_PROTOCOL EventLog size"
> > > > > > >       depends on EFI_TCG2_PROTOCOL
> > > > > > > -     default 4096
> > > > > > > +     default 16384
> > > > > >
> > > > > > I found this text in EDK II:
> > > > > >
> > > > > > Minimum length(in bytes) of the system preboot TCG event log area(LAML)
> > > > > > -----------------------------------------------------------------------
> > > > > >
> > > > > > For PC Client Implementation spec up to and including 1.2 the minimum
> > > > > > log size is 64KB. (SecurityPkg/SecurityPkg.dec)
> > > > >
> > > > > Thank you for your feedback.
> > > > > I have not checked this.
> > > > > TCG spec also says "The Log Area Minimum Length for the TCG event log
> > > > > MUST be at least 64KB." in ACPI chapter.
> > > > > I will update to set 64KB as default.
> > > > >
> > > >
> > > > Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we
> > > > put this in the bloblist? We want to avoid adding code in EFI which is
> > > > in U-Boot.
> > >
> > > I think bloblist is used for data passing from SPL/TPL to u-boot.
> >
> > It can also be used to place things in memory that end up accessed by
> > Linux, e.g. ACPI tables. So I think it fits.
> 
> I understand bloblist can be used for eventlog, I will check further.
> 
> >
> > > Is your comment saying that the eventlog generated
> > > in u-boot(done in efi_tcg2.c with this patch series) should be appended
> > > into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?
> >
> > I suppose so, but this logic should be implemented in the TPM layer I
> > think, not just in EFI. Otherwise we end up with another parallel
> > implementation.
> 
> Current u-boot/lib/efi_loader/efi_tcg2.c includes the code
> not directly related to EFI.
> I would like to suggest to divide u-boot/lib/efi_loader/efi_tcg2.c
> into two files.
> 
>  1)  u-boot/lib/efi_loader/efi_tcg2.c
>   This file implements the EFI interfaces required in TCG EFI Protocol
> Specification(https://trustedcomputinggroup.org/resource/tcg-efi-protocol-specification/).
> 

The only problem I see with the way the efi tcg2 eventlog is currently
created, is that it's all done during the efi init.  Ideally this should
happen earlier, especially if SPL or TF-A create their own eventlog.

The status with TF-A atm is that it only creates an eventlog which then
copies to secure and non-secure memory, but it doesnt extend the PCRs. 
We should pick that eventlog from u-boot (or better op-tee), extend the 
PCRs based on the information of the log and then use it as our basis and
start appending events there. 

>  2) u-boot/lib/tcg2.c(new file)
>  This file implements the functionality required in TCG PC Client
> Platform Firmware Profile
> Specification(https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmware-profile-specification/).
>  This file contains the common functions to extend eventlog and PCRs, etc.

Splitting up the pc client spec bits is probably a good idea. 
What do you have in mind? Moving tcg2_pcr_extend() and
tcg2_agile_log_append() as well, or just the pc client related wrappers?

> 
> In addition, this is different topic, I found some tpm related files
> under u-boot/lib directory, I think it better to have new directory
> like "tcg" and move tpm related files such as tpm_api.c, tpm-v2.c
> and tpm-common.c  into lib/tcg(new directory).

+1

Regards
/Ilias
> 
> Thanks,
> Masahisa Kojima
> 
> >
> > Regards,
> > Simon