Hi Rasmus,
On Mon, 24 Jan 2022 at 03:42, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
On 14/01/2022 17.51, Simon Glass wrote:
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.
Eh, does make(1) exit with a code that depends on the exit code from a failing target? What if multiple targets failed to build? Unless one can quote chapter-and-verse from make documentation, I don't think that's a particularly robust solution.
Make is not the issue here. The basic problem is that we want to have three results:
- build failed, e.g. a compiler error - build succeeded but some binary blobs or tools are missing so the image won't work - build succeeded, image is valid
Normally people use a separate output directory from the source directory, which is why the 'pollution' is not normally a big problem.
Well, build systems like Yocto do set up a separate build dir, but that's not really important, as I rarely look inside ${S} nor ${B} - but when doing my own tinkering, I almost always build in the source tree, simply because that's a lot faster for quick experiments. So I'd wish all binman's temporary files were stowed away in some .binman_tmp dir in the top build dir (whether that's the source dir or an out-of-tree dir). Commits like 824204e42 are a strong indication that the current state of things is broken and doesn't scale.
Well that commit was really just rearranging the deck chairs. I didn't even see it.
The topic has come up before and I have suggested how it could be done. There is even a long-standing TODO in the binman readme about it.
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?
My main grievance is that when binman is merely a thin wrapper around some external tool (often mkimage or one of its derivates), there's no .<target>.cmd file generated allowing me to see just what command was run to generate <target>. Another thing is that I can't do "make <target>".
You can enable logging with BINMAN_VERBOSE. See also the bintool series (which I should apply soon) which provides a single place for running these tools. So we could perhaps improve things in this area.
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.
Oh, I couldn't agree more, and in fact I've already done that for all the kernels I build; Yocto's kernel-fitimage.bbclass is simply too inflexible to use directly from the kernel recipe, so I just build an Image (or Image.gz) in the kernel recipe, then have a separate kernel-fit.bb recipe for actually assembling the different FIT images I need (often one for normal use and another for bootstrapping, but possibly more).
But I really can't see how binman helps in any way or form - in fact, it obscures the process of creating such a separate recipe, exactly because I can't extract the necessary magic invocation of mkimage that is needed to wrap SPL in the header format expected by the hardware.
Why are you wanting to do that? You can just run binman again, can't you?
And the thing about "adding the signature" - yes, indeed, _signing_ can and should be done after building. But that is not at all what this started with, this is about embedding the metadata that U-Boot (or SPL) will need for _verifying_ during the build itself - when the private key may not even be available. Again, I think that it's a fundamental design bug that generating and adding that metadata in the form needed by U-Boot can only be done as a side effect of signing some unrelated image.
It is a side effect of signing *the same* image, i.e. the image that holds the signature and the public key. There is only one image, the firmware image produced by binman.
Regards, Simon