Skip to content

create_principal extension + don't leak memory when raising errors #21

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.

Sign up for GitHub

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

Closed
denys-duchier wants to merge 2 commits into from
Closed

create_principal extension + don't leak memory when raising errors #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f8c417d
add support for database options in create_principal
denys-duchier Oct 23, 2014
f3990be
changed PyKAdmin_RETURN_ERROR to not leak memory
denys-duchier Oct 23, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion PyKAdminErrors.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

#include "pykadmin.h"

#define PyKAdmin_RETURN_ERROR(value, caller) { PyKAdminError_raise_error((long)value, caller); return NULL; }
#define PyKAdmin_RETURN_ERROR(value, caller) { PyKAdminError_raise_error((long)value, caller); has_error=1; goto epilog; }

//#define PyKAdmin_RETURN_KADM5_ERROR(retval, caller) { PyKAdminError_raise_kadm_error(retval, caller); return NULL; }
//#define PyKAdmin_RETURN_KRB5_ERROR(code, caller) { PyKAdminError_raise_krb5_error(code, caller); return NULL; }
Expand Down
8 changes: 6 additions & 2 deletions PyKAdminIterator.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ PyKAdminIterator *PyKAdminIterator_principal_iterator(PyKAdminObject *kadmin, ch

kadm5_ret_t retval = KADM5_OK;
PyKAdminIterator *iter = PyObject_New(PyKAdminIterator, &PyKAdminIterator_Type);
int has_error = 0;

if (iter) {

Expand All @@ -94,7 +95,8 @@ PyKAdminIterator *PyKAdminIterator_principal_iterator(PyKAdminObject *kadmin, ch
retval = kadm5_get_principals(kadmin->server_handle, match, &iter->names, &iter->count);
if (retval != KADM5_OK) { PyKAdmin_RETURN_ERROR(retval, "kadm5_get_principals"); }
}

epilog:
if (has_error) return NULL;
Py_XINCREF(iter);
return iter;
}
Expand All @@ -104,6 +106,7 @@ PyKAdminIterator *PyKAdminIterator_policy_iterator(PyKAdminObject *kadmin, char

kadm5_ret_t retval = KADM5_OK;
PyKAdminIterator *iter = PyObject_New(PyKAdminIterator, &PyKAdminIterator_Type);
int has_error = 0;

if (iter) {

Expand All @@ -116,7 +119,8 @@ PyKAdminIterator *PyKAdminIterator_policy_iterator(PyKAdminObject *kadmin, char
retval = kadm5_get_policies(kadmin->server_handle, match, &iter->names, &iter->count);
if (retval != KADM5_OK) { PyKAdmin_RETURN_ERROR(retval, "kadm5_get_policies"); }
}

epilog:
if (has_error) return NULL;
Py_XINCREF(iter);
return iter;
}
Expand Down
140 changes: 120 additions & 20 deletions PyKAdminObject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ static PyObject *PyKAdminObject_new(PyTypeObject *type, PyObject *args, PyObject
PyKAdminObject *self = NULL;
kadm5_ret_t retval = KADM5_OK;
krb5_error_code code = 0;
int has_error = 0;

self = (PyKAdminObject *)type->tp_alloc(type, 0);

Expand All @@ -55,9 +56,9 @@ static PyObject *PyKAdminObject_new(PyTypeObject *type, PyObject *args, PyObject

self->_storage = PyDict_New();
}

epilog:
if (has_error) return NULL;
return (PyObject *)self;

}

static int PyKAdminObject_init(PyKAdminObject *self, PyObject *args, PyObject *kwds) {
Expand All @@ -75,6 +76,9 @@ static PyObject *PyKAdminObject_principal_exists(PyKAdminObject *self, PyObject
PyObject *result = NULL;

kadm5_principal_ent_rec entry;
int has_error = 0;

memset(&entry, 0, sizeof(entry));

if (!PyArg_ParseTuple(args, "s", &client_name))
return NULL;
Expand All @@ -89,10 +93,10 @@ static PyObject *PyKAdminObject_principal_exists(PyKAdminObject *self, PyObject
else if (retval == KADM5_UNK_PRINC) { result = Py_False; }
else { PyKAdmin_RETURN_ERROR(retval, "kadm5_delete_principal"); }
}

krb5_free_principal(self->context, princ);
epilog:
if (princ) krb5_free_principal(self->context, princ);
kadm5_free_principal_ent(self->server_handle, &entry);

if (has_error) return NULL;
Py_XINCREF(result);
return result;

Expand All @@ -104,6 +108,7 @@ static PyObject *PyKAdminObject_delete_principal(PyKAdminObject *self, PyObject
kadm5_ret_t retval = KADM5_OK;
krb5_error_code code = 0;
krb5_principal princ = NULL;
int has_error = 0;

char *client_name = NULL;

Expand All @@ -116,23 +121,111 @@ static PyObject *PyKAdminObject_delete_principal(PyKAdminObject *self, PyObject
if (code) { PyKAdmin_RETURN_ERROR(retval, "krb5_parse_name"); }

retval = kadm5_delete_principal(self->server_handle, princ);
if (retval != KADM5_OK) { PyKAdmin_RETURN_ERROR(retval, "kadm5_delete_principal"); }

if (retval != KADM5_OK) { PyKAdmin_RETURN_ERROR(retval, "kadm5_create_principal"); }
}
epilog:
krb5_free_principal(self->context, princ);

if (has_error) return NULL;
Py_RETURN_TRUE;
}

/******************************************************************************
add_tl_data
free_tl_data
these functions come from the KRB5 code base
******************************************************************************/

static void
free_tl_data(krb5_int16 *n_tl_datap, krb5_tl_data **tl_datap)
{
krb5_tl_data *tl_data = *tl_datap, *next;
int n_tl_data = *n_tl_datap;
int i;

*n_tl_datap = 0;
*tl_datap = NULL;

for (i = 0; tl_data && (i < n_tl_data); i++) {
next = tl_data->tl_data_next;
free(tl_data->tl_data_contents);
free(tl_data);
tl_data = next;
}
}

/* Construct a tl_data element and add it to the tail of *tl_datap. */
static int
add_tl_data(krb5_int16 *n_tl_datap, krb5_tl_data **tl_datap,
krb5_int16 tl_type, krb5_ui_2 len, krb5_octet *contents)
{
krb5_tl_data *tl_data;
krb5_octet *copy;

copy = malloc(len);
tl_data = calloc(1, sizeof(*tl_data));
if (copy == NULL || tl_data == NULL) {
PyErr_SetString(PyExc_RuntimeError, "Not enough memory");
return 1;
}
memcpy(copy, contents, len);

tl_data->tl_data_type = tl_type;
tl_data->tl_data_length = len;
tl_data->tl_data_contents = copy;
tl_data->tl_data_next = NULL;

for (; *tl_datap != NULL; tl_datap = &(*tl_datap)->tl_data_next);
*tl_datap = tl_data;
(*n_tl_datap)++;
return 0;
}

static int
process_principal_x_arg(kadm5_principal_ent_rec *entry, PyObject *arg, long *mask)
{
if (PyUnicode_Check(arg)) {
const char* data = PyUnicode_AS_DATA(arg);
add_tl_data(&entry->n_tl_data, &entry->tl_data,
KRB5_TL_DB_ARGS, strlen(data)+1,
(krb5_octet*) data);
*mask |= KADM5_TL_DATA;
}
else if (PyString_Check(arg)) {
char* data = PyString_AS_STRING(arg);
add_tl_data(&entry->n_tl_data, &entry->tl_data,
KRB5_TL_DB_ARGS, strlen(data)+1,
(krb5_octet*) data);
*mask |= KADM5_TL_DATA;
}
else if (PySequence_Check(arg)) {
int n = PySequence_Length(arg);
int i;
for (i=0; i<n; ++i) {
PyObject* e = PySequence_GetItem(arg, i);
process_principal_x_arg(entry, e, mask);
}
}
return 0;
}

static int
process_principal_kwargs(kadm5_principal_ent_rec *entry, PyObject *kwargs, long* mask) {
if (! PyMapping_Check(kwargs)) return 0;
if (PyMapping_HasKeyString(kwargs, "x")) {
PyObject* val = PyMapping_GetItemString(kwargs, "x");
process_principal_x_arg(entry, val, mask);
}
return 0;
}

static PyObject *PyKAdminObject_create_principal(PyKAdminObject *self, PyObject *args, PyObject *kwds) {

kadm5_ret_t retval = KADM5_OK;
krb5_error_code code = 0;
char *princ_name = NULL;
char *princ_pass = NULL;
long mask = KADM5_PRINCIPAL;
int has_error = 0;

kadm5_principal_ent_rec entry;

Expand All @@ -144,18 +237,21 @@ static PyObject *PyKAdminObject_create_principal(PyKAdminObject *self, PyObject
if (!PyArg_ParseTuple(args, "s|z", &princ_name, &princ_pass))
return NULL;

process_principal_kwargs(&entry, kwds, &mask);

if (self->server_handle) {

code = krb5_parse_name(self->context, princ_name, &entry.principal);
if (code) { PyKAdmin_RETURN_ERROR(retval, "krb5_parse_name"); }

retval = kadm5_create_principal(self->server_handle, &entry, KADM5_PRINCIPAL, princ_pass);
retval = kadm5_create_principal(self->server_handle, &entry, mask, princ_pass);
if (retval != KADM5_OK) { PyKAdmin_RETURN_ERROR(retval, "kadm5_create_principal"); }

}

epilog:
kadm5_free_principal_ent(self->server_handle, &entry);

free_tl_data(&entry.n_tl_data, &entry.tl_data);
if (has_error) return NULL;
Py_RETURN_TRUE;
}

Expand Down Expand Up @@ -280,7 +376,8 @@ static PyObject *PyKAdminObject_each_principal(PyKAdminObject *self, PyObject *a

char *match = NULL;
krb5_error_code code = 0;
kadm5_ret_t lock = KADM5_OK;
kadm5_ret_t lock = KADM5_OK;
int has_error = 0;

static char *kwlist[] = {"callback", "data", "match", NULL};

Expand All @@ -301,7 +398,7 @@ static PyObject *PyKAdminObject_each_principal(PyKAdminObject *self, PyObject *a

krb5_clear_error_message(self->context);

code = krb5_db_iterate(self->context, match, kdb_iter_princs, (void *)self);
code = krb5_db_iterate(self->context, match, kdb_iter_princs, (void *)self, 0);

if (lock != KRB5_PLUGIN_OP_NOTSUPP)
lock = kadm5_unlock(self->server_handle);
Expand All @@ -317,7 +414,8 @@ static PyObject *PyKAdminObject_each_principal(PyKAdminObject *self, PyObject *a
return NULL;
}


epilog:
if (has_error) return NULL;
Py_RETURN_TRUE;

}
Expand Down Expand Up @@ -353,7 +451,8 @@ static PyObject *PyKAdminObject_each_policy(PyKAdminObject *self, PyObject *args

char *match = NULL;
krb5_error_code code = 0;
kadm5_ret_t lock = KADM5_OK;
kadm5_ret_t lock = KADM5_OK;
int has_error = 0;

static char *kwlist[] = {"", "data", "match", NULL};

Expand Down Expand Up @@ -387,17 +486,18 @@ static PyObject *PyKAdminObject_each_policy(PyKAdminObject *self, PyObject *args
_pykadmin_each_restore_error(self->each_policy.error);
return NULL;
}

epilog:
if (has_error) return NULL;
Py_RETURN_TRUE;

}
#endif

static PyMethodDef PyKAdminObject_methods[] = {

{"ank", (PyCFunction)PyKAdminObject_create_principal, METH_VARARGS, ""},
{"addprinc", (PyCFunction)PyKAdminObject_create_principal, METH_VARARGS, ""},
{"add_principal", (PyCFunction)PyKAdminObject_create_principal, METH_VARARGS, ""},
{"ank", (PyCFunction)PyKAdminObject_create_principal, (METH_VARARGS | METH_KEYWORDS), ""},
{"addprinc", (PyCFunction)PyKAdminObject_create_principal, (METH_VARARGS | METH_KEYWORDS), ""},
{"add_principal", (PyCFunction)PyKAdminObject_create_principal, (METH_VARARGS | METH_KEYWORDS), ""},

{"delprinc", (PyCFunction)PyKAdminObject_delete_principal, METH_VARARGS, ""},
{"delete_principal", (PyCFunction)PyKAdminObject_delete_principal, METH_VARARGS, ""},
Expand Down
Loading