Skip to content

Commit b7f6fce

Browse files
committed
cryptocb: always run software cleanup in key Free functions
The WOLF_CRYPTO_CB_FREE path in wc_MlKemKey_Free, wc_dilithium_free, and wc_ecc_free returned early when the crypto callback succeeded, skipping local cleanup: ForceZero on private key material, PRF/hash object frees (ML-KEM), SHAKE free and cached vector frees (ML-DSA), and mp_forcezero on the private scalar and all hardware port frees (ECC). Any non-PKCS#11 callback returning 0 would silently leave key material in memory. The PKCS#11 backend worked around this by returning CRYPTOCB_UNAVAILABLE on success to force the fallthrough — a fragile contract that is not part of the documented callback interface. Fix by always continuing to software cleanup after invoking the callback. Remove the CRYPTOCB_UNAVAILABLE workaround from the three PKCS#11 free dispatchers (ECC, ML-DSA, ML-KEM); they now return the real result of C_DestroyObject.
1 parent fc81f06 commit b7f6fce

4 files changed

Lines changed: 15 additions & 45 deletions

File tree

wolfcrypt/src/dilithium.c

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10905,22 +10905,15 @@ int wc_dilithium_get_level(dilithium_key* key, byte* level)
1090510905
*/
1090610906
void wc_dilithium_free(dilithium_key* key)
1090710907
{
10908-
#if defined(WOLF_CRYPTO_CB) && defined(WOLF_CRYPTO_CB_FREE)
10909-
int ret = 0;
10910-
#endif
10911-
1091210908
if (key != NULL) {
1091310909
#if defined(WOLF_CRYPTO_CB) && defined(WOLF_CRYPTO_CB_FREE)
1091410910
if (key->devId != INVALID_DEVID) {
10915-
ret = wc_CryptoCb_Free(key->devId, WC_ALGO_TYPE_PK,
10911+
(void)wc_CryptoCb_Free(key->devId, WC_ALGO_TYPE_PK,
1091610912
WC_PK_TYPE_PQC_SIG_KEYGEN,
1091710913
WC_PQC_SIG_TYPE_DILITHIUM,
1091810914
(void*)key);
10919-
if (ret != WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE))
10920-
return;
10921-
/* fall-through to software cleanup */
10915+
/* always continue to software cleanup */
1092210916
}
10923-
(void)ret;
1092410917
#endif
1092510918
#ifdef WOLFSSL_WC_DILITHIUM
1092610919
#ifndef WC_DILITHIUM_FIXED_ARRAY

wolfcrypt/src/ecc.c

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7945,23 +7945,19 @@ void wc_ecc_free_curve(const ecc_set_type* curve, void* heap)
79457945
WOLFSSL_ABI
79467946
int wc_ecc_free(ecc_key* key)
79477947
{
7948-
#if defined(WOLF_CRYPTO_CB) && defined(WOLF_CRYPTO_CB_FREE)
7949-
int ret = 0;
7950-
#endif
7951-
79527948
if (key == NULL) {
79537949
return 0;
79547950
}
79557951

79567952
#if defined(WOLF_CRYPTO_CB) && defined(WOLF_CRYPTO_CB_FREE)
79577953
if (key->devId != INVALID_DEVID) {
7958-
ret = wc_CryptoCb_Free(key->devId, WC_ALGO_TYPE_PK,
7954+
/* Best-effort HSM resource release; errors are intentionally discarded
7955+
* so that software cleanup always runs and wc_ecc_free() retains its
7956+
* ABI guarantee of returning 0 on success. */
7957+
(void)wc_CryptoCb_Free(key->devId, WC_ALGO_TYPE_PK,
79597958
WC_PK_TYPE_EC_KEYGEN, 0, key);
7960-
if (ret != WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE))
7961-
return ret;
7962-
/* fall-through to software cleanup */
7959+
/* always continue to software cleanup */
79637960
}
7964-
(void)ret;
79657961
#endif
79667962

79677963
#if defined(WOLFSSL_ECDSA_SET_K) || defined(WOLFSSL_ECDSA_SET_K_ONE_LOOP) || \
@@ -7975,6 +7971,7 @@ int wc_ecc_free(ecc_key* key)
79757971
mp_free(key->sign_k);
79767972
#ifndef WOLFSSL_NO_MALLOC
79777973
XFREE(key->sign_k, key->heap, DYNAMIC_TYPE_ECC);
7974+
key->sign_k = NULL;
79787975
#endif
79797976
}
79807977
#endif
@@ -8040,8 +8037,10 @@ int wc_ecc_free(ecc_key* key)
80408037
#endif
80418038

80428039
#ifdef WOLFSSL_CUSTOM_CURVES
8043-
if (key->deallocSet && key->dp != NULL)
8040+
if (key->deallocSet && key->dp != NULL) {
80448041
wc_ecc_free_curve(key->dp, key->heap);
8042+
key->dp = NULL;
8043+
}
80458044
#endif
80468045

80478046
#ifdef WOLFSSL_CHECK_MEM_ZERO

wolfcrypt/src/wc_mlkem.c

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -389,21 +389,18 @@ int wc_MlKemKey_Init_Label(MlKemKey* key, int type, const char* label,
389389
*/
390390
int wc_MlKemKey_Free(MlKemKey* key)
391391
{
392-
#if defined(WOLF_CRYPTO_CB) && defined(WOLF_CRYPTO_CB_FREE)
393392
int ret = 0;
394-
#endif
395393

396394
if (key != NULL) {
397395
#if defined(WOLF_CRYPTO_CB) && defined(WOLF_CRYPTO_CB_FREE)
398396
if (key->devId != INVALID_DEVID) {
399397
ret = wc_CryptoCb_Free(key->devId, WC_ALGO_TYPE_PK,
400398
WC_PK_TYPE_PQC_KEM_KEYGEN, WC_PQC_KEM_TYPE_KYBER, (void*)key);
401-
if (ret != WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE)) {
402-
return ret;
399+
if (ret == WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE)) {
400+
ret = 0;
403401
}
404-
/* fall-through to software cleanup */
402+
/* always continue to software cleanup */
405403
}
406-
(void)ret;
407404
#endif
408405
/* Dispose of PRF object. */
409406
mlkem_prf_free(&key->prf);
@@ -416,7 +413,7 @@ int wc_MlKemKey_Free(MlKemKey* key)
416413
ForceZero(key->z, sizeof(key->z));
417414
}
418415

419-
return 0;
416+
return ret;
420417
}
421418

422419
/******************************************************************************/

wolfcrypt/src/wc_pkcs11.c

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6574,12 +6574,6 @@ int wc_Pkcs11_CryptoDevCb(int devId, wc_CryptoInfo* info, void* ctx)
65746574
(ecc_key*)info->free.obj);
65756575
Pkcs11CloseSession(token, &session);
65766576
}
6577-
/* Return CRYPTOCB_UNAVAILABLE so wc_ecc_free() still
6578-
* performs software cleanup. This callback only releases
6579-
* the HSM object. Conditional because wc_ecc_free returns
6580-
* int and can propagate an HSM error to the caller. */
6581-
if (ret == 0)
6582-
ret = CRYPTOCB_UNAVAILABLE;
65836577
}
65846578
else
65856579
#endif
@@ -6593,11 +6587,6 @@ int wc_Pkcs11_CryptoDevCb(int devId, wc_CryptoInfo* info, void* ctx)
65936587
(MlDsaKey*)info->free.obj);
65946588
Pkcs11CloseSession(token, &session);
65956589
}
6596-
/* Always return CRYPTOCB_UNAVAILABLE so wc_dilithium_free()
6597-
* performs software cleanup. This callback only releases
6598-
* the HSM object. Unconditional because wc_dilithium_free
6599-
* returns void and cannot propagate an error. */
6600-
ret = CRYPTOCB_UNAVAILABLE;
66016590
}
66026591
else
66036592
#endif
@@ -6611,14 +6600,6 @@ int wc_Pkcs11_CryptoDevCb(int devId, wc_CryptoInfo* info, void* ctx)
66116600
(MlKemKey*)info->free.obj);
66126601
Pkcs11CloseSession(token, &session);
66136602
}
6614-
/* Return CRYPTOCB_UNAVAILABLE so wc_MlKemKey_Free() still
6615-
* performs software cleanup. This callback only releases
6616-
* the HSM object. Conditional because wc_MlKemKey_Free
6617-
* returns int and can propagate an HSM error to the caller.
6618-
*/
6619-
if (ret == 0) {
6620-
ret = CRYPTOCB_UNAVAILABLE;
6621-
}
66226603
}
66236604
else
66246605
#endif

0 commit comments

Comments
 (0)