HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpModerate

Iteration of password hashing in PHP

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
hashingiterationphppassword

Problem

I'm trying to improve the security for my backend:

/**
 *  This is the script that is executed when I add a new user - after the input has passed the validation.
 */
    $salt = base64_encode(mcrypt_create_iv(24,MCRYPT_DEV_URANDOM));  //  generate a random 32 character salt
    $hash = hash('sha256',$salt.$_POST['secret']);  //  create a 64 character hash based on the salt and the password

    $ins = $con['site']->prepare('INSERT INTO users (handle, salt, hash) VALUES (:handle, :salt, :hash)');
    $ins->execute(array(':handle'=>$_POST['handle'], ':salt'=>$salt, ':hash'=>$hash));
    if($ins){
        KD::addNotice('success','...message...');
    } else {
        KD::addNotice('error','...message...');
    }

/**
 *  This is the script that is executed when the user is logging in - also after the input has passed the validation.
 */
    $qry = $con['site']->prepare('SELECT handle, salt, hash FROM users WHERE handle = :handle');
    $qry->execute(array(':handle'=>$_POST['handle']));  //  look up the handle (username)
    if($qry->rowCount()==1){
        $get = $qry->fetch();
        if(hash('sha256',$get['salt'].$_POST['secret'])==$get['hash']){
            KD::addNotice('success','...message...');
            session_regenerate_id();
            $_SESSION['backend']->login($get['handle']);
        } else {
            KD::addNotice('error','...message...');
        }
    } else {
        KD::addNotice('error','...message...');
    }


This is as far as I've gotten and it works. I'm generating an unpredictable salt, creating a hash and saving it to a database. Although this is an improvement from what it used to be, I'm fully aware of that there's more work to be done here.

I've been reading a bit about iterations. Or, rehashing the password, or the salt, or the hash, a couple of thousand times in order to prevent different forms of attacks. Could some of you please tell me how something like that could be implemented into my code?

Is it just putting

Solution

I'm no expert on security, but I believe this is not current best practice. I'll copy from an another answer of mine.

Cryptographic Right Answers (2009, Colin Percival, author of scrypt)


Password handling: As soon as you receive a password, hash it using
scrypt or PBKDF2 and erase the plaintext password from memory.


Do NOT
store users' passwords. Do NOT hash them with MD5. Use a real key
derivation algorithm. PBKDF2 is the most official standard; but scrypt
is stronger. Please keep in mind that even if YOUR application isn't
particularly sensitive, your users are probably re-using passwords
which they have used on other, more sensitive, websites -- so if you
screw up how you store your users' passwords, you might end up doing
them a lot of harm.

How To Safely Store A Password (2010)


Use bcrypt


Use bcrypt. Use bcrypt. Use bcrypt. Use bcrypt. Use bcrypt.
Use bcrypt. Use bcrypt. Use bcrypt. Use bcrypt.


...


Why Not {MD5, SHA1, SHA256, SHA512, SHA-3, etc}?


These are all general purpose hash functions, designed to calculate a digest of huge amounts of data in as short a time as possible. This means that they are fantastic for ensuring the integrity of data and utterly rubbish for storing passwords.


...


Salts Will Not Help You


It’s important to note that salts are useless for preventing dictionary attacks or brute force attacks. You can use huge salts or many salts or hand-harvested, shade-grown, organic Himalayan pink salt. It doesn’t affect how fast an attacker can try a candidate password, given the hash and the salt from your database.


Salt or no, if you’re using a general-purpose hash function designed for speed you’re well and truly effed.

How to securely hash passwords? (2013)


Conclusion


Use bcrypt. PBKDF2 is not bad either. If you use scrypt you will be a
"slightly early adopter" with the risks that are implied by this
expression; but it would be a good move for scientific progress
("crash dummy" is a very honourable profession).

Context

StackExchange Code Review Q#63729, answer score: 11

Revisions (0)

No revisions yet.