improve proxy trust with ip address list

closes #2099

History.md

@@ -1,6 +1,14 @@
 3.x
 ===
 
+ * proper proxy trust with `app.set('trust proxy', trust)`
+   - `app.set('trust proxy', 1)` trust first hop
+   - `app.set('trust proxy', 'loopback')` trust loopback addresses
+   - `app.set('trust proxy', '10.0.0.1')` trust single IP
+   - `app.set('trust proxy', '10.0.0.1/16')` trust subnet
+   - `app.set('trust proxy', '10.0.0.1, 10.0.0.2')` trust list
+   - `app.set('trust proxy', false)` turn off
+   - `app.set('trust proxy', true)` trust everything
  * update connect to 2.16.1
    - deprecate `res.headerSent` -- use `res.headersSent`
    - deprecate `res.on("header")` -- use on-headers module instead

lib/application.js

@@ -8,6 +8,7 @@ var connect = require('connect')
   , middleware = require('./middleware')
   , debug = require('debug')('express:application')
   , locals = require('./utils').locals
+  , compileTrust = require('./utils').compileTrust
   , View = require('./view')
   , utils = connect.utils
   , deprecate = require('./utils').deprecate
@@ -48,6 +49,7 @@ app.defaultConfiguration = function(){
   this.enable('etag');
   this.set('env', process.env.NODE_ENV || 'development');
   this.set('subdomain offset', 2);
+  this.set('trust proxy', false);
   debug('booting in %s mode', this.get('env'));
 
   // implicit middleware
@@ -253,6 +255,12 @@ app.set = function(setting, val){
     return this.settings[setting];
   } else {
     this.settings[setting] = val;
+
+    if (setting === 'trust proxy') {
+      debug('compile trust proxy %j', val);
+      this.set('trust proxy fn', compileTrust(val));
+    }
+
     return this;
   }
 };

lib/request.js

@@ -9,6 +9,7 @@ var http = require('http')
   , fresh = require('fresh')
   , parseRange = require('range-parser')
   , parse = require('parseurl')
+  , proxyaddr = require('proxy-addr')
   , mime = connect.mime;
 
 /**
@@ -337,19 +338,26 @@ req.is = function(type){
 /**
  * Return the protocol string "http" or "https"
  * when requested with TLS. When the "trust proxy"
- * setting is enabled the "X-Forwarded-Proto" header
- * field will be trusted. If you're running behind
- * a reverse proxy that supplies https for you this
- * may be enabled.
+ * setting trusts the socket address, the
+ * "X-Forwarded-Proto" header field will be trusted.
+ * If you're running behind a reverse proxy that
+ * supplies https for you this may be enabled.
  *
  * @return {String}
  * @api public
  */
 
 req.__defineGetter__('protocol', function(){
-  var trustProxy = this.app.get('trust proxy');
-  if (this.connection.encrypted) return 'https';
-  if (!trustProxy) return 'http';
+  var trust = this.app.get('trust proxy fn');
+
+  if (!trust(this.connection.remoteAddress)) {
+    return this.connection.encrypted
+      ? 'https'
+      : 'http';
+  }
+
+  // Note: X-Forwarded-Proto is normally only ever a
+  //       single value, but this is to be safe.
   var proto = this.get('X-Forwarded-Proto') || 'http';
   return proto.split(/\s*,\s*/)[0];
 });
@@ -368,36 +376,36 @@ req.__defineGetter__('secure', function(){
 });
 
 /**
- * Return the remote address, or when
- * "trust proxy" is `true` return
- * the upstream addr.
+ * Return the remote address from the trusted proxy.
+ *
+ * The is the remote address on the socket unless
+ * "trust proxy" is set.
  *
  * @return {String}
  * @api public
  */
 
 req.__defineGetter__('ip', function(){
-  return this.ips[0] || this.connection.remoteAddress;
+  var trust = this.app.get('trust proxy fn');
+  return proxyaddr(this, trust);
 });
 
 /**
- * When "trust proxy" is `true`, parse
- * the "X-Forwarded-For" ip address list.
+ * When "trust proxy" is set, trusted proxy addresses + client.
  *
  * For example if the value were "client, proxy1, proxy2"
  * you would receive the array `["client", "proxy1", "proxy2"]`
- * where "proxy2" is the furthest down-stream.
+ * where "proxy2" is the furthest down-stream and "proxy1" and
+ * "proxy2" were trusted.
  *
  * @return {Array}
  * @api public
  */
 
 req.__defineGetter__('ips', function(){
-  var trustProxy = this.app.get('trust proxy');
-  var val = this.get('X-Forwarded-For');
-  return trustProxy && val
-    ? val.split(/ *, */)
-    : [];
+  var trust = this.app.get('trust proxy fn');
+  var addrs = proxyaddr.all(this, trust);
+  return addrs.slice(1).reverse();
 });
 
 /**
@@ -467,19 +475,30 @@ req.__defineGetter__('path', function(){
 /**
  * Parse the "Host" header field hostname.
  *
+ * When the "trust proxy" setting trusts the socket
+ * address, the "X-Forwarded-Host" header field will
+ * be trusted.
+ *
  * @return {String}
  * @api public
  */
 
 req.__defineGetter__('host', function(){
-  var trustProxy = this.app.get('trust proxy');
-  var host = trustProxy && this.get('X-Forwarded-Host');
-  host = host || this.get('Host');
+  var trust = this.app.get('trust proxy fn');
+  var host = this.get('X-Forwarded-Host');
+
+  if (!host || !trust(this.connection.remoteAddress)) {
+    host = this.get('Host');
+  }
+
   if (!host) return;
+
+  // IPv6 literal support
   var offset = host[0] === '['
     ? host.indexOf(']') + 1
     : 0;
   var index = host.indexOf(':', offset);
+
   return ~index
     ? host.substring(0, index)
     : host;

lib/utils.js

@@ -5,6 +5,7 @@
 
 var mime = require('connect').mime
   , deprecate = require('util').deprecate
+  , proxyaddr = require('proxy-addr')
   , crc32 = require('buffer-crc32');
 
 /**
@@ -337,3 +338,32 @@ exports.pathRegexp = function(path, keys, sensitive, strict) {
     .replace(/\*/g, '(.*)');
   return new RegExp('^' + path + '$', sensitive ? '' : 'i');
 }
+
+/**
+ * Compile "proxy trust" value to function.
+ *
+ * @param  {Boolean|String|Number|Array|Function} val
+ * @return {Function}
+ * @api private
+ */
+
+exports.compileTrust = function(val) {
+  if (typeof val === 'function') return val;
+
+  if (val === true) {
+    // Support plain true/false
+    return function(){ return true };
+  }
+
+  if (typeof val === 'number') {
+    // Support trusting hop count
+    return function(a, i){ return i < val };
+  }
+
+  if (typeof val === 'string') {
+    // Support comma-separated values
+    val = val.split(/ *, */);
+  }
+
+  return proxyaddr.compile(val || []);
+}

package.json

@@ -39,6 +39,7 @@
     "methods": "1.0.0",
     "mkdirp": "0.5.0",
     "parseurl": "1.0.1",
+    "proxy-addr": "1.0.0",
     "range-parser": "1.0.0",
     "cookie": "0.1.2",
     "buffer-crc32": "0.2.1",

test/req.host.js

@@ -69,5 +69,70 @@ describe('req', function(){
       .set('Host', '[::1]:3000')
       .expect('[::1]', done);
     })
+
+    describe('when "trust proxy" is enabled', function(){
+      it('should respect X-Forwarded-Host', function(done){
+        var app = express();
+
+        app.enable('trust proxy');
+
+        app.use(function(req, res){
+          res.end(req.host);
+        });
+
+        request(app)
+        .get('/')
+        .set('Host', 'localhost')
+        .set('X-Forwarded-Host', 'example.com')
+        .expect('example.com', done);
+      })
+
+      it('should ignore X-Forwarded-Host if socket addr not trusted', function(done){
+        var app = express();
+
+        app.set('trust proxy', '10.0.0.1');
+
+        app.use(function(req, res){
+          res.end(req.host);
+        });
+
+        request(app)
+        .get('/')
+        .set('Host', 'localhost')
+        .set('X-Forwarded-Host', 'example.com')
+        .expect('localhost', done);
+      })
+
+      it('should default to Host', function(done){
+        var app = express();
+
+        app.enable('trust proxy');
+
+        app.use(function(req, res){
+          res.end(req.host);
+        });
+
+        request(app)
+        .get('/')
+        .set('Host', 'example.com')
+        .expect('example.com', done);
+      })
+    })
+
+    describe('when "trust proxy" is disabled', function(){
+      it('should ignore X-Forwarded-Host', function(done){
+        var app = express();
+
+        app.use(function(req, res){
+          res.end(req.host);
+        });
+
+        request(app)
+        .get('/')
+        .set('Host', 'localhost')
+        .set('X-Forwarded-Host', 'evil')
+        .expect('localhost', done);
+      })
+    })
   })
 })

test/req.ip.js

@@ -20,6 +20,21 @@ describe('req', function(){
           .set('X-Forwarded-For', 'client, p1, p2')
           .expect('client', done);
         })
+
+        it('should return the addr after trusted proxy', function(done){
+          var app = express();
+
+          app.set('trust proxy', 2);
+
+          app.use(function(req, res, next){
+            res.send(req.ip);
+          });
+
+          request(app)
+          .get('/')
+          .set('X-Forwarded-For', 'client, p1, p2')
+          .expect('p1', done);
+        })
       })
 
       describe('when "trust proxy" is disabled', function(){

test/req.ips.js

@@ -20,6 +20,21 @@ describe('req', function(){
           .set('X-Forwarded-For', 'client, p1, p2')
           .expect('["client","p1","p2"]', done);
         })
+
+        it('should stop at first untrusted', function(done){
+          var app = express();
+
+          app.set('trust proxy', 2);
+
+          app.use(function(req, res, next){
+            res.send(req.ips);
+          });
+
+          request(app)
+          .get('/')
+          .set('X-Forwarded-For', 'client, p1, p2')
+          .expect('["p1","p2"]', done);
+        })
       })
 
       describe('when "trust proxy" is disabled', function(){

test/req.protocol.js

@@ -32,6 +32,21 @@ describe('req', function(){
         .expect('https', done);
       })
 
+      it('should ignore X-Forwarded-Proto if socket addr not trusted', function(done){
+        var app = express();
+
+        app.set('trust proxy', '10.0.0.1');
+
+        app.use(function(req, res){
+          res.end(req.protocol);
+        });
+
+        request(app)
+        .get('/')
+        .set('X-Forwarded-Proto', 'https')
+        .expect('http', done);
+      })
+
       it('should default to http', function(done){
         var app = express();