fix(port-manager, certificate-manager): Improve port binding and ACME challenge route integration in SmartProxy
This commit is contained in:
		| @@ -3,6 +3,6 @@ | ||||
|  */ | ||||
| export const commitinfo = { | ||||
|   name: '@push.rocks/smartproxy', | ||||
|   version: '19.3.12', | ||||
|   version: '19.3.13', | ||||
|   description: 'A powerful proxy package with unified route-based configuration for high traffic management. Features include SSL/TLS support, flexible routing patterns, WebSocket handling, advanced security options, and automatic ACME certificate management.' | ||||
| } | ||||
|   | ||||
| @@ -416,6 +416,33 @@ export class SmartCertManager { | ||||
|     if (!this.challengeRoute) { | ||||
|       throw new Error('Challenge route not initialized'); | ||||
|     } | ||||
|  | ||||
|     // Get the challenge port | ||||
|     const challengePort = this.globalAcmeDefaults?.port || 80; | ||||
|      | ||||
|     // Check if any existing routes are already using this port | ||||
|     const portInUseByRoutes = this.routes.some(route => { | ||||
|       const routePorts = Array.isArray(route.match.ports) ? route.match.ports : [route.match.ports]; | ||||
|       return routePorts.some(p => { | ||||
|         // Handle both number and port range objects | ||||
|         if (typeof p === 'number') { | ||||
|           return p === challengePort; | ||||
|         } else if (typeof p === 'object' && 'from' in p && 'to' in p) { | ||||
|           // Port range case - check if challengePort is in range | ||||
|           return challengePort >= p.from && challengePort <= p.to; | ||||
|         } | ||||
|         return false; | ||||
|       }); | ||||
|     }); | ||||
|      | ||||
|     if (portInUseByRoutes) { | ||||
|       logger.log('info', `Port ${challengePort} is already used by another route, merging ACME challenge route`, { | ||||
|         port: challengePort, | ||||
|         component: 'certificate-manager' | ||||
|       }); | ||||
|     } | ||||
|      | ||||
|     // Add the challenge route | ||||
|     const challengeRoute = this.challengeRoute; | ||||
|      | ||||
|     try { | ||||
| @@ -430,10 +457,27 @@ export class SmartCertManager { | ||||
|        | ||||
|       logger.log('info', 'ACME challenge route successfully added', { component: 'certificate-manager' }); | ||||
|     } catch (error) { | ||||
|       logger.log('error', `Failed to add challenge route: ${error.message}`, { error: error.message, component: 'certificate-manager' }); | ||||
|       // Handle specific EADDRINUSE errors differently based on whether it's an internal conflict | ||||
|       if ((error as any).code === 'EADDRINUSE') { | ||||
|         throw new Error(`Port ${this.globalAcmeDefaults?.port || 80} is already in use for ACME challenges`); | ||||
|         logger.log('error', `Failed to add challenge route on port ${challengePort}: ${error.message}`, {  | ||||
|           error: error.message,  | ||||
|           port: challengePort, | ||||
|           component: 'certificate-manager' | ||||
|         }); | ||||
|          | ||||
|         // Provide a more informative error message | ||||
|         throw new Error( | ||||
|           `Port ${challengePort} is already in use. ` +  | ||||
|           `If it's in use by an external process, configure a different port in the ACME settings. ` + | ||||
|           `If it's in use by SmartProxy, there may be a route configuration issue.` | ||||
|         ); | ||||
|       } | ||||
|        | ||||
|       // Log and rethrow other errors | ||||
|       logger.log('error', `Failed to add challenge route: ${error.message}`, {  | ||||
|         error: error.message,  | ||||
|         component: 'certificate-manager'  | ||||
|       }); | ||||
|       throw error; | ||||
|     } | ||||
|   } | ||||
|   | ||||
| @@ -1,6 +1,7 @@ | ||||
| import * as plugins from '../../plugins.js'; | ||||
| import type { ISmartProxyOptions } from './models/interfaces.js'; | ||||
| import { RouteConnectionHandler } from './route-connection-handler.js'; | ||||
| import { logger } from '../../core/utils/logger.js'; | ||||
|  | ||||
| /** | ||||
|  * PortManager handles the dynamic creation and removal of port listeners | ||||
| @@ -8,12 +9,17 @@ import { RouteConnectionHandler } from './route-connection-handler.js'; | ||||
|  * This class provides methods to add and remove listening ports at runtime, | ||||
|  * allowing SmartProxy to adapt to configuration changes without requiring | ||||
|  * a full restart. | ||||
|  *  | ||||
|  * It includes a reference counting system to track how many routes are using | ||||
|  * each port, so ports can be automatically released when they are no longer needed. | ||||
|  */ | ||||
| export class PortManager { | ||||
|   private servers: Map<number, plugins.net.Server> = new Map(); | ||||
|   private settings: ISmartProxyOptions; | ||||
|   private routeConnectionHandler: RouteConnectionHandler; | ||||
|   private isShuttingDown: boolean = false; | ||||
|   // Track how many routes are using each port | ||||
|   private portRefCounts: Map<number, number> = new Map(); | ||||
|  | ||||
|   /** | ||||
|    * Create a new PortManager | ||||
| @@ -38,10 +44,18 @@ export class PortManager { | ||||
|   public async addPort(port: number): Promise<void> { | ||||
|     // Check if we're already listening on this port | ||||
|     if (this.servers.has(port)) { | ||||
|       console.log(`PortManager: Already listening on port ${port}`); | ||||
|       // Port is already bound, just increment the reference count | ||||
|       this.incrementPortRefCount(port); | ||||
|       logger.log('debug', `PortManager: Port ${port} is already bound by SmartProxy, reusing binding`, {  | ||||
|         port, | ||||
|         component: 'port-manager' | ||||
|       }); | ||||
|       return; | ||||
|     } | ||||
|  | ||||
|     // Initialize reference count for new port | ||||
|     this.portRefCounts.set(port, 1); | ||||
|  | ||||
|     // Create a server for this port | ||||
|     const server = plugins.net.createServer((socket) => { | ||||
|       // Check if shutting down | ||||
| @@ -54,24 +68,56 @@ export class PortManager { | ||||
|       // Delegate to route connection handler | ||||
|       this.routeConnectionHandler.handleConnection(socket); | ||||
|     }).on('error', (err: Error) => { | ||||
|       console.log(`Server Error on port ${port}: ${err.message}`); | ||||
|       logger.log('error', `Server Error on port ${port}: ${err.message}`, { | ||||
|         port, | ||||
|         error: err.message, | ||||
|         component: 'port-manager' | ||||
|       }); | ||||
|     }); | ||||
|      | ||||
|     // Start listening on the port | ||||
|     return new Promise<void>((resolve, reject) => { | ||||
|       server.listen(port, () => { | ||||
|         const isHttpProxyPort = this.settings.useHttpProxy?.includes(port); | ||||
|         console.log( | ||||
|           `SmartProxy -> OK: Now listening on port ${port}${ | ||||
|             isHttpProxyPort ? ' (HttpProxy forwarding enabled)' : '' | ||||
|           }` | ||||
|         ); | ||||
|         logger.log('info', `SmartProxy -> OK: Now listening on port ${port}${ | ||||
|           isHttpProxyPort ? ' (HttpProxy forwarding enabled)' : '' | ||||
|         }`, { | ||||
|           port, | ||||
|           isHttpProxyPort: !!isHttpProxyPort, | ||||
|           component: 'port-manager' | ||||
|         }); | ||||
|          | ||||
|         // Store the server reference | ||||
|         this.servers.set(port, server); | ||||
|         resolve(); | ||||
|       }).on('error', (err) => { | ||||
|         console.log(`Failed to listen on port ${port}: ${err.message}`); | ||||
|         // Check if this is an external conflict | ||||
|         const { isConflict, isExternal } = this.isPortConflict(err); | ||||
|          | ||||
|         if (isConflict && !isExternal) { | ||||
|           // This is an internal conflict (port already bound by SmartProxy) | ||||
|           // This shouldn't normally happen because we check servers.has(port) above | ||||
|           logger.log('warn', `Port ${port} binding conflict: already in use by SmartProxy`, { | ||||
|             port, | ||||
|             component: 'port-manager' | ||||
|           }); | ||||
|           // Still increment reference count to maintain tracking | ||||
|           this.incrementPortRefCount(port); | ||||
|           resolve(); | ||||
|           return; | ||||
|         } | ||||
|          | ||||
|         // Log the error and propagate it | ||||
|         logger.log('error', `Failed to listen on port ${port}: ${err.message}`, { | ||||
|           port, | ||||
|           error: err.message, | ||||
|           code: (err as any).code, | ||||
|           component: 'port-manager' | ||||
|         }); | ||||
|          | ||||
|         // Clean up reference count since binding failed | ||||
|         this.portRefCounts.delete(port); | ||||
|          | ||||
|         reject(err); | ||||
|       }); | ||||
|     }); | ||||
| @@ -84,10 +130,28 @@ export class PortManager { | ||||
|    * @returns Promise that resolves when the server is closed | ||||
|    */ | ||||
|   public async removePort(port: number): Promise<void> { | ||||
|     // Decrement the reference count first | ||||
|     const newRefCount = this.decrementPortRefCount(port); | ||||
|      | ||||
|     // If there are still references to this port, keep it open | ||||
|     if (newRefCount > 0) { | ||||
|       logger.log('debug', `PortManager: Port ${port} still has ${newRefCount} references, keeping open`, { | ||||
|         port, | ||||
|         refCount: newRefCount, | ||||
|         component: 'port-manager' | ||||
|       }); | ||||
|       return; | ||||
|     } | ||||
|  | ||||
|     // Get the server for this port | ||||
|     const server = this.servers.get(port); | ||||
|     if (!server) { | ||||
|       console.log(`PortManager: Not listening on port ${port}`); | ||||
|       logger.log('warn', `PortManager: Not listening on port ${port}`, { | ||||
|         port, | ||||
|         component: 'port-manager' | ||||
|       }); | ||||
|       // Ensure reference count is reset | ||||
|       this.portRefCounts.delete(port); | ||||
|       return; | ||||
|     } | ||||
|  | ||||
| @@ -95,13 +159,21 @@ export class PortManager { | ||||
|     return new Promise<void>((resolve) => { | ||||
|       server.close((err) => { | ||||
|         if (err) { | ||||
|           console.log(`Error closing server on port ${port}: ${err.message}`); | ||||
|           logger.log('error', `Error closing server on port ${port}: ${err.message}`, { | ||||
|             port, | ||||
|             error: err.message, | ||||
|             component: 'port-manager' | ||||
|           }); | ||||
|         } else { | ||||
|           console.log(`SmartProxy -> Stopped listening on port ${port}`); | ||||
|           logger.log('info', `SmartProxy -> Stopped listening on port ${port}`, { | ||||
|             port, | ||||
|             component: 'port-manager' | ||||
|           }); | ||||
|         } | ||||
|          | ||||
|         // Remove the server reference | ||||
|         // Remove the server reference and clean up reference counting | ||||
|         this.servers.delete(port); | ||||
|         this.portRefCounts.delete(port); | ||||
|         resolve(); | ||||
|       }); | ||||
|     }); | ||||
| @@ -192,4 +264,89 @@ export class PortManager { | ||||
|   public getServers(): Map<number, plugins.net.Server> { | ||||
|     return new Map(this.servers); | ||||
|   } | ||||
|  | ||||
|   /** | ||||
|    * Check if a port is bound by this SmartProxy instance | ||||
|    *  | ||||
|    * @param port The port number to check | ||||
|    * @returns True if the port is currently bound by SmartProxy | ||||
|    */ | ||||
|   public isPortBoundBySmartProxy(port: number): boolean { | ||||
|     return this.servers.has(port); | ||||
|   } | ||||
|  | ||||
|   /** | ||||
|    * Get the current reference count for a port | ||||
|    *  | ||||
|    * @param port The port number to check | ||||
|    * @returns The number of routes using this port, 0 if none | ||||
|    */ | ||||
|   public getPortRefCount(port: number): number { | ||||
|     return this.portRefCounts.get(port) || 0; | ||||
|   } | ||||
|  | ||||
|   /** | ||||
|    * Increment the reference count for a port | ||||
|    *  | ||||
|    * @param port The port number to increment | ||||
|    * @returns The new reference count | ||||
|    */ | ||||
|   public incrementPortRefCount(port: number): number { | ||||
|     const currentCount = this.portRefCounts.get(port) || 0; | ||||
|     const newCount = currentCount + 1; | ||||
|     this.portRefCounts.set(port, newCount); | ||||
|      | ||||
|     logger.log('debug', `Port ${port} reference count increased to ${newCount}`, {  | ||||
|       port,  | ||||
|       refCount: newCount, | ||||
|       component: 'port-manager'  | ||||
|     }); | ||||
|      | ||||
|     return newCount; | ||||
|   } | ||||
|  | ||||
|   /** | ||||
|    * Decrement the reference count for a port | ||||
|    *  | ||||
|    * @param port The port number to decrement | ||||
|    * @returns The new reference count | ||||
|    */ | ||||
|   public decrementPortRefCount(port: number): number { | ||||
|     const currentCount = this.portRefCounts.get(port) || 0; | ||||
|      | ||||
|     if (currentCount <= 0) { | ||||
|       logger.log('warn', `Attempted to decrement reference count for port ${port} below zero`, {  | ||||
|         port, | ||||
|         component: 'port-manager' | ||||
|       }); | ||||
|       return 0; | ||||
|     } | ||||
|      | ||||
|     const newCount = currentCount - 1; | ||||
|     this.portRefCounts.set(port, newCount); | ||||
|      | ||||
|     logger.log('debug', `Port ${port} reference count decreased to ${newCount}`, {  | ||||
|       port,  | ||||
|       refCount: newCount, | ||||
|       component: 'port-manager' | ||||
|     }); | ||||
|      | ||||
|     return newCount; | ||||
|   } | ||||
|  | ||||
|   /** | ||||
|    * Determine if a port binding error is due to an external or internal conflict | ||||
|    *  | ||||
|    * @param error The error object from a failed port binding | ||||
|    * @returns Object indicating if this is a conflict and if it's external | ||||
|    */ | ||||
|   private isPortConflict(error: any): { isConflict: boolean; isExternal: boolean } { | ||||
|     if (error.code !== 'EADDRINUSE') { | ||||
|       return { isConflict: false, isExternal: false }; | ||||
|     } | ||||
|      | ||||
|     // Check if we already have this port | ||||
|     const isBoundInternally = this.servers.has(Number(error.port)); | ||||
|     return { isConflict: true, isExternal: !isBoundInternally }; | ||||
|   } | ||||
| } | ||||
| @@ -64,6 +64,9 @@ export class SmartProxy extends plugins.EventEmitter { | ||||
|   private routeUpdateLock: any = null; // Will be initialized as AsyncMutex | ||||
|   private acmeStateManager: AcmeStateManager; | ||||
|    | ||||
|   // Track port usage across route updates | ||||
|   private portUsageMap: Map<number, Set<string>> = new Map(); | ||||
|    | ||||
|   /** | ||||
|    * Constructor for SmartProxy | ||||
|    * | ||||
| @@ -342,6 +345,16 @@ export class SmartProxy extends plugins.EventEmitter { | ||||
|     // Get listening ports from RouteManager | ||||
|     const listeningPorts = this.routeManager.getListeningPorts(); | ||||
|  | ||||
|     // Initialize port usage tracking | ||||
|     this.portUsageMap = this.updatePortUsageMap(this.settings.routes); | ||||
|      | ||||
|     // Log port usage for startup | ||||
|     logger.log('info', `SmartProxy starting with ${listeningPorts.length} ports: ${listeningPorts.join(', ')}`, { | ||||
|       portCount: listeningPorts.length, | ||||
|       ports: listeningPorts, | ||||
|       component: 'smart-proxy' | ||||
|     }); | ||||
|  | ||||
|     // Provision NFTables rules for routes that use NFTables | ||||
|     for (const route of this.settings.routes) { | ||||
|       if (route.action.forwardingEngine === 'nftables') { | ||||
| @@ -548,6 +561,18 @@ export class SmartProxy extends plugins.EventEmitter { | ||||
|     return this.routeUpdateLock.runExclusive(async () => { | ||||
|       logger.log('info', `Updating routes (${newRoutes.length} routes)`, { routeCount: newRoutes.length, component: 'route-manager' }); | ||||
|  | ||||
|       // Track port usage before and after updates | ||||
|       const oldPortUsage = this.updatePortUsageMap(this.settings.routes); | ||||
|       const newPortUsage = this.updatePortUsageMap(newRoutes); | ||||
|        | ||||
|       // Find orphaned ports - ports that no longer have any routes | ||||
|       const orphanedPorts = this.findOrphanedPorts(oldPortUsage, newPortUsage); | ||||
|        | ||||
|       // Find new ports that need binding | ||||
|       const currentPorts = new Set(this.portManager.getListeningPorts()); | ||||
|       const newPortsSet = new Set(newPortUsage.keys()); | ||||
|       const newBindingPorts = Array.from(newPortsSet).filter(p => !currentPorts.has(p)); | ||||
|  | ||||
|       // Get existing routes that use NFTables | ||||
|       const oldNfTablesRoutes = this.settings.routes.filter( | ||||
|         r => r.action.forwardingEngine === 'nftables' | ||||
| @@ -584,14 +609,29 @@ export class SmartProxy extends plugins.EventEmitter { | ||||
|       // Update routes in RouteManager | ||||
|       this.routeManager.updateRoutes(newRoutes); | ||||
|  | ||||
|       // Get the new set of required ports | ||||
|       const requiredPorts = this.routeManager.getListeningPorts(); | ||||
|  | ||||
|       // Update port listeners to match the new configuration | ||||
|       await this.portManager.updatePorts(requiredPorts); | ||||
|       // Release orphaned ports first | ||||
|       if (orphanedPorts.length > 0) { | ||||
|         logger.log('info', `Releasing ${orphanedPorts.length} orphaned ports: ${orphanedPorts.join(', ')}`, {  | ||||
|           ports: orphanedPorts, | ||||
|           component: 'smart-proxy' | ||||
|         }); | ||||
|         await this.portManager.removePorts(orphanedPorts); | ||||
|       } | ||||
|        | ||||
|       // Add new ports | ||||
|       if (newBindingPorts.length > 0) { | ||||
|         logger.log('info', `Binding to ${newBindingPorts.length} new ports: ${newBindingPorts.join(', ')}`, {  | ||||
|           ports: newBindingPorts, | ||||
|           component: 'smart-proxy' | ||||
|         }); | ||||
|         await this.portManager.addPorts(newBindingPorts); | ||||
|       } | ||||
|        | ||||
|       // Update settings with the new routes | ||||
|       this.settings.routes = newRoutes; | ||||
|        | ||||
|       // Save the new port usage map for future reference | ||||
|       this.portUsageMap = newPortUsage; | ||||
|  | ||||
|       // If HttpProxy is initialized, resync the configurations | ||||
|       if (this.httpProxyBridge.getHttpProxy()) { | ||||
| @@ -637,6 +677,78 @@ export class SmartProxy extends plugins.EventEmitter { | ||||
|      | ||||
|     await this.certManager.provisionCertificate(route); | ||||
|   } | ||||
|  | ||||
|   /** | ||||
|    * Update the port usage map based on the provided routes | ||||
|    *  | ||||
|    * This tracks which ports are used by which routes, allowing us to | ||||
|    * detect when a port is no longer needed and can be released. | ||||
|    */ | ||||
|   private updatePortUsageMap(routes: IRouteConfig[]): Map<number, Set<string>> { | ||||
|     // Reset the usage map | ||||
|     const portUsage = new Map<number, Set<string>>(); | ||||
|      | ||||
|     for (const route of routes) { | ||||
|       // Get the ports for this route | ||||
|       const portsConfig = Array.isArray(route.match.ports)  | ||||
|         ? route.match.ports  | ||||
|         : [route.match.ports]; | ||||
|        | ||||
|       // Expand port range objects to individual port numbers | ||||
|       const expandedPorts: number[] = []; | ||||
|       for (const portConfig of portsConfig) { | ||||
|         if (typeof portConfig === 'number') { | ||||
|           expandedPorts.push(portConfig); | ||||
|         } else if (typeof portConfig === 'object' && 'from' in portConfig && 'to' in portConfig) { | ||||
|           // Expand the port range | ||||
|           for (let p = portConfig.from; p <= portConfig.to; p++) { | ||||
|             expandedPorts.push(p); | ||||
|           } | ||||
|         } | ||||
|       } | ||||
|        | ||||
|       // Use route name if available, otherwise generate a unique ID | ||||
|       const routeName = route.name || `unnamed_${Math.random().toString(36).substring(2, 9)}`; | ||||
|        | ||||
|       // Add each port to the usage map | ||||
|       for (const port of expandedPorts) { | ||||
|         if (!portUsage.has(port)) { | ||||
|           portUsage.set(port, new Set()); | ||||
|         } | ||||
|         portUsage.get(port)!.add(routeName); | ||||
|       } | ||||
|     } | ||||
|      | ||||
|     // Log port usage for debugging | ||||
|     for (const [port, routes] of portUsage.entries()) { | ||||
|       logger.log('debug', `Port ${port} is used by ${routes.size} routes: ${Array.from(routes).join(', ')}`, { | ||||
|         port, | ||||
|         routeCount: routes.size, | ||||
|         component: 'smart-proxy' | ||||
|       }); | ||||
|     } | ||||
|      | ||||
|     return portUsage; | ||||
|   } | ||||
|    | ||||
|   /** | ||||
|    * Find ports that have no routes in the new configuration | ||||
|    */ | ||||
|   private findOrphanedPorts(oldUsage: Map<number, Set<string>>, newUsage: Map<number, Set<string>>): number[] { | ||||
|     const orphanedPorts: number[] = []; | ||||
|      | ||||
|     for (const [port, routes] of oldUsage.entries()) { | ||||
|       if (!newUsage.has(port) || newUsage.get(port)!.size === 0) { | ||||
|         orphanedPorts.push(port); | ||||
|         logger.log('info', `Port ${port} no longer has any associated routes, will be released`, { | ||||
|           port, | ||||
|           component: 'smart-proxy' | ||||
|         }); | ||||
|       } | ||||
|     } | ||||
|      | ||||
|     return orphanedPorts; | ||||
|   } | ||||
|    | ||||
|   /** | ||||
|    * Force renewal of a certificate | ||||
|   | ||||
		Reference in New Issue
	
	Block a user