From e6655e696c4e9fb27342b272dbc464e657f81b9e Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 11 Aug 2026 08:09:10 +0200 Subject: [PATCH 01/25] F-7989: clamp unaligned head size in ext_flash_decrypt_read() When a read starts in the middle of an encryption block, the head copy size was computed as ENCRYPT_BLOCK_SIZE - row_offset without regard for the requested length. A read shorter than the remainder of the block (e.g. 1 byte at offset 1) copied up to 15 decrypted bytes into a buffer sized for fewer, and left read_remaining negative, so the subsequent flash_read_size = read_remaining & ~(ENCRYPT_BLOCK_SIZE - 1) was passed to ext_flash_read() as a negative length. Clamp the head size to the bytes actually requested. Add a unit test covering short unaligned reads (offsets 1/4/8/15) that checks the return value, the decrypted contents and that no byte past the requested length is written; the ext_flash_read() mock now also rejects negative lengths. --- src/libwolfboot.c | 3 +++ tools/unit-tests/unit-extflash.c | 39 ++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index 62fa63e55c..63714a6445 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -2702,6 +2702,9 @@ int RAMFUNCTION ext_flash_decrypt_read(uintptr_t address, uint8_t *data, int len */ if (row_offset != 0) { unaligned_head_size = ENCRYPT_BLOCK_SIZE - row_offset; + /* Never copy more than the caller asked for */ + if (unaligned_head_size > read_remaining) + unaligned_head_size = read_remaining; if (ext_flash_read(row_address, block, ENCRYPT_BLOCK_SIZE) != ENCRYPT_BLOCK_SIZE) { return -1; diff --git a/tools/unit-tests/unit-extflash.c b/tools/unit-tests/unit-extflash.c index c25d7b8f7e..c4bfe5133c 100644 --- a/tools/unit-tests/unit-extflash.c +++ b/tools/unit-tests/unit-extflash.c @@ -96,6 +96,9 @@ uint8_t flash[FLASH_SIZE]; int ext_flash_read(uintptr_t address, uint8_t *data, int len) { printf("Called ext_flash_read %p %p %d\n", (void *)address, (void *)data, len); + /* A negative length is never a valid request */ + ck_assert_int_ge(len, 0); + /* Check that the read address and size are within the bounds of the flash memory */ ck_assert_int_le(address + len, FLASH_SIZE); @@ -260,6 +263,37 @@ START_TEST(test_ext_enc_flash_operations) { } END_TEST +START_TEST(test_ext_enc_flash_short_unaligned_read) { + uint32_t address = 0x1000; + uint32_t size = 64; + uint8_t data[64]; + uint8_t dataw[64]; + /* Reads shorter than the remainder of the encryption block they start in: + * { offset within the block, number of bytes requested } */ + static const int cases[][2] = { {1, 1}, {4, 4}, {8, 3}, {15, 1} }; + unsigned int c; + int i, rres, wres; + + memcpy(dataw, test_buffer, size); + wres = ext_flash_check_write(address, dataw, size); + ck_assert_int_eq(wres, 0); + + for (c = 0; c < sizeof(cases) / sizeof(cases[0]); c++) { + int off = cases[c][0]; + int len = cases[c][1]; + + memset(data, 0xA5, sizeof(data)); + rres = ext_flash_check_read(address + off, data, len); + ck_assert_int_eq(rres, len); + ck_assert_mem_eq(data, test_buffer + off, len); + + /* No byte past the requested length may be written */ + for (i = len; i < (int)sizeof(data); i++) + ck_assert_uint_eq(data[i], 0xA5); + } +} +END_TEST + Suite *wolfboot_suite(void) @@ -271,15 +305,20 @@ Suite *wolfboot_suite(void) /* Test cases */ TCase *ext_flash_operations = tcase_create("External flash operations: API"); TCase *ext_enc_flash_operations = tcase_create("External encrypted flash operations"); + TCase *ext_enc_flash_short_read = tcase_create("External encrypted flash short unaligned read"); /* Set parameters + add to suite */ tcase_add_test(ext_flash_operations, test_ext_flash_operations); tcase_add_test(ext_enc_flash_operations, test_ext_enc_flash_operations); + tcase_add_test(ext_enc_flash_short_read, + test_ext_enc_flash_short_unaligned_read); tcase_set_timeout(ext_flash_operations, 20); tcase_set_timeout(ext_enc_flash_operations, 20); + tcase_set_timeout(ext_enc_flash_short_read, 20); suite_add_tcase(s, ext_flash_operations); suite_add_tcase(s, ext_enc_flash_operations); + suite_add_tcase(s, ext_enc_flash_short_read); return s; } From 7608e333a85ce2715e87f73bc080f9c893577722 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 11 Aug 2026 08:14:12 +0200 Subject: [PATCH 02/25] F-7988: clamp unaligned head size in ext_flash_encrypt_write() When a write starts in the middle of an encryption block, or is shorter than a full block, the head copy size was computed as ENCRYPT_BLOCK_SIZE - row_offset without regard for the requested length. A write shorter than the remainder of the block (e.g. 1 byte at offset 1) copied up to ENCRYPT_BLOCK_SIZE-1 bytes out of the caller's buffer into the read-modify-write block, and left sz negative, so the subsequent step = sz & ~(ENCRYPT_BLOCK_SIZE - 1) was passed to ext_flash_write() as a negative length. This is reachable from wb_flash_write_verify_word() (4-byte writes), from wolfBoot_nsc_write_update() and from the delta patch writer. Clamp the head size to the bytes actually requested, and return the result of the head block write when the request fits within that block. Add a unit test covering short unaligned writes (offsets 1/0/8/blk-1) that checks the return value, the data read back and that the bytes past the requested length were not taken from the caller's buffer; the ext_flash_write() mock now also rejects negative lengths. --- src/libwolfboot.c | 10 ++++- tools/unit-tests/unit-extflash.c | 63 ++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index 63714a6445..aad3421045 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -2576,7 +2576,7 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data, uint8_t block[ENCRYPT_BLOCK_SIZE]; uint8_t enc_block[ENCRYPT_BLOCK_SIZE]; uint32_t row_address = address, row_offset; - int sz = len, i, step; + int sz = len, i, step, ret; uint8_t part; uint32_t iv_counter = 0; #if defined(EXT_ENCRYPTED) && !defined(WOLFBOOT_SMALL_STACK) && \ @@ -2616,13 +2616,19 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data, /* encrypt blocks */ if (sz > len) { step = ENCRYPT_BLOCK_SIZE - row_offset; + /* Never consume more than the caller provided */ + if (step > len) + step = len; if (ext_flash_read(row_address, block, ENCRYPT_BLOCK_SIZE) != ENCRYPT_BLOCK_SIZE) { return -1; } XMEMCPY(block + row_offset, data, step); crypto_encrypt(enc_block, block, ENCRYPT_BLOCK_SIZE); - ext_flash_write(row_address, enc_block, ENCRYPT_BLOCK_SIZE); + ret = ext_flash_write(row_address, enc_block, ENCRYPT_BLOCK_SIZE); + /* The request fits entirely within this block: nothing left to do */ + if (step == len) + return ret; address += step; data += step; sz = len - step; diff --git a/tools/unit-tests/unit-extflash.c b/tools/unit-tests/unit-extflash.c index c4bfe5133c..b68e9adc1b 100644 --- a/tools/unit-tests/unit-extflash.c +++ b/tools/unit-tests/unit-extflash.c @@ -111,6 +111,9 @@ int ext_flash_read(uintptr_t address, uint8_t *data, int len) { int ext_flash_write(uintptr_t address, const uint8_t *data, int len) { printf("Called ext_flash_write %p %p %d\n", (void *)address, (const void *)data, len); + /* A negative length is never a valid request */ + ck_assert_int_ge(len, 0); + /* Check that the write address and size are within the bounds of the flash memory */ ck_assert_int_le(address + len, FLASH_SIZE); @@ -294,6 +297,61 @@ START_TEST(test_ext_enc_flash_short_unaligned_read) { } END_TEST +/* This test is also built without EXT_ENCRYPTED, where there is no block size */ +#ifdef ENCRYPT_BLOCK_SIZE + #define TEST_BLOCK_SIZE ENCRYPT_BLOCK_SIZE +#else + #define TEST_BLOCK_SIZE 16 +#endif + +START_TEST(test_ext_enc_flash_short_unaligned_write) { + uint32_t address = 0x1000; + uint8_t data[TEST_BLOCK_SIZE]; + uint8_t dataw[TEST_BLOCK_SIZE]; + /* Writes shorter than the remainder of the encryption block they start in: + * { offset within the block, number of bytes provided } */ + static const int cases[][2] = { {1, 1}, {0, 4}, {8, 3}, + {TEST_BLOCK_SIZE - 1, 1} }; + unsigned int c; + int i, rres, wres; + + /* Prime the target block with known content */ + memcpy(dataw, test_buffer, TEST_BLOCK_SIZE); + wres = ext_flash_check_write(address, dataw, TEST_BLOCK_SIZE); + ck_assert_int_eq(wres, 0); + + for (c = 0; c < sizeof(cases) / sizeof(cases[0]); c++) { + int off = cases[c][0]; + int len = cases[c][1]; + int tail = TEST_BLOCK_SIZE - (off + len); + + /* Payload followed by a guard pattern that must never be consumed */ + memset(dataw, 0x5A, sizeof(dataw)); + for (i = 0; i < len; i++) + dataw[i] = (uint8_t)(0xC0 + i); + + wres = ext_flash_check_write(address + off, dataw, len); + ck_assert_int_eq(wres, 0); + + rres = ext_flash_check_read(address + off, data, len); + ck_assert_int_eq(rres, len); + ck_assert_mem_eq(data, dataw, len); + + /* Bytes past the requested length must not have been taken from the + * caller's buffer */ + if (tail > 0) { + rres = ext_flash_check_read(address + off + len, data, tail); + ck_assert_int_eq(rres, tail); + for (i = 0; i < tail; i++) { + if (data[i] != 0x5A) + break; + } + ck_assert_int_lt(i, tail); + } + } +} +END_TEST + Suite *wolfboot_suite(void) @@ -306,19 +364,24 @@ Suite *wolfboot_suite(void) TCase *ext_flash_operations = tcase_create("External flash operations: API"); TCase *ext_enc_flash_operations = tcase_create("External encrypted flash operations"); TCase *ext_enc_flash_short_read = tcase_create("External encrypted flash short unaligned read"); + TCase *ext_enc_flash_short_write = tcase_create("External encrypted flash short unaligned write"); /* Set parameters + add to suite */ tcase_add_test(ext_flash_operations, test_ext_flash_operations); tcase_add_test(ext_enc_flash_operations, test_ext_enc_flash_operations); tcase_add_test(ext_enc_flash_short_read, test_ext_enc_flash_short_unaligned_read); + tcase_add_test(ext_enc_flash_short_write, + test_ext_enc_flash_short_unaligned_write); tcase_set_timeout(ext_flash_operations, 20); tcase_set_timeout(ext_enc_flash_operations, 20); tcase_set_timeout(ext_enc_flash_short_read, 20); + tcase_set_timeout(ext_enc_flash_short_write, 20); suite_add_tcase(s, ext_flash_operations); suite_add_tcase(s, ext_enc_flash_operations); suite_add_tcase(s, ext_enc_flash_short_read); + suite_add_tcase(s, ext_enc_flash_short_write); return s; } From e200579d362694100edfdde5852748a9f7bdbd5c Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 11 Aug 2026 09:05:36 +0200 Subject: [PATCH 03/25] F-7969: validate boot-side digest before delta base hash compare wolfBoot_delta_update() compared the boot partition digest against the delta base hash using base_hash_sz, the length returned by wolfBoot_find_header() for the boot header's hash TLV, without ever checking it. When the tag is absent, find_header() sets base_hash to NULL and returns 0, so wolfBoot_hardened_CT_compare(NULL, ..., 0) compared zero bytes and reported a match: the base image digest gate silently succeeded instead of rejecting the patch. A short or oversized TLV length would likewise truncate the comparison or read past the delta base hash in the update header. The gate is reachable because wolfBoot_update() runs before the boot partition is verified, so the boot header contents are not guaranteed to carry a well-formed digest TLV at that point. Reject the patch when the base image has no usable digest, and compare a fixed WOLFBOOT_SHA_DIGEST_SIZE. The inverse and resume paths are unaffected, as they do not use this gate. Add unit-update-flash-delta coverage for a boot header without a digest TLV. --- src/update_flash.c | 8 ++-- tools/unit-tests/unit-update-flash.c | 62 ++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/src/update_flash.c b/src/update_flash.c index 0cfeaab62e..fffa5c0d8b 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -710,9 +710,11 @@ static int wolfBoot_delta_update(struct wolfBoot_image *boot, cur_v, delta_base_v); ret = -1; } else if (!resume && delta_base_hash && - wolfBoot_hardened_CT_compare(base_hash, delta_base_hash, - base_hash_sz) != 0) { - /* Wrong base image digest, cannot apply delta patch */ + ((base_hash == NULL) || + (base_hash_sz != WOLFBOOT_SHA_DIGEST_SIZE) || + (wolfBoot_hardened_CT_compare(base_hash, delta_base_hash, + WOLFBOOT_SHA_DIGEST_SIZE) != 0))) { + /* Wrong or missing base image digest, cannot apply delta patch */ wolfBoot_printf("Delta Base hash mismatch\n"); ret = -1; } else { diff --git a/tools/unit-tests/unit-update-flash.c b/tools/unit-tests/unit-update-flash.c index f3145ff562..245b15e03b 100644 --- a/tools/unit-tests/unit-update-flash.c +++ b/tools/unit-tests/unit-update-flash.c @@ -1358,6 +1358,67 @@ START_TEST (test_delta_base_version_match_accepts) } END_TEST +START_TEST (test_delta_base_hash_missing_in_boot_header_rejected) +{ + struct wolfBoot_image boot, update, swap; + uint32_t word; + uint32_t delta_sz = 0x00001020; + uint32_t delta_base = 1; + uint8_t base_hash[SHA256_DIGEST_SIZE]; + uint8_t *boot_base = (uint8_t *)(uintptr_t)WOLFBOOT_PARTITION_BOOT_ADDRESS; + int ret; + + reset_mock_stats(); + prepare_flash(); + + add_payload(PART_BOOT, 1, TEST_SIZE_SMALL); + add_payload(PART_UPDATE, 2, TEST_SIZE_SMALL); + + /* Remove the digest TLV from the boot header, keeping the TLV chain + * well-formed by retagging it to an unused custom type */ + hal_flash_unlock(); + word = SHA256_DIGEST_SIZE << 16 | 0x0031; + hal_flash_write((uintptr_t)boot_base + DIGEST_TLV_OFF_IN_HDR, + (void *)&word, 4); + hal_flash_lock(); + + /* The delta patch declares a base digest that cannot match */ + memset(base_hash, 0xA5, sizeof(base_hash)); + + ext_flash_unlock(); + word = (4u << 16) | HDR_IMG_DELTA_SIZE; + ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 64, + (const uint8_t *)&word, sizeof(word)); + word = host_to_img_u32(delta_sz); + ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 68, + (const uint8_t *)&word, sizeof(word)); + word = (4u << 16) | HDR_IMG_DELTA_BASE; + ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 72, + (const uint8_t *)&word, sizeof(word)); + word = host_to_img_u32(delta_base); + ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 76, + (const uint8_t *)&word, sizeof(word)); + word = (SHA256_DIGEST_SIZE << 16) | HDR_IMG_DELTA_BASE_HASH; + ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 80, + (const uint8_t *)&word, sizeof(word)); + ext_flash_write(WOLFBOOT_PARTITION_UPDATE_ADDRESS + 84, + base_hash, sizeof(base_hash)); + ext_flash_lock(); + + ck_assert_int_eq(wolfBoot_open_image(&boot, PART_BOOT), 0); + ck_assert_int_eq(wolfBoot_open_image(&update, PART_UPDATE), 0); + memset(&swap, 0, sizeof(swap)); + swap.part = PART_SWAP; + swap.hdr = (void *)(uintptr_t)WOLFBOOT_PARTITION_SWAP_ADDRESS; + + ret = wolfBoot_delta_update(&boot, &update, &swap, 0, 0); + ck_assert_int_eq(ret, -1); + ck_assert_int_eq(mock_wb_patch_init_calls, 0); + + cleanup_flash(); +} +END_TEST + START_TEST (test_delta_inverse_values_passed_with_native_endian) { struct wolfBoot_image boot, update, swap; @@ -1567,6 +1628,7 @@ Suite *wolfboot_suite(void) tcase_add_test(delta_zero_size, test_delta_zero_size_erased_header_uses_recovery_heuristic); tcase_add_test(delta_base_version, test_delta_base_version_mismatch_rejected); tcase_add_test(delta_base_version, test_delta_base_version_match_accepts); + tcase_add_test(delta_base_version, test_delta_base_hash_missing_in_boot_header_rejected); tcase_add_test(delta_base_version, test_delta_inverse_values_passed_with_native_endian); tcase_add_test(delta_base_version, test_delta_inverse_accepts_when_current_matches_update); tcase_add_test(delta_base_version, test_delta_inverse_accepts_when_current_matches_delta_base); From 748fa8a19a967c9e26e39a4d94f71d361558a2d7 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 11 Aug 2026 09:13:31 +0200 Subject: [PATCH 04/25] F-8006: return errors from delta base-hash validation in sign tool make_header_ex() validated the delta base image digest with direct exit(1) calls. Those are reachable in normal use: base_diff() looks up the base digest for the selected hash algorithm, and when the base image was signed with a different algorithm the lookup yields NULL, yet make_header_delta() is still called. Aborting there skips base_diff()'s cleanup (the temporary patch file is left in /tmp) and, more importantly, main()'s zero_and_free(kbuf, key_buffer_sz) and algorithm-specific key free, so the raw and decoded private signing key are never scrubbed. Use the function's existing 'failure:' path instead, which returns -1 and propagates through base_diff() to main()'s unified cleanup. Reaching 'failure:' from there uncovered a latent double fclose(): the image-size probe closes 'f' without clearing it, so the cleanup block closed the same stream again. Clear the pointer after the fclose(). Add unit-sign-delta-basehash-cleanup.py, which signs a SHA256 base image, requests a SHA384 delta against it, and asserts the run fails with the temporary patch file removed. --- tools/keytools/sign.c | 9 +- tools/unit-tests/Makefile | 1 + .../unit-sign-delta-basehash-cleanup.py | 136 ++++++++++++++++++ 3 files changed, 142 insertions(+), 4 deletions(-) create mode 100644 tools/unit-tests/unit-sign-delta-basehash-cleanup.py diff --git a/tools/keytools/sign.c b/tools/keytools/sign.c index eb10176a45..8040e71afb 100644 --- a/tools/keytools/sign.c +++ b/tools/keytools/sign.c @@ -1465,6 +1465,7 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz, image_sz = ftell(f); fseek(f, 0, SEEK_SET); fclose(f); + f = NULL; /* Append Magic header (spells 'WOLF') */ header_append_u32(header, &header_idx, WOLFBOOT_MAGIC); @@ -1524,26 +1525,26 @@ static int make_header_ex(int is_diff, uint8_t *pubkey, uint32_t pubkey_sz, ALIGN_8(header_idx); if (!base_hash) { fprintf(stderr, "Base hash for delta image not found.\n"); - exit(1); + goto failure; } if (CMD.hash_algo == HASH_SHA256) { if (base_hash_sz != HDR_SHA256_LEN) { fprintf(stderr, "Invalid base hash size for SHA256.\n"); - exit(1); + goto failure; } 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); + goto failure; } 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); + goto failure; } header_append_tag(header, &header_idx, HDR_IMG_DELTA_BASE_HASH, HDR_SHA3_384_LEN, base_hash); diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index c07bc4ca9d..a9bfb278f3 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -139,6 +139,7 @@ run: $(TESTS) done python3 unit-sign-delta-tlv.py || exit 1 python3 unit-sign-delta-cert-inv-off.py || exit 1 + python3 unit-sign-delta-basehash-cleanup.py || exit 1 python3 unit-sign-custom-tlv-le.py || exit 1 python3 unit-sign-custom-tlv-large.py || exit 1 python3 unit-sign-custom-tlv-pubkey-der.py || exit 1 diff --git a/tools/unit-tests/unit-sign-delta-basehash-cleanup.py b/tools/unit-tests/unit-sign-delta-basehash-cleanup.py new file mode 100644 index 0000000000..ebcdb46374 --- /dev/null +++ b/tools/unit-tests/unit-sign-delta-basehash-cleanup.py @@ -0,0 +1,136 @@ +#!/usr/bin/env python3 +# unit-sign-delta-basehash-cleanup.py +# +# Regression test for the delta base-hash validation error path in the C +# signing tool. +# +# make_header_ex() in tools/keytools/sign.c validates the base image digest +# handed to it by base_diff() when signing a delta update (is_diff=1). If the +# base image carries no digest for the selected hash algorithm, or one whose +# size does not match, the function used to call exit(1) directly instead of +# taking its 'failure:' path. That terminates the process from deep inside the +# call chain, so none of the unwinding runs: base_diff()'s 'cleanup:' block +# never unlinks the temporary patch file, and main() never reaches +# zero_and_free(kbuf, key_buffer_sz) or the algorithm-specific key free, so the +# raw and decoded private signing key stay in memory unscrubbed. +# +# The reachable trigger is a base image signed with a different hash algorithm +# than the delta: base_diff() looks up HDR_SHA256/HDR_SHA384/HDR_SHA3_384 +# according to CMD.hash_algo, finds nothing, and still calls +# make_header_delta(). +# +# Key zeroization is not directly observable from outside the process, but the +# leftover temporary patch file is: it proves that the error return unwound +# through base_diff()'s cleanup instead of aborting the process. This test +# signs a base image with SHA256, asks for a SHA384 delta against it, and +# asserts that the run fails cleanly with /tmp/wolfboot-delta.bin removed. +# Before the fix the file is left behind. +# +# Copyright (C) 2026 wolfSSL Inc. +# +# This file is part of wolfBoot. +# +# wolfBoot is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# wolfBoot is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +import os +import subprocess +import sys +import tempfile + +SECTOR_SIZE = 0x1000 + +# wolfboot_delta_file[] in tools/keytools/sign.c +DELTA_TMP = "/tmp/wolfboot-delta.bin" + +THIS_DIR = os.path.dirname(os.path.abspath(__file__)) +ROOT = os.path.abspath(os.path.join(THIS_DIR, "..", "..")) +KEYTOOLS = os.path.join(ROOT, "tools", "keytools") +SIGN = os.path.join(KEYTOOLS, "sign") +KEYGEN = os.path.join(KEYTOOLS, "keygen") + + +def skip(msg): + print("SKIP unit-sign-delta-basehash-cleanup: " + msg) + sys.exit(0) + + +def ensure_tool(path, target): + if os.path.exists(path): + return True + try: + subprocess.run(["make", target], cwd=KEYTOOLS, + check=True, capture_output=True, text=True) + except (subprocess.CalledProcessError, OSError): + return False + return os.path.exists(path) + + +def main(): + if not ensure_tool(SIGN, "sign"): + skip("could not build tools/keytools/sign") + if not ensure_tool(KEYGEN, "keygen"): + skip("could not build tools/keytools/keygen") + + with tempfile.TemporaryDirectory() as work: + key = os.path.join(work, "priv.der") + r = subprocess.run([KEYGEN, "--ed25519", "-g", key, + "-keystoreDir", work], + cwd=work, capture_output=True, text=True) + if r.returncode != 0 or not os.path.exists(key): + skip("keygen failed: " + r.stderr.strip()) + + base = os.path.join(work, "image_v1.bin") + upd = os.path.join(work, "image_v2.bin") + payload = bytes((i * 7) & 0xFF for i in range(2048)) + with open(base, "wb") as f: + f.write(payload) + with open(upd, "wb") as f: + f.write(payload[:512] + b"PATCHED!" + payload[520:]) + + env = dict(os.environ) + env["WOLFBOOT_SECTOR_SIZE"] = str(SECTOR_SIZE) + + # Sign the base image (v1) with SHA256, so it carries HDR_SHA256 only. + r = subprocess.run([SIGN, "--ed25519", "--sha256", base, key, "1"], + cwd=ROOT, env=env, capture_output=True, text=True) + if r.returncode != 0: + skip("sign base failed: " + r.stderr.strip()) + signed_base = base.replace(".bin", "_v1_signed.bin") + if not os.path.exists(signed_base): + skip("sign did not produce a signed base image") + + if os.path.exists(DELTA_TMP): + os.unlink(DELTA_TMP) + + # Ask for a SHA384 delta: base_diff() looks up HDR_SHA384 in the base + # image, finds nothing, and make_header_ex() must fail cleanly. + r = subprocess.run([SIGN, "--ed25519", "--sha384", "--delta", + signed_base, upd, key, "2"], + cwd=ROOT, env=env, capture_output=True, text=True) + if r.returncode == 0: + print("FAIL unit-sign-delta-basehash-cleanup: signing a delta " + "against a base image with no matching digest succeeded") + sys.exit(1) + + if os.path.exists(DELTA_TMP): + os.unlink(DELTA_TMP) + print("FAIL unit-sign-delta-basehash-cleanup: %s was left behind, " + "so make_header_ex() aborted the process instead of " + "returning an error; base_diff() cleanup and main()'s " + "signing key zeroization were skipped" % DELTA_TMP) + sys.exit(1) + + print("unit-sign-delta-basehash-cleanup: OK") + sys.exit(0) + + +if __name__ == "__main__": + main() From f9957da5238fcf3509c18cef8797b5d48d3bc159 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 11 Aug 2026 09:25:54 +0200 Subject: [PATCH 05/25] F-8003: separate decoded key objects for hybrid signers The sign tool kept a single file-static struct for the decoded private key, so a hybrid run that picks two algorithms sharing one member (e.g. ECC521 primary + ECC256 secondary, or RSA2048 + RSAPSS2048) had the secondary load_key() re-init and overwrite the still-live primary key before either signature was produced. The primary signature was then made with the secondary key, and the final cleanup in main() dispatched only on CMD.sign, so the secondary key never reached its algorithm specific zeroizing free. Give the primary and the secondary signer their own storage, select it with key_obj(secondary) in load_key()/load_key_ecc()/load_key_rsa()/ sign_digest()/set_signature_sizes(), and free both keys at exit through the new free_key() helper. --- tools/keytools/sign.c | 204 +++++++++++--------- tools/unit-tests/unit-sign-hybrid-keyload.c | 117 +++++++++++ 2 files changed, 231 insertions(+), 90 deletions(-) diff --git a/tools/keytools/sign.c b/tools/keytools/sign.c index 8040e71afb..8872b7331c 100644 --- a/tools/keytools/sign.c +++ b/tools/keytools/sign.c @@ -307,7 +307,7 @@ static void header_append_tag_u64(uint8_t *header, uint32_t *idx, uint16_t tag, /* Globals */ static const char wolfboot_delta_file[] = "/tmp/wolfboot-delta.bin"; -static struct { +struct signing_key { ed25519_key ed; ed448_key ed4; ecc_key ecc; @@ -315,7 +315,51 @@ static struct { LmsKey lms; XmssKey xmss; wc_MlDsaKey ml_dsa; -} key; +}; + +/* Hybrid signing keeps the primary and the secondary private key decoded at + * the same time, so the two signers must not share the same storage. */ +static struct signing_key key; +static struct signing_key key2; + +static struct signing_key *key_obj(int secondary) +{ + return secondary ? &key2 : &key; +} + +/* Run the algorithm specific (zeroizing) free on a decoded signing key. */ +static void free_key(int sign, int secondary) +{ + struct signing_key *k = key_obj(secondary); + if (sign == SIGN_ED25519) { + wc_ed25519_free(&k->ed); + } + else if (sign == SIGN_ED448) { + wc_ed448_free(&k->ed4); + } + else if (sign == SIGN_ECC256 || + sign == SIGN_ECC384 || + sign == SIGN_ECC521) { + wc_ecc_free(&k->ecc); + } + else if (sign == SIGN_RSA2048 || + sign == SIGN_RSA3072 || + sign == SIGN_RSA4096 || + sign == SIGN_RSAPSS2048 || + sign == SIGN_RSAPSS3072 || + sign == SIGN_RSAPSS4096) { + wc_FreeRsaKey(&k->rsa); + } + else if (sign == SIGN_LMS) { + wc_LmsKey_Free(&k->lms); + } + else if (sign == SIGN_XMSS) { + wc_XmssKey_Free(&k->xmss); + } + else if (sign == SIGN_ML_DSA) { + wc_MlDsaKey_Free(&k->ml_dsa); + } +} struct cmd_options { int manual_sign; @@ -443,6 +487,7 @@ static int load_key_ecc(int sign_type, uint32_t curve_sz, int curve_id, uint32_t idx; uint32_t qxSz = curve_sz; uint32_t qySz = curve_sz; + struct signing_key *k = key_obj(secondary); *pubkey_sz = curve_sz * 2; *pubkey = malloc(*pubkey_sz); /* assume malloc works */ @@ -450,7 +495,7 @@ static int load_key_ecc(int sign_type, uint32_t curve_sz, int curve_id, printf("Pubkey malloc error!\n"); return -1; } - initRet = ret = wc_ecc_init(&key.ecc); + initRet = ret = wc_ecc_init(&k->ecc); if (CMD.manual_sign || CMD.sha_only) { /* raw (public x + public y) */ if (*key_buffer_sz == (curve_sz * 2)) { @@ -460,16 +505,16 @@ static int load_key_ecc(int sign_type, uint32_t curve_sz, int curve_id, else { if (ret == 0) { idx = 0; - ret = wc_EccPublicKeyDecode(*key_buffer, &idx, &key.ecc, + ret = wc_EccPublicKeyDecode(*key_buffer, &idx, &k->ecc, *key_buffer_sz); } /* we could decode another type of key in auto so check */ - if (ret == 0 && key.ecc.dp->id != curve_id) { + if (ret == 0 && k->ecc.dp->id != curve_id) { ret = -1; } if (ret == 0) { - ret = wc_ecc_export_public_raw(&key.ecc, + ret = wc_ecc_export_public_raw(&k->ecc, *pubkey, &qxSz, /* public x */ *pubkey + curve_sz, &qySz /* public y */ ); @@ -481,7 +526,7 @@ static int load_key_ecc(int sign_type, uint32_t curve_sz, int curve_id, memcpy(*pubkey, *key_buffer, *pubkey_sz); if (ret == 0) { - ret = wc_ecc_import_unsigned(&key.ecc, + ret = wc_ecc_import_unsigned(&k->ecc, *key_buffer, /* public x */ (*key_buffer) + curve_sz, /* public y */ (*key_buffer) + (curve_sz * 2), /* private d */ @@ -497,15 +542,15 @@ static int load_key_ecc(int sign_type, uint32_t curve_sz, int curve_id, else { if (ret == 0) { idx = 0; - ret = wc_EccPrivateKeyDecode(*key_buffer, &idx, &key.ecc, + ret = wc_EccPrivateKeyDecode(*key_buffer, &idx, &k->ecc, *key_buffer_sz); } /* we could decode another type of key in auto so check */ - if (ret == 0 && key.ecc.dp->id != curve_id) { + if (ret == 0 && k->ecc.dp->id != curve_id) { ret = -1; } if (ret == 0) { - ret = wc_ecc_export_public_raw(&key.ecc, + ret = wc_ecc_export_public_raw(&k->ecc, *pubkey, &qxSz, /* public x */ *pubkey + curve_sz, &qySz /* public y */ ); @@ -517,7 +562,7 @@ static int load_key_ecc(int sign_type, uint32_t curve_sz, int curve_id, } if (ret != 0 && initRet == 0) { - wc_ecc_free(&key.ecc); + wc_ecc_free(&k->ecc); } if (ret != 0) { free(*pubkey); @@ -549,6 +594,7 @@ static int load_key_rsa(int sign_type, uint32_t rsa_keysz, uint32_t rsa_pubkeysz int initRet = -1; uint32_t idx; uint32_t keySzOut = 0; + struct signing_key *k = key_obj(secondary); if (CMD.manual_sign || CMD.sha_only) { /* Allocate and copy pubkey instead of using key_buffer directly */ @@ -573,15 +619,15 @@ static int load_key_rsa(int sign_type, uint32_t rsa_keysz, uint32_t rsa_pubkeysz ret = 0; } else { - initRet = ret = wc_InitRsaKey(&key.rsa, NULL); + initRet = ret = wc_InitRsaKey(&k->rsa, NULL); if (ret == 0) { idx = 0; - ret = wc_RsaPrivateKeyDecode(*key_buffer, &idx, &key.rsa, + ret = wc_RsaPrivateKeyDecode(*key_buffer, &idx, &k->rsa, *key_buffer_sz); } if (ret == 0) { - ret = wc_RsaKeyToPublicDer(&key.rsa, *key_buffer, *key_buffer_sz); + ret = wc_RsaKeyToPublicDer(&k->rsa, *key_buffer, *key_buffer_sz); } if (ret > 0) { @@ -592,7 +638,7 @@ static int load_key_rsa(int sign_type, uint32_t rsa_keysz, uint32_t rsa_pubkeysz printf("Pubkey malloc error!\n"); ret = -1; if (initRet == 0) { - wc_FreeRsaKey(&key.rsa); + wc_FreeRsaKey(&k->rsa); } return -1; } @@ -601,11 +647,11 @@ static int load_key_rsa(int sign_type, uint32_t rsa_keysz, uint32_t rsa_pubkeysz } if (ret == 0) { - keySzOut = wc_RsaEncryptSize(&key.rsa); + keySzOut = wc_RsaEncryptSize(&k->rsa); } if (ret != 0 && initRet == 0) { - wc_FreeRsaKey(&key.rsa); + wc_FreeRsaKey(&k->rsa); } if (ret == 0 || CMD.sign != SIGN_AUTO) { @@ -636,6 +682,7 @@ static uint8_t *load_key(uint8_t **key_buffer, uint32_t *key_buffer_sz, word32 pub_sz = 0; int sign = CMD.sign; const char *key_file = CMD.key_file; + struct signing_key *k = key_obj(secondary); /* open and load key buffer */ *key_buffer = NULL; @@ -692,20 +739,20 @@ static uint8_t *load_key(uint8_t **key_buffer, uint32_t *key_buffer_sz, ret = 0; } else { - initRet = ret = wc_ed25519_init(&key.ed); + initRet = ret = wc_ed25519_init(&k->ed); if (ret == 0) { idx = 0; ret = wc_Ed25519PublicKeyDecode(*key_buffer, &idx, - &key.ed, *key_buffer_sz); + &k->ed, *key_buffer_sz); } if (ret == 0) { - ret = wc_ed25519_export_public(&key.ed, *pubkey, + ret = wc_ed25519_export_public(&k->ed, *pubkey, pubkey_sz); } /* free key no matter what */ if (initRet == 0) - wc_ed25519_free(&key.ed); + wc_ed25519_free(&k->ed); } } /* raw only */ @@ -713,15 +760,15 @@ static uint8_t *load_key(uint8_t **key_buffer, uint32_t *key_buffer_sz, memcpy(*pubkey, *key_buffer + ED25519_KEY_SIZE, KEYSTORE_PUBKEY_SIZE_ED25519); - initRet = ret = wc_ed25519_init(&key.ed); + initRet = ret = wc_ed25519_init(&k->ed); if (ret == 0) { ret = wc_ed25519_import_private_key(*key_buffer, - ED25519_KEY_SIZE, *pubkey, *pubkey_sz, &key.ed); + ED25519_KEY_SIZE, *pubkey, *pubkey_sz, &k->ed); } /* only free the key if we failed after allocating */ if (ret != 0 && initRet == 0) - wc_ed25519_free(&key.ed); + wc_ed25519_free(&k->ed); } if (ret != 0) { @@ -760,20 +807,20 @@ static uint8_t *load_key(uint8_t **key_buffer, uint32_t *key_buffer_sz, ret = 0; } else { - initRet = ret = wc_ed448_init(&key.ed4); + initRet = ret = wc_ed448_init(&k->ed4); if (ret == 0) { idx = 0; ret = wc_Ed448PublicKeyDecode(*key_buffer, &idx, - &key.ed4, *key_buffer_sz); + &k->ed4, *key_buffer_sz); } if (ret == 0) { - ret = wc_ed448_export_public(&key.ed4, *pubkey, + ret = wc_ed448_export_public(&k->ed4, *pubkey, pubkey_sz); } /* free key no matter what */ if (initRet == 0) - wc_ed448_free(&key.ed4); + wc_ed448_free(&k->ed4); } } @@ -782,15 +829,15 @@ static uint8_t *load_key(uint8_t **key_buffer, uint32_t *key_buffer_sz, memcpy(*pubkey, *key_buffer + ED448_KEY_SIZE, ED448_PUB_KEY_SIZE); - initRet = ret = wc_ed448_init(&key.ed4); + initRet = ret = wc_ed448_init(&k->ed4); if (ret == 0) { ret = wc_ed448_import_private_key(*key_buffer, - ED448_KEY_SIZE, *pubkey, *pubkey_sz, &key.ed4); + ED448_KEY_SIZE, *pubkey, *pubkey_sz, &k->ed4); } /* only free the key if we failed after allocating */ if (ret != 0 && initRet == 0) - wc_ed448_free(&key.ed4); + wc_ed448_free(&k->ed4); } if (ret != 0) { @@ -935,7 +982,7 @@ static uint8_t *load_key(uint8_t **key_buffer, uint32_t *key_buffer_sz, * If both priv/pub are present: * - The first ?? bytes is the private key. * - The next 68 bytes is the public key. */ - ret = wc_XmssKey_GetPrivLen(&key.xmss, &priv_sz); + ret = wc_XmssKey_GetPrivLen(&k->xmss, &priv_sz); if (ret != 0 || priv_sz <= 0) { printf("error: wc_XmssKey_GetPrivLen returned %d\n", ret); break; @@ -977,7 +1024,7 @@ static uint8_t *load_key(uint8_t **key_buffer, uint32_t *key_buffer_sz, } FALL_THROUGH; /* we didn't solve the key, keep trying */ case SIGN_ML_DSA: - ret = wc_MlDsaKey_GetPubLen(&key.ml_dsa, (int *)&pub_sz); + ret = wc_MlDsaKey_GetPubLen(&k->ml_dsa, (int *)&pub_sz); if (ret != 0 || pub_sz <= 0) { printf("error: wc_MlDsaKey_GetPubLen returned %d\n", ret); @@ -986,7 +1033,7 @@ static uint8_t *load_key(uint8_t **key_buffer, uint32_t *key_buffer_sz, /* Get the ML-DSA private key length. This API returns * the public + private length. */ - ret = wc_MlDsaKey_GetPrivLen(&key.ml_dsa, (int*)&priv_sz); + ret = wc_MlDsaKey_GetPrivLen(&k->ml_dsa, (int*)&priv_sz); if (ret != 0 || priv_sz <= 0) { printf("error: wc_MlDsaKey_GetPrivLen returned %d\n", ret); @@ -1007,7 +1054,7 @@ static uint8_t *load_key(uint8_t **key_buffer, uint32_t *key_buffer_sz, if (*key_buffer_sz == (priv_sz + pub_sz)) { /* priv + pub */ - ret = wc_MlDsaKey_ImportPrivRaw(&key.ml_dsa, *key_buffer, + ret = wc_MlDsaKey_ImportPrivRaw(&k->ml_dsa, *key_buffer, priv_sz); *pubkey_sz = pub_sz; *pubkey = malloc(*pubkey_sz); @@ -1072,8 +1119,8 @@ static int sign_digest(int sign, int hash_algo, { int ret; WC_RNG rng; + struct signing_key *k = key_obj(secondary); printf("Sign: %02x\n", sign >> 8); - (void)secondary; if ((ret = wc_InitRng(&rng)) != 0) { return ret; @@ -1081,12 +1128,12 @@ static int sign_digest(int sign, int hash_algo, if (sign == SIGN_ED25519) { ret = wc_ed25519_sign_msg(digest, digest_sz, signature, - signature_sz, &key.ed); + signature_sz, &k->ed); } else if (sign == SIGN_ED448) { ret = wc_ed448_sign_msg(digest, digest_sz, signature, - signature_sz, &key.ed4, NULL, 0); + signature_sz, &k->ed4, NULL, 0); } else if (sign == SIGN_ECC256 || @@ -1103,7 +1150,7 @@ static int sign_digest(int sign, int hash_algo, memset(signature, 0, *signature_sz); mp_init(&r); mp_init(&s); - ret = wc_ecc_sign_hash_ex(digest, digest_sz, &rng, &key.ecc, + ret = wc_ecc_sign_hash_ex(digest, digest_sz, &rng, &k->ecc, &r, &s); if (ret == 0) { word32 rSz, sSz; @@ -1139,7 +1186,7 @@ static int sign_digest(int sign, int hash_algo, enchash = buf; } ret = wc_RsaSSL_Sign(enchash, enchash_sz, signature, *signature_sz, - &key.rsa, &rng); + &k->rsa, &rng); if (ret > 0) { *signature_sz = ret; ret = 0; @@ -1163,7 +1210,7 @@ static int sign_digest(int sign, int hash_algo, return -1; } ret = wc_RsaPSS_Sign(digest, digest_sz, signature, *signature_sz, - hash_type, mgf, &key.rsa, &rng); + hash_type, mgf, &k->rsa, &rng); if (ret > 0) { *signature_sz = ret; ret = 0; @@ -1176,18 +1223,18 @@ static int sign_digest(int sign, int hash_algo, key_file = CMD.secondary_key_file; } /* Set the callbacks, so LMS can update the private key while signing */ - ret = wc_LmsKey_SetWriteCb(&key.lms, lms_write_key); + ret = wc_LmsKey_SetWriteCb(&k->lms, lms_write_key); if (ret == 0) { - ret = wc_LmsKey_SetReadCb(&key.lms, lms_read_key); + ret = wc_LmsKey_SetReadCb(&k->lms, lms_read_key); } if (ret == 0) { - ret = wc_LmsKey_SetContext(&key.lms, (void*)key_file); + ret = wc_LmsKey_SetContext(&k->lms, (void*)key_file); } if (ret == 0) { - ret = wc_LmsKey_Reload(&key.lms); + ret = wc_LmsKey_Reload(&k->lms); } if (ret == 0) { - ret = wc_LmsKey_Sign(&key.lms, signature, signature_sz, digest, + ret = wc_LmsKey_Sign(&k->lms, signature, signature_sz, digest, digest_sz); } if (ret != 0) { @@ -1200,25 +1247,25 @@ static int sign_digest(int sign, int hash_algo, if (secondary) { key_file = CMD.secondary_key_file; } - ret = wc_XmssKey_Init(&key.xmss, NULL, INVALID_DEVID); + ret = wc_XmssKey_Init(&k->xmss, NULL, INVALID_DEVID); /* Set the callbacks, so XMSS can update the private key while signing */ if (ret == 0) { - ret = wc_XmssKey_SetWriteCb(&key.xmss, xmss_write_key); + ret = wc_XmssKey_SetWriteCb(&k->xmss, xmss_write_key); } if (ret == 0) { - ret = wc_XmssKey_SetReadCb(&key.xmss, xmss_read_key); + ret = wc_XmssKey_SetReadCb(&k->xmss, xmss_read_key); } if (ret == 0) { - ret = wc_XmssKey_SetContext(&key.xmss, (void*)key_file); + ret = wc_XmssKey_SetContext(&k->xmss, (void*)key_file); } if (ret == 0) { - ret = wc_XmssKey_SetParamStr(&key.xmss, WOLFBOOT_XMSS_PARAMS); + ret = wc_XmssKey_SetParamStr(&k->xmss, WOLFBOOT_XMSS_PARAMS); } if (ret == 0) { - ret = wc_XmssKey_Reload(&key.xmss); + ret = wc_XmssKey_Reload(&k->xmss); } if (ret == 0) { - ret = wc_XmssKey_Sign(&key.xmss, signature, signature_sz, digest, + ret = wc_XmssKey_Sign(&k->xmss, signature, signature_sz, digest, digest_sz); } if (ret != 0) { @@ -1229,7 +1276,7 @@ static int sign_digest(int sign, int hash_algo, if (sign == SIGN_ML_DSA) { /* Nothing else to do, ready to sign. */ if (ret == 0) { - ret = wc_MlDsaKey_SignCtx(&key.ml_dsa, NULL, 0, + ret = wc_MlDsaKey_SignCtx(&k->ml_dsa, NULL, 0, signature, signature_sz, digest, digest_sz, &rng); } @@ -2880,6 +2927,7 @@ static void set_signature_sizes(int secondary) int *sign = &CMD.sign; uint32_t suggested_sz = 0; char *env_image_header_size; + struct signing_key *k = key_obj(secondary); if (secondary) { sz = &CMD.secondary_signature_sz; sign = &CMD.secondary_sign; @@ -2966,12 +3014,12 @@ static void set_signature_sizes(int secondary) else lms_winternitz = atoi(lms_winternitz_str); - lms_ret = wc_LmsKey_Init(&key.lms, NULL, INVALID_DEVID); + lms_ret = wc_LmsKey_Init(&k->lms, NULL, INVALID_DEVID); if (lms_ret != 0) { fprintf(stderr, "error: wc_LmsKey_Init returned %d\n", lms_ret); exit(1); } - lms_ret = wc_LmsKey_SetParameters(&key.lms, lms_levels, lms_height, + lms_ret = wc_LmsKey_SetParameters(&k->lms, lms_levels, lms_height, lms_winternitz); if (lms_ret != 0) { fprintf(stderr, "error: wc_LmsKey_SetParameters(%d, %d, %d)" \ @@ -2983,7 +3031,7 @@ static void set_signature_sizes(int secondary) printf("info: using LMS parameters: L%d-H%d-W%d\n", lms_levels, lms_height, lms_winternitz); - lms_ret = wc_LmsKey_GetSigLen(&key.lms, &sig_sz); + lms_ret = wc_LmsKey_GetSigLen(&k->lms, &sig_sz); if (lms_ret != 0) { fprintf(stderr, "error: wc_LmsKey_GetSigLen returned %d\n", lms_ret); @@ -3007,13 +3055,13 @@ static void set_signature_sizes(int secondary) printf("info: using XMSS parameters: %s\n", xmss_params); - xmss_ret = wc_XmssKey_Init(&key.xmss, NULL, INVALID_DEVID); + xmss_ret = wc_XmssKey_Init(&k->xmss, NULL, INVALID_DEVID); if (xmss_ret != 0) { fprintf(stderr, "error: wc_XmssKey_Init returned %d\n", xmss_ret); exit(1); } - xmss_ret = wc_XmssKey_SetParamStr(&key.xmss, xmss_params); + xmss_ret = wc_XmssKey_SetParamStr(&k->xmss, xmss_params); if (xmss_ret != 0) { fprintf(stderr, "error: wc_XmssKey_SetParamStr(%s)" \ " returned %d\n", xmss_params, xmss_ret); @@ -3021,7 +3069,7 @@ static void set_signature_sizes(int secondary) } - xmss_ret = wc_XmssKey_GetSigLen(&key.xmss, &sig_sz); + xmss_ret = wc_XmssKey_GetSigLen(&k->xmss, &sig_sz); if (xmss_ret != 0) { fprintf(stderr, "error: wc_XmssKey_GetSigLen returned %d\n", xmss_ret); @@ -3043,13 +3091,13 @@ static void set_signature_sizes(int secondary) if (env_ml_dsa_level) ml_dsa_level = atoi(env_ml_dsa_level); - ml_dsa_ret = wc_MlDsaKey_Init(&key.ml_dsa, NULL, INVALID_DEVID); + ml_dsa_ret = wc_MlDsaKey_Init(&k->ml_dsa, NULL, INVALID_DEVID); if (ml_dsa_ret != 0) { fprintf(stderr, "error: wc_MlDsaKey_Init returned %d\n", ml_dsa_ret); exit(1); } - ml_dsa_ret = wc_MlDsaKey_SetParams(&key.ml_dsa, ml_dsa_level); + ml_dsa_ret = wc_MlDsaKey_SetParams(&k->ml_dsa, ml_dsa_level); if (ml_dsa_ret != 0) { fprintf(stderr, "error: wc_MlDsaKey_SetParamStr(%d)" \ " returned %d\n", ml_dsa_level, ml_dsa_ret); @@ -3058,7 +3106,7 @@ static void set_signature_sizes(int secondary) printf("info: using ML-DSA parameters: %d\n", ml_dsa_level); - ml_dsa_ret = wc_MlDsaKey_GetSigLen(&key.ml_dsa, (int *)&sig_sz); + ml_dsa_ret = wc_MlDsaKey_GetSigLen(&k->ml_dsa, (int *)&sig_sz); if (ml_dsa_ret != 0) { fprintf(stderr, "error: wc_MlDsaKey_GetSigLen returned %d\n", ml_dsa_ret); @@ -3808,33 +3856,9 @@ int main(int argc, char** argv) if (kbuf) zero_and_free(kbuf, key_buffer_sz); - if (CMD.sign == SIGN_ED25519) { - wc_ed25519_free(&key.ed); - } - else if (CMD.sign == SIGN_ED448) { - wc_ed448_free(&key.ed4); - } - else if (CMD.sign == SIGN_ECC256 || - CMD.sign == SIGN_ECC384 || - CMD.sign == SIGN_ECC521) { - wc_ecc_free(&key.ecc); - } - else if (CMD.sign == SIGN_RSA2048 || - CMD.sign == SIGN_RSA3072 || - CMD.sign == SIGN_RSA4096 || - CMD.sign == SIGN_RSAPSS2048 || - CMD.sign == SIGN_RSAPSS3072 || - CMD.sign == SIGN_RSAPSS4096) { - wc_FreeRsaKey(&key.rsa); - } - else if (CMD.sign == SIGN_LMS) { - wc_LmsKey_Free(&key.lms); - } - else if (CMD.sign == SIGN_XMSS) { - wc_XmssKey_Free(&key.xmss); - } - else if (CMD.sign == SIGN_ML_DSA) { - wc_MlDsaKey_Free(&key.ml_dsa); + free_key(CMD.sign, 0); + if (CMD.hybrid) { + free_key(CMD.secondary_sign, 1); } return ret; } diff --git a/tools/unit-tests/unit-sign-hybrid-keyload.c b/tools/unit-tests/unit-sign-hybrid-keyload.c index 461dd495f5..c3a95ead89 100644 --- a/tools/unit-tests/unit-sign-hybrid-keyload.c +++ b/tools/unit-tests/unit-sign-hybrid-keyload.c @@ -133,6 +133,122 @@ START_TEST(test_sign_main_fails_when_secondary_key_missing) } END_TEST +/* Export a freshly generated ECC key as the raw Qx || Qy || d blob accepted + * by load_key(). */ +static int make_raw_ecc_key(int curve_id, int curve_sz, uint8_t *raw) +{ + WC_RNG rng; + ecc_key ek; + word32 qxSz = curve_sz, qySz = curve_sz, dSz = curve_sz; + int ret; + + if (wc_InitRng(&rng) != 0) { + return -1; + } + if (wc_ecc_init(&ek) != 0) { + wc_FreeRng(&rng); + return -1; + } + ret = wc_ecc_make_key_ex(&rng, curve_sz, &ek, curve_id); + if (ret == 0) { + ret = wc_ecc_export_private_raw(&ek, raw, &qxSz, raw + curve_sz, &qySz, + raw + (curve_sz * 2), &dSz); + } + wc_ecc_free(&ek); + wc_FreeRng(&rng); + + return ret; +} + +/* Check a raw r || s signature against a raw Qx || Qy public key. */ +static int verify_raw_ecc(int curve_id, int curve_sz, const uint8_t *pubkey, + const uint8_t *signature, const uint8_t *digest, uint32_t digest_sz) +{ + ecc_key vk; + mp_int r, s; + int res = 0; + int ret; + + if (wc_ecc_init(&vk) != 0) { + return -1; + } + ret = wc_ecc_import_unsigned(&vk, (byte*)pubkey, (byte*)pubkey + curve_sz, + NULL, curve_id); + if (ret == 0) { + mp_init(&r); + mp_init(&s); + mp_read_unsigned_bin(&r, signature, curve_sz); + mp_read_unsigned_bin(&s, signature + curve_sz, curve_sz); + ret = wc_ecc_verify_hash_ex(&r, &s, digest, digest_sz, &res, &vk); + mp_clear(&r); + mp_clear(&s); + } + wc_ecc_free(&vk); + + if (ret != 0) { + return -1; + } + return res; +} + +/* Hybrid signing loads both private keys before either signature is made, so + * the secondary key must not land on top of the decoded primary key. */ +START_TEST(test_hybrid_secondary_key_does_not_clobber_primary) +{ + char tempdir[] = "/tmp/wolfboot-sign-XXXXXX"; + char primary_path[PATH_MAX]; + char secondary_path[PATH_MAX]; + uint8_t primary_raw[66 * 3]; /* ECC521 Qx + Qy + d */ + uint8_t secondary_raw[32 * 3]; /* ECC256 Qx + Qy + d */ + uint8_t *kbuf = NULL, *kbuf2 = NULL; + uint32_t kbuf_sz = 0, kbuf2_sz = 0; + uint8_t *pubkey = NULL, *pubkey2 = NULL; + uint32_t pubkey_sz = 0, pubkey_sz2 = 0; + uint8_t digest[32]; + uint8_t signature[132]; + uint8_t signature2[64]; + uint32_t signature_sz = sizeof(signature); + uint32_t signature_sz2 = sizeof(signature2); + + ck_assert_int_eq(make_raw_ecc_key(ECC_SECP521R1, 66, primary_raw), 0); + ck_assert_int_eq(make_raw_ecc_key(ECC_SECP256R1, 32, secondary_raw), 0); + + ck_assert_ptr_nonnull(mkdtemp(tempdir)); + snprintf(primary_path, sizeof(primary_path), "%s/ecc521.raw", tempdir); + snprintf(secondary_path, sizeof(secondary_path), "%s/ecc256.raw", tempdir); + ck_assert_int_eq(write_file(primary_path, primary_raw, + sizeof(primary_raw)), 0); + ck_assert_int_eq(write_file(secondary_path, secondary_raw, + sizeof(secondary_raw)), 0); + + reset_cmd_defaults(); + CMD.sign = SIGN_ECC521; + CMD.key_file = primary_path; + CMD.hybrid = 1; + CMD.secondary_sign = SIGN_ECC256; + CMD.secondary_key_file = secondary_path; + + ck_assert_ptr_nonnull(load_key(&kbuf, &kbuf_sz, &pubkey, &pubkey_sz, 0)); + ck_assert_ptr_nonnull(load_key(&kbuf2, &kbuf2_sz, &pubkey2, &pubkey_sz2, + 1)); + + memset(digest, 0x5C, sizeof(digest)); + ck_assert_int_eq(sign_digest(CMD.sign, CMD.hash_algo, signature, + &signature_sz, digest, sizeof(digest), 0), 0); + ck_assert_int_eq(sign_digest(CMD.secondary_sign, CMD.hash_algo, signature2, + &signature_sz2, digest, sizeof(digest), 1), 0); + + ck_assert_int_eq(verify_raw_ecc(ECC_SECP521R1, 66, pubkey, signature, + digest, sizeof(digest)), 1); + ck_assert_int_eq(verify_raw_ecc(ECC_SECP256R1, 32, pubkey2, signature2, + digest, sizeof(digest)), 1); + + unlink(primary_path); + unlink(secondary_path); + rmdir(tempdir); +} +END_TEST + Suite *wolfboot_suite(void) { Suite *s = suite_create("sign-hybrid-keyload"); @@ -140,6 +256,7 @@ Suite *wolfboot_suite(void) tcase_add_test(tcase, test_load_key_clears_pubkey_when_file_missing); tcase_add_test(tcase, test_load_key_clears_pubkey_when_decode_fails); + tcase_add_test(tcase, test_hybrid_secondary_key_does_not_clobber_primary); tcase_add_exit_test(tcase, test_sign_main_fails_when_secondary_key_missing, 1); suite_add_tcase(s, tcase); From 5dfdec31b183d11db4ad13221843987a845f8f98 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 11 Aug 2026 09:29:53 +0200 Subject: [PATCH 06/25] F-7992: bound staged ciphertext in ext_flash_encrypt_write() ext_flash_encrypt_write() encrypted the whole caller-supplied buffer into ENCRYPT_CACHE, which is only NVM_CACHE_SIZE bytes, without any check that the request fits. A request longer than the cache (reachable from the non-secure world through wolfBoot_nsc_write_update(), which only bounds len against the partition size) overran the staging buffer and made ext_flash_write() read past its end. Stage and flush the ciphertext in NVM_CACHE_SIZE chunks instead. The encryption stream is not restarted between chunks, so the resulting flash content is unchanged for requests that already fitted. --- src/libwolfboot.c | 24 ++++++++++++++++++------ tools/unit-tests/unit-extflash.c | 25 +++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index aad3421045..6e04dc50d6 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -2634,15 +2634,27 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data, sz = len - step; } - /* encrypt remainder */ + /* encrypt remainder, staging at most one cache worth at a time */ + ret = 0; step = sz & ~(ENCRYPT_BLOCK_SIZE - 1); - for (i = 0; i < step / ENCRYPT_BLOCK_SIZE; i++) { - XMEMCPY(block, data + (ENCRYPT_BLOCK_SIZE * i), ENCRYPT_BLOCK_SIZE); - crypto_encrypt(ENCRYPT_CACHE + (ENCRYPT_BLOCK_SIZE * i), block, - ENCRYPT_BLOCK_SIZE); + while (step > 0) { + int chunk = step; + if (chunk > NVM_CACHE_SIZE) + chunk = NVM_CACHE_SIZE; + for (i = 0; i < chunk / ENCRYPT_BLOCK_SIZE; i++) { + XMEMCPY(block, data + (ENCRYPT_BLOCK_SIZE * i), ENCRYPT_BLOCK_SIZE); + crypto_encrypt(ENCRYPT_CACHE + (ENCRYPT_BLOCK_SIZE * i), block, + ENCRYPT_BLOCK_SIZE); + } + ret = ext_flash_write(address, ENCRYPT_CACHE, chunk); + if (ret < 0) + return ret; + address += chunk; + data += chunk; + step -= chunk; } - return ext_flash_write(address, ENCRYPT_CACHE, step); + return ret; } /** diff --git a/tools/unit-tests/unit-extflash.c b/tools/unit-tests/unit-extflash.c index b68e9adc1b..3fdf22d20b 100644 --- a/tools/unit-tests/unit-extflash.c +++ b/tools/unit-tests/unit-extflash.c @@ -353,6 +353,26 @@ START_TEST(test_ext_enc_flash_short_unaligned_write) { END_TEST +/* A single request longer than the staging cache must not overrun it */ +START_TEST(test_ext_enc_flash_oversized_write) { + uint32_t address = 0x1000; + static uint8_t dataw[3 * WOLFBOOT_SECTOR_SIZE]; + static uint8_t data[3 * WOLFBOOT_SECTOR_SIZE]; + int i, rres, wres; + + for (i = 0; i < (int)sizeof(dataw); i++) + dataw[i] = (uint8_t)(i ^ (i >> 8)); + + wres = ext_flash_check_write(address, dataw, sizeof(dataw)); + ck_assert_int_eq(wres, 0); + + memset(data, 0xA5, sizeof(data)); + rres = ext_flash_check_read(address, data, sizeof(data)); + ck_assert_int_eq(rres, (int)sizeof(data)); + ck_assert_mem_eq(data, dataw, sizeof(dataw)); +} +END_TEST + Suite *wolfboot_suite(void) { @@ -365,6 +385,7 @@ Suite *wolfboot_suite(void) TCase *ext_enc_flash_operations = tcase_create("External encrypted flash operations"); TCase *ext_enc_flash_short_read = tcase_create("External encrypted flash short unaligned read"); TCase *ext_enc_flash_short_write = tcase_create("External encrypted flash short unaligned write"); + TCase *ext_enc_flash_oversized_write = tcase_create("External encrypted flash oversized write"); /* Set parameters + add to suite */ tcase_add_test(ext_flash_operations, test_ext_flash_operations); @@ -373,15 +394,19 @@ Suite *wolfboot_suite(void) test_ext_enc_flash_short_unaligned_read); tcase_add_test(ext_enc_flash_short_write, test_ext_enc_flash_short_unaligned_write); + tcase_add_test(ext_enc_flash_oversized_write, + test_ext_enc_flash_oversized_write); tcase_set_timeout(ext_flash_operations, 20); tcase_set_timeout(ext_enc_flash_operations, 20); tcase_set_timeout(ext_enc_flash_short_read, 20); tcase_set_timeout(ext_enc_flash_short_write, 20); + tcase_set_timeout(ext_enc_flash_oversized_write, 20); suite_add_tcase(s, ext_flash_operations); suite_add_tcase(s, ext_enc_flash_operations); suite_add_tcase(s, ext_enc_flash_short_read); suite_add_tcase(s, ext_enc_flash_short_write); + suite_add_tcase(s, ext_enc_flash_oversized_write); return s; } From 93edc290823c3b43bcfb433477469bba716c84f3 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 11 Aug 2026 12:16:39 +0200 Subject: [PATCH 07/25] F-7985: enter legacy uImage at ih_ep when it differs from ih_load wolfBoot_start() parsed both ih_load and ih_ep from the U-Boot legacy uImage header, relocated the payload to ih_load, then discarded ih_ep and passed the load address to do_boot(). An image built with the entry point ahead of the load address (a preamble before the entry, as U-Boot bootm handles by copying to ih_load and jumping to ih_ep) was staged correctly but entered at the wrong address. Keep ih_load as the relocation destination and remember ih_ep as the entry point when the two differ, then override load_address just before do_boot(). The override is skipped when a later stage (ELF/FIT) re-derived the load address, since that stage supplies its own entry point. Extend unit-update-ram-uboot with a case where ih_ep = ih_load + 0x40: it asserts the payload lands at ih_load and do_boot() is entered at ih_ep. Fails before this change (jumps to ih_load). --- src/update_ram.c | 21 ++++++++- tools/unit-tests/unit-update-ram-uboot.c | 54 ++++++++++++++++++------ 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/src/update_ram.c b/src/update_ram.c index ff4a01a212..b4d1b662af 100644 --- a/src/update_ram.c +++ b/src/update_ram.c @@ -273,6 +273,10 @@ void RAMFUNCTION wolfBoot_start(void) BENCHMARK_DECLARE(); #ifdef WOLFBOOT_UBOOT_LEGACY uint8_t *image_ptr; + /* uImage ih_load/ih_ep, kept only when the entry point differs from the + * load address (see the do_boot() entry override below). */ + uint32_t *uboot_load = NULL; + uint32_t *uboot_entry = NULL; #endif uint32_t *load_address = NULL; uint32_t *source_address = NULL; @@ -506,6 +510,14 @@ void RAMFUNCTION wolfBoot_start(void) os_image.fw_size); } #endif + /* bootm relocates to ih_load but enters at ih_ep: kernels built + * with a preamble ahead of the entry point set the two to + * different addresses. Remember the entry point; ih_load remains + * the relocation destination. */ + if ((ih_ep != 0) && (ih_ep != ih_load)) { + uboot_load = load_address; + uboot_entry = (uint32_t*)(uintptr_t)ih_ep; + } } else { /* Linux PPC path: leave load_address alone, just advance it * past the header to match upstream behaviour. load_address is @@ -513,7 +525,6 @@ void RAMFUNCTION wolfBoot_start(void) load_address = (uint32_t*)((uint8_t*)load_address + UBOOT_IMG_HDR_SZ); } - (void)ih_ep; /* TODO: pass through to do_boot when ih_ep != ih_load */ } #endif @@ -650,6 +661,14 @@ void RAMFUNCTION wolfBoot_start(void) } #endif /* MMU */ +#ifdef WOLFBOOT_UBOOT_LEGACY + /* Enter the uImage at ih_ep. Skipped if a later stage (ELF/FIT) re-derived + * the load address, since that stage provides its own entry point. */ + if ((uboot_entry != NULL) && (load_address == uboot_load)) { + load_address = uboot_entry; + } +#endif + wolfBoot_printf("Booting at %p\n", load_address); #ifdef WOLFBOOT_ENABLE_WOLFHSM_CLIENT diff --git a/tools/unit-tests/unit-update-ram-uboot.c b/tools/unit-tests/unit-update-ram-uboot.c index 5baa5c1928..b6dd5d8f65 100644 --- a/tools/unit-tests/unit-update-ram-uboot.c +++ b/tools/unit-tests/unit-update-ram-uboot.c @@ -170,11 +170,11 @@ static void cleanup_ram(void) #define DIGEST_TLV_OFF_IN_HDR 28 /* Write a wolfBoot image to the BOOT partition whose firmware payload is a - * uImage: [64-byte uImage header][KERNEL_LEN kernel bytes]. ih_load is set to - * the caller-provided value; the uImage magic/size/header-CRC are made valid so - * uboot_legacy_header_valid() accepts it. Fills expected_kernel[] with the - * kernel pattern for later comparison. Returns 0 on success. */ -static int add_uimage_payload(uint32_t version, uint32_t ih_load) + * uImage: [64-byte uImage header][KERNEL_LEN kernel bytes]. ih_load/ih_ep are + * set to the caller-provided values; the uImage magic/size/header-CRC are made + * valid so uboot_legacy_header_valid() accepts it. Fills expected_kernel[] with + * the kernel pattern for later comparison. Returns 0 on success. */ +static int add_uimage_payload(uint32_t version, uint32_t ih_load, uint32_t ih_ep) { uint8_t *base = (uint8_t *)WOLFBOOT_PARTITION_BOOT_ADDRESS; uint8_t uimg[UBOOT_IMG_HDR_SZ + KERNEL_LEN]; @@ -194,7 +194,7 @@ static int add_uimage_payload(uint32_t version, uint32_t ih_load) /* uimg[4..8] = ih_hcrc, left 0 while computing the header CRC. */ store_be32(uimg + 0x0C, KERNEL_LEN); /* ih_size */ store_be32(uimg + 0x10, ih_load); /* ih_load */ - store_be32(uimg + 0x14, ih_load); /* ih_ep (unused by wolfBoot) */ + store_be32(uimg + 0x14, ih_ep); /* ih_ep */ for (i = 0; i < KERNEL_LEN; i++) { uint8_t b = (uint8_t)(0xA5u ^ (uint8_t)i); uimg[UBOOT_IMG_HDR_SZ + i] = b; @@ -265,21 +265,28 @@ static void fixture_teardown(void) cleanup_flash(); } -/* Run the full wolfBoot_start() flow for a given uImage ih_load and check that - * do_boot() was reached with expect_addr and the kernel payload is present - * there. (mmap setup/teardown is handled by the checked fixture.) */ -static void run_and_check(uint32_t ih_load, uintptr_t expect_addr) +/* Run the full wolfBoot_start() flow for a given uImage ih_load/ih_ep and check + * that the kernel payload landed at expect_load and that do_boot() was reached + * with expect_boot. (mmap setup/teardown is handled by the checked fixture.) */ +static void run_and_check_ep(uint32_t ih_load, uint32_t ih_ep, + uintptr_t expect_load, uintptr_t expect_boot) { - ck_assert_int_eq(add_uimage_payload(1, ih_load), 0); + ck_assert_int_eq(add_uimage_payload(1, ih_load, ih_ep), 0); wolfBoot_start(); ck_assert_int_eq(g_boot_called, 1); - ck_assert_uint_eq((uintptr_t)g_boot_addr, expect_addr); - ck_assert_int_eq(memcmp((void *)expect_addr, expected_kernel, KERNEL_LEN), + ck_assert_uint_eq((uintptr_t)g_boot_addr, expect_boot); + ck_assert_int_eq(memcmp((void *)expect_load, expected_kernel, KERNEL_LEN), 0); } +/* Common case: entry point == load address. */ +static void run_and_check(uint32_t ih_load, uintptr_t expect_addr) +{ + run_and_check_ep(ih_load, ih_load, expect_addr, expect_addr); +} + /* Case 1: ih_load coincides with the staged kernel address -> no relocation * needed; the payload is already there. */ START_TEST (test_uboot_ihload_coincident) @@ -306,7 +313,21 @@ START_TEST (test_uboot_ihload_lower_overlap) } END_TEST -/* Case 4: ih_load == 0 (Linux/PPC convention) -> load_address is only advanced +/* Case 4: ih_ep distinct from ih_load (kernels built with a preamble ahead of + * the entry point). U-Boot bootm copies the payload to ih_load and jumps to + * ih_ep: the payload must land at ih_load, but do_boot() must be entered at + * ih_ep. */ +#define UIMAGE_EP_OFFSET 0x40 +START_TEST (test_uboot_ihep_distinct) +{ + run_and_check_ep((uint32_t)IHLOAD_HI_BASE, + (uint32_t)(IHLOAD_HI_BASE + UIMAGE_EP_OFFSET), + (uintptr_t)IHLOAD_HI_BASE, + (uintptr_t)(IHLOAD_HI_BASE + UIMAGE_EP_OFFSET)); +} +END_TEST + +/* Case 5: ih_load == 0 (Linux/PPC convention) -> load_address is only advanced * past the 64-byte header; behavior is unchanged by the fix. */ START_TEST (test_uboot_ihload_zero) { @@ -320,26 +341,31 @@ Suite *wolfboot_suite(void) TCase *coincident = tcase_create("uImage ih_load coincident"); TCase *higher = tcase_create("uImage ih_load higher"); TCase *lower = tcase_create("uImage ih_load lower overlap"); + TCase *ep = tcase_create("uImage ih_ep distinct"); TCase *zero = tcase_create("uImage ih_load zero"); tcase_add_checked_fixture(coincident, fixture_setup, fixture_teardown); tcase_add_checked_fixture(higher, fixture_setup, fixture_teardown); tcase_add_checked_fixture(lower, fixture_setup, fixture_teardown); + tcase_add_checked_fixture(ep, fixture_setup, fixture_teardown); tcase_add_checked_fixture(zero, fixture_setup, fixture_teardown); tcase_add_test(coincident, test_uboot_ihload_coincident); tcase_add_test(higher, test_uboot_ihload_higher); tcase_add_test(lower, test_uboot_ihload_lower_overlap); + tcase_add_test(ep, test_uboot_ihep_distinct); tcase_add_test(zero, test_uboot_ihload_zero); suite_add_tcase(s, coincident); suite_add_tcase(s, higher); suite_add_tcase(s, lower); + suite_add_tcase(s, ep); suite_add_tcase(s, zero); tcase_set_timeout(coincident, 5); tcase_set_timeout(higher, 5); tcase_set_timeout(lower, 5); + tcase_set_timeout(ep, 5); tcase_set_timeout(zero, 5); return s; From f446a4ad9f773006693bd136370a105de092a715 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 11 Aug 2026 12:23:09 +0200 Subject: [PATCH 08/25] F-8007: wipe TPM advanced-IO staging buffers in TPM2_IoCb() With WOLFTPM_ADV_IO the TIS layer hands the raw command payload to the HAL callback, so TPM2_IoCb() stages it in stack-local txBuf/rxBuf. Both were left intact on the normal return and on the wait-state error return, keeping a TPM command's plaintext authValue (and the response bytes) in bootloader stack SRAM. Wipe them like TPM2_TIS_Read()/TPM2_TIS_Write() already do for their own staging buffers in the non-advanced-IO path. Adds unit-tpm-advio-zeroize, which drives TPM2_IoCb() through the write, read, payload-error and wait-state-timeout paths with a mock SPI slave and inspects the staging buffers afterwards. --- .gitignore | 1 + src/tpm.c | 9 + tools/unit-tests/Makefile | 8 + tools/unit-tests/unit-tpm-advio-zeroize.c | 260 ++++++++++++++++++++++ 4 files changed, 278 insertions(+) create mode 100644 tools/unit-tests/unit-tpm-advio-zeroize.c diff --git a/.gitignore b/.gitignore index 6358c2a6b8..5e93e1ca96 100644 --- a/.gitignore +++ b/.gitignore @@ -204,6 +204,7 @@ tools/unit-tests/unit-otp-keystore tools/unit-tests/unit-otp-keystore-gen-zeroize tools/unit-tests/unit-tpm-api-names tools/unit-tests/unit-tpm-nsc-cert +tools/unit-tests/unit-tpm-advio-zeroize tools/unit-tests/unit-elf-bss-guard tools/unit-tests/unit-fit-fpga tools/unit-tests/unit-flash-erase-c0 diff --git a/src/tpm.c b/src/tpm.c index 08285bee23..6215d97111 100644 --- a/src/tpm.c +++ b/src/tpm.c @@ -202,6 +202,11 @@ static int TPM2_IoCb(TPM2_CTX* ctx, const uint8_t* txBuf, uint8_t* rxBuf, /* On error make sure SPI is de-asserted */ else { spi_xfer(SPI_CS_TPM, NULL, NULL, 0, 0); + #ifdef WOLFTPM_ADV_IO + /* don't leave the command (may hold an authValue) on the stack */ + TPM2_ForceZero(txBuf, sizeof(txBuf)); + TPM2_ForceZero(rxBuf, sizeof(rxBuf)); + #endif return ret; } #else /* Send Entire Message - no wait states */ @@ -221,6 +226,10 @@ static int TPM2_IoCb(TPM2_CTX* ctx, const uint8_t* txBuf, uint8_t* rxBuf, wolfBoot_print_bin(buf, size); #endif } + /* the staging buffers hold the raw command / response, which can carry + * a plaintext authValue - wipe them like TPM2_TIS_Read/Write() do */ + TPM2_ForceZero(txBuf, sizeof(txBuf)); + TPM2_ForceZero(rxBuf, sizeof(rxBuf)); #endif return ret; diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index a9bfb278f3..3f531f4ef6 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -72,6 +72,7 @@ TESTS:=unit-parser unit-fdt unit-extflash unit-string unit-spi-flash unit-aes128 TESTS+=unit-tpm-check-rot-auth TESTS+=unit-tpm-api-names TESTS+=unit-tpm-nsc-cert +TESTS+=unit-tpm-advio-zeroize TESTS+=unit-pkcs11-nsc-zeroize TESTS+=unit-diagnostics TESTS+=unit-diagnostics-256 @@ -324,6 +325,13 @@ unit-tpm-blob: ../../include/target.h unit-tpm-blob.c -DWOLFBOOT_HASH_SHA256 \ -ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections +unit-tpm-advio-zeroize: ../../include/target.h unit-tpm-advio-zeroize.c + gcc -o $@ $^ $(CFLAGS) -I$(WOLFBOOT_LIB_WOLFTPM) -DWOLFBOOT_TPM \ + -DWOLFTPM_USER_SETTINGS -DWOLFTPM_ADV_IO \ + -DWOLFTPM_CHECK_WAIT_STATE -DWOLFBOOT_SIGN_RSA2048 \ + -DWOLFBOOT_HASH_SHA256 \ + -ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections + unit-policy-create: ../../include/target.h unit-policy-create.c \ $(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/memory.c gcc -o $@ $^ -I../tpm $(CFLAGS) -I$(WOLFBOOT_LIB_WOLFTPM) -DWOLFBOOT_TPM \ diff --git a/tools/unit-tests/unit-tpm-advio-zeroize.c b/tools/unit-tests/unit-tpm-advio-zeroize.c new file mode 100644 index 0000000000..ae1d8ff5e7 --- /dev/null +++ b/tools/unit-tests/unit-tpm-advio-zeroize.c @@ -0,0 +1,260 @@ +/* unit-tpm-advio-zeroize.c + * + * Regression test for the WOLFTPM_ADV_IO variant of TPM2_IoCb() in src/tpm.c + * leaving the TPM command/response frame resident in its stack staging + * buffers (txBuf/rxBuf) when it returns. With advanced IO the TIS layer in + * wolfTPM hands the raw payload straight to the HAL callback, so the wipe + * that TPM2_TIS_Read()/TPM2_TIS_Write() perform on their own txBuf/rxBuf + * (lib/wolfTPM/src/tpm2_tis.c) is only done here. A TPM command carrying a + * plaintext password authorization therefore stays readable in bootloader + * stack SRAM after the transfer completes. + * + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfBoot. + * + * wolfBoot is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfBoot is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + */ + +#include +#include +#include +#include +#include + +#ifndef SPI_CS_TPM +#define SPI_CS_TPM 1 +#endif + +#include "wolfboot/wolfboot.h" +#include "tpm.h" +#include "wolftpm/tpm2_tis.h" + +#define ADV_BUF_SZ (MAX_SPI_FRAMESIZE + TPM_TIS_HEADER_SZ) + +/* Plaintext authValue as it would appear inside a TPM2_NV_Write / + * TPM2_Load authorization area handed down to the HAL callback. */ +static const uint8_t test_auth[] = { + 'u', 'n', 'i', 't', '-', 't', 'p', 'm', '-', 'a', 'u', 't', 'h' +}; + +static uint8_t* captured_tx; +static uint8_t* captured_rx; +static int spi_calls; +static int spi_fail_payload; +static int spi_never_ready; + +/* Snapshots of the (now dead) TPM2_IoCb() frame, taken by the tests with an + * inline volatile copy loop so that no intervening call can reuse the stack + * before the contents are inspected. */ +static uint8_t snapshot_tx[ADV_BUF_SZ]; +static uint8_t snapshot_rx[ADV_BUF_SZ]; + +int wolfBoot_printf(const char* fmt, ...) +{ + (void)fmt; + return 0; +} + +void spi_init(int polarity, int phase) +{ + (void)polarity; + (void)phase; +} + +void spi_release(void) +{ +} + +/* Minimal TIS-speaking SPI slave: acknowledges the header (LSB of the last + * header byte set) and answers a payload read with a recognizable pattern. */ +int spi_xfer(int cs, const uint8_t* tx, uint8_t* rx, uint32_t sz, int flags) +{ + uint32_t i; + + (void)cs; + (void)flags; + spi_calls++; + + if (sz == 0) /* de-assert only */ + return 0; + + if (spi_calls == 1) { + captured_tx = (uint8_t*)tx; + captured_rx = rx; + for (i = 0; i < sz; i++) + rx[i] = 0; + if (!spi_never_ready) + rx[sz - 1] = TPM_TIS_READY_MASK; + return 0; + } + + if (spi_fail_payload) + return -1; + + for (i = 0; i < sz; i++) + rx[i] = (uint8_t)(0xA0 + (i & 0x0F)); + return 0; +} + +void TPM2_ForceZero(void* mem, word32 len) +{ + volatile uint8_t* p = (volatile uint8_t*)mem; + word32 i; + + for (i = 0; i < len; i++) + p[i] = 0; +} + +#include "../../src/tpm.c" + +/* Copy the dead frame without calling anything (a memcpy() or a helper + * function would push its own frame over the bytes under test). */ +#define SNAPSHOT_FRAME() \ + do { \ + volatile const uint8_t* _t = (volatile const uint8_t*)captured_tx;\ + volatile const uint8_t* _r = (volatile const uint8_t*)captured_rx;\ + unsigned _i; \ + for (_i = 0; _i < ADV_BUF_SZ; _i++) { \ + snapshot_tx[_i] = _t[_i]; \ + snapshot_rx[_i] = _r[_i]; \ + } \ + } while (0) + +static void assert_no_residue(const uint8_t* snap, const char* which) +{ + unsigned i, j; + + for (i = 0; i + sizeof(test_auth) <= ADV_BUF_SZ; i++) { + for (j = 0; j < sizeof(test_auth); j++) { + if (snap[i + j] != test_auth[j]) + break; + } + ck_assert_msg(j != sizeof(test_auth), + "%s still holds the plaintext TPM authValue at offset %u", which, + i); + } +} + +static void setup(void) +{ + captured_tx = NULL; + captured_rx = NULL; + spi_calls = 0; + spi_fail_payload = 0; + spi_never_ready = 0; + memset(snapshot_tx, 0xFF, sizeof(snapshot_tx)); + memset(snapshot_rx, 0xFF, sizeof(snapshot_rx)); +} + +/* Normal return: the command (including its authorization area) was copied + * into txBuf and must not survive the call. */ +START_TEST(test_advio_write_wipes_txbuf) +{ + int rc; + + rc = TPM2_IoCb(&wolftpm_dev.ctx, 0 /* write */, 0x24, (uint8_t*)test_auth, + (word16)sizeof(test_auth), NULL); + SNAPSHOT_FRAME(); + + ck_assert_int_eq(rc, 0); + ck_assert_ptr_ne(captured_tx, NULL); + assert_no_residue(snapshot_tx, "txBuf"); +} +END_TEST + +/* Wait-state error return: spi_xfer() fails on the payload transfer, so the + * function returns early - the copy of the command is still in txBuf. */ +START_TEST(test_advio_write_wipes_txbuf_on_error) +{ + int rc; + + spi_fail_payload = 1; + rc = TPM2_IoCb(&wolftpm_dev.ctx, 0 /* write */, 0x24, (uint8_t*)test_auth, + (word16)sizeof(test_auth), NULL); + SNAPSHOT_FRAME(); + + ck_assert_int_ne(rc, 0); + ck_assert_ptr_ne(captured_tx, NULL); + assert_no_residue(snapshot_tx, "txBuf"); +} +END_TEST + +/* Timeout error return: the wait-state loop never sees the ready bit, so + * TPM2_IoCb() bails out through the de-assert path. */ +START_TEST(test_advio_write_wipes_txbuf_on_timeout) +{ + int rc; + + spi_never_ready = 1; + rc = TPM2_IoCb(&wolftpm_dev.ctx, 0 /* write */, 0x24, (uint8_t*)test_auth, + (word16)sizeof(test_auth), NULL); + SNAPSHOT_FRAME(); + + ck_assert_int_ne(rc, 0); + ck_assert_ptr_ne(captured_tx, NULL); + assert_no_residue(snapshot_tx, "txBuf"); +} +END_TEST + +/* Read: the TPM response lands in rxBuf and is copied out to the caller; + * the staging copy must not be left behind. */ +START_TEST(test_advio_read_wipes_rxbuf) +{ + uint8_t out[sizeof(test_auth)]; + unsigned i; + int rc; + + rc = TPM2_IoCb(&wolftpm_dev.ctx, 1 /* read */, 0x24, out, + (word16)sizeof(out), NULL); + SNAPSHOT_FRAME(); + + ck_assert_int_eq(rc, 0); + ck_assert_ptr_ne(captured_rx, NULL); + /* the response really was delivered to the caller ... */ + for (i = 0; i < sizeof(out); i++) + ck_assert_uint_eq(out[i], (uint8_t)(0xA0 + (i & 0x0F))); + /* ... and no copy of it remains in the staging buffer */ + for (i = 0; i < ADV_BUF_SZ; i++) { + ck_assert_msg(snapshot_rx[i] == 0, + "rxBuf still holds TPM response byte 0x%02x at offset %u", + snapshot_rx[i], i); + } +} +END_TEST + +static Suite *tpm_advio_zeroize_suite(void) +{ + Suite *s = suite_create("tpm_advio_zeroize"); + TCase *tc = tcase_create("zeroize"); + tcase_add_checked_fixture(tc, setup, NULL); + tcase_add_test(tc, test_advio_write_wipes_txbuf); + tcase_add_test(tc, test_advio_write_wipes_txbuf_on_error); + tcase_add_test(tc, test_advio_write_wipes_txbuf_on_timeout); + tcase_add_test(tc, test_advio_read_wipes_rxbuf); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int failed; + Suite *s = tpm_advio_zeroize_suite(); + SRunner *sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + failed = srunner_ntests_failed(sr); + srunner_free(sr); + return failed == 0 ? 0 : 1; +} From bc743ad3d79b14698d37b635f7be3e6c933cf71e Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 11 Aug 2026 12:48:05 +0200 Subject: [PATCH 09/25] F-7987: abort the swap when a sector copy fails wolfBoot_copy_sector() discarded the return value of every flash operation it performed and unconditionally returned the number of bytes processed. Callers therefore treated a partially written sector as a completed one and advanced the persistent sector flags, which are the only record used to resume an interrupted swap. A write error while copying BOOT into UPDATE (the backup step) could leave both the running image and its backup corrupted with no way to redo the sector. Check the result of every erase/read/write in wolfBoot_copy_sector() and return -1 on the first failure. In the interruptible swap loop, the delta loop and the DISABLE_BACKUP direct copy, stop on a negative return without advancing the sector flag or confirming the boot partition, so the swap is retried from the last completed step on the next boot. --- src/update_flash.c | 92 +++++++++++++++++++++++----- tools/unit-tests/unit-update-flash.c | 22 +++++++ 2 files changed, 97 insertions(+), 17 deletions(-) diff --git a/src/update_flash.c b/src/update_flash.c index fffa5c0d8b..0a7205603c 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -312,22 +312,35 @@ static int RAMFUNCTION wolfBoot_copy_sector(struct wolfBoot_image *src, #define BUFFER_DECLARED static uint8_t buffer[FLASHBUFFER_SIZE] XALIGNED(4); #endif - wb_flash_erase(dst, dst_sector_offset, WOLFBOOT_SECTOR_SIZE); + if (wb_flash_erase(dst, dst_sector_offset, WOLFBOOT_SECTOR_SIZE) < 0) { + ret = -1; + goto out; + } while (pos < WOLFBOOT_SECTOR_SIZE) { if (src_sector_offset + pos < (src->fw_size + IMAGE_HEADER_SIZE + FLASHBUFFER_SIZE)) { /* bypass decryption, copy encrypted data into swap if its external */ if (dst->part == PART_SWAP && SWAP_EXT) { - ext_flash_read((uintptr_t)(src->hdr) + src_sector_offset + pos, - (void *)buffer, FLASHBUFFER_SIZE); + if (ext_flash_read((uintptr_t)(src->hdr) + src_sector_offset + + pos, + (void *)buffer, FLASHBUFFER_SIZE) < 0) { + ret = -1; + goto out; + } } else { - ext_flash_check_read((uintptr_t)(src->hdr) + src_sector_offset + - pos, - (void *)buffer, FLASHBUFFER_SIZE); + if (ext_flash_check_read((uintptr_t)(src->hdr) + + src_sector_offset + pos, + (void *)buffer, FLASHBUFFER_SIZE) < 0) { + ret = -1; + goto out; + } } - wb_flash_write(dst, dst_sector_offset + pos, buffer, - FLASHBUFFER_SIZE); + if (wb_flash_write(dst, dst_sector_offset + pos, buffer, + FLASHBUFFER_SIZE) < 0) { + ret = -1; + goto out; + } } pos += FLASHBUFFER_SIZE; } @@ -335,19 +348,24 @@ static int RAMFUNCTION wolfBoot_copy_sector(struct wolfBoot_image *src, goto out; } #endif - wb_flash_erase(dst, dst_sector_offset, WOLFBOOT_SECTOR_SIZE); + if (wb_flash_erase(dst, dst_sector_offset, WOLFBOOT_SECTOR_SIZE) < 0) { + ret = -1; + goto out; + } while (pos < WOLFBOOT_SECTOR_SIZE) { if (src_sector_offset + pos < (src->fw_size + IMAGE_HEADER_SIZE + FLASHBUFFER_SIZE)) { uint8_t *orig = (uint8_t*)(src->hdr + src_sector_offset + pos); - wb_flash_write(dst, dst_sector_offset + pos, orig, FLASHBUFFER_SIZE); + if (wb_flash_write(dst, dst_sector_offset + pos, orig, + FLASHBUFFER_SIZE) < 0) { + ret = -1; + goto out; + } } pos += FLASHBUFFER_SIZE; } ret = pos; -#if defined(EXT_FLASH) || defined(EXT_ENCRYPTED) out: -#endif #ifdef EXT_ENCRYPTED wolfBoot_zeroize(key, sizeof(key)); wolfBoot_zeroize(nonce, sizeof(nonce)); @@ -605,6 +623,7 @@ static int wolfBoot_delta_update(struct wolfBoot_image *boot, { int sector = 0; int ret; + int copy_ret; uint8_t flag; uint8_t delta_blk[DELTA_BLOCK_SIZE]; uint32_t *img_offset; @@ -777,7 +796,11 @@ static int wolfBoot_delta_update(struct wolfBoot_image *boot, } } if (flag == SECT_FLAG_SWAPPING) { - wolfBoot_copy_sector(swap, boot, sector); + copy_ret = wolfBoot_copy_sector(swap, boot, sector); + if (copy_ret < 0) { + ret = -1; + goto out; + } flag = SECT_FLAG_UPDATED; if (((sector + 1) * WOLFBOOT_SECTOR_SIZE) < WOLFBOOT_PARTITION_SIZE) wolfBoot_set_update_sector_flag(sector, flag); @@ -919,6 +942,7 @@ static int RAMFUNCTION wolfBoot_update(int fallback_allowed) int bootStateRet = -1; uint8_t bootState = 0; #endif + int copy_ret = 0; #if defined(DISABLE_BACKUP) && defined(EXT_ENCRYPTED) uint8_t key[ENCRYPT_KEY_SIZE]; uint8_t nonce[ENCRYPT_NONCE_SIZE]; @@ -1125,7 +1149,9 @@ static int RAMFUNCTION wolfBoot_update(int fallback_allowed) switch (flag) { case SECT_FLAG_NEW: flag = SECT_FLAG_SWAPPING; - wolfBoot_copy_sector(&update, &swap, sector); + copy_ret = wolfBoot_copy_sector(&update, &swap, sector); + if (copy_ret < 0) + break; if (((sector + 1) * sector_size) < WOLFBOOT_PARTITION_SIZE) wolfBoot_set_update_sector_flag(sector, flag); /* FALL THROUGH */ @@ -1145,11 +1171,13 @@ static int RAMFUNCTION wolfBoot_update(int fallback_allowed) */ int prev_iv = wolfBoot_enable_fallback_iv(1); #endif - wolfBoot_copy_sector(&boot, &update, sector); + copy_ret = wolfBoot_copy_sector(&boot, &update, sector); #ifdef EXT_ENCRYPTED wolfBoot_enable_fallback_iv(prev_iv); #endif } + if (copy_ret < 0) + break; if (((sector + 1) * sector_size) < WOLFBOOT_PARTITION_SIZE) wolfBoot_set_update_sector_flag(sector, flag); /* FALL THROUGH */ @@ -1158,7 +1186,9 @@ static int RAMFUNCTION wolfBoot_update(int fallback_allowed) if (size > sector_size) size = sector_size; flag = SECT_FLAG_UPDATED; - wolfBoot_copy_sector(&swap, &boot, sector); + copy_ret = wolfBoot_copy_sector(&swap, &boot, sector); + if (copy_ret < 0) + break; if (((sector + 1) * sector_size) < WOLFBOOT_PARTITION_SIZE) wolfBoot_set_update_sector_flag(sector, flag); break; @@ -1167,6 +1197,20 @@ static int RAMFUNCTION wolfBoot_update(int fallback_allowed) default: break; } + if (copy_ret < 0) { + /* A flash operation failed: do not advance any further, the + * sector flags still describe the last completed step so the + * swap can be resumed from there. */ + wolfBoot_printf("Sector %d copy failed, aborting swap\n", sector); +#ifdef EXT_FLASH + ext_flash_lock(); +#endif + hal_flash_lock(); +#ifdef EXT_ENCRYPTED + wolfBoot_enable_fallback_iv(0); +#endif + return -1; + } sector++; /* headers that can be in different positions depending on when the @@ -1291,7 +1335,21 @@ static int RAMFUNCTION wolfBoot_update(int fallback_allowed) /* Directly copy the content of the UPDATE partition into the BOOT * partition. */ while ((sector * sector_size) < total_size) { - wolfBoot_copy_sector(&update, &boot, sector); + copy_ret = wolfBoot_copy_sector(&update, &boot, sector); + if (copy_ret < 0) { + /* Never confirm a boot image that was not fully written. */ + wolfBoot_printf("Sector %d copy failed, aborting swap\n", sector); +#ifdef EXT_FLASH + ext_flash_lock(); +#endif + hal_flash_lock(); +#ifdef EXT_ENCRYPTED + wolfBoot_zeroize(key, sizeof(key)); + wolfBoot_zeroize(nonce, sizeof(nonce)); + wolfBoot_enable_fallback_iv(0); +#endif + return -1; + } sector++; } /* erase remainder of partition */ diff --git a/tools/unit-tests/unit-update-flash.c b/tools/unit-tests/unit-update-flash.c index 245b15e03b..cee449cc5a 100644 --- a/tools/unit-tests/unit-update-flash.c +++ b/tools/unit-tests/unit-update-flash.c @@ -787,6 +787,27 @@ START_TEST (test_forward_update_samesize) { } END_TEST +/* A failing flash write must abort the swap instead of marking the sector as + * updated: the sector flags are the only record used to resume an + * interrupted swap. */ +START_TEST (test_update_aborts_on_sector_copy_failure) { + uint8_t flag = SECT_FLAG_NEW; + reset_mock_stats(); + prepare_flash(); + add_payload(PART_BOOT, 1, TEST_SIZE_SMALL); + add_payload(PART_UPDATE, 2, TEST_SIZE_SMALL); + wolfBoot_update_trigger(); + /* BOOT is the only internal partition here, so the first write to + * internal flash is the copy of sector 0 from SWAP into BOOT. */ + hal_flash_write_fail = 1; + ck_assert_int_lt(wolfBoot_update(0), 0); + ck_assert_int_eq(hal_flash_write_fail, 0); + wolfBoot_get_update_sector_flag(0, &flag); + ck_assert_int_ne(flag, SECT_FLAG_UPDATED); + cleanup_flash(); +} +END_TEST + START_TEST (test_forward_update_tolarger) { reset_mock_stats(); prepare_flash(); @@ -1602,6 +1623,7 @@ Suite *wolfboot_suite(void) #endif tcase_add_test(sunnyday_noupdate, test_sunnyday_noupdate); tcase_add_test(forward_update_samesize, test_forward_update_samesize); + tcase_add_test(forward_update_samesize, test_update_aborts_on_sector_copy_failure); tcase_add_test(forward_update_tolarger, test_forward_update_tolarger); tcase_add_test(forward_update_tosmaller, test_forward_update_tosmaller); tcase_add_test(forward_update_sameversion_denied, test_forward_update_sameversion_denied); From ab7b79cd16be9d52d2a33e0cbb8ff788a7f097f1 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 11 Aug 2026 12:52:20 +0200 Subject: [PATCH 10/25] F-7383: use one consistent sector size in mcxw hal_flash_erase hal_flash_erase() in hal/mcxw.c rounded the start address down with the runtime pflash_sector_size (queried from FLASH_GetProperty() in hal_init()) but stepped address and len by the compile-time WOLFBOOT_SECTOR_SIZE. When the two differ, a larger WOLFBOOT_SECTOR_SIZE steps over hardware sectors inside the requested range and leaves them unerased, while a smaller one issues erase commands at non-sector-aligned addresses. A zero size reported by the driver would also divide by zero. Take a local sector_size, fall back to WOLFBOOT_SECTOR_SIZE when the driver reports zero and use it for the alignment and both loop steps, as hal/mcxn.c already does. Add unit-flash-erase-mcxw, using the existing WOLFBOOT_UNIT_TEST_FLASH_ERASE guard convention to compile hal_flash_erase() in isolation without the NXP MCUXpresso SDK headers. --- .gitignore | 1 + hal/mcxw.c | 18 ++- tools/unit-tests/Makefile | 10 ++ tools/unit-tests/unit-flash-erase-mcxw.c | 164 +++++++++++++++++++++++ 4 files changed, 189 insertions(+), 4 deletions(-) create mode 100644 tools/unit-tests/unit-flash-erase-mcxw.c diff --git a/.gitignore b/.gitignore index 5e93e1ca96..60b5ec03b2 100644 --- a/.gitignore +++ b/.gitignore @@ -212,6 +212,7 @@ tools/unit-tests/unit-flash-erase-g0 tools/unit-tests/unit-flash-erase-l0 tools/unit-tests/unit-flash-erase-u3 tools/unit-tests/unit-flash-erase-wb +tools/unit-tests/unit-flash-erase-mcxw tools/unit-tests/unit-fwtpm-nv-oob tools/unit-tests/unit-x86-paging-oob tools/unit-tests/unit-ahci-unlock-panic diff --git a/hal/mcxw.c b/hal/mcxw.c index 6a06019c51..5c32e5b5c7 100644 --- a/hal/mcxw.c +++ b/hal/mcxw.c @@ -22,6 +22,7 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA */ +#ifndef WOLFBOOT_UNIT_TEST_FLASH_ERASE #include #include #include "image.h" @@ -44,8 +45,11 @@ /*!< Core clock frequency: 48000000Hz */ #define BOARD_BOOTCLOCKRUN_CORE_CLOCK 48000000U static flash_config_t pflash; +#endif /* !WOLFBOOT_UNIT_TEST_FLASH_ERASE */ + static uint32_t pflash_sector_size = WOLFBOOT_SECTOR_SIZE; +#ifndef WOLFBOOT_UNIT_TEST_FLASH_ERASE uint32_t SystemCoreClock; #ifdef TZEN @@ -223,15 +227,21 @@ static void erase_flash_sector(uint32_t *dst) { /* Wait for completion */ while (!(FMU0->FSTAT & 0x00000080)) {} } +#endif /* !WOLFBOOT_UNIT_TEST_FLASH_ERASE */ int RAMFUNCTION hal_flash_erase(uint32_t address, int len) { - if (address % pflash_sector_size) - address -= address % pflash_sector_size; + uint32_t sector_size = pflash_sector_size; + + if (sector_size == 0U) + sector_size = WOLFBOOT_SECTOR_SIZE; + + if (address % sector_size) + address -= address % sector_size; while (len > 0) { erase_flash_sector((uint32_t *)address); - address += WOLFBOOT_SECTOR_SIZE; - len -= WOLFBOOT_SECTOR_SIZE; + address += sector_size; + len -= (int)sector_size; } return 0; } diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 3f531f4ef6..43e0a2bf79 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -85,6 +85,7 @@ TESTS+=unit-flash-erase-l0 TESTS+=unit-flash-erase-g0 TESTS+=unit-flash-erase-c0 TESTS+=unit-flash-erase-u3 +TESTS+=unit-flash-erase-mcxw TESTS+=unit-otp-keystore TESTS+=unit-otp-keystore-gen-zeroize TESTS+=unit-x86-paging-oob @@ -458,6 +459,15 @@ unit-flash-erase-c0: unit-flash-erase-c0.c ../../hal/stm32c0.c unit-flash-erase-u3: unit-flash-erase-u3.c ../../hal/stm32u3.c ../../hal/stm32u3.h gcc -o $@ unit-flash-erase-u3.c -I../../ $(CFLAGS) $(LDFLAGS) +# unit-flash-erase-mcxw includes hal/mcxw.c directly (guarded to hal_flash_erase +# via WOLFBOOT_UNIT_TEST_FLASH_ERASE), so mcxw.c is not a separate input and the +# (not vendored) NXP MCUXpresso SDK headers are not needed. The erase command +# takes a uint32_t flash address as a pointer, which is only a narrowing cast on +# the 64-bit host. +unit-flash-erase-mcxw: unit-flash-erase-mcxw.c ../../hal/mcxw.c + gcc -o $@ unit-flash-erase-mcxw.c -Wno-int-to-pointer-cast \ + $(CFLAGS) $(LDFLAGS) + # unit-otp-keystore includes src/flash_otp_keystore.c directly (guarded to its # host-portable code via WOLFBOOT_UNIT_TEST_OTP_KEYSTORE), so it is not a # separate input. diff --git a/tools/unit-tests/unit-flash-erase-mcxw.c b/tools/unit-tests/unit-flash-erase-mcxw.c new file mode 100644 index 0000000000..dc8222ce13 --- /dev/null +++ b/tools/unit-tests/unit-flash-erase-mcxw.c @@ -0,0 +1,164 @@ +/* unit-flash-erase-mcxw.c + * + * Unit tests for the sector stride in hal_flash_erase() (hal/mcxw.c). + * Regression for F-7383: the start address was rounded down with the runtime + * pflash_sector_size (queried from FLASH_GetProperty() in hal_init()) while + * the loop stepped address/len by the compile-time WOLFBOOT_SECTOR_SIZE. + * When the two differ, a larger WOLFBOOT_SECTOR_SIZE steps over hardware + * sectors inside the requested range, leaving them unerased. + * hal/mcxn.c already uses one consistent sector_size, with a zero guard. + * + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfBoot. + * + * wolfBoot is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfBoot is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + */ + +#include +#include + +/* hal/mcxw.c is tightly coupled to the MCXW FMU registers and to the (not + * vendored) NXP MCUXpresso SDK headers. Compile only hal_flash_erase() in + * isolation by defining this guard; everything else is excluded and replaced + * below. */ +#define WOLFBOOT_UNIT_TEST_FLASH_ERASE + +/* RAMFUNCTION must be empty on the host */ +#define RAMFUNCTION + +/* Same value as config/examples/mcxw.config */ +#define WOLFBOOT_SECTOR_SIZE 0x2000 + +/* Record every sector-erase command issued by hal_flash_erase(). */ +#define ERASE_LOG_MAX 64 +static uint32_t erase_addr[ERASE_LOG_MAX]; +static int erase_log_n; + +static void erase_flash_sector(uint32_t *dst) +{ + if (erase_log_n < ERASE_LOG_MAX) + erase_addr[erase_log_n] = (uint32_t)(uintptr_t)dst; + erase_log_n++; +} + +#include "../../hal/mcxw.c" + +#define FLASH_BASE 0x00008000UL + +static void reset_mocks(uint32_t sector_size) +{ + erase_log_n = 0; + pflash_sector_size = sector_size; +} + +/* Baseline: runtime and compile-time sector size agree (the stock mcxw + * configuration), so two sectors take exactly two erase commands. */ +START_TEST(test_erase_two_sectors_matching_size) +{ + reset_mocks(WOLFBOOT_SECTOR_SIZE); + ck_assert_int_eq(hal_flash_erase(FLASH_BASE, 2 * WOLFBOOT_SECTOR_SIZE), 0); + + ck_assert_int_eq(erase_log_n, 2); + ck_assert_uint_eq(erase_addr[0], FLASH_BASE); + ck_assert_uint_eq(erase_addr[1], FLASH_BASE + WOLFBOOT_SECTOR_SIZE); +} +END_TEST + +/* Regression for F-7383: the part reports 4KB hardware sectors while + * WOLFBOOT_SECTOR_SIZE is 8KB. Erasing 0x4000 bytes must issue four erase + * commands, one per hardware sector. Before the fix the loop stepped by + * WOLFBOOT_SECTOR_SIZE and issued only two, leaving the sectors at + * FLASH_BASE + 0x1000 and FLASH_BASE + 0x3000 unerased. */ +START_TEST(test_erase_runtime_sector_smaller_covers_range) +{ + int i; + + reset_mocks(0x1000); + ck_assert_int_eq(hal_flash_erase(FLASH_BASE, 0x4000), 0); + + ck_assert_int_eq(erase_log_n, 4); + for (i = 0; i < 4; i++) + ck_assert_uint_eq(erase_addr[i], FLASH_BASE + (uint32_t)i * 0x1000U); +} +END_TEST + +/* The mirror case: the part reports 16KB sectors. Every erase command must + * land on a hardware sector boundary; before the fix the 8KB step issued + * commands in the middle of a sector, and erased the same sector twice. */ +START_TEST(test_erase_runtime_sector_larger_stays_aligned) +{ + reset_mocks(0x4000); + ck_assert_int_eq(hal_flash_erase(FLASH_BASE, 0x8000), 0); + + ck_assert_int_eq(erase_log_n, 2); + ck_assert_uint_eq(erase_addr[0], FLASH_BASE); + ck_assert_uint_eq(erase_addr[1], FLASH_BASE + 0x4000U); +} +END_TEST + +/* An unaligned start address is rounded down to the runtime sector boundary, + * and the stride keeps every following command aligned too. */ +START_TEST(test_erase_unaligned_start_rounds_down) +{ + reset_mocks(0x1000); + ck_assert_int_eq(hal_flash_erase(FLASH_BASE + 0x800, 0x1800), 0); + + ck_assert_int_eq(erase_log_n, 2); + ck_assert_uint_eq(erase_addr[0], FLASH_BASE); + ck_assert_uint_eq(erase_addr[1], FLASH_BASE + 0x1000U); +} +END_TEST + +/* FLASH_GetProperty() failing to report a size must not divide by zero: + * fall back to WOLFBOOT_SECTOR_SIZE, as hal/mcxn.c does. */ +START_TEST(test_erase_zero_runtime_sector_falls_back) +{ + reset_mocks(0); + ck_assert_int_eq(hal_flash_erase(FLASH_BASE + 0x10, WOLFBOOT_SECTOR_SIZE), + 0); + + ck_assert_int_eq(erase_log_n, 1); + ck_assert_uint_eq(erase_addr[0], FLASH_BASE); +} +END_TEST + +Suite *flash_erase_suite(void) +{ + Suite *s = suite_create("flash-erase-mcxw"); + TCase *tc = tcase_create("flash-erase-mcxw"); + + tcase_add_test(tc, test_erase_two_sectors_matching_size); + tcase_add_test(tc, test_erase_runtime_sector_smaller_covers_range); + tcase_add_test(tc, test_erase_runtime_sector_larger_stays_aligned); + tcase_add_test(tc, test_erase_unaligned_start_rounds_down); + tcase_add_test(tc, test_erase_zero_runtime_sector_falls_back); + + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s = flash_erase_suite(); + SRunner *sr = srunner_create(s); + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + + return fails; +} From a799a98c70d1ad9669c1f11190323183f18c3e2d Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 11 Aug 2026 12:54:36 +0200 Subject: [PATCH 11/25] F-7382: clip QSPI page program transfers at the device page boundary spi_flash_write() chunked purely by length, issuing up to a full FLASH_PAGE_SIZE page program at address + page*FLASH_PAGE_SIZE. NOR flash page program wraps within the device's own page, so a transfer starting mid-page (e.g. 0x10F0 with 256 bytes) programmed the tail of the page and then wrapped the rest back over the start of the same page, corrupting already-programmed data and leaving the intended range unwritten. Drive the loop from the running address and clip each transfer to the bytes remaining in the current page, matching src/spi_flash.c. --- src/qspi_flash.c | 21 ++++++++++----------- tools/unit-tests/unit-qspi-flash.c | 27 +++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/src/qspi_flash.c b/src/qspi_flash.c index bc6095b7ea..64638904ff 100644 --- a/src/qspi_flash.c +++ b/src/qspi_flash.c @@ -419,8 +419,8 @@ int spi_flash_write(uint32_t address, const void *data, int len) { int ret = 0; int remaining = len; - uint32_t xferSz, page, pages; - uintptr_t addr; + uint32_t xferSz; + uintptr_t addr = address; uint8_t* ptr = (uint8_t*)data; #ifdef DEBUG_QSPI @@ -437,21 +437,19 @@ int spi_flash_write(uint32_t address, const void *data, int len) return -1; } - /* write by page */ - pages = ((len + (FLASH_PAGE_SIZE-1)) / FLASH_PAGE_SIZE); - for (page = 0; page < pages; page++) { + /* write by page: the device's page program wraps within its own page, so + * each transfer must terminate at the next page boundary */ + while (remaining > 0) { ret = qspi_write_enable(); if (ret != 0) { break; } - xferSz = (uint32_t)remaining; - if (xferSz > FLASH_PAGE_SIZE) { - xferSz = FLASH_PAGE_SIZE; + xferSz = FLASH_PAGE_SIZE - ((uint32_t)addr % FLASH_PAGE_SIZE); + if (xferSz > (uint32_t)remaining) { + xferSz = (uint32_t)remaining; } - addr = address + (page * FLASH_PAGE_SIZE); - /* ------ Write Flash (page at a time) ------ */ ret = qspi_transfer(QSPI_MODE_WRITE, FLASH_WRITE_CMD, addr, QSPI_ADDR_SZ, QSPI_DATA_MODE_SPI, /* Address */ @@ -463,7 +461,7 @@ int spi_flash_write(uint32_t address, const void *data, int len) #ifdef DEBUG_QSPI wolfBoot_printf("QSPI Flash Sector Write: " "Ret %d, Cmd 0x%x, Len %d, %p -> 0x%x\n", - ret, FLASH_WRITE_CMD, xferSz, ptr, address); + ret, FLASH_WRITE_CMD, xferSz, ptr, (uint32_t)addr); #endif if (ret != 0) break; @@ -475,6 +473,7 @@ int spi_flash_write(uint32_t address, const void *data, int len) /* write disable is automatic */ remaining -= (int)xferSz; ptr += xferSz; + addr += xferSz; } return ret; diff --git a/tools/unit-tests/unit-qspi-flash.c b/tools/unit-tests/unit-qspi-flash.c index 344c7272a5..db6cb828c8 100644 --- a/tools/unit-tests/unit-qspi-flash.c +++ b/tools/unit-tests/unit-qspi-flash.c @@ -113,6 +113,32 @@ START_TEST(test_qspi_write_splits_last_page_to_remaining_bytes) } END_TEST +START_TEST(test_qspi_write_clips_first_page_at_page_boundary) +{ + uint8_t buf[FLASH_PAGE_SIZE + 32]; + uint32_t off = FLASH_PAGE_SIZE - 16; + int ret; + + memset(buf, 0x5A, sizeof(buf)); + + /* Start 16 bytes before a page boundary: the device's page program wraps + * within its own page, so the first transfer must stop at the boundary. */ + ret = spi_flash_write(0x1000 + off, buf, sizeof(buf)); + + ck_assert_int_eq(ret, 0); + ck_assert_int_eq(program_call_count, 3); + ck_assert_uint_eq(program_addrs[0], 0x1000 + off); + ck_assert_uint_eq(program_sizes[0], 16); + ck_assert_ptr_eq(program_ptrs[0], buf); + ck_assert_uint_eq(program_addrs[1], 0x1000 + FLASH_PAGE_SIZE); + ck_assert_uint_eq(program_sizes[1], FLASH_PAGE_SIZE); + ck_assert_ptr_eq(program_ptrs[1], buf + 16); + ck_assert_uint_eq(program_addrs[2], 0x1000 + (FLASH_PAGE_SIZE * 2)); + ck_assert_uint_eq(program_sizes[2], 16); + ck_assert_ptr_eq(program_ptrs[2], buf + 16 + FLASH_PAGE_SIZE); +} +END_TEST + START_TEST(test_qspi_write_stops_after_midloop_write_enable_failure) { uint8_t buf[FLASH_PAGE_SIZE * 3]; @@ -176,6 +202,7 @@ static Suite *qspi_flash_suite(void) tc = tcase_create("Write"); tcase_add_checked_fixture(tc, setup, NULL); tcase_add_test(tc, test_qspi_write_splits_last_page_to_remaining_bytes); + tcase_add_test(tc, test_qspi_write_clips_first_page_at_page_boundary); tcase_add_test(tc, test_qspi_write_stops_after_midloop_write_enable_failure); tcase_add_test(tc, test_qspi_read_rejects_address_at_device_size); tcase_add_test(tc, test_qspi_read_rejects_transfer_extending_past_device_size); From 40021b8c81f441131545207a666d534b9b604c76 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 11 Aug 2026 12:59:01 +0200 Subject: [PATCH 12/25] F-7069: clear the EH authValue from the stack in wolfBoot_tpm2_get_timestamp wolfBoot_tpm2_get_timestamp() derives (or copies) the endorsement-hierarchy authValue into the stack-local eh_handle before issuing TPM2_GetTime. The wolfTPM2_UnsetAuth() calls on the way out only clear the copies wolfTPM keeps in the device session slots, and the existing TPM2_ForceZero() only clears the reel master secret, so the derived per-device authValue was left resident in the Secure stack frame after the non-secure entry veneer returned. Wipe eh_handle before returning, matching the scrubbing already done for the master secret. Add unit-tpm-mfgid-eh-zeroize, which captures the handle passed to wolfTPM2_SetIdentityAuth() and snapshots the dead frame on both the success and TPM2_GetTime-error paths. --- src/tpm.c | 2 + tools/unit-tests/Makefile | 8 + tools/unit-tests/unit-tpm-mfgid-eh-zeroize.c | 215 +++++++++++++++++++ 3 files changed, 225 insertions(+) create mode 100644 tools/unit-tests/unit-tpm-mfgid-eh-zeroize.c diff --git a/src/tpm.c b/src/tpm.c index 6215d97111..551485d4ac 100644 --- a/src/tpm.c +++ b/src/tpm.c @@ -1468,6 +1468,8 @@ int CSME_NSE_API wolfBoot_tpm2_get_timestamp(WOLFTPM2_KEY* aik, GetTime_Out* get wolfTPM2_UnsetAuth(&wolftpm_dev, 1); wolfTPM2_UnsetAuth(&wolftpm_dev, 0); + /* EH authValue consumed; clear it from the stack */ + TPM2_ForceZero(&eh_handle, sizeof(eh_handle)); return rc; } diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 43e0a2bf79..2dc3ef9fea 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -73,6 +73,7 @@ TESTS+=unit-tpm-check-rot-auth TESTS+=unit-tpm-api-names TESTS+=unit-tpm-nsc-cert TESTS+=unit-tpm-advio-zeroize +TESTS+=unit-tpm-mfgid-eh-zeroize TESTS+=unit-pkcs11-nsc-zeroize TESTS+=unit-diagnostics TESTS+=unit-diagnostics-256 @@ -333,6 +334,13 @@ unit-tpm-advio-zeroize: ../../include/target.h unit-tpm-advio-zeroize.c -DWOLFBOOT_HASH_SHA256 \ -ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections +unit-tpm-mfgid-eh-zeroize: ../../include/target.h unit-tpm-mfgid-eh-zeroize.c + gcc -o $@ $^ $(CFLAGS) -I$(WOLFBOOT_LIB_WOLFTPM) -DWOLFBOOT_TPM \ + -DWOLFTPM_USER_SETTINGS -DWOLFTPM_MFG_IDENTITY \ + -DWOLFBOOT_TPM_MFG_AUTH_DERIVE -DWOLFBOOT_SIGN_RSA2048 \ + -DWOLFBOOT_HASH_SHA256 -D__ARM_FEATURE_CMSE=3U -DCSME_NSE_API= \ + -ffunction-sections -fdata-sections $(LDFLAGS) -Wl,--gc-sections + unit-policy-create: ../../include/target.h unit-policy-create.c \ $(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/memory.c gcc -o $@ $^ -I../tpm $(CFLAGS) -I$(WOLFBOOT_LIB_WOLFTPM) -DWOLFBOOT_TPM \ diff --git a/tools/unit-tests/unit-tpm-mfgid-eh-zeroize.c b/tools/unit-tests/unit-tpm-mfgid-eh-zeroize.c new file mode 100644 index 0000000000..139de5c806 --- /dev/null +++ b/tools/unit-tests/unit-tpm-mfgid-eh-zeroize.c @@ -0,0 +1,215 @@ +/* unit-tpm-mfgid-eh-zeroize.c + * + * Regression test for wolfBoot_tpm2_get_timestamp() in src/tpm.c leaving the + * endorsement-hierarchy authValue in its stack-local WOLFTPM2_HANDLE when it + * returns. In derive mode (WOLFBOOT_TPM_MFG_AUTH_DERIVE) that value is the + * per-device secret computed by wolfTPM2_SetIdentityAuth() from the reel + * master secret, and it authorises use of the endorsement hierarchy. The + * function already scrubs the master secret from the stack and clears the + * copies wolfTPM keeps in the device session slots (wolfTPM2_UnsetAuth()), + * but the handle holding the derived value is left untouched, so it survives + * in Secure stack SRAM after the non-secure entry veneer returns. + * + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfBoot. + * + * wolfBoot is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfBoot is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + */ + +#include +#include +#include +#include +#include + +#ifndef SPI_CS_TPM +#define SPI_CS_TPM 1 +#endif + +#include "tpm.h" + +/* Size and value of the authValue wolfTPM2_SetIdentityAuth() derives: the low + * 16 bytes of SHA-256(TPM serial || master), see lib/wolfTPM/src/tpm2_wrap.c */ +#define EH_AUTH_SZ 16 +static const uint8_t derived_eh_auth[EH_AUTH_SZ] = { + 0x5A, 0xC3, 0x11, 0x92, 0x7E, 0x40, 0xB6, 0x08, + 0xD1, 0x2F, 0x63, 0xAA, 0x0C, 0x74, 0xE9, 0x35 +}; + +/* The stack-local handle wolfBoot_tpm2_get_timestamp() derives into, captured + * through the wolfTPM entry point it hands the handle to. */ +static WOLFTPM2_HANDLE* captured_eh; +static int gettime_rc; + +/* Snapshot of the (now dead) frame, taken by the tests with an inline + * volatile copy loop so no intervening call can reuse the stack first. */ +static uint8_t snapshot_eh[sizeof(WOLFTPM2_HANDLE)]; + +void *cmse_check_address_range(void *ptr, size_t size, int flags) +{ + (void)size; + (void)flags; + return ptr; +} + +int wolfBoot_printf(const char* fmt, ...) +{ + (void)fmt; + return 0; +} + +/* Stand-in for wolfTPM2_SetIdentityAuth(): the real one hashes the TPM serial + * number with the master secret and stores the low 16 bytes of the digest in + * the handle's auth field. */ +int wolfTPM2_SetIdentityAuth(WOLFTPM2_DEV* dev, WOLFTPM2_HANDLE* handle, + uint8_t* masterPassword, uint16_t masterPasswordSz) +{ + (void)dev; + (void)masterPassword; + (void)masterPasswordSz; + + captured_eh = handle; + handle->auth.size = EH_AUTH_SZ; + memcpy(handle->auth.buffer, derived_eh_auth, EH_AUTH_SZ); + return 0; +} + +int wolfTPM2_SetAuthHandle(WOLFTPM2_DEV* dev, int index, + const WOLFTPM2_HANDLE* handle) +{ + (void)dev; + (void)index; + (void)handle; + return 0; +} + +int wolfTPM2_UnsetAuth(WOLFTPM2_DEV* dev, int index) +{ + if (dev == NULL || index < 0 || index >= MAX_SESSION_NUM) { + return BAD_FUNC_ARG; + } + memset(&dev->session[index], 0, sizeof(dev->session[index])); + return 0; +} + +int wolfTPM2_GetTime(WOLFTPM2_KEY* aikKey, GetTime_Out* getTimeOut) +{ + (void)aikKey; + (void)getTimeOut; + return gettime_rc; +} + +void TPM2_ForceZero(void* mem, word32 len) +{ + volatile uint8_t* p = (volatile uint8_t*)mem; + word32 i; + + for (i = 0; i < len; i++) + p[i] = 0; +} + +#include "../../src/tpm.c" + +/* Copy the dead frame without calling anything (a memcpy() or a helper + * function would push its own frame over the bytes under test). */ +#define SNAPSHOT_EH() \ + do { \ + volatile const uint8_t* _h = (volatile const uint8_t*)captured_eh; \ + unsigned _i; \ + for (_i = 0; _i < sizeof(snapshot_eh); _i++) { \ + snapshot_eh[_i] = _h[_i]; \ + } \ + } while (0) + +static void assert_no_residue(void) +{ + unsigned i, j; + + for (i = 0; i + EH_AUTH_SZ <= sizeof(snapshot_eh); i++) { + for (j = 0; j < EH_AUTH_SZ; j++) { + if (snapshot_eh[i + j] != derived_eh_auth[j]) + break; + } + ck_assert_msg(j != EH_AUTH_SZ, + "eh_handle still holds the derived EH authValue at offset %u", i); + } +} + +static void setup(void) +{ + captured_eh = NULL; + gettime_rc = 0; + memset(snapshot_eh, 0xFF, sizeof(snapshot_eh)); + memset(&wolftpm_dev, 0, sizeof(wolftpm_dev)); +} + +/* Normal return: the derived EH authValue must not survive the veneer. */ +START_TEST(test_get_timestamp_wipes_eh_auth) +{ + WOLFTPM2_KEY aik; + GetTime_Out getTime; + int rc; + + memset(&aik, 0, sizeof(aik)); + rc = wolfBoot_tpm2_get_timestamp(&aik, &getTime); + SNAPSHOT_EH(); + + ck_assert_int_eq(rc, 0); + ck_assert_ptr_ne(captured_eh, NULL); + assert_no_residue(); +} +END_TEST + +/* Error return: TPM2_GetTime fails after the authValue was derived, so the + * handle still holds it on the way out. */ +START_TEST(test_get_timestamp_wipes_eh_auth_on_error) +{ + WOLFTPM2_KEY aik; + GetTime_Out getTime; + int rc; + + memset(&aik, 0, sizeof(aik)); + gettime_rc = TPM_RC_FAILURE; + rc = wolfBoot_tpm2_get_timestamp(&aik, &getTime); + SNAPSHOT_EH(); + + ck_assert_int_ne(rc, 0); + ck_assert_ptr_ne(captured_eh, NULL); + assert_no_residue(); +} +END_TEST + +static Suite *tpm_mfgid_eh_zeroize_suite(void) +{ + Suite *s = suite_create("tpm_mfgid_eh_zeroize"); + TCase *tc = tcase_create("zeroize"); + tcase_add_checked_fixture(tc, setup, NULL); + tcase_add_test(tc, test_get_timestamp_wipes_eh_auth); + tcase_add_test(tc, test_get_timestamp_wipes_eh_auth_on_error); + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int failed; + Suite *s = tpm_mfgid_eh_zeroize_suite(); + SRunner *sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + failed = srunner_ntests_failed(sr); + srunner_free(sr); + return failed == 0 ? 0 : 1; +} From f7551256d03e940371d347d12c8e4f1c90eacb3f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 11 Aug 2026 13:04:10 +0200 Subject: [PATCH 13/25] F-6757: fix partial-word hal_flash_write on nrf52, nrf5340 and stm32l0 The byte-wise branch of hal_flash_write() derived the containing word from the call-time "address" instead of the current position "address + i": int off = (address + i) - (((address + i) >> 2) << 2); dst = (uint32_t *)(address - off); val = dst[i >> 2]; so "dst[i >> 2]" addressed physical byte "address - off + (i & ~3)". Any iteration with "i" not a multiple of 4 modified the wrong byte, and with off != 0 it did so through a misaligned 32-bit flash access (a HardFault on the Cortex-M0+ of stm32l0). A word-aligned 6-byte write, for instance, put data[5] at "address + 4" and left "address + 5" erased. Use the form already applied to hal/samr21.c and hal/same51.c: base the word on "address + i - off", read it with a single aligned access, and fill it byte by byte up to the next word boundary. Add unit-flash-write-nrf52, covering the aligned-with-tail, mismatched alignment and single-word cases against hal/nrf52.c. --- hal/nrf52.c | 16 +- hal/nrf5340.c | 16 +- hal/stm32l0.c | 16 +- tools/unit-tests/Makefile | 4 + tools/unit-tests/unit-flash-write-nrf52.c | 190 ++++++++++++++++++++++ 5 files changed, 224 insertions(+), 18 deletions(-) create mode 100644 tools/unit-tests/unit-flash-write-nrf52.c diff --git a/hal/nrf52.c b/hal/nrf52.c index f88ff22809..1412ac7a36 100644 --- a/hal/nrf52.c +++ b/hal/nrf52.c @@ -83,15 +83,19 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) } else { uint32_t val; uint8_t *vbytes = (uint8_t *)(&val); - int off = (address + i) - (((address + i) >> 2) << 2); - dst = (uint32_t *)(address - off); - val = dst[i >> 2]; - vbytes[off] = data[i]; + uint32_t off = ((address + i) % 4); + dst = (uint32_t *)(address + i - off); + val = *dst; + while (off < 4) { + if (i < len) + vbytes[off++] = data[i++]; + else + off++; + } NVMC_CONFIG = NVMC_CONFIG_WEN; flash_wait_complete(); - dst[i >> 2] = val; + *dst = val; flash_wait_complete(); - i++; } } return 0; diff --git a/hal/nrf5340.c b/hal/nrf5340.c index 2958919812..79f2635a6d 100644 --- a/hal/nrf5340.c +++ b/hal/nrf5340.c @@ -330,18 +330,22 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) } else { uint32_t val; uint8_t *vbytes = (uint8_t *)(&val); - int off = (address + i) - (((address + i) >> 2) << 2); - dst = (uint32_t *)(address - off); - val = dst[i >> 2]; - vbytes[off] = data[i]; + uint32_t off = ((address + i) % 4); + dst = (uint32_t *)(address + i - off); + val = *dst; + while (off < 4) { + if (i < len) + vbytes[off++] = data[i++]; + else + off++; + } #if TZ_SECURE() || defined(TARGET_nrf5340_net) NVMC_CONFIG = NVMC_CONFIG_WEN; #endif NVMC_CONFIGNS = NVMC_CONFIG_WEN; while (NVMC_READY == 0); - dst[i >> 2] = val; + *dst = val; while (NVMC_READY == 0); - i++; } } return 0; diff --git a/hal/stm32l0.c b/hal/stm32l0.c index 6c35e80071..59ffea708c 100644 --- a/hal/stm32l0.c +++ b/hal/stm32l0.c @@ -120,14 +120,18 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) } else { uint32_t val; uint8_t *vbytes = (uint8_t *)(&val); - int off = (address + i) - (((address + i) >> 2) << 2); - dst = (uint32_t *)(address + FLASHMEM_ADDRESS_SPACE - off); - val = dst[i >> 2]; - vbytes[off] = data[i]; + uint32_t off = ((address + i) % 4); + dst = (uint32_t *)(address + FLASHMEM_ADDRESS_SPACE + i - off); + val = *dst; + while (off < 4) { + if (i < len) + vbytes[off++] = data[i++]; + else + off++; + } flash_wait_complete(); - dst[i >> 2] = val; + *dst = val; flash_wait_complete(); - i++; } } return 0; diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 2dc3ef9fea..0e5afc76d9 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -100,6 +100,7 @@ TESTS+=unit-dice-token-size TESTS+=unit-dice-token-nosign TESTS+=unit-va416x0-fram TESTS+=unit-flash-write-mcxa +TESTS+=unit-flash-write-nrf52 TESTS+=unit-flash-write-samr21 TESTS+=unit-flash-write-same51 TESTS+=unit-imx-rt-cache-align @@ -718,6 +719,9 @@ unit-ata-security-passphrase-zeroize: ../../include/target.h unit-ata-security-p unit-flash-write-mcxa: unit-flash-write-mcxa.c ../../hal/mcxa.c gcc -o $@ unit-flash-write-mcxa.c -Imcxa_fsl_stub $(CFLAGS) $(LDFLAGS) +unit-flash-write-nrf52: unit-flash-write-nrf52.c ../../hal/nrf52.c + gcc -o $@ unit-flash-write-nrf52.c -DTARGET_nrf52 -I../../hal $(CFLAGS) $(LDFLAGS) + unit-flash-write-samr21: unit-flash-write-samr21.c ../../hal/samr21.c gcc -o $@ unit-flash-write-samr21.c $(CFLAGS) $(LDFLAGS) diff --git a/tools/unit-tests/unit-flash-write-nrf52.c b/tools/unit-tests/unit-flash-write-nrf52.c new file mode 100644 index 0000000000..18ac09f2fa --- /dev/null +++ b/tools/unit-tests/unit-flash-write-nrf52.c @@ -0,0 +1,190 @@ +/* unit-flash-write-nrf52.c + * + * Regression test for F-6757: in the byte-wise (partial word) path of + * hal_flash_write() (hal/nrf52.c, hal/nrf5340.c, hal/stm32l0.c) the base of + * the containing word was derived from the original (call-time) "address" + * instead of the current position "address + i": + * int off = (address + i) - (((address + i) >> 2) << 2); + * dst = (uint32_t *)(address - off); + * val = dst[i >> 2]; + * "dst[i >> 2]" then addresses physical byte "address - off + (i & ~3)" + * rather than the intended word containing "address + i". For every + * iteration with "i" not a multiple of 4 the wrong destination byte is + * modified (and, when off != 0, through a misaligned 32-bit flash access, + * which faults outright on the Cortex-M0+ of stm32l0). This is the same + * defect already fixed for hal/samr21.c and hal/same51.c under F-5964. + * + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfBoot. + * + * wolfBoot is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfBoot is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + */ + +#include +#include +#include +#include + +#include "image.h" + +#include "../../hal/nrf52.c" + +/* hal_flash_write() polls NVMC_READY and pokes NVMC_CONFIG directly (both + * within one page of the fixed NVMC_BASE); map that page and leave READY + * asserted so the flash state machine appears idle. */ +static void map_nvmc(void) +{ + int flags = MAP_PRIVATE | MAP_ANONYMOUS; +#ifdef MAP_FIXED_NOREPLACE + flags |= MAP_FIXED_NOREPLACE; +#else + flags |= MAP_FIXED; +#endif + void *p = mmap((void *)(uintptr_t)NVMC_BASE, 4096, + PROT_READ | PROT_WRITE, flags, -1, 0); + ck_assert_ptr_eq(p, (void *)(uintptr_t)NVMC_BASE); + NVMC_READY = 1; +} + +static void unmap_nvmc(void) +{ + munmap((void *)(uintptr_t)NVMC_BASE, 4096); +} + +/* "address" is treated as a real pointer into memory-mapped flash. Keep the + * mock flash buffer inside the 32-bit range, matching how "address" (a + * uint32_t) is used by the real target. The buffer sits in the middle of a + * larger mapping so that the out-of-word accesses made by the buggy code + * report as byte mismatches instead of killing the test with SIGSEGV. */ +#define MOCK_FLASH_SIZE 64 +#define MOCK_MAP_SIZE 4096 +#define MOCK_MAP_OFFSET 128 +static uint8_t *mock_map; +static uint8_t *mock_flash; + +static void setup(void) +{ + map_nvmc(); + mock_map = mmap(NULL, MOCK_MAP_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_32BIT, -1, 0); + ck_assert_ptr_ne(mock_map, MAP_FAILED); + memset(mock_map, 0xFF, MOCK_MAP_SIZE); + mock_flash = mock_map + MOCK_MAP_OFFSET; +} + +static void teardown(void) +{ + munmap(mock_map, MOCK_MAP_SIZE); + unmap_nvmc(); +} + +/* Word-aligned destination and source, length not a multiple of 4: the fast + * 32-bit path copies the first word, then the 2-byte tail goes through the + * byte-wise path. Before the fix the second tail byte (i = 5) lands on + * physical byte "address + 4", overwriting the first tail byte and leaving + * "address + 5" erased. */ +START_TEST(test_aligned_write_unaligned_tail) +{ + uint8_t data[6] __attribute__((aligned(4))); + uint32_t base = (uint32_t)(uintptr_t)mock_flash; + int i; + + for (i = 0; i < 6; i++) + data[i] = (uint8_t)(0xA0 + i); + + ck_assert_int_eq(hal_flash_write(base, data, 6), 0); + + for (i = 0; i < 6; i++) + ck_assert_uint_eq(mock_flash[i], data[i]); + for (i = 6; i < MOCK_FLASH_SIZE; i++) + ck_assert_uint_eq(mock_flash[i], 0xFF); +} +END_TEST + +/* Destination misaligned by 1 (mod 4), source buffer misaligned by 2 (mod + * 4): the two never share the same alignment, so the fast 32-bit path is + * never taken and the whole transfer runs through the byte-wise path. */ +START_TEST(test_unaligned_write_mismatched_alignment) +{ + uint8_t rawbuf[64]; + uint8_t *data = rawbuf; + uint32_t base = (uint32_t)(uintptr_t)mock_flash; + int i; + + while (((uintptr_t)data % 4) != 2) + data++; + for (i = 0; i < 8; i++) + data[i] = (uint8_t)(0xC0 + i); + + ck_assert_int_eq(hal_flash_write(base + 5, data, 8), 0); + + for (i = 0; i < 5; i++) + ck_assert_uint_eq(mock_flash[i], 0xFF); + for (i = 0; i < 8; i++) + ck_assert_uint_eq(mock_flash[5 + i], data[i]); + for (i = 13; i < MOCK_FLASH_SIZE; i++) + ck_assert_uint_eq(mock_flash[i], 0xFF); +} +END_TEST + +/* A write that fits entirely inside a single flash word must still work: + * buggy and fixed forms agree here (i is always 0 in the byte-wise path), + * guarding against a fix that breaks the common case. */ +START_TEST(test_unaligned_write_single_word) +{ + uint8_t data[3]; + uint32_t base = (uint32_t)(uintptr_t)mock_flash; + int i; + + for (i = 0; i < 3; i++) + data[i] = (uint8_t)(0xB0 + i); + + ck_assert_int_eq(hal_flash_write(base + 1, data, 3), 0); + + ck_assert_uint_eq(mock_flash[0], 0xFF); + for (i = 0; i < 3; i++) + ck_assert_uint_eq(mock_flash[1 + i], data[i]); + for (i = 4; i < MOCK_FLASH_SIZE; i++) + ck_assert_uint_eq(mock_flash[i], 0xFF); +} +END_TEST + +Suite *flash_write_suite(void) +{ + Suite *s = suite_create("flash-write-nrf52"); + TCase *tc = tcase_create("flash-write-nrf52"); + + tcase_add_checked_fixture(tc, setup, teardown); + tcase_add_test(tc, test_aligned_write_unaligned_tail); + tcase_add_test(tc, test_unaligned_write_mismatched_alignment); + tcase_add_test(tc, test_unaligned_write_single_word); + + suite_add_tcase(s, tc); + return s; +} + +int main(void) +{ + int fails; + Suite *s = flash_write_suite(); + SRunner *sr = srunner_create(s); + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + + return fails; +} From f9fe1386acaabd023e8ff9d920b771ee490a61c7 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 11 Aug 2026 13:19:56 +0200 Subject: [PATCH 14/25] F-6130: clear disk_encrypt_key/nonce on the FIT DTS load failure path dd0712ec added the disk_decrypted_header_clear()/disk_crypto_clear() pair to the wolfBoot_start() panic paths that were missing it, but the FIT flat-device-tree load failure was one more: when wolfBoot_fit_memcpy() fails to relocate the DTS, wolfBoot_panic() is entered with disk_encrypt_key/disk_encrypt_nonce still live in BSS, and that call never returns on a real target. Add unit-update-disk-fit, which drives wolfBoot_start() through the FIT branch with DISK_ENCRYPT enabled and snapshots the module statics from the WOLFBOOT_HOOK_PANIC hook. --- .gitignore | 1 + src/update_disk.c | 4 + tools/unit-tests/Makefile | 11 +- tools/unit-tests/unit-update-disk-fit.c | 356 ++++++++++++++++++++++++ 4 files changed, 371 insertions(+), 1 deletion(-) create mode 100644 tools/unit-tests/unit-update-disk-fit.c diff --git a/.gitignore b/.gitignore index 60b5ec03b2..598af99037 100644 --- a/.gitignore +++ b/.gitignore @@ -224,6 +224,7 @@ tools/unit-tests/unit-flash-write-samr21 tools/unit-tests/unit-image-elf-scatter tools/unit-tests/unit-image-hybrid tools/unit-tests/unit-imx-rt-cache-align +tools/unit-tests/unit-update-disk-fit tools/unit-tests/unit-update-disk-oob tools/unit-tests/unit-update-ram-enc tools/unit-tests/unit-update-ram-enc-nopart diff --git a/src/update_disk.c b/src/update_disk.c index d30a37f770..e833b9f4ac 100644 --- a/src/update_disk.c +++ b/src/update_disk.c @@ -633,6 +633,10 @@ void RAMFUNCTION wolfBoot_start(void) dts_ptr, dts_addr, dts_size); if (wolfBoot_fit_memcpy(dts_addr, dts_ptr, dts_size) != 0) { wolfBoot_printf("FIT: failed to load DTS\r\n"); +#ifdef DISK_ENCRYPT + disk_decrypted_header_clear(dec_hdr); + disk_crypto_clear(); +#endif wolfBoot_panic(); } } diff --git a/tools/unit-tests/Makefile b/tools/unit-tests/Makefile index 0e5afc76d9..1e629f3d88 100644 --- a/tools/unit-tests/Makefile +++ b/tools/unit-tests/Makefile @@ -62,7 +62,7 @@ TESTS:=unit-parser unit-fdt unit-extflash unit-string unit-spi-flash unit-aes128 unit-update-flash-hook \ unit-update-flash-self-update \ unit-update-flash-enc unit-update-ram unit-update-ram-uboot unit-update-ram-enc unit-update-ram-enc-nopart unit-update-ram-nofixed unit-update-ram-noramboot unit-update-flash-hwswap unit-pkcs11_store unit-psa_store unit-wolfhsm_flash_hal unit-disk \ - unit-update-disk unit-update-disk-oob unit-multiboot unit-boot-x86-fsp unit-loader-tpm-init unit-qspi-flash unit-fwtpm-stub unit-tpm-rsa-exp \ + unit-update-disk unit-update-disk-oob unit-update-disk-fit unit-multiboot unit-boot-x86-fsp unit-loader-tpm-init unit-qspi-flash unit-fwtpm-stub unit-tpm-rsa-exp \ unit-image-nopart unit-image-sha384 unit-image-sha3-384 unit-store-sbrk \ unit-tpm-blob unit-policy-create unit-policy-sign unit-rot-auth unit-sdhci-response-bits \ unit-sdhci-disk-unaligned unit-sign-encrypted-output \ @@ -239,6 +239,12 @@ unit-update-disk:CFLAGS+=-DMOCK_PARTITIONS -DPRINTF_ENABLED -DWOLFBOOT_RAMBOOT_M unit-update-disk-oob:CFLAGS+=-DMOCK_PARTITIONS -DPRINTF_ENABLED \ -DWOLFBOOT_RAMBOOT_MAX_SIZE=0x1000 \ -DWOLFBOOT_ORIGIN=MOCK_ADDRESS_BOOT -DBOOTLOADER_PARTITION_SIZE=WOLFBOOT_PARTITION_SIZE +# FIT (flattened uImage tree) exits of the encrypted disk loader. The panic hook +# is what lets the test observe the key material at the instant wolfBoot_panic() +# is entered, since on target that call never returns. +unit-update-disk-fit:CFLAGS+=-DMOCK_PARTITIONS -DPRINTF_ENABLED -DWOLFBOOT_FDT \ + -DWOLFBOOT_HOOK_PANIC -DWOLFBOOT_RAMBOOT_MAX_SIZE=0x40 \ + -DWOLFBOOT_ORIGIN=MOCK_ADDRESS_BOOT -DBOOTLOADER_PARTITION_SIZE=WOLFBOOT_PARTITION_SIZE # Regression coverage for wolfBoot_check_flash_image_elf() (scattered-ELF # integrity check). WOLFBOOT_NO_SIGN keeps this to the hashing path only (no # signature verification is exercised by that function). @@ -675,6 +681,9 @@ unit-update-disk: ../../include/target.h unit-update-disk.c unit-update-disk-oob: ../../include/target.h unit-update-disk-oob.c gcc -o $@ unit-update-disk-oob.c $(CFLAGS) $(LDFLAGS) +unit-update-disk-fit: ../../include/target.h unit-update-disk-fit.c + gcc -o $@ unit-update-disk-fit.c $(CFLAGS) $(LDFLAGS) + unit-pkcs11_store: ../../include/target.h unit-pkcs11_store.c gcc -o $@ $(WOLFCRYPT_SRC) unit-pkcs11_store.c $(CFLAGS) $(WOLFCRYPT_CFLAGS) $(LDFLAGS) diff --git a/tools/unit-tests/unit-update-disk-fit.c b/tools/unit-tests/unit-update-disk-fit.c new file mode 100644 index 0000000000..eb3a9f670d --- /dev/null +++ b/tools/unit-tests/unit-update-disk-fit.c @@ -0,0 +1,356 @@ +/* unit-update-disk-fit.c + * + * Regression coverage for the FIT (flattened uImage tree) exit paths of + * wolfBoot_start() in src/update_disk.c, with DISK_ENCRYPT enabled. + * + * Every terminal exit of wolfBoot_start() must scrub the disk decryption + * key/nonce before handing control away. wolfBoot_panic() is an unbounded + * spin on real targets, so key material left resident there stays resident + * forever. These tests snapshot the module statics from the panic hook, + * which runs at the top of wolfBoot_panic(), i.e. exactly at the moment the + * bootloader stops making progress. + * + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfBoot. + * + * wolfBoot is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfBoot is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + */ + +#define WOLFBOOT_UPDATE_DISK +#define WOLFBOOT_SELF_UPDATE_MONOLITHIC +#define RAM_CODE +#define WOLFBOOT_SELF_HEADER +#define EXT_ENCRYPTED +#define ENCRYPT_WITH_CHACHA +#define HAVE_CHACHA +#define IMAGE_HEADER_SIZE 256 +#define BOOT_PART_A 0 +#define BOOT_PART_B 1 +#define MOCK_ADDRESS_BOOT 0xCD000000 + +#include +#include +#include +#include + +#include "hal.h" +#include "target.h" +#include "wolfboot/wolfboot.h" +#include "image.h" +#include "loader.h" +#include + +#define TEST_PAYLOAD_SIZE 64 +#define TEST_DTS_SIZE 32 + +static uint8_t load_buffer[TEST_PAYLOAD_SIZE]; +#define WOLFBOOT_LOAD_ADDRESS ((uintptr_t)load_buffer) + +static uint8_t dts_buffer[TEST_DTS_SIZE]; +#define WOLFBOOT_LOAD_DTS_ADDRESS ((uintptr_t)dts_buffer) + +static uint8_t part_a_image[IMAGE_HEADER_SIZE + TEST_PAYLOAD_SIZE]; +static uint8_t part_b_image[IMAGE_HEADER_SIZE + TEST_PAYLOAD_SIZE]; +static uint8_t fit_dts_image[TEST_DTS_SIZE]; +static int mock_do_boot_called; +static int mock_fit_memcpy_ret; +static int mock_fit_memcpy_called; +static int mock_panic_hook_called; +/* Snapshot of the key material taken from inside wolfBoot_panic() */ +static uint8_t panic_key_snapshot[ENCRYPT_KEY_SIZE]; +static uint8_t panic_nonce_snapshot[ENCRYPT_NONCE_SIZE]; + +ChaCha chacha; + +static void set_u16_le(uint8_t *dst, uint16_t value) +{ + dst[0] = (uint8_t)(value & 0xFF); + dst[1] = (uint8_t)(value >> 8); +} + +static void set_u32_le(uint8_t *dst, uint32_t value) +{ + dst[0] = (uint8_t)(value & 0xFF); + dst[1] = (uint8_t)((value >> 8) & 0xFF); + dst[2] = (uint8_t)((value >> 16) & 0xFF); + dst[3] = (uint8_t)(value >> 24); +} + +static void build_image(uint8_t *image, uint32_t version, uint8_t fill) +{ + memset(image, 0, IMAGE_HEADER_SIZE + TEST_PAYLOAD_SIZE); + set_u32_le(image, WOLFBOOT_MAGIC); + set_u32_le(image + sizeof(uint32_t), TEST_PAYLOAD_SIZE); + set_u16_le(image + IMAGE_HEADER_OFFSET, HDR_VERSION); + set_u16_le(image + IMAGE_HEADER_OFFSET + sizeof(uint16_t), 4); + set_u32_le(image + IMAGE_HEADER_OFFSET + 2 * sizeof(uint16_t), version); + memset(image + IMAGE_HEADER_SIZE, fill, TEST_PAYLOAD_SIZE); +} + +static void reset_mocks(void) +{ + memset(load_buffer, 0, sizeof(load_buffer)); + memset(dts_buffer, 0, sizeof(dts_buffer)); + build_image(part_a_image, 1, 0xA1); + build_image(part_b_image, 2, 0xB2); + memset(fit_dts_image, 0xDD, sizeof(fit_dts_image)); + mock_do_boot_called = 0; + mock_fit_memcpy_ret = 0; + mock_fit_memcpy_called = 0; + mock_panic_hook_called = 0; + memset(panic_key_snapshot, 0xFF, sizeof(panic_key_snapshot)); + memset(panic_nonce_snapshot, 0xFF, sizeof(panic_nonce_snapshot)); + wolfBoot_panicked = 0; +} + +int chacha_init(void) +{ + return 0; +} + +int wc_Chacha_SetIV(ChaCha* ctx, const byte* inIv, word32 counter) +{ + (void)ctx; + (void)inIv; + (void)counter; + return 0; +} + +int wc_Chacha_Process(ChaCha* ctx, byte* output, const byte* input, word32 msglen) +{ + (void)ctx; + memmove(output, input, msglen); + return 0; +} + +void ForceZero(void* mem, size_t len) +{ + volatile uint8_t *p = (volatile uint8_t *)mem; + while (len-- > 0) { + *p++ = 0; + } +} + +int wolfBoot_initialize_encryption(void) +{ + return 0; +} + +int wolfBoot_get_encrypt_key(uint8_t *key, uint8_t *nonce) +{ + memset(key, 0x5A, ENCRYPT_KEY_SIZE); + memset(nonce, 0xC3, ENCRYPT_NONCE_SIZE); + return 0; +} + +int disk_init(int drv) +{ + (void)drv; + return 0; +} + +int disk_open(int drv) +{ + (void)drv; + return 0; +} + +void disk_close(int drv) +{ + (void)drv; +} + +int disk_part_read(int drv, int part, uint64_t off, uint64_t sz, uint8_t *buf) +{ + uint8_t *image; + uint64_t max = IMAGE_HEADER_SIZE + TEST_PAYLOAD_SIZE; + + (void)drv; + image = (part == BOOT_PART_B) ? part_b_image : part_a_image; + if ((off > max) || (sz > (max - off))) + return -1; + memcpy(buf, image + off, (size_t)sz); + return (int)sz; +} + +int wolfBoot_open_image_address(struct wolfBoot_image* img, uint8_t* image) +{ + uint32_t magic; + uint32_t fw_size; + + memcpy(&magic, image, sizeof(magic)); + + if (magic != WOLFBOOT_MAGIC) + return -1; + memset(img, 0, sizeof(*img)); + img->hdr = image; + memcpy(&fw_size, image + sizeof(uint32_t), sizeof(fw_size)); + img->fw_size = fw_size; + img->fw_base = image + IMAGE_HEADER_SIZE; + img->hdr_ok = 1; + return 0; +} + +int wolfBoot_verify_integrity(struct wolfBoot_image* img) +{ + img->sha_ok = 1; + return 0; +} + +int wolfBoot_verify_authenticity(struct wolfBoot_image* img) +{ + img->signature_ok = 1; + return 0; +} + +/* The loaded payload is treated as a FIT container, and the sub-image + * returned by fit_load_image() is a valid flat device tree. */ +int wolfBoot_get_dts_size(void *dts_addr) +{ + (void)dts_addr; + return TEST_DTS_SIZE; +} + +/* Only reached through the fdt_version()/fdt_totalsize() trace macros here. */ +uint32_t fdt32_to_cpu(uint32_t x) +{ + return ((x & 0x000000FFU) << 24) | ((x & 0x0000FF00U) << 8) | + ((x & 0x00FF0000U) >> 8) | ((x & 0xFF000000U) >> 24); +} + +const char* fit_find_images(void* fdt, const char** pkernel, + const char** pflat_dt, const char** pramdisk, const char** pfpga) +{ + (void)fdt; + if (pkernel != NULL) + *pkernel = NULL; + if (pflat_dt != NULL) + *pflat_dt = "fdt"; + if (pramdisk != NULL) + *pramdisk = NULL; + if (pfpga != NULL) + *pfpga = NULL; + return "conf"; +} + +void* fit_load_image(void* fdt, const char* image, int* lenp) +{ + (void)fdt; + (void)image; + if (lenp != NULL) + *lenp = TEST_DTS_SIZE; + return fit_dts_image; +} + +int wolfBoot_fit_memcpy(void *dst, const void *src, uint32_t len) +{ + mock_fit_memcpy_called++; + if (mock_fit_memcpy_ret != 0) + return mock_fit_memcpy_ret; + memcpy(dst, src, len); + return 0; +} + +void hal_prepare_boot(void) +{ +} + +void do_boot(const uint32_t *address, const uint32_t *dts_address) +{ + (void)dts_address; + (void)address; + mock_do_boot_called++; +} + +int hal_flash_protect(haladdr_t address, int len) +{ + (void)address; + (void)len; + return 0; +} + +#include "update_disk.c" + +/* Runs from inside wolfBoot_panic(), before it spins forever on target. */ +void wolfBoot_hook_panic(void) +{ + mock_panic_hook_called++; + memcpy(panic_key_snapshot, disk_encrypt_key, sizeof(panic_key_snapshot)); + memcpy(panic_nonce_snapshot, disk_encrypt_nonce, + sizeof(panic_nonce_snapshot)); +} + +static void assert_snapshot_zeroized(void) +{ + size_t i; + + for (i = 0; i < sizeof(panic_key_snapshot); i++) { + ck_assert_uint_eq(panic_key_snapshot[i], 0); + } + for (i = 0; i < sizeof(panic_nonce_snapshot); i++) { + ck_assert_uint_eq(panic_nonce_snapshot[i], 0); + } +} + +START_TEST(test_update_disk_fit_dts_copy_failure_zeroizes_key_material) +{ + reset_mocks(); + mock_fit_memcpy_ret = -1; + + wolfBoot_start(); + + ck_assert_int_gt(mock_fit_memcpy_called, 0); + ck_assert_int_gt(wolfBoot_panicked, 0); + ck_assert_int_gt(mock_panic_hook_called, 0); + assert_snapshot_zeroized(); +} +END_TEST + +START_TEST(test_update_disk_fit_dts_copy_success_boots) +{ + reset_mocks(); + + wolfBoot_start(); + + ck_assert_int_eq(wolfBoot_panicked, 0); + ck_assert_int_eq(mock_do_boot_called, 1); + ck_assert_int_eq(memcmp(dts_buffer, fit_dts_image, TEST_DTS_SIZE), 0); +} +END_TEST + +Suite *wolfboot_suite(void) +{ + Suite *s = suite_create("wolfBoot"); + TCase *tc = tcase_create("update-disk-fit"); + + tcase_add_test(tc, test_update_disk_fit_dts_copy_failure_zeroizes_key_material); + tcase_add_test(tc, test_update_disk_fit_dts_copy_success_boots); + suite_add_tcase(s, tc); + + return s; +} + +int main(void) +{ + int fails; + Suite *s = wolfboot_suite(); + SRunner *sr = srunner_create(s); + + srunner_run_all(sr, CK_NORMAL); + fails = srunner_ntests_failed(sr); + srunner_free(sr); + return fails; +} From 4726dce42ba787ce892d1eb12571568ad7588364 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 11 Aug 2026 13:24:32 +0200 Subject: [PATCH 15/25] update_disk: use wc_ForceZero() in the DISK_ENCRYPT helpers ForceZero() is only visible in libwolfboot.c, which pulls in misc.c inline; update_disk.c called it without a declaration, so any config with disk encryption failed to build. Use the exported wc_ForceZero() from memory.o instead, which is always linked. Add a zynqmp_sdcard ENCRYPT build job to CI, the only one that compiles these paths. --- .github/workflows/test-configs.yml | 8 ++++++++ src/update_disk.c | 7 ++++--- tools/unit-tests/unit-update-disk-fit.c | 2 +- tools/unit-tests/unit-update-disk.c | 2 +- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/.github/workflows/test-configs.yml b/.github/workflows/test-configs.yml index 6332801975..825c2ad247 100644 --- a/.github/workflows/test-configs.yml +++ b/.github/workflows/test-configs.yml @@ -784,6 +784,14 @@ jobs: arch: aarch64 config-file: ./config/examples/zynqmp_sdcard.config + # Only build that compiles the DISK_ENCRYPT paths of src/update_disk.c. + zynqmp_sdcard_encrypt_test: + uses: ./.github/workflows/test-build-aarch64.yml + with: + arch: aarch64 + config-file: ./config/examples/zynqmp_sdcard.config + make-args: ENCRYPT=1 ENCRYPT_WITH_AES256=1 + zynqmp_fsbl_test: uses: ./.github/workflows/test-build-aarch64.yml with: diff --git a/src/update_disk.c b/src/update_disk.c index e833b9f4ac..ce3def06a1 100644 --- a/src/update_disk.c +++ b/src/update_disk.c @@ -56,6 +56,7 @@ defined(ENCRYPT_WITH_CHACHA) #define DISK_ENCRYPT #include "encrypt.h" +#include /* wc_ForceZero */ /* Module-level storage for encryption nonce */ static uint8_t disk_encrypt_nonce[ENCRYPT_NONCE_SIZE]; @@ -235,13 +236,13 @@ static int decrypt_header(const uint8_t *src, uint8_t *dst) static void disk_crypto_clear(void) { - ForceZero(disk_encrypt_key, sizeof(disk_encrypt_key)); - ForceZero(disk_encrypt_nonce, sizeof(disk_encrypt_nonce)); + wc_ForceZero(disk_encrypt_key, sizeof(disk_encrypt_key)); + wc_ForceZero(disk_encrypt_nonce, sizeof(disk_encrypt_nonce)); } static void disk_decrypted_header_clear(uint8_t *hdr) { - ForceZero(hdr, IMAGE_HEADER_SIZE); + wc_ForceZero(hdr, IMAGE_HEADER_SIZE); } #endif /* DISK_ENCRYPT */ diff --git a/tools/unit-tests/unit-update-disk-fit.c b/tools/unit-tests/unit-update-disk-fit.c index eb3a9f670d..5799ada798 100644 --- a/tools/unit-tests/unit-update-disk-fit.c +++ b/tools/unit-tests/unit-update-disk-fit.c @@ -136,7 +136,7 @@ int wc_Chacha_Process(ChaCha* ctx, byte* output, const byte* input, word32 msgle return 0; } -void ForceZero(void* mem, size_t len) +void wc_ForceZero(void* mem, size_t len) { volatile uint8_t *p = (volatile uint8_t *)mem; while (len-- > 0) { diff --git a/tools/unit-tests/unit-update-disk.c b/tools/unit-tests/unit-update-disk.c index 072f239e6a..dae1349d7b 100644 --- a/tools/unit-tests/unit-update-disk.c +++ b/tools/unit-tests/unit-update-disk.c @@ -108,7 +108,7 @@ int wc_Chacha_Process(ChaCha* ctx, byte* output, const byte* input, word32 msgle return 0; } -void ForceZero(void* mem, size_t len) +void wc_ForceZero(void* mem, size_t len) { volatile uint8_t *p = (volatile uint8_t *)mem; while (len-- > 0) { From db8e768bbc8b4abb70ec527e4c58511f58c41cc1 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 11 Aug 2026 13:25:36 +0200 Subject: [PATCH 16/25] aarch64: link the ARM ChaCha port when ChaCha is selected Under WOLFSSL_ARMASM, chacha.c calls wc_chacha_crypt_bytes(), which arch.mk never adds for AArch64 -- it only pulls in the aes/sha ports. Any AArch64 build using ChaCha failed to link. Add the object in options.mk, where ChaCha is selected. Also add a ChaCha variant of the zynqmp_sdcard ENCRYPT build to CI. --- .github/workflows/test-configs.yml | 7 +++++++ options.mk | 11 +++++++++++ 2 files changed, 18 insertions(+) diff --git a/.github/workflows/test-configs.yml b/.github/workflows/test-configs.yml index 825c2ad247..5b8d3566ba 100644 --- a/.github/workflows/test-configs.yml +++ b/.github/workflows/test-configs.yml @@ -792,6 +792,13 @@ jobs: config-file: ./config/examples/zynqmp_sdcard.config make-args: ENCRYPT=1 ENCRYPT_WITH_AES256=1 + zynqmp_sdcard_encrypt_chacha_test: + uses: ./.github/workflows/test-build-aarch64.yml + with: + arch: aarch64 + config-file: ./config/examples/zynqmp_sdcard.config + make-args: ENCRYPT=1 ENCRYPT_WITH_CHACHA=1 + zynqmp_fsbl_test: uses: ./.github/workflows/test-build-aarch64.yml with: diff --git a/options.mk b/options.mk index 739a5b693b..0f340fa856 100644 --- a/options.mk +++ b/options.mk @@ -1885,3 +1885,14 @@ endif # includers (test-app), where a self-referencing += would not terminate. AUX_WOLFCRYPT_OBJS_NEW:=$(filter-out $(WOLFCRYPT_OBJS),$(sort $(AUX_WOLFCRYPT_OBJS))) WOLFCRYPT_OBJS+=$(AUX_WOLFCRYPT_OBJS_NEW) + +# Under WOLFSSL_ARMASM, chacha.c defers the block function to +# wc_chacha_crypt_bytes(), which lives in the port. arch.mk adds the aes/sha +# equivalents unconditionally; ChaCha is only selected here, so add it last. +ifeq ($(ARCH),AARCH64) + ifneq ($(NO_ARM_ASM),1) + ifneq (,$(filter %/wolfcrypt/src/chacha.o,$(WOLFCRYPT_OBJS))) + WOLFCRYPT_OBJS+=$(WOLFBOOT_LIB_WOLFSSL)/wolfcrypt/src/port/arm/armv8-chacha-asm_c.o + endif + endif +endif From e24fff0a38703fab99aaf1bc546ac3074f1313eb Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 11 Aug 2026 17:20:24 +0200 Subject: [PATCH 17/25] libwolfboot: declare ForceZero() in the test-app build The encrypted key handling calls ForceZero(), but misc.c is only included inline under __WOLFBOOT or UNIT_TEST. The test-app build of libwolfboot.c defines neither, so on MMU targets with EXT_ENCRYPTED the call had no declaration. GCC 14 rejects that; older compilers only warned. Add EXT_ENCRYPTED to the guard, which keeps ForceZero() static and adds no link dependency. --- src/libwolfboot.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index 6e04dc50d6..7994e3cabd 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -183,7 +183,10 @@ static uint32_t ext_cache; #endif -#if defined(__WOLFBOOT) || defined(UNIT_TEST) +/* EXT_ENCRYPTED is listed because the key-handling code below calls + * ForceZero() unconditionally, including from the test-app build of this file, + * where __WOLFBOOT is not defined. */ +#if defined(__WOLFBOOT) || defined(UNIT_TEST) || defined(EXT_ENCRYPTED) #define WOLFSSL_MISC_INCLUDED /* allow misc.c code to be inlined */ #include #include From 31a9adc6d26228e7816cfd148931efb11404f620 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 11 Aug 2026 17:26:28 +0200 Subject: [PATCH 18/25] test: bump footprint limits The error handling added by the fixes in this branch costs 40 bytes of common code, so every stm32f407-discovery configuration grew by that amount. Raise each limit by 40, keeping the previous headroom. --- tools/test.mk | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/tools/test.mk b/tools/test.mk index 2f3c4f7260..fb85614547 100644 --- a/tools/test.mk +++ b/tools/test.mk @@ -1185,52 +1185,52 @@ test-all: clean test-size-all: - make test-size SIGN=NONE LIMIT=5072 NO_ARM_ASM=1 + make test-size SIGN=NONE LIMIT=5112 NO_ARM_ASM=1 make keysclean - make test-size SIGN=ED25519 LIMIT=12184 NO_ARM_ASM=1 + make test-size SIGN=ED25519 LIMIT=12224 NO_ARM_ASM=1 make keysclean - make test-size SIGN=ECC256 LIMIT=18880 NO_ARM_ASM=1 + make test-size SIGN=ECC256 LIMIT=18920 NO_ARM_ASM=1 make clean - make test-size SIGN=ECC256 NO_ASM=1 LIMIT=13912 NO_ARM_ASM=1 + make test-size SIGN=ECC256 NO_ASM=1 LIMIT=13952 NO_ARM_ASM=1 make keysclean - make test-size SIGN=RSA2048 LIMIT=11768 NO_ARM_ASM=1 + make test-size SIGN=RSA2048 LIMIT=11808 NO_ARM_ASM=1 make clean - make test-size SIGN=RSA2048 NO_ASM=1 LIMIT=12328 NO_ARM_ASM=1 + make test-size SIGN=RSA2048 NO_ASM=1 LIMIT=12368 NO_ARM_ASM=1 make keysclean - make test-size SIGN=RSA4096 LIMIT=12068 NO_ARM_ASM=1 + make test-size SIGN=RSA4096 LIMIT=12108 NO_ARM_ASM=1 make clean - make test-size SIGN=RSA4096 NO_ASM=1 LIMIT=12608 NO_ARM_ASM=1 + make test-size SIGN=RSA4096 NO_ASM=1 LIMIT=12648 NO_ARM_ASM=1 make keysclean - make test-size SIGN=ECC384 LIMIT=19564 NO_ARM_ASM=1 + make test-size SIGN=ECC384 LIMIT=19604 NO_ARM_ASM=1 make clean - make test-size SIGN=ECC384 NO_ASM=1 LIMIT=15260 NO_ARM_ASM=1 + make test-size SIGN=ECC384 NO_ASM=1 LIMIT=15300 NO_ARM_ASM=1 make keysclean - make test-size SIGN=ED448 LIMIT=14212 NO_ARM_ASM=1 + make test-size SIGN=ED448 LIMIT=14252 NO_ARM_ASM=1 make keysclean - make test-size SIGN=RSA3072 LIMIT=11908 NO_ARM_ASM=1 + make test-size SIGN=RSA3072 LIMIT=11948 NO_ARM_ASM=1 make clean - make test-size SIGN=RSA3072 NO_ASM=1 LIMIT=12436 NO_ARM_ASM=1 + make test-size SIGN=RSA3072 NO_ASM=1 LIMIT=12476 NO_ARM_ASM=1 make keysclean - make test-size SIGN=RSAPSS2048 LIMIT=13704 NO_ARM_ASM=1 + make test-size SIGN=RSAPSS2048 LIMIT=13744 NO_ARM_ASM=1 make clean - make test-size SIGN=RSAPSS2048 NO_ASM=1 LIMIT=14264 NO_ARM_ASM=1 + make test-size SIGN=RSAPSS2048 NO_ASM=1 LIMIT=14304 NO_ARM_ASM=1 make keysclean - make test-size SIGN=RSAPSS3072 LIMIT=13872 NO_ARM_ASM=1 + make test-size SIGN=RSAPSS3072 LIMIT=13912 NO_ARM_ASM=1 make clean - make test-size SIGN=RSAPSS3072 NO_ASM=1 LIMIT=14396 NO_ARM_ASM=1 + make test-size SIGN=RSAPSS3072 NO_ASM=1 LIMIT=14436 NO_ARM_ASM=1 make keysclean - make test-size SIGN=RSAPSS4096 LIMIT=14044 NO_ARM_ASM=1 + make test-size SIGN=RSAPSS4096 LIMIT=14084 NO_ARM_ASM=1 make clean - make test-size SIGN=RSAPSS4096 NO_ASM=1 LIMIT=14584 NO_ARM_ASM=1 + make test-size SIGN=RSAPSS4096 NO_ASM=1 LIMIT=14624 NO_ARM_ASM=1 make keysclean make test-size SIGN=LMS LMS_LEVELS=2 LMS_HEIGHT=5 LMS_WINTERNITZ=8 \ WOLFBOOT_SMALL_STACK=0 IMAGE_SIGNATURE_SIZE=2644 \ - IMAGE_HEADER_SIZE?=5288 LIMIT=8076 NO_ARM_ASM=1 + IMAGE_HEADER_SIZE?=5288 LIMIT=8116 NO_ARM_ASM=1 make keysclean make test-size SIGN=XMSS XMSS_PARAMS='XMSS-SHA2_10_256' \ IMAGE_SIGNATURE_SIZE=2500 IMAGE_HEADER_SIZE?=4096 \ - LIMIT=8728 NO_ARM_ASM=1 + LIMIT=8768 NO_ARM_ASM=1 make keysclean make clean - make test-size SIGN=ML_DSA ML_DSA_LEVEL=2 LIMIT=19538 \ + make test-size SIGN=ML_DSA ML_DSA_LEVEL=2 LIMIT=19578 \ IMAGE_SIGNATURE_SIZE=2420 IMAGE_HEADER_SIZE?=8192 From fca6bf0fa66e7e08ecc39c3bd45294a70a312ed7 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 11 Aug 2026 18:31:29 +0200 Subject: [PATCH 19/25] Address PR review on the encrypted write and sector copy paths ext_flash_encrypt_write() writes whole ENCRYPT_BLOCK_SIZE blocks: - A length that is not a multiple of the block size dropped the trailing bytes, since the remainder loop rounds down. Merge them into the block that already backs them, as the unaligned head is handled. - len == 0 fell through to a read-modify-write of the containing block, re-encrypting it in place. Return early instead. wolfBoot_copy_sector() checked the reads added in F-7987 for a negative return, but ext_flash_read() and ext_flash_check_read() return the number of bytes read (docs/HAL.md), so a short read was accepted and a partially filled buffer copied on. Require the full FLASHBUFFER_SIZE. Add unit-update-flash-enc coverage for the two write cases. --- src/libwolfboot.c | 21 +++++++++ src/update_flash.c | 6 ++- tools/unit-tests/unit-update-flash.c | 69 ++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 2 deletions(-) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index 7994e3cabd..41e3380d27 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -2587,6 +2587,13 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data, uint8_t ENCRYPT_CACHE[NVM_CACHE_SIZE] XALIGNED_STACK(32); #endif + /* A zero-length request must not turn into a read-modify-write of the + * containing block. */ + if (len < 0) + return -1; + if (len == 0) + return 0; + row_offset = address & (ENCRYPT_BLOCK_SIZE - 1); if (row_offset != 0) { row_address = address & ~(ENCRYPT_BLOCK_SIZE - 1); @@ -2657,6 +2664,20 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data, step -= chunk; } + /* Trailing bytes that do not fill a whole block. "address" is block + * aligned here, so merge them into the block that already backs them, + * the same way the unaligned head above is handled. */ + step = sz & (ENCRYPT_BLOCK_SIZE - 1); + if (step > 0) { + if (ext_flash_read(address, block, ENCRYPT_BLOCK_SIZE) + != ENCRYPT_BLOCK_SIZE) { + return -1; + } + XMEMCPY(block, data, step); + crypto_encrypt(enc_block, block, ENCRYPT_BLOCK_SIZE); + ret = ext_flash_write(address, enc_block, ENCRYPT_BLOCK_SIZE); + } + return ret; } diff --git a/src/update_flash.c b/src/update_flash.c index 0a7205603c..1baf63e6aa 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -323,14 +323,16 @@ static int RAMFUNCTION wolfBoot_copy_sector(struct wolfBoot_image *src, if (dst->part == PART_SWAP && SWAP_EXT) { if (ext_flash_read((uintptr_t)(src->hdr) + src_sector_offset + pos, - (void *)buffer, FLASHBUFFER_SIZE) < 0) { + (void *)buffer, FLASHBUFFER_SIZE) + != FLASHBUFFER_SIZE) { ret = -1; goto out; } } else { if (ext_flash_check_read((uintptr_t)(src->hdr) + src_sector_offset + pos, - (void *)buffer, FLASHBUFFER_SIZE) < 0) { + (void *)buffer, FLASHBUFFER_SIZE) + != FLASHBUFFER_SIZE) { ret = -1; goto out; } diff --git a/tools/unit-tests/unit-update-flash.c b/tools/unit-tests/unit-update-flash.c index cee449cc5a..4f0978e31c 100644 --- a/tools/unit-tests/unit-update-flash.c +++ b/tools/unit-tests/unit-update-flash.c @@ -601,6 +601,67 @@ static int add_payload_encrypted(uint8_t part, uint32_t version, uint32_t size, } #endif +#ifdef EXT_ENCRYPTED +/* ext_flash_encrypt_write() writes whole ENCRYPT_BLOCK_SIZE blocks. A request + * whose length is not a multiple of the block size used to drop the trailing + * bytes, and a zero-length request used to rewrite the containing block. */ +START_TEST (test_encrypt_write_keeps_trailing_partial_block) +{ + uintptr_t base = (uintptr_t)WOLFBOOT_PARTITION_UPDATE_ADDRESS; + int len = (2 * ENCRYPT_BLOCK_SIZE) + 5; + uint8_t out[(2 * ENCRYPT_BLOCK_SIZE) + 5]; + uint8_t in[(2 * ENCRYPT_BLOCK_SIZE) + 5]; + int i, ret; + + reset_mock_stats(); + prepare_flash(); + for (i = 0; i < len; i++) + in[i] = (uint8_t)(0x30 + i); + + ext_flash_unlock(); + ret = ext_flash_encrypt_write(base, in, len); + ext_flash_lock(); + ck_assert_int_ge(ret, 0); + + memset(out, 0, sizeof(out)); + ck_assert_int_eq(ext_flash_decrypt_read(base, out, len), len); + ck_assert_int_eq(memcmp(out, in, len), 0); + + cleanup_flash(); +} +END_TEST + +START_TEST (test_encrypt_write_zero_length_leaves_flash_untouched) +{ + uintptr_t base = (uintptr_t)WOLFBOOT_PARTITION_UPDATE_ADDRESS; + uint8_t before[ENCRYPT_BLOCK_SIZE]; + uint8_t after[ENCRYPT_BLOCK_SIZE]; + uint8_t in[ENCRYPT_BLOCK_SIZE]; + int i, ret; + + reset_mock_stats(); + prepare_flash(); + for (i = 0; i < ENCRYPT_BLOCK_SIZE; i++) + in[i] = (uint8_t)(0x70 + i); + + ext_flash_unlock(); + ck_assert_int_ge(ext_flash_encrypt_write(base, in, ENCRYPT_BLOCK_SIZE), 0); + ck_assert_int_eq(ext_flash_read(base, before, ENCRYPT_BLOCK_SIZE), + ENCRYPT_BLOCK_SIZE); + + ret = ext_flash_encrypt_write(base, in, 0); + ext_flash_lock(); + ck_assert_int_eq(ret, 0); + + ck_assert_int_eq(ext_flash_read(base, after, ENCRYPT_BLOCK_SIZE), + ENCRYPT_BLOCK_SIZE); + ck_assert_int_eq(memcmp(before, after, ENCRYPT_BLOCK_SIZE), 0); + + cleanup_flash(); +} +END_TEST +#endif /* EXT_ENCRYPTED */ + START_TEST (test_empty_panic) { reset_mock_stats(); @@ -1604,6 +1665,9 @@ Suite *wolfboot_suite(void) TCase *fallback_verify = tcase_create("Fallback verify"); #endif #endif +#ifdef EXT_ENCRYPTED + TCase *encrypt_write_bounds = tcase_create("Encrypted write bounds"); +#endif #ifdef UNIT_TEST_FALLBACK_ONLY @@ -1612,6 +1676,11 @@ Suite *wolfboot_suite(void) tcase_add_test(fallback_verify, test_final_swap_propagates_encrypt_key_read_failure); tcase_add_test(fallback_verify, test_final_swap_propagates_encrypt_key_persist_failure); suite_add_tcase(s, fallback_verify); + tcase_add_test(encrypt_write_bounds, + test_encrypt_write_keeps_trailing_partial_block); + tcase_add_test(encrypt_write_bounds, + test_encrypt_write_zero_length_leaves_flash_untouched); + suite_add_tcase(s, encrypt_write_bounds); #endif return s; #else From cfaf145649bdeb2759d3e5767c77aa3f0e6a524f Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 12 Aug 2026 13:20:56 +0200 Subject: [PATCH 20/25] libwolfboot: propagate the head-block write failure ext_flash_encrypt_write() captured the unaligned head block's write result but only returned it when the request fit inside that block; otherwise the remainder loop overwrote it with its own status. On an external encrypted partition this function is wb_flash_write(), so a failed head program was reported as success and defeated the swap abort added in F-7987. Also round the staging size down to a whole number of encryption blocks. NVM_CACHE_SIZE defaults to WOLFBOOT_SECTOR_SIZE and is always a multiple today, but an override would write stale cache bytes and desynchronise the keystream. Assert the invariant at compile time. --- src/libwolfboot.c | 18 +++++++++++++++-- tools/unit-tests/unit-mock-flash.c | 7 +++++++ tools/unit-tests/unit-update-flash.c | 30 ++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/libwolfboot.c b/src/libwolfboot.c index 41e3380d27..87e9bbfaa0 100644 --- a/src/libwolfboot.c +++ b/src/libwolfboot.c @@ -2560,6 +2560,18 @@ static uint8_t RAMFUNCTION part_address(uintptr_t a) } #ifdef EXT_FLASH + +/* ENCRYPT_CACHE is staged one whole encryption block at a time, so the amount + * written per pass must be a block multiple: a partial block at the end of a + * pass would be written as stale cache content and would leave the address + * unaligned, desynchronising the keystream from ext_flash_decrypt_read(). + * NVM_CACHE_SIZE defaults to WOLFBOOT_SECTOR_SIZE, which is always a multiple, + * but it can be overridden. */ +#define ENCRYPT_STAGE_SIZE \ + ((NVM_CACHE_SIZE) - ((NVM_CACHE_SIZE) % ENCRYPT_BLOCK_SIZE)) +typedef char wolfBoot_encrypt_stage_size_check[ + (ENCRYPT_STAGE_SIZE >= ENCRYPT_BLOCK_SIZE) ? 1 : -1]; + /** * @brief Write encrypted data to an external flash. * @@ -2636,6 +2648,8 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data, XMEMCPY(block + row_offset, data, step); crypto_encrypt(enc_block, block, ENCRYPT_BLOCK_SIZE); ret = ext_flash_write(row_address, enc_block, ENCRYPT_BLOCK_SIZE); + if (ret < 0) + return ret; /* The request fits entirely within this block: nothing left to do */ if (step == len) return ret; @@ -2649,8 +2663,8 @@ int RAMFUNCTION ext_flash_encrypt_write(uintptr_t address, const uint8_t *data, step = sz & ~(ENCRYPT_BLOCK_SIZE - 1); while (step > 0) { int chunk = step; - if (chunk > NVM_CACHE_SIZE) - chunk = NVM_CACHE_SIZE; + if (chunk > (int)ENCRYPT_STAGE_SIZE) + chunk = (int)ENCRYPT_STAGE_SIZE; for (i = 0; i < chunk / ENCRYPT_BLOCK_SIZE; i++) { XMEMCPY(block, data + (ENCRYPT_BLOCK_SIZE * i), ENCRYPT_BLOCK_SIZE); crypto_encrypt(ENCRYPT_CACHE + (ENCRYPT_BLOCK_SIZE * i), block, diff --git a/tools/unit-tests/unit-mock-flash.c b/tools/unit-tests/unit-mock-flash.c index e43cd4cf1b..3dd9e44b56 100644 --- a/tools/unit-tests/unit-mock-flash.c +++ b/tools/unit-tests/unit-mock-flash.c @@ -25,6 +25,9 @@ static int locked = 1; static int ext_locked = 1; +/* When set, the next ext_flash_write() fails and the hook clears itself, + * so a test can target one specific write in a multi-write sequence. */ +static int ext_flash_write_fail = 0; static int erased_boot = 0; static int erased_update = 0; static int erased_swap = 0; @@ -184,6 +187,10 @@ int ext_flash_write(uintptr_t address, const uint8_t *data, int len) uint8_t *a = (uint8_t *)address; ck_assert_msg(!ext_locked, "Attempting to write to a locked FLASH"); ck_assert_msg(len >= 0, "ext_flash_write invalid len %d", len); + if (ext_flash_write_fail) { + ext_flash_write_fail = 0; + return -1; + } ck_assert_msg( ((address >= WOLFBOOT_PARTITION_BOOT_ADDRESS) && (address < WOLFBOOT_PARTITION_BOOT_ADDRESS + WOLFBOOT_PARTITION_SIZE) && diff --git a/tools/unit-tests/unit-update-flash.c b/tools/unit-tests/unit-update-flash.c index 4f0978e31c..19e58805df 100644 --- a/tools/unit-tests/unit-update-flash.c +++ b/tools/unit-tests/unit-update-flash.c @@ -631,6 +631,34 @@ START_TEST (test_encrypt_write_keeps_trailing_partial_block) } END_TEST +/* wb_flash_write() on an external encrypted partition is this function, and + * F-7987 makes wolfBoot_copy_sector() abort the swap on a negative return. A + * failure programming the unaligned head block must therefore propagate, + * rather than be overwritten by the remainder loop's own status. */ +START_TEST (test_encrypt_write_reports_head_block_write_failure) +{ + uintptr_t base = (uintptr_t)WOLFBOOT_PARTITION_UPDATE_ADDRESS; + uint8_t in[3 * ENCRYPT_BLOCK_SIZE]; + int i, ret; + + reset_mock_stats(); + prepare_flash(); + for (i = 0; i < (int)sizeof(in); i++) + in[i] = (uint8_t)(0x10 + i); + + ext_flash_unlock(); + /* Start mid-block so the head path runs, and extend past it so the + * remainder loop runs too. */ + ext_flash_write_fail = 1; + ret = ext_flash_encrypt_write(base + 4, in, (2 * ENCRYPT_BLOCK_SIZE)); + ext_flash_lock(); + + ck_assert_int_lt(ret, 0); + + cleanup_flash(); +} +END_TEST + START_TEST (test_encrypt_write_zero_length_leaves_flash_untouched) { uintptr_t base = (uintptr_t)WOLFBOOT_PARTITION_UPDATE_ADDRESS; @@ -1680,6 +1708,8 @@ Suite *wolfboot_suite(void) test_encrypt_write_keeps_trailing_partial_block); tcase_add_test(encrypt_write_bounds, test_encrypt_write_zero_length_leaves_flash_untouched); + tcase_add_test(encrypt_write_bounds, + test_encrypt_write_reports_head_block_write_failure); suite_add_tcase(s, encrypt_write_bounds); #endif return s; From a69b3384d76152571f90aa2f2986bb40dcc10aca Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 12 Aug 2026 13:20:56 +0200 Subject: [PATCH 21/25] nrf52/nrf5340/stm32l0: fix the 32-bit fast path in hal_flash_write F-6757 fixed the byte-wise path but left the fast path above it indexing dst[i >> 2]/src[i >> 2] off the call-time base. The guard only proves that "address + i" and "data + i" are word aligned, so when the destination and source share a non-zero misalignment the byte path advances i to the next word boundary and the fast path then copies the wrong word, through an unaligned 32-bit access that faults on the Cortex-M0+ of stm32l0. Index both pointers by i directly, and cover the case the existing tests deliberately avoided. --- hal/nrf52.c | 10 +++++--- hal/nrf5340.c | 10 +++++--- hal/stm32l0.c | 10 +++++--- tools/unit-tests/unit-flash-write-nrf52.c | 29 +++++++++++++++++++++++ 4 files changed, 50 insertions(+), 9 deletions(-) diff --git a/hal/nrf52.c b/hal/nrf52.c index 1412ac7a36..9fad82209d 100644 --- a/hal/nrf52.c +++ b/hal/nrf52.c @@ -73,11 +73,15 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) while (i < len) { if ((len - i > 3) && ((((address + i) & 0x03) == 0) && ((((uint32_t)data) + i) & 0x03) == 0)) { - src = (uint32_t *)data; - dst = (uint32_t *)address; + /* Index by "i" directly: the condition above only guarantees + * that "address + i" and "data + i" are word aligned, so + * dst[i >> 2] off the unaligned base would address the wrong + * word (and fault on a strict-alignment core). */ + src = (uint32_t *)(data + i); + dst = (uint32_t *)(address + i); NVMC_CONFIG = NVMC_CONFIG_WEN; flash_wait_complete(); - dst[i >> 2] = src[i >> 2]; + *dst = *src; flash_wait_complete(); i+=4; } else { diff --git a/hal/nrf5340.c b/hal/nrf5340.c index 79f2635a6d..f1762086b1 100644 --- a/hal/nrf5340.c +++ b/hal/nrf5340.c @@ -317,14 +317,18 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) while (i < len) { if ((len - i > 3) && ((((address + i) & 0x03) == 0) && ((((uint32_t)data) + i) & 0x03) == 0)) { - src = (uint32_t *)data; - dst = (uint32_t *)address; + /* Index by "i" directly: the condition above only guarantees + * that "address + i" and "data + i" are word aligned, so + * dst[i >> 2] off the unaligned base would address the wrong + * word (and fault on a strict-alignment core). */ + src = (uint32_t *)(data + i); + dst = (uint32_t *)(address + i); #if TZ_SECURE() || defined(TARGET_nrf5340_net) NVMC_CONFIG = NVMC_CONFIG_WEN; #endif NVMC_CONFIGNS = NVMC_CONFIG_WEN; while (NVMC_READY == 0); - dst[i >> 2] = src[i >> 2]; + *dst = *src; while (NVMC_READY == 0); i+=4; } else { diff --git a/hal/stm32l0.c b/hal/stm32l0.c index 59ffea708c..5e508343b3 100644 --- a/hal/stm32l0.c +++ b/hal/stm32l0.c @@ -111,10 +111,14 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) while (i < len) { if ((len - i > 3) && ((((address + i) & 0x03) == 0) && ((((uint32_t)data) + i) & 0x03) == 0)) { - src = (uint32_t *)data; - dst = (uint32_t *)(address + FLASHMEM_ADDRESS_SPACE); + /* Index by "i" directly: the condition above only guarantees + * that "address + i" and "data + i" are word aligned, so + * dst[i >> 2] off the unaligned base would address the wrong + * word, and the Cortex-M0+ faults on the unaligned access. */ + src = (uint32_t *)(data + i); + dst = (uint32_t *)(address + i + FLASHMEM_ADDRESS_SPACE); flash_wait_complete(); - dst[i >> 2] = src[i >> 2]; + *dst = *src; flash_wait_complete(); i+=4; } else { diff --git a/tools/unit-tests/unit-flash-write-nrf52.c b/tools/unit-tests/unit-flash-write-nrf52.c index 18ac09f2fa..3872d3d00c 100644 --- a/tools/unit-tests/unit-flash-write-nrf52.c +++ b/tools/unit-tests/unit-flash-write-nrf52.c @@ -140,6 +140,34 @@ START_TEST(test_unaligned_write_mismatched_alignment) } END_TEST +/* Destination and source share the same non-zero misalignment, so once the + * byte-wise path has advanced i to the next word boundary both fast-path + * conditions hold and the 32-bit branch is entered with i != 0. Before the + * fix that branch indexed dst[i >> 2]/src[i >> 2] off the unaligned bases, + * writing data[0..3] to "address..address+3" instead of data[3..6] to + * "address+3..address+6" -- through a misaligned 32-bit flash access. */ +START_TEST(test_unaligned_write_matching_alignment_fast_path) +{ + uint8_t rawbuf[64]; + uint8_t *data = rawbuf; + uint32_t base = (uint32_t)(uintptr_t)mock_flash; + int i; + + while (((uintptr_t)data % 4) != 1) + data++; + for (i = 0; i < 12; i++) + data[i] = (uint8_t)(0xD0 + i); + + ck_assert_int_eq(hal_flash_write(base + 1, data, 12), 0); + + ck_assert_uint_eq(mock_flash[0], 0xFF); + for (i = 0; i < 12; i++) + ck_assert_uint_eq(mock_flash[1 + i], data[i]); + for (i = 13; i < MOCK_FLASH_SIZE; i++) + ck_assert_uint_eq(mock_flash[i], 0xFF); +} +END_TEST + /* A write that fits entirely inside a single flash word must still work: * buggy and fixed forms agree here (i is always 0 in the byte-wise path), * guarding against a fix that breaks the common case. */ @@ -170,6 +198,7 @@ Suite *flash_write_suite(void) tcase_add_checked_fixture(tc, setup, teardown); tcase_add_test(tc, test_aligned_write_unaligned_tail); tcase_add_test(tc, test_unaligned_write_mismatched_alignment); + tcase_add_test(tc, test_unaligned_write_matching_alignment_fast_path); tcase_add_test(tc, test_unaligned_write_single_word); suite_add_tcase(s, tc); From a2706b98d5c57312ac13373937294a7658cea099 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 12 Aug 2026 13:21:08 +0200 Subject: [PATCH 22/25] mcxw: extend the erase length when rounding the start down hal_flash_erase() rounded an unaligned address down to the sector boundary but left len at the caller's value, so a request ending in a later sector erased only the first one. Grow len by the same amount. test_erase_zero_runtime_sector_falls_back covered a request that ends 0x10 into the second sector, so its one-erase expectation encoded the under-erase; it now expects both. --- hal/mcxw.c | 9 +++++++-- tools/unit-tests/unit-flash-erase-mcxw.c | 22 ++++++++++++++++++++-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/hal/mcxw.c b/hal/mcxw.c index 5c32e5b5c7..c8bc6720de 100644 --- a/hal/mcxw.c +++ b/hal/mcxw.c @@ -236,8 +236,13 @@ int RAMFUNCTION hal_flash_erase(uint32_t address, int len) if (sector_size == 0U) sector_size = WOLFBOOT_SECTOR_SIZE; - if (address % sector_size) - address -= address % sector_size; + /* Rounding the start down extends the range, so the length must grow by + * the same amount or the last sector of the request is left unerased. */ + if (address % sector_size) { + uint32_t offset = address % sector_size; + address -= offset; + len += (int)offset; + } while (len > 0) { erase_flash_sector((uint32_t *)address); address += sector_size; diff --git a/tools/unit-tests/unit-flash-erase-mcxw.c b/tools/unit-tests/unit-flash-erase-mcxw.c index dc8222ce13..125306d71e 100644 --- a/tools/unit-tests/unit-flash-erase-mcxw.c +++ b/tools/unit-tests/unit-flash-erase-mcxw.c @@ -122,16 +122,33 @@ START_TEST(test_erase_unaligned_start_rounds_down) } END_TEST +/* Rounding the start down extends the range backwards, so the length has to + * grow by the same amount. A request that starts near the end of one sector + * and reaches into the next must erase both, not just the first. */ +START_TEST(test_erase_unaligned_start_spanning_next_sector) +{ + reset_mocks(0x1000); + ck_assert_int_eq(hal_flash_erase(FLASH_BASE + 0xF00, 0x200), 0); + + ck_assert_int_eq(erase_log_n, 2); + ck_assert_uint_eq(erase_addr[0], FLASH_BASE); + ck_assert_uint_eq(erase_addr[1], FLASH_BASE + 0x1000U); +} +END_TEST + /* FLASH_GetProperty() failing to report a size must not divide by zero: - * fall back to WOLFBOOT_SECTOR_SIZE, as hal/mcxn.c does. */ + * fall back to WOLFBOOT_SECTOR_SIZE, as hal/mcxn.c does. The requested range + * starts 0x10 into the first sector and so ends 0x10 into the second, which + * takes two erase commands. */ START_TEST(test_erase_zero_runtime_sector_falls_back) { reset_mocks(0); ck_assert_int_eq(hal_flash_erase(FLASH_BASE + 0x10, WOLFBOOT_SECTOR_SIZE), 0); - ck_assert_int_eq(erase_log_n, 1); + ck_assert_int_eq(erase_log_n, 2); ck_assert_uint_eq(erase_addr[0], FLASH_BASE); + ck_assert_uint_eq(erase_addr[1], FLASH_BASE + WOLFBOOT_SECTOR_SIZE); } END_TEST @@ -144,6 +161,7 @@ Suite *flash_erase_suite(void) tcase_add_test(tc, test_erase_runtime_sector_smaller_covers_range); tcase_add_test(tc, test_erase_runtime_sector_larger_stays_aligned); tcase_add_test(tc, test_erase_unaligned_start_rounds_down); + tcase_add_test(tc, test_erase_unaligned_start_spanning_next_sector); tcase_add_test(tc, test_erase_zero_runtime_sector_falls_back); suite_add_tcase(s, tc); From acf07799a553a6905b5e940138d96ffec0328f67 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 12 Aug 2026 13:21:08 +0200 Subject: [PATCH 23/25] update_ram: track the uImage entry override explicitly The ih_ep override was skipped when load_address no longer equalled the recorded ih_load, as a proxy for "a later stage supplied its own entry point". elf_load_image_mmu() publishes *pentry before validating the program headers, so a rejected ELF also rewrites load_address and silently suppressed the override; conversely a stage landing on ih_load would let it through. Use a flag set in the ELF and FIT success paths instead, and publish *pentry only after validation. Also note in the ih_load == 0 branch that ih_ep is ignored there, which the TODO removed by F-7985 used to record. --- src/elf.c | 8 +++++--- src/update_ram.c | 30 +++++++++++++++++++++++------- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/elf.c b/src/elf.c index 8838dbff2f..a4a633d3e7 100644 --- a/src/elf.c +++ b/src/elf.c @@ -107,9 +107,6 @@ int elf_load_image_mmu(uint8_t *image, uint32_t image_sz, uintptr_t *pentry, is_elf32 ? 32 : 64, is_le ? "little" : "big"); #endif - /* set entry point */ - *pentry = GET_H64(entry); - /* programs */ ph_offset = GET_H32(ph_offset); entry_size = GET_H16(ph_entry_size); @@ -222,6 +219,11 @@ int elf_load_image_mmu(uint8_t *image, uint32_t image_sz, uintptr_t *pentry, #endif /* !ELF_PARSER */ } + /* Publish the entry point only once every check above has passed: callers + * fall back to the raw binary on failure and must not be left with a + * partially validated ELF's declared entry. */ + *pentry = GET_H64(entry); + #ifdef DEBUG_ELF wolfBoot_printf("Entry point %p\r\n", (void*)*pentry); #endif diff --git a/src/update_ram.c b/src/update_ram.c index b4d1b662af..e83dad81f8 100644 --- a/src/update_ram.c +++ b/src/update_ram.c @@ -273,10 +273,12 @@ void RAMFUNCTION wolfBoot_start(void) BENCHMARK_DECLARE(); #ifdef WOLFBOOT_UBOOT_LEGACY uint8_t *image_ptr; - /* uImage ih_load/ih_ep, kept only when the entry point differs from the - * load address (see the do_boot() entry override below). */ - uint32_t *uboot_load = NULL; + /* uImage ih_ep, kept only when the entry point differs from the load + * address (see the do_boot() entry override below). */ uint32_t *uboot_entry = NULL; + /* Set when a later stage (ELF/FIT) re-derives the load address and so + * supplies its own entry point, which then wins over ih_ep. */ + int stage_entry_override = 0; #endif uint32_t *load_address = NULL; uint32_t *source_address = NULL; @@ -515,13 +517,16 @@ void RAMFUNCTION wolfBoot_start(void) * different addresses. Remember the entry point; ih_load remains * the relocation destination. */ if ((ih_ep != 0) && (ih_ep != ih_load)) { - uboot_load = load_address; uboot_entry = (uint32_t*)(uintptr_t)ih_ep; } } else { /* Linux PPC path: leave load_address alone, just advance it * past the header to match upstream behaviour. load_address is - * a uint32_t*, so advance by BYTES, not words. */ + * a uint32_t*, so advance by BYTES, not words. + * ih_ep is deliberately ignored here: with ih_load == 0 there is + * no relocation destination to enter past, and upstream enters at + * the payload start. A uImage built with "mkimage -a 0 -e " + * is therefore entered at the header offset, not at ih_ep. */ load_address = (uint32_t*)((uint8_t*)load_address + UBOOT_IMG_HDR_SZ); } @@ -560,6 +565,11 @@ void RAMFUNCTION wolfBoot_start(void) (uintptr_t*)&load_address, NULL) != 0){ wolfBoot_printf("Invalid elf, falling back to raw binary\n"); } +#ifdef WOLFBOOT_UBOOT_LEGACY + else { + stage_entry_override = 1; + } +#endif #endif #ifdef MMU @@ -595,6 +605,9 @@ void RAMFUNCTION wolfBoot_start(void) wolfBoot_panic(); } load_address = new_load; +#ifdef WOLFBOOT_UBOOT_LEGACY + stage_entry_override = 1; +#endif } #if defined(WOLFBOOT_ZYNQMP_FSBL) && defined(MMU) /* Load BL31 (ARM Trusted Firmware) to its DDR exec address. Its entry @@ -663,8 +676,11 @@ void RAMFUNCTION wolfBoot_start(void) #ifdef WOLFBOOT_UBOOT_LEGACY /* Enter the uImage at ih_ep. Skipped if a later stage (ELF/FIT) re-derived - * the load address, since that stage provides its own entry point. */ - if ((uboot_entry != NULL) && (load_address == uboot_load)) { + * the load address, since that stage provides its own entry point. The + * flag is tracked explicitly rather than by comparing load_address: + * elf_load_image_mmu() publishes its entry point before it finishes + * validating, so a rejected ELF also leaves load_address rewritten. */ + if ((uboot_entry != NULL) && !stage_entry_override) { load_address = uboot_entry; } #endif From 6201ba4a38a1f6e8d3f53bba2f66d17a5b3171f7 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 12 Aug 2026 13:21:08 +0200 Subject: [PATCH 24/25] sign: scrub the primary key when the key load fails main() exit(1)'d on a key load failure. For the hybrid secondary key that happens with the primary raw buffer live and the primary key object initialized, so neither zero_and_free(kbuf) nor free_key() ran -- the case F-8006 set out to fix. Jump to the tail cleanup instead; the exit status is unchanged. Also document why free_key() tolerates an uninitialized or already-freed object, since load_key() has paths that produce both. --- tools/keytools/sign.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tools/keytools/sign.c b/tools/keytools/sign.c index 8872b7331c..7f8022aaf2 100644 --- a/tools/keytools/sign.c +++ b/tools/keytools/sign.c @@ -328,6 +328,11 @@ static struct signing_key *key_obj(int secondary) } /* Run the algorithm specific (zeroizing) free on a decoded signing key. */ +/* Safe to call on an object that was never initialized, or twice: "key" and + * "key2" are zero-initialized file-scope statics and every wolfCrypt free + * below is NULL-checked and idempotent. load_key() has paths that never + * initialize the object (--manual-sign, --sha-only, raw-public-key inputs) and + * paths that already free it, so both cases do occur. */ static void free_key(int sign, int secondary) { struct signing_key *k = key_obj(secondary); @@ -3813,7 +3818,8 @@ int main(int argc, char** argv) } else { kbuf = load_key(&key_buffer, &key_buffer_sz, &pubkey, &pubkey_sz, 0); if (!kbuf) { - exit(1); + ret = 1; + goto cleanup; } } /* CMD.sign != NO_SIGN */ @@ -3824,7 +3830,11 @@ int main(int argc, char** argv) DEBUG_PRINT("Loading secondary key\n"); kbuf2 = load_key(&key_buffer2, &key_buffer_sz2, &pubkey2, &pubkey_sz2, 1); if (!kbuf2) { - exit(1); + /* Fall through to the tail cleanup: the primary raw key buffer is + * still live and the primary key object is initialized, and + * exiting here would scrub neither. */ + ret = 1; + goto cleanup; } printf("Creating hybrid signature\n"); ret = make_hybrid_header(pubkey, pubkey_sz, CMD.image_file, @@ -3850,6 +3860,7 @@ int main(int argc, char** argv) ret = base_diff(CMD.delta_base_file, pubkey, pubkey_sz, 16); } +cleanup: /* Add pubkey cleanup */ if (pubkey) free(pubkey); From e4fd913acc8d4efba4d5f0b2c40a73c77b9cad36 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Wed, 12 Aug 2026 13:21:08 +0200 Subject: [PATCH 25/25] gitignore: add two missing unit-test binaries unit-flash-write-nrf52 and unit-tpm-mfgid-eh-zeroize were added to the unit-tests Makefile without the matching ignore entries. --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index 598af99037..b6817d2b06 100644 --- a/.gitignore +++ b/.gitignore @@ -205,6 +205,7 @@ tools/unit-tests/unit-otp-keystore-gen-zeroize tools/unit-tests/unit-tpm-api-names tools/unit-tests/unit-tpm-nsc-cert tools/unit-tests/unit-tpm-advio-zeroize +tools/unit-tests/unit-tpm-mfgid-eh-zeroize tools/unit-tests/unit-elf-bss-guard tools/unit-tests/unit-fit-fpga tools/unit-tests/unit-flash-erase-c0 @@ -219,6 +220,7 @@ tools/unit-tests/unit-ahci-unlock-panic tools/unit-tests/unit-ata-security-passphrase-zeroize tools/unit-tests/unit-arm-tee-psa-ipc tools/unit-tests/unit-flash-write-mcxa +tools/unit-tests/unit-flash-write-nrf52 tools/unit-tests/unit-flash-write-same51 tools/unit-tests/unit-flash-write-samr21 tools/unit-tests/unit-image-elf-scatter