Re: [OE-core] [PATCH v3] kernel-fitimage: make signing failure fatal

[Mikko Rapeli <mikko.rapeli@linaro.org>]

Hi,

On Tue, Sep 03, 2024 at 07:24:31AM +0000, Sverdlin, Alexander wrote:
> Hi Mikko!
> 
> On Tue, 2024-09-03 at 09:03 +0300, Mikko Rapeli wrote:
> > > mkimage doesn't fail if it is not able to sign FIT nodes.
> > > This may lead to unbootable images in secure boot configurations.
> > > Make signing failures fatal by parsing the mkimage output.
> > > 
> > > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> > > ---
> > > Changes in v3:
> > > - bbfatag_log -> bberror + bbfatal_log with relevant mkimage output snippets
> > > Changes in v2:
> > > - bbfatal -> bbfatal_log
> > > 
> > > � meta/classes-recipe/kernel-fitimage.bbclass | 9 +++++++--
> > > � 1 file changed, 7 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/meta/classes-recipe/kernel-fitimage.bbclass b/meta/classes-recipe/kernel-fitimage.bbclass
> > > index 67c98adb232..ccf848e643f 100644
> > > --- a/meta/classes-recipe/kernel-fitimage.bbclass
> > > +++ b/meta/classes-recipe/kernel-fitimage.bbclass
> > > @@ -753,11 +753,16 @@ fitimage_assemble() {
> > > ��	# Step 8: Sign the image
> > > ��	#
> > > ��	if [ "x${UBOOT_SIGN_ENABLE}" = "x1" ] ; then
> > > -		${UBOOT_MKIMAGE_SIGN} \
> > > +		output=$(${UBOOT_MKIMAGE_SIGN} \
> > 
> > Will this subshell return errors as before or is "set -e" propagated there?
> 
> Good point, I need to test if I'm not masking real errors here...
> 
> > > ��			${@'-D "${UBOOT_MKIMAGE_DTCOPTS}"' if len('${UBOOT_MKIMAGE_DTCOPTS}') else ''} \
> > > ��			-F -k "${UBOOT_SIGN_KEYDIR}" \
> > > ��			-r ${KERNEL_OUTPUT_DIR}/$2 \
> > > -			${UBOOT_MKIMAGE_SIGN_ARGS}
> > > +			${UBOOT_MKIMAGE_SIGN_ARGS})
> > > +		echo "$output"
> > > +		if err=$(echo "$output" | grep -C9 -E "Sign value:\s*unavailable"); then
> > > +			bberror "${UBOOT_MKIMAGE_SIGN} failed to provide signatures for these images:"
> > > +			bbfatal_log "\n$err"
> > 
> > Is the problem really in mkimage since it does not return errors when signing fails?
> 
> I'd say yes, but it's explicitly implemented as best effort approach:
> https://github.com/u-boot/u-boot/commit/56518e71041f#diff-b0d9a26d538f8dc3aff2e1b518534e9e2026713b1f4204e2680d8a84244e3408R234
> 
> But how many years would it take to propagate new mkimage in practice?..

I think this should be challenged with u-boot upstream. If the tool is explicitly
used to sign images then failure to do so should be captured and error value
returned. Patching this in should be straight forward. Working
around the issue by grep'ing logs is not good in the long run.
What other failure modes may exist?

Cheers,

-Mikko