fix(proxy): add strict MITM certificate identifiers

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).

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)
+}

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<T>(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