From 7347f334b34067070646eed3986c67fddf0cd29e Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Thu, 21 Nov 2024 14:28:40 +0100 Subject: [PATCH] Delta update: check sha digest of base image Only allow delta update if the sha digest of the base image matches the expected value. This is to prevent the delta update from being applied to an image that is not the expected base image, even if the version matches. The mismatch should never happen in real-life scenarios, but it is to check for false positives during integration, testing and development. --- include/delta.h | 3 +- include/wolfboot/wolfboot.h | 1 + src/libwolfboot.c | 5 +- src/update_flash.c | 35 +++++++++++- tools/keytools/sign.c | 110 +++++++++++++++++++++++++++++++++--- 5 files changed, 140 insertions(+), 14 deletions(-) diff --git a/include/delta.h b/include/delta.h index cd3819287..c60b85d01 100644 --- a/include/delta.h +++ b/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 diff --git a/include/wolfboot/wolfboot.h b/include/wolfboot/wolfboot.h index 00131bc07..8a9f99e82 100644 --- a/include/wolfboot/wolfboot.h +++ b/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 diff --git a/src/libwolfboot.c b/src/libwolfboot.c index e078b39dc..93e27b19f 100644 --- a/src/libwolfboot.c +++ b/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 diff --git a/src/update_flash.c b/src/update_flash.c index 81062493a..dff574a01 100644 --- a/src/update_flash.c +++ b/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,34 @@ 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) { + wolfBoot_printf("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 +371,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 && 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); diff --git a/tools/keytools/sign.c b/tools/keytools/sign.c index ff492aa05..0e38485cc 100644 --- a/tools/keytools/sign.c +++ b/tools/keytools/sign.c @@ -48,6 +48,7 @@ #include #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 @@ -316,6 +318,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 +1116,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 +1195,40 @@ 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); + + /* 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,18 +1772,19 @@ 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, @@ -1709,7 +1792,7 @@ static int make_hybrid_header(uint8_t *pubkey, uint32_t pubkey_sz, 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 +1817,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,7 +1875,6 @@ 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; @@ -1798,6 +1882,14 @@ static int base_diff(const char *f_base, uint8_t *pubkey, uint32_t pubkey_sz, in 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 +2044,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 */