From 1a90566622a3f9db501159cb4d7d73d2b09803b8 Mon Sep 17 00:00:00 2001 From: Philipp Kunz Date: Wed, 12 Mar 2025 09:56:21 +0000 Subject: [PATCH] fix(TLS/SNI): Improve TLS session resumption handling and logging. Now, session resumption attempts are always logged with details, and connections without a proper SNI are rejected when allowSessionTicket is disabled. In addition, empty SNI extensions are explicitly treated as missing, ensuring stricter and more consistent TLS handshake validation. --- changelog.md | 7 +++ ts/00_commitinfo_data.ts | 2 +- ts/classes.portproxy.ts | 104 ++++++++++++++++++++++++--------------- ts/classes.snihandler.ts | 18 ++++++- 4 files changed, 88 insertions(+), 43 deletions(-) diff --git a/changelog.md b/changelog.md index 8eb6742..cf8918f 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,12 @@ # Changelog +## 2025-03-12 - 3.41.3 - fix(TLS/SNI) +Improve TLS session resumption handling and logging. Now, session resumption attempts are always logged with details, and connections without a proper SNI are rejected when allowSessionTicket is disabled. In addition, empty SNI extensions are explicitly treated as missing, ensuring stricter and more consistent TLS handshake validation. + +- Always log session resumption in both renegotiation and initial ClientHello processing. +- Terminate connections that attempt session resumption without SNI when allowSessionTicket is false. +- Treat empty SNI extensions as absence of SNI to improve consistency in TLS handshake processing. + ## 2025-03-11 - 3.41.2 - fix(SniHandler) Refactor hasSessionResumption to return detailed session resumption info diff --git a/ts/00_commitinfo_data.ts b/ts/00_commitinfo_data.ts index ffc6ec2..bff60f7 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: '3.41.2', + version: '3.41.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.portproxy.ts b/ts/classes.portproxy.ts index 0385bb1..afc702f 100644 --- a/ts/classes.portproxy.ts +++ b/ts/classes.portproxy.ts @@ -943,20 +943,28 @@ export class PortProxy { // Analyze for session resumption attempt (session ticket or PSK) const resumptionInfo = SniHandler.hasSessionResumption(renegChunk, this.settings.enableTlsDebugLogging); - // Only block if there's a session ticket without SNI - if (resumptionInfo.isResumption && !resumptionInfo.hasSNI) { + if (resumptionInfo.isResumption) { + // Always log resumption attempt for easier debugging console.log( - `[${connectionId}] Session ticket detected in renegotiation without SNI and allowSessionTicket=false. ` + - `Terminating connection to force new TLS handshake.` + `[${connectionId}] Session resumption detected in renegotiation. ` + + `Has SNI: ${resumptionInfo.hasSNI ? 'Yes' : 'No'}, allowSessionTicket: ${this.settings.allowSessionTicket}` ); - this.initiateCleanupOnce(record, 'session_ticket_blocked'); - return; - } else if (resumptionInfo.isResumption && resumptionInfo.hasSNI) { - if (this.settings.enableTlsDebugLogging) { + + // Block if there's session resumption without SNI + if (!resumptionInfo.hasSNI) { console.log( - `[${connectionId}] Session ticket with SNI detected in renegotiation. ` + - `Allowing connection since SNI is present.` + `[${connectionId}] Session resumption detected in renegotiation without SNI and allowSessionTicket=false. ` + + `Terminating connection to force new TLS handshake.` ); + this.initiateCleanupOnce(record, 'session_ticket_blocked'); + return; + } else { + if (this.settings.enableDetailedLogging) { + console.log( + `[${connectionId}] Session resumption with SNI detected in renegotiation. ` + + `Allowing connection since SNI is present.` + ); + } } } } @@ -1575,25 +1583,33 @@ export class PortProxy { // Analyze for session resumption attempt const resumptionInfo = SniHandler.hasSessionResumption(chunk, this.settings.enableTlsDebugLogging); - // Only block if there's a session ticket without SNI - if (resumptionInfo.isResumption && !resumptionInfo.hasSNI) { + if (resumptionInfo.isResumption) { + // Always log resumption attempt for easier debugging console.log( - `[${connectionId}] Session ticket detected in initial ClientHello without SNI and allowSessionTicket=false. ` + - `Terminating connection to force new TLS handshake.` + `[${connectionId}] Session resumption detected in initial ClientHello. ` + + `Has SNI: ${resumptionInfo.hasSNI ? 'Yes' : 'No'}, allowSessionTicket: ${this.settings.allowSessionTicket}` ); - if (connectionRecord.incomingTerminationReason === null) { - connectionRecord.incomingTerminationReason = 'session_ticket_blocked'; - this.incrementTerminationStat('incoming', 'session_ticket_blocked'); - } - socket.end(); - this.cleanupConnection(connectionRecord, 'session_ticket_blocked'); - return; - } else if (resumptionInfo.isResumption && resumptionInfo.hasSNI) { - if (this.settings.enableTlsDebugLogging) { + + // Block if there's session resumption without SNI + if (!resumptionInfo.hasSNI) { console.log( - `[${connectionId}] Session ticket with SNI detected in initial ClientHello. ` + - `Allowing connection since SNI is present.` + `[${connectionId}] Session resumption detected in initial ClientHello without SNI and allowSessionTicket=false. ` + + `Terminating connection to force new TLS handshake.` ); + if (connectionRecord.incomingTerminationReason === null) { + connectionRecord.incomingTerminationReason = 'session_ticket_blocked'; + this.incrementTerminationStat('incoming', 'session_ticket_blocked'); + } + socket.end(); + this.cleanupConnection(connectionRecord, 'session_ticket_blocked'); + return; + } else { + if (this.settings.enableDetailedLogging) { + console.log( + `[${connectionId}] Session resumption with SNI detected in initial ClientHello. ` + + `Allowing connection since SNI is present.` + ); + } } } } @@ -1949,25 +1965,33 @@ export class PortProxy { // Analyze for session resumption attempt const resumptionInfo = SniHandler.hasSessionResumption(chunk, this.settings.enableTlsDebugLogging); - // Only block if there's a session ticket without SNI - if (resumptionInfo.isResumption && !resumptionInfo.hasSNI) { + if (resumptionInfo.isResumption) { + // Always log resumption attempt for easier debugging console.log( - `[${connectionId}] Session ticket detected in initial ClientHello without SNI and allowSessionTicket=false. ` + - `Terminating connection to force new TLS handshake.` + `[${connectionId}] Session resumption detected in SNI handler. ` + + `Has SNI: ${resumptionInfo.hasSNI ? 'Yes' : 'No'}, allowSessionTicket: ${this.settings.allowSessionTicket}` ); - if (connectionRecord.incomingTerminationReason === null) { - connectionRecord.incomingTerminationReason = 'session_ticket_blocked'; - this.incrementTerminationStat('incoming', 'session_ticket_blocked'); - } - socket.end(); - this.cleanupConnection(connectionRecord, 'session_ticket_blocked'); - return; - } else if (resumptionInfo.isResumption && resumptionInfo.hasSNI) { - if (this.settings.enableTlsDebugLogging) { + + // Block if there's session resumption without SNI + if (!resumptionInfo.hasSNI) { console.log( - `[${connectionId}] Session ticket with SNI detected in initial ClientHello. ` + - `Allowing connection since SNI is present.` + `[${connectionId}] Session resumption detected in SNI handler without SNI and allowSessionTicket=false. ` + + `Terminating connection to force new TLS handshake.` ); + if (connectionRecord.incomingTerminationReason === null) { + connectionRecord.incomingTerminationReason = 'session_ticket_blocked'; + this.incrementTerminationStat('incoming', 'session_ticket_blocked'); + } + socket.end(); + this.cleanupConnection(connectionRecord, 'session_ticket_blocked'); + return; + } else { + if (this.settings.enableDetailedLogging) { + console.log( + `[${connectionId}] Session resumption with SNI detected in SNI handler. ` + + `Allowing connection since SNI is present.` + ); + } } } } diff --git a/ts/classes.snihandler.ts b/ts/classes.snihandler.ts index 91088c0..196e26b 100644 --- a/ts/classes.snihandler.ts +++ b/ts/classes.snihandler.ts @@ -410,8 +410,13 @@ export class SniHandler { pos += 2; if (extensionType === this.TLS_SNI_EXTENSION_TYPE) { - hasSNI = true; - log('Found SNI extension'); + // Check that the SNI extension actually has content + if (extensionLength > 0) { + hasSNI = true; + log('Found SNI extension with length: ' + extensionLength); + } else { + log('Found empty SNI extension, treating as no SNI'); + } break; } @@ -438,6 +443,15 @@ export class SniHandler { } // Return an object with both flags + // For clarity: connections should be blocked if they have session resumption without SNI + if (isResumption) { + log(`Resumption summary - hasSNI: ${hasSNI ? 'yes' : 'no'}, resumption type: ${ + hasSessionTicket ? 'session ticket, ' : '' + }${hasPSK ? 'PSK, ' : ''}${hasEarlyData ? 'early data, ' : ''}${ + hasNonEmptySessionId ? 'session ID' : '' + }`); + } + return { isResumption, hasSNI