feat(tcp): route the tunnel through an HTTP/SOCKS5 forward proxy#691
Open
desimone wants to merge 4 commits into
Open
feat(tcp): route the tunnel through an HTTP/SOCKS5 forward proxy#691desimone wants to merge 4 commits into
desimone wants to merge 4 commits into
Conversation
pomerium-cli tcp previously dialed the Pomerium edge directly, so it could not run from networks whose only egress is a forward proxy. Honor a forward proxy for the tunnel when configured via HTTP_PROXY/HTTPS_PROXY/NO_PROXY (HTTP CONNECT), ALL_PROXY (SOCKS5), or an explicit --forward-proxy flag; the flag also routes the auth fetch. When no proxy is configured behavior is unchanged, and HTTP/3 is skipped when a proxy is in use. Forward proxying is TCP-only; UDP always dials direct. ENG-4082
desimone
force-pushed
the
bdd/cli-tcp-forward-proxy
branch
from
be1829c to
9e6b8e0
Compare
This was referenced May 29, 2026
Follow-up hardening for the forward-proxy support added in this PR: - Validate an explicit --forward-proxy at startup, before binding the listener, via httputil.ValidateForwardProxyFlag. It is explicit-only and never consults the proxy environment, so a bad flag fails as a normal CLI error while env-derived proxies keep resolving at request time. - Route the auth-path Fetch through DialThroughProxy instead of transport.Proxy. An https proxy hop is now verified against the system trust store on both the tunnel and auth paths (previously the auth path validated the proxy cert against the edge CA, a trust split). The target's TLS still uses the edge tls config over the CONNECT tunnel. - Return an error from the dead SOCKS5 non-ContextDialer branch instead of silently dropping context cancellation. - Fix a per-connection goroutine leak in the http/1 tunneler: replace the parked ctx.Done waiter with context.AfterFunc + defer stop() in both TunnelTCP and TunnelUDP (UDP previously discarded the stop too). - Delete the dead http2tunneler: it had no Name() method, so it never satisfied TCPTunneler and could never be selected. Tests: explicit-flag validation (asserts env is not consulted), an https-proxy system-trust regression, an https target round-trip through a proxy with a single CONNECT hop, proxy Basic-auth credential lock-in, and a goroutine-leak assertion. make build/test/lint green; go test -race green on internal/httputil, tunnel, and authclient. Assisted-by: Claude (Opus 4.8); reviewed by Go-specialist + independent reviewer agents and the workspace review helper; verified with make build/test/lint and go test -race. ENG-4082
normalizeForwardProxy rejected any path on an explicit --forward-proxy value, so a harmless trailing slash (http://proxy:3128/) was an error. Treat a lone "/" path as empty before the path/query/fragment check; real paths, queries, and fragments are still rejected. Covers explicit-scheme, socks5, and bare host:port forms. Assisted-by: Claude (Opus 4.8); reviewed by Go-specialist and codex agents; make build/test/lint and go test -race green. ENG-4082
- never echo credentials in --forward-proxy parse errors: unwrap url.Error and replace url.EscapeError, both of which echo raw input; cover the parse-failure paths in the redaction test - keep plain-http targets on the default transport when the proxy comes from the environment (absolute-form proxying instead of CONNECT to :80, which proxy ACLs commonly deny); an explicit --forward-proxy still applies everywhere; new ProxyFetchOptions helper shared by authclient and the routes portal so one flow takes one path to the server - splice the test CONNECT proxy to its per-connection target instead of the shared last-recorded one - drop the dead nextProtos parameter on dialEdgeTLS (WithTLSConfig already pins ALPN) and the no-op context.AfterFunc(...)() calls in the UDP tests - disambiguate forward-proxy dial errors from the Pomerium edge, shorten the --forward-proxy help text, consolidate the tunnel package's CONNECT edge fakes and proxy-env helpers, trim diff-relative comments and duplicate scheme-validation tests
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
pomerium-cli tcphas always dialed the Pomerium edge directly. Only the login and token requests honoredHTTP_PROXY-style variables; the tunnel itself ignored them. On a network whose only way out is a forward proxy, that meant the tunnel couldn't connect at all, often before login even started. This is a recurring request from people behind corporate egress proxies.This change sends both the tunnel and its auth requests (that same login/token traffic) through a forward proxy. The proxy is chosen in order:
--forward-proxyif set, otherwiseHTTP_PROXY/HTTPS_PROXY/ALL_PROXY. The flag is authoritative: it applies to every connection, bypassingNO_PROXY, while the environment variables honorNO_PROXYas usual. HTTP CONNECT (http,https) and SOCKS5 (socks5,socks5h) are supported; both SOCKS schemes send the hostname to the proxy for resolution. A barehost:portis treated ashttp.(
db.internal:5432is the service you're reaching, not the proxy.)A few things worth knowing:
pomerium-cli udpstill goes direct; UDP/MASQUE over a proxy is a separate problem.jwt,k8s exec-credential, the routes portal): an https Pomerium URL through an env proxy uses the same CONNECT dialer (proxy hop verified against system trust), andALL_PROXYis honored there too. Plain-httpPomerium URLs stay on Go's default transport semantics (absolute-form proxying, noALL_PROXY), since proxy ACLs commonly deny CONNECT to port 80.--forward-proxynow fails immediately with a clear error, before the local listener opens. Bad values in the environment variables still surface when the connection is first made.Known limitations
https://proxy is verified against the system trust store only, so a TLS-intercepting proxy with an internal CA will fail the handshake. A plainhttp://CONNECT proxy (Squid and the like) is unaffected: it tunnels the edge's TLS through untouched.--forward-proxy-ca-pathis the planned follow-up.http://user:pass@proxyURL reach the proxy in cleartext (likewise SOCKS5 user/pass). Through anhttps://proxy they're protected by the proxy-leg TLS.Testing
Unit and integration tests cover proxy precedence, the CONNECT and SOCKS5 dialers against real in-process proxies, the auth requests, and a guard that UDP never touches a proxy. Verified end to end against Squid and a SOCKS5 proxy with the client's direct egress firewalled off.
AI assistance
Drafted and implemented with Claude (Opus 4.8). I reviewed and adjusted every change manually and confirmed the build, lint,
go test -race, and end-to-end checks above myself. The review-fix commit (ff6a163) was drafted with Claude (Fable 5) from a multi-agent review, cross-reviewed with Codex and a Go-specialist pass, and re-validated locally with build, lint, andgo test -race.ENG-4082