From 0005a1faaf3a388e6018e1111ad71f42e7f5e1a1 Mon Sep 17 00:00:00 2001 From: Dan Rubins Date: Thu, 24 Aug 2017 22:02:42 -0700 Subject: [PATCH 01/10] Make bcrypt rounds configurable --- packages/accounts-password/password_server.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/accounts-password/password_server.js b/packages/accounts-password/password_server.js index b088b0aafdd..4dd0ef88080 100644 --- a/packages/accounts-password/password_server.js +++ b/packages/accounts-password/password_server.js @@ -22,7 +22,7 @@ var bcryptCompare = Meteor.wrapAsync(bcrypt.compare); // "sha-256" and then passes the digest to bcrypt. -Accounts._bcryptRounds = 10; +Accounts._bcryptRounds = Accounts._options.bcryptRounds || 10; // Given a 'password' from the client, extract the string that we should // bcrypt. 'password' can be one of: @@ -67,6 +67,15 @@ Accounts._checkPassword = function (user, password) { if (! bcryptCompare(password, user.services.password.bcrypt)) { result.error = handleError("Incorrect password", false); + } else { + // password checks out, but user bcrypt may need update + if (user.services.password.bcrypt && Accounts._bcryptRounds > Number(user.services.password.bcrypt.substring(4, 6))) { + Meteor.defer(() => { + Meteor.users.update({ _id: user._id }, { + $set: { 'services.password.bcrypt': bcryptHash(password, Accounts._bcryptRounds) } + }); + }); + } } return result; From 633bc262cf24e59ca422776e028c2596497cbb7f Mon Sep 17 00:00:00 2001 From: Dan Rubins Date: Fri, 25 Aug 2017 14:31:05 -0700 Subject: [PATCH 02/10] add key "bcryptRounds" to whitelist --- packages/accounts-base/accounts_common.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/accounts-base/accounts_common.js b/packages/accounts-base/accounts_common.js index 37ec282985a..aa23c2e2191 100644 --- a/packages/accounts-base/accounts_common.js +++ b/packages/accounts-base/accounts_common.js @@ -135,7 +135,7 @@ export class AccountsCommon { // validate option keys var VALID_KEYS = ["sendVerificationEmail", "forbidClientAccountCreation", "passwordEnrollTokenExpirationInDays", "restrictCreationByEmailDomain", "loginExpirationInDays", "passwordResetTokenExpirationInDays", - "ambiguousErrorMessages"]; + "ambiguousErrorMessages", "bcryptRounds"]; _.each(_.keys(options), function (key) { if (!_.contains(VALID_KEYS, key)) { throw new Error("Accounts.config: Invalid key: " + key); From a5206a68e76341b3fe7c25d3911a2c37bccc4420 Mon Sep 17 00:00:00 2001 From: Dan Rubins Date: Tue, 5 Sep 2017 21:17:34 -0700 Subject: [PATCH 03/10] improve readability --- packages/accounts-password/password_server.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/accounts-password/password_server.js b/packages/accounts-password/password_server.js index 4dd0ef88080..e733564ef10 100644 --- a/packages/accounts-password/password_server.js +++ b/packages/accounts-password/password_server.js @@ -67,15 +67,13 @@ Accounts._checkPassword = function (user, password) { if (! bcryptCompare(password, user.services.password.bcrypt)) { result.error = handleError("Incorrect password", false); - } else { + } else if (user.services.password.bcrypt && Accounts._bcryptRounds > Number(user.services.password.bcrypt.substring(4, 6))) { // password checks out, but user bcrypt may need update - if (user.services.password.bcrypt && Accounts._bcryptRounds > Number(user.services.password.bcrypt.substring(4, 6))) { - Meteor.defer(() => { - Meteor.users.update({ _id: user._id }, { - $set: { 'services.password.bcrypt': bcryptHash(password, Accounts._bcryptRounds) } - }); + Meteor.defer(() => { + Meteor.users.update({ _id: user._id }, { + $set: { 'services.password.bcrypt': bcryptHash(password, Accounts._bcryptRounds) } }); - } + }); } return result; From b5cd99a818795504eeb8d8bc1e39f46f9f45f49f Mon Sep 17 00:00:00 2001 From: Dan Rubins Date: Tue, 5 Sep 2017 21:21:07 -0700 Subject: [PATCH 04/10] wrap changes @ 80 char --- packages/accounts-password/password_server.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/accounts-password/password_server.js b/packages/accounts-password/password_server.js index e733564ef10..4ee96e98651 100644 --- a/packages/accounts-password/password_server.js +++ b/packages/accounts-password/password_server.js @@ -67,11 +67,18 @@ Accounts._checkPassword = function (user, password) { if (! bcryptCompare(password, user.services.password.bcrypt)) { result.error = handleError("Incorrect password", false); - } else if (user.services.password.bcrypt && Accounts._bcryptRounds > Number(user.services.password.bcrypt.substring(4, 6))) { + } else if ( + user.services.password.bcrypt && + Accounts._bcryptRounds > + Number(user.services.password.bcrypt.substring(4, 6)) + ) { // password checks out, but user bcrypt may need update Meteor.defer(() => { Meteor.users.update({ _id: user._id }, { - $set: { 'services.password.bcrypt': bcryptHash(password, Accounts._bcryptRounds) } + $set: { + 'services.password.bcrypt': + bcryptHash(password, Accounts._bcryptRounds) + } }); }); } From add6fbc8f732b2c2f94e0514f826e0f657dca77a Mon Sep 17 00:00:00 2001 From: Dan Rubins Date: Tue, 5 Sep 2017 21:24:35 -0700 Subject: [PATCH 05/10] Add jsdoc comments for new option --- packages/accounts-base/accounts_common.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/accounts-base/accounts_common.js b/packages/accounts-base/accounts_common.js index aa23c2e2191..6d86013dc99 100644 --- a/packages/accounts-base/accounts_common.js +++ b/packages/accounts-base/accounts_common.js @@ -89,6 +89,9 @@ export class AccountsCommon { // - ambiguousErrorMessages {Boolean} // Return ambiguous error messages from login failures to prevent // user enumeration. + // - bcryptRounds {Number} + // Allows override of number of bcrypt rounds (aka work factor) used + // to store passwords. /** * @summary Set global accounts options. From b23ee817c0461ce393b0762d10876da832e1df29 Mon Sep 17 00:00:00 2001 From: Dan Rubins Date: Tue, 5 Sep 2017 21:31:09 -0700 Subject: [PATCH 06/10] Add notes to history.md --- History.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/History.md b/History.md index 8e0f52adafd..24f423aca0e 100644 --- a/History.md +++ b/History.md @@ -1,5 +1,9 @@ ## v.NEXT +* `Accounts.config` now supports a `bcryptRounds` option which enabled + override of the default 10 rounds currently used to secure passwords. + [PR #9044](https://github.com/meteor/meteor/pull/9044) + * The `fastclick` package (previously included by default in Cordova applications through the `mobile-experience` package) has been deprecated. This package is no longer maintained and has years of outstanding From 45f81abe694ab77bb7072e14aa2de8e6118c9511 Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Wed, 6 Sep 2017 11:04:26 -0400 Subject: [PATCH 07/10] Small History.md tweak --- History.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/History.md b/History.md index 24f423aca0e..4e626de0186 100644 --- a/History.md +++ b/History.md @@ -1,7 +1,7 @@ ## v.NEXT -* `Accounts.config` now supports a `bcryptRounds` option which enabled - override of the default 10 rounds currently used to secure passwords. +* `Accounts.config` now supports a `bcryptRounds` option that + overrides the default 10 rounds currently used to secure passwords. [PR #9044](https://github.com/meteor/meteor/pull/9044) * The `fastclick` package (previously included by default in Cordova From 0c72a6374264ca98e7fdd9517a05f0ae35ee16c0 Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Tue, 17 Oct 2017 07:47:55 -0400 Subject: [PATCH 08/10] Code cleanup; Add tests --- packages/accounts-password/password_server.js | 26 +++++----- packages/accounts-password/password_tests.js | 47 +++++++++++++++++++ 2 files changed, 61 insertions(+), 12 deletions(-) diff --git a/packages/accounts-password/password_server.js b/packages/accounts-password/password_server.js index f0282b515f8..beb8abcdf01 100644 --- a/packages/accounts-password/password_server.js +++ b/packages/accounts-password/password_server.js @@ -52,6 +52,10 @@ var hashPassword = function (password) { return bcryptHash(password, Accounts._bcryptRounds); }; +// Extract the number of rounds used in the specified bcrypt hash. +const getRoundsFromBcryptHash = + hash => hash ? Number(hash.substring(4, 6)) : null; + // Check whether the provided password matches the bcrypt'ed password in // the database user record. `password` can be a string (in which case // it will be run through SHA256 before bcrypt) or an object with @@ -63,21 +67,19 @@ Accounts._checkPassword = function (user, password) { userId: user._id }; - password = getPasswordString(password); + const formattedPassword = getPasswordString(password); + const hash = user.services.password.bcrypt; + const hashRounds = getRoundsFromBcryptHash(hash); - if (! bcryptCompare(password, user.services.password.bcrypt)) { + if (! bcryptCompare(formattedPassword, hash)) { result.error = handleError("Incorrect password", false); - } else if ( - user.services.password.bcrypt && - Accounts._bcryptRounds > - Number(user.services.password.bcrypt.substring(4, 6)) - ) { - // password checks out, but user bcrypt may need update + } else if (hash && Accounts._bcryptRounds != hashRounds) { + // The password checks out, but the user's bcrypt hash needs to be updated. Meteor.defer(() => { Meteor.users.update({ _id: user._id }, { - $set: { - 'services.password.bcrypt': - bcryptHash(password, Accounts._bcryptRounds) + $set: { + 'services.password.bcrypt': + bcryptHash(formattedPassword, Accounts._bcryptRounds) } }); }); @@ -92,7 +94,7 @@ var checkPassword = Accounts._checkPassword; /// const handleError = (msg, throwError = true) => { const error = new Meteor.Error( - 403, + 403, Accounts._options.ambiguousErrorMessages ? "Something went wrong. Please check your credentials." : msg diff --git a/packages/accounts-password/password_tests.js b/packages/accounts-password/password_tests.js index a34b3b9a7d5..cb1115298e0 100644 --- a/packages/accounts-password/password_tests.js +++ b/packages/accounts-password/password_tests.js @@ -1886,4 +1886,51 @@ if (Meteor.isServer) (function () { ]); }); + Tinytest.addAsync( + 'passwords - allow custom bcrypt rounds', + function (test, done) { + function getUserHashRounds(user) { + return Number(user.services.password.bcrypt.substring(4, 6)); + } + + // Verify that a bcrypt hash generated for a new account uses the + // default number of rounds. + let username = Random.id(); + const password = 'abc123'; + const userId1 = Accounts.createUser({ username, password }); + let user1 = Meteor.users.findOne(userId1); + let rounds = getUserHashRounds(user1); + test.equal(rounds, Accounts._bcryptRounds); + + // When a custom number of bcrypt rounds is set via Accounts.config, + // and an account was already created using the default number of rounds, + // make sure that a new hash is created (and stored) using the new number + // of rounds, the next time the password is checked. + const defaultRounds = Accounts._bcryptRounds; + const customRounds = 11; + Accounts._bcryptRounds = customRounds; + Accounts._checkPassword(user1, password); + Meteor.setTimeout(() => { + user1 = Meteor.users.findOne(userId1); + rounds = getUserHashRounds(user1); + test.equal(rounds, customRounds); + Accounts._options.bcryptRounds = null; + + // When a custom number of bcrypt rounds is set, make sure it's + // used for new bcrypt password hashes. + username = Random.id(); + const userId2 = Accounts.createUser({ username, password }); + const user2 = Meteor.users.findOne(userId2); + rounds = getUserHashRounds(user2); + test.equal(rounds, customRounds); + + // Cleanup + Accounts._bcryptRounds = defaultRounds; + Meteor.users.remove(userId1); + Meteor.users.remove(userId2); + done(); + }, 5000); + } + ); + }) (); From 739ad1666fa3cdc3ae132953dc76c1f31a43f04a Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Tue, 17 Oct 2017 07:50:40 -0400 Subject: [PATCH 09/10] Bump package versions --- packages/accounts-base/package.js | 2 +- packages/accounts-password/package.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/accounts-base/package.js b/packages/accounts-base/package.js index 7a804355477..404e2bccadd 100644 --- a/packages/accounts-base/package.js +++ b/packages/accounts-base/package.js @@ -1,6 +1,6 @@ Package.describe({ summary: "A user account system", - version: "1.3.4" + version: "1.3.6" }); Package.onUse(function (api) { diff --git a/packages/accounts-password/package.js b/packages/accounts-password/package.js index 88500d93d54..47a214f1006 100644 --- a/packages/accounts-password/package.js +++ b/packages/accounts-password/package.js @@ -4,7 +4,7 @@ Package.describe({ // release process, so versions 2.0.0-beta.2 through -beta.5 and -rc.0 // have already been published. The next time this package reaches 2.x // territory, I would recommend jumping straight to 2.1.0. - version: "1.4.1" + version: "1.4.2" }); Package.onUse(function(api) { From f9e152fba0ad7ac9469e684b723aafcdedf08727 Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Thu, 19 Oct 2017 07:41:12 -0400 Subject: [PATCH 10/10] Switch to parseInt; Use safer method of extracting rounds --- packages/accounts-password/password_server.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/accounts-password/password_server.js b/packages/accounts-password/password_server.js index beb8abcdf01..18043447249 100644 --- a/packages/accounts-password/password_server.js +++ b/packages/accounts-password/password_server.js @@ -53,8 +53,16 @@ var hashPassword = function (password) { }; // Extract the number of rounds used in the specified bcrypt hash. -const getRoundsFromBcryptHash = - hash => hash ? Number(hash.substring(4, 6)) : null; +const getRoundsFromBcryptHash = hash => { + let rounds; + if (hash) { + const hashSegments = hash.split('$'); + if (hashSegments.length > 2) { + rounds = parseInt(hashSegments[2], 10); + } + } + return rounds; +}; // Check whether the provided password matches the bcrypt'ed password in // the database user record. `password` can be a string (in which case