🔒 Fixed rate limiting for user login (#15336)

refs https://github.com/TryGhost/Team/issues/1074

Rather than relying on the global block to stop malicious actors from
enumerating email addresses to determine who is and isn't a user, we
want our user login brute force protection to be on an IP basis,
rather than tied to the username.
This commit is contained in:
Fabien 'egg' O'Carroll 2022-08-31 10:33:42 -04:00 committed by GitHub
parent ae28cdffbb
commit 2ff81cc5d3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 101 additions and 15 deletions

View File

@ -29,26 +29,13 @@ module.exports = {
})(req, res, next);
},
/**
* block per user
* username === email!
* block per ip
*/
userLogin(req, res, next) {
return spamPrevention.userLogin().getMiddleware({
ignoreIP: false,
key(_req, _res, _next) {
if (_req.body.username) {
return _next(`${_req.body.username}login`);
}
if (_req.body.authorizationCode) {
return _next(`${_req.body.authorizationCode}login`);
}
if (_req.body.refresh_token) {
return _next(`${_req.body.refresh_token}login`);
}
return _next();
return _next('user_login');
}
})(req, res, next);
},

View File

@ -0,0 +1,25 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`Sessions API Is rate limited to protect against brute forcing a users password 1: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "286",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;
exports[`Sessions API Is rate limited to protect against brute forcing whether a user exists 1: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "286",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;

View File

@ -0,0 +1,74 @@
const {
agentProvider,
fixtureManager,
matchers: {
anyEtag
},
dbUtils,
configUtils
} = require('../../utils/e2e-framework');
describe('Sessions API', function () {
let agent;
before(async function () {
agent = await agentProvider.getAdminAPIAgent();
await fixtureManager.init();
});
it('Is rate limited to protect against brute forcing a users password', async function () {
await dbUtils.truncate('brute');
// +1 because this is a retry count, so we have one request + the retries, then blocked
const userLoginRateLimit = configUtils.config.get('spam').user_login.freeRetries + 1;
for (let i = 0; i < userLoginRateLimit; i++) {
await agent
.post('session/')
.body({
grant_type: 'password',
username: 'user@domain.tld',
password: 'parseword'
});
}
await agent
.post('session/')
.body({
grant_type: 'password',
username: 'user@domain.tld',
password: 'parseword'
})
.expectStatus(429)
.matchHeaderSnapshot({
etag: anyEtag
});
});
it('Is rate limited to protect against brute forcing whether a user exists', async function () {
await dbUtils.truncate('brute');
// +1 because this is a retry count, so we have one request + the retries, then blocked
const userLoginRateLimit = configUtils.config.get('spam').user_login.freeRetries + 1;
for (let i = 0; i < userLoginRateLimit; i++) {
await agent
.post('session/')
.body({
grant_type: 'password',
username: `user+${i}@domain.tld`,
password: `parseword`
});
}
await agent
.post('session/')
.body({
grant_type: 'password',
username: 'user@domain.tld',
password: 'parseword'
})
.expectStatus(429)
.matchHeaderSnapshot({
etag: anyEtag
});
});
});