Sicherheit: Allowlist anhand des echten Socket-Peers statt X-Forwarded-For
Die Allowlist-Pruefung nutzte extractDebugClientIp, das X-Forwarded-For zuerst auswertet - ein angreiferkontrollierter Header. Damit konnte ein entfernter Client auf einem 0.0.0.0-Bind die Allowlist komplett umgehen (X-Forwarded-For: 127.0.0.1 -> als Loopback gewertet). Die Allowlist bot so keinerlei Schutz; nur das Token blieb. Fix: Enforcement liest jetzt ausschliesslich req.socket.remoteAddress (getPeerIp), das vom Kernel aus der TCP-Verbindung gesetzt wird und nicht per Header faelschbar ist. X-Forwarded-For wird nur noch fuer das Trace-Log (Beobachtbarkeit) verwendet, nie fuer die Zugriffsentscheidung. Reine, exportierte Matcher-Funktion evaluateClientAllowed. Tests umgebaut auf den Threat-Model statt den faelschbaren Kanal: Unit-Tests fuer Loopback/exakt/CIDR/fail-closed + expliziter Forged-XFF-Test (socket.remoteAddress 8.8.8.8 + X-Forwarded-For 127.0.0.1 -> verweigert). Integration: Loopback verbindet, gespooftes X-Forwarded-For wird ignoriert, Auth bleibt erzwungen, Live-Reload.
This commit is contained in:
parent
c8beaf96ed
commit
95d284f687
@ -219,15 +219,23 @@ function matchIpRule(clientIp: string, rule: string): boolean {
|
||||
return false;
|
||||
}
|
||||
|
||||
function isClientAllowed(clientIp: string): boolean {
|
||||
export function evaluateClientAllowed(clientIp: string, rules: string[]): boolean {
|
||||
const client = normalizeIp(clientIp);
|
||||
if (isLoopbackIp(client) || client === "") {
|
||||
return true;
|
||||
}
|
||||
if (allowlist.length === 0) {
|
||||
if (rules.length === 0) {
|
||||
return false;
|
||||
}
|
||||
return allowlist.some((rule) => matchIpRule(client, rule));
|
||||
return rules.some((rule) => matchIpRule(client, rule));
|
||||
}
|
||||
|
||||
export function getPeerIp(req: http.IncomingMessage): string {
|
||||
return normalizeIp(req.socket?.remoteAddress || "");
|
||||
}
|
||||
|
||||
function isClientAllowed(clientIp: string): boolean {
|
||||
return evaluateClientAllowed(clientIp, allowlist);
|
||||
}
|
||||
|
||||
function checkAuth(req: http.IncomingMessage): boolean {
|
||||
@ -551,15 +559,16 @@ function handleRequest(req: http.IncomingMessage, res: http.ServerResponse): voi
|
||||
return;
|
||||
}
|
||||
|
||||
const clientIp = extractDebugClientIp(req);
|
||||
if (!isClientAllowed(clientIp)) {
|
||||
const peerIp = getPeerIp(req);
|
||||
if (!isClientAllowed(peerIp)) {
|
||||
if (traceConfig.enabled && traceConfig.logDebugRequests) {
|
||||
logTraceEvent("WARN", "debug-http", "Durch Allowlist blockiert", {
|
||||
clientIp,
|
||||
peerIp,
|
||||
forwardedFor: extractDebugClientIp(req),
|
||||
url: sanitizeRequestUrlForTrace(req.url || "/")
|
||||
});
|
||||
}
|
||||
jsonResponse(res, 403, { error: "Forbidden", reason: "Client-IP nicht in Allowlist", clientIp });
|
||||
jsonResponse(res, 403, { error: "Forbidden", reason: "Client-IP nicht in Allowlist", clientIp: peerIp });
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
@ -10,7 +10,9 @@ import {
|
||||
stopDebugServer,
|
||||
restartDebugServer,
|
||||
writeDebugServerConfig,
|
||||
getDebugServerRuntimeStatus
|
||||
getDebugServerRuntimeStatus,
|
||||
evaluateClientAllowed,
|
||||
getPeerIp
|
||||
} from "../src/main/debug-server";
|
||||
import type { DownloadManager } from "../src/main/download-manager";
|
||||
|
||||
@ -45,13 +47,13 @@ async function waitForReady(url: string): Promise<void> {
|
||||
throw new Error(`debug server not ready: ${url}`);
|
||||
}
|
||||
|
||||
async function startWithAllowlist(allowlist: string[]): Promise<{ baseUrl: string }> {
|
||||
async function startWithAllowlist(allowlist: string[], host = "0.0.0.0"): Promise<{ baseUrl: string }> {
|
||||
const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), "rd-allow-"));
|
||||
tempDirs.push(baseDir);
|
||||
const port = await getFreePort();
|
||||
fs.writeFileSync(path.join(baseDir, "debug_token.txt"), TOKEN, "utf8");
|
||||
fs.writeFileSync(path.join(baseDir, "debug_port.txt"), String(port), "utf8");
|
||||
fs.writeFileSync(path.join(baseDir, "debug_host.txt"), "0.0.0.0", "utf8");
|
||||
fs.writeFileSync(path.join(baseDir, "debug_host.txt"), host, "utf8");
|
||||
fs.writeFileSync(path.join(baseDir, "debug_allowlist.txt"), allowlist.join("\n"), "utf8");
|
||||
const manager = {} as unknown as DownloadManager;
|
||||
startDebugServer(manager, baseDir);
|
||||
@ -60,14 +62,6 @@ async function startWithAllowlist(allowlist: string[]): Promise<{ baseUrl: strin
|
||||
return { baseUrl };
|
||||
}
|
||||
|
||||
function getAs(baseUrl: string, clientIp: string | null): Promise<Response> {
|
||||
const headers: Record<string, string> = {};
|
||||
if (clientIp) {
|
||||
headers["X-Forwarded-For"] = clientIp;
|
||||
}
|
||||
return fetch(`${baseUrl}/health?token=${TOKEN}`, { headers });
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
stopDebugServer();
|
||||
while (tempDirs.length > 0) {
|
||||
@ -82,58 +76,71 @@ afterEach(() => {
|
||||
}
|
||||
});
|
||||
|
||||
describe("debug-server allowlist enforcement", () => {
|
||||
it("always allows loopback regardless of allowlist", async () => {
|
||||
describe("debug-server allowlist matcher (pure)", () => {
|
||||
it("always allows loopback regardless of rules", () => {
|
||||
expect(evaluateClientAllowed("127.0.0.1", [])).toBe(true);
|
||||
expect(evaluateClientAllowed("::1", [])).toBe(true);
|
||||
expect(evaluateClientAllowed("::ffff:127.0.0.1", ["8.8.8.8"])).toBe(true);
|
||||
});
|
||||
|
||||
it("matches an exact allowlisted IP and rejects others", () => {
|
||||
expect(evaluateClientAllowed("8.8.8.8", ["8.8.8.8"])).toBe(true);
|
||||
expect(evaluateClientAllowed("9.9.9.9", ["8.8.8.8"])).toBe(false);
|
||||
});
|
||||
|
||||
it("matches inside a CIDR and rejects outside it", () => {
|
||||
expect(evaluateClientAllowed("10.0.0.42", ["10.0.0.0/24"])).toBe(true);
|
||||
expect(evaluateClientAllowed("10.0.1.42", ["10.0.0.0/24"])).toBe(false);
|
||||
});
|
||||
|
||||
it("fail-closed: empty rules reject every non-loopback client", () => {
|
||||
expect(evaluateClientAllowed("203.0.113.7", [])).toBe(false);
|
||||
expect(evaluateClientAllowed("8.8.8.8", [])).toBe(false);
|
||||
});
|
||||
|
||||
it("derives the client IP from the socket peer, never from X-Forwarded-For", () => {
|
||||
const forgedLoopback = {
|
||||
socket: { remoteAddress: "8.8.8.8" },
|
||||
headers: { "x-forwarded-for": "127.0.0.1" }
|
||||
} as unknown as http.IncomingMessage;
|
||||
expect(getPeerIp(forgedLoopback)).toBe("8.8.8.8");
|
||||
expect(evaluateClientAllowed(getPeerIp(forgedLoopback), [])).toBe(false);
|
||||
expect(evaluateClientAllowed(getPeerIp(forgedLoopback), ["9.9.9.9"])).toBe(false);
|
||||
expect(evaluateClientAllowed(getPeerIp(forgedLoopback), ["8.8.8.8"])).toBe(true);
|
||||
|
||||
const ipv6Mapped = {
|
||||
socket: { remoteAddress: "::ffff:10.0.0.5" },
|
||||
headers: {}
|
||||
} as unknown as http.IncomingMessage;
|
||||
expect(getPeerIp(ipv6Mapped)).toBe("10.0.0.5");
|
||||
});
|
||||
});
|
||||
|
||||
describe("debug-server allowlist enforcement (wired)", () => {
|
||||
it("allows a loopback connection and ignores a spoofed X-Forwarded-For", async () => {
|
||||
const { baseUrl } = await startWithAllowlist(["8.8.8.8"]);
|
||||
const res = await getAs(baseUrl, null);
|
||||
expect(res.status).toBe(200);
|
||||
const plain = await fetch(`${baseUrl}/health?token=${TOKEN}`);
|
||||
expect(plain.status).toBe(200);
|
||||
const spoofed = await fetch(`${baseUrl}/health?token=${TOKEN}`, {
|
||||
headers: { "X-Forwarded-For": "203.0.113.9" }
|
||||
});
|
||||
expect(spoofed.status).toBe(200);
|
||||
});
|
||||
|
||||
it("allows an exact allowlisted client IP", async () => {
|
||||
it("still enforces the token for loopback clients", async () => {
|
||||
const { baseUrl } = await startWithAllowlist(["8.8.8.8"]);
|
||||
const res = await getAs(baseUrl, "8.8.8.8");
|
||||
expect(res.status).toBe(200);
|
||||
});
|
||||
|
||||
it("allows a client inside an allowlisted CIDR and blocks outside it", async () => {
|
||||
const { baseUrl } = await startWithAllowlist(["10.0.0.0/24"]);
|
||||
const inside = await getAs(baseUrl, "10.0.0.42");
|
||||
expect(inside.status).toBe(200);
|
||||
const outside = await getAs(baseUrl, "10.0.1.42");
|
||||
expect(outside.status).toBe(403);
|
||||
});
|
||||
|
||||
it("blocks a non-allowlisted remote client with 403 before auth", async () => {
|
||||
const { baseUrl } = await startWithAllowlist(["8.8.8.8"]);
|
||||
const res = await fetch(`${baseUrl}/health`, { headers: { "X-Forwarded-For": "9.9.9.9" } });
|
||||
expect(res.status).toBe(403);
|
||||
const body = await res.json() as Record<string, unknown>;
|
||||
expect(body.reason).toContain("Allowlist");
|
||||
});
|
||||
|
||||
it("fail-closed: empty allowlist on 0.0.0.0 blocks every non-loopback client", async () => {
|
||||
const { baseUrl } = await startWithAllowlist([]);
|
||||
expect(getDebugServerRuntimeStatus().allowlistCount).toBe(0);
|
||||
const loopback = await getAs(baseUrl, null);
|
||||
expect(loopback.status).toBe(200);
|
||||
const remote = await getAs(baseUrl, "203.0.113.7");
|
||||
expect(remote.status).toBe(403);
|
||||
});
|
||||
|
||||
it("still enforces auth for allowlisted clients", async () => {
|
||||
const { baseUrl } = await startWithAllowlist(["8.8.8.8"]);
|
||||
const res = await fetch(`${baseUrl}/health`, { headers: { "X-Forwarded-For": "8.8.8.8" } });
|
||||
const res = await fetch(`${baseUrl}/health`);
|
||||
expect(res.status).toBe(401);
|
||||
});
|
||||
|
||||
it("applies a new allowlist live via restartDebugServer", async () => {
|
||||
it("reloads the allowlist live via restartDebugServer", async () => {
|
||||
const { baseUrl } = await startWithAllowlist(["8.8.8.8"]);
|
||||
expect((await getAs(baseUrl, "9.9.9.9")).status).toBe(403);
|
||||
writeDebugServerConfig({ allowlist: ["9.9.9.9"] });
|
||||
expect(getDebugServerRuntimeStatus().allowlistCount).toBe(1);
|
||||
writeDebugServerConfig({ allowlist: ["9.9.9.9", "10.0.0.0/24"] });
|
||||
const status = await restartDebugServer();
|
||||
expect(status.running).toBe(true);
|
||||
expect(status.allowlistCount).toBe(2);
|
||||
await waitForReady(`${baseUrl}/health?token=${TOKEN}`);
|
||||
expect((await getAs(baseUrl, "9.9.9.9")).status).toBe(200);
|
||||
expect((await getAs(baseUrl, "8.8.8.8")).status).toBe(403);
|
||||
expect((await fetch(`${baseUrl}/health?token=${TOKEN}`)).status).toBe(200);
|
||||
});
|
||||
});
|
||||
|
||||
@ -202,7 +202,6 @@ async function createFixture() {
|
||||
fs.writeFileSync(path.join(baseDir, "debug_token.txt"), token, "utf8");
|
||||
fs.writeFileSync(path.join(baseDir, "debug_port.txt"), String(port), "utf8");
|
||||
fs.writeFileSync(path.join(baseDir, "debug_host.txt"), "0.0.0.0", "utf8");
|
||||
fs.writeFileSync(path.join(baseDir, "debug_allowlist.txt"), "159.195.63.46\n", "utf8");
|
||||
const debridLinkApiKeys = "key-a\nkey-b";
|
||||
const debridLinkKeyIds = getDebridLinkApiKeyIds(debridLinkApiKeys);
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user