Marek Vasut <[email protected]> escreveu (domingo, 23/02/2025 à(s) 23:23):
> On 2/23/25 11:13 PM, Rogerio Guerra Borin wrote:
> > On 2/21/25 21:20, Marek Vasut wrote:
> >> This message originated from outside your organization
> >>
> >> OE FIT_SIGN_INDIVIDUAL is implemented in an unusual manner,
> >> where the resulting signed fitImage contains both signed
> >> images and signed configurations, possibly using different
> >> keys. This kind of signing of images is redundant, but so is
> >> the behavior of FIT_SIGN_INDIVIDUAL="1" and that is here to
> >> stay.
> >>
> >> Adjust the process of public key insertion into u-boot.dtb
> >> such that if FIT_SIGN_INDIVIDUAL==1, the image signing key
> >> is inserted into u-boot.dtb first, and in any case the
> >> configuration signing key is inserted into u-boot.dtb last.
> >>
> >> The verification of the keys inserted into u-boot.dtb against
> >> unused.itb is performed only for FIT_SIGN_INDIVIDUAL!=1 due to
> >> mkimage limitation, which does not allow mkimage -f auto-conf
> >> to update the generated unused.itb, and instead rewrites it.
> >>
> >> Fixes: 259bfa86f384 ("u-boot: kernel-fitimage: Fix dependency loop if
> >> UBOOT_SIGN_ENABLE and UBOOT_ENV enabled")
> >> Signed-off-by: Marek Vasut <[email protected]>
> >> ---
> >> Cc: Adrian Freihofer <[email protected]>
> >> Cc: Jose Quaresma <[email protected]>
> >> Cc: Leonard Anderweit <[email protected]>
> >> Cc: Quentin Schulz <[email protected]>
> >> Cc: Richard Purdie <[email protected]>
> >> Cc: Rogerio Guerra Borin <[email protected]>
> >> Cc: Sean Anderson <[email protected]>
> >> ---
> >> meta/classes-recipe/uboot-sign.bbclass | 60 ++++++++++++++++++++++----
> >> 1 file changed, 51 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/meta/classes-recipe/uboot-sign.bbclass b/meta/classes-
> >> recipe/uboot-sign.bbclass
> >> index 96c47ab0165..5c579a9fb0e 100644
> >> --- a/meta/classes-recipe/uboot-sign.bbclass
> >> +++ b/meta/classes-recipe/uboot-sign.bbclass
> >> @@ -101,27 +101,69 @@ concat_dtb() {
> >> binary="$2"
> >> if [ -e "${UBOOT_DTB_BINARY}" ]; then
> >> - # Re-sign the kernel in order to add the keys to our dtb
> >> - UBOOT_MKIMAGE_MODE="auto-conf"
> >> # Signing individual images is not recommended as that
> >> # makes fitImage susceptible to mix-and-match attack.
> >> + #
> >> + # OE FIT_SIGN_INDIVIDUAL is implemented in an unusual manner,
> >> + # where the resulting signed fitImage contains both signed
> >> + # images and signed configurations. This is redundant. In
> >> + # order to prevent mix-and-match attack, it is sufficient
> >> + # to sign configurations. The FIT_SIGN_INDIVIDUAL = "1"
> >> + # support is kept to avoid breakage of existing layers, but
> >> + # it is highly recommended to avoid FIT_SIGN_INDIVIDUAL = "1",
> >> + # i.e. set FIT_SIGN_INDIVIDUAL = "0" .
> >> if [ "${FIT_SIGN_INDIVIDUAL}" = "1" ] ; then
> >> - UBOOT_MKIMAGE_MODE="auto"
> >> + # Sign dummy image images in order to
> >> + # add the image signing keys to our dtb
> >> + ${UBOOT_MKIMAGE_SIGN} \
> >> + ${@'-D "${UBOOT_MKIMAGE_DTCOPTS}"' if
> >> len('${UBOOT_MKIMAGE_DTCOPTS}') else ''} \
> >> + -f auto \
> >> + -k "${UBOOT_SIGN_KEYDIR}" \
> >> + -o "${FIT_HASH_ALG},${FIT_SIGN_ALG}" \
> >> + -g "${UBOOT_SIGN_IMG_KEYNAME}" \
> >> + -K "${UBOOT_DTB_BINARY}" \
> >> + -d /dev/null \
> >> + -r ${B}/unused.itb \
> >> + ${UBOOT_MKIMAGE_SIGN_ARGS}
> >> fi
> >> +
> >> + # Sign dummy image configurations in order to
> >> + # add the configuration signing keys to our dtb
> >> ${UBOOT_MKIMAGE_SIGN} \
> >> ${@'-D "${UBOOT_MKIMAGE_DTCOPTS}"' if
> >> len('${UBOOT_MKIMAGE_DTCOPTS}') else ''} \
> >> - -f $UBOOT_MKIMAGE_MODE \
> >> + -f auto-conf \
> >> -k "${UBOOT_SIGN_KEYDIR}" \
> >> -o "${FIT_HASH_ALG},${FIT_SIGN_ALG}" \
> >> - -g "${UBOOT_SIGN_IMG_KEYNAME}" \
> >> + -g "${UBOOT_SIGN_KEYNAME}" \
> >> -K "${UBOOT_DTB_BINARY}" \
> >> -d /dev/null \
> >> -r ${B}/unused.itb \
> >> ${UBOOT_MKIMAGE_SIGN_ARGS}
> >> - # Verify the kernel image and u-boot dtb
> >> - ${UBOOT_FIT_CHECK_SIGN} \
> >> - -k "${UBOOT_DTB_BINARY}" \
> >> - -f ${B}/unused.itb
> >> +
> >> + # Verify the dummy fitImage signature against u-boot.dtb
> >> + # augmented using public key material.
> >> + #
> >> + # This only works for FIT_SIGN_INDIVIDUAL = "0", because
> >> + # mkimage -f auto-conf does not support -F to extend the
> >> + # existing unused.itb , and instead rewrites unused.itb
> >> + # from scratch.
> >> + #
> >> + # Using two separate unused.itb for mkimage -f auto and
> >> + # mkimage -f auto-conf invocation above would not help, as
> >> + # the signature verification process below checks whether
> >> + # all keys inserted into u-boot.dtb /signature node pass
> >> + # the verification. Separate unused.itb would each miss one
> >> + # of the signatures.
> >> + #
> >> + # The FIT_SIGN_INDIVIDUAL = "1" support is kept to avoid
> >> + # breakage of existing layers, but it is highly recommended
> >> + # to not use FIT_SIGN_INDIVIDUAL = "1", i.e. set
> >> + # FIT_SIGN_INDIVIDUAL = "0" .
> >> + if [ "${FIT_SIGN_INDIVIDUAL}" != "1" ] ; then
> >> + ${UBOOT_FIT_CHECK_SIGN} \
> >> + -k "${UBOOT_DTB_BINARY}" \
> >> + -f ${B}/unused.itb
> >> + fi
> >> cp ${UBOOT_DTB_BINARY} ${UBOOT_DTB_SIGNED}
> >> fi
> >
> > Hey Marek,
>
> Hi,
>
> > I've built images with both FIT_SIGN_INDIVIDUAL set to "0" and "1' and
> > checked the resulting artifacts for the presence of keys and signatures.
> > Everything looks fine, as far as I can tell.
> Thank you for testing.
>
> Could you also provide Tested-by tag ?
>
Tested-by: Jose Quaresma <[email protected]>
Hi Marek,
I tested it on a couple of machines on my side and it fixes the regression.
My tests were only on the scarthgap branch.
Thanks for working on the solution.
Jose
--
Best regards,
José Quaresma
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#211872):
https://lists.openembedded.org/g/openembedded-core/message/211872
Mute This Topic: https://lists.openembedded.org/mt/111319286/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-