RFC: Proposal for changes to password hashing

💡 Suggestion Box
1

I have been looking at the password hashing section of the Users module and found that it could do with a refresh for the current age. Please read my proposals and comment on whether you think this area is worthy of some work.

What the current system does right:

  • It checks for a stronger hashing mechanism (CRYPT_EXT_DES)
  • It uses per-record random salt
  • It falls back to MD5 when the stronger hash mechanism isn’t there - weaker security but portable, it’s guaranteed to hash
  • It uses the built-in PHP crypt() method, rather than calling MD5 or SHA1 directly

What the current system does wrong:

  • It stops looking if it finds CRYPT_EXT_DES and there are far stronger hash methods these days
  • There is no way for the system to record the current method used for the hash that is saved in the database - it guesses the method by looking at the first character of the hash and the length of the hash
  • It MD5’s the plaintext before passing it to the hashing mechanism - why?

Proposals:
2 things need to change. The first is fairly easy, which is to choose a stronger hash. I have created an experimental branch that uses openwall.com’s phpass - a portable password hasher that uses work units and strong hashing. I have tested it on a clean install and it works fine, for the initial user and created users.

The second is a lot harder. My simple testing works for fresh installs but a lot of people are going to have legacy installs with current users in the databases. suiteCRM needs to be updated to know what the hash method used is. This requires:

  • Field password_hash_method VARCHAR(6) DEFAULT ‘LEGACY’ being added to the users table
  • Field password_work_unit INT DEFAULT 8 being added to the users table
  • Preferred hash method and work unit being added to the global options
  • Code in the check areas to read the current hash method and use the correct check for logins
  • Code in the hash areas to read the current preferred method from globals to use the current hash method
  • Code on login to see if the preferred method has changed since the last user hash and re-hash the password using the current method - this is (should be) the only time we will have the plaintext in memory

Steps to test:

  • Grab a fresh copy
  • Grab PasswordHash.php and Users.php from my branch
  • Run through the install as normal (fixing issue #45 if needed)
  • Check your users table for the stronger hashes

Please let me know what you think - whether this work has merit and deserves further work to fully implement.

1 Like
2

I agree it would be great to upgrade the password technology, encryption etc., from 2005 to 2014 standards.

These 4 points should bring it up to date.

  1. The standard for password security in 2014 is “Two Factor Authentication” (2FA). Something you have (your mobile phone) and something you know (your password). This could be enabled/disabled per-user in their own user preferences page, with a default “on” or “off” depending on the CRM admin settings. When the user is trying to log in from a different machine than usual, they would also need to enter a 5-10 digit one-time code number, that they receive immediately in their smartphone, with a free app such as “Google Authenticator”. The admin must install a free SugarCRM module to actually do the work of sending out the one-time code to users who are trying to login away from their usual machines.
    http://sourceforge.net/projects/openotppluginsugarcrm/

  2. Add columns to the database schema. Which hashing technology is used in the password hash, and intelligently upgrade old MD5 etc. hashes by re-hashing the passwords on the fly to the strongest one the server supports, and storing them. Also the user preference of 2FA: on, off, default. The user’s list of machine ID’s they usually log in from and therefore don’t want to use 2FA login from them, just regular one factor password authentication.

  3. Might as well also handle the issue of encrypting sensitive personal information fields that would be vulnerable to fraud or identity theft if left in plaintext, such as “social security number”, “date of birth”, “passport number”, “drivers license number”, :“bank account number”, “security question”, “security answer”, etc. To achieve this, all that needs to be done is to enable the code in Studio to turn on MySQL “encrypted” fields. This enables modules that don’t leave sensitive information in plaintext in the database. This is a huge security vulnerability that needs fixing for Suite to be used in certain business or government environments.

  4. Password policy. Something the company CRM admin would set in the Admin page. Minimum password length (default to 10), different case letters required (default to on), At least one symbol required (default to on), At least one number (default to on), Expires after X months (default to 0 meaning no password expiration). The password generator , which is the code that sends out a new password to new users and to users who forgot their passwords, would have to generate passwords that fit the settings in the password policy. Settings would be saved in config.php

3

Hi Skrrp,

Sounds like this would be a great addition to SuiteCRM. As you say making it compatible with legacy versions would be the tricky part. However:

* Field password_hash_method VARCHAR(6) DEFAULT 'LEGACY' being added to the users table
* Field password_work_unit INT DEFAULT 8 being added to the users table

I don’t think storing these in the database is necessary, PHPass (and other implementations I believe) store the salt, hashing algorithm, e.t.c. in the hash that is returned which keeps things simpler.

Thanks,
Jim

4

Jim,

phpass does indeed store the method and work unit in the hash, but I propose adding these fields for both backwards and forwards compatibility. The system will need a way to differentiate between the current method (for legacy) and the new method. Adding the fields will help with this, far better than any fuzzy method of examining the hash and guessing.

It also means that implementations other than phpass can be brought in in the future, with no further structural changes needed.

I include work unit as a field because even though phpass does store the work unit in the hash, it is a required value to instantiate the object. As the hashing method may change in the future it may not be easy (or possible) to inspect the hash to get the work unit. Storing it database side also means that if the admin changes the work unit but not the algorithm in the future, the current hash can be invalidated and recalculated for the new preferred work unit.

Chris,

Thank you for your well informed post.

Point 3 is a little outside the scope of discussion of password security but is a very valid point in its own right. During my travels in the codebase I did notice some attempts at encryption routines but I think this is best handled by the database layer, if only to allow a sys admin to extract the data from the database without having to run PHP against the fields. The thing to remember here is that suiteCRM does not use MySQL only and any changes should not break functionality in MS SQL - a problem I have encountered in other open source projects that support multiple database engines.

Point 4 is UI side only and looks to be easily achievable.

We are agreed on point 2.

Point 1 - I hadn’t even thought of 2FA up to this point, I was simply looking at the strength of the stored hash, but this is a good idea. I don’t think that a mobile app is needed - this assumes that each user has a mobile device capable of running the non-open-source Google Authenticator or that we write an app for each mobile OS.

A simpler to implement approach would be similar to how Valve operates its Steam Guard system - on every ‘new’ login (computer, location change, etc) they email a 5 character one time key to your email address to allow access to this machine. This is slightly less secure than having a separate physical device but easier to implement.

Obviously, both can be implemented and the sys admin can decide which system they wish to use. I would also recommend some form of IP whitelisting for any 2FA implementation, so that the sys admin can mark corporate offices as not needing to 2FA at all.

5

To avoid changing the users table schema, rather than store only the hashed password in the column, instead store a serialized json array of hashed password, hash method, and work unit (this is just a long string surrounded by brackets and delimited by type, colon, value, semicolon). The php code can determine dynamically whether the text is a json array, or a legacy hash string, and process the text accordingly.

6

This is a clever idea in that it gets out of having to modify the schema and reduces the complexity on upgrading.

My only worries about this (and they are not major) are -

I was trying to get away from string sniffing to determine type, but if we have a guaranteed pattern for legacy (not starting with a “(”) and for new (starting with a “(”) and new then self-contains the metadata needed to interpret, I don’t see this as being a major problem.

The other is adding extra characters into the hash field, a varchar(255). That isn’t going to be an issue today, where phpass hashes are 60 characters and even SHA512 hashes are only 128 but might become so in the future when metadata + hash > 255 characters. Still, if the current methods are ~10 years old and we fix them today, I’m willing to kick this issue into the future for devs in 2025 to deal with.

Forgive me for my ignorance on this next bit, I’m fairly new to the codebase. I’m guessing when you said to store the 2FA yes/no preference, you would be storing this in the user_preferences table as 1 key/value record per user? As in no further schema changes for this too?

7

And one quick question before I dive in and start modifying code - does anyone know why the current method MD5’s the plaintext before passing it to the hashing method?

I’ve noticed this pattern inside functions that call the hash and I think for the sake of not breaking things it might be easier to continue using this pattern. I know this is intellectually lazy but can anyone provide any arguments for keeping it in or attempting to remove it?

8
  • To check the hash is/isn’t the new json encoded serialized object.

function is_json($string,$return_data = false) {
  $data = json_decode($string);
     return (json_last_error() == JSON_ERROR_NONE) ? ($return_data ? $data : TRUE) : FALSE;
}

More here… http://arjunphp.com/check-is-a-string-valid-json-php/

  • Definitely increase the size of the hash to varchar(255) the extra characters will be needed when storing the json encoded object.

  • Why is cleartext password MD5’d before passing around to other authentication code ? Probably because of security. “Trust no one.” You don’t just go passing clearext passwords around when a hash will do. And you’re only storing/comparing to the hash anyway. So you don’t ever even want the cleartext password. You want to be working with the hash. So, you hash the cleartext password immediately.

9

I don’t know the reason that the password is MD5’d before being passed to the hash method but why not just use the proper hash? If the plaintext is being MD5’d then passed around it’s probably easy enough to change this to use the actual hash method straight away.

10

[quote=“Jim” post=10140]

The reason for hashing to md5 first probably was because you should be very careful when comparing hashes. In certain cases, information can be leaked by using a timing attack. It takes advantage of the == operator only comparing until it finds a difference in the two strings. To prevent it, you have two options.

Option 1: hash both hashed strings first - this doesn’t stop the timing difference, but it makes the information useless.
Option 2: always compare the whole string.

11

Can you elaborate on why this makes the information useless? It seems to me like this would still leak the same amount of information.

12

Here is the RFC for Time-Based One Time Passwords, “TOTP”, used to make a 2FA pin code.
http://tools.ietf.org/html/rfc6238

Here is some example PHP code to generate a TOTP pin code, default 8 digits long.

<?php
/**
* This function implements the algorithm outlined
* in RFC 6238 for Time-Based One-Time Passwords
*
* @link http://tools.ietf.org/html/rfc6238
* @param string $key    the string to use for the HMAC key
* @param mixed  $time   a value that reflects a time (unix
*                       time in the example)
* @param int    $digits the desired length of the OTP
* @param string $crypto the desired HMAC crypto algorithm
* @return string the generated OTP
*/
function oauth_totp($key, $time, $digits=8, $crypto='sha256')
{
    $digits = intval($digits);
    $result = null;
    
    // Convert counter to binary (64-bit)       
    $data = pack('NNC*', $time >> 32, $time & 0xFFFFFFFF);
    
    // Pad to 8 chars (if necessary)
    if (strlen ($data) < 8) {
        $data = str_pad($data, 8, chr(0), STR_PAD_LEFT);
    }        
    
    // Get the hash
    $hash = hash_hmac($crypto, $data, $key);
    
    // Grab the offset
    $offset = 2 * hexdec(substr($hash, strlen($hash) - 1, 1));
    
    // Grab the portion we're interested in
    $binary = hexdec(substr($hash, $offset, 8)) & 0x7fffffff;
    
    // Modulus
    $result = $binary % pow(10, $digits);
    
    // Pad (if necessary)
    $result = str_pad($result, $digits, "0", STR_PAD_LEFT);
    
    return $result;
}
?>
13

[quote=“Jim” post=10153]

Knowing the first X digits of a password hash is better than knowing the first X digits of the hash of the hash. But it’s close, given today’s technology (GPU cards that can calculate 700 million password sha1 hashes per second).

14

I suggest using bcrypt instead of the high speed hashing algorithms such as sha1, sha256, sha512, etc.

It’s far safer against today’s technology on brute force attacks because it’s made to run slower which frustrates unbounded offline attacks against a stolen/copied database.

http://codahale.com/how-to-safely-store-a-password/

A password should be hashed in a way that even an offline attack (for example from a leaked backup copy of the database, or a rogue sysadmin) will not compromise the passwords.

Bcrypt can protect against this because it uses a work factor, so it’s about 1,000,000 times slower than the high speed hashing algorithms. Under 1 microsecond for something like a sha1, vs 0.3 seconds to compute a bcrypt hash. So it’d be 40 seconds vs. 12 years to guess the password.

More info
http://www.the-art-of-web.com/php/blowfish-crypt/
http://stackoverflow.com/questions/4795385/how-do-you-use-bcrypt-for-hashing-passwords-in-php
Example that uses Zend library (which appears to be included in SuiteCRM) to use bcrypt.

Zend\Crypt\Password\Bcrypt (PHP 5.3.2+)

function register($username, $password) {
    $bcrypt = new Zend\Crypt\Password\Bcrypt();
    $hash = $bcrypt->create($password);
    save($user, $hash);
}

function login($username, $password) {
    $hash = loadHashByUsername($username);
    $bcrypt = new Zend\Crypt\Password\Bcrypt();
    if ($bcrypt->verify($password, $hash)) {
        //login
    } else {
        // failure
    }
}
15

More details on using zend library to bcrypt
http://websec.io/2013/01/21/Password-Hashing-with-Zend-Crypt.html