From a82cabb00134c79c5ee77afd1b1628a1f768e050 Mon Sep 17 00:00:00 2001 From: Sabir Hassan Date: Tue, 28 Mar 2023 21:43:10 +0500 Subject: [PATCH] feat: prevent brute force attack by rate limiting login endpoint --- README.md | 12 ++++ api/.env.example | 3 + api/package-lock.json | 11 +++ api/package.json | 1 + api/src/controllers/web.ts | 105 +++++++++++++++++++++++++--- api/src/routes/api/spec/web.spec.ts | 69 +++++++++++++++++- api/src/routes/web/web.ts | 6 +- api/src/types/system/process.d.ts | 1 + api/src/utils/connectDB.ts | 4 +- api/src/utils/getRateLimiters.ts | 26 +++++++ api/src/utils/index.ts | 4 +- api/src/utils/secondsToHms.ts | 10 +++ api/src/utils/verifyEnvVariables.ts | 50 ++++++++++++- 13 files changed, 286 insertions(+), 16 deletions(-) create mode 100644 api/src/utils/getRateLimiters.ts create mode 100644 api/src/utils/secondsToHms.ts diff --git a/README.md b/README.md index 1bdf184..1b3c460 100644 --- a/README.md +++ b/README.md @@ -175,6 +175,18 @@ HELMET_COEP= # } HELMET_CSP_CONFIG_PATH=./csp.config.json +# To prevent brute force attack on login route we have implemented rate limiter +# Only valid for MODE: server +# Following are configurable env variable rate limiter + +MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY = default: 100; +# After this, access is blocked for 1 day + +MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP = default: 10; +# After this, access is blocked for an hour +# Store number for 90 days since first fail +# Once a successful login is attempted, it resets + # LOG_FORMAT_MORGAN options: [combined|common|dev|short|tiny] default: `common` # Docs: https://www.npmjs.com/package/morgan#predefined-formats LOG_FORMAT_MORGAN= diff --git a/api/.env.example b/api/.env.example index 58212f0..c61dec2 100644 --- a/api/.env.example +++ b/api/.env.example @@ -24,6 +24,9 @@ LDAP_BIND_PASSWORD = LDAP_USERS_BASE_DN = LDAP_GROUPS_BASE_DN = +MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY=[100] default value is 100 +MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP=[10] default value is 10 + RUN_TIMES=[sas,js,py | js,py | sas | sas,js] default considered as sas SAS_PATH=/opt/sas/sas9/SASHome/SASFoundation/9.4/sas NODE_PATH=~/.nvm/versions/node/v16.14.0/bin/node diff --git a/api/package-lock.json b/api/package-lock.json index 358a80e..f0ded1d 100644 --- a/api/package-lock.json +++ b/api/package-lock.json @@ -24,6 +24,7 @@ "mongoose-sequence": "^5.3.1", "morgan": "^1.10.0", "multer": "^1.4.5-lts.1", + "rate-limiter-flexible": "2.4.1", "rotating-file-stream": "^3.0.4", "swagger-ui-express": "4.3.0", "unzipper": "^0.10.11", @@ -9594,6 +9595,11 @@ "node": ">= 0.6" } }, + "node_modules/rate-limiter-flexible": { + "version": "2.4.1", + "resolved": "https://registry.npmjs.org/rate-limiter-flexible/-/rate-limiter-flexible-2.4.1.tgz", + "integrity": "sha512-dgH4T44TzKVO9CLArNto62hJOwlWJMLUjVVr/ii0uUzZXEXthDNr7/yefW5z/1vvHAfycc1tnuiYyNJ8CTRB3g==" + }, "node_modules/raw-body": { "version": "2.5.1", "resolved": "https://registry.npmjs.org/raw-body/-/raw-body-2.5.1.tgz", @@ -18811,6 +18817,11 @@ "resolved": "https://registry.npmjs.org/range-parser/-/range-parser-1.2.1.tgz", "integrity": "sha512-Hrgsx+orqoygnmhFbKaHE6c296J+HTAQXoxEF6gNupROmmGJRoyzfG3ccAveqCBrwr/2yxQ5BVd/GTl5agOwSg==" }, + "rate-limiter-flexible": { + "version": "2.4.1", + "resolved": "https://registry.npmjs.org/rate-limiter-flexible/-/rate-limiter-flexible-2.4.1.tgz", + "integrity": "sha512-dgH4T44TzKVO9CLArNto62hJOwlWJMLUjVVr/ii0uUzZXEXthDNr7/yefW5z/1vvHAfycc1tnuiYyNJ8CTRB3g==" + }, "raw-body": { "version": "2.5.1", "resolved": "https://registry.npmjs.org/raw-body/-/raw-body-2.5.1.tgz", diff --git a/api/package.json b/api/package.json index 31782ab..51a79eb 100644 --- a/api/package.json +++ b/api/package.json @@ -64,6 +64,7 @@ "mongoose-sequence": "^5.3.1", "morgan": "^1.10.0", "multer": "^1.4.5-lts.1", + "rate-limiter-flexible": "2.4.1", "rotating-file-stream": "^3.0.4", "swagger-ui-express": "4.3.0", "unzipper": "^0.10.11", diff --git a/api/src/controllers/web.ts b/api/src/controllers/web.ts index 9ed18ca..6deda8d 100644 --- a/api/src/controllers/web.ts +++ b/api/src/controllers/web.ts @@ -8,8 +8,10 @@ import Client from '../model/Client' import { getWebBuildFolder, generateAuthCode, + getRateLimiters, AuthProviderType, - LDAPClient + LDAPClient, + secondsToHms } from '../utils' import { InfoJWT } from '../types' import { AuthController } from './auth' @@ -81,19 +83,98 @@ const login = async ( req: express.Request, { username, password }: LoginPayload ) => { + // code for preventing brute force attack + + const { + MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY, + MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP + } = process.env + + const { limiterSlowBruteByIP, limiterConsecutiveFailsByUsernameAndIP } = + getRateLimiters() + + const ipAddr = req.ip + const usernameIPkey = getUsernameIPkey(username, ipAddr) + + const [resSlowByIP, resUsernameAndIP] = await Promise.all([ + limiterSlowBruteByIP.get(ipAddr), + limiterConsecutiveFailsByUsernameAndIP.get(usernameIPkey) + ]) + + let retrySecs = 0 + + // Check if IP or Username + IP is already blocked + if ( + resSlowByIP !== null && + resSlowByIP.consumedPoints >= Number(MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY) + ) { + retrySecs = Math.round(resSlowByIP.msBeforeNext / 1000) || 1 + } else if ( + resUsernameAndIP !== null && + resUsernameAndIP.consumedPoints >= + Number(MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP) + ) { + retrySecs = Math.round(resUsernameAndIP.msBeforeNext / 1000) || 1 + } + + if (retrySecs > 0) { + throw { + code: 429, + message: `Too Many Requests! Retry after ${secondsToHms(retrySecs)}` + } + } + // Authenticate User const user = await User.findOne({ username }) - if (!user) throw new Error('Username is not found.') - if ( - process.env.AUTH_PROVIDERS === AuthProviderType.LDAP && - user.authProvider === AuthProviderType.LDAP - ) { - const ldapClient = await LDAPClient.init() - await ldapClient.verifyUser(username, password) - } else { - const validPass = user.comparePassword(password) - if (!validPass) throw new Error('Invalid password.') + let validPass = false + + if (user) { + if ( + process.env.AUTH_PROVIDERS === AuthProviderType.LDAP && + user.authProvider === AuthProviderType.LDAP + ) { + const ldapClient = await LDAPClient.init() + validPass = await ldapClient + .verifyUser(username, password) + .catch(() => false) + } else { + validPass = user.comparePassword(password) + } + } + + // Consume 1 point from limiters on wrong attempt and block if limits reached + if (!validPass) { + try { + const promises = [limiterSlowBruteByIP.consume(ipAddr)] + if (user) { + // Count failed attempts by Username + IP only for registered users + promises.push( + limiterConsecutiveFailsByUsernameAndIP.consume(usernameIPkey) + ) + } + + await Promise.all(promises) + } catch (rlRejected: any) { + if (rlRejected instanceof Error) { + throw rlRejected + } else { + retrySecs = Math.round(rlRejected.msBeforeNext / 1000) || 1 + + throw { + code: 429, + message: `Too Many Requests! Retry after ${secondsToHms(retrySecs)}` + } + } + } + } + + if (!user) throw { code: 401, message: 'Username is not found.' } + if (!validPass) throw { code: 401, message: 'Invalid Password.' } + + if (resUsernameAndIP !== null && resUsernameAndIP.consumedPoints > 0) { + // Reset on successful authorization + await limiterConsecutiveFailsByUsernameAndIP.delete(usernameIPkey) } req.session.loggedIn = true @@ -144,6 +225,8 @@ const authorize = async ( return { code } } +const getUsernameIPkey = (username: string, ip: string) => `${username}_${ip}` + interface LoginPayload { /** * Username for user diff --git a/api/src/routes/api/spec/web.spec.ts b/api/src/routes/api/spec/web.spec.ts index c5ce0ca..ba5babe 100644 --- a/api/src/routes/api/spec/web.spec.ts +++ b/api/src/routes/api/spec/web.spec.ts @@ -47,7 +47,7 @@ describe('web', () => { }) }) - describe('SASLogon/login', () => { + describe.only('SASLogon/login', () => { let csrfToken: string beforeAll(async () => { @@ -63,6 +63,7 @@ describe('web', () => { it('should respond with successful login', async () => { await userController.createUser(user) + process.dbInstance = con const res = await request(app) .post('/SASLogon/login') .set('x-xsrf-token', csrfToken) @@ -82,6 +83,72 @@ describe('web', () => { }) }) + it('should respond with too many requests when attempting with invalid password for a same user 10 times', async () => { + await userController.createUser(user) + + process.dbInstance = con + + const promises: request.Test[] = [] + + Array(10) + .fill(0) + .map((_, i) => { + promises.push( + request(app) + .post('/SASLogon/login') + .set('x-xsrf-token', csrfToken) + .send({ + username: user.username, + password: 'invalid-password' + }) + ) + }) + + await Promise.all(promises) + + const res = await request(app) + .post('/SASLogon/login') + .set('x-xsrf-token', csrfToken) + .send({ + username: user.username, + password: user.password + }) + .expect(429) + }) + + it.only('should respond with too many requests when attempting with invalid credentials for different users but with same ip 100 times', async () => { + await userController.createUser(user) + + process.dbInstance = con + + const promises: request.Test[] = [] + + Array(100) + .fill(0) + .map((_, i) => { + promises.push( + request(app) + .post('/SASLogon/login') + .set('x-xsrf-token', csrfToken) + .send({ + username: `user${i}`, + password: 'invalid-password' + }) + ) + }) + + await Promise.all(promises) + + const res = await request(app) + .post('/SASLogon/login') + .set('x-xsrf-token', csrfToken) + .send({ + username: user.username, + password: user.password + }) + .expect(429) + }) + it('should respond with Bad Request if CSRF Token is not present', async () => { await userController.createUser(user) diff --git a/api/src/routes/web/web.ts b/api/src/routes/web/web.ts index 6db26ed..6f73e47 100644 --- a/api/src/routes/web/web.ts +++ b/api/src/routes/web/web.ts @@ -35,7 +35,11 @@ webRouter.post('/SASLogon/login', desktopRestrict, async (req, res) => { const response = await controller.login(req, body) res.send(response) } catch (err: any) { - res.status(403).send(err.toString()) + if (err instanceof Error) { + res.status(500).send(err.toString()) + } else { + res.status(err.code).send(err.message) + } } }) diff --git a/api/src/types/system/process.d.ts b/api/src/types/system/process.d.ts index 6eb65e8..0e3ec80 100644 --- a/api/src/types/system/process.d.ts +++ b/api/src/types/system/process.d.ts @@ -14,5 +14,6 @@ declare namespace NodeJS { logger: import('@sasjs/utils/logger').Logger runTimes: import('../../utils').RunTimeType[] secrets: import('../../model/Configuration').ConfigurationType + dbInstance: import('mongoose').Mongoose } } diff --git a/api/src/utils/connectDB.ts b/api/src/utils/connectDB.ts index 8849cd5..beaee14 100644 --- a/api/src/utils/connectDB.ts +++ b/api/src/utils/connectDB.ts @@ -3,7 +3,9 @@ import { seedDB } from './seedDB' export const connectDB = async () => { try { - await mongoose.connect(process.env.DB_CONNECT as string) + process.dbInstance = await mongoose.connect( + process.env.DB_CONNECT as string + ) } catch (err) { throw new Error('Unable to connect to DB!') } diff --git a/api/src/utils/getRateLimiters.ts b/api/src/utils/getRateLimiters.ts new file mode 100644 index 0000000..ae65792 --- /dev/null +++ b/api/src/utils/getRateLimiters.ts @@ -0,0 +1,26 @@ +import { RateLimiterMongo } from 'rate-limiter-flexible' + +export const getRateLimiters = () => { + const { + MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY, + MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP + } = process.env + + const limiterSlowBruteByIP = new RateLimiterMongo({ + storeClient: process.dbInstance.connection, + keyPrefix: 'login_fail_ip_per_day', + points: Number(MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY), + duration: 60 * 60 * 24, + blockDuration: 60 * 60 * 24 // Block for 1 day + }) + + const limiterConsecutiveFailsByUsernameAndIP = new RateLimiterMongo({ + storeClient: process.dbInstance.connection, + keyPrefix: 'login_fail_consecutive_username_and_ip', + points: Number(MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP), + duration: 60 * 60 * 24 * 90, // Store number for 90 days since first fail + blockDuration: 60 * 60 // Block for 1 hour + }) + + return { limiterSlowBruteByIP, limiterConsecutiveFailsByUsernameAndIP } +} diff --git a/api/src/utils/index.ts b/api/src/utils/index.ts index 1814297..d32b3d1 100644 --- a/api/src/utils/index.ts +++ b/api/src/utils/index.ts @@ -13,6 +13,7 @@ export * from './getAuthorizedRoutes' export * from './getCertificates' export * from './getDesktopFields' export * from './getPreProgramVariables' +export * from './getRateLimiters' export * from './getRunTimeAndFilePath' export * from './getServerUrl' export * from './getTokensFromDB' @@ -20,10 +21,10 @@ export * from './instantiateLogger' export * from './isDebugOn' export * from './isPublicRoute' export * from './ldapClient' -export * from './zipped' export * from './parseLogToArray' export * from './removeTokensInDB' export * from './saveTokensInDB' +export * from './secondsToHms' export * from './seedDB' export * from './setProcessVariables' export * from './setupFolders' @@ -32,3 +33,4 @@ export * from './upload' export * from './validation' export * from './verifyEnvVariables' export * from './verifyTokenInDB' +export * from './zipped' diff --git a/api/src/utils/secondsToHms.ts b/api/src/utils/secondsToHms.ts new file mode 100644 index 0000000..7d8e8cb --- /dev/null +++ b/api/src/utils/secondsToHms.ts @@ -0,0 +1,10 @@ +export const secondsToHms = (seconds: number) => { + const h = Math.floor(seconds / 3600) + const m = Math.floor((seconds % 3600) / 60) + const s = Math.floor((seconds % 3600) % 60) + + const hDisplay = h > 0 ? h + (h == 1 ? ' hour, ' : ' hours, ') : '' + const mDisplay = m > 0 ? m + (m == 1 ? ' minute, ' : ' minutes, ') : '' + const sDisplay = s > 0 ? s + (s == 1 ? ' second' : ' seconds') : '' + return hDisplay + mDisplay + sDisplay +} diff --git a/api/src/utils/verifyEnvVariables.ts b/api/src/utils/verifyEnvVariables.ts index 043be7b..d6ec69b 100644 --- a/api/src/utils/verifyEnvVariables.ts +++ b/api/src/utils/verifyEnvVariables.ts @@ -77,6 +77,8 @@ export const verifyEnvVariables = (): ReturnCode => { errors.push(...verifyDbType()) + errors.push(...verifyRateLimiter()) + if (errors.length) { process.logger?.error( `Invalid environment variable(s) provided: \n${errors.join('\n')}` @@ -367,6 +369,50 @@ const verifyDbType = () => { return errors } +const verifyRateLimiter = () => { + const errors: string[] = [] + const { + MODE, + MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY, + MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP + } = process.env + if (MODE === ModeType.Server) { + if (MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY) { + if ( + !isNumeric(MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY) || + Number(MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY) < 1 + ) { + errors.push( + `- Invalid value for 'MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY' - Only positive number is acceptable` + ) + } + } else { + process.env.MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY = + DEFAULTS.MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY + } + + if (MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP) { + if ( + !isNumeric(MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP) || + Number(MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP) < 1 + ) { + errors.push( + `- Invalid value for 'MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP' - Only positive number is acceptable` + ) + } + } else { + process.env.MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP = + DEFAULTS.MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP + } + } + + return errors +} + +const isNumeric = (val: string): boolean => { + return !isNaN(Number(val)) +} + const DEFAULTS = { MODE: ModeType.Desktop, PROTOCOL: ProtocolType.HTTP, @@ -374,5 +420,7 @@ const DEFAULTS = { HELMET_COEP: HelmetCoepType.TRUE, LOG_FORMAT_MORGAN: LOG_FORMAT_MORGANType.Common, RUN_TIMES: RunTimeType.SAS, - DB_TYPE: DatabaseType.MONGO + DB_TYPE: DatabaseType.MONGO, + MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY: '100', + MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP: '10' }