boot: zephyr: Avoid relying on assert statement and add explicit checks

[namjoshiniks]

Avoid relying on assert statement and add explicit checks

Fixes /issues/2661

[de-nordic]

This is not a best idea.
When we fail in these places it is not like you can fix a device nor react to the problem, yet these errors will be carried verbatim wasting space on device boot partition.
There is a difference between this issues and when, for example, signature check fails; the later has a chance to happen due to actually bad signature or image and tells you that something may be done to address the problem
If any of fixed errors here happen we have either configuration issue or device is failing anyway and nothing can be done; these are either catch in testing or device can play dead anyway.

[namjoshiniks]

This is not a best idea. When we fail in these places it is not like you can fix a device nor react to the problem, yet these errors will be carried verbatim wasting space on device boot partition. There is a difference between this issues and when, for example, signature check fails; the later has a chance to happen due to actually bad signature or image and tells you that something may be done to address the problem If any of fixed errors here happen we have either configuration issue or device is failing anyway and nothing can be done; these are either catch in testing or device can play dead anyway.

@de-nordic

The problem here is the ASSERT statements get stripped during compile time if DEBUG flag is disabled and CONFIG_ASSERT=n, which is the case in all production projects. If asserts are stripped off, the behavior is undefined and error can go undetected (and possibly a security vulnerability too).

As for space, Zephyr based MCUboot is already bulky so a few bytes may not matter. But I do agree, that having string statements for unrecoverable errors that shouldn't really happen in production code is unnecessary. So made the logs to be DBG type logs

[de-nordic]

The problem here is the ASSERT statements get stripped during compile time if DEBUG flag is disabled and CONFIG_ASSERT=n, which is the case in all production projects. If asserts are stripped off, the behavior is undefined and error can go undetected (and possibly a security vulnerability too).

Maybe change this to LOG_DBG and panic when tests fail, even when non in debug mode.

As for space, Zephyr based MCUboot is already bulky so a few bytes may not matter.

They do when they are generally dead code unless unrecoverable problem occurs. And it does matter, because that is space taken out of apps, that is space I am asked to find for customers, that is space that is taking extra time to verify on signature, when mcuboot is protected in other means, this also takes out extra protection bits in soc.

But I do agree, that having string statements for unrecoverable errors that shouldn't really happen in production code is unnecessary. So made the logs to be DBG type logs

Fine with this.

[namjoshiniks]

Maybe change this to LOG_DBG and panic when tests fail, even when non in debug mode.

Not sure if you got chance to look at the code, but I changed the logging statements to Debug. Additionally, wherever possible I am returning errors (from non void functions or functions that can handle errors gracefully). For void functions, I added FIH_PANIC.