Ticket: https://fedorahosted.org/freeipa/ticket/4657
Patch attached. -- Martin Basti
From 15509e09b658e9b2460ce42081baa1bb77a4d1e8 Mon Sep 17 00:00:00 2001 From: Martin Basti <mba...@redhat.com> Date: Tue, 24 Feb 2015 19:25:31 +0100 Subject: [PATCH] Fix memory leaks in ipap11helper Ticket: https://fedorahosted.org/freeipa/ticket/4657 --- ipapython/ipap11helper/p11helper.c | 295 ++++++++++++++++++++++++++----------- 1 file changed, 212 insertions(+), 83 deletions(-) diff --git a/ipapython/ipap11helper/p11helper.c b/ipapython/ipap11helper/p11helper.c index c638bbe849f1bbddc8004bd1c4cccc1128b1c9e7..e3a7a9399c12759c537d13a291dcdf6ec1a1efa4 100644 --- a/ipapython/ipap11helper/p11helper.c +++ b/ipapython/ipap11helper/p11helper.c @@ -197,9 +197,11 @@ unsigned char* unicode_to_char_array(PyObject *unicode, Py_ssize_t *l) { /* Copy string first, then DECREF * https://docs.python.org/2/c-api/string.html#c.PyString_AS_STRING */ - result = (unsigned char *) malloc((size_t) *l); + result = (unsigned char *) PyMem_Malloc((size_t) *l); if (result == NULL){ - PyErr_SetString(ipap11helperError, "Memory allocation error"); + Py_DECREF(utf8_str); + PyErr_NoMemory(); + return NULL; } else { memcpy(result, bytes, *l); } @@ -650,7 +652,9 @@ P11_Helper_generate_master_key(P11_Helper* self, PyObject *args, PyObject *kwds) PyObject *label_unicode = NULL; Py_ssize_t label_length = 0; + CK_BYTE *label = NULL; int r; + int error = 0; static char *kwlist[] = { "subject", "id", "key_length", "cka_copyable", "cka_decrypt", "cka_derive", "cka_encrypt", "cka_extractable", "cka_modifiable", "cka_private", "cka_sensitive", "cka_sign", @@ -672,26 +676,28 @@ P11_Helper_generate_master_key(P11_Helper* self, PyObject *args, PyObject *kwds) return NULL; } - CK_BYTE *label = (unsigned char*) unicode_to_char_array(label_unicode, + label = (unsigned char*) unicode_to_char_array(label_unicode, &label_length); CK_MECHANISM mechanism = { //TODO param? CKM_AES_KEY_GEN, NULL_PTR, 0 }; + if (label == NULL) return NULL; if ((key_length != 16) && (key_length != 24) && (key_length != 32)) { PyErr_SetString(ipap11helperError, "generate_master_key: key length allowed values are: 16, 24 and 32"); - return NULL; + error = 1; + goto final; } - //TODO free label if check failed - //TODO is label freed inside???? dont we use freed value later r = _id_exists(self, id, id_length, CKO_SECRET_KEY); if (r == 1) { PyErr_SetString(ipap11helperDuplicationError, "Master key with same ID already exists"); - return NULL; + error = 1; + goto final; } else if (r == -1) { - return NULL; + error = 1; + goto final; } /* Process keyword boolean arguments */ @@ -719,9 +725,15 @@ P11_Helper_generate_master_key(P11_Helper* self, PyObject *args, PyObject *kwds) rv = self->p11->C_GenerateKey(self->session, &mechanism, symKeyTemplate, sizeof(symKeyTemplate) / sizeof(CK_ATTRIBUTE), &master_key); - if (!check_return_value(rv, "generate master key")) + if (!check_return_value(rv, "generate master key")){ + error = 1; + goto final; + } +final: + if (label != NULL) PyMem_Free(label); + if (error){ return NULL; - + } return Py_BuildValue("k", master_key); } @@ -740,6 +752,8 @@ P11_Helper_generate_replica_key_pair(P11_Helper* self, PyObject *args, int id_length = 0; PyObject* label_unicode = NULL; Py_ssize_t label_length = 0; + CK_BYTE *label = NULL; + int error = 0; PyObj2Bool_mapping_t attrs_pub[] = { { NULL, &true }, //pub_en_cka_copyable { NULL, &false }, //pub_en_cka_derive @@ -806,30 +820,33 @@ P11_Helper_generate_replica_key_pair(P11_Helper* self, PyObject *args, return NULL; } - CK_BYTE *label = unicode_to_char_array(label_unicode, &label_length); + label = unicode_to_char_array(label_unicode, &label_length); + if (label == NULL) return NULL; CK_OBJECT_HANDLE public_key, private_key; CK_MECHANISM mechanism = { CKM_RSA_PKCS_KEY_PAIR_GEN, NULL_PTR, 0 }; - //TODO free variables - r = _id_exists(self, id, id_length, CKO_PRIVATE_KEY); if (r == 1) { PyErr_SetString(ipap11helperDuplicationError, "Private key with same ID already exists"); - return NULL; + error = 1; + goto final; } else if (r == -1) { - return NULL; + error = 1; + goto final; } r = _id_exists(self, id, id_length, CKO_PUBLIC_KEY); if (r == 1) { PyErr_SetString(ipap11helperDuplicationError, "Public key with same ID already exists"); - return NULL; + error = 1; + goto final; } else if (r == -1) { - return NULL; + error = 1; + goto final; } /* Process keyword boolean arguments */ @@ -878,9 +895,16 @@ P11_Helper_generate_replica_key_pair(P11_Helper* self, PyObject *args, privateKeyTemplate, sizeof(privateKeyTemplate) / sizeof(CK_ATTRIBUTE), &public_key, &private_key); - if (!check_return_value(rv, "generate key pair")) - return NULL; + if (!check_return_value(rv, "generate key pair")){ + error = 1; + goto final; + } +final: + if (label != NULL) PyMem_Free(label); + if (error) { + return NULL; + } return Py_BuildValue("(kk)", public_key, private_key); } @@ -891,7 +915,7 @@ static PyObject * P11_Helper_find_keys(P11_Helper* self, PyObject *args, PyObject *kwds) { CK_OBJECT_CLASS class = CKO_VENDOR_DEFINED; CK_OBJECT_CLASS *class_ptr = &class; - CK_BYTE *id = NULL; //TODO free + CK_BYTE *id = NULL; CK_BBOOL *ckawrap = NULL; CK_BBOOL *ckaunwrap = NULL; int id_length = 0; @@ -902,12 +926,13 @@ P11_Helper_find_keys(P11_Helper* self, PyObject *args, PyObject *kwds) { CK_OBJECT_HANDLE *objects = NULL; unsigned int objects_len = 0; PyObject *result_list = NULL; - const char *uri_str = NULL; //TODO free? + const char *uri_str = NULL; P11KitUri *uri = NULL; - CK_BYTE *label = NULL; //TODO free + CK_BYTE *label = NULL; CK_ATTRIBUTE template_static[MAX_TEMPLATE_LEN]; CK_ATTRIBUTE_PTR template = template_static; CK_ULONG template_len = MAX_TEMPLATE_LEN; + int error = 0; static char *kwlist[] = { "objclass", "label", "id", "cka_wrap", "cka_unwrap", "uri", NULL }; @@ -946,25 +971,26 @@ P11_Helper_find_keys(P11_Helper* self, PyObject *args, PyObject *kwds) { _fill_template_from_parts(template, &template_len, id, id_length, label, label_length, class_ptr, ckawrap, ckaunwrap); else { - if (!_parse_uri(uri_str, &uri)) - return 0; + if (!_parse_uri(uri_str, &uri)) { + error = 1; + goto final; + } template = p11_kit_uri_get_attributes(uri, &template_len); /* Do not deallocate URI while you are using the template. * Template contains pointers to values inside URI! */ } - if (!_find_key(self, template, template_len, &objects, &objects_len)) - return NULL; - - if (uri != NULL) - p11_kit_uri_free(uri); + if (!_find_key(self, template, template_len, &objects, &objects_len)){ + error = 1; + goto final; + } result_list = PyList_New(objects_len); if (result_list == NULL) { PyErr_SetString(ipap11helperError, "Unable to create list with results"); - free(objects); - return NULL; + error = 1; + goto final; } for (int i = 0; i < objects_len; ++i) { @@ -972,13 +998,18 @@ P11_Helper_find_keys(P11_Helper* self, PyObject *args, PyObject *kwds) { == -1) { PyErr_SetString(ipap11helperError, "Unable to add to value to result list"); - free(objects); Py_DECREF(result_list); - return NULL; + error = 1; + goto final; } } - - free(objects); +final: + if (label != NULL) PyMem_Free(label); + if (objects != NULL) free(objects); + if (uri != NULL) p11_kit_uri_free(uri); + if (error){ + return NULL; + } return result_list; } @@ -1068,38 +1099,55 @@ P11_Helper_export_RSA_public_key(P11_Helper* self, CK_OBJECT_HANDLE object) { CK_BYTE_PTR exponent = NULL; CK_OBJECT_CLASS class = CKO_PUBLIC_KEY; CK_KEY_TYPE key_type = CKK_RSA; + int error = 0; CK_ATTRIBUTE obj_template[] = { { CKA_MODULUS, NULL_PTR, 0 }, { CKA_PUBLIC_EXPONENT, NULL_PTR, 0 }, { CKA_CLASS, &class, sizeof(class) }, { CKA_KEY_TYPE, &key_type, sizeof(key_type) } }; rv = self->p11->C_GetAttributeValue(self->session, object, obj_template, 4); - if (!check_return_value(rv, "get RSA public key values - prepare")) - return NULL; + if (!check_return_value(rv, "get RSA public key values - prepare")){ + error = 1; + goto final; + } /* Set proper size for attributes*/ - modulus = (CK_BYTE_PTR) malloc( + modulus = (CK_BYTE_PTR) PyMem_Malloc( obj_template[0].ulValueLen * sizeof(CK_BYTE)); + if (modulus == NULL){ + PyErr_NoMemory(); + error = 1; + goto final; + } obj_template[0].pValue = modulus; - exponent = (CK_BYTE_PTR) malloc( + exponent = (CK_BYTE_PTR) PyMem_Malloc( obj_template[1].ulValueLen * sizeof(CK_BYTE)); + if (exponent == NULL){ + PyErr_NoMemory(); + error = 1; + goto final; + } obj_template[1].pValue = exponent; rv = self->p11->C_GetAttributeValue(self->session, object, obj_template, 4); - if (!check_return_value(rv, "get RSA public key values")) - return NULL; + if (!check_return_value(rv, "get RSA public key values")){ + error = 1; + goto final; + } /* Check if the key is RSA public key */ if (class != CKO_PUBLIC_KEY) { PyErr_SetString(ipap11helperError, "export_RSA_public_key: required public key class"); - return NULL; + error = 1; + goto final; } if (key_type != CKK_RSA) { PyErr_SetString(ipap11helperError, "export_RSA_public_key: required RSA key type"); - return NULL; + error = 1; + goto final; } rsa = RSA_new(); @@ -1109,6 +1157,7 @@ P11_Helper_export_RSA_public_key(P11_Helper* self, CK_OBJECT_HANDLE object) { if (n == NULL) { PyErr_SetString(ipap11helperError, "export_RSA_public_key: internal error: unable to convert modulus"); + error = 1; goto final; } @@ -1117,6 +1166,7 @@ P11_Helper_export_RSA_public_key(P11_Helper* self, CK_OBJECT_HANDLE object) { if (e == NULL) { PyErr_SetString(ipap11helperError, "export_RSA_public_key: internal error: unable to convert exponent"); + error = 1; goto final; } @@ -1127,6 +1177,7 @@ P11_Helper_export_RSA_public_key(P11_Helper* self, CK_OBJECT_HANDLE object) { if (EVP_PKEY_set1_RSA(pkey, rsa) == 0) { PyErr_SetString(ipap11helperError, "export_RSA_public_key: internal error: EVP_PKEY_set1_RSA failed"); + error = 1; goto final; } @@ -1146,6 +1197,11 @@ P11_Helper_export_RSA_public_key(P11_Helper* self, CK_OBJECT_HANDLE object) { EVP_PKEY_free(pkey); if (pp != NULL) free(pp); + if (modulus != NULL) PyMem_Free(modulus); + if (exponent != NULL) PyMem_Free(exponent); + if (error){ + return NULL; + } return ret; } @@ -1211,36 +1267,49 @@ P11_Helper_import_RSA_public_key(P11_Helper* self, CK_UTF8CHAR *label, int modulus_len = 0; CK_BYTE_PTR exponent = NULL; int exponent_len = 0; + int error = 0; if (pkey->type != EVP_PKEY_RSA) { PyErr_SetString(ipap11helperError, "Required RSA public key"); - return NULL; + error = 1; + goto final; } rsa = EVP_PKEY_get1_RSA(pkey); if (rsa == NULL) { PyErr_SetString(ipap11helperError, "import_RSA_public_key: EVP_PKEY_get1_RSA error"); - return NULL; + error = 1; + goto final; } /* convert BIGNUM to binary array */ - modulus = (CK_BYTE_PTR) malloc(BN_num_bytes(rsa->n)); + modulus = (CK_BYTE_PTR) PyMem_Malloc(BN_num_bytes(rsa->n)); + if (modulus == NULL){ + PyErr_NoMemory(); + error = 1; + goto final; + } modulus_len = BN_bn2bin(rsa->n, (unsigned char *) modulus); if (modulus == NULL) { PyErr_SetString(ipap11helperError, "import_RSA_public_key: BN_bn2bin modulus error"); - //TODO free - return NULL; + error = 1; + goto final; } - exponent = (CK_BYTE_PTR) malloc(BN_num_bytes(rsa->e)); + exponent = (CK_BYTE_PTR) PyMem_Malloc(BN_num_bytes(rsa->e)); + if (exponent == NULL){ + PyErr_NoMemory(); + error = 1; + goto final; + } exponent_len = BN_bn2bin(rsa->e, (unsigned char *) exponent); if (exponent == NULL) { PyErr_SetString(ipap11helperError, "import_RSA_public_key: BN_bn2bin exponent error"); - //TODO free - return NULL; + error = 1; + goto final; } CK_ATTRIBUTE template[] = { @@ -1267,9 +1336,13 @@ P11_Helper_import_RSA_public_key(P11_Helper* self, CK_UTF8CHAR *label, if (!check_return_value(rv, "create public key object")) return NULL; - if (rsa != NULL) - RSA_free(rsa); - +final: + if (rsa != NULL) RSA_free(rsa); + if (modulus != NULL) PyMem_Free(modulus); + if (exponent != NULL) PyMem_Free(exponent); + if (error){ + return NULL; + } return Py_BuildValue("k", object); } @@ -1289,6 +1362,7 @@ P11_Helper_import_public_key(P11_Helper* self, PyObject *args, PyObject *kwds) { Py_ssize_t data_length = 0; Py_ssize_t label_length = 0; EVP_PKEY *pkey = NULL; + int error = 0; PyObj2Bool_mapping_t attrs_pub[] = { { NULL, &true }, //pub_en_cka_copyable { NULL, &false }, //pub_en_cka_derive @@ -1328,9 +1402,11 @@ P11_Helper_import_public_key(P11_Helper* self, PyObject *args, PyObject *kwds) { if (r == 1) { PyErr_SetString(ipap11helperDuplicationError, "Public key with same ID already exists"); - return NULL; + error = 1; + goto final; } else if (r == -1) { - return NULL; + error = 1; + goto final; } /* Process keyword boolean arguments */ @@ -1342,7 +1418,8 @@ P11_Helper_import_public_key(P11_Helper* self, PyObject *args, PyObject *kwds) { if (pkey == NULL) { PyErr_SetString(ipap11helperError, "import_public_key: d2i_PUBKEY error"); - return NULL; + error = 1; + goto final; } switch (pkey->type) { case EVP_PKEY_RSA: @@ -1358,19 +1435,24 @@ P11_Helper_import_public_key(P11_Helper* self, PyObject *args, PyObject *kwds) { attrs_pub[pub_en_cka_wrap].bool); break; case EVP_PKEY_DSA: - ret = NULL; + error = 1; PyErr_SetString(ipap11helperError, "DSA is not supported"); break; case EVP_PKEY_EC: - ret = NULL; + error = 1; PyErr_SetString(ipap11helperError, "EC is not supported"); break; default: - ret = NULL; + error = 1; PyErr_SetString(ipap11helperError, "Unsupported key type"); } +final: if (pkey != NULL) EVP_PKEY_free(pkey); + if (label != NULL) PyMem_Free(label); + if (error){ + return NULL; + } return ret; } @@ -1387,6 +1469,8 @@ P11_Helper_export_wrapped_key(P11_Helper* self, PyObject *args, PyObject *kwds) CK_ULONG wrapped_key_len = 0; CK_MECHANISM wrapping_mech = { CKM_RSA_PKCS, NULL, 0 }; /* currently we don't support parameter in mechanism */ + PyObject *result = NULL; + int error = 0; static char *kwlist[] = { "key", "wrapping_key", "wrapping_mech", NULL }; //TODO check long overflow @@ -1419,20 +1503,37 @@ P11_Helper_export_wrapped_key(P11_Helper* self, PyObject *args, PyObject *kwds) rv = self->p11->C_WrapKey(self->session, &wrapping_mech, object_wrapping_key, object_key, NULL, &wrapped_key_len); - if (!check_return_value(rv, "key wrapping: get buffer length")) - return 0; - wrapped_key = malloc(wrapped_key_len); + if (!check_return_value(rv, "key wrapping: get buffer length")){ + error = 1; + goto final; + } + wrapped_key = PyMem_Malloc(wrapped_key_len); + if (wrapped_key == NULL){ + PyErr_NoMemory(); + error = 1; + goto final; + } if (wrapped_key == NULL) { rv = CKR_HOST_MEMORY; - if (!check_return_value(rv, "key wrapping: buffer allocation")) - return 0; + if (!check_return_value(rv, "key wrapping: buffer allocation")){ + error = 1; + goto final; + } } rv = self->p11->C_WrapKey(self->session, &wrapping_mech, object_wrapping_key, object_key, wrapped_key, &wrapped_key_len); - if (!check_return_value(rv, "key wrapping: wrapping")) - return NULL; + if (!check_return_value(rv, "key wrapping: wrapping")){ + error = 1; + goto final; + } + result = Py_BuildValue("s#", wrapped_key, wrapped_key_len); - return Py_BuildValue("s#", wrapped_key, wrapped_key_len); +final: + if (wrapped_key != NULL) PyMem_Free(wrapped_key); + if (error){ + return NULL; + } + return result; } @@ -1457,6 +1558,7 @@ P11_Helper_import_wrapped_secret_key(P11_Helper* self, PyObject *args, CK_MECHANISM wrapping_mech = { CKM_RSA_PKCS, NULL, 0 }; CK_OBJECT_CLASS key_class = CKO_SECRET_KEY; CK_KEY_TYPE key_type = CKK_RSA; + int error = 0; PyObj2Bool_mapping_t attrs[] = { { NULL, &true }, //sec_en_cka_copyable { NULL, &false }, //sec_en_cka_decrypt @@ -1519,15 +1621,17 @@ P11_Helper_import_wrapped_secret_key(P11_Helper* self, PyObject *args, } label = (unsigned char*) unicode_to_char_array(label_unicode, - &label_length); //TODO verify signed/unsigned + &label_length); r = _id_exists(self, id, id_length, key_class); if (r == 1) { PyErr_SetString(ipap11helperDuplicationError, "Secret key with same ID already exists"); - return NULL; + error = 1; + goto final; } else if (r == -1) { - return NULL; + error = 1; + goto final; } /* Process keyword boolean arguments */ @@ -1558,9 +1662,14 @@ P11_Helper_import_wrapped_secret_key(P11_Helper* self, PyObject *args, unwrapping_key_object, wrapped_key, wrapped_key_len, template, sizeof(template) / sizeof(CK_ATTRIBUTE), &unwrapped_key_object); if (!check_return_value(rv, "import_wrapped_key: key unwrapping")) { + error = 1; + goto final; + } +final: + if (label != NULL) PyMem_Free(label); + if (error) { return NULL; } - return Py_BuildValue("k", unwrapped_key_object); } @@ -1586,6 +1695,7 @@ P11_Helper_import_wrapped_private_key(P11_Helper* self, PyObject *args, CK_MECHANISM wrapping_mech = { CKM_RSA_PKCS, NULL, 0 }; CK_OBJECT_CLASS key_class = CKO_PRIVATE_KEY; CK_KEY_TYPE key_type = CKK_RSA; + int error = 0; PyObj2Bool_mapping_t attrs_priv[] = { { NULL, &false }, //priv_en_cka_always_authenticate { NULL, &true }, //priv_en_cka_copyable @@ -1629,15 +1739,17 @@ P11_Helper_import_wrapped_private_key(P11_Helper* self, PyObject *args, } label = (unsigned char*) unicode_to_char_array(label_unicode, - &label_length); //TODO verify signed/unsigned + &label_length); r = _id_exists(self, id, id_length, CKO_SECRET_KEY); if (r == 1) { PyErr_SetString(ipap11helperDuplicationError, "Secret key with same ID already exists"); - return NULL; + error = 1; + goto final; } else if (r == -1) { - return NULL; + error = 1; + goto final; } /* Process keyword boolean arguments */ @@ -1668,9 +1780,14 @@ P11_Helper_import_wrapped_private_key(P11_Helper* self, PyObject *args, unwrapping_key_object, wrapped_key, wrapped_key_len, template, sizeof(template) / sizeof(CK_ATTRIBUTE), &unwrapped_key_object); if (!check_return_value(rv, "import_wrapped_key: key unwrapping")) { + error = 1; + goto final; + } +final: + if (label != NULL) PyMem_Free(label); + if (error){ return NULL; } - return PyLong_FromUnsignedLong(unwrapped_key_object); } @@ -1687,6 +1804,7 @@ P11_Helper_set_attribute(P11_Helper* self, PyObject *args, PyObject *kwds) { CK_ATTRIBUTE attribute; CK_RV rv; Py_ssize_t len = 0; + CK_UTF8CHAR *label = NULL; static char *kwlist[] = { "key_object", "attr", "value", NULL }; if (!PyArg_ParseTupleAndKeywords(args, kwds, "kkO|", kwlist, &object, &attr, @@ -1739,7 +1857,8 @@ P11_Helper_set_attribute(P11_Helper* self, PyObject *args, PyObject *kwds) { ret = NULL; goto final; } - attribute.pValue = unicode_to_char_array(value, &len); + label = unicode_to_char_array(value, &len); + attribute.pValue = label; /* check for conversion error */ if (attribute.pValue == NULL) { ret = NULL; @@ -1769,6 +1888,7 @@ P11_Helper_set_attribute(P11_Helper* self, PyObject *args, PyObject *kwds) { if (!check_return_value(rv, "set_attribute")) ret = NULL; final: + if (label != NULL) PyMem_Free(label); Py_XINCREF(ret); return ret; } @@ -1784,6 +1904,7 @@ P11_Helper_get_attribute(P11_Helper* self, PyObject *args, PyObject *kwds) { unsigned long attr = 0; CK_ATTRIBUTE attribute; CK_RV rv; + int error = 0; static char *kwlist[] = { "key_object", "attr", NULL }; if (!PyArg_ParseTupleAndKeywords(args, kwds, "kk|", kwlist, &object, @@ -1801,19 +1922,24 @@ P11_Helper_get_attribute(P11_Helper* self, PyObject *args, PyObject *kwds) { if (rv == CKR_ATTRIBUTE_TYPE_INVALID || template[0].ulValueLen == (unsigned long) -1) { PyErr_SetString(ipap11helperNotFound, "attribute does not exist"); - ret = NULL; + error = 1; goto final; } if (!check_return_value(rv, "get_attribute init")) { - ret = NULL; + error = 1; + goto final; + } + value = PyMem_Malloc(template[0].ulValueLen); + if (value == NULL){ + PyErr_NoMemory(); + error = 1; goto final; } - value = malloc(template[0].ulValueLen); template[0].pValue = value; rv = self->p11->C_GetAttributeValue(self->session, object, template, 1); if (!check_return_value(rv, "get_attribute")) { - ret = NULL; + error = 1; goto final; } @@ -1857,13 +1983,16 @@ P11_Helper_get_attribute(P11_Helper* self, PyObject *args, PyObject *kwds) { ret = Py_BuildValue("k", *(unsigned long *) value); break; default: - ret = NULL; + error = 1; PyErr_SetString(ipap11helperError, "Unknown attribute"); goto final; } final: if (value != NULL) - free(value); + PyMem_Free(value); + if (error){ + return NULL; + } return ret; } -- 2.1.0
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
