From 67ddf975471adc217542b7afe446060a5e7b2f3b Mon Sep 17 00:00:00 2001
From: Philipp Kunz <code@philkunz.com>
Date: Sun, 16 Mar 2025 13:47:34 +0000
Subject: [PATCH] fix(classes.pp.connectionhandler): Improve TLS alert handling
 in ClientHello when SNI is missing and session tickets are disallowed

---
 changelog.md                       |  7 ++++++
 ts/00_commitinfo_data.ts           |  2 +-
 ts/classes.pp.connectionhandler.ts | 35 ++++++++++++++++++++----------
 3 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/changelog.md b/changelog.md
index 232e43b..6ef0c9f 100644
--- a/changelog.md
+++ b/changelog.md
@@ -1,5 +1,12 @@
 # Changelog
 
+## 2025-03-16 - 4.1.7 - fix(classes.pp.connectionhandler)
+Improve TLS alert handling in ClientHello when SNI is missing and session tickets are disallowed
+
+- Replace the unrecognized_name alert with a handshake_failure alert to ensure better client behavior.
+- Refactor the alert sending mechanism using cork/uncork and add a safety timeout for the drain event.
+- Enhance logging for debugging TLS handshake failures when SNI is absent.
+
 ## 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
 
diff --git a/ts/00_commitinfo_data.ts b/ts/00_commitinfo_data.ts
index d68dc96..e5d626a 100644
--- a/ts/00_commitinfo_data.ts
+++ b/ts/00_commitinfo_data.ts
@@ -3,6 +3,6 @@
  */
 export const commitinfo = {
   name: '@push.rocks/smartproxy',
-  version: '4.1.6',
+  version: '4.1.7',
   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.'
 }
diff --git a/ts/classes.pp.connectionhandler.ts b/ts/classes.pp.connectionhandler.ts
index 6d1a86f..fc7ed80 100644
--- a/ts/classes.pp.connectionhandler.ts
+++ b/ts/classes.pp.connectionhandler.ts
@@ -560,9 +560,9 @@ export class ConnectionHandler {
             // Block ClientHello without SNI when allowSessionTicket is false
             console.log(
               `[${connectionId}] No SNI detected in ClientHello and allowSessionTicket=false. ` +
-              `Sending warning unrecognized_name alert to encourage immediate retry with SNI.`
+                `Sending warning unrecognized_name alert to encourage immediate retry with SNI.`
             );
-          
+
             // Set the termination reason first
             if (record.incomingTerminationReason === null) {
               record.incomingTerminationReason = 'session_ticket_blocked_no_sni';
@@ -571,10 +571,10 @@ export class ConnectionHandler {
                 'session_ticket_blocked_no_sni'
               );
             }
-          
+
             // Create a warning-level alert for unrecognized_name
             // This encourages Chrome to retry immediately with SNI
-            const alertData = Buffer.from([
+            const serverNameUnknownAlertData = Buffer.from([
               0x15, // Alert record type
               0x03,
               0x03, // TLS 1.2 version
@@ -583,18 +583,29 @@ export class ConnectionHandler {
               0x01, // Warning alert level (not fatal)
               0x70, // unrecognized_name alert (code 112)
             ]);
-          
+
+            // Send a handshake_failure alert instead of unrecognized_name
+            const sslHandshakeFailureAlertData = Buffer.from([
+              0x15, // Alert record type
+              0x03,
+              0x03, // TLS 1.2 version
+              0x00,
+              0x02, // Length
+              0x01, // Warning alert level (not fatal)
+              0x28, // handshake_failure alert (40) instead of unrecognized_name (112)
+            ]);
+
             try {
               // Use cork/uncork to ensure the alert is sent as a single packet
               socket.cork();
-              const writeSuccessful = socket.write(alertData);
+              const writeSuccessful = socket.write(sslHandshakeFailureAlertData);
               socket.uncork();
-              
+
               // Function to handle the clean socket termination
               const finishConnection = () => {
                 // First call end() to initiate a graceful close (sends FIN)
                 socket.end();
-                
+
                 // Allow a short delay for the alert and FIN to be transmitted
                 // before we fully close the socket
                 setTimeout(() => {
@@ -604,7 +615,7 @@ export class ConnectionHandler {
                   this.connectionManager.cleanupConnection(record, 'session_ticket_blocked_no_sni');
                 }, 150); // Short delay, but longer than the standard TCP ACK timeout
               };
-              
+
               if (writeSuccessful) {
                 // If the data was successfully written to the kernel buffer,
                 // we can finish the connection after a short delay to ensure transmission
@@ -614,7 +625,7 @@ export class ConnectionHandler {
                 socket.once('drain', () => {
                   setTimeout(finishConnection, 50);
                 });
-                
+
                 // Set a safety timeout in case drain never happens
                 setTimeout(() => {
                   socket.removeAllListeners('drain');
@@ -627,7 +638,7 @@ export class ConnectionHandler {
               socket.end();
               this.connectionManager.cleanupConnection(record, 'session_ticket_blocked_no_sni');
             }
-          
+
             return;
           }
         }
@@ -1075,4 +1086,4 @@ export class ConnectionHandler {
       }
     });
   }
-}
\ No newline at end of file
+}