BCrypt fixes

[Hackwar]

Reverting API changes to JUserHelper::getCryptedPassword()
implementing new methods to hash and verify a password in JUserHelper
changing core code to use new password methods

See #2555

[Bakual]

Can you please open a tracker item for this? I don't think the tracker item mentioned in the original PR does fit. The scope of this PR is much bigger.
Some description what exactly you want to solve should also be added.
Also can you provide some testing instructions? This would help the people who have to reproduce the issue and test the fix.

[betweenbrain]

I'd also add that we need to be sure to extensively test this on different server environments as well on sites moving from 3.1.5 / 3.1.6, as well as sites currently running 3.2.0.

[Hackwar]

Here is the tracker item: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32804&start=0

[mbabker]

To assist with testing this, I've built test packages based on master plus this pull request (synced to Hackwar/joomla-cms@2d8f524). Packages are available at http://developer.joomla.org/PR-packages/2589/ and will allow testing for all scenarios (new installs and updates from 2.5 forward).

[Bakual]

It also removes the arbitrary truncation of passwords to 55 chars that was falsely introduced.

Comment from Elin in the original tracker put here for reference:

The problem with allowing longer passwords is that it is not forward compatible. Users using such password that were always truncated will then have non working passwords if the bcrypt limit is ever changed.

Further too long passwords can be an attack. It's also totally unrelated to the issue at hand.

I would add here that truncating passwords (even if done by PHP password_hash() natively) without telling the user is certainly not transparent.

[Hackwar]

1. We are truncating passwords in JForm already to 100 chars max.
2. bcrypt internally indeed uses only the first 55 chars as it seems. But that is a limitation that the bcrypt implementation has to care for.
3. Also, if we ever switch away from bcrypt again, for example to scrypt, why should we have the arbitrary limit of bcrypt in our code? Similarly, why should we have that limitation if we are not using bcrypt, but SHA256 right now?
4. Last but not least, password_hash() takes care of all of these issues for us. If we use longer passwords, it truncates the password internally. If we use a different algorithm, it automatically hashes to that new algorithm and still validates the old passwords. Hashes created with password_hash() will always be verifyable with password_verify(), except for everything before 5.3.10.

If there is one thing that is not forward compatible, its forcing the passwords to an implementation specific length.

[Hackwar]

I would recommend implementing bcrypt in 4.0 and not earlier and at the same time dropping the SHA256 code that is in the verify-method.

[nikosdion]

-1

I disagree with this approach. Rolling back to a password hashing algorithm that's currently trivial to crack and sticking with it for 2-3 years is irresponsible.

In 2010 a password cracking rig capable of computing 33 billion hashes per second used to cost $2,500 USD (Whitepixel). In 2013, a password cracking rig of the same capacity costs about $600 USD (using Hashcat). Extrapolating from that thought, a password cracking rig capable of cracking the majority of passwords in a realistic amount of time will cost petty cash in 2-3 years and will be much faster than the previous iterations. Moreover, in the last 3 years we've seen strides in password cracking techniques. I don't know if anyone noticed how much and how fast stolen password databases are being cracked these days (cough Adobe cough).

Maybe we should stop being hypocrites and think of the bigger picture. We are hypocritically shouting "BUT, BACKWARDS COMPATIBILITY!" when we trash it in small, yet significant, ways in every minor release of Joomla! without any good reason. Ask extension developers, they know. Now we have a really damn good reason to break b/c: security! So what do we do? We decide to screw it up... Is supporting crap hosts which haven't been updated in the last 5 years worth putting our users (and their users!) at risk for the next 3+ years? That's the question we should all be asking.

[Hackwar]

This proposal is actually meant to provide a foundation from where we can move forward. It cleans up the code and fixes the bugs, concentrating the necessary changes for a different hashing system in one place. Right now we are flopping around, discussing about a dozen different issues at once and we aren't getting forward, because we don't agree on the hashing algorithm. So this is the proposal to fix all the other bugs and pushing the hashing algorithm to another thread. I didn't want to leave the trunk in a non-working state, so I added MD5 back in for the time being. When we decide how to proceed, I'm more than happy to change that.

"Nicholas K. Dionysopoulos" notifications@github.com schrieb:

-1

I disagree with this approach. Rolling back to a password hashing algorithm that's currently trivial to crack and sticking with it for 2-3 years is irresponsible.

In 2010 a password cracking rig capable of computing 33 billion hashes per second used to cost $2,500 USD (Whitepixel). In 2013, a password cracking rig of the same capacity costs about $600 USD (using Hashcat). Extrapolating from that thought, a password cracking rig capable of cracking the majority of passwords in a realistic amount of time will cost petty cash in 2-3 years and will be much faster than the previous iterations. Moreover, in the last 3 years we've seen strides in password cracking techniques. I don't know if anyone noticed how much and how fast stolen password databases are being cracked these days (cough Adobe cough).

Maybe we should stop being hypocrites and think of the bigger picture. We are hypocritically shouting "BUT, BACKWARDS COMPATIBILITY!" when we trash it in small, yet significant, ways in every minor release of Joomla! without any good reason. Ask extension developers, they know. Now we have a really damn good reason to break b/c: security! So what do we do? We decide to screw it up... Is supporting crap hosts which haven't been updated in the last 5 years worth putting our users (and their users!) at risk for the next 3+ years? That's the question we should all be asking.

---

Reply to this email directly or view it on GitHub:
#2589 (comment)

[nikosdion]

@Hackwar I understand your point of view and I agree with it. Since I'm not PLT I can only say how I would handle this situation in a way that's both backwards compatible and future-proof:

• Release Joomla! 3.2.1 with your patch. This fixes broken sites which is a pressing objective.
• Release Joomla! 3.3 with a proper bCrypt implementation and the minimum PHP version requirement increased to 5.3.7.

This way we can both fix broken sites and not put our CMS' future in harm's way. In this context I agree with your PR, i.e. I agree with the PR providing a clean slate on which we can develop a proper bCrypt implementation a.s.a.p. and not in 2-3 years. In so many words, I disagree with your "I would recommend implementing bcrypt in 4.0 and not earlier" comment. Implementing bCrypt in 4.0 is too little, too late.

[infograf768]

@nikosdion

+1. I see no other sensible solution.

[mbabker]

Now that it seems like we're coming to a consensus on things, if this is the patch we are going to go with, then it will require careful testing across various supported server environments and dealing with all scenarios:

• Users updating from 2.5 thru 3.1 to 3.2.1
• Users updating from 3.2.0 to 3.2.1
• New installs of 3.2.1

To assist with testing this, full install and update packages are available at http://developer.joomla.org/PR-packages/2589/. You'll need to download and install the packages through the Extension Manager, a XML file for testing updates via the update component hasn't been built (yet).

Please note that all testing results and issues should be reported at http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32804&start=0. Let's try to leave this PR for code related discussions unrelated to the testing of this patch from here on.

[ircmaxell]

This limit is way too short. If you want to prevent abuse, set the limit on the order of 512kb. Limiting reasonable lengths is just bad...

[Hackwar]

This is reverting the change that reduced it to 55 chars. It was 99 chars before. I'm open for changing this to something higher, but I'd like Michael to decide this one.

[mbabker]

At a minimum we should have the old behavior (so 99). If we can do longer,
then go for it. I'd just make sure the password field in the users table
can support whatever we're supporting in the PHP code.

On Mon, Nov 25, 2013 at 9:54 AM, Hannes Papenberg
notifications@github.comwrote:

In libraries/cms/form/rule/password.php:

@@ -77,11 +77,8 @@ public function test(SimpleXMLElement $element, $value, $group = null, JRegistry

    $valueLength = strlen($value);

•   /*
  

•    \* We set a maximum length to prevent abuse since it is unfiltered.
  

•    \* 55 is the length we use because that is roughly the maximum for bcrypt
  

•    */
  

•   if ($valueLength > 55)
  

•   // We set a maximum length to prevent abuse since it is unfiltered.
  

•   if ($valueLength > 99)
  

This is reverting the change that reduced it to 55 chars. It was 99 chars
before. I'm open for changing this to something higher, but I'd like
Michael to decide this one.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/2589/files#r7894354
.

[Hackwar]

I don't see any reason why we shouldn't allow longer passwords. Since we are hashing them, there is also no reason to limit it to some DB field. The hash always has identical length. So I'm going to set this to 512kb.

Stupid question, but: bcrypt runs inside of the PHP process, so it consumes the same memory as the original process. Considering that it is intended as a slow and memory intensive algorithm, would it make sense to have this limit lower in order to not run into memory issues? Like, shouldn't 8kb be good enough?

[nikosdion]

Memory consumption is a relative term. Please see http://crypto.stackexchange.com/questions/400/why-cant-one-implement-bcrypt-in-cuda The idea is that the table required to run bCrypt is too big to be stored in GPU registers. If it's stored in GDDR it can't be accessed in parallel, neutering the massive parallelism of GPU solutions. If it's stored in regular RAM it can be accessed in parallel, but very slowly, nullifying the benefits of GPU optimisation. Therefore, unless a major brakthrough in GPU password cracking is achieved, the relatively high memory usage of bCrypt prevents the use of super efficient GPU code to crack it. The main (but not only) reason why salted MD5 is considered unsafe is because it is extremely fast to compute by a massively parallelised GPU. So, no, the memory usage is not as high as you think it is.

Disclaimer: I am not a cryptographer. At best I am a complete n00b in password cracking. If I am wrong, please feel free to correct me – and let me learn something :)

[Hackwar]

I've raised the length to 2^12=4096 chars, which should be long enough. If anybody gives me good reason to raise it even further (remember, this is only in the rule for JForm, not an actual limitation for any code that circumvents this) besides "We HAVE to!!", I'm open to even longer strings. Anyway, I hope that solves this one.

[mbabker]

Looking at our code coverage report, we really should try and get this code unit tested too.

[Chris-Jones-Gill]

I tested this and it doesn't work for one important use case...

TL;DR
Upgrade from 3.1.6 to 3.2.0 to 3.2.1 causes failure, users cannot login on 3.2.1 when strong passwords are enabled in 3.2.0 and user updates password.

Full test description on JoomlaCode

Posting here because not everybody monitors JoomlaCode.

[mbabker]

Thank you for that @KISS-Web-Design - that is exactly the type of testing and feedback we need on this issue.

[nikosdion]

Joomla! site upgraded from 3.0.x to 3.1.5 to 3.2.0 to 3.2.1; Strong passwords were ENABLED.

Login with existing user: NEGATIVE

• Before the upgrade I checked the database. The Super User password is hashed using bCrypt (signature $2y$).
• Installed Joomla!_3.2.1-Stable-Patch_Package (from loaction provided by @mbabker) via Extensions Manager
• Tried to log in: FAILURE
• Replaced the password in the database with an MD5 hashed one from a different user, now I was able to log in.

Login with new user: SUCCESS

• Create a new super user
• I was able to log in with the new super user
• Checked the database, the password is hashed with salted MD5

Ergo: this patch is unable to authentication using passwords hashed with the botched bCrypt implementation

** FIX FOR THIS ISSUE **

In plugins/authentication/joomla/joomla.php add the following code in line 58, right after
$match = JUserHelper::verifyPassword($credentials['password'], $result->password, $result->id);

        // Work around for Joomla! 3.2.0's botched bCrypt implementation
        if (($match === false) && (substr($result->password, 0, 4) == '$2y$'))
        {
            $password60 = substr($result->password, 0, 60);

            if (JCrypt::hasStrongPasswordSupport())
            {
                $match = password_verify($credentials['password'], $password60);
            }
        }

Now the patch tests successful with both upgraded and new installations. Moreover we give priority to a future, correct, implementation of bCrypt over the existing botched attempt as this ugly, crappy code will only run if JUserHelper::verifyPassword fails.

[mbabker]

@nikosdion fix works for me as well. Pulled my site down (PHP 5.4 live and local, strong passwords enabled), tested update using the packages provided, could not log in with packages as posted (this PR basically). Added in @nikosdion code and got in. So I read through verifyPassword() and where it's checking the hash, it's actually using the clear text password. Changing $password to $hash in lines 332 and 342 got me into the site with @nikosdion code removed. But, the password isn't rehashed from BCrypt to MD5; is this intended (seems like no the way the code reads to me)?

[mbabker]

And digging a little bit more, $user->save(); in verifyPassword() is returning a false. The User not Super Administrator message is getting thrown by one of the exceptions in JUser::save() for me. Ironically, I'm using my site's only account which is a Super Administrator.

[mbabker]

And last comment spam for now. In JUser::save(), it sets $my = JFactory::getUser(); which is then checked later at $iAmSuperAdmin = $my->authorise('core.admin');. This fails to return a usable result in this case because $my returns a JUser instance of a guest user. So either JUser::save() will have to be adapted for this specific case or the rehash will have to be done after the user is authenticated and JFactory::getUser() returns an instance of the logged in user.

[FrankM1964]

I tested http://developer.joomla.org/PR-packages/2589/

New installation on

• ubuntu 12.04
• php version: 5.3.10-1ubuntu3.8
• joomla! 3.2.1

Results:

Admin login after installation -ok
New user created in FE - ok
New user created in BE - ok
Password changed in BE - ok
Password resetting FE - ok

Passwords looks like "05c9e83394fad8c1ab8baadb765162c9:BnRrQ5piy8HR48xMs2IhFOOr6EIr2fQV"

[colivier]

I tested also and for me all is good !

[mbabker]

Merged. Now that the broken API is fixed, let's focus on the other TODO items that have come out of these discussions. Thanks to everyone for assisting with testing and code review.

[dongilbert]

I have created another PR that gives enhanced security on the passwords by implementing (PHPass)[http://www.openwall.com/phpass/].

#2656

I had originally implemented crypt-sha256 (since it was @ircmaxell's second-best solution) but was (sadly) reminded by @mbabker that we still need to support 5.3.1, and crypt-sha256 was added in 5.3.2. So I updated the PR again to use Anthony's 3rd best solution, which was implementing PHPass.

I know you all did a lot of testing here already, but would you mind one more round so that we aren't falling back to low security for the remainder of the 3.x series? Thanks.

[wilsonge]

Are we happy to upgrade requirements to 5.3.2. I think the problem that we
had was the number of people on 5.3.3 and that's still fine if we upgrade
requirements to 5.3.2?

Thanks for your work Don! Will try and test later

On Tuesday, December 10, 2013, Don Gilbert wrote:

I have created another PR that gives enhanced security on the passwords by
implementing (PHPass)[http://www.openwall.com/phpass/]. #2656#2656

I had originally implemented crypt-sha256 (since it was @ircmaxellhttps://github.com/ircmaxell's
second-best solution) but was (sadly) reminded by @mbabkerhttps://github.com/mbabkerthat we still need to support 5.3.1, and
crypt-sha256 was added in 5.3.2. So I updated the PR again to use
Anthony's 3rd best solution, which was implementing PHPass.

I know you all did a lot of testing here already, but would you mind one
more round so that we aren't falling back to low security for the remainder
of the 3.x series? Thanks.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/2589#issuecomment-30200376
.

[Bakual]

Are we happy to upgrade requirements to 5.3.2.

👎

[dongilbert]

No, can't upgrade minimum at all, even to 5.3.2, although I would love it, because it would reduce our dependencies on 3rd party libraries.

[nikosdion]

As far as I remember, 5.3.0 up to 5.3.3 had a bug in the Late Static Binding implementation (correct me if I'm wrong). I'm also convinced that Joomla! RAD won't run on this kind of ancient PHP versions for this reason and we do use Joomla! RAD in our Control Panel. Have we ever actually tested Joomla! 3.2 on PHP 5.3.1 or are we just making a wild guess about our minimum version requirements?

[mbabker]

5.3.1 is our documented minimum as decided last summer (don't remember where those discussions occurred at the moment).

[nikosdion]

I'm not questioning what's documented, I'm questioning whether it reflects reality. We can document that it runs on a tin can full of sardines, but if nobody tests whether it does run on a tin can full of sardines we are just documenting a wild guess. My question is: has anyone ever tested Joomla! 3.2.0 against PHP 5.3.1? Not just guess, but test, like install, use and make sure it actually does work without blowing up. The reason I am asking is because I've not seen PHP 5.3.x less than 5.3.3 in the last 18 months in the wild. So maybe this entire "can we raise min req to 5.3.2" is a moot point because a. we might not actually be shipping software running on PHP 5.3.1 and b. we might not have enough users with servers running PHP 5.3.1 to justify a degradation in security. The 5.3.1 figure has come from the Platform. Now the Platform is no more, we have modified it in 3.1.x and 3.2.x, yet we still use that figure without questioning it.

So, I am asking again: has anyone actually tested that Joomla! 3.2.0 runs on PHP 5.3.1?

[mbabker]

Answering the question, the answer is 99% likely to be a no.

On Tue, Dec 10, 2013 at 9:05 AM, Nicholas K. Dionysopoulos <
notifications@github.com> wrote:

I'm not questioning what's documented, I'm questioning whether it reflects
reality. We can document that it runs on a tin can full of sardines, but if
nobody tests whether it does run on a tin can full of sardines we are
just documenting a wild guess. My question is: has anyone ever _tested_Joomla! 3.2.0 against PHP 5.3.1? Not just guess, but test, like install,
use and make sure it actually does work without blowing up. The reason I am
asking is because I've not seen PHP 5.3.x less than 5.3.3 in the last 18
months in the wild. So maybe this entire "can we raise min req to 5.3.2" is
a moot point because a. we might not actually be shipping software running
on PHP 5.3.1 and b. we might not have enough users with servers running PHP
5.3.1 to justify a degradation in security. The 5.3.1 figure has come from
the Platform. Now the Platform is no more, we have modified it in 3.1.x and
3.2.x, yet we still use that figure without questioning it.

So, I am asking again: has anyone actually tested that Joomla! 3.2.0
runs on PHP 5.3.1?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/2589#issuecomment-30234437
.

[Bakual]

At least nobody raised an issue on JoomlaCode that it breaks on 5.3.1. That means either nobody cares or it works.
I'm testing on 5.3.3 which is the lowest I have available.

[wilsonge]

Exactly - which is why I think we can rise to 5.3.2 minimum safely - will
still support the reasonably large number of people on 5.3.3 but we can
support sha256 now

On 10 December 2013 15:14, Thomas Hunziker notifications@github.com wrote:

At least nobody raised an issue on JoomlaCode that it breaks on 5.3.1.
That means either nobody cares or it works.
I'm testing on 5.3.3 which is the lowest I have available.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/2589#issuecomment-30235226
.

[nikosdion]

OK, I'll take it up to myself to test with PHP 5.3.1. FYI, I found out that XAMPP 1.7.3 did indeed ship with PHP 5.3.1. Now I'll just have to find a way to install it on a machine and test Joomla! 3.2...

[dongilbert]

Thanks @nikosdion. I was looking for installing 5.3.1 locally, but it seems all those resources are gone now. I was also looking for a vagrant box setup with 5.3.1 but have so far come up empty.

[brianteeman]

If you have access to windows then wampserver still has php 5.3.1 available

[Hils]

localhost: Unable to start Joomla 3.2 on wamp php 5.3.1 (apache 2.2.9).
Reverting to php 5.4.3 (apache 2.4.2) with same joomla install - it works
fine again.

[Hils]

Should have added - this is the browser error page I received on php 5.3.1:

"Error displaying the error page: Application Instantiation Error"

[dongilbert]

I couldn't find 5.3.1, but I did test on 5.3.2, and it works as far as I can tell.

[dongilbert]

Thanks Hils!

[nikosdion]

@Hils Your test is a red herring. What actually happened is that WAMPserver's PHP 5.3.1 has some issues connecting to the database server using hostname localhost instead of 127.0.0.1 (i.e. trying to connect via socket instead of over TCP/IP).

I did some testing with PHP 5.3.1 and 5.3.13, the only versions I could find for WAMPserver. I discovered some interesting issues:

• Joomla! 3.2.0 comes with the strong passwords enabled by default
• You cannot log in to the site's back-end when strong passwords are enabled. You can, however, log in to the front-end
• I switched to PHP 5.3.13 and turned off strong passwords. I logged out and back in, my password wasn't converted to salted MD5. Grrrr.... I had to edit my user account and save my password again to convert it to MD5.
• Switched back to PHP 5.3.1. Now that my password was salted MD5 I could log in to the back-end and front-end without a problem.
• I observed no problems with regards to the Joomla! RAD Framework. I was wrong, FOF does run on PHP 5.3.1. I stand corrected.

Takeaway: Joomla! 3.2.0 is already NOT running on PHP 5.3.1. However, nobody ever said anything in the forums, well over 1 month after its release. As a result it is safe to assume that nobody actually uses it with PHP 5.3.1. As a result there seems to be no reason not to bump the min req to PHP 5.3.2, allowing us to provide a better password hashing fallback. Therefore I propose to raise the minimum requirement to PHP 5.3.2 and change our documentation to reflect this.

[Hils]

Thanks for that info Nicholas. My wamp was set up by Nic Savov specifically
for testing Joomla & both those php versions had worked previously with 3.1

• ah well! Cheers :)

[Bakual]

Joomla! 3.2.0 is already NOT running on PHP 5.3.1.

Given all the issues related to "strong passwords", I'm not that sure it's specifially related to 5.3.1. 3.2.0 is probably just broken on anything lesser than 5.3.10.

I observed no problems with regards to the Joomla! RAD Framework. I was wrong, FOF does run on PHP 5.3.1. I stand corrected.

You should know better than to doubt your code 😄

[rgmears]

At the risk of being the fly in the ointment my local server is PHP 5.3.1. The live server is 5.3.27.
Just before taking the local installation live, I upgraded to J3.2 and could not log in. When I did manage to get in (by rebooting the server) I disabled "use strong passwords" and everything worked.
Now I am contemplating whether to upgrade XAMPP to version 1.8.0 which uses PHP 5.4.4. And set the optional PHP level to 5.4 on the live server. Seems this will be necessary from this discussion.

[Hackwar]

It would be a very wise decision.

[Chris-Jones-Gill]

@rgmears - Def update both your dev and live servers. PHP5.3 is not recommended by PHP, and is actually end-of-life, although security releases are still being produced (5.3.28 is the latest 5.3 release, and recommended if you can't move to 5.4 or 5.5).

Please note that the PHP 5.3 series will enter an end of life cycle and receive only critical fixes as of March 2013. All users of PHP are encouraged to upgrade to PHP 5.4.

Although 5.4 is stable and supported for bug fixes and security, it is actually considered 'legacy' because 5.5 has been released!

5.4.23 was released earlier this month, and is the stable version you should use for 5.4 (5.5.7 was also released and is considered the current stable version).

It is always recommended to update to the latest version in the branch you are using, as these contain the latest bug fixes and security updates.

[rgmears]

Thanks Hannes and Chris.

In case anyone is interested old versions of XAMPP can be found here: http://www.oldapps.com/xampp.php?old_xampp=46&ModPagespeed=noscript