[PATCH v2] spl: Add callback for preprocessing loaded FIT header before parsing

[Simon Glass]

Hi Alex,

On Tue, 23 Mar 2021 at 04:12, Alex G. <mr.nuke.me at gmail.com> wrote:
>
> On 3/22/21 9:27 AM, Philippe REYNES wrote:
> > Hi all,
> >
> >
> > Le 11/03/2021 à 00:10, Alex G a écrit :
> [snip]
> >
> > I reach the same issue, my customers are also worried with the actual
> > signature check scheme on u-boot.
> > The fit data/node are parsed before being checked : data should be used
> > only after being checked, not before.
> > The code become quite complex for a signature, and the more complex the
> > code is more risk to have/introduce a bug or security issue.
>
> [snip]
>
> >>> The reason I used a weak function was to mirror the already
> >>> upstreamed board_spl_fit_post_load(),
> >>
> >> I see why you'd think it was a good idea. board_spl_fit_pre_load()
> >> sneaks in a dependency on arch-specific code (CONFIG_IMX_HAB). I don't
> >> really like the way it's implemented, and I don't know if it would
> >> work with SPL_LOAD_FIT_FULL or bootm.
> >>
> >
> > As I reach the same issue, I was also thinking strongly about adding a
> > "hook" before the fit image is launched/analyzed. In my mind this "pre
> > load" function should be able to do some check/update to the fit image,
> > but also modify the beginning of the fit image (to remove a header for
> > example). Such function/feature may allow to:
> > - check a signature for the full fit (without parsing the node)
> > - cipher the full fit (even the node)
> > - compress the full fit
> > - probably that users will find a lot of others ideas .....
> >
> > I think that this feature pre load should be implemented in spl and
> > bootm command.
> >
> > I have understood the feedback about a useful implementation/usage of
> > pre_load.
> > I propose to sent an example soon (probably based on signature check).
>
> So "what" you want to do is verify untrusted metadata before using it.
> That's a very logical and reasonable thing to do.
>
> "How" you are trying to do this is by
>   (1) adding a weak function
>   (2) allowing each board to have a completely different implementation
>
> Those are two terrible ideas.
>
> I agree that there is a deficiency in the way FIT images are signed. Can
> we stick the signature between the fdt_header and before dt_struct?

That seems like a reasonable idea to me. Even better might be to have
it completely separate, e.g. before the FIT starts, so no parsing at
all is needed?

Also, which signature? FIT supports multiple signatures which can be
added at different times. Perhaps this could be for a base signature,
enough to get through to verifying the 'real' signature.

Regards,
Simon