src-advisory/SRCmail
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: stop pulling node:dns into client bundle via OAuth discovery

Browse Source
This commit is contained in:
Linus Rath
2026-05-18 19:31:38 +02:00
parent 43ac0725ce
commit ecd0467ffa
5 changed files with 54 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/api/auth/sso/start/route.ts
+2 -1
View File
@@ -5,6 +5,7 @@ import { encryptPayload } from '@/lib/auth/crypto';
import { generateCodeVerifierServer, generateCodeChallengeServer, generateStateServer } from '@/lib/oauth/pkce-server'; import { generateCodeVerifierServer, generateCodeChallengeServer, generateStateServer } from '@/lib/oauth/pkce-server';
import { getRequiredConfig } from '@/lib/oauth/token-exchange'; import { getRequiredConfig } from '@/lib/oauth/token-exchange';
import { discoverOAuth } from '@/lib/oauth/discovery'; import { discoverOAuth } from '@/lib/oauth/discovery';
import { isPublicHttpUrl } from '@/lib/security/url-guard';
import { getOauthScopes } from '@/lib/oauth/tokens'; import { getOauthScopes } from '@/lib/oauth/tokens';
import { getCookieOptions } from '@/lib/oauth/cookie-config'; import { getCookieOptions } from '@/lib/oauth/cookie-config';
import { hasSessionSecret } from '@/lib/auth/session-secret'; import { hasSessionSecret } from '@/lib/auth/session-secret';
@@ -39,7 +40,7 @@ export async function POST(request: NextRequest) {
} }
const { clientId, discoveryUrl } = getRequiredConfig(serverId); const { clientId, discoveryUrl } = getRequiredConfig(serverId);
const metadata = await discoverOAuth(discoveryUrl); const metadata = await discoverOAuth(discoveryUrl, { validateEndpoint: isPublicHttpUrl });
if (!metadata?.authorization_endpoint) { if (!metadata?.authorization_endpoint) {
return NextResponse.json({ error: 'OAuth discovery failed' }, { status: 502 }); return NextResponse.json({ error: 'OAuth discovery failed' }, { status: 502 });
app/api/auth/totp-token-exchange/route.ts
+1 -1
View File
@@ -54,7 +54,7 @@ async function tryTokenRequest(
async function findTokenEndpoint(serverUrl: string): Promise<string | null> { async function findTokenEndpoint(serverUrl: string): Promise<string | null> {
// 1. Try OAuth discovery // 1. Try OAuth discovery
const metadata = await discoverOAuth(serverUrl); const metadata = await discoverOAuth(serverUrl, { validateEndpoint: isPublicHttpUrl });
if (metadata?.token_endpoint) return metadata.token_endpoint; if (metadata?.token_endpoint) return metadata.token_endpoint;
// 2. Try common Stalwart token endpoint paths directly // 2. Try common Stalwart token endpoint paths directly
lib/__tests__/oauth-discovery.test.ts
+23 -25
View File
@@ -1,22 +1,20 @@
import { describe, it, expect, vi, beforeEach } from 'vitest'; import { describe, it, expect, vi, beforeEach } from 'vitest';
import type { OAuthMetadata } from '../oauth/discovery'; import type { OAuthMetadata } from '../oauth/discovery';
vi.mock('../security/url-guard', () => ({ const validateEndpoint = async (urlString: string) => {
isPublicHttpUrl: vi.fn(async (urlString: string) => { try {
try { const url = new URL(urlString);
const url = new URL(urlString); if (url.protocol !== 'http:' && url.protocol !== 'https:') return false;
if (url.protocol !== 'http:' && url.protocol !== 'https:') return false; if (url.username || url.password) return false;
if (url.username || url.password) return false; const host = url.hostname.toLowerCase();
const host = url.hostname.toLowerCase(); if (host === 'localhost' || host.endsWith('.local') || host.endsWith('.internal')) return false;
if (host === 'localhost' || host.endsWith('.local') || host.endsWith('.internal')) return false; if (/^(127\.|169\.254\.|10\.|192\.168\.)/.test(host)) return false;
if (/^(127\.|169\.254\.|10\.|192\.168\.)/.test(host)) return false; if (host === '::1' || host === '0.0.0.0') return false;
if (host === '::1' || host === '0.0.0.0') return false; return true;
return true; } catch {
} catch { return false;
return false; }
} };
}),
}));
const VALID_METADATA: OAuthMetadata = { const VALID_METADATA: OAuthMetadata = {
issuer: 'https://auth.example.com', issuer: 'https://auth.example.com',
@@ -43,7 +41,7 @@ describe('oauth/discovery', () => {
json: () => Promise.resolve(VALID_METADATA), json: () => Promise.resolve(VALID_METADATA),
})); }));
const result = await discoverOAuth('https://mail.example.com'); const result = await discoverOAuth('https://mail.example.com', { validateEndpoint });
expect(result).toEqual(VALID_METADATA); expect(result).toEqual(VALID_METADATA);
expect(fetch).toHaveBeenCalledTimes(1); expect(fetch).toHaveBeenCalledTimes(1);
@@ -60,7 +58,7 @@ describe('oauth/discovery', () => {
json: () => Promise.resolve(VALID_METADATA), json: () => Promise.resolve(VALID_METADATA),
})); }));
const result = await discoverOAuth('https://fallback.example.com'); const result = await discoverOAuth('https://fallback.example.com', { validateEndpoint });
expect(result).toEqual(VALID_METADATA); expect(result).toEqual(VALID_METADATA);
expect(fetch).toHaveBeenCalledTimes(2); expect(fetch).toHaveBeenCalledTimes(2);
@@ -76,7 +74,7 @@ describe('oauth/discovery', () => {
.mockResolvedValueOnce({ ok: false, status: 404 }) .mockResolvedValueOnce({ ok: false, status: 404 })
.mockResolvedValueOnce({ ok: false, status: 404 })); .mockResolvedValueOnce({ ok: false, status: 404 }));
const result = await discoverOAuth('https://fail.example.com'); const result = await discoverOAuth('https://fail.example.com', { validateEndpoint });
expect(result).toBeNull(); expect(result).toBeNull();
expect(consoleSpy).toHaveBeenCalled(); expect(consoleSpy).toHaveBeenCalled();
@@ -88,7 +86,7 @@ describe('oauth/discovery', () => {
json: () => Promise.resolve(VALID_METADATA), json: () => Promise.resolve(VALID_METADATA),
})); }));
const result = await discoverOAuth('https://optional.example.com'); const result = await discoverOAuth('https://optional.example.com', { validateEndpoint });
expect(result?.revocation_endpoint).toBe('https://auth.example.com/revoke'); expect(result?.revocation_endpoint).toBe('https://auth.example.com/revoke');
expect(result?.end_session_endpoint).toBe('https://auth.example.com/logout'); expect(result?.end_session_endpoint).toBe('https://auth.example.com/logout');
@@ -103,7 +101,7 @@ describe('oauth/discovery', () => {
}) })
.mockResolvedValueOnce({ ok: false, status: 404 })); .mockResolvedValueOnce({ ok: false, status: 404 }));
const result = await discoverOAuth('https://incomplete.example.com'); const result = await discoverOAuth('https://incomplete.example.com', { validateEndpoint });
expect(result).toBeNull(); expect(result).toBeNull();
expect(consoleSpy).toHaveBeenCalled(); expect(consoleSpy).toHaveBeenCalled();
@@ -122,7 +120,7 @@ describe('oauth/discovery', () => {
}) })
.mockResolvedValueOnce({ ok: false, status: 404 })); .mockResolvedValueOnce({ ok: false, status: 404 }));
const result = await discoverOAuth('https://evil.example.com'); const result = await discoverOAuth('https://evil.example.com', { validateEndpoint });
expect(result).toBeNull(); expect(result).toBeNull();
expect(consoleSpy).toHaveBeenCalled(); expect(consoleSpy).toHaveBeenCalled();
@@ -142,7 +140,7 @@ describe('oauth/discovery', () => {
}) })
.mockResolvedValueOnce({ ok: false, status: 404 })); .mockResolvedValueOnce({ ok: false, status: 404 }));
const result = await discoverOAuth('https://private-revoke.example.com'); const result = await discoverOAuth('https://private-revoke.example.com', { validateEndpoint });
expect(result).toBeNull(); expect(result).toBeNull();
expect(consoleSpy).toHaveBeenCalled(); expect(consoleSpy).toHaveBeenCalled();
@@ -154,8 +152,8 @@ describe('oauth/discovery', () => {
json: () => Promise.resolve(VALID_METADATA), json: () => Promise.resolve(VALID_METADATA),
})); }));
const first = await discoverOAuth('https://cached.example.com'); const first = await discoverOAuth('https://cached.example.com', { validateEndpoint });
const second = await discoverOAuth('https://cached.example.com'); const second = await discoverOAuth('https://cached.example.com', { validateEndpoint });
expect(first).toEqual(VALID_METADATA); expect(first).toEqual(VALID_METADATA);
expect(second).toEqual(VALID_METADATA); expect(second).toEqual(VALID_METADATA);
lib/oauth/discovery.ts
+25 -8
View File
@@ -1,5 +1,3 @@
import { isPublicHttpUrl } from '../security/url-guard';
export interface OAuthMetadata { export interface OAuthMetadata {
issuer: string; issuer: string;
authorization_endpoint: string; authorization_endpoint: string;
@@ -8,6 +6,17 @@ export interface OAuthMetadata {
end_session_endpoint?: string; end_session_endpoint?: string;
} }
// Validates that a discovered endpoint URL is safe to follow. Server-side
// callers must pass this to gate against SSRF (typically isPublicHttpUrl from
// @/lib/security/url-guard, which uses node:dns and cannot be bundled for the
// browser). Client callers omit it: the browser handles outbound networking
// and an SSRF check isn't meaningful there.
export type EndpointValidator = (url: string) => Promise<boolean>;
export interface DiscoverOAuthOptions {
validateEndpoint?: EndpointValidator;
}
const CACHE_TTL_MS = 10 * 60 * 1000; const CACHE_TTL_MS = 10 * 60 * 1000;
const CACHE_MAX_ENTRIES = 64; const CACHE_MAX_ENTRIES = 64;
const metadataCache = new Map<string, { metadata: OAuthMetadata; expiresAt: number }>(); const metadataCache = new Map<string, { metadata: OAuthMetadata; expiresAt: number }>();
@@ -26,19 +35,27 @@ function rememberMetadata(serverUrl: string, metadata: OAuthMetadata): void {
// Endpoints come from an attacker-controllable JSON document when callers pass // Endpoints come from an attacker-controllable JSON document when callers pass
// a user-supplied serverUrl (e.g. /api/auth/totp-token-exchange under // a user-supplied serverUrl (e.g. /api/auth/totp-token-exchange under
// allowCustomJmapEndpoint). Without this gate, a malicious metadata document // allowCustomJmapEndpoint). Without a validator, a malicious metadata document
// could point token_endpoint at 169.254.169.254 or 127.0.0.1:* and turn the // could point token_endpoint at 169.254.169.254 or 127.0.0.1:* and turn the
// downstream fetch() into an SSRF with response-body reflection. // downstream fetch() into an SSRF with response-body reflection. Server-side
async function endpointsArePublic(endpoints: Array<string | undefined>): Promise<boolean> { // callers must pass `validateEndpoint`.
async function endpointsArePublic(
endpoints: Array<string | undefined>,
validate: EndpointValidator | undefined,
): Promise<boolean> {
if (!validate) return true;
for (const endpoint of endpoints) { for (const endpoint of endpoints) {
if (endpoint === undefined) continue; if (endpoint === undefined) continue;
if (typeof endpoint !== 'string') return false; if (typeof endpoint !== 'string') return false;
if (!(await isPublicHttpUrl(endpoint))) return false; if (!(await validate(endpoint))) return false;
} }
return true; return true;
} }
export async function discoverOAuth(serverUrl: string): Promise<OAuthMetadata | null> { export async function discoverOAuth(
serverUrl: string,
options?: DiscoverOAuthOptions,
): Promise<OAuthMetadata | null> {
const cached = metadataCache.get(serverUrl); const cached = metadataCache.get(serverUrl);
if (cached && cached.expiresAt > Date.now()) return cached.metadata; if (cached && cached.expiresAt > Date.now()) return cached.metadata;
if (cached) metadataCache.delete(serverUrl); if (cached) metadataCache.delete(serverUrl);
@@ -65,7 +82,7 @@ export async function discoverOAuth(serverUrl: string): Promise<OAuthMetadata |
data.token_endpoint, data.token_endpoint,
data.revocation_endpoint, data.revocation_endpoint,
data.end_session_endpoint, data.end_session_endpoint,
]); ], options?.validateEndpoint);
if (!allPublic) { if (!allPublic) {
errors.push(`${url} returned non-public or invalid endpoint URL`); errors.push(`${url} returned non-public or invalid endpoint URL`);
continue; continue;
lib/oauth/token-exchange.ts
+3 -2
View File
@@ -1,6 +1,7 @@
import { logger } from '@/lib/logger'; import { logger } from '@/lib/logger';
import { discoverOAuth } from '@/lib/oauth/discovery'; import { discoverOAuth } from '@/lib/oauth/discovery';
import type { OAuthMetadata } from '@/lib/oauth/discovery'; import type { OAuthMetadata } from '@/lib/oauth/discovery';
import { isPublicHttpUrl } from '@/lib/security/url-guard';
import { readFileEnv } from '@/lib/read-file-env'; import { readFileEnv } from '@/lib/read-file-env';
import { configManager } from '@/lib/admin/config-manager'; import { configManager } from '@/lib/admin/config-manager';
import { parseJmapServers, findServerById } from '@/lib/admin/jmap-servers'; import { parseJmapServers, findServerById } from '@/lib/admin/jmap-servers';
@@ -46,7 +47,7 @@ function getClientSecret(serverId?: string | null): string {
export async function getTokenEndpoint(serverId?: string | null): Promise<string> { export async function getTokenEndpoint(serverId?: string | null): Promise<string> {
const { discoveryUrl } = getRequiredConfig(serverId); const { discoveryUrl } = getRequiredConfig(serverId);
const metadata = await discoverOAuth(discoveryUrl); const metadata = await discoverOAuth(discoveryUrl, { validateEndpoint: isPublicHttpUrl });
if (!metadata?.token_endpoint) { if (!metadata?.token_endpoint) {
throw new Error('OAuth token endpoint not found'); throw new Error('OAuth token endpoint not found');
} }
@@ -55,7 +56,7 @@ export async function getTokenEndpoint(serverId?: string | null): Promise<string
export async function getMetadata(serverId?: string | null): Promise<OAuthMetadata | null> { export async function getMetadata(serverId?: string | null): Promise<OAuthMetadata | null> {
const { discoveryUrl } = getRequiredConfig(serverId); const { discoveryUrl } = getRequiredConfig(serverId);
return discoverOAuth(discoveryUrl); return discoverOAuth(discoveryUrl, { validateEndpoint: isPublicHttpUrl });
} }
export function buildOAuthParams(base: Record<string, string>, serverId?: string | null): URLSearchParams { export function buildOAuthParams(base: Record<string, string>, serverId?: string | null): URLSearchParams {