Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions end2end/tests/hono-xml-allowlists.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
],
}),
});
Expand Down Expand Up @@ -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);
Expand Down
12 changes: 12 additions & 0 deletions library/helpers/ip-matcher/IPMatcher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
71 changes: 70 additions & 1 deletion library/sources/http-server/ipAllowedToAccessRoute.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}),
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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
);
});

Expand Down
35 changes: 28 additions & 7 deletions library/sources/http-server/ipAllowedToAccessRoute.ts
Original file line number Diff line number Diff line change
@@ -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)) {
Expand Down
10 changes: 10 additions & 0 deletions sample-apps/hono-xml/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down