diff --git a/ext/node/ops/tls_wrap.rs b/ext/node/ops/tls_wrap.rs index 0a4b76eee3e7d7..7358c1612419a2 100644 --- a/ext/node/ops/tls_wrap.rs +++ b/ext/node/ops/tls_wrap.rs @@ -781,6 +781,11 @@ struct TLSWrapInner { /// is freed, so in-flight write callbacks can avoid a dangling deref. alive: Rc>, + /// Set to true by JS when the TLSSocket is being destroyed/closed. + /// Checked after handshake callback to prevent sending application + /// data when checkServerIdentity fails. + closing: bool, + // Error string (like Node's error_) error: Option, @@ -899,6 +904,7 @@ impl TLSWrapInner { bytes_read: 0, bytes_written: 0, alive: Rc::new(Cell::new(true)), + closing: false, error: None, verify_error: Arc::new(std::sync::Mutex::new(None)), pending_client_config: None, @@ -934,12 +940,36 @@ impl TLSWrapInner { return; } TLSWrapInner::do_enc_out_action(ptr, enc_action); + + // After handshake completes, the JS callback (onhandshakedone -> + // onConnectSecure) has run. If the connection was accepted (e.g. + // checkServerIdentity passed), drain any pending cleartext that + // was buffered before the handshake. If JS destroyed the socket + // (identity check failed), the closing flag prevents sending. + if result.handshake_done && !(*ptr).closing { + (*ptr).cycling = true; + (*ptr).clear_in(); + let enc_action2 = (*ptr).enc_out_collect(); + (*ptr).cycling = false; + TLSWrapInner::do_enc_out_action(ptr, enc_action2); + } } } /// Feed pending cleartext into rustls writer. /// Mirrors Node's TLSWrap::ClearIn(). fn clear_in(&mut self) { + // Don't feed application data to rustls until the handshake is + // complete and JS has confirmed the connection (via onhandshakedone). + // In Node.js, the SSL_write for app data effectively happens after + // the handshake callback because OpenSSL fires the info callback + // synchronously during SSL_read. Without this gate, app data would + // be encrypted and sent before checkServerIdentity can reject the + // connection. + if !self.established || self.closing { + return; + } + let Some(ref mut conn) = self.tls_conn else { return; }; @@ -2155,6 +2185,15 @@ impl TLSWrap { 0 } + /// Mark the TLSWrap as closing. Called synchronously from JS + /// TLSWrap.close() so the Rust side knows not to send buffered + /// application data after a handshake callback rejects the connection. + #[fast] + fn set_closing(&self) { + let inner = unsafe { &mut *self.inner.as_mut_ptr() }; + inner.closing = true; + } + /// Destroy the SSL connection. Tears down the TLS state without /// re-entering JS (no write-completion callbacks). #[nofast] diff --git a/ext/node/polyfills/_tls_wrap.js b/ext/node/polyfills/_tls_wrap.js index 05075a311c2b2a..33859385bc7710 100644 --- a/ext/node/polyfills/_tls_wrap.js +++ b/ext/node/polyfills/_tls_wrap.js @@ -278,6 +278,10 @@ Object.setPrototypeOf(TLSSocket.prototype, net.Socket.prototype); Object.setPrototypeOf(TLSSocket, net.Socket); tlsWrap.TLSWrap.prototype.close = function close(cb) { + // Signal to Rust that this TLSWrap is being closed, so it won't + // send buffered application data after a failed identity check. + this.setClosing(); + let ssl; if (this._owner) { ssl = this._owner.ssl; @@ -301,7 +305,7 @@ tlsWrap.TLSWrap.prototype.close = function close(cb) { if (this._parentWrap) { if (this._parentWrap._handle === null) { - queueMicrotask(done); + nextTick(done); return; } @@ -313,7 +317,7 @@ tlsWrap.TLSWrap.prototype.close = function close(cb) { } // Defer so callers can register "close" listeners after destroy(). - queueMicrotask(done); + nextTick(done); }; TLSSocket.prototype._wrapHandle = function (wrap, handle) { diff --git a/ext/node/polyfills/net.ts b/ext/node/polyfills/net.ts index 77f979ed7030ac..35e6a7a8bf9ca0 100644 --- a/ext/node/polyfills/net.ts +++ b/ext/node/polyfills/net.ts @@ -1839,15 +1839,19 @@ Socket.prototype._destroy = function (exception, cb) { } } - this._handle.close(() => { - this._handle.onread = _noop; - this._handle = null; - this._sockname = undefined; + const handle = this._handle; + // Call cb first so the stream's error emission (via nextTick) is + // scheduled before the handle close callback. This ensures 'error' + // fires before 'close', matching Node.js behavior. + this._handle = null; + this._sockname = undefined; + cb(exception); + handle.close(() => { + handle.onread = _noop; debug("emit close"); this.emit("close", isException); }); - cb(exception); } else { cb(exception); nextTick(_emitCloseNT, this); diff --git a/tests/node_compat/config.jsonc b/tests/node_compat/config.jsonc index 7660342d6c7fca..1079546d9a4b8e 100644 --- a/tests/node_compat/config.jsonc +++ b/tests/node_compat/config.jsonc @@ -1732,6 +1732,7 @@ "parallel/test-https-agent-unref-socket.js": {}, "parallel/test-https-agent.js": {}, "parallel/test-https-byteswritten.js": {}, + "parallel/test-https-client-checkServerIdentity.js": {}, "parallel/test-https-close.js": {}, "parallel/test-https-connect-address-family.js": {}, "parallel/test-https-connecting-to-http.js": { @@ -1756,7 +1757,12 @@ "parallel/test-https-server-options-server-response.js": {}, "parallel/test-https-server-request-timeout.js": {}, "parallel/test-https-set-timeout-server.js": {}, + "parallel/test-https-simple.js": {}, "parallel/test-https-socket-options.js": {}, + "parallel/test-https-strict.js": { + "ignore": true, + "reason": "NODE_TLS_REJECT_UNAUTHORIZED=0 does not fully suppress cert verification errors" + }, "parallel/test-https-timeout-server-2.js": {}, "parallel/test-https-timeout.js": {}, "parallel/test-https-truncate.js": {},