Fenrir fixes 2026 08 04 - #842
Merged
Merged
Conversation
arm_tee_psa_call() only ran cmse_check_address_range() on descriptors
whose .len was non-zero, so a non-secure caller could pass an outvec of
{Secure address, 0} and skip attribution checking entirely. Several
dispatch handlers write a fixed-size object through out_vec[0].base
without consulting out_vec[0].len (ARM_TEE_PS_GET_SUPPORT,
ARM_TEE_CRYPTO_OPEN_KEY/IMPORT_KEY/GENERATE_KEY and
GET_KEY_ATTRIBUTES), which turned that into an arbitrary write into
Secure memory from the non-secure world.
Check every non-NULL .base with at least one byte regardless of the
declared length, and require the handlers that write a fixed-size object
to be given a large enough output descriptor.
Adds unit tests covering the zero-length Secure outvec and the
PS_GET_SUPPORT length check; the CMSE stub is now a test-provided
function so it can model a Secure region.
wolfBoot_tpm2_read_cert() is a cmse_nonsecure_entry veneer. It dereferenced the caller-supplied 'certSz' to bound-check 'cert' with cmse_check_address_range(), then passed the same non-secure pointer to wolfTPM2_NVReadCert(), which re-reads '*len' as the destination capacity before copying the NV data (lib/wolfTPM/src/tpm2_wrap.c:7221). The length was therefore fetched twice from non-secure memory with no snapshot in between, so a racing non-secure agent could present a small capacity to pass the CMSE check and enlarge it before wolfTPM's own check, making the secure world write the certificate past the validated range and into adjacent Secure SRAM. Single-fetch the capacity into a secure local before validating, hand wolfTPM the local, and copy the result back, matching ns_outlen_begin() in src/pkcs11_callable.c and the rsp_capacity handling in src/wolfhsm_callable.c and src/fwtpm_callable.c. Add unit-tpm-nsc-cert, which drives the veneer through a CMSE stub that models Secure SRAM immediately after the validated non-secure buffer and a wolfTPM stub that enlarges the non-secure length word in the race window. The out-of-bounds write test fails before this fix and passes after it.
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.
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.
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.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens several TrustZone-M/NSC and signing tool paths by preventing insecure pointer/length manipulation, ensuring secret bounce buffers are scrubbed, and making the signing tool correctly fail fast (and propagate failures) instead of continuing with invalid state.
Changes:
- PKCS#11 NSC: zeroize secure-world bounce buffers (attribute values and mechanism parameter allocations) before freeing.
- Sign tool: fail when hybrid secondary key load fails; propagate
make_header()/make_hybrid_header()failures to the tool’s exit status (skipping delta generation on failure). - TPM + ARM TEE: snapshot/validate non-secure length for
wolfBoot_tpm2_read_cert(); tighten PSA IPC vector validation (reject Secure pointers even with zero-length iovecs; enforce output buffer size for key IDs / attributes).
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tools/unit-tests/unit-tpm-nsc-cert.c | Adds regression tests for NS length-race hardening in wolfBoot_tpm2_read_cert(). |
| tools/unit-tests/unit-sign-hybrid-keyload.c | Adds tests ensuring hybrid secondary key-load failures clear pubkey outputs and cause sign tool failure. |
| tools/unit-tests/unit-sign-header-failure.c | Adds tests ensuring sign tool returns non-zero when manifest/header creation fails (normal + hybrid). |
| tools/unit-tests/unit-pkcs11-nsc-zeroize.c | Adds tests verifying PKCS#11 NSC secure bounce buffers are scrubbed before free. |
| tools/unit-tests/unit-arm-tee-psa-ipc.c | Extends PSA IPC unit tests for rejecting Secure pointers in zero-length outvecs and for GET_SUPPORT validation. |
| tools/unit-tests/Makefile | Registers new unit tests and adds build rules for them. |
| tools/unit-tests/arm_cmse.h | Changes CMSE stub behavior for unit tests to use a test-provided cmse_check_address_range() implementation. |
| tools/keytools/sign.c | Fixes hybrid secondary key-load failure handling; propagates header creation failures to exit status and gates delta step on success. |
| src/tpm.c | Snapshots NS *certSz before validating the cert buffer to prevent TOCTOU/race expansion. |
| src/pkcs11_callable.c | Scrubs secure allocations for mechanism/template bounce buffers before freeing. |
| src/arm_tee_psa_ipc.c | Tightens PSA IPC argument validation (output buffer sizing + zero-length iovec Secure-pointer rejection + GET_SUPPORT checks). |
| .gitignore | Adds ignore for the new unit-tpm-nsc-cert binary (but not the other newly added unit test binaries). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mattia-moffa
approved these changes
Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
35a23bf F-6875: pkcs11: zeroize NSC bounce buffers before freeing them
dfdcf7e F-7053: sign: propagate make_header() failure to the exit status
675a927 F-7054: sign: fail when the hybrid secondary key cannot be loaded
0e53cac F-7065: tpm: snapshot NS length in wolfBoot_tpm2_read_cert veneer
307c2e8 F-7389: arm_tee: reject Secure .base in zero-length PSA iovecs