Hardware hashing on unsigned fitImage nodes.

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:

common/image-fit.c +19 -1

index 4ffc5aa..1cb26cc 100644
@@ -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.

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.

Hi Andre,

Quoted 47 lines · Andre Wolokita

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:

common/image-fit.c +19 -1

index 4ffc5aa..1cb26cc 100644
@@ -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;
        }
 

Quoted 1 line

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.

Regards,
Simon

Quoted 15 lines

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.