From de51a231e3b924de1c857b26eb85fb3b5bdef52b Mon Sep 17 00:00:00 2001 From: Brandon Aaron Date: Tue, 23 Mar 2021 07:44:43 -0400 Subject: [PATCH] feat: Fallback to X-Forwarded-* headers * Fallback to X-Forwarded-* headers This uses the first value from X-Forwarded-Host and X-Forwarded-Proto if they're present and the standard Forwarded header is not. * Update parseForwarded to handle X-Forwarded-* This updates the signature for parseForwarded to take in the headers and handle the logic of falling back to X-Forwarded-* headers. * Update src/util/HeaderUtil.ts Co-authored-by: Ruben Verborgh * Inline parseXForwarded helper Additionally fixes a typo, updates a unit test, and removes a typing that is no longer necessary. * Tweak handling of X-Forwarded value checking and assignment * Fix: terminology & consistency suggestions from review Co-authored-by: Ruben Verborgh Co-authored-by: Ruben Verborgh Co-authored-by: Wouter Termont --- src/ldp/UnsecureWebSocketsProtocol.ts | 2 +- src/ldp/http/OriginalUrlExtractor.ts | 18 +++---- src/util/HeaderUtil.ts | 19 +++++-- .../ldp/UnsecureWebSocketsProtocol.test.ts | 16 ++++++ .../ldp/http/OriginalUrlExtractor.test.ts | 49 +++++++++++++++++++ test/unit/util/HeaderUtil.test.ts | 49 ++++++++++++++++--- 6 files changed, 131 insertions(+), 22 deletions(-) diff --git a/src/ldp/UnsecureWebSocketsProtocol.ts b/src/ldp/UnsecureWebSocketsProtocol.ts index 40741a270..5bfe5040b 100644 --- a/src/ldp/UnsecureWebSocketsProtocol.ts +++ b/src/ldp/UnsecureWebSocketsProtocol.ts @@ -44,7 +44,7 @@ class WebSocketListener extends EventEmitter { } // Store the HTTP host and protocol - const forwarded = parseForwarded(headers.forwarded); + const forwarded = parseForwarded(headers); this.host = forwarded.host ?? headers.host ?? 'localhost'; this.protocol = forwarded.proto === 'https' || (socket as any).secure ? 'https:' : 'http:'; } diff --git a/src/ldp/http/OriginalUrlExtractor.ts b/src/ldp/http/OriginalUrlExtractor.ts index ea36dde8c..facf4865a 100644 --- a/src/ldp/http/OriginalUrlExtractor.ts +++ b/src/ldp/http/OriginalUrlExtractor.ts @@ -21,17 +21,17 @@ export class OriginalUrlExtractor extends TargetExtractor { throw new Error('Missing URL'); } - // Extract host and protocol (possibly overridden by the Forwarded header) + // Extract host and protocol (possibly overridden by the Forwarded/X-Forwarded-* header) let { host } = headers; let protocol = (connection as TLSSocket)?.encrypted ? 'https' : 'http'; - if (headers.forwarded) { - const forwarded = parseForwarded(headers.forwarded); - if (forwarded.host) { - ({ host } = forwarded); - } - if (forwarded.proto) { - ({ proto: protocol } = forwarded); - } + + // Check Forwarded/X-Forwarded-* headers + const forwarded = parseForwarded(headers); + if (forwarded.host) { + ({ host } = forwarded); + } + if (forwarded.proto) { + ({ proto: protocol } = forwarded); } // Perform a sanity check on the host diff --git a/src/util/HeaderUtil.ts b/src/util/HeaderUtil.ts index 7c9d47276..311c31baf 100644 --- a/src/util/HeaderUtil.ts +++ b/src/util/HeaderUtil.ts @@ -1,3 +1,4 @@ +import type { IncomingHttpHeaders } from 'http'; import { getLoggerFor } from '../logging/LogUtil'; import type { HttpResponse } from '../server/HttpResponse'; import { BadRequestHttpError } from './errors/BadRequestHttpError'; @@ -429,21 +430,29 @@ export interface Forwarded { } /** - * Parses a Forwarded header value. + * Parses a Forwarded header value and will fall back to X-Forwarded-* headers. * - * @param value - The Forwarded header value. + * @param headers - The incoming HTTP headers. * * @returns The parsed Forwarded header. */ -export function parseForwarded(value = ''): Forwarded { +export function parseForwarded(headers: IncomingHttpHeaders): Forwarded { const forwarded: Record = {}; - if (value) { - for (const pair of value.replace(/\s*,.*$/u, '').split(';')) { + if (headers.forwarded) { + for (const pair of headers.forwarded.replace(/\s*,.*/u, '').split(';')) { const components = /^(by|for|host|proto)=(.+)$/u.exec(pair); if (components) { forwarded[components[1]] = components[2]; } } + } else { + const suffixes = [ 'host', 'proto' ]; + for (const suffix of suffixes) { + const value = headers[`x-forwarded-${suffix}`] as string; + if (value) { + forwarded[suffix] = value.trim().replace(/\s*,.*/u, ''); + } + } } return forwarded; } diff --git a/test/unit/ldp/UnsecureWebSocketsProtocol.test.ts b/test/unit/ldp/UnsecureWebSocketsProtocol.test.ts index 3fa182daa..8f241203a 100644 --- a/test/unit/ldp/UnsecureWebSocketsProtocol.test.ts +++ b/test/unit/ldp/UnsecureWebSocketsProtocol.test.ts @@ -182,4 +182,20 @@ describe('An UnsecureWebSocketsProtocol', (): void => { expect(webSocket.messages).toHaveLength(2); expect(webSocket.messages.pop()).toBe('ack https://other.example/protocol/foo'); }); + + it('respects the X-Forwarded-* headers if Forwarded header is not present.', async(): Promise => { + const webSocket = new DummySocket(); + const upgradeRequest = { + headers: { + 'x-forwarded-host': 'other.example', + 'x-forwarded-proto': 'https', + 'sec-websocket-protocol': 'solid-0.1', + }, + socket: {}, + } as any as HttpRequest; + await protocol.handle({ webSocket, upgradeRequest } as any); + webSocket.emit('message', 'sub https://other.example/protocol/foo'); + expect(webSocket.messages).toHaveLength(2); + expect(webSocket.messages.pop()).toBe('ack https://other.example/protocol/foo'); + }); }); diff --git a/test/unit/ldp/http/OriginalUrlExtractor.test.ts b/test/unit/ldp/http/OriginalUrlExtractor.test.ts index 83cdc664d..c1c6061ba 100644 --- a/test/unit/ldp/http/OriginalUrlExtractor.test.ts +++ b/test/unit/ldp/http/OriginalUrlExtractor.test.ts @@ -81,4 +81,53 @@ describe('A OriginalUrlExtractor', (): void => { await expect(extractor.handle({ request: { url: '/foo/bar', headers } as any })) .resolves.toEqual({ path: 'https://pod.example/foo/bar' }); }); + + it('should fallback to x-fowarded-* headers.', async(): Promise => { + const headers = { + host: 'test.com', + 'x-forwarded-host': 'pod.example', + 'x-forwarded-proto': 'https', + }; + await expect(extractor.handle({ request: { url: '/foo/bar', headers } as any })) + .resolves.toEqual({ path: 'https://pod.example/foo/bar' }); + }); + + it('should just take x-forwarded-host if provided.', async(): Promise => { + const headers = { + host: 'test.com', + 'x-forwarded-host': 'pod.example', + }; + await expect(extractor.handle({ request: { url: '/foo/bar', headers } as any })) + .resolves.toEqual({ path: 'http://pod.example/foo/bar' }); + }); + + it('should just take x-forwarded-protocol if provided.', async(): Promise => { + const headers = { + host: 'test.com', + 'x-forwarded-proto': 'https', + }; + await expect(extractor.handle({ request: { url: '/foo/bar', headers } as any })) + .resolves.toEqual({ path: 'https://test.com/foo/bar' }); + }); + + it('should prefer forwarded header to x-forwarded-* headers.', async(): Promise => { + const headers = { + host: 'test.com', + forwarded: 'proto=http;host=pod.example', + 'x-forwarded-proto': 'https', + 'x-forwarded-host': 'anotherpod.example', + }; + await expect(extractor.handle({ request: { url: '/foo/bar', headers } as any })) + .resolves.toEqual({ path: 'http://pod.example/foo/bar' }); + }); + + it('should just take the first x-forwarded-* value.', async(): Promise => { + const headers = { + host: 'test.com', + 'x-forwarded-host': 'pod.example, another.domain', + 'x-forwarded-proto': 'http,https', + }; + await expect(extractor.handle({ request: { url: '/foo/bar', headers } as any })) + .resolves.toEqual({ path: 'http://pod.example/foo/bar' }); + }); }); diff --git a/test/unit/util/HeaderUtil.test.ts b/test/unit/util/HeaderUtil.test.ts index f6884f5fc..3610d395e 100644 --- a/test/unit/util/HeaderUtil.test.ts +++ b/test/unit/util/HeaderUtil.test.ts @@ -188,16 +188,18 @@ describe('HeaderUtil', (): void => { }); describe('#parseForwarded', (): void => { - it('parses an undefined value.', (): void => { - expect(parseForwarded()).toEqual({}); + it('handles an empty set of headers.', (): void => { + expect(parseForwarded({})).toEqual({}); }); - it('parses an empty string.', (): void => { - expect(parseForwarded('')).toEqual({}); + it('handles empty string values.', (): void => { + const headers = { forwarded: '', 'x-forwarded-host': '', 'x-forwarded-proto': '' }; + expect(parseForwarded(headers)).toEqual({}); }); it('parses a Forwarded header value.', (): void => { - expect(parseForwarded('for=192.0.2.60;proto=http;by=203.0.113.43;host=example.org')).toEqual({ + const headers = { forwarded: 'for=192.0.2.60;proto=http;by=203.0.113.43;host=example.org' }; + expect(parseForwarded(headers)).toEqual({ by: '203.0.113.43', for: '192.0.2.60', host: 'example.org', @@ -206,15 +208,48 @@ describe('HeaderUtil', (): void => { }); it('skips empty fields.', (): void => { - expect(parseForwarded('for=192.0.2.60;proto=;by=;host=')).toEqual({ + const headers = { forwarded: 'for=192.0.2.60;proto=;by=;host=' }; + expect(parseForwarded(headers)).toEqual({ for: '192.0.2.60', }); }); it('takes only the first value into account.', (): void => { - expect(parseForwarded('host=pod.example, for=192.0.2.43, host=other')).toEqual({ + const headers = { forwarded: 'host=pod.example, for=192.0.2.43, host=other' }; + expect(parseForwarded(headers)).toEqual({ host: 'pod.example', }); }); + + it('should fall back to X-Forwarded-Host and X-Forwarded-Proto without Forward header.', (): void => { + const headers = { 'x-forwarded-host': 'pod.example', 'x-forwarded-proto': 'https' }; + expect(parseForwarded(headers)).toEqual({ + host: 'pod.example', + proto: 'https', + }); + }); + + it('should prefer Forwarded to X-Forwarded-Host and X-Forwarded-Proto with Forward header.', (): void => { + const headers = { + forwarded: 'proto=http;host=pod.example', + 'x-forwarded-host': 'anotherpod.example', + 'x-forwarded-proto': 'https', + }; + expect(parseForwarded(headers)).toEqual({ + host: 'pod.example', + proto: 'http', + }); + }); + + it('should properly handle multiple values with varying spaces for X-Forwarded-*.', (): void => { + const headers = { + 'x-forwarded-host': ' pod.example ,192.0.2.60, 192.0.2.43', + 'x-forwarded-proto': ' https ,http', + }; + expect(parseForwarded(headers)).toEqual({ + host: 'pod.example', + proto: 'https', + }); + }); }); });