From 621deed047d63ad65c82bf8799902a7c7f07279d Mon Sep 17 00:00:00 2001 From: Nicolas Williams Date: Wed, 23 Mar 2022 17:00:11 -0500 Subject: [PATCH] kadmin: Fix re-entrance in iterations Any callback of kadm5_iter_principals() that wants to call other kadm5 functions (such as kadm5_get_principal()) needs to do so on a different kadm5 handle than the one used for kadm5_iter_principals(). --- kadmin/cpw.c | 43 +++++++++++++++++--------- kadmin/del.c | 22 ++++++++------ kadmin/ext.c | 11 +++++-- kadmin/get.c | 10 +++++-- kadmin/mod.c | 63 +++++++++++++++++++++++++-------------- tests/kdc/check-kadmin.in | 11 +++++++ 6 files changed, 109 insertions(+), 51 deletions(-) diff --git a/kadmin/cpw.c b/kadmin/cpw.c index 2f3c1c1bc..0ee88183b 100644 --- a/kadmin/cpw.c +++ b/kadmin/cpw.c @@ -40,18 +40,19 @@ struct cpw_entry_data { int random_password; char *password; krb5_key_data *key_data; + void *kadm_handle; }; static int -set_random_key (krb5_principal principal, int keepold) +set_random_key(void *dup_kadm_handle, krb5_principal principal, int keepold) { krb5_error_code ret; int i; krb5_keyblock *keys; int num_keys; - ret = kadm5_randkey_principal_3(kadm_handle, principal, keepold, 0, NULL, - &keys, &num_keys); + ret = kadm5_randkey_principal_3(dup_kadm_handle, principal, keepold, 0, + NULL, &keys, &num_keys); if(ret) return ret; for(i = 0; i < num_keys; i++) @@ -61,7 +62,9 @@ set_random_key (krb5_principal principal, int keepold) } static int -set_random_password (krb5_principal principal, int keepold) +set_random_password(void *dup_kadm_handle, + krb5_principal principal, + int keepold) { krb5_error_code ret; char pw[128]; @@ -72,7 +75,8 @@ set_random_password (krb5_principal principal, int keepold) return ret; random_password(pw, sizeof(pw)); - ret = kadm5_chpass_principal_3(kadm_handle, principal, keepold, 0, NULL, pw); + ret = kadm5_chpass_principal_3(dup_kadm_handle, principal, keepold, 0, + NULL, pw); if (ret == 0) printf ("%s's password set to \"%s\"\n", princ_name, pw); free(princ_name); @@ -81,7 +85,10 @@ set_random_password (krb5_principal principal, int keepold) } static int -set_password (krb5_principal principal, char *password, int keepold) +set_password(void *dup_kadm_handle, + krb5_principal principal, + char *password, + int keepold) { krb5_error_code ret = 0; char pwbuf[128]; @@ -108,18 +115,21 @@ set_password (krb5_principal principal, char *password, int keepold) password = pwbuf; } if(ret == 0) - ret = kadm5_chpass_principal_3(kadm_handle, principal, keepold, 0, NULL, - password); + ret = kadm5_chpass_principal_3(dup_kadm_handle, principal, keepold, 0, + NULL, password); memset_s(pwbuf, sizeof(pwbuf), 0, sizeof(pwbuf)); return ret; } static int -set_key_data (krb5_principal principal, krb5_key_data *key_data, int keepold) +set_key_data(void *dup_kadm_handle, + krb5_principal principal, + krb5_key_data *key_data, + int keepold) { krb5_error_code ret; - ret = kadm5_chpass_principal_with_key_3(kadm_handle, principal, keepold, + ret = kadm5_chpass_principal_with_key_3(dup_kadm_handle, principal, keepold, 3, key_data); return ret; } @@ -130,13 +140,13 @@ do_cpw_entry(krb5_principal principal, void *data) struct cpw_entry_data *e = data; if (e->random_key) - return set_random_key (principal, e->keepold); + return set_random_key(e->kadm_handle, principal, e->keepold); else if (e->random_password) - return set_random_password (principal, e->keepold); + return set_random_password(e->kadm_handle, principal, e->keepold); else if (e->key_data) - return set_key_data (principal, e->key_data, e->keepold); + return set_key_data(e->kadm_handle, principal, e->key_data, e->keepold); else - return set_password (principal, e->password, e->keepold); + return set_password(e->kadm_handle, principal, e->password, e->keepold); } int @@ -148,6 +158,9 @@ cpw_entry(struct passwd_options *opt, int argc, char **argv) int num; krb5_key_data key_data[3]; + ret = kadm5_dup_context(kadm_handle, &data.kadm_handle); + if (ret) + krb5_err(context, 1, ret, "Could not duplicate kadmin connection"); data.random_key = opt->random_key_flag; data.random_password = opt->random_password_flag; data.password = opt->password_string; @@ -206,6 +219,8 @@ cpw_entry(struct passwd_options *opt, int argc, char **argv) for(i = 0; i < argc; i++) ret = foreach_principal(argv[i], do_cpw_entry, "cpw", &data); + kadm5_destroy(data.kadm_handle); + if (data.key_data) { int16_t dummy; kadm5_free_key_data (kadm_handle, &dummy, key_data); diff --git a/kadmin/del.c b/kadmin/del.c index a066f56ea..75d532b1f 100644 --- a/kadmin/del.c +++ b/kadmin/del.c @@ -37,7 +37,7 @@ static int do_del_entry(krb5_principal principal, void *data) { - return kadm5_delete_principal(kadm_handle, principal); + return kadm5_delete_principal(data, principal); } int @@ -45,12 +45,15 @@ del_entry(void *opt, int argc, char **argv) { int i; krb5_error_code ret = 0; + void *dup_kadm_handle = NULL; - for(i = 0; i < argc; i++) { + ret = kadm5_dup_context(kadm_handle, &kadm_handle); + + for (i = 0; ret == 0 && i < argc; i++) ret = foreach_principal(argv[i], do_del_entry, "del", NULL); - if (ret) - break; - } + + if (dup_kadm_handle) + kadm5_destroy(dup_kadm_handle); return ret != 0; } @@ -91,12 +94,13 @@ del_namespace(void *opt, int argc, char **argv) { int i; krb5_error_code ret = 0; + void *dup_kadm_handle = NULL; - for(i = 0; i < argc; i++) { + ret = kadm5_dup_context(kadm_handle, &dup_kadm_handle); + for (i = 0; ret == 0 && i < argc; i++) ret = foreach_principal(argv[i], do_del_ns_entry, "del_ns", NULL); - if (ret) - break; - } + if (dup_kadm_handle) + kadm5_destroy(dup_kadm_handle); return ret != 0; } diff --git a/kadmin/ext.c b/kadmin/ext.c index adb2e2851..1f4cfe1a7 100644 --- a/kadmin/ext.c +++ b/kadmin/ext.c @@ -40,6 +40,7 @@ struct ext_keytab_data { int random_key_flag; size_t nkstuple; krb5_key_salt_tuple *kstuple; + void *kadm_handle; }; static int @@ -59,7 +60,7 @@ do_ext_keytab(krb5_principal principal, void *data) if (!e->random_key_flag) mask |= KADM5_KVNO | KADM5_KEY_DATA; - ret = kadm5_get_principal(kadm_handle, principal, &princ, mask); + ret = kadm5_get_principal(e->kadm_handle, principal, &princ, mask); if (ret) return ret; @@ -112,7 +113,7 @@ do_ext_keytab(krb5_principal principal, void *data) n_k++; } } else if (e->random_key_flag) { - ret = kadm5_randkey_principal_3(kadm_handle, principal, e->keep, + ret = kadm5_randkey_principal_3(e->kadm_handle, principal, e->keep, e->nkstuple, e->kstuple, &k, &n_k); if (ret) goto out; @@ -140,7 +141,7 @@ do_ext_keytab(krb5_principal principal, void *data) } out: - kadm5_free_principal_ent(kadm_handle, &princ); + kadm5_free_principal_ent(e->kadm_handle, &princ); if (k) { for (i = 0; i < n_k; i++) memset(k[i].keyvalue.data, 0, k[i].keyvalue.length); @@ -159,6 +160,9 @@ ext_keytab(struct ext_keytab_options *opt, int argc, char **argv) const char *enctypes; size_t i; + ret = kadm5_dup_context(kadm_handle, &data.kadm_handle); + if (ret) + krb5_err(context, 1, ret, "Could not duplicate kadmin connection"); data.random_key_flag = opt->random_key_flag; data.keep = 1; i = 0; @@ -209,6 +213,7 @@ ext_keytab(struct ext_keytab_options *opt, int argc, char **argv) break; } + kadm5_destroy(data.kadm_handle); krb5_kt_close(context, data.keytab); free(data.kstuple); return ret != 0; diff --git a/kadmin/get.c b/kadmin/get.c index 38c1b7494..838615ae8 100644 --- a/kadmin/get.c +++ b/kadmin/get.c @@ -83,6 +83,7 @@ struct get_entry_data { uint32_t extra_mask; struct field_info *chead, **ctail; const char *krb5_config_fname; + void *kadm_handle; uint32_t n; int upto; }; @@ -485,12 +486,12 @@ do_get_entry(krb5_principal principal, void *data) e->upto--; memset(&princ, 0, sizeof(princ)); - ret = kadm5_get_principal(kadm_handle, principal, + ret = kadm5_get_principal(e->kadm_handle, principal, &princ, e->mask | e->extra_mask); if (ret == 0) { (e->format)(e, &princ); - kadm5_free_principal_ent(kadm_handle, &princ); + kadm5_free_principal_ent(e->kadm_handle, &princ); } e->n++; @@ -591,6 +592,9 @@ getit(struct get_options *opt, const char *name, int argc, char **argv) if (opt->terse_flag) return listit(name, opt->upto_integer, argc, argv); + ret = kadm5_dup_context(kadm_handle, &data.kadm_handle); + if (ret) + krb5_err(context, 1, ret, "Could not duplicate kadmin connection"); data.table = NULL; data.chead = NULL; data.ctail = &data.chead; @@ -623,6 +627,8 @@ getit(struct get_options *opt, const char *name, int argc, char **argv) for(i = 0; i < argc; i++) ret = foreach_principal(argv[i], do_get_entry, name, &data); + kadm5_destroy(data.kadm_handle); + if(data.table != NULL) { rtbl_format(data.table, stdout); rtbl_destroy(data.table); diff --git a/kadmin/mod.c b/kadmin/mod.c index 7c7b2dd7c..43c25d73a 100644 --- a/kadmin/mod.c +++ b/kadmin/mod.c @@ -308,16 +308,24 @@ add_krb5_config(kadm5_principal_ent_rec *princ, const char *fname) add_tl(princ, KRB5_TL_EXTENSION, &buf); } +struct mod_data { + struct modify_namespace_key_rotation_options *opt_ns_kr; + struct modify_namespace_options *opt_ns; + struct modify_options *opt; + void *kadm_handle; +}; + static int do_mod_entry(krb5_principal principal, void *data) { krb5_error_code ret; kadm5_principal_ent_rec princ; int mask = 0; - struct modify_options *e = data; + struct mod_data *m = data; + struct modify_options *e = m->opt; memset (&princ, 0, sizeof(princ)); - ret = kadm5_get_principal(kadm_handle, principal, &princ, + ret = kadm5_get_principal(m->kadm_handle, principal, &princ, KADM5_PRINCIPAL | KADM5_ATTRIBUTES | KADM5_MAX_LIFE | KADM5_MAX_RLIFE | KADM5_PRINC_EXPIRE_TIME | @@ -382,12 +390,12 @@ do_mod_entry(krb5_principal principal, void *data) } else ret = edit_entry(&princ, &mask, NULL, 0); if(ret == 0) { - ret = kadm5_modify_principal(kadm_handle, &princ, mask); + ret = kadm5_modify_principal(m->kadm_handle, &princ, mask); if(ret) krb5_warn(context, ret, "kadm5_modify_principal"); } - kadm5_free_principal_ent(kadm_handle, &princ); + kadm5_free_principal_ent(m->kadm_handle, &princ); return ret; } @@ -395,13 +403,16 @@ int mod_entry(struct modify_options *opt, int argc, char **argv) { krb5_error_code ret = 0; + struct mod_data data; int i; - for(i = 0; i < argc; i++) { - ret = foreach_principal(argv[i], do_mod_entry, "mod", opt); - if (ret) - break; - } + data.opt_ns_kr = NULL; + data.opt_ns = NULL; + data.opt = opt; + + ret = kadm5_dup_context(kadm_handle, &data.kadm_handle); + for (i = 0; ret == 0 && i < argc; i++) + ret = foreach_principal(argv[i], do_mod_entry, "mod", &data); return ret != 0; } @@ -411,10 +422,11 @@ do_mod_ns_entry(krb5_principal principal, void *data) krb5_error_code ret; kadm5_principal_ent_rec princ; int mask = 0; - struct modify_namespace_options *e = data; + struct mod_data *m = data; + struct modify_namespace_options *e = m->opt_ns; memset (&princ, 0, sizeof(princ)); - ret = kadm5_get_principal(kadm_handle, principal, &princ, + ret = kadm5_get_principal(m->kadm_handle, principal, &princ, KADM5_PRINCIPAL | KADM5_ATTRIBUTES | KADM5_MAX_LIFE | KADM5_MAX_RLIFE | KADM5_PRINC_EXPIRE_TIME | @@ -441,12 +453,12 @@ do_mod_ns_entry(krb5_principal principal, void *data) } else ret = edit_entry(&princ, &mask, NULL, 0); if(ret == 0) { - ret = kadm5_modify_principal(kadm_handle, &princ, mask); + ret = kadm5_modify_principal(m->kadm_handle, &princ, mask); if(ret) krb5_warn(context, ret, "kadm5_modify_principal"); } - kadm5_free_principal_ent(kadm_handle, &princ); + kadm5_free_principal_ent(m->kadm_handle, &princ); return ret; } @@ -454,13 +466,16 @@ int modify_namespace(struct modify_namespace_options *opt, int argc, char **argv) { krb5_error_code ret = 0; + struct mod_data data; int i; - for(i = 0; i < argc; i++) { - ret = foreach_principal(argv[i], do_mod_ns_entry, "mod_ns", opt); - if (ret) - break; - } + data.opt_ns_kr = NULL; + data.opt_ns = opt; + data.opt = NULL; + + ret = kadm5_dup_context(kadm_handle, &data.kadm_handle); + for (i = 0; ret == 0 && i < argc; i++) + ret = foreach_principal(argv[i], do_mod_ns_entry, "mod_ns", &data); return ret != 0; } @@ -672,15 +687,17 @@ modify_ns_kr(struct modify_namespace_key_rotation_options *opt, char **argv) { krb5_error_code ret = 0; + struct mod_data data; int i; - for(i = 0; i < argc; i++) { + data.opt_ns_kr = opt; + data.opt_ns = NULL; + data.opt = NULL; + + ret = kadm5_dup_context(kadm_handle, &data.kadm_handle); + for (i = 0; ret == 0 && i < argc; i++) ret = foreach_principal(argv[i], do_mod_ns_kr, "mod_ns", opt); - if (ret) - break; - } return ret != 0; - return 0; } #define princ_realm(P) ((P)->realm) diff --git a/tests/kdc/check-kadmin.in b/tests/kdc/check-kadmin.in index 7084e17ed..5fabeff20 100644 --- a/tests/kdc/check-kadmin.in +++ b/tests/kdc/check-kadmin.in @@ -318,6 +318,17 @@ env KRB5CCNAME=${cache} \ [ `wc -l < kadmin.tmp` -eq 3 ] || { echo "kadmin list --upto 3 produced `wc -l < kadmin.tmp` results!"; exit 1; } +#---------------------------------- +echo "kadmin get '*' (re-entrance)"; > messages.log +${kadmin} -l get '*' > kadmin.tmp || + { echo "failed to list principals"; cat messages.log ; exit 1; } +> messages.log +env KRB5CCNAME=${cache} \ + ${kadmin} -p foo/admin@${R} get '*' > kadmin.tmp2 || + { echo "failed to list principals"; cat messages.log ; exit 1; } +diff -u kadmin.tmp kadmin.tmp2 || + { echo "local and remote get all differ"; exit 1; } + #---------------------------------- # We have 20 principals in the DB. Test two chunks of 1 (since that's how we # started kadmind above.