[U-Boot] [PATCH 9/9] [v3] rsa: Use checksum algorithms from struct hash_algo

Simon Glass
2014-12-24T00:50:21Z

Hi Ruchika,

On 23 December 2014 at 04:32, Ruchika Gupta <ruchika.gupta@freescale.com> wrote:
> Currently the hash functions used in RSA are called directly from the sha1
> and sha256 libraries. Change the RSA checksum library to use the progressive
> hash API's registered with struct hash_algo. This will allow the checksum
> library to use the hardware accelerated progressive hash API's once available.
>
> Signed-off-by: Ruchika Gupta <ruchika.gupta@freescale.com>
> CC: Simon Glass <sjg@chromium.org>
> ---
> Changes in v3:
> Modified rsa-verify to check for return from checksum function
>
> Changes in v2:
> Added generic function hash_calculate. Pass an additional
> argument as name of algorithm.
>
> common/image-sig.c | 6 ++---
> include/image.h | 5 ++--
> include/u-boot/rsa-checksum.h | 7 +++---
> lib/rsa/rsa-checksum.c | 53 +++++++++++++++++++++++++++++++++++++++----
> lib/rsa/rsa-verify.c | 7 +++++-
> 5 files changed, 64 insertions(+), 14 deletions(-)
>
> diff --git a/common/image-sig.c b/common/image-sig.c
> index 8601eda..2c9f0cd 100644
> --- a/common/image-sig.c
> +++ b/common/image-sig.c
> @@ -38,7 +38,7 @@ struct checksum_algo checksum_algos[] = {
> #if IMAGE_ENABLE_SIGN
> EVP_sha1,
> #endif
> - sha1_calculate,
> + hash_calculate,
> padding_sha1_rsa2048,
> },
> {
> @@ -48,7 +48,7 @@ struct checksum_algo checksum_algos[] = {
> #if IMAGE_ENABLE_SIGN
> EVP_sha256,
> #endif
> - sha256_calculate,
> + hash_calculate,
> padding_sha256_rsa2048,
> },
> {
> @@ -58,7 +58,7 @@ struct checksum_algo checksum_algos[] = {
> #if IMAGE_ENABLE_SIGN
> EVP_sha256,
> #endif
> - sha256_calculate,
> + hash_calculate,
> padding_sha256_rsa4096,
> }
>
> diff --git a/include/image.h b/include/image.h
> index af30d60..ec55f23 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -926,8 +926,9 @@ struct checksum_algo {
> #if IMAGE_ENABLE_SIGN
> const EVP_MD *(*calculate_sign)(void);
> #endif
> - void (*calculate)(const struct image_region region[],
> - int region_count, uint8_t *checksum);
> + int (*calculate)(const char *name,
> + const struct image_region region[],
> + int region_count, uint8_t *checksum);
> const uint8_t *rsa_padding;
> };
>
> diff --git a/include/u-boot/rsa-checksum.h b/include/u-boot/rsa-checksum.h
> index c996fb3..c546c80 100644
> --- a/include/u-boot/rsa-checksum.h
> +++ b/include/u-boot/rsa-checksum.h
> @@ -16,9 +16,8 @@ extern const uint8_t padding_sha256_rsa4096[];
> extern const uint8_t padding_sha256_rsa2048[];
> extern const uint8_t padding_sha1_rsa2048[];
>
> -void sha256_calculate(const struct image_region region[], int region_count,
> - uint8_t *checksum);
> -void sha1_calculate(const struct image_region region[], int region_count,
> - uint8_t *checksum);
> +int hash_calculate(const char *name,
> + const struct image_region region[], int region_count,
> + uint8_t *checksum);
>

This could use a function comment.

> #endif
> diff --git a/lib/rsa/rsa-checksum.c b/lib/rsa/rsa-checksum.c
> index 8d8b59f..7f1909a 100644
> --- a/lib/rsa/rsa-checksum.c
> +++ b/lib/rsa/rsa-checksum.c
> @@ -10,12 +10,13 @@
> #include <asm/byteorder.h>
> #include <asm/errno.h>
> #include <asm/unaligned.h>
> +#include <hash.h>
> #else
> #include "fdt_host.h"
> -#endif
> -#include <u-boot/rsa.h>
> #include <u-boot/sha1.h>
> #include <u-boot/sha256.h>
> +#endif
> +#include <u-boot/rsa.h>
>
> /* PKCS 1.5 paddings as described in the RSA PKCS#1 v2.1 standard. */
>
> @@ -136,7 +137,33 @@ const uint8_t padding_sha256_rsa4096[RSA4096_BYTES - SHA256_SUM_LEN] = {
> 0x03, 0x04, 0x02, 0x01, 0x05, 0x00, 0x04, 0x20
> };
>
> -void sha1_calculate(const struct image_region region[], int region_count,
> +#ifndef USE_HOSTCC
> +int hash_calculate(const char *name,
> + const struct image_region region[],
> + int region_count, uint8_t *checksum)
> +{
> + struct hash_algo *algo;
> + int ret = 0;
> + void *ctx;
> + uint32_t i;
> + i = 0;
> +
> + ret = hash_progressive_lookup_algo(name, &algo);
> + if (ret)
> + return ret;
> +
> + algo->hash_init(algo, &ctx);
> + for (i = 0; i < region_count - 1; i++)
> + algo->hash_update(algo, ctx, region[i].data, region[i].size, 0);
> +
> + algo->hash_update(algo, ctx, region[i].data, region[i].size, 1);
> + algo->hash_finish(algo, ctx, checksum, algo->digest_size);
> +
> + return 0;
> +}
> +
> +#else

The above looks good, but what is happening here? Why do you need to
do something different for USE_HOSTCC?

> +int sha1_calculate(const struct image_region region[], int region_count,
> uint8_t *checksum)
> {
> sha1_context ctx;
> @@ -147,9 +174,11 @@ void sha1_calculate(const struct image_region region[], int region_count,
> for (i = 0; i < region_count; i++)
> sha1_update(&ctx, region[i].data, region[i].size);
> sha1_finish(&ctx, checksum);
> +
> + return 0;
> }
>
> -void sha256_calculate(const struct image_region region[], int region_count,
> +int sha256_calculate(const struct image_region region[], int region_count,
> uint8_t *checksum)
> {
> sha256_context ctx;
> @@ -160,4 +189,20 @@ void sha256_calculate(const struct image_region region[], int region_count,
> for (i = 0; i < region_count; i++)
> sha256_update(&ctx, region[i].data, region[i].size);
> sha256_finish(&ctx, checksum);
> +
> + return 0;
> }
> +
> +int hash_calculate(const char *name,
> + const struct image_region region[], int region_count,
> + uint8_t *checksum)
> +{
> + if (!strcmp(name, "sha1"))
> + sha1_calculate(region, region_count, checksum);
> +
> + if (!strcmp(name, "sha256"))
> + sha256_calculate(region, region_count, checksum);
> +
> + return 0;
> +}
> +#endif
> diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c
> index 27f10ef..0028304 100644
> --- a/lib/rsa/rsa-verify.c
> +++ b/lib/rsa/rsa-verify.c
> @@ -188,7 +188,12 @@ int rsa_verify(struct image_sign_info *info,
> }
>
> /* Calculate checksum with checksum-algorithm */
> - info->algo->checksum->calculate(region, region_count, hash);
> + ret = info->algo->checksum->calculate(info->algo->checksum->name,
> + region, region_count, hash);
> + if (ret < 0) {
> + debug("%s: Error in checksum calculation\n", __func__);
> + return -EINVAL;
> + }
>
> /* See if we must use a particular key */
> if (info->required_keynode != -1) {
> --
> 1.8.1.4
>

Regards,
Simon