diff --git a/package-lock.json b/package-lock.json index 42e622b..e2d3bcd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,7 +12,8 @@ "@modelcontextprotocol/sdk": "^1.29.0", "axios": "^1.18.1", "countly-sdk-nodejs": "^24.10.4", - "dotenv": "^17.4.2" + "dotenv": "^17.4.2", + "ipaddr.js": "^1.9.1" }, "bin": { "countly-mcp-server": "build/index.js" diff --git a/package.json b/package.json index f852e01..4fd6efb 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,8 @@ "@modelcontextprotocol/sdk": "^1.29.0", "axios": "^1.18.1", "countly-sdk-nodejs": "^24.10.4", - "dotenv": "^17.4.2" + "dotenv": "^17.4.2", + "ipaddr.js": "^1.9.1" }, "devDependencies": { "@types/node": "^20.19.43", diff --git a/src/index.ts b/src/index.ts index 40722aa..7a7dc1b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -9,6 +9,7 @@ dotenv.config({ quiet: true }); import { AsyncLocalStorage } from 'async_hooks'; import { realpathSync } from 'fs'; import http from 'http'; +import https from 'https'; import { createRequire } from 'module'; import url from 'url'; @@ -38,7 +39,7 @@ import axios, { AxiosInstance } from 'axios'; import { AppCache, AppCacheRegistry, resolveAppIdentifier, type CountlyApp } from './lib/app-cache.js'; import { resolveAuthToken, createMissingAuthError } from './lib/auth.js'; import { analytics } from './lib/analytics.js'; -import { assertSafeServerUrl, buildConfig } from './lib/config.js'; +import { assertSafeServerUrl, buildConfig, safeLookup } from './lib/config.js'; import { ConcurrencyLimiter, enforceBodySizeLimit, @@ -83,6 +84,14 @@ interface HttpConfig { interface RequestState { authToken?: string; serverUrl: string; + /** + * True when `serverUrl` came from a caller-supplied source (the + * X-Countly-Server-Url header or a URL parameter) rather than the operator's + * trusted COUNTLY_SERVER_URL config. Only the caller-controlled path gets + * connect-time DNS validation (safeLookup) + redirect suppression, so a + * legitimate on-prem COUNTLY_SERVER_URL on a private IP is never blocked. + */ + serverUrlFromCaller?: boolean; } interface ToolCallHistory { @@ -341,8 +350,13 @@ class CountlyMCPServer { // Build a fresh axios client for this request so concurrent tenants // cannot share headers / baseURL on the same object. The shared - // `this.httpClient` is intentionally untouched. - const perReqHttpClient = this.createRequestHttpClient(authToken, serverUrl); + // `this.httpClient` is intentionally untouched. Caller-supplied server + // URLs additionally get connect-time DNS validation + no-redirects. + const perReqHttpClient = this.createRequestHttpClient( + authToken, + serverUrl, + reqState?.serverUrlFromCaller === true + ); // Per-tenant app cache. Keyed by SHA-256(authToken) inside the // registry so one tenant's apps cannot leak into another's @@ -596,17 +610,30 @@ class CountlyMCPServer { */ private createRequestHttpClient( authToken: string | undefined, - serverUrl: string + serverUrl: string, + untrusted = false ): AxiosInstance { const headers: Record = {}; if (authToken) { headers['countly-token'] = authToken; } - return axios.create({ + const config: Parameters[0] = { baseURL: serverUrl, timeout: this.config.timeout, headers, - }); + }; + // For caller-controlled server URLs, pin DNS resolution through + // safeLookup so the socket only ever connects to a public unicast IP + // (closes plain DNS-based SSRF *and* DNS-rebinding TOCTOU), and refuse + // redirects so a 30x cannot bounce the request to an internal target. + // The operator's trusted COUNTLY_SERVER_URL path skips this so an on-prem + // Countly on a private IP keeps working. + if (untrusted) { + config.httpAgent = new http.Agent({ lookup: safeLookup }); + config.httpsAgent = new https.Agent({ lookup: safeLookup }); + config.maxRedirects = 0; + } + return axios.create(config); } /** @@ -634,7 +661,11 @@ class CountlyMCPServer { authToken = process.env.COUNTLY_AUTH_TOKEN; } const serverUrl = reqState?.serverUrl || this.config.serverUrl; - const client = this.createRequestHttpClient(authToken, serverUrl); + const client = this.createRequestHttpClient( + authToken, + serverUrl, + reqState?.serverUrlFromCaller === true + ); const cache = this.appCacheRegistry.for(authToken); return { client, cache, authToken }; } @@ -1007,7 +1038,14 @@ class CountlyMCPServer { // shared-state mutation. This closes the cross-tenant token-mixing // window previously present in the HTTP transport. await this.requestContext.run( - { authToken: authToken || undefined, serverUrl: effectiveServerUrl }, + { + authToken: authToken || undefined, + serverUrl: effectiveServerUrl, + // `serverUrl` here is the caller-supplied header/param value (if + // any). When present, the effective URL is attacker-controlled + // and its outbound client must get connect-time SSRF validation. + serverUrlFromCaller: !!serverUrl, + }, async () => { await transport.handleRequest(req, res); } diff --git a/src/lib/config.ts b/src/lib/config.ts index 6df9878..c40dc4a 100644 --- a/src/lib/config.ts +++ b/src/lib/config.ts @@ -3,6 +3,10 @@ * Pure functions for processing and validating configuration */ +import dns from 'node:dns'; +import { isIP } from 'node:net'; +import ipaddr from 'ipaddr.js'; + export interface CountlyConfig { serverUrl: string; timeout?: number; @@ -78,6 +82,73 @@ export function validateServerUrl(url: string): boolean { } } +/** + * Hostnames known to serve cloud-metadata / internal orchestration services. + * Matched case-insensitively as an exact hostname. Kept in sync with the + * countly-server `api/utils/ssrf-protection.js` blocklist. + */ +const BLOCKED_HOSTNAMES = new Set([ + 'metadata.google.internal', + 'metadata.goog', + 'metadata.google.com', + 'kubernetes.default.svc', + 'kubernetes.default', + 'kubernetes', +]); + +/** + * Classify a literal IP (v4 or v6) using ipaddr.js range() detection. Only + * globally-routable `unicast` addresses are considered safe; every other range + * (loopback, private, link-local, unique-local, carrier-grade NAT, multicast, + * reserved, unspecified, broadcast, NAT64, …) is unsafe. + * + * IPv4-mapped IPv6 (`::ffff:a.b.c.d`, e.g. `::ffff:127.0.0.1`) is unwrapped to + * its embedded IPv4 address and re-classified, closing the representation + * bypass where a mapped address is routed to IPv4 loopback/metadata by the OS + * but slips past naive string/prefix checks. + * + * Returns the offending range name (e.g. "loopback"), the literal string + * "unparseable" when the input cannot be parsed, or null when the IP is a safe + * public unicast address. + */ +function blockedIpRange(ip: string): string | null { + let parsed: ipaddr.IPv4 | ipaddr.IPv6; + try { + parsed = ipaddr.parse(ip); + } catch { + // Unparseable — refuse to be safe. + return 'unparseable'; + } + + let range = parsed.range(); + + // Unwrap IPv4-mapped IPv6 (::ffff:0:0/96) and classify the inner IPv4 so + // e.g. ::ffff:127.0.0.1 is treated as 127.0.0.1 (loopback). + if ( + parsed.kind() === 'ipv6' && + (parsed as ipaddr.IPv6).isIPv4MappedAddress() + ) { + range = (parsed as ipaddr.IPv6).toIPv4Address().range(); + } + + return range === 'unicast' ? null : range; +} + +/** + * Human-readable wrapper over blockedIpRange for the syntactic host check. + * Returns a reason string when the IP is unsafe, or null when safe. + */ +function classifyIpLiteral(ip: string, original: string): string | null { + const range = blockedIpRange(ip); + if (range === null) { + return null; + } + if (range === 'unparseable') { + return `IP literal "${original}" could not be classified`; + } + return `IP "${original}" is in a non-public range (${range}) and is not allowed`; +} + /** * Test whether a hostname or literal IP resolves to a "sensitive" network * target that the MCP server must refuse to call. This is an SSRF mitigation @@ -95,50 +166,35 @@ export function assertSafeServerHost(hostname: string): string | null { if (!hostname) { return 'hostname is empty'; } - const lower = hostname.toLowerCase(); - // Block bare localhost aliases and mDNS names - if (lower === 'localhost' || lower.endsWith('.localhost') || lower.endsWith('.local')) { - return `hostname "${hostname}" points at the local machine`; + // URL parsers hand back bracketed IPv6 literals ("[::1]"); strip the + // brackets so net.isIP / ipaddr.js can classify the address. + let host = hostname; + if (host.startsWith('[') && host.endsWith(']')) { + host = host.slice(1, -1); } + const lower = host.toLowerCase(); - // Block IPv6 loopback / link-local / unique-local / unspecified - if (lower === '::1' || lower === '[::1]' || lower === '::' || lower === '[::]') { - return `IPv6 loopback/unspecified address "${hostname}" is not allowed`; + // Block bare localhost aliases, mDNS, and internal orchestration TLDs + if ( + lower === 'localhost' || + lower.endsWith('.localhost') || + lower.endsWith('.local') || + lower.endsWith('.internal') + ) { + return `hostname "${hostname}" points at the local machine or an internal service`; } - // IPv6 link-local (fe80::/10) and unique-local (fc00::/7) — strip brackets first - const stripped = lower.replace(/^\[|\]$/g, ''); - if (stripped.startsWith('fe8') || stripped.startsWith('fe9') || stripped.startsWith('fea') || stripped.startsWith('feb') || - stripped.startsWith('fc') || stripped.startsWith('fd')) { - return `IPv6 private/link-local address "${hostname}" is not allowed`; + + // Block known cloud-metadata / internal-service hostnames + if (BLOCKED_HOSTNAMES.has(lower)) { + return `hostname "${hostname}" is a blocked internal/metadata service`; } - // Parse IPv4 literal if present - const ipv4Match = /^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/.exec(lower); - if (ipv4Match) { - const [a, b] = ipv4Match.slice(1).map(n => parseInt(n, 10)); - if (a === 0) { - return `IPv4 "${hostname}" in 0.0.0.0/8 (unspecified) is not allowed`; - } - if (a === 10) { - return `IPv4 "${hostname}" in 10.0.0.0/8 (private) is not allowed`; - } - if (a === 127) { - return `IPv4 "${hostname}" in 127.0.0.0/8 (loopback) is not allowed`; - } - if (a === 169 && b === 254) { - // 169.254.0.0/16: link-local; also covers AWS IMDS (169.254.169.254) - return `IPv4 "${hostname}" in 169.254.0.0/16 (link-local, includes cloud metadata) is not allowed`; - } - if (a === 172 && b >= 16 && b <= 31) { - return `IPv4 "${hostname}" in 172.16.0.0/12 (private) is not allowed`; - } - if (a === 192 && b === 168) { - return `IPv4 "${hostname}" in 192.168.0.0/16 (private) is not allowed`; - } - if (a === 100 && b >= 64 && b <= 127) { - return `IPv4 "${hostname}" in 100.64.0.0/10 (shared carrier-grade NAT) is not allowed`; - } + // If the host is a literal IP (v4 or v6, in any representation), classify it + // with ipaddr.js. This covers dotted-quad, integer/hex-collapsed forms that + // Node's URL parser already normalizes, full IPv6, and IPv4-mapped IPv6. + if (isIP(lower)) { + return classifyIpLiteral(lower, hostname); } return null; @@ -160,6 +216,13 @@ export function assertSafeServerUrl(url: string): void { `Refusing server URL with scheme "${parsed.protocol}" — only http:/https: are allowed.` ); } + // Reject embedded credentials (user:pass@host) — they are never needed for a + // Countly server URL and are a common SSRF/credential-smuggling vector. + if (parsed.username || parsed.password) { + throw new Error( + `Refusing server URL "${url}" for SSRF safety: embedded credentials are not allowed.` + ); + } const reason = assertSafeServerHost(parsed.hostname); if (reason) { throw new Error( @@ -168,6 +231,101 @@ export function assertSafeServerUrl(url: string): void { } } +/** + * Error thrown by safeLookup when a hostname resolves to a blocked address. + * Carries a stable `code` so callers can distinguish an SSRF block from a + * generic connection failure. + */ +export function blockedLookupError(hostname: string, address: string, range: string): Error { + const err = new Error( + `Blocked SSRF target: "${hostname}" resolved to non-public IP "${address}" (${range})` + ); + (err as NodeJS.ErrnoException).code = 'ESSRFBLOCKED'; + return err; +} + +type LookupAllCallback = (err: NodeJS.ErrnoException | null, addresses: dns.LookupAddress[]) => void; +type LookupSingleCallback = ( + err: NodeJS.ErrnoException | null, + address?: string, + family?: number +) => void; + +/** + * A `dns.lookup`-compatible function that resolves a hostname and then rejects + * the lookup if the resolved address is private/reserved/internal. + * + * Passing this as the `lookup` option of a Node http/https Agent (which axios + * honours via httpAgent/httpsAgent) validates the IP AT CONNECT TIME. This is + * the piece that a parse-time-only string check cannot provide: + * + * 1. A hostname whose A/AAAA record simply points at a private/loopback/ + * metadata IP is caught here (the syntactic host check never resolves + * names, so it would otherwise pass such a hostname straight through). + * 2. DNS-rebinding (TOCTOU) is closed: even if a name resolved to a public + * IP a moment ago, the socket only ever connects to an address that + * passes blockedIpRange here, at the instant of connection. + * + * Mirrors countly-server's api/utils/ssrf-protection.js `safeLookup`. + * + * IMPORTANT: only wire this into the client used for CALLER-CONTROLLED server + * URLs. The operator's own COUNTLY_SERVER_URL is frequently a private-IP + * on-prem host and must NOT be forced through this guard. + */ +export function safeLookup( + hostname: string, + options: dns.LookupOneOptions | dns.LookupAllOptions | dns.LookupOptions | LookupSingleCallback, + callback?: LookupSingleCallback | LookupAllCallback +): void { + let opts: dns.LookupOptions; + let cb: LookupSingleCallback | LookupAllCallback; + if (typeof options === 'function') { + cb = options; + opts = {}; + } else { + opts = options; + cb = callback as LookupSingleCallback | LookupAllCallback; + } + + // Use a single explicit signature to sidestep dns.lookup's overloads — + // we handle both the single-address and options.all (array) shapes below. + const doLookup = dns.lookup as ( + h: string, + o: dns.LookupOptions, + cb: ( + err: NodeJS.ErrnoException | null, + address: string | dns.LookupAddress[], + family?: number + ) => void + ) => void; + doLookup(hostname, opts, (err, address, family) => { + if (err) { + (cb as LookupSingleCallback)(err); + return; + } + // When options.all is true, `address` is an array of {address, family}. + if (opts && (opts as dns.LookupAllOptions).all) { + const list = (Array.isArray(address) ? address : [address]) as dns.LookupAddress[]; + for (const entry of list) { + const range = blockedIpRange(entry.address); + if (range) { + (cb as LookupAllCallback)(blockedLookupError(hostname, entry.address, range), []); + return; + } + } + (cb as LookupAllCallback)(null, list); + return; + } + const addr = address as unknown as string; + const range = blockedIpRange(addr); + if (range) { + (cb as LookupSingleCallback)(blockedLookupError(hostname, addr, range)); + return; + } + (cb as LookupSingleCallback)(null, addr, family as number); + }); +} + /** * Build full configuration with validation * Server URL is optional - can be provided by client or environment diff --git a/tests/security.test.ts b/tests/security.test.ts index d728cd3..0efcc62 100644 --- a/tests/security.test.ts +++ b/tests/security.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect } from 'vitest'; import { AppCache, AppCacheRegistry } from '../src/lib/app-cache.js'; -import { assertSafeServerHost, assertSafeServerUrl } from '../src/lib/config.js'; +import { assertSafeServerHost, assertSafeServerUrl, safeLookup } from '../src/lib/config.js'; import { redactSensitiveInMessage } from '../src/lib/error-handler.js'; import { ConcurrencyLimiter, @@ -38,16 +38,16 @@ describe('assertSafeServerHost: SSRF denylist', () => { }); it('rejects AWS / cloud metadata endpoint 169.254.169.254', () => { - expect(assertSafeServerHost('169.254.169.254')).toMatch(/169\.254/); + expect(assertSafeServerHost('169.254.169.254')).toMatch(/linkLocal/); }); it('rejects RFC 1918 10/8', () => { - expect(assertSafeServerHost('10.0.0.1')).toMatch(/10\.0\.0\.0\/8/); + expect(assertSafeServerHost('10.0.0.1')).toMatch(/private/); }); it('rejects RFC 1918 172.16/12', () => { - expect(assertSafeServerHost('172.16.0.1')).toMatch(/172\.16/); - expect(assertSafeServerHost('172.31.255.254')).toMatch(/172\.16/); + expect(assertSafeServerHost('172.16.0.1')).toMatch(/private/); + expect(assertSafeServerHost('172.31.255.254')).toMatch(/private/); }); it('accepts a public IPv4 (1.1.1.1)', () => { @@ -55,11 +55,11 @@ describe('assertSafeServerHost: SSRF denylist', () => { }); it('rejects RFC 1918 192.168/16', () => { - expect(assertSafeServerHost('192.168.1.1')).toMatch(/192\.168/); + expect(assertSafeServerHost('192.168.1.1')).toMatch(/private/); }); it('rejects carrier-grade NAT 100.64/10', () => { - expect(assertSafeServerHost('100.64.0.1')).toMatch(/100\.64/); + expect(assertSafeServerHost('100.64.0.1')).toMatch(/carrierGradeNat/); }); it('rejects 0.0.0.0/8', () => { @@ -74,16 +74,25 @@ describe('assertSafeServerHost: SSRF denylist', () => { expect(assertSafeServerHost('my-countly.local')).toMatch(/local machine/); }); + it('rejects .internal hostnames', () => { + expect(assertSafeServerHost('foo.internal')).toMatch(/internal service/); + }); + + it('rejects known cloud-metadata hostnames', () => { + expect(assertSafeServerHost('metadata.google.internal')).toBeTruthy(); + expect(assertSafeServerHost('kubernetes.default.svc')).toMatch(/metadata service/); + }); + it('rejects IPv6 loopback', () => { expect(assertSafeServerHost('::1')).toMatch(/loopback/); }); it('rejects IPv6 link-local fe80::', () => { - expect(assertSafeServerHost('fe80::1')).toMatch(/link-local/); + expect(assertSafeServerHost('fe80::1')).toMatch(/linkLocal/); }); it('rejects IPv6 unique-local fd00::', () => { - expect(assertSafeServerHost('fd00::1')).toMatch(/link-local/); + expect(assertSafeServerHost('fd00::1')).toMatch(/uniqueLocal/); }); it('accepts api.count.ly (normal case)', () => { @@ -93,6 +102,35 @@ describe('assertSafeServerHost: SSRF denylist', () => { it('accepts public IPv4 like 1.1.1.1', () => { expect(assertSafeServerHost('1.1.1.1')).toBeNull(); }); + + // ---- Regression: IPv4-mapped IPv6 representation bypass ---- + // https://github.com/Countly/countly-mcp-server — the previous string/regex + // guard let `::ffff:127.0.0.1` (which the OS routes to IPv4 loopback) slip + // through because its normalized form `::ffff:7f00:1` matched neither the + // dotted-quad IPv4 regex nor the blocked IPv6 prefixes. + it('rejects IPv4-mapped IPv6 loopback (::ffff:127.0.0.1)', () => { + expect(assertSafeServerHost('::ffff:127.0.0.1')).toMatch(/not allowed/); + }); + + it('rejects IPv4-mapped IPv6 cloud metadata (::ffff:169.254.169.254)', () => { + expect(assertSafeServerHost('::ffff:169.254.169.254')).toMatch(/not allowed/); + }); + + it('rejects bracketed IPv4-mapped IPv6 loopback ([::ffff:127.0.0.1])', () => { + expect(assertSafeServerHost('[::ffff:127.0.0.1]')).toMatch(/not allowed/); + }); + + it('rejects the hex-collapsed IPv4-mapped form (::ffff:7f00:1)', () => { + expect(assertSafeServerHost('::ffff:7f00:1')).toMatch(/not allowed/); + }); + + it('rejects IPv4-mapped RFC1918 (::ffff:10.0.0.1)', () => { + expect(assertSafeServerHost('::ffff:10.0.0.1')).toMatch(/not allowed/); + }); + + it('accepts an IPv4-mapped public address (::ffff:1.1.1.1)', () => { + expect(assertSafeServerHost('::ffff:1.1.1.1')).toBeNull(); + }); }); describe('assertSafeServerUrl: full URL validation', () => { @@ -122,15 +160,115 @@ describe('assertSafeServerUrl: full URL validation', () => { expect(() => assertSafeServerUrl('http://localhost/')).toThrow(/SSRF/); }); + // ---- Regression: end-to-end URL bypass via IPv4-mapped IPv6 ---- + it('throws on IPv4-mapped IPv6 loopback URL', () => { + expect(() => + assertSafeServerUrl('http://[::ffff:127.0.0.1]:8080') + ).toThrow(/SSRF/); + }); + + it('throws on IPv4-mapped IPv6 cloud-metadata URL', () => { + expect(() => + assertSafeServerUrl('http://[::ffff:169.254.169.254]/latest/meta-data/') + ).toThrow(/SSRF/); + }); + + it('rejects URLs with embedded credentials', () => { + expect(() => + assertSafeServerUrl('http://user:pass@api.count.ly') + ).toThrow(/credentials/); + }); + it('accepts a normal Countly URL', () => { expect(() => assertSafeServerUrl('https://api.count.ly')).not.toThrow(); }); - it('accepts an on-prem Countly on a public IP', () => { - expect(() => assertSafeServerUrl('https://203.0.113.10')).not.toThrow(); + it('accepts an on-prem Countly on a routable public IP', () => { + expect(() => assertSafeServerUrl('https://8.8.8.8')).not.toThrow(); }); }); +describe('safeLookup: connect-time DNS validation (DNS-rebinding / DNS-based SSRF)', () => { + // safeLookup is a dns.lookup-compatible function. We drive it directly with + // IP-literal "hostnames" (dns.lookup short-circuits those without a network + // query) so the test is hermetic — no external DNS needed. + + it('passes a resolved public address through', () => + new Promise((resolve, reject) => { + safeLookup('1.1.1.1', {}, (err, address) => { + try { + expect(err).toBeNull(); + expect(address).toBe('1.1.1.1'); + resolve(); + } catch (e) { + reject(e as Error); + } + }); + })); + + it('blocks a hostname resolving to loopback', () => + new Promise((resolve, reject) => { + safeLookup('127.0.0.1', {}, (err) => { + try { + expect(err).toBeTruthy(); + expect((err as NodeJS.ErrnoException).code).toBe('ESSRFBLOCKED'); + resolve(); + } catch (e) { + reject(e as Error); + } + }); + })); + + it('blocks a hostname resolving to cloud metadata', () => + new Promise((resolve, reject) => { + safeLookup('169.254.169.254', {}, (err) => { + try { + expect((err as NodeJS.ErrnoException | null)?.code).toBe('ESSRFBLOCKED'); + resolve(); + } catch (e) { + reject(e as Error); + } + }); + })); + + it('blocks an IPv4-mapped IPv6 resolution', () => + new Promise((resolve, reject) => { + safeLookup('::ffff:127.0.0.1', {}, (err) => { + try { + expect((err as NodeJS.ErrnoException | null)?.code).toBe('ESSRFBLOCKED'); + resolve(); + } catch (e) { + reject(e as Error); + } + }); + })); + + it('supports the options.all array form and blocks if any address is private', () => + new Promise((resolve, reject) => { + safeLookup('10.0.0.1', { all: true }, (err) => { + try { + expect((err as NodeJS.ErrnoException | null)?.code).toBe('ESSRFBLOCKED'); + resolve(); + } catch (e) { + reject(e as Error); + } + }); + })); + + it('accepts the callback-as-second-arg form', () => + new Promise((resolve, reject) => { + safeLookup('8.8.8.8', (err, address) => { + try { + expect(err).toBeNull(); + expect(address).toBe('8.8.8.8'); + resolve(); + } catch (e) { + reject(e as Error); + } + }); + })); +}); + describe('AppCacheRegistry: per-tenant isolation', () => { it('returns different AppCache instances for different tokens', () => { const reg = new AppCacheRegistry();