A hole in crypt_blowfish
A longstanding bug that was recently found in the crypt_blowfish password hashing library highlights the problems that can occur when a bug is found in a widely used low-level library. Because crypt_blowfish has been around for so long (this bug is said to go back to 1998 or possibly 1997), it has been used by various other packages (PHP for example) as well as some Linux distributions. The security impact is not likely to be huge, because it only affects passwords with somewhat uncommon characteristics, but the impact on those who have stored hashed passwords generated using the library may be a bit more painful.
Password hashing is a standard technique that is used by authentication mechanisms (for logging into a system or web application for example), so that the plaintext password does not need to be stored. Instead, something derived from the plaintext password is stored. Typically, one-way cryptographic hash functions like SHA-1 or the Blowfish cipher are used for that purpose. When the user presents the password, the same function is applied to that input and compared to what is stored. The idea is that even if an attacker gets access to the password database, they would need to "crack" the hashed values stored there. As its name implies, crypt_blowfish implements password hashing based on Blowfish.
The bug itself is quite simple, and the change to fix it will likely make the problem obvious to C programmers:
tmp |= *ptr;to:
tmp |= (unsigned char)*ptr;
The problem was actually discovered in the John the Ripper (JtR) password cracking program. As part of creating a test suite, "magnum" was attempting to crack a password that consisted of a single non-ASCII character (£ or 0xa3), which was hashed using a library other than crypt_blowfish. Because JtR and crypt_blowfish share their implementation of the Blowfish encryption algorithm, tests using both would pass, but independent implementations would generate the correct hash, thus failing to match what was generated by JtR.
JtR and crypt_blowfish developer Alexander Peslyak (aka Solar Designer) analyzed the effects of the bug and found that some password pairs would hash to the same value with only minimal differences (e.g. "ab£" hashed to the same value as "£"), which would make password cracking easier. A further analysis shows that some characters appearing just before one with the high bit set may be effectively ignored when calculating the hash. That would mean that a simpler password than that given by the user could be used and would still be considered valid—a significant weakening of the user's password.
It should be noted that Solar Designer has been very forthcoming with details of the problem and its effects. His CVE request is quite detailed, and he takes full responsibility for the error. Other projects who find security holes in their code would do well to follow his lead here.
There is no real problem for JtR, as it can get the updated code so that it can crack passwords with high-bit-set characters in them. In addition, it could continue to use the broken code to crack passwords that were hashed incorrectly. But for applications that use crypt_blowfish, it's a different story. Passwords with the high bit set may be fairly uncommon—at least for sites with typically ASCII-only users—but there is no easy way to determine whether stored hashes are invalid or not.
The safest solution for administrators with potentially bad password hashes (which could include those running Openwall Linux (Owl), SUSE, and ALT Linux which can use crypt_blowfish for hashing the password database) is to invalidate all passwords and have their users input new ones. That could prove to be a logistical nightmare, however, depending on how easily, and securely, users can set new passwords without authenticating with their existing password. Requiring that users change their existing passwords after logging in is an alternative, but one that might give attackers a window in which to operate. It also leaves passwords unchanged for any users that do not log in.
The risks of attackers using this flaw to log in to a site are likely to be fairly small, but they do exist. If a site's login process is susceptible to brute force attacks, this bug makes it somewhat easier to do so, at least for specific password types. On the other hand, if the password database has been exposed, and some passwords were not crackable (at least for attackers using cracking programs other than JtR), this information would give them the means to crack those passwords. In the end analysis, it is an exploitable hole, but not the kind to send administrators into panic mode.
It is somewhat amazing that this bug has existed for 13+ years in a fairly widely used library. Up until now, no one has tested it in this way, at least publicly, which should serve as something of a warning to those who are using well-established software, both free and proprietary. With free software (crypt_blowfish has been placed into the public domain which may be a bit legally murky but is in keeping with free software ideals) there are more and easier opportunities to examine and test the code, but that's only effective if the testing is actually done.
There are, without doubt, bugs still lurking in various security-oriented libraries that we depend on every day, so testing those libraries (as well as code that is not being used for security purposes) regularly and systematically can only help find them. While it took more than a decade for this bug to come to light, it's worth pointing out that it certainly could have been discovered by others in the interim. Attackers clearly do their own testing, and are generally not inclined to release their results. That may not be the case here, but one should always recognize that public disclosure of a vulnerability is not necessarily tied to its discovery.
Index entries for this article | |
---|---|
Security | Encryption |
Security | Password hashing |
Posted Jun 22, 2011 19:28 UTC (Wed)
by martinfick (subscriber, #4455)
[Link] (10 responses)
Seems like another alternative would be to simply lazily update the stored hash on login since the unencrypted password is available then. This way users would not be forced to change their passwords.
Posted Jun 22, 2011 20:01 UTC (Wed)
by mjthayer (guest, #39183)
[Link] (9 responses)
Posted Jun 22, 2011 20:27 UTC (Wed)
by dark (guest, #8483)
[Link] (8 responses)
The users will have to get their passwords changed, but this will be limited to users who were affected by the bug, and there's no window of opportunity for attackers.
One would hope this approach is coupled with a plan to phase out the restriction, though :)
Posted Jun 22, 2011 22:18 UTC (Wed)
by bgilbert (subscriber, #4738)
[Link] (7 responses)
Posted Jun 23, 2011 4:39 UTC (Thu)
by nirbheek (subscriber, #54111)
[Link] (6 responses)
Posted Jun 23, 2011 4:41 UTC (Thu)
by nirbheek (subscriber, #54111)
[Link]
Posted Jun 23, 2011 10:18 UTC (Thu)
by job (guest, #670)
[Link] (4 responses)
Posted Jun 23, 2011 21:15 UTC (Thu)
by solardiz (guest, #35993)
[Link] (3 responses)
Also, see comments by iabervon in here. And discussion on how to make it possible to upgrade transparently or safely: http://www.openwall.com/lists/oss-security/2011/06/23/3
Posted Jun 23, 2011 23:44 UTC (Thu)
by dlang (guest, #313)
[Link] (2 responses)
the fact that there are alternate inputs isn't the problem.
a hash is only considered broken if you can predict what inputs will produce a particular output.
In this case, that is exactly what happens, this bug means that someone can test far fewer inputs when trying to find one that matches the output, because you can predict that a large number of inputs will all produce the same output, and therefor only test one of them.
Posted Jun 24, 2011 0:08 UTC (Fri)
by solardiz (guest, #35993)
[Link] (1 responses)
Posted Jun 26, 2011 11:28 UTC (Sun)
by job (guest, #670)
[Link]
Posted Jun 22, 2011 20:15 UTC (Wed)
by iabervon (subscriber, #722)
[Link] (3 responses)
Posted Jun 22, 2011 21:22 UTC (Wed)
by jzbiciak (guest, #5246)
[Link] (2 responses)
I'm not sure I follow. Suppose my password was "ab£", as given in the article. With this bug, someone could log in as me with simply "£", or "xy£". If you have a corrected library and are generating fresh hashes, no prohibition on char 255 or any high-bit-set character is necessary. If you're trying to let in folks with high-bit-set characters and old hashes (so that they can fix their passwords), the char 255 restriction achieves nothing. What was your goal again?
Posted Jun 22, 2011 22:20 UTC (Wed)
by iabervon (subscriber, #722)
[Link] (1 responses)
If you had set your password to "ab£", the hash actually stored would be the proper hash of "ÿÿ£". If an attacker typed "xy£" or simply "£", the hash they got would also be the proper hash of "ÿÿ£". If the code is fixed, the only way to type your password would be "ÿÿ£", but this would work for any of the other passwords saved with the buggy code as well, so those passwords are still weak. However, when the bug is fixed, the only way to type any password whose saved hash was weakened by the bug is to use "ÿ". So it would invalidate weak passwords if using "ÿ" became impossible, either by simply rejecting it or by replacing it with some other value.
Posted Jun 22, 2011 22:44 UTC (Wed)
by jzbiciak (guest, #5246)
[Link]
That leaves the process of "how do you securely reset the affected users' passwords" as an exercise to be solved by the reader. ;-)
Posted Jun 22, 2011 20:33 UTC (Wed)
by eru (subscriber, #2753)
[Link] (12 responses)
I assume ptr is a char*, in which case this is not completely correct: The unadorned char actually has implementation-defined signedness. Most C implementations make it signed, but there are some that don't, and they are quite legal. This bug would not be present if compiled with them.
This almost-but-not-quite-always signedness of char is one of the stupidest features of C, contributing to a quite a lot of bugs (including some in my own code as well). I really hate it.
Posted Jun 22, 2011 20:47 UTC (Wed)
by daney (guest, #24551)
[Link] (6 responses)
#include <stdint.h>
And use uint8_t (or int8_t) instead of char.
Posted Jun 22, 2011 21:49 UTC (Wed)
by samroberts (subscriber, #46749)
[Link] (5 responses)
Posted Jun 22, 2011 22:19 UTC (Wed)
by dgm (subscriber, #49227)
[Link] (4 responses)
So, if you plan to do arithmetic on characters, better cast them to int or unsigned char, or whatever type has the properties you need for your operations.
Posted Jun 23, 2011 4:18 UTC (Thu)
by eru (subscriber, #2753)
[Link] (2 responses)
Not just arithmetic. Even indexing an array with a char value will bite the unwary.
Posted Jun 23, 2011 8:48 UTC (Thu)
by kris.shannon (subscriber, #45828)
[Link]
Posted Jun 23, 2011 12:13 UTC (Thu)
by dgm (subscriber, #49227)
[Link]
Posted Jun 23, 2011 15:27 UTC (Thu)
by jani (subscriber, #74547)
[Link]
"In all cases c is an int, the value of which must be representable as an unsigned char or must equal the value of the macro EOF. If the argument has any other value, the behaviour is undefined."
Posted Jun 22, 2011 22:22 UTC (Wed)
by vapier (guest, #15768)
[Link]
$ grep DEFAULT_SIGNED_CHAR gcc/config/*/*.h | grep -c 0
e.g. mips is generally a 1, but mips/irix is 0
long story short, if you want a signed char, then you better use "signed char"
Posted Jun 23, 2011 15:17 UTC (Thu)
by HelloWorld (guest, #56129)
[Link]
Posted Jun 24, 2011 12:38 UTC (Fri)
by wookey (guest, #5501)
[Link]
Posted Jun 28, 2011 10:30 UTC (Tue)
by lacos (guest, #70616)
[Link] (1 responses)
paragraph 2:
----v----
The real fuckup here is that "ptr" was declared pointer-to-char, instead of pointer-to-char-unsigned. "char unsigned" is the type to access binary data or the object representation of objects.
"char" (which is a different type from "char signed", but may have identical value representation) does not even have to be able to represent more than 255 (NOT 256!) distinct values, if we rely on nothing else than the C89 standard. This accomodates sign-magnitude and one's complement representations. See C89 "5.2.4.2.1 Sizes of integral types <limits.h>":
----v----
- number of bits for smallest object that is not a bit-field (byte)
- minimum value for an object of type signed char
- maximum value for an object of type signed char
- maximum value for an object of type unsigned char
- minimum value for an object of type char
- maximum value for an object of type char
[...]
If the value of an object of type char is treated as a signed integer when used in an expression, the value of CHAR_MIN shall be the same as that of SCHAR_MIN and the value of CHAR_MAX shall be the same as that of SCHAR_MAX. Otherwise, the value of CHAR_MIN shall be 0 and the value of CHAR_MAX shall be the same as that of UCHAR_MAX.
"char *" is there so you can work with _TEXT_ that consists of elements of the (basic or extended) execution character set. "char unsigned *" is there for everything else "binary". If in doubt, use "char unsigned *".
Another example: reading something from a socket and using string functions (like strstr(), strcmp() etc) on the result is _broken_, from a portability aspect.
Posted Jun 29, 2011 6:34 UTC (Wed)
by eru (subscriber, #2753)
[Link]
unsigned char by the way is a fairly "new" invention. The original K&R C did not have it, just the char with implementation-dependent signedness. Therefore there probably still is legacy code around that has bugs like this because char was the only way to represent a byte, and the coders did not remember to put masking around all widening accesses.
Posted Jun 22, 2011 21:07 UTC (Wed)
by jonabbey (guest, #2736)
[Link] (1 responses)
Posted Jun 22, 2011 21:17 UTC (Wed)
by jonabbey (guest, #2736)
[Link]
Posted Jun 22, 2011 22:04 UTC (Wed)
by jengelh (guest, #33263)
[Link]
Posted Jun 23, 2011 5:57 UTC (Thu)
by Cato (guest, #7643)
[Link] (3 responses)
This makes the bug much more serious for such users than in locales where only a few accented characters are normally used - anyone setting a Cyrillic-only password in KOI8-R may find their password maps to the same hash as a huge class of other Cyrillic-only passwords.
Posted Jun 24, 2011 0:18 UTC (Fri)
by solardiz (guest, #35993)
[Link] (2 responses)
Some excerpts:
"Lengths that are _not_ at risk: 1, 2, 4, 6, 8, 10, 12, 14, 16, 18.
For 97946 different Russian words, I got "70890 (72%) and 97213 (99%) unique hashes for koi8-r and utf-8, respectively."
"For koi8-r, 22 hashes are seen over 100 times each, with the top one being seen 190 times. For utf-8, the top hash (most common) is seen 4 times, then 84 hashes are seen 3 times each.
Thus, obviously the bug does cause collisions. There are not as many of those as some people might expect for nearly purely 8-bit inputs. Yet the very common hashes for koi8-r are worrisome. Even though if one were to run the entire koi8-r wordlist against a bunch of hashes they'd only achieve a 30% speedup due to the bug, if they focus on words producing 22 top hashes - so they only try 22 words - they'd crack around 3% of passwords based on randomly picked words from that list (assuming uniform distribution of random word numbers). For utf-8, this risk is much lower: trying top 85 passwords (0.087% of candidates) effectively tests 256 of them (0.26%)."
Posted Jun 24, 2011 7:48 UTC (Fri)
by Cato (guest, #7643)
[Link] (1 responses)
Posted Jun 24, 2011 16:17 UTC (Fri)
by kozerog (guest, #75519)
[Link]
Posted Jun 23, 2011 10:53 UTC (Thu)
by PaXTeam (guest, #24616)
[Link] (2 responses)
Posted Jun 23, 2011 15:34 UTC (Thu)
by clugstj (subscriber, #4020)
[Link] (1 responses)
Running any kernel that you've just downloaded form kernel.org is inherently risky - we all know that (or should). That's why we have Debian/RedHat/SuSE/etc.
Linus and friends are not your personal security risk assessment team.
Posted Jun 23, 2011 20:42 UTC (Thu)
by PaXTeam (guest, #24616)
[Link]
Posted Jun 23, 2011 14:05 UTC (Thu)
by felixfix (subscriber, #242)
[Link] (3 responses)
Posted Jun 23, 2011 18:16 UTC (Thu)
by jeremiah (subscriber, #1221)
[Link] (2 responses)
Posted Jun 23, 2011 18:37 UTC (Thu)
by felixfix (subscriber, #242)
[Link] (1 responses)
Posted Jun 24, 2011 12:52 UTC (Fri)
by jeremiah (subscriber, #1221)
[Link]
Posted Jun 23, 2011 14:06 UTC (Thu)
by proski (subscriber, #104)
[Link]
Posted Jun 23, 2011 17:55 UTC (Thu)
by zooko (guest, #2589)
[Link] (2 responses)
Posted Jun 23, 2011 20:52 UTC (Thu)
by solardiz (guest, #35993)
[Link] (1 responses)
There were unit tests. crypt_blowfish had "make check" and even "make check_threads" (for thread-safety testing) for years. It's just that its test vectors were limited to more typical passwords, without 8-bit characters in them. The only somewhat unusual test vectors were an empty string and a 72-character string (maximum supported by this hashing method). The rest were more typical for passwords. And no 8-bit chars in any of them.
Similarly, John the Ripper tested its bcrypt implementation each time it was run on hashes of this type. And it used the same limited set of test vectors.
Both have now been corrected to include 8-bit test vectors, and crypt_blowfish to do a quick self-test every time it's called to hash a password.
BTW, I think the same lack of 8-bit test vectors applies to SHA-crypt. Anyone wants to fix that?
Posted Jun 24, 2011 8:17 UTC (Fri)
by tialaramex (subscriber, #21167)
[Link]
That's the first thing I worried about when I saw this bug reported.
Posted Jun 23, 2011 23:37 UTC (Thu)
by gdt (subscriber, #6284)
[Link]
This is the second widespread bug where it would have been very useful to systems administrators to store the name and version of programs and libraries alongside the key material that software created.
Posted Jun 24, 2011 11:07 UTC (Fri)
by copsewood (subscriber, #199)
[Link] (4 responses)
In the end I have to strike a balance, between a system which is possible for my users (some of them retired and very recent computer users) to learn and use, and one which is secure under all circumstances. I don't think I can achieve both. Another improvement is not allowing users to choose their own passwords. If they want to change their password generate a new random one for them.
Posted Jun 24, 2011 14:37 UTC (Fri)
by solardiz (guest, #35993)
[Link] (3 responses)
Posted Jun 27, 2011 15:50 UTC (Mon)
by nye (subscriber, #51576)
[Link] (2 responses)
I think the opposite of this. Source limiting is like saying "you drove here via the A32? The last guy who did that was a thief so you're barred".
A lot of people share an IP address, especially on mobile connections or large institutions (nice way to lock out 1000 students at once, or everyone in an office because a few people took 3 attempts to remember which of their 2 or 3 passwords they used for your service), and this problem is only going to get worse - likely rapidly worse - as the address crunch squeezes in.
Posted Jun 27, 2011 21:13 UTC (Mon)
by solardiz (guest, #35993)
[Link] (1 responses)
Posted Jun 29, 2011 16:07 UTC (Wed)
by nye (subscriber, #51576)
[Link]
Posted Nov 24, 2012 5:58 UTC (Sat)
by LoneWolf (guest, #87996)
[Link] (1 responses)
Funny that the bug is still causing problems.
Posted Nov 24, 2012 6:05 UTC (Sat)
by LoneWolf (guest, #87996)
[Link]
A hole in crypt_blowfish
A hole in crypt_blowfish
If I understood correctly, someone who had managed to grab an old copy of the password database and who used it to find one of the "alternative" versions of an affected password might be able to use it to find the real one more easily.
There's a more drastic solution: don't allow characters with the high bit set in passwords at all. Deny login to any attempts to log in with such passwords.
A hole in crypt_blowfish
The restriction could exclude passwords that were set after the bug had been fixed.
A hole in crypt_blowfish
A hole in crypt_blowfish
A hole in crypt_blowfish
A hole in crypt_blowfish
A hole in crypt_blowfish
A hole in crypt_blowfish
A hole in crypt_blowfish
A hole in crypt_blowfish
A hole in crypt_blowfish
A hole in crypt_blowfish
A hole in crypt_blowfish
A hole in crypt_blowfish
When the byte value stored at *ptr has its high bit set, it is treated as a negative number.
Language misfeature
Language misfeature
Language misfeature
Language misfeature
This "miss-feature" is only troublesome when you start to do arithmetic with chars, which most library functions do not.
Language misfeature
Language misfeature
Language misfeature
Language misfeature
Language misfeature
19
$ grep DEFAULT_SIGNED_CHAR gcc/config/*/*.h | grep -c 1
28
Language misfeature
Language misfeature
NOT a Language misfeature, it's a Programming Error
An object declared as type _char_ is large enough to store any member of the basic execution character set. If a member of the required source character set enumerated in 5.2.1 is stored in a _char_ object, its value is guaranteed to be positive. If other quantities are stored in a char object, the behavior is implementation-defined; the values are treated as either signed or nonnegative integers.
----^----
[...] Their implementation-defined values shall be equal or greater in magnitude (absolute value) to those shown, with the same sign.
CHAR_BIT 8
SCHAR_MIN -127
SCHAR_MAX +127
UCHAR_MAX 255
CHAR_MIN see below
CHAR_MAX see below
----^----
I think you missed my point. I do agree it is a programming error. But this error (and countless like it) were made much easier to commit by this language misfeature, which makes straightforward code work in a very non-intuitive way.
NOT a Language misfeature, it's a Programming Error
A hole in crypt_blowfish
A hole in crypt_blowfish
openSUSE fix
More serious in some locales
More serious in some locales
The rest are at risk (meaning that 8-bit chars in _some_ positions result in 1 to 3 preceding chars being ignored)."
More serious in some locales
More serious in some locales
Other projects who find security holes in their code would do well to follow his lead here.
Trolling - again
Trolling - again
A hole in crypt_blowfish
A hole in crypt_blowfish
A hole in crypt_blowfish
A hole in crypt_blowfish
Sparse could find it
an example of why unit tests help with security
an example of why unit tests help with security
an example of why unit tests help with security
A hole in crypt_blowfish
Blocking brute forcing
Blocking brute forcing
Blocking brute forcing
Blocking brute forcing
Blocking brute forcing
A hole in crypt_blowfish
A hole in crypt_blowfish