diff --git a/src/services/bridge/webhook-server/index.ts b/src/services/bridge/webhook-server/index.ts index 9c17cf861..58ef57f8d 100644 --- a/src/services/bridge/webhook-server/index.ts +++ b/src/services/bridge/webhook-server/index.ts @@ -27,9 +27,9 @@ type RawBodyRequest = express.Request & { rawBody?: string } // `validate: { xForwardedForHeader: false }` on both limiters: without Express // `trust proxy`, express-rate-limit v7 throws ERR_ERL_UNEXPECTED_X_FORWARDED_FOR // on any request carrying X-Forwarded-For (i.e. anything behind an LB), turning -// every webhook into a 500. Skipping that validation degrades an unset trust -// proxy to keying on the LB's socket address (one shared bucket) instead of an -// outage. Set `trust proxy` in the server for per-sender buckets. +// every webhook into a 500. `trust proxy` is set on the app (see below), which +// makes this validation moot; the skip stays so a future trust-proxy +// misconfiguration degrades to one shared bucket instead of an outage. const webhookRateLimit = rateLimitMiddleware({ windowMs: 60_000, limit: 120, @@ -49,6 +49,13 @@ const replayRateLimit = rateLimitMiddleware({ export const startBridgeWebhookServer = () => { const app = express() + // Exactly one XFF-writing hop sits in front of the pod: the nginx ingress + // (the DO load balancer is L4 and does not touch headers). Trusting that one + // hop makes req.ip the real sender — per-sender rate-limit buckets and a + // non-spoofable IP for the replay allowlist — while entries a client forges + // into X-Forwarded-For stay untrusted. + app.set("trust proxy", 1) + // Middleware - MUST capture raw body for signature verification app.use( express.json({ diff --git a/src/services/bridge/webhook-server/routes/replay.ts b/src/services/bridge/webhook-server/routes/replay.ts index 8053f54a7..73e2a4af0 100644 --- a/src/services/bridge/webhook-server/routes/replay.ts +++ b/src/services/bridge/webhook-server/routes/replay.ts @@ -2,7 +2,6 @@ import crypto from "crypto" import { Request, Response } from "express" import ipaddr from "ipaddr.js" -import requestIp from "request-ip" import { BridgeConfig } from "@config" @@ -144,14 +143,16 @@ export const replayIngressMiddleware = ( res: Response, next: () => void, ) => { - // Loopback trust must come from the socket address, never from request-ip: - // request-ip prefers X-Forwarded-For, which any external caller can set to - // "127.0.0.1" and walk through this gate. The allowlist path below still uses - // request-ip so operators behind a trusted LB match on their real IP — it is - // only meaningful when the ingress strips or overwrites client-supplied XFF. + // Loopback trust must come from the socket address, never from a header any + // external caller can set to "127.0.0.1" and walk through this gate. if (isLoopbackIp(req.socket?.remoteAddress)) return next() - const clientIp = requestIp.getClientIp(req) + // req.ip, not request-ip: with `trust proxy` set on the server, Express + // resolves the proxy-appended (rightmost untrusted) X-Forwarded-For entry, + // so operators behind the ingress match on their real IP while forged XFF + // entries stay untrusted. request-ip prefers the LEFTMOST entry, which a + // caller could forge to an allowlisted IP. + const clientIp = req.ip ?? req.socket?.remoteAddress if (!isReplayIpAllowed(clientIp)) { baseLogger.warn({ clientIp, path: req.path }, "Rejected Bridge replay request") diff --git a/src/services/ibex/webhook-server/index.ts b/src/services/ibex/webhook-server/index.ts index 54304d2bc..a5741142d 100644 --- a/src/services/ibex/webhook-server/index.ts +++ b/src/services/ibex/webhook-server/index.ts @@ -8,6 +8,13 @@ import { onPay, onReceive, cryptoReceive } from "./routes" const start = () => { const app = express() + // Exactly one XFF-writing hop sits in front of the pod: the nginx ingress + // (the DO load balancer is L4 and does not touch headers). Trusting that one + // hop makes req.ip the real sender — per-sender rate-limit buckets and a + // non-spoofable IP for the `ibex.webhook.allowedIps` allowlist — while + // entries a client forges into X-Forwarded-For stay untrusted. + app.set("trust proxy", 1) + app.use(express.json()) app.get("/health", (_: Request, resp: Response) => resp.send("Ibex server is running")) diff --git a/src/services/ibex/webhook-server/middleware/validate-ibex-ip.ts b/src/services/ibex/webhook-server/middleware/validate-ibex-ip.ts index a48980adf..64011f291 100644 --- a/src/services/ibex/webhook-server/middleware/validate-ibex-ip.ts +++ b/src/services/ibex/webhook-server/middleware/validate-ibex-ip.ts @@ -1,5 +1,4 @@ import { Request, Response, NextFunction } from "express" -import requestIp from "request-ip" import ipaddr from "ipaddr.js" import { IbexConfig } from "@config" @@ -63,7 +62,11 @@ export const isIpInAllowlist = ( export const validateIbexIp = (req: Request, resp: Response, next: NextFunction) => { if (!ibexWebhookIpAllowlistEnabled) return next() - const clientIp = requestIp.getClientIp(req) + // req.ip, not request-ip: with `trust proxy` set on the server, Express + // resolves the proxy-appended (rightmost untrusted) X-Forwarded-For entry. + // request-ip prefers the LEFTMOST entry, which any caller can forge to an + // allowlisted IP and walk through this gate. + const clientIp = req.ip ?? req.socket?.remoteAddress if (!isIpInAllowlist(clientIp)) { logger.warn( { clientIp, path: req.path }, diff --git a/test/flash/unit/services/bridge/webhook-server/replay.spec.ts b/test/flash/unit/services/bridge/webhook-server/replay.spec.ts index 0800e3d7f..7b4b490f3 100644 --- a/test/flash/unit/services/bridge/webhook-server/replay.spec.ts +++ b/test/flash/unit/services/bridge/webhook-server/replay.spec.ts @@ -12,11 +12,6 @@ jest.mock("@services/logger", () => ({ baseLogger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, })) -jest.mock("request-ip", () => ({ - __esModule: true, - default: { getClientIp: jest.fn() }, -})) - jest.mock("@services/mongoose/bridge-replay-log", () => ({ createBridgeReplay: jest.fn(), })) @@ -45,7 +40,6 @@ import * as ReplayLog from "@services/mongoose/bridge-replay-log" import { depositHandler } from "@services/bridge/webhook-server/routes/deposit" import { kycHandler } from "@services/bridge/webhook-server/routes/kyc" import { transferHandler } from "@services/bridge/webhook-server/routes/transfer" -import requestIp from "request-ip" // ── Helpers ─────────────────────────────────────────────────────────────────── @@ -60,10 +54,14 @@ const makeReq = ( body: Record = {}, headers: Record = {}, socketRemoteAddress?: string, + // Express-resolved client IP (`req.ip`, honoring `trust proxy`); absent on + // a bare socket the way it is when Express itself hasn't populated it. + ip?: string, ) => ({ body, headers, + ip, socket: { remoteAddress: socketRemoteAddress }, }) as unknown as Request @@ -95,7 +93,6 @@ describe("replayIngressMiddleware", () => { }) it("allows loopback replay calls (by socket address) without an explicit allowlist", () => { - ;(requestIp.getClientIp as jest.Mock).mockReturnValue(null) const res = makeRes() const next = jest.fn() @@ -105,38 +102,47 @@ describe("replayIngressMiddleware", () => { expect(res.status as jest.Mock).not.toHaveBeenCalled() }) - it("rejects a spoofed loopback X-Forwarded-For from a public socket", () => { - // request-ip resolves headers like X-Forwarded-For, which the caller - // controls — only the socket address may grant the loopback exemption. - ;(requestIp.getClientIp as jest.Mock).mockReturnValue("127.0.0.1") + it("rejects a loopback req.ip from a public socket", () => { + // req.ip derives from X-Forwarded-For; only the socket address may grant + // the loopback exemption, whatever the proxy chain reports. const res = makeRes() const next = jest.fn() - replayIngressMiddleware(makeReq({}, {}, "198.51.100.9"), res, next) + replayIngressMiddleware(makeReq({}, {}, "198.51.100.9", "127.0.0.1"), res, next) expect(next).not.toHaveBeenCalled() expect(res.status as jest.Mock).toHaveBeenCalledWith(403) }) it("rejects public replay calls when no allowlist matches", () => { - ;(requestIp.getClientIp as jest.Mock).mockReturnValue("198.51.100.9") const res = makeRes() const next = jest.fn() - replayIngressMiddleware(makeReq({}, {}, "198.51.100.9"), res, next) + replayIngressMiddleware(makeReq({}, {}, "198.51.100.9", "198.51.100.9"), res, next) expect(next).not.toHaveBeenCalled() expect(res.status as jest.Mock).toHaveBeenCalledWith(403) }) - it("allows public replay calls from the configured allowlist", () => { + it("allows public replay calls whose req.ip is on the configured allowlist", () => { expect(isReplayIpAllowed("198.51.100.9", "198.51.100.0/24")).toBe(true) - ;(requestIp.getClientIp as jest.Mock).mockReturnValue("198.51.100.9") process.env.BRIDGE_WEBHOOK_REPLAY_ALLOWED_IPS = "198.51.100.0/24" const res = makeRes() const next = jest.fn() - replayIngressMiddleware(makeReq({}, {}, "10.0.0.7"), res, next) + // Socket is the ingress pod; req.ip (trust proxy) is the real operator IP. + replayIngressMiddleware(makeReq({}, {}, "10.0.0.7", "198.51.100.9"), res, next) + + expect(next).toHaveBeenCalledTimes(1) + expect(res.status as jest.Mock).not.toHaveBeenCalled() + }) + + it("falls back to the socket address when req.ip is absent", () => { + process.env.BRIDGE_WEBHOOK_REPLAY_ALLOWED_IPS = "198.51.100.0/24" + const res = makeRes() + const next = jest.fn() + + replayIngressMiddleware(makeReq({}, {}, "198.51.100.9"), res, next) expect(next).toHaveBeenCalledTimes(1) expect(res.status as jest.Mock).not.toHaveBeenCalled()