Fix PKCS11 object leak in Pkcs11ECDH#9780
Fix PKCS11 object leak in Pkcs11ECDH#9780mattia-moffa wants to merge 2 commits intowolfSSL:masterfrom
Conversation
|
Jenkins retest this please. FIPS v3 no history |
wolfcrypt/src/wc_pkcs11.c
Outdated
| if (secret != CK_INVALID_HANDLE) | ||
| session->func->C_DestroyObject(session->handle, secret); | ||
|
|
||
| if (sessionKey) { |
There was a problem hiding this comment.
Please check against NULL.
wolfcrypt/src/wc_pkcs11.c
Outdated
| else { | ||
| ret = Pkcs11FindEccKey(&privateKey, CKO_PRIVATE_KEY, session, | ||
| info->pk.ecdh.public_key, CKA_DERIVE); | ||
| if (ret == 0) { |
There was a problem hiding this comment.
The private key lifecycle should be bound to the ECC object.
I don't think the private key should be destroyed here.
There was a problem hiding this comment.
That would be the best option. But AFAICT, we currently don't have logic to handle that case in the ecc_free() method. Would be a great addition, as we will also face the same issue once ML-KEM is added to the PKCS#11 interface (private decapsulation key must also be deleted once the handshake is done on the client side — I'm currently working on that ML-KEM integration).
|
|
||
| if (sessionKey) | ||
| if (secret != CK_INVALID_HANDLE) | ||
| session->func->C_DestroyObject(session->handle, secret); |
There was a problem hiding this comment.
May want the secret held on the device!
When doing TLS on the device, you don't want the secret to leave it. Instead the secret is used to generate the TLS session keys.
There was a problem hiding this comment.
In that case, the lifetime of the secret key object should be bound to the handshake duration, shouldn't it? Then, we can still use it in the future for potential key derivations on the external token via PKCS#11.
There was a problem hiding this comment.
But do we have a way of binding it to the handshake duration from the wolfCrypt side?
5 participants
Description
Pkcs11ECDHleaked two PKCS11 objects per ECDH operation:C_DeriveKeyis never destroyed after its value is extracted viaC_GetAttributeValuePkcs11EcKeyGen(e.g. for TLS ECDHE) is not cleaned up.Pkcs11EcKeyGenintentionally leaves the private key alive for the subsequentPkcs11ECDHcall, butPkcs11ECDHonly destroys private keys created from raw scalar data (if (sessionKey)), not keys found viaPkcs11FindEccKey.Reported via ZD#20700
Testing