From ad2162aaf209abdd84ea0f933e6194bef35ac479 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Wed, 28 Oct 2026 14:29:52 +0000 Subject: [PATCH 1/4] Let custom stores report records that did not commit A custom store (WOLFPKCS11_CUSTOM_STORE) closes a written record with wolfPKCS11_Store_Close(), which returns void. A store that commits on close, such as wolfBoot's flash keyvault, cannot report a failed commit, so storeDirty stays clear and the next store does not write the records again. Declare wolfPKCS11_Store_CloseCommit() in store.h. The file and TPM stores already implement it. 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. Without the define, custom stores build and behave as before. wolfPKCS11_Store_Close() still closes records that were read. A custom store's failure code is now mapped to BUFFER_E, on close and on write. Passed through, wolfBoot's -1 from a failed write read 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. tests/custom_store_test.c builds the library sources with a store kept in memory, once with the define and once without. It checks that a failed commit or write fails the create, that a failed write closes the record with commit set to 0, and that the next create writes every object again, then that the objects load back after C_Finalize. It is not built for NSS, which needs the file store, or with WOLFPKCS11_TPM_STORE, which cannot be built with a custom store. --- README.md | 10 + configure.ac | 9 + src/internal.c | 17 +- tests/README.md | 3 + tests/custom_store_test.c | 516 ++++++++++++++++++++++++++++++++++++++ tests/include.am | 23 ++ wolfpkcs11/store.h | 17 ++ 7 files changed, 591 insertions(+), 4 deletions(-) create mode 100644 tests/custom_store_test.c diff --git a/README.md b/README.md index 849cdba2..84f42ba1 100644 --- a/README.md +++ b/README.md @@ -128,6 +128,16 @@ Enables debugging printf's for store. Removes default implementation of storage functions. See wolfpkcs11/store.h for prototypes of functions to implement. +#### Define WOLFPKCS11_CUSTOM_STORE_CLOSE_COMMIT + +With `WOLFPKCS11_CUSTOM_STORE`, records that were written are closed with +`wolfPKCS11_Store_CloseCommit()` instead of `wolfPKCS11_Store_Close()`, so the +custom store must implement it too (`wolfPKCS11_Store_Close()` still closes +records that were read). It returns non-zero when the record did not commit, +for example when a flash write failed. wolfPKCS11 then fails the operation and +writes all records of the token on its next store. Without this define, the +custom store cannot report a failed commit. + #### Define WOLFPKCS11_KEYPAIR_GEN_COMMON_LABEL Sets the private key's label against the public key when generating key pairs. diff --git a/configure.ac b/configure.ac index 569d38d7..561b33b2 100644 --- a/configure.ac +++ b/configure.ac @@ -664,6 +664,15 @@ AX_HARDEN_CC_COMPILER_FLAGS OPTION_FLAGS="$CFLAGS $CPPFLAGS $AM_CFLAGS" +# The custom store tests build the library with WOLFPKCS11_CUSTOM_STORE. NSS +# needs the file store, and a custom store cannot be built with the TPM store. +AS_CASE([" $OPTION_FLAGS "], + [*" -DWOLFPKCS11_NSS "*|*" -DWOLFPKCS11_TPM_STORE "*], + [ENABLED_CUSTOM_STORE_TEST=no], + [ENABLED_CUSTOM_STORE_TEST=yes]) +AM_CONDITIONAL([BUILD_CUSTOM_STORE_TEST], + [test "x$ENABLED_CUSTOM_STORE_TEST" = "xyes"]) + AS_IF([test "x$WOLFPKCS11_STATIC_WOLFSSL" = "xyes"], [WOLFPKCS11_CONFIG_LIBS="$LIBS -lm" LIBS="$STATIC_WOLFSSL $LIBS -lm"], diff --git a/src/internal.c b/src/internal.c index 8efcee80..bcc63a14 100644 --- a/src/internal.c +++ b/src/internal.c @@ -2999,7 +2999,7 @@ static int wolfPKCS11_Store_TpmCommit(WP11_TpmStore* tpmStore) * @return 0 on success. * @return Other value when written data could not be committed. */ -static int wolfPKCS11_Store_CloseCommit(void* store, int commit) +int wolfPKCS11_Store_CloseCommit(void* store, int commit) { int ret = 0; #ifdef WOLFPKCS11_TPM_STORE @@ -3289,11 +3289,19 @@ static void wp11_storage_close(void* storage) */ static int wp11_storage_close_write(void* storage, int ret) { -#ifdef WOLFPKCS11_CUSTOM_STORE +#if defined(WOLFPKCS11_CUSTOM_STORE) && \ + !defined(WOLFPKCS11_CUSTOM_STORE_CLOSE_COMMIT) + /* The store cannot report a failed commit. */ wolfPKCS11_Store_Close(storage); #else int closeRet = wolfPKCS11_Store_CloseCommit(storage, ret == 0); +#ifdef WOLFPKCS11_CUSTOM_STORE + /* Any failure means the data did not commit. A custom store's code could + * otherwise read as NOT_AVAILABLE_E, which means storage is turned off. */ + if (closeRet != 0) + closeRet = BUFFER_E; +#endif if (ret == 0) ret = closeRet; #endif @@ -3425,8 +3433,9 @@ static int wp11_storage_write(void* storage, unsigned char* buffer, int len) /* All written successfully. */ ret = 0; } - else if (ret == 0) { - /* Failed to write all data. */ + else { + /* Failed to write all data. A custom store's failure code could + * otherwise read as another status, such as PIN_INVALID_E. */ ret = BUFFER_E; } diff --git a/tests/README.md b/tests/README.md index 32fbe0be..d5af5493 100644 --- a/tests/README.md +++ b/tests/README.md @@ -5,3 +5,6 @@ The test files have the following purposes: * `pkcs11mtt.c`: is the multithreaded tests * `pkcs11str.c`: is a storage test * `debug_test.c`: is a debug functionality demonstration test +* `custom_store_test.c`: is a custom store (`WOLFPKCS11_CUSTOM_STORE`) test. It + builds the library sources in, with and without + `WOLFPKCS11_CUSTOM_STORE_CLOSE_COMMIT` diff --git a/tests/custom_store_test.c b/tests/custom_store_test.c new file mode 100644 index 00000000..80540357 --- /dev/null +++ b/tests/custom_store_test.c @@ -0,0 +1,516 @@ +/* custom_store_test.c + * + * Copyright (C) 2006-2026 wolfSSL Inc. + * + * This file is part of wolfPKCS11. + * + * wolfPKCS11 is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfPKCS11 is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + * + * The library sources are built into this test with WOLFPKCS11_CUSTOM_STORE, + * and the test implements the store in memory. It is built twice: + * - custom_store_test, with WOLFPKCS11_CUSTOM_STORE_CLOSE_COMMIT, checks that + * a record whose commit fails, or whose write fails, fails the create, that + * a failed write closes the record with commit set to 0, and that the next + * store writes the records of every object again, + * - custom_store_legacy_test, without it, checks that a store implementing + * only wolfPKCS11_Store_Close() still builds and keeps token objects. + */ + +#ifdef HAVE_CONFIG_H + #include +#endif + +#ifndef WOLFSSL_USER_SETTINGS + #include +#endif +#include +#include + +#ifndef WOLFPKCS11_USER_SETTINGS + #include +#endif +#include +#include + +#include +#include +#include + +#if !defined(WOLFPKCS11_NO_STORE) && !defined(WOLFPKCS11_TPM_STORE) +#define RUN_TEST +#endif + +/* Store status codes, as in wolfpkcs11/internal.h. */ +#define NOT_AVAILABLE_E -4 +/* Failure code of the store. The same value as PIN_INVALID_E, so a failed + * create only reports CKR_FUNCTION_FAILED when the code is not passed on. */ +#define STORE_FAIL_E -1 + +/* ---- Store kept in memory ---- */ + +#define STORE_RECORDS 64 + +typedef struct StoreRecord { + int used; + int type; + CK_ULONG id1; + CK_ULONG id2; + unsigned char* data; + int len; + int commits; /* Times the record was committed */ +} StoreRecord; + +typedef struct StoreHandle { + int read; + int type; + CK_ULONG id1; + CK_ULONG id2; + unsigned char* data; /* Record read, or data written */ + int len; + int pos; /* Read position */ +} StoreHandle; + +static StoreRecord records[STORE_RECORDS]; +static int commitCnt; /* Commits tried */ +static int writeCnt; /* Writes tried */ +static int discardCnt; /* Written records closed without commit */ +static int failCommitAt = -1; /* Commit to fail, counted by commitCnt */ +static int failWriteAt = -1; /* Write to fail, counted by writeCnt */ + +static StoreRecord* store_find(int type, CK_ULONG id1, CK_ULONG id2) +{ + int i; + + for (i = 0; i < STORE_RECORDS; i++) { + if (records[i].used && records[i].type == type && + records[i].id1 == id1 && records[i].id2 == id2) { + return &records[i]; + } + } + return NULL; +} + +static int store_commit(StoreHandle* handle) +{ + StoreRecord* rec = store_find(handle->type, handle->id1, handle->id2); + int i; + + for (i = 0; rec == NULL && i < STORE_RECORDS; i++) { + if (!records[i].used) { + rec = &records[i]; + XMEMSET(rec, 0, sizeof(*rec)); + rec->used = 1; + rec->type = handle->type; + rec->id1 = handle->id1; + rec->id2 = handle->id2; + } + } + if (rec == NULL) + return STORE_FAIL_E; + + free(rec->data); + rec->data = handle->data; + rec->len = handle->len; + rec->commits++; + handle->data = NULL; + return 0; +} + +static int store_close(void* store, int commit) +{ + StoreHandle* handle = (StoreHandle*)store; + int ret = 0; + + if (handle == NULL) + return 0; + if (!handle->read) { + if (!commit) { + discardCnt++; + } + else if (commitCnt++ == failCommitAt) { + /* As a failed flash commit: the record keeps its old data. */ + ret = STORE_FAIL_E; + } + else { + ret = store_commit(handle); + } + free(handle->data); + } + free(handle); + return ret; +} + +int wolfPKCS11_Store_Open(int type, CK_ULONG id1, CK_ULONG id2, int read, + void** store) +{ + StoreRecord* rec = store_find(type, id1, id2); + StoreHandle* handle; + + *store = NULL; + if (read && rec == NULL) + return NOT_AVAILABLE_E; + handle = (StoreHandle*)calloc(1, sizeof(*handle)); + if (handle == NULL) + return STORE_FAIL_E; + handle->read = read; + handle->type = type; + handle->id1 = id1; + handle->id2 = id2; + if (read) { + /* Reads use the record's data, which no write changes while open. */ + handle->data = rec->data; + handle->len = rec->len; + } + *store = handle; + return 0; +} + +int wolfPKCS11_Store_Remove(int type, CK_ULONG id1, CK_ULONG id2) +{ + StoreRecord* rec = store_find(type, id1, id2); + + if (rec == NULL) + return NOT_AVAILABLE_E; + free(rec->data); + XMEMSET(rec, 0, sizeof(*rec)); + return 0; +} + +void wolfPKCS11_Store_Close(void* store) +{ + (void)store_close(store, 1); +} + +#ifdef WOLFPKCS11_CUSTOM_STORE_CLOSE_COMMIT +int wolfPKCS11_Store_CloseCommit(void* store, int commit) +{ + return store_close(store, commit); +} +#endif + +int wolfPKCS11_Store_Read(void* store, unsigned char* buffer, int len) +{ + StoreHandle* handle = (StoreHandle*)store; + + if (handle == NULL || !handle->read || len < 0) + return STORE_FAIL_E; + if (len > handle->len - handle->pos) + len = handle->len - handle->pos; + if (len > 0) + XMEMCPY(buffer, handle->data + handle->pos, len); + handle->pos += len; + return len; +} + +int wolfPKCS11_Store_Write(void* store, unsigned char* buffer, int len) +{ + StoreHandle* handle = (StoreHandle*)store; + unsigned char* data; + + if (handle == NULL || handle->read || len < 0) + return STORE_FAIL_E; + if (writeCnt++ == failWriteAt) + return STORE_FAIL_E; + data = (unsigned char*)realloc(handle->data, handle->len + len + 1); + if (data == NULL) + return STORE_FAIL_E; + if (len > 0) + XMEMCPY(data + handle->len, buffer, len); + handle->data = data; + handle->len += len; + return len; +} + +#ifdef RUN_TEST + +#define CHECK(cond, msg) \ + do { \ + if (!(cond)) { \ + fprintf(stderr, "%s:%d - %s - FAIL\n", __FILE__, __LINE__, \ + msg); \ + return -1; \ + } \ + } \ + while (0) +#define CHECK_RV(rv, msg) \ + do { \ + if ((rv) != CKR_OK) { \ + fprintf(stderr, "%s:%d - %s: 0x%lx - FAIL\n", __FILE__, \ + __LINE__, msg, (unsigned long)(rv)); \ + return -1; \ + } \ + } \ + while (0) + +static CK_FUNCTION_LIST* funcList; +static CK_SLOT_ID slot; +static unsigned char tokenName[32] = "wolfpkcs11"; +static byte* soPin = (byte*)"password123456"; +static int soPinLen = 14; +static byte* userPin = (byte*)"wolfpkcs11-test"; +static int userPinLen = 15; + +static CK_BBOOL ckTrue = CK_TRUE; +static CK_BBOOL ckFalse = CK_FALSE; + +static void store_free(void) +{ + int i; + + for (i = 0; i < STORE_RECORDS; i++) + free(records[i].data); + XMEMSET(records, 0, sizeof(records)); +} + +/* Commits of the records of the object with the given store id. */ +static int object_commits(CK_ULONG id) +{ + int i; + int cnt = 0; + + for (i = 0; i < STORE_RECORDS; i++) { + if (records[i].used && records[i].type != WOLFPKCS11_STORE_TOKEN && + records[i].id2 == id) { + cnt += records[i].commits; + } + } + return cnt; +} + +static CK_RV pkcs11_init(void) +{ + CK_RV rv; + CK_C_INITIALIZE_ARGS args; + CK_SLOT_ID slotList[16]; + CK_ULONG slotCount = sizeof(slotList) / sizeof(slotList[0]); + + rv = C_GetFunctionList(&funcList); + if (rv != CKR_OK) + return rv; + + XMEMSET(&args, 0, sizeof(args)); + args.flags = CKF_OS_LOCKING_OK; + rv = funcList->C_Initialize(&args); + if (rv == CKR_OK) + rv = funcList->C_GetSlotList(CK_TRUE, slotList, &slotCount); + if (rv == CKR_OK && slotCount == 0) + rv = CKR_GENERAL_ERROR; + if (rv == CKR_OK) + slot = slotList[0]; + return rv; +} + +static CK_RV provision(void) +{ + CK_RV rv; + CK_SESSION_HANDLE session; + + rv = funcList->C_InitToken(slot, soPin, soPinLen, tokenName); + if (rv == CKR_OK) { + rv = funcList->C_OpenSession(slot, + CKF_SERIAL_SESSION | CKF_RW_SESSION, NULL, NULL, &session); + } + if (rv == CKR_OK) { + rv = funcList->C_Login(session, CKU_SO, soPin, soPinLen); + if (rv == CKR_OK) + rv = funcList->C_InitPIN(session, userPin, userPinLen); + (void)funcList->C_Logout(session); + (void)funcList->C_CloseSession(session); + } + return rv; +} + +static CK_RV open_user_session(CK_SESSION_HANDLE* session) +{ + CK_RV rv; + + rv = funcList->C_OpenSession(slot, CKF_SERIAL_SESSION | CKF_RW_SESSION, + NULL, NULL, session); + if (rv == CKR_OK) + rv = funcList->C_Login(*session, CKU_USER, userPin, userPinLen); + return rv; +} + +/* Create a token data object whose label and value are the name. */ +static CK_RV create_obj(CK_SESSION_HANDLE session, const char* name) +{ + CK_OBJECT_CLASS dataClass = CKO_DATA; + CK_OBJECT_HANDLE obj; + CK_ATTRIBUTE tmpl[] = { + { CKA_CLASS, &dataClass, sizeof(dataClass) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIVATE, &ckFalse, sizeof(ckFalse) }, + { CKA_LABEL, (void*)name, strlen(name) }, + { CKA_VALUE, (void*)name, strlen(name) }, + }; + + return funcList->C_CreateObject(session, tmpl, + sizeof(tmpl) / sizeof(tmpl[0]), &obj); +} + +/* Find the token object with the name as label, and check its value. + * Returns the number found, or -1 on a wrong value. */ +static int find_obj(CK_SESSION_HANDLE session, const char* name) +{ + CK_ATTRIBUTE tmpl[] = { + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_LABEL, (void*)name, strlen(name) }, + }; + CK_OBJECT_HANDLE found[4]; + CK_ULONG cnt = 0; + byte value[32]; + CK_ATTRIBUTE attr = { CKA_VALUE, value, sizeof(value) }; + + if (funcList->C_FindObjectsInit(session, tmpl, 2) != CKR_OK) + return -1; + if (funcList->C_FindObjects(session, found, 4, &cnt) != CKR_OK) + cnt = 0; + (void)funcList->C_FindObjectsFinal(session); + if (cnt > 0) { + if (funcList->C_GetAttributeValue(session, found[0], &attr, 1) != + CKR_OK) { + return -1; + } + if (attr.ulValueLen != strlen(name) || + XMEMCMP(value, name, attr.ulValueLen) != 0) { + return -1; + } + } + return (int)cnt; +} + +static int count_token_objects(CK_SESSION_HANDLE session) +{ + CK_ATTRIBUTE tmpl[] = { { CKA_TOKEN, &ckTrue, sizeof(ckTrue) } }; + CK_OBJECT_HANDLE found[16]; + CK_ULONG cnt = 0; + + if (funcList->C_FindObjectsInit(session, tmpl, 1) != CKR_OK) + return -1; + if (funcList->C_FindObjects(session, found, 16, &cnt) != CKR_OK) + cnt = 0; + (void)funcList->C_FindObjectsFinal(session); + return (int)cnt; +} + +static int custom_store_test(void) +{ + CK_RV rv; + CK_SESSION_HANDLE session; + /* Objects that exist at the end. */ + const char* kept[] = { "objA", "objB", "objD", "objE", "objG" }; + int keptCnt; + int commits; + int discards = 0; + int i; + + rv = pkcs11_init(); + CHECK_RV(rv, "Initialize"); + rv = provision(); + CHECK_RV(rv, "Provision token"); + rv = open_user_session(&session); + CHECK_RV(rv, "Open user session"); + + /* objA takes store id 0. Creating objB writes only objB and the token. */ + rv = create_obj(session, "objA"); + CHECK_RV(rv, "Create objA"); + commits = object_commits(0); + CHECK(commits > 0, "objA stored"); + rv = create_obj(session, "objB"); + CHECK_RV(rv, "Create objB"); + CHECK(object_commits(0) == commits, "Create leaves objA's records"); + keptCnt = 2; + +#ifdef WOLFPKCS11_CUSTOM_STORE_CLOSE_COMMIT + /* The first record of objC does not commit, so the create fails. */ + failCommitAt = commitCnt; + rv = create_obj(session, "objC"); + failCommitAt = -1; + CHECK(rv == CKR_FUNCTION_FAILED, "Failed commit fails the create"); + + /* The next create writes every object again, then only its own. */ + rv = create_obj(session, "objD"); + CHECK_RV(rv, "Create objD"); + CHECK(object_commits(0) > commits, "objA rewritten after failure"); + commits = object_commits(0); + rv = create_obj(session, "objE"); + CHECK_RV(rv, "Create objE"); + CHECK(object_commits(0) == commits, "objA not rewritten again"); + + /* A failed write fails the create, and the record is not committed. */ + failWriteAt = writeCnt; + rv = create_obj(session, "objF"); + failWriteAt = -1; + CHECK(rv == CKR_FUNCTION_FAILED, "Failed write fails the create"); + discards++; + CHECK(discardCnt == discards, "Record closed with commit of 0"); + + rv = create_obj(session, "objG"); + CHECK_RV(rv, "Create objG"); + CHECK(object_commits(0) > commits, "objA rewritten after failure"); + keptCnt = (int)(sizeof(kept) / sizeof(kept[0])); + + CHECK(find_obj(session, "objC") == 0, "Failed create not added"); + CHECK(find_obj(session, "objF") == 0, "Failed create not added"); +#endif + /* Records written without a failure are closed with commit of 1. */ + CHECK(discardCnt == discards, "No other record discarded"); + + (void)funcList->C_Logout(session); + (void)funcList->C_CloseSession(session); + (void)funcList->C_Finalize(NULL); + + /* The objects load back from the store. */ + rv = pkcs11_init(); + CHECK_RV(rv, "Initialize again"); + rv = open_user_session(&session); + CHECK_RV(rv, "Open user session again"); + CHECK(count_token_objects(session) == keptCnt, "Object count"); + for (i = 0; i < keptCnt; i++) + CHECK(find_obj(session, kept[i]) == 1, "Object loaded"); + (void)funcList->C_Logout(session); + (void)funcList->C_CloseSession(session); + (void)funcList->C_Finalize(NULL); + + return 0; +} + +#endif /* RUN_TEST */ + +int main(int argc, char* argv[]) +{ +#ifdef RUN_TEST + int ret; + + (void)argc; + (void)argv; + +#ifdef WOLFPKCS11_CUSTOM_STORE_CLOSE_COMMIT + printf("Custom store test (close with commit status)\n"); +#else + printf("Custom store test (close only)\n"); +#endif + ret = custom_store_test(); + store_free(); + if (ret == 0) + printf("All tests passed!\n"); + return (ret == 0) ? 0 : 1; +#else + (void)argc; + (void)argv; + printf("Test not built for this configuration\n"); + return 77; +#endif +} diff --git a/tests/include.am b/tests/include.am index 21098b8f..2469d806 100644 --- a/tests/include.am +++ b/tests/include.am @@ -408,6 +408,29 @@ noinst_PROGRAMS += tests/token_partial_store_test tests_token_partial_store_test_SOURCES = tests/token_partial_store_test.c tests_token_partial_store_test_LDADD = +# The custom store tests build the library sources in, with the store in the +# test. See BUILD_CUSTOM_STORE_TEST in configure.ac for when they are built. +if BUILD_CUSTOM_STORE_TEST +check_PROGRAMS += tests/custom_store_test +noinst_PROGRAMS += tests/custom_store_test +tests_custom_store_test_SOURCES = tests/custom_store_test.c \ + $(src_libwolfpkcs11_la_SOURCES) +tests_custom_store_test_CFLAGS = -DBUILDING_WOLFPKCS11 \ + -DWOLFPKCS11_CUSTOM_STORE \ + -DWOLFPKCS11_CUSTOM_STORE_CLOSE_COMMIT \ + $(AM_CFLAGS) +tests_custom_store_test_LDADD = + +check_PROGRAMS += tests/custom_store_legacy_test +noinst_PROGRAMS += tests/custom_store_legacy_test +tests_custom_store_legacy_test_SOURCES = tests/custom_store_test.c \ + $(src_libwolfpkcs11_la_SOURCES) +tests_custom_store_legacy_test_CFLAGS = -DBUILDING_WOLFPKCS11 \ + -DWOLFPKCS11_CUSTOM_STORE \ + $(AM_CFLAGS) +tests_custom_store_legacy_test_LDADD = +endif + if !BUILD_STATIC_WOLFSSL check_PROGRAMS += tests/token_store_test check_PROGRAMS += tests/logout_retire_test diff --git a/wolfpkcs11/store.h b/wolfpkcs11/store.h index 7852aacb..9d45c125 100644 --- a/wolfpkcs11/store.h +++ b/wolfpkcs11/store.h @@ -97,6 +97,23 @@ WP11_LOCAL int wolfPKCS11_Store_OpenSz(int type, CK_ULONG id1, CK_ULONG id2, int */ WP11_LOCAL void wolfPKCS11_Store_Close(void* store); +/* + * Closes access to location being written and reports whether the written + * data was committed. + * Any dynamic memory associated with the store is freed here. + * + * A custom store (WOLFPKCS11_CUSTOM_STORE) implements this only when + * WOLFPKCS11_CUSTOM_STORE_CLOSE_COMMIT is defined. Otherwise records are + * closed with wolfPKCS11_Store_Close() and are taken to be committed. + * + * @param [in] store Context for operation. + * @param [in] commit 1 to commit the written data. 0 when writing failed: + * the store discards the data when it can. + * @return 0 on success. + * @return Other value when the written data was not committed. + */ +WP11_LOCAL int wolfPKCS11_Store_CloseCommit(void* store, int commit); + /* * Reads a specific number of bytes into buffer. * From 9c01d2f340e62355511c0c5dc31a4657068d0a52 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Thu, 29 Oct 2026 12:19:46 +0000 Subject: [PATCH 2/4] Map custom store open and read failures, skip the test without AES A custom store's open and read failures still passed its own code through. wolfBoot's -1 from a failed open for writing reached C_CreateObject as PIN_INVALID_E, so it returned CKR_USER_NOT_LOGGED_IN. Map them to BUFFER_E as for write and close, keeping NOT_AVAILABLE_E from an open, which means the data is not stored. Read failures of the TPM store keep their codes. custom_store_test now fails a record's open for writing in both builds and expects CKR_FUNCTION_FAILED. It also skips when NO_AES is defined: the library then turns storage off, which the test cannot see without including internal.h, so it failed instead of skipping. --- src/internal.c | 40 +++++++++++++++++++++++++++++++++++++-- tests/custom_store_test.c | 17 ++++++++++++++++- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/src/internal.c b/src/internal.c index bcc63a14..88449f8a 100644 --- a/src/internal.c +++ b/src/internal.c @@ -3220,6 +3220,24 @@ int wolfPKCS11_Store_Write(void* store, unsigned char* buffer, int len) } #endif +#ifdef WOLFPKCS11_CUSTOM_STORE +/* + * Maps a custom store's failure to open to BUFFER_E. + * The store's own code could otherwise read as another status, such as + * PIN_INVALID_E. NOT_AVAILABLE_E is kept: the data is not stored. + * + * @param [in] ret Result of opening. + * @return 0 or NOT_AVAILABLE_E as passed in. + * @return BUFFER_E for any other failure. + */ +static int wp11_storage_open_ret(int ret) +{ + if (ret != 0 && ret != NOT_AVAILABLE_E) + ret = BUFFER_E; + return ret; +} +#endif + /* * Opens access to location to read token data. * @@ -3236,7 +3254,12 @@ int wolfPKCS11_Store_Write(void* store, unsigned char* buffer, int len) static int wp11_storage_open_readonly(int type, CK_ULONG id1, CK_ULONG id2, void** storage) { - return wolfPKCS11_Store_Open(type, id1, id2, 1, storage); + int ret = wolfPKCS11_Store_Open(type, id1, id2, 1, storage); + +#ifdef WOLFPKCS11_CUSTOM_STORE + ret = wp11_storage_open_ret(ret); +#endif + return ret; } /* @@ -3257,8 +3280,14 @@ static int wp11_storage_open(int type, CK_ULONG id1, CK_ULONG id2, #ifdef WOLFPKCS11_TPM_STORE return wolfPKCS11_Store_OpenSz(type, id1, id2, 0, variableSz, storage); #else + int ret; + (void)variableSz; /* not used */ - return wolfPKCS11_Store_Open(type, id1, id2, 0, storage); + ret = wolfPKCS11_Store_Open(type, id1, id2, 0, storage); +#ifdef WOLFPKCS11_CUSTOM_STORE + ret = wp11_storage_open_ret(ret); +#endif + return ret; #endif } @@ -3359,6 +3388,13 @@ static int wp11_storage_read(void* storage, unsigned char* buffer, int len) /* Failed to read all data. */ ret = BUFFER_E; } +#ifdef WOLFPKCS11_CUSTOM_STORE + else { + /* A custom store's failure code could otherwise read as another + * status, such as PIN_INVALID_E. */ + ret = BUFFER_E; + } +#endif return ret; } diff --git a/tests/custom_store_test.c b/tests/custom_store_test.c index 80540357..fbf2efc0 100644 --- a/tests/custom_store_test.c +++ b/tests/custom_store_test.c @@ -26,6 +26,8 @@ * store writes the records of every object again, * - custom_store_legacy_test, without it, checks that a store implementing * only wolfPKCS11_Store_Close() still builds and keeps token objects. + * Both check that a record that fails to open for writing fails the create + * with CKR_FUNCTION_FAILED, whatever code the store returns. */ #ifdef HAVE_CONFIG_H @@ -48,7 +50,9 @@ #include #include -#if !defined(WOLFPKCS11_NO_STORE) && !defined(WOLFPKCS11_TPM_STORE) +/* The library turns storage off without AES (wolfpkcs11/internal.h). */ +#if !defined(WOLFPKCS11_NO_STORE) && !defined(WOLFPKCS11_TPM_STORE) && \ + !defined(NO_AES) #define RUN_TEST #endif @@ -84,9 +88,11 @@ typedef struct StoreHandle { static StoreRecord records[STORE_RECORDS]; static int commitCnt; /* Commits tried */ +static int openCnt; /* Opens for writing tried */ static int writeCnt; /* Writes tried */ static int discardCnt; /* Written records closed without commit */ static int failCommitAt = -1; /* Commit to fail, counted by commitCnt */ +static int failOpenAt = -1; /* Open to fail, counted by openCnt */ static int failWriteAt = -1; /* Write to fail, counted by writeCnt */ static StoreRecord* store_find(int type, CK_ULONG id1, CK_ULONG id2) @@ -161,6 +167,8 @@ int wolfPKCS11_Store_Open(int type, CK_ULONG id1, CK_ULONG id2, int read, *store = NULL; if (read && rec == NULL) return NOT_AVAILABLE_E; + if (!read && openCnt++ == failOpenAt) + return STORE_FAIL_E; handle = (StoreHandle*)calloc(1, sizeof(*handle)); if (handle == NULL) return STORE_FAIL_E; @@ -433,6 +441,13 @@ static int custom_store_test(void) CHECK(object_commits(0) == commits, "Create leaves objA's records"); keptCnt = 2; + /* objX's first record does not open for writing, so the create fails. */ + failOpenAt = openCnt; + rv = create_obj(session, "objX"); + failOpenAt = -1; + CHECK(rv == CKR_FUNCTION_FAILED, "Failed open fails the create"); + CHECK(find_obj(session, "objX") == 0, "Failed create not added"); + #ifdef WOLFPKCS11_CUSTOM_STORE_CLOSE_COMMIT /* The first record of objC does not commit, so the create fails. */ failCommitAt = commitCnt; From 05a46d6aa2ecf3792f59dc730f7c735d415a0a71 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Thu, 29 Oct 2026 14:55:43 +0000 Subject: [PATCH 3/4] Close a data record only when it opened wp11_Object_Load_Data() and wp11_Object_Store_Data() closed the record even when opening it had failed, so a custom store's close got NULL. wolfBoot's store writes through the handle, so creating a data object whose record cannot be opened, or loading one whose record is missing, would fault. Records of the other types are closed only once they have opened. custom_store_test now fails the open of a data record on create, removes one before loading the token, and checks that the store is never closed without an open. --- src/internal.c | 8 ++++++-- tests/custom_store_test.c | 41 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/internal.c b/src/internal.c index 88449f8a..4d08ca38 100644 --- a/src/internal.c +++ b/src/internal.c @@ -5052,7 +5052,9 @@ static int wp11_Object_Load_Data(WP11_Object* object, int tokenId, int objId) object->data.genericData.objectIdLen = (word32)tempLen; } - wp11_storage_close(storage); + /* Nothing to close when the open failed. */ + if (storage != NULL) + wp11_storage_close(storage); return ret; } @@ -5622,7 +5624,9 @@ static int wp11_Object_Store_Data(WP11_Object* object, int tokenId, int objId) object->data.genericData.objectIdLen); } - ret = wp11_storage_close_write(storage, ret); + /* Nothing to close when the open failed. */ + if (storage != NULL) + ret = wp11_storage_close_write(storage, ret); return ret; } diff --git a/tests/custom_store_test.c b/tests/custom_store_test.c index fbf2efc0..280e42d3 100644 --- a/tests/custom_store_test.c +++ b/tests/custom_store_test.c @@ -27,7 +27,8 @@ * - custom_store_legacy_test, without it, checks that a store implementing * only wolfPKCS11_Store_Close() still builds and keeps token objects. * Both check that a record that fails to open for writing fails the create - * with CKR_FUNCTION_FAILED, whatever code the store returns. + * with CKR_FUNCTION_FAILED, whatever code the store returns, and that a record + * that fails to open, for writing or reading, is not closed. */ #ifdef HAVE_CONFIG_H @@ -91,6 +92,7 @@ static int commitCnt; /* Commits tried */ static int openCnt; /* Opens for writing tried */ static int writeCnt; /* Writes tried */ static int discardCnt; /* Written records closed without commit */ +static int nullCloseCnt; /* Closes of a store that was not opened */ static int failCommitAt = -1; /* Commit to fail, counted by commitCnt */ static int failOpenAt = -1; /* Open to fail, counted by openCnt */ static int failWriteAt = -1; /* Write to fail, counted by writeCnt */ @@ -139,8 +141,10 @@ static int store_close(void* store, int commit) StoreHandle* handle = (StoreHandle*)store; int ret = 0; - if (handle == NULL) + if (handle == NULL) { + nullCloseCnt++; return 0; + } if (!handle->read) { if (!commit) { discardCnt++; @@ -282,6 +286,23 @@ static void store_free(void) XMEMSET(records, 0, sizeof(records)); } +/* Remove the record of the given type for the object with the given store id. + * Returns 1 when removed. */ +static int store_drop(int type, CK_ULONG id) +{ + int i; + + for (i = 0; i < STORE_RECORDS; i++) { + if (records[i].used && records[i].type == type && + records[i].id2 == id) { + free(records[i].data); + XMEMSET(&records[i], 0, sizeof(records[i])); + return 1; + } + } + return 0; +} + /* Commits of the records of the object with the given store id. */ static int object_commits(CK_ULONG id) { @@ -448,6 +469,14 @@ static int custom_store_test(void) CHECK(rv == CKR_FUNCTION_FAILED, "Failed open fails the create"); CHECK(find_obj(session, "objX") == 0, "Failed create not added"); + /* objY's object record is written, then its data record does not open. */ + failOpenAt = openCnt + 1; + rv = create_obj(session, "objY"); + failOpenAt = -1; + CHECK(rv == CKR_FUNCTION_FAILED, "Failed open fails the create"); + CHECK(find_obj(session, "objY") == 0, "Failed create not added"); + CHECK(nullCloseCnt == 0, "Record that did not open not closed"); + #ifdef WOLFPKCS11_CUSTOM_STORE_CLOSE_COMMIT /* The first record of objC does not commit, so the create fails. */ failCommitAt = commitCnt; @@ -499,6 +528,14 @@ static int custom_store_test(void) (void)funcList->C_CloseSession(session); (void)funcList->C_Finalize(NULL); + /* A data record that is missing does not open for reading. Loading the + * token fails, and the record is not closed. */ + CHECK(store_drop(WOLFPKCS11_STORE_DATA, 0), "Data record removed"); + rv = pkcs11_init(); + if (rv == CKR_OK) + (void)funcList->C_Finalize(NULL); + CHECK(nullCloseCnt == 0, "Record that did not open not closed"); + return 0; } From 066836f2c15b9e915cb874a6f94f6a889a62cb79 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Thu, 29 Oct 2026 15:17:11 +0000 Subject: [PATCH 4/4] Check that a failed commit fails a destroy With WOLFPKCS11_CUSTOM_STORE_CLOSE_COMMIT, custom_store_test now fails the first commit of a C_DestroyObject. It expects CKR_FUNCTION_FAILED, the object gone, and the token written in full by C_Finalize, so the object does not load back. Master already maps the failure. wolfPKCS11 2.1.0 returned the internal error code, as a board run of a backport of this change showed. --- tests/custom_store_test.c | 40 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/tests/custom_store_test.c b/tests/custom_store_test.c index 280e42d3..e556b974 100644 --- a/tests/custom_store_test.c +++ b/tests/custom_store_test.c @@ -22,8 +22,9 @@ * and the test implements the store in memory. It is built twice: * - custom_store_test, with WOLFPKCS11_CUSTOM_STORE_CLOSE_COMMIT, checks that * a record whose commit fails, or whose write fails, fails the create, that - * a failed write closes the record with commit set to 0, and that the next - * store writes the records of every object again, + * a failed write closes the record with commit set to 0, that a failed + * commit fails a destroy, and that the next store writes the records of + * every object again, * - custom_store_legacy_test, without it, checks that a store implementing * only wolfPKCS11_Store_Close() still builds and keeps token objects. * Both check that a record that fails to open for writing fails the create @@ -389,6 +390,31 @@ static CK_RV create_obj(CK_SESSION_HANDLE session, const char* name) sizeof(tmpl) / sizeof(tmpl[0]), &obj); } +#ifdef WOLFPKCS11_CUSTOM_STORE_CLOSE_COMMIT +/* Destroy the token object with the name as label. */ +static CK_RV destroy_obj(CK_SESSION_HANDLE session, const char* name) +{ + CK_ATTRIBUTE tmpl[] = { + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_LABEL, (void*)name, strlen(name) }, + }; + CK_OBJECT_HANDLE obj; + CK_ULONG cnt = 0; + CK_RV rv; + + rv = funcList->C_FindObjectsInit(session, tmpl, 2); + if (rv == CKR_OK) { + rv = funcList->C_FindObjects(session, &obj, 1, &cnt); + (void)funcList->C_FindObjectsFinal(session); + } + if (rv == CKR_OK && cnt != 1) + rv = CKR_OBJECT_HANDLE_INVALID; + if (rv == CKR_OK) + rv = funcList->C_DestroyObject(session, obj); + return rv; +} +#endif + /* Find the token object with the name as label, and check its value. * Returns the number found, or -1 on a wrong value. */ static int find_obj(CK_SESSION_HANDLE session, const char* name) @@ -439,7 +465,7 @@ static int custom_store_test(void) CK_RV rv; CK_SESSION_HANDLE session; /* Objects that exist at the end. */ - const char* kept[] = { "objA", "objB", "objD", "objE", "objG" }; + const char* kept[] = { "objA", "objB", "objD", "objG" }; int keptCnt; int commits; int discards = 0; @@ -508,6 +534,14 @@ static int custom_store_test(void) CHECK(find_obj(session, "objC") == 0, "Failed create not added"); CHECK(find_obj(session, "objF") == 0, "Failed create not added"); + + /* A destroy whose commit fails reports it. The object is gone, and the + * token is written in full when finalized. */ + failCommitAt = commitCnt; + rv = destroy_obj(session, "objE"); + failCommitAt = -1; + CHECK(rv == CKR_FUNCTION_FAILED, "Failed commit fails the destroy"); + CHECK(find_obj(session, "objE") == 0, "Destroyed object removed"); #endif /* Records written without a failure are closed with commit of 1. */ CHECK(discardCnt == discards, "No other record discarded");