diff --git a/lib/hdb/hdb-mitdb.c b/lib/hdb/hdb-mitdb.c index b987922ff..66b392a80 100644 --- a/lib/hdb/hdb-mitdb.c +++ b/lib/hdb/hdb-mitdb.c @@ -196,133 +196,6 @@ fix_salt(krb5_context context, hdb_entry *ent, int key_num) return 0; } -/** - * This function outputs a pointer to a Key or array of @key_count Keys - * where the caller may place Keys. - * - * @param context Context - * @param entry HDB entry - * @param kvno kvno of the keys to be added - * @param is_hist Whether the keys will be historical keys or current keys - * @param key_count Size of array of keys to set. MUST be zero if !is_hist. - * @param out Pointer to Key * variable where to put the resulting Key * - * - * See three call sites below for more information. - */ -static krb5_error_code -get_entry_key_location(krb5_context context, hdb_entry *entry, krb5_kvno kvno, - krb5_boolean is_hist, size_t key_count, Key **out) -{ - HDB_extension ext; - HDB_Ext_KeySet *hist_keys; - hdb_keyset *keyset = NULL; - size_t keyset_count = 0; - Key *k = NULL; - size_t i; - krb5_error_code ret; - - *out = NULL; - - if (!is_hist) { - Key *tmp; - - /* Extend current keyset */ - tmp = realloc(entry->keys.val, sizeof(entry->keys.val[0]) * (entry->keys.len + 1)); - if (tmp == NULL) { - ret = ENOMEM; - goto out; - } - entry->keys.val = tmp; - - /* k points to current Key */ - k = &entry->keys.val[entry->keys.len]; - - memset(k, 0, sizeof(*k)); - entry->keys.len += 1; - - goto done; - } - - /* Find a history keyset and extend it or extend the history keyset */ - memset(&ext, 0, sizeof (ext)); - ext.data.element = choice_HDB_extension_data_hist_keys; - hist_keys = &ext.data.u.hist_keys; - - /* hdb_replace_extension() makes a copy of ext */ - ret = hdb_replace_extension(context, entry, &ext); - if (ret) - return ret; - - for (i = 0; i < hist_keys->len; i++) { - if (hist_keys->val[i].kvno == kvno) { - /* We're adding a key to an existing history keyset */ - keyset = &hist_keys->val[i]; - if ((keyset->keys.len % 8) == 0) { - Key *tmp; - - /* We're adding the 9th, 17th, ... key to the set */ - tmp = realloc(keyset->keys.val, - (keyset->keys.len + 8) * sizeof (*tmp)); - if (tmp == NULL) { - ret = ENOMEM; - goto out; - } - } - break; - } - } - - if (keyset == NULL) { - /* We're adding the first key of a new history keyset */ - if (hist_keys->val == NULL) { - if (key_count == 0) - keyset_count = 8; /* There's not that many enctypes */ - else - keyset_count = key_count; - hist_keys->val = calloc(keyset_count, - sizeof (*hist_keys->val)); - if (hist_keys->val == NULL) { - ret = ENOMEM; - goto out; - } - keyset = &hist_keys->val[0]; - } else if (hist_keys->len == keyset_count) { - hdb_keyset *tmp; - - keyset_count *= 2; - tmp = realloc(hist_keys->val, - keyset_count * sizeof (*hist_keys->val)); - if (tmp == NULL) { - ret = ENOMEM; - goto out; - } - hist_keys->val = tmp; - } - } - - k = &keyset->keys.val[keyset->keys.len]; - - if (key_count != 0) - keyset->keys.len += key_count; - -done: - memset(k, 0, sizeof (*k)); - k->mkvno = malloc(sizeof(*k->mkvno)); - if (k->mkvno == NULL) { - ret = ENOMEM; - goto out; - } - *k->mkvno = 1; - *out = k; - -out: - if (ret && !is_hist) - entry->keys.len--; - if (is_hist) - free_HDB_extension(&ext); - return ret; -} - /** * This function takes a key from a krb5_storage from an MIT KDB encoded @@ -342,6 +215,13 @@ mdb_keyvalue2key(krb5_context context, hdb_entry *entry, krb5_storage *sp, uint1 uint16_t u16, type; krb5_error_code ret; + k->mkvno = malloc(sizeof(*k->mkvno)); + if (k->mkvno == NULL) { + ret = ENOMEM; + goto out; + } + *k->mkvno = 1; + for (i = 0; i < version; i++) { CHECK(ret = krb5_ret_uint16(sp, &type)); CHECK(ret = krb5_ret_uint16(sp, &u16)); @@ -400,7 +280,6 @@ mdb_keyvalue2key(krb5_context context, hdb_entry *entry, krb5_storage *sp, uint1 out: free_Key(k); - memset(k, 0, sizeof (*k)); return ret; } @@ -408,19 +287,28 @@ out: /** * This function parses an MIT krb5 encoded KDB entry and fills in the * given HDB entry with it. + * + * @param context krb5_context + * @param data Encoded MIT KDB entry + * @param target_kvno Desired kvno, or 0 for the entry's current kvno + * @param entry Desired kvno, or 0 for the entry's current kvno */ static krb5_error_code -mdb_value2entry(krb5_context context, krb5_data *data, krb5_kvno kvno, hdb_entry *entry) +mdb_value2entry(krb5_context context, krb5_data *data, krb5_kvno target_kvno, + hdb_entry *entry) { krb5_error_code ret; krb5_storage *sp; - Key *k; + Key k; krb5_kvno key_kvno; uint32_t u32; uint16_t u16, num_keys, num_tl; - size_t i, j; + size_t i; char *p; + memset(&k, 0, sizeof (k)); + memset(entry, 0, sizeof(*entry)); + sp = krb5_storage_from_data(data); if (sp == NULL) { krb5_set_error_message(context, ENOMEM, "out of memory"); @@ -442,7 +330,8 @@ mdb_value2entry(krb5_context context, krb5_data *data, krb5_kvno kvno, hdb_entry * XXX But... surely we ought to log about this extra data, or skip * it, or something, in case anyone has MIT KDBs with ancient * entries in them... Logging would allow the admin to know which - * entries to dump with MIT krb5's kdb5_util. + * entries to dump with MIT krb5's kdb5_util. But logging would be + * noisy. For now we do nothing. */ CHECK(ret = krb5_ret_uint16(sp, &u16)); if (u16 != KDB_V1_BASE_LENGTH) { ret = EINVAL; goto out; } @@ -540,7 +429,6 @@ mdb_value2entry(krb5_context context, krb5_data *data, krb5_kvno kvno, hdb_entry * That's right... hold that gag reflex, you can do it. */ for (i = 0; i < num_keys; i++) { - int keep = 0; uint16_t version; CHECK(ret = krb5_ret_uint16(sp, &u16)); @@ -548,70 +436,81 @@ mdb_value2entry(krb5_context context, krb5_data *data, krb5_kvno kvno, hdb_entry CHECK(ret = krb5_ret_uint16(sp, &u16)); key_kvno = u16; - /* - * First time through, and until we find one matching key, - * entry->kvno == 0. - */ - if ((entry->kvno < key_kvno) && (kvno == 0 || kvno == key_kvno)) { + ret = mdb_keyvalue2key(context, entry, sp, version, &k); + if (ret) + goto out; + if (k.key.keytype == 0 || k.key.keyvalue.length == 0) { /* - * Found a higher kvno than earlier, we aren't looking for - * any particular kvno, so save the previously saved keys as - * historical keys. + * Older MIT KDBs may have enctype 0 / length 0 keys. We + * ignore these. */ - keep = 1; + free_Key(&k); + continue; + } - /* Get an array of Keys to save the current keyset into */ - ret = get_entry_key_location(context, entry, entry->kvno, TRUE, - entry->keys.len, &k); - - for (j = 0; j < entry->keys.len; j++) - copy_Key(&entry->keys.val[j], &k[j]); - - /* Change the entry's current kvno */ + if ((target_kvno == 0 && entry->kvno < key_kvno) || + (target_kvno == key_kvno && entry->kvno != target_kvno)) { + /* + * MIT's KDB doesn't keep track of kvno. The highest kvno + * is the current kvno, and we just found a new highest + * kvno or the desired kvno. + * + * Note that there's no guarantee of any key ordering, but + * generally MIT KDB entries have keys in strictly + * descending kvno order. + * + * XXX We do assume that keys are clustered by kvno. If + * not, then bad. It might be possible to construct + * non-clustered keys via the kadm5 API. It wouldn't be + * hard to cope with this, since if it happens the worst + * that will happen is that some of the current keys can be + * found in the history extension, and we could just pull + * them back out in that case. + */ + ret = hdb_add_current_keys_to_history(context, entry); + if (ret) + goto out; + free_Keys(&entry->keys); + ret = add_Keys(&entry->keys, &k); + free_Key(&k); + if (ret) + goto out; entry->kvno = key_kvno; + continue; + } - for (j = 0; j < entry->keys.len; j++) - free_Key(&entry->keys.val[j]); - free(entry->keys.val); - entry->keys.len = 0; - entry->keys.val = NULL; - } else if (entry->kvno == key_kvno) - /* Accumulate keys */ - keep = 1; - - if (keep) { - ret = get_entry_key_location(context, entry, key_kvno, - FALSE, 0, &k); - if (ret) - goto out; - - ret = mdb_keyvalue2key(context, entry, sp, version, k); - if (ret) - goto out; - } else { + if (entry->kvno == key_kvno) { /* - * XXX For now we skip older kvnos, but we should extract - * them... XXX Finish. + * Note that if key_kvno == 0 and target_kvno == 0 then we + * end up adding those keys here. Yeah, kvno 0 is very + * special for us, but just in case, we keep such keys. */ - ret = get_entry_key_location(context, entry, key_kvno, TRUE, 0, &k); + ret = add_Keys(&entry->keys, &k); + free_Key(&k); if (ret) goto out; - ret = mdb_keyvalue2key(context, entry, sp, version, k); + entry->kvno = key_kvno; + } else { + ret = hdb_add_history_key(context, entry, key_kvno, &k); if (ret) goto out; + free_Key(&k); } } - if (entry->kvno == 0 && kvno != 0) { - ret = HDB_ERR_NOT_FOUND_HERE; + if (target_kvno != 0 && entry->kvno != target_kvno) { + ret = HDB_ERR_KVNO_NOT_FOUND; goto out; } return 0; - out: + +out: if (ret == HEIM_ERR_EOF) /* Better error code than "end of file" */ ret = HEIM_ERR_BAD_HDBENT_ENCODING; + free_hdb_entry(entry); + free_Key(&k); return ret; } diff --git a/lib/hdb/hdb_err.et b/lib/hdb/hdb_err.et index 0bdcb385f..cbaa01d0d 100644 --- a/lib/hdb/hdb_err.et +++ b/lib/hdb/hdb_err.et @@ -27,5 +27,6 @@ error_code MANDATORY_OPTION, "Entry contains unknown mandatory extension" error_code NO_WRITE_SUPPORT, "HDB backend doesn't contain write support" error_code NOT_FOUND_HERE, "The secret for this entry is not replicated to this database" error_code MISUSE, "Incorrect use of the API" +error_code KVNO_NOT_FOUND, "Entry key version number not found" end diff --git a/lib/hdb/keys.c b/lib/hdb/keys.c index 0bc3392fb..ddcc92c7a 100644 --- a/lib/hdb/keys.c +++ b/lib/hdb/keys.c @@ -211,9 +211,11 @@ hdb_add_current_keys_to_history(krb5_context context, hdb_entry *entry) krb5_boolean replace = FALSE; krb5_error_code ret; HDB_extension *ext; - hdb_keyset newkey; + hdb_keyset newkeyset; time_t newtime; + if (entry->keys.len == 0) + return 0; /* nothing to do */ ext = hdb_find_extension(entry, choice_HDB_extension_data_hist_keys); if (ext == NULL) { @@ -228,17 +230,16 @@ hdb_add_current_keys_to_history(krb5_context context, hdb_entry *entry) /* * Copy in newest old keyset */ - ret = hdb_entry_get_pw_change_time(entry, &newtime); if (ret) goto out; - memset(&newkey, 0, sizeof(newkey)); - newkey.keys = entry->keys; - newkey.kvno = entry->kvno; - newkey.set_time = &newtime; + memset(&newkeyset, 0, sizeof(newkeyset)); + newkeyset.keys = entry->keys; + newkeyset.kvno = entry->kvno; + newkeyset.set_time = &newtime; - ret = add_HDB_Ext_KeySet(&ext->data.u.hist_keys, &newkey); + ret = add_HDB_Ext_KeySet(&ext->data.u.hist_keys, &newkeyset); if (ret) goto out; @@ -257,6 +258,124 @@ hdb_add_current_keys_to_history(krb5_context context, hdb_entry *entry) return ret; } +/** + * This function adds a key to an HDB entry's key history. + * + * @param context Context + * @param entry HDB entry + * @param kvno Key version number of the key to add to the history + * @param key The Key to add + */ +krb5_error_code +hdb_add_history_key(krb5_context context, hdb_entry *entry, krb5_kvno kvno, Key *key) +{ + size_t i; + hdb_keyset keyset; + HDB_Ext_KeySet *hist_keys; + HDB_extension ext; + HDB_extension *extp; + krb5_error_code ret; + + memset(&keyset, 0, sizeof (keyset)); + memset(&ext, 0, sizeof (ext)); + + extp = hdb_find_extension(entry, choice_HDB_extension_data_hist_keys); + if (extp == NULL) { + ext.data.element = choice_HDB_extension_data_hist_keys; + extp = &ext; + } + + hist_keys = &extp->data.u.hist_keys; + + for (i = 0; i < hist_keys->len; i++) { + if (hist_keys->val[i].kvno == kvno) { + ret = add_Keys(&hist_keys->val[i].keys, key); + goto out; + } + } + + keyset.kvno = kvno; + ret = add_Keys(&keyset.keys, key); + if (ret) + goto out; + ret = add_HDB_Ext_KeySet(hist_keys, &keyset); + if (ret) + goto out; + if (extp == &ext) { + ret = hdb_replace_extension(context, entry, &ext); + if (ret) + goto out; + } + +out: + free_hdb_keyset(&keyset); + free_HDB_extension(&ext); + return ret; +} + + +/** + * This function changes an hdb_entry's kvno, swapping the current key + * set with a historical keyset. If no historical keys are found then + * an error is returned (the caller can still set entry->kvno directly). + * + * @param context krb5_context + * @param new_kvno New kvno for the entry + * @param entry hdb_entry to modify + */ +krb5_error_code +hdb_change_kvno(krb5_context context, krb5_kvno new_kvno, hdb_entry *entry) +{ + HDB_extension ext; + HDB_extension *extp; + hdb_keyset keyset; + HDB_Ext_KeySet *hist_keys; + size_t i; + int found = 0; + krb5_error_code ret; + + if (entry->kvno == new_kvno) + return 0; + + extp = hdb_find_extension(entry, choice_HDB_extension_data_hist_keys); + if (extp == NULL) { + memset(&ext, 0, sizeof (ext)); + ext.data.element = choice_HDB_extension_data_hist_keys; + extp = &ext; + } + + memset(&keyset, 0, sizeof (keyset)); + hist_keys = &extp->data.u.hist_keys; + for (i = 0; i < hist_keys->len; i++) { + if (hist_keys->val[i].kvno == new_kvno) { + found = 1; + ret = copy_hdb_keyset(&hist_keys->val[i], &keyset); + if (ret) + goto out; + ret = remove_HDB_Ext_KeySet(hist_keys, i); + if (ret) + goto out; + break; + } + } + + if (!found) + return HDB_ERR_KVNO_NOT_FOUND; + + ret = hdb_add_current_keys_to_history(context, entry); + if (ret) + goto out; + + /* Note: we do nothing with keyset.set_time */ + entry->kvno = new_kvno; + entry->keys = keyset.keys; /* shortcut */ + memset(&keyset.keys, 0, sizeof (keyset.keys)); + +out: + free_hdb_keyset(&keyset); + return ret; +} + static krb5_error_code add_enctype_to_key_set(Key **key_set, size_t *nkeyset, diff --git a/lib/hdb/version-script.map b/lib/hdb/version-script.map index 9f05d7fbe..3b68296f5 100644 --- a/lib/hdb/version-script.map +++ b/lib/hdb/version-script.map @@ -5,6 +5,7 @@ HEIMDAL_HDB_1.0 { encode_hdb_keyset; hdb_add_master_key; hdb_add_current_keys_to_history; + hdb_change_kvno; hdb_check_db_format; hdb_clear_extension; hdb_clear_master_key; diff --git a/lib/kadm5/ent_setup.c b/lib/kadm5/ent_setup.c index 4beb024a3..deb6e64fd 100644 --- a/lib/kadm5/ent_setup.c +++ b/lib/kadm5/ent_setup.c @@ -178,26 +178,13 @@ _kadm5_setup_entry(kadm5_server_context *context, } } if(mask & KADM5_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; - } + && (princ_mask & KADM5_KVNO)) { + krb5_error_code ret; + + ret = hdb_change_kvno(context->context, princ->kvno, &ent->entry); + if (ret && ret != HDB_ERR_KVNO_NOT_FOUND) + return ret; + ent->entry.kvno = princ->kvno; /* force it */ } if(mask & KADM5_MAX_RLIFE) { if(princ_mask & KADM5_MAX_RLIFE) {