From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex G. Date: Mon, 22 Mar 2021 10:12:23 -0500 Subject: [PATCH v2] spl: Add callback for preprocessing loaded FIT header before parsing In-Reply-To: <389855fe-9401-83b6-350f-eb453ab214ae@softathome.com> References: <20210224232531.22899-1-farhan.ali@broadcom.com> <20210309235530.184179-1-farhan.ali@broadcom.com> <389855fe-9401-83b6-350f-eb453ab214ae@softathome.com> Message-ID: <96f54554-b201-12c4-287b-794efaff19e9@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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