Boot: Mitigate TOCTOU attack when RAM loading
First load the image into RAM, and then perform hash and signature
checks on the RAM image. Replaces verify then copy that was susceptible
to the image being replaced between the operations. Use RAM image to
generate boot record.
Change-Id: I519cf0d5e2757791e9706008caf4098bfe5884c9
Signed-off-by: Raef Coles <raef.coles@arm.com>
diff --git a/bl2/ext/mcuboot/bootutil/include/bootutil/image.h b/bl2/ext/mcuboot/bootutil/include/bootutil/image.h
index 46fdbfc..596f0f8 100644
--- a/bl2/ext/mcuboot/bootutil/include/bootutil/image.h
+++ b/bl2/ext/mcuboot/bootutil/include/bootutil/image.h
@@ -131,9 +131,6 @@
const struct flash_area *fap,
uint32_t *security_cnt);
-#ifdef MCUBOOT_RAM_LOADING
-int bootutil_check_hash_after_loading(struct image_header *hdr);
-#endif
#ifdef __cplusplus
}
diff --git a/bl2/ext/mcuboot/bootutil/src/bootutil_priv.h b/bl2/ext/mcuboot/bootutil/src/bootutil_priv.h
index c861e6f..516d196 100644
--- a/bl2/ext/mcuboot/bootutil/src/bootutil_priv.h
+++ b/bl2/ext/mcuboot/bootutil/src/bootutil_priv.h
@@ -375,6 +375,14 @@
#endif /* !defined(MCUBOOT_USE_FLASH_AREA_GET_SECTORS) */
+#ifdef MCUBOOT_RAM_LOADING
+#define LOAD_IMAGE_DATA(fap, start, output, size)\
+ (memcpy((output),(void*)(hdr->ih_load_addr + (start)), (size)) != (output))
+#else
+#define LOAD_IMAGE_DATA(fap, start, output, size)\
+ (flash_area_read((fap), (start), (output), (size)))
+#endif /* MCUBOOT_RAM_LOADING */
+
#ifdef __cplusplus
}
#endif
diff --git a/bl2/ext/mcuboot/bootutil/src/image_validate.c b/bl2/ext/mcuboot/bootutil/src/image_validate.c
index 77b079c..94a25d0 100644
--- a/bl2/ext/mcuboot/bootutil/src/image_validate.c
+++ b/bl2/ext/mcuboot/bootutil/src/image_validate.c
@@ -56,9 +56,11 @@
uint8_t *hash_result, uint8_t *seed, int seed_len)
{
bootutil_sha256_context sha256_ctx;
- uint32_t blk_sz;
uint32_t size;
+#ifndef MCUBOOT_RAM_LOADING
+ uint32_t blk_sz;
uint32_t off;
+#endif /* MCUBOOT_RAM_LOADING */
bootutil_sha256_init(&sha256_ctx);
@@ -79,26 +81,20 @@
size += hdr->ih_protect_tlv_size;
}
+#ifdef MCUBOOT_RAM_LOADING
+ bootutil_sha256_update(&sha256_ctx,(void*)(hdr->ih_load_addr), size);
+#else
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_RAM_LOADING
- if (fap == NULL) { /* The image is in SRAM */
- memcpy(tmp_buf, (uint32_t *)(hdr->ih_load_addr + off), blk_sz);
- } else { /* The image is in flash */
-#endif
- if(flash_area_read(fap, off, tmp_buf, blk_sz)) {
- return -1;
- }
-#ifdef MCUBOOT_RAM_LOADING
+ if(flash_area_read(fap, off, tmp_buf, blk_sz)) {
+ return -1;
}
-#endif
-
bootutil_sha256_update(&sha256_ctx, tmp_buf, blk_sz);
}
+#endif
bootutil_sha256_finish(&sha256_ctx, hash_result);
return 0;
@@ -178,89 +174,6 @@
#endif
#endif
-#ifdef MCUBOOT_RAM_LOADING
-/* Check the hash of an image after it has been copied to SRAM */
-int
-bootutil_check_hash_after_loading(struct image_header *hdr)
-{
- uint32_t off;
- uint32_t end;
- int sha256_valid = 0;
- struct image_tlv_info info;
- struct image_tlv tlv;
- uint8_t tmp_buf[BOOT_TMPBUF_SZ];
- uint8_t hash[32] = {0};
- int rc;
- uint32_t load_address;
- uint32_t tlv_sz;
-
- rc = bootutil_img_hash(hdr, NULL, tmp_buf, BOOT_TMPBUF_SZ, hash, NULL, 0);
-
- if (rc) {
- return rc;
- }
-
- load_address = (uint32_t) hdr->ih_load_addr;
-
- /* The TLVs come after the image. */
- off = hdr->ih_img_size + hdr->ih_hdr_size;
-
- info = *((struct image_tlv_info *)(load_address + off));
-
- if (info.it_magic != IMAGE_TLV_INFO_MAGIC) {
- return BOOT_EBADMAGIC;
- }
- if (boot_add_uint32_overflow_check(off, (info.it_tlv_tot + sizeof(info)))) {
- return -1;
- }
- end = off + info.it_tlv_tot;
- off += sizeof(info);
-
- /*
- * Traverse through all of the TLVs, performing any checks we know
- * and are able to do.
- */
- if (boot_add_uint32_overflow_check(load_address, end)) {
- return -1;
- }
- while (off < end) {
- tlv = *((struct image_tlv *)(load_address + off));
- tlv_sz = sizeof(tlv);
-
- if (tlv.it_type == IMAGE_TLV_SHA256) {
- /*
- * Verify the SHA256 image hash. This must always be present.
- */
- if (tlv.it_len != sizeof(hash)) {
- return -1;
- }
-
- if (boot_secure_memequal(hash, (uint32_t *)(load_address + off + tlv_sz),
- sizeof(hash))) {
- return -1;
- }
-
- sha256_valid = 1;
- break;
- }
-
- /* Avoid integer overflow. */
- if (boot_add_uint32_overflow_check(off, (sizeof(tlv) + tlv.it_len))) {
- /* Potential overflow. */
- break;
- } else {
- off += sizeof(tlv) + tlv.it_len;
- }
- }
-
- if (!sha256_valid) {
- return -1;
- }
-
- return 0;
-}
-#endif /* MCUBOOT_RAM_LOADING */
-
/**
* Reads the value of an image's security counter.
*
@@ -294,7 +207,7 @@
off = hdr->ih_hdr_size + hdr->ih_img_size;
/* The TLV area always starts with an image_tlv_info structure. */
- rc = flash_area_read(fap, off, &info, sizeof(info));
+ rc = LOAD_IMAGE_DATA(fap, off, &info, sizeof(info));
if (rc != 0) {
return BOOT_EFLASH;
}
@@ -312,7 +225,7 @@
* security counter TLV.
*/
while (off < end) {
- rc = flash_area_read(fap, off, &tlv, sizeof(tlv));
+ rc = LOAD_IMAGE_DATA(fap, off, &tlv, sizeof(tlv));
if (rc != 0) {
return BOOT_EFLASH;
}
@@ -324,7 +237,7 @@
break;
}
- rc = flash_area_read(fap, off + sizeof(tlv),
+ rc = LOAD_IMAGE_DATA(fap, off + sizeof(tlv),
img_security_cnt, tlv.it_len);
if (rc != 0) {
return BOOT_EFLASH;
@@ -395,7 +308,7 @@
/* The TLVs come after the image. */
off = hdr->ih_img_size + hdr->ih_hdr_size;
- rc = flash_area_read(fap, off, &info, sizeof(info));
+ rc = LOAD_IMAGE_DATA(fap, off, &info, sizeof(info));
if (rc) {
return rc;
}
@@ -413,7 +326,7 @@
* and are able to do.
*/
while (off < end) {
- rc = flash_area_read(fap, off, &tlv, sizeof(tlv));
+ rc = LOAD_IMAGE_DATA(fap, off, &tlv, sizeof(tlv));
if (rc) {
return rc;
}
@@ -426,7 +339,7 @@
if (tlv.it_len != sizeof(hash)) {
return -1;
}
- rc = flash_area_read(fap, off + sizeof(tlv), buf, sizeof(hash));
+ rc = LOAD_IMAGE_DATA(fap, off + sizeof(tlv), buf, sizeof(hash));
if (rc) {
return rc;
}
@@ -444,7 +357,7 @@
if (tlv.it_len > 32) {
return -1;
}
- rc = flash_area_read(fap, off + sizeof(tlv), buf, tlv.it_len);
+ rc = LOAD_IMAGE_DATA(fap, off + sizeof(tlv), buf, tlv.it_len);
if (rc) {
return rc;
}
@@ -461,7 +374,7 @@
if (tlv.it_len > sizeof(key_buf)) {
return -1;
}
- rc = flash_area_read(fap, off + sizeof(tlv), key_buf, tlv.it_len);
+ rc = LOAD_IMAGE_DATA(fap, off + sizeof(tlv), key_buf, tlv.it_len);
if (rc) {
return rc;
}
@@ -477,7 +390,7 @@
if (!EXPECTED_SIG_LEN(tlv.it_len) || tlv.it_len > sizeof(buf)) {
return -1;
}
- rc = flash_area_read(fap, off + sizeof(tlv), buf, tlv.it_len);
+ rc = LOAD_IMAGE_DATA(fap, off + sizeof(tlv), buf, tlv.it_len);
if (rc) {
return -1;
}
@@ -499,7 +412,7 @@
return -1;
}
- rc = flash_area_read(fap, off + sizeof(tlv),
+ rc = LOAD_IMAGE_DATA(fap, off + sizeof(tlv),
&img_security_cnt, tlv.it_len);
if (rc) {
return rc;
diff --git a/bl2/ext/mcuboot/bootutil/src/loader.c b/bl2/ext/mcuboot/bootutil/src/loader.c
index 0d741ff..744ccc7 100644
--- a/bl2/ext/mcuboot/bootutil/src/loader.c
+++ b/bl2/ext/mcuboot/bootutil/src/loader.c
@@ -2350,7 +2350,7 @@
* @param slot The flash slot of the image to be copied to SRAM.
*
* @param hdr Pointer to the image header structure of the image
- * that needs to be copid to SRAM
+ * that needs to be copied to SRAM
*
* @return 0 on success; nonzero on failure.
*/
@@ -2406,6 +2406,34 @@
}
return rc;
}
+
+/**
+ * Removes an image from SRAM, by overwriting it with zeros.
+ *
+ * @param slot The flash slot of the image to be removed from SRAM.
+ *
+ * @param hdr Pointer to the image header structure of the image
+ * that needs to be removed from SRAM
+ *
+ * @return 0 on success; nonzero on failure.
+ */
+static int
+boot_remove_image_from_sram(int slot, struct image_header *hdr)
+{
+ uint32_t dst = (uint32_t) hdr->ih_load_addr;
+ int rc;
+ uint32_t img_sz;
+
+ rc = boot_read_image_size(slot, hdr, &img_sz);
+ if (rc != 0) {
+ return BOOT_EFLASH;
+ }
+
+ BOOT_LOG_INF("Removing image from SRAM at address 0x%x", dst);
+ memset((void*)dst, 0, img_sz);
+
+ return 0;
+}
#endif /* MCUBOOT_RAM_LOADING */
/**
@@ -2425,7 +2453,10 @@
int fa_id;
uint32_t boot_sequence[BOOT_NUM_SLOTS];
uint32_t img_cnt;
- struct image_header *newest_image_header;
+ struct image_header *selected_image_header;
+#ifdef MCUBOOT_RAM_LOADING
+ int image_copied = 0;
+#endif /* MCUBOOT_RAM_LOADING */
static boot_sector_t primary_slot_sectors[BOOT_MAX_IMG_SECTORS];
static boot_sector_t secondary_slot_sectors[BOOT_MAX_IMG_SECTORS];
@@ -2460,11 +2491,57 @@
if (img_cnt) {
/* Authenticate images */
for (i = 0; i < img_cnt; i++) {
- rc = boot_validate_slot(boot_sequence[i], NULL);
+
+ slot = boot_sequence[i];
+ selected_image_header = boot_img_hdr(&boot_data, slot);
+
+#ifdef MCUBOOT_RAM_LOADING
+ if (selected_image_header->ih_flags & IMAGE_F_RAM_LOAD) {
+ /* Copy image to the load address from where it
+ * currently resides in flash
+ */
+ rc = boot_copy_image_to_sram(slot, selected_image_header);
+ if (rc != 0) {
+ rc = BOOT_EBADIMAGE;
+ BOOT_LOG_INF("Could not copy image from the %s slot in "
+ "the Flash to load address 0x%x in SRAM, "
+ "aborting..", (slot == BOOT_PRIMARY_SLOT) ?
+ "primary" : "secondary",
+ selected_image_header->ih_load_addr);
+ continue;
+ } else {
+ BOOT_LOG_INF("Image has been copied from the %s slot in "
+ "the flash to SRAM address 0x%x",
+ (slot == BOOT_PRIMARY_SLOT) ?
+ "primary" : "secondary",
+ selected_image_header->ih_load_addr);
+ image_copied = 1;
+ }
+ } else {
+ /* Only images that support IMAGE_F_RAM_LOAD are allowed if
+ * MCUBOOT_RAM_LOADING is set.
+ */
+ rc = BOOT_EBADIMAGE;
+ continue;
+ }
+#endif /* MCUBOOT_RAM_LOADING */
+ rc = boot_validate_slot(slot, NULL);
if (rc == 0) {
- slot = boot_sequence[i];
+ /* If a valid image is found then there is no reason to check
+ * the rest of the images, as they were already ordered by
+ * preference.
+ */
break;
}
+#ifdef MCUBOOT_RAM_LOADING
+ else if (image_copied) {
+ /* If an image is found to be invalid then it is removed from
+ * RAM to prevent it being a shellcode vector.
+ */
+ boot_remove_image_from_sram(slot, selected_image_header);
+ image_copied = 0;
+ }
+#endif /* MCUBOOT_RAM_LOADING */
}
if (rc) {
/* If there was no valid image at all */
@@ -2472,60 +2549,26 @@
goto out;
}
- /* The slot variable now refers to the newest image's slot in flash */
- newest_image_header = boot_img_hdr(&boot_data, slot);
-
/* Update the security counter with the newest image's security
* counter value.
*/
- rc = boot_update_security_counter(slot, newest_image_header);
+ rc = boot_update_security_counter(slot, selected_image_header);
if (rc != 0) {
BOOT_LOG_ERR("Security counter update failed after image "
"validation.");
goto out;
}
- #ifdef MCUBOOT_RAM_LOADING
- if (newest_image_header->ih_flags & IMAGE_F_RAM_LOAD) {
- /* Copy image to the load address from where it
- * currently resides in flash */
- rc = boot_copy_image_to_sram(slot, newest_image_header);
- if (rc != 0) {
- rc = BOOT_EBADIMAGE;
- BOOT_LOG_INF("Could not copy image from the %s slot in "
- "the Flash to load address 0x%x in SRAM, "
- "aborting..", (slot == BOOT_PRIMARY_SLOT) ?
- "primary" : "secondary",
- newest_image_header->ih_load_addr);
- goto out;
- } else {
- BOOT_LOG_INF("Image has been copied from the %s slot in "
- "the flash to SRAM address 0x%x",
- (slot == BOOT_PRIMARY_SLOT) ?
- "primary" : "secondary",
- newest_image_header->ih_load_addr);
- }
- /* Validate the image hash in SRAM after the copy was successful */
- rc = bootutil_check_hash_after_loading(newest_image_header);
- if (rc != 0) {
- rc = BOOT_EBADIMAGE;
- BOOT_LOG_INF("Cannot validate the hash of the image that was "
- "copied to SRAM, aborting..");
- goto out;
- }
-
- BOOT_LOG_INF("Booting image from SRAM at address 0x%x",
- newest_image_header->ih_load_addr);
- } else {
-#endif /* MCUBOOT_RAM_LOADING */
- BOOT_LOG_INF("Booting image from the %s slot",
- (slot == BOOT_PRIMARY_SLOT) ? "primary" : "secondary");
#ifdef MCUBOOT_RAM_LOADING
- }
-#endif
+ BOOT_LOG_INF("Booting image from SRAM at address 0x%x",
+ selected_image_header->ih_load_addr);
+#else
+ BOOT_LOG_INF("Booting image from the %s slot",
+ (slot == BOOT_PRIMARY_SLOT) ? "primary" : "secondary");
+#endif /* MCUBOOT_RAM_LOADING */
- rsp->br_hdr = newest_image_header;
+ rsp->br_hdr = selected_image_header;
rsp->br_image_off = boot_img_slot_off(&boot_data, slot);
rsp->br_flash_dev_id = BOOT_IMG_AREA(&boot_data, slot)->fa_device_id;
} else {