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

[Bryan O'Donoghue]

On 10/03/18 01: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.")
> 
> It has never been intended for DCD to be used in any post
> boot image 

Breno,

There's extensive documentation from NXP in the CST docs detailing usage 
of the DCD by post 1st-stage images.

High Assurance Boot Version 4 Application Programming Interface 
Reference Manual version 2.3.2 section "3.3 Authenticate Image"

"Purpose:
This function combines _DCD_, CSF and Assert functions in a standard 
sequence in order to authenticate a loaded image. It is intended for use 
by post-ROM boot stage components, via the ROM Vector Table. Support for 
images partially loaded to an initial location is provided
via a callback function"

<snip>

"Postconditions:
The post-conditions of the functions hab_rvt.check_target(), 
_hab_rvt.run_dcd()_,hab_rvt.run_csf() and hab_rvt.assert() apply also to 
this function. In particular, any audit events logged within the given 
functions have the context field appropriate to that function rather 
than HAB_CTX_AUTHENTICATE. In addition, the side-effects and 
post-conditions of any callback function supplied apply."

More than that - there's even a BootROM API callback "section 3.4 Run DCD"

"3.4
Run DCD

hab_status_t(* hab_rvt::run_dcd)(const uint8_t *dcd)
Execute boot configuration script.

Purpose:
This function configures the IC based upon a Device Configuration Data 
table. It is intended for use by post-ROM boot stage components, via the 
ROM Vector Table.

This function may be invoked as often as required for each boot stage.
The difference between the configuration functionality in this function 
and hab_rvt.run_csf() arises because the Device Configuration Data table 
is not authenticated prior to running the commands. Hence, there is a 
more limited range of commands allowed, and a limited range of 
parameters to allowed commands."

I don't think its reasonable to go forcing people to NULL out the DCD 
(which is work for them - and forces a OTA updates) - let alone reading 
the docs now - people might even be _doing_ DCD things right now.

There's even a callback that allows you to run the DCD from u-boot !

By all means restrict on a per-SoC basis but that should be version 
checked and justified - particularly if there is a derogation from the 
official documentation that comes with the code-signing tools.

---
bod