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

22 Oct 2019

Hi Takahiro,
On Tue, 17 Sep 2019 at 20:59, AKASHI Takahiro
takahiro.akashi@linaro.org wrote:
...
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)
Can you use if() instead of #if ?
...
...
...

  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) ?
Yes
...
...
...

  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.
Sounds good.
...
...
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?
OK good.
...
[1] https://lists.denx.de/pipermail/u-boot/2019-September/383897.html
Thanks,
-Takahiro Akashi
Regards,
Simon

    