smartproxy/readme.problems.md

6.7 KiB

SmartProxy Module Problems

Based on test analysis, the following potential issues have been identified in the SmartProxy module:

1. HttpProxy Route Configuration Issue

Location: ts/proxies/http-proxy/http-proxy.ts:380 Problem: The HttpProxy is trying to read the 'type' property of an undefined object when updating route configurations. Evidence: test.http-forwarding-fix.ts fails with:

TypeError: Cannot read properties of undefined (reading 'type')
    at HttpProxy.updateRouteConfigs (/mnt/data/lossless/push.rocks/smartproxy/ts/proxies/http-proxy/http-proxy.ts:380:24)

Impact: Routes with useHttpProxy configuration may not work properly.

2. Connection Forwarding Issues

Problem: Basic TCP forwarding appears to not be working correctly after the simplification to just 'forward' and 'socket-handler' action types. Evidence: Multiple forwarding tests timeout waiting for data to be forwarded:

  • test.forwarding-fix-verification.ts - times out waiting for forwarded data
  • test.connection-forwarding.ts - times out on SNI-based forwarding Impact: The 'forward' action type may not be properly forwarding connections to target servers.

3. Missing Certificate Manager Methods

Problem: Tests expect provisionAllCertificates method on certificate manager but it may not exist or may not be properly initialized. Evidence: Multiple tests fail with "this.certManager.provisionAllCertificates is not a function" Impact: Certificate provisioning may not work as expected.

4. Route Update Mechanism

Problem: The route update mechanism may have issues preserving certificate manager callbacks and other state. Evidence: Tests specifically designed to verify callback preservation after route updates. Impact: Dynamic route updates might break certificate management functionality.

5. Route-Specific Security Not Fully Implemented

Problem: While the route definitions support security configurations (ipAllowList, ipBlockList, authentication), these are not being enforced at the route level. Evidence:

  • SecurityManager has methods like isIPAuthorized for route-specific security
  • Route connection handler only checks global IP validation, not route-specific security rules
  • No evidence of route.action.security being checked when handling connections Impact: Route-specific security rules defined in configuration are not enforced, potentially allowing unauthorized access. Status: FIXED - Route-specific IP allow/block lists are now enforced when a route is matched. Authentication is logged as not enforceable for non-terminated connections. Additional Fix: Removed security checks from route matching logic - security is now properly enforced AFTER a route is matched, not during matching.

6. Security Property Location Consolidation

Problem: Security was defined in two places - route.security and route.action.security - causing confusion. Status: FIXED - Consolidated to only route.security. Removed action.security from types and updated all references.

Recommendations

  1. Verify Forward Action Implementation: Check that the 'forward' action type properly establishes bidirectional data flow between client and target server. FIXED - Basic forwarding now works correctly.

  2. Fix HttpProxy Route Handling: Ensure that route objects passed to HttpProxy.updateRouteConfigs have the expected structure with all required properties. FIXED - Routes now preserve their structure.

  3. Review Certificate Manager API: Ensure all expected methods exist and are properly documented.

  4. Add Integration Tests: Many unit tests are testing internal implementation details. Consider adding more integration tests that test the public API.

  5. Implement Route-Specific Security: Add security checks when a route is matched to enforce route-specific IP allow/block lists and authentication rules. FIXED - IP allow/block lists are now enforced at the route level.

  6. Fix TLS Detection Logic: The connection handler was treating all connections as TLS. This has been partially fixed but needs proper testing for all TLS modes.

7. HTTP Domain Matching Issue

Problem: Routes with domain restrictions fail to match HTTP connections because domain information is only available after HTTP headers are received, but route matching happens immediately upon connection. Evidence:

  • test.http-port8080-forwarding.ts - "No route found for connection on port 8080" despite having a matching route
  • HTTP connections provide domain info via the Host header, which arrives after the initial TCP connection
  • Route matching in handleInitialData happens before HTTP headers are parsed Impact: HTTP routes with domain restrictions cannot be matched, forcing users to remove domain restrictions for HTTP routes. Root Cause: For non-TLS connections, SmartProxy attempts to match routes immediately, but the domain information needed for matching is only available after parsing HTTP headers. Status: FIXED - Added skipDomainCheck parameter to route matching for HTTP proxy ports. When a port is configured with useHttpProxy and the connection is not TLS, domain validation is skipped at the initial route matching stage, allowing the HttpProxy to handle domain-based routing after headers are received.

8. HttpProxy Plain HTTP Forwarding Issue

Problem: HttpProxy is an HTTPS server but SmartProxy forwards plain HTTP connections to it via useHttpProxy configuration. Evidence:

  • test.http-port8080-forwarding.ts - Connection immediately closed after forwarding to HttpProxy
  • HttpProxy is created with http2.createSecureServer expecting TLS connections
  • SmartProxy forwards raw HTTP data to HttpProxy's HTTPS port Impact: Plain HTTP connections cannot be handled by HttpProxy, despite useHttpProxy configuration suggesting this should work. Root Cause: Design mismatch - HttpProxy is designed for HTTPS/TLS termination, not plain HTTP forwarding. Status: Documented. The useHttpProxy configuration should only be used for ports that receive TLS connections requiring termination. For plain HTTP forwarding, use direct forwarding without HttpProxy.

9. Route Security Configuration Location Issue

Problem: Tests were placing security configuration in route.action.security instead of route.security. Evidence:

  • test.route-security.ts - IP block list test failing because security was in wrong location
  • IRouteConfig interface defines security at route level, not inside action Impact: Security rules defined in action.security were ignored, causing tests to fail. Status: FIXED - Updated tests to place security configuration at the correct location (route.security).