From 50ebc1491ac686fcdd5022401c784af72e65e06b Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Wed, 20 Jun 2018 23:47:16 -0400 Subject: [PATCH] lib/kadm5: improve kadm_c_ error handling Perform error checking for each function call and consistently return errors at the point of failure. Refactor functions to use a common exit path. Preserve error messages stored in the kadm5_client_context.context when appropriate. Change-Id: I7aa04020e4de3454066f0d88ba805fed999dbd1a --- lib/kadm5/chpass_c.c | 109 +++++++++++++++++++++++++-------------- lib/kadm5/create_c.c | 59 +++++++++++++-------- lib/kadm5/delete_c.c | 49 +++++++++++------- lib/kadm5/get_c.c | 58 ++++++++++++--------- lib/kadm5/get_princs_c.c | 87 ++++++++++++++++++++----------- lib/kadm5/modify_c.c | 56 ++++++++++++-------- lib/kadm5/privs_c.c | 39 ++++++++------ lib/kadm5/randkey_c.c | 87 +++++++++++++++++-------------- lib/kadm5/rename_c.c | 57 +++++++++++++------- 9 files changed, 370 insertions(+), 231 deletions(-) diff --git a/lib/kadm5/chpass_c.c b/lib/kadm5/chpass_c.c index 4512942b4..309e37236 100644 --- a/lib/kadm5/chpass_c.c +++ b/lib/kadm5/chpass_c.c @@ -59,36 +59,49 @@ kadm5_c_chpass_principal(void *server_handle, return KADM5_KS_TUPLE_NOSUPP; ret = _kadm5_connect(server_handle); - if(ret) + if (ret) return ret; + krb5_data_zero(&reply); + sp = krb5_storage_from_mem(buf, sizeof(buf)); if (sp == NULL) { - krb5_clear_error_message(context->context); - return ENOMEM; + ret = ENOMEM; + goto out; } - krb5_store_int32(sp, kadm_chpass); - krb5_store_principal(sp, princ); - krb5_store_string(sp, password); - krb5_store_int32(sp, keepold); /* extension */ - ret = _kadm5_client_send(context, sp); - krb5_storage_free(sp); + ret = krb5_store_int32(sp, kadm_chpass); if (ret) - return ret; + goto out; + ret = krb5_store_principal(sp, princ); + if (ret) + goto out; + ret = krb5_store_string(sp, password); + if (ret) + goto out; + ret = krb5_store_int32(sp, keepold); /* extension */ + if (ret) + goto out; + ret = _kadm5_client_send(context, sp); + if (ret) + goto out_keep_error; ret = _kadm5_client_recv(context, &reply); - if(ret) - return ret; - sp = krb5_storage_from_data (&reply); + if (ret) + goto out_keep_error; + krb5_storage_free(sp); + sp = krb5_storage_from_data(&reply); if (sp == NULL) { - krb5_clear_error_message(context->context); - krb5_data_free (&reply); - return ENOMEM; + ret = ENOMEM; + goto out; } - krb5_ret_int32(sp, &tmp); + ret = krb5_ret_int32(sp, &tmp); + if (ret == 0) + ret = tmp; + + out: krb5_clear_error_message(context->context); krb5_storage_free(sp); - krb5_data_free (&reply); - return tmp; + krb5_data_free(&reply); + return ret; } kadm5_ret_t @@ -107,36 +120,54 @@ kadm5_c_chpass_principal_with_key(void *server_handle, int i; ret = _kadm5_connect(server_handle); - if(ret) + if (ret) return ret; + krb5_data_zero(&reply); + sp = krb5_storage_from_mem(buf, sizeof(buf)); if (sp == NULL) { - krb5_clear_error_message(context->context); - return ENOMEM; + ret = ENOMEM; + goto out; } - krb5_store_int32(sp, kadm_chpass_with_key); - krb5_store_principal(sp, princ); - krb5_store_int32(sp, n_key_data); - for (i = 0; i < n_key_data; ++i) - kadm5_store_key_data (sp, &key_data[i]); - krb5_store_int32(sp, keepold); /* extension */ - ret = _kadm5_client_send(context, sp); - krb5_storage_free(sp); + ret = krb5_store_int32(sp, kadm_chpass_with_key); if (ret) - return ret; + goto out; + ret = krb5_store_principal(sp, princ); + if (ret) + goto out; + ret = krb5_store_int32(sp, n_key_data); + if (ret) + goto out; + for (i = 0; i < n_key_data; ++i) { + ret = kadm5_store_key_data (sp, &key_data[i]); + if (ret) + goto out; + } + ret = krb5_store_int32(sp, keepold); /* extension */ + if (ret) + goto out; + ret = _kadm5_client_send(context, sp); + if (ret) + goto out_keep_error; ret = _kadm5_client_recv(context, &reply); - if(ret) - return ret; + if (ret) + goto out_keep_error; + krb5_storage_free(sp); sp = krb5_storage_from_data (&reply); if (sp == NULL) { - krb5_clear_error_message(context->context); - krb5_data_free (&reply); - return ENOMEM; + ret = ENOMEM; + goto out; } - krb5_ret_int32(sp, &tmp); + ret = krb5_ret_int32(sp, &tmp); + if (ret == 0) + ret = tmp; + + out: krb5_clear_error_message(context->context); + + out_keep_error: krb5_storage_free(sp); - krb5_data_free (&reply); - return tmp; + krb5_data_free(&reply); + return ret; } diff --git a/lib/kadm5/create_c.c b/lib/kadm5/create_c.c index f6706b027..63fff3e50 100644 --- a/lib/kadm5/create_c.c +++ b/lib/kadm5/create_c.c @@ -59,35 +59,50 @@ kadm5_c_create_principal(void *server_handle, return KADM5_KS_TUPLE_NOSUPP; ret = _kadm5_connect(server_handle); - if(ret) + if (ret) return ret; + krb5_data_zero(&reply); + sp = krb5_storage_from_mem(buf, sizeof(buf)); if (sp == NULL) { - krb5_clear_error_message(context->context); - return ENOMEM; + ret = ENOMEM; + goto out; } - krb5_store_int32(sp, kadm_create); - kadm5_store_principal_ent(sp, princ); - krb5_store_int32(sp, mask); - krb5_store_string(sp, password); - ret = _kadm5_client_send(context, sp); - krb5_storage_free(sp); + ret = krb5_store_int32(sp, kadm_create); if (ret) - return ret; + goto out; + ret = kadm5_store_principal_ent(sp, princ); + if (ret) + goto out; + ret = krb5_store_int32(sp, mask); + if (ret) + goto out; + ret = krb5_store_string(sp, password); + if (ret) + goto out; + ret = _kadm5_client_send(context, sp); + if (ret) + goto out_keep_error; ret = _kadm5_client_recv(context, &reply); - if(ret) - return ret; - sp = krb5_storage_from_data (&reply); - if (sp == NULL) { - krb5_clear_error_message(context->context); - krb5_data_free (&reply); - return ENOMEM; - } - krb5_ret_int32(sp, &tmp); - krb5_clear_error_message(context->context); + if (ret) + goto out_keep_error; krb5_storage_free(sp); - krb5_data_free (&reply); - return tmp; + sp = krb5_storage_from_data(&reply); + if (sp == NULL) { + ret = ENOMEM; + goto out; + } + ret = krb5_ret_int32(sp, &tmp); + if (ret == 0) + ret = tmp; + + out: + krb5_clear_error_message(context->context); + + out_keep_error: + krb5_storage_free(sp); + krb5_data_free(&reply); + return ret; } diff --git a/lib/kadm5/delete_c.c b/lib/kadm5/delete_c.c index 2c4ed7749..1fd43d778 100644 --- a/lib/kadm5/delete_c.c +++ b/lib/kadm5/delete_c.c @@ -46,32 +46,43 @@ kadm5_c_delete_principal(void *server_handle, krb5_principal princ) krb5_data reply; ret = _kadm5_connect(server_handle); - if(ret) + if (ret) return ret; + krb5_data_zero(&reply); + sp = krb5_storage_from_mem(buf, sizeof(buf)); if (sp == NULL) { - krb5_clear_error_message(context->context); - return ENOMEM; + ret = ENOMEM; + goto out; } - krb5_store_int32(sp, kadm_delete); - krb5_store_principal(sp, princ); - ret = _kadm5_client_send(context, sp); - krb5_storage_free(sp); + ret = krb5_store_int32(sp, kadm_delete); if (ret) - return ret; + goto out; + ret = krb5_store_principal(sp, princ); + if (ret) + goto out; + ret = _kadm5_client_send(context, sp); + if (ret) + goto out_keep_error; ret = _kadm5_client_recv(context, &reply); if (ret) - return ret; - sp = krb5_storage_from_data (&reply); - if(sp == NULL) { - krb5_clear_error_message(context->context); - krb5_data_free (&reply); - return ENOMEM; - } - krb5_ret_int32(sp, &tmp); - krb5_clear_error_message(context->context); + goto out_keep_error; krb5_storage_free(sp); - krb5_data_free (&reply); - return tmp; + sp = krb5_storage_from_data(&reply); + if (sp == NULL) { + ret = ENOMEM; + goto out; + } + ret = krb5_ret_int32(sp, &tmp); + if (ret == 0) + ret = tmp; + + out: + krb5_clear_error_message(context->context); + + out_keep_error: + krb5_storage_free(sp); + krb5_data_free(&reply); + return ret; } diff --git a/lib/kadm5/get_c.c b/lib/kadm5/get_c.c index 3c31a515b..25a92c4b7 100644 --- a/lib/kadm5/get_c.c +++ b/lib/kadm5/get_c.c @@ -38,7 +38,7 @@ RCSID("$Id$"); kadm5_ret_t kadm5_c_get_principal(void *server_handle, krb5_principal princ, - kadm5_principal_ent_t out, + kadm5_principal_ent_t ent, uint32_t mask) { kadm5_client_context *context = server_handle; @@ -49,36 +49,48 @@ kadm5_c_get_principal(void *server_handle, krb5_data reply; ret = _kadm5_connect(server_handle); - if(ret) + if (ret) return ret; + krb5_data_zero(&reply); + sp = krb5_storage_from_mem(buf, sizeof(buf)); if (sp == NULL) { - krb5_clear_error_message(context->context); - return ENOMEM; + ret = ENOMEM; + goto out; } - krb5_store_int32(sp, kadm_get); - krb5_store_principal(sp, princ); - krb5_store_int32(sp, mask); + ret = krb5_store_int32(sp, kadm_get); + if (ret) + goto out; + ret = krb5_store_principal(sp, princ); + if (ret) + goto out; + ret = krb5_store_int32(sp, mask); + if (ret) + goto out; ret = _kadm5_client_send(context, sp); - krb5_storage_free(sp); - if(ret) - return ret; + if (ret) + goto out_keep_error; ret = _kadm5_client_recv(context, &reply); if (ret) - return ret; - sp = krb5_storage_from_data (&reply); - if (sp == NULL) { - krb5_clear_error_message(context->context); - krb5_data_free (&reply); - return ENOMEM; - } - krb5_ret_int32(sp, &tmp); - ret = tmp; - krb5_clear_error_message(context->context); - if(ret == 0) - kadm5_ret_principal_ent(sp, out); + goto out_keep_error; krb5_storage_free(sp); - krb5_data_free (&reply); + sp = krb5_storage_from_data(&reply); + if (sp == NULL) { + ret = ENOMEM; + goto out; + } + ret = krb5_ret_int32(sp, &tmp); + if (ret == 0) + ret = tmp; + + out: + krb5_clear_error_message(context->context); + + out_keep_error: + if (ret == 0) + kadm5_ret_principal_ent(sp, ent); + krb5_storage_free(sp); + krb5_data_free(&reply); return ret; } diff --git a/lib/kadm5/get_princs_c.c b/lib/kadm5/get_princs_c.c index d5e3461d8..db44cff4c 100644 --- a/lib/kadm5/get_princs_c.c +++ b/lib/kadm5/get_princs_c.c @@ -47,46 +47,73 @@ kadm5_c_get_principals(void *server_handle, unsigned char buf[1024]; int32_t tmp; krb5_data reply; + int i; + + *count = 0; + *princs = NULL; ret = _kadm5_connect(server_handle); - if(ret) - return ret; - - sp = krb5_storage_from_mem(buf, sizeof(buf)); - if (sp == NULL) - return ENOMEM; - krb5_store_int32(sp, kadm_get_princs); - krb5_store_int32(sp, expression != NULL); - if(expression) - krb5_store_string(sp, expression); - ret = _kadm5_client_send(context, sp); - krb5_storage_free(sp); if (ret) return ret; + + krb5_data_zero(&reply); + + sp = krb5_storage_from_mem(buf, sizeof(buf)); + if (sp == NULL) { + ret = ENOMEM; + goto out; + } + ret = krb5_store_int32(sp, kadm_get_princs); + if (ret) + goto out; + ret = krb5_store_int32(sp, expression != NULL); + if (ret) + goto out; + if (expression) { + ret = krb5_store_string(sp, expression); + if (ret) + goto out; + } + ret = _kadm5_client_send(context, sp); + if (ret) + goto out_keep_error; ret = _kadm5_client_recv(context, &reply); - if(ret) - return ret; + if (ret) + goto out_keep_error; + krb5_storage_free(sp); sp = krb5_storage_from_data (&reply); if (sp == NULL) { - krb5_data_free (&reply); - return ENOMEM; + ret = ENOMEM; + goto out; } - krb5_ret_int32(sp, &tmp); - ret = tmp; - if(ret == 0) { - int i; - krb5_ret_int32(sp, &tmp); - *princs = calloc(tmp + 1, sizeof(**princs)); - if (*princs == NULL) { - ret = ENOMEM; + ret = krb5_ret_int32(sp, &tmp); + if (ret == 0) + ret = tmp; + + if (ret) + goto out; + + ret = krb5_ret_int32(sp, &tmp); + if (ret) + goto out; + + *princs = calloc(tmp + 1, sizeof(**princs)); + if (*princs == NULL) { + ret = ENOMEM; + goto out; + } + for (i = 0; i < tmp; i++) { + ret = krb5_ret_string(sp, &(*princs)[i]); + if (ret) goto out; - } - for(i = 0; i < tmp; i++) - krb5_ret_string(sp, &(*princs)[i]); - *count = tmp; } -out: + *count = tmp; + + out: + krb5_clear_error_message(context->context); + + out_keep_error: krb5_storage_free(sp); - krb5_data_free (&reply); + krb5_data_free(&reply); return ret; } diff --git a/lib/kadm5/modify_c.c b/lib/kadm5/modify_c.c index dd96ae274..47b0f1c94 100644 --- a/lib/kadm5/modify_c.c +++ b/lib/kadm5/modify_c.c @@ -48,34 +48,46 @@ kadm5_c_modify_principal(void *server_handle, krb5_data reply; ret = _kadm5_connect(server_handle); - if(ret) + if (ret) return ret; + krb5_data_zero(&reply); + sp = krb5_storage_from_mem(buf, sizeof(buf)); if (sp == NULL) { - krb5_clear_error_message(context->context); - return ENOMEM; + ret = ENOMEM; + goto out; } - krb5_store_int32(sp, kadm_modify); - kadm5_store_principal_ent(sp, princ); - krb5_store_int32(sp, mask); + ret = krb5_store_int32(sp, kadm_modify); + if (ret) + goto out; + ret = kadm5_store_principal_ent(sp, princ); + if (ret) + goto out; + ret = krb5_store_int32(sp, mask); + if (ret) + goto out; ret = _kadm5_client_send(context, sp); - krb5_storage_free(sp); - if(ret) - return ret; + if (ret) + goto out_keep_error; ret = _kadm5_client_recv(context, &reply); - if(ret) - return ret; - sp = krb5_storage_from_data (&reply); - if (sp == NULL) { - krb5_clear_error_message(context->context); - krb5_data_free (&reply); - return ENOMEM; - } - krb5_ret_int32(sp, &tmp); - krb5_clear_error_message(context->context); + if (ret) + goto out_keep_error; krb5_storage_free(sp); - krb5_data_free (&reply); - return tmp; -} + sp = krb5_storage_from_data(&reply); + if (sp == NULL) { + ret = ENOMEM; + goto out; + } + ret = krb5_ret_int32(sp, &tmp); + if (ret == 0) + ret = tmp; + out: + krb5_clear_error_message(context->context); + + out_keep_error: + krb5_storage_free(sp); + krb5_data_free(&reply); + return ret; +} diff --git a/lib/kadm5/privs_c.c b/lib/kadm5/privs_c.c index 60facf68e..c2b6f4c07 100644 --- a/lib/kadm5/privs_c.c +++ b/lib/kadm5/privs_c.c @@ -48,34 +48,41 @@ kadm5_c_get_privs(void *server_handle, uint32_t *privs) *privs = 0; ret = _kadm5_connect(server_handle); - if(ret) + if (ret) return ret; + krb5_data_zero(&reply); + sp = krb5_storage_from_mem(buf, sizeof(buf)); if (sp == NULL) { - krb5_clear_error_message(context->context); - return ENOMEM; + ret = ENOMEM; + goto out; } - krb5_store_int32(sp, kadm_get_privs); + ret = krb5_store_int32(sp, kadm_get_privs); + if (ret) + goto out; ret = _kadm5_client_send(context, sp); - krb5_storage_free(sp); - if(ret) - return ret; + if (ret) + goto out_keep_error; ret = _kadm5_client_recv(context, &reply); if (ret) - return ret; + goto out_keep_error; + krb5_storage_free(sp); sp = krb5_storage_from_data(&reply); if (sp == NULL) { - krb5_clear_error_message(context->context); - krb5_data_free (&reply); - return ENOMEM; + ret = ENOMEM; + goto out; } - krb5_ret_int32(sp, &tmp); - krb5_clear_error_message(context->context); - ret = tmp; - if(ret == 0){ + ret = krb5_ret_int32(sp, &tmp); + if (ret == 0) + ret = tmp; + if (ret == 0) krb5_ret_uint32(sp, privs); - } + + out: + krb5_clear_error_message(context->context); + + out_keep_error: krb5_storage_free(sp); krb5_data_free (&reply); return ret; diff --git a/lib/kadm5/randkey_c.c b/lib/kadm5/randkey_c.c index 24f38c591..3efbcdf99 100644 --- a/lib/kadm5/randkey_c.c +++ b/lib/kadm5/randkey_c.c @@ -51,15 +51,18 @@ kadm5_c_randkey_principal(void *server_handle, int32_t tmp; size_t i; krb5_data reply; + krb5_keyblock *k; ret = _kadm5_connect(server_handle); - if(ret) + if (ret) return ret; + krb5_data_zero(&reply); + sp = krb5_storage_from_mem(buf, sizeof(buf)); if (sp == NULL) { - krb5_clear_error_message(context->context); - return ENOMEM; + ret = ENOMEM; + goto out; } /* @@ -92,56 +95,60 @@ kadm5_c_randkey_principal(void *server_handle, if (ret == 0) krb5_store_int32(sp, ks_tuple[i].ks_salttype); } - if (ret) - return ret; /* Future extensions go here */ + if (ret) + goto out; ret = _kadm5_client_send(context, sp); - krb5_storage_free(sp); if (ret) - return ret; + goto out_keep_error; ret = _kadm5_client_recv(context, &reply); - if(ret) - return ret; + if (ret) + goto out_keep_error; + krb5_storage_free(sp); sp = krb5_storage_from_data(&reply); if (sp == NULL) { - krb5_clear_error_message(context->context); - krb5_data_free (&reply); - return ENOMEM; + ret = ENOMEM; + goto out; } - krb5_clear_error_message(context->context); ret = krb5_ret_int32(sp, &tmp); if (ret == 0) ret = tmp; - if (ret == 0){ - krb5_keyblock *k; + if (ret) + goto out; - ret = krb5_ret_int32(sp, &tmp); - if (ret) - goto out; - if (tmp < 0) { - ret = EOVERFLOW; - goto out; - } - k = calloc(tmp, sizeof(*k)); - if (k == NULL) { - ret = ENOMEM; - goto out; - } - for(i = 0; ret == 0 && i < tmp; i++) - ret = krb5_ret_keyblock(sp, &k[i]); - if (ret == 0 && n_keys && new_keys) { - *n_keys = tmp; - *new_keys = k; - } else { - krb5_free_keyblock_contents(context->context, &k[i]); - for (; i > 0; i--) - krb5_free_keyblock_contents(context->context, &k[i - 1]); - free(k); - } + ret = krb5_ret_int32(sp, &tmp); + if (ret) + goto out; + if (tmp < 0) { + ret = EOVERFLOW; + goto out; } -out: + k = calloc(tmp, sizeof(*k)); + if (k == NULL) { + ret = ENOMEM; + goto out; + } + for (i = 0; ret == 0 && i < tmp; i++) { + ret = krb5_ret_keyblock(sp, &k[i]); + if (ret) + goto out; + } + if (ret == 0 && n_keys && new_keys) { + *n_keys = tmp; + *new_keys = k; + } else { + krb5_free_keyblock_contents(context->context, &k[i]); + for (; i > 0; i--) + krb5_free_keyblock_contents(context->context, &k[i - 1]); + free(k); + } + + out: + krb5_clear_error_message(context->context); + + out_keep_error: krb5_storage_free(sp); - krb5_data_free (&reply); + krb5_data_free(&reply); return ret; } diff --git a/lib/kadm5/rename_c.c b/lib/kadm5/rename_c.c index 25fcea2f1..01ed20673 100644 --- a/lib/kadm5/rename_c.c +++ b/lib/kadm5/rename_c.c @@ -48,30 +48,47 @@ kadm5_c_rename_principal(void *server_handle, krb5_data reply; ret = _kadm5_connect(server_handle); - if(ret) - return ret; - - sp = krb5_storage_from_mem(buf, sizeof(buf)); - if (sp == NULL) - return ENOMEM; - krb5_store_int32(sp, kadm_rename); - krb5_store_principal(sp, source); - krb5_store_principal(sp, target); - ret = _kadm5_client_send(context, sp); - krb5_storage_free(sp); if (ret) return ret; - ret = _kadm5_client_recv(context, &reply); - if(ret) - return ret; - sp = krb5_storage_from_data (&reply); + + krb5_data_zero(&reply); + + sp = krb5_storage_from_mem(buf, sizeof(buf)); if (sp == NULL) { - krb5_data_free (&reply); - return ENOMEM; + ret = ENOMEM; + goto out; } - krb5_ret_int32(sp, &tmp); - ret = tmp; + + ret = krb5_store_int32(sp, kadm_rename); + if (ret) + goto out; + ret = krb5_store_principal(sp, source); + if (ret) + goto out; + ret = krb5_store_principal(sp, target); + if (ret) + goto out; + ret = _kadm5_client_send(context, sp); + if (ret) + goto out_keep_error; + ret = _kadm5_client_recv(context, &reply); + if (ret) + goto out_keep_error; krb5_storage_free(sp); - krb5_data_free (&reply); + sp = krb5_storage_from_data(&reply); + if (sp == NULL) { + ret = ENOMEM; + goto out; + } + ret = krb5_ret_int32(sp, &tmp); + if (ret == 0) + ret = tmp; + + out: + krb5_clear_error_message(context->context); + + out_keep_error: + krb5_storage_free(sp); + krb5_data_free(&reply); return ret; }