Harden wolfTrust service isolation and recovery - #22
aidangarske wants to merge 27 commits into
Conversation
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: 2
Open (3)
The byte-wise clearing loop fixes correctness for non-4-byte sizes, but it can be significantly… · Newwt_hsm_relay_submitnow unconditionally dereferencesresp_lenbefore any validation. If… · Newwt_forceZerointroduces a mixed-case public API name in a codebase that otherwise predominantly… · New
What changed in this PR
This PR addresses low/info Fenrir findings by tightening failure paths, improving fault handling, and clearing sensitive buffers across FWU/vault/HSM relay paths, along with related documentation updates.
Changes:
- Add explicit zeroization of sensitive request/response/staging buffers in multiple services and transports.
- Harden scheduler/SPM/coroutine/IRQ/SysTick paths and guest restart/quarantine cleanup behavior.
- Update vault sealed-object deletion/recovery logic and align FWU write behavior with padded final blocks.
| File | Description |
|---|---|
| tests/firmware/zephyr-stm32h5/apps/guest0_psa/src/main.c | Updates test documentation/comments for PS/FWU service behavior. |
| src/sync/mutex.c | Prevents double-enqueue by detecting if a coroutine is already waiting on a mutex. |
| src/spm_partitions.c | Clarifies PS partition behavior and trust boundaries in comments. |
| src/spm_gate.c | Forces panic on unexpected gate outcomes in one path. |
| src/services/wolfhsm/wt_hsm_vault.c | Adds sealed deletion marker + recovery and tightens remove semantics. |
| src/services/wolfhsm/wt_hsm_seal.c | Updates trust-boundary documentation for seal key caching. |
| src/services/wolfhsm/wt_hsm.c | Adds zeroization of relay buffers/server/crypto state on cleanup, reinit, and fault. |
| src/services/wolfhsm/runner/secure.ld | Updates memory-region comments to reflect confined/unprivileged SPs. |
| src/services/vnet/vnet_relay_service.c | Corrects RX_FETCH behavior description. |
| src/services/vault_service.c | Restructures dispatch to avoid early returns and zeroizes buffers before return. |
| src/services/storage_service.c | Restructures dispatch similarly and clears request buffer before return. |
| src/services/hsm_relay_service.c | Returns unified status + zeroizes relay I/O buffer before return. |
| src/services/fwu_service.c | Pads unaligned final block to backend alignment; tightens reset/install failure behavior. |
| src/sched/coroutine.c | Adds canary checks around context switches and null-safety. |
| src/monitor.c | Clears restart window earlier and fails connections on quarantine. |
| src/ffm_boot.c | Updates caller-validation comment to match new pointer validation model. |
| src/client/psa_storage_client.c | Zeroizes client-side request buffer after use. |
| src/client/hsm_psa_transport.c | Ensures response buffer is cleared on init/cleanup/send/recv error paths. |
| src/arch/common/spm_gate_core.c | Tracks lock waits explicitly; avoids waking non-blocked coroutines; runs RUNNABLE SPs. |
| src/arch/armv8m/sp_fault_armv8m.c | Updates fault-recovery comment to match new behavior. |
| src/arch/armv8m/irq_armv8m.c | Uses NS PPB alias for guest IRQ mask ops; simplifies secure IRQ enable/disable. |
| src/arch/armv8m/guest_context_armv8m.c | Fixes SysTick restore sequencing; fixes guest memory zeroing for non-word sizes. |
| port/stm32h563/platform_stm32h563.c | Updates MPU whitelist comment for NS RAM usage. |
| port/stm32h563/memory_map.h | Updates vault/FWU stack comments reflecting confined SP model. |
| port/stm32h563/hsm_flash.c | Zeroizes temporary flash buffers; improves verify/blank-check error flow. |
| include/wolftrust/zeroize.h | Introduces a force-zero helper for sensitive data clearing. |
| include/wolftrust/spm_sched.h | Updates service-start documentation to match confined/unprivileged SP model. |
| include/wolftrust/services/vnet_relay.h | Updates RX_FETCH token semantics documentation. |
| include/wolftrust/services/vault_service.h | Updates vault/key/RNG trust-boundary documentation. |
| include/wolftrust/services/storage_service.h | Updates sealing trust-boundary documentation. |
| include/wolftrust/services/hsm.h | Updates fault notification/sealer/RNG documentation and semantics. |
| docs/TF-M-Compatibility.md | Updates FWU compatibility notes to reflect padding behavior. |
| docs/Services.md | Updates FWU service description to reflect padding + offset alignment. |
| docs/API-Reference.md | Updates FWU API reference to reflect padding + component-id deviation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| void wt_arch_zero_guest_memory(uintptr_t base, size_t size) | ||
| { | ||
| volatile uint32_t* ptr = (volatile uint32_t*)base; | ||
| size_t words = size / sizeof(uint32_t); | ||
| volatile uint8_t* ptr = (volatile uint8_t*)base; | ||
| size_t i; | ||
|
|
||
| for (i = 0; i < words; ++i) { | ||
| for (i = 0U; i < size; ++i) { | ||
| ptr[i] = 0u; | ||
| } |
| *resp_len = 0U; | ||
| if (!g->ready || g->transport_ctx != buf) { | ||
| return WH_ERROR_NOTREADY; | ||
| } |
|
|
||
| #include <stddef.h> | ||
|
|
||
| static inline void wt_forceZero(void* memory, size_t size) |
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #22
Scan targets checked: wolftrust-src, wolftrust-bugs
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
| sizeof(g_relay_bufs[guest_id])); | ||
| wt_hsm_force_zero(&g->server, sizeof(g->server)); | ||
| wt_hsm_force_zero(&g->crypto, sizeof(g->crypto)); | ||
| wt_hsm_force_zero(&g_co_stack_slots[guest_id], |
There was a problem hiding this comment.
Erasing the tasklet stack on fault destroys its canary and panics the whole system · Fault containment failure
wt_hsm_signal_fault zeroes all of g_co_stack_slots[guest_id], including the WT_CO_STACK_CANARY word at co->stack_base. The fault handler then EXC_RETURNs into do_switch right after wt_co_arch_enter(to), where the newly added check_canary(to) reads 0 and calls wt_platform_panic(), so every contained HSM tasklet fault becomes a system-wide panic instead of a per-guest quarantine.
Suggested fix: Restore the canary word at stack[0] after erasing, or exclude the first sizeof(uint32_t) bytes of stack from the erase.
Related known findings (similar but distinct; listed for context, not part of this finding)


Uh oh!
There was an error while loading. Please reload this page.