[breaking] rename xForwardedForIndex to XFF_DEPTH (#4332)

* [breaking] rename xForwardedForIndex to xForwardedForNumProxies * add back link to blog post * changeset * update based on code review * update changeset * XFF_DEPTH_ENV (#4357) * use XFF_DEPTH_ENV * get rid of get_xff_depth * typo * Update packages/adapter-node/README.md Co-authored-by: Maurício Kishi <[email protected]> * Update packages/adapter-node/README.md * Update README.md * overhaul readme Co-authored-by: Rich Harris <[email protected]> Co-authored-by: Maurício Kishi <[email protected]> Co-authored-by: Rich Harris <[email protected]>
sveltejs · Mar 16, 2022 · 1bde07d · 1bde07d
1 parent 0151ee7
commit 1bde07d
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 53 deletions.
diff --git a/.changeset/sweet-parents-sell.md b/.changeset/sweet-parents-sell.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/adapter-node': patch
+---
+
+[breaking] rename `xForwardedForIndex` to `XFF_DEPTH` and make it an environment variable
diff --git a/packages/adapter-node/README.md b/packages/adapter-node/README.md
@@ -12,44 +12,23 @@ import adapter from '@sveltejs/adapter-node';
 
 export default {
 	kit: {
-		adapter: adapter({
-			// default options are shown
-			out: 'build',
-			precompress: false,
-			env: {
-				path: 'SOCKET_PATH',
-				host: 'HOST',
-				port: 'PORT',
-				origin: 'ORIGIN',
-				headers: {
-					protocol: 'PROTOCOL_HEADER',
-					host: 'HOST_HEADER'
-				}
-			},
-			xForwardedForIndex: -1
-		})
+		adapter: adapter()
 	}
 };
 ```
 
-## Options
-
-### out
-
-The directory to build the server to. It defaults to `build` — i.e. `node build` would start the server locally after it has been created.
-
-### precompress
-
-Enables precompressing using gzip and brotli for assets and prerendered pages. It defaults to `false`.
+## Environment variables
 
-### env
+### `PORT` and `HOST`
 
 By default, the server will accept connections on `0.0.0.0` using port 3000. These can be customised with the `PORT` and `HOST` environment variables:
 
 ```
 HOST=127.0.0.1 PORT=4000 node build
 ```
 
+### `ORIGIN`, `PROTOCOL_HEADER` and `HOST_HEADER`
+
 HTTP doesn't give SvelteKit a reliable way to know the URL that is currently being requested. The simplest way to tell SvelteKit where the app is being served is to set the `ORIGIN` environment variable:
 
 ```
@@ -64,6 +43,8 @@ PROTOCOL_HEADER=x-forwarded-proto HOST_HEADER=x-forwarded-host node build
 
 > [`x-forwarded-proto`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto) and [`x-forwarded-host`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Host) are de facto standard headers that forward the original protocol and host if you're using a reverse proxy (think load balancers and CDNs). You should only set these variables if you trust the reverse proxy.
 
+### `ADDRESS_HEADER` and `XFF_DEPTH`
+
 The [RequestEvent](https://kit.svelte.dev/docs/types#additional-types-requestevent) object passed to hooks and endpoints includes an `event.clientAddress` property representing the client's IP address. By default this is the connecting `remoteAddress`. If your server is behind one or more proxies (such as a load balancer), this value will contain the innermost proxy's IP address rather than the client's, so we need to specify an `ADDRESS_HEADER` to read the address from:
 
 ```
@@ -72,13 +53,71 @@ ADDRESS_HEADER=True-Client-IP node build
 
 > Headers can easily be spoofed. As with `PROTOCOL_HEADER` and `HOST_HEADER`, you should [know what you're doing](https://adam-p.ca/blog/2022/03/x-forwarded-for/) before setting these.
 
-All of these environment variables can be changed, if necessary, using the `env` option:
+If the `ADDRESS_HEADER` is `X-Forwarded-For`, the header value will contain a comma-separated list of IP addresses. The `XFF_DEPTH` environment variable should specify how many trusted proxies sit in front of your server. E.g. if there are three trusted proxies, proxy 3 will forward the addresses of the original connection and the first two proxies:
+
+```
+<client address>, <proxy 1 address>, <proxy 2 address>
+```
+
+Some guides will tell you to read the left-most address, but this leaves you [vulnerable to spoofing](https://adam-p.ca/blog/2022/03/x-forwarded-for/):
+
+```
+<spoofed address>, <client address>, <proxy 1 address>, <proxy 2 address>
+```
+
+Instead, we read from the _right_, accounting for the number of trusted proxies. In this case, we would use `XFF_DEPTH=3`.
+
+> If you need to read the left-most address instead (and don't care about spoofing) — for example, to offer a geolocation service, where it's more important for the IP address to be _real_ than _trusted_, you can do so by inspecting the `x-forwarded-for` header within your app.
+
+## Options
+
+The adapter can be configured with various options:
+
+```js
+// svelte.config.js
+import adapter from '@sveltejs/adapter-node';
+
+export default {
+	kit: {
+		adapter: adapter({
+			// default options are shown
+			out: 'build',
+			precompress: false,
+			env: {
+				path: 'SOCKET_PATH',
+				host: 'HOST',
+				port: 'PORT',
+				origin: 'ORIGIN',
+				xffDepth: 'XFF_DEPTH',
+				headers: {
+					address: 'ADDRESS_HEADER',
+					protocol: 'PROTOCOL_HEADER',
+					host: 'HOST_HEADER'
+				}
+			}
+		})
+	}
+};
+```
+
+### out
+
+The directory to build the server to. It defaults to `build` — i.e. `node build` would start the server locally after it has been created.
+
+### precompress
+
+Enables precompressing using gzip and brotli for assets and prerendered pages. It defaults to `false`.
+
+### env
+
+If you need to change the name of the environment variables used to configure the deployment (for example, you need to run multiple deployments from a single environment), you can tell the app to expect custom environment variables using the `env` option:
 
 ```js
 env: {
 	host: 'MY_HOST_VARIABLE',
 	port: 'MY_PORT_VARIABLE',
 	origin: 'MY_ORIGINURL',
+	xffDepth: 'MY_XFF_DEPTH',
 	headers: {
 		address: 'MY_ADDRESS_HEADER',
 		protocol: 'MY_PROTOCOL_HEADER',
@@ -94,24 +133,6 @@ MY_ORIGINURL=https://my.site \
 node build
 ```
 
-### xForwardedForIndex
-
-If the `ADDRESS_HEADER` is `X-Forwarded-For`, the header value will contain a comma-separated list of IP addresses. For example, if there are three proxies between your server and the client, proxy 3 will forward the addresses of the client and the first two proxies:
-
-```
-<client address>, <proxy 1 address>, <proxy 2 address>
-```
-
-To get the client address we could use `xForwardedFor: 0` or `xForwardedFor: -3`, which counts back from the number of addresses.
-
-**X-Forwarded-For is [trivial to spoof](https://adam-p.ca/blog/2022/03/x-forwarded-for/), howevever**:
-
-```
-<spoofed address>, <client address>, <proxy 1 address>, <proxy 2 address>
-```
-
-For that reason you should always use a negative number (depending on the number of proxies) if you need to trust `event.clientAddress`. In the above example, `0` would yield the spoofed address while `-3` would continue to work.
-
 ## Custom server
 
 The adapter creates two files in your build directory — `index.js` and `handler.js`. Running `index.js` — e.g. `node build`, if you use the default build directory — will start a server on the configured port.

diff --git a/packages/adapter-node/index.d.ts b/packages/adapter-node/index.d.ts
@@ -14,13 +14,13 @@ interface AdapterOptions {
 		host?: string;
 		port?: string;
 		origin?: string;
+		xffDepth?: string;
 		headers?: {
 			address?: string;
 			protocol?: string;
 			host?: string;
 		};
 	};
-	xForwardedForIndex?: number;
 }
 
 declare function plugin(options?: AdapterOptions): Adapter;

diff --git a/packages/adapter-node/index.js b/packages/adapter-node/index.js
@@ -18,13 +18,13 @@ export default function ({
 		host: host_env = 'HOST',
 		port: port_env = 'PORT',
 		origin: origin_env = 'ORIGIN',
+		xffDepth: xff_depth_env = 'XFF_DEPTH',
 		headers: {
 			address: address_header_env = 'ADDRESS_HEADER',
 			protocol: protocol_header_env = 'PROTOCOL_HEADER',
 			host: host_header_env = 'HOST_HEADER'
 		} = {}
-	} = {},
-	xForwardedForIndex = -1
+	} = {}
 } = {}) {
 	return {
 		name: '@sveltejs/adapter-node',
@@ -53,10 +53,10 @@ export default function ({
 					HOST_ENV: JSON.stringify(host_env),
 					PORT_ENV: JSON.stringify(port_env),
 					ORIGIN: origin_env ? `process.env[${JSON.stringify(origin_env)}]` : 'undefined',
+					XFF_DEPTH_ENV: xff_depth_env,
 					PROTOCOL_HEADER: JSON.stringify(protocol_header_env),
 					HOST_HEADER: JSON.stringify(host_header_env),
-					ADDRESS_HEADER: JSON.stringify(address_header_env),
-					X_FORWARDED_FOR_INDEX: JSON.stringify(xForwardedForIndex)
+					ADDRESS_HEADER: JSON.stringify(address_header_env)
 				}
 			});
 

diff --git a/packages/adapter-node/src/handler.d.ts b/packages/adapter-node/src/handler.d.ts
@@ -5,7 +5,7 @@ declare global {
 	const ADDRESS_HEADER: string;
 	const HOST_HEADER: string;
 	const PROTOCOL_HEADER: string;
-	const X_FORWARDED_FOR_INDEX: number;
+	const XFF_DEPTH_ENV: string;
 }
 
 export const handler: Handle;
diff --git a/packages/adapter-node/src/handler.js b/packages/adapter-node/src/handler.js
@@ -7,10 +7,11 @@ import { getRequest, setResponse } from '@sveltejs/kit/node';
 import { Server } from 'SERVER';
 import { manifest } from 'MANIFEST';
 
-/* global ORIGIN, ADDRESS_HEADER, PROTOCOL_HEADER, HOST_HEADER, X_FORWARDED_FOR_INDEX */
+/* global ORIGIN, ADDRESS_HEADER, PROTOCOL_HEADER, HOST_HEADER, XFF_DEPTH_ENV */
 
 const server = new Server(manifest);
 const origin = ORIGIN;
+const xff_depth = XFF_DEPTH_ENV ? parseInt(process.env[XFF_DEPTH_ENV]) : 1;
 
 const address_header = ADDRESS_HEADER && (process.env[ADDRESS_HEADER] || '').toLowerCase();
 const protocol_header = PROTOCOL_HEADER && process.env[PROTOCOL_HEADER];
@@ -63,7 +64,17 @@ const ssr = async (req, res) => {
 
 					if (address_header === 'x-forwarded-for') {
 						const addresses = value.split(',');
-						return addresses[(addresses.length + X_FORWARDED_FOR_INDEX) % addresses.length].trim();
+
+						if (xff_depth < 1) {
+							throw new Error(`${XFF_DEPTH_ENV} must be a positive integer`);
+						}
+
+						if (xff_depth > addresses.length) {
+							throw new Error(
+								`${XFF_DEPTH_ENV} is ${xff_depth}, but only found ${addresses.length} addresses`
+							);
+						}
+						return addresses[addresses.length - xff_depth].trim();
 					}
 
 					return value;