From 1a62fa21678ccd78c37856bb9c1b68b3f33582fb Mon Sep 17 00:00:00 2001 From: Russ Allbery Date: Sun, 6 Nov 2016 15:37:17 -0800 Subject: [PATCH] Add NEWS, documentation, and test suite for cracklib_maxlen Also fix a few coding style nits. --- NEWS | 7 +++++++ README | 10 ++++++++++ plugin/cracklib.c | 5 +++++ plugin/general.c | 17 ++++++----------- plugin/internal.h | 2 +- tests/plugin/heimdal-t.c | 23 ++++++++++++++++++----- tests/plugin/mit-t.c | 36 ++++++++++++++++++++++++++++++------ 7 files changed, 77 insertions(+), 23 deletions(-) diff --git a/NEWS b/NEWS index dde582b..e9242cc 100644 --- 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 8d88ed4..508d21c 100644 --- 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 ed5d4ae..7bb8c7e 100644 --- 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 aeb43b9..66f2d2d 100644 --- 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 fc0cf14..bbd20ae 100644 --- 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 706bd6b..ddbe176 100644 --- 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 5922ab9..838964f 100644 --- 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; -- 2.39.2