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.

This commit is contained in:
Philipp Kunz 2025-03-12 09:56:21 +00:00
parent b48b90d613
commit 1a90566622
4 changed files with 88 additions and 43 deletions

View File

@ -1,5 +1,12 @@
# Changelog # 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) ## 2025-03-11 - 3.41.2 - fix(SniHandler)
Refactor hasSessionResumption to return detailed session resumption info Refactor hasSessionResumption to return detailed session resumption info

View File

@ -3,6 +3,6 @@
*/ */
export const commitinfo = { export const commitinfo = {
name: '@push.rocks/smartproxy', 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.' 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.'
} }

View File

@ -943,20 +943,28 @@ export class PortProxy {
// Analyze for session resumption attempt (session ticket or PSK) // Analyze for session resumption attempt (session ticket or PSK)
const resumptionInfo = SniHandler.hasSessionResumption(renegChunk, this.settings.enableTlsDebugLogging); const resumptionInfo = SniHandler.hasSessionResumption(renegChunk, this.settings.enableTlsDebugLogging);
// Only block if there's a session ticket without SNI if (resumptionInfo.isResumption) {
if (resumptionInfo.isResumption && !resumptionInfo.hasSNI) { // Always log resumption attempt for easier debugging
console.log( console.log(
`[${connectionId}] Session ticket detected in renegotiation without SNI and allowSessionTicket=false. ` + `[${connectionId}] Session resumption detected in renegotiation. ` +
`Terminating connection to force new TLS handshake.` `Has SNI: ${resumptionInfo.hasSNI ? 'Yes' : 'No'}, allowSessionTicket: ${this.settings.allowSessionTicket}`
); );
this.initiateCleanupOnce(record, 'session_ticket_blocked');
return; // Block if there's session resumption without SNI
} else if (resumptionInfo.isResumption && resumptionInfo.hasSNI) { if (!resumptionInfo.hasSNI) {
if (this.settings.enableTlsDebugLogging) {
console.log( console.log(
`[${connectionId}] Session ticket with SNI detected in renegotiation. ` + `[${connectionId}] Session resumption detected in renegotiation without SNI and allowSessionTicket=false. ` +
`Allowing connection since SNI is present.` `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 // Analyze for session resumption attempt
const resumptionInfo = SniHandler.hasSessionResumption(chunk, this.settings.enableTlsDebugLogging); const resumptionInfo = SniHandler.hasSessionResumption(chunk, this.settings.enableTlsDebugLogging);
// Only block if there's a session ticket without SNI if (resumptionInfo.isResumption) {
if (resumptionInfo.isResumption && !resumptionInfo.hasSNI) { // Always log resumption attempt for easier debugging
console.log( console.log(
`[${connectionId}] Session ticket detected in initial ClientHello without SNI and allowSessionTicket=false. ` + `[${connectionId}] Session resumption detected in initial ClientHello. ` +
`Terminating connection to force new TLS handshake.` `Has SNI: ${resumptionInfo.hasSNI ? 'Yes' : 'No'}, allowSessionTicket: ${this.settings.allowSessionTicket}`
); );
if (connectionRecord.incomingTerminationReason === null) {
connectionRecord.incomingTerminationReason = 'session_ticket_blocked'; // Block if there's session resumption without SNI
this.incrementTerminationStat('incoming', 'session_ticket_blocked'); if (!resumptionInfo.hasSNI) {
}
socket.end();
this.cleanupConnection(connectionRecord, 'session_ticket_blocked');
return;
} else if (resumptionInfo.isResumption && resumptionInfo.hasSNI) {
if (this.settings.enableTlsDebugLogging) {
console.log( console.log(
`[${connectionId}] Session ticket with SNI detected in initial ClientHello. ` + `[${connectionId}] Session resumption detected in initial ClientHello without SNI and allowSessionTicket=false. ` +
`Allowing connection since SNI is present.` `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 // Analyze for session resumption attempt
const resumptionInfo = SniHandler.hasSessionResumption(chunk, this.settings.enableTlsDebugLogging); const resumptionInfo = SniHandler.hasSessionResumption(chunk, this.settings.enableTlsDebugLogging);
// Only block if there's a session ticket without SNI if (resumptionInfo.isResumption) {
if (resumptionInfo.isResumption && !resumptionInfo.hasSNI) { // Always log resumption attempt for easier debugging
console.log( console.log(
`[${connectionId}] Session ticket detected in initial ClientHello without SNI and allowSessionTicket=false. ` + `[${connectionId}] Session resumption detected in SNI handler. ` +
`Terminating connection to force new TLS handshake.` `Has SNI: ${resumptionInfo.hasSNI ? 'Yes' : 'No'}, allowSessionTicket: ${this.settings.allowSessionTicket}`
); );
if (connectionRecord.incomingTerminationReason === null) {
connectionRecord.incomingTerminationReason = 'session_ticket_blocked'; // Block if there's session resumption without SNI
this.incrementTerminationStat('incoming', 'session_ticket_blocked'); if (!resumptionInfo.hasSNI) {
}
socket.end();
this.cleanupConnection(connectionRecord, 'session_ticket_blocked');
return;
} else if (resumptionInfo.isResumption && resumptionInfo.hasSNI) {
if (this.settings.enableTlsDebugLogging) {
console.log( console.log(
`[${connectionId}] Session ticket with SNI detected in initial ClientHello. ` + `[${connectionId}] Session resumption detected in SNI handler without SNI and allowSessionTicket=false. ` +
`Allowing connection since SNI is present.` `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.`
);
}
} }
} }
} }

View File

@ -410,8 +410,13 @@ export class SniHandler {
pos += 2; pos += 2;
if (extensionType === this.TLS_SNI_EXTENSION_TYPE) { if (extensionType === this.TLS_SNI_EXTENSION_TYPE) {
hasSNI = true; // Check that the SNI extension actually has content
log('Found SNI extension'); if (extensionLength > 0) {
hasSNI = true;
log('Found SNI extension with length: ' + extensionLength);
} else {
log('Found empty SNI extension, treating as no SNI');
}
break; break;
} }
@ -438,6 +443,15 @@ export class SniHandler {
} }
// Return an object with both flags // 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 { return {
isResumption, isResumption,
hasSNI hasSNI