Re: [U-Boot] RSA verify code and required keys

2019-09-14 Thread Daniele Alessandrelli
Hi Simon,

On Fri, Sep 13, 2019 at 5:36 PM Simon Glass  wrote:
>
> Hi Daniele,
>
> >
> > If I understand it correctly, at Line 440 we check if verification
> > with the required key succeeded and if so we return otherwise we
> > continue, trying other keys.
>
> Yes that's my understanding too.
>
> >
> > Is that the intended behavior? Shouldn't the code return in any case
> > (thus making the FIT verification process fail if the image couldn't
> > be verified with the required key)? Or am I missing something?
>
> Yes I think you are right. The documentation says:
>
> - required: If present this indicates that the key must be verified for the
> image / configuration to be considered valid. Only required keys are
> normally verified by the FIT image booting algorithm. Valid values are
> "image" to force verification of all images, and "conf" to force verification
> of the selected configuration (which then relies on hashes in the images to
> verify those).

Thanks for confirming it. I'll prepare a small patch to fix it... that
would be my first U-boot patch :D

>
> The test coverage does not handle that case at present, but it should.
>

I'm afraid I won't have time to fix that in my patch. I had a quick
look at 'test_fit.py' and it looks like there is quite some code to
write to add this test case.

Regards,
Daniele
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] RSA verify code and required keys

2019-09-13 Thread Daniele Alessandrelli
Hi,

I was looking at the RSA image authentication code and I'm a bit
puzzled by the following line of codes in lib/rsa/rsa-verify.c
(https://gitlab.denx.de/u-boot/u-boot/blob/master/lib/rsa/rsa-verify.c#L440):

436 /* See if we must use a particular key */
437 if (info->required_keynode != -1) {
438 ret = rsa_verify_with_keynode(info, hash, sig, sig_len,
439 info->required_keynode);
440 if (!ret)
441 return ret;
442 }
443
444 /* Look for a key that matches our hint */
445 snprintf(name, sizeof(name), "key-%s", info->keyname);
446 node = fdt_subnode_offset(blob, sig_node, name);
447 ret = rsa_verify_with_keynode(info, hash, sig, sig_len, node);
448 if (!ret)
449 return ret;

If I understand it correctly, at Line 440 we check if verification
with the required key succeeded and if so we return otherwise we
continue, trying other keys.

Is that the intended behavior? Shouldn't the code return in any case
(thus making the FIT verification process fail if the image couldn't
be verified with the required key)? Or am I missing something?

Regards,
Daniele
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] RSA verify code and required keys

2019-09-13 Thread Simon Glass
Hi Daniele,

On Fri, 13 Sep 2019 at 09:50, Daniele Alessandrelli
 wrote:
>
> Hi,
>
> I was looking at the RSA image authentication code and I'm a bit
> puzzled by the following line of codes in lib/rsa/rsa-verify.c
> (https://gitlab.denx.de/u-boot/u-boot/blob/master/lib/rsa/rsa-verify.c#L440):
>
> 436 /* See if we must use a particular key */
> 437 if (info->required_keynode != -1) {
> 438 ret = rsa_verify_with_keynode(info, hash, sig, sig_len,
> 439 info->required_keynode);
> 440 if (!ret)
> 441 return ret;
> 442 }
> 443
> 444 /* Look for a key that matches our hint */
> 445 snprintf(name, sizeof(name), "key-%s", info->keyname);
> 446 node = fdt_subnode_offset(blob, sig_node, name);
> 447 ret = rsa_verify_with_keynode(info, hash, sig, sig_len, node);
> 448 if (!ret)
> 449 return ret;
>
> If I understand it correctly, at Line 440 we check if verification
> with the required key succeeded and if so we return otherwise we
> continue, trying other keys.

Yes that's my understanding too.

>
> Is that the intended behavior? Shouldn't the code return in any case
> (thus making the FIT verification process fail if the image couldn't
> be verified with the required key)? Or am I missing something?

Yes I think you are right. The documentation says:

- required: If present this indicates that the key must be verified for the
image / configuration to be considered valid. Only required keys are
normally verified by the FIT image booting algorithm. Valid values are
"image" to force verification of all images, and "conf" to force verification
of the selected configuration (which then relies on hashes in the images to
verify those).

The test coverage does not handle that case at present, but it should.

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot