Zeroize key material (#5473, #5474, #6030, #5476)#427
Closed
jackctj117 wants to merge 1 commit into
Closed
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens wolfHSM against key-material remanence by explicitly zeroizing sensitive transient buffers (client comm buffers, shared transport buffers, and DMA bounce buffers) after use, and adds regression tests intended to catch future regressions where those zeroization calls might be removed.
Changes:
- Add server-side clearing of the shared request data buffer in the mem transport after processing a request and before publishing the response.
- Zeroize client-side comm buffers after key import/export and unwrap+export operations.
- Zeroize POSIX shm DMA bounce buffers before returning them to the shared static-memory heap, plus add POSIX in-process remanence regression tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/wh_test.c | Adds key-material remanence regression tests that scan long-lived/shared buffers for leftover key bytes. |
| src/wh_transport_mem.c | Clears shared request data buffer (with cache flush) before publishing a response CSR. |
| src/wh_client.c | Zeroizes client comm buffer regions holding imported/exported key material. |
| src/wh_client_keywrap.c | Zeroizes unwrap+export response payload buffer before returning. |
| port/posix/posix_transport_shm.c | Zeroizes DMA bounce buffers before freeing them back to the shared heap. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+224
to
+226
| if (req.s.len != 0) { | ||
| (void)wh_Utils_memset_flush(context->req_data, 0, req.s.len); | ||
| } |
Comment on lines
+849
to
+852
| /* The imported key bytes have been serialized into the transport; do not | ||
| * leave a copy lingering in the long-lived client comm buffer. The shared | ||
| * request transport buffer is cleared server-side once the request has been | ||
| * consumed (see wh_TransportMem_RecvRequest). */ |
| goto out_server; | ||
| } | ||
|
|
||
| WH_TEST_RETURN_ON_FAIL(wh_Client_Init(client, c_conf)); |
Comment on lines
+470
to
+472
| g_remServerRun = 0; | ||
| pthread_join(sthread, NULL); | ||
| (void)wh_Server_Cleanup(server); |
| uint8_t* ptr = (uint8_t*)dmaPtr + (uintptr_t)*xformedCliAddr; | ||
| /* The bounce buffer holds a copy of client input (possibly key | ||
| * material); clear it before returning it to the shared DMA heap. */ | ||
| wh_Utils_ForceZero(ptr, len); |
| len); /* copy results of what server wrote */ | ||
| /* The bounce buffer holds server output (possibly key material); | ||
| * clear it before returning it to the shared DMA heap. */ | ||
| wh_Utils_ForceZero(ptr, len); |
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.
This pull request focuses on improving the security of sensitive key material handling in client-server communications by ensuring that temporary buffers and shared memory regions are securely zeroed out after use. This prevents lingering secrets in memory, reducing the risk of unintended disclosure. The changes add explicit zeroization steps after key material is copied or used, and update buffer management in both client and server code.
The most important changes include:
Sensitive Buffer Zeroization:
wh_Utils_ForceZeroto explicitly clear bounce buffers holding client input or server output (potentially key material) inposix_transport_shm.cbefore returning memory to the shared DMA heap. [1] [2]wh_Client_KeyCacheRequest_exandwh_Client_KeyExportResponseinwh_client.cto zeroize client communication buffers after key material is sent or received, preventing secrets from lingering in long-lived session buffers. [1] [2]wh_Client_KeyUnwrapAndExportResponseinwh_client_keywrap.cto zeroize the entire response buffer containing plaintext keys before returning.Server-Side Shared Memory Management:
wh_transport_mem.c, added a step to clear the shared request data buffer with a cache flush immediately after the request is processed and before publishing the response, ensuring that sensitive request payloads are not left accessible.Code Maintenance:
wh_Utils_ForceZeroin affected source files to support the new zeroization calls. [1] [2]These changes collectively harden the codebase against potential leakage of cryptographic material through shared or long-lived memory buffers.