Merge pull request #282 from CardContact/fix-deleted-related-public-key

framework-pkcs15: Duplicate public key related to private key rather than referencing the framework object

Referencing the related public key is required to return PKCS#11 attributes for a private key only available
in the public key object (i.e. CKA_MODULUS). This patch adds a copy of the public key to the private key object rather than
referencing the public key object in the framework. This prevents SEGV when the public key framework object
is deleted with C_DestroyObject, but the reference from the public key remains intact.

The bug leads to all kind of stability problems when keys are created and deleted in the same session.

The patch is in particular important if OpenSC is used with EJBCA or any other application using the
SUN PKCS#11 provider: When generating key pairs, then the public key object is eventually garbage collected
which removes the related object in the PKCS#11 module. Because there is no fixed time for this operation,
corruption occurs at random.

In a next step, the remaining related_xxx fields in sc_pkcs11_object should be revised and possibly removed.

framework: Added more error checking

src/libopensc/libopensc.exports

@@ -170,6 +170,7 @@ sc_pkcs15_encode_pukdf_entry
 sc_pkcs15_encode_tokeninfo
 sc_pkcs15_encode_unusedspace
 sc_pkcs15_erase_pubkey
+sc_pkcs15_dup_pubkey
 sc_pkcs15_find_cert_by_id
 sc_pkcs15_find_data_object_by_app_oid
 sc_pkcs15_find_data_object_by_id

src/libopensc/pkcs15-pubkey.c

@@ -1024,6 +1024,86 @@ sc_pkcs15_pubkey_from_prvkey(struct sc_context *ctx, struct sc_pkcs15_prkey *prv
 }
 
 
+int
+sc_pkcs15_dup_pubkey(struct sc_context *ctx, struct sc_pkcs15_pubkey *key, struct sc_pkcs15_pubkey **out)
+{
+	struct sc_pkcs15_pubkey *pubkey = NULL;
+	int rv = SC_SUCCESS;
+	u8* alg;
+	size_t alglen;
+
+	assert(key && out);
+
+	*out = NULL;
+	pubkey = calloc(1, sizeof(struct sc_pkcs15_pubkey));
+	if (!pubkey)
+		LOG_FUNC_RETURN(ctx, SC_ERROR_OUT_OF_MEMORY);
+
+	pubkey->algorithm = key->algorithm;
+
+	if (key->alg_id) {
+		rv = sc_asn1_encode_algorithm_id(ctx, &alg, &alglen,key->alg_id, 0);
+		if (rv == SC_SUCCESS) {
+			pubkey->alg_id = (struct sc_algorithm_id *)calloc(1, sizeof(struct sc_algorithm_id));
+			rv = sc_asn1_decode_algorithm_id(ctx, alg, alglen, pubkey->alg_id, 0);
+			free(alg);
+		}
+	}
+
+	switch (key->algorithm) {
+	case SC_ALGORITHM_RSA:
+		rv = sc_pkcs15_dup_bignum(&pubkey->u.rsa.modulus, &key->u.rsa.modulus);
+		if (!rv)
+			rv = sc_pkcs15_dup_bignum(&pubkey->u.rsa.exponent, &key->u.rsa.exponent);
+		break;
+	case SC_ALGORITHM_DSA:
+		rv = sc_pkcs15_dup_bignum(&pubkey->u.dsa.pub, &key->u.dsa.pub);
+		if (!rv)
+			rv = sc_pkcs15_dup_bignum(&pubkey->u.dsa.p, &key->u.dsa.p);
+		if (!rv)
+			rv = sc_pkcs15_dup_bignum(&pubkey->u.dsa.q, &key->u.dsa.q);
+		if (!rv)
+			rv = sc_pkcs15_dup_bignum(&pubkey->u.dsa.g, &key->u.dsa.g);
+		break;
+	case SC_ALGORITHM_GOSTR3410:
+		break;
+	case SC_ALGORITHM_EC:
+		pubkey->u.ec.ecpointQ.value = malloc(key->u.ec.ecpointQ.len);
+		if (!pubkey->u.ec.ecpointQ.value) {
+			rv = SC_ERROR_OUT_OF_MEMORY;
+			break;
+		}
+		memcpy(pubkey->u.ec.ecpointQ.value, key->u.ec.ecpointQ.value, key->u.ec.ecpointQ.len);
+		pubkey->u.ec.ecpointQ.len = key->u.ec.ecpointQ.len;
+
+		pubkey->u.ec.params.der.value = malloc(key->u.ec.params.der.len);
+		if (!pubkey->u.ec.params.der.value) {
+			rv = SC_ERROR_OUT_OF_MEMORY;
+			break;
+		}
+		memcpy(pubkey->u.ec.params.der.value, key->u.ec.params.der.value, key->u.ec.params.der.len);
+		pubkey->u.ec.params.der.len = key->u.ec.params.der.len;
+
+		pubkey->u.ec.params.named_curve = strdup(key->u.ec.params.named_curve);
+		if (!pubkey->u.ec.params.named_curve)
+			rv = SC_ERROR_OUT_OF_MEMORY;
+
+		break;
+	default:
+		sc_log(ctx, "Unsupported private key algorithm");
+		rv = SC_ERROR_NOT_SUPPORTED;
+	}
+
+	if (rv)
+		sc_pkcs15_free_pubkey(pubkey);
+	else
+		*out = pubkey;
+
+	LOG_FUNC_RETURN(ctx, rv);
+}
+
+
+
 void
 sc_pkcs15_erase_pubkey(struct sc_pkcs15_pubkey *key)
 {

src/libopensc/pkcs15.h

@@ -739,6 +739,8 @@ void sc_pkcs15_erase_pubkey(struct sc_pkcs15_pubkey *);
 void sc_pkcs15_free_pubkey(struct sc_pkcs15_pubkey *);
 int sc_pkcs15_pubkey_from_prvkey(struct sc_context *, struct sc_pkcs15_prkey *,
 		struct sc_pkcs15_pubkey **);
+int sc_pkcs15_dup_pubkey(struct sc_context *, struct sc_pkcs15_pubkey *,
+		struct sc_pkcs15_pubkey **);
 int sc_pkcs15_pubkey_from_cert(struct sc_context *, struct sc_pkcs15_der *,
 		struct sc_pkcs15_pubkey **);
 int sc_pkcs15_pubkey_from_spki_file(struct sc_context *,

src/pkcs11/framework-pkcs15.c

@@ -89,6 +89,7 @@ struct pkcs15_prkey_object {
 	struct pkcs15_any_object	base;
 
 	struct sc_pkcs15_prkey_info *	prv_info;
+	struct sc_pkcs15_pubkey *	pub_data;
 };
 #define prv_flags		base.base.flags
 #define prv_p15obj		base.p15_object
@@ -766,6 +767,7 @@ __pkcs15_prkey_bind_related(struct pkcs15_fw_data *fw_data, struct pkcs15_prkey_
 			if (sc_pkcs15_compare_id(&pubkey->pub_info->id, id)) {
 				sc_log(context, "Associating object %d as public key", i);
 				pk->prv_pubkey = pubkey;
+				sc_pkcs15_dup_pubkey(context, pubkey->pub_data, &pk->pub_data);
 				if (pk->prv_info->modulus_length == 0)
 					pk->prv_info->modulus_length = pubkey->pub_info->modulus_length;
 			}
@@ -2666,6 +2668,7 @@ pkcs15_gen_keypair(struct sc_pkcs11_slot *slot, CK_MECHANISM_PTR pMechanism,
 	struct sc_pkcs15init_pubkeyargs pub_args;
 	struct sc_pkcs15_object	 *priv_key_obj = NULL, *pub_key_obj = NULL;
 	struct pkcs15_any_object *priv_any_obj = NULL, *pub_any_obj = NULL;
+	struct pkcs15_prkey_object *priv_prk_obj = NULL;
 	struct sc_pkcs15_id id;
 	size_t		len;
 	CK_KEY_TYPE	keytype;
@@ -2818,8 +2821,13 @@ pkcs15_gen_keypair(struct sc_pkcs11_slot *slot, CK_MECHANISM_PTR pMechanism,
 	}
 	pkcs15_add_object(slot, priv_any_obj, phPrivKey);
 	pkcs15_add_object(slot, pub_any_obj, phPubKey);
-	((struct pkcs15_prkey_object *) priv_any_obj)->prv_pubkey =
-		(struct pkcs15_pubkey_object *)pub_any_obj;
+
+	priv_prk_obj = (struct pkcs15_prkey_object *) priv_any_obj;
+
+	priv_prk_obj->prv_pubkey = (struct pkcs15_pubkey_object *)pub_any_obj;
+
+	/* Duplicate public key so that parameters can be retrieved even if public key object is deleted */
+	rv = sc_pkcs15_dup_pubkey(context, ((struct pkcs15_pubkey_object *)pub_any_obj)->pub_data, &priv_prk_obj->pub_data);
 
 kpgen_done:
 	sc_pkcs15init_unbind(profile);
@@ -2906,19 +2914,26 @@ pkcs15_any_destroy(struct sc_pkcs11_session *session, void *object)
 		struct pkcs15_any_object *ao_pubkey = (struct pkcs15_any_object *)any_obj->related_pubkey;
 		struct pkcs15_pubkey_object *pubkey = any_obj->related_pubkey;
 
-		/* Delete reference to related certificate of the public key PKCS#11 object */
-		pubkey->pub_genfrom = NULL;
-		if (ao_pubkey->p15_object == NULL)   {
-			/* Unlink related public key FW object if it has no corresponding PKCS#15 object
-			 * and was created from certificate. */
-			--ao_pubkey->refcount;
-			list_delete(&session->slot->objects, ao_pubkey);
-			/* Delete public key object in pkcs15 */
-			if (pubkey->pub_data)   {
-				sc_pkcs15_free_pubkey(pubkey->pub_data);
-				pubkey->pub_data = NULL;
+		/* Check if key is not removed in between */
+		if (list_locate(&session->slot->objects, ao_pubkey) > 0) {
+			sc_log(context, "Found related pubkey %p", any_obj->related_pubkey);
+
+			/* Delete reference to related certificate of the public key PKCS#11 object */
+			pubkey->pub_genfrom = NULL;
+			if (ao_pubkey->p15_object == NULL)   {
+				sc_log(context, "Found related p15 object %p", ao_pubkey->p15_object);
+				/* Unlink related public key FW object if it has no corresponding PKCS#15 object
+				 * and was created from certificate. */
+				--ao_pubkey->refcount;
+				list_delete(&session->slot->objects, ao_pubkey);
+				/* Delete public key object in pkcs15 */
+				if (pubkey->pub_data)   {
+					sc_log(context, "Found pub_data %p", pubkey->pub_data);
+					sc_pkcs15_free_pubkey(pubkey->pub_data);
+					pubkey->pub_data = NULL;
+				}
+				__pkcs15_delete_object(fw_data, ao_pubkey);
 			}
-			__pkcs15_delete_object(fw_data, ao_pubkey);
 		}
 	}
 
@@ -3255,7 +3270,12 @@ struct sc_pkcs11_object_ops pkcs15_cert_ops = {
  */
 static void pkcs15_prkey_release(void *object)
 {
-	__pkcs15_release_object((struct pkcs15_any_object *) object);
+	struct pkcs15_prkey_object *prkey = (struct pkcs15_prkey_object*) object;
+	struct sc_pkcs15_pubkey *key_data = prkey->pub_data;
+
+	if (__pkcs15_release_object((struct pkcs15_any_object *) object) == 0)
+		if (key_data)
+			sc_pkcs15_free_pubkey(key_data);
 }
 
 static CK_RV pkcs15_prkey_set_attribute(struct sc_pkcs11_session *session,
@@ -3299,12 +3319,10 @@ pkcs15_prkey_get_attribute(struct sc_pkcs11_session *session,
 	if ((attr->type == CKA_MODULUS) || (attr->type == CKA_PUBLIC_EXPONENT) ||
 		((attr->type == CKA_MODULUS_BITS) && (prkey->prv_p15obj->type == SC_PKCS15_TYPE_PRKEY_EC)) ||
 		(attr->type == CKA_ECDSA_PARAMS)) {
-		/* First see if we have a associated public key */
-		if (prkey->prv_pubkey && prkey->prv_pubkey->pub_data)   {
-			key = prkey->prv_pubkey->pub_data;
-			sc_log(context, "use friend public key data %p", key);
-		}
-		else {
+		/* First see if we have an associated public key */
+		if (prkey->pub_data) {
+			key = prkey->pub_data;
+		} else {
 			/* Try to find public key or certificate with the public key */
 			unsigned int i;