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
