[U-Boot] [PATCH] SPL: Add signature verification when loading image

[Jun Nie]

2018-02-26 17:56 GMT+08:00 Andre Przywara <andre.przywara at arm.com>:
> Hi,
>
> On 11/02/18 11:56, Jun Nie wrote:
>> Add signature verification when loading FIT image in SPL.
>
> this message is very thin. Can you explain WHY you did this change and
> what it's supposed to do and what it improves?
> For my part I am completely clueless what you are after.
>
> Cheers,
> Andre.

There is no support for signature check to u-boot proper in SPL,
except platform specific implementation. This patch add signature
check to u-boot in SPL, that can be shared on most platforms, if not
all.

Jun

>
>> Signed-off-by: Jun Nie <jun.nie at linaro.org>
>> ---
>>  common/image-fit.c   | 56 +++++++++++++++++++++++++++++++---------------------
>>  common/spl/spl_fit.c | 12 +++++++++++
>>  include/image.h      |  2 ++
>>  3 files changed, 48 insertions(+), 22 deletions(-)
>>
>> diff --git a/common/image-fit.c b/common/image-fit.c
>> index f6e956a..94d9d4b 100644
>> --- a/common/image-fit.c
>> +++ b/common/image-fit.c
>> @@ -1068,34 +1068,14 @@ static int fit_image_check_hash(const void *fit, int noffset, const void *data,
>>       return 0;
>>  }
>>
>> -/**
>> - * fit_image_verify - verify data integrity
>> - * @fit: pointer to the FIT format image header
>> - * @image_noffset: component image node offset
>> - *
>> - * fit_image_verify() goes over component image hash nodes,
>> - * re-calculates each data hash and compares with the value stored in hash
>> - * node.
>> - *
>> - * returns:
>> - *     1, if all hashes are valid
>> - *     0, otherwise (or on error)
>> - */
>> -int fit_image_verify(const void *fit, int image_noffset)
>> +int fit_image_verify_with_data(const void *fit, int image_noffset,
>> +                             const void *data, size_t size)
>>  {
>> -     const void      *data;
>> -     size_t          size;
>>       int             noffset = 0;
>>       char            *err_msg = "";
>>       int verify_all = 1;
>>       int ret;
>>
>> -     /* Get image data and data length */
>> -     if (fit_image_get_data(fit, image_noffset, &data, &size)) {
>> -             err_msg = "Can't get image data/size";
>> -             goto error;
>> -     }
>> -
>>       /* Verify all required signatures */
>>       if (IMAGE_ENABLE_VERIFY &&
>>           fit_image_verify_required_sigs(fit, image_noffset, data, size,
>> @@ -1153,6 +1133,38 @@ error:
>>  }
>>
>>  /**
>> + * fit_image_verify - verify data integrity
>> + * @fit: pointer to the FIT format image header
>> + * @image_noffset: component image node offset
>> + *
>> + * fit_image_verify() goes over component image hash nodes,
>> + * re-calculates each data hash and compares with the value stored in hash
>> + * node.
>> + *
>> + * returns:
>> + *     1, if all hashes are valid
>> + *     0, otherwise (or on error)
>> + */
>> +int fit_image_verify(const void *fit, int image_noffset)
>> +{
>> +     const void      *data;
>> +     size_t          size;
>> +     int             noffset = 0;
>> +     char            *err_msg = "";
>> +
>> +     /* Get image data and data length */
>> +     if (fit_image_get_data(fit, image_noffset, &data, &size)) {
>> +             err_msg = "Can't get image data/size";
>> +             printf("error!\n%s for '%s' hash node in '%s' image node\n",
>> +                    err_msg, fit_get_name(fit, noffset, NULL),
>> +                    fit_get_name(fit, image_noffset, NULL));
>> +             return 0;
>> +     }
>> +
>> +     return fit_image_verify_with_data(fit, image_noffset, data, size);
>> +}
>> +
>> +/**
>>   * fit_all_image_verify - verify data integrity for all images
>>   * @fit: pointer to the FIT format image header
>>   *
>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>> index cc07fbc..8d382eb 100644
>> --- a/common/spl/spl_fit.c
>> +++ b/common/spl/spl_fit.c
>> @@ -174,6 +174,9 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
>>       uint8_t image_comp = -1, type = -1;
>>       const void *data;
>>       bool external_data = false;
>> +#ifdef CONFIG_SPL_FIT_SIGNATURE
>> +     int ret;
>> +#endif
>>
>>       if (IS_ENABLED(CONFIG_SPL_OS_BOOT) && IS_ENABLED(CONFIG_SPL_GZIP)) {
>>               if (fit_image_get_comp(fit, node, &image_comp))
>> @@ -252,7 +255,16 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
>>               image_info->entry_point = fdt_getprop_u32(fit, node, "entry");
>>       }
>>
>> +#ifdef CONFIG_SPL_FIT_SIGNATURE
>> +     printf("## Checking hash(es) for Image %s ...\n",
>> +            fit_get_name(fit, node, NULL));
>> +     ret = fit_image_verify_with_data(fit, node,
>> +                                      (const void *)load_addr, length);
>> +     printf("\n");
>> +     return !ret;
>> +#else
>>       return 0;
>> +#endif
>>  }
>>
>>  static int spl_fit_append_fdt(struct spl_image_info *spl_image,
>> diff --git a/include/image.h b/include/image.h
>> index 325b014..77c11f8 100644
>> --- a/include/image.h
>> +++ b/include/image.h
>> @@ -1013,6 +1013,8 @@ int fit_add_verification_data(const char *keydir, void *keydest, void *fit,
>>                             const char *comment, int require_keys,
>>                             const char *engine_id);
>>
>> +int fit_image_verify_with_data(const void *fit, int image_noffset,
>> +                            const void *data, size_t size);
>>  int fit_image_verify(const void *fit, int noffset);
>>  int fit_config_verify(const void *fit, int conf_noffset);
>>  int fit_all_image_verify(const void *fit);
>>