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

[Bryan O'Donoghue]

On 12/03/18 16:33, Breno Matheus Lima wrote:

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


Breno if that is so, why is there a ROM provided callback "run_dcd" ?

3.4
hab_status_t(* hab_rvt::run_dcd)(const uint8_t *dcd)

It may be the case that you are moving to the DCD being a bootrom only 
interface but that is certainly not the case right now.

> re-executing it could lead to an
> incorrect authentication boot flow. 

Which is the difference between "the DCD" i.e. the only DCD that can run 
and "a DCD" - meaning the DCD associated with an image.

You've provided APIs to run a DCD, make extensive reference to running 
'a DCD' with a given image.

How can you be so sure that all users of u-boot HAB don't have a DCD 
phase with images after the first phase ?

> If we convert DCD non-NULL error
> to warning may also break supported devices, not only the new ones.

Which ones ? Can you give some details to back this up ?


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

But Breno - until and unless you publish something that super-cedes the 
current published standard - you are introducing breakage into the 
current HAB layer.

IMO the right thing to do is to publish a description the issue you are 
trying to address, then discuss fixes for it.


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

Which is an assertion you are making now, without reference to any 
supporting litreature and proposing that everybody using the HAB 
interface just adopts this change and churns their downstream images.


> DCD should not be present in images that will be
> verified by software using HAB RVT authentication APIs.

Not according to your latest published standard on HAB. Can you really 
prove that nobody is using DCD specifically "run_dcd()" as provided by 
your own BootROM at this time ?

On what basis are you forcing end-users to rewrite their code-signing 
infrastructure and re-sign all of their binaries - potentially binaries 
in the field ?

I really can't agree with this approach. If you want to force such a 
change on people - you need a reason.

Consider a user with an i.MX6 board who wants to pick up a fix for an 
unrelated issue - USB for agrgument's sake. They then need to re-sign 
all of the binaries u-boot authenticates via HAB for no benefit to that 
user at all.

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

Then version check it ! Why do existing users need to suck up the change 
for upcoming (unrleased?) HAB implementations ?

> Older versions of HAB
> will process and run DCD if it is present, and this could lead to an
> incorrect authentication boot flow."

Sorry I really don't accept this - you provide a _callback_ called 
"run_dcd()" in your BootROM.

Meaning I provide a pointer to a signed image that includes a DCD phase. 
I can then run the DCD in isolation.

Why is that now broken on older HAB implementations ?

Honestly - I think this change is pretty bogus - we should either revert 
it or as I've proposed her "Warn".

You can then come along and version check on later SoCs once you've 
published _supporting_documentation_ to go with it - that justifies and 
explains (in detail) why it is necessary to restrict this interface on 
new (or existing SoCs).

---
bod