From a8611e205bfe7b7538523ec492069987f5d7de64 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy@redhat.com>
Date: Wed, 8 Apr 2020 15:00:38 +0300
Subject: [PATCH] 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 | 19 ++++-
client/man/ipa-getkeytab.1 | 2 +-
daemons/ipa-kdb/ipa_kdb_passwords.c | 6 ++
.../ipa-slapi-plugins/ipa-pwd-extop/common.c | 9 +++
.../ipa-pwd-extop/ipa_pwd_extop.c | 13 +++
.../ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h | 1 +
.../ipa-slapi-plugins/ipa-pwd-extop/prepost.c | 29 ++++++-
ipatests/test_integration/test_commands.py | 79 +++++++++++++++++++
util/ipa_krb5.c | 18 +++++
util/ipa_krb5.h | 3 +
10 files changed, 171 insertions(+), 8 deletions(-)
diff --git a/client/ipa-getkeytab.c b/client/ipa-getkeytab.c
index 8a5e98bed1947344247f9d6146e595d5f7f7a963..b174093d3762f8a6bfa27045bed393c2cd422fe0 100644
--- a/client/ipa-getkeytab.c
+++ b/client/ipa-getkeytab.c
@@ -633,6 +633,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)
@@ -640,8 +645,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;
@@ -664,7 +671,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;
}
@@ -1017,6 +1029,7 @@ int main(int argc, const char *argv[])
}
fprintf(stderr, _("Failed to create key material\n"));
+ free_keys_contents(krbctx, &keys);
exit(8);
}
diff --git a/client/man/ipa-getkeytab.1 b/client/man/ipa-getkeytab.1
index 6e7fdf39ee4e28772365edafd4c7e86d0c37d343..21ba651c4ac78d09bc57d498b38591fdbfd1d151 100644
--- a/client/man/ipa-getkeytab.1
+++ b/client/man/ipa-getkeytab.1
@@ -95,7 +95,7 @@ DES cbc mode with RSA\-MD5
DES cbc mode with RSA\-MD4
.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.
diff --git a/daemons/ipa-kdb/ipa_kdb_passwords.c b/daemons/ipa-kdb/ipa_kdb_passwords.c
index a3d4fe2436da60d081040754780d3e815acb1473..9362f4305d9973004a8c890540b5fa1622de772b 100644
--- a/daemons/ipa-kdb/ipa_kdb_passwords.c
+++ b/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;
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
index ba5c54e58e9b0b5dcc657d88c530c237e321495c..716b71333050f1d05063289f9890918b86ddb108 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
+++ b/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;
+}
+
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
index f92706810d875cc6c7d8bc7a676c13ecc5d50e54..be413742cd2d54ab8bc7c51e6600b3dbbd26cec7 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
@@ -318,6 +318,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) */
@@ -1661,6 +1666,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,
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h
index 31c76b3f1a3854a5126bf6c7bbb9bf7b3bcf02e7..5a49fa7e6c787f15b641da794ec5ee3e7a525292 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h
+++ b/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 {
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
index 001f615ecdb87ac62fe237d5d9a932f0292c2e24..04cd2b10f3ba4375e6a278afe87cbd9d257d528f 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
+++ b/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);
}
@@ -560,6 +564,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) {
@@ -782,7 +791,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;
@@ -1416,6 +1430,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);
@@ -1477,8 +1493,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);
@@ -1502,8 +1524,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;
}
diff --git a/ipatests/test_integration/test_commands.py b/ipatests/test_integration/test_commands.py
index a14a324ec2db26400aa67d2fc61f9c30b9b1d045..715a1f1a8f4105a470cc6f205a6bb9bc9db030e0 100644
--- a/ipatests/test_integration/test_commands.py
+++ b/ipatests/test_integration/test_commands.py
@@ -33,6 +33,7 @@ from ipatests.test_integration.base import IntegrationTest
from ipatests.pytest_ipa.integration import tasks
from ipaplatform.tasks import tasks as platform_tasks
from ipatests.pytest_ipa.integration.create_external_ca import ExternalCA
+from ipapython.ipautil import ipa_generate_password
logger = logging.getLogger(__name__)
@@ -337,6 +338,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) # pylint: disable=no-member
+
+ # 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
diff --git a/util/ipa_krb5.c b/util/ipa_krb5.c
index c09c3daa505655f2e5292a79c03683faa75ad244..1ba6d25eecb27935ffb14923015f08745aad20fe 100644
--- a/util/ipa_krb5.c
+++ b/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);
}
diff --git a/util/ipa_krb5.h b/util/ipa_krb5.h
index b039c1a7f3d0bc215376f8f1dd2ac93e75a0c626..8392a85b6740ece1ba7085a4733ea0f2f6b1fe64 100644
--- a/util/ipa_krb5.h
+++ b/util/ipa_krb5.h
@@ -30,6 +30,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
--
2.25.2