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

[Andre Przywara]

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.

> 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);
>