Conversation
f1a0807 to
c577968
Compare
There was a problem hiding this comment.
Pull request overview
Adds explicit zeroization of sensitive buffers (ForceZero + optional wc_MemZero_Check) to address several missing wipe sites across TLS 1.2/1.3 key derivation paths.
Changes:
- Ensure temporary key material in TLS 1.3 secret derivation is wiped before returning.
- Wipe PRF/key-derivation intermediate buffers in TLS 1.2/legacy paths before freeing.
- Introduce misc include plumbing in
keys.cto makeForceZeroavailable.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/tls13.c | Adds cleanup path to wipe derived key buffer(s) in TLS 1.3 secret derivation. |
| src/tls.c | Wipes PRF digest buffer before freeing in TLS key derivation. |
| src/keys.c | Wipes multiple TLS 1.0–1.2 derivation intermediates; replaces manual loops with ForceZero; adds ForceZero include wiring. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ForceZero(shaInput, KEY_PREFIX + SECRET_LEN + 2 * RAN_LEN); | ||
| ForceZero(keyData, KEY_PREFIX * WC_MD5_DIGEST_SIZE); | ||
| #ifdef WOLFSSL_CHECK_MEM_ZERO | ||
| wc_MemZero_Check(shaOutput, WC_SHA_DIGEST_SIZE); |
There was a problem hiding this comment.
Change this to wc_MemZero_Add at the top.
That way all code paths will be checked.
Check will then happen in free.
There was a problem hiding this comment.
Added the missing wc_MemZero_Add() calls at the top (TIL that these are required to make wc_MemZero_Check() actually check something...).
However, we cannot remove the manual wc_MemZero_Check() calls here, as XFREE is only called for WOLFSSL_SMALL_STACK builds. Hence, the check would be missing for stack allocations.
c577968 to
e615773
Compare
e615773 to
001eae7
Compare
F-13, F-14, F-197, F-198, F-199, and F-200. All at once as these are very similar.