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

[Farhan Ali]

Phillipe,

In our implementation we store our binaries outside the FIT header, and
introduce a gap between the header and the start of binary data (-p and -E
option in mkimage). After the FIT has been generated, we sign the FIT
header and insert the signature into this gap. The weak function then
checks the signature after 'only' the header has been loaded, but before
any of the FIT fields have been parsed.

Whatever common implementation we decide on, it is imperative that the
signature can be inserted 'AFTER' the complete FIT has been generated. The
reason this is so critical is to allow for off-line signing via customer
HSMs.

Regards,
Farhan

On Wed, Mar 24, 2021 at 12:09 AM Simon Glass <sjg at chromium.org> wrote:

> 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
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4203 bytes
Desc: S/MIME Cryptographic Signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210329/41bbd525/attachment.bin>