Update to latest QA tests (v1.0.4)

.github/workflows/qa-tests.yml

@@ -49,7 +49,7 @@ jobs:
           cp firewall-node/.github/workflows/Dockerfile.qa zen-demo-nodejs/Dockerfile
 
       - name: Run Firewall QA Tests
-        uses: AikidoSec/firewall-tester-action@releases/v1
+        uses: AikidoSec/firewall-tester-action@v1.0.4
         with:
           dockerfile_path: ./zen-demo-nodejs/Dockerfile
           app_port: 3000

end2end/tests/hono-xml-blocklists.test.js

@@ -300,11 +300,8 @@ t.test("it does not block bypass IP if in blocklist", (t) => {
           "X-Forwarded-For": "1.3.2.2",
         },
       });
-      t.same(resp3.status, 403);
-      t.same(
-        await resp3.text(),
-        `Your IP address is not allowed to access this resource. (Your IP: 1.3.2.2)`
-      );
+      t.same(resp3.status, 200);
+      t.match(await resp3.text(), "Admin panel");
     })
     .catch((error) => {
       t.fail(error);

library/agent/Agent.ts

@@ -33,6 +33,7 @@ import { isNewInstrumentationUnitTest } from "../helpers/isNewInstrumentationUni
 import { AttackWaveDetector } from "../vulnerabilities/attack-wave-detection/AttackWaveDetector";
 import type { FetchListsAPI } from "./api/FetchListsAPI";
 import { PendingEvents } from "./PendingEvents";
+import { domainToUnicode } from "node:url";
 
 type WrappedPackage = { version: string | null; supported: boolean };
 
@@ -576,6 +577,14 @@ export class Agent {
   }
 
   onConnectHostname(hostname: string, port: number) {
+    try {
+      // new URL(...) always converts hostnames to punycode
+      // When reporting them in heartbeats, we want to send the unicode version
+      hostname = domainToUnicode(hostname);
+    } catch (e: any) {
+      this.logger.log(`Failed to convert hostname to unicode: ${e.message}`);
+    }
+
     this.hostnames.add(hostname, port);
   }
 

library/agent/ServiceConfig.ts

@@ -4,6 +4,7 @@ import { isPrivateIP } from "../vulnerabilities/ssrf/isPrivateIP";
 import type { Endpoint, EndpointConfig, Domain } from "./Config";
 import type { IPList, UserAgentDetails } from "./api/FetchListsAPI";
 import { safeCreateRegExp } from "./safeCreateRegExp";
+import { addIPv4MappedAddresses } from "../helpers/addIPv4MappedAddresses";
 
 export class ServiceConfig {
   private blockedUserIds: Map<string, string> = new Map();
@@ -99,7 +100,9 @@ export class ServiceConfig {
       this.bypassedIPAddresses = undefined;
       return;
     }
-    this.bypassedIPAddresses = new IPMatcher(ipAddresses);
+    this.bypassedIPAddresses = new IPMatcher(
+      addIPv4MappedAddresses(ipAddresses)
+    );
   }
 
   isBypassedIP(ip: string) {

library/agent/applyHooks.test.ts

@@ -88,74 +88,79 @@ t.test(
   }
 );
 
-t.test("it ignores route if force protection off is on", async (t) => {
-  const inspectionCalls: { args: unknown[] }[] = [];
+t.test(
+  "it still inspects outbound connections if force protection off is on",
+  async (t) => {
+    const inspectionCalls: { args: unknown[] }[] = [];
 
-  const hooks = new Hooks();
-  hooks.addBuiltinModule("dns/promises").onRequire((exports, pkgInfo) => {
-    wrapExport(exports, "lookup", pkgInfo, {
-      kind: "outgoing_http_op",
-      inspectArgs: (args, agent) => {
-        inspectionCalls.push({ args });
-      },
+    const hooks = new Hooks();
+    hooks.addBuiltinModule("dns/promises").onRequire((exports, pkgInfo) => {
+      wrapExport(exports, "lookup", pkgInfo, {
+        kind: "outgoing_http_op",
+        inspectArgs: (args, agent) => {
+          inspectionCalls.push({ args });
+        },
+      });
     });
-  });
 
-  applyHooks(hooks, agent.isUsingNewInstrumentation());
+    applyHooks(hooks, agent.isUsingNewInstrumentation());
 
-  reportingAPI.setResult({
-    success: true,
-    endpoints: [
-      {
-        method: "GET",
-        route: "/route",
-        forceProtectionOff: true,
-        rateLimiting: {
-          enabled: false,
-          maxRequests: 0,
-          windowSizeInMS: 0,
+    reportingAPI.setResult({
+      success: true,
+      endpoints: [
+        {
+          method: "GET",
+          route: "/route",
+          forceProtectionOff: true,
+          rateLimiting: {
+            enabled: false,
+            maxRequests: 0,
+            windowSizeInMS: 0,
+          },
         },
-      },
-    ],
-    heartbeatIntervalInMS: 10 * 60 * 1000,
-    blockedUserIds: [],
-    allowedIPAddresses: [],
-    configUpdatedAt: 0,
-  });
+      ],
+      heartbeatIntervalInMS: 10 * 60 * 1000,
+      blockedUserIds: [],
+      allowedIPAddresses: [],
+      configUpdatedAt: 0,
+    });
 
-  // Read rules from API
-  await agent.flushStats(1000);
+    // Read rules from API
+    await agent.flushStats(1000);
 
-  const { lookup } = require("dns/promises");
+    const { lookup } = require("dns/promises");
 
-  await lookup("www.google.com");
-  t.same(inspectionCalls, [{ args: ["www.google.com"] }]);
+    await lookup("www.google.com");
+    t.same(inspectionCalls, [{ args: ["www.google.com"] }]);
 
-  await runWithContext(context, async () => {
-    await lookup("www.aikido.dev");
-  });
+    await runWithContext(context, async () => {
+      await lookup("www.aikido.dev");
+    });
 
-  t.same(inspectionCalls, [
-    { args: ["www.google.com"] },
-    { args: ["www.aikido.dev"] },
-  ]);
-
-  await runWithContext(
-    {
-      ...context,
-      method: "GET",
-      route: "/route",
-    },
-    async () => {
-      await lookup("www.times.com");
-    }
-  );
+    t.same(inspectionCalls, [
+      { args: ["www.google.com"] },
+      { args: ["www.aikido.dev"] },
+    ]);
 
-  t.same(inspectionCalls, [
-    { args: ["www.google.com"] },
-    { args: ["www.aikido.dev"] },
-  ]);
-});
+    // forceProtectionOff still allows outbound connection inspection
+    await runWithContext(
+      {
+        ...context,
+        method: "GET",
+        route: "/route",
+      },
+      async () => {
+        await lookup("www.times.com");
+      }
+    );
+
+    t.same(inspectionCalls, [
+      { args: ["www.google.com"] },
+      { args: ["www.aikido.dev"] },
+      { args: ["www.times.com"] },
+    ]);
+  }
+);
 
 t.test("it does not report attack if IP is allowed", async (t) => {
   const hooks = new Hooks();

library/agent/hooks/wrapExport.ts

@@ -3,7 +3,7 @@ import type { Agent } from "../Agent";
 import { getInstance } from "../AgentSingleton";
 import { OperationKind } from "../api/Event";
 import { bindContext, getContext } from "../Context";
-import type { InterceptorResult } from "./InterceptorResult";
+import { type InterceptorResult, isAttackResult } from "./InterceptorResult";
 import type { PartialWrapPackageInfo } from "./WrapPackageInfo";
 import { wrapDefaultOrNamed } from "./wrapDefaultOrNamed";
 import { onInspectionInterceptorResult } from "./onInspectionInterceptorResult";
@@ -151,14 +151,6 @@ export function inspectArgs(
   methodName: string,
   kind: OperationKind | undefined
 ) {
-  if (context) {
-    const matches = agent.getConfig().getEndpoints(context);
-
-    if (matches.find((match) => match.forceProtectionOff)) {
-      return;
-    }
-  }
-
   const start = performance.now();
   let result: InterceptorResult = undefined;
 
@@ -177,6 +169,16 @@ export function inspectArgs(
     });
   }
 
+  // When forceProtectionOff is enabled, skip attack detection
+  // but still allow outbound connection blocking
+  if (context && isAttackResult(result)) {
+    const matches = agent.getConfig().getEndpoints(context);
+
+    if (matches.find((match) => match.forceProtectionOff)) {
+      return;
+    }
+  }
+
   onInspectionInterceptorResult(
     context,
     agent,

library/helpers/addIPv4MappedAddresses.test.ts

@@ -0,0 +1,21 @@
+import * as t from "tap";
+import { addIPv4MappedAddresses } from "./addIPv4MappedAddresses";
+
+t.test("it adds IPv4-mapped IPv6 addresses", async (t) => {
+  t.same(
+    addIPv4MappedAddresses([
+      "1.2.3.4",
+      "23.45.67.89/24",
+      "2606:2800:220:1:248:1893:25c8:1946",
+      "2001:0db9:abcd:1234::/64",
+    ]),
+    [
+      "1.2.3.4",
+      "23.45.67.89/24",
+      "2606:2800:220:1:248:1893:25c8:1946",
+      "2001:0db9:abcd:1234::/64",
+      "::ffff:1.2.3.4/128",
+      "::ffff:23.45.67.89/120",
+    ]
+  );
+});

library/helpers/addIPv4MappedAddresses.ts

@@ -0,0 +1,10 @@
+import mapIPv4ToIPv6 from "./mapIPv4ToIPv6";
+
+/**
+ * Adds IPv4-mapped IPv6 versions for all IPv4 addresses in the array.
+ * e.g. ["1.2.3.4", "2001:db8::/32"] -> ["1.2.3.4", "2001:db8::/32", "::ffff:1.2.3.4/128"]
+ */
+export function addIPv4MappedAddresses(ips: string[]): string[] {
+  const ipv4Addresses = ips.filter((ip) => !ip.includes(":"));
+  return ips.concat(ipv4Addresses.map(mapIPv4ToIPv6));
+}

library/middleware/shouldBlockRequest.ts

@@ -33,6 +33,14 @@ export function shouldBlockRequest(): Result {
   updateContext(context, "executedMiddleware", true);
   agent.onMiddlewareExecuted();
 
+  const isBypassedIP =
+    context.remoteAddress &&
+    agent.getConfig().isBypassedIP(context.remoteAddress);
+
+  if (isBypassedIP) {
+    return { block: false };
+  }
+
   if (context.user && agent.getConfig().isUserBlocked(context.user.id)) {
     return { block: true, type: "blocked", trigger: "user" };
   }

library/ratelimiting/shouldRateLimitRequest.test.ts

@@ -182,37 +182,6 @@ t.test("it rate limits localhost when not in production mode", async (t) => {
   });
 });
 
-t.test("it does not rate limit when the IP is allowed", async (t) => {
-  const agent = await createAgent(
-    [
-      {
-        method: "POST",
-        route: "/login",
-        forceProtectionOff: false,
-        rateLimiting: {
-          enabled: true,
-          maxRequests: 3,
-          windowSizeInMS: 1000,
-        },
-      },
-    ],
-    ["1.2.3.4"]
-  );
-
-  t.same(shouldRateLimitRequest(createContext("1.2.3.4"), agent), {
-    block: false,
-  });
-  t.same(shouldRateLimitRequest(createContext("1.2.3.4"), agent), {
-    block: false,
-  });
-  t.same(shouldRateLimitRequest(createContext("1.2.3.4"), agent), {
-    block: false,
-  });
-  t.same(shouldRateLimitRequest(createContext("1.2.3.4"), agent), {
-    block: false,
-  });
-});
-
 t.test("it rate limits by user", async (t) => {
   const agent = await createAgent([
     {
@@ -437,40 +406,6 @@ t.test(
   }
 );
 
-t.test(
-  "it does not rate limit requests from allowed ip with user",
-  async (t) => {
-    const agent = await createAgent(
-      [
-        {
-          method: "POST",
-          route: "/login",
-          forceProtectionOff: false,
-          rateLimiting: {
-            enabled: true,
-            maxRequests: 3,
-            windowSizeInMS: 1000,
-          },
-        },
-      ],
-      ["1.2.3.4"]
-    );
-
-    t.same(shouldRateLimitRequest(createContext("1.2.3.4", "123"), agent), {
-      block: false,
-    });
-    t.same(shouldRateLimitRequest(createContext("1.2.3.4", "123"), agent), {
-      block: false,
-    });
-    t.same(shouldRateLimitRequest(createContext("1.2.3.4", "123"), agent), {
-      block: false,
-    });
-    t.same(shouldRateLimitRequest(createContext("1.2.3.4", "123"), agent), {
-      block: false,
-    });
-  }
-);
-
 t.test(
   "it does not consume rate limit for user a second time (same request)",
   async (t) => {

library/ratelimiting/shouldRateLimitRequest.ts

@@ -53,12 +53,7 @@ export function shouldRateLimitRequest(
     isLocalhostIP(context.remoteAddress) &&
     isProduction;
 
-  // Allow requests from allowed IPs, e.g. never rate limit office IPs
-  const isBypassedIP =
-    context.remoteAddress &&
-    agent.getConfig().isBypassedIP(context.remoteAddress);
-
-  if (isFromLocalhostInProduction || isBypassedIP) {
+  if (isFromLocalhostInProduction) {
     return { block: false };
   }