Skip to content

Static code analysis fixes #178

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

Open
LinuxJedi wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Static code analysis fixes #178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
fd1520f
Fix op check
LinuxJedi Apr 7, 2026
65c4089
Fix HKDF endianess issue
LinuxJedi Apr 7, 2026
665c4c1
Fix return codes
LinuxJedi Apr 7, 2026
148e1d6
Set CKA_LOCAL for symmetric keys
LinuxJedi Apr 7, 2026
33efd26
Add mechanism to symmetric keys
LinuxJedi Apr 7, 2026
dc5bd0d
Fix pin length error handling
LinuxJedi Apr 7, 2026
c8d6a4d
Add mechanism to NSS generated keys
LinuxJedi Apr 7, 2026
99ebafd
Add correct flags for PBKDF2 keys
LinuxJedi Apr 7, 2026
9ff0150
Fix build issues and findings
LinuxJedi Apr 7, 2026
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
56 changes: 38 additions & 18 deletions src/crypto.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3839,7 +3839,10 @@ CK_RV C_DigestInit(CK_SESSION_HANDLE hSession,
WP11_Session_SetOpInitialized(session, init);
}

rv = ret;
if (ret != 0 && ret != (int)CKR_MECHANISM_INVALID)
rv = CKR_FUNCTION_FAILED;
else
rv = ret;
WOLFPKCS11_LEAVE("C_DigestInit", rv);
return rv;
}
Expand Down Expand Up @@ -3892,7 +3895,9 @@ CK_RV C_Digest(CK_SESSION_HANDLE hSession, CK_BYTE_PTR pData,
session);
*pulDigestLen = hashLen;

Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C_Digest() maps any negative return from WP11_Digest_Single() to CKR_FUNCTION_FAILED. WP11_Digest_Single can return BUFFER_E when the output buffer is too small, which should be surfaced as CKR_BUFFER_TOO_SMALL (similar to how C_GetAttributeValue maps BUFFER_E). Consider translating BUFFER_E to CKR_BUFFER_TOO_SMALL instead of collapsing it into CKR_FUNCTION_FAILED.

Suggested change
if (ret == BUFFER_E)
return CKR_BUFFER_TOO_SMALL;

Copilot uses AI. Check for mistakes.
return ret;
if (ret < 0)
return CKR_FUNCTION_FAILED;
return CKR_OK;
}

/**
Expand Down Expand Up @@ -3936,7 +3941,9 @@ CK_RV C_DigestUpdate(CK_SESSION_HANDLE hSession, CK_BYTE_PTR pPart,

ret = WP11_Digest_Update(pPart, (word32)ulPartLen, session);

return ret;
if (ret < 0)
return CKR_FUNCTION_FAILED;
return CKR_OK;
}

/**
Expand Down Expand Up @@ -3979,7 +3986,9 @@ CK_RV C_DigestKey(CK_SESSION_HANDLE hSession, CK_OBJECT_HANDLE hKey)

ret = WP11_Digest_Key(obj, session);

return ret;
if (ret < 0)
return CKR_FUNCTION_FAILED;
return CKR_OK;
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C_DigestKey() now returns CKR_OK for any non-negative value from WP11_Digest_Key(). However, WP11_Digest_Key() can return a non-zero CK_RV like CKR_FUNCTION_NOT_SUPPORTED (e.g., when WOLFPKCS11_NO_STORE is enabled), which should be propagated to the caller instead of being treated as success.

Suggested change
return CKR_OK;
return (CK_RV)ret;

Copilot uses AI. Check for mistakes.
}

/**
Expand Down Expand Up @@ -4027,7 +4036,9 @@ CK_RV C_DigestFinal(CK_SESSION_HANDLE hSession, CK_BYTE_PTR pDigest,
ret = WP11_Digest_Final(pDigest, &hashLen, session);
*pulDigestLen = hashLen;

Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C_DigestFinal() maps BUFFER_E (returned when the caller-provided digest buffer is too small) to CKR_FUNCTION_FAILED. This loses the PKCS#11-required CKR_BUFFER_TOO_SMALL signal; please translate BUFFER_E to CKR_BUFFER_TOO_SMALL (and only use CKR_FUNCTION_FAILED for true internal failures).

Suggested change
if (ret == BUFFER_E)
return CKR_BUFFER_TOO_SMALL;

Copilot uses AI. Check for mistakes.
return ret;
if (ret < 0)
return CKR_FUNCTION_FAILED;
return CKR_OK;
}

#ifdef WOLFSSL_HAVE_PRF
Expand Down Expand Up @@ -6170,7 +6181,7 @@ CK_RV C_VerifyRecoverInit(CK_SESSION_HANDLE hSession,
return CKR_KEY_TYPE_INCONSISTENT;
}

ret = CheckOpSupported(obj, CKA_VERIFY);
ret = CheckOpSupported(obj, CKA_VERIFY_RECOVER);
if (ret != CKR_OK)
return ret;

Expand Down Expand Up @@ -6748,18 +6759,22 @@ CK_RV C_GenerateKey(CK_SESSION_HANDLE hSession,

ret = WP11_Object_SetSecretKey(pbkdf2Key, secretKeyData, secretKeyLen);
if (ret == 0) {
rv = AddObject(session, pbkdf2Key, pTemplate, ulCount, phKey);
if (rv != CKR_OK) {
WP11_Object_Free(pbkdf2Key);
}
WP11_Object_SetKeyGeneration(pbkdf2Key, pMechanism->mechanism);
rv = SetInitialStates(pbkdf2Key);
} else {
WP11_Object_Free(pbkdf2Key);
rv = CKR_FUNCTION_FAILED;
}
if (rv == CKR_OK) {
rv = AddObject(session, pbkdf2Key, pTemplate, ulCount, phKey);
}
if (rv != CKR_OK) {
WP11_Object_Free(pbkdf2Key);
}
}

wc_ForceZero(derivedKey, derivedKeyLen);
XFREE(derivedKey, NULL, DYNAMIC_TYPE_TMP_BUFFER);

return rv;
}
#ifdef WOLFPKCS11_NSS
Expand Down Expand Up @@ -6845,18 +6860,22 @@ CK_RV C_GenerateKey(CK_SESSION_HANDLE hSession,

ret = WP11_Object_SetSecretKey(pbeKey, secretKeyData, secretKeyLen);
if (ret == 0) {
rv = AddObject(session, pbeKey, pTemplate, ulCount, phKey);
if (rv != CKR_OK) {
WP11_Object_Free(pbeKey);
}
WP11_Object_SetKeyGeneration(pbeKey, pMechanism->mechanism);
rv = SetInitialStates(pbeKey);
} else {
WP11_Object_Free(pbeKey);
rv = CKR_FUNCTION_FAILED;
}
if (rv == CKR_OK) {
rv = AddObject(session, pbeKey, pTemplate, ulCount, phKey);
}
if (rv != CKR_OK) {
WP11_Object_Free(pbeKey);
}
}

wc_ForceZero(derivedKey, derivedKeyLen);
XFREE(derivedKey, NULL, DYNAMIC_TYPE_TMP_BUFFER);

return rv;
}
#endif
Expand Down Expand Up @@ -6884,7 +6903,8 @@ CK_RV C_GenerateKey(CK_SESSION_HANDLE hSession,
&key);
if (rv == CKR_OK) {
int ret = WP11_GenerateRandomKey(key,
WP11_Session_GetSlot(session));
WP11_Session_GetSlot(session),
pMechanism->mechanism);
if (ret != 0) {
WP11_Object_Free(key);
rv = CKR_FUNCTION_FAILED;
Expand Down Expand Up @@ -7972,7 +7992,7 @@ CK_RV C_DeriveKey(CK_SESSION_HANDLE hSession,
if (!lenAttr) {
return CKR_MECHANISM_PARAM_INVALID;
}
keyLen = *(word32*)lenAttr->pValue;
keyLen = (word32)*(CK_ULONG*)lenAttr->pValue;
}
else {
keyLen = WC_MAX_DIGEST_SIZE;
Expand Down
26 changes: 23 additions & 3 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10627,6 +10627,19 @@ int WP11_Object_SetAttr(WP11_Object* object, CK_ATTRIBUTE_TYPE type, byte* data,
return ret;
}

/**
* Mark an object as locally generated and record the mechanism used.
*
* @param object [in] Object to update.
* @param mechanism [in] Generation mechanism.
*/
void WP11_Object_SetKeyGeneration(WP11_Object* object,
CK_MECHANISM_TYPE mechanism)
{
object->local = 1;
object->keyGenMech = mechanism;
}

/**
* Check whether the attribute matches in the object.
*
Expand Down Expand Up @@ -12340,12 +12353,14 @@ int WP11_Mldsa_Verify(unsigned char* sig, word32 sigLen, unsigned char* data,
/**
* Generate a secret key.
*
* @param secret [in] Secret object.
* @param slot [in] Slot operation is performed on.
* @param secret [in] Secret object.
* @param slot [in] Slot operation is performed on.
* @param mechanism [in] Key generation mechanism.
* @return -ve on random number generation failure.
* 0 on success.
*/
int WP11_GenerateRandomKey(WP11_Object* secret, WP11_Slot* slot)
int WP11_GenerateRandomKey(WP11_Object* secret, WP11_Slot* slot,
CK_MECHANISM_TYPE mechanism)
{
int ret;
WP11_Data* key = secret->data.symmKey;
Expand All @@ -12354,6 +12369,11 @@ int WP11_GenerateRandomKey(WP11_Object* secret, WP11_Slot* slot)
ret = wc_RNG_GenerateBlock(&slot->token.rng, key->data, key->len);
WP11_Lock_UnlockRW(&slot->token.rngLock);

if (ret == 0) {
secret->local = 1;
secret->keyGenMech = mechanism;
}

return ret;
}
#endif /* WOLFPKCS11_HKDF || !NO_AES */
Expand Down
14 changes: 7 additions & 7 deletions src/slot.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ static CK_RV checkPinLen(CK_ULONG pinLen)
#else
if (pinLen > WP11_MAX_PIN_LEN)
#endif
return CKR_PIN_INCORRECT;
return CKR_PIN_LEN_RANGE;
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkPinLen() now returns CKR_PIN_LEN_RANGE, but some callers (e.g., C_Login and the ulOldLen check in C_SetPIN) still discard the return code and hardcode CKR_PIN_INCORRECT. This makes PIN-length failures inconsistent across APIs. Consider either propagating CKR_PIN_LEN_RANGE consistently everywhere checkPinLen() is used, or changing checkPinLen() to return only CKR_OK/CKR_GENERAL_ERROR and letting each API choose its CKR_* mapping.

Suggested change
return CKR_PIN_LEN_RANGE;
return CKR_PIN_INCORRECT;

Copilot uses AI. Check for mistakes.
return CKR_OK;
}

Expand Down Expand Up @@ -1255,8 +1255,8 @@ CK_RV C_InitToken(CK_SLOT_ID slotID, CK_UTF8CHAR_PTR pPin,
return rv;
}

if (checkPinLen(ulPinLen) != CKR_OK) {
rv = CKR_PIN_INCORRECT;
rv = checkPinLen(ulPinLen);
if (rv != CKR_OK) {
WOLFPKCS11_LEAVE("C_InitToken", rv);
return rv;
}
Expand Down Expand Up @@ -1339,8 +1339,8 @@ CK_RV C_InitPIN(CK_SESSION_HANDLE hSession, CK_UTF8CHAR_PTR pPin,
return rv;
}

if (checkPinLen(ulPinLen) != CKR_OK) {
rv = CKR_PIN_INCORRECT;
rv = checkPinLen(ulPinLen);
if (rv != CKR_OK) {
WOLFPKCS11_LEAVE("C_InitPIN", rv);
return rv;
}
Expand Down Expand Up @@ -1414,8 +1414,8 @@ CK_RV C_SetPIN(CK_SESSION_HANDLE hSession, CK_UTF8CHAR_PTR pOldPin,
WOLFPKCS11_LEAVE("C_SetPIN", rv);
return rv;
}
if (checkPinLen(ulNewLen) != CKR_OK) {
rv = CKR_PIN_INCORRECT;
rv = checkPinLen(ulNewLen);
if (rv != CKR_OK) {
WOLFPKCS11_LEAVE("C_SetPIN", rv);
return rv;
}
Expand Down
Loading
Loading