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

[Alex G.]

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?

Alex