[RFC] kdb: store mkey list in context and permit NULL mkey for kdb_dbe_decrypt_key_data

[Will Fiveash]

On Sat, Sep 11, 2010 at 12:37:29PM -0400, Sam Hartman wrote:
> From: Sam Hartman <hartmans at debian.org>
> 
> Previously, code needed to run a loop to find the current master key,
> possibly fetch a new master key list and try finding the master key
> again around each key decryption.  This was not universally done;
> there are cases where only the current master key was used.
> In addition, the correct ideom for decrypting key data is too complicated and is unavailable to plugins that do not have access to the master key.
> Instead, store the master key list in the dal_handle whenever it is fetched and permit a NULL master key for krb5_dbe_decrypt_key_data.
> 
> * Remove APIs for krb5_db_{get|set}_mkey_list
> * krb5_db_fetch_mkey_list: memoize master key list in dal_handle
> * krb5_db_free_mkey_list: don't free the memoized list; arrange for it to be freed later
> * krb5_dbe_decrypt_key_data: Search for correct master key on NULL argument
> * change call sites to take advantage

Since we have synced up Solaris krb with 1.8, I've noticed some memory
leaks relating to the mkey list (this was exacerbated when I added a
call to krb5_db_fetch_mkey_list() in kadm5_create_principal_3()).

Given you have added krb5_keylist_node *master_keylist to
_kdb5_dal_handle what I would like to do is eliminate the following
other instances of master_keylist:

  File                       Function Line
2 src/kdc/extern.h           <global> 91 #define master_keylist kdc_active_realm->mkey_list
3 lib/kadm5/srv/server_kdb.c <global> 21 krb5_keylist_node *master_keylist = NULL;

thus making access to the master key list only via
kcontext->dal_handle->master_keylist.  krb5_db_fetch_mkey_list() would
be modified to just update that field and not return that master_keylist
in an output parameter as it does now.  It would free the existing mkey
list if v->fetch_master_key_list() succeeded and set
kcontext->dal_handle->master_keylist to the new mkey list.  It would not
set the context->dal_handle->free_keylist as I don't think this is
needed.  kdb_free_lib_handle() would free dal_handle->master_keylist if
it was not NULL.  Is this a reasonable modification?  Note, I'd like to
do this both for MIT and Solaris implementations so they are in sync.

-- 
Will Fiveash
Oracle Solaris Software Engineer
http://opensolaris.org/os/project/kerberos/
Sent using mutt, a sweet, text based e-mail app <http://www.mutt.org/>