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.
For rsa_verify_with_pkey(), as with vtest for FIT signature,
we can test it with my "UEFI secure boot" pytest.
> >---
> > 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
