mbox series

[00/16] tools: Add support for signing devicetree blobs

Message ID 20211112192817.199075-1-sjg@chromium.org
Headers show
Series tools: Add support for signing devicetree blobs | expand

Message

Simon Glass Nov. 12, 2021, 7:28 p.m. UTC
At present mkimage supports signing FITs, the standard U-Boot image type.

Various people are opposed to using FIT since:

a) it requires adding support for FIT into other bootloaders, notably
   UEFI
b) it requires packaging a kernel in this standard U-Boot format, meaning
   that distros must run 'mkimage' and deal with the kernel and initrd
   being inside a FIT

The kernel and initrd can be dealt with in other ways. But without FIT,
we have no standard way of signing and grouping FDT files. Instead we must
include them in the distro as separate files.

In particular, some sort of mechanism for verifying FDT files is needed.
One option would be to tack a signature on before or after the file,
processing it accordingly. But due to the nature of the FDT binary format,
it is possible to embed a signature inside the FDT itself, which is very
convenient.

This series provides a tool, fdt_sign, which can add a signature to an
FDT. The signature can be checked later, preventing any change to the FDT,
other than in permitted nodes (e.g. /chosen).

This series also provides a fdt_check_sign tool, used to check signatures.

Both of these tools are stand-alone do not require mkimage or FIT.

As with FIT signing, multiple signatures are possible, but in this case
that requires that fit_sign be called once for each signature. To make the
check fail if a signature does not match, it should be marked as
'required' using the -r flag to fdt_sign.

Run-time support for checking FDT signatures could be added to U-Boot
fairly easily, but needs further discussion as the correct plumbing needs
to be determined.

For now there is absolutely no configurability in the signature mechanism.
It would of course be possible to adjust which nodes are signed, as is
done for FIT, but that needs further discussion also. The omission of the
/chosen node is implemented in h_exclude_nodes() like this:

   if (type == FDT_IS_NODE) {
      /* Ignore the chosen node as well as /signature and subnodes */
      if (!strcmp("/chosen", data) || !strncmp("/signature", data, 10))
         return 0;
   }

Man pages are provided with example usage of the tools. Use this to view
them:

   man -l doc/fdt_check_sign.1

This series also includes various clean-ups noticed along the way, as well
as refactoring to avoid code duplication with the new tools. The last four
patches are the new code.

This series is available at u-boot-dm/fdt-sign-working :

  https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working


Simon Glass (16):
  rsa: Add debugging for failure cases
  fit_check_sign: Update help to mention the key is in a dtb
  tools: Move copyfile() into a common file
  tools: Avoid leaving extra data at the end of copied files
  tools: Improve comments in signing functions
  tools: Drop unused name in image-host
  tools: Avoid confusion between keys and signatures
  tools: Tidy up argument order in fit_config_check_sig()
  tools: Pass the key blob around
  image: Return destination node for add_verify_data() method
  tools: Pass public-key node through to caller
  tools: mkimage: Show where signatures/keys are written
  tools: Add a new tool to sign FDT blobs
  tools: Add a new tool to check FDT-blob signatures
  test: Add a test for FDT signing
  tools: Add man pages for fdt_sign and fdt_check_sign

 MAINTAINERS                      |   7 +
 boot/image-fit-sig.c             | 151 +++++++++----
 boot/image-fit.c                 |  12 +-
 common/spl/spl_fit.c             |   3 +-
 doc/fdt_check_sign.1             |  74 +++++++
 doc/fdt_sign.1                   | 111 ++++++++++
 include/image.h                  |  80 ++++++-
 include/u-boot/ecdsa.h           |   5 +-
 include/u-boot/rsa.h             |   5 +-
 lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
 lib/rsa/rsa-sign.c               |   5 +-
 lib/rsa/rsa-verify.c             |  13 +-
 test/py/tests/test_fdt_sign.py   |  83 ++++++++
 test/py/tests/test_vboot.py      |  21 +-
 test/py/tests/vboot/sign-fdt.dts |  23 ++
 test/py/tests/vboot_comm.py      |  22 ++
 tools/Makefile                   |  10 +-
 tools/fdt-host.c                 | 353 +++++++++++++++++++++++++++++++
 tools/fdt_check_sign.c           |  85 ++++++++
 tools/fdt_host.h                 |  46 ++++
 tools/fdt_sign.c                 | 210 ++++++++++++++++++
 tools/fit_check_sign.c           |   4 +-
 tools/fit_common.c               |  69 ++++++
 tools/fit_common.h               |  23 ++
 tools/fit_image.c                |  59 +-----
 tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
 tools/image-host.c               | 155 +++++++++++---
 tools/imagetool.h                |   4 +
 tools/mkimage.c                  |   4 +
 29 files changed, 1714 insertions(+), 181 deletions(-)
 create mode 100644 doc/fdt_check_sign.1
 create mode 100644 doc/fdt_sign.1
 create mode 100644 test/py/tests/test_fdt_sign.py
 create mode 100644 test/py/tests/vboot/sign-fdt.dts
 create mode 100644 test/py/tests/vboot_comm.py
 create mode 100644 tools/fdt-host.c
 create mode 100644 tools/fdt_check_sign.c
 create mode 100644 tools/fdt_sign.c
 create mode 100644 tools/image-fdt-sig.c

Comments

François Ozog Nov. 12, 2021, 7:49 p.m. UTC | #1
Hi Simon

Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org> a écrit :

> At present mkimage supports signing FITs, the standard U-Boot image type.
>
> Various people are opposed to using FIT since:
>
just to be sure: I am not one of those.

>
> a) it requires adding support for FIT into other bootloaders, notably
>    UEFI

whatever happens to FIT is entirely orthogonal to U-Boot UEFI subsystem.
FIT can evolve,  U-Boot UEFI does not have to change.

>
> b) it requires packaging a kernel in this standard U-Boot format, meaning
>    that distros must run 'mkimage' and deal with the kernel and initrd
>    being inside a FIT
>
> The kernel and initrd can be dealt with in other ways. But without FIT,
> we have no standard way of signing and grouping FDT files. Instead we must
> include them in the distro as separate files.
>
> In particular, some sort of mechanism for verifying FDT files is needed.
> One option would be to tack a signature on before or after the file,
> processing it accordingly. But due to the nature of the FDT binary format,
> it is possible to embed a signature inside the FDT itself, which is very
> convenient.
>
> This series provides a tool, fdt_sign, which can add a signature to an
> FDT. The signature can be checked later, preventing any change to the FDT,
> other than in permitted nodes (e.g. /chosen).
>
> This series also provides a fdt_check_sign tool, used to check signatures.
>
> Both of these tools are stand-alone do not require mkimage or FIT.
>
> As with FIT signing, multiple signatures are possible, but in this case
> that requires that fit_sign be called once for each signature. To make the
> check fail if a signature does not match, it should be marked as
> 'required' using the -r flag to fdt_sign.
>
> Run-time support for checking FDT signatures could be added to U-Boot
> fairly easily, but needs further discussion as the correct plumbing needs
> to be determined.
>
> For now there is absolutely no configurability in the signature mechanism.
> It would of course be possible to adjust which nodes are signed, as is
> done for FIT, but that needs further discussion also. The omission of the
> /chosen node is implemented in h_exclude_nodes() like this:
>
>    if (type == FDT_IS_NODE) {
>       /* Ignore the chosen node as well as /signature and subnodes */
>       if (!strcmp("/chosen", data) || !strncmp("/signature", data, 10))
>          return 0;
>    }
>
> Man pages are provided with example usage of the tools. Use this to view
> them:
>
>    man -l doc/fdt_check_sign.1
>
> This series also includes various clean-ups noticed along the way, as well
> as refactoring to avoid code duplication with the new tools. The last four
> patches are the new code.
>
> This series is available at u-boot-dm/fdt-sign-working :
>
>
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
>
>
> Simon Glass (16):
>   rsa: Add debugging for failure cases
>   fit_check_sign: Update help to mention the key is in a dtb
>   tools: Move copyfile() into a common file
>   tools: Avoid leaving extra data at the end of copied files
>   tools: Improve comments in signing functions
>   tools: Drop unused name in image-host
>   tools: Avoid confusion between keys and signatures
>   tools: Tidy up argument order in fit_config_check_sig()
>   tools: Pass the key blob around
>   image: Return destination node for add_verify_data() method
>   tools: Pass public-key node through to caller
>   tools: mkimage: Show where signatures/keys are written
>   tools: Add a new tool to sign FDT blobs
>   tools: Add a new tool to check FDT-blob signatures
>   test: Add a test for FDT signing
>   tools: Add man pages for fdt_sign and fdt_check_sign
>
>  MAINTAINERS                      |   7 +
>  boot/image-fit-sig.c             | 151 +++++++++----
>  boot/image-fit.c                 |  12 +-
>  common/spl/spl_fit.c             |   3 +-
>  doc/fdt_check_sign.1             |  74 +++++++
>  doc/fdt_sign.1                   | 111 ++++++++++
>  include/image.h                  |  80 ++++++-
>  include/u-boot/ecdsa.h           |   5 +-
>  include/u-boot/rsa.h             |   5 +-
>  lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
>  lib/rsa/rsa-sign.c               |   5 +-
>  lib/rsa/rsa-verify.c             |  13 +-
>  test/py/tests/test_fdt_sign.py   |  83 ++++++++
>  test/py/tests/test_vboot.py      |  21 +-
>  test/py/tests/vboot/sign-fdt.dts |  23 ++
>  test/py/tests/vboot_comm.py      |  22 ++
>  tools/Makefile                   |  10 +-
>  tools/fdt-host.c                 | 353 +++++++++++++++++++++++++++++++
>  tools/fdt_check_sign.c           |  85 ++++++++
>  tools/fdt_host.h                 |  46 ++++
>  tools/fdt_sign.c                 | 210 ++++++++++++++++++
>  tools/fit_check_sign.c           |   4 +-
>  tools/fit_common.c               |  69 ++++++
>  tools/fit_common.h               |  23 ++
>  tools/fit_image.c                |  59 +-----
>  tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
>  tools/image-host.c               | 155 +++++++++++---
>  tools/imagetool.h                |   4 +
>  tools/mkimage.c                  |   4 +
>  29 files changed, 1714 insertions(+), 181 deletions(-)
>  create mode 100644 doc/fdt_check_sign.1
>  create mode 100644 doc/fdt_sign.1
>  create mode 100644 test/py/tests/test_fdt_sign.py
>  create mode 100644 test/py/tests/vboot/sign-fdt.dts
>  create mode 100644 test/py/tests/vboot_comm.py
>  create mode 100644 tools/fdt-host.c
>  create mode 100644 tools/fdt_check_sign.c
>  create mode 100644 tools/fdt_sign.c
>  create mode 100644 tools/image-fdt-sig.c
>
> --
> 2.34.0.rc1.387.gb447b232ab-goog
>
> --
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.ozog@linaro.org | Skype: ffozog
Heinrich Schuchardt Nov. 12, 2021, 11:56 p.m. UTC | #2
On 11/12/21 20:49, François Ozog wrote:
> Hi Simon
>
> Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
> <mailto:sjg@chromium.org>> a écrit :
>
>     At present mkimage supports signing FITs, the standard U-Boot image
>     type.
>
>     Various people are opposed to using FIT since:
>
> just to be sure: I am not one of those.
>
>
>     a) it requires adding support for FIT into other bootloaders, notably
>         UEFI
>
> whatever happens to FIT is entirely orthogonal to U-Boot UEFI subsystem.
> FIT can evolve,  U-Boot UEFI does not have to change.

We already can create signed FIT images containing a UEFI binary and a
devcie-tree and launch the FIT image with the bootm command.

Cf.
CONFIG_BOOTM_EFI
test/py/tests/test_efi_fit.py
doc/usage/bootefi.rst

Simon, what are you missing?

>
>
>     b) it requires packaging a kernel in this standard U-Boot format,
>     meaning
>         that distros must run 'mkimage' and deal with the kernel and initrd
>         being inside a FIT

U-Boot tools are contained in distros like Debian and Ubuntu.
Package flash-kernel uses a script in /etc/initramfs/post-update.d/ for
a similar purpose. The same hook directory can be used to create a FIT
image with a simple bash script.

Why do we need a new tool for signing device-trees?

The real problem to solve is the protection of the private key used for
signing any file containing an initrd.

>
>     The kernel and initrd can be dealt with in other ways. But without FIT,

How can the initrd be checked without FIT?

>     we have no standard way of signing and grouping FDT files. Instead
>     we must
>     include them in the distro as separate files.
>
>     In particular, some sort of mechanism for verifying FDT files is needed.
>     One option would be to tack a signature on before or after the file,
>     processing it accordingly. But due to the nature of the FDT binary
>     format,
>     it is possible to embed a signature inside the FDT itself, which is very
>     convenient.
>
>     This series provides a tool, fdt_sign, which can add a signature to an
>     FDT. The signature can be checked later, preventing any change to
>     the FDT,
>     other than in permitted nodes (e.g. /chosen).

I am not aware of any standard defining which nodes may be changed in
the FDT fixup and which may not be changed.

How can we discover which nodes were excluded from the signature after
the signature?

>
>     This series also provides a fdt_check_sign tool, used to check
>     signatures.
>
>     Both of these tools are stand-alone do not require mkimage or FIT.
>
>     As with FIT signing, multiple signatures are possible, but in this case
>     that requires that fit_sign be called once for each signature. To
>     make the
>     check fail if a signature does not match, it should be marked as
>     'required' using the -r flag to fdt_sign.
>
>     Run-time support for checking FDT signatures could be added to U-Boot
>     fairly easily, but needs further discussion as the correct plumbing
>     needs
>     to be determined.
>
>     For now there is absolutely no configurability in the signature
>     mechanism.

I would have expected a description of what a signature looks like. I
don't wont to reverse engineer your patches.

Please, describe this in doc/develop/ and in this cover-letter.

This series should have been sent as RFC.

Best regards

Heinrich

>     It would of course be possible to adjust which nodes are signed, as is
>     done for FIT, but that needs further discussion also. The omission
>     of the
>     /chosen node is implemented in h_exclude_nodes() like this:
>
>         if (type == FDT_IS_NODE) {
>            /* Ignore the chosen node as well as /signature and subnodes */
>            if (!strcmp("/chosen", data) || !strncmp("/signature", data, 10))
>               return 0;
>         }
>
>     Man pages are provided with example usage of the tools. Use this to view
>     them:
>
>         man -l doc/fdt_check_sign.1
>
>     This series also includes various clean-ups noticed along the way,
>     as well
>     as refactoring to avoid code duplication with the new tools. The
>     last four
>     patches are the new code.
>
>     This series is available at u-boot-dm/fdt-sign-working :
>
>     https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
>     <https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working>
>
>
>     Simon Glass (16):
>        rsa: Add debugging for failure cases
>        fit_check_sign: Update help to mention the key is in a dtb
>        tools: Move copyfile() into a common file
>        tools: Avoid leaving extra data at the end of copied files
>        tools: Improve comments in signing functions
>        tools: Drop unused name in image-host
>        tools: Avoid confusion between keys and signatures
>        tools: Tidy up argument order in fit_config_check_sig()
>        tools: Pass the key blob around
>        image: Return destination node for add_verify_data() method
>        tools: Pass public-key node through to caller
>        tools: mkimage: Show where signatures/keys are written
>        tools: Add a new tool to sign FDT blobs
>        tools: Add a new tool to check FDT-blob signatures
>        test: Add a test for FDT signing
>        tools: Add man pages for fdt_sign and fdt_check_sign
>
>       MAINTAINERS                      |   7 +
>       boot/image-fit-sig.c             | 151 +++++++++----
>       boot/image-fit.c                 |  12 +-
>       common/spl/spl_fit.c             |   3 +-
>       doc/fdt_check_sign.1             |  74 +++++++
>       doc/fdt_sign.1                   | 111 ++++++++++
>       include/image.h                  |  80 ++++++-
>       include/u-boot/ecdsa.h           |   5 +-
>       include/u-boot/rsa.h             |   5 +-
>       lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
>       lib/rsa/rsa-sign.c               |   5 +-
>       lib/rsa/rsa-verify.c             |  13 +-
>       test/py/tests/test_fdt_sign.py   |  83 ++++++++
>       test/py/tests/test_vboot.py      |  21 +-
>       test/py/tests/vboot/sign-fdt.dts |  23 ++
>       test/py/tests/vboot_comm.py      |  22 ++
>       tools/Makefile                   |  10 +-
>       tools/fdt-host.c                 | 353 +++++++++++++++++++++++++++++++
>       tools/fdt_check_sign.c           |  85 ++++++++
>       tools/fdt_host.h                 |  46 ++++
>       tools/fdt_sign.c                 | 210 ++++++++++++++++++
>       tools/fit_check_sign.c           |   4 +-
>       tools/fit_common.c               |  69 ++++++
>       tools/fit_common.h               |  23 ++
>       tools/fit_image.c                |  59 +-----
>       tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
>       tools/image-host.c               | 155 +++++++++++---
>       tools/imagetool.h                |   4 +
>       tools/mkimage.c                  |   4 +
>       29 files changed, 1714 insertions(+), 181 deletions(-)
>       create mode 100644 doc/fdt_check_sign.1
>       create mode 100644 doc/fdt_sign.1
>       create mode 100644 test/py/tests/test_fdt_sign.py
>       create mode 100644 test/py/tests/vboot/sign-fdt.dts
>       create mode 100644 test/py/tests/vboot_comm.py
>       create mode 100644 tools/fdt-host.c
>       create mode 100644 tools/fdt_check_sign.c
>       create mode 100644 tools/fdt_sign.c
>       create mode 100644 tools/image-fdt-sig.c
>
>     --
>     2.34.0.rc1.387.gb447b232ab-goog
>
> --
>
> François-Frédéric Ozog | /Director Business Development/
> T: +33.67221.6485
> francois.ozog@linaro.org <mailto:francois.ozog@linaro.org> | Skype: ffozog
>
>
Simon Glass Nov. 13, 2021, 3:30 a.m. UTC | #3
Hi Heinrich,

On Fri, 12 Nov 2021 at 17:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>
>
> On 11/12/21 20:49, François Ozog wrote:
> > Hi Simon
> >
> > Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
> > <mailto:sjg@chromium.org>> a écrit :
> >
> >     At present mkimage supports signing FITs, the standard U-Boot image
> >     type.
> >
> >     Various people are opposed to using FIT since:
> >
> > just to be sure: I am not one of those.
> >
> >
> >     a) it requires adding support for FIT into other bootloaders, notably
> >         UEFI
> >
> > whatever happens to FIT is entirely orthogonal to U-Boot UEFI subsystem.
> > FIT can evolve,  U-Boot UEFI does not have to change.
>
> We already can create signed FIT images containing a UEFI binary and a
> devcie-tree and launch the FIT image with the bootm command.
>
> Cf.
> CONFIG_BOOTM_EFI
> test/py/tests/test_efi_fit.py
> doc/usage/bootefi.rst
>
> Simon, what are you missing?

Some people don't want to use FIT.

>
> >
> >
> >     b) it requires packaging a kernel in this standard U-Boot format,
> >     meaning
> >         that distros must run 'mkimage' and deal with the kernel and initrd
> >         being inside a FIT
>
> U-Boot tools are contained in distros like Debian and Ubuntu.
> Package flash-kernel uses a script in /etc/initramfs/post-update.d/ for
> a similar purpose. The same hook directory can be used to create a FIT
> image with a simple bash script.
>
> Why do we need a new tool for signing device-trees?
>
> The real problem to solve is the protection of the private key used for
> signing any file containing an initrd.

Well FIT already solves that one. Either FIT is not being used, in
which case this series is useful, or it is being used, in which case
the initrd problem is solved.

>
> >
> >     The kernel and initrd can be dealt with in other ways. But without FIT,
>
> How can the initrd be checked without FIT?

I don't know. Please check with the EFI people.

>
> >     we have no standard way of signing and grouping FDT files. Instead
> >     we must
> >     include them in the distro as separate files.
> >
> >     In particular, some sort of mechanism for verifying FDT files is needed.
> >     One option would be to tack a signature on before or after the file,
> >     processing it accordingly. But due to the nature of the FDT binary
> >     format,
> >     it is possible to embed a signature inside the FDT itself, which is very
> >     convenient.
> >
> >     This series provides a tool, fdt_sign, which can add a signature to an
> >     FDT. The signature can be checked later, preventing any change to
> >     the FDT,
> >     other than in permitted nodes (e.g. /chosen).
>
> I am not aware of any standard defining which nodes may be changed in
> the FDT fixup and which may not be changed.
>
> How can we discover which nodes were excluded from the signature after
> the signature?

There is no way at present. I decided against adding a list of signed
nodes. We can of course add whatever is useful here.

>
> >
> >     This series also provides a fdt_check_sign tool, used to check
> >     signatures.
> >
> >     Both of these tools are stand-alone do not require mkimage or FIT.
> >
> >     As with FIT signing, multiple signatures are possible, but in this case
> >     that requires that fit_sign be called once for each signature. To
> >     make the
> >     check fail if a signature does not match, it should be marked as
> >     'required' using the -r flag to fdt_sign.
> >
> >     Run-time support for checking FDT signatures could be added to U-Boot
> >     fairly easily, but needs further discussion as the correct plumbing
> >     needs
> >     to be determined.
> >
> >     For now there is absolutely no configurability in the signature
> >     mechanism.
>
> I would have expected a description of what a signature looks like. I
> don't wont to reverse engineer your patches.
>
> Please, describe this in doc/develop/ and in this cover-letter.

It is the same format as the FIT signature, an RSA signature. See here:

https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L162

>
> This series should have been sent as RFC.

The last time I did that it disappeared without trace. You can of
course make comments on any series I send.

Regards,
Simon

>
> Best regards
>
> Heinrich
>
> >     It would of course be possible to adjust which nodes are signed, as is
> >     done for FIT, but that needs further discussion also. The omission
> >     of the
> >     /chosen node is implemented in h_exclude_nodes() like this:
> >
> >         if (type == FDT_IS_NODE) {
> >            /* Ignore the chosen node as well as /signature and subnodes */
> >            if (!strcmp("/chosen", data) || !strncmp("/signature", data, 10))
> >               return 0;
> >         }
> >
> >     Man pages are provided with example usage of the tools. Use this to view
> >     them:
> >
> >         man -l doc/fdt_check_sign.1
> >
> >     This series also includes various clean-ups noticed along the way,
> >     as well
> >     as refactoring to avoid code duplication with the new tools. The
> >     last four
> >     patches are the new code.
> >
> >     This series is available at u-boot-dm/fdt-sign-working :
> >
> >     https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
> >     <https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working>
> >
> >
> >     Simon Glass (16):
> >        rsa: Add debugging for failure cases
> >        fit_check_sign: Update help to mention the key is in a dtb
> >        tools: Move copyfile() into a common file
> >        tools: Avoid leaving extra data at the end of copied files
> >        tools: Improve comments in signing functions
> >        tools: Drop unused name in image-host
> >        tools: Avoid confusion between keys and signatures
> >        tools: Tidy up argument order in fit_config_check_sig()
> >        tools: Pass the key blob around
> >        image: Return destination node for add_verify_data() method
> >        tools: Pass public-key node through to caller
> >        tools: mkimage: Show where signatures/keys are written
> >        tools: Add a new tool to sign FDT blobs
> >        tools: Add a new tool to check FDT-blob signatures
> >        test: Add a test for FDT signing
> >        tools: Add man pages for fdt_sign and fdt_check_sign
> >
> >       MAINTAINERS                      |   7 +
> >       boot/image-fit-sig.c             | 151 +++++++++----
> >       boot/image-fit.c                 |  12 +-
> >       common/spl/spl_fit.c             |   3 +-
> >       doc/fdt_check_sign.1             |  74 +++++++
> >       doc/fdt_sign.1                   | 111 ++++++++++
> >       include/image.h                  |  80 ++++++-
> >       include/u-boot/ecdsa.h           |   5 +-
> >       include/u-boot/rsa.h             |   5 +-
> >       lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
> >       lib/rsa/rsa-sign.c               |   5 +-
> >       lib/rsa/rsa-verify.c             |  13 +-
> >       test/py/tests/test_fdt_sign.py   |  83 ++++++++
> >       test/py/tests/test_vboot.py      |  21 +-
> >       test/py/tests/vboot/sign-fdt.dts |  23 ++
> >       test/py/tests/vboot_comm.py      |  22 ++
> >       tools/Makefile                   |  10 +-
> >       tools/fdt-host.c                 | 353 +++++++++++++++++++++++++++++++
> >       tools/fdt_check_sign.c           |  85 ++++++++
> >       tools/fdt_host.h                 |  46 ++++
> >       tools/fdt_sign.c                 | 210 ++++++++++++++++++
> >       tools/fit_check_sign.c           |   4 +-
> >       tools/fit_common.c               |  69 ++++++
> >       tools/fit_common.h               |  23 ++
> >       tools/fit_image.c                |  59 +-----
> >       tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
> >       tools/image-host.c               | 155 +++++++++++---
> >       tools/imagetool.h                |   4 +
> >       tools/mkimage.c                  |   4 +
> >       29 files changed, 1714 insertions(+), 181 deletions(-)
> >       create mode 100644 doc/fdt_check_sign.1
> >       create mode 100644 doc/fdt_sign.1
> >       create mode 100644 test/py/tests/test_fdt_sign.py
> >       create mode 100644 test/py/tests/vboot/sign-fdt.dts
> >       create mode 100644 test/py/tests/vboot_comm.py
> >       create mode 100644 tools/fdt-host.c
> >       create mode 100644 tools/fdt_check_sign.c
> >       create mode 100644 tools/fdt_sign.c
> >       create mode 100644 tools/image-fdt-sig.c
> >
> >     --
> >     2.34.0.rc1.387.gb447b232ab-goog
> >
> > --
> >
> > François-Frédéric Ozog | /Director Business Development/
> > T: +33.67221.6485
> > francois.ozog@linaro.org <mailto:francois.ozog@linaro.org> | Skype: ffozog
> >
> >
François Ozog Nov. 13, 2021, 10:18 a.m. UTC | #4
Hi Simon,

Le sam. 13 nov. 2021 à 04:31, Simon Glass <sjg@chromium.org> a écrit :

> Hi Heinrich,
>
> On Fri, 12 Nov 2021 at 17:02, Heinrich Schuchardt <xypron.glpk@gmx.de>
> wrote:
> >
> >
> >
> > On 11/12/21 20:49, François Ozog wrote:
> > > Hi Simon
> > >
> > > Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
> > > <mailto:sjg@chromium.org>> a écrit :
> > >
> > >     At present mkimage supports signing FITs, the standard U-Boot image
> > >     type.
> > >
> > >     Various people are opposed to using FIT since:
> > >
> > > just to be sure: I am not one of those.
> > >
> > >
> > >     a) it requires adding support for FIT into other bootloaders,
> notably
> > >         UEFI
> > >
> > > whatever happens to FIT is entirely orthogonal to U-Boot UEFI
> subsystem.
> > > FIT can evolve,  U-Boot UEFI does not have to change.
> >
> > We already can create signed FIT images containing a UEFI binary and a
> > devcie-tree and launch the FIT image with the bootm command.
> >
> > Cf.
> > CONFIG_BOOTM_EFI
> > test/py/tests/test_efi_fit.py
> > doc/usage/bootefi.rst
> >
> > Simon, what are you missing?
>
> Some people don't want to use FIT.
>
> >
> > >
> > >
> > >     b) it requires packaging a kernel in this standard U-Boot format,
> > >     meaning
> > >         that distros must run 'mkimage' and deal with the kernel and
> initrd
> > >         being inside a FIT
> >
> > U-Boot tools are contained in distros like Debian and Ubuntu.
> > Package flash-kernel uses a script in /etc/initramfs/post-update.d/ for
> > a similar purpose. The same hook directory can be used to create a FIT
> > image with a simple bash script.
> >
> > Why do we need a new tool for signing device-trees?
> >
> > The real problem to solve is the protection of the private key used for
> > signing any file containing an initrd.
>
> Well FIT already solves that one. Either FIT is not being used, in
> which case this series is useful, or it is being used, in which case
> the initrd problem is solved.
>
> >
> > >
> > >     The kernel and initrd can be dealt with in other ways. But without
> FIT,
> >
> > How can the initrd be checked without FIT?
>
> I don't know. Please check with the EFI people.
>
Ilias has made a proposal to “measure” files that do not have signatures
until files include signature.

>
> >
> > >     we have no standard way of signing and grouping FDT files. Instead
> > >     we must
> > >     include them in the distro as separate files.
> > >
> > >     In particular, some sort of mechanism for verifying FDT files is
> needed.
> > >     One option would be to tack a signature on before or after the
> file,
> > >     processing it accordingly. But due to the nature of the FDT binary
> > >     format,
> > >     it is possible to embed a signature inside the FDT itself, which
> is very
> > >     convenient.
> > >
> > >     This series provides a tool, fdt_sign, which can add a signature
> to an
> > >     FDT. The signature can be checked later, preventing any change to
> > >     the FDT,
> > >     other than in permitted nodes (e.g. /chosen).
> >

Shouldn’t devicetree.org and kernel.org folks involved in this DT signing
effort ? I believe there are parrallel efforts in this area. Or is it a
private <u-boot FDT sign>? in that case, they do not need to be involved.
Depending on the case, you may want to split the patch series in a number
of chunks.

>
> > I am not aware of any standard defining which nodes may be changed in
> > the FDT fixup and which may not be changed.
> >
> > How can we discover which nodes were excluded from the signature after
> > the signature?
>
> There is no way at present. I decided against adding a list of signed
> nodes. We can of course add whatever is useful here.
>
> >
> > >
> > >     This series also provides a fdt_check_sign tool, used to check
> > >     signatures.
> > >
> > >     Both of these tools are stand-alone do not require mkimage or FIT.
> > >
> > >     As with FIT signing, multiple signatures are possible, but in this
> case
> > >     that requires that fit_sign be called once for each signature. To
> > >     make the
> > >     check fail if a signature does not match, it should be marked as
> > >     'required' using the -r flag to fdt_sign.
> > >
> > >     Run-time support for checking FDT signatures could be added to
> U-Boot
> > >     fairly easily, but needs further discussion as the correct plumbing
> > >     needs
> > >     to be determined.
> > >
> > >     For now there is absolutely no configurability in the signature
> > >     mechanism.
> >
> > I would have expected a description of what a signature looks like. I
> > don't wont to reverse engineer your patches.
> >
> > Please, describe this in doc/develop/ and in this cover-letter.
>
> It is the same format as the FIT signature, an RSA signature. See here:
>
>
> https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L162
>
> >
> > This series should have been sent as RFC.
>
> The last time I did that it disappeared without trace. You can of
> course make comments on any series I send.
>
> Regards,
> Simon
>
> >
> > Best regards
> >
> > Heinrich
> >
> > >     It would of course be possible to adjust which nodes are signed,
> as is
> > >     done for FIT, but that needs further discussion also. The omission
> > >     of the
> > >     /chosen node is implemented in h_exclude_nodes() like this:
> > >
> > >         if (type == FDT_IS_NODE) {
> > >            /* Ignore the chosen node as well as /signature and
> subnodes */
> > >            if (!strcmp("/chosen", data) || !strncmp("/signature",
> data, 10))
> > >               return 0;
> > >         }
> > >
> > >     Man pages are provided with example usage of the tools. Use this
> to view
> > >     them:
> > >
> > >         man -l doc/fdt_check_sign.1
> > >
> > >     This series also includes various clean-ups noticed along the way,
> > >     as well
> > >     as refactoring to avoid code duplication with the new tools. The
> > >     last four
> > >     patches are the new code.
> > >
> > >     This series is available at u-boot-dm/fdt-sign-working :
> > >
> > >
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
> > >     <
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
> >
> > >
> > >
> > >     Simon Glass (16):
> > >        rsa: Add debugging for failure cases
> > >        fit_check_sign: Update help to mention the key is in a dtb
> > >        tools: Move copyfile() into a common file
> > >        tools: Avoid leaving extra data at the end of copied files
> > >        tools: Improve comments in signing functions
> > >        tools: Drop unused name in image-host
> > >        tools: Avoid confusion between keys and signatures
> > >        tools: Tidy up argument order in fit_config_check_sig()
> > >        tools: Pass the key blob around
> > >        image: Return destination node for add_verify_data() method
> > >        tools: Pass public-key node through to caller
> > >        tools: mkimage: Show where signatures/keys are written
> > >        tools: Add a new tool to sign FDT blobs
> > >        tools: Add a new tool to check FDT-blob signatures
> > >        test: Add a test for FDT signing
> > >        tools: Add man pages for fdt_sign and fdt_check_sign
> > >
> > >       MAINTAINERS                      |   7 +
> > >       boot/image-fit-sig.c             | 151 +++++++++----
> > >       boot/image-fit.c                 |  12 +-
> > >       common/spl/spl_fit.c             |   3 +-
> > >       doc/fdt_check_sign.1             |  74 +++++++
> > >       doc/fdt_sign.1                   | 111 ++++++++++
> > >       include/image.h                  |  80 ++++++-
> > >       include/u-boot/ecdsa.h           |   5 +-
> > >       include/u-boot/rsa.h             |   5 +-
> > >       lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
> > >       lib/rsa/rsa-sign.c               |   5 +-
> > >       lib/rsa/rsa-verify.c             |  13 +-
> > >       test/py/tests/test_fdt_sign.py   |  83 ++++++++
> > >       test/py/tests/test_vboot.py      |  21 +-
> > >       test/py/tests/vboot/sign-fdt.dts |  23 ++
> > >       test/py/tests/vboot_comm.py      |  22 ++
> > >       tools/Makefile                   |  10 +-
> > >       tools/fdt-host.c                 | 353
> +++++++++++++++++++++++++++++++
> > >       tools/fdt_check_sign.c           |  85 ++++++++
> > >       tools/fdt_host.h                 |  46 ++++
> > >       tools/fdt_sign.c                 | 210 ++++++++++++++++++
> > >       tools/fit_check_sign.c           |   4 +-
> > >       tools/fit_common.c               |  69 ++++++
> > >       tools/fit_common.h               |  23 ++
> > >       tools/fit_image.c                |  59 +-----
> > >       tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
> > >       tools/image-host.c               | 155 +++++++++++---
> > >       tools/imagetool.h                |   4 +
> > >       tools/mkimage.c                  |   4 +
> > >       29 files changed, 1714 insertions(+), 181 deletions(-)
> > >       create mode 100644 doc/fdt_check_sign.1
> > >       create mode 100644 doc/fdt_sign.1
> > >       create mode 100644 test/py/tests/test_fdt_sign.py
> > >       create mode 100644 test/py/tests/vboot/sign-fdt.dts
> > >       create mode 100644 test/py/tests/vboot_comm.py
> > >       create mode 100644 tools/fdt-host.c
> > >       create mode 100644 tools/fdt_check_sign.c
> > >       create mode 100644 tools/fdt_sign.c
> > >       create mode 100644 tools/image-fdt-sig.c
> > >
> > >     --
> > >     2.34.0.rc1.387.gb447b232ab-goog
> > >
> > > --
> > >
> > > François-Frédéric Ozog | /Director Business Development/
> > > T: +33.67221.6485
> > > francois.ozog@linaro.org <mailto:francois.ozog@linaro.org> | Skype:
> ffozog
> > >
> > >
>
Heinrich Schuchardt Nov. 13, 2021, 11:51 a.m. UTC | #5
On 11/13/21 04:30, Simon Glass wrote:
> Hi Heinrich,
>
> On Fri, 12 Nov 2021 at 17:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>>
>>
>> On 11/12/21 20:49, François Ozog wrote:
>>> Hi Simon
>>>
>>> Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
>>> <mailto:sjg@chromium.org>> a écrit :
>>>
>>>      At present mkimage supports signing FITs, the standard U-Boot image
>>>      type.
>>>
>>>      Various people are opposed to using FIT since:
>>>
>>> just to be sure: I am not one of those.
>>>
>>>
>>>      a) it requires adding support for FIT into other bootloaders, notably
>>>          UEFI
>>>
>>> whatever happens to FIT is entirely orthogonal to U-Boot UEFI subsystem.
>>> FIT can evolve,  U-Boot UEFI does not have to change.
>>
>> We already can create signed FIT images containing a UEFI binary and a
>> devcie-tree and launch the FIT image with the bootm command.
>>
>> Cf.
>> CONFIG_BOOTM_EFI
>> test/py/tests/test_efi_fit.py
>> doc/usage/bootefi.rst
>>
>> Simon, what are you missing?
>
> Some people don't want to use FIT.

The problem with FIT is that other firmware like EDK II does not support it.

Why do you expect those people to like your new tool better?

>
>>
>>>
>>>
>>>      b) it requires packaging a kernel in this standard U-Boot format,
>>>      meaning
>>>          that distros must run 'mkimage' and deal with the kernel and initrd
>>>          being inside a FIT
>>
>> U-Boot tools are contained in distros like Debian and Ubuntu.
>> Package flash-kernel uses a script in /etc/initramfs/post-update.d/ for
>> a similar purpose. The same hook directory can be used to create a FIT
>> image with a simple bash script.
>>
>> Why do we need a new tool for signing device-trees?
>>
>> The real problem to solve is the protection of the private key used for
>> signing any file containing an initrd.
>
> Well FIT already solves that one. Either FIT is not being used, in
> which case this series is useful, or it is being used, in which case
> the initrd problem is solved.

The question was:

How do you protect the private key used by Linux to sign the FIT image
with the updated initrd generated by intramfs-tools?

Best regards

Heinrich

>
>>
>>>
>>>      The kernel and initrd can be dealt with in other ways. But without FIT,
>>
>> How can the initrd be checked without FIT?
>
> I don't know. Please check with the EFI people.
>
>>
>>>      we have no standard way of signing and grouping FDT files. Instead
>>>      we must
>>>      include them in the distro as separate files.
>>>
>>>      In particular, some sort of mechanism for verifying FDT files is needed.
>>>      One option would be to tack a signature on before or after the file,
>>>      processing it accordingly. But due to the nature of the FDT binary
>>>      format,
>>>      it is possible to embed a signature inside the FDT itself, which is very
>>>      convenient.
>>>
>>>      This series provides a tool, fdt_sign, which can add a signature to an
>>>      FDT. The signature can be checked later, preventing any change to
>>>      the FDT,
>>>      other than in permitted nodes (e.g. /chosen).
>>
>> I am not aware of any standard defining which nodes may be changed in
>> the FDT fixup and which may not be changed.
>>
>> How can we discover which nodes were excluded from the signature after
>> the signature?
>
> There is no way at present. I decided against adding a list of signed
> nodes. We can of course add whatever is useful here.
>
>>
>>>
>>>      This series also provides a fdt_check_sign tool, used to check
>>>      signatures.
>>>
>>>      Both of these tools are stand-alone do not require mkimage or FIT.
>>>
>>>      As with FIT signing, multiple signatures are possible, but in this case
>>>      that requires that fit_sign be called once for each signature. To
>>>      make the
>>>      check fail if a signature does not match, it should be marked as
>>>      'required' using the -r flag to fdt_sign.
>>>
>>>      Run-time support for checking FDT signatures could be added to U-Boot
>>>      fairly easily, but needs further discussion as the correct plumbing
>>>      needs
>>>      to be determined.
>>>
>>>      For now there is absolutely no configurability in the signature
>>>      mechanism.
>>
>> I would have expected a description of what a signature looks like. I
>> don't wont to reverse engineer your patches.
>>
>> Please, describe this in doc/develop/ and in this cover-letter.
>
> It is the same format as the FIT signature, an RSA signature. See here:
>
> https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L162
>
>>
>> This series should have been sent as RFC.
>
> The last time I did that it disappeared without trace. You can of
> course make comments on any series I send.
>
> Regards,
> Simon
>
>>
>> Best regards
>>
>> Heinrich
>>
>>>      It would of course be possible to adjust which nodes are signed, as is
>>>      done for FIT, but that needs further discussion also. The omission
>>>      of the
>>>      /chosen node is implemented in h_exclude_nodes() like this:
>>>
>>>          if (type == FDT_IS_NODE) {
>>>             /* Ignore the chosen node as well as /signature and subnodes */
>>>             if (!strcmp("/chosen", data) || !strncmp("/signature", data, 10))
>>>                return 0;
>>>          }
>>>
>>>      Man pages are provided with example usage of the tools. Use this to view
>>>      them:
>>>
>>>          man -l doc/fdt_check_sign.1
>>>
>>>      This series also includes various clean-ups noticed along the way,
>>>      as well
>>>      as refactoring to avoid code duplication with the new tools. The
>>>      last four
>>>      patches are the new code.
>>>
>>>      This series is available at u-boot-dm/fdt-sign-working :
>>>
>>>      https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
>>>      <https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working>
>>>
>>>
>>>      Simon Glass (16):
>>>         rsa: Add debugging for failure cases
>>>         fit_check_sign: Update help to mention the key is in a dtb
>>>         tools: Move copyfile() into a common file
>>>         tools: Avoid leaving extra data at the end of copied files
>>>         tools: Improve comments in signing functions
>>>         tools: Drop unused name in image-host
>>>         tools: Avoid confusion between keys and signatures
>>>         tools: Tidy up argument order in fit_config_check_sig()
>>>         tools: Pass the key blob around
>>>         image: Return destination node for add_verify_data() method
>>>         tools: Pass public-key node through to caller
>>>         tools: mkimage: Show where signatures/keys are written
>>>         tools: Add a new tool to sign FDT blobs
>>>         tools: Add a new tool to check FDT-blob signatures
>>>         test: Add a test for FDT signing
>>>         tools: Add man pages for fdt_sign and fdt_check_sign
>>>
>>>        MAINTAINERS                      |   7 +
>>>        boot/image-fit-sig.c             | 151 +++++++++----
>>>        boot/image-fit.c                 |  12 +-
>>>        common/spl/spl_fit.c             |   3 +-
>>>        doc/fdt_check_sign.1             |  74 +++++++
>>>        doc/fdt_sign.1                   | 111 ++++++++++
>>>        include/image.h                  |  80 ++++++-
>>>        include/u-boot/ecdsa.h           |   5 +-
>>>        include/u-boot/rsa.h             |   5 +-
>>>        lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
>>>        lib/rsa/rsa-sign.c               |   5 +-
>>>        lib/rsa/rsa-verify.c             |  13 +-
>>>        test/py/tests/test_fdt_sign.py   |  83 ++++++++
>>>        test/py/tests/test_vboot.py      |  21 +-
>>>        test/py/tests/vboot/sign-fdt.dts |  23 ++
>>>        test/py/tests/vboot_comm.py      |  22 ++
>>>        tools/Makefile                   |  10 +-
>>>        tools/fdt-host.c                 | 353 +++++++++++++++++++++++++++++++
>>>        tools/fdt_check_sign.c           |  85 ++++++++
>>>        tools/fdt_host.h                 |  46 ++++
>>>        tools/fdt_sign.c                 | 210 ++++++++++++++++++
>>>        tools/fit_check_sign.c           |   4 +-
>>>        tools/fit_common.c               |  69 ++++++
>>>        tools/fit_common.h               |  23 ++
>>>        tools/fit_image.c                |  59 +-----
>>>        tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
>>>        tools/image-host.c               | 155 +++++++++++---
>>>        tools/imagetool.h                |   4 +
>>>        tools/mkimage.c                  |   4 +
>>>        29 files changed, 1714 insertions(+), 181 deletions(-)
>>>        create mode 100644 doc/fdt_check_sign.1
>>>        create mode 100644 doc/fdt_sign.1
>>>        create mode 100644 test/py/tests/test_fdt_sign.py
>>>        create mode 100644 test/py/tests/vboot/sign-fdt.dts
>>>        create mode 100644 test/py/tests/vboot_comm.py
>>>        create mode 100644 tools/fdt-host.c
>>>        create mode 100644 tools/fdt_check_sign.c
>>>        create mode 100644 tools/fdt_sign.c
>>>        create mode 100644 tools/image-fdt-sig.c
>>>
>>>      --
>>>      2.34.0.rc1.387.gb447b232ab-goog
>>>
>>> --
>>>
>>> François-Frédéric Ozog | /Director Business Development/
>>> T: +33.67221.6485
>>> francois.ozog@linaro.org <mailto:francois.ozog@linaro.org> | Skype: ffozog
>>>
>>>
Simon Glass Nov. 13, 2021, 1:57 p.m. UTC | #6
Hi François,

On Sat, 13 Nov 2021 at 03:18, François Ozog <francois.ozog@linaro.org> wrote:
>
> Hi Simon,
>
> Le sam. 13 nov. 2021 à 04:31, Simon Glass <sjg@chromium.org> a écrit :
>>
>> Hi Heinrich,
>>
>> On Fri, 12 Nov 2021 at 17:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> >
>> >
>> >
>> > On 11/12/21 20:49, François Ozog wrote:
>> > > Hi Simon
>> > >
>> > > Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
>> > > <mailto:sjg@chromium.org>> a écrit :
>> > >
>> > >     At present mkimage supports signing FITs, the standard U-Boot image
>> > >     type.
>> > >
>> > >     Various people are opposed to using FIT since:
>> > >
>> > > just to be sure: I am not one of those.
>> > >
>> > >
>> > >     a) it requires adding support for FIT into other bootloaders, notably
>> > >         UEFI
>> > >
>> > > whatever happens to FIT is entirely orthogonal to U-Boot UEFI subsystem.
>> > > FIT can evolve,  U-Boot UEFI does not have to change.
>> >
>> > We already can create signed FIT images containing a UEFI binary and a
>> > devcie-tree and launch the FIT image with the bootm command.
>> >
>> > Cf.
>> > CONFIG_BOOTM_EFI
>> > test/py/tests/test_efi_fit.py
>> > doc/usage/bootefi.rst
>> >
>> > Simon, what are you missing?
>>
>> Some people don't want to use FIT.
>>
>> >
>> > >
>> > >
>> > >     b) it requires packaging a kernel in this standard U-Boot format,
>> > >     meaning
>> > >         that distros must run 'mkimage' and deal with the kernel and initrd
>> > >         being inside a FIT
>> >
>> > U-Boot tools are contained in distros like Debian and Ubuntu.
>> > Package flash-kernel uses a script in /etc/initramfs/post-update.d/ for
>> > a similar purpose. The same hook directory can be used to create a FIT
>> > image with a simple bash script.
>> >
>> > Why do we need a new tool for signing device-trees?
>> >
>> > The real problem to solve is the protection of the private key used for
>> > signing any file containing an initrd.
>>
>> Well FIT already solves that one. Either FIT is not being used, in
>> which case this series is useful, or it is being used, in which case
>> the initrd problem is solved.
>>
>> >
>> > >
>> > >     The kernel and initrd can be dealt with in other ways. But without FIT,
>> >
>> > How can the initrd be checked without FIT?
>>
>> I don't know. Please check with the EFI people.
>
> Ilias has made a proposal to “measure” files that do not have signatures until files include signature.

OK

>>
>>
>> >
>> > >     we have no standard way of signing and grouping FDT files. Instead
>> > >     we must
>> > >     include them in the distro as separate files.
>> > >
>> > >     In particular, some sort of mechanism for verifying FDT files is needed.
>> > >     One option would be to tack a signature on before or after the file,
>> > >     processing it accordingly. But due to the nature of the FDT binary
>> > >     format,
>> > >     it is possible to embed a signature inside the FDT itself, which is very
>> > >     convenient.
>> > >
>> > >     This series provides a tool, fdt_sign, which can add a signature to an
>> > >     FDT. The signature can be checked later, preventing any change to
>> > >     the FDT,
>> > >     other than in permitted nodes (e.g. /chosen).
>> >
>
> Shouldn’t devicetree.org and kernel.org folks involved in this DT signing effort ? I believe there are parrallel efforts in this area. Or is it a private <u-boot FDT sign>? in that case, they do not need to be involved. Depending on the case, you may want to split the patch series in a number of chunks.

Yes indeed these people should be involved, but I also think that
interested parties should be part of the boot-architecture effort. The
meeting notes where this tool was discussed are here:

https://lists.linaro.org/pipermail/boot-architecture/2021-October/001955.html

My slides are available there, too.

Here is what I have done so far: I sent a v4 series to
devicetree-compiler a week ago with the fdt_region and fdtgrep parts.
Rob and Bill are copied on this series, as you can see. I was not sure
about sending U-Boot patches to the boot-architecture list, but I will
cover it in a future meeting.

But if there are others that should be copied, please feel free to add
them, or point me to them.

Regards,
Simon


>>
>>
>> > I am not aware of any standard defining which nodes may be changed in
>> > the FDT fixup and which may not be changed.
>> >
>> > How can we discover which nodes were excluded from the signature after
>> > the signature?
>>
>> There is no way at present. I decided against adding a list of signed
>> nodes. We can of course add whatever is useful here.
>>
>> >
>> > >
>> > >     This series also provides a fdt_check_sign tool, used to check
>> > >     signatures.
>> > >
>> > >     Both of these tools are stand-alone do not require mkimage or FIT.
>> > >
>> > >     As with FIT signing, multiple signatures are possible, but in this case
>> > >     that requires that fit_sign be called once for each signature. To
>> > >     make the
>> > >     check fail if a signature does not match, it should be marked as
>> > >     'required' using the -r flag to fdt_sign.
>> > >
>> > >     Run-time support for checking FDT signatures could be added to U-Boot
>> > >     fairly easily, but needs further discussion as the correct plumbing
>> > >     needs
>> > >     to be determined.
>> > >
>> > >     For now there is absolutely no configurability in the signature
>> > >     mechanism.
>> >
>> > I would have expected a description of what a signature looks like. I
>> > don't wont to reverse engineer your patches.
>> >
>> > Please, describe this in doc/develop/ and in this cover-letter.
>>
>> It is the same format as the FIT signature, an RSA signature. See here:
>>
>> https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L162
>>
>> >
>> > This series should have been sent as RFC.
>>
>> The last time I did that it disappeared without trace. You can of
>> course make comments on any series I send.
>>
>> Regards,
>> Simon
>>
>> >
>> > Best regards
>> >
>> > Heinrich
>> >
>> > >     It would of course be possible to adjust which nodes are signed, as is
>> > >     done for FIT, but that needs further discussion also. The omission
>> > >     of the
>> > >     /chosen node is implemented in h_exclude_nodes() like this:
>> > >
>> > >         if (type == FDT_IS_NODE) {
>> > >            /* Ignore the chosen node as well as /signature and subnodes */
>> > >            if (!strcmp("/chosen", data) || !strncmp("/signature", data, 10))
>> > >               return 0;
>> > >         }
>> > >
>> > >     Man pages are provided with example usage of the tools. Use this to view
>> > >     them:
>> > >
>> > >         man -l doc/fdt_check_sign.1
>> > >
>> > >     This series also includes various clean-ups noticed along the way,
>> > >     as well
>> > >     as refactoring to avoid code duplication with the new tools. The
>> > >     last four
>> > >     patches are the new code.
>> > >
>> > >     This series is available at u-boot-dm/fdt-sign-working :
>> > >
>> > >     https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
>> > >     <https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working>
>> > >
>> > >
>> > >     Simon Glass (16):
>> > >        rsa: Add debugging for failure cases
>> > >        fit_check_sign: Update help to mention the key is in a dtb
>> > >        tools: Move copyfile() into a common file
>> > >        tools: Avoid leaving extra data at the end of copied files
>> > >        tools: Improve comments in signing functions
>> > >        tools: Drop unused name in image-host
>> > >        tools: Avoid confusion between keys and signatures
>> > >        tools: Tidy up argument order in fit_config_check_sig()
>> > >        tools: Pass the key blob around
>> > >        image: Return destination node for add_verify_data() method
>> > >        tools: Pass public-key node through to caller
>> > >        tools: mkimage: Show where signatures/keys are written
>> > >        tools: Add a new tool to sign FDT blobs
>> > >        tools: Add a new tool to check FDT-blob signatures
>> > >        test: Add a test for FDT signing
>> > >        tools: Add man pages for fdt_sign and fdt_check_sign
>> > >
>> > >       MAINTAINERS                      |   7 +
>> > >       boot/image-fit-sig.c             | 151 +++++++++----
>> > >       boot/image-fit.c                 |  12 +-
>> > >       common/spl/spl_fit.c             |   3 +-
>> > >       doc/fdt_check_sign.1             |  74 +++++++
>> > >       doc/fdt_sign.1                   | 111 ++++++++++
>> > >       include/image.h                  |  80 ++++++-
>> > >       include/u-boot/ecdsa.h           |   5 +-
>> > >       include/u-boot/rsa.h             |   5 +-
>> > >       lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
>> > >       lib/rsa/rsa-sign.c               |   5 +-
>> > >       lib/rsa/rsa-verify.c             |  13 +-
>> > >       test/py/tests/test_fdt_sign.py   |  83 ++++++++
>> > >       test/py/tests/test_vboot.py      |  21 +-
>> > >       test/py/tests/vboot/sign-fdt.dts |  23 ++
>> > >       test/py/tests/vboot_comm.py      |  22 ++
>> > >       tools/Makefile                   |  10 +-
>> > >       tools/fdt-host.c                 | 353 +++++++++++++++++++++++++++++++
>> > >       tools/fdt_check_sign.c           |  85 ++++++++
>> > >       tools/fdt_host.h                 |  46 ++++
>> > >       tools/fdt_sign.c                 | 210 ++++++++++++++++++
>> > >       tools/fit_check_sign.c           |   4 +-
>> > >       tools/fit_common.c               |  69 ++++++
>> > >       tools/fit_common.h               |  23 ++
>> > >       tools/fit_image.c                |  59 +-----
>> > >       tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
>> > >       tools/image-host.c               | 155 +++++++++++---
>> > >       tools/imagetool.h                |   4 +
>> > >       tools/mkimage.c                  |   4 +
>> > >       29 files changed, 1714 insertions(+), 181 deletions(-)
>> > >       create mode 100644 doc/fdt_check_sign.1
>> > >       create mode 100644 doc/fdt_sign.1
>> > >       create mode 100644 test/py/tests/test_fdt_sign.py
>> > >       create mode 100644 test/py/tests/vboot/sign-fdt.dts
>> > >       create mode 100644 test/py/tests/vboot_comm.py
>> > >       create mode 100644 tools/fdt-host.c
>> > >       create mode 100644 tools/fdt_check_sign.c
>> > >       create mode 100644 tools/fdt_sign.c
>> > >       create mode 100644 tools/image-fdt-sig.c
>> > >
>> > >     --
>> > >     2.34.0.rc1.387.gb447b232ab-goog
>> > >
>> > > --
>> > >
>> > > François-Frédéric Ozog | /Director Business Development/
>> > > T: +33.67221.6485
>> > > francois.ozog@linaro.org <mailto:francois.ozog@linaro.org> | Skype: ffozog
>> > >
>> > >
>
> --
> François-Frédéric Ozog | Director Business Development
> T: +33.67221.6485
> francois.ozog@linaro.org | Skype: ffozog
>
Simon Glass Nov. 13, 2021, 1:57 p.m. UTC | #7
Hi Heinrich,

On Sat, 13 Nov 2021 at 04:57, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>
>
> On 11/13/21 04:30, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Fri, 12 Nov 2021 at 17:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>
> >>
> >>
> >> On 11/12/21 20:49, François Ozog wrote:
> >>> Hi Simon
> >>>
> >>> Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
> >>> <mailto:sjg@chromium.org>> a écrit :
> >>>
> >>>      At present mkimage supports signing FITs, the standard U-Boot image
> >>>      type.
> >>>
> >>>      Various people are opposed to using FIT since:
> >>>
> >>> just to be sure: I am not one of those.
> >>>
> >>>
> >>>      a) it requires adding support for FIT into other bootloaders, notably
> >>>          UEFI
> >>>
> >>> whatever happens to FIT is entirely orthogonal to U-Boot UEFI subsystem.
> >>> FIT can evolve,  U-Boot UEFI does not have to change.
> >>
> >> We already can create signed FIT images containing a UEFI binary and a
> >> devcie-tree and launch the FIT image with the bootm command.
> >>
> >> Cf.
> >> CONFIG_BOOTM_EFI
> >> test/py/tests/test_efi_fit.py
> >> doc/usage/bootefi.rst
> >>
> >> Simon, what are you missing?
> >
> > Some people don't want to use FIT.
>
> The problem with FIT is that other firmware like EDK II does not support it.
>
> Why do you expect those people to like your new tool better?

I believe the EDK decision is not so much that it *does* not support
FIT, which is after all not a lot of code, but that it *should* not.
If I have that wrong, please let me know.

The goal here is to support signing in an FDT without FIT. I believe
EDK supports FDT, at least.

>
> >
> >>
> >>>
> >>>
> >>>      b) it requires packaging a kernel in this standard U-Boot format,
> >>>      meaning
> >>>          that distros must run 'mkimage' and deal with the kernel and initrd
> >>>          being inside a FIT
> >>
> >> U-Boot tools are contained in distros like Debian and Ubuntu.
> >> Package flash-kernel uses a script in /etc/initramfs/post-update.d/ for
> >> a similar purpose. The same hook directory can be used to create a FIT
> >> image with a simple bash script.
> >>
> >> Why do we need a new tool for signing device-trees?
> >>
> >> The real problem to solve is the protection of the private key used for
> >> signing any file containing an initrd.
> >
> > Well FIT already solves that one. Either FIT is not being used, in
> > which case this series is useful, or it is being used, in which case
> > the initrd problem is solved.
>
> The question was:
>
> How do you protect the private key used by Linux to sign the FIT image
> with the updated initrd generated by intramfs-tools?

Well, if it is a FIT we can add a seperate signature at the time the
initramfs-tools runs. It does support multiple signatures. If that is
not suitable I am sure something else can be devised. What are the
constraints / requirements?

Regards,
Simon

>
> Best regards
>
> Heinrich
>
> >
> >>
> >>>
> >>>      The kernel and initrd can be dealt with in other ways. But without FIT,
> >>
> >> How can the initrd be checked without FIT?
> >
> > I don't know. Please check with the EFI people.
> >
> >>
> >>>      we have no standard way of signing and grouping FDT files. Instead
> >>>      we must
> >>>      include them in the distro as separate files.
> >>>
> >>>      In particular, some sort of mechanism for verifying FDT files is needed.
> >>>      One option would be to tack a signature on before or after the file,
> >>>      processing it accordingly. But due to the nature of the FDT binary
> >>>      format,
> >>>      it is possible to embed a signature inside the FDT itself, which is very
> >>>      convenient.
> >>>
> >>>      This series provides a tool, fdt_sign, which can add a signature to an
> >>>      FDT. The signature can be checked later, preventing any change to
> >>>      the FDT,
> >>>      other than in permitted nodes (e.g. /chosen).
> >>
> >> I am not aware of any standard defining which nodes may be changed in
> >> the FDT fixup and which may not be changed.
> >>
> >> How can we discover which nodes were excluded from the signature after
> >> the signature?
> >
> > There is no way at present. I decided against adding a list of signed
> > nodes. We can of course add whatever is useful here.
> >
> >>
> >>>
> >>>      This series also provides a fdt_check_sign tool, used to check
> >>>      signatures.
> >>>
> >>>      Both of these tools are stand-alone do not require mkimage or FIT.
> >>>
> >>>      As with FIT signing, multiple signatures are possible, but in this case
> >>>      that requires that fit_sign be called once for each signature. To
> >>>      make the
> >>>      check fail if a signature does not match, it should be marked as
> >>>      'required' using the -r flag to fdt_sign.
> >>>
> >>>      Run-time support for checking FDT signatures could be added to U-Boot
> >>>      fairly easily, but needs further discussion as the correct plumbing
> >>>      needs
> >>>      to be determined.
> >>>
> >>>      For now there is absolutely no configurability in the signature
> >>>      mechanism.
> >>
> >> I would have expected a description of what a signature looks like. I
> >> don't wont to reverse engineer your patches.
> >>
> >> Please, describe this in doc/develop/ and in this cover-letter.
> >
> > It is the same format as the FIT signature, an RSA signature. See here:
> >
> > https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L162
> >
> >>
> >> This series should have been sent as RFC.
> >
> > The last time I did that it disappeared without trace. You can of
> > course make comments on any series I send.
> >
> > Regards,
> > Simon
> >
> >>
> >> Best regards
> >>
> >> Heinrich
> >>
> >>>      It would of course be possible to adjust which nodes are signed, as is
> >>>      done for FIT, but that needs further discussion also. The omission
> >>>      of the
> >>>      /chosen node is implemented in h_exclude_nodes() like this:
> >>>
> >>>          if (type == FDT_IS_NODE) {
> >>>             /* Ignore the chosen node as well as /signature and subnodes */
> >>>             if (!strcmp("/chosen", data) || !strncmp("/signature", data, 10))
> >>>                return 0;
> >>>          }
> >>>
> >>>      Man pages are provided with example usage of the tools. Use this to view
> >>>      them:
> >>>
> >>>          man -l doc/fdt_check_sign.1
> >>>
> >>>      This series also includes various clean-ups noticed along the way,
> >>>      as well
> >>>      as refactoring to avoid code duplication with the new tools. The
> >>>      last four
> >>>      patches are the new code.
> >>>
> >>>      This series is available at u-boot-dm/fdt-sign-working :
> >>>
> >>>      https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
> >>>      <https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working>
> >>>
> >>>
> >>>      Simon Glass (16):
> >>>         rsa: Add debugging for failure cases
> >>>         fit_check_sign: Update help to mention the key is in a dtb
> >>>         tools: Move copyfile() into a common file
> >>>         tools: Avoid leaving extra data at the end of copied files
> >>>         tools: Improve comments in signing functions
> >>>         tools: Drop unused name in image-host
> >>>         tools: Avoid confusion between keys and signatures
> >>>         tools: Tidy up argument order in fit_config_check_sig()
> >>>         tools: Pass the key blob around
> >>>         image: Return destination node for add_verify_data() method
> >>>         tools: Pass public-key node through to caller
> >>>         tools: mkimage: Show where signatures/keys are written
> >>>         tools: Add a new tool to sign FDT blobs
> >>>         tools: Add a new tool to check FDT-blob signatures
> >>>         test: Add a test for FDT signing
> >>>         tools: Add man pages for fdt_sign and fdt_check_sign
> >>>
> >>>        MAINTAINERS                      |   7 +
> >>>        boot/image-fit-sig.c             | 151 +++++++++----
> >>>        boot/image-fit.c                 |  12 +-
> >>>        common/spl/spl_fit.c             |   3 +-
> >>>        doc/fdt_check_sign.1             |  74 +++++++
> >>>        doc/fdt_sign.1                   | 111 ++++++++++
> >>>        include/image.h                  |  80 ++++++-
> >>>        include/u-boot/ecdsa.h           |   5 +-
> >>>        include/u-boot/rsa.h             |   5 +-
> >>>        lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
> >>>        lib/rsa/rsa-sign.c               |   5 +-
> >>>        lib/rsa/rsa-verify.c             |  13 +-
> >>>        test/py/tests/test_fdt_sign.py   |  83 ++++++++
> >>>        test/py/tests/test_vboot.py      |  21 +-
> >>>        test/py/tests/vboot/sign-fdt.dts |  23 ++
> >>>        test/py/tests/vboot_comm.py      |  22 ++
> >>>        tools/Makefile                   |  10 +-
> >>>        tools/fdt-host.c                 | 353 +++++++++++++++++++++++++++++++
> >>>        tools/fdt_check_sign.c           |  85 ++++++++
> >>>        tools/fdt_host.h                 |  46 ++++
> >>>        tools/fdt_sign.c                 | 210 ++++++++++++++++++
> >>>        tools/fit_check_sign.c           |   4 +-
> >>>        tools/fit_common.c               |  69 ++++++
> >>>        tools/fit_common.h               |  23 ++
> >>>        tools/fit_image.c                |  59 +-----
> >>>        tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
> >>>        tools/image-host.c               | 155 +++++++++++---
> >>>        tools/imagetool.h                |   4 +
> >>>        tools/mkimage.c                  |   4 +
> >>>        29 files changed, 1714 insertions(+), 181 deletions(-)
> >>>        create mode 100644 doc/fdt_check_sign.1
> >>>        create mode 100644 doc/fdt_sign.1
> >>>        create mode 100644 test/py/tests/test_fdt_sign.py
> >>>        create mode 100644 test/py/tests/vboot/sign-fdt.dts
> >>>        create mode 100644 test/py/tests/vboot_comm.py
> >>>        create mode 100644 tools/fdt-host.c
> >>>        create mode 100644 tools/fdt_check_sign.c
> >>>        create mode 100644 tools/fdt_sign.c
> >>>        create mode 100644 tools/image-fdt-sig.c
> >>>
> >>>      --
> >>>      2.34.0.rc1.387.gb447b232ab-goog
> >>>
> >>> --
> >>>
> >>> François-Frédéric Ozog | /Director Business Development/
> >>> T: +33.67221.6485
> >>> francois.ozog@linaro.org <mailto:francois.ozog@linaro.org> | Skype: ffozog
> >>>
> >>>
François Ozog Nov. 13, 2021, 6:41 p.m. UTC | #8
Hi Simon

Le sam. 13 nov. 2021 à 14:57, Simon Glass <sjg@chromium.org> a écrit :

> Hi François,
> On Sat, 13 Nov 2021 at 03:18, François Ozog <francois.ozog@linaro.org>
> wrote:
> >
> > Hi Simon,
> >
> > Le sam. 13 nov. 2021 à 04:31, Simon Glass <sjg@chromium.org> a écrit :
> >>
> >> Hi Heinrich,
> >>
> >> On Fri, 12 Nov 2021 at 17:02, Heinrich Schuchardt <xypron.glpk@gmx.de>
> wrote:
> >> >
> >> >
> >> >
> >> > On 11/12/21 20:49, François Ozog wrote:
> >> > > Hi Simon
> >> > >
> >> > > Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
> >> > > <mailto:sjg@chromium.org>> a écrit :
> >> > >
> >> > >     At present mkimage supports signing FITs, the standard U-Boot
> image
> >> > >     type.
> >> > >
> >> > >     Various people are opposed to using FIT since:
> >> > >
> >> > > just to be sure: I am not one of those.
> >> > >
> >> > >
> >> > >     a) it requires adding support for FIT into other bootloaders,
> notably
> >> > >         UEFI
> >> > >
> >> > > whatever happens to FIT is entirely orthogonal to U-Boot UEFI
> subsystem.
> >> > > FIT can evolve,  U-Boot UEFI does not have to change.
> >> >
> >> > We already can create signed FIT images containing a UEFI binary and a
> >> > devcie-tree and launch the FIT image with the bootm command.
> >> >
> >> > Cf.
> >> > CONFIG_BOOTM_EFI
> >> > test/py/tests/test_efi_fit.py
> >> > doc/usage/bootefi.rst
> >> >
> >> > Simon, what are you missing?
> >>
> >> Some people don't want to use FIT.
> >>
> >> >
> >> > >
> >> > >
> >> > >     b) it requires packaging a kernel in this standard U-Boot
> format,
> >> > >     meaning
> >> > >         that distros must run 'mkimage' and deal with the kernel
> and initrd
> >> > >         being inside a FIT
> >> >
> >> > U-Boot tools are contained in distros like Debian and Ubuntu.
> >> > Package flash-kernel uses a script in /etc/initramfs/post-update.d/
> for
> >> > a similar purpose. The same hook directory can be used to create a FIT
> >> > image with a simple bash script.
> >> >
> >> > Why do we need a new tool for signing device-trees?
> >> >
> >> > The real problem to solve is the protection of the private key used
> for
> >> > signing any file containing an initrd.
> >>
> >> Well FIT already solves that one. Either FIT is not being used, in
> >> which case this series is useful, or it is being used, in which case
> >> the initrd problem is solved.
> >>
> >> >
> >> > >
> >> > >     The kernel and initrd can be dealt with in other ways. But
> without FIT,
> >> >
> >> > How can the initrd be checked without FIT?
> >>
> >> I don't know. Please check with the EFI people.
> >
> > Ilias has made a proposal to “measure” files that do not have signatures
> until files include signature.
>
> OK
>
> >>
> >>
> >> >
> >> > >     we have no standard way of signing and grouping FDT files.
> Instead
> >> > >     we must
> >> > >     include them in the distro as separate files.
> >> > >
> >> > >     In particular, some sort of mechanism for verifying FDT files
> is needed.
> >> > >     One option would be to tack a signature on before or after the
> file,
> >> > >     processing it accordingly. But due to the nature of the FDT
> binary
> >> > >     format,
> >> > >     it is possible to embed a signature inside the FDT itself,
> which is very
> >> > >     convenient.
> >> > >
> >> > >     This series provides a tool, fdt_sign, which can add a
> signature to an
> >> > >     FDT. The signature can be checked later, preventing any change
> to
> >> > >     the FDT,
> >> > >     other than in permitted nodes (e.g. /chosen).
> >> >
> >
> > Shouldn’t devicetree.org and kernel.org folks involved in this DT
> signing effort ? I believe there are parrallel efforts in this area. Or is
> it a private <u-boot FDT sign>? in that case, they do not need to be
> involved. Depending on the case, you may want to split the patch series in
> a number of chunks.
>
> Yes indeed these people should be involved, but I also think that
> interested parties should be part of the boot-architecture effort. The
> meeting notes where this tool was discussed are here:
>
>
> https://lists.linaro.org/pipermail/boot-architecture/2021-October/001955.html
>
> My slides are available there, too.
>
> Here is what I have done so far: I sent a v4 series to
> devicetree-compiler a week ago with the fdt_region and fdtgrep parts.
> Rob and Bill are copied on this series, as you can see. I was not sure
> about sending U-Boot patches to the boot-architecture list, but I will
> cover it in a future meeting.
>
> But if there are others that should be copied, please feel free to add
> them, or point me to them.
>
Sorry. I missed those. Now the reference is made that’s good.

>
> Regards,
> Simon
>
>
> >>
> >>
> >> > I am not aware of any standard defining which nodes may be changed in
> >> > the FDT fixup and which may not be changed.
> >> >
> >> > How can we discover which nodes were excluded from the signature after
> >> > the signature?
> >>
> >> There is no way at present. I decided against adding a list of signed
> >> nodes. We can of course add whatever is useful here.
> >>
> >> >
> >> > >
> >> > >     This series also provides a fdt_check_sign tool, used to check
> >> > >     signatures.
> >> > >
> >> > >     Both of these tools are stand-alone do not require mkimage or
> FIT.
> >> > >
> >> > >     As with FIT signing, multiple signatures are possible, but in
> this case
> >> > >     that requires that fit_sign be called once for each signature.
> To
> >> > >     make the
> >> > >     check fail if a signature does not match, it should be marked as
> >> > >     'required' using the -r flag to fdt_sign.
> >> > >
> >> > >     Run-time support for checking FDT signatures could be added to
> U-Boot
> >> > >     fairly easily, but needs further discussion as the correct
> plumbing
> >> > >     needs
> >> > >     to be determined.
> >> > >
> >> > >     For now there is absolutely no configurability in the signature
> >> > >     mechanism.
> >> >
> >> > I would have expected a description of what a signature looks like. I
> >> > don't wont to reverse engineer your patches.
> >> >
> >> > Please, describe this in doc/develop/ and in this cover-letter.
> >>
> >> It is the same format as the FIT signature, an RSA signature. See here:
> >>
> >>
> https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L162
> >>
> >> >
> >> > This series should have been sent as RFC.
> >>
> >> The last time I did that it disappeared without trace. You can of
> >> course make comments on any series I send.
> >>
> >> Regards,
> >> Simon
> >>
> >> >
> >> > Best regards
> >> >
> >> > Heinrich
> >> >
> >> > >     It would of course be possible to adjust which nodes are
> signed, as is
> >> > >     done for FIT, but that needs further discussion also. The
> omission
> >> > >     of the
> >> > >     /chosen node is implemented in h_exclude_nodes() like this:
> >> > >
> >> > >         if (type == FDT_IS_NODE) {
> >> > >            /* Ignore the chosen node as well as /signature and
> subnodes */
> >> > >            if (!strcmp("/chosen", data) || !strncmp("/signature",
> data, 10))
> >> > >               return 0;
> >> > >         }
> >> > >
> >> > >     Man pages are provided with example usage of the tools. Use
> this to view
> >> > >     them:
> >> > >
> >> > >         man -l doc/fdt_check_sign.1
> >> > >
> >> > >     This series also includes various clean-ups noticed along the
> way,
> >> > >     as well
> >> > >     as refactoring to avoid code duplication with the new tools. The
> >> > >     last four
> >> > >     patches are the new code.
> >> > >
> >> > >     This series is available at u-boot-dm/fdt-sign-working :
> >> > >
> >> > >
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
> >> > >     <
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
> >
> >> > >
> >> > >
> >> > >     Simon Glass (16):
> >> > >        rsa: Add debugging for failure cases
> >> > >        fit_check_sign: Update help to mention the key is in a dtb
> >> > >        tools: Move copyfile() into a common file
> >> > >        tools: Avoid leaving extra data at the end of copied files
> >> > >        tools: Improve comments in signing functions
> >> > >        tools: Drop unused name in image-host
> >> > >        tools: Avoid confusion between keys and signatures
> >> > >        tools: Tidy up argument order in fit_config_check_sig()
> >> > >        tools: Pass the key blob around
> >> > >        image: Return destination node for add_verify_data() method
> >> > >        tools: Pass public-key node through to caller
> >> > >        tools: mkimage: Show where signatures/keys are written
> >> > >        tools: Add a new tool to sign FDT blobs
> >> > >        tools: Add a new tool to check FDT-blob signatures
> >> > >        test: Add a test for FDT signing
> >> > >        tools: Add man pages for fdt_sign and fdt_check_sign
> >> > >
> >> > >       MAINTAINERS                      |   7 +
> >> > >       boot/image-fit-sig.c             | 151 +++++++++----
> >> > >       boot/image-fit.c                 |  12 +-
> >> > >       common/spl/spl_fit.c             |   3 +-
> >> > >       doc/fdt_check_sign.1             |  74 +++++++
> >> > >       doc/fdt_sign.1                   | 111 ++++++++++
> >> > >       include/image.h                  |  80 ++++++-
> >> > >       include/u-boot/ecdsa.h           |   5 +-
> >> > >       include/u-boot/rsa.h             |   5 +-
> >> > >       lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
> >> > >       lib/rsa/rsa-sign.c               |   5 +-
> >> > >       lib/rsa/rsa-verify.c             |  13 +-
> >> > >       test/py/tests/test_fdt_sign.py   |  83 ++++++++
> >> > >       test/py/tests/test_vboot.py      |  21 +-
> >> > >       test/py/tests/vboot/sign-fdt.dts |  23 ++
> >> > >       test/py/tests/vboot_comm.py      |  22 ++
> >> > >       tools/Makefile                   |  10 +-
> >> > >       tools/fdt-host.c                 | 353
> +++++++++++++++++++++++++++++++
> >> > >       tools/fdt_check_sign.c           |  85 ++++++++
> >> > >       tools/fdt_host.h                 |  46 ++++
> >> > >       tools/fdt_sign.c                 | 210 ++++++++++++++++++
> >> > >       tools/fit_check_sign.c           |   4 +-
> >> > >       tools/fit_common.c               |  69 ++++++
> >> > >       tools/fit_common.h               |  23 ++
> >> > >       tools/fit_image.c                |  59 +-----
> >> > >       tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
> >> > >       tools/image-host.c               | 155 +++++++++++---
> >> > >       tools/imagetool.h                |   4 +
> >> > >       tools/mkimage.c                  |   4 +
> >> > >       29 files changed, 1714 insertions(+), 181 deletions(-)
> >> > >       create mode 100644 doc/fdt_check_sign.1
> >> > >       create mode 100644 doc/fdt_sign.1
> >> > >       create mode 100644 test/py/tests/test_fdt_sign.py
> >> > >       create mode 100644 test/py/tests/vboot/sign-fdt.dts
> >> > >       create mode 100644 test/py/tests/vboot_comm.py
> >> > >       create mode 100644 tools/fdt-host.c
> >> > >       create mode 100644 tools/fdt_check_sign.c
> >> > >       create mode 100644 tools/fdt_sign.c
> >> > >       create mode 100644 tools/image-fdt-sig.c
> >> > >
> >> > >     --
> >> > >     2.34.0.rc1.387.gb447b232ab-goog
> >> > >
> >> > > --
> >> > >
> >> > > François-Frédéric Ozog | /Director Business Development/
> >> > > T: +33.67221.6485
> >> > > francois.ozog@linaro.org <mailto:francois.ozog@linaro.org> |
> Skype: ffozog
> >> > >
> >> > >
> >
> > --
> > François-Frédéric Ozog | Director Business Development
> > T: +33.67221.6485
> > francois.ozog@linaro.org | Skype: ffozog
> >
>
François Ozog Nov. 13, 2021, 6:53 p.m. UTC | #9
Hi Simon

Le sam. 13 nov. 2021 à 14:57, Simon Glass <sjg@chromium.org> a écrit :

> Hi Heinrich,
>
> On Sat, 13 Nov 2021 at 04:57, Heinrich Schuchardt <xypron.glpk@gmx.de>
> wrote:
> >
> >
> >
> > On 11/13/21 04:30, Simon Glass wrote:
> > > Hi Heinrich,
> > >
> > > On Fri, 12 Nov 2021 at 17:02, Heinrich Schuchardt <xypron.glpk@gmx.de>
> wrote:
> > >>
> > >>
> > >>
> > >> On 11/12/21 20:49, François Ozog wrote:
> > >>> Hi Simon
> > >>>
> > >>> Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
> > >>> <mailto:sjg@chromium.org>> a écrit :
> > >>>
> > >>>      At present mkimage supports signing FITs, the standard U-Boot
> image
> > >>>      type.
> > >>>
> > >>>      Various people are opposed to using FIT since:
> > >>>
> > >>> just to be sure: I am not one of those.
> > >>>
> > >>>
> > >>>      a) it requires adding support for FIT into other bootloaders,
> notably
> > >>>          UEFI
> > >>>
> > >>> whatever happens to FIT is entirely orthogonal to U-Boot UEFI
> subsystem.
> > >>> FIT can evolve,  U-Boot UEFI does not have to change.
> > >>
> > >> We already can create signed FIT images containing a UEFI binary and a
> > >> devcie-tree and launch the FIT image with the bootm command.
> > >>
> > >> Cf.
> > >> CONFIG_BOOTM_EFI
> > >> test/py/tests/test_efi_fit.py
> > >> doc/usage/bootefi.rst
> > >>
> > >> Simon, what are you missing?
> > >
> > > Some people don't want to use FIT.
> >
> > The problem with FIT is that other firmware like EDK II does not support
> it.
> >
> > Why do you expect those people to like your new tool better?
>
> I believe the EDK decision is not so much that it *does* not support
> FIT, which is after all not a lot of code, but that it *should* not.
> If I have that wrong, please let me know.
>
UEFI (EDK2 and now U-Boot) design choice is that it does not include *any*
technologies for OS: UEFI launches an OS loader that know about those.
A possible way to introduce a FIT is by adding a protocol that can register
a vendor media file , assign a UUID to a FIT. Add support for this in the
UEFI stub of the kernel.
Another way would be a minimalistic UEFI app that will fetch the second
parameter as a FIT. The FIT format parsing code is only in this
minimalistic loader. U-Boot can play this role based on your efforts to
have U-Boot as UEFI application.
You could also promote FIT in the systemd-boot UeFi application. I believe
this is a very good option BTW.

>
> The goal here is to support signing in an FDT without FIT. I believe
> EDK supports FDT, at least.
>
> >
> > >
> > >>
> > >>>
> > >>>
> > >>>      b) it requires packaging a kernel in this standard U-Boot
> format,
> > >>>      meaning
> > >>>          that distros must run 'mkimage' and deal with the kernel
> and initrd
> > >>>          being inside a FIT
> > >>
> > >> U-Boot tools are contained in distros like Debian and Ubuntu.
> > >> Package flash-kernel uses a script in /etc/initramfs/post-update.d/
> for
> > >> a similar purpose. The same hook directory can be used to create a FIT
> > >> image with a simple bash script.
> > >>
> > >> Why do we need a new tool for signing device-trees?
> > >>
> > >> The real problem to solve is the protection of the private key used
> for
> > >> signing any file containing an initrd.
> > >
> > > Well FIT already solves that one. Either FIT is not being used, in
> > > which case this series is useful, or it is being used, in which case
> > > the initrd problem is solved.
> >
> > The question was:
> >
> > How do you protect the private key used by Linux to sign the FIT image
> > with the updated initrd generated by intramfs-tools?
>
> Well, if it is a FIT we can add a seperate signature at the time the
> initramfs-tools runs. It does support multiple signatures. If that is
> not suitable I am sure something else can be devised. What are the
> constraints / requirements?
>
> Regards,
> Simon
>
> >
> > Best regards
> >
> > Heinrich
> >
> > >
> > >>
> > >>>
> > >>>      The kernel and initrd can be dealt with in other ways. But
> without FIT,
> > >>
> > >> How can the initrd be checked without FIT?
> > >
> > > I don't know. Please check with the EFI people.
> > >
> > >>
> > >>>      we have no standard way of signing and grouping FDT files.
> Instead
> > >>>      we must
> > >>>      include them in the distro as separate files.
> > >>>
> > >>>      In particular, some sort of mechanism for verifying FDT files
> is needed.
> > >>>      One option would be to tack a signature on before or after the
> file,
> > >>>      processing it accordingly. But due to the nature of the FDT
> binary
> > >>>      format,
> > >>>      it is possible to embed a signature inside the FDT itself,
> which is very
> > >>>      convenient.
> > >>>
> > >>>      This series provides a tool, fdt_sign, which can add a
> signature to an
> > >>>      FDT. The signature can be checked later, preventing any change
> to
> > >>>      the FDT,
> > >>>      other than in permitted nodes (e.g. /chosen).
> > >>
> > >> I am not aware of any standard defining which nodes may be changed in
> > >> the FDT fixup and which may not be changed.
> > >>
> > >> How can we discover which nodes were excluded from the signature after
> > >> the signature?
> > >
> > > There is no way at present. I decided against adding a list of signed
> > > nodes. We can of course add whatever is useful here.
> > >
> > >>
> > >>>
> > >>>      This series also provides a fdt_check_sign tool, used to check
> > >>>      signatures.
> > >>>
> > >>>      Both of these tools are stand-alone do not require mkimage or
> FIT.
> > >>>
> > >>>      As with FIT signing, multiple signatures are possible, but in
> this case
> > >>>      that requires that fit_sign be called once for each signature.
> To
> > >>>      make the
> > >>>      check fail if a signature does not match, it should be marked as
> > >>>      'required' using the -r flag to fdt_sign.
> > >>>
> > >>>      Run-time support for checking FDT signatures could be added to
> U-Boot
> > >>>      fairly easily, but needs further discussion as the correct
> plumbing
> > >>>      needs
> > >>>      to be determined.
> > >>>
> > >>>      For now there is absolutely no configurability in the signature
> > >>>      mechanism.
> > >>
> > >> I would have expected a description of what a signature looks like. I
> > >> don't wont to reverse engineer your patches.
> > >>
> > >> Please, describe this in doc/develop/ and in this cover-letter.
> > >
> > > It is the same format as the FIT signature, an RSA signature. See here:
> > >
> > >
> https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L162
> > >
> > >>
> > >> This series should have been sent as RFC.
> > >
> > > The last time I did that it disappeared without trace. You can of
> > > course make comments on any series I send.
> > >
> > > Regards,
> > > Simon
> > >
> > >>
> > >> Best regards
> > >>
> > >> Heinrich
> > >>
> > >>>      It would of course be possible to adjust which nodes are
> signed, as is
> > >>>      done for FIT, but that needs further discussion also. The
> omission
> > >>>      of the
> > >>>      /chosen node is implemented in h_exclude_nodes() like this:
> > >>>
> > >>>          if (type == FDT_IS_NODE) {
> > >>>             /* Ignore the chosen node as well as /signature and
> subnodes */
> > >>>             if (!strcmp("/chosen", data) || !strncmp("/signature",
> data, 10))
> > >>>                return 0;
> > >>>          }
> > >>>
> > >>>      Man pages are provided with example usage of the tools. Use
> this to view
> > >>>      them:
> > >>>
> > >>>          man -l doc/fdt_check_sign.1
> > >>>
> > >>>      This series also includes various clean-ups noticed along the
> way,
> > >>>      as well
> > >>>      as refactoring to avoid code duplication with the new tools. The
> > >>>      last four
> > >>>      patches are the new code.
> > >>>
> > >>>      This series is available at u-boot-dm/fdt-sign-working :
> > >>>
> > >>>
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
> > >>>      <
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
> >
> > >>>
> > >>>
> > >>>      Simon Glass (16):
> > >>>         rsa: Add debugging for failure cases
> > >>>         fit_check_sign: Update help to mention the key is in a dtb
> > >>>         tools: Move copyfile() into a common file
> > >>>         tools: Avoid leaving extra data at the end of copied files
> > >>>         tools: Improve comments in signing functions
> > >>>         tools: Drop unused name in image-host
> > >>>         tools: Avoid confusion between keys and signatures
> > >>>         tools: Tidy up argument order in fit_config_check_sig()
> > >>>         tools: Pass the key blob around
> > >>>         image: Return destination node for add_verify_data() method
> > >>>         tools: Pass public-key node through to caller
> > >>>         tools: mkimage: Show where signatures/keys are written
> > >>>         tools: Add a new tool to sign FDT blobs
> > >>>         tools: Add a new tool to check FDT-blob signatures
> > >>>         test: Add a test for FDT signing
> > >>>         tools: Add man pages for fdt_sign and fdt_check_sign
> > >>>
> > >>>        MAINTAINERS                      |   7 +
> > >>>        boot/image-fit-sig.c             | 151 +++++++++----
> > >>>        boot/image-fit.c                 |  12 +-
> > >>>        common/spl/spl_fit.c             |   3 +-
> > >>>        doc/fdt_check_sign.1             |  74 +++++++
> > >>>        doc/fdt_sign.1                   | 111 ++++++++++
> > >>>        include/image.h                  |  80 ++++++-
> > >>>        include/u-boot/ecdsa.h           |   5 +-
> > >>>        include/u-boot/rsa.h             |   5 +-
> > >>>        lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
> > >>>        lib/rsa/rsa-sign.c               |   5 +-
> > >>>        lib/rsa/rsa-verify.c             |  13 +-
> > >>>        test/py/tests/test_fdt_sign.py   |  83 ++++++++
> > >>>        test/py/tests/test_vboot.py      |  21 +-
> > >>>        test/py/tests/vboot/sign-fdt.dts |  23 ++
> > >>>        test/py/tests/vboot_comm.py      |  22 ++
> > >>>        tools/Makefile                   |  10 +-
> > >>>        tools/fdt-host.c                 | 353
> +++++++++++++++++++++++++++++++
> > >>>        tools/fdt_check_sign.c           |  85 ++++++++
> > >>>        tools/fdt_host.h                 |  46 ++++
> > >>>        tools/fdt_sign.c                 | 210 ++++++++++++++++++
> > >>>        tools/fit_check_sign.c           |   4 +-
> > >>>        tools/fit_common.c               |  69 ++++++
> > >>>        tools/fit_common.h               |  23 ++
> > >>>        tools/fit_image.c                |  59 +-----
> > >>>        tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
> > >>>        tools/image-host.c               | 155 +++++++++++---
> > >>>        tools/imagetool.h                |   4 +
> > >>>        tools/mkimage.c                  |   4 +
> > >>>        29 files changed, 1714 insertions(+), 181 deletions(-)
> > >>>        create mode 100644 doc/fdt_check_sign.1
> > >>>        create mode 100644 doc/fdt_sign.1
> > >>>        create mode 100644 test/py/tests/test_fdt_sign.py
> > >>>        create mode 100644 test/py/tests/vboot/sign-fdt.dts
> > >>>        create mode 100644 test/py/tests/vboot_comm.py
> > >>>        create mode 100644 tools/fdt-host.c
> > >>>        create mode 100644 tools/fdt_check_sign.c
> > >>>        create mode 100644 tools/fdt_sign.c
> > >>>        create mode 100644 tools/image-fdt-sig.c
> > >>>
> > >>>      --
> > >>>      2.34.0.rc1.387.gb447b232ab-goog
> > >>>
> > >>> --
> > >>>
> > >>> François-Frédéric Ozog | /Director Business Development/
> > >>> T: +33.67221.6485
> > >>> francois.ozog@linaro.org <mailto:francois.ozog@linaro.org> | Skype:
> ffozog
> > >>>
> > >>>
>
Simon Glass Nov. 25, 2021, 12:12 a.m. UTC | #10
Hi François,

On Sat, 13 Nov 2021 at 11:53, François Ozog <francois.ozog@linaro.org> wrote:
>
> Hi Simon
>
> Le sam. 13 nov. 2021 à 14:57, Simon Glass <sjg@chromium.org> a écrit :
>>
>> Hi Heinrich,
>>
>> On Sat, 13 Nov 2021 at 04:57, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> >
>> >
>> >
>> > On 11/13/21 04:30, Simon Glass wrote:
>> > > Hi Heinrich,
>> > >
>> > > On Fri, 12 Nov 2021 at 17:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> > >>
>> > >>
>> > >>
>> > >> On 11/12/21 20:49, François Ozog wrote:
>> > >>> Hi Simon
>> > >>>
>> > >>> Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
>> > >>> <mailto:sjg@chromium.org>> a écrit :
>> > >>>
>> > >>>      At present mkimage supports signing FITs, the standard U-Boot image
>> > >>>      type.
>> > >>>
>> > >>>      Various people are opposed to using FIT since:
>> > >>>
>> > >>> just to be sure: I am not one of those.
>> > >>>
>> > >>>
>> > >>>      a) it requires adding support for FIT into other bootloaders, notably
>> > >>>          UEFI
>> > >>>
>> > >>> whatever happens to FIT is entirely orthogonal to U-Boot UEFI subsystem.
>> > >>> FIT can evolve,  U-Boot UEFI does not have to change.
>> > >>
>> > >> We already can create signed FIT images containing a UEFI binary and a
>> > >> devcie-tree and launch the FIT image with the bootm command.
>> > >>
>> > >> Cf.
>> > >> CONFIG_BOOTM_EFI
>> > >> test/py/tests/test_efi_fit.py
>> > >> doc/usage/bootefi.rst
>> > >>
>> > >> Simon, what are you missing?
>> > >
>> > > Some people don't want to use FIT.
>> >
>> > The problem with FIT is that other firmware like EDK II does not support it.
>> >
>> > Why do you expect those people to like your new tool better?
>>
>> I believe the EDK decision is not so much that it *does* not support
>> FIT, which is after all not a lot of code, but that it *should* not.
>> If I have that wrong, please let me know.
>
> UEFI (EDK2 and now U-Boot) design choice is that it does not include *any* technologies for OS: UEFI launches an OS loader that know about those.
> A possible way to introduce a FIT is by adding a protocol that can register a vendor media file , assign a UUID to a FIT. Add support for this in the UEFI stub of the kernel.
> Another way would be a minimalistic UEFI app that will fetch the second parameter as a FIT. The FIT format parsing code is only in this minimalistic loader. U-Boot can play this role based on your efforts to have U-Boot as UEFI application.
> You could also promote FIT in the systemd-boot UeFi application. I believe this is a very good option BTW.

I think UEFI is making things very complicated. If we define a
standard boot flow we should not need grub.

U-Boot already supports FIT so we don't have a problem there.

Since FIT is a container format, and UEFI supports other container
formats (capsule update for example), there is no obvious
philosophical reason why UEFI should not support FIT.

Anyway, at least you can understand why I sent this patch.

Regards,
SImon


>>
>>
>> The goal here is to support signing in an FDT without FIT. I believe
>> EDK supports FDT, at least.
>>
>> >
>> > >
>> > >>
>> > >>>
>> > >>>
>> > >>>      b) it requires packaging a kernel in this standard U-Boot format,
>> > >>>      meaning
>> > >>>          that distros must run 'mkimage' and deal with the kernel and initrd
>> > >>>          being inside a FIT
>> > >>
>> > >> U-Boot tools are contained in distros like Debian and Ubuntu.
>> > >> Package flash-kernel uses a script in /etc/initramfs/post-update.d/ for
>> > >> a similar purpose. The same hook directory can be used to create a FIT
>> > >> image with a simple bash script.
>> > >>
>> > >> Why do we need a new tool for signing device-trees?
>> > >>
>> > >> The real problem to solve is the protection of the private key used for
>> > >> signing any file containing an initrd.
>> > >
>> > > Well FIT already solves that one. Either FIT is not being used, in
>> > > which case this series is useful, or it is being used, in which case
>> > > the initrd problem is solved.
>> >
>> > The question was:
>> >
>> > How do you protect the private key used by Linux to sign the FIT image
>> > with the updated initrd generated by intramfs-tools?
>>
>> Well, if it is a FIT we can add a seperate signature at the time the
>> initramfs-tools runs. It does support multiple signatures. If that is
>> not suitable I am sure something else can be devised. What are the
>> constraints / requirements?
>>
>> Regards,
>> Simon
>>
>> >
>> > Best regards
>> >
>> > Heinrich
>> >
>> > >
>> > >>
>> > >>>
>> > >>>      The kernel and initrd can be dealt with in other ways. But without FIT,
>> > >>
>> > >> How can the initrd be checked without FIT?
>> > >
>> > > I don't know. Please check with the EFI people.
>> > >
>> > >>
>> > >>>      we have no standard way of signing and grouping FDT files. Instead
>> > >>>      we must
>> > >>>      include them in the distro as separate files.
>> > >>>
>> > >>>      In particular, some sort of mechanism for verifying FDT files is needed.
>> > >>>      One option would be to tack a signature on before or after the file,
>> > >>>      processing it accordingly. But due to the nature of the FDT binary
>> > >>>      format,
>> > >>>      it is possible to embed a signature inside the FDT itself, which is very
>> > >>>      convenient.
>> > >>>
>> > >>>      This series provides a tool, fdt_sign, which can add a signature to an
>> > >>>      FDT. The signature can be checked later, preventing any change to
>> > >>>      the FDT,
>> > >>>      other than in permitted nodes (e.g. /chosen).
>> > >>
>> > >> I am not aware of any standard defining which nodes may be changed in
>> > >> the FDT fixup and which may not be changed.
>> > >>
>> > >> How can we discover which nodes were excluded from the signature after
>> > >> the signature?
>> > >
>> > > There is no way at present. I decided against adding a list of signed
>> > > nodes. We can of course add whatever is useful here.
>> > >
>> > >>
>> > >>>
>> > >>>      This series also provides a fdt_check_sign tool, used to check
>> > >>>      signatures.
>> > >>>
>> > >>>      Both of these tools are stand-alone do not require mkimage or FIT.
>> > >>>
>> > >>>      As with FIT signing, multiple signatures are possible, but in this case
>> > >>>      that requires that fit_sign be called once for each signature. To
>> > >>>      make the
>> > >>>      check fail if a signature does not match, it should be marked as
>> > >>>      'required' using the -r flag to fdt_sign.
>> > >>>
>> > >>>      Run-time support for checking FDT signatures could be added to U-Boot
>> > >>>      fairly easily, but needs further discussion as the correct plumbing
>> > >>>      needs
>> > >>>      to be determined.
>> > >>>
>> > >>>      For now there is absolutely no configurability in the signature
>> > >>>      mechanism.
>> > >>
>> > >> I would have expected a description of what a signature looks like. I
>> > >> don't wont to reverse engineer your patches.
>> > >>
>> > >> Please, describe this in doc/develop/ and in this cover-letter.
>> > >
>> > > It is the same format as the FIT signature, an RSA signature. See here:
>> > >
>> > > https://github.com/u-boot/u-boot/blob/master/doc/uImage.FIT/signature.txt#L162
>> > >
>> > >>
>> > >> This series should have been sent as RFC.
>> > >
>> > > The last time I did that it disappeared without trace. You can of
>> > > course make comments on any series I send.
>> > >
>> > > Regards,
>> > > Simon
>> > >
>> > >>
>> > >> Best regards
>> > >>
>> > >> Heinrich
>> > >>
>> > >>>      It would of course be possible to adjust which nodes are signed, as is
>> > >>>      done for FIT, but that needs further discussion also. The omission
>> > >>>      of the
>> > >>>      /chosen node is implemented in h_exclude_nodes() like this:
>> > >>>
>> > >>>          if (type == FDT_IS_NODE) {
>> > >>>             /* Ignore the chosen node as well as /signature and subnodes */
>> > >>>             if (!strcmp("/chosen", data) || !strncmp("/signature", data, 10))
>> > >>>                return 0;
>> > >>>          }
>> > >>>
>> > >>>      Man pages are provided with example usage of the tools. Use this to view
>> > >>>      them:
>> > >>>
>> > >>>          man -l doc/fdt_check_sign.1
>> > >>>
>> > >>>      This series also includes various clean-ups noticed along the way,
>> > >>>      as well
>> > >>>      as refactoring to avoid code duplication with the new tools. The
>> > >>>      last four
>> > >>>      patches are the new code.
>> > >>>
>> > >>>      This series is available at u-boot-dm/fdt-sign-working :
>> > >>>
>> > >>>      https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working
>> > >>>      <https://source.denx.de/u-boot/custodians/u-boot-dm/-/tree/fdt-sign-working>
>> > >>>
>> > >>>
>> > >>>      Simon Glass (16):
>> > >>>         rsa: Add debugging for failure cases
>> > >>>         fit_check_sign: Update help to mention the key is in a dtb
>> > >>>         tools: Move copyfile() into a common file
>> > >>>         tools: Avoid leaving extra data at the end of copied files
>> > >>>         tools: Improve comments in signing functions
>> > >>>         tools: Drop unused name in image-host
>> > >>>         tools: Avoid confusion between keys and signatures
>> > >>>         tools: Tidy up argument order in fit_config_check_sig()
>> > >>>         tools: Pass the key blob around
>> > >>>         image: Return destination node for add_verify_data() method
>> > >>>         tools: Pass public-key node through to caller
>> > >>>         tools: mkimage: Show where signatures/keys are written
>> > >>>         tools: Add a new tool to sign FDT blobs
>> > >>>         tools: Add a new tool to check FDT-blob signatures
>> > >>>         test: Add a test for FDT signing
>> > >>>         tools: Add man pages for fdt_sign and fdt_check_sign
>> > >>>
>> > >>>        MAINTAINERS                      |   7 +
>> > >>>        boot/image-fit-sig.c             | 151 +++++++++----
>> > >>>        boot/image-fit.c                 |  12 +-
>> > >>>        common/spl/spl_fit.c             |   3 +-
>> > >>>        doc/fdt_check_sign.1             |  74 +++++++
>> > >>>        doc/fdt_sign.1                   | 111 ++++++++++
>> > >>>        include/image.h                  |  80 ++++++-
>> > >>>        include/u-boot/ecdsa.h           |   5 +-
>> > >>>        include/u-boot/rsa.h             |   5 +-
>> > >>>        lib/ecdsa/ecdsa-libcrypto.c      |   4 +-
>> > >>>        lib/rsa/rsa-sign.c               |   5 +-
>> > >>>        lib/rsa/rsa-verify.c             |  13 +-
>> > >>>        test/py/tests/test_fdt_sign.py   |  83 ++++++++
>> > >>>        test/py/tests/test_vboot.py      |  21 +-
>> > >>>        test/py/tests/vboot/sign-fdt.dts |  23 ++
>> > >>>        test/py/tests/vboot_comm.py      |  22 ++
>> > >>>        tools/Makefile                   |  10 +-
>> > >>>        tools/fdt-host.c                 | 353 +++++++++++++++++++++++++++++++
>> > >>>        tools/fdt_check_sign.c           |  85 ++++++++
>> > >>>        tools/fdt_host.h                 |  46 ++++
>> > >>>        tools/fdt_sign.c                 | 210 ++++++++++++++++++
>> > >>>        tools/fit_check_sign.c           |   4 +-
>> > >>>        tools/fit_common.c               |  69 ++++++
>> > >>>        tools/fit_common.h               |  23 ++
>> > >>>        tools/fit_image.c                |  59 +-----
>> > >>>        tools/image-fdt-sig.c            | 254 ++++++++++++++++++++++
>> > >>>        tools/image-host.c               | 155 +++++++++++---
>> > >>>        tools/imagetool.h                |   4 +
>> > >>>        tools/mkimage.c                  |   4 +
>> > >>>        29 files changed, 1714 insertions(+), 181 deletions(-)
>> > >>>        create mode 100644 doc/fdt_check_sign.1
>> > >>>        create mode 100644 doc/fdt_sign.1
>> > >>>        create mode 100644 test/py/tests/test_fdt_sign.py
>> > >>>        create mode 100644 test/py/tests/vboot/sign-fdt.dts
>> > >>>        create mode 100644 test/py/tests/vboot_comm.py
>> > >>>        create mode 100644 tools/fdt-host.c
>> > >>>        create mode 100644 tools/fdt_check_sign.c
>> > >>>        create mode 100644 tools/fdt_sign.c
>> > >>>        create mode 100644 tools/image-fdt-sig.c
>> > >>>
>> > >>>      --
>> > >>>      2.34.0.rc1.387.gb447b232ab-goog
>> > >>>
>> > >>> --
>> > >>>
>> > >>> François-Frédéric Ozog | /Director Business Development/
>> > >>> T: +33.67221.6485
>> > >>> francois.ozog@linaro.org <mailto:francois.ozog@linaro.org> | Skype: ffozog
>> > >>>
>> > >>>
>
> --
> François-Frédéric Ozog | Director Business Development
> T: +33.67221.6485
> francois.ozog@linaro.org | Skype: ffozog
>
Tom Rini Nov. 25, 2021, 3:07 a.m. UTC | #11
On Wed, Nov 24, 2021 at 05:12:12PM -0700, Simon Glass wrote:
> Hi François,
> 
> On Sat, 13 Nov 2021 at 11:53, François Ozog <francois.ozog@linaro.org> wrote:
> >
> > Hi Simon
> >
> > Le sam. 13 nov. 2021 à 14:57, Simon Glass <sjg@chromium.org> a écrit :
> >>
> >> Hi Heinrich,
> >>
> >> On Sat, 13 Nov 2021 at 04:57, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >> >
> >> >
> >> >
> >> > On 11/13/21 04:30, Simon Glass wrote:
> >> > > Hi Heinrich,
> >> > >
> >> > > On Fri, 12 Nov 2021 at 17:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >> > >>
> >> > >>
> >> > >>
> >> > >> On 11/12/21 20:49, François Ozog wrote:
> >> > >>> Hi Simon
> >> > >>>
> >> > >>> Le ven. 12 nov. 2021 à 20:36, Simon Glass <sjg@chromium.org
> >> > >>> <mailto:sjg@chromium.org>> a écrit :
> >> > >>>
> >> > >>>      At present mkimage supports signing FITs, the standard U-Boot image
> >> > >>>      type.
> >> > >>>
> >> > >>>      Various people are opposed to using FIT since:
> >> > >>>
> >> > >>> just to be sure: I am not one of those.
> >> > >>>
> >> > >>>
> >> > >>>      a) it requires adding support for FIT into other bootloaders, notably
> >> > >>>          UEFI
> >> > >>>
> >> > >>> whatever happens to FIT is entirely orthogonal to U-Boot UEFI subsystem.
> >> > >>> FIT can evolve,  U-Boot UEFI does not have to change.
> >> > >>
> >> > >> We already can create signed FIT images containing a UEFI binary and a
> >> > >> devcie-tree and launch the FIT image with the bootm command.
> >> > >>
> >> > >> Cf.
> >> > >> CONFIG_BOOTM_EFI
> >> > >> test/py/tests/test_efi_fit.py
> >> > >> doc/usage/bootefi.rst
> >> > >>
> >> > >> Simon, what are you missing?
> >> > >
> >> > > Some people don't want to use FIT.
> >> >
> >> > The problem with FIT is that other firmware like EDK II does not support it.
> >> >
> >> > Why do you expect those people to like your new tool better?
> >>
> >> I believe the EDK decision is not so much that it *does* not support
> >> FIT, which is after all not a lot of code, but that it *should* not.
> >> If I have that wrong, please let me know.
> >
> > UEFI (EDK2 and now U-Boot) design choice is that it does not include *any* technologies for OS: UEFI launches an OS loader that know about those.
> > A possible way to introduce a FIT is by adding a protocol that can register a vendor media file , assign a UUID to a FIT. Add support for this in the UEFI stub of the kernel.
> > Another way would be a minimalistic UEFI app that will fetch the second parameter as a FIT. The FIT format parsing code is only in this minimalistic loader. U-Boot can play this role based on your efforts to have U-Boot as UEFI application.
> > You could also promote FIT in the systemd-boot UeFi application. I believe this is a very good option BTW.
> 
> I think UEFI is making things very complicated. If we define a
> standard boot flow we should not need grub.

To be clear, UEFI doesn't require grub (nor systemd-boot) but off the
shelf distributions tend to really really like that, rather than
modifying UEFI variables more directly to implement some sort of boot
menu and other related features.
Rasmus Villemoes Nov. 26, 2021, 8:36 a.m. UTC | #12
On 12/11/2021 20.28, Simon Glass wrote:
> At present mkimage supports signing FITs, the standard U-Boot image type.
> 
> Various people are opposed to using FIT since:
> 
> a) it requires adding support for FIT into other bootloaders, notably
>    UEFI
> b) it requires packaging a kernel in this standard U-Boot format, meaning
>    that distros must run 'mkimage' and deal with the kernel and initrd
>    being inside a FIT
> 
> The kernel and initrd can be dealt with in other ways. But without FIT,
> we have no standard way of signing and grouping FDT files. Instead we must
> include them in the distro as separate files.
> 
> In particular, some sort of mechanism for verifying FDT files is needed.
> One option would be to tack a signature on before or after the file,
> processing it accordingly. But due to the nature of the FDT binary format,
> it is possible to embed a signature inside the FDT itself, which is very
> convenient.
> 
> This series provides a tool, fdt_sign, which can add a signature to an
> FDT. The signature can be checked later, preventing any change to the FDT,
> other than in permitted nodes (e.g. /chosen).
> 
> This series also provides a fdt_check_sign tool, used to check signatures.

This could be useful. Not because I'm interested in signing device-tree
blobs as such, but as a replacement for the current incomprehensible way
FIT images are signed and verified. This seems to be a much better and
simpler scheme, that avoids (can avoid) the overly simplistic approach
of signing just image nodes, and the way too complex
signing-configurations-and-then-add-a-property-saying-which-nodes-I-signed-and-sign-that-as-well.

In order not to hard-code anything in the tool about /chosen or whatnot,
make the rule be that the blob (I'll use that word, because it can as
well be a FIT image containing kernel image+initramfs etc., or a FIT
containing a U-Boot script) has a property in the top node

  unsigned-nodes = "/chosen", "/signatures";

(maybe it could be

  unsigned-nodes = <&chosen &sigs>

but that may make the implementation a little more complex). That
property itself is by definition part of what gets signed. The verifier
can and should choose to reject the whole thing if "/" is mentioned.

Then, please, for debugging, inside the /signatures node, beside the
signature data, also add a copy of the offset/len pairs that were
actually hashed, and that hash value.

  /signatures {
    hashed-regions = <offset0 len0 offset1 len1 ...>;
    hash-1 {
      value = 0xabcd;
      algo = "sha256";
    };
    signature-1 {
       ...
    };
    ...

And make the tool start by adding dummy properties, so those strings
"hashed-regions", "value", "algo", "signer" and whatever other property
names are used already exist in the string table. After that, fill in
the actual values - and have the rule that the entire string table is
hashed, not just some initial part of it.

The verifier will of course not rely on /signatures/hashed-regions, but
will compute the regions itself based on /unsigned-nodes (and implicitly
add a region consisting of the string table and the memreserve table).

Then we'd have a generic scheme for signing blobs, disentangled from
whether they are actually describing hardware or used as a generic
container format. And instead of "image" or "conf", we could mark a
public key as being required for "whole-blob" (bikeshedding welcome),
using that same scheme for verifying a container with a boot script and
a container with kernel+initramfs+dtbs.
> For now there is absolutely no configurability in the signature mechanism.
> It would of course be possible to adjust which nodes are signed, as is

Yes, let the blob itself define that. And don't add any ad hoc "oh, skip
a property if it is called data-offset or data or...".

Rasmus
Simon Glass Jan. 24, 2022, 2:17 p.m. UTC | #13
Hi Rasmus,

On Fri, 26 Nov 2021 at 01:36, Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> On 12/11/2021 20.28, Simon Glass wrote:
> > At present mkimage supports signing FITs, the standard U-Boot image type.
> >
> > Various people are opposed to using FIT since:
> >
> > a) it requires adding support for FIT into other bootloaders, notably
> >    UEFI
> > b) it requires packaging a kernel in this standard U-Boot format, meaning
> >    that distros must run 'mkimage' and deal with the kernel and initrd
> >    being inside a FIT
> >
> > The kernel and initrd can be dealt with in other ways. But without FIT,
> > we have no standard way of signing and grouping FDT files. Instead we must
> > include them in the distro as separate files.
> >
> > In particular, some sort of mechanism for verifying FDT files is needed.
> > One option would be to tack a signature on before or after the file,
> > processing it accordingly. But due to the nature of the FDT binary format,
> > it is possible to embed a signature inside the FDT itself, which is very
> > convenient.
> >
> > This series provides a tool, fdt_sign, which can add a signature to an
> > FDT. The signature can be checked later, preventing any change to the FDT,
> > other than in permitted nodes (e.g. /chosen).
> >
> > This series also provides a fdt_check_sign tool, used to check signatures.
>
> This could be useful. Not because I'm interested in signing device-tree
> blobs as such, but as a replacement for the current incomprehensible way
> FIT images are signed and verified. This seems to be a much better and
> simpler scheme, that avoids (can avoid) the overly simplistic approach
> of signing just image nodes, and the way too complex
> signing-configurations-and-then-add-a-property-saying-which-nodes-I-signed-and-sign-that-as-well.

That property is not actually signed. It is just a hint.

>
> In order not to hard-code anything in the tool about /chosen or whatnot,
> make the rule be that the blob (I'll use that word, because it can as
> well be a FIT image containing kernel image+initramfs etc., or a FIT
> containing a U-Boot script) has a property in the top node
>
>   unsigned-nodes = "/chosen", "/signatures";
>
> (maybe it could be
>
>   unsigned-nodes = <&chosen &sigs>
>
> but that may make the implementation a little more complex). That
> property itself is by definition part of what gets signed. The verifier
> can and should choose to reject the whole thing if "/" is mentioned.

If you list unsigned nodes, then the signature will become invalid if
a new image or config is added later.

>
> Then, please, for debugging, inside the /signatures node, beside the
> signature data, also add a copy of the offset/len pairs that were
> actually hashed, and that hash value.
>
>   /signatures {
>     hashed-regions = <offset0 len0 offset1 len1 ...>;
>     hash-1 {
>       value = 0xabcd;
>       algo = "sha256";
>     };
>     signature-1 {
>        ...
>     };
>     ...

The offsets may change whenever the devicetree is modified.

>
> And make the tool start by adding dummy properties, so those strings
> "hashed-regions", "value", "algo", "signer" and whatever other property
> names are used already exist in the string table. After that, fill in
> the actual values - and have the rule that the entire string table is
> hashed, not just some initial part of it.

The hashed part of the string table is the part used for each
signature. It is in fact the whole table at the time that signing
takes place. It's just that the table may expand late.

>
> The verifier will of course not rely on /signatures/hashed-regions, but
> will compute the regions itself based on /unsigned-nodes (and implicitly
> add a region consisting of the string table and the memreserve table).
>
> Then we'd have a generic scheme for signing blobs, disentangled from
> whether they are actually describing hardware or used as a generic
> container format. And instead of "image" or "conf", we could mark a
> public key as being required for "whole-blob" (bikeshedding welcome),
> using that same scheme for verifying a container with a boot script and
> a container with kernel+initramfs+dtbs.

What does this 'whole blob' help with?

> > For now there is absolutely no configurability in the signature mechanism.
> > It would of course be possible to adjust which nodes are signed, as is
>
> Yes, let the blob itself define that. And don't add any ad hoc "oh, skip
> a property if it is called data-offset or data or...".

We don't want to sign the data because it is already hashed once.
There is no point and it just makes verification slower.

For data-offset we want tools to be able to move the data around, e.g.
for memory-allocation reasons.

Overall I can grant that you are describing another way of doing
things, but I'm not sure what it would bring us.

The scheme in this series is aimed at a different purpose: signing a
devicetree blob, rather than a FIT.

Regards,
Simon