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

AKASHI Takahiro takahiro.akashi at linaro.org
Thu Oct 17 07:37:01 UTC 2019


On Tue, Oct 15, 2019 at 04:17:04PM +0900, AKASHI Takahiro wrote:
> On Sat, Oct 12, 2019 at 02:47:33PM +0200, Heinrich Schuchardt wrote:
> > On 10/10/19 3:04 AM, AKASHI Takahiro wrote:
> > >On Wed, Oct 09, 2019 at 07:56:04PM +0200, Heinrich Schuchardt wrote:
> > >>On 10/9/19 7:30 AM, AKASHI Takahiro 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 at linaro.org>
> > >>
> > >>Is this code tested by test/py/tests/test_vboot.py? Or is there another
> > >>unit test?
> > >
> > >I haven't run vboot test yet.
> > 
> > I would have assumed that you to through Travis CI before submitting.
> > 
> > >For rsa_verify_with_pkey(), as with vtest for FIT signature,
> > >we can test it with my "UEFI secure boot" pytest.
> > 
> > I can't see such a test in this patch series. So how I can test the
> > changes before merging?
> 
> The only solution that I can give you is that I would submit
> rsa patch with my "UEFI secure boot" patch.

I added a very simple function test in "ut" command but will defer
sending a new version until after v2 of "x509 parser" patch
as I found there is some dependency on public key parser in rsa_keyprop.c.

-Takahiro Akashi

> 
> 
> > Best regards
> > 
> > Heinrich
> > 
> > >
> > >>>---
> > >>>  lib/rsa/Kconfig      |  7 -----
> > >>>  lib/rsa/Makefile     |  1 -
> > >>>  lib/rsa/rsa-verify.c | 63 ++++++++++++++++++++++++++++++++++++++------
> > >>>  3 files changed, 55 insertions(+), 16 deletions(-)
> > >>>
> > >>>diff --git a/lib/rsa/Kconfig b/lib/rsa/Kconfig
> > >>>index d1743d7a4c47..62b7ab9c5e5c 100644
> > >>>--- a/lib/rsa/Kconfig
> > >>>+++ b/lib/rsa/Kconfig
> > >>>@@ -30,13 +30,6 @@ 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.
> > >>>-
> > >>>  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 14ed3cb4012b..c07305188e0c 100644
> > >>>--- a/lib/rsa/Makefile
> > >>>+++ b/lib/rsa/Makefile
> > >>>@@ -6,5 +6,4 @@
> > >>>  # Wolfgang Denk, DENX Software Engineering, wd at denx.de.
> > >>>
> > >>>  obj-$(CONFIG_$(SPL_)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 1df42f28c64a..ce79984b30f9 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
> > >>
> > >>Where should it have been defined?
> > >
> > >defined in patch#2
> > >
> > >>>+#endif
> > >>>+
> > >>>  /* Default public exponent for backward compatibility */
> > >>>  #define RSA_DEFAULT_PUBEXP	65537
> > >>>
> > >>>@@ -344,6 +349,34 @@ static int rsa_verify_key(struct image_sign_info *info,
> > >>>  }
> > >>>  #endif
> > >>>
> > >>>+#ifdef CONFIG_RSA_VERIFY_WITH_PKEY
> > >>
> > >>Where would CONFIG_RSA_VERIFY_WITH_PKEY be defined? You just a removed
> > >>it from Kconfig.
> > >
> > >Because rsa-keyprop.c be compiled only under CONFIG_RSA_VERIFY_WITH_PKEY
> > >per Simon's comment.
> > >
> > >>>+/**
> > >>>+ * rsa_verify_with_pkey()
> > >>
> > >>The short text for the function is missing.
> > >>
> > >>>+ *
> > >>
> > >>Please, describe the parameters.
> > >
> > >Sure.
> > >
> > >-Takahiro Akashi
> > >
> > >>>+ */
> > >>>+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.
> > >>>@@ -397,18 +430,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
> > >>>@@ -421,12 +457,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);
> > >>>@@ -435,6 +465,22 @@ int rsa_verify(struct image_sign_info *info,
> > >>>  		return -EINVAL;
> > >>>  	}
> > >>>
> > >>>+#ifdef CONFIG_RSA_VERIFY_WITH_PKEY
> > >>
> > >>Where should this have been defined?
> > >>
> > >>Best regards
> > >>
> > >>Heinrich
> > >>
> > >>>+	if (!info->fdt_blob) {
> > >>>+		/* don't rely on fdt properties */
> > >>>+		ret = rsa_verify_with_pkey(info, hash, sig, sig_len);
> > >>>+
> > >>>+		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,
> > >>>@@ -461,6 +507,7 @@ int rsa_verify(struct image_sign_info *info,
> > >>>  				break;
> > >>>  		}
> > >>>  	}
> > >>>+#endif
> > >>>
> > >>>  	return ret;
> > >>>  }
> > >>>
> > >>
> > >
> > 


More information about the U-Boot mailing list