diff --git a/docs/content/configuration.md b/docs/content/configuration.md index 2d44b6805..1dec4fdad 100644 --- a/docs/content/configuration.md +++ b/docs/content/configuration.md @@ -78,7 +78,7 @@ these are rarely used for various reasons. ## Web security aspects | config file | environment | **default** and example value | description | -| ----------------------------- | ------------------------------ | ------------------------------------------------------------------------------------------| ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| ----------------------------- | ------------------------------ |-------------------------------------------------------------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | `hsts` | | `{"enable": true, "maxAgeSeconds": 31536000, "includeSubdomains": true, "preload": true}` | [HSTS](https://en.wikipedia.org/wiki/HTTP_Strict_Transport_Security) options to use with HTTPS (default is the example value, max age is a year) | | | `CMD_HSTS_ENABLE` | **`true`** or `false` | set to enable [HSTS](https://en.wikipedia.org/wiki/HTTP_Strict_Transport_Security) if HTTPS is also enabled (default is ` true`) | | | `CMD_HSTS_INCLUDE_SUBDOMAINS` | **`true`** or `false` | set to include subdomains in HSTS (default is `true`) | @@ -95,6 +95,7 @@ these are rarely used for various reasons. | `csp.allowFraming` | `CMD_CSP_ALLOW_FRAMING` | **`true`** or `false` | Disable to disallow embedding of the instance via iframe. We **strongly recommend disabling** this option, as it increases the attack surface of XSS attacks. | | `csp.allowPDFEmbed` | `CMD_CSP_ALLOW_PDF_EMBED` | **`true`** or `false` | Disable to disallow embedding PDFs. We recommend disabling this option, as it increases the attack surface of XSS attacks. | | `cookiePolicy` | `CMD_COOKIE_POLICY` | **`lax`**, `strict` or `none` | Set a SameSite policy whether cookies are send from cross-origin. Be careful: setting a SameSite value of none without https breaks the editor. | +| `rateLimitNewNotes` | `CMD_RATE_LIMIT_NEW_NOTES` | **`20`**, `0` or any positive number | Sets the maximum amount of new note creations per 5-minute window per user. Can be disabled by setting to `0`. | ## Privacy and External Requests diff --git a/lib/config/default.js b/lib/config/default.js index d038e5311..34c65c9c5 100644 --- a/lib/config/default.js +++ b/lib/config/default.js @@ -29,6 +29,7 @@ module.exports = { allowFraming: true, allowPDFEmbed: true }, + rateLimitNewNotes: 20, cookiePolicy: 'lax', protocolUseSSL: false, allowAnonymous: true, diff --git a/lib/config/environment.js b/lib/config/environment.js index da50a660d..0720541ff 100644 --- a/lib/config/environment.js +++ b/lib/config/environment.js @@ -26,6 +26,7 @@ module.exports = { allowFraming: toBooleanConfig(process.env.CMD_CSP_ALLOW_FRAMING), allowPDFEmbed: toBooleanConfig(process.env.CMD_CSP_ALLOW_PDF_EMBED) }, + rateLimitNewNotes: toIntegerConfig(process.env.CMD_RATE_LIMIT_NEW_NOTES), cookiePolicy: process.env.CMD_COOKIE_POLICY, protocolUseSSL: toBooleanConfig(process.env.CMD_PROTOCOL_USESSL), allowOrigin: toArrayConfig(process.env.CMD_ALLOW_ORIGIN), diff --git a/lib/errors.js b/lib/errors.js index b8f983685..12c570b2b 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -26,6 +26,9 @@ module.exports = { errorTooLong: function (res) { responseError(res, 413, 'Payload Too Large', 'Shorten your note!') }, + errorTooManyRequests: function (res) { + responseError(res, 429, 'Too Many Requests', 'Try again later.') + }, errorInternalError: function (res) { responseError(res, 500, 'Internal Error', 'wtf.') }, diff --git a/lib/web/auth/email/index.js b/lib/web/auth/email/index.js index 2ca0cec76..2a434799f 100644 --- a/lib/web/auth/email/index.js +++ b/lib/web/auth/email/index.js @@ -9,6 +9,7 @@ const models = require('../../../models') const logger = require('../../../logger') const { urlencodedParser } = require('../../utils') const errors = require('../../../errors') +const rateLimit = require('../../middleware/rateLimit') const emailAuth = module.exports = Router() @@ -37,7 +38,7 @@ passport.use(new LocalStrategy({ })) if (config.allowEmailRegister) { - emailAuth.post('/register', urlencodedParser, function (req, res, next) { + emailAuth.post('/register', rateLimit.userEndpoints, urlencodedParser, function (req, res, next) { if (!req.body.email || !req.body.password) return errors.errorBadRequest(res) if (!validator.isEmail(req.body.email)) return errors.errorBadRequest(res) models.User.findOrCreate({ @@ -67,7 +68,7 @@ if (config.allowEmailRegister) { }) } -emailAuth.post('/login', urlencodedParser, function (req, res, next) { +emailAuth.post('/login', rateLimit.userEndpoints, urlencodedParser, function (req, res, next) { if (!req.body.email || !req.body.password) return errors.errorBadRequest(res) if (!validator.isEmail(req.body.email)) return errors.errorBadRequest(res) passport.authenticate('local', { diff --git a/lib/web/middleware/rateLimit.js b/lib/web/middleware/rateLimit.js new file mode 100644 index 000000000..a7864e51b --- /dev/null +++ b/lib/web/middleware/rateLimit.js @@ -0,0 +1,33 @@ +'use strict' + +const { rateLimit } = require('express-rate-limit') +const errors = require('../../errors') +const config = require('../../config') + +const determineKey = (req) => { + if (req.user) { + return req.user.id + } + return req.header('cf-connecting-ip') || req.ip +} + +// limits requests to user endpoints (login, signup) to 10 requests per 5 minutes +const userEndpoints = rateLimit({ + windowMs: 5 * 60 * 1000, + limit: 10, + keyGenerator: determineKey, + handler: (req, res) => errors.errorTooManyRequests(res) +}) + +// limits the amount of requests to the new note endpoint per 5 minutes based on configuration +const newNotes = rateLimit({ + windowMs: 5 * 60 * 1000, + limit: config.rateLimitNewNotes, + keyGenerator: determineKey, + handler: (req, res) => errors.errorTooManyRequests(res) +}) + +module.exports = { + userEndpoints, + newNotes +} diff --git a/lib/web/note/router.js b/lib/web/note/router.js index cf6fdf431..e1e5bc878 100644 --- a/lib/web/note/router.js +++ b/lib/web/note/router.js @@ -7,13 +7,22 @@ const router = module.exports = Router() const noteController = require('./controller') const slide = require('./slide') +const rateLimit = require('../middleware/rateLimit') +const config = require('../../config') + +const applyRateLimitIfConfigured = (req, res, next) => { + if (config.rateLimitNewNotes > 0) { + return rateLimit.newNotes(req, res, next) + } + next() +} // get new note -router.get('/new', noteController.createFromPOST) +router.get('/new', applyRateLimitIfConfigured, noteController.createFromPOST) // post new note with content -router.post('/new', markdownParser, noteController.createFromPOST) +router.post('/new', applyRateLimitIfConfigured, markdownParser, noteController.createFromPOST) // post new note with content and alias -router.post('/new/:noteId', markdownParser, noteController.createFromPOST) +router.post('/new/:noteId', applyRateLimitIfConfigured, markdownParser, noteController.createFromPOST) // get publish note router.get('/s/:shortid', noteController.showPublishNote) // publish note actions diff --git a/package.json b/package.json index 50f9b6cf7..06376e713 100644 --- a/package.json +++ b/package.json @@ -38,6 +38,7 @@ "diff-match-patch": "git+https://github.com/hackmdio/diff-match-patch.git#commit=59a9395ad9fe143e601e7ae5765ed943bdd2b11e", "ejs": "3.1.10", "express": "4.21.2", + "express-rate-limit": "7.4.1", "express-session": "1.18.1", "file-type": "18.7.0", "formidable": "2.1.2", diff --git a/public/docs/release-notes.md b/public/docs/release-notes.md index 213f7214d..ade8a1901 100644 --- a/public/docs/release-notes.md +++ b/public/docs/release-notes.md @@ -2,6 +2,10 @@ ## 1.x.x UNRELEASED +### Features +- Add fixed rate-limiting to the login and register endpoints +- Add configurable rate-limiting to the new notes endpoint + ### Bugfixes - Fix a crash when cannot read user profile in Oauth - Fix CSP Header for mermaid embedded images ([#5887](https://github.com/hedgedoc/hedgedoc/pull/5887) by [@domrim](https://github.com/domrim)) diff --git a/yarn.lock b/yarn.lock index c6f75ac12..ba3df3dea 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1286,6 +1286,7 @@ __metadata: exports-loader: "npm:1.1.1" expose-loader: "npm:1.0.3" express: "npm:4.21.2" + express-rate-limit: "npm:7.4.1" express-session: "npm:1.18.1" file-loader: "npm:6.2.0" file-saver: "npm:2.0.5" @@ -6473,6 +6474,15 @@ __metadata: languageName: node linkType: hard +"express-rate-limit@npm:7.4.1": + version: 7.4.1 + resolution: "express-rate-limit@npm:7.4.1" + peerDependencies: + express: 4 || 5 || ^5.0.0-beta.1 + checksum: 10/230cebc90d9a6baf0b471fa9039b5bf3d82f0a29dc7b304adee38eaa4803493266584108ca3d79d21993bdd45f9497c0b4eac9db8037cd3f10b19c529a9bdf66 + languageName: node + linkType: hard + "express-session@npm:1.18.1": version: 1.18.1 resolution: "express-session@npm:1.18.1"