diff --git a/changelog.d/fixes/13679-deploy-manifest-literal-secrets.md b/changelog.d/fixes/13679-deploy-manifest-literal-secrets.md new file mode 100644 index 0000000000..fdb470929e --- /dev/null +++ b/changelog.d/fixes/13679-deploy-manifest-literal-secrets.md @@ -0,0 +1 @@ +- **fix(security):** removed the copy-pasteable placeholder `JWT_SECRET`/`API_KEY_SECRET`/`INITIAL_PASSWORD` values from the Podman Quadlet deploy manifest, and blocked remote dashboard logins with the well-known default `INITIAL_PASSWORD=CHANGEME` (#13679) diff --git a/contrib/podman/README.md b/contrib/podman/README.md index 41ee3651d7..c008077a87 100644 --- a/contrib/podman/README.md +++ b/contrib/podman/README.md @@ -40,7 +40,28 @@ cp contrib/podman/*.network ~/.config/containers/systemd/omniroute/ cp contrib/podman/*.volume ~/.config/containers/systemd/omniroute/ ``` -### 3. Mount the project .env for secrets +### 3. Generate secrets before first start + +`omniroute.container` no longer ships `JWT_SECRET` / `API_KEY_SECRET` / +`INITIAL_PASSWORD` values — earlier versions shipped copy-pasteable +placeholders (`change-me-to-a-random-base64-string`, +`change-me-to-a-random-hex-string`) that an operator could forget to +rotate, leaving the deployment with a public, guessable secret/password +(#13679). Generate real ones and put them in your project `.env`: + +```bash +echo "JWT_SECRET=$(openssl rand -base64 48)" >> .env +echo "API_KEY_SECRET=$(openssl rand -hex 32)" >> .env +echo "INITIAL_PASSWORD=$(openssl rand -hex 24)" >> .env +``` + +If you skip this: `JWT_SECRET`/`API_KEY_SECRET` are auto-generated and +persisted on first boot, and the dashboard requires setup from `localhost` +before it accepts any password — safer than a literal default, but a real +`INITIAL_PASSWORD` is still recommended so a non-interactive first boot has +a known credential to log in with. + +### 4. Mount the project .env for secrets Edit `~/.config/containers/systemd/omniroute/omniroute.container` and uncomment/replace the `EnvironmentFile` line with the absolute path to @@ -54,7 +75,7 @@ Make sure `CONTAINER_HOST=podman` is set in that `.env`. Alternatively, edit the env vars directly in the `.container` file. -### 4. Reload systemd and start +### 5. Reload systemd and start ```bash systemctl --user daemon-reload @@ -62,7 +83,7 @@ systemctl --user start omniroute-redis systemctl --user start omniroute ``` -### 5. Verify +### 6. Verify ```bash systemctl --user status omniroute diff --git a/contrib/podman/omniroute.container b/contrib/podman/omniroute.container index 31d85257e1..9e2f33d982 100644 --- a/contrib/podman/omniroute.container +++ b/contrib/podman/omniroute.container @@ -28,14 +28,23 @@ Environment=DASHBOARD_PORT=20128 Environment=API_PORT=20129 Environment=API_HOST=0.0.0.0 Environment=REDIS_URL=redis://redis:6379 -Environment=JWT_SECRET=change-me-to-a-random-base64-string -Environment=API_KEY_SECRET=change-me-to-a-random-base64-string -Environment=INITIAL_PASSWORD=change-me-to-a-random-hex-string Environment=NODE_ENV=production Environment=REQUIRE_API_KEY=true -# Load additional secrets (API keys, OAuth creds) from the project .env: +# JWT_SECRET, API_KEY_SECRET and INITIAL_PASSWORD are deliberately NOT set here. +# This unit used to ship copy-pasteable "replace-me" placeholder literals — an +# operator who forgot to replace them ran production with a public, guessable +# secret and dashboard password (#13679). Generate real values and load them from +# your project .env before the FIRST start — see "Generate secrets before first +# start" in contrib/podman/README.md — by uncommenting and pointing this at your +# project .env: # EnvironmentFile=%h/code/docker/OmniRoute/.env +# +# If left unset: JWT_SECRET and API_KEY_SECRET are auto-generated and persisted +# on first boot, and the dashboard requires setup from localhost before it +# accepts any password (see managementPassword.ts / apiAuth.ts) — safer than a +# known-literal default either way, but a real INITIAL_PASSWORD is still +# recommended for non-interactive first boots. HealthCmd=node /app/healthcheck.mjs HealthInterval=30s diff --git a/src/app/api/auth/login/route.ts b/src/app/api/auth/login/route.ts index c4142a4768..558e3c64f6 100644 --- a/src/app/api/auth/login/route.ts +++ b/src/app/api/auth/login/route.ts @@ -8,6 +8,7 @@ import { cookies } from "next/headers"; import { ensurePersistentManagementPasswordHash, getStoredManagementPassword, + isKnownInsecureManagementPassword, verifyManagementPassword, } from "@/lib/auth/managementPassword"; import { isFeatureFlagEnabled } from "@/shared/utils/featureFlags"; @@ -153,6 +154,41 @@ export async function POST(request: NextRequest) { const isValid = await verifyManagementPassword(password, storedHash); + // #8336: tag the origin scope so the audit view can distinguish a mistyped + // password from the host itself / the LAN (loopback / private) from a + // genuinely external attempt, instead of every failure reading as intrusion. + // Computed once and reused below for the #13679 insecure-default gate. + const sourceScope = classifyIpScope(auditContext.ipAddress); + + // #13679 (PR D, item #5): the well-known INITIAL_PASSWORD placeholder shipped + // in .env.example / contrib/podman/omniroute.container / docker deploy + // manifests is a public, guessable credential. Anyone who knows it (i.e. + // everyone) can otherwise sign in from anywhere the dashboard is reachable. + // `ensurePersistentManagementPasswordHash()` already warns loudly on boot, + // but that is a log line, not a control — refuse the login here instead + // whenever it matches AND the request is not loopback, forcing the operator + // to rotate the password from a trusted local console first. + if (isValid && isKnownInsecureManagementPassword(password) && sourceScope !== "loopback") { + logAuditEvent({ + action: "auth.login.insecure_default_blocked", + actor: "anonymous", + target: "dashboard-auth", + resourceType: "auth_session", + status: "failed", + ipAddress: auditContext.ipAddress || undefined, + requestId: auditContext.requestId, + metadata: { reason: "well_known_default_password_non_loopback", sourceScope }, + }); + return NextResponse.json( + { + error: + "The management password is still set to the well-known default. " + + "Log in from localhost and change it before signing in remotely.", + }, + { status: 403 } + ); + } + if (isValid) { const forceSecureCookie = process.env.AUTH_COOKIE_SECURE === "true"; const forwardedProtoHeader = request.headers.get("x-forwarded-proto") || ""; @@ -197,11 +233,6 @@ export async function POST(request: NextRequest) { const failureDecision = recordLoginFailure(clientIp, { enabled: bruteForceEnabled }); - // #8336: tag the origin scope so the audit view can distinguish a mistyped - // password from the host itself / the LAN (loopback / private) from a - // genuinely external attempt, instead of every failure reading as intrusion. - const sourceScope = classifyIpScope(auditContext.ipAddress); - logAuditEvent({ action: "auth.login.failed", actor: "anonymous", diff --git a/src/lib/auth/managementPassword.ts b/src/lib/auth/managementPassword.ts index 2b18c011ae..dfc8549d12 100644 --- a/src/lib/auth/managementPassword.ts +++ b/src/lib/auth/managementPassword.ts @@ -45,6 +45,14 @@ export function isBcryptHash(value: unknown): value is string { return typeof value === "string" && BCRYPT_HASH_PATTERN.test(value); } +// #13679 (PR D, item #5): the well-known INITIAL_PASSWORD placeholder shipped in +// .env.example / contrib/podman/omniroute.container / docker deploy manifests is a +// public, guessable credential. Callers on the authentication path use this to refuse +// a successful match from non-loopback requests instead of only warning on boot. +export function isKnownInsecureManagementPassword(password: string): boolean { + return INSECURE_DEFAULT_PASSWORDS.has(password); +} + export async function hashManagementPassword(password: string) { return bcrypt.hash(password, MANAGEMENT_PASSWORD_SALT_ROUNDS); } diff --git a/tests/unit/13679-insecure-default-password-nonloopback-login.test.ts b/tests/unit/13679-insecure-default-password-nonloopback-login.test.ts new file mode 100644 index 0000000000..e1737f0f2a --- /dev/null +++ b/tests/unit/13679-insecure-default-password-nonloopback-login.test.ts @@ -0,0 +1,116 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +/** + * Regression test for issue #13679 (PR D, item #5) — deploy manifests + * (`contrib/podman/omniroute.container`, `.env.example`) ship the well-known + * placeholder `INITIAL_PASSWORD=CHANGEME`. `ensurePersistentManagementPasswordHash()` + * already warns loudly on boot when the bootstrap password is this literal, but it + * does NOT stop a remote attacker who simply tries the well-known default from + * logging in over the network — only a local console warning fires. + * + * Fix: `/api/auth/login` now refuses a successful password match against a + * known-insecure default (e.g. "CHANGEME") when the request does not originate + * from loopback, forcing the operator to log in from localhost and rotate the + * password before the dashboard is reachable from the network with the + * default credential. + */ + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-13679d-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.JWT_SECRET = "test-jwt-secret-13679d"; + +const ORIGINAL_INITIAL_PASSWORD = process.env.INITIAL_PASSWORD; + +const core = await import("../../src/lib/db/core.ts"); +const compliance = await import("../../src/lib/compliance/index.ts"); +const loginRoute = await import("../../src/app/api/auth/login/route.ts"); + +const originalGetCookieStore = loginRoute.authRouteInternals.getCookieStore; + +async function resetStorage() { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); + process.env.INITIAL_PASSWORD = "CHANGEME"; +} + +test.beforeEach(async () => { + await resetStorage(); + loginRoute.authRouteInternals.getCookieStore = async () => ({ set() {} }); +}); + +test.afterEach(() => { + loginRoute.authRouteInternals.getCookieStore = originalGetCookieStore; +}); + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + if (ORIGINAL_INITIAL_PASSWORD === undefined) { + delete process.env.INITIAL_PASSWORD; + } else { + process.env.INITIAL_PASSWORD = ORIGINAL_INITIAL_PASSWORD; + } +}); + +function postLogin(password: string, forwardedFor: string) { + return loginRoute.POST( + new Request("http://localhost/api/auth/login", { + method: "POST", + headers: { + "content-type": "application/json", + "x-forwarded-for": forwardedFor, + }, + body: JSON.stringify({ password }), + }) + ); +} + +test("a public-IP login with the well-known default password CHANGEME is rejected", async () => { + const response = await postLogin("CHANGEME", "203.0.113.77"); + + assert.equal( + response.status, + 403, + "a remote attacker guessing the well-known default INITIAL_PASSWORD must not be able to " + + "authenticate — only a console warning fires today, which is not a real control" + ); + assert.equal(response.headers.get("set-cookie"), null, "no session cookie must be issued"); + + const [entry] = compliance.getAuditLog({ + action: "auth.login.insecure_default_blocked", + limit: 1, + }); + assert.ok( + entry, + "expected an auth.login.insecure_default_blocked audit entry for the blocked attempt" + ); +}); + +test("a loopback login with the well-known default password CHANGEME still succeeds", async () => { + const response = await postLogin("CHANGEME", "127.0.0.1"); + + assert.equal( + response.status, + 200, + "the operator must still be able to bootstrap/rotate the password from loopback" + ); + const body = await response.json(); + assert.equal(body.success, true); +}); + +test("a public-IP login with a non-default (rotated) password still succeeds", async () => { + process.env.INITIAL_PASSWORD = "a-real-rotated-password-13679d"; + const response = await postLogin("a-real-rotated-password-13679d", "203.0.113.77"); + + assert.equal( + response.status, + 200, + "the insecure-default check must not block legitimate remote logins once the password " + + "has actually been rotated away from the well-known default" + ); +}); diff --git a/tests/unit/13679-podman-manifest-no-literal-secrets.test.ts b/tests/unit/13679-podman-manifest-no-literal-secrets.test.ts new file mode 100644 index 0000000000..ac023a0c6a --- /dev/null +++ b/tests/unit/13679-podman-manifest-no-literal-secrets.test.ts @@ -0,0 +1,54 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; + +/** + * Regression test for issue #13679 (PR D, item #4) — `contrib/podman/omniroute.container` + * used to ship copy-pasteable placeholder secrets: + * JWT_SECRET=change-me-to-a-random-base64-string + * API_KEY_SECRET=change-me-to-a-random-base64-string + * INITIAL_PASSWORD=change-me-to-a-random-hex-string + * + * An operator who forgot to replace them ran production with a public, guessable + * JWT/API-key secret and dashboard password (anyone can find these literals on GitHub). + * The fix removes the literal `Environment=` lines for these three variables and + * documents generating real values via `contrib/podman/README.md` before first start. + */ + +const MANIFEST_PATH = path.join( + import.meta.dirname, + "..", + "..", + "contrib", + "podman", + "omniroute.container" +); +const README_PATH = path.join(import.meta.dirname, "..", "..", "contrib", "podman", "README.md"); + +test("omniroute.container no longer ships a literal placeholder secret value", () => { + const content = fs.readFileSync(MANIFEST_PATH, "utf8"); + + assert.ok( + !/change-me-to-a-random/i.test(content), + "the Quadlet unit must not ship a copy-pasteable placeholder secret literal" + ); + + for (const varName of ["JWT_SECRET", "API_KEY_SECRET", "INITIAL_PASSWORD"]) { + const literalAssignment = new RegExp(`^Environment=${varName}=\\S+`, "m"); + assert.ok( + !literalAssignment.test(content), + `${varName} must not be assigned a literal value directly in the checked-in unit file` + ); + } +}); + +test("podman README documents generating secrets before first start", () => { + const readme = fs.readFileSync(README_PATH, "utf8"); + assert.match( + readme, + /Generate secrets before first start/i, + "README must document the generate-secrets step referenced by the unit file's comments" + ); + assert.match(readme, /openssl rand/, "README must give a concrete generation command"); +});