Skip to content

Commit b3dbb10

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: - ML-KEM / ECC (int-returning): translate CRYPTOCB_UNAVAILABLE to 0, surface real HSM errors to the caller via the return value. - ML-DSA (void): discard the callback return with (void) cast since there is no way to propagate it. 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 f3dbeee commit b3dbb10

File tree

4 files changed

+15
-43
lines changed

4 files changed

+15
-43
lines changed

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 & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7945,9 +7945,7 @@ 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)
79497948
int ret = 0;
7950-
#endif
79517949

79527950
if (key == NULL) {
79537951
return 0;
@@ -7957,11 +7955,11 @@ int wc_ecc_free(ecc_key* key)
79577955
if (key->devId != INVALID_DEVID) {
79587956
ret = wc_CryptoCb_Free(key->devId, WC_ALGO_TYPE_PK,
79597957
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 */
7958+
if (ret == WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE)) {
7959+
ret = 0;
7960+
}
7961+
/* always continue to software cleanup */
79637962
}
7964-
(void)ret;
79657963
#endif
79667964

79677965
#if defined(WOLFSSL_ECDSA_SET_K) || defined(WOLFSSL_ECDSA_SET_K_ONE_LOOP) || \
@@ -7975,6 +7973,7 @@ int wc_ecc_free(ecc_key* key)
79757973
mp_free(key->sign_k);
79767974
#ifndef WOLFSSL_NO_MALLOC
79777975
XFREE(key->sign_k, key->heap, DYNAMIC_TYPE_ECC);
7976+
key->sign_k = NULL;
79787977
#endif
79797978
}
79807979
#endif
@@ -8040,15 +8039,17 @@ int wc_ecc_free(ecc_key* key)
80408039
#endif
80418040

80428041
#ifdef WOLFSSL_CUSTOM_CURVES
8043-
if (key->deallocSet && key->dp != NULL)
8042+
if (key->deallocSet && key->dp != NULL) {
80448043
wc_ecc_free_curve(key->dp, key->heap);
8044+
key->dp = NULL;
8045+
}
80458046
#endif
80468047

80478048
#ifdef WOLFSSL_CHECK_MEM_ZERO
80488049
wc_MemZero_Check(key, sizeof(ecc_key));
80498050
#endif
80508051

8051-
return 0;
8052+
return ret;
80528053
}
80538054

80548055
#if !defined(WOLFSSL_ATECC508A) && !defined(WOLFSSL_ATECC608A) && \

wolfcrypt/src/wc_mlkem.c

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -391,21 +391,18 @@ int wc_MlKemKey_Init_Label(MlKemKey* key, const char* label, void* heap,
391391
*/
392392
int wc_MlKemKey_Free(MlKemKey* key)
393393
{
394-
#if defined(WOLF_CRYPTO_CB) && defined(WOLF_CRYPTO_CB_FREE)
395394
int ret = 0;
396-
#endif
397395

398396
if (key != NULL) {
399397
#if defined(WOLF_CRYPTO_CB) && defined(WOLF_CRYPTO_CB_FREE)
400398
if (key->devId != INVALID_DEVID) {
401399
ret = wc_CryptoCb_Free(key->devId, WC_ALGO_TYPE_PK,
402400
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;
401+
if (ret == WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE)) {
402+
ret = 0;
405403
}
406-
/* fall-through to software cleanup */
404+
/* always continue to software cleanup */
407405
}
408-
(void)ret;
409406
#endif
410407
/* Dispose of PRF object. */
411408
mlkem_prf_free(&key->prf);
@@ -418,7 +415,7 @@ int wc_MlKemKey_Free(MlKemKey* key)
418415
ForceZero(key->z, sizeof(key->z));
419416
}
420417

421-
return 0;
418+
return ret;
422419
}
423420

424421
/******************************************************************************/

wolfcrypt/src/wc_pkcs11.c

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6570,12 +6570,6 @@ int wc_Pkcs11_CryptoDevCb(int devId, wc_CryptoInfo* info, void* ctx)
65706570
(ecc_key*)info->free.obj);
65716571
Pkcs11CloseSession(token, &session);
65726572
}
6573-
/* Return CRYPTOCB_UNAVAILABLE so wc_ecc_free() still
6574-
* performs software cleanup. This callback only releases
6575-
* the HSM object. Conditional because wc_ecc_free returns
6576-
* int and can propagate an HSM error to the caller. */
6577-
if (ret == 0)
6578-
ret = CRYPTOCB_UNAVAILABLE;
65796573
}
65806574
else
65816575
#endif
@@ -6589,11 +6583,6 @@ int wc_Pkcs11_CryptoDevCb(int devId, wc_CryptoInfo* info, void* ctx)
65896583
(MlDsaKey*)info->free.obj);
65906584
Pkcs11CloseSession(token, &session);
65916585
}
6592-
/* Always return CRYPTOCB_UNAVAILABLE so wc_dilithium_free()
6593-
* performs software cleanup. This callback only releases
6594-
* the HSM object. Unconditional because wc_dilithium_free
6595-
* returns void and cannot propagate an error. */
6596-
ret = CRYPTOCB_UNAVAILABLE;
65976586
}
65986587
else
65996588
#endif
@@ -6607,14 +6596,6 @@ int wc_Pkcs11_CryptoDevCb(int devId, wc_CryptoInfo* info, void* ctx)
66076596
(MlKemKey*)info->free.obj);
66086597
Pkcs11CloseSession(token, &session);
66096598
}
6610-
/* Return CRYPTOCB_UNAVAILABLE so wc_MlKemKey_Free() still
6611-
* performs software cleanup. This callback only releases
6612-
* the HSM object. Conditional because wc_MlKemKey_Free
6613-
* returns int and can propagate an HSM error to the caller.
6614-
*/
6615-
if (ret == 0) {
6616-
ret = CRYPTOCB_UNAVAILABLE;
6617-
}
66186599
}
66196600
else
66206601
#endif

0 commit comments

Comments
 (0)