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

[Ruchika Gupta <ruchika.gupta@freescale.com>]

Hi Simon,

> -----Original Message-----
> From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
> Sent: Saturday, January 03, 2015 3:55 AM
> To: Gupta Ruchika-R66431
> Cc: U-Boot Mailing List; Sun York-R58495
> Subject: Re: [PATCH 9/9] [v4] rsa: Use checksum algorithms from struct
> hash_algo
> 
> Hi Ruchika,
> 
> On 30 December 2014 at 02:30, 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 v4:
> > No changes in this patch. Still under discussion
> >
> > 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 | 17 ++++++++++----
> >  lib/rsa/rsa-checksum.c        | 53
> +++++++++++++++++++++++++++++++++++++++----
> >  lib/rsa/rsa-verify.c          |  7 +++++-
> >  5 files changed, 74 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..3c69d85 100644
> > --- a/include/u-boot/rsa-checksum.h
> > +++ b/include/u-boot/rsa-checksum.h
> > @@ -16,9 +16,18 @@ 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);
> > +/**
> > + * hash_calculate() - Calculate hash over the data
> > + *
> > + * @name:  Name of algorithm to be used for hash calculation
> > + * @region: Array having info of regions over which hash needs to be
> > +calculated
> > + * @region_count: Number of regions in the region array
> > + * @checksum: Buffer contanining the output hash
> > + *
> > + * @return 0 if OK, < 0 if error
> > + */
> > +int hash_calculate(const char *name,
> > +                  const struct image_region region[], int region_count,
> > +                  uint8_t *checksum);
> >
> >  #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);
> 
> Shouldn't you have error checking on these calls?
I will add the checks
> 
> > +
> > +       return 0;
> > +}
> > +
> > +#else
> 
> It seems odd to duplicate this code. I'll email you a WIP patch to correct
> this. Perhaps you could tidy it up and attach it to your series? Then I think
> you can remove this #else part.
I will try the WIP patch to see I this can be corrected.
> 
> > +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
> > af915d3..cf5acdf 100644
> > --- a/lib/rsa/rsa-verify.c
> > +++ b/lib/rsa/rsa-verify.c
> > @@ -201,7 +201,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
Regards,
Ruchika