1
votes

Yet another question regarding password hashing.

Hi,

I am developing a NodeJS back-end-server real-time-transfer module and authentication for a website that runs a single thread, there's a multitude of reasons for this. However as functions such as bcrypt/scrypt is heavily CPU intensive these could potentially cause thread blocking and in return bad end-user experience.

For many years I have used a function like this, for generating hashes and comparison.

const crypt = require("crypto");

let p = "some_random_password";
let s = salt();
let h = hash(p,s);

console.log("Hash               :", h);
console.log("Salt               :", s);
console.log("Password matched   :", comparepass(p,h,s));

// Generate hash.
function hash(password,salt){
    let r = crypt.createHash("sha256").update(password).digest("hex");
    let s = crypt.createHash("sha256").update(salt).digest("hex");
    for(i=0;i<10;i++)
        r = crypt.createHash("sha256").update(r+s).digest("hex");
    return r;
}

// Compare hash with a given password + salt.
function comparepass(password,hash,salt) {
    let c = crypt.createHash("sha256").update(password).digest("hex");
    let s = crypt.createHash("sha256").update(salt).digest("hex");
    for(i=0;i<10;i++)
        c = crypt.createHash("sha256").update(c+s).digest("hex");
    return c == hash;
}

// Generate random string 42 characters long obviously.
function salt(length = 42) {
    var r = '';
    var characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
    var cLen = characters.length;
    for (var i = 0; i < length; i++)
        r += characters.charAt(Math.floor(Math.random()* cLen));
    return crypt.createHash("sha256").update(r).digest("hex");
}

Thought proses behind why I initially don't salt the password is that hash and salt is stored in same table. Hence attacker could potentially brute force and once string that includes salt is present know that attack was successful.

But is this safe? Or am I overlooking a obvious flaw by doing this?

1

1 Answers

3
votes

Password stretchers like scrypt, bcrypt, and PBKDF2 are intended to be CPU intensive. That's why they exist. They are all tunable to change the work requirement if a given configuration is too expensive for your use case.

Just hashing 10 times is not as secure as applying a proper password stretcher with an iteration count of 10. If it were, no one would have bothered developing password stretchers.

Where possible, I highly recommend that you move the stretching component to the client, and send the output to the server. This takes load off the server, but more importantly avoids sending the raw password to the server at all. The server can then apply a single hash iteration at very low cost. Several thousand iterations of PBKDF2 can be easily done in under 100ms on most browsers, and 10k iterations can be easily performed using native code. 100ms on the server can be a big deal. 100ms on the client can generally be hidden one time.

If the client saves passwords, it can save the output of the password stretcher. Again, this improves security because the raw password is never stored anywhere (even encrypted), and improves performance by avoiding re-stretching.

But at a minimum, I would replace your ad hoc solution with a standard password stretcher, and tune its iteration count to your performance requirements.