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.aka...@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, w...@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; > > >>> } > > >>> > > >> > > > > > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot