From 19756e4337b223d2fd96f76124469d096edf4408 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 26 Aug 2026 23:02:19 +0200 Subject: [PATCH] fix: pin resolved IP for stored custom JMAP endpoints and branding URLs --- app/api/account/stalwart/jmap/route.ts | 12 ++- app/api/admin/oauth/setup/route.ts | 39 +++---- app/api/caldav/discover/route.ts | 14 ++- app/api/calendar-agenda/route.ts | 10 +- app/api/push/preview/route.ts | 31 ++++-- app/api/setup/test-jmap/route.ts | 51 ++++++--- app/api/webdav/route.ts | 12 ++- lib/__tests__/branding-asset.test.ts | 44 ++++++++ lib/__tests__/server-fetch.test.ts | 138 +++++++++++++++++++++++++ lib/admin/branding-asset.ts | 35 ++++++- lib/stalwart/credentials.ts | 12 ++- lib/stalwart/jmap-api.ts | 20 +++- lib/stalwart/server-fetch.ts | 111 ++++++++++++++++++++ 13 files changed, 463 insertions(+), 66 deletions(-) create mode 100644 lib/__tests__/branding-asset.test.ts create mode 100644 lib/__tests__/server-fetch.test.ts create mode 100644 lib/stalwart/server-fetch.ts diff --git a/app/api/account/stalwart/jmap/route.ts b/app/api/account/stalwart/jmap/route.ts index 76c9a37d6..86b2d1b9c 100644 --- a/app/api/account/stalwart/jmap/route.ts +++ b/app/api/account/stalwart/jmap/route.ts @@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from 'next/server'; import { logger } from '@/lib/logger'; import { getStalwartCredentials } from '@/lib/stalwart/credentials'; import { JmapRedirectError, fetchJmapSession, postJmap, rebaseApiUrl } from '@/lib/stalwart/jmap-api'; +import { DisallowedUrlError } from '@/lib/security/url-guard'; /** * POST /api/account/stalwart/jmap @@ -24,17 +25,18 @@ export async function POST(request: NextRequest) { const body = await request.text(); + const fetchOptions = { trusted: creds.trusted }; const directUrl = `${creds.serverUrl}/jmap/`; - let response = await postJmap(directUrl, creds.authHeader, body); + let response = await postJmap(directUrl, creds.authHeader, body, fetchOptions); if (response.status === 404) { // `${serverUrl}/jmap/` is not the API endpoint on this deployment // (path prefix, non-Stalwart URL layout). Resolve the session's // advertised apiUrl on the same host and retry once. - const session = await fetchJmapSession(creds.serverUrl, creds.authHeader); + const session = await fetchJmapSession(creds.serverUrl, creds.authHeader, fetchOptions); const apiUrl = rebaseApiUrl(session, creds.serverUrl); if (apiUrl && apiUrl !== directUrl) { - response = await postJmap(apiUrl, creds.authHeader, body); + response = await postJmap(apiUrl, creds.authHeader, body, fetchOptions); } } @@ -55,6 +57,10 @@ export async function POST(request: NextRequest) { logger.error('Stalwart JMAP passthrough redirect error', { error: error.message }); return NextResponse.json({ error: error.message }, { status: 502 }); } + if (error instanceof DisallowedUrlError) { + logger.warn('Stalwart JMAP passthrough refused non-public server address', { error: error.message }); + return NextResponse.json({ error: 'JMAP server address is not allowed' }, { status: 502 }); + } // `fetch failed` from undici is too generic to debug — the real reason // (ENOTFOUND, ECONNREFUSED, self-signed TLS, …) lives on `error.cause`. const err = error as Error & { cause?: { code?: string; message?: string } }; diff --git a/app/api/admin/oauth/setup/route.ts b/app/api/admin/oauth/setup/route.ts index 81aa6cf1c..0bc03fb31 100644 --- a/app/api/admin/oauth/setup/route.ts +++ b/app/api/admin/oauth/setup/route.ts @@ -1,7 +1,8 @@ import { NextRequest, NextResponse } from 'next/server'; import { randomBytes } from 'node:crypto'; import { requireAdminAuth, getClientIP } from '@/lib/admin/session'; -import { getStalwartCredentials } from '@/lib/stalwart/credentials'; +import { getStalwartCredentials, type StalwartCredentials } from '@/lib/stalwart/credentials'; +import { fetchJmapServer } from '@/lib/stalwart/server-fetch'; import { configManager } from '@/lib/admin/config-manager'; import { auditLog } from '@/lib/admin/audit'; import { logger } from '@/lib/logger'; @@ -20,26 +21,31 @@ interface JmapMethodResponse { methodResponses?: Array<[string, Record, string]>; } -async function fetchWithTimeout(url: string, init: Parameters[1]): Promise { +type ServerCreds = Pick; + +async function fetchWithTimeout( + url: string, + init: Parameters[1], + trusted: boolean, +): Promise { const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), JMAP_TIMEOUT_MS); try { - return await fetch(url, { ...init, signal: controller.signal }); + return await fetchJmapServer(url, { ...init, signal: controller.signal }, trusted); } finally { clearTimeout(timer); } } async function jmapCall( - serverUrl: string, - authHeader: string, + creds: ServerCreds, body: JmapMethodCall, ): Promise { - const res = await fetchWithTimeout(`${serverUrl}/jmap/`, { + const res = await fetchWithTimeout(`${creds.serverUrl}/jmap/`, { method: 'POST', - headers: { 'Authorization': authHeader, 'Content-Type': 'application/json' }, + headers: { 'Authorization': creds.authHeader, 'Content-Type': 'application/json' }, body: JSON.stringify(body), - }); + }, creds.trusted); if (!res.ok) { const text = await res.text().catch(() => ''); throw new Error(`JMAP HTTP ${res.status} ${text.slice(0, 200)}`); @@ -47,14 +53,11 @@ async function jmapCall( return res.json() as Promise; } -async function getStalwartAccountId( - serverUrl: string, - authHeader: string, -): Promise { - const res = await fetchWithTimeout(`${serverUrl}/.well-known/jmap`, { +async function getStalwartAccountId(creds: ServerCreds): Promise { + const res = await fetchWithTimeout(`${creds.serverUrl}/.well-known/jmap`, { method: 'GET', - headers: { 'Authorization': authHeader }, - }); + headers: { 'Authorization': creds.authHeader }, + }, creds.trusted); if (!res.ok) return null; const session = await res.json() as { primaryAccounts?: Record }; return session.primaryAccounts?.['urn:stalwart:jmap'] @@ -119,7 +122,7 @@ export async function POST(request: NextRequest) { } const oauthOnly = body.oauthOnly === true; - const accountId = await getStalwartAccountId(creds.serverUrl, creds.authHeader); + const accountId = await getStalwartAccountId(creds); if (!accountId) { return NextResponse.json( { error: 'Could not resolve Stalwart account from JMAP session.' }, @@ -127,7 +130,7 @@ export async function POST(request: NextRequest) { ); } - const queryRes = await jmapCall(creds.serverUrl, creds.authHeader, { + const queryRes = await jmapCall(creds, { using: ['urn:ietf:params:jmap:core', 'urn:stalwart:jmap'], methodCalls: [[ 'x:OAuthClient/query', @@ -179,7 +182,7 @@ export async function POST(request: NextRequest) { }; } - const setRes = await jmapCall(creds.serverUrl, creds.authHeader, { + const setRes = await jmapCall(creds, { using: ['urn:ietf:params:jmap:core', 'urn:stalwart:jmap'], methodCalls: [['x:OAuthClient/set', setArgs, '0']], }); diff --git a/app/api/caldav/discover/route.ts b/app/api/caldav/discover/route.ts index 35d9bf2ce..1f82a1fc9 100644 --- a/app/api/caldav/discover/route.ts +++ b/app/api/caldav/discover/route.ts @@ -1,6 +1,7 @@ import { NextRequest, NextResponse } from 'next/server'; import { logger } from '@/lib/logger'; import { getStalwartCredentials } from '@/lib/stalwart/credentials'; +import { fetchJmapServer } from '@/lib/stalwart/server-fetch'; interface DiscoveryAccountRequest { key: string; @@ -16,9 +17,14 @@ function buildPublicUrl(serverUrl: string, path: string): string { return new URL(path, serverUrl).toString(); } -async function probeCalendarHome(serverUrl: string, authHeader: string, accountName: string): Promise { +async function probeCalendarHome( + serverUrl: string, + authHeader: string, + accountName: string, + trusted: boolean, +): Promise { const targetUrl = buildPublicUrl(serverUrl, `/dav/cal/${encodeURIComponent(accountName)}`); - const response = await fetch(targetUrl, { + const response = await fetchJmapServer(targetUrl, { method: 'PROPFIND', headers: { Authorization: authHeader, @@ -33,7 +39,7 @@ async function probeCalendarHome(serverUrl: string, authHeader: string, accountN `, redirect: 'manual', - }); + }, trusted); if (response.status === 207) { return targetUrl; @@ -75,7 +81,7 @@ export async function POST(request: NextRequest) { for (const candidate of candidates) { try { - url = await probeCalendarHome(creds.serverUrl, creds.authHeader, candidate); + url = await probeCalendarHome(creds.serverUrl, creds.authHeader, candidate, creds.trusted); if (url) { resolvedAccount = candidate; break; diff --git a/app/api/calendar-agenda/route.ts b/app/api/calendar-agenda/route.ts index c3b260845..4c345b958 100644 --- a/app/api/calendar-agenda/route.ts +++ b/app/api/calendar-agenda/route.ts @@ -121,7 +121,8 @@ export async function POST(request: NextRequest) { // configured public hostname, which the host process may not be able to // resolve (the browser client rewrites those URLs back to the origin for // the same reason). Fall back to /.well-known/jmap for non-Stalwart servers. - const session = await fetchJmapSession(creds.serverUrl, creds.authHeader); + const fetchOptions = { trusted: creds.trusted }; + const session = await fetchJmapSession(creds.serverUrl, creds.authHeader, fetchOptions); if (!session) { return NextResponse.json({ error: 'JMAP session fetch failed' }, { status: 502 }); } @@ -171,7 +172,7 @@ export async function POST(request: NextRequest) { ], }; - const queryRes = await jmapPost(apiUrl, creds.authHeader, queryReq); + const queryRes = await jmapPost(apiUrl, creds.authHeader, queryReq, fetchOptions); const queryResp = findResponse(queryRes, 'CalendarEvent/query', '0'); if (!queryResp) { const err = findResponse(queryRes, 'error', '0'); @@ -207,7 +208,7 @@ export async function POST(request: NextRequest) { methodCalls: [ ['CalendarEvent/get', { accountId, ids: batch, properties: EVENT_PROPERTIES }, '0'], ], - }); + }, fetchOptions); const getResp = findResponse(getRes, 'CalendarEvent/get', '0'); if (getResp?.list) raw.push(...(getResp.list as Array>)); } @@ -272,8 +273,9 @@ async function jmapPost( apiUrl: string, authHeader: string, payload: unknown, + options: { trusted?: boolean } = {}, ): Promise { - const res = await postJmap(apiUrl, authHeader, JSON.stringify(payload)); + const res = await postJmap(apiUrl, authHeader, JSON.stringify(payload), options); if (!res.ok) { throw new Error(`JMAP request failed (${res.status})`); } diff --git a/app/api/push/preview/route.ts b/app/api/push/preview/route.ts index a6bb7771c..80e5c752a 100644 --- a/app/api/push/preview/route.ts +++ b/app/api/push/preview/route.ts @@ -7,6 +7,8 @@ import { getStalwartCredentials, type StalwartCredentials, } from '@/lib/stalwart/credentials'; +import { fetchJmapServer, isTrustedJmapServerUrl } from '@/lib/stalwart/server-fetch'; +import { DisallowedUrlError } from '@/lib/security/url-guard'; export const runtime = 'nodejs'; export const dynamic = 'force-dynamic'; @@ -15,6 +17,8 @@ interface ResolvedTarget { authHeader: string; apiUrl: string; accountId: string; + /** See StalwartCredentials.trusted - false routes through the guarded fetch. */ + trusted: boolean; } // When the SW passes ?accountId=, we need the slot whose JMAP session owns @@ -31,9 +35,10 @@ async function resolveTargetForAccount(accountId: string): Promise { try { - const res = await fetch(`${serverUrl}/.well-known/jmap`, { + const trusted = await isTrustedJmapServerUrl(serverUrl); + const res = await fetchJmapServer(`${serverUrl}/.well-known/jmap`, { headers: { Authorization: ctx.authHeader }, - }); + }, trusted); if (!res.ok) return null; const session = (await res.json()) as { apiUrl?: string; @@ -42,7 +47,7 @@ async function resolveTargetForAccount(accountId: string): Promise { - const sessionRes = await fetch(`${creds.serverUrl}/.well-known/jmap`, { + const sessionRes = await fetchJmapServer(`${creds.serverUrl}/.well-known/jmap`, { headers: { Authorization: creds.authHeader }, - }); + }, creds.trusted); if (!sessionRes.ok) return null; const session = (await sessionRes.json()) as { apiUrl?: string; @@ -65,7 +70,7 @@ async function resolveDefaultTarget(creds: StalwartCredentials): Promise { + let current = new URL(url); + for (let hop = 0; hop <= MAX_REDIRECTS; hop++) { + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), FETCH_TIMEOUT_MS); + let res: Response; + try { + res = await fetch(current, { method, redirect: 'manual', signal: controller.signal }); + } finally { + clearTimeout(timer); + } + if (res.status < 300 || res.status >= 400) return res; + const location = res.headers.get('location'); + if (!location) return res; + const next = new URL(location, current); + const sameHost = next.host === current.host && next.protocol === current.protocol; + const httpsUpgrade = next.protocol === 'https:' && next.hostname === current.hostname; + if (!sameHost && !httpsUpgrade) { + throw new Error(`Redirected to a different host (${next.host})`); + } + await res.body?.cancel().catch(() => {}); + current = next; + } + throw new Error('Too many redirects'); +} /** * POST /api/setup/test-jmap - server-side probe of a JMAP server. Mirrors @@ -50,14 +83,7 @@ export async function POST(request: NextRequest) { for (const endpoint of JMAP_ENDPOINTS) { const target = base + endpoint; try { - const controller = new AbortController(); - const timer = setTimeout(() => controller.abort(), FETCH_TIMEOUT_MS); - const res = await fetch(target, { - method: 'GET', - redirect: 'follow', - signal: controller.signal, - }); - clearTimeout(timer); + const res = await probe(target, 'GET'); if (!res.ok) continue; const text = await res.text(); @@ -76,14 +102,7 @@ export async function POST(request: NextRequest) { // No JMAP session found. Was the server even reachable? try { - const controller = new AbortController(); - const timer = setTimeout(() => controller.abort(), FETCH_TIMEOUT_MS); - const res = await fetch(base, { - method: 'HEAD', - redirect: 'follow', - signal: controller.signal, - }); - clearTimeout(timer); + const res = await probe(base, 'HEAD'); return NextResponse.json({ status: 'reachable_no_jmap', httpStatus: res.status, diff --git a/app/api/webdav/route.ts b/app/api/webdav/route.ts index 64a8bca40..7a537d411 100644 --- a/app/api/webdav/route.ts +++ b/app/api/webdav/route.ts @@ -1,6 +1,8 @@ import { NextRequest, NextResponse } from 'next/server'; import { logger } from '@/lib/logger'; import { getStalwartCredentials } from '@/lib/stalwart/credentials'; +import { fetchJmapServer } from '@/lib/stalwart/server-fetch'; +import { DisallowedUrlError } from '@/lib/security/url-guard'; const ALLOWED_METHODS = new Set(['PROPFIND', 'MKCOL', 'MKCALENDAR', 'GET', 'PUT', 'DELETE', 'MOVE', 'COPY']); @@ -100,14 +102,16 @@ export async function POST(request: NextRequest) { body = request.body; } - const response = await fetch(targetUrl, { + // A user-chosen server (allowCustomJmapEndpoint) goes through the + // rebinding-safe fetch; see lib/stalwart/server-fetch.ts. + const response = await fetchJmapServer(targetUrl, { method, headers: upstreamHeaders, body, redirect: 'follow', // `duplex: 'half'` is required by undici when sending a streaming request body. ...(method === 'PUT' ? { duplex: 'half' } : {}), - } as Parameters[1] & { duplex?: 'half' }); + }, creds.trusted); // For file downloads (GET), stream the response back if (method === 'GET') { @@ -144,6 +148,10 @@ export async function POST(request: NextRequest) { if (error instanceof Error && error.message.startsWith('Invalid WebDAV path')) { return NextResponse.json({ error: error.message }, { status: 400 }); } + if (error instanceof DisallowedUrlError) { + logger.warn('WebDAV proxy refused non-public server address', { error: error.message }); + return NextResponse.json({ error: 'JMAP server address is not allowed' }, { status: 502 }); + } logger.error('WebDAV proxy error', { error: error instanceof Error ? error.message : 'Unknown' }); return NextResponse.json({ error: 'Internal server error' }, { status: 500 }); diff --git a/lib/__tests__/branding-asset.test.ts b/lib/__tests__/branding-asset.test.ts new file mode 100644 index 000000000..3c71a77d3 --- /dev/null +++ b/lib/__tests__/branding-asset.test.ts @@ -0,0 +1,44 @@ +import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; + +vi.mock('@/lib/admin/paths', () => ({ getConfigDir: () => '/config' })); +vi.mock('@/lib/security/url-guard', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, fetchPublicUrl: vi.fn() }; +}); + +import { fetchPublicUrl, DisallowedUrlError } from '@/lib/security/url-guard'; +import { fetchBrandingAsset } from '@/lib/admin/branding-asset'; + +const guardedFetch = fetchPublicUrl as unknown as Mock; + +beforeEach(() => { + guardedFetch.mockReset(); +}); + +describe('fetchBrandingAsset (remote URLs)', () => { + it('fetches remote assets through the rebinding-safe fetch', async () => { + guardedFetch.mockResolvedValueOnce(new Response('PNG', { status: 200 })); + const buf = await fetchBrandingAsset('https://cdn.example.com/logo.png', 'logo'); + expect(buf.toString()).toBe('PNG'); + expect(guardedFetch.mock.calls[0][0]).toBe('https://cdn.example.com/logo.png'); + }); + + it('re-validates every redirect hop', async () => { + guardedFetch + .mockResolvedValueOnce(new Response(null, { status: 302, headers: { location: 'https://cdn2.example.com/logo.png' } })) + .mockResolvedValueOnce(new Response('PNG2', { status: 200 })); + const buf = await fetchBrandingAsset('https://cdn.example.com/logo.png', 'logo'); + expect(buf.toString()).toBe('PNG2'); + expect(guardedFetch.mock.calls[1][0]).toBe('https://cdn2.example.com/logo.png'); + }); + + it('surfaces a blocked address as an error', async () => { + guardedFetch.mockRejectedValueOnce(new DisallowedUrlError('http://169.254.169.254/')); + await expect(fetchBrandingAsset('http://169.254.169.254/', 'logo')).rejects.toBeInstanceOf(DisallowedUrlError); + }); + + it('reports non-2xx upstream status', async () => { + guardedFetch.mockResolvedValueOnce(new Response(null, { status: 404 })); + await expect(fetchBrandingAsset('https://cdn.example.com/missing.png', 'logo')).rejects.toThrow('Failed to fetch logo: 404'); + }); +}); diff --git a/lib/__tests__/server-fetch.test.ts b/lib/__tests__/server-fetch.test.ts new file mode 100644 index 000000000..bb6b29acf --- /dev/null +++ b/lib/__tests__/server-fetch.test.ts @@ -0,0 +1,138 @@ +import { describe, it, expect, vi, beforeEach, afterEach, type Mock } from 'vitest'; + +vi.mock('@/lib/admin/config-manager', () => ({ + configManager: { + ensureLoaded: vi.fn(async () => {}), + get: vi.fn(), + }, +})); + +vi.mock('@/lib/security/url-guard', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + fetchPublicUrl: vi.fn(), + }; +}); + +import { configManager } from '@/lib/admin/config-manager'; +import { fetchPublicUrl } from '@/lib/security/url-guard'; +import { fetchJmapServer, isTrustedJmapServerUrl } from '@/lib/stalwart/server-fetch'; + +const mockGet = configManager.get as unknown as Mock; +const guardedFetch = fetchPublicUrl as unknown as Mock; +let plainFetch: Mock; + +function configure(values: Record) { + mockGet.mockImplementation((key: string, fallback?: unknown) => (key in values ? values[key] : fallback)); +} + +function response(status: number, headers: Record = {}, body: string | null = null) { + return new Response(status >= 300 && status < 400 ? null : body, { status, headers }); +} + +beforeEach(() => { + plainFetch = vi.fn(async () => response(200, {}, 'plain')); + vi.stubGlobal('fetch', plainFetch); + guardedFetch.mockReset(); + mockGet.mockReset(); + configure({}); +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe('isTrustedJmapServerUrl', () => { + it('trusts the global jmapServerUrl regardless of trailing slash or host case', async () => { + configure({ jmapServerUrl: 'https://Mail.Example.com/' }); + expect(await isTrustedJmapServerUrl('https://mail.example.com')).toBe(true); + }); + + it('trusts entries of the jmapServers list', async () => { + configure({ + jmapServers: [{ id: 'corp', label: 'Corp', url: 'http://10.0.0.5:8080', domains: ['corp.example'] }], + }); + expect(await isTrustedJmapServerUrl('http://10.0.0.5:8080')).toBe(true); + }); + + it('does not trust a URL that matches no configured server', async () => { + configure({ jmapServerUrl: 'https://mail.example.com' }); + expect(await isTrustedJmapServerUrl('https://evil.example')).toBe(false); + }); +}); + +describe('fetchJmapServer', () => { + it('uses the plain fetch for trusted servers', async () => { + const res = await fetchJmapServer('http://10.0.0.5/jmap/', { method: 'POST', body: '{}' }, true); + expect(res.status).toBe(200); + expect(plainFetch).toHaveBeenCalledTimes(1); + expect(guardedFetch).not.toHaveBeenCalled(); + }); + + it('routes untrusted servers through fetchPublicUrl', async () => { + guardedFetch.mockResolvedValueOnce(response(200, {}, 'guarded')); + const res = await fetchJmapServer( + 'https://custom.example/jmap/', + { method: 'POST', body: '{}', headers: { Authorization: 'Basic abc' } }, + false, + ); + expect(await res.text()).toBe('guarded'); + expect(plainFetch).not.toHaveBeenCalled(); + const [url, init] = guardedFetch.mock.calls[0]; + expect(url).toBe('https://custom.example/jmap/'); + expect(init.method).toBe('POST'); + expect(init.headers).toEqual({ authorization: 'Basic abc' }); + }); + + it('returns the 3xx untouched when redirect is manual', async () => { + guardedFetch.mockResolvedValueOnce(response(301, { location: 'https://custom.example/jmap' })); + const res = await fetchJmapServer('https://custom.example/jmap/', { redirect: 'manual' }, false); + expect(res.status).toBe(301); + expect(guardedFetch).toHaveBeenCalledTimes(1); + }); + + it('follows same-origin redirects, keeping Authorization', async () => { + guardedFetch + .mockResolvedValueOnce(response(308, { location: '/jmap/' })) + .mockResolvedValueOnce(response(200, {}, 'ok')); + const res = await fetchJmapServer( + 'https://custom.example/jmap', + { method: 'POST', body: '{}', headers: { Authorization: 'Basic abc' }, redirect: 'follow' }, + false, + ); + expect(res.status).toBe(200); + const [url, init] = guardedFetch.mock.calls[1]; + expect(url).toBe('https://custom.example/jmap/'); + expect(init.method).toBe('POST'); + expect(init.headers.authorization).toBe('Basic abc'); + }); + + it('drops Authorization on a cross-origin redirect and downgrades 302 POST to GET', async () => { + guardedFetch + .mockResolvedValueOnce(response(302, { location: 'https://other.example/x' })) + .mockResolvedValueOnce(response(200, {}, 'ok')); + await fetchJmapServer( + 'https://custom.example/jmap/', + { method: 'POST', body: '{}', headers: { Authorization: 'Basic abc', 'Content-Type': 'application/json' } }, + false, + ); + const [url, init] = guardedFetch.mock.calls[1]; + expect(url).toBe('https://other.example/x'); + expect(init.method).toBe('GET'); + expect(init.body).toBeUndefined(); + expect(init.headers.authorization).toBeUndefined(); + expect(init.headers['content-type']).toBeUndefined(); + }); + + it('gives up after too many redirects', async () => { + guardedFetch.mockResolvedValue(response(302, { location: '/loop' })); + await expect(fetchJmapServer('https://custom.example/loop', {}, false)).rejects.toThrow(/Too many redirects/); + }); + + it('propagates DisallowedUrlError from the guard', async () => { + const { DisallowedUrlError } = await import('@/lib/security/url-guard'); + guardedFetch.mockRejectedValueOnce(new DisallowedUrlError('http://custom.example/')); + await expect(fetchJmapServer('http://custom.example/', {}, false)).rejects.toBeInstanceOf(DisallowedUrlError); + }); +}); diff --git a/lib/admin/branding-asset.ts b/lib/admin/branding-asset.ts index 84398c265..65cc76630 100644 --- a/lib/admin/branding-asset.ts +++ b/lib/admin/branding-asset.ts @@ -9,10 +9,41 @@ import path from 'node:path'; import { readFile } from 'node:fs/promises'; import { getConfigDir } from '@/lib/admin/paths'; +import { fetchPublicUrl, type PublicFetchResponse } from '@/lib/security/url-guard'; /** Admin-uploaded assets are served from here but stored under getConfigDir()/branding/. */ const ADMIN_BRANDING_PREFIX = '/api/admin/branding/'; +const REMOTE_TIMEOUT_MS = 10_000; +const MAX_REDIRECTS = 5; + +/** + * Remote branding URLs come from admin config, but they are still fetched by + * this process and rendered by public routes, so they go through the + * rebinding-safe fetch: only public addresses, every redirect hop + * re-validated. Private-network logos should be uploaded via the admin + * branding upload instead of linked. + */ +async function fetchRemoteAsset(url: string, label: string): Promise { + let currentUrl = url; + let res: PublicFetchResponse | undefined; + for (let hop = 0; hop <= MAX_REDIRECTS; hop++) { + res = await fetchPublicUrl(currentUrl, { signal: AbortSignal.timeout(REMOTE_TIMEOUT_MS) }); + if (res.status >= 300 && res.status < 400) { + const location = res.headers.get('location'); + if (!location) break; + await res.body?.cancel().catch(() => {}); + currentUrl = new URL(location, currentUrl).toString(); + continue; + } + break; + } + if (!res || !res.ok) { + throw new Error(`Failed to fetch ${label}: ${res?.status ?? 'too many redirects'}`); + } + return Buffer.from(await res.arrayBuffer()); +} + /** * Read the bytes behind a branding URL: a remote http(s) URL, an * admin-uploaded asset, or a path relative to public/. @@ -22,9 +53,7 @@ const ADMIN_BRANDING_PREFIX = '/api/admin/branding/'; export async function fetchBrandingAsset(url: string, label = 'branding asset'): Promise { // Absolute URL (http/https) if (url.startsWith('http://') || url.startsWith('https://')) { - const res = await fetch(url); - if (!res.ok) throw new Error(`Failed to fetch ${label}: ${res.status}`); - return Buffer.from(await res.arrayBuffer()); + return fetchRemoteAsset(url, label); } if (url.startsWith(ADMIN_BRANDING_PREFIX)) { diff --git a/lib/stalwart/credentials.ts b/lib/stalwart/credentials.ts index 0c2cd5661..94a17f5ca 100644 --- a/lib/stalwart/credentials.ts +++ b/lib/stalwart/credentials.ts @@ -2,6 +2,7 @@ import { cookies } from 'next/headers'; import { NextRequest } from 'next/server'; import { sessionCookieName } from '@/lib/auth/session-cookie'; import { readStalwartAuthContextFromStore } from '@/lib/stalwart/auth-context'; +import { isTrustedJmapServerUrl } from '@/lib/stalwart/server-fetch'; import { MAX_ACCOUNT_SLOTS } from '@/lib/account-utils'; export interface StalwartCredentials { @@ -11,6 +12,13 @@ export interface StalwartCredentials { username: string; hasSessionCookie: boolean; slot: number; + /** + * Whether `serverUrl` matches an admin-configured server. A `false` here + * means the user picked the URL themselves (`allowCustomJmapEndpoint`), so + * server-side requests to it must use the rebinding-safe fetch - see + * `fetchJmapServer` in `@/lib/stalwart/server-fetch`. + */ + trusted: boolean; } function parseSlot(raw: string | null): number | null { @@ -35,12 +43,14 @@ export async function getStalwartCredentials(request: NextRequest): Promise; @@ -51,10 +61,11 @@ export async function postJmap( apiUrl: string, authHeader: string, body: string, + options: JmapFetchOptions = {}, ): Promise { let url = new URL(apiUrl); for (let attempt = 0; attempt <= MAX_REDIRECTS; attempt++) { - const response = await fetch(url, { + const response = await fetchJmapServer(url.toString(), { method: 'POST', headers: { 'Authorization': authHeader, @@ -62,7 +73,7 @@ export async function postJmap( }, body, redirect: 'manual', - }); + }, options.trusted); if (response.status < 300 || response.status >= 400) { return response; @@ -91,15 +102,16 @@ export async function postJmap( export async function fetchJmapSession( serverUrl: string, authHeader: string, + options: JmapFetchOptions = {}, ): Promise { const candidates = [`${serverUrl}/jmap/session`, `${serverUrl}/.well-known/jmap`]; for (const url of candidates) { try { - const res = await fetch(url, { + const res = await fetchJmapServer(url, { method: 'GET', headers: { Authorization: authHeader }, redirect: 'follow', - }); + }, options.trusted); if (!res.ok) continue; const session = (await res.json()) as JmapSessionDocument; if (session && typeof session === 'object' && session.primaryAccounts) { diff --git a/lib/stalwart/server-fetch.ts b/lib/stalwart/server-fetch.ts new file mode 100644 index 000000000..eac47724d --- /dev/null +++ b/lib/stalwart/server-fetch.ts @@ -0,0 +1,111 @@ +/** + * Server-side fetch for the JMAP server stored in a user's auth-context + * cookie. + * + * The stored `serverUrl` is trusted when it matches an admin-configured server + * (`jmapServerUrl` / `jmapServers`); such servers may legitimately live on a + * private network, so they get the plain `fetch`. When `allowCustomJmapEndpoint` + * is on, the URL was chosen by the user at login. It was pre-checked and the + * login verification was pinned, but a DNS name can be re-pointed at an + * internal address *after* login, so every later request to it must go through + * the rebinding-safe `fetchPublicUrl` (GHSA-24w9-8r42-8jwm, follow-up). + * + * Trust is derived from the current config on every read rather than persisted + * in the cookie: existing sessions keep working after an upgrade, and removing + * a server from the config immediately drops its sessions to the guarded path. + */ + +import { configManager } from '@/lib/admin/config-manager'; +import { parseJmapServers, resolveTrustedJmapUrl } from '@/lib/admin/jmap-servers'; +import { fetchPublicUrl, type PublicFetchResponse } from '@/lib/security/url-guard'; + +const MAX_REDIRECTS = 5; + +export async function isTrustedJmapServerUrl(serverUrl: string): Promise { + await configManager.ensureLoaded(); + const configuredServerUrl = + configManager.get('jmapServerUrl', '') || + process.env.JMAP_SERVER_URL || + process.env.NEXT_PUBLIC_JMAP_SERVER_URL || + ''; + const serverList = parseJmapServers(configManager.get('jmapServers', [])); + return resolveTrustedJmapUrl(serverUrl, configuredServerUrl, serverList) !== null; +} + +export type ServerFetchInit = RequestInit & { duplex?: 'half' }; + +function headersToRecord(init: HeadersInit | undefined): Record { + const out: Record = {}; + if (!init) return out; + const iterable = init instanceof Headers ? init : new Headers(init); + iterable.forEach((value, key) => { out[key] = value; }); + return out; +} + +/** + * Fetch `url` on behalf of a stored JMAP server. + * + * `trusted === false` routes through {@link fetchPublicUrl}; anything else + * (including `undefined`, which only test doubles produce - real credentials + * always carry an explicit boolean) uses the plain `fetch`. + * + * On the guarded path redirects are followed by hand so that every hop is + * re-validated: `Authorization` is dropped when the origin changes, and a + * 303 (or 301/302 on a POST) downgrades to GET like `fetch` would. Pass + * `redirect: 'manual'` to get the 3xx response back instead. + */ +export async function fetchJmapServer( + url: string, + init: ServerFetchInit = {}, + trusted: boolean | undefined, +): Promise { + if (trusted !== false) { + return fetch(url, init); + } + + const { redirect, headers: initHeaders, ...rest } = init; + const headers = headersToRecord(initHeaders); + let currentUrl = url; + let method = (rest.method ?? 'GET').toUpperCase(); + let body = rest.body; + + for (let hop = 0; hop <= MAX_REDIRECTS; hop++) { + const response: PublicFetchResponse = await fetchPublicUrl(currentUrl, { + ...(rest as Omit), + method, + body: body as never, + headers, + }); + + if (redirect === 'manual' || response.status < 300 || response.status >= 400) { + return response as unknown as Response; + } + + const location = response.headers.get('location'); + if (!location) { + return response as unknown as Response; + } + await response.body?.cancel().catch(() => {}); + + const next = new URL(location, currentUrl); + if (next.origin !== new URL(currentUrl).origin) { + for (const key of Object.keys(headers)) { + if (key.toLowerCase() === 'authorization' || key.toLowerCase() === 'cookie') { + delete headers[key]; + } + } + } + if (response.status === 303 || ((response.status === 301 || response.status === 302) && method === 'POST')) { + method = 'GET'; + body = undefined; + for (const key of Object.keys(headers)) { + if (key.toLowerCase() === 'content-type' || key.toLowerCase() === 'content-length') { + delete headers[key]; + } + } + } + currentUrl = next.toString(); + } + + throw new Error(`Too many redirects fetching ${url}`); +}