[U-Boot] [PATCH v2] imx: hab: extend hab_auth_img to calculate ivt

Discussion:

[U-Boot] [PATCH v2] imx: hab: extend hab_auth_img to calculate ivt_offset

Parthiban Nallathambi

2018-11-21 13:50:40 UTC

Current implementation of hab_auth_img command needs ivt_offset to
authenticate the image. But ivt header is placed at the end of image
date after padding.

This leaves the usage of hab_auth_img command to fixed size or static
offset for ivt header. New function "get_image_ivt_offset" is introduced
to find the ivt offset during runtime. The case conditional check in this
function is same as boot_get_kernel in common/bootm.c

With this variable length image e.g. FIT image with any random size can
have IVT at the end and ivt_offset option can be left optional

Can be used as "hab_auth_img $loadaddr $filesize" from u-boot script

Signed-off-by: Parthiban Nallathambi <***@denx.de>
---

Notes:
Changelog in v2:
- Finding IVT offset doesn't need length. Removed the
length argument from get_image_ivt_offset

arch/arm/mach-imx/hab.c | 29 +++++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index b88acd13da..dbfd692fa3 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -6,6 +6,8 @@
#include <common.h>
#include <config.h>
#include <fuse.h>
+#include <mapmem.h>
+#include <image.h>
#include <asm/io.h>
#include <asm/system.h>
#include <asm/arch/clock.h>
@@ -302,18 +304,41 @@ static int do_hab_status(cmd_tbl_t *cmdtp, int flag, int argc,
return 0;
}

+static ulong get_image_ivt_offset(ulong img_addr)
+{
+ const void *buf;
+
+ buf = map_sysmem(img_addr, 0);
+ switch (genimg_get_format(buf)) {
+#if defined(CONFIG_IMAGE_FORMAT_LEGACY)
+ case IMAGE_FORMAT_LEGACY:
+ return (image_get_image_size((image_header_t *)img_addr)
+ + 0x1000 - 1) & ~(0x1000 - 1);
+#endif
+#if IMAGE_ENABLE_FIT
+ case IMAGE_FORMAT_FIT:
+ return (fit_get_size(buf) + 0x1000 - 1) & ~(0x1000 - 1);
+#endif
+ default:
+ return 0;
+ }
+}
+
static int do_authenticate_image(cmd_tbl_t *cmdtp, int flag, int argc,
char * const argv[])
{
ulong addr, length, ivt_offset;
int rcode = 0;

- if (argc < 4)
+ if (argc < 3)
return CMD_RET_USAGE;

addr = simple_strtoul(argv[1], NULL, 16);
length = simple_strtoul(argv[2], NULL, 16);
- ivt_offset = simple_strtoul(argv[3], NULL, 16);
+ if (argc == 3)
+ ivt_offset = get_image_ivt_offset(addr);
+ else
+ ivt_offset = simple_strtoul(argv[3], NULL, 16);

rcode = imx_hab_authenticate_image(addr, length, ivt_offset);
if (rcode == 0)

--
2.17.2

Breno Matheus Lima

2018-11-21 16:45:19 UTC

Permalink

Hi Parthiban,

Post by Parthiban Nallathambi
Current implementation of hab_auth_img command needs ivt_offset to
authenticate the image. But ivt header is placed at the end of image
date after padding.
This leaves the usage of hab_auth_img command to fixed size or static
offset for ivt header. New function "get_image_ivt_offset" is introduced
to find the ivt offset during runtime. The case conditional check in this
function is same as boot_get_kernel in common/bootm.c
With this variable length image e.g. FIT image with any random size can
have IVT at the end and ivt_offset option can be left optional
Can be used as "hab_auth_img $loadaddr $filesize" from u-boot script
---
- Finding IVT offset doesn't need length. Removed the
length argument from get_image_ivt_offset
arch/arm/mach-imx/hab.c | 29 +++++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index b88acd13da..dbfd692fa3 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -6,6 +6,8 @@
#include <common.h>
#include <config.h>
#include <fuse.h>
+#include <mapmem.h>
+#include <image.h>
#include <asm/io.h>
#include <asm/system.h>
#include <asm/arch/clock.h>
@@ -302,18 +304,41 @@ static int do_hab_status(cmd_tbl_t *cmdtp, int flag, int argc,
return 0;
}
+static ulong get_image_ivt_offset(ulong img_addr)
+{
+ const void *buf;
+
+ buf = map_sysmem(img_addr, 0);
+ switch (genimg_get_format(buf)) {
+#if defined(CONFIG_IMAGE_FORMAT_LEGACY)
+ return (image_get_image_size((image_header_t *)img_addr)
+ + 0x1000 - 1) & ~(0x1000 - 1);
+#endif
+#if IMAGE_ENABLE_FIT
+ return (fit_get_size(buf) + 0x1000 - 1) & ~(0x1000 - 1);
+#endif
+ return 0;
+ }
+}

Do you have more details about the image header I should use here?

I'm trying to get the IVT offset for my Kernel image based on NXP
4.9.11_2.0.0_GA Linux release loaded at 0x80800000:

=> fatload mmc 0:1 0x80800000 zImage
=> hab_auth_img 0x80800000 <Len not being used>

The zImage size in Header looks correct (0x00726690), but
get_image_ivt_offset() is returning 0x0

$ hexdump zImage | head
0000000 0000 e1a0 0000 e1a0 0000 e1a0 0000 e1a0
*
0000020 0003 ea00 2818 016f 0000 0000 6690 0072
0000030 0201 0403 9000 e10f 04f8 eb00 7001 e1a0
0000040 8002 e1a0 2000 e10f 0003 e312 0001 1a00

Seems that genimg_get_format() is returning 0x0.

Any ideias if I'm missing something?

Post by Parthiban Nallathambi
+
static int do_authenticate_image(cmd_tbl_t *cmdtp, int flag, int argc,
char * const argv[])
{
ulong addr, length, ivt_offset;
int rcode = 0;
- if (argc < 4)
+ if (argc < 3)

I think we can also change here to argc < 2, the function
get_image_ivt_offset() only requires the img addr now.

Post by Parthiban Nallathambi
return CMD_RET_USAGE;
addr = simple_strtoul(argv[1], NULL, 16);
length = simple_strtoul(argv[2], NULL, 16);
- ivt_offset = simple_strtoul(argv[3], NULL, 16);
+ if (argc == 3)

argc ==2

Parthiban Nallathambi

2018-11-21 17:52:21 UTC

Permalink

Hi Breno,

Post by Breno Matheus Lima
Hi Parthiban,

Do you have more details about the image header I should use here?

Is the image signed using CST or similar tool? Is so, the signature data
(HAB data: CSF, Certs and signature) pads at the end of the kernel
image.

Post by Breno Matheus Lima
I'm trying to get the IVT offset for my Kernel image based on NXP
=> fatload mmc 0:1 0x80800000 zImage
=> hab_auth_img 0x80800000 <Len not being used>

Length for hab_auth_img is still mandatory. Length hear means the file
size or total length of the image, this is required for the HAB API to
authenticate (HAB_RVT_AUTHENTICATE_IMAGE).

Post by Breno Matheus Lima
The zImage size in Header looks correct (0x00726690), but
get_image_ivt_offset() is returning 0x0

Looks like IVT offset is not found in the image.

Post by Breno Matheus Lima
$ hexdump zImage | head
0000000 0000 e1a0 0000 e1a0 0000 e1a0 0000 e1a0
*
0000020 0003 ea00 2818 016f 0000 0000 6690 0072
0000030 0201 0403 9000 e10f 04f8 eb00 7001 e1a0
0000040 8002 e1a0 2000 e10f 0003 e312 0001 1a00
Seems that genimg_get_format() is returning 0x0.

Image is not signed?

Post by Breno Matheus Lima
Any ideias if I'm missing something?

I think we can also change here to argc < 2, the function
get_image_ivt_offset() only requires the img addr now.

No, length is mandatory for authentication. To brief, this patch just
removes the ivt_offset as argument and make it optional. This is needed
because, in images like FIT, the location of the ivt header varies
dynamically depending on the total number of images clubbed.

In such cases, the existing hab_auth_img is hard to use as ivt header
offset needs to be pre-calculated everytime and fed into.

Post by Breno Matheus Lima

argc ==2

No, length is needed as stated above.

--
Thanks,
Parthiban N

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-22 Fax: (+49)-8142-66989-80 Email: ***@denx.de

Breno Matheus Lima

2018-11-21 18:42:21 UTC

Permalink

Hi Parthiban,

Post by Parthiban Nallathambi
Hi Breno,

Post by Breno Matheus Lima
Hi Parthiban,

Do you have more details about the image header I should use here?

Is the image signed using CST or similar tool? Is so, the signature data
(HAB data: CSF, Certs and signature) pads at the end of the kernel
image.

Yes, my Kernel image contains an IVT and is signed with CST. The image
layout looks like link below:
https://pastebin.com/5qEt7ETa

Post by Parthiban Nallathambi

Post by Breno Matheus Lima
I'm trying to get the IVT offset for my Kernel image based on NXP
=> fatload mmc 0:1 0x80800000 zImage
=> hab_auth_img 0x80800000 <Len not being used>

Length for hab_auth_img is still mandatory. Length hear means the file
size or total length of the image, this is required for the HAB API to
authenticate (HAB_RVT_AUTHENTICATE_IMAGE).

Oh ok, I understood the scenario right now.

From my first overview I thought we would add IVT_SIZE + CSF_PAD_SIZE
in ivt_offset to calculate the image length, similar approach we have
in spl.c:
http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/mach-imx/spl.c;h=a20b30d154d788e4ebd4e22e9a6568a4f24c057e;hb=HEAD#l226

In this case only load addr would be necessary.

Post by Parthiban Nallathambi

Post by Breno Matheus Lima
The zImage size in Header looks correct (0x00726690), but
get_image_ivt_offset() is returning 0x0

Looks like IVT offset is not found in the image.

Image is not signed?

No, my zImage is signed.

Post by Parthiban Nallathambi

Post by Breno Matheus Lima
Any ideias if I'm missing something?

I think we can also change here to argc < 2, the function
get_image_ivt_offset() only requires the img addr now.

No, length is mandatory for authentication. To brief, this patch just
removes the ivt_offset as argument and make it optional. This is needed
because, in images like FIT, the location of the ivt header varies
dynamically depending on the total number of images clubbed.
In such cases, the existing hab_auth_img is hard to use as ivt header
offset needs to be pre-calculated everytime and fed into.

Yes I understood your scenario right now, as I stated above I thought
your patch would also calculate the image length as we have in spl.c.

I'm only wondering why genimg_get_format() is returning
IMAGE_FORMAT_INVALID, do you happen to know if zImage header should
return IMAGE_FORMAT_LEGACY?

Thanks,
Breno Lima

Parthiban Nallathambi

2018-11-21 20:47:32 UTC

Permalink

Hi Breno,

Post by Breno Matheus Lima
Hi Parthiban,

Post by Parthiban Nallathambi
Hi Breno,

Post by Breno Matheus Lima
Hi Parthiban,

Do you have more details about the image header I should use here?

Is the image signed using CST or similar tool? Is so, the signature data
(HAB data: CSF, Certs and signature) pads at the end of the kernel
image.

Yes, my Kernel image contains an IVT and is signed with CST. The image
https://pastebin.com/5qEt7ETa

Post by Parthiban Nallathambi

Post by Breno Matheus Lima
I'm trying to get the IVT offset for my Kernel image based on NXP
=> fatload mmc 0:1 0x80800000 zImage
=> hab_auth_img 0x80800000 <Len not being used>

Length for hab_auth_img is still mandatory. Length hear means the file
size or total length of the image, this is required for the HAB API to
authenticate (HAB_RVT_AUTHENTICATE_IMAGE).

Oh ok, I understood the scenario right now.

Post by Parthiban Nallathambi
From my first overview I thought we would add IVT_SIZE + CSF_PAD_SIZE

in ivt_offset to calculate the image length, similar approach we have
http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/mach-imx/spl.c;h=a20b30d154d788e4ebd4e22e9a6568a4f24c057e;hb=HEAD#l226
In this case only load addr would be necessary.

Post by Parthiban Nallathambi

Post by Breno Matheus Lima
The zImage size in Header looks correct (0x00726690), but
get_image_ivt_offset() is returning 0x0

Looks like IVT offset is not found in the image.

Image is not signed?

No, my zImage is signed.

Post by Parthiban Nallathambi

Post by Breno Matheus Lima
Any ideias if I'm missing something?

I think we can also change here to argc < 2, the function
get_image_ivt_offset() only requires the img addr now.

No, length is mandatory for authentication. To brief, this patch just
removes the ivt_offset as argument and make it optional. This is needed
because, in images like FIT, the location of the ivt header varies
dynamically depending on the total number of images clubbed.
In such cases, the existing hab_auth_img is hard to use as ivt header
offset needs to be pre-calculated everytime and fed into.

Yes I understood your scenario right now, as I stated above I thought
your patch would also calculate the image length as we have in spl.c.
I'm only wondering why genimg_get_format() is returning
IMAGE_FORMAT_INVALID, do you happen to know if zImage header should
return IMAGE_FORMAT_LEGACY?

No, LEGACY directly checks for uImage magic "0x27051956". In general,
genimg_get_format handles only uImage, fitimage and android magic alone.

Post by Breno Matheus Lima
Thanks,
Breno Lima

Breno Matheus Lima

2018-11-21 23:53:06 UTC

Permalink

Hi Parthiban,

Post by Parthiban Nallathambi
Hi Breno,

Post by Breno Matheus Lima
Hi Parthiban,

Post by Parthiban Nallathambi
Hi Breno,

Post by Breno Matheus Lima
Hi Parthiban,

Do you have more details about the image header I should use here?

Is the image signed using CST or similar tool? Is so, the signature data
(HAB data: CSF, Certs and signature) pads at the end of the kernel
image.

Yes, my Kernel image contains an IVT and is signed with CST. The image
https://pastebin.com/5qEt7ETa

Post by Parthiban Nallathambi

Post by Breno Matheus Lima
I'm trying to get the IVT offset for my Kernel image based on NXP
=> fatload mmc 0:1 0x80800000 zImage
=> hab_auth_img 0x80800000 <Len not being used>

Length for hab_auth_img is still mandatory. Length hear means the file
size or total length of the image, this is required for the HAB API to
authenticate (HAB_RVT_AUTHENTICATE_IMAGE).

Oh ok, I understood the scenario right now.

Post by Parthiban Nallathambi
From my first overview I thought we would add IVT_SIZE + CSF_PAD_SIZE

Post by Parthiban Nallathambi

Post by Breno Matheus Lima
The zImage size in Header looks correct (0x00726690), but
get_image_ivt_offset() is returning 0x0

Looks like IVT offset is not found in the image.

Image is not signed?

No, my zImage is signed.

Post by Parthiban Nallathambi

Post by Breno Matheus Lima
Any ideias if I'm missing something?

I think we can also change here to argc < 2, the function
get_image_ivt_offset() only requires the img addr now.

No, length is mandatory for authentication. To brief, this patch just
removes the ivt_offset as argument and make it optional. This is needed
because, in images like FIT, the location of the ivt header varies
dynamically depending on the total number of images clubbed.
In such cases, the existing hab_auth_img is hard to use as ivt header
offset needs to be pre-calculated everytime and fed into.

Yes I understood your scenario right now, as I stated above I thought
your patch would also calculate the image length as we have in spl.c.
I'm only wondering why genimg_get_format() is returning
IMAGE_FORMAT_INVALID, do you happen to know if zImage header should
return IMAGE_FORMAT_LEGACY?

No, LEGACY directly checks for uImage magic "0x27051956". In general,
genimg_get_format handles only uImage, fitimage and android magic alone.

Thanks for the details, the use case is now clear.

Reviewed-by: Breno Lima <***@nxp.com>

Stefano Babic

2018-12-08 17:40:03 UTC

Permalink

Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

--
=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: ***@denx.de
=====================================================================