From 7e0a801e284d6896017bcec2a9bcece0d2123949 Mon Sep 17 00:00:00 2001 From: Nicolas Williams Date: Fri, 15 Jul 2011 18:09:05 -0500 Subject: [PATCH] Changed decrypt key history logic and added HDB_F_ALL_KVNOS. --- lib/hdb/common.c | 49 ++++++++++++++++++++++++++++------------------- lib/hdb/hdb.h | 1 + lib/hdb/mkey.c | 27 ++++++++++++++++++++------ lib/kadm5/get_s.c | 3 ++- lib/kadm5/log.c | 5 ++++- 5 files changed, 57 insertions(+), 28 deletions(-) diff --git a/lib/hdb/common.c b/lib/hdb/common.c index 089c06a01..125ca997a 100644 --- a/lib/hdb/common.c +++ b/lib/hdb/common.c @@ -155,38 +155,47 @@ _hdb_fetch_kvno(krb5_context context, HDB *db, krb5_const_principal principal, } } krb5_data_free(&value); - if (db->hdb_master_key_set && (flags & HDB_F_DECRYPT)) { - if ((flags & HDB_F_KVNO_SPECIFIED) == 0 && - (flags & HDB_F_CURRENT_KVNO) == 0) { - + if (db->hdb_master_key_set && (flags & HDB_F_DECRYPT) && + (flags & HDB_F_ALL_KVNOS)) { + /* Decrypt the current keys */ + ret = hdb_unseal_keys(context, db, &entry->entry); + if (ret) { + hdb_free_entry(context, entry); + return ret; + } + /* Decrypt the key history too */ + ret = hdb_unseal_keys_kvno(context, db, 0, &entry->entry); + if (ret) { + hdb_free_entry(context, entry); + return ret; + } + } else if (db->hdb_master_key_set && (flags & HDB_F_DECRYPT)) { + if ((flags & HDB_F_KVNO_SPECIFIED) == 0 || kvno == entry->entry.kvno) { + /* Decrypt the current keys */ + ret = hdb_unseal_keys(context, db, &entry->entry); + if (ret) { + hdb_free_entry(context, entry); + return ret; + } + } else { /* - * Decrypt all the old keys too, since we don't know which - * the caller will need. + * Find and decrypt the keys from the history that we want, + * and swap them with the current keys */ ret = hdb_unseal_keys_kvno(context, db, 0, &entry->entry); if (ret) { hdb_free_entry(context, entry); return ret; } - } else if ((flags & HDB_F_KVNO_SPECIFIED) != 0 && - kvno != entry->entry.kvno && - kvno < entry->entry.kvno && - kvno > 0) { - - /* Decrypt the keys we were asked for, if not the current ones */ - ret = hdb_unseal_keys_kvno(context, db, kvno, &entry->entry); + } + if ((flags & HDB_F_ALL_KVNOS)) { + /* Decrypt the history, post current/requested switcheroo */ + ret = hdb_unseal_keys_kvno(context, db, 0, &entry->entry); if (ret) { hdb_free_entry(context, entry); return ret; } } - - /* Always decrypt the current keys too */ - ret = hdb_unseal_keys(context, db, &entry->entry); - if (ret) { - hdb_free_entry(context, entry); - return ret; - } } return ret; diff --git a/lib/hdb/hdb.h b/lib/hdb/hdb.h index b8e6b8a47..61314153c 100644 --- a/lib/hdb/hdb.h +++ b/lib/hdb/hdb.h @@ -58,6 +58,7 @@ enum hdb_lockop{ HDB_RLOCK, HDB_WLOCK }; #define HDB_F_ADMIN_DATA 64 /* want data that kdc don't use */ #define HDB_F_KVNO_SPECIFIED 128 /* we want a particular KVNO */ #define HDB_F_CURRENT_KVNO 256 /* we want the current KVNO */ +#define HDB_F_ALL_KVNOS 512 /* we want all the keys */ /* hdb_capability_flags */ #define HDB_CAP_F_HANDLE_ENTERPRISE_PRINCIPAL 1 diff --git a/lib/hdb/mkey.c b/lib/hdb/mkey.c index e8f55c6aa..c2685e64b 100644 --- a/lib/hdb/mkey.c +++ b/lib/hdb/mkey.c @@ -508,11 +508,26 @@ hdb_unseal_keys_kvno(krb5_context context, HDB *db, krb5_kvno kvno, for (i = hist_keys->len - 1; i >= 0; i++) { if (kvno != 0 && hist_keys->val[i].kvno != kvno) continue; + + /* Either the keys we want, or all the keys */ for (k = 0; k < hist_keys->val[i].keys.len; k++) { ret = hdb_unseal_key_mkey(context, &hist_keys->val[i].keys.val[k], db->hdb_master_key); - if (ret) + /* + * If kvno == 0 we might not want to bail here! E.g., if we + * no longer have the right master key, so just ignore this. + * + * Might we want to filter out keys that we can't decrypt + * because of HDB_ERR_NO_MKEY? Probably. If nothing else + * so we don't leave turds behind. But also in case of old + * master keys derived from passwords -- we don't want to + * help kadmin clients with fetch authorization be able to + * mount dictionary attacks on old master keys. Also, + * mayber we're misconfigured and simply can't find a live + * master key. + */ + if (ret != HDB_ERR_NO_MKEY) return (ret); } @@ -520,12 +535,12 @@ hdb_unseal_keys_kvno(krb5_context context, HDB *db, krb5_kvno kvno, continue; /* - * NOTE: What follows is a bit of an ugly hack. + * What follows is a bit of a hack. * - * This is the keyset we're being asked for, so we add the - * current keyset to the history, leave the one we were asked - * for in the history, and pretend the one we were asked for is - * also the current keyset. + * This is the keyset we're being asked for, but it's not the + * current keyset. So we add the current keyset to the history, + * leave the one we were asked for in the history, and pretend + * the one we were asked for is also the current keyset. * * This is a bit of a defensive hack in case an entry fetched * this way ever gets modified then stored: if the keyset is not diff --git a/lib/kadm5/get_s.c b/lib/kadm5/get_s.c index 8c20dc2ca..eac735f6b 100644 --- a/lib/kadm5/get_s.c +++ b/lib/kadm5/get_s.c @@ -130,7 +130,8 @@ kadm5_s_get_principal(void *server_handle, if(ret) return ret; ret = context->db->hdb_fetch_kvno(context->context, context->db, princ, - HDB_F_DECRYPT|HDB_F_GET_ANY|HDB_F_ADMIN_DATA, 0, &ent); + HDB_F_DECRYPT|HDB_F_ALL_KVNOS| + HDB_F_GET_ANY|HDB_F_ADMIN_DATA, 0, &ent); context->db->hdb_close(context->context, context->db); if(ret) return _kadm5_error_code(ret); diff --git a/lib/kadm5/log.c b/lib/kadm5/log.c index 05b84b1e0..14e0f73a7 100644 --- a/lib/kadm5/log.c +++ b/lib/kadm5/log.c @@ -585,7 +585,8 @@ kadm5_log_replay_modify (kadm5_server_context *context, memset(&ent, 0, sizeof(ent)); ret = context->db->hdb_fetch_kvno(context->context, context->db, log_ent.entry.principal, - HDB_F_DECRYPT|HDB_F_GET_ANY|HDB_F_ADMIN_DATA, 0, &ent); + HDB_F_DECRYPT|HDB_F_ALL_KVNOS| + HDB_F_GET_ANY|HDB_F_ADMIN_DATA, 0, &ent); if (ret) goto out; if (mask & KADM5_PRINC_EXPIRE_TIME) { @@ -698,6 +699,8 @@ kadm5_log_replay_modify (kadm5_server_context *context, size_t num; size_t i; + /* XXX Take care of key history!! */ + for (i = 0; i < ent.entry.keys.len; ++i) free_Key(&ent.entry.keys.val[i]); free (ent.entry.keys.val);