[U-Boot] [PATCH 3/3] imx: hab: Convert DCD non-NULL error to warning

[Breno Matheus Lima]

Hi All,

2018-03-12 13:07 GMT-03:00 Tom Rini <trini at konsulko.com>:
> On Sun, Mar 11, 2018 at 03:36:16PM +0100, Stefano Babic wrote:
>> Hi Everybody,
>>
>> I have applied 1-2 as Fabio suggested. I have a couple of comments for
>> this, too:
>>
>> On 10/03/2018 02:10, Breno Matheus Lima wrote:
>> > Hi Bryan,
>> >
>> > 2018-03-09 10:07 GMT-03:00 Bryan O'Donoghue <bryan.odonoghue at linaro.org>:
>> >> commit 8c4037a09a5c ("imx: hab: Ensure the IVT DCD pointer is Null prior
>> >> to calling HAB authenticate function.") makes the DCD field being NULL a
>> >> dependency.
>> >>
>> >> This change though will break loading and executing of existing pre-signed
>> >> binaries on a u-boot update i.e. if this change is deployed on a board you
>> >> will be forced to redo all images on that board to NULL out the DCD.
>> >>
>> >> There is no prior guidance from NXP that the DCD must be NULL similarly
>> >> public guidance on usage of the HAB doesn't call out this NULL dependency
>> >> (see boundary devices link).
>> >>
>> >> Since later SoCs will reject a non-NULL DCD there's no reason to make a
>> >> NULL DCD a requirement, however if there is an actual dependency for later
>> >> SoCs the appropriate fix would be to do SoC version checking.
>> >>
>> >> Earlier SoCs are capable (and happy) to authenticate images with non-NULL
>> >> DCDs, we should not be forcing this change on downstream users -
>> >> particularly if it means those users now must rewrite their build systems
>> >> and/or redeploy signed images in the field.
>> >>
>> >> Fixes: 8c4037a09a5c ("imx: hab: Ensure the IVT DCD pointer is Null prior
>> >> to calling HAB authenticate function.")
>> >
>> > We understand the concern being raised however would prefer to leave
>> > this as an error, and selected users relying on images with DCD
>> > pointers can modify the code accordingly. This does not just apply to
>> > new SoC’s but also applies to existing SoC’s.
>>
>> Anyway, this is not so easy to identify. What we current know is that
>> older SOCs have no problem if the pointer is not set to Null. I agree
>> with Brian that if some constraint is coming, it should be defined with
>> a version checking as we currently do for a lot of things (is_soc() to
>> be clear).
>>
>> I am quite lost because I do not know which SOCs are affected and which
>> not. What does it hide under "later SOCSs" ? Or better which new version
>> of HAB ? I know HAB was updated due to other issues - are the
>> corresponding SOC involved ?
>>
>> If a not-null DCD pointer affects just future SOCs, we should fix it
>> when these SOCs will be available. This means both new SOSs (imx8x) as
>> new version of supported SOCs (mx5 / mx6 / mx7).
>>
>> > Users performing such an
>> > update should definitely test the image prior to making an OTA
>> > available.
>>
>> I think this is another topic and not what Brian is trying to address. I
>> hope as you that *any* update is tested before deploying. But this is
>> unrelated.
>>
>> > It has never been intended for DCD to be used in any post
>> > boot image and we realize the lack of documentation is a hindsight by
>> > us, and we are currently addressing that with updated guidance.
>> >
>>
>> ok, thanks for this, very appreciated.
>>
>> Anyway, I am facing what we should do with this patch for 2018.03. As I
>> said, I am not forcing anyone and I have just picked up 1/3 and 2/3. But
>> IMHO, if there are not good reason to say that not raising an error
>> breaks a lot of supplied device, this should flow into 2018.03, too. And
>> then we get enough time to better explore this issue.
>
> And I need an answer to this final part, before I can release v2018.03.
> Thanks all!
>

The purpose of hab_rvt_authenticate_image() API function is to
authenticate additional boot images in a post-ROM stage, initial boot
images are supposed to be authenticate only once by the initial ROM
code. The HAB implementation in older devices will process and run DCD
if we provide a DCD pointer. DCD commands are supposed to be executed
only once in an early boot stage, re-executing it could lead to an
incorrect authentication boot flow. If we convert DCD non-NULL error
to warning may also break supported devices, not only the new ones.

We understand Bryan's point based in CST documentation but
unfortunately our documentation is outdated, we are currently working
in a new version.

As Utkarsh mentioned in commit 8c4037a09a5c ("imx: hab: Ensure the IVT
DCD pointer is Null prior to calling HAB authenticate function."):
"DCD commands should only be present in the initial boot image loaded
by the SoC ROM. DCD should not be present in images that will be
verified by software using HAB RVT authentication APIs. Newer versions
of HAB will generate an error if a DCD pointer is present in an image
being authenticated by calling the HAB RVT API. Older versions of HAB
will process and run DCD if it is present, and this could lead to an
incorrect authentication boot flow."

Thanks,
Breno Lima