Skip to content

Commit

Permalink
fix(ip)!: Rework priority of IP detection (#799)
Browse files Browse the repository at this point in the history 
Closes #798 

This is a breaking change to how we detect IP from headers. I've changed it to prefer IP values on a "request-like" object, such as `ip` or `socket.remoteAddress`.

I also prioritized `CF-Connecting-IP` and `Fly-Client-IP` over `X-Forwarded-For` headers but they are only used when actually deployed to Cloudflare or Fly respectively. We'll want to guard other platforms where an environment variable is detectable but I couldn't others in some cursory research.

Additionally, we now iterate the `X-Forwarded-For` IPs in reverse since the first items are the easiest to spoof. For example, a user can just submit whatever IP address as the first entry in `X-Forwarded-For` along with their request.

All of the above should be documented via code comments too.

Oh, and I removed the `CF-Pseudo-IPv4` header because it will always be wrong since Cloudflare generates a non-public IP address as the translation of the IPv6 so we're preferring `CF-Connecting-IPv6` correctly now.
  • Loading branch information
@blaine-arcjet
blaine-arcjet authored May 22, 2024
1 parent a85d18c commit 1df6291
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 61 deletions.
99 changes: 54 additions & 45 deletions ip/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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");
Expand All @@ -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");
Expand Down Expand Up @@ -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 "";
}

Expand Down
3 changes: 3 additions & 0 deletions ip/jest.config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ const config = {
coverageProvider: "v8",
verbose: true,
testEnvironment: "node",
globals: {
navigator: {},
},
};

export default config;
37 changes: 28 additions & 9 deletions ip/test/ipv4.test.ts
View file
Original file line number Diff line number Diff line change
@@ -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();
Expand Down Expand Up @@ -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");
});
Expand Down
34 changes: 27 additions & 7 deletions ip/test/ipv6.test.ts
View file
Original file line number Diff line number Diff line change
@@ -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();
Expand Down Expand Up @@ -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");
Expand All @@ -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::");
});
Expand All @@ -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::");
});
Expand All @@ -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");
});
Expand Down

0 comments on commit 1df6291

Please sign in to comment.