[U-Boot] [RFC 3/3] lib: rsa: add rsa_verify_with_pkey()

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

Simon,

Overall, do you agree to my approach here?

On Mon, Sep 16, 2019 at 10:48:07PM -0700, Simon Glass wrote:
> Hi AKASHI,
> 
> On Fri, 6 Sep 2019 at 00:06, AKASHI Takahiro <takahiro.akashi@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.akashi@linaro.org>
> > ---
> >  lib/rsa/Kconfig      |  7 +++++
> >  lib/rsa/Makefile     |  3 ++-
> >  lib/rsa/rsa-verify.c | 63 ++++++++++++++++++++++++++++++++++++++------
> >  3 files changed, 64 insertions(+), 9 deletions(-)
> >
> > diff --git a/lib/rsa/Kconfig b/lib/rsa/Kconfig
> > index 338c8124da59..3c1986a26f8c 100644
> > --- a/lib/rsa/Kconfig
> > +++ b/lib/rsa/Kconfig
> > @@ -25,6 +25,13 @@ config RSA_VERIFY
> >         help
> >           Add RSA signature verification support.
> >
> > +config RSA_VERIFY_WITH_PKEY
> > +       bool "Execute RSA verification without key parameters from FDT"
> > +       depends on RSA
> > +       help
> > +         This options enables RSA signature verification without
> > +         using public key parameters which is embedded control FDT.
> 
> Please expand this, a lot. It is too brief.

Will add more description.

> > +
> >  config RSA_SOFTWARE_EXP
> >         bool "Enable driver for RSA Modular Exponentiation in software"
> >         depends on DM
> > diff --git a/lib/rsa/Makefile b/lib/rsa/Makefile
> > index d66eef74c514..fd4592fd6a8a 100644
> > --- a/lib/rsa/Makefile
> > +++ b/lib/rsa/Makefile
> > @@ -5,5 +5,6 @@
> >  # (C) Copyright 2000-2007
> >  # Wolfgang Denk, DENX Software Engineering, wd at denx.de.
> >
> > -obj-$(CONFIG_RSA_VERIFY) += rsa-verify.o rsa-checksum.o rsa-keyprop.o
> > +obj-$(CONFIG_RSA_VERIFY) += rsa-verify.o rsa-checksum.o
> > +obj-$(CONFIG_RSA_VERIFY_WITH_PKEY) += rsa-keyprop.o
> >  obj-$(CONFIG_RSA_SOFTWARE_EXP) += rsa-mod-exp.o
> > diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c
> > index 287fcc4d234d..80eabff3e940 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 */
> > +#undef CONFIG_RSA_VERIFY_WITH_PKEY
> > +#endif
> > +
> >  /* Default public exponent for backward compatibility */
> >  #define RSA_DEFAULT_PUBEXP     65537
> >
> > @@ -342,6 +347,34 @@ static int rsa_verify_key(struct image_sign_info *info,
> >         return 0;
> >  }
> >
> > +#ifdef CONFIG_RSA_VERIFY_WITH_PKEY
> > +/**
> > + * rsa_verify_with_pkey()
> > + *
> > + */
> > +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);
> > +       if (!prop) {
> > +               debug("Generating necessary parameter for decoding failed\n");
> > +               return -EACCES;
> > +       }
> > +
> > +       ret = rsa_verify_key(info, prop, sig, sig_len, hash,
> > +                            info->crypto->key_len);
> > +
> > +       rsa_free_key_prop(prop);
> > +
> > +       return ret;
> > +}
> > +#endif
> > +
> > +#if CONFIG_IS_ENABLED(FIT_SIGNATURE)
> >  /**
> >   * rsa_verify_with_keynode() - Verify a signature against some data using
> >   * information in node with prperties of RSA Key like modulus, exponent etc.
> > @@ -395,18 +428,21 @@ static int rsa_verify_with_keynode(struct image_sign_info *info,
> >
> >         return ret;
> >  }
> > +#endif
> >
> >  int rsa_verify(struct image_sign_info *info,
> >                const struct image_region region[], int region_count,
> >                uint8_t *sig, uint sig_len)
> >  {
> > -       const void *blob = info->fdt_blob;
> >         /* Reserve memory for maximum checksum-length */
> >         uint8_t hash[info->crypto->key_len];
> > +       int ret = -EACCES;
> > +#if CONFIG_IS_ENABLED(FIT_SIGNATURE)
> > +       const void *blob = info->fdt_blob;
> >         int ndepth, noffset;
> >         int sig_node, node;
> >         char name[100];
> > -       int ret;
> > +#endif
> >
> >         /*
> >          * Verify that the checksum-length does not exceed the
> > @@ -419,12 +455,6 @@ int rsa_verify(struct image_sign_info *info,
> >                 return -EINVAL;
> >         }
> >
> > -       sig_node = fdt_subnode_offset(blob, 0, FIT_SIG_NODENAME);
> > -       if (sig_node < 0) {
> > -               debug("%s: No signature node found\n", __func__);
> > -               return -ENOENT;
> > -       }
> > -
> >         /* Calculate checksum with checksum-algorithm */
> >         ret = info->checksum->calculate(info->checksum->name,
> >                                         region, region_count, hash);
> > @@ -433,6 +463,22 @@ int rsa_verify(struct image_sign_info *info,
> >                 return -EINVAL;
> >         }
> >
> > +#ifdef CONFIG_RSA_VERIFY_WITH_PKEY
> 
> Can this use if() instead of #ifdef?

Do you mean
  if (IS_ENABLED(CONFIG_RSA_VERIFY_WITH_PKEY) && !info->fdt_blob) ?

> > +       if (!info->fdt_blob) {
> > +               /* don't rely on fdt properties */
> > +               ret = rsa_verify_with_pkey(info, hash, sig, sig_len);
> 
> Does this support required_keynode?

IICU, no because required_keynode is an offset into fdt.

> Please add to the documentation for secure boot in uImage, as this
> seems to be a new case.

It may work with FIT in case of no extra key parameters,
I intended that the feature is used only by UEFI.
So I don't think it appropriate to add a doc under doc/uImage.FIT.

> Also, how do we test this new code?

The code is exercised and tested only through UEFI's verification code
and relevant pytest (in my secure boot patch[1]). Like test_vboot?

[1] https://lists.denx.de/pipermail/u-boot/2019-September/383897.html

Thanks,
-Takahiro Akashi


> 
> > +
> > +               return ret;
> > +       }
> > +#endif
> > +
> > +#if CONFIG_IS_ENABLED(FIT_SIGNATURE)
> > +       sig_node = fdt_subnode_offset(blob, 0, FIT_SIG_NODENAME);
> > +       if (sig_node < 0) {
> > +               debug("%s: No signature node found\n", __func__);
> > +               return -ENOENT;
> > +       }
> > +
> >         /* See if we must use a particular key */
> >         if (info->required_keynode != -1) {
> >                 ret = rsa_verify_with_keynode(info, hash, sig, sig_len,
> > @@ -459,6 +505,7 @@ int rsa_verify(struct image_sign_info *info,
> >                                 break;
> >                 }
> >         }
> > +#endif
> >
> >         return ret;
> >  }
> > --
> > 2.21.0
> >