OpenSC / libp11

PKCS#11 wrapper library
GNU Lesser General Public License v2.1
314 stars 188 forks source link

Use reference counter instead of copying private key objects #471

Closed Jakuje closed 1 year ago

Jakuje commented 2 years ago

This is a follow-up from #470, which turned out that it does not completely addressed all the corner cases.

After some more debugging I found out that indeed libssh does something weird/stupid with the engines, which OpenSSL does not like and it was repetitively opening and closing engines. This worked for the first key, but follow-up calls to ENGINE_finish() never invoked libp11 cleanup procedures and left mess behind. After fixing this and applying this change, the valgrind no logner complains for invalid memory access nor memory leaks.

Given the previous solution with copying objects lead to some more memory accesses, I would rather use this approach, which usually leads only to memory leaks if stuff go wrong (or application/library does something stupid).

One of the memory leaks was caused by the pkcs11_get_evp_key_ec() assigning the private key structure to the EVP_PKEY, but not setting any method to free it for public keys and certs. This is also fixed so the private key structure is set only for private keys. It should not be needed for the others.

mtrojnar commented 1 year ago

Web editor didn't work for me, so I rebased and merged the commits from terminal: https://github.com/OpenSC/libp11/commit/3061a99dc93524baace689a84b16d19f142707a2 https://github.com/OpenSC/libp11/commit/4a291466a2b43263f36bae791bf3d85a7b588c04