Try all history keys to decrypt password history
authorGreg Hudson <ghudson@mit.edu>
Tue, 24 Apr 2012 01:05:41 +0000 (01:05 +0000)
committerGreg Hudson <ghudson@mit.edu>
Tue, 24 Apr 2012 01:05:41 +0000 (01:05 +0000)
A database created prior to 1.3 will have multiple password history
keys, and kadmin prior to 1.8 won't necessarily choose the first one.
So if there are multiple keys, we have to try them all.  If none of
the keys can decrypt a password history entry, don't fail the password
change operation; it's not worth it without positive evidence of
password reuse.

ticket: 7099

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

src/lib/kadm5/server_internal.h
src/lib/kadm5/srv/server_kdb.c
src/lib/kadm5/srv/svr_principal.c
src/tests/Makefile.in
src/tests/hist.c [new file with mode: 0644]
src/tests/t_pwhist.py [new file with mode: 0644]

index 8778522546cda647f1551655b257f13b2c29e28a..220e2b694f6de2b87c7da991337870ec39bfb765 100644 (file)
@@ -81,8 +81,10 @@ krb5_error_code     kdb_init_master(kadm5_server_handle_t handle,
 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_keyblock **keyblocks_out,
+                                     krb5_kvno *kvno_out);
+void                kdb_free_keyblocks(kadm5_server_handle_t handle,
+                                       krb5_keyblock *keyblocks);
 krb5_error_code     kdb_get_entry(kadm5_server_handle_t handle,
                                   krb5_principal principal,
                                   krb5_db_entry **kdb, osa_princ_ent_rec *adb);
index 3860b6b2f62fd345016b98be278f8273ddd61c7d..f4217dd4987c402c482ef525a155fbcb279d811c 100644 (file)
@@ -151,27 +151,20 @@ create_hist(kadm5_server_handle_t handle)
 }
 
 /*
- * Function: kdb_get_hist_key
- *
- * Purpose: Fetches the current history key, creating it if necessary
- *
- * 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.  If the history principal does not exist,
- * it will be created.
+ * Fetch the current history key(s), creating the history principal if
+ * necessary.  Database created since krb5 1.3 will have only one key, but
+ * databases created before that may have multiple keys (of the same kvno)
+ * and we need to try them all.  History keys will be returned in a list
+ * terminated by an entry with enctype 0.
  */
 krb5_error_code
-kdb_get_hist_key(kadm5_server_handle_t handle, krb5_keyblock *hist_keyblock,
-                 krb5_kvno *hist_kvno)
+kdb_get_hist_key(kadm5_server_handle_t handle, krb5_keyblock **keyblocks_out,
+                 krb5_kvno *kvno_out)
 {
     krb5_error_code ret;
     krb5_db_entry *kdb;
-    krb5_keyblock *mkey;
+    krb5_keyblock *mkey, *kblist = NULL;
+    krb5_int16 i;
 
     /* Fetch the history principal, creating it if necessary. */
     ret = kdb_get_entry(handle, hist_princ, &kdb, NULL);
@@ -195,18 +188,40 @@ kdb_get_hist_key(kadm5_server_handle_t handle, krb5_keyblock *hist_keyblock,
     if (ret)
         goto done;
 
-    ret = krb5_dbe_decrypt_key_data(handle->context, mkey, &kdb->key_data[0],
-                                    hist_keyblock, NULL);
-    if (ret)
+    kblist = k5alloc((kdb->n_key_data + 1) * sizeof(*kblist), &ret);
+    if (kblist == NULL)
         goto done;
+    for (i = 0; i < kdb->n_key_data; i++) {
+        ret = krb5_dbe_decrypt_key_data(handle->context, mkey,
+                                        &kdb->key_data[i], &kblist[i],
+                                        NULL);
+        if (ret)
+            goto done;
+    }
 
-    *hist_kvno = kdb->key_data[0].key_data_kvno;
+    *keyblocks_out = kblist;
+    kblist = NULL;
+    *kvno_out = kdb->key_data[0].key_data_kvno;
 
 done:
     kdb_free_entry(handle, kdb, NULL);
+    kdb_free_keyblocks(handle, kblist);
     return ret;
 }
 
+/* Free all keyblocks in a list (terminated by a keyblock with enctype 0). */
+void
+kdb_free_keyblocks(kadm5_server_handle_t handle, krb5_keyblock *keyblocks)
+{
+    krb5_keyblock *kb;
+
+    if (keyblocks == NULL)
+        return;
+    for (kb = keyblocks; kb->enctype != 0; kb++)
+        krb5_free_keyblock_contents(handle->context, kb);
+    free(keyblocks);
+}
+
 /*
  * Function: kdb_get_entry
  *
index f77490fe1846a98cd6a71d00100737169344d60a..00541dff16b39497e45d8b0855bc823e1d105475 100644 (file)
@@ -962,12 +962,13 @@ done:
  */
 static kadm5_ret_t
 check_pw_reuse(krb5_context context,
-               krb5_keyblock *hist_keyblock,
+               krb5_keyblock *hist_keyblocks,
                int n_new_key_data, krb5_key_data *new_key_data,
                unsigned int n_pw_hist_data, osa_pw_hist_ent *pw_hist_data)
 {
     unsigned int x, y, z;
-    krb5_keyblock newkey, histkey;
+    krb5_keyblock newkey, histkey, *kb;
+    krb5_key_data *key_data;
     krb5_error_code ret;
 
     assert (n_new_key_data >= 0);
@@ -981,22 +982,22 @@ check_pw_reuse(krb5_context context,
             return(ret);
         for (y = 0; y < n_pw_hist_data; y++) {
             for (z = 0; z < (unsigned int) pw_hist_data[y].n_key_data; z++) {
-                ret = krb5_dbe_decrypt_key_data(context, hist_keyblock,
-                                                &pw_hist_data[y].key_data[z],
-                                                &histkey, NULL);
-                if (ret)
-                    return(ret);
-
-                if ((newkey.length == histkey.length) &&
-                    (newkey.enctype == histkey.enctype) &&
-                    (memcmp(newkey.contents, histkey.contents,
-                            histkey.length) == 0)) {
+                for (kb = hist_keyblocks; kb->enctype != 0; kb++) {
+                    key_data = &pw_hist_data[y].key_data[z];
+                    ret = krb5_dbe_decrypt_key_data(context, kb, key_data,
+                                                    &histkey, NULL);
+                    if (ret)
+                        continue;
+                    if (newkey.length == histkey.length &&
+                        newkey.enctype == histkey.enctype &&
+                        memcmp(newkey.contents, histkey.contents,
+                               histkey.length) == 0) {
+                        krb5_free_keyblock_contents(context, &histkey);
+                        krb5_free_keyblock_contents(context, &newkey);
+                        return KADM5_PASS_REUSE;
+                    }
                     krb5_free_keyblock_contents(context, &histkey);
-                    krb5_free_keyblock_contents(context, &newkey);
-
-                    return(KADM5_PASS_REUSE);
                 }
-                krb5_free_keyblock_contents(context, &histkey);
             }
         }
         krb5_free_keyblock_contents(context, &newkey);
@@ -1341,7 +1342,7 @@ 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, hist_keyblock;
+    krb5_keyblock               *act_mkey, *hist_keyblocks = NULL;
     krb5_kvno                   act_kvno, hist_kvno;
 
     CHECK_HANDLE(server_handle);
@@ -1350,7 +1351,6 @@ 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;
@@ -1373,10 +1373,10 @@ kadm5_chpass_principal_3(void *server_handle,
         have_pol = 1;
 
         /* Create a password history entry before we change kdb's key_data. */
-        ret = kdb_get_hist_key(handle, &hist_keyblock, &hist_kvno);
+        ret = kdb_get_hist_key(handle, &hist_keyblocks, &hist_kvno);
         if (ret)
             goto done;
-        ret = create_history_entry(handle->context, &hist_keyblock,
+        ret = create_history_entry(handle->context, &hist_keyblocks[0],
                                    kdb->n_key_data, kdb->key_data, &hist);
         if (ret)
             goto done;
@@ -1428,7 +1428,7 @@ kadm5_chpass_principal_3(void *server_handle,
         }
 #endif
 
-        ret = check_pw_reuse(handle->context, &hist_keyblock,
+        ret = check_pw_reuse(handle->context, hist_keyblocks,
                              kdb->n_key_data, kdb->key_data,
                              1, &hist);
         if (ret)
@@ -1438,7 +1438,7 @@ kadm5_chpass_principal_3(void *server_handle,
             /* 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, &hist_keyblock,
+                ret = check_pw_reuse(handle->context, hist_keyblocks,
                                      kdb->n_key_data, kdb->key_data,
                                      adb.old_key_len, adb.old_keys);
                 if (ret)
@@ -1518,7 +1518,7 @@ done:
     if (!hist_added && hist.key_data)
         free_history_entry(handle->context, &hist);
     kdb_free_entry(handle, kdb, &adb);
-    krb5_free_keyblock_contents(handle->context, &hist_keyblock);
+    kdb_free_keyblocks(handle, hist_keyblocks);
 
     if (have_pol && (ret2 = kadm5_free_policy_ent(handle->lhandle, &pol))
         && !ret)
index 9c5d269a1396971c69f2c310070bc741a8fe8cb0..5ee619a1ebcaf248f8ddb2cf247844b6f8adfcb0 100644 (file)
@@ -18,6 +18,9 @@ TEST_PREFIX = "foo bar"
 KADMIN_OPTS= -d $(TEST_DB) -r $(TEST_REALM) -P $(TEST_MKEY)
 KTEST_OPTS= $(KADMIN_OPTS) -p $(TEST_PREFIX) -n $(TEST_NUM) -D $(TEST_DEPTH)
 
+hist: hist.o $(KDB5_DEPLIBS) $(KADMSRV_DEPLIBS) $(KRB5_BASE_DEPLIBS)
+       $(CC_LINK) -o $@ hist.o $(KDB5_LIBS) $(KADMSRV_LIBS) $(KRB5_BASE_LIBS)
+
 check-unix:: kdb_check
 
 kdc.conf: Makefile
@@ -60,7 +63,7 @@ kdb_check: kdc.conf krb5.conf
        $(RUN_SETUP) $(VALGRIND) ../kadmin/dbutil/kdb5_util $(KADMIN_OPTS) destroy -f
        $(RM) $(TEST_DB)* stash_file
 
-check-pytests::
+check-pytests:: hist
        $(RUNPYTEST) $(srcdir)/t_general.py $(PYTESTFLAGS)
        $(RUNPYTEST) $(srcdir)/t_anonpkinit.py $(PYTESTFLAGS)
        $(RUNPYTEST) $(srcdir)/t_lockout.py $(PYTESTFLAGS)
@@ -73,6 +76,7 @@ check-pytests::
        $(RUNPYTEST) $(srcdir)/t_crossrealm.py $(PYTESTFLAGS)
        $(RUNPYTEST) $(srcdir)/t_skew.py $(PYTESTFLAGS)
        $(RUNPYTEST) $(srcdir)/t_keytab.py $(PYTESTFLAGS)
+       $(RUNPYTEST) $(srcdir)/t_pwhist.py $(PYTESTFLAGS)
 #      $(RUNPYTEST) $(srcdir)/kdc_realm/kdcref.py $(PYTESTFLAGS)
 
 clean::
diff --git a/src/tests/hist.c b/src/tests/hist.c
new file mode 100644 (file)
index 0000000..c0b2b97
--- /dev/null
@@ -0,0 +1,99 @@
+/* -*- mode: c; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/* tests/hist.c - Perform unusual operations on history keys */
+/*
+ * Copyright (C) 2012 by the Massachusetts Institute of Technology.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ *   notice, this list of conditions and the following disclaimer in
+ *   the documentation and/or other materials provided with the
+ *   distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+ * OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/*
+ * This program is invoked from t_pwhist.py to simulate some conditions
+ * normally only seen in databases created before krb5 1.3.  With the "make"
+ * argument, the history key is rolled over to a kvno containing two keys
+ * (since krb5 1.3 we ordinarily ensure that there's only one).  With the
+ * "swap" argument, the two history keys are swapped in order; we use this
+ * operation to simulate the case where krb5 1.7 or earlier chose something
+ * other than the first history key to create pasword history entries.
+ */
+
+#include <k5-int.h>
+#include <kadm5/admin.h>
+
+static void
+check(krb5_error_code ret)
+{
+    if (ret) {
+       fprintf(stderr, "Unexpected failure, aborting\n");
+       abort();
+    }
+}
+
+int
+main(int argc, char **argv)
+{
+    krb5_context ctx;
+    krb5_db_entry *ent;
+    krb5_principal hprinc;
+    kadm5_principal_ent_rec kent;
+    krb5_key_salt_tuple ks[2];
+    krb5_key_data kd;
+    kadm5_config_params params = { 0 };
+    void *handle;
+    char *realm;
+    long mask = KADM5_PRINCIPAL | KADM5_MAX_LIFE | KADM5_ATTRIBUTES;
+
+    check(kadm5_init_krb5_context(&ctx));
+    check(krb5_parse_name(ctx, "kadmin/history", &hprinc));
+    check(krb5_get_default_realm(ctx, &realm));
+    params.mask |= KADM5_CONFIG_REALM;
+    params.realm = realm;
+    check(kadm5_init(ctx, "user", "", "", &params, KADM5_STRUCT_VERSION,
+                     KADM5_API_VERSION_3, NULL, &handle));
+    if (strcmp(argv[1], "make") == 0) {
+        memset(&kent, 0, sizeof(kent));
+        kent.principal = hprinc;
+        kent.max_life = KRB5_KDB_DISALLOW_ALL_TIX;
+        kent.attributes = 0;
+       ks[0].ks_enctype = ENCTYPE_AES256_CTS_HMAC_SHA1_96;
+       ks[0].ks_salttype = KRB5_KDB_SALTTYPE_NORMAL;
+       ks[1].ks_enctype = ENCTYPE_AES128_CTS_HMAC_SHA1_96;
+       ks[1].ks_salttype = KRB5_KDB_SALTTYPE_NORMAL;
+        check(kadm5_create_principal_3(handle, &kent, mask, 2, ks, NULL));
+    } else if (strcmp(argv[1], "swap") == 0) {
+        check(krb5_db_get_principal(ctx, hprinc, 0, &ent));
+       kd = ent->key_data[0];
+       ent->key_data[0] = ent->key_data[1];
+       ent->key_data[1] = kd;
+        check(krb5_db_put_principal(ctx, ent));
+        krb5_db_free_principal(ctx, ent);
+    }
+    krb5_free_default_realm(ctx, realm);
+    kadm5_destroy(handle);
+    krb5_free_principal(ctx, hprinc);
+    krb5_free_context(ctx);
+    return 0;
+}
diff --git a/src/tests/t_pwhist.py b/src/tests/t_pwhist.py
new file mode 100644 (file)
index 0000000..4ae5466
--- /dev/null
@@ -0,0 +1,20 @@
+#!/usr/bin/python
+from k5test import *
+
+# Regression test for issue #7099: databases created prior to krb5 1.3 have
+# multiple history keys, and kadmin prior to 1.7 didn't necessarily use the
+# first one to create history entries.
+realm = K5Realm(start_kadmind=False, start_kdc=False)
+# Create a history principal with two keys.
+realm.run_as_master(['./hist', 'make'])
+realm.run_kadminl('addpol -history 2 pol')
+realm.run_kadminl('modprinc -policy pol user')
+realm.run_kadminl('cpw -pw pw2 user')
+# Swap the keys, simulating older kadmin having chosen the second entry.
+realm.run_as_master(['./hist', 'swap'])
+# Make sure we can read the history entry.
+output = realm.run_kadminl('cpw -pw %s user' % password('user'))
+if 'Cannot reuse password' not in output:
+    fail('Expected error not seen in output')
+
+success('Password history tests')