Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from

Conversation

abbra
Copy link
Contributor

@abbra abbra commented Apr 14, 2020

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

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

@abbra abbra added the ack Pull Request approved, can be merged label Apr 14, 2020
@abbra
Copy link
Contributor Author

abbra commented Apr 14, 2020

ACK was added because this fix was pre-approved during several offline review sessions.

@abbra abbra force-pushed the fix-CVE-2020-1722 branch 2 times, most recently from 5040a97 to 339aa09 Compare April 14, 2020 06:09
@abbra abbra added ipa-4-6 Mark for backport to ipa 4.6 ipa-4-8 Mark for backport to ipa 4.8 labels Apr 14, 2020
@kaleemsiddiqu
Copy link
Contributor

@abbra
Should we add tests for mod*(user-mod, host-mod) operations as well with long password?

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>
@abbra
Copy link
Contributor Author

abbra commented Apr 14, 2020

Forgot to update the test after the error message was changed to include 'clear-text password is too long'.

@abbra
Copy link
Contributor Author

abbra commented Apr 14, 2020

Should we add tests for mod*(user-mod, host-mod) operations as well with long password?

no, we don't need them because they are doing LDAP MODIFY operations and those tested already. The test suite covers all actual ways of modifying passwords from the LDAP server and KDC server perspectives.

@abbra abbra added the pushed Pull Request has already been pushed label Apr 14, 2020
@abbra
Copy link
Contributor Author

abbra commented Apr 14, 2020

master:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged ipa-4-6 Mark for backport to ipa 4.6 ipa-4-8 Mark for backport to ipa 4.8 pushed Pull Request has already been pushed
Projects
None yet
2 participants