From 67336672bddb761b7f448b10c44f930655f6686e Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 26 Jan 2017 16:18:58 +1100 Subject: [PATCH] OTA: Improve verification of OTA image before writing, incl. secure boot Verify 0xE9 magic byte on first write, verify entire image before switching. Enable verification for secure boot signature (was using invalid ifdef guard) --- components/app_update/esp_ota_ops.c | 25 +++++++++++++++++-------- examples/system/ota/main/ota.c | 19 ++++++------------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/components/app_update/esp_ota_ops.c b/components/app_update/esp_ota_ops.c index 8b1a43163e..23b6f870c6 100644 --- a/components/app_update/esp_ota_ops.c +++ b/components/app_update/esp_ota_ops.c @@ -106,6 +106,7 @@ esp_err_t esp_ota_begin(const esp_partition_t *partition, size_t image_size, esp esp_err_t esp_ota_write(esp_ota_handle_t handle, const void *data, size_t size) { + const uint8_t *data_bytes = (const uint8_t *)data; esp_err_t ret; ota_ops_entry_t *it; @@ -119,6 +120,12 @@ esp_err_t esp_ota_write(esp_ota_handle_t handle, const void *data, size_t size) if (it->handle == handle) { // must erase the partition before writing to it assert(it->erased_size > 0 && "must erase the partition before writing to it"); + + if(it->wrote_size == 0 && size > 0 && data_bytes[0] != 0xE9) { + ESP_LOGE(TAG, "OTA image has invalid magic byte (expected 0xE9, saw 0x%02x", data_bytes[0]); + return ESP_ERR_OTA_VALIDATE_FAILED; + } + ret = esp_partition_write(&it->part, it->wrote_size, data, size); if(ret == ESP_OK){ it->wrote_size += size; @@ -135,6 +142,7 @@ esp_err_t esp_ota_write(esp_ota_handle_t handle, const void *data, size_t size) esp_err_t esp_ota_end(esp_ota_handle_t handle) { ota_ops_entry_t *it; + size_t image_size; for (it = LIST_FIRST(&s_ota_ops_entries_head); it != NULL; it = LIST_NEXT(it, entries)) { if (it->handle == handle) { // an ota handle need to be ended after erased and wrote data in it @@ -142,12 +150,12 @@ esp_err_t esp_ota_end(esp_ota_handle_t handle) return ESP_ERR_INVALID_ARG; } -#ifdef CONFIG_SECUREBOOTLOADER - esp_err_t ret; - size_t image_size; - if (esp_image_basic_verify(it->part.address, &image_size) != ESP_OK) { + if (esp_image_basic_verify(it->part.address, true, &image_size) != ESP_OK) { return ESP_ERR_OTA_VALIDATE_FAILED; } + +#ifdef CONFIG_SECURE_BOOT_ENABLED + esp_err_t ret; ret = esp_secure_boot_verify_signature(it->part.address, image_size); if (ret != ESP_OK) { return ESP_ERR_OTA_VALIDATE_FAILED; @@ -285,17 +293,18 @@ static esp_err_t esp_rewrite_ota_data(esp_partition_subtype_t subtype) esp_err_t esp_ota_set_boot_partition(const esp_partition_t *partition) { + size_t image_size; const esp_partition_t *find_partition = NULL; if (partition == NULL) { return ESP_ERR_INVALID_ARG; } -#ifdef CONFIG_SECUREBOOTLOADER - size_t image_size; - if (esp_image_basic_verify(partition->address, &image_size) != ESP_OK) { + if (esp_image_basic_verify(partition->address, true, &image_size) != ESP_OK) { return ESP_ERR_OTA_VALIDATE_FAILED; } - ret = esp_secure_boot_verify_signature(partition->address, image_size); + +#ifdef CONFIG_SECURE_BOOT_ENABLED + esp_err_t ret = esp_secure_boot_verify_signature(partition->address, image_size); if (ret != ESP_OK) { return ESP_ERR_OTA_VALIDATE_FAILED; } diff --git a/examples/system/ota/main/ota.c b/examples/system/ota/main/ota.c index 4956129e5d..21ebcc3c21 100644 --- a/examples/system/ota/main/ota.c +++ b/examples/system/ota/main/ota.c @@ -95,7 +95,7 @@ static void initialise_wifi(void) } /*read buffer by byte still delim ,return read bytes counts*/ -int read_until(char *buffer, char delim, int len) +static int read_until(char *buffer, char delim, int len) { // /*TODO: delim check,buffer check,further: do an buffer length limited*/ int i = 0; @@ -109,7 +109,7 @@ int read_until(char *buffer, char delim, int len) * return true if packet including \r\n\r\n that means http packet header finished,start to receive packet body * otherwise return false * */ -bool resolve_pkg(char text[], int total_len, esp_ota_handle_t out_handle) +static bool read_past_http_header(char text[], int total_len, esp_ota_handle_t out_handle) { /* i means current position */ int i = 0, i_read_len = 0; @@ -121,13 +121,6 @@ bool resolve_pkg(char text[], int total_len, esp_ota_handle_t out_handle) memset(ota_write_data, 0, BUFFSIZE); /*copy first http packet body to write buffer*/ memcpy(ota_write_data, &(text[i + 2]), i_write_len); - /*check write packet header first byte:0xE9 second byte:0x09 */ - if (ota_write_data[0] == 0xE9 && i_write_len >= 2 && ota_write_data[1] == 0x09) { - ESP_LOGI(TAG, "OTA Write Header format Check OK. first byte is %02x ,second byte is %02x", ota_write_data[0], ota_write_data[1]); - } else { - ESP_LOGE(TAG, "OTA Write Header format Check Failed! first byte is %02x ,second byte is %02x", ota_write_data[0], ota_write_data[1]); - return false; - } esp_err_t err = esp_ota_write( out_handle, (const void *)ota_write_data, i_write_len); if (err != ESP_OK) { @@ -266,7 +259,7 @@ void main_task(void *pvParameter) task_fatal_error(); } - bool pkg_body_start = false, flag = true; + bool resp_body_start = false, flag = true; /*deal with all receive packet*/ while (flag) { memset(text, 0, TEXT_BUFFSIZE); @@ -275,10 +268,10 @@ void main_task(void *pvParameter) if (buff_len < 0) { /*receive error*/ ESP_LOGE(TAG, "Error: receive data error! errno=%d", errno); task_fatal_error(); - } else if (buff_len > 0 && !pkg_body_start) { /*deal with packet header*/ + } else if (buff_len > 0 && !resp_body_start) { /*deal with response header*/ memcpy(ota_write_data, text, buff_len); - pkg_body_start = resolve_pkg(text, buff_len, out_handle); - } else if (buff_len > 0 && pkg_body_start) { /*deal with packet body*/ + resp_body_start = read_past_http_header(text, buff_len, out_handle); + } else if (buff_len > 0 && resp_body_start) { /*deal with response body*/ memcpy(ota_write_data, text, buff_len); err = esp_ota_write( out_handle, (const void *)ota_write_data, buff_len); if (err != ESP_OK) {