[PATCH] fit: display proper node on error

[Angelo Dureghello]

Hi,

On Sun, Oct 24, 2021 at 9:53 PM Simon Glass <sjg at chromium.org> wrote:

> Hi Alex,
>
> On Wed, 6 Oct 2021 at 10:00, Alex G. <mr.nuke.me at gmail.com> wrote:
> >
> > + Simon
> >
> > On 10/6/21 10:47 AM, Angelo Dureghello wrote:
> > > Fix final error message from
> > >
> > > Verification failed for '<NULL>' hash node in 'conf at 1' config node
> > >
> > > to
> > >
> > > Verification failed for 'signature at 1' hash node in 'conf at 1' config
> node
> > >
> > > Signed-off-by: Angelo Dureghello <angelo.dureghello at timesys.com>
> > > ---
> > >   common/image-fit-sig.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/common/image-fit-sig.c b/common/image-fit-sig.c
> > > index b979cd2a4b..4f2a6ef214 100644
> > > --- a/common/image-fit-sig.c
> > > +++ b/common/image-fit-sig.c
> > > @@ -166,8 +166,8 @@ static int fit_image_verify_sig(const void *fit,
> int image_noffset,
> > >                       } else {
> > >                               puts("+ ");
> > >                               verified = 1;
> > > -                             break;
> > >                       }
> > > +                     break;
> >
> > This would stop checking after the first signature- node. It seems
> > counter-intuitive, as I would expect all signatures to be checked.
> >

> In my mind, the 'break;' clause should only happen when
> > fit_image_check_sig() returns an error. I have no idea why it happened
> > on success. Simon, any thoughts?
>
> If you have a 'required' signature you can use the signed-configs
> approach. Checking the signature of individual images is not actually
> all that useful.
>
> So I think the break is in the right place. It checks all signatures
> and reports them, but only cares whether at least one was verified.
>
> For the error message to be correct, we need to save the noffset of
> the failed node in a separate variable, I think, so we can report the
> last error we got.
>
>
Oh, looks like i sent a wrong patch also, since the
error was related on signature check in a config node:

Verification failed for 'signature at 1' hash node in 'conf at 1' config node

and i patched the image check, this since i couldn't retest on
the board. But the check mechanism seems the same.

Anyway, fit_image_check_sig() was properly returning an error, and
issue was related to imx caam driver used for rsa calc. With sw calc
i have signature verification on conf node passed:

Verifying Hash Integrity ... sha1,rsa2048:dev+ OK

So my understanding is that after an error we want to check for
further "signature" subnodes inside the same "conf" or image node,
but i have never seen more than one signature, is it something
supported/allowed ?



> Regards,
> Simon
>

Regards,
--
Angelo Dureghello