Repository navigation
Fix the no-AES-CBC, no-store and NSS store build configurations - #295
Conversation
The WOLFSSL_HAVE_PRF block in internal.c, holding MechToMac and the TLS 1.2 master key derive functions, sat inside the AES-CBC section. A --disable-aescbc build against a wolfSSL with the TLS PRF failed to compile, because the TLS MAC code calls MechToMac whatever the AES configuration. The block uses no AES, so move it ahead of the AES section. Replace the CI TODO for --disable-aescbc with a job. The no_derive_secret_key job also disables AES-CBC, but builds against a wolfSSL without the PRF, so it never reached this code.
A build with -DWOLFPKCS11_NO_STORE and --enable-mlkem, --enable-lms or --enable-xmss did not compile. WP11_Object_Free and the LMS and XMSS public-key setters use object->encoded and object->keyData, which only exist with a store. The LMS and XMSS import, verify and attribute code also sat in the store-only part of internal.c, so its callers saw undeclared functions. Keep only the LMS and XMSS encode, decode, load and store functions in the store-only part and guard the store fields. Without a store a key is never encoded, so it is always freed. Add a CI job that builds all post-quantum algorithms without a store.
With WOLFPKCS11_NO_STORE, a debug build failed because the WP11_Slot_TokenKeyIsZero test hook reads the token key, which only exists with a store. Its tests already skip builds without a store, so declare and build the hook only with one. NSS builds used the file store's storeDir under conditions that did not exclude the other stores. Without a store or with WOLFPKCS11_CUSTOM_STORE, storeDirLock was defined but unused. With WOLFPKCS11_TPM_STORE, WP11_Library_Final freed storeDir, which does not exist there. With either of them, C_Initialize called WP11_SetStoreDir, which only the file store defines. Define WP11_NSS_STORE_DIR for NSS with the file store and guard storeDir, its lock, the free, WP11_SetStoreDir and its caller with it. test_private_object_access expects a private token object to be hidden from a public session after logout. NSS mode lets public sessions find private objects, and with a store the object is skipped only because logout encodes it. Without a store it stays decoded, so for NSS without a store the test checks that the object is found instead. Add CI jobs for debug and NSS builds without a store.
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are recorded for the build-configuration fixes and added CI coverage.
No open issues 🎉
What changed in this PR
This PR fixes wolfPKCS11 build configurations with AES-CBC disabled, storage disabled, or NSS using non-file storage, and adds CI coverage to prevent regressions.
Changes:
- Separates TLS PRF code from AES-CBC guards and makes post-quantum key handling compatible with no-store builds.
- Restricts NSS store-directory handling and the token-key debug hook to applicable storage configurations.
- Adds four CI jobs and adjusts the private-object access test for NSS without storage.
| File | Description |
|---|---|
| wolfpkcs11/internal.h | Defines the NSS file-store guard and guards storage-dependent declarations. |
| tests/pkcs11test.c | Adjusts private-object lookup expectations for NSS no-store builds. |
| src/wolfpkcs11.c | Limits NSS config-directory parsing to file-store builds. |
| src/internal.c | Corrects conditional compilation for TLS PRF, post-quantum keys, and storage helpers. |
| .github/workflows/unit-test.yml | Adds no-AES-CBC and three no-store CI configurations. |
Review effort: Balanced
|
@claude review |
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #295
Scan targets checked: wolfpkcs11-src, wolfpkcs11-bugs
Not reviewed (no scan target covers them): .github/workflows/unit-test.yml
Coverage: 4 of 4 in-scope changed file(s) opened by the reviewer
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Max
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and found no bugs. It changes key-free paths and an access-control test expectation in a PKCS#11 crypto library, so a human look is still worthwhile.
What was reviewed:
- The moved and added
WOLFPKCS11_NO_STOREguards in src/internal.c, checked for #if/#endif balance around the ML-KEM, LMS/HSS, XMSS and symmetric stored-record helpers. - The PQC free paths in
WP11_Object_FreeandSetHssKey/SetXmssKey: under NO_STORE,encodednever exists, so thewc_*_Freecall runs every time. This is consistent and causes no double free. WP11_NSS_STORE_DIRnow gatesstoreDir,storeDirLock,WP11_SetStoreDirand configdir parsing the same way in all of them. The AES-CBC helpers moved below the PRF block without being changed.- No build or test was run, so the four new CI configurations were not compiled here.
Extended reasoning...
The change rearranges preprocessor guards in src/internal.c, src/wolfpkcs11.c and wolfpkcs11/internal.h, adds four CI build jobs, and changes one test. The guards affect key ownership and free logic for ML-KEM, LMS and XMSS objects. The test now treats it as expected that NSS+NO_STORE builds let a public session find a private object, which is an access-control behavior a maintainer should confirm is intended. On NSS builds with CUSTOM_STORE or TPM_STORE, C_Initialize now ignores configdir without any error. The change is in a crypto library and alters security-relevant test expectations, so deferring was chosen over approving.
Fix four build configurations that fail to compile on master, and add CI jobs so they stay fixed.
Changes
MechToMac, TLS 1.2 master key derive) out of the AES-CBC section so--disable-aescbcbuilds against a wolfSSL with the PRF.WOLFPKCS11_NO_STORE: guard the store-only object fields and keep the LMS/XMSS import, verify and attribute code outside the store-only block.WP11_Slot_TokenKeyIsZero(debug test hook) only with a store, since it reads the token key.WP11_NSS_STORE_DIRfor NSS with the file store and use it forstoreDir,storeDirLock,WP11_SetStoreDirand the configdir parsing inC_Initialize. This fixes NSS builds without a store, withWOLFPKCS11_CUSTOM_STOREand withWOLFPKCS11_TPM_STORE.Testing
unit-test.ymljobs:no_aescbc,no_keystore_pqc,no_keystore_debug,no_keystore_nss; the--disable-aescbcTODO is removed.test_private_object_access: NSS lets public sessions find private objects, and with a store logout hides them only by encoding them. Without a store the test now checks that NSS finds the object after logout.