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? 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 | >> >> >
