Re: [PATCH 08/13] ARM: am625: support hash verification of full barebox

[Sascha Hauer <s.hauer@pengutronix.de>]

On Mon, Mar 10, 2025 at 08:22:26PM +0100, Marco Felsch wrote:
> On 25-02-28, Sascha Hauer wrote:
> > This implements the necessary SoC code to check the full barebox against
> > a sha256 compiled into the first stage barebox.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  arch/arm/mach-k3/Kconfig |  1 +
> >  arch/arm/mach-k3/r5.c    | 14 ++++++++++++++
> >  2 files changed, 15 insertions(+)
> > 
> > diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
> > index 50919dc7e3..561ad1dac4 100644
> > --- a/arch/arm/mach-k3/Kconfig
> > +++ b/arch/arm/mach-k3/Kconfig
> > @@ -16,6 +16,7 @@ config MACH_K3_CORTEX_R5
> >  	select ELF
> >  	select K3_DDRSS
> >  	select FIP
> > +	select HAVE_FIRMWARE_VERIFY_NEXT_IMAGE
> >  	depends on 32BIT
> >  	select ARM_USE_COMPRESSED_DTB
> >  	default y
> > diff --git a/arch/arm/mach-k3/r5.c b/arch/arm/mach-k3/r5.c
> > index e12c888afa..cb52ff364d 100644
> > --- a/arch/arm/mach-k3/r5.c
> > +++ b/arch/arm/mach-k3/r5.c
> > @@ -248,6 +248,8 @@ static int load_fip(const char *filename, off_t offset)
> >  {
> >  	struct fip_state *fip;
> >  	struct fip_image_desc *desc;
> > +	unsigned char shasum[SHA256_DIGEST_SIZE];
> > +	int ret;
> >  
> >  	fip = fip_image_open(filename, offset);
> >  	if (IS_ERR(fip)) {
> > @@ -255,6 +257,18 @@ static int load_fip(const char *filename, off_t offset)
> >  		return PTR_ERR(fip);
> >  	}
> >  
> > +	if (IS_ENABLED(CONFIG_FIRMWARE_VERIFY_NEXT_IMAGE)) {
> > +		ret = fip_sha256(fip, shasum);
> > +		if (ret) {
> > +			pr_err("Cannot calc fip sha256: %pe\n", ERR_PTR(ret));
> > +			return ret;
> > +		}
> > +
> > +		ret = firmware_next_image_verify(shasum, SHA256_DIGEST_SIZE, true);
> > +		if (ret)
> > +			return ret;
> 
> Albeit it would involve way more effort, I would like to see that the
> FIP image format does have support for signatures within their "struct
> image_desc" for each image.
> This way it would be far easier for us to verify each image separately
> and in a common way. Also it wouldn't require to rebuild the "r5"
> tiboot3.bin to include the the updated sha256sum each time.

Having to rebuild the tiboot3.bin for the updated sha256sum is not a
downside in this case, but actually the feature I wanted to implement.

Using a hash avoids mix-and-match attacks between different 1st stage
images combined with different 2nd stage images.

So yes, using the FIP image signing mechanisms would be nice to have,
but doesn't meet my goal.

> Also the shasum size seems like the user would have a choice to choose
> other sha-sums which he hasn't, therefore I would drop it.

ok.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |