Explorar el Código

Merge pull request #1406 from sharelatex/spd-more-rate-limits

Add additional rate limits to prevent resource-exhaustion attacks

GitOrigin-RevId: 428cf8a16e062267dd92e7fba73ef5c192a8e668
Simon Detheridge hace 7 años
padre
commit
64f69529e0

+ 15 - 2
services/web/app/coffee/Features/Editor/EditorRouter.coffee

@@ -1,11 +1,24 @@
 EditorHttpController = require('./EditorHttpController')
 AuthenticationController = require "../Authentication/AuthenticationController"
 AuthorizationMiddlewear = require('../Authorization/AuthorizationMiddlewear')
+RateLimiterMiddlewear = require('../Security/RateLimiterMiddlewear')
 
 module.exports =
 	apply: (webRouter, apiRouter) ->
-		webRouter.post   '/project/:Project_id/doc', AuthorizationMiddlewear.ensureUserCanWriteProjectContent, EditorHttpController.addDoc
-		webRouter.post   '/project/:Project_id/folder', AuthorizationMiddlewear.ensureUserCanWriteProjectContent, EditorHttpController.addFolder
+		webRouter.post   '/project/:Project_id/doc', AuthorizationMiddlewear.ensureUserCanWriteProjectContent,
+			RateLimiterMiddlewear.rateLimit({
+				endpointName: "add-doc-to-project"
+				params: ["Project_id"]
+				maxRequests: 30
+				timeInterval: 60
+			}), EditorHttpController.addDoc
+		webRouter.post   '/project/:Project_id/folder', AuthorizationMiddlewear.ensureUserCanWriteProjectContent,
+			RateLimiterMiddlewear.rateLimit({
+				endpointName: "add-folder-to-project"
+				params: ["Project_id"]
+				maxRequests: 60
+				timeInterval: 60
+			}), EditorHttpController.addFolder
 
 		webRouter.post   '/project/:Project_id/:entity_type/:entity_id/rename', AuthorizationMiddlewear.ensureUserCanWriteProjectContent, EditorHttpController.renameEntity
 		webRouter.post   '/project/:Project_id/:entity_type/:entity_id/move', AuthorizationMiddlewear.ensureUserCanWriteProjectContent, EditorHttpController.moveEntity

+ 13 - 0
services/web/app/coffee/Features/LinkedFiles/LinkedFilesRouter.coffee

@@ -1,5 +1,6 @@
 AuthorizationMiddlewear = require('../Authorization/AuthorizationMiddlewear')
 AuthenticationController = require('../Authentication/AuthenticationController')
+RateLimiterMiddlewear = require('../Security/RateLimiterMiddlewear')
 LinkedFilesController = require "./LinkedFilesController"
 
 module.exports =
@@ -7,9 +8,21 @@ module.exports =
 		webRouter.post '/project/:project_id/linked_file',
 			AuthenticationController.requireLogin(),
 			AuthorizationMiddlewear.ensureUserCanWriteProjectContent,
+			RateLimiterMiddlewear.rateLimit({
+				endpointName: "create-linked-file"
+				params: ["project_id"]
+				maxRequests: 100
+				timeInterval: 60
+			}),
 			LinkedFilesController.createLinkedFile
 
 		webRouter.post '/project/:project_id/linked_file/:file_id/refresh',
 			AuthenticationController.requireLogin(),
 			AuthorizationMiddlewear.ensureUserCanWriteProjectContent,
+			RateLimiterMiddlewear.rateLimit({
+				endpointName: "refresh-linked-file"
+				params: ["project_id"]
+				maxRequests: 100
+				timeInterval: 60
+			}),
 			LinkedFilesController.refreshLinkedFile

+ 6 - 1
services/web/app/coffee/Features/Templates/TemplatesRouter.coffee

@@ -1,10 +1,15 @@
 AuthenticationController = require('../Authentication/AuthenticationController')
 TemplatesController = require("./TemplatesController")
 TemplatesMiddlewear = require('./TemplatesMiddlewear')
+RateLimiterMiddlewear = require('../Security/RateLimiterMiddlewear')
 
 module.exports = 
 	apply: (app)->
 
 		app.get '/project/new/template/:Template_version_id', TemplatesMiddlewear.saveTemplateDataInSession, AuthenticationController.requireLogin(), TemplatesController.getV1Template
 
-		app.post '/project/new/template', AuthenticationController.requireLogin(), TemplatesController.createProjectFromV1Template
+		app.post '/project/new/template', AuthenticationController.requireLogin(), RateLimiterMiddlewear.rateLimit({
+			endpointName: "create-project-from-template"
+			maxRequests: 20
+			timeInterval: 60
+		}), TemplatesController.createProjectFromV1Template

+ 11 - 0
services/web/app/coffee/Features/UserMembership/UserMembershipRouter.coffee

@@ -3,6 +3,7 @@ UserMembershipController = require './UserMembershipController'
 SubscriptionGroupController = require '../Subscription/SubscriptionGroupController'
 TeamInvitesController = require '../Subscription/TeamInvitesController'
 AuthorizationMiddlewear = require('../Authorization/AuthorizationMiddlewear')
+RateLimiterMiddlewear = require('../Security/RateLimiterMiddlewear')
 
 module.exports =
 	apply: (webRouter) ->
@@ -12,6 +13,11 @@ module.exports =
 			UserMembershipController.index
 		webRouter.post '/manage/groups/:id/invites',
 			UserMembershipAuthorization.requireGroupManagementAccess,
+			RateLimiterMiddlewear.rateLimit({
+				endpointName: "create-team-invite"
+				maxRequests: 100
+				timeInterval: 60
+			}),
 			TeamInvitesController.createInvite
 		webRouter.delete '/manage/groups/:id/user/:user_id',
 			UserMembershipAuthorization.requireGroupManagementAccess,
@@ -21,6 +27,11 @@ module.exports =
 			TeamInvitesController.revokeInvite
 		webRouter.get '/manage/groups/:id/members/export',
 			UserMembershipAuthorization.requireGroupManagementAccess,
+			RateLimiterMiddlewear.rateLimit({
+				endpointName: "export-team-csv"
+				maxRequests: 30
+				timeInterval: 60
+			}),
 			UserMembershipController.exportCsv
 
 		# group managers routes

+ 2 - 0
services/web/app/coffee/infrastructure/RateLimiter.coffee

@@ -1,4 +1,5 @@
 settings = require("settings-sharelatex")
+Metrics = require('metrics-sharelatex')
 RedisWrapper = require('./RedisWrapper')
 rclient = RedisWrapper.client('ratelimiter')
 RollingRateLimiter = require('rolling-rate-limiter')
@@ -19,6 +20,7 @@ module.exports = RateLimiter =
 			if err?
 				return callback(err)
 			allowed = timeLeft == 0
+			Metrics.inc "rate-limit-hit.#{opts.endpointName}", 1, {path: opts.endpointName} unless allowed
 			callback(null, allowed)
 
 	clearRateLimit: (endpointName, subject, callback) ->

+ 56 - 9
services/web/app/coffee/router.coffee

@@ -121,6 +121,12 @@ module.exports = class Router
 		webRouter.get '/user/emails/confirm',
 			UserEmailsController.showConfirm
 		webRouter.post '/user/emails/confirm',
+			AuthenticationController.requireLogin(),
+			RateLimiterMiddlewear.rateLimit({
+				endpointName: "confirm-email"
+				maxRequests: 10
+				timeInterval: 60
+			}),
 			UserEmailsController.confirm
 		webRouter.post '/user/emails/resend_confirmation',
 			AuthenticationController.requireLogin(),
@@ -153,6 +159,11 @@ module.exports = class Router
 				UserEmailsController.setDefault
 			webRouter.post '/user/emails/endorse',
 				AuthenticationController.requireLogin(),
+				RateLimiterMiddlewear.rateLimit({
+					endpointName: "endorse-email"
+					maxRequests: 30
+					timeInterval: 60
+				}),
 				UserEmailsController.endorse
 
 
@@ -174,7 +185,11 @@ module.exports = class Router
 			ProjectController.projectEntitiesJson
 
 		webRouter.get  '/project', AuthenticationController.requireLogin(), ProjectController.projectListPage
-		webRouter.post '/project/new', AuthenticationController.requireLogin(), ProjectController.newProject
+		webRouter.post '/project/new', AuthenticationController.requireLogin(), RateLimiterMiddlewear.rateLimit({
+			endpointName: "create-project"
+			maxRequests: 20
+			timeInterval: 60
+		}), ProjectController.newProject
 
 		webRouter.get  '/Project/:Project_id', RateLimiterMiddlewear.rateLimit({
 			endpointName: "open-project"
@@ -278,11 +293,31 @@ module.exports = class Router
 
 
 		webRouter.get    '/tag', AuthenticationController.requireLogin(), TagsController.getAllTags
-		webRouter.post   '/tag', AuthenticationController.requireLogin(), TagsController.createTag
-		webRouter.post   '/tag/:tag_id/rename', AuthenticationController.requireLogin(), TagsController.renameTag
-		webRouter.delete '/tag/:tag_id', AuthenticationController.requireLogin(), TagsController.deleteTag
-		webRouter.post   '/tag/:tag_id/project/:project_id', AuthenticationController.requireLogin(), TagsController.addProjectToTag
-		webRouter.delete '/tag/:tag_id/project/:project_id', AuthenticationController.requireLogin(), TagsController.removeProjectFromTag
+		webRouter.post   '/tag', AuthenticationController.requireLogin(), RateLimiterMiddlewear.rateLimit({
+			endpointName: "create-tag"
+			maxRequests: 30
+			timeInterval: 60
+		}), TagsController.createTag
+		webRouter.post   '/tag/:tag_id/rename', AuthenticationController.requireLogin(), RateLimiterMiddlewear.rateLimit({
+			endpointName: "rename-tag"
+			maxRequests: 30
+			timeInterval: 60
+		}), TagsController.renameTag
+		webRouter.delete '/tag/:tag_id', AuthenticationController.requireLogin(), RateLimiterMiddlewear.rateLimit({
+			endpointName: "delete-tag"
+			maxRequests: 30
+			timeInterval: 60
+		}), TagsController.deleteTag
+		webRouter.post   '/tag/:tag_id/project/:project_id', AuthenticationController.requireLogin(), RateLimiterMiddlewear.rateLimit({
+			endpointName: "add-project-to-tag"
+			maxRequests: 30
+			timeInterval: 60
+		}), TagsController.addProjectToTag
+		webRouter.delete '/tag/:tag_id/project/:project_id', AuthenticationController.requireLogin(), RateLimiterMiddlewear.rateLimit({
+			endpointName: "remove-project-from-tag"
+			maxRequests: 30
+			timeInterval: 60
+		}), TagsController.removeProjectFromTag
 
 		webRouter.get '/notifications', AuthenticationController.requireLogin(), NotificationsController.getAllUnreadNotifications
 		webRouter.delete '/notifications/:notification_id', AuthenticationController.requireLogin(), NotificationsController.markNotificationAsRead
@@ -323,10 +358,22 @@ module.exports = class Router
 		webRouter.post "/spelling/learn", AuthenticationController.requireLogin(), SpellingController.proxyRequestToSpellingApi
 
 		webRouter.get  "/project/:project_id/messages", AuthorizationMiddlewear.ensureUserCanReadProject, ChatController.getMessages
-		webRouter.post "/project/:project_id/messages", AuthorizationMiddlewear.ensureUserCanReadProject, ChatController.sendMessage
+		webRouter.post "/project/:project_id/messages", AuthorizationMiddlewear.ensureUserCanReadProject, RateLimiterMiddlewear.rateLimit({
+			endpointName: "send-chat-message"
+			maxRequests: 100
+			timeInterval: 60
+		}), ChatController.sendMessage
 
-		webRouter.post "/project/:Project_id/references/index", AuthorizationMiddlewear.ensureUserCanReadProject, ReferencesController.index
-		webRouter.post "/project/:Project_id/references/indexAll", AuthorizationMiddlewear.ensureUserCanReadProject, ReferencesController.indexAll
+		webRouter.post "/project/:Project_id/references/index", AuthorizationMiddlewear.ensureUserCanReadProject, RateLimiterMiddlewear.rateLimit({
+			endpointName: "index-project-references"
+			maxRequests: 30
+			timeInterval: 60
+		}), ReferencesController.index
+		webRouter.post "/project/:Project_id/references/indexAll", AuthorizationMiddlewear.ensureUserCanReadProject, RateLimiterMiddlewear.rateLimit({
+			endpointName: "index-all-project-references"
+			maxRequests: 30
+			timeInterval: 60
+		}), ReferencesController.indexAll
 
 		# disable beta program while v2 is in beta
 		# webRouter.get "/beta/participate",  AuthenticationController.requireLogin(), BetaProgramController.optInPage

+ 11 - 0
services/web/test/unit/coffee/infrastructure/RateLimterTests.coffee

@@ -32,6 +32,7 @@ describe "RateLimiter", ->
 		@requires =
 			"settings-sharelatex":@settings
 			"logger-sharelatex" : @logger = {log:sinon.stub(), err:sinon.stub()}
+			"metrics-sharelatex" : @Metrics = {inc: sinon.stub()}
 			"./RedisWrapper": @RedisWrapper
 
 		@details = 
@@ -61,6 +62,11 @@ describe "RateLimiter", ->
 				expect(should).to.equal true
 				done()
 
+		it 'should not increment the metric', (done) ->
+			@limiter.addCount {endpointName: @endpointName}, (err, should) =>
+				sinon.assert.notCalled(@Metrics.inc)
+				done()
+
 	describe 'when action is not permitted', ->
 
 		beforeEach ->
@@ -78,6 +84,11 @@ describe "RateLimiter", ->
 				expect(should).to.equal false
 				done()
 
+		it 'should increment the metric', (done) ->
+			@limiter.addCount {endpointName: @endpointName}, (err, should) =>
+				sinon.assert.calledWith(@Metrics.inc, "rate-limit-hit.#{@endpointName}", 1, {path: @endpointName})
+				done()
+
 	describe 'when limiter produces an error', ->
 
 		beforeEach ->