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

[Heinrich Schuchardt]

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?

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

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

> +/**
> + * rsa_verify_with_pkey()

The short text for the function is missing.

> + *

Please, describe the parameters.

> + */
> +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;
>   }
>