* Broken FIT image signing since "u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled"
@ 2025-02-16 18:36 Rogerio Guerra Borin
2025-02-16 20:09 ` Rogerio Guerra Borin
2025-02-17 10:34 ` [OE-core] " Quentin Schulz
0 siblings, 2 replies; 15+ messages in thread
From: Rogerio Guerra Borin @ 2025-02-16 18:36 UTC (permalink / raw)
To: openembedded-core
[-- Attachment #1: Type: text/plain, Size: 3412 bytes --]
Hello,
At Toradex we're using class "kernel-fitimage" to produce our signed kernel FIT images and we've found that the recent commit d7bd9c627661 ("u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled") broke our builds. The logs of a failed build show the following message that I found noteworthy:
log.do_uboot_assemble_fitimage.841485:
...
Couldn't open RSA private key: '/secboot-data/fit-keys/dev2.key': No such file or directory
The odd thing was that the build was setting:
UBOOT_SIGN_KEYNAME = "dev"
UBOOT_SIGN_IMG_KEYNAME = "dev2"
FIT_SIGN_INDIVIDUAL = "0"
FIT_GENERATE_KEYS = "0"
From the above we see the individual images signing is disabled but the system is trying to access the key file for the individual signing (dev2) regardless of that. Checking the code in uboot-sign.bbclass I found this line:
https://git.openembedded.org/openembedded-core/tree/meta/classes-recipe/uboot-sign.bbclass?h=scarthgap#n116
that is responsible for that access (inside function concat_dtb()). The code is always referencing variable UBOOT_SIGN_IMG_KEYNAME regardless of FIT_SIGN_INDIVIDUAL being set to "1" or "0" even though there's a condition a couple lines above L116 where FIT_SIGN_INDIVIDUAL is tested to decide in which mode to run the signing tool. To better understand the behavior I created dev2.{key,crt} files and retried a build which worked. However, checking the contents of the final u-boot.dtb file I found that the "dev2" key node was present but the "dev" key wasn't. Then, checking the contents of the final fitImage I noticed the configuration nodes were signed by the dev key while the individual images were not signed (as expected). But that means such a system wouldn't boot since the required "dev" key is not present in the U-Boot DTB, from what I conclude the FIT signing is broken at the moment.
So, first of all I wanted to let you guys know about this issue (hoping we're not doing anything wrong with our builds).
As a second point, I wanted to confirm the expected behavior regarding variable FIT_SIGN_INDIVIDUAL. According to the documentation:
https://docs.yoctoproject.org/5.0.6/ref-manual/variables.html#term-FIT_SIGN_INDIVIDUAL
when its value is "1" the individual images will be signed *in addition* to the FIT image being signed. From that, I understand that the configurations will be always signed and the individual images may or may not be signed depending on FIT_SIGN_INDIVIDUAL. I kind of confirmed this behavior by checking our build artifacts on kirkstone: with FIT_SIGN_INDIVIDUAL="0" the U-Boot DTB has only the "dev" key and the fit image is signed by that same key; with FIT_SIGN_INDIVIDUAL="1" both keys are present in the DTB, the configurations are signed by the "dev" key and the images by the "dev2" key. I understand this is the correct behavior, right?
I'm asking because looking at the comment at:
https://git.openembedded.org/openembedded-core/tree/meta/classes-recipe/uboot-sign.bbclass?h=scarthgap#n106
it gives me the impression that FIT_SIGN_INDIVIDUAL would select between either signing individual images or signing the configurations, which doesn't match the documented behavior. I believe the code in concat_dtb() also seems to assume only one of the keys (dev/dev2) would need be present in the DTB. So the fix would involve a review on this as well I imagine.
[-- Attachment #2: Type: text/html, Size: 4266 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Broken FIT image signing since "u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled"
2025-02-16 18:36 Broken FIT image signing since "u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled" Rogerio Guerra Borin
@ 2025-02-16 20:09 ` Rogerio Guerra Borin
2025-02-17 10:34 ` [OE-core] " Quentin Schulz
1 sibling, 0 replies; 15+ messages in thread
From: Rogerio Guerra Borin @ 2025-02-16 20:09 UTC (permalink / raw)
To: openembedded-core
[-- Attachment #1: Type: text/plain, Size: 208 bytes --]
As a follow up, I performed some tests by reverting the d7bd9c627661 and I found that without that commit the behavior matches what I got on kirkstone or, in other words, it matches the documented behavior.
[-- Attachment #2: Type: text/html, Size: 238 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [OE-core] Broken FIT image signing since "u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled"
2025-02-16 18:36 Broken FIT image signing since "u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled" Rogerio Guerra Borin
2025-02-16 20:09 ` Rogerio Guerra Borin
@ 2025-02-17 10:34 ` Quentin Schulz
2025-02-17 22:37 ` Marek Vasut
1 sibling, 1 reply; 15+ messages in thread
From: Quentin Schulz @ 2025-02-17 10:34 UTC (permalink / raw)
To: rogerio.borin, openembedded-core; +Cc: Marek Vasut
+Cc Marek
On 2/16/25 7:36 PM, Rogerio Guerra Borin via lists.openembedded.org wrote:
> You don't often get email from rogerio.borin=toradex.com@lists.openembedded.org. Learn why this is important<https://aka.ms/LearnAboutSenderIdentification>
> Hello,
>
> At Toradex we're using class "kernel-fitimage" to produce our signed kernel FIT images and we've found that the recent commit d7bd9c627661 ("u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled") broke our builds. The logs of a failed build show the following message that I found noteworthy:
>
> log.do_uboot_assemble_fitimage.841485:
> ...
> Couldn't open RSA private key: '/secboot-data/fit-keys/dev2.key': No such file or directory
>
> The odd thing was that the build was setting:
>
> UBOOT_SIGN_KEYNAME = "dev"
> UBOOT_SIGN_IMG_KEYNAME = "dev2"
> FIT_SIGN_INDIVIDUAL = "0"
> FIT_GENERATE_KEYS = "0"
>
> From the above we see the individual images signing is disabled but the system is trying to access the key file for the individual signing (dev2) regardless of that. Checking the code in uboot-sign.bbclass I found this line:
>
> https://git.openembedded.org/openembedded-core/tree/meta/classes-recipe/uboot-sign.bbclass?h=scarthgap#n116
>
> that is responsible for that access (inside function concat_dtb()). The code is always referencing variable UBOOT_SIGN_IMG_KEYNAME regardless of FIT_SIGN_INDIVIDUAL being set to "1" or "0" even though there's a condition a couple lines above L116 where FIT_SIGN_INDIVIDUAL is tested to decide in which mode to run the signing tool. To better understand the behavior I created dev2.{key,crt} files and retried a build which worked. However, checking the contents of the final u-boot.dtb file I found that the "dev2" key node was present but the "dev" key wasn't. Then, checking the contents of the final fitImage I noticed the configuration nodes were signed by the dev key while the individual images were not signed (as expected). But that means such a system wouldn't boot since the required "dev" key is not present in the U-Boot DTB, from what I conclude the FIT signing is broken at the moment.
>
> So, first of all I wanted to let you guys know about this issue (hoping we're not doing anything wrong with our builds).
>
> As a second point, I wanted to confirm the expected behavior regarding variable FIT_SIGN_INDIVIDUAL. According to the documentation:
>
> https://docs.yoctoproject.org/5.0.6/ref-manual/variables.html#term-FIT_SIGN_INDIVIDUAL
>
> when its value is "1" the individual images will be signed *in addition* to the FIT image being signed. From that, I understand that the configurations will be always signed and the individual images may or may not be signed depending on FIT_SIGN_INDIVIDUAL. I kind of confirmed this behavior by checking our build artifacts on kirkstone: with FIT_SIGN_INDIVIDUAL="0" the U-Boot DTB has only the "dev" key and the fit image is signed by that same key; with FIT_SIGN_INDIVIDUAL="1" both keys are present in the DTB, the configurations are signed by the "dev" key and the images by the "dev2" key. I understand this is the correct behavior, right?
>
> I'm asking because looking at the comment at:
>
> https://git.openembedded.org/openembedded-core/tree/meta/classes-recipe/uboot-sign.bbclass?h=scarthgap#n106
>
> it gives me the impression that FIT_SIGN_INDIVIDUAL would select between either signing individual images or signing the configurations, which doesn't match the documented behavior. I believe the code in concat_dtb() also seems to assume only one of the keys (dev/dev2) would need be present in the DTB. So the fix would involve a review on this as well I imagine.
>
>
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#211507): https://lists.openembedded.org/g/openembedded-core/message/211507
> Mute This Topic: https://lists.openembedded.org/mt/111218371/6293953
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [quentin.schulz@cherry.de]
> -=-=-=-=-=-=-=-=-=-=-=-
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [OE-core] Broken FIT image signing since "u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled"
2025-02-17 10:34 ` [OE-core] " Quentin Schulz
@ 2025-02-17 22:37 ` Marek Vasut
2025-02-18 3:49 ` Rogerio Guerra Borin
0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2025-02-17 22:37 UTC (permalink / raw)
To: Quentin Schulz, rogerio.borin, openembedded-core
Cc: Richard Purdie, Freihofer, Adrian
On 2/17/25 11:34 AM, Quentin Schulz wrote:
> +Cc Marek
Hi,
> On 2/16/25 7:36 PM, Rogerio Guerra Borin via lists.openembedded.org wrote:
>> You don't often get email from
>> rogerio.borin=toradex.com@lists.openembedded.org. Learn why this is
>> important<https://aka.ms/LearnAboutSenderIdentification>
>> Hello,
>>
>> At Toradex we're using class "kernel-fitimage" to produce our signed
>> kernel FIT images and we've found that the recent commit d7bd9c627661
>> ("u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE
>> and UBOOT_ENV enabled") broke our builds. The logs of a failed build
>> show the following message that I found noteworthy:
Can you mail me the entire log file for completeness ?
>> log.do_uboot_assemble_fitimage.841485:
>> ...
>> Couldn't open RSA private key: '/secboot-data/fit-keys/dev2.key': No
>> such file or directory
>>
>> The odd thing was that the build was setting:
>>
>> UBOOT_SIGN_KEYNAME = "dev"
>> UBOOT_SIGN_IMG_KEYNAME = "dev2"
>> FIT_SIGN_INDIVIDUAL = "0"
>> FIT_GENERATE_KEYS = "0"
FIT_SIGN_INDIVIDUAL = "0" is correct, you do want to sign
configurations, not individual images in the fitImage, because signing
individual images is susceptible to mix-and-match attack.
>> From the above we see the individual images signing is disabled but
>> the system is trying to access the key file for the individual signing
>> (dev2) regardless of that. Checking the code in uboot-sign.bbclass I
>> found this line:
>>
>> https://git.openembedded.org/openembedded-core/tree/meta/classes-
>> recipe/uboot-sign.bbclass?h=scarthgap#n116
>>
>> that is responsible for that access (inside function concat_dtb()).
>> The code is always referencing variable UBOOT_SIGN_IMG_KEYNAME
>> regardless of FIT_SIGN_INDIVIDUAL being set to "1" or "0" even though
>> there's a condition a couple lines above L116 where
>> FIT_SIGN_INDIVIDUAL is tested to decide in which mode to run the
>> signing tool.
Uh, good find, does the following patch fix your problem ?
"""
diff --git a/meta/classes-recipe/uboot-sign.bbclass
b/meta/classes-recipe/uboot-sign.bbclass
index 96c47ab016..74ef5697ca 100644
--- a/meta/classes-recipe/uboot-sign.bbclass
+++ b/meta/classes-recipe/uboot-sign.bbclass
@@ -107,13 +107,16 @@ concat_dtb() {
# makes fitImage susceptible to mix-and-match attack.
if [ "${FIT_SIGN_INDIVIDUAL}" = "1" ] ; then
UBOOT_MKIMAGE_MODE="auto"
+ UBOOT_MKIMAGE_KEYNAME=${UBOOT_SIGN_IMG_KEYNAME}
+ else
+ UBOOT_MKIMAGE_KEYNAME=${UBOOT_SIGN_KEYNAME}
fi
${UBOOT_MKIMAGE_SIGN} \
${@'-D "${UBOOT_MKIMAGE_DTCOPTS}"' if
len('${UBOOT_MKIMAGE_DTCOPTS}') else ''} \
-f $UBOOT_MKIMAGE_MODE \
-k "${UBOOT_SIGN_KEYDIR}" \
-o "${FIT_HASH_ALG},${FIT_SIGN_ALG}" \
- -g "${UBOOT_SIGN_IMG_KEYNAME}" \
+ -g "$UBOOT_MKIMAGE_KEYNAME" \
-K "${UBOOT_DTB_BINARY}" \
-d /dev/null \
-r ${B}/unused.itb \
"""
>> To better understand the behavior I created dev2.
>> {key,crt} files and retried a build which worked. However, checking
>> the contents of the final u-boot.dtb file I found that the "dev2" key
>> node was present but the "dev" key wasn't. Then, checking the contents
>> of the final fitImage I noticed the configuration nodes were signed by
>> the dev key while the individual images were not signed (as expected).
>> But that means such a system wouldn't boot since the required "dev"
>> key is not present in the U-Boot DTB, from what I conclude the FIT
>> signing is broken at the moment.
>>
>> So, first of all I wanted to let you guys know about this issue
>> (hoping we're not doing anything wrong with our builds).
No, your builds are fine, this is a real bug.
>> As a second point, I wanted to confirm the expected behavior regarding
>> variable FIT_SIGN_INDIVIDUAL. According to the documentation:
>>
>> https://docs.yoctoproject.org/5.0.6/ref-manual/variables.html#term-
>> FIT_SIGN_INDIVIDUAL
>>
>> when its value is "1" the individual images will be signed *in
>> addition* to the FIT image being signed.
That is a bit weird and no, I don't think that is how it is supposed to
work.
FIT_SIGN_INDIVIDUAL=1 -> only image subnodes in fitImage /images nodes
are signed AND fitImage is susceptible to mix-and-match attack
(basically attacker can replace individual signed images in fitImage
with different individual signed images, e.g. with older versions, to
achieve a combination of images which allows them to somehow break the
system).
FIT_SIGN_INDIVIDUAL=0 (recommended) -> images are hashed, configurations
are hashed, hashes are combined and signed. Because images and
configurations (which images are to be used together) are signed,
attacker cannot mix-and-match images into an un-allowed configuration.
>> From that, I understand that
>> the configurations will be always signed and the individual images may
>> or may not be signed depending on FIT_SIGN_INDIVIDUAL.
The other way around. The configurations are signed by default, which is
good, but this can be disabled if needed (unlikely).
>> I kind of
>> confirmed this behavior by checking our build artifacts on kirkstone:
>> with FIT_SIGN_INDIVIDUAL="0" the U-Boot DTB has only the "dev" key and
>> the fit image is signed by that same key; with FIT_SIGN_INDIVIDUAL="1"
>> both keys are present in the DTB, the configurations are signed by the
>> "dev" key and the images by the "dev2" key. I understand this is the
>> correct behavior, right?
Images shouldn't be signed by different keys than configurations, that
is a bit odd.
>> I'm asking because looking at the comment at:
>>
>> https://git.openembedded.org/openembedded-core/tree/meta/classes-
>> recipe/uboot-sign.bbclass?h=scarthgap#n106
>>
>> it gives me the impression that FIT_SIGN_INDIVIDUAL would select
>> between either signing individual images
Yes
>> or signing the
>> configurations
Signed configuration also includes hash of the images that the
configuration references in it, that's what prevents the mix-and-match
attack.
>> , which doesn't match the documented behavior. I believe
>> the code in concat_dtb() also seems to assume only one of the keys
>> (dev/dev2) would need be present in the DTB. So the fix would involve
>> a review on this as well I imagine.
IIRC Adrian was looking into improving those docs.
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [OE-core] Broken FIT image signing since "u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled"
2025-02-17 22:37 ` Marek Vasut
@ 2025-02-18 3:49 ` Rogerio Guerra Borin
2025-02-20 10:46 ` Jose Quaresma
2025-02-20 23:04 ` Marek Vasut
0 siblings, 2 replies; 15+ messages in thread
From: Rogerio Guerra Borin @ 2025-02-18 3:49 UTC (permalink / raw)
To: Marek Vasut, Quentin Schulz, openembedded-core
Cc: Richard Purdie, Freihofer, Adrian
[-- Attachment #1: Type: text/plain, Size: 10739 bytes --]
> This message originated from outside your organization
>
> On 2/17/25 11:34 AM, Quentin Schulz wrote:
> > +Cc Marek
>
> Hi,
Thanks for the quick answer!
>
> > On 2/16/25 7:36 PM, Rogerio Guerra Borin via lists.openembedded.org
> wrote:
> >> You don't often get email from
> >> rogerio.borin=toradex.com@lists.openembedded.org. Learn why this is
> >> important<https://aka.ms/LearnAboutSenderIdentification
> <https://aka.ms/LearnAboutSenderIdentification>>
> >> Hello,
> >>
> >> At Toradex we're using class "kernel-fitimage" to produce our signed
> >> kernel FIT images and we've found that the recent commit d7bd9c627661
> >> ("u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE
> >> and UBOOT_ENV enabled") broke our builds. The logs of a failed build
> >> show the following message that I found noteworthy:
>
> Can you mail me the entire log file for completeness ?
Sure.
I've added it as attachment (log.do_uboot_assemble_fitimage.841485).
> >> log.do_uboot_assemble_fitimage.841485:
> >> ...
> >> Couldn't open RSA private key: '/secboot-data/fit-keys/dev2.key': No
> >> such file or directory
> >>
> >> The odd thing was that the build was setting:
> >>
> >> UBOOT_SIGN_KEYNAME = "dev"
> >> UBOOT_SIGN_IMG_KEYNAME = "dev2"
> >> FIT_SIGN_INDIVIDUAL = "0"
> >> FIT_GENERATE_KEYS = "0"
>
> FIT_SIGN_INDIVIDUAL = "0" is correct, you do want to sign
> configurations, not individual images in the fitImage, because signing
> individual images is susceptible to mix-and-match attack.
>
> >> From the above we see the individual images signing is disabled but
> >> the system is trying to access the key file for the individual signing
> >> (dev2) regardless of that. Checking the code in uboot-sign.bbclass I
> >> found this line:
> >>
> >> https://git.openembedded.org/openembedded-core/tree/meta/classes-
> <https://git.openembedded.org/openembedded-core/tree/meta/classes->
> >> recipe/uboot-sign.bbclass?h=scarthgap#n116
> >>
> >> that is responsible for that access (inside function concat_dtb()).
> >> The code is always referencing variable UBOOT_SIGN_IMG_KEYNAME
> >> regardless of FIT_SIGN_INDIVIDUAL being set to "1" or "0" even though
> >> there's a condition a couple lines above L116 where
> >> FIT_SIGN_INDIVIDUAL is tested to decide in which mode to run the
> >> signing tool.
>
> Uh, good find, does the following patch fix your problem ?
>
> """
> diff --git a/meta/classes-recipe/uboot-sign.bbclass
> b/meta/classes-recipe/uboot-sign.bbclass
> index 96c47ab016..74ef5697ca 100644
> --- a/meta/classes-recipe/uboot-sign.bbclass
> +++ b/meta/classes-recipe/uboot-sign.bbclass
> @@ -107,13 +107,16 @@ concat_dtb() {
> # makes fitImage susceptible to mix-and-match attack.
> if [ "${FIT_SIGN_INDIVIDUAL}" = "1" ] ; then
> UBOOT_MKIMAGE_MODE="auto"
> + UBOOT_MKIMAGE_KEYNAME=${UBOOT_SIGN_IMG_KEYNAME}
> + else
> + UBOOT_MKIMAGE_KEYNAME=${UBOOT_SIGN_KEYNAME}
> fi
> ${UBOOT_MKIMAGE_SIGN} \
> ${@'-D "${UBOOT_MKIMAGE_DTCOPTS}"' if
> len('${UBOOT_MKIMAGE_DTCOPTS}') else ''} \
> -f $UBOOT_MKIMAGE_MODE \
> -k "${UBOOT_SIGN_KEYDIR}" \
> -o "${FIT_HASH_ALG},${FIT_SIGN_ALG}" \
> - -g "${UBOOT_SIGN_IMG_KEYNAME}" \
> + -g "$UBOOT_MKIMAGE_KEYNAME" \
> -K "${UBOOT_DTB_BINARY}" \
> -d /dev/null \
> -r ${B}/unused.itb \
> """
Looking at the patch I'm pretty sure it would solve my problem,
particularly because we set FIT_SIGN_INDIVIDUAL="0" in our builds.
However, I see it keeps the concept of the signing being either on the
config or on the individual images, which I wanted to further comment
down below.
> >> To better understand the behavior I created dev2.
> >> {key,crt} files and retried a build which worked. However, checking
> >> the contents of the final u-boot.dtb file I found that the "dev2" key
> >> node was present but the "dev" key wasn't. Then, checking the contents
> >> of the final fitImage I noticed the configuration nodes were signed by
> >> the dev key while the individual images were not signed (as expected).
> >> But that means such a system wouldn't boot since the required "dev"
> >> key is not present in the U-Boot DTB, from what I conclude the FIT
> >> signing is broken at the moment.
> >>
> >> So, first of all I wanted to let you guys know about this issue
> >> (hoping we're not doing anything wrong with our builds).
>
> No, your builds are fine, this is a real bug.
Thanks for the confirmation.
> >> As a second point, I wanted to confirm the expected behavior regarding
> >> variable FIT_SIGN_INDIVIDUAL. According to the documentation:
> >>
> >> https://docs.yoctoproject.org/5.0.6/ref-manual/variables.html#term-
> <https://docs.yoctoproject.org/5.0.6/ref-manual/variables.html#term->
> >> FIT_SIGN_INDIVIDUAL
> >>
> >> when its value is "1" the individual images will be signed *in
> >> addition* to the FIT image being signed.
>
> That is a bit weird and no, I don't think that is how it is supposed to
> work.
>
> FIT_SIGN_INDIVIDUAL=1 -> only image subnodes in fitImage /images nodes
> are signed AND fitImage is susceptible to mix-and-match attack
> (basically attacker can replace individual signed images in fitImage
> with different individual signed images, e.g. with older versions, to
> achieve a combination of images which allows them to somehow break the
> system).
>
> FIT_SIGN_INDIVIDUAL=0 (recommended) -> images are hashed, configurations
> are hashed, hashes are combined and signed. Because images and
> configurations (which images are to be used together) are signed,
> attacker cannot mix-and-match images into an un-allowed configuration.
>
> >> From that, I understand that
> >> the configurations will be always signed and the individual images may
> >> or may not be signed depending on FIT_SIGN_INDIVIDUAL.
>
> The other way around. The configurations are signed by default, which is
> good, but this can be disabled if needed (unlikely).
>
> >> I kind of
> >> confirmed this behavior by checking our build artifacts on kirkstone:
> >> with FIT_SIGN_INDIVIDUAL="0" the U-Boot DTB has only the "dev" key and
> >> the fit image is signed by that same key; with FIT_SIGN_INDIVIDUAL="1"
> >> both keys are present in the DTB, the configurations are signed by the
> >> "dev" key and the images by the "dev2" key. I understand this is the
> >> correct behavior, right?
>
> Images shouldn't be signed by different keys than configurations, that
> is a bit odd.
>
> >> I'm asking because looking at the comment at:
> >>
> >> https://git.openembedded.org/openembedded-core/tree/meta/classes-
> <https://git.openembedded.org/openembedded-core/tree/meta/classes->
> >> recipe/uboot-sign.bbclass?h=scarthgap#n106
> >>
> >> it gives me the impression that FIT_SIGN_INDIVIDUAL would select
> >> between either signing individual images
>
> Yes
>
> >> or signing the
> >> configurations
>
> Signed configuration also includes hash of the images that the
> configuration references in it, that's what prevents the mix-and-match
> attack.
>
> >> , which doesn't match the documented behavior. I believe
> >> the code in concat_dtb() also seems to assume only one of the keys
> >> (dev/dev2) would need be present in the DTB. So the fix would involve
> >> a review on this as well I imagine.
> IIRC Adrian was looking into improving those docs.
Regarding this second topic, let me show you some other information I
gathered from a couple of builds that may help with your assessment:
FSI := FIT_SIGN_INDIVIDUAL
CFG := Whether or not the configurations inside the FIT image were signed
IMG := Whether or not the subimages inside the FIT image were signed
| Branch | Version | FSI | Keys inside DTB | CFG | IMG |
|-----------+---------------+-----+------------------+-----+-----|
| kirkstone | latest | "0" | {dev} | Y | N |
| kirkstone | latest | "1" | {dev,dev2} | Y | Y |
|-----------+---------------+-----+------------------+-----+-----|
| scarthgap | latest | "0" | Build failed (!) | n/a | n/a |
| | + dummy dev2 | | {dev2} | Y | N |
| scarthgap | latest | "1" | {dev2} (!) | Y | Y |
|-----------+---------------+-----+------------------+-----+-----|
| scarthgap | w/o @d7bd9c62 | "0" | {dev} | Y | N |
| scarthgap | w/o @d7bd9c62 | "1" | {dev,dev2} | Y | Y |
The first two lines show that on kirkstone when FIT_SIGN_INDIVIDUAL="0"
only the configurations are signed and the DTB contains only the "dev"
key. When FIT_SIGN_INDIVIDUAL="1" both "dev" and "dev2" are present in
the DTB and both the configurations and the images are signed. And
that's the exact same behavior I got on scarthgap after reverting the
commit d7bd9c62. And, as I said, this seems to match the documentation.
I've attached snippets of the fdtdump for the FIT image and U-Boot DTB
from my build on kirkstone to illustrate what I said.
So, if the expected behavior is according to what you described then the
behavior so far was wrong (for a long time) and the new behavior is
introducing a breaking change for anyone relying on what's documented
for the FIT_SIGN_INDIVIDUAL="1" case, right? Btw, for us at Toradex this
is not a big deal because we don't fall into that case but considering
the general use of the layer the change in behavior could be an issue.
Another piece of information that might be useful: I made a change to
concat_dtb() to make it add both keys (dev/dev2) to the DTB (by invoking
mkimage twice) and I found the invocation to "uboot-fit_check_sign"
failed afterwards. I confirmed that the DTB had the two keys, but
because the FIT image only had the configurations signed, the tool threw
an error saying the signature of "require key dev2" could not be
verified but the showed that the check of the "dev" key succeeded.
Unfortunately I don't have the logs available for that case, but I could
repeat the tests if needed. Anyway, my point is that the
"uboot-fit_check_sign" tool seems capable of checking both keys as well.
Interestingly, if I removed the verification step (performed on the
"unused.itb" file), then the image generation succeeded and everything
seemed to work as before both when FIT_SIGN_INDIVIDUAL is "0" or "1".
So, it's not hard to get the old behavior, though we'd need to figure
out how to properly do the verification when mkimage is invoked with -f
"auto" or -f "auto-conf". I could post my unfinished patch if helpful.
[-- Attachment #2: log.do_uboot_assemble_fitimage.841485 --]
[-- Type: text/plain, Size: 3615 bytes --]
DEBUG: Executing shell function do_uboot_assemble_fitimage
Couldn't open RSA private key: '/secboot-data/fit-keys/dev2.key': No such file or directory
Failed to sign 'signature' signature node in 'conf-1' conf node
FIT description: Kernel Image image with one or more FDT blobs
Created: Thu Jan 1 00:00:00 1970
Image 0 (kernel-1)
Description:
Created: Thu Jan 1 00:00:00 1970
Type: Kernel Image
Compression: gzip compressed
Data Size: 0 Bytes = 0.00 KiB = 0.00 MiB
Architecture: PowerPC
OS: Linux
Load Address: 0x00000000
Entry Point: 0x00000000
Hash algo: sha1
Hash value: da39a3ee5e6b4b0d3255bfef95601890afd80709
Default Configuration: 'conf-1'
Configuration 0 (conf-1)
Description: unavailable
Kernel: kernel-1
Sign algo: sha256,rsa2048:dev2
Sign value: unavailable
Timestamp: unavailable
Signature written to '/workdir/build-verdin-imx8mm-sec/tmp/work/verdin_imx8mm-tdx-linux/u-boot-toradex/2024.07/u-boot-toradex-2024.07/unused.itb', node '/configurations/conf-1/signature'
Public key written to 'u-boot.dtb', node '/'
Verifying Hash Integrity for node 'conf-1'... Verified OK, loading images
## Loading kernel from FIT Image at 7f2757f1c000 ...
Using 'conf-1' configuration
Verifying Hash Integrity ...
OK
Trying 'kernel-1' kernel subimage
Description:
Created: Thu Jan 1 00:00:00 1970
Type: Kernel Image
Compression: gzip compressed
Data Size: 0 Bytes = 0.00 KiB = 0.00 MiB
Architecture: PowerPC
OS: Linux
Load Address: 0x00000000
Entry Point: 0x00000000
Hash algo: sha1
Hash value: da39a3ee5e6b4b0d3255bfef95601890afd80709
Verifying Hash Integrity ...
sha1+
OK
Decrypting Data ...
OK
Uncompressing Kernel Image
Unimplemented compression type 1
## Loading fdt from FIT Image at 7f2757f1c000 ...
Using 'conf-1' configuration
Verifying Hash Integrity ...
OK
Could not find subimage node type 'fdt'
## Loading ramdisk from FIT Image at 7f2757f1c000 ...
Using 'conf-1' configuration
Verifying Hash Integrity ...
OK
Could not find subimage node type 'ramdisk'
Signature check OK
DEBUG: Re-generating flash.bin with an updated DTB
Error at '/signature': FDT_ERR_NOTFOUND
Error at '/signature': FDT_ERR_NOTFOUND
WARNING: /workdir/build-verdin-imx8mm-sec/tmp/work/verdin_imx8mm-tdx-linux/u-boot-toradex/2024.07/temp/run.do_uboot_assemble_fitimage.841485:253 exit 1 from '/workdir/build-verdin-imx8mm-sec/tmp/work/verdin_imx8mm-tdx-linux/u-boot-toradex/2024.07/imx8m_copy_signature_node_to_fdts.sh "u-boot.dtb-signed"'
WARNING: Backtrace (BB generated script):
#1: concat_dtb, /workdir/build-verdin-imx8mm-sec/tmp/work/verdin_imx8mm-tdx-linux/u-boot-toradex/2024.07/temp/run.do_uboot_assemble_fitimage.841485, line 253
#2: uboot_assemble_fitimage_helper, /workdir/build-verdin-imx8mm-sec/tmp/work/verdin_imx8mm-tdx-linux/u-boot-toradex/2024.07/temp/run.do_uboot_assemble_fitimage.841485, line 182
#3: do_uboot_assemble_fitimage, /workdir/build-verdin-imx8mm-sec/tmp/work/verdin_imx8mm-tdx-linux/u-boot-toradex/2024.07/temp/run.do_uboot_assemble_fitimage.841485, line 168
#4: main, /workdir/build-verdin-imx8mm-sec/tmp/work/verdin_imx8mm-tdx-linux/u-boot-toradex/2024.07/temp/run.do_uboot_assemble_fitimage.841485, line 480
[-- Attachment #3: fdtdump-kirkstone-fitImage.txt --]
[-- Type: text/plain, Size: 32379 bytes --]
deploy/images/verdin-imx8mm/fitImage: found fdt at offset 0
/dts-v1/;
// magic: 0xd00dfeed
// totalsize: 0xd572ce (13988558)
// off_dt_struct: 0x38
// off_dt_strings: 0xd55cf8
// off_mem_rsvmap: 0x28
// version: 17
// last_comp_version: 16
// boot_cpuid_phys: 0x0
// size_dt_strings: 0xc5
// size_dt_struct: 0xd55cc0
/ {
timestamp = <0x00000000>;
description = "Kernel fitImage for TorizonCore/5.15.148+gitAUTOINC+c8df6466e9_0b02be14a0/verdin-
#address-cells = <0x00000001>;
images {
kernel-1 {
description = "Linux kernel";
data = <0x1f8b0808 0xf8f1b367 0x02036c69 0x6e75782e 0x62696e00 0xec9c7b74 0x14559ec7 0x7
type = "kernel";
arch = "arm64";
os = "linux";
compression = "gzip";
load = <0x48200000>;
entry = <0x48200000>;
hash-1 {
value = <0x70badbca 0x9c6a6aa9 0x7138edeb 0xb6fb0e0f 0x4882fcab 0x09a3c421 0x0b4e991
algo = "sha256";
};
signature-1 {
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0xa3df2fed 0x3c29bc9d 0xb9e6b875 0x6653a739 0x60892d1f 0x7ee19771 0x7229745
algo = "sha256,rsa2048";
key-name-hint = "dev2";
};
};
fdt-freescale_imx8mm-verdin-nonwifi-dahlia.dtb {
description = "Flattened Device Tree blob";
data = [d0 0d fe ed 00 01 05 49 00 00 00 38 00 00 ef 84 00 00 00 28 00 00 00 11 00 00 00
type = "flat_dt";
arch = "arm64";
compression = "none";
load = <0x50200000>;
hash-1 {
value = <0xe2bacc5f 0x8415decf 0x87c7dede 0xc7791ff5 0x684bdc0e 0xbb3c6ebc 0xa0a242f
algo = "sha256";
};
signature-1 {
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x193e2109 0x0fe332d4 0x08ab055d 0xcadc3184 0x8123db27 0x12b3359d 0xc606dbd
algo = "sha256,rsa2048";
key-name-hint = "dev2";
};
};
fdt-freescale_imx8mm-verdin-nonwifi-dev.dtb {
description = "Flattened Device Tree blob";
data = [d0 0d fe ed 00 01 03 11 00 00 00 38 00 00 ed 70 00 00 00 28 00 00 00 11 00 00 00
type = "flat_dt";
arch = "arm64";
compression = "none";
load = <0x50200000>;
hash-1 {
value = <0x5ec18626 0x0c8da78e 0x41d04423 0x9f59064f 0xe88093e1 0xc2cc7699 0x757d877
algo = "sha256";
};
signature-1 {
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x6c6afdff 0x46082d8e 0x9e59021e 0x8979cdc5 0x1964a4d2 0xeb77b3e3 0x4b5a36d
algo = "sha256,rsa2048";
key-name-hint = "dev2";
};
};
fdt-freescale_imx8mm-verdin-nonwifi-mallow.dtb {
description = "Flattened Device Tree blob";
data = [d0 0d fe ed 00 01 02 e2 00 00 00 38 00 00 ee 48 00 00 00 28 00 00 00 11 00 00 00
type = "flat_dt";
arch = "arm64";
compression = "none";
load = <0x50200000>;
hash-1 {
value = <0x3fa25fc1 0xf7d0b99a 0x330f4328 0xcf989daa 0x1eca383e 0x680a852b 0xfa1d625
algo = "sha256";
};
signature-1 {
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x1d182d2d 0x0cb5c92f 0x1f74d5d7 0xc89ea2d6 0xd5c9b384 0xa6e470dd 0x9455242
algo = "sha256,rsa2048";
key-name-hint = "dev2";
};
};
fdt-freescale_imx8mm-verdin-nonwifi-yavia.dtb {
description = "Flattened Device Tree blob";
data = [d0 0d fe ed 00 01 02 ed 00 00 00 38 00 00 ee 54 00 00 00 28 00 00 00 11 00 00 00
type = "flat_dt";
arch = "arm64";
compression = "none";
load = <0x50200000>;
hash-1 {
value = <0x41de4542 0x9ff2ec6b 0x29bd93e1 0x6ce1f691 0x4774fc4f 0x3e83dded 0x45ece55
algo = "sha256";
};
signature-1 {
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x945bc860 0xc2dcd9fe 0x16986a9a 0x6f34bd7e 0x7e86857c 0x7f204fb7 0xfb1f4eb
algo = "sha256,rsa2048";
key-name-hint = "dev2";
};
};
fdt-freescale_imx8mm-verdin-wifi-dahlia.dtb {
description = "Flattened Device Tree blob";
data = [d0 0d fe ed 00 01 06 15 00 00 00 38 00 00 f0 44 00 00 00 28 00 00 00 11 00 00 00
type = "flat_dt";
arch = "arm64";
compression = "none";
load = <0x50200000>;
hash-1 {
value = <0xd8e43403 0x0c9ef6e3 0xdaf7004d 0x2dfb2223 0x4953be20 0x07cbe59d 0x3bdb34c
algo = "sha256";
};
signature-1 {
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x3959d291 0x90718550 0x1b1f2ba9 0xea466c40 0x33b89869 0x9110dee5 0x8bb51e9
algo = "sha256,rsa2048";
key-name-hint = "dev2";
};
};
fdt-freescale_imx8mm-verdin-wifi-dev.dtb {
description = "Flattened Device Tree blob";
data = [d0 0d fe ed 00 01 03 d9 00 00 00 38 00 00 ee 2c 00 00 00 28 00 00 00 11 00 00 00
type = "flat_dt";
arch = "arm64";
compression = "none";
load = <0x50200000>;
hash-1 {
value = <0xce4f99bf 0x31069743 0x9e3f11a8 0x17901977 0xbc7077a4 0x10270221 0x750de27
algo = "sha256";
};
signature-1 {
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x5a059248 0x47757e2d 0xea34df68 0xc1df9e51 0x6bb738f8 0x06b618b7 0xf399e00
algo = "sha256,rsa2048";
key-name-hint = "dev2";
};
};
fdt-freescale_imx8mm-verdin-wifi-mallow.dtb {
description = "Flattened Device Tree blob";
data = [d0 0d fe ed 00 01 03 ae 00 00 00 38 00 00 ef 08 00 00 00 28 00 00 00 11 00 00 00
type = "flat_dt";
arch = "arm64";
compression = "none";
load = <0x50200000>;
hash-1 {
value = <0xacb32493 0x10224610 0x308e4529 0xa02eb48d 0xb5748784 0x12366d39 0x6edd6a0
algo = "sha256";
};
signature-1 {
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x7428a203 0x57d8f7f1 0x483aaa40 0x3e105a09 0x85df8f29 0x40cb2979 0x3486ce4
algo = "sha256,rsa2048";
key-name-hint = "dev2";
};
};
fdt-freescale_imx8mm-verdin-wifi-yavia.dtb {
description = "Flattened Device Tree blob";
data = [d0 0d fe ed 00 01 03 b5 00 00 00 38 00 00 ef 10 00 00 00 28 00 00 00 11 00 00 00
type = "flat_dt";
arch = "arm64";
compression = "none";
load = <0x50200000>;
hash-1 {
value = <0x767893e5 0x3eae0144 0x96c465cd 0xde51adfe 0xf95875ef 0x93d3590b 0x878f88e
algo = "sha256";
};
signature-1 {
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x31e7f2d1 0x4de1871c 0xe9307746 0xa61af7da 0x3bb3c884 0x253fe5e7 0xe4cdc26
algo = "sha256,rsa2048";
key-name-hint = "dev2";
};
};
fdt-verdin-imx8mm_20mhz_can1.dtbo {
description = "Flattened Device Tree blob";
data = [d0 0d fe ed 00 00 01 95 00 00 00 b8 00 00 01 6c 00 00 00 28 00 00 00 11 00 00 00
type = "flat_dt";
arch = "arm64";
compression = "none";
load = <0x50240000>;
hash-1 {
value = <0x06765191 0x32afa429 0x4be40760 0x2395a5c4 0x93a3eae0 0xd953e186 0xdda1d58
algo = "sha256";
};
signature-1 {
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x7f69b116 0x26ef63ef 0xde22a330 0x6d939e6e 0x6af86d90 0x7536fba7 0xb5749df
algo = "sha256,rsa2048";
key-name-hint = "dev2";
};
};
fdt-verdin-imx8mm_disable_can1.dtbo {
description = "Flattened Device Tree blob";
data = [d0 0d fe ed 00 00 01 92 00 00 00 b8 00 00 01 74 00 00 00 28 00 00 00 11 00 00 00
type = "flat_dt";
arch = "arm64";
compression = "none";
load = <0x50240000>;
hash-1 {
value = <0x5ecd4ad1 0x10505554 0x84edef9a 0x827785db 0xbda5c8d9 0xbf90041f 0x79b9b61
algo = "sha256";
};
signature-1 {
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x364a59ce 0x29275cae 0x65469f36 0x8969f3a5 0xf6c609e0 0xd9dd01e0 0x0604bd1
algo = "sha256,rsa2048";
key-name-hint = "dev2";
};
};
fdt-verdin-imx8mm_dsi-to-hdmi_overlay.dtbo {
description = "Flattened Device Tree blob";
data = <0xd00dfeed 0x00000af4 0x000000b8 0x00000998 0x00000028 0x00000011 0x00000010 0x0
type = "flat_dt";
arch = "arm64";
compression = "none";
load = <0x50240000>;
hash-1 {
value = <0xa48018bc 0x42bc78d2 0xc38b3773 0xea359df2 0x3fff0b48 0xe2222d69 0xad9b615
algo = "sha256";
};
signature-1 {
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x9deec084 0xa13e6055 0x41d8fe70 0xc5531dfd 0x0353ded1 0x8d6a8459 0xc9d77b2
algo = "sha256,rsa2048";
key-name-hint = "dev2";
};
};
fdt-verdin-imx8mm_dsi-to-lvds_panel-cap-touch-10inch-lvds_overlay.dtbo {
description = "Flattened Device Tree blob";
data = [d0 0d fe ed 00 00 0f 06 00 00 00 b8 00 00 0d 08 00 00 00 28 00 00 00 11 00 00 00
type = "flat_dt";
arch = "arm64";
compression = "none";
load = <0x50240000>;
hash-1 {
value = <0x5d18925f 0xa4e6db75 0x76bdc4a0 0xc951ff4e 0xcf1cde15 0x0fbab7b9 0x9eccac0
algo = "sha256";
};
signature-1 {
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0xba13e09f 0xccaeb63c 0xdd57c5ea 0x8c568036 0x2f47630d 0x30389375 0x68affe0
algo = "sha256,rsa2048";
key-name-hint = "dev2";
};
};
fdt-verdin-imx8mm_hmp_overlay.dtbo {
description = "Flattened Device Tree blob";
data = [d0 0d fe ed 00 00 08 62 00 00 00 b8 00 00 07 74 00 00 00 28 00 00 00 11 00 00 00
type = "flat_dt";
arch = "arm64";
compression = "none";
load = <0x50240000>;
hash-1 {
value = <0xc6513627 0xb4943e60 0x7572426b 0x6d78aabf 0x1eeadb09 0xba39c993 0x684969f
algo = "sha256";
};
signature-1 {
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x23728112 0x39141f88 0x73749db8 0x8915b95c 0x6fe29455 0xb24087eb 0x92909ef
algo = "sha256,rsa2048";
key-name-hint = "dev2";
};
};
fdt-verdin-imx8mm_nau8822-btl_overlay.dtbo {
description = "Flattened Device Tree blob";
data = [d0 0d fe ed 00 00 01 95 00 00 00 b8 00 00 01 68 00 00 00 28 00 00 00 11 00 00 00
type = "flat_dt";
arch = "arm64";
compression = "none";
load = <0x50240000>;
hash-1 {
value = <0x9f2bedc2 0x1803eea5 0x07727765 0x9f97374d 0x8967f710 0xe681d763 0x5a725f0
algo = "sha256";
};
signature-1 {
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x22a5b401 0x205dad36 0x8d6bbfb4 0x23f8b039 0xc85da829 0xe7df8c92 0x96c5fc5
algo = "sha256,rsa2048";
key-name-hint = "dev2";
};
};
fdt-verdin-imx8mm_ov5640_overlay.dtbo {
description = "Flattened Device Tree blob";
data = [d0 0d fe ed 00 00 0c 3b 00 00 00 b8 00 00 0a 70 00 00 00 28 00 00 00 11 00 00 00
type = "flat_dt";
arch = "arm64";
compression = "none";
load = <0x50240000>;
hash-1 {
value = <0x1c99f44d 0xc33efd07 0x4f6d8877 0x20c52995 0x043d2425 0x2656358c 0x23e1046
algo = "sha256";
};
signature-1 {
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x4330f3e2 0x7c926645 0x8c69c7b1 0x200c316c 0xabf68fd1 0x84a2edea 0x5da9af6
algo = "sha256,rsa2048";
key-name-hint = "dev2";
};
};
fdt-verdin-imx8mm_panel-cap-touch-10inch-dsi_overlay.dtbo {
description = "Flattened Device Tree blob";
data = <0xd00dfeed 0x00000e98 0x000000b8 0x00000c60 0x00000028 0x00000011 0x00000010 0x0
type = "flat_dt";
arch = "arm64";
compression = "none";
load = <0x50240000>;
hash-1 {
value = <0xc46e8252 0x65d12bd4 0xb2cb54c4 0xa93b0878 0x01051e21 0x79b42a6b 0xfcb9100
algo = "sha256";
};
signature-1 {
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x769840f2 0x559c4857 0xebf6f934 0x8b089bf5 0x2ff9b645 0x4e74254f 0x4c52a04
algo = "sha256,rsa2048";
key-name-hint = "dev2";
};
};
fdt-verdin-imx8mm-secboot-kargs_overlay.dtbo {
description = "Flattened Device Tree blob";
data = [d0 0d fe ed 00 00 01 da 00 00 00 b8 00 00 01 bc 00 00 00 28 00 00 00 11 00 00 00
type = "flat_dt";
arch = "arm64";
compression = "none";
load = <0x50240000>;
hash-1 {
value = <0x5b7f3572 0xd12bcb34 0xec72c53b 0x76414469 0x5625e965 0xbd5b166f 0xad185b3
algo = "sha256";
};
signature-1 {
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x17eee45f 0xde98f709 0x634d35c1 0x5fef0834 0x5c956e32 0x1fc934bf 0x7279bc7
algo = "sha256,rsa2048";
key-name-hint = "dev2";
};
};
fdt-verdin-imx8mm_spidev_overlay.dtbo {
description = "Flattened Device Tree blob";
data = [d0 0d fe ed 00 00 02 31 00 00 00 b8 00 00 01 e0 00 00 00 28 00 00 00 11 00 00 00
type = "flat_dt";
arch = "arm64";
compression = "none";
load = <0x50240000>;
hash-1 {
value = <0x34c3c748 0xa085815c 0x94364b33 0x79e6cb73 0xb4567935 0x9e3ae7a7 0x820f363
algo = "sha256";
};
signature-1 {
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0xae80cbca 0x060f426f 0x806edc2c 0xeac8cd85 0xe92b4bbe 0x56297dad 0xd551145
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 0x000000a9>;
hashed-nodes = "/", "/configurations/conf-freescale_imx8mm-verdin-nonwifi-dahlia.dtb
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x2ffb46a3 0x8293ba9d 0x2622d8f9 0xf0d6285d 0x55097645 0x55aaecc0 0x10bf05f
algo = "sha256,rsa2048";
key-name-hint = "dev";
padding = "pkcs-1.5";
sign-images = "kernel", "fdt";
};
};
conf-freescale_imx8mm-verdin-nonwifi-dev.dtb {
description = "0 Linux kernel, FDT blob";
kernel = "kernel-1";
fdt = "fdt-freescale_imx8mm-verdin-nonwifi-dev.dtb";
hash-1 {
algo = "sha256";
};
signature-1 {
hashed-strings = <0x00000000 0x000000c5>;
hashed-nodes = "/", "/configurations/conf-freescale_imx8mm-verdin-nonwifi-dev.dtb",
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x80408e6d 0xcb6b460a 0xc5c6bf2e 0x6f6b8f66 0x82b584fc 0x10cc9000 0x01ce3a4
algo = "sha256,rsa2048";
key-name-hint = "dev";
padding = "pkcs-1.5";
sign-images = "kernel", "fdt";
};
};
conf-freescale_imx8mm-verdin-nonwifi-mallow.dtb {
description = "0 Linux kernel, FDT blob";
kernel = "kernel-1";
fdt = "fdt-freescale_imx8mm-verdin-nonwifi-mallow.dtb";
hash-1 {
algo = "sha256";
};
signature-1 {
hashed-strings = <0x00000000 0x000000c5>;
hashed-nodes = "/", "/configurations/conf-freescale_imx8mm-verdin-nonwifi-mallow.dtb
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x26fbb946 0x05bed05f 0xa85054f9 0xc5d89144 0x86dbd55f 0x3322bab7 0x26a8c67
algo = "sha256,rsa2048";
key-name-hint = "dev";
padding = "pkcs-1.5";
sign-images = "kernel", "fdt";
};
};
conf-freescale_imx8mm-verdin-nonwifi-yavia.dtb {
description = "0 Linux kernel, FDT blob";
kernel = "kernel-1";
fdt = "fdt-freescale_imx8mm-verdin-nonwifi-yavia.dtb";
hash-1 {
algo = "sha256";
};
signature-1 {
hashed-strings = <0x00000000 0x000000c5>;
hashed-nodes = "/", "/configurations/conf-freescale_imx8mm-verdin-nonwifi-yavia.dtb"
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x50a403b8 0xc11ef631 0xadaee8ee 0xecce6caf 0x6d3dda83 0xfd3a3f6e 0x03f477d
algo = "sha256,rsa2048";
key-name-hint = "dev";
padding = "pkcs-1.5";
sign-images = "kernel", "fdt";
};
};
conf-freescale_imx8mm-verdin-wifi-dahlia.dtb {
description = "0 Linux kernel, FDT blob";
kernel = "kernel-1";
fdt = "fdt-freescale_imx8mm-verdin-wifi-dahlia.dtb";
hash-1 {
algo = "sha256";
};
signature-1 {
hashed-strings = <0x00000000 0x000000c5>;
hashed-nodes = "/", "/configurations/conf-freescale_imx8mm-verdin-wifi-dahlia.dtb",
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x6516a337 0x69308467 0x467d6c4b 0x2eec02d3 0xd1ad0096 0x0c0a3e99 0x1b1a91f
algo = "sha256,rsa2048";
key-name-hint = "dev";
padding = "pkcs-1.5";
sign-images = "kernel", "fdt";
};
};
conf-freescale_imx8mm-verdin-wifi-dev.dtb {
description = "0 Linux kernel, FDT blob";
kernel = "kernel-1";
fdt = "fdt-freescale_imx8mm-verdin-wifi-dev.dtb";
hash-1 {
algo = "sha256";
};
signature-1 {
hashed-strings = <0x00000000 0x000000c5>;
hashed-nodes = "/", "/configurations/conf-freescale_imx8mm-verdin-wifi-dev.dtb", "/i
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x28ff968e 0x4cf80198 0x64e9a1a3 0x16e8fb2a 0x33531d86 0x863ce366 0xb1f1975
algo = "sha256,rsa2048";
key-name-hint = "dev";
padding = "pkcs-1.5";
sign-images = "kernel", "fdt";
};
};
conf-freescale_imx8mm-verdin-wifi-mallow.dtb {
description = "0 Linux kernel, FDT blob";
kernel = "kernel-1";
fdt = "fdt-freescale_imx8mm-verdin-wifi-mallow.dtb";
hash-1 {
algo = "sha256";
};
signature-1 {
hashed-strings = <0x00000000 0x000000c5>;
hashed-nodes = "/", "/configurations/conf-freescale_imx8mm-verdin-wifi-mallow.dtb",
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x41c3078d 0x7523574e 0xdea2f5a9 0xc354ca12 0x3083b758 0x6df6cef7 0x8b6dfff
algo = "sha256,rsa2048";
key-name-hint = "dev";
padding = "pkcs-1.5";
sign-images = "kernel", "fdt";
};
};
conf-freescale_imx8mm-verdin-wifi-yavia.dtb {
description = "0 Linux kernel, FDT blob";
kernel = "kernel-1";
fdt = "fdt-freescale_imx8mm-verdin-wifi-yavia.dtb";
hash-1 {
algo = "sha256";
};
signature-1 {
hashed-strings = <0x00000000 0x000000c5>;
hashed-nodes = "/", "/configurations/conf-freescale_imx8mm-verdin-wifi-yavia.dtb", "
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x84ebdac2 0x4905be21 0xa5aeae31 0xbd5ba57a 0x757f833a 0xfd8183fc 0xa73eda6
algo = "sha256,rsa2048";
key-name-hint = "dev";
padding = "pkcs-1.5";
sign-images = "kernel", "fdt";
};
};
conf-verdin-imx8mm_20mhz_can1.dtbo {
description = "0 FDT blob";
fdt = "fdt-verdin-imx8mm_20mhz_can1.dtbo";
hash-1 {
algo = "sha256";
};
signature-1 {
hashed-strings = <0x00000000 0x000000c5>;
hashed-nodes = "/", "/configurations/conf-verdin-imx8mm_20mhz_can1.dtbo", "/images/f
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x38fd7fc1 0xda5ce8a3 0xf53f9a7d 0x2445a573 0x02f6c19f 0xac03ff4a 0xa354063
algo = "sha256,rsa2048";
key-name-hint = "dev";
padding = "pkcs-1.5";
sign-images = "fdt";
};
};
conf-verdin-imx8mm_disable_can1.dtbo {
description = "0 FDT blob";
fdt = "fdt-verdin-imx8mm_disable_can1.dtbo";
hash-1 {
algo = "sha256";
};
signature-1 {
hashed-strings = <0x00000000 0x000000c5>;
hashed-nodes = "/", "/configurations/conf-verdin-imx8mm_disable_can1.dtbo", "/images
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0xa2c01830 0x6b6934f2 0x26a634f4 0x5126934e 0x5871335b 0xcef84575 0xfed3df0
algo = "sha256,rsa2048";
key-name-hint = "dev";
padding = "pkcs-1.5";
sign-images = "fdt";
};
};
conf-verdin-imx8mm_dsi-to-hdmi_overlay.dtbo {
description = "0 FDT blob";
fdt = "fdt-verdin-imx8mm_dsi-to-hdmi_overlay.dtbo";
hash-1 {
algo = "sha256";
};
signature-1 {
hashed-strings = <0x00000000 0x000000c5>;
hashed-nodes = "/", "/configurations/conf-verdin-imx8mm_dsi-to-hdmi_overlay.dtbo", "
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x7387fd9a 0x4e144379 0x98c510e3 0x5b52dae6 0xee256b69 0xa01d6233 0x1951a30
algo = "sha256,rsa2048";
key-name-hint = "dev";
padding = "pkcs-1.5";
sign-images = "fdt";
};
};
conf-verdin-imx8mm_dsi-to-lvds_panel-cap-touch-10inch-lvds_overlay.dtbo {
description = "0 FDT blob";
fdt = "fdt-verdin-imx8mm_dsi-to-lvds_panel-cap-touch-10inch-lvds_overlay.dtbo";
hash-1 {
algo = "sha256";
};
signature-1 {
hashed-strings = <0x00000000 0x000000c5>;
hashed-nodes = "/", "/configurations/conf-verdin-imx8mm_dsi-to-lvds_panel-cap-touch-
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x12375c93 0x5168ecd9 0x5966edc5 0xccb9032d 0xeb44778d 0x3e492cd9 0x656ed91
algo = "sha256,rsa2048";
key-name-hint = "dev";
padding = "pkcs-1.5";
sign-images = "fdt";
};
};
conf-verdin-imx8mm_hmp_overlay.dtbo {
description = "0 FDT blob";
fdt = "fdt-verdin-imx8mm_hmp_overlay.dtbo";
hash-1 {
algo = "sha256";
};
signature-1 {
hashed-strings = <0x00000000 0x000000c5>;
hashed-nodes = "/", "/configurations/conf-verdin-imx8mm_hmp_overlay.dtbo", "/images/
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x0b5dad4a 0xce1faa1c 0x993c5245 0xaa253d6f 0xec95ac92 0xe8b9a025 0x3159dda
algo = "sha256,rsa2048";
key-name-hint = "dev";
padding = "pkcs-1.5";
sign-images = "fdt";
};
};
conf-verdin-imx8mm_nau8822-btl_overlay.dtbo {
description = "0 FDT blob";
fdt = "fdt-verdin-imx8mm_nau8822-btl_overlay.dtbo";
hash-1 {
algo = "sha256";
};
signature-1 {
hashed-strings = <0x00000000 0x000000c5>;
hashed-nodes = "/", "/configurations/conf-verdin-imx8mm_nau8822-btl_overlay.dtbo", "
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x199a7b2e 0xd3f1ab09 0xe6775318 0x3e1f80da 0xddb11fab 0xe14ead8f 0x39b2b98
algo = "sha256,rsa2048";
key-name-hint = "dev";
padding = "pkcs-1.5";
sign-images = "fdt";
};
};
conf-verdin-imx8mm_ov5640_overlay.dtbo {
description = "0 FDT blob";
fdt = "fdt-verdin-imx8mm_ov5640_overlay.dtbo";
hash-1 {
algo = "sha256";
};
signature-1 {
hashed-strings = <0x00000000 0x000000c5>;
hashed-nodes = "/", "/configurations/conf-verdin-imx8mm_ov5640_overlay.dtbo", "/imag
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0xb1d8bc6d 0x04733b59 0xf5a61178 0xd0442940 0x3a365dcb 0xa384840d 0x15e6b62
algo = "sha256,rsa2048";
key-name-hint = "dev";
padding = "pkcs-1.5";
sign-images = "fdt";
};
};
conf-verdin-imx8mm_panel-cap-touch-10inch-dsi_overlay.dtbo {
description = "0 FDT blob";
fdt = "fdt-verdin-imx8mm_panel-cap-touch-10inch-dsi_overlay.dtbo";
hash-1 {
algo = "sha256";
};
signature-1 {
hashed-strings = <0x00000000 0x000000c5>;
hashed-nodes = "/", "/configurations/conf-verdin-imx8mm_panel-cap-touch-10inch-dsi_o
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x62f71498 0xfdfd3e8f 0x11bba238 0x3ac08033 0x6ade2ddf 0xff6ba553 0x3cff575
algo = "sha256,rsa2048";
key-name-hint = "dev";
padding = "pkcs-1.5";
sign-images = "fdt";
};
};
conf-verdin-imx8mm-secboot-kargs_overlay.dtbo {
description = "0 FDT blob";
fdt = "fdt-verdin-imx8mm-secboot-kargs_overlay.dtbo";
hash-1 {
algo = "sha256";
};
signature-1 {
hashed-strings = <0x00000000 0x000000c5>;
hashed-nodes = "/", "/configurations/conf-verdin-imx8mm-secboot-kargs_overlay.dtbo",
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x47c97d4e 0x53310ded 0x0d75ac31 0xd75c4aba 0x766ebdc7 0x45c0f7c7 0x7026361
algo = "sha256,rsa2048";
key-name-hint = "dev";
padding = "pkcs-1.5";
sign-images = "fdt";
};
};
conf-verdin-imx8mm_spidev_overlay.dtbo {
description = "0 FDT blob";
fdt = "fdt-verdin-imx8mm_spidev_overlay.dtbo";
hash-1 {
algo = "sha256";
};
signature-1 {
hashed-strings = <0x00000000 0x000000c5>;
hashed-nodes = "/", "/configurations/conf-verdin-imx8mm_spidev_overlay.dtbo", "/imag
timestamp = <0x00000000>;
signer-version = "2022.01";
signer-name = "mkimage";
value = <0x043d3eec 0x53d3e418 0x9c9afed4 0x0cdd1abb 0x828b3242 0xd4c43a2b 0xda191c0
algo = "sha256,rsa2048";
key-name-hint = "dev";
padding = "pkcs-1.5";
sign-images = "fdt";
};
};
};
};
[-- Attachment #4: fdtdump-kirkstone-u-boot-dtb.txt --]
[-- Type: text/plain, Size: 4098 bytes --]
deploy/images/verdin-imx8mm/u-boot-verdin-imx8mm-2022.04-r0.dtb: found fdt at offset 0
/dts-v1/;
// magic: 0xd00dfeed
// totalsize: 0x139a0 (80288)
// off_dt_struct: 0x38
// off_dt_strings: 0xd7d0
// off_mem_rsvmap: 0x28
// version: 17
// last_comp_version: 16
// boot_cpuid_phys: 0x0
// size_dt_strings: 0x1381
// size_dt_struct: 0xd798
/ {
interrupt-parent = <0x00000001>;
#address-cells = <0x00000002>;
#size-cells = <0x00000002>;
model = "Toradex Verdin iMX8M Mini Quad/DualLite";
compatible = "toradex,verdin-imx8mm", "fsl,imx8mm";
signature {
key-dev {
required = "conf";
algo = "sha256,rsa2048";
rsa,r-squared = <0x06d11c22 0x56f31661 0x133143bb 0xfdbcde51 0xd653d2a5 0xcd139dfd 0x4b6
rsa,modulus = <0xdb06864a 0x12a1e033 0x04b0b457 0x46e5d967 0xc6a6f78d 0x3132b66d 0xf1598
rsa,exponent = <0x00000000 0x00010001>;
rsa,n0-inverse = <0x8925c5c7>;
rsa,num-bits = <0x00000800>;
key-name-hint = "dev";
};
key-dev2 {
required = "image";
algo = "sha256,rsa2048";
rsa,r-squared = <0x3956dbd5 0x8b651d3b 0xa621244b 0xd0c718fd 0x6c8a2743 0x6d973b4f 0x197
rsa,modulus = <0xbcb94b0f 0x77f059c2 0xfb77ccac 0x67e7735e 0xc5626747 0x0d39b30c 0x5b347
rsa,exponent = <0x00000000 0x00010001>;
rsa,n0-inverse = <0x707813fd>;
rsa,num-bits = <0x00000800>;
key-name-hint = "dev2";
};
};
aliases {
ethernet0 = "/soc@0/bus@30800000/ethernet@30be0000";
gpio0 = "/soc@0/bus@30000000/gpio@30200000";
gpio1 = "/soc@0/bus@30000000/gpio@30210000";
gpio2 = "/soc@0/bus@30000000/gpio@30220000";
gpio3 = "/soc@0/bus@30000000/gpio@30230000";
gpio4 = "/soc@0/bus@30000000/gpio@30240000";
i2c0 = "/soc@0/bus@30800000/i2c@30a20000";
i2c1 = "/soc@0/bus@30800000/i2c@30a30000";
i2c2 = "/soc@0/bus@30800000/i2c@30a40000";
i2c3 = "/soc@0/bus@30800000/i2c@30a50000";
mmc0 = "/soc@0/bus@30800000/mmc@30b40000";
mmc1 = "/soc@0/bus@30800000/mmc@30b50000";
mmc2 = "/soc@0/bus@30800000/mmc@30b60000";
serial0 = "/soc@0/bus@30800000/serial@30860000";
serial1 = "/soc@0/bus@30800000/serial@30890000";
serial2 = "/soc@0/bus@30800000/serial@30880000";
serial3 = "/soc@0/bus@30800000/serial@30a60000";
spi0 = "/soc@0/bus@30800000/spi@30820000";
spi1 = "/soc@0/bus@30800000/spi@30830000";
spi2 = "/soc@0/bus@30800000/spi@30840000";
usb0 = "/soc@0/bus@32c00000/usb@32e40000";
usb1 = "/soc@0/bus@32c00000/usb@32e50000";
video0 = "/soc@0/bus@32c00000/lcdif@32e00000";
eeprom0 = "/soc@0/bus@30800000/i2c@30a20000/eeprom@50";
eeprom1 = "/soc@0/bus@30800000/i2c@30a50000/eeprom@57";
eeprom2 = "/soc@0/bus@30800000/i2c@30a50000/eeprom@50";
usbgadget0 = "/usbg1";
};
cpus {
#address-cells = <0x00000001>;
#size-cells = <0x00000000>;
idle-states {
entry-method = "psci";
cpu-pd-wait {
compatible = "arm,idle-state";
arm,psci-suspend-param = <0x00010033>;
local-timer-stop;
entry-latency-us = <0x000003e8>;
exit-latency-us = <0x000002bc>;
min-residency-us = <0x00000a8c>;
phandle = <0x00000006>;
};
};
cpu@0 {
device_type = "cpu";
compatible = "arm,cortex-a53";
reg = <0x00000000>;
clock-latency = <0x0000ee6c>;
clocks = <0x00000002 0x000000d7>;
enable-method = "psci";
next-level-cache = <0x00000003>;
operating-points-v2 = <0x00000004>;
nvmem-cells = <0x00000005>;
nvmem-cell-names = "speed_grade";
cpu-idle-states = <0x00000006>;
#cooling-cells = <0x00000002>;
arm-supply = <0x00000007>;
phandle = <0x00000008>;
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [OE-core] Broken FIT image signing since "u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled"
2025-02-18 3:49 ` Rogerio Guerra Borin
@ 2025-02-20 10:46 ` Jose Quaresma
2025-02-20 14:42 ` Leonard Anderweit
2025-02-20 23:04 ` Marek Vasut
1 sibling, 1 reply; 15+ messages in thread
From: Jose Quaresma @ 2025-02-20 10:46 UTC (permalink / raw)
To: rogerio.borin
Cc: Marek Vasut, Quentin Schulz, openembedded-core, Richard Purdie,
Freihofer, Adrian
[-- Attachment #1: Type: text/plain, Size: 11981 bytes --]
Hi,
Any news about this regression?
If we don't get a fix for this bug asap maybe we should revert and drop
this patch since it broke our scarthgap stable LTS
Jose
Rogerio Guerra Borin via lists.openembedded.org <rogerio.borin=
toradex.com@lists.openembedded.org> escreveu (terça, 18/02/2025 à(s) 03:49):
> > This message originated from outside your organization
> >
> > On 2/17/25 11:34 AM, Quentin Schulz wrote:
> > > +Cc Marek
> >
> > Hi,
>
> Thanks for the quick answer!
>
> >
> > > On 2/16/25 7:36 PM, Rogerio Guerra Borin via lists.openembedded.org
> > wrote:
> > >> You don't often get email from
> > >> rogerio.borin=toradex.com@lists.openembedded.org. Learn why this is
> > >> important<https://aka.ms/LearnAboutSenderIdentification
> > <https://aka.ms/LearnAboutSenderIdentification>>
> > >> Hello,
> > >>
> > >> At Toradex we're using class "kernel-fitimage" to produce our signed
> > >> kernel FIT images and we've found that the recent commit d7bd9c627661
> > >> ("u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE
> > >> and UBOOT_ENV enabled") broke our builds. The logs of a failed build
> > >> show the following message that I found noteworthy:
> >
> > Can you mail me the entire log file for completeness ?
>
> Sure.
>
> I've added it as attachment (log.do_uboot_assemble_fitimage.841485).
>
> > >> log.do_uboot_assemble_fitimage.841485:
> > >> ...
> > >> Couldn't open RSA private key: '/secboot-data/fit-keys/dev2.key': No
> > >> such file or directory
> > >>
> > >> The odd thing was that the build was setting:
> > >>
> > >> UBOOT_SIGN_KEYNAME = "dev"
> > >> UBOOT_SIGN_IMG_KEYNAME = "dev2"
> > >> FIT_SIGN_INDIVIDUAL = "0"
> > >> FIT_GENERATE_KEYS = "0"
> >
> > FIT_SIGN_INDIVIDUAL = "0" is correct, you do want to sign
> > configurations, not individual images in the fitImage, because signing
> > individual images is susceptible to mix-and-match attack.
> >
> > >> From the above we see the individual images signing is disabled but
> > >> the system is trying to access the key file for the individual
> signing
> > >> (dev2) regardless of that. Checking the code in uboot-sign.bbclass I
> > >> found this line:
> > >>
> > >> https://git.openembedded.org/openembedded-core/tree/meta/classes-
> > <https://git.openembedded.org/openembedded-core/tree/meta/classes->
> > >> recipe/uboot-sign.bbclass?h=scarthgap#n116
> > >>
> > >> that is responsible for that access (inside function concat_dtb()).
> > >> The code is always referencing variable UBOOT_SIGN_IMG_KEYNAME
> > >> regardless of FIT_SIGN_INDIVIDUAL being set to "1" or "0" even though
> > >> there's a condition a couple lines above L116 where
> > >> FIT_SIGN_INDIVIDUAL is tested to decide in which mode to run the
> > >> signing tool.
> >
> > Uh, good find, does the following patch fix your problem ?
> >
> > """
> > diff --git a/meta/classes-recipe/uboot-sign.bbclass
> > b/meta/classes-recipe/uboot-sign.bbclass
> > index 96c47ab016..74ef5697ca 100644
> > --- a/meta/classes-recipe/uboot-sign.bbclass
> > +++ b/meta/classes-recipe/uboot-sign.bbclass
> > @@ -107,13 +107,16 @@ concat_dtb() {
> > # makes fitImage susceptible to mix-and-match attack.
> > if [ "${FIT_SIGN_INDIVIDUAL}" = "1" ] ; then
> > UBOOT_MKIMAGE_MODE="auto"
> > + UBOOT_MKIMAGE_KEYNAME=${UBOOT_SIGN_IMG_KEYNAME}
> > + else
> > + UBOOT_MKIMAGE_KEYNAME=${UBOOT_SIGN_KEYNAME}
> > fi
> > ${UBOOT_MKIMAGE_SIGN} \
> > ${@'-D "${UBOOT_MKIMAGE_DTCOPTS}"' if
> > len('${UBOOT_MKIMAGE_DTCOPTS}') else ''} \
> > -f $UBOOT_MKIMAGE_MODE \
> > -k "${UBOOT_SIGN_KEYDIR}" \
> > -o "${FIT_HASH_ALG},${FIT_SIGN_ALG}" \
> > - -g "${UBOOT_SIGN_IMG_KEYNAME}" \
> > + -g "$UBOOT_MKIMAGE_KEYNAME" \
> > -K "${UBOOT_DTB_BINARY}" \
> > -d /dev/null \
> > -r ${B}/unused.itb \
> > """
>
> Looking at the patch I'm pretty sure it would solve my problem,
> particularly because we set FIT_SIGN_INDIVIDUAL="0" in our builds.
> However, I see it keeps the concept of the signing being either on the
> config or on the individual images, which I wanted to further comment
> down below.
>
> > >> To better understand the behavior I created dev2.
> > >> {key,crt} files and retried a build which worked. However, checking
> > >> the contents of the final u-boot.dtb file I found that the "dev2" key
> > >> node was present but the "dev" key wasn't. Then, checking the
> contents
> > >> of the final fitImage I noticed the configuration nodes were signed
> by
> > >> the dev key while the individual images were not signed (as
> expected).
> > >> But that means such a system wouldn't boot since the required "dev"
> > >> key is not present in the U-Boot DTB, from what I conclude the FIT
> > >> signing is broken at the moment.
> > >>
> > >> So, first of all I wanted to let you guys know about this issue
> > >> (hoping we're not doing anything wrong with our builds).
> >
> > No, your builds are fine, this is a real bug.
>
> Thanks for the confirmation.
>
> > >> As a second point, I wanted to confirm the expected behavior
> regarding
> > >> variable FIT_SIGN_INDIVIDUAL. According to the documentation:
> > >>
> > >> https://docs.yoctoproject.org/5.0.6/ref-manual/variables.html#term-
> > <https://docs.yoctoproject.org/5.0.6/ref-manual/variables.html#term->
> > >> FIT_SIGN_INDIVIDUAL
> > >>
> > >> when its value is "1" the individual images will be signed *in
> > >> addition* to the FIT image being signed.
> >
> > That is a bit weird and no, I don't think that is how it is supposed to
> > work.
> >
> > FIT_SIGN_INDIVIDUAL=1 -> only image subnodes in fitImage /images nodes
> > are signed AND fitImage is susceptible to mix-and-match attack
> > (basically attacker can replace individual signed images in fitImage
> > with different individual signed images, e.g. with older versions, to
> > achieve a combination of images which allows them to somehow break the
> > system).
> >
> > FIT_SIGN_INDIVIDUAL=0 (recommended) -> images are hashed, configurations
> > are hashed, hashes are combined and signed. Because images and
> > configurations (which images are to be used together) are signed,
> > attacker cannot mix-and-match images into an un-allowed configuration.
> >
> > >> From that, I understand that
> > >> the configurations will be always signed and the individual images
> may
> > >> or may not be signed depending on FIT_SIGN_INDIVIDUAL.
> >
> > The other way around. The configurations are signed by default, which is
> > good, but this can be disabled if needed (unlikely).
> >
> > >> I kind of
> > >> confirmed this behavior by checking our build artifacts on kirkstone:
> > >> with FIT_SIGN_INDIVIDUAL="0" the U-Boot DTB has only the "dev" key
> and
> > >> the fit image is signed by that same key; with
> FIT_SIGN_INDIVIDUAL="1"
> > >> both keys are present in the DTB, the configurations are signed by
> the
> > >> "dev" key and the images by the "dev2" key. I understand this is the
> > >> correct behavior, right?
> >
> > Images shouldn't be signed by different keys than configurations, that
> > is a bit odd.
> >
> > >> I'm asking because looking at the comment at:
> > >>
> > >> https://git.openembedded.org/openembedded-core/tree/meta/classes-
> > <https://git.openembedded.org/openembedded-core/tree/meta/classes->
> > >> recipe/uboot-sign.bbclass?h=scarthgap#n106
> > >>
> > >> it gives me the impression that FIT_SIGN_INDIVIDUAL would select
> > >> between either signing individual images
> >
> > Yes
> >
> > >> or signing the
> > >> configurations
> >
> > Signed configuration also includes hash of the images that the
> > configuration references in it, that's what prevents the mix-and-match
> > attack.
> >
> > >> , which doesn't match the documented behavior. I believe
> > >> the code in concat_dtb() also seems to assume only one of the keys
> > >> (dev/dev2) would need be present in the DTB. So the fix would involve
> > >> a review on this as well I imagine.
> > IIRC Adrian was looking into improving those docs.
>
> Regarding this second topic, let me show you some other information I
> gathered from a couple of builds that may help with your assessment:
>
> FSI := FIT_SIGN_INDIVIDUAL
> CFG := Whether or not the configurations inside the FIT image were signed
> IMG := Whether or not the subimages inside the FIT image were signed
>
> | Branch | Version | FSI | Keys inside DTB | CFG | IMG |
> |-----------+---------------+-----+------------------+-----+-----|
> | kirkstone | latest | "0" | {dev} | Y | N |
> | kirkstone | latest | "1" | {dev,dev2} | Y | Y |
> |-----------+---------------+-----+------------------+-----+-----|
> | scarthgap | latest | "0" | Build failed (!) | n/a | n/a |
> | | + dummy dev2 | | {dev2} | Y | N |
> | scarthgap | latest | "1" | {dev2} (!) | Y | Y |
> |-----------+---------------+-----+------------------+-----+-----|
> | scarthgap | w/o @d7bd9c62 | "0" | {dev} | Y | N |
> | scarthgap | w/o @d7bd9c62 | "1" | {dev,dev2} | Y | Y |
>
> The first two lines show that on kirkstone when FIT_SIGN_INDIVIDUAL="0"
> only the configurations are signed and the DTB contains only the "dev"
> key. When FIT_SIGN_INDIVIDUAL="1" both "dev" and "dev2" are present in
> the DTB and both the configurations and the images are signed. And
> that's the exact same behavior I got on scarthgap after reverting the
> commit d7bd9c62. And, as I said, this seems to match the documentation.
> I've attached snippets of the fdtdump for the FIT image and U-Boot DTB
> from my build on kirkstone to illustrate what I said.
>
> So, if the expected behavior is according to what you described then the
> behavior so far was wrong (for a long time) and the new behavior is
> introducing a breaking change for anyone relying on what's documented
> for the FIT_SIGN_INDIVIDUAL="1" case, right? Btw, for us at Toradex this
> is not a big deal because we don't fall into that case but considering
> the general use of the layer the change in behavior could be an issue.
>
> Another piece of information that might be useful: I made a change to
> concat_dtb() to make it add both keys (dev/dev2) to the DTB (by invoking
> mkimage twice) and I found the invocation to "uboot-fit_check_sign"
> failed afterwards. I confirmed that the DTB had the two keys, but
> because the FIT image only had the configurations signed, the tool threw
> an error saying the signature of "require key dev2" could not be
> verified but the showed that the check of the "dev" key succeeded.
> Unfortunately I don't have the logs available for that case, but I could
> repeat the tests if needed. Anyway, my point is that the
> "uboot-fit_check_sign" tool seems capable of checking both keys as well.
>
> Interestingly, if I removed the verification step (performed on the
> "unused.itb" file), then the image generation succeeded and everything
> seemed to work as before both when FIT_SIGN_INDIVIDUAL is "0" or "1".
> So, it's not hard to get the old behavior, though we'd need to figure
> out how to properly do the verification when mkimage is invoked with -f
> "auto" or -f "auto-conf". I could post my unfinished patch if helpful.
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#211561):
> https://lists.openembedded.org/g/openembedded-core/message/211561
> Mute This Topic: https://lists.openembedded.org/mt/111218371/5052612
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> quaresma.jose@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
--
Best regards,
José Quaresma
[-- Attachment #2: Type: text/html, Size: 16367 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [OE-core] Broken FIT image signing since "u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled"
2025-02-20 10:46 ` Jose Quaresma
@ 2025-02-20 14:42 ` Leonard Anderweit
2025-02-20 16:26 ` Jose Quaresma
2025-02-20 23:05 ` Marek Vasut
0 siblings, 2 replies; 15+ messages in thread
From: Leonard Anderweit @ 2025-02-20 14:42 UTC (permalink / raw)
To: rogerio.borin@toradex.com, quaresma.jose@gmail.com
Cc: quentin.schulz@cherry.de, marex@denx.de,
openembedded-core@lists.openembedded.org,
adrian.freihofer@siemens.com, richard.purdie@linuxfoundation.org
Hi,
I've also noticed this bug and sent patch [1] to fix it.
[1] https://lists.openembedded.org/g/openembedded-core/message/211761
Leonard
Am Donnerstag, dem 20.02.2025 um 10:46 +0000 schrieb Jose Quaresma via
lists.openembedded.org:
> Hi,
>
> Any news about this regression?
> If we don't get a fix for this bug asap maybe we should revert and
> drop this patch since it broke our scarthgap stable LTS
>
> Jose
>
>
> Rogerio Guerra Borin via lists.openembedded.org
> <rogerio.borin=toradex.com@lists.openembedded.org> escreveu (terça,
> 18/02/2025 à(s) 03:49):
> > > This message originated from outside your organization
> > >
> > > On 2/17/25 11:34 AM, Quentin Schulz wrote:
> > > > +Cc Marek
> > >
> > > Hi,
> >
> > Thanks for the quick answer!
> >
> > >
> > > > On 2/16/25 7:36 PM, Rogerio Guerra Borin via
> > > lists.openembedded.org
> > > wrote:
> > > >> You don't often get email from
> > > >> rogerio.borin=toradex.com@lists.openembedded.org. Learn why
> > > this is
> > > >> important<https://aka.ms/LearnAboutSenderIdentification
> > > <https://aka.ms/LearnAboutSenderIdentification>>
> > > >> Hello,
> > > >>
> > > >> At Toradex we're using class "kernel-fitimage" to produce
> > > our signed
> > > >> kernel FIT images and we've found that the recent commit
> > > d7bd9c627661
> > > >> ("u-boot: kernel-fitimage: Fix dependency loop if
> > > UBOOT_SIGN_ENABLE
> > > >> and UBOOT_ENV enabled") broke our builds. The logs of a
> > > failed build
> > > >> show the following message that I found noteworthy:
> > >
> > > Can you mail me the entire log file for completeness ?
> >
> > Sure.
> >
> > I've added it as attachment
> > (log.do_uboot_assemble_fitimage.841485).
> >
> > > >> log.do_uboot_assemble_fitimage.841485:
> > > >> ...
> > > >> Couldn't open RSA private key: '/secboot-data/fit-
> > > keys/dev2.key': No
> > > >> such file or directory
> > > >>
> > > >> The odd thing was that the build was setting:
> > > >>
> > > >> UBOOT_SIGN_KEYNAME = "dev"
> > > >> UBOOT_SIGN_IMG_KEYNAME = "dev2"
> > > >> FIT_SIGN_INDIVIDUAL = "0"
> > > >> FIT_GENERATE_KEYS = "0"
> > >
> > > FIT_SIGN_INDIVIDUAL = "0" is correct, you do want to sign
> > > configurations, not individual images in the fitImage, because
> > > signing
> > > individual images is susceptible to mix-and-match attack.
> > >
> > > >> From the above we see the individual images signing is
> > > disabled but
> > > >> the system is trying to access the key file for the
> > > individual signing
> > > >> (dev2) regardless of that. Checking the code in uboot-
> > > sign.bbclass I
> > > >> found this line:
> > > >>
> > > >>
> > > https://git.openembedded.org/openembedded-core/tree/meta/classes-
> > > <
> > > https://git.openembedded.org/openembedded-core/tree/meta/classes-
> > > >
> > > >> recipe/uboot-sign.bbclass?h=scarthgap#n116
> > > >>
> > > >> that is responsible for that access (inside function
> > > concat_dtb()).
> > > >> The code is always referencing variable
> > > UBOOT_SIGN_IMG_KEYNAME
> > > >> regardless of FIT_SIGN_INDIVIDUAL being set to "1" or "0"
> > > even though
> > > >> there's a condition a couple lines above L116 where
> > > >> FIT_SIGN_INDIVIDUAL is tested to decide in which mode to run
> > > the
> > > >> signing tool.
> > >
> > > Uh, good find, does the following patch fix your problem ?
> > >
> > > """
> > > diff --git a/meta/classes-recipe/uboot-sign.bbclass
> > > b/meta/classes-recipe/uboot-sign.bbclass
> > > index 96c47ab016..74ef5697ca 100644
> > > --- a/meta/classes-recipe/uboot-sign.bbclass
> > > +++ b/meta/classes-recipe/uboot-sign.bbclass
> > > @@ -107,13 +107,16 @@ concat_dtb() {
> > > # makes fitImage susceptible to mix-and-match attack.
> > > if [ "${FIT_SIGN_INDIVIDUAL}" = "1" ] ; then
> > > UBOOT_MKIMAGE_MODE="auto"
> > > + UBOOT_MKIMAGE_KEYNAME=${UBOOT_SIGN_IMG_KEYNAME}
> > > + else
> > > + UBOOT_MKIMAGE_KEYNAME=${UBOOT_SIGN_KEYNAME}
> > > fi
> > > ${UBOOT_MKIMAGE_SIGN} \
> > > ${@'-D "${UBOOT_MKIMAGE_DTCOPTS}"' if
> > > len('${UBOOT_MKIMAGE_DTCOPTS}') else ''} \
> > > -f $UBOOT_MKIMAGE_MODE \
> > > -k "${UBOOT_SIGN_KEYDIR}" \
> > > -o "${FIT_HASH_ALG},${FIT_SIGN_ALG}" \
> > > - -g "${UBOOT_SIGN_IMG_KEYNAME}" \
> > > + -g "$UBOOT_MKIMAGE_KEYNAME" \
> > > -K "${UBOOT_DTB_BINARY}" \
> > > -d /dev/null \
> > > -r ${B}/unused.itb \
> > > """
> >
> > Looking at the patch I'm pretty sure it would solve my problem,
> > particularly because we set FIT_SIGN_INDIVIDUAL="0" in our builds.
> > However, I see it keeps the concept of the signing being either on
> > the
> > config or on the individual images, which I wanted to further
> > comment
> > down below.
> >
> > > >> To better understand the behavior I created dev2.
> > > >> {key,crt} files and retried a build which worked. However,
> > > checking
> > > >> the contents of the final u-boot.dtb file I found that the
> > > "dev2" key
> > > >> node was present but the "dev" key wasn't. Then, checking
> > > the contents
> > > >> of the final fitImage I noticed the configuration nodes were
> > > signed by
> > > >> the dev key while the individual images were not signed (as
> > > expected).
> > > >> But that means such a system wouldn't boot since the
> > > required "dev"
> > > >> key is not present in the U-Boot DTB, from what I conclude
> > > the FIT
> > > >> signing is broken at the moment.
> > > >>
> > > >> So, first of all I wanted to let you guys know about this
> > > issue
> > > >> (hoping we're not doing anything wrong with our builds).
> > >
> > > No, your builds are fine, this is a real bug.
> >
> > Thanks for the confirmation.
> >
> > > >> As a second point, I wanted to confirm the expected behavior
> > > regarding
> > > >> variable FIT_SIGN_INDIVIDUAL. According to the
> > > documentation:
> > > >>
> > > >>
> > > https://docs.yoctoproject.org/5.0.6/ref-manual/variables.html#term
> > > -
> > > <
> > > https://docs.yoctoproject.org/5.0.6/ref-manual/variables.html#term
> > > ->
> > > >> FIT_SIGN_INDIVIDUAL
> > > >>
> > > >> when its value is "1" the individual images will be signed
> > > *in
> > > >> addition* to the FIT image being signed.
> > >
> > > That is a bit weird and no, I don't think that is how it is
> > > supposed to
> > > work.
> > >
> > > FIT_SIGN_INDIVIDUAL=1 -> only image subnodes in fitImage /images
> > > nodes
> > > are signed AND fitImage is susceptible to mix-and-match attack
> > > (basically attacker can replace individual signed images in
> > > fitImage
> > > with different individual signed images, e.g. with older
> > > versions, to
> > > achieve a combination of images which allows them to somehow
> > > break the
> > > system).
> > >
> > > FIT_SIGN_INDIVIDUAL=0 (recommended) -> images are hashed,
> > > configurations
> > > are hashed, hashes are combined and signed. Because images and
> > > configurations (which images are to be used together) are signed,
> > > attacker cannot mix-and-match images into an un-allowed
> > > configuration.
> > >
> > > >> From that, I understand that
> > > >> the configurations will be always signed and the individual
> > > images may
> > > >> or may not be signed depending on FIT_SIGN_INDIVIDUAL.
> > >
> > > The other way around. The configurations are signed by default,
> > > which is
> > > good, but this can be disabled if needed (unlikely).
> > >
> > > >> I kind of
> > > >> confirmed this behavior by checking our build artifacts on
> > > kirkstone:
> > > >> with FIT_SIGN_INDIVIDUAL="0" the U-Boot DTB has only the
> > > "dev" key and
> > > >> the fit image is signed by that same key; with
> > > FIT_SIGN_INDIVIDUAL="1"
> > > >> both keys are present in the DTB, the configurations are
> > > signed by the
> > > >> "dev" key and the images by the "dev2" key. I understand
> > > this is the
> > > >> correct behavior, right?
> > >
> > > Images shouldn't be signed by different keys than configurations,
> > > that
> > > is a bit odd.
> > >
> > > >> I'm asking because looking at the comment at:
> > > >>
> > > >>
> > > https://git.openembedded.org/openembedded-core/tree/meta/classes-
> > > <
> > > https://git.openembedded.org/openembedded-core/tree/meta/classes-
> > > >
> > > >> recipe/uboot-sign.bbclass?h=scarthgap#n106
> > > >>
> > > >> it gives me the impression that FIT_SIGN_INDIVIDUAL would
> > > select
> > > >> between either signing individual images
> > >
> > > Yes
> > >
> > > >> or signing the
> > > >> configurations
> > >
> > > Signed configuration also includes hash of the images that the
> > > configuration references in it, that's what prevents the mix-and-
> > > match
> > > attack.
> > >
> > > >> , which doesn't match the documented behavior. I believe
> > > >> the code in concat_dtb() also seems to assume only one of
> > > the keys
> > > >> (dev/dev2) would need be present in the DTB. So the fix
> > > would involve
> > > >> a review on this as well I imagine.
> > > IIRC Adrian was looking into improving those docs.
> >
> > Regarding this second topic, let me show you some other information
> > I
> > gathered from a couple of builds that may help with your
> > assessment:
> >
> > FSI := FIT_SIGN_INDIVIDUAL
> > CFG := Whether or not the configurations inside the FIT image were
> > signed
> > IMG := Whether or not the subimages inside the FIT image were
> > signed
> >
> > | Branch | Version | FSI | Keys inside DTB | CFG | IMG |
> > |-----------+---------------+-----+------------------+-----+-----|
> > | kirkstone | latest | "0" | {dev} | Y | N |
> > | kirkstone | latest | "1" | {dev,dev2} | Y | Y |
> > |-----------+---------------+-----+------------------+-----+-----|
> > | scarthgap | latest | "0" | Build failed (!) | n/a | n/a |
> > | | + dummy dev2 | | {dev2} | Y | N |
> > | scarthgap | latest | "1" | {dev2} (!) | Y | Y |
> > |-----------+---------------+-----+------------------+-----+-----|
> > | scarthgap | w/o @d7bd9c62 | "0" | {dev} | Y | N |
> > | scarthgap | w/o @d7bd9c62 | "1" | {dev,dev2} | Y | Y |
> >
> > The first two lines show that on kirkstone when
> > FIT_SIGN_INDIVIDUAL="0"
> > only the configurations are signed and the DTB contains only the
> > "dev"
> > key. When FIT_SIGN_INDIVIDUAL="1" both "dev" and "dev2" are present
> > in
> > the DTB and both the configurations and the images are signed. And
> > that's the exact same behavior I got on scarthgap after reverting
> > the
> > commit d7bd9c62. And, as I said, this seems to match the
> > documentation.
> > I've attached snippets of the fdtdump for the FIT image and U-Boot
> > DTB
> > from my build on kirkstone to illustrate what I said.
> >
> > So, if the expected behavior is according to what you described
> > then the
> > behavior so far was wrong (for a long time) and the new behavior is
> > introducing a breaking change for anyone relying on what's
> > documented
> > for the FIT_SIGN_INDIVIDUAL="1" case, right? Btw, for us at Toradex
> > this
> > is not a big deal because we don't fall into that case but
> > considering
> > the general use of the layer the change in behavior could be an
> > issue.
> >
> > Another piece of information that might be useful: I made a change
> > to
> > concat_dtb() to make it add both keys (dev/dev2) to the DTB (by
> > invoking
> > mkimage twice) and I found the invocation to "uboot-fit_check_sign"
> > failed afterwards. I confirmed that the DTB had the two keys, but
> > because the FIT image only had the configurations signed, the tool
> > threw
> > an error saying the signature of "require key dev2" could not be
> > verified but the showed that the check of the "dev" key succeeded.
> > Unfortunately I don't have the logs available for that case, but I
> > could
> > repeat the tests if needed. Anyway, my point is that the
> > "uboot-fit_check_sign" tool seems capable of checking both keys as
> > well.
> >
> > Interestingly, if I removed the verification step (performed on the
> > "unused.itb" file), then the image generation succeeded and
> > everything
> > seemed to work as before both when FIT_SIGN_INDIVIDUAL is "0" or
> > "1".
> > So, it's not hard to get the old behavior, though we'd need to
> > figure
> > out how to properly do the verification when mkimage is invoked
> > with -f
> > "auto" or -f "auto-conf". I could post my unfinished patch if
> > helpful.
> >
> >
> >
>
>
> --
> Best regards,
>
> José Quaresma
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#211753):
> https://lists.openembedded.org/g/openembedded-core/message/211753
> Mute This Topic: https://lists.openembedded.org/mt/111218371/7270266
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe:
> https://lists.openembedded.org/g/openembedded-core/unsub [
> L.Anderweit@phytec.de]
> -=-=-=-=-=-=-=-=-=-=-=-
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [OE-core] Broken FIT image signing since "u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled"
2025-02-20 14:42 ` Leonard Anderweit
@ 2025-02-20 16:26 ` Jose Quaresma
2025-02-20 23:05 ` Marek Vasut
1 sibling, 0 replies; 15+ messages in thread
From: Jose Quaresma @ 2025-02-20 16:26 UTC (permalink / raw)
To: Leonard Anderweit
Cc: rogerio.borin@toradex.com, quentin.schulz@cherry.de,
marex@denx.de, openembedded-core@lists.openembedded.org,
adrian.freihofer@siemens.com, richard.purdie@linuxfoundation.org
[-- Attachment #1: Type: text/plain, Size: 14274 bytes --]
Hi Leonard,
I will test on my side and then I'll come back with the results.
Thanks
Jose
Leonard Anderweit <L.Anderweit@phytec.de> escreveu (quinta, 20/02/2025 à(s)
14:42):
> Hi,
>
> I've also noticed this bug and sent patch [1] to fix it.
>
> [1] https://lists.openembedded.org/g/openembedded-core/message/211761
>
> Leonard
>
> Am Donnerstag, dem 20.02.2025 um 10:46 +0000 schrieb Jose Quaresma via
> lists.openembedded.org:
> > Hi,
> >
> > Any news about this regression?
> > If we don't get a fix for this bug asap maybe we should revert and
> > drop this patch since it broke our scarthgap stable LTS
> >
> > Jose
> >
> >
> > Rogerio Guerra Borin via lists.openembedded.org
> > <rogerio.borin=toradex.com@lists.openembedded.org> escreveu (terça,
> > 18/02/2025 à(s) 03:49):
> > > > This message originated from outside your organization
> > > >
> > > > On 2/17/25 11:34 AM, Quentin Schulz wrote:
> > > > > +Cc Marek
> > > >
> > > > Hi,
> > >
> > > Thanks for the quick answer!
> > >
> > > >
> > > > > On 2/16/25 7:36 PM, Rogerio Guerra Borin via
> > > > lists.openembedded.org
> > > > wrote:
> > > > >> You don't often get email from
> > > > >> rogerio.borin=toradex.com@lists.openembedded.org. Learn why
> > > > this is
> > > > >> important<https://aka.ms/LearnAboutSenderIdentification
> > > > <https://aka.ms/LearnAboutSenderIdentification>>
> > > > >> Hello,
> > > > >>
> > > > >> At Toradex we're using class "kernel-fitimage" to produce
> > > > our signed
> > > > >> kernel FIT images and we've found that the recent commit
> > > > d7bd9c627661
> > > > >> ("u-boot: kernel-fitimage: Fix dependency loop if
> > > > UBOOT_SIGN_ENABLE
> > > > >> and UBOOT_ENV enabled") broke our builds. The logs of a
> > > > failed build
> > > > >> show the following message that I found noteworthy:
> > > >
> > > > Can you mail me the entire log file for completeness ?
> > >
> > > Sure.
> > >
> > > I've added it as attachment
> > > (log.do_uboot_assemble_fitimage.841485).
> > >
> > > > >> log.do_uboot_assemble_fitimage.841485:
> > > > >> ...
> > > > >> Couldn't open RSA private key: '/secboot-data/fit-
> > > > keys/dev2.key': No
> > > > >> such file or directory
> > > > >>
> > > > >> The odd thing was that the build was setting:
> > > > >>
> > > > >> UBOOT_SIGN_KEYNAME = "dev"
> > > > >> UBOOT_SIGN_IMG_KEYNAME = "dev2"
> > > > >> FIT_SIGN_INDIVIDUAL = "0"
> > > > >> FIT_GENERATE_KEYS = "0"
> > > >
> > > > FIT_SIGN_INDIVIDUAL = "0" is correct, you do want to sign
> > > > configurations, not individual images in the fitImage, because
> > > > signing
> > > > individual images is susceptible to mix-and-match attack.
> > > >
> > > > >> From the above we see the individual images signing is
> > > > disabled but
> > > > >> the system is trying to access the key file for the
> > > > individual signing
> > > > >> (dev2) regardless of that. Checking the code in uboot-
> > > > sign.bbclass I
> > > > >> found this line:
> > > > >>
> > > > >>
> > > > https://git.openembedded.org/openembedded-core/tree/meta/classes-
> > > > <
> > > > https://git.openembedded.org/openembedded-core/tree/meta/classes-
> > > > >
> > > > >> recipe/uboot-sign.bbclass?h=scarthgap#n116
> > > > >>
> > > > >> that is responsible for that access (inside function
> > > > concat_dtb()).
> > > > >> The code is always referencing variable
> > > > UBOOT_SIGN_IMG_KEYNAME
> > > > >> regardless of FIT_SIGN_INDIVIDUAL being set to "1" or "0"
> > > > even though
> > > > >> there's a condition a couple lines above L116 where
> > > > >> FIT_SIGN_INDIVIDUAL is tested to decide in which mode to run
> > > > the
> > > > >> signing tool.
> > > >
> > > > Uh, good find, does the following patch fix your problem ?
> > > >
> > > > """
> > > > diff --git a/meta/classes-recipe/uboot-sign.bbclass
> > > > b/meta/classes-recipe/uboot-sign.bbclass
> > > > index 96c47ab016..74ef5697ca 100644
> > > > --- a/meta/classes-recipe/uboot-sign.bbclass
> > > > +++ b/meta/classes-recipe/uboot-sign.bbclass
> > > > @@ -107,13 +107,16 @@ concat_dtb() {
> > > > # makes fitImage susceptible to mix-and-match attack.
> > > > if [ "${FIT_SIGN_INDIVIDUAL}" = "1" ] ; then
> > > > UBOOT_MKIMAGE_MODE="auto"
> > > > + UBOOT_MKIMAGE_KEYNAME=${UBOOT_SIGN_IMG_KEYNAME}
> > > > + else
> > > > + UBOOT_MKIMAGE_KEYNAME=${UBOOT_SIGN_KEYNAME}
> > > > fi
> > > > ${UBOOT_MKIMAGE_SIGN} \
> > > > ${@'-D "${UBOOT_MKIMAGE_DTCOPTS}"' if
> > > > len('${UBOOT_MKIMAGE_DTCOPTS}') else ''} \
> > > > -f $UBOOT_MKIMAGE_MODE \
> > > > -k "${UBOOT_SIGN_KEYDIR}" \
> > > > -o "${FIT_HASH_ALG},${FIT_SIGN_ALG}" \
> > > > - -g "${UBOOT_SIGN_IMG_KEYNAME}" \
> > > > + -g "$UBOOT_MKIMAGE_KEYNAME" \
> > > > -K "${UBOOT_DTB_BINARY}" \
> > > > -d /dev/null \
> > > > -r ${B}/unused.itb \
> > > > """
> > >
> > > Looking at the patch I'm pretty sure it would solve my problem,
> > > particularly because we set FIT_SIGN_INDIVIDUAL="0" in our builds.
> > > However, I see it keeps the concept of the signing being either on
> > > the
> > > config or on the individual images, which I wanted to further
> > > comment
> > > down below.
> > >
> > > > >> To better understand the behavior I created dev2.
> > > > >> {key,crt} files and retried a build which worked. However,
> > > > checking
> > > > >> the contents of the final u-boot.dtb file I found that the
> > > > "dev2" key
> > > > >> node was present but the "dev" key wasn't. Then, checking
> > > > the contents
> > > > >> of the final fitImage I noticed the configuration nodes were
> > > > signed by
> > > > >> the dev key while the individual images were not signed (as
> > > > expected).
> > > > >> But that means such a system wouldn't boot since the
> > > > required "dev"
> > > > >> key is not present in the U-Boot DTB, from what I conclude
> > > > the FIT
> > > > >> signing is broken at the moment.
> > > > >>
> > > > >> So, first of all I wanted to let you guys know about this
> > > > issue
> > > > >> (hoping we're not doing anything wrong with our builds).
> > > >
> > > > No, your builds are fine, this is a real bug.
> > >
> > > Thanks for the confirmation.
> > >
> > > > >> As a second point, I wanted to confirm the expected behavior
> > > > regarding
> > > > >> variable FIT_SIGN_INDIVIDUAL. According to the
> > > > documentation:
> > > > >>
> > > > >>
> > > > https://docs.yoctoproject.org/5.0.6/ref-manual/variables.html#term
> > > > -
> > > > <
> > > > https://docs.yoctoproject.org/5.0.6/ref-manual/variables.html#term
> > > > ->
> > > > >> FIT_SIGN_INDIVIDUAL
> > > > >>
> > > > >> when its value is "1" the individual images will be signed
> > > > *in
> > > > >> addition* to the FIT image being signed.
> > > >
> > > > That is a bit weird and no, I don't think that is how it is
> > > > supposed to
> > > > work.
> > > >
> > > > FIT_SIGN_INDIVIDUAL=1 -> only image subnodes in fitImage /images
> > > > nodes
> > > > are signed AND fitImage is susceptible to mix-and-match attack
> > > > (basically attacker can replace individual signed images in
> > > > fitImage
> > > > with different individual signed images, e.g. with older
> > > > versions, to
> > > > achieve a combination of images which allows them to somehow
> > > > break the
> > > > system).
> > > >
> > > > FIT_SIGN_INDIVIDUAL=0 (recommended) -> images are hashed,
> > > > configurations
> > > > are hashed, hashes are combined and signed. Because images and
> > > > configurations (which images are to be used together) are signed,
> > > > attacker cannot mix-and-match images into an un-allowed
> > > > configuration.
> > > >
> > > > >> From that, I understand that
> > > > >> the configurations will be always signed and the individual
> > > > images may
> > > > >> or may not be signed depending on FIT_SIGN_INDIVIDUAL.
> > > >
> > > > The other way around. The configurations are signed by default,
> > > > which is
> > > > good, but this can be disabled if needed (unlikely).
> > > >
> > > > >> I kind of
> > > > >> confirmed this behavior by checking our build artifacts on
> > > > kirkstone:
> > > > >> with FIT_SIGN_INDIVIDUAL="0" the U-Boot DTB has only the
> > > > "dev" key and
> > > > >> the fit image is signed by that same key; with
> > > > FIT_SIGN_INDIVIDUAL="1"
> > > > >> both keys are present in the DTB, the configurations are
> > > > signed by the
> > > > >> "dev" key and the images by the "dev2" key. I understand
> > > > this is the
> > > > >> correct behavior, right?
> > > >
> > > > Images shouldn't be signed by different keys than configurations,
> > > > that
> > > > is a bit odd.
> > > >
> > > > >> I'm asking because looking at the comment at:
> > > > >>
> > > > >>
> > > > https://git.openembedded.org/openembedded-core/tree/meta/classes-
> > > > <
> > > > https://git.openembedded.org/openembedded-core/tree/meta/classes-
> > > > >
> > > > >> recipe/uboot-sign.bbclass?h=scarthgap#n106
> > > > >>
> > > > >> it gives me the impression that FIT_SIGN_INDIVIDUAL would
> > > > select
> > > > >> between either signing individual images
> > > >
> > > > Yes
> > > >
> > > > >> or signing the
> > > > >> configurations
> > > >
> > > > Signed configuration also includes hash of the images that the
> > > > configuration references in it, that's what prevents the mix-and-
> > > > match
> > > > attack.
> > > >
> > > > >> , which doesn't match the documented behavior. I believe
> > > > >> the code in concat_dtb() also seems to assume only one of
> > > > the keys
> > > > >> (dev/dev2) would need be present in the DTB. So the fix
> > > > would involve
> > > > >> a review on this as well I imagine.
> > > > IIRC Adrian was looking into improving those docs.
> > >
> > > Regarding this second topic, let me show you some other information
> > > I
> > > gathered from a couple of builds that may help with your
> > > assessment:
> > >
> > > FSI := FIT_SIGN_INDIVIDUAL
> > > CFG := Whether or not the configurations inside the FIT image were
> > > signed
> > > IMG := Whether or not the subimages inside the FIT image were
> > > signed
> > >
> > > | Branch | Version | FSI | Keys inside DTB | CFG | IMG |
> > > |-----------+---------------+-----+------------------+-----+-----|
> > > | kirkstone | latest | "0" | {dev} | Y | N |
> > > | kirkstone | latest | "1" | {dev,dev2} | Y | Y |
> > > |-----------+---------------+-----+------------------+-----+-----|
> > > | scarthgap | latest | "0" | Build failed (!) | n/a | n/a |
> > > | | + dummy dev2 | | {dev2} | Y | N |
> > > | scarthgap | latest | "1" | {dev2} (!) | Y | Y |
> > > |-----------+---------------+-----+------------------+-----+-----|
> > > | scarthgap | w/o @d7bd9c62 | "0" | {dev} | Y | N |
> > > | scarthgap | w/o @d7bd9c62 | "1" | {dev,dev2} | Y | Y |
> > >
> > > The first two lines show that on kirkstone when
> > > FIT_SIGN_INDIVIDUAL="0"
> > > only the configurations are signed and the DTB contains only the
> > > "dev"
> > > key. When FIT_SIGN_INDIVIDUAL="1" both "dev" and "dev2" are present
> > > in
> > > the DTB and both the configurations and the images are signed. And
> > > that's the exact same behavior I got on scarthgap after reverting
> > > the
> > > commit d7bd9c62. And, as I said, this seems to match the
> > > documentation.
> > > I've attached snippets of the fdtdump for the FIT image and U-Boot
> > > DTB
> > > from my build on kirkstone to illustrate what I said.
> > >
> > > So, if the expected behavior is according to what you described
> > > then the
> > > behavior so far was wrong (for a long time) and the new behavior is
> > > introducing a breaking change for anyone relying on what's
> > > documented
> > > for the FIT_SIGN_INDIVIDUAL="1" case, right? Btw, for us at Toradex
> > > this
> > > is not a big deal because we don't fall into that case but
> > > considering
> > > the general use of the layer the change in behavior could be an
> > > issue.
> > >
> > > Another piece of information that might be useful: I made a change
> > > to
> > > concat_dtb() to make it add both keys (dev/dev2) to the DTB (by
> > > invoking
> > > mkimage twice) and I found the invocation to "uboot-fit_check_sign"
> > > failed afterwards. I confirmed that the DTB had the two keys, but
> > > because the FIT image only had the configurations signed, the tool
> > > threw
> > > an error saying the signature of "require key dev2" could not be
> > > verified but the showed that the check of the "dev" key succeeded.
> > > Unfortunately I don't have the logs available for that case, but I
> > > could
> > > repeat the tests if needed. Anyway, my point is that the
> > > "uboot-fit_check_sign" tool seems capable of checking both keys as
> > > well.
> > >
> > > Interestingly, if I removed the verification step (performed on the
> > > "unused.itb" file), then the image generation succeeded and
> > > everything
> > > seemed to work as before both when FIT_SIGN_INDIVIDUAL is "0" or
> > > "1".
> > > So, it's not hard to get the old behavior, though we'd need to
> > > figure
> > > out how to properly do the verification when mkimage is invoked
> > > with -f
> > > "auto" or -f "auto-conf". I could post my unfinished patch if
> > > helpful.
> > >
> > >
> > >
> >
> >
> > --
> > Best regards,
> >
> > José Quaresma
> >
> > -=-=-=-=-=-=-=-=-=-=-=-
> > Links: You receive all messages sent to this group.
> > View/Reply Online (#211753):
> > https://lists.openembedded.org/g/openembedded-core/message/211753
> > Mute This Topic: https://lists.openembedded.org/mt/111218371/7270266
> > Group Owner: openembedded-core+owner@lists.openembedded.org
> > Unsubscribe:
> > https://lists.openembedded.org/g/openembedded-core/unsub [
> > L.Anderweit@phytec.de]
> > -=-=-=-=-=-=-=-=-=-=-=-
> >
>
>
--
Best regards,
José Quaresma
[-- Attachment #2: Type: text/html, Size: 21369 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [OE-core] Broken FIT image signing since "u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled"
2025-02-18 3:49 ` Rogerio Guerra Borin
2025-02-20 10:46 ` Jose Quaresma
@ 2025-02-20 23:04 ` Marek Vasut
2025-02-21 1:54 ` Sean Anderson
1 sibling, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2025-02-20 23:04 UTC (permalink / raw)
To: Rogerio Guerra Borin, Quentin Schulz, openembedded-core,
Sean Anderson
Cc: Richard Purdie, Freihofer, Adrian
On 2/18/25 4:49 AM, Rogerio Guerra Borin wrote:
[...]
>> >> that is responsible for that access (inside function concat_dtb()).
>> >> The code is always referencing variable UBOOT_SIGN_IMG_KEYNAME
>> >> regardless of FIT_SIGN_INDIVIDUAL being set to "1" or "0" even though
>> >> there's a condition a couple lines above L116 where
>> >> FIT_SIGN_INDIVIDUAL is tested to decide in which mode to run the
>> >> signing tool.
>>
>> Uh, good find, does the following patch fix your problem ?
>>
>> """
>> diff --git a/meta/classes-recipe/uboot-sign.bbclass
>> b/meta/classes-recipe/uboot-sign.bbclass
>> index 96c47ab016..74ef5697ca 100644
>> --- a/meta/classes-recipe/uboot-sign.bbclass
>> +++ b/meta/classes-recipe/uboot-sign.bbclass
>> @@ -107,13 +107,16 @@ concat_dtb() {
>> # makes fitImage susceptible to mix-and-match attack.
>> if [ "${FIT_SIGN_INDIVIDUAL}" = "1" ] ; then
>> UBOOT_MKIMAGE_MODE="auto"
>> + UBOOT_MKIMAGE_KEYNAME=${UBOOT_SIGN_IMG_KEYNAME}
>> + else
>> + UBOOT_MKIMAGE_KEYNAME=${UBOOT_SIGN_KEYNAME}
>> fi
>> ${UBOOT_MKIMAGE_SIGN} \
>> ${@'-D "${UBOOT_MKIMAGE_DTCOPTS}"' if
>> len('${UBOOT_MKIMAGE_DTCOPTS}') else ''} \
>> -f $UBOOT_MKIMAGE_MODE \
>> -k "${UBOOT_SIGN_KEYDIR}" \
>> -o "${FIT_HASH_ALG},${FIT_SIGN_ALG}" \
>> - -g "${UBOOT_SIGN_IMG_KEYNAME}" \
>> + -g "$UBOOT_MKIMAGE_KEYNAME" \
>> -K "${UBOOT_DTB_BINARY}" \
>> -d /dev/null \
>> -r ${B}/unused.itb \
>> """
>
> Looking at the patch I'm pretty sure it would solve my problem,
> particularly because we set FIT_SIGN_INDIVIDUAL="0" in our builds.
> However, I see it keeps the concept of the signing being either on the
> config or on the individual images, which I wanted to further comment
> down below.
That is correct, the signing is supposed to operate either on signed
configurations (where the configuration signature includes the hash of
all the images referenced by the configuration), or separate images
(which is susceptible to mix and match attack).
[...]
>> Signed configuration also includes hash of the images that the
>> configuration references in it, that's what prevents the mix-and-match
>> attack.
>>
>> >> , which doesn't match the documented behavior. I believe
>> >> the code in concat_dtb() also seems to assume only one of the keys
>> >> (dev/dev2) would need be present in the DTB. So the fix would involve
>> >> a review on this as well I imagine.
>> IIRC Adrian was looking into improving those docs.
>
> Regarding this second topic, let me show you some other information I
> gathered from a couple of builds that may help with your assessment:
>
> FSI := FIT_SIGN_INDIVIDUAL
> CFG := Whether or not the configurations inside the FIT image were signed
> IMG := Whether or not the subimages inside the FIT image were signed
>
> | Branch | Version | FSI | Keys inside DTB | CFG | IMG |
> |-----------+---------------+-----+------------------+-----+-----|
> | kirkstone | latest | "0" | {dev} | Y | N |
> | kirkstone | latest | "1" | {dev,dev2} | Y | Y |
> |-----------+---------------+-----+------------------+-----+-----|
> | scarthgap | latest | "0" | Build failed (!) | n/a | n/a |
> | | + dummy dev2 | | {dev2} | Y | N |
> | scarthgap | latest | "1" | {dev2} (!) | Y | Y |
> |-----------+---------------+-----+------------------+-----+-----|
> | scarthgap | w/o @d7bd9c62 | "0" | {dev} | Y | N |
> | scarthgap | w/o @d7bd9c62 | "1" | {dev,dev2} | Y | Y |
>
> The first two lines show that on kirkstone when FIT_SIGN_INDIVIDUAL="0"
> only the configurations are signed and the DTB contains only the "dev"
> key. When FIT_SIGN_INDIVIDUAL="1" both "dev" and "dev2" are present in
> the DTB and both the configurations and the images are signed.
This last part is a bit odd. The behavior you describe would imply that
FIT_SIGN_INDIVIDUAL="1" is some sort of setup which signs both
configurations (which include signature of referenced images) and images
themselves, which would be redundant.
> And
> that's the exact same behavior I got on scarthgap after reverting the
> commit d7bd9c62. And, as I said, this seems to match the documentation.
> I've attached snippets of the fdtdump for the FIT image and U-Boot DTB
> from my build on kirkstone to illustrate what I said.
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.
> So, if the expected behavior is according to what you described then the
> behavior so far was wrong (for a long time) and the new behavior is
> introducing a breaking change for anyone relying on what's documented
> for the FIT_SIGN_INDIVIDUAL="1" case, right?
I believe FIT_SIGN_INDIVIDUAL="1" did not work correctly, yes. I also
think FIT_SIGN_INDIVIDUAL="1" should not be used, because of the mix and
match attack, please see U-Boot documentation above.
+CC Sean to get second opinion here.
> Btw, for us at Toradex this
> is not a big deal because we don't fall into that case but considering
> the general use of the layer the change in behavior could be an issue.
>
> Another piece of information that might be useful: I made a change to
> concat_dtb() to make it add both keys (dev/dev2) to the DTB (by invoking
> mkimage twice) and I found the invocation to "uboot-fit_check_sign"
> failed afterwards. I confirmed that the DTB had the two keys, but
> because the FIT image only had the configurations signed, the tool threw
> an error saying the signature of "require key dev2" could not be
> verified but the showed that the check of the "dev" key succeeded.
> Unfortunately I don't have the logs available for that case, but I could
> repeat the tests if needed. Anyway, my point is that the "uboot-
> fit_check_sign" tool seems capable of checking both keys as well.
>
> Interestingly, if I removed the verification step (performed on the
> "unused.itb" file), then the image generation succeeded and everything
> seemed to work as before both when FIT_SIGN_INDIVIDUAL is "0" or "1".
> So, it's not hard to get the old behavior, though we'd need to figure
> out how to properly do the verification when mkimage is invoked with -f
> "auto" or -f "auto-conf". I could post my unfinished patch if helpful.
I am not convinced adding the old behavior is the right thing to do,
please see above.
But I am also unsure what to do about people depending on the old
behavior, which I think is odd. Thoughts ?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [OE-core] Broken FIT image signing since "u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled"
2025-02-20 14:42 ` Leonard Anderweit
2025-02-20 16:26 ` Jose Quaresma
@ 2025-02-20 23:05 ` Marek Vasut
1 sibling, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2025-02-20 23:05 UTC (permalink / raw)
To: Leonard Anderweit, rogerio.borin@toradex.com,
quaresma.jose@gmail.com
Cc: quentin.schulz@cherry.de,
openembedded-core@lists.openembedded.org,
adrian.freihofer@siemens.com, richard.purdie@linuxfoundation.org
On 2/20/25 3:42 PM, Leonard Anderweit wrote:
> Hi,
Hello Leonard,
> I've also noticed this bug and sent patch [1] to fix it.
>
> [1] https://lists.openembedded.org/g/openembedded-core/message/211761
Please CC me on the patch so I can review it ?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [OE-core] Broken FIT image signing since "u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled"
2025-02-20 23:04 ` Marek Vasut
@ 2025-02-21 1:54 ` Sean Anderson
2025-02-21 14:15 ` Rogerio Guerra Borin
0 siblings, 1 reply; 15+ messages in thread
From: Sean Anderson @ 2025-02-21 1:54 UTC (permalink / raw)
To: Marek Vasut, Rogerio Guerra Borin, Quentin Schulz,
openembedded-core
Cc: Richard Purdie, Freihofer, Adrian
On 2/20/25 18:04, Marek Vasut wrote:
> On 2/18/25 4:49 AM, Rogerio Guerra Borin wrote:
>
> [...]
>
>>> >> that is responsible for that access (inside function concat_dtb()).
>>> >> The code is always referencing variable UBOOT_SIGN_IMG_KEYNAME
>>> >> regardless of FIT_SIGN_INDIVIDUAL being set to "1" or "0" even though
>>> >> there's a condition a couple lines above L116 where
>>> >> FIT_SIGN_INDIVIDUAL is tested to decide in which mode to run the
>>> >> signing tool.
>>>
>>> Uh, good find, does the following patch fix your problem ?
>>>
>>> """
>>> diff --git a/meta/classes-recipe/uboot-sign.bbclass
>>> b/meta/classes-recipe/uboot-sign.bbclass
>>> index 96c47ab016..74ef5697ca 100644
>>> --- a/meta/classes-recipe/uboot-sign.bbclass
>>> +++ b/meta/classes-recipe/uboot-sign.bbclass
>>> @@ -107,13 +107,16 @@ concat_dtb() {
>>> # makes fitImage susceptible to mix-and-match attack.
>>> if [ "${FIT_SIGN_INDIVIDUAL}" = "1" ] ; then
>>> UBOOT_MKIMAGE_MODE="auto"
>>> + UBOOT_MKIMAGE_KEYNAME=${UBOOT_SIGN_IMG_KEYNAME}
>>> + else
>>> + UBOOT_MKIMAGE_KEYNAME=${UBOOT_SIGN_KEYNAME}
>>> fi
>>> ${UBOOT_MKIMAGE_SIGN} \
>>> ${@'-D "${UBOOT_MKIMAGE_DTCOPTS}"' if
>>> len('${UBOOT_MKIMAGE_DTCOPTS}') else ''} \
>>> -f $UBOOT_MKIMAGE_MODE \
>>> -k "${UBOOT_SIGN_KEYDIR}" \
>>> -o "${FIT_HASH_ALG},${FIT_SIGN_ALG}" \
>>> - -g "${UBOOT_SIGN_IMG_KEYNAME}" \
>>> + -g "$UBOOT_MKIMAGE_KEYNAME" \
>>> -K "${UBOOT_DTB_BINARY}" \
>>> -d /dev/null \
>>> -r ${B}/unused.itb \
>>> """
>>
>> Looking at the patch I'm pretty sure it would solve my problem, particularly because we set FIT_SIGN_INDIVIDUAL="0" in our builds. However, I see it keeps the concept of the signing being either on the config or on the individual images, which I wanted to further comment down below.
>
> That is correct, the signing is supposed to operate either on signed configurations (where the configuration signature includes the hash of all the images referenced by the configuration), or separate images (which is susceptible to mix and match attack).
>
> [...]
>
>>> Signed configuration also includes hash of the images that the
>>> configuration references in it, that's what prevents the mix-and-match
>>> attack.
>>>
>>> >> , which doesn't match the documented behavior. I believe
>>> >> the code in concat_dtb() also seems to assume only one of the keys
>>> >> (dev/dev2) would need be present in the DTB. So the fix would involve
>>> >> a review on this as well I imagine.
>>> IIRC Adrian was looking into improving those docs.
>>
>> Regarding this second topic, let me show you some other information I gathered from a couple of builds that may help with your assessment:
>>
>> FSI := FIT_SIGN_INDIVIDUAL
>> CFG := Whether or not the configurations inside the FIT image were signed
>> IMG := Whether or not the subimages inside the FIT image were signed
>>
>> | Branch | Version | FSI | Keys inside DTB | CFG | IMG |
>> |-----------+---------------+-----+------------------+-----+-----|
>> | kirkstone | latest | "0" | {dev} | Y | N |
>> | kirkstone | latest | "1" | {dev,dev2} | Y | Y |
>> |-----------+---------------+-----+------------------+-----+-----|
>> | scarthgap | latest | "0" | Build failed (!) | n/a | n/a |
>> | | + dummy dev2 | | {dev2} | Y | N |
>> | scarthgap | latest | "1" | {dev2} (!) | Y | Y |
>> |-----------+---------------+-----+------------------+-----+-----|
>> | scarthgap | w/o @d7bd9c62 | "0" | {dev} | Y | N |
>> | scarthgap | w/o @d7bd9c62 | "1" | {dev,dev2} | Y | Y |
>>
>> The first two lines show that on kirkstone when FIT_SIGN_INDIVIDUAL="0" only the configurations are signed and the DTB contains only the "dev" key. When FIT_SIGN_INDIVIDUAL="1" both "dev" and "dev2" are present in the DTB and both the configurations and the images are signed.
>
> This last part is a bit odd. The behavior you describe would imply that FIT_SIGN_INDIVIDUAL="1" is some sort of setup which signs both configurations (which include signature of referenced images) and images themselves, which would be redundant.
>
>> And that's the exact same behavior I got on scarthgap after reverting the commit d7bd9c62. And, as I said, this seems to match the documentation.
>> I've attached snippets of the fdtdump for the FIT image and U-Boot DTB from my build on kirkstone to illustrate what I said.
>
> 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.
>
>> So, if the expected behavior is according to what you described then the behavior so far was wrong (for a long time) and the new behavior is introducing a breaking change for anyone relying on what's documented for the FIT_SIGN_INDIVIDUAL="1" case, right?
>
> I believe FIT_SIGN_INDIVIDUAL="1" did not work correctly, yes. I also think FIT_SIGN_INDIVIDUAL="1" should not be used, because of the mix and match attack, please see U-Boot documentation above.
>
> +CC Sean to get second opinion here.
(disclaimer: I have not been following the discussion closely)
IMO image signing should not be used, and really should come with a big fat warning
or something. Ideally it should be removed in a future release (if possible; not sure what
the OE perspective is on compatibility in this area). It remains in U-Boot only for
compatibility; it probably should get some warnings there too so people don't accidentally
use it for new boards.
--Sean
>> Btw, for us at Toradex this is not a big deal because we don't fall into that case but considering the general use of the layer the change in behavior could be an issue.
>>
>> Another piece of information that might be useful: I made a change to concat_dtb() to make it add both keys (dev/dev2) to the DTB (by invoking mkimage twice) and I found the invocation to "uboot-fit_check_sign" failed afterwards. I confirmed that the DTB had the two keys, but because the FIT image only had the configurations signed, the tool threw an error saying the signature of "require key dev2" could not be verified but the showed that the check of the "dev" key succeeded. Unfortunately I don't have the logs available for that case, but I could repeat the tests if needed. Anyway, my point is that the "uboot- fit_check_sign" tool seems capable of checking both keys as well.
>>
>> Interestingly, if I removed the verification step (performed on the "unused.itb" file), then the image generation succeeded and everything seemed to work as before both when FIT_SIGN_INDIVIDUAL is "0" or "1".
>> So, it's not hard to get the old behavior, though we'd need to figure out how to properly do the verification when mkimage is invoked with -f "auto" or -f "auto-conf". I could post my unfinished patch if helpful.
>
> I am not convinced adding the old behavior is the right thing to do, please see above.
>
> But I am also unsure what to do about people depending on the old behavior, which I think is odd. Thoughts ?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [OE-core] Broken FIT image signing since "u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled"
2025-02-21 1:54 ` Sean Anderson
@ 2025-02-21 14:15 ` Rogerio Guerra Borin
2025-02-21 14:45 ` Adrian Freihofer
2025-02-21 23:54 ` Marek Vasut
0 siblings, 2 replies; 15+ messages in thread
From: Rogerio Guerra Borin @ 2025-02-21 14:15 UTC (permalink / raw)
To: Sean Anderson, Marek Vasut, Quentin Schulz, openembedded-core
Cc: Richard Purdie, Freihofer, Adrian
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...
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [OE-core] Broken FIT image signing since "u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled"
2025-02-21 14:15 ` Rogerio Guerra Borin
@ 2025-02-21 14:45 ` Adrian Freihofer
2025-02-22 0:22 ` Marek Vasut
2025-02-21 23:54 ` Marek Vasut
1 sibling, 1 reply; 15+ messages in thread
From: Adrian Freihofer @ 2025-02-21 14:45 UTC (permalink / raw)
To: rogerio.borin, Sean Anderson, Marek Vasut, Quentin Schulz,
openembedded-core
Cc: Richard Purdie, Freihofer, Adrian
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 (#211799):
> https://lists.openembedded.org/g/openembedded-core/message/211799
> Mute This Topic: https://lists.openembedded.org/mt/111218371/4454582
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe:
> https://lists.openembedded.org/g/openembedded-core/unsub [
> adrian.freihofer@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [OE-core] Broken FIT image signing since "u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled"
2025-02-21 14:15 ` Rogerio Guerra Borin
2025-02-21 14:45 ` Adrian Freihofer
@ 2025-02-21 23:54 ` Marek Vasut
1 sibling, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2025-02-21 23:54 UTC (permalink / raw)
To: Rogerio Guerra Borin, Sean Anderson, Quentin Schulz,
openembedded-core
Cc: Richard Purdie, Freihofer, Adrian
On 2/21/25 3:15 PM, Rogerio Guerra Borin wrote:
> Hey Marek and Sean,
Hi,
> 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).
This is also redundant. Signing configurations also signs the images
referenced in those configurations.
Signing both images and configurations the way FIT_SIGN_INDIVIDUAL=1
only adds to complexity and confusion without really adding anything in
terms of security, I think this should be eventually removed.
> 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...
I am increasingly convinced that FIT_SIGN_INDIVIDUAL=1 is poorly
designed, but sadly here to stay.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [OE-core] Broken FIT image signing since "u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled"
2025-02-21 14:45 ` Adrian Freihofer
@ 2025-02-22 0:22 ` Marek Vasut
0 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2025-02-22 0:22 UTC (permalink / raw)
To: Adrian Freihofer, rogerio.borin, Sean Anderson, Quentin Schulz,
openembedded-core
Cc: Richard Purdie, Freihofer, Adrian
On 2/21/25 3:45 PM, Adrian Freihofer wrote:
[...]
>> 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.
That would break the use of signed fitImages with embedded env script,
which does not work either.
I've posted the following patch instead, you should all be on CC:
[PATCH] u-boot: kernel-fitimage: Restore FIT_SIGN_INDIVIDUAL="1" behavior
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2025-02-22 0:23 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-16 18:36 Broken FIT image signing since "u-boot: kernel-fitimage: Fix dependency loop if UBOOT_SIGN_ENABLE and UBOOT_ENV enabled" Rogerio Guerra Borin
2025-02-16 20:09 ` Rogerio Guerra Borin
2025-02-17 10:34 ` [OE-core] " Quentin Schulz
2025-02-17 22:37 ` Marek Vasut
2025-02-18 3:49 ` Rogerio Guerra Borin
2025-02-20 10:46 ` Jose Quaresma
2025-02-20 14:42 ` Leonard Anderweit
2025-02-20 16:26 ` Jose Quaresma
2025-02-20 23:05 ` Marek Vasut
2025-02-20 23:04 ` Marek Vasut
2025-02-21 1:54 ` Sean Anderson
2025-02-21 14:15 ` Rogerio Guerra Borin
2025-02-21 14:45 ` Adrian Freihofer
2025-02-22 0:22 ` Marek Vasut
2025-02-21 23:54 ` Marek Vasut
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox