diff --git a/README.md b/README.md index 786212da0..1e13c4b5e 100644 --- a/README.md +++ b/README.md @@ -183,6 +183,7 @@ ignored. | `--source` | `FUNCTION_SOURCE` | The path to the directory of your function. Default: `cwd` (the current working directory) | | `--log-execution-id`| `LOG_EXECUTION_ID` | Enables execution IDs in logs, either `true` or `false`. When not specified, default to disable. Requires Node.js 13.0.0 or later. | | `--ignored-routes`| `IGNORED_ROUTES` | A route expression for requests that should not be routed the function. An empty 404 response will be returned. This is set to `/favicon.ico|/robots.txt` by default for `http` functions. | +| `--trust-proxy` | `TRUST_PROXY` | The Express [`trust proxy`](https://expressjs.com/en/guide/behind-proxies.html) setting used to derive `req.ip`, `req.hostname` and `req.protocol` from the `X-Forwarded-*` headers: a hop count, `true`, `false`, or a list of trusted addresses. Default: `1`, the one proxy Cloud Run places in front of a function. Raise it when another proxy or load balancer sits in front of Cloud Run. | You can set command-line flags in your `package.json` via the `start` script. For example: diff --git a/src/options.ts b/src/options.ts index b8c63cb8e..c3fef444e 100644 --- a/src/options.ts +++ b/src/options.ts @@ -59,6 +59,12 @@ export interface FrameworkOptions { * Routes that should return a 404 without invoking the function. */ ignoredRoutes: string | null; + /** + * The Express "trust proxy" setting: how many proxy hops in front of the + * function are trusted when deriving req.ip, req.hostname and req.protocol + * from the X-Forwarded-* headers. + */ + trustProxy: boolean | number | string; } /** @@ -139,6 +145,43 @@ const IgnoredRoutesOption = new ConfigurableOption( null, // null by default so we can detect if it is explicitly set to "" ); +/** + * Express "trust proxy" setting. Cloud Run places one proxy in front of the + * function; it appends the address it accepted the connection from as the + * last X-Forwarded-For entry, and anything before that entry is whatever the + * client sent. Trusting every hop (true) makes req.ip, req.hostname and + * req.protocol follow client-supplied X-Forwarded-For, X-Forwarded-Host and + * X-Forwarded-Proto values instead of the platform's. + */ +const TrustProxyOption = new ConfigurableOption( + 'trust-proxy', + 'TRUST_PROXY', + 1, + (x: boolean | number | string) => { + if (typeof x === 'boolean' || typeof x === 'number') { + return x; + } + const value = x.trim(); + if (value.toLowerCase() === 'true') { + return true; + } + if (value.toLowerCase() === 'false') { + return false; + } + if (/^\d+$/.test(value)) { + return parseInt(value, 10); + } + if (value === '') { + throw new OptionsError( + 'Trust proxy must be a hop count, true, false, or a list of trusted addresses', + ); + } + // Anything else is passed to Express as is: a comma-separated list of + // addresses or subnets, or one of its named presets. + return value; + }, +); + export const requiredNodeJsVersionForLogExecutionID = '13.0.0'; export const logExecutionIdSupported = @@ -189,6 +232,7 @@ export const parseOptions = ( SourceLocationOption.cliOption, TimeoutOption.cliOption, IgnoredRoutesOption.cliOption, + TrustProxyOption.cliOption, ], }); return { @@ -200,5 +244,6 @@ export const parseOptions = ( printHelp: cliArgs[2] === '-h' || cliArgs[2] === '--help', enableExecutionId: ExecutionIdOption.parse(argv, envVars), ignoredRoutes: IgnoredRoutesOption.parse(argv, envVars), + trustProxy: TrustProxyOption.parse(argv, envVars), }; }; diff --git a/src/server.ts b/src/server.ts index 7710088d1..2eaeda613 100644 --- a/src/server.ts +++ b/src/server.ts @@ -95,7 +95,10 @@ export function getServer( // The parser will process ALL content types so MUST come last. // Subsequent parsers will be skipped when one is matched. app.use(bodyParser.raw(rawBodySavingOptions)); - app.enable('trust proxy'); // To respect X-Forwarded-For header. + // Derive req.ip, req.hostname and req.protocol from the X-Forwarded-* + // headers, trusting only the configured number of proxy hops (one on Cloud + // Run) so that values a client put in those headers are not used. + app.set('trust proxy', options.trustProxy); // Disable Express 'x-powered-by' header: // http://expressjs.com/en/advanced/best-practice-security.html#at-a-minimum-disable-x-powered-by-header diff --git a/src/testing.ts b/src/testing.ts index 16f465f3b..66753ddeb 100644 --- a/src/testing.ts +++ b/src/testing.ts @@ -57,5 +57,6 @@ export const getTestServer = (functionName: string): Server => { sourceLocation: '', printHelp: false, ignoredRoutes: null, + trustProxy: 1, }); }; diff --git a/test/integration/http.ts b/test/integration/http.ts index ec78d4f91..5eb3418a6 100644 --- a/test/integration/http.ts +++ b/test/integration/http.ts @@ -18,6 +18,7 @@ import * as supertest from 'supertest'; import * as functions from '../../src/index'; import {getTestServer} from '../../src/testing'; +import {getServer} from '../../src/server'; describe('HTTP Function', () => { let callCount = 0; @@ -117,4 +118,58 @@ describe('HTTP Function', () => { assert.strictEqual(callCount, test.expectedCallCount); }); }); + + describe('proxy headers', () => { + before(() => { + functions.http('testProxyHeaders', (req, res) => { + res.send({ + ip: req.ip, + ips: req.ips, + hostname: req.hostname, + protocol: req.protocol, + }); + }); + }); + + it('derives the client address from the hop Cloud Run adds, not from a value the client sent', async () => { + // Cloud Run appends the connecting client's address to whatever the + // client already put in X-Forwarded-For, so the last entry is the one the + // platform vouches for. + const response = await supertest(getTestServer('testProxyHeaders')) + .get('/') + .set('X-Forwarded-For', '198.51.100.77, 203.0.113.9') + .set('X-Forwarded-Proto', 'https'); + assert.strictEqual(response.status, 200); + assert.strictEqual(response.body.ip, '203.0.113.9'); + assert.deepStrictEqual(response.body.ips, ['203.0.113.9']); + assert.strictEqual(response.body.protocol, 'https'); + }); + + it('only trusts every hop when trust proxy is set to true explicitly', async () => { + const handler: functions.HttpFunction = (req, res) => { + res.send({ip: req.ip, ips: req.ips}); + }; + const server = getServer(handler, { + signatureType: 'http', + enableExecutionId: false, + timeoutMilliseconds: 0, + port: '0', + target: '', + sourceLocation: '', + printHelp: false, + ignoredRoutes: null, + trustProxy: true, + }); + const response = await supertest(server) + .get('/') + .set('X-Forwarded-For', '198.51.100.77, 203.0.113.9'); + assert.strictEqual(response.status, 200); + // The previous default: the first, client-supplied entry wins. + assert.strictEqual(response.body.ip, '198.51.100.77'); + assert.deepStrictEqual(response.body.ips, [ + '198.51.100.77', + '203.0.113.9', + ]); + }); + }); }); diff --git a/test/integration/legacy_event.ts b/test/integration/legacy_event.ts index 231d0f62e..8d4f86083 100644 --- a/test/integration/legacy_event.ts +++ b/test/integration/legacy_event.ts @@ -41,6 +41,7 @@ const testOptions = { sourceLocation: '', printHelp: false, ignoredRoutes: null, + trustProxy: 1, }; describe('Event Function', () => { diff --git a/test/options.ts b/test/options.ts index 069e78d6f..182b1b7c7 100644 --- a/test/options.ts +++ b/test/options.ts @@ -60,6 +60,7 @@ describe('parseOptions', () => { enableExecutionId: false, timeoutMilliseconds: 0, ignoredRoutes: null, + trustProxy: 1, }, }, { @@ -124,6 +125,7 @@ describe('parseOptions', () => { '--source=/source', '--timeout=3', '--ignored-routes=avocado', + '--trust-proxy=2', ], envVars: { PORT: '4567', @@ -132,6 +134,7 @@ describe('parseOptions', () => { FUNCTION_SOURCE: '/somewhere/else', CLOUD_RUN_TIMEOUT_SECONDS: '5', IGNORED_ROUTES: 'banana', + TRUST_PROXY: 'true', }, expectedOptions: { port: '1234', @@ -142,6 +145,23 @@ describe('parseOptions', () => { enableExecutionId: false, timeoutMilliseconds: 3000, ignoredRoutes: 'avocado', + trustProxy: 2, + }, + }, + { + name: 'parses the trust proxy option as a hop count, a boolean, or an address list', + cliOpts: ['bin/node', '/index.js'], + envVars: {TRUST_PROXY: 'loopback, 10.0.0.0/8'}, + expectedOptions: { + trustProxy: 'loopback, 10.0.0.0/8', + }, + }, + { + name: 'parses a boolean trust proxy option', + cliOpts: ['bin/node', '/index.js', '--trust-proxy=false'], + envVars: {}, + expectedOptions: { + trustProxy: false, }, }, ];