-
Notifications
You must be signed in to change notification settings - Fork 52
fix(vis-server): add Host-header guard to block DNS rebinding #67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| import type { Context, Next } from 'hono'; | ||
|
|
||
| import { isLoopbackHost } from './config'; | ||
|
|
||
| /** Extract the hostname (without port) from a Host header or URL authority. | ||
| * Handles IPv6 bracket form (`[::1]:3001`), bare IPv6 literals (`2001:db8::1`), | ||
| * and `host:port`. Returns null for empty/missing input. | ||
| * | ||
| * A malformed authority is returned verbatim (lowercased) rather than coerced, | ||
| * so it fails every allow check instead of being silently reduced to a value | ||
| * that looks allowed (e.g. `[::1]evil.com` must not become `::1`). */ | ||
| export function hostnameFromAuthority(authority: string | undefined | null): string | null { | ||
| if (authority === undefined || authority === null) return null; | ||
| const trimmed = authority.trim(); | ||
| if (trimmed.length === 0) return null; | ||
| if (trimmed.startsWith('[')) { | ||
| const end = trimmed.indexOf(']'); | ||
| if (end < 0) return trimmed.toLowerCase(); // unterminated bracket → cannot match | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Treating an unterminated bracket host as a pass-through string here creates a guard bypass: Useful? React with 👍 / 👎. |
||
| const rest = trimmed.slice(end + 1); | ||
| // After the closing bracket only an empty string or `:port` is valid; any | ||
| // other trailing text means the authority is malformed — don't let the | ||
| // bracketed prefix masquerade as the real host. | ||
| if (rest.length > 0 && !/^:\d+$/u.test(rest)) return trimmed.toLowerCase(); | ||
| return trimmed.slice(1, end).toLowerCase(); | ||
| } | ||
| const firstColon = trimmed.indexOf(':'); | ||
| if (firstColon < 0) return trimmed.toLowerCase(); | ||
| // An unbracketed authority with more than one colon is a bare IPv6 literal | ||
| // with no port (a port requires bracket form), so the whole string is the | ||
| // host. A single colon is an ordinary `host:port`. | ||
| if (trimmed.includes(':', firstColon + 1)) return trimmed.toLowerCase(); | ||
| return trimmed.slice(0, firstColon).toLowerCase(); | ||
| } | ||
|
|
||
| /** Wildcard bind addresses (`0.0.0.0`, `::`) mean "every interface" and never | ||
| * appear as a concrete client's Host, so they cannot serve as a match target. */ | ||
| function isWildcardHost(host: string): boolean { | ||
| return host === '0.0.0.0' || host === '::' || host === '0:0:0:0:0:0:0:0'; | ||
| } | ||
|
|
||
| function parseAllowedHosts(raw: string | undefined): ReadonlySet<string> { | ||
| const set = new Set<string>(); | ||
| if (raw === undefined) return set; | ||
| for (const part of raw.split(',')) { | ||
| const h = hostnameFromAuthority(part); | ||
| if (h !== null) set.add(h); | ||
| } | ||
| return set; | ||
| } | ||
|
|
||
| /** Decide whether a single request hostname is one we expect to serve. Loopback | ||
| * names are always allowed (a DNS-rebinding attacker cannot forge a loopback | ||
| * Host from a browser). Otherwise the hostname must match the configured bind | ||
| * host or an explicit allow-list entry. */ | ||
| function hostnameAllowed( | ||
| hostname: string, | ||
| normalizedBindHost: string, | ||
| allowedHosts: ReadonlySet<string>, | ||
| ): boolean { | ||
| if (isLoopbackHost(hostname)) return true; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Already addressed in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The guard claims loopback names are always allowed, but it delegates to Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Already addressed in |
||
| if (allowedHosts.has(hostname)) return true; | ||
| if (normalizedBindHost.length > 0 && hostname === normalizedBindHost) return true; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The host check only allows an exact match with Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed at the wiring level rather than here: as of |
||
| return false; | ||
| } | ||
|
|
||
| export interface HostGuardOptions { | ||
| /** The host the server is bound to (e.g. resolveHost()). */ | ||
| readonly bindHost: string; | ||
| /** Raw comma-separated VIS_ALLOWED_HOSTS value, if any. */ | ||
| readonly allowedHosts?: string; | ||
| } | ||
|
|
||
| /** Hono middleware that rejects requests whose Host header / URL authority is | ||
| * neither a loopback name nor an explicitly allowed host. | ||
| * | ||
| * This is a DNS-rebinding defense for the *no-token loopback* mode: the vis | ||
| * server binds to loopback by default and serves no auth token there, so | ||
| * without this check any web page the user visits could rebind its own | ||
| * hostname to 127.0.0.1 and read or delete the user's local agent sessions | ||
| * cross-origin. A browser performing that attack still sends the *original* | ||
| * attacker hostname in the Host header, which will not match a loopback name | ||
| * or the configured bind host. When an auth token is configured the token is | ||
| * the access control — a rebinding attacker cannot read it cross-origin — so | ||
| * this guard is not installed in that mode (see createApp), which keeps LAN / | ||
| * wildcard binds working without listing every reachable host. */ | ||
| export function hostGuard(options: HostGuardOptions) { | ||
| const allowedHosts = parseAllowedHosts(options.allowedHosts); | ||
| const bindHost = hostnameFromAuthority(options.bindHost) ?? ''; | ||
| const normalizedBindHost = isWildcardHost(bindHost) ? '' : bindHost; | ||
|
|
||
| return async (c: Context, next: Next): Promise<Response | void> => { | ||
| // Collect every hostname the request claims. In @hono/node-server the URL | ||
| // authority is built from the client's Host header, so both reflect what a | ||
| // rebinding attacker controls; require all present hostnames to be allowed. | ||
| const hostnames: string[] = []; | ||
| try { | ||
| const fromUrl = hostnameFromAuthority(new URL(c.req.url).host); | ||
| if (fromUrl !== null) hostnames.push(fromUrl); | ||
| } catch { | ||
| // ignore unparseable URL; fall back to the Host header below | ||
| } | ||
| const fromHeader = hostnameFromAuthority(c.req.header('host')); | ||
| if (fromHeader !== null) hostnames.push(fromHeader); | ||
|
|
||
| const ok = | ||
| hostnames.length > 0 && | ||
| hostnames.every((h) => hostnameAllowed(h, normalizedBindHost, allowedHosts)); | ||
| if (!ok) { | ||
| return c.json( | ||
| { | ||
| error: | ||
| 'forbidden host: request Host is not loopback, the configured bind host, or in VIS_ALLOWED_HOSTS', | ||
| code: 'FORBIDDEN_HOST', | ||
| }, | ||
| 403, | ||
| ); | ||
| } | ||
| await next(); | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| // apps/vis/server/test/host-guard.test.ts | ||
| import { Hono } from 'hono'; | ||
| import { afterEach, describe, expect, it } from 'vitest'; | ||
|
|
||
| import { createApp } from '../src/app'; | ||
| import { isLoopbackHost } from '../src/config'; | ||
| import { hostGuard, hostnameFromAuthority } from '../src/host-guard'; | ||
|
|
||
| /** Minimal app exercising only the guard, so assertions don't depend on the | ||
| * session routes or KIMI_CODE_HOME. */ | ||
| function guarded(options: { bindHost: string; allowedHosts?: string }): Hono { | ||
| const app = new Hono(); | ||
| app.use('*', hostGuard(options)); | ||
| app.get('/api/ping', (c) => c.json({ ok: true })); | ||
| return app; | ||
| } | ||
|
|
||
| describe('hostnameFromAuthority', () => { | ||
| it('parses host:port and bare hostnames', () => { | ||
| expect(hostnameFromAuthority('example.com:3001')).toBe('example.com'); | ||
| expect(hostnameFromAuthority('127.0.0.1')).toBe('127.0.0.1'); | ||
| expect(hostnameFromAuthority(' Example.COM ')).toBe('example.com'); | ||
| expect(hostnameFromAuthority('')).toBeNull(); | ||
| expect(hostnameFromAuthority(undefined)).toBeNull(); | ||
| }); | ||
|
|
||
| it('does not truncate bare IPv6 literals at the first colon', () => { | ||
| expect(hostnameFromAuthority('2001:db8::10')).toBe('2001:db8::10'); | ||
| expect(hostnameFromAuthority('[2001:db8::10]:3001')).toBe('2001:db8::10'); | ||
| expect(hostnameFromAuthority('[::1]')).toBe('::1'); | ||
| }); | ||
|
|
||
| it('does not let trailing junk after a bracket masquerade as the host', () => { | ||
| // `[::1]evil.com` must NOT normalize to the loopback `::1`. | ||
| const parsed = hostnameFromAuthority('[::1]evil.com'); | ||
| expect(parsed).not.toBe('::1'); | ||
| expect(isLoopbackHost(parsed ?? '')).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe('isLoopbackHost', () => { | ||
| it('accepts loopback names and the 127.0.0.0/8 range', () => { | ||
| for (const h of ['localhost', 'tenant.localhost', '127.0.0.1', '127.0.0.2', '::1', '[::1]']) { | ||
| expect(isLoopbackHost(h), h).toBe(true); | ||
| } | ||
| }); | ||
|
|
||
| it('rejects hostnames that merely start with 127.', () => { | ||
| for (const h of ['127.0.0.1.nip.io', '127.evil.com', '1270.0.0.1', '127.0.0.256']) { | ||
| expect(isLoopbackHost(h), h).toBe(false); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| describe('hostGuard (DNS-rebinding defense)', () => { | ||
| it('rejects a rebound non-loopback Host', async () => { | ||
| const app = guarded({ bindHost: '127.0.0.1' }); | ||
| const res = await app.request('http://attacker.example/api/ping'); | ||
| expect(res.status).toBe(403); | ||
| await expect(res.json()).resolves.toMatchObject({ code: 'FORBIDDEN_HOST' }); | ||
| }); | ||
|
|
||
| it('allows loopback and *.localhost Host values', async () => { | ||
| const app = guarded({ bindHost: '127.0.0.1' }); | ||
| for (const origin of [ | ||
| 'http://localhost/api/ping', | ||
| 'http://127.0.0.1/api/ping', | ||
| 'http://127.0.0.2/api/ping', | ||
| 'http://tenant.localhost/api/ping', | ||
| ]) { | ||
| expect((await app.request(origin)).status, origin).toBe(200); | ||
| } | ||
| }); | ||
|
|
||
| it('does not allow a domain that only starts with 127. (no prefix bypass)', async () => { | ||
| const app = guarded({ bindHost: '127.0.0.1' }); | ||
| expect((await app.request('http://127.0.0.1.nip.io/api/ping')).status).toBe(403); | ||
| }); | ||
|
|
||
| it('allows hosts listed in VIS_ALLOWED_HOSTS, still blocks others', async () => { | ||
| const app = guarded({ bindHost: '127.0.0.1', allowedHosts: 'vis.internal,dev.box' }); | ||
| expect((await app.request('http://vis.internal/api/ping')).status).toBe(200); | ||
| expect((await app.request('http://attacker.example/api/ping')).status).toBe(403); | ||
| }); | ||
| }); | ||
|
|
||
| describe('createApp host-guard wiring', () => { | ||
| const savedEnv = { ...process.env }; | ||
| afterEach(() => { | ||
| process.env = { ...savedEnv }; | ||
| }); | ||
|
|
||
| it('guards requests when no auth token is configured', async () => { | ||
| const app = await createApp({ host: '127.0.0.1' }); | ||
| const res = await app.request('http://attacker.example/api/sessions'); | ||
| expect(res.status).toBe(403); | ||
| await expect(res.json()).resolves.toMatchObject({ code: 'FORBIDDEN_HOST' }); | ||
| }); | ||
|
|
||
| it('does not host-guard in token mode, so wildcard LAN access keeps working', async () => { | ||
| // Binding to a wildcard requires a token; the token (not the Host) is the | ||
| // access control, so a concrete LAN client must not be rejected as a | ||
| // forbidden host. | ||
| const app = await createApp({ authToken: 'secret-token', host: '0.0.0.0' }); | ||
| const res = await app.request('http://192.168.1.10/api/sessions', { | ||
| headers: { authorization: 'Bearer secret-token' }, | ||
| }); | ||
| expect(res.status).not.toBe(403); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the new host guard enabled in no-token mode, requests using valid FQDN loopback spellings like
localhost.are rejected withFORBIDDEN_HOSTbecause loopback detection only accepts exactlocalhostor*.localhostand never normalizes a trailing DNS root dot. This is a functional regression from pre-guard behavior (which accepted any host) and can block legitimate local clients that preserve the trailing dot form.Useful? React with 👍 / 👎.