diff --git a/kadmin/ank.c b/kadmin/ank.c index c8e429f53..7ed66ff73 100644 --- a/kadmin/ank.c +++ b/kadmin/ank.c @@ -160,6 +160,12 @@ add_one_principal (const char *name, kadm5_get_principal(kadm_handle, princ_ent, &princ, KADM5_PRINCIPAL | KADM5_KVNO | KADM5_ATTRIBUTES); princ.attributes &= (~KRB5_KDB_DISALLOW_ALL_TIX); + /* + * Updating kvno w/o key data and vice-versa gives _kadm5_setup_entry() + * and _kadm5_set_keys2() headaches. But we used to, so we handle + * this in in those two functions. Might as well leave this code as + * it was then. + */ princ.kvno = 1; kadm5_modify_principal(kadm_handle, &princ, KADM5_ATTRIBUTES | KADM5_KVNO); diff --git a/kadmin/del_enctype.c b/kadmin/del_enctype.c index 01d2036a4..c9aae13ae 100644 --- a/kadmin/del_enctype.c +++ b/kadmin/del_enctype.c @@ -106,6 +106,10 @@ del_enctype(void *opt, int argc, char **argv) } free (princ.key_data); + if (j == 0) { + free(new_key_data); + new_key_data = NULL; + } princ.n_key_data = j; princ.key_data = new_key_data; diff --git a/kadmin/get.c b/kadmin/get.c index 98ace8927..466a8d88b 100644 --- a/kadmin/get.c +++ b/kadmin/get.c @@ -60,7 +60,7 @@ static struct field_name { { "last_failed", KADM5_LAST_FAILED, 0, 0, "Last fail", "Last failed login", 0 }, { "fail_auth_count", KADM5_FAIL_AUTH_COUNT, 0, 0, "Fail count", "Failed login count", RTBL_ALIGN_RIGHT }, { "policy", KADM5_POLICY, 0, 0, "Policy", "Policy", 0 }, - { "keytypes", KADM5_KEY_DATA, 0, KADM5_PRINCIPAL, "Keytypes", "Keytypes", 0 }, + { "keytypes", KADM5_KEY_DATA, 0, KADM5_PRINCIPAL | KADM5_KVNO, "Keytypes", "Keytypes", 0 }, { "password", KADM5_TL_DATA, KRB5_TL_PASSWORD, KADM5_KEY_DATA, "Password", "Password", 0 }, { "pkinit-acl", KADM5_TL_DATA, KRB5_TL_PKINIT_ACL, 0, "PK-INIT ACL", "PK-INIT ACL", 0 }, { "aliases", KADM5_TL_DATA, KRB5_TL_ALIASES, 0, "Aliases", "Aliases", 0 }, diff --git a/lib/kadm5/common_glue.c b/lib/kadm5/common_glue.c index 20f111d74..38eda082b 100644 --- a/lib/kadm5/common_glue.c +++ b/lib/kadm5/common_glue.c @@ -258,7 +258,7 @@ kadm5_setkey_principal_3(void *server_handle, return KADM5_SETKEY3_ETYPE_MISMATCH; ret = kadm5_get_principal(server_handle, princ, &princ_ent, - KADM5_PRINCIPAL | KADM5_KEY_DATA); + KADM5_KVNO | KADM5_PRINCIPAL | KADM5_KEY_DATA); if (ret) return ret; diff --git a/lib/kadm5/create_s.c b/lib/kadm5/create_s.c index 1033ca103..632575856 100644 --- a/lib/kadm5/create_s.c +++ b/lib/kadm5/create_s.c @@ -111,6 +111,12 @@ kadm5_s_create_principal_with_key(void *server_handle, hdb_entry_ex ent; kadm5_server_context *context = server_handle; + if ((mask & KADM5_KVNO) == 0) { + /* create_principal() through _kadm5_setup_entry(), will need this */ + princ->kvno = 1; + mask |= KADM5_KVNO; + } + ret = create_principal(context, princ, mask, &ent, KADM5_PRINCIPAL | KADM5_KEY_DATA, KADM5_LAST_PWD_CHANGE | KADM5_MOD_TIME @@ -121,9 +127,6 @@ kadm5_s_create_principal_with_key(void *server_handle, if(ret) goto out; - if ((mask & KADM5_KVNO) == 0) - ent.entry.kvno = 1; - ret = hdb_seal_keys(context->context, context->db, &ent.entry); if (ret) goto out; @@ -153,6 +156,12 @@ kadm5_s_create_principal(void *server_handle, hdb_entry_ex ent; kadm5_server_context *context = server_handle; + if ((mask & KADM5_KVNO) == 0) { + /* create_principal() through _kadm5_setup_entry(), will need this */ + princ->kvno = 1; + mask |= KADM5_KVNO; + } + ret = create_principal(context, princ, mask, &ent, KADM5_PRINCIPAL, KADM5_LAST_PWD_CHANGE | KADM5_MOD_TIME @@ -163,9 +172,6 @@ kadm5_s_create_principal(void *server_handle, if(ret) goto out; - if ((mask & KADM5_KVNO) == 0) - ent.entry.kvno = 1; - ent.entry.keys.len = 0; ent.entry.keys.val = NULL; diff --git a/lib/kadm5/ent_setup.c b/lib/kadm5/ent_setup.c index f2d1f2f28..4beb024a3 100644 --- a/lib/kadm5/ent_setup.c +++ b/lib/kadm5/ent_setup.c @@ -178,8 +178,27 @@ _kadm5_setup_entry(kadm5_server_context *context, } } if(mask & KADM5_KVNO - && princ_mask & KADM5_KVNO) - ent->entry.kvno = princ->kvno; + && princ_mask & KADM5_KVNO) { + /* + * For some reason kadmin's ank changes the kvno after calling + * randkey. Now that we have key history, what are we to do + * when we update kvno but not keys?! + * + * For now just clear the key history if the kvno changes. + * Eventually we may want to search the key history for matching + * keys and use those to replace the current key set (putting + * the old current keyset in the history keysets list?!). + */ + if (ent->entry.kvno != princ->kvno && + (mask & princ_mask & KADM5_KEY_DATA)) { + hdb_clear_extension(context->context, &ent->entry, + choice_HDB_extension_data_hist_keys); + princ->kvno = ent->entry.kvno; + } else { + /* _kadm5_set_keys2() expects this to have been done here */ + ent->entry.kvno = princ->kvno; + } + } if(mask & KADM5_MAX_RLIFE) { if(princ_mask & KADM5_MAX_RLIFE) { if(princ->max_renewable_life) diff --git a/lib/kadm5/set_keys.c b/lib/kadm5/set_keys.c index 36c0a5e05..06ba35491 100644 --- a/lib/kadm5/set_keys.c +++ b/lib/kadm5/set_keys.c @@ -102,19 +102,71 @@ _kadm5_set_keys2(kadm5_server_context *context, krb5_error_code ret; size_t i, k; HDB_extension ext; - HDB_extension *extp; + HDB_extension *extp = NULL; HDB_Ext_KeySet *hist_keys = &ext.data.u.hist_keys; Key key; Salt salt; Keys keys; hdb_keyset hkset; + krb5_kvno kvno = -1; + int one_key_set = 1; + int replace_hist_keys = 0; + + if (n_key_data == 0) { + /* Clear all keys! */ + ret = hdb_clear_extension(context->context, ent, + choice_HDB_extension_data_hist_keys); + if (ret) + return ret; + free_Keys(&ent->keys); + return 0; + } memset(&keys, 0, sizeof (keys)); memset(&hkset, 0, sizeof (hkset)); /* set set_time */ ext.data.element = choice_HDB_extension_data_hist_keys; memset(hist_keys, 0, sizeof (*hist_keys)); - for(i = 0; i < n_key_data; i++) { + for (i = 0; i < n_key_data; i++) { + if (kvno != -1 && kvno != key_data[i].key_data_kvno) { + one_key_set = 0; + break; + } + kvno = key_data[i].key_data_kvno; + } + if (one_key_set) { + /* + * If we're updating KADM5_KEY_DATA with a single keyset then we + * assume we must be setting the principal's kvno as well! + * + * Just have to be careful about old clients that might have + * sent 0 as the kvno... This may seem ugly, but it's the price + * of backwards compatibility with pre-multi-kvno kadmin clients + * (besides, who's to say that updating KADM5_KEY_DATA requires + * updating the entry's kvno?) + * + * Note that we do nothing special for the case where multiple + * keysets are given but the entry's kvno is not set and not in + * the given set of keysets. If this happens we'll just update + * the key history only and leave the current keyset alone. + */ + if (kvno == 0) { + /* Force kvno to 1 if it was 0; (ank would do this anyways) */ + if (ent->kvno == 0) + ent->kvno = 1; + /* Below we need key_data[*].kvno to be reasonable */ + for (i = 0; i < n_key_data; i++) + key_data[i].key_data_kvno = ent->kvno; + } else { + /* + * Or force the entry's kvno to match the one from the new, + * singular keyset + */ + ent->kvno = kvno; + } + } + + for (i = 0; i < n_key_data; i++) { if (key_data[i].key_data_kvno == ent->kvno) { /* A current key; add to current key set */ setup_Key(&key, &salt, key_data, i); @@ -150,22 +202,20 @@ _kadm5_set_keys2(kadm5_server_context *context, ret = add_HDB_Ext_KeySet(hist_keys, &hkset); if (ret) goto out; + replace_hist_keys = 1; } - - /* - * A structure copy is more efficient here than this would be: - * - * copy_Keys(&keys, &ent->keys); - * free_Keys(&keys); - */ - free_Keys(&ent->keys); - ent->keys = keys; - /* Try to keep the set_time values from the old hist keys */ - extp = hdb_find_extension(ent, choice_HDB_extension_data_hist_keys); + if (replace_hist_keys) + /* No key history given -> leave it alone */ + extp = hdb_find_extension(ent, choice_HDB_extension_data_hist_keys); if (extp != NULL) { HDB_Ext_KeySet *old_hist_keys; + /* + * Try to keep the very useful set_time values from the old hist + * keys. kadm5 loses this info, so this heuristic is the best we + * can do. + */ old_hist_keys = &extp->data.u.hist_keys; for (i = 0; i < old_hist_keys->len; i++) { if (old_hist_keys->val[i].set_time == NULL) @@ -179,7 +229,23 @@ _kadm5_set_keys2(kadm5_server_context *context, } } - hdb_replace_extension(context->context, ent, &ext); + if (replace_hist_keys) { + /* If hist keys not given in key_data then don't blow away hist_keys */ + ret = hdb_replace_extension(context->context, ent, &ext); + if (ret) + goto out; + } + + /* + * A structure copy is more efficient here than this would be: + * + * copy_Keys(&keys, &ent->keys); + * free_Keys(&keys); + * + * Of course, the above hdb_replace_extension() is not at all efficient... + */ + free_Keys(&ent->keys); + ent->keys = keys; hdb_entry_set_pw_change_time(context->context, ent, 0); hdb_entry_clear_password(context->context, ent);