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

[Jun Nie]

Hi,

Does anyone have comments on this patch?

Thank you!
Jun

2018-02-11 19:56 GMT+08:00 Jun Nie <jun.nie at linaro.org>:
> Add signature verification when loading FIT image in SPL.
>
> 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);
> --
> 1.9.1
>