[U-Boot] [PATCH 0/4] imx: hab: Add helper functions for scripted HAB auth
Greetings.
This set adds some helper functions as a pre-cursor to an upcoming set of changes to a BSP adding scripted HAB authentication.
Calculating a HAB IVT address based on a base address and a +/- offset is a trivial but, useful function for HAB. It means you can have a load address for a HAB image inside of your environment and specify the IVT offset relative to that address. All you need to do then is to call the function to obtain the correct IVT address to pass into hab_auth_img.
Two relatively minor changes then - one encasing the hab.h in ifndef __ASSEMBLY__ which is required if you want to include hab.h in a board.h.
Specifying the IVT padding size is again properly done as a define as opposed to a magic number in code.
The final patch then is wrappering up two common use-cases in the upcoming BSP - hab_auth_image ? continue-to-boot : drop-to-bootrom USB mode.
In other words if you fail to authenticate an image on the secure-boot path the appropriate next step is typically to drop into USB recovery mode.
In USB recovery mode you need to provide a signed image on a secure-boot (closed in the parlance) board. So hab_auth_img_or_fail() encapsulates that behaviour in one place - again allowing for scripting to reuse instead of replicate functionality over and over again.
These helper functions could all be buried in the board-port but, they are made available here in the hopes they will be of use to others.
Bryan O'Donoghue (4): imx: hab: Add routine to set HAB IVT address imx: hab: Encase majority of header in __ASSEMBLY__ declaration imx: hab: Specify IVT padding size imx: hab: Provide hab_auth_img_or_fail command
arch/arm/include/asm/mach-imx/hab.h | 9 ++++-- arch/arm/mach-imx/hab.c | 59 +++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-)
This patch takes a given address applies a plus or minus offset to locate the putative address of an IVT given a non-IVT link location.
It then sets hab_ivt_address to allow for further logic/scripting based on the calculated address.
This routine is useful when scripting hab_auth_img calls from boot.scr. Subsequent patches will illustrate its utility in a board-port.
Signed-off-by: Bryan O'Donoghue bryan.odonoghue@linaro.org Cc: Utkarsh Gupta utkarsh.gupta@nxp.com Cc: Breno Lima breno.lima@nxp.com Cc: Fabio Estevam fabio.estevam@nxp.com --- arch/arm/mach-imx/hab.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c index c730c8f..0c18b2e 100644 --- a/arch/arm/mach-imx/hab.c +++ b/arch/arm/mach-imx/hab.c @@ -341,6 +341,31 @@ static int do_hab_failsafe(cmd_tbl_t *cmdtp, int flag, int argc, return 0; }
+/* + * This routine takes a given address and applies a plus or minus offset to that + * address. + */ +static int do_hab_get_ivt_addr(cmd_tbl_t *cmdtp, int flag, int argc, + char * const argv[]) +{ + ulong addr; + long ivt_offset; + + if (argc < 3) + return CMD_RET_USAGE; + + if (!imx_hab_is_enabled()) + return CMD_RET_FAILURE; + + addr = simple_strtoul(argv[1], NULL, 16); + ivt_offset = simple_strtol(argv[2], NULL, 16); + addr += ivt_offset; + + env_set_hex("hab_ivt_addr", addr); + + return CMD_RET_SUCCESS; +} + U_BOOT_CMD( hab_status, CONFIG_SYS_MAXARGS, 1, do_hab_status, "display HAB status", @@ -362,6 +387,14 @@ U_BOOT_CMD( "" );
+U_BOOT_CMD( + hab_get_ivt_addr, 3, 0, do_hab_get_ivt_addr, + "determine IVT header location and store in $hab_ivt_addr", + "addr ivt_offset\n" + "addr - image hex address\n" + "ivt_offset - hex offset of IVT in the image" + ); + #endif /* !defined(CONFIG_SPL_BUILD) */
/* Get CSF Header length */
Hi Bryan,
2018-03-09 14:35 GMT-03:00 Bryan O'Donoghue bryan.odonoghue@linaro.org:
This patch takes a given address applies a plus or minus offset to locate the putative address of an IVT given a non-IVT link location.
It then sets hab_ivt_address to allow for further logic/scripting based on the calculated address.
This routine is useful when scripting hab_auth_img calls from boot.scr. Subsequent patches will illustrate its utility in a board-port.
Signed-off-by: Bryan O'Donoghue bryan.odonoghue@linaro.org Cc: Utkarsh Gupta utkarsh.gupta@nxp.com Cc: Breno Lima breno.lima@nxp.com Cc: Fabio Estevam fabio.estevam@nxp.com
Tested-by: breno.lima@nxp.com
Thanks, Breno Lima
Hi,
On Fri, 9 Mar 2018 17:35:46 +0000 Bryan O'Donoghue wrote:
This patch takes a given address applies a plus or minus offset to locate the putative address of an IVT given a non-IVT link location.
It then sets hab_ivt_address to allow for further logic/scripting based on the calculated address.
This routine is useful when scripting hab_auth_img calls from boot.scr. Subsequent patches will illustrate its utility in a board-port.
Signed-off-by: Bryan O'Donoghue bryan.odonoghue@linaro.org Cc: Utkarsh Gupta utkarsh.gupta@nxp.com Cc: Breno Lima breno.lima@nxp.com Cc: Fabio Estevam fabio.estevam@nxp.com
arch/arm/mach-imx/hab.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c index c730c8f..0c18b2e 100644 --- a/arch/arm/mach-imx/hab.c +++ b/arch/arm/mach-imx/hab.c @@ -341,6 +341,31 @@ static int do_hab_failsafe(cmd_tbl_t *cmdtp, int flag, int argc, return 0; }
+/*
- This routine takes a given address and applies a plus or minus offset to that
- address.
- */
+static int do_hab_get_ivt_addr(cmd_tbl_t *cmdtp, int flag, int argc,
char * const argv[])+{
- ulong addr;
- long ivt_offset;
- if (argc < 3)
return CMD_RET_USAGE;- if (!imx_hab_is_enabled())
return CMD_RET_FAILURE;- addr = simple_strtoul(argv[1], NULL, 16);
- ivt_offset = simple_strtol(argv[2], NULL, 16);
- addr += ivt_offset;
- env_set_hex("hab_ivt_addr", addr);
- return CMD_RET_SUCCESS;
+}
U_BOOT_CMD( hab_status, CONFIG_SYS_MAXARGS, 1, do_hab_status, "display HAB status",
What does this function offer, that a 'setexpr hab_ivt_addr ${loadaddr} + 0x400' could not do as well?
Lothar Waßmann
Subsequent patches will want to include hab.h but in doing so include it on an assembly compile path causing a range of compile errors. Fix the errors pre-emptively by encasing the majority of the declarations in hab.h inside an ifdef __ASSEMBLY__ block.
Signed-off-by: Bryan O'Donoghue bryan.odonoghue@linaro.org Cc: Utkarsh Gupta utkarsh.gupta@nxp.com Cc: Breno Lima breno.lima@nxp.com Cc: Fabio Estevam fabio.estevam@nxp.com --- arch/arm/include/asm/mach-imx/hab.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h index ce9a44d..1bebdbe 100644 --- a/arch/arm/include/asm/mach-imx/hab.h +++ b/arch/arm/include/asm/mach-imx/hab.h @@ -8,6 +8,7 @@ #ifndef __SECURE_MX6Q_H__ #define __SECURE_MX6Q_H__
+#ifndef __ASSEMBLY__ #include <linux/types.h> #include <linux/compiler.h>
@@ -196,13 +197,14 @@ typedef void hapi_clock_init_t(void); #define HAB_CMD_SET 0xB1 /* Set command tag */ #define HAB_PAR_MID 0x01 /* MID parameter value */
-#define IVT_SIZE 0x20 -#define CSF_PAD_SIZE 0x2000 - /* ----------- end of HAB API updates ------------*/
int imx_hab_authenticate_image(uint32_t ddr_start, uint32_t image_size, uint32_t ivt_offset); bool imx_hab_is_enabled(void); +#endif /* __ASSEMBLY__ */ + +#define IVT_SIZE 0x20 +#define CSF_PAD_SIZE 0x2000
#endif
Hi Bryan,
2018-03-09 14:35 GMT-03:00 Bryan O'Donoghue bryan.odonoghue@linaro.org:
Subsequent patches will want to include hab.h but in doing so include it on an assembly compile path causing a range of compile errors. Fix the errors pre-emptively by encasing the majority of the declarations in hab.h inside an ifdef __ASSEMBLY__ block.
Signed-off-by: Bryan O'Donoghue bryan.odonoghue@linaro.org Cc: Utkarsh Gupta utkarsh.gupta@nxp.com Cc: Breno Lima breno.lima@nxp.com Cc: Fabio Estevam fabio.estevam@nxp.com
Tested-by: breno.lima@nxp.com
Thanks, Breno Lima
This patch adds IVT_PAD_SIZE at 0xC00. The IVT header is padded to this size. Defining the size explicitly makes it possible to use the define to locate the start/end of an IVT header without using magic numbers in code.
Signed-off-by: Bryan O'Donoghue bryan.odonoghue@linaro.org Cc: Utkarsh Gupta utkarsh.gupta@nxp.com Cc: Breno Lima breno.lima@nxp.com Cc: Fabio Estevam fabio.estevam@nxp.com --- arch/arm/include/asm/mach-imx/hab.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h index 1bebdbe..f903e3e 100644 --- a/arch/arm/include/asm/mach-imx/hab.h +++ b/arch/arm/include/asm/mach-imx/hab.h @@ -205,6 +205,7 @@ bool imx_hab_is_enabled(void); #endif /* __ASSEMBLY__ */
#define IVT_SIZE 0x20 +#define IVT_PAD_SIZE 0xC00 #define CSF_PAD_SIZE 0x2000
#endif
Hi Bryan,
2018-03-09 14:35 GMT-03:00 Bryan O'Donoghue bryan.odonoghue@linaro.org:
This patch adds IVT_PAD_SIZE at 0xC00. The IVT header is padded to this size. Defining the size explicitly makes it possible to use the define to locate the start/end of an IVT header without using magic numbers in code.
As far as I know the 0xC00 pad size is only mandatory in U-Boot/SPL images, for instance in some U-Boot images the first 0xC00 should include IVT + Boot data + DCD table + padding to 0xC00. Are you using the IVT_PAD_SIZE in your current code? Or this macro will be used in a next series?
Thanks, Breno Lima
On 15/03/18 16:54, Breno Matheus Lima wrote:
Hi Bryan,
2018-03-09 14:35 GMT-03:00 Bryan O'Donoghue bryan.odonoghue@linaro.org:
This patch adds IVT_PAD_SIZE at 0xC00. The IVT header is padded to this size. Defining the size explicitly makes it possible to use the define to locate the start/end of an IVT header without using magic numbers in code.
As far as I know the 0xC00 pad size is only mandatory in U-Boot/SPL images, for instance in some U-Boot images the first 0xC00 should include IVT + Boot data + DCD table + padding to 0xC00. Are you using the IVT_PAD_SIZE in your current code? Or this macro will be used in a next series?
Thanks, Breno Lima
All of my images - kernel, u-boot, optee, boot.scr are signed in the same first-stage format with this padding present - for simplicity.
Maybe this better named "BOOTROM_IVT_PAD_SIZE" since only the bootrom requires it.
Hi Bryan,
2018-03-17 8:06 GMT-03:00 Bryan O'Donoghue bryan.odonoghue@linaro.org:
On 15/03/18 16:54, Breno Matheus Lima wrote:
Hi Bryan,
2018-03-09 14:35 GMT-03:00 Bryan O'Donoghue bryan.odonoghue@linaro.org:
This patch adds IVT_PAD_SIZE at 0xC00. The IVT header is padded to this size. Defining the size explicitly makes it possible to use the define to locate the start/end of an IVT header without using magic numbers in code.
As far as I know the 0xC00 pad size is only mandatory in U-Boot/SPL images, for instance in some U-Boot images the first 0xC00 should include IVT + Boot data + DCD table + padding to 0xC00. Are you using the IVT_PAD_SIZE in your current code? Or this macro will be used in a next series?
Thanks, Breno Lima
All of my images - kernel, u-boot, optee, boot.scr are signed in the same first-stage format with this padding present - for simplicity.
Maybe this better named "BOOTROM_IVT_PAD_SIZE" since only the bootrom requires it.
As far as I know the IVT has a fixed size of 0x20, and the set of IVT + Boot Data + DCD cannot exceed 0xC00.
This value is calculated by the following operation in function imximage_generate() at tools/imximage.c: alloc_len = imximage_init_loadsize - imximage_ivt_offset;
Maybe we can rename to something like UBOOT_IMX_HDR_SIZE? We also have to ensure this definition is being used by U-Boot code.
Thanks, Breno Lima
On 20/03/18 01:53, Breno Matheus Lima wrote:
Hi Bryan,
2018-03-17 8:06 GMT-03:00 Bryan O'Donoghue bryan.odonoghue@linaro.org:
On 15/03/18 16:54, Breno Matheus Lima wrote:
Hi Bryan,
2018-03-09 14:35 GMT-03:00 Bryan O'Donoghue bryan.odonoghue@linaro.org:
This patch adds IVT_PAD_SIZE at 0xC00. The IVT header is padded to this size. Defining the size explicitly makes it possible to use the define to locate the start/end of an IVT header without using magic numbers in code.
As far as I know the 0xC00 pad size is only mandatory in U-Boot/SPL images, for instance in some U-Boot images the first 0xC00 should include IVT + Boot data + DCD table + padding to 0xC00. Are you using the IVT_PAD_SIZE in your current code? Or this macro will be used in a next series?
Thanks, Breno Lima
All of my images - kernel, u-boot, optee, boot.scr are signed in the same first-stage format with this padding present - for simplicity.
Maybe this better named "BOOTROM_IVT_PAD_SIZE" since only the bootrom requires it.
As far as I know the IVT has a fixed size of 0x20, and the set of IVT
- Boot Data + DCD cannot exceed 0xC00.
This value is calculated by the following operation in function imximage_generate() at tools/imximage.c: alloc_len = imximage_init_loadsize - imximage_ivt_offset;
Maybe we can rename to something like UBOOT_IMX_HDR_SIZE? We also have to ensure this definition is being used by U-Boot code.
Thanks, Breno Lima
So.
I've stupidly called this "PAD_SIZE" and we've gone off on a padding size discussion.
This define is supposed to represent the IVT _offset_ in that initial BootROM image.
Note to self: the hint is in the variable "setenv ivt_offset=some_number"
Anyway Breno - we could add padding size checks to images but, _this_ define is related to IVT offset so... my bad.
I'll redo this patch with a better name :(
This patch adds hab_auth_img_or_fail() a command line function that encapsulates a common usage of authenticate and failover, namely if authenticate image fails, then drop to BootROM USB recovery mode.
For secure-boot systems, this type of locked down behavior is important to ensure no unsigned images can be run.
It's possible to script this logic but, when done over and over again the environment starts get very complex and repetitive, reducing that script repetition down to a command line function makes sense.
Signed-off-by: Bryan O'Donoghue bryan.odonoghue@linaro.org Cc: Utkarsh Gupta utkarsh.gupta@nxp.com Cc: Breno Lima breno.lima@nxp.com Cc: Fabio Estevam fabio.estevam@nxp.com --- arch/arm/mach-imx/hab.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c index 0c18b2e..61ccdeb 100644 --- a/arch/arm/mach-imx/hab.c +++ b/arch/arm/mach-imx/hab.c @@ -366,6 +366,22 @@ static int do_hab_get_ivt_addr(cmd_tbl_t *cmdtp, int flag, int argc, return CMD_RET_SUCCESS; }
+static int do_authenticate_image_or_failover(cmd_tbl_t *cmdtp, int flag, + int argc, char * const argv[]) +{ + if (!imx_hab_is_enabled()) + goto done; + + if (do_authenticate_image(NULL, flag, argc, argv) != CMD_RET_SUCCESS) { + fprintf(stderr, "authentication fail -> %s %s %s %s\n", + argv[0], argv[1], argv[2], argv[3]); + do_hab_failsafe(0, 0, 1, NULL); + }; + +done: + return CMD_RET_SUCCESS; +} + U_BOOT_CMD( hab_status, CONFIG_SYS_MAXARGS, 1, do_hab_status, "display HAB status", @@ -395,6 +411,16 @@ U_BOOT_CMD( "ivt_offset - hex offset of IVT in the image" );
+U_BOOT_CMD( + hab_auth_img_or_fail, 4, 0, + do_authenticate_image_or_failover, + "authenticate image via HAB on failure drop to USB BootROM mode", + "addr length ivt_offset\n" + "addr - image hex address\n" + "length - image hex length\n" + "ivt_offset - hex offset of IVT in the image" + ); + #endif /* !defined(CONFIG_SPL_BUILD) */
/* Get CSF Header length */
Hi Bryan,
2018-03-09 14:35 GMT-03:00 Bryan O'Donoghue bryan.odonoghue@linaro.org:
This patch adds hab_auth_img_or_fail() a command line function that encapsulates a common usage of authenticate and failover, namely if authenticate image fails, then drop to BootROM USB recovery mode.
For secure-boot systems, this type of locked down behavior is important to ensure no unsigned images can be run.
It's possible to script this logic but, when done over and over again the environment starts get very complex and repetitive, reducing that script repetition down to a command line function makes sense.
Signed-off-by: Bryan O'Donoghue bryan.odonoghue@linaro.org Cc: Utkarsh Gupta utkarsh.gupta@nxp.com Cc: Breno Lima breno.lima@nxp.com Cc: Fabio Estevam fabio.estevam@nxp.com
arch/arm/mach-imx/hab.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c index 0c18b2e..61ccdeb 100644 --- a/arch/arm/mach-imx/hab.c +++ b/arch/arm/mach-imx/hab.c @@ -366,6 +366,22 @@ static int do_hab_get_ivt_addr(cmd_tbl_t *cmdtp, int flag, int argc, return CMD_RET_SUCCESS; }
+static int do_authenticate_image_or_failover(cmd_tbl_t *cmdtp, int flag,
int argc, char * const argv[])+{
if (!imx_hab_is_enabled())goto done;
It would be nice to return CMD_RET_USAGE on this case, or maybe print something like "Secure boot disabled". If I run in a non HAB enabled board I get the following output:
=> hab_auth_img_or_fail <addr> <length> <ivt_offset> =>
We may also need to add the following here:
if (argc < 4) return CMD_RET_USAGE;
If I run this command without any parameter the code is wrongly executed, and the system goes to USB recovery mode.
Thanks, Breno Lima
On 15/03/18 17:15, Breno Matheus Lima wrote:
If I run this command without any parameter the code is wrongly executed, and the system goes to USB recovery mode.
Oops.
I'll fix that so.
--- bod
participants (3)
-
Breno Matheus Lima -
Bryan O'Donoghue -
Lothar Waßmann