Skip to content

Commit

Permalink
Replace memory spam prevention with brute-express (#7579)
Browse files Browse the repository at this point in the history
no issue

- removes count from user checks model
- uses brute express brute with brute-knex adaptor to store persisted data on spam prevention
- implement brute force protection for password/token exchange, password resets and private blogging
  • Loading branch information
cobbspur authored and kirrg001 committed Nov 8, 2016
1 parent b3f0934 commit 68af214
Show file tree
Hide file tree
Showing 20 changed files with 464 additions and 456 deletions.
12 changes: 8 additions & 4 deletions core/server/api/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ var debug = require('debug')('ghost:api'),
// API specific
auth = require('../auth'),
cors = require('../middleware/api/cors'), // routes only?!
spamPrevention = require('../middleware/api/spam-prevention'), // routes only
brute = require('../middleware/brute'), // routes only
versionMatch = require('../middleware/api/version-match'), // global

// Handling uploads & imports
Expand Down Expand Up @@ -170,17 +170,21 @@ function apiRoutes() {

// ## Authentication
apiRouter.post('/authentication/passwordreset',
spamPrevention.forgotten,
// Prevent more than 5 password resets from an ip in an hour for any email address
brute.globalReset,
// Prevent more than 5 password resets in an hour for an email+IP pair
brute.userReset,
api.http(api.authentication.generateResetToken)
);
apiRouter.put('/authentication/passwordreset', api.http(api.authentication.resetPassword));
apiRouter.put('/authentication/passwordreset', brute.globalBlock, api.http(api.authentication.resetPassword));
apiRouter.post('/authentication/invitation', api.http(api.authentication.acceptInvitation));
apiRouter.get('/authentication/invitation', api.http(api.authentication.isInvitation));
apiRouter.post('/authentication/setup', api.http(api.authentication.setup));
apiRouter.put('/authentication/setup', authenticatePrivate, api.http(api.authentication.updateSetup));
apiRouter.get('/authentication/setup', api.http(api.authentication.isSetup));
apiRouter.post('/authentication/token',
spamPrevention.signin,
brute.globalBlock,
brute.userLogin,
auth.authenticate.authenticateClient,
auth.oauth.generateAccessToken
);
Expand Down
3 changes: 3 additions & 0 deletions core/server/api/authentication.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ var _ = require('lodash'),
events = require('../events'),
config = require('../config'),
i18n = require('../i18n'),
spamPrevention = require('../middleware/api/spam-prevention'),
authentication,
tokenSecurity = {};

Expand Down Expand Up @@ -342,6 +343,8 @@ authentication = {
}));
}

spamPrevention.userLogin.reset(null, options.data.connection + tokenParts.email + 'login');

return models.User.changePassword({
oldPassword: oldPassword,
newPassword: newPassword,
Expand Down
46 changes: 0 additions & 46 deletions core/server/apps/private-blogging/lib/middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@ var _ = require('lodash'),
config = require('../../../config'),
api = require('../../../api'),
errors = require('../../../errors'),
logging = require('../../../logging'),
utils = require('../../../utils'),
i18n = require('../../../i18n'),
privateRoute = '/' + config.get('routeKeywords').private + '/',
protectedSecurity = [],
privateBlogging;

function verifySessionHash(salt, hash) {
Expand Down Expand Up @@ -136,50 +134,6 @@ privateBlogging = {
return next();
}
});
},

spamPrevention: function spamPrevention(req, res, next) {
var currentTime = process.hrtime()[0],
remoteAddress = req.connection.remoteAddress,
rateProtectedPeriod = config.rateProtectedPeriod || 3600,
rateProtectedAttempts = config.rateProtectedAttempts || 10,
ipCount = '',
message = i18n.t('errors.middleware.spamprevention.tooManyAttempts'),
deniedRateLimit = '',
password = req.body.password;

if (password) {
protectedSecurity.push({ip: remoteAddress, time: currentTime});
} else {
res.error = {
message: i18n.t('errors.middleware.spamprevention.noPassword')
};
return next();
}

// filter entries that are older than rateProtectedPeriod
protectedSecurity = _.filter(protectedSecurity, function filter(logTime) {
return (logTime.time + rateProtectedPeriod > currentTime);
});

ipCount = _.chain(protectedSecurity).countBy('ip').value();
deniedRateLimit = (ipCount[remoteAddress] > rateProtectedAttempts);

if (deniedRateLimit) {
logging.error(new errors.GhostError({
message: i18n.t('errors.middleware.spamprevention.forgottenPasswordIp.error', {rfa: rateProtectedAttempts, rfp: rateProtectedPeriod}),
context: i18n.t('errors.middleware.spamprevention.forgottenPasswordIp.context')
}));

message += rateProtectedPeriod === 3600 ? i18n.t('errors.middleware.spamprevention.waitOneHour') : i18n.t('errors.middleware.spamprevention.tryAgainLater');

// @TODO: why?
res.error = {
message: message
};
}

return next();
}
};

Expand Down
3 changes: 2 additions & 1 deletion core/server/apps/private-blogging/lib/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ var path = require('path'),
bodyParser = require('body-parser'),
templates = require('../../../controllers/frontend/templates'),
setResponseContext = require('../../../controllers/frontend/context'),
brute = require('../../../middleware/brute'),
privateRouter = express.Router();

function controller(req, res) {
Expand Down Expand Up @@ -32,7 +33,7 @@ privateRouter.route('/')
.post(
bodyParser.urlencoded({extended: true}),
middleware.isPrivateSessionAuth,
middleware.spamPrevention,
brute.privateBlog,
middleware.authenticateProtection,
controller
);
Expand Down
75 changes: 1 addition & 74 deletions core/server/apps/private-blogging/tests/middleware_spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*globals describe, beforeEach, afterEach, before, it*/
/*globals describe, beforeEach, afterEach, it*/
var crypto = require('crypto'),
should = require('should'),
sinon = require('sinon'),
Expand Down Expand Up @@ -288,77 +288,4 @@ describe('Private Blogging', function () {
});
});
});

describe('spamPrevention', function () {
var error = null,
res, req, spyNext;

before(function () {
spyNext = sinon.spy(function (param) {
error = param;
});
});

beforeEach(function () {
res = sinon.spy();
req = {
connection: {
remoteAddress: '10.0.0.0'
},
body: {
password: 'password'
}
};
});

it ('sets an error when there is no password', function (done) {
req.body = {};

privateBlogging.spamPrevention(req, res, spyNext);
res.error.message.should.equal('No password entered');
spyNext.calledOnce.should.be.true();

done();
});

it ('sets and error message after 10 tries', function (done) {
var ndx;

for (ndx = 0; ndx < 10; ndx = ndx + 1) {
privateBlogging.spamPrevention(req, res, spyNext);
}

should.not.exist(res.error);
privateBlogging.spamPrevention(req, res, spyNext);
should.exist(res.error);
should.exist(res.error.message);

done();
});

it ('allows more tries after an hour', function (done) {
var ndx,
stub = sinon.stub(process, 'hrtime', function () {
return [10, 10];
});

for (ndx = 0; ndx < 11; ndx = ndx + 1) {
privateBlogging.spamPrevention(req, res, spyNext);
}

should.exist(res.error);
process.hrtime.restore();
stub = sinon.stub(process, 'hrtime', function () {
return [3610000, 10];
});

res = sinon.spy();

privateBlogging.spamPrevention(req, res, spyNext);
should.not.exist(res.error);

process.hrtime.restore();
done();
});
});
});
7 changes: 4 additions & 3 deletions core/server/auth/oauth.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ function exchangeRefreshToken(client, refreshToken, scope, done) {
}
});
}

function exchangePassword(client, username, password, scope, done) {
// We are required to pass in authInfo in order to reset spam counter for user login
function exchangePassword(client, username, password, scope, body, authInfo, done) {
// Validate the client
models.Client.findOne({slug: client.slug})
.then(function then(client) {
Expand All @@ -54,7 +54,8 @@ function exchangePassword(client, username, password, scope, done) {
return authenticationAPI.createTokens({}, {context: {client_id: client.id, user: user.id}});
})
.then(function then(response) {
spamPrevention.resetCounter(username);
// Reset spam count for username and IP pair
spamPrevention.userLogin.reset(null, authInfo.ip + username + 'login');
return done(null, response.access_token, response.refresh_token, {expires_in: response.expires_in});
});
})
Expand Down
31 changes: 31 additions & 0 deletions core/server/config/defaults.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,36 @@
"enabled": false
},
"transports": ["stdout"]
},
"spam": {
"user_login": {
"minWait": 600000,
"maxWait": 604800000,
"freeRetries": 4
},
"user_reset": {
"minWait": 3600000,
"maxWait": 3600000,
"lifetime": 3600,
"freeRetries": 4
},
"global_reset": {
"minWait": 3600000,
"maxWait": 3600000,
"lifetime": 3600,
"freeRetries":4
},
"global_block": {
"minWait": 3600000,
"maxWait": 3600000,
"lifetime": 3600,
"freeRetries":99
},
"private_block": {
"minWait": 3600000,
"maxWait": 3600000,
"lifetime": 3600,
"freeRetries":99
}
}
}
31 changes: 31 additions & 0 deletions core/server/config/env/config.testing-mysql.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,36 @@
},
"logging": {
"level": "fatal"
},
"spam": {
"user_login": {
"minWait": 600000,
"maxWait": 604800000,
"freeRetries": 3
},
"user_reset": {
"minWait": 3600000,
"maxWait": 3600000,
"lifetime": 3600,
"freeRetries": 4
},
"global_reset": {
"minWait": 3600000,
"maxWait": 3600000,
"lifetime": 3600,
"freeRetries":4
},
"global_block": {
"minWait": 3600000,
"maxWait": 3600000,
"lifetime": 3600,
"freeRetries":4
},
"private_block": {
"minWait": 3600000,
"maxWait": 3600000,
"lifetime": 3600,
"freeRetries":99
}
}
}
31 changes: 31 additions & 0 deletions core/server/config/env/config.testing.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,36 @@
},
"logging": {
"level": "fatal"
},
"spam": {
"user_login": {
"minWait": 600000,
"maxWait": 604800000,
"freeRetries": 3
},
"user_reset": {
"minWait": 3600000,
"maxWait": 3600000,
"lifetime": 3600,
"freeRetries": 4
},
"global_reset": {
"minWait": 3600000,
"maxWait": 3600000,
"lifetime": 3600,
"freeRetries":4
},
"global_block": {
"minWait": 3600000,
"maxWait": 3600000,
"lifetime": 3600,
"freeRetries":4
},
"private_block": {
"minWait": 3600000,
"maxWait": 3600000,
"lifetime": 3600,
"freeRetries":99
}
}
}
7 changes: 7 additions & 0 deletions core/server/data/schema/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,5 +232,12 @@ module.exports = {
id: {type: 'increments', nullable: false, primary: true},
role_id: {type: 'integer', nullable: false},
invite_id: {type: 'integer', nullable: false}
},
brute: {
key: {type: 'string'},
firstRequest: {type: 'timestamp'},
lastRequest: {type: 'timestamp'},
lifetime: {type: 'bigInteger'},
count: {type: 'integer'}
}
};
Loading

0 comments on commit 68af214

Please sign in to comment.