Merge pull request #178 from LinuxJedi/f-fixes7

Static code analysis fixes

Author: dgarske

src/crypto.c

@@ -3960,7 +3960,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;
 }
@@ -4063,7 +4066,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;
 }
 
 /**
@@ -4106,7 +4111,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;
+    if (ret > 0)
+        return (CK_RV)ret;
+    return CKR_OK;
 }
 
 /**
@@ -6347,7 +6356,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;
 
@@ -6717,9 +6726,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");
@@ -6928,18 +6934,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
@@ -7025,18 +7035,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
@@ -7064,31 +7078,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;
             }
-            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);
         }
     }
 
@@ -9053,11 +9055,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);
@@ -9159,11 +9161,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);

src/internal.c

@@ -2730,6 +2730,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: {
@@ -4927,10 +4936,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;
@@ -4953,7 +4961,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;
 
@@ -9442,7 +9450,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). */
@@ -11496,6 +11506,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.
  *
@@ -13205,12 +13228,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;
@@ -13219,6 +13244,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 */
@@ -13424,6 +13454,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);
@@ -13460,7 +13497,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;
@@ -13473,6 +13510,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) {
@@ -13538,7 +13576,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;
 
@@ -13550,6 +13588,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,

src/slot.c

@@ -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;
     return CKR_OK;
 }
 
@@ -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;
     }
@@ -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;
     }
@@ -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;
     }