From 161659de5cd7633161b0788799b641ff6b7e55f9 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Sat, 28 May 2022 05:06:47 +0200 Subject: [PATCH] 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 --- CHANGELOG.md | 2 + locales/ja-JP.yml | 1 + packages/backend/src/server/api/call.ts | 49 +++++++++++++------ packages/backend/src/server/api/endpoints.ts | 1 - packages/backend/src/server/api/limiter.ts | 26 ++++------ .../backend/src/server/api/private/signin.ts | 41 ++++++++-------- packages/client/src/components/signin.vue | 12 +++-- 7 files changed, 75 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6aadc0b2ae..2a463c3a0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ 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 + Admins should make sure the reverse proxy sets the `X-Forwarded-For` header to the original address. ### Bugfixes - Client: fix settings page @tamaina diff --git a/locales/ja-JP.yml b/locales/ja-JP.yml index f64246d155..6354fcfda1 100644 --- a/locales/ja-JP.yml +++ b/locales/ja-JP.yml @@ -842,6 +842,7 @@ oneDay: "1日" oneWeek: "1週間" reflectMayTakeTime: "反映されるまで時間がかかる場合があります。" failedToFetchAccountInformation: "アカウント情報の取得に失敗しました" +rateLimitExceeded: "レート制限を超えました" _emailUnavailable: used: "既に使用されています" 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)) { diff --git a/packages/backend/src/server/api/endpoints.ts b/packages/backend/src/server/api/endpoints.ts index e2db03f13a..1e7afd8cdd 100644 --- a/packages/backend/src/server/api/endpoints.ts +++ b/packages/backend/src/server/api/endpoints.ts @@ -654,7 +654,6 @@ export interface IEndpointMeta { /** * エンドポイントのリミテーションに関するやつ * 省略した場合はリミテーションは無いものとして解釈されます。 - * また、withCredential が false の場合はリミテーションを行うことはできません。 */ readonly limit?: { diff --git a/packages/backend/src/server/api/limiter.ts b/packages/backend/src/server/api/limiter.ts index e74db8466e..23430cf8b6 100644 --- a/packages/backend/src/server/api/limiter.ts +++ b/packages/backend/src/server/api/limiter.ts @@ -1,25 +1,17 @@ import Limiter from 'ratelimiter'; import { redisClient } from '../../db/redis.js'; -import { IEndpoint } from './endpoints.js'; -import * as Acct from '@/misc/acct.js'; +import { IEndpointMeta } from './endpoints.js'; import { CacheableLocalUser, User } from '@/models/entities/user.js'; import Logger from '@/services/logger.js'; const logger = new Logger('limiter'); -export const limiter = (endpoint: IEndpoint & { meta: { limit: NonNullable } }, user: CacheableLocalUser) => new Promise((ok, reject) => { - const limitation = endpoint.meta.limit; - - const key = Object.prototype.hasOwnProperty.call(limitation, 'key') - ? limitation.key - : endpoint.name; - - const hasShortTermLimit = - Object.prototype.hasOwnProperty.call(limitation, 'minInterval'); +export const limiter = (limitation: IEndpointMeta['limit'] & { key: NonNullable }, actor: string) => new Promise((ok, reject) => { + const hasShortTermLimit = typeof limitation.minInterval === 'number'; const hasLongTermLimit = - Object.prototype.hasOwnProperty.call(limitation, 'duration') && - Object.prototype.hasOwnProperty.call(limitation, 'max'); + typeof limitation.duration === 'number' && + typeof limitation.max === 'number'; if (hasShortTermLimit) { min(); @@ -32,7 +24,7 @@ export const limiter = (endpoint: IEndpoint & { meta: { limit: NonNullable { ctx.set('Access-Control-Allow-Origin', config.url); ctx.set('Access-Control-Allow-Credentials', 'true'); const body = ctx.request.body as any; - - const instance = await fetchMeta(true); - const username = body['username']; const password = body['password']; const token = body['token']; @@ -29,6 +25,21 @@ export default async (ctx: Koa.Context) => { ctx.body = { error }; } + 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); + } catch (err) { + ctx.status = 429; + ctx.body = { + error: { + message: 'Too many failed attempts to sign in. Try again later.', + code: 'TOO_MANY_AUTHENTICATION_FAILURES', + id: '22d05606-fbcf-421a-a2db-b32610dcfd1b', + }, + }; + return; + } + if (typeof username !== 'string') { ctx.status = 400; return; @@ -84,18 +95,6 @@ export default async (ctx: Koa.Context) => { } if (!profile.twoFactorEnabled) { - if (instance.enableHcaptcha && instance.hcaptchaSecretKey) { - await verifyHcaptcha(instance.hcaptchaSecretKey, body['hcaptcha-response']).catch(e => { - ctx.throw(400, e); - }); - } - - if (instance.enableRecaptcha && instance.recaptchaSecretKey) { - await verifyRecaptcha(instance.recaptchaSecretKey, body['g-recaptcha-response']).catch(e => { - ctx.throw(400, e); - }); - } - if (same) { signin(ctx, user); return; @@ -172,7 +171,7 @@ export default async (ctx: Koa.Context) => { body.credentialId .replace(/-/g, '+') .replace(/_/g, '/'), - 'base64', + 'base64' ).toString('hex'), }); diff --git a/packages/client/src/components/signin.vue b/packages/client/src/components/signin.vue index d283a758a6..be87274020 100644 --- a/packages/client/src/components/signin.vue +++ b/packages/client/src/components/signin.vue @@ -14,8 +14,6 @@ - - {{ signing ? i18n.ts.loggingIn : i18n.ts.login }}