Skip Menu |
 

Subject: Decrypting history key entries can fail after 1.8 upgrade
If a KDB is created with krb5 1.2 or earlier, kadmin/history will be
created with all supported enctypes. (In krb5 1.3 and later,
kadmin/history is created with only one key entry, for the master key
enctype.)

In krb5 1.7 and earlier, the kadmin/history key is selected by looking
for a key of the master key enctype. In krb5 1.8, the key is selected
by using the first key data entry.

So if a KDB is created with <=1.2, and has password history entries
created with <=1.7, check_pw_reuse() in >=1.8 could try to decrypt those
entries with a different key. Decryption will fail, causing the
password change operation to fail.

To make sure we properly use history entries in the presence of multiple
kadmin/history keys, we need to try all keys when decrypting.

We should also consider whether failure to decrypt a history entry
should be fatal for the password change operation, or if the history
entry should just be ignored (possibly allowing a historical user
password to be reused when it shouldn't be allowed, although there are
other cases where that can happen).
From: ghudson@mit.edu
Subject: SVN Commit
Try all history keys to decrypt password history

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.

https://github.com/krb5/krb5/commit/2084129a9e7fbc3567f094cb5886bcbefa88dd18
Commit By: ghudson
Revision: 25819
Changed Files:
U trunk/src/lib/kadm5/server_internal.h
U trunk/src/lib/kadm5/srv/server_kdb.c
U trunk/src/lib/kadm5/srv/svr_principal.c
U trunk/src/tests/Makefile.in
A trunk/src/tests/hist.c
A trunk/src/tests/t_pwhist.py
The original reporter asked for a 1.8 backport, so I'm attaching the code
part (not the test case).
Download patch.txt
text/plain 8.3KiB
Index: src/lib/kadm5/server_internal.h
===================================================================
--- src/lib/kadm5/server_internal.h (revision 25068)
+++ src/lib/kadm5/server_internal.h (working copy)
@@ -74,8 +74,10 @@
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: src/lib/kadm5/srv/svr_principal.c
===================================================================
--- src/lib/kadm5/srv/svr_principal.c (revision 25068)
+++ src/lib/kadm5/srv/svr_principal.c (working copy)
@@ -968,12 +968,13 @@
static kadm5_ret_t
check_pw_reuse(krb5_context context,
krb5_keyblock *mkey,
- 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)
{
int x, y, z;
- krb5_keyblock newkey, histkey;
+ krb5_keyblock newkey, histkey, *kb;
+ krb5_key_data *key_data;
krb5_error_code ret;

for (x = 0; x < n_new_key_data; x++) {
@@ -985,21 +986,20 @@
return(ret);
for (y = 0; y < n_pw_hist_data; y++) {
for (z = 0; z < pw_hist_data[y].n_key_data; z++) {
- ret = krb5_dbekd_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)) {
- krb5_free_keyblock_contents(context, &histkey);
- krb5_free_keyblock_contents(context, &newkey);
-
- return(KADM5_PASS_REUSE);
+ for (kb = hist_keyblocks; kb->enctype != 0; kb++) {
+ key_data = &pw_hist_data[y].key_data[z];
+ ret = krb5_dbekd_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);
}
@@ -1349,7 +1349,7 @@
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);
@@ -1358,7 +1358,6 @@

hist_added = 0;
memset(&hist, 0, sizeof(hist));
- memset(&hist_keyblock, 0, sizeof(hist_keyblock));

if (principal == NULL || password == NULL)
return EINVAL;
@@ -1430,18 +1429,18 @@
}
#endif

- 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,
- act_mkey, &hist_keyblock,
+ act_mkey, &hist_keyblocks[0],
kdb_save.n_key_data,
kdb_save.key_data, &hist);
if (ret)
goto done;

- ret = check_pw_reuse(handle->context, act_mkey, &hist_keyblock,
+ ret = check_pw_reuse(handle->context, act_mkey, hist_keyblocks,
kdb.n_key_data, kdb.key_data,
1, &hist);
if (ret)
@@ -1451,7 +1450,7 @@
/* 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,
+ ret = check_pw_reuse(handle->context, act_mkey, hist_keyblocks,
kdb.n_key_data, kdb.key_data,
adb.old_key_len, adb.old_keys);
if (ret)
@@ -1524,7 +1523,7 @@
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);
+ kdb_free_keyblocks(handle, hist_keyblocks);

if (have_pol && (ret2 = kadm5_free_policy_ent(handle->lhandle, &pol))
&& !ret)
Index: src/lib/kadm5/srv/server_kdb.c
===================================================================
--- src/lib/kadm5/srv/server_kdb.c (revision 25068)
+++ src/lib/kadm5/srv/server_kdb.c (working copy)
@@ -177,26 +177,20 @@
}

/*
- * 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.
+ * 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;

ret = kdb_get_entry(handle, hist_princ, &kdb, NULL);
if (ret)
@@ -214,18 +208,39 @@
if (ret)
goto done;

- ret = krb5_dbekd_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_dbekd_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;
}

+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
*
From: tlyu@mit.edu
Subject: SVN Commit

Try all history keys to decrypt password history

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.

(backported from commit 2782e80a12bccd920fa71e23166ac97c4470a637)

https://github.com/krb5/krb5/commit/c7b8525b7240428beb5f73f97484056385d11db5
Author: Greg Hudson <ghudson@mit.edu>
Committer: Tom Yu <tlyu@mit.edu>
Commit: c7b8525b7240428beb5f73f97484056385d11db5
src/lib/kadm5/server_internal.h | 6 ++-
src/lib/kadm5/srv/server_kdb.c | 55 +++++++++++++-------
src/lib/kadm5/srv/svr_principal.c | 46 +++++++++---------
src/tests/Makefile.in | 6 ++-
src/tests/hist.c | 99 +++++++++++++++++++++++++++++++++++++
src/tests/t_pwhist.py | 20 +++++++
6 files changed, 186 insertions(+), 46 deletions(-)