fix(ConnectionHandler): Refactor ConnectionHandler code formatting for improved readability and consistency in log messages and whitespace handling

This commit is contained in:
Philipp Kunz 2025-03-15 19:10:54 +00:00
parent 794e1292e5
commit cad0e6a2b2
3 changed files with 149 additions and 162 deletions

View File

@ -1,5 +1,12 @@
# Changelog # Changelog
## 2025-03-15 - 4.1.4 - fix(ConnectionHandler)
Refactor ConnectionHandler code formatting for improved readability and consistency in log messages and whitespace handling
- Standardized indentation and spacing in method signatures and log statements
- Aligned inline comments and string concatenations for clarity
- Minor refactoring of parameter formatting without changing functionality
## 2025-03-15 - 4.1.3 - fix(connectionhandler) ## 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. 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.

View File

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

@ -1,5 +1,9 @@
import * as plugins from './plugins.js'; import * as plugins from './plugins.js';
import type { IConnectionRecord, IDomainConfig, IPortProxySettings } from './classes.pp.interfaces.js'; import type {
IConnectionRecord,
IDomainConfig,
IPortProxySettings,
} from './classes.pp.interfaces.js';
import { ConnectionManager } from './classes.pp.connectionmanager.js'; import { ConnectionManager } from './classes.pp.connectionmanager.js';
import { SecurityManager } from './classes.pp.securitymanager.js'; import { SecurityManager } from './classes.pp.securitymanager.js';
import { DomainConfigManager } from './classes.pp.domainconfigmanager.js'; import { DomainConfigManager } from './classes.pp.domainconfigmanager.js';
@ -73,8 +77,8 @@ export class ConnectionHandler {
if (this.settings.enableDetailedLogging) { if (this.settings.enableDetailedLogging) {
console.log( console.log(
`[${connectionId}] New connection from ${remoteIP} on port ${localPort}. ` + `[${connectionId}] New connection from ${remoteIP} on port ${localPort}. ` +
`Keep-Alive: ${record.hasKeepAlive ? 'Enabled' : 'Disabled'}. ` + `Keep-Alive: ${record.hasKeepAlive ? 'Enabled' : 'Disabled'}. ` +
`Active connections: ${this.connectionManager.getConnectionCount()}` `Active connections: ${this.connectionManager.getConnectionCount()}`
); );
} else { } else {
console.log( console.log(
@ -94,7 +98,10 @@ export class ConnectionHandler {
/** /**
* Handle a connection that should be forwarded to NetworkProxy * Handle a connection that should be forwarded to NetworkProxy
*/ */
private handleNetworkProxyConnection(socket: plugins.net.Socket, record: IConnectionRecord): void { private handleNetworkProxyConnection(
socket: plugins.net.Socket,
record: IConnectionRecord
): void {
const connectionId = record.id; const connectionId = record.id;
let initialDataReceived = false; let initialDataReceived = false;
@ -144,7 +151,7 @@ export class ConnectionHandler {
if (!this.tlsManager.isTlsHandshake(chunk) && localPort === 443) { if (!this.tlsManager.isTlsHandshake(chunk) && localPort === 443) {
console.log( console.log(
`[${connectionId}] Non-TLS connection detected on port 443. ` + `[${connectionId}] Non-TLS connection detected on port 443. ` +
`Terminating connection - only TLS traffic is allowed on standard HTTPS port.` `Terminating connection - only TLS traffic is allowed on standard HTTPS port.`
); );
if (record.incomingTerminationReason === null) { if (record.incomingTerminationReason === null) {
record.incomingTerminationReason = 'non_tls_blocked'; record.incomingTerminationReason = 'non_tls_blocked';
@ -159,8 +166,8 @@ export class ConnectionHandler {
if (this.tlsManager.isTlsHandshake(chunk)) { if (this.tlsManager.isTlsHandshake(chunk)) {
record.isTLS = true; record.isTLS = true;
// Check session tickets if they're disabled // Check for ClientHello to extract SNI - but don't enforce it for NetworkProxy
if (this.settings.allowSessionTicket === false && this.tlsManager.isClientHello(chunk)) { if (this.tlsManager.isClientHello(chunk)) {
// Create connection info for SNI extraction // Create connection info for SNI extraction
const connInfo = { const connInfo = {
sourceIp: record.remoteIP, sourceIp: record.remoteIP,
@ -169,83 +176,46 @@ export class ConnectionHandler {
destPort: socket.localPort || 0, destPort: socket.localPort || 0,
}; };
// Extract SNI for domain-specific NetworkProxy handling // Extract SNI for domain-specific NetworkProxy handling if available
const serverName = this.tlsManager.extractSNI(chunk, connInfo); const serverName = this.tlsManager.extractSNI(chunk, connInfo);
// If allowSessionTicket is false and we can't determine SNI, terminate the connection // For NetworkProxy connections, we'll allow session tickets even without SNI
if (!serverName) { // We'll only use the serverName if available to determine the specific NetworkProxy port
// Always block when allowSessionTicket is false and there's no SNI if (serverName) {
console.log( // Save domain config and SNI in connection record
`[${connectionId}] No SNI detected in ClientHello and allowSessionTicket=false. ` + const domainConfig = this.domainConfigManager.findDomainConfig(serverName);
`Terminating connection to force new TLS handshake with SNI.` record.domainConfig = domainConfig;
); record.lockedDomain = serverName;
// Send a proper TLS alert before ending the connection // Use domain-specific NetworkProxy port if configured
// Using "unrecognized_name" (112) alert which is a warning level alert (1) if (domainConfig && this.domainConfigManager.shouldUseNetworkProxy(domainConfig)) {
// that encourages clients to retry with proper SNI const networkProxyPort = this.domainConfigManager.getNetworkProxyPort(domainConfig);
const alertData = 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)
]);
try { if (this.settings.enableDetailedLogging) {
socket.write(alertData, () => { console.log(
// Only close the socket after we're sure the alert was sent `[${connectionId}] Using domain-specific NetworkProxy for ${serverName} on port ${networkProxyPort}`
// Give the alert time to be processed by the client );
setTimeout(() => { }
socket.end();
// Ensure complete cleanup happens a bit later // Forward to NetworkProxy with domain-specific port
setTimeout(() => { this.networkProxyBridge.forwardToNetworkProxy(
if (!socket.destroyed) { connectionId,
socket.destroy(); socket,
} record,
this.connectionManager.cleanupConnection(record, 'session_ticket_blocked_no_sni'); chunk,
}, 100); networkProxyPort,
}, 100); (reason) => this.connectionManager.initiateCleanupOnce(record, reason)
});
} catch (err) {
// 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) {
record.incomingTerminationReason = 'session_ticket_blocked_no_sni';
this.connectionManager.incrementTerminationStat('incoming', 'session_ticket_blocked_no_sni');
}
return;
}
// Save domain config and SNI in connection record
const domainConfig = this.domainConfigManager.findDomainConfig(serverName);
record.domainConfig = domainConfig;
record.lockedDomain = serverName;
// Use domain-specific NetworkProxy port if configured
if (domainConfig && this.domainConfigManager.shouldUseNetworkProxy(domainConfig)) {
const networkProxyPort = this.domainConfigManager.getNetworkProxyPort(domainConfig);
if (this.settings.enableDetailedLogging) {
console.log(
`[${connectionId}] Using domain-specific NetworkProxy for ${serverName} on port ${networkProxyPort}`
); );
return;
} }
} else if (
// Forward to NetworkProxy with domain-specific port this.settings.allowSessionTicket === false &&
this.networkProxyBridge.forwardToNetworkProxy( this.settings.enableDetailedLogging
connectionId, ) {
socket, // Log that we're allowing a session resumption without SNI for NetworkProxy
record, console.log(
chunk, `[${connectionId}] Allowing session resumption without SNI for NetworkProxy forwarding`
networkProxyPort,
(reason) => this.connectionManager.initiateCleanupOnce(record, reason)
); );
return;
} }
} }
@ -260,14 +230,10 @@ export class ConnectionHandler {
); );
} else { } else {
// If not TLS, use normal direct connection // If not TLS, use normal direct connection
console.log(`[${connectionId}] Non-TLS connection on NetworkProxy port ${record.localPort}`); console.log(
this.setupDirectConnection( `[${connectionId}] Non-TLS connection on NetworkProxy port ${record.localPort}`
socket,
record,
undefined,
undefined,
chunk
); );
this.setupDirectConnection(socket, record, undefined, undefined, chunk);
} }
}); });
} }
@ -385,14 +351,13 @@ export class ConnectionHandler {
record.domainConfig = domainConfig; record.domainConfig = domainConfig;
// Check if this domain should use NetworkProxy (domain-specific setting) // Check if this domain should use NetworkProxy (domain-specific setting)
if (domainConfig && if (
this.domainConfigManager.shouldUseNetworkProxy(domainConfig) && domainConfig &&
this.networkProxyBridge.getNetworkProxy()) { this.domainConfigManager.shouldUseNetworkProxy(domainConfig) &&
this.networkProxyBridge.getNetworkProxy()
) {
if (this.settings.enableDetailedLogging) { if (this.settings.enableDetailedLogging) {
console.log( console.log(`[${connectionId}] Domain ${serverName} is configured to use NetworkProxy`);
`[${connectionId}] Domain ${serverName} is configured to use NetworkProxy`
);
} }
const networkProxyPort = this.domainConfigManager.getNetworkProxyPort(domainConfig); const networkProxyPort = this.domainConfigManager.getNetworkProxyPort(domainConfig);
@ -418,19 +383,20 @@ export class ConnectionHandler {
// Skip IP validation if allowedIPs is empty // Skip IP validation if allowedIPs is empty
if ( if (
domainConfig.allowedIPs.length > 0 && domainConfig.allowedIPs.length > 0 &&
!this.securityManager.isIPAuthorized(record.remoteIP, ipRules.allowedIPs, ipRules.blockedIPs) !this.securityManager.isIPAuthorized(
record.remoteIP,
ipRules.allowedIPs,
ipRules.blockedIPs
)
) { ) {
return rejectIncomingConnection( return rejectIncomingConnection(
'rejected', 'rejected',
`Connection rejected: IP ${record.remoteIP} not allowed for domain ${domainConfig.domains.join( `Connection rejected: IP ${
', ' record.remoteIP
)}` } not allowed for domain ${domainConfig.domains.join(', ')}`
); );
} }
} else if ( } else if (this.settings.defaultAllowedIPs && this.settings.defaultAllowedIPs.length > 0) {
this.settings.defaultAllowedIPs &&
this.settings.defaultAllowedIPs.length > 0
) {
if ( if (
!this.securityManager.isIPAuthorized( !this.securityManager.isIPAuthorized(
record.remoteIP, record.remoteIP,
@ -501,9 +467,17 @@ export class ConnectionHandler {
if (forcedDomain) { if (forcedDomain) {
const ipRules = this.domainConfigManager.getEffectiveIPRules(forcedDomain); const ipRules = this.domainConfigManager.getEffectiveIPRules(forcedDomain);
if (!this.securityManager.isIPAuthorized(record.remoteIP, ipRules.allowedIPs, ipRules.blockedIPs)) { if (
!this.securityManager.isIPAuthorized(
record.remoteIP,
ipRules.allowedIPs,
ipRules.blockedIPs
)
) {
console.log( console.log(
`[${connectionId}] Connection from ${record.remoteIP} rejected: IP not allowed for domain ${forcedDomain.domains.join( `[${connectionId}] Connection from ${
record.remoteIP
} rejected: IP not allowed for domain ${forcedDomain.domains.join(
', ' ', '
)} on port ${localPort}.` )} on port ${localPort}.`
); );
@ -513,9 +487,9 @@ export class ConnectionHandler {
if (this.settings.enableDetailedLogging) { if (this.settings.enableDetailedLogging) {
console.log( console.log(
`[${connectionId}] Port-based connection from ${record.remoteIP} on port ${localPort} matched domain ${forcedDomain.domains.join( `[${connectionId}] Port-based connection from ${
', ' record.remoteIP
)}.` } on port ${localPort} matched domain ${forcedDomain.domains.join(', ')}.`
); );
} }
@ -543,7 +517,7 @@ export class ConnectionHandler {
if (!this.tlsManager.isTlsHandshake(chunk) && localPort === 443) { if (!this.tlsManager.isTlsHandshake(chunk) && localPort === 443) {
console.log( console.log(
`[${connectionId}] Non-TLS connection detected on port 443 in SNI handler. ` + `[${connectionId}] Non-TLS connection detected on port 443 in SNI handler. ` +
`Terminating connection - only TLS traffic is allowed on standard HTTPS port.` `Terminating connection - only TLS traffic is allowed on standard HTTPS port.`
); );
if (record.incomingTerminationReason === null) { if (record.incomingTerminationReason === null) {
record.incomingTerminationReason = 'non_tls_blocked'; record.incomingTerminationReason = 'non_tls_blocked';
@ -578,25 +552,28 @@ export class ConnectionHandler {
serverName = this.tlsManager.extractSNI(chunk, connInfo) || ''; serverName = this.tlsManager.extractSNI(chunk, connInfo) || '';
// If allowSessionTicket is false and this is a ClientHello with no SNI, terminate the connection // If allowSessionTicket is false and this is a ClientHello with no SNI, terminate the connection
if (this.settings.allowSessionTicket === false && if (
this.tlsManager.isClientHello(chunk) && this.settings.allowSessionTicket === false &&
!serverName) { this.tlsManager.isClientHello(chunk) &&
!serverName
) {
// Always block ClientHello without SNI when allowSessionTicket is false // Always block ClientHello without SNI when allowSessionTicket is false
console.log( console.log(
`[${connectionId}] No SNI detected in ClientHello and allowSessionTicket=false. ` + `[${connectionId}] No SNI detected in ClientHello and allowSessionTicket=false. ` +
`Terminating connection to force new TLS handshake with SNI.` `Terminating connection to force new TLS handshake with SNI.`
); );
// Send a proper TLS alert before ending the connection // Send a proper TLS alert before ending the connection
// Using "unrecognized_name" (112) alert which is a warning level alert (1) // Using "unrecognized_name" (112) alert which is a warning level alert (1)
// that encourages clients to retry with proper SNI // that encourages clients to retry with proper SNI
const alertData = Buffer.from([ const alertData = Buffer.from([
0x15, // Alert record type 0x15, // Alert record type
0x03, 0x03, // TLS 1.2 version 0x03,
0x00, 0x02, // Length 0x03, // TLS 1.2 version
0x01, // Warning alert level (not fatal) 0x00,
0x70 // unrecognized_name alert (code 112) 0x02, // Length
0x01, // Warning alert level (not fatal)
0x70, // unrecognized_name alert (code 112)
]); ]);
try { try {
@ -611,7 +588,10 @@ export class ConnectionHandler {
if (!socket.destroyed) { if (!socket.destroyed) {
socket.destroy(); socket.destroy();
} }
this.connectionManager.cleanupConnection(record, 'session_ticket_blocked_no_sni'); this.connectionManager.cleanupConnection(
record,
'session_ticket_blocked_no_sni'
);
}, 100); }, 100);
}, 100); }, 100);
}); });
@ -623,7 +603,10 @@ export class ConnectionHandler {
if (record.incomingTerminationReason === null) { if (record.incomingTerminationReason === null) {
record.incomingTerminationReason = 'session_ticket_blocked_no_sni'; record.incomingTerminationReason = 'session_ticket_blocked_no_sni';
this.connectionManager.incrementTerminationStat('incoming', 'session_ticket_blocked_no_sni'); this.connectionManager.incrementTerminationStat(
'incoming',
'session_ticket_blocked_no_sni'
);
} }
return; return;
@ -681,9 +664,7 @@ export class ConnectionHandler {
: this.settings.targetIP!; : this.settings.targetIP!;
// Determine target port // Determine target port
const targetPort = overridePort !== undefined const targetPort = overridePort !== undefined ? overridePort : this.settings.toPort;
? overridePort
: this.settings.toPort;
// Setup connection options // Setup connection options
const connectionOptions: plugins.net.NetConnectOpts = { const connectionOptions: plugins.net.NetConnectOpts = {
@ -956,7 +937,9 @@ export class ConnectionHandler {
const combinedData = Buffer.concat(record.pendingData); const combinedData = Buffer.concat(record.pendingData);
if (this.settings.enableDetailedLogging) { if (this.settings.enableDetailedLogging) {
console.log(`[${connectionId}] Forwarding ${combinedData.length} bytes of initial data to target`); console.log(
`[${connectionId}] Forwarding ${combinedData.length} bytes of initial data to target`
);
} }
// Write pending data immediately // Write pending data immediately
@ -1054,15 +1037,12 @@ export class ConnectionHandler {
} }
// Set connection timeout // Set connection timeout
record.cleanupTimer = this.timeoutManager.setupConnectionTimeout( record.cleanupTimer = this.timeoutManager.setupConnectionTimeout(record, (record, reason) => {
record, console.log(
(record, reason) => { `[${connectionId}] Connection from ${record.remoteIP} exceeded max lifetime, forcing cleanup.`
console.log( );
`[${connectionId}] Connection from ${record.remoteIP} exceeded max lifetime, forcing cleanup.` this.connectionManager.initiateCleanupOnce(record, reason);
); });
this.connectionManager.initiateCleanupOnce(record, reason);
}
);
// Mark TLS handshake as complete for TLS connections // Mark TLS handshake as complete for TLS connections
if (record.isTLS) { if (record.isTLS) {