Coding style cleanup and tests for minimum classes

Add tests for specifying a minimum number of classes, refactor for
coding style a bit, and add new tests for the new syntax errors.

diff --git a/plugin/classes.c b/plugin/classes.c
index 866caa145283a0bb5459fa3d4ab50849f5806640..bf9ef019888820a2c6626621397cdc42575f827a 100644 (file)
--- a/plugin/classes.c
+++ b/plugin/classes.c
@@ -4,6 +4,7 @@
  * Checks whether the password satisfies a set of character class rules.
  *
  * Written by Russ Allbery <eagle@eyrie.org>
+ * Copyright 2016 Russ Allbery <eagle@eyrie.org>
  * Copyright 2013, 2014
  *     The Board of Trustees of the Leland Stanford Junior University
  *
@@ -47,15 +48,10 @@ analyze_password(const char *password, struct password_classes *classes)
         else
             classes->symbol = true;
     }
-
-    if (classes->lower)
-       classes->num_classes++;
-    if (classes->upper)
-       classes->num_classes++;
-    if (classes->digit)
-       classes->num_classes++;
-    if (classes->symbol)
-       classes->num_classes++;
+    if (classes->lower)  classes->num_classes++;
+    if (classes->upper)  classes->num_classes++;
+    if (classes->digit)  classes->num_classes++;
+    if (classes->symbol) classes->num_classes++;
 }
 
 
@@ -70,20 +66,16 @@ check_rule(krb5_context ctx, struct class_rule *rule, size_t length,
 {
     if (length < rule->min || (rule->max > 0 && length > rule->max))
         return 0;
-
     if (classes->num_classes < rule->num_classes)
-        return strength_error_class((ctx),
-                                   "password must have %lu character classes",
-                                   rule->num_classes);
-    
+        return strength_error_class(ctx, ERROR_CLASS_MIN, rule->num_classes);
     if (rule->lower && !classes->lower)
-        return strength_error_class((ctx), ERROR_CLASS_LOWER);
+        return strength_error_class(ctx, ERROR_CLASS_LOWER);
     if (rule->upper && !classes->upper)
-        return strength_error_class((ctx), ERROR_CLASS_UPPER);
+        return strength_error_class(ctx, ERROR_CLASS_UPPER);
     if (rule->digit && !classes->digit)
-        return strength_error_class((ctx), ERROR_CLASS_DIGIT);
+        return strength_error_class(ctx, ERROR_CLASS_DIGIT);
     if (rule->symbol && !classes->symbol)
-        return strength_error_class((ctx), ERROR_CLASS_SYMBOL);
+        return strength_error_class(ctx, ERROR_CLASS_SYMBOL);
     return 0;
 }
 

diff --git a/plugin/config.c b/plugin/config.c
index 7fc3f66cd2a318b841584056a9abcc6cd6a18c77..0114740ad480f7bcaaa575d1614df2f927295f5a 100644 (file)
--- a/plugin/config.c
+++ b/plugin/config.c
@@ -6,6 +6,7 @@
  * krb5_appdefaults_* functions.
  *
  * Written by Russ Allbery <eagle@eyrie.org>
+ * Copyright 2016 Russ Allbery <eagle@eyrie.org>
  * Copyright 2013
  *     The Board of Trustees of the Leland Stanford Junior University
  *
@@ -22,9 +23,6 @@
 #include <plugin/internal.h>
 #include <util/macros.h>
 
-/* maximum number of character classes */
-#define MAX_CLASSES 4
-
 /* The representation of the realm differs between MIT and Kerberos. */
 #ifdef HAVE_KRB5_REALM
 typedef krb5_realm realm_type;
@@ -32,6 +30,9 @@ typedef krb5_realm realm_type;
 typedef krb5_data *realm_type;
 #endif
 
+/* Maximum number of character classes. */
+#define MAX_CLASSES 4
+
 
 /*
  * Obtain the default realm and translate it into the format required by
@@ -163,7 +164,7 @@ parse_class(krb5_context ctx, const char *spec, struct class_rule **rule)
     struct vector *classes = NULL;
     size_t i;
     krb5_error_code code;
-    const char *end;
+    const char *class, *end;
     bool okay;
 
     /* Create the basic rule structure. */
@@ -204,28 +205,26 @@ parse_class(krb5_context ctx, const char *spec, struct class_rule **rule)
      * unknown character class.
      */
     for (i = 0; i < classes->count; i++) {
-        if (strcmp(classes->strings[i], "upper") == 0)
+        class = classes->strings[i];
+        if (strcmp(class, "upper") == 0)
             (*rule)->upper = true;
-        else if (strcmp(classes->strings[i], "lower") == 0)
+        else if (strcmp(class, "lower") == 0)
             (*rule)->lower = true;
-        else if (strcmp(classes->strings[i], "digit") == 0)
+        else if (strcmp(class, "digit") == 0)
             (*rule)->digit = true;
-        else if (strcmp(classes->strings[i], "symbol") == 0)
+        else if (strcmp(class, "symbol") == 0)
             (*rule)->symbol = true;
-       else if (isdigit((unsigned char) *classes->strings[i])) {
-           okay = parse_number(classes->strings[i],
-                               &(*rule)->num_classes, &end);
+       else if (isdigit((unsigned char) *class)) {
+           okay = parse_number(class, &(*rule)->num_classes, &end);
            if (!okay || *end != '\0' || (*rule)->num_classes > MAX_CLASSES) {
-                code = strength_error_config(ctx, "bad character class"
-                                             " requirement in configuration:"
-                                             " %s",
-                                            classes->strings[i]);
+                code = strength_error_config(ctx, "bad character class minimum"
+                                             " in configuration: %s", class);
                goto fail;
            }
        }
         else {
             code = strength_error_config(ctx, "unknown character class %s",
-                                         classes->strings[i]);
+                                         class);
             goto fail;
         }
     }

diff --git a/plugin/internal.h b/plugin/internal.h
index 94d73c436085e007cb3efc223d08b8af45246f5b..011bbb6cd6a31aff6f20d0f29964d43d54d30678 100644 (file)
--- a/plugin/internal.h
+++ b/plugin/internal.h
@@ -38,6 +38,9 @@ typedef struct krb5_pwqual_moddata_st *krb5_pwqual_moddata;
 #define ERROR_CLASS_DIGIT  "Password must contain a number"
 #define ERROR_CLASS_SYMBOL \
     "Password must contain a space or punctuation character"
+#define ERROR_CLASS_MIN \
+    "Password must contain %lu types of characters (lowercase, uppercase," \
+    " numbers, symbols)"
 #define ERROR_DICT         "Password found in list of common passwords"
 #define ERROR_LETTER       "Password is only letters and spaces"
 #define ERROR_MINDIFF      "Password does not contain enough unique characters"

diff --git a/tests/data/passwords/classes.json b/tests/data/passwords/classes.json
index 0e03106696345ca097b26a75f49218cf5b4fe397..8db1a92ecf7f38d78d7d7907f5754bc566376569 100644 (file)
--- a/tests/data/passwords/classes.json
+++ b/tests/data/passwords/classes.json
@@ -146,5 +146,29 @@
         "name": "all classes with space (20)",
         "principal": "test@EXAMPLE.ORG",
         "password": "pass wordX9wordwordw"
+    },
+    {
+        "name": "only lowercase (24)",
+        "principal": "test@EXAMPLE.ORG",
+        "password": "alllowercasewithclassreq",
+        "code": "KADM5_PASS_Q_CLASS",
+        "error": "Password must contain 3 types of characters (lowercase, uppercase, numbers, symbols)"
+    },
+    {
+        "name": "lower and uppercase (24)",
+        "principal": "test@EXAMPLE.ORG",
+        "password": "LowerUprcasewithclassreq",
+        "code": "KADM5_PASS_Q_CLASS",
+        "error": "Password must contain 3 types of characters (lowercase, uppercase, numbers, symbols)"
+    },
+    {
+        "name": "lower, uppercase, symbols (24)",
+        "principal": "test@EXAMPLE.ORG",
+        "password": "LowerUp!casewithclassreq"
+    },
+    {
+        "name": "only lowercase (25)",
+        "principal": "test@EXAMPLE.ORG",
+        "password": "alllowercasewithclassreqr"
     }
 ]

diff --git a/tests/plugin/heimdal-t.c b/tests/plugin/heimdal-t.c
index ddbe1766174a22cc17ed0242bcd857c8489cd88a..1ec61f436905a692688638b33ce7f94b5e379494 100644 (file)
--- a/tests/plugin/heimdal-t.c
+++ b/tests/plugin/heimdal-t.c
@@ -224,7 +224,7 @@ main(void)
     /* Add complex character class restrictions and remove the dictionary. */
     free(setup_argv[4]);
     setup_argv[3] = (char *) "require_classes";
-    setup_argv[4] = (char *) "8-19:lower,upper 8-15:digit 8-11:symbol";
+    setup_argv[4] = (char *) "8-19:lower,upper 8-15:digit 8-11:symbol 24-24:3";
     setup_argv[5] = NULL;
     run_setup((const char **) setup_argv);
 

diff --git a/tests/plugin/mit-t.c b/tests/plugin/mit-t.c
index 838964ff4452513fe7bfe1f9f6b33e29e51c8a85..7eba594156091f927048869c99de21d5d3b74dd1 100644 (file)
--- a/tests/plugin/mit-t.c
+++ b/tests/plugin/mit-t.c
@@ -301,7 +301,7 @@ main(void)
      * the dictionary configuration.
      */
     setup_argv[3] = (char *) "require_classes";
-    setup_argv[4] = (char *) "8-19:lower,upper 8-15:digit 8-11:symbol";
+    setup_argv[4] = (char *) "8-19:lower,upper 8-15:digit 8-11:symbol 24-24:3";
     setup_argv[5] = NULL;
     run_setup((const char **) setup_argv);
 
@@ -315,7 +315,7 @@ main(void)
     code = vtable->open(ctx, NULL, &data);
     is_int(0, code, "Plugin initialization (complex character class)");
     if (code != 0)
-        bail("cannot continue after plugin initialization failure");
+        bail_krb5(ctx, code, "plugin initialization failure");
     for (i = 0; i < ARRAY_SIZE(classes_tests); i++)
         is_password_test(ctx, vtable, data, &classes_tests[i]);
     vtable->close(ctx, data);

diff --git a/tests/tools/heimdal-strength-t b/tests/tools/heimdal-strength-t
index 4f2657e03f419c07891f5273cbd9ec6360dec098..e6924405c9e57e86cdfb2e73fe2ba786c894667b 100755 (executable)
--- a/tests/tools/heimdal-strength-t
+++ b/tests/tools/heimdal-strength-t
@@ -152,6 +152,37 @@ sub load_password_tests {
     return $json->decode($testdata);
 }
 
+# Test a required_classes syntax error.  Takes the string for required_classes
+# and verifies that the appropriate error message is returned.
+#
+# $bad_class - Bad class specification
+#
+# Returns: undef
+sub test_require_classes_syntax {
+    my ($bad_class)  = @_;
+    my $error_prefix = 'Cannot initialize strength checking';
+    my $bad_message  = 'bad character class requirement in configuration';
+    my $bad_minimum  = 'bad character class minimum in configuration';
+
+    # Run heimdal-strength.
+    my $krb5_conf = create_krb5_conf({ require_classes => $bad_class });
+    local $ENV{KRB5_CONFIG} = $krb5_conf;
+    my ($status, $output, $err) = run_heimdal_strength('test', 'password', 1);
+
+    # Check the results.
+    is($status, 1,   "Bad class specification '$bad_class' (status)");
+    is($output, q{}, '...no output');
+    my $expected;
+    if ($bad_class =~ m{ \A (\d+ [^-]*) \z | : (\d+) \z }xms) {
+        my $minimum = $1 || $2;
+        $expected = "$error_prefix: $bad_minimum: $minimum\n";
+    } else {
+        $expected = "$error_prefix: $bad_message: $bad_class\n";
+    }
+    is($err, $expected, '...correct error');
+    return;
+}
+
 # Load the password tests from JSON.  Accumulate a total count of tests for
 # the testing plan.
 my (%tests, $count);
@@ -168,8 +199,8 @@ $count += 2 * scalar(@{ $tests{principal} });
 $count += scalar(@{ $tests{length} });
 
 # We can now calculate our plan based on three tests for each password test,
-# plus 21 additional tests for error handling.
-plan(tests => $count * 3 + 21);
+# plus 27 additional tests for error handling.
+plan(tests => $count * 3 + 27);
 
 # Install the krb5.conf file with a configuration pointing to the test
 # CrackLib dictionary.
@@ -233,7 +264,7 @@ for my $test (@{ $tests{letter} }) {
 }
 
 # Install the krb5.conf file for complex character class restrictions.
-my $classes = '8-19:lower,upper 8-15:digit 8-11:symbol';
+my $classes = '8-19:lower,upper 8-15:digit 8-11:symbol 24-24:3';
 $krb5_conf = create_krb5_conf({ require_classes => $classes });
 local $ENV{KRB5_CONFIG} = $krb5_conf;
 
@@ -306,16 +337,10 @@ is($err, "$error_prefix: unknown character class bogus\n", '...correct error');
 
 # Test a variety of configuration syntax errors in require_classes.
 my @bad_classes = qw(
-  8 8bogus 8:bogus 4-:bogus 4-bogus 4-8bogus
+  8 8bogus 8:bogus 4-:bogus 4-bogus 4-8bogus 10:3 10-11:5
 );
-my $bad_message = 'bad character class requirement in configuration';
 for my $bad_class (@bad_classes) {
-    $krb5_conf = create_krb5_conf({ require_classes => $bad_class });
-    local $ENV{KRB5_CONFIG} = $krb5_conf;
-    ($status, $output, $err) = run_heimdal_strength('test', 'password', 1);
-    is($status, 1,   "Bad class specification '$bad_class' (status)");
-    is($output, q{}, '...no output');
-    is($err, "$error_prefix: $bad_message: $bad_class\n", '...correct error');
+    test_require_classes_syntax($bad_class);
 }
 
 # Clean up our temporary krb5.conf file on any exit.