[U-Boot] Hardware hashing on unsigned fitImage nodes.

[Andre Wolokita <andre.wolokita@analog.com>]

Hi Simon,

On 10/07/15 04:52, Simon Glass wrote:
> Hi Andre,
>
> On 7 July 2015 at 19:22, Andre Wolokita <andre.wolokita@analog.com> wrote:
>> Hi Simon,
>>
>> Hopefully you're the right person to be contacting about this; if not, please point me in the right direction.
>>
>> We've recently had a contractor write a hash driver for our crypto accelerator, intended to accelerate fitImage verification. They noticed that the hardware hashing API would only be called for the signed nodes (we are only signing the config node) while the unsigned nodes were still being processed in software. Their proposed solution was to add logic to fit_image_check_hash() in common/fit-image.c:
>>
>> diff --git a/common/image-fit.c b/common/image-fit.c
>> index 4ffc5aa..1cb26cc 100644
>> --- a/common/image-fit.c
>> +++ b/common/image-fit.c
>> @@ -906,7 +906,10 @@ static int fit_image_check_hash(const void *fit, int noffset, const void *data,
>>         uint8_t *fit_value;
>>         int fit_value_len;
>>         int ignore;
>> -
>> +#ifdef CONFIG_ADI_HASH
>> +       struct image_region region;
>> +       int i;
>> +#endif
>>         *err_msgp = NULL;
>>
>>         if (fit_image_hash_get_algo(fit, noffset, &algo)) {
>> @@ -928,12 +931,28 @@ static int fit_image_check_hash(const void *fit, int noffset, const void *data,
>>                 *err_msgp = "Can't get hash value property";
>>                 return -1;
>>         }
>> +#ifdef CONFIG_ADI_HASH
>> +       region.data = data;
>> +       region.size = size;
>> +       memset(value, '\0', FIT_MAX_HASH_LEN);
>> +       /* h/w acceleration */
>> +       if(hash_calculate(algo, &region, 1, value)){
>> +               *err_msgp = "Unsupported hash algorithm";
>> +               return -1;
>> +       }
>> +
>> +       if(strcmp(algo, "sha1")==0)
>> +               value_len = SHA1_SUM_LEN;
>> +       else
>> +               value_len = FIT_MAX_HASH_LEN;
>>
>> +#else
>>         if (calculate_hash(data, size, algo, value, &value_len)) {
>>                 *err_msgp = "Unsupported hash algorithm";
>>                 return -1;
>>         }
>>
>> I realise that adding to common code is probably discouraged. Is there any other solutions that do not involve doing so? Thank you in advance.
> We had a similar thing come up with the RSA algorithm. If you see
> drivers/crypto you will find an API that was created for
> hardware-accelerated modular exponentiation. There is a software
> driver as a fallback.
>
> For hashing, we have hash.c which provides a reasonable API. We could
> convert this to driver model with a uclass for hashing. Perhaps it
> could support multiple algorithms, with -ENOSYS indicating that the
> driver does not support that algorithm, in which case the uclass could
> try the next driver? There is something similar in reset_walk() in
> this patch:
>
> http://patchwork.ozlabs.org/patch/487822/
>
> This would involve replacing the existing hash.c code, but I suggest
> you could do this by creating CONFIG_DM_HASH and when that is defined,
> hash. c does not get compiled. You can implement the API in hash.h
> using your uclass.
Thanks for your reply. We're short on time at the moment, unfortunately, so we will just have to keep using our hack until we can dive into the DM solution you've suggested.

Regards,

Andre.
>
> Regards,
> Simon
>
>> Regards,
>>
>> Andre.
>>
>> --
>> Andre Wolokita (andre.wolokita at analog.com)
>> Design Engineer, Analog Devices Australia Pty Ltd
>> Unit 3, 97 Lewis Road, Wantirna, Victoria, 3152, AUSTRALIA
>> Direct: +61 3 9881 9933   Main: +61 3 9881 9999
>> Fax: +61 3 9881 9988      Web: www.analog.com/au
>>
>> Embedded & Digital Signal Processing Software.
>>
>> This communication is proprietary and confidential.
>>

-- 
Andre Wolokita (andre.wolokita at analog.com)
Design Engineer, Analog Devices Australia Pty Ltd
Unit 3, 97 Lewis Road, Wantirna, Victoria, 3152, AUSTRALIA
Direct: +61 3 9881 9933   Main: +61 3 9881 9999
Fax: +61 3 9881 9988      Web: www.analog.com/au

Embedded & Digital Signal Processing Software.

This communication is proprietary and confidential.