Node.js password salting/hashing

In view of the recent LinkedIn breach, would anyone care to review my data access routines relating to user access?

This is a standard node.js module, accessing a Postgres database.

(function () {

    'use strict';

    var crypto = require('crypto'),
        hash = function (pass, salt) {
            var h = crypto.createHash('sha512');

            h.update(pass);
            h.update(salt);

            return h.digest('base64');
        };

    module.exports.getUser = function (conn, email, password, callback) {
        conn.query({
            name: 'getUser',
            text: 'SELECT "id", "passwordHash" FROM "user" WHERE "email" = $1 LIMIT 1',
            values: [email]
        }, function (err, result) {
            if (err) {
                throw err;
            }
            if (result.rows.length === 0) {
                callback(null);
            } else {
                var newHash = hash(password, email),
                    row = result.rows[0];

                if (row.passwordHash === newHash) {
                    callback({id: row.id, email: email});
                } else {
                    callback(null);
                }
            }
        });
    };

    module.exports.addUser = function (conn, email, password, callback) {
        var newHash = hash(password, email);

        conn.query({
            name: 'addUser',
            text: 'INSERT INTO "user" ("email", "passwordHash") VALUES ($1, $2) RETURNING "id"',
            values: [email, newHash]
        }, function (err, result) {
            var id;

            if (err) {
                console.error(err);
                callback(null);
            } else {
                id = result.rows[0].id;

                callback({id: id, email: email});
            }
        });
    };

}());

I do not see anything immediately wrong with your addUser() and getUser() implementations.

Your hash() implementation could be improved in two areas though.

-
Salt

As already indicated by Bill Barry in the comments, your should use a better salt.

Some rules to keep in mind when handling salts,

-
Salts should be unique per password.

Every password should have a unique salt. Sharing a salt reduces its effectiveness.

-
Salts should be unpredictable

You should generate a salt using a Cryptographically Secure Pseudo-Random Number Generator (CSPRNG).

You may want to try using crypto.randomBytes(size, [callback]) if available.

-
Salts should be large

PKCS#5 recommends a salt be no smaller than 64-bits (8 bytes).

I like to start with 128-bit salts (16 bytes), favoring higher salt-lengths for more passwords and better security.

-
Salts are not secret

Although purposefully disclosing the salt isn't a good idea, you can store them in cleartext.

-
Key stretching / Iterations

Wikipedia has a good description on Key Stretching,


key stretching refers to techniques used to make a possibly weak key, typically a password or passphrase, more secure against a brute force attack by increasing the time it takes to test each possible key.

The crypto module does not appear to offer key stretching natively for sha512.

You should look into using crypto.pbkdf2(password, salt, iterations, keylen, callback) which uses HMAC-SHA1. You shouldn't use SHA1 on its own, but pbkdf2 using HMAC-SHA1 is still secure.

See Password-Based Key Derivation Function 2 for more information.

You will want to set the number of iterations as high as possible while still maintaining an acceptable responsiveness for your application. Though PKCS#5 recommends a minimum of 1000 iterations, 10000+ iterations is fairly typical; computers are only getting faster.

StackExchange Code Review Q#12330, answer score: 7