Fixes 20260408

[danielinux]

F/2273 - sign: scrub key buffers before free (1c824539)
F/2274 - Scrub RSA keygen private material (87d5a7fc)
F/2275 - Zero ECC keygen private buffers (114140c2)
F/2276 - Zero EdDSA keygen private data (6f40afae)
F/2277 - zero ML-DSA private key buffer (32783034)
F/2258 - Add restricted key mask authenticity tests (8399e3ed)
F/1892 - Propagate encrypt key flash errors (afef21fe)
F/1893 - Propagate erase encrypt key write failures (13370613)
F/1894 - Fix policy_create PCR digest validation (6b8ade6b)
F/1895 - Fix sign encrypted output open failure (34438999)
F/1898 - Check image reopen failures in sign tool (5fd09eed)
F/2247 - Use constant-time RSA hash comparison (9a7930dd)
F/2255 - Protect bootloader before application boot (f32c275d)
F/2259 - Add auth type coverage for unit-image (644e70e7)
F/2260 - Add auth-only invalid update test (ff60286e)
F/2261 - Add RAM_CODE self-update unit coverage (3659d210)
F/2262 - Strengthen same-version RAM update test (1353e933)
F/2266 - Fix sign header TLV overflow sizing (ea6700f1)
F/2267 - Reject oversized delta source offsets (fefd74e0)
F/1897 - fix memmove large-length backward copy (7526fd23)
F/2248 - Use constant-time TPM secret checks (78de4a79)
F/2249 - Use constant-time encryption key validation (56c46be7)
F/2252 - Use fixed-length erased-key check (830869c3)
F/2256 - enforce skip-verify prerequisites (a09babb5)
F/2264 - Add equal-version update-disk regression test (395202bb)
F/2268 - Reject valid zero-size delta images (29367f81)
F/2269 - Fix total size type in update flash (fffd8543)
F/1888 - zeroize update key material (05823677)
F/1889 - zero custom encrypt stack buffers (7c1c8631)
F/1890 - zeroize swap trailer key buffer (a870af20)
F/1891 - Scrub sign-tool encryption material (3f1906fa)
F/2253 - Use constant-time delta base hash compare (10035dbc)
F/2257 - Warn when DISABLE_BACKUP is enabled (13d232c4)
F/2270 - Add sign/parser roundtrip tests (fb9faffc)
F/2271 - Add delta roundtrip edge-case coverage (08d39cf4)

[Copilot]

Pull request overview

This PR is a security- and robustness-focused sweep across wolfBoot tooling and update paths, adding constant-time comparisons, zeroization of sensitive material, stricter bounds/validation, and expanding unit-test coverage for regressions and edge cases.

Changes:

• Add zeroization/scrubbing for signing/keygen/encryption-key material and convert several comparisons to constant-time.
• Harden update/self-update/delta flows (bounds checks, error propagation, bootloader flash protection hook).
• Add/extend unit tests for header sizing, encrypted-sign output error handling, PCR digest validation, delta/update edge cases, and keymask/auth-type behavior.

Reviewed changes

Copilot reviewed 39 out of 39 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tools/unit-tests/unit-update-ram.c	Adds an assertion validating the RAM-loaded image size metadata.
tools/unit-tests/unit-update-flash.c	Adds self-update tests, auth-type/size edge-case tests, and helpers for image payload construction.
tools/unit-tests/unit-update-disk.c	Adds monolithic/self-header config and a regression test for equal-version partition selection.
tools/unit-tests/unit-string.c	Adds a Linux-only regression test targeting large-length overlapping memmove.
tools/unit-tests/unit-sign-header-size.c	New unit test validating sign-tool manifest header-size enforcement logic.
tools/unit-tests/unit-sign-encrypted-output.mkfrag	Adds a source list fragment to build the encrypted-output sign-tool unit test.
tools/unit-tests/unit-sign-encrypted-output.c	New unit test exercising sign tool encrypted output error-handling and TLV/header roundtrips.
tools/unit-tests/unit-policy-create.c	New unit test covering PCR digest argument validation in policy_create tool.
tools/unit-tests/unit-keystore.c	Adjusts keystore slot mask to a restricted “app-only” verify mask for tests.
tools/unit-tests/unit-image.c	Adds authenticity tests for mismatched auth type and key mask restrictions.
tools/unit-tests/unit-enc-nvm.c	Adds tests ensuring flash write failures propagate for encrypt-key set/erase APIs.
tools/unit-tests/unit-delta.c	Adds delta offset-limit test and significantly expands roundtrip/diff/patch edge coverage.
tools/unit-tests/Makefile	Adds new unit-test targets and specialized build variants (delta/self-update/sign tests).
tools/tpm/policy_create.c	Fixes PCR digest length validation by correctly treating the size as signed for the check.
tools/keytools/sign.c	Enforces header-size match, adds header overflow checks, improves error handling, and scrubs key buffers.
tools/keytools/Makefile	Links header_size.o into the sign tool build.
tools/keytools/keygen.c	Scrubs keygen private buffers (RSA/ECC/EdDSA/ML-DSA) and improves cleanup paths.
tools/keytools/header_size.h	New helper API for enforcing manifest header-size alignment with compiled bootloader config.
tools/keytools/header_size.c	Implements manifest header-size enforcement helper.
tools/delta/bmdiff.c	Adds maximum input size validation to prevent oversized file processing.
src/x86/ahci.c	Uses constant-time comparison for TPM-sealed secret verification.
src/update_ram.c	Calls hal_flash_protect() before handoff (outside TrustZone) to protect bootloader region.
src/update_flash.c	Zeroizes encryption buffers, tightens delta edge-case handling, adds constant-time hash/secret checks, and adjusts total-size type.
src/update_flash_hwswap.c	Calls hal_flash_protect() before hal_prepare_boot() to protect bootloader region.
src/update_disk.c	Calls hal_flash_protect() before hal_prepare_boot() to protect bootloader region.
src/tpm.c	Exposes constant-time compare for TPM use-cases.
src/string.c	Fixes memmove backward-copy loop to correctly handle large size_t lengths.
src/libwolfboot.c	Improves encrypt-key validity/erased checks, propagates flash errors, and scrubs key/nonce buffers in init paths.
src/image.c	Exposes image_CT_compare() (noinline) for constant-time digest comparisons.
src/delta.c	Rejects delta match offsets beyond 24-bit limit.
options.mk	Enforces WOLFBOOT_SKIP_BOOT_VERIFY prerequisites and warns when DISABLE_BACKUP=1.
include/wolfboot/wolfboot.h	Adds compile-time enforcement of WOLFBOOT_SKIP_BOOT_VERIFY prerequisites.
include/tpm.h	Declares wolfBoot_constant_compare() when TPM seal/keystore is enabled.
include/image.h	Switches RSA/digest verification macros to use image_CT_compare() and adds its prototype.
include/hal.h	Adds hal_flash_protect() API to HAL interface.
hal/skeleton.c	Documents that hal_flash_protect() is invoked before hal_prepare_boot().
hal/nrf5340.c	Removes duplicated bootloader flash-protect call now handled centrally.
hal/hal.c	Provides a weak default hal_flash_protect() implementation (no-op).
docs/compile.md	Documents unrecoverable power-loss implications when DISABLE_BACKUP=1.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 35 out of 35 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

tools/unit-tests/unit-string.c:1

• This test attempts a ~2GB overlapping memmove (INT_MAX+2), which will be extremely slow (byte-wise implementations will take billions of iterations) and may also fail due to virtual-memory limits/ulimits in CI, making the suite flaky or timing out. Consider redesigning the regression to avoid O(INT_MAX) work (e.g., gate it behind an opt-in flag, add a much smaller boundary-focused test that still exercises the original failure mode, or use a dedicated long-test target with an explicit timeout).
  src/libwolfboot.c:1
• wolfBoot_erase_encrypt_key() now returns from inside the non-MMU branch (line 1700) but still has an unconditional return 0; after the #endif, which becomes unreachable in that configuration and can trigger warnings/confusion. Consider restructuring to a single exit/return value (e.g., keep a ret initialized to 0 and return it once at the end) so the control flow is clearer across build configurations.

/* libwolfboot.c

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This test file uses PATH_MAX (and also uses errno/EACCES in mock_fopen) but does not include the headers that declare them. Add the appropriate includes (typically <limits.h> for PATH_MAX and <errno.h> for errno/EACCES) to avoid non-portable builds and compile failures.

[Copilot]

The prerequisite check mixes naming conventions: the C-side requirement added in include/wolfboot/wolfboot.h is WOLFBOOT_SELF_UPDATE_MONOLITHIC, but the Makefile gate checks SELF_UPDATE_MONOLITHIC. Align these to a single option name (or explicitly map one to the other) so build-time errors remain consistent and users don't have to guess which variable to set.

Suggested change

	ifeq ($(WOLFBOOT_SKIP_BOOT_VERIFY),1)
	ifneq ($(WOLFBOOT_SELF_HEADER),1)
	$(error WOLFBOOT_SKIP_BOOT_VERIFY=1 requires WOLFBOOT_SELF_HEADER=1)
	endif
	ifneq ($(SELF_UPDATE_MONOLITHIC),1)
	$(error WOLFBOOT_SKIP_BOOT_VERIFY=1 requires SELF_UPDATE_MONOLITHIC=1)
	ifeq ($(strip $(WOLFBOOT_SELF_UPDATE_MONOLITHIC)),)
	ifneq ($(strip $(SELF_UPDATE_MONOLITHIC)),)
	WOLFBOOT_SELF_UPDATE_MONOLITHIC:=$(SELF_UPDATE_MONOLITHIC)
	endif
	endif
	ifeq ($(WOLFBOOT_SKIP_BOOT_VERIFY),1)
	ifneq ($(WOLFBOOT_SELF_HEADER),1)
	$(error WOLFBOOT_SKIP_BOOT_VERIFY=1 requires WOLFBOOT_SELF_HEADER=1)
	endif
	ifneq ($(WOLFBOOT_SELF_UPDATE_MONOLITHIC),1)
	$(error WOLFBOOT_SKIP_BOOT_VERIFY=1 requires WOLFBOOT_SELF_UPDATE_MONOLITHIC=1)

[Copilot]

Adding hal_flash_protect() to the public HAL API with an int return type is an API contract change. To avoid integration breakage across existing target HALs, ensure all non-weak platform implementations (if any) match this exact signature/return type, and document expected return semantics (e.g., 0 on success, negative on failure) so callers can handle errors consistently.

Suggested change

	void hal_flash_lock(void);
	void hal_flash_lock(void);
	/*
	* Protect the flash range starting at `address` for `len` bytes.
	*
	* This is part of the public HAL API: all platform implementations must
	* use this exact signature and return type to preserve compatibility.
	*
	* Return value semantics:
	* 0 on success
	* <0 on failure
	*/

[Copilot]

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

hal_flash_protect's definition uses (uint32_t, uint32_t), but include/hal.h now declares int hal_flash_protect(haladdr_t address, int len). Since this file includes image.h (which includes hal.h), this will be a conflicting prototype on many toolchains. Update the signature here to match the header (use haladdr_t for the address and int for the length, or include the exact same typedefs).

Suggested change

	int RAMFUNCTION hal_flash_protect(uint32_t start, uint32_t len)
	int RAMFUNCTION hal_flash_protect(haladdr_t start, int len)

[Copilot]

wolfBoot_set_encrypt_key() now returns the underlying flash error code, but this call ignores the return value. If persisting the key fails after swap, the device can end up in a state where updates/encrypted boot can’t proceed correctly. Capture the return value and propagate/handle the failure (e.g., panic, abort update, or fall back) before zeroizing the buffers.

Suggested change

	wolfBoot_set_encrypt_key(key, nonce);
	{
	int ret = wolfBoot_set_encrypt_key(key, nonce);
	if (ret != 0) {
	wolfBoot_zeroize(key, sizeof(key));
	wolfBoot_zeroize(nonce, sizeof(nonce));
	return ret;
	}
	}

[Copilot]

The doc comment for wolfBoot_erase_encrypt_key() still says it returns 0 on success, but the implementation now returns the underlying flash error code (ret) when erasing/writing the key sector. Update the documentation to reflect the new return semantics (and consider removing the unreachable return 0; after the #endif to avoid dead-code warnings).

[Copilot]

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (3)

tools/unit-tests/unit-string.c:1

• The test function is only compiled when !defined(__GCOV__), but it is added to the suite under a different condition. Under __GCOV__, this can produce a missing-symbol build failure. Make the suite’s tcase_add_test guard match the function’s guard (include && !defined(__GCOV__)).
  tools/unit-tests/unit-sign-encrypted-output.c:1
• This file uses errno/EACCES and PATH_MAX but does not include <errno.h> and <limits.h>, which can break the build on some platforms/toolchains. Add the missing headers explicitly.
  src/libwolfboot.c:1
• This introduces an unconditional return 0; after the #endif, while the #else branch returns ret inside the preprocessor block. This can leave unreachable code in some configurations and triggers warnings on stricter builds. Prefer a single ret variable (set in each branch) and one return ret; at the end.

/* libwolfboot.c

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The aligned word-copy fast path type-puns a char* buffer as unsigned long*, which can violate C strict-aliasing rules (undefined behavior) even when aligned. To keep the optimization safely, use a __attribute__((__may_alias__)) word type (or equivalent project macro) for the cast, or perform the word moves via memcpy into a temporary unsigned long.

[Copilot]

hal_flash_protect()’s return value is ignored. If protection fails on a target that supports runtime write protection, the boot chain proceeds without the intended bootloader protection. Consider checking the return code and panicking/aborting boot (or at least logging and entering a safe state) when protection cannot be applied.

Suggested change

	(void)hal_flash_protect(WOLFBOOT_ORIGIN, BOOTLOADER_PARTITION_SIZE);
	if (hal_flash_protect(WOLFBOOT_ORIGIN, BOOTLOADER_PARTITION_SIZE) < 0) {
	wolfBoot_printf("ERROR: failed to protect bootloader flash\n");
	wolfBoot_panic();
	}

[Copilot]

The weak/default implementation and at least one target implementation declare hal_flash_protect as RAMFUNCTION, but the header prototype does not. To avoid attribute/prototype mismatches (and to document that this can be called from RAM-resident code paths), make the declaration consistent with the implementation.

Suggested change

	int hal_flash_protect(haladdr_t address, int len);
	RAMFUNCTION int hal_flash_protect(haladdr_t address, int len);