diff --git a/changelog.md b/changelog.md index 0fdc8c5..1dbda28 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,12 @@ # Changelog +## 2025-03-15 - 4.1.3 - fix(connectionhandler) +Improve handling of TLS ClientHello messages when allowSessionTicket is disabled and no SNI is provided by sending a warning alert (unrecognized_name, code 0x70) with a proper callback and delay to ensure the alert is transmitted before closing the connection. + +- Replace the fatal alert (0x02/0x40) with a warning alert (0x01/0x70) to notify clients to send SNI. +- Use socket.write callback to wait 100ms after sending the alert before terminating the connection. +- Remove the previous short (50ms) delay in favor of a more reliable delay mechanism before cleanup. + ## 2025-03-15 - 4.1.2 - fix(connectionhandler) Send proper TLS alert before terminating connections when SNI is missing and session tickets are disallowed. diff --git a/ts/00_commitinfo_data.ts b/ts/00_commitinfo_data.ts index 054c9eb..dea6342 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.2', + version: '4.1.3', 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 61d4c0f..d5fa620 100644 --- a/ts/classes.pp.connectionhandler.ts +++ b/ts/classes.pp.connectionhandler.ts @@ -175,26 +175,42 @@ export class ConnectionHandler { // If allowSessionTicket is false and we can't determine SNI, terminate the connection if (!serverName) { // Always block when allowSessionTicket is false and there's no SNI - // Don't even check for session resumption - be strict console.log( `[${connectionId}] No SNI detected in ClientHello and allowSessionTicket=false. ` + `Terminating connection to force new TLS handshake with SNI.` ); // Send a proper TLS alert before ending the connection - // This helps browsers like Chrome properly recognize the error + // Using "unrecognized_name" (112) alert which is a warning level alert (1) + // that encourages clients to retry with proper SNI const alertData = Buffer.from([ 0x15, // Alert record type 0x03, 0x03, // TLS 1.2 version 0x00, 0x02, // Length - 0x02, // Fatal alert level - 0x40 // Handshake failure alert + 0x01, // Warning alert level (not fatal) + 0x70 // unrecognized_name alert (code 112) ]); try { - socket.write(alertData); + socket.write(alertData, () => { + // Only close the socket after we're sure the alert was sent + // Give the alert time to be processed by the client + setTimeout(() => { + socket.end(); + + // Ensure complete cleanup happens a bit later + setTimeout(() => { + if (!socket.destroyed) { + socket.destroy(); + } + this.connectionManager.cleanupConnection(record, 'session_ticket_blocked_no_sni'); + }, 100); + }, 100); + }); } catch (err) { - // Ignore write errors, we're closing anyway + // If we can't send the alert, fall back to immediate termination + socket.end(); + this.connectionManager.cleanupConnection(record, 'session_ticket_blocked_no_sni'); } if (record.incomingTerminationReason === null) { @@ -202,12 +218,6 @@ export class ConnectionHandler { this.connectionManager.incrementTerminationStat('incoming', 'session_ticket_blocked_no_sni'); } - // Add a small delay before ending to allow alert to be sent - setTimeout(() => { - socket.end(); - this.connectionManager.cleanupConnection(record, 'session_ticket_blocked_no_sni'); - }, 50); - return; } @@ -573,25 +583,42 @@ export class ConnectionHandler { !serverName) { // Always block ClientHello without SNI when allowSessionTicket is false - // Don't even check for session resumption - be strict console.log( `[${connectionId}] No SNI detected in ClientHello and allowSessionTicket=false. ` + `Terminating connection to force new TLS handshake with SNI.` ); // Send a proper TLS alert before ending the connection + // Using "unrecognized_name" (112) alert which is a warning level alert (1) + // that encourages clients to retry with proper SNI const alertData = Buffer.from([ 0x15, // Alert record type 0x03, 0x03, // TLS 1.2 version 0x00, 0x02, // Length - 0x02, // Fatal alert level - 0x40 // Handshake failure alert + 0x01, // Warning alert level (not fatal) + 0x70 // unrecognized_name alert (code 112) ]); try { - socket.write(alertData); + socket.write(alertData, () => { + // Only close the socket after we're sure the alert was sent + // Give the alert time to be processed by the client + setTimeout(() => { + socket.end(); + + // Ensure complete cleanup happens a bit later + setTimeout(() => { + if (!socket.destroyed) { + socket.destroy(); + } + this.connectionManager.cleanupConnection(record, 'session_ticket_blocked_no_sni'); + }, 100); + }, 100); + }); } catch (err) { - // Ignore write errors, we're closing anyway + // If we can't send the alert, fall back to immediate termination + socket.end(); + this.connectionManager.cleanupConnection(record, 'session_ticket_blocked_no_sni'); } if (record.incomingTerminationReason === null) { @@ -599,12 +626,6 @@ export class ConnectionHandler { this.connectionManager.incrementTerminationStat('incoming', 'session_ticket_blocked_no_sni'); } - // Add a small delay before ending to allow alert to be sent - setTimeout(() => { - socket.end(); - this.connectionManager.cleanupConnection(record, 'session_ticket_blocked_no_sni'); - }, 50); - return; } }