* [PATCH] fit: display proper node on error @ 2021-10-06 15:47 Angelo Dureghello 2021-10-06 16:00 ` Alex G. 0 siblings, 1 reply; 5+ messages in thread From: Angelo Dureghello @ 2021-10-06 15:47 UTC (permalink / raw) To: trini; +Cc: u-boot, Angelo Dureghello Fix final error message from Verification failed for '<NULL>' hash node in 'conf@1' config node to Verification failed for 'signature@1' hash node in 'conf@1' config node Signed-off-by: Angelo Dureghello <angelo.dureghello@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; } } -- 2.32.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] fit: display proper node on error 2021-10-06 15:47 [PATCH] fit: display proper node on error Angelo Dureghello @ 2021-10-06 16:00 ` Alex G. 2021-10-24 19:53 ` Simon Glass 0 siblings, 1 reply; 5+ messages in thread From: Alex G. @ 2021-10-06 16:00 UTC (permalink / raw) To: Angelo Dureghello, trini, Simon Glass; +Cc: u-boot + Simon On 10/6/21 10:47 AM, Angelo Dureghello wrote: > Fix final error message from > > Verification failed for '<NULL>' hash node in 'conf@1' config node > > to > > Verification failed for 'signature@1' hash node in 'conf@1' config node > > Signed-off-by: Angelo Dureghello <angelo.dureghello@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? Alex ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fit: display proper node on error 2021-10-06 16:00 ` Alex G. @ 2021-10-24 19:53 ` Simon Glass 2021-10-25 21:09 ` Angelo Dureghello 0 siblings, 1 reply; 5+ messages in thread From: Simon Glass @ 2021-10-24 19:53 UTC (permalink / raw) To: Alex G.; +Cc: Angelo Dureghello, Tom Rini, U-Boot Mailing List Hi Alex, On Wed, 6 Oct 2021 at 10:00, Alex G. <mr.nuke.me@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@1' config node > > > > to > > > > Verification failed for 'signature@1' hash node in 'conf@1' config node > > > > Signed-off-by: Angelo Dureghello <angelo.dureghello@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. Regards, Simon ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fit: display proper node on error 2021-10-24 19:53 ` Simon Glass @ 2021-10-25 21:09 ` Angelo Dureghello 2022-03-12 2:24 ` Simon Glass 0 siblings, 1 reply; 5+ messages in thread From: Angelo Dureghello @ 2021-10-25 21:09 UTC (permalink / raw) To: Simon Glass; +Cc: Alex G., Tom Rini, U-Boot Mailing List Hi, On Sun, Oct 24, 2021 at 9:53 PM Simon Glass <sjg@chromium.org> wrote: > Hi Alex, > > On Wed, 6 Oct 2021 at 10:00, Alex G. <mr.nuke.me@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@1' config node > > > > > > to > > > > > > Verification failed for 'signature@1' hash node in 'conf@1' config > node > > > > > > Signed-off-by: Angelo Dureghello <angelo.dureghello@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@1' hash node in 'conf@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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fit: display proper node on error 2021-10-25 21:09 ` Angelo Dureghello @ 2022-03-12 2:24 ` Simon Glass 0 siblings, 0 replies; 5+ messages in thread From: Simon Glass @ 2022-03-12 2:24 UTC (permalink / raw) To: Angelo Dureghello; +Cc: Alex G., Tom Rini, U-Boot Mailing List Hi Angelo, On Mon, 25 Oct 2021 at 15:11, Angelo Dureghello <angelo.dureghello@timesys.com> wrote: > > Hi, > > On Sun, Oct 24, 2021 at 9:53 PM Simon Glass <sjg@chromium.org> wrote: >> >> Hi Alex, >> >> On Wed, 6 Oct 2021 at 10:00, Alex G. <mr.nuke.me@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@1' config node >> > > >> > > to >> > > >> > > Verification failed for 'signature@1' hash node in 'conf@1' config node >> > > >> > > Signed-off-by: Angelo Dureghello <angelo.dureghello@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@1' hash node in 'conf@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 ? Yes it should check all signatures until it runs out or finds a match. Regards, Simon ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-03-12 2:26 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-06 15:47 [PATCH] fit: display proper node on error Angelo Dureghello 2021-10-06 16:00 ` Alex G. 2021-10-24 19:53 ` Simon Glass 2021-10-25 21:09 ` Angelo Dureghello 2022-03-12 2:24 ` Simon Glass
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.