Hi Philippe, On Wed, 24 Mar 2021 at 06:16, Philippe REYNES <philippe.rey...@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...@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
