From 490b595a01c5824ff63ffb87f0efdfca95f4bf3b Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Thu, 29 Mar 2018 10:58:24 +0200 Subject: [PATCH] Prevent brute force login attack --- client/src/app/core/auth/auth.service.ts | 8 +- .../app/shared/rest/rest-extractor.service.ts | 32 +++++--- client/src/app/signup/signup.component.ts | 2 +- config/default.yaml | 6 ++ config/production.yaml.example | 6 ++ package.json | 2 + server.ts | 4 + server/controllers/api/users.ts | 14 +++- server/controllers/api/videos/index.ts | 2 +- server/initializers/checker.ts | 1 + server/initializers/constants.ts | 9 ++ server/initializers/installer.ts | 2 +- server/tests/api/server/reverse-proxy.ts | 82 +++++++++++++++++++ server/tests/utils/videos/videos.ts | 11 ++- .../docker/production/config/production.yaml | 8 ++ yarn.lock | 22 +++++ 16 files changed, 189 insertions(+), 22 deletions(-) create mode 100644 server/tests/api/server/reverse-proxy.ts diff --git a/client/src/app/core/auth/auth.service.ts b/client/src/app/core/auth/auth.service.ts index f5ca2fcdc..d31c61496 100644 --- a/client/src/app/core/auth/auth.service.ts +++ b/client/src/app/core/auth/auth.service.ts @@ -66,8 +66,12 @@ export class AuthService { }, error => { - let errorMessage = `Cannot retrieve OAuth Client credentials: ${error.text}. \n` - errorMessage += 'Ensure you have correctly configured PeerTube (config/ directory), in particular the "webserver" section.' + let errorMessage = error.message + + if (error.status === 403) { + errorMessage = `Cannot retrieve OAuth Client credentials: ${error.text}. \n` + errorMessage += 'Ensure you have correctly configured PeerTube (config/ directory), in particular the "webserver" section.' + } // We put a bigger timeout // This is an important message diff --git a/client/src/app/shared/rest/rest-extractor.service.ts b/client/src/app/shared/rest/rest-extractor.service.ts index ad08a32f8..b1e22a76c 100644 --- a/client/src/app/shared/rest/rest-extractor.service.ts +++ b/client/src/app/shared/rest/rest-extractor.service.ts @@ -42,25 +42,33 @@ export class RestExtractor { console.error('An error occurred:', errorMessage) } else if (err.status !== undefined) { // A server-side error occurred. - if (err.error) { - if (err.error.errors) { - const errors = err.error.errors - const errorsArray: string[] = [] + if (err.error && err.error.errors) { + const errors = err.error.errors + const errorsArray: string[] = [] - Object.keys(errors).forEach(key => { - errorsArray.push(errors[key].msg) - }) + Object.keys(errors).forEach(key => { + errorsArray.push(errors[key].msg) + }) - errorMessage = errorsArray.join('. ') - } else if (err.error.error) { - errorMessage = err.error.error - } + errorMessage = errorsArray.join('. ') + } else if (err.error && err.error.error) { + errorMessage = err.error.error } else if (err.status === 413) { errorMessage = 'Request is too large for the server. Please contact you administrator if you want to increase the limit size.' + } else if (err.status === 429) { + const secondsLeft = err.headers.get('retry-after') + if (secondsLeft) { + const minutesLeft = Math.floor(parseInt(secondsLeft, 10) / 60) + errorMessage = 'Too many attempts, please try again after ' + minutesLeft + ' minutes.' + } else { + errorMessage = 'Too many attempts, please try again later.' + } + } else if (err.status === 500) { + errorMessage = 'Server error. Please retry later.' } errorMessage = errorMessage ? errorMessage : 'Unknown error.' - console.error(`Backend returned code ${err.status}, body was: ${errorMessage}`) + console.error(`Backend returned code ${err.status}, errorMessage is: ${errorMessage}`) } else { errorMessage = err } diff --git a/client/src/app/signup/signup.component.ts b/client/src/app/signup/signup.component.ts index 93d73a11e..1f3e2e146 100644 --- a/client/src/app/signup/signup.component.ts +++ b/client/src/app/signup/signup.component.ts @@ -101,7 +101,7 @@ export class SignupComponent extends FormReactive implements OnInit { const lines = [ SignupComponent.getApproximateTime(fullHdSeconds) + ' of full HD videos', SignupComponent.getApproximateTime(hdSeconds) + ' of HD videos', - SignupComponent.getApproximateTime(normalSeconds) + ' of normal quality videos' + SignupComponent.getApproximateTime(normalSeconds) + ' of average quality videos' ] this.quotaHelpIndication = lines.join('
') diff --git a/config/default.yaml b/config/default.yaml index 26fc5c128..bf772faf6 100644 --- a/config/default.yaml +++ b/config/default.yaml @@ -6,6 +6,12 @@ webserver: hostname: 'localhost' port: 9000 +# Proxies to trust to get real client IP +# If you run PeerTube just behind a local proxy (nginx), keep 'loopback' +# If you run PeerTube behind a remote proxy, add the proxy IP address (or subnet) +trust_proxy: + - 'loopback' + # Your database name will be "peertube"+database.suffix database: hostname: 'localhost' diff --git a/config/production.yaml.example b/config/production.yaml.example index 43cacee3b..d362e0b8a 100644 --- a/config/production.yaml.example +++ b/config/production.yaml.example @@ -7,6 +7,12 @@ webserver: hostname: 'example.com' port: 443 +# Proxies to trust to get real client IP +# If you run PeerTube just behind a local proxy (nginx), keep 'loopback' +# If you run PeerTube behind a remote proxy, add the proxy IP address (or subnet) +trust_proxy: + - 'loopback' + # Your database name will be "peertube"+database.suffix database: hostname: 'localhost' diff --git a/package.json b/package.json index 8a013e2d5..933bd7aa3 100644 --- a/package.json +++ b/package.json @@ -65,6 +65,7 @@ "create-torrent": "^3.24.5", "express": "^4.12.4", "express-oauth-server": "^2.0.0", + "express-rate-limit": "^2.11.0", "express-validator": "^5.0.0", "fluent-ffmpeg": "^2.1.0", "js-yaml": "^3.5.4", @@ -104,6 +105,7 @@ "@types/chai": "^4.0.4", "@types/config": "^0.0.34", "@types/express": "^4.0.35", + "@types/express-rate-limit": "^2.9.3", "@types/kue": "^0.11.8", "@types/lodash": "^4.14.64", "@types/magnet-uri": "^5.1.1", diff --git a/server.ts b/server.ts index f6794b897..b307e67a1 100644 --- a/server.ts +++ b/server.ts @@ -48,6 +48,9 @@ if (errorMessage !== null) { throw new Error(errorMessage) } +// Trust our proxy (IP forwarding...) +app.set('trust proxy', CONFIG.TRUST_PROXY) + // ----------- Database ----------- // Initialize database and models @@ -81,6 +84,7 @@ if (isTestInstance()) { ) { return (cors({ origin: 'http://localhost:3000', + exposedHeaders: 'Retry-After', credentials: true }))(req, res, next) } diff --git a/server/controllers/api/users.ts b/server/controllers/api/users.ts index 583376c38..5e96d789e 100644 --- a/server/controllers/api/users.ts +++ b/server/controllers/api/users.ts @@ -2,12 +2,13 @@ import * as express from 'express' import 'multer' import { extname, join } from 'path' import * as uuidv4 from 'uuid/v4' +import * as RateLimit from 'express-rate-limit' import { UserCreate, UserRight, UserRole, UserUpdate, UserUpdateMe, UserVideoRate as FormattedUserVideoRate } from '../../../shared' import { retryTransactionWrapper } from '../../helpers/database-utils' import { processImage } from '../../helpers/image-utils' import { logger } from '../../helpers/logger' import { createReqFiles, getFormattedObjects } from '../../helpers/utils' -import { AVATARS_SIZE, CONFIG, IMAGE_MIMETYPE_EXT, sequelizeTypescript } from '../../initializers' +import { AVATARS_SIZE, CONFIG, IMAGE_MIMETYPE_EXT, RATES_LIMIT, sequelizeTypescript } from '../../initializers' import { updateActorAvatarInstance } from '../../lib/activitypub' import { sendUpdateActor } from '../../lib/activitypub/send' import { Emailer } from '../../lib/emailer' @@ -43,6 +44,11 @@ import { OAuthTokenModel } from '../../models/oauth/oauth-token' import { VideoModel } from '../../models/video/video' const reqAvatarFile = createReqFiles([ 'avatarfile' ], IMAGE_MIMETYPE_EXT, { avatarfile: CONFIG.STORAGE.AVATARS_DIR }) +const loginRateLimiter = new RateLimit({ + windowMs: RATES_LIMIT.LOGIN.WINDOW_MS, + max: RATES_LIMIT.LOGIN.MAX, + delayMs: 0 +}) const usersRouter = express.Router() @@ -136,7 +142,11 @@ usersRouter.post('/:id/reset-password', asyncMiddleware(resetUserPassword) ) -usersRouter.post('/token', token, success) +usersRouter.post('/token', + loginRateLimiter, + token, + success +) // TODO: Once https://github.com/oauthjs/node-oauth2-server/pull/289 is merged, implement revoke token route // --------------------------------------------------------------------------- diff --git a/server/controllers/api/videos/index.ts b/server/controllers/api/videos/index.ts index c0a8ac118..552e5edac 100644 --- a/server/controllers/api/videos/index.ts +++ b/server/controllers/api/videos/index.ts @@ -353,7 +353,7 @@ function getVideo (req: express.Request, res: express.Response) { async function viewVideo (req: express.Request, res: express.Response) { const videoInstance = res.locals.video - const ip = req.headers['x-real-ip'] as string || req.ip + const ip = req.ip const exists = await Redis.Instance.isViewExists(ip, videoInstance.uuid) if (exists) { logger.debug('View for ip %s and video %s already exists.', ip, videoInstance.uuid) diff --git a/server/initializers/checker.ts b/server/initializers/checker.ts index cd93f19a9..45f1d79c3 100644 --- a/server/initializers/checker.ts +++ b/server/initializers/checker.ts @@ -20,6 +20,7 @@ function checkConfig () { function checkMissedConfig () { const required = [ 'listen.port', 'webserver.https', 'webserver.hostname', 'webserver.port', + 'trust_proxy', 'database.hostname', 'database.port', 'database.suffix', 'database.username', 'database.password', 'redis.hostname', 'redis.port', 'redis.auth', 'smtp.hostname', 'smtp.port', 'smtp.username', 'smtp.password', 'smtp.tls', 'smtp.from_address', diff --git a/server/initializers/constants.ts b/server/initializers/constants.ts index 284acf8f3..986fed099 100644 --- a/server/initializers/constants.ts +++ b/server/initializers/constants.ts @@ -127,6 +127,7 @@ const CONFIG = { URL: '', HOST: '' }, + TRUST_PROXY: config.get('trust_proxy'), LOG: { LEVEL: config.get('log.level') }, @@ -234,6 +235,13 @@ const CONSTRAINTS_FIELDS = { } } +const RATES_LIMIT = { + LOGIN: { + WINDOW_MS: 5 * 60 * 1000, // 5 minutes + MAX: 10 // 10 attempts + } +} + let VIDEO_VIEW_LIFETIME = 60000 * 60 // 1 hour const VIDEO_TRANSCODING_FPS = { MIN: 10, @@ -468,6 +476,7 @@ export { USER_PASSWORD_RESET_LIFETIME, IMAGE_MIMETYPE_EXT, SCHEDULER_INTERVAL, + RATES_LIMIT, JOB_COMPLETED_LIFETIME, VIDEO_VIEW_LIFETIME } diff --git a/server/initializers/installer.ts b/server/initializers/installer.ts index d2f6c7c8c..f0adf8c9e 100644 --- a/server/initializers/installer.ts +++ b/server/initializers/installer.ts @@ -112,7 +112,7 @@ async function createOAuthAdminIfNotExist () { // Our password is weak so do not validate it validatePassword = false } else { - password = passwordGenerator(8, true) + password = passwordGenerator(16, true) } const userData = { diff --git a/server/tests/api/server/reverse-proxy.ts b/server/tests/api/server/reverse-proxy.ts new file mode 100644 index 000000000..aa4b3ae81 --- /dev/null +++ b/server/tests/api/server/reverse-proxy.ts @@ -0,0 +1,82 @@ +/* tslint:disable:no-unused-expression */ + +import 'mocha' +import * as chai from 'chai' +import { About } from '../../../../shared/models/server/about.model' +import { CustomConfig } from '../../../../shared/models/server/custom-config.model' +import { deleteCustomConfig, getAbout, getVideo, killallServers, login, reRunServer, uploadVideo, userLogin, viewVideo } from '../../utils' +const expect = chai.expect + +import { + getConfig, + flushTests, + runServer, + registerUser, getCustomConfig, setAccessTokensToServers, updateCustomConfig +} from '../../utils/index' + +describe('Test application behind a reverse proxy', function () { + let server = null + let videoId + + before(async function () { + this.timeout(30000) + + await flushTests() + server = await runServer(1) + await setAccessTokensToServers([ server ]) + + const { body } = await uploadVideo(server.url, server.accessToken, {}) + videoId = body.video.uuid + }) + + it('Should view a video only once with the same IP by default', async function () { + await viewVideo(server.url, videoId) + await viewVideo(server.url, videoId) + + const { body } = await getVideo(server.url, videoId) + expect(body.views).to.equal(1) + }) + + it('Should view a video 2 times with the X-Forwarded-For header set', async function () { + await viewVideo(server.url, videoId, 204, '0.0.0.1,127.0.0.1') + await viewVideo(server.url, videoId, 204, '0.0.0.2,127.0.0.1') + + const { body } = await getVideo(server.url, videoId) + expect(body.views).to.equal(3) + }) + + it('Should view a video only once with the same client IP in the X-Forwarded-For header', async function () { + await viewVideo(server.url, videoId, 204, '0.0.0.4,0.0.0.3,::ffff:127.0.0.1') + await viewVideo(server.url, videoId, 204, '0.0.0.5,0.0.0.3,127.0.0.1') + + const { body } = await getVideo(server.url, videoId) + expect(body.views).to.equal(4) + }) + + it('Should view a video two times with a different client IP in the X-Forwarded-For header', async function () { + await viewVideo(server.url, videoId, 204, '0.0.0.8,0.0.0.6,127.0.0.1') + await viewVideo(server.url, videoId, 204, '0.0.0.8,0.0.0.7,127.0.0.1') + + const { body } = await getVideo(server.url, videoId) + expect(body.views).to.equal(6) + }) + + it('Should rate limit logins', async function () { + const user = { username: 'root', password: 'fail' } + + for (let i = 0; i < 9; i++) { + await userLogin(server, user, 400) + } + + await userLogin(server, user, 429) + }) + + after(async function () { + process.kill(-server.app.pid) + + // Keep the logs if the test failed + if (this['ok']) { + await flushTests() + } + }) +}) diff --git a/server/tests/utils/videos/videos.ts b/server/tests/utils/videos/videos.ts index 424f41ed8..9bda53371 100644 --- a/server/tests/utils/videos/videos.ts +++ b/server/tests/utils/videos/videos.ts @@ -85,13 +85,18 @@ function getVideo (url: string, id: number | string, expectedStatus = 200) { .expect(expectedStatus) } -function viewVideo (url: string, id: number | string, expectedStatus = 204) { +function viewVideo (url: string, id: number | string, expectedStatus = 204, xForwardedFor?: string) { const path = '/api/v1/videos/' + id + '/views' - return request(url) + const req = request(url) .post(path) .set('Accept', 'application/json') - .expect(expectedStatus) + + if (xForwardedFor) { + req.set('X-Forwarded-For', xForwardedFor) + } + + return req.expect(expectedStatus) } function getVideoWithToken (url: string, token: string, id: number | string, expectedStatus = 200) { diff --git a/support/docker/production/config/production.yaml b/support/docker/production/config/production.yaml index 41272ba26..7b6de32e5 100644 --- a/support/docker/production/config/production.yaml +++ b/support/docker/production/config/production.yaml @@ -7,6 +7,14 @@ webserver: hostname: undefined port: 443 +# Proxies to trust to get real client IP +# If you run PeerTube just behind a local proxy (nginx), keep 'loopback' +# If you run PeerTube behind a remote proxy, add the proxy IP address (or subnet) +trust_proxy: + - 'loopback' + - 'linklocal' + - 'uniquelocal' + # Your database name will be "peertube"+database.suffix database: hostname: 'db' diff --git a/yarn.lock b/yarn.lock index a56dd022d..726f1b4ed 100644 --- a/yarn.lock +++ b/yarn.lock @@ -63,6 +63,12 @@ version "1.2.0" resolved "https://registry.yarnpkg.com/@types/events/-/events-1.2.0.tgz#81a6731ce4df43619e5c8c945383b3e62a89ea86" +"@types/express-rate-limit@^2.9.3": + version "2.9.3" + resolved "https://registry.yarnpkg.com/@types/express-rate-limit/-/express-rate-limit-2.9.3.tgz#e83a548bf251ad12ca49055c22d3f2da4e16b62d" + dependencies: + "@types/express" "*" + "@types/express-serve-static-core@*": version "4.11.1" resolved "https://registry.yarnpkg.com/@types/express-serve-static-core/-/express-serve-static-core-4.11.1.tgz#f6f7212382d59b19d696677bcaa48a37280f5d45" @@ -1140,6 +1146,10 @@ cliui@^3.2.0: strip-ansi "^3.0.1" wrap-ansi "^2.0.0" +clone@^1.0.2: + version "1.0.4" + resolved "https://registry.yarnpkg.com/clone/-/clone-1.0.4.tgz#da309cc263df15994c688ca902179ca3c7cd7c7e" + closest-to@~2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/closest-to/-/closest-to-2.0.0.tgz#bb2a860edb7769b62d04821748ae50da24dbefaa" @@ -1569,6 +1579,12 @@ deep-extend@~0.4.0: version "0.4.2" resolved "https://registry.yarnpkg.com/deep-extend/-/deep-extend-0.4.2.tgz#48b699c27e334bf89f10892be432f6e4c7d34a7f" +defaults@^1.0.3: + version "1.0.3" + resolved "https://registry.yarnpkg.com/defaults/-/defaults-1.0.3.tgz#c656051e9817d9ff08ed881477f3fe4019f3ef7d" + dependencies: + clone "^1.0.2" + define-property@^0.2.5: version "0.2.5" resolved "https://registry.yarnpkg.com/define-property/-/define-property-0.2.5.tgz#c35b1ef918ec3c990f9a5bc57be04aacec5c8116" @@ -1914,6 +1930,12 @@ express-oauth-server@^2.0.0: express "^4.13.3" oauth2-server "3.0.0" +express-rate-limit@^2.11.0: + version "2.11.0" + resolved "https://registry.yarnpkg.com/express-rate-limit/-/express-rate-limit-2.11.0.tgz#092122218c86eddb56fb350f431e522fb8024ea9" + dependencies: + defaults "^1.0.3" + express-validator@^5.0.0: version "5.0.3" resolved "https://registry.yarnpkg.com/express-validator/-/express-validator-5.0.3.tgz#c31176740f216c5ce043d6e20c7afa1db1a2691e"