tls: report a handshake that the peer left, and reset the chain check for a renegotiation

packages/bun-usockets/src/crypto/openssl.c

@@ -1962,6 +1962,8 @@ static int ssl_renegotiate(struct us_socket_t *s) {
   us_reneg_policy(s_ssl(s), &limit, &window);
   struct us_ssl_rare_t *st = us_ssl_rare_ensure(s_ssl(s));
   s->ssl_handshake_state = HANDSHAKE_RENEGOTIATION_PENDING;
+  /* The check of the handshake before is not the check of this one. */
+  s->ssl_peer_chain_checked = 0;
   /* Wall-clock time can step backwards (NTP, manual adjustment); the
    * unsigned subtraction below would underflow and reset the window every
    * time. Only treat the window as elapsed when time has moved forward. */

src/js/node/net.ts

@@ -1324,6 +1324,15 @@ function onSocketTLSError(err) {
   }
 }
 
+// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L1246-L1257
+function onSocketTLSClose(hadError) {
+  if (hadError) return;
+  if (!this._controlReleased && !this[kerrorEmitted]) {
+    this[kerrorEmitted] = true;
+    this.server?.emit("tlsClientError", new ConnResetException("socket hang up"), this);
+  }
+}
+
 function onHandshakeTimeout() {
   this._emitTLSError($ERR_TLS_HANDSHAKE_TIMEOUT());
 }
@@ -1346,6 +1355,7 @@ function initAcceptedTLSSocket(server, socket) {
   socket[kerrorEmitted] = false;
   socket.on("_tlsError", onSocketTLSError);
   socket.on("error", onSocketTLSError);
+  socket.on("close", onSocketTLSClose);
   const handshakeTimeout = server._handshakeTimeout;
   if (!(handshakeTimeout > 0)) return;
   // The deadline surfaces as the socket's own 'timeout' event with

test/js/bun/net/socket.test.ts

@@ -19,7 +19,7 @@ import { once } from "node:events";
 import net from "node:net";
 import { join } from "node:path";
 import { Duplex } from "node:stream";
-import { createSecureContext, connect as tlsConnect } from "node:tls";
+import { createSecureContext, connect as tlsConnect, createServer as tlsCreateServer } from "node:tls";
 describe.concurrent("socket", () => {
   it("should throw when a socket from a file descriptor has a bad file descriptor", async () => {
     const open = jest.fn();
@@ -3950,7 +3950,7 @@ Reo=
 
     it("a rejecting client closes when its handshake fails with no error", async () => {
       const events: string[] = [];
-      const closed = Promise.withResolvers<void>();
+      const reported = Promise.withResolvers<void>();
       // Never answers, and keeps the connection open after the client's FIN.
       const peer = net.createServer({ allowHalfOpen: true }, socket => socket.on("error", () => {}));
       await once(peer.listen(0, "127.0.0.1"), "listening");
@@ -3966,19 +3966,21 @@ Reo=
             },
             handshake(_socket, success, authorizationError) {
               events.push(`handshake success=${success} error=${authorizationError?.message ?? null}`);
+              reported.resolve();
             },
             data() {},
             close() {
               events.push("close");
-              closed.resolve();
             },
             error() {},
             connectError(_socket, err) {
-              closed.reject(err);
+              reported.reject(err);
             },
           },
         });
-        await closed.promise;
+        await reported.promise;
+        // The close comes with the report, not when the peer closes.
+        await pendingReadsDone();
         expect(events).toEqual(["handshake success=false error=null", "close"]);
       } finally {
         peer.close();
@@ -4033,6 +4035,8 @@ Reo=
         callbackCode: string | null;
         getterCode: string | null;
       }>();
+      // Before the handshake reports, the SSL answers: it has no certificate to show.
+      const beforeTheReport: (string | null)[] = [];
       // Never answers, and keeps the connection open after the client's FIN.
       const peer = net.createServer({ allowHalfOpen: true }, socket => socket.on("error", () => {}));
       await once(peer.listen(0, "127.0.0.1"), "listening");
@@ -4044,7 +4048,10 @@ Reo=
           socket: {
             // Before the ClientHello, so the handshake never starts.
             open(socket) {
+              const code = () => (socket.getAuthorizationError() as NodeJS.ErrnoException | null)?.code ?? null;
+              beforeTheReport.push(code());
               socket.shutdown();
+              beforeTheReport.push(code());
             },
             handshake(socket, success, authorizationError) {
               handshake.resolve({
@@ -4062,12 +4069,85 @@ Reo=
           },
         });
         expect(await handshake.promise).toEqual({ successArg: false, callbackCode: null, getterCode: null });
+        expect(beforeTheReport).toEqual(["UNABLE_TO_GET_ISSUER_CERT", "UNABLE_TO_GET_ISSUER_CERT"]);
         // This client does not reject, and its peer did not close: the socket is still open.
         expect(client.getAuthorizationError()).toBeNull();
       } finally {
         peer.close();
       }
     });
+
+    // A TLS 1.2 client checks the chain of the server when it reads the
+    // server's first flight. Its handshake is not complete then.
+    it("a handshake that fails after shutdown() reports the check of the chain that it read", async () => {
+      const server = tlsCreateServer({ key: ROGUE_KEY, cert: ROGUE_CRT, maxVersion: "TLSv1.2" }, socket =>
+        socket.on("error", () => {}),
+      );
+      server.on("tlsClientError", () => {});
+      await once(server.listen(0, "127.0.0.1"), "listening");
+      // Holds the server's flight until the client's FIN arrived, and keeps the connection open.
+      const flightForwarded = Promise.withResolvers<void>();
+      const relayed: net.Socket[] = [];
+      const relay = net.createServer({ allowHalfOpen: true }, downstream => {
+        const upstream = net.connect({
+          port: (server.address() as net.AddressInfo).port,
+          host: "127.0.0.1",
+          allowHalfOpen: true,
+        });
+        relayed.push(downstream, upstream);
+        const held: Buffer[] = [];
+        let sawClientFin = false;
+        const forward = (data: Buffer) => downstream.write(data, () => flightForwarded.resolve());
+        downstream.on("data", data => upstream.write(data));
+        downstream.on("end", () => {
+          sawClientFin = true;
+          for (const data of held.splice(0)) forward(data);
+        });
+        upstream.on("data", data => (sawClientFin ? forward(data) : held.push(data)));
+        downstream.on("error", () => {});
+        upstream.on("error", () => {});
+      });
+      await once(relay.listen(0, "127.0.0.1"), "listening");
+      const events: string[] = [];
+      const closed = Promise.withResolvers<void>();
+      try {
+        const client = await Bun.connect({
+          hostname: "127.0.0.1",
+          port: (relay.address() as net.AddressInfo).port,
+          tls: { ca: CA_CRT, rejectUnauthorized: false },
+          socket: {
+            // The ClientHello leaves when `open` returns.
+            open(socket) {
+              setImmediate(() => socket.shutdown());
+            },
+            handshake(_socket, success, authorizationError) {
+              const code = (authorizationError as NodeJS.ErrnoException | null)?.code ?? null;
+              events.push(`handshake success=${success} code=${code}`);
+            },
+            data() {},
+            close() {
+              events.push("close");
+              closed.resolve();
+            },
+            error() {},
+            connectError(_socket, err) {
+              closed.reject(err);
+            },
+          },
+        });
+        await flightForwarded.promise;
+        await pendingReadsDone();
+        // The server waits for the client's second flight, which cannot leave.
+        expect(events).toEqual([]);
+        client.terminate();
+        await closed.promise;
+        expect(events).toEqual(["handshake success=false code=UNABLE_TO_VERIFY_LEAF_SIGNATURE", "close"]);
+      } finally {
+        for (const socket of relayed) socket.destroy();
+        relay.close();
+        server.close();
+      }
+    });
   });
 });
 

test/js/node/tls/node-tls-duplex-end-verify.test.ts

@@ -183,10 +183,10 @@ async function behindProxy(server, wire, proxyOptions = {}, overDuplex = false)
   };
 }
 
-// The same shape on a plain TCP socket. The client calls end() when its last handshake flight left. The proxy holds
-// the server's final flight until the client's FIN arrived, so the handshake completes on a socket that is already
-// shut down. The proxy never forwards the FIN, so the server keeps writing. The server's certificate is not trusted,
-// unless `trusted`. Returns the ordered events of the client.
+// The same shape on a plain TCP socket. The client calls end() when its last handshake flight left. The proxy
+// delivers what the server sends from then on after the client's FIN arrived, in `pieces` parts, so the handshake
+// completes on a socket that is already shut down. The proxy never forwards the FIN, so the server keeps writing.
+// The server's certificate is not trusted, unless `trusted`. Returns the ordered events of the client.
 async function endMidHandshakeOverTcp(maxVersion, rejectUnauthorized, { pieces = 1, trusted = false } = {}) {
   const events = [];
   const { promise, resolve } = Promise.withResolvers();
@@ -202,8 +202,8 @@ async function endMidHandshakeOverTcp(maxVersion, rejectUnauthorized, { pieces =
     (downstream, upstream) => {
       let sawChangeCipherSpec = false;
       let ending = false;
-      let clientEnded = false;
-      const held = [];
+      const clientEnded = Promise.withResolvers();
+      let delivered = clientEnded.promise;
       eachRecord(downstream, record => {
         upstream.write(record);
         if (ending) return;
@@ -215,14 +215,11 @@ async function endMidHandshakeOverTcp(maxVersion, rejectUnauthorized, { pieces =
         }
         sawChangeCipherSpec ||= record[0] === CHANGE_CIPHER_SPEC;
       });
-      downstream.on("end", async () => {
-        await inPieces(part => downstream.write(part), Buffer.concat(held.splice(0)), pieces);
-        clientEnded = true;
-        for (const chunk of held.splice(0)) downstream.write(chunk);
-      });
+      downstream.on("end", clientEnded.resolve);
       upstream.on("data", chunk => {
-        if (ending && !clientEnded) held.push(chunk);
-        else downstream.write(chunk);
+        if (!ending) return void downstream.write(chunk);
+        // The FIN can arrive before the server's answer does: each chunk waits for it, and for the chunk before.
+        delivered = delivered.then(() => inPieces(part => downstream.write(part), chunk, pieces));
       });
     },
     { allowHalfOpen: true },
@@ -582,3 +579,65 @@ test("a bad record behind the client's Finished does not make a server accept an
   // Node reports the bad record. Its code depends on the cipher, so only the class of the error is fixed.
   assert.match(events[0], isBun ? /^tlsClientError DEPTH_ZERO_SELF_SIGNED_CERT$/ : /^tlsClientError ERR_SSL_/);
 });
+
+// The peer can go away while the handshake of a socket that called end() still runs. The socket closes with no error,
+// and the server reports the handshake that never finished. `half`: the peer sends the first half of its last flight
+// before its FIN.
+for (const maxVersion of ["TLSv1.2", "TLSv1.3"]) {
+  for (const half of [false, true]) {
+    test(`${maxVersion}: a server that end()s reports the handshake that its peer left ${half ? "in the middle of a flight" : "before its last flight"}`, async () => {
+      const events = [];
+      const refused = Promise.withResolvers();
+      const serverClosed = Promise.withResolvers();
+      let serverSocket;
+      const server = tls.createServer({ key, cert, maxVersion }, () => events.push("secureConnection"));
+      server.on("tlsClientError", err => {
+        events.push(`tlsClientError ${err.code}`);
+        refused.resolve();
+      });
+      server.on("connection", socket => {
+        serverSocket = socket;
+        socket.on("error", () => {});
+        socket.on("close", serverClosed.resolve);
+      });
+      const { port, close } = await behindProxy(
+        server,
+        (downstream, upstream) => {
+          let chunks = 0;
+          const held = [];
+          downstream.on("data", chunk => {
+            // The ClientHello goes through. The proxy holds the client's last flight, and the server calls end().
+            if (++chunks === 1) return void upstream.write(chunk);
+            held.push(chunk);
+            if (held.length === 1) serverSocket.end();
+          });
+          upstream.on("data", chunk => downstream.write(chunk));
+          upstream.on("end", async () => {
+            const flight = Buffer.concat(held.splice(0));
+            if (half) {
+              upstream.write(flight.subarray(0, Math.ceil(flight.length / 2)));
+              await pendingReadsDone();
+            }
+            upstream.end();
+          });
+        },
+        { allowHalfOpen: true },
+      );
+      const client = tls.connect({
+        port,
+        host: "127.0.0.1",
+        servername: "agent1",
+        rejectUnauthorized: false,
+        maxVersion,
+      });
+      client.on("error", () => {});
+      try {
+        await Promise.all([refused.promise, serverClosed.promise]);
+        assert.deepStrictEqual(events, ["tlsClientError ECONNRESET"]);
+      } finally {
+        client.destroy();
+        close();
+      }
+    });
+  }
+}

test/js/node/tls/node-tls-server.test.ts

@@ -1254,7 +1254,7 @@ it("a socket that end()s while an asynchronous SNICallback is pending reports tl
   const events: string[] = [];
   const sniCalled = Promise.withResolvers<void>();
   const sawServerFin = Promise.withResolvers<void>();
-  const closed = Promise.withResolvers<void>();
+  const refused = Promise.withResolvers<void>();
   let answerSni: (() => void) | undefined;
   let serverSocket: net.Socket | undefined;
   const server: Server = createServer({
@@ -1267,14 +1267,14 @@ it("a socket that end()s while an asynchronous SNICallback is pending reports tl
     },
   });
   server.on("secureConnection", () => events.push("secureConnection"));
-  server.on("tlsClientError", err => events.push(`tlsClientError ${(err as NodeJS.ErrnoException).code}`));
+  server.on("tlsClientError", err => {
+    events.push(`tlsClientError ${(err as NodeJS.ErrnoException).code}`);
+    refused.resolve();
+  });
   server.on("connection", socket => {
     serverSocket = socket;
     socket.on("error", () => {});
-    socket.on("close", hadError => {
-      events.push(`close hadError=${hadError}`);
-      closed.resolve();
-    });
+    socket.on("close", hadError => events.push(`close hadError=${hadError}`));
   });
   server.listen(0);
   await once(server, "listening");
@@ -1308,7 +1308,9 @@ it("a socket that end()s while an asynchronous SNICallback is pending reports tl
     await sawServerFin.promise;
     // The handshake fails inside this call: the socket is shut down.
     answerSni!();
-    await closed.promise;
+    await refused.promise;
+    // 'close' follows in the check phase of the loop.
+    for (let turn = 0; turn < 10 && events.length < 2; turn++) await new Promise(resolve => setImmediate(resolve));
     expect(events).toEqual(["tlsClientError ECONNRESET", "close hadError=true"]);
   } finally {
     client.destroy();