From 422e539e8989e65ba43ecc39ddbaa3c4f755d465 Mon Sep 17 00:00:00 2001 From: Yiyu He Date: Mon, 28 Oct 2019 10:39:21 +0800 Subject: [PATCH] feat: support app.proxyIPHeader and app.maxIpsCount to make ctx.ips more security --- docs/api/index.md | 4 +++- docs/api/request.md | 44 +++++++++++++++++++++++++++++++++++++++++--- lib/application.js | 4 ++++ lib/request.js | 8 ++++++-- test/request/ips.js | 44 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 98 insertions(+), 6 deletions(-) diff --git a/docs/api/index.md b/docs/api/index.md index 010d4d44c..7a91c0b77 100644 --- a/docs/api/index.md +++ b/docs/api/index.md @@ -114,7 +114,9 @@ app.listen(3000); - `app.env` defaulting to the __NODE_ENV__ or "development" - `app.keys` array of signed cookie keys - `app.proxy` when true proxy header fields will be trusted - - `app.subdomainOffset` offset of `.subdomains` to ignore [2] + - `app.subdomainOffset` offset of `.subdomains` to ignore, default to 2 + - `app.proxyIpHeader` proxy ip header, default to `X-Forwarded-For` + - `app.maxIpsCount` max ips read from proxy ip header, default to 0 (means infinity) You can pass the settings to the constructor: ```js diff --git a/docs/api/request.md b/docs/api/request.md index 0317c30f8..95768c9c4 100644 --- a/docs/api/request.md +++ b/docs/api/request.md @@ -99,7 +99,7 @@ ctx.request.href; Get hostname when present. Supports `X-Forwarded-Host` when `app.proxy` is __true__, otherwise `Host` is used. - + If host is IPv6, Koa delegates parsing to [WHATWG URL API](https://nodejs.org/dist/latest-v8.x/docs/api/url.html#url_the_whatwg_url_api), *Note* This may impact performance. @@ -193,8 +193,46 @@ ctx.body = await db.find('something'); ### request.ips When `X-Forwarded-For` is present and `app.proxy` is enabled an array - of these ips is returned, ordered from upstream -> downstream. When disabled - an empty array is returned. + of these ips is returned, ordered from upstream -> downstream. When + disabled an empty array is returned. + + For example if the value were "client, proxy1, proxy2", + you would receive the array `["client", "proxy1", "proxy2"]`. + + Most of the reverse proxy(nginx) set x-forwarded-for via + `proxy_add_x_forwarded_for`, which poses a certain security risk. + A malicious attacker can forge a client's ip address by forging + a `X-Forwarded-For`request header. The request sent by the client + has an `X-Forwarded-For` request header for 'forged'. After being + forwarded by the reverse proxy, `request.ips` will be + ['forged', 'client', 'proxy1', 'proxy2']. + + Koa offers two options to avoid being bypassed. + + If you can control the reverse proxy, you can avoid bypassing + by adjusting the configuration, or use the `app.proxyIpHeader` + provided by koa to avoid reading `x-forwarded-for` to get ips. + + ```js + const app = new Koa({ + proxy: true, + proxyIpHeader: 'X-Real-IP', + }); + ``` + + If you know exactly how many reverse proxies are in front of + the server, you can avoid reading the user's forged request + header by configuring `app.maxIpsCount`: + + ```js + const app = new Koa({ + proxy: true, + maxIpsCount: 1, // only one proxy in front of the server + }); + + // request.header['X-Forwarded-For'] === [ '127.0.0.1', '127.0.0.2' ]; + // ctx.ips === [ '127.0.0.2' ]; + ``` ### request.subdomains diff --git a/lib/application.js b/lib/application.js index 46f43134e..57b701cef 100644 --- a/lib/application.js +++ b/lib/application.js @@ -41,6 +41,8 @@ module.exports = class Application extends Emitter { * @param {string[]} [options.keys] Signed cookie keys * @param {boolean} [options.proxy] Trust proxy headers * @param {number} [options.subdomainOffset] Subdomain offset + * @param {boolean} [options.proxyIpHeader] proxy ip header, default to X-Forwarded-For + * @param {boolean} [options.maxIpsCount] max ips read from proxy ip header, default to 0 (means infinity) * */ @@ -49,6 +51,8 @@ module.exports = class Application extends Emitter { options = options || {}; this.proxy = options.proxy || false; this.subdomainOffset = options.subdomainOffset || 2; + this.proxyIpHeader = options.proxyIpHeader || 'X-Forwarded-For'; + this.maxIpsCount = options.maxIpsCount || 0; this.env = options.env || process.env.NODE_ENV || 'development'; if (options.keys) this.keys = options.keys; this.middleware = []; diff --git a/lib/request.js b/lib/request.js index b49d6404c..71d4a0f0b 100644 --- a/lib/request.js +++ b/lib/request.js @@ -432,10 +432,14 @@ module.exports = { get ips() { const proxy = this.app.proxy; - const val = this.get('X-Forwarded-For'); - return proxy && val + const val = this.get(this.app.proxyIpHeader); + let ips = proxy && val ? val.split(/\s*,\s*/) : []; + if (this.app.maxIpsCount > 0) { + ips = ips.slice(-this.app.maxIpsCount); + } + return ips; }, /** diff --git a/test/request/ips.js b/test/request/ips.js index 8e0a5f0c7..18ec82ea4 100644 --- a/test/request/ips.js +++ b/test/request/ips.js @@ -24,4 +24,48 @@ describe('req.ips', () => { }); }); }); + + describe('when options.proxyIpHeader is present', () => { + describe('and proxy is not trusted', () => { + it('should be ignored', () => { + const req = request(); + req.app.proxy = false; + req.app.proxyIpHeader = 'x-client-ip'; + req.header['x-client-ip'] = '127.0.0.1,127.0.0.2'; + assert.deepEqual(req.ips, []); + }); + }); + + describe('and proxy is trusted', () => { + it('should be used', () => { + const req = request(); + req.app.proxy = true; + req.app.proxyIpHeader = 'x-client-ip'; + req.header['x-client-ip'] = '127.0.0.1,127.0.0.2'; + assert.deepEqual(req.ips, ['127.0.0.1', '127.0.0.2']); + }); + }); + }); + + describe('when options.maxIpsCount is present', () => { + describe('and proxy is not trusted', () => { + it('should be ignored', () => { + const req = request(); + req.app.proxy = false; + req.app.maxIpsCount = 1; + req.header['x-forwarded-for'] = '127.0.0.1,127.0.0.2'; + assert.deepEqual(req.ips, []); + }); + }); + + describe('and proxy is trusted', () => { + it('should be used', () => { + const req = request(); + req.app.proxy = true; + req.app.maxIpsCount = 1; + req.header['x-forwarded-for'] = '127.0.0.1,127.0.0.2'; + assert.deepEqual(req.ips, ['127.0.0.2']); + }); + }); + }); });