Fix build issues and findings

LinuxJedi · LinuxJedi · commit 438516377faa · 2026-04-16T14:47:02.000+01:00
diff --git a/src/crypto.c b/src/crypto.c
@@ -6916,37 +6916,21 @@ CK_RV C_GenerateKey(CK_SESSION_HANDLE hSession,
                 ret = WP11_Object_SetSecretKey(pbkdf2Key, secretKeyData, secretKeyLen);
                 if (ret == 0) {
                     WP11_Object_SetKeyGeneration(pbkdf2Key, pMechanism->mechanism);
-                    rv = AddObject(session, pbkdf2Key, pTemplate, ulCount, phKey);
-                    if (rv != CKR_OK) {
-                        WP11_Object_Free(pbkdf2Key);
-                    }
+                    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);
 
-            if (rv == CKR_OK) {
-                rv = WP11_Object_GetAttr(pbkdf2Key, CKA_SENSITIVE, &getVar,
-                                         &getVarLen);
-                if ((rv == CKR_OK) && (getVar == CK_TRUE)) {
-                    rv = WP11_Object_SetAttr(pbkdf2Key, CKA_ALWAYS_SENSITIVE,
-                                             &trueVar, sizeof(CK_BBOOL));
-                }
-                if (rv == CKR_OK) {
-                    rv = WP11_Object_GetAttr(pbkdf2Key, CKA_EXTRACTABLE,
-                                             &getVar, &getVarLen);
-                    if ((rv == CKR_OK) && (getVar == CK_FALSE)) {
-                        rv = WP11_Object_SetAttr(pbkdf2Key,
-                                                 CKA_NEVER_EXTRACTABLE,
-                                                 &trueVar, sizeof(CK_BBOOL));
-                    }
-                }
-            }
-
             return rv;
         }
 #ifdef WOLFPKCS11_NSS
@@ -7033,37 +7017,21 @@ CK_RV C_GenerateKey(CK_SESSION_HANDLE hSession,
                 ret = WP11_Object_SetSecretKey(pbeKey, secretKeyData, secretKeyLen);
                 if (ret == 0) {
                     WP11_Object_SetKeyGeneration(pbeKey, pMechanism->mechanism);
-                    rv = AddObject(session, pbeKey, pTemplate, ulCount, phKey);
-                    if (rv != CKR_OK) {
-                        WP11_Object_Free(pbeKey);
-                    }
+                    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);
 
-            if (rv == CKR_OK) {
-                rv = WP11_Object_GetAttr(pbeKey, CKA_SENSITIVE, &getVar,
-                                         &getVarLen);
-                if ((rv == CKR_OK) && (getVar == CK_TRUE)) {
-                    rv = WP11_Object_SetAttr(pbeKey, CKA_ALWAYS_SENSITIVE,
-                                             &trueVar, sizeof(CK_BBOOL));
-                }
-                if (rv == CKR_OK) {
-                    rv = WP11_Object_GetAttr(pbeKey, CKA_EXTRACTABLE,
-                                             &getVar, &getVarLen);
-                    if ((rv == CKR_OK) && (getVar == CK_FALSE)) {
-                        rv = WP11_Object_SetAttr(pbeKey,
-                                                 CKA_NEVER_EXTRACTABLE,
-                                                 &trueVar, sizeof(CK_BBOOL));
-                    }
-                }
-            }
-
             return rv;
         }
 #endif
@@ -9052,11 +9020,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);
@@ -9158,11 +9126,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);
diff --git a/src/internal.c b/src/internal.c
@@ -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: {
@@ -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;
@@ -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;
 
@@ -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). */
@@ -13387,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);
@@ -13423,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;
@@ -13436,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) {
@@ -13501,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;
 
@@ -13513,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,
diff --git a/tests/pbkdf2_keygen_attrs_test.c b/tests/pbkdf2_keygen_attrs_test.c
@@ -45,7 +45,7 @@
 
 #include "testdata.h"
 
-#ifndef NO_PWDBASED
+#if !defined(NO_PWDBASED) && !defined(NO_HMAC)
 
 #define TEST_DIR "./store/pbkdf2_keygen_attrs_test"
 #define WOLFPKCS11_TOKEN_FILENAME "wp11_token_0000000000000001"
@@ -428,14 +428,14 @@ int main(int argc, char* argv[])
     return (test_failed == 0) ? 0 : 1;
 }
 
-#else /* NO_PWDBASED */
+#else /* NO_PWDBASED || NO_HMAC */
 
 int main(int argc, char* argv[])
 {
     (void)argc;
     (void)argv;
 
-    printf("PWDBASED not available, skipping PBKDF2 keygen attributes test\n");
+    printf("PWDBASED/HMAC not available, skipping PBKDF2 keygen attributes test\n");
     return 0;
 }
 
diff --git a/tests/pkcs11mtt.c b/tests/pkcs11mtt.c
@@ -2649,7 +2649,7 @@ static CK_RV rsa_pkcs15_sig_test(CK_SESSION_HANDLE session,
     }
     if (ret == CKR_OK) {
         ret = funcList->C_VerifyInit(session, &mech, pub);
-        CHECK_CKR(ret, "RSA PKCS#1.5 Verify Init bad hash");
+        CHECK_CKR(ret, "RSA PKCS#1.5 Verify Init before bad hash");
     }
     if (ret == CKR_OK) {
         ret = funcList->C_Verify(session, badHash, sizeof(badHash), out, outSz);
@@ -2713,7 +2713,7 @@ static CK_RV rsa_pss_test(CK_SESSION_HANDLE session, CK_OBJECT_HANDLE priv,
     }
     if (ret == CKR_OK) {
         ret = funcList->C_VerifyInit(session, &mech, pub);
-        CHECK_CKR(ret, "RSA PKCS#1 PSS Verify Init bad hash");
+        CHECK_CKR(ret, "RSA PKCS#1 PSS Verify Init before bad hash");
     }
     if (ret == CKR_OK) {
         ret = funcList->C_Verify(session, badHash, hashSz, out, outSz);
@@ -3842,15 +3842,15 @@ static CK_RV ecdsa_test(CK_SESSION_HANDLE session, CK_OBJECT_HANDLE privKey,
     }
     if (ret == CKR_OK) {
         ret = funcList->C_VerifyInit(session, &mech, pubKey);
-        CHECK_CKR(ret, "ECDSA Verify Init bad hash");
+        CHECK_CKR(ret, "ECDSA Verify Init before bad hash");
     }
     if (ret == CKR_OK) {
         ret = funcList->C_Verify(session, hash, hashSz - 1, out, outSz);
         CHECK_CKR_FAIL(ret, CKR_SIGNATURE_INVALID, "ECDSA Verify bad hash");
     }
     if (ret == CKR_OK) {
         ret = funcList->C_VerifyInit(session, &mech, pubKey);
-        CHECK_CKR(ret, "ECDSA Verify Init bad sig");
+        CHECK_CKR(ret, "ECDSA Verify Init before bad sig");
     }
     if (ret == CKR_OK) {
         outSz = 1;
@@ -6644,6 +6644,87 @@ static CK_RV find_mlkem_priv_key(CK_SESSION_HANDLE session,
     return ret;
 }
 
+static CK_RV mlkem_encap_decap(CK_SESSION_HANDLE session,
+                               CK_OBJECT_HANDLE pubKey,
+                               CK_OBJECT_HANDLE privKey)
+{
+    CK_RV ret = CKR_OK;
+    CK_FUNCTION_LIST_3_2* funcListExt = (CK_FUNCTION_LIST_3_2*)funcList;
+    CK_MECHANISM mech;
+    CK_OBJECT_CLASS secClass = CKO_SECRET_KEY;
+    CK_BBOOL extr = CK_TRUE;
+    CK_ATTRIBUTE secretTmpl[] = {
+        { CKA_CLASS,       &secClass,       sizeof(secClass)       },
+        { CKA_KEY_TYPE,    &genericKeyType, sizeof(genericKeyType) },
+        { CKA_EXTRACTABLE, &extr,           sizeof(extr)           },
+    };
+    CK_ULONG secretTmplCnt = sizeof(secretTmpl) / sizeof(*secretTmpl);
+    CK_OBJECT_HANDLE encapKey = CK_INVALID_HANDLE;
+    CK_OBJECT_HANDLE decapKey = CK_INVALID_HANDLE;
+    CK_BYTE* ciphertext = NULL;
+    CK_ULONG ctLen = 0;
+    CK_BYTE ss1[64];
+    CK_BYTE ss2[64];
+    CK_ULONG ss1Len = sizeof(ss1);
+    CK_ULONG ss2Len = sizeof(ss2);
+    CK_ATTRIBUTE getValueTmpl[] = { { CKA_VALUE, NULL, 0 } };
+
+    mech.mechanism = CKM_ML_KEM;
+    mech.pParameter = NULL;
+    mech.ulParameterLen = 0;
+
+    ret = funcListExt->C_EncapsulateKey(session, &mech, pubKey, secretTmpl,
+                                        secretTmplCnt, NULL, &ctLen, &encapKey);
+    CHECK_CKR(ret, "ML-KEM Encapsulate size query");
+
+    if (ret == CKR_OK) {
+        ciphertext = (CK_BYTE*)malloc(ctLen);
+        if (ciphertext == NULL)
+            ret = CKR_HOST_MEMORY;
+    }
+    if (ret == CKR_OK) {
+        ret = funcListExt->C_EncapsulateKey(session, &mech, pubKey, secretTmpl,
+                                            secretTmplCnt, ciphertext, &ctLen,
+                                            &encapKey);
+        CHECK_CKR(ret, "ML-KEM Encapsulate");
+    }
+    if (ret == CKR_OK) {
+        ret = funcListExt->C_DecapsulateKey(session, &mech, privKey, secretTmpl,
+                                            secretTmplCnt, ciphertext, ctLen,
+                                            &decapKey);
+        CHECK_CKR(ret, "ML-KEM Decapsulate");
+    }
+    if (ret == CKR_OK) {
+        getValueTmpl[0].pValue = ss1;
+        getValueTmpl[0].ulValueLen = ss1Len;
+        ret = funcList->C_GetAttributeValue(session, encapKey, getValueTmpl, 1);
+        CHECK_CKR(ret, "ML-KEM Get encap shared secret");
+        if (ret == CKR_OK)
+            ss1Len = getValueTmpl[0].ulValueLen;
+    }
+    if (ret == CKR_OK) {
+        getValueTmpl[0].pValue = ss2;
+        getValueTmpl[0].ulValueLen = ss2Len;
+        ret = funcList->C_GetAttributeValue(session, decapKey, getValueTmpl, 1);
+        CHECK_CKR(ret, "ML-KEM Get decap shared secret");
+        if (ret == CKR_OK)
+            ss2Len = getValueTmpl[0].ulValueLen;
+    }
+    if (ret == CKR_OK) {
+        CHECK_COND(ss1Len == ss2Len && XMEMCMP(ss1, ss2, ss1Len) == 0,
+                   ret, "ML-KEM Shared secrets match");
+    }
+
+    if (ciphertext != NULL)
+        free(ciphertext);
+    if (encapKey != CK_INVALID_HANDLE)
+        funcList->C_DestroyObject(session, encapKey);
+    if (decapKey != CK_INVALID_HANDLE)
+        funcList->C_DestroyObject(session, decapKey);
+
+    return ret;
+}
+
 static CK_RV test_mlkem_gen_keys(void* args)
 {
     CK_SESSION_HANDLE session = *(CK_SESSION_HANDLE*)args;
@@ -6653,9 +6734,11 @@ static CK_RV test_mlkem_gen_keys(void* args)
     unsigned char* privId = (unsigned char*)"123mlkemmttpriv";
     int privIdLen = (int)strlen((char*)privId);
 
-    /* Generate key pair */
+    /* Generate key pair and exercise encap/decap */
     ret = gen_mlkem_keys(session, CKP_ML_KEM_512, &pub, &priv, NULL, 0,
                          NULL, 0, 0);
+    if (ret == CKR_OK)
+        ret = mlkem_encap_decap(session, pub, priv);
 
     funcList->C_DestroyObject(session, pub);
     funcList->C_DestroyObject(session, priv);
diff --git a/tests/pkcs11test.c b/tests/pkcs11test.c
diff --git a/tests/pkcs11v3test.c b/tests/pkcs11v3test.c

Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,7 @@`
`45`	`45`
`46`	`46`	`#include "testdata.h"`
`47`	`47`
`48`		`-#ifndef NO_PWDBASED`
	`48`	`+#if !defined(NO_PWDBASED) && !defined(NO_HMAC)`
`49`	`49`
`50`	`50`	`#define TEST_DIR "./store/pbkdf2_keygen_attrs_test"`
`51`	`51`	`#define WOLFPKCS11_TOKEN_FILENAME "wp11_token_0000000000000001"`
`@@ -428,14 +428,14 @@ int main(int argc, char* argv[])`
`428`	`428`	`return (test_failed == 0) ? 0 : 1;`
`429`	`429`	`}`
`430`	`430`
`431`		`-#else /* NO_PWDBASED */`
	`431`	`+#else /* NO_PWDBASED \|\| NO_HMAC */`
`432`	`432`
`433`	`433`	`int main(int argc, char* argv[])`
`434`	`434`	`{`
`435`	`435`	`(void)argc;`
`436`	`436`	`(void)argv;`
`437`	`437`
`438`		`- printf("PWDBASED not available, skipping PBKDF2 keygen attributes test\n");`
	`438`	`+ printf("PWDBASED/HMAC not available, skipping PBKDF2 keygen attributes test\n");`
`439`	`439`	`return 0;`
`440`	`440`	`}`
`441`	`441`