Hi Raghu,

Let me try to clarify/reword my idea:

We complete the fconf documentation with a binding document, which defines the nodes that should be present in the config DTB to consider it as valid/well-formed. The document contains two kinds of node:

* mandatory (critical): the firmware can't proceed without this information (example: load address, image UUID, ...). If this node is not present in the DTS, the build fails and/or the code panic.

* optional (no-critical): the firmware can assume or assign a default value and proceed (example: uart config, enable authentication flag, ...). Such a property is used to influence the default behavior of the firmware.

...

This is non-deterministic failure.

The miss of a mandatory (critical) node will always lead to a build failure / code panic. The miss of an optional (no-critical) node should not influence the default behavior of the firmware.

...

Is it not confusing to make the assumption that a DTB is "well formed",i.e expect the build process/integrator to not mess up the structure or number of nodes but allow the same integrator to miss a critical property in the DTB?

A DTB with a missing mandatory (critical) property should not be considered well formed.

...

If there is a missing critical property, is that not a badly formed DTB?

With the above definition, it is.

...

And if so, why not check for badly formed DTB's uniformly in code and why only check for missing "critical" properties?

With the rewording of "critical" / "no-critical" to "mandatory" / "optional", I think the situation is clearer: A DTB with a missing "optional" node/property is is still considered well-formed.

I hope I answered your questions and clarify the idea behind the design.

regards, Louis

________________________________ From: Raghu Krishnamurthy raghu.ncstate@icloud.com Sent: 05 April 2020 00:06 To: Louis Mayencourt Louis.Mayencourt@arm.com Subject: Re: [TF-A] fconf: Validating config data

Thanks Louis.

...

you can imagine a well-formed DTB which contain a critical set of properties and can contain some optional properties.

This makes things even more confusing. The assumption we are asking code to make is that the DTB is always "well formed", ie don't check for structural issues such as extra nodes etc, but we are still making the distinctions between critical and non-critical properties, that may or may not exist in the DTB, which may or may not cause a panic. This is non-deterministic failure. Is it not confusing to make the assumption that a DTB is "well formed",i.e expect the build process/integrator to not mess up the structure or number of nodes but allow the same integrator to miss a critical property in the DTB? If there is a missing critical property, is that not a badly formed DTB ? And if so, why not check for badly formed DTB's uniformly in code and why only check for missing "critical" properties?

-Raghu

On 4/3/20 3:16 AM, Louis Mayencourt wrote:

Hi Raghu,

I do agree that we need something similar to a binding document for fconf properties. (similar to https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/3694/2/docs/c...). At least for the common properties.

The main idea behind the return code of the populator function was to allow the code to handle no-critical property misses or to handle critical failure by calling a platform hook. With this in mind, you can imagine a well-formed DTB which contain a critical set of properties and can contain some optional properties. The return code and the populator "name" / "config" can be used to handle this two cases.

I tried to keep the design of fconf really simple, to leave room for improvement according to feedbacks. Thanks for helping improving it!

Regards, Louis

---

*From:* TF-A tf-a-bounces@lists.trustedfirmware.org on behalf of Raghu Krishnamurthy via TF-A tf-a@lists.trustedfirmware.org *Sent:* 03 April 2020 10:15 *To:* tf-a@lists.trustedfirmware.org tf-a@lists.trustedfirmware.org *Subject:* Re: [TF-A] fconf: Validating config data A further point is that the fconf populators return an error code and panics on error today. But if we are making the assumption that the DTB's are well formed, do we really need to fail or even return an error code?

-Raghu

On 4/3/20 1:51 AM, Raghu Krishnamurthy via TF-A wrote:

...

Hi All, (Sorry for the long email)

The review https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/3845 attempts to fix bounds check in the fconf populator code for the topology and SP's. During review, Sandrine thoughtfully pointed out that there were discussions around bounds check along the same lines in the review https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/3492 and it was deemed sufficient to have assertions in code and it was safe to make the assumption that the DTB is always well formed and contains valid values. I think this email mostly echoes Sandrine's concern from review 3492.

While i agree with the assumptions, I am generally of the opinion that we should validate/range-check any data, even if it is signed. Being signed does not necessarily mean the data is well formed/valid. If there is a mistake in the build process and it is validly signed, it is possible that we silently corrupt state/data that could later be used to exploit firmware and/or make debugging hard. This is probably far fetched, but the cost of adding the check is trivial to avoid this possibility.

I imagine the case where you have secure partitions signed by different entity other than the silicon provider(dual root-of-trust). A silicon provider provides a dev system for the SP provider to test and validate the SP's on silicon. The silicon has production firmware(and hence no assertions), but loads signed data from the SP provider which has some invalid values. There could be silent corruption without any indication whatsoever about what went wrong and it may be hard to debug if/when there are issues. Also, testing does not necessarily catch all invalid values since you will likely not get 100% coverage, given the number of config options available. Moreover, the code today, is not consistent in asserting on every property for valid values and the failure mode is not consistent/deterministic. It seems like every config option should have a list of valid values or a range of acceptable values that must be at a minimum asserted on. I also wouldn't discount platforms such as RPI, where TRUSTED_BOARD_BOOT is likely to be turned off since it really does not provide any security, so assuming we always have signed data might not be valid.

Anyway, is this decision worth revisiting? Too paranoid perhaps? :P

Thanks Raghu

-- TF-A mailing list TF-A@lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-a IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.