diff --git a/.github/workflows/test-powerfail-simulator.yml b/.github/workflows/test-powerfail-simulator.yml index 7d2f11e05..3892dc19f 100644 --- a/.github/workflows/test-powerfail-simulator.yml +++ b/.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 diff --git a/config/examples/sim-delta-update.config b/config/examples/sim-delta-update.config index 8992b91c5..2cba7a249 100644 --- a/config/examples/sim-delta-update.config +++ b/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 diff --git a/config/examples/sim-encrypt-delta-nvm-writeonce-update.config b/config/examples/sim-encrypt-delta-nvm-writeonce-update.config index 0ea3ba0a3..ea27253ef 100644 --- a/config/examples/sim-encrypt-delta-nvm-writeonce-update.config +++ b/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 diff --git a/config/examples/sim-encrypt-delta-update.config b/config/examples/sim-encrypt-delta-update.config index 68a79dceb..a4209b19d 100644 --- a/config/examples/sim-encrypt-delta-update.config +++ b/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 diff --git a/docs/Signing.md b/docs/Signing.md index c0e989b65..1f9f81822 100644 --- a/docs/Signing.md +++ b/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) 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..20cc67cc0 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,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); diff --git a/tools/keytools/sign.c b/tools/keytools/sign.c index ff492aa05..77a3d4f4a 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 @@ -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,18 +1775,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 +1795,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 +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,7 +1878,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 +1885,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 +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; diff --git a/tools/test.mk b/tools/test.mk index 46c21781a..7ff4b3e9f 100644 --- a/tools/test.mk +++ b/tools/test.mk @@ -246,6 +246,14 @@ test-sim-internal-flash-with-delta-update: $$(($(WOLFBOOT_PARTITION_UPDATE_ADDRESS)-$(ARCH_FLASH_OFFSET))) test-app/image_v$(TEST_UPDATE_VERSION)_signed_diff.bin \ $$(($(WOLFBOOT_PARTITION_SWAP_ADDRESS)-$(ARCH_FLASH_OFFSET))) erased_sec.dd +test-sim-internal-flash-with-delta-update-no-base-sha: + make test-sim-internal-flash-with-update DELTA_UPDATE_OPTIONS="--no-base-sha --delta test-app/image_v1_signed.bin" + $(Q)$(BINASSEMBLE) internal_flash.dd \ + 0 wolfboot.bin \ + $$(($(WOLFBOOT_PARTITION_BOOT_ADDRESS) - $(ARCH_FLASH_OFFSET))) test-app/image_v1_signed.bin \ + $$(($(WOLFBOOT_PARTITION_UPDATE_ADDRESS)-$(ARCH_FLASH_OFFSET))) test-app/image_v$(TEST_UPDATE_VERSION)_signed_diff.bin \ + $$(($(WOLFBOOT_PARTITION_SWAP_ADDRESS)-$(ARCH_FLASH_OFFSET))) erased_sec.dd + test-sim-internal-flash-with-wrong-delta-update: make test-sim-internal-flash-with-update DELTA_UPDATE_OPTIONS="--delta test-app/image_v1_signed.bin" make test-sim-internal-flash-with-update DELTA_UPDATE_OPTIONS="--delta test-app/image_v2_signed.bin" TEST_UPDATE_VERSION=3