Hi Rasmus,
On Tue, 26 Oct 2021 at 02:08, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
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@preva... [2] https://lore.kernel.org/u-boot/2184f1e6dd6247e48133c76205feeabe@kaspersky.co...
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.
I still think binman is the best solution here. The points you are raising are annoyances that can be resolved.
We could return a non-zero value from binman when external blobs are missing; we'd just need to use a special value (we use 101 for buildman) and update the CI to detect and ignore that.
Normally people use a separate output directory from the source directory, which is why the 'pollution' is not normally a big problem. But it is possible to resolve that problem and it has been discussed on the mailing list. It's just that no one has done it yet.
In what way does binman play badly with Kbuild?
I'd really suggest changing the mindset to separating build from packaging, perhaps by having a separate yocto package/recipe that puts the firmware together, adds the signature, etc. Images are only getting more complicated. The idea that you'll be able to build everyone in the U-Boot tree and it won't need any post-processing is not going to survive very long IMO, so you may as well invest in it now :-)
Regards, Simon