Use X-Forwarded-For header on trusted proxies

If the controller is behind a proxy then the socket.remoteAddress will
be that of the proxy instead of the end client that connected.  In this
case the X-Forwarded-For header will contain the address of the end
client, but this should only be relied upon if the party connecting is a
trusted proxy, otherwise a malicious user could spoof the IP address
that is logged by providing a forged X-Forwarded-For header.

Implement a trusted proxy list consisting of a configured list of IP
addresses and/or CIDR blocks and use the last address[1] in the
X-Forwarded-For header if the remoteAddress address of the connecting
party is in the trusted proxy list.

[1]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For

packages/controller/index.ts

@@ -5,6 +5,7 @@ export { default as ControlConnection } from "./src/ControlConnection";
 export { default as InstanceInfo } from "./src/InstanceInfo";
 export { default as BaseControllerPlugin } from "./src/BaseControllerPlugin";
 export { default as UserManager } from "./src/UserManager";
+export { default as WsServer } from "./src/WsServer";
 
 if (module === require.main) {
 	bootstrap();

packages/controller/src/Controller.ts

@@ -1,5 +1,5 @@
 import type winston from "winston";
-import type { AddressInfo } from "net";
+import { BlockList, type AddressInfo, isIPv4, isIPv6 } from "net";
 import type { ControllerArgs } from "../controller";
 import express, { type Request, type Response, type NextFunction, type Application } from "express";
 import type { Static } from "@sinclair/typebox";
@@ -74,6 +74,7 @@ export default class Controller {
 
 	/** WebSocket server */
 	wsServer: WsServer;
+	trustedProxies: BlockList;
 	debugEvents: events.EventEmitter = new events.EventEmitter();
 	private _events: events.EventEmitter = new events.EventEmitter();
 
@@ -153,6 +154,7 @@ export default class Controller {
 		this.app.locals.controller = this;
 		this.app.locals.streams = new Map();
 
+		this.trustedProxies = this.parseTrustedProxies();
 		this.wsServer = new WsServer(this);
 
 		this.modStore.on("change", mod => {
@@ -228,6 +230,8 @@ export default class Controller {
 				this.onAutosaveIntervalChanged();
 			} else if (field === "controller.system_metrics_interval") {
 				this.onSystemMetricsIntervalChanged();
+			} else if (field === "controller.trusted_proxies") {
+				this.trustedProxies = this.parseTrustedProxies();
 			}
 			lib.invokeHook(this.plugins, "onControllerConfigFieldChanged", field, curr, prev);
 		});
@@ -404,6 +408,34 @@ export default class Controller {
 		logger.info("Goodbye");
 	}
 
+	parseTrustedProxies() {
+		const trustedProxies = new BlockList();
+		const proxiesString = this.config.get("controller.trusted_proxies");
+		if (proxiesString) {
+			const proxies = proxiesString.split(",").map(s => s.trim());
+			for (const proxy of proxies) {
+				const [ip, prefix] = proxy.split("/");
+				// eslint-disable-next-line no-nested-ternary
+				const type = isIPv4(ip) ? "ipv4" : isIPv6(ip) ? "ipv6" : undefined;
+				if (!type) {
+					logger.error(`Invalid proxy '${proxy}': not an IP address`);
+					continue;
+				}
+				try {
+					if (prefix) {
+						trustedProxies.addSubnet(ip, Number.parseInt(prefix, 10), type);
+					} else {
+						trustedProxies.addAddress(ip, type);
+					}
+				} catch (err: any) {
+					logger.error(`Invalid proxy '${proxy}': ${err.message}`);
+				}
+			}
+		}
+
+		return trustedProxies;
+	}
+
 	onSystemMetricsIntervalChanged() {
 		if (this.systemMetricsInterval) {
 			clearInterval(this.systemMetricsInterval);

packages/controller/src/WsServer.ts

@@ -59,6 +59,20 @@ export default class WsServer {
 		);
 	}
 
+	remoteAddr(req: IncomingMessage) {
+		const remote = req.socket.remoteAddress ?? "";
+		if (
+			remote && req.headers["x-forwarded-for"]
+			&& (
+				(req.socket.remoteFamily === "IPv4" && this.controller.trustedProxies.check(remote, "ipv4"))
+				|| (req.socket.remoteFamily === "IPv6" && this.controller.trustedProxies.check(remote, "ipv6"))
+			)
+		) {
+			return (req.headers["x-forwarded-for"] as string).split(",").slice(-1)[0].trim();
+		}
+		return remote;
+	}
+
 	async stop(): Promise<void> {
 		this.stopAcceptingNewSessions = true;
 
@@ -100,7 +114,7 @@ export default class WsServer {
 	}
 
 	handleConnection(socket: lib.WebSocketClusterio, req: IncomingMessage) {
-		logger.verbose(`WsServer | new connection from ${req.socket.remoteAddress}`);
+		logger.verbose(`WsServer | new connection from ${this.remoteAddr(req)}`);
 
 		// Track statistics
 		wsConnectionsCounter.inc();
@@ -136,7 +150,7 @@ export default class WsServer {
 		this.pendingSockets.add(socket);
 
 		let timeoutId = setTimeout(() => {
-			logger.verbose(`WsServer | closing ${req.socket.remoteAddress} after timing out on handshake`);
+			logger.verbose(`WsServer | closing ${this.remoteAddr(req)} after timing out on handshake`);
 			wsRejectedConnectionsCounter.inc();
 			socket.terminate();
 			this.pendingSockets.delete(socket);
@@ -168,7 +182,7 @@ ${err.stack}`
 			lib.Message.validate(json);
 			message = lib.Message.fromJSON(json);
 		} catch (err: any) {
-			logger.verbose(`WsServer | closing ${req.socket.remoteAddress} after receiving invalid message`);
+			logger.verbose(`WsServer | closing ${this.remoteAddr(req)} after receiving invalid message`);
 			wsRejectedConnectionsCounter.inc();
 			socket.close(1002, `Invalid message: ${err.message}`);
 			return;
@@ -180,14 +194,14 @@ ${err.stack}`
 		}
 
 		if (message.type !== "registerHost" && message.type !== "registerControl") {
-			logger.verbose(`WsServer | closing ${req.socket.remoteAddress} after receiving invalid handshake`);
+			logger.verbose(`WsServer | closing ${this.remoteAddr(req)} after receiving invalid handshake`);
 			wsRejectedConnectionsCounter.inc();
 			socket.close(1002, "Bad handshake");
 			return;
 		}
 
 		if (this.stopAcceptingNewSessions) {
-			logger.verbose(`WsServer | closing ${req.socket.remoteAddress}, server is shutting down`);
+			logger.verbose(`WsServer | closing ${this.remoteAddr(req)}, server is shutting down`);
 			wsRejectedConnectionsCounter.inc();
 			socket.close(1001, "Shutting down");
 			return;
@@ -247,7 +261,7 @@ ${err.stack}`
 				throw new Error("invalid token");
 			}
 		} catch (err: any) {
-			logger.verbose(`WsServer | authentication failed for ${req.socket.remoteAddress}`);
+			logger.verbose(`WsServer | authentication failed for ${this.remoteAddr(req)}`);
 			wsRejectedConnectionsCounter.inc();
 			socket.close(4003, `Authentication failed: ${err.message}`);
 			return;
@@ -306,7 +320,7 @@ ${err.stack}`
 			user.checkPermission("core.control.connect");
 
 		} catch (err: any) {
-			logger.verbose(`WsServer | authentication failed for ${req.socket.remoteAddress}`);
+			logger.verbose(`WsServer | authentication failed for ${this.remoteAddr(req)}`);
 			wsRejectedConnectionsCounter.inc();
 			socket.close(4003, `Authentication failed: ${err.message}`);
 			return;
@@ -316,7 +330,7 @@ ${err.stack}`
 		this.nextControlId += 1;
 		let [connector, sessionToken] = this.createSession(new lib.Address(lib.Address.control, id));
 
-		logger.verbose(`WsServer | registered control ${id} from ${req.socket.remoteAddress}`);
+		logger.verbose(`WsServer | registered control ${id} from ${this.remoteAddr(req)}`);
 		let connection = new ControlConnection(data, connector, this.controller, user, id);
 		connector.on("close", () => {
 			this.controlConnections.delete(id);

packages/lib/src/config/definitions.ts

@@ -10,6 +10,7 @@ export interface ControllerConfigFields {
 	"controller.http_port": number | null;
 	"controller.https_port": number | null;
 	"controller.bind_address": string | null;
+	"controller.trusted_proxies": string | null;
 	"controller.external_address": string | null;
 	"controller.tls_certificate": string | null;
 	"controller.tls_private_key": string | null;
@@ -78,6 +79,13 @@ export class ControllerConfig extends classes.Config<ControllerConfigFields> {
 			type: "string",
 			optional: true,
 		},
+		"controller.trusted_proxies": {
+			title: "Trusted Proxies",
+			description:
+				"Comma separated list of IP addresses and/or CIDR blocks to trust the X-Forwarded-For header on",
+			type: "string",
+			optional: true,
+		},
 		"controller.external_address": {
 			title: "External Address",
 			description: "Public facing address the controller is hosted on.",

test/controller/Controller.js

@@ -0,0 +1,54 @@
+"use strict";
+const assert = require("assert").strict;
+const { Controller } = require("@clusterio/controller");
+const { ControllerConfig } = require("@clusterio/lib");
+
+describe("controller/src/Controller", function() {
+	describe("class Controller", function() {
+		/** @type {Controller} */
+		let controller;
+		before(function() {
+			controller = new Controller({}, [], "", new ControllerConfig("controller"));
+		});
+		function check(list, ip, present = true) {
+			if (present === false) {
+				assert(!list.check(ip), `${ip} unexpectedly in list`);
+			} else {
+				assert(list.check(ip), `${ip} missing from list`);
+			}
+		}
+		describe(".parseTrustedProxies()", function() {
+			it("should parse addresses", function() {
+				controller.config.set("controller.trusted_proxies", "10.0.0.1");
+				let list = controller.parseTrustedProxies();
+				check(list, "10.0.0.1");
+				check(list, "10.0.0.2", false);
+				controller.config.set("controller.trusted_proxies", "10.0.0.1, 10.0.0.2, 10.1.2.3");
+				list = controller.parseTrustedProxies();
+				check(list, "10.0.0.1");
+				check(list, "10.0.0.2");
+				check(list, "10.0.0.3", false);
+			});
+			it("should ignore invalid entries", function() {
+				controller.config.set("controller.trusted_proxies", "10.0.0.1, invalid, 0.0.0.0/200, 10.0.0.2");
+				const list = controller.parseTrustedProxies();
+				check(list, "10.0.0.1");
+				check(list, "10.0.0.2");
+				check(list, "10.0.0.3", false);
+			});
+			it("should parse CIDR blocks", function() {
+				controller.config.set("controller.trusted_proxies", "10.0.0.1/24, 192.168.0.16/31");
+				const list = controller.parseTrustedProxies();
+				check(list, "10.0.0.0");
+				check(list, "10.0.0.1");
+				check(list, "10.0.0.10");
+				check(list, "10.0.0.255");
+				check(list, "10.0.1.0", false);
+				check(list, "192.168.0.15", false);
+				check(list, "192.168.0.16");
+				check(list, "192.168.0.17");
+				check(list, "192.168.0.18", false);
+			});
+		});
+	});
+});

test/controller/WsServer.js

@@ -0,0 +1,73 @@
+"use strict";
+const assert = require("assert").strict;
+const { Controller, WsServer } = require("@clusterio/controller");
+const { ControllerConfig } = require("@clusterio/lib");
+
+describe("controller/src/WsServer", function() {
+	describe("class WsServer", function() {
+		/** @type {WsServer} */
+		let wsServer;
+		before(function() {
+			let config = new ControllerConfig("controller");
+			config.set("controller.trusted_proxies", "127.0.0.0/8, ::1");
+			let controller = new Controller({}, [], "", config);
+			wsServer = controller.wsServer;
+		});
+		describe(".remoteAddr()", function() {
+			it("should resolve X-Forward-For header for trusted proxies", function() {
+				const ipv4 = wsServer.remoteAddr({
+					socket: {
+						remoteAddress: "127.0.0.1",
+						remoteFamily: "IPv4",
+					},
+					headers: { "x-forwarded-for": "1.2.3.4" },
+				});
+				assert.equal(ipv4, "1.2.3.4");
+				const ipv6 = wsServer.remoteAddr({
+					socket: {
+						remoteAddress: "::1",
+						remoteFamily: "IPv6",
+					},
+					headers: { "x-forwarded-for": "2000::1" },
+				});
+				assert.equal(ipv6, "2000::1");
+			});
+			it("should pick the last address in X-Forward-For header for trusted proxies", function() {
+				const ipv4 = wsServer.remoteAddr({
+					socket: {
+						remoteAddress: "127.0.0.1",
+						remoteFamily: "IPv4",
+					},
+					headers: { "x-forwarded-for": "1.0.0.0, 2.0.0.0, 1.2.3.4" },
+				});
+				assert.equal(ipv4, "1.2.3.4");
+				const ipv6 = wsServer.remoteAddr({
+					socket: {
+						remoteAddress: "::1",
+						remoteFamily: "IPv6",
+					},
+					headers: { "x-forwarded-for": "2001::, 2002::, 2000::1" },
+				});
+				assert.equal(ipv6, "2000::1");
+			});
+			it("should not resolve X-Forward-For header for untrusted proxies", function() {
+				const ipv4 = wsServer.remoteAddr({
+					socket: {
+						remoteAddress: "1.2.3.4",
+						remoteFamily: "IPv4",
+					},
+					headers: { "x-forwarded-for": "127.0.0.1" },
+				});
+				assert.equal(ipv4, "1.2.3.4");
+				const ipv6 = wsServer.remoteAddr({
+					socket: {
+						remoteAddress: "2000::1",
+						remoteFamily: "IPv6",
+					},
+					headers: { "x-forwarded-for": "::1" },
+				});
+				assert.equal(ipv6, "2000::1");
+			});
+		});
+	});
+});