keygen_die() (F-9767) calls wc_FreeRng(); the test mocks
keygen.c's wolfCrypt dependencies one by one and was missing
this one, breaking the unit_tests and test_external_libs jobs.
security_command_passphrase() only wiped the static DMA buffer on the
synchronous path; in async mode (ata_security_erase_unit) the
passphrase stayed resident in the file-static buffer for the rest of
the boot, since ata_cmd_complete_async() never scrubbed it.
Add a scrub_buffer flag to the async state, set it when the command
goes in flight (or wipe immediately if it never started), and scrub in
ata_cmd_complete_async() on both the success and task-file-error exits,
once the HBA has retired the command.
Covered by two new unit tests: scrub after async completion and after
an async port error; the in-flight state is asserted unscrubbed.
Reported by Fenrir.
The TPM NV auth copy and the HMAC session state lived on the
stack until process teardown. Zeroize both at the common exit
label after the TPM device is unloaded, on success and error
paths alike.
keygen_ed25519/ed448/lms/xmss/ml_dsa have the same bypass as
the RSA/ECC helpers: exit() from their cleanup labels skips
main()'s wc_FreeRng(). Route their failure exits through
keygen_die() as well.
keygen_rsa/keygen_ecc exit() from their own cleanup labels,
bypassing main()'s wc_FreeRng() and leaving DRBG state resident.
Add keygen_die() that frees and zeroes the RNG before exit and
use it in the two helpers' failure paths.
The error path returned before the common wc_FreeRng(), leaving
DRBG state on the stack. Route it through the shared cleanup and
guard the sign call so hash_type/mgf are only used when set.
The F-12922 test checks fallback to the lower version, but the
TCase was still named "rollback denied" from before the fix,
describing the opposite behavior.
ext_flash_erase() now checks NAND_STATUS_FAIL | NAND_STATUS_WPS
(F-12881), but the sed extraction rule only pulled the command
defines, so the unit_tests CI jobs failed to compile the test.
pkcs11_crypto_init() tore down the session on failure but left the
pre-populated pkcs11_pin credential in retained bootloader memory
after the handoff. Wipe it in the failure path, same as the deinit
path (F-12114). Regression test in unit-pkcs11-pin-zeroize: login
rejected -> init fails -> every pin byte is zero.
The version gate in the retry loop panicked whenever the selected
candidate failed verification and the fallback partition carried a
lower version, even though that image is valid and signed. A single
corrupted higher-version image bricked the device instead of falling
back. The gate only ever fired on the failure path (candidate
selection already prefers the higher version), so remove it; the
TESTING-state anti-rollback check in wolfBoot_dualboot_candidate()
still guards the confirmed-update path.
wolfBoot_start() zeroed os_image once, before the retry loop, so the
fallback iteration kept the stale img->hdr of the failed partition and
wolfBoot_open_image_address() (which only adopts load_address when
hdr is NULL) re-verified the wrong image. The external header cache
had the same problem: it kept the first image opened.
memset the struct and invalidate the EXT_FLASH header cache at the
top of each loop iteration, per the documented precondition of
wolfBoot_open_image_address().
Add unit-update-ram-nofixed-noramboot: the non-fixed-partition,
non-RAMBOOT (XIP) layout where load_address varies per retry, with
fallback tests both directions plus candidate-selection checks.
The mock abandoned a faulting erase/program whole, so half-written sectors
went untested; it can now tear one part-way and the sweep runs every crash
point at 1/4, 1/2 and 3/4. An empty read must also leave the node in place
with size 8, so a lost or torn header no longer passes as an empty rewrite.
The crash == ops iteration injects nothing, so it must read back new_p; the
empty/absent branch was letting a silently lost complete rewrite pass. Also
assert the uninjected baseline write lands before measuring against it.
Under MOCK_STALE_CACHE the shadow is the flash array and vault_base only the
CPU's view, so restoring vault_base alone let the power cycle copy the last
iteration's result back: no crash case started from the old generation.
vault_obj_read() now separates "object absent" from a failed read, so the
power-fail test rejects a corrupted vault instead of accepting any negative.
hal_cache_invalidate() shrank 120 -> 44 bytes; the 48 it still costs every
STM32F407 build is folded into the test-size-all limits.
The store commits sectors with hal_flash_erase()/hal_flash_write() and
then reads them back through the memory map - sector_ptr(),
cache_get_sector()'s refill, and the raw magic reads in check_vault().
On a part that caches flash reads (STM32 ICACHE) those reads can return
pre-erase bytes. check_vault() is the worst case: a stale magic there
does not merely read wrong, it triggers restore_backup() or a full vault
re-initialisation, losing the token.
Adds power-fail injection to the flash mock and a test that cuts power at
every flash operation of a rewrite window, asserting the object always reads
back as the whole old payload, the whole new payload, or empty. Fails at
op 3 without this fix, passes with it.
unit-pkcs11_store 10/10; full unit-tests suite green.
test_concurrent_reader_sees_pending_writes only ever read from flash:
the reader's Store_Open calls check_vault(), which flushes the sector
cache, so the sector_ptr() read path in Store_Read was never exercised
and the test passed identically against the pre-PR memcpy.
Write more on the still-open writer after the reader is open. That
batch lands only in the sector cache, so the reader can only see it
through the cached read path and the live header size; a flash-only or
snapshot-size read returns EOF here. Verified: the new assertion fails
against the pre-fix store (ret == 0) and passes with the live-size fix.
Addresses PR #873 review comments (wolfSSL-Fenrir-bot,
tools/unit-tests/unit-pkcs11_store.c:587, both near-duplicate findings).
Store_Read and Store_Write used handle->size, a snapshot taken at
Store_Open. The payload path reads through the sector cache, so once
another window's batch (e.g. a write-open truncation) sat pending in
the cache, the window saw live erased data under a stale size and
returned 0xFF bytes past the true end instead of EOF. Pre-PR the size
was read live from the flash header on every call, so the PR regressed
that case.
Read the size from the same (possibly cached) header sector the payload
comes from, via store_live_size(), so size and data share one source of
truth. Drop the now-dead handle->size snapshot; update_store_size()
only writes the cached header node.
Addresses PR #873 review comment (wolfSSL-Fenrir-bot,
src/pkcs11_store.c:711).
check_vault() dropped the shared sector cache on every vault
validation, silently losing the pending writes of any still-open
window when another handle was opened or an object removed
(MAX_OPEN_STORES allows 16). Flush instead - the atomic header-last
commit - so an in-flight batch only gets an earlier commit point;
its data is never discarded.
wolfPKCS11_Store_Read() now reads through sector_ptr() like every
other read in the file, so a sector still in the cache can never be
read stale against a live size.
Add unit tests covering the interleaved-window data loss and a
concurrent reader observing a pending write; both fail without the
check_vault fix.
Every wolfPKCS11 field write flushed the payload sector and the header
sector to flash (2 erases + 2 programs of a full sector each), and the
token store re-serializes all objects per C_CreateObject/C_DestroyObject,
so those calls cost hundreds of sector erases and tens of seconds on
flash with slow erase times.
Cache modified sectors in RAM and commit them together when the store
window closes:
- sector cache sized to the worst-case span of one object plus the
header sector (WOLFBOOT_PKCS11_STORE_CACHE_SECTORS), LRU eviction
when exceeded
- header sector commits last, so a committed header is the atomic
commit point of the batch: power failure during a flush leaves the
flash in either the pre-batch or the post-batch state
- per-commit backup sector write preserved, keeping recovery of the
sector in flight at failure time
- delete_object commits on return (durability contract, unit-tested)
- nodes table, bitmap, payload ids and the live object size
(handle->size) are read from the cache when the sector is dirty
Measured on an STM32H5 with 8KB sectors, wolfPKCS11 in the secure
world: C_CreateObject 1.5s -> 0.15s, C_DestroyObject 1.3s -> 0.12s,
456 -> 40 sector erases per create, and the count no longer scales
with the number of objects in the token.
PKCS11_STORE_STATS (off by default) adds flash-activity counters and a
test-app bench to quantify store traffic: make PKCS11_STORE_STATS=1.
The new unaligned tests passed against the pre-fix HAL (identical
final bytes), so the alignment fix had no regression coverage. Mock
hal_flash_wait_complete now diffs the flash per program window and
asserts the changed bytes fit in one aligned unit; the 20-byte
unaligned test goes red on the pre-fix HAL (l5: bytes 4-11 across two
8-byte units, u5: bytes 4-19 across two 16-byte units).
hal/stm32wb.c: dedicated RCC_CFGR_SWS_{MSI,MASK} macros for the
clock-switch confirmation wait; SW/SWS encodings verified identical
in RM0434 6.4.3 and the STM32WB55 SVD.
unit-stm32u5-write.c: START_TEST brace on the next line, matching
the file and the unit-suite convention.
An unaligned starting address split the four word stores across two
16-byte program units, leaving partial quad-words that set
FLASH_SR_WDW and hang the wait for completion. Align the destination
down to the unit, read-modify-write the whole unit, and store through
the aligned pointer. The unit test gains unaligned-start cases.
hal_flash_write() stored both words of the 64-bit program unit
relative to the caller's address, so a write starting inside a unit
split the two stores across two units: the flash has no 32-bit
program mode, so nothing is programmed and the second store faults
on alignment. Align the destination down to the unit, take the bytes
outside the requested span from the unit itself, and store through
the aligned pointer, as hal/stm32h5.c does. The TrustZone claim is
unchanged: it is page-granular and already covers every non-secure
byte the aligned program touches.
Extend unit-stm32l5-write with unaligned-start cases. The host data
model cannot observe the program-unit split (pre-fix the bytes land
identically), so these pin the fixed layout: bytes before the
request preserved, nothing past it touched.
The HAL contract is absolute (0x08000000-based) addresses, as the
erase path and every NVM caller use, but the double-word fast path
added the flash base on top and targeted an address space past the
flash. The read-modify-write path located its unit from the request
base indexed by i/4, landing in the wrong 8-byte unit once a request
started inside one. Both paths now program the 8-byte unit at
(address + i), and the now-dead FLASHMEM_ADDRESS_SPACE define is
gone.
Add unit-stm32c0-write: runs the extracted hal_flash_write() against
a host register/flash model with a second mapping standing in for
the wrong address space (4/5 checks fail pre-fix, 5/5 pass post-fix).
The final image-size check re-derived the low-memory limit with a
uint32 subtraction and no ordering check, so an inverted tolum
wrapped into a near-2^32 limit and accepted any image. Compare the
tolum/load_address ordering in 32-bit (low-memory) form when
computing slot_max, and reuse that validated value in the check.
Add a unit test for the inverted-tolum case (fails closed, both
slots rejected).
The PKCS#11 and PSA store Remove paths invalidated the metadata and
freed the bitmap slot but left the payload in flash, so removed keys
stayed recoverable by a physical reader. Both Remove paths now call
erase_object_payload() before invalidating the metadata; the existing
sector read-modify-write preserves neighboring slots. Raw-flash
deletion tests added to both unit suites.
Cap the library fw_size clamp at UINT32_MAX (a > 4 GiB file would
otherwise truncate to a small value), say 'restore' not 'store' in the
scatter-restore error message, and print stdout as well as stderr in
the compile-check scripts on failure.
The ELF scatter destination is the exec region, which sits outside the
boot partition that stores the signed ELF. Bounding it to the boot
partition rejected every legitimate segment (aurix exec is below boot,
sim scatter is above it) and bricked corruption recovery. The paddr is
covered by the image signature verified before this restore path and the
overflow check keeps it from wrapping, so no destination bound is needed;
this also matches the check function, which bounds no destination.
If the snap allocation fails but the work allocation succeeds, the
initialisation loop never runs and nsc_tmpl_free() would release
indeterminate work[].pValue pointers. Zero work[] right after the
allocation (before the NULL check) so every path that reaches the
free hands out initialised, NULL pValue entries.
keystore_get_size() returns -1 on invalid or oversized OTP slot
data; storing it in uint16_t made -1 become 65535, which passed the
hdrSz <= 0 check and was fed to the ECC/RSA parser as a 65535-byte
read from the keystore buffer. Keep it as int and reject values
<= 0 or above KEYSTORE_PUBKEY_SIZE.
wolfPKCS11/PSA_Store_Read/Write added int len to unsigned
in_buffer_offset before validation: a sufficiently negative len
wrapped, hit the truncation branch, and was replaced by the remaining
object or capacity bytes, bypassing Write's later len < 0 guard.
Reject len < 0 first in all four functions (post-clamp guard kept).
get_sha_block() accepted offset == fw_size and always read
WOLFBOOT_SHA_BLOCK_SIZE bytes; wolfBoot_peek_image() always reported
the full block size. Both could hand callers a window past the end
of the image. Reject offset >= fw_size, clamp the external read to
the bytes remaining, and report the clamped size (0 when no bytes
remain). Hash callers already clamp their input, so this is API
hardening; pinned by test_peek_image_bounds (internal + ext cases).
wolfBoot_load_flash_image_elf() ignored read_flash_fwimage failures
and used the program-header fields unchecked, so a failed read
consumed indeterminate stack data and a malformed (but signed) ELF
could drive an out-of-bounds source read and erase/write at an
unintended destination.
Check the ELF header and program header reads, then validate each
PT_LOAD segment before copying: file_size fits a 32-bit length, the
source stays inside the manifest image, the paddr range fits the
destination address width, and the destination stays inside the boot
partition (mirrors the sibling check-function validation). Check the
copy result.
New unit tests in unit-image-elf-scatter.c cover the load path: a
valid restore (positive control) and rejections for source past
fw_size, paddr range overflow, destination outside the boot
partition, and a program header that cannot be read. The first three
fail pre-fix (the mock flash layer also catches the two
out-of-range destinations with its own address check); the phdr
read-failure case consumed uninitialized stack data pre-fix
(valgrind: conditional jump on uninitialised value at the
is_loadable check).
wc_ecc_rs_raw_to_sig() takes a word32* outlen, but the wolfHSM
client/server path in wolfBoot_verify_signature_ecc() declared the
buffer length as size_t and cast the pointer. On a 64-bit
big-endian target the API reads the high (zero) half, so the DER
conversion sees outlen 0 and the write-back lands in the wrong
half of the size_t.
Declare tmpSigSz as word32 and pass &tmpSigSz directly; this also
matches the word32 sigLen of wc_ecc_verify_hash() below.
Add a host compile check for the WOLFBOOT_ENABLE_WOLFHSM_CLIENT
build of image.c, which unit CI did not cover (only the PIC32CZ
cross build).
boot_x86_64.c declared x86_64_efi_do_boot(uint8_t *) while the HAL
defines (uint32_t *, uint8_t *): the linker connected the incompatible
pair and the call was undefined behavior, mostly latent because the
second parameter was discarded. Match the AArch64 sibling: single
const uint32_t *boot_addr in the declaration, definition and call,
and remove the unused dts_address parameter.
The UEFI MEMMAP_DEVICE_PATH EndingAddress is inclusive (last valid
byte), but x86_64_efi_do_boot() set it to boot_addr + size, describing
every image as one byte longer than it is; the AArch64 sibling already
uses size - 1. A zero-size image would underflow that computation and
hand an empty range to LoadImage, so reject it up front, as the
AArch64 sibling does. The local panic() returns to the unit test under
UNIT_TEST so the zero-size path is observable.
The WOLFBOOT_FSP low-memory size check was the only per-slot rejection
in the boot retry loop that used a bare break, so an image whose
header-declared fw_size exceeded the tolum window aborted the boot
instead of trying the other slot. Every other rejection in the same
loop switches partitions and retries; match that.
STAGE1_AUTH only authenticates the stage2 wolfBoot payload; the FSP-M and
FSP-S blobs are executed unverified. Remove the dead fsp_m/ret
declarations in start(), the orphaned .sig_fsp_s placeholder section
(no stage1 linker script places it), and correct the comment that
claimed the FSPs were authenticated. Add a compile check for the
STAGE1_AUTH variant (unit-x86-fsp-stage1auth-build.py), which the unit
test CI never builds for lack of an i686 toolchain.
Verification: full unit suite 1096 checks, 0 failures; both
STAGE1_AUTH variants of boot_x86_fsp.c compile clean; no references to
sig_fsp_s remain.
In the DISABLE_BACKUP branch of wolfBoot_update() the ELF-scatter restore
block passed the boot struct by value to the pointer-taking PART_IS_EXT
macro, so DISABLE_BACKUP + WOLFBOOT_ELF_FLASH_SCATTER + EXT_FLASH did not
even compile, and the load result was discarded. Mirror the
wolfBoot_start() pattern (PART_IS_EXT(&boot), panic on load failure), drop
the dead base local, and add a compile check for the combination
(unit-elf-scatter-db-build.py) guarding the one test that needs a
DISABLE_BACKUP-excluded symbol.
Verification: full unit suite 1096 checks, 0 failures; new test fails
pre-fix (struct vs pointer compile error), passes post-fix.
The per-sector fill loop in wolfBoot_delta_update() advances in
DELTA_BLOCK_SIZE steps, so a WOLFBOOT_SECTOR_SIZE that is not a multiple
of DELTA_BLOCK_SIZE writes past the one-sector SWAP partition and
misaligns the resume path. Enforce the invariant with a #error and add a
negative build test (unit-delta-sector-align.py).
Verification: full unit suite 1096 checks, 0 failures; new test fails
pre-fix (misaligned config built), passes post-fix (build rejected).
PR review (wolfSSL/wolfBoot#880, Fenrir bot) flagged that
test_noramboot_ext_flash_short_read_rejected set the shared mock
globals mock_ext_flash_short_len/mock_ext_flash_short_bytes and only
cleared them after the ck_assert. The suite runs CK_NOFORK, so a
failing assertion longjmps out of the test and leaves every full-size
ext_flash_read truncated for test_noramboot_highversion_rollback_denied,
which then fails for an unrelated reason.
Clear the mock globals immediately after wolfBoot_start() returns,
before the assertion: the assert only checks wolfBoot_staged_ok, which
is set during wolfBoot_start(), so the ordering changes nothing about
what is tested and the mock state can no longer leak into the next
test on a failure path.
Verification: full unit suite in wolfboot-ci-sim (make -C
tools/unit-tests; make run) exit 0; unit-update-ram-noramboot 3/3.
Daniele Lacamera
David Garske
Brett Nicholas