22

I'm wondering if this function (which is in part taken from a ~2 year old phpBB version), is good enough.

If not, why?
And how would you change it (making the transition seamless for existing users) ?

The result of hash_pwd() is what will be saved in a DB.

function hash_pwd($password)
{
    $itoa64 = './0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz';

    $random_state = $this->unique_id();
    $random = '';
    $count = 6;

    if (($fh = @fopen('/dev/urandom', 'rb')))
    {
        $random = fread($fh, $count);
        fclose($fh);
    }

    if (strlen($random) < $count)
    {
        $random = '';

        for ($i = 0; $i < $count; $i += 16)
        {
            $random_state = md5($this->unique_id() . $random_state);
            $random .= pack('H*', md5($random_state));
        }
        $random = substr($random, 0, $count);
    }

    $hash = $this->_hash_crypt_private($password, $this->_hash_gensalt_private($random, $itoa64), $itoa64);

    if (strlen($hash) == 34)
    {
        return $hash;
    }

    return false;
}


function unique_id()
{
    $val = microtime();
    $val = md5($val);

    return substr($val, 4, 16);
}

function _hash_crypt_private($password, $setting, &$itoa64)
{
    $output = '*';

    // Check for correct hash
    if (substr($setting, 0, 3) != '$H$')
    {
        return $output;
    }

    $count_log2 = strpos($itoa64, $setting[3]);

    if ($count_log2 < 7 || $count_log2 > 30)
    {
        return $output;
    }

    $count = 1 << $count_log2;
    $salt = substr($setting, 4, 8);

    if (strlen($salt) != 8)
    {
        return $output;
    }

    /**
    * We're kind of forced to use MD5 here since it's the only
    * cryptographic primitive available in all versions of PHP
    * currently in use.  To implement our own low-level crypto
    * in PHP would result in much worse performance and
    * consequently in lower iteration counts and hashes that are
    * quicker to crack (by non-PHP code).
    */
    if (PHP_VERSION >= 5)
    {
        $hash = md5($salt . $password, true);
        do
        {
            $hash = md5($hash . $password, true);
        }
        while (--$count);
    }
    else
    {
        $hash = pack('H*', md5($salt . $password));
        do
        {
            $hash = pack('H*', md5($hash . $password));
        }
        while (--$count);
    }

    $output = substr($setting, 0, 12);
    $output .= $this->_hash_encode64($hash, 16, $itoa64);

    return $output;
}

function _hash_gensalt_private($input, &$itoa64, $iteration_count_log2 = 6)
{
    if ($iteration_count_log2 < 4 || $iteration_count_log2 > 31)
    {
        $iteration_count_log2 = 8;
    }

    $output = '$H$';
    $output .= $itoa64[min($iteration_count_log2 + ((PHP_VERSION >= 5) ? 5 : 3), 30)];
    $output .= $this->_hash_encode64($input, 6, $itoa64);

    return $output;
}

function _hash_encode64($input, $count, &$itoa64)
{
    $output = '';
    $i = 0;

    do
    {
        $value = ord($input[$i++]);
        $output .= $itoa64[$value & 0x3f];

        if ($i < $count)
        {
            $value |= ord($input[$i]) << 8;
        }

        $output .= $itoa64[($value >> 6) & 0x3f];

        if ($i++ >= $count)
        {
            break;
        }

        if ($i < $count)
        {
            $value |= ord($input[$i]) << 16;
        }

        $output .= $itoa64[($value >> 12) & 0x3f];

        if ($i++ >= $count)
        {
            break;
        }

        $output .= $itoa64[($value >> 18) & 0x3f];
    }
    while ($i < $count);

    return $output;
}
Jimbo
  • 25,790
  • 15
  • 86
  • 131
koichirose
  • 1,815
  • 4
  • 22
  • 30
  • 11
    This is *way* too complex and too custom. Use a simple, well-known, well-tested implementation like `bcrypt` instead! – ThiefMaster Apr 16 '13 at 16:20
  • 2
    all about that bcrypt. SLOW IS GOOD BTW – j_mcnally Apr 16 '13 at 16:22
  • http://www.php.net/manual/en/function.crypt.php 4.3+ isn't good enough? If thats the case you may have bigger issues. – j_mcnally Apr 16 '13 at 16:23
  • it's all rather pointless. `md5(password + random state)` would be just as effective as all that individual per-char md5+substring junk is... – Marc B Apr 16 '13 at 16:26
  • ok, but still, complex as it is, is it safe enough? Would you change it for bcrypt? And why, considering all the work that would need to be done? – koichirose Apr 16 '13 at 16:29
  • 7
    If ever there is a programming problem where you want to rely on already written, tested and debugged code rather than making up your own, encryption would be it. – Andy Lester Apr 16 '13 at 17:03
  • @AndyLester that, in fact, is not my code, but something the phpBB team wrote. I've been using it on one side-project for a while now. – koichirose Apr 16 '13 at 17:07
  • Well said @AndyLester very very well said! – Rixhers Ajazi Apr 16 '13 at 20:30
  • There was a nice related discussion on StackExchange's IT Security: ["Is my developer's home-brew password security right or wrong, and why?"](http://security.stackexchange.com/questions/25585/is-my-developers-home-brew-password-security-right-or-wrong-and-why) – metadings May 15 '13 at 19:49
  • 1
    Also see Openwall's [PHP password hashing framework](http://www.openwall.com/phpass/) (PHPass). Its portable and hardened against a number of common attacks on user passwords. The guy who wrote the framework (SolarDesigner) is the same guy who wrote [John The Ripper](http://www.openwall.com/john/) and sits as a judge in the [Password Hashing Competition](http://password-hashing.net/). So he knows a thing or two about attacks on passwords. – jww Oct 12 '14 at 01:48

4 Answers4

52

The code that you have given is a port of PHPASS, specifically the "portable" algorithm. Note the qualification of portable. This will only apply to the phpass library if you pass true as the second constructor parameter. From here on out in this answer, phpass refers ONLY to the portable algorithm, and not the library itself. The library will do bcrypt by default if you do not explicitly specify portable...

The PHPBB team did not develop this themselves (a very good thing), but ported it from phpass directly (arguable).

There are a few questions we should ask here:

Is It Bad?

The short answer is no, it's not bad. It offers pretty good security. If you have code on this right now, I wouldn't be in a rush to get off it. It's adequate for most usages. But with that said, there are far better alternatives if you were starting a new project that I wouldn't pick it.

What are some weaknesses?

  • Relative To pbkdf2: The phpass algorithm uses hash() where pbkdf2() uses hash_hmac(). Now, a HMAC runs 2 hashes for every call internally, but the PHP implementation only takes approximately 1.6 times the execution of a single call to hash() (isn't C wonderful?). So we get 2 hashes from hash_hmac in 62% of the time it would take hash() to execute 2 hashes.

    What does that mean? Well, for a given runtime, pbkdf2 will run approximately 37.5% more hashes than the phpass algorithm. More hashes in a given time == good, because it results in more computation being performed.

    So pbkdf2 is approximately 37.5% stronger than phpass when using the same primitive (md5 in this case). But pbkdf2 can also take stronger primitives. So we can use pbkdf2 with sha512 to gain a very significant advantage over the phpass algorithm (mainly because sha512 is a harder algorithm with more computation than md5).

    This means that not only is pbkdf2 able to generate more computations in the same amount of time, it's able to generate harder computations.

    With that said, the difference isn't overly significant. It's very much measurable, and pbkdf2 is definitely "stronger" than phpass.

  • Relative To bcrypt: This is a lot harder of a comparison to make. But let's look at the surface of it. phpass uses md5, and a loop in PHP. pbkdf2 uses any primitive (in C) and a loop in PHP. bcrypt uses a custom algorithm all in C (meaning it's a different algorithm from any available hash). So right of the bat, bcrypt has a significant advantage just do to the fact that the algorithm is all in C. This allows for more "computation" per unit time. Thereby making it a more efficient slow algorithm (more computations in the given runtime).

    But just as important as how many computations it does is the quality of the computations. This could an entire research paper, but in short it comes down to the fact that the computations that bcrypt uses internally are much harder to perform than a normal hash function.

    One example of the stronger nature of bcrypt is the fact that bcrypt uses a far larger internal state than a normal hash function. SHA512 uses a 512 bit internal state to compute against a block of 1024 bits. bcrypt uses instead about 32kb of internal state to compute against a single block of 576 bits. The fact that bcrypt's internal state is so much bigger than SHA512 (and md5 and phpass) partially accounts for the stronger nature of bcrypt.

Should It Be Avoided

For new projects, absolutely. It's not that it's bad. It isn't. It's that there are demonstrably stronger algorithms out there (by orders of magnitude). So why not use them?

For further proof of how bcrypt is stronger, check out the Slides from Password13 (PDF) which launched a 25 GPU cluster for cracking password hashes. Here are the relevant results:

  • md5($password)
    • 180 BILLION guesses per second
    • 9.4 Hours - All possible 8 character passwords
  • sha1($password)
    • 61 BILLION guesses per second
    • 27 Hours - All possible 8 character passwords
  • md5crypt (which is very similar to phpass with a cost of 10):
    • 77 Million guesses per second
    • 2.5 Years - All possible 8 character passwords
  • bcrypt with a cost of 5
    • 71 Thousand guesses per second
    • 2700 Years - All possible 8 character passwords

Note: all possible 8 character passwords are using a 94 character set:

a-zA-Z0-9~`!@#$%^&*()_+-={}|[]\:";'<>,.?/

The Bottom Line

So if you're writing new code, without a doubt use bcrypt. If you have phpass or pbkdf2 in production now, you may want to upgrade, but it's not a clear cut "you're significantly vulnerable".

ircmaxell
  • 163,128
  • 34
  • 264
  • 314
  • The linear predictions are misleading, computer power doesn't stand still! Taking [Moore's Law](http://en.wikipedia.org/wiki/Moore's_law) into account 8chr passwords *can* be cracked in ~20 years with enough dedication by switching to a more powerful machine every time and picking up where the other one left off. Passwords don't stand the test of time! – CSᵠ Apr 16 '13 at 19:25
  • @ka: of course. And considering that this used 25 GPUs and can scale linearly to 99 GPUs, these numbers shouldn't be taken as gospel. Instead, the relative scale is what's important. Not that 27 hours vs 30 hours is significant, but 10,000 *times* longer (approximately 2.5 years vs 27 hours)... – ircmaxell Apr 16 '13 at 19:29
  • Indeed. More of a reason to use encryption at the latest standards. – CSᵠ Apr 16 '13 at 19:36
  • What does "with a cost of 5" or "with of 10" mean? – user93353 Apr 25 '13 at 01:43
  • 1
    @user93353: `cost` is a parameter that both bcrypt and the phpass algorithm accept to "tune" how expensive the operation is. That way, as hardware gets faster, the cost can be increased over time to make it stay just as safe. Today, a cost of 10 with bcrypt takes less than 100 ms to run. But a cost of 12 would take nearly 1/2 second. So by tuning the cost, we can keep execution time reasonable, but at the same time provide additional defense against brute forcing by attackers. – ircmaxell Apr 25 '13 at 11:22
  • @ircmaxell hey Anthony, that link to the PDF report is dead (access forbidden) by any chance do you have a copy of it? - Need it for a report for college. Any help would be great! – Rixhers Ajazi May 09 '13 at 17:26
  • @RixhersAjazi: I don't unfortunately. I would suggest searching for the slides, they are out there on the internet available somewhere... It's the presentation by Jeremi Gosney at Passwords12. – ircmaxell May 09 '13 at 18:23
  • Found a video (even better) :D – Rixhers Ajazi May 09 '13 at 18:33
19

Quick answer:

Use bcrypt (when it is ready) or password_compat library from ircmaxell - it's a bcrypt library.

Long answer:

This is too complex and outdated for current technology. Md5 should be avoided and just salting isn't good enough. Use bcrypt and save yourself the headache.

You might ask yourself why that specific library? Well the same functions will be available in php 5.5 so you won't have to change any coding. Best of luck and keep it simple and efficient. Also slow is good for logging in and password stuff.

Update 1

@ Gumbo -> No MD5 is not broken, but the main purpose of hashes like MD5 now and in the past was for file checking (as you know to check if the contents of the file can be trusted NOT password storage) as hashes are very quick to decipher since you wouldn't use Bcrypt to check the contents of file since you wait for 30 - 45 seconds... So this means that a hash was specifically meant to be read quickly. Even a SHA512 compared to bcrypt is still inferior completely. Thats why it is imperative to push for strong password algorithms like Blowfish/Bcrypt in PHP. We as users and programmers alike must expand the knowledge that plain password storing or low level hashing algorithms ARE NOT the answer - and should never be used for such sensitive information.

Now to the OP to make your transition over to the new system you would first send a notification to all users stating that "For your security the password encryption system has been updated ........", then you would ask them for a one time password update, once you do the password update you would use the the function called password_verify and if you want to have maximum control of your cost ratio's you use password_needs_rehash if for some reason you choose to change the cost associated with the passwords.

This won't take a massive block of code to do this as the gains you would receive in pass word protection out weigh the negatives of having to "recode" the new password system.

Hopefully this answers most questions, but none the less IRCmaxell's answer is far superior further going into detail on different algorithms! Best of luck OP, and a big thanks to ircmaxell!

Update 2

Also, would a password stored like this be actually crackable? how? (I'm getting curious now)

Any thing and everything is broken is what my Network security professor tells me.. I laughed at him. Now that I see things in his eyes... well yes.. that is absolutely true!

Bcrypt CURRENTLY is the best method of storing passwords! However if you look into Scrypt that seems to be promising but not supported by PHP. None the less any thing and everything is broken, its just a matter of time until some "geek" in a basement will crack Bcrypt. But for now we are safe.. just like we are safe with IPv4 never running out.... oh wait?... ;)

Hopefully this answers your issue that you brought up sir. Also just to put it into context my system is at a cost of 17 and it takes ~20 - ~30 seconds for logging in, but when I presented the system to my professor and my clients and why it should be mandatory they loved it and felt reassured that they were protected.

Rixhers Ajazi
  • 1,303
  • 11
  • 18
  • how would you manage the transition for existing users, though? – koichirose Apr 16 '13 at 16:32
  • Also, would a password stored like this be actually crackable? how? (I'm getting curious now) – koichirose Apr 16 '13 at 16:34
  • Why should MD5 be avoided? (Please don’t say “it’s broken”.) – Gumbo Apr 16 '13 at 17:10
  • 2
    @koichirose *all passwords are "crackable"*. The real question is how easily? Even as obfuscated as it is PHPBB relies on md5 which is designed to run fast, and this implementation can be easily parallellized. Even a strong password can be bruteforced by a motivated hacker within a few hours and a few thousand dollars of commodity hardware. The reason bcrypt is recommended so often is because it is slow, and it is difficult [if not impossible] to parallellize. See: http://security.stackexchange.com/questions/211/how-to-securely-hash-passwords – Sammitch Apr 16 '13 at 17:11
  • @Gumbo: Watch this presentation that I did that points to why you should avoid MD5 and fast hashes (including SHA512, etc): [YouTube](https://www.youtube.com/watch?v=eNdW5HWBhG0) – ircmaxell Apr 16 '13 at 17:33
  • @ircmaxell *I* know the reason why hash functions should not be used for hashing passwords – at least not as is. But people tend to say things like “don’t use MD5, it’s broken” without knowing why. At best they say something like “use SHA-256 or X instead” although that won’t change much. – Gumbo Apr 16 '13 at 18:50
  • @Gumbo: completely fair. I wasn't sure if the way you phrased it was asking for an explanation (for others) or because you didn't understand. Either way, the video is likely a good source for others to learn the same thing... – ircmaxell Apr 16 '13 at 18:52
  • By the way: this implementation does use a load/cost factor which defaults to 2^6 iterations. – Gumbo Apr 16 '13 at 18:54
  • @koichirose - Every password-storing-system must have the option to switch to a better hash algorithm, your problem is not a one-time migration problem. Have a look at this [answer](http://stackoverflow.com/a/14402451/575765), it's definitely worth a question of its own. – martinstoeckli Apr 16 '13 at 20:02
  • I'm sorry for making you feel like that Gumbo, and @kioichirose I'll update my answer. – Rixhers Ajazi Apr 16 '13 at 20:19
  • @koichirose I updated my answer. Definitely look into ircmaxell's answer much much much more in depth hell he created the bcrypt library and built in functions for php lol – Rixhers Ajazi Apr 16 '13 at 20:34
  • Actually md5 *is* broken. And has been for some time now. @RixhersAjazi – PeeHaa Apr 17 '13 at 16:39
  • @PeeHaa埽: Yes, it is broken. But not for password hashing in the way you're indicating. It's only broken when you know the input beforehand (or that's the only case the attacks against it can produce faster-than-brute-force answers). – ircmaxell Apr 17 '13 at 16:52
  • @RixhersAjazi I definitely like this answer as well. Well written. Once the question is eligible for a bounty, expect one coming your way... – ircmaxell Apr 17 '13 at 16:57
  • Thanks @ircmaxell means a lot coming from you. – Rixhers Ajazi Apr 17 '13 at 17:19
  • @PeeHaa埽 md5 is very useful for when it comes to file checking, every time in class we did a download from a source we didn't trust for a common linux-distro we checked its hash to see if it should be trusted. In that sense no MD5 is "perfect" for the job... but yes md5 for passwords is "broken" - (as broken is the popular terminology for representing md5)... – Rixhers Ajazi Apr 17 '13 at 17:24
  • Yeah @RixhersAjazi but that doesn't make it less broken – PeeHaa Apr 17 '13 at 17:27
  • 2
    @RixhersAjazi I think PeeHaa is referring to [preimage and collision attacks](http://en.wikipedia.org/wiki/MD5#Security) that MD5 is vulnerable to when used in a signing or integrity checking role... The case you just mentioned is exactly where md5 should *not* be used (because it's cryptographically broken)... – ircmaxell Apr 17 '13 at 17:29
  • Oh and I completely agree with you, md5 should be as far away from data as possible. But just for the sake of talking about it saying md5 is broken is like saying your mad at the Apple(fruit) because it's not an orange. They are both two different things and for some reason before I was interested in php some one thought that they should use md5 for password "encryption". – Rixhers Ajazi Apr 17 '13 at 17:35
  • Ahh thanks @ircmaxell then yes peehaa you are absolutely correct – Rixhers Ajazi Apr 17 '13 at 17:36
1

I would go for bcrypt.It is good Besides incorporating a salt to protect against rainbow table attacks, bcrypt is an adaptive function: over time, the iteration count can be increased to make it slower, so it remains resistant to brute-force search attacks even with increasing computation power.

Implementation: How do you use bcrypt for hashing passwords in PHP?

Community
  • 1
  • 1
Sudo Reboot
  • 220
  • 2
  • 11
-5

(Sorry for my English at first)

There is few questions, which all of commenters have to ask before answering:

Where will be that code used?

What kind of data it will protect?

Will it be used for critical-level system?

Ok we have some use-case. I am using simplified password generator for out IS, yes it is really not '3rd World War ready', but the chance, that user will tell password to someone, or it will be leaked by some malware from his computer is still much bigger.

  • md5 is weak, use newest fingerprint generator (sha128,sha256,sha512)
  • use random length salt like:

    private function generateSalt() {
      $salt = '';
      $i = rand(20, 40); //min,max length
      for($length = 0; $length < $i; $length++) {
        $salt .= chr(rand(33, 126));
      }        
      return $salt;
    }
    
  • then generate some password:

    private function generatePass() {
      $vocabulary = 'abcdefghijklmnopqrstuvwxyz0123456789';
      $password = '';
      $i = rand(7,10);
      for($length = 0; $length < $i; $length++) {
          $password .= substr($vocabulary,rand(0, 35),1);
      }        
      return $password.'-'; // avoid copy`n`paste :)
    }
    
  • yes password should be stronger, but user will never remember it, it will be saved to browser or written somewhere. Expectations and security versus reality.

    $newSalt = $this->generateSalt();
    $newPass = $this->generatePass();
    $newHash = hash('sha512', $newPass.':'.$newSalt); 
    // now store hash and salt into database
    
  • There is new hash, but it is not over, true work has begun:

    1. logging
    2. session protecting
    3. user,user+ip,ip temp/perm banning
    4. etc

login+logout+re/generate password: 1 or 2 SQL tables and few kb of code

other things about security: many tables, really more than few kb of code

  • 2
    Haven't you seen the above two posts? – PeeHaa Apr 20 '13 at 13:33
  • Yes, but there is another problem: The weakest link is still user, hashing is our protection, to avoid compromise whole data. – Michal Taneček Apr 21 '13 at 00:42
  • 1
    If you call this password security / storing then this makes me wonder exactly how people on the internet / sites I use store my password. (I am serious...) Take a look at ircmaxell's answer, and then mine but his first :P – Rixhers Ajazi Apr 22 '13 at 00:41
  • Ok then calculate collisions possibility and brutal force time for fingerprint in sha512, when you know, that firs 8 to 15 chars is password, then is ':', followed with 20 to 40 unkown chars. – Michal Taneček Apr 22 '13 at 01:49