From 69b77531e328ddf73cd65e171d8d98fc3134731c Mon Sep 17 00:00:00 2001 From: dholms Date: Mon, 11 Sep 2023 16:24:25 -0500 Subject: [PATCH 1/2] get rate limit ip correctly --- packages/pds/src/index.ts | 1 + packages/xrpc-server/src/index.ts | 2 +- packages/xrpc-server/src/rate-limiter.ts | 3 +-- packages/xrpc-server/src/util.ts | 4 ---- 4 files changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/pds/src/index.ts b/packages/pds/src/index.ts index 32abb30056d..dc7e36b8d62 100644 --- a/packages/pds/src/index.ts +++ b/packages/pds/src/index.ts @@ -137,6 +137,7 @@ export class PDS { ) const app = express() + app.set('trust proxy', true) app.use(cors()) app.use(loggerMiddleware) app.use(compression()) diff --git a/packages/xrpc-server/src/index.ts b/packages/xrpc-server/src/index.ts index 6bc4147cb44..1458d2ba070 100644 --- a/packages/xrpc-server/src/index.ts +++ b/packages/xrpc-server/src/index.ts @@ -5,4 +5,4 @@ export * from './stream' export * from './rate-limiter' export type { ServerTiming } from './util' -export { getReqIp, serverTimingHeader, ServerTimer } from './util' +export { serverTimingHeader, ServerTimer } from './util' diff --git a/packages/xrpc-server/src/rate-limiter.ts b/packages/xrpc-server/src/rate-limiter.ts index e650ec599ba..55d54cfe726 100644 --- a/packages/xrpc-server/src/rate-limiter.ts +++ b/packages/xrpc-server/src/rate-limiter.ts @@ -14,7 +14,6 @@ import { RateLimiterStatus, XRPCReqContext, } from './types' -import { getReqIp } from './util' export type RateLimiterOpts = { keyPrefix: string @@ -155,5 +154,5 @@ export const getTightestLimit = ( return lowest } -const defaultKey: CalcKeyFn = (ctx: XRPCReqContext) => getReqIp(ctx.req) +const defaultKey: CalcKeyFn = (ctx: XRPCReqContext) => ctx.req.ip const defaultPoints: CalcPointsFn = () => 1 diff --git a/packages/xrpc-server/src/util.ts b/packages/xrpc-server/src/util.ts index 813587382ba..730db950fbd 100644 --- a/packages/xrpc-server/src/util.ts +++ b/packages/xrpc-server/src/util.ts @@ -268,10 +268,6 @@ function decodeBodyStream( return stream } -export const getReqIp = (req: express.Request): string => { - return req.ips.at(-1) ?? req.ip -} - export function serverTimingHeader(timings: ServerTiming[]) { return timings .map((timing) => { From eec0187f29048a234342e3e8813ffb4824e63df6 Mon Sep 17 00:00:00 2001 From: dholms Date: Mon, 11 Sep 2023 17:11:32 -0500 Subject: [PATCH 2/2] add write rate-limits --- .../src/api/com/atproto/repo/applyWrites.ts | 28 +++++++++++++++++++ .../src/api/com/atproto/repo/createRecord.ts | 12 ++++++++ .../src/api/com/atproto/repo/deleteRecord.ts | 12 ++++++++ .../pds/src/api/com/atproto/repo/putRecord.ts | 12 ++++++++ packages/pds/src/index.ts | 14 +++++++++- 5 files changed, 77 insertions(+), 1 deletion(-) diff --git a/packages/pds/src/api/com/atproto/repo/applyWrites.ts b/packages/pds/src/api/com/atproto/repo/applyWrites.ts index 5dc65100855..d5be8bb720d 100644 --- a/packages/pds/src/api/com/atproto/repo/applyWrites.ts +++ b/packages/pds/src/api/com/atproto/repo/applyWrites.ts @@ -3,6 +3,7 @@ import { InvalidRequestError, AuthRequiredError } from '@atproto/xrpc-server' import { prepareCreate, prepareDelete, prepareUpdate } from '../../../../repo' import { Server } from '../../../../lexicon' import { + HandlerInput, isCreate, isUpdate, isDelete, @@ -15,9 +16,36 @@ import { import AppContext from '../../../../context' import { ConcurrentWriteError } from '../../../../services/repo' +const ratelimitPoints = ({ input }: { input: HandlerInput }) => { + let points = 0 + for (const op of input.body.writes) { + if (isCreate(op)) { + points += 3 + } else if (isUpdate(op)) { + points += 2 + } else { + points += 1 + } + } + return points +} + export default function (server: Server, ctx: AppContext) { server.com.atproto.repo.applyWrites({ auth: ctx.accessVerifierCheckTakedown, + rateLimit: [ + { + name: 'repo-write-hour', + calcKey: ({ auth }) => auth.credentials.did, + calcPoints: ratelimitPoints, + }, + { + name: 'repo-write-day', + calcKey: ({ auth }) => auth.credentials.did, + calcPoints: ratelimitPoints, + }, + ], + handler: async ({ input, auth }) => { const tx = input.body const { repo, validate, swapCommit } = tx diff --git a/packages/pds/src/api/com/atproto/repo/createRecord.ts b/packages/pds/src/api/com/atproto/repo/createRecord.ts index 08ff46ecf78..26bc5614785 100644 --- a/packages/pds/src/api/com/atproto/repo/createRecord.ts +++ b/packages/pds/src/api/com/atproto/repo/createRecord.ts @@ -16,6 +16,18 @@ import { ConcurrentWriteError } from '../../../../services/repo' export default function (server: Server, ctx: AppContext) { server.com.atproto.repo.createRecord({ auth: ctx.accessVerifierCheckTakedown, + rateLimit: [ + { + name: 'repo-write-hour', + calcKey: ({ auth }) => auth.credentials.did, + calcPoints: () => 3, + }, + { + name: 'repo-write-day', + calcKey: ({ auth }) => auth.credentials.did, + calcPoints: () => 3, + }, + ], handler: async ({ input, auth }) => { const { repo, collection, rkey, record, swapCommit, validate } = input.body diff --git a/packages/pds/src/api/com/atproto/repo/deleteRecord.ts b/packages/pds/src/api/com/atproto/repo/deleteRecord.ts index 042367e5b65..99f171e0849 100644 --- a/packages/pds/src/api/com/atproto/repo/deleteRecord.ts +++ b/packages/pds/src/api/com/atproto/repo/deleteRecord.ts @@ -9,6 +9,18 @@ import { ConcurrentWriteError } from '../../../../services/repo' export default function (server: Server, ctx: AppContext) { server.com.atproto.repo.deleteRecord({ auth: ctx.accessVerifierCheckTakedown, + rateLimit: [ + { + name: 'repo-write-hour', + calcKey: ({ auth }) => auth.credentials.did, + calcPoints: () => 1, + }, + { + name: 'repo-write-day', + calcKey: ({ auth }) => auth.credentials.did, + calcPoints: () => 1, + }, + ], handler: async ({ input, auth }) => { const { repo, collection, rkey, swapCommit, swapRecord } = input.body const did = await ctx.services.account(ctx.db).getDidForActor(repo) diff --git a/packages/pds/src/api/com/atproto/repo/putRecord.ts b/packages/pds/src/api/com/atproto/repo/putRecord.ts index 311083359f2..8fdcc776bb9 100644 --- a/packages/pds/src/api/com/atproto/repo/putRecord.ts +++ b/packages/pds/src/api/com/atproto/repo/putRecord.ts @@ -16,6 +16,18 @@ import { ConcurrentWriteError } from '../../../../services/repo' export default function (server: Server, ctx: AppContext) { server.com.atproto.repo.putRecord({ auth: ctx.accessVerifierCheckTakedown, + rateLimit: [ + { + name: 'repo-write-hour', + calcKey: ({ auth }) => auth.credentials.did, + calcPoints: () => 2, + }, + { + name: 'repo-write-day', + calcKey: ({ auth }) => auth.credentials.did, + calcPoints: () => 2, + }, + ], handler: async ({ auth, input }) => { const { repo, diff --git a/packages/pds/src/index.ts b/packages/pds/src/index.ts index dc7e36b8d62..d0f8cf98e9c 100644 --- a/packages/pds/src/index.ts +++ b/packages/pds/src/index.ts @@ -20,7 +20,7 @@ import { RateLimiterOpts, Options as XrpcServerOptions, } from '@atproto/xrpc-server' -import { MINUTE } from '@atproto/common' +import { DAY, HOUR, MINUTE } from '@atproto/common' import * as appviewConsumers from './app-view/event-stream/consumers' import inProcessAppView from './app-view/api' import API from './api' @@ -286,6 +286,18 @@ export class PDS { points: 3000, }, ], + shared: [ + { + name: 'repo-write-hour', + durationMs: HOUR, + points: 5000, // creates=3, puts=2, deletes=1 + }, + { + name: 'repo-write-day', + durationMs: DAY, + points: 35000, // creates=3, puts=2, deletes=1 + }, + ], } }