feat: support app.proxyIPHeader and app.maxIpsCount to make ctx.ips more security

Author: dead-horse

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

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
 

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 = [];

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;
   },
 
   /**

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']);
+      });
+    });
+  });
 });