From 2ac89f3365e971f09517108e268640b09bcd0a51 Mon Sep 17 00:00:00 2001 From: root Date: Thu, 27 Apr 2017 16:35:53 -0400 Subject: [PATCH] Report processor worker refactored to run under another user (nobody) and have its own mysql credentials. --- config/default.toml | 15 +- config/reports.toml | 7 + index.js | 2 +- lib/db.js | 2 +- lib/file-helpers.js | 7 +- lib/privilege-helpers.js | 133 +++++------------- package.json | 5 +- routes/report-templates.js | 2 +- services/executor.js | 85 +++++++---- setup/install-centos7.sh | 2 + workers/reports/config/default.toml | 18 +++ workers/reports/config/production.toml | 7 + .../reports}/report-processor.js | 78 +++------- 13 files changed, 159 insertions(+), 204 deletions(-) create mode 100644 config/reports.toml create mode 100644 workers/reports/config/default.toml create mode 100644 workers/reports/config/production.toml rename {services => workers/reports}/report-processor.js (61%) diff --git a/config/default.toml b/config/default.toml index 54fe11c0..abcc128e 100644 --- a/config/default.toml +++ b/config/default.toml @@ -43,8 +43,14 @@ language="en" # If you start out as a root user (eg. if you want to use ports lower than 1000) # then you can downgrade the user once all services are up and running -#user="nobody" -#group="nogroup" +#user="mailtrain" +#group="mailtrain" + +# If Mailtrain is started as root, "Reports" feature drops the privileges of script generating the report to disallow +# any modifications of Mailtrain code and even prohibits reading the production configuration (which contains the MySQL +# password for read/write operations). The rouser/rogroup determines the user to be used +#rouser="nobody" +#rogroup="nogroup" [log] # silly|verbose|info|http|warn|error|silent @@ -74,11 +80,6 @@ postsize="2MB" host="localhost" user="mailtrain" password="mailtrain" -# If more security is desired when running reports (which use user-defined JS scripts located in DB), -# one can specify a DB user with read-only permissions. If these are not specified, Mailtrain uses the -# regular DB user (which has also write permissions). -# userRO="mailtrain-ro" -# passwordRO="mailtrain-ro" database="mailtrain" # Some installations, eg. MAMP can use a different port (8889) # MAMP users should also turn on "Allow network access to MySQL" otherwise MySQL might not be accessible diff --git a/config/reports.toml b/config/reports.toml new file mode 100644 index 00000000..53d2315e --- /dev/null +++ b/config/reports.toml @@ -0,0 +1,7 @@ +[log] +level="verbose" + +[mysql] +user="mailtrain_ro" +password="S6Woc9hwWiV9RsWt" + diff --git a/index.js b/index.js index 6893ff3e..305ec2ad 100644 --- a/index.js +++ b/index.js @@ -141,7 +141,7 @@ server.on('listening', () => { } if (config.reports && config.reports.enabled === true) { - executor.spawn(() => startNextServices); + executor.spawn(startNextServices); } else { startNextServices(); } diff --git a/lib/db.js b/lib/db.js index 095608ae..2f221d44 100644 --- a/lib/db.js +++ b/lib/db.js @@ -6,7 +6,7 @@ let redis = require('redis'); let Lock = require('redfour'); module.exports = mysql.createPool(config.mysql); -if (config.redis.enabled) { +if (config.redis && config.redis.enabled) { module.exports.redis = redis.createClient(config.redis); diff --git a/lib/file-helpers.js b/lib/file-helpers.js index aca70e02..fdad5caf 100644 --- a/lib/file-helpers.js +++ b/lib/file-helpers.js @@ -12,22 +12,21 @@ function nameToFileName(name) { } -function getReportDir(report) { +function getReportFileBase(report) { return path.join(__dirname, '..', 'protected', 'reports', report.id + '-' + nameToFileName(report.name)); } function getReportContentFile(report) { - return path.join(getReportDir(report), 'report'); + return getReportFileBase(report) + '.out'; } function getReportOutputFile(report) { - return getReportDir(report) + '.output'; + return getReportFileBase(report) + '.err'; } module.exports = { getReportContentFile, - getReportDir, getReportOutputFile, nameToFileName }; diff --git a/lib/privilege-helpers.js b/lib/privilege-helpers.js index ce648d70..708c70bc 100644 --- a/lib/privilege-helpers.js +++ b/lib/privilege-helpers.js @@ -2,52 +2,52 @@ const log = require('npmlog'); const config = require('config'); -const path = require('path'); -const promise = require('bluebird'); -const fsExtra = promise.promisifyAll(require('fs-extra')); -const fs = promise.promisifyAll(require('fs')); -const walk = require('walk'); +const fs = require('fs'); const tryRequire = require('try-require'); const posix = tryRequire('posix'); +function _getConfigUidGid(prefix) { + let uid = process.getuid(); + let gid = process.getgid(); + + if (posix) { + try { + if (config.user) { + uid = posix.getpwnam(config[prefix + 'user']).uid; + } + } catch (err) { + log.info('PrivilegeHelpers', 'Failed to resolve user id "%s"', config[prefix + 'user']); + } + + try { + if (config.user) { + gid = posix.getpwnam(config[prefix + 'group']).gid; + } + } catch (err) { + log.info('PrivilegeHelpers', 'Failed to resolve group id "%s"', config[prefix + 'group']); + } + } else { + log.info('PrivilegeHelpers', 'Posix module not installed. Cannot resolve uid/gid'); + } + + return { uid, gid }; +} + +function getConfigUidGid() { + return _getConfigUidGid(''); +} + +function getConfigROUidGid() { + return _getConfigUidGid('ro'); +} function ensureMailtrainOwner(file, callback) { - try { - const uid = config.user ? posix.getpwnam(config.user).uid : 0; - const gid = config.group ? posix.getgrnam(config.group).gid : 0; - - fs.chown(file, uid, gid, callback); - - } catch (err) { - return callback(err); - } + const ids = getConfigUidGid(); + fs.chown(file, ids.uid, ids.gid, callback); } -function ensureMailtrainOwnerRecursive(dir, callback) { - try { - const uid = config.user ? posix.getpwnam(config.user).uid : 0; - const gid = config.group ? posix.getgrnam(config.group).gid : 0; - - fs.chown(dir, uid, gid, err => { - if (err) { - return callback(err); - } - - walk.walk(dir) - .on('node', (root, stat, next) => { - fs.chown(path.join(root, stat.name), uid, gid, next); - }) - .on('end', callback); - }); - } catch (err) { - return callback(err); - } -} - -const ensureMailtrainOwnerRecursiveAsync = promise.promisify(ensureMailtrainOwnerRecursive); - function dropRootPrivileges() { if (config.group) { try { @@ -68,64 +68,9 @@ function dropRootPrivileges() { } } -function setupChrootDir(newRoot, callback) { - try { - fsExtra.emptyDirAsync(newRoot) - .then(() => fsExtra.ensureDirAsync(path.join(newRoot, 'etc'))) - .then(() => fsExtra.copyAsync('/etc/hosts', path.join(newRoot, 'etc', 'hosts'))) - .then(() => ensureMailtrainOwnerRecursiveAsync(newRoot)) - .then(() => { - log.info('PrivilegeHelpers', 'Chroot directory "%s" set up', newRoot); - callback(); - }) - .catch(err => { - log.info('PrivilegeHelpers', 'Failed to setup chroot directory "%s"', newRoot); - callback(err); - }); - - } catch(err) { - log.info('PrivilegeHelpers', 'Failed to setup chroot directory "%s"', newRoot); - } -} - -function tearDownChrootDir(root, callback) { - if (posix) { - fsExtra.removeAsync(path.join('/', 'etc')) - .then(() => { - log.info('PrivilegeHelpers', 'Chroot directory "%s" torn down', root); - callback(); - }) - .catch(err => { - log.info('PrivilegeHelpers', 'Failed to tear down chroot directory "%s"', root); - callback(err); - }); - } -} - -function chrootAndDropRootPrivileges(newRoot) { - - try { - const uid = config.user ? posix.getpwnam(config.user).uid : 0; - const gid = config.group ? posix.getgrnam(config.group).gid : 0; - - posix.chroot(newRoot); - process.chdir('/'); - - process.setgid(gid); - process.setuid(uid); - - log.info('PrivilegeHelpers', 'Changed root to "%s" and privileges to %s.%s', newRoot, uid, gid); - } catch(err) { - log.info('PrivilegeHelpers', 'Failed to change root to "%s" and set privileges', newRoot); - } - -} - module.exports = { dropRootPrivileges, - chrootAndDropRootPrivileges, - setupChrootDir, - tearDownChrootDir, ensureMailtrainOwner, - ensureMailtrainOwnerRecursive + getConfigUidGid, + getConfigROUidGid }; diff --git a/package.json b/package.json index 80a97843..7d82f7cd 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,6 @@ "async": "^2.3.0", "aws-sdk": "^2.37.0", "bcrypt-nodejs": "0.0.3", - "bluebird": "^3.5.0", "body-parser": "^1.17.1", "bounce-handler": "^7.3.2-fork.2", "compression": "^1.6.2", @@ -60,7 +59,6 @@ "faker": "^4.1.0", "feedparser": "^2.1.0", "file-type": "^4.1.0", - "fs-extra": "^2.1.2", "geoip-ultralight": "^0.1.5", "gettext-parser": "^1.2.2", "gm": "^1.23.0", @@ -103,7 +101,6 @@ "smtp-server": "^2.0.3", "striptags": "^3.0.1", "toml": "^2.3.2", - "try-require": "^1.2.1", - "walk": "^2.3.9" + "try-require": "^1.2.1" } } diff --git a/routes/report-templates.js b/routes/report-templates.js index f411dd47..7fdc4fbc 100644 --- a/routes/report-templates.js +++ b/routes/report-templates.js @@ -174,7 +174,7 @@ router.get('/create', passport.csrfProtection, (req, res) => { ' {{#each results}}\n' + ' \n' + ' \n' + - ' {{custom_zone}}\n' + + ' {{custom_country}}\n' + ' \n' + ' \n' + ' {{count_opened}}\n' + diff --git a/services/executor.js b/services/executor.js index 8873a66b..fa001578 100644 --- a/services/executor.js +++ b/services/executor.js @@ -13,50 +13,71 @@ const privilegeHelpers = require('../lib/privilege-helpers'); let processes = {}; -function spawnProcess(tid, executable, args, outputFile, cwd) { +function spawnProcess(tid, executable, args, outFile, errFile, cwd, uid, gid) { - fs.open(outputFile, 'w', (err, outFd) => { + fs.open(outFile, 'w', (err, outFd) => { if (err) { log.error('Executor', err); return; } - privilegeHelpers.ensureMailtrainOwner(outputFile, (err) => { + fs.open(errFile, 'w', (err, errFd) => { if (err) { - log.info('Executor', 'Cannot change owner of output file of process tid:%s.', tid) + log.error('Executor', err); + return; } - const options = { - stdio: ['ignore', outFd, outFd, 'ipc'], - cwd: cwd, - env: {NODE_ENV: process.env.NODE_ENV} - }; + privilegeHelpers.ensureMailtrainOwner(outFile, (err) => { + if (err) { + log.info('Executor', 'Cannot change owner of output file of process tid:%s.', tid) + } - const child = fork(executable, args, options); - const pid = child.pid; - processes[tid] = child; - - log.info('Executor', 'Process started with tid:%s pid:%s.', tid, pid); - process.send({ - type: 'process-started', - tid - }); - - child.on('close', (code, signal) => { - - delete processes[tid]; - log.info('Executor', 'Process tid:%s pid:%s exited with code %s signal %s.', tid, pid, code, signal); - - fs.close(outFd, (err) => { + privilegeHelpers.ensureMailtrainOwner(errFile, (err) => { if (err) { - log.error('Executor', err); + log.info('Executor', 'Cannot change owner of error output file of process tid:%s.', tid) } + const options = { + stdio: ['ignore', outFd, errFd, 'ipc'], + cwd, + env: {NODE_ENV: process.env.NODE_ENV}, + uid, + gid + }; + + const child = fork(executable, args, options); + const pid = child.pid; + processes[tid] = child; + + log.info('Executor', 'Process started with tid:%s pid:%s.', tid, pid); process.send({ - type: 'process-finished', - tid, - code, - signal + type: 'process-started', + tid + }); + + child.on('close', (code, signal) => { + + delete processes[tid]; + log.info('Executor', 'Process tid:%s pid:%s exited with code %s signal %s.', tid, pid, code, signal); + + fs.close(outFd, (err) => { + if (err) { + log.error('Executor', err); + } + + fs.close(errFd, (err) => { + if (err) { + log.error('Executor', err); + } + + process.send({ + type: 'process-finished', + tid, + code, + signal + }); + }); + }); }); }); }); @@ -69,7 +90,9 @@ process.on('message', msg => { const type = msg.type; if (type === 'start-report-processor-worker') { - spawnProcess(msg.tid, path.join(__dirname, 'report-processor.js'), [msg.data.id], fileHelpers.getReportOutputFile(msg.data), path.join(__dirname, '..')); + + const ids = privilegeHelpers.getConfigROUidGid(); + spawnProcess(msg.tid, path.join(__dirname, '..', 'workers', 'reports', 'report-processor.js'), [msg.data.id], fileHelpers.getReportContentFile(msg.data), fileHelpers.getReportOutputFile(msg.data), path.join(__dirname, '..', 'workers', 'reports'), ids.uid, ids.gid); } else if (type === 'stop-process') { const child = processes[msg.tid]; diff --git a/setup/install-centos7.sh b/setup/install-centos7.sh index 524d0310..938f43db 100755 --- a/setup/install-centos7.sh +++ b/setup/install-centos7.sh @@ -36,6 +36,8 @@ SMTP_PASS=`pwgen 12 -1` # Setup MySQL user for Mailtrain mysql -u root -e "CREATE USER 'mailtrain'@'localhost' IDENTIFIED BY '$MYSQL_PASSWORD';" mysql -u root -e "GRANT ALL PRIVILEGES ON mailtrain.* TO 'mailtrain'@'localhost';" +mysql -u root -e "CREATE USER 'mailtrain_ro'@'localhost' IDENTIFIED BY '$MYSQL_PASSWORD';" +mysql -u root -e "GRANT SELECT ON mailtrain.* TO 'mailtrain_ro'@'localhost';" mysql -u mailtrain --password="$MYSQL_PASSWORD" -e "CREATE database mailtrain;" # Enable firewall, allow connections to SSH, HTTP, HTTPS and SMTP diff --git a/workers/reports/config/default.toml b/workers/reports/config/default.toml new file mode 100644 index 00000000..9e7fb70d --- /dev/null +++ b/workers/reports/config/default.toml @@ -0,0 +1,18 @@ +# Process title visible in monitoring logs and process listing +title="mailtrain" + +# Default language to use +language="en" + +[log] +# silly|verbose|info|http|warn|error|silent +level="verbose" + +[mysql] +host="localhost" +user="mailtrain" +password="mailtrain" +database="mailtrain" +port=3306 +charset="utf8mb4" +timezone="local" diff --git a/workers/reports/config/production.toml b/workers/reports/config/production.toml new file mode 100644 index 00000000..53d2315e --- /dev/null +++ b/workers/reports/config/production.toml @@ -0,0 +1,7 @@ +[log] +level="verbose" + +[mysql] +user="mailtrain_ro" +password="S6Woc9hwWiV9RsWt" + diff --git a/services/report-processor.js b/workers/reports/report-processor.js similarity index 61% rename from services/report-processor.js rename to workers/reports/report-processor.js index 8318008a..f1c95c8d 100644 --- a/services/report-processor.js +++ b/workers/reports/report-processor.js @@ -1,20 +1,17 @@ 'use strict'; -const reports = require('../lib/models/reports'); -const reportTemplates = require('../lib/models/report-templates'); -const lists = require('../lib/models/lists'); -const subscriptions = require('../lib/models/subscriptions'); -const campaigns = require('../lib/models/campaigns'); +const reports = require('../../lib/models/reports'); +const reportTemplates = require('../../lib/models/report-templates'); +const lists = require('../../lib/models/lists'); +const subscriptions = require('../../lib/models/subscriptions'); +const campaigns = require('../../lib/models/campaigns'); const handlebars = require('handlebars'); -const handlebarsHelpers = require('../lib/handlebars-helpers'); -const _ = require('../lib/translate')._; +const handlebarsHelpers = require('../../lib/handlebars-helpers'); +const _ = require('../../lib/translate')._; const hbs = require('hbs'); const vm = require('vm'); const log = require('npmlog'); const fs = require('fs'); -const fileHelpers = require('../lib/file-helpers'); -const path = require('path'); -const privilegeHelpers = require('../lib/privilege-helpers'); handlebarsHelpers.registerHelpers(handlebars); @@ -78,27 +75,12 @@ function resolveUserFields(userFields, params, callback) { setImmediate(doWork); } -function tearDownChrootDir(callback) { - if (reportDir) { - privilegeHelpers.tearDownChrootDir(reportDir, callback); - } else { - callback(); - } -} - function doneSuccess() { - tearDownChrootDir((err) => { - if (err) - process.exit(1) - else - process.exit(0); - }); + process.exit(0); } function doneFail() { - tearDownChrootDir((err) => { - process.exit(1) - }); + process.exit(1) } @@ -107,21 +89,18 @@ reports.get(reportId, (err, report) => { if (err || !report) { log.error('reports', err && err.message || err || _('Could not find report with specified ID')); doneFail(); - return; } reportTemplates.get(report.reportTemplate, (err, reportTemplate) => { if (err) { log.error('reports', err && err.message || err || _('Could not find report template')); doneFail(); - return; } resolveUserFields(reportTemplate.userFieldsObject, report.paramsObject, (err, inputs) => { if (err) { log.error('reports', err.message || err); doneFail(); - return; } const campaignsProxy = { @@ -134,8 +113,6 @@ reports.get(reportId, (err, report) => { list: subscriptions.list }; - const reportFile = fileHelpers.getReportContentFile(report); - const sandbox = { console, campaigns: campaignsProxy, @@ -146,45 +123,24 @@ reports.get(reportId, (err, report) => { if (err) { log.error('reports', err.message || err); doneFail(); - return; } const hbsTmpl = handlebars.compile(reportTemplate.hbs); const reportText = hbsTmpl(outputs); - fs.writeFile(path.basename(reportFile), reportText, (err, reportContent) => { - if (err) { - log.error('reports', err && err.message || err || _('Could not find report with specified ID')); - doneFail(); - return; - } - - doneSuccess(); - return; - }); + process.stdout.write(reportText); + doneSuccess(); } }; const script = new vm.Script(reportTemplate.js); - reportDir = fileHelpers.getReportDir(report); - privilegeHelpers.setupChrootDir(reportDir, (err) => { - if (err) { - doneFail(); - return; - } - - privilegeHelpers.chrootAndDropRootPrivileges(reportDir); - - try { - script.runInNewContext(sandbox, {displayErrors: true, timeout: 120000}); - } catch (err) { - console.log(err); - - doneFail(); - return; - } - }); + try { + script.runInNewContext(sandbox, {displayErrors: true, timeout: 120000}); + } catch (err) { + console.error(err); + doneFail(); + } }); }); });