Skip to content

Commit 85dd923

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 21f1587 commit 85dd923

4 files changed

Lines changed: 16 additions & 49 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: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -391,21 +391,15 @@ int wc_MlKemKey_Init_Label(MlKemKey* key, int type, const char* label,
391391
*/
392392
int wc_MlKemKey_Free(MlKemKey* key)
393393
{
394-
#if defined(WOLF_CRYPTO_CB) && defined(WOLF_CRYPTO_CB_FREE)
395-
int ret = 0;
396-
#endif
397-
398394
if (key != NULL) {
399395
#if defined(WOLF_CRYPTO_CB) && defined(WOLF_CRYPTO_CB_FREE)
400396
if (key->devId != INVALID_DEVID) {
401-
ret = wc_CryptoCb_Free(key->devId, WC_ALGO_TYPE_PK,
402-
WC_PK_TYPE_PQC_KEM_KEYGEN, WC_PQC_KEM_TYPE_KYBER, (void*)key);
403-
if (ret != WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE)) {
404-
return ret;
405-
}
406-
/* fall-through to software cleanup */
397+
(void)wc_CryptoCb_Free(key->devId, WC_ALGO_TYPE_PK,
398+
WC_PK_TYPE_PQC_KEM_KEYGEN,
399+
WC_PQC_KEM_TYPE_KYBER,
400+
(void*)key);
401+
/* always continue to software cleanup */
407402
}
408-
(void)ret;
409403
#endif
410404
/* Dispose of PRF object. */
411405
mlkem_prf_free(&key->prf);

wolfcrypt/src/wc_pkcs11.c

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6582,12 +6582,6 @@ int wc_Pkcs11_CryptoDevCb(int devId, wc_CryptoInfo* info, void* ctx)
65826582
(ecc_key*)info->free.obj);
65836583
Pkcs11CloseSession(token, &session);
65846584
}
6585-
/* Return CRYPTOCB_UNAVAILABLE so wc_ecc_free() still
6586-
* performs software cleanup. This callback only releases
6587-
* the HSM object. Conditional because wc_ecc_free returns
6588-
* int and can propagate an HSM error to the caller. */
6589-
if (ret == 0)
6590-
ret = CRYPTOCB_UNAVAILABLE;
65916585
}
65926586
else
65936587
#endif
@@ -6601,11 +6595,6 @@ int wc_Pkcs11_CryptoDevCb(int devId, wc_CryptoInfo* info, void* ctx)
66016595
(MlDsaKey*)info->free.obj);
66026596
Pkcs11CloseSession(token, &session);
66036597
}
6604-
/* Always return CRYPTOCB_UNAVAILABLE so wc_dilithium_free()
6605-
* performs software cleanup. This callback only releases
6606-
* the HSM object. Unconditional because wc_dilithium_free
6607-
* returns void and cannot propagate an error. */
6608-
ret = CRYPTOCB_UNAVAILABLE;
66096598
}
66106599
else
66116600
#endif
@@ -6619,14 +6608,6 @@ int wc_Pkcs11_CryptoDevCb(int devId, wc_CryptoInfo* info, void* ctx)
66196608
(MlKemKey*)info->free.obj);
66206609
Pkcs11CloseSession(token, &session);
66216610
}
6622-
/* Return CRYPTOCB_UNAVAILABLE so wc_MlKemKey_Free() still
6623-
* performs software cleanup. This callback only releases
6624-
* the HSM object. Conditional because wc_MlKemKey_Free
6625-
* returns int and can propagate an HSM error to the caller.
6626-
*/
6627-
if (ret == 0) {
6628-
ret = CRYPTOCB_UNAVAILABLE;
6629-
}
66306611
}
66316612
else
66326613
#endif

0 commit comments

Comments
 (0)