]> eyrie.org Git - kerberos/krb5-sync.git/commitdiff
Rework internal error handling to use Kerberos errors
authorRuss Allbery <eagle@eyrie.org>
Thu, 21 Nov 2013 02:15:08 +0000 (18:15 -0800)
committerRuss Allbery <eagle@eyrie.org>
Thu, 21 Nov 2013 02:15:08 +0000 (18:15 -0800)
Now that we've dropped the old API, we can drop the error handling
mode, which predates rich Kerberos errors.  Replace it with use of
krb5_set_error_message everywhere, and change a lot of functions
to return a krb5_error_code instead of a boolean or some special
status code.

As part of this change, all password changes are queued for Active
Directory if they fail regardless of the reason for the failure.

13 files changed:
plugin/ad.c
plugin/api.c
plugin/error.c
plugin/heimdal.c
plugin/internal.h
plugin/mit.c
plugin/queue.c
portable/kadmin.h
tests/plugin/heimdal-t.c
tests/plugin/mit-t.c
tests/plugin/queue-only-t.c
tests/plugin/queuing-t.c
tools/krb5-sync.c

index 5fe371c1b98fe2ffd4237d1a822dfbdc8c060769..9a02d53cf041d3b9cd8acc36cab7572475842e6e 100644 (file)
 #define STRINGIFY(s) #s
 #define CHECK_CONFIG(c)                                                 \
     do {                                                                \
-        if (config->c == NULL) {                                        \
-            pwupdate_set_error(errstr, errstrlen, NULL, 0,              \
-                               "configuration setting %s missing",      \
-                               STRINGIFY(c));                           \
-            return 1;                                                   \
-        }                                                               \
+        if (config->c == NULL)                                          \
+            return sync_error_config(ctx, "configuration setting %s"    \
+                                     " missing", STRINGIFY(c));         \
     } while (0)
 
 
 /*
- * Given the plugin options, a Kerberos context, a pointer to krb5_ccache
- * storage, and the buffer into which to store an error message if any,
- * initialize a memory cache using the configured keytab to obtain initial
- * credentials.  Return 0 on success, non-zero on failure.
+ * Given the plugin options, a Kerberos context, and a pointer to krb5_ccache
+ * storage, initialize a memory cache using the configured keytab to obtain
+ * initial credentials.  Returns a Kerberos status code.
  */
-static int
-get_creds(struct plugin_config *config, krb5_context ctx, krb5_ccache *cc,
-          char *errstr, int errstrlen)
+static krb5_error_code
+get_creds(struct plugin_config *config, krb5_context ctx, krb5_ccache *cc)
 {
-    krb5_keytab kt;
+    krb5_error_code code;
+    krb5_keytab kt = NULL;
+    krb5_principal princ = NULL;
+    krb5_get_init_creds_opt *opts = NULL;
     krb5_creds creds;
-    krb5_principal princ;
-    krb5_get_init_creds_opt *opts;
-    krb5_error_code ret;
+    bool creds_valid = false;
     const char *realm UNUSED;
 
+    /* Initialize the credential cache pointer to NULL. */
+    *cc = NULL;
+
+    /* Ensure the configuration is sane. */
     CHECK_CONFIG(ad_keytab);
     CHECK_CONFIG(ad_principal);
 
-    ret = krb5_kt_resolve(ctx, config->ad_keytab, &kt);
-    if (ret != 0) {
-        pwupdate_set_error(errstr, errstrlen, ctx, ret,
-                           "unable to resolve keytab \"%s\"",
-                           config->ad_keytab);
-        return 1;
-    }
-    ret = krb5_parse_name(ctx, config->ad_principal, &princ);
-    if (ret != 0) {
-        pwupdate_set_error(errstr, errstrlen, ctx, ret,
-                           "unable to parse principal \"%s\"",
-                           config->ad_principal);
-        return 1;
-    }
-    ret = krb5_get_init_creds_opt_alloc(ctx, &opts);
-    if (ret != 0) {
-        pwupdate_set_error(errstr, errstrlen, ctx, ret,
-                           "error allocating credential options");
-        return 1;
-    }
+    /* Resolve the keytab and principal used to get credentials. */
+    code = krb5_kt_resolve(ctx, config->ad_keytab, &kt);
+    if (code != 0)
+        goto fail;
+    code = krb5_parse_name(ctx, config->ad_principal, &princ);
+    if (code != 0)
+        goto fail;
+
+    /* Set our credential acquisition options. */
+    code = krb5_get_init_creds_opt_alloc(ctx, &opts);
+    if (code != 0)
+        goto fail;
     realm = krb5_principal_get_realm(ctx, princ);
     krb5_get_init_creds_opt_set_default_flags(ctx, "krb5-sync", realm, opts);
+
+    /* Obtain credentials. */
     memset(&creds, 0, sizeof(creds));
-    ret = krb5_get_init_creds_keytab(ctx, &creds, princ, kt, 0, NULL, opts);
-    if (ret != 0) {
-        pwupdate_set_error(errstr, errstrlen, ctx, ret,
-                           "unable to get initial credentials");
-        krb5_get_init_creds_opt_free(ctx, opts);
-        return 1;
-    }
+    code = krb5_get_init_creds_keytab(ctx, &creds, princ, kt, 0, NULL, opts);
+    if (code != 0)
+        goto fail;
     krb5_get_init_creds_opt_free(ctx, opts);
-    ret = krb5_kt_close(ctx, kt);
-    if (ret != 0) {
-        pwupdate_set_error(errstr, errstrlen, ctx, ret,
-                           "unable to close keytab");
-        return 1;
-    }
-    ret = krb5_cc_resolve(ctx, CACHE_NAME, cc);
-    if (ret != 0) {
-        pwupdate_set_error(errstr, errstrlen, ctx, ret,
-                           "unable to resolve memory cache");
-        return 1;
-    }
-    ret = krb5_cc_initialize(ctx, *cc, princ);
-    if (ret != 0) {
-        pwupdate_set_error(errstr, errstrlen, ctx, ret,
-                           "unable to initialize memory cache");
-        return 1;
-    }
-    ret = krb5_cc_store_cred(ctx, *cc, &creds);
-    if (ret != 0) {
-        pwupdate_set_error(errstr, errstrlen, ctx, ret,
-                           "unable to store credentials");
-        return 1;
+    opts = NULL;
+    krb5_kt_close(ctx, kt);
+    kt = NULL;
+    creds_valid = true;
+
+    /* Open and initialize the credential cache. */
+    code = krb5_cc_resolve(ctx, CACHE_NAME, cc);
+    if (code != 0)
+        goto fail;
+    code = krb5_cc_initialize(ctx, *cc, princ);
+    if (code != 0)
+        code = krb5_cc_store_cred(ctx, *cc, &creds);
+    if (code != 0) {
+        krb5_cc_close(ctx, *cc);
+        *cc = NULL;
+        goto fail;
     }
+
+    /* Clean up and return success. */
     krb5_free_cred_contents(ctx, &creds);
     krb5_free_principal(ctx, princ);
     return 0;
+
+fail:
+    if (kt != NULL)
+        krb5_kt_close(ctx, kt);
+    if (princ != NULL)
+        krb5_free_principal(ctx, princ);
+    if (opts != NULL)
+        krb5_get_init_creds_opt_free(ctx, opts);
+    if (creds_valid)
+        krb5_free_cred_contents(ctx, &creds);
+    return code;
 }
 
 
 /*
  * Given the krb5_principal from kadmind, convert it to the corresponding
  * principal in Active Directory.  This may involve removing ad_base_instance
- * and always involves changing the realm.  Returns 0 on success and a
- * Kerberos error code on failure.
+ * and always involves changing the realm.  Returns a Kerberos error code.
  */
 static krb5_error_code
 get_ad_principal(struct plugin_config *config, krb5_context ctx,
                  krb5_const_principal principal, krb5_principal *ad_principal)
 {
-    krb5_error_code ret;
+    krb5_error_code code;
     int ncomp;
 
     /*
@@ -161,19 +156,19 @@ get_ad_principal(struct plugin_config *config, krb5_context ctx,
         instance = krb5_principal_get_comp_string(ctx, principal, 1);
         if (strcmp(instance, config->ad_base_instance) == 0) {
             base = krb5_principal_get_comp_string(ctx, principal, 0);
-            ret = krb5_build_principal(ctx, ad_principal,
+            code = krb5_build_principal(ctx, ad_principal,
                                        strlen(config->ad_realm),
                                        config->ad_realm, base, (char *) 0);
-            if (ret != 0)
-                return ret;
+            if (code != 0)
+                return code;
         }
     }
 
     /* Otherwise, copy the principal and set the realm. */
     if (*ad_principal == NULL) {
-        ret = krb5_copy_principal(ctx, principal, ad_principal);
-        if (ret != 0)
-            return ret;
+        code = krb5_copy_principal(ctx, principal, ad_principal);
+        if (code != 0)
+            return code;
         krb5_principal_set_realm(ctx, *ad_principal, config->ad_realm);
     }
     return 0;
@@ -183,79 +178,65 @@ get_ad_principal(struct plugin_config *config, krb5_context ctx,
 /*
  * Push a password change to Active Directory.  Takes the module
  * configuration, a Kerberos context, the principal whose password is being
- * changed (we will have to change the realm), the new password and its
- * length, and a buffer into which to put error messages and its length.
- *
- * Returns 1 for any general failure, 2 if the password change was rejected by
- * the remote system, and 3 if the password change was rejected for a reason
- * that may mean that the user doesn't exist.
+ * changed (we will have to change the realm), and the new password and its
+ * length.  Returns a Kerberos error code.
  */
-int
+krb5_error_code
 pwupdate_ad_change(struct plugin_config *config, krb5_context ctx,
                    krb5_principal principal, const char *password,
-                   int pwlen UNUSED, char *errstr, int errstrlen)
+                   int pwlen UNUSED)
 {
-    krb5_error_code ret;
+    krb5_error_code code;
     char *target = NULL;
     krb5_ccache ccache;
     krb5_principal ad_principal = NULL;
     int result_code;
     krb5_data result_code_string, result_string;
-    int code = 0;
 
+    /* Ensure the configuration is sane. */
     CHECK_CONFIG(ad_realm);
 
-    if (get_creds(config, ctx, &ccache, errstr, errstrlen) != 0)
-        return 1;
+    /* Get the credentials we'll use to make the change in AD. */
+    code = get_creds(config, ctx, &ccache);
+    if (code != 0)
+        return code;
 
-    /* Get the corresponding Active Directory principal. */
-    ret = get_ad_principal(config, ctx, principal, &ad_principal);
-    if (ret != 0) {
-        pwupdate_set_error(errstr, errstrlen, ctx, ret,
-                           "unable to get AD principal");
-        code = 1;
+    /* Get the corresponding AD principal. */
+    code = get_ad_principal(config, ctx, principal, &ad_principal);
+    if (code != 0)
         goto done;
-    }
 
     /* This is just for logging purposes. */
-    ret = krb5_unparse_name(ctx, ad_principal, &target);
-    if (ret != 0) {
-        pwupdate_set_error(errstr, errstrlen, ctx, ret,
-                           "unable to parse target principal");
-        code = 1;
+    code = krb5_unparse_name(ctx, ad_principal, &target);
+    if (code != 0)
         goto done;
-    }
 
-    /* Do the actual password change. */
-    ret = krb5_set_password_using_ccache(ctx, ccache, (char *) password,
-                                         ad_principal, &result_code,
-                                         &result_code_string, &result_string);
-    krb5_free_principal(ctx, ad_principal);
-    if (ret != 0) {
-        pwupdate_set_error(errstr, errstrlen, ctx, ret,
-                           "password change failed for %s in %s",
-                           target, config->ad_realm);
-        code = 3;
+    /* Do the actual password change and record any error. */
+    code = krb5_set_password_using_ccache(ctx, ccache, (char *) password,
+                                          ad_principal, &result_code,
+                                          &result_code_string, &result_string);
+    if (code != 0)
         goto done;
-    }
     if (result_code != 0) {
-        snprintf(errstr, errstrlen, "password change failed for %s in %s:"
-                 " (%d) %.*s%s%.*s", target, config->ad_realm, result_code,
-                 result_code_string.length, (char *) result_code_string.data,
-                 result_string.length ? ": " : "",
-                 result_string.length, (char *) result_string.data);
-        code = 3;
+        code = sync_error_generic(ctx, "password change failed for %s: (%d)"
+                                  " %.*s%s%.*s", target, result_code,
+                                  result_code_string.length,
+                                  (char *) result_code_string.data,
+                                  result_string.length ? ": " : "",
+                                  result_string.length,
+                                  (char *) result_string.data);
         goto done;
     }
     free(result_string.data);
     free(result_code_string.data);
-    syslog(LOG_INFO, "pwupdate: %s password changed", target);
-    strlcpy(errstr, "Password changed", errstrlen);
+    syslog(LOG_INFO, "krb5-sync: %s password changed", target);
 
 done:
+    krb5_cc_destroy(ctx, ccache);
     if (target != NULL)
         krb5_free_unparsed_name(ctx, target);
-    krb5_cc_destroy(ctx, ccache);
+    if (ad_principal != NULL)
+        krb5_free_principal(ctx, ad_principal);
     return code;
 }
 
@@ -279,14 +260,13 @@ ad_interact_sasl(LDAP *ld UNUSED, unsigned flags UNUSED,
  * account is enabled, and a buffer into which to put error messages and its
  * length.
  */
-int
+krb5_error_code
 pwupdate_ad_status(struct plugin_config *config, krb5_context ctx,
-                   krb5_principal principal, int enabled, char *errstr,
-                   int errstrlen)
+                   krb5_principal principal, int enabled)
 {
     krb5_ccache ccache;
     krb5_principal ad_principal = NULL;
-    LDAP *ld;
+    LDAP *ld = NULL;
     LDAPMessage *res = NULL;
     LDAPMod mod, *mod_array[2];
     char ldapuri[256], ldapbase[256], ldapdn[256], *dname, *lb, *end, *dn;
@@ -295,15 +275,18 @@ pwupdate_ad_status(struct plugin_config *config, krb5_context ctx,
     char *value;
     const char *attrs[] = { "userAccountControl", NULL };
     char *strvals[2];
-    int option, ret;
+    int option;
     unsigned int acctcontrol;
-    int code = 1;
+    krb5_error_code code;
 
+    /* Ensure the configuration is sane. */
     CHECK_CONFIG(ad_admin_server);
     CHECK_CONFIG(ad_realm);
 
-    if (get_creds(config, ctx, &ccache, errstr, errstrlen) != 0)
-        return 1;
+    /* Get the credentials we'll use to make the change in AD. */
+    code = get_creds(config, ctx, &ccache);
+    if (code != 0)
+        return code;
 
     /*
      * Point SASL at the memory cache we're about to create.  This is changing
@@ -313,32 +296,28 @@ pwupdate_ad_status(struct plugin_config *config, krb5_context ctx,
      * hard.
      */
     if (putenv((char *) "KRB5CCNAME=" CACHE_NAME) != 0) {
-        snprintf(errstr, errstrlen, "putenv of KRB5CCNAME failed: %s",
-                 strerror(errno));
-        return 1;
+        code = sync_error_system(ctx, "putenv of KRB5CCNAME failed");
+        goto done;
     }
 
     /* Now, bind to the directory server using GSSAPI. */
     snprintf(ldapuri, sizeof(ldapuri), "ldap://%s", config->ad_admin_server);
-    ret = ldap_initialize(&ld, ldapuri);
-    if (ret != LDAP_SUCCESS) {
-        snprintf(errstr, errstrlen, "LDAP initialization failed: %s",
-                 ldap_err2string(ret));
-        return 1;
+    code = ldap_initialize(&ld, ldapuri);
+    if (code != LDAP_SUCCESS) {
+        code = sync_error_ldap(ctx, code, "LDAP initialization failed");
+        goto done;
     }
     option = LDAP_VERSION3;
-    ret = ldap_set_option(ld, LDAP_OPT_PROTOCOL_VERSION, &option);
-    if (ret != LDAP_SUCCESS) {
-        snprintf(errstr, errstrlen, "LDAP protocol selection failed: %s",
-                 ldap_err2string(ret));
+    code = ldap_set_option(ld, LDAP_OPT_PROTOCOL_VERSION, &option);
+    if (code != LDAP_SUCCESS) {
+        code = sync_error_ldap(ctx, code, "LDAP protocol selection failed");
         goto done;
     }
-    ret = ldap_sasl_interactive_bind_s(ld, NULL, "GSSAPI", NULL, NULL,
+    code = ldap_sasl_interactive_bind_s(ld, NULL, "GSSAPI", NULL, NULL,
                                        LDAP_SASL_QUIET, ad_interact_sasl,
                                        NULL);
-    if (ret != LDAP_SUCCESS) {
-        snprintf(errstr, errstrlen, "LDAP bind failed: %s",
-                 ldap_err2string(ret));
+    if (code != LDAP_SUCCESS) {
+        code = sync_error_ldap(ctx, code, "LDAP bind failed");
         goto done;
     }
 
@@ -371,43 +350,39 @@ pwupdate_ad_status(struct plugin_config *config, krb5_context ctx,
      * the AD principal and then query Active Directory via LDAP to get back
      * the CN for the user to construct the full DN.
      */
-    ret = get_ad_principal(config, ctx, principal, &ad_principal);
-    if (ret != 0) {
-        pwupdate_set_error(errstr, errstrlen, ctx, ret,
-                           "unable to get AD principal");
+    code = get_ad_principal(config, ctx, principal, &ad_principal);
+    if (code != 0)
         goto done;
-    }
-    ret = krb5_unparse_name(ctx, ad_principal, &target);
-    if (ret != 0) {
-        pwupdate_set_error(errstr, errstrlen, ctx, ret,
-                           "unable to parse target principal");
+    code = krb5_unparse_name(ctx, ad_principal, &target);
+    if (code != 0)
         goto done;
-    }
     snprintf(ldapdn, sizeof(ldapdn), "(userPrincipalName=%s)", target);
-    ret = ldap_search_ext_s(ld, ldapbase, LDAP_SCOPE_SUBTREE, ldapdn,
+    code = ldap_search_ext_s(ld, ldapbase, LDAP_SCOPE_SUBTREE, ldapdn,
                             (char **) attrs, 0, NULL, NULL, NULL, 0, &res);
-    if (ret != LDAP_SUCCESS) {
-        snprintf(errstr, errstrlen, "LDAP search on \"%s\" failed: %s",
-                 ldapdn, ldap_err2string(ret));
+    if (code != LDAP_SUCCESS) {
+        code = sync_error_ldap(ctx, code, "LDAP search for \"%s\" failed",
+                               ldapdn);
         goto done;
     }
     if (ldap_count_entries(ld, res) == 0) {
-        snprintf(errstr, errstrlen, "user \"%s\" not found in %s",
-                 target, config->ad_realm);
+        code = sync_error_generic(ctx, "user \"%s\" not found via LDAP",
+                                  target);
         goto done;
     }
     res = ldap_first_entry(ld, res);
     dn = ldap_get_dn(ld, res);
     if (ldap_msgtype(res) != LDAP_RES_SEARCH_ENTRY) {
-        snprintf(errstr, errstrlen, "expected msgtype of RES_SEARCH_ENTRY"
-                 " (0x61), but got type %x instead", ldap_msgtype(res));
+        code = sync_error_generic(ctx, "expected LDAP msgtype of"
+                                  " RES_SEARCH_ENTRY (0x61), but got type %x"
+                                  " instead", ldap_msgtype(res));
         goto done;
     }
     vals = ldap_get_values_len(ld, res, "userAccountControl");
     if (ldap_count_values_len(vals) != 1) {
-        snprintf(errstr, errstrlen, "expected one value for"
-                 " userAccountControl for user \"%s\" and got %d", target,
-                 ldap_count_values_len(vals));
+        code = sync_error_generic(ctx, "expected one value for"
+                                  " userAccountControl for user \"%s\" and"
+                                  " got %d", target,
+                                  ldap_count_values_len(vals));
         goto done;
     }
 
@@ -418,24 +393,22 @@ pwupdate_ad_status(struct plugin_config *config, krb5_context ctx,
      */
     value = malloc(vals[0]->bv_len + 1);
     if (value == NULL) {
-        snprintf(errstr, errstrlen, "cannot allocate memory: %s",
-                 strerror(errno));
+        code = sync_error_system(ctx, "cannot allocate memory");
         goto done;
     }
     memcpy(value, vals[0]->bv_val, vals[0]->bv_len);
     value[vals[0]->bv_len] = '\0';
     if (sscanf(value, "%u", &acctcontrol) != 1) {
         free(value);
-        snprintf(errstr, errstrlen, "unable to parse userAccountControl for"
-                 " user \"%s\" (%s)", target, value);
+        code = sync_error_generic(ctx, "unable to parse userAccountControl"
+                                  " for user \"%s\" (%s)", target, value);
         goto done;
     }
     free(value);
-    if (enabled) {
+    if (enabled)
         acctcontrol &= ~UF_ACCOUNTDISABLE;
-    } else {
+    else
         acctcontrol |= UF_ACCOUNTDISABLE;
-    }
     memset(&mod, 0, sizeof(mod));
     mod.mod_op = LDAP_MOD_REPLACE;
     mod.mod_type = (char *) "userAccountControl";
@@ -445,25 +418,27 @@ pwupdate_ad_status(struct plugin_config *config, krb5_context ctx,
     mod.mod_vals.modv_strvals = strvals;
     mod_array[0] = &mod;
     mod_array[1] = NULL;
-    ret = ldap_modify_ext_s(ld, dn, mod_array, NULL, NULL);
-    if (ret != LDAP_SUCCESS) {
-        snprintf(errstr, errstrlen, "LDAP modification for user \"%s\""
-                 " failed: %s", target, ldap_err2string(ret));
+    code = ldap_modify_ext_s(ld, dn, mod_array, NULL, NULL);
+    if (code != LDAP_SUCCESS) {
+        code = sync_error_ldap(ctx, code, "LDAP modification for user \"%s\""
+                               " failed", target);
         goto done;
     }
 
     /* Success. */
     code = 0;
-    syslog(LOG_INFO, "successfully set account %s to %s", target,
-           enabled ? "enabled" : "disabled");
+    syslog(LOG_INFO, "successfully %s account %s",
+           enabled ? "enabled" : "disabled", target);
 
 done:
+    krb5_cc_destroy(ctx, ccache);
     if (target != NULL)
         krb5_free_unparsed_name(ctx, target);
     if (res != NULL)
         ldap_msgfree(res);
     if (vals != NULL)
         ldap_value_free_len(vals);
-    ldap_unbind_ext_s(ld, NULL, NULL);
+    if (ld == NULL)
+        ldap_unbind_ext_s(ld, NULL, NULL);
     return code;
 }
index 5f34bb45ff17796af523672fe03c5ca21cb61373..c1d0ad2f3227c77f764cb9e1855db28fa904b4d6 100644 (file)
@@ -36,7 +36,7 @@
  * argument to this function.  Returns 0 on success, non-zero on failure.
  * This function returns failure only if it could not allocate memory.
  */
-int
+krb5_error_code
 pwupdate_init(struct plugin_config **result, krb5_context ctx)
 {
     struct plugin_config *config;
@@ -44,7 +44,7 @@ pwupdate_init(struct plugin_config **result, krb5_context ctx)
     /* Allocate our internal data. */
     config = calloc(1, sizeof(struct plugin_config));
     if (config == NULL)
-        return 1;
+        return sync_error_system(ctx, "cannot allocate memory");
 
     /* Get Active Directory connection information from krb5.conf. */
     sync_config_string(ctx, "ad_keytab", &config->ad_keytab);
@@ -215,13 +215,13 @@ principal_allowed(struct plugin_config *config, krb5_context ctx,
  * If the new password is NULL, that means that the keys are being randomized.
  * Currently, we can't do anything in that case, so just skip it.
  */
-int
+krb5_error_code
 pwupdate_precommit_password(struct plugin_config *config, krb5_context ctx,
                             krb5_principal principal,
-                            const char *password, int pwlen,
-                            char *errstr, int errstrlen)
+                            const char *password, int pwlen)
 {
-    int status;
+    krb5_error_code code;
+    const char *message;
 
     if (config->ad_realm == NULL)
         return 0;
@@ -233,24 +233,19 @@ pwupdate_precommit_password(struct plugin_config *config, krb5_context ctx,
         goto queue;
     if (config->ad_queue_only)
         goto queue;
-    status = pwupdate_ad_change(config, ctx, principal, password, pwlen,
-                                errstr, errstrlen);
-    if (status == 3) {
-        syslog(LOG_INFO, "pwupdate: AD password change failed, queuing: %s",
-               errstr);
+    code = pwupdate_ad_change(config, ctx, principal, password, pwlen);
+    if (code != 0) {
+        message = krb5_get_error_message(ctx, code);
+        syslog(LOG_INFO, "krb5-sync: AD password change failed, queuing: %s",
+               message);
+        krb5_free_error_message(ctx, message);
         goto queue;
     }
-    return status;
+    return 0;
 
 queue:
-    status = pwupdate_queue_write(config, ctx, principal, "ad", "password",
-                                  password);
-    if (status)
-        return 0;
-    else {
-        strlcpy(errstr, "queueing AD password change failed", errstrlen);
-        return 1;
-    }
+    return pwupdate_queue_write(config, ctx, principal, "ad", "password",
+                                password);
 }
 
 
@@ -258,12 +253,11 @@ queue:
  * Actions to take after the password is changed in the local database.
  * Currently, there are none.
  */
-int
+krb5_error_code
 pwupdate_postcommit_password(struct plugin_config *config UNUSED,
                              krb5_context ctx UNUSED,
                              krb5_principal principal UNUSED,
-                             const char *password UNUSED, int pwlen UNUSED,
-                             char *errstr UNUSED, int errstrlen UNUSED)
+                             const char *password UNUSED, int pwlen UNUSED)
 {
     return 0;
 }
@@ -278,12 +272,11 @@ pwupdate_postcommit_password(struct plugin_config *config UNUSED,
  * If a status change is already queued, or if making the status change fails,
  * queue it for later processing.
  */
-int
+krb5_error_code
 pwupdate_postcommit_status(struct plugin_config *config, krb5_context ctx,
-                           krb5_principal principal, int enabled,
-                           char *errstr, int errstrlen)
+                           krb5_principal principal, int enabled)
 {
-    int status;
+    krb5_error_code code;
 
     if (config->ad_admin_server == NULL
         || config->ad_keytab == NULL
@@ -296,19 +289,12 @@ pwupdate_postcommit_status(struct plugin_config *config, krb5_context ctx,
         goto queue;
     if (config->ad_queue_only)
         goto queue;
-    status = pwupdate_ad_status(config, ctx, principal, enabled, errstr,
-                                errstrlen);
-    if (status != 0)
+    code = pwupdate_ad_status(config, ctx, principal, enabled);
+    if (code != 0)
         goto queue;
-    return status;
+    return 0;
 
 queue:
-    status = pwupdate_queue_write(config, ctx, principal, "ad",
-                                  enabled ? "enable" : "disable", NULL);
-    if (status)
-        return 0;
-    else {
-        strlcpy(errstr, "queueing AD status change failed", errstrlen);
-        return 1;
-    }
+    return pwupdate_queue_write(config, ctx, principal, "ad",
+                                enabled ? "enable" : "disable", NULL);
 }
index af49855180a6b88e47f42f61ea93d349c6150c95..ff623fb6fe0921a94382d5da02539f473d4f901b 100644 (file)
 /*
- * Error reporting routines.
+ * Store errors in the Kerberos context.
  *
- * Set the plugin error string based on a provided error message and an
- * optional Kerberos error to append to the end of the string.
+ * Provides helper functions for the rest of the plugin code to store an error
+ * message in the Kerberos context.
  *
- * Written by Russ Allbery <eagle@eyrie.org>
- * Copyright 2006, 2007, 2010, 2012
+ * Written by Russ Allbery <rra@stanford.edu>
+ * Copyright 2013
  *     The Board of Trustees of the Leland Stanford Junior University
  *
  * See LICENSE for licensing terms.
  */
 
 #include <config.h>
+#include <portable/kadmin.h>
 #include <portable/krb5.h>
 #include <portable/system.h>
 
+#include <errno.h>
+#include <ldap.h>
+
 #include <plugin/internal.h>
 
 
 /*
- * Given an error buffer, its length, a Kerberos context, a Kerberos error,
- * and a format string, write the resulting error string into the buffer and
- * append the Kerberos error.
+ * Internal helper function to set the Kerberos error message given a format,
+ * an error code, and a variable argument structure.  Returns the error code
+ * set, which is normally the same as the one passed in, but which may change
+ * if we can't allocate memory.
+ */
+static krb5_error_code
+set_error(krb5_context ctx, krb5_error_code code, const char *format,
+          va_list args)
+{
+    char *message;
+
+    if (vasprintf(&message, format, args) < 0)
+        return sync_error_system(ctx, "cannot allocate memory");
+    krb5_set_error_message(ctx, code, "%s", message);
+    free(message);
+    return code;
+}
+
+
+/*
+ * Set the Kerberos error code to indicate a server configuration error and
+ * set the message to the format and arguments passed to this function.
+ */
+krb5_error_code
+sync_error_config(krb5_context ctx, const char *format, ...)
+{
+    va_list args;
+    krb5_error_code code;
+
+    va_start(args, format);
+    code = set_error(ctx, KADM5_MISSING_KRB5_CONF_PARAMS, format, args);
+    va_end(args);
+    return code;
+}
+
+
+/*
+ * Set the Kerberos error code to a generic kadmin failure error and the
+ * message to the format and arguments passed to this function.  This is used
+ * for internal failures of various types.
+ */
+krb5_error_code
+sync_error_generic(krb5_context ctx, const char *format, ...)
+{
+    va_list args;
+    krb5_error_code code;
+
+    va_start(args, format);
+    code = set_error(ctx, KADM5_FAILURE, format, args);
+    va_end(args);
+    return code;
+}
+
+
+/*
+ * Set the Kerberos error code to a generic service unavailable error and the
+ * message to the format and arguments passed to this function with the LDAP
+ * error string appended.
+ */
+krb5_error_code
+sync_error_ldap(krb5_context ctx, int code, const char *format, ...)
+{
+    va_list args;
+    char *message;
+    bool okay = true;
+    int oerrno;
+
+    va_start(args, format);
+    if (vasprintf(&message, format, args) < 0) {
+        oerrno = errno;
+        krb5_set_error_message(ctx, errno, "cannot allocate memory: %s",
+                               strerror(errno));
+        okay = false;
+    }
+    va_end(args);
+    if (!okay)
+        return oerrno;
+    krb5_set_error_message(ctx, KADM5_FAILURE, "%s: %s", message,
+                           ldap_err2string(code));
+    free(message);
+    return KADM5_FAILURE;
+}
+
+
+/*
+ * Set the Kerberos error code to the current errno and the message to the
+ * format and arguments passed to this function.
  */
-void
-pwupdate_set_error(char *buffer, size_t length, krb5_context ctx,
-                   krb5_error_code code, const char *format, ...)
+krb5_error_code
+sync_error_system(krb5_context ctx, const char *format, ...)
 {
     va_list args;
-    ssize_t used;
-    const char *message;
+    char *message;
+    bool okay = true;
+    int oerrno = errno;
 
     va_start(args, format);
-    used = vsnprintf(buffer, length, format, args);
+    if (vasprintf(&message, format, args) < 0) {
+        oerrno = errno;
+        krb5_set_error_message(ctx, errno, "cannot allocate memory: %s",
+                               strerror(errno));
+        okay = false;
+    }
     va_end(args);
-    if (ctx == NULL || code == 0)
-        return;
-    if (used < 0 || (size_t) used >= length)
-        return;
-    message = krb5_get_error_message(ctx, code);
-    if (message != NULL)
-        snprintf(buffer + used, length - used, ": %s", message);
-    krb5_free_error_message(ctx, message);
+    if (!okay)
+        return oerrno;
+    krb5_set_error_message(ctx, oerrno, "%s: %s", message, strerror(oerrno));
+    free(message);
+    return oerrno;
 }
index 79c89610f54818d537194dec78bd27b3203ee454..5b6eb38d67001a519bdee272846e8095ddf7691d 100644 (file)
@@ -55,11 +55,7 @@ typedef struct kadm5_hook {
 static krb5_error_code
 init(krb5_context ctx, void **data)
 {
-    krb5_error_code code = 0;
-
-    if (pwupdate_init((struct plugin_config **) data, ctx) != 0)
-        code = errno;
-    return code;
+    return pwupdate_init((struct plugin_config **) data, ctx);
 }
 
 
@@ -80,9 +76,8 @@ static krb5_error_code
 chpass(krb5_context ctx, void *data, enum kadm5_hook_stage stage,
        krb5_principal princ, const char *password)
 {
-    char error[BUFSIZ];
     size_t length;
-    int status = 0;
+    krb5_error_code code = 0;
 
     /*
      * If password is NULL, we have a new key set but no password (meaning
@@ -95,18 +90,11 @@ chpass(krb5_context ctx, void *data, enum kadm5_hook_stage stage,
 
     /* Dispatch to the appropriate function. */
     if (stage == KADM5_HOOK_STAGE_PRECOMMIT)
-        status = pwupdate_precommit_password(data, ctx, princ, password,
-                                             length, error, sizeof(error));
+        code = pwupdate_precommit_password(data, ctx, princ, password, length);
     else if (stage == KADM5_HOOK_STAGE_POSTCOMMIT)
-        status = pwupdate_postcommit_password(data, ctx, princ, password,
-                                              length, error, sizeof(error));
-    if (status == 0)
-        return 0;
-    else {
-        krb5_set_error_message(ctx, KADM5_FAILURE,
-                               "cannot synchronize password: %s", error);
-        return KADM5_FAILURE;
-    }
+        code = pwupdate_postcommit_password(data, ctx, princ, password,
+                                            length);
+    return code;
 }
 
 
@@ -136,20 +124,12 @@ static krb5_error_code
 modify(krb5_context ctx, void *data, enum kadm5_hook_stage stage,
        kadm5_principal_ent_t entry, uint32_t mask)
 {
-    char error[BUFSIZ];
-    int enabled, status;
+    int enabled;
 
     if (mask & KADM5_ATTRIBUTES && stage == KADM5_HOOK_STAGE_POSTCOMMIT) {
         enabled = !(entry->attributes & KRB5_KDB_DISALLOW_ALL_TIX);
-        status = pwupdate_postcommit_status(data, ctx, entry->principal,
-                                            enabled, error, sizeof(error));
-        if (status == 0)
-            return 0;
-        else {
-            krb5_set_error_message(ctx, KADM5_FAILURE,
-                                   "cannot synchronize status: %s", error);
-            return KADM5_FAILURE;
-        }
+        return pwupdate_postcommit_status(data, ctx, entry->principal,
+                                          enabled);
     }
     return 0;
 }
index 4dd363c1c2e450be7d65c4dafb37ebd7815f7e6e..c562216e1523001229d219afdb33c8828f88a34f 100644 (file)
@@ -44,26 +44,28 @@ BEGIN_DECLS
 #pragma GCC visibility push(hidden)
 
 /* General public API. */
-int pwupdate_init(struct plugin_config **, krb5_context);
+krb5_error_code pwupdate_init(struct plugin_config **, krb5_context);
 void pwupdate_close(struct plugin_config *);
-int pwupdate_precommit_password(struct plugin_config *, krb5_context,
-                                krb5_principal, const char *password,
-                                int pwlen, char *errstr, int errstrlen);
-int pwupdate_postcommit_password(struct plugin_config *, krb5_context,
-                                 krb5_principal, const char *password,
-                                 int pwlen, char *errstr, int errstrlen);
-int pwupdate_postcommit_status(struct plugin_config *, krb5_context,
-                               krb5_principal, int enabled, char *errstr,
-                               int errstrlen);
+krb5_error_code pwupdate_precommit_password(struct plugin_config *,
+                                            krb5_context, krb5_principal,
+                                            const char *password,
+                                            int pwlen);
+krb5_error_code pwupdate_postcommit_password(struct plugin_config *,
+                                             krb5_context, krb5_principal,
+                                             const char *password,
+                                             int pwlen);
+krb5_error_code pwupdate_postcommit_status(struct plugin_config *,
+                                           krb5_context, krb5_principal,
+                                           int enabled);
 
 /* Password changing. */
-int pwupdate_ad_change(struct plugin_config *, krb5_context, krb5_principal,
-                       const char *password, int pwlen, char *errstr,
-                       int errstrlen);
+krb5_error_code pwupdate_ad_change(struct plugin_config *, krb5_context,
+                                   krb5_principal, const char *password,
+                                   int pwlen);
 
 /* Account status update. */
-int pwupdate_ad_status(struct plugin_config *, krb5_context, krb5_principal,
-                       int enabled, char *errstr, int errstrlen);
+krb5_error_code pwupdate_ad_status(struct plugin_config *, krb5_context,
+                                   krb5_principal, int enabled);
 
 /* Instance lookups. */
 int pwupdate_instance_exists(struct plugin_config *, krb5_context,
@@ -73,13 +75,10 @@ int pwupdate_instance_exists(struct plugin_config *, krb5_context,
 int pwupdate_queue_conflict(struct plugin_config *, krb5_context,
                             krb5_principal, const char *domain,
                             const char *operation);
-int pwupdate_queue_write(struct plugin_config *, krb5_context, krb5_principal,
-                         const char *domain, const char *operation,
-                         const char *password);
-
-/* Error handling. */
-void pwupdate_set_error(char *, size_t, krb5_context, krb5_error_code,
-                        const char *, ...);
+krb5_error_code pwupdate_queue_write(struct plugin_config *, krb5_context,
+                                     krb5_principal, const char *domain,
+                                     const char *operation,
+                                     const char *password);
 
 /*
  * Obtain configuration settings from krb5.conf.  These are wrappers around
@@ -92,6 +91,20 @@ void sync_config_boolean(krb5_context, const char *, bool *)
 void sync_config_string(krb5_context, const char *, char **)
     __attribute__((__nonnull__));
 
+/*
+ * Store a configuration, generic, or system error in the Kerberos context,
+ * appending the strerror results to the message in the _system case and the
+ * LDAP error string in the _ldap case.  Returns the error code set.
+ */
+krb5_error_code sync_error_config(krb5_context, const char *format, ...)
+    __attribute__((__nonnull__, __format__(printf, 2, 3)));
+krb5_error_code sync_error_generic(krb5_context, const char *format, ...)
+    __attribute__((__nonnull__, __format__(printf, 2, 3)));
+krb5_error_code sync_error_ldap(krb5_context, int, const char *format, ...)
+    __attribute__((__nonnull__, __format__(printf, 3, 4)));
+krb5_error_code sync_error_system(krb5_context, const char *format, ...)
+    __attribute__((__nonnull__, __format__(printf, 2, 3)));
+
 /* Undo default visibility change. */
 #pragma GCC visibility pop
 
index 250163c303556279e79fb04cfd0428ae92675b70..4cae5a7c8fa708cc70cc6825f610ba6183c17cd2 100644 (file)
@@ -49,11 +49,7 @@ krb5_error_code kadm5_hook_krb5_sync_initvt(krb5_context, int, int,
 static kadm5_ret_t
 init(krb5_context ctx, kadm5_hook_modinfo **data)
 {
-    krb5_error_code code = 0;
-
-    if (pwupdate_init((struct plugin_config **) data, ctx) != 0)
-        code = errno;
-    return code;
+    return pwupdate_init((struct plugin_config **) data, ctx);
 }
 
 
@@ -76,9 +72,8 @@ chpass(krb5_context ctx, kadm5_hook_modinfo *data, int stage,
        int n_ks_tuple UNUSED, krb5_key_salt_tuple *ks_tuple UNUSED,
        const char *password)
 {
-    char error[BUFSIZ];
     size_t length;
-    int status = 0;
+    krb5_error_code code = 0;
 
     /*
      * If password is NULL, we have a new key set but no password (meaning
@@ -91,20 +86,12 @@ chpass(krb5_context ctx, kadm5_hook_modinfo *data, int stage,
     /* Dispatch to the appropriate function. */
     length = strlen(password);
     if (stage == KADM5_HOOK_STAGE_PRECOMMIT)
-        status = pwupdate_precommit_password((struct plugin_config *) data,
-                                             ctx, princ, password,
-                                             length, error, sizeof(error));
+        code = pwupdate_precommit_password((struct plugin_config *) data,
+                                           ctx, princ, password, length);
     else if (stage == KADM5_HOOK_STAGE_POSTCOMMIT)
-        status = pwupdate_postcommit_password((struct plugin_config *) data,
-                                              ctx, princ, password,
-                                              length, error, sizeof(error));
-    if (status == 0)
-        return 0;
-    else {
-        krb5_set_error_message(ctx, KADM5_FAILURE,
-                               "cannot synchronize password: %s", error);
-        return KADM5_FAILURE;
-    }
+        code = pwupdate_postcommit_password((struct plugin_config *) data,
+                                            ctx, princ, password, length);
+    return code;
 }
 
 
@@ -135,21 +122,12 @@ static kadm5_ret_t
 modify(krb5_context ctx, kadm5_hook_modinfo *data, int stage,
        kadm5_principal_ent_t entry, long mask)
 {
-    char error[BUFSIZ];
-    int enabled, status;
+    int enabled;
 
     if (mask & KADM5_ATTRIBUTES && stage == KADM5_HOOK_STAGE_POSTCOMMIT) {
         enabled = !(entry->attributes & KRB5_KDB_DISALLOW_ALL_TIX);
-        status = pwupdate_postcommit_status((struct plugin_config *) data,
-                                            ctx, entry->principal,
-                                            enabled, error, sizeof(error));
-        if (status == 0)
-            return 0;
-        else {
-            krb5_set_error_message(ctx, KADM5_FAILURE,
-                                   "cannot synchronize status: %s", error);
-            return KADM5_FAILURE;
-        }
+        return pwupdate_postcommit_status((struct plugin_config *) data,
+                                          ctx, entry->principal, enabled);
     }
     return 0;
 }
index 8f42d607b403c488741af471207695e614473f82..50d9661484a9208d52797a539b54805a92c98ea2 100644 (file)
@@ -9,7 +9,7 @@
  * undone.
  *
  * Written by Russ Allbery <eagle@eyrie.org>
- * Copyright 2006, 2007, 2010
+ * Copyright 2006, 2007, 2010, 2013
  *     The Board of Trustees of the Leland Stanford Junior University
  *
  * See LICENSE for licensing terms.
 #define MAX_QUEUE_STR "99"
 
 /* Write out a string, checking that all of it was written. */
-#define WRITE_CHECK(fd, s)                              \
-    do {                                                \
-        ssize_t result;                                 \
-        result = write((fd), (s), strlen(s));           \
-        if (result < 0 || (size_t) result != strlen(s)) \
-            goto fail;                                  \
+#define WRITE_CHECK(fd, s)                                              \
+    do {                                                                \
+        ssize_t result;                                                 \
+        result = write((fd), (s), strlen(s));                           \
+        if (result < 0 || (size_t) result != strlen(s)) {               \
+            code = sync_error_system(ctx, "cannot write queue file");   \
+            goto fail;                                                  \
+        }                                                               \
     } while (0)
 
 
@@ -100,13 +102,13 @@ queue_prefix(krb5_context ctx, krb5_principal principal, const char *domain,
 {
     char *user = NULL, *prefix = NULL;
     char *p;
-    krb5_error_code retval;
+    krb5_error_code code;
 
     /* Enable and disable should go into the same queue. */
     if (strcmp(operation, "disable") == 0)
         operation = "enable";
-    retval = krb5_unparse_name(ctx, principal, &user);
-    if (retval != 0)
+    code = krb5_unparse_name(ctx, principal, &user);
+    if (code != 0)
         return NULL;
     p = strchr(user, '@');
     if (p != NULL)
@@ -204,9 +206,9 @@ fail:
 /*
  * Queue an action.  Takes the plugin configuration, the Kerberos context, the
  * principal, the domain, the operation, and a password (which may be NULL for
- * enable and disable).  Returns true on success, false on failure.
+ * enable and disable).  Returns a Kerberos error code.
  */
-int
+krb5_error_code
 pwupdate_queue_write(struct plugin_config *config, krb5_context ctx,
                      krb5_principal principal, const char *domain,
                      const char *operation, const char *password)
@@ -214,26 +216,30 @@ pwupdate_queue_write(struct plugin_config *config, krb5_context ctx,
     char *prefix = NULL, *timestamp = NULL, *path = NULL, *user = NULL;
     char *p;
     unsigned int i;
-    int status;
+    krb5_error_code code;
     int lock = -1, fd = -1;
-    krb5_error_code retval;
 
     if (config->queue_dir == NULL)
-        return 0;
+        return sync_error_config(ctx, "configuration setting queue_dir"
+                                 " missing");
     prefix = queue_prefix(ctx, principal, domain, operation);
     if (prefix == NULL)
-        return 0;
+        return sync_error_system(ctx, "cannot generate queue prefix");
 
     /*
      * Lock the queue before the timestamp so that another writer coming up
      * at the same time can't get an earlier timestamp.
      */
     lock = lock_queue(config);
-    if (lock < 0)
+    if (lock < 0) {
+        code = sync_error_system(ctx, "cannot lock queue");
         goto fail;
+    }
     timestamp = queue_timestamp();
-    if (timestamp == NULL)
+    if (timestamp == NULL) {
+        code = sync_error_system(ctx, "cannot generate timestamp");
         goto fail;
+    }
 
     /* Find a unique filename for the queue file. */
     for (i = 0; i < MAX_QUEUE; i++) {
@@ -241,10 +247,12 @@ pwupdate_queue_write(struct plugin_config *config, krb5_context ctx,
             free(path);
             path = NULL;
         }
-        status = asprintf(&path, "%s/%s%s-%02d", config->queue_dir, prefix,
-                          timestamp, i);
-        if (status < 0)
+        code = asprintf(&path, "%s/%s%s-%02d", config->queue_dir, prefix,
+                        timestamp, i);
+        if (code < 0) {
+            code = sync_error_system(ctx, "cannot create queue file name");
             goto fail;
+        }
         fd = open(path, O_WRONLY | O_CREAT | O_EXCL, 0600);
         if (fd >= 0)
             break;
@@ -254,8 +262,8 @@ pwupdate_queue_write(struct plugin_config *config, krb5_context ctx,
      * Get the username from the principal and chop off the realm, dealing
      * properly with escaped @ characters.
      */
-    retval = krb5_unparse_name(ctx, principal, &user);
-    if (retval != 0)
+    code = krb5_unparse_name(ctx, principal, &user);
+    if (code != 0)
         goto fail;
     for (p = user; *p != '\0'; p++) {
         if (p[0] == '\\' && p[1] != '\0') {
@@ -285,7 +293,7 @@ pwupdate_queue_write(struct plugin_config *config, krb5_context ctx,
     free(prefix);
     free(timestamp);
     free(path);
-    return 1;
+    return 0;
 
 fail:
     if (fd >= 0) {
@@ -303,5 +311,5 @@ fail:
         free(timestamp);
     if (path != NULL)
         free(path);
-    return 0;
+    return code;
 }
index d4c591c8ed5036b144c16793bbbcd32bafc0fbc5..f4104814f2d96193e23e9796c8a04cd24ddc041f 100644 (file)
 # define KADM5_PASS_Q_GENERIC KADM5_PASS_Q_DICT
 #endif
 
+/* Heimdal doesn't define KADM5_MISSING_KRB5_CONF_PARAMS. */
+#ifndef KADM5_MISSING_KRB5_CONF_PARAMS
+# define KADM5_MISSING_KRB5_CONF_PARAMS KADM5_MISSING_CONF_PARAMS
+#endif
+
 /*
  * Heimdal provides _ctx functions that take an existing context.  MIT always
  * requires the context be passed in.  Code should use the _ctx variant, and
index ef40914e4d35e0f4643b0b30c1bf5b650d9626e1..33c4fa2b14ad058e8d789c748b695ad0fb9878c2 100644 (file)
@@ -2,7 +2,7 @@
  * Tests for the Heimdal module API.
  *
  * Written by Russ Allbery <eagle@eyrie.org>
- * Copyright 2012
+ * Copyright 2012, 2013
  *     The Board of Trustees of the Leland Stanford Junior University
  *
  * See LICENSE for licensing terms.
@@ -62,6 +62,7 @@ main(void)
     void *data = NULL;
     struct kadm5_hook *hook = NULL;
     kadm5_principal_ent_rec entity;
+    const char *message;
 
     krb5conf = test_file_path("data/krb5.conf");
     if (krb5conf == NULL)
@@ -123,10 +124,12 @@ main(void)
         ok(data != NULL, "...and data is not NULL");
         code = hook->chpass(ctx, data, KADM5_HOOK_STAGE_PRECOMMIT, princ,
                             "test");
-        is_int(KADM5_FAILURE, code, "chpass");
-        is_string("cannot synchronize password: queueing AD password change"
-                  " failed", krb5_get_error_message(ctx, code),
-                  "...with correct error message");
+        is_int(ENOENT, code, "chpass");
+        message = krb5_get_error_message(ctx, code);
+        is_int(strncmp("cannot lock queue", message,
+                       strlen("cannot lock queue")),
+               0, "...with correct error message");
+        krb5_free_error_message(ctx, message);
 
         /* Test chpass with a NULL password. */
         code = hook->chpass(ctx, data, KADM5_HOOK_STAGE_PRECOMMIT, princ,
@@ -142,21 +145,25 @@ main(void)
         entity.attributes = KRB5_KDB_DISALLOW_ALL_TIX;
         code = hook->create(ctx, data, KADM5_HOOK_STAGE_PRECOMMIT, &entity,
                             0, "test");
-        is_int(KADM5_FAILURE, code, "create");
-        is_string("cannot synchronize password: queueing AD password change"
-                  " failed", krb5_get_error_message(ctx, code),
-                  "...with correct error message");
+        is_int(ENOENT, code, "create");
+        message = krb5_get_error_message(ctx, code);
+        is_int(strncmp("cannot lock queue", message,
+                       strlen("cannot lock queue")),
+               0, "...with correct error message");
+        krb5_free_error_message(ctx, message);
         code = hook->modify(ctx, data, KADM5_HOOK_STAGE_POSTCOMMIT, &entity,
                             KADM5_ATTRIBUTES);
-        is_int(KADM5_FAILURE, code, "modify");
-        is_string("cannot synchronize status: queueing AD status change"
-                  " failed", krb5_get_error_message(ctx, code),
-                  "...with correct error message");
+        is_int(ENOENT, code, "modify");
+        message = krb5_get_error_message(ctx, code);
+        is_int(strncmp("cannot lock queue", message,
+                       strlen("cannot lock queue")),
+               0, "...with correct error message");
+        krb5_free_error_message(ctx, message);
 
         /* Test create with a NULL password. */
         code = hook->create(ctx, data, KADM5_HOOK_STAGE_PRECOMMIT, &entity,
                             0, NULL);
-        is_int(0, code, "create");
+        is_int(0, code, "create with NULL password");
 
         /* Close down the module. */
         hook->fini(ctx, data);
index 19c3887a523db997cb663646b1051dca8572cda2..09631a078116aed43a531e7a9c468dca004c28af 100644 (file)
@@ -2,7 +2,7 @@
  * Tests for the MIT Kerberos module API.
  *
  * Written by Russ Allbery <eagle@eyrie.org>
- * Copyright 2012
+ * Copyright 2012, 2013
  *     The Board of Trustees of the Leland Stanford Junior University
  *
  * See LICENSE for licensing terms.
@@ -48,6 +48,7 @@ main(void)
     kadm5_hook_vftable_1 hook;
     kadm5_hook_modinfo *data = NULL;
     kadm5_principal_ent_rec entity;
+    const char *message;
 
     krb5conf = test_file_path("data/krb5.conf");
     if (krb5conf == NULL)
@@ -115,10 +116,12 @@ main(void)
         ok(data != NULL, "...and data is not NULL");
         code = hook.chpass(ctx, data, KADM5_HOOK_STAGE_PRECOMMIT, princ,
                            false, 0, NULL, "test");
-        is_int(KADM5_FAILURE, code, "chpass");
-        is_string("cannot synchronize password: queueing AD password change"
-                  " failed", krb5_get_error_message(ctx, code),
-                  "...with correct error message");
+        is_int(ENOENT, code, "chpass");
+        message = krb5_get_error_message(ctx, code);
+        is_int(strncmp("cannot lock queue", message,
+                       strlen("cannot lock queue")),
+               0, "...with correct error message");
+        krb5_free_error_message(ctx, message);
 
         /* Test chpass with a NULL password. */
         code = hook.chpass(ctx, data, KADM5_HOOK_STAGE_PRECOMMIT, princ,
@@ -134,21 +137,25 @@ main(void)
         entity.attributes = KRB5_KDB_DISALLOW_ALL_TIX;
         code = hook.create(ctx, data, KADM5_HOOK_STAGE_PRECOMMIT, &entity,
                            0, 0, NULL, "test");
-        is_int(KADM5_FAILURE, code, "create");
-        is_string("cannot synchronize password: queueing AD password change"
-                  " failed", krb5_get_error_message(ctx, code),
-                  "...with correct error message");
+        is_int(ENOENT, code, "create");
+        message = krb5_get_error_message(ctx, code);
+        is_int(strncmp("cannot lock queue", message,
+                       strlen("cannot lock queue")),
+               0, "...with correct error message");
+        krb5_free_error_message(ctx, message);
         code = hook.modify(ctx, data, KADM5_HOOK_STAGE_POSTCOMMIT, &entity,
                            KADM5_ATTRIBUTES);
-        is_int(KADM5_FAILURE, code, "modify");
-        is_string("cannot synchronize status: queueing AD status change"
-                  " failed", krb5_get_error_message(ctx, code),
-                  "...with correct error message");
+        is_int(ENOENT, code, "modify");
+        message = krb5_get_error_message(ctx, code);
+        is_int(strncmp("cannot lock queue", message,
+                       strlen("cannot lock queue")),
+               0, "...with correct error message");
+        krb5_free_error_message(ctx, message);
 
         /* Test create with a NULL password. */
         code = hook.create(ctx, data, KADM5_HOOK_STAGE_PRECOMMIT, &entity, 0,
                            0, NULL, NULL);
-        is_int(0, code, "create");
+        is_int(0, code, "create with NUL password");
 
         /* Close down the module. */
         hook.fini(ctx, data);
index 6a9074773c2821e0b0f7dcab94af72e91788ce9a..89c3e314da665d4db699596c905b2e12a8b8b1b8 100644 (file)
@@ -15,6 +15,7 @@
 #include <portable/krb5.h>
 #include <portable/system.h>
 
+#include <errno.h>
 #include <fcntl.h>
 #include <sys/stat.h>
 #include <time.h>
@@ -32,7 +33,7 @@ main(void)
     krb5_principal princ;
     krb5_error_code code;
     struct plugin_config *config;
-    char errstr[BUFSIZ], buffer[BUFSIZ];
+    char buffer[BUFSIZ];
     time_t now, try;
     struct tm *date;
     FILE *file;
@@ -56,19 +57,16 @@ main(void)
     if (code != 0)
         bail("cannot parse principal: %s", krb5_get_error_message(ctx, code));
 
-    plan(27);
+    plan(24);
 
     /* Test init. */
     is_int(0, pwupdate_init(&config, ctx), "pwupdate_init succeeds");
     ok(config != NULL, "...and config is non-NULL");
 
     /* Create a password change and be sure it's queued. */
-    errstr[0] = '\0';
     code = pwupdate_precommit_password(config, ctx, princ, "foobar",
-                                       strlen("foobar"), errstr,
-                                       sizeof(errstr));
+                                       strlen("foobar"));
     is_int(0, code, "pwupdate_precommit_password succeeds");
-    is_string("", errstr, "...and there is no error string");
     queue = NULL;
     now = time(NULL);
     for (try = now - 1; try <= now; try++) {
@@ -110,11 +108,8 @@ main(void)
     free(queue);
 
     /* Test queuing of enable. */
-    errstr[0] = '\0';
-    code = pwupdate_postcommit_status(config, ctx, princ, 1, errstr,
-                                      sizeof(errstr));
+    code = pwupdate_postcommit_status(config, ctx, princ, 1);
     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++) {
@@ -148,11 +143,8 @@ main(void)
     ok(unlink(queue) == 0, "Remove queued enable");
 
     /* Test queuing of disable. */
-    errstr[0] = '\0';
-    code = pwupdate_postcommit_status(config, ctx, princ, 0, errstr,
-                                      sizeof(errstr));
+    code = pwupdate_postcommit_status(config, ctx, princ, 0);
     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++) {
index 65e741386c99135fcc77d28040e941a609e8d0bc..e3ace201ad903f638a78dd05b764d912d25bba6d 100644 (file)
  */
 
 #include <config.h>
+#include <portable/kadmin.h>
 #include <portable/krb5.h>
 #include <portable/system.h>
 
+#include <errno.h>
 #include <fcntl.h>
 #include <sys/stat.h>
 #include <time.h>
@@ -35,11 +37,12 @@ main(void)
     krb5_error_code code;
     struct plugin_config *data;
     int fd;
-    char errstr[BUFSIZ], buffer[BUFSIZ];
+    char buffer[BUFSIZ];
     time_t now, try;
     struct tm *date;
     FILE *file;
     struct stat st;
+    const char *message;
 
     tmpdir = test_tmpdir();
     if (chdir(tmpdir) < 0)
@@ -59,7 +62,7 @@ main(void)
     if (code != 0)
         bail("cannot parse principal: %s", krb5_get_error_message(ctx, code));
 
-    plan(42);
+    plan(36);
 
     /* Test init. */
     is_int(0, pwupdate_init(&data, ctx), "pwupdate_init succeeds");
@@ -71,13 +74,10 @@ main(void)
     if (fd < 0)
         sysbail("cannot create fake queue file");
     close(fd);
-    errstr[0] = '\0';
     code = pwupdate_precommit_password(data, ctx, princ, "foobar",
-                                       strlen("foobar"), errstr,
-                                       sizeof(errstr));
+                                       strlen("foobar"));
     is_int(0, code, "pwupdate_precommit_password succeeds");
     ok(access("queue/.lock", F_OK) == 0, "...lock file now exists");
-    is_string("", errstr, "...and there is no error");
     queue = NULL;
     now = time(NULL);
     for (try = now - 1; try <= now; try++) {
@@ -117,12 +117,9 @@ main(void)
     }
 
     /* pwupdate_postcommit_password should do nothing, silently. */
-    errstr[0] = '\0';
     code = pwupdate_postcommit_password(data, ctx, princ, "foobar",
-                                        strlen("foobar"), errstr,
-                                        sizeof(errstr));
+                                        strlen("foobar"));
     is_int(0, code, "pwupdate_postcommit_password succeeds");
-    is_string("", errstr, "...and there is no error");
 
     /* Clean up password change queue files. */
     ok(unlink("queue/test-ad-password-19700101T000000Z") == 0,
@@ -136,11 +133,8 @@ main(void)
     if (fd < 0)
         sysbail("cannot create fake queue file");
     close(fd);
-    errstr[0] = '\0';
-    code = pwupdate_postcommit_status(data, ctx, princ, 1, errstr,
-                                      sizeof(errstr));
+    code = pwupdate_postcommit_status(data, ctx, princ, 1);
     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++) {
@@ -178,11 +172,8 @@ main(void)
      * Do the same thing for disables, which should still be blocked by the
      * same marker.
      */
-    errstr[0] = '\0';
-    code = pwupdate_postcommit_status(data, ctx, princ, 0, errstr,
-                                      sizeof(errstr));
+    code = pwupdate_postcommit_status(data, ctx, princ, 0);
     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++) {
@@ -223,18 +214,21 @@ main(void)
     ok(rmdir("queue") == 0, "No other files in queue directory");
 
     /* Check failure when there's no queue directory. */
-    errstr[0] = '\0';
     code = pwupdate_precommit_password(data, ctx, princ, "foobar",
-                                       strlen("foobar"), errstr,
-                                       sizeof(errstr));
-    is_int(1, code, "pwupdate_precommit_password fails with no queue");
-    is_string("queueing AD password change failed", errstr,
-              "...with correct error");
-    code = pwupdate_postcommit_status(data, ctx, princ, 0, errstr,
-                                      sizeof(errstr));
-    is_int(1, code, "pwupdate_postcommit_status disable fails with no queue");
-    is_string("queueing AD status change failed", errstr,
-              "...with correct error");
+                                       strlen("foobar"));
+    is_int(ENOENT, code,
+           "pwupdate_precommit_password fails with no queue");
+    message = krb5_get_error_message(ctx, code);
+    is_int(strncmp("cannot lock queue", message, strlen("cannot lock queue")),
+           0, "...with correct error message");
+    krb5_free_error_message(ctx, message);
+    code = pwupdate_postcommit_status(data, ctx, princ, 0);
+    is_int(ENOENT, code,
+           "pwupdate_postcommit_status disable fails with no queue");
+    message = krb5_get_error_message(ctx, code);
+    is_int(strncmp("cannot lock queue", message, strlen("cannot lock queue")),
+           0, "...with correct error message");
+    krb5_free_error_message(ctx, message);
 
     /* Shut down the plugin. */
     pwupdate_close(data);
@@ -262,17 +256,11 @@ main(void)
         bail("cannot parse principal: %s", krb5_get_error_message(ctx, code));
     is_int(0, pwupdate_init(&data, ctx), "pwupdate_init succeeds");
     ok(data != NULL, "...and data is non-NULL");
-    errstr[0] = '\0';
     code = pwupdate_precommit_password(data, ctx, princ, "foobar",
-                                       strlen("foobar"), errstr,
-                                       sizeof(errstr));
+                                       strlen("foobar"));
     is_int(0, code, "pwupdate_precommit_password succeeds");
-    is_string("", errstr, "...and there is no error");
-    errstr[0] = '\0';
-    code = pwupdate_postcommit_status(data, ctx, princ, 0, errstr,
-                                      sizeof(errstr));
+    code = pwupdate_postcommit_status(data, ctx, princ, 0);
     is_int(0, code, "pwupdate_postcommit_status disable succeeds");
-    is_string("", errstr, "...and there is no error");
 
     /* Clean up. */
     krb5_free_principal(ctx, princ);
index fd1a9b5722da3e82ff0adf6477f094208f2e407f..d127fd2c4b41e0f17450d47bbc8806a98fb59caf 100644 (file)
@@ -36,13 +36,12 @@ static void
 ad_password(struct plugin_config *data, krb5_context ctx,
             krb5_principal principal, char *password, const char *user)
 {
-    char errbuf[BUFSIZ];
-    int status;
+    krb5_error_code code;
 
-    status = pwupdate_ad_change(data, ctx, principal, password,
-                                strlen(password), errbuf, sizeof(errbuf));
-    if (status != 0)
-        die("AD password change for %s failed (%d): %s", user, status, errbuf);
+    code = pwupdate_ad_change(data, ctx, principal, password,
+                              strlen(password));
+    if (code != 0)
+        die_krb5(ctx, code, "AD password change for %s failed", user);
     notice("AD password change for %s succeeded", user);
 }
 
@@ -55,13 +54,11 @@ static void
 ad_status(struct plugin_config *data, krb5_context ctx,
           krb5_principal principal, bool enable, const char *user)
 {
-    char errbuf[BUFSIZ];
-    int status;
+    krb5_error_code code;
 
-    status = pwupdate_ad_status(data, ctx, principal, enable, errbuf,
-                                sizeof(errbuf));
-    if (status != 0)
-        die("AD status change for %s failed (%d): %s", user, status, errbuf);
+    code = pwupdate_ad_status(data, ctx, principal, enable);
+    if (code != 0)
+        die_krb5(ctx, code, "AD status change for %s failed", user);
     notice("AD status change for %s succeeded", user);
 }
 
@@ -108,6 +105,7 @@ process_queue_file(struct plugin_config *data, krb5_context ctx,
     bool disable = false;
     bool password = false;
 
+    /* Open the queue file. */
     queue = fopen(filename, "r");
     if (queue == NULL)
         sysdie("cannot open queue file %s", filename);
@@ -163,7 +161,7 @@ main(int argc, char *argv[])
     char *user;
     struct plugin_config *config;
     krb5_context ctx;
-    krb5_error_code ret;
+    krb5_error_code code;
     krb5_principal principal;
 
     /*
@@ -173,6 +171,7 @@ main(int argc, char *argv[])
     openlog("krb5-sync", LOG_PID, LOG_AUTH);
     message_program_name = "krb5-sync";
 
+    /* Parse command-line options. */
     while ((option = getopt(argc, argv, "def:p:")) != EOF) {
         switch (option) {
         case 'd': disable = true;       break;
@@ -204,26 +203,26 @@ main(int argc, char *argv[])
         die("must specify queue file or action, not both");
 
     /* Create a Kerberos context for plugin initialization. */
-    ret = krb5_init_context(&ctx);
-    if (ret != 0)
-        die_krb5(ctx, ret, "cannot initialize Kerberos context");
+    code = krb5_init_context(&ctx);
+    if (code != 0)
+        die_krb5(ctx, code, "cannot initialize Kerberos context");
 
     /* Initialize the plugin. */
-    if (pwupdate_init(&config, ctx))
-        die("plugin initialization failed");
+    code = pwupdate_init(&config, ctx);
+    if (code != 0)
+        die_krb5(ctx, code, "plugin initialization failed");
 
     /* Now, do whatever we were supposed to do. */
     if (filename != NULL)
         process_queue_file(config, ctx, filename);
     else {
-        ret = krb5_parse_name(ctx, user, &principal);
-        if (ret != 0)
-            die_krb5(ctx, ret, "cannot parse user %s into principal", user);
+        code = krb5_parse_name(ctx, user, &principal);
+        if (code != 0)
+            die_krb5(ctx, code, "cannot parse user %s into principal", user);
         if (password != NULL)
             ad_password(config, ctx, principal, password, user);
         if (enable || disable)
             ad_status(config, ctx, principal, enable, user);
     }
-
     exit(0);
 }