Merge pull request #603 from AikidoSec/allowed-ips-exact-match

Allow access to route if exact match does

Author: hansott

end2end/tests/hono-xml-allowlists.test.ts

@@ -32,6 +32,24 @@ t.beforeEach(async () => {
             enabled: false,
           },
         },
+        {
+          route: "/admin/*",
+          method: "GET",
+          forceProtectionOff: false,
+          allowedIPAddresses: ["10.0.0.1/16"],
+          rateLimiting: {
+            enabled: false,
+          },
+        },
+        {
+          route: "/admin/public",
+          method: "GET",
+          forceProtectionOff: false,
+          allowedIPAddresses: ["0.0.0.0/0", "::/0"],
+          rateLimiting: {
+            enabled: false,
+          },
+        },
       ],
     }),
   });
@@ -150,6 +168,22 @@ t.test("it blocks non-allowed IP addresses", (t) => {
         signal: AbortSignal.timeout(5000),
       });
       t.same(resp6.status, 403);
+
+      const resp7 = await fetch("http://127.0.0.1:4002/admin/public", {
+        headers: {
+          "X-Forwarded-For": "5.6.7.8",
+        },
+        signal: AbortSignal.timeout(5000),
+      });
+      t.same(resp7.status, 200);
+
+      const resp8 = await fetch("http://127.0.0.1:4002/admin/private", {
+        headers: {
+          "X-Forwarded-For": "5.6.7.8",
+        },
+        signal: AbortSignal.timeout(5000),
+      });
+      t.same(resp8.status, 403);
     })
     .catch((error) => {
       t.fail(error);

library/helpers/ip-matcher/IPMatcher.test.ts

@@ -192,3 +192,15 @@ t.test("Different cidr ranges", async (t) => {
   t.same(new IPMatcher(["123.2.0.2/31"]).has("123.2.0.1"), false);
   t.same(new IPMatcher(["123.2.0.2/32"]).has("123.2.0.2"), true);
 });
+
+t.test("allow all ips", async (t) => {
+  const matcher = new IPMatcher(["0.0.0.0/0", "::/0"]);
+  t.same(matcher.has("1.2.3.4"), true);
+  t.same(matcher.has("::1"), true);
+  t.same(matcher.has("::ffff:1234"), true);
+  t.same(matcher.has("1.1.1.1"), true);
+  t.same(matcher.has("2002:db8::1"), true);
+  t.same(matcher.has("10.0.0.1"), true);
+  t.same(matcher.has("10.0.0.255"), true);
+  t.same(matcher.has("192.168.1.1"), true);
+});

library/sources/http-server/ipAllowedToAccessRoute.test.ts

@@ -39,6 +39,31 @@ t.beforeEach(async () => {
           allowedIPAddresses: ["1.2.3.4", "192.168.2.0/24"],
           forceProtectionOff: false,
         },
+
+        {
+          route: "/private/public",
+          // @ts-expect-error Test
+          rateLimiting: undefined,
+          method: "GET",
+          allowedIPAddresses: ["0.0.0.0/0", "::/0"],
+          forceProtectionOff: false,
+        },
+        {
+          route: "/private/public/*",
+          // @ts-expect-error Test
+          rateLimiting: undefined,
+          method: "GET",
+          allowedIPAddresses: ["0.0.0.0/0", "::/0"],
+          forceProtectionOff: false,
+        },
+        {
+          route: "/private/*",
+          // @ts-expect-error Test
+          rateLimiting: undefined,
+          method: "GET",
+          allowedIPAddresses: ["127.0.0.1", "::1"],
+          forceProtectionOff: false,
+        },
       ],
       block: true,
     }),
@@ -88,6 +113,50 @@ t.test("it blocks request if no IP address", async () => {
   );
 });
 
+t.test("public subroute of private route", async () => {
+  t.same(
+    ipAllowedToAccessRoute(
+      {
+        ...context,
+        url: "/private/test",
+        route: "/private/test",
+        method: "GET",
+        remoteAddress: "1.1.1.1",
+      },
+      agent
+    ),
+    false
+  );
+  t.same(
+    ipAllowedToAccessRoute(
+      {
+        ...context,
+        url: "/private/public",
+        route: "/private/public",
+        method: "GET",
+        remoteAddress: "1.1.1.1",
+      },
+      agent
+    ),
+    true
+  );
+
+  // No exact match and not all matching endpoints allow the IP address
+  t.same(
+    ipAllowedToAccessRoute(
+      {
+        ...context,
+        url: "/private/public/test",
+        route: "/private/public/test",
+        method: "GET",
+        remoteAddress: "1.1.1.1",
+      },
+      agent
+    ),
+    false
+  );
+});
+
 t.test("it allows request if configuration is broken", async () => {
   const agent = createTestAgent({
     token: new Token("123"),
@@ -203,7 +272,7 @@ t.test("it checks every matching endpoint", async () => {
 
   t.same(
     ipAllowedToAccessRoute({ ...context, remoteAddress: "3.4.5.6" }, agent),
-    false
+    true // Because exact match allows the IP address
   );
 });
 

library/sources/http-server/ipAllowedToAccessRoute.ts

@@ -1,23 +1,44 @@
 import { Agent } from "../../agent/Agent";
 import { Context } from "../../agent/Context";
 import { isLocalhostIP } from "../../helpers/isLocalhostIP";
+import type { Endpoint } from "../../agent/Config";
+import type { IPMatcher } from "../../helpers/ip-matcher/IPMatcher";
 
 export function ipAllowedToAccessRoute(context: Context, agent: Agent) {
+  // Always allow localhost IPs
   if (context.remoteAddress && isLocalhostIP(context.remoteAddress)) {
     return true;
   }
 
-  const matches = agent.getConfig().getEndpoints(context);
+  // Get all matching endpoints with allowedIPAddresses defined
+  const matches = agent
+    .getConfig()
+    .getEndpoints(context)
+    .filter(
+      (m): m is Endpoint & { allowedIPAddresses: IPMatcher } =>
+        m.allowedIPAddresses !== undefined
+    );
 
-  for (const endpoint of matches) {
-    if (!endpoint.allowedIPAddresses) {
-      continue;
-    }
+  if (!matches.length) {
+    // No matches found, so we can allow access
+    return true;
+  }
 
-    if (!context.remoteAddress) {
-      return false;
+  if (!context.remoteAddress) {
+    // Always block if remote address is unknown
+    return false;
+  }
+
+  // Check exact match first
+  // If exact match allows the IP address, we can allow access without checking other matching endpoint configurations
+  const exact = matches.find((m) => m.route === context.route);
+  if (exact && exact.allowedIPAddresses) {
+    if (exact.allowedIPAddresses.has(context.remoteAddress)) {
+      return true;
     }
+  }
 
+  for (const endpoint of matches) {
     const { allowedIPAddresses } = endpoint;
 
     if (!allowedIPAddresses.has(context.remoteAddress)) {

sample-apps/hono-xml/app.js

@@ -110,6 +110,16 @@ async function main() {
     );
   });
 
+  app.get("/admin/public", async (c) => {
+    return c.html(
+      `<html lang="en">
+        <body>
+          <h1>Public subpage of the admin panel</h1>
+        </body>
+      </html>`
+    );
+  });
+
   app.post("/add-fast", async (c) => {
     const body = await c.req.text();