From c5e0dee3a39562ecacc1f57cc475c054fb532476 Mon Sep 17 00:00:00 2001 From: Pavel Djundik Date: Fri, 21 Oct 2016 22:00:43 +0300 Subject: [PATCH] Change bcrypt rounds from 8 to 11 --- src/command-line/add.js | 4 +--- src/command-line/reset.js | 3 +-- src/helper.js | 19 +++++++++++++++++++ src/server.js | 39 +++++++++++++++++++++++++-------------- test/tests/passwords.js | 27 +++++++++++++++++++++++++++ 5 files changed, 73 insertions(+), 19 deletions(-) create mode 100644 test/tests/passwords.js diff --git a/src/command-line/add.js b/src/command-line/add.js index 2e3723e0..2a852663 100644 --- a/src/command-line/add.js +++ b/src/command-line/add.js @@ -1,7 +1,6 @@ "use strict"; var ClientManager = new require("../clientManager"); -var bcrypt = require("bcrypt-nodejs"); var program = require("commander"); var Helper = require("../helper"); @@ -26,8 +25,7 @@ program }); function add(manager, name, password) { - var salt = bcrypt.genSaltSync(8); - var hash = bcrypt.hashSync(password, salt); + var hash = Helper.password.hash(password); manager.addUser( name, hash diff --git a/src/command-line/reset.js b/src/command-line/reset.js index e5cf6609..511bd554 100644 --- a/src/command-line/reset.js +++ b/src/command-line/reset.js @@ -1,6 +1,5 @@ "use strict"; -var bcrypt = require("bcrypt-nodejs"); var ClientManager = new require("../clientManager"); var fs = require("fs"); var program = require("commander"); @@ -24,7 +23,7 @@ program if (err) { return; } - user.password = bcrypt.hashSync(password, bcrypt.genSaltSync(8)); + user.password = Helper.password.hash(password); user.token = null; // Will be regenerated when the user is loaded fs.writeFileSync( file, diff --git a/src/helper.js b/src/helper.js index 3db79ca1..d4af765f 100644 --- a/src/helper.js +++ b/src/helper.js @@ -5,6 +5,7 @@ var _ = require("lodash"); var path = require("path"); var os = require("os"); var fs = require("fs"); +var bcrypt = require("bcrypt-nodejs"); var Helper = { config: null, @@ -14,6 +15,12 @@ var Helper = { setHome: setHome, getVersion: getVersion, getGitCommit: getGitCommit, + + password: { + hash: passwordHash, + compare: passwordCompare, + requiresUpdate: passwordRequiresUpdate, + }, }; module.exports = Helper; @@ -83,3 +90,15 @@ function expandHome(shortenedPath) { return path.resolve(shortenedPath.replace(/^~($|\/|\\)/, home + "$1")); } + +function passwordRequiresUpdate(password) { + return bcrypt.getRounds(password) !== 11; +} + +function passwordHash(password) { + return bcrypt.hashSync(password, bcrypt.genSaltSync(11)); +} + +function passwordCompare(password, expected) { + return bcrypt.compareSync(password, expected); +} diff --git a/src/server.js b/src/server.js index bea7776b..d30a2449 100644 --- a/src/server.js +++ b/src/server.js @@ -2,7 +2,6 @@ var _ = require("lodash"); var pkg = require("../package.json"); -var bcrypt = require("bcrypt-nodejs"); var Client = require("./client"); var ClientManager = require("./clientManager"); var express = require("express"); @@ -192,15 +191,14 @@ function init(socket, client) { }); return; } - if (!bcrypt.compareSync(old || "", client.config.password)) { + if (!Helper.password.compare(old || "", client.config.password)) { socket.emit("change-password", { error: "The current password field does not match your account password" }); return; } - var salt = bcrypt.genSaltSync(8); - var hash = bcrypt.hashSync(p1, salt); + var hash = Helper.password.hash(p1); client.setPassword(hash, function(success) { var obj = {}; @@ -259,17 +257,30 @@ function reverseDnsLookup(socket, client) { } function localAuth(client, user, password, callback) { - var result = false; - try { - result = bcrypt.compareSync(password || "", client.config.password); - } catch (error) { - if (error === "Not a valid BCrypt hash.") { - log.error("User (" + user + ") with no local password set tried to sign in. (Probably a LDAP user)"); - } - result = false; - } finally { - callback(result); + if (!client || !password) { + return callback(false); } + + if (!client.config.password) { + log.error("User", user, "with no local password set tried to sign in. (Probably a LDAP user)"); + return callback(false); + } + + var result = Helper.password.compare(password, client.config.password); + + if (result && Helper.password.requiresUpdate(client.config.password)) { + var hash = Helper.password.hash(password); + + client.setPassword(hash, function(success) { + if (!success) { + log.error("Failed to update password of", client.name, "to match new security requirements"); + } else { + log.info("User", client.name, "logged in and their hashed password has been updated to match new security requirements"); + } + }); + } + + return callback(result); } function ldapAuth(client, user, password, callback) { diff --git a/test/tests/passwords.js b/test/tests/passwords.js new file mode 100644 index 00000000..d074477d --- /dev/null +++ b/test/tests/passwords.js @@ -0,0 +1,27 @@ +"use strict"; + +const expect = require("chai").expect; +const Helper = require("../../src/helper"); + +describe("Client passwords", function() { + const inputPassword = "my$Super@Cool Password"; + + it("hashed password should match", function() { + // Generated with third party tool to test implementation + let comparedPassword = Helper.password.compare(inputPassword, "$2a$11$zrPPcfZ091WNfs6QrRHtQeUitlgrJcecfZhxOFiQs0FWw7TN3Q1oS"); + + expect(comparedPassword).to.be.true; + }); + + it("freshly hashed password should match", function() { + let hashedPassword = Helper.password.hash(inputPassword); + let comparedPassword = Helper.password.compare(inputPassword, hashedPassword); + + expect(comparedPassword).to.be.true; + }); + + it("shout passwords should be marked as old", function() { + expect(Helper.password.requiresUpdate("$2a$08$K4l.hteJcCP9D1G5PANzYuBGvdqhUSUDOLQLU.xeRxTbvtp01KINm")).to.be.true; + expect(Helper.password.requiresUpdate("$2a$11$zrPPcfZ091WNfs6QrRHtQeUitlgrJcecfZhxOFiQs0FWw7TN3Q1oS")).to.be.false; + }); +});