smpmgr: upgrade: add --bypass-inspect option for non-MCUboot images

[axelnxp]

Add a new --bypass-inspect flag to the upgrade command that allows uploading firmware images that are not in MCUboot format.

When this option is enabled, the image hash is read from the device's image state via an SMP request instead of being extracted from the local file's TLV data.

[JPHutchins]

Generally, this option looks OK to me. I'm curious about the scenario where this is required - build system, SMP server being used - perhaps we could document that?

Should this client be calculating the hash so that it can verify the integrity of the upload? It seems that leaving it out entirely allows us to load corrupted firmware, calculate the hash over the corrupted firmware, then permanently mark the corrupted image as not corrupted.

[axelnxp]

@JPHutchins so basically I'm working on a feature for Zephyr, you can find everything in this RFC: zephyrproject-rtos/zephyr#103999

The idea is to support custom bootloaders and not only mcuboot. In our case, the image format is very specific (NXP's SB3.1).
Calculating the hash locally might not always work properly since it assumes the hash showed in the image state is the hash of the whole binary. It might not be always the case. As an example, in our SB3.1 format, the file is structured in blocks, each block has a hash, so we decided to report the hash of the first block, as it's enough to determine the image.

Regarding your point on the potential for uploading corrupted firmware: in our case, the image integrity is validated by our bootloader, so the upgrade won't happen if the image is corrupted. But I totally agree that if the device is unable to validate the image integrity, then there's a risk.

So, I think I can improve the option documentation, clearly stating that this option puts the responsibility on the device to validate the image, and if it's not the case, the user understands the image could be corrupted.

Would that work for you ?

[JPHutchins]

The idea is to support custom bootloaders and not only mcuboot.

Excellent! I will review ASAP.

upgrade won't happen if the image is corrupted

If a SHA256 is not present in the image format itself, nor calculated by the client and then compared to the calculation done by the server, then how can smpmgr trust that the image has been copied over the wire and into NVM perfectly? A SHA256 calculated by the SMP server or bootloader after transport is only half of what's required, from smpmgr's perspective.

I realize that you likely have this taken care of for the RFC, I am investigating to understand what warnings we might add to avoid misuse by less rigorous implementations.

[axelnxp]

If a SHA256 is not present in the image format itself, nor calculated by the client and then compared to the calculation done by the server, then how can smpmgr trust that the image has been copied over the wire and into NVM perfectly? A SHA256 calculated by the SMP server or bootloader after transport is only half of what's required, from smpmgr's perspective.

Sorry I wasn't clear, the hash is present in the image file, at least in our case (SB3.1). When the SB3 file is generated, the hash is encoded in the file. So, the bootloader checks the hash encoded in the file actually corresponds to what's written in flash.
It's like MCUboot's TLVs, but it's just that it's encoded differently, the concept is still the same.

Edit: reading twice I realized you probably meant if someone uses another image format which doesn't encode a hash. Is that right?
In this case, is it even smpmgr responsibility to catch that? I would think it's the bootloader's concern. I could add that to the option helper, something like "it's assumed the image format encodes image's hash to ensure image integrity can be validated by the device."

[JPHutchins]

If a SHA256 is not present in the image format itself, nor calculated by the client and then compared to the calculation done by the server, then how can smpmgr trust that the image has been copied over the wire and into NVM perfectly? A SHA256 calculated by the SMP server or bootloader after transport is only half of what's required, from smpmgr's perspective.

Sorry I wasn't clear, the hash is present in the image file, at least in our case (SB3.1). When the SB3 file is generated, the hash is encoded in the file. So, the bootloader checks the hash encoded in the file actually corresponds to what's written in flash.

It's like MCUboot's TLVs, but it's just that it's encoded differently, the concept is still the same.

Perfect! In the long run, we'll probably add "image type" argument, including "unknown", which would bypass any checks like this implementation does. But we can run with this for now to facilitate testing and integration.

[axelnxp]

@JPHutchins I updated the change to clarify that it's assumed the image format encodes any sort of integrity check.