Add NEWS, documentation, and test suite for cracklib_maxlen

Also fix a few coding style nits.

diff --git a/NEWS b/NEWS
index dde582b5f56414445c5b7e521172d0c0348cce65..e9242ccb82e59071f92030f86576dee0e35efa67 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,13 @@
 
 krb5-strength 3.1 (unreleased)
 
+    A new configuration option, cracklib_maxlen, can be set to skip
+    CrackLib checks of passwords longer than that length.  The CrackLib
+    rules were designed in a world in which most passwords were four to
+    eight characters long and tend to spuriously reject longer passwords.
+    SQLite dictionaries work better for checking longer passwords and
+    passphrases.  Patch from Jorj Bauer.
+
     Change the error messages returned for passwords that fail strength
     checking to start with a capital letter.  This appears to be more
     consistent with the error message conventions used inside Heimdal.

diff --git a/README b/README
index 8d88ed46d3d63a3a54a9416adaafada9ef66def5..508d21cd257b3bd59133e5ede7e62bbfe6a4b0ed 100644 (file)
--- a/README
+++ b/README
@@ -411,6 +411,16 @@ CONFIGURATION
   The following additional settings are supported in the [appdefaults]
   section of krb5.conf when running under either Heimdal or MIT Kerberos.
 
+  cracklib_maxlen
+
+      Normally, all passwords are checked with CrackLib if a CrackLib
+      dictionary is defined.  However, CrackLib's rules were designed for
+      a world in which most passwords were four to eight characters long,
+      and tends to spuriously reject a lot of passphrases.  If this option
+      is set to something other than its default of 0, passwords longer
+      than that length bypass CrackLib checks.  (Using a SQLite dictionary
+      for longer passwords is strongly recommended.)
+
   minimum_different
 
       If set to a numeric value, passwords with fewer than this number of

diff --git a/plugin/cracklib.c b/plugin/cracklib.c
index ed5d4aeb8fca21e210886a99a7273a9df73fc66a..7bb8c7e863a8fad6164c91702d05be685eb40c7c 100644 (file)
--- a/plugin/cracklib.c
+++ b/plugin/cracklib.c
@@ -88,6 +88,11 @@ strength_check_cracklib(krb5_context ctx, krb5_pwqual_moddata data,
     if (data->dictionary == NULL)
         return 0;
 
+    /* Nothing to do if the password is longer than the maximum length. */
+    if (data->cracklib_maxlen > 0)
+        if (strlen(password) > (size_t) data->cracklib_maxlen)
+            return 0;
+
     /* Check the password against CrackLib and return the results. */
     result = FascistCheck(password, data->dictionary);
     if (result != NULL)

diff --git a/plugin/general.c b/plugin/general.c
index aeb43b9e70c12430c8058f41d262a70e41a9ed02..66f2d2da87f7a2462c402c6499822ce7a67bdc9c 100644 (file)
--- a/plugin/general.c
+++ b/plugin/general.c
@@ -55,14 +55,14 @@ strength_init(krb5_context ctx, const char *dictionary,
     strength_config_boolean(ctx, "require_ascii_printable", &data->ascii);
     strength_config_boolean(ctx, "require_non_letter", &data->nonletter);
 
-    /* Get trapdoor length from krb5.conf. */
-    strength_config_number(ctx, "cracklib_maxlen", &data->cracklib_maxlen);
-
     /* Get complex character class restrictions from krb5.conf. */
     code = strength_config_classes(ctx, "require_classes", &data->rules);
     if (code != 0)
         goto fail;
 
+    /* Get CrackLib maximum length from krb5.conf. */
+    strength_config_number(ctx, "cracklib_maxlen", &data->cracklib_maxlen);
+
     /*
      * Try to initialize CDB, CrackLib, and SQLite dictionaries.  These
      * functions handle their own configuration parsing and will do nothing if
@@ -202,15 +202,10 @@ strength_check(krb5_context ctx UNUSED, krb5_pwqual_moddata data,
     if (code != 0)
         return code;
 
-    if (data->cracklib_maxlen == 0 ||
-       ((long) strlen(password) <= data->cracklib_maxlen)) {
-
-      /* Check the password against CDB, CrackLib, and SQLite if configured. */
-      code = strength_check_cracklib(ctx, data, password);
-      if (code != 0)
+    /* Check the password against CDB, CrackLib, and SQLite if configured. */
+    code = strength_check_cracklib(ctx, data, password);
+    if (code != 0)
         return code;
-    }
-
     code = strength_check_cdb(ctx, data, password);
     if (code != 0)
         return code;

diff --git a/plugin/internal.h b/plugin/internal.h
index fc0cf1467e8045302177b22c5dccb6d9c5e0d60b..bbd20ae24f14cffdaeb94cd87c5bb09e088f89cb 100644 (file)
--- a/plugin/internal.h
+++ b/plugin/internal.h
@@ -80,6 +80,7 @@ struct krb5_pwqual_moddata_st {
     bool nonletter;             /* Whether to require a non-letter */
     struct class_rule *rules;   /* Linked list of character class rules */
     char *dictionary;           /* Base path to CrackLib dictionary */
+    long cracklib_maxlen;       /* Longer passwords skip CrackLib checks */
     bool have_cdb;              /* Whether we have a CDB dictionary */
     int cdb_fd;                 /* File descriptor of CDB dictionary */
 #ifdef HAVE_CDB_H
@@ -90,7 +91,6 @@ struct krb5_pwqual_moddata_st {
     sqlite3_stmt *prefix_query; /* Query using the password prefix */
     sqlite3_stmt *suffix_query; /* Query using the reversed password suffix */
 #endif
-    long cracklib_maxlen;       /* Longer passwords skip cracklib */
 };
 
 BEGIN_DECLS

diff --git a/tests/plugin/heimdal-t.c b/tests/plugin/heimdal-t.c
index 706bd6b271f979b108b0c320e3308c062263b23a..ddbe1766174a22cc17ed0242bcd857c8489cd88a 100644 (file)
--- a/tests/plugin/heimdal-t.c
+++ b/tests/plugin/heimdal-t.c
@@ -150,10 +150,10 @@ main(void)
      * three times, once each with CrackLib, CDB, and SQLite.
      */
     count = ARRAY_SIZE(cracklib_tests);
+    count += 2 * ARRAY_SIZE(length_tests);
     count += ARRAY_SIZE(cdb_tests);
     count += ARRAY_SIZE(sqlite_tests);
     count += ARRAY_SIZE(classes_tests);
-    count += ARRAY_SIZE(length_tests);
     count += ARRAY_SIZE(letter_tests);
     count += ARRAY_SIZE(principal_tests) * 3;
     plan(5 + count * 2);
@@ -191,6 +191,22 @@ main(void)
     for (i = 0; i < ARRAY_SIZE(principal_tests); i++)
         is_password_test(verifier, &principal_tests[i]);
 
+    /*
+     * Add length restrictions and a maximum length for CrackLib.  This should
+     * reject passwords as too short, but let through a password that's
+     * actually in the CrackLib dictionary.
+     */
+    setup_argv[5] = (char *) "minimum_length";
+    setup_argv[6] = (char *) "12";
+    setup_argv[7] = (char *) "cracklib_maxlen";
+    setup_argv[8] = (char *) "11";
+    setup_argv[9] = NULL;
+    run_setup((const char **) setup_argv);
+
+    /* Run the length tests. */
+    for (i = 0; i < ARRAY_SIZE(length_tests); i++)
+        is_password_test(verifier, &length_tests[i]);
+
     /* Add simple character class restrictions. */
     setup_argv[5] = (char *) "minimum_different";
     setup_argv[6] = (char *) "8";
@@ -216,10 +232,7 @@ main(void)
     for (i = 0; i < ARRAY_SIZE(classes_tests); i++)
         is_password_test(verifier, &classes_tests[i]);
 
-    /*
-     * Add length restrictions.  This should only do length checks without any
-     * dictionary checks.
-     */
+    /* Try the length checks again with no dictionary at all. */
     setup_argv[3] = (char *) "minimum_length";
     setup_argv[4] = (char *) "12";
     setup_argv[5] = NULL;

diff --git a/tests/plugin/mit-t.c b/tests/plugin/mit-t.c
index 5922ab9a2d8deda95e7845892f9874a064502055..838964ff4452513fe7bfe1f9f6b33e29e51c8a85 100644 (file)
--- a/tests/plugin/mit-t.c
+++ b/tests/plugin/mit-t.c
@@ -168,13 +168,13 @@ main(void)
      * with CrackLib, CDB, and SQLite configurations.
      */
     count = 2 * ARRAY_SIZE(cracklib_tests);
+    count += 2 * ARRAY_SIZE(length_tests);
     count += ARRAY_SIZE(cdb_tests);
     count += ARRAY_SIZE(sqlite_tests);
     count += ARRAY_SIZE(classes_tests);
-    count += ARRAY_SIZE(length_tests);
     count += ARRAY_SIZE(letter_tests);
     count += 3 * ARRAY_SIZE(principal_tests);
-    plan(2 + 7 + count * 2);
+    plan(2 + 8 + count * 2);
 
     /* Start with the krb5.conf that contains no dictionary configuration. */
     path = test_file_path("data/krb5.conf");
@@ -244,6 +244,33 @@ main(void)
         is_password_test(ctx, vtable, data, &cracklib_tests[i]);
     vtable->close(ctx, data);
 
+    /*
+     * Add length restrictions and a maximum length for CrackLib.  This should
+     * reject passwords as too short, but let through a password that's
+     * actually in the CrackLib dictionary.
+     */
+    setup_argv[5] = (char *) "minimum_length";
+    setup_argv[6] = (char *) "12";
+    setup_argv[7] = (char *) "cracklib_maxlen";
+    setup_argv[8] = (char *) "11";
+    setup_argv[9] = NULL;
+    run_setup((const char **) setup_argv);
+
+    /* Obtain a new Kerberos context with that krb5.conf file. */
+    krb5_free_context(ctx);
+    code = krb5_init_context(&ctx);
+    if (code != 0)
+        bail_krb5(ctx, code, "cannot initialize Kerberos context");
+
+    /* Run all of the length tests. */
+    code = vtable->open(ctx, NULL, &data);
+    is_int(0, code, "Plugin initialization (length)");
+    if (code != 0)
+        bail("cannot continue after plugin initialization failure");
+    for (i = 0; i < ARRAY_SIZE(length_tests); i++)
+        is_password_test(ctx, vtable, data, &length_tests[i]);
+    vtable->close(ctx, data);
+
     /* Add simple character class configuration to krb5.conf. */
     setup_argv[5] = (char *) "minimum_different";
     setup_argv[6] = (char *) "8";
@@ -293,10 +320,7 @@ main(void)
         is_password_test(ctx, vtable, data, &classes_tests[i]);
     vtable->close(ctx, data);
 
-    /*
-     * Add length restrictions.  This should only do length checks without any
-     * dictionary checks.
-     */
+    /* Re-run the length restriction checks with no dictionary at all. */
     setup_argv[3] = (char *) "minimum_length";
     setup_argv[4] = (char *) "12";
     setup_argv[5] = NULL;