Hold off on testing. Was just reminded we support 5.3.1, where crypt-sha256 isn't available.

Ok - you can test again. Since we have to support 5.3.1, we can't depend on crypt-sha256 being available. The third best option presented by @ircmaxell was to integrate (PHPass)[http://www.openwall.com/phpass/], which is what the updated PR does.

I'm against this PR. We currently have the following situation:

• There is a PHP built-in function to handle all that what we need for us, but that function is not available in the versions that we support.
• To get around that, we throw lots of code at it and tie ourselfs to yet another third party library with a more or less non-standard way of handling things.
• When we actually raise the minimum requirements and can use the much simpler password_hash(), we not only have to care for unsalted-MD5, salted-MD5, SHA256 and bcrypt, we also then have to have code in for PHPass passwords. None of those methods we can remove again anytime soon in the next ~3 years, since there might be accounts out there that were hashed with something other than bcrypt.

I also don't see if this also solves the issue of sites moving hosts and then not having the correct encryption available to "decode" the passwords again.

I'd vote to keep this simple, leave it at salted-MD5 for the 3.2.x series, raise the minimum requirements for 3.3 and then introduce bcrypt again.

No need to take offense, @hackwar

None taken, just stating my opinion.

This PR is meant to provide password strength for our users on the 3.x series, and that's it. Salted MD5 is insecure. This fixes that.

As for your opinion on usage of JCrypt::timingSafeCompare(), while not strictly needed, it is useful to prevent timing attacks. Consider an enterprise intranet with an internal disgruntled employee. Network latency isn't an issue there at all, so it would be entirely possible to execute a timing attack in that scenario. Having that in place adds little overhead, and such overhead is justified by the protection it affords.

Also, if you look at PHPs password_verify https://github.com/ircmaxell/password_compat/blob/master/lib/password.php#L202 - you can see a timing safe comparison is done there as well.

The JCrypt::timingSafeCompare() method was contributed by me at the direction of @ircmaxell during the summer when I was consulting with him on a proper fix for Remember Me.

Can we please have a tracker item for this? I think this is just too sensible to just track here.

As for testing. I think the fresh installation should be based on current master, right? So we have the fix from Hannes already in it.
For updating tests we would first have to use the update patch created by Michael and then apply this patch from there?

We would have strong passwords if the initial code would have been correct and the requirements for 3.2.0 had been raised accordingly. Salted MD5 is insecure, but it has been like that for several years now. As I stated earlier, there are two sensible solutions to this whole thing and that is either to stick to salted MD5 or to raise the minimum requirements to 5.3.10 and then use bcrypt/password_hash. It was decided to not raise the minimum requirements.

I strongly believe that if we keep up this type of coding, our verifyPassword() method will soon look like getCryptedPassword() and instead of solving issues, we are going to run into even more, since nobody will understand these parts anymore. I already hate that we now have 4 different types of password verification in our codebase.

As I stated earlier, I also don't see how PHPass makes sure that all hashes are readable on all systems. I think we run into the same problem here that we did with the previous solution, that it isn't portable from higher to lower PHP versions. I might be wrong here.

Regarding timinigSafeCompare(): Network latency is only the most obvious part that introduces entropy here. Considering all the different parts that are involved in loading Joomla, you rarely get the same rendering time for the same output. Simply the database can introduce so much entropy here, that it skews any numbers regarding that comparison. Someone else loading the site while you are accessing it, swapping data to disk, reading data from disk, etc. skew those numbers. Yes, you might be able to do a timing attack on a PHP program, but that would require the program to be sufficiently small. I don't think that you can distinguish the maybe +-50 clock cycles that this comparison takes from all the noise around it.
If Anthony regards that method as necessary, I'd like to see some more background to that than simply "you should use one." Yes, I'm no crypto expert, but I know the behavior of Joomla and how even its own debug data jumps around all over the place. Considering that spread, I need some further proof that such a timing safe comparison is necessary.

As for too many methods being in verifyPassword, we can remove the one that hasn't been put into the wild yet, which is SHA256. 

PHPass is supported all the way down to PHP 3, so no, there is not an issue with using PHPass in a 5.5 dev environment and deploying to 5.3.1.

As an aside, if we were supporting only 5.3.2+, we could offer strong passwords via crypt-sha256 natively, but that is sadly not the case.

Tested on PHP 5.3.3 (CentOS)

• I've updated a 3.1.6 to 3.2.0 to "master" (using http://developer.joomla.org/PR-packages/2589/) and then applied this patch. After each step ensuring I have the expected results with a new user.
• After applying this patch, everything works as expected. Passwords are resaved correctly.

@Hackwar If we can make this safer without raising the requirements, why shouldn't we? We can only win. It's your PR which made this quite easy to implement
Upgrading the passwords to bcrypt in a later release will not really be an issue.

@dongilbert
SHA256 is already in the wild for all installations with PHP < 5.3.10

@Bakual
Because when you introduce it now, you have to carry it around until at least 5.0. Otherwise you have hashes that have been hashed with for example SHA256 or PHPass, but can't verify them without having that code in the system. Remember that there are hundreds of thousands of accounts out there, that are maybe used once a year or even less.

With 3.2.0 we introduced bcrypt and SHA256 into the wild and we will have to support that for a LONG time. Do you want to introduce PHPass in 3.2.1, too, having to support it at least equally long to SHA256? We already plan to switch to bcrypt at the next possible moment, which from my understanding would be 3.3 or 3.5, whatever the next version will be named. That might be only a few weeks or months away. Do you really want to introduce yet another hashing scheme, that we will have to support for half of eternity only to span <6 months?

If this security was so important to us, we could release 3.2.1 now with the fix to the passwords, leaving people with a working site and then state, that we will release 3.3 shortly, which will raise requirements to 5.3.10. Then we could release 3.3 in 2 weeks or something like that and have bcrypt in there. Less code, clear process, no one is left with a broken site.

Oh, and SHA256 != secure. You'd have to iterate that, too, and then we might as well use iterated MD5...

@dongilbert
I did not say that PHPass does not work on lower PHP versions. I said that I don't see how PHPass solves the issue, that hashes created on one version are still verifyable on a lower version. "Who in their right mind would move the site from a higher to a lower PHP version?" That would be @betweenbrain and really a lot of other people out there.

The current implementation uses portable_hashes and from what I understand, that is simply salted, iterated MD5. Yes, that is an improvement, but I would not say that the additional complexity and overhead that we would introduce simply for salted, iterated MD5 is worth it. Release 3.2.1 now to fix the current issues people have, announce 3.3 to be released in 2 weeks with raised requirements and go to bcrypt directly and we all benefit a lot more from this.

The current implementation uses portable_hashes and from what I understand, that is simply salted, iterated MD5.

According to Anthony, the difference is worth it.

announce 3.3 to be released in 2 weeks

That would be right at Christmas. Forget it, that's not going to happen at all. We're talking about some months here till 3.3 could be released. It's not everything about bcrypt, there are also some GSoC features which may end up in there and need proper testing as well.

No one is saying, that 3.3 needs to be the last STS release. We've had long discussions on the mailinglists that we should/might/could/will/will not change the development strategy and to me that means that we could release a 3.3 now and a 3.4 in 6 months or 3 months or whatever.

@Hackwar - (excuse the exaggeration) but, every senior PHP dev in the world is backing @ircmaxell and asking that Joomla strengthen it's hashing strategy. Let's get behind this.

@dongilbert Much, much, much respect. Thank you. (I'm busy the next couple of hours but I will be back to test. )

@Hackwar

Ok, let's go over your concerns one by one.

PHPASS is little more than iterated md5

That is completely correct. By very definition it is just iterated md5.

With the cost applied here (10), it would therefore be by very definition 1024 times stronger, as it's 1024 times slower and does 1024 times more work (2^10).

1024 is 3 orders of magnitude. It would be the difference between a successful brute-force taking minutes and days. For a single password.

PHPASS isn't worth it

Tell that to the boat load of projects that are using it (or a fork) like Drupal, Wordpress, PHPBB, etc.

There are better methods

Yes there are. You could use PBKDF2. But that would require you to maintain the crypto implementation as well as the hooks into your code. Which is why I recommended phpass, because you only need to maintain the hooks.

You could use crypt-sha256/512. But your version requirements are too low. That ship has sailed. Move on.

You could use bcrypt. But your version requirements are too low. That ship has sailed. Move on.

You could use scrypt. But it's not included in PHP core. So no.

The best option that doesn't require you maintaining crypto code is to use PHPASS. Which is why it's my recommendation.

More Code To Maintain

You're only maintaining it in the verify function. You won't need to maintain a whole host of other code. And since it's a 3pd library, you don't even need to maintain that code.

And all you're talking about is a single switch statement (or collection of ifs). That are very deterministic and easy to debug. It's not like you're talking about a significant amount of complex logic...

Just Ship This, Fix Later

That's all well and fine, except that md5+salt is insecure enough that it's irresponsible shipping today with it. It should be counted as a security issue, and all old versions in security maintenance mode should have its method upgraded. Yes, it's that big of an issue. Don't believe me? Ask Linked-in about it...

It's Been Insecure For Years, Why Rush Now

Yeah... Not going there...

Verify Becoming GetCryptedPassword

The problem with getCryptedPassword was not its complexity (although that was a problem). The problem was that it was the wrong abstraction. Creating and verifying were two different tasks the one method tried to do. That led to all sorts of cludge around bcrypt (since it does require two different branches for that).

This fixes the abstraction. So even if verify does get more complex, it will be isolated and specifically around verifying. That is easy enough to maintain to be a significant win.

Also, since you should never need to generate a salt in a verify method, that entire branch of logic that was in getCryptedPassword should disappear.

Using Non-Standard Solutions

I find it very ironic that someone makes the argument about phpass being "non-standard", when looking at the history of Joomla in this particular context is the very definition of "non-standard".

PHPASS has been around for a LONG time. It is seen as a defacto standard. It is the recommendation if you can't use bcrypt (or you can use it to generate bcrypt hashes as well). password_hash is an evolution on that concept. Not just "more standard"...

@Bakual
Please link to the tracker item if/when it gets created.

@AmyStephen I'm not denying that we have a problem here. I'm also not saying that we should not improve our system. I'm saying that we should reduce the number of hashing algorithms that we use to the necessary count. We decided that we can't raise the system requirements from 3.2.0 to 3.2.1. We also can't leave the people hanging in mid-air that already updated to 3.2.0. So all I am saying is, to release 3.2.1 with the fix to the password system and if you want, we can release 3.3 an hour later with raised system requirements and bcrypt as default hashing algorithm.

@ircmaxell Yes, I know all those arguments. Excluding the timing attacks, I also agree with all of those arguments. But I think/thought, that we should move to password_hash() as the solution to use. Considering how difficult it is already to deprecate broken functions/methods/classes in Joomla, I get chills when thinking about deprecating something that will potentially stay in our databases for years. As I said, there is very little that speaks against releasing 3.2.1 now, 3.3 an hour later and use bcrypt in there. That temporarily repairs peoples sites, allows them to take their time to update their systems (like, a few days/weeks) and at the same time allows us to provide 90% of our users with the best solution available. Using PHPass, we force people first from MD5 to bcrypt or SHA256, then to iterated MD5 and then to bcrypt again. I'd just like to cut out the iterated MD5 here.

And all that is needed, is a decision by the PLT. I promise to do all the necessary code changes. I will provide you with a version checker in the updating code and all that is necessary to get this rolling and I promise to get all that coded and ready to be tested by wednesday 22:00 GMT+1, provided that I get that decision until 22:00 GMT+1 today. I even keep my hands of that timingSafeComparison() method. ;-) The community and PLT would "only" have to prepare the release statement and test the code provided by me. I'm here to invest as much time as needed, although I have a family and a full time job that needs my attention.

Regarding a 3.3 release, it won't be only to raise the tech requirements and implement BCrypt; we have quite a bit of code sitting around pending review and merge from the 3.2 pool too. The earliest we will do a minor release like 3.3 is March.

I can't speak for the full PLT, but if this secures our system just that much more and can be done transparently to the user, then I personally would support going this route for now with 3.2 and looking at backporting it into 2.5.

if you want, we can release 3.3 an hour later with raised system requirements and bcrypt as default hashing algorithm.

As explained already, we can't do that this fast. It's already a lot of work to get one release done (about a week worth). Doing a new minor release is just even much more work and it doesn't make sense at all to rush that. Ask @mbabker if you don't believe me. I'm sure he could use some rest.
Especially with this PR here, we don't have that much pressure anymore, which is a very good thing. Pressure and lack of ressources got us into the troubles we are now.

The closest to an official word from PLT you can get till this evening is my statement that we will not release a 3.3 that shortly after 3.2.1. I'm sure other PLT members can confirm that

Why can't you release 3.3 now and a 3.4 in March or even later? Then we would even have a solid line from 3.0 to 3.5 in terms of versions. A first for Joomla!! Woohooo! ;-)

+1

SUCCESSFUL TEST.

PHP Built On:
Darwin webdev.local 13.0.0 Darwin Kernel Version 13.0.0: Thu Sep 19 22:22:27 PDT 2013; root:xnu-2422.1.72~6/RELEASE_X86_64 x86_64

Database Version:
5.5.29

PHP Version:
5.4.10

Web Server:
Apache/2.2.23 (Unix) mod_ssl/2.2.23 OpenSSL/0.9.8y DAV/2 PHP/5.4.10

WebServer to PHP Interface:
apache2handler

Joomla! Version:
Joomla! 3.2.1 Stable [ Ember ] 6-November-2013 14:00 GMT

Joomla! Platform Version:
Joomla Platform 13.1.0 Stable [ Curiosity ] 24-Apr-2013 00:00 GMT

The marketers are exhausted from pulling double duty to support CMS 3.2 and Framework 1.0. The guy pushing the release button is exhausted from supporting all of that and beginning a career change. The contributors are all getting exhausted from the non stop discussions and testing we've been asking for. And I'm sure a few spouses would like for their loved ones to step away from their computers for the holidays

Let's not rush. We have a working solution now in core and a proposal to enhance it, even if it's only short term.

" Let's not rush. We have a working solution now in core and a proposal to enhance it, even if it's only short term. "

In order to test, create a fresh Joomla installation. Then, apply this patch and log out. 
Once your logged out, check your database #__users table, you should see your 3.2 password 
in there prefixed with $2y$. 

I don't believe the password changed -- and for certain it was not prefixed with $2y$. (Installed - applied patch - logged out.)

Then log back into the site (you should have no issues logging in, if you do, the test failed, 
and leave a -1 with details about your PHP environment). 

I was able to successfully log back in.

The entry in the database table should be changed, and it will now be prefixed 
with $P$ which indicates that you password is now hashed using phpass.

Yes, my password was changed to thus:

$P$DehLtueUuyjmlDiFhLHF/b9npcBT5D0

And, could log in again and out, etc.

I think it worked fine.

MUCH appreciated.

Thanks @AmyStephen. I just realized that on PHP versions less than 5.3.7, you default to using sha256 (not crypt-sha256), so your password may have been prefixed with {SHA256} instead. I updated the instructions to reflect this other possibility.

Added Tracker: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32930

Tested by creating a Joomla install on a box using PHP 5.3.10, applied patch, then moved the site to a box using PHP 5.3.2. I did see the hashes change, after the patch was applied, and was also able to log into both sites.

I'd like to hear more about @Hackwar's concerns regarding having to use PHPass for some time. Do I understand correctly, that if we introduce PHPass now, we can't simply re-hash / migrate user password to bcrypt / password_hash() if there is a 3.3 release and the minimum version of PHP required becomes 5.3.10?

if we introduce PHPass now, we can't simply re-hash / migrate user password

As a bulk operation, this is a very basic requirement of any hashing algorithm so you are technically correct. You would have to take the approach of rehashing I understand this PR takes which is to authenticate against the old hash then write the new hash using the new algorithm value.

We can easily migrate to bcrypt / password_hash even while including PHPass now. The supposed problem is maintaining this in the long run, which I don't see as a problem.

@betweenbrain You can do the rehashing, that's not the problem. The issue he refers is that you only can do the rehashing when the user logs in, since you need the password for that. You can't bulk update the hashs.
Thus if we want to make sure all users can always log in without having to reset their password, we need to keep this library for a long time (basically forever). To take an extreme example: Jon Doe logs in with 3.2.1 and gets his password converted by PHPass. Now he doesn't log in for the next 5 years and comes back when we are at Joomla 6.0. If we deprecated PHPass by then, he would no longer be able to log in because we can't verify his password anymore. However in practice he will blame himself for forgetting the password and just reset it

So we can remove it in future. It just means that those users who didn't log in for a while have to reset their passwords.

@betweenbrain that is not correct. We can move from PHPass passwords to other formats again, but as it is with the change in 3.2.0 right now, all the changes have to happen incrementally. So each time a user with an old hash logs in, its hash gets updated to the new format. That means, that an existing website that had some manual password resets from before 3.2.0 would have a database full of unsalted MD5, salted MD5 and bcrypt hashes, all happily mixed together. Since we also introduced SHA256, it might also be that instead of bcrypt. The PR now proposes to introduce another hashing algorithm, iterated MD5 with the prefix $P$.

I was opposing this, because I wanted to keep the "variety" of hashing algorithms that we use right now limited to those 4 algos. The problem is, that you never know when all users have logged in again at least once to their account and so you might have an account with a password hash that is in SHA256 or iterated MD5, even though we are already releasing Joomla 9.0. We could almost never deprecate that code. Since I think password_hash() is the future for Joomla, I don't want to introduce code that will be obsolete in March already, and instead I propose to do a double release, first 3.2.1 to leave people with a fixed password situation and then a 3.3 with raised requirements and the switch to bcrypt.

@Hackwar I don't see the long term maintenance on this to be an issue, and the added security greatly outweighs the minimal ongoing maintenance this will require.

I agree with @Bakual on this, that while technically we could break someones password by removing this, it's more likely that they will have actually forgotten their password by that time, several years down the road, and will be resetting it anyways.

Since this has had several good tests, I'm going to put together a PR to fix this in 2.5 as well.

Then please also backport the hashPassword() and verifyPassword() methods. :-)

Why would we want to introduce the insecure {SHA256} and borked password_hash / bcrypt implementations to 2.5? I'll copy the useful parts required for rehashing the passwords from salted-MD5 up to PHPass, but definitely don't see the benefits of copying them verbatim.

That was refering to the interface. Of course we wouldn't introduce the SHA256 stuff to 2.5. But we definitely want the same methods for third party devs to use.

On Tue, Dec 10, 2013 at 2:39 PM, Hannes Papenberg
notifications@github.comwrote:

I was opposing this, because I wanted to keep the "variety" of hashing

algorithms that we use right now limited to those 4 algos. The problem is,
that you never know when all users have logged in again at least once to
their account and so you might have an account with a password hash that is
in SHA256 or iterated MD5, even though we are already releasing Joomla 9.0.
We could almost never deprecate that code.

While I understand the point, I look at it a little differently. The md5
passwords are not safe. At some point (IMO 3.3+ and 4.0+), users who have
not logged on (and therefore now have safe passwords) should reset their
password. The entire point of replacing the MD5 hash is that it's unsafe.
Preserving unsafe passwords for too long makes very little sense, IMO, past
a necessary transition period.

Since SHA256 was short-lived, I'd include those with the MD5 passwords, as
well, for simplicity.

Meaning, I'd deprecate MD5, SHA256, iterated MD5 methods/classes/code right
now for 3.3/4.0+. Then, one would remain and only test the login against
iterated MD5 for a specific period of time before it would also be removed.
Give a word to the wise.

—

Reply to this email directly or view it on GitHub#2656 (comment)
.

@Hackwar Ahh, ok. We are in agreement then for 2.5

+1 successfull test on php 5.4.21
(http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32930&start=8200)