Skip to content

Commit 8d2c40f

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 97ea4f8 commit 8d2c40f

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
@@ -10963,22 +10963,15 @@ int wc_dilithium_get_level(dilithium_key* key, byte* level)
1096310963
*/
1096410964
void wc_dilithium_free(dilithium_key* key)
1096510965
{
10966-
#if defined(WOLF_CRYPTO_CB) && defined(WOLF_CRYPTO_CB_FREE)
10967-
int ret = 0;
10968-
#endif
10969-
1097010966
if (key != NULL) {
1097110967
#if defined(WOLF_CRYPTO_CB) && defined(WOLF_CRYPTO_CB_FREE)
1097210968
if (key->devId != INVALID_DEVID) {
10973-
ret = wc_CryptoCb_Free(key->devId, WC_ALGO_TYPE_PK,
10969+
(void)wc_CryptoCb_Free(key->devId, WC_ALGO_TYPE_PK,
1097410970
WC_PK_TYPE_PQC_SIG_KEYGEN,
1097510971
WC_PQC_SIG_TYPE_DILITHIUM,
1097610972
(void*)key);
10977-
if (ret != WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE))
10978-
return;
10979-
/* fall-through to software cleanup */
10973+
/* always continue to software cleanup */
1098010974
}
10981-
(void)ret;
1098210975
#endif
1098310976
#ifdef WOLFSSL_WC_DILITHIUM
1098410977
#ifndef WC_DILITHIUM_FIXED_ARRAY

wolfcrypt/src/ecc.c

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7930,23 +7930,19 @@ void wc_ecc_free_curve(ecc_set_type* curve, void* heap)
79307930
WOLFSSL_ABI
79317931
int wc_ecc_free(ecc_key* key)
79327932
{
7933-
#if defined(WOLF_CRYPTO_CB) && defined(WOLF_CRYPTO_CB_FREE)
7934-
int ret = 0;
7935-
#endif
7936-
79377933
if (key == NULL) {
79387934
return 0;
79397935
}
79407936

79417937
#if defined(WOLF_CRYPTO_CB) && defined(WOLF_CRYPTO_CB_FREE)
79427938
if (key->devId != INVALID_DEVID) {
7943-
ret = wc_CryptoCb_Free(key->devId, WC_ALGO_TYPE_PK,
7939+
/* Best-effort HSM resource release; errors are intentionally discarded
7940+
* so that software cleanup always runs and wc_ecc_free() retains its
7941+
* ABI guarantee of returning 0 on success. */
7942+
(void)wc_CryptoCb_Free(key->devId, WC_ALGO_TYPE_PK,
79447943
WC_PK_TYPE_EC_KEYGEN, 0, key);
7945-
if (ret != WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE))
7946-
return ret;
7947-
/* fall-through to software cleanup */
7944+
/* always continue to software cleanup */
79487945
}
7949-
(void)ret;
79507946
#endif
79517947

79527948
#if defined(WOLFSSL_ECDSA_SET_K) || defined(WOLFSSL_ECDSA_SET_K_ONE_LOOP) || \
@@ -7960,6 +7956,7 @@ int wc_ecc_free(ecc_key* key)
79607956
mp_free(key->sign_k);
79617957
#ifndef WOLFSSL_NO_MALLOC
79627958
XFREE(key->sign_k, key->heap, DYNAMIC_TYPE_ECC);
7959+
key->sign_k = NULL;
79637960
#endif
79647961
}
79657962
#endif
@@ -8025,8 +8022,10 @@ int wc_ecc_free(ecc_key* key)
80258022
#endif
80268023

80278024
#ifdef WOLFSSL_CUSTOM_CURVES
8028-
if (key->deallocSet && key->dp != NULL)
8025+
if (key->deallocSet && key->dp != NULL) {
80298026
wc_ecc_free_curve((ecc_set_type *)(wc_ptr_t)key->dp, key->heap);
8027+
key->dp = NULL;
8028+
}
80308029
#endif
80318030

80328031
#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)