From 8d2c7e5ff432b3cc7a12b2952e9bdb8ec0fd31f4 Mon Sep 17 00:00:00 2001 From: Guillem Borrell <1861811+guillemborrell@users.noreply.github.com> Date: Mon, 5 Oct 2026 16:43:56 +0200 Subject: [PATCH] fix(net): keep guest TLS aborts from crashing Bun hosts When a guest closes an intercepted TLS flow while the host still has ciphertext pending (e.g. after rejecting the MITM certificate), the pending guest write fails with "guest closed" and the wrapped GuestTlsStream emits 'error'. Node forwards it to the TLS socket; Bun does not, so it became an uncaught exception that terminated the host process and every VM in it. Listen for the stream error and destroy only that flow's TLS socket. Adds a VM-free regression test that reproduces the crash on Bun. --- CHANGELOG.md | 2 + host/src/qemu/net.ts | 12 ++++ host/test/tls-mitm-sni.test.ts | 103 +++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b71533..e80da09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ All notable changes to Gondolin are documented here. ## Unreleased +- Fix a host-process crash on Bun when a guest closes an intercepted TLS flow while the host still has ciphertext to write, for example after rejecting the MITM certificate (custom CA bundles, certifi, pinning). The wrapped guest stream now has an error listener that tears down only that flow. + - Fix `chmod` from the guest being silently ignored on VFS mounts (`MemoryProvider`, `RealFSProvider`, `ShadowProvider`, ...): sandboxfs now forwards mode changes to the host and providers gain an optional `chmod()` method. `RealFSProvider` never applies setuid/setgid bits to host files. Requires a guest image built with this version. #115 - Fix a TOCTOU race in VFS path validation: fs-rpc requests are now processed strictly in order, so a guest pipelining requests can no longer swap a directory between `RealFSProvider`'s path check and the host syscall to reach files outside the mount root. #143 diff --git a/host/src/qemu/net.ts b/host/src/qemu/net.ts index b8fca0d..1c71df7 100644 --- a/host/src/qemu/net.ts +++ b/host/src/qemu/net.ts @@ -64,6 +64,7 @@ import { import type { WebSocketState } from "./ws.ts"; import { createGuestClosedError, + isGuestClosedError, type DnsMode, type DnsOptions, type HttpFetch, @@ -1323,6 +1324,17 @@ export class QemuNetworkBackend extends EventEmitter { }; const tlsSocket = new tls.TLSSocket(stream, serverOptions); + // A guest can close its flow while the host still has ciphertext to write (e.g. after + // rejecting the MITM certificate). The pending guest write then fails with "guest + // closed" and the wrapped stream emits `error`. Node forwards that to the TLS socket; + // Bun does not, so without a listener it is an uncaught error that crashes the host. + stream.on("error", (err) => { + if (!isGuestClosedError(err) && this.options.debug) { + this.emitDebug(`tls guest stream error ${(err as Error).message}`); + } + tlsSocket.destroy(); + }); + tlsSocket.on("data", (data) => { handleTlsHttpData(this, key, session, Buffer.from(data)); }); diff --git a/host/test/tls-mitm-sni.test.ts b/host/test/tls-mitm-sni.test.ts index 600b970..4dd2bad 100644 --- a/host/test/tls-mitm-sni.test.ts +++ b/host/test/tls-mitm-sni.test.ts @@ -7,6 +7,8 @@ import { Duplex } from "node:stream"; import test from "node:test"; import tls from "node:tls"; +import forge from "node-forge"; + import { QemuNetworkBackend } from "../src/qemu/net.ts"; import { MAX_CLIENT_HELLO_PREPARSE_BYTES, @@ -252,3 +254,104 @@ for (const tlsSniPreparse of [false, true]) { for (const result of results) assert.equal(result.body.toString(), "ok"); }); } + +/** + * A guest that rejects the MITM certificate (custom CA bundle, certifi, pinning) aborts the + * handshake and closes its flow while the host still has handshake ciphertext pending. + * The pending guest write then fails with "guest closed"; that must stay a per-flow event + * and never become an uncaught stream error in the host process. + */ +async function guestRejectsCertificate(tlsSniPreparse: boolean) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "gondolin-tls-abort-")); + const backend = new QemuNetworkBackend({ + socketPath: path.join( + os.tmpdir(), + `gondolin-tls-abort-${crypto.randomUUID()}.sock`, + ), + mitmCertDir: dir, + tlsSniPreparse, + }); + const internals = backend as any; + await internals.ensureCaAsync(); + const key = "TCP:192.168.127.3:41000:203.0.113.10:443"; + const session: any = { + socket: null, + srcIP: "192.168.127.3", + srcPort: 41000, + dstIP: "203.0.113.10", + dstPort: 443, + connectIP: "203.0.113.10", + syntheticHostname: null, + flowControlPaused: false, + protocol: "tls", + connected: false, + pendingWrites: [], + pendingWriteBytes: 0, + }; + const wire = new CaptureDuplex(); + internals.stack = { + handleTcpData: ({ data }: { data: Buffer }) => { + wire.push(Buffer.from(data)); + // A slow guest: every host->guest chunk waits for a flow resume. + session.flowControlPaused = true; + }, + handleTcpEnd: () => {}, + handleTcpClosed: () => {}, + handleTcpError: () => {}, + }; + internals.flush = () => {}; + internals.tcpSessions.set(key, session); + wire.onWrite = (chunk) => internals.handleTlsData(key, session, chunk); + + const uncaught: unknown[] = []; + const onUncaught = (error: unknown) => uncaught.push(error); + process.on("uncaughtException", onUncaught); + try { + // Trust an unrelated CA: the client rejects the MITM leaf and aborts. + const unrelated = forge.pki.rsa.generateKeyPair(1024); + const caCert = forge.pki.createCertificate(); + caCert.publicKey = unrelated.publicKey; + caCert.serialNumber = "01"; + caCert.validity.notBefore = new Date(Date.now() - 60_000); + caCert.validity.notAfter = new Date(Date.now() + 3_600_000); + caCert.setSubject([{ name: "commonName", value: "unrelated" }]); + caCert.setIssuer([{ name: "commonName", value: "unrelated" }]); + caCert.sign(unrelated.privateKey, forge.md.sha256.create()); + const client = tls.connect({ + socket: wire, + servername: "example.com", + ca: forge.pki.certificateToPem(caCert), + }); + const failure = await new Promise((resolve) => { + client.on("error", resolve); + client.on("secureConnect", () => + resolve(new Error("unexpected handshake success")), + ); + }); + assert.notEqual(failure.message, "unexpected handshake success"); + // The guest closes its flow (FIN), as the network stack reports for a real VM: the host + // then ends its TLS socket, whose final write targets the closed flow. + internals.handleTcpClose({ key, destroy: false }); + await new Promise((resolve) => setTimeout(resolve, 100)); + assert.deepEqual( + uncaught, + [], + "guest TLS abort must not raise in the host process", + ); + } finally { + process.off("uncaughtException", onUncaught); + await backend.close().catch(() => {}); + fs.rmSync(dir, { recursive: true, force: true }); + } +} + +for (const tlsSniPreparse of [false, true]) { + const mode = tlsSniPreparse ? "SNI pre-parse" : "SNICallback"; + const skip = + isBun && !tlsSniPreparse ? "Bun does not call SNICallback" : false; + test(`tls-mitm (${mode}): guest certificate rejection stays a per-flow event`, { + skip, + }, async () => { + await guestRejectsCertificate(tlsSniPreparse); + }); +}