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

[Parthiban Nallathambi]

Hi Breno,

On 11/21/18 5:45 PM, Breno Matheus Lima wrote:
> Hi Parthiban,
> 
> Em qua, 21 de nov de 2018 às 11:50, Parthiban Nallathambi <pn at denx.de> escreveu:
>>
>> 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 <pn at 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;
>> +       }
>> +}
> 
> 
> 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.

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

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).

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

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

> 
> Any ideias if I'm missing something?
> 
>> +
>>   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.

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.

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

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: pn at denx.de