diff --git a/.env.example b/.env.example index 70d62ee2eb..05cee19c8f 100644 --- a/.env.example +++ b/.env.example @@ -6,10 +6,14 @@ NEXTAUTH_SECRET="secret" # [[CRYPTO]] # Application Key for symmetric encryption and decryption -# REQUIRED: This should be a random string of at least 32 characters -NEXT_PRIVATE_ENCRYPTION_KEY="CAFEBABE" -# REQUIRED: This should be a random string of at least 32 characters -NEXT_PRIVATE_ENCRYPTION_SECONDARY_KEY="DEADBEEF" +# REQUIRED: a random string of at least 32 characters. The server refuses to +# boot when these are missing, shorter than 32 characters, equal to each other, +# or still set to the published placeholders CAFEBABE / DEADBEEF. +# Generate one with: openssl rand -base64 32 +NEXT_PRIVATE_ENCRYPTION_KEY="" +# REQUIRED: a random string of at least 32 characters, different from the above. +# Generate one with: openssl rand -base64 32 +NEXT_PRIVATE_ENCRYPTION_SECONDARY_KEY="" # [[AUTH OPTIONAL]] # Find documentation on setting up Google OAuth here: @@ -35,7 +39,10 @@ NEXT_PRIVATE_OIDC_PROMPT="login" NEXT_PUBLIC_WEBAPP_URL="http://localhost:3000" # URL used by the web app to request itself (e.g. local background jobs) NEXT_PRIVATE_INTERNAL_WEBAPP_URL="http://localhost:3000" -# OPTIONAL: Comma-separated hostnames or IPs whose webhooks are allowed to resolve to private/loopback addresses. (e.g., internal.example.com,192.168.1.5). +# OPTIONAL: Comma-separated hostnames or IPs allowed to resolve to private/loopback addresses. (e.g., internal.example.com,192.168.1.5). +# Applies to outbound webhook calls AND to OpenID Connect discovery +# (wellKnownUrl), so a self-hosted identity provider such as a Keycloak on +# localhost must be listed here or SSO configuration will be rejected. NEXT_PRIVATE_WEBHOOK_SSRF_BYPASS_HOSTS= # [[SERVER]] diff --git a/apps/remix/server/router.ts b/apps/remix/server/router.ts index c6212455b6..14a06cf2ab 100644 --- a/apps/remix/server/router.ts +++ b/apps/remix/server/router.ts @@ -1,6 +1,7 @@ import { tsRestHonoApp } from '@documenso/api/hono'; import { auth } from '@documenso/auth/server'; import { csc } from '@documenso/ee/server-only/signing/csc/hono'; +import { assertEncryptionKeysConfigured } from '@documenso/lib/constants/crypto'; import { jobsClient } from '@documenso/lib/jobs/client'; import { LicenseClient } from '@documenso/lib/server-only/license/license-client'; import { createRateLimitMiddleware } from '@documenso/lib/server-only/rate-limit/rate-limit-middleware'; @@ -41,6 +42,13 @@ import { reactRouterTrpcServer } from './trpc/hono-trpc-remix'; // output to wire it into the React Router adapter. export { getLoadContext } from './load-context'; +// Refuse to serve traffic with encryption keys that are missing, still set to a +// published placeholder, or too short to survive an offline attack. Everything +// this app describes as encrypted at rest - DKIM private keys, SSO client +// secrets, account-link tokens - depends on these two values, and a weak key +// cannot be detected after data has already been written with it. +assertEncryptionKeysConfigured(); + export interface HonoEnv { Variables: RequestIdVariables & { context: AppContext; diff --git a/docker/testing/compose.yml b/docker/testing/compose.yml index 7251ee523c..e9a97161f8 100644 --- a/docker/testing/compose.yml +++ b/docker/testing/compose.yml @@ -35,8 +35,11 @@ services: - ../../.env.example environment: - NEXTAUTH_SECRET=secret - - NEXT_PRIVATE_ENCRYPTION_KEY=CAFEBABE - - NEXT_PRIVATE_ENCRYPTION_SECONDARY_KEY=DEADBEEF + # Test-only values. Must satisfy the boot guard in + # packages/lib/constants/crypto.ts: at least 32 characters, not equal to + # each other, and not one of the published placeholders. + - NEXT_PRIVATE_ENCRYPTION_KEY=testing-only-encryption-key-0123456789 + - NEXT_PRIVATE_ENCRYPTION_SECONDARY_KEY=testing-only-secondary-key-0123456789 - NEXT_PRIVATE_DATABASE_URL=postgres://documenso:password@database:5432/documenso - NEXT_PRIVATE_DIRECT_DATABASE_URL=postgres://documenso:password@database:5432/documenso - NEXT_PUBLIC_UPLOAD_TRANSPORT=database diff --git a/packages/auth/server/lib/utils/open-id.ts b/packages/auth/server/lib/utils/open-id.ts index cd89c241dd..ca45e830ee 100644 --- a/packages/auth/server/lib/utils/open-id.ts +++ b/packages/auth/server/lib/utils/open-id.ts @@ -1,3 +1,4 @@ +import { assertNotPrivateUrl } from '@documenso/lib/server-only/webhooks/assert-webhook-url'; import { z } from 'zod'; const ZOpenIdConfigurationSchema = z.object({ @@ -17,6 +18,21 @@ export const getOpenIdConfiguration = async ( wellKnownUrl: string, _options: GetOpenIdConfigurationOptions = {}, ): Promise => { + // The discovery URL is operator-supplied — it comes from an organisation's + // authentication portal row or from NEXT_PRIVATE_OIDC_WELL_KNOWN — so it is + // treated like any other outbound target. A self-hosted identity provider on a + // private address must be listed in NEXT_PRIVATE_WEBHOOK_SSRF_BYPASS_HOSTS. + // + // Best-effort, like every caller of this helper: it resolves DNS separately + // from the fetch below so it does not defeat rebinding, and it fails open on + // lookup errors or timeouts. Egress filtering at the deployment level remains + // the real control. + try { + await assertNotPrivateUrl(wellKnownUrl); + } catch (error) { + throw new Error('OIDC discovery URL resolves to a private or loopback address', { cause: error }); + } + const response = await fetch(wellKnownUrl); if (!response.ok) { diff --git a/packages/lib/constants/crypto.test.ts b/packages/lib/constants/crypto.test.ts new file mode 100644 index 0000000000..9d3ba32258 --- /dev/null +++ b/packages/lib/constants/crypto.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { describeEncryptionKeyProblem, MINIMUM_ENCRYPTION_KEY_LENGTH } from './crypto'; + +const VALID_KEY = 'a'.repeat(MINIMUM_ENCRYPTION_KEY_LENGTH); +const VALID_SECONDARY_KEY = 'b'.repeat(MINIMUM_ENCRYPTION_KEY_LENGTH); + +describe('describeEncryptionKeyProblem', () => { + it('accepts two distinct keys that meet the minimum length', () => { + expect(describeEncryptionKeyProblem(VALID_KEY, VALID_SECONDARY_KEY)).toBeNull(); + }); + + it('accepts keys longer than the minimum', () => { + expect(describeEncryptionKeyProblem(`${VALID_KEY}extra`, `${VALID_SECONDARY_KEY}extra`)).toBeNull(); + }); + + it('rejects a missing primary key', () => { + expect(describeEncryptionKeyProblem(undefined, VALID_SECONDARY_KEY)).toContain('both required'); + }); + + it('rejects an empty secondary key', () => { + expect(describeEncryptionKeyProblem(VALID_KEY, '')).toContain('both required'); + }); + + it('rejects the published placeholder values in either slot', () => { + expect(describeEncryptionKeyProblem('CAFEBABE', 'DEADBEEF')).toContain('placeholder'); + expect(describeEncryptionKeyProblem(VALID_KEY, 'DEADBEEF')).toContain('placeholder'); + expect(describeEncryptionKeyProblem('CAFEBABE', VALID_SECONDARY_KEY)).toContain('placeholder'); + }); + + it('rejects a key one character below the minimum', () => { + const tooShort = 'a'.repeat(MINIMUM_ENCRYPTION_KEY_LENGTH - 1); + + expect(describeEncryptionKeyProblem(tooShort, VALID_SECONDARY_KEY)).toContain( + `at least ${MINIMUM_ENCRYPTION_KEY_LENGTH} characters`, + ); + }); + + it('rejects identical keys so that rotation can tell old ciphertext from new', () => { + expect(describeEncryptionKeyProblem(VALID_KEY, VALID_KEY)).toContain('must differ'); + }); + + it('reports the placeholder problem before the length problem, since it is the more specific one', () => { + // CAFEBABE is also shorter than the minimum; naming it as a placeholder is + // what tells the operator they copied a shipped default rather than typed a + // short secret. + expect(describeEncryptionKeyProblem('CAFEBABE', VALID_SECONDARY_KEY)).not.toContain('at least'); + }); +}); + +describe('assertEncryptionKeysConfigured', () => { + it('refuses to start when the process environment still holds the shipped defaults', async () => { + vi.stubEnv('NEXT_PRIVATE_ENCRYPTION_KEY', 'CAFEBABE'); + vi.stubEnv('NEXT_PRIVATE_ENCRYPTION_SECONDARY_KEY', 'DEADBEEF'); + vi.resetModules(); + + try { + const reloaded = await import('./crypto'); + + expect(() => reloaded.assertEncryptionKeysConfigured()).toThrow(/Refusing to start/); + expect(() => reloaded.assertEncryptionKeysConfigured()).toThrow(/placeholder/); + } finally { + vi.unstubAllEnvs(); + vi.resetModules(); + } + }); + + it('starts when the process environment holds usable keys', async () => { + vi.stubEnv('NEXT_PRIVATE_ENCRYPTION_KEY', VALID_KEY); + vi.stubEnv('NEXT_PRIVATE_ENCRYPTION_SECONDARY_KEY', VALID_SECONDARY_KEY); + vi.resetModules(); + + try { + const reloaded = await import('./crypto'); + + expect(() => reloaded.assertEncryptionKeysConfigured()).not.toThrow(); + } finally { + vi.unstubAllEnvs(); + vi.resetModules(); + } + }); +}); diff --git a/packages/lib/constants/crypto.ts b/packages/lib/constants/crypto.ts index 8bf232632f..cff1ae608c 100644 --- a/packages/lib/constants/crypto.ts +++ b/packages/lib/constants/crypto.ts @@ -4,24 +4,64 @@ export const DOCUMENSO_ENCRYPTION_KEY = env('NEXT_PRIVATE_ENCRYPTION_KEY'); export const DOCUMENSO_ENCRYPTION_SECONDARY_KEY = env('NEXT_PRIVATE_ENCRYPTION_SECONDARY_KEY'); -// if (typeof window === 'undefined') { -// if (!DOCUMENSO_ENCRYPTION_KEY || !DOCUMENSO_ENCRYPTION_SECONDARY_KEY) { -// throw new Error('Missing DOCUMENSO_ENCRYPTION_KEY or DOCUMENSO_ENCRYPTION_SECONDARY_KEY keys'); -// } - -// if (DOCUMENSO_ENCRYPTION_KEY === DOCUMENSO_ENCRYPTION_SECONDARY_KEY) { -// throw new Error( -// 'DOCUMENSO_ENCRYPTION_KEY and DOCUMENSO_ENCRYPTION_SECONDARY_KEY cannot be equal', -// ); -// } -// } - -// if (DOCUMENSO_ENCRYPTION_KEY === 'CAFEBABE') { -// console.warn('*********************************************************************'); -// console.warn('*'); -// console.warn('*'); -// console.warn('Please change the encryption key from the default value of "CAFEBABE"'); -// console.warn('*'); -// console.warn('*'); -// console.warn('*********************************************************************'); -// } +/** + * Both values are used as symmetric AEAD keys (see `universal/crypto.ts`). + * A short key is not merely weak: anyone who can read a single ciphertext can + * brute-force the key offline and then decrypt everything else protected by it, + * which in this app includes DKIM private keys and SSO client secrets. + */ +export const MINIMUM_ENCRYPTION_KEY_LENGTH = 32; + +/** + * Placeholder values that `.env.example` used to ship and that + * `docker/Dockerfile` still bakes in as build-time ENV defaults. They are public + * knowledge, so an instance running on them has no encryption at all. + */ +const PLACEHOLDER_ENCRYPTION_KEYS = ['CAFEBABE', 'DEADBEEF']; + +const GENERATE_HINT = 'Generate one with: openssl rand -base64 32'; + +/** + * Explain why the configured encryption keys are unusable, or return null when + * they are acceptable. Pure and parameterised so it can be tested without + * touching process.env or re-importing the module. + */ +export const describeEncryptionKeyProblem = ( + key: string | undefined, + secondaryKey: string | undefined, +): string | null => { + if (!key || !secondaryKey) { + return `NEXT_PRIVATE_ENCRYPTION_KEY and NEXT_PRIVATE_ENCRYPTION_SECONDARY_KEY are both required. ${GENERATE_HINT}`; + } + + if (PLACEHOLDER_ENCRYPTION_KEYS.includes(key) || PLACEHOLDER_ENCRYPTION_KEYS.includes(secondaryKey)) { + return `An encryption key is still set to a published placeholder (${PLACEHOLDER_ENCRYPTION_KEYS.join(' / ')}), which is not a secret. ${GENERATE_HINT}`; + } + + if (key.length < MINIMUM_ENCRYPTION_KEY_LENGTH || secondaryKey.length < MINIMUM_ENCRYPTION_KEY_LENGTH) { + return `Both encryption keys must be at least ${MINIMUM_ENCRYPTION_KEY_LENGTH} characters. ${GENERATE_HINT}`; + } + + if (key === secondaryKey) { + return 'NEXT_PRIVATE_ENCRYPTION_KEY and NEXT_PRIVATE_ENCRYPTION_SECONDARY_KEY must differ, otherwise rotation cannot tell old ciphertext from new.'; + } + + return null; +}; + +/** + * Refuse to serve traffic with unusable encryption keys. + * + * Called from the server entry point rather than at module load on purpose: the + * Docker image carries the placeholder values as ENV defaults, so throwing while + * modules are being imported would fail the image build itself, and every test + * or one-off script that imports this module would need a full server + * environment to do so. + */ +export const assertEncryptionKeysConfigured = (): void => { + const problem = describeEncryptionKeyProblem(DOCUMENSO_ENCRYPTION_KEY, DOCUMENSO_ENCRYPTION_SECONDARY_KEY); + + if (problem) { + throw new Error(`Refusing to start: ${problem}`); + } +};