Merging 7dde7b7 into trunk-temp/pr-799/a8978a18-6b66-408e-886c-935034…

…4ad9ca
arcjet · May 22, 2024 · 99733c1 · 99733c1
2 parents a85d18c + 7dde7b7
commit 99733c1
Show file tree

Hide file tree

Showing 4 changed files with 112 additions and 61 deletions.
diff --git a/ip/index.ts b/ip/index.ts
@@ -583,10 +583,59 @@ export interface RequestLike {
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 // SOFTWARE.
 function findIP(request: RequestLike, headers: Headers): string {
+  // Prefer anything available via the platform over headers since headers can
+  // be set by users. Only if we don't have an IP available in `request` do we
+  // search the `headers`.
   if (isGlobalIP(request.ip)) {
     return request.ip;
   }
 
+  const socketRemoteAddress = request.socket?.remoteAddress;
+  if (isGlobalIP(socketRemoteAddress)) {
+    return socketRemoteAddress;
+  }
+
+  const infoRemoteAddress = request.info?.remoteAddress;
+  if (isGlobalIP(infoRemoteAddress)) {
+    return infoRemoteAddress;
+  }
+
+  // AWS Api Gateway + Lambda
+  const requestContextIdentitySourceIP =
+    request.requestContext?.identity?.sourceIp;
+  if (isGlobalIP(requestContextIdentitySourceIP)) {
+    return requestContextIdentitySourceIP;
+  }
+
+  // Platform-specific headers should only be accepted when we can determine
+  // that we are running on that platform. For example, the `CF-Connecting-IP`
+  // header should only be accepted when running on Cloudflare; otherwise, it
+  // can be spoofed.
+
+  // Cloudflare: https://developers.cloudflare.com/workers/configuration/compatibility-dates/#global-navigator
+  if (globalThis.navigator?.userAgent === "Cloudflare-Workers") {
+    // CF-Connecting-IPv6: https://developers.cloudflare.com/fundamentals/reference/http-request-headers/#cf-connecting-ipv6
+    const cfConnectingIPv6 = headers.get("cf-connecting-ipv6");
+    if (isGlobalIPv6(cfConnectingIPv6)) {
+      return cfConnectingIPv6;
+    }
+
+    // CF-Connecting-IP: https://developers.cloudflare.com/fundamentals/reference/http-request-headers/#cf-connecting-ip
+    const cfConnectingIP = headers.get("cf-connecting-ip");
+    if (isGlobalIP(cfConnectingIP)) {
+      return cfConnectingIP;
+    }
+  }
+
+  // Fly.io: https://fly.io/docs/machines/runtime-environment/#fly_app_name
+  if (process.env["FLY_APP_NAME"] !== "") {
+    // Fly-Client-IP: https://fly.io/docs/networking/request-headers/#fly-client-ip
+    const flyClientIP = headers.get("fly-client-ip");
+    if (isGlobalIP(flyClientIP)) {
+      return flyClientIP;
+    }
+  }
+
   // Standard headers used by Amazon EC2, Heroku, and others.
   const xClientIP = headers.get("x-client-ip");
   if (isGlobalIP(xClientIP)) {
@@ -598,24 +647,15 @@ function findIP(request: RequestLike, headers: Headers): string {
   const xForwardedForItems = parseXForwardedFor(xForwardedFor);
   // As per MDN X-Forwarded-For Headers documentation at
   // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For
-  // We may find more than one IP in the `x-forwarded-for` header. We want to
-  // iterate left-to-right, since left-most IP will be closest to the client,
-  // and we'll return the first public IP in the list.
-  for (const item of xForwardedForItems) {
+  // We may find more than one IP in the `x-forwarded-for` header. Since the
+  // first IP will be closest to the user (and the most likely to be spoofed),
+  // we want to iterate tail-to-head so we reverse the list.
+  for (const item of xForwardedForItems.reverse()) {
     if (isGlobalIP(item)) {
       return item;
     }
   }
 
-  // Cloudflare.
-  // CF-Connecting-IP: https://developers.cloudflare.com/fundamentals/reference/http-request-headers/#cf-connecting-ip
-  const cfConnectingIP = headers.get("cf-connecting-ip");
-  if (isGlobalIP(cfConnectingIP)) {
-    return cfConnectingIP;
-  }
-
-  // TODO: CF-Connecting-IPv6: https://developers.cloudflare.com/fundamentals/reference/http-request-headers/#cf-connecting-ipv6
-
   // DigitalOcean.
   // DO-Connecting-IP: https://www.digitalocean.com/community/questions/app-platform-client-ip
   const doConnectingIP = headers.get("do-connecting-ip");
@@ -630,20 +670,13 @@ function findIP(request: RequestLike, headers: Headers): string {
     return fastlyClientIP;
   }
 
-  // Akamai and Cloudflare
+  // Akamai
   // True-Client-IP
   const trueClientIP = headers.get("true-client-ip");
   if (isGlobalIP(trueClientIP)) {
     return trueClientIP;
   }
 
-  // Fly.io
-  // Fly-Client-IP: https://fly.io/docs/networking/request-headers/#fly-client-ip
-  const flyClientIP = headers.get("fly-client-ip");
-  if (isGlobalIP(flyClientIP)) {
-    return flyClientIP;
-  }
-
   // Default nginx proxy/fcgi; alternative to x-forwarded-for, used by some proxies
   // X-Real-IP
   const xRealIP = headers.get("x-real-ip");
@@ -679,30 +712,6 @@ function findIP(request: RequestLike, headers: Headers): string {
     return xAppEngineUserIP;
   }
 
-  const socketRemoteAddress = request.socket?.remoteAddress;
-  if (isGlobalIP(socketRemoteAddress)) {
-    return socketRemoteAddress;
-  }
-
-  const infoRemoteAddress = request.info?.remoteAddress;
-  if (isGlobalIP(infoRemoteAddress)) {
-    return infoRemoteAddress;
-  }
-
-  // AWS Api Gateway + Lambda
-  const requestContextIdentitySourceIP =
-    request.requestContext?.identity?.sourceIp;
-  if (isGlobalIP(requestContextIdentitySourceIP)) {
-    return requestContextIdentitySourceIP;
-  }
-
-  // Cloudflare fallback
-  // Cf-Pseudo-IPv4: https://blog.cloudflare.com/eliminating-the-last-reasons-to-not-enable-ipv6/#introducingpseudoipv4
-  const cfPseudoIPv4 = headers.get("cf-pseudo-ipv4");
-  if (isGlobalIP(cfPseudoIPv4)) {
-    return cfPseudoIPv4;
-  }
-
   return "";
 }
 

diff --git a/ip/jest.config.js b/ip/jest.config.js
@@ -11,6 +11,9 @@ const config = {
   coverageProvider: "v8",
   verbose: true,
   testEnvironment: "node",
+  globals: {
+    navigator: {},
+  },
 };
 
 export default config;
diff --git a/ip/test/ipv4.test.ts b/ip/test/ipv4.test.ts
@@ -1,11 +1,31 @@
 /**
  * @jest-environment node
  */
-import { describe, expect, test, afterEach, jest } from "@jest/globals";
+import {
+  describe,
+  expect,
+  test,
+  beforeEach,
+  afterEach,
+  jest,
+} from "@jest/globals";
 import ip, { RequestLike } from "../index";
 
 type MakeTest = (ip: unknown) => [RequestLike, Headers];
 
+beforeEach(() => {
+  jest.replaceProperty(process, "env", {
+    ...process.env,
+    FLY_APP_NAME: "testing",
+  });
+  // We inject an empty `navigator` object via jest.config.js to act like
+  // Cloudflare Workers
+  jest.replaceProperty(globalThis, "navigator", {
+    ...globalThis.navigator,
+    userAgent: "Cloudflare-Workers",
+  });
+});
+
 afterEach(() => {
   jest.clearAllMocks();
   jest.restoreAllMocks();
@@ -214,39 +234,38 @@ describe("find public IPv4", () => {
   headerSuite("Forwarded-For");
   headerSuite("Forwarded");
   headerSuite("X-Appengine-User-IP");
-  headerSuite("CF-Pseudo-IPv4");
 
   describe("X-Forwarded-For with multiple IP", () => {
-    test("returns the first public IP", () => {
+    test("returns the last public IP", () => {
       const request = {};
       const headers = new Headers([
         ["X-Forwarded-For", "1.1.1.1, 2.2.2.2, 3.3.3.3"],
       ]);
-      expect(ip(request, headers)).toEqual("1.1.1.1");
+      expect(ip(request, headers)).toEqual("3.3.3.3");
     });
 
     test("skips any `unknown` IP", () => {
       const request = {};
       const headers = new Headers([
-        ["X-Forwarded-For", "unknown, 1.1.1.1, 2.2.2.2, 3.3.3.3"],
+        ["X-Forwarded-For", "1.1.1.1, 2.2.2.2, 3.3.3.3, unknown"],
       ]);
-      expect(ip(request, headers)).toEqual("1.1.1.1");
+      expect(ip(request, headers)).toEqual("3.3.3.3");
     });
 
     test("skips any private IP (in production)", () => {
       jest.replaceProperty(process.env, "NODE_ENV", "production");
       const request = {};
       const headers = new Headers([
-        ["X-Forwarded-For", "127.0.0.1, 1.1.1.1, 2.2.2.2, 3.3.3.3"],
+        ["X-Forwarded-For", "1.1.1.1, 2.2.2.2, 3.3.3.3, 127.0.0.1"],
       ]);
-      expect(ip(request, headers)).toEqual("1.1.1.1");
+      expect(ip(request, headers)).toEqual("3.3.3.3");
     });
 
     test("returns the loopback IP (in development)", () => {
       jest.replaceProperty(process.env, "NODE_ENV", "development");
       const request = {};
       const headers = new Headers([
-        ["X-Forwarded-For", "127.0.0.1, 1.1.1.1, 2.2.2.2, 3.3.3.3"],
+        ["X-Forwarded-For", "1.1.1.1, 2.2.2.2, 3.3.3.3, 127.0.0.1"],
       ]);
       expect(ip(request, headers)).toEqual("127.0.0.1");
     });

diff --git a/ip/test/ipv6.test.ts b/ip/test/ipv6.test.ts
@@ -1,11 +1,31 @@
 /**
  * @jest-environment node
  */
-import { describe, expect, test, afterEach, jest } from "@jest/globals";
+import {
+  describe,
+  expect,
+  test,
+  beforeEach,
+  afterEach,
+  jest,
+} from "@jest/globals";
 import ip, { RequestLike } from "../index";
 
 type MakeTest = (ip: unknown) => [RequestLike, Headers];
 
+beforeEach(() => {
+  jest.replaceProperty(process, "env", {
+    ...process.env,
+    FLY_APP_NAME: "testing",
+  });
+  // We inject an empty `navigator` object via jest.config.js to act like
+  // Cloudflare Workers
+  jest.replaceProperty(globalThis, "navigator", {
+    ...globalThis.navigator,
+    userAgent: "Cloudflare-Workers",
+  });
+});
+
 afterEach(() => {
   jest.clearAllMocks();
   jest.restoreAllMocks();
@@ -158,14 +178,15 @@ function headerSuite(key: string) {
   });
 }
 
-describe("find public IPv4", () => {
+describe("find public IPv6", () => {
   requestSuite("ip");
   requestSuite("socket", "remoteAddress");
   requestSuite("info", "remoteAddress");
   requestSuite("requestContext", "identity", "sourceIp");
 
   headerSuite("X-Client-IP");
   headerSuite("X-Forwarded-For");
+  headerSuite("CF-Connecting-IPv6");
   headerSuite("CF-Connecting-IP");
   headerSuite("DO-Connecting-IP");
   headerSuite("Fastly-Client-IP");
@@ -177,21 +198,20 @@ describe("find public IPv4", () => {
   headerSuite("Forwarded-For");
   headerSuite("Forwarded");
   headerSuite("X-Appengine-User-IP");
-  headerSuite("CF-Pseudo-IPv4");
 
   describe("X-Forwarded-For with multiple IP", () => {
     test("returns the first public IP", () => {
       const request = {};
       const headers = new Headers([
-        ["X-Forwarded-For", "abcd::, e123::, 3.3.3.3"],
+        ["X-Forwarded-For", "e123::, 3.3.3.3, abcd::"],
       ]);
       expect(ip(request, headers)).toEqual("abcd::");
     });
 
     test("skips any `unknown` IP", () => {
       const request = {};
       const headers = new Headers([
-        ["X-Forwarded-For", "unknown, abcd::, e123::, 3.3.3.3"],
+        ["X-Forwarded-For", "e123::, 3.3.3.3, abcd::, unknown"],
       ]);
       expect(ip(request, headers)).toEqual("abcd::");
     });
@@ -200,7 +220,7 @@ describe("find public IPv4", () => {
       jest.replaceProperty(process.env, "NODE_ENV", "production");
       const request = {};
       const headers = new Headers([
-        ["X-Forwarded-For", "::1, abcd::, e123::, 3.3.3.3"],
+        ["X-Forwarded-For", "e123::, 3.3.3.3, abcd::, ::1"],
       ]);
       expect(ip(request, headers)).toEqual("abcd::");
     });
@@ -209,7 +229,7 @@ describe("find public IPv4", () => {
       jest.replaceProperty(process.env, "NODE_ENV", "development");
       const request = {};
       const headers = new Headers([
-        ["X-Forwarded-For", "::1, abcd::, e123::, 3.3.3.3"],
+        ["X-Forwarded-For", "abcd::, e123::, 3.3.3.3, ::1"],
       ]);
       expect(ip(request, headers)).toEqual("::1");
     });