diff --git a/resources/buildConfigDefinitions.js b/resources/buildConfigDefinitions.js index 81492a75d4..c415469c64 100644 --- a/resources/buildConfigDefinitions.js +++ b/resources/buildConfigDefinitions.js @@ -50,6 +50,7 @@ function getENVPrefix(iface) { 'IdempotencyOptions' : 'PARSE_SERVER_EXPERIMENTAL_IDEMPOTENCY_', 'AccountLockoutOptions' : 'PARSE_SERVER_ACCOUNT_LOCKOUT_', 'PasswordPolicyOptions' : 'PARSE_SERVER_PASSWORD_POLICY_', + 'SecurityChecksOptions' : 'PARSE_SERVER_SECURITY_CHECKS_', 'FileUploadOptions' : 'PARSE_SERVER_FILE_UPLOAD_', } if (options[iface.id.name]) { @@ -166,7 +167,7 @@ function parseDefaultValue(elt, value, t) { if (type == 'NumberOrBoolean') { literalValue = t.numericLiteral(parsers.numberOrBoolParser('')(value)); } - const literalTypes = ['Object', 'IdempotencyOptions','FileUploadOptions','CustomPagesOptions', 'PagesCustomUrlsOptions', 'PagesOptions']; + const literalTypes = ['Object', 'IdempotencyOptions','FileUploadOptions','CustomPagesOptions', 'PagesCustomUrlsOptions', 'PagesOptions','SecurityChecksOptions']; if (literalTypes.includes(type)) { const object = parsers.objectParser(value); const props = Object.keys(object).map((key) => { diff --git a/spec/ParseServer.Security.spec.js b/spec/ParseServer.Security.spec.js new file mode 100644 index 0000000000..59754e0d31 --- /dev/null +++ b/spec/ParseServer.Security.spec.js @@ -0,0 +1,76 @@ +'use strict'; +const Parse = require('parse/node'); +const request = require('../lib/request'); + +const defaultHeaders = { + 'X-Parse-Application-Id': 'test', + 'X-Parse-Rest-API-Key': 'rest', + 'Content-Type': 'application/json', +}; +const masterKeyHeaders = { + 'X-Parse-Application-Id': 'test', + 'X-Parse-Rest-API-Key': 'rest', + 'X-Parse-Master-Key': 'test', + 'Content-Type': 'application/json', +}; +const defaultOptions = { + headers: defaultHeaders, + json: true, +}; +const masterKeyOptions = { + headers: masterKeyHeaders, + json: true, +}; + +describe('SecurityChecks', () => { + it('should reject access when not using masterKey (/securityChecks)', done => { + request( + Object.assign({ url: Parse.serverURL + '/securityChecks' }, defaultOptions) + ).then(done.fail, () => done()); + }); + it('should reject access by default to /securityChecks, even with masterKey', done => { + request( + Object.assign({ url: Parse.serverURL + '/securityChecks' }, masterKeyOptions) + ).then(done.fail, () => done()); + }); + it('can get security advice', async done => { + await reconfigureServer({ + securityChecks: { + enableSecurityChecks: true, + enableLogOutput: true, + }, + }); + const options = Object.assign({}, masterKeyOptions, { + method: 'GET', + url: Parse.serverURL + '/securityChecks', + }); + request(options).then(res => { + expect(res.data.CLP).not.toBeUndefined(); + expect(res.data.ServerConfiguration).not.toBeUndefined(); + expect(res.data.Database).not.toBeUndefined(); + expect(res.data.Total).not.toBeUndefined(); + done(); + }); + }); + + it('can get security on start', async done => { + await reconfigureServer({ + securityChecks: { + enableSecurityChecks: true, + enableLogOutput: true, + }, + }); + const logger = require('../lib/logger').logger; + spyOn(logger, 'warn').and.callFake(() => {}); + await new Promise(resolve => { + setTimeout(resolve, 2000); + }); + let messagesCalled = ''; + for (const item in logger.warn.calls.all()) { + const call = logger.warn.calls.all()[item]; + messagesCalled = messagesCalled + ' ' + (call.args || []).join(' '); + } + expect(messagesCalled).toContain('Clients are currently allowed to create new classes.'); + done(); + }); +}); diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index cac2f6614c..9889545f62 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -1057,5 +1057,4 @@ export class MongoStorageAdapter implements StorageAdapter { }); } } - export default MongoStorageAdapter; diff --git a/src/Options/Definitions.js b/src/Options/Definitions.js index c67017a585..857df87b9b 100644 --- a/src/Options/Definitions.js +++ b/src/Options/Definitions.js @@ -373,6 +373,11 @@ module.exports.ParseServerOptions = { action: parsers.numberParser('schemaCacheTTL'), default: 5000, }, + securityChecks: { + env: 'PARSE_SERVER_SECURITY_CHECKS_OPTIONS', + help: 'View recommendations for server improvements', + action: parsers.objectParser, + }, serverCloseComplete: { env: 'PARSE_SERVER_SERVER_CLOSE_COMPLETE', help: 'Callback when server has closed', @@ -664,6 +669,21 @@ module.exports.IdempotencyOptions = { default: 300, }, }; +module.exports.SecurityChecksOptions = { + enableLogOutput: { + env: 'PARSE_SERVER_SECURITY_CHECKS_ENABLE_LOG_OUTPUT', + help: + 'Is true if security warnings should be written to logs. This should only be enabled temporarily to not expose weak security settings in logs.', + action: parsers.booleanParser, + default: false, + }, + enableSecurityChecks: { + env: 'PARSE_SERVER_SECURITY_CHECKS_ENABLE_SECURITY_CHECKS', + help: 'If true if Parse Server should self-check the security of its current configuration.', + action: parsers.booleanParser, + default: false, + }, +}; module.exports.AccountLockoutOptions = { duration: { env: 'PARSE_SERVER_ACCOUNT_LOCKOUT_DURATION', diff --git a/src/Options/docs.js b/src/Options/docs.js index da90760389..61569d2fa6 100644 --- a/src/Options/docs.js +++ b/src/Options/docs.js @@ -68,6 +68,7 @@ * @property {Boolean} revokeSessionOnPasswordReset When a user changes their password, either through the reset password email or while logged in, all sessions are revoked if this is true. Set to false if you don't want to revoke sessions. * @property {Boolean} scheduledPush Configuration for push scheduling, defaults to false. * @property {Number} schemaCacheTTL The TTL for caching the schema for optimizing read/write operations. You should put a long TTL when your DB is in production. default to 5000; set 0 to disable. + * @property {SecurityChecksOptions} securityChecks View recommendations for server improvements * @property {Function} serverCloseComplete Callback when server has closed * @property {Function} serverStartComplete Callback when server has started * @property {String} serverURL URL to your parse server with http:// or https://. @@ -150,6 +151,12 @@ * @property {Number} ttl The duration in seconds after which a request record is discarded from the database, defaults to 300s. */ +/** + * @interface SecurityChecksOptions + * @property {Boolean} enableLogOutput Is true if security warnings should be written to logs. This should only be enabled temporarily to not expose weak security settings in logs. + * @property {Boolean} enableSecurityChecks If true if Parse Server should self-check the security of its current configuration. + */ + /** * @interface AccountLockoutOptions * @property {Number} duration number of minutes that a locked-out account remains locked out before automatically becoming unlocked. diff --git a/src/Options/index.js b/src/Options/index.js index e333b53694..263f489132 100644 --- a/src/Options/index.js +++ b/src/Options/index.js @@ -201,6 +201,9 @@ export interface ParseServerOptions { :ENV: PARSE_SERVER_EXPERIMENTAL_IDEMPOTENCY_OPTIONS :DEFAULT: false */ idempotencyOptions: ?IdempotencyOptions; + /* View recommendations for server improvements + :ENV: PARSE_SERVER_SECURITY_CHECKS_OPTIONS */ + securityChecks: ?SecurityChecksOptions; /* Options for file uploads :ENV: PARSE_SERVER_FILE_UPLOAD_OPTIONS :DEFAULT: {} */ @@ -351,6 +354,15 @@ export interface IdempotencyOptions { ttl: ?number; } +export interface SecurityChecksOptions { + /* If true if Parse Server should self-check the security of its current configuration. + :DEFAULT: false */ + enableSecurityChecks: ?boolean; + /* Is true if security warnings should be written to logs. This should only be enabled temporarily to not expose weak security settings in logs. + :DEFAULT: false */ + enableLogOutput: ?boolean; +} + export interface AccountLockoutOptions { /* number of minutes that a locked-out account remains locked out before automatically becoming unlocked. */ duration: ?number; diff --git a/src/ParseServer.js b/src/ParseServer.js index a81e97fc60..811d4fe777 100644 --- a/src/ParseServer.js +++ b/src/ParseServer.js @@ -41,6 +41,8 @@ import { AggregateRouter } from './Routers/AggregateRouter'; import { ParseServerRESTController } from './ParseServerRESTController'; import * as controllers from './Controllers'; import { ParseGraphQLServer } from './GraphQL/ParseGraphQLServer'; +import SecurityCheck from './SecurityCheck'; +import { registerServerSecurityChecks } from './SecurityChecks'; // Mutate the Parse object to add the Cloud Code handlers addParseCloud(); @@ -79,7 +81,19 @@ class ParseServer { Promise.all([dbInitPromise, hooksLoadPromise]) .then(() => { if (serverStartComplete) { - serverStartComplete(); + return serverStartComplete(); + } + return Promise.resolve(); + }) + .then(() => { + if ((options.securityChecks || {}).enableLogOutput) { + return registerServerSecurityChecks(this.config); + } + return Promise.resolve(); + }) + .then(() => { + if ((options.securityChecks || {}).enableLogOutput) { + this.getSecurityChecks(); } }) .catch(error => { @@ -110,6 +124,28 @@ class ParseServer { return this._app; } + async getSecurityChecks() { + const checks = await SecurityCheck.getChecks(); + const logger = logging.getLogger(); + const { Total } = checks; + delete checks.Total; + for (const category in checks) { + const data = checks[category]; + for (const check of data) { + const { title, warning, error, result, success } = check; + if (result === 'success') { + logger.warn(`āœ… ${success}\n`); + } else { + const appendString = error && error !== 'Check failed.' ? ` with error: ${error}` : ''; + logger.warn(`āŒ ${warning}\nā— Check "${title}" failed${appendString}\n`); + } + } + } + if (Total !== 0) { + logger.warn(`ā— ${Total} security warning(s) for Parse Server`); + } + } + handleShutdown() { const promises = []; const { adapter: databaseAdapter } = this.config.databaseController; diff --git a/src/Routers/CloudCodeRouter.js b/src/Routers/CloudCodeRouter.js index 327353123a..d6bd2526aa 100644 --- a/src/Routers/CloudCodeRouter.js +++ b/src/Routers/CloudCodeRouter.js @@ -1,6 +1,7 @@ import PromiseRouter from '../PromiseRouter'; import Parse from 'parse/node'; import rest from '../rest'; +import SecurityCheck from '../SecurityCheck'; const triggers = require('../triggers'); const middleware = require('../middlewares'); @@ -53,6 +54,12 @@ export class CloudCodeRouter extends PromiseRouter { middleware.promiseEnforceMasterKeyAccess, CloudCodeRouter.deleteJob ); + this.route( + 'GET', + '/securityChecks', + middleware.promiseEnforceMasterKeyAccess, + CloudCodeRouter.getSecurityChecks + ); } static getJobs(req) { @@ -120,4 +127,14 @@ export class CloudCodeRouter extends PromiseRouter { }; }); } + + static async getSecurityChecks(req) { + if (!req.config.securityChecks || !req.config.securityChecks.enableSecurityChecks) { + throw new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'Unauthorized'); + } + const response = await SecurityCheck.getChecks(); + return { + response, + }; + } } diff --git a/src/SecurityCheck.js b/src/SecurityCheck.js new file mode 100644 index 0000000000..5a4acbb4df --- /dev/null +++ b/src/SecurityCheck.js @@ -0,0 +1,102 @@ +import logger from './logger'; +class SecurityCheck { + constructor(data) { + const { group, title, warning, check, failed, success } = data; + try { + if (!group || !title || !warning) { + throw 'Security checks must have a group, title, and a warning.'; + } + if (typeof group !== 'string') { + throw '"group" of the security check must be a string, e.g SecurityCheck.Category.Database'; + } + if (typeof success !== 'string') { + throw '"success" message of the security check must be a string.'; + } + if (typeof title !== 'string') { + throw '"title" of the security check must be a string.'; + } + if (typeof warning !== 'string') { + throw '"warning" message of the security check must be a string.'; + } + if (check && typeof check !== 'function') { + throw '"check" of the security check must be a function.'; + } + this.group = group; + this.title = title; + this.warning = warning; + this.check = check; + this.failed = failed; + this.success = success; + } catch (e) { + logger.error(e); + return; + } + _registerCheck(this); + } + async run() { + try { + if (this.failed) { + throw 'Check failed.'; + } + if (!this.check) { + return { + result: 'success', + }; + } + const result = await this.check(); + if (result != null && result === false) { + throw 'Check failed.'; + } + return { + result: 'success', + }; + } catch (error) { + return { + result: 'fail', + error, + }; + } + } + setFailed() { + this.failed = true; + } +} +SecurityCheck.Category = { + Database: 'Database', + CLP: 'CLP', + ServerConfiguration: 'ServerConfiguration', +}; +SecurityCheck.getChecks = async () => { + const resultsByGroup = {}; + let total = 0; + const resolveSecurityCheck = async check => { + const { group, title, warning, success } = check; + const { result, error } = await check.run(); + const category = resultsByGroup[group] || []; + category.push({ + title, + warning, + error, + result, + success, + }); + resultsByGroup[group] = category; + if (result !== 'success') { + total++; + } + }; + await Promise.all(securityCheckStore.map(check => resolveSecurityCheck(check))); + resultsByGroup.Total = total; + return resultsByGroup; +}; +const securityCheckStore = []; +function _registerCheck(securityCheck) { + for (const [i, check] of securityCheckStore.entries()) { + if (check.title == securityCheck.title && check.warning == securityCheck.warning) { + securityCheckStore[i] = securityCheck; + return; + } + } + securityCheckStore.push(securityCheck); +} +module.exports = SecurityCheck; diff --git a/src/SecurityChecks.js b/src/SecurityChecks.js new file mode 100644 index 0000000000..0e1f896613 --- /dev/null +++ b/src/SecurityChecks.js @@ -0,0 +1,171 @@ +import SecurityCheck from './SecurityCheck'; +import url from 'url'; +const ServerChecks = {}; +const mongodb = require('mongodb'); +const MongoClient = mongodb.MongoClient; + +ServerChecks.registerCLP = async options => { + const schema = await options.databaseController.loadSchema(); + const all = await schema.getAllClasses(); + for (const field of all) { + const { className, clp } = field; + const clpCheck = new SecurityCheck({ + group: SecurityCheck.Category.CLP, + title: `No Class Level Permissions on ${className}`, + warning: `Any client can create, find, count, get, update, delete, or add field on ${className}. This allows an attacker to create new objects or fieldNames without restriction and potentially flood the database. Set CLPs using Parse Dashboard.`, + success: `Class Level Permissions on ${className}`, + }); + if (!clp) { + clpCheck.setFailed(); + continue; + } + const keys = ['find', 'count', 'get', 'create', 'update', 'delete', 'addField']; + for (const key of keys) { + const option = clp[key]; + if (className === '_User' && key === 'create') { + continue; + } + const optionCheck = new SecurityCheck({ + group: SecurityCheck.Category.CLP, + title: `Unrestricted access to ${key}.`, + warning: `Any client can ${key} on ${className}.`, + success: `${key} is restricted on ${className}`, + }); + const addFileCheck = new SecurityCheck({ + group: SecurityCheck.Category.CLP, + title: `Certain users can add fields.`, + warning: `Certain users can add fields on ${className}. This allows these users to create new fieldNames and potentially flood the schema. Set CLPs using Parse Dashboard.`, + success: `AddField is restricted on ${className}`, + }); + if (!option || option['*']) { + optionCheck.setFailed(); + } else if (Object.keys(option).length != 0 && key === 'addField') { + addFileCheck.setFailed(); + } + } + } +}; +ServerChecks.checkServerConfig = async options => { + new SecurityCheck({ + group: SecurityCheck.Category.ServerConfiguration, + title: 'Client class creation allowed', + warning: + 'Clients are currently allowed to create new classes. This allows an attacker to create new classes without restriction and potentially flood the database. Change the Parse Server configuration to allowClientClassCreation: false.', + success: `Client class creation is turned off`, + check() { + return !options.allowClientClassCreation; + }, + }); + new SecurityCheck({ + group: SecurityCheck.Category.ServerConfiguration, + title: 'Weak masterKey.', + warning: + 'The masterKey set to your configuration lacks complexity and length. This could potentially allow an attacker to brute force the masterKey, exposing all the entire Parse Server.', + success: 'The masterKey is complex and long', + check() { + return ( + options.masterKey.match('^(?=.*[a-z])(?=.*[A-Z])(?=.*[0-9])(?=.*[!@#$%^&*])(?=.{14,})') || + false + ); + }, + }); + let https = false; + try { + const serverURL = url.parse(options.publicServerUrl || options.serverURL); + https = serverURL.protocol === 'https:'; + } catch (e) { + /* */ + } + new SecurityCheck({ + group: SecurityCheck.Category.ServerConfiguration, + title: `Parse Server served over HTTP`, + warning: + 'The server url is currently HTTP. This allows an attacker to listen to all traffic in-between the server and the client. Change the Parse Server configuration serverURL to HTTPS.', + success: 'The server url uses HTTPS', + check() { + return https; + }, + }); +}; +ServerChecks.checkFiles = options => { + new SecurityCheck({ + group: SecurityCheck.Category.ServerConfiguration, + title: `Public File Upload Enabled`, + warning: + 'Public file upload is currently enabled. This allows a client to upload files without requiring login or authentication. Remove enableForPublic from fileUpload in the Parse Server configuration.', + success: 'Public File Upload is disabled', + check() { + return !(options.fileUpload && options.fileUpload.enableForPublic); + }, + }); +}; +ServerChecks.checkDatabase = options => { + let databaseURI = options.databaseURI; + if (options.databaseAdapter && options.databaseAdapter._uri) { + databaseURI = options.databaseAdapter._uri; + } + const databaseCheck = new SecurityCheck({ + group: SecurityCheck.Category.Database, + title: `Weak Database Password`, + warning: + 'The database password set lacks complexity and length. This could potentially allow an attacker to brute force their way into the database, exposing the database.', + success: `Strong Database Password`, + }); + if (databaseURI.includes('@')) { + const copyURI = `${databaseURI}`; + databaseURI = `mongodb://${databaseURI.split('@')[1]}`; + const pwd = copyURI.split('//')[1].split('@')[0].split(':')[1] || ''; + if (!pwd.match('^(?=.*[a-z])(?=.*[A-Z])(?=.*[0-9])(?=.*[!@#$%^&*])(?=.{14,})')) { + databaseCheck.setFailed(); + } + } else { + databaseCheck.setFailed(); + } + let databaseAdmin = '' + databaseURI; + try { + const parsedURI = url.parse(databaseAdmin); + parsedURI.port = '27017'; + databaseAdmin = url.format(parsedURI); + } catch (e) { + /* */ + } + new SecurityCheck({ + group: SecurityCheck.Category.Database, + title: `Unrestricted access to port 27017`, + warning: + 'The database requires no authentication to the admin port. This could potentially allow an attacker to easily access the database, exposing all of the database.', + success: `Restricted port 27017`, + check: async () => { + try { + await MongoClient.connect(databaseAdmin.toString(), { useNewUrlParser: true }); + return false; + } catch (e) { + console.log(e); + } + }, + }); + new SecurityCheck({ + group: SecurityCheck.Category.Database, + title: `Unrestricted access to the database`, + warning: + 'The database requires no authentication to connect. This could potentially allow an attacker to easily access the database, exposing all of the database.', + success: `Restricted access to the database`, + check: async () => { + try { + await MongoClient.connect(databaseURI, { useNewUrlParser: true }); + return false; + } catch (e) { + console.log(e); + } + }, + }); +}; + +const registerServerSecurityChecks = async req => { + const options = req.config || req; + const serverFuncs = Object.values(ServerChecks); + await Promise.all(serverFuncs.map(func => func(options))); +}; +module.exports = { + registerServerSecurityChecks, +};