Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Private key received via ENGINE_load_private_key from pkcs11.so ENGINE not free’d by EVP_PKEY_free #522

Closed
tougantium opened this issue Dec 1, 2023 · 1 comment

Comments

@tougantium
Copy link

Hi,
I get an EVP_PKEY from the pkcs11 ENGINE using the following code:

ENGINE *e = ENGINE_by_id("pkcs11");

if(e == NULL) {
	printf("pkcs11 ENGINE is NULL\n");
	return;
}

if(!ENGINE_init(e)) {
	printf("unable to initialize pkcs11 ENGINE\n");
	ENGINE_free(e);
	return;
}

EVP_PKEY *key = ENGINE_load_private_key(e, "1234", NULL, NULL);
if(key == NULL) {
	printf("Unable to load private key from ENGINE\n");
	ENGINE_finish(e);
	ENGINE_free(e);
	return;
}

/*use key*/

EVP_PKEY_free(key);
ENGINE_finish(e);
ENGINE_free(e);

After using the key, I free the key and ENGINE by calling the appropriate free/finish functions. However, what I notice is that the neither the key nor the ENGINE are actually free’d. When I step through the code with gdb I get after the last line:

(gdb) p e->struct_ref 
$1 = 2
(gdb) p e->funct_ref 
$2 = 1
(gdb) p key->references 
$1 = 1

So the ref counts of the ENGINE (structural and functional) and of the key are > 0. So neither the ENGINE nor the key are actually free’d.

After a quick look at the libp11 code, I got the impression that the ENGINE holds a reference to the key and the key holds a reference to the ENGINE (in the pmeth_engine field). If this is the case, it would be a cyclic reference that prevents ENGINE and key from being free’d.

If I add the line EVP_PKEY_set1_engine(key, NULL); just before the line EVP_PKEY_free(key); everything seems to work as expected. The EVP_PKEY_set1_engine function finishes the ENGINE referenced in the pmeth_engine field (reduces the ref counts of the ENGINE) and sets the pmeth_engine field to NULL, thus breaking the cycle.

After calling the free/finish functions as before, the ENGINE is free’d first, which ultimately also frees the key via its internal reference.

Am I missing some obvious point here? If so, could you please show me how to clean up correctly?

If my interpretation is correct and there is no straightforward cleanup strategy, I would find it irritating to be responsible for manually breaking the cycle to get a the desired cleanup.

The observed behavior could be produced with openSSL 1.1.1w and openSSL 3.0.2 and libp11 0.4.12.

I appreciate your help.

@olszomal
Copy link
Contributor

olszomal commented Nov 7, 2024

This memory leak was fixed in PR #550 and is no longer present in the latest master branch. I recommend closing this issue.

@mtrojnar mtrojnar closed this as completed Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants