diff mbox

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

Message ID 1419334379-32294-10-git-send-email-ruchika.gupta@freescale.com
State Superseded
Delegated to: Tom Rini
Headers show

Commit Message

Ruchika Gupta Dec. 23, 2014, 11:32 a.m. UTC
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(-)

Comments

Simon Glass Dec. 24, 2014, 12:50 a.m. UTC | #1
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
Ruchika Gupta Dec. 29, 2014, 8 a.m. UTC | #2
Sorry. Resending as message bounced on uboot mailing list.

Hi Simon,

> -----Original Message-----
> From: sjg@google.com [mailto:sjg@google.com] On Behalf Of Simon Glass
> Sent: Wednesday, December 24, 2014 6:20 AM
> To: Gupta Ruchika-R66431
> Cc: U-Boot Mailing List; Sun York-R58495
> Subject: Re: [PATCH 9/9] [v3] rsa: Use checksum algorithms from struct
> hash_algo
>
> 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?
The hash_algo struct is defined in common/hash.c which doesn’t get compiled for tools. That is why I did it differently 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
Regards,
Ruchika
Simon Glass Dec. 29, 2014, 9:12 p.m. UTC | #3
Hi Ruchika,

On 29 December 2014 at 00:59, Ruchika Gupta <ruchika.gupta@freescale.com> wrote:
> Hi Simon,
>
>> -----Original Message-----
>> From: sjg@google.com [mailto:sjg@google.com] On Behalf Of Simon Glass
>> Sent: Wednesday, December 24, 2014 6:20 AM
>> To: Gupta Ruchika-R66431
>> Cc: U-Boot Mailing List; Sun York-R58495
>> Subject: Re: [PATCH 9/9] [v3] rsa: Use checksum algorithms from struct
>> hash_algo
>>
>> 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?
> The hash_algo struct is defined in common/hash.c which doesn’t get compiled for tools. That is why I did it differently for USE_HOSTCC

I wonder if we should compile hash.c for tools? Would that be easier or harder?

[snip]

Regards,
Simon
Ruchika Gupta Dec. 30, 2014, 8:58 a.m. UTC | #4
Hi Simon,

> -----Original Message-----
> From: sjg@google.com [mailto:sjg@google.com] On Behalf Of Simon Glass
> Sent: Tuesday, December 30, 2014 2:42 AM
> To: Gupta Ruchika-R66431
> Cc: U-Boot Mailing List; Sun York-R58495
> Subject: Re: [PATCH 9/9] [v3] rsa: Use checksum algorithms from struct
> hash_algo
> 
> Hi Ruchika,
> 
> On 29 December 2014 at 00:59, Ruchika Gupta <ruchika.gupta@freescale.com>
> wrote:
> > Hi Simon,
> >
> >> -----Original Message-----
> >> From: sjg@google.com [mailto:sjg@google.com] On Behalf Of Simon Glass
> >> Sent: Wednesday, December 24, 2014 6:20 AM
> >> To: Gupta Ruchika-R66431
> >> Cc: U-Boot Mailing List; Sun York-R58495
> >> Subject: Re: [PATCH 9/9] [v3] rsa: Use checksum algorithms from
> >> struct hash_algo
> >>
> >> 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?
> > The hash_algo struct is defined in common/hash.c which doesn’t get
> > compiled for tools. That is why I did it differently for USE_HOSTCC
> 
> I wonder if we should compile hash.c for tools? Would that be easier or
> harder?
I had tried doing that but it gave me loads of compilation errors. I thought it would be better to leave it for now.
> 
> [snip]
> 
> Regards,
> Simon

Regards,
Ruchika
diff mbox

Patch

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);
 
 #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
+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) {