The keytools read and write private keys through fully buffered
stdio streams: for key files smaller than the stdio buffer, libc
copies the raw key bytes into its internal FILE buffer, and
fclose() frees that buffer without scrubbing it, so a copy of
every key survives the explicit zero_and_free()/wc_ForceZero()
wipes. Open every secret-material stream unbuffered with
setvbuf(f, NULL, _IONBF, 0) right after fopen():
- sign.c load_key(): the private key file. Also close the
stream on the short-read failure path, which previously
leaked it.
- sign.c make_header_ex(): the firmware encryption key/IV file.
- keygen.c: the five private-key write sites (RSA, ECC,
Ed25519, Ed448, ML-DSA).
- lms_common.h / xmss_common.h: the LMS/XMSS private-state
write, verify-read and read callbacks.
Public-key export streams are left buffered: not secret.
Host-side hardening only: whoever can read the tool heap can
read the key file too.
The key/key2 structs in main() hold the decoded private key material
for the rest of the process lifetime, and free_key() only releases
what its dispatch knows about. Scrub both objects unconditionally in
cleanup so no key residue survives, for any algorithm.
sign.py and keygen.py are superseded by the C tools/keytools/sign and
keygen (built by the same Makefile, no python/wolfcrypt-py
dependency). Rewrite unit-sign-delta-tlv.py to sign a real delta
image with the C sign tool instead of the python bmdiff/sign.py
pipeline, and update README.md and include/delta.h references.
The snprintf building output_diff_file passed
sizeof(CMD.output_image_file); both are char[PATH_MAX] today, so the
guarantee was accidental. Use sizeof of the destination.
argv[argc] is NULL, so a missing trailing argument (e.g. 'sign
--ecc256 img.bin key.der') yielded a NULL fw_version that crashed
strtoul, and --id/--encrypt/--delta/--policy as the last token
dereferenced NULL directly. Guard the four value-taking options like
--cert-chain already does, and validate the positional argument
count against the selected signing mode before indexing.
fread() returns size_t, so the (io_sz < 0) guard could never trigger,
and a short read silently folded stale buffer bytes into the image
digest. Compare the read count against the requested size in all
three loops (SHA-256, SHA-384, SHA3-384), matching the existing
io_sz != expected pattern used elsewhere in this file.
case 4 masked to 32 bits then fell through into case 8, whose only
statement is break. Harmless today, but indistinguishable from a
genuine missing break; add the explicit break.
The keystore keygen emits (default for non-OTP, non-wolfHSM builds)
lacked the id < 0 guard the OTP backend has: get_buffer/get_size/get_mask
tested only id >= num (a negative id indexed PubKeys[] out of bounds) and
get_key_type had no bounds check. Add 'id < 0 ||' to all four. Add
unit-keygen-keystore, which sed-extracts the real Keystore_API template
from keygen.c, emits a self-contained keystore.c (keystore_gen.c), compiles
it, and asserts the same out-of-range contract unit-otp-keystore pins.
Mutation (drop the get_buffer guard) makes keystore_get_buffer(-1) return
non-NULL, caught.
The size model was only compared against make_header_ex for NO_SIGN,
no-timestamp, non-delta. Drive the cross product of sign/policy/
timestamp/delta/dts/hash-algo and assert the writer's final content
size stays <= the model, via a test hook that records header_idx
before the 0xFF padding. Hybrid is excluded (its secondary signature
needs a real key context). Mutation-verified: removing the model's
timestamp term makes the test fail (idx > required).
Also reject a custom TLV reusing the --dts digest tag (F-9754): it
would serialize ahead of the real digest and shadow it.
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.
wolfBoot_find_header() and the sign tool's re-parser checked each
field's 4+len against (uint16_t)(header_size - IMAGE_HEADER_OFFSET).
For any header of 64 KiB or more the cast wraps (0x10000 -> 0), so the
guard rejects every field and an image the tool signs cannot be parsed
by the bootloader - a pack/parse roundtrip break, fail-safe but fatal
for large TLVs (post-quantum signatures, big cert chains).
Compare in the uint32_t domain in both walkers. No shipped config
reaches this size yet (largest example is 12288), so this pins the
roundtrip for future large-header configs.
unit-parser-large-header (new) builds the walker with
IMAGE_HEADER_SIZE = 0x10008 - exactly the wrap boundary - and asserts
a 300-byte TLV and a 4-byte version field are located (both were
rejected pre-fix, proven against the pre-fix walker in a scratch
build).
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 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.
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.
main() checked load_key() for the primary key but not for the hybrid
secondary key, and load_key() left *pubkey/*pubkey_sz untouched (or
dangling, after the ED25519/ED448 free(*pubkey)) on its failure paths.
With a missing or undecodable secondary key file the sign tool therefore
either silently emitted a manifest with no secondary public key hashed,
dereferenced a freed pubkey buffer and double-freed it, or crashed on the
uninitialized pubkey_sz2 stack value.
Clear *pubkey/*pubkey_sz on every load_key() failure path, initialize
pubkey_sz2, and exit(1) when the secondary key fails to load.
Add tools/unit-tests/unit-sign-hybrid-keyload, covering the missing-file
and decode-failure contracts of load_key() plus the end-to-end exit
status of the sign tool.
Allows extracting a public key from a DER file and adding it in the same
format as the keystore:
- `X||Y` for ECC
- Raw for Ed25519/Ed448
- Public key DER for RSA
arg2num() parsed --custom-tlv values with signed strtoll(), which
saturates to LLONG_MAX (0x7FFFFFFFFFFFFFFF) on positive overflow. For
LEN==8 no masking is applied afterwards (unlike LEN 1/2/4), so any
value >= 2^63 silently encoded as 0x7FFFFFFFFFFFFFFF instead of the
value the user supplied, breaking the TLV encode/decode roundtrip.
Switch to strtoull() and reject (exit 16) when it reports ERANGE for
an 8-byte value, mirroring the existing fw_version range check.
otp-keystore-gen.c reads the device root UDS into the stack buffer
`uds` and copies it into the heap buffer `otp_buf` at OTP_UDS_OFFSET,
then on every exit path calls free(otp_buf) without wiping it first,
and never clears `uds`. Both copies of the highest-value device secret
remain in the host process's freed heap chunk and stack frame.
Add a local secure_zero() helper (no wolfSSL dependency, matching this
standalone host tool's existing bare-gcc build) and call it on the
success path and on the write-failure/short-UDS-read error paths,
before free()/exit(), mirroring the zeroize-before-release pattern
already used elsewhere in this tree (src/x86/ata.c,
src/x86/ahci.c). Paths that exit before `uds` is populated are left
untouched since there is no secret to wipe yet.
The --custom-tlv TAG LEN VAL path in make_header_ex() passed
&CMD.custom_tlv[i].val (a raw uint64_t pointer) directly to
header_append_tag(), which does a memcpy. On big-endian build hosts the
first LEN bytes of the uint64_t are the high bytes, producing the wrong
LE encoding (e.g. 4-byte value 0x12345678 encodes as 00 00 00 00). The
fix serialises through header_store_u64_le() before calling
header_append_tag(), matching the pattern already used by
header_append_tag_u16/u32/u64 for all system TLVs.
Add unit-sign-custom-tlv-le.py to verify the LE byte encoding of 4-byte
and 8-byte custom TLV values in the signed image header.
strtol saturates to LONG_MAX (INT32_MAX) and sets errno=ERANGE for
version strings above 2147483647 on Windows LLP64 and 32-bit hosts,
silently encoding the wrong version. strtoul covers the full uint32_t
range on all platforms (ULONG_MAX >= UINT32_MAX). Add explicit
out-of-range error to match existing pattern (lines 2063-2071).
Replace bare exit(1) calls with goto cleanup / exit_code pattern matching
keygen_xmss; wc_LmsKey_Free + wc_ForceZero now run on every error path
after wc_LmsKey_Init succeeds, preventing LMS private-state exposure.
Replace bare exit(1) calls with goto cleanup / exit_code pattern matching
keygen_ml_dsa; wc_XmssKey_Free + wc_ForceZero now run on every error path
after wc_XmssKey_Init succeeds, preventing XMSS private-state exposure.
sign_tool_find_header returns a raw pointer into LE-encoded TLV bytes.
Aliasing it as uint32_t* and dereferencing produces the wrong value on
big-endian build hosts (e.g. version 5 read as 0x05000000). Decode
explicitly byte-by-byte like header_store_u32_le does for writes.
wc_ecc_free does not zero the struct; add wc_ForceZero(&k, sizeof(k))
immediately after, matching the pattern used for ed448 (F-4971), ml_dsa
(F-4972), and the d[] / priv_der[] zeroing already present on the
following lines.
wc_ed448_free does not zero the struct; add wc_ForceZero(&k, sizeof(k))
immediately after, matching the pattern used for ml_dsa (F-4972) and the
priv[] zeroing already present on the preceding line.
wc_MlDsaKey_Free does not zero the struct; add wc_ForceZero(&key,
sizeof(key)) immediately after, matching the explicit pattern already
used in the adjacent keygen_xmss function.
base_diff() captured patch_inv_off = len3 + CMD.header_sz before calling
make_header_delta(), which signs the delta image via make_header_ex(is_diff=1).
When a certificate chain is present, the delta (is_diff=1) header needs ~72
more bytes than the non-delta header for the four delta TLVs plus the base-hash
TLV. For a window of cert-chain sizes, header_required_size(is_diff=0) still fit
the current CMD.header_sz while header_required_size(is_diff=1) did not, so
make_header_ex(is_diff=1) grew CMD.header_sz to the next power of two *after*
patch_inv_off was captured. The HDR_IMG_DELTA_INVERSE TLV then encoded a stale,
too-small offset; the bootloader (update_flash.c) uses it as a raw byte offset
into the update partition to locate the inverse patch, so rollback read from the
wrong offset and failed.
Resolve the is_diff=1 header-size expansion (same logic as make_header_ex)
before computing patch_inv_off. Add unit-sign-delta-cert-inv-off.py, which signs
an ed25519 delta with a 300-byte chain (inside the triggering window) and
asserts the inverse patch is the trailing HDR_IMG_DELTA_INVERSE_SIZE bytes of
the file; it fails before this fix.
sign.py encoded HDR_IMG_DELTA_SIZE and HDR_IMG_DELTA_INVERSE_SIZE with a
2-byte length via struct.pack("<H", ...), but wolfBoot_get_delta_info()
accepts those tags only when wolfBoot_find_header() returns
sizeof(uint32_t). Delta images produced by sign.py were therefore signed
with parseable TLVs yet rejected by the bootloader before the patch was
applied. Encode both size TLVs as 4-byte little-endian values, matching
sign.c (header_append_tag_u32) and the bootloader parser.
Add a regression test that signs a real delta image with sign.py and
asserts the bootloader-side parse recovers each delta TLV with the
required 4-byte length.
Daniele Lacamera
David Garske
Brett Nicholas
Mattia Moffa