[LinuxJedi]

#257 writes only the token objects that changed, and relies on storeDirty: a store pass that fails or is skipped makes the next pass write every object again. A custom store (WOLFPKCS11_CUSTOM_STORE) cannot set it when a record fails to commit, because wolfPKCS11_Store_Close() returns void. wolfBoot's TrustZone keyvault commits flash on close, so a failed flash commit leaves the record missing or stale while wolfPKCS11 carries on as if it were stored.

This declares wolfPKCS11_Store_CloseCommit(void* store, int commit) in store.h. The file and TPM stores already implement it (it was a static function). A custom store opts in by defining WOLFPKCS11_CUSTOM_STORE_CLOSE_COMMIT and implementing it. Written records are then closed through it, and a non-zero return fails the operation and marks the token for a full store. commit is 0 when writing the record failed, so a store that can discard the record may do so. Without the define, custom stores build and behave exactly as before, and wolfPKCS11_Store_Close() still closes records that were read.

Why a define: changing wolfPKCS11_Store_Close() to return int would break every existing custom store at compile time, and weak symbols are not portable to every compiler a custom store is built with. A store can define the macro and the function unconditionally: older wolfPKCS11 versions ignore the define and keep calling wolfPKCS11_Store_Close(). wolfSSL/wolfBoot#957 does this for the keyvault.

A custom store's failure code is now mapped to BUFFER_E on open, write, close and remove. Passed through, wolfBoot's -1 from a failed open or write reads as PIN_INVALID_E, so C_CreateObject returned CKR_USER_NOT_LOGGED_IN, and a -4 on close would read as storage being turned off. NOT_AVAILABLE_E is kept from an open for reading and from a remove, where it means the data is not stored. From an open for writing it fails the operation (store.h says so): it read as storage turned off, so the create returned CKR_OK with nothing stored, and wolfBoot's store returns it from a write open in one failure case. A custom store's read failure is mapped to WC_HW_E instead, because the object and token loaders take BUFFER_E (a short read) as the end of a record in an older format. Read failures of the TPM store keep their codes.

wp11_Object_Load_Data() and wp11_Object_Store_Data() now close the data record only when it opened, inside the open's success branch like the other record types. They passed NULL to the store's close when the open failed, and wolfBoot's store wrote through it, so creating a data object whose record could not be opened, or loading one whose record was missing, faulted. wolfSSL/wolfBoot#957 also makes wolfBoot's close ignore NULL.

New test tests/custom_store_test.c. It builds the library sources with a store kept in memory, twice: with the define (custom_store_test) and without it (custom_store_legacy_test, which checks that a store with only wolfPKCS11_Store_Close() still builds and keeps objects). The test store can fail an open, a read, a write, a commit or a remove, with a chosen code for opens for writing and commits. Both builds check that a record failing to open for writing, with -1 or -4, fails the create with CKR_FUNCTION_FAILED, that a failed remove fails C_DestroyObject, and that the store is never closed without an open (a data record fails to open on create, and one is removed before loading). They also fail each read and each open for reading that loading the token makes, in turn (105 reads and 9 opens with the define), and check that every load fails rather than loading a record without its later fields. With the define, it also checks that a failed commit, with -1 or -4, and a failed write fail the create with CKR_FUNCTION_FAILED, that the failed write closes the record with commit set to 0, that a failed commit fails C_DestroyObject with CKR_FUNCTION_FAILED, that the next create writes every object again and the one after only its own, and that the objects load back after C_Finalize. It fails on master at the failed open check. The tests skip without AES, where the library turns storage off, and are not built with --enable-nss, which needs the file store, or with -DWOLFPKCS11_TPM_STORE, which cannot be built with a custom store (BUILD_CUSTOM_STORE_TEST in configure.ac). They are built for make check only, and are also the first CI coverage of a custom store build.

Testing:

• make check passes with no failures in the default, --enable-debug, --enable-nss, --enable-static --disable-shared, C++ (CC=clang++) and C++ debug configs. The pre-commit hook make test passes.
• With Report failed flash commits from the PKCS#11 keyvault, check STM32U5 flash errors wolfBoot#957, the stm32h5-tz PKCS#11 test app passes in m33mu (as in the wolfBoot integration job) and wolfPKCS11 closes written records with wolfPKCS11_Store_CloseCommit(). The stm32u5-wolfcrypt-tz config builds. wolfBoot master with this change also passes, using the old close.
• A 2.1.0 backport of this change ran on an STM32U585 (TrustZone, stm32u5-wolfcrypt-tz.config) with Report failed flash commits from the PKCS#11 keyvault, check STM32U5 flash errors wolfBoot#957. Creating 8 token objects, reading them back and destroying them across reboots reported no failures. With the keyvault pages write-protected (WRP), each C_DestroyObject failed with CKR_FUNCTION_FAILED instead of returning CKR_OK, and once the protection was removed the objects were intact. Master itself was not run on hardware; its C_DestroyObject already maps the failure (2.1.0 needed that too).

ZD 21908

[LinuxJedi]

@claude review

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #265

Scan targets checked: wolfpkcs11-src, wolfpkcs11-bugs
Not reviewed (no scan target covers them): README.md, configure.ac, tests/README.md, tests/include.am
Coverage: 1 of 3 in-scope changed file(s) opened by the reviewer; not opened: tests/custom_store_test.c, wolfpkcs11/store.h

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: Lite

[claude]

Additional findings (outside the current diff — GitHub can't attach inline comments there):

• 🟣 src/internal.c — pre-existing: Custom stores whose open or read fails with their own code (e.g. wolfBoot's -1) still have it read as PIN_INVALID_E, so callers get CKR_USER_NOT_LOGGED_IN. This change maps store codes to BUFFER_E only on write and close. wp11_storage_open passes the store's code through unchanged, and so does wp11_storage_read's else if (ret == 0) branch. Fix: under WOLFPKCS11_CUSTOM_STORE, turn every non-zero open result except NOT_AVAILABLE_E into BUFFER_E, and every short read into BUFFER_E, which covers the 2 sites listed. Same pattern at 2 sites (src/internal.c:3261, src/internal.c:3358).

  Why this was flagged

  Trigger: a custom store's wolfPKCS11_Store_Open fails a write open (keyvault full, flash error) or its wolfPKCS11_Store_Read fails, returning -1 as wolfBoot does. The test store does this too (tests/custom_store_test.c:166, :208). wp11_storage_open (src/internal.c:3261) returns that -1 straight to writers such as wp11_Object_Store_Data (src/internal.c:5566), and from there it reaches C_CreateObject. wp11_storage_read (src/internal.c:3358) only maps the result to BUFFER_E when ret == 0, so a negative store code also passes through on load. -1 equals PIN_INVALID_E, so the caller gets CKR_USER_NOT_LOGGED_IN, which is the same misreport the PR fixes for write and close (src/internal.c:3436, :3302). Open failures that are NOT_AVAILABLE_E still need to pass through unchanged, because callers treat that code as 'storage off / not found'. The base behaves the same way, so this is a partial fix rather than a regression.

  Verification: Trigger: a custom store whose wolfPKCS11_Store_Open or wolfPKCS11_Store_Read fails with its own negative code, such as -1 (PIN_INVALID_E, wolfpkcs11/internal.h:386). wp11_storage_open (src/internal.c:3261) returns it unchanged, and wp11_storage_read (src/internal.c:3358) maps to BUFFER_E only else if (ret == 0). The base code fails the same way, so this is not a regression.

[LinuxJedi]

Alpine CI fixed in #266

[LinuxJedi]

Re the open/read finding in the review above: fixed in 9c01d2f. For a custom store, a failed open now maps to BUFFER_E (read-only and write opens), except NOT_AVAILABLE_E, which still means the data is not stored. A failed read maps to BUFFER_E as well. The read mapping applies only to custom stores, so the TPM store's read codes are unchanged. custom_store_test now fails a record's open for writing with -1 in both builds and expects CKR_FUNCTION_FAILED; that check fails without the change. On load, the loader already reported any failure as CKR_DEVICE_ERROR, so mapping reads there is for consistency.

[Frauschi]

The main concern is the custom store read-error mapping colliding with the older-format BUFFER_E sentinel in the loaders.

[Copilot]

🟡 Changes recommended

The configure.ac exclusion misses explicit-value definitions such as -DWOLFPKCS11_NSS=1 and -DWOLFPKCS11_TPM_STORE=1, allowing incompatible tests to run and causing make check to fail with undefined symbols.

1 open issue

• Exclude tests when store macros are defined with explicit values · New

What changed in this PR

Adds opt-in commit-status reporting for custom stores, extending the existing dirty-token recovery mechanism to handle failed storage commits while preserving the legacy close interface.

Changes:

• Exposes wolfPKCS11_Store_CloseCommit() and documents custom-store opt-in.
• Normalizes storage errors and avoids closing data records that failed to open.
• Adds in-memory custom-store tests for commit-aware and legacy implementations.

File	Description
wolfpkcs11/​store.h	Declares and documents the commit-status interface.
tests/​README.md	Describes the new custom-store tests.
tests/​include.am	Adds commit-aware and legacy test targets.
tests/​custom_store_test.c	Tests failure handling, recovery, and persistence.
src/​internal.c	Handles custom-store errors and guards record closure.
README.md	Documents the opt-in macro and commit-failure behavior.
configure.ac	Gates tests for incompatible storage configurations.

_{Review effort: Balanced}