Re: [PATCH v2 7/9] i.MX8M: HABv4: add an option to allow key revocation

[Marco Felsch <m.felsch@pengutronix.de>]

On 24-07-30, Ahmad Fatoum wrote:
> Hello Marco,
> 
> On 7/4/24 10:15, Marco Felsch wrote:
> > Hi Ahmad,
> > 
> > On 24-07-03, Ahmad Fatoum wrote:
> >> Hello Marco,
> >>
> >> On 03.07.24 19:20, Marco Felsch wrote:
> >>> The HAB code needs an special [Unlock] instruction to keep the
> >>> SRK_REVOKE fuse bank unlocked. This is required if a key needs to be
> >>> revoked.
> >>>
> >>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> >>> ---
> >>>  arch/arm/mach-imx/Kconfig            | 8 ++++++++
> >>>  include/mach/imx/habv4-imx8-gencsf.h | 6 ++++++
> >>>  2 files changed, 14 insertions(+)
> >>>
> >>> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> >>> index 61258137736f..68f55971506b 100644
> >>> --- a/arch/arm/mach-imx/Kconfig
> >>> +++ b/arch/arm/mach-imx/Kconfig
> >>> @@ -835,6 +835,14 @@ config HABV4_QSPI
> >>>  	help
> >>>  	  Enable this option to build signed QSPI/FlexSPI images.
> >>>  
> >>> +config HABV4_CSF_UNLOCK_SRK_REVOKE
> >>> +	depends on HABV4
> >>> +	bool "Unlock SRK revocation"
> >>> +	help
> >>> +	  Enable this option to instruct the HAB code to not lock
> >>> +	  the SRK_REVOKE_LOCK sticky bit. This is required for key
> >>> +	  revocation. Don't enable this if you are unsure.
> >>
> >> I think for added safety we should have an extra option that prompts
> >> for the key to be revoked and an initcall that is activated depending
> >> on it, e.g.:
> >>
> >> config HABV4_CSF_SRK_REVOKE_INDEX
> >>         int "SRK to revoke"
> >>         range 0 3
> >>         default 0
> >>         depends on HABV4_CSF_SRK_REVOKE_UNLOCK
> >>         help
> >>           Which of the first three SRKs to revoke. The SRK indices are
> >>           1-based. Saying 0 here will just print the SRK Revocation
> >>           register without modification. SRK #4 is immutable.
> >>                                                                        
> >>           Proceed with caution, revoking a SRK is irreversible and
> >>           manual manipulation of this code can brick the board!
> >>                                                                        
> >> if HABV4_CSF_SRK_REVOKE_INDEX = HABV4_SRK_INDEX
> >> comment "Can't revoke same SRK used for signing"
> >> comment "Attempts to build a signed barebox image will fail"
> >> endif
> >>
> >> and then some code that checks the same above condition during final
> >> assembly of the signed image.
> >>
> >> What do you think?
> > 
> > That's an good idea to make it more user-friendly for most users :)
> > Regarding this patchset I do see it more as an addition since for my
> > project the revocation is checked on every startup and we do allow the
> > revocation of multiple SRK slots at the same time.
> 
> Ok. I just have a single nit then, can you rename
> HABV4_CSF_UNLOCK_SRK_REVOKE to HABV4_CSF_SRK_REVOKE_UNLOCK, so we can
> add HABV4_CSF_SRK_REVOKE_INDEX in future with the same prefix?

Sure, I can do this.

Regards,
  Marco

> 
> Thanks,
> Ahmad
> 
> > 
> > Regards,
> >   Marco
> > 
> >>>  config HAB_CERTS_ENV
> >>>  	depends on HAB
> >>>  	bool "Specify certificates in environment"
> >>> diff --git a/include/mach/imx/habv4-imx8-gencsf.h b/include/mach/imx/habv4-imx8-gencsf.h
> >>> index 5f92ceceab00..56d9ef2de92f 100644
> >>> --- a/include/mach/imx/habv4-imx8-gencsf.h
> >>> +++ b/include/mach/imx/habv4-imx8-gencsf.h
> >>> @@ -36,6 +36,12 @@ hab [Unlock]
> >>>  hab Engine = CAAM
> >>>  hab Features = RNG, MID
> >>>  
> >>> +#if defined(CONFIG_HABV4_CSF_UNLOCK_SRK_REVOKE)
> >>> +hab [Unlock]
> >>> +hab Engine = OCOTP
> >>> +hab Features = SRK REVOKE
> >>> +#endif
> >>> +
> >>>  hab [Install Key]
> >>>  /* verification key index in key store (0, 2...4) */
> >>>  hab Verification index = 0
> >>>
> >>
> >> -- 
> >> 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 |
> >>
> >>
> > 
>