Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bootloader fails to load signed image with secure version enabled (IDFGH-4038) #5911

Closed
MartinTJDK opened this issue Sep 24, 2020 · 7 comments

Comments

@MartinTJDK
Copy link

When programming a module with a bootloader supporting encrypted flash, and support for secure version, the bootloader stops accepting the image (on the first boot, where flash is encrypted).
When using a different bootloader (without secure flash enabled), the exact same application is loaded as expected.
What could cause the error "bootloader_flash: bootloader_mmap excess size XXXXX"?

Output from console:
I (60) boot: ESP-IDF v3.3-6-g76d93ca75 2nd stage bootloader
I (60) boot: compile time 14:36:22
I (60) boot: Enabling RNG early entropy source...
I (66) boot: SPI Speed : 80MHz
I (71) boot: SPI Mode : DIO
I (75) boot: SPI Flash Size : 8MB
I (79) boot: Partition Table:
I (82) boot: ## Label Usage Type ST Offset Length
I (90) boot: 0 conf_1 unknown 40 00 00012000 00003000
I (97) boot: 1 conf_2 unknown 40 01 00015000 00003000
I (104) boot: 2 otadata OTA data 01 00 00018000 00002000
I (112) boot: 3 phy_init RF data 01 01 0001a000 00001000
I (120) boot: 4 nvs_keys NVS keys 01 04 0001b000 00001000
I (127) boot: 5 nvs WiFi data 01 02 0001c000 00014000
I (135) boot: 6 ota_0 OTA app 00 10 000b0000 003a0000
I (142) boot: 7 ota_1 OTA app 00 11 00450000 003a0000
I (150) boot: 8 coredump Unknown data 01 03 007f0000 00010000
I (157) boot: End of partition table
I (162) boot: Enabled a check secure version of app for anti rollback
I (169) boot: Secure version (from eFuse) = 0
I (174) boot: otadata[0..1] in initial state
E (179) bootloader_flash: bootloader_mmap excess size 3a0000
E (185) boot_comm: bootloader_mmap(0xb0000, 0x3a0000) failed
E (191) boot: OTA app partition slot 0 is not bootable
E (197) bootloader_flash: bootloader_mmap excess size 3a0000
E (203) boot_comm: bootloader_mmap(0x450000, 0x3a0000) failed

@github-actions github-actions bot changed the title Bootloader fails to load signed image with secure version enabled Bootloader fails to load signed image with secure version enabled (IDFGH-4038) Sep 24, 2020
@negativekelvin
Copy link
Contributor

Because the bootloader tries to mmap the whole partition instead of just the image size.

#ifdef CONFIG_BOOTLOADER_APP_ANTI_ROLLBACK
esp_app_desc_t app_desc;
esp_err_t err = bootloader_common_get_partition_description(partition, &app_desc);
return err == ESP_OK && esp_efuse_check_secure_version(app_desc.secure_version) == true;
#else

@BitProgress
Copy link

Because the bootloader tries to mmap the whole partition instead of just the image size.

#ifdef CONFIG_BOOTLOADER_APP_ANTI_ROLLBACK
esp_app_desc_t app_desc;
esp_err_t err = bootloader_common_get_partition_description(partition, &app_desc);
return err == ESP_OK && esp_efuse_check_secure_version(app_desc.secure_version) == true;
#else

I am a little bit confused.
That behaviour is incorrect, right?
I have not checked for any updates to bootloader in newer esp-idf versions...

Thanks
Martin

@negativekelvin
Copy link
Contributor

I think so because there is no need to mmap whole partition to get app_desc and there are provisions elsewhere to deal with larger apps

/* Handling firmware images larger than MMU capacity */

@MartinTJDK
Copy link
Author

Could someone from espressif (@projectgus @igrr) comment on, if it is a bug, or just a wrong way of using secure boot and secure version?
I do not like to make any changes to such a vital part of the security chain.
Thanks

@mahavirj
Copy link
Member

@MartinTJDK This looks like an issue, especially with larger partition size (greater than 0x320000, 50 MMU pages) and anti-rollback feature. This is present on master branch as well. Can you please try following patch and see if it fixes the issue?

diff --git a/components/bootloader_support/src/bootloader_common.c b/components/bootloader_support/src/bootloader_common.c
index 729da47b2be..5236aa0a54a 100644
--- a/components/bootloader_support/src/bootloader_common.c
+++ b/components/bootloader_support/src/bootloader_common.c
@@ -183,13 +183,15 @@ esp_err_t bootloader_common_get_partition_description(const esp_partition_pos_t
         return ESP_ERR_INVALID_ARG;
     }
 
-    const uint8_t *image = bootloader_mmap(partition->offset, partition->size);
+    const uint32_t app_desc_offset = sizeof(esp_image_header_t) + sizeof(esp_image_segment_header_t);
+    const uint32_t mmap_size = app_desc_offset + sizeof(esp_app_desc_t);
+    const uint8_t *image = bootloader_mmap(partition->offset, mmap_size);
     if (image == NULL) {
-        ESP_LOGE(TAG, "bootloader_mmap(0x%x, 0x%x) failed", partition->offset, partition->size);
+        ESP_LOGE(TAG, "bootloader_mmap(0x%x, 0x%x) failed", partition->offset, mmap_size);
         return ESP_FAIL;
     }
 
-    memcpy(app_desc, image + sizeof(esp_image_header_t) + sizeof(esp_image_segment_header_t), sizeof(esp_app_desc_t));
+    memcpy(app_desc, image + app_desc_offset, sizeof(esp_app_desc_t));
     bootloader_munmap(image);
 
     if (app_desc->magic_word != ESP_APP_DESC_MAGIC_WORD) {

@MartinTJDK
Copy link
Author

@mahavirj I have tested the changed bootloader code, and it seem like it is working.
I will continue testing to see, if upgrade/downgrade works as expected.
Could you please write me back, when the fix is ready for merge on master.
We would like to use the exact same code, as you plan to put on master.
Thanks

@mahavirj
Copy link
Member

@MartinTJDK

Thank you for confirmation.

I have put this change in internal review queue and also linked this issue. This issue will get auto closed once fix reaches to github. There will be similar followup change for all previous release branches till release/v3.3.

MartinTJDK pushed a commit to Linak/esp-idf that referenced this issue Sep 29, 2020
espressif-bot pushed a commit that referenced this issue Oct 11, 2020
…iptor

For getting secure_version field in anti rollback case, bootloader tries
to map whole firmware partition but fails for cases where partition size
is beyond available MMU free pages capacity.

Fix here ensures to map only required length upto application descriptor
size in firmware partition.

Closes #5911
espressif-bot pushed a commit that referenced this issue Oct 18, 2020
…iptor

For getting secure_version field in anti rollback case, bootloader tries
to map whole firmware partition but fails for cases where partition size
is beyond available MMU free pages capacity.

Fix here ensures to map only required length upto application descriptor
size in firmware partition.

Closes #5911
espressif-bot pushed a commit that referenced this issue Oct 30, 2020
…iptor

For getting secure_version field in anti rollback case, bootloader tries
to map whole firmware partition but fails for cases where partition size
is beyond available MMU free pages capacity.

Fix here ensures to map only required length upto application descriptor
size in firmware partition.

Closes #5911
espressif-bot pushed a commit that referenced this issue Nov 19, 2020
…iptor

For getting secure_version field in anti rollback case, bootloader tries
to map whole firmware partition but fails for cases where partition size
is beyond available MMU free pages capacity.

Fix here ensures to map only required length upto application descriptor
size in firmware partition.

Closes #5911
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants