Merge pull request #520 from danielinux/delta-base-hash

Delta update: check sha digest of base image

Author: dgarske

.github/workflows/test-powerfail-simulator.yml

@@ -235,6 +235,14 @@ jobs:
         run: |
           tools/scripts/sim-update-powerfail-resume.sh
 
+      - name: Rebuild without SHA of base image to test compatibility
+        run: |
+          make clean && make test-sim-internal-flash-with-delta-update-no-base-sha
+
+      - name: Run sunny day update test (DELTA with no-base-sha)
+        run: |
+          tools/scripts/sim-sunnyday-update.sh
+
       - name: Rebuild with wrong delta base version
         run: |
           make clean && make test-sim-internal-flash-with-wrong-delta-update

config/examples/sim-delta-update.config

@@ -6,6 +6,7 @@ WOLFBOOT_SMALL_STACK=1
 SPI_FLASH=0
 DEBUG=1
 DELTA_UPDATES=1
+IMAGE_HEADER_SIZE=512
 
 # sizes should be multiple of system page size
 WOLFBOOT_PARTITION_SIZE=0x40000

config/examples/sim-encrypt-delta-nvm-writeonce-update.config

@@ -10,6 +10,7 @@ ENCRYPT_WITH_AES128=1
 DEBUG=1
 DELTA_UPDATES=1
 NVM_FLASH_WRITEONCE=1
+IMAGE_HEADER_SIZE=512
 
 # sizes should be multiple of system page size
 WOLFBOOT_PARTITION_SIZE=0x40000

config/examples/sim-encrypt-delta-update.config

@@ -9,6 +9,7 @@ ENCRYPT=1
 ENCRYPT_WITH_AES128=1
 DEBUG=1
 DELTA_UPDATES=1
+IMAGE_HEADER_SIZE=512
 
 # sizes should be multiple of system page size
 WOLFBOOT_PARTITION_SIZE=0x40000

docs/Signing.md

@@ -180,6 +180,15 @@ result is stored in a file ending in `_signed_diff.bin`.
 
 The compression scheme used is Bentley–McIlroy.
 
+Options:
+  * `--no-base-sha` : Avoid adding the sha of the base image to the manifest header.
+    By default, the sign tool appends the sha of the base image to the manifest header,
+    so wolfBoot will refuse to start a delta update if the sha does not match the
+    one of the existing image. However, this takes up 32 to 48 bytes extra in the
+    manifest header, so this option is available to provide compatibility on 
+    existing installations without this feature, where the header size does not
+    allow to accommodate the field
+
 
 #### Policy signing (for sealing/unsealing with a TPM)
 

include/delta.h

@@ -67,7 +67,8 @@ int wb_diff_init(WB_DIFF_CTX *ctx, uint8_t *src_a, uint32_t len_a, uint8_t *src_
 int wb_diff(WB_DIFF_CTX *ctx, uint8_t *patch, uint32_t len);
 int wb_patch_init(WB_PATCH_CTX *bm, uint8_t *src, uint32_t ssz, uint8_t *patch, uint32_t psz);
 int wb_patch(WB_PATCH_CTX *ctx, uint8_t *dst, uint32_t len);
-int wolfBoot_get_delta_info(uint8_t part, int inverse, uint32_t **img_offset, uint32_t **img_size);
+int wolfBoot_get_delta_info(uint8_t part, int inverse, uint32_t **img_offset,
+    uint32_t **img_size, uint8_t **base_hash, uint16_t *base_hash_size);
 
 #endif
 

include/wolfboot/wolfboot.h

@@ -66,6 +66,7 @@ extern "C" {
 #define HDR_IMG_TYPE                0x04
 #define HDR_IMG_DELTA_BASE          0x05
 #define HDR_IMG_DELTA_SIZE          0x06
+#define HDR_IMG_DELTA_BASE_HASH     0x07
 #define HDR_PUBKEY                  0x10
 #define HDR_SECONDARY_CIPHER        0x11
 #define HDR_SECONDARY_PUBKEY        0x12

src/libwolfboot.c

@@ -935,9 +935,8 @@ static inline uint16_t im2ns(uint16_t val)
  *
  */
 int wolfBoot_get_delta_info(uint8_t part, int inverse, uint32_t **img_offset,
-    uint16_t **img_size)
+    uint32_t **img_size, uint8_t **base_hash, uint16_t *base_hash_size)
 {
-    uint32_t *version_field = NULL;
     uint32_t *magic = NULL;
     uint8_t *image = (uint8_t *)0x00000000;
     if (part == PART_UPDATE) {
@@ -986,6 +985,8 @@ int wolfBoot_get_delta_info(uint8_t part, int inverse, uint32_t **img_offset,
             return -1;
         }
     }
+    *base_hash_size = wolfBoot_find_header((uint8_t *)(image + IMAGE_HEADER_OFFSET),
+            HDR_IMG_DELTA_BASE_HASH, base_hash);
     return 0;
 }
 #endif

src/update_flash.c

@@ -313,6 +313,10 @@ static int wolfBoot_delta_update(struct wolfBoot_image *boot,
     uint8_t nonce[ENCRYPT_NONCE_SIZE];
     uint8_t enc_blk[DELTA_BLOCK_SIZE];
 #endif
+    uint16_t delta_base_hash_sz;
+    uint8_t *delta_base_hash;
+    uint16_t base_hash_sz;
+    uint8_t *base_hash;
 
     /* Use biggest size for the swap */
     total_size = boot->fw_size + IMAGE_HEADER_SIZE;
@@ -327,12 +331,39 @@ static int wolfBoot_delta_update(struct wolfBoot_image *boot,
 #ifdef EXT_ENCRYPTED
     wolfBoot_get_encrypt_key(key, nonce);
 #endif
-    if (wolfBoot_get_delta_info(PART_UPDATE, inverse, &img_offset, &img_size) < 0) {
+    if (wolfBoot_get_delta_info(PART_UPDATE, inverse, &img_offset, &img_size,
+                &delta_base_hash, &delta_base_hash_sz) < 0) {
         return -1;
     }
     cur_v = wolfBoot_current_firmware_version();
     upd_v = wolfBoot_update_firmware_version();
     delta_base_v = wolfBoot_get_diffbase_version(PART_UPDATE);
+
+    if (delta_base_hash_sz != WOLFBOOT_SHA_DIGEST_SIZE) {
+        if (delta_base_hash_sz == 0) {
+            wolfBoot_printf("Warning: delta update: Base hash not found in image\n");
+            delta_base_hash = NULL;
+        } else {
+            wolfBoot_printf("Error: delta update: Base hash size mismatch"
+                    " (size: %x expected %x)\n", delta_base_hash_sz,
+                    WOLFBOOT_SHA_DIGEST_SIZE);
+            return -1;
+        }
+    }
+
+#if defined(WOLFBOOT_HASH_SHA256)
+    base_hash_sz = wolfBoot_find_header(boot->hdr + IMAGE_HEADER_OFFSET,
+            HDR_SHA256, &base_hash);
+#elif defined(WOLFBOOT_HASH_SHA384)
+    base_hash_sz = wolfBoot_find_header(boot->hdr + IMAGE_HEADER_OFFSET,
+            HDR_SHA384, &base_hash);
+#elif defined(WOLFBOOT_HASH_SHA3_384)
+    base_hash_sz = wolfBoot_find_header(boot->hdr + IMAGE_HEADER_OFFSET,
+            HDR_SHA3_384, &base_hash);
+#else
+    #error "Delta update: Fatal error, no hash algorithm defined!"
+#endif
+
     if (inverse) {
         if (((cur_v == upd_v) && (delta_base_v < cur_v)) || resume) {
             ret = wb_patch_init(&ctx, boot->hdr, boot->fw_size +
@@ -345,10 +376,15 @@ static int wolfBoot_delta_update(struct wolfBoot_image *boot,
         }
     } else {
         if (!resume && (cur_v != delta_base_v)) {
-            /* Wrong base image, cannot apply delta patch */
+            /* Wrong base image version, cannot apply delta patch */
             wolfBoot_printf("Delta Base 0x%x != Cur 0x%x\n",
                 cur_v, delta_base_v);
             ret = -1;
+        } else if (!resume && delta_base_hash &&
+                memcmp(base_hash, delta_base_hash, base_hash_sz) != 0) {
+            /* Wrong base image digest, cannot apply delta patch */
+            wolfBoot_printf("Delta Base hash mismatch\n");
+            ret = -1;
         } else {
             ret = wb_patch_init(&ctx, boot->hdr, boot->fw_size + IMAGE_HEADER_SIZE,
                     update->hdr + IMAGE_HEADER_SIZE, *img_size);

tools/keytools/sign.c

@@ -48,6 +48,7 @@
 #include <delta.h>
 
 #include "wolfboot/version.h"
+#include "wolfboot/wolfboot.h"
 
 #ifdef DEBUG_SIGNTOOL
 #define DEBUG_PRINT(...) fprintf(stderr, __VA_ARGS__)
@@ -176,6 +177,7 @@ static inline int fp_truncate(FILE *f, size_t len)
 
 #define HDR_IMG_DELTA_BASE 0x05
 #define HDR_IMG_DELTA_SIZE 0x06
+#define HDR_IMG_DELTA_BASE_HASH 0x07
 #define HDR_IMG_DELTA_INVERSE 0x15
 #define HDR_IMG_DELTA_INVERSE_SIZE 0x16
 
@@ -289,6 +291,7 @@ struct cmd_options {
     const char *policy_file;
     const char *encrypt_key_file;
     const char *delta_base_file;
+    int no_base_sha;
     char output_image_file[PATH_MAX];
     char output_diff_file[PATH_MAX];
     char output_encrypted_image_file[PATH_MAX];
@@ -316,6 +319,57 @@ static struct cmd_options CMD = {
     .hybrid = 0
 };
 
+static uint16_t sign_tool_find_header(uint8_t *haystack, uint16_t type, uint8_t **ptr)
+{
+    uint8_t *p = haystack;
+    uint16_t len, htype;
+    const volatile uint8_t *max_p = (haystack - IMAGE_HEADER_OFFSET) +
+                                                    IMAGE_HEADER_SIZE;
+    *ptr = NULL;
+    if (p > max_p) {
+        fprintf(stderr, "Illegal address (too high)\n");
+        return 0;
+    }
+    while ((p + 4) < max_p) {
+        htype = p[0] | (p[1] << 8);
+        if (htype == 0) {
+            fprintf(stderr, "Explicit end of options reached\n");
+            break;
+        }
+        /* skip unaligned half-words and padding bytes */
+        if ((p[0] == HDR_PADDING) || ((((size_t)p) & 0x01) != 0)) {
+            p++;
+            continue;
+        }
+
+        len = p[2] | (p[3] << 8);
+        /* check len */
+        if ((4 + len) > (uint16_t)(IMAGE_HEADER_SIZE - IMAGE_HEADER_OFFSET)) {
+            fprintf(stderr, "This field is too large (bigger than the space available "
+                     "in the current header)\n");
+            //fprintf(stderr, "%d %d %d\n", len, IMAGE_HEADER_SIZE, IMAGE_HEADER_OFFSET);
+            break;
+        }
+        /* check max pointer */
+        if (p + 4 + len > max_p) {
+            fprintf(stderr, "This field is too large and would overflow the image "
+                     "header\n");
+            break;
+        }
+
+        /* skip header [type|len] */
+        p += 4;
+
+        if (htype == type) {
+            /* found, return pointer to data portion */
+            *ptr = p;
+            return len;
+        }
+        p += len;
+    }
+    return 0;
+}
+
 static int load_key_ecc(int sign_type, uint32_t curve_sz, int curve_id,
     int header_sz,
     uint8_t **key_buffer, uint32_t *key_buffer_sz,
@@ -1063,7 +1117,8 @@ static int sign_digest(int sign, int hash_algo,
 static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz,
         const char *image_file, const char *outfile,
         uint32_t delta_base_version, uint32_t patch_len, uint32_t patch_inv_off,
-        uint32_t patch_inv_len, const uint8_t *secondary_key, uint32_t secondary_key_sz)
+        uint32_t patch_inv_len, const uint8_t *secondary_key, uint32_t secondary_key_sz,
+        uint8_t *base_hash, uint32_t base_hash_sz)
 {
     uint32_t header_idx;
     uint8_t *header;
@@ -1141,12 +1196,42 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz,
                 &patch_len);
 
         /* Append pad bytes, so fields are 4-byte aligned */
-        while ((header_idx % 4) != 0)
-            header_idx++;
+        ALIGN_4(header_idx);
         header_append_tag(header, &header_idx, HDR_IMG_DELTA_INVERSE, 4,
                 &patch_inv_off);
         header_append_tag(header, &header_idx, HDR_IMG_DELTA_INVERSE_SIZE, 4,
                 &patch_inv_len);
+
+        if (!CMD.no_base_sha) {
+            /* Append pad bytes, so base hash is 8-byte aligned */
+            ALIGN_8(header_idx);
+            if (!base_hash) {
+                fprintf(stderr, "Base hash for delta image not found.\n");
+                exit(1);
+            }
+            if (CMD.hash_algo == HASH_SHA256) {
+                if (base_hash_sz != HDR_SHA256_LEN) {
+                    fprintf(stderr, "Invalid base hash size for SHA256.\n");
+                    exit(1);
+                }
+                header_append_tag(header, &header_idx, HDR_IMG_DELTA_BASE_HASH,
+                        HDR_SHA256_LEN, base_hash);
+            } else if (CMD.hash_algo == HASH_SHA384) {
+                if  (base_hash_sz != HDR_SHA384_LEN) {
+                    fprintf(stderr, "Invalid base hash size for SHA384.\n");
+                    exit(1);
+                }
+                header_append_tag(header, &header_idx, HDR_IMG_DELTA_BASE_HASH,
+                        HDR_SHA384_LEN, base_hash);
+            } else if (CMD.hash_algo == HASH_SHA3) {
+                if (base_hash_sz != HDR_SHA3_384_LEN) {
+                    fprintf(stderr, "Invalid base hash size for SHA3-384.\n");
+                    exit(1);
+                }
+                header_append_tag(header, &header_idx, HDR_IMG_DELTA_BASE_HASH,
+                        HDR_SHA3_384_LEN, base_hash);
+            }
+        }
     }
 
     /* Add custom TLVs */
@@ -1690,26 +1775,27 @@ static int make_header(uint8_t *pubkey, uint32_t pubkey_sz,
         const char *image_file, const char *outfile)
 {
     return make_header_ex(0, pubkey, pubkey_sz, image_file, outfile, 0, 0, 0, 0,
-            NULL, 0);
+            NULL, 0, NULL, 0);
 }
 
 static int make_header_delta(uint8_t *pubkey, uint32_t pubkey_sz,
         const char *image_file, const char *outfile,
         uint32_t delta_base_version, uint32_t patch_len,
-        uint32_t patch_inv_off, uint32_t patch_inv_len)
+        uint32_t patch_inv_off, uint32_t patch_inv_len,
+        uint8_t *base_hash, uint32_t base_hash_sz)
 {
     return make_header_ex(1, pubkey, pubkey_sz, image_file, outfile,
             delta_base_version, patch_len,
             patch_inv_off, patch_inv_len,
-            NULL, 0);
+            NULL, 0, base_hash, base_hash_sz);
 }
 
 static int make_hybrid_header(uint8_t *pubkey, uint32_t pubkey_sz,
         const char *image_file, const char *outfile,
         const uint8_t *secondary_key, uint32_t secondary_key_sz)
 {
     return make_header_ex(0, pubkey, pubkey_sz, image_file, outfile, 0, 0, 0, 0,
-            secondary_key, secondary_key_sz);
+            secondary_key, secondary_key_sz, NULL, 0);
 }
 
 static int base_diff(const char *f_base, uint8_t *pubkey, uint32_t pubkey_sz, int padding)
@@ -1734,6 +1820,8 @@ static int base_diff(const char *f_base, uint8_t *pubkey, uint32_t pubkey_sz, in
     WB_DIFF_CTX diff_ctx;
     int ret = -1;
     int io_sz;
+    uint8_t *base_hash = NULL;
+    uint32_t base_hash_sz = 0;
 
     /* Get source file size */
     if (stat(f_base, &st) < 0) {
@@ -1790,14 +1878,21 @@ static int base_diff(const char *f_base, uint8_t *pubkey, uint32_t pubkey_sz, in
                 delta_base_version = (uint32_t)(retval&0xFFFFFFFF);
         }
     }
-
     if (delta_base_version == 0) {
         printf("Could not read firmware version from base file %s\n", f_base);
         goto cleanup;
     } else {
         printf("Delta base version: %u\n", delta_base_version);
     }
 
+    /* Retrieve the hash digest of the base image */
+    if (CMD.hash_algo == HASH_SHA256)
+        base_hash_sz = sign_tool_find_header(base + 8, HDR_SHA256, &base_hash);
+    else if (CMD.hash_algo == HASH_SHA384)
+        base_hash_sz = sign_tool_find_header(base + 8, HDR_SHA384, &base_hash);
+    else if (CMD.hash_algo == HASH_SHA3)
+        base_hash_sz = sign_tool_find_header(base + 8, HDR_SHA3_384, &base_hash);
+
 #if HAVE_MMAP
     /* Open second image file */
     fd2 = open(CMD.output_image_file, O_RDONLY);
@@ -1952,7 +2047,7 @@ static int base_diff(const char *f_base, uint8_t *pubkey, uint32_t pubkey_sz, in
     /* Create delta file, with header, from the resulting patch */
 
     ret = make_header_delta(pubkey, pubkey_sz, wolfboot_delta_file, CMD.output_diff_file,
-            delta_base_version, patch_sz, patch_inv_off, patch_inv_sz);
+            delta_base_version, patch_sz, patch_inv_off, patch_inv_sz, base_hash, base_hash_sz);
 
 cleanup:
     /* Unlink output file */
@@ -2398,6 +2493,8 @@ int main(int argc, char** argv)
         else if (strcmp(argv[i], "--delta") == 0) {
             CMD.delta = 1;
             CMD.delta_base_file = argv[++i];
+        } else if (strcmp(argv[i], "--no-base-sha") == 0) {
+            CMD.no_base_sha = 1;
         }
         else if (strcmp(argv[i], "--no-ts") == 0) {
             CMD.no_ts = 1;