Skip to content
Open
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
10 changes: 8 additions & 2 deletions packages/angular/ssr/node/src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
getFirstHeaderValue,
isProxyHeaderAllowed,
normalizeTrustProxyHeaders,
parseForwardedHeader,
} from '../../src/utils/validation';

/**
Expand Down Expand Up @@ -40,7 +41,7 @@ const HTTP2_PSEUDO_HEADERS: ReadonlySet<string> = new Set([
* @param trustProxyHeaders - A boolean or an array of proxy headers to trust when constructing the request URL.
*
* @remarks
* When `trustProxyHeaders` is enabled, headers such as `X-Forwarded-Host` and
* When `trustProxyHeaders` is enabled, headers such as `Forwarded`, `X-Forwarded-Host`, and
* `X-Forwarded-Prefix` should ideally be strictly validated at a higher infrastructure
* level (e.g., at the reverse proxy or API gateway) before reaching the application.
*
Expand Down Expand Up @@ -97,7 +98,7 @@ function createRequestHeaders(nodeHeaders: IncomingHttpHeaders): Headers {
* @param trustProxyHeaders - A set of allowed proxy headers.
*
* @remarks
* When `trustProxyHeaders` is enabled, headers such as `X-Forwarded-Host` and
* When `trustProxyHeaders` is enabled, headers such as `Forwarded`, `X-Forwarded-Host`, and
* `X-Forwarded-Prefix` should ideally be strictly validated at a higher infrastructure
* level (e.g., at the reverse proxy or API gateway) before reaching the application.
*
Expand All @@ -114,11 +115,16 @@ export function createRequestUrl(
originalUrl,
} = nodeRequest as IncomingMessage & { originalUrl?: string };

const forwardedHeaderValue = getAllowedProxyHeaderValue(headers, 'forwarded', trustProxyHeaders);
const forwardedParams = parseForwardedHeader(forwardedHeaderValue);

const protocol =
forwardedParams.proto ??
getAllowedProxyHeaderValue(headers, 'x-forwarded-proto', trustProxyHeaders) ??
('encrypted' in socket && socket.encrypted ? 'https' : 'http');

const hostname =
forwardedParams.host ??
getAllowedProxyHeaderValue(headers, 'x-forwarded-host', trustProxyHeaders) ??
headers.host ??
headers[':authority'];
Expand Down
58 changes: 58 additions & 0 deletions packages/angular/ssr/node/test/request_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,62 @@ describe('createRequestUrl', () => {
);
expect(url.href).toBe('https://example.com:8443/test');
});

it('should prioritize "forwarded" header over standard and x-forwarded headers', () => {
const url = createRequestUrl(
createRequest({
headers: {
host: 'localhost:8080',
'x-forwarded-host': 'other.com',
'x-forwarded-proto': 'http',
'forwarded': 'host=example.com;proto=https',
},
url: '/test',
}),
normalizeTrustProxyHeaders(true),
);
expect(url.href).toBe('https://example.com/test');
});

it('should parse forwarded parameters correctly (including quoted values)', () => {
const url = createRequestUrl(
createRequest({
headers: {
host: 'localhost:8080',
'forwarded': 'host="example.com:8443";proto="https"',
},
url: '/test',
}),
normalizeTrustProxyHeaders(true),
);
expect(url.href).toBe('https://example.com:8443/test');
});

it('should not treat parameters inside quoted values as top-level parameters in "forwarded" header', () => {
const url = createRequestUrl(
createRequest({
headers: {
host: 'localhost:8080',
'forwarded': 'for="192.0.2.60;host=evil.com";proto=https',
},
url: '/test',
}),
normalizeTrustProxyHeaders(true),
);
expect(url.href).toBe('https://localhost:8080/test');
});

it('should ignore "forwarded" header when it is not trusted', () => {
const url = createRequestUrl(
createRequest({
headers: {
host: 'localhost:8080',
'forwarded': 'host=example.com;proto=https',
},
url: '/test',
}),
normalizeTrustProxyHeaders(false),
);
expect(url.href).toBe('http://localhost:8080/test');
});
});
Comment thread
alan-agius4 marked this conversation as resolved.
4 changes: 2 additions & 2 deletions packages/angular/ssr/src/app-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ export interface AngularAppEngineOptions {
allowedHosts?: readonly string[];

/**
* Extends the scope of trusted proxy headers (`X-Forwarded-*`).
* Extends the scope of trusted proxy headers (`Forwarded` or `X-Forwarded-*`).
*
* @remarks
* **This is a security-sensitive option!**
*
* When `trustProxyHeaders` is enabled, request headers such as `X-Forwarded-Host` and
* When `trustProxyHeaders` is enabled, request headers such as `Forwarded`, `X-Forwarded-Host`, and
* `X-Forwarded-Prefix` are trusted by the server and used for routing. These
* headers must be strictly validated and provided by a trusted client (e.g., at a reverse proxy, load
* balancer, or API gateway) and must *not* be provided by untrusted end users.
Expand Down
67 changes: 61 additions & 6 deletions packages/angular/ssr/src/utils/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ export function sanitizeRequestHeaders(

for (const [key, value] of request.headers) {
const lowerKey = key.toLowerCase();
if (lowerKey.startsWith('x-forwarded-') && !isProxyHeaderAllowed(lowerKey, trustProxyHeaders)) {
const isProxyHeader = lowerKey === 'forwarded' || lowerKey.startsWith('x-forwarded-');
if (isProxyHeader && !isProxyHeaderAllowed(lowerKey, trustProxyHeaders)) {
// eslint-disable-next-line no-console
console.warn(
`Received "${key}" header but "trustProxyHeaders" was not set up to allow it.\n` +
Expand Down Expand Up @@ -199,6 +200,17 @@ function validateHeaders(
}
}

const forwarded = headers.get('forwarded');
if (forwarded) {
const forwardedParams = parseForwardedHeader(forwarded);
if (forwardedParams.host && !disableHostCheck) {
verifyHostAllowed('Forwarded "host"', forwardedParams.host, allowedHosts);
}
if (forwardedParams.proto && !VALID_PROTO_REGEX.test(forwardedParams.proto)) {
throw new Error('Header "forwarded" proto parameter must be either "http" or "https".');
}
}

const xForwardedPort = getFirstHeaderValue(headers.get('x-forwarded-port'));
if (xForwardedPort && !VALID_PORT_REGEX.test(xForwardedPort)) {
throw new Error('Header "x-forwarded-port" must be a numeric value.');
Expand Down Expand Up @@ -251,12 +263,55 @@ export function normalizeTrustProxyHeaders(
return new Set([TRUST_ALL_PROXY_HEADERS]);
}

const normalizedTrustedProxyHeaders = new Set(trustProxyHeaders.map((h) => h.toLowerCase()));
if (normalizedTrustedProxyHeaders.has(TRUST_ALL_PROXY_HEADERS)) {
throw new Error(
`"${TRUST_ALL_PROXY_HEADERS}" is not allowed as a value for the "trustProxyHeaders" option.`,
);
const normalizedTrustedProxyHeaders = new Set<string>();
for (const header of trustProxyHeaders) {
const lowerHeader = header.toLowerCase();
if (lowerHeader === TRUST_ALL_PROXY_HEADERS) {
throw new Error(
`"${TRUST_ALL_PROXY_HEADERS}" is not allowed as a value for the "trustProxyHeaders" option.`,
);
}
const isValid = lowerHeader === 'forwarded' || lowerHeader.startsWith('x-forwarded-');
if (!isValid) {
throw new Error(
`"${header}" is not a valid proxy header. Trusted proxy headers must be "forwarded" or start with "x-forwarded-".`,
);
}
normalizedTrustedProxyHeaders.add(lowerHeader);
}

return normalizedTrustedProxyHeaders;
}

/**
* Parses the standard `Forwarded` header (RFC 7239).
* It extracts the parameters from the first (leftmost) element in the header.
*
* @param headerValue - The value of the `Forwarded` header.
* @returns A record of lowercase parameter names to their values.
*/
export function parseForwardedHeader(
headerValue: string | null | undefined,
): Record<string, string> {
if (!headerValue) {
return {};
}

const firstElement = headerValue.split(',', 1)[0];
const params: Record<string, string> = {};
const paramRegex = /([^\s;=]+)\s*=\s*("(?:[^"\\]|\\.)*"|[^;\s]*)/gi;

let match;
while ((match = paramRegex.exec(firstElement)) !== null) {
const key = match[1].toLowerCase();
let val = match[2];

if (val[0] === '"' && val.at(-1) === '"') {
val = val.slice(1, -1).replace(/\\(.)/g, '$1');
}

params[key] = val;
}

return params;
}
Comment thread
alan-agius4 marked this conversation as resolved.
82 changes: 82 additions & 0 deletions packages/angular/ssr/test/utils/validation_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ describe('Validation Utils', () => {
);
});

it('should return a set containing "forwarded" when input is an array containing it', () => {
expect(normalizeTrustProxyHeaders(['Forwarded'])).toEqual(new Set(['forwarded']));
});

it('should throw an error if input array contains "*"', () => {
expect(() => normalizeTrustProxyHeaders(['*'])).toThrowError(
'"*" is not allowed as a value for the "trustProxyHeaders" option.',
Expand All @@ -65,6 +69,15 @@ describe('Validation Utils', () => {
'"*" is not allowed as a value for the "trustProxyHeaders" option.',
);
});

it('should throw an error if input array contains an invalid proxy header name', () => {
expect(() => normalizeTrustProxyHeaders(['invalid-header'])).toThrowError(
'"invalid-header" is not a valid proxy header. Trusted proxy headers must be "forwarded" or start with "x-forwarded-".',
);
expect(() => normalizeTrustProxyHeaders(['x-forward-host'])).toThrowError(
'"x-forward-host" is not a valid proxy header. Trusted proxy headers must be "forwarded" or start with "x-forwarded-".',
);
});
});

describe('validateUrl', () => {
Expand Down Expand Up @@ -184,6 +197,49 @@ describe('Validation Utils', () => {
);
});

it('should pass for valid request with forwarded header', () => {
const req = new Request('http://example.com', {
headers: {
'forwarded': 'host=example.com;proto=https',
},
});

expect(() => validateRequest(req, allowedHosts, false)).not.toThrow();
});

it('should throw if forwarded host contains path separators', () => {
const req = new Request('http://example.com', {
headers: {
'forwarded': 'host="example.com/bad"',
},
});
expect(() => validateRequest(req, allowedHosts, false)).toThrowError(
'Header "Forwarded "host"" with value "example.com/bad" contains characters that are not allowed.',
);
});

it('should throw if forwarded host is not allowed', () => {
const req = new Request('http://example.com', {
headers: {
'forwarded': 'host=evil.com',
},
});
expect(() => validateRequest(req, allowedHosts, false)).toThrowError(
'Header "Forwarded "host"" with value "evil.com" is not allowed.',
);
});

it('should throw if forwarded proto is invalid', () => {
const req = new Request('http://example.com', {
headers: {
'forwarded': 'proto=ftp',
},
});
expect(() => validateRequest(req, allowedHosts, false)).toThrowError(
'Header "forwarded" proto parameter must be either "http" or "https".',
);
});

it('should throw error if x-forwarded-prefix is invalid', () => {
const inputs = [
'//evil',
Expand Down Expand Up @@ -317,5 +373,31 @@ describe('Validation Utils', () => {
expect(secured).toBe(req);
expect(secured.headers.get('accept')).toBe('application/json');
});

it('should scrub unallowed forwarded header by default', () => {
const req = new Request('http://example.com', {
headers: {
'host': 'example.com',
'forwarded': 'host=evil.com;proto=https',
},
});
const secured = sanitizeRequestHeaders(req, normalizeTrustProxyHeaders(undefined));

expect(secured.headers.get('host')).toBe('example.com');
expect(secured.headers.has('forwarded')).toBeFalse();
});

it('should retain allowed forwarded header when explicitly provided', () => {
const req = new Request('http://example.com', {
headers: {
'host': 'example.com',
'forwarded': 'host=proxy.com;proto=https',
},
});
const secured = sanitizeRequestHeaders(req, normalizeTrustProxyHeaders(['forwarded']));

expect(secured.headers.get('host')).toBe('example.com');
expect(secured.headers.get('forwarded')).toBe('host=proxy.com;proto=https');
});
});
});
Loading