From 9aea9858ee4114d46861d57612e2f79785f417fb Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Wed, 15 Jul 2026 23:12:40 +0000 Subject: [PATCH 01/14] Log out application when all sessions are closed (F-6492) WP11_Slot_CloseSessions freed/finalized every session but never reset the token login state, unlike the single-session WP11_Slot_CloseSession path which calls WP11_Slot_Logout once no in-use sessions remain. Because loginState is token-level, a session opened after C_CloseAllSessions inherited the stale RW_USER/RW_SO state and was treated as logged in without any C_Login, exposing CKA_PRIVATE objects and SO/User functions. Call WP11_Slot_Logout after closing all sessions, mirroring the single-session close path. --- src/internal.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/internal.c b/src/internal.c index dad45296..3ed61180 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7050,6 +7050,12 @@ void WP11_Slot_CloseSessions(WP11_Slot* slot) for (curr = slot->session; curr != NULL; curr = curr->next) wp11_Session_Final(curr); WP11_Lock_UnlockRW(&slot->lock); + + /* PKCS#11: closing an application's last session with a token logs the + * application out. Mirror the single-session close path and reset the + * token login state (outside the slot lock, as WP11_Slot_Logout takes it). + */ + WP11_Slot_Logout(slot); } /** From a77d1cf51312a25fd1ada80ed575fcf3b5aaf1c0 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Wed, 15 Jul 2026 23:13:28 +0000 Subject: [PATCH 02/14] Reset AES-CBC streaming state in WP11_Session_SetCbcParams (F-6229) session->params is a union shared by all mechanisms and is only zeroed at session allocation. Every other Set*Params routine re-initializes its parameters, but WP11_Session_SetCbcParams left cbc->partial/partialSz untouched. A CBC operation following a different mechanism could therefore read a stale partialSz: values 1..15 silently corrupt the stream, and a value >16 makes sz = AES_BLOCK_SIZE - partialSz negative, which widens to a huge size_t in the XMEMCPY and overflows the 16-byte partial buffer. Zero partial/partialSz at (re)initialization, matching SetGcmParams. --- src/internal.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/internal.c b/src/internal.c index 3ed61180..f72f25b6 100644 --- a/src/internal.c +++ b/src/internal.c @@ -8523,6 +8523,13 @@ int WP11_Session_SetCbcParams(WP11_Session* session, unsigned char* iv, WP11_CbcParams* cbc = &session->params.cbc; WP11_Data* key; + /* The session params union is shared by every mechanism and is only zeroed + * at allocation, so a prior operation can leave stale multi-part streaming + * state here. Reset it before use (as the other Set*Params routines do) so + * a fresh CBC operation cannot inherit a bogus partial-block count. */ + cbc->partialSz = 0; + XMEMSET(cbc->partial, 0, sizeof(cbc->partial)); + /* AES object on session. */ ret = wc_AesInit(&cbc->aes, NULL, object->devId); #ifdef WOLFSSL_STM32U5_DHUK From 3e68058f9c1dd9b3380725282270fabf04becfb7 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Wed, 15 Jul 2026 23:15:15 +0000 Subject: [PATCH 03/14] Require user login to copy an object as CKA_PRIVATE=TRUE (F-5762) C_CopyObject created the new object's CKA_PRIVATE flag from the caller template without checking the session login state. A public (unauthenticated) session holding any copyable non-private object handle could call C_CopyObject with {CKA_PRIVATE=TRUE, CKA_TOKEN=TRUE} to create a persistent private token object without ever calling C_Login. The R/W gate only covered CKA_TOKEN placement and WP11_Object_Find only enforced login on the source object. Gate the copy with CheckPrivateLogin (the same helper used by C_CreateObject/C_GenerateKey for Fenrir 3144). WP11_NO_IMPLICIT_CLASS is used so only an explicit template CKA_PRIVATE=TRUE triggers the check - the inherited default already had its login enforced on the source object. --- src/crypto.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/crypto.c b/src/crypto.c index b4ce9c9e..45daabff 100644 --- a/src/crypto.c +++ b/src/crypto.c @@ -1583,6 +1583,18 @@ CK_RV C_CopyObject(CK_SESSION_HANDLE hSession, CK_OBJECT_HANDLE hObject, return rv; } + /* Creating a private object requires an authenticated user session. The + * copy template can set CKA_PRIVATE=CK_TRUE, so gate it like + * C_CreateObject. The copy's default CKA_PRIVATE is inherited from the + * source object, whose own login requirement was already enforced by + * WP11_Object_Find above, so only an explicit template override is checked + * here (WP11_NO_IMPLICIT_CLASS = template inspection only). */ + rv = CheckPrivateLogin(session, pTemplate, ulCount, WP11_NO_IMPLICIT_CLASS); + if (rv != CKR_OK) { + WOLFPKCS11_LEAVE("C_CopyObject", rv); + return rv; + } + keyType = WP11_Object_GetType(obj); /* The copy inherits the source's CKA_TOKEN (PKCS#11 v2.40 4.6.2) unless From ecae114ba31697e0ddc8e72dd5168191aeedfcdf Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Wed, 15 Jul 2026 23:24:04 +0000 Subject: [PATCH 04/14] Require SO session to set CKA_TRUSTED (F-5867) SetAttributeValue guarded CKA_SENSITIVE/EXTRACTABLE/COPYABLE/DESTROYABLE but not CKA_TRUSTED. Per PKCS#11 v2.40 sec 4.5 only the SO may set CKA_TRUSTED to CK_TRUE. Without the guard a regular-user session could forge CKA_TRUSTED on a wrapping key it owns (via C_SetAttributeValue or directly in C_CreateObject) and then use C_WrapKey to export keys protected by CKA_WRAP_WITH_TRUSTED, defeating that export control. Reject setting CKA_TRUSTED=CK_TRUE from any non-SO session. The check is not qualified with newObject so it also blocks C_CreateObject/C_GenerateKey from minting a trusted key. De-trusting (CK_TRUE->CK_FALSE) is left unrestricted: PKCS#11 does not make CKA_TRUSTED sticky. test_wrap_key_wrap_with_trusted created the trusted wrapping key under the harness user session, which the guard now correctly rejects. Rework it to provision the trusted key as a public token object under an SO session (logout user / login SO / create / logout SO / login user) and add an assertion that a user session forging CKA_TRUSTED is rejected. --- src/crypto.c | 11 +++++++++++ tests/pkcs11test.c | 43 +++++++++++++++++++++++++++++++++++++++---- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/crypto.c b/src/crypto.c index 45daabff..a01e18af 100644 --- a/src/crypto.c +++ b/src/crypto.c @@ -1028,6 +1028,17 @@ static CK_RV SetAttributeValue(WP11_Session* session, WP11_Object* obj, *(CK_BBOOL*)attr->pValue == CK_TRUE) return CKR_ATTRIBUTE_READ_ONLY; } + /* PKCS#11 v2.40 sec 4.5: only an SO session may set CKA_TRUSTED to + * CK_TRUE. A regular-user session must not forge trust and bypass the + * CKA_WRAP_WITH_TRUSTED export gate enforced by C_WrapKey. Not + * qualified with !newObject so it also stops C_CreateObject / + * C_GenerateKey from minting a trusted key. CheckAttributes above has + * already validated CKA_TRUSTED as a well-formed CK_BBOOL. */ + if (attr->type == CKA_TRUSTED && + *(CK_BBOOL*)attr->pValue == CK_TRUE && + WP11_Session_GetState(session) != WP11_APP_STATE_RW_SO) { + return CKR_ATTRIBUTE_READ_ONLY; + } /* These class/identity and generated-state attributes are read-only * once the object exists; reject a change. Setting the current value * is a no-op. */ diff --git a/tests/pkcs11test.c b/tests/pkcs11test.c index e23c3f49..71a9355f 100644 --- a/tests/pkcs11test.c +++ b/tests/pkcs11test.c @@ -6538,12 +6538,16 @@ static CK_RV test_wrap_key_wrap_with_trusted(void* args) }; CK_ULONG untrustedWrapTmplCnt = sizeof(untrustedWrapTmpl) / sizeof(*untrustedWrapTmpl); + /* CKA_TRUSTED may only be set by the SO, so the trusted wrapping key is + * provisioned as a public token object under an SO session. */ CK_ATTRIBUTE trustedWrapTmpl[] = { { CKA_CLASS, &secretKeyClass, sizeof(secretKeyClass) }, { CKA_KEY_TYPE, &aesKeyType, sizeof(aesKeyType) }, { CKA_VALUE, aes_128_key, sizeof(aes_128_key) }, { CKA_WRAP, &ckTrue, sizeof(ckTrue) }, { CKA_TRUSTED, &ckTrusted, sizeof(CK_BBOOL) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIVATE, &ckFalse, sizeof(ckFalse) }, }; CK_ULONG trustedWrapTmplCnt = sizeof(trustedWrapTmpl) / sizeof(*trustedWrapTmpl); @@ -6558,13 +6562,44 @@ static CK_RV test_wrap_key_wrap_with_trusted(void* args) memset(keyData, 0x55, sizeof(keyData)); - ret = funcList->C_CreateObject(session, untrustedWrapTmpl, - untrustedWrapTmplCnt, &untrustedKey); - CHECK_CKR(ret, "Create untrusted AES wrapping key"); + /* Provision the trusted wrapping key as the SO. The token has a single + * login state, so log the harness user out, log in as SO to create the + * public token key, then restore the user session. */ + ret = funcList->C_Logout(session); + CHECK_CKR(ret, "Logout user before SO provisioning"); + if (ret == CKR_OK) { + ret = funcList->C_Login(session, CKU_SO, soPin, soPinLen); + CHECK_CKR(ret, "Login SO to create trusted key"); + } if (ret == CKR_OK) { ret = funcList->C_CreateObject(session, trustedWrapTmpl, trustedWrapTmplCnt, &trustedKey); - CHECK_CKR(ret, "Create trusted AES wrapping key"); + CHECK_CKR(ret, "Create trusted AES wrapping key (SO)"); + } + if (ret == CKR_OK) { + ret = funcList->C_Logout(session); + CHECK_CKR(ret, "Logout SO after provisioning"); + } + if (ret == CKR_OK) { + ret = funcList->C_Login(session, CKU_USER, userPin, userPinLen); + CHECK_CKR(ret, "Re-login user after SO provisioning"); + } + + /* A user session must not be able to forge CKA_TRUSTED (F-5867). */ + if (ret == CKR_OK) { + CK_OBJECT_HANDLE forgedKey = CK_INVALID_HANDLE; + ret = funcList->C_CreateObject(session, trustedWrapTmpl, + trustedWrapTmplCnt, &forgedKey); + if (ret == CKR_OK) + funcList->C_DestroyObject(session, forgedKey); + CHECK_CKR_FAIL(ret, CKR_ATTRIBUTE_READ_ONLY, + "User session forging CKA_TRUSTED must be rejected"); + } + + if (ret == CKR_OK) { + ret = funcList->C_CreateObject(session, untrustedWrapTmpl, + untrustedWrapTmplCnt, &untrustedKey); + CHECK_CKR(ret, "Create untrusted AES wrapping key"); } if (ret == CKR_OK) { ret = funcList->C_CreateObject(session, wwtTmpl, wwtTmplCnt, &key); From db2d785bcdecf2ccf1b3424612a08bf4964ad045 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Wed, 15 Jul 2026 23:28:59 +0000 Subject: [PATCH 05/14] Serialize storeDir writes against C_Finalize free in NSS builds (F-5868) WP11_SetStoreDir held no lock while doing XFREE(storeDir)/XMALLOC/XMEMCPY. A concurrent C_Finalize -> WP11_Library_Final frees the same global storeDir (after releasing globalLock), so the window between XMALLOC and XMEMCPY could free the fresh block and turn the XMEMCPY of attacker-controlled configdir bytes into a heap use-after-free or NULL-pointer write. Add a permanently-live leaf mutex storeDirLock (statically initialized, gated like storeDir on NSS && !TPM_STORE) and hold it across the whole free/malloc/copy in WP11_SetStoreDir and across the free in WP11_Library_Final. It is always taken as a leaf so it cannot invert any lock ordering; in Library_Final it nests inside libraryInitLock in a fixed order. --- src/internal.c | 47 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/src/internal.c b/src/internal.c index f72f25b6..daae96f4 100644 --- a/src/internal.c +++ b/src/internal.c @@ -641,6 +641,20 @@ static wolfSSL_Mutex libraryInitLock #define WP11_HAVE_LIBRARY_INIT_LOCK #endif +#if !defined(SINGLE_THREADED) && defined(WOLFSSL_MUTEX_INITIALIZER) && \ + defined(WOLFSSL_MUTEX_INITIALIZER_CLAUSE) && \ + !defined(WOLFPKCS11_TPM_STORE) && defined(WOLFPKCS11_NSS) +/* Permanently-live leaf mutex serializing the module-global storeDir, which + * is set at C_Initialize (before globalLock exists), read in + * wolfPKCS11_Store_Name, and freed in WP11_Library_Final after globalLock is + * released. Without it the free races the set/read (Fenrir F-5868, F-5150). + * Static init mirrors libraryInitLock. It is always acquired as a leaf (no + * other lock is taken while it is held), so it cannot invert any ordering. */ +static wolfSSL_Mutex storeDirLock + WOLFSSL_MUTEX_INITIALIZER_CLAUSE(storeDirLock); +#define WP11_HAVE_STORE_DIR_LOCK +#endif + #ifndef SINGLE_THREADED /** @@ -1149,17 +1163,30 @@ static char* storeDir = NULL; int WP11_SetStoreDir(const char *dir, size_t dirSz) { + int ret = 0; +#ifdef WP11_HAVE_STORE_DIR_LOCK + /* Serialize against the free in WP11_Library_Final and any concurrent set + * so the XFREE/XMALLOC/XMEMCPY sequence is not raced (F-5868). */ + if (wc_LockMutex(&storeDirLock) != 0) + return BAD_MUTEX_E; +#endif if (storeDir != NULL) XFREE(storeDir, NULL, DYNAMIC_TYPE_TMP_BUFFER); storeDir = NULL; if (dir != NULL) { storeDir = (char*) XMALLOC(dirSz + 1, NULL, DYNAMIC_TYPE_TMP_BUFFER); - if (storeDir == NULL) - return MEMORY_E; - XMEMCPY(storeDir, dir, dirSz); - storeDir[dirSz] = '\0'; /* Ensure null termination */ + if (storeDir == NULL) { + ret = MEMORY_E; + } + else { + XMEMCPY(storeDir, dir, dirSz); + storeDir[dirSz] = '\0'; /* Ensure null termination */ + } } - return 0; +#ifdef WP11_HAVE_STORE_DIR_LOCK + wc_UnLockMutex(&storeDirLock); +#endif + return ret; } #endif @@ -6784,8 +6811,18 @@ void WP11_Library_Final(void) (void)ret; /* store failure cannot be returned, so log and ignore */ } #if !defined (WOLFPKCS11_CUSTOM_STORE) && defined(WOLFPKCS11_NSS) + /* Serialize the free against a concurrent WP11_SetStoreDir / + * wolfPKCS11_Store_Name (F-5868, F-5150). Nested inside libraryInitLock + * (held here); storeDirLock is a leaf so the fixed order + * libraryInitLock -> storeDirLock cannot deadlock. */ +#ifdef WP11_HAVE_STORE_DIR_LOCK + wc_LockMutex(&storeDirLock); +#endif XFREE(storeDir, NULL, DYNAMIC_TYPE_TMP_BUFFER); storeDir = NULL; +#ifdef WP11_HAVE_STORE_DIR_LOCK + wc_UnLockMutex(&storeDirLock); +#endif #endif #endif /* Cleanup the slots. */ From 2b2415e383869c52b7b67893032720763a688057 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Wed, 15 Jul 2026 23:30:51 +0000 Subject: [PATCH 06/14] Serialize storeDir reads against C_Finalize free in NSS builds (F-5150) wolfPKCS11_Store_Name read the module-global storeDir with no lock and then dereferenced it (XSTRLEN/XSNPRINTF) to build the token path. A concurrent C_Finalize -> WP11_Library_Final frees storeDir after releasing globalLock, so the read path could use freed memory. Under the same storeDirLock introduced for F-5868, copy storeDir into a local buffer and release the lock before formatting the path, so the free cannot race the use. The lock is a leaf (nothing else is acquired while held), so the token->lock -> storeDirLock order on this path introduces no deadlock. --- src/internal.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/internal.c b/src/internal.c index daae96f4..774107f0 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1379,6 +1379,9 @@ static int wolfPKCS11_Store_Name(int type, CK_ULONG id1, CK_ULONG id2, char* nam */ enum { WP11_STORE_SUFFIX_RESERVE = 48 }; char homePath[256]; +#ifdef WP11_HAVE_STORE_DIR_LOCK + char storeDirCopy[WP11_STORE_MAX_PATH]; +#endif /* Path order: * 1. Environment variable WOLFPKCS11_TOKEN_PATH @@ -1392,9 +1395,28 @@ static int wolfPKCS11_Store_Name(int type, CK_ULONG id1, CK_ULONG id2, char* nam #endif #ifdef WOLFPKCS11_NSS - if (str == NULL) + if (str == NULL) { +#ifdef WP11_HAVE_STORE_DIR_LOCK + /* Copy storeDir into a local under storeDirLock so a concurrent + * WP11_Library_Final free cannot leave str dangling while we format + * the path below (F-5150). The lock is released before use. */ + if (wc_LockMutex(&storeDirLock) != 0) + return -1; + if (storeDir != NULL) { + size_t sdLen = XSTRLEN(storeDir); + if (sdLen >= sizeof(storeDirCopy)) { + wc_UnLockMutex(&storeDirLock); + return -1; + } + XMEMCPY(storeDirCopy, storeDir, sdLen + 1); + str = storeDirCopy; + } + wc_UnLockMutex(&storeDirLock); +#else str = storeDir; #endif + } +#endif if (str == NULL) { const char* homeDir = NULL; From 5daf41ba600b7748a404ceca1961d88919708fc6 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Wed, 15 Jul 2026 23:34:37 +0000 Subject: [PATCH 07/14] Serialize AES-CBC multi-part partialSz updates on a session (F-5764) WP11_AesCbc_EncryptUpdate/DecryptUpdate (and WP11_AesCbcPad_DecryptUpdate) performed a non-atomic read-modify-write on cbc->partialSz with no lock held: WP11_Session_Get releases slot->lock before returning the session pointer and WP11_Session has no lock of its own. Two threads sharing one session handle could both read partialSz=N, both XMEMCPY into cbc->partial+N and both add, leaving partialSz = N + 2*(AES_BLOCK_SIZE-N) > 16. The next update then makes sz = AES_BLOCK_SIZE - partialSz negative, which widens to a huge size_t in XMEMCPY and overflows the 16-byte partial buffer into adjacent session memory. Hold session->slot->lock (RW) across the whole body of each *Update, matching the existing lock idiom. WP11_AesCbcPad_EncryptUpdate is left untouched: it forwards to WP11_AesCbc_EncryptUpdate, which now locks, so locking it too would self-deadlock (the RW lock is non-recursive). No caller holds slot->lock at dispatch, so no ordering or deadlock is introduced; in SINGLE_THREADED the lock macros compile to no-ops. --- src/internal.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/internal.c b/src/internal.c index 774107f0..490b0e2c 100644 --- a/src/internal.c +++ b/src/internal.c @@ -14503,6 +14503,13 @@ int WP11_AesCbc_EncryptUpdate(unsigned char* plain, word32 plainSz, int sz = 0; int outSz = 0; + /* Serialize the read-modify-write of cbc->partial/partialSz. Without this + * two threads sharing one session handle can both read partialSz, both + * copy into cbc->partial and both add, driving partialSz past + * AES_BLOCK_SIZE so the next call computes a negative sz and overflows the + * 16-byte partial buffer (F-5764). No caller holds slot->lock here. */ + WP11_Lock_LockRW(&session->slot->lock); + if (cbc->partialSz > 0) { sz = AES_BLOCK_SIZE - cbc->partialSz; if (sz > (int)plainSz) @@ -14536,6 +14543,7 @@ int WP11_AesCbc_EncryptUpdate(unsigned char* plain, word32 plainSz, if (ret == 0) *encSz = outSz; + WP11_Lock_UnlockRW(&session->slot->lock); return ret; } @@ -14610,6 +14618,10 @@ int WP11_AesCbc_DecryptUpdate(unsigned char* enc, word32 encSz, int sz = 0; int outSz = 0; + /* Serialize the partial-block read-modify-write against a concurrent + * update on the same session (F-5764); see WP11_AesCbc_EncryptUpdate. */ + WP11_Lock_LockRW(&session->slot->lock); + if (cbc->partialSz > 0) { sz = AES_BLOCK_SIZE - cbc->partialSz; if (sz > (int)encSz) @@ -14642,6 +14654,7 @@ int WP11_AesCbc_DecryptUpdate(unsigned char* enc, word32 encSz, if (ret == 0) *decSz = outSz; + WP11_Lock_UnlockRW(&session->slot->lock); return ret; } @@ -14814,6 +14827,10 @@ int WP11_AesCbcPad_DecryptUpdate(unsigned char* enc, word32 encSz, int sz = 0; int outSz = 0; + /* Serialize the partial-block read-modify-write against a concurrent + * update on the same session (F-5764); see WP11_AesCbc_EncryptUpdate. */ + WP11_Lock_LockRW(&session->slot->lock); + if (cbc->partialSz > 0) { sz = AES_BLOCK_SIZE - cbc->partialSz; if (sz > (int)encSz) @@ -14827,6 +14844,7 @@ int WP11_AesCbcPad_DecryptUpdate(unsigned char* enc, word32 encSz, * far and leave the operation active (CKR_BUFFER_TOO_SMALL). */ if ((word32)(outSz + AES_BLOCK_SIZE) > bufSz) { *decSz = (word32)outSz + AES_BLOCK_SIZE; + WP11_Lock_UnlockRW(&session->slot->lock); return BUFFER_E; } ret = wc_AesCbcDecrypt(&cbc->aes, dec, cbc->partial, @@ -14842,6 +14860,7 @@ int WP11_AesCbcPad_DecryptUpdate(unsigned char* enc, word32 encSz, sz -= AES_BLOCK_SIZE; if ((word32)(outSz + sz) > bufSz) { *decSz = (word32)(outSz + sz); + WP11_Lock_UnlockRW(&session->slot->lock); return BUFFER_E; } ret = wc_AesCbcDecrypt(&cbc->aes, dec, enc, sz); @@ -14856,6 +14875,7 @@ int WP11_AesCbcPad_DecryptUpdate(unsigned char* enc, word32 encSz, if (ret == 0) *decSz = outSz; + WP11_Lock_UnlockRW(&session->slot->lock); return ret; } From 11e79b17e77bd5faa72f6fe2cec02260e83ae2d9 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Wed, 15 Jul 2026 23:50:38 +0000 Subject: [PATCH 08/14] Warn when PIN KDF falls back to unsalted SHA-256 (F-6232) The default (non-FIPS, non-PBKDF2, non-scrypt) build derives the SO/user PIN hashes and the AES-256 token storage key with a single unsalted SHA-256 pass that discards the persisted per-token seed. This offers no salt and no key stretching, letting an attacker who obtains the token-store file brute-force a weak PIN offline and recover the storage key. The KDF is selected at compile time and was otherwise silent, so integrators were unaware the strong KDF was not compiled in. Emit a portable build-time warning (#pragma message on MSVC, #warning on GCC/Clang) on this path, silenceable with WOLFPKCS11_ALLOW_WEAK_PIN_KDF, and document the weakness in-code. Actually strengthening the derivation in place (salting/stretching, or enabling PBKDF2 by default) is deliberately NOT done here: it changes the persisted PIN-hash and storage-key bytes and would render every existing default-build token store unrecoverable with no migration path. That compatibility break is a maintainer/product decision. --- src/internal.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/internal.c b/src/internal.c index 490b0e2c..89a22b81 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7183,6 +7183,21 @@ static int HashPIN(char* pin, int pinLen, byte* seed, int seedLen, byte* hash, WP11_HASH_PIN_COST, WP11_HASH_PIN_BLOCKSIZE, WP11_HASH_PIN_PARALLEL, hashLen); #elif !defined(NO_SHA256) + /* Fallback: unsalted single-pass SHA-256 of the PIN. This provides no + * salt (the per-token seed is discarded) and no key stretching, so an + * attacker with the token-store file can brute-force a weak PIN offline + * and recover the token storage key. Configure WOLFPKCS11_PBKDF2 or + * HAVE_SCRYPT for a salted, stretched KDF. The selection is compile-time + * and otherwise silent, so warn integrators unless they opt out (F-6232). + * Note: changing this derivation would invalidate existing token stores, + * so hardening it in place is left as a deliberate maintainer decision. */ +#ifndef WOLFPKCS11_ALLOW_WEAK_PIN_KDF + #if defined(_MSC_VER) + #pragma message("wolfPKCS11: no PBKDF2/scrypt - PIN hashing and token key derivation use unsalted SHA-256; define WOLFPKCS11_PBKDF2 or HAVE_SCRYPT for a strong KDF, or WOLFPKCS11_ALLOW_WEAK_PIN_KDF to silence") + #elif defined(__GNUC__) || defined(__clang__) + #warning "wolfPKCS11: no PBKDF2/scrypt - PIN hashing and token key derivation use unsalted SHA-256; define WOLFPKCS11_PBKDF2 or HAVE_SCRYPT for a strong KDF, or WOLFPKCS11_ALLOW_WEAK_PIN_KDF to silence" + #endif +#endif /* fallback to simple SHA2-256 hash of pin */ (void)seed; (void)seedLen; From e4b400b0d74139a5f9db02792df630df4655af21 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Wed, 15 Jul 2026 23:52:29 +0000 Subject: [PATCH 09/14] Require explicit opt-in for the MAXQ10xx test provisioning key (F-5074) When WOLFSSL_MAXQ10XX_CRYPTO was set without MAXQ10XX_PRODUCTION_KEY, a 96-byte hardcoded P-256 key pair was compiled in and used by ECDSA_sign to authenticate every root certificate provisioned into the MAXQ10xx secure element. The private scalar is public in the source, so anyone could forge a valid provisioning signature for an arbitrary malicious root certificate, reachable via C_CreateObject(CKO_CERTIFICATE, CKA_TOKEN=TRUE). Only a source comment guarded this; a production build missing MAXQ10XX_PRODUCTION_KEY silently shipped the test key. Make the insecure test key opt-in: production builds must define MAXQ10XX_PRODUCTION_KEY, and using the built-in test key now requires explicitly defining WOLFPKCS11_MAXQ10XX_TEST_KEY. With neither, the build fails with an #error instead of silently embedding a public private key. Only WOLFSSL_MAXQ10XX_CRYPTO builds are affected; such dev builds must now pass -DWOLFPKCS11_MAXQ10XX_TEST_KEY. --- src/internal.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/internal.c b/src/internal.c index 89a22b81..43d87613 100644 --- a/src/internal.c +++ b/src/internal.c @@ -3339,8 +3339,11 @@ static int wp11_Object_Load_Data(WP11_Object* object, int tokenId, int objId) #ifdef WOLFSSL_MAXQ10XX_CRYPTO #ifdef MAXQ10XX_PRODUCTION_KEY #include "maxq10xx_key.h" -#else -/* TEST KEY. This must be changed for production environments!! */ +#elif defined(WOLFPKCS11_MAXQ10XX_TEST_KEY) +/* INSECURE TEST KEY. The private scalar (last 32 bytes) is public in the + * source, so anyone can forge a valid MXQ_ImportRootCert provisioning + * signature for an arbitrary root certificate. For development against the + * MAXQ10xx evaluation kit only - never ship this. */ static mxq_u1 KeyPairImport[] = { 0xd0,0x97,0x31,0xc7,0x63,0xc0,0x9e,0xe3,0x9a,0xb4,0xd0,0xce,0xa7,0x89,0xab, 0x52,0xc8,0x80,0x3a,0x91,0x77,0x29,0xc3,0xa0,0x79,0x2e,0xe6,0x61,0x8b,0x2d, @@ -3350,6 +3353,8 @@ static mxq_u1 KeyPairImport[] = { 0x72,0x5e,0x88,0xaf,0xc2,0xee,0x8b,0x6f,0xe5,0x36,0xe3,0x60,0x7c,0xf8,0x2c, 0xea,0x3a,0x4f,0xe3,0x6d,0x73 }; +#else +#error "MAXQ10xx root-cert provisioning key is undefined. Define MAXQ10XX_PRODUCTION_KEY (with a real maxq10xx_key.h) for production, or WOLFPKCS11_MAXQ10XX_TEST_KEY to explicitly opt into the built-in INSECURE test key for evaluation-kit development." #endif /* MAXQ10XX_PRODUCTION_KEY */ static int crypto_sha256(const byte *buf, word32 len, byte *hash, From 4565576f2d18a892f2fedcc416f82b75e562f015 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 16 Jul 2026 08:41:49 +0000 Subject: [PATCH 10/14] Set *pulCount on CKR_BUFFER_TOO_SMALL in C_GetMechanismList (F-3398) When pMechanismList was too small, C_GetMechanismList returned CKR_BUFFER_TOO_SMALL without writing the required count to *pulCount. PKCS#11 mandates that *pulCount be set on this error path so the standard two-call (size-query then fetch) idiom works; callers could otherwise loop or fail. Set *pulCount = mechanismCnt before returning, mirroring the success path. --- src/slot.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/slot.c b/src/slot.c index 4764eef4..7bf76125 100644 --- a/src/slot.c +++ b/src/slot.c @@ -539,6 +539,9 @@ CK_RV C_GetMechanismList(CK_SLOT_ID slotID, return rv; } else if (*pulCount < (CK_ULONG)mechanismCnt) { + /* PKCS#11: on CKR_BUFFER_TOO_SMALL *pulCount must be set to the + * required count so the two-call (size-query then fetch) idiom works. */ + *pulCount = mechanismCnt; rv = CKR_BUFFER_TOO_SMALL; WOLFPKCS11_LEAVE("C_GetMechanismList", rv); return rv; From 78423d9d868affb3b64418762f6a41b21d55f472 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 16 Jul 2026 08:42:43 +0000 Subject: [PATCH 11/14] Reject C_InitToken with an open session regardless of init state (F-3141) The CKR_SESSION_EXISTS check was gated behind WP11_Slot_IsTokenInitialized, so a never-initialized token with open sessions skipped it and proceeded to WP11_Slot_TokenReset. PKCS#11 requires an open session on the token to fail C_InitToken with CKR_SESSION_EXISTS regardless of initialization state. Hoist the WP11_Slot_HasSession check out of the IsTokenInitialized branch so it runs unconditionally before any token-reset logic. --- src/slot.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/slot.c b/src/slot.c index 7bf76125..73276432 100644 --- a/src/slot.c +++ b/src/slot.c @@ -1286,12 +1286,16 @@ CK_RV C_InitToken(CK_SLOT_ID slotID, CK_UTF8CHAR_PTR pPin, return rv; } + /* PKCS#11: an open session on the token must fail C_InitToken with + * CKR_SESSION_EXISTS regardless of whether the token is already + * initialized. Check this unconditionally before any token-reset logic. */ + if (WP11_Slot_HasSession(slot)) { + rv = CKR_SESSION_EXISTS; + WOLFPKCS11_LEAVE("C_InitToken", rv); + return rv; + } + if (WP11_Slot_IsTokenInitialized(slot)) { - if (WP11_Slot_HasSession(slot)) { - rv = CKR_SESSION_EXISTS; - WOLFPKCS11_LEAVE("C_InitToken", rv); - return rv; - } if (WP11_Slot_SOPin_IsSet(slot)) { /* Verify the SO PIN with the failed-login lockout applied, so this * path cannot be used to brute-force the SO PIN (Fenrir F-4632). From 929b41a4150a7cf987c440c584f8eca8f4aaa7b2 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 16 Jul 2026 08:54:19 +0000 Subject: [PATCH 12/14] Advertise AES key wrap under the same macro that implements it (F-3401) slot.c gated CKM_AES_KEY_WRAP/CKM_AES_KEY_WRAP_PAD advertisement (mechanism list and mechanismInfo) on HAVE_AES_KEY_WRAP, while every implementation path in crypto.c/internal.c gates on the wolfSSL-standard HAVE_AES_KEYWRAP. The two macros are independent, so a normal build (HAVE_AES_KEYWRAP from wolfSSL) implemented the mechanisms but never advertised them, while --enable-aeskeywrap (HAVE_AES_KEY_WRAP only) advertised them but EncryptInit/DecryptInit rejected them with CKR_MECHANISM_INVALID. Gate slot.c on HAVE_AES_KEYWRAP so advertisement matches the implementation in every build. slot.c receives HAVE_AES_KEYWRAP transitively from wolfSSL, the same source the implementation uses. The build-system macro is intentionally not renamed: --enable-aeskeywrap's -DHAVE_AES_KEY_WRAP path also feeds DISABLE_DEFS, and renaming it would regress the default/CI config. --- src/slot.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/slot.c b/src/slot.c index 73276432..6d1d792e 100644 --- a/src/slot.c +++ b/src/slot.c @@ -384,7 +384,7 @@ static CK_MECHANISM_TYPE mechanismList[] = { #endif #ifndef NO_AES CKM_AES_KEY_GEN, -#ifdef HAVE_AES_KEY_WRAP +#ifdef HAVE_AES_KEYWRAP CKM_AES_KEY_WRAP, CKM_AES_KEY_WRAP_PAD, #endif @@ -719,7 +719,7 @@ static CK_MECHANISM_INFO tlsMacMechInfo = { static CK_MECHANISM_INFO aesKeyGenMechInfo = { 16, 32, CKF_GENERATE }; -#ifdef HAVE_AES_KEY_WRAP +#ifdef HAVE_AES_KEYWRAP static CK_MECHANISM_INFO aesKeyWrapMechInfo = { 16, 32, CKF_ENCRYPT | CKF_DECRYPT | CKF_WRAP | CKF_UNWRAP }; @@ -1032,7 +1032,7 @@ CK_RV C_GetMechanismInfo(CK_SLOT_ID slotID, CK_MECHANISM_TYPE type, case CKM_AES_KEY_GEN: XMEMCPY(pInfo, &aesKeyGenMechInfo, sizeof(CK_MECHANISM_INFO)); break; -#ifdef HAVE_AES_KEY_WRAP +#ifdef HAVE_AES_KEYWRAP case CKM_AES_KEY_WRAP: case CKM_AES_KEY_WRAP_PAD: XMEMCPY(pInfo, &aesKeyWrapMechInfo, sizeof(CK_MECHANISM_INFO)); From 378dda1a5d42af6d67a495726502069c99892f57 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 16 Jul 2026 08:55:13 +0000 Subject: [PATCH 13/14] Document that NSS builds default to empty-PIN (no auth) (F-2371) On --enable-nss builds WP11_MIN_PIN_LEN defaults to 0, permitting a zero-length user PIN which intentionally disables PIN-based authentication (C_GetTokenInfo clears CKF_LOGIN_REQUIRED and token objects are decoded at load without C_Login). This is required for NSS tools (certutil/PK11_InitPin) that bootstrap empty-password databases. The default is deliberately left unchanged: flipping it to non-zero would break that NSS bootstrap and the empty-PIN CI/tests. Document the behavior and the -DWP11_MIN_PIN_LEN=N opt-in in the header and README so integrators are aware the auth model is disabled by default on NSS. --- README.md | 8 ++++++++ wolfpkcs11/internal.h | 7 ++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index c7908a49..db32fb56 100644 --- a/README.md +++ b/README.md @@ -144,6 +144,14 @@ versions may need to update templates or error-handling: (`--enable-nss`) keep the empty-PIN probe accepted on uninitialized tokens because `PK11_InitPin` bootstraps an empty-password NSS database that way; non-empty PINs are still rejected. +- On `--enable-nss` builds `WP11_MIN_PIN_LEN` defaults to `0`, permitting a + zero-length user PIN. An empty user PIN intentionally disables PIN-based + authentication: `C_GetTokenInfo` clears `CKF_LOGIN_REQUIRED` and all + private/token objects are decoded and accessible at token load without + `C_Login`. This is required for NSS tools (`certutil`, `PK11_InitPin`) that + bootstrap empty-password databases. Integrators who require an enforced + minimum can opt in at build time with `C_EXTRA_FLAGS="-DWP11_MIN_PIN_LEN=N"` + (`N>0`); non-NSS builds already default to `4`. #### Analog Devices, Inc. MAXQ10xx Secure Elements ([MAXQ1065](https://www.analog.com/en/products/maxq1065.html)/MAXQ1080) diff --git a/wolfpkcs11/internal.h b/wolfpkcs11/internal.h index 0e72be16..29f31184 100644 --- a/wolfpkcs11/internal.h +++ b/wolfpkcs11/internal.h @@ -311,7 +311,12 @@ C_EXTRA_FLAGS="-DWOLFSSL_PUBLIC_MP -DWC_RSA_DIRECT" #define WP11_HASH_PIN_PARALLEL 1 #endif -/* PIN length constraints. */ +/* PIN length constraints. The NSS default is 0: an empty user PIN is allowed + * and intentionally disables login (C_GetTokenInfo clears CKF_LOGIN_REQUIRED + * and token objects are decoded at load without C_Login) for NSS tool + * compatibility (certutil / PK11_InitPin bootstrap empty-password databases). + * Integrators who require an enforced minimum can override with + * -DWP11_MIN_PIN_LEN=N (N>0). */ #ifndef WP11_MIN_PIN_LEN #ifdef WOLFPKCS11_NSS #define WP11_MIN_PIN_LEN 0 From fab1cb182438fc21605738e234373b1abd8b863a Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 16 Jul 2026 13:49:27 +0000 Subject: [PATCH 14/14] Only unlock storeDirLock in WP11_Library_Final if lock succeeded (F-5868) The free path acquired storeDirLock without checking wc_LockMutex's return and always called wc_UnLockMutex, which is an invalid unlock (and may fail/crash on some mutex implementations) if the lock was not actually acquired. Track the lock result and only unlock when it succeeded. (WP11_SetStoreDir already returns BAD_MUTEX_E on failure; WP11_Library_Final is void so it proceeds with the free either way, but no longer unlocks a lock it does not hold.) --- src/internal.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/internal.c b/src/internal.c index 43d87613..d66074c5 100644 --- a/src/internal.c +++ b/src/internal.c @@ -6842,14 +6842,18 @@ void WP11_Library_Final(void) * wolfPKCS11_Store_Name (F-5868, F-5150). Nested inside libraryInitLock * (held here); storeDirLock is a leaf so the fixed order * libraryInitLock -> storeDirLock cannot deadlock. */ + { #ifdef WP11_HAVE_STORE_DIR_LOCK - wc_LockMutex(&storeDirLock); + int storeDirLocked = (wc_LockMutex(&storeDirLock) == 0); #endif - XFREE(storeDir, NULL, DYNAMIC_TYPE_TMP_BUFFER); - storeDir = NULL; + XFREE(storeDir, NULL, DYNAMIC_TYPE_TMP_BUFFER); + storeDir = NULL; #ifdef WP11_HAVE_STORE_DIR_LOCK - wc_UnLockMutex(&storeDirLock); + /* Only unlock if the lock was actually acquired. */ + if (storeDirLocked) + wc_UnLockMutex(&storeDirLock); #endif + } #endif #endif /* Cleanup the slots. */