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

Simon Glass
2014-12-29T21:12:03Z

Hi Ruchika,

On 29 December 2014 at 00:59, Ruchika Gupta <ruchika.gupta@freescale.com> wrote:
> Hi Simon,
>
>> -----Original Message-----
>> From: sjg at google.com [mailto:sjg at 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