Success

User tests: Successful: Unsuccessful:

avatar andywer
andywer
10 Apr 2013

Feature tracker ticket: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_id=8549&tracker_item_id=30530

Problem
User passwords are salted and hashed and the password's hash and salt are stored in the users table in the database. If there is an SQL injection vulnerability then an attacker finds everything he needs to crack the hashes in the database.

Solution
Adding a secret, second salt that is unique for each Joomla site and that is not stored in the DB. I propose a 'pwsecret' property in the configuration.php.
There is already a 'secret' property (which is only used by the session handling and caching code), but by introducing a new property, the new code won't break existing password hashes, because it recognizes that the site was not yet set-up to use this second salt in it's hashes.
The feature cannot easily be applied to existing installations, but it will not break them and should be used for all new installations.

Regards
Andy

avatar andywer andywer - open - 10 Apr 2013
avatar elinw
elinw - comment - 11 Apr 2013

I'm not sure what you mean in saying " password's hash and salt are stored in the users table" what field are you referring to? The hashed and salted password is stored. The salt uses the secret word from configuration already (as you see you just changed secret to pwsecret) and I'm not sure how adding another one adds security since if someone has access to one secret word he probably would have access to the other. If you have a proof of concept for decoding the password without access to the secret word would you please send it to security@joomla.org. (I do know that in Word Press you need to manually enter something like 4 or 5 secret words but they use a somewhat different procedure for encrypting than Joomla.)

avatar andywer
andywer - comment - 11 Apr 2013

No, the current 'secret' property of configuration.php is not used for password salting. That is the core problem.

Just take a Joomla installation of your choice and check the 'password' column of the users table. You will see that the stored hash (column data format "$hash:$salt") is just:

$encrypted = ($salt) ? md5($plaintext . $salt) : md5($plaintext);
(see libraries/joomla/user/helper.php, getCryptedPassword())

That is no real bug, but just a missing security feature. $hash = md5($password . $salt), that's how people cracked Joomla passwords for years.

I do not suggest entering a secret string manually. We could just use the configuration.php's 'secret' property for salting/hashing, but the code would then break existing hashes...

avatar HermanPeeren
HermanPeeren - comment - 11 Apr 2013

Stored in the user table is the salted + hashed password: md5($plaintext.$salt) plus in the same password-column the salt (needed to hash the given plaintext password). The line of code you show is not how it is stored in the db, but how the plaintext password, given by the user, is hashed to compare it to the stored hash in the database.

The salt is not a secret. I once used it to hash the password client-side, to avoid the plaintext password going over the line and entering the server-side system unhashed (which is an easy way to intercept the passwords in a hacked system). BTW: we added a nonce to it too, to not send the stored hash over the line.

BUT: as hardware for brute forcing a hash is faster and faster (and md5 was made to be fast), there are indeed possibilities to brute-force a password if someone has that password-field from the database (with the salt stored in it). With SQL-injection as one of the most occurring vulnaribilities, someone could get those data from the database. The solution to take a slower algorithm, specially when the slowing-down factor can be adjusted, is one of the possible cures to avoid brute force hacking.

Your proposal to add a secret from outside the database is a solution to avoid brute-forcing the password when "only" the database password-data are in wrong hands. It could add a little to the safety, although I'm a bit pessimistic: in the past months we've seen several hacks, like the Pharma Hack (via JCE). When you look at that code you'll see, that the attacker has access to the database as easy as to the configuration file. So, storing an extra salt there won't help much. More: if I were that hacker, the first thing I would do is to catch the plaintext passwords that are sent to Joomla and send them via my backdoor. But that is another story than what you suggest.

TL;DR: It is some effort adding another secret. There is some gain, but not much. Is the gain worth the effort?

avatar andywer
andywer - comment - 11 Apr 2013

Thank you for your reply. I think it is worth the effort, since the effort is just ~10 lines of changed code.
I assume an SQLi using a vulnerability found in of the extensions is the most common way for a hacker (and for most script kiddies) to attack a Joomla site.

Introducing this little security feature should prevent an attacker of gaining backend access using only a single SQLi exploit.

avatar HermanPeeren
HermanPeeren - comment - 11 Apr 2013

You can allready now implement your password with secrets and other things, whatever you want: just define another authorisation-plugin. Those plugins are incremental, so all old stuff would still work.

Changing the default way Joomla does it, could be a lot of effort: how many extensions use that implementation? How many would break? What happens if we want to upgrade an existing userbase? Not to discourage you, just looking how to avoid backward compatiblity issues.

avatar andywer
andywer - comment - 11 Apr 2013

Hmm, maybe you are right with the plugin idea. But I still think that this should be a basic, built-in security feature that you don't have to install separately.

Upgrading existing databases will be a problem. But they will still work without a problem, they just don't use this feature. Existing extensions should work without a problem unless they manually access the database directly, instead of using the Joomla API. I am not sure if that's a problem.

But I am glad for any help and advice! Especially since this is my first Joomla contribution effort...

avatar mahagr
mahagr - comment - 11 Apr 2013

I've followed this discussion from the beginning. I would vote against this change as I agree that the right place for the change would be inside authorisation plugin, not in the standard hash generation methods. I know that some components use the feature for other things than storing user passwords and breaking the standard API call would end up a lot of complaints not only from the site administrators, but also from developers.

I also agree with @HermanPeeren that it would be better to default on more secure password hashing than trying to keep using the broken MD5 hashes. This is because of trying to make insecure hashing better by adding more layers into it can in worst case make passwords less safe than they were before that attempt.

I've not looked deeply on how Joomla stores the salt and password, but I believe that it's easy to figure out which algorithm was used to store it..? In that case it would be easy to change the implementation to use better algorithm and to update most of the old password hashes automatically during the next login.

avatar andywer
andywer - comment - 11 Apr 2013

Ahh, ok. Maybe I misunderstood the plugin idea, too.
So if I move the new salting/hashing logic to plugins/authentication/joomla, would this solution have a chance to be incorporated by the Joomla main repository?

If the code would do something similar to this:
$oldHash = JUserHelper::getCryptedPassword($credentials['password'], $salt)
$newHash = JUserHelper::getCryptedPassword($oldHash, JFactory()::getConfig()->get('pwsalt'))
then one could also upgrade the existing data.

avatar HermanPeeren
HermanPeeren - comment - 11 Apr 2013

The main reason why MD5 would eventually be less apt for password encoding is because it is fast, not because it would be broken. That has nothing to do with password storage and is only important for the use of authenticating a document. Most people just say that it is "broken", because almost everybody else is saying that. Please don't put the word "broken" in my mouth when talking about MD5 and passwords. Last thing I say about it.

As for a plugin: you can make it, use it if you need it, put it on the JED if you want to share it and if you think it is worth it, of course you can make a pull request, like you did now. I don't give you much chance, however, that particularly this will be incorporated in Jommla!'s core. But you can always try.

If you need better/other passwords, you can use it, no problem. You also don't need the passwords from the current Joomla! tables to get it all working; you are completely free to brew your own.

avatar HermanPeeren
HermanPeeren - comment - 11 Apr 2013

As for your code-lines about migrating a $oldHash to a $newHash: mind the difference between md5(md5($plainpassword.$oldsalt).$pwsecret) as you write here and md5($pwsecret.$oldsalt.$plainpassword) that you wrote in your proposed patch.

avatar mahagr
mahagr - comment - 11 Apr 2013

@andywer You misunderstood me, the reason why you cannot change passwords before user logs in is that you need to have unencrypted password in order to generate a new hash. Now you're getting hash from hash.

@HermanPeeren You're right, MD5 is probably one of the longest living hash algorithms because of it isn't broken. Computers are just getting too fast with all the GPU power which helps on brute force attacks.

avatar andywer
andywer - comment - 11 Apr 2013

@HermanPeeren I am aware of the difference between the two approaches. I was just "thinking aloud" and looking for a solution to upgrade existing data.

@mahagr That's why I had the idea of $oldHash = ... and $newHash = md5( $oldHash . $pwsalt ). In this case you may upgrade the existing hashes smoothely, because you don't need to know the plaintext password. You just use the old hash and hash it one more time using the second (out-of-database) salt.

The problem that I still see is that people do not like to install additional extensions for doing basic security stuff that doesn't add new real features. That's why I don't like the idea of putting it into a new extension.

avatar elinw
elinw - comment - 18 Apr 2013

So if you look at the crypt in the platform https://github.com/joomla/joomla-cms/blob/master/libraries/joomla/crypt/password/simple.
you will see that there is a whole new set of possiblities,

avatar andywer
andywer - comment - 18 Apr 2013

Yes, that's nice, but I see two major shortcomings regarding crypt/password/simple:
1. One still needs to add SHA256 or a similar, more secure hash algorithm as a hashing option.
2. There is no support for the additional out-of-database salt.

One could extend this class, but unfortunately it is not even used yet. Neither for the creation of users (installation/model/configuration.php), nor for the authentication (plugins/authentication/joomla/joomla.php) =(

Or maybe I should extend crypt/password/simple to solve the above mentioned problems and hope that it will be used for authentication and creation of new users in near future? ...

avatar elinw
elinw - comment - 18 Apr 2013

Of course once can extend the class, that's why it is designed hte way it is just like all the modern code in the platform/framework. And yes you could try doing that. I have to finish the work I was doing on that for the platform, but basically all its doing is giving us the php 5.5. behavior. It would be ideal to change over to using the newer class.

avatar Hackwar
Hackwar - comment - 5 Dec 2013

This proposal does not add any security to the problem at hand and it also creates a BIG issue in terms of using Joomla. An additional second salt does not help you with preventing password cracking, since right now the techniques to crack MD5 hashes are advanced enough so that it doesn't really matter how long your salt is. In fact, by adding a constant salt, you add a constant string into the whole equation. If I add "secret" as salt to every password, it doesn't make the hash any more secure.

Worst of all however is, that you can't transfer your database from one Joomla installation to the other. If you have a local test installation and set up Joomla there, then installed a fresh Joomla on your server and overwrote your database with your local one, you would have passwords that were hashed with the salt from your local installation on your remote system that has its own hash, different from your local install.

Last but not least, this would break all existing passwords and the documented method to reset your password through the DB with simply using md5().

In any case, this is superseeded by the PR #2589. I'd request to reject this PR.

And with the words of Cato the Elder: Ceterum censeo Joomlacode esse delendam (I believe that Joomlacode has to be destroyed. ;-) )

avatar betweenbrain
betweenbrain - comment - 5 Dec 2013

@andywer Thanks for the contribution. As there hasn't been any activity in 5 months, and In lieu of #2589, I'm going to close this PR. Feel free to re-open it for any further discussion of some of the points that have been brought up.

avatar betweenbrain betweenbrain - change - 5 Dec 2013
Status New Closed
Closed_Date 0000-00-00 00:00:00 2013-12-05 18:38:31
avatar betweenbrain betweenbrain - close - 5 Dec 2013

Add a Comment

Login with GitHub to post a comment