diff --git a/changelog.md b/changelog.md index b8f0c1e..42af299 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,33 @@ # Changelog +## 2025-05-19 - 19.3.10 - fix(certificate-manager, smart-proxy) +Fix race condition in ACME certificate provisioning and refactor certificate manager initialization to defer provisioning until after port listeners are active + +- Removed superfluous provisionCertificatesAfterPortsReady method +- Made provisionAllCertificates public so that SmartProxy.start() calls it after ports are listening +- Updated SmartProxy.start() to wait for port setup (via PortManager) before triggering certificate provisioning +- Improved ACME HTTP-01 challenge timing so that port 80 (or configured ACME port) is guaranteed to be ready +- Updated documentation (changelog and Acme timing docs) and tests to reflect the change + +## 2025-05-19 - 19.3.10 - refactor(certificate-manager, smart-proxy) +Simplify certificate provisioning code by removing unnecessary wrapper method + +- Removed superfluous SmartCertManager.provisionCertificatesAfterPortsReady() method +- Made SmartCertManager.provisionAllCertificates() public instead +- Updated SmartProxy.start() to call provisionAllCertificates() directly +- Updated documentation and tests to reflect the change +- No functional changes, just code simplification + +## 2025-05-19 - 19.3.9 - fix(certificate-manager, smart-proxy) +Fix ACME certificate provisioning timing to ensure ports are listening first + +- Fixed race condition where certificate provisioning would start before ports were listening +- Modified SmartCertManager.initialize() to defer certificate provisioning +- Added SmartCertManager.provisionCertificatesAfterPortsReady() for delayed provisioning +- Updated SmartProxy.start() to call certificate provisioning after ports are ready +- This fix prevents ACME HTTP-01 challenges from failing due to port 80 not being ready +- Added test/test.acme-timing-simple.ts to verify the timing synchronization + ## 2025-05-19 - 19.3.9 - fix(route-connection-handler) Forward non-TLS connections on HttpProxy ports to fix ACME HTTP-01 challenge handling diff --git a/docs/acme-timing-fix.md b/docs/acme-timing-fix.md new file mode 100644 index 0000000..28f1eaf --- /dev/null +++ b/docs/acme-timing-fix.md @@ -0,0 +1,100 @@ +# ACME Certificate Provisioning Timing Fix (v19.3.9) + +## Problem Description + +In SmartProxy v19.3.8 and earlier, ACME certificate provisioning would start immediately during SmartProxy initialization, before the required ports were actually listening. This caused ACME HTTP-01 challenges to fail because the challenge port (typically port 80) was not ready to accept connections when Let's Encrypt tried to validate the challenge. + +## Root Cause + +The certificate manager was initialized and immediately started provisioning certificates as part of the SmartProxy startup sequence: + +1. SmartProxy.start() called +2. Certificate manager initialized +3. Certificate provisioning started immediately (including ACME challenges) +4. Port listeners started afterwards +5. ACME challenges would fail because port 80 wasn't listening yet + +This race condition meant that when Let's Encrypt tried to connect to port 80 to validate the HTTP-01 challenge, the connection would be refused. + +## Solution + +The fix defers certificate provisioning until after all ports are listening and ready: + +### Changes to SmartCertManager + +```typescript +// Modified initialize() to skip automatic provisioning +public async initialize(): Promise { + // ... initialization code ... + + // Skip automatic certificate provisioning during initialization + console.log('Certificate manager initialized. Deferring certificate provisioning until after ports are listening.'); + + // Start renewal timer + this.startRenewalTimer(); +} + +// Made provisionAllCertificates public to allow direct calling after ports are ready +public async provisionAllCertificates(): Promise { + // ... certificate provisioning code ... +} +``` + +### Changes to SmartProxy + +```typescript +public async start() { + // ... initialization code ... + + // Start port listeners using the PortManager + await this.portManager.addPorts(listeningPorts); + + // Now that ports are listening, provision any required certificates + if (this.certManager) { + console.log('Starting certificate provisioning now that ports are ready'); + await this.certManager.provisionAllCertificates(); + } + + // ... rest of startup code ... +} +``` + +## Timing Sequence + +### Before (v19.3.8 and earlier) +1. Initialize certificate manager +2. Start ACME provisioning immediately +3. ACME challenge fails (port not ready) +4. Start port listeners +5. Port 80 now listening (too late) + +### After (v19.3.9) +1. Initialize certificate manager (provisioning deferred) +2. Start port listeners +3. Port 80 now listening +4. Start ACME provisioning +5. ACME challenge succeeds + +## Configuration + +No configuration changes are required. The timing fix is automatic and transparent to users. + +## Testing + +The fix is verified by the test in `test/test.acme-timing-simple.ts` which ensures: + +1. Certificate manager is initialized first +2. Ports start listening +3. Certificate provisioning happens only after ports are ready + +## Impact + +This fix ensures that: +- ACME HTTP-01 challenges succeed on first attempt +- No more "connection refused" errors during certificate provisioning +- Certificate acquisition is more reliable +- No manual retries needed for failed challenges + +## Migration + +Simply update to SmartProxy v19.3.9 or later. The fix is backward compatible and requires no changes to existing code or configuration. \ No newline at end of file diff --git a/readme.hints.md b/readme.hints.md index 3371600..25bbe51 100644 --- a/readme.hints.md +++ b/readme.hints.md @@ -132,4 +132,27 @@ const proxy = new SmartProxy({ // Your routes here ] }); -``` \ No newline at end of file +``` + +## ACME Certificate Provisioning Timing Fix (v19.3.9) + +### Issue +Certificate provisioning would start before ports were listening, causing ACME HTTP-01 challenges to fail with connection refused errors. + +### Root Cause +SmartProxy initialization sequence: +1. Certificate manager initialized → immediately starts provisioning +2. Ports start listening (too late for ACME challenges) + +### Solution +Deferred certificate provisioning until after ports are ready: +```typescript +// SmartCertManager.initialize() now skips automatic provisioning +// SmartProxy.start() calls provisionAllCertificates() directly after ports are listening +``` + +### Test Coverage +- `test/test.acme-timing-simple.ts` - Verifies proper timing sequence + +### Migration +Update to v19.3.9+, no configuration changes needed. \ No newline at end of file diff --git a/readme.md b/readme.md index 4dbbde8..957e545 100644 --- a/readme.md +++ b/readme.md @@ -1481,8 +1481,11 @@ HttpProxy now supports full route-based configuration including: - Enable `enableDetailedLogging` or `enableTlsDebugLogging` for debugging ### ACME HTTP-01 Challenges -- If ACME HTTP-01 challenges fail on port 80, ensure port 80 is included in `useHttpProxy` -- Since v19.3.8, non-TLS connections on ports listed in `useHttpProxy` are properly forwarded to HttpProxy +- If ACME HTTP-01 challenges fail, ensure: + 1. Port 80 (or configured ACME port) is included in `useHttpProxy` + 2. You're using SmartProxy v19.3.9+ for proper timing (ports must be listening before provisioning) +- Since v19.3.8: Non-TLS connections on ports listed in `useHttpProxy` are properly forwarded to HttpProxy +- Since v19.3.9: Certificate provisioning waits for ports to be ready before starting ACME challenges - Example configuration for ACME on port 80: ```typescript const proxy = new SmartProxy({ @@ -1495,6 +1498,9 @@ HttpProxy now supports full route-based configuration including: routes: [/* your routes */] }); ``` +- Common issues: + - "Connection refused" during challenges → Update to v19.3.9+ for timing fix + - HTTP requests not parsed → Ensure port is in `useHttpProxy` array ### NFTables Integration - Ensure NFTables is installed: `apt install nftables` or `yum install nftables` diff --git a/test/test.acme-http01-challenge.ts b/test/test.acme-http01-challenge.ts new file mode 100644 index 0000000..7b5682a --- /dev/null +++ b/test/test.acme-http01-challenge.ts @@ -0,0 +1,174 @@ +import { tap, expect } from '@git.zone/tstest/tapbundle'; +import { SmartProxy } from '../ts/index.js'; +import * as net from 'net'; + +// Test that HTTP-01 challenges are properly processed when the initial data arrives +tap.test('should correctly handle HTTP-01 challenge requests with initial data chunk', async (tapTest) => { + // Prepare test data + const challengeToken = 'test-acme-http01-challenge-token'; + const challengeResponse = 'mock-response-for-challenge'; + const challengePath = `/.well-known/acme-challenge/${challengeToken}`; + + // Create a handler function that responds to ACME challenges + const acmeHandler = (context: any) => { + // Log request details for debugging + console.log(`Received request: ${context.method} ${context.path}`); + + // Check if this is an ACME challenge request + if (context.path.startsWith('/.well-known/acme-challenge/')) { + const token = context.path.substring('/.well-known/acme-challenge/'.length); + + // If the token matches our test token, return the response + if (token === challengeToken) { + return { + status: 200, + headers: { + 'Content-Type': 'text/plain' + }, + body: challengeResponse + }; + } + } + + // For any other requests, return 404 + return { + status: 404, + headers: { + 'Content-Type': 'text/plain' + }, + body: 'Not found' + }; + }; + + // Create a proxy with the ACME challenge route + const proxy = new SmartProxy({ + routes: [{ + name: 'acme-challenge-route', + match: { + ports: 8080, + paths: ['/.well-known/acme-challenge/*'] + }, + action: { + type: 'static', + handler: acmeHandler + } + }] + }); + + await proxy.start(); + + // Create a client to test the HTTP-01 challenge + const testClient = new net.Socket(); + let responseData = ''; + + // Set up client handlers + testClient.on('data', (data) => { + responseData += data.toString(); + }); + + // Connect to the proxy and send the HTTP-01 challenge request + await new Promise((resolve, reject) => { + testClient.connect(8080, 'localhost', () => { + // Send HTTP request for the challenge token + testClient.write( + `GET ${challengePath} HTTP/1.1\r\n` + + 'Host: test.example.com\r\n' + + 'User-Agent: ACME Challenge Test\r\n' + + 'Accept: */*\r\n' + + '\r\n' + ); + resolve(); + }); + + testClient.on('error', reject); + }); + + // Wait for the response + await new Promise(resolve => setTimeout(resolve, 100)); + + // Verify that we received a valid HTTP response with the challenge token + expect(responseData).toContain('HTTP/1.1 200'); + expect(responseData).toContain('Content-Type: text/plain'); + expect(responseData).toContain(challengeResponse); + + // Cleanup + testClient.destroy(); + await proxy.stop(); +}); + +// Test that non-existent challenge tokens return 404 +tap.test('should return 404 for non-existent challenge tokens', async (tapTest) => { + // Create a handler function that behaves like a real ACME handler + const acmeHandler = (context: any) => { + if (context.path.startsWith('/.well-known/acme-challenge/')) { + const token = context.path.substring('/.well-known/acme-challenge/'.length); + // In this test, we only recognize one specific token + if (token === 'valid-token') { + return { + status: 200, + headers: { 'Content-Type': 'text/plain' }, + body: 'valid-response' + }; + } + } + + // For all other paths or unrecognized tokens, return 404 + return { + status: 404, + headers: { 'Content-Type': 'text/plain' }, + body: 'Not found' + }; + }; + + // Create a proxy with the ACME challenge route + const proxy = new SmartProxy({ + routes: [{ + name: 'acme-challenge-route', + match: { + ports: 8081, + paths: ['/.well-known/acme-challenge/*'] + }, + action: { + type: 'static', + handler: acmeHandler + } + }] + }); + + await proxy.start(); + + // Create a client to test the invalid challenge request + const testClient = new net.Socket(); + let responseData = ''; + + testClient.on('data', (data) => { + responseData += data.toString(); + }); + + // Connect and send a request for a non-existent token + await new Promise((resolve, reject) => { + testClient.connect(8081, 'localhost', () => { + testClient.write( + 'GET /.well-known/acme-challenge/invalid-token HTTP/1.1\r\n' + + 'Host: test.example.com\r\n' + + '\r\n' + ); + resolve(); + }); + + testClient.on('error', reject); + }); + + // Wait for the response + await new Promise(resolve => setTimeout(resolve, 100)); + + // Verify we got a 404 Not Found + expect(responseData).toContain('HTTP/1.1 404'); + expect(responseData).toContain('Not found'); + + // Cleanup + testClient.destroy(); + await proxy.stop(); +}); + +tap.start(); \ No newline at end of file diff --git a/test/test.acme-timing-simple.ts b/test/test.acme-timing-simple.ts new file mode 100644 index 0000000..095ace3 --- /dev/null +++ b/test/test.acme-timing-simple.ts @@ -0,0 +1,103 @@ +import { tap, expect } from '@git.zone/tstest/tapbundle'; +import { SmartProxy } from '../ts/index.js'; + +// Test that certificate provisioning is deferred until after ports are listening +tap.test('should defer certificate provisioning until ports are ready', async (tapTest) => { + // Track when operations happen + let portsListening = false; + let certProvisioningStarted = false; + let operationOrder: string[] = []; + + // Create proxy with certificate route but without real ACME + const proxy = new SmartProxy({ + routes: [{ + name: 'test-route', + match: { + ports: 8443, + domains: ['test.local'] + }, + action: { + type: 'forward', + target: { host: 'localhost', port: 8181 }, + tls: { + mode: 'terminate', + certificate: 'auto', + acme: { + email: 'test@local.dev', + useProduction: false + } + } + } + }] + }); + + // Override the certificate manager creation to avoid real ACME + const originalCreateCertManager = proxy['createCertificateManager']; + proxy['createCertificateManager'] = async function(...args: any[]) { + console.log('Creating mock cert manager'); + operationOrder.push('create-cert-manager'); + const mockCertManager = { + initialize: async () => { + operationOrder.push('cert-manager-init'); + console.log('Mock cert manager initialized'); + }, + provisionAllCertificates: async () => { + operationOrder.push('cert-provisioning'); + certProvisioningStarted = true; + // Check that ports are listening when provisioning starts + if (!portsListening) { + throw new Error('Certificate provisioning started before ports ready!'); + } + console.log('Mock certificate provisioning (ports are ready)'); + }, + stop: async () => {}, + setHttpProxy: () => {}, + setGlobalAcmeDefaults: () => {}, + setAcmeStateManager: () => {}, + setUpdateRoutesCallback: () => {}, + getAcmeOptions: () => ({}), + getState: () => ({ challengeRouteActive: false }) + }; + + // Call initialize immediately as the real createCertificateManager does + await mockCertManager.initialize(); + + return mockCertManager; + }; + + // Track port manager operations + const originalAddPorts = proxy['portManager'].addPorts; + proxy['portManager'].addPorts = async function(ports: number[]) { + operationOrder.push('ports-starting'); + const result = await originalAddPorts.call(this, ports); + operationOrder.push('ports-ready'); + portsListening = true; + console.log('Ports are now listening'); + return result; + }; + + // Start the proxy + await proxy.start(); + + // Log the operation order for debugging + console.log('Operation order:', operationOrder); + + // Verify operations happened in the correct order + expect(operationOrder).toContain('create-cert-manager'); + expect(operationOrder).toContain('cert-manager-init'); + expect(operationOrder).toContain('ports-starting'); + expect(operationOrder).toContain('ports-ready'); + expect(operationOrder).toContain('cert-provisioning'); + + // Verify ports were ready before certificate provisioning + const portsReadyIndex = operationOrder.indexOf('ports-ready'); + const certProvisioningIndex = operationOrder.indexOf('cert-provisioning'); + + expect(portsReadyIndex).toBeLessThan(certProvisioningIndex); + expect(certProvisioningStarted).toEqual(true); + expect(portsListening).toEqual(true); + + await proxy.stop(); +}); + +tap.start(); \ No newline at end of file diff --git a/test/test.acme-timing.ts b/test/test.acme-timing.ts new file mode 100644 index 0000000..9f76d9b --- /dev/null +++ b/test/test.acme-timing.ts @@ -0,0 +1,159 @@ +import { tap, expect } from '@git.zone/tstest/tapbundle'; +import { SmartProxy } from '../ts/index.js'; +import * as net from 'net'; + +// Test that certificate provisioning waits for ports to be ready +tap.test('should defer certificate provisioning until after ports are listening', async (tapTest) => { + // Track the order of operations + const operationLog: string[] = []; + + // Create a mock server to verify ports are listening + let port80Listening = false; + const testServer = net.createServer(() => { + // We don't need to handle connections, just track that we're listening + }); + + // Try to use port 8080 instead of 80 to avoid permission issues in testing + const acmePort = 8080; + + // Create proxy with ACME certificate requirement + const proxy = new SmartProxy({ + useHttpProxy: [acmePort], + httpProxyPort: 8844, + acme: { + email: 'test@example.com', + useProduction: false, + port: acmePort + }, + routes: [{ + name: 'test-acme-route', + match: { + ports: 8443, + domains: ['test.local'] + }, + action: { + type: 'forward', + target: { host: 'localhost', port: 8181 }, + tls: { + mode: 'terminate', + certificate: 'auto', + acme: { + email: 'test@example.com', + useProduction: false + } + } + } + }] + }); + + // Mock some internal methods to track operation order + const originalAddPorts = proxy['portManager'].addPorts; + proxy['portManager'].addPorts = async function(ports: number[]) { + operationLog.push('Starting port listeners'); + const result = await originalAddPorts.call(this, ports); + operationLog.push('Port listeners started'); + port80Listening = true; + return result; + }; + + // Track certificate provisioning + const originalProvisionAll = proxy['certManager'] ? + proxy['certManager']['provisionAllCertificates'] : null; + + if (proxy['certManager']) { + proxy['certManager']['provisionAllCertificates'] = async function() { + operationLog.push('Starting certificate provisioning'); + // Check if port 80 is listening + if (!port80Listening) { + operationLog.push('ERROR: Certificate provisioning started before ports ready'); + } + // Don't actually provision certificates in the test + operationLog.push('Certificate provisioning completed'); + }; + } + + // Start the proxy + await proxy.start(); + + // Verify the order of operations + expect(operationLog).toContain('Starting port listeners'); + expect(operationLog).toContain('Port listeners started'); + expect(operationLog).toContain('Starting certificate provisioning'); + + // Ensure port listeners started before certificate provisioning + const portStartIndex = operationLog.indexOf('Port listeners started'); + const certStartIndex = operationLog.indexOf('Starting certificate provisioning'); + + expect(portStartIndex).toBeLessThan(certStartIndex); + expect(operationLog).not.toContain('ERROR: Certificate provisioning started before ports ready'); + + await proxy.stop(); +}); + +// Test that ACME challenge route is available when certificate is requested +tap.test('should have ACME challenge route ready before certificate provisioning', async (tapTest) => { + let challengeRouteActive = false; + let certificateProvisioningStarted = false; + + const proxy = new SmartProxy({ + useHttpProxy: [8080], + httpProxyPort: 8844, + acme: { + email: 'test@example.com', + useProduction: false, + port: 8080 + }, + routes: [{ + name: 'test-route', + match: { + ports: 8443, + domains: ['test.example.com'] + }, + action: { + type: 'forward', + target: { host: 'localhost', port: 8181 }, + tls: { + mode: 'terminate', + certificate: 'auto' + } + } + }] + }); + + // Mock the certificate manager to track operations + const originalInitialize = proxy['certManager'] ? + proxy['certManager'].initialize : null; + + if (proxy['certManager']) { + const certManager = proxy['certManager']; + + // Track when challenge route is added + const originalAddChallenge = certManager['addChallengeRoute']; + certManager['addChallengeRoute'] = async function() { + await originalAddChallenge.call(this); + challengeRouteActive = true; + }; + + // Track when certificate provisioning starts + const originalProvisionAcme = certManager['provisionAcmeCertificate']; + certManager['provisionAcmeCertificate'] = async function(...args: any[]) { + certificateProvisioningStarted = true; + // Verify challenge route is active + expect(challengeRouteActive).toEqual(true); + // Don't actually provision in test + return; + }; + } + + await proxy.start(); + + // Give it a moment to complete initialization + await new Promise(resolve => setTimeout(resolve, 100)); + + // Verify challenge route was added before any certificate provisioning + expect(challengeRouteActive).toEqual(true); + + await proxy.stop(); +}); + +tap.start(); \ No newline at end of file diff --git a/ts/00_commitinfo_data.ts b/ts/00_commitinfo_data.ts index 38d3f7f..8996eb9 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: '19.3.9', + version: '19.3.10', 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.' } diff --git a/ts/proxies/http-proxy/handlers/static-handler.ts b/ts/proxies/http-proxy/handlers/static-handler.ts index 1b36c5a..d3b2011 100644 --- a/ts/proxies/http-proxy/handlers/static-handler.ts +++ b/ts/proxies/http-proxy/handlers/static-handler.ts @@ -24,7 +24,8 @@ export class StaticHandler { socket: plugins.net.Socket, route: IRouteConfig, context: IStaticHandlerContext, - record: IConnectionRecord + record: IConnectionRecord, + initialChunk?: Buffer ): Promise { const { connectionId, connectionManager, settings } = context; const logger = context.logger || createLogger(settings.logLevel || 'info'); @@ -239,7 +240,16 @@ export class StaticHandler { } }; - // Listen for data + // Process initial chunk if provided + if (initialChunk && initialChunk.length > 0) { + if (settings.enableDetailedLogging) { + logger.info(`[${connectionId}] Processing initial data chunk (${initialChunk.length} bytes)`); + } + // Process the initial chunk immediately + handleHttpData(initialChunk); + } + + // Listen for additional data socket.on('data', handleHttpData); // Ensure cleanup on socket close diff --git a/ts/proxies/smart-proxy/certificate-manager.ts b/ts/proxies/smart-proxy/certificate-manager.ts index b5b3e10..fc8ca58 100644 --- a/ts/proxies/smart-proxy/certificate-manager.ts +++ b/ts/proxies/smart-proxy/certificate-manager.ts @@ -132,8 +132,9 @@ export class SmartCertManager { } } - // Provision certificates for all routes - await this.provisionAllCertificates(); + // Skip automatic certificate provisioning during initialization + // This will be called later after ports are listening + console.log('Certificate manager initialized. Deferring certificate provisioning until after ports are listening.'); // Start renewal timer this.startRenewalTimer(); @@ -142,7 +143,7 @@ export class SmartCertManager { /** * Provision certificates for all routes that need them */ - private async provisionAllCertificates(): Promise { + public async provisionAllCertificates(): Promise { const certRoutes = this.routes.filter(r => r.action.tls?.mode === 'terminate' || r.action.tls?.mode === 'terminate-and-reencrypt' diff --git a/ts/proxies/smart-proxy/route-connection-handler.ts b/ts/proxies/smart-proxy/route-connection-handler.ts index fdb1f34..2499ef3 100644 --- a/ts/proxies/smart-proxy/route-connection-handler.ts +++ b/ts/proxies/smart-proxy/route-connection-handler.ts @@ -352,7 +352,7 @@ export class RouteConnectionHandler { return this.handleBlockAction(socket, record, route); case 'static': - this.handleStaticAction(socket, record, route); + this.handleStaticAction(socket, record, route, initialChunk); return; default: @@ -674,14 +674,15 @@ export class RouteConnectionHandler { private async handleStaticAction( socket: plugins.net.Socket, record: IConnectionRecord, - route: IRouteConfig + route: IRouteConfig, + initialChunk?: Buffer ): Promise { // Delegate to HttpProxy's StaticHandler await StaticHandler.handleStatic(socket, route, { connectionId: record.id, connectionManager: this.connectionManager, settings: this.settings - }, record); + }, record, initialChunk); } /** diff --git a/ts/proxies/smart-proxy/smart-proxy.ts b/ts/proxies/smart-proxy/smart-proxy.ts index 5a1840d..d625119 100644 --- a/ts/proxies/smart-proxy/smart-proxy.ts +++ b/ts/proxies/smart-proxy/smart-proxy.ts @@ -350,6 +350,12 @@ export class SmartProxy extends plugins.EventEmitter { // Start port listeners using the PortManager await this.portManager.addPorts(listeningPorts); + + // Now that ports are listening, provision any required certificates + if (this.certManager) { + console.log('Starting certificate provisioning now that ports are ready'); + await this.certManager.provisionAllCertificates(); + } // Set up periodic connection logging and inactivity checks this.connectionLogger = setInterval(() => {