fix(agent): resolve the ip whitelist client ip through trusted proxies

packages/agent/src/routes/security/client-ip.ts

@@ -0,0 +1,36 @@
+import { BlockList, isIPv6 } from 'net';
+
+const TRUSTED_PROXY_SUBNETS: [ip: string, prefix: number, type: 'ipv4' | 'ipv6'][] = [
+  ['127.0.0.0', 8, 'ipv4'],
+  ['10.0.0.0', 8, 'ipv4'],
+  ['172.16.0.0', 12, 'ipv4'],
+  ['192.168.0.0', 16, 'ipv4'],
+  ['169.254.0.0', 16, 'ipv4'],
+  ['::1', 128, 'ipv6'],
+  ['fc00::', 7, 'ipv6'],
+  ['fe80::', 10, 'ipv6'],
+];
+
+const trustedProxies = new BlockList();
+TRUSTED_PROXY_SUBNETS.forEach(([ip, prefix, type]) => trustedProxies.addSubnet(ip, prefix, type));
+
+export const ipVersion = (ip: string): 'ipv4' | 'ipv6' => (isIPv6(ip) ? 'ipv6' : 'ipv4');
+
+const isTrustedProxy = (ip: string): boolean => trustedProxies.check(ip.trim(), ipVersion(ip));
+
+const resolveClientIp = (socketIp: string, forwardedFor?: string | string[]): string => {
+  const header = `${forwardedFor ?? ''}`;
+  const hops = header === '' ? [] : header.split(',').map(hop => hop.trim());
+
+  hops.push(socketIp.trim());
+
+  let index = hops.length - 1;
+
+  while (index > 0 && isTrustedProxy(hops[index])) {
+    index -= 1;
+  }
+
+  return hops[index];
+};
+
+export default resolveClientIp;

packages/agent/src/routes/security/ip-whitelist.ts

@@ -2,17 +2,16 @@ import type { IpWhitelistConfiguration } from '@forestadmin/forestadmin-client';
 import type Router from '@koa/router';
 import type { Context, Next } from 'koa';
 
-import { BlockList, isIPv6 } from 'net';
+import { BlockList } from 'net';
 
 import { HttpCode, RouteType } from '../../types';
 import BaseRoute from '../base-route';
+import resolveClientIp, { ipVersion } from './client-ip';
 
 type IpRule = IpWhitelistConfiguration['ipRules'][number];
 
 const LOOPBACK_IPS = ['127.0.0.1', '::1', '::ffff:127.0.0.1'];
 
-const ipVersion = (ip: string): 'ipv4' | 'ipv6' => (isIPv6(ip) ? 'ipv6' : 'ipv4');
-
 export default class IpWhitelist extends BaseRoute {
   type = RouteType.Authentication;
 
@@ -58,7 +57,10 @@ export default class IpWhitelist extends BaseRoute {
         return next();
       }
 
-      const currentIp = `${context.request.headers['x-forwarded-for'] ?? context.request.ip}`;
+      const currentIp = resolveClientIp(
+        context.req?.socket?.remoteAddress ?? '',
+        context.request.headers['x-forwarded-for'],
+      );
 
       if (!this.allowedIps.check(currentIp, ipVersion(currentIp))) {
         return context.throw(HttpCode.Forbidden, `IP address rejected (${currentIp})`);

packages/agent/test/routes/security/client-ip.test.ts

@@ -0,0 +1,90 @@
+import resolveClientIp from '../../../src/routes/security/client-ip';
+
+describe('resolveClientIp', () => {
+  describe('when the socket peer is a public address', () => {
+    test.each(['9.9.9.9', '', '9.9.9.9, 10.20.15.10', '127.0.0.1'])(
+      'resolves to the socket peer with header %s',
+      forwardedFor => {
+        expect(resolveClientIp('203.0.113.77', forwardedFor)).toBe('203.0.113.77');
+      },
+    );
+
+    test('resolves to the socket peer when there is no header', () => {
+      expect(resolveClientIp('203.0.113.77')).toBe('203.0.113.77');
+    });
+
+    test('resolves to an ipv6 socket peer', () => {
+      expect(resolveClientIp('2001:db8::1', '9.9.9.9')).toBe('2001:db8::1');
+    });
+
+    test('resolves to an ipv4-mapped public socket peer', () => {
+      expect(resolveClientIp('::ffff:203.0.113.5', '9.9.9.9')).toBe('::ffff:203.0.113.5');
+    });
+  });
+
+  describe('when the socket peer is a trusted proxy', () => {
+    test('resolves to the single hop behind a private proxy', () => {
+      expect(resolveClientIp('10.0.0.2', '9.9.9.9')).toBe('9.9.9.9');
+    });
+
+    test('resolves to the rightmost untrusted hop of a multi-hop header', () => {
+      expect(resolveClientIp('10.0.0.2', '9.9.9.9, 203.0.113.77')).toBe('203.0.113.77');
+    });
+
+    test('walks a chain of trusted proxies from the right', () => {
+      expect(resolveClientIp('127.0.0.1', '9.9.9.9, 10.0.0.3')).toBe('9.9.9.9');
+    });
+
+    test('returns the leftmost hop when the whole chain is trusted', () => {
+      expect(resolveClientIp('127.0.0.1', '10.0.0.3, 192.168.1.1')).toBe('10.0.0.3');
+    });
+
+    test.each([
+      ['10.0.0.2', '172.16.0.5'],
+      ['10.0.0.2', '192.168.1.1'],
+      ['10.0.0.2', '127.0.0.99'],
+      ['10.0.0.2', '169.254.1.1'],
+      ['10.0.0.2', '::1'],
+      ['fd00::1', '2001:db8::1'],
+      ['fe80::1', '2001:db8::1'],
+      ['10.0.0.2', 'fd12::1'],
+      ['::ffff:10.0.0.2', '203.0.113.77'],
+    ])('resolves to the hop behind trusted proxy socket %s', (socketIp, expected) => {
+      expect(resolveClientIp(socketIp, expected)).toBe(expected);
+    });
+
+    test.each(['::ffff:10.0.0.3', '::ffff:a00:3', '::FFFF:10.0.0.3'])(
+      'skips the ipv4-mapped trusted proxy hop %s while walking',
+      mappedProxy => {
+        expect(resolveClientIp('10.0.0.2', `203.0.113.77, ${mappedProxy}`)).toBe('203.0.113.77');
+      },
+    );
+
+    test('trims the hops whatever the spacing', () => {
+      expect(resolveClientIp('  10.0.0.2 ', '  203.0.113.77 ,  10.0.0.3  ')).toBe('203.0.113.77');
+    });
+
+    test('joins a repeated header and walks it from the right', () => {
+      expect(resolveClientIp('10.0.0.2', ['203.0.113.77', '10.0.0.3'])).toBe('203.0.113.77');
+    });
+  });
+
+  describe('when a hop is not a valid address', () => {
+    test('returns a malformed hop met during the walk instead of skipping it', () => {
+      expect(resolveClientIp('10.0.0.2', '9.9.9.9, not-an-ip')).toBe('not-an-ip');
+    });
+
+    test('returns a hop carrying a port instead of skipping it', () => {
+      expect(resolveClientIp('10.0.0.2', '203.0.113.77:1234')).toBe('203.0.113.77:1234');
+    });
+
+    test('returns an empty slot met during the walk', () => {
+      expect(resolveClientIp('10.0.0.2', ', 10.0.0.3')).toBe('');
+    });
+
+    test('returns an empty string when the socket cannot be read and no header exists', () => {
+      expect(resolveClientIp('')).toBe('');
+      expect(resolveClientIp('', undefined)).toBe('');
+    });
+  });
+});