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@xxxxxxxxxxxxxx> > >>> --- > >>> 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 | > >> > >> > > >
