bootutil: Add compressed image flags and TLV, fix misc. bugs in bootutil #2028
Conversation
| * SHA256 of compressed/encrypted image data in the slot | ||
| * which should be checked to ensure image integrity | ||
| * before decrypting/decompressing data |
There was a problem hiding this comment.
Isn't this little confusing? In case when image is encrypted actually of the other SHA TLV are used but the image is first decrypted and then sha is calculated on the raw image.
So if only the existing encryption is used the other TLV serve as the RAW sha.
Shouldn't we have opposite?
Currently what happens is the bootutil_img_validate will first call bootutil_img_hash that will decrypt image into RAM and calculate the sha with pre-selected, during compilation, sha algorithm and then compare the resulting sha with expected TLV (EXPECTED_HASH_TLV ), which would be the IMAGE_TLV_SHA256 by default.
This also means that if we validate slot, that has image already decrypted and decompressed we will check it against EXPECTED_HASH_TLV stored sha, which currently serves as the "RAW" sha.
There was a problem hiding this comment.
There needs to be a way to validate the image post-decompression which the existing tag does, however you also need to check the image before doing any sort of decompression on it, it's not an issue for decryption because you have a block size and the output size is the same as the padded input size, but that is not the case with decompression, a 4000 byte input can have a megabyte output, and you need to know that the image you have is actually a valid image, so a hash of it in the protected TLV region needs to be used.
(both TLVs would be used, decompressed image will still continue to use the original tag, new tag is for the compressed data itself, can optionally be used with encrypted data as well since it doesn't make sense to decrypt data that is wrong and delay device boot up by a long period of time)
|
Flags: TLVs: The image header field for the image size should be the compressed size, and the hash there should be the hash of the compressed data, since the only current supported route will be to load a compressed update to the secondary slot and decompress it into the primary slot, the image can be verified first, then decompression can run to generate the complete decompressed image which can be written back to the primary slot. Some fields in the secondary slot will be changed when the primary slot data is written, the unprotected fields for hash and signature will be replaced with those from the decompressed TLVs above and the image size will also be replaced with the decompressed TLV, then those fields will be removed from the protected TLV section and the size of that section will be reduced to account for it. The compressed flag will also be removed from the image as it is no longer compressed. Benefit of this is to not store additional fields in the primary slot that are not needed. |
| @@ -101,6 +112,7 @@ struct flash_area; | |||
| #define IMAGE_TLV_DEPENDENCY 0x40 /* Image depends on other image */ | |||
| #define IMAGE_TLV_SEC_CNT 0x50 /* security counter */ | |||
| #define IMAGE_TLV_BOOT_RECORD 0x60 /* measured boot record */ | |||
| #define IMAGE_TLV_COMP_SIZE 0x70 /* Compressed image size */ | |||
There was a problem hiding this comment.
Additionally, it is a little unclear what the sizes mean. Does this mean the has the uncompressed size? How would we find the TLV if the actual data stored is different than the size in the header?
There was a problem hiding this comment.
For some reason I thought it went through looking for the magic bytes, apparently not... Will have a rethink
There was a problem hiding this comment.
Have updated the PR to instead have 3 TLVs, the hash and signature would be equivalent to the compressed image fields but on the decompressed image data instead (this is so that the data can be validated before decompressing it)
Adds some flags to indicate if the data of an image is compressed (lzma1 and lzma2) and adds new TLVs for compressed images relating to the hash, signature and size of the decompressed image data, this allows the image to be validated before decompressing, then validated after decompression to ensure an image is always valid for a device Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
Adds a Kconfig allowing the decompression option to be selected Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
Checks if images have compressed or encrypted image flags and, if so, and those options are not enabled in that MCUboot build, will class the images as invalid and delete them (these images cannot be used without support anyway) Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
Marks images as invalid if they have conflicting flags, e.g. more than one type of LZMA compression or more than one type of encryption Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
The protected TLV section was not included in the size check of if an image could fit into a slot, which means that it was possible for file to be deemed as OK for storing but then failing due to insufficient flash space during the update Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
nordicjm
changed the title
When compression is used, it allows for the secondary slot to be smaller than the primary slot, therefore do not ensure that the number of sectors in each slot are the same Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
Adds release notes based on these changes Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
|
@davidvincze Can you review this? |
Adds some flags to indicate if the data of an image is compressed (lzma1 and lzma2) and adds a new TLV for a sha256 hash of the decompressed/decrypted image data, this would (optionally) be used for images where one or both options are enabled to be able to verify the data is correct and that is can be written/booted successfully
Note: no code additions relating to actual compression are present here, just addition of TLVs and flags
Also includes some bug fixes to bootutil's image header checks.
Requires #2045 be merged