From ada5acad138cc4cd41039947fd9bb6b2a4a1bf7b Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 6 Oct 2026 09:57:04 -0700 Subject: [PATCH 01/28] F-4262 - Reject negative stored array lengths when loading token objects --- src/internal.c | 6 + tests/include.am | 7 + tests/object_store_decode_test.c | 436 +++++++++++++++++++++++++++++++ 3 files changed, 449 insertions(+) create mode 100644 tests/object_store_decode_test.c diff --git a/src/internal.c b/src/internal.c index 7c61a3f6..db6e1ad0 100644 --- a/src/internal.c +++ b/src/internal.c @@ -2462,6 +2462,7 @@ static int wp11_storage_write_array(void* storage, * @param [in] len Length of data to read. * @return 0 on success. * @return BUFFER_E to indicate failure. + * @return ASN_PARSE_E when the stored length is negative. * @return MEMORY_E when dynamic memory allocation fails. */ static int wp11_storage_read_alloc_array(void* storage, @@ -2471,6 +2472,11 @@ static int wp11_storage_read_alloc_array(void* storage, /* Read length of array. */ ret = wp11_storage_read_int(storage, len); + if (ret == 0 && *len < 0) { + *len = 0; + /* Not BUFFER_E, which callers accept as an older record's end. */ + ret = ASN_PARSE_E; + } if (ret == 0 && *len > 0) { /* Allocate buffer to hold data. */ *buffer = (unsigned char*)XMALLOC(*len, NULL, DYNAMIC_TYPE_TMP_BUFFER); diff --git a/tests/include.am b/tests/include.am index 584136e7..ed95d9e3 100644 --- a/tests/include.am +++ b/tests/include.am @@ -320,6 +320,11 @@ noinst_PROGRAMS += tests/login_pin_state_test tests_login_pin_state_test_SOURCES = tests/login_pin_state_test.c tests_login_pin_state_test_LDADD = +check_PROGRAMS += tests/object_store_decode_test +noinst_PROGRAMS += tests/object_store_decode_test +tests_object_store_decode_test_SOURCES = tests/object_store_decode_test.c +tests_object_store_decode_test_LDADD = + if BUILD_STATIC tests_pkcs11test_LDADD += src/libwolfpkcs11.la tests_pkcs11mtt_LDADD += src/libwolfpkcs11.la @@ -385,6 +390,7 @@ tests_set_pin_rekey_test_LDADD += src/libwolfpkcs11.la tests_nss_configdir_prefix_test_LDADD += src/libwolfpkcs11.la tests_logout_retire_test_LDADD += src/libwolfpkcs11.la tests_login_pin_state_test_LDADD += src/libwolfpkcs11.la +tests_object_store_decode_test_LDADD += src/libwolfpkcs11.la else tests_object_id_uniqueness_test_LDADD += src/libwolfpkcs11.la tests_empty_pin_store_test_LDADD += src/libwolfpkcs11.la @@ -442,6 +448,7 @@ tests_set_pin_rekey_test_LDADD += src/libwolfpkcs11.la tests_nss_configdir_prefix_test_LDADD += src/libwolfpkcs11.la tests_logout_retire_test_LDADD += src/libwolfpkcs11.la tests_login_pin_state_test_LDADD += src/libwolfpkcs11.la +tests_object_store_decode_test_LDADD += src/libwolfpkcs11.la endif EXTRA_DIST += tests/unit.h \ diff --git a/tests/object_store_decode_test.c b/tests/object_store_decode_test.c new file mode 100644 index 00000000..b240c46f --- /dev/null +++ b/tests/object_store_decode_test.c @@ -0,0 +1,436 @@ +/* object_store_decode_test.c + * + * Copyright (C) 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 + * + * Stored token objects whose recorded lengths do not describe valid data must + * be rejected cleanly when the token is loaded or logged into. Each reload runs + * in a child process so a failure is reported instead of ending the test. + */ + +#ifdef HAVE_CONFIG_H + #include +#endif + +#include +#include +#include + +#ifndef WOLFSSL_USER_SETTINGS + #include +#endif +#include +#include + +#ifndef WOLFPKCS11_USER_SETTINGS + #include +#endif +#include + +#ifndef HAVE_PKCS11_STATIC + #include +#endif + +#include "testdata.h" +#include "pkcs11_test_util.h" + +#if !defined(WOLFPKCS11_NO_STORE) && !defined(WOLFPKCS11_TPM_STORE) && \ + !defined(WOLFPKCS11_CUSTOM_STORE) && !defined(WOLFPKCS11_NO_ENV) && \ + !defined(_WIN32) + +#include +#include +#include + +#define TEST_DIR "./store/object_store_decode_test" +#define MAX_OBJ_ID 8 +#define MAX_FILE_SZ 16384 + +/* Offset of the first variable-length attribute in a stored object record. */ +#define OBJ_ID_LEN_OFF (12 + 3 * sizeof(CK_ULONG) + 1 + 1 + 4 + 4 + 8 + 8) +#define OBJ_LABEL_LEN_OFF (OBJ_ID_LEN_OFF + 4) +#define OBJ_ISSUER_LEN_OFF (OBJ_LABEL_LEN_OFF + 4) + +#define CHILD_PASS 0 +#define CHILD_FAIL 1 +#define CHILD_SETUP 2 + +static byte soPin[] = "password123456"; +static byte userPin[] = "wolfpkcs11-test"; +static CK_BBOOL ckTrue = CK_TRUE; +static CK_BBOOL ckFalse = CK_FALSE; + +static const char* storeKinds[] = { + "obj", "data", "symmkey", "rsakey_priv", "rsakey_pub", "ecckey_priv", + "ecckey_pub", "dhkey_priv", "dhkey_pub", "cert", "trust" +}; + +static void store_path(char* path, size_t sz, const char* kind, int objId) +{ + if (objId < 0) { + (void)snprintf(path, sz, "%s/wp11_%s_%016lx", TEST_DIR, kind, 1UL); + } + else { + (void)snprintf(path, sz, "%s/wp11_%s_%016lx_%016lx", TEST_DIR, kind, + 1UL, (unsigned long)objId); + } +} + +static void cleanup_test_files(void) +{ + char path[256]; + size_t k; + int i; + + for (k = 0; k < sizeof(storeKinds) / sizeof(*storeKinds); k++) { + for (i = 0; i < MAX_OBJ_ID; i++) { + store_path(path, sizeof(path), storeKinds[k], i); + (void)remove(path); + } + } + store_path(path, sizeof(path), "token", -1); + (void)remove(path); +} + +/* Locate the stored file of the given kind for the single object of that kind. */ +static int find_store_file(const char* kind, char* path, size_t sz) +{ + FILE* f; + int i; + + for (i = 0; i < MAX_OBJ_ID; i++) { + store_path(path, sz, kind, i); + f = fopen(path, "rb"); + if (f != NULL) { + fclose(f); + return 0; + } + } + return -1; +} + +static int read_file(const char* path, byte* data, size_t sz, size_t* readSz) +{ + FILE* f = fopen(path, "rb"); + + if (f == NULL) + return -1; + *readSz = fread(data, 1, sz, f); + if (ferror(f) || !feof(f)) { + fclose(f); + return -1; + } + fclose(f); + return 0; +} + +static int write_file(const char* path, const byte* data, size_t sz) +{ + FILE* f = fopen(path, "wb"); + size_t written; + + if (f == NULL) + return -1; + written = fwrite(data, 1, sz, f); + if (fclose(f) != 0 || written != sz) + return -1; + return 0; +} + +static word32 get_be32(const byte* p) +{ + return ((word32)p[0] << 24) | ((word32)p[1] << 16) | + ((word32)p[2] << 8) | (word32)p[3]; +} + +static void put_be32(byte* p, word32 v) +{ + p[0] = (byte)(v >> 24); + p[1] = (byte)(v >> 16); + p[2] = (byte)(v >> 8); + p[3] = (byte)v; +} + +/* Overwrite a 4-byte length field that is expected to hold 'expect'. */ +static int patch_length(const char* path, size_t off, word32 expect, + word32 value) +{ + static byte data[MAX_FILE_SZ]; + size_t sz = 0; + + if (read_file(path, data, sizeof(data), &sz) != 0 || sz < off + 4) + return -1; + if (get_be32(data + off) != expect) + return -1; + put_be32(data + off, value); + return write_file(path, data, sz); +} + +static CK_RV init_slot(CK_SLOT_ID* slot) +{ + CK_RV rv; + CK_C_INITIALIZE_ARGS args; + CK_SLOT_ID slots[16]; + CK_ULONG count = sizeof(slots) / sizeof(*slots); + + 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, slots, &count); + if (rv == CKR_OK && count == 0) + rv = CKR_TOKEN_NOT_PRESENT; + if (rv == CKR_OK) + *slot = slots[0]; + return rv; +} + +static CK_RV provision_token(CK_SLOT_ID slot) +{ + CK_RV rv; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_UTF8CHAR label[32]; + CK_FLAGS flags = CKF_SERIAL_SESSION | CKF_RW_SESSION; + + XMEMSET(label, ' ', sizeof(label)); + XMEMCPY(label, "wolfpkcs11", 10); + rv = funcList->C_InitToken(slot, soPin, sizeof(soPin) - 1, label); + if (rv == CKR_OK) + rv = funcList->C_OpenSession(slot, flags, NULL, NULL, &session); + if (rv == CKR_OK) + rv = funcList->C_Login(session, CKU_SO, soPin, sizeof(soPin) - 1); + if (rv == CKR_OK) + rv = funcList->C_InitPIN(session, userPin, sizeof(userPin) - 1); + if (session != CK_INVALID_HANDLE) { + (void)funcList->C_Logout(session); + (void)funcList->C_CloseSession(session); + } + return rv; +} + +static CK_RV open_session(CK_SLOT_ID slot, int login, + CK_SESSION_HANDLE* session) +{ + CK_RV rv; + CK_FLAGS flags = CKF_SERIAL_SESSION | CKF_RW_SESSION; + + rv = funcList->C_OpenSession(slot, flags, NULL, NULL, session); + if (rv == CKR_OK && login) { + rv = funcList->C_Login(*session, CKU_USER, userPin, + sizeof(userPin) - 1); + } + return rv; +} + +typedef CK_RV (*create_fn)(CK_SESSION_HANDLE session); + +/* Provision a fresh token, create objects with 'create' and persist them. */ +static CK_RV store_objects(create_fn create) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + + cleanup_test_files(); + rv = pkcs11_load(); + if (rv != CKR_OK) + return rv; + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = provision_token(slot); + if (rv == CKR_OK) + rv = open_session(slot, 1, &session); + if (rv == CKR_OK) + rv = create(session); + if (session != CK_INVALID_HANDLE) + (void)funcList->C_CloseSession(session); + (void)funcList->C_Finalize(NULL); + pkcs11_unload(); + return rv; +} + +typedef int (*child_fn)(void); + +/* Run fn in a child process. Returns CHILD_PASS only when it exits normally + * reporting that the invariant held. */ +static int run_in_child(child_fn fn) +{ + pid_t pid; + int status = 0; + + fflush(stdout); + fflush(stderr); + pid = fork(); + if (pid < 0) + return CHILD_SETUP; + if (pid == 0) { + int res = fn(); + fflush(stdout); + fflush(stderr); + _exit(res); + } + if (waitpid(pid, &status, 0) != pid) + return CHILD_SETUP; + if (WIFSIGNALED(status)) { + fprintf(stderr, " reload terminated by signal %d\n", + WTERMSIG(status)); + return CHILD_FAIL; + } + if (!WIFEXITED(status)) + return CHILD_FAIL; + return WEXITSTATUS(status); +} + +static CK_RV create_data_object(CK_SESSION_HANDLE session) +{ + CK_OBJECT_CLASS dataClass = CKO_DATA; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + static byte app[] = "wolfpkcs11"; + static byte value[] = "stored data object value"; + CK_ATTRIBUTE tmpl[] = { + { CKA_CLASS, &dataClass, sizeof(dataClass) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIVATE, &ckFalse, sizeof(ckFalse) }, + { CKA_APPLICATION, app, sizeof(app) - 1 }, + { CKA_VALUE, value, sizeof(value) - 1 }, + }; + + return funcList->C_CreateObject(session, tmpl, + sizeof(tmpl) / sizeof(*tmpl), &obj); +} + +/* Either the token refuses to load or the data object reports a real label + * length. */ +static int child_data_label_length(void) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_OBJECT_CLASS dataClass = CKO_DATA; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_ULONG count = 0; + CK_ATTRIBUTE findTmpl[] = { + { CKA_CLASS, &dataClass, sizeof(dataClass) }, + }; + CK_ATTRIBUTE label = { CKA_LABEL, NULL, 0 }; + int res = CHILD_SETUP; + + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv != CKR_OK) + return CHILD_PASS; + + rv = open_session(slot, 0, &session); + if (rv == CKR_OK) + rv = funcList->C_FindObjectsInit(session, findTmpl, 1); + if (rv == CKR_OK) + rv = funcList->C_FindObjects(session, &obj, 1, &count); + if (rv == CKR_OK) + rv = funcList->C_FindObjectsFinal(session); + if (rv == CKR_OK && count == 1) + rv = funcList->C_GetAttributeValue(session, obj, &label, 1); + if (rv == CKR_OK && count == 1) { + res = (label.ulValueLen == CK_UNAVAILABLE_INFORMATION || + label.ulValueLen > 64) ? CHILD_FAIL : CHILD_PASS; + if (res != CHILD_PASS) { + fprintf(stderr, " loaded label length 0x%lx\n", + (unsigned long)label.ulValueLen); + } + } + (void)funcList->C_Finalize(NULL); + return res; +} + +/* A negative length must not pass as the end of an older record format. */ +static int child_token_not_loaded(void) +{ + CK_SLOT_ID slot = 0; + int res; + + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + res = (init_slot(&slot) != CKR_OK) ? CHILD_PASS : CHILD_FAIL; + (void)funcList->C_Finalize(NULL); + return res; +} + +static void test_negative_optional_length(void) +{ + CK_RV rv; + char path[256]; + int res = CHILD_SETUP; + + printf("\n--- stored issuer length must not be negative ---\n"); + rv = store_objects(create_data_object); + CHECK_RV(rv, "store data object", CKR_OK); + if (rv != CKR_OK) + return; + if (find_store_file("obj", path, sizeof(path)) == 0 && + patch_length(path, OBJ_ISSUER_LEN_OFF, 0, 0xFFFFFFFFUL) == 0) { + res = run_in_child(child_token_not_loaded); + } + CHECK_TRUE(res == CHILD_PASS, + "negative stored issuer length is not loaded"); + cleanup_test_files(); +} + +static void test_negative_attribute_length(void) +{ + CK_RV rv; + char path[256]; + int res = CHILD_SETUP; + + printf("\n--- stored attribute length must not be negative ---\n"); + rv = store_objects(create_data_object); + CHECK_RV(rv, "store data object", CKR_OK); + if (rv != CKR_OK) + return; + if (find_store_file("obj", path, sizeof(path)) == 0 && + patch_length(path, OBJ_LABEL_LEN_OFF, 0, 0xFFFFFFFFUL) == 0) { + res = run_in_child(child_data_label_length); + } + CHECK_TRUE(res == CHILD_PASS, + "negative stored attribute length is not loaded"); + cleanup_test_files(); +} + +int main(int argc, char* argv[]) +{ + (void)argc; + (void)argv; + XSETENV("WOLFPKCS11_TOKEN_PATH", TEST_DIR, 1); + + printf("=== wolfPKCS11 stored object decode test ===\n"); + test_negative_optional_length(); + test_negative_attribute_length(); + + return pkcs11_test_summary(); +} + +#else + +int main(int argc, char* argv[]) +{ + (void)argc; + (void)argv; + printf("File-backed token storage not available, skipping test\n"); + return 77; +} + +#endif From 8437bbfc1dff4c515979224f402085b3457005ac Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 6 Oct 2026 10:16:19 -0700 Subject: [PATCH 02/28] F-5078 - Grow stored array buffers only as data is read --- src/internal.c | 118 +++++++++++++++-- tests/object_store_decode_test.c | 219 +++++++++++++++++++++++++++++++ 2 files changed, 327 insertions(+), 10 deletions(-) diff --git a/src/internal.c b/src/internal.c index db6e1ad0..1461aedf 100644 --- a/src/internal.c +++ b/src/internal.c @@ -147,6 +147,11 @@ #define WP11_MAX_CERT_SZ 4096 #endif +/* Initial allocation when reading a variable-length array from storage. */ +#ifndef WP11_STORE_READ_CHUNK_SZ +#define WP11_STORE_READ_CHUNK_SZ 4096 +#endif + #define PKCS11_CHECK_VALUE_SIZE 3 /* Sizes for storage. */ @@ -1979,6 +1984,36 @@ int wolfPKCS11_Store_Read(void* store, unsigned char* buffer, int len) return ret; } +#ifndef WOLFPKCS11_TPM_STORE +/** + * Get the number of bytes left to read from a store file. + * + * @param [in] store Context for operation. + * @param [out] left Number of bytes after the read position. + * @return 0 on success. + * @return BUFFER_E when the position cannot be worked out or restored. + */ +static int wolfPKCS11_Store_Left(void* store, long* left) +{ + WP11_FileStoreCtx* ctx = (WP11_FileStoreCtx*)store; + long pos = -1; + long end = -1; + + /* stdio directly: the file store's handles are FILE* (see fdopen()). */ + if (ctx != NULL && ctx->file != XBADFILE && ctx->file != NULL) { + pos = ftell(ctx->file); + if (pos >= 0 && fseek(ctx->file, 0, SEEK_END) == 0) + end = ftell(ctx->file); + if (pos >= 0 && fseek(ctx->file, pos, SEEK_SET) != 0) + end = -1; + } + if (pos < 0 || end < pos) + return BUFFER_E; + *left = end - pos; + return 0; +} +#endif + /** * Writes a specific number of bytes from buffer. * @@ -2080,6 +2115,26 @@ static void wp11_storage_close(void* storage) wolfPKCS11_Store_Close(storage); } +/** + * Get the number of bytes left to read, when the store can tell. + * + * @param [in] storage Context for operation. + * @param [out] left Number of bytes left to read. + * @return 0 on success. + * @return NOT_AVAILABLE_E when the store cannot tell. + * @return Other value on failure. + */ +static int wp11_storage_left(void* storage, long* left) +{ +#if !defined(WOLFPKCS11_CUSTOM_STORE) && !defined(WOLFPKCS11_TPM_STORE) + return wolfPKCS11_Store_Left(storage, left); +#else + (void)storage; + (void)left; + return NOT_AVAILABLE_E; +#endif +} + /** * Reads a specific number of bytes into buffer. * @@ -2469,6 +2524,13 @@ static int wp11_storage_read_alloc_array(void* storage, unsigned char** buffer, int* len) { int ret; + int have = 0; + int bufSz = 0; + int newSz; + int first = WP11_STORE_READ_CHUNK_SZ; + long left = 0; + unsigned char* buf = NULL; + unsigned char* tmp; /* Read length of array. */ ret = wp11_storage_read_int(storage, len); @@ -2478,21 +2540,57 @@ static int wp11_storage_read_alloc_array(void* storage, ret = ASN_PARSE_E; } if (ret == 0 && *len > 0) { - /* Allocate buffer to hold data. */ - *buffer = (unsigned char*)XMALLOC(*len, NULL, DYNAMIC_TYPE_TMP_BUFFER); - if (*buffer == NULL) - ret = MEMORY_E; + /* When the store knows its size, allocate once if the data is there. */ + ret = wp11_storage_left(storage, &left); + if (ret == 0 && (long)*len > left) + ret = BUFFER_E; + else if (ret == 0) + first = *len; + else if (ret == NOT_AVAILABLE_E) + ret = 0; + } + /* Otherwise grow the buffer only as stored data is read, so a corrupt + * length cannot drive an allocation larger than the data present. */ + while (ret == 0 && have < *len) { + if (bufSz == 0) + newSz = first; + else if (bufSz <= *len / 2) + newSz = bufSz * 2; + else + newSz = *len; + if (newSz > *len) + newSz = *len; - if (ret == 0) { - /* Read array data into allocated buffer. */ - ret = wp11_storage_read(storage, *buffer, *len); - if (ret != 0) { - XFREE(*buffer, NULL, DYNAMIC_TYPE_TMP_BUFFER); - *buffer = NULL; + tmp = (unsigned char*)XMALLOC(newSz, NULL, DYNAMIC_TYPE_TMP_BUFFER); + if (tmp == NULL) { + ret = MEMORY_E; + } + else { + if (buf != NULL) { + XMEMCPY(tmp, buf, have); + wc_ForceZero(buf, (word32)have); + XFREE(buf, NULL, DYNAMIC_TYPE_TMP_BUFFER); } + buf = tmp; + bufSz = newSz; + /* Read array data into the newly allocated space. */ + ret = wp11_storage_read(storage, buf + have, bufSz - have); + if (ret == 0) + have = bufSz; } } + if (ret != 0 && (buf != NULL || ret == MEMORY_E)) { + if (buf != NULL) { + wc_ForceZero(buf, (word32)bufSz); + XFREE(buf, NULL, DYNAMIC_TYPE_TMP_BUFFER); + } + *buffer = NULL; + } + else if (buf != NULL) { + *buffer = buf; + } + return ret; } diff --git a/tests/object_store_decode_test.c b/tests/object_store_decode_test.c index b240c46f..e26332f6 100644 --- a/tests/object_store_decode_test.c +++ b/tests/object_store_decode_test.c @@ -36,6 +36,7 @@ #endif #include #include +#include #ifndef WOLFPKCS11_USER_SETTINGS #include @@ -80,6 +81,74 @@ static const char* storeKinds[] = { "ecckey_pub", "dhkey_priv", "dhkey_pub", "cert", "trust" }; +#if defined(USE_WOLFSSL_MEMORY) && !defined(WOLFSSL_STATIC_MEMORY) && \ + !defined(WOLFSSL_DEBUG_MEMORY) +#define HAVE_ALLOC_TRACKING +#define ALLOC_REFUSE_SZ ((size_t)64 * 1024 * 1024) + +#define TRACK_HDR_SZ 16 + +static size_t largestAlloc = 0; +/* Live bytes in blocks no larger than peakCap, and their peak. */ +static size_t peakCap = 0; +static size_t liveBytes = 0; +static size_t peakLive = 0; + +static void* track_malloc(size_t sz) +{ + unsigned char* p; + + if (sz > largestAlloc) + largestAlloc = sz; + if (sz >= ALLOC_REFUSE_SZ) + return NULL; + p = (unsigned char*)malloc(sz + TRACK_HDR_SZ); + if (p == NULL) + return NULL; + XMEMCPY(p, &sz, sizeof(sz)); + if (sz <= peakCap) { + liveBytes += sz; + if (liveBytes > peakLive) + peakLive = liveBytes; + } + return p + TRACK_HDR_SZ; +} + +static void track_free(void* ptr) +{ + unsigned char* p; + size_t sz; + + if (ptr == NULL) + return; + p = (unsigned char*)ptr - TRACK_HDR_SZ; + XMEMCPY(&sz, p, sizeof(sz)); + if (sz <= peakCap) + liveBytes -= sz; + free(p); +} + +static void* track_realloc(void* ptr, size_t sz) +{ + void* n; + size_t old; + + if (ptr == NULL) + return track_malloc(sz); + if (sz == 0) { + track_free(ptr); + return NULL; + } + n = track_malloc(sz); + if (n == NULL) + return NULL; + XMEMCPY(&old, (unsigned char*)ptr - TRACK_HDR_SZ, sizeof(old)); + XMEMCPY(n, ptr, (old < sz) ? old : sz); + track_free(ptr); + return n; +} +#endif + static void store_path(char* path, size_t sz, const char* kind, int objId) { if (objId < 0) { @@ -410,6 +479,149 @@ static void test_negative_attribute_length(void) cleanup_test_files(); } +#define LARGE_VALUE_SZ 300001 +static byte largeValue[LARGE_VALUE_SZ]; +static byte largeRead[LARGE_VALUE_SZ]; + +static CK_RV create_large_data_object(CK_SESSION_HANDLE session) +{ + CK_OBJECT_CLASS dataClass = CKO_DATA; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_ATTRIBUTE tmpl[] = { + { CKA_CLASS, &dataClass, sizeof(dataClass) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIVATE, &ckFalse, sizeof(ckFalse) }, + { CKA_VALUE, largeValue, sizeof(largeValue) }, + }; + size_t i; + + for (i = 0; i < sizeof(largeValue); i++) + largeValue[i] = (byte)(i * 7 + (i >> 8)); + return funcList->C_CreateObject(session, tmpl, + sizeof(tmpl) / sizeof(*tmpl), &obj); +} + +static void test_large_stored_value_round_trip(void) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_OBJECT_CLASS dataClass = CKO_DATA; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_ULONG count = 0; + CK_ATTRIBUTE findTmpl[] = { + { CKA_CLASS, &dataClass, sizeof(dataClass) }, + }; + CK_ATTRIBUTE value = { CKA_VALUE, largeRead, sizeof(largeRead) }; + + printf("\n--- large stored value reloads intact ---\n"); + rv = store_objects(create_large_data_object); + CHECK_RV(rv, "store large data object", CKR_OK); + if (rv != CKR_OK) + return; + rv = pkcs11_load(); + if (rv == CKR_OK) { + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = open_session(slot, 0, &session); + if (rv == CKR_OK) + rv = funcList->C_FindObjectsInit(session, findTmpl, 1); + if (rv == CKR_OK) + rv = funcList->C_FindObjects(session, &obj, 1, &count); + if (rv == CKR_OK) + rv = funcList->C_FindObjectsFinal(session); + if (rv == CKR_OK && count != 1) + rv = CKR_GENERAL_ERROR; + if (rv == CKR_OK) + rv = funcList->C_GetAttributeValue(session, obj, &value, 1); + (void)funcList->C_Finalize(NULL); + pkcs11_unload(); + } + CHECK_RV(rv, "reload large data object", CKR_OK); + CHECK_TRUE(rv == CKR_OK && value.ulValueLen == sizeof(largeValue) && + XMEMCMP(largeRead, largeValue, sizeof(largeValue)) == 0, + "large stored value is unchanged after reload"); + cleanup_test_files(); +} + +#ifdef HAVE_ALLOC_TRACKING +/* Loading must not size an allocation from a length the data cannot back. */ +static int child_load_allocation_bound(void) +{ + CK_SLOT_ID slot = 0; + + if (wolfSSL_SetAllocators(track_malloc, track_free, track_realloc) != 0) + return CHILD_SETUP; + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + if (init_slot(&slot) == CKR_OK) + (void)funcList->C_Finalize(NULL); + if (largestAlloc >= ALLOC_REFUSE_SZ) { + fprintf(stderr, " largest allocation while loading: %lu\n", + (unsigned long)largestAlloc); + return CHILD_FAIL; + } + return CHILD_PASS; +} + +static void test_oversized_attribute_length(void) +{ + CK_RV rv; + char path[256]; + int res = CHILD_SETUP; + + printf("\n--- stored attribute length must be bounded ---\n"); + rv = store_objects(create_data_object); + CHECK_RV(rv, "store data object", CKR_OK); + if (rv != CKR_OK) + return; + if (find_store_file("obj", path, sizeof(path)) == 0 && + patch_length(path, OBJ_LABEL_LEN_OFF, 0, 0x7FFFFFF8UL) == 0) { + res = run_in_child(child_load_allocation_bound); + } + CHECK_TRUE(res == CHILD_PASS, + "oversized stored attribute length is not allocated"); + cleanup_test_files(); +} + +/* Loading a large stored value holds about one copy of it, not two. Larger + * blocks cannot be partial copies of it, so they are not counted. */ +static int child_large_value_peak(void) +{ + CK_SLOT_ID slot = 0; + + peakCap = LARGE_VALUE_SZ; + if (wolfSSL_SetAllocators(track_malloc, track_free, track_realloc) != 0) + return CHILD_SETUP; + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + if (init_slot(&slot) == CKR_OK) + (void)funcList->C_Finalize(NULL); + if (peakLive >= (size_t)LARGE_VALUE_SZ + LARGE_VALUE_SZ / 2) { + fprintf(stderr, " peak live bytes while loading: %lu\n", + (unsigned long)peakLive); + return CHILD_FAIL; + } + return CHILD_PASS; +} + +static void test_large_stored_value_peak(void) +{ + CK_RV rv; + int res; + + printf("\n--- large stored value loads without a second copy ---\n"); + rv = store_objects(create_large_data_object); + CHECK_RV(rv, "store large data object", CKR_OK); + if (rv != CKR_OK) + return; + res = run_in_child(child_large_value_peak); + CHECK_TRUE(res == CHILD_PASS, + "peak memory while loading stays near one copy"); + cleanup_test_files(); +} +#endif + int main(int argc, char* argv[]) { (void)argc; @@ -419,6 +631,13 @@ int main(int argc, char* argv[]) printf("=== wolfPKCS11 stored object decode test ===\n"); test_negative_optional_length(); test_negative_attribute_length(); +#ifdef HAVE_ALLOC_TRACKING + test_oversized_attribute_length(); +#endif + test_large_stored_value_round_trip(); +#ifdef HAVE_ALLOC_TRACKING + test_large_stored_value_peak(); +#endif return pkcs11_test_summary(); } From 8427035a6461d3fd3fb60d289848a67873c6b5bf Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 6 Oct 2026 10:25:41 -0700 Subject: [PATCH 03/28] F-13814 - Set data object lengths only after a successful storage read --- src/internal.c | 11 +++-- tests/object_store_decode_test.c | 75 +++++++++++++++++++++++++++++++- 2 files changed, 80 insertions(+), 6 deletions(-) diff --git a/src/internal.c b/src/internal.c index 1461aedf..3a1a1e04 100644 --- a/src/internal.c +++ b/src/internal.c @@ -3596,7 +3596,7 @@ static int wp11_Object_Load_Data(WP11_Object* object, int tokenId, int objId) { int ret; void* storage = NULL; - int tempLen; + int tempLen = 0; /* Open access to data. */ ret = wp11_storage_open_readonly(WOLFPKCS11_STORE_DATA, tokenId, objId, @@ -3605,7 +3605,8 @@ static int wp11_Object_Load_Data(WP11_Object* object, int tokenId, int objId) ret = wp11_storage_read_alloc_array(storage, &object->data.genericData.data, &tempLen); - object->data.genericData.dataLen = (word32)tempLen; + if (ret == 0) + object->data.genericData.dataLen = (word32)tempLen; } /* Read application length and application. */ @@ -3613,7 +3614,8 @@ static int wp11_Object_Load_Data(WP11_Object* object, int tokenId, int objId) ret = wp11_storage_read_alloc_array(storage, &object->data.genericData.application, &tempLen); - object->data.genericData.applicationLen = (word32)tempLen; + if (ret == 0) + object->data.genericData.applicationLen = (word32)tempLen; } /* Read object ID length and object ID. */ @@ -3621,7 +3623,8 @@ static int wp11_Object_Load_Data(WP11_Object* object, int tokenId, int objId) ret = wp11_storage_read_alloc_array(storage, &object->data.genericData.objectId, &tempLen); - object->data.genericData.objectIdLen = (word32)tempLen; + if (ret == 0) + object->data.genericData.objectIdLen = (word32)tempLen; } wp11_storage_close(storage); diff --git a/tests/object_store_decode_test.c b/tests/object_store_decode_test.c index e26332f6..d90771cc 100644 --- a/tests/object_store_decode_test.c +++ b/tests/object_store_decode_test.c @@ -75,6 +75,7 @@ static byte soPin[] = "password123456"; static byte userPin[] = "wolfpkcs11-test"; static CK_BBOOL ckTrue = CK_TRUE; static CK_BBOOL ckFalse = CK_FALSE; +static byte dataValue[] = "stored data object value"; static const char* storeKinds[] = { "obj", "data", "symmkey", "rsakey_priv", "rsakey_pub", "ecckey_priv", @@ -370,13 +371,12 @@ static CK_RV create_data_object(CK_SESSION_HANDLE session) CK_OBJECT_CLASS dataClass = CKO_DATA; CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; static byte app[] = "wolfpkcs11"; - static byte value[] = "stored data object value"; CK_ATTRIBUTE tmpl[] = { { CKA_CLASS, &dataClass, sizeof(dataClass) }, { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, { CKA_PRIVATE, &ckFalse, sizeof(ckFalse) }, { CKA_APPLICATION, app, sizeof(app) - 1 }, - { CKA_VALUE, value, sizeof(value) - 1 }, + { CKA_VALUE, dataValue, sizeof(dataValue) - 1 }, }; return funcList->C_CreateObject(session, tmpl, @@ -479,6 +479,76 @@ static void test_negative_attribute_length(void) cleanup_test_files(); } +static int truncate_file(const char* path, size_t keep) +{ + static byte data[MAX_FILE_SZ]; + size_t sz = 0; + + if (read_file(path, data, sizeof(data), &sz) != 0 || sz <= keep) + return -1; + return write_file(path, data, keep); +} + +/* Either the token refuses to load or the data object reads back its full + * stored value. */ +static int child_data_value_intact(void) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_OBJECT_CLASS dataClass = CKO_DATA; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_ULONG count = 0; + CK_ATTRIBUTE findTmpl[] = { + { CKA_CLASS, &dataClass, sizeof(dataClass) }, + }; + byte value[64]; + CK_ATTRIBUTE attr = { CKA_VALUE, value, sizeof(value) }; + int res = CHILD_FAIL; + + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv != CKR_OK) + return CHILD_PASS; + + rv = open_session(slot, 0, &session); + if (rv == CKR_OK) + rv = funcList->C_FindObjectsInit(session, findTmpl, 1); + if (rv == CKR_OK) + rv = funcList->C_FindObjects(session, &obj, 1, &count); + if (rv == CKR_OK) + rv = funcList->C_FindObjectsFinal(session); + if (rv == CKR_OK && count == 1) + rv = funcList->C_GetAttributeValue(session, obj, &attr, 1); + if (rv == CKR_OK && count == 1 && + attr.ulValueLen == sizeof(dataValue) - 1 && + XMEMCMP(value, dataValue, sizeof(dataValue) - 1) == 0) { + res = CHILD_PASS; + } + (void)funcList->C_Finalize(NULL); + return res; +} + +static void test_truncated_data_record(void) +{ + CK_RV rv; + char path[256]; + int res = CHILD_SETUP; + + printf("\n--- truncated data object record is not loaded ---\n"); + rv = store_objects(create_data_object); + CHECK_RV(rv, "store data object", CKR_OK); + if (rv != CKR_OK) + return; + if (find_store_file("data", path, sizeof(path)) == 0 && + truncate_file(path, 2) == 0) { + res = run_in_child(child_data_value_intact); + } + CHECK_TRUE(res == CHILD_PASS, "truncated data object record is rejected"); + cleanup_test_files(); +} + #define LARGE_VALUE_SZ 300001 static byte largeValue[LARGE_VALUE_SZ]; static byte largeRead[LARGE_VALUE_SZ]; @@ -638,6 +708,7 @@ int main(int argc, char* argv[]) #ifdef HAVE_ALLOC_TRACKING test_large_stored_value_peak(); #endif + test_truncated_data_record(); return pkcs11_test_summary(); } From 651c6c8bb39e87769b8edecb1d48ec5bc618477f Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 6 Oct 2026 10:45:11 -0700 Subject: [PATCH 04/28] F-4264 - Reject stored private key records shorter than the tag --- src/internal.c | 44 ++++--- tests/object_store_decode_test.c | 189 ++++++++++++++++++++++++++++++- 2 files changed, 218 insertions(+), 15 deletions(-) diff --git a/src/internal.c b/src/internal.c index 3a1a1e04..77960a7b 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4230,11 +4230,14 @@ static int wp11_Object_Decode_RsaKey(WP11_Object* object) else #endif if (object->objClass == CKO_PRIVATE_KEY) { - unsigned char* der; + unsigned char* der = NULL; int len = object->keyDataLen - AES_BLOCK_SIZE; - der = (unsigned char*)XMALLOC(len, NULL, DYNAMIC_TYPE_TMP_BUFFER); - if (der == NULL) { + if (object->keyDataLen <= AES_BLOCK_SIZE) + ret = BAD_FUNC_ARG; + if (ret == 0) + der = (unsigned char*)XMALLOC(len, NULL, DYNAMIC_TYPE_TMP_BUFFER); + if (ret == 0 && der == NULL) { ret = MEMORY_E; } if (ret == 0) { @@ -4578,11 +4581,14 @@ static int wp11_Object_Decode_EccKey(WP11_Object* object) else #endif if (object->objClass == CKO_PRIVATE_KEY) { - unsigned char* der; + unsigned char* der = NULL; int len = object->keyDataLen - AES_BLOCK_SIZE; - der = (unsigned char*)XMALLOC(len, NULL, DYNAMIC_TYPE_TMP_BUFFER); - if (der == NULL) { + if (object->keyDataLen <= AES_BLOCK_SIZE) + ret = BAD_FUNC_ARG; + if (ret == 0) + der = (unsigned char*)XMALLOC(len, NULL, DYNAMIC_TYPE_TMP_BUFFER); + if (ret == 0 && der == NULL) { ret = MEMORY_E; } if (ret == 0) { @@ -4818,11 +4824,14 @@ static int wp11_Object_Decode_MldsaKey(WP11_Object* object) int ret = 0; if (object->objClass == CKO_PRIVATE_KEY) { - unsigned char* der; + unsigned char* der = NULL; int len = object->keyDataLen - AES_BLOCK_SIZE; - der = (unsigned char*)XMALLOC(len, NULL, DYNAMIC_TYPE_TMP_BUFFER); - if (der == NULL) { + if (object->keyDataLen <= AES_BLOCK_SIZE) + ret = BAD_FUNC_ARG; + if (ret == 0) + der = (unsigned char*)XMALLOC(len, NULL, DYNAMIC_TYPE_TMP_BUFFER); + if (ret == 0 && der == NULL) { ret = MEMORY_E; } if (ret == 0) { @@ -5035,11 +5044,18 @@ static int wp11_Object_Decode_DhKey(WP11_Object* object) int ret = 0; if (object->objClass == CKO_PRIVATE_KEY) { - ret = wp11_DecryptData(object->data.dhKey->key, object->keyData, - object->keyDataLen - AES_BLOCK_SIZE, - object->slot->token.key, - sizeof(object->slot->token.key), object->iv, - sizeof(object->iv), object->devId); + if (object->keyDataLen <= AES_BLOCK_SIZE) + ret = BAD_FUNC_ARG; + else if (object->keyDataLen - AES_BLOCK_SIZE > WP11_MAX_DH_KEY_SZ) + ret = BUFFER_E; + if (ret == 0) { + ret = wp11_DecryptData(object->data.dhKey->key, object->keyData, + object->keyDataLen - AES_BLOCK_SIZE, + object->slot->token.key, + sizeof(object->slot->token.key), + object->iv, sizeof(object->iv), + object->devId); + } if (ret == 0) object->data.dhKey->len = object->keyDataLen - AES_BLOCK_SIZE; } diff --git a/tests/object_store_decode_test.c b/tests/object_store_decode_test.c index d90771cc..963e34b6 100644 --- a/tests/object_store_decode_test.c +++ b/tests/object_store_decode_test.c @@ -79,7 +79,8 @@ static byte dataValue[] = "stored data object value"; static const char* storeKinds[] = { "obj", "data", "symmkey", "rsakey_priv", "rsakey_pub", "ecckey_priv", - "ecckey_pub", "dhkey_priv", "dhkey_pub", "cert", "trust" + "ecckey_pub", "dhkey_priv", "dhkey_pub", "cert", "trust", "mldsakey_priv", + "mldsakey_pub" }; #if defined(USE_WOLFSSL_MEMORY) && !defined(WOLFSSL_STATIC_MEMORY) && \ @@ -549,6 +550,191 @@ static void test_truncated_data_record(void) cleanup_test_files(); } +/* Replace the first length-prefixed array in a stored key record with newLen + * bytes, keeping the rest of the record unchanged. */ +static int rewrite_first_array(const char* path, word32 newLen) +{ + static byte data[MAX_FILE_SZ]; + static byte out[MAX_FILE_SZ]; + size_t sz = 0; + word32 oldLen; + size_t tail; + + if (read_file(path, data, sizeof(data), &sz) != 0 || sz < 4) + return -1; + oldLen = get_be32(data); + if (oldLen > sz - 4 || 4 + (size_t)newLen + (sz - 4 - oldLen) > sizeof(out)) + return -1; + tail = sz - 4 - oldLen; + put_be32(out, newLen); + XMEMSET(out + 4, 0x5A, newLen); + XMEMCPY(out + 4, data + 4, (newLen < oldLen) ? newLen : oldLen); + XMEMCPY(out + 4 + newLen, data + 4 + oldLen, tail); + return write_file(path, out, 4 + newLen + tail); +} + +/* Loading may succeed but logging in must reject the damaged key record. */ +static int child_login_rejected(void) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + int res = CHILD_FAIL; + + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv != CKR_OK) + return CHILD_PASS; + rv = open_session(slot, 1, &session); + if (rv != CKR_OK) + res = CHILD_PASS; + (void)funcList->C_Finalize(NULL); + return res; +} + +static void check_damaged_key_record(const char* name, create_fn create, + const char* kind, word32 newLen) +{ + CK_RV rv; + char path[256]; + char msg[128]; + int res = CHILD_SETUP; + + rv = store_objects(create); + (void)snprintf(msg, sizeof(msg), "store %s", name); + CHECK_RV(rv, msg, CKR_OK); + if (rv != CKR_OK) + return; + if (find_store_file(kind, path, sizeof(path)) == 0 && + rewrite_first_array(path, newLen) == 0) { + res = run_in_child(child_login_rejected); + } + (void)snprintf(msg, sizeof(msg), "damaged %s record is rejected", name); + CHECK_TRUE(res == CHILD_PASS, msg); + cleanup_test_files(); +} + +#ifndef NO_DH +static CK_RV create_dh_private_key(CK_SESSION_HANDLE session) +{ + CK_OBJECT_CLASS keyClass = CKO_PRIVATE_KEY; + CK_KEY_TYPE keyType = CKK_DH; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_ATTRIBUTE tmpl[] = { + { CKA_CLASS, &keyClass, sizeof(keyClass) }, + { CKA_KEY_TYPE, &keyType, sizeof(keyType) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIVATE, &ckTrue, sizeof(ckTrue) }, + { CKA_DERIVE, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIME, dh_ffdhe2048_p, sizeof(dh_ffdhe2048_p) }, + { CKA_BASE, dh_ffdhe2048_g, sizeof(dh_ffdhe2048_g) }, + { CKA_VALUE, dh_2048_priv, sizeof(dh_2048_priv) }, + }; + + return funcList->C_CreateObject(session, tmpl, + sizeof(tmpl) / sizeof(*tmpl), &obj); +} +#endif + +#ifdef HAVE_ECC +static CK_RV create_ecc_private_key(CK_SESSION_HANDLE session) +{ + CK_OBJECT_CLASS keyClass = CKO_PRIVATE_KEY; + CK_KEY_TYPE keyType = CKK_EC; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_ATTRIBUTE tmpl[] = { + { CKA_CLASS, &keyClass, sizeof(keyClass) }, + { CKA_KEY_TYPE, &keyType, sizeof(keyType) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIVATE, &ckTrue, sizeof(ckTrue) }, + { CKA_SIGN, &ckTrue, sizeof(ckTrue) }, + { CKA_EC_PARAMS, ecc_p256_params, sizeof(ecc_p256_params) }, + { CKA_VALUE, ecc_p256_priv, sizeof(ecc_p256_priv) }, + }; + + return funcList->C_CreateObject(session, tmpl, + sizeof(tmpl) / sizeof(*tmpl), &obj); +} +#endif + +#ifndef NO_RSA +static CK_RV create_rsa_private_key(CK_SESSION_HANDLE session) +{ + CK_OBJECT_CLASS keyClass = CKO_PRIVATE_KEY; + CK_KEY_TYPE keyType = CKK_RSA; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_ATTRIBUTE tmpl[] = { + { CKA_CLASS, &keyClass, sizeof(keyClass) }, + { CKA_KEY_TYPE, &keyType, sizeof(keyType) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIVATE, &ckTrue, sizeof(ckTrue) }, + { CKA_SIGN, &ckTrue, sizeof(ckTrue) }, + { CKA_MODULUS, rsa_2048_modulus, sizeof(rsa_2048_modulus) }, + { CKA_PUBLIC_EXPONENT, rsa_2048_pub_exp, sizeof(rsa_2048_pub_exp) }, + { CKA_PRIVATE_EXPONENT, rsa_2048_priv_exp, sizeof(rsa_2048_priv_exp) }, + { CKA_PRIME_1, rsa_2048_p, sizeof(rsa_2048_p) }, + { CKA_PRIME_2, rsa_2048_q, sizeof(rsa_2048_q) }, + { CKA_EXPONENT_1, rsa_2048_dP, sizeof(rsa_2048_dP) }, + { CKA_EXPONENT_2, rsa_2048_dQ, sizeof(rsa_2048_dQ) }, + { CKA_COEFFICIENT, rsa_2048_u, sizeof(rsa_2048_u) }, + }; + + return funcList->C_CreateObject(session, tmpl, + sizeof(tmpl) / sizeof(*tmpl), &obj); +} +#endif + +#ifdef WOLFPKCS11_MLDSA +static CK_RV create_mldsa_key_pair(CK_SESSION_HANDLE session) +{ + CK_MECHANISM mech = { CKM_ML_DSA_KEY_PAIR_GEN, NULL, 0 }; + CK_ULONG paramSet = CKP_ML_DSA_44; + CK_OBJECT_HANDLE pub = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE priv = CK_INVALID_HANDLE; + CK_ATTRIBUTE pubTmpl[] = { + { CKA_PARAMETER_SET, ¶mSet, sizeof(paramSet) }, + { CKA_VERIFY, &ckTrue, sizeof(ckTrue) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + }; + CK_ATTRIBUTE privTmpl[] = { + { CKA_SIGN, &ckTrue, sizeof(ckTrue) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + }; + + return funcList->C_GenerateKeyPair(session, &mech, pubTmpl, + sizeof(pubTmpl) / sizeof(*pubTmpl), privTmpl, + sizeof(privTmpl) / sizeof(*privTmpl), &pub, &priv); +} +#endif + +/* A stored private key record shorter than its authentication tag, or + * longer than the key it decrypts into. */ +static void test_short_private_key_record(void) +{ + printf("\n--- short private key record is rejected ---\n"); +#ifndef NO_DH + check_damaged_key_record("DH private key", create_dh_private_key, + "dhkey_priv", 8); + check_damaged_key_record("tag-only DH private key", create_dh_private_key, + "dhkey_priv", 16); + check_damaged_key_record("oversized DH private key", + create_dh_private_key, "dhkey_priv", 4096 + 16); +#endif +#ifdef HAVE_ECC + check_damaged_key_record("tag-only ECC private key", + create_ecc_private_key, "ecckey_priv", 16); +#endif +#ifndef NO_RSA + check_damaged_key_record("tag-only RSA private key", + create_rsa_private_key, "rsakey_priv", 16); +#endif +#ifdef WOLFPKCS11_MLDSA + check_damaged_key_record("tag-only ML-DSA private key", + create_mldsa_key_pair, "mldsakey_priv", 16); +#endif +} + #define LARGE_VALUE_SZ 300001 static byte largeValue[LARGE_VALUE_SZ]; static byte largeRead[LARGE_VALUE_SZ]; @@ -709,6 +895,7 @@ int main(int argc, char* argv[]) test_large_stored_value_peak(); #endif test_truncated_data_record(); + test_short_private_key_record(); return pkcs11_test_summary(); } From 2982321af146b93b9e05b773d4ef054addcff6da Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 6 Oct 2026 10:53:42 -0700 Subject: [PATCH 05/28] F-4733 - Bound stored DH public key length to the key buffer --- src/internal.c | 3 +++ tests/object_store_decode_test.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/internal.c b/src/internal.c index 77960a7b..70a522de 100644 --- a/src/internal.c +++ b/src/internal.c @@ -5059,6 +5059,9 @@ static int wp11_Object_Decode_DhKey(WP11_Object* object) if (ret == 0) object->data.dhKey->len = object->keyDataLen - AES_BLOCK_SIZE; } + else if (object->keyDataLen > WP11_MAX_DH_KEY_SZ) { + ret = BUFFER_E; + } else { XMEMCPY(object->data.dhKey->key, object->keyData, object->keyDataLen); object->data.dhKey->len = object->keyDataLen; diff --git a/tests/object_store_decode_test.c b/tests/object_store_decode_test.c index 963e34b6..15a57d96 100644 --- a/tests/object_store_decode_test.c +++ b/tests/object_store_decode_test.c @@ -735,6 +735,34 @@ static void test_short_private_key_record(void) #endif } +#ifndef NO_DH +static CK_RV create_dh_public_key(CK_SESSION_HANDLE session) +{ + CK_OBJECT_CLASS keyClass = CKO_PUBLIC_KEY; + CK_KEY_TYPE keyType = CKK_DH; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_ATTRIBUTE tmpl[] = { + { CKA_CLASS, &keyClass, sizeof(keyClass) }, + { CKA_KEY_TYPE, &keyType, sizeof(keyType) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIME, dh_ffdhe2048_p, sizeof(dh_ffdhe2048_p) }, + { CKA_BASE, dh_ffdhe2048_g, sizeof(dh_ffdhe2048_g) }, + { CKA_VALUE, dh_2048_pub, sizeof(dh_2048_pub) }, + }; + + return funcList->C_CreateObject(session, tmpl, + sizeof(tmpl) / sizeof(*tmpl), &obj); +} + +/* A stored public key record longer than the largest supported key. */ +static void test_oversized_public_key_record(void) +{ + printf("\n--- oversized public key record is rejected ---\n"); + check_damaged_key_record("oversized DH public key", create_dh_public_key, + "dhkey_pub", 4096); +} +#endif + #define LARGE_VALUE_SZ 300001 static byte largeValue[LARGE_VALUE_SZ]; static byte largeRead[LARGE_VALUE_SZ]; @@ -896,6 +924,9 @@ int main(int argc, char* argv[]) #endif test_truncated_data_record(); test_short_private_key_record(); +#ifndef NO_DH + test_oversized_public_key_record(); +#endif return pkcs11_test_summary(); } From 4a951dfdf53b6f216715801481a7b5e3aa28a1cd Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 6 Oct 2026 11:06:36 -0700 Subject: [PATCH 06/28] F-3641 - Reject stored trust records that are not the trust size --- src/internal.c | 28 ++++++++++++++++++------ tests/object_store_decode_test.c | 37 ++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/src/internal.c b/src/internal.c index 70a522de..37a9c6b3 100644 --- a/src/internal.c +++ b/src/internal.c @@ -3511,14 +3511,25 @@ static void wp11_Object_Decode_Cert(WP11_Object* object) * Trust is not encrypted. * * @param [in, out] object Trust object. + * @return 0 on success. + * @return BUFFER_E when the stored trust data is not the expected size. */ -static void wp11_Object_Decode_Trust(WP11_Object* object) +static int wp11_Object_Decode_Trust(WP11_Object* object) { + int ret = 0; + if (object->keyData != NULL) { - XMEMCPY((unsigned char*)&object->data.trust, object->keyData, - object->keyDataLen); + if (object->keyDataLen != (int)sizeof(WP11_Trust)) { + ret = BUFFER_E; + } + else { + XMEMCPY((unsigned char*)&object->data.trust, object->keyData, + object->keyDataLen); + } } - object->encoded = 0; + object->encoded = (ret != 0); + + return ret; } #endif @@ -3585,7 +3596,11 @@ static int wp11_Object_Load_Trust(WP11_Object* object, int tokenId, int objId) ret = wp11_storage_read_alloc_array(storage, &object->keyData, &object->keyDataLen); wp11_storage_close(storage); - wp11_Object_Decode_Trust(object); + /* An empty record would leave the trust zeroed but decoded. */ + if (ret == 0 && object->keyData == NULL) + ret = BUFFER_E; + if (ret == 0) + ret = wp11_Object_Decode_Trust(object); } return ret; @@ -6960,8 +6975,7 @@ static int wp11_Object_Decode(WP11_Object* object) } #ifdef WOLFPKCS11_NSS else if (object->objClass == CKO_NSS_TRUST) { - wp11_Object_Decode_Trust(object); - ret = 0; + ret = wp11_Object_Decode_Trust(object); } #endif else if (object->objClass == CKO_DATA) { diff --git a/tests/object_store_decode_test.c b/tests/object_store_decode_test.c index 15a57d96..43d034be 100644 --- a/tests/object_store_decode_test.c +++ b/tests/object_store_decode_test.c @@ -763,6 +763,40 @@ static void test_oversized_public_key_record(void) } #endif +#ifdef WOLFPKCS11_NSS +static CK_RV create_trust_object(CK_SESSION_HANDLE session) +{ + CK_OBJECT_CLASS trustClass = CKO_NSS_TRUST; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + static byte issuer[] = "CN=Test,O=wolfSSL,C=US"; + static byte serial[] = { 0x02, 0x05, 0x00, 0xC6, 0xA7, 0x91, 0x84 }; + static byte sha1Hash[20] = { 0 }; + static byte md5Hash[16] = { 0 }; + CK_ULONG trustValue = 0xCE534352; + CK_ATTRIBUTE tmpl[] = { + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_CLASS, &trustClass, sizeof(trustClass) }, + { CKA_ISSUER, issuer, sizeof(issuer) - 1 }, + { CKA_SERIAL_NUMBER, serial, sizeof(serial) }, + { CKA_CERT_SHA1_HASH, sha1Hash, sizeof(sha1Hash) }, + { CKA_CERT_MD5_HASH, md5Hash, sizeof(md5Hash) }, + { CKA_TRUST_SERVER_AUTH, &trustValue, sizeof(trustValue) }, + }; + + return funcList->C_CreateObject(session, tmpl, + sizeof(tmpl) / sizeof(*tmpl), &obj); +} + +/* Trust records have a fixed size and are stored unencrypted. */ +static void test_trust_record_length(void) +{ + printf("\n--- trust record length must match ---\n"); + check_damaged_key_record("oversized trust", create_trust_object, "trust", + 1024); + check_damaged_key_record("empty trust", create_trust_object, "trust", 0); +} +#endif + #define LARGE_VALUE_SZ 300001 static byte largeValue[LARGE_VALUE_SZ]; static byte largeRead[LARGE_VALUE_SZ]; @@ -927,6 +961,9 @@ int main(int argc, char* argv[]) #ifndef NO_DH test_oversized_public_key_record(); #endif +#ifdef WOLFPKCS11_NSS + test_trust_record_length(); +#endif return pkcs11_test_summary(); } From 4ab2ef30d6eb0fb7bee5f5635b34df8c31d7d05e Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 6 Oct 2026 11:14:03 -0700 Subject: [PATCH 07/28] F-3404 - Write the encoded TPM key into the caller's buffer --- src/internal.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/internal.c b/src/internal.c index 37a9c6b3..9b803a83 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4150,15 +4150,15 @@ static int WP11_Object_EncodeTpmKey(WP11_Object* object, byte* keyData, if (keyData != NULL) { if (ret <= keyDataLen) { /* Write size marker for the public part */ - XMEMCPY(object->keyData, &pubAreaSize, + XMEMCPY(keyData, &pubAreaSize, sizeof(UINT16)); idx += sizeof(UINT16); /* Write the public part with bytes aligned */ - XMEMCPY(object->keyData + idx, pubAreaBuffer, + XMEMCPY(keyData + idx, pubAreaBuffer, sizeof(UINT16) + pubAreaSize); idx += sizeof(UINT16) + pubAreaSize; /* Write the private part, size marker is included */ - XMEMCPY(object->keyData + idx, &object->tpmKey->priv, + XMEMCPY(keyData + idx, &object->tpmKey->priv, sizeof(UINT16) + object->tpmKey->priv.size); idx += sizeof(UINT16) + object->tpmKey->priv.size; From af7c5da2ed97aaf4b0a4331c83c2634fa9df55f2 Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 6 Oct 2026 11:20:21 -0700 Subject: [PATCH 08/28] F-8643 - Keep key buffer allocation errors in TPM RSA and ECC encoders --- src/internal.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/internal.c b/src/internal.c index 9b803a83..578ccf85 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4336,7 +4336,10 @@ static int wp11_Object_Encode_RsaKey(WP11_Object* object) } #ifdef WOLFPKCS11_TPM - ret = WP11_Object_EncodeTpmKey(object, object->keyData, object->keyDataLen); + if (ret == 0) { + ret = WP11_Object_EncodeTpmKey(object, object->keyData, + object->keyDataLen); + } if (ret > 0) { ret = 0; } @@ -4678,7 +4681,10 @@ static int wp11_Object_Encode_EccKey(WP11_Object* object) } #ifdef WOLFPKCS11_TPM - ret = WP11_Object_EncodeTpmKey(object, object->keyData, object->keyDataLen); + if (ret == 0) { + ret = WP11_Object_EncodeTpmKey(object, object->keyData, + object->keyDataLen); + } if (ret > 0) { ret = 0; } From 004016c8cfc9e272da271cdc4628bf3e2b5f84cf Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 6 Oct 2026 11:27:25 -0700 Subject: [PATCH 09/28] F-13813 - Release DH and symmetric key data when encoding fails --- src/internal.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/internal.c b/src/internal.c index 578ccf85..1256dfcb 100644 --- a/src/internal.c +++ b/src/internal.c @@ -5130,6 +5130,14 @@ static int wp11_Object_Encode_DhKey(WP11_Object* object) } } + if (ret != 0) { + if (object->keyData != NULL) + wc_ForceZero(object->keyData, object->keyDataLen); + XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); + object->keyData = NULL; + object->keyDataLen = 0; + } + return ret; } @@ -6507,6 +6515,14 @@ static int wp11_Object_Encode_SymmKey(WP11_Object* object) object->keyDataLen = object->data.symmKey->len + AES_BLOCK_SIZE; } + if (ret != 0) { + if (object->keyData != NULL) + wc_ForceZero(object->keyData, object->keyDataLen); + XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); + object->keyData = NULL; + object->keyDataLen = 0; + } + return ret; } From c240e05f3bc45a89cb4b9f4db4645c594ec2d3f7 Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 6 Oct 2026 11:33:20 -0700 Subject: [PATCH 10/28] F-3017 - Zeroize encoded private keys when RSA, ECC or ML-DSA encoding fails --- src/internal.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/internal.c b/src/internal.c index 1256dfcb..a760ef73 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4370,6 +4370,8 @@ static int wp11_Object_Encode_RsaKey(WP11_Object* object) } if (ret != 0) { + if (object->keyData != NULL) + wc_ForceZero(object->keyData, object->keyDataLen); XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); object->keyData = NULL; object->keyDataLen = 0; @@ -4711,6 +4713,8 @@ static int wp11_Object_Encode_EccKey(WP11_Object* object) } if (ret != 0) { + if (object->keyData != NULL) + wc_ForceZero(object->keyData, object->keyDataLen); XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); object->keyData = NULL; object->keyDataLen = 0; @@ -4961,6 +4965,8 @@ static int wp11_Object_Encode_MldsaKey(WP11_Object* object) } if (ret != 0) { + if (object->keyData != NULL) + wc_ForceZero(object->keyData, object->keyDataLen); XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); object->keyData = NULL; object->keyDataLen = 0; From bb0085e11e172b05102ac892abc4656b4a0cd490 Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 6 Oct 2026 11:47:05 -0700 Subject: [PATCH 11/28] F-3413 - Zeroize decrypted private key buffers on every decode path --- src/internal.c | 21 ++-- tests/object_store_decode_test.c | 170 +++++++++++++++++++++++++++++++ 2 files changed, 185 insertions(+), 6 deletions(-) diff --git a/src/internal.c b/src/internal.c index a760ef73..ea0f1950 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4264,16 +4264,20 @@ static int wp11_Object_Decode_RsaKey(WP11_Object* object) if (ret == 0) { /* Decode RSA private key. */ ret = wc_RsaPrivateKeyDecode(der, &idx, key, len); - wc_ForceZero(der, len); } - if (der != NULL) + if (der != NULL) { + wc_ForceZero(der, len); XFREE(der, NULL, DYNAMIC_TYPE_TMP_BUFFER); + } } else { /* Decode RSA public key. */ ret = wc_RsaPublicKeyDecode(object->keyData, &idx, key, object->keyDataLen); } + /* Drop partially decoded components and the init before a retry. */ + if (ret != 0) + wc_FreeRsaKey(key); object->encoded = (ret != 0); return ret; @@ -4620,16 +4624,20 @@ static int wp11_Object_Decode_EccKey(WP11_Object* object) if (ret == 0) { /* Decode ECC private key. */ ret = wc_EccPrivateKeyDecode(der, &idx, key, len); - wc_ForceZero(der, len); } - if (der != NULL) + if (der != NULL) { + wc_ForceZero(der, len); XFREE(der, NULL, DYNAMIC_TYPE_TMP_BUFFER); + } } else { /* Decode ECC public key. */ ret = wc_EccPublicKeyDecode(object->keyData, &idx, key, object->keyDataLen); } + /* Drop partially decoded components and the init before a retry. */ + if (ret != 0) + wc_ecc_free(key); object->encoded = (ret != 0); return ret; @@ -4877,10 +4885,11 @@ static int wp11_Object_Decode_MldsaKey(WP11_Object* object) ret = MldsaKeyTryDecode(object->data.mldsaKey, WC_ML_DSA_87, der, len, object->objClass); } - wc_ForceZero(der, len); } - if (der != NULL) + if (der != NULL) { + wc_ForceZero(der, len); XFREE(der, NULL, DYNAMIC_TYPE_TMP_BUFFER); + } } else { /* Decode ML-DSA public key. */ diff --git a/tests/object_store_decode_test.c b/tests/object_store_decode_test.c index 43d034be..6432fc66 100644 --- a/tests/object_store_decode_test.c +++ b/tests/object_store_decode_test.c @@ -940,6 +940,173 @@ static void test_large_stored_value_peak(void) } #endif +#ifdef HAVE_ALLOC_TRACKING +#define MAX_TRACKED 8192 + +typedef struct TrackedAlloc { + void* ptr; + size_t sz; +} TrackedAlloc; + +static TrackedAlloc tracked[MAX_TRACKED]; +static const byte* secretNeedle = NULL; +static size_t secretNeedleSz = 0; +static int secretFreed = 0; + +static void track_add(void* ptr, size_t sz) +{ + int i; + + for (i = 0; ptr != NULL && i < MAX_TRACKED; i++) { + if (tracked[i].ptr == NULL) { + tracked[i].ptr = ptr; + tracked[i].sz = sz; + break; + } + } +} + +static int track_find(void* ptr) +{ + int i; + + for (i = 0; ptr != NULL && i < MAX_TRACKED; i++) { + if (tracked[i].ptr == ptr) + return i; + } + return -1; +} + +static int holds_secret(const byte* buf, size_t sz) +{ + size_t i; + + for (i = 0; sz >= secretNeedleSz && i <= sz - secretNeedleSz; i++) { + if (XMEMCMP(buf + i, secretNeedle, secretNeedleSz) == 0) + return 1; + } + return 0; +} + +static void* scan_malloc(size_t sz) +{ + void* ptr = malloc(sz); + + /* Zeroed so a scan only sees bytes the library wrote. */ + if (ptr != NULL) + XMEMSET(ptr, 0, sz); + track_add(ptr, sz); + return ptr; +} + +static void scan_free(void* ptr) +{ + int i = track_find(ptr); + + if (i >= 0) { + if (holds_secret((const byte*)ptr, tracked[i].sz)) + secretFreed++; + tracked[i].ptr = NULL; + } + free(ptr); +} + +static void* scan_realloc(void* ptr, size_t sz) +{ + int i = track_find(ptr); + int oldKnown = (ptr == NULL || i >= 0); + size_t oldSz = (i >= 0) ? tracked[i].sz : 0; + void* newPtr = realloc(ptr, sz); + + if (newPtr != NULL) { + if (i >= 0) + tracked[i].ptr = NULL; + if (oldKnown && sz > oldSz) + XMEMSET((byte*)newPtr + oldSz, 0, sz - oldSz); + track_add(newPtr, sz); + } + return newPtr; +} + +/* Logging in with a stored private key that fails authentication must not + * release decrypted key material to the heap. */ +static int child_no_key_material_freed(void) +{ + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + + if (wolfSSL_SetAllocators(scan_malloc, scan_free, scan_realloc) != 0) + return CHILD_SETUP; + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + if (init_slot(&slot) == CKR_OK) { + (void)open_session(slot, 1, &session); + (void)funcList->C_Finalize(NULL); + } + if (secretFreed != 0) { + fprintf(stderr, " %d freed buffer(s) held key material\n", + secretFreed); + return CHILD_FAIL; + } + return CHILD_PASS; +} + +/* Flip the last byte of the first length-prefixed array (the GCM tag). */ +static int corrupt_first_array_tag(const char* path) +{ + static byte data[MAX_FILE_SZ]; + size_t sz = 0; + word32 len; + + if (read_file(path, data, sizeof(data), &sz) != 0 || sz < 4) + return -1; + len = get_be32(data); + if (len == 0 || len > sz - 4) + return -1; + data[4 + len - 1] ^= 0x01; + return write_file(path, data, sz); +} + +static void check_failed_decrypt_scrubbed(const char* name, create_fn create, + const char* kind, + const byte* secret, size_t secretSz) +{ + CK_RV rv; + char path[256]; + char msg[128]; + int res = CHILD_SETUP; + + rv = store_objects(create); + (void)snprintf(msg, sizeof(msg), "store %s", name); + CHECK_RV(rv, msg, CKR_OK); + if (rv != CKR_OK) + return; + secretNeedle = secret; + secretNeedleSz = secretSz; + if (find_store_file(kind, path, sizeof(path)) == 0 && + corrupt_first_array_tag(path) == 0) { + res = run_in_child(child_no_key_material_freed); + } + (void)snprintf(msg, sizeof(msg), + "%s buffer is cleared after failed decrypt", name); + CHECK_TRUE(res == CHILD_PASS, msg); + cleanup_test_files(); +} + +static void test_failed_decrypt_scrubbed(void) +{ + printf("\n--- private key buffer cleared after failed decrypt ---\n"); +#ifdef HAVE_ECC + check_failed_decrypt_scrubbed("ECC private key", create_ecc_private_key, + "ecckey_priv", ecc_p256_priv, sizeof(ecc_p256_priv)); +#endif +#ifndef NO_RSA + check_failed_decrypt_scrubbed("RSA private key", create_rsa_private_key, + "rsakey_priv", rsa_2048_priv_exp, 32); +#endif +} +#endif + int main(int argc, char* argv[]) { (void)argc; @@ -964,6 +1131,9 @@ int main(int argc, char* argv[]) #ifdef WOLFPKCS11_NSS test_trust_record_length(); #endif +#ifdef HAVE_ALLOC_TRACKING + test_failed_decrypt_scrubbed(); +#endif return pkcs11_test_summary(); } From 808664451bd7a26279c50beb2abe4a9db618f3d3 Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 6 Oct 2026 11:53:02 -0700 Subject: [PATCH 12/28] F-8667 - Scrub symmetric and DH key buffers after a failed decrypt --- src/internal.c | 8 +++++++ tests/object_store_decode_test.c | 41 ++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/src/internal.c b/src/internal.c index ea0f1950..0bff0a83 100644 --- a/src/internal.c +++ b/src/internal.c @@ -5091,6 +5091,10 @@ static int wp11_Object_Decode_DhKey(WP11_Object* object) sizeof(object->slot->token.key), object->iv, sizeof(object->iv), object->devId); + if (ret != 0) { + wc_ForceZero(object->data.dhKey->key, + (word32)(object->keyDataLen - AES_BLOCK_SIZE)); + } } if (ret == 0) object->data.dhKey->len = object->keyDataLen - AES_BLOCK_SIZE; @@ -6493,6 +6497,10 @@ static int wp11_Object_Decode_SymmKey(WP11_Object* object) object->slot->token.key, sizeof(object->slot->token.key), object->iv, sizeof(object->iv), object->devId); + if (ret != 0) { + wc_ForceZero(object->data.symmKey->data, + (word32)(object->keyDataLen - AES_BLOCK_SIZE)); + } } if (ret == 0) object->data.symmKey->len = object->keyDataLen - AES_BLOCK_SIZE; diff --git a/tests/object_store_decode_test.c b/tests/object_store_decode_test.c index 6432fc66..dc3f434b 100644 --- a/tests/object_store_decode_test.c +++ b/tests/object_store_decode_test.c @@ -1105,6 +1105,46 @@ static void test_failed_decrypt_scrubbed(void) "rsakey_priv", rsa_2048_priv_exp, 32); #endif } + +#ifndef NO_AES +static byte aesKeyValue[32] = { + 0x5b, 0x1e, 0xc4, 0x27, 0x9a, 0x30, 0xe8, 0x71, + 0x0d, 0xb6, 0x43, 0xf9, 0x82, 0x2c, 0x95, 0x6e, + 0xa7, 0x14, 0xd0, 0x3b, 0x68, 0xcf, 0x51, 0x0a, + 0xe3, 0x9d, 0x26, 0x7c, 0xb1, 0x48, 0xf5, 0x13 +}; + +static CK_RV create_aes_key(CK_SESSION_HANDLE session) +{ + CK_OBJECT_CLASS keyClass = CKO_SECRET_KEY; + CK_KEY_TYPE keyType = CKK_AES; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_ATTRIBUTE tmpl[] = { + { CKA_CLASS, &keyClass, sizeof(keyClass) }, + { CKA_KEY_TYPE, &keyType, sizeof(keyType) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIVATE, &ckTrue, sizeof(ckTrue) }, + { CKA_ENCRYPT, &ckTrue, sizeof(ckTrue) }, + { CKA_VALUE, aesKeyValue, sizeof(aesKeyValue) }, + }; + + return funcList->C_CreateObject(session, tmpl, + sizeof(tmpl) / sizeof(*tmpl), &obj); +} +#endif + +static void test_failed_key_decrypt_scrubbed(void) +{ + printf("\n--- secret key storage cleared after failed decrypt ---\n"); +#ifndef NO_AES + check_failed_decrypt_scrubbed("AES key", create_aes_key, "symmkey", + aesKeyValue, sizeof(aesKeyValue)); +#endif +#ifndef NO_DH + check_failed_decrypt_scrubbed("DH private key value", create_dh_private_key, + "dhkey_priv", dh_2048_priv, sizeof(dh_2048_priv)); +#endif +} #endif int main(int argc, char* argv[]) @@ -1133,6 +1173,7 @@ int main(int argc, char* argv[]) #endif #ifdef HAVE_ALLOC_TRACKING test_failed_decrypt_scrubbed(); + test_failed_key_decrypt_scrubbed(); #endif return pkcs11_test_summary(); From 0d8ae911e4ee33195de4503b4366e9fddd7255fb Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 13 Oct 2026 13:32:45 -0700 Subject: [PATCH 13/28] Keep the old key record when encoding fails --- src/internal.c | 144 +++++++++++++++---------------- tests/object_store_decode_test.c | 90 ++++++++++++++++++- 2 files changed, 157 insertions(+), 77 deletions(-) diff --git a/src/internal.c b/src/internal.c index 0bff0a83..96a6bfed 100644 --- a/src/internal.c +++ b/src/internal.c @@ -3488,6 +3488,33 @@ static int wp11_DecryptData(byte* out, byte* data, int len, byte* key, return ret; } +/** + * Finish encoding key data into a new buffer: on success release the previous + * record, on failure drop the new buffer and put the previous record back so + * the key still decodes at the next login. + * + * @param [in, out] object Key object. + * @param [in] oldData Key data buffer before the encode. + * @param [in] oldLen Length of key data before the encode. + * @param [in] newLen Size of the buffer allocated for the encode. + * @param [in] ret Result of the encode. + */ +static void wp11_Object_EncodeDone(WP11_Object* object, unsigned char* oldData, + int oldLen, word32 newLen, int ret) +{ + if (ret == 0) { + XFREE(oldData, NULL, DYNAMIC_TYPE_TMP_BUFFER); + } + else { + if (object->keyData != oldData && object->keyData != NULL) { + wc_ForceZero(object->keyData, newLen); + XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); + } + object->keyData = oldData; + object->keyDataLen = oldLen; + } +} + /** * "Decode" the certificate. * @@ -4295,6 +4322,8 @@ static int wp11_Object_Decode_RsaKey(WP11_Object* object) static int wp11_Object_Encode_RsaKey(WP11_Object* object) { int ret; + unsigned char* oldData = object->keyData; + int oldLen = object->keyDataLen; #ifdef WOLFPKCS11_TPM ret = WP11_Object_EncodeTpmKey(object, NULL, 0); @@ -4331,7 +4360,6 @@ static int wp11_Object_Encode_RsaKey(WP11_Object* object) } if (ret == 0) { - XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); /* Allocate buffer to hold encoded key. */ object->keyData = (unsigned char*)XMALLOC(object->keyDataLen, NULL, DYNAMIC_TYPE_TMP_BUFFER); @@ -4373,13 +4401,8 @@ static int wp11_Object_Encode_RsaKey(WP11_Object* object) } } - if (ret != 0) { - if (object->keyData != NULL) - wc_ForceZero(object->keyData, object->keyDataLen); - XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); - object->keyData = NULL; - object->keyDataLen = 0; - } + wp11_Object_EncodeDone(object, oldData, oldLen, (word32)object->keyDataLen, + ret); return ret; } @@ -4655,6 +4678,8 @@ static int wp11_Object_Decode_EccKey(WP11_Object* object) static int wp11_Object_Encode_EccKey(WP11_Object* object) { int ret; + unsigned char* oldData = object->keyData; + int oldLen = object->keyDataLen; #ifdef WOLFPKCS11_TPM ret = WP11_Object_EncodeTpmKey(object, NULL, 0); @@ -4682,7 +4707,6 @@ static int wp11_Object_Encode_EccKey(WP11_Object* object) } if (ret == 0) { - XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); /* Allocate buffer to hold encoded key. */ object->keyData = (unsigned char*)XMALLOC(object->keyDataLen, NULL, DYNAMIC_TYPE_TMP_BUFFER); @@ -4720,13 +4744,8 @@ static int wp11_Object_Encode_EccKey(WP11_Object* object) } } - if (ret != 0) { - if (object->keyData != NULL) - wc_ForceZero(object->keyData, object->keyDataLen); - XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); - object->keyData = NULL; - object->keyDataLen = 0; - } + wp11_Object_EncodeDone(object, oldData, oldLen, (word32)object->keyDataLen, + ret); return ret; } @@ -4924,6 +4943,8 @@ static int wp11_Object_Decode_MldsaKey(WP11_Object* object) static int wp11_Object_Encode_MldsaKey(WP11_Object* object) { int ret; + unsigned char* oldData = object->keyData; + int oldLen = object->keyDataLen; if (object->objClass == CKO_PRIVATE_KEY) { /* Get length of encoded private key. */ @@ -4943,7 +4964,6 @@ static int wp11_Object_Encode_MldsaKey(WP11_Object* object) } if (ret == 0) { - XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); /* Allocate buffer to hold encoded key. */ object->keyData = (unsigned char*)XMALLOC(object->keyDataLen, NULL, DYNAMIC_TYPE_TMP_BUFFER); @@ -4973,13 +4993,8 @@ static int wp11_Object_Encode_MldsaKey(WP11_Object* object) } } - if (ret != 0) { - if (object->keyData != NULL) - wc_ForceZero(object->keyData, object->keyDataLen); - XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); - object->keyData = NULL; - object->keyDataLen = 0; - } + wp11_Object_EncodeDone(object, oldData, oldLen, (word32)object->keyDataLen, + ret); return ret; } @@ -5123,12 +5138,14 @@ static int wp11_Object_Decode_DhKey(WP11_Object* object) static int wp11_Object_Encode_DhKey(WP11_Object* object) { int ret = 0; + unsigned char* oldData = object->keyData; + int oldLen = object->keyDataLen; + word32 newLen = object->data.dhKey->len + AES_BLOCK_SIZE; - object->keyDataLen = object->data.dhKey->len + AES_BLOCK_SIZE; - XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); + object->keyDataLen = (int)newLen; /* Allocate buffer to hold encoded key. */ - object->keyData = (unsigned char*)XMALLOC(object->keyDataLen, NULL, - DYNAMIC_TYPE_TMP_BUFFER); + object->keyData = (unsigned char*)XMALLOC(newLen, NULL, + DYNAMIC_TYPE_TMP_BUFFER); if (object->keyData == NULL) ret = MEMORY_E; @@ -5148,14 +5165,7 @@ static int wp11_Object_Encode_DhKey(WP11_Object* object) object->keyDataLen = object->data.dhKey->len; } } - - if (ret != 0) { - if (object->keyData != NULL) - wc_ForceZero(object->keyData, object->keyDataLen); - XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); - object->keyData = NULL; - object->keyDataLen = 0; - } + wp11_Object_EncodeDone(object, oldData, oldLen, newLen, ret); return ret; } @@ -5527,6 +5537,8 @@ static int wp11_Object_Encode_MlKemKey(WP11_Object* object) { int ret; word32 keyLen = 0; + unsigned char* oldData = object->keyData; + int oldLen = object->keyDataLen; if (object->objClass == CKO_PRIVATE_KEY) { ret = wc_MlKemKey_PrivateKeySize(object->data.mlKemKey, &keyLen); @@ -5542,7 +5554,6 @@ static int wp11_Object_Encode_MlKemKey(WP11_Object* object) } if (ret == 0) { - XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); object->keyData = (unsigned char*)XMALLOC(object->keyDataLen, NULL, DYNAMIC_TYPE_TMP_BUFFER); if (object->keyData == NULL) @@ -5564,13 +5575,8 @@ static int wp11_Object_Encode_MlKemKey(WP11_Object* object) object->keyData, keyLen); } - if (ret != 0) { - if (object->keyData != NULL) - wc_ForceZero(object->keyData, object->keyDataLen); - XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); - object->keyData = NULL; - object->keyDataLen = 0; - } + wp11_Object_EncodeDone(object, oldData, oldLen, (word32)object->keyDataLen, + ret); return ret; } @@ -5749,13 +5755,14 @@ static int wp11_Object_Encode_HssKey(WP11_Object* object) { int ret; word32 pubLen = 0; + unsigned char* oldData = object->keyData; + int oldLen = object->keyDataLen; if (object->objClass != CKO_PUBLIC_KEY) return NOT_AVAILABLE_E; /* verify-only: no private-key objects */ ret = wc_LmsKey_GetPubLen(object->data.lmsKey, &pubLen); if (ret == 0) { - XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); object->keyData = (unsigned char*)XMALLOC(pubLen, NULL, DYNAMIC_TYPE_TMP_BUFFER); if (object->keyData == NULL) @@ -5767,14 +5774,7 @@ static int wp11_Object_Encode_HssKey(WP11_Object* object) if (ret == 0) object->keyDataLen = (int)pubLen; } - /* On failure release the buffer so a later store does not skip re-encoding - * (keyData != NULL) and persist an uninitialised buffer with a stale - * length. This is public key material only, so no zeroization is needed. */ - if (ret != 0) { - XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); - object->keyData = NULL; - object->keyDataLen = 0; - } + wp11_Object_EncodeDone(object, oldData, oldLen, pubLen, ret); return ret; } @@ -6194,13 +6194,14 @@ static int wp11_Object_Encode_XmssKey(WP11_Object* object) { int ret; word32 pubLen = 0; + unsigned char* oldData = object->keyData; + int oldLen = object->keyDataLen; if (object->objClass != CKO_PUBLIC_KEY) return NOT_AVAILABLE_E; ret = wc_XmssKey_GetPubLen(object->data.xmssKey, &pubLen); if (ret == 0) { - XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); object->keyData = (unsigned char*)XMALLOC(pubLen, NULL, DYNAMIC_TYPE_TMP_BUFFER); if (object->keyData == NULL) @@ -6212,14 +6213,7 @@ static int wp11_Object_Encode_XmssKey(WP11_Object* object) if (ret == 0) object->keyDataLen = (int)pubLen; } - /* On failure release the buffer so a later store does not skip re-encoding - * (keyData != NULL) and persist an uninitialised buffer with a stale - * length. This is public key material only, so no zeroization is needed. */ - if (ret != 0) { - XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); - object->keyData = NULL; - object->keyDataLen = 0; - } + wp11_Object_EncodeDone(object, oldData, oldLen, pubLen, ret); return ret; } @@ -6519,12 +6513,14 @@ static int wp11_Object_Decode_SymmKey(WP11_Object* object) static int wp11_Object_Encode_SymmKey(WP11_Object* object) { int ret = 0; + unsigned char* oldData = object->keyData; + int oldLen = object->keyDataLen; + word32 newLen = object->data.symmKey->len + AES_BLOCK_SIZE; - object->keyDataLen = object->data.symmKey->len + AES_BLOCK_SIZE; - XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); + object->keyDataLen = (int)newLen; /* Allocate buffer to hold encoded key. */ - object->keyData = (unsigned char*)XMALLOC(object->keyDataLen, NULL, - DYNAMIC_TYPE_TMP_BUFFER); + object->keyData = (unsigned char*)XMALLOC(newLen, NULL, + DYNAMIC_TYPE_TMP_BUFFER); if (object->keyData == NULL) ret = MEMORY_E; @@ -6534,17 +6530,8 @@ static int wp11_Object_Encode_SymmKey(WP11_Object* object) object->slot->token.key, sizeof(object->slot->token.key), object->iv, sizeof(object->iv), object->devId); - if (ret == 0) - object->keyDataLen = object->data.symmKey->len + AES_BLOCK_SIZE; - } - - if (ret != 0) { - if (object->keyData != NULL) - wc_ForceZero(object->keyData, object->keyDataLen); - XFREE(object->keyData, NULL, DYNAMIC_TYPE_TMP_BUFFER); - object->keyData = NULL; - object->keyDataLen = 0; } + wp11_Object_EncodeDone(object, oldData, oldLen, newLen, ret); return ret; } @@ -7211,10 +7198,12 @@ static int wp11_Object_Encode(WP11_Object* object, int protect) { int ret = 0; int encrypt = wp11_Object_IsEncrypted(object); + byte oldIv[GCM_NONCE_MID_SZ]; /* Every AES-GCM encryption under the token key needs a fresh nonce. Do * this immediately before encoding, while the plaintext is still the * source of the ciphertext that will be persisted. */ + XMEMCPY(oldIv, object->iv, sizeof(oldIv)); if (encrypt) { WP11_Lock_LockRW(&object->slot->token.rngLock); ret = wc_RNG_GenerateBlock(&object->slot->token.rng, object->iv, @@ -7223,6 +7212,9 @@ static int wp11_Object_Encode(WP11_Object* object, int protect) } if (ret == 0) ret = wp11_Object_EncodeData(object, protect); + /* The previous record stays in use when the encode fails. */ + if (ret != 0) + XMEMCPY(object->iv, oldIv, sizeof(object->iv)); return ret; } diff --git a/tests/object_store_decode_test.c b/tests/object_store_decode_test.c index dc3f434b..74fcdb2f 100644 --- a/tests/object_store_decode_test.c +++ b/tests/object_store_decode_test.c @@ -95,6 +95,7 @@ static size_t largestAlloc = 0; static size_t peakCap = 0; static size_t liveBytes = 0; static size_t peakLive = 0; +static int refuseAllocs = 0; static void* track_malloc(size_t sz) { @@ -102,7 +103,7 @@ static void* track_malloc(size_t sz) if (sz > largestAlloc) largestAlloc = sz; - if (sz >= ALLOC_REFUSE_SZ) + if (sz >= ALLOC_REFUSE_SZ || refuseAllocs) return NULL; p = (unsigned char*)malloc(sz + TRACK_HDR_SZ); if (p == NULL) @@ -1133,6 +1134,90 @@ static CK_RV create_aes_key(CK_SESSION_HANDLE session) } #endif +#ifndef NO_AES +/* Count the AES keys visible after a login and check one still encrypts. */ +static CK_RV use_stored_aes_key(CK_SESSION_HANDLE session) +{ + CK_RV rv; + CK_OBJECT_CLASS keyClass = CKO_SECRET_KEY; + CK_KEY_TYPE keyType = CKK_AES; + CK_ATTRIBUTE tmpl[] = { + { CKA_CLASS, &keyClass, sizeof(keyClass) }, + { CKA_KEY_TYPE, &keyType, sizeof(keyType) }, + }; + CK_OBJECT_HANDLE key = CK_INVALID_HANDLE; + CK_ULONG count = 0; + byte iv[16]; + byte plain[16]; + byte enc[32]; + CK_ULONG encLen = sizeof(enc); + CK_MECHANISM mech = { CKM_AES_CBC, iv, sizeof(iv) }; + + XMEMSET(iv, 0, sizeof(iv)); + XMEMSET(plain, 0, sizeof(plain)); + rv = funcList->C_FindObjectsInit(session, tmpl, + sizeof(tmpl) / sizeof(*tmpl)); + if (rv == CKR_OK) { + rv = funcList->C_FindObjects(session, &key, 1, &count); + (void)funcList->C_FindObjectsFinal(session); + } + if (rv == CKR_OK && count != 1) + rv = CKR_OBJECT_HANDLE_INVALID; + if (rv == CKR_OK) + rv = funcList->C_EncryptInit(session, &mech, key); + if (rv == CKR_OK) + rv = funcList->C_Encrypt(session, plain, sizeof(plain), enc, &encLen); + return rv; +} + +/* A logout whose re-encryption of a stored key fails must leave the stored + * record in place, so the next login still decodes it. */ +static int child_login_after_failed_encode(void) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + int res = CHILD_FAIL; + + if (wolfSSL_SetAllocators(track_malloc, track_free, track_realloc) != 0) + return CHILD_SETUP; + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = open_session(slot, 1, &session); + if (rv != CKR_OK) + return CHILD_SETUP; + refuseAllocs = 1; + (void)funcList->C_Logout(session); + refuseAllocs = 0; + rv = funcList->C_Login(session, CKU_USER, userPin, sizeof(userPin) - 1); + if (rv == CKR_OK) + rv = use_stored_aes_key(session); + if (rv == CKR_OK) + res = CHILD_PASS; + else + fprintf(stderr, " login or key use after failed encode: 0x%lx\n", + (unsigned long)rv); + (void)funcList->C_Finalize(NULL); + return res; +} + +static void test_login_after_failed_encode(void) +{ + CK_RV rv; + int res = CHILD_SETUP; + + printf("\n--- stored key survives a failed re-encryption at logout ---\n"); + rv = store_objects(create_aes_key); + CHECK_RV(rv, "store AES key", CKR_OK); + if (rv == CKR_OK) + res = run_in_child(child_login_after_failed_encode); + CHECK_TRUE(res == CHILD_PASS, "login decodes the key after a failed encode"); + cleanup_test_files(); +} +#endif + static void test_failed_key_decrypt_scrubbed(void) { printf("\n--- secret key storage cleared after failed decrypt ---\n"); @@ -1174,6 +1259,9 @@ int main(int argc, char* argv[]) #ifdef HAVE_ALLOC_TRACKING test_failed_decrypt_scrubbed(); test_failed_key_decrypt_scrubbed(); +#ifndef NO_AES + test_login_after_failed_encode(); +#endif #endif return pkcs11_test_summary(); From 885ac28fc3d126e39d8aa4f349e7a77a38d71c07 Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 6 Oct 2026 17:11:24 -0700 Subject: [PATCH 14/28] F-10400 - Authenticate stored object policy with the key encryption --- src/crypto.c | 6 + src/internal.c | 910 +++++++++++++-- tests/include.am | 8 + tests/object_policy_integrity_test.c | 1538 ++++++++++++++++++++++++++ wolfpkcs11/internal.h | 6 + 5 files changed, 2395 insertions(+), 73 deletions(-) create mode 100644 tests/object_policy_integrity_test.c diff --git a/src/crypto.c b/src/crypto.c index 92de2f52..a301c4d2 100644 --- a/src/crypto.c +++ b/src/crypto.c @@ -1189,6 +1189,12 @@ static CK_RV SetAttributeValue(WP11_Session* session, WP11_Object* obj, return CKR_ATTRIBUTE_VALUE_INVALID; else if (ret == BUFFER_E) return CKR_BUFFER_TOO_SMALL; + else if (ret == BAD_STATE_E) { + /* Logged in yet undecoded: the stored key failed to verify. */ + if (WP11_Slot_IsUserLoggedIn(WP11_Session_GetSlot(session))) + return CKR_FUNCTION_FAILED; + return CKR_USER_NOT_LOGGED_IN; + } else if (ret != 0) return CKR_FUNCTION_FAILED; } diff --git a/src/internal.c b/src/internal.c index 96a6bfed..70975161 100644 --- a/src/internal.c +++ b/src/internal.c @@ -303,6 +303,8 @@ struct WP11_Object { int keyDataLen; /* Length of encoded key data */ byte iv[GCM_NONCE_MID_SZ]; /* IV/nonce for encrypt/decrypt */ byte encoded:1; /* Key isn't in decoded form */ + byte undecoded:1; /* Loaded key data not yet decoded */ + byte unbound:1; /* Key data not bound to the policy */ #endif WP11_Session* session; /* Session object belongs to */ @@ -601,6 +603,9 @@ struct WP11_Slot { WP11_Object* discarded; /* Destroyed objects awaiting free */ WP11_Lock callLock; /* Lock for object call epochs */ byte reclaim:1; /* Unlinked objects await free */ +#ifndef WOLFPKCS11_NO_STORE + WP11_Lock storeLock; /* Held for a whole token store pass */ +#endif }; #if defined(HAVE_FIPS) && FIPS_VERSION_LT(6,0) @@ -1272,25 +1277,52 @@ static void wolfPKCS11_StoreAbortTemp(WP11_FileStoreCtx* ctx) } } -static int wolfPKCS11_StoreCommitTemp(WP11_FileStoreCtx* ctx) +#if defined(DEBUG_WOLFPKCS11) && !defined(_WIN32) && !defined(_MSC_VER) +static int wp11_storeExitAfter = 0; + +/** + * Test hook: end the process right after the given number of store renames, + * standing in for a crash part way through a store pass. + * + * @param renames [in] Renames to allow, or 0 to disable the hook. + */ +WP11_API void WP11_Test_StoreExitAfterRenames(int renames) { - int ret = 0; + wp11_storeExitAfter = renames; +} +#endif - if (ctx == NULL || ctx->has_temp == 0) { - return 0; - } +static int wolfPKCS11_StoreRename(const char* from, const char* to) +{ + int ret = 0; #if defined(_WIN32) || defined(_MSC_VER) - if (!MoveFileExA(ctx->temp_name, ctx->final_name, + if (!MoveFileExA(from, to, MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH)) { ret = READ_ONLY_E; } #else - if (rename(ctx->temp_name, ctx->final_name) != 0) { + if (rename(from, to) != 0) { ret = READ_ONLY_E; } #endif +#if defined(DEBUG_WOLFPKCS11) && !defined(_WIN32) && !defined(_MSC_VER) + if (ret == 0 && wp11_storeExitAfter > 0 && --wp11_storeExitAfter == 0) + _exit(WP11_TEST_STORE_EXIT_CODE); +#endif + + return ret; +} + +static int wolfPKCS11_StoreCommitTemp(WP11_FileStoreCtx* ctx) +{ + int ret = 0; + if (ctx == NULL || ctx->has_temp == 0) { + return 0; + } + + ret = wolfPKCS11_StoreRename(ctx->temp_name, ctx->final_name); if (ret == 0) { ctx->has_temp = 0; } @@ -1614,6 +1646,478 @@ static int wolfPKCS11_Store_Name(int type, CK_ULONG id1, CK_ULONG id2, char* nam return ret; } + +/* One token store pass commits as a unit: staged records, then a marker. */ +#define WP11_STORE_BATCH +#define WP11_STORE_STAGED_MAX (WP11_STORE_MAX_PATH + 8) + +#define WP11_BATCH_DISCARD 0 +#define WP11_BATCH_APPLY 1 +#define WP11_BATCH_CHECK 2 + +typedef struct WP11_StoreBatch { + WP11_Slot* slot; + XFILE intent; +#if defined(_WIN32) || defined(_MSC_VER) + HANDLE lockFile; +#else + int lockFd; +#endif + int locked; + int active; + int failed; + size_t dirLen; + char dir[WP11_STORE_MAX_PATH]; + char intentName[WP11_STORE_STAGED_MAX]; + char commitName[WP11_STORE_STAGED_MAX]; + char lockName[WP11_STORE_STAGED_MAX]; +} WP11_StoreBatch; + +static WP11_StoreBatch storeBatch; + +/** + * Work out the store directory and the batch file names for a token. + * + * @param [in] tokenId Id of token. + * @return 0 on success. + * @return READ_ONLY_E when the names cannot be built. + */ +static int wolfPKCS11_StoreBatchNames(int tokenId) +{ + int ret = 0; + int len; + int i; + int sep = -1; + + len = wolfPKCS11_Store_Name(WOLFPKCS11_STORE_TOKEN, (CK_ULONG)tokenId, 0, + storeBatch.dir, sizeof(storeBatch.dir)); + if (len <= 0 || len >= (int)sizeof(storeBatch.dir)) + ret = READ_ONLY_E; + for (i = 0; ret == 0 && i < len; i++) { + if (storeBatch.dir[i] == '/' || storeBatch.dir[i] == '\\') + sep = i; + } + if (ret == 0 && sep <= 0) + ret = READ_ONLY_E; + if (ret == 0) { + storeBatch.dir[sep] = '\0'; + storeBatch.dirLen = (size_t)sep; + len = XSNPRINTF(storeBatch.intentName, sizeof(storeBatch.intentName), + "%s/wp11_txn_%016lx_intent", storeBatch.dir, + (unsigned long)tokenId); + if (len <= 0 || len >= (int)sizeof(storeBatch.intentName)) + ret = READ_ONLY_E; + } + if (ret == 0) { + len = XSNPRINTF(storeBatch.commitName, sizeof(storeBatch.commitName), + "%s/wp11_txn_%016lx_commit", storeBatch.dir, + (unsigned long)tokenId); + if (len <= 0 || len >= (int)sizeof(storeBatch.commitName)) + ret = READ_ONLY_E; + } + if (ret == 0) { + len = XSNPRINTF(storeBatch.lockName, sizeof(storeBatch.lockName), + "%s/wp11_txn_%016lx_lock", storeBatch.dir, + (unsigned long)tokenId); + if (len <= 0 || len >= (int)sizeof(storeBatch.lockName)) + ret = READ_ONLY_E; + } + + return ret; +} + +/** + * Take or release the lock file that keeps other processes out of the batch + * files while a pass or a recovery runs. + * + * @param [in] lock 1 to take the lock, 0 to release it. + * @return 0 on success. + * @return READ_ONLY_E when the lock cannot be taken. + */ +static int wolfPKCS11_StoreBatchLockFile(int lock) +{ + int ret = 0; +#if defined(_WIN32) || defined(_MSC_VER) + OVERLAPPED ov; + + XMEMSET(&ov, 0, sizeof(ov)); + if (lock) { + storeBatch.lockFile = CreateFileA(storeBatch.lockName, + GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, + NULL, OPEN_ALWAYS, + FILE_ATTRIBUTE_NORMAL | FILE_FLAG_OPEN_REPARSE_POINT, NULL); + if (storeBatch.lockFile == INVALID_HANDLE_VALUE) + ret = READ_ONLY_E; + if (ret == 0 && !LockFileEx(storeBatch.lockFile, + LOCKFILE_EXCLUSIVE_LOCK, 0, 1, 0, &ov)) { + CloseHandle(storeBatch.lockFile); + ret = READ_ONLY_E; + } + storeBatch.locked = (ret == 0); + } + else if (storeBatch.locked) { + (void)UnlockFileEx(storeBatch.lockFile, 0, 1, 0, &ov); + CloseHandle(storeBatch.lockFile); + storeBatch.locked = 0; + } +#else + struct flock fl; + + XMEMSET(&fl, 0, sizeof(fl)); + fl.l_whence = SEEK_SET; + if (lock) { + storeBatch.lockFd = open(storeBatch.lockName, + O_RDWR | O_CREAT | O_NOFOLLOW, S_IRUSR | S_IWUSR); + if (storeBatch.lockFd < 0) + ret = READ_ONLY_E; + fl.l_type = F_WRLCK; + while (ret == 0 && fcntl(storeBatch.lockFd, F_SETLKW, &fl) != 0) { + if (errno != EINTR) { + close(storeBatch.lockFd); + ret = READ_ONLY_E; + } + } + storeBatch.locked = (ret == 0); + } + else if (storeBatch.locked) { + fl.l_type = F_UNLCK; + (void)fcntl(storeBatch.lockFd, F_SETLK, &fl); + close(storeBatch.lockFd); + storeBatch.locked = 0; + } +#endif + + return ret; +} + +/** + * Check that a listed name is a store file directly in the store directory. + * + * @param [in] name Name read from a batch list. + * @return 1 when acceptable, 0 otherwise. + */ +static int wolfPKCS11_StoreBatchNameOk(const char* name) +{ + size_t len = XSTRLEN(name); + size_t i; + int ok; + + ok = len > storeBatch.dirLen + 6 && + XSTRNCMP(name, storeBatch.dir, storeBatch.dirLen) == 0 && + name[storeBatch.dirLen] == '/' && + XSTRNCMP(name + storeBatch.dirLen + 1, "wp11_", 5) == 0; + for (i = storeBatch.dirLen + 1; ok && i < len; i++) { + if (name[i] == '/' || name[i] == '\\') + ok = 0; + } + + return ok; +} + +/** + * Check that a final record name can be replaced by a rename. + * + * @param [in] name Final record name. + * @return 1 when absent or a regular file, 0 otherwise. + */ +static int wolfPKCS11_StoreBatchReplaceable(const char* name) +{ +#if defined(_WIN32) || defined(_MSC_VER) + DWORD attrs = GetFileAttributesA(name); + + return attrs == INVALID_FILE_ATTRIBUTES || + (attrs & (FILE_ATTRIBUTE_DIRECTORY | + FILE_ATTRIBUTE_REPARSE_POINT)) == 0; +#else + struct stat st; + + if (lstat(name, &st) != 0) + return errno == ENOENT; + return S_ISREG(st.st_mode); +#endif +} + +/** + * Act on each staged record a batch list names: remove it, rename it into + * place, or check that it can be renamed into place. + * + * @param [in] listName Intent or commit file. + * @param [in] mode WP11_BATCH_DISCARD, WP11_BATCH_APPLY or + * WP11_BATCH_CHECK. + * @return 0 on success. + * @return READ_ONLY_E when the list is unreadable or malformed, or a record + * cannot be put in place. Discarding never fails. + */ +static int wolfPKCS11_StoreBatchApply(const char* listName, int mode) +{ + int ret = 0; + XFILE list; + XFILE staged; + size_t len; + char line[WP11_STORE_MAX_PATH + 2]; + char stagedName[WP11_STORE_STAGED_MAX]; + + list = XFOPEN(listName, "rb"); + if (list == XBADFILE) + return (mode == WP11_BATCH_DISCARD) ? 0 : READ_ONLY_E; + while (fgets(line, sizeof(line), list) != NULL) { + len = XSTRLEN(line); + /* The intent can end in a line a crash cut short; a marker cannot. */ + if (len == 0 || line[len - 1] != '\n') { + if (mode != WP11_BATCH_DISCARD) + ret = READ_ONLY_E; + continue; + } + line[len - 1] = '\0'; + len = (size_t)XSNPRINTF(stagedName, sizeof(stagedName), "%s.new", + line); + if (!wolfPKCS11_StoreBatchNameOk(line) || len >= sizeof(stagedName)) { + if (mode != WP11_BATCH_DISCARD) + ret = READ_ONLY_E; + continue; + } + if (mode == WP11_BATCH_DISCARD) { + (void)remove(stagedName); + } + else if (mode == WP11_BATCH_CHECK) { + if (!wolfPKCS11_StoreBatchReplaceable(line)) + ret = READ_ONLY_E; + } + else if (wolfPKCS11_StoreRename(stagedName, line) != 0) { + /* Already renamed by an earlier, interrupted roll forward. */ + staged = XFOPEN(stagedName, "rb"); + if (staged != XBADFILE) { + XFCLOSE(staged); + ret = READ_ONLY_E; + } + } + } + if (ferror(list) && mode != WP11_BATCH_DISCARD) + ret = READ_ONLY_E; + XFCLOSE(list); + + return ret; +} + +/** + * Finish a store pass that reached its commit point, or drop one that did + * not. Caller holds the batch locks and has set the batch names. + * + * @return 0 on success. + * @return READ_ONLY_E when a committed pass could not be finished; the + * marker is kept so a later attempt can finish it. + */ +static int wolfPKCS11_StoreBatchRecover(void) +{ + int ret = 0; + XFILE marker; + + marker = XFOPEN(storeBatch.commitName, "rb"); + if (marker != XBADFILE) { + XFCLOSE(marker); + ret = wolfPKCS11_StoreBatchApply(storeBatch.commitName, + WP11_BATCH_APPLY); + if (ret == 0) + (void)remove(storeBatch.commitName); + } + else { + (void)wolfPKCS11_StoreBatchApply(storeBatch.intentName, + WP11_BATCH_DISCARD); + (void)remove(storeBatch.intentName); + } + + return ret; +} + +/** + * Bring the store for a token to a committed state before it is read. + * + * @param [in] tokenId Id of token. + * @return 0 on success. + * @return Other value when an interrupted pass could not be finished. + */ +static int wolfPKCS11_StoreBatchCheck(int tokenId) +{ + int ret = 0; + XFILE marker; + WP11_Slot* slot; + + if (WP11_Slot_Get((CK_SLOT_ID)tokenId, &slot) != 0) + return 0; + WP11_Lock_LockRW(&slot->storeLock); + if (wolfPKCS11_StoreBatchNames(tokenId) == 0) { + if (wolfPKCS11_StoreBatchLockFile(1) == 0) { + ret = wolfPKCS11_StoreBatchRecover(); + (void)wolfPKCS11_StoreBatchLockFile(0); + } + else { + /* A committed pass that cannot be finished must not be read. */ + marker = XFOPEN(storeBatch.commitName, "rb"); + if (marker != XBADFILE) { + XFCLOSE(marker); + ret = READ_ONLY_E; + } + } + } + WP11_Lock_UnlockRW(&slot->storeLock); + + return ret; +} + +/** + * Start a store pass: records closed from now on are staged, not committed. + * On success the batch locks are held until wolfPKCS11_StoreBatchEnd. Without + * a usable store directory the pass runs unstaged and fails as before. + * + * @param [in] tokenId Id of token being stored. + * @return 0 on success. + * @return Other value when an interrupted pass could not be finished. + */ +static int wolfPKCS11_StoreBatchBegin(int tokenId) +{ + int ret = 0; +#ifndef WOLFPKCS11_NO_ENV + const char* str; +#endif +#if defined(_WIN32) || defined(_MSC_VER) + HANDLE h; +#endif + int fd; + +#ifndef WOLFPKCS11_NO_ENV + /* Storage is turned off, so the pass writes nothing. */ + str = XGETENV("WOLFPKCS11_NO_STORE"); + if (str != NULL) + return 0; +#endif + if (WP11_Slot_Get((CK_SLOT_ID)tokenId, &storeBatch.slot) != 0) + return BAD_FUNC_ARG; + WP11_Lock_LockRW(&storeBatch.slot->storeLock); + ret = wolfPKCS11_StoreBatchNames(tokenId); + if (ret == 0) + ret = wolfPKCS11_StoreEnsureDir(storeBatch.dir); + if (ret == 0) + ret = wolfPKCS11_StoreBatchLockFile(1); + if (ret == 0) + ret = wolfPKCS11_StoreBatchRecover(); + if (ret == 0) { + #if defined(_WIN32) || defined(_MSC_VER) + /* Create afresh without following a link planted at the name. */ + (void)DeleteFileA(storeBatch.intentName); + h = CreateFileA(storeBatch.intentName, GENERIC_WRITE, 0, NULL, + CREATE_NEW, FILE_ATTRIBUTE_NORMAL | FILE_FLAG_OPEN_REPARSE_POINT, + NULL); + fd = (h != INVALID_HANDLE_VALUE) ? + _open_osfhandle((intptr_t)h, _O_WRONLY | _O_BINARY) : -1; + if (h != INVALID_HANDLE_VALUE && fd < 0) + CloseHandle(h); + storeBatch.intent = (fd >= 0) ? _fdopen(fd, "wb") : XBADFILE; + if (fd >= 0 && storeBatch.intent == XBADFILE) + _close(fd); + #else + fd = open(storeBatch.intentName, + O_WRONLY | O_CREAT | O_TRUNC | O_NOFOLLOW, S_IRUSR | S_IWUSR); + storeBatch.intent = (fd >= 0) ? fdopen(fd, "wb") : XBADFILE; + if (fd >= 0 && storeBatch.intent == XBADFILE) + close(fd); + #endif + if (storeBatch.intent == XBADFILE) + ret = READ_ONLY_E; + } + if (ret == 0) { + storeBatch.active = 1; + storeBatch.failed = 0; + } + else { + (void)wolfPKCS11_StoreBatchLockFile(0); + WP11_Lock_UnlockRW(&storeBatch.slot->storeLock); + storeBatch.slot = NULL; + } + + return ret; +} + +/** + * Stage a written record as ".new" and list it in the intent file. + * + * @param [in, out] ctx File store context with a written temporary file. + * @return 0 on success. + * @return READ_ONLY_E when the record could not be staged. + */ +static int wolfPKCS11_StoreStageTemp(WP11_FileStoreCtx* ctx) +{ + int ret = 0; + int len; + char stagedName[WP11_STORE_STAGED_MAX]; + + len = XSNPRINTF(stagedName, sizeof(stagedName), "%s.new", + ctx->final_name); + if (len <= 0 || len >= (int)sizeof(stagedName)) + ret = READ_ONLY_E; + if (ret == 0) + ret = wolfPKCS11_StoreRename(ctx->temp_name, stagedName); + if (ret == 0) { + ctx->has_temp = 0; + if (fputs(ctx->final_name, storeBatch.intent) < 0 || + fputc('\n', storeBatch.intent) == EOF || + fflush(storeBatch.intent) != 0) { + ret = READ_ONLY_E; + } + } + + return ret; +} + +/** + * End a store pass. When committing, the intent becomes the commit marker and + * the staged records are renamed into place; otherwise they are removed. + * Once the marker exists the pass is committed and reported as such, and any + * record left unrenamed is finished by the next store or load. + * Releases the batch locks taken by wolfPKCS11_StoreBatchBegin. + * + * @param [in] commit 1 when every record of the pass was written. + * @return 0 when the pass is committed or there was nothing to commit. + * @return READ_ONLY_E when the pass was not committed. + */ +static int wolfPKCS11_StoreBatchEnd(int commit) +{ + int ret = 0; + + if (storeBatch.active) { + storeBatch.active = 0; + if (XFCLOSE(storeBatch.intent) != 0) + storeBatch.failed = 1; + storeBatch.intent = XBADFILE; + if (storeBatch.failed) + ret = READ_ONLY_E; + if (ret == 0 && commit) { + ret = wolfPKCS11_StoreBatchApply(storeBatch.intentName, + WP11_BATCH_CHECK); + } + if (ret == 0 && commit) { + ret = wolfPKCS11_StoreRename(storeBatch.intentName, + storeBatch.commitName); + } + if (ret == 0 && commit) { + if (wolfPKCS11_StoreBatchApply(storeBatch.commitName, + WP11_BATCH_APPLY) == 0) { + (void)remove(storeBatch.commitName); + } + } + else { + (void)wolfPKCS11_StoreBatchApply(storeBatch.intentName, + WP11_BATCH_DISCARD); + (void)remove(storeBatch.intentName); + } + (void)wolfPKCS11_StoreBatchLockFile(0); + } + if (storeBatch.slot != NULL) { + WP11_Lock_UnlockRW(&storeBatch.slot->storeLock); + storeBatch.slot = NULL; + } + + return ret; +} #endif int wolfPKCS11_Store_Remove(int type, CK_ULONG id1, CK_ULONG id2) @@ -1914,8 +2418,17 @@ void wolfPKCS11_Store_Close(void* store) } if (ctx->is_write && ctx->has_temp) { - commitRet = wolfPKCS11_StoreCommitTemp(ctx); + #ifdef WP11_STORE_BATCH + if (storeBatch.active) + commitRet = wolfPKCS11_StoreStageTemp(ctx); + else + #endif + commitRet = wolfPKCS11_StoreCommitTemp(ctx); if (commitRet != 0) { + #ifdef WP11_STORE_BATCH + if (storeBatch.active) + storeBatch.failed = 1; + #endif wolfPKCS11_StoreAbortTemp(ctx); #ifdef WOLFPKCS11_DEBUG_STORE printf("Store commit failed for %s (ret %d)\n", @@ -2986,6 +3499,8 @@ int WP11_Object_Copy(WP11_Object *src, WP11_Object *dest) OBJ_COPY_DATA(src, dest, keyData); XMEMCPY(dest->iv, src->iv, sizeof(dest->iv)); dest->encoded = src->encoded; + dest->undecoded = src->undecoded; + dest->unbound = src->unbound; #endif dest->objClass = src->objClass; dest->keyGenMech = src->keyGenMech; @@ -3431,11 +3946,14 @@ int WP11_Object_Copy(WP11_Object *src, WP11_Object *dest) * @param [in] keySz Length of AES key in bytes. * @param [in] iv IV/nonce. * @param [in] ivSz Length of IV in bytes. + * @param [in] aad Additional authenticated data. May be NULL. + * @param [in] aadSz Length of additional authenticated data in bytes. * @return 0 on success. * @return -ve on failure. */ static int wp11_EncryptData(byte* out, byte* data, int len, byte* key, - int keySz, byte* iv, int ivSz, int devId) + int keySz, byte* iv, int ivSz, const byte* aad, + word32 aadSz, int devId) { Aes aes; int ret; @@ -3446,7 +3964,7 @@ static int wp11_EncryptData(byte* out, byte* data, int len, byte* key, } if (ret == 0) { ret = wc_AesGcmEncrypt(&aes, out, data, len, iv, ivSz, out + len, - AES_BLOCK_SIZE, NULL, 0); + AES_BLOCK_SIZE, aad, aadSz); } wc_AesFree(&aes); @@ -3465,12 +3983,15 @@ static int wp11_EncryptData(byte* out, byte* data, int len, byte* key, * @param [in] keySz Length of AES key in bytes. * @param [in] iv IV/nonce. * @param [in] ivSz Length of IV in bytes. + * @param [in] aad Additional authenticated data. May be NULL. + * @param [in] aadSz Length of additional authenticated data in bytes. * @return 0 on success. * @return AES_GCM_AUTH_E when encrypted data could not be verified. * @return Other -ve on failure. */ static int wp11_DecryptData(byte* out, byte* data, int len, byte* key, - int keySz, byte* iv, int ivSz, int devId) + int keySz, byte* iv, int ivSz, const byte* aad, + word32 aadSz, int devId) { Aes aes; int ret; @@ -3481,13 +4002,115 @@ static int wp11_DecryptData(byte* out, byte* data, int len, byte* key, } if (ret == 0) { ret = wc_AesGcmDecrypt(&aes, out, data, len, iv, ivSz, data + len, - AES_BLOCK_SIZE, NULL, 0); + AES_BLOCK_SIZE, aad, aadSz); } wc_AesFree(&aes); return ret; } +/* Set by the SO, who never holds the user's key, or by the library itself. */ +#ifdef WOLFPKCS11_TPM +#define WP11_FLAG_UNBOUND (WP11_FLAG_TRUSTED | WP11_FLAG_TPM) +#else +#define WP11_FLAG_UNBOUND WP11_FLAG_TRUSTED +#endif + +#define WP11_POLICY_AAD_VER 1 +#define WP11_POLICY_AAD_SZ (1 + 3 * sizeof(CK_ULONG) + 1 + sizeof(word32)) + +/** + * Serialize the object attributes that govern use of its key material. + * + * @param [in] object Key object. + * @param [out] aad Buffer of WP11_POLICY_AAD_SZ bytes. + */ +static void wp11_Object_PolicyAad(WP11_Object* object, byte* aad) +{ + CK_ULONG vals[3]; + word32 flags = object->opFlag & ~(word32)WP11_FLAG_UNBOUND; + int idx = 0; + int i; + int j; + + vals[0] = object->objClass; + vals[1] = object->type; + vals[2] = object->keyGenMech; + aad[idx++] = WP11_POLICY_AAD_VER; + for (i = 0; i < 3; i++) { + for (j = (int)sizeof(CK_ULONG) - 1; j >= 0; j--) + aad[idx++] = (byte)(vals[i] >> (j * 8)); + } + aad[idx++] = (byte)object->local; + for (j = (int)sizeof(word32) - 1; j >= 0; j--) + aad[idx++] = (byte)(flags >> (j * 8)); +} + +/** + * Encrypt key material with the token key, bound to the object's policy. + * + * @param [in, out] object Key object. + * @param [out] out Buffer for encrypted data and tag. + * @param [in] data Key material. + * @param [in] len Length of key material. + * @return 0 on success. + * @return -ve on failure. + */ +static int wp11_Object_EncryptKeyData(WP11_Object* object, byte* out, + byte* data, int len) +{ + int ret; + byte aad[WP11_POLICY_AAD_SZ]; + + wp11_Object_PolicyAad(object, aad); + ret = wp11_EncryptData(out, data, len, object->slot->token.key, + sizeof(object->slot->token.key), object->iv, + sizeof(object->iv), aad, sizeof(aad), + object->devId); + if (ret == 0) + object->unbound = 0; + + return ret; +} + +/** + * Decrypt key material with the token key, verifying the object's policy. + * Key data stored before the policy was bound is accepted and marked so it is + * bound when next stored. + * + * @param [in, out] object Key object. + * @param [out] out Buffer for decrypted key material. + * @param [in] data Encrypted data and tag. + * @param [in] len Length of encrypted data. + * @return 0 on success. + * @return AES_GCM_AUTH_E when encrypted data could not be verified. + * @return Other -ve on failure. + */ +static int wp11_Object_DecryptKeyData(WP11_Object* object, byte* out, + byte* data, int len) +{ + int ret; + byte aad[WP11_POLICY_AAD_SZ]; + + wp11_Object_PolicyAad(object, aad); + ret = wp11_DecryptData(out, data, len, object->slot->token.key, + sizeof(object->slot->token.key), object->iv, + sizeof(object->iv), aad, sizeof(aad), + object->devId); + if (ret == AES_GCM_AUTH_E) { + ret = wp11_DecryptData(out, data, len, object->slot->token.key, + sizeof(object->slot->token.key), object->iv, + sizeof(object->iv), NULL, 0, object->devId); + if (ret == 0) + object->unbound = 1; + } + + return ret; +} + +static int wp11_Object_IsEncrypted(WP11_Object* object); +static int wp11_Object_Encode(WP11_Object* object, int protect); + /** * Finish encoding key data into a new buffer: on success release the previous * record, on failure drop the new buffer and put the previous record back so @@ -4283,10 +4906,7 @@ static int wp11_Object_Decode_RsaKey(WP11_Object* object) ret = MEMORY_E; } if (ret == 0) { - ret = wp11_DecryptData(der, object->keyData, len, - object->slot->token.key, - sizeof(object->slot->token.key), object->iv, - sizeof(object->iv), object->devId); + ret = wp11_Object_DecryptKeyData(object, der, object->keyData, len); } if (ret == 0) { /* Decode RSA private key. */ @@ -4383,10 +5003,8 @@ static int wp11_Object_Encode_RsaKey(WP11_Object* object) ret = wc_RsaKeyToDer(object->data.rsaKey, object->keyData, object->keyDataLen); if (ret >= 0) { - ret = wp11_EncryptData(object->keyData, object->keyData, ret, - object->slot->token.key, - sizeof(object->slot->token.key), object->iv, - sizeof(object->iv), object->devId); + ret = wp11_Object_EncryptKeyData(object, object->keyData, + object->keyData, ret); } #else ret = NOT_COMPILED_IN; @@ -4639,10 +5257,7 @@ static int wp11_Object_Decode_EccKey(WP11_Object* object) ret = MEMORY_E; } if (ret == 0) { - ret = wp11_DecryptData(der, object->keyData, len, - object->slot->token.key, - sizeof(object->slot->token.key), object->iv, - sizeof(object->iv), object->devId); + ret = wp11_Object_DecryptKeyData(object, der, object->keyData, len); } if (ret == 0) { /* Decode ECC private key. */ @@ -4729,10 +5344,8 @@ static int wp11_Object_Encode_EccKey(WP11_Object* object) ret = wc_EccPrivateKeyToDer(object->data.ecKey, object->keyData, object->keyDataLen); if (ret >= 0) { - ret = wp11_EncryptData(object->keyData, object->keyData, ret, - object->slot->token.key, - sizeof(object->slot->token.key), object->iv, - sizeof(object->iv), object->devId); + ret = wp11_Object_EncryptKeyData(object, object->keyData, + object->keyData, ret); } } else if (ret == 0 && object->objClass == CKO_PUBLIC_KEY) { @@ -4887,10 +5500,7 @@ static int wp11_Object_Decode_MldsaKey(WP11_Object* object) ret = MEMORY_E; } if (ret == 0) { - ret = wp11_DecryptData(der, object->keyData, len, - object->slot->token.key, - sizeof(object->slot->token.key), object->iv, - sizeof(object->iv), object->devId); + ret = wp11_Object_DecryptKeyData(object, der, object->keyData, len); } if (ret == 0) { /* Decode ML-DSA private key. */ @@ -4977,10 +5587,8 @@ static int wp11_Object_Encode_MldsaKey(WP11_Object* object) object->keyData, object->keyDataLen); if (ret >= 0) { - ret = wp11_EncryptData(object->keyData, object->keyData, ret, - object->slot->token.key, - sizeof(object->slot->token.key), object->iv, - sizeof(object->iv), object->devId); + ret = wp11_Object_EncryptKeyData(object, object->keyData, + object->keyData, ret); } } else if (ret == 0 && object->objClass == CKO_PUBLIC_KEY) { @@ -5100,12 +5708,8 @@ static int wp11_Object_Decode_DhKey(WP11_Object* object) else if (object->keyDataLen - AES_BLOCK_SIZE > WP11_MAX_DH_KEY_SZ) ret = BUFFER_E; if (ret == 0) { - ret = wp11_DecryptData(object->data.dhKey->key, object->keyData, - object->keyDataLen - AES_BLOCK_SIZE, - object->slot->token.key, - sizeof(object->slot->token.key), - object->iv, sizeof(object->iv), - object->devId); + ret = wp11_Object_DecryptKeyData(object, object->data.dhKey->key, + object->keyData, object->keyDataLen - AES_BLOCK_SIZE); if (ret != 0) { wc_ForceZero(object->data.dhKey->key, (word32)(object->keyDataLen - AES_BLOCK_SIZE)); @@ -5151,11 +5755,9 @@ static int wp11_Object_Encode_DhKey(WP11_Object* object) if (ret == 0) { if (object->objClass == CKO_PRIVATE_KEY) { - ret = wp11_EncryptData(object->keyData, object->data.dhKey->key, - object->data.dhKey->len, - object->slot->token.key, - sizeof(object->slot->token.key), object->iv, - sizeof(object->iv), object->devId); + ret = wp11_Object_EncryptKeyData(object, object->keyData, + object->data.dhKey->key, + object->data.dhKey->len); if (ret == 0) object->keyDataLen = object->data.dhKey->len + AES_BLOCK_SIZE; } @@ -5480,10 +6082,7 @@ static int wp11_Object_Decode_MlKemKey(WP11_Object* object) ret = MEMORY_E; } if (ret == 0) { - ret = wp11_DecryptData(der, object->keyData, len, - object->slot->token.key, - sizeof(object->slot->token.key), object->iv, - sizeof(object->iv), object->devId); + ret = wp11_Object_DecryptKeyData(object, der, object->keyData, len); } if (ret == 0) { ret = MlKemKeyTryDecode(object->data.mlKemKey, WC_ML_KEM_512, @@ -5564,10 +6163,8 @@ static int wp11_Object_Encode_MlKemKey(WP11_Object* object) ret = wc_MlKemKey_EncodePrivateKey(object->data.mlKemKey, object->keyData, keyLen); if (ret == 0) { - ret = wp11_EncryptData(object->keyData, object->keyData, keyLen, - object->slot->token.key, - sizeof(object->slot->token.key), object->iv, - sizeof(object->iv), object->devId); + ret = wp11_Object_EncryptKeyData(object, object->keyData, + object->keyData, keyLen); } } else if (ret == 0 && object->objClass == CKO_PUBLIC_KEY) { @@ -6486,11 +7083,9 @@ static int wp11_Object_Decode_SymmKey(WP11_Object* object) (word32)(object->keyDataLen - AES_BLOCK_SIZE) > WP11_MAX_SYM_KEY_SZ) ret = BUFFER_E; if (ret == 0) { - ret = wp11_DecryptData(object->data.symmKey->data, object->keyData, - object->keyDataLen - AES_BLOCK_SIZE, - object->slot->token.key, - sizeof(object->slot->token.key), object->iv, - sizeof(object->iv), object->devId); + ret = wp11_Object_DecryptKeyData(object, object->data.symmKey->data, + object->keyData, + object->keyDataLen - AES_BLOCK_SIZE); if (ret != 0) { wc_ForceZero(object->data.symmKey->data, (word32)(object->keyDataLen - AES_BLOCK_SIZE)); @@ -6525,11 +7120,9 @@ static int wp11_Object_Encode_SymmKey(WP11_Object* object) ret = MEMORY_E; if (ret == 0) { - ret = wp11_EncryptData(object->keyData, object->data.symmKey->data, - object->data.symmKey->len, - object->slot->token.key, - sizeof(object->slot->token.key), object->iv, - sizeof(object->iv), object->devId); + ret = wp11_Object_EncryptKeyData(object, object->keyData, + object->data.symmKey->data, + object->data.symmKey->len); } wp11_Object_EncodeDone(object, oldData, oldLen, newLen, ret); @@ -6796,6 +7389,8 @@ static int wp11_Object_Load(WP11_Object* object, int tokenId, int objId) } } } + if (ret == 0 && wp11_Object_IsEncrypted(object)) + object->undecoded = 1; return ret; } @@ -6913,10 +7508,15 @@ static int wp11_Object_Store_Object(WP11_Object* object, int tokenId, int objId) */ static int wp11_Object_Store(WP11_Object* object, int tokenId, int objId) { - int ret; + int ret = 0; + /* Bind decoded key data to the current policy before writing anything, so + * a failure leaves the stored record unchanged. */ + if (object->unbound && !object->encoded && wp11_Object_IsEncrypted(object)) + ret = wp11_Object_Encode(object, 0); /* Open access to key object. */ - ret = wp11_Object_Store_Object(object, tokenId, objId); + if (ret == 0) + ret = wp11_Object_Store_Object(object, tokenId, objId); if (ret == 0) { if (object->objClass == CKO_CERTIFICATE) { @@ -7066,6 +7666,8 @@ static int wp11_Object_Decode(WP11_Object* object) } } + if (ret == 0) + object->undecoded = 0; /* Authentication failure means this object isn't for this user. */ if (ret == AES_GCM_AUTH_E) ret = 0; @@ -7267,6 +7869,7 @@ static int wp11_Object_ReKey(WP11_Object* object, byte* oldKey, byte* newKey, int ret = 0; int len = 0; byte* plain = NULL; + byte aad[WP11_POLICY_AAD_SZ]; /* Key data is the encrypted payload followed by the AES-GCM tag. */ if (object->keyDataLen < AES_BLOCK_SIZE) @@ -7281,9 +7884,18 @@ static int wp11_Object_ReKey(WP11_Object* object, byte* oldKey, byte* newKey, ret = MEMORY_E; } if (ret == 0) { + wp11_Object_PolicyAad(object, aad); ret = wp11_DecryptData(plain, object->keyData, len, oldKey, sizeof(object->slot->token.key), object->iv, - sizeof(object->iv), object->devId); + sizeof(object->iv), aad, sizeof(aad), + object->devId); + /* Key data stored before policy binding has no AAD. */ + if (ret == AES_GCM_AUTH_E) { + ret = wp11_DecryptData(plain, object->keyData, len, oldKey, + sizeof(object->slot->token.key), + object->iv, sizeof(object->iv), NULL, 0, + object->devId); + } } if (ret == 0) { WP11_Lock_LockRW(&object->slot->token.rngLock); @@ -7294,7 +7906,8 @@ static int wp11_Object_ReKey(WP11_Object* object, byte* oldKey, byte* newKey, if (ret == 0) { ret = wp11_EncryptData(reKey->keyData, plain, len, newKey, sizeof(object->slot->token.key), reKey->iv, - sizeof(reKey->iv), object->devId); + sizeof(reKey->iv), aad, sizeof(aad), + object->devId); } if (plain != NULL) { @@ -7833,9 +8446,16 @@ static int wp11_Token_Load(WP11_Slot* slot, int tokenId, WP11_Token* token) int objCnt = 0; word32 len; + ret = 0; +#ifdef WP11_STORE_BATCH + /* Finish or drop a store pass that a crash interrupted. */ + ret = wolfPKCS11_StoreBatchCheck(tokenId); +#endif /* Open access to token object. */ - ret = wp11_storage_open_readonly(WOLFPKCS11_STORE_TOKEN, tokenId, 0, - &storage); + if (ret == 0) { + ret = wp11_storage_open_readonly(WOLFPKCS11_STORE_TOKEN, tokenId, 0, + &storage); + } if (ret == 0) { /* Read label for token. (32) */ ret = wp11_storage_read_string(storage, token->label, @@ -7990,7 +8610,7 @@ static int wp11_Token_Load(WP11_Slot* slot, int tokenId, WP11_Token* token) * @return BUFFER_E when storing fails. * @return NOT_AVAILABLE_E when unable to write data. */ -static int wp11_Token_Store(WP11_Token* token, int tokenId) +static int wp11_Token_StorePass(WP11_Token* token, int tokenId) { int ret; int i; @@ -8123,6 +8743,34 @@ static int wp11_Token_Store(WP11_Token* token, int tokenId) return ret; } + +/** + * Store the token and its objects as one pass that commits as a unit. + * + * @param [in] token Token object. + * @param [in] tokenId Id of token. + * @return 0 on success. + * @return Other value when the pass did not commit. + */ +static int wp11_Token_Store(WP11_Token* token, int tokenId) +{ + int ret; +#ifdef WP11_STORE_BATCH + int endRet; + + ret = wolfPKCS11_StoreBatchBegin(tokenId); + if (ret == 0) { + ret = wp11_Token_StorePass(token, tokenId); + endRet = wolfPKCS11_StoreBatchEnd(ret == 0); + if (ret == 0) + ret = endRet; + } +#else + ret = wp11_Token_StorePass(token, tokenId); +#endif + + return ret; +} #endif /* !WOLFPKCS11_NO_STORE */ /** @@ -8242,6 +8890,9 @@ static void wp11_Slot_Final(WP11_Slot* slot) wp11_Token_Final(&slot->token); #ifdef WOLFPKCS11_TPM wp11_TpmFinal(slot); +#endif +#ifndef WOLFPKCS11_NO_STORE + WP11_Lock_Free(&slot->storeLock); #endif WP11_Lock_Free(&slot->callLock); WP11_Lock_Free(&slot->lock); @@ -8277,6 +8928,15 @@ static int wp11_Slot_Init(WP11_Slot* slot, int id) WP11_Lock_Free(&slot->lock); } } +#ifndef WOLFPKCS11_NO_STORE + if (ret == 0) { + ret = WP11_Lock_Init(&slot->storeLock); + if (ret != 0) { + WP11_Lock_Free(&slot->callLock); + WP11_Lock_Free(&slot->lock); + } + } +#endif if (ret == 0) { #if defined(WOLFPKCS11_TPM) ret = wp11_TpmInit(slot); @@ -8844,6 +9504,9 @@ int WP11_Slot_TokenReset(WP11_Slot* slot, char* pin, int pinLen, char* label) { int ret; WP11_Token* token; +#ifdef WP11_STORE_BATCH + char emptyLabel[LABEL_SZ]; +#endif WP11_Lock_LockRW(&slot->lock); /* Zeroizes token. */ @@ -8858,8 +9521,25 @@ int WP11_Slot_TokenReset(WP11_Slot* slot, char* pin, int pinLen, char* label) WP11_Lock_UnlockRW(&slot->lock); /* Locking used in setting SO PIN. */ - if (ret == 0) + if (ret == 0) { ret = WP11_Slot_SetSOPin(slot, pin, pinLen); +#ifdef WP11_STORE_BATCH + /* Only a staged store pass is known to leave storage unchanged. */ + if (ret != 0) { + /* Reset not stored: restore the token that is still in storage. */ + XMEMSET(emptyLabel, 0, sizeof(emptyLabel)); + WP11_Lock_LockRW(&slot->lock); + wp11_Token_Final(token); + if (wp11_Token_Init(token, emptyLabel) == 0) { + WP11_Lock_UnlockRW(&slot->lock); + (void)wp11_Slot_Load(slot, (int)slot->id); + } + else { + WP11_Lock_UnlockRW(&slot->lock); + } + } +#endif + } return ret; } @@ -9625,9 +10305,21 @@ int WP11_Slot_SetSOPin(WP11_Slot* slot, char* pin, int pinLen) { int ret = 0; WP11_Token* token; +#ifdef WP11_STORE_BATCH + byte oldPin[PIN_HASH_SZ]; + byte oldPinSeed[PIN_SEED_SZ]; + int oldPinLen; + int oldFlags; +#endif WP11_Lock_LockRW(&slot->lock); token = &slot->token; +#ifdef WP11_STORE_BATCH + XMEMCPY(oldPin, token->soPin, sizeof(oldPin)); + XMEMCPY(oldPinSeed, token->soPinSeed, sizeof(oldPinSeed)); + oldPinLen = token->soPinLen; + oldFlags = token->tokenFlags; +#endif /* New seed each time. */ WP11_Lock_LockRW(&slot->token.rngLock); ret = wc_RNG_GenerateBlock(&slot->token.rng, token->soPinSeed, @@ -9646,9 +10338,22 @@ int WP11_Slot_SetSOPin(WP11_Slot* slot, char* pin, int pinLen) token->tokenFlags |= WP11_TOKEN_FLAG_SO_PIN_SET; #ifndef WOLFPKCS11_NO_STORE ret = wp11_Token_Store(token, (int)slot->id); + #ifdef WP11_STORE_BATCH + if (ret != 0 && (oldFlags & WP11_TOKEN_FLAG_SO_PIN_SET)) { + /* Keep the PIN that is still in storage in effect. */ + XMEMCPY(token->soPin, oldPin, sizeof(oldPin)); + XMEMCPY(token->soPinSeed, oldPinSeed, sizeof(oldPinSeed)); + token->soPinLen = oldPinLen; + token->tokenFlags = oldFlags; + } + #endif #endif } WP11_Lock_UnlockRW(&slot->lock); +#ifdef WP11_STORE_BATCH + wc_ForceZero(oldPin, sizeof(oldPin)); + wc_ForceZero(oldPinSeed, sizeof(oldPinSeed)); +#endif return ret; } @@ -9757,6 +10462,7 @@ static int wp11_Slot_SetUserPin(WP11_Slot* slot, char* oldPin, int oldPinLen, #ifndef WOLFPKCS11_NO_STORE WP11_ReKeyData* reKey = NULL; int reKeyCnt = 0; + WP11_Object* object; #endif #ifndef WOLFPKCS11_NO_TIME time_t now = 0; @@ -9866,6 +10572,15 @@ static int wp11_Slot_SetUserPin(WP11_Slot* slot, char* oldPin, int oldPinLen, sizeof(d->prev.seed)) != 0) { ret = BAD_STATE_E; } + /* Bind pending policy changes under the old key first, so every + * record re-keys from key data that verifies. */ + for (object = token->object; ret == 0 && object != NULL; + object = object->next) { + if (object->unbound && !object->encoded && + wp11_Object_IsEncrypted(object)) { + ret = wp11_Object_Encode(object, 0); + } + } if (ret == 0) { ret = wp11_Token_ReKeyPrepare(token, d->oldKey, d->key, &reKey, &reKeyCnt); @@ -9890,9 +10605,11 @@ static int wp11_Slot_SetUserPin(WP11_Slot* slot, char* oldPin, int oldPinLen, if (ret != 0) { wp11_Token_ReKeySwap(reKey, reKeyCnt); wp11_Token_RestoreUserPinState(token, &d->prev); + #ifndef WP11_STORE_BATCH /* Best effort to put the previous state back in storage. */ (void)wp11_Token_Store(token, (int)slot->id); token->objCnt = d->prev.objCnt; + #endif } #endif } @@ -14482,6 +15199,40 @@ static int WP11_Object_SetEndDate(WP11_Object* object, char* endDate, int len) return ret; } +#ifndef WOLFPKCS11_NO_STORE +/** + * Track a change to a token object's policy so its key data is bound to the + * new policy when next stored. The change is undone when the key data is not + * decoded, as it could not be bound. Caller holds the object's lock when it + * is on a token. + * + * @param object [in] Object object. + * @param prevClass [in] Object class before the change. + * @param prevFlag [in] Operational flags before the change. + * @return BAD_STATE_E when the key data is not decoded. + * 0 on success. + */ +static int wp11_Object_PolicyChanged(WP11_Object* object, + CK_OBJECT_CLASS prevClass, word32 prevFlag) +{ + int ret = 0; + + if (object->onToken && wp11_Object_IsEncrypted(object) && + (object->objClass != prevClass || + ((object->opFlag ^ prevFlag) & ~(word32)WP11_FLAG_UNBOUND) != 0)) { + if (object->encoded || object->undecoded) { + object->objClass = prevClass; + object->opFlag = prevFlag; + ret = BAD_STATE_E; + } + else { + object->unbound = 1; + } + } + + return ret; +} +#endif /** * Set an attribute against the object. @@ -14492,15 +15243,24 @@ static int WP11_Object_SetEndDate(WP11_Object* object, char* endDate, int len) * @param len [in] Length of attribute data in bytes. * @return BUFFER_E when data is too small. * BAD_FUNC_ARG when type is not supported with object. + * BAD_STATE_E when a policy change cannot be bound to the stored key. * 0 on success. */ int WP11_Object_SetAttr(WP11_Object* object, CK_ATTRIBUTE_TYPE type, byte* data, CK_ULONG len) { int ret = 0; +#ifndef WOLFPKCS11_NO_STORE + CK_OBJECT_CLASS prevClass; + word32 prevFlag; +#endif if (object->onToken) WP11_Lock_LockRW(object->lock); +#ifndef WOLFPKCS11_NO_STORE + prevClass = object->objClass; + prevFlag = object->opFlag; +#endif switch (type) { case CKA_CLASS: @@ -14875,6 +15635,10 @@ int WP11_Object_SetAttr(WP11_Object* object, CK_ATTRIBUTE_TYPE type, byte* data, ret = BAD_FUNC_ARG; break; } +#ifndef WOLFPKCS11_NO_STORE + if (ret == 0) + ret = wp11_Object_PolicyChanged(object, prevClass, prevFlag); +#endif if (object->onToken) WP11_Lock_UnlockRW(object->lock); diff --git a/tests/include.am b/tests/include.am index ed95d9e3..44afb51b 100644 --- a/tests/include.am +++ b/tests/include.am @@ -325,6 +325,12 @@ noinst_PROGRAMS += tests/object_store_decode_test tests_object_store_decode_test_SOURCES = tests/object_store_decode_test.c tests_object_store_decode_test_LDADD = +check_PROGRAMS += tests/object_policy_integrity_test +noinst_PROGRAMS += tests/object_policy_integrity_test +tests_object_policy_integrity_test_SOURCES = \ + tests/object_policy_integrity_test.c +tests_object_policy_integrity_test_LDADD = + if BUILD_STATIC tests_pkcs11test_LDADD += src/libwolfpkcs11.la tests_pkcs11mtt_LDADD += src/libwolfpkcs11.la @@ -391,6 +397,7 @@ tests_nss_configdir_prefix_test_LDADD += src/libwolfpkcs11.la tests_logout_retire_test_LDADD += src/libwolfpkcs11.la tests_login_pin_state_test_LDADD += src/libwolfpkcs11.la tests_object_store_decode_test_LDADD += src/libwolfpkcs11.la +tests_object_policy_integrity_test_LDADD += src/libwolfpkcs11.la else tests_object_id_uniqueness_test_LDADD += src/libwolfpkcs11.la tests_empty_pin_store_test_LDADD += src/libwolfpkcs11.la @@ -449,6 +456,7 @@ tests_nss_configdir_prefix_test_LDADD += src/libwolfpkcs11.la tests_logout_retire_test_LDADD += src/libwolfpkcs11.la tests_login_pin_state_test_LDADD += src/libwolfpkcs11.la tests_object_store_decode_test_LDADD += src/libwolfpkcs11.la +tests_object_policy_integrity_test_LDADD += src/libwolfpkcs11.la endif EXTRA_DIST += tests/unit.h \ diff --git a/tests/object_policy_integrity_test.c b/tests/object_policy_integrity_test.c new file mode 100644 index 00000000..afda742f --- /dev/null +++ b/tests/object_policy_integrity_test.c @@ -0,0 +1,1538 @@ +/* object_policy_integrity_test.c + * + * Copyright (C) 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 policy attributes of a protected token key are only honored when they + * match the policy the key material was stored under. Legitimate attribute + * updates, PIN changes and stores written before the policy was bound must + * keep working. Each reload runs in a child process. + */ + +#ifdef HAVE_CONFIG_H + #include +#endif + +#include +#include +#include + +#ifndef WOLFSSL_USER_SETTINGS + #include +#endif +#include +#include +#include +#include + +#ifndef WOLFPKCS11_USER_SETTINGS + #include +#endif +#include +#include + +#ifndef HAVE_PKCS11_STATIC + #include +#endif + +#include "testdata.h" +#include "pkcs11_test_util.h" + +#if !defined(WOLFPKCS11_NO_STORE) && !defined(WOLFPKCS11_TPM_STORE) && \ + !defined(WOLFPKCS11_CUSTOM_STORE) && !defined(WOLFPKCS11_NO_ENV) && \ + !defined(_WIN32) && !defined(NO_AES) && defined(HAVE_AESGCM) && \ + defined(HAVE_AES_CBC) + +#include +#include +#include +#include + +#define TEST_DIR "./store/object_policy_integrity_test" +#define MAX_OBJ_ID 8 +#define MAX_FILE_SZ 16384 + +/* Offsets of the class and operational flags in a stored object record. */ +#define OBJ_CLASS_OFF (12 + sizeof(CK_ULONG)) +#define OBJ_FLAGS_OFF (12 + 3 * sizeof(CK_ULONG) + 1 + 1 + 4) +#define OBJ_IV_SZ 12 +#define TAG_SZ 16 +#define BLOCK_SZ 16 + +#define CHILD_PASS 0 +#define CHILD_FAIL 1 +#define CHILD_SETUP 2 + +static byte soPin[] = "password123456"; +static byte userPin[] = "wolfpkcs11-test"; +static CK_BBOOL ckTrue = CK_TRUE; +static CK_BBOOL ckFalse = CK_FALSE; + +static const char* storeKinds[] = { + "obj", "data", "symmkey", "rsakey_priv", "rsakey_pub", "ecckey_priv", + "ecckey_pub", "dhkey_priv", "dhkey_pub", "cert", "trust", "mldsakey_priv", + "mldsakey_pub", "mlkemkey_priv", "mlkemkey_pub" +}; + +static byte aesKeyValue[32] = { + 0x2f, 0x91, 0x0c, 0x6a, 0xd3, 0x48, 0x75, 0xbe, + 0x13, 0xe0, 0x5c, 0x87, 0x29, 0xfa, 0x64, 0x0b, + 0xc8, 0x3d, 0x9e, 0x51, 0x06, 0xb7, 0x7a, 0x22, + 0xe4, 0x1f, 0x98, 0x6d, 0x30, 0xab, 0x55, 0xc1 +}; +static byte refPlain[BLOCK_SZ] = { + 0x70, 0x6f, 0x6c, 0x69, 0x63, 0x79, 0x2d, 0x72, + 0x65, 0x66, 0x2d, 0x62, 0x6c, 0x6f, 0x63, 0x6b +}; +static byte refCipher[BLOCK_SZ]; + +/* Parameters of the current check, read by the child process. */ +static CK_OBJECT_CLASS keyClass = CKO_SECRET_KEY; +static CK_ATTRIBUTE_TYPE secretAttr = CKA_VALUE; +static const byte* secretNeedle = NULL; +static size_t secretNeedleSz = 0; +static int checkAesUse = 0; +static CK_MECHANISM_TYPE signMech = CKM_VENDOR_DEFINED; +static CK_BBOOL expSensitive = CK_TRUE; +static CK_BBOOL expExtractable = CK_FALSE; +static CK_BBOOL expDecrypt = CK_TRUE; + +static void store_path(char* path, size_t sz, const char* kind, int objId) +{ + if (objId < 0) { + (void)snprintf(path, sz, "%s/wp11_%s_%016lx", TEST_DIR, kind, 1UL); + } + else { + (void)snprintf(path, sz, "%s/wp11_%s_%016lx_%016lx", TEST_DIR, kind, + 1UL, (unsigned long)objId); + } +} + +static void cleanup_test_files(void) +{ + char path[256]; + size_t k; + int i; + + for (k = 0; k < sizeof(storeKinds) / sizeof(*storeKinds); k++) { + for (i = 0; i < MAX_OBJ_ID; i++) { + store_path(path, sizeof(path), storeKinds[k], i); + (void)remove(path); + } + } + store_path(path, sizeof(path), "token", -1); + (void)remove(path); +} + +/* Locate the stored file of the given kind for its single object. */ +static int find_store_file(const char* kind, char* path, size_t sz) +{ + FILE* f; + int i; + + for (i = 0; i < MAX_OBJ_ID; i++) { + store_path(path, sz, kind, i); + f = fopen(path, "rb"); + if (f != NULL) { + fclose(f); + return 0; + } + } + return -1; +} + +static int read_file(const char* path, byte* data, size_t sz, size_t* readSz) +{ + FILE* f = fopen(path, "rb"); + + if (f == NULL) + return -1; + *readSz = fread(data, 1, sz, f); + if (ferror(f) || !feof(f)) { + fclose(f); + return -1; + } + fclose(f); + return 0; +} + +static int write_file(const char* path, const byte* data, size_t sz) +{ + FILE* f = fopen(path, "wb"); + size_t written; + + if (f == NULL) + return -1; + written = fwrite(data, 1, sz, f); + if (fclose(f) != 0 || written != sz) + return -1; + return 0; +} + +static word32 get_be32(const byte* p) +{ + return ((word32)p[0] << 24) | ((word32)p[1] << 16) | + ((word32)p[2] << 8) | (word32)p[3]; +} + +static void put_be32(byte* p, word32 v) +{ + p[0] = (byte)(v >> 24); + p[1] = (byte)(v >> 16); + p[2] = (byte)(v >> 8); + p[3] = (byte)v; +} + +static CK_ULONG get_be_ulong(const byte* p) +{ + CK_ULONG v = 0; + size_t i; + + for (i = 0; i < sizeof(CK_ULONG); i++) + v = (v << 8) | p[i]; + return v; +} + +/* Rewrite the stored flags of the object of the checked class so they read as + * exportable. */ +static int mark_stored_key_exportable(void) +{ + static byte data[MAX_FILE_SZ]; + char path[256]; + size_t sz = 0; + word32 flags; + int i; + int found = 0; + + for (i = 0; i < MAX_OBJ_ID && !found; i++) { + store_path(path, sizeof(path), "obj", i); + if (read_file(path, data, sizeof(data), &sz) == 0 && + sz >= OBJ_FLAGS_OFF + 4 && + get_be_ulong(data + OBJ_CLASS_OFF) == keyClass) { + found = 1; + } + } + if (!found) + return -1; + flags = get_be32(data + OBJ_FLAGS_OFF); + if ((flags & WP11_FLAG_SENSITIVE) == 0 || + (flags & WP11_FLAG_EXTRACTABLE) != 0) + return -1; + flags &= ~(word32)WP11_FLAG_SENSITIVE; + flags |= WP11_FLAG_EXTRACTABLE; + put_be32(data + OBJ_FLAGS_OFF, flags); + return write_file(path, data, sz); +} + +static CK_RV init_slot(CK_SLOT_ID* slot) +{ + CK_RV rv; + CK_C_INITIALIZE_ARGS args; + CK_SLOT_ID slots[16]; + CK_ULONG count = sizeof(slots) / sizeof(*slots); + + 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, slots, &count); + if (rv == CKR_OK && count == 0) + rv = CKR_TOKEN_NOT_PRESENT; + if (rv == CKR_OK) + *slot = slots[0]; + return rv; +} + +static CK_RV provision_token(CK_SLOT_ID slot) +{ + CK_RV rv; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_UTF8CHAR label[32]; + CK_FLAGS flags = CKF_SERIAL_SESSION | CKF_RW_SESSION; + + XMEMSET(label, ' ', sizeof(label)); + XMEMCPY(label, "wolfpkcs11", 10); + rv = funcList->C_InitToken(slot, soPin, sizeof(soPin) - 1, label); + if (rv == CKR_OK) + rv = funcList->C_OpenSession(slot, flags, NULL, NULL, &session); + if (rv == CKR_OK) + rv = funcList->C_Login(session, CKU_SO, soPin, sizeof(soPin) - 1); + if (rv == CKR_OK) + rv = funcList->C_InitPIN(session, userPin, sizeof(userPin) - 1); + if (session != CK_INVALID_HANDLE) { + (void)funcList->C_Logout(session); + (void)funcList->C_CloseSession(session); + } + return rv; +} + +static CK_RV open_session(CK_SLOT_ID slot, int login, + CK_SESSION_HANDLE* session) +{ + CK_RV rv; + CK_FLAGS flags = CKF_SERIAL_SESSION | CKF_RW_SESSION; + + rv = funcList->C_OpenSession(slot, flags, NULL, NULL, session); + if (rv == CKR_OK && login) { + rv = funcList->C_Login(*session, CKU_USER, userPin, + (CK_ULONG)sizeof(userPin) - 1); + } + return rv; +} + +static CK_RV find_key(CK_SESSION_HANDLE session, CK_OBJECT_HANDLE* obj) +{ + CK_RV rv; + CK_ULONG count = 0; + CK_ATTRIBUTE tmpl[] = { + { CKA_CLASS, &keyClass, sizeof(keyClass) }, + }; + + rv = funcList->C_FindObjectsInit(session, tmpl, 1); + if (rv == CKR_OK) { + rv = funcList->C_FindObjects(session, obj, 1, &count); + (void)funcList->C_FindObjectsFinal(session); + } + if (rv == CKR_OK && count != 1) + rv = CKR_OBJECT_HANDLE_INVALID; + return rv; +} + +static CK_RV aes_encrypt_block(CK_SESSION_HANDLE session, CK_OBJECT_HANDLE obj, + byte* out) +{ + CK_RV rv; + byte iv[BLOCK_SZ]; + CK_MECHANISM mech; + CK_ULONG outSz = BLOCK_SZ; + + XMEMSET(iv, 0, sizeof(iv)); + mech.mechanism = CKM_AES_CBC; + mech.pParameter = iv; + mech.ulParameterLen = sizeof(iv); + rv = funcList->C_EncryptInit(session, &mech, obj); + if (rv == CKR_OK) + rv = funcList->C_Encrypt(session, refPlain, sizeof(refPlain), out, + &outSz); + if (rv == CKR_OK && outSz != BLOCK_SZ) + rv = CKR_FUNCTION_FAILED; + return rv; +} + +/* Sign a fixed digest-sized message with the key. */ +static CK_RV sign_block(CK_SESSION_HANDLE session, CK_OBJECT_HANDLE obj) +{ + CK_RV rv; + CK_MECHANISM mech; + byte msg[32]; + static byte sig[8192]; + CK_ULONG sigSz = sizeof(sig); + + XMEMSET(msg, 0x5a, sizeof(msg)); + mech.mechanism = signMech; + mech.pParameter = NULL; + mech.ulParameterLen = 0; + rv = funcList->C_SignInit(session, &mech, obj); + if (rv == CKR_OK) + rv = funcList->C_Sign(session, msg, sizeof(msg), sig, &sigSz); + return rv; +} + +static CK_RV get_bool(CK_SESSION_HANDLE session, CK_OBJECT_HANDLE obj, + CK_ATTRIBUTE_TYPE type, CK_BBOOL* val) +{ + CK_ATTRIBUTE attr; + + attr.type = type; + attr.pValue = val; + attr.ulValueLen = sizeof(*val); + return funcList->C_GetAttributeValue(session, obj, &attr, 1); +} + +typedef CK_RV (*create_fn)(CK_SESSION_HANDLE session); + +/* Provision a fresh token, create objects with 'create' and persist them. */ +static CK_RV store_objects(create_fn create) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + + cleanup_test_files(); + rv = pkcs11_load(); + if (rv != CKR_OK) + return rv; + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = provision_token(slot); + if (rv == CKR_OK) + rv = open_session(slot, 1, &session); + if (rv == CKR_OK) + rv = create(session); + if (session != CK_INVALID_HANDLE) + (void)funcList->C_CloseSession(session); + (void)funcList->C_Finalize(NULL); + pkcs11_unload(); + return rv; +} + +typedef int (*child_fn)(void); + +/* Run fn in a child process. Returns CHILD_PASS only when it exits normally + * reporting that the invariant held. */ +static int run_in_child(child_fn fn) +{ + pid_t pid; + int status = 0; + + fflush(stdout); + fflush(stderr); + pid = fork(); + if (pid < 0) + return CHILD_SETUP; + if (pid == 0) { + int res = fn(); + fflush(stdout); + fflush(stderr); + _exit(res); + } + if (waitpid(pid, &status, 0) != pid) + return CHILD_SETUP; + if (WIFSIGNALED(status)) { + fprintf(stderr, " reload terminated by signal %d\n", + WTERMSIG(status)); + return CHILD_FAIL; + } + if (!WIFEXITED(status)) + return CHILD_FAIL; + return WEXITSTATUS(status); +} + +/* After logging in, the key signs. */ +static int child_key_signs(void) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = open_session(slot, 1, &session); + if (rv == CKR_OK) + rv = find_key(session, &obj); + if (rv == CKR_OK) + rv = sign_block(session, obj); + (void)funcList->C_Finalize(NULL); + if (rv != CKR_OK) { + fprintf(stderr, " signing after reload failed: 0x%lx\n", + (unsigned long)rv); + return CHILD_FAIL; + } + return CHILD_PASS; +} + +static int holds_secret(const byte* buf, size_t sz) +{ + size_t i; + + if (secretNeedle == NULL || sz < secretNeedleSz) + return 0; + for (i = 0; i + secretNeedleSz <= sz; i++) { + if (XMEMCMP(buf + i, secretNeedle, secretNeedleSz) == 0) + return 1; + } + return 0; +} + +static CK_RV create_aes_key_ex(CK_SESSION_HANDLE session, CK_BBOOL priv, + CK_BBOOL sensitive, CK_BBOOL extractable) +{ + CK_RV rv; + CK_OBJECT_CLASS cls = CKO_SECRET_KEY; + CK_KEY_TYPE keyType = CKK_AES; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_ATTRIBUTE tmpl[] = { + { CKA_CLASS, &cls, sizeof(cls) }, + { CKA_KEY_TYPE, &keyType, sizeof(keyType) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIVATE, &priv, sizeof(priv) }, + { CKA_SENSITIVE, &sensitive, sizeof(sensitive) }, + { CKA_EXTRACTABLE, &extractable, sizeof(extractable) }, + { CKA_ENCRYPT, &ckTrue, sizeof(ckTrue) }, + { CKA_DECRYPT, &ckTrue, sizeof(ckTrue) }, + { CKA_VALUE, aesKeyValue, sizeof(aesKeyValue) }, + }; + + rv = funcList->C_CreateObject(session, tmpl, + sizeof(tmpl) / sizeof(*tmpl), &obj); + if (rv == CKR_OK) + rv = aes_encrypt_block(session, obj, refCipher); + return rv; +} + +static CK_RV create_aes_key(CK_SESSION_HANDLE session) +{ + return create_aes_key_ex(session, CK_TRUE, CK_TRUE, CK_FALSE); +} + +#ifdef HAVE_ECC +static CK_RV create_ecc_private_key(CK_SESSION_HANDLE session) +{ + CK_OBJECT_CLASS cls = CKO_PRIVATE_KEY; + CK_KEY_TYPE keyType = CKK_EC; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_ATTRIBUTE tmpl[] = { + { CKA_CLASS, &cls, sizeof(cls) }, + { CKA_KEY_TYPE, &keyType, sizeof(keyType) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIVATE, &ckTrue, sizeof(ckTrue) }, + { CKA_SENSITIVE, &ckTrue, sizeof(ckTrue) }, + { CKA_EXTRACTABLE, &ckFalse, sizeof(ckFalse) }, + { CKA_SIGN, &ckTrue, sizeof(ckTrue) }, + { CKA_EC_PARAMS, ecc_p256_params, sizeof(ecc_p256_params) }, + { CKA_VALUE, ecc_p256_priv, sizeof(ecc_p256_priv) }, + }; + + return funcList->C_CreateObject(session, tmpl, + sizeof(tmpl) / sizeof(*tmpl), &obj); +} +#endif + +#ifndef NO_RSA +static CK_RV create_rsa_private_key(CK_SESSION_HANDLE session) +{ + CK_OBJECT_CLASS cls = CKO_PRIVATE_KEY; + CK_KEY_TYPE keyType = CKK_RSA; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_ATTRIBUTE tmpl[] = { + { CKA_CLASS, &cls, sizeof(cls) }, + { CKA_KEY_TYPE, &keyType, sizeof(keyType) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIVATE, &ckTrue, sizeof(ckTrue) }, + { CKA_SENSITIVE, &ckTrue, sizeof(ckTrue) }, + { CKA_EXTRACTABLE, &ckFalse, sizeof(ckFalse) }, + { CKA_SIGN, &ckTrue, sizeof(ckTrue) }, + { CKA_MODULUS, rsa_2048_modulus, sizeof(rsa_2048_modulus) }, + { CKA_PUBLIC_EXPONENT, rsa_2048_pub_exp, sizeof(rsa_2048_pub_exp) }, + { CKA_PRIVATE_EXPONENT, rsa_2048_priv_exp, sizeof(rsa_2048_priv_exp) }, + { CKA_PRIME_1, rsa_2048_p, sizeof(rsa_2048_p) }, + { CKA_PRIME_2, rsa_2048_q, sizeof(rsa_2048_q) }, + { CKA_EXPONENT_1, rsa_2048_dP, sizeof(rsa_2048_dP) }, + { CKA_EXPONENT_2, rsa_2048_dQ, sizeof(rsa_2048_dQ) }, + { CKA_COEFFICIENT, rsa_2048_u, sizeof(rsa_2048_u) }, + }; + + return funcList->C_CreateObject(session, tmpl, + sizeof(tmpl) / sizeof(*tmpl), &obj); +} +#endif + +#ifndef NO_DH +static CK_RV create_dh_private_key(CK_SESSION_HANDLE session) +{ + CK_OBJECT_CLASS cls = CKO_PRIVATE_KEY; + CK_KEY_TYPE keyType = CKK_DH; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_ATTRIBUTE tmpl[] = { + { CKA_CLASS, &cls, sizeof(cls) }, + { CKA_KEY_TYPE, &keyType, sizeof(keyType) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIVATE, &ckTrue, sizeof(ckTrue) }, + { CKA_SENSITIVE, &ckTrue, sizeof(ckTrue) }, + { CKA_EXTRACTABLE, &ckFalse, sizeof(ckFalse) }, + { CKA_DERIVE, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIME, dh_ffdhe2048_p, sizeof(dh_ffdhe2048_p) }, + { CKA_BASE, dh_ffdhe2048_g, sizeof(dh_ffdhe2048_g) }, + { CKA_VALUE, dh_2048_priv, sizeof(dh_2048_priv) }, + }; + + return funcList->C_CreateObject(session, tmpl, + sizeof(tmpl) / sizeof(*tmpl), &obj); +} +#endif + +#ifdef WOLFPKCS11_MLDSA +static CK_RV create_mldsa_key_pair(CK_SESSION_HANDLE session) +{ + CK_MECHANISM mech = { CKM_ML_DSA_KEY_PAIR_GEN, NULL, 0 }; + CK_ULONG paramSet = CKP_ML_DSA_44; + CK_OBJECT_HANDLE pub = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE priv = CK_INVALID_HANDLE; + CK_ATTRIBUTE pubTmpl[] = { + { CKA_PARAMETER_SET, ¶mSet, sizeof(paramSet) }, + { CKA_VERIFY, &ckTrue, sizeof(ckTrue) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + }; + CK_ATTRIBUTE privTmpl[] = { + { CKA_SIGN, &ckTrue, sizeof(ckTrue) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIVATE, &ckTrue, sizeof(ckTrue) }, + { CKA_SENSITIVE, &ckTrue, sizeof(ckTrue) }, + { CKA_EXTRACTABLE, &ckFalse, sizeof(ckFalse) }, + }; + + return funcList->C_GenerateKeyPair(session, &mech, pubTmpl, + sizeof(pubTmpl) / sizeof(*pubTmpl), privTmpl, + sizeof(privTmpl) / sizeof(*privTmpl), &pub, &priv); +} +#endif + +#if defined(WOLFPKCS11_MLDSA) || defined(WOLFPKCS11_MLKEM) +static byte pqValue[8192]; +static CK_ULONG pqValueSz = 0; +static CK_MECHANISM_TYPE pqGenMech = CKM_VENDOR_DEFINED; +static CK_ULONG pqParamSet = 0; + +/* Generate an exportable post-quantum key pair and record the private value. */ +static CK_RV create_pq_exportable_key_pair(CK_SESSION_HANDLE session) +{ + CK_RV rv; + CK_MECHANISM mech; + CK_OBJECT_HANDLE pub = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE priv = CK_INVALID_HANDLE; + CK_ATTRIBUTE pubTmpl[] = { + { CKA_PARAMETER_SET, &pqParamSet, sizeof(pqParamSet) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + }; + CK_ATTRIBUTE privTmpl[] = { + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIVATE, &ckTrue, sizeof(ckTrue) }, + { CKA_SENSITIVE, &ckFalse, sizeof(ckFalse) }, + { CKA_EXTRACTABLE, &ckTrue, sizeof(ckTrue) }, + }; + CK_ATTRIBUTE attr; + + mech.mechanism = pqGenMech; + mech.pParameter = NULL; + mech.ulParameterLen = 0; + rv = funcList->C_GenerateKeyPair(session, &mech, pubTmpl, + sizeof(pubTmpl) / sizeof(*pubTmpl), privTmpl, + sizeof(privTmpl) / sizeof(*privTmpl), &pub, &priv); + if (rv == CKR_OK) { + attr.type = CKA_VALUE; + attr.pValue = pqValue; + attr.ulValueLen = sizeof(pqValue); + rv = funcList->C_GetAttributeValue(session, priv, &attr, 1); + pqValueSz = attr.ulValueLen; + } + if (rv == CKR_OK && (pqValueSz == 0 || pqValueSz > sizeof(pqValue))) + rv = CKR_FUNCTION_FAILED; + return rv; +} + +/* After logging in, the private key value reads back unchanged. */ +static int child_pq_value_kept(void) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + static byte buf[sizeof(pqValue)]; + CK_ATTRIBUTE attr; + + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = open_session(slot, 1, &session); + if (rv == CKR_OK) + rv = find_key(session, &obj); + if (rv == CKR_OK) { + attr.type = CKA_VALUE; + attr.pValue = buf; + attr.ulValueLen = sizeof(buf); + rv = funcList->C_GetAttributeValue(session, obj, &attr, 1); + } + (void)funcList->C_Finalize(NULL); + if (rv != CKR_OK || attr.ulValueLen != pqValueSz || + XMEMCMP(buf, pqValue, pqValueSz) != 0) { + fprintf(stderr, " private value not kept: 0x%lx\n", + (unsigned long)rv); + return CHILD_FAIL; + } + return CHILD_PASS; +} + +static void check_pq_round_trip(const char* name, CK_MECHANISM_TYPE genMech, + CK_ULONG paramSet) +{ + CK_RV rv; + char msg[160]; + int res = CHILD_SETUP; + + keyClass = CKO_PRIVATE_KEY; + pqGenMech = genMech; + pqParamSet = paramSet; + rv = store_objects(create_pq_exportable_key_pair); + (void)snprintf(msg, sizeof(msg), "store %s", name); + CHECK_RV(rv, msg, CKR_OK); + if (rv == CKR_OK) + res = run_in_child(child_pq_value_kept); + (void)snprintf(msg, sizeof(msg), "%s private value kept after reload", + name); + CHECK_TRUE(res == CHILD_PASS, msg); + cleanup_test_files(); +} + +static void test_pq_keys_round_trip(void) +{ + printf("\n--- post-quantum private keys round trip ---\n"); +#ifdef WOLFPKCS11_MLDSA + check_pq_round_trip("ML-DSA key pair", CKM_ML_DSA_KEY_PAIR_GEN, + CKP_ML_DSA_44); +#endif +#ifdef WOLFPKCS11_MLKEM + check_pq_round_trip("ML-KEM key pair", CKM_ML_KEM_KEY_PAIR_GEN, + CKP_ML_KEM_768); +#endif +} +#endif + +/* After logging in, the key must not be exported and, for an AES key, must not + * work as the original key. */ +static int child_key_not_exported(void) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + static byte buf[4096]; + byte out[BLOCK_SZ]; + CK_BBOOL derive = CK_FALSE; + CK_ATTRIBUTE attr; + int res = CHILD_PASS; + + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = open_session(slot, 1, &session); + if (rv != CKR_OK) { + fprintf(stderr, " login failed: 0x%lx\n", (unsigned long)rv); + res = CHILD_FAIL; + } + if (res == CHILD_PASS && find_key(session, &obj) == CKR_OK) { + attr.type = secretAttr; + attr.pValue = buf; + attr.ulValueLen = sizeof(buf); + rv = funcList->C_GetAttributeValue(session, obj, &attr, 1); + if (rv == CKR_OK && (holds_secret(buf, attr.ulValueLen) || + (secretNeedle == NULL && attr.ulValueLen > 0))) { + fprintf(stderr, " key value was returned\n"); + res = CHILD_FAIL; + } + if (checkAesUse && aes_encrypt_block(session, obj, out) == CKR_OK && + XMEMCMP(out, refCipher, sizeof(out)) == 0) { + fprintf(stderr, " key still operates as the original key\n"); + res = CHILD_FAIL; + } + if (signMech != CKM_VENDOR_DEFINED && + sign_block(session, obj) == CKR_OK) { + fprintf(stderr, " key still signs\n"); + res = CHILD_FAIL; + } + if (get_bool(session, obj, CKA_DERIVE, &derive) == CKR_OK) { + derive = (derive == CK_TRUE) ? CK_FALSE : CK_TRUE; + attr.type = CKA_DERIVE; + attr.pValue = &derive; + attr.ulValueLen = sizeof(derive); + rv = funcList->C_SetAttributeValue(session, obj, &attr, 1); + if (rv == CKR_OK || rv == CKR_USER_NOT_LOGGED_IN) { + fprintf(stderr, " policy update returned 0x%lx\n", + (unsigned long)rv); + res = CHILD_FAIL; + } + } + } + (void)funcList->C_Finalize(NULL); + return res; +} + +/* After logging in, the AES key works as the original key and reports the + * expected policy. */ +static int child_key_intact(void) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + byte out[BLOCK_SZ]; + byte value[sizeof(aesKeyValue)]; + CK_BBOOL sensitive = CK_FALSE; + CK_BBOOL extractable = CK_TRUE; + CK_BBOOL decrypt = CK_FALSE; + CK_ATTRIBUTE attr; + int res = CHILD_PASS; + + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = open_session(slot, 1, &session); + if (rv == CKR_OK) + rv = find_key(session, &obj); + if (rv == CKR_OK) + rv = aes_encrypt_block(session, obj, out); + if (rv == CKR_OK && XMEMCMP(out, refCipher, sizeof(out)) != 0) + rv = CKR_FUNCTION_FAILED; + if (rv == CKR_OK) + rv = get_bool(session, obj, CKA_SENSITIVE, &sensitive); + if (rv == CKR_OK) + rv = get_bool(session, obj, CKA_EXTRACTABLE, &extractable); + if (rv == CKR_OK) + rv = get_bool(session, obj, CKA_DECRYPT, &decrypt); + if (rv != CKR_OK) { + fprintf(stderr, " key unusable after reload: 0x%lx\n", + (unsigned long)rv); + res = CHILD_FAIL; + } + else if (sensitive != expSensitive || extractable != expExtractable || + decrypt != expDecrypt) { + fprintf(stderr, " policy not kept: sensitive %d extractable %d " + "decrypt %d\n", sensitive, extractable, decrypt); + res = CHILD_FAIL; + } + else if (sensitive == CK_TRUE) { + attr.type = CKA_VALUE; + attr.pValue = value; + attr.ulValueLen = sizeof(value); + if (funcList->C_GetAttributeValue(session, obj, &attr, 1) == CKR_OK) { + fprintf(stderr, " sensitive key value was returned\n"); + res = CHILD_FAIL; + } + } + (void)funcList->C_Finalize(NULL); + return res; +} + +static void set_aes_check(void) +{ + keyClass = CKO_SECRET_KEY; + secretAttr = CKA_VALUE; + secretNeedle = aesKeyValue + sizeof(aesKeyValue) - 16; + secretNeedleSz = 16; + checkAesUse = 1; + expSensitive = CK_TRUE; + expExtractable = CK_FALSE; + expDecrypt = CK_TRUE; +} + +static void check_stored_policy(const char* name, create_fn create, + CK_OBJECT_CLASS cls, CK_ATTRIBUTE_TYPE type, + const byte* secret, size_t secretSz, int aes, + CK_MECHANISM_TYPE sign) +{ + CK_RV rv; + char msg[160]; + int res = CHILD_SETUP; + + rv = store_objects(create); + (void)snprintf(msg, sizeof(msg), "store %s", name); + CHECK_RV(rv, msg, CKR_OK); + if (rv != CKR_OK) + return; + keyClass = cls; + secretAttr = type; + secretNeedle = NULL; + secretNeedleSz = 0; + if (secret != NULL && secretSz >= 16) { + secretNeedle = secret + secretSz - 16; + secretNeedleSz = 16; + } + checkAesUse = aes; + signMech = sign; + if (sign != CKM_VENDOR_DEFINED) { + res = run_in_child(child_key_signs); + (void)snprintf(msg, sizeof(msg), "%s signs after reload", name); + CHECK_TRUE(res == CHILD_PASS, msg); + res = CHILD_SETUP; + } + if (mark_stored_key_exportable() == 0) + res = run_in_child(child_key_not_exported); + signMech = CKM_VENDOR_DEFINED; + (void)snprintf(msg, sizeof(msg), + "%s keeps the policy it was stored under", name); + CHECK_TRUE(res == CHILD_PASS, msg); + cleanup_test_files(); +} + +static void test_stored_policy_is_authenticated(void) +{ + printf("\n--- stored key policy is authenticated ---\n"); + check_stored_policy("AES key", create_aes_key, CKO_SECRET_KEY, CKA_VALUE, + aesKeyValue, sizeof(aesKeyValue), 1, CKM_VENDOR_DEFINED); +#ifdef HAVE_ECC + check_stored_policy("ECC private key", create_ecc_private_key, + CKO_PRIVATE_KEY, CKA_VALUE, ecc_p256_priv, sizeof(ecc_p256_priv), 0, + CKM_ECDSA); +#endif +#ifndef NO_RSA + check_stored_policy("RSA private key", create_rsa_private_key, + CKO_PRIVATE_KEY, CKA_PRIVATE_EXPONENT, rsa_2048_priv_exp, + sizeof(rsa_2048_priv_exp), 0, CKM_RSA_PKCS); +#endif +#ifndef NO_DH + check_stored_policy("DH private key", create_dh_private_key, + CKO_PRIVATE_KEY, CKA_VALUE, dh_2048_priv, sizeof(dh_2048_priv), 0, + CKM_VENDOR_DEFINED); +#endif +#ifdef WOLFPKCS11_MLDSA + check_stored_policy("ML-DSA private key", create_mldsa_key_pair, + CKO_PRIVATE_KEY, CKA_VALUE, NULL, 0, 0, CKM_ML_DSA); +#endif +} + +/* Tighten the policy of an exportable key while logged in. */ +static CK_RV create_and_restrict_aes_key(CK_SESSION_HANDLE session) +{ + CK_RV rv; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_ATTRIBUTE tmpl[] = { + { CKA_SENSITIVE, &ckTrue, sizeof(ckTrue) }, + { CKA_EXTRACTABLE, &ckFalse, sizeof(ckFalse) }, + { CKA_DECRYPT, &ckFalse, sizeof(ckFalse) }, + }; + + rv = create_aes_key_ex(session, CK_TRUE, CK_FALSE, CK_TRUE); + if (rv == CKR_OK) + rv = find_key(session, &obj); + if (rv == CKR_OK) { + rv = funcList->C_SetAttributeValue(session, obj, tmpl, + sizeof(tmpl) / sizeof(*tmpl)); + } + return rv; +} + +/* Restrict the key while logged in, then persist the token by adding another + * object and stop without finalizing. */ +static int child_restrict_and_store(void) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE data = CK_INVALID_HANDLE; + CK_OBJECT_CLASS dataClass = CKO_DATA; + byte value[] = "stored"; + CK_ATTRIBUTE restrictTmpl[] = { + { CKA_SENSITIVE, &ckTrue, sizeof(ckTrue) }, + { CKA_EXTRACTABLE, &ckFalse, sizeof(ckFalse) }, + }; + CK_ATTRIBUTE dataTmpl[] = { + { CKA_CLASS, &dataClass, sizeof(dataClass) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_VALUE, value, sizeof(value) - 1 }, + }; + + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = open_session(slot, 1, &session); + if (rv == CKR_OK) + rv = find_key(session, &obj); + if (rv == CKR_OK) + rv = funcList->C_SetAttributeValue(session, obj, restrictTmpl, 2); + if (rv == CKR_OK) + rv = funcList->C_CreateObject(session, dataTmpl, 3, &data); + if (rv != CKR_OK) { + fprintf(stderr, " restrict and store failed: 0x%lx\n", + (unsigned long)rv); + return CHILD_FAIL; + } + return CHILD_PASS; +} + +static CK_RV create_exportable_aes_key(CK_SESSION_HANDLE session) +{ + return create_aes_key_ex(session, CK_TRUE, CK_FALSE, CK_TRUE); +} + +static CK_RV create_public_aes_key(CK_SESSION_HANDLE session) +{ + return create_aes_key_ex(session, CK_FALSE, CK_TRUE, CK_FALSE); +} + +/* Without the user logged in, a policy change cannot be bound to the stored + * key and is refused, while a label change is accepted. */ +static int child_unbound_update_refused(void) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + char label[] = "renamed"; + CK_ATTRIBUTE policy[] = { + { CKA_DECRYPT, &ckFalse, sizeof(ckFalse) }, + }; + CK_ATTRIBUTE name[] = { + { CKA_LABEL, label, sizeof(label) - 1 }, + }; + int res = CHILD_PASS; + + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = open_session(slot, 0, &session); + if (rv == CKR_OK) + rv = find_key(session, &obj); + if (rv != CKR_OK) { + fprintf(stderr, " public key not found: 0x%lx\n", (unsigned long)rv); + res = CHILD_FAIL; + } + if (res == CHILD_PASS) { + rv = funcList->C_SetAttributeValue(session, obj, policy, 1); + if (rv != CKR_USER_NOT_LOGGED_IN) { + fprintf(stderr, " policy change returned 0x%lx\n", + (unsigned long)rv); + res = CHILD_FAIL; + } + rv = funcList->C_SetAttributeValue(session, obj, name, 1); + if (rv != CKR_OK) { + fprintf(stderr, " label change returned 0x%lx\n", + (unsigned long)rv); + res = CHILD_FAIL; + } + } + (void)funcList->C_Finalize(NULL); + return res; +} + +static void test_legitimate_updates_round_trip(void) +{ + CK_RV rv; + int res = CHILD_SETUP; + + printf("\n--- legitimate policy updates survive a reload ---\n"); + set_aes_check(); + rv = store_objects(create_and_restrict_aes_key); + CHECK_RV(rv, "restrict AES key policy while logged in", CKR_OK); + if (rv == CKR_OK) { + expDecrypt = CK_FALSE; + res = run_in_child(child_key_intact); + } + CHECK_TRUE(res == CHILD_PASS, "restricted policy is kept after reload"); + res = CHILD_SETUP; + if (rv == CKR_OK && mark_stored_key_exportable() == 0) + res = run_in_child(child_key_not_exported); + CHECK_TRUE(res == CHILD_PASS, + "restricted policy cannot be undone in store"); + cleanup_test_files(); + + res = CHILD_SETUP; + set_aes_check(); + rv = store_objects(create_exportable_aes_key); + CHECK_RV(rv, "store exportable AES key", CKR_OK); + if (rv == CKR_OK) + res = run_in_child(child_restrict_and_store); + CHECK_TRUE(res == CHILD_PASS, "restrict key and store while logged in"); + if (res == CHILD_PASS) + res = run_in_child(child_key_intact); + CHECK_TRUE(res == CHILD_PASS, "policy stored while logged in is kept"); + if (res == CHILD_PASS && mark_stored_key_exportable() == 0) + res = run_in_child(child_key_not_exported); + else + res = CHILD_SETUP; + CHECK_TRUE(res == CHILD_PASS, "policy stored while logged in is bound"); + cleanup_test_files(); + + res = CHILD_SETUP; + set_aes_check(); + rv = store_objects(create_public_aes_key); + CHECK_RV(rv, "store public AES key", CKR_OK); + if (rv == CKR_OK) + res = run_in_child(child_unbound_update_refused); + CHECK_TRUE(res == CHILD_PASS, "policy change needs the user's key"); + res = CHILD_SETUP; + if (rv == CKR_OK) + res = run_in_child(child_key_intact); + CHECK_TRUE(res == CHILD_PASS, "public key usable after refused change"); + cleanup_test_files(); +} + +#if (defined(WOLFPKCS11_PBKDF2) || defined(HAVE_SCRYPT)) && \ + !defined(WOLFSSL_STM32U5_DHUK) && \ + !(defined(HAVE_FIPS) && defined(WOLFPKCS11_NSS)) +#define HAVE_LEGACY_STORE_TEST + +/* Derive the token storage key from the user PIN and the stored token seed, + * the way stores have always been keyed. */ +static int derive_token_key(byte* key, word32 keySz) +{ + static byte data[MAX_FILE_SZ]; + char path[256]; + size_t sz = 0; + size_t off = 32; + byte* seed; + + store_path(path, sizeof(path), "token", -1); + if (read_file(path, data, sizeof(data), &sz) != 0) + return -1; + /* label, SO PIN, SO seed, SO fail count and times */ + if (sz < off + 4) + return -1; + off += 4 + get_be32(data + off) + 16 + 4 + 2 * sizeof(time_t); + /* user PIN, user seed, user fail count and times */ + if (sz < off + 4) + return -1; + off += 4 + get_be32(data + off) + 16 + 4 + 2 * sizeof(time_t); + if (sz < off + 16) + return -1; + seed = data + off; +#ifdef WOLFPKCS11_PBKDF2 + return wc_PBKDF2(key, userPin, (int)sizeof(userPin) - 1, seed, 16, + PBKDF2_ITERATIONS, (int)keySz, WC_SHA256); +#else + return wc_scrypt(key, userPin, (int)sizeof(userPin) - 1, seed, 16, + WP11_HASH_PIN_COST, WP11_HASH_PIN_BLOCKSIZE, + WP11_HASH_PIN_PARALLEL, (int)keySz); +#endif +} + +static int read_stored_iv(byte* iv) +{ + static byte data[MAX_FILE_SZ]; + char path[256]; + size_t sz = 0; + + if (find_store_file("obj", path, sizeof(path)) != 0 || + read_file(path, data, sizeof(data), &sz) != 0 || sz < OBJ_IV_SZ) + return -1; + XMEMCPY(iv, data, OBJ_IV_SZ); + return 0; +} + +/* Replace the stored AES key with an encryption that binds no policy. */ +static int write_unbound_key_record(void) +{ + byte key[32]; + byte iv[OBJ_IV_SZ]; + byte rec[4 + sizeof(aesKeyValue) + TAG_SZ]; + char path[256]; + Aes aes; + int ret; + + ret = derive_token_key(key, sizeof(key)); + if (ret == 0) + ret = read_stored_iv(iv); + if (ret == 0) + ret = find_store_file("symmkey", path, sizeof(path)); + if (ret == 0) + ret = wc_AesInit(&aes, NULL, INVALID_DEVID); + if (ret == 0) { + ret = wc_AesGcmSetKey(&aes, key, sizeof(key)); + if (ret == 0) { + ret = wc_AesGcmEncrypt(&aes, rec + 4, aesKeyValue, + sizeof(aesKeyValue), iv, sizeof(iv), + rec + 4 + sizeof(aesKeyValue), TAG_SZ, NULL, 0); + } + wc_AesFree(&aes); + } + if (ret == 0) { + put_be32(rec, (word32)(sizeof(aesKeyValue) + TAG_SZ)); + ret = write_file(path, rec, sizeof(rec)); + } + wc_ForceZero(key, sizeof(key)); + return ret; +} + +/* Returns 1 when the stored AES key record still verifies without a policy. */ +static int stored_key_is_unbound(void) +{ + static byte data[MAX_FILE_SZ]; + byte key[32]; + byte iv[OBJ_IV_SZ]; + byte plain[sizeof(aesKeyValue)]; + char path[256]; + size_t sz = 0; + Aes aes; + int ret; + int unbound = 0; + + ret = derive_token_key(key, sizeof(key)); + if (ret == 0) + ret = read_stored_iv(iv); + if (ret == 0) + ret = find_store_file("symmkey", path, sizeof(path)); + if (ret == 0) + ret = read_file(path, data, sizeof(data), &sz); + if (ret == 0 && (sz != 4 + sizeof(aesKeyValue) + TAG_SZ || + get_be32(data) != sizeof(aesKeyValue) + TAG_SZ)) + ret = -1; + if (ret == 0) + ret = wc_AesInit(&aes, NULL, INVALID_DEVID); + if (ret == 0) { + ret = wc_AesGcmSetKey(&aes, key, sizeof(key)); + if (ret == 0) { + ret = wc_AesGcmDecrypt(&aes, plain, data + 4, sizeof(plain), iv, + sizeof(iv), data + 4 + sizeof(plain), TAG_SZ, NULL, 0); + unbound = (ret == 0); + } + wc_AesFree(&aes); + } + wc_ForceZero(key, sizeof(key)); + wc_ForceZero(plain, sizeof(plain)); + return unbound; +} + +static void test_unbound_store_upgraded(void) +{ + CK_RV rv; + int res = CHILD_SETUP; + + printf("\n--- store without bound policy loads and is upgraded ---\n"); + rv = store_objects(create_aes_key); + CHECK_RV(rv, "store AES key", CKR_OK); + if (rv == CKR_OK && write_unbound_key_record() == 0 && + stored_key_is_unbound()) { + set_aes_check(); + res = run_in_child(child_key_intact); + } + CHECK_TRUE(res == CHILD_PASS, "unbound key record still loads"); + CHECK_TRUE(res == CHILD_PASS && !stored_key_is_unbound(), + "key record is rebound to its policy after use"); + if (res == CHILD_PASS) + res = run_in_child(child_key_intact); + CHECK_TRUE(res == CHILD_PASS, "rebound key loads again"); + res = CHILD_SETUP; + if (mark_stored_key_exportable() == 0) + res = run_in_child(child_key_not_exported); + CHECK_TRUE(res == CHILD_PASS, "rebound key keeps its policy"); + cleanup_test_files(); +} +#endif + +static byte otherUserPin[] = "wolfpkcs11-other"; + +/* Store passes that cannot start must leave the PIN and token in effect. */ +static int child_failed_store_keeps_state(void) +{ + CK_RV rv; + CK_RV setRv = CKR_OK; + CK_RV initRv = CKR_OK; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_UTF8CHAR label[32]; + byte out[BLOCK_SZ]; + char lockPath[256]; + + (void)snprintf(lockPath, sizeof(lockPath), "%s/wp11_txn_%016lx_lock", + TEST_DIR, 1UL); + XMEMSET(label, ' ', sizeof(label)); + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = open_session(slot, 1, &session); + /* A directory at the lock file name makes every store pass fail. */ + (void)unlink(lockPath); + if (rv == CKR_OK && mkdir(lockPath, 0700) != 0) + rv = CKR_GENERAL_ERROR; + if (rv == CKR_OK) { + setRv = funcList->C_SetPIN(session, userPin, sizeof(userPin) - 1, + otherUserPin, sizeof(otherUserPin) - 1); + (void)funcList->C_CloseSession(session); + session = CK_INVALID_HANDLE; + initRv = funcList->C_InitToken(slot, soPin, sizeof(soPin) - 1, label); + } + (void)rmdir(lockPath); + if (rv == CKR_OK && (setRv == CKR_OK || initRv == CKR_OK)) + rv = CKR_GENERAL_ERROR; + if (rv == CKR_OK) + rv = open_session(slot, 1, &session); + if (rv == CKR_OK) + rv = find_key(session, &obj); + if (rv == CKR_OK) + rv = aes_encrypt_block(session, obj, out); + if (rv == CKR_OK && XMEMCMP(out, refCipher, sizeof(out)) != 0) + rv = CKR_FUNCTION_FAILED; + (void)funcList->C_Finalize(NULL); + if (rv != CKR_OK) { + fprintf(stderr, " old PIN or key lost after failed stores: 0x%lx\n", + (unsigned long)rv); + return CHILD_FAIL; + } + return CHILD_PASS; +} + +static void test_failed_store_keeps_state(void) +{ + CK_RV rv; + + printf("\n--- failed PIN change and token init keep stored state ---\n"); + set_aes_check(); + rv = store_objects(create_aes_key); + CHECK_RV(rv, "store AES key", CKR_OK); + if (rv == CKR_OK) { + CHECK_TRUE(run_in_child(child_failed_store_keeps_state) == CHILD_PASS, + "old PIN and key work after failed stores"); + } + cleanup_test_files(); +} + +#ifdef DEBUG_WOLFPKCS11 +/* Matches WP11_TEST_STORE_EXIT_CODE in wolfpkcs11/internal.h. */ +#define STORE_EXIT_CODE 75 +#define CRASH_MAX_RENAMES 40 + +extern void WP11_Test_StoreExitAfterRenames(int renames); + +static int crashAt = 0; + +/* Restrict the key and store the token, ending after crashAt renames. */ +static int child_restrict_and_crash(void) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE data = CK_INVALID_HANDLE; + CK_OBJECT_CLASS dataClass = CKO_DATA; + byte value[] = "stored"; + CK_ATTRIBUTE restrictTmpl[] = { + { CKA_SENSITIVE, &ckTrue, sizeof(ckTrue) }, + { CKA_EXTRACTABLE, &ckFalse, sizeof(ckFalse) }, + }; + CK_ATTRIBUTE dataTmpl[] = { + { CKA_CLASS, &dataClass, sizeof(dataClass) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_VALUE, value, sizeof(value) - 1 }, + }; + + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = open_session(slot, 1, &session); + if (rv == CKR_OK) + rv = find_key(session, &obj); + if (rv == CKR_OK) + rv = funcList->C_SetAttributeValue(session, obj, restrictTmpl, 2); + if (rv == CKR_OK) { + WP11_Test_StoreExitAfterRenames(crashAt); + rv = funcList->C_CreateObject(session, dataTmpl, 3, &data); + WP11_Test_StoreExitAfterRenames(0); + } + return (rv == CKR_OK) ? CHILD_PASS : CHILD_FAIL; +} + +/* The key loads and encrypts as before, under the old or the new policy. */ +static int child_key_old_or_new(void) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + byte out[BLOCK_SZ]; + CK_BBOOL sensitive = CK_FALSE; + CK_BBOOL extractable = CK_FALSE; + + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = open_session(slot, 1, &session); + if (rv == CKR_OK) + rv = find_key(session, &obj); + if (rv == CKR_OK) + rv = aes_encrypt_block(session, obj, out); + if (rv == CKR_OK && XMEMCMP(out, refCipher, sizeof(out)) != 0) + rv = CKR_FUNCTION_FAILED; + if (rv == CKR_OK) + rv = get_bool(session, obj, CKA_SENSITIVE, &sensitive); + if (rv == CKR_OK) + rv = get_bool(session, obj, CKA_EXTRACTABLE, &extractable); + if (rv == CKR_OK && sensitive == extractable) + rv = CKR_GENERAL_ERROR; + (void)funcList->C_Finalize(NULL); + if (rv != CKR_OK) { + fprintf(stderr, " key not usable after crash at rename %d: 0x%lx\n", + crashAt, (unsigned long)rv); + return CHILD_FAIL; + } + return CHILD_PASS; +} + +static void test_store_pass_is_atomic(void) +{ + CK_RV rv; + int res; + int done = 0; + int bad = 0; + + printf("\n--- a store pass cut short at any rename keeps the key ---\n"); + for (crashAt = 1; !done && bad == 0 && crashAt <= CRASH_MAX_RENAMES; + crashAt++) { + set_aes_check(); + rv = store_objects(create_exportable_aes_key); + if (rv != CKR_OK) { + bad++; + break; + } + res = run_in_child(child_restrict_and_crash); + if (res == CHILD_PASS) + done = 1; + else if (res != STORE_EXIT_CODE) + bad++; + if (run_in_child(child_key_old_or_new) != CHILD_PASS) + bad++; + } + CHECK_TRUE(done, "store pass runs to completion without the hook"); + CHECK_TRUE(bad == 0, "key is old or new after a crash at every rename"); + cleanup_test_files(); +} + +static byte newUserPin[] = "wolfpkcs11-new1"; + +/* Restrict the key and change the user PIN, ending after crashAt renames. */ +static int child_rebind_and_change_pin(void) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_ATTRIBUTE restrictTmpl[] = { + { CKA_SENSITIVE, &ckTrue, sizeof(ckTrue) }, + { CKA_EXTRACTABLE, &ckFalse, sizeof(ckFalse) }, + }; + + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = open_session(slot, 1, &session); + if (rv == CKR_OK) + rv = find_key(session, &obj); + if (rv == CKR_OK) + rv = funcList->C_SetAttributeValue(session, obj, restrictTmpl, 2); + if (rv == CKR_OK) { + WP11_Test_StoreExitAfterRenames(crashAt); + rv = funcList->C_SetPIN(session, userPin, sizeof(userPin) - 1, + newUserPin, sizeof(newUserPin) - 1); + WP11_Test_StoreExitAfterRenames(0); + } + return (rv == CKR_OK) ? CHILD_PASS : CHILD_FAIL; +} + +/* The key works after logging in with either the old or the new PIN. */ +static int child_key_under_either_pin(void) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + byte out[BLOCK_SZ]; + + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = open_session(slot, 0, &session); + if (rv == CKR_OK) { + rv = funcList->C_Login(session, CKU_USER, userPin, + sizeof(userPin) - 1); + if (rv == CKR_PIN_INCORRECT) { + rv = funcList->C_Login(session, CKU_USER, newUserPin, + sizeof(newUserPin) - 1); + } + } + if (rv == CKR_OK) + rv = find_key(session, &obj); + if (rv == CKR_OK) + rv = aes_encrypt_block(session, obj, out); + if (rv == CKR_OK && XMEMCMP(out, refCipher, sizeof(out)) != 0) + rv = CKR_FUNCTION_FAILED; + (void)funcList->C_Finalize(NULL); + if (rv != CKR_OK) { + fprintf(stderr, " key not usable after crash at rename %d: 0x%lx\n", + crashAt, (unsigned long)rv); + return CHILD_FAIL; + } + return CHILD_PASS; +} + +static void test_pin_change_pass_is_atomic(void) +{ + CK_RV rv; + int res; + int done = 0; + int bad = 0; + + printf("\n--- a PIN change cut short at any rename keeps the key ---\n"); + for (crashAt = 1; !done && bad == 0 && crashAt <= CRASH_MAX_RENAMES; + crashAt++) { + set_aes_check(); + rv = store_objects(create_exportable_aes_key); + if (rv != CKR_OK) { + bad++; + break; + } + res = run_in_child(child_rebind_and_change_pin); + if (res == CHILD_PASS) + done = 1; + else if (res != STORE_EXIT_CODE) + bad++; + if (run_in_child(child_key_under_either_pin) != CHILD_PASS) + bad++; + } + CHECK_TRUE(done, "PIN change runs to completion without the hook"); + CHECK_TRUE(bad == 0, "key usable with old or new PIN after every crash"); + cleanup_test_files(); +} +#endif + +int main(int argc, char* argv[]) +{ + (void)argc; + (void)argv; + XSETENV("WOLFPKCS11_TOKEN_PATH", TEST_DIR, 1); + + printf("=== wolfPKCS11 stored object policy integrity test ===\n"); + test_stored_policy_is_authenticated(); + test_legitimate_updates_round_trip(); + test_failed_store_keeps_state(); +#ifdef DEBUG_WOLFPKCS11 + test_store_pass_is_atomic(); + test_pin_change_pass_is_atomic(); +#endif +#if defined(WOLFPKCS11_MLDSA) || defined(WOLFPKCS11_MLKEM) + test_pq_keys_round_trip(); +#endif +#ifdef HAVE_LEGACY_STORE_TEST + test_unbound_store_upgraded(); +#endif + + return pkcs11_test_summary(); +} + +#else + +int main(int argc, char* argv[]) +{ + (void)argc; + (void)argv; + printf("File-backed token storage not available, skipping test\n"); + return 77; +} + +#endif diff --git a/wolfpkcs11/internal.h b/wolfpkcs11/internal.h index 9d2aec7a..5f59f28f 100644 --- a/wolfpkcs11/internal.h +++ b/wolfpkcs11/internal.h @@ -424,6 +424,12 @@ WP11_API void WP11_Object_SetFindHook(void (*hook)(void)); WP11_API void WP11_Session_SetAddObjectHook(void (*hook)(void)); WP11_API void WP11_Slot_SetPinCheckHook(void (*hook)(void)); WP11_API void WP11_Test_FailTokenInit(int fail); +#if !defined(WOLFPKCS11_NO_STORE) && !defined(WOLFPKCS11_CUSTOM_STORE) && \ + !defined(WOLFPKCS11_TPM_STORE) && !defined(_WIN32) && !defined(_MSC_VER) +/* Exit status of a process ended by the store rename test hook. */ +#define WP11_TEST_STORE_EXIT_CODE 75 +WP11_API void WP11_Test_StoreExitAfterRenames(int renames); +#endif #if defined(WOLFPKCS11_TPM) && (!defined(NO_RSA) || defined(HAVE_ECC)) WP11_API int WP11_Test_DecodeTpmKey(CK_SLOT_ID slotId, unsigned char* keyData, int keyDataLen); From 80fb60444e275c85e6333d292391131a9d36ebde Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 13 Oct 2026 13:33:15 -0700 Subject: [PATCH 15/28] Take the store lock before publishing the store batch slot --- src/internal.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/internal.c b/src/internal.c index 70975161..aedbf6d7 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1983,6 +1983,7 @@ static int wolfPKCS11_StoreBatchBegin(int tokenId) HANDLE h; #endif int fd; + WP11_Slot* slot; #ifndef WOLFPKCS11_NO_ENV /* Storage is turned off, so the pass writes nothing. */ @@ -1990,9 +1991,11 @@ static int wolfPKCS11_StoreBatchBegin(int tokenId) if (str != NULL) return 0; #endif - if (WP11_Slot_Get((CK_SLOT_ID)tokenId, &storeBatch.slot) != 0) + if (WP11_Slot_Get((CK_SLOT_ID)tokenId, &slot) != 0) return BAD_FUNC_ARG; - WP11_Lock_LockRW(&storeBatch.slot->storeLock); + /* The batch state is only written while the store lock is held. */ + WP11_Lock_LockRW(&slot->storeLock); + storeBatch.slot = slot; ret = wolfPKCS11_StoreBatchNames(tokenId); if (ret == 0) ret = wolfPKCS11_StoreEnsureDir(storeBatch.dir); @@ -2030,8 +2033,8 @@ static int wolfPKCS11_StoreBatchBegin(int tokenId) } else { (void)wolfPKCS11_StoreBatchLockFile(0); - WP11_Lock_UnlockRW(&storeBatch.slot->storeLock); storeBatch.slot = NULL; + WP11_Lock_UnlockRW(&slot->storeLock); } return ret; @@ -2082,6 +2085,7 @@ static int wolfPKCS11_StoreStageTemp(WP11_FileStoreCtx* ctx) static int wolfPKCS11_StoreBatchEnd(int commit) { int ret = 0; + WP11_Slot* slot; if (storeBatch.active) { storeBatch.active = 0; @@ -2111,9 +2115,10 @@ static int wolfPKCS11_StoreBatchEnd(int commit) } (void)wolfPKCS11_StoreBatchLockFile(0); } - if (storeBatch.slot != NULL) { - WP11_Lock_UnlockRW(&storeBatch.slot->storeLock); + slot = storeBatch.slot; + if (slot != NULL) { storeBatch.slot = NULL; + WP11_Lock_UnlockRW(&slot->storeLock); } return ret; From e990c0d5aadb4dba0fa982c4bb3915331162c64b Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 13 Oct 2026 13:33:16 -0700 Subject: [PATCH 16/28] Skip store batch recovery when token storage is disabled --- src/internal.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/internal.c b/src/internal.c index aedbf6d7..53a619b5 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1942,6 +1942,11 @@ static int wolfPKCS11_StoreBatchCheck(int tokenId) XFILE marker; WP11_Slot* slot; +#ifndef WOLFPKCS11_NO_ENV + /* Storage is turned off, so nothing is read and nothing is finished. */ + if (XGETENV("WOLFPKCS11_NO_STORE") != NULL) + return 0; +#endif if (WP11_Slot_Get((CK_SLOT_ID)tokenId, &slot) != 0) return 0; WP11_Lock_LockRW(&slot->storeLock); From 014bbf45daf94220741fe4e4f53045542a50f1f0 Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 13 Oct 2026 13:33:16 -0700 Subject: [PATCH 17/28] Hold the store batch locks until the token load finishes --- src/internal.c | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/src/internal.c b/src/internal.c index 53a619b5..860a0076 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1930,18 +1930,22 @@ static int wolfPKCS11_StoreBatchRecover(void) } /** - * Bring the store for a token to a committed state before it is read. + * Bring the store for a token to a committed state before it is read. On + * return the batch locks are held, so the token and object records read + * until wolfPKCS11_StoreBatchRelease belong to one committed pass. * - * @param [in] tokenId Id of token. + * @param [in] tokenId Id of token. + * @param [out] held Slot whose batch locks are held, or NULL. * @return 0 on success. * @return Other value when an interrupted pass could not be finished. */ -static int wolfPKCS11_StoreBatchCheck(int tokenId) +static int wolfPKCS11_StoreBatchCheck(int tokenId, WP11_Slot** held) { int ret = 0; XFILE marker; WP11_Slot* slot; + *held = NULL; #ifndef WOLFPKCS11_NO_ENV /* Storage is turned off, so nothing is read and nothing is finished. */ if (XGETENV("WOLFPKCS11_NO_STORE") != NULL) @@ -1950,10 +1954,10 @@ static int wolfPKCS11_StoreBatchCheck(int tokenId) if (WP11_Slot_Get((CK_SLOT_ID)tokenId, &slot) != 0) return 0; WP11_Lock_LockRW(&slot->storeLock); + *held = slot; if (wolfPKCS11_StoreBatchNames(tokenId) == 0) { if (wolfPKCS11_StoreBatchLockFile(1) == 0) { ret = wolfPKCS11_StoreBatchRecover(); - (void)wolfPKCS11_StoreBatchLockFile(0); } else { /* A committed pass that cannot be finished must not be read. */ @@ -1964,11 +1968,23 @@ static int wolfPKCS11_StoreBatchCheck(int tokenId) } } } - WP11_Lock_UnlockRW(&slot->storeLock); return ret; } +/** + * Release the batch locks held since wolfPKCS11_StoreBatchCheck. + * + * @param [in] held Slot returned by wolfPKCS11_StoreBatchCheck. + */ +static void wolfPKCS11_StoreBatchRelease(WP11_Slot* held) +{ + if (held != NULL) { + (void)wolfPKCS11_StoreBatchLockFile(0); + WP11_Lock_UnlockRW(&held->storeLock); + } +} + /** * Start a store pass: records closed from now on are staged, not committed. * On success the batch locks are held until wolfPKCS11_StoreBatchEnd. Without @@ -8455,11 +8471,15 @@ static int wp11_Token_Load(WP11_Slot* slot, int tokenId, WP11_Token* token) WP11_Object** current; int objCnt = 0; word32 len; +#ifdef WP11_STORE_BATCH + WP11_Slot* batchHeld = NULL; +#endif ret = 0; #ifdef WP11_STORE_BATCH - /* Finish or drop a store pass that a crash interrupted. */ - ret = wolfPKCS11_StoreBatchCheck(tokenId); + /* Finish or drop a store pass that a crash interrupted, then keep other + * store passes out until every record of the token has been read. */ + ret = wolfPKCS11_StoreBatchCheck(tokenId, &batchHeld); #endif /* Open access to token object. */ if (ret == 0) { @@ -8606,6 +8626,9 @@ static int wp11_Token_Load(WP11_Slot* slot, int tokenId, WP11_Token* token) /* No data to read. */ ret = 0; } +#ifdef WP11_STORE_BATCH + wolfPKCS11_StoreBatchRelease(batchHeld); +#endif return ret; } From e17221dd01f99fceb0232de02c0374c1f72422eb Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 13 Oct 2026 13:33:55 -0700 Subject: [PATCH 18/28] Record store batch entries by file name --- src/internal.c | 59 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 43 insertions(+), 16 deletions(-) diff --git a/src/internal.c b/src/internal.c index 860a0076..13f34d5d 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1791,7 +1791,8 @@ static int wolfPKCS11_StoreBatchLockFile(int lock) } /** - * Check that a listed name is a store file directly in the store directory. + * Check that a listed name is the basename of a store file. Lists hold + * basenames so a recovery does not depend on how the directory was spelled. * * @param [in] name Name read from a batch list. * @return 1 when acceptable, 0 otherwise. @@ -1802,11 +1803,9 @@ static int wolfPKCS11_StoreBatchNameOk(const char* name) size_t i; int ok; - ok = len > storeBatch.dirLen + 6 && - XSTRNCMP(name, storeBatch.dir, storeBatch.dirLen) == 0 && - name[storeBatch.dirLen] == '/' && - XSTRNCMP(name + storeBatch.dirLen + 1, "wp11_", 5) == 0; - for (i = storeBatch.dirLen + 1; ok && i < len; i++) { + ok = len > 5 && XSTRNCMP(name, "wp11_", 5) == 0 && + storeBatch.dirLen + 1 + len < WP11_STORE_MAX_PATH; + for (i = 0; ok && i < len; i++) { if (name[i] == '/' || name[i] == '\\') ok = 0; } @@ -1854,7 +1853,10 @@ static int wolfPKCS11_StoreBatchApply(const char* listName, int mode) XFILE list; XFILE staged; size_t len; + size_t i; + const char* base; char line[WP11_STORE_MAX_PATH + 2]; + char finalName[WP11_STORE_MAX_PATH]; char stagedName[WP11_STORE_STAGED_MAX]; list = XFOPEN(listName, "rb"); @@ -1869,9 +1871,24 @@ static int wolfPKCS11_StoreBatchApply(const char* listName, int mode) continue; } line[len - 1] = '\0'; - len = (size_t)XSNPRINTF(stagedName, sizeof(stagedName), "%s.new", - line); - if (!wolfPKCS11_StoreBatchNameOk(line) || len >= sizeof(stagedName)) { + /* A list written with full paths is read by its basenames too. */ + base = line; + for (i = 0; i + 1 < len; i++) { + if (line[i] == '/' || line[i] == '\\') + base = line + i + 1; + } + if (!wolfPKCS11_StoreBatchNameOk(base)) { + if (mode != WP11_BATCH_DISCARD) + ret = READ_ONLY_E; + continue; + } + len = (size_t)XSNPRINTF(finalName, sizeof(finalName), "%s/%s", + storeBatch.dir, base); + if (len < sizeof(finalName)) { + len = (size_t)XSNPRINTF(stagedName, sizeof(stagedName), "%s.new", + finalName); + } + if (len >= sizeof(finalName) || len >= sizeof(stagedName)) { if (mode != WP11_BATCH_DISCARD) ret = READ_ONLY_E; continue; @@ -1880,10 +1897,10 @@ static int wolfPKCS11_StoreBatchApply(const char* listName, int mode) (void)remove(stagedName); } else if (mode == WP11_BATCH_CHECK) { - if (!wolfPKCS11_StoreBatchReplaceable(line)) + if (!wolfPKCS11_StoreBatchReplaceable(finalName)) ret = READ_ONLY_E; } - else if (wolfPKCS11_StoreRename(stagedName, line) != 0) { + else if (wolfPKCS11_StoreRename(stagedName, finalName) != 0) { /* Already renamed by an earlier, interrupted roll forward. */ staged = XFOPEN(stagedName, "rb"); if (staged != XBADFILE) { @@ -2062,7 +2079,8 @@ static int wolfPKCS11_StoreBatchBegin(int tokenId) } /** - * Stage a written record as ".new" and list it in the intent file. + * Stage a written record as ".new" and list its basename in the intent + * file. The record must be in the store directory of the pass. * * @param [in, out] ctx File store context with a written temporary file. * @return 0 on success. @@ -2072,17 +2090,26 @@ static int wolfPKCS11_StoreStageTemp(WP11_FileStoreCtx* ctx) { int ret = 0; int len; + const char* base; char stagedName[WP11_STORE_STAGED_MAX]; - len = XSNPRINTF(stagedName, sizeof(stagedName), "%s.new", - ctx->final_name); - if (len <= 0 || len >= (int)sizeof(stagedName)) + base = ctx->final_name + storeBatch.dirLen + 1; + if (XSTRNCMP(ctx->final_name, storeBatch.dir, storeBatch.dirLen) != 0 || + ctx->final_name[storeBatch.dirLen] != '/' || + !wolfPKCS11_StoreBatchNameOk(base)) { ret = READ_ONLY_E; + } + if (ret == 0) { + len = XSNPRINTF(stagedName, sizeof(stagedName), "%s.new", + ctx->final_name); + if (len <= 0 || len >= (int)sizeof(stagedName)) + ret = READ_ONLY_E; + } if (ret == 0) ret = wolfPKCS11_StoreRename(ctx->temp_name, stagedName); if (ret == 0) { ctx->has_temp = 0; - if (fputs(ctx->final_name, storeBatch.intent) < 0 || + if (fputs(base, storeBatch.intent) < 0 || fputc('\n', storeBatch.intent) == EOF || fflush(storeBatch.intent) != 0) { ret = READ_ONLY_E; From d9ee4f2f8e177fa836a6b191a9a2a500a9b4b04b Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 13 Oct 2026 13:35:04 -0700 Subject: [PATCH 19/28] Sync staged store files before commit --- src/internal.c | 119 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 111 insertions(+), 8 deletions(-) diff --git a/src/internal.c b/src/internal.c index 13f34d5d..6ed704a2 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1314,6 +1314,93 @@ static int wolfPKCS11_StoreRename(const char* from, const char* to) return ret; } +/** + * Flush a written store file to stable storage. + * + * @param [in] file Open file written by the store. + * @return 0 on success. + * @return READ_ONLY_E when the data may not have reached the device. + */ +static int wolfPKCS11_StoreSyncFile(XFILE file) +{ + int ret = 0; +#if defined(_WIN32) || defined(_MSC_VER) + intptr_t osHandle; +#endif + + if (XFFLUSH(file) != 0) + ret = READ_ONLY_E; +#if defined(_WIN32) || defined(_MSC_VER) + if (ret == 0) { + osHandle = _get_osfhandle(_fileno(file)); + if (!FlushFileBuffers((HANDLE)osHandle)) + ret = READ_ONLY_E; + } +#else + if (ret == 0 && fsync(fileno(file)) != 0) + ret = READ_ONLY_E; +#endif + + return ret; +} + +/** + * Make the renames done in a store directory durable. Windows has no + * directory flush; its renames are done write-through instead. + * + * @param [in] dir Store directory. + * @return 0 on success. + * @return READ_ONLY_E when the directory could not be flushed. + */ +static int wolfPKCS11_StoreSyncDir(const char* dir) +{ + int ret = 0; +#if !defined(_WIN32) && !defined(_MSC_VER) + int fd; + + fd = open(dir, O_RDONLY); + if (fd < 0) { + ret = READ_ONLY_E; + } + else { + /* Filesystems without directory flushing answer EINVAL. */ + if (fsync(fd) != 0 && errno != EINVAL) + ret = READ_ONLY_E; + close(fd); + } +#else + (void)dir; +#endif + + return ret; +} + +/** + * Make a rename of a store file durable by flushing its directory. + * + * @param [in] name Renamed store file. + * @return 0 on success. + * @return READ_ONLY_E when the directory could not be flushed. + */ +static int wolfPKCS11_StoreSyncParent(const char* name) +{ + size_t len = XSTRLEN(name); + size_t i; + size_t sep = 0; + char dir[WP11_STORE_MAX_PATH]; + + for (i = 0; i < len; i++) { + if (name[i] == '/' || name[i] == '\\') + sep = i; + } + if (sep == 0 || sep >= sizeof(dir)) + return READ_ONLY_E; + XMEMCPY(dir, name, sep); + dir[sep] = '\0'; + + return wolfPKCS11_StoreSyncDir(dir); +} + static int wolfPKCS11_StoreCommitTemp(WP11_FileStoreCtx* ctx) { int ret = 0; @@ -1325,6 +1412,7 @@ static int wolfPKCS11_StoreCommitTemp(WP11_FileStoreCtx* ctx) ret = wolfPKCS11_StoreRename(ctx->temp_name, ctx->final_name); if (ret == 0) { ctx->has_temp = 0; + ret = wolfPKCS11_StoreSyncParent(ctx->final_name); } return ret; @@ -1934,6 +2022,9 @@ static int wolfPKCS11_StoreBatchRecover(void) XFCLOSE(marker); ret = wolfPKCS11_StoreBatchApply(storeBatch.commitName, WP11_BATCH_APPLY); + /* The marker outlives the records it names until they are durable. */ + if (ret == 0) + ret = wolfPKCS11_StoreSyncDir(storeBatch.dir); if (ret == 0) (void)remove(storeBatch.commitName); } @@ -2137,6 +2228,8 @@ static int wolfPKCS11_StoreBatchEnd(int commit) if (storeBatch.active) { storeBatch.active = 0; + if (commit && wolfPKCS11_StoreSyncFile(storeBatch.intent) != 0) + storeBatch.failed = 1; if (XFCLOSE(storeBatch.intent) != 0) storeBatch.failed = 1; storeBatch.intent = XBADFILE; @@ -2146,13 +2239,18 @@ static int wolfPKCS11_StoreBatchEnd(int commit) ret = wolfPKCS11_StoreBatchApply(storeBatch.intentName, WP11_BATCH_CHECK); } + /* The staged records must be durable before the marker can be. */ + if (ret == 0 && commit) + ret = wolfPKCS11_StoreSyncDir(storeBatch.dir); if (ret == 0 && commit) { ret = wolfPKCS11_StoreRename(storeBatch.intentName, storeBatch.commitName); } if (ret == 0 && commit) { + (void)wolfPKCS11_StoreSyncDir(storeBatch.dir); if (wolfPKCS11_StoreBatchApply(storeBatch.commitName, - WP11_BATCH_APPLY) == 0) { + WP11_BATCH_APPLY) == 0 && + wolfPKCS11_StoreSyncDir(storeBatch.dir) == 0) { (void)remove(storeBatch.commitName); } } @@ -2466,17 +2564,22 @@ void wolfPKCS11_Store_Close(void* store) int commitRet = 0; if (ctx->file != XBADFILE && ctx->file != NULL) { - XFCLOSE(ctx->file); + if (ctx->is_write && wolfPKCS11_StoreSyncFile(ctx->file) != 0) + commitRet = BUFFER_E; + if (XFCLOSE(ctx->file) != 0 && ctx->is_write) + commitRet = BUFFER_E; ctx->file = XBADFILE; } if (ctx->is_write && ctx->has_temp) { - #ifdef WP11_STORE_BATCH - if (storeBatch.active) - commitRet = wolfPKCS11_StoreStageTemp(ctx); - else - #endif - commitRet = wolfPKCS11_StoreCommitTemp(ctx); + if (commitRet == 0) { + #ifdef WP11_STORE_BATCH + if (storeBatch.active) + commitRet = wolfPKCS11_StoreStageTemp(ctx); + else + #endif + commitRet = wolfPKCS11_StoreCommitTemp(ctx); + } if (commitRet != 0) { #ifdef WP11_STORE_BATCH if (storeBatch.active) From 05872b9f900147ba23e882c0edc6aa34d83b5482 Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 13 Oct 2026 13:35:04 -0700 Subject: [PATCH 20/28] Correct the store batch setup comment --- src/internal.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/internal.c b/src/internal.c index 6ed704a2..7e9ba205 100644 --- a/src/internal.c +++ b/src/internal.c @@ -2095,8 +2095,8 @@ static void wolfPKCS11_StoreBatchRelease(WP11_Slot* held) /** * Start a store pass: records closed from now on are staged, not committed. - * On success the batch locks are held until wolfPKCS11_StoreBatchEnd. Without - * a usable store directory the pass runs unstaged and fails as before. + * On success the batch locks are held until wolfPKCS11_StoreBatchEnd. When + * the pass cannot start, nothing is written and the error is returned. * * @param [in] tokenId Id of token being stored. * @return 0 on success. From 84665b1b4d01722e5aec5f21e48bf7e4f6e53d50 Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 13 Oct 2026 13:35:41 -0700 Subject: [PATCH 21/28] Rename WP11_FLAG_UNBOUND to WP11_FLAG_POLICY_EXCLUDED --- src/internal.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/internal.c b/src/internal.c index 7e9ba205..d26cbeba 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4165,11 +4165,12 @@ static int wp11_DecryptData(byte* out, byte* data, int len, byte* key, return ret; } -/* Set by the SO, who never holds the user's key, or by the library itself. */ +/* Flags left out of the bound policy: set by the SO, who never holds the + * user's key, or by the library itself. */ #ifdef WOLFPKCS11_TPM -#define WP11_FLAG_UNBOUND (WP11_FLAG_TRUSTED | WP11_FLAG_TPM) +#define WP11_FLAG_POLICY_EXCLUDED (WP11_FLAG_TRUSTED | WP11_FLAG_TPM) #else -#define WP11_FLAG_UNBOUND WP11_FLAG_TRUSTED +#define WP11_FLAG_POLICY_EXCLUDED WP11_FLAG_TRUSTED #endif #define WP11_POLICY_AAD_VER 1 @@ -4184,7 +4185,7 @@ static int wp11_DecryptData(byte* out, byte* data, int len, byte* key, static void wp11_Object_PolicyAad(WP11_Object* object, byte* aad) { CK_ULONG vals[3]; - word32 flags = object->opFlag & ~(word32)WP11_FLAG_UNBOUND; + word32 flags = object->opFlag & ~(word32)WP11_FLAG_POLICY_EXCLUDED; int idx = 0; int i; int j; @@ -15382,7 +15383,7 @@ static int wp11_Object_PolicyChanged(WP11_Object* object, if (object->onToken && wp11_Object_IsEncrypted(object) && (object->objClass != prevClass || - ((object->opFlag ^ prevFlag) & ~(word32)WP11_FLAG_UNBOUND) != 0)) { + ((object->opFlag ^ prevFlag) & ~(word32)WP11_FLAG_POLICY_EXCLUDED) != 0)) { if (object->encoded || object->undecoded) { object->objClass = prevClass; object->opFlag = prevFlag; From 5ea1e480bf263d34bf72e8434ef1f5290d66385b Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Thu, 15 Oct 2026 08:43:16 -0700 Subject: [PATCH 22/28] Hold the token lock across the SO PIN change store --- src/internal.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/internal.c b/src/internal.c index d26cbeba..a56300f3 100644 --- a/src/internal.c +++ b/src/internal.c @@ -10501,6 +10501,9 @@ int WP11_Slot_SetSOPin(WP11_Slot* slot, char* pin, int pinLen) token->soPinLen = sizeof(token->soPin); token->tokenFlags |= WP11_TOKEN_FLAG_SO_PIN_SET; #ifndef WOLFPKCS11_NO_STORE + /* Lock order is slot, token, then store: no object changes under + * the pass, as in C_CreateObject. */ + WP11_Lock_LockRW(&token->lock); ret = wp11_Token_Store(token, (int)slot->id); #ifdef WP11_STORE_BATCH if (ret != 0 && (oldFlags & WP11_TOKEN_FLAG_SO_PIN_SET)) { @@ -10511,6 +10514,7 @@ int WP11_Slot_SetSOPin(WP11_Slot* slot, char* pin, int pinLen) token->tokenFlags = oldFlags; } #endif + WP11_Lock_UnlockRW(&token->lock); #endif } WP11_Lock_UnlockRW(&slot->lock); From e1e32f2c2cd61c80ea58788ee86e8a7af6943967 Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 13 Oct 2026 13:35:42 -0700 Subject: [PATCH 23/28] Clean up staged store files in the policy test --- tests/object_policy_integrity_test.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/tests/object_policy_integrity_test.c b/tests/object_policy_integrity_test.c index afda742f..c0da2e72 100644 --- a/tests/object_policy_integrity_test.c +++ b/tests/object_policy_integrity_test.c @@ -123,8 +123,18 @@ static void store_path(char* path, size_t sz, const char* kind, int objId) } } +static void remove_record(const char* path) +{ + char staged[280]; + + (void)remove(path); + (void)snprintf(staged, sizeof(staged), "%s.new", path); + (void)remove(staged); +} + static void cleanup_test_files(void) { + static const char* txnKinds[] = { "lock", "intent", "commit" }; char path[256]; size_t k; int i; @@ -132,11 +142,17 @@ static void cleanup_test_files(void) for (k = 0; k < sizeof(storeKinds) / sizeof(*storeKinds); k++) { for (i = 0; i < MAX_OBJ_ID; i++) { store_path(path, sizeof(path), storeKinds[k], i); - (void)remove(path); + remove_record(path); } } store_path(path, sizeof(path), "token", -1); - (void)remove(path); + remove_record(path); + for (k = 0; k < sizeof(txnKinds) / sizeof(*txnKinds); k++) { + (void)snprintf(path, sizeof(path), "%s/wp11_txn_%016lx_%s", TEST_DIR, + 1UL, txnKinds[k]); + if (remove(path) != 0) + (void)rmdir(path); + } } /* Locate the stored file of the given kind for its single object. */ From e79a8d181f12cb4edd7f0804e8301cf82eb498b1 Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 13 Oct 2026 13:35:43 -0700 Subject: [PATCH 24/28] Test store failures with undecoded keys --- tests/object_policy_integrity_test.c | 280 ++++++++++++++++++++++++++- 1 file changed, 275 insertions(+), 5 deletions(-) diff --git a/tests/object_policy_integrity_test.c b/tests/object_policy_integrity_test.c index c0da2e72..fc659127 100644 --- a/tests/object_policy_integrity_test.c +++ b/tests/object_policy_integrity_test.c @@ -1239,6 +1239,14 @@ static void test_unbound_store_upgraded(void) static byte otherUserPin[] = "wolfpkcs11-other"; +/* A directory at the lock file name makes every store pass fail. */ +static int block_store(char* lockPath, size_t sz) +{ + (void)snprintf(lockPath, sz, "%s/wp11_txn_%016lx_lock", TEST_DIR, 1UL); + (void)unlink(lockPath); + return (mkdir(lockPath, 0700) == 0) ? 0 : -1; +} + /* Store passes that cannot start must leave the PIN and token in effect. */ static int child_failed_store_keeps_state(void) { @@ -1252,17 +1260,13 @@ static int child_failed_store_keeps_state(void) byte out[BLOCK_SZ]; char lockPath[256]; - (void)snprintf(lockPath, sizeof(lockPath), "%s/wp11_txn_%016lx_lock", - TEST_DIR, 1UL); XMEMSET(label, ' ', sizeof(label)); if (pkcs11_load() != CKR_OK) return CHILD_SETUP; rv = init_slot(&slot); if (rv == CKR_OK) rv = open_session(slot, 1, &session); - /* A directory at the lock file name makes every store pass fail. */ - (void)unlink(lockPath); - if (rv == CKR_OK && mkdir(lockPath, 0700) != 0) + if (rv == CKR_OK && block_store(lockPath, sizeof(lockPath)) != 0) rv = CKR_GENERAL_ERROR; if (rv == CKR_OK) { setRv = funcList->C_SetPIN(session, userPin, sizeof(userPin) - 1, @@ -1306,6 +1310,269 @@ static void test_failed_store_keeps_state(void) cleanup_test_files(); } +/* Create a public AES key on the token and change its policy so the next + * store pass has to rebind it. The key is only decoded in this process. */ +static CK_RV create_unbound_public_key(CK_SESSION_HANDLE session) +{ + CK_RV rv; + CK_OBJECT_CLASS cls = CKO_SECRET_KEY; + CK_KEY_TYPE keyType = CKK_AES; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_ATTRIBUTE tmpl[] = { + { CKA_CLASS, &cls, sizeof(cls) }, + { CKA_KEY_TYPE, &keyType, sizeof(keyType) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIVATE, &ckFalse, sizeof(ckFalse) }, + { CKA_ENCRYPT, &ckTrue, sizeof(ckTrue) }, + { CKA_DECRYPT, &ckTrue, sizeof(ckTrue) }, + { CKA_VALUE, aesKeyValue, sizeof(aesKeyValue) }, + }; + CK_ATTRIBUTE policy[] = { + { CKA_DECRYPT, &ckFalse, sizeof(ckFalse) }, + }; + + rv = funcList->C_CreateObject(session, tmpl, + sizeof(tmpl) / sizeof(*tmpl), &obj); + if (rv == CKR_OK) + rv = funcList->C_SetAttributeValue(session, obj, policy, 1); + return rv; +} + +/* With the user's key never decoded in this process, a PIN change whose + * store fails must not touch the stored key. */ +static int child_undecoded_keys_survive_failed_pin_change(int asSO) +{ + CK_RV rv; + CK_RV setRv = CKR_OK; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + char lockPath[256]; + + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = open_session(slot, 0, &session); + if (rv == CKR_OK && asSO) + rv = funcList->C_Login(session, CKU_SO, soPin, sizeof(soPin) - 1); + if (rv == CKR_OK) + rv = create_unbound_public_key(session); + if (rv == CKR_OK && block_store(lockPath, sizeof(lockPath)) != 0) + rv = CKR_GENERAL_ERROR; + if (rv == CKR_OK && asSO) { + setRv = funcList->C_InitPIN(session, otherUserPin, + sizeof(otherUserPin) - 1); + } + else if (rv == CKR_OK) { + setRv = funcList->C_SetPIN(session, userPin, sizeof(userPin) - 1, + otherUserPin, sizeof(otherUserPin) - 1); + } + (void)rmdir(lockPath); + if (rv == CKR_OK && setRv == CKR_OK) + rv = CKR_GENERAL_ERROR; + (void)funcList->C_Finalize(NULL); + if (rv != CKR_OK) { + fprintf(stderr, " forced PIN change failure not set up: 0x%lx\n", + (unsigned long)rv); + return CHILD_FAIL; + } + return CHILD_PASS; +} + +static int child_public_pin_change_fails(void) +{ + return child_undecoded_keys_survive_failed_pin_change(0); +} + +static int child_so_init_pin_fails(void) +{ + return child_undecoded_keys_survive_failed_pin_change(1); +} + +/* The user's key still works under the PIN that stayed in storage. */ +static int child_user_key_works(void) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + byte out[BLOCK_SZ]; + + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = open_session(slot, 1, &session); + if (rv == CKR_OK) + rv = find_key(session, &obj); + if (rv == CKR_OK) + rv = aes_encrypt_block(session, obj, out); + if (rv == CKR_OK && XMEMCMP(out, refCipher, sizeof(out)) != 0) + rv = CKR_FUNCTION_FAILED; + (void)funcList->C_Finalize(NULL); + if (rv != CKR_OK) { + fprintf(stderr, " stored key lost by a failed PIN change: 0x%lx\n", + (unsigned long)rv); + return CHILD_FAIL; + } + return CHILD_PASS; +} + +static void test_failed_store_keeps_undecoded_keys(void) +{ + CK_RV rv; + + printf("\n--- failed PIN change with undecoded keys keeps them ---\n"); + set_aes_check(); + rv = store_objects(create_aes_key); + CHECK_RV(rv, "store AES key", CKR_OK); + if (rv == CKR_OK) { + CHECK_TRUE(run_in_child(child_public_pin_change_fails) == CHILD_PASS, + "public session PIN change store failure is forced"); + CHECK_TRUE(run_in_child(child_user_key_works) == CHILD_PASS, + "key survives a failed public session PIN change"); + } + cleanup_test_files(); + rv = store_objects(create_aes_key); + CHECK_RV(rv, "store AES key", CKR_OK); + if (rv == CKR_OK) { + CHECK_TRUE(run_in_child(child_so_init_pin_fails) == CHILD_PASS, + "SO user PIN init store failure is forced"); + CHECK_TRUE(run_in_child(child_user_key_works) == CHILD_PASS, + "key survives a failed SO user PIN init"); + } + cleanup_test_files(); +} + +/* A token init whose store fails leaves nothing behind, and a retry works. */ +static int child_init_token_retry(void) +{ + CK_RV rv; + CK_RV initRv = CKR_OK; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_TOKEN_INFO info; + char lockPath[256]; + + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv == CKR_OK && block_store(lockPath, sizeof(lockPath)) != 0) + rv = CKR_GENERAL_ERROR; + if (rv == CKR_OK) { + initRv = funcList->C_InitToken(slot, soPin, sizeof(soPin) - 1, + (CK_UTF8CHAR*)"wolfpkcs11 " + " "); + (void)rmdir(lockPath); + if (initRv == CKR_OK) + rv = CKR_GENERAL_ERROR; + } + if (rv == CKR_OK) + rv = funcList->C_GetTokenInfo(slot, &info); + if (rv == CKR_OK && (info.flags & CKF_TOKEN_INITIALIZED) != 0) + rv = CKR_GENERAL_ERROR; + if (rv == CKR_OK) + rv = provision_token(slot); + if (rv == CKR_OK) + rv = open_session(slot, 1, &session); + (void)funcList->C_Finalize(NULL); + if (rv != CKR_OK) { + fprintf(stderr, " token init retry failed: 0x%lx\n", + (unsigned long)rv); + return CHILD_FAIL; + } + return CHILD_PASS; +} + +static void test_init_token_failure_on_fresh_store(void) +{ + printf("\n--- failed token init on a fresh store can be retried ---\n"); + cleanup_test_files(); + CHECK_TRUE(run_in_child(child_init_token_retry) == CHILD_PASS, + "token init works after a failed init of a fresh store"); + cleanup_test_files(); +} + +#define ALT_TEST_DIR "store/./object_policy_integrity_test" + +static byte stagedRecord[] = "staged-record"; + +/* Load the token with the store directory spelled differently. */ +static int child_load_from_alt_dir(void) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + + XSETENV("WOLFPKCS11_TOKEN_PATH", ALT_TEST_DIR, 1); + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = open_session(slot, 1, &session); + if (rv == CKR_OK) + rv = find_key(session, &obj); + (void)funcList->C_Finalize(NULL); + if (rv != CKR_OK) { + fprintf(stderr, " load from the renamed store dir failed: 0x%lx\n", + (unsigned long)rv); + return CHILD_FAIL; + } + return CHILD_PASS; +} + +/* A committed pass left by a crash is finished by a process that reaches the + * store directory through a different path, whether the marker lists the + * records by basename or by the path the crashed process used. */ +static void test_recovery_ignores_dir_spelling(void) +{ + CK_RV rv; + char path[256]; + char fullPath[256]; + char staged[280]; + char marker[256]; + char lines[600]; + static byte data[MAX_FILE_SZ]; + size_t sz = 0; + int res; + + printf("\n--- committed pass is finished from any store dir path ---\n"); + set_aes_check(); + rv = store_objects(create_aes_key); + CHECK_RV(rv, "store AES key", CKR_OK); + store_path(path, sizeof(path), "data", MAX_OBJ_ID - 1); + store_path(fullPath, sizeof(fullPath), "data", MAX_OBJ_ID - 2); + (void)snprintf(marker, sizeof(marker), "%s/wp11_txn_%016lx_commit", + TEST_DIR, 1UL); + (void)snprintf(lines, sizeof(lines), "wp11_data_%016lx_%016lx\n%s\n", 1UL, + (unsigned long)(MAX_OBJ_ID - 1), fullPath); + if (rv == CKR_OK) { + (void)snprintf(staged, sizeof(staged), "%s.new", path); + CHECK_TRUE(write_file(staged, stagedRecord, sizeof(stagedRecord)) == 0, + "stage a record"); + (void)snprintf(staged, sizeof(staged), "%s.new", fullPath); + CHECK_TRUE(write_file(staged, stagedRecord, sizeof(stagedRecord)) == 0, + "stage a second record"); + CHECK_TRUE(write_file(marker, (const byte*)lines, XSTRLEN(lines)) == 0, + "write a commit marker naming both"); + res = run_in_child(child_load_from_alt_dir); + CHECK_TRUE(res == CHILD_PASS, "token loads through the other path"); + CHECK_TRUE(read_file(path, data, sizeof(data), &sz) == 0 && + sz == sizeof(stagedRecord) && + XMEMCMP(data, stagedRecord, sz) == 0, + "record listed by basename was put in place"); + CHECK_TRUE(read_file(fullPath, data, sizeof(data), &sz) == 0 && + sz == sizeof(stagedRecord) && + XMEMCMP(data, stagedRecord, sz) == 0, + "record listed by full path was put in place"); + (void)snprintf(staged, sizeof(staged), "%s.new", path); + CHECK_TRUE(access(staged, F_OK) != 0, "staged copy is gone"); + CHECK_TRUE(access(marker, F_OK) != 0, "commit marker is gone"); + } + cleanup_test_files(); +} + #ifdef DEBUG_WOLFPKCS11 /* Matches WP11_TEST_STORE_EXIT_CODE in wolfpkcs11/internal.h. */ #define STORE_EXIT_CODE 75 @@ -1527,6 +1794,9 @@ int main(int argc, char* argv[]) test_stored_policy_is_authenticated(); test_legitimate_updates_round_trip(); test_failed_store_keeps_state(); + test_failed_store_keeps_undecoded_keys(); + test_init_token_failure_on_fresh_store(); + test_recovery_ignores_dir_spelling(); #ifdef DEBUG_WOLFPKCS11 test_store_pass_is_atomic(); test_pin_change_pass_is_atomic(); From d8d715fae157a63b366153001640da8c6b7bd1aa Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 13 Oct 2026 13:35:43 -0700 Subject: [PATCH 25/28] Cache the empty user PIN verdict when loading a token --- src/internal.c | 47 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/src/internal.c b/src/internal.c index a56300f3..e6d9c644 100644 --- a/src/internal.c +++ b/src/internal.c @@ -8584,7 +8584,35 @@ static int wp11_token_write_seed(void* storage, WP11_Token* token) } /** - * Load a token from storage. + * Check for an empty user PIN while loading, when the caller alone can reach + * the token, and cache the verdict as WP11_Slot_Has_Empty_Pin would. + * + * @param [in] slot Slot object. + * @param [in, out] token Token being loaded. + * @return 1 when the user PIN is set and empty, 0 otherwise. + */ +static int wp11_Token_HasEmptyPin(WP11_Slot* slot, WP11_Token* token) +{ + int ret = 0; + byte hash[PIN_HASH_SZ]; + + if ((token->tokenFlags & WP11_TOKEN_FLAG_USER_PIN_SET) != 0) { + if (HashPIN((char*)"", 0, token->userPinSeed, + sizeof(token->userPinSeed), hash, sizeof(hash), slot) == 0) { + ret = WP11_ConstantCompare(hash, token->userPin, + token->userPinLen) != 0; + } + token->userPinEmpty = ret ? 1 : 2; + wc_ForceZero(hash, sizeof(hash)); + } + + return ret; +} + +/** + * Load a token from storage. Runs without the slot lock from library + * initialization and under it from a token reset; neither its callees nor + * this function take the slot lock. * * @param [in, out] object Token object. * @param [in] tokenId Id of token this key belongs to. @@ -8736,7 +8764,7 @@ static int wp11_Token_Load(WP11_Slot* slot, int tokenId, WP11_Token* token) } /* If there is no pin, there is no login, so decode now */ - if (WP11_Slot_Has_Empty_Pin(slot) && (ret == 0)) { + if (ret == 0 && wp11_Token_HasEmptyPin(slot, token)) { #ifndef WOLFPKCS11_NO_STORE /* Derive token->key from empty PIN + seed before decoding */ ret = HashPIN((char*)"", 0, token->seed, sizeof(token->seed), @@ -9690,17 +9718,18 @@ int WP11_Slot_TokenReset(WP11_Slot* slot, char* pin, int pinLen, char* label) #ifdef WP11_STORE_BATCH /* Only a staged store pass is known to leave storage unchanged. */ if (ret != 0) { - /* Reset not stored: restore the token that is still in storage. */ + /* Reset not stored: restore the token that is still in storage, + * under the lock so no session sees it half loaded. */ XMEMSET(emptyLabel, 0, sizeof(emptyLabel)); WP11_Lock_LockRW(&slot->lock); wp11_Token_Final(token); - if (wp11_Token_Init(token, emptyLabel) == 0) { - WP11_Lock_UnlockRW(&slot->lock); - (void)wp11_Slot_Load(slot, (int)slot->id); - } - else { - WP11_Lock_UnlockRW(&slot->lock); + if (wp11_Token_Init(token, emptyLabel) == 0 && + wp11_Slot_Load(slot, (int)slot->id) != 0) { + /* Storage still holds the token; leave a blank one here. */ + wp11_Token_Final(token); + (void)wp11_Token_Init(token, emptyLabel); } + WP11_Lock_UnlockRW(&slot->lock); } #endif } From a5706b858e3dc192afcdc5bb7df77e7875bf882a Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Thu, 15 Oct 2026 08:43:27 -0700 Subject: [PATCH 26/28] Test that a stored PIN change needs no logout to take effect --- tests/object_policy_integrity_test.c | 76 ++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/tests/object_policy_integrity_test.c b/tests/object_policy_integrity_test.c index fc659127..dd98ba31 100644 --- a/tests/object_policy_integrity_test.c +++ b/tests/object_policy_integrity_test.c @@ -1444,6 +1444,81 @@ static void test_failed_store_keeps_undecoded_keys(void) cleanup_test_files(); } +/* Change the user PIN and end without logging out or finalizing. */ +static int child_change_pin_and_exit(void) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = open_session(slot, 1, &session); + if (rv == CKR_OK) { + rv = funcList->C_SetPIN(session, userPin, sizeof(userPin) - 1, + otherUserPin, sizeof(otherUserPin) - 1); + } + if (rv != CKR_OK) { + fprintf(stderr, " PIN change failed: 0x%lx\n", (unsigned long)rv); + return CHILD_FAIL; + } + return CHILD_PASS; +} + +/* The key works after logging in with the new PIN. */ +static int child_key_under_new_pin(void) +{ + CK_RV rv; + CK_SLOT_ID slot = 0; + CK_SESSION_HANDLE session = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + byte out[BLOCK_SZ]; + + if (pkcs11_load() != CKR_OK) + return CHILD_SETUP; + rv = init_slot(&slot); + if (rv == CKR_OK) + rv = open_session(slot, 0, &session); + if (rv == CKR_OK) { + rv = funcList->C_Login(session, CKU_USER, otherUserPin, + sizeof(otherUserPin) - 1); + } + if (rv == CKR_OK) + rv = find_key(session, &obj); + if (rv == CKR_OK) + rv = aes_encrypt_block(session, obj, out); + if (rv == CKR_OK && XMEMCMP(out, refCipher, sizeof(out)) != 0) + rv = CKR_FUNCTION_FAILED; + (void)funcList->C_Finalize(NULL); + if (rv != CKR_OK) { + fprintf(stderr, " key not usable under the new PIN: 0x%lx\n", + (unsigned long)rv); + return CHILD_FAIL; + } + return CHILD_PASS; +} + +/* The store pass of a PIN change must itself leave every key usable under + * the new PIN, without a later logout or finalize rewriting the records. */ +static void test_pin_change_is_complete_when_stored(void) +{ + CK_RV rv; + + printf("\n--- a stored PIN change needs no logout to take effect ---\n"); + set_aes_check(); + rv = store_objects(create_aes_key); + CHECK_RV(rv, "store AES key", CKR_OK); + if (rv == CKR_OK) { + CHECK_TRUE(run_in_child(child_change_pin_and_exit) == CHILD_PASS, + "PIN change stored"); + CHECK_TRUE(run_in_child(child_key_under_new_pin) == CHILD_PASS, + "key usable under the new PIN after an exit"); + } + cleanup_test_files(); +} + /* A token init whose store fails leaves nothing behind, and a retry works. */ static int child_init_token_retry(void) { @@ -1795,6 +1870,7 @@ int main(int argc, char* argv[]) test_legitimate_updates_round_trip(); test_failed_store_keeps_state(); test_failed_store_keeps_undecoded_keys(); + test_pin_change_is_complete_when_stored(); test_init_token_failure_on_fresh_store(); test_recovery_ignores_dir_spelling(); #ifdef DEBUG_WOLFPKCS11 From 64f8bcdc1d872b03fc54716806385cc36d79f605 Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Tue, 13 Oct 2026 13:35:45 -0700 Subject: [PATCH 27/28] Tidy store batch comments --- src/internal.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/internal.c b/src/internal.c index e6d9c644..d02e2204 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4165,8 +4165,7 @@ static int wp11_DecryptData(byte* out, byte* data, int len, byte* key, return ret; } -/* Flags left out of the bound policy: set by the SO, who never holds the - * user's key, or by the library itself. */ +/* Left out of the bound policy: set by the SO or the library, not the user. */ #ifdef WOLFPKCS11_TPM #define WP11_FLAG_POLICY_EXCLUDED (WP11_FLAG_TRUSTED | WP11_FLAG_TPM) #else @@ -8636,8 +8635,7 @@ static int wp11_Token_Load(WP11_Slot* slot, int tokenId, WP11_Token* token) ret = 0; #ifdef WP11_STORE_BATCH - /* Finish or drop a store pass that a crash interrupted, then keep other - * store passes out until every record of the token has been read. */ + /* Finish or drop an interrupted pass, then keep passes out until read. */ ret = wolfPKCS11_StoreBatchCheck(tokenId, &batchHeld); #endif /* Open access to token object. */ @@ -9718,8 +9716,7 @@ int WP11_Slot_TokenReset(WP11_Slot* slot, char* pin, int pinLen, char* label) #ifdef WP11_STORE_BATCH /* Only a staged store pass is known to leave storage unchanged. */ if (ret != 0) { - /* Reset not stored: restore the token that is still in storage, - * under the lock so no session sees it half loaded. */ + /* Reload under the lock so no session sees a half loaded token. */ XMEMSET(emptyLabel, 0, sizeof(emptyLabel)); WP11_Lock_LockRW(&slot->lock); wp11_Token_Final(token); @@ -10530,8 +10527,7 @@ int WP11_Slot_SetSOPin(WP11_Slot* slot, char* pin, int pinLen) token->soPinLen = sizeof(token->soPin); token->tokenFlags |= WP11_TOKEN_FLAG_SO_PIN_SET; #ifndef WOLFPKCS11_NO_STORE - /* Lock order is slot, token, then store: no object changes under - * the pass, as in C_CreateObject. */ + /* Order is slot, token, storeLock: no object changes under the pass. */ WP11_Lock_LockRW(&token->lock); ret = wp11_Token_Store(token, (int)slot->id); #ifdef WP11_STORE_BATCH @@ -15416,7 +15412,8 @@ static int wp11_Object_PolicyChanged(WP11_Object* object, if (object->onToken && wp11_Object_IsEncrypted(object) && (object->objClass != prevClass || - ((object->opFlag ^ prevFlag) & ~(word32)WP11_FLAG_POLICY_EXCLUDED) != 0)) { + ((object->opFlag ^ prevFlag) & + ~(word32)WP11_FLAG_POLICY_EXCLUDED) != 0)) { if (object->encoded || object->undecoded) { object->objClass = prevClass; object->opFlag = prevFlag; From c988a2fb8e0fc474ec61108a05770e1d508bcbee Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Thu, 15 Oct 2026 08:43:55 -0700 Subject: [PATCH 28/28] Publish SO PIN changes under the token lock --- src/internal.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/internal.c b/src/internal.c index d02e2204..e9db7767 100644 --- a/src/internal.c +++ b/src/internal.c @@ -10495,6 +10495,8 @@ int WP11_Slot_SetSOPin(WP11_Slot* slot, char* pin, int pinLen) { int ret = 0; WP11_Token* token; + byte pinSeed[PIN_SEED_SZ]; + byte pinHash[PIN_HASH_SZ]; #ifdef WP11_STORE_BATCH byte oldPin[PIN_HASH_SZ]; byte oldPinSeed[PIN_SEED_SZ]; @@ -10512,23 +10514,25 @@ int WP11_Slot_SetSOPin(WP11_Slot* slot, char* pin, int pinLen) #endif /* New seed each time. */ WP11_Lock_LockRW(&slot->token.rngLock); - ret = wc_RNG_GenerateBlock(&slot->token.rng, token->soPinSeed, - sizeof(token->soPinSeed)); + ret = wc_RNG_GenerateBlock(&slot->token.rng, pinSeed, sizeof(pinSeed)); WP11_Lock_UnlockRW(&slot->token.rngLock); if (ret == 0) { WP11_Lock_UnlockRW(&slot->lock); /* Costly Operation done out of lock. */ - ret = HashPIN(pin, pinLen, token->soPinSeed, - sizeof(token->soPinSeed), token->soPin, - sizeof(token->soPin), slot); + ret = HashPIN(pin, pinLen, pinSeed, sizeof(pinSeed), pinHash, + sizeof(pinHash), slot); WP11_Lock_LockRW(&slot->lock); } if (ret == 0) { + #ifndef WOLFPKCS11_NO_STORE + /* Order is slot, token, storeLock: no store runs from here on. */ + WP11_Lock_LockRW(&token->lock); + #endif + XMEMCPY(token->soPinSeed, pinSeed, sizeof(token->soPinSeed)); + XMEMCPY(token->soPin, pinHash, sizeof(token->soPin)); token->soPinLen = sizeof(token->soPin); token->tokenFlags |= WP11_TOKEN_FLAG_SO_PIN_SET; #ifndef WOLFPKCS11_NO_STORE - /* Order is slot, token, storeLock: no object changes under the pass. */ - WP11_Lock_LockRW(&token->lock); ret = wp11_Token_Store(token, (int)slot->id); #ifdef WP11_STORE_BATCH if (ret != 0 && (oldFlags & WP11_TOKEN_FLAG_SO_PIN_SET)) { @@ -10543,6 +10547,7 @@ int WP11_Slot_SetSOPin(WP11_Slot* slot, char* pin, int pinLen) #endif } WP11_Lock_UnlockRW(&slot->lock); + wc_ForceZero(pinHash, sizeof(pinHash)); #ifdef WP11_STORE_BATCH wc_ForceZero(oldPin, sizeof(oldPin)); wc_ForceZero(oldPinSeed, sizeof(oldPinSeed));