IMX8M Mini HAB secure boot - working?

IMX8M Mini HAB secure boot - working?

[Tim Harvey]

Greetings,

Has anyone successfully used secure boot with IMX8M Mini or other
IMX8M? Peng's recent series got merged with the exception of what
looks like the addition of couple of 'caam' commands to blob/deblob
DEK's.

There are no guides yet however I'm following the guides for the
downstream NXP U-Boot and thus far have been able to get the SPL to
boot with no HAB events but when it tries to authenticate the FIT
image it validate_ivt fails with 'Error: Invalid IVT structure'. I'm
not entirely clear if my CSF is wrong, or in the wrong place or if
there is something missing.

Best regards,

Tim

Re: IMX8M Mini HAB secure boot - working?

[Stefano Babic]

Hi Tim,

On 10.07.21 02:05, Tim Harvey wrote:
> Greetings,
> 
> Has anyone successfully used secure boot with IMX8M Mini or other
> IMX8M? Peng's recent series got merged with the exception of what
> looks like the addition of couple of 'caam' commands to blob/deblob
> DEK's.
> 
> There are no guides yet however I'm following the guides for the
> downstream NXP U-Boot and thus far have been able to get the SPL to
> boot with no HAB events but when it tries to authenticate the FIT
> image it validate_ivt fails with 'Error: Invalid IVT structure'.

Heiko tested this and found it, if I am not wrong he found the cause. 
Added him in CC.

I have also planned to test this, it is on my TODO list...

> I'm
> not entirely clear if my CSF is wrong, or in the wrong place or if
> there is something missing.
> 

Best regards,
Stefano


-- 
=====================================================================
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: sbabic@denx.de
=====================================================================

Re: IMX8M Mini HAB secure boot - working?

[Heiko Schocher]

[-- Attachment #1: Type: text/plain, Size: 1882 bytes --]

Hi Tim, Stefano,

On 10.07.21 11:14, Stefano Babic wrote:
> Hi Tim,
> 
> On 10.07.21 02:05, Tim Harvey wrote:
>> Greetings,
>>
>> Has anyone successfully used secure boot with IMX8M Mini or other
>> IMX8M? Peng's recent series got merged with the exception of what
>> looks like the addition of couple of 'caam' commands to blob/deblob
>> DEK's.
>>
>> There are no guides yet however I'm following the guides for the
>> downstream NXP U-Boot and thus far have been able to get the SPL to
>> boot with no HAB events but when it tries to authenticate the FIT
>> image it validate_ivt fails with 'Error: Invalid IVT structure'.
> 
> Heiko tested this and found it, if I am not wrong he found the cause. Added him in CC.
> 
> I have also planned to test this, it is on my TODO list...

I am currently not in my office, the whole next week ... so I could not
check my current state of the patches... but I found a problem, yes.

The problem was that the ROM API loaded the IVT header to a
memallocated address, which does of course not fit with the
address you have in IVT header ...

I have not full access to my development setup ,and found on my local
some old state of the patches .... may you can try them?

Of course they need a rework, other solution, but it shows the problem
hopefully...

bye,
Heiko
>> I'm
>> not entirely clear if my CSF is wrong, or in the wrong place or if
>> there is something missing.
>>
> 
> Best regards,
> Stefano
> 
> 

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


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


[-- Attachment #2: 0001-spl_fit.-add-hook-to-make-fixes-after-fit-header-is-.patch --]
[-- Type: text/x-patch, Size: 1766 bytes --]

>From 4a753221b5f60bb1c7a1c40f96cfa6a757a47f33 Mon Sep 17 00:00:00 2001
From: Heiko Schocher <hs@denx.de>
Date: Tue, 8 Jun 2021 07:25:18 +0200
Subject: [PATCH 1/2] spl_fit. add hook to make fixes after fit header is
 loaded

add hook function spl_load_simple_fit_fix_load()
which is called after fit image header is loaded.

Signed-off-by: Heiko Schocher <hs@denx.de>
---
 common/spl/spl_fit.c | 11 +++++++++++
 include/spl.h        |  8 ++++++++
 2 files changed, 19 insertions(+)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index caddf51196..f1374ee672 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -547,6 +547,15 @@ __weak bool spl_load_simple_fit_skip_processing(void)
 	return false;
 }
 
+/*
+ * Weak default function to allow fixes after fit header
+ * is loaded.
+ */
+__weak void *spl_load_simple_fit_fix_load(void *fit)
+{
+	return fit;
+}
+
 static void warn_deprecated(const char *msg)
 {
 	printf("DEPRECATED: %s\n", msg);
@@ -684,6 +693,8 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
 	if (spl_load_simple_fit_skip_processing())
 		return 0;
 
+	ctx.fit = spl_load_simple_fit_fix_load(ctx.fit);
+
 	ret = spl_simple_fit_parse(&ctx);
 	if (ret < 0)
 		return ret;
diff --git a/include/spl.h b/include/spl.h
index cee9a42ddb..f8ff7217a4 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -282,6 +282,14 @@ ulong spl_get_image_text_base(void);
  */
 bool spl_load_simple_fit_skip_processing(void);
 
+/**
+ * spl_load_simple_fit_fix_load() - Hook to make fixes
+ * after fit image header is loaded
+ *
+ * Returns pointer to fit
+ */
+void *spl_load_simple_fit_fix_load(void *fit);
+
 /**
  * spl_load_simple_fit() - Loads a fit image from a device.
  * @spl_image:	Image description to set up
-- 
2.20.1




[-- Attachment #3: 0002-imx-spl-implement-spl_load_simple_fit_fix_load.patch --]
[-- Type: text/x-patch, Size: 1576 bytes --]

>From 30507c10fdff6e56442b128bc96522afae6ba4ea Mon Sep 17 00:00:00 2001
From: Heiko Schocher <hs@denx.de>
Date: Tue, 8 Jun 2021 08:10:35 +0200
Subject: [PATCH 2/2] imx: spl: implement spl_load_simple_fit_fix_load

read the address where the IVT header must sit
from IVT image header, loaded from SPL into
an malloced buffer and copy the IVT header
to this address

May make this dependend on SoC ?

Signed-off-by: Heiko Schocher <hs@denx.de>
---
 arch/arm/mach-imx/spl.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
index dc6aebcaf7..a35e84caa4 100644
--- a/arch/arm/mach-imx/spl.c
+++ b/arch/arm/mach-imx/spl.c
@@ -345,3 +345,32 @@ int dram_init_banksize(void)
 	return 0;
 }
 #endif
+
+/*
+ * read the address where the IVT header must sit
+ * from IVT image header, loaded from SPL into
+ * an malloced buffer and copy the IVT header
+ * to this address
+ */
+void *spl_load_simple_fit_fix_load(void *fit)
+{
+	struct ivt *ivt;
+	unsigned long new;
+	unsigned long offset;
+	unsigned long size;
+	u8 *tmp = (u8 *)fit;
+
+	offset = ALIGN(fdt_totalsize(fit), 0x1000);
+        size = ALIGN(fdt_totalsize(fit), 4);
+        size = board_spl_fit_size_align(size);
+	tmp += offset;
+	ivt = (struct ivt *)tmp;
+	debug("%s: ivt: %p offset: %lx size: %lx\n", __func__, ivt, offset, size);
+	debug("%s: ivt self: %x\n", __func__, ivt->self);
+	new = ivt->self;
+	new -= offset;
+	debug("%s: new %lx\n", __func__, new);
+	memcpy((void *)new, fit, size);
+
+	return (void *)new;
+}
-- 
2.20.1

Re: IMX8M Mini HAB secure boot - working?

[Tim Harvey]

On Sat, Jul 10, 2021 at 5:24 AM Heiko Schocher <hs@denx.de> wrote:
>
> Hi Tim, Stefano,
>
> On 10.07.21 11:14, Stefano Babic wrote:
> > Hi Tim,
> >
> > On 10.07.21 02:05, Tim Harvey wrote:
> >> Greetings,
> >>
> >> Has anyone successfully used secure boot with IMX8M Mini or other
> >> IMX8M? Peng's recent series got merged with the exception of what
> >> looks like the addition of couple of 'caam' commands to blob/deblob
> >> DEK's.
> >>
> >> There are no guides yet however I'm following the guides for the
> >> downstream NXP U-Boot and thus far have been able to get the SPL to
> >> boot with no HAB events but when it tries to authenticate the FIT
> >> image it validate_ivt fails with 'Error: Invalid IVT structure'.
> >
> > Heiko tested this and found it, if I am not wrong he found the cause. Added him in CC.
> >
> > I have also planned to test this, it is on my TODO list...
>
> I am currently not in my office, the whole next week ... so I could not
> check my current state of the patches... but I found a problem, yes.
>
> The problem was that the ROM API loaded the IVT header to a
> memallocated address, which does of course not fit with the
> address you have in IVT header ...
>
> I have not full access to my development setup ,and found on my local
> some old state of the patches .... may you can try them?
>
> Of course they need a rework, other solution, but it shows the problem
> hopefully...
>

Heiko,

Thank you - that was indeed the issue and your patches resolve it. I
have not seen your patch posted to the list and your commit msg makes
it seem like your not sure if you should make it SoC dependent. Do you
plan on submitting these to the mailing list?

Best regards,

Tim

Re: IMX8M Mini HAB secure boot - working?

[Heiko Schocher]

Hello Tim,

On 12.07.21 18:06, Tim Harvey wrote:
> On Sat, Jul 10, 2021 at 5:24 AM Heiko Schocher <hs@denx.de> wrote:
>>
>> Hi Tim, Stefano,
>>
>> On 10.07.21 11:14, Stefano Babic wrote:
>>> Hi Tim,
>>>
>>> On 10.07.21 02:05, Tim Harvey wrote:
>>>> Greetings,
>>>>
>>>> Has anyone successfully used secure boot with IMX8M Mini or other
>>>> IMX8M? Peng's recent series got merged with the exception of what
>>>> looks like the addition of couple of 'caam' commands to blob/deblob
>>>> DEK's.
>>>>
>>>> There are no guides yet however I'm following the guides for the
>>>> downstream NXP U-Boot and thus far have been able to get the SPL to
>>>> boot with no HAB events but when it tries to authenticate the FIT
>>>> image it validate_ivt fails with 'Error: Invalid IVT structure'.
>>>
>>> Heiko tested this and found it, if I am not wrong he found the cause. Added him in CC.
>>>
>>> I have also planned to test this, it is on my TODO list...
>>
>> I am currently not in my office, the whole next week ... so I could not
>> check my current state of the patches... but I found a problem, yes.
>>
>> The problem was that the ROM API loaded the IVT header to a
>> memallocated address, which does of course not fit with the
>> address you have in IVT header ...
>>
>> I have not full access to my development setup ,and found on my local
>> some old state of the patches .... may you can try them?
>>
>> Of course they need a rework, other solution, but it shows the problem
>> hopefully...
>>
> 
> Heiko,
> 
> Thank you - that was indeed the issue and your patches resolve it. I

Cool! Thanks for testing!

> have not seen your patch posted to the list and your commit msg makes

Yes, as they are WIP not posted yet ... and I thought, I make something
wrong ... but if you have the same problem, it seems it is a real bug!

> it seem like your not sure if you should make it SoC dependent. Do you
> plan on submitting these to the mailing list?

The question is: is my approach to fix it the way to go? If you think so,
I can send them .. but give me please some time as I am just back from
vacation...

Fast look into it:

spl_load_simple_fit_fix_load() must also check if there is at "fit"
pointer an IVT header, if not, return simply fit pointer.

Question: dependent on SoC ... as it is in common code, and I think we
  need this "fix" only for imx8m?, so yes, it should be SoC dependent
  or better only imx8m? code defines this function... if possible.
  Therefore the weak function approach.

If you find time for looking at it, I am fine, if you use my patches
as base and you can post them?

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

Re: IMX8M Mini HAB secure boot - working?

[Tim Harvey]

On Sun, Jul 18, 2021 at 9:01 PM Heiko Schocher <hs@denx.de> wrote:
>
> Hello Tim,
>
> On 12.07.21 18:06, Tim Harvey wrote:
> > On Sat, Jul 10, 2021 at 5:24 AM Heiko Schocher <hs@denx.de> wrote:
> >>
> >> Hi Tim, Stefano,
> >>
> >> On 10.07.21 11:14, Stefano Babic wrote:
> >>> Hi Tim,
> >>>
> >>> On 10.07.21 02:05, Tim Harvey wrote:
> >>>> Greetings,
> >>>>
> >>>> Has anyone successfully used secure boot with IMX8M Mini or other
> >>>> IMX8M? Peng's recent series got merged with the exception of what
> >>>> looks like the addition of couple of 'caam' commands to blob/deblob
> >>>> DEK's.
> >>>>
> >>>> There are no guides yet however I'm following the guides for the
> >>>> downstream NXP U-Boot and thus far have been able to get the SPL to
> >>>> boot with no HAB events but when it tries to authenticate the FIT
> >>>> image it validate_ivt fails with 'Error: Invalid IVT structure'.
> >>>
> >>> Heiko tested this and found it, if I am not wrong he found the cause. Added him in CC.
> >>>
> >>> I have also planned to test this, it is on my TODO list...
> >>
> >> I am currently not in my office, the whole next week ... so I could not
> >> check my current state of the patches... but I found a problem, yes.
> >>
> >> The problem was that the ROM API loaded the IVT header to a
> >> memallocated address, which does of course not fit with the
> >> address you have in IVT header ...
> >>
> >> I have not full access to my development setup ,and found on my local
> >> some old state of the patches .... may you can try them?
> >>
> >> Of course they need a rework, other solution, but it shows the problem
> >> hopefully...
> >>
> >
> > Heiko,
> >
> > Thank you - that was indeed the issue and your patches resolve it. I
>
> Cool! Thanks for testing!
>
> > have not seen your patch posted to the list and your commit msg makes
>
> Yes, as they are WIP not posted yet ... and I thought, I make something
> wrong ... but if you have the same problem, it seems it is a real bug!
>
> > it seem like your not sure if you should make it SoC dependent. Do you
> > plan on submitting these to the mailing list?
>
> The question is: is my approach to fix it the way to go? If you think so,
> I can send them .. but give me please some time as I am just back from
> vacation...
>
> Fast look into it:
>
> spl_load_simple_fit_fix_load() must also check if there is at "fit"
> pointer an IVT header, if not, return simply fit pointer.

Yes, otherwise non-hab will fail, correct?

>
> Question: dependent on SoC ... as it is in common code, and I think we
>   need this "fix" only for imx8m?, so yes, it should be SoC dependent
>   or better only imx8m? code defines this function... if possible.
>   Therefore the weak function approach.
>
> If you find time for looking at it, I am fine, if you use my patches
> as base and you can post them?
>

I may be able to look at it again later this week or next.

I did run into another snag in that I could not figure out a proper
CSF template for multiple DTB's. When I try to include mulitple DTB's
I get several HAB events.

flash.log:
...
========= OFFSET dump =========
Loader IMAGE:
 header_image_off       0x0
 image_off              0x40
 csf_off                0x33200
 spl hab block:         0x7e0fc0 0x0 0x33200

Second Loader IMAGE:
 sld_header_off         0x57c00
 sld_csf_off            0x58c20
 sld hab block:         0x401fcdc0 0x57c00 0x1020

csf_fit.txt:
...
[Authenticate Data]
    # Key slot index used to authenticate the image data
    Verification index = 2
    # Authenticate Start Address, Offset, Length and file
    Blocks = \
        0x401fcdc0 0x00057c00 0x00001020 "flash.bin", \
        0x40200000 0x0005ac00 0x000b0150 "flash.bin", \
        0x402b0150 0x0010ad50 0x00008bc4 "flash.bin", \
        0x00920000 0x00113914 0x00009159 "flash.bin"

So that first block is the FIT image header I think (only 4128 bytes
long) using data from flash.log 'sld hab block:', the second block is
u-boot-nodtb.bin from within the FIT image, the third block is the dtb
from within the FIT, and the fourth block is the ATF from within the
FIT.

When I have multiple DTB's do I need a block per DTB? I haven't found
any example of a CSF template where CONFIG_OF_LIST has multiple dtb's.

Thanks,

Tim

Re: IMX8M Mini HAB secure boot - working?

[Heiko Schocher]

Hello Tim,

On 20.07.21 00:23, Tim Harvey wrote:
> On Sun, Jul 18, 2021 at 9:01 PM Heiko Schocher <hs@denx.de> wrote:
>>
>> Hello Tim,
>>
>> On 12.07.21 18:06, Tim Harvey wrote:
>>> On Sat, Jul 10, 2021 at 5:24 AM Heiko Schocher <hs@denx.de> wrote:
>>>>
>>>> Hi Tim, Stefano,
>>>>
>>>> On 10.07.21 11:14, Stefano Babic wrote:
>>>>> Hi Tim,
>>>>>
>>>>> On 10.07.21 02:05, Tim Harvey wrote:
>>>>>> Greetings,
>>>>>>
>>>>>> Has anyone successfully used secure boot with IMX8M Mini or other
>>>>>> IMX8M? Peng's recent series got merged with the exception of what
>>>>>> looks like the addition of couple of 'caam' commands to blob/deblob
>>>>>> DEK's.
>>>>>>
>>>>>> There are no guides yet however I'm following the guides for the
>>>>>> downstream NXP U-Boot and thus far have been able to get the SPL to
>>>>>> boot with no HAB events but when it tries to authenticate the FIT
>>>>>> image it validate_ivt fails with 'Error: Invalid IVT structure'.
>>>>>
>>>>> Heiko tested this and found it, if I am not wrong he found the cause. Added him in CC.
>>>>>
>>>>> I have also planned to test this, it is on my TODO list...
>>>>
>>>> I am currently not in my office, the whole next week ... so I could not
>>>> check my current state of the patches... but I found a problem, yes.
>>>>
>>>> The problem was that the ROM API loaded the IVT header to a
>>>> memallocated address, which does of course not fit with the
>>>> address you have in IVT header ...
>>>>
>>>> I have not full access to my development setup ,and found on my local
>>>> some old state of the patches .... may you can try them?
>>>>
>>>> Of course they need a rework, other solution, but it shows the problem
>>>> hopefully...
>>>>
>>>
>>> Heiko,
>>>
>>> Thank you - that was indeed the issue and your patches resolve it. I
>>
>> Cool! Thanks for testing!
>>
>>> have not seen your patch posted to the list and your commit msg makes
>>
>> Yes, as they are WIP not posted yet ... and I thought, I make something
>> wrong ... but if you have the same problem, it seems it is a real bug!
>>
>>> it seem like your not sure if you should make it SoC dependent. Do you
>>> plan on submitting these to the mailing list?
>>
>> The question is: is my approach to fix it the way to go? If you think so,
>> I can send them .. but give me please some time as I am just back from
>> vacation...
>>
>> Fast look into it:
>>
>> spl_load_simple_fit_fix_load() must also check if there is at "fit"
>> pointer an IVT header, if not, return simply fit pointer.
> 
> Yes, otherwise non-hab will fail, correct?

Yes.

>> Question: dependent on SoC ... as it is in common code, and I think we
>>   need this "fix" only for imx8m?, so yes, it should be SoC dependent
>>   or better only imx8m? code defines this function... if possible.
>>   Therefore the weak function approach.
>>
>> If you find time for looking at it, I am fine, if you use my patches
>> as base and you can post them?
>>
> 
> I may be able to look at it again later this week or next.

Fine.

> I did run into another snag in that I could not figure out a proper
> CSF template for multiple DTB's. When I try to include mulitple DTB's
> I get several HAB events.
> 
> flash.log:
> ...
> ========= OFFSET dump =========
> Loader IMAGE:
>  header_image_off       0x0
>  image_off              0x40
>  csf_off                0x33200
>  spl hab block:         0x7e0fc0 0x0 0x33200
> 
> Second Loader IMAGE:
>  sld_header_off         0x57c00
>  sld_csf_off            0x58c20
>  sld hab block:         0x401fcdc0 0x57c00 0x1020
> 
> csf_fit.txt:
> ...
> [Authenticate Data]
>     # Key slot index used to authenticate the image data
>     Verification index = 2
>     # Authenticate Start Address, Offset, Length and file
>     Blocks = \
>         0x401fcdc0 0x00057c00 0x00001020 "flash.bin", \
>         0x40200000 0x0005ac00 0x000b0150 "flash.bin", \
>         0x402b0150 0x0010ad50 0x00008bc4 "flash.bin", \
>         0x00920000 0x00113914 0x00009159 "flash.bin"
> 
> So that first block is the FIT image header I think (only 4128 bytes
> long) using data from flash.log 'sld hab block:', the second block is

Yes

> u-boot-nodtb.bin from within the FIT image, the third block is the dtb

Yes and yes

> from within the FIT, and the fourth block is the ATF from within the
> FIT.

Yes

> When I have multiple DTB's do I need a block per DTB? I haven't found
> any example of a CSF template where CONFIG_OF_LIST has multiple dtb's.

Hmm... good question... I think (not tested) when you have multiple
DTBs the DTBs are packed into a FIT image, so you have again only one
"dtb" part to sign...

./doc/README.multi-dtb-fit says:
"""
The relevant DTBs are packed into a FIT (list provided by CONFIG_OF_LIST). The
FIT is automatically generated at the end of the compilation and appended to
u-boot.bin so that U-Boot can locate it and select the correct DTB from inside
the FIT.
"""

So I think no problem...or?

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