From 51399347671fb706597cdbc0b76ed2f49d835238 Mon Sep 17 00:00:00 2001
From: Angus Gratton <angus@espressif.com>
Date: Thu, 28 Nov 2019 12:11:50 +1100
Subject: [PATCH] bootloader_common: Fix esp_partition_get_sha256(), add unit
 tests

Regression in 438d513a95a

Reported here: https://esp32.com/viewtopic.php?f=13&t=13250&p=52460
---
 .../bootloader_support/src/esp_image_format.c | 23 ++++----
 components/spi_flash/test/test_partitions.c   | 56 +++++++++++++++++++
 tools/ci/config/target-test.yml               |  2 +-
 3 files changed, 69 insertions(+), 12 deletions(-)

diff --git a/components/bootloader_support/src/esp_image_format.c b/components/bootloader_support/src/esp_image_format.c
index ea1932307..cfad3acf7 100644
--- a/components/bootloader_support/src/esp_image_format.c
+++ b/components/bootloader_support/src/esp_image_format.c
@@ -229,18 +229,19 @@ static esp_err_t image_load(esp_image_load_mode_t mode, const esp_partition_pos_
             if (sha_handle != NULL) {
                 bootloader_sha256_finish(sha_handle, NULL);
             }
-
-            if (data->image.hash_appended) {
-                const void *hash = bootloader_mmap(data->start_addr + data->image_len - HASH_LEN, HASH_LEN);
-                if (hash == NULL) {
-                    err = ESP_FAIL;
-                    goto err;
-                }
-                memcpy(data->image_digest, hash, HASH_LEN);
-                bootloader_munmap(hash);
-            }
             sha_handle = NULL;
-        } // verify_sha
+        } //verify_sha
+
+        // Separately, if there's a hash appended to the image then copy it out to the data->image_digest field
+        if (data->image.hash_appended) {
+            const void *hash = bootloader_mmap(data->start_addr + data->image_len - HASH_LEN, HASH_LEN);
+            if (hash == NULL) {
+                err = ESP_FAIL;
+                goto err;
+            }
+            memcpy(data->image_digest, hash, HASH_LEN);
+            bootloader_munmap(hash);
+        }
     } // do_verify
 
     if (err != ESP_OK) {
diff --git a/components/spi_flash/test/test_partitions.c b/components/spi_flash/test/test_partitions.c
index 705f0bed7..cd46f723c 100644
--- a/components/spi_flash/test/test_partitions.c
+++ b/components/spi_flash/test/test_partitions.c
@@ -22,6 +22,8 @@
 
 #include <unity.h>
 #include <test_utils.h>
+#include <esp_image_format.h>
+#include <esp_log.h>
 #include <esp_partition.h>
 #include <esp_attr.h>
 
@@ -66,3 +68,57 @@ TEST_CASE("Test erase partition", "[spi_flash][esp_flash]")
         }
     }
 }
+
+static bool s_test_nonzero_sha_of_partition(const esp_partition_t *part, bool allow_invalid_image)
+{
+    uint8_t sha256[32] = { 0 };
+
+    TEST_ASSERT_NOT_NULL(part);
+
+    esp_err_t err = esp_partition_get_sha256(part, sha256);
+
+    if (allow_invalid_image && err == ESP_ERR_IMAGE_INVALID) {
+        printf("App partition at 0x%x doesn't hold a valid app\n", part->address);
+        return false;
+    }
+
+    // Otherwise, err should be ESP_OK
+    ESP_ERROR_CHECK(err);
+
+    ESP_LOG_BUFFER_HEX("sha", sha256, sizeof(sha256));
+
+    for (int i = 0; i < sizeof(sha256); i++) {
+        if (sha256[i] != 0) {
+            return true; // At least one non-zero byte!
+        }
+    }
+    TEST_FAIL_MESSAGE("SHA-256 of data partition should not be all zeroes");
+    abort(); // unreachable
+}
+
+TEST_CASE("Test esp_partition_get_sha256() with data", "[spi_flash]")
+{
+    const esp_partition_t *part = get_test_data_partition();
+    s_test_nonzero_sha_of_partition(part, false);
+}
+
+TEST_CASE("Test esp_partition_get_sha256() with app", "[spi_flash]")
+{
+    bool found_valid_app = false;
+    esp_partition_iterator_t it = esp_partition_find(ESP_PARTITION_TYPE_APP,
+                                                     ESP_PARTITION_SUBTYPE_ANY,
+                                                     NULL);
+    TEST_ASSERT_NOT_NULL(it); /* has to be at least one app partition */
+
+    while (it != NULL) {
+        const esp_partition_t *part = esp_partition_get(it);
+        printf("Hashing app partition at 0x%x\n", part->address);
+        bool valid = s_test_nonzero_sha_of_partition(part, true);
+        found_valid_app |= valid;
+
+        it = esp_partition_next(it);
+    }
+
+    TEST_ASSERT_MESSAGE(found_valid_app, "At least one app partition should be a valid app partition");
+}
+
diff --git a/tools/ci/config/target-test.yml b/tools/ci/config/target-test.yml
index 827e31858..3e169f97a 100644
--- a/tools/ci/config/target-test.yml
+++ b/tools/ci/config/target-test.yml
@@ -453,7 +453,7 @@ UT_034:
 
 UT_035:
   extends: .unit_test_template
-  parallel: 30
+  parallel: 31
   tags:
     - ESP32S2BETA_IDF
     - UT_T1_1