summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJohann150 <johann.galle@protonmail.com>2022-05-28 05:06:47 +0200
committerGitHub <noreply@github.com>2022-05-28 12:06:47 +0900
commit161659de5cd7633161b0788799b641ff6b7e55f9 (patch)
tree8dff8d5a7ae31a20d38e32ca6dcaa1b34eb95850
parentenhance: clearly link documentation (diff)
downloadmisskey-161659de5cd7633161b0788799b641ff6b7e55f9.tar.gz
misskey-161659de5cd7633161b0788799b641ff6b7e55f9.tar.bz2
misskey-161659de5cd7633161b0788799b641ff6b7e55f9.zip
enhance: replace signin CAPTCHA with rate limit (#8740)
* 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
-rw-r--r--CHANGELOG.md2
-rw-r--r--locales/ja-JP.yml1
-rw-r--r--packages/backend/src/server/api/call.ts49
-rw-r--r--packages/backend/src/server/api/endpoints.ts1
-rw-r--r--packages/backend/src/server/api/limiter.ts26
-rw-r--r--packages/backend/src/server/api/private/signin.ts41
-rw-r--r--packages/client/src/components/signin.vue12
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<string> }, 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<IEndpoint['meta']['limit']> } }, 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<IEndpoint['meta']['limit']> } }, user: CacheableLocalUser) => new Promise<void>((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<string> }, actor: string) => new Promise<void>((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<IEndp
// Short-term limit
function min(): void {
const minIntervalLimiter = new Limiter({
- id: `${user.id}:${key}:min`,
+ id: `${actor}:${limitation.key}:min`,
duration: limitation.minInterval,
max: 1,
db: redisClient,
@@ -43,7 +35,7 @@ export const limiter = (endpoint: IEndpoint & { meta: { limit: NonNullable<IEndp
return reject('ERR');
}
- logger.debug(`@${Acct.toString(user)} ${endpoint.name} min remaining: ${info.remaining}`);
+ logger.debug(`${actor} ${limitation.key} min remaining: ${info.remaining}`);
if (info.remaining === 0) {
reject('BRIEF_REQUEST_INTERVAL');
@@ -60,7 +52,7 @@ export const limiter = (endpoint: IEndpoint & { meta: { limit: NonNullable<IEndp
// Long term limit
function max(): void {
const limiter = new Limiter({
- id: `${user.id}:${key}`,
+ id: `${actor}:${limitation.key}`,
duration: limitation.duration,
max: limitation.max,
db: redisClient,
@@ -71,7 +63,7 @@ export const limiter = (endpoint: IEndpoint & { meta: { limit: NonNullable<IEndp
return reject('ERR');
}
- logger.debug(`@${Acct.toString(user)} ${endpoint.name} max remaining: ${info.remaining}`);
+ logger.debug(`${actor} ${limitation.key} max remaining: ${info.remaining}`);
if (info.remaining === 0) {
reject('RATE_LIMIT_EXCEEDED');
diff --git a/packages/backend/src/server/api/private/signin.ts b/packages/backend/src/server/api/private/signin.ts
index 0024b8ce3e..b304550e29 100644
--- a/packages/backend/src/server/api/private/signin.ts
+++ b/packages/backend/src/server/api/private/signin.ts
@@ -1,25 +1,21 @@
-import { randomBytes } from 'node:crypto';
import Koa from 'koa';
import bcrypt from 'bcryptjs';
import * as speakeasy from 'speakeasy';
-import { IsNull } from 'typeorm';
+import signin from '../common/signin.js';
import config from '@/config/index.js';
import { Users, Signins, UserProfiles, UserSecurityKeys, AttestationChallenges } from '@/models/index.js';
import { ILocalUser } from '@/models/entities/user.js';
import { genId } from '@/misc/gen-id.js';
-import { fetchMeta } from '@/misc/fetch-meta.js';
-import { verifyHcaptcha, verifyRecaptcha } from '@/misc/captcha.js';
import { verifyLogin, hash } from '../2fa.js';
-import signin from '../common/signin.js';
+import { randomBytes } from 'node:crypto';
+import { IsNull } from 'typeorm';
+import { limiter } from '../limiter.js';
export default async (ctx: Koa.Context) => {
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 @@
<template #prefix><i class="fas fa-lock"></i></template>
<template #caption><button class="_textButton" type="button" @click="resetPassword">{{ i18n.ts.forgotPassword }}</button></template>
</MkInput>
- <MkCaptcha v-if="meta.enableHcaptcha" ref="hcaptcha" v-model="hCaptchaResponse" class="_formBlock captcha" provider="hcaptcha" :sitekey="meta.hcaptchaSiteKey"/>
- <MkCaptcha v-if="meta.enableRecaptcha" ref="recaptcha" v-model="reCaptchaResponse" class="_formBlock captcha" provider="recaptcha" :sitekey="meta.recaptchaSiteKey"/>
<MkButton class="_formBlock" type="submit" primary :disabled="signing" style="margin: 0 auto;">{{ signing ? i18n.ts.loggingIn : i18n.ts.login }}</MkButton>
</div>
<div v-if="totpLogin" class="2fa-signin" :class="{ securityKeys: user && user.securityKeys }">
@@ -64,8 +62,6 @@ import { showSuspendedDialog } from '../scripts/show-suspended-dialog';
import { instance } from '@/instance';
import { i18n } from '@/i18n';
-const MkCaptcha = defineAsyncComponent(() => import('./captcha.vue'));
-
let signing = $ref(false);
let user = $ref(null);
let username = $ref('');
@@ -217,6 +213,14 @@ function loginFailed(err) {
showSuspendedDialog();
break;
}
+ case '22d05606-fbcf-421a-a2db-b32610dcfd1b': {
+ os.alert({
+ type: 'error',
+ title: i18n.ts.loginFailed,
+ text: i18n.ts.rateLimitExceeded,
+ });
+ break;
+ }
default: {
console.log(err)
os.alert({