krb5 commit: Add new DAL function for renaming principals

Greg Hudson ghudson at mit.edu
Mon May 23 16:25:51 EDT 2016


https://github.com/krb5/krb5/commit/c38838be956ce72fcd7142f14bc374dc13dd8bb2
commit c38838be956ce72fcd7142f14bc374dc13dd8bb2
Author: Sarah Day <sarahday at mit.edu>
Date:   Thu Mar 31 17:49:55 2016 -0400

    Add new DAL function for renaming principals
    
    Previously libkadm5srv renamed principals by getting the principal
    entry, renaming the entry, putting it in the DB, then deleting the old
    one.  This does not work in certain KDB modules such as LDAP.  A new
    DAL function is necessary to support all KDB modules.  Add a new DAL
    function to support custom renames in all KDB modules, with a default
    implementation that performs the previous functionality of adding and
    deleting the principal entry.
    
    NOTE: if the default rename function isn't used and iprop logging is
    enabled, iprop would fail since it doesn't formally support renaming.
    In that case, the call to krb5_db_rename_principal() will fail with
    the code KRB5_PLUGIN_OP_NOTSUPP.
    
    ticket: 8065

 src/include/kdb.h                 |   23 +++++++++-
 src/lib/kadm5/srv/svr_principal.c |   86 ++++++------------------------------
 src/lib/kdb/kdb5.c                |   33 ++++++++++++++
 src/lib/kdb/kdb_default.c         |   39 +++++++++++++++++
 src/lib/kdb/libkdb5.exports       |    1 +
 src/plugins/kdb/db2/db2_exp.c     |    1 +
 src/plugins/kdb/ldap/ldap_exp.c   |    1 +
 src/plugins/kdb/test/kdb_test.c   |    1 +
 src/tests/t_iprop.py              |   25 ++++++++++-
 9 files changed, 136 insertions(+), 74 deletions(-)

diff --git a/src/include/kdb.h b/src/include/kdb.h
index 63eadc4..a2ac3d3 100644
--- a/src/include/kdb.h
+++ b/src/include/kdb.h
@@ -379,6 +379,9 @@ krb5_error_code krb5_db_put_principal ( krb5_context kcontext,
                                         krb5_db_entry *entry );
 krb5_error_code krb5_db_delete_principal ( krb5_context kcontext,
                                            krb5_principal search_for );
+krb5_error_code krb5_db_rename_principal ( krb5_context kcontext,
+                                           krb5_principal source,
+                                           krb5_principal target );
 
 /*
  * Iterate over principals in the KDB.  If the callback may write to the DB,
@@ -766,6 +769,11 @@ krb5_dbe_def_encrypt_key_data( krb5_context             context,
                                krb5_key_data          * key_data);
 
 krb5_error_code
+krb5_db_def_rename_principal( krb5_context kcontext,
+                              krb5_const_principal source,
+                              krb5_const_principal target);
+
+krb5_error_code
 krb5_db_create_policy( krb5_context kcontext,
                        osa_policy_ent_t policy);
 
@@ -841,7 +849,7 @@ krb5_dbe_free_string(krb5_context, char *);
  * This number indicates the date of the last incompatible change to the DAL.
  * The maj_ver field of the module's vtable structure must match this version.
  */
-#define KRB5_KDB_DAL_MAJOR_VERSION 5
+#define KRB5_KDB_DAL_MAJOR_VERSION 6
 
 /*
  * A krb5_context can hold one database object.  Modules should use
@@ -1036,6 +1044,19 @@ typedef struct _kdb_vftabl {
                                         krb5_const_principal search_for);
 
     /*
+     * Optional with default: Rename a principal.  If the source principal does
+     * not exist, return KRB5_KDB_NOENTRY.  If the target exists, return an
+     * error.
+     *
+     * NOTE: If the module chooses to implement a custom function for renaming
+     * a principal instead of using the default, then rename operations will
+     * fail if iprop logging is enabled.
+     */
+    krb5_error_code (*rename_principal)(krb5_context kcontext,
+                                        krb5_const_principal source,
+                                        krb5_const_principal target);
+
+    /*
      * Optional: For each principal entry in the database, invoke func with the
      * argments func_arg and the entry data.  If match_entry is specified, the
      * module may narrow the iteration to principal names matching that regular
diff --git a/src/lib/kadm5/srv/svr_principal.c b/src/lib/kadm5/srv/svr_principal.c
index 800e8db..5a340b6 100644
--- a/src/lib/kadm5/srv/svr_principal.c
+++ b/src/lib/kadm5/srv/svr_principal.c
@@ -86,25 +86,6 @@ kadm5_copy_principal(krb5_context context, krb5_const_principal inprinc, krb5_pr
     return 0;
 }
 
-static void
-kadm5_free_principal(krb5_context context, krb5_principal val)
-{
-    register krb5_int32 i;
-
-    if (!val)
-        return;
-
-    if (val->data) {
-        i = krb5_princ_size(context, val);
-        while(--i >= 0)
-            krb5_db_free(context, krb5_princ_component(context, val, i)->data);
-        krb5_db_free(context, val->data);
-    }
-    if (val->realm.data)
-        krb5_db_free(context, val->realm.data);
-    krb5_db_free(context, val);
-}
-
 /*
  * XXX Functions that ought to be in libkrb5.a, but aren't.
  */
@@ -784,9 +765,6 @@ kadm5_rename_principal(void *server_handle,
     osa_princ_ent_rec adb;
     krb5_error_code ret;
     kadm5_server_handle_t handle = server_handle;
-    krb5_int16 stype, i;
-    krb5_data *salt = NULL;
-    krb5_tl_data tl;
 
     CHECK_HANDLE(server_handle);
 
@@ -800,62 +778,28 @@ kadm5_rename_principal(void *server_handle,
         return(KADM5_DUP);
     }
 
-    if ((ret = kdb_get_entry(handle, source, &kdb, &adb)))
-        return ret;
-
-    /*
-     * This rename procedure does not work with the LDAP KDB module (see issue
-     * #8065).  As a stopgap, look for tl-data indicating LDAP and error out.
-     * 0x7FFE is KDB_TL_USER_INFO as defined in kdb_ldap.h.
-     */
-    tl.tl_data_type = 0x7FFE;
-    if (krb5_dbe_lookup_tl_data(handle->context, kdb, &tl) == 0 &&
-        tl.tl_data_length > 0) {
-        ret = KRB5_PLUGIN_OP_NOTSUPP;
-        goto done;
-    }
-
-    /* Transform salts as necessary. */
-    for (i = 0; i < kdb->n_key_data; i++) {
-        ret = krb5_dbe_compute_salt(handle->context, &kdb->key_data[i],
-                                    kdb->princ, &stype, &salt);
-        if (ret == KRB5_KDB_BAD_SALTTYPE)
-            ret = KADM5_NO_RENAME_SALT;
-        if (ret)
-            goto done;
-        kdb->key_data[i].key_data_type[1] = KRB5_KDB_SALTTYPE_SPECIAL;
-        free(kdb->key_data[i].key_data_contents[1]);
-        kdb->key_data[i].key_data_contents[1] = (krb5_octet *)salt->data;
-        kdb->key_data[i].key_data_length[1] = salt->length;
-        kdb->key_data[i].key_data_ver = 2;
-        free(salt);
-        salt = NULL;
-    }
-
-    kadm5_free_principal(handle->context, kdb->princ);
-    ret = kadm5_copy_principal(handle->context, target, &kdb->princ);
-    if (ret) {
-        kdb->princ = NULL; /* so freeing the dbe doesn't lose */
-        goto done;
-    }
-
     ret = k5_kadm5_hook_rename(handle->context, handle->hook_handles,
                                KADM5_HOOK_STAGE_PRECOMMIT, source, target);
     if (ret)
-        goto done;
+        return ret;
 
-    if ((ret = kdb_put_entry(handle, kdb, &adb)))
-        goto done;
+    ret = krb5_db_rename_principal(handle->context, source, target);
+    if (ret)
+        return ret;
+
+    /* Update the principal mod data. */
+    ret = kdb_get_entry(handle, target, &kdb, &adb);
+    if (ret)
+        return ret;
+    kdb->mask = 0;
+    ret = kdb_put_entry(handle, kdb, &adb);
+    kdb_free_entry(handle, kdb, &adb);
+    if (ret)
+        return ret;
 
     (void) k5_kadm5_hook_rename(handle->context, handle->hook_handles,
                                 KADM5_HOOK_STAGE_POSTCOMMIT, source, target);
-
-    ret = kdb_delete_entry(handle, source);
-
-done:
-    krb5_free_data(handle->context, salt);
-    kdb_free_entry(handle, kdb, &adb);
-    return ret;
+    return 0;
 }
 
 kadm5_ret_t
diff --git a/src/lib/kdb/kdb5.c b/src/lib/kdb/kdb5.c
index 68bec6e..168a25d 100644
--- a/src/lib/kdb/kdb5.c
+++ b/src/lib/kdb/kdb5.c
@@ -298,6 +298,8 @@ kdb_setup_opt_functions(db_library lib)
         lib->vftabl.decrypt_key_data = krb5_dbe_def_decrypt_key_data;
     if (lib->vftabl.encrypt_key_data == NULL)
         lib->vftabl.encrypt_key_data = krb5_dbe_def_encrypt_key_data;
+    if (lib->vftabl.rename_principal == NULL)
+        lib->vftabl.rename_principal = krb5_db_def_rename_principal;
 }
 
 #ifdef STATIC_PLUGINS
@@ -951,6 +953,37 @@ krb5_db_delete_principal(krb5_context kcontext, krb5_principal search_for)
     return status;
 }
 
+krb5_error_code
+krb5_db_rename_principal(krb5_context kcontext, krb5_principal source,
+                         krb5_principal target)
+{
+    kdb_vftabl *v;
+    krb5_error_code status;
+    krb5_db_entry *entry;
+
+    status = get_vftabl(kcontext, &v);
+    if (status)
+        return status;
+
+    /*
+     * If the default rename function isn't used and logging is enabled, iprop
+     * would fail since it doesn't formally support renaming.  In that case
+     * return KRB5_PLUGIN_OP_NOTSUPP.
+     */
+    if (v->rename_principal != krb5_db_def_rename_principal &&
+        logging(kcontext))
+        return KRB5_PLUGIN_OP_NOTSUPP;
+
+    status = krb5_db_get_principal(kcontext, target, KRB5_KDB_FLAG_ALIAS_OK,
+                                   &entry);
+    if (status == 0) {
+        krb5_db_free_principal(kcontext, entry);
+        return KRB5_KDB_INUSE;
+    }
+
+    return v->rename_principal(kcontext, source, target);
+}
+
 /*
  * Use a proxy function for iterate so that we can sort the keys before sending
  * them to the callback.
diff --git a/src/lib/kdb/kdb_default.c b/src/lib/kdb/kdb_default.c
index ebda9d6..7a75148 100644
--- a/src/lib/kdb/kdb_default.c
+++ b/src/lib/kdb/kdb_default.c
@@ -538,3 +538,42 @@ clean_n_exit:
         krb5_dbe_free_key_list(context, mkey_list_head);
     return retval;
 }
+
+krb5_error_code
+krb5_db_def_rename_principal(krb5_context kcontext,
+                             krb5_const_principal source,
+                             krb5_const_principal target)
+{
+    krb5_db_entry *kdb = NULL;
+    krb5_principal oldprinc;
+    krb5_error_code ret;
+
+    if (source == NULL || target == NULL)
+        return EINVAL;
+
+    ret = krb5_db_get_principal(kcontext, source, KRB5_KDB_FLAG_ALIAS_OK,
+                                &kdb);
+    if (ret)
+        goto cleanup;
+
+    /* Store salt values explicitly so that they don't depend on the principal
+     * name. */
+    ret = krb5_dbe_specialize_salt(kcontext, kdb);
+    if (ret)
+        goto cleanup;
+
+    /* Temporarily alias kdb->princ to target and put the principal entry. */
+    oldprinc = kdb->princ;
+    kdb->princ = (krb5_principal)target;
+    ret = krb5_db_put_principal(kcontext, kdb);
+    kdb->princ = oldprinc;
+    if (ret)
+        goto cleanup;
+
+    ret = krb5_db_delete_principal(kcontext, (krb5_principal)source);
+
+
+cleanup:
+    krb5_db_free_principal(kcontext, kdb);
+    return ret;
+}
diff --git a/src/lib/kdb/libkdb5.exports b/src/lib/kdb/libkdb5.exports
index 60ab4b2..130f8d8 100644
--- a/src/lib/kdb/libkdb5.exports
+++ b/src/lib/kdb/libkdb5.exports
@@ -24,6 +24,7 @@ krb5_db_lock
 krb5_db_mkey_list_alias
 krb5_db_put_principal
 krb5_db_refresh_config
+krb5_db_rename_principal
 krb5_db_set_context
 krb5_db_setup_mkey_name
 krb5_db_sign_authdata
diff --git a/src/plugins/kdb/db2/db2_exp.c b/src/plugins/kdb/db2/db2_exp.c
index 529b943..a666123 100644
--- a/src/plugins/kdb/db2/db2_exp.c
+++ b/src/plugins/kdb/db2/db2_exp.c
@@ -218,6 +218,7 @@ kdb_vftabl PLUGIN_SYMBOL_NAME(krb5_db2, kdb_function_table) = {
     /* free_principal */                wrap_krb5_db2_free_principal,
     /* put_principal */                 wrap_krb5_db2_put_principal,
     /* delete_principal */              wrap_krb5_db2_delete_principal,
+    /* rename_principal */              NULL,
     /* iterate */                       wrap_krb5_db2_iterate,
     /* create_policy */                 wrap_krb5_db2_create_policy,
     /* get_policy */                    wrap_krb5_db2_get_policy,
diff --git a/src/plugins/kdb/ldap/ldap_exp.c b/src/plugins/kdb/ldap/ldap_exp.c
index f71a379..66f4891 100644
--- a/src/plugins/kdb/ldap/ldap_exp.c
+++ b/src/plugins/kdb/ldap/ldap_exp.c
@@ -61,6 +61,7 @@ kdb_vftabl PLUGIN_SYMBOL_NAME(krb5_ldap, kdb_function_table) = {
     /* free_principal */                    krb5_ldap_free_principal,
     /* put_principal */                     krb5_ldap_put_principal,
     /* delete_principal */                  krb5_ldap_delete_principal,
+    /* rename_principal */                  NULL,
     /* iterate */                           krb5_ldap_iterate,
     /* create_policy */                     krb5_ldap_create_password_policy,
     /* get_policy */                        krb5_ldap_get_password_policy,
diff --git a/src/plugins/kdb/test/kdb_test.c b/src/plugins/kdb/test/kdb_test.c
index db939b9..d8c2c54 100644
--- a/src/plugins/kdb/test/kdb_test.c
+++ b/src/plugins/kdb/test/kdb_test.c
@@ -559,6 +559,7 @@ kdb_vftabl PLUGIN_SYMBOL_NAME(krb5_test, kdb_function_table) = {
     test_free_principal,
     NULL, /* put_principal */
     NULL, /* delete_principal */
+    NULL, /* rename_principal */
     NULL, /* iterate */
     NULL, /* create_policy */
     NULL, /* get_policy */
diff --git a/src/tests/t_iprop.py b/src/tests/t_iprop.py
index 6b38b8a..71dbfb2 100755
--- a/src/tests/t_iprop.py
+++ b/src/tests/t_iprop.py
@@ -342,14 +342,35 @@ out = realm.run([kadminl, 'getprinc', pr3], env=slave2, expected_code=1)
 if 'Principal does not exist' not in out:
     fail('slave2 does not have principal deletion from slave1')
 
+# Rename a principal and test that it propagates incrementally.
+renpr = "quacked@" + realm.realm
+realm.run([kadminl, 'renprinc', pr1, renpr])
+check_ulog(6, 1, 6, [None, pr1, pr3, renpr, pr1, renpr])
+kpropd1.send_signal(signal.SIGUSR1)
+wait_for_prop(kpropd1, False, 3, 6)
+check_ulog(6, 1, 6, [None, pr1, pr3, renpr, pr1, renpr], slave1)
+out = realm.run([kadminl, 'getprinc', pr1], env=slave1, expected_code=1)
+if 'Principal does not exist' not in out:
+    fail('slave1 does not have principal deletion from master')
+realm.run([kadminl, 'getprinc', renpr], env=slave1)
+kpropd2.send_signal(signal.SIGUSR1)
+wait_for_prop(kpropd2, False, 3, 6)
+check_ulog(6, 1, 6, [None, pr1, pr3, renpr, pr1, renpr], slave2)
+out = realm.run([kadminl, 'getprinc', pr1], env=slave2, expected_code=1)
+if 'Principal does not exist' not in out:
+    fail('slave2 does not have principal deletion from master')
+realm.run([kadminl, 'getprinc', renpr], env=slave2)
+
+pr1 = renpr
+
 # Reset the ulog on the master to force a full resync.
 realm.run([kproplog, '-R'])
 check_ulog(1, 1, 1, [None])
 kpropd1.send_signal(signal.SIGUSR1)
-wait_for_prop(kpropd1, True, 3, 1)
+wait_for_prop(kpropd1, True, 6, 1)
 check_ulog(1, 1, 1, [None], slave1)
 kpropd2.send_signal(signal.SIGUSR1)
-wait_for_prop(kpropd2, True, 3, 1)
+wait_for_prop(kpropd2, True, 6, 1)
 check_ulog(1, 1, 1, [None], slave2)
 
 # Stop the kprop daemons so we can test kpropd -t.


More information about the cvs-krb5 mailing list