On 26/10/2021 03.28, Simon Glass wrote: > Hi Rasmus, > > On Tue, 28 Sept 2021 at 02:57, Rasmus Villemoes > <rasmus.villem...@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.villem...@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.villem...@prevas.dk/ >> [2] >> https://lore.kernel.org/u-boot/2184f1e6dd6247e48133c76205fee...@kaspersky.com/ >> >> dts/Kconfig | 9 +++++++++ >> scripts/Makefile.lib | 2 ++ >> 2 files changed, 11 insertions(+) > > Reviewed-by: Simon Glass <s...@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