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
[email protected]. 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.
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#211556):
https://lists.openembedded.org/g/openembedded-core/message/211556
Mute This Topic: https://lists.openembedded.org/mt/111218371/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-