[PATCH] i.MX: hab: write srk lock with hab command

[PATCH] i.MX: hab: write srk lock with hab command

[Fabian Pflug]

The write_srk_hash functions already support the flag to write the SRK
lock, but it is never used in barebox. To prevent an attacker from
calculating an SRK hash that has the same bits set as the current SRK
hash, but with maybe more, we lock the SRK hash to prevent turning bits.

Writing the lock twice will probably result in unusable garbage and the
hab command itself already is written in a way to write the complete
hash and not parts of it.

Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
---
 commands/hab.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/commands/hab.c b/commands/hab.c
index 8ae943a4c8..b8ef770066 100644
--- a/commands/hab.c
+++ b/commands/hab.c
@@ -14,7 +14,7 @@ static int do_hab(int argc, char *argv[])
 {
 	int opt, ret, i;
 	char *srkhashfile = NULL, *srkhash = NULL;
-	unsigned flags = 0;
+	unsigned flags = IMX_SRK_HASH_WRITE_LOCK;
 	u8 srk[SRK_HASH_SIZE];
 	int lockdown = 0, info = 0;
 
-- 
2.47.3

Re: [PATCH] i.MX: hab: write srk lock with hab command

[Marco Felsch]

Hi Fabian,

On 26-03-06, Fabian Pflug wrote:
> The write_srk_hash functions already support the flag to write the SRK
> lock, but it is never used in barebox. To prevent an attacker from
> calculating an SRK hash that has the same bits set as the current SRK
> hash, but with maybe more, we lock the SRK hash to prevent turning bits.
> 
> Writing the lock twice will probably result in unusable garbage and the
> hab command itself already is written in a way to write the complete
> hash and not parts of it.
> 
> Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
> ---
>  commands/hab.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/commands/hab.c b/commands/hab.c
> index 8ae943a4c8..b8ef770066 100644
> --- a/commands/hab.c
> +++ b/commands/hab.c
> @@ -14,7 +14,7 @@ static int do_hab(int argc, char *argv[])
>  {
>  	int opt, ret, i;
>  	char *srkhashfile = NULL, *srkhash = NULL;
> -	unsigned flags = 0;
> +	unsigned flags = IMX_SRK_HASH_WRITE_LOCK;

This would fix only the hab cmd, not the C-API. Instead we should fix
the C-API to write the LOCK after the SRK was burned/fused.

Regards,
  Marco

>  	u8 srk[SRK_HASH_SIZE];
>  	int lockdown = 0, info = 0;
>  
> -- 
> 2.47.3
> 
> 
> 

-- 
#gernperDu 
#CallMeByMyFirstName

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

Re: [PATCH] i.MX: hab: write srk lock with hab command

[Fabian Pflug]

Hello Marco,

the C APi already does it, if you give it this flag.

It is device dependen on which fuse to burn, so have a look at:


static int imx6_hab_write_srk_hash_ocotp(const u8 *newsrk, unsigned flags)
{
	int ret;

	ret = imx_hab_write_srk_hash_ocotp(newsrk);
	if (ret)
		return ret;

	if (flags & IMX_SRK_HASH_WRITE_LOCK) {
		ret = imx_ocotp_write_field(OCOTP_SRK_LOCK, 1);
		if (ret < 0)
			return ret;
	}

	return 0;
}

static int imx8m_hab_write_srk_hash_ocotp(const u8 *newsrk, unsigned flags)
{
	int ret;

	ret = imx_hab_write_srk_hash_ocotp(newsrk);
	if (ret)
		return ret;

	if (flags & IMX_SRK_HASH_WRITE_LOCK) {
		ret = imx_ocotp_write_field(MX8M_OCOTP_SRK_LOCK, 1);
		if (ret < 0)
			return ret;
	}

	return 0;
}

which get called by 
int imx_hab_write_srk_hash(const void *buf, unsigned flags)
Which is the C API.

I don't believe it is good to always write the lock bit in the C API, as this could be used to write partial hashes.

Kind regards
Fabian

On Fri, 2026-03-06 at 11:26 +0100, Marco Felsch wrote:
> Hi Fabian,
> 
> On 26-03-06, Fabian Pflug wrote:
> > The write_srk_hash functions already support the flag to write the SRK
> > lock, but it is never used in barebox. To prevent an attacker from
> > calculating an SRK hash that has the same bits set as the current SRK
> > hash, but with maybe more, we lock the SRK hash to prevent turning bits.
> > 
> > Writing the lock twice will probably result in unusable garbage and the
> > hab command itself already is written in a way to write the complete
> > hash and not parts of it.
> > 
> > Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
> > ---
> >  commands/hab.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/commands/hab.c b/commands/hab.c
> > index 8ae943a4c8..b8ef770066 100644
> > --- a/commands/hab.c
> > +++ b/commands/hab.c
> > @@ -14,7 +14,7 @@ static int do_hab(int argc, char *argv[])
> >  {
> >  	int opt, ret, i;
> >  	char *srkhashfile = NULL, *srkhash = NULL;
> > -	unsigned flags = 0;
> > +	unsigned flags = IMX_SRK_HASH_WRITE_LOCK;
> 
> This would fix only the hab cmd, not the C-API. Instead we should fix
> the C-API to write the LOCK after the SRK was burned/fused.
> 
> Regards,
>   Marco
> 
> >  	u8 srk[SRK_HASH_SIZE];
> >  	int lockdown = 0, info = 0;
> >  
> > -- 
> > 2.47.3
> > 
> > 
> >