Re: [PATCH 0/2] add selftest for EFI_TCG2_PROTOCOL and Measured Boot

[Simon Glass <sjg@chromium.org>]

Hi Masahisa,

On Tue, 2 Nov 2021 at 02:03, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:
>
> Hi Simon,
>
> On Mon, 25 Oct 2021 at 04:54, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Masahisa,
> >
> > On Fri, 22 Oct 2021 at 05:23, Masahisa Kojima
> > <masahisa.kojima@linaro.org> wrote:
> > >
> > > This patch series adds the selftest for the EFI_TCG2_PROTOCOL and
> > > Measured Boot flow.
> > > This selftest is verified on qemu with swtpm.
> >
> > Is this in CI? Where are the instructions for doing this?
>
> Not yet included in CI.
> For the instructions, Ilias is preparing the documentation at:
> https://github.com/apalos/u-boot/commit/6edcf3c02996edf8c50a38632aac1091f8bcbf0b

OK I see.

>
> >
> > I have expressed my preference for expanding the in-tree emulator to
> > handle this.
>
> For the measured boot selftest, I need to access the efi internal data such as
> SMBIOS table, that is why I chose the C based efi_selftest.
> Tcg2 efi_selftest does not rely on the specific TPM backend.

I don't understand this answer. I would still like to know how much
code we would be talking about if we expand the TPM emulator in U-Boot
for these extra features. From my understanding, the TPMs themselves
are not that complicated and don't have a lot of code in them,
particularly for the features we use. I am willing to believe that
this might not be feasible, or be too much effort, but I have not seen
anyone attempt it, or part of it, so I don't know. With a TPM emulator
we can encode any sort of test behaviour we want. It is much harder
with QEMU. But I have not seen an answer for what is actually involved
in doing this.

I know Ilias talked about bringing in swtpm to U-Boot. It seems like a
lot of code so I am not sure if it is worth it. Perhaps it could be
built against U-Boot sandbox as an option...I'm not sure. But the goal
here is not to emulate a TPM, but to provide test coverage in an
easily maintainable way.

So to put my mind at ease, what would be involved in running this on
sandbox? Are we talking hundreds of lines of code (which I think is
worth it) or thousands (which is not)?

Regards,
Simon


>
> Thanks,
> Masahisa Kojima
>
>
> >
> > Regards,
> > Simon
> >
> >
> > >
> > > This covers most of the functionalities, but there are some
> > > limitations and TODO items.
> > >
> > > [Limitation]
> > > - tcg2 selftest must run at the beginning of the efi_selftest because
> > >   some measurement occurs in efi_tcg2_register() and boottime->image_load().
> > >   Need to configure the efi_selftest with "setenv efi_selftest tcg2; bootefi selftest"
> > > - Skip ExitBootService measurement test
> > >    - EFI application can not read PCR after calling ExitBootService
> > > - Skip EventLog Validation
> > >    - Measured Boot measures U-Boot version, so EventLog varies every build having
> > >      different commit hash.
> > > - Skip PCR[0] validation
> > >    - PCR[0] include U-Boot version measurement, this value varies every build
> > >      having different commit hash.
> > > - Skip PCR[7] validation
> > >    - Secure Boot Variables can not be updated through efi_selftest.
> > > - The initial PCR value of PCR[17 - 22] is all 0xff, I'm not sure
> > >   it is expected or not.
> > >
> > > [TODO]
> > > - GPT measurement test
> > > - Secure Boot Variable test
> > > - Eventlog validation
> > >
> > > Masahisa Kojima (2):
> > >   efi_loader: add missing const qualifier
> > >   efi_selftest: add selftest for EFI_TCG2_PROTOCOL and Measured Boot
> > >
> > >  include/efi_api.h                             |   2 +-
> > >  lib/efi_loader/efi_boottime.c                 |   5 +-
> > >  lib/efi_selftest/Makefile                     |  10 +
> > >  .../efi_selftest_miniapp_measuredboot.c       |  93 ++
> > >  lib/efi_selftest/efi_selftest_tcg2.c          | 804 +++++++++++++++++-
> > >  5 files changed, 910 insertions(+), 4 deletions(-)
> > >  create mode 100644 lib/efi_selftest/efi_selftest_miniapp_measuredboot.c
> > >
> > > --
> > > 2.17.1
> > >