Simon, On Tue, Nov 19, 2019 at 06:59:56PM -0800, Simon Glass wrote: > Hi Takahiro, > > On Tue, 12 Nov 2019 at 16:47, AKASHI Takahiro > <takahiro.aka...@linaro.org> wrote: > > > > This function, and hence rsa_verify(), will perform RSA verification > > with two essential parameters for a RSA public key in contract of > > rsa_verify_with_keynode(), which requires additional three parameters > > stored in FIT image. > > > > It will be used in implementing UEFI secure boot, i.e. image authentication > > and variable authentication. > > > > Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org> > > --- > > lib/rsa/rsa-verify.c | 57 +++++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 56 insertions(+), 1 deletion(-) > > Reviewed-by: Simon Glass <s...@chromium.org> > > Nits below though > > > > > diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c > > index d2fd0692fa13..3f63bd9c175b 100644 > > --- a/lib/rsa/rsa-verify.c > > +++ b/lib/rsa/rsa-verify.c > > @@ -17,9 +17,14 @@ > > #include "mkimage.h" > > #include <fdt_support.h> > > #endif > > +#include <linux/kconfig.h> > > #include <u-boot/rsa-mod-exp.h> > > #include <u-boot/rsa.h> > > > > +#ifndef __UBOOT__ /* for host tools */ > > Comment here as to why this is needed
Sure. > > +#undef CONFIG_RSA_VERIFY_WITH_PKEY > > +#endif > > + > > /* Default public exponent for backward compatibility */ > > #define RSA_DEFAULT_PUBEXP 65537 > > > > @@ -270,7 +275,7 @@ out: > > } > > #endif > > > > -#if CONFIG_IS_ENABLED(FIT_SIGNATURE) > > +#if CONFIG_IS_ENABLED(FIT_SIGNATURE) || > > IS_ENABLED(CONFIG_RSA_VERIFY_WITH_PKEY) > > /** > > * rsa_verify_key() - Verify a signature against some data using RSA Key > > * > > @@ -344,6 +349,49 @@ static int rsa_verify_key(struct image_sign_info *info, > > } > > #endif > > > > +#ifdef CONFIG_RSA_VERIFY_WITH_PKEY > > +/** > > + * rsa_verify_with_pkey() - Verify a signature against some data using > > + * only modulus and exponent as RSA key properties. > > + * @info: Specifies key information > > + * @hash: Pointer to the expected hash > > + * @sig: Signature > > + * @sig_len: Number of bytes in signature > > + * > > + * Parse a RSA public key blob in DER format pointed to in @info and fill > > + * a key_prop structure with properties of the key. Then verify a RSA > > PKCS1.5 > > + * signature against an expected hash using the calculated properties. > > + * > > + * Return 0 if verified, -ve on error > > + */ > > +static int rsa_verify_with_pkey(struct image_sign_info *info, > > + const void *hash, uint8_t *sig, uint > > sig_len) > > +{ > > + struct key_prop *prop; > > + int ret; > > + > > + /* Public key is self-described to fill key_prop */ > > + prop = rsa_gen_key_prop(info->key, info->keylen); > > This function should return an int error code, with the pointer as one > of the parameters. It isn't good to lose the real error here. Okay. > > + if (!prop) { > > + debug("Generating necessary parameter for decoding > > failed\n"); > > + return -EACCES; > > return real error here. Okay. Thanks, -Takahiro Akashi > > + } > > + > > + ret = rsa_verify_key(info, prop, sig, sig_len, hash, > > + info->crypto->key_len); > > + > > + rsa_free_key_prop(prop); > > + > > + return ret; > > +} > > +#else > > +static int rsa_verify_with_pkey(struct image_sign_info *info, > > + const void *hash, uint8_t *sig, uint > > sig_len) > > +{ > > + return -EACCES; > > +} > > +#endif > > + > > #if CONFIG_IS_ENABLED(FIT_SIGNATURE) > > /** > > * rsa_verify_with_keynode() - Verify a signature against some data using > > @@ -434,6 +482,13 @@ int rsa_verify(struct image_sign_info *info, > > return -EINVAL; > > } > > > > + if (IS_ENABLED(CONFIG_RSA_VERIFY_WITH_PKEY) && !info->fdt_blob) { > > + /* don't rely on fdt properties */ > > + ret = rsa_verify_with_pkey(info, hash, sig, sig_len); > > + > > + return ret; > > + } > > + > > if (CONFIG_IS_ENABLED(FIT_SIGNATURE)) { > > const void *blob = info->fdt_blob; > > int ndepth, noffset; > > -- > > 2.21.0 > > > > Regards, > Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot