fix(tls): Refine TLS ClientHello handling when allowSessionTicket is false by replacing extensive alert timeout logic with a concise warning alert and short delay, encouraging immediate client retry with proper SNI

changelog.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## 2025-03-16 - 4.1.6 - fix(tls)
+Refine TLS ClientHello handling when allowSessionTicket is false by replacing extensive alert timeout logic with a concise warning alert and short delay, encouraging immediate client retry with proper SNI
+
+- Update the TLS alert sending mechanism to use cork/uncork and a short, fixed delay instead of long timeouts
+- Remove redundant event listeners and excessive cleanup logic after sending the alert
+- Improve code clarity and encourage clients (e.g., Chrome) to retry handshake with SNI more responsively
+
 ## 2025-03-16 - 4.1.5 - fix(TLS/ConnectionHandler)
 Improve handling of TLS session resumption without SNI by sending an 'unrecognized_name' alert instead of immediately terminating the connection. This change adds a grace period for the client to retry the handshake with proper SNI and cleans up the connection if no valid response is received.
 

ts/00_commitinfo_data.ts

@@ -3,6 +3,6 @@
  */
 export const commitinfo = {
   name: '@push.rocks/smartproxy',
-  version: '4.1.5',
+  version: '4.1.6',
   description: 'A powerful proxy package that effectively handles high traffic, with features such as SSL/TLS support, port proxying, WebSocket handling, dynamic routing with authentication options, and automatic ACME certificate management.'
 }

ts/classes.pp.connectionhandler.ts

@@ -557,13 +557,13 @@ export class ConnectionHandler {
             this.tlsManager.isClientHello(chunk) &&
             !serverName
           ) {
-            // Always block ClientHello without SNI when allowSessionTicket is false
+            // Block ClientHello without SNI when allowSessionTicket is false
             console.log(
               `[${connectionId}] No SNI detected in ClientHello and allowSessionTicket=false. ` +
-                `Sending unrecognized_name alert to encourage client to retry with SNI.`
+              `Sending warning unrecognized_name alert to encourage immediate retry with SNI.`
             );
-
+          
-            // Set the termination reason first to avoid races
+            // Set the termination reason first
             if (record.incomingTerminationReason === null) {
               record.incomingTerminationReason = 'session_ticket_blocked_no_sni';
               this.connectionManager.incrementTerminationStat(
@@ -571,10 +571,9 @@ export class ConnectionHandler {
                 'session_ticket_blocked_no_sni'
               );
             }
-
+          
-            // Send a proper TLS alert before ending the connection
+            // Create a warning-level alert for unrecognized_name
-            // Using "unrecognized_name" (112) alert which is a warning level alert (1)
+            // This encourages Chrome to retry immediately with SNI
-            // that encourages clients to retry with proper SNI
             const alertData = Buffer.from([
               0x15, // Alert record type
               0x03,
@@ -584,97 +583,51 @@ export class ConnectionHandler {
               0x01, // Warning alert level (not fatal)
               0x70, // unrecognized_name alert (code 112)
             ]);
-
+          
             try {
-              // Make sure the alert is sent as a single packet
+              // Use cork/uncork to ensure the alert is sent as a single packet
               socket.cork();
-              socket.write(alertData);
+              const writeSuccessful = socket.write(alertData);
               socket.uncork();
               
-              // Set a longer timeout to allow the client to properly handle the alert
+              // Function to handle the clean socket termination
-              // The client might respond by closing the connection or initiating a new handshake
+              const finishConnection = () => {
-              const alertTimeout = setTimeout(() => {
+                // First call end() to initiate a graceful close (sends FIN)
-                if (!socket.destroyed) {
+                socket.end();
-                  console.log(`[${connectionId}] Client didn't respond to TLS alert, closing connection gracefully.`);
-                  
-                  // Gracefully end the connection
-                  socket.end();
-                  
-                  // Only destroy after a delay if it's still hanging
-                  setTimeout(() => {
-                    if (!socket.destroyed) {
-                      console.log(`[${connectionId}] Forcibly closing connection that didn't terminate properly.`);
-                      socket.destroy();
-                    }
-                    this.connectionManager.cleanupConnection(record, 'session_ticket_blocked_no_sni');
-                  }, 2000);
-                }
-              }, 5000); // Give the client 5 seconds to respond to our alert
-              
-              // Don't let this timeout keep the process alive
-              if (alertTimeout.unref) {
-                alertTimeout.unref();
-              }
-              
-              // Handle a proper close from the client
-              socket.once('close', () => {
-                clearTimeout(alertTimeout);
-                console.log(`[${connectionId}] Client closed connection after receiving TLS alert.`);
-                this.connectionManager.cleanupConnection(record, 'session_ticket_blocked_no_sni_client_closed');
-              });
-              
-              // Also handle if the client sends more data (possibly a new handshake with SNI)
-              socket.once('data', (newChunk) => {
-                clearTimeout(alertTimeout);
-                console.log(`[${connectionId}] Client sent new data after TLS alert, checking for SNI...`);
                 
-                // This would normally be handled by our renegotiation handler,
+                // Allow a short delay for the alert and FIN to be transmitted
-                // but since we're in a special case, we'll check for SNI again
+                // before we fully close the socket
-                if (this.tlsManager.isTlsHandshake(newChunk) && this.tlsManager.isClientHello(newChunk)) {
+                setTimeout(() => {
-                  const newServerName = this.tlsManager.extractSNI(newChunk, connInfo);
+                  if (!socket.destroyed) {
-                  
+                    socket.destroy();
-                  if (newServerName) {
-                    console.log(`[${connectionId}] Client provided SNI in new handshake: ${newServerName}`);
-                    
-                    // Update the record
-                    record.incomingTerminationReason = null;
-                    
-                    // Remove termination stats increment
-                    // Note: This is a little hacky as we don't have a proper way to decrement stats
-                    
-                    // Process the new handshake with SNI
-                    record.lockedDomain = newServerName;
-                    setupConnection(newServerName, newChunk);
-                    return;
-                  } else {
-                    console.log(`[${connectionId}] Client sent new handshake but still without SNI, closing connection.`);
-                    socket.end();
-                    setTimeout(() => {
-                      if (!socket.destroyed) {
-                        socket.destroy();
-                      }
-                      this.connectionManager.cleanupConnection(record, 'session_ticket_blocked_no_sni_retry_failed');
-                    }, 500);
                   }
-                } else {
+                  this.connectionManager.cleanupConnection(record, 'session_ticket_blocked_no_sni');
-                  console.log(`[${connectionId}] Client sent non-handshake data after TLS alert, closing connection.`);
+                }, 150); // Short delay, but longer than the standard TCP ACK timeout
-                  socket.end();
+              };
-                  setTimeout(() => {
-                    if (!socket.destroyed) {
-                      socket.destroy();
-                    }
-                    this.connectionManager.cleanupConnection(record, 'session_ticket_blocked_invalid_response');
-                  }, 500);
-                }
-              });
               
+              if (writeSuccessful) {
+                // If the data was successfully written to the kernel buffer,
+                // we can finish the connection after a short delay to ensure transmission
+                setTimeout(finishConnection, 50);
+              } else {
+                // If the kernel buffer was full, wait for the drain event
+                socket.once('drain', () => {
+                  setTimeout(finishConnection, 50);
+                });
+                
+                // Set a safety timeout in case drain never happens
+                setTimeout(() => {
+                  socket.removeAllListeners('drain');
+                  finishConnection();
+                }, 250);
+              }
             } catch (err) {
               // If we can't send the alert, fall back to immediate termination
               console.log(`[${connectionId}] Error sending TLS alert: ${err.message}`);
               socket.end();
               this.connectionManager.cleanupConnection(record, 'session_ticket_blocked_no_sni');
             }
-
+          
             return;
           }
         }