Skip to content

Commit 2829c59

Browse files
hyperfinitismgotthardp
authored andcommitted
fix(keymgmt): fix pkey and parent handle leaks
Fix cleanup paths in EC and RSA key generation. - In `tpm2_ec_keymgmt_gen()`, the allocated `TPM2_PKEY` object was leaked when `tpm2_semaphore_lock()` failed. - In both `tpm2_ec_keymgmt_gen()` and `tpm2_rsa_keymgmt_gen()`, a loaded or created parent handle could be leaked on key generation failure before the normal parent cleanup path was reached. Clean up the parent handle on failure paths and avoid unlocking the ESYS semaphore when the lock was not acquired. Signed-off-by: Takuma IMAMURA <209989118+hyperfinitism@users.noreply.github.com>
1 parent e447603 commit 2829c59

2 files changed

Lines changed: 31 additions & 12 deletions

File tree

src/tpm2-provider-keymgmt-ec.c

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -245,13 +245,13 @@ tpm2_ec_keymgmt_gen(void *ctx, OSSL_CALLBACK *cb, void *cbarg)
245245
TPML_PCR_SELECTION creation_pcr = { .count = 0 };
246246

247247
if (!tpm2_semaphore_lock(gen->esys_lock))
248-
return 0;
248+
goto error2;
249249
/* older TPM2 chips do not support Esys_CreateLoaded */
250250
r = Esys_Create(gen->esys_ctx, parent,
251251
ESYS_TR_PASSWORD, ESYS_TR_NONE, ESYS_TR_NONE,
252252
&gen->inSensitive, &gen->inPublic, &outside_info, &creation_pcr,
253253
&keyPrivate, &keyPublic, NULL, NULL, NULL);
254-
TPM2_CHECK_RC(gen->core, r, TPM2_ERR_CANNOT_CREATE_KEY, goto error1);
254+
TPM2_CHECK_RC(gen->core, r, TPM2_ERR_CANNOT_CREATE_KEY, goto error3);
255255

256256
pkey->data.pub = *keyPublic;
257257
pkey->data.privatetype = KEY_TYPE_BLOB;
@@ -262,7 +262,7 @@ tpm2_ec_keymgmt_gen(void *ctx, OSSL_CALLBACK *cb, void *cbarg)
262262
keyPrivate, keyPublic, &pkey->object);
263263
free(keyPublic);
264264
cleanse_free(keyPrivate, sizeof(TPM2B_PRIVATE));
265-
TPM2_CHECK_RC(gen->core, r, TPM2_ERR_CANNOT_CREATE_KEY, goto error1);
265+
TPM2_CHECK_RC(gen->core, r, TPM2_ERR_CANNOT_CREATE_KEY, goto error3);
266266

267267
if (gen->parentHandle && gen->parentHandle != TPM2_RH_OWNER)
268268
Esys_TR_Close(gen->esys_ctx, &parent);
@@ -271,14 +271,24 @@ tpm2_ec_keymgmt_gen(void *ctx, OSSL_CALLBACK *cb, void *cbarg)
271271

272272
if (gen->inSensitive.sensitive.userAuth.size > 0) {
273273
r = Esys_TR_SetAuth(gen->esys_ctx, pkey->object, &gen->inSensitive.sensitive.userAuth);
274-
TPM2_CHECK_RC(gen->core, r, TPM2_ERR_CANNOT_LOAD_KEY, goto error2);
274+
TPM2_CHECK_RC(gen->core, r, TPM2_ERR_CANNOT_LOAD_KEY, goto error4);
275275
}
276276
tpm2_semaphore_unlock(gen->esys_lock);
277277
return pkey;
278-
error2:
278+
error4:
279279
Esys_FlushContext(gen->esys_ctx, pkey->object);
280-
error1:
280+
pkey->object = ESYS_TR_NONE;
281+
error3:
281282
tpm2_semaphore_unlock(gen->esys_lock);
283+
error2:
284+
if (parent != ESYS_TR_NONE) {
285+
if (gen->parentHandle && gen->parentHandle != TPM2_RH_OWNER)
286+
Esys_TR_Close(gen->esys_ctx, &parent);
287+
else
288+
Esys_FlushContext(gen->esys_ctx, parent);
289+
parent = ESYS_TR_NONE;
290+
}
291+
error1:
282292
OPENSSL_clear_free(pkey, sizeof(TPM2_PKEY));
283293
return NULL;
284294
}

src/tpm2-provider-keymgmt-rsa.c

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -287,13 +287,13 @@ tpm2_rsa_keymgmt_gen(void *ctx, OSSL_CALLBACK *cb, void *cbarg)
287287
TPML_PCR_SELECTION creation_pcr = { .count = 0 };
288288

289289
if (!tpm2_semaphore_lock(gen->esys_lock))
290-
goto error1;
290+
goto error2;
291291
/* older TPM2 chips do not support Esys_CreateLoaded */
292292
r = Esys_Create(gen->esys_ctx, parent,
293293
ESYS_TR_PASSWORD, ESYS_TR_NONE, ESYS_TR_NONE,
294294
&gen->inSensitive, &gen->inPublic, &outside_info, &creation_pcr,
295295
&keyPrivate, &keyPublic, NULL, NULL, NULL);
296-
TPM2_CHECK_RC(gen->core, r, TPM2_ERR_CANNOT_CREATE_KEY, goto error2);
296+
TPM2_CHECK_RC(gen->core, r, TPM2_ERR_CANNOT_CREATE_KEY, goto error3);
297297

298298
pkey->data.pub = *keyPublic;
299299
pkey->data.privatetype = KEY_TYPE_BLOB;
@@ -304,7 +304,7 @@ tpm2_rsa_keymgmt_gen(void *ctx, OSSL_CALLBACK *cb, void *cbarg)
304304
keyPrivate, keyPublic, &pkey->object);
305305
free(keyPublic);
306306
cleanse_free(keyPrivate, sizeof(TPM2B_PRIVATE));
307-
TPM2_CHECK_RC(gen->core, r, TPM2_ERR_CANNOT_CREATE_KEY, goto error2);
307+
TPM2_CHECK_RC(gen->core, r, TPM2_ERR_CANNOT_CREATE_KEY, goto error3);
308308

309309
if (gen->parentHandle && gen->parentHandle != TPM2_RH_OWNER)
310310
Esys_TR_Close(gen->esys_ctx, &parent);
@@ -313,14 +313,23 @@ tpm2_rsa_keymgmt_gen(void *ctx, OSSL_CALLBACK *cb, void *cbarg)
313313

314314
if (gen->inSensitive.sensitive.userAuth.size > 0) {
315315
r = Esys_TR_SetAuth(gen->esys_ctx, pkey->object, &gen->inSensitive.sensitive.userAuth);
316-
TPM2_CHECK_RC(gen->core, r, TPM2_ERR_CANNOT_LOAD_KEY, goto error3);
316+
TPM2_CHECK_RC(gen->core, r, TPM2_ERR_CANNOT_LOAD_KEY, goto error4);
317317
}
318318
tpm2_semaphore_unlock(gen->esys_lock);
319319
return pkey;
320-
error3:
320+
error4:
321321
Esys_FlushContext(gen->esys_ctx, pkey->object);
322-
error2:
322+
pkey->object = ESYS_TR_NONE;
323+
error3:
323324
tpm2_semaphore_unlock(gen->esys_lock);
325+
error2:
326+
if (parent != ESYS_TR_NONE) {
327+
if (gen->parentHandle && gen->parentHandle != TPM2_RH_OWNER)
328+
Esys_TR_Close(gen->esys_ctx, &parent);
329+
else
330+
Esys_FlushContext(gen->esys_ctx, parent);
331+
parent = ESYS_TR_NONE;
332+
}
324333
error1:
325334
OPENSSL_clear_free(pkey, sizeof(TPM2_PKEY));
326335
return NULL;

0 commit comments

Comments
 (0)