Re: [PATCH] introduce CONFIG_DEVICE_TREE_INCLUDES

Rasmus Villemoes
2021-10-26T08:08:25Z

On 26/10/2021 03.28, Simon Glass wrote:
> Hi Rasmus,
>
> On Tue, 28 Sept 2021 at 02:57, Rasmus Villemoes
> <rasmus.villemoes@prevas.dk> wrote:
>>
>> The build system already automatically looks for and includes an
>> in-tree *-u-boot.dtsi when building the control .dtb. However, there
>> are some things that are awkward to maintain in such an in-tree file,
>> most notably the metadata associated to public keys used for verified
>> boot.
>>
>> The only "official" API to get that metadata into the .dtb is via
>> mkimage, as a side effect of building an actual signed image. But
>> there are multiple problems with that. First of all, the final U-Boot
>> (be it U-Boot proper or an SPL) image is built based on a binary
>> image, the .dtb, and possibly some other binary artifacts. So
>> modifying the .dtb after the build requires the meta-buildsystem
>> (Yocto, buildroot, whatnot) to know about and repeat some of the steps
>> that are already known to and handled by U-Boot's build system,
>> resulting in needless duplication of code. It's also somewhat annoying
>> and inconsistent to have a .dtb file in the build folder which is not
>> generated by the command listed in the corresponding .cmd file (that
>> of course applies to any generated file).
>>
>> So the contents of the /signature node really needs to be baked into
>> the .dtb file when it is first created, which means providing the
>> relevant data in the form of a .dtsi file. One could in theory put
>> that data into the *-u-boot.dtsi file, but it's more convenient to be
>> able to provide it externally: For example, when developing for a
>> customer, it's common to use a set of dummy keys for development,
>> while the consultants do not (and should not) have access to the
>> actual keys used in production. For such a setup, it's easier if the
>> keys used are chosen via the meta-buildsystem and the path(s) patched
>> in during the configure step. And of course, nothing prevents anybody
>> from having DEVICE_TREE_INCLUDES point at files maintained in git, or
>> for that matter from including the public key metadata in the
>> *-u-boot.dtsi directly and ignore this feature.
>>
>> There are other uses for this, e.g. in combination with ENV_IMPORT_FDT
>> it can be used for providing the contents of the /config/environment
>> node, so I don't want to tie this exclusively to use for verified
>> boot.
>>
>> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
>> ---
>>
>> Getting the public key metadata into .dtsi form can be done with a
>> little scripting (roughly 'mkimage -K' of a dummy image followed by
>> 'dtc -I dtb -O dts'). I have a script that does that, along with
>> options to set 'required' and 'required-mode' properties, include
>> u-boot,dm-spl properties in case one wants to check U-Boot proper from
>> SPL with the verified boot mechanism, etc. I'm happy to share that
>> script if this gets accepted, but it's moot if this is rejected.
>>
>> I have previously tried to get an fdt_add_pubkey tool accepted [1,2]
>> to disentangle the kernel and u-boot builds (or u-boot and SPL builds
>> for that matter!), but as I've since realized, that isn't quite enough
>> - the above points re modifying the .dtb after it is created but
>> before that is used to create further build artifacts still
>> stand. However, such a tool could still be useful for creating the
>> .dtsi info without the private keys being present, and my key2dtsi.sh
>> script could easily be modified to use a tool like that should it ever
>> appear.
>>
>> [1] https://lore.kernel.org/u-boot/20200211094818.14219-3-rasmus.villemoes@prevas.dk/
>> [2] https://lore.kernel.org/u-boot/2184f1e6dd6247e48133c76205feeabe@kaspersky.com/
>>
>> dts/Kconfig | 9 +++++++++
>> scripts/Makefile.lib | 2 ++
>> 2 files changed, 11 insertions(+)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> I suggest adding this to the documentation somewhere in doc/develop/devicetree/

Will do.

> Getting the key into the U-Boot .dtb is normally done with mkimage, as
> you say. I don't really understand why this approach is easier.

I think I explained that in the commit message, but let me try with a
more concrete example. I'm building, using Yocto as the umbrella build
system, for a SOC that needs an SPL + U-Boot proper.

So I have a U-Boot recipe that handles the configuration and
compilation. That's mostly a matter of "make foo_defconfig" followed by
"make ${UBOOT_TARGETS}" where UBOOT_TARGETS is something set in the
machine config. That results in two artifacts, say SPL and u-boot.itb
(the names don't really matter) that are almost ready to be written to
whatever storage is used for them. Most likely, the SPL binary is built
from a u-boot-spl-nodtb.bin and a spl.dtb and perhaps there's some
SOC-specific header in front of it all that the hardware/firmware needs,
those details are hidden by U-Boot's build system invoking the right
flavour of mkimage with the right parameters. If I really want to know,
I can look in spl/.SPL.cmd to see just how it was made [unless it's
binman creating a flash.bin, because then it's just black magic]. But I
usually don't need to.

Enter verified boot. SPL needs to verify U-Boot, and U-Boot must in turn
verify the kernel. I can easily, as a post-compile step, create a signed
version of u-boot.itb. But the -K option is rather useless here, because
SPL has already been built. So if I were to only add the corresponding
public key to SPL's dtb after/while signing U-Boot proper, I'd have to
manually repeat the step(s) needed to create SPL in the first place. Not
to mention that the .dtb living inside u-boot.itb doesn't have the
public key needed for verifying the kernel, so I'd _actually_ first have
to repeat whatever steps were done to create u-boot.itb, after using
mkimage to sign some dummy image just to use the -K option to modify
u-boot.dtb. It's really cumbersome.

So part of the problem is this "you can only get the public keys in the
form required inside the .dtb as a side-effect of signing an image". I
believe that's a fundamental design mistake, hence my attempt at
creating the fdt_add_pubkey tool. But even with that available, adding
the pubkey info into an already-compiled .dtb isn't really enough,
because the .dtb gets built as part of a normal "make". Hence the need
for a way to ensure the pubkey info gets baked into that .dtb during the
build.

Yes, I then need to prepare proper .dtsi files. But since key generation
is a mostly one-time/manual thing, that easily goes along with the
instructions (or script) that generates those, and for every foo.crt,
I'll just have that directory also contain a foo.dtsi, which I can then
point at during do_configure.

> Also, is there any interest in using binman?

Not really. I mean, it's fine if U-Boot internally uses that, and I can
just take the final output and use that. But as long as binman doesn't
play nice with Kbuild and e.g. tells the commands that were used to
create a given binary, and pollutes the build dir with stuff that's not
removed by "make clean", I'm not very enthusiastic about adding more
uses myself.

Also, this:

BINMAN all
Image 'main-section' is missing external blobs and is non-functional:
blob-ext@3

Some images are invalid
$ echo $?
0

Really? When building manually, perhaps the developer sees that warning
(unless there were other non-BINMAN targets that make decided to build
later, making the above scroll away...), but it's not very useful in
some CI scenario where artifacts get deployed automatically to a test
system after a successful build. Recovering boards with a broken
bootloader easily costs many man-hours, plus the time to figure out
what's wrong.

Rasmus