Conversation
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #336
Scan targets checked: wolfhsm-consttime, wolfhsm-core-bugs, wolfhsm-crypto-bugs, wolfhsm-defaults, wolfhsm-mutation, wolfhsm-proptest, wolfhsm-src, wolfhsm-zeroize
Findings: 5
5 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
Implement full client-server ML-KEM (Module-Lattice-Based Key Encapsulation Mechanism) support across all wolfHSM layers, enabling post-quantum key exchange operations to be offloaded to the HSM. Client API (wh_client_crypto): - Key management: import, export, set/get key ID - Key generation: MakeExportKey (ephemeral) and MakeCacheKey (server-cached) - Encapsulation and decapsulation operations - DMA variants for all operations Server handling (wh_server_crypto): - Request handlers for ML-KEM keygen, encapsulate, and decapsulate - Auto-import with evict-after-use for uncached keys - DMA request handlers Crypto callback integration (wh_client_cryptocb): - Register PQC KEM keygen/encaps/decaps handlers so wolfCrypt ML-KEM calls are transparently forwarded to the HSM via WH_DEV_ID Message layer (wh_message_crypto): - Define request/response structures for keygen, encapsulate, decapsulate - Endian translation functions for cross-platform support Shared utilities (wh_crypto): - ML-KEM key serialization/deserialization with automatic level probing Supports all three ML-KEM parameter sets (512, 768, 1024). Includes tests for all operations and DMA paths, and benchmarks for keygen, encaps, and decaps at each security level. Also fixes key export response to use actual stored key length from NVM metadata instead of the request size.
bigbrett
left a comment
bigbrett
left a comment
There was a problem hiding this comment.
Quick request before diving too deep into the review - can you add the new message structs to the struct padding check test?
|
Added the new structures (and also missing ML-DSA ones) to the test. |
| return WH_ERROR_BADARGS; | ||
| } | ||
|
|
||
| buffer = (byte*)XMALLOC(allocSz, NULL, DYNAMIC_TYPE_TMP_BUFFER); |
There was a problem hiding this comment.
We currently have a policy of no dynamic allocations in the wolfHSM library. Obviously wolfCrypt does allocations internally, but we want to limit the allocation boundary to that. Can you make this a statically allocated stack buffer plz. Look at how ML-DSA does it
| return WH_ERROR_BADARGS; | ||
| } | ||
|
|
||
| buffer = (byte*)XMALLOC(WC_ML_KEM_MAX_PRIVATE_KEY_SIZE, NULL, |
4 participants
Implement full client-server ML-KEM support across all wolfHSM layers, enabling post-quantum key exchange operations to be offloaded to the HSM.
Client API (wh_client_crypto):
Server handling (wh_server_crypto):
Crypto callback integration (wh_client_cryptocb):
Message layer (wh_message_crypto):
Shared utilities (wh_crypto):
Supports all three ML-KEM parameter sets (512, 768, 1024). Includes tests for all operations and DMA paths, and benchmarks for keygen, encaps, and decaps at each security level.
Also fixes key export response to use actual stored key length from NVM metadata instead of the request size. This is required for the ML-KEM deserialization, as the exact size is needed (all other deserialization methods allow oversized buffers, as DER encoding is used; Hence, this bug never caused issues).