From fcc569d62e750a8476a525662ec9cdf1f4868a83 Mon Sep 17 00:00:00 2001 From: Russ Allbery Date: Thu, 12 Dec 2013 16:25:03 -0800 Subject: [PATCH] Fix various character class check mistakes, add test suite This is the first working version of the character class checking, which is now plugged into the module initialization. It also adds a test suite for the external password check utility, although not the embedded modules yet. --- plugin/classes.c | 9 +++---- plugin/config.c | 4 ++-- plugin/general.c | 7 +++++- tests/data/passwords/classes.json | 40 +++++++++++++++++++++++++++++++ tests/tools/heimdal-strength-t | 15 ++++++++++-- 5 files changed, 66 insertions(+), 9 deletions(-) create mode 100644 tests/data/passwords/classes.json diff --git a/plugin/classes.c b/plugin/classes.c index 683b5de..6e79d14 100644 --- a/plugin/classes.c +++ b/plugin/classes.c @@ -27,7 +27,7 @@ struct password_classes { /* Abbreviate the most common error reporting syntax. */ #define MUST_HAVE(ctx, err) \ - strength_error_class((ctx), "password must contain" err) + strength_error_class((ctx), "password must contain " err) /* @@ -39,7 +39,8 @@ analyze_password(const char *password, struct password_classes *classes) { const char *p; - for (p = password; p != '\0'; p++) { + memset(classes, 0, sizeof(struct password_classes)); + for (p = password; *p != '\0'; p++) { if (islower((unsigned char) *p)) classes->lower = true; else if (isupper((unsigned char) *p)) @@ -68,9 +69,9 @@ check_rule(krb5_context ctx, struct class_rule *rule, size_t length, if (rule->upper && !classes->upper) return MUST_HAVE(ctx, "an uppercase letter"); if (rule->digit && !classes->digit) - return MUST_HAVE(ctx, "a digit"); + return MUST_HAVE(ctx, "a number"); if (rule->symbol && !classes->symbol) - return MUST_HAVE(ctx, "a symbol"); + return MUST_HAVE(ctx, "a space or punctuation character"); return 0; } diff --git a/plugin/config.c b/plugin/config.c index 70394b5..414de04 100644 --- a/plugin/config.c +++ b/plugin/config.c @@ -186,7 +186,7 @@ krb5_error_code strength_config_classes(krb5_context ctx, const char *opt, struct class_rule **result) { - struct vector *config; + struct vector *config = NULL; struct class_rule *rules, *last, *tmp; krb5_error_code code; size_t i; @@ -248,7 +248,7 @@ strength_config_list(krb5_context ctx, const char *opt, /* Obtain the string from [appdefaults]. */ realm = default_realm(ctx); - krb5_appdefault_string(ctx, "krb5-sync", realm, opt, "", &value); + krb5_appdefault_string(ctx, "krb5-strength", realm, opt, "", &value); free_default_realm(ctx, realm); /* If we got something back, store it in result. */ diff --git a/plugin/general.c b/plugin/general.c index 4dde3ea..ca0c23c 100644 --- a/plugin/general.c +++ b/plugin/general.c @@ -50,10 +50,15 @@ strength_init(krb5_context ctx, const char *dictionary, /* Get minimum length information from krb5.conf. */ strength_config_number(ctx, "minimum_length", &data->minimum_length); - /* Get character class restrictions from krb5.conf. */ + /* Get simple character class restrictions from krb5.conf. */ strength_config_boolean(ctx, "require_ascii_printable", &data->ascii); strength_config_boolean(ctx, "require_non_letter", &data->nonletter); + /* Get complex character class restrictions from krb5.conf. */ + code = strength_config_classes(ctx, "require_classes", &data->rules); + if (code != 0) + goto fail; + /* * Try to initialize CDB and CrackLib dictionaries. Both functions handle * their own configuration parsing and will do nothing if the diff --git a/tests/data/passwords/classes.json b/tests/data/passwords/classes.json new file mode 100644 index 0000000..b8dfbff --- /dev/null +++ b/tests/data/passwords/classes.json @@ -0,0 +1,40 @@ +[ + { + "name": "no lowercase", + "principal": "test@EXAMPLE.ORG", + "password": "PASSWORD98!", + "code": "KADM5_PASS_Q_CLASS", + "error": "password must contain a lowercase letter" + }, + { + "name": "no uppercase", + "principal": "test@EXAMPLE.ORG", + "password": "password98!", + "code": "KADM5_PASS_Q_CLASS", + "error": "password must contain an uppercase letter" + }, + { + "name": "no digit", + "principal": "test@EXAMPLE.ORG", + "password": "passwordXX!", + "code": "KADM5_PASS_Q_CLASS", + "error": "password must contain a number" + }, + { + "name": "no symbol", + "principal": "test@EXAMPLE.ORG", + "password": "passwordXX9", + "code": "KADM5_PASS_Q_CLASS", + "error": "password must contain a space or punctuation character" + }, + { + "name": "all classes", + "principal": "test@EXAMPLE.ORG", + "password": "passwordX9!" + }, + { + "name": "all classes with space", + "principal": "test@EXAMPLE.ORG", + "password": "pass wordX9" + } +] diff --git a/tests/tools/heimdal-strength-t b/tests/tools/heimdal-strength-t index 918b884..769f5fc 100755 --- a/tests/tools/heimdal-strength-t +++ b/tests/tools/heimdal-strength-t @@ -149,7 +149,7 @@ sub load_password_tests { # Load the password tests from JSON. Accumulate a total count of tests for # the testing plan. my (%tests, $count); -for my $type (qw(cdb cracklib length letter principal)) { +for my $type (qw(cdb classes cracklib length letter principal)) { my $tests = load_password_tests("$type.json"); $tests{$type} = $tests; $count += scalar(@{$tests}); @@ -197,12 +197,23 @@ $krb5_conf = create_krb5_conf( ); local $ENV{KRB5_CONFIG} = $krb5_conf; -# Run the character class tests. +# Run the simple character class tests. note('Simple password character class checks'); for my $test (@{ $tests{letter} }) { check_password($test); } +# Install the krb5.conf file for complex character class restrictions. +$krb5_conf + = create_krb5_conf({ require_classes => 'lower,upper,digit,symbol' }); +local $ENV{KRB5_CONFIG} = $krb5_conf; + +# Run the complex character class tests. +note('Complex password character class checks'); +for my $test (@{ $tests{classes} }) { + check_password($test); +} + # Install the krb5.conf file with configuration pointing to the CDB # dictionary. my $cdb_database = test_file_path('data/wordlist.cdb'); -- 2.39.2