kadm5: Add missing error checks

This commit is contained in:
Nicolas Williams
2022-01-19 16:33:17 -06:00
parent 9d6dd21256
commit 655c057769

View File

@@ -33,7 +33,7 @@
#include "kadm5_locl.h" #include "kadm5_locl.h"
RCSID("$Id$"); #define CHECK(e) do { if ((ret = e)) goto out; } while (0)
int int
kadm5_some_keys_are_bogus(size_t n_keys, krb5_key_data *keys) kadm5_some_keys_are_bogus(size_t n_keys, krb5_key_data *keys)
@@ -72,29 +72,34 @@ kadm5_ret_t
kadm5_store_key_data(krb5_storage *sp, kadm5_store_key_data(krb5_storage *sp,
krb5_key_data *key) krb5_key_data *key)
{ {
kadm5_ret_t ret;
krb5_data c; krb5_data c;
krb5_store_int32(sp, key->key_data_ver);
krb5_store_int32(sp, key->key_data_kvno); CHECK(krb5_store_int32(sp, key->key_data_ver));
krb5_store_int32(sp, key->key_data_type[0]); CHECK(krb5_store_int32(sp, key->key_data_kvno));
CHECK(krb5_store_int32(sp, key->key_data_type[0]));
c.length = key->key_data_length[0]; c.length = key->key_data_length[0];
c.data = key->key_data_contents[0]; c.data = key->key_data_contents[0];
krb5_store_data(sp, c); CHECK(krb5_store_data(sp, c));
krb5_store_int32(sp, key->key_data_type[1]); CHECK(krb5_store_int32(sp, key->key_data_type[1]));
c.length = key->key_data_length[1]; c.length = key->key_data_length[1];
c.data = key->key_data_contents[1]; c.data = key->key_data_contents[1];
krb5_store_data(sp, c); CHECK(krb5_store_data(sp, c));
return 0;
out:
return ret;
} }
kadm5_ret_t kadm5_ret_t
kadm5_store_fake_key_data(krb5_storage *sp, kadm5_store_fake_key_data(krb5_storage *sp,
krb5_key_data *key) krb5_key_data *key)
{ {
kadm5_ret_t ret;
krb5_data c; krb5_data c;
krb5_store_int32(sp, key->key_data_ver); CHECK(krb5_store_int32(sp, key->key_data_ver));
krb5_store_int32(sp, key->key_data_kvno); CHECK(krb5_store_int32(sp, key->key_data_kvno));
krb5_store_int32(sp, key->key_data_type[0]); CHECK(krb5_store_int32(sp, key->key_data_type[0]));
/* /*
* This is the key contents. We want it to be obvious to the client * This is the key contents. We want it to be obvious to the client
@@ -106,63 +111,88 @@ kadm5_store_fake_key_data(krb5_storage *sp,
*/ */
c.length = sizeof (KADM5_BOGUS_KEY_DATA) - 1; c.length = sizeof (KADM5_BOGUS_KEY_DATA) - 1;
c.data = KADM5_BOGUS_KEY_DATA; c.data = KADM5_BOGUS_KEY_DATA;
krb5_store_data(sp, c); CHECK(krb5_store_data(sp, c));
/* This is the salt -- no need to send garbage */ /* This is the salt -- no need to send garbage */
krb5_store_int32(sp, key->key_data_type[1]); CHECK(krb5_store_int32(sp, key->key_data_type[1]));
c.length = key->key_data_length[1]; c.length = key->key_data_length[1];
c.data = key->key_data_contents[1]; c.data = key->key_data_contents[1];
krb5_store_data(sp, c); CHECK(krb5_store_data(sp, c));
return 0;
out:
return ret;
} }
kadm5_ret_t kadm5_ret_t
kadm5_ret_key_data(krb5_storage *sp, kadm5_ret_key_data(krb5_storage *sp,
krb5_key_data *key) krb5_key_data *key)
{ {
kadm5_ret_t ret;
krb5_data c; krb5_data c;
int32_t tmp; int32_t tmp;
krb5_ret_int32(sp, &tmp);
ret = krb5_ret_int32(sp, &tmp);
if (ret == 0) {
key->key_data_ver = tmp; key->key_data_ver = tmp;
krb5_ret_int32(sp, &tmp); ret = krb5_ret_int32(sp, &tmp);
}
if (ret == 0) {
key->key_data_kvno = tmp; key->key_data_kvno = tmp;
krb5_ret_int32(sp, &tmp); ret = krb5_ret_int32(sp, &tmp);
}
if (ret == 0) {
key->key_data_type[0] = tmp; key->key_data_type[0] = tmp;
krb5_ret_data(sp, &c); ret = krb5_ret_data(sp, &c);
}
if (ret == 0) {
key->key_data_length[0] = c.length; key->key_data_length[0] = c.length;
key->key_data_contents[0] = c.data; key->key_data_contents[0] = c.data;
krb5_ret_int32(sp, &tmp); ret = krb5_ret_int32(sp, &tmp);
}
if (ret == 0) {
key->key_data_type[1] = tmp; key->key_data_type[1] = tmp;
krb5_ret_data(sp, &c); ret = krb5_ret_data(sp, &c);
}
if (ret == 0) {
key->key_data_length[1] = c.length; key->key_data_length[1] = c.length;
key->key_data_contents[1] = c.data; key->key_data_contents[1] = c.data;
return 0; return 0;
} }
return KADM5_FAILURE;
}
kadm5_ret_t kadm5_ret_t
kadm5_store_tl_data(krb5_storage *sp, kadm5_store_tl_data(krb5_storage *sp,
krb5_tl_data *tl) krb5_tl_data *tl)
{ {
kadm5_ret_t ret;
krb5_data c; krb5_data c;
krb5_store_int32(sp, tl->tl_data_type);
CHECK(krb5_store_int32(sp, tl->tl_data_type));
c.length = tl->tl_data_length; c.length = tl->tl_data_length;
c.data = tl->tl_data_contents; c.data = tl->tl_data_contents;
krb5_store_data(sp, c); CHECK(krb5_store_data(sp, c));
return 0;
out:
return ret;
} }
kadm5_ret_t kadm5_ret_t
kadm5_ret_tl_data(krb5_storage *sp, kadm5_ret_tl_data(krb5_storage *sp,
krb5_tl_data *tl) krb5_tl_data *tl)
{ {
kadm5_ret_t ret;
krb5_data c; krb5_data c;
int32_t tmp; int32_t tmp;
krb5_ret_int32(sp, &tmp);
CHECK(krb5_ret_int32(sp, &tmp));
tl->tl_data_type = tmp; tl->tl_data_type = tmp;
krb5_ret_data(sp, &c); CHECK(krb5_ret_data(sp, &c));
tl->tl_data_length = c.length; tl->tl_data_length = c.length;
tl->tl_data_contents = c.data; tl->tl_data_contents = c.data;
return 0;
out:
return ret;
} }
static kadm5_ret_t static kadm5_ret_t
@@ -170,63 +200,66 @@ store_principal_ent(krb5_storage *sp,
kadm5_principal_ent_t princ, kadm5_principal_ent_t princ,
uint32_t mask, int wkeys) uint32_t mask, int wkeys)
{ {
kadm5_ret_t ret = 0;
int i; int i;
if (mask & KADM5_PRINCIPAL) if (mask & KADM5_PRINCIPAL)
krb5_store_principal(sp, princ->principal); CHECK(krb5_store_principal(sp, princ->principal));
if (mask & KADM5_PRINC_EXPIRE_TIME) if (mask & KADM5_PRINC_EXPIRE_TIME)
krb5_store_int32(sp, princ->princ_expire_time); CHECK(krb5_store_int32(sp, princ->princ_expire_time));
if (mask & KADM5_PW_EXPIRATION) if (mask & KADM5_PW_EXPIRATION)
krb5_store_int32(sp, princ->pw_expiration); CHECK(krb5_store_int32(sp, princ->pw_expiration));
if (mask & KADM5_LAST_PWD_CHANGE) if (mask & KADM5_LAST_PWD_CHANGE)
krb5_store_int32(sp, princ->last_pwd_change); CHECK(krb5_store_int32(sp, princ->last_pwd_change));
if (mask & KADM5_MAX_LIFE) if (mask & KADM5_MAX_LIFE)
krb5_store_int32(sp, princ->max_life); CHECK(krb5_store_int32(sp, princ->max_life));
if (mask & KADM5_MOD_NAME) { if (mask & KADM5_MOD_NAME) {
krb5_store_int32(sp, princ->mod_name != NULL); CHECK(krb5_store_int32(sp, princ->mod_name != NULL));
if(princ->mod_name) if(princ->mod_name)
krb5_store_principal(sp, princ->mod_name); CHECK(krb5_store_principal(sp, princ->mod_name));
} }
if (mask & KADM5_MOD_TIME) if (mask & KADM5_MOD_TIME)
krb5_store_int32(sp, princ->mod_date); CHECK(krb5_store_int32(sp, princ->mod_date));
if (mask & KADM5_ATTRIBUTES) if (mask & KADM5_ATTRIBUTES)
krb5_store_int32(sp, princ->attributes); CHECK(krb5_store_int32(sp, princ->attributes));
if (mask & KADM5_KVNO) if (mask & KADM5_KVNO)
krb5_store_int32(sp, princ->kvno); CHECK(krb5_store_int32(sp, princ->kvno));
if (mask & KADM5_MKVNO) if (mask & KADM5_MKVNO)
krb5_store_int32(sp, princ->mkvno); CHECK(krb5_store_int32(sp, princ->mkvno));
if (mask & KADM5_POLICY) { if (mask & KADM5_POLICY) {
krb5_store_int32(sp, princ->policy != NULL); CHECK(krb5_store_int32(sp, princ->policy != NULL));
if(princ->policy) if(princ->policy)
krb5_store_string(sp, princ->policy); CHECK(krb5_store_string(sp, princ->policy));
} }
if (mask & KADM5_AUX_ATTRIBUTES) if (mask & KADM5_AUX_ATTRIBUTES)
krb5_store_int32(sp, princ->aux_attributes); CHECK(krb5_store_int32(sp, princ->aux_attributes));
if (mask & KADM5_MAX_RLIFE) if (mask & KADM5_MAX_RLIFE)
krb5_store_int32(sp, princ->max_renewable_life); CHECK(krb5_store_int32(sp, princ->max_renewable_life));
if (mask & KADM5_LAST_SUCCESS) if (mask & KADM5_LAST_SUCCESS)
krb5_store_int32(sp, princ->last_success); CHECK(krb5_store_int32(sp, princ->last_success));
if (mask & KADM5_LAST_FAILED) if (mask & KADM5_LAST_FAILED)
krb5_store_int32(sp, princ->last_failed); CHECK(krb5_store_int32(sp, princ->last_failed));
if (mask & KADM5_FAIL_AUTH_COUNT) if (mask & KADM5_FAIL_AUTH_COUNT)
krb5_store_int32(sp, princ->fail_auth_count); CHECK(krb5_store_int32(sp, princ->fail_auth_count));
if (mask & KADM5_KEY_DATA) { if (mask & KADM5_KEY_DATA) {
krb5_store_int32(sp, princ->n_key_data); CHECK(krb5_store_int32(sp, princ->n_key_data));
for(i = 0; i < princ->n_key_data; i++) { for(i = 0; i < princ->n_key_data; i++) {
if (wkeys) if (wkeys)
kadm5_store_key_data(sp, &princ->key_data[i]); CHECK(kadm5_store_key_data(sp, &princ->key_data[i]));
else else
kadm5_store_fake_key_data(sp, &princ->key_data[i]); CHECK(kadm5_store_fake_key_data(sp, &princ->key_data[i]));
} }
} }
if (mask & KADM5_TL_DATA) { if (mask & KADM5_TL_DATA) {
krb5_tl_data *tp; krb5_tl_data *tp;
krb5_store_int32(sp, princ->n_tl_data); CHECK(krb5_store_int32(sp, princ->n_tl_data));
for (tp = princ->tl_data; tp; tp = tp->tl_data_next) for (tp = princ->tl_data; tp; tp = tp->tl_data_next)
kadm5_store_tl_data(sp, tp); CHECK(kadm5_store_tl_data(sp, tp));
} }
return 0;
out:
return ret;
} }
@@ -249,8 +282,12 @@ kadm5_store_principal_ent_mask(krb5_storage *sp,
kadm5_principal_ent_t princ, kadm5_principal_ent_t princ,
uint32_t mask) uint32_t mask)
{ {
krb5_store_int32(sp, mask); kadm5_ret_t ret;
return store_principal_ent (sp, princ, mask, 1);
ret = krb5_store_int32(sp, mask);
if (ret == 0)
ret = store_principal_ent(sp, princ, mask, 1);
return ret;
} }
static kadm5_ret_t static kadm5_ret_t
@@ -258,101 +295,107 @@ ret_principal_ent(krb5_storage *sp,
kadm5_principal_ent_t princ, kadm5_principal_ent_t princ,
uint32_t mask) uint32_t mask)
{ {
kadm5_ret_t ret = 0;
int i; int i;
int32_t tmp; int32_t tmp;
if (mask & KADM5_PRINCIPAL) if (mask & KADM5_PRINCIPAL)
krb5_ret_principal(sp, &princ->principal); CHECK(krb5_ret_principal(sp, &princ->principal));
if (mask & KADM5_PRINC_EXPIRE_TIME) { if (mask & KADM5_PRINC_EXPIRE_TIME) {
krb5_ret_int32(sp, &tmp); CHECK(krb5_ret_int32(sp, &tmp));
princ->princ_expire_time = tmp; princ->princ_expire_time = tmp;
} }
if (mask & KADM5_PW_EXPIRATION) { if (mask & KADM5_PW_EXPIRATION) {
krb5_ret_int32(sp, &tmp); CHECK(krb5_ret_int32(sp, &tmp));
princ->pw_expiration = tmp; princ->pw_expiration = tmp;
} }
if (mask & KADM5_LAST_PWD_CHANGE) { if (mask & KADM5_LAST_PWD_CHANGE) {
krb5_ret_int32(sp, &tmp); CHECK(krb5_ret_int32(sp, &tmp));
princ->last_pwd_change = tmp; princ->last_pwd_change = tmp;
} }
if (mask & KADM5_MAX_LIFE) { if (mask & KADM5_MAX_LIFE) {
krb5_ret_int32(sp, &tmp); CHECK(krb5_ret_int32(sp, &tmp));
princ->max_life = tmp; princ->max_life = tmp;
} }
if (mask & KADM5_MOD_NAME) { if (mask & KADM5_MOD_NAME) {
krb5_ret_int32(sp, &tmp); CHECK(krb5_ret_int32(sp, &tmp));
if(tmp) if(tmp)
krb5_ret_principal(sp, &princ->mod_name); CHECK(krb5_ret_principal(sp, &princ->mod_name));
else else
princ->mod_name = NULL; princ->mod_name = NULL;
} }
if (mask & KADM5_MOD_TIME) { if (mask & KADM5_MOD_TIME) {
krb5_ret_int32(sp, &tmp); CHECK(krb5_ret_int32(sp, &tmp));
princ->mod_date = tmp; princ->mod_date = tmp;
} }
if (mask & KADM5_ATTRIBUTES) { if (mask & KADM5_ATTRIBUTES) {
krb5_ret_int32(sp, &tmp); CHECK(krb5_ret_int32(sp, &tmp));
princ->attributes = tmp; princ->attributes = tmp;
} }
if (mask & KADM5_KVNO) { if (mask & KADM5_KVNO) {
krb5_ret_int32(sp, &tmp); CHECK(krb5_ret_int32(sp, &tmp));
princ->kvno = tmp; princ->kvno = tmp;
} }
if (mask & KADM5_MKVNO) { if (mask & KADM5_MKVNO) {
krb5_ret_int32(sp, &tmp); CHECK(krb5_ret_int32(sp, &tmp));
princ->mkvno = tmp; princ->mkvno = tmp;
} }
if (mask & KADM5_POLICY) { if (mask & KADM5_POLICY) {
krb5_ret_int32(sp, &tmp); CHECK(krb5_ret_int32(sp, &tmp));
if(tmp) if(tmp)
krb5_ret_string(sp, &princ->policy); CHECK(krb5_ret_string(sp, &princ->policy));
else else
princ->policy = NULL; princ->policy = NULL;
} }
if (mask & KADM5_AUX_ATTRIBUTES) { if (mask & KADM5_AUX_ATTRIBUTES) {
krb5_ret_int32(sp, &tmp); CHECK(krb5_ret_int32(sp, &tmp));
princ->aux_attributes = tmp; princ->aux_attributes = tmp;
} }
if (mask & KADM5_MAX_RLIFE) { if (mask & KADM5_MAX_RLIFE) {
krb5_ret_int32(sp, &tmp); CHECK(krb5_ret_int32(sp, &tmp));
princ->max_renewable_life = tmp; princ->max_renewable_life = tmp;
} }
if (mask & KADM5_LAST_SUCCESS) { if (mask & KADM5_LAST_SUCCESS) {
krb5_ret_int32(sp, &tmp); CHECK(krb5_ret_int32(sp, &tmp));
princ->last_success = tmp; princ->last_success = tmp;
} }
if (mask & KADM5_LAST_FAILED) { if (mask & KADM5_LAST_FAILED) {
krb5_ret_int32(sp, &tmp); CHECK(krb5_ret_int32(sp, &tmp));
princ->last_failed = tmp; princ->last_failed = tmp;
} }
if (mask & KADM5_FAIL_AUTH_COUNT) { if (mask & KADM5_FAIL_AUTH_COUNT) {
krb5_ret_int32(sp, &tmp); CHECK(krb5_ret_int32(sp, &tmp));
princ->fail_auth_count = tmp; princ->fail_auth_count = tmp;
} }
if (mask & KADM5_KEY_DATA) { if (mask & KADM5_KEY_DATA) {
krb5_ret_int32(sp, &tmp); CHECK(krb5_ret_int32(sp, &tmp));
princ->n_key_data = tmp; princ->n_key_data = tmp;
princ->key_data = malloc(princ->n_key_data * sizeof(*princ->key_data)); princ->key_data = malloc(princ->n_key_data * sizeof(*princ->key_data));
if (princ->key_data == NULL && princ->n_key_data != 0) if (princ->key_data == NULL && princ->n_key_data != 0)
return ENOMEM; return ENOMEM;
for(i = 0; i < princ->n_key_data; i++) for(i = 0; i < princ->n_key_data; i++)
kadm5_ret_key_data(sp, &princ->key_data[i]); ret = kadm5_ret_key_data(sp, &princ->key_data[i]);
} }
if (mask & KADM5_TL_DATA) { if (mask & KADM5_TL_DATA) {
krb5_ret_int32(sp, &tmp); CHECK(krb5_ret_int32(sp, &tmp));
princ->n_tl_data = tmp; princ->n_tl_data = tmp;
princ->tl_data = NULL; princ->tl_data = NULL;
for(i = 0; i < princ->n_tl_data; i++){ for(i = 0; i < princ->n_tl_data; i++){
krb5_tl_data *tp = malloc(sizeof(*tp)); krb5_tl_data *tp = malloc(sizeof(*tp));
if (tp == NULL) if (tp == NULL) {
return ENOMEM; ret = ENOMEM;
kadm5_ret_tl_data(sp, tp); goto out;
}
CHECK(kadm5_ret_tl_data(sp, tp));
tp->tl_data_next = princ->tl_data; tp->tl_data_next = princ->tl_data;
princ->tl_data = tp; princ->tl_data = tp;
} }
} }
return 0;
out:
/* Can't free princ here -- we don't have a context */
return ret;
} }
kadm5_ret_t kadm5_ret_t
@@ -367,9 +410,14 @@ kadm5_ret_principal_ent_mask(krb5_storage *sp,
kadm5_principal_ent_t princ, kadm5_principal_ent_t princ,
uint32_t *mask) uint32_t *mask)
{ {
kadm5_ret_t ret;
int32_t tmp; int32_t tmp;
krb5_ret_int32 (sp, &tmp); ret = krb5_ret_int32 (sp, &tmp);
if (ret) {
*mask = 0;
return ret;
}
*mask = tmp; *mask = tmp;
return ret_principal_ent (sp, princ, *mask); return ret_principal_ent (sp, princ, *mask);
} }
@@ -379,18 +427,19 @@ _kadm5_marshal_params(krb5_context context,
kadm5_config_params *params, kadm5_config_params *params,
krb5_data *out) krb5_data *out)
{ {
kadm5_ret_t ret;
krb5_storage *sp = krb5_storage_emem(); krb5_storage *sp = krb5_storage_emem();
if (sp == NULL) if (sp == NULL)
return krb5_enomem(context); return krb5_enomem(context);
krb5_store_int32(sp, params->mask & (KADM5_CONFIG_REALM)); ret = krb5_store_int32(sp, params->mask & (KADM5_CONFIG_REALM));
if (ret == 0 && (params->mask & KADM5_CONFIG_REALM))
if(params->mask & KADM5_CONFIG_REALM) ret = krb5_store_string(sp, params->realm);
krb5_store_string(sp, params->realm); if (ret == 0)
krb5_storage_to_data(sp, out); ret = krb5_storage_to_data(sp, out);
krb5_storage_free(sp); krb5_storage_free(sp);
return ret;
return 0;
} }
kadm5_ret_t kadm5_ret_t
@@ -398,7 +447,7 @@ _kadm5_unmarshal_params(krb5_context context,
krb5_data *in, krb5_data *in,
kadm5_config_params *params) kadm5_config_params *params)
{ {
krb5_error_code ret; kadm5_ret_t ret;
krb5_storage *sp; krb5_storage *sp;
int32_t mask; int32_t mask;