Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20131209123945.GA16680@openwall.com>
Date: Mon, 9 Dec 2013 16:39:45 +0400
From: Solar Designer <solar@...nwall.com>
To: oss-security@...ts.openwall.com
Subject: Re: CVE request: pam: password hashes aren't compared case-sensitively

On Mon, Dec 09, 2013 at 03:21:39PM +0530, Ratul Gupta wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1038555
> 
> It was found that in pam_userdb module for Pam, password hashes weren't 
> compared case-sensitively, which could lead to acceptance of hashes for 
> completely different passwords, which shouldn't be accepted.
> 
> After hashing the user's password with crypt(), pam_userdb compares the 
> result to the stored hash case-insensitively with strncasecmp(), which 
> should be avoided, as it could result in an increased possibility of a 
> successful brute-force attack.

Ouch.  A funny bug.  I just took a look at the code:

https://git.fedorahosted.org/cgit/linux-pam.git/tree/modules/pam_userdb/pam_userdb.c

and here are some additional observations:

The limited length comparison is also slightly problematic.  In an older
revision of the code (e.g. in Linux-PAM-1.1.2), pam_userdb.c had:

	  if (data.dsize != 13) {
	    compare = -2;

which was later changed to:

	  if (data.dsize < 13) {
	    compare = -2;

I think that along with this change, a length comparison of the computed
vs. stored hash should have been added, but this was not done.  I think
the line:

	    if (cryptpw) {

should be changed to:

	    if (cryptpw && strlen(cryptpw) == (size_t)data.dsize) {

This is consistent with the approach pam_userdb already uses when
comparing plaintext passwords (ouch).  Speaking of which:

There are two code paths that compare plaintext passwords.  Luckily,
both check exact lengths and both only use case-insensitive comparison
if PAM_ICASE_ARG is set.  However, they are not constant time, so
correct passwords may possibly be determined remotely in linear rather
than exponential time (as function of password length):

http://rdist.root.org/2010/07/19/exploiting-remote-timing-attacks/
http://rdist.root.org/2010/08/05/optimized-memcmp-leaks-useful-timing-differences/
http://rdist.root.org/2010/11/09/blackhat-2010-video-on-remote-timing-attacks/

Additionally, the length comparison, while having it is crucial, leaks
even more timing information: it tells a remote attacker whether the
tested password length is correct or not.  The result of such comparison
could be obtained and merged into the final authentication outcome in a
constant time fashion, although doing so may be non-trivial (especially
given possible compiler optimizations).

Is repairing all of this for real worth the effort?  Repairing plaintext
password comparisons (so that they are not even weaker than they appear
at first) feels weird.  Can pam_userdb be dropped from Linux-PAM and
from distros?  I hope people aren't using it because it looks unsuitable
for use, but I'm probably wrong.

(The hash comparisons aren't constant time either, but with large enough
salts that are unpredictable by a remote attacker and that are stored
only along with the hashes, this is OK.  In practice not all of these
conditions might be met all the time, but anyhow this is a more general
topic that is not limited to pam_userdb, so let's not focus on it in
this context.)

Alexander

Powered by blists - more mailing lists

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.