From 7180b763c0847a8a0c947e21309bcafd0ec69de3 Mon Sep 17 00:00:00 2001 From: germondai Date: Fri, 4 Sep 2026 02:22:41 +0200 Subject: [PATCH] fix(proxy): add strict MITM certificate identifiers --- CHANGELOG.md | 1 + apps/api/src/proxy/ca.test.ts | 189 +++++++++++++++++++++++++++++ apps/api/src/proxy/ca.ts | 151 ++++++++++++++++++++--- apps/docs/proxy/ca-installation.md | 25 +++- 4 files changed, 350 insertions(+), 16 deletions(-) create mode 100644 apps/api/src/proxy/ca.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index c125815..1eab5bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Optional viewport screenshot: `screenshot: true` on `POST /scrape` returns a base64 JPEG of the viewport in `ScrapeResult.screenshot`, captured by the browser tiers (2-4) immediately before the HTML read so image and markup describe the same moment. Off by default; a stock request attaches nothing and does no extra work. Settle wait, capture timeout, JPEG quality, and maximum image size are bounded and tunable via `SCREENSHOT_*`, and a capture failure leaves the field unset rather than failing the scrape. ### Fixed +- Add RFC 5280 Subject Key Identifiers to generated MITM roots and leaf certificates and a matching Authority Key Identifier to leaves, restoring compatibility with strict TLS clients such as Python 3.13 (#113). Existing roots missing SKI are re-signed once with the same CA key and identity fields; initialization is serialized across processes and invalid existing identifiers fail safely. Because migration changes the certificate fingerprint, existing MITM deployments should download and re-import the updated `ca.crt` into client trust stores. - Recover the Redis-backed Tier 2 cache after a transient startup timeout without restarting TRAWL. Connection attempts are bounded by `REDIS_CONNECT_TIMEOUT_MS`, retry in the background after `REDIS_RETRY_DELAY_MS`, and stop cleanly during shutdown; setting the retry delay to `0` disables reconnects for intentionally cacheless deployments (#92). - Correct Docker troubleshooting commands to use the actual `trawl` Compose service name, and distinguish Prowlarr's always-available FlareSolverr API on port 8191 from the opt-in forward proxy on port 8192 (#96). - Wait for the bundled Redis service to pass a `PING` healthcheck before starting TRAWL, preventing a transient Compose startup race from disabling the Tier 2 session cache for the process lifetime (#90). diff --git a/apps/api/src/proxy/ca.test.ts b/apps/api/src/proxy/ca.test.ts new file mode 100644 index 0000000..1a939f5 --- /dev/null +++ b/apps/api/src/proxy/ca.test.ts @@ -0,0 +1,189 @@ +import { afterEach, describe, expect, test } from "bun:test" +import { existsSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs" +import { tmpdir } from "node:os" +import { join } from "node:path" +import forge from "node-forge" +import { MitmCa } from "./ca" + +const temporaryDirectories: string[] = [] + +afterEach(() => { + for (const dir of temporaryDirectories.splice(0)) rmSync(dir, { recursive: true, force: true }) +}) + +describe("MitmCa certificates", () => { + test("new roots and leaves contain matching key identifiers and pass strict verification", () => { + const dir = temporaryDirectory() + const ca = new MitmCa(dir) + const root = certificate(ca.caCertPem) + const leafPem = ca.leafCertPem("example.test") + const leaf = certificate(leafPem) + + const rootSki = extension(root, "subjectKeyIdentifier") + const leafSki = extension(leaf, "subjectKeyIdentifier") + const leafAki = extension(leaf, "authorityKeyIdentifier") + expect(rootSki.critical).not.toBe(true) + expect(leafSki.critical).not.toBe(true) + expect(leafAki.critical).not.toBe(true) + expect(authorityKeyIdentifier(leafAki)).toBe(rootSki.subjectKeyIdentifier as string) + expect(existsSync(join(dir, ".ca.lock"))).toBe(false) + + verifyStrict(dir, ca.caCertPem, leafPem) + }) + + test("migrates a legacy root once without changing its identity fields or private key", () => { + const dir = temporaryDirectory() + const legacy = createLegacyCa() + const certPath = join(dir, "ca.crt") + const keyPath = join(dir, "ca.key") + const originalKeyPem = forge.pki.privateKeyToPem(legacy.key) + const originalCertPem = forge.pki.certificateToPem(legacy.cert) + const originalCert = certificate(originalCertPem) + writeFileSync(certPath, originalCertPem) + writeFileSync(keyPath, originalKeyPem) + + const ca = new MitmCa(dir) + const migrated = certificate(ca.caCertPem) + expect(extension(migrated, "subjectKeyIdentifier").subjectKeyIdentifier).toBe( + migrated.generateSubjectKeyIdentifier().toHex(), + ) + expect(readFileSync(keyPath, "utf8")).toBe(originalKeyPem) + expect(migrated.subject.attributes).toEqual(originalCert.subject.attributes) + expect(migrated.serialNumber).toBe(originalCert.serialNumber) + expect(migrated.validity.notBefore.getTime()).toBe(originalCert.validity.notBefore.getTime()) + expect(migrated.validity.notAfter.getTime()).toBe(originalCert.validity.notAfter.getTime()) + + const leafPem = ca.leafCertPem("migrated.example") + verifyStrict(dir, ca.caCertPem, leafPem) + + const inodeAfterMigration = statSync(certPath).ino + const pemAfterMigration = readFileSync(certPath, "utf8") + const loadedAgain = new MitmCa(dir) + expect(statSync(certPath).ino).toBe(inodeAfterMigration) + expect(loadedAgain.caCertPem).toBe(pemAfterMigration) + }) + + test("rejects a certificate and private key that do not match", () => { + const dir = temporaryDirectory() + const legacy = createLegacyCa() + const other = forge.pki.rsa.generateKeyPair(2048) + writeFileSync(join(dir, "ca.crt"), forge.pki.certificateToPem(legacy.cert)) + writeFileSync(join(dir, "ca.key"), forge.pki.privateKeyToPem(other.privateKey)) + + expect(() => new MitmCa(dir)).toThrow(/Cannot safely load MITM CA.*public key does not match ca\.key/) + expect(existsSync(join(dir, ".ca.lock"))).toBe(false) + }) + + test("rejects an existing Subject Key Identifier that does not match the CA public key", () => { + const dir = temporaryDirectory() + const malformed = createCaWithIncorrectSki() + writeFileSync(join(dir, "ca.crt"), forge.pki.certificateToPem(malformed.cert)) + writeFileSync(join(dir, "ca.key"), forge.pki.privateKeyToPem(malformed.key)) + + expect(() => new MitmCa(dir)).toThrow(/Subject Key Identifier that does not match its public key/) + expect(existsSync(join(dir, ".ca.lock"))).toBe(false) + }) + + test("rejects an incomplete persisted CA instead of silently rotating it", () => { + const dir = temporaryDirectory() + writeFileSync(join(dir, "ca.crt"), "not used") + expect(() => new MitmCa(dir)).toThrow(/Incomplete MITM CA.*refusing to generate a new identity/) + }) + + test("serializes concurrent first-time initialization across processes", async () => { + const dir = temporaryDirectory() + const projectRoot = join(import.meta.dir, "../../../..") + const script = `import { MitmCa } from "./apps/api/src/proxy/ca.ts"; new MitmCa(process.argv.at(-1)!)` + const processes = [ + Bun.spawn([process.execPath, "-e", script, dir], { cwd: projectRoot, stderr: "pipe" }), + Bun.spawn([process.execPath, "-e", script, dir], { cwd: projectRoot, stderr: "pipe" }), + ] + const exitCodes = await Promise.all(processes.map((process) => process.exited)) + + expect(exitCodes).toEqual([0, 0]) + expect(existsSync(join(dir, ".ca.lock"))).toBe(false) + expect(() => new MitmCa(dir)).not.toThrow() + }) +}) + +function temporaryDirectory(): string { + const dir = mkdtempSync(join(tmpdir(), "trawl-ca-test-")) + temporaryDirectories.push(dir) + return dir +} + +function certificate(pem: string): forge.pki.Certificate { + return forge.pki.certificateFromPem(pem) +} + +type ParsedExtension = forge.pki.CertificateField & { + critical?: boolean + subjectKeyIdentifier?: string + value: string +} + +function extension(cert: forge.pki.Certificate, name: string): ParsedExtension { + const found = cert.getExtension({ name }) + if (!found) throw new Error(`Missing ${name} extension`) + return found as ParsedExtension +} + +function authorityKeyIdentifier(ext: ParsedExtension): string { + const sequence = forge.asn1.fromDer(ext.value) + const keyIdentifier = (sequence.value as forge.asn1.Asn1[]).find((entry) => entry.type === 0) + if (!keyIdentifier || typeof keyIdentifier.value !== "string") throw new Error("Missing AKI keyIdentifier") + return forge.util.bytesToHex(keyIdentifier.value) +} + +function createLegacyCa(): { cert: forge.pki.Certificate; key: forge.pki.rsa.PrivateKey } { + const keys = forge.pki.rsa.generateKeyPair(2048) + const cert = forge.pki.createCertificate() + cert.publicKey = keys.publicKey + cert.serialNumber = "00112233445566778899aabbccddeeff" + cert.validity.notBefore = new Date("2025-01-01T00:00:00Z") + cert.validity.notAfter = new Date("2035-01-01T00:00:00Z") + const attrs = [ + { name: "commonName", value: "Legacy TRAWL MITM CA" }, + { name: "organizationName", value: "TRAWL" }, + ] + cert.setSubject(attrs) + cert.setIssuer(attrs) + cert.setExtensions([ + { name: "basicConstraints", cA: true, critical: true }, + { name: "keyUsage", keyCertSign: true, cRLSign: true, critical: true }, + ]) + cert.sign(keys.privateKey, forge.md.sha256.create()) + return { cert, key: keys.privateKey } +} + +function createCaWithIncorrectSki(): { cert: forge.pki.Certificate; key: forge.pki.rsa.PrivateKey } { + const originalKeys = forge.pki.rsa.generateKeyPair(2048) + const actualKeys = forge.pki.rsa.generateKeyPair(2048) + const cert = forge.pki.createCertificate() + cert.publicKey = originalKeys.publicKey + cert.serialNumber = "00aabbccddeeff" + cert.validity.notBefore = new Date("2025-01-01T00:00:00Z") + cert.validity.notAfter = new Date("2035-01-01T00:00:00Z") + const attrs = [{ name: "commonName", value: "Malformed TRAWL MITM CA" }] + cert.setSubject(attrs) + cert.setIssuer(attrs) + cert.setExtensions([ + { name: "basicConstraints", cA: true, critical: true }, + { name: "keyUsage", keyCertSign: true, cRLSign: true, critical: true }, + { name: "subjectKeyIdentifier" }, + ]) + cert.publicKey = actualKeys.publicKey + cert.sign(actualKeys.privateKey, forge.md.sha256.create()) + return { cert, key: actualKeys.privateKey } +} + +function verifyStrict(dir: string, caPem: string, leafPem: string): void { + const openssl = Bun.which("openssl") + if (!openssl) return + const caPath = join(dir, "strict-ca.crt") + const leafPath = join(dir, "strict-leaf.crt") + writeFileSync(caPath, caPem) + writeFileSync(leafPath, leafPem) + const result = Bun.spawnSync([openssl, "verify", "-x509_strict", "-CAfile", caPath, leafPath]) + expect(result.exitCode, result.stderr.toString()).toBe(0) +} diff --git a/apps/api/src/proxy/ca.ts b/apps/api/src/proxy/ca.ts index 18c519c..ac4ca25 100644 --- a/apps/api/src/proxy/ca.ts +++ b/apps/api/src/proxy/ca.ts @@ -1,4 +1,13 @@ -import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs" +import { + closeSync, + existsSync, + mkdirSync, + openSync, + readFileSync, + renameSync, + unlinkSync, + writeFileSync, +} from "node:fs" import { join } from "node:path" import forge from "node-forge" @@ -25,19 +34,10 @@ export class MitmCa { mkdirSync(dir, { recursive: true }) this.caCertPath = join(dir, "ca.crt") const keyPath = join(dir, "ca.key") - - if (existsSync(this.caCertPath) && existsSync(keyPath)) { - this.caCertPem = readFileSync(this.caCertPath, "utf8") - this.caCert = forge.pki.certificateFromPem(this.caCertPem) - this.caKey = forge.pki.privateKeyFromPem(readFileSync(keyPath, "utf8")) - } else { - const { cert, key } = createCaCertificate() - this.caCert = cert - this.caKey = key - this.caCertPem = forge.pki.certificateToPem(cert) - writeFileSync(this.caCertPath, this.caCertPem) - writeFileSync(keyPath, forge.pki.privateKeyToPem(key), { mode: 0o600 }) - } + const initialized = withCaInitializationLock(dir, () => initializeCa(dir, this.caCertPath, keyPath)) + this.caCert = initialized.cert + this.caKey = initialized.key + this.caCertPem = initialized.pem // One leaf keypair shared across every minted host cert — only the certificate // (subject + SAN) differs per host, so there's no need to pay RSA keygen per host. @@ -76,12 +76,77 @@ export class MitmCa { { name: "keyUsage", digitalSignature: true, keyEncipherment: true }, { name: "extKeyUsage", serverAuth: true }, { name: "subjectAltName", altNames: altNamesFor(host) }, + { name: "subjectKeyIdentifier" }, + { + name: "authorityKeyIdentifier", + keyIdentifier: this.caCert.generateSubjectKeyIdentifier().getBytes(), + }, ]) cert.sign(this.caKey, forge.md.sha256.create()) return cert } } +function initializeCa( + dir: string, + certPath: string, + keyPath: string, +): { cert: forge.pki.Certificate; key: forge.pki.rsa.PrivateKey; pem: string } { + const certExists = existsSync(certPath) + const keyExists = existsSync(keyPath) + if (certExists !== keyExists) { + throw new Error( + `[proxy] Incomplete MITM CA in ${dir}: ca.crt and ca.key must either both exist or both be absent; refusing to generate a new identity`, + ) + } + + if (!certExists) { + const { cert, key } = createCaCertificate() + const pem = forge.pki.certificateToPem(cert) + writeFileSync(certPath, pem) + writeFileSync(keyPath, forge.pki.privateKeyToPem(key), { mode: 0o600 }) + return { cert, key, pem } + } + + const loaded = loadCa(certPath, keyPath) + if (loaded.cert.getExtension({ name: "subjectKeyIdentifier" })) return loaded + + const cert = addSubjectKeyIdentifier(loaded.cert, loaded.key) + const pem = forge.pki.certificateToPem(cert) + replaceFileAtomically(certPath, pem) + console.warn( + `[proxy] Updated MITM CA certificate at ${certPath} with a Subject Key Identifier. The CA key is unchanged, but clients pinning the certificate fingerprint must re-import ca.crt into their trust store.`, + ) + return { cert, key: loaded.key, pem } +} + +function withCaInitializationLock(dir: string, initialize: () => T): T { + const lockPath = join(dir, ".ca.lock") + const deadline = Date.now() + 10_000 + let descriptor: number | undefined + + while (descriptor === undefined) { + try { + descriptor = openSync(lockPath, "wx", 0o600) + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error + if (Date.now() >= deadline) { + throw new Error( + `[proxy] Timed out waiting for MITM CA initialization lock ${lockPath}; verify that no other instance is starting and remove a stale lock only when it is safe`, + ) + } + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 50) + } + } + + try { + return initialize() + } finally { + closeSync(descriptor) + unlinkSync(lockPath) + } +} + function createCaCertificate(): { cert: forge.pki.Certificate key: forge.pki.rsa.PrivateKey @@ -101,11 +166,69 @@ function createCaCertificate(): { cert.setExtensions([ { name: "basicConstraints", cA: true, critical: true }, { name: "keyUsage", keyCertSign: true, cRLSign: true, critical: true }, + { name: "subjectKeyIdentifier" }, ]) cert.sign(keys.privateKey, forge.md.sha256.create()) return { cert, key: keys.privateKey } } +function loadCa( + certPath: string, + keyPath: string, +): { cert: forge.pki.Certificate; key: forge.pki.rsa.PrivateKey; pem: string } { + try { + const pem = readFileSync(certPath, "utf8") + const cert = forge.pki.certificateFromPem(pem) + const key = forge.pki.privateKeyFromPem(readFileSync(keyPath, "utf8")) + const basicConstraints = cert.getExtension({ name: "basicConstraints" }) as { cA?: boolean } | undefined + if (!basicConstraints?.cA || !cert.isIssuer(cert) || !cert.verify(cert)) { + throw new Error("ca.crt is not a valid self-signed CA certificate") + } + if (cert.getExtension({ name: "subjectKeyIdentifier" }) && !cert.verifySubjectKeyIdentifier()) { + throw new Error("ca.crt contains a Subject Key Identifier that does not match its public key") + } + if (!rsaKeysMatch(cert.publicKey as forge.pki.rsa.PublicKey, key)) { + throw new Error("ca.crt public key does not match ca.key") + } + return { cert, key, pem } + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + throw new Error(`[proxy] Cannot safely load MITM CA from ${certPath}: ${message}`) + } +} + +function rsaKeysMatch(publicKey: forge.pki.rsa.PublicKey, privateKey: forge.pki.rsa.PrivateKey): boolean { + if (!publicKey?.n || !publicKey.e) return false + return ( + publicKey.n.toString(16) === privateKey.n.toString(16) && publicKey.e.toString(16) === privateKey.e.toString(16) + ) +} + +function addSubjectKeyIdentifier(source: forge.pki.Certificate, key: forge.pki.rsa.PrivateKey): forge.pki.Certificate { + const cert = forge.pki.createCertificate() + cert.version = source.version + cert.publicKey = source.publicKey + cert.serialNumber = source.serialNumber + cert.validity.notBefore = new Date(source.validity.notBefore) + cert.validity.notAfter = new Date(source.validity.notAfter) + cert.setSubject(source.subject.attributes) + cert.setIssuer(source.issuer.attributes) + cert.setExtensions([...source.extensions, { name: "subjectKeyIdentifier" }]) + cert.sign(key, forge.md.sha256.create()) + return cert +} + +function replaceFileAtomically(path: string, contents: string): void { + const temporaryPath = `${path}.tmp-${process.pid}-${forge.util.bytesToHex(forge.random.getBytesSync(8))}` + try { + writeFileSync(temporaryPath, contents, { mode: 0o644, flag: "wx" }) + renameSync(temporaryPath, path) + } catch (error) { + if (existsSync(temporaryPath)) unlinkSync(temporaryPath) + throw error + } +} + // SAN must carry an IP entry (type 7) for literal-IP hosts and a DNS entry (type 2) // otherwise, or strict clients reject the leaf. node-forge's TypeScript types narrow // `type` to string at the CertificateField boundary, but the runtime accepts the diff --git a/apps/docs/proxy/ca-installation.md b/apps/docs/proxy/ca-installation.md index 1a84753..a0f180c 100644 --- a/apps/docs/proxy/ca-installation.md +++ b/apps/docs/proxy/ca-installation.md @@ -14,6 +14,25 @@ creates: Both files live in `MITM_CA_DIR`. Per-host certificates are generated in memory and signed by this root. Persist the directory so clients only need to install the root once. +## Upgrading an existing CA + +On the first startup after upgrading to a version containing the fix for issue #113, TRAWL checks +the persisted root certificate. If it lacks a Subject Key Identifier, TRAWL updates `ca.crt` once +and logs the certificate path. The subject, serial number, validity period, and CA key stay the +same; `ca.key` is not changed. TRAWL refuses to migrate if the certificate and private key do not +match or an existing Subject Key Identifier is invalid, rather than silently creating a new CA +identity. Initialization is serialized with a short-lived `.ca.lock` file so multiple instances +sharing the same CA volume cannot race to create different identities. If a process is forcibly +terminated during initialization, remove a stale lock only after confirming no other instance is +starting. + +Adding the extension changes the certificate fingerprint. Clients that pin the exact certificate, +including strict TLS clients, must download the updated `ca.crt`, remove the previously installed +TRAWL root, and import the updated certificate using the relevant instructions below. Restart the +client afterward if it caches its trust store. Clients that identify trust anchors by their public +key and subject may continue to work without re-importing, but updating every trust store is the +safest deployment procedure. + ::: danger Anyone with `ca.key` can issue certificates trusted by clients that installed this CA. Keep the directory private, do not publish it, and never distribute `ca.key`. @@ -126,8 +145,10 @@ The exact startup-hook directory depends on the image. LinuxServer images suppor ## Rotation and recovery -Do not delete or replace `ca.crt` or `ca.key` during normal upgrades. If either is lost, TRAWL -generates a new root on the next startup and every client must install the new certificate. +Do not delete or replace `ca.crt` or `ca.key` during normal upgrades. TRAWL only generates a new +root when both files are absent. If just one file is missing, startup fails to prevent an accidental +identity change. If both are lost, TRAWL generates a new root on the next startup and every client +must install the new certificate. To intentionally rotate the CA: