]> eyrie.org Git - kerberos/krb5-sync.git/commitdiff
Add ad_queue_only to force queuing of all changes
authorRuss Allbery <eagle@eyrie.org>
Sat, 16 Nov 2013 00:27:09 +0000 (16:27 -0800)
committerRuss Allbery <eagle@eyrie.org>
Sat, 16 Nov 2013 00:27:09 +0000 (16:27 -0800)
Add a new boolean krb5.conf option, ad_queue_only, which, if set to
true, forces all changes to be queued even if there are no conflicting
changes already queued.  The changes can then be processed later with
krb5-sync-backend.  This can be useful if real-time updates to Active
Directory cause performance issues in kadmind or kpasswdd.  kpasswd
clients in particular are often intolerant of delays.

.gitignore
Makefile.am
NEWS
README
plugin/api.c
plugin/internal.h
tests/TESTS
tests/data/queue.conf [new file with mode: 0644]
tests/plugin/queue-only-t.c [new file with mode: 0644]

index e5b38cba8a204205e63b0d2e9529feee858f6559..89f31a2927f34521ff512ffb45723840334b9e6e 100644 (file)
@@ -17,6 +17,7 @@
 /stamp-h1
 /tests/plugin/heimdal-t
 /tests/plugin/mit-t
+/tests/plugin/queue-only-t
 /tests/plugin/queuing-t
 /tests/portable/asprintf-t
 /tests/portable/snprintf-t
index 85f404c57dc2c6bd7c0be9c392915dac8f7e4a36..01cc873423218171f0e3064e7bea1a74f5a0c447 100644 (file)
@@ -1,7 +1,7 @@
 # Automake makefile for krb5-sync.
 #
 # Written by Russ Allbery <rra@stanford.edu>
-# Copyright 2006, 2007, 2010, 2012
+# Copyright 2006, 2007, 2010, 2012, 2013
 #     The Board of Trustees of the Leland Stanford Junior University
 #
 # See LICENSE for licensing terms.
@@ -10,8 +10,8 @@ ACLOCAL_AMFLAGS = -I m4
 EXTRA_DIST = .gitignore LICENSE autogen patches/README                     \
        patches/heimdal-1.3.1 patches/mit-krb5-1.4.4 patches/mit-krb5-1.8.3 \
        tests/README tests/TESTS tests/data/empty.conf tests/data/krb5.conf \
-       tests/docs/pod-spelling-t tests/docs/pod-t tests/tap/libtap.sh      \
-       tests/util/xmalloc-t tools/krb5-sync.pod
+       tests/data/queue.conf tests/docs/pod-spelling-t tests/docs/pod-t    \
+       tests/tap/libtap.sh tests/util/xmalloc-t tools/krb5-sync.pod
 
 AM_CPPFLAGS = $(KRB5_CPPFLAGS)
 
@@ -84,11 +84,12 @@ warnings:
        $(MAKE) V=0 CFLAGS='$(WARNINGS)' $(check_PROGRAMS)
 
 # The bits below are for the test suite, not for the main package.
-check_PROGRAMS = tests/runtests tests/plugin/heimdal-t tests/plugin/mit-t   \
-        tests/plugin/queuing-t tests/portable/asprintf-t                   \
-        tests/portable/snprintf-t tests/portable/strlcat-t                 \
-        tests/portable/strlcpy-t tests/portable/strndup-t                  \
-        tests/util/messages-krb5-t tests/util/messages-t tests/util/xmalloc
+check_PROGRAMS = tests/runtests tests/plugin/heimdal-t tests/plugin/mit-t \
+        tests/plugin/queue-only-t tests/plugin/queuing-t                 \
+        tests/portable/asprintf-t tests/portable/snprintf-t              \
+        tests/portable/strlcat-t tests/portable/strlcpy-t                \
+        tests/portable/strndup-t tests/util/messages-krb5-t              \
+        tests/util/messages-t tests/util/xmalloc
 check_LIBRARIES = tests/tap/libtap.a
 tests_runtests_CPPFLAGS = -DSOURCE='"$(abs_top_srcdir)/tests"' \
        -DBUILD='"$(abs_top_builddir)/tests"'
@@ -103,6 +104,12 @@ tests_plugin_heimdal_t_LDADD = tests/tap/libtap.a portable/libportable.la \
        $(DL_LIBS)
 tests_plugin_mit_t_LDADD = tests/tap/libtap.a portable/libportable.la \
        $(DL_LIBS)
+tests_plugin_queue_only_t_SOURCES = tests/plugin/queue-only-t.c \
+       $(plugin_krb5_sync_la_SOURCES)
+tests_plugin_queue_only_t_CPPFLAGS = $(LDAP_CPPFLAGS) $(AM_CPPFLAGS)
+tests_plugin_queue_only_t_LDFLAGS = $(LDAP_LDFLAGS) $(KRB5_LDFLAGS)
+tests_plugin_queue_only_t_LDADD = tests/tap/libtap.a portable/libportable.la \
+       $(LDAP_LIBS) $(KRB5_LIBS)
 tests_plugin_queuing_t_SOURCES = tests/plugin/queuing-t.c \
        $(plugin_krb5_sync_la_SOURCES)
 tests_plugin_queuing_t_CPPFLAGS = $(LDAP_CPPFLAGS) $(AM_CPPFLAGS)
diff --git a/NEWS b/NEWS
index a3d8fa0ae0411f4973c7d4eb419720f7d614e6c0..dd2fd2c35e978fd6fe6bfaec288db4ad35ac33a5 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,13 @@
 
 krb5-sync 2.4 (unreleased)
 
+    Add a new boolean krb5.conf option, ad_queue_only, which, if set to
+    true, forces all changes to be queued even if there are no conflicting
+    changes already queued.  The changes can then be processed later with
+    krb5-sync-backend.  This can be useful if real-time updates to Active
+    Directory cause performance issues in kadmind or kpasswdd.  kpasswd
+    clients in particular are often intolerant of delays.
+
     krb5-sync-backend supports a new flag, -d, which specifies the
     location of the queue directory, changing the default of
     /var/spool/krb5-sync.
diff --git a/README b/README
index dc8bef45e1e38deffbb10fd94c2ad660f6e95ccd..5bc7dd3ed3f776f6cc894c42ea7648403ae2f712 100644 (file)
--- a/README
+++ b/README
@@ -220,6 +220,7 @@ CONFIGURATION
           ad_admin_server = dc1.windows.example.com
           ad_ldap_base    = ou=People
           ad_instances    = root ipass
+          ad_queue_only   = false
 
           queue_dir       = /var/spool/krb5-sync
       }
@@ -246,6 +247,14 @@ CONFIGURATION
   Active Directory, so you can deactivate this plugin while still loading
   it by removing that part of the configuration.
 
+  The ad_queue_only option controls whether we attempt to push changes
+  directly to Active Directory or always queue them.  It can be set to
+  true to write all changes to the queue where they can be processed later
+  (by krb5-sync-backend, for example).  This may be helpful if the delay
+  from pushing changes to Active Directory causes problems for clients
+  (such as kpasswd clients, which are aggressive about retries and don't
+  like long delays).
+
   The queue_dir setting specifies where to queue changes that couldn't be
   made.  If password changes fail in AD, the whole password change is
   failed, but status changes are done before synchronization with AD is
index 2dfa86c119a44e4277e8f1fe0a12c209e90b42a5..cb821b3806755b5da467b222bbc7b5e6502ac596 100644 (file)
@@ -13,7 +13,7 @@
  * Written by Russ Allbery <rra@stanford.edu>
  * Based on code developed by Derrick Brashear and Ken Hornstein of Sine
  *     Nomine Associates, on behalf of Stanford University.
- * Copyright 2006, 2007, 2010
+ * Copyright 2006, 2007, 2010, 2013
  *     The Board of Trustees of the Leland Stanford Junior University
  *
  * See LICENSE for licensing terms.
@@ -48,6 +48,25 @@ config_string(krb5_context ctx, const char *opt, char **result)
 }
 
 
+/*
+ * Load a boolean option from Kerberos appdefaults, setting the default to
+ * false if the setting was not found.
+ */
+static void
+config_boolean(krb5_context ctx, const char *opt, bool *result)
+{
+    int tmp;
+
+    /*
+     * The MIT version of krb5_appdefault_boolean takes an int * and the
+     * Heimdal version takes a krb5_boolean *, so hope that Heimdal always
+     * defines krb5_boolean to int or this will require more portability work.
+     */
+    krb5_appdefault_boolean(ctx, "krb5-strength", NULL, opt, *result, &tmp);
+    *result = tmp;
+}
+
+
 /*
  * Initialize the module.  This consists solely of loading our configuration
  * options from krb5.conf into a newly allocated struct stored in the second
@@ -68,6 +87,7 @@ pwupdate_init(krb5_context ctx, void **data)
     config_string(ctx, "ad_admin_server", &config->ad_admin_server);
     config_string(ctx, "ad_ldap_base", &config->ad_ldap_base);
     config_string(ctx, "ad_instances", &config->ad_instances);
+    config_boolean(ctx, "ad_queue_only", &config->ad_queue_only);
     config_string(ctx, "queue_dir", &config->queue_dir);
     *data = config;
     return 0;
@@ -225,6 +245,8 @@ pwupdate_precommit_password(void *data, krb5_principal principal,
         return 0;
     if (pwupdate_queue_conflict(config, ctx, principal, "ad", "password"))
         goto queue;
+    if (config->ad_queue_only)
+        goto queue;
     status = pwupdate_ad_change(config, ctx, principal, password, pwlen,
                                 errstr, errstrlen);
     if (status == 3) {
@@ -290,6 +312,8 @@ pwupdate_postcommit_status(void *data, krb5_principal principal, int enabled,
         return 0;
     if (pwupdate_queue_conflict(config, ctx, principal, "ad", "enable"))
         goto queue;
+    if (config->ad_queue_only)
+        goto queue;
     status = pwupdate_ad_status(config, ctx, principal, enabled, errstr,
                                 errstrlen);
     if (status != 0)
index 56790c188562450884b6bae5b67e53ab102fa63f..0e914019a2dd31f79b4019b3deb562cb9e611d13 100644 (file)
@@ -4,7 +4,7 @@
  * Written by Russ Allbery <rra@stanford.edu>
  * Based on code developed by Derrick Brashear and Ken Hornstein of Sine
  *     Nomine Associates, on behalf of Stanford University.
- * Copyright 2006, 2007, 2010
+ * Copyright 2006, 2007, 2010, 2013
  *     The Board of Trustees of the Leland Stanford Junior University
  *
  * See LICENSE for licensing terms.
@@ -32,6 +32,7 @@ struct plugin_config {
     char *ad_admin_server;
     char *ad_ldap_base;
     char *ad_instances;
+    bool ad_queue_only;
     char *queue_dir;
 };
 
index 2a96a85b1c0ad4190b6697b8628bdd10c7d4204e..762f67e1c736973e24f613aca3ee5c6ba0d79b24 100644 (file)
@@ -2,6 +2,7 @@ docs/pod
 docs/pod-spelling
 plugin/heimdal
 plugin/mit
+plugin/queue-only
 plugin/queuing
 portable/asprintf
 portable/snprintf
diff --git a/tests/data/queue.conf b/tests/data/queue.conf
new file mode 100644 (file)
index 0000000..2c228e8
--- /dev/null
@@ -0,0 +1,17 @@
+# Test krb5.conf for testing the plugins.  This doesn't have to represent
+# any actual realm.  Note use of a relative path for the queue directory.
+
+[appdefaults]
+    krb5-sync = {
+        ad_keytab         = ad-keytab
+        ad_principal      = service/krb5-sync@EXAMPLE.COM
+        ad_realm          = AD.EXAMPLE.COM
+        ad_admin_server   = ad.example.com
+        ad_instances      = exclude
+        ad_queue_only     = true
+
+        queue_dir         = queue
+    }
+
+[libdefaults]
+    default_realm         = EXAMPLE.COM
diff --git a/tests/plugin/queue-only-t.c b/tests/plugin/queue-only-t.c
new file mode 100644 (file)
index 0000000..e9b367a
--- /dev/null
@@ -0,0 +1,200 @@
+/*
+ * Tests for forced queuing in the krb5-sync plugin.
+ *
+ * Disable immediate changes and force queuing, and test that this works
+ * correctly.
+ *
+ * Written by Russ Allbery <eagle@eyrie.org>
+ * Copyright 2013
+ *     The Board of Trustees of the Leland Stanford Junior University
+ *
+ * See LICENSE for licensing terms.
+ */
+
+#include <config.h>
+#include <portable/krb5.h>
+#include <portable/system.h>
+
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <time.h>
+
+#include <plugin/internal.h>
+#include <tests/tap/basic.h>
+#include <tests/tap/string.h>
+
+
+int
+main(void)
+{
+    char *tmpdir, *krb5conf, *env, *queue;
+    krb5_context ctx;
+    krb5_principal princ;
+    krb5_error_code code;
+    void *data;
+    char errstr[BUFSIZ], buffer[BUFSIZ];
+    time_t now, try;
+    struct tm *date;
+    FILE *file;
+    struct stat st;
+
+    tmpdir = test_tmpdir();
+    if (chdir(tmpdir) < 0)
+        sysbail("cannot cd to %s", tmpdir);
+    krb5conf = test_file_path("data/queue.conf");
+    if (krb5conf == NULL)
+        bail("cannot find tests/data/queue.conf");
+    if (mkdir("queue", 0777) < 0)
+        sysbail("cannot mkdir queue");
+    basprintf(&env, "KRB5_CONFIG=%s", krb5conf);
+    if (putenv(env) < 0)
+        sysbail("cannot set KRB5CCNAME");
+    code = krb5_init_context(&ctx);
+    if (code != 0)
+        bail("cannot create Kerberos context (%d)", (int) code);
+    code = krb5_parse_name(ctx, "test@EXAMPLE.COM", &princ);
+    if (code != 0)
+        bail("cannot parse principal: %s", krb5_get_error_message(ctx, code));
+
+    plan(26);
+
+    /* Test init. */
+    is_int(0, pwupdate_init(ctx, &data), "pwupdate_init succeeds");
+    ok(data != NULL, "...and data is non-NULL");
+
+    /* Create a password change and be sure it's queued. */
+    code = pwupdate_precommit_password(data, princ, "foobar", strlen("foobar"),
+                                       errstr, sizeof(errstr));
+    is_int(0, code, "pwupdate_precommit_password succeeds");
+    queue = NULL;
+    now = time(NULL);
+    for (try = now - 1; try <= now; try++) {
+        date = gmtime(&try);
+        basprintf(&queue,
+                  "queue/test-ad-password-%04d%02d%02dT%02d%02d%02dZ-00",
+                  date->tm_year + 1900, date->tm_mon + 1, date->tm_mday,
+                  date->tm_hour, date->tm_min, date->tm_sec);
+        if (access(queue, F_OK) == 0)
+            break;
+        free(queue);
+        queue = NULL;
+    }
+    ok(queue != NULL, "...password change was queued");
+    if (queue == NULL)
+        ok_block(5, false, "No queued change to check");
+    else {
+        if (stat(queue, &st) < 0)
+            sysbail("cannot stat %s", queue);
+        is_int(0600, st.st_mode & 0777, "...mode of queue file is correct");
+        file = fopen(queue, "r");
+        if (file == NULL)
+            sysbail("cannot open %s", queue);
+        if (fgets(buffer, sizeof(buffer), file) == NULL)
+            buffer[0] = '\0';
+        is_string("test\n", buffer, "...queued user is correct");
+        if (fgets(buffer, sizeof(buffer), file) == NULL)
+            buffer[0] = '\0';
+        is_string("ad\n", buffer, "...queued domain is correct");
+        if (fgets(buffer, sizeof(buffer), file) == NULL)
+            buffer[0] = '\0';
+        is_string("password\n", buffer, "...queued operation is correct");
+        if (fgets(buffer, sizeof(buffer), file) == NULL)
+            buffer[0] = '\0';
+        is_string("foobar\n", buffer, "...queued password is correct");
+        fclose(file);
+    }
+    ok(unlink(queue) == 0, "Remove queued password change");
+    free(queue);
+
+    /* Test queuing of enable. */
+    errstr[0] = '\0';
+    code = pwupdate_postcommit_status(data, princ, 1, errstr, sizeof(errstr));
+    is_int(0, code, "pwupdate_postcommit_status enable succeeds");
+    is_string("", errstr, "...and there is no error");
+    queue = NULL;
+    now = time(NULL);
+    for (try = now - 1; try <= now; try++) {
+        date = gmtime(&try);
+        basprintf(&queue, "queue/test-ad-enable-%04d%02d%02dT%02d%02d%02dZ-00",
+                  date->tm_year + 1900, date->tm_mon + 1, date->tm_mday,
+                  date->tm_hour, date->tm_min, date->tm_sec);
+        if (access(queue, F_OK) == 0)
+            break;
+        free(queue);
+        queue = NULL;
+    }
+    ok(queue != NULL, "...enable was queued");
+    if (queue == NULL)
+        ok_block(3, false, "No queued change to check");
+    else {
+        file = fopen(queue, "r");
+        if (file == NULL)
+            sysbail("cannot open %s", queue);
+        if (fgets(buffer, sizeof(buffer), file) == NULL)
+            buffer[0] = '\0';
+        is_string("test\n", buffer, "...queued user is correct");
+        if (fgets(buffer, sizeof(buffer), file) == NULL)
+            buffer[0] = '\0';
+        is_string("ad\n", buffer, "...queued domain is correct");
+        if (fgets(buffer, sizeof(buffer), file) == NULL)
+            buffer[0] = '\0';
+        is_string("enable\n", buffer, "...queued operation is correct");
+        fclose(file);
+    }
+    ok(unlink(queue) == 0, "Remove queued enable");
+
+    /* Test queuing of disable. */
+        errstr[0] = '\0';
+    code = pwupdate_postcommit_status(data, princ, 0, errstr, sizeof(errstr));
+    is_int(0, code, "pwupdate_postcommit_status disable succeeds");
+    is_string("", errstr, "...and there is no error");
+    queue = NULL;
+    now = time(NULL);
+    for (try = now - 1; try <= now; try++) {
+        date = gmtime(&try);
+        basprintf(&queue, "queue/test-ad-enable-%04d%02d%02dT%02d%02d%02dZ-00",
+                  date->tm_year + 1900, date->tm_mon + 1, date->tm_mday,
+                  date->tm_hour, date->tm_min, date->tm_sec);
+        if (access(queue, F_OK) == 0)
+            break;
+        free(queue);
+        queue = NULL;
+    }
+    ok(queue != NULL, "...enable was queued");
+    if (queue == NULL)
+        ok_block(3, false, "No queued change to check");
+    else {
+        file = fopen(queue, "r");
+        if (file == NULL)
+            sysbail("cannot open %s", queue);
+        if (fgets(buffer, sizeof(buffer), file) == NULL)
+            buffer[0] = '\0';
+        is_string("test\n", buffer, "...queued user is correct");
+        if (fgets(buffer, sizeof(buffer), file) == NULL)
+            buffer[0] = '\0';
+        is_string("ad\n", buffer, "...queued domain is correct");
+        if (fgets(buffer, sizeof(buffer), file) == NULL)
+            buffer[0] = '\0';
+        is_string("disable\n", buffer, "...queued operation is correct");
+        fclose(file);
+    }
+    ok(unlink(queue) == 0, "Remove queued disable");
+    free(queue);
+
+    /* Unwind the queue and be sure all the right files exist. */
+    ok(unlink("queue/.lock") == 0, "Lock file still exists");
+    ok(rmdir("queue") == 0, "No other files in queue directory");
+
+    /* Shut down the plugin. */
+    pwupdate_close(data);
+
+    /* Clean up. */
+    krb5_free_principal(ctx, princ);
+    krb5_free_context(ctx);
+    test_file_path_free(krb5conf);
+    if (chdir("..") < 0)
+        sysbail("cannot chdir to parent directory");
+    test_tmpdir_free(tmpdir);
+    free(env);
+    return 0;
+}