Minimal support for updating history key

Add minimal support for re-randomizing the history key:

* cpw -randkey kadmin/history now works, but creates only one key.
* cpw -randkey -keepold kadmin/history still fails.
* libkadm5 no longer caches the history key.  Performance impact
  is minimal since password changes are not common.
* randkey no longer checks the newly randomized key against old keys,
  and the disabled code to do so in setkey/setv4key is gone, so now
  only kadm5_chpass_principal_3 accesses the password history.

ticket: 6660
target_version: 1.8
tags: pullup

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@23716 dc483132-0cff-0310-8789-dd5450dbe970

5 files changed, 113 insertions, 131 deletions

diff --git a/src/lib/kadm5/server_internal.h b/src/lib/kadm5/server_internal.h
index 7f58750..3276b27 100644
--- a/src/lib/kadm5/server_internal.h
+++ b/src/lib/kadm5/server_internal.h
@@ -24,6 +24,15 @@
 #include    <kadm5/admin.h>
 #include    "admin_internal.h"
 
+/*
+ * This is the history key version for a newly created DB.  We use this value
+ * for principals which have no password history yet to avoid having to look up
+ * the history key.  Values other than 2 will cause compatibility issues with
+ * pre-1.8 libkadm5 code; the older code will reject key changes when it sees
+ * an unexpected value of admin_history_kvno.
+ */
+#define INITIAL_HIST_KVNO 2
+
 typedef struct _kadm5_server_handle_t {
     krb5_ui_4       magic_number;
     krb5_ui_4       struct_version;
@@ -64,6 +73,9 @@ krb5_error_code     kdb_init_master(kadm5_server_handle_t handle,
                                     char *r, int from_keyboard);
 krb5_error_code     kdb_init_hist(kadm5_server_handle_t handle,
                                   char *r);
+krb5_error_code     kdb_get_hist_key(kadm5_server_handle_t handle,
+                                     krb5_keyblock *hist_keyblock,
+                                     krb5_kvno *hist_kvno);
 krb5_error_code     kdb_get_entry(kadm5_server_handle_t handle,
                                   krb5_principal principal, krb5_db_entry *kdb,
                                   osa_princ_ent_rec *adb);
diff --git a/src/lib/kadm5/srv/libkadm5srv_mit.exports b/src/lib/kadm5/srv/libkadm5srv_mit.exports
index d8d3b22..7ba5c1a 100644
--- a/src/lib/kadm5/srv/libkadm5srv_mit.exports
+++ b/src/lib/kadm5/srv/libkadm5srv_mit.exports
@@ -9,9 +9,6 @@ adb_policy_close
 adb_policy_init
 destroy_dict
 find_word
-hist_db
-hist_key
-hist_kvno
 hist_princ
 init_dict
 kadm5_set_use_password_server
diff --git a/src/lib/kadm5/srv/server_kdb.c b/src/lib/kadm5/srv/server_kdb.c
index e1ffca2..337c142 100644
--- a/src/lib/kadm5/srv/server_kdb.c
+++ b/src/lib/kadm5/srv/server_kdb.c
@@ -27,9 +27,6 @@ krb5_actkvno_node   *active_mkey_list = NULL;
 krb5_db_entry       master_db;
 
 krb5_principal      hist_princ;
-krb5_keyblock       hist_key;
-krb5_db_entry       hist_db;
-krb5_kvno           hist_kvno;
 
 /* much of this code is stolen from the kdc.  there should be some
    library code to deal with this. */
@@ -116,28 +113,16 @@ done:
  *      handle          (r) kadm5 api server handle
  *      r               (r) realm of history principal to use, or NULL
  *
- * Effects: This function sets the value of the following global
- * variables:
- *
- *      hist_princ      krb5_principal holding the history principal
- *      hist_db         krb5_db_entry of the history principal
- *      hist_key        krb5_keyblock holding the history principal's key
- *      hist_encblock   krb5_encrypt_block holding the procssed hist_key
- *      hist_kvno       the version number of the history key
- *
- * If the history principal does not already exist, this function
- * attempts to create it with kadm5_create_principal.  WARNING!
- * If the history principal is deleted and this function is executed
- * (by kadmind, or kadmin.local, or anything else with permission),
- * the principal will be assigned a new random key and all existing
- * password history information will become useless.
+ * Effects: This function sets the value of the hist_princ global variable.  If
+ * the history principal does not already exist, this function attempts to
+ * create it with kadm5_create_principal.
  */
 krb5_error_code kdb_init_hist(kadm5_server_handle_t handle, char *r)
 {
     int     ret = 0;
     char    *realm, *hist_name;
     krb5_key_salt_tuple ks[1];
-    krb5_keyblock *tmp_mkey;
+    krb5_db_entry kdb;
 
     if (r == NULL)  {
         if ((ret = krb5_get_default_realm(handle->context, &realm)))
@@ -154,78 +139,90 @@ krb5_error_code kdb_init_hist(kadm5_server_handle_t handle, char *r)
     if ((ret = krb5_parse_name(handle->context, hist_name, &hist_princ)))
         goto done;
 
-    if ((ret = kdb_get_entry(handle, hist_princ, &hist_db, NULL))) {
+    if ((ret = kdb_get_entry(handle, hist_princ, &kdb, NULL))) {
         kadm5_principal_ent_rec ent;
 
         if (ret != KADM5_UNK_PRINC)
             goto done;
 
-        /* try to create the principal */
-
+        /* Create the history principal. */
         memset(&ent, 0, sizeof(ent));
-
         ent.principal = hist_princ;
         ent.max_life = KRB5_KDB_DISALLOW_ALL_TIX;
         ent.attributes = 0;
-
-        /* this uses hist_kvno.  So we set it to 2, which will be the
-           correct value once the principal is created and randomized.
-           Of course, it doesn't make sense to keep a history for the
-           history principal, anyway. */
-
-        hist_kvno = 2;
         ks[0].ks_enctype = handle->params.enctype;
         ks[0].ks_salttype = KRB5_KDB_SALTTYPE_NORMAL;
         ret = kadm5_create_principal_3(handle, &ent,
                                        (KADM5_PRINCIPAL | KADM5_MAX_LIFE |
                                         KADM5_ATTRIBUTES),
-                                       1, ks,
-                                       "to-be-random");
+                                       1, ks, NULL);
         if (ret)
             goto done;
 
-        /* this won't let us randomize the hist_princ.  So we cheat. */
-
-        hist_princ = NULL;
-
+        /* For better compatibility with pre-1.8 libkadm5 code, we want the
+         * initial history kvno to be 2, so re-randomize it. */
         ret = kadm5_randkey_principal_3(handle, ent.principal, 0, 1, ks,
                                         NULL, NULL);
-
-        hist_princ = ent.principal;
-
         if (ret)
             goto done;
+    } else {
+        kdb_free_entry(handle, &kdb, NULL);
+    }
 
-        /* now read the newly-created kdb record out of the
-           database. */
+done:
+    free(hist_name);
+    if (r == NULL)
+        free(realm);
+    return ret;
+}
 
-        if ((ret = kdb_get_entry(handle, hist_princ, &hist_db, NULL)))
-            goto done;
+/*
+ * Function: kdb_get_hist_key
+ *
+ * Purpose: Fetches the current history key
+ *
+ * Arguments:
+ *
+ *      handle          (r) kadm5 api server handle
+ *      hist_keyblock   (w) keyblock to fill in with history key
+ *      hist_kvno       (w) kvno to fill in with history kvno
+ *
+ * Effects: This function looks up the history principal and retrieves the
+ * current history key and version.
+ */
+krb5_error_code
+kdb_get_hist_key(kadm5_server_handle_t handle, krb5_keyblock *hist_keyblock,
+                 krb5_kvno *hist_kvno)
+{
+    krb5_error_code ret;
+    krb5_db_entry kdb;
+    krb5_keyblock *mkey;
 
-    }
+    ret = kdb_get_entry(handle, hist_princ, &kdb, NULL);
+    if (ret)
+        return ret;
 
-    if (hist_db.n_key_data <= 0) {
-        krb5_set_error_message(handle->context, KRB5_KDB_NO_MATCHING_KEY,
+    if (kdb.n_key_data <= 0) {
+        ret = KRB5_KDB_NO_MATCHING_KEY;
+        krb5_set_error_message(handle->context, ret,
                                "History entry contains no key data");
-        return KRB5_KDB_NO_MATCHING_KEY;
+        goto done;
     }
 
-    ret = krb5_dbe_find_mkey(handle->context, master_keylist, &hist_db,
-                             &tmp_mkey);
+    ret = krb5_dbe_find_mkey(handle->context, master_keylist, &kdb,
+                             &mkey);
     if (ret)
         goto done;
 
-    ret = krb5_dbekd_decrypt_key_data(handle->context, tmp_mkey,
-                                      &hist_db.key_data[0], &hist_key, NULL);
+    ret = krb5_dbekd_decrypt_key_data(handle->context, mkey,
+                                      &kdb.key_data[0], hist_keyblock, NULL);
     if (ret)
         goto done;
 
-    hist_kvno = hist_db.key_data[0].key_data_kvno;
+    *hist_kvno = kdb.key_data[0].key_data_kvno;
 
 done:
-    free(hist_name);
-    if (r == NULL)
-        free(realm);
+    kdb_free_entry(handle, &kdb, NULL);
     return ret;
 }
 
@@ -289,7 +286,7 @@ kdb_get_entry(kadm5_server_handle_t handle,
                in), and when the entry is written, the admin
                data will get stored correctly. */
 
-            adb->admin_history_kvno = hist_kvno;
+            adb->admin_history_kvno = INITIAL_HIST_KVNO;
 
             return(ret);
         }
diff --git a/src/lib/kadm5/srv/svr_principal.c b/src/lib/kadm5/srv/svr_principal.c
index a01f072..3cb3b6b 100644
--- a/src/lib/kadm5/srv/svr_principal.c
+++ b/src/lib/kadm5/srv/svr_principal.c
@@ -36,10 +36,7 @@ extern  krb5_principal      hist_princ;
 extern  krb5_keyblock       master_keyblock;
 extern  krb5_keylist_node  *master_keylist;
 extern  krb5_actkvno_node  *active_mkey_list;
-extern  krb5_keyblock       hist_key;
 extern  krb5_db_entry       master_db;
-extern  krb5_db_entry       hist_db;
-extern  krb5_kvno           hist_kvno;
 
 static int decrypt_key_data(krb5_context context, krb5_keyblock *mkey,
                             int n_key_data, krb5_key_data *key_data,
@@ -428,7 +425,7 @@ kadm5_create_principal_3(void *server_handle,
        I'm going to keep it, and make all the admin stuff occupy a
        single tl_data record, */
 
-    adb.admin_history_kvno = hist_kvno;
+    adb.admin_history_kvno = INITIAL_HIST_KVNO;
     if ((mask & KADM5_POLICY)) {
         adb.aux_attributes = KADM5_POLICY;
 
@@ -1020,6 +1017,8 @@ check_pw_reuse(krb5_context context,
  * Arguments:
  *
  *      context         (r) krb5_context to use
+ *      mkey            (r) master keyblock to decrypt key data with
+ *      hist_key        (r) history keyblock to encrypt key data with
  *      n_key_data      (r) number of elements in key_data
  *      key_data        (r) keys to add to the history entry
  *      hist            (w) history entry to fill in
@@ -1032,7 +1031,8 @@ check_pw_reuse(krb5_context context,
  * set to n_key_data.
  */
 static
-int create_history_entry(krb5_context context, krb5_keyblock *mkey, int n_key_data,
+int create_history_entry(krb5_context context, krb5_keyblock *mkey,
+                         krb5_keyblock *hist_key, int n_key_data,
                          krb5_key_data *key_data, osa_pw_hist_ent *hist)
 {
     int i, ret;
@@ -1052,7 +1052,7 @@ int create_history_entry(krb5_context context, krb5_keyblock *mkey, int n_key_da
         if (ret)
             return ret;
 
-        ret = krb5_dbekd_encrypt_key_data(context, &hist_key,
+        ret = krb5_dbekd_encrypt_key_data(context, hist_key,
                                           &key, &salt,
                                           key_data[i].key_data_kvno,
                                           &hist->key_data[i]);
@@ -1085,6 +1085,7 @@ void free_history_entry(krb5_context context, osa_pw_hist_ent *hist)
  * Arguments:
  *
  *      context         (r) krb5_context to use
+ *      hist_kvno       (r) kvno of current history key
  *      adb             (r/w) admin principal entry to add keys to
  *      pol             (r) adb's policy
  *      pw              (r) keys for the password to add to adb's key history
@@ -1104,6 +1105,7 @@ void free_history_entry(krb5_context context, osa_pw_hist_ent *hist)
  * adb->old_key_len).
  */
 static kadm5_ret_t add_to_history(krb5_context context,
+                                  krb5_kvno hist_kvno,
                                   osa_princ_ent_t adb,
                                   kadm5_policy_ent_t pol,
                                   osa_pw_hist_ent *pw)
@@ -1117,6 +1119,16 @@ static kadm5_ret_t add_to_history(krb5_context context,
     if (nhist <= 1)
         return 0;
 
+    if (adb->admin_history_kvno != hist_kvno) {
+        /* The history key has changed since the last password change, so we
+         * have to reset the password history. */
+        free(adb->old_keys);
+        adb->old_keys = NULL;
+        adb->old_key_len = 0;
+        adb->old_key_next = 0;
+        adb->admin_history_kvno = hist_kvno;
+    }
+
     nkeys = adb->old_key_len;
     knext = adb->old_key_next;
     /* resize the adb->old_keys array if necessary */
@@ -1335,8 +1347,8 @@ kadm5_chpass_principal_3(void *server_handle,
     int                         have_pol = 0;
     kadm5_server_handle_t       handle = server_handle;
     osa_pw_hist_ent             hist;
-    krb5_keyblock               *act_mkey;
-    krb5_kvno                   act_kvno;
+    krb5_keyblock               *act_mkey, hist_keyblock;
+    krb5_kvno                   act_kvno, hist_kvno;
 
     CHECK_HANDLE(server_handle);
 
@@ -1344,6 +1356,7 @@ kadm5_chpass_principal_3(void *server_handle,
 
     hist_added = 0;
     memset(&hist, 0, sizeof(hist));
+    memset(&hist_keyblock, 0, sizeof(hist_keyblock));
 
     if (principal == NULL || password == NULL)
         return EINVAL;
@@ -1415,32 +1428,36 @@ kadm5_chpass_principal_3(void *server_handle,
         }
 #endif
 
+        ret = kdb_get_hist_key(handle, &hist_keyblock, &hist_kvno);
+        if (ret)
+            goto done;
+
         ret = create_history_entry(handle->context,
-                                   act_mkey,
+                                   act_mkey, &hist_keyblock,
                                    kdb_save.n_key_data,
                                    kdb_save.key_data, &hist);
         if (ret)
             goto done;
 
-        ret = check_pw_reuse(handle->context, act_mkey, &hist_key,
+        ret = check_pw_reuse(handle->context, act_mkey, &hist_keyblock,
                              kdb.n_key_data, kdb.key_data,
                              1, &hist);
         if (ret)
             goto done;
 
         if (pol.pw_history_num > 1) {
-            if (adb.admin_history_kvno != hist_kvno) {
-                ret = KADM5_BAD_HIST_KEY;
-                goto done;
+            /* If hist_kvno has changed since the last password change, we
+             * can't check the history. */
+            if (adb.admin_history_kvno == hist_kvno) {
+                ret = check_pw_reuse(handle->context, act_mkey, &hist_keyblock,
+                                     kdb.n_key_data, kdb.key_data,
+                                     adb.old_key_len, adb.old_keys);
+                if (ret)
+                    goto done;
             }
 
-            ret = check_pw_reuse(handle->context, act_mkey, &hist_key,
-                                 kdb.n_key_data, kdb.key_data,
-                                 adb.old_key_len, adb.old_keys);
-            if (ret)
-                goto done;
-
-            ret = add_to_history(handle->context, &adb, &pol, &hist);
+            ret = add_to_history(handle->context, hist_kvno, &adb, &pol,
+                                 &hist);
             if (ret)
                 goto done;
             hist_added = 1;
@@ -1505,6 +1522,7 @@ done:
     kdb_free_entry(handle, &kdb, &adb);
     kdb_free_entry(handle, &kdb_save, NULL);
     krb5_db_free_principal(handle->context, &kdb, 1);
+    krb5_free_keyblock_contents(handle->context, &hist_keyblock);
 
     if (have_pol && (ret2 = kadm5_free_policy_ent(handle->lhandle, &pol))
         && !ret)
@@ -1549,10 +1567,14 @@ kadm5_randkey_principal_3(void *server_handle,
 
     if (principal == NULL)
         return EINVAL;
-    if (hist_princ && /* this will be NULL when initializing the databse */
-        ((krb5_principal_compare(handle->context,
-                                 principal, hist_princ)) == TRUE))
-        return KADM5_PROTECT_PRINCIPAL;
+    if (krb5_principal_compare(handle->context, principal, hist_princ)) {
+        /* If changing the history entry, the new entry must have exactly one
+         * key. */
+        if (keepold)
+            return KADM5_PROTECT_PRINCIPAL;
+        ks_tuple = n_ks_tuple ? ks_tuple : handle->params.keysalts,
+        n_ks_tuple = 1;
+    }
 
     if ((ret = kdb_get_entry(handle, principal, &kdb, &adb)))
         return(ret);
@@ -1601,18 +1623,6 @@ kadm5_randkey_principal_3(void *server_handle,
         }
 #endif
 
-        if(pol.pw_history_num > 1) {
-            if(adb.admin_history_kvno != hist_kvno) {
-                ret = KADM5_BAD_HIST_KEY;
-                goto done;
-            }
-
-            ret = check_pw_reuse(handle->context, act_mkey, &hist_key,
-                                 kdb.n_key_data, kdb.key_data,
-                                 adb.old_key_len, adb.old_keys);
-            if (ret)
-                goto done;
-        }
         if (pol.pw_max_life)
             kdb.pw_expiration = now + pol.pw_max_life;
         else
@@ -1776,23 +1786,6 @@ kadm5_setv4key_principal(void *server_handle,
             goto done;
         }
 #endif
-#if 0
-        /*
-         * Should we be checking/updating pw history here?
-         */
-        if(pol.pw_history_num > 1) {
-            if(adb.admin_history_kvno != hist_kvno) {
-                ret = KADM5_BAD_HIST_KEY;
-                goto done;
-            }
-
-            if (ret = check_pw_reuse(handle->context,
-                                     &hist_key,
-                                     kdb.n_key_data, kdb.key_data,
-                                     adb.old_key_len, adb.old_keys))
-                goto done;
-        }
-#endif
 
         if (pol.pw_max_life)
             kdb.pw_expiration = now + pol.pw_max_life;
@@ -2017,23 +2010,6 @@ kadm5_setkey_principal_3(void *server_handle,
             goto done;
         }
 #endif
-#if 0
-        /*
-         * Should we be checking/updating pw history here?
-         */
-        if (pol.pw_history_num > 1) {
-            if(adb.admin_history_kvno != hist_kvno) {
-                ret = KADM5_BAD_HIST_KEY;
-                goto done;
-            }
-
-            if (ret = check_pw_reuse(handle->context,
-                                     &hist_key,
-                                     kdb.n_key_data, kdb.key_data,
-                                     adb.old_key_len, adb.old_keys))
-                goto done;
-        }
-#endif
 
         if (pol.pw_max_life)
             kdb.pw_expiration = now + pol.pw_max_life;
diff --git a/src/lib/kadm5/unit-test/api.current/randkey-principal.exp b/src/lib/kadm5/unit-test/api.current/randkey-principal.exp
index f003863..5fcb1c0 100644
--- a/src/lib/kadm5/unit-test/api.current/randkey-principal.exp
+++ b/src/lib/kadm5/unit-test/api.current/randkey-principal.exp
@@ -279,9 +279,9 @@ proc test32 {} {
 	perror "$test: unexpected failure in init"
 	return
     }
-    one_line_fail_test {
+    one_line_succeed_test {
 	kadm5_randkey_principal $server_handle kadmin/history keys num_keys
-    } "PROTECT"
+    }
     if { ! [cmd {kadm5_destroy $server_handle}]} {
 	perror "$test: unexpected failure in destroy"
 	return