]> eyrie.org Git - kerberos/krb5-strength.git/commitdiff
Use the Kerberos context to set the error message
authorRuss Allbery <rra@stanford.edu>
Fri, 27 Sep 2013 05:21:35 +0000 (22:21 -0700)
committerRuss Allbery <rra@stanford.edu>
Fri, 27 Sep 2013 05:21:35 +0000 (22:21 -0700)
The plugin now sets the Kerberos error message in the context to pass
error information, resulting in higher-quality error reporting in the
MIT Kerberos plugin.

NEWS
external/heimdal-strength.c
plugin/api.c
plugin/api.h
plugin/heimdal.c
plugin/mit.c
tests/data/cracklib.json
tests/heimdal/external-t
tests/heimdal/plugin-t.c

diff --git a/NEWS b/NEWS
index a8fc32ad67042c3f36b7d215d48959365bac161b..d6da64862dd1443859011943c0df5f15f4dd1583 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,10 @@ krb5-strength 2.0 (unreleased)
     and MIT.  Drop the patch for MIT Kerberos 1.4 (and hence support for
     versions of MIT Kerberos prior to 1.9).
 
+    The plugin now sets the Kerberos error message in the context to pass
+    error information, resulting in higher-quality error reporting in the
+    MIT Kerberos plugin.
+
     CrackLib checks for passwords where a character is a simple increment
     or decrement of the previous character.  In previous versions, the
     embedded version of CrackLib allowed at most four such occurrences in
index c74ffa8ac10f8943b776d2fb610298e0f6d8b376..9f7be35b59e2fa445c1df7c386c8c1bdb57e3c79 100644 (file)
@@ -60,7 +60,9 @@ read_key(const char *key, char *buffer, size_t length)
 static void
 check_password(krb5_context ctx, krb5_pwqual_moddata data)
 {
-    char principal[BUFSIZ], password[BUFSIZ], error[BUFSIZ], end[BUFSIZ];
+    char principal[BUFSIZ], password[BUFSIZ], end[BUFSIZ];
+    krb5_error_code code;
+    const char *message;
 
     read_key("principal", principal, sizeof(principal));
     read_key("new-password", password, sizeof(password));
@@ -68,10 +70,14 @@ check_password(krb5_context ctx, krb5_pwqual_moddata data)
         sysdie("Cannot read end of entry");
     if (strcmp(end, "end\n") != 0)
         die("Malformed end line");
-    if (pwcheck_check(ctx, data, password, principal, error, sizeof(error)))
-        fprintf(stderr, "%s\n", error);
-    else
+    code = pwcheck_check(ctx, data, password, principal);
+    if (code == 0)
         printf("APPROVED\n");
+    else {
+        message = krb5_get_error_message(ctx, code);
+        fprintf(stderr, "%s\n", message);
+        krb5_free_error_message(ctx, message);
+    }
 }
 
 
@@ -96,9 +102,10 @@ main(int argc, char *argv[] UNUSED)
     /* Initialize Kerberos and the module. */
     code = krb5_init_context(&ctx);
     if (code != 0)
-        die_krb5(ctx, code, "cannot create Kerberos context");
-    if (pwcheck_init(ctx, NULL, &data) != 0)
-        die("cannot initialize strength checking");
+        die_krb5(ctx, code, "Cannot create Kerberos context");
+    code = pwcheck_init(ctx, NULL, &data);
+    if (code != 0)
+        die_krb5(ctx, code, "Cannot initialize strength checking");
 
     /* Check the password and report results. */
     check_password(ctx, data);
index 1a343ddfb7a133e534a82269ea6198066e82af2f..0f31a6dc5f92f80f72442f41e2eaa340eca03418 100644 (file)
@@ -117,22 +117,31 @@ pwcheck_init(krb5_context ctx, const char *dictionary,
         default_string(ctx, "krb5-strength", "password_dictionary", &path);
     else {
         path = strdup(dictionary);
-        if (path == NULL)
-            return errno;
+        if (path == NULL) {
+            oerrno = errno;
+            krb5_set_error_message(ctx, oerrno, "cannot allocate memory");
+            return oerrno;
+        }
     }
 
     /* If there is no dictionary, abort our setup with an error. */
-    if (path == NULL)
+    if (path == NULL) {
+        krb5_set_error_message(ctx, KADM5_MISSING_CONF_PARAMS,
+            "password_dictionary not configured in krb5.conf");
         return KADM5_MISSING_CONF_PARAMS;
+    }
 
     /* Sanity-check the dictionary path. */
     if (asprintf(&file, "%s.pwd", path) < 0) {
         oerrno = errno;
+        krb5_set_error_message(ctx, oerrno, "cannot allocate memory");
         free(path);
         return oerrno;
     }
     if (access(file, R_OK) != 0) {
         oerrno = errno;
+        krb5_set_error_message(ctx, oerrno, "dictionary %s does not exist",
+                               file);
         free(path);
         free(file);
         return oerrno;
@@ -143,6 +152,7 @@ pwcheck_init(krb5_context ctx, const char *dictionary,
     *data = malloc(sizeof(**data));
     if (*data == NULL) {
         oerrno = errno;
+        krb5_set_error_message(ctx, oerrno, "cannot allocate memory");
         free(path);
         return oerrno;
     }
@@ -158,14 +168,13 @@ pwcheck_init(krb5_context ctx, const char *dictionary,
  */
 krb5_error_code
 pwcheck_check(krb5_context ctx UNUSED, krb5_pwqual_moddata data,
-              const char *password, const char *principal, char *errstr,
-              int errstrlen)
+              const char *password, const char *principal)
 {
     char *user, *p;
     const char *q;
     size_t i, j;
     char c;
-    int err;
+    int oerrno;
     const char *result;
 
     /*
@@ -175,14 +184,15 @@ pwcheck_check(krb5_context ctx UNUSED, krb5_pwqual_moddata data,
      * have to copy the string so that we can manipulate it a bit.
      */
     if (strcasecmp(password, principal) == 0) {
-        snprintf(errstr, errstrlen, "Password based on username");
+        krb5_set_error_message(ctx, KADM5_PASS_Q_GENERIC,
+                               "password based on username");
         return KADM5_PASS_Q_GENERIC;
     }
     user = strdup(principal);
     if (user == NULL) {
-        err = errno;
-        snprintf(errstr, errstrlen, "Cannot allocate memory");
-        return err;
+        oerrno = errno;
+        krb5_set_error_message(ctx, oerrno, "cannot allocate memory");
+        return oerrno;
     }
     for (p = user; p[0] != '\0'; p++) {
         if (p[0] == '\\' && p[1] != '\0') {
@@ -197,7 +207,8 @@ pwcheck_check(krb5_context ctx UNUSED, krb5_pwqual_moddata data,
     if (strlen(password) == strlen(user)) {
         if (strcasecmp(password, user) == 0) {
             free(user);
-            snprintf(errstr, errstrlen, "Password based on username");
+            krb5_set_error_message(ctx, KADM5_PASS_Q_GENERIC,
+                                   "password based on username");
             return KADM5_PASS_Q_GENERIC;
         }
 
@@ -209,7 +220,8 @@ pwcheck_check(krb5_context ctx UNUSED, krb5_pwqual_moddata data,
         }
         if (strcasecmp(password, user) == 0) {
             free(user);
-            snprintf(errstr, errstrlen, "Password based on username");
+            krb5_set_error_message(ctx, KADM5_PASS_Q_GENERIC,
+                                   "password based on username");
             return KADM5_PASS_Q_GENERIC;
         }
     }
@@ -220,14 +232,15 @@ pwcheck_check(krb5_context ctx UNUSED, krb5_pwqual_moddata data,
                 q++;
             if (*q == '\0') {
                 free(user);
-                snprintf(errstr, errstrlen, "Password based on username");
+                krb5_set_error_message(ctx, KADM5_PASS_Q_GENERIC,
+                                       "password based on username");
                 return KADM5_PASS_Q_GENERIC;
             }
         }
     free(user);
     result = FascistCheck(password, data->dictionary);
     if (result != NULL) {
-        strlcpy(errstr, result, errstrlen);
+        krb5_set_error_message(ctx, KADM5_PASS_Q_GENERIC, "%s", result);
         return KADM5_PASS_Q_GENERIC;
     }
     return 0;
index 2ea7eb3fcab84cbe72d60aced0c92e4cf1327bbe..0a7668e67655ebb6ca88f2cd5ae10ccbba6065b4 100644 (file)
@@ -36,12 +36,11 @@ krb5_error_code pwcheck_init(krb5_context, const char *dictionary,
                              krb5_pwqual_moddata *);
 
 /*
- * Check a password.  Returns 0 if okay, non-zero on rejection, and stores
- * the rejection method in the provided errstr buffer.
+ * Check a password.  Returns 0 if okay.  On error, sets the Kerberos error
+ * message and returns a Kerberos status code.
  */
 krb5_error_code pwcheck_check(krb5_context, krb5_pwqual_moddata,
-                              const char *password, const char *principal,
-                              char *errstr, int errstrlen);
+                              const char *password, const char *principal);
 
 /* Finished checking passwords.  Free internal data. */
 void pwcheck_close(krb5_context, krb5_pwqual_moddata);
index 4aa550b81df001e8743c257f5623cc0197d8f948..d6583d6c12fcbbfe1331fdfc02d592d88a931dea 100644 (file)
@@ -71,7 +71,7 @@ heimdal_pwcheck(krb5_context ctx, krb5_principal principal,
     char *pastring;
     char *name = NULL;
     krb5_error_code code;
-    int result;
+    const char *error;
 
     pastring = malloc(password->length + 1);
     if (pastring == NULL) {
@@ -81,23 +81,34 @@ heimdal_pwcheck(krb5_context ctx, krb5_principal principal,
     }
     memcpy(pastring, password->data, password->length);
     pastring[password->length] = '\0';
-    if (pwcheck_init(ctx, NULL, &data) != 0) {
-        snprintf(message, length, "cannot initialize strength checking");
+    code = pwcheck_init(ctx, NULL, &data);
+    if (code != 0) {
+        error = krb5_get_error_message(ctx, code);
+        snprintf(message, length, "cannot initialize strength checking: %s",
+                 error);
+        krb5_free_error_message(ctx, error);
         free(pastring);
         return 1;
     }
     code = krb5_unparse_name(ctx, principal, &name);
     if (code != 0) {
-        strlcpy(message, "cannot unparse principal name", length);
+        error = krb5_get_error_message(ctx, code);
+        snprintf(message, length, "cannot unparse principal name: %s", error);
+        krb5_free_error_message(ctx, error);
         free(pastring);
         pwcheck_close(ctx, data);
         return 1;
     }
-    result = pwcheck_check(ctx, data, pastring, name, message, length);
+    code = pwcheck_check(ctx, data, pastring, name);
+    if (code != 0) {
+        error = krb5_get_error_message(ctx, code);
+        snprintf(message, length, "%s", error);
+        krb5_free_error_message(ctx, error);
+    }
     krb5_free_unparsed_name(ctx, name);
     free(pastring);
     pwcheck_close(ctx, data);
-    return (result == 0) ? 0 : 1;
+    return (code == 0) ? 0 : 1;
 }
 
 /* The public symbol that Heimdal looks for. */
index 0ef01b8aece86637ea2290f9f663171a1b57e51b..5649c343426166b851de219af8e025cf39e67e42 100644 (file)
@@ -41,18 +41,7 @@ krb5_error_code pwqual_strength_initvt(krb5_context, int, int,
 static krb5_error_code
 init(krb5_context ctx, const char *dictionary, krb5_pwqual_moddata *data)
 {
-    krb5_error_code code;
-
-    code = pwcheck_init(ctx, dictionary, data);
-    if (code != 0) {
-        krb5_set_error_message(ctx, code, "cannot initialize strength"
-                               " checking%s%s: %s",
-                               dictionary == NULL ? "" : " with dictionary",
-                               dictionary == NULL ? "" : dictionary,
-                               strerror(errno));
-        return code;
-    }
-    return 0;
+    return pwcheck_init(ctx, dictionary, data);
 }
 
 
@@ -66,17 +55,14 @@ check(krb5_context ctx, krb5_pwqual_moddata data, const char *password,
       const char **languages UNUSED)
 {
     char *name = NULL;
-    krb5_error_code status;
-    char error[BUFSIZ];
-
-    status = krb5_unparse_name(ctx, princ, &name);
-    if (status != 0)
-        return status;
-    status = pwcheck_check(ctx, data, password, name, error, sizeof(error));
-    if (status != 0)
-        krb5_set_error_message(ctx, status, "%s", error);
+    krb5_error_code code;
+
+    code = krb5_unparse_name(ctx, princ, &name);
+    if (code != 0)
+        return code;
+    code = pwcheck_check(ctx, data, password, name);
     krb5_free_unparsed_name(ctx, name);
-    return status;
+    return code;
 }
 
 
index e0178c8f7274b865a6f0ed776c57329f443330a5..be7ec4d3352b3c2ba9a0dce755d68c5093acd28d 100644 (file)
         "principal": "someuser@EXAMPLE.ORG",
         "password": "someuser",
         "code": "KADM5_PASS_Q_GENERIC",
-        "error": "Password based on username"
+        "error": "password based on username"
     },
     {
         "name": "based on principal (reversed)",
         "principal": "someuser@EXAMPLE.ORG",
         "password": "resuemos",
         "code": "KADM5_PASS_Q_GENERIC",
-        "error": "Password based on username"
+        "error": "password based on username"
     },
     {
         "name": "based on principal with digits",
         "principal": "someuser@EXAMPLE.ORG",
         "password": "someuser123",
         "code": "KADM5_PASS_Q_GENERIC",
-        "error": "Password based on username"
+        "error": "password based on username"
     },
     {
         "name": "is full principal",
         "principal": "test@EXAMPLE.ORG",
         "password": "test@EXAMPLE.ORG",
         "code": "KADM5_PASS_Q_GENERIC",
-        "error": "Password based on username"
+        "error": "password based on username"
     },
     {
         "name": "long password complexity",
index 0c862a26ece145a9f952a6f34263672ccb4277c5..66d4761864def5d7c5b82e54e34a0e507cad1864 100755 (executable)
@@ -134,12 +134,14 @@ local $ENV{KRB5_CONFIG} = test_file_path('data/krb5.conf');
 
 # Run a test with no Kerberos password dictionary configuration and check that
 # we get the correct error message.
+my $error = 'Cannot initialize strength checking: password_dictionary not'
+  . ' configured in krb5.conf';
 my $test = {
     name      => 'no dictionary configured',
     principal => 'test@EXAMPLE.COM',
     password  => 'password',
     status    => 1,
-    error     => 'cannot initialize strength checking',
+    error     => $error,
 };
 check_password($test);
 
index 7567780ac7c9ae776c64a8abb36ac1afb3583a75..44153febe13d1ded7cfc6f62df49b8574c923200 100644 (file)
@@ -150,7 +150,8 @@ main(void)
         "test@EXAMPLE.ORG",
         "password",
         1,
-        "cannot initialize strength checking",
+        "cannot initialize strength checking: password_dictionary not"
+        " configured in krb5.conf",
     };
 
     /* If we're not building with Heimdal, we can't run this test. */