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

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@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