Not blocking requests from "unallowed" CIDR addresses

[stephent]

I'm trying to whitelist some CIDR blocks (Fastly CDN, to be specific) such that request from any other IPs are denied. However, requests from any IP appear to be allowed. Am I missing something obvious?

` var listenCallback = function(/err/) {

var port = Number(process.env.PORT || 3005); app.listen(port, function () { winston.info('Elevation proxy is listening on port ' + port); }); };

var whitelistFastly = function(cb) {

var options = { method: 'GET', uri: 'https://api.fastly.com/public-ip-list' };

winston.debug('Requesting Fastly IP Addresses:', options); request(options, function (error, response, body) {

if (error) {
  winston.error('Error received for Fastly IPs request:', error);
  return cb(err);
}

if (200 !== response.statusCode) {
  // No error, but not a 200 either
  winston.warn('Fastly IPs response NOT OK:', options.uri, response.statusCode);
  return cb();
}

var ips = JSON.parse(body).addresses;
winston.info('Whitelisting Fastly IP addresses:', ips);

app.use(ipfilter(ips, {mode: 'allow', allowedHeaders: ['X-Forwarded-For']}));

cb();

}); };

// Start the server once we have Fastly whitelisted whitelistFastly(listenCallback);`

This is with express-ipfilter 0.2.0 running on node 6.9.1, express 4.14.0.

Thanks.

[longstone]

I used the following testcase to verify the issue:

describe('should block other ips than allowed when using cidr', function(){
  var ips = [ '23.235.32.0/20',
    '43.249.72.0/22',
    '103.244.50.0/24',
    '103.245.222.0/23',
    '103.245.224.0/24',
    '104.156.80.0/20',
    '151.101.0.0/16',
    '157.52.64.0/18',
    '172.111.64.0/18',
    '185.31.16.0/22',
    '199.27.72.0/21',
    '199.232.0.0/16',
    '202.21.128.0/24',
    '203.57.145.0/24' ];

  beforeEach(function () {
    this.ipfilter = ipfilter(ips, { mode: 'allow', allowedHeaders: ['x-forwarded-for'] });
    this.req = {
      session: {},
      headers: [],
      connection: {
        remoteAddress: ''
      }
    };
  });

  it('should allow all whitelisted ips', function (done) {
    this.req.connection.remoteAddress= '202.21.128.1';
    this.ipfilter(this.req, {}, function () {
      done();
    });
  });

  it('should allow all whitelisted forwarded ips', function (done) {
    this.req.headers['x-forwarded-for'] = '202.21.128.1';
    this.ipfilter(this.req, {}, function () {
      done();
    });
  });

  it('should deny all not whitelisted ips', function (done) {
    this.req.connection.remoteAddress = '11.12.13.14';
    checkError(this.ipfilter, this.req, done);
  });

  it('should deny all not whitelisted forwarded ips', function (done) {
    this.req.headers['x-forwarded-for'] = '11.12.13.14';
    checkError(this.ipfilter, this.req, done);
  });
});

which leads to following result:

  should block other ips than allowed when using cidr
Access granted to IP address: 202.21.128.1
    ✓ should allow all whitelisted ips
Access granted to IP address: 202.21.128.1
    ✓ should allow all whitelisted forwarded ips
Access denied to IP address: 11.12.13.14
    ✓ should deny all not whitelisted ips
Access denied to IP address: 11.12.13.14
    ✓ should deny all not whitelisted forwarded ips

could it be an instanciating problem? does the same error happen if you use ipfilter in a static way?

[stephent]

@longstone could you give me an example of what you mean by use ipfilter in a static way? Happy to give it a try.

[longstone]

well, passing the array without loding it dynamic, when you set up express: ex

app.use(...
app.use(ipfilter(ips, { mode: 'allow', allowedHeaders: ['x-forwarded-for'] }));
app.use('/', routes);

I'm wondering why it works in test environment....

[stephent]

Finally worked this one out - the problem is caused by the sequence of calls to app.use. If I add ipfilter first after the call to express(), things work as expected. If I add it last (but before the call to app.listen), then the ipfilter middleware is never called. (Probably some lack of understanding on my part.)

[ryanbillingsley]

@stephent it sounds like something further up in the middleware stack is handling the request and sending a response which will short circuit the middleware chain I believe.

Glad you found a solution though. If you have further problems feel free to reopen the issue.