From b09148840f2b8f87b3eeb6ed7c740d40ca8231fe Mon Sep 17 00:00:00 2001 From: Crimekillz Date: Wed, 20 Nov 2024 21:48:48 +0100 Subject: [PATCH] Critical - Upstream: zotan - Don't treat HTTP 429 errors as non-retryable, Apply rate limits to proxyServer and fileServer --- packages/backend/src/misc/fetch.ts | 2 ++ .../backend/src/queue/processors/deliver.ts | 2 +- .../backend/src/queue/processors/inbox.ts | 2 +- .../src/queue/processors/webhook-deliver.ts | 2 +- .../activitypub/kernel/announce/note.ts | 2 +- .../remote/activitypub/kernel/create/note.ts | 2 +- .../src/remote/activitypub/models/note.ts | 2 +- .../src/server/file/send-drive-file.ts | 32 +++++++++++++++++-- .../backend/src/server/proxy/proxy-media.ts | 31 +++++++++++++++++- 9 files changed, 68 insertions(+), 9 deletions(-) diff --git a/packages/backend/src/misc/fetch.ts b/packages/backend/src/misc/fetch.ts index 25dff6533..33674fd93 100644 --- a/packages/backend/src/misc/fetch.ts +++ b/packages/backend/src/misc/fetch.ts @@ -195,6 +195,7 @@ export class StatusError extends Error { public statusCode: number; public statusMessage?: string; public isClientError: boolean; + public isRetryable: boolean; constructor(message: string, statusCode: number, statusMessage?: string) { super(message); @@ -205,5 +206,6 @@ export class StatusError extends Error { typeof this.statusCode === "number" && this.statusCode >= 400 && this.statusCode < 500; + this.isRetryable = this.isClientError && this.statusCode != 429; } } diff --git a/packages/backend/src/queue/processors/deliver.ts b/packages/backend/src/queue/processors/deliver.ts index 65471a559..4a2257de3 100644 --- a/packages/backend/src/queue/processors/deliver.ts +++ b/packages/backend/src/queue/processors/deliver.ts @@ -65,7 +65,7 @@ export default async (job: Bull.Job) => { if (res instanceof StatusError) { // 4xx - if (res.isClientError) { + if (!res.isRetryable) { // HTTPステータスコード4xxはクライアントエラーであり、それはつまり // 何回再送しても成功することはないということなのでエラーにはしないでおく return `${res.statusCode} ${res.statusMessage}`; diff --git a/packages/backend/src/queue/processors/inbox.ts b/packages/backend/src/queue/processors/inbox.ts index 6f3a98698..5f79224af 100644 --- a/packages/backend/src/queue/processors/inbox.ts +++ b/packages/backend/src/queue/processors/inbox.ts @@ -75,7 +75,7 @@ export default async (job: Bull.Job): Promise => { } catch (e) { // Skip if target is 4xx if (e instanceof StatusError) { - if (e.isClientError) { + if (!e.isRetryable) { return `skip: Ignored deleted actors on both ends ${activity.actor} - ${e.statusCode}`; } throw new Error( diff --git a/packages/backend/src/queue/processors/webhook-deliver.ts b/packages/backend/src/queue/processors/webhook-deliver.ts index dd92e345b..f18fdf3df 100644 --- a/packages/backend/src/queue/processors/webhook-deliver.ts +++ b/packages/backend/src/queue/processors/webhook-deliver.ts @@ -52,7 +52,7 @@ export default async (job: Bull.Job) => { if (res instanceof StatusError) { // 4xx - if (res.isClientError) { + if (!res.isRetryable) { return `${res.statusCode} ${res.statusMessage}`; } diff --git a/packages/backend/src/remote/activitypub/kernel/announce/note.ts b/packages/backend/src/remote/activitypub/kernel/announce/note.ts index 6cdaa6166..0f23aa27b 100644 --- a/packages/backend/src/remote/activitypub/kernel/announce/note.ts +++ b/packages/backend/src/remote/activitypub/kernel/announce/note.ts @@ -48,7 +48,7 @@ export default async function ( } catch (e) { // Skip if target is 4xx if (e instanceof StatusError) { - if (e.isClientError) { + if (!e.isRetryable) { logger.warn(`Ignored announce target ${targetUri} - ${e.statusCode}`); return; } diff --git a/packages/backend/src/remote/activitypub/kernel/create/note.ts b/packages/backend/src/remote/activitypub/kernel/create/note.ts index 09c492730..2e743ace8 100644 --- a/packages/backend/src/remote/activitypub/kernel/create/note.ts +++ b/packages/backend/src/remote/activitypub/kernel/create/note.ts @@ -40,7 +40,7 @@ export default async function ( await createNote(note, resolver, silent); return "ok"; } catch (e) { - if (e instanceof StatusError && e.isClientError) { + if (e instanceof StatusError && !e.isRetryable) { return `skip ${e.statusCode}`; } else { throw e; diff --git a/packages/backend/src/remote/activitypub/models/note.ts b/packages/backend/src/remote/activitypub/models/note.ts index e03a97df5..b76fba899 100644 --- a/packages/backend/src/remote/activitypub/models/note.ts +++ b/packages/backend/src/remote/activitypub/models/note.ts @@ -290,7 +290,7 @@ export async function createNote( } catch (e) { return { status: - e instanceof StatusError && e.isClientError + e instanceof StatusError && !e.isRetryable ? "permerror" : "temperror", }; diff --git a/packages/backend/src/server/file/send-drive-file.ts b/packages/backend/src/server/file/send-drive-file.ts index 2482f0ce6..7d412888c 100644 --- a/packages/backend/src/server/file/send-drive-file.ts +++ b/packages/backend/src/server/file/send-drive-file.ts @@ -14,7 +14,11 @@ import { detectType } from "@/misc/get-file-info.js"; import { convertToWebp } from "@/services/drive/image-processor.js"; import { GenerateVideoThumbnail } from "@/services/drive/generate-video-thumbnail.js"; import { StatusError } from "@/misc/fetch.js"; -import { FILE_TYPE_BROWSERSAFE } from "@/const.js"; +import { FILE_TYPE_BROWSERSAFE, MINUTE } from "@/const.js"; + +import { IEndpointMeta } from "@/server/api/endpoints.js"; +import { getIpHash } from "@/misc/get-ip-hash.js"; +import { limiter } from "@/server/api/limiter.js"; const _filename = fileURLToPath(import.meta.url); const _dirname = dirname(_filename); @@ -31,6 +35,30 @@ const commonReadableHandlerGenerator = export default async function (ctx: Koa.Context) { const key = ctx.params.key; + // koa will automatically load the `X-Forwarded-For` header if `proxy: true` is configured in the app. + const limitActor = getIpHash(ctx.ip); + + const limit: IEndpointMeta["limit"] = { + key: `drive-file:${key}`, + duration: MINUTE * 10, + max: 10 + } + + // Rate limit + await limiter( + limit as IEndpointMeta["limit"] & { key: NonNullable }, + limitActor, + ).catch((e) => { + const remainingTime = e.remainingTime + ? `Please try again in ${e.remainingTime}.` + : "Please try again later."; + + ctx.status = 429; + ctx.body = `Rate limit exceeded. ${remainingTime}`; + }); + + if (ctx.status === 429) return; + // Fetch drive file const file = await DriveFiles.createQueryBuilder("file") .where("file.accessKey = :accessKey", { accessKey: key }) @@ -106,7 +134,7 @@ export default async function (ctx: Koa.Context) { } catch (e) { serverLogger.error(`${e}`); - if (e instanceof StatusError && e.isClientError) { + if (e instanceof StatusError && !e.isRetryable) { ctx.status = e.statusCode; ctx.set("Cache-Control", "max-age=86400"); } else { diff --git a/packages/backend/src/server/proxy/proxy-media.ts b/packages/backend/src/server/proxy/proxy-media.ts index 2da2c0045..625c7fcf3 100644 --- a/packages/backend/src/server/proxy/proxy-media.ts +++ b/packages/backend/src/server/proxy/proxy-media.ts @@ -9,9 +9,12 @@ import { createTemp } from "@/misc/create-temp.js"; import { downloadUrl } from "@/misc/download-url.js"; import { detectType } from "@/misc/get-file-info.js"; import { StatusError } from "@/misc/fetch.js"; -import { FILE_TYPE_BROWSERSAFE } from "@/const.js"; +import { FILE_TYPE_BROWSERSAFE, MINUTE } from "@/const.js"; import { serverLogger } from "../index.js"; import { isMimeImage } from "@/misc/is-mime-image.js"; +import { getIpHash } from "@/misc/get-ip-hash.js"; +import { limiter } from "@/server/api/limiter.js"; +import { IEndpointMeta } from "@/server/api/endpoints.js"; export async function proxyMedia(ctx: Koa.Context) { const url = "url" in ctx.query ? ctx.query.url : `https://${ctx.params.url}`; @@ -21,6 +24,32 @@ export async function proxyMedia(ctx: Koa.Context) { return; } + // koa will automatically load the `X-Forwarded-For` header if `proxy: true` is configured in the app. + const limitActor = getIpHash(ctx.ip); + + const parsedUrl = new URL(url); + + const limit: IEndpointMeta["limit"] = { + key: `media-proxy:${parsedUrl.host}:${parsedUrl.pathname}`, + duration: MINUTE * 10, + max: 10 + } + + // Rate limit + await limiter( + limit as IEndpointMeta["limit"] & { key: NonNullable }, + limitActor, + ).catch((e) => { + const remainingTime = e.remainingTime + ? `Please try again in ${e.remainingTime}.` + : "Please try again later."; + + ctx.status = 429; + ctx.body = `Rate limit exceeded. ${remainingTime}`; + }); + + if (ctx.status === 429) return; + const { hostname } = new URL(url); let resolvedIps; try {