Skip to content

oneshot hash build option #736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bigbrett wants to merge 2 commits into
base: master
Choose a base branch
from
Open

oneshot hash build option #736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8aa838
Add oneshot hash build option
bigbrett Mar 23, 2026
edaa67c
add sanity checks on oneshot hash pointers
bigbrett Mar 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .github/workflows/test-elf-scattered.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,11 @@ jobs:
- name: Run bootloader with no arguments
run: |
./wolfboot.elf

- name: Build wolfboot.elf (ELF_SCATTERED, ONESHOT_HASH)
run: |
make clean && make test-sim-internal-flash-with-update ELF=1 ELF_SCATTERED=1 WOLFBOOT_IMG_HASH_ONESHOT=1

- name: Run bootloader with no arguments (ONESHOT_HASH)
run: |
./wolfboot.elf
24 changes: 24 additions & 0 deletions .github/workflows/test-sunnyday-simulator.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,30 @@ jobs:
run: |
tools/scripts/sim-sunnyday-update.sh

- name: Build wolfboot.elf (ECC256, ONESHOT_HASH, SHA256)
run: |
make clean && make test-sim-internal-flash-with-update SIGN=ECC256 SPMATH=1 WOLFBOOT_IMG_HASH_ONESHOT=1

- name: Run sunny day update test (ONESHOT_HASH, SHA256)
run: |
tools/scripts/sim-sunnyday-update.sh

- name: Build wolfboot.elf (ECC256, ONESHOT_HASH, SHA384)
run: |
make clean && make test-sim-internal-flash-with-update SIGN=ECC256 SPMATH=1 HASH=SHA384 WOLFBOOT_IMG_HASH_ONESHOT=1

- name: Run sunny day update test (ONESHOT_HASH, SHA384)
run: |
tools/scripts/sim-sunnyday-update.sh

- name: Build wolfboot.elf (ECC256, ONESHOT_HASH, SHA3)
run: |
make clean && make test-sim-internal-flash-with-update SIGN=ECC256 SPMATH=1 HASH=SHA3 WOLFBOOT_IMG_HASH_ONESHOT=1

- name: Run sunny day update test (ONESHOT_HASH, SHA3)
run: |
tools/scripts/sim-sunnyday-update.sh

- name: Cleanup to change key type
run: |
make keysclean
Expand Down
16 changes: 16 additions & 0 deletions docs/compile.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,22 @@ falls in one of these cases, wolfBoot compilation will terminate with an explici
In some cases you might have enough memory available to allow large stack allocations.
To circumvent the compile-time checks on the maximum allowed stack size, use `WOLFBOOT_HUGE_STACK=1`.

### One-shot hash verification

By default, wolfBoot hashes firmware images in blocks of `WOLFBOOT_SHA_BLOCK_SIZE` bytes during
verification. This block-by-block approach is required when firmware resides in external flash or
other non-memory-mapped storage, where data must be read through intermediate buffers.

When firmware images are stored in directly memory-mapped flash (e.g. internal flash with
execute-in-place support), the block-by-block overhead can be eliminated by enabling
`WOLFBOOT_IMG_HASH_ONESHOT=1`. With this option, the entire image buffer is passed to the wolfCrypt
hash function in a single call, which can improve verification performance.

**warning** This option assumes that `fw_base` pointers are directly dereferenceable for the full
firmware size. It is incompatible with `EXT_FLASH=1` configurations where partitions reside on
external SPI or UART flash. Only use `WOLFBOOT_IMG_HASH_ONESHOT=1` when all firmware partitions are
in directly addressable, memory-mapped flash.

### Disable Backup of current running firmware

Optionally, it is possible to disable the backup copy of the current running firmware upon the installation of the
Expand Down
4 changes: 4 additions & 0 deletions options.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,10 @@ ifeq ($(ARMORED),1)
CFLAGS+=-DWOLFBOOT_ARMORED
endif

ifeq ($(WOLFBOOT_IMG_HASH_ONESHOT),1)
CFLAGS+=-DWOLFBOOT_IMG_HASH_ONESHOT
endif

ifeq ($(WOLFBOOT_HUGE_STACK),1)
CFLAGS+=-DWOLFBOOT_HUGE_STACK
endif
Expand Down
185 changes: 122 additions & 63 deletions src/image.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -973,8 +973,7 @@ static int header_sha256(wc_Sha256 *sha256_ctx, struct wolfBoot_image *img)
{
uint8_t *stored_sha, *end_sha;
uint16_t stored_sha_len;
uint8_t *p;
int blksz;
uint8_t* p;
if (!img)
return -1;

Expand All @@ -988,13 +987,22 @@ static int header_sha256(wc_Sha256 *sha256_ctx, struct wolfBoot_image *img)
wc_InitSha256(sha256_ctx);
Copy link
Copy Markdown
Contributor

@dgarske dgarske Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [High] Hash context resource leak on ONESHOT error path in header_sha functions*
🚫 BLOCK bug

In all three header_sha* functions, the hash context is initialized (e.g. wc_InitSha256(sha256_ctx) at line 987) before the new ONESHOT guard if (end_sha <= p) return -1 at line 991-992. When this error path is taken, the hash context is initialized but never freed. The callers (image_sha256, etc.) return -1 immediately without calling wc_Sha*Free. This is a resource leak introduced by this PR.

The pre-existing error returns at lines 978 and 983 are safe because they occur before the init call. The non-ONESHOT path has no such early return after init — the while (p < end_sha) loop simply doesn't execute when p >= end_sha and returns 0.

This is especially concerning when WOLFBOOT_ENABLE_WOLFHSM_CLIENT is defined (line 984-985), as wc_InitSha256_ex(sha256_ctx, NULL, hsmDevIdHash) allocates HSM session resources that require explicit cleanup. The PR description explicitly mentions wolfHSM as a target use case.

Suggestion:

Suggested change
wc_InitSha256(sha256_ctx);
Move the ONESHOT guard before the init call, or add cleanup:
end_sha = stored_sha - (2 * sizeof(uint16_t));
#ifdef WOLFBOOT_IMG_HASH_ONESHOT
if (end_sha <= p)
return -1;
#endif
#ifdef WOLFBOOT_ENABLE_WOLFHSM_CLIENT
(void)wc_InitSha256_ex(sha256_ctx, NULL, hsmDevIdHash);
#else
wc_InitSha256(sha256_ctx);
#endif
Alternatively, call wc_Sha256Free before the error return. Apply the same fix to header_sha384 (line 1094-1098) and header_sha3_384 (line 1208-1212).

#endif
end_sha = stored_sha - (2 * sizeof(uint16_t)); /* Subtract 2 Type + 2 Len */
while (p < end_sha) {
blksz = WOLFBOOT_SHA_BLOCK_SIZE;
if (end_sha - p < blksz)
blksz = end_sha - p;
wc_Sha256Update(sha256_ctx, p, blksz);
p += blksz;
#ifdef WOLFBOOT_IMG_HASH_ONESHOT
Copy link
Copy Markdown
Contributor

@dgarske dgarske Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 [Low] Behavioral difference between ONESHOT and non-ONESHOT when end_sha <= p
🔧 NIT convention

In the non-ONESHOT path, when end_sha <= p (malformed/empty header region), the while (p < end_sha) loop simply doesn't execute and the function returns 0, proceeding to hash the firmware body. In the ONESHOT path, the same condition causes return -1, failing the entire verification.

While the ONESHOT behavior is arguably more correct (this condition indicates a corrupt header), the inconsistency means the same malformed image would pass verification without ONESHOT but fail with ONESHOT enabled. This could cause confusion during testing or field deployment when switching between the two modes.

Suggestion:

Suggested change
#ifdef WOLFBOOT_IMG_HASH_ONESHOT
For consistency, consider adding the same guard to the non-ONESHOT path, or documenting this behavioral difference.

if (end_sha <= p)
return -1;
wc_Sha256Update(sha256_ctx, p, (word32)(end_sha - p));
#else
{
int blksz;
while (p < end_sha) {
blksz = WOLFBOOT_SHA_BLOCK_SIZE;
if (end_sha - p < blksz)
blksz = end_sha - p;
wc_Sha256Update(sha256_ctx, p, blksz);
p += blksz;
}
}
#endif
return 0;
}

Expand All @@ -1007,23 +1015,31 @@ static int header_sha256(wc_Sha256 *sha256_ctx, struct wolfBoot_image *img)
*/
static int image_sha256(struct wolfBoot_image *img, uint8_t *hash)
{
uint32_t position = 0;
uint8_t *p;
int blksz;
wc_Sha256 sha256_ctx;

if (header_sha256(&sha256_ctx, img) != 0)
return -1;
do {
p = get_sha_block(img, position);
if (p == NULL)
break;
blksz = WOLFBOOT_SHA_BLOCK_SIZE;
if (position + blksz > img->fw_size)
blksz = img->fw_size - position;
wc_Sha256Update(&sha256_ctx, p, blksz);
position += blksz;
} while(position < img->fw_size);
#ifdef WOLFBOOT_IMG_HASH_ONESHOT
Copy link
Copy Markdown
Contributor

@dgarske dgarske Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [High] Hash context resource leak on ONESHOT fw_base NULL check in image_sha functions*
🚫 BLOCK bug

In image_sha256 (and the SHA384/SHA3 equivalents), when header_sha256 succeeds (context is initialized and has header data hashed into it), the subsequent ONESHOT guard if (img->fw_base == NULL) return -1 at line 1023-1024 returns without calling wc_Sha256Final or wc_Sha256Free. The same pattern repeats in image_sha384 (line 1133-1134) and image_sha3_384 (line 1245-1246).

This leaks the hash context resources. For HSM-backed crypto, this leaks a hardware session.

Suggestion:

Suggested change
#ifdef WOLFBOOT_IMG_HASH_ONESHOT
Add cleanup before the error return:
#ifdef WOLFBOOT_IMG_HASH_ONESHOT
if (img->fw_base == NULL) {
wc_Sha256Free(&sha256_ctx);
return -1;
}
wc_Sha256Update(&sha256_ctx, img->fw_base, img->fw_size);
Apply the same fix to image_sha384 and image_sha3_384.

if (img->fw_base == NULL)
return -1;
wc_Sha256Update(&sha256_ctx, img->fw_base, img->fw_size);
#else
{
uint32_t position = 0;
uint8_t* p;
int blksz;
do {
p = get_sha_block(img, position);
if (p == NULL)
break;
blksz = WOLFBOOT_SHA_BLOCK_SIZE;
if (position + blksz > img->fw_size)
blksz = img->fw_size - position;
wc_Sha256Update(&sha256_ctx, p, blksz);
position += blksz;
} while (position < img->fw_size);
}
#endif

wc_Sha256Final(&sha256_ctx, hash);
wc_Sha256Free(&sha256_ctx);
Expand Down Expand Up @@ -1064,8 +1080,7 @@ static int header_sha384(wc_Sha384 *sha384_ctx, struct wolfBoot_image *img)
{
uint16_t stored_sha_len;
uint8_t *stored_sha, *end_sha;
uint8_t *p;
int blksz;
uint8_t* p;
if (!img)
return -1;

Expand All @@ -1079,13 +1094,22 @@ static int header_sha384(wc_Sha384 *sha384_ctx, struct wolfBoot_image *img)
wc_InitSha384(sha384_ctx);
#endif
end_sha = stored_sha - (2 * sizeof(uint16_t)); /* Subtract 2 Type + 2 Len */
while (p < end_sha) {
blksz = WOLFBOOT_SHA_BLOCK_SIZE;
if (end_sha - p < blksz)
blksz = end_sha - p;
wc_Sha384Update(sha384_ctx, p, blksz);
p += blksz;
#ifdef WOLFBOOT_IMG_HASH_ONESHOT
if (end_sha <= p)
return -1;
wc_Sha384Update(sha384_ctx, p, (word32)(end_sha - p));
#else
{
int blksz;
while (p < end_sha) {
blksz = WOLFBOOT_SHA_BLOCK_SIZE;
if (end_sha - p < blksz)
blksz = end_sha - p;
wc_Sha384Update(sha384_ctx, p, blksz);
p += blksz;
}
}
#endif
return 0;
}

Expand All @@ -1101,23 +1125,31 @@ static int header_sha384(wc_Sha384 *sha384_ctx, struct wolfBoot_image *img)
*/
static int image_sha384(struct wolfBoot_image *img, uint8_t *hash)
{
uint32_t position = 0;
uint8_t *p;
int blksz;
wc_Sha384 sha384_ctx;

if (header_sha384(&sha384_ctx, img) != 0)
return -1;
do {
p = get_sha_block(img, position);
if (p == NULL)
break;
blksz = WOLFBOOT_SHA_BLOCK_SIZE;
if (position + blksz > img->fw_size)
blksz = img->fw_size - position;
wc_Sha384Update(&sha384_ctx, p, blksz);
position += blksz;
} while(position < img->fw_size);
#ifdef WOLFBOOT_IMG_HASH_ONESHOT
if (img->fw_base == NULL)
return -1;
wc_Sha384Update(&sha384_ctx, img->fw_base, img->fw_size);
#else
{
uint32_t position = 0;
uint8_t* p;
int blksz;
do {
p = get_sha_block(img, position);
if (p == NULL)
break;
blksz = WOLFBOOT_SHA_BLOCK_SIZE;
if (position + blksz > img->fw_size)
blksz = img->fw_size - position;
wc_Sha384Update(&sha384_ctx, p, blksz);
position += blksz;
} while (position < img->fw_size);
}
#endif

wc_Sha384Final(&sha384_ctx, hash);
wc_Sha384Free(&sha384_ctx);
Expand Down Expand Up @@ -1164,8 +1196,7 @@ static int header_sha3_384(wc_Sha3 *sha3_ctx, struct wolfBoot_image *img)
{
uint16_t stored_sha_len;
uint8_t *stored_sha, *end_sha;
uint8_t *p;
int blksz;
uint8_t* p;

if (!img)
return -1;
Expand All @@ -1176,13 +1207,22 @@ static int header_sha3_384(wc_Sha3 *sha3_ctx, struct wolfBoot_image *img)
return -1;
wc_InitSha3_384(sha3_ctx, NULL, INVALID_DEVID);
end_sha = stored_sha - (2 * sizeof(uint16_t)); /* Subtract 2 Type + 2 Len */
while (p < end_sha) {
blksz = WOLFBOOT_SHA_BLOCK_SIZE;
if (end_sha - p < blksz)
blksz = end_sha - p;
wc_Sha3_384_Update(sha3_ctx, p, blksz);
p += blksz;
#ifdef WOLFBOOT_IMG_HASH_ONESHOT
if (end_sha <= p)
return -1;
wc_Sha3_384_Update(sha3_ctx, p, (word32)(end_sha - p));
#else
{
int blksz;
while (p < end_sha) {
blksz = WOLFBOOT_SHA_BLOCK_SIZE;
if (end_sha - p < blksz)
blksz = end_sha - p;
wc_Sha3_384_Update(sha3_ctx, p, blksz);
p += blksz;
}
}
#endif
return 0;
}

Expand All @@ -1197,23 +1237,31 @@ static int header_sha3_384(wc_Sha3 *sha3_ctx, struct wolfBoot_image *img)
*/
static int image_sha3_384(struct wolfBoot_image *img, uint8_t *hash)
{
uint8_t *p;
int blksz;
uint32_t position = 0;
wc_Sha3 sha3_ctx;

if (header_sha3_384(&sha3_ctx, img) != 0)
return -1;
do {
p = get_sha_block(img, position);
if (p == NULL)
break;
blksz = WOLFBOOT_SHA_BLOCK_SIZE;
if (position + blksz > img->fw_size)
blksz = img->fw_size - position;
wc_Sha3_384_Update(&sha3_ctx, p, blksz);
position += blksz;
} while(position < img->fw_size);
#ifdef WOLFBOOT_IMG_HASH_ONESHOT
if (img->fw_base == NULL)
return -1;
wc_Sha3_384_Update(&sha3_ctx, img->fw_base, img->fw_size);
#else
{
uint8_t* p;
int blksz;
uint32_t position = 0;
do {
p = get_sha_block(img, position);
if (p == NULL)
break;
blksz = WOLFBOOT_SHA_BLOCK_SIZE;
if (position + blksz > img->fw_size)
blksz = img->fw_size - position;
wc_Sha3_384_Update(&sha3_ctx, p, blksz);
position += blksz;
} while (position < img->fw_size);
}
#endif

wc_Sha3_384_Final(&sha3_ctx, hash);
wc_Sha3_384_Free(&sha3_ctx);
Expand Down Expand Up @@ -1728,6 +1776,10 @@ static int update_hash_flash_fwimg(wolfBoot_hash_t* ctx,
struct wolfBoot_image* img, uint32_t offset,
uint32_t size)
{
#ifdef WOLFBOOT_IMG_HASH_ONESHOT
Copy link
Copy Markdown
Contributor

@dgarske dgarske Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [Medium] Missing NULL and bounds checks in update_hash_flash_fwimg ONESHOT path
💡 SUGGEST bug

The ONESHOT path in update_hash_flash_fwimg directly dereferences img->fw_base + offset without any validation:

  1. No NULL check on img->fw_base (unlike the image_sha* ONESHOT paths which check fw_base == NULL)
  2. No bounds check on offset + size vs img->fw_size (the non-ONESHOT path gets this via read_flash_fwimage which checks at line 1716)

The callers in wolfBoot_check_flash_image_elf compute offset/size from parsed ELF headers, so a malformed ELF could cause out-of-bounds reads. The non-ONESHOT path has defense-in-depth with bounds checking in read_flash_fwimage.

Suggestion:

Suggested change
#ifdef WOLFBOOT_IMG_HASH_ONESHOT
#ifdef WOLFBOOT_IMG_HASH_ONESHOT
if (img->fw_base == NULL)
return -1;
if ((uint64_t)offset + size > img->fw_size)
return -1;
update_hash(ctx, img->fw_base + offset, size);
return 0;

update_hash(ctx, img->fw_base + offset, size);
return 0;
#else
uint32_t current_offset = offset;
uint32_t remaining_size = size;
uint8_t read_buf[WOLFBOOT_SHA_BLOCK_SIZE] XALIGNED_STACK(4); /* Use local buffer */
Expand All @@ -1750,6 +1802,7 @@ static int update_hash_flash_fwimg(wolfBoot_hash_t* ctx,
current_offset += read_size;
}
return 0;
#endif
}

/*
Expand All @@ -1759,6 +1812,11 @@ static int update_hash_flash_fwimg(wolfBoot_hash_t* ctx,
static int update_hash_flash_addr(wolfBoot_hash_t* ctx, uintptr_t addr,
uint32_t size, int src_ext)
{
#ifdef WOLFBOOT_IMG_HASH_ONESHOT
Copy link
Copy Markdown
Contributor

@dgarske dgarske Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [Medium] update_hash_flash_addr ONESHOT path silently ignores src_ext flag
💡 SUGGEST bug

The ONESHOT path in update_hash_flash_addr casts addr to uint8_t* and passes it directly to update_hash, voiding the src_ext parameter. If WOLFBOOT_IMG_HASH_ONESHOT=1 is accidentally combined with an EXT_FLASH configuration (which the PR description acknowledges is logically incompatible but deliberately not compile-time guarded), this function will attempt to dereference an external flash address as if it were memory-mapped, leading to incorrect reads or a crash.

The caller at line 1988-1989 passes PART_IS_EXT(&boot) as src_ext. A runtime assertion or at least a debug warning would prevent silent misuse.

Suggestion:

Suggested change
#ifdef WOLFBOOT_IMG_HASH_ONESHOT
Consider adding a runtime check:
#ifdef WOLFBOOT_IMG_HASH_ONESHOT
if (src_ext) {
wolfBoot_printf("ERROR: ONESHOT hash incompatible with external flash\n");
return -1;
}
update_hash(ctx, (uint8_t*)addr, size);
return 0;

(void)src_ext;
update_hash(ctx, (uint8_t*)addr, size);
return 0;
#else
uint8_t buffer[WOLFBOOT_SHA_BLOCK_SIZE] XALIGNED_STACK(4);
uint32_t remaining_size = size;
uintptr_t current_addr = addr;
Expand All @@ -1783,6 +1841,7 @@ static int update_hash_flash_addr(wolfBoot_hash_t* ctx, uintptr_t addr,
}

return 0;
#endif
}

int wolfBoot_check_flash_image_elf(uint8_t part, unsigned long* entry_out)
Expand Down
3 changes: 2 additions & 1 deletion tools/config.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ ifeq ($(ARCH),)
WOLFBOOT_SMALL_STACK?=0
DELTA_UPDATES?=0
DELTA_BLOCK_SIZE?=256
WOLFBOOT_IMG_HASH_ONESHOT?=0
WOLFBOOT_HUGE_STACK?=0
ARMORED?=0
ELF?=0
Expand Down Expand Up @@ -107,7 +108,7 @@ CONFIG_VARS:= ARCH TARGET SIGN HASH MCUXSDK MCUXPRESSO MCUXPRESSO_CPU MCUXPRESSO
WOLFBOOT_PARTITION_BOOT_ADDRESS WOLFBOOT_PARTITION_UPDATE_ADDRESS \
WOLFBOOT_PARTITION_SWAP_ADDRESS WOLFBOOT_LOAD_ADDRESS \
WOLFBOOT_LOAD_DTS_ADDRESS WOLFBOOT_DTS_BOOT_ADDRESS WOLFBOOT_DTS_UPDATE_ADDRESS \
WOLFBOOT_SMALL_STACK DELTA_UPDATES DELTA_BLOCK_SIZE \
WOLFBOOT_SMALL_STACK DELTA_UPDATES DELTA_BLOCK_SIZE WOLFBOOT_IMG_HASH_ONESHOT \
WOLFBOOT_HUGE_STACK FORCE_32BIT\
ENCRYPT_WITH_CHACHA ENCRYPT_WITH_AES128 ENCRYPT_WITH_AES256 ARMORED \
LMS_LEVELS LMS_HEIGHT LMS_WINTERNITZ \
Expand Down
Loading