[PATCH] image: fit: Fix not verifying data configuration

[Simon Glass]

Hi Sean,

On Tue, 11 Oct 2022 at 17:25, Sean Anderson <sean.anderson at seco.com> wrote:
>
> When reading data from a FIT image, we must verify the configuration we
> get it from. This is because when we have a key with required = "conf",
> the image does not need any particular signature or hash. The
> configuration is the only required verification, so we must verify it.
>
> Users of fit_get_data_node are liable to load unsigned data unless the
> user has set required = "image". Even then, they are vulnerable to
> mix-and-match attacks. This also affects other callers of
> fit_image_verify which don't first call fit_config_verify, such as
> source and imxtract. I don't think there is a backwards-compatible way
> to fix these interfaces. Fundamentally, selecting data by image when
> images are not required to be verified is unsafe.
>
> Fixes: 37feaf2f727 ("image: fit: Add some helpers for getting data")
> Signed-off-by: Sean Anderson <sean.anderson at seco.com>
> ---
>
>  boot/image-fit.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/boot/image-fit.c b/boot/image-fit.c
> index 9c04ff78a15..632fd405e29 100644
> --- a/boot/image-fit.c
> +++ b/boot/image-fit.c
> @@ -1948,7 +1948,14 @@ int fit_get_data_node(const void *fit, const char *image_uname,
>  int fit_get_data_conf_prop(const void *fit, const char *prop_name,
>                            const void **data, size_t *size)
>  {
> -       int noffset = fit_conf_get_node(fit, NULL);
> +       int ret, noffset = fit_conf_get_node(fit, NULL);
> +
> +       if (noffset < 0)
> +               return noffset;
> +
> +       ret = fit_config_verify(fit, noffset);
> +       if (ret)
> +               return ret;
>
>         noffset = fit_conf_get_prop_node(fit, noffset, prop_name);
>         return fit_get_data_tail(fit, noffset, data, size);
> --
> 2.35.1.1320.gc452695387.dirty
>

This is supposed to work by first verifying the configuration with
fit_config_verify(). After that, images in that configuration can be
freely loaded, verified by the hash that each image has.

So we need to make sure that first step is taken in all code paths,
rather than adding it willy nilly.

If people don't add hashes (or perhaps signature if they want to use
more CPU time) for the images, then there is no protection. We could
add warnings or errors to mkimage for this case?

Regards,
Simon