More review changes:

Have both LDAP and DB2 back ends been tried with Will's new code?  Looks
like some default routines like kdb_def_get_mkey_list won't do anything; is
that okay?

    Done but not tested.

"XXX" comments in kdc/extern.h and elsewhere need to be looked into,
obviously.

    Almost done (working on the mkey_convert issue).

git-svn-id: svn://anonsvn.mit.edu/krb5/branches/mkey_migrate@21806 dc483132-0cff-0310-8789-dd5450dbe970

3 files changed, 200 insertions, 186 deletions

diff --git a/src/kadmin/dbutil/dump.c b/src/kadmin/dbutil/dump.c
index 68a8270..f5b6cd6 100644
--- a/src/kadmin/dbutil/dump.c
+++ b/src/kadmin/dbutil/dump.c
@@ -178,6 +178,7 @@ extern krb5_boolean	dbactive;
 extern int		exit_status;
 extern krb5_context	util_context;
 extern kadm5_config_params global_params;
+extern krb5_keylist_node *master_keylist;
 
 /* Strings */
 
@@ -274,48 +275,49 @@ static krb5_error_code master_key_convert(context, db_entry)
 
     is_mkey = krb5_principal_compare(context, master_princ, db_entry->princ);
 
-    /* XXX WAF: need to fix this! */
-    if (is_mkey && db_entry->n_key_data != 1)
-	    fprintf(stderr,
-		    "Master key db entry has %d keys, expecting only 1!\n",
-		    db_entry->n_key_data);
-    for (i=0; i < db_entry->n_key_data; i++) {
-	key_data = &db_entry->key_data[i];
-	if (key_data->key_data_length == 0)
-	    continue;
-	retval = krb5_dbekd_decrypt_key_data(context, &master_keyblock,
-					     key_data, &v5plainkey,
-					     &keysalt);
-	if (retval)
-		return retval;
-
-	memset(&new_key_data, 0, sizeof(new_key_data));
-
-	if (is_mkey) {
-		key_ptr = &new_master_keyblock;
-		/* override mkey princ's kvno */
-		if (global_params.mask & KADM5_CONFIG_KVNO)
-			kvno = global_params.kvno;
-		else
-			kvno = (krb5_kvno) key_data->key_data_kvno;
-	} else {
-		key_ptr = &v5plainkey;
-		kvno = (krb5_kvno) key_data->key_data_kvno;
-	}
-
-	retval = krb5_dbekd_encrypt_key_data(context, &new_master_keyblock,
-					     key_ptr, &keysalt,
-					     (int) kvno,
-					     &new_key_data);
-	if (retval)
-		return retval;
-	krb5_free_keyblock_contents(context, &v5plainkey);
-	for (j = 0; j < key_data->key_data_ver; j++) {
-	    if (key_data->key_data_length[j]) {
-		free(key_data->key_data_contents[j]);
-	    }
-	}
-	*key_data = new_key_data;
+    if (is_mkey) {
+        retval = add_new_mkey(context, db_entry, &new_master_keyblock);
+        if (retval)
+            return retval;
+    } else {
+        for (i=0; i < db_entry->n_key_data; i++) {
+            key_data = &db_entry->key_data[i];
+            if (key_data->key_data_length == 0)
+                continue;
+            retval = krb5_dbekd_decrypt_key_data(context, &master_keyblock,
+                                                 key_data, &v5plainkey,
+                                                 &keysalt);
+            if (retval)
+                    return retval;
+
+            memset(&new_key_data, 0, sizeof(new_key_data));
+
+            if (is_mkey) {
+                    key_ptr = &new_master_keyblock;
+                    /* override mkey princ's kvno */
+                    if (global_params.mask & KADM5_CONFIG_KVNO)
+                            kvno = global_params.kvno;
+                    else
+                            kvno = (krb5_kvno) key_data->key_data_kvno;
+            } else {
+                    key_ptr = &v5plainkey;
+                    kvno = (krb5_kvno) key_data->key_data_kvno;
+            }
+
+            retval = krb5_dbekd_encrypt_key_data(context, &new_master_keyblock,
+                                                 key_ptr, &keysalt,
+                                                 (int) kvno,
+                                                 &new_key_data);
+            if (retval)
+                    return retval;
+            krb5_free_keyblock_contents(context, &v5plainkey);
+            for (j = 0; j < key_data->key_data_ver; j++) {
+                if (key_data->key_data_length[j]) {
+                    free(key_data->key_data_contents[j]);
+                }
+            }
+            *key_data = new_key_data;
+        }
     }
     return 0;
 }
diff --git a/src/kadmin/dbutil/kdb5_mkey.c b/src/kadmin/dbutil/kdb5_mkey.c
index 57d469b..d127b41 100644
--- a/src/kadmin/dbutil/kdb5_mkey.c
+++ b/src/kadmin/dbutil/kdb5_mkey.c
@@ -34,6 +34,130 @@ static char *strdate(krb5_timestamp when)
     return out;
 }
 
+krb5_error_code
+add_new_mkey(krb5_context context, krb5_db_entry *master_entry, krb5_keyblock *new_mkey)
+{
+    krb5_error_code retval = 0;
+    int old_key_data_count, i;
+    krb5_kvno old_kvno, new_mkey_kvno;
+    krb5_keyblock new_mkeyblock;
+    krb5_key_data tmp_key_data, *old_key_data;
+    krb5_enctype new_master_enctype = ENCTYPE_UNKNOWN;
+    krb5_mkey_aux_node  *mkey_aux_data_head = NULL, **mkey_aux_data,
+                        *cur_mkey_aux_data, *next_mkey_aux_data;
+    krb5_keylist_node  *keylist_node;
+
+    /* First save the old keydata */
+    old_kvno = krb5_db_get_key_data_kvno(context, master_entry->n_key_data,
+					 master_entry->key_data);
+    old_key_data_count = master_entry->n_key_data;
+    old_key_data = master_entry->key_data;
+
+    /* alloc enough space to hold new and existing key_data */
+    /*
+     * The encrypted key is malloc'ed by krb5_dbekd_encrypt_key_data and
+     * krb5_key_data key_data_contents is a pointer to this key.  Using some
+     * logic from master_key_convert().
+     */
+    master_entry->key_data = (krb5_key_data *) malloc(sizeof(krb5_key_data) *
+                                                     (old_key_data_count + 1));
+    if (master_entry->key_data == NULL)
+        return (ENOMEM);
+
+    memset((char *) master_entry->key_data, 0,
+           sizeof(krb5_key_data) * (old_key_data_count + 1));
+    master_entry->n_key_data = old_key_data_count + 1;
+
+    new_mkey_kvno = old_kvno + 1;
+    /* deal with wrapping? */
+    if (new_mkey_kvno == 0)
+        new_mkey_kvno = 1; /* knvo must not be 0 as this is special value (IGNORE_VNO) */
+
+    /* Note, mkey does not have salt */
+    /* add new mkey encrypted with itself to mkey princ entry */
+    if ((retval = krb5_dbekd_encrypt_key_data(context, new_mkey,
+                                              new_mkey, NULL, 
+                                              (int) new_mkey_kvno,
+                                              &master_entry->key_data[0]))) {
+        return (retval);
+    }
+
+    /*
+     * Need to decrypt old keys with the current mkey which is in the global
+     * master_keyblock and encrypt those keys with the latest mkey.  And while
+     * the old keys are being decrypted, use those to create the
+     * KRB5_TL_MKEY_AUX entries which store the latest mkey encrypted by one of
+     * the older mkeys.
+     *
+     * The new mkey is followed by existing keys.
+     *
+     * First, set up for creating a krb5_mkey_aux_node list which will be used
+     * to update the mkey aux data for the mkey princ entry.
+     */
+    mkey_aux_data_head = (krb5_mkey_aux_node *) malloc(sizeof(krb5_mkey_aux_node));
+    if (mkey_aux_data_head == NULL) {
+        retval = ENOMEM;
+        goto clean_n_exit;
+    }
+    memset(mkey_aux_data_head, 0, sizeof(krb5_mkey_aux_node));
+    mkey_aux_data = &mkey_aux_data_head;
+
+    for (keylist_node = master_keylist, i = 1; keylist_node != NULL;
+         keylist_node = keylist_node->next, i++) {
+
+        /*
+         * Create a list of krb5_mkey_aux_node nodes.  One node contains the new
+         * mkey encrypted by an old mkey and the old mkey's kvno (one node per
+         * old mkey).
+         */
+        if (*mkey_aux_data == NULL) {
+            /* *mkey_aux_data points to next field of previous node */
+            *mkey_aux_data = (krb5_mkey_aux_node *) malloc(sizeof(krb5_mkey_aux_node));
+            if (*mkey_aux_data == NULL) {
+                retval = ENOMEM;
+                goto clean_n_exit;
+            }
+            memset(*mkey_aux_data, 0, sizeof(krb5_mkey_aux_node));
+        }
+
+        memset(&tmp_key_data, 0, sizeof(tmp_key_data));
+        /* encrypt the new mkey with the older mkey */
+        retval = krb5_dbekd_encrypt_key_data(context, &keylist_node->keyblock,
+                                             new_mkey,
+                                             NULL, /* no keysalt */
+                                             (int) new_mkey_kvno,
+                                             &tmp_key_data);
+        if (retval)
+            goto clean_n_exit;
+
+        (*mkey_aux_data)->latest_mkey = tmp_key_data;
+        (*mkey_aux_data)->mkey_kvno = keylist_node->kvno;
+        mkey_aux_data = &((*mkey_aux_data)->next);
+
+        /*
+         * Store old key in master_entry keydata past the new mkey
+         */
+        retval = krb5_dbekd_encrypt_key_data(context, new_mkey,
+                                             &keylist_node->keyblock,
+                                             NULL, /* no keysalt */
+                                             (int) keylist_node->kvno,
+                                             &master_entry->key_data[i]);
+        if (retval)
+            goto clean_n_exit;
+    }
+    assert(i == old_key_data_count + 1);
+
+    if ((retval = krb5_dbe_update_mkey_aux(context, master_entry,
+                                           mkey_aux_data_head))) {
+        goto clean_n_exit;
+    }
+
+clean_n_exit:
+    if (mkey_aux_data_head)
+        krb5_dbe_free_mkey_aux_list(context, mkey_aux_data_head);
+    return (retval);
+}
+
 void
 kdb5_add_mkey(int argc, char *argv[])
 {
@@ -47,7 +171,7 @@ kdb5_add_mkey(int argc, char *argv[])
     krb5_boolean more = 0;
     krb5_data pwd;
     krb5_kvno old_kvno, new_mkey_kvno;
-    krb5_keyblock new_master_keyblock;
+    krb5_keyblock new_mkeyblock;
     krb5_key_data tmp_key_data, *old_key_data;
     krb5_enctype new_master_enctype = ENCTYPE_UNKNOWN;
     char *new_mkey_password;
@@ -95,8 +219,7 @@ kdb5_add_mkey(int argc, char *argv[])
     }
 
     retval = krb5_db_get_principal(util_context, master_princ, &master_entry,
-                                   &nentries,
-                                   &more);
+                                   &nentries, &more);
     if (retval != 0) {
         com_err(progname, retval, "while setting up master key name");
         exit_status++;
@@ -137,129 +260,16 @@ kdb5_add_mkey(int argc, char *argv[])
     }
 
     retval = krb5_c_string_to_key(util_context, new_master_enctype, 
-                                  &pwd, &master_salt, &new_master_keyblock);
+                                  &pwd, &master_salt, &new_mkeyblock);
     if (retval) {
         com_err(progname, retval, "while transforming master key from password");
         exit_status++;
         return;
     }
 
-    /* First save the old keydata */
-    old_kvno = krb5_db_get_key_data_kvno(util_context, master_entry.n_key_data,
-					 master_entry.key_data);
-    old_key_data_count = master_entry.n_key_data;
-    old_key_data = master_entry.key_data;
-
-    /* alloc enough space to hold new and existing key_data */
-    /*
-     * The encrypted key is malloc'ed by krb5_dbekd_encrypt_key_data and
-     * krb5_key_data key_data_contents is a pointer to this key.  Using some
-     * logic from master_key_convert().
-     */
-    master_entry.key_data = (krb5_key_data *) malloc(sizeof(krb5_key_data) *
-                                                     (old_key_data_count + 1));
-    if (master_entry.key_data == NULL) {
-        com_err(progname, ENOMEM, "while adding new master key");
-        exit_status++;
-        return;
-    }
-    memset((char *) master_entry.key_data, 0,
-           sizeof(krb5_key_data) * (old_key_data_count + 1));
-    master_entry.n_key_data = old_key_data_count + 1;
-
-    new_mkey_kvno = old_kvno + 1;
-    /* deal with wrapping? */
-    if (new_mkey_kvno == 0)
-        new_mkey_kvno = 1; /* knvo must not be 0 as this is special value (IGNORE_VNO) */
-
-    /* Note, mkey does not have salt */
-    /* add new mkey encrypted with itself to mkey princ entry */
-    if ((retval = krb5_dbekd_encrypt_key_data(util_context, &new_master_keyblock,
-                                              &new_master_keyblock, NULL, 
-                                              (int) new_mkey_kvno,
-                                              master_entry.key_data))) {
-        com_err(progname, retval, "while creating new master key");
-        exit_status++;
-        return;
-    }
-
-    /*
-     * Need to decrypt old keys with the current mkey which is in the global
-     * master_keyblock and encrypt those keys with the latest mkey.  And while
-     * the old keys are being decrypted, use those to create the
-     * KRB5_TL_MKEY_AUX entries which store the latest mkey encrypted by one of
-     * the older mkeys.
-     *
-     * The new mkey is followed by existing keys.
-     *
-     * First, set up for creating a krb5_mkey_aux_node list which will be used
-     * to update the mkey aux data for the mkey princ entry.
-     */
-    mkey_aux_data_head = (krb5_mkey_aux_node *) malloc(sizeof(krb5_mkey_aux_node));
-    if (mkey_aux_data_head == NULL) {
-        com_err(progname, ENOMEM, "while creating mkey_aux_data");
-        exit_status++;
-        return;
-    }
-    memset(mkey_aux_data_head, 0, sizeof(krb5_mkey_aux_node));
-    mkey_aux_data = &mkey_aux_data_head;
-
-    for (keylist_node = master_keylist, i = 1; keylist_node != NULL;
-         keylist_node = keylist_node->next, i++) {
-
-        /*
-         * Create a list of krb5_mkey_aux_node nodes.  One node contains the new
-         * mkey encrypted by an old mkey and the old mkey's kvno (one node per
-         * old mkey).
-         */
-        if (*mkey_aux_data == NULL) {
-            /* *mkey_aux_data points to next field of previous node */
-            *mkey_aux_data = (krb5_mkey_aux_node *) malloc(sizeof(krb5_mkey_aux_node));
-            if (*mkey_aux_data == NULL) {
-                com_err(progname, ENOMEM, "while creating mkey_aux_data");
-                exit_status++;
-                return;
-            }
-            memset(*mkey_aux_data, 0, sizeof(krb5_mkey_aux_node));
-        }
-
-        memset(&tmp_key_data, 0, sizeof(tmp_key_data));
-        /* encrypt the new mkey with the older mkey */
-        retval = krb5_dbekd_encrypt_key_data(util_context, &keylist_node->keyblock,
-                                             &new_master_keyblock,
-                                             NULL, /* no keysalt */
-                                             (int) new_mkey_kvno,
-                                             &tmp_key_data);
-        if (retval) {
-            com_err(progname, retval, "while encrypting master keys");
-            exit_status++;
-            return;
-        }
-
-        (*mkey_aux_data)->latest_mkey = tmp_key_data;
-        (*mkey_aux_data)->mkey_kvno = keylist_node->kvno;
-        mkey_aux_data = &((*mkey_aux_data)->next);
-
-        /*
-         * Store old key in master_entry keydata past the new mkey
-         */
-        retval = krb5_dbekd_encrypt_key_data(util_context, &new_master_keyblock,
-                                             &keylist_node->keyblock,
-                                             NULL, /* no keysalt */
-                                             (int) keylist_node->kvno,
-                                             &master_entry.key_data[i]);
-        if (retval) {
-            com_err(progname, retval, "while encrypting master keys");
-            exit_status++;
-            return;
-        }
-    }
-
-    assert(i == old_key_data_count + 1);
-
-    if ((retval = krb5_dbe_update_mkey_aux(util_context, &master_entry,
-                                           mkey_aux_data_head))) {
-        com_err(progname, retval, "while updating mkey aux data");
+    retval = add_new_mkey(util_context, &master_entry, &new_mkeyblock);
+    if (retval) {
+        com_err(progname, retval, "adding new master key to master principal");
         exit_status++;
         return;
     }
@@ -286,11 +296,11 @@ kdb5_add_mkey(int argc, char *argv[])
 
     if (do_stash) {
         retval = krb5_db_store_master_key(util_context,
-            global_params.stash_file,
-            master_princ,
-            new_mkey_kvno,
-            &new_master_keyblock,
-            mkey_password);
+                                          global_params.stash_file,
+                                          master_princ,
+                                          new_mkey_kvno,
+                                          &new_mkeyblock,
+                                          mkey_password);
         if (retval) {
             com_err(progname, errno, "while storing key");
             printf("Warning: couldn't stash master key.\n");
@@ -300,8 +310,8 @@ kdb5_add_mkey(int argc, char *argv[])
     (void) krb5_db_fini(util_context);
     zap((char *)master_keyblock.contents, master_keyblock.length);
     free(master_keyblock.contents);
-    zap((char *)new_master_keyblock.contents, new_master_keyblock.length);
-    free(new_master_keyblock.contents);
+    zap((char *)new_mkeyblock.contents, new_mkeyblock.length);
+    free(new_mkeyblock.contents);
     if (pw_str) {
         zap(pw_str, pw_size);
         free(pw_str);
@@ -309,12 +319,6 @@ kdb5_add_mkey(int argc, char *argv[])
     free(master_salt.data);
     free(mkey_fullname);
 
-    for (cur_mkey_aux_data = mkey_aux_data_head; cur_mkey_aux_data != NULL;
-         cur_mkey_aux_data = next_mkey_aux_data) {
-        next_mkey_aux_data = cur_mkey_aux_data->next;
-        krb5_dbe_free_key_data_contents(util_context, &(cur_mkey_aux_data->latest_mkey));
-        free(cur_mkey_aux_data);
-    }
     return;
 }
 
@@ -329,19 +333,33 @@ kdb5_use_mkey(int argc, char *argv[])
                       *prev_actkvno, *cur_actkvno;
     krb5_db_entry master_entry;
     int   nentries = 0;
-    krb5_boolean more = 0;
+    krb5_boolean more = 0, found;
+    krb5_keylist_node  *keylist_node;
 
     if (argc < 2 || argc > 3) {
         /* usage calls exit */
         usage();
     }
 
-    /* use_kvno = (int) strtol(argv[0], (char **)NULL, 10); */
     use_kvno = atoi(argv[1]);
     if (use_kvno == 0) {
         com_err(progname, EINVAL, ": 0 is an invalid KVNO value.");
         exit_status++;
         return;
+    } else {
+        /* verify use_kvno is valid */
+        for (keylist_node = master_keylist, found = FALSE; keylist_node != NULL;
+             keylist_node = keylist_node->next) {
+            if (use_kvno == keylist_node->kvno) {
+                found = TRUE;
+                break;
+            }
+        }
+        if (!found) {
+            com_err(progname, EINVAL, ": %d is an invalid KVNO value.", use_kvno);
+            exit_status++;
+            return;
+        }
     }
 
     if ((retval = krb5_timeofday(util_context, &now))) {
@@ -360,7 +378,6 @@ kdb5_use_mkey(int argc, char *argv[])
      * Need to:
      *
      * 1. get mkey princ
-     * 2. verify that mprinc actually has a mkey with the new actkvno
      * 2. get krb5_actkvno_node list
      * 3. add use_kvno to actkvno list (sorted in right spot)
      * 4. update mkey princ's tl data
@@ -385,8 +402,6 @@ kdb5_use_mkey(int argc, char *argv[])
         return;
     }
 
-    /* XXX WAF: verify that the provided kvno is valid */
-
     retval = krb5_dbe_lookup_actkvno(util_context, &master_entry, &actkvno_list);
     if (retval != 0) {
         com_err(progname, retval, "while setting up master key name");
@@ -478,12 +493,7 @@ kdb5_use_mkey(int argc, char *argv[])
     /* clean up */
     (void) krb5_db_fini(util_context);
     free(mkey_fullname);
-    for (cur_actkvno = actkvno_list; cur_actkvno != NULL;) {
-
-        prev_actkvno = cur_actkvno;
-        cur_actkvno = cur_actkvno->next;
-        free(prev_actkvno);
-    }
+    krb5_dbe_free_actkvno_list(util_context, actkvno_list);
     return;
 }
 
diff --git a/src/kadmin/dbutil/kdb5_util.h b/src/kadmin/dbutil/kdb5_util.h
index cc69c90..dd74654 100644
--- a/src/kadmin/dbutil/kdb5_util.h
+++ b/src/kadmin/dbutil/kdb5_util.h
@@ -88,5 +88,7 @@ extern void update_ok_file (char *file_name);
 
 extern int kadm5_create (kadm5_config_params *params);
 
+extern krb5_error_code add_new_mkey(krb5_context, krb5_db_entry *, krb5_keyblock *);
+
 void usage (void);