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