Skip to content

Commit 7fb0dc0

Browse files
hazefullymtrojnar
authored andcommitted
Improve handling of RSA keys and linked PKCS#11 objects
This commit refactors the pkcs11_rsa function, used in operations on RSA EVP_PKEYs, to return a new reference to the underlying RSA object instead of a pointer to an existing reference. This avoids having to make assumptions about the reference count of the underlying RSA object of an EVP_PKEY in pkcs11_rsa. To reflect the fact that the returned reference must be freed after use, the function is renamed to pkcs11_get1_rsa following the conventions of OpenSSL functions. In addition, this commit ensures that the reference count of a PKCS11_OBJECT_private object that is attached to an RSA object is incremented only once for each RSA object. This assures that a PKCS11_OBJECT_private object can be freed once all RSA objects that reference it are freed.
1 parent 3a93c89 commit 7fb0dc0

File tree

2 files changed

+29
-17
lines changed

2 files changed

+29
-17
lines changed

src/p11_key.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -569,9 +569,7 @@ EVP_PKEY *pkcs11_get_key(PKCS11_OBJECT_private *key0, CK_OBJECT_CLASS object_cla
569569
EVP_PKEY_free(ret);
570570
goto err;
571571
}
572-
if (key->object_class == CKO_PRIVATE_KEY)
573-
pkcs11_object_ref(key);
574-
else /* Public key -> detach PKCS11_OBJECT */
572+
if (key->object_class != CKO_PRIVATE_KEY)
575573
pkcs11_set_ex_data_rsa(rsa, NULL);
576574
break;
577575
case EVP_PKEY_EC:

src/p11_rsa.c

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,21 +29,16 @@
2929
static int rsa_ex_index = 0;
3030
static RSA_METHOD *pkcs11_rsa_method = NULL;
3131

32-
static RSA *pkcs11_rsa(PKCS11_OBJECT_private *key)
32+
static RSA *pkcs11_get1_rsa(PKCS11_OBJECT_private *key)
3333
{
3434
EVP_PKEY *evp_key = pkcs11_get_key(key, key->object_class);
3535
RSA *rsa;
3636

3737
if (!evp_key)
3838
return NULL;
39-
rsa = (RSA *)EVP_PKEY_get0_RSA(evp_key);
40-
/* Danger: this assumes evp_key returned above has at least reference
41-
* count of 2. Which is true in current code as long as key->object_class
42-
* is used for the object_class. */
39+
rsa = (RSA *)EVP_PKEY_get1_RSA(evp_key);
4340
EVP_PKEY_free(evp_key);
44-
/* Freeing the object is necessary because the pkcs11_get_key()
45-
* function increments the reference count */
46-
pkcs11_object_free(key);
41+
4742
return rsa;
4843
}
4944

@@ -52,11 +47,16 @@ static RSA *pkcs11_rsa(PKCS11_OBJECT_private *key)
5247
int pkcs11_sign(int type, const unsigned char *m, unsigned int m_len,
5348
unsigned char *sigret, unsigned int *siglen, PKCS11_OBJECT_private *key)
5449
{
55-
RSA *rsa = pkcs11_rsa(key);
50+
RSA *rsa = pkcs11_get1_rsa(key);
51+
int ret;
5652

5753
if (!rsa)
5854
return -1;
59-
return RSA_sign(type, m, m_len, sigret, siglen, rsa);
55+
56+
ret = RSA_sign(type, m, m_len, sigret, siglen, rsa);
57+
58+
RSA_free(rsa);
59+
return ret;
6060
}
6161

6262
/* Setup PKCS#11 mechanisms for encryption/decryption */
@@ -304,6 +304,8 @@ static EVP_PKEY *pkcs11_get_evp_key_rsa(PKCS11_OBJECT_private *key)
304304
return NULL;
305305
}
306306
if (key->object_class == CKO_PRIVATE_KEY) {
307+
/* This creates a new RSA_KEY object which requires its own key object reference */
308+
key = pkcs11_object_ref(key);
307309
RSA_set_method(rsa, PKCS11_get_rsa_method());
308310
#if OPENSSL_VERSION_NUMBER >= 0x10100005L || ( defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER >= 0x3050000fL )
309311
RSA_set_flags(rsa, RSA_FLAG_EXT_PKEY);
@@ -328,7 +330,7 @@ static EVP_PKEY *pkcs11_get_evp_key_rsa(PKCS11_OBJECT_private *key)
328330
/* TODO: remove this function in libp11 0.5.0 */
329331
int pkcs11_get_key_modulus(PKCS11_OBJECT_private *key, BIGNUM **bn)
330332
{
331-
RSA *rsa = pkcs11_rsa(key);
333+
RSA *rsa = pkcs11_get1_rsa(key);
332334
const BIGNUM *rsa_n;
333335

334336
if (!rsa)
@@ -339,13 +341,16 @@ int pkcs11_get_key_modulus(PKCS11_OBJECT_private *key, BIGNUM **bn)
339341
rsa_n = rsa->n;
340342
#endif
341343
*bn = BN_dup(rsa_n);
344+
345+
RSA_free(rsa);
346+
342347
return *bn == NULL ? 0 : 1;
343348
}
344349

345350
/* TODO: remove this function in libp11 0.5.0 */
346351
int pkcs11_get_key_exponent(PKCS11_OBJECT_private *key, BIGNUM **bn)
347352
{
348-
RSA *rsa = pkcs11_rsa(key);
353+
RSA *rsa = pkcs11_get1_rsa(key);
349354
const BIGNUM *rsa_e;
350355

351356
if (!rsa)
@@ -356,17 +361,26 @@ int pkcs11_get_key_exponent(PKCS11_OBJECT_private *key, BIGNUM **bn)
356361
rsa_e = rsa->e;
357362
#endif
358363
*bn = BN_dup(rsa_e);
364+
365+
RSA_free(rsa);
366+
359367
return *bn == NULL ? 0 : 1;
360368
}
361369

362370
/* TODO: make this function static in libp11 0.5.0 */
363371
int pkcs11_get_key_size(PKCS11_OBJECT_private *key)
364372
{
365-
RSA *rsa = pkcs11_rsa(key);
373+
RSA *rsa = pkcs11_get1_rsa(key);
374+
int size;
366375

367376
if (!rsa)
368377
return 0;
369-
return RSA_size(rsa);
378+
379+
size = RSA_size(rsa);
380+
381+
RSA_free(rsa);
382+
383+
return size;
370384
}
371385

372386
#if ( ( defined (OPENSSL_VERSION_NUMBER) && OPENSSL_VERSION_NUMBER < 0x10100005L ) || ( defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x3020199L ) )

0 commit comments

Comments
 (0)