From 161659de5cd7633161b0788799b641ff6b7e55f9 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Sat, 28 May 2022 05:06:47 +0200 Subject: enhance: replace signin CAPTCHA with rate limit (#8740) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * enhance: rate limit works without signed in user * fix: make limit key required for limiter As before the fallback limiter key will be set from the endpoint name. * enhance: use limiter for signin * Revert "CAPTCHA求めるのは2fa認証が無効になっているときだけにした" This reverts commit 02a43a310f6ad0cc9e9beccc26e51ab5b339e15f. * Revert "feat: make captcha required when signin to improve security" This reverts commit b21b0580058c14532ff3f4033e2a9147643bfca6. * fix undefined reference * fix: better error message * enhance: only handle prefix of IPv6 --- packages/backend/src/server/api/call.ts | 49 +++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 14 deletions(-) (limited to 'packages/backend/src/server/api/call.ts') diff --git a/packages/backend/src/server/api/call.ts b/packages/backend/src/server/api/call.ts index 9a85e4565b..fbe25e1732 100644 --- a/packages/backend/src/server/api/call.ts +++ b/packages/backend/src/server/api/call.ts @@ -2,10 +2,11 @@ import Koa from 'koa'; import { performance } from 'perf_hooks'; import { limiter } from './limiter.js'; import { CacheableLocalUser, User } from '@/models/entities/user.js'; -import endpoints, { IEndpoint } from './endpoints.js'; +import endpoints, { IEndpointMeta } from './endpoints.js'; import { ApiError } from './error.js'; import { apiLogger } from './logger.js'; import { AccessToken } from '@/models/entities/access-token.js'; +import IPCIDR from 'ip-cidr'; const accessDenied = { message: 'Access denied.', @@ -15,6 +16,7 @@ const accessDenied = { export default async (endpoint: string, user: CacheableLocalUser | null | undefined, token: AccessToken | null | undefined, data: any, ctx?: Koa.Context) => { const isSecure = user != null && token == null; + const isModerator = user != null && (user.isModerator || user.isAdmin); const ep = endpoints.find(e => e.name === endpoint); @@ -31,6 +33,37 @@ export default async (endpoint: string, user: CacheableLocalUser | null | undefi throw new ApiError(accessDenied); } + if (ep.meta.requireCredential && ep.meta.limit && !isModerator) { + // koa will automatically load the `X-Forwarded-For` header if `proxy: true` is configured in the app. + let limitActor: string; + if (user) { + limitActor = user.id; + } else { + // because a single person may control many IPv6 addresses, + // only a /64 subnet prefix of any IP will be taken into account. + // (this means for IPv4 the entire address is used) + const ip = IPCIDR.createAddress(ctx.ip).mask(64); + + limitActor = 'ip-' + parseInt(ip, 2).toString(36); + } + + const limit = Object.assign({}, ep.meta.limit); + + if (!limit.key) { + limit.key = ep.name; + } + + // Rate limit + await limiter(limit as IEndpointMeta['limit'] & { key: NonNullable }, limitActor).catch(e => { + throw new ApiError({ + message: 'Rate limit exceeded. Please try again later.', + code: 'RATE_LIMIT_EXCEEDED', + id: 'd5826d14-3982-4d2e-8011-b9e9f02499ef', + httpStatusCode: 429, + }); + }); + } + if (ep.meta.requireCredential && user == null) { throw new ApiError({ message: 'Credential required.', @@ -53,7 +86,7 @@ export default async (endpoint: string, user: CacheableLocalUser | null | undefi throw new ApiError(accessDenied, { reason: 'You are not the admin.' }); } - if (ep.meta.requireModerator && !user!.isAdmin && !user!.isModerator) { + if (ep.meta.requireModerator && !isModerator) { throw new ApiError(accessDenied, { reason: 'You are not a moderator.' }); } @@ -65,18 +98,6 @@ export default async (endpoint: string, user: CacheableLocalUser | null | undefi }); } - if (ep.meta.requireCredential && ep.meta.limit && !user!.isAdmin && !user!.isModerator) { - // Rate limit - await limiter(ep as IEndpoint & { meta: { limit: NonNullable } }, user!).catch(e => { - throw new ApiError({ - message: 'Rate limit exceeded. Please try again later.', - code: 'RATE_LIMIT_EXCEEDED', - id: 'd5826d14-3982-4d2e-8011-b9e9f02499ef', - httpStatusCode: 429, - }); - }); - } - // Cast non JSON input if (ep.meta.requireFile && ep.params.properties) { for (const k of Object.keys(ep.params.properties)) { -- cgit v1.2.3-freya From c05723ca6ad4f17b823662e83ed8b442fe10626a Mon Sep 17 00:00:00 2001 From: MeiMei <30769358+mei23@users.noreply.github.com> Date: Tue, 31 May 2022 17:44:22 +0900 Subject: Fix IP address rate limit (#8758) * Fix IP address rate limit * CHANGELOG * Tune getIpHash --- CHANGELOG.md | 2 +- packages/backend/src/misc/get-ip-hash.ts | 9 +++++++++ packages/backend/src/server/api/call.ts | 11 +++-------- packages/backend/src/server/api/private/signin.ts | 3 ++- 4 files changed, 15 insertions(+), 10 deletions(-) create mode 100644 packages/backend/src/misc/get-ip-hash.ts (limited to 'packages/backend/src/server/api/call.ts') diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b086ddba9..cfcf52ce92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,7 @@ You should also include the user name that made the change. Your own theme color may be unset if it was in an invalid format. Admins should check their instance settings if in doubt. - Perform port diagnosis at startup only when Listen fails @mei23 -- Rate limiting is now also usable for non-authenticated users. @Johann150 +- Rate limiting is now also usable for non-authenticated users. @Johann150 @mei23 Admins should make sure the reverse proxy sets the `X-Forwarded-For` header to the original address. ### Bugfixes diff --git a/packages/backend/src/misc/get-ip-hash.ts b/packages/backend/src/misc/get-ip-hash.ts new file mode 100644 index 0000000000..379325bb13 --- /dev/null +++ b/packages/backend/src/misc/get-ip-hash.ts @@ -0,0 +1,9 @@ +import IPCIDR from 'ip-cidr'; + +export function getIpHash(ip: string) { + // because a single person may control many IPv6 addresses, + // only a /64 subnet prefix of any IP will be taken into account. + // (this means for IPv4 the entire address is used) + const prefix = IPCIDR.createAddress(ip).mask(64); + return 'ip-' + BigInt('0b' + prefix).toString(36); +} diff --git a/packages/backend/src/server/api/call.ts b/packages/backend/src/server/api/call.ts index fbe25e1732..cd3e0abc06 100644 --- a/packages/backend/src/server/api/call.ts +++ b/packages/backend/src/server/api/call.ts @@ -6,7 +6,7 @@ import endpoints, { IEndpointMeta } from './endpoints.js'; import { ApiError } from './error.js'; import { apiLogger } from './logger.js'; import { AccessToken } from '@/models/entities/access-token.js'; -import IPCIDR from 'ip-cidr'; +import { getIpHash } from '@/misc/get-ip-hash.js'; const accessDenied = { message: 'Access denied.', @@ -33,18 +33,13 @@ export default async (endpoint: string, user: CacheableLocalUser | null | undefi throw new ApiError(accessDenied); } - if (ep.meta.requireCredential && ep.meta.limit && !isModerator) { + if (ep.meta.limit && !isModerator) { // koa will automatically load the `X-Forwarded-For` header if `proxy: true` is configured in the app. let limitActor: string; if (user) { limitActor = user.id; } else { - // because a single person may control many IPv6 addresses, - // only a /64 subnet prefix of any IP will be taken into account. - // (this means for IPv4 the entire address is used) - const ip = IPCIDR.createAddress(ctx.ip).mask(64); - - limitActor = 'ip-' + parseInt(ip, 2).toString(36); + limitActor = getIpHash(ctx!.ip); } const limit = Object.assign({}, ep.meta.limit); diff --git a/packages/backend/src/server/api/private/signin.ts b/packages/backend/src/server/api/private/signin.ts index b304550e29..79b31764fd 100644 --- a/packages/backend/src/server/api/private/signin.ts +++ b/packages/backend/src/server/api/private/signin.ts @@ -10,6 +10,7 @@ import { verifyLogin, hash } from '../2fa.js'; import { randomBytes } from 'node:crypto'; import { IsNull } from 'typeorm'; import { limiter } from '../limiter.js'; +import { getIpHash } from '@/misc/get-ip-hash.js'; export default async (ctx: Koa.Context) => { ctx.set('Access-Control-Allow-Origin', config.url); @@ -27,7 +28,7 @@ export default async (ctx: Koa.Context) => { try { // not more than 1 attempt per second and not more than 10 attempts per hour - await limiter({ key: 'signin', duration: 60 * 60 * 1000, max: 10, minInterval: 1000 }, ctx.ip); + await limiter({ key: 'signin', duration: 60 * 60 * 1000, max: 10, minInterval: 1000 }, getIpHash(ctx.ip)); } catch (err) { ctx.status = 429; ctx.body = { -- cgit v1.2.3-freya