Conversation
Frauschi
left a comment
There was a problem hiding this comment.
All three review comments addressed in commit b621705:
-
MlKemKey pointer (internal.c): Changed union member from embedded
MlKemKey mlKemKeytoMlKemKey* mlKemKey, matching the pattern used by ML-DSA, DH, RSA, and ECC. AddedXMALLOCin theNewObjectkey-type switch,wc_MlKemKey_Free+XFREE+ null-set inWP11_Object_Free, and updated all dereferences (&object->data.mlKemKey→object->data.mlKemKey,.field→->field). -
SHA-3 check in CMakeLists.txt: Added
if(NOT WOLFPKCS11_SHA3) override_cache(WOLFPKCS11_MLKEM "no") endif(), mirroring the existing ML-DSA guard. -
SHA-3 check in configure.ac: Added the same guard — if
ENABLED_SHA3isno, ML-KEM is disabled with a descriptive message, matching the ML-DSA pattern.
Frauschi
left a comment
There was a problem hiding this comment.
Also add mlkem support to the cmake and clang-tidy CI jobs
Adds support for ML-KEM (Kyber, FIPS 203) key encapsulation mechanism, following the same patterns established by the ML-DSA integration. Disabled by default; enable with --enable-mlkem (autotools) or -DWOLFPKCS11_MLKEM=yes (CMake). Enabling ML-KEM automatically enables PKCS#11 v3.2 support. Capabilities added: - Key generation (CKM_ML_KEM_KEY_PAIR_GEN) for KEM-512/768/1024 - Key import/export via C_CreateObject / C_GetAttributeValue - Token persistence (WOLFPKCS11_STORE_MLKEMKEY_PRIV/PUB, 0x0E/0x0F) - Encapsulation (C_EncapsulateKey / CKM_ML_KEM) - Decapsulation (C_DecapsulateKey / CKM_ML_KEM) - New PKCS#11 constants: CKK_ML_KEM, CKM_ML_KEM_KEY_PAIR_GEN, CKM_ML_KEM, CKA_ENCAPSULATE, CKA_DECAPSULATE, CKF_ENCAPSULATE, CKF_DECAPSULATE, CKP_ML_KEM_512/768/1024 - New internal flags: WP11_FLAG_ENCAPSULATE, WP11_FLAG_DECAPSULATE Tests added to tests/pkcs11v3test.c (inside WOLFPKCS11_MLKEM guard): - Key generation in session and with ID - Token key persistence round-trip - Export/reimport round-trip (exercises import path) - Encapsulate/decapsulate shared-secret equality check - Wrong-key implicit-rejection test
Replace the dynamically-allocated `byte* ctx` pointer in WP11_MldsaParams with an inline `byte ctx[256]` array. PKCS#11 v3.2 (§2.3.12) caps the ML-DSA context length at 255 bytes, so heap allocation is unnecessary and introduced several memory-management hazards: - ctx was freed at the end of WP11_Mldsa_Sign/Verify before session teardown, leaving a dangling pointer if the session was reused - the cleanup in wp11_Session_Final checked the wrong mechanism set, meaning it could free ctx a second time - WP11_Session_SetMldsaParams freed ctx before re-initialising, which was safe only if the pointer was always valid (it wasn't on first call) Embedding the buffer in the struct eliminates all manual lifetime tracking.
Summary
--enable-mlkem(autotools) or-DWOLFPKCS11_MLKEM=yes(CMake) — enabling auto-activates PKCS#11 v3.2mlkem.patch) against the older codebase: wrong length argument towp11_EncryptData, missingdevIdparameter, unchecked return value in store path,XMEMSET→wc_ForceZero,object->slot->devId→object->devId, store constant conflicts, and wrong internal flag valuesNew capabilities
CKM_ML_KEM_KEY_PAIR_GEN— key pair generation for KEM-512/768/1024C_EncapsulateKey/C_DecapsulateKeywithCKM_ML_KEMC_CreateObject/C_GetAttributeValueWOLFPKCS11_STORE_MLKEMKEY_PRIV= 0x0E,WOLFPKCS11_STORE_MLKEMKEY_PUB= 0x0F)CKK_ML_KEM,CKA_ENCAPSULATE,CKA_DECAPSULATE,CKF_ENCAPSULATE,CKF_DECAPSULATE,CKP_ML_KEM_512/768/1024Tests (
tests/pkcs11v3test.c, insideWOLFPKCS11_MLKEMguard)test_mlkem_gen_keys— generate key pair, encapsulate/decapsulate, verify shared secrets matchtest_mlkem_gen_keys_id— generate with ID, find by ID, encapsulate/decapsulatetest_mlkem_gen_keys_token— generate key pair to token storagetest_mlkem_token_keys— load persisted keys, encapsulate/decapsulatetest_mlkem_fixed_keys— generate, export raw key material, destroy, reimport, verify (exercises import path)test_mlkem_encap_decap_fail— encapsulate with key A, decapsulate with wrong key B, verify shared secrets differ (ML-KEM implicit rejection)Test plan
./configure --enable-mlkem && make && make checkpkcs11v3testshows all ML-KEM tests passingtest_mlkem_gen_keys_token, restart, runtest_mlkem_token_keysto confirm persistence./configure --disable-mlkem && make— verify clean build without ML-KEM🤖 Generated with Claude Code