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
2c75f67
Fix op check
LinuxJedi Apr 7, 2026
a7256fe
Fix HKDF endianess issue
LinuxJedi Apr 7, 2026
c326454
Fix return codes
LinuxJedi Apr 7, 2026
2eb7e7c
Set CKA_LOCAL for symmetric keys
LinuxJedi Apr 7, 2026
d43355c
Add mechanism to symmetric keys
LinuxJedi Apr 7, 2026
9f069de
Fix pin length error handling
LinuxJedi Apr 7, 2026
65f89f5
Add mechanism to NSS generated keys
LinuxJedi Apr 7, 2026
bb8028a
Add correct flags for PBKDF2 keys
LinuxJedi Apr 7, 2026
5b7a4ed
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
86 changes: 44 additions & 42 deletions src/crypto.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3940,7 +3940,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 @@ -4043,7 +4046,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;
Comment thread
LinuxJedi marked this conversation as resolved.
}

/**
Expand Down Expand Up @@ -4086,7 +4091,11 @@ 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;
Comment thread
LinuxJedi marked this conversation as resolved.
if (ret > 0)
return (CK_RV)ret;
return CKR_OK;
Comment thread
LinuxJedi marked this conversation as resolved.
}

/**
Expand Down Expand Up @@ -6327,7 +6336,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 @@ -6697,9 +6706,6 @@ CK_RV C_GenerateKey(CK_SESSION_HANDLE hSession,
CK_RV rv = CKR_OK;
WP11_Session* session = NULL;
WP11_Object* key = NULL;
CK_BBOOL trueVar = CK_TRUE;
CK_BBOOL getVar;
CK_ULONG getVarLen = sizeof(CK_BBOOL);
CK_KEY_TYPE keyType;

WOLFPKCS11_ENTER("C_GenerateKey");
Expand Down Expand Up @@ -6908,18 +6914,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 @@ -7005,18 +7015,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 @@ -7044,31 +7058,19 @@ 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;
Comment thread
LinuxJedi marked this conversation as resolved.
}
else {
rv = AddObject(session, key, pTemplate, ulCount, phKey);
if (rv != CKR_OK) {
WP11_Object_Free(key);
}
}
}
}
if (rv == CKR_OK) {
rv = WP11_Object_GetAttr(key, CKA_SENSITIVE, &getVar, &getVarLen);
if ((rv == CKR_OK) && (getVar == CK_TRUE)) {
rv = WP11_Object_SetAttr(key, CKA_ALWAYS_SENSITIVE, &trueVar,
sizeof(CK_BBOOL));
}
if (rv == CKR_OK)
rv = SetInitialStates(key);
if (rv == CKR_OK) {
rv = WP11_Object_GetAttr(key, CKA_EXTRACTABLE, &getVar, &getVarLen);
if ((rv == CKR_OK) && (getVar == CK_FALSE)) {
rv = WP11_Object_SetAttr(key, CKA_NEVER_EXTRACTABLE, &trueVar,
sizeof(CK_BBOOL));
}
rv = AddObject(session, key, pTemplate, ulCount, phKey);
}
if (rv != CKR_OK && key != NULL) {
WP11_Object_Free(key);
}
}

Expand Down Expand Up @@ -8222,7 +8224,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 Expand Up @@ -9004,11 +9006,11 @@ CK_RV C_EncapsulateKey(CK_SESSION_HANDLE hSession, CK_MECHANISM_PTR pMechanism,
secretKeyLen);
if (ret != 0)
rv = CKR_FUNCTION_FAILED;
if (rv == CKR_OK)
rv = SetInitialStates(secretObj);
if (rv == CKR_OK)
rv = AddObject(session, secretObj, pTemplate, ulAttributeCount,
phKey);
if (rv == CKR_OK)
rv = SetInitialStates(secretObj);
}
if (rv != CKR_OK && secretObj != NULL) {
WP11_Object_Free(secretObj);
Expand Down Expand Up @@ -9110,11 +9112,11 @@ CK_RV C_DecapsulateKey(CK_SESSION_HANDLE hSession, CK_MECHANISM_PTR pMechanism,
secretKeyLen);
if (ret != 0)
rv = CKR_FUNCTION_FAILED;
if (rv == CKR_OK)
rv = SetInitialStates(secretObj);
if (rv == CKR_OK)
rv = AddObject(session, secretObj, pTemplate, ulAttributeCount,
phKey);
if (rv == CKR_OK)
rv = SetInitialStates(secretObj);
}
if (rv != CKR_OK && secretObj != NULL) {
WP11_Object_Free(secretObj);
Expand Down
61 changes: 50 additions & 11 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2699,6 +2699,15 @@ int WP11_Object_Copy(WP11_Object *src, WP11_Object *dest)
else
#endif
{
#ifndef WOLFPKCS11_NO_STORE
/* When the source key is encoded (encrypted at rest), the crypto
* key struct has been freed. The keyData blob is already copied
* above via OBJ_COPY_DATA, so skip the deep key copy. */
if (src->encoded) {
dest->type = src->type;
}
else
#endif
switch (src->type) {
#ifndef NO_RSA
case CKK_RSA: {
Expand Down Expand Up @@ -4896,10 +4905,9 @@ static int MlKemKeyTryDecode(MlKemKey* key, int level, byte* data, word32 len,
else {
ret = wc_MlKemKey_DecodePublicKey(key, data, len);
}
}

if (ret != 0) {
wc_MlKemKey_Free(key);
if (ret != 0) {
wc_MlKemKey_Free(key);
}
}

return ret;
Expand All @@ -4922,7 +4930,7 @@ static int wp11_Object_Decode_MlKemKey(WP11_Object* object)
unsigned char* der;
int len;

if (object->keyDataLen < AES_BLOCK_SIZE)
if (object->keyDataLen <= AES_BLOCK_SIZE)
return BAD_FUNC_ARG;
len = object->keyDataLen - AES_BLOCK_SIZE;

Expand Down Expand Up @@ -9403,7 +9411,9 @@ int WP11_Object_SetMlKemKey(WP11_Object* object, unsigned char** data,
object->devId);
break;
default:
ret = ASN_PARSE_E;
if (object->onToken)
WP11_Lock_UnlockRW(object->lock);
return ASN_PARSE_E;
}

/* Set seed (only for private keys). */
Expand Down Expand Up @@ -11439,6 +11449,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;
}
Comment thread
LinuxJedi marked this conversation as resolved.

/**
* Check whether the attribute matches in the object.
*
Expand Down Expand Up @@ -13148,12 +13171,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 @@ -13162,6 +13187,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;
Comment thread
LinuxJedi marked this conversation as resolved.
}

return ret;
}
#endif /* WOLFPKCS11_HKDF || !NO_AES */
Expand Down Expand Up @@ -13367,6 +13397,13 @@ int WP11_MlKem_GenerateKeyPair(WP11_Object* pub, WP11_Object* priv,
ret = wc_MlKemKey_EncodePublicKey(priv->data.mlKemKey, pubKeyBytes,
pubKeyLen);
}
if (ret == 0) {
/* Re-init the public key before decoding into it since it was
* already Init'd during NewObject -> WP11_Object_SetMlKemKey. */
wc_MlKemKey_Free(pub->data.mlKemKey);
ret = wc_MlKemKey_Init(pub->data.mlKemKey, priv->data.mlKemKey->type,
NULL, pub->devId);
}
if (ret == 0) {
ret = wc_MlKemKey_DecodePublicKey(pub->data.mlKemKey, pubKeyBytes,
pubKeyLen);
Expand Down Expand Up @@ -13403,7 +13440,7 @@ int WP11_MlKem_Encapsulate(WP11_Object* pub, unsigned char** sharedSecret,
int ret;
int rngInit = 0;
WC_RNG rng;
MlKemKey* mlKemKey = pub->data.mlKemKey;
MlKemKey* mlKemKey;
word32 ctLen = 0;

*sharedSecret = NULL;
Expand All @@ -13416,6 +13453,7 @@ int WP11_MlKem_Encapsulate(WP11_Object* pub, unsigned char** sharedSecret,
if (pub->onToken)
WP11_Lock_LockRO(pub->lock);

mlKemKey = pub->data.mlKemKey;
ret = wc_MlKemKey_CipherTextSize(mlKemKey, &ctLen);
if (ret == 0) {
if (pCiphertext == NULL) {
Expand Down Expand Up @@ -13481,7 +13519,7 @@ int WP11_MlKem_Decapsulate(WP11_Object* priv, unsigned char** sharedSecret,
CK_ULONG ulCiphertextLen)
{
int ret;
MlKemKey* mlKemKey = priv->data.mlKemKey;
MlKemKey* mlKemKey;

*sharedSecret = NULL;

Expand All @@ -13493,6 +13531,7 @@ int WP11_MlKem_Decapsulate(WP11_Object* priv, unsigned char** sharedSecret,
if (priv->onToken)
WP11_Lock_LockRO(priv->lock);

mlKemKey = priv->data.mlKemKey;
ret = wc_MlKemKey_SharedSecretSize(mlKemKey, ssLen);
if (ret == 0) {
*sharedSecret = (unsigned char*)XMALLOC(*ssLen, NULL,
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;
Comment thread
LinuxJedi marked this conversation as resolved.
return CKR_OK;
}

Expand Down Expand Up @@ -1277,8 +1277,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 @@ -1361,8 +1361,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 @@ -1436,8 +1436,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