Fix handling of encrypted images
Encrypted images were known to be failing when the header size was larger
than 256 bytes because of incorrect handling of blocks sent to decryption
and hashing routines. An assert was previously added to check the header
and read block sizes matched to avoid incurring into the know error, but
it was incorrectly enabled also for non-encrypted images.
Now the handling of the header, which is not encrypted, is correctly
separated from the handling of the remaining image, when encryption is
used, to avoid ever sending header data into the decryption routines.
Signed-off-by: Fabio Utzig <utzig@apache.org>
diff --git a/boot/bootutil/include/bootutil/image.h b/boot/bootutil/include/bootutil/image.h
index 021a3a4..b2d1d94 100644
--- a/boot/bootutil/include/bootutil/image.h
+++ b/boot/bootutil/include/bootutil/image.h
@@ -103,6 +103,8 @@
uint16_t it_len; /* Data length (not including TLV header). */
};
+#define IS_ENCRYPTED(hdr) ((hdr)->ih_flags & IMAGE_F_ENCRYPTED)
+
_Static_assert(sizeof(struct image_header) == IMAGE_HEADER_SIZE,
"struct image_header not required size");
diff --git a/boot/bootutil/src/image_validate.c b/boot/bootutil/src/image_validate.c
index e2e989c..d2d741a 100644
--- a/boot/bootutil/src/image_validate.c
+++ b/boot/bootutil/src/image_validate.c
@@ -56,11 +56,11 @@
bootutil_sha256_context sha256_ctx;
uint32_t blk_sz;
uint32_t size;
+ uint16_t hdr_size;
uint32_t off;
int rc;
#ifdef MCUBOOT_ENC_IMAGES
uint32_t blk_off;
- uint8_t idx;
#endif
bootutil_sha256_init(&sha256_ctx);
@@ -73,9 +73,7 @@
#ifdef MCUBOOT_ENC_IMAGES
/* Encrypted images only exist in slot1 */
- if (fap->fa_id == FLASH_AREA_IMAGE_1 &&
- (hdr->ih_flags & IMAGE_F_ENCRYPTED) &&
- !boot_enc_valid(fap)) {
+ if (fap->fa_id == FLASH_AREA_IMAGE_1 && IS_ENCRYPTED(hdr) && !boot_enc_valid(fap)) {
return -1;
}
#endif
@@ -84,29 +82,30 @@
* Hash is computed over image header and image itself. No TLV is
* included ATM.
*/
- size = hdr->ih_img_size + hdr->ih_hdr_size;
- assert(tmp_buf_sz > hdr->ih_hdr_size);
+ hdr_size = hdr->ih_hdr_size;
+ size = hdr->ih_img_size + hdr_size;
for (off = 0; off < size; off += blk_sz) {
blk_sz = size - off;
if (blk_sz > tmp_buf_sz) {
blk_sz = tmp_buf_sz;
}
+#ifdef MCUBOOT_ENC_IMAGES
+ /* Avoid reading header data together with other image data
+ * because the header is not encrypted, so maintain correct
+ * bounds when sending encrypted data to decrypt routine.
+ */
+ if ((off < hdr_size) && ((off + blk_sz) > hdr_size)) {
+ blk_sz = hdr_size - off;
+ }
+#endif
rc = flash_area_read(fap, off, tmp_buf, blk_sz);
if (rc) {
return rc;
}
#ifdef MCUBOOT_ENC_IMAGES
- if (fap->fa_id == FLASH_AREA_IMAGE_1 && hdr->ih_flags & IMAGE_F_ENCRYPTED) {
- /* FIXME: fails if header size is larger than blk_sz */
- if (off < hdr->ih_hdr_size) {
- idx = hdr->ih_hdr_size;
- blk_off = 0;
- } else {
- idx = 0;
- blk_off = (off - hdr->ih_hdr_size) & 0xf;
- }
- boot_encrypt(fap, (off + idx) - hdr->ih_hdr_size, blk_sz - idx,
- blk_off, &tmp_buf[idx]);
+ if (fap->fa_id == FLASH_AREA_IMAGE_1 && IS_ENCRYPTED(hdr) && off >= hdr_size) {
+ blk_off = (off - hdr_size) & 0xf;
+ boot_encrypt(fap, off - hdr_size, blk_sz, blk_off, tmp_buf);
}
#endif
bootutil_sha256_update(&sha256_ctx, tmp_buf, blk_sz);
diff --git a/boot/bootutil/src/loader.c b/boot/bootutil/src/loader.c
index 6839ddf..3b21b79 100644
--- a/boot/bootutil/src/loader.c
+++ b/boot/bootutil/src/loader.c
@@ -586,7 +586,7 @@
(void)bs;
(void)rc;
#else
- if (fap->fa_id == FLASH_AREA_IMAGE_1 && hdr->ih_flags & IMAGE_F_ENCRYPTED) {
+ if (fap->fa_id == FLASH_AREA_IMAGE_1 && IS_ENCRYPTED(hdr)) {
rc = boot_enc_load(hdr, fap, bs->enckey[1]);
if (rc < 0) {
return BOOT_EBADIMAGE;
@@ -821,7 +821,7 @@
hdr = boot_img_hdr(&boot_data, 0);
off = off_dst;
}
- if (hdr->ih_flags & IMAGE_F_ENCRYPTED) {
+ if (IS_ENCRYPTED(hdr)) {
blk_sz = chunk_sz;
idx = 0;
if (off + bytes_copied < hdr->ih_hdr_size) {
@@ -1136,7 +1136,7 @@
}
#ifdef MCUBOOT_ENC_IMAGES
- if (boot_img_hdr(&boot_data, 1)->ih_flags & IMAGE_F_ENCRYPTED) {
+ if (IS_ENCRYPTED(boot_img_hdr(&boot_data, 1))) {
rc = boot_enc_load(boot_img_hdr(&boot_data, 1), fap_slot1, bs->enckey[1]);
if (rc < 0) {
return BOOT_EBADIMAGE;
@@ -1220,7 +1220,7 @@
}
#ifdef MCUBOOT_ENC_IMAGES
- if (hdr->ih_flags & IMAGE_F_ENCRYPTED) {
+ if (IS_ENCRYPTED(hdr)) {
fap = BOOT_IMG_AREA(&boot_data, 0);
rc = boot_enc_load(hdr, fap, bs->enckey[0]);
assert(rc >= 0);
@@ -1244,7 +1244,7 @@
#ifdef MCUBOOT_ENC_IMAGES
hdr = boot_img_hdr(&boot_data, 1);
- if (hdr->ih_flags & IMAGE_F_ENCRYPTED) {
+ if (IS_ENCRYPTED(hdr)) {
fap = BOOT_IMG_AREA(&boot_data, 1);
rc = boot_enc_load(hdr, fap, bs->enckey[1]);
assert(rc >= 0);