Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/services/bridge/webhook-server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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({
Expand Down
15 changes: 8 additions & 7 deletions src/services/bridge/webhook-server/routes/replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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")
Expand Down
7 changes: 7 additions & 0 deletions src/services/ibex/webhook-server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Request, Response, NextFunction } from "express"
import requestIp from "request-ip"
import ipaddr from "ipaddr.js"

import { IbexConfig } from "@config"
Expand Down Expand Up @@ -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 },
Expand Down
40 changes: 23 additions & 17 deletions test/flash/unit/services/bridge/webhook-server/replay.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}))
Expand Down Expand Up @@ -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 ───────────────────────────────────────────────────────────────────

Expand All @@ -60,10 +54,14 @@ const makeReq = (
body: Record<string, unknown> = {},
headers: Record<string, string> = {},
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

Expand Down Expand Up @@ -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()

Expand All @@ -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()
Expand Down
Loading