[PATCH] imx: hab: Make imx_hab_is_enabled dependent on FIELD_RETURN

[Paul Geurts]

Hi Ye,

> Hi Paul,
>
>On 7/1/2024 8:39 PM, Paul Geurts wrote:
>> Hi Ye,
>>
>>> Hi Paul,
>>>
>>> On 6/26/2024 3:17 PM, Paul Geurts wrote:
>>>> Hi,
>>>> Thanks for the feedback.
>>>>
>>>>> Hi Paul,
>>>>>
>>>>> On 6/24/2024 8:09 PM, Fabio Estevam wrote:
>>>>>
>>>>>> Hi Paul,
>>>>>>
>>>>>> On Fri, Jun 21, 2024 at 10:06 AM Paul Geurts
>>>>>> <paul.geurts at prodrive-technologies.com>  wrote:
>>>>>>
>>>>>>> -struct imx_sec_config_fuse_t {
>>>>>>> +struct imx_fuse_t {
>>>>>> Please make the struct renaming a separate patch.
>>>>>>
>>>>>> Peng Fan, Ye Li,
>>>>>>
>>>>>> Could you please help review this patch?
>>>>>>
>>>>>> Thanks
>>>>> Can you take a look iMX8MP FIELD_RETURN fuse, I think it does not
>>>>> have 1 bit but 8 bits which requires to burn a sequence. Only when
>>>>> the bits sequence is matched, the field return can work.  So checking
>>>>> the bit 0 is not enough.
>>>> Are you sure about that? The security reference manual (IMX8MPSRM)
>>>> says in Table 5-5
>>>> that the FIELD_RETURN fuse is located on fuse 0x630[0], which is a
>>>> single bit. Also,
>>>> the "Chip Security Lifecycle" section (2.15.1) says the following:
>>>>
>>>> FEILD RETURN (SEC_CONFIG[1] fuse = 1 and FIELD_RETURN fuse = 1)
>>>>
>>>> Are you maybe confusing the FIELD_RETURN fuse with the
>>>> FIELD_RETURN_LOCK sticky bit?
>>>> clearing the lock bit _is_ quite the procedure, but it is unrelated to
>>>> U-Boot, as
>>>> this is done by ROM code through CSF.
>>>>
>>>> I tested this on an i.MX8M Plus and it seems to work fine.
>>> I know the steps for field return.  What I mean is the FIELD_RETURN
>>> fuse.  It is true that security RM mentions it as you quote. But from
>>> 8MP fuse map and ROM codes,  I get different things.
>>>
>>> FIELD_RETURN 8-bit code.
>>> FIELD_RETURN = 0, is non-field return mode, functional/secure mode.
>>> FIELD_RETURN = Matching Sequence, device is in field_return mode
>>> FIELD_RETURN != Matching Sequence, device asserts security violation
>> That is indeed different from what is mentioned in documentation. I have
>> asked our NXP FAE about the discrepancy and I will adjust the code if
>> needed.
>
> Thanks for confirm. I also cross checked with teams. 8MP must burn a 
> pattern. Otherwise HAB won't covert to field return.

Okay, thanks for checking, I will wait for the details and make the necessary adjustments

>
> Additional, do you think it is very necessary to add this patch set?  
> Because field return is a pure debug feature, it won't be deployed on 
> productions. The developers working on field return parts can re-build 
> u-boot with CONFIG_IMX_HAB disabled.

In an OEM situation, is a lot of cases, the company creating the bootloader (OEM) is typically neither the one
singing the bootloader nor performing the FIELD_RETURN setting (end customer/VAR). The end
customer is typically neither interested nor capable of rebuilding the bootloader with CONFIG_IMX_HAB
disabled.

This means 2 bootloaders need to be maintained in parallel by the OEM, creating unnecessary overhead. This
also introduces additional risk as the end customer may sign the wrong bootloader (with HAB disabled).

>
> This patch may introduce risk to HAB in some sense,  especially for 
> productions. One mistake would make unsigned image bypass authentication 
> result.

I think this risk is mitigated by the fuse unlocking procedure imposed by HAB. I don't think someone
will accidentally go through the entire procedure of unlocking the FIELD_RETURN fuse and then
also accidentally burning the fuse. The risk in code IMO is not greater then the risk already there by
reading out the SEC_CONFIG fuse.

>
>
> Best regards,
>
> Ye Li
>
>>>
>>> However, I'm not sure how is it implemented in HAB. Since you have
>>> tested 8M plus, can you confirm the closed part is successfully
>>> converted to field return and can boot without signing?
>> Maybe I did something wrong while testing. I will retest it on a new
>> board when I have received some more information from NXP.
>>
>>>
>>>   Best regards,
>>>
>>> Ye Li
>>>