fix(PortProxy): Move TLS renegotiation detection before socket piping

Fundamentally restructured TLS renegotiation handling to ensure handshake packets are properly detected. The previous implementation attached event handlers after pipe() was established, which might have caused handshake packets to bypass detection. Key changes:

1. Moved renegotiation detection before pipe() to ensure all TLS handshake packets are detected
2. Added explicit lockedDomain setting for all SNI connections
3. Simplified the NetworkProxy TLS handshake detection
4. Removed redundant data handlers that could interfere with each other

These changes should make renegotiation detection more reliable regardless of how Node.js internal pipe() implementation handles data events.

🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Philipp Kunz 2025-03-11 02:45:51 +00:00
parent 2bdd6f8c1f
commit ef8f583a90

View File

@ -501,28 +501,17 @@ export class PortProxy {
}
this.cleanupConnection(record, 'client_closed');
});
// Update activity on data transfer
// Special handler for TLS handshake detection with NetworkProxy
socket.on('data', (chunk: Buffer) => {
this.updateActivity(record);
// Check for potential TLS renegotiation or reconnection packets
// Check for TLS handshake packets (ContentType.handshake)
if (chunk.length > 0 && chunk[0] === 22) {
// ContentType.handshake
if (this.settings.enableDetailedLogging) {
console.log(
`[${connectionId}] Detected potential TLS handshake data while connected to NetworkProxy`
);
}
// NOTE: We don't need to explicitly forward the renegotiation packets
// because socket.pipe(proxySocket) is already handling that.
// The pipe ensures all data (including renegotiation) flows through properly.
// Just update the activity timestamp to prevent timeouts
record.lastActivity = Date.now();
console.log(`[${connectionId}] Detected potential TLS handshake with NetworkProxy, updating activity`);
this.updateActivity(record);
}
});
// Update activity on data transfer from the proxy socket
proxySocket.on('data', () => this.updateActivity(record));
if (this.settings.enableDetailedLogging) {
@ -778,6 +767,75 @@ export class PortProxy {
return this.initiateCleanupOnce(record, 'write_error');
}
// Set up the renegotiation listener *before* piping if this is a TLS connection with SNI
if (serverName && connectionRecord.isTLS) {
// This listener handles TLS renegotiation detection
socket.on('data', (renegChunk) => {
if (renegChunk.length > 0 && renegChunk.readUInt8(0) === 22) {
// Always update activity timestamp for any handshake packet
this.updateActivity(connectionRecord);
try {
// Try to extract SNI from potential renegotiation
const newSNI = extractSNI(renegChunk, this.settings.enableTlsDebugLogging);
// IMPORTANT: If we can't extract an SNI from renegotiation, we MUST allow it through
if (newSNI === undefined) {
console.log(`[${connectionId}] Rehandshake detected without SNI, allowing it through.`);
return;
}
// Check if the SNI has changed
if (newSNI !== serverName) {
console.log(`[${connectionId}] Rehandshake with different SNI: ${newSNI} vs original ${serverName}`);
// Allow if the new SNI matches existing domain config or find a new matching config
let allowed = false;
if (connectionRecord.domainConfig) {
allowed = connectionRecord.domainConfig.domains.some(d => plugins.minimatch(newSNI, d));
}
if (!allowed) {
const newDomainConfig = this.settings.domainConfigs.find((config) =>
config.domains.some((d) => plugins.minimatch(newSNI, d))
);
if (newDomainConfig) {
const effectiveAllowedIPs = [
...newDomainConfig.allowedIPs,
...(this.settings.defaultAllowedIPs || []),
];
const effectiveBlockedIPs = [
...(newDomainConfig.blockedIPs || []),
...(this.settings.defaultBlockedIPs || []),
];
allowed = isGlobIPAllowed(connectionRecord.remoteIP, effectiveAllowedIPs, effectiveBlockedIPs);
if (allowed) {
connectionRecord.domainConfig = newDomainConfig;
}
}
}
if (allowed) {
console.log(`[${connectionId}] Updated domain for connection from ${connectionRecord.remoteIP} to: ${newSNI}`);
connectionRecord.lockedDomain = newSNI;
} else {
console.log(`[${connectionId}] Rehandshake SNI ${newSNI} not allowed. Terminating connection.`);
this.initiateCleanupOnce(connectionRecord, 'sni_mismatch');
}
} else {
console.log(`[${connectionId}] Rehandshake with same SNI: ${newSNI}`);
}
} catch (err) {
console.log(`[${connectionId}] Error processing renegotiation: ${err}. Allowing to continue.`);
}
}
});
}
// Now set up piping for future data and resume the socket
socket.pipe(targetSocket);
targetSocket.pipe(socket);
@ -811,7 +869,76 @@ export class PortProxy {
}
});
} else {
// No pending data, so just set up piping
// Set up the renegotiation listener *before* piping if this is a TLS connection with SNI
if (serverName && connectionRecord.isTLS) {
// This listener handles TLS renegotiation detection
socket.on('data', (renegChunk) => {
if (renegChunk.length > 0 && renegChunk.readUInt8(0) === 22) {
// Always update activity timestamp for any handshake packet
this.updateActivity(connectionRecord);
try {
// Try to extract SNI from potential renegotiation
const newSNI = extractSNI(renegChunk, this.settings.enableTlsDebugLogging);
// IMPORTANT: If we can't extract an SNI from renegotiation, we MUST allow it through
if (newSNI === undefined) {
console.log(`[${connectionId}] Rehandshake detected without SNI, allowing it through.`);
return;
}
// Check if the SNI has changed
if (newSNI !== serverName) {
console.log(`[${connectionId}] Rehandshake with different SNI: ${newSNI} vs original ${serverName}`);
// Allow if the new SNI matches existing domain config or find a new matching config
let allowed = false;
if (connectionRecord.domainConfig) {
allowed = connectionRecord.domainConfig.domains.some(d => plugins.minimatch(newSNI, d));
}
if (!allowed) {
const newDomainConfig = this.settings.domainConfigs.find((config) =>
config.domains.some((d) => plugins.minimatch(newSNI, d))
);
if (newDomainConfig) {
const effectiveAllowedIPs = [
...newDomainConfig.allowedIPs,
...(this.settings.defaultAllowedIPs || []),
];
const effectiveBlockedIPs = [
...(newDomainConfig.blockedIPs || []),
...(this.settings.defaultBlockedIPs || []),
];
allowed = isGlobIPAllowed(connectionRecord.remoteIP, effectiveAllowedIPs, effectiveBlockedIPs);
if (allowed) {
connectionRecord.domainConfig = newDomainConfig;
}
}
}
if (allowed) {
console.log(`[${connectionId}] Updated domain for connection from ${connectionRecord.remoteIP} to: ${newSNI}`);
connectionRecord.lockedDomain = newSNI;
} else {
console.log(`[${connectionId}] Rehandshake SNI ${newSNI} not allowed. Terminating connection.`);
this.initiateCleanupOnce(connectionRecord, 'sni_mismatch');
}
} else {
console.log(`[${connectionId}] Rehandshake with same SNI: ${newSNI}`);
}
} catch (err) {
console.log(`[${connectionId}] Error processing renegotiation: ${err}. Allowing to continue.`);
}
}
});
}
// Now set up piping
socket.pipe(targetSocket);
targetSocket.pipe(socket);
socket.resume(); // Resume the socket after piping is established
@ -848,121 +975,8 @@ export class PortProxy {
record.pendingData = [];
record.pendingDataSize = 0;
// Add the renegotiation listener for SNI validation
if (serverName) {
// This listener will check for TLS renegotiation attempts
// Note: We don't need to explicitly forward the renegotiation packets
// since socket.pipe(targetSocket) is already set up earlier and handles that
socket.on('data', (renegChunk: Buffer) => {
if (renegChunk.length > 0 && renegChunk.readUInt8(0) === 22) {
try {
// Try to extract SNI from potential renegotiation
const newSNI = extractSNI(renegChunk, this.settings.enableTlsDebugLogging);
// IMPORTANT: If we can't extract an SNI from renegotiation, we MUST allow it through
// Otherwise valid renegotiations that don't explicitly repeat the SNI will break
if (newSNI === undefined) {
if (this.settings.enableDetailedLogging) {
console.log(
`[${connectionId}] Rehandshake detected without SNI, allowing it through.`
);
}
// Update the activity timestamp - critical for keeping the connection alive during renegotiation
this.updateActivity(record);
// Let it pass through - this is critical for Chrome's TLS handling
return;
}
// Check if the SNI has changed
if (newSNI && newSNI !== record.lockedDomain) {
// Always check whether the new SNI would be allowed by the EXISTING domain config first
// This ensures we're using the same ruleset that allowed the initial connection
let allowed = false;
// First check if the exact original domain config would allow this new SNI
if (record.domainConfig) {
// Check if the new SNI matches any domain pattern in the original domain config
allowed = record.domainConfig.domains.some(d => plugins.minimatch(newSNI, d));
if (allowed && this.settings.enableDetailedLogging) {
console.log(
`[${connectionId}] Rehandshake with new SNI: ${newSNI} matched existing domain config ` +
`patterns ${record.domainConfig.domains.join(', ')}. Allowing connection reuse.`
);
}
}
// If not allowed by the existing domain config, try to find another domain config
if (!allowed) {
const newDomainConfig = this.settings.domainConfigs.find((config) =>
config.domains.some((d) => plugins.minimatch(newSNI, d))
);
// If we found a matching domain config, check IP rules
if (newDomainConfig) {
const effectiveAllowedIPs = [
...newDomainConfig.allowedIPs,
...(this.settings.defaultAllowedIPs || []),
];
const effectiveBlockedIPs = [
...(newDomainConfig.blockedIPs || []),
...(this.settings.defaultBlockedIPs || []),
];
// Check if the IP is allowed for the new domain
allowed = isGlobIPAllowed(record.remoteIP, effectiveAllowedIPs, effectiveBlockedIPs);
if (allowed && this.settings.enableDetailedLogging) {
console.log(
`[${connectionId}] Rehandshake with new SNI: ${newSNI} (previously ${record.lockedDomain}). ` +
`New domain is allowed by different domain config rules, permitting connection reuse.`
);
}
// Update the domain config reference to the new one
if (allowed) {
record.domainConfig = newDomainConfig;
}
}
}
if (allowed) {
// Update the locked domain to the new domain
record.lockedDomain = newSNI;
if (this.settings.enableDetailedLogging) {
console.log(
`[${connectionId}] Updated locked domain for connection from ${record.remoteIP} to: ${newSNI}`
);
}
} else {
// If we get here, either no matching domain config was found or the IP is not allowed
console.log(
`[${connectionId}] Rehandshake detected with different SNI: ${newSNI} vs locked ${record.lockedDomain}. ` +
`New domain not allowed by any rules. Terminating connection.`
);
this.initiateCleanupOnce(record, 'sni_mismatch');
}
} else if (newSNI) {
if (this.settings.enableDetailedLogging) {
console.log(
`[${connectionId}] Rehandshake detected with same SNI: ${newSNI}. Allowing.`
);
}
// Update the activity timestamp
this.updateActivity(record);
}
} catch (err) {
// Always allow the renegotiation to continue if we encounter an error
// This ensures Chrome can complete its TLS renegotiation
console.log(
`[${connectionId}] Error processing potential renegotiation: ${err}. Allowing connection to continue.`
);
// Update the activity timestamp even on error
this.updateActivity(record);
}
}
});
}
// Renegotiation detection is now handled before piping is established
// This ensures the data listener receives all packets properly
// Set connection timeout with simpler logic
if (record.cleanupTimer) {
@ -1692,6 +1706,12 @@ export class PortProxy {
// Save domain config in connection record
connectionRecord.domainConfig = domainConfig;
// Always set the lockedDomain, even for non-SNI connections
if (serverName) {
connectionRecord.lockedDomain = serverName;
console.log(`[${connectionId}] Locked connection to domain: ${serverName}`);
}
// IP validation is skipped if allowedIPs is empty
if (domainConfig) {