From cdf69d9b8f79032ecefa181f3be4755a963ddc4e Mon Sep 17 00:00:00 2001 From: Alix-007 Date: Tue, 21 Jul 2026 23:57:21 +0800 Subject: [PATCH] fix(tls): bound self-signed certificate generation (#109139) * fix(tls): bound self-signed certificate generation * fix(tls): stage generated certificate material * docs(changelog): note gateway TLS generation hardening Co-authored-by: Alix-007 * fix(tls): make generated pair publication recoverable Co-authored-by: Alix-007 * test(tls): narrow recovery directory fixture * fix(tls): harden atomic certificate publication * chore: drop PR changelog edit Co-authored-by: Alix-007 * fix(tls): degrade atomic publication failures Keep self-signed certificate generation bounded and staged, but fall back to exclusive best-effort writes when hard-link publication is unavailable. Emit a typed gateway TLS degradation warning and preserve startup availability on supported non-atomic filesystems.\n\nCo-authored-by: Alix-007 --------- Co-authored-by: Peter Steinberger --- src/infra/tls/gateway.test.ts | 139 ++++++++++++++++++++++++++++++-- src/infra/tls/gateway.ts | 146 +++++++++++++++++++++++++++------- 2 files changed, 250 insertions(+), 35 deletions(-) diff --git a/src/infra/tls/gateway.test.ts b/src/infra/tls/gateway.test.ts index 03917b810089..9ac67a57db01 100644 --- a/src/infra/tls/gateway.test.ts +++ b/src/infra/tls/gateway.test.ts @@ -1,16 +1,44 @@ // Covers gateway TLS loading, fingerprint reporting, generated certificate // paths, and error handling for missing or invalid material. import { X509Certificate } from "node:crypto"; -import { writeFile } from "node:fs/promises"; +import fs from "node:fs/promises"; import path from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { createTrackedTempDirs } from "../../test-utils/tracked-temp-dirs.js"; import { normalizeFingerprint } from "./fingerprint.js"; + +const { resolveSystemBinMock, runExecMock } = vi.hoisted(() => ({ + resolveSystemBinMock: vi.fn(() => "/usr/bin/openssl"), + runExecMock: vi.fn(), +})); + +vi.mock("../../process/exec.js", async (importOriginal) => ({ + ...(await importOriginal()), + runExec: runExecMock, +})); + +vi.mock("../resolve-system-bin.js", () => ({ resolveSystemBin: resolveSystemBinMock })); + import { loadGatewayTlsRuntime } from "./gateway.js"; const tempDirs = createTrackedTempDirs(); const createTempDir = () => tempDirs.make("openclaw-gateway-tls-test-"); +function resolveOpenSslOutput(args: string[], flag: "-keyout" | "-out"): string { + const outputPath = args.at(args.indexOf(flag) + 1); + if (!outputPath) { + throw new Error(`missing ${flag} output path`); + } + return outputPath; +} + +async function writeGeneratedTlsPair(args: string[]): Promise { + await Promise.all([ + fs.writeFile(resolveOpenSslOutput(args, "-out"), CERT_PEM, "utf8"), + fs.writeFile(resolveOpenSslOutput(args, "-keyout"), KEY_PEM, "utf8"), + ]); +} + const KEY_PEM = [ "-----BEGIN PRIVATE KEY-----", // pragma: allowlist secret "MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQDrur5CWp4psMMb", @@ -63,6 +91,8 @@ r1USnb+wUdA7Zoj/mQ== -----END CERTIFICATE-----`; afterEach(async () => { + resolveSystemBinMock.mockClear(); + runExecMock.mockReset(); await tempDirs.cleanup(); }); @@ -83,9 +113,9 @@ describe("loadGatewayTlsRuntime", () => { const certPath = path.join(dir, "gateway-cert.pem"); const keyPath = path.join(dir, "gateway-key.pem"); const caPath = path.join(dir, "gateway-ca.pem"); - await writeFile(certPath, CERT_PEM, "utf8"); - await writeFile(keyPath, KEY_PEM, "utf8"); - await writeFile(caPath, CERT_PEM, "utf8"); + await fs.writeFile(certPath, CERT_PEM, "utf8"); + await fs.writeFile(keyPath, KEY_PEM, "utf8"); + await fs.writeFile(caPath, CERT_PEM, "utf8"); const result = await loadGatewayTlsRuntime({ enabled: true, @@ -129,12 +159,107 @@ describe("loadGatewayTlsRuntime", () => { expect(result.error).toBe("gateway tls: cert/key missing"); }); + it.each(["key", "cert"] as const)( + "bounds generation and cleans a partial staged %s", + async (partialOutput) => { + const dir = await createTempDir(); + const certPath = path.join(dir, "gateway-cert.pem"); + const keyPath = path.join(dir, "gateway-key.pem"); + runExecMock.mockImplementationOnce(async (_command: string, args: string[]) => { + const outputPath = resolveOpenSslOutput(args, partialOutput === "key" ? "-keyout" : "-out"); + await fs.writeFile(outputPath, partialOutput === "key" ? KEY_PEM : CERT_PEM, "utf8"); + throw new Error("openssl timed out"); + }); + + const result = await loadGatewayTlsRuntime({ enabled: true, certPath, keyPath }); + + expect(runExecMock).toHaveBeenCalledOnce(); + const [command, args, options] = runExecMock.mock.calls[0] as [ + string, + string[], + { logOutput: boolean; timeoutMs: number }, + ]; + expect(command).toBe("/usr/bin/openssl"); + expect(resolveOpenSslOutput(args, "-keyout")).not.toBe(keyPath); + expect(resolveOpenSslOutput(args, "-out")).not.toBe(certPath); + expect(options).toEqual({ logOutput: false, timeoutMs: 30_000 }); + expect(result).toMatchObject({ enabled: false, required: true, certPath, keyPath }); + expect(result.error).toContain("openssl timed out"); + await expect(fs.access(certPath)).rejects.toMatchObject({ code: "ENOENT" }); + await expect(fs.access(keyPath)).rejects.toMatchObject({ code: "ENOENT" }); + await expect(fs.readdir(dir)).resolves.toEqual([]); + }, + ); + + it("validates and publishes a generated pair with private modes", async () => { + const dir = await createTempDir(); + const certPath = path.join(dir, "gateway-cert.pem"); + const keyPath = path.join(dir, "gateway-key.pem"); + runExecMock.mockImplementation(async (_command: string, args: string[]) => { + await writeGeneratedTlsPair(args); + }); + + const result = await loadGatewayTlsRuntime({ enabled: true, certPath, keyPath }); + + expect(result.enabled).toBe(true); + await expect(fs.readFile(certPath, "utf8")).resolves.toBe(CERT_PEM); + await expect(fs.readFile(keyPath, "utf8")).resolves.toBe(KEY_PEM); + await expect(fs.readdir(dir).then((entries) => entries.toSorted())).resolves.toEqual([ + "gateway-cert.pem", + "gateway-key.pem", + ]); + if (process.platform !== "win32") { + expect((await fs.stat(certPath)).mode & 0o777).toBe(0o600); + expect((await fs.stat(keyPath)).mode & 0o777).toBe(0o600); + } + }); + + it("publishes best-effort and warns once when hard links are unavailable", async () => { + const dir = await createTempDir(); + const certPath = path.join(dir, "gateway-cert.pem"); + const keyPath = path.join(dir, "gateway-key.pem"); + const warn = vi.fn(); + runExecMock.mockImplementation(async (_command: string, args: string[]) => { + await writeGeneratedTlsPair(args); + }); + const linkSpy = vi + .spyOn(fs, "link") + .mockRejectedValue(Object.assign(new Error("hard links unsupported"), { code: "ENOTSUP" })); + + let result: Awaited> | undefined; + try { + result = await loadGatewayTlsRuntime({ enabled: true, certPath, keyPath }, { warn }); + } finally { + linkSpy.mockRestore(); + } + + expect(result?.enabled).toBe(true); + expect(warn).toHaveBeenCalledOnce(); + expect(warn).toHaveBeenCalledWith(expect.stringContaining("[GATEWAY_TLS_DEGRADED]"), { + event: "gateway.tls.degraded", + ownerKind: "gateway", + ownerId: "tls", + reason: "atomic hard-link publication unavailable", + state: "best-effort", + }); + await expect(fs.readFile(certPath, "utf8")).resolves.toBe(CERT_PEM); + await expect(fs.readFile(keyPath, "utf8")).resolves.toBe(KEY_PEM); + await expect(fs.readdir(dir).then((entries) => entries.toSorted())).resolves.toEqual([ + "gateway-cert.pem", + "gateway-key.pem", + ]); + if (process.platform !== "win32") { + expect((await fs.stat(certPath)).mode & 0o777).toBe(0o600); + expect((await fs.stat(keyPath)).mode & 0o777).toBe(0o600); + } + }); + it("reports load failures for invalid pem files", async () => { const dir = await createTempDir(); const certPath = path.join(dir, "gateway-cert.pem"); const keyPath = path.join(dir, "gateway-key.pem"); - await writeFile(certPath, "not a certificate\n", "utf8"); - await writeFile(keyPath, KEY_PEM, "utf8"); + await fs.writeFile(certPath, "not a certificate\n", "utf8"); + await fs.writeFile(keyPath, KEY_PEM, "utf8"); const result = await loadGatewayTlsRuntime({ enabled: true, diff --git a/src/infra/tls/gateway.ts b/src/infra/tls/gateway.ts index 14e03cf2f09e..0ae5a2600b73 100644 --- a/src/infra/tls/gateway.ts +++ b/src/infra/tls/gateway.ts @@ -11,6 +11,61 @@ import { pathExists } from "../fs-safe.js"; import { resolveSystemBin } from "../resolve-system-bin.js"; import { normalizeFingerprint } from "./fingerprint.js"; +const GATEWAY_TLS_CERT_GENERATION_TIMEOUT_MS = 30_000; + +type GatewayTlsLog = { + info?: (message: string) => void; + warn?: (message: string, meta?: Record) => void; +}; + +type GatewayTlsDegradation = { + event: "gateway.tls.degraded"; + ownerKind: "gateway"; + ownerId: "tls"; + reason: "atomic hard-link publication unavailable"; + state: "best-effort"; +}; + +const GATEWAY_TLS_DEGRADATION: GatewayTlsDegradation = { + event: "gateway.tls.degraded", + ownerKind: "gateway", + ownerId: "tls", + reason: "atomic hard-link publication unavailable", + state: "best-effort", +}; + +function isHardLinkUnsupportedError(error: unknown): boolean { + const code = (error as NodeJS.ErrnoException | undefined)?.code; + return code === "ENOTSUP" || code === "EOPNOTSUPP" || code === "EPERM"; +} + +async function publishGeneratedTlsOutput( + stagedPath: string, + finalPath: string, + contents: string, +): Promise { + try { + await fs.link(stagedPath, finalPath); + return false; + } catch (error) { + if (!isHardLinkUnsupportedError(error)) { + throw error; + } + } + + // Some supported filesystems cannot publish with hard links. An exclusive handle keeps + // no-overwrite semantics without pathname cleanup that could delete concurrent output; + // a failed best-effort write may leave this attempt's partial file for operator cleanup. + const handle = await fs.open(finalPath, "wx", 0o600); + try { + await handle.writeFile(contents, "utf8"); + await handle.sync(); + } finally { + await handle.close(); + } + return true; +} + // Gateway TLS runtime carries loaded cert material plus the normalized SHA-256 // fingerprint advertised to clients. export type GatewayTlsRuntime = { @@ -27,7 +82,7 @@ export type GatewayTlsRuntime = { async function generateSelfSignedCert(params: { certPath: string; keyPath: string; - log?: { info?: (msg: string) => void }; + log?: GatewayTlsLog; }): Promise { const certDir = path.dirname(params.certPath); const keyDir = path.dirname(params.keyPath); @@ -41,39 +96,74 @@ async function generateSelfSignedCert(params: { "openssl not found in trusted system directories. Install it in an OS-managed location.", ); } - // Use argv execution with a trusted system binary; certificate paths are arguments, - // not shell text. - await runExec( - opensslBin, - [ - "req", - "-x509", - "-newkey", - "rsa:2048", - "-sha256", - "-days", - "3650", - "-nodes", - "-keyout", - params.keyPath, - "-out", + const certStageDir = await fs.mkdtemp(path.join(certDir, ".openclaw-gateway-tls-cert-")); + const stagedCertPath = path.join(certStageDir, "cert.pem"); + let keyStageDir: string | undefined; + try { + keyStageDir = await fs.mkdtemp(path.join(keyDir, ".openclaw-gateway-tls-key-")); + const stagedKeyPath = path.join(keyStageDir, "key.pem"); + await Promise.all([fs.chmod(certStageDir, 0o700), fs.chmod(keyStageDir, 0o700)]); + // OpenSSL never sees the configured final paths, so timeout and generation + // failures cannot strand a half-written certificate pair there. + await runExec( + opensslBin, + [ + "req", + "-x509", + "-newkey", + "rsa:2048", + "-sha256", + "-days", + "3650", + "-nodes", + "-keyout", + stagedKeyPath, + "-out", + stagedCertPath, + "-subj", + "/CN=openclaw-gateway", + ], + { + logOutput: false, + timeoutMs: GATEWAY_TLS_CERT_GENERATION_TIMEOUT_MS, + }, + ); + await Promise.all([fs.chmod(stagedKeyPath, 0o600), fs.chmod(stagedCertPath, 0o600)]); + const [cert, key] = await Promise.all([ + fs.readFile(stagedCertPath, "utf8"), + fs.readFile(stagedKeyPath, "utf8"), + ]); + tls.createSecureContext({ cert, key, minVersion: "TLSv1.3" }); + let usedBestEffortPublication = await publishGeneratedTlsOutput( + stagedCertPath, params.certPath, - "-subj", - "/CN=openclaw-gateway", - ], - { logOutput: false }, - ); - await fs.chmod(params.keyPath, 0o600).catch(() => {}); - await fs.chmod(params.certPath, 0o600).catch(() => {}); - params.log?.info?.( - `gateway tls: generated self-signed cert at ${shortenHomeInString(params.certPath)}`, - ); + cert, + ); + usedBestEffortPublication = + (await publishGeneratedTlsOutput(stagedKeyPath, params.keyPath, key)) || + usedBestEffortPublication; + if (usedBestEffortPublication) { + params.log?.warn?.( + `[GATEWAY_TLS_DEGRADED] best-effort gateway:tls: ${GATEWAY_TLS_DEGRADATION.reason}.`, + GATEWAY_TLS_DEGRADATION, + ); + } + params.log?.info?.( + `gateway tls: generated self-signed cert at ${shortenHomeInString(params.certPath)}`, + ); + } finally { + await Promise.allSettled( + [certStageDir, keyStageDir] + .filter((dir): dir is string => Boolean(dir)) + .map((dir) => fs.rm(dir, { force: true, recursive: true })), + ); + } } /** Load or generate gateway TLS material and return server-ready TLS options. */ export async function loadGatewayTlsRuntime( cfg: GatewayTlsConfig | undefined, - log?: { info?: (msg: string) => void; warn?: (msg: string) => void }, + log?: GatewayTlsLog, ): Promise { if (!cfg || cfg.enabled !== true) { return { enabled: false, required: false };