This is related to this tracker item: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32595&start=0

I don't think we should be truncating pw's without informing the user. It's just asking for people moaning to site admins that they created an account and they can't log in with the pw they created. I think we should be keeping that check

@wilsonge I asked @ircmaxell (the one who wrote password_hash for PHP 5.5, and he said to leave well enough alone, no need to notify or truncate, and he linked me to his extended answer on Stack Exchange - http://stackoverflow.com/questions/16594613/how-to-hash-long-passwords-72-characters-with-blowfish/16597402#16597402

@wilsonge the length check+notification or truncation was really not needed, as both encryption and decryption in bcrypt truncate, the login will work just fine with bcrypt. Limiting password length for other algorythms is a wrong approach (even to 55 chars) and not needed either imho.

@Hackwar re: "Joomla should not fuzz the encryption algorithm used for the password" : I don't think Joomla fuzzes the algorithm, but there is superb mix of the standard Linux prefix for bcrypt and of own prefix for sha256 (ignoring the Linux standard one) and missing one for salted md5 (which is understandable for historical reasons). That's not nicest way, but we can live with it (and have to, since 3.2.0 has been released that way). No need to add yet another way to code same password-storage algorightms imho.

I haven't yet reviewed the proposed changes, but description of changes looks architecture-wise very reasonable.

@dongilbert thx for that find. I fixed that.

Regarding the fuzzing: We potentially have quite a few different algorithms here (even though it's unlikely, we might have to switch away from bcrypt some time in the future) and I honestly don't have the expertise to say if I can properly identify all hashing algorithms by their first x characters. That would also mean that I have to specifically check for each and every algorithm that we provide in JUserHelper::getCryptedPassword(). The way it is in this proposal, I can simply check based on that prefix.

In the end, this is only a proposal in order to get around the issue that the system more or less failed when switching between algorithms. I'm open for suggestions. ;-)

Besides that, a friend of mine suggested to not use a constant, but instead store the encryption in a class variable in JUserHelper. I wanna say that I only used a constant because I didn't want to create a new method that might in the end prove to be stupid, but would then have to be kept around for b/c reasons. So, here, too, I'm open for suggestions.

@Hackwar what's the reason for not appending the salt to md5 hashs? Appending the salts to md5 hash would result in:
1. a consistent behavior of all hashing algorithms
2. the removal of the code for handling this special case

Besides that, you missed the necessary changes in UserModelReset in the frontend (method processResetComplete) which will cause double appended salts with bcrypt and sha512

The reason was simply, that it would change the behavior of JUserHelper::getCryptedPassword(). I agree that it would be the better solution, but I didn't want to introduce a b/c issue here. If we can agree that this is acceptable, I'd be happy to change the method in that way.

I'll look through UserModelReset.

+1 for append the hash to MD5

I fixed the UserModelReset and for the record:

+1 for append the hash to MD5

Regarding the earlier proposal of a class attribute to JUserHelper (with getter and setter) for the encryption type: Any comments regarding that? I start to think that that would really be the better solution. What do you guys think? something like JUserHelper::getPasswordEncryption()/JUserHelper::setPasswordEncryption($encryption)?

Oh, and shouldn't JUserHelper::getSalt() return a random salt for md5 if none is provided?

Both ideas sound good to me!

For reference: PR #2465

Ok, after reading this discussion and going through the code that's here and being proposed, please no...

Hashing Is Not Encryption

Please stop conflating the two. Encryption is by definition a two-way process, hashing is a one-way process. Confusing the two and mis-using the names is a problem.

SHA-256 is UNACCEPTABLE

Right now, if bcrypt is not available, you're falling back to regular sha256 without a salt (hash('sha256', $password)). I can't stress how bad this. This is worse than the salted-md5 method that has been used for years.

This cannot be used. And the fact that it is in shipping software is quite bad. Beyond quite bad actually...

Most of getCryptedPassword is horrifically bad

Looking at all of the methods implemented, there are only 4 that are even close to being acceptable (and 2 are just proxies for each other). They are aprmd5 (which is also crypt-md5) and bcrypt (which is also crypt-bcrypt).

The rest are just horrifically weak hashes. And should be removed. Even crypt-md5 is weak.

Actually the entire function should be refactored to only return if a strong hash is requested (bcrypt, crypt-sha256, phpass, etc). The other "supported algorithms" should be removed entirely. Yes, they are that bad.

Suggested Fix

The best fix would be to require 5.3.7 as the minimum version. This will allow for bcrypt to be relied upon. I stress this point because almost all of 5.3 less than 5.3.9 are vulnerable to the Hash-DOS attack (and have other significant security vulnerabilities). I understand that Linux distributions ship with "patched" versions, but that's no reason to put the rest of the community at significant risk.

The second best fix (significantly far down) would be to use crypt-sha256 with a strong salt, and configured cost for those users on <= 5.3.6.

The third best fix would be to pull in PHPASS for those users on <= 5.3.6.

The crypt format includes a type specifier ($2y for bcrypt, $5$ for sha256, $P$ for phpass, etc). There's no need to store another one if you're using the standard implementation.

TL/DR

The current implementation is massively broken for users who are not on 5.3.7+. As in MUCH weaker than the J1.5 days, as it's using an unstalted sha-256, which is significantly weaker than salted-md5. This must be fixed.

@ircmaxell sha512 IS using salts. Each getCrypedPassword call provides a salt so this is fine.

There was a lot of talk about the minimum PHP version being raised - but
even just looking through the number of people who tested pw stuff in the
3.2Alpha/Beta/RC it's clear there is a significant number of users in this
region and technically it's a b/c break in our 3.x release. So it's not
going to happen. So we're going to have to start at the second best fix I'm
afraid. I think requirements are going to be raised for Joomla 4 though :)

On 19 November 2013 13:32, David Jardin notifications@github.com wrote:

@ircmaxell https://github.com/ircmaxell sha512 IS using salts. Each
getCrypedPassword call provides a salt so this is fine.

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

@SniperSister Even if it was salted, it is very much not fine. Even salted-sha512 is trivially weak. That's why iterated hashing is the recommendation (crypt-sha256, crypt-sha512, crypt-bcrypt, phpass, pbkdf2, etc).

And you're not using sha-512. You're using sha256. The call does not use a salt, so one is generated. Great. But you're still using a trivially weak hashing function (for password storage). We could go into the flaws in the genRandomPassword method, which has pretty significant bias towards a-f, but I don't think that's worth it at this point.

@wilsonge Fair enough. But Minor versions are allowed to have some backwards compatibility breaks. Just not fundamental ones. I think it could have been raised, and I think that the security implications of supporting < 5.3.9 are really significant enough that it's basically silly for a new shipping version to support anything less.

It's amazing how quickly the lessons of the GOPHP5 movement have been forgotten. Hosts will upgrade if the projects demand newer versions. If projects wait for hosts to upgrade, it will never happen. Be the change you need to see in the world. Don't play politics with the security of people's business. Everyone loses that way.

FWIW, I personally have no issue raising the minimum supported version
mid-cycle. But there are others who made better arguments for not
changing, hence the mess we're in now.

On Tuesday, November 19, 2013, George Wilson wrote:

There was a lot of talk about the minimum PHP version being raised - but
even just looking through the number of people who tested pw stuff in the
3.2Alpha/Beta/RC it's clear there is a significant number of users in this
region and technically it's a b/c break in our 3.x release. So it's not
going to happen. So we're going to have to start at the second best fix
I'm
afraid. I think requirements are going to be raised for Joomla 4 though :)

On 19 November 2013 13:32, David Jardin >
wrote:

@ircmaxell https://github.com/ircmaxell sha512 IS using salts. Each
getCrypedPassword call provides a salt so this is fine.

—
Reply to this email directly or view it on GitHub<
https://github.com/joomla/joomla-cms/pull/2555#issuecomment-28789772>
.

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

I'd say that this one of those areas where a B/C break might be necessary, and is certainly justified. The next LTS has not been released yet, so now is the time to do this.

Don't play politics with the security of people's business. Everyone loses that way.

Please heed the words of @ircmaxell, he is an expert in this realm. Thank you for your help Anthony.

The "improvement" on passwords storage should certainly not have been added as is (I had raised that several times on trackers and in private) and a real security-review (including a rewrite) is needed asap (even a paid-for security review, OSM will probably agree to paying that) as it's already released. Yes, iterations are needed if not using bcrypt (which has them inside).

I have no issues leaving hosts which are <php 5.3.9 on md5-salt (or an iterated one), and have an orange warning in backend panel "Your PHP version is below minimum version and thus security is not optimal".

As Linux distributions power the majority of hosters, we can't completely ignore them.
Ubuntu 12.04 (latest LTS) has php 5.3.10+patches, so for Ubuntu servers it's ok to raise the bar. We need to look at CentOS which is much used and at Debian and then we can make a decision (but as both have LTS distributions which are younger, I think it should be ok for all 3).

@ircmaxell thanks for your input. That is why I asked for feedback. Your remarks are almost entirely aimed at JUserHelper::getCryptedPassword(). Where do we go from here?

I would propose the following:

1. Do we allow people to decide which hashing algorithm to use?
2. We add a JUserHelper::getHashingAlgorithm()/JUserHelper::setHashingAlgorithm($algorithm) method that lets us set the algorithm to use throughout the code without that constant that I introduced.
3. JUserHelper::getCryptedPassword() should always return the complete hashed and salted md5 string when md5-hex is selected, like we are using up to 3.1.
4. We introduce a new md5 method with iterations like requested here. This will be the fallback for bcrypt.
5. JUserHelper::getSalt() is changed to return a random salt for MD5 if no salt is given.
6. ...

Now the question from my side is, if we can seperate the issues from JUserHelper from the rest of the issues that we have in this PR.

Funny thing to keep in mind: If we really drop sh256 as hashing method fallback for bcrypt, what do those people do that updated to 3.2.0 and have people limping along on that algorithm?

Should Joomla! pull the current 3.2.0 release in order to prevent further problems?

I'm willing to invest more time into this in order to get this right, but I'd like to get consensus first on which direction to go.

I think Anthony's comments here further reinforce our need to evaluate our
minimum supported version. Anyone who was testing through the betas knows
the headaches that have gone into supporting those old (and insecure)
versions of PHP. At some point, we owe it to ourselves to make a
responsible decision about how much we are willing to support. Since the 3
series hasn't hit our LTS mark yet, I'm more inclined to change the support
minimum now than wait until say 3.5.3.

1. Do we allow people to decide which hashing algorithm to use?

Are there valid use cases with using that code separate from password
hashing? If not, I'd support 2 methods only so we make a fallback
available if for some reason the primary won't work.

I'm willing to invest more time into this in order to get this right, but
I'd like to get consensus first on which direction to go.

Let's talk it out and get it right once and for all.

I'm hesitant to only allow md5 and bcrypt, since this assumes that bcrypt will be secure for the forseeable future. Considering the NSA leaks in the last few months, I simply don't really trust any algorithm to be safe enough to never be replaced by something else, so I would rather have a system in place that lets us easily add hashing algorithms to our setup than to having to refactor this again in 3 years or something to add a third method...

I am, however, open to be overruled by the majority. :-)

(Yes, I know that bcrypt is pretty safe and I also know quite a bit about the inner workings of this algorithm, but that still makes me wanna go on the safe side here.)

My $0.02 (or £/€/etc)

Security should default to the best available (or at least the most suitable) on the installed platform, no user decisions to downgrade should ever be available. Give people a choice and they will exercise it, even when it is not in their (or their users) best interest.

Have a fallback as a contingency, with a notification to the site admin that security is not optimal.

Are there any hard facts about what PHP version(s) are being used in the wild for J3.x?
Are there any hard facts about the PHP version(s) being used by the top 20 hosts (by popularity)?
Stratify the numbers and see where the cut-offs would be.

Given that 5.3 is in its end of life cycle (as of March 2013) - see http://php.net/archive/2012.php#id2012-12-20-1 - is it worth supporting 5.3.x at all?

If it is possible to abstract the cryptography away from the encrypt password function it will be possible, if necessary (some time in the future if the NSA / GCHQ / anyone / etc. crack bcrypt), to replace the primary and secondary (and tertiary) encryption methods without affecting anything the users (or devs) see in terms of the API.

bcrypt is pretty safe, for now, but in 2 years will it still be?

If some clever bods can make it so that the encryption method is transparent to everyone other than the core password function(s), then any solutions can be added later without affecting backwards compatibility. If you read that last bit quickly it sounds easy, but I doubt it is.

I do know that implementing security / cryptography is easy to screw up and very very very hard to do well - so I am all for listening to the experts and implementing their recommendations. And if that advice costs money then so be it - it would make a great marketing point if Joomla! has implemented the security recomendations of . That doesn't mean that @Hackwar and @ircmaxell 's (and others) contributions, knowledge and experience is any less valuable just because they give it freely.

I know enough about security and encryption to know I shouldn't touch it, but I am happy to play the 'idiot user' or 'smart user' for testing anything in this area.

Chris.

As general rule: Safest and easiest approach is to follow the recommended algorithms of latest Linux kernels.

There are 2 algorithms that are considered "safe" and "battle-proven" today: bcrypt which was developed independantly from NSA (with a follow-up promising scrypt algorithm that requires more RAM than bcrypt, and thus more hardware for FPGA-based attacks) and PBKDF2 which is NIST-approved for US-government use (using RSA algorithms and went through NSA, so very hard to tell how safe it is).

Thus bcrypt is nowadays generally considered as a safe approach, provided the iterations ("cost" the option for php's password_has() function) are high enough (hint: you could even have a higher number of iterations for super-admin accounts). Actually some systems like OsX measure CPU power to adjust the number of iterations, and thus the iterations-based security improves automatically with CPUs while keeping login-times (CPU use and number of logins possible per second reasonable).

A highly-iterated (>10000) salted-md5 or salted-SHA could be fallback, preferably with a custom bits-permutation between 2 series of iterations to make existing rainbow-tables not directly usable.

In all cases, we should always prefer using battle-proven libraries than inventing our own ones, as usually security flaws are not inside the algorithm, but in its implementation!

I don't think anyone is questioning if we should run our own implementations. We simply wont. But the question is, if we implement a system that allows to set a broader range of algorithms or if we hardcode it to "old-MD5", "new-MD5" and bcrypt?

I highly recommend that you all read the PHP RFC for Simplified Password Hashing. It covers basically all of this.

Basically, the password_hash() API was designed so that it could be internally and transparently extended with future algorithms as needed.

If you use the API properly (hint: it's not implemented correctly here), then upgrades over time will be transparent to the end-user and the application. As new and stronger hashing algorithms are implemented, PHP will automagically upgrade to them as it can (assuming you use PASSWORD_DEFAULT). The RFC clearly explains the conditions around this. It is also explained on the password_hash() doc page.

As far as "if a flaw is found in bcrypt":

Yes, it is possible and likely that flaws will be found. Flaws already exist that we know about. But at this point the chance of a critical "all your data is trivially lost" flaw is quite low. The algorithm has been around for a very long time, and has been reviewed by a lot of people.

So the chances of a major break are quite low. It is possible, but the password hashing API already deals with this effectively enough where you shouldn't have to worry about it. It was designed specifically for that reason. If there is something it doesn't do, than discuss that. But to re-invent its functionality just because you can or want to...

Remember, the central enemy of security is complexity. Keep your side simple, and let's house the complexity centrally in PHP where it can be reviewed a lot easier. Where it can be maintained a lot easier.

Remember Cryptography Is Hard. If a library exists that does what you need it to, use the library. Especially in this case where the library is maintained along side of core.

And no, please don't invent your own iteration strategies. If we were talking about a one-off custom implementation that would be irresponsible. We're talking about a distributed piece of open source software...

Ok, ok, you convinced me already. ;-) I'll see that I'll update the PR.

With the input from @ircmaxell, I changed the code a bit. As far as I understand the RFC, this is the way the password_hash() lib is supposed to be used and we should be forward proof and backwards compatible.

If you all sign this of, we only have to take a look what we do with JUserHelper::getCryptedPassword() and getSalt(). Both are either useless or dangerous or both, according to @ircmaxell, so we might consider deprecating them and replacing their content with a JLog.

Whatever we do, we must keep backwards compatibility to existing hashed passwords, and to APIs as much as possible.

getCryptedPassword() and getSalt() are part of API and first one at least should/coud use password_hash() ?

Yes, we must keep b/c if possible, but if the API is wrong and by that dangerous, since it does not do what it claims to do, we should think about removing it rather than letting people run into their demise.

We are already using password_hash() in getCryptedPassword(). But there are so many issues in this method, that it is nearly impossible not to break b/c in order to fix them. Starting with the missing md5 implementation right now. And even if we implement md5 again, it would still not be correct, since the hashes it produces are not the end-result that we would expect from that method. I would expect to be able to hand in a password and salt and to get a hash in return. getCryptedPassword() only returned the md5 string, without the salt, so you would have to have additional steps to get a complete hash. Fixing that would break all existing implementations. Considering that it is unsafe anyway, we might as well scrap it completely.

There should be 3 functions for the entire process: getSecurePassword, getCryptedPassword, and getSalt.

getCryptedPassword should do the simple task of taking a plaintext and applying whatever hashing method is being used. This is how it has always functioned. Changing it otherwise breaks backwards compatibility.

getSalt should generate a salt like it always has (why change this? no need..).

Finally a new function called getSecurePassword would do the final password combination. This means appending the salt. So in case of SHA256, MD5, etc.. this is where we'd add ":$salt". For hashing methods that don't need a salt it simply isn't appended in this stage (like bcrypt, we'd just return the hashed password). This used to be done in the ->bind of the user object, which now would just call getSecurePassword. 3rd party extensions would call bind, pull out the hashed password, then store to their system or call getCryptPassword and getSalt then add together. getSecurePassword can then be used to do all of that automatically.

In short this guarantees backwards compatibility. Previous methods would still work and new usages can just use getSecurePassword.

At this point you'd probably want a 4th function to handle verification. For example verifyPassword( PASSWORD, PLAINTEXT ). This should handle comparing a typed password (maybe from login? doesn't matter where..) to the already hashed password to determine if it's correct or not.

But why should we repeat the code in every place that we need password hashing? Why not return the complete hash directly? Besides that, as @ircmaxell rightfully stated, PHP has password_hash(), which does the complete hashing and verification for us AND is secure AND future proof.

@ircmaxell, could you review the latest changes?

I would do what @elinw suggested a while ago: to deprecate all the current functions and move to a simpler API which follows closely the new password functions in PHP. Of course we need to deal also with the current passwords, but those should really be converted to the new ones when user logs in, see: password_needs_rehash().

So the old passwords would still need the current functions to validate, but they shouldn't be used to generate new passwords. For this I would split the authentication plugin into two: the one that handles only the new hashes and another one to convert legacy passwords. That would allow administrator to disable weak hashes from the site -- feature which would be turned off in the new sites and something that I would do after 6-12 months in all of my own sites.

Here's the current usage for each PHP version:
http://w3techs.com/technologies/details/pl-php/5.3/all

And this lists PHP versions used in a few distros:
http://www.sasaprolic.com/2013/02/list-of-current-php-version-in-major.html

And yes, I would drop the support from oldest (<5.3.10) PHP versions in J!3.5 -- even if they are used in 27% of hosts that are running PHP 5.3. The reason is that even if the distros use patched versions of PHP most users do not apply security updates to their sites. And if they do use cpanel or plesk, they probably have a later version anyway...

I would be happy to start the iniative in the upcoming Kunena 3.1 and set the minimum PHP version from 5.3.1 to 5.3.10 in all sites, including in Joomla! 2.5. If the version is older than that, installation will just fail with an error telling users to upgrade.

Please look at the current code of this PR

Already did. I have troubles on understranding what the original crypto code (before all the crypto changes in J!3.2) was supposed to do, which is the reason why I just gave up on trying to review the code. I cannot review something which I don't fully understand.

We really need to replace the current fucntions with something that's both simple to use and follows PHP implementation without adding any legacy on top of it.

Which the current PR would do. What I wanted to say was, that there is no gain here to wrap password_hash() and password_verify in some Joomla class or something. The RFC is very good here and keeps all the necessary stuff in mind, so no need to do anything else but to use the functions that are there.

We need to wrap them to account for legacy stored passwords and for backwards-compatibility. We managed to keep B/C on this from 1.5 until 3.1, and suddenly it broke in 3.2. Now important extensions have adapted and there is a proposal to break B/C unneededly ? I don't get it. What's wrong with @krileon's proposal ?

At some point in future, the PHP password_hash() function and related ones might (most probably will) not be secure (at least in some php versions) and without wrapping functions, we will have no API for compatible passwords, and we will be breaking again B/C of APIs. By careful planing today the correct wrapper functions (and making existing ones B/C compatible) we can avoid this easily. Proposal: just create 1:1 wrapper functions to the php ones, which also handle legacy passwords already stored.

We CAN'T create a wrapper function that handles both, because Joomla never HAD such a function. Instead Joomla had to do the whole password hashing stuff manually at each and every place where needed. No, JUserHelper::getCryptedPassword() does NOT handle that. The result of that function was always only the result of md5($password . $salt) and not md5($password . $salt) . ':' . $salt.

password_hash() is secure now and if PHP ever has a regression that makes it unsecure, all old versions of Joomla will be unsecure, too. So then you need to create an exception for some PHP versions and that means that you have to provide your own implementation of password_hash(), which I seriously don't want to provide. Because providing that would mean that we would have to implement crypt() in PHP again in a way that is safe. I've read several books and RFCs about encryption/hashing algorithms and saw enough bad examples that I know that this project is not capable of shipping its own secure hashing algorithm (implementation). So simply spoken: If PHP screws up in this area, we are screwed anyway.

Regarding your question, what is wrong with @krileon's proposal: We don't have a wrapper function right now. Whatever we do, be it changing JUserHelper::getCryptedPassword(), creating new wrapper methods in some class or using the PHP built in functions, we break backwards compatibility to extensions messing around with this. Why I don't want to introduce those wrapper functions? password_hash() and password_verify() ARE those wrapper functions! I know that Joomla loves to reinvent the wheel. I do, too. But this is one of those cases where it is simply stupid to do so, from an economical point, from a technical point, from a security wise point and from a convenience point for users.

@beat - Regarding this:

The "improvement" on passwords storage should certainly not have been added as is (I had raised that several times on trackers and in private)

I do remember you raising this point a few times, but as far as I remember, there wasn't much else in terms of code or discussion beyond you saying that you had issues with the code. If you have a suggestion (and code), please share it. Simply saying a "real security-review (including a rewrite) is needed asap" isn't very helpful at this point; most everyone reading this thread realizes we have issues with the code right now, many of them self imposed.

Which brings me to point number two, and that's the B/C claims. For all intents and purposes, I don't see a way to retain any resemblance of B/C at this point, and just dropping it isn't an option either (there have been over 225,000 downloads of the 3.2 packages). Our ONLY option at this point is to just get it right this time around.

As far as wrapper methods go, why? What exactly are we intending to wrap? We have a habit for implementing code ourselves for the sake of doing it instead of using what's in core PHP (or B/C libraries like @ircmaxell and password_hash()) and we need to break that habit. Why do we still have JObject when stdClass works just fine!?

As the guy who is going to catch most of the hell for whatever comes out of this by means of the pretty little PLT badge I've been issued, to me we have two issues to solve: fixing our password hashing and raising our minimum supported PHP version so we don't end up with another mess like what we're fixing right now.

Holy bike-shed batman.

It doesn't have to be this hard. It really doesn't. Let's walk through it logically:

Does BC Need To Break

The current API has a single method for both creating and verifying hashes. This would require splitting logic all over the place (splitting salts, etc). This specifically leads to all of the complexity that's been talked about here.

Additionally, neither of the existing method signatures is appropriate for future hashing.

So Yes, BC Does need to break to split creation from verification. And it needs to break to actually slim down to what's needed.

Also remember that this is not a public API that people are supposed to be extending. There may be a few extension developers that do so, but the majority of developers will never actually hit this API directly. So to change this API, which is needed for security reasons, isn't that impactful of a break. A little PR will go a long way here, and doing it to improve security will be a significant win...

Keep It Simple?

If you want to keep it simple, you could create 2 simple methods:

class Whatever {
    const STATUS_FAILURE = 0;
    const STATUS_SUCCESS = 1;
    const STATUS_NEEDS_REHASH = 2;
    public function createHash($password) {
        return password_hash($password, PASSWORD_BCRYPT, $this->options);
    }
    public function verifyHash($password, $hash) {
        if (detectMd5($hash)) {
             if ($this->verifyMd5($password, $hash)) {
                 return self::STATUS_NEEDS_REHASH;
             }
             return self::STATUS_FAILURE;
         } elseif (detectWhatever($hash)) {
             //etc
         if (password_verify($password, $hash)) {
             if (password_needs_rehash($hash, PASSWORD_BCRYPT, $this->options)) {
                 return self::STATUS_NEEDS_REHASH;
             }
             return self::STATUS_SUCCESS;
        }
        return self::STATUS_FAILURE;
    }

This isolates all behavior centrally to the class. If a re-hash is needed, the caller is told and can re-create the hash.

This is simple, and straight forward. Additionally, it removes the ability to create weak hashes. Which is a good thing. It also creates the proper abstraction should you ever wish to move away from password_hash...

It doesn't need to be exactly like this. This just shows the flow and logic between splitting creation and hashing, and retaining methods for communicating that a hash needs to be updated...

Make It Flexible?

The other alternative would be similar to what I did in PasswordLib. Basically, have a series of strategy classes, and delegate responsibility to them. This is not difficult, and it does have advantages.

I would highly recommend keeping it simple. Otherwise you risk someone registering a black-hole password implementation via a plugin, which would then let them do funky things (like return true for verify if a specific password is set, giving them a nice backdoor).

Now, granted, if they can get code on the server it's done. But that's no reason to allow for flexibility where it's not needed and potentially exposing yet another attack vector.

As far as getSalt

As far as the getSalt method, it should not be public, yet alone exist. In the simple situation, it's not needed because all password hashes are created via password_hash which creates its own salt. I would suggest removing it as it's not necessary and isn't the best random string generator anyway...

Could we get back to the problem at hand? I proposed code that fixes our password system, validates old passwords and then rehashes to bcrypt. The whole thing is also forward "proof" as far as I can tell, for a time when PHP does not use bcrypt, but scrypt or something like that. It should automatically adapt in that case.

This leaves 3 questions from my POV:

1. What to do with JUserHelper::getCryptedPassword()? It seems as if that method does more harm than good and we should probably either let it throw a fatal error or return immediately and add a JLog entry. Most likely the same with JUserHelper::getSalt().
2. What to do with the minimum system requirements? Do we raise those to 5.3.7? If yes, do we do this in this PR or in a seperate issue?
3. Will password_hash() always return strings that start with $2y$? Because that is how we currently distinguish hashes from password_hash() from old md5 stuff. In that case, we are golden.

I would not add yet another class like the above mentioned by @ircmaxell. Please look at the code, since I think its already implemented in a pretty nice way. :-)

1. What to do with JUserHelper::getCryptedPassword()? It seems as if that method does more harm than good and we should probably either let it throw a fatal error or return immediately and add a JLog entry. Most likely the same with JUserHelper::getSalt().

If their functionality isn't needed and is insecure as is, deprecate them,
drop the code, super bonus points if they can proxy into good code but I
wouldn't set a requirement on it. I'm still not totally familiar with the
code, but from my reading and the conversations here, that sounds like a
sound plan.

1. What to do with the minimum system requirements? Do we raise those to 5.3.7? If yes, do we do this in this PR or in a seperate issue?

Personally, I'd raise the minimum. But that's something that if we're
going to do, it needs to be taken to list today and decided quickly if
we're still aiming for a Nov 26 release to fix these issues. Though I'm
not leaving that date set in stone either, but our release window isn't
that great looking for December so as much as I'd rather not rush a major
decision, it does have to be kept in mind.

I would not add yet another class like the above mentioned by @ircmaxellhttps://github.com/ircmaxell.
Please look at the code, since I think its already implemented in a pretty
nice way. :-)

If JUserHelper is doing what we need, awesome. If not, add whatever
methods we do need to it. But totally agree on not adding a whole new
class.

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

• Michael

Please pardon any errors, this message was sent from my iPhone.

I have one little question, which might be a stupid remark not relevant to this discussion. Does the possibility to code your own (cascading) authentication-plugin in Joomla stay unchanged with all those proposals? If so, then we can still authenticate users with custom methods, isn't it?

I would be happy to start the initiative in the upcoming Kunena 3.1 and set the minimum PHP version from 5.3.1 to 5.3.10 in all sites, including in Joomla! 2.5. If the version is older than that, installation will just fail with an error telling users to upgrade.

@mahagr thanks for stepping up to help push Joomla forward. I do appreciate that. As @ircmaxell said:

Hosts will upgrade if the projects demand newer versions.

I do realize this may cause some people pain, but it is in the sake of better security, and I personally support that.

I have troubles on understanding what the original crypto code (before all the crypto changes in J!3.2) was supposed to do,.. I cannot review something which I don't fully understand.

There is a lot to be said for this. If core code can't be understood, it cannot be evaluated, let alone used. I'm a big fan for keeping things, simple...

We really need to replace the current functions with something that's both simple to use and follows PHP implementation without adding any legacy on top of it.

If you want to keep it simple, you could create 2 simple methods

And this makes the most sense to me as it is simple, effective.

We have a habit for implementing code ourselves for the sake of doing it instead of using what's in core PHP

Which is something that has always baffled me. I'd personally like to see use get away from that.

Please look at the code, since I think its already implemented in a pretty nice way. :-)

@Hackwar - I have. And it is seemingly over complicated as compared to what @ircmaxell suggsted at #2555 (comment). We need to think about maintainability in regards to these issues as well, and if we can implement something that is easily understood and maintained by more people, that seems like a win to me.

If JUserHelper is doing what we need, awesome. If not, add whatever methods we do need to it. But totally agree on not adding a whole new class.

That makes sense to me if it can be done.

Custom authentication methods shouldn't be lost at all.

On Wednesday, November 20, 2013, HermanPeeren wrote:

I have one little question, which might be a stupid remark not relevant to
this discussion. Does the possibility to code your own (cascading)
authentication-plugin in Joomla stay unchanged with all those proposals? If
so, then we can still authenticate users with custom methods, isn't it?

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

• Michael

Please pardon any errors, this message was sent from my iPhone.

In a quick chat with @ircmaxell, he convinced me, that we should move this to some proper methods and I will propose something in the next few hours. I will also delete the two methods that we talked about in the last few comments.

@mbabker can you bring the raised requirements up on the mailinglist?

@HermanPeeren as @mbabker said, the custom authentication methods are still there.

I'd appreciate if someone else could start said thread. I'm stuck in more
boring briefings through the rest of the day. Otherwise, I'll get to it at
lunch.

On Wednesday, November 20, 2013, Hannes Papenberg wrote:

In a quick chat with @ircmaxell https://github.com/ircmaxell, he
convinced me, that we should move this to some proper methods and I will
propose something in the next few hours. I will also delete the two methods
that we talked about in the last few comments.

@mbabker https://github.com/mbabker can you bring the raised
requirements up on the mailinglist?

@HermanPeeren https://github.com/HermanPeeren as @mbabkerhttps://github.com/mbabkersaid, the custom authentication methods are still there.

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

• Michael

Please pardon any errors, this message was sent from my iPhone.

@mbabker PHP 5.3.10 minimum requirement (with check in panel, could be same place as for not-up-to-date Joomla version message) is very reasonable at this time imho. Mailing-list thread started as asked:
https://groups.google.com/forum/#!topic/joomla-dev-cms/GRlyfBfbMpU

@Hackwar : $2y$ prefix means bcrypt, this is a GNU/Linux convention: http://en.wikipedia.org/wiki/Passwd : "$id$salt$hashed", the printable form of a password hash as produced by crypt (C), where "$id" is the algorithm used. (On GNU/Linux, "$1$" stands for MD5, "$2a$" is Blowfish, "$2y$" is Blowfish (correct handling of 8-bit chars), "$5$" is SHA-256 and "$6$" is SHA-512.

All: Regarding the huge and unneeded (and now with PHP 5.3.10 minimum even less needed) complexity added for this simple feature by #1745 that also mixed in remember-me upgrade in a complex way and broke both, I would suggest a radical approach to revert PR1745 (which would bring us back to a solid long-time security-scrutinized code-base), and re-implement both features as separate PRs the simplest possible way, for Passwords computations like @ircmaxell suggested it, and for Remember-me function simplest way too. That way we can start to proprely review the security without huge time needed for an over-complex implementation which spreads wide into too many places things that should be as the same place for security review. (that's my 2cents on this subject).

I think that patched versions of debian had a different prefix to $2y$ but still did bcrypt securely - which is why this test was added: a274026#diff-dfbab2dcfdfdf301f4f3aecef7bc5b76R14

I should add I'm running on my 5th day with minimal sleep so I may be remembering this wrong!!

@wilsonge Thanks for pointing that out.

Really? Patching libraries? That are maintained that way for a very specific reason? Way to go...

I really accepted that!? Excuse me while I go hang my picture on the wall
of shame...

On Wednesday, November 20, 2013, Anthony Ferrara wrote:

@wilsonge https://github.com/wilsonge Thanks for pointing that out.

Really? Patching libraries? That are maintained that way for a very
specific reason? Way to go...

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

• Michael

Please pardon any errors, this message was sent from my iPhone.

I'm going to throw this out there (and probably go run to my nuclear bunker)

But if we really wanna keep 5.3.2-7 and are happy to loose b/c we could implement something like this https://github.com/ircmaxell/PHP-PasswordLib and completely abstract it all.

I'm sure @ircmaxell could explain more as it's his baby - but it supports old Joomla MD5 usage and bcrypt plus a few and leaves it abstract for whatever the next encryption scheme will be.

Major positive to me of using something like this is it's well unit tested for the future and well written. Major negative we need to deal with the namespaces - I'm not sure what stage @dongilbert got with JLoader and dealing with that.

You missed the commits where JLoader supports PSR-0 and namespaced class
loading then. 3.2 is ready to go with all that stuff.

On Wednesday, November 20, 2013, George Wilson wrote:

I'm going to throw this out there (and probably go run to my nuclear
bunker)

But if we really wanna keep 5.3.2-7 and are happy to loose b/c we could
implement something like this https://github.com/ircmaxell/PHP-PasswordLiband completely abstract it all.

I'm sure @ircmaxell https://github.com/ircmaxell could explain more as
it's his baby - but it supports old Joomla MD5 usage and bcrypt plus a few
and leaves it abstract for whatever the next encryption scheme will be.

Major positive to me of using something like this is it's well unit tested
for the future and well written. Major negative we need to deal with the
namespaces - I'm not sure what stage @dongilberthttps://github.com/dongilbertgot with JLoader and dealing with that.

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

In which case then do we consider abstracting it out with a library like that? Unit test coverage good. Easily accessible to 3PD's and supports the PHP versions we need to support.....

I'm personally the wrong person to ask that of. But if it's doing what we
need and making our lives simple by using it, why not.

On Wednesday, November 20, 2013, George Wilson wrote:

In which case then do we consider abstracting it out with a library like
that? Unit test coverage good. Easily accessible to 3PD's and supports the
PHP versions we need to support.....

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

I saw during my security-review-attempt some of that library already (re)factored into Joomla 3.2: https://github.com/joomla/joomla-cms/blob/master/libraries/compat/password/LICENSE.md in that huge #1745 .

Instead of that PR mess, all password hashing and verification should be nicely grouped into a single generic class (like @ircmaxell proposes it, that is very simple to security-review, then have simple documented uses of that library at clearly marked places.

At this time, except that $user->bind() should hash the password field if there is one like it always did, B/C should not be our priority. Simplification to the maximum in a way that makes a code security review easy. Then we can take a second look to see if B/C can be improved if even needed.

Finally, if we agree to continue to support PHP 5.3.2-9 a B/C "compat" way of supporting it and existing Joomla 3.2.0 installs should be ok.

Would that be a plan forward ?

Ok, based on the feedbacks of the groups-question, and as concluded in the groups , we should not raise PHP minimum requirement for Joomla 3.2.1, and if even needed, not above PHP 5.3.3.

I am proposing to stay backwards-compatible for existing password hashes (except for very old Joomla 1.0 non-salted-md5-passwords which can be reset anyway), but to from now on use exclusively the 2 best battle-proven algorithms:

1. bcrypt with PHP's password_hash() functions on installations using PHP >= 5.3.10
2. the similarly-secure ­­-- NIST-recommended -- standard PBKDF2 alternative when PHP is < 5.3.10.

https://defuse.ca/php-pbkdf2.htm is a battle-proven, well tested, very simple to use and review, PHP 5.3.1-compatible, implementation of PBKDF2, and there are examples of using it in the php.net comments of hash_pbkdf2() function which is directly used instead on PHP >=5.5.0.

It is also possible to use @ircmaxell 's library to use just PBKDF2, as it implements it too, but is quite more complex than the defuse one, as it brings many other features with it.

That way, we solve the fallback implementation for PHP < 5.3.10 and keep the hashing security to reasonably high standards. :-)

• What do you think regarding the proposal of fall-back algorythm ?
• What do you think regarding the 2 possible implementations ?

And again: I disagree with your assumption that the discussion is over. See my response on the mailinglist.

BTW: This does not solve the issue of sites switching hosts and randomly landing on unsupported PHP versions. "Oh, when would that happen? Nobody would be that stupid!" Except for developers that set up a site on their up-to-date system and then move it over to the customers host, which is not. This is not a question of if we use memcache or filesystem for caching, it is a bit more complex. And since shit already hit the fan, we also have some more considerations to make. Right now, this to me looks like the worst incident that has happened so far in the Joomla project. Every other bug, security vulnerability or WSOD could be simply fixed by fixing the code and be happy. This one is not solvable that easily.

@Hackwar :

Replied to your post on the mailing list. Let's keep mailing-list for PHP version, and this discussion for password hashing algorithms please. ;-)

Also, let's keep cool, we do no need to make a drama out of this. It is not a vulnerability by itself. You still need a vulnerability (e.g. SQL-injection one) to be able to take advantage of the SHA256 less-secure-than-bcrypt password storage. bcrypt on php 5.3.10+ is still state of the art security. ;-)

Your point about moving from higher php-version to lower one is a good one, but that is already an issue in Joomal 3.2.0's implementation. ;-)

And? Of course its an issue in 3.2.0! As I wrote, this is a huge fuck up. And even if we need another vulnerability to get to the data: This is still a huge gaping hole! Just because you gotta climb over the fence to go to the vault and steal the gold, doesn't mean that you can keep the safe unlocked. Let me give you another analogy: Of course you need to have an accident to hurt yourself in your car, but that doesn't mean that we don't put on the seatbelts when we don't have an accident. So please stop lulling people into a false sense of security here.

@beat that library Elin included is just https://github.com/ircmaxell/password_compat the password compat function - different library (with that added 'fix')

I think we probably have to live on the assumption that majority of people staying back on 5.3.3 are people on linux lts versions and are unlikely if they haven't changed yet to upgrade. I mean even my rubbish massively cheap host I use is on 5.3.22. So I think we should take the risk of leaving that as a 'known bug'. Don't forget we have already changed the recommended version of php to 5.4 as well.

@wilsonge Thanks for the clarification

@Hackwar Keep in mind that if someone can access your SQL database through a different SQL-injection vulnerability to read the hashed password, he can most probably also update it by the same way (and thus put his own hash even with state-of-the-art bcrypt). So strong hashing won't help to mitigate another vulnerability as an site-attack vector.

Also, I think it's perfectly ok that users have to reset their passwords when you move your site to a crappier hoster. The particular case of a developer moving a site from his dev environment to a crappy host will only affect his own password(s), so perfectly acceptable too. So no real issue there either, and 3.2.0 handles that ok already imho.

Let's be pragmatic and simply fix it for 3.2.1 by going the proposed PR way of here, with the improved API proposed by @ircmaxell and considering PBKDF2 as a fallback instead of salted-SHA256.

Any other way would considerably delay 3.2.1 without adding any more algorythmic security, and it would mean a huge implementation and test effort.

After talking to Elin I have come into conclusion that it's possible to implement bcrypt safely even on the broken PHP versions (with the old hashing $2a$ or $2x$).

It will still raise minimum PHP version into 5.3.3, but that would only affect ~2% of the users, which I find reasonable as that would allow strong passwords for all users by using a single implementation -- albeit two variations of it.

Here is the bug that caused the vulnerability:

"Versions of jBCrypt before 0.3 suffered from a bug related to character encoding that substantially reduced the entropy of hashed passwords containing non US-ASCII characters. An incorrect encoding step transparently replaced such characters by '?' prior to hashing. In the worst case of a password consisting solely of non-US-ASCII characters, this would cause its hash to be equivalent to all other such passwords of the same length."

The fix is very simple. On the affected systems we can keep most of the entropy just by filtering the highest bit ($char & 0x7f and perhaps dealing with 0x00) for all letters inside the password. Or the password could be base64 encoded before passing it. That would make the passwords containing non-ascii characters almost as strong as in the fixed versions. This special case would be easy to detect even after upgrading the PHP as they have different prefix. Also most passwords would not be affected at all, because of most of the (western) people do not use non-ascii characters in their passwords.

This also rises another question: do you allow people to use UTF8 in their passwords? If you don't, there's no vulnerability in here...

So the question is: is it possible to raise minimum version requirement from PHP 5.3.1 to 5.3.3, which is the lowest supported PHP version in all current distributions?

@beat No one is talking about preventing a hack. None of the hashing is done to prevent a hack. The hashing is only done to protect the data of our users and I don't know suiss laws, but if you handle data recklessly in germany, they might make you liable. So don't act as if this is no big deal, because in that case we might as well not change anything at all.
Oh, and if devs don't count, then expect them to move to another system that values them. Seriously, who do you think is using our system here? Even if they call themselfs developers, they are not Joomla core developers. They don't follow these discussions here and they don't know the issues. And we already have the forums flooded with people that have issues with 3.2.0. You seriously want to have those popping up for the next 2 years?

@mahagr Sorry, but I can't trust Elin in this regard. It's not only the mess that we have with the hashes themselfs, that she caused, but also all the code around it. Truncating passwords, implementing a convoluted bcrypt system to have a remember me cookie and not pushing for a proper review of this absolutely vital code, are taking her out of the equation here. We are not talking about missing entropy here. We are talking about password_hash() returning the salt instead of a hash in bad situations.

@Hackwar laws say to protect all user data, not only passwords, so you should be ok in Germany, and I'm ok in Switzerland as long as we keep our hosting and sites safe. :-)

Even a state-of-the-art bcrypt hash of a 6-characters password can be brute-force guessed without huge budget and long time.

I'm not trying to minimize the issue, just to put it in relation with much worse vulnerabilities.

As a matter of fact I did already raise it before it got merged but didn't have the free time available to do another PR proposal due to the my other 3.2-tasks. Also a simpler private proposal made for rememberme improvements got refused at that time, so it didn't encourage me to spend time and fighting energy there in addition of other 3.2-areas.

I made a few proposals to move forward for 3.2.1 here #2555 (comment) , here #2555 (comment) and here #2555 (comment)

So what do you and others think of my 3 proposals, and what do you propose ? :-)

All I asked from Elin was what is the crypt bug and if there's more information about it. Rest of the text came from me. I'm not a security expert myself (and to me it looks that we only have one in this discussion), but I was working in a security company with security experts for a few years.

I'm also not talking about the current implementation (yes, it does need to get fixed), but to use a single solution that works in all supported systems -- making the implementation simpler and safer to use.

If crypto algorithm had a flaw in it for PHP 5.3.3 - 5.3.7 and we still want to support those versions, we need to know how the algorithm was broken. If the issue was non-ASCII letters converted to ?, the solution to keep the entropy is to encode the letters so that the entropy will remain the same.

The only side effect from using base64 encoded string is that instead of having maximum password length of 55 the limit lowers to 41 characters. Which is longer than 99.9% of the passwords anyway.

The other solution to filter the bytes would also work, but they would slightly lower the entropy and there might be some special characters which "are as bad as using the 8th bit".

The third solution is to prevent those bad characters from being used in the passwords. If those are already prevented, there wouldn't be any issues with entropy loss and thus no real reason trying to fix the issue either.

Thanks @mahagr , questioning to understand the exact bug that was hindering bcrypt in PHP 5.3.7- is a sound approach

Also the first solution that you propose seems almost too simple too be true Wonder why it didn't get implemented instead of the much more complicated and less secure SH256 one...

Somehow I recall the $2a$ prefix was not linked to a PHP version, but to a bug in the crypt() library ;-)

So here are the details:

"$2a$" is documented in wikipedia here and was announced here under CVE-2011-2483 :

crypt_blowfish before 1.1, as used in PHP before 5.3.7 on certain platforms, PostgreSQL before 8.4.9, and other products, does not properly handle 8-bit characters, which makes it easier for context-dependent attackers to determine a cleartext password by leveraging knowledge of a password hash.

The PHP 5.3.7 fix details are documented in this PHP 5.3.7 security advisory and summarized there as follows:

Summary: for passwords without characters with the 8th bit set, there's no issue, all three prefixes work exactly the same. For occasional passwords with characters with the 8th bit set, if the app prefers security and correctness over backwards compatibility, no action is needed - just upgrade to new PHP and use its new behavior (with $2a$). However, if an app install admin truly prefers backwards compatibility over security, and the problem is seen on the specific install (which is not always the case because not all platforms/builds were affected), then $2a$ in existing hashes in the database may be changed to $2x$. Alternatively, a similar thing may be achieved by changing $2a$ to $2x$ in PHP app code after database queries, and using $2y$ on newly set passwords (such that the app's automatic change to $2x$ on queries is not triggered for them).

My conclusions (to be confirmed by the experts in here):

We can use bcrypt for all PHP 5.3.x versions with upwards compatibility as before and a much more reasonable implementation using PHP's built-in password_hash() function (and the compat library from Anthony for PHP 5.3+), and do not need to provide a less secure fall-back.

We can simply check for the version and have a special case for such versions for 8-bits password-bytes-escaping (that still results in a forward-compatible password-checks) as @mahagr proposed above.

I personally like that solution for the reasons of extreme simplicity exposed by @ircmaxell above, and the fact that it can be easily implemented.

Advantage: Much more secure than salted-md5 and even more than the SHA256 fallback of 3.2.0, compatible with all PHP 5.3 versions, doesn't necessitate a rewrite (code clean-up can be a different PR for 3.2.2).

Disadvantage: Someone moving from a PHP 5.3.7+ install to a PHP 5.3.6- installation would have all users to recover passwords with 8-bit characters only. That could be a warning on the backend login screen if we detect PHP 5.3.6- + no $2y$ passwords. But 3.2.0 already has that disadvantage without warning. With the warning added, it seems a reasonable prerequisite to not move back in so old PHP versions, and it is acceptable in regards of the advantages above. And most passwords with 7-bits characters would be compatible moving back with proper handling.

Feedbacks?
Thoughts?

How do we move forward for 3.2.1 ?

Correction to @beat : Most of the new passwords from PHP 5.3.7+ keep on working in older versions with very simple change: $2y$ -> $2a$. Non-working ones can easily be detected and warning can be raised/logged. And if you happen to have broken password after going to live, you can always request a new password...

Edit: looks like he was faster than me...

Thanks for the correction @mahagr , indeed I fixed my comment above accordingly.

Ok, I'll reply one last time before wiping my hands of this lunacy. Here are the major points so far in this thread, and my take on them:

Just use $2a

The documentation around $2a vs $2x and $2y are a little bit misleading. When in doubt, always go to the source.

If you read that, you'll realize that post 5.3.7, only $2y is still technically bcrypt. $2x is the old buggy behavior, and $2a is a new algorithm that deviates from bcrypt when a high bit is present (technically not all high bit bytes are dealt with this way, but).

There is a very specific reason that I chose not to support $2a in password_hash(). And it has to do with a lot of things, not least of which was what code was reviewed by the public and interoperability. I was not alone when I made that choice, I made it along with a group of other experts and based on feedback from the community as a whole.

So I would think very strongly before you assume to know better and ignore the advice of recognized experts.

Nobody uses UTF-8

That's a major misnomer in this entire debacle. Sure, high-byte UTF-8 characters are more rare than low byte ones. That's fine. But what happens when someone's computer isn't using UTF-8. What happens if they are using Latin-1 or UCS2-LE? In either case, high byte characters are far more common. Use an accented letter? Guess what, you're using a high-byte character.

And don't even think about filtering acceptable passwords. That sort of thing would be even worse.

BCrypt Isn't That Strong

Someone mentioned that bcrypt isn't the end-all-be-all. And that is very much correct. But I think the scale needs to be stated.

Even a state-of-the-art bcrypt hash of a 6-characters password can be brute-force guessed without huge budget and long time.

Patently false. The current record for bcrypt is 70000 hashes per second on a $50,000 piece of hardware. At that rate, brute-forcing a 6 character password (a-zA-Z0-9 + common symbols) would take 6 months. For a single password.

The salted MD5 that Joomla used to use? The same password would fall in 4 seconds.

So yes, there is a very significant gain to be had.

Dictionary passwords are always going to be fast. Because they are dictionary passwords and there are very few of them. It's a very complex topic, but it requires perspective and understanding of how crackers work to make sense out of the scales we are talking about.

Version Confusion

There's a lot of confusion between 5.3.7 and 5.3.10 in this thread.

5.3.7 fixed bcrypt.

5.3.10 fixed the hashdos attack, which is significantly more critical, and which is why I mentioned it.

PBKDF2

I would highly suggest not doing another copy/paste of a library. If you can use something like composer to manage dependencies, then do that. But copying the library into the release is going to lead to what happened here, somebody patching it without knowing why, and you're going to be back in the same boat.

For that reason, I would highly suggest not using PBKDF2 for this project.

If you must support PHP < 5.3.7 (which I think is completely ridiculous given the current state of releases), then use crypt-sha512 ($6 prefix) with a significantly high number of rounds.

The cryptographic algorithm will still be implemented in C (yay), and you don't have to worry about potential screwups. Although I would highly recommend whoever implements it be familiar with this article as most of them apply to crypt as a whole.

@ircmaxell thank you.

I'm with @ircmaxell and @Hackwar here, security trumps all other considerations.

As I wrote in the mailing list

Yes, there will be pain. However, all it takes is one high profile password leak from a site running Joomla and bang goes the party - especially as we are discussing this in public (as it should be), any cursory search will show that the weakness was known about but nothing was done. That kind of publicity is something nobody needs. Clients (and potential clients) read the news, and getting new Joomla work after such a public fail would be hard - not to mention people that will move from Joomla to another CMS.

Lets not compound past mistakes by fudging a fix now.

Fix the problem, not the blame, and move forward.

Security is hard, so lets listen to those who know about it and follow their advice. Failure to do so will come back and bite us later.

Another (highly controversial) bit from my mailing list contribution

The other option is to shelve the entire 3.x release as is, change to 4.0 as of this change and tell eveyone to move off the 3.x tree. 4.0 becomes the secure STS release, with a higher php requirement (even push for 5.4 rather than a release that is already in its end of life cycle). We could then merge in the GSOC stuff for 4.1 or 4.2, and begin strict following of semver.

That would be a brave and scary thing to do, but I think it is worth considering, we can break b/c moving to 4.0 - and admit that 3.x had some problems that we have fixes for. State that we are bravely shelving the 3.x tree because it does not meet our minimum security requirements. It may be easier from a PR / Marketing POV to 'sell' that than it is to sell the 3.2.x breaks b/c even though we said it wouldn't.

In this 4.0 scenario, the 4.0 release is actually the 3.2.1 that is planned. The existing 4.0 in the plans simply becomes 5.0 (just the number changes, not the code/direction/etc).

Having a clean break could help with the GSOC inclusions, new features added in an x.2.1 or x.3 release is not normal for Joomla and an exception is being made just for the GSOC stuff (which I also think is the right thing to do).

Any of the fudging solutions proposed seem like "arse-covering", or at the very least trying to stick to less important promises and risking a known security problem. Do the hard thing, stop supporting/pandering to those who are using an out of date PHP version. Break backwards compatibility for the sake of the security, and the perceptions of the whole ecosystem (the Joomla brand, the community of core developers, part-time contributors, 3rd party devs, site integrators, theme designers and end users).

When the shit hits the fan everybody gets dirty, it doesn't matter who you are of what you said - if you are advocating Joomla and something goes tits up (that could have been fixed but wasn't) you look like you are part of the problem, and loose some credibility. Lets face it, reputation and credibility is hard to earn and really easy to loose.

Perhaps I am being too 'hard-line' and a 'pain-in-the-ass' by saying this, but I firmly believe that by not raising the minimum supported version of PHP to >5.3.10 (I would prefer 5.4) that the whole Joomla ecosystem will suffer.

Sorry people who are using mega-cheap hosts on old PHP versions, but if you want to run a modern secure CMS like Joomla you need to 1) make them update; or 2) move to a better host.

There are plenty of cheap hosts around who are using PHP5.4 (either as an option or by default), so this is not a 'bleeding-edge' requirement - just a sensible one - IMNSHO!

As for the password security stuff I think that @ircmaxell has the best solution (along with the knowledge and experience), and we should implement what he says.

Instead of pushing responsibility on to the end users to upgrade or change hosts, the Project needs to shoulder the weight of extra technical debt and figure out how to work around this without breaking backward compatibility. There's almost always a solution. password_needs_rehash() is a good way to do this moving forward and needs to be thoroughly investigated before giving up, throwing in the towel and saying "we fucked up".

After long talks at JWC with @nacin discussing WordPress's approach of making life easy for end users in exchange for excess technical debt for core developers, it really hit home for me. Instead of breaking BC at every opportunity because we can, we really need to man up (or woman up) and do the hard thing, which is produce compatible code that is both secure and can migrate users to a more secure system.

I'm not convinced the only way to fix this is by breaking backwards compatibility. I'm sure the final result that works, keeps BC and is secure isn't going to be pretty, but we're not here to write pretty code, we're here to provide the best publishing platform we can.

Having researched the points @ircmaxell bought up there I'm in favour of upping the version to 5.3.10!!

But having said that I don't want to strand people in the merde

I think we should implement @Hackwar 's fixes first. THEN work on JCompatibility as a matter of priority. We put that into 3.2 asap and THEN release a new 3.3 which has the brcypt only workflow. Everyone who can should upgrade to 3.3. We then have to work out how to support (security only??) fixes to the few stuck between 3.2 and 3.3

P.S. Don I want to frame your post there :)

Can we stop arguing theoreticals at this point and just fix the current
issue? Everything else is a decision point for later, there is ONE
actionable point right now.

On Thursday, November 21, 2013, George Wilson wrote:

Having researched the points @ircmaxell https://github.com/ircmaxellbought up there I'm in favour of upping the version to 5.3.10!!

But having said that I don't want to strand people in the merde

I think we should implement @Hackwar https://github.com/Hackwar 's
fixes first. THEN work on JCompatibility as a matter of priority. We put
that into 3.2 asap and THEN release a new 3.3 which has the brcypt only
workflow. Everyone who can should upgrade to 3.3. We then have to work out
how to support (security only??) fixes to the few stuck between 3.2 and 3.3

P.S. Don I want to frame your post there :)

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

@mbabker I wish github had a fav-star, but they don't so I'll give you this -

The code doesn't have to be pretty, but is has to be written to be maintainable, easy to understand, not re-write or over-wrap anything 'just because we can'.

Saying "we fucked up" is not the same as throwing in the towel and giving up, it is an admission of error and 'manning up' (womaning up) by taking responsibility.

I agree that a full investigation is a good thing to do, but lets not get sucked into designing by committee.

There's a time for gathering information and perspectives, and a time for clear decisions. Seek permission to lead and be decisive. There is more risk this way, but also much more chance of success. You can't succeed and please everyone.

how to not suck online

we're here to provide the best publishing platform we can

Agreed. But lets not code ourselves in knots by trying to be all things to all people, be the best by making the hard choices - which can piss some people off - instead of trying to apply patch on top of patch on top of.... you get the picture.
There comes a time when what is broken needs to be cut out and replaced instead of being repaired again. Knowing when that time has come is a challenge, and not everyone will agree, but I think that time has come.

I don't want to be sat here in 6 months or a year retreading this again, or saying "I told you this would happen" - which doesn't help anybody.

I'm posting here and sticking my head above the parapet, risking getting shot, because I believe that Joomla is the best CMS available and I want it to continue to be that.

These lists are a great example of why I think it is the best - open discussion, open disagreement, clear and transparent solutions that are achieved because people care and want the best.

I think the proposed solutions that continue to support PHP versions less than 5.3.10 will make things more difficult, and are the wrong thing to do.

If that is the only 'actionable' on the table - i.e. support <5.3.10 or raise the minimum PHP version now, my vote is clear.

• Support <5.3.10
• Raise the minimum PHP version to 5.3.10

Looks like I missed a couple of posts while typing.

We are within a Major Series (3.x). We can not raise the minimum required version as it knowingly breaks BC and instantly alienates a good chunk of our users who would then be stuck on an insecure version, because they can't upgrade their host. This is not an option IMO. The PLT will discuss it and come back with an answer to the community.

The time to cut out what is broken is NOT on a patch release. 

—
Sent from Mailbox for iPhone

On Thu, Nov 21, 2013 at 11:49 AM, Chris Jones-Gill
notifications@github.com wrote:

The code doesn't have to be pretty, but is has to be written to be maintainable, easy to understand, not re-write or over-wrap anything 'just because we can'.
Saying "we fucked up" is not the same as throwing in the towel and giving up, it is an admission of error and 'manning up' (womaning up) by taking responsibility.
I agree that a full investigation is a good thing to do, but lets not get sucked into designing by committee.

There's a time for gathering information and perspectives, and a time for clear decisions. Seek permission to lead and be decisive. There is more risk this way, but also much more chance of success. You can't succeed and please everyone.
how to not suck online
we're here to provide the best publishing platform we can
Agreed. But lets not code ourselves in knots by trying to be all things to all people, be the best by making the hard choices - which can piss some people off - instead of trying to apply patch on top of patch on top of.... you get the picture.
There comes a time when what is broken needs to be cut out and replaced instead of being repaired again. Knowing when that time has come is a challenge, and not everyone will agree, but I think that time has come.
I don't want to be sat here in 6 months or a year retreading this again, or saying "I told you this would happen" - which doesn't help anybody.
I'm posting here and sticking my head above the parapet, risking getting shot, because I believe that Joomla is the best CMS available and I want it to continue to be that.
These lists are a great example of why I think it is the best - open discussion, open disagreement, clear and transparent solutions that are achieved because people care and want the best.
I think the proposed solutions that continue to support PHP versions less than 5.3.10 will make things more difficult, and are the wrong thing to do.
If that is the only 'actionable' on the table - i.e. support <5.3.10 or raise the minimum PHP version now, my vote is clear.

• Support <5.3.10

- [x] Raise the minimum PHP version to 5.3.10

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

Which is why I suggested that shelving the 3.x major series (which can have security updates, I'm not saying mothball it) and switching to 4.0 major series is an option that overcomes this BC breaking within a major series.

It provides a route to still support those who cannot upgrade on the 3.2 tree, while those who can meet the new minimum requirements can move forward in a new major branch - that can have a clearly defined migration path for those currently stuck with <5.3.10 (when they are able to update their PHP version).

How do you propose to support those who can't upgrade from 3.x? The security issue must still be addressed. 

—
Sent from Mailbox for iPhone

On Thu, Nov 21, 2013 at 12:12 PM, Chris Jones-Gill
notifications@github.com wrote:

Which is why I suggested that shelving the 3.x major series (which can have security updates, I'm not saying mothball it) and switching to 4.0 major series is an option that overcomes this BC breaking within a major series.

It provides a route to still support those who cannot upgrade on the 3.2 tree, while those who can meet the new minimum requirements can move forward in a new major branch - that can have a clearly defined migration path for those currently stuck with <5.3.10 (when they are able to update their PHP version).

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

And if we're addressing the security issue for them,  why abandon 3.x? Let's just address it now and provide the BC fix for all users of the software. 

—
Sent from Mailbox for iPhone

On Thu, Nov 21, 2013 at 12:12 PM, Chris Jones-Gill
notifications@github.com wrote:

Which is why I suggested that shelving the 3.x major series (which can have security updates, I'm not saying mothball it) and switching to 4.0 major series is an option that overcomes this BC breaking within a major series.

It provides a route to still support those who cannot upgrade on the 3.2 tree, while those who can meet the new minimum requirements can move forward in a new major branch - that can have a clearly defined migration path for those currently stuck with <5.3.10 (when they are able to update their PHP version).

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

Those who cannot upgrade would have to accept that the passwords are less secure, but by patching the issue - using one of the proposed methods (or by integrating https://github.com/ircmaxell/PHP-PasswordLib by @ircmaxell ), those users would get the best possible solution given their reliance on a pre 5.3.10 version of PHP.

Because the codebase would be identical (at the time of splitting) except for the password stuff, any security updates (except password related) for the new 4.x tree can be applied to the 3.x tree. If desirable, the GSOC stuff could be released as optional 3.x extensions - as well as being built-in to the 4.x tree.

3.x series effectively becomes 'end of life' with the focus on security updates only, which keeps those users supported - all be it at a reduced level - while moving forward with 4.x for those that are using >5.3.10 (I think they are the majority of users, and as hosts update/upgrade there will be more that will be able to migrate from 3.x to 4.x

4.x provides a clean break from the password POV, cleaning out any unnecessary code - using the standard functions to provide a codebase that is future-proof as possible, is easy to understand and maintain, doesn't have any J specials causing problems and could give the brand a better perception regarding security than it currently has (Joomla is considered less secure the Drupal for example, anecdotally at any rate, and perception is everything to those who don't dig the code). It might even provide a PR win for the brand.

I understand the view that says why not just provide the BC fix and move on, but I feel that the BC fixes are not ideal. And because BC breaking is a bad idea in a major series, moving to a 4.x major series which breaks BC for security reasons is the best of both worlds.

It also means that adding functionality in (GSOC) is not going against the usual pattern of not allowing added functionality between .2 and .5 releases.

Regardless of feelings, we have a contract with developers that we won't break BC within a major series (and yes, I realize that we do sometimes break BC within a major series).

We messed up. We need to fix it. IF there's a way to fix it while maintaining BC, then let's do it and then move on. IF there's no way to do it without breaking BC, then lets break BC and release it as Joomla 4. That leaves us still with the issue of what to do with those who've upgraded to 3.2; their installations are still running insecure passwords. Saying "screw 'em, let them change hosts or upgrade" doesn't seem like a reasonable answer to me.

Also, what are we going to do for Joomla 4? Are we just going to make this one BC break and then call it a day? I sure hope not. There's a LOT of stuff that needs fixing, and incrementing major versions every couple months when we break BC isn't a sustainable solution. It needs to be addressed as a whole and break / fix as much as possible between major versions. A new major version isn't anything to sneeze at, it's a big deal, and should be treated as such.

So again, we messed up with our implementation. I'm fairly certain we can fix it in a BC way, even though it means additional technical debt that the project needs to shoulder in order to make it happen.

I'll state this again: Setup a site on a >5.3.10 host and then move it over to a <5.3.10 host and you have a problem, potentially not validating your password anymore. Consider the fallout for our supporters, handling all those requests of people with healthy systems, but can't login. As I stated before: Either raise requirements or drop back to MD5 for all.

Definitely should be one solution for all. No checking for capabilities of
running version, otherwise your run into those problems.

Except for the fact that we now have multiple options and any code will
have to account for 3.2.0's logic, so there will still have to be some sort
of checks.

On Thursday, November 21, 2013, Don Gilbert wrote:

Definitely should be one solution for all. No checking for capabilities of
running version, otherwise your run into those problems.

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

Also, what are we going to do for Joomla 4? Are we just going to make this one BC break and then call it a day? I sure hope not. There's a LOT of stuff that needs fixing, and incrementing major versions every couple months when we break BC isn't a sustainable solution. It needs to be addressed as a whole and break / fix as much as possible between major versions. A new major version isn't anything to sneeze at, it's a big deal, and should be treated as such.

Totally agree, fix as much as possible if BC is being broken anyway we should make the most of it - with a sensible time-limit on the release or it could go on for months.

Joomla 4 is a minimum of a year away with the amount if stuff that needs fixing and updating. 

—
Sent from Mailbox for iPhone

On Thu, Nov 21, 2013 at 4:49 PM, Chris Jones-Gill
notifications@github.com wrote:

Also, what are we going to do for Joomla 4? Are we just going to make this one BC break and then call it a day? I sure hope not. There's a LOT of stuff that needs fixing, and incrementing major versions every couple months when we break BC isn't a sustainable solution. It needs to be addressed as a whole and break / fix as much as possible between major versions. A new major version isn't anything to sneeze at, it's a big deal, and should be treated as such.

Totally agree, fix as much as possible if BC is being broken anyway we should make the most of it - with a sensible time-limit on the release or it could go on for months.

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

Then only hit the big issue(s) with a 4 week schedule.

All the other stuff that would take the year+ to do can become Joomla 5

Sounds good, but it still begs the question of what to do with 3.x users

—
Sent from Mailbox for iPhone

On Thu, Nov 21, 2013 at 4:55 PM, Chris Jones-Gill
notifications@github.com wrote:

Then only hit the big issue(s) with a 4 week schedule.

All the other stuff that would take the year+ to do can become Joomla 5

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

Just as a side-note about moving a Joomla site from a newer PHP version to an older PHP version host:

• This is already the case with Joomla 3.2.0 with its fallback, but also with password_hash():
• If in the future password_hash() gets upgraded to more secure hashes (and that will sure happen), that's the purpose of password_needs_rehash() , then once that is done on a newer PHP version, e.g. 6.7, then the hashed passwords will most likely not be usable in older existing PHP versions like 5.5.

So that's a natural side-effect of delegating automatic upgrades in security to the system. And nothing exceptional in the software industry (anyone already tried downgrading your smartphone or a Joomla version ?) ;-)

A bit tired of these periodic endless minimum PHP versions discussions, I have decided to move one step forward and raised a proposal and a call for a volunteer for an initiative to resolve the old PHP versions issue in Linux distrirbutions here: https://groups.google.com/forum/#!topic/joomla-dev-cms/DG1PfODkVHU

I must be missing something because the solution seems fairly simple:

https://github.com/joomla/joomla-cms/blob/master/libraries/joomla/user/helper.php#L351

Add a new option to the switch:

case 'bcyrpt-strong':
    return $codeThatUsesPHP5_3_10;

Add that option to the user plugin. Make that option the default when PHP >= 5.3.10. Introduce a "recommended" version of PHP not less than 5.3.10. Disable the option in the plugin wher PHP < 5.3.10. Add a new post-install message to 3.2.1 for people to make people aware (in the upgrade, if you are on 5.3.10 or better, just change the user plugin setting).

What am I missing?

A more basic question though. Where are the bug reports for the problem we are actually trying to fix? Thanks.

May I ask if anyone of you read the RFC that @ircmaxell linked in here at the beginning of this discussion? Please read that one. Its not very long, its good to read and it explains the whole password_hash() API in a good way. It also explains, that the hashing algorithm is encoded in the hash, thus allowing password_verify() to verify a password with an "old" hash. So the remark by @beat that it will break anyway with a change of the algorithm in PHP's password_hash() is not right.

@eddieajau What you are missing is the change from a good to a bad host, as I've written here and on the mailinglist numerous times. The issue is not so much that those people can't generate new safe hashes, they also can't verify existing hashes. So every site that moves from an old to a new host is screwed. And the fun thing is, that they don't even get a message right now about that. They simply sit there and get a "Your username and password do not match" message.

@beat The fact that the people are already sitting on these broken hashes does not make this any better. To be honest, the only thing that I see as being a reasonable solution, is to change everything back to before 3.2.0 times and then build in a system that verifies the bcrypt hashes and then rehashes them as salted MD5. And then we can tackle this whole thing again for 4.0. Maybe with a proper code review. Without messing around in third party libraries. And with properly setting this everywhere.

Hannes, at this stage I'm more interested in due diligence. Can you please post references to the bug tracker issues you are fixing. At the moment we have no information on how to replicate a problem and then demonstrate your patch "fixes" it. Let's follow some standard procedure before we start arguing about whether people should or shouldn't upgrade their hosts.

Sorry, didn't mean to sound so snarky - I just need to see the bug before working out whether your solution is the best way, which it very well could be :)

My understanding so far and if it comes to a vote within PLT, that will currently be my stance:

• Raising minimum requirements within a major series is not going to happen. We have decided for minimum requirements for a reason
• Releasing 4.0 within a few weeks just to fix this issue is not going to happen as well and is no solution as we still need a fix for 3.x
• Correct me if I am wrong: We are talking about an optional feature (Strong Passwords) introduced with Joomla 3.2.0 which doesn't work well on PHP < 5.3.10. This feature has to be manually enabled for updating users, but is enabled by default for new installations.

So why not just disable this feature for PHP < 5.3.10 by making the option a conditional formfield based on PHP version. It could be readonly/disabled if PHP 5.3.3 and showing a notice that it's only available on PHP 5.3.10 and higher.
Then in code we just check that option (and maybe also doublecheck PHP version, just to be sure). If enabled use bcrypt, if disabled use md5.

If we can provide a way to migrate the passwords back to md5 for the users on PHP < 5.3.10, then that's fine. Otherwise they need to request a new password which will then work again without issues.

We don't have to account for users downgrading PHP versions or moving sites from a newer to an older PHP version. Honestly I'm not sure why anyone would think that has to work. Downgrading stuff never works well in any Software.
But then all that would happen is that users need to request a new password.

Are there any issues with that approach I don't see?

There are bug reports right now concerning wrong hashing because of a "missing" default value in 3.2.0. Looking at the code, I saw several more issues, like truncating the password, no fall back when moving from a bcrypt host to an MD5 one (we are currently hashing as SHA256 without salt when no bcrypt is available, regardless of if we want MD5 or bcrypt) and lots of other stuff that is just plain wrong. I started trying to fix it with my initial PR, uninformed as I was and thinking that we had a choice here and that there might be more than those two hashing algorithms that we could use. Turns out, that the whole approach is wrong.

Right now we have one big commit that introduced bcrypt hashing for our passwords and a behemoth of a remember-me feature, which is also based on that and wont work for hosts without bcrypt as far as I can see. I already know that this PR, that we are commenting on, is outdated and will NOT be the solution to fix these issues, because it does not adress the remember-me feature. Since we are having the discussion here, however, I'm not closing it as of yet. The only sane approach would be to revert the commit that brought us into this mess and then to put in code to clean up the mess that 3.2.0 made, including the additional table(s?) for the remember-me feature. Then we can raise the requirements for 4.0 and go at this again.

With a quarter of a million downloads and 15% of hosts running <5.3.10, that makes almost 40,000 installations that are not using bcrypt. Having only 5% of those that would be developed on a devs up to date machine and then moved to such a crap host, would mean 2000 sites where from one moment to the other, the password does not work anymore. No message, no nothing.

Actually we don't know if it's 15% of those 250k downloads using PHP < 5.3.10. You are mixing two statistics and assume an even spread. It may well be that a lot of those installations are to test the new Joomla version and on newer settings like XAMPP. But this is an assumption as well. Basically we know nothing :)

Again, downgrading the PHP version is neither recommended nor supported. Also one should test it on the production server before going live, that's just common sense. Testing on a local XAMPP installation and then going straight live expecting everything working on an outdated live server is just asking for troubles.
So this really is an issue to me we need to solve.

@Hackwar :

First off all, I agree that the code that you are reviewing is ...not the best and has diverse bugs at diverse levels. But reverting it and redoing it is no simple and quick task. Regarding your other posts above:

1. read my remark again before saying that i'm not right: It's not when upgrading PHP but when downgrading it that you might not be able to verify existing (newer) hashes with the older PHP version, same as you just wrote to @eddieajau. And that's perfectly OK. Nobody complained that moving a Windows harddrive back to an older machine where it worked before didn't work anymore.
2. the existing hashes are not broken: they do work (and can be verified in any old, new and future forseable PHP version). It is not an exploitable vulnerability by itself, but outdated hashes, as the CPU time and cost has decreased since 8 years. Same will happen to hashes done with the bcrypt of now in 8 years time, except that this time we rely on PHP to upgrade the hashing algorithms for us, since nobody had time, will or was welcome (choose your pick) to update them a few years back.
3. there are many different issues that you are trying to fix at once here (several bugs, code from that bcrypt+rememberme PR that you do not like and would like to revert, and the new SHA256+salt hash that isn't significantly more secure than the existing MD5+salt because it's missing the iterations, and to simplify code for better ability to security review). Howerver that's a larger undertaking, that won't fit into 3.2.1 imho, as once reverted, a new implementation for password hash AND remember-me security improvements would have to be made and thoroughly tested + betas + RCs on many hosts before being able to release. That's something that can and should be done for 3.5.0 beta 1 (not far away). For now we need to fix the bugs, and if possible to add a better fall-back (actually it's same as we have, just with an iteration around it).
4. The remember-me feature improvement is an important one, that was fixing a real vulnerabilty. We cannot release a 3.2.1 without a security-fixed remember-me (and that issue is not as simple to fix as the password hashings which is easy from a reverted master).
5. Storing the highest PHP version a site was running under in configuration.php and displaying a message "you downgraded your PHP from x.y.z to a.b.c" in the cpanel is not a big deal (should be a separate PR).

Another PR could add the iterated hash that @ircmaxell suggested above #2555 (comment) although I remember in another discussion when i suggested SHA512 to have got reply that SHA512 was not available everywhere, while SHA256 was. I didn't re-check on that statement as it was just for the fall-back for old PHP versions. That iterated fall-back is not a lot of lines of code compared to the non-iterated one (basically it's a "for" loop), but should be implemented carefully and security-reviewed. Still very possible in time for 3.2.1 (or for 3.2.2 if simply reverting fall-back to md5-salt for php 5.3.6-).

Now that need for PHP 5.3.1+ support seems to have been answered, we have another basic decision to do: Revert first or not revert first #1745. To me, unfortunately, that's not looking as a timewise feasible an option although for next major release 3.5 it should be rewritten. Then based on that decision, we should have separate PRs to fix (or reimplement the features) of that PR properly and securely. Not all of it needs to go into 3.2.1, some can wait 3.2.2 and others 3.5.0 beta 1.

This is my 5th proposal for a way forward in this thread.

I've proposed my stuff, too, but so far, I've not seen a proposal here, that does not introduce a known bug into our system. Regarding the remember me feature: The way it is now, its broken, too.

So, not to sound like I'm trying to rush decisions or anything, but we have a mix of issues to sort through centering around this fix. Of course we have to actually code, test, and commit to a patch (which it sounds like there is a general consensus on a plan?). We have smaller windows of opportunity to deal with (next week is a major holiday in the US, and December has its own plethora of holidays and individuals traveling). And actually issuing the release fixing the issues within this window (does anyone really want a holiday or weekend release for this?).

I had hoped to release next Tuesday (November 26) but given how everything's turned out with discussions and not yet having a plan of action, that's out the window now. Is December 3/4 realistic at this point, or do we put a pin on the calendar for December 10/11?

As @eddieajau already requested it may help to have a list of the issues at hand which need to be solved. Only one tracker item has be mentioned so far but Hannes raised the list of issues to at least five more issues?
Also it was mentioned that there is an issue with the remember-me function?

Imho we should only fix what is broken at the moment, and from my understanding this boils down to

• a faulty bcrypt implementation in PHP < 5.3.10 which leaves some user being unable to login.
• a backward compatibility break in the API between 3.1.5 and 3.2.0 (?).

The other issues can be threated separately afterwards imho.

I believe this pull request will display some of the issues in the code:
https://github.com/joomla/joomla-cms/pull/2358/files

To me most of the issues in the current implementation come from API change, which you can clearly see from the above pull request. Additionally in some systems BCrypt implementation might be the reason. So the code is broken because of 3-4 different hashing methods are used (md5:salt, sha256:salt, $2a$, $2y$) and the responsibility to figure out if salt should or should not be added was left to the caller.

The above pull request attempts to fix this by moving the salt part to the JCrypt class, but that only removes the issue from CMS -- and everyone else would still need to change their code (again, if they already did that).

My proposal is very simple -- leaving remember me issues out for now (someone needs to explain those to me, too):

• Revert (only) the JCrypt class changes
• Introduce new class proposed by @ircmaxell
• Change all the CMS code to use the new API

So basically if someone still uses the old class, it will keep on working both on creating a new password and for all the existing passwords. If the extension creates its own passwords, they will still keep on using salted md5, which is OK. It's less secure, but at least we will provide those extensions time to update their code -- it's very likely that those extensions handle both registration and user profile changes by their own.

For those extensions that integrate another system to Joomla (TapaTalk, RokBridge etc), they will need to change their code to support the new and more secure crypto algorithms. Before they do that, they just cannot do the authentications with the new hashes. That is true for J!3.2.0 and that will be true for J!3.2.1 - no matter what we do.

The proposed Foo::createHash() and Foo::verifyHash() would be optimal solution for them as the proposed functions are very easy to use compared to the current code in J3.2. Some documentation on how to use the new API (maybe together with J! 2.5 API) would save everyones time.

My last remark: It is very important that the new API can be used outside Joomla without having any dependencies to the other classes. That's why it shouldn't be implemented to JUser or any class that depend that Joomla has been bootstrapped.

to @mahagr : Thanks Matias, description looks very reasonable and thoughtful! Thanks also for having worked on the code as well and for your solutions-oriented approach. . I'll look at code in details later this week-end when I have free time.

If I got that right, then getCryptedPassword now returns always with the salt and before it was not?
Would it be possible to add an additional parameter $addSalt = false to that function which will always add the salt and creating one if needed. Then we could keep the old behavior if it's false and in CMS we use the new behavior and set it to true.

Introduce new class proposed by @ircmaxell

If you are referring to https://github.com/ircmaxell/PHP-PasswordLib, then that will not work. It has the minimum requirement listed as PHP 5.3.2 and Joomla 3.x has PHP 5.3.1.

Can you make a PR with your changes to fix the API break? Then we would at least be a step further :)
Introducing new APIs should be handled in another PR if possible.

I'm saying this again: We should start by reverting the commit that introduced all of this and then we can start over and implement the stuff that was discussed here. That way we can really prevent API changes. Regarding another parameter to add salt or not: Please don't. As @ircmaxell said, cryptography should be simple, everything that makes this more complex will make it worse. Don't use getCryptedPassword() anymore, as not only does it have unsecure hashing algorithms in it, but it is also very complicated to use. Either you hand in a salt or you don't, but if you don't, it does not generate a proper salt by itself and a whole host of other issues. After discussing this for 5 days, I'm very sure that we NEED a completely new method for this.

The class that @ircmaxell proposed was a wrapper around password_hash(), not PHPass. JUserHelper would also meet the requirement that it does not depend on any other Joomla libraries.

@Hackwar Can you make a fresh PR with your proposed code and reference it here? Also good testing instructions would be needed.
I'm not sure if we want another complete rewrite of this stuff (I doubt it) as it will need proper testing, will only delay the fix and likely introduce new bugs nobody thought of. But if there is no simpler way to fix the current issue then that's maybe the way to go.

However if there is a way to fix the current issue (and only that), then I'd rather see that fixed and refactor the whole thing for 4.0.

Well, I already thought of adding additional parameter to the current API, but unfortunately it doesn't fix the problems. In many occasions trying to change the existing classes "to keep backwards compatibility" just make things worse.

One example is the JTable in J!3.2.0 which causes severe data corruption in all extensions which implemented multi-key support in their tables. For me it took days to find that bug and the only reason why the extensions broke, was the "backwards compatibility code" which was added to the class. There's existing PR on the issue.

Back to the subject:

I'm against of reverting all the code because of very simple reason: I've tried it before and doing that after a few hundred commits you probably miss a few other commits that actually depend on the new code. It's also way too easy to revert other important bug fixes in the process and the task will probably take much more time than you originally thought. I'm just saying that there is a significant risk of delaying the next release for a few weeks if not months.

@Hackwar Sorry if you said JUserHelper before -- I remembered that you were talking of JUser itself. I agree that it would work, but at the same time I'm a bit buzzled what's the purpose of all those functions in there. :) That said, it might be better to have separate class for it, just to keep the new implementation separated from the old one. Maybe JUserPassword? I don't have answer to that.

Yes, let's start from something that's simple: create the new proposed functions. Nothing more, nothing less. I wouldn't even start using them yet.

Also an important note. The only trackeritem referenced here is in the state ready to commit. There is already a fix which is tested apparently.
The most important question is: With current master and the proposed fix from that tracker http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32595. Is there even still a problem we need to fix before releasing 3.2.1?

• If the same problem still exists with the proposed fix in the tracker, please state it there. If the fix works and fixes the issue, please also state it there. This fix could be merged within a few hours.
• If there are other issues please open a new tracker item for it and provide steps to reproduce the problem. We can then decide for that tracker if it should go into 3.2.1 or if it can wait for a later release.

It would be a shame if we delay this patch release only to find out at the end that the already proposed fix would have worked fine.

Please refer to pull requests directly as the issue contains a few different versions.

If you refer to https://github.com/mbabker/joomla-cms/compare/doubleSaltEdited I must have missed that one.

Yes, it seems to fix the issue in Joomla and it looks like the far the best and cleanest solution I've seen this far, but it still breaks backwards compatibility in JUserHelper::getCryptedPassword() and JUserHelper::getSalt() and raises risks that 3rd party developers get it all wrong, especially if they want their code to work in both Joomla! 2.5 and 3.2.

Copying my comment on the free possible options from the mailing list into here:

1) keep it like it is and throw the responsibility of figuring out how to handle 3 different types of passwords to the 3rd party developers
2) break the API again and have database full of broken passwords because of some 3rd party dev didn't know how to deal with the API change (J! 2.5 vs J!3.2.0 vs J!3.2.1)
3) revert JCrypt back to what it was and implement a new API next to it, which is well documented and easy to use

The original code was falling to the first category where the proposal in doubleSaltEdited branch falls into the second category. It is much better than letting everyone to figure out how to handle salt by themselves, but it does introduce some new issues by breaking the API also for the old passwords which wasn't true before.

That said, all the proposed solutions have their good and bad sides and there likely isn't an optimal way to solve this issue. In long term I would introduce JPassword class to clean up the old code, but in short term we could live with this change IF we provide good enough documentation which is linked to the release notes to help 3rd party devs to deal with this change.

My biggest concern on this is that all 3rd party devs who use there two functions and skip JUser will need to change their code before J!3.2.1 releases in order to work at all. That includes the ones who already changed their code to match J!3.2.0.

Please post your findings for the doubleSaltEdited PR in the tracker. It's the point of thruth.

And then please open a new tracker for the backward compatibility issue as it isn't related to the issue and fix itself as far as I know.

We can rewrite the whole thing for Joomla 4.0 if there is interest to do that. We could then maybe even add it as a new class in a 3.x release to provide the possibility of a smooth transition. But that's another topic :)

@Bakual Can you post links to the pertinent and valid and PRs at this point? Thanks!

I created a new PR with my proposal for the changes that we talked in here here: #2589

If this is a suitable basis for the password fixes, please either accept the PR or make a PR against my branch and then I'll accept it and we can let that be accepted by the project.

For the time being, this implements salted MD5 as the current hashing algorithm. Feel free to change this to the one that we agree upon in the end.

This does not look through the remember me feature. This needs to be handled seperately.

@betweenbrain To my knowledge http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32595 is still the only tracker which was mentioned. In this one there are two current PRs:

• https://github.com/elinw/joomla-cms/compare/doublesalt which got two good tests and thus tracker got RTC status
• After Dennis raised an issue @mbabker created https://github.com/mbabker/joomla-cms/compare/doubleSaltEdited.diff which was tested I think once by David.

At this point this PR was created to solve this and some other issues. As far as I know the other issues mentioned don't have yet a tracker.

@Hackwar Thanks for your work!

https://github.com/elinw/joomla-cms/compare/doublesalt which got two good tests and thus tracker got RTC status

Would you agree that this needs more than two tests before committing?

Would you agree that this needs more than two tests before committing?

It's certainly not a big fix and thus two tests would initially have been sufficient imho. The fix itself looks quite reasonable to me. Not overachieving and easy enough to understand what is done.
However given the discussions we have. Yes, more actual testing and using the tracker to post results (or open new items for new issues) would certainly help.

That PR introduces an API break. Considering what we discussed here in the last few days, that is not acceptable. It also still has double-appended salt in some places. Then again it makes comparing hashes even more complicated. Further more, it changes the default behavior again from bcrypt to MD5. And last but not least, that code is superseeded by my PR.

So please don't.

That PR introduces an API break.

Which most likely could be fixed without rewriting everything.

It also still has double-appended salt in some places.

This is a useful testing result which should go into the tracker. Can you post your findings there? This sort of information is missing.

Further more, it changes the default behavior again from bcrypt to MD5.

This actualy is needed for updating users who didn't save the plugin parameters yet. It is an appropriate and easy fix for one of the current issues.

And last but not least, that code is superseded by my PR.

Only if your PR is accepted. Otherwise we still need a fix.

Removing the break in the API means adding a case switch at every place that we use this API in. So in fact, it is more work to fix this than to start over. It also still breaks the system for every third party extension.

Changing the default behavior would be an easy fix, if the system was meant to rehash the password in the other direction, too. Right now, the password is rehashed to bcrypt. Period. The reverse is not meant to happen in that code. We both agree that bcrypt would be better, but when the user has the choice, you should actually honor that choice and not force him into bcrypt anyway.

I'm with @Hackwar on this as you can see from my previous posting. Previous fix is API compatible except on the new hashes, but pushes the responsibility to 3rd party, who are very confused on how the API should now be used. The proposed fix breaks the API by changing how it works. And, no, it doesn't remove double hashing issue, but it does offer some (local) protection against it. It is clearly the best way to fix the current API, if we decide to go into that way. But be aware that it does break backwards compatibility and will cause double hashing and non-working passwords in 3rd party software until they release a version that works around the issues (hopefully understanding the change in the API).

On the other hand we have some new code from @Hackwar, which removes a lot of complexity from the client code by introducing a simple API which was earlier introduced in here. It will keep the current password code working as long as 3rd party doesn't need to deal with the new hashes. Yes, also with this solution the new sha256 and brcypt hashes will fail unless 3rd party starts to use the new API where it's available.

Neither of those solutions are safe -- we need to make sure that we find all the code which was dealing with passwords and both of the solutions will likely have some bugs, even after the proposed changes. Neither of those solutions will be fully backwards compatible either. But that said the current implementation isn't either.

The new PR uses salted MD5 again, so for the time being, it would be 100% compatible with all 3rd party extensions. Due to all of these issues, I would recommend pushing bcrypt implementation to 4.0 and not earlier.

Ran in to this tonight, and tracked it down to the MD5 salt thing, before finding out it's a known bug, and got pointed here by Matias.

My $0.02. Fix salted MD5 now. Like ... erm ... now. Then have the meta-discussion about everything else.

I know this whole pesky strong password thing needs some serious thought, but we've known MD5 is broken for several weeks now, and to me it looks like a two line fix.

Apologies if this is what has been decided, I've been reading through several reams of bug tracker and PR commentary, and it's coming up 7am, and it's been long night, and my brain stopped working a while back.

@cheesegrits It's very possible that you're correct; a lot of us have been looking at this for far too long, we might be blind to the simple fix. Could you put together a PR or gist of your two-liner and submit it for review? Thanks.

I will say this again one more time. Shipping a new version using MD5 (salted or not) or sha256 (salted or not) is completely unacceptable.

If you want a single, secure method that works on 5.3.1, then check out the suggestion that I made in my very first comment to this thread: crypt-sha256 (I gave 2 options, the best raising the minimum version, but the second best going with a crypt(3) implementation of sha).

And as I've said before, the abstraction that's currently being used is wrong. If you don't want to clean it up, that's on you. But the entire reason you're facing this mess instead of being transparent to end-users is that the abstraction is wrong. There's no better time than the present to clean that.

Thanks for coming back to take another look at this again Anthony. In the interest of a timely release that can address this issue, can you point us towards a sample of the proper usage for crypt-sha256? I don't want to start heading down that path without a good map.

And I know, we're dug this hole we're in, but we do have to fix it in a BC way. Trust me when I say this spaghetti will be fixed and simplified as soon as we can have a release that can break BC; until then we need to shoulder the technical debt that comes with attempting to securely maintain 8 year old software.

Do what you want, but BC is wrong.

Maintaining BC will maintain the vulnerabilities. They will maintain the problem.

The only way to fix a problem like this is to break all code that's contributing to the problem.

Will it hurt? Sure. But it won't be that difficult to overcome, and it will show the world why it is important.

After all, what's the better end? To break compatibility in order to improve security for everyone, or to keep compatibility and put the entire world at risk?

Personally, I think @nikosdion has it right with regards to B/C here - #2589 (comment) - this is the one time where it would be acceptable to toss the SemVer stuff and B/C promises out the window and do the right thing for all of us, and ironically this is one of the few times where there seems to be an agreement in the community about purposefully breaking B/C mid-cycle.

@ircmaxell I'm not convinced the only way to fix it is to toss out everything and start anew. I've said it before, but the BC way to fix this will not be pretty or even necessarily maintainable / sustainable in the long run. The reason I keep harping on BC is because it's a promise the project made to the community. And after talking with community members at JWC, it seems they already don't think we do a good enough job with it (and I agree with them), so I was just trying to change that perception.

However, I'm here to serve the needs of the community, and if (as @mbabker has pointed out) the community wants to break BC in favor of fixing a long standing security issue, then who am I to stand against that?

BC is a promise that IS important, and many people are (justifiably) angry when it it is broken (by mistake or design).

However, secure software is also a promise made to the community.

See http://www.joomla.org/about-joomla/the-project/mission-vision-and-values.html - in the section about Vision it states

Software that is free, secure and of high-quality;

When the 2 promises (one of not breaking BC, the other of being secure software) clash as they seem to have done here, one must take priority over the other.

In this case security trumps BC.

Also, would anyone state that the software around this password issue (and the remember me) is 'high quality'? Even though 'high quality' is very subjective, the comments in this (and other) threads lead to believe that not many people would.

In the "mission and values' article I referenced above, it also states

A project that is socially responsible;

People believe that any socially (or otherwise) responsible project will keep their passwords as safe as possible, and close any potential security issues as soon as they are known about - and that their security is put before all other considerations.

The internet is rife with people railing against companies that have had password breaches - and the scathing comments from HN (and reddit, amongst others) about how poor and badly implemented their security was/is has a lasting effect - not just on the tech community at large but also on the technology leaders - those easy adopters, those who advocate for certain brands, use the software in their projects, etc - whose choices drive the opinions of others.

I'm sure Adobe had some very painful meetings after their breach, and their PR team went into overdrive. I would not want the same people on HN saying things about Joomla password security.

Paraphrasing @ircmaxell, maintaining BC also maintains the problem.

My beliefs are clear from my previous posts, but for the TL;DR crowd:

Break BC, raise the PHP minimum requirement, fix the code in a forward compatible way, and cut out the code at the heart of the problem.

Chris.

@dongilbert Backwards compatibility is not a purpose in itself. Extension developers and site integrators shun backwards incompatible changes done without any justification or for puny reasons (naming preferences and "just because we can" are puny reasons to most people). When there is a pressing reason –especially security– nobody minds terribly much if b/c breaks, as long as it's documented. It wouldn't be the first time a security fix breaks b/c.

In the end of the day if there are any doubts, ask the people who are affected and might complain: extension developers. The project already has their email addresses in JED. A focused poll is easy to run and would dispel any doubts.

Regarding backward compatibility: It depends what you mean with this term. If you are talking about a break in the API, I agree that security trumps BC. Especially in this case, only a few extensions probably deal with authentification itself. And if we document the break well, they can update their extensions easy.

However if we are talking about raising the PHP minimum requirement, it becomes a whole different topic. Because it will mean that we tell our users "Hey we know we broke your site, but you have to find another hoster to get it fixed. Remember: Security trumps BC". They will likely not share that opinion. For them, most likely BC and money trumps security. Otherwise they had chosen a better hoster long ago.
And if next week a major security issue arises, they are in an even worse position than today. Then they have a vulnerable system with weak encryption.

@Bakual I disagree on two grounds.

1. Using bCrypt is optional in Joomla! 3.2. If a user decides to use the old method (salted MD5) they don't have a problem. Moreover, using a PHP version older than 5.3.7 won't brick their site, it will result in their hashed passwords being easier to crack – but, then again, not as easy as salted MD5.

2. PHP 5.3 is already end of life and 5.3.6 was released over 4 years ago. A server which has not been updated that long has other security issues. Moreover, talking from experience with my clients, most hosts with an ancient version of PHP 5.3 offer a newer PHP 5.4 version if you ask them. The very few that don't are so desperately outdated and insecure that sites can be hacked despite our best efforts.

As a result I consider the elevation of minimum version requirements in the context discussed here a non-issue. If we were bricking sites running on PHP 5.3.6 or earlier upon upgrade I would consider it an issue that needs a proper project management solution. But compromising the security of the CMS so that we can stupidly claim that we support an ancient version of PHP? Why?!

I would be very happy if the energy invested into this discussion could go into testing the proposed fix here #2589, fixing issues like double salted hashes, etc. and then having that discussion.

@nikosdion Actually part of the current issue is imho that "strong passwords" defaulted to be enabled in the code for updating users. As long as they didn't save the options. But I may be wrong.

@Hackwar Agree with Hannes. Please those who understand the issue, please test :)

@Bakual Strong passwords were disabled by default. We have a post installation message which allows Super Admins to enable this feature. Even if they did enable it and they are in a pre-5.3.7 PHP version the new code will not brick their sites. It will simply result in hashes that are easier to break (but, still, more secure than plain old salted MD5).

I would recommend reading Anthony Ferrara's very detailed comments on why he suggests the use of PHP 5.3.10 or later. You'll see these are security reasons, not code breaking hard (bricked sites).

Moreover, I would suggest trying to crack a few password with Hashcat to get an idea of the difference bCrypt makes in cracking passwords. There is a Windows GUI for it, making it dead easy to use. If you're looking for a word list try googling "RockYou wordlist". It's a dump of real world passwords, allowing you to easily crack most common passwords. Try a simple password like admin123 hashed using both methods to see the speed difference and you'll understand why we freak out in the idea of switching back to salted MD5. Of, for what is worth, Hashcat has a password hashing mode called "Joomla!". It's salted MD5. Our current password hashing implementation is so popular and so weak that it gained us a named spot in one of the most efficient password cracking programmes... Another reason I personally freak out in the mere thought of rolling back to salted MD5.

@nikosdion I had this lines in mind: https://github.com/joomla/joomla-cms/blob/master/plugins/user/joomla/joomla.php#L60 and https://github.com/joomla/joomla-cms/blob/master/plugins/user/joomla/joomla.php#L505
It does a check for the parameter strong_passwords and defaults to true. Since updating users don't have the settings saved yet, this code will assume it is enabled.
Also if you have a look at https://github.com/joomla/joomla-cms/blob/master/plugins/user/joomla/joomla.xml#L27, the default is set to 1 in the XML as well, which means if the users opens the plugin options and just saves them, the parameter will be magically enabled for them.

So I'm not that sure that it's disabled for updating users. Not in all places at least. It is correctly defaulting to disabled in the application.

+1 @nikosdion

I've set aside the next two days (UK time) for testing this proposed fix.

Questions:
1. Is there a scenario order of importance to test from?
e.g. 2.5.latest -> 3.2.1 or is 2.5.latest -> 3.0 -> 3.1 -> 3.2 -> 3.21 more important, a list would be nice :-)

1. Is there a set of standard data set being used to test with (usernames, passwords, profiles)
   So they can be loaded in to the database via SQL instead of manually adding them - useful for having 2 users (one very old, who last logged in on version 2.5 and not since VS one who has updated their password after every update notice for example).

2. Are there any specific user interactions that need testing
   e.g. Admin updates to 3.2, enables strong passwords, tells everyone to change their password (which they do), then disables strong passwords (either roll-back entire site or disable the strong password functions), then tell users to change passwords again (which they do)

There are so many possible scenarios and only a finite amount of time available to test, so I would rather start with the most important and go from there.

I will be testing on a live server, not on my development environment, so that any testing is as close to normal deployment as possible. My test server runs in the default configuration:

• PHP 5.4.16,
• MySQL 5.5.32,
• cPanel 11.40.0 (build 24)
• running on X86_64 linux(kernel 2.6.18-408.8.2.el5.lve0.8.61.3xen)
• Apache 2.2.24.

I can change the native PHP version via cPanel (back to 5.2 or up to 5.5), so that is another test scenario possibility.

Chris.

@Bakual Oh, man, our current code is even worse than I thought... In this case we've totally blew it. We ask the users to enable strong passwords but they are silently enabled by default. So, basically, our only option right now is to allow decryption of passwords hashed with the botched bCrypt method. As for new passwords, we can temporarily roll back to salted MD5 (sites are currently broken due to our botched code...) then get our collective buttocks down and write a proper bCrypt implementation which will be automatically used in newer versions of PHP. Basically we'll be saying: if you have an ancient PHP version you're stuck with the vulnerable method, if you have a decent host you get the most secure method.

I'll try to test patch #2589 today and tomorrow.

@nikosdion I hope you see why I freaked out when I saw this and started this PR. And again, I throw in the fact that sites move hosts pretty often, which might let a site with a proper PHP version end up on a non-working PHP version.

@KISS-Web-Design Please look at the PR #2589 and its Joomla tracker for what needs to be tested and how to do that. This PR is actually old news and should be closed, but since we use it for these truelly productive discussions... ;-)

We seem to have a consensus on the mailinglist on how to handle this and we also seem to have a solution with #2589, as frustrating as it is. We will have to visit this topic again for the next release, but for now I'm going to close my PR here.