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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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).
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.
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.
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.
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.
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.
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.
The PKCS#11 non-secure-callable veneers deep-copy every NS attribute
value and every mechanism parameter into secure-world heap. On key
import (C_CreateObject/C_UnwrapKey/C_CopyObject/C_SetAttributeValue
carrying CKA_VALUE or the RSA private components) and on password-based
derivation (CKM_PKCS5_PBKD2 pPassword) those bounce buffers hold
plaintext secrets, but nsc_tmpl_free() and nsc_mech_free() released them
with a bare XFREE(), leaving the material in the freed secure heap block
until something else happens to overwrite it.
Scrub each block with wc_ForceZero() before releasing it. The template
values use the prepare-time snapshot length, since wolfPKCS11 rewrites
work[].ulValueLen on the C_GetAttributeValue path; nsc_alloc() now
records the length of each mechanism allocation for the same reason.
Adds unit-pkcs11-nsc-zeroize, which drives C_CreateObject_nsc_call and
C_DeriveKey_nsc_call over a secure-heap stand-in that is never cleared,
and fails if the imported key or the PBKDF2 password survives the free.
main() called make_header()/make_hybrid_header() and discarded their
return value. Both are wrappers around make_header_ex(), which returns
-1 on every "goto failure" path (image file not openable, header malloc
failure, firmware version out of range, certificate chain errors,
signing and output write errors). Since ret is initialized to 0 and is
only reassigned by the optional base_diff() delta step, a signing run
that produced no output image still terminated with status 0, so
Makefile recipes and CI treated the failure as success and moved on
with a missing or stale *_v<ver>_signed.bin. This was also asymmetric
with the key loading path just above, which exits on failure.
Capture the return value of both header helpers, skip the delta step
when header generation failed, and let main() return it.
Add tools/unit-tests/unit-sign-header-failure, covering the exit status
of both the plain and the hybrid signing path when the input image
cannot be opened.
David Garske
Josh Holtrop
Daniele Lacamera
Brett Nicholas
Marco Oliverio
Aidan Garske
Mattia Moffa