Reject excessively long passwords

Reject passwords as long or longer than PAM_MAX_RESP_SIZE (normally
512 octets), since extremely long passwords can be used for a denial
of service attack via the Kerberos string to key function.  Thanks to
Florian Best for pointing out this issue and suggesting a good fix.

.gitignore

@@ -45,6 +45,7 @@
 /tests/module/expired-t
 /tests/module/fast-anon-t
 /tests/module/fast-t
+/tests/module/long-t
 /tests/module/no-cache-t
 /tests/module/pam-user-t
 /tests/module/password-t

Makefile.am

@@ -69,7 +69,7 @@ check_PROGRAMS = tests/runtests tests/module/alt-auth-t			    \
 	tests/module/bad-authtok-t tests/module/basic-t			    \
 	tests/module/cache-cleanup-t tests/module/cache-t		    \
 	tests/module/expired-t tests/module/fast-anon-t tests/module/fast-t \
-	tests/module/no-cache-t tests/module/pam-user-t			    \
+	tests/module/long-t tests/module/no-cache-t tests/module/pam-user-t \
 	tests/module/password-t tests/module/pkinit-t tests/module/realm-t  \
 	tests/module/stacked-t tests/module/trace-t tests/pam-util/args-t   \
 	tests/pam-util/fakepam-t tests/pam-util/logging-t		    \
@@ -114,6 +114,8 @@ tests_module_fast_anon_t_LDADD = $(MODULE_OBJECTS) tests/tap/libtap.a \
 	portable/libportable.la $(KRB5_LIBS)
 tests_module_fast_t_LDADD = $(MODULE_OBJECTS) tests/tap/libtap.a \
 	portable/libportable.la $(KRB5_LIBS)
+tests_module_long_t_LDADD = $(MODULE_OBJECTS) tests/tap/libtap.a \
+	portable/libportable.la $(KRB5_LIBS)
 tests_module_no_cache_t_LDADD = $(MODULE_OBJECTS) tests/tap/libtap.a \
 	portable/libportable.la $(KRB5_LIBS)
 tests_module_pam_user_t_LDADD = $(MODULE_OBJECTS) tests/tap/libtap.a \

NEWS

@@ -2,6 +2,11 @@
 
 pam-krb5 4.9 (unreleased)
 
+    Reject passwords as long or longer than PAM_MAX_RESP_SIZE (normally
+    512 octets), since extremely long passwords can be used for a denial
+    of service attack via the Kerberos string to key function.  Thanks to
+    Florian Best for pointing out this issue and suggesting a good fix.
+
     Update to rra-c-util 8.1:
 
     * Drop support for Perl 5.6.

auth.c

@@ -246,6 +246,11 @@ maybe_retrieve_password(struct pam_args *args, int authtok, const char **pass)
         }
         *pass = NULL;
     }
+    if (*pass != NULL && strlen(*pass) > PAM_MAX_RESP_SIZE - 1) {
+        putil_debug(args, "rejecting password longer than %d",
+                    PAM_MAX_RESP_SIZE - 1);
+        return PAM_AUTH_ERR;
+    }
     if (force && (status != PAM_SUCCESS || *pass == NULL)) {
         putil_debug_pam(args, status, "no stored password");
         return PAM_AUTH_ERR;
@@ -287,6 +292,13 @@ prompt_password(struct pam_args *args, int authtok, const char **pass)
         free(password);
         return PAM_AUTH_ERR;
     }
+    if (strlen(password) > PAM_MAX_RESP_SIZE - 1) {
+        putil_debug(args, "rejecting password longer than %d",
+                    PAM_MAX_RESP_SIZE - 1);
+        memset(password, 0, strlen(password));
+        free(password);
+        return PAM_AUTH_ERR;
+    }
 
     /* Set this for the next PAM module. */
     status = pam_set_item(args->pamh, authtok, password);

pam_krb5.pod

@@ -45,6 +45,10 @@ temporary ticket cache and writes it out to a persistent ticket cache
 owned by the user or uses the temporary ticket cache to refresh an
 existing user ticket cache.
 
+Passwords as long or longer than PAM_MAX_RESP_SIZE octets (normally 512
+octets) will be rejected, since excessively long passwords can be used as
+a denial of service attack.
+
 After doing the initial authentication, the Kerberos PAM module will
 attempt to obtain tickets for a key in the local system keytab and then
 verify those tickets.  Unless this step is performed, the authentication
@@ -123,6 +127,10 @@ then prompts the user for a new password, twice to ensure that the user
 entered it properly (again, unless configured to use an already entered
 password), and then does a Kerberos password change.
 
+Passwords as long or longer than PAM_MAX_RESP_SIZE octets (normally 512
+octets) will be rejected, since excessively long passwords can be used as
+a denial of service attack.
+
 Unlike the normal Unix password module, this module will allow any user to
 change any other user's password if they know the old password.  Also,
 unlike the normal Unix password module, root will always be prompted for

password.c

@@ -47,6 +47,12 @@ pamk5_password_prompt(struct pam_args *args, char **pass)
             pamret = PAM_AUTHTOK_ERR;
             goto done;
         }
+        if (strlen(tmp) > PAM_MAX_RESP_SIZE - 1) {
+            putil_debug(args, "rejecting password longer than %d",
+                        PAM_MAX_RESP_SIZE - 1);
+            pamret = PAM_AUTHTOK_ERR;
+            goto done;
+        }
         pass1 = strdup((const char *) tmp);
     }
 
@@ -58,6 +64,14 @@ pamk5_password_prompt(struct pam_args *args, char **pass)
             pamret = PAM_AUTHTOK_ERR;
             goto done;
         }
+        if (strlen(pass1) > PAM_MAX_RESP_SIZE - 1) {
+            putil_debug(args, "rejecting password longer than %d",
+                        PAM_MAX_RESP_SIZE - 1);
+            pamret = PAM_AUTHTOK_ERR;
+            memset(pass1, 0, strlen(pass1));
+            free(pass1);
+            goto done;
+        }
         pamret = pamk5_get_password(args, "Retype new", &pass2);
         if (pamret != PAM_SUCCESS) {
             putil_debug_pam(args, pamret, "error getting new password");

portable/pam.h

@@ -9,7 +9,7 @@
  * which can be found at <https://www.eyrie.org/~eagle/software/rra-c-util/>.
  *
  * Written by Russ Allbery <eagle@eyrie.org>
- * Copyright 2015 Russ Allbery <eagle@eyrie.org>
+ * Copyright 2015, 2020 Russ Allbery <eagle@eyrie.org>
  * Copyright 2010-2011, 2014
  *     The Board of Trustees of the Leland Stanford Junior University
  *
@@ -83,6 +83,11 @@
 #    define PAM_BAD_ITEM PAM_SYMBOL_ERR
 #endif
 
+/* We use this as a limit on password length, so make sure it's defined. */
+#ifndef PAM_MAX_RESP_SIZE
+#    define PAM_MAX_RESP_SIZE 512
+#endif
+
 /*
  * Some PAM implementations support building the module static and exporting
  * the call points via a struct instead.  (This is the default in OpenPAM, for

tests/TESTS

@@ -27,6 +27,7 @@ module/cache-cleanup    valgrind
 module/expired          valgrind
 module/fast             valgrind
 module/fast-anon
+module/long             valgrind
 module/no-cache         valgrind
 module/pam-user         valgrind
 module/password         valgrind

tests/data/cppcheck.supp

@@ -52,4 +52,4 @@ uninitvar:php/php5_remctl.c:129
 uninitvar:php/php5_remctl.c:321
 
 // (pam-krb5) cppcheck doesn't recognize the unused attribute on labels.
-unusedLabel:auth.c:872
+unusedLabel:auth.c:884

tests/data/scripts/long/password

@@ -0,0 +1,14 @@
+# Test authentication with an excessively long password.  -*- conf -*-
+#
+# Copyright 2020 Russ Allbery <eagle@eyrie.org>
+#
+# SPDX-License-Identifier: BSD-3-clause or GPL-1+
+
+[run]
+    authenticate = PAM_AUTH_ERR
+
+[prompts]
+    echo_off = Password: |%p
+
+[output]
+    NOTICE authentication failure; logname=%u uid=%i euid=%i tty= ruser= rhost=

tests/data/scripts/long/password-debug

@@ -0,0 +1,20 @@
+# Test excessively long password handling with debug logging.  -*- conf -*-
+#
+# Copyright 2020 Russ Allbery <eagle@eyrie.org>
+#
+# SPDX-License-Identifier: BSD-3-clause or GPL-1+
+
+[options]
+    auth = debug
+
+[run]
+    authenticate = PAM_AUTH_ERR
+
+[prompts]
+    echo_off = Password: |%p
+
+[output]
+    DEBUG pam_sm_authenticate: entry
+    DEBUG /^\(user %u\) rejecting password longer than [0-9]+$/
+    NOTICE authentication failure; logname=%u uid=%i euid=%i tty= ruser= rhost=
+    DEBUG pam_sm_authenticate: exit (failure)

tests/data/scripts/long/use-first

@@ -0,0 +1,14 @@
+# Test use_first_pass with an excessively long password.  -*- conf -*-
+#
+# Copyright 2020 Russ Allbery <eagle@eyrie.org>
+#
+# SPDX-License-Identifier: BSD-3-clause or GPL-1+
+
+[options]
+    auth = use_first_pass
+
+[run]
+    authenticate = PAM_AUTH_ERR
+
+[output]
+    NOTICE authentication failure; logname=%u uid=%i euid=%i tty= ruser= rhost=

tests/data/scripts/long/use-first-debug

@@ -0,0 +1,17 @@
+# Test use_first_pass with a long password and debug.  -*- conf -*-
+#
+# Copyright 2020 Russ Allbery <eagle@eyrie.org>
+#
+# SPDX-License-Identifier: BSD-3-clause or GPL-1+
+
+[options]
+    auth = use_first_pass debug
+
+[run]
+    authenticate = PAM_AUTH_ERR
+
+[output]
+    DEBUG pam_sm_authenticate: entry
+    DEBUG /^\(user %u\) rejecting password longer than [0-9]+$/
+    NOTICE authentication failure; logname=%u uid=%i euid=%i tty= ruser= rhost=
+    DEBUG pam_sm_authenticate: exit (failure)

tests/data/scripts/password/authtok-too-long

@@ -0,0 +1,17 @@
+# Test use_authtok with an excessively long password.  -*- conf -*-
+#
+# Copyright 2020 Russ Allbery <eagle@eyrie.org>
+#
+# SPDX-License-Identifier: BSD-3-clause or GPL-1+
+
+[options]
+    password = use_authtok
+
+[run]
+    chauthtok(PRELIM_CHECK)   = PAM_SUCCESS
+    chauthtok(UPDATE_AUTHTOK) = PAM_AUTHTOK_ERR
+
+[prompts]
+    echo_off = Current Kerberos password: |%p
+
+[output]

tests/data/scripts/password/authtok-too-long-debug

@@ -0,0 +1,23 @@
+# Test use_authtok with an excessively long password.  -*- conf -*-
+#
+# Copyright 2020 Russ Allbery <eagle@eyrie.org>
+#
+# SPDX-License-Identifier: BSD-3-clause or GPL-1+
+
+[options]
+    password = use_authtok debug
+
+[run]
+    chauthtok(PRELIM_CHECK)   = PAM_SUCCESS
+    chauthtok(UPDATE_AUTHTOK) = PAM_AUTHTOK_ERR
+
+[prompts]
+    echo_off = Current Kerberos password: |%p
+
+[output]
+    DEBUG pam_sm_chauthtok: entry (prelim)
+    DEBUG (user %u) attempting authentication as %0 for kadmin/changepw
+    DEBUG pam_sm_chauthtok: exit (success)
+    DEBUG pam_sm_chauthtok: entry (update)
+    DEBUG /^\(user %u\) rejecting password longer than [0-9]+$/
+    DEBUG pam_sm_chauthtok: exit (failure)

tests/data/scripts/password/too-long

@@ -0,0 +1,15 @@
+# Test password change to an excessively long password.  -*- conf -*-
+#
+# Copyright 2020 Russ Allbery <eagle@eyrie.org>
+#
+# SPDX-License-Identifier: BSD-3-clause or GPL-1+
+
+[run]
+    chauthtok(PRELIM_CHECK)   = PAM_SUCCESS
+    chauthtok(UPDATE_AUTHTOK) = PAM_AUTHTOK_ERR
+
+[prompts]
+    echo_off = Current Kerberos password: |%p
+    echo_off = Enter new Kerberos password: |%n
+
+[output]

tests/data/scripts/password/too-long-debug

@@ -0,0 +1,24 @@
+# Test password change to an excessively long password.  -*- conf -*-
+#
+# Copyright 2020 Russ Allbery <eagle@eyrie.org>
+#
+# SPDX-License-Identifier: BSD-3-clause or GPL-1+
+
+[options]
+    password = debug
+
+[run]
+    chauthtok(PRELIM_CHECK)   = PAM_SUCCESS
+    chauthtok(UPDATE_AUTHTOK) = PAM_AUTHTOK_ERR
+
+[prompts]
+    echo_off = Current Kerberos password: |%p
+    echo_off = Enter new Kerberos password: |%n
+
+[output]
+    DEBUG pam_sm_chauthtok: entry (prelim)
+    DEBUG (user %u) attempting authentication as %0 for kadmin/changepw
+    DEBUG pam_sm_chauthtok: exit (success)
+    DEBUG pam_sm_chauthtok: entry (update)
+    DEBUG /^\(user %u\) rejecting password longer than [0-9]+$/
+    DEBUG pam_sm_chauthtok: exit (failure)

tests/fakepam/config.c

@@ -108,6 +108,7 @@ static const struct {
     /* clang-format off */
     {"PAM_AUTH_ERR",         PAM_AUTH_ERR        },
     {"PAM_AUTHINFO_UNAVAIL", PAM_AUTHINFO_UNAVAIL},
+    {"PAM_AUTHTOK_ERR",      PAM_AUTHTOK_ERR     },
     {"PAM_IGNORE",           PAM_IGNORE          },
     {"PAM_NEW_AUTHTOK_REQD", PAM_NEW_AUTHTOK_REQD},
     {"PAM_SESSION_ERR",      PAM_SESSION_ERR     },

tests/module/basic-t.c

@@ -2,7 +2,8 @@
  * Basic tests for the pam-krb5 module.
  *
  * This test case includes all tests that can be done without having Kerberos
- * configured and a username and password available.
+ * configured and a username and password available, and without any special
+ * configuration.
  *
  * Written by Russ Allbery <eagle@eyrie.org>
  * Copyright 2020 Russ Allbery <eagle@eyrie.org>

tests/module/long-t.c

@@ -0,0 +1,46 @@
+/*
+ * Excessively long password tests for the pam-krb5 module.
+ *
+ * This test case includes all tests for excessively long passwords that can
+ * be done without having Kerberos configured and a username and password
+ * available.
+ *
+ * Copyright 2020 Russ Allbery <eagle@eyrie.org>
+ *
+ * SPDX-License-Identifier: BSD-3-clause or GPL-1+
+ */
+
+#include <config.h>
+#include <portable/system.h>
+
+#include <tests/fakepam/script.h>
+#include <tests/tap/basic.h>
+
+
+int
+main(void)
+{
+    struct script_config config;
+    char *password;
+
+    plan_lazy();
+
+    memset(&config, 0, sizeof(config));
+    config.user = "test";
+
+    /* Test a password that is too long. */
+    password = bcalloc_type(PAM_MAX_RESP_SIZE + 1, char);
+    memset(password, 'a', PAM_MAX_RESP_SIZE);
+    config.password = password;
+    run_script("data/scripts/long/password", &config);
+    run_script("data/scripts/long/password-debug", &config);
+
+    /* Test a stored authtok that's too long. */
+    config.authtok = password;
+    config.password = "testing";
+    run_script("data/scripts/long/use-first", &config);
+    run_script("data/scripts/long/use-first-debug", &config);
+
+    free(password);
+    return 0;
+}

tests/module/password-t.c

@@ -49,10 +49,28 @@ main(void)
 
     plan_lazy();
 
+    /*
+     * First test trying to change the password to something that's
+     * excessively long.
+     */
+    newpass = bcalloc_type(PAM_MAX_RESP_SIZE + 1, char);
+    memset(newpass, 'a', PAM_MAX_RESP_SIZE);
+    config.newpass = newpass;
+    run_script("data/scripts/password/too-long", &config);
+    run_script("data/scripts/password/too-long-debug", &config);
+
+    /* Test use_authtok with an excessively long password. */
+    config.newpass = NULL;
+    config.authtok = newpass;
+    run_script("data/scripts/password/authtok-too-long", &config);
+    run_script("data/scripts/password/authtok-too-long-debug", &config);
+
     /*
      * Change the password to something new.  This needs to be sufficiently
      * random that it's unlikely to fall afoul of password strength checking.
      */
+    free(newpass);
+    config.authtok = NULL;
     basprintf(&newpass, "ngh1,a%lu nn9af6%lu", (unsigned long) getpid(),
               (unsigned long) time(NULL));
     config.newpass = newpass;