CVE-2020-1722: prevent use of too long passwords

NIST SP 800-63-3B sets a recommendation to have password length upper bound limited in A.2:

https://pages.nist.gov/800-63-3/sp800-63b.html#appA

	Users should be encouraged to make their passwords as lengthy as they
	want, within reason. Since the size of a hashed password is independent
	of its length, there is no reason not to permit the use of lengthy
	passwords (or pass phrases) if the user wishes. Extremely long passwords
	(perhaps megabytes in length) could conceivably require excessive
	processing time to hash, so it is reasonable to have some limit.

FreeIPA already applied 256 characters limit for non-random passwords
set through ipa-getkeytab tool. The limit was not, however, enforced in
other places.

MIT Kerberos limits the length of the password to 1024 characters in its
tools. However, these tools (kpasswd and 'cpw' command of kadmin) do not
differentiate between a password larger than 1024 and a password of 1024
characters. As a result, longer passwords are silently cut off.

To prevent silent cut off for user passwords, use limit of 1000
characters.

Thus, this patch enforces common limit of 1000 characters everywhere:
 - LDAP-based password changes
   - LDAP password change control
   - LDAP ADD and MOD operations on clear-text userPassword
   - Keytab setting with ipa-getkeytab
 - Kerberos password setting and changing

Fixes: https://pagure.io/freeipa/issue/8268

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-by: Simo Sorce <ssorce@redhat.com>
Reviewed-By: Simo Sorce <ssorce@redhat.com>

client/ipa-getkeytab.c

@@ -562,6 +562,11 @@ done:
  * set match=true to enforce that the two entered passwords match.
  *
  * To prompt for an existing password provide prompt1 and set match=false.
+ *
+ * Implementation details:
+ * krb5_prompter_posix() does not differentiate between too long entry or
+ * an entry exactly the size of a buffer. Thus, allocate a bigger buffer
+ * and do the check for a too long password afterwards.
  */
 static char *ask_password(krb5_context krbctx, char *prompt1, char *prompt2,
                           bool match)
@@ -569,8 +574,10 @@ static char *ask_password(krb5_context krbctx, char *prompt1, char *prompt2,
     krb5_prompt ap_prompts[2];
     krb5_data k5d_pw0;
     krb5_data k5d_pw1;
-    char pw0[256];
-    char pw1[256];
+#define MAX(a,b) (((a)>(b))?(a):(b))
+#define PWD_BUFFER_SIZE MAX((IPAPWD_PASSWORD_MAX_LEN + 2), 1024)
+    char pw0[PWD_BUFFER_SIZE];
+    char pw1[PWD_BUFFER_SIZE];
     char *password;
     int num_prompts = match ? 2:1;
 
@@ -593,7 +600,12 @@ static char *ask_password(krb5_context krbctx, char *prompt1, char *prompt2,
                 num_prompts, ap_prompts);
 
     if (match && (strcmp(pw0, pw1))) {
-        fprintf(stderr, _("Passwords do not match!"));
+        fprintf(stderr, _("Passwords do not match!\n"));
+        return NULL;
+    }
+
+    if (k5d_pw0.length > IPAPWD_PASSWORD_MAX_LEN) {
+        fprintf(stderr, "%s\n", ipapwd_password_max_len_errmsg);
         return NULL;
     }
 
@@ -998,6 +1010,7 @@ int main(int argc, const char *argv[])
             }
 
             fprintf(stderr, _("Failed to create key material\n"));
+            free_keys_contents(krbctx, &keys);
             exit(8);
         }
 

client/man/ipa-getkeytab.1

@@ -93,7 +93,7 @@ AES\-256 CTS mode with 192\-bit SHA\-384 HMAC
 ArcFour with HMAC/md5
 .TP
 \fB\-P, \-\-password\fR
-Use this password for the key instead of one randomly generated.
+Use this password for the key instead of one randomly generated. The length of the password is limited by 1024 characters. Note that MIT Kerberos also limits passwords entered through kpasswd and kadmin commands to the same length.
 .TP
 \fB\-D, \-\-binddn\fR
 The LDAP DN to bind as when retrieving a keytab without Kerberos credentials. Generally used with the \fB\-w\fR or \fB\-W\fR options.

daemons/ipa-kdb/ipa_kdb_passwords.c

@@ -80,6 +80,12 @@ static krb5_error_code ipadb_check_pw_policy(krb5_context context,
         return EINVAL;
     }
 
+    if (strlen(passwd) > IPAPWD_PASSWORD_MAX_LEN) {
+        krb5_set_error_message(context, E2BIG, "%s",
+                               ipapwd_password_max_len_errmsg);
+        return E2BIG;
+    }
+
     ied->passwd = strdup(passwd);
     if (!ied->passwd) {
         return ENOMEM;

daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c

@@ -1087,3 +1087,12 @@ void free_ipapwd_krbcfg(struct ipapwd_krbcfg **cfg)
     *cfg = NULL;
 };
 
+int ipapwd_check_max_pwd_len(size_t len, char **errMesg) {
+    if (len > IPAPWD_PASSWORD_MAX_LEN) {
+        LOG("%s\n", ipapwd_password_max_len_errmsg);
+        *errMesg = ipapwd_password_max_len_errmsg;
+        return LDAP_CONSTRAINT_VIOLATION;
+    }
+    return 0;
+}
+

daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c

@@ -334,6 +334,11 @@ parse_req_done:
 		goto free_and_return;
 	}
 
+        rc = ipapwd_check_max_pwd_len(strlen(newPasswd), &errMesg);
+        if (rc) {
+            goto free_and_return;
+        }
+
 	if (oldPasswd == NULL || *oldPasswd == '\0') {
 		/* If user is authenticated, they already gave their password during
 		the bind operation (or used sasl or client cert auth or OS creds) */
@@ -1684,6 +1689,14 @@ static int ipapwd_getkeytab(Slapi_PBlock *pb, struct ipapwd_krbcfg *krbcfg)
 
     } else {
 
+        if (password != NULL) {
+            /* if password was passed-in, check its length */
+            rc = ipapwd_check_max_pwd_len(strlen(password), &err_msg);
+            if (rc) {
+                goto free_and_return;
+            }
+	}
+
         /* check if we are allowed to *write* keys */
         acl_ok = is_allowed_to_access_attr(pb, bind_dn, target_entry,
                                            WRITEKEYS_OP_CHECK, NULL,

daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h

@@ -133,6 +133,7 @@ int ipapwd_set_extradata(const char *dn,
                          time_t unixtime);
 void ipapwd_free_slapi_value_array(Slapi_Value ***svals);
 void free_ipapwd_krbcfg(struct ipapwd_krbcfg **cfg);
+int ipapwd_check_max_pwd_len(size_t len, char **errMesg);
 
 /* from encoding.c */
 struct ipapwd_keyset {

daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c

@@ -278,6 +278,10 @@ static int ipapwd_pre_add(Slapi_PBlock *pb)
                 rc = LDAP_CONSTRAINT_VIOLATION;
                 slapi_ch_free_string(&userpw);
             } else {
+                rc = ipapwd_check_max_pwd_len(strlen(userpw_clear), &errMesg);
+                if (rc) {
+                    goto done;
+                }
                 userpw = slapi_ch_strdup(userpw_clear);
             }
 
@@ -561,6 +565,11 @@ static int ipapwd_pre_mod(Slapi_PBlock *pb)
                 goto done;
             }
             bv = lmod->mod_bvalues[0];
+
+            rc = ipapwd_check_max_pwd_len(bv->bv_len, &errMesg);
+            if (rc) {
+                goto done;
+            }
             slapi_ch_free_string(&unhashedpw);
             unhashedpw = slapi_ch_malloc(bv->bv_len+1);
             if (!unhashedpw) {
@@ -783,7 +792,12 @@ static int ipapwd_pre_mod(Slapi_PBlock *pb)
     if (! unhashedpw && (gen_krb_keys || is_smb || is_ipant)) {
         if ((userpw != NULL) && ('{' == userpw[0])) {
             if (0 == strncasecmp(userpw, "{CLEAR}", strlen("{CLEAR}"))) {
-                unhashedpw = slapi_ch_strdup(&userpw[strlen("{CLEAR}")]);
+                const char *userpw_clear = &userpw[strlen("{CLEAR}")];
+                rc = ipapwd_check_max_pwd_len(strlen(userpw_clear), &errMesg);
+                if (rc) {
+                    goto done;
+                }
+                unhashedpw = slapi_ch_strdup(userpw_clear);
                 if (NULL == unhashedpw) {
                     LOG_OOM();
                     rc = LDAP_OPERATIONS_ERROR;
@@ -1419,6 +1433,8 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
     time_t expire_time;
     char *principal_expire = NULL;
     struct tm expire_tm;
+    int rc = LDAP_INVALID_CREDENTIALS;
+    char *errMesg = NULL;
 
     /* get BIND parameters */
     ret |= slapi_pblock_get(pb, SLAPI_BIND_TARGET_SDN, &target_sdn);
@@ -1480,8 +1496,14 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
         goto invalid_creds;
 
     /* Ensure that there is a password. */
-    if (credentials->bv_len == 0)
+    if (credentials->bv_len == 0) {
         goto invalid_creds;
+    } else {
+        rc = ipapwd_check_max_pwd_len(credentials->bv_len, &errMesg);
+        if (rc) {
+            goto invalid_creds;
+        }
+    }
 
     /* Authenticate the user. */
     ret = ipapwd_authenticate(dn, entry, credentials);
@@ -1505,8 +1527,7 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb)
 invalid_creds:
     slapi_entry_free(entry);
     slapi_sdn_free(&sdn);
-    slapi_send_ldap_result(pb, LDAP_INVALID_CREDENTIALS,
-                           NULL, NULL, 0, NULL);
+    slapi_send_ldap_result(pb, rc, NULL, errMesg, 0, NULL);
     return 1;
 }
 

ipatests/test_integration/test_commands.py

@@ -35,7 +35,7 @@ from ipatests.pytest_ipa.integration import tasks
 from ipaplatform.tasks import tasks as platform_tasks
 from ipatests.create_external_ca import ExternalCA
 from ipatests.test_ipalib.test_x509 import good_pkcs7, badcert
-from ipapython.ipautil import realm_to_suffix
+from ipapython.ipautil import realm_to_suffix, ipa_generate_password
 
 logger = logging.getLogger(__name__)
 
@@ -445,6 +445,84 @@ class TestIPACommand(IntegrationTest):
         except CalledProcessError:
             pytest.fail("Password change failed when it should not")
 
+    def test_huge_password(self):
+        user = 'toolonguser'
+        hostname = 'toolong.{}'.format(self.master.domain.name)
+        huge_password = ipa_generate_password(min_len=1536)
+        original_passwd = 'Secret123'
+        master = self.master
+        base_dn = str(master.domain.basedn)
+
+        # Create a user with a password that is too long
+        tasks.kinit_admin(master)
+        add_password_stdin_text = "{pwd}\n{pwd}".format(pwd=huge_password)
+        result = master.run_command(['ipa', 'user-add', user,
+                                     '--first', user,
+                                     '--last', user,
+                                     '--password'],
+                                    stdin_text=add_password_stdin_text,
+                                    raiseonerr=False)
+        assert result.returncode != 0
+
+        # Try again with a normal password
+        add_password_stdin_text = "{pwd}\n{pwd}".format(pwd=original_passwd)
+        master.run_command(['ipa', 'user-add', user,
+                            '--first', user,
+                            '--last', user,
+                            '--password'],
+                           stdin_text=add_password_stdin_text)
+
+        # kinit as that user in order to modify the pwd
+        user_kinit_stdin_text = "{old}\n%{new}\n%{new}\n".format(
+            old=original_passwd,
+            new=original_passwd)
+        master.run_command(['kinit', user], stdin_text=user_kinit_stdin_text)
+        # sleep 1 sec (krblastpwdchange and krbpasswordexpiration have at most
+        # a 1s precision)
+        time.sleep(1)
+        # perform ldapmodify on userpassword as dir mgr
+        entry_ldif = textwrap.dedent("""
+            dn: uid={user},cn=users,cn=accounts,{base_dn}
+            changetype: modify
+            replace: userpassword
+            userpassword: {new_passwd}
+        """).format(
+            user=user,
+            base_dn=base_dn,
+            new_passwd=huge_password)
+
+        result = tasks.ldapmodify_dm(master, entry_ldif, raiseonerr=False)
+        assert result.returncode != 0
+
+        # ask_password in ipa-getkeytab will complain about too long password
+        keytab_file = os.path.join(self.master.config.test_dir,
+                                   'user.keytab')
+        password_stdin_text = "{pwd}\n{pwd}".format(pwd=huge_password)
+        result = self.master.run_command(['ipa-getkeytab',
+                                          '-p', user,
+                                          '-P',
+                                          '-k', keytab_file,
+                                          '-s', self.master.hostname],
+                                         stdin_text=password_stdin_text,
+                                         raiseonerr=False)
+        assert result.returncode != 0
+        assert "clear-text password is too long" in result.stderr_text
+
+        # Create a host with a user-set OTP that is too long
+        tasks.kinit_admin(master)
+        result = master.run_command(['ipa', 'host-add', '--force',
+                                     hostname,
+                                     '--password', huge_password],
+                                    raiseonerr=False)
+        assert result.returncode != 0
+
+        # Try again with a valid password
+        result = master.run_command(['ipa', 'host-add', '--force',
+                                     hostname,
+                                     '--password', original_passwd],
+                                    raiseonerr=False)
+        assert result.returncode == 0
+
     def test_change_selinuxusermaporder(self):
         """
         An update file meant to ensure a more sane default was

util/ipa_krb5.c

@@ -31,6 +31,13 @@
 
 #include "ipa_krb5.h"
 
+#define TOSTR(x) STR(x)
+#define STR(x) #x
+const char *ipapwd_password_max_len_errmsg = \
+    "clear-text password is too long (max " \
+    TOSTR(IPAPWD_PASSWORD_MAX_LEN) \
+    " chars)!";
+
 /* Salt types */
 #define KRB5P_SALT_SIZE 16
 
@@ -125,6 +132,13 @@ krb5_error_code ipa_krb5_generate_key_data(krb5_context krbctx,
     int num_keys;
     int i;
 
+    if ((pwd.data != NULL) && (pwd.length > IPAPWD_PASSWORD_MAX_LEN)) {
+        kerr = E2BIG;
+        krb5_set_error_message(krbctx, kerr, "%s",
+                               ipapwd_password_max_len_errmsg);
+        return kerr;
+    }
+
     num_keys = num_encsalts;
     keys = calloc(num_keys, sizeof(krb5_key_data));
     if (!keys) {
@@ -970,6 +984,10 @@ int create_keys(krb5_context krbctx,
     if (password) {
         key_password.data = password;
         key_password.length = strlen(password);
+        if (key_password.length > IPAPWD_PASSWORD_MAX_LEN) {
+            *err_msg = _("Password is too long!\n");
+            return 0;
+        }
 
         realm = krb5_princ_realm(krbctx, princ);
     }

util/ipa_krb5.h

@@ -31,6 +31,9 @@ struct keys_container {
 #define KEYTAB_RET_OID "2.16.840.1.113730.3.8.10.2"
 #define KEYTAB_GET_OID "2.16.840.1.113730.3.8.10.5"
 
+#define IPAPWD_PASSWORD_MAX_LEN 1000
+extern const char *ipapwd_password_max_len_errmsg;
+
 int krb5_klog_syslog(int, const char *, ...);
 
 void