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.

Regards,
Simon


Regards,
Philippe


Reply via email to