diff --git a/js/ip_access_control.js b/js/ip_access_control.js index 0fef1a4e..3357d378 100644 --- a/js/ip_access_control.js +++ b/js/ip_access_control.js @@ -33,6 +33,27 @@ function isAllowed (clientIp, whitelist) { } } +/** + * Resolves a client IP for both Express and Socket.IO requests. + * If the direct peer is loopback, trust the first X-Forwarded-For value (local reverse proxy case). + * Otherwise ignore X-Forwarded-For to prevent spoofing. + * @param {object} req - Incoming request object (Express request or Socket.IO handshake request) + * @returns {string} The resolved client IP address + */ +function resolveClientIp (req) { + const directIp = req.socket?.remoteAddress || req.connection?.remoteAddress || req.ip; + const LOOPBACK_WHITELIST = ["127.0.0.1", "::ffff:127.0.0.1", "::1"]; + + if (isAllowed(directIp, LOOPBACK_WHITELIST)) { + const forwardedFor = req.headers?.["x-forwarded-for"]; + if (typeof forwardedFor === "string" && forwardedFor.trim().length > 0) { + return forwardedFor.split(",")[0].trim(); + } + } + + return directIp; +} + /** * Creates an Express middleware for IP whitelisting * @param {string[]} whitelist - Array of allowed IP addresses or CIDR ranges @@ -48,16 +69,41 @@ function ipAccessControl (whitelist) { } return function (req, res, next) { - const clientIp = req.ip || req.socket.remoteAddress; + const clientIp = resolveClientIp(req); if (isAllowed(clientIp, whitelist)) { res.header("Access-Control-Allow-Origin", "*"); next(); } else { - Log.log(`IP ${clientIp} is not allowed to access the mirror`); + Log.warn(`IP ${clientIp} is not allowed to access the mirror`); res.status(403).send("This device is not allowed to access your mirror.
Please check your config.js or config.js.sample to change this."); } }; } -module.exports = { ipAccessControl }; +/** + * Creates a Socket.IO `allowRequest` handler that enforces the same IP whitelist as the HTTP middleware. + * This closes the gap where Socket.IO handshakes bypassed the Express-only `ipAccessControl` middleware. + * @param {string[]} whitelist - Array of allowed IP addresses or CIDR ranges + * @returns {(req: object, callback: (err: string | null, success: boolean) => void) => void} Socket.IO allowRequest handler + */ +function socketIpAccessControl (whitelist) { + // Empty whitelist means allow all + if (!Array.isArray(whitelist) || whitelist.length === 0) { + return function (req, callback) { + callback(null, true); // allow the connection + }; + } + + return function (req, callback) { + const clientIp = resolveClientIp(req); + if (isAllowed(clientIp, whitelist)) { + callback(null, true); // allow the connection + } else { + Log.warn(`IP ${clientIp} is not allowed to connect to the mirror socket`); + callback("This device is not allowed to access your mirror.", false); + } + }; +} + +module.exports = { ipAccessControl, socketIpAccessControl }; diff --git a/js/server.js b/js/server.js index f8a51e71..9aa264c0 100644 --- a/js/server.js +++ b/js/server.js @@ -7,7 +7,7 @@ const helmet = require("helmet"); const socketio = require("socket.io"); const Log = require("logger"); -const { ipAccessControl } = require("./ip_access_control"); +const { ipAccessControl, socketIpAccessControl } = require("./ip_access_control"); const vendor = require("./vendor"); @@ -41,6 +41,7 @@ function Server (configObj) { server = http.Server(app); } const io = socketio(server, { + allowRequest: socketIpAccessControl(config.ipWhitelist), cors: { origin: /.*$/, credentials: true diff --git a/tests/e2e/ipWhitelist_spec.js b/tests/e2e/ipWhitelist_spec.js index e83809c6..3a7c9e42 100644 --- a/tests/e2e/ipWhitelist_spec.js +++ b/tests/e2e/ipWhitelist_spec.js @@ -15,6 +15,12 @@ describe("ipWhitelist directive configuration", () => { const res = await fetch(`http://localhost:${port}`); expect(res.status).toBe(403); }); + + it("should also reject Socket.IO handshake with 403 (Forbidden) — not just HTTP routes", async () => { + const port = global.testPort || 8080; + const res = await fetch(`http://localhost:${port}/socket.io/?EIO=4&transport=polling`); + expect(res.status).toBe(403); + }); }); describe("When whitelist is empty (allow all IPs)", () => { @@ -31,5 +37,11 @@ describe("ipWhitelist directive configuration", () => { const res = await fetch(`http://localhost:${port}`); expect(res.status).toBe(200); }); + + it("should also allow Socket.IO handshake with 200 (OK) — not just HTTP routes", async () => { + const port = global.testPort || 8080; + const res = await fetch(`http://localhost:${port}/socket.io/?EIO=4&transport=polling`); + expect(res.status).toBe(200); + }); }); }); diff --git a/tests/unit/functions/ip_access_control_spec.js b/tests/unit/functions/ip_access_control_spec.js new file mode 100644 index 00000000..fa6e1a62 --- /dev/null +++ b/tests/unit/functions/ip_access_control_spec.js @@ -0,0 +1,79 @@ +import { describe, expect, it, vi } from "vitest"; + +import { ipAccessControl, socketIpAccessControl } from "../../../js/ip_access_control"; + +/** + * Creates a minimal Express-like response mock used by the middleware tests. + * @returns {{ header: ReturnType, status: ReturnType, send: ReturnType }} Mock response object. + */ +function createResponseMock () { + return { + header: vi.fn(), + status: vi.fn(function () { + return this; + }), + send: vi.fn() + }; +} + +describe("ip_access_control", () => { + describe("ipAccessControl", () => { + it("trusts first X-Forwarded-For entry when direct peer is loopback", () => { + const middleware = ipAccessControl(["203.0.113.10"]); + const req = { + socket: { remoteAddress: "127.0.0.1" }, + headers: { "x-forwarded-for": "203.0.113.10, 10.0.0.2" } + }; + const res = createResponseMock(); + const next = vi.fn(); + + middleware(req, res, next); + + expect(next).toHaveBeenCalledOnce(); + expect(res.status).not.toHaveBeenCalled(); + }); + + it("ignores X-Forwarded-For when direct peer is not loopback", () => { + const middleware = ipAccessControl(["203.0.113.10"]); + const req = { + socket: { remoteAddress: "198.51.100.7" }, + headers: { "x-forwarded-for": "203.0.113.10" } + }; + const res = createResponseMock(); + const next = vi.fn(); + + middleware(req, res, next); + + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(403); + }); + }); + + describe("socketIpAccessControl", () => { + it("accepts socket handshake using forwarded client IP when direct peer is loopback", () => { + const allowRequest = socketIpAccessControl(["203.0.113.10"]); + const req = { + socket: { remoteAddress: "::1" }, + headers: { "x-forwarded-for": "203.0.113.10, 10.0.0.2" } + }; + const callback = vi.fn(); + + allowRequest(req, callback); + + expect(callback).toHaveBeenCalledWith(null, true); + }); + + it("rejects socket handshake when only forwarded IP matches whitelist", () => { + const allowRequest = socketIpAccessControl(["203.0.113.10"]); + const req = { + socket: { remoteAddress: "198.51.100.7" }, + headers: { "x-forwarded-for": "203.0.113.10" } + }; + const callback = vi.fn(); + + allowRequest(req, callback); + + expect(callback).toHaveBeenCalledWith("This device is not allowed to access your mirror.", false); + }); + }); +});