Skip to content

Fix Fenrir PKCS#11 compliance findings #186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dgarske merged 1 commit into from
May 12, 2026
Merged

Fix Fenrir PKCS#11 compliance findings #186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,8 @@ if(WOLFPKCS11_TESTS)
token_path_test
rsa_session_persistence_test
debug_test
object_id_uniqueness_test)
object_id_uniqueness_test
pkcs11_compliance_test)

foreach(test_target IN LISTS WPKCS11_TEST_TARGETS)
add_wpkcs11_test(${test_target}
Expand Down
70 changes: 66 additions & 4 deletions src/crypto.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6644,7 +6644,7 @@ CK_RV C_DigestEncryptUpdate(CK_SESSION_HANDLE hSession,

(void)pEncryptedPart;

rv = CKR_OPERATION_NOT_INITIALIZED;
rv = CKR_FUNCTION_NOT_SUPPORTED;
WOLFPKCS11_LEAVE("C_DigestEncryptUpdate", rv);
return rv;
}
Expand Down Expand Up @@ -6700,7 +6700,7 @@ CK_RV C_DecryptDigestUpdate(CK_SESSION_HANDLE hSession,

(void)pPart;

rv = CKR_OPERATION_NOT_INITIALIZED;
rv = CKR_FUNCTION_NOT_SUPPORTED;
WOLFPKCS11_LEAVE("C_DecryptDigestUpdate", rv);
return rv;
}
Expand Down Expand Up @@ -6756,7 +6756,7 @@ CK_RV C_SignEncryptUpdate(CK_SESSION_HANDLE hSession,

(void)pEncryptedPart;

rv = CKR_OPERATION_NOT_INITIALIZED;
rv = CKR_FUNCTION_NOT_SUPPORTED;
WOLFPKCS11_LEAVE("C_SignEncryptUpdate", rv);
return rv;
}
Expand Down Expand Up @@ -6812,7 +6812,7 @@ CK_RV C_DecryptVerifyUpdate(CK_SESSION_HANDLE hSession,

(void)pPart;

rv = CKR_OPERATION_NOT_INITIALIZED;
rv = CKR_FUNCTION_NOT_SUPPORTED;
WOLFPKCS11_LEAVE("C_DecryptVerifyUpdate", rv);
return rv;
}
Expand Down Expand Up @@ -6870,6 +6870,24 @@ CK_RV C_GenerateKey(CK_SESSION_HANDLE hSession,
WOLFPKCS11_LEAVE("C_GenerateKey", rv);
return rv;
}
/* Only require R/W session for token objects */
if (!WP11_Session_IsRW(session)) {
CK_ATTRIBUTE* tokenAttr = NULL;
FindAttributeType(pTemplate, ulCount, CKA_TOKEN, &tokenAttr);
if (tokenAttr != NULL) {
if (tokenAttr->pValue == NULL ||
tokenAttr->ulValueLen != sizeof(CK_BBOOL)) {
rv = CKR_ATTRIBUTE_VALUE_INVALID;
WOLFPKCS11_LEAVE("C_GenerateKey", rv);
return rv;
}
if (*(CK_BBOOL*)tokenAttr->pValue == CK_TRUE) {
rv = CKR_SESSION_READ_ONLY;
WOLFPKCS11_LEAVE("C_GenerateKey", rv);
return rv;
}
}
}

switch (pMechanism->mechanism) {
#ifndef NO_AES
Expand Down Expand Up @@ -7287,6 +7305,32 @@ CK_RV C_GenerateKeyPair(CK_SESSION_HANDLE hSession,
WOLFPKCS11_LEAVE("C_GenerateKeyPair", rv);
return rv;
}
/* Only require R/W session for token objects. Each template must be
* inspected independently — a public template with CKA_TOKEN=FALSE must
* not mask a private template requesting CKA_TOKEN=TRUE. */
if (!WP11_Session_IsRW(session)) {
CK_ATTRIBUTE_PTR tpls[2] = { pPublicKeyTemplate, pPrivateKeyTemplate };
CK_ULONG counts[2] = { ulPublicKeyAttributeCount,
ulPrivateKeyAttributeCount };
int i;
for (i = 0; i < 2; i++) {
CK_ATTRIBUTE* tokenAttr = NULL;
FindAttributeType(tpls[i], counts[i], CKA_TOKEN, &tokenAttr);
if (tokenAttr == NULL)
continue;
if (tokenAttr->pValue == NULL ||
tokenAttr->ulValueLen != sizeof(CK_BBOOL)) {
rv = CKR_ATTRIBUTE_VALUE_INVALID;
WOLFPKCS11_LEAVE("C_GenerateKeyPair", rv);
return rv;
}
if (*(CK_BBOOL*)tokenAttr->pValue == CK_TRUE) {
rv = CKR_SESSION_READ_ONLY;
WOLFPKCS11_LEAVE("C_GenerateKeyPair", rv);
return rv;
}
}
}

switch (pMechanism->mechanism) {
#if !defined(NO_RSA) && defined(WOLFSSL_KEY_GEN)
Expand Down Expand Up @@ -8308,6 +8352,24 @@ CK_RV C_DeriveKey(CK_SESSION_HANDLE hSession,
WOLFPKCS11_LEAVE("C_DeriveKey", rv);
return rv;
}
/* Only require R/W session for token objects */
if (!WP11_Session_IsRW(session)) {
CK_ATTRIBUTE* tokenAttr = NULL;
FindAttributeType(pTemplate, ulAttributeCount, CKA_TOKEN, &tokenAttr);
if (tokenAttr != NULL) {
if (tokenAttr->pValue == NULL ||
tokenAttr->ulValueLen != sizeof(CK_BBOOL)) {
rv = CKR_ATTRIBUTE_VALUE_INVALID;
WOLFPKCS11_LEAVE("C_DeriveKey", rv);
return rv;
}
if (*(CK_BBOOL*)tokenAttr->pValue == CK_TRUE) {
rv = CKR_SESSION_READ_ONLY;
WOLFPKCS11_LEAVE("C_DeriveKey", rv);
return rv;
}
}
}

ret = WP11_Object_Find(session, hBaseKey, &obj);
if (ret != 0)
Expand Down
27 changes: 22 additions & 5 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6728,8 +6728,13 @@ int WP11_GetSlotList(int tokenIn, CK_SLOT_ID* slotIdList, CK_ULONG* count)

if (slotIdList == NULL)
*count = slotCnt;
else if ((int)*count < slotCnt)
else if ((int)*count < slotCnt) {
/* PKCS#11 spec: report the required size so the caller can resize
* and retry. Without this, callers using the standard two-call size
* query pattern stall at BUFFER_TOO_SMALL. */
*count = slotCnt;
ret = BUFFER_E;
}
else {
for (i = 0; i < slotCnt && i < (int)*count; i++)
slotIdList[i] = i + 1;
Expand Down Expand Up @@ -8330,15 +8335,21 @@ int WP11_Session_SetGcmParams(WP11_Session* session, unsigned char* iv,
int ret = 0;
WP11_GcmParams* gcm = &session->params.gcm;

if (tagBits > 128 || ivSz > WP11_MAX_GCM_NONCE_SZ)
if (tagBits > 128 || ivSz < 0 || ivSz > WP11_MAX_GCM_NONCE_SZ)
ret = BAD_FUNC_ARG;
/* Caller-supplied IV pointer and length must agree: NULL pairs with 0
* and only with 0, and a non-NULL buffer must come with a positive
* length. Either mismatch is a contract bug. */
if (ret == 0 && (iv == NULL) != (ivSz == 0))
ret = BAD_FUNC_ARG;

if (ret == 0) {
if (session->mechanism == CKM_AES_GCM && gcm->aad != NULL) {
XFREE(gcm->aad, NULL, DYNAMIC_TYPE_TMP_BUFFER);
}
XMEMSET(gcm, 0, sizeof(*gcm));
XMEMCPY(gcm->iv, iv, ivSz);
if (ivSz > 0)
XMEMCPY(gcm->iv, iv, ivSz);
gcm->ivSz = ivSz;
gcm->tagBits = tagBits;
if (aad != NULL) {
Expand Down Expand Up @@ -8380,7 +8391,12 @@ int WP11_Session_SetCcmParams(WP11_Session* session, int dataSz,
int ret = 0;
WP11_CcmParams* ccm = &session->params.ccm;

if (ivSz > WP11_MAX_GCM_NONCE_SZ)
if (ivSz < 0 || ivSz > WP11_MAX_GCM_NONCE_SZ)
ret = BAD_FUNC_ARG;
/* Caller-supplied IV pointer and length must agree: NULL pairs with 0
* and only with 0, and a non-NULL buffer must come with a positive
* length. Either mismatch is a contract bug. */
if (ret == 0 && (iv == NULL) != (ivSz == 0))
ret = BAD_FUNC_ARG;

if (ret == 0) {
Expand All @@ -8389,7 +8405,8 @@ int WP11_Session_SetCcmParams(WP11_Session* session, int dataSz,
}
XMEMSET(ccm, 0, sizeof(*ccm));
ccm->dataSz = dataSz;
XMEMCPY(ccm->iv, iv, ivSz);
if (ivSz > 0)
XMEMCPY(ccm->iv, iv, ivSz);
ccm->ivSz = ivSz;
if (aad != NULL) {
ccm->aad = (unsigned char*)XMALLOC(aadSz, NULL,
Expand Down
21 changes: 16 additions & 5 deletions src/slot.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2062,12 +2062,23 @@ CK_RV C_WaitForSlotEvent(CK_FLAGS flags, CK_SLOT_ID_PTR pSlot,
WOLFPKCS11_LEAVE("C_WaitForSlotEvent", rv);
return rv;
}
/* PKCS#11: pSlot is an output parameter, pReserved must be NULL, and
* only the CKF_DONT_BLOCK bit is defined for flags. Reject malformed
* callers before returning the supported non-blocking result. */
if (pSlot == NULL || pReserved != NULL ||
(flags & ~CKF_DONT_BLOCK) != 0) {
rv = CKR_ARGUMENTS_BAD;
WOLFPKCS11_LEAVE("C_WaitForSlotEvent", rv);
return rv;
}

(void)pSlot;
(void)flags;
(void)pReserved;

rv = CKR_FUNCTION_NOT_SUPPORTED;
/* wolfPKCS11 has no removable slots, so no slot event ever occurs. For a
* non-blocking query the spec answer is CKR_NO_EVENT; blocking calls
* would deadlock forever, so report unsupported. */
if ((flags & CKF_DONT_BLOCK) != 0)
rv = CKR_NO_EVENT;
else
rv = CKR_FUNCTION_NOT_SUPPORTED;
WOLFPKCS11_LEAVE("C_WaitForSlotEvent", rv);
return rv;
}
Expand Down
36 changes: 35 additions & 1 deletion src/wolfpkcs11.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,17 @@
#include <wolfpkcs11/pkcs11.h>
#include <wolfpkcs11/internal.h>

/* Windows headers macro-rewrite CreateMutex (and friends) to CreateMutexA/W
* when UNICODE is defined, which collides with the CK_C_INITIALIZE_ARGS
* struct member names. Undef the four Win32 macros so direct field access
* compiles. */
#ifdef _WIN32
#undef CreateMutex
#undef DestroyMutex
#undef LockMutex
#undef UnlockMutex
#endif

/* Function list table for PKCS#11 v2.40 */
static CK_FUNCTION_LIST wolfpkcs11FunctionList = {
/* Version: Major, Minor */
Expand Down Expand Up @@ -509,6 +520,19 @@ CK_RV C_Initialize(CK_VOID_PTR pInitArgs)
WOLFPKCS11_ENTER("C_Initialize");

if (args != NULL) {
/* PKCS#11 spec: the four mutex callbacks must be all-set or all-NULL;
* any partial combination is CKR_ARGUMENTS_BAD. pReserved must also
* be NULL. */
int callbacks_set =
(args->CreateMutex != NULL) + (args->DestroyMutex != NULL) +
(args->LockMutex != NULL) + (args->UnlockMutex != NULL);
if ((callbacks_set != 0 && callbacks_set != 4) ||
args->pReserved != NULL) {
ret = CKR_ARGUMENTS_BAD;
WOLFPKCS11_LEAVE("C_Initialize", ret);
return ret;
}

Comment thread
LinuxJedi marked this conversation as resolved.
WOLFPKCS11_MSG("Warning: C_Initialize called with arguments, but most "
"are ignored.");
#if (defined(WOLFPKCS11_NSS) && !defined(WOLFPKCS11_NO_STORE))
Expand Down Expand Up @@ -556,9 +580,19 @@ CK_RV C_Finalize(CK_VOID_PTR pReserved)
CK_RV ret;
WOLFPKCS11_ENTER("C_Finalize");

if (!WP11_Library_IsInitialized()) {
ret = CKR_CRYPTOKI_NOT_INITIALIZED;
WOLFPKCS11_LEAVE("C_Finalize", ret);
return ret;
}
if (pReserved != NULL) {
ret = CKR_ARGUMENTS_BAD;
WOLFPKCS11_LEAVE("C_Finalize", ret);
return ret;
}

WP11_Library_Final();

(void)pReserved;
ret = CKR_OK;
WOLFPKCS11_LEAVE("C_Finalize", ret);
return ret;
Expand Down
7 changes: 7 additions & 0 deletions tests/include.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ noinst_PROGRAMS += tests/rsa_exponent_test
tests_rsa_exponent_test_SOURCES = tests/rsa_exponent_test.c
tests_rsa_exponent_test_LDADD =

check_PROGRAMS += tests/pkcs11_compliance_test
noinst_PROGRAMS += tests/pkcs11_compliance_test
tests_pkcs11_compliance_test_SOURCES = tests/pkcs11_compliance_test.c
tests_pkcs11_compliance_test_LDADD =

Comment thread
LinuxJedi marked this conversation as resolved.
if BUILD_STATIC
tests_pkcs11test_LDADD += src/libwolfpkcs11.la
tests_pkcs11mtt_LDADD += src/libwolfpkcs11.la
Expand All @@ -104,6 +109,7 @@ tests_aes_keygen_attrs_test_LDADD += src/libwolfpkcs11.la
tests_pbkdf2_keygen_attrs_test_LDADD += src/libwolfpkcs11.la
tests_pkcs11v3test_LDADD += src/libwolfpkcs11.la
tests_rsa_exponent_test_LDADD += src/libwolfpkcs11.la
tests_pkcs11_compliance_test_LDADD += src/libwolfpkcs11.la
else
tests_object_id_uniqueness_test_LDADD += src/libwolfpkcs11.la
tests_empty_pin_store_test_LDADD += src/libwolfpkcs11.la
Expand All @@ -114,6 +120,7 @@ tests_ecb_check_value_error_test_LDADD += src/libwolfpkcs11.la
tests_operation_active_test_LDADD += src/libwolfpkcs11.la
tests_aes_keygen_attrs_test_LDADD += src/libwolfpkcs11.la
tests_pbkdf2_keygen_attrs_test_LDADD += src/libwolfpkcs11.la
tests_pkcs11_compliance_test_LDADD += src/libwolfpkcs11.la
endif

EXTRA_DIST += tests/unit.h \
Expand Down
Loading
Loading