Skip to content

Commit

Permalink
Delta update: check sha digest of base image
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
danielinux committed Nov 21, 2024
1 parent 6d1adc2 commit 7347f33
Show file tree
Hide file tree
Showing 5 changed files with 140 additions and 14 deletions.
3 changes: 2 additions & 1 deletion include/delta.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

1 change: 1 addition & 0 deletions include/wolfboot/wolfboot.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions src/libwolfboot.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
35 changes: 33 additions & 2 deletions src/update_flash.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 +
Expand All @@ -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);
Expand Down
110 changes: 101 additions & 9 deletions tools/keytools/sign.c
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -1690,26 +1772,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)
Expand All @@ -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) {
Expand Down Expand Up @@ -1790,14 +1875,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);
Expand Down Expand Up @@ -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 */
Expand Down

0 comments on commit 7347f33

Please sign in to comment.