From ca6f6de7985d8764137224721c35292d1f9b5521 Mon Sep 17 00:00:00 2001
From: Philipp Kunz <code@philkunz.com>
Date: Mon, 17 Mar 2025 13:37:48 +0000
Subject: [PATCH] fix(tls): Improve TLS alert handling in connection handler:
use the new TlsAlert class to send proper unrecognized_name alerts when a
ClientHello is missing SNI and wait for a retry on the same connection before
closing. Also, add alertFallbackTimeout tracking to connection records for
better timeout management.
---
changelog.md | 8 ++
ts/00_commitinfo_data.ts | 2 +-
ts/classes.pp.connectionhandler.ts | 198 +++++++++++++++-----------
ts/classes.pp.interfaces.ts | 1 +
ts/classes.pp.tlsalert.ts | 218 +++++++++++++++++++++++++++++
5 files changed, 343 insertions(+), 84 deletions(-)
create mode 100644 ts/classes.pp.tlsalert.ts
diff --git a/changelog.md b/changelog.md
index e632a6f..1102378 100644
--- a/changelog.md
+++ b/changelog.md
@@ -1,5 +1,13 @@
# Changelog
+## 2025-03-17 - 4.1.16 - fix(tls)
+Improve TLS alert handling in connection handler: use the new TlsAlert class to send proper unrecognized_name alerts when a ClientHello is missing SNI and wait for a retry on the same connection before closing. Also, add alertFallbackTimeout tracking to connection records for better timeout management.
+
+- Replaced hardcoded alert buffers in ConnectionHandler with calls to the TlsAlert class.
+- Removed old warnings and implemented a mechanism to remove existing 'data' listeners and await a new ClientHello.
+- Introduced alertFallbackTimeout property in connection records to track fallback timeout and ensure proper cleanup.
+- Extended the delay before closing the connection after sending an alert, providing the client more time to retry.
+
## 2025-03-17 - 4.1.15 - fix(connectionhandler)
Delay socket termination in TLS session resumption handling to allow proper alert processing
diff --git a/ts/00_commitinfo_data.ts b/ts/00_commitinfo_data.ts
index cd86256..428d661 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.15',
+ version: '4.1.16',
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 40b38e3..f7a1389 100644
--- a/ts/classes.pp.connectionhandler.ts
+++ b/ts/classes.pp.connectionhandler.ts
@@ -11,6 +11,7 @@ import { TlsManager } from './classes.pp.tlsmanager.js';
import { NetworkProxyBridge } from './classes.pp.networkproxybridge.js';
import { TimeoutManager } from './classes.pp.timeoutmanager.js';
import { PortRangeManager } from './classes.pp.portrangemanager.js';
+import { TlsAlert } from './classes.pp.tlsalert.js';
/**
* Handles new connection processing and setup logic
@@ -560,95 +561,125 @@ 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 unrecognized_name alert to encourage immediate retry with SNI on same connection.`
);
- // Set the termination reason first
- if (record.incomingTerminationReason === null) {
- record.incomingTerminationReason = 'session_ticket_blocked_no_sni';
- this.connectionManager.incrementTerminationStat(
- 'incoming',
- 'session_ticket_blocked_no_sni'
- );
- }
-
- // Create a warning-level alert for unrecognized_name
- // This encourages Chrome to retry immediately with SNI
- const serverNameUnknownAlertData = Buffer.from([
- 0x15, // Alert record type
- 0x03,
- 0x03, // TLS 1.2 version
- 0x00,
- 0x02, // Length
- 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)
- ]);
-
- const closeNotifyAlert = Buffer.from([
- 0x15, // Alert record type
- 0x03,
- 0x03, // TLS 1.2 version
- 0x00,
- 0x02, // Length
- 0x01, // Warning alert level (1)
- 0x00, // close_notify alert (0)
- ]);
-
- const certificateExpiredAlert = Buffer.from([
- 0x15, // Alert record type
- 0x03,
- 0x03, // TLS 1.2 version
- 0x00,
- 0x02, // Length
- 0x01, // Warning alert level (1)
- 0x2F, // certificate_expired alert (47)
- ]);
-
try {
- // Use cork/uncork to ensure the alert is sent as a single packet
+ // Send the alert but do NOT end the connection
+ // Using our new TlsAlert class for better alert management
socket.cork();
- const writeSuccessful = socket.write(serverNameUnknownAlertData);
+ socket.write(TlsAlert.alerts.unrecognizedName);
socket.uncork();
-
- // Function to handle the clean socket termination - but more gradually
- const finishConnection = () => {
- // Give Chrome more time to process the alert before closing
- // We won't call destroy() at all - just end() and let the socket close naturally
-
- // Log the cleanup but wait for natural closure
- setTimeout(() => {
- socket.end();
- this.connectionManager.cleanupConnection(record, 'session_ticket_blocked_no_sni');
- }, 5000); // Longer delay to let socket cleanup happen naturally
- };
-
- if (writeSuccessful) {
- // Wait longer before ending connection to ensure alert is processed by client
- setTimeout(finishConnection, 200); // Increased from 50ms to 200ms
- } else {
- // If the kernel buffer was full, wait for the drain event
- socket.once('drain', () => {
- // Wait longer after drain as well
- setTimeout(finishConnection, 200);
- });
-
- // Safety timeout is increased too
- setTimeout(() => {
- socket.removeAllListeners('drain');
- finishConnection();
- }, 400); // Increased from 250ms to 400ms
+
+ console.log(
+ `[${connectionId}] Alert sent, waiting for new ClientHello on same connection...`
+ );
+
+ // Remove existing data listener and wait for a new ClientHello
+ socket.removeAllListeners('data');
+
+ // Set up a new data handler to capture the next message
+ socket.once('data', (retryChunk) => {
+ // Cancel the fallback timeout as we received data
+ if (record.alertFallbackTimeout) {
+ clearTimeout(record.alertFallbackTimeout);
+ record.alertFallbackTimeout = null;
+ }
+
+ // Check if this is a new ClientHello
+ if (this.tlsManager.isClientHello(retryChunk)) {
+ console.log(`[${connectionId}] Received new ClientHello after alert`);
+
+ // Extract SNI from the new ClientHello
+ const newServerName = this.tlsManager.extractSNI(retryChunk, connInfo) || '';
+
+ if (newServerName) {
+ console.log(`[${connectionId}] New ClientHello contains SNI: ${newServerName}`);
+
+ // Update the record with the new SNI
+ record.lockedDomain = newServerName;
+
+ // Continue with normal connection setup using the new chunk with SNI
+ setupConnection(newServerName, retryChunk);
+ } else {
+ console.log(
+ `[${connectionId}] New ClientHello still missing SNI, closing connection`
+ );
+
+ // If still no SNI after retry, now we can close the connection
+ if (record.incomingTerminationReason === null) {
+ record.incomingTerminationReason = 'session_ticket_blocked_no_sni';
+ this.connectionManager.incrementTerminationStat(
+ 'incoming',
+ 'session_ticket_blocked_no_sni'
+ );
+ }
+
+ // Send a close_notify alert before ending the connection
+ TlsAlert.sendCloseNotify(socket)
+ .catch((err) => {
+ console.log(`[${connectionId}] Error sending close_notify: ${err.message}`);
+ })
+ .finally(() => {
+ // Clean up even if sending the alert fails
+ this.connectionManager.cleanupConnection(
+ record,
+ 'session_ticket_blocked_no_sni'
+ );
+ });
+ }
+ } else {
+ console.log(
+ `[${connectionId}] Received non-ClientHello data after alert, closing connection`
+ );
+
+ // If we got something other than a ClientHello, close the connection
+ if (record.incomingTerminationReason === null) {
+ record.incomingTerminationReason = 'invalid_protocol';
+ this.connectionManager.incrementTerminationStat('incoming', 'invalid_protocol');
+ }
+
+ // Send a protocol_version alert before ending the connection
+ TlsAlert.send(socket, TlsAlert.LEVEL_FATAL, TlsAlert.PROTOCOL_VERSION, true)
+ .catch((err) => {
+ console.log(
+ `[${connectionId}] Error sending protocol_version alert: ${err.message}`
+ );
+ })
+ .finally(() => {
+ // Clean up even if sending the alert fails
+ this.connectionManager.cleanupConnection(record, 'invalid_protocol');
+ });
+ }
+ });
+
+ // Set a fallback timeout in case the client doesn't respond
+ const fallbackTimeout = setTimeout(() => {
+ console.log(`[${connectionId}] No response after alert, closing connection`);
+
+ if (record.incomingTerminationReason === null) {
+ record.incomingTerminationReason = 'alert_timeout';
+ this.connectionManager.incrementTerminationStat('incoming', 'alert_timeout');
+ }
+
+ // Send a close_notify alert before ending the connection
+ TlsAlert.sendCloseNotify(socket)
+ .catch((err) => {
+ console.log(`[${connectionId}] Error sending close_notify: ${err.message}`);
+ })
+ .finally(() => {
+ // Clean up even if sending the alert fails
+ this.connectionManager.cleanupConnection(record, 'alert_timeout');
+ });
+ }, 10000); // 10 second timeout
+
+ // Make sure the timeout doesn't keep the process alive
+ if (fallbackTimeout.unref) {
+ fallbackTimeout.unref();
}
+
+ // Store the timeout in the record so it can be cleared during cleanup
+ record.alertFallbackTimeout = fallbackTimeout;
} catch (err) {
// If we can't send the alert, fall back to immediate termination
console.log(`[${connectionId}] Error sending TLS alert: ${err.message}`);
@@ -656,6 +687,7 @@ export class ConnectionHandler {
this.connectionManager.cleanupConnection(record, 'session_ticket_blocked_no_sni');
}
+ // Return early to prevent the normal flow
return;
}
}
diff --git a/ts/classes.pp.interfaces.ts b/ts/classes.pp.interfaces.ts
index f096cf7..fd77d5e 100644
--- a/ts/classes.pp.interfaces.ts
+++ b/ts/classes.pp.interfaces.ts
@@ -104,6 +104,7 @@ export interface IConnectionRecord {
lockedDomain?: string; // Used to lock this connection to the initial SNI
connectionClosed: boolean; // Flag to prevent multiple cleanup attempts
cleanupTimer?: NodeJS.Timeout; // Timer for max lifetime/inactivity
+ alertFallbackTimeout?: NodeJS.Timeout; // Timer for fallback after alert
lastActivity: number; // Last activity timestamp for inactivity detection
pendingData: Buffer[]; // Buffer to hold data during connection setup
pendingDataSize: number; // Track total size of pending data
diff --git a/ts/classes.pp.tlsalert.ts b/ts/classes.pp.tlsalert.ts
new file mode 100644
index 0000000..f5aeecc
--- /dev/null
+++ b/ts/classes.pp.tlsalert.ts
@@ -0,0 +1,218 @@
+import * as net from 'net';
+
+/**
+ * TlsAlert class for managing TLS alert messages
+ */
+export class TlsAlert {
+ // TLS Alert Levels
+ static readonly LEVEL_WARNING = 0x01;
+ static readonly LEVEL_FATAL = 0x02;
+
+ // TLS Alert Description Codes - RFC 8446 (TLS 1.3) / RFC 5246 (TLS 1.2)
+ static readonly CLOSE_NOTIFY = 0x00;
+ static readonly UNEXPECTED_MESSAGE = 0x0A;
+ static readonly BAD_RECORD_MAC = 0x14;
+ static readonly DECRYPTION_FAILED = 0x15; // TLS 1.0 only
+ static readonly RECORD_OVERFLOW = 0x16;
+ static readonly DECOMPRESSION_FAILURE = 0x1E; // TLS 1.2 and below
+ static readonly HANDSHAKE_FAILURE = 0x28;
+ static readonly NO_CERTIFICATE = 0x29; // SSLv3 only
+ static readonly BAD_CERTIFICATE = 0x2A;
+ static readonly UNSUPPORTED_CERTIFICATE = 0x2B;
+ static readonly CERTIFICATE_REVOKED = 0x2C;
+ static readonly CERTIFICATE_EXPIRED = 0x2F;
+ static readonly CERTIFICATE_UNKNOWN = 0x30;
+ static readonly ILLEGAL_PARAMETER = 0x2F;
+ static readonly UNKNOWN_CA = 0x30;
+ static readonly ACCESS_DENIED = 0x31;
+ static readonly DECODE_ERROR = 0x32;
+ static readonly DECRYPT_ERROR = 0x33;
+ static readonly EXPORT_RESTRICTION = 0x3C; // TLS 1.0 only
+ static readonly PROTOCOL_VERSION = 0x46;
+ static readonly INSUFFICIENT_SECURITY = 0x47;
+ static readonly INTERNAL_ERROR = 0x50;
+ static readonly INAPPROPRIATE_FALLBACK = 0x56;
+ static readonly USER_CANCELED = 0x5A;
+ static readonly NO_RENEGOTIATION = 0x64; // TLS 1.2 and below
+ static readonly MISSING_EXTENSION = 0x6D; // TLS 1.3
+ static readonly UNSUPPORTED_EXTENSION = 0x6E; // TLS 1.3
+ static readonly CERTIFICATE_REQUIRED = 0x6F; // TLS 1.3
+ static readonly UNRECOGNIZED_NAME = 0x70;
+ static readonly BAD_CERTIFICATE_STATUS_RESPONSE = 0x71;
+ static readonly BAD_CERTIFICATE_HASH_VALUE = 0x72; // TLS 1.2 and below
+ static readonly UNKNOWN_PSK_IDENTITY = 0x73;
+ static readonly CERTIFICATE_REQUIRED_1_3 = 0x74; // TLS 1.3
+ static readonly NO_APPLICATION_PROTOCOL = 0x78;
+
+ /**
+ * Create a TLS alert buffer with the specified level and description code
+ *
+ * @param level Alert level (warning or fatal)
+ * @param description Alert description code
+ * @param tlsVersion TLS version bytes (default is TLS 1.2: 0x0303)
+ * @returns Buffer containing the TLS alert message
+ */
+ static create(
+ level: number,
+ description: number,
+ tlsVersion: [number, number] = [0x03, 0x03]
+ ): Buffer {
+ return Buffer.from([
+ 0x15, // Alert record type
+ tlsVersion[0],
+ tlsVersion[1], // TLS version (default to TLS 1.2: 0x0303)
+ 0x00,
+ 0x02, // Length
+ level, // Alert level
+ description, // Alert description
+ ]);
+ }
+
+ /**
+ * Create a warning-level TLS alert
+ *
+ * @param description Alert description code
+ * @returns Buffer containing the warning-level TLS alert message
+ */
+ static createWarning(description: number): Buffer {
+ return this.create(this.LEVEL_WARNING, description);
+ }
+
+ /**
+ * Create a fatal-level TLS alert
+ *
+ * @param description Alert description code
+ * @returns Buffer containing the fatal-level TLS alert message
+ */
+ static createFatal(description: number): Buffer {
+ return this.create(this.LEVEL_FATAL, description);
+ }
+
+ /**
+ * Send a TLS alert to a socket and optionally close the connection
+ *
+ * @param socket The socket to send the alert to
+ * @param level Alert level (warning or fatal)
+ * @param description Alert description code
+ * @param closeAfterSend Whether to close the connection after sending the alert
+ * @param closeDelay Milliseconds to wait before closing the connection (default: 200ms)
+ * @returns Promise that resolves when the alert has been sent
+ */
+ static async send(
+ socket: net.Socket,
+ level: number,
+ description: number,
+ closeAfterSend: boolean = false,
+ closeDelay: number = 200
+ ): Promise<void> {
+ const alert = this.create(level, description);
+
+ return new Promise<void>((resolve, reject) => {
+ try {
+ // Ensure the alert is written as a single packet
+ socket.cork();
+ const writeSuccessful = socket.write(alert, (err) => {
+ if (err) {
+ reject(err);
+ return;
+ }
+
+ if (closeAfterSend) {
+ setTimeout(() => {
+ socket.end();
+ resolve();
+ }, closeDelay);
+ } else {
+ resolve();
+ }
+ });
+ socket.uncork();
+
+ // If write wasn't successful immediately, wait for drain
+ if (!writeSuccessful && !closeAfterSend) {
+ socket.once('drain', () => {
+ resolve();
+ });
+ }
+ } catch (err) {
+ reject(err);
+ }
+ });
+ }
+
+ /**
+ * Pre-defined TLS alert messages
+ */
+ static readonly alerts = {
+ // Warning level alerts
+ closeNotify: TlsAlert.createWarning(TlsAlert.CLOSE_NOTIFY),
+ unsupportedExtension: TlsAlert.createWarning(TlsAlert.UNSUPPORTED_EXTENSION),
+ certificateRequired: TlsAlert.createWarning(TlsAlert.CERTIFICATE_REQUIRED),
+ unrecognizedName: TlsAlert.createWarning(TlsAlert.UNRECOGNIZED_NAME),
+ noRenegotiation: TlsAlert.createWarning(TlsAlert.NO_RENEGOTIATION),
+ userCanceled: TlsAlert.createWarning(TlsAlert.USER_CANCELED),
+
+ // Warning level alerts for session resumption
+ certificateExpiredWarning: TlsAlert.createWarning(TlsAlert.CERTIFICATE_EXPIRED),
+ handshakeFailureWarning: TlsAlert.createWarning(TlsAlert.HANDSHAKE_FAILURE),
+ insufficientSecurityWarning: TlsAlert.createWarning(TlsAlert.INSUFFICIENT_SECURITY),
+
+ // Fatal level alerts
+ unexpectedMessage: TlsAlert.createFatal(TlsAlert.UNEXPECTED_MESSAGE),
+ badRecordMac: TlsAlert.createFatal(TlsAlert.BAD_RECORD_MAC),
+ recordOverflow: TlsAlert.createFatal(TlsAlert.RECORD_OVERFLOW),
+ handshakeFailure: TlsAlert.createFatal(TlsAlert.HANDSHAKE_FAILURE),
+ badCertificate: TlsAlert.createFatal(TlsAlert.BAD_CERTIFICATE),
+ certificateExpired: TlsAlert.createFatal(TlsAlert.CERTIFICATE_EXPIRED),
+ certificateUnknown: TlsAlert.createFatal(TlsAlert.CERTIFICATE_UNKNOWN),
+ illegalParameter: TlsAlert.createFatal(TlsAlert.ILLEGAL_PARAMETER),
+ unknownCA: TlsAlert.createFatal(TlsAlert.UNKNOWN_CA),
+ accessDenied: TlsAlert.createFatal(TlsAlert.ACCESS_DENIED),
+ decodeError: TlsAlert.createFatal(TlsAlert.DECODE_ERROR),
+ decryptError: TlsAlert.createFatal(TlsAlert.DECRYPT_ERROR),
+ protocolVersion: TlsAlert.createFatal(TlsAlert.PROTOCOL_VERSION),
+ insufficientSecurity: TlsAlert.createFatal(TlsAlert.INSUFFICIENT_SECURITY),
+ internalError: TlsAlert.createFatal(TlsAlert.INTERNAL_ERROR),
+ };
+
+ /**
+ * Utility method to send a warning-level unrecognized_name alert
+ * Specifically designed for SNI issues to encourage the client to retry with SNI
+ *
+ * @param socket The socket to send the alert to
+ * @returns Promise that resolves when the alert has been sent
+ */
+ static async sendSniRequired(socket: net.Socket): Promise<void> {
+ return this.send(socket, this.LEVEL_WARNING, this.UNRECOGNIZED_NAME);
+ }
+
+ /**
+ * Utility method to send a close_notify alert and close the connection
+ *
+ * @param socket The socket to send the alert to
+ * @param closeDelay Milliseconds to wait before closing the connection (default: 200ms)
+ * @returns Promise that resolves when the alert has been sent and the connection closed
+ */
+ static async sendCloseNotify(socket: net.Socket, closeDelay: number = 200): Promise<void> {
+ return this.send(socket, this.LEVEL_WARNING, this.CLOSE_NOTIFY, true, closeDelay);
+ }
+
+ /**
+ * Utility method to send a certificate_expired alert to force new TLS session
+ *
+ * @param socket The socket to send the alert to
+ * @param fatal Whether to send as a fatal alert (default: false)
+ * @param closeAfterSend Whether to close the connection after sending the alert (default: true)
+ * @param closeDelay Milliseconds to wait before closing the connection (default: 200ms)
+ * @returns Promise that resolves when the alert has been sent
+ */
+ static async sendCertificateExpired(
+ socket: net.Socket,
+ fatal: boolean = false,
+ closeAfterSend: boolean = true,
+ closeDelay: number = 200
+ ): Promise<void> {
+ const level = fatal ? this.LEVEL_FATAL : this.LEVEL_WARNING;
+ return this.send(socket, level, this.CERTIFICATE_EXPIRED, closeAfterSend, closeDelay);
+ }
+}
\ No newline at end of file