fix: force-zero key cache on server cleanup#443
Open
MarkAtwood wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens server cleanup by ensuring the whServerContext (which can contain plaintext cached key material) is wiped using the project’s secure zeroization helper rather than a plain memset that compilers may optimize away.
Changes:
- Include
wolfhsm/wh_utils.hinsrc/wh_server.cto access secure wipe utilities. - Replace
memset(server, 0, sizeof(*server))withwh_Utils_ForceZero(server, sizeof(*server))inwh_Server_Cleanup().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
wh_Server_Cleanup() ends by wiping the whole server context with plain memset(server, 0, sizeof(*server)). whServerContext embeds the key cache (whKeyCacheContext localCache -> whCacheSlot/whBigCacheSlot, each holding a plaintext uint8_t buffer[] of cached key material), so this store clears sensitive key bytes. wolfHSM already uses wh_Utils_ForceZero for exactly this purpose across wh_server_she.c and wh_server_keystore.c; using plain memset here is inconsistent with that convention and is a store the compiler is permitted to reason about.
Fix: include wolfhsm/wh_utils.h and replace the memset in wh_Server_Cleanup with wh_Utils_ForceZero(server, sizeof(*server)). The matching memset in wh_Server_Init is left untouched (it zeroes an incoming, not-yet-populated context; no key material).
Verified: compiled src/wh_server.c under the POSIX test config (server enabled, crypto on) with -Werror; objdump -dr of wh_Server_Cleanup shows the call now relocates to wh_Utils_ForceZero and the memset is gone from that function.
Reported by static analysis (Fenrir finding 131).