Open
Conversation
F-2139 Previously the plaintext private key DER buffer was freed via XFREE without a preceding ForceZero when no password encryption was requested. Track the actual allocation size and zeroize the buffer before release.
F-2140 wolfSSL_PEM_write_mem_DSAPrivateKey serializes the DSA private key to a heap DER buffer and freed it on five paths without ForceZero. Zeroize the buffer before each XFREE.
F-2141 The error path in wolfSSL_PEM_write_mem_ECPrivateKey freed the EC private key DER staging buffer without ForceZero. Zeroize before free.
F-2142 wolfSSL_RSA_To_Der could free a buffer holding RSA private key material when the DER encoding step failed. Record the allocation size and ForceZero the buffer before XFREE on the private key path.
F-2143 ssl_SetWatchKey_file loaded a private key file into a heap buffer and freed it without ForceZero on both error and success paths. Zeroize before XFREE.
F-2144 SetStaticEphemeralKey loaded a private key file into keyBuf and freed it without ForceZero. Static ephemeral keys are long-lived, so zeroize the buffer before XFREE.
F-2145 wolfSSL_CTX_use_RSAPrivateKey staged the RSA private key DER (PKCS#1: n, e, d, p, q, dP, dQ, qInv) in a heap buffer and freed it without ForceZero. Zeroize before XFREE.
F-2146 wolfSSL_d2i_RSAPrivateKey_bio read PKCS#1-encoded RSA private key DER from a BIO into a heap buffer and freed it without ForceZero. Zeroize before XFREE on both success and error paths.
F-2147 The error path in wolfSSL_i2d_ECPrivateKey could free an EC private key DER staging buffer that may contain a partial private scalar. Zeroize before XFREE.
F-2148 pem_write_mem_pkcs8privatekey stages the PKCS#8 DER encoded private key at the tail of the PEM buffer, then writes the shorter PEM output at the head of the same buffer. The DER tail is not overwritten, leaking the plaintext private key to heap memory after the callers free. Zero the DER staging area before returning.
F-2148 The prior fix zeroed the computed DER staging area, but PEM output from wc_DerToPemEx fills most of the buffer and overlaps that region, corrupting the valid PEM. Preserve the allocation size and zero only the bytes beyond the actual PEM length, or the whole buffer on failure.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR hardens key-handling paths by zeroing sensitive buffers before freeing them, reducing the chance of plaintext key material lingering in heap memory.
Changes:
- Add
ForceZero()calls beforeXFREE()for RSA/EC/DSA key DER buffers and key-file buffers. - Track allocation sizes in a few paths to wipe the full allocation (including any unused tail) before freeing.
- Add post-conversion cleanup for the PKCS#8 PEM-write path to clear DER staging remnants.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ssl_load.c | Zero RSA private key DER buffer before freeing in wolfSSL_CTX_use_RSAPrivateKey. |
| src/ssl.c | Zero ephemeral key-file buffer before freeing in SetStaticEphemeralKey. |
| src/sniffer.c | Zero watch-key buffers on both error and success paths before freeing. |
| src/pk_rsa.c | Zero temporary RSA DER buffers before freeing; introduce derAllocSz tracking. |
| src/pk_ec.c | Zero EC private key DER buffers on error paths before freeing. |
| src/pk.c | Zero DER buffers before freeing in PEM/PKCS#8 conversion paths and track allocation size for tail wiping. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Two follow-ups raised by Copilot review on PR wolfSSL#10247: src/pk_rsa.c: Make derAllocSz a word32 instead of int and only assign it after a successful XMALLOC, so the cleanup path can never call ForceZero with a wrapped-around size derived from a negative derSz. src/pk.c: Capture allocSz at the XMALLOC call site (and clear it back to 0 on allocation failure) so the relationship between the buffer allocation and the recorded size is explicit and cannot drift if the surrounding control flow changes.
Member
Author
|
Retest this please flaky test |
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.
2139
2140
2141
2142
2143
2144
2145
2146
2147
2148