Merge pull request #711 from thelounge/xpaw/bcrypt

Change bcrypt rounds from 8 to 11

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

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,

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);
+}

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) {

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;
+	});
+});