On Fri, 2025-02-21 at 11:15 -0300, Rogerio Guerra Borin via lists.openembedded.org wrote: > Hey Marek and Sean, > > I think we're discussing two different questions here and perhaps > mixing > up them two :-) > > One question is: Should people sign "only" the individual subimages > inside the FIT image? And I think we all agree that the answer is no > because that image would be susceptible to mix-and-match attacks. > > Another question is: What's the behavior associated with the variable > FIT_SIGN_INDIVIDUAL? And we've got two conflicting answers: > > (1) It selects between *either* signing individual images *or* > signing > the configuration. > (2) It simply enables the signing of individual images but does not > impact the signing of the configuration (which is always performed if > UBOOT_SIGN_ENABLE="1"). > > From what we talked, Marek believes the behavior follows (or should > follow) item (1), but both the documentation and the observed > behavior > so far (up until commit d7bd9c62) follow item (2). > > So, in the end all we need to do is to decide on the *expected* > behavior > between (1) and (2). > > In my view, behavior (2), which is the documented one, is the best > choice because the user simply cannot sign *only* the individual > images, > being thus more foolproof (that is, the user simply cannot produce a > signed image susceptible to the mix-and-match attack). Besides that, > since (2) is the observed behavior (up until commit d7bd9c62), > changing > it would be a breaking change, at least in my understanding. > > > > > > > See the U-Boot documentation: > > > > > > https://docs.u-boot.org/en/latest/usage/fit/signature.html > > > > > > And especially this section: > > > > > > https://docs.u-boot.org/en/latest/usage/fit/signature.html#signed-configurations > > > > > > That is why I find the signing of both images and configurations > > > odd. > > Checking the U-Boot docs, I see it says that it is possible to sign > with > multiple keys. So I don't think it contradicts or prevents what Yocto > is > doing (signing config or config+images). > > Btw, the docs also says that each key has a property "required" which > accepts values "image" or "conf". Here's a snippet of the produced > U-Boot DTB (same on kirkstone and scarthgap pre d7bd9c62) when > FIT_SIGN_INDIVIDUAL="1": > > """ > $ fdtdump -s deploy/images/verdin-imx8mm/u-boot.dtb > deploy/images/verdin-imx8mm/u-boot.dtb: found fdt at offset 0 > /dts-v1/; > // magic: 0xd00dfeed > // totalsize: 0x10de8 (69096) > // off_dt_struct: 0x38 > // off_dt_strings: 0xf530 > // off_mem_rsvmap: 0x28 > // version: 17 > // last_comp_version: 16 > // boot_cpuid_phys: 0x0 > // size_dt_strings: 0x1664 > // size_dt_struct: 0xf4f8 > > / { > interrupt-parent = <0x00000001>; > #address-cells = <0x00000002>; > #size-cells = <0x00000002>; > model = "Toradex Verdin iMX8M Mini WB on Verdin Development > Board"; > compatible = "toradex,verdin-imx8mm-wifi-dev", "toradex,verdin- > ... > signature { > key-dev2 { > required = "image"; <=== > algo = "sha256,rsa2048"; > rsa,r-squared = <0x3956dbd5 0x8b651d3b 0xa621244b... > rsa,modulus = <0xbcb94b0f 0x77f059c2 0xfb77ccac... > rsa,exponent = <0x00000000 0x00010001>; > rsa,n0-inverse = <0x707813fd>; > rsa,num-bits = <0x00000800>; > key-name-hint = "dev2"; <=== > }; > key-dev { > required = "conf"; <=== > algo = "sha256,rsa2048"; > rsa,r-squared = <0x06d11c22 0x56f31661 0x133143bb... > rsa,modulus = <0xdb06864a 0x12a1e033 0x04b0b457... > rsa,exponent = <0x00000000 0x00010001>; > rsa,n0-inverse = <0x8925c5c7>; > rsa,num-bits = <0x00000800>; > key-name-hint = "dev"; <=== > }; > }; > aliases { > ... > """ > > Basically, the "dev2" key is marked as required for checking images > while the "dev" key is marked as required for checking the > configurations. This seems perfectly acceptable by the U-Boot > documentation and it works fine because the resulting kernel FIT > image > does get both signatures. To prove, here's the snippet for the > kernel: > > """ > deploy/images/verdin-imx8mm/fitImage: found fdt at offset 0 > /dts-v1/; > // magic: 0xd00dfeed > // totalsize: 0xb86379 (12084089) > // off_dt_struct: 0x38 > // off_dt_strings: 0xb86010 > // off_mem_rsvmap: 0x28 > // version: 17 > // last_comp_version: 16 > // boot_cpuid_phys: 0x0 > // size_dt_strings: 0xd0 > // size_dt_struct: 0xb85fd8 > > / { > timestamp = <0x00000000>; > description = "Kernel fitImage for Torizon OS/6.6.74"; > #address-cells = <0x00000001>; > images { > kernel-1 { > description = "Linux kernel"; > data = [1f 8b 08 08 a6 ... > type = "kernel"; > arch = "arm64"; > os = "linux"; > compression = "gzip"; > load = <0x48200000>; > entry = <0x48200000>; > hash-1 { > value = <0x4abf5358 0x1069f770 0x4eccebee > 0x6fa2fce0... > algo = "sha256"; > }; > signature-1 { <==== > timestamp = <0x00000000>; > signer-version = "2024.01"; > signer-name = "mkimage"; > value = <0x1cc3293e 0x97759447 0x970a4cb2 > 0x4d904336... > algo = "sha256,rsa2048"; > key-name-hint = "dev2"; <==== > }; > }; > ... > configurations { > default = "conf-freescale_imx8mm-verdin-nonwifi-dahlia.dtb"; > conf-freescale_imx8mm-verdin-nonwifi-dahlia.dtb { > description = "1 Linux kernel, FDT blob"; > kernel = "kernel-1"; > fdt = "fdt-freescale_imx8mm-verdin-nonwifi-dahlia.dtb"; > hash-1 { > algo = "sha256"; > }; > signature-1 { <==== > hashed-strings = <0x00000000 0x000000b4>; > hashed-nodes = "/", > "/configurations/conf-freescale_imx8mm-verdin-nonwifi-dahlia.dtb", > "/images/kernel-1", "/images/k... > timestamp = <0x00000000>; > signer-version = "2024.01"; > signer-name = "mkimage"; > value = <0x85d3c35c 0xeac899db... > algo = "sha256,rsa2048"; > key-name-hint = "dev"; <==== > padding = "pkcs-1.5"; > sign-images = "kernel", "fdt"; > }; > }; > ... > """ > > Of course, I also agree with what Marek said that signing both > configurations and images is redundant, but the Yocto docs > ( > https://docs.yoctoproject.org/5.0.6/ref-manual/variables.html#term-FIT > _SIGN_INDIVIDUAL) > says: > > > If set to “1”, then the kernel-fitimage class will sign the > kernel, > > dtb and ramdisk images individually *in addition* to signing the > FIT > > image itself. This could be useful if you are intending to verify > > signatures in *another context than booting via U-Boot*. > > Which leads me to the conclusion that the behavior pre d7bd9c62 is > intended. > > Having said all of that, notice that if the decision was really to > switch to interpretation (1) then other changes would be required > besides those made to concat_dtb(). In particular, I imagine > fitimage_emit_section_config() would require changes to avoid > emitting > the signature node for the configurations. > > Hope this clarifies things...
Yes, from my point of view that is the case. I came to exactly the same conclusion. The use of FIT_SIGN_INDIVIDUAL is strange, but not wrong. It means that it's definitely not something that can be changed on an LTS branch. Although I really like Marek's patch, we need to revert that part of it. I'm currently working on improving the oe-selftests to catch this bug. Once I've done that, I'll start reviewing and testing https://lists.openembedded.org/g/openembedded-core/message/211761. Regards, Adrian > > > >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#211805): https://lists.openembedded.org/g/openembedded-core/message/211805 Mute This Topic: https://lists.openembedded.org/mt/111218371/21656 Group Owner: [email protected] Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
