mbox series

Pull request for tpm-next-27102023

Message ID 20231027112616.112555-1-ilias.apalodimas@linaro.org
State New
Headers show
Series Pull request for tpm-next-27102023 | expand

Pull-request

https://source.denx.de/u-boot/custodians/u-boot-tpm/ tags/tpm-next-27102023

Message

Ilias Apalodimas Oct. 27, 2023, 11:26 a.m. UTC
Hi Tom,
The following changes since commit e29b932aa07fa0226d325b35d96cd4eea0370129:

  Merge branch '2023-09-30-Kconfig-updates' into next (2023-10-01 11:54:31 -0400)

are available in the Git repository at:

  https://source.denx.de/u-boot/custodians/u-boot-tpm/ tags/tpm-next-27102023

for you to fetch changes up to 4fd7d27ccb763ce8b836a0e4c5dd005392d38e18:

  test/py: always use autostart on tpm2 selftests (2023-10-27 13:17:21 +0300)

The pipeline https://source.denx.de/u-boot/custodians/u-boot-tpm/-/pipelines/18327
showed had no red lights.

Heinrich I did not ignore your reports and I agree with both of your
observations.  The doc needs to mention EFI and the DT Kconfig options
can be squashed in the future.  However this is a big patchset with
a time consuming test procedure.  The changes you requested are
trivial and we can send them later.  I hope that's fine.

Simon there's one patch you haven't acked/reviewed yet, but I've
responded to your concerns in the ML.
The response is here
https://lore.kernel.org/u-boot/CAC_iWjJL_taKWWEi4kancnQ6Dsg1V5+4hMSvTifcdr_aCH-Ykg@mail.gmail.com/

----------------------------------------------------------------
Up to now, U-Boot could perform measurements and EventLog
creation as described by the TCG spec when booting via EFI.

The EFI code was residing in lib/efi_loader/efi_tcg2.c and
contained both EFI specific code + the API needed to access
the TPM, extend PCRs and create an EventLog. The non-EFI part
proved modular enough and moving it around to the TPM
subsystem was straightforward.

With that in place we can have a common API for measuring
binaries regardless of the boot command, EFI or boot(m|i|z),
and contructing an EventLog.

I've tested all of the EFI cases -- booting with an empty
EventLog and booting with a previous stage loader providing one
and found no regressions.  Eddie tested the bootX part.

Eddie also fixed the sandbox TPM which couldn't be used for the
EFI code and it now supports all the required capabilities. This
had a slight sideeffect in our testing since the EFI subsystem
initializes the TPM early and 'tpm2 init' failed during some
python tests. That code only opens the device though, so we
can replace it with 'tpm2 autostart' which doesn't error out and
still allows you to perfom the rest of the tests but doesn't
report an error if the device is already opened.

There's a few minor issues with this PR as well but since testing
and verifying the changes takes a considerable amount of time,
I prefer merging it now.

Heinrich has already sent a PR for -master containing
"efi_loader: fix EFI_ENTRY point on get_active_pcr_banks" and
I am not sure if that will cause any conflicts, but in any case
they should be trivial to resolve.

Both the EFI and non-EFI code have a Kconfig for measuring the
loaded Device Tree.  The reason this is optional is that we
can't reason when/if devices add random info like kaslr-seed,
mac addresses etc in the DT. In that case measurements are
random, board specific and eventually useless.
The reason it was difficult to fix it prior to this patchset is
because the EFI subsystem and thus measurements was brought up
late and DT fixups might have already been applied. With this
patchset we can measure the DT really early in the future.

Heinrich also pointed out that the two Kconfigs for the DTB
measurements can be squashed in a single one and that the
documentation only explains the non-EFI case.  I agree on both
but as I said this is a sane working version, so let's pull this
first it's aleady big enough and painful to test.

I prefer pulling into -next, although as I said, a big portion
of the changes consists of moving the API around from the EFI
subsystem to the TPM subsystem.
----------------------------------------------------------------
Eddie James (6):
      tpm: Fix spelling for tpmu_ha union
      tpm: sandbox: Update for needed TPM2 capabilities
      tpm: Support boot measurements
      bootm: Support boot measurement
      test: Add sandbox TPM boot measurement
      doc: Add measured boot documentation

Ilias Apalodimas (3):
      efi_loader: fix EFI_ENTRY point on get_active_pcr_banks
      test: use a non system PCR for testing PCR extend
      test/py: always use autostart on tpm2 selftests

 arch/sandbox/dts/sandbox.dtsi  |   13 +
 arch/sandbox/dts/test.dts      |   13 +
 boot/Kconfig                   |   32 ++
 boot/bootm.c                   |   74 +++
 cmd/booti.c                    |    1 +
 cmd/bootm.c                    |    2 +
 cmd/bootz.c                    |    1 +
 configs/sandbox_defconfig      |    1 +
 doc/usage/index.rst            |    1 +
 doc/usage/measured_boot.rst    |   31 ++
 drivers/tpm/tpm2_tis_sandbox.c |  100 ++--
 include/bootm.h                |   11 +
 include/efi_tcg2.h             |   44 --
 include/image.h                |    1 +
 include/test/suites.h          |    1 +
 include/tpm-v2.h               |  263 +++++++++-
 lib/Kconfig                    |    4 +
 lib/efi_loader/Kconfig         |    2 -
 lib/efi_loader/efi_tcg2.c      | 1055 +++-------------------------------------
 lib/tpm-v2.c                   |  814 +++++++++++++++++++++++++++++++
 test/boot/Makefile             |    1 +
 test/boot/measurement.c        |   66 +++
 test/cmd_ut.c                  |    4 +
 test/py/tests/test_tpm2.py     |   20 +-
 24 files changed, 1492 insertions(+), 1063 deletions(-)
 create mode 100644 doc/usage/measured_boot.rst
 create mode 100644 test/boot/measurement.c

Comments

Heinrich Schuchardt Oct. 27, 2023, 12:51 p.m. UTC | #1
On 27.10.23 13:26, Ilias Apalodimas wrote:
> Hi Tom,
> The following changes since commit e29b932aa07fa0226d325b35d96cd4eea0370129:
>
>    Merge branch '2023-09-30-Kconfig-updates' into next (2023-10-01 11:54:31 -0400)
>
> are available in the Git repository at:
>
>    https://source.denx.de/u-boot/custodians/u-boot-tpm/ tags/tpm-next-27102023
>
> for you to fetch changes up to 4fd7d27ccb763ce8b836a0e4c5dd005392d38e18:
>
>    test/py: always use autostart on tpm2 selftests (2023-10-27 13:17:21 +0300)
>
> The pipeline https://source.denx.de/u-boot/custodians/u-boot-tpm/-/pipelines/18327
> showed had no red lights.
>
> Heinrich I did not ignore your reports and I agree with both of your
> observations.  The doc needs to mention EFI and the DT Kconfig options
> can be squashed in the future.  However this is a big patchset with
> a time consuming test procedure.  The changes you requested are
> trivial and we can send them later.  I hope that's fine.
>
> Simon there's one patch you haven't acked/reviewed yet, but I've
> responded to your concerns in the ML.
> The response is here
> https://lore.kernel.org/u-boot/CAC_iWjJL_taKWWEi4kancnQ6Dsg1V5+4hMSvTifcdr_aCH-Ykg@mail.gmail.com/
>
> ----------------------------------------------------------------
> Up to now, U-Boot could perform measurements and EventLog
> creation as described by the TCG spec when booting via EFI.
>
> The EFI code was residing in lib/efi_loader/efi_tcg2.c and
> contained both EFI specific code + the API needed to access
> the TPM, extend PCRs and create an EventLog. The non-EFI part
> proved modular enough and moving it around to the TPM
> subsystem was straightforward.
>
> With that in place we can have a common API for measuring
> binaries regardless of the boot command, EFI or boot(m|i|z),
> and contructing an EventLog.
>
> I've tested all of the EFI cases -- booting with an empty
> EventLog and booting with a previous stage loader providing one
> and found no regressions.  Eddie tested the bootX part.
>
> Eddie also fixed the sandbox TPM which couldn't be used for the
> EFI code and it now supports all the required capabilities. This
> had a slight sideeffect in our testing since the EFI subsystem
> initializes the TPM early and 'tpm2 init' failed during some
> python tests. That code only opens the device though, so we
> can replace it with 'tpm2 autostart' which doesn't error out and
> still allows you to perfom the rest of the tests but doesn't
> report an error if the device is already opened.
>
> There's a few minor issues with this PR as well but since testing
> and verifying the changes takes a considerable amount of time,
> I prefer merging it now.
>
> Heinrich has already sent a PR for -master containing
> "efi_loader: fix EFI_ENTRY point on get_active_pcr_banks" and
> I am not sure if that will cause any conflicts, but in any case
> they should be trivial to resolve.
>
> Both the EFI and non-EFI code have a Kconfig for measuring the
> loaded Device Tree.  The reason this is optional is that we
> can't reason when/if devices add random info like kaslr-seed,
> mac addresses etc in the DT. In that case measurements are
> random, board specific and eventually useless.
> The reason it was difficult to fix it prior to this patchset is
> because the EFI subsystem and thus measurements was brought up
> late and DT fixups might have already been applied. With this
> patchset we can measure the DT really early in the future.
>
> Heinrich also pointed out that the two Kconfigs for the DTB
> measurements can be squashed in a single one and that the
> documentation only explains the non-EFI case.  I agree on both
> but as I said this is a sane working version, so let's pull this
> first it's already big enough and painful to test.

I am fine if with adding those changes in a future merge request.

Best regards

Heinrich

>
> I prefer pulling into -next, although as I said, a big portion
> of the changes consists of moving the API around from the EFI
> subsystem to the TPM subsystem.
> ----------------------------------------------------------------
> Eddie James (6):
>        tpm: Fix spelling for tpmu_ha union
>        tpm: sandbox: Update for needed TPM2 capabilities
>        tpm: Support boot measurements
>        bootm: Support boot measurement
>        test: Add sandbox TPM boot measurement
>        doc: Add measured boot documentation
>
> Ilias Apalodimas (3):
>        efi_loader: fix EFI_ENTRY point on get_active_pcr_banks
>        test: use a non system PCR for testing PCR extend
>        test/py: always use autostart on tpm2 selftests
>
>   arch/sandbox/dts/sandbox.dtsi  |   13 +
>   arch/sandbox/dts/test.dts      |   13 +
>   boot/Kconfig                   |   32 ++
>   boot/bootm.c                   |   74 +++
>   cmd/booti.c                    |    1 +
>   cmd/bootm.c                    |    2 +
>   cmd/bootz.c                    |    1 +
>   configs/sandbox_defconfig      |    1 +
>   doc/usage/index.rst            |    1 +
>   doc/usage/measured_boot.rst    |   31 ++
>   drivers/tpm/tpm2_tis_sandbox.c |  100 ++--
>   include/bootm.h                |   11 +
>   include/efi_tcg2.h             |   44 --
>   include/image.h                |    1 +
>   include/test/suites.h          |    1 +
>   include/tpm-v2.h               |  263 +++++++++-
>   lib/Kconfig                    |    4 +
>   lib/efi_loader/Kconfig         |    2 -
>   lib/efi_loader/efi_tcg2.c      | 1055 +++-------------------------------------
>   lib/tpm-v2.c                   |  814 +++++++++++++++++++++++++++++++
>   test/boot/Makefile             |    1 +
>   test/boot/measurement.c        |   66 +++
>   test/cmd_ut.c                  |    4 +
>   test/py/tests/test_tpm2.py     |   20 +-
>   24 files changed, 1492 insertions(+), 1063 deletions(-)
>   create mode 100644 doc/usage/measured_boot.rst
>   create mode 100644 test/boot/measurement.c
Tom Rini Oct. 28, 2023, 12:51 a.m. UTC | #2
On Fri, Oct 27, 2023 at 02:26:16PM +0300, Ilias Apalodimas wrote:
> Hi Tom,

> The following changes since commit e29b932aa07fa0226d325b35d96cd4eea0370129:
> 
>   Merge branch '2023-09-30-Kconfig-updates' into next (2023-10-01 11:54:31 -0400)
> 
> are available in the Git repository at:
> 
>   https://source.denx.de/u-boot/custodians/u-boot-tpm/ tags/tpm-next-27102023
> 
> for you to fetch changes up to 4fd7d27ccb763ce8b836a0e4c5dd005392d38e18:
> 
>   test/py: always use autostart on tpm2 selftests (2023-10-27 13:17:21 +0300)
> 
> The pipeline https://source.denx.de/u-boot/custodians/u-boot-tpm/-/pipelines/18327
> showed had no red lights.

Heinrich, I see that the docs job passed.  But I see:
+++ b/doc/usage/measured_boot.rst
@@ -0,0 +1,31 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+Measured Boot
+=====================

And was expecting htmldocs to complain and fail. It does neither (and
did neither locally which I believe is a regression in the doc building
side of things.

> Heinrich I did not ignore your reports and I agree with both of your
> observations.  The doc needs to mention EFI and the DT Kconfig options
> can be squashed in the future.  However this is a big patchset with
> a time consuming test procedure.  The changes you requested are
> trivial and we can send them later.  I hope that's fine.
> 
> Simon there's one patch you haven't acked/reviewed yet, but I've
> responded to your concerns in the ML.
> The response is here
> https://lore.kernel.org/u-boot/CAC_iWjJL_taKWWEi4kancnQ6Dsg1V5+4hMSvTifcdr_aCH-Ykg@mail.gmail.com/
> 

Applied to u-boot/master, thanks!
Heinrich Schuchardt Oct. 28, 2023, 10:07 a.m. UTC | #3
On 10/28/23 02:51, Tom Rini wrote:
> On Fri, Oct 27, 2023 at 02:26:16PM +0300, Ilias Apalodimas wrote:
>> Hi Tom,
>
>> The following changes since commit e29b932aa07fa0226d325b35d96cd4eea0370129:
>>
>>    Merge branch '2023-09-30-Kconfig-updates' into next (2023-10-01 11:54:31 -0400)
>>
>> are available in the Git repository at:
>>
>>    https://source.denx.de/u-boot/custodians/u-boot-tpm/ tags/tpm-next-27102023
>>
>> for you to fetch changes up to 4fd7d27ccb763ce8b836a0e4c5dd005392d38e18:
>>
>>    test/py: always use autostart on tpm2 selftests (2023-10-27 13:17:21 +0300)
>>
>> The pipeline https://source.denx.de/u-boot/custodians/u-boot-tpm/-/pipelines/18327
>> showed had no red lights.
>
> Heinrich, I see that the docs job passed.  But I see:
> +++ b/doc/usage/measured_boot.rst
> @@ -0,0 +1,31 @@
> +.. SPDX-License-Identifier: GPL-2.0+
> +
> +Measured Boot
> +=====================

If I make the underline too short I get an error:

Warning, treated as error:
doc/usage/measured_boot.rst:4:Title underline too short.

Measured Boot
===========

The message is created in
lib/python3.11/site-packages/docutils/parsers/rst/states.py:2750:
         msg = self.reporter.warning('Title underline too short.',

docutils does not create warnings for title lines that are too long.

I have sent a patch fixing our documentation:

https://lore.kernel.org/u-boot/20231028100312.20623-1-heinrich.schuchardt@canonical.com/T/#u

Best regards

Heinrich