From 8ecb80960ffd2ae2f241b0bbb62367821a79ff63 Mon Sep 17 00:00:00 2001 From: John Molomby Date: Tue, 10 Oct 2017 16:42:45 +1100 Subject: [PATCH] Expanding password field configuration options and improving defaults (to align with current NIST guidelines). Updating, expanding on and clarifying relevant docs. --- fields/types/password/PasswordType.js | 51 ++++++++++++++---------- fields/types/password/Readme.md | 57 ++++++++++++++++++++------- package.json | 1 + 3 files changed, 73 insertions(+), 36 deletions(-) diff --git a/fields/types/password/PasswordType.js b/fields/types/password/PasswordType.js index f5aa86155cf..b46965fdb6c 100644 --- a/fields/types/password/PasswordType.js +++ b/fields/types/password/PasswordType.js @@ -3,6 +3,8 @@ var bcrypt = require('bcrypt-nodejs'); var FieldType = require('../Type'); var util = require('util'); var utils = require('keystone-utils'); +var dumbPasswords = require('dumb-passwords'); + var regexChunk = { digitChar: /\d/, @@ -18,20 +20,23 @@ var detailMsg = { lowChar: 'use at least one lower case character', upperChar: 'use at least one upper case character', }; +const defaultOptions = { min: 8, max: 72, workFactor: 10, rejectCommon: true }; + /** * password FieldType Constructor * @extends Field * @api public */ function password (list, path, options) { - this.options = options; + // Apply default and enforced options (you can't sort on password fields) + options = Object.assign({}, defaultOptions, options, { nosort: false }); + this._nativeType = String; this._underscoreMethods = ['format', 'compare']; this._fixedSize = 'full'; - // You can't sort on password fields - options.nosort = true; - this.workFactor = options.workFactor || 10; + password.super_.call(this, list, path, options); + for (var key in this.options.complexity) { if ({}.hasOwnProperty.call(this.options.complexity, key)) { if (key in regexChunk !== key in this.options.complexity) { @@ -42,8 +47,8 @@ function password (list, path, options) { } } } - if (this.options.max <= this.options.min) { - throw new Error('FieldType.Password: options - min must be set at a lower value than max.'); + if (this.options.max < this.options.min) { + throw new Error('FieldType.Password: options - maximum password length cannot be less than the minimum length.'); } } password.properName = 'Password'; @@ -90,7 +95,7 @@ password.prototype.addToSchema = function (schema) { return next(); } var item = this; - bcrypt.genSalt(field.workFactor, function (err, salt) { + bcrypt.genSalt(field.options.workFactor, function (err, salt) { if (err) { return next(err); } @@ -161,44 +166,48 @@ password.prototype.compare = function (item, candidate, callback) { * Asynchronously confirms that the provided password is valid */ password.prototype.validateInput = function (data, callback) { - var min = this.options.min; - var max = this.options.max; - var complexity = this.options.complexity; + var { min, max, complexity, rejectCommon } = this.options; var confirmValue = this.getValueFromData(data, '_confirm'); var passwordValue = this.getValueFromData(data); - var validation = validate(passwordValue, confirmValue, min, max, complexity); + var validation = validate(passwordValue, confirmValue, min, max, complexity, rejectCommon); utils.defer(callback, validation.result, validation.detail); }; -var validate = password.validate = function (pass, confirm, min, max, complexity) { - var detail = ''; - var result = true; - max = max || 72; +var validate = password.validate = function (pass, confirm, min, max, complexity, rejectCommon) { + var messages = []; + if (confirm !== undefined && pass !== confirm) { - detail = 'passwords must match\n'; + messages.push('Passwords must match.'); } if (min && typeof pass === 'string' && pass.length < min) { - detail += 'password must be longer than ' + min + ' characters\n'; + messages.push('Password must be longer than ' + min + ' characters.'); } if (max && typeof pass === 'string' && pass.length > max) { - detail += 'password must not be longer than ' + max + ' characters\n'; + messages.push('Password must not be longer than ' + max + ' characters.'); } for (var prop in complexity) { if (complexity[prop] && typeof pass === 'string') { var complexityCheck = (regexChunk[prop]).test(pass); if (!complexityCheck) { - detail += detailMsg[prop] + '\n'; + messages.push(detailMsg[prop]); } } } - result = detail.length === 0; - return { result, detail }; + + if (rejectCommon && dumbPasswords.check(pass)) { + messages.push('Password must not be a common, frequently-used password.'); + } + + return { + result: messages.length === 0, + detail: messages.join(' \n'), + }; }; /** diff --git a/fields/types/password/Readme.md b/fields/types/password/Readme.md index 8d24c8160cb..3357be68494 100644 --- a/fields/types/password/Readme.md +++ b/fields/types/password/Readme.md @@ -4,7 +4,7 @@ Stores a `String` in the model. Displayed as a password field in the Admin UI, with a 'change' button. -Passwords are automatically encrypted with bcrypt, and expose a method to compare a string to the encrypted hash. +Passwords are automatically encrypted with `bcrypt`, and expose a method to compare a string to the encrypted hash. > Note: The encryption happens with a **pre-save hook** added to the **schema**, so passwords set will not be encrypted until an item has been saved to the database. @@ -18,35 +18,62 @@ Passwords are automatically encrypted with bcrypt, and expose a method to compar `workFactor` `Number` -The bcrypt workfactor to use when generating the hash, higher numbers are slower but more secure (defaults to `10`). +Supplied as the `bcrypt` cost parameter; controls the computational cost of generating and validating a hash. +Higher values are slower but, since they take longer to generate, more secure against brute force attacks. + +Defaults to `10`. +At this level, a modern laptop (Late 2016 MacBook Pro, 3.3 GHz Intel Core i7) can produces around ~4 hashes/second. + +The `bcrypt` algorithim applies this value as a power of two. +As such, passwords with a workfactor of `11` will take twice as long to store and validate as those with a workfactor of `10`. + +Values lower than `4` are ignored by the underlying implementation (a value `10` is substituted). + +`min` `Number` + +Defines the minimum allowed password length in characters. + +Defaults to `8` in accordance with the [NIST Digital Identity Guidelines](http://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-63b.pdf). + +`max` `Number` + +Defines the maximum allowed password length in characters. + +The `bcrypt` algorithm, used by this field, operates on a 72 byte value. +Most implementation (including [the one we use](https://www.npmjs.com/package/bcrypt-nodejs)), silently truncate the string provided if it exceeds this limit. +The `max` length option defaults to 72 characters in an attempt to align with this limit. + +> Note: If multi-byte (ie. non-ASCII) characters are allowed, it will be possible to exceed the 72 byte limit without triggering the 72 character validation limit. + +Can be set to `false` to disable the max length check. + +> Note: Disabling `max` or setting its value to >72 prevents validation errors but does not address the underlying algorithmic limitation. + +`rejectCommon` `Boolean` + +Controls whether values should be validated against a list of known-common passwords. + +Defaults to `true` in accordance with the [NIST Digital Identity Guidelines](http://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-63b.pdf). + +Implemented with the [`dumb-passwords` package](https://www.npmjs.com/package/dumb-passwords) +which validates against 10,000 common passwords complied by [security analyst Mark Burnett](https://xato.net/10-000-top-passwords-6d6380716fe0). `complexity` `Object` Allows to set complexity requirements: * `digitChar` `Boolean` - when set to `true`, requires at least one digit -* `spChar` `Boolean` - when set to `true`, requires at least one from the following special characters: !, @, #, $, %, ^, &, \*, (, ), + +* `spChar` `Boolean` - when set to `true`, requires at least one from the following special characters: `!`, `@`, `#`, `$`, `%`, `^`, `&`, `*`, `(`, `)`, `+` * `asciiChar` `Boolean` - when set to `true`, allows only ASCII characters (from range U+0020--U+007E) * `lowChar` `Boolean` - when set to `true`, requires at least one lower case character * `upperChar` `Boolean` - when set to `true`, requires at least one upper case character -### Example +Example: ```js { type: Types.Password, complexity: { digitChar: true, asciiChar: true } } ``` -`max` `Number` - -Sets the maximum password length; defaults to 72, in accordance with [bcrypt](https://www.google.com/search?q=bcrypt+max+length), which truncates the password to the first 72 bytes. - -Can be set to `false` to disable the max length. - -> Note: Disabling `max` or setting its value to >72 does not override the bcrypt specification. - -`min` `Number` - -Defines the minimum password length; disabled by default. ## Underscore methods diff --git a/package.json b/package.json index e1fc6b2e002..3861e949d22 100644 --- a/package.json +++ b/package.json @@ -36,6 +36,7 @@ "debug": "2.6.0", "display-name": "0.1.0", "ejs": "2.5.5", + "dumb-passwords": "^0.2.1", "elemental": "0.6.1", "embedly": "2.1.0", "errorhandler": "1.5.0",