Fix fenrir security findings for wolfcrypt#9830
Fix fenrir security findings for wolfcrypt#9830JacobBarthelmeh merged 1 commit intowolfSSL:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses multiple Fenrir security findings in wolfCrypt by switching sensitive comparisons to constant-time operations and zeroizing secret material before freeing/returning.
Changes:
- Replaced timing-sensitive
XMEMCMPchecks withConstantCompareacross several crypto paths. - Added
ForceZerocleanups for sensitive buffers (HPKE, ML-KEM, PBKDF, ECIES, ChaCha20-Poly1305). - Fixed an allocation macro using an incorrect
sizeof()type.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| wolfcrypt/src/wc_mlkem.c | Zeroizes encaps/decaps key-derivation material before returning. |
| wolfcrypt/src/srp.c | Uses constant-time proof comparison and clears proof digest after use. |
| wolfcrypt/src/sakke.c | Uses constant-time authentication point comparison. |
| wolfcrypt/src/pwdbased.c | Clears PBKDF intermediate digest/buffer before free/return. |
| wolfcrypt/src/pkcs12.c | Uses constant-time MAC comparison and returns a defined error on mismatch. |
| wolfcrypt/src/hpke.c | Zeroizes DH shared secret / KEM context / base context structures. |
| wolfcrypt/src/evp.c | Uses constant-time HMAC verification comparison. |
| wolfcrypt/src/ecc.c | Zeroizes ECIES shared secret/derived keys and uses constant-time HMAC tag compare. |
| wolfcrypt/src/chacha20_poly1305.c | Zeroizes Poly1305 auth key, including early-return paths. |
| wolfcrypt/src/asn.c | Fixes ALLOC_ASNSETDATA macro to allocate correct element size. |
| wolfcrypt/src/aes.c | Uses constant-time compares for KeyWrap IV and AES-SIV tag verification. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Has performance before and after these additions been considered? I suspect it is not a large impact but would like to know if this causes performance decrease with the additional ForceZero's and buffer compare changes. |
Honestly the performance impact is probably negligible the buffers are small 8-266 bytes. If you want I can run some comparison. |
aidangarske
force-pushed
the
fix-wolfcrypt-fenrir
branch
from
3eb9fd9 to
8c4ae79
Compare
aidangarske
force-pushed
the
fix-wolfcrypt-fenrir
branch
from
8c4ae79 to
f812c3a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
aidangarske
force-pushed
the
fix-wolfcrypt-fenrir
branch
from
f812c3a to
3295a65
Compare
|
Jenkins retest this please |
Yes please. Nothing crazy for benchmark comparison, but running the bundled benchmark application before and after and checking if there is a statistically significant impact on performance so that we are aware of it if there. |
comparing master and this PR I cannot see any measurable differences running out of 3 runs only thing to note was I was experiencing thermal throttling differences on my PI device but swapping order reveals same pattern of one faster than the other after concurrent runs |
No description provided.