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.

This commit is contained in:
Philipp Kunz 2025-03-15 18:51:50 +00:00
parent 107bc3b50b
commit ee79f9ab7c
3 changed files with 52 additions and 24 deletions

View File

@ -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.

View File

@ -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.'
}

View File

@ -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;
}
}