Marek Vasut <ma...@denx.de> 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 <ma...@denx.de>
> >> ---
> >> Cc: Adrian Freihofer <adrian.freiho...@siemens.com>
> >> Cc: Jose Quaresma <quaresma.j...@gmail.com>
> >> Cc: Leonard Anderweit <l.anderw...@phytec.de>
> >> Cc: Quentin Schulz <quentin.sch...@cherry.de>
> >> Cc: Richard Purdie <richard.pur...@linuxfoundation.org>
> >> Cc: Rogerio Guerra Borin <rogerio.bo...@toradex.com>
> >> Cc: Sean Anderson <sean...@gmail.com>
> >> ---
> >>   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 <jose.quare...@foundries.io>

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: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to