Changed decrypt key history logic and added HDB_F_ALL_KVNOS.

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 (db->hdb_master_key_set && (flags & HDB_F_DECRYPT) &&
-	if ((flags & HDB_F_KVNO_SPECIFIED) == 0 &&
+	(flags & HDB_F_ALL_KVNOS)) {
-	    (flags & HDB_F_CURRENT_KVNO) == 0) {
+	/* 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
+	     * Find and decrypt the keys from the history that we want,
-	     * the caller will need.
+	     * 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 &&
+	if ((flags & HDB_F_ALL_KVNOS)) {
-	    kvno < entry->entry.kvno &&
+	    /* Decrypt the history, post current/requested switcheroo */
-	    kvno > 0) {
+	    ret = hdb_unseal_keys_kvno(context, db, 0, &entry->entry);
-
-	    /* Decrypt the keys we were asked for, if not the current ones */
-	    ret = hdb_unseal_keys_kvno(context, db, kvno, &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;

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

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
+	 * This is the keyset we're being asked for, but it's not the
-	 * current keyset to the history, leave the one we were asked
+	 * current keyset.  So we add the current keyset to the history,
-	 * for in the history, and pretend the one we were asked for is
+	 * leave the one we were asked for in the history, and pretend
-	 * also the current keyset.
+	 * 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

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);

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);