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