From: Russ Allbery Date: Fri, 23 Dec 2016 19:43:11 +0000 (-0800) Subject: Coding style cleanup and tests for minimum classes X-Git-Tag: release/3.1~3 X-Git-Url: https://git.eyrie.org/?a=commitdiff_plain;h=98189413ba53f4a7ba675ce480b3a7323a58ca78;p=kerberos%2Fkrb5-strength.git 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 866caa1..bf9ef01 100644 --- 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 + * Copyright 2016 Russ Allbery * 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 7fc3f66..0114740 100644 --- a/plugin/config.c +++ b/plugin/config.c @@ -6,6 +6,7 @@ * krb5_appdefaults_* functions. * * Written by Russ Allbery + * Copyright 2016 Russ Allbery * Copyright 2013 * The Board of Trustees of the Leland Stanford Junior University * @@ -22,9 +23,6 @@ #include #include -/* 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 94d73c4..011bbb6 100644 --- 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 0e03106..8db1a92 100644 --- 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 ddbe176..1ec61f4 100644 --- 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 838964f..7eba594 100644 --- 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 4f2657e..e692440 100755 --- 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.