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

[Simon Glass]

Hi Philippe,

On Wed, 24 Mar 2021 at 06:16, Philippe REYNES
<philippe.reynes at softathome.com> wrote:
>
> Hi Simon and Alex,
>
> Le 23/03/2021 à 01:56, Simon Glass a écrit :
> > 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?
>
>
> That's my idea, a header with only the minimum information (like fit
> size and signature).
> The information about the signature (hash, algo, padding, public key,
> ...) may be stored
> in the u-boot device tree. So u-boot won't parse the fit image, only
> compute the hash
> to check the signature.
>
> >
> > 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.
>
>
> I was thinking that the signature information could be stored in the
> u-boot device tree
> (or hardcoded in the u-boot configuration). The idea is to have a very
> simple header.
> I also think that this signature may be used with the signature in the
> fit.  It is two
> options, so users may eanble both options.
>
> As we agree on the principle, I will sent a RFC asap.

You can store the public key (or whatever is used) in the U-Boot
devicetree, but the signature presumably has to be attached to the
FIT, right?

Regards,
Simon