diff --git a/authclient/authclient.go b/authclient/authclient.go index 010eeaa9..507fa7c1 100644 --- a/authclient/authclient.go +++ b/authclient/authclient.go @@ -42,10 +42,19 @@ func (client *AuthClient) CheckBearerToken(ctx context.Context, serverURL *url.U req.Header.Set("Accept", "application/json") req.Header.Set("Authorization", "Bearer "+bearerToken) - _, err = httputil.Fetch(ctx, client.cfg.tlsConfig, req) + opts, err := client.fetchOpts(serverURL) + if err != nil { + return err + } + _, err = httputil.Fetch(ctx, client.cfg.tlsConfig, req, opts...) return err } +// fetchOpts resolves the forward proxy for serverURL, if any. +func (client *AuthClient) fetchOpts(serverURL *url.URL) ([]httputil.FetchOption, error) { + return httputil.ProxyFetchOptions(client.cfg.forwardProxy, serverURL) +} + // GetJWT retrieves a JWT from Pomerium. func (client *AuthClient) GetJWT(ctx context.Context, serverURL *url.URL, onOpenBrowser func(string)) (rawJWT string, err error) { if client.cfg.serviceAccount != "" { @@ -136,7 +145,11 @@ func (client *AuthClient) runOpenBrowser(ctx context.Context, li net.Listener, s return err } - bs, err := httputil.Fetch(ctx, client.cfg.tlsConfig, req) + opts, err := client.fetchOpts(serverURL) + if err != nil { + return err + } + bs, err := httputil.Fetch(ctx, client.cfg.tlsConfig, req, opts...) if err != nil { return err } diff --git a/authclient/authclient_test.go b/authclient/authclient_test.go index ee82d7c5..d78e7d76 100644 --- a/authclient/authclient_test.go +++ b/authclient/authclient_test.go @@ -2,8 +2,11 @@ package authclient import ( "context" + "crypto/tls" + "crypto/x509" "net" "net/http" + "net/http/httptest" "net/url" "os" "path/filepath" @@ -13,8 +16,37 @@ import ( "github.com/go-chi/chi/v5" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/pomerium/cli/internal/testutil" ) +// Auth requests route through the forward proxy. +func TestCheckBearerTokenViaProxy(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + pool := x509.NewCertPool() + pool.AddCert(srv.Certificate()) + + proxy := testutil.NewConnectProxy(t) + + ac := New( + WithForwardProxy(proxy.Addr), + WithTLSConfig(&tls.Config{RootCAs: pool})) + + serverURL, err := url.Parse(srv.URL) + require.NoError(t, err) + + require.NoError(t, ac.CheckBearerToken(ctx, serverURL, "token")) + assert.Positive(t, proxy.Conns(), "auth request should route through the forward proxy") + assert.Equal(t, serverURL.Host, proxy.Target()) +} + func TestAuthClient(t *testing.T) { t.Parallel() diff --git a/authclient/config.go b/authclient/config.go index e349c3af..9180a8bc 100644 --- a/authclient/config.go +++ b/authclient/config.go @@ -11,6 +11,7 @@ type config struct { serviceAccount string serviceAccountFile string tlsConfig *tls.Config + forwardProxy string } func getConfig(options ...Option) *config { @@ -38,6 +39,14 @@ func WithBrowserCommand(browserCommand string) Option { } } +// WithForwardProxy sets an explicit forward proxy override (host:port or URL). +// An empty value falls back to the standard proxy environment variables. +func WithForwardProxy(forwardProxy string) Option { + return func(cfg *config) { + cfg.forwardProxy = forwardProxy + } +} + // WithServiceAccount sets the service account in the config. func WithServiceAccount(serviceAccount string) Option { return func(cfg *config) { diff --git a/cmd/pomerium-cli/tcp.go b/cmd/pomerium-cli/tcp.go index 243f7e82..6c6bb1e7 100644 --- a/cmd/pomerium-cli/tcp.go +++ b/cmd/pomerium-cli/tcp.go @@ -9,14 +9,17 @@ import ( "os/signal" "syscall" + "github.com/rs/zerolog/log" "github.com/spf13/cobra" + "github.com/pomerium/cli/internal/httputil" "github.com/pomerium/cli/tunnel" ) var tcpCmdOptions struct { - listen string - pomeriumURL string + listen string + pomeriumURL string + forwardProxy string } func init() { @@ -28,6 +31,8 @@ func init() { "local address to start a listener on") flags.StringVar(&tcpCmdOptions.pomeriumURL, "pomerium-url", "", "the URL of the pomerium server to connect to") + flags.StringVar(&tcpCmdOptions.forwardProxy, "forward-proxy", "", + "forward proxy for the tunnel and auth (host:port or http/https/socks5/socks5h URL); overrides proxy environment variables and ignores NO_PROXY") rootCmd.AddCommand(tcpCmd) } @@ -42,6 +47,19 @@ var tcpCmd = &cobra.Command{ } cacheLastURL(proxyURL.String()) + // Validate an explicit --forward-proxy before binding the listener so a + // bad value fails as a normal CLI error. Environment proxies are + // resolved later, at request time. + fwdProxyURL, err := httputil.ValidateForwardProxyFlag(tcpCmdOptions.forwardProxy) + if err != nil { + return err + } + if fwdProxyURL != nil { + log.Info(). + Str("forward_proxy", fwdProxyURL.Redacted()). + Msg("routing tunnel and auth through forward proxy (--forward-proxy); HTTP/3 disabled") + } + var tlsConfig *tls.Config if proxyURL.Scheme == "https" { tlsConfig, err = getTLSConfig() @@ -65,6 +83,7 @@ var tcpCmd = &cobra.Command{ tunnel.WithServiceAccount(serviceAccountOptions.serviceAccount), tunnel.WithServiceAccountFile(serviceAccountOptions.serviceAccountFile), tunnel.WithTLSConfig(tlsConfig), + tunnel.WithForwardProxy(tcpCmdOptions.forwardProxy), ) if tcpCmdOptions.listen == "-" { diff --git a/internal/httputil/fetch.go b/internal/httputil/fetch.go index 1cadc9e2..225f0ce0 100644 --- a/internal/httputil/fetch.go +++ b/internal/httputil/fetch.go @@ -6,21 +6,53 @@ import ( "errors" "fmt" "io" + "net" "net/http" + "net/url" "time" ) // ErrUnauthenticated indicates the user needs to authenticate. var ErrUnauthenticated = errors.New("unauthenticated") +type fetchConfig struct { + proxyURL *url.URL +} + +// A FetchOption modifies the behavior of Fetch. +type FetchOption func(*fetchConfig) + +// WithProxyURL routes the request through the given forward proxy. When unset, +// the default transport still honors HTTP_PROXY/HTTPS_PROXY. +func WithProxyURL(u *url.URL) FetchOption { + return func(c *fetchConfig) { + c.proxyURL = u + } +} + // Fetch fetches the http request. -func Fetch(ctx context.Context, tlsConfig *tls.Config, req *http.Request) ([]byte, error) { +func Fetch(ctx context.Context, tlsConfig *tls.Config, req *http.Request, opts ...FetchOption) ([]byte, error) { + var cfg fetchConfig + for _, o := range opts { + o(&cfg) + } + ctx, clearTimeout := context.WithTimeout(ctx, 10*time.Second) defer clearTimeout() req = req.WithContext(ctx) transport := http.DefaultTransport.(*http.Transport).Clone() transport.TLSClientConfig = tlsConfig + if cfg.proxyURL != nil { + // Dial the proxy ourselves so the proxy hop's TLS is validated against + // system trust (matching the tunnel path), while the target's TLS still + // uses tlsConfig. Setting transport.Proxy would instead validate an https + // proxy against tlsConfig and re-read the proxy environment. + transport.Proxy = nil + transport.DialContext = func(ctx context.Context, _, addr string) (net.Conn, error) { + return DialThroughProxy(ctx, cfg.proxyURL, addr) + } + } hc := &http.Client{ Transport: transport, } diff --git a/internal/httputil/fetch_test.go b/internal/httputil/fetch_test.go new file mode 100644 index 00000000..bfc62e93 --- /dev/null +++ b/internal/httputil/fetch_test.go @@ -0,0 +1,155 @@ +package httputil + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/base64" + "math/big" + "net" + "net/http" + "net/http/httptest" + "net/url" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/pomerium/cli/internal/testutil" +) + +// One CONNECT hop to the target; the target's TLS is still validated against +// tlsConfig over the tunnel. +func TestFetchHTTPSTargetThroughProxy(t *testing.T) { + target := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte("hello through proxy")) + })) + defer target.Close() + + pool := x509.NewCertPool() + pool.AddCert(target.Certificate()) + + proxy := testutil.NewConnectProxy(t) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + req, err := http.NewRequest(http.MethodGet, target.URL, nil) + require.NoError(t, err) + + body, err := Fetch(ctx, &tls.Config{RootCAs: pool}, req, + WithProxyURL(&url.URL{Scheme: "http", Host: proxy.Addr})) + require.NoError(t, err) + assert.Equal(t, "hello through proxy", string(body)) + + // Exactly one CONNECT, addressed to the target host:port (not the proxy). + assert.Equal(t, int64(1), proxy.Conns(), "expected a single CONNECT hop") + targetURL, err := url.Parse(target.URL) + require.NoError(t, err) + assert.Equal(t, targetURL.Host, proxy.Target()) +} + +// With transport.Proxy nil, net/http no longer injects Proxy-Authorization, so +// the header must come from dialHTTPConnect using the proxy URL's userinfo. +func TestFetchProxyBasicAuth(t *testing.T) { + target := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte("ok")) + })) + defer target.Close() + + pool := x509.NewCertPool() + pool.AddCert(target.Certificate()) + + proxy := testutil.NewConnectProxy(t) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + req, err := http.NewRequest(http.MethodGet, target.URL, nil) + require.NoError(t, err) + + _, err = Fetch(ctx, &tls.Config{RootCAs: pool}, req, + WithProxyURL(&url.URL{Scheme: "http", User: url.UserPassword("alice", "s3cret"), Host: proxy.Addr})) + require.NoError(t, err) + + want := "Basic " + base64.StdEncoding.EncodeToString([]byte("alice:s3cret")) + assert.Equal(t, want, proxy.ProxyAuth(), "proxy credentials must reach the proxy via CONNECT") +} + +// An https forward proxy is validated against system trust, not the edge +// tlsConfig pool: the proxy cert here is trusted by the edge pool but not by +// system trust, so the dial must fail. +func TestFetchHTTPSProxyValidatesAgainstSystemTrust(t *testing.T) { + cert, leaf := selfSignedCert(t, "127.0.0.1") + proxyAddr := startTLSProxy(t, cert) + + pool := x509.NewCertPool() + pool.AddCert(leaf) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + req, err := http.NewRequest(http.MethodGet, "https://edge.example.com/path", nil) + require.NoError(t, err) + + _, err = Fetch(ctx, &tls.Config{RootCAs: pool}, req, + WithProxyURL(&url.URL{Scheme: "https", Host: proxyAddr})) + require.Error(t, err) + assert.Contains(t, err.Error(), "system-trusted") +} + +func selfSignedCert(t *testing.T, host string) (tls.Certificate, *x509.Certificate) { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: host}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + BasicConstraintsValid: true, + IsCA: true, + } + if ip := net.ParseIP(host); ip != nil { + tmpl.IPAddresses = []net.IP{ip} + } else { + tmpl.DNSNames = []string{host} + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + require.NoError(t, err) + leaf, err := x509.ParseCertificate(der) + require.NoError(t, err) + return tls.Certificate{Certificate: [][]byte{der}, PrivateKey: key, Leaf: leaf}, leaf +} + +// startTLSProxy starts a TLS listener that presents cert and drives the TLS +// handshake on each connection. A client that does not trust cert fails during +// the handshake, before any CONNECT is exchanged. +func startTLSProxy(t *testing.T, cert tls.Certificate) string { + t.Helper() + li, err := tls.Listen("tcp", "127.0.0.1:0", &tls.Config{Certificates: []tls.Certificate{cert}}) + require.NoError(t, err) + t.Cleanup(func() { _ = li.Close() }) + go func() { + for { + conn, err := li.Accept() + if err != nil { + return + } + go func() { + defer conn.Close() + if tc, ok := conn.(*tls.Conn); ok { + _ = tc.Handshake() + } + }() + } + }() + return li.Addr().String() +} diff --git a/internal/httputil/proxy.go b/internal/httputil/proxy.go new file mode 100644 index 00000000..accafcdb --- /dev/null +++ b/internal/httputil/proxy.go @@ -0,0 +1,234 @@ +package httputil + +import ( + "bufio" + "context" + "crypto/tls" + "encoding/base64" + "errors" + "fmt" + "net" + "net/http" + "net/url" + "os" + "strings" + "time" + + "golang.org/x/net/http/httpproxy" + "golang.org/x/net/proxy" +) + +// ResolveProxy determines the forward proxy to use to reach edge, if any. +// +// Precedence: an explicit override (authoritative, ignores NO_PROXY), then +// HTTP_PROXY/HTTPS_PROXY honoring NO_PROXY (scheme-aware), then ALL_PROXY as a +// fallback. It returns (nil, nil) when no proxy applies. +func ResolveProxy(override string, edge *url.URL) (*url.URL, error) { + if override = strings.TrimSpace(override); override != "" { + return normalizeForwardProxy(override) + } + + if u, err := httpproxy.FromEnvironment().ProxyFunc()(edge); err != nil { + return nil, err + } else if u != nil { + return u, nil + } + + all := os.Getenv("ALL_PROXY") + if all == "" { + all = os.Getenv("all_proxy") + } + if all == "" { + return nil, nil + } + noProxy := os.Getenv("NO_PROXY") + if noProxy == "" { + noProxy = os.Getenv("no_proxy") + } + return (&httpproxy.Config{HTTPProxy: all, HTTPSProxy: all, NoProxy: noProxy}).ProxyFunc()(edge) +} + +// ProxyFetchOptions resolves the forward proxy for target and returns the +// Fetch options that route requests through it. Plain-http targets with an +// environment-resolved proxy return no options: the default transport already +// proxies them in absolute form, which proxy ACLs commonly allow where a +// CONNECT to port 80 is denied. That carve-out keeps the default transport's +// env semantics for http targets (no ALL_PROXY, proxy TLS verified against +// the request's tls config). An explicit override always applies. +func ProxyFetchOptions(override string, target *url.URL) ([]FetchOption, error) { + if strings.TrimSpace(override) == "" && target.Scheme != "https" { + return nil, nil + } + proxyURL, err := ResolveProxy(override, target) + if err != nil { + return nil, err + } + if proxyURL == nil { + return nil, nil + } + return []FetchOption{WithProxyURL(proxyURL)}, nil +} + +// ValidateForwardProxyFlag validates an explicit --forward-proxy value without +// consulting the environment. An empty value is valid and returns (nil, nil); +// the caller then falls back to environment-based resolution at request time. +func ValidateForwardProxyFlag(raw string) (*url.URL, error) { + if raw = strings.TrimSpace(raw); raw == "" { + return nil, nil + } + return normalizeForwardProxy(raw) +} + +// normalizeForwardProxy validates and normalizes an explicit --forward-proxy +// override: a bare host:port defaults to http, the scheme must be one we can +// dial, and a host is required. +func normalizeForwardProxy(override string) (*url.URL, error) { + if !strings.Contains(override, "://") { + override = "http://" + override + } + u, err := url.Parse(override) + if err != nil { + // url.Error echoes the raw URL and url.EscapeError echoes the bytes + // around a bad escape; either may carry credentials, so surface only a + // credential-free cause. + if ue, ok := errors.AsType[*url.Error](err); ok { + err = ue.Err + } + if _, ok := errors.AsType[url.EscapeError](err); ok { + err = errors.New("invalid percent-escape") + } + return nil, fmt.Errorf("invalid forward proxy: %w", err) + } + switch u.Scheme { + case "http", "https", "socks5", "socks5h": + default: + return nil, fmt.Errorf("unsupported forward proxy scheme %q (want http, https, socks5, or socks5h)", u.Scheme) + } + if u.Hostname() == "" { + return nil, fmt.Errorf("forward proxy %q has no host", u.Redacted()) + } + // A lone trailing slash (http://proxy:3128/) is harmless; normalize it away. + if u.Path == "/" { + u.Path = "" + } + if u.Path != "" || u.RawQuery != "" || u.Fragment != "" { + return nil, fmt.Errorf("forward proxy %q must not have a path, query, or fragment", u.Redacted()) + } + return u, nil +} + +// proxyDialTimeout bounds the proxy connect and CONNECT exchange when the +// caller's context carries no deadline, so a dead proxy fails fast. +const proxyDialTimeout = 30 * time.Second + +// DialThroughProxy dials target (host:port) through the given forward proxy. +func DialThroughProxy(ctx context.Context, proxyURL *url.URL, target string) (net.Conn, error) { + if _, ok := ctx.Deadline(); !ok { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, proxyDialTimeout) + defer cancel() + } + switch proxyURL.Scheme { + case "socks5", "socks5h": + d, err := proxy.FromURL(proxyURL, proxy.Direct) + if err != nil { + return nil, fmt.Errorf("failed to create socks5 dialer: %w", err) + } + cd, ok := d.(proxy.ContextDialer) + if !ok { + return nil, errors.New("socks5 dialer does not support context cancellation") + } + return cd.DialContext(ctx, "tcp", target) + case "http", "https", "": + return dialHTTPConnect(ctx, proxyURL, target) + default: + return nil, fmt.Errorf("unsupported proxy scheme: %q", proxyURL.Scheme) + } +} + +func dialHTTPConnect(ctx context.Context, proxyURL *url.URL, target string) (_ net.Conn, retErr error) { + proxyAddr := proxyURL.Host + if proxyURL.Port() == "" { + if proxyURL.Scheme == "https" { + proxyAddr = net.JoinHostPort(proxyURL.Hostname(), "443") + } else { + proxyAddr = net.JoinHostPort(proxyURL.Hostname(), "80") + } + } + + conn, err := (&net.Dialer{}).DialContext(ctx, "tcp", proxyAddr) + if err != nil { + return nil, fmt.Errorf("failed to dial forward proxy: %w", err) + } + defer func() { + if retErr != nil { + _ = conn.Close() + } + }() + + if deadline, ok := ctx.Deadline(); ok { + if err := conn.SetDeadline(deadline); err != nil { + return nil, err + } + } + + if proxyURL.Scheme == "https" { + tc := tls.Client(conn, &tls.Config{ServerName: proxyURL.Hostname()}) + if err := tc.HandshakeContext(ctx); err != nil { + return nil, fmt.Errorf("forward proxy %s TLS handshake failed (only system-trusted CAs are supported): %w", proxyURL.Redacted(), err) + } + conn = tc + } + + // abort the CONNECT exchange on cancellation; stop() detaches the conn from + // ctx before it is handed to the caller. + stop := context.AfterFunc(ctx, func() { _ = conn.Close() }) + defer stop() + + req := &http.Request{ + Method: http.MethodConnect, + URL: &url.URL{Opaque: target}, + Host: target, + Header: http.Header{}, + } + if proxyURL.User != nil { + user := proxyURL.User.Username() + pass, _ := proxyURL.User.Password() + auth := base64.StdEncoding.EncodeToString([]byte(user + ":" + pass)) + req.Header.Set("Proxy-Authorization", "Basic "+auth) + } + if err := req.Write(conn); err != nil { + return nil, fmt.Errorf("failed to write CONNECT request: %w", err) + } + + br := bufio.NewReader(conn) + res, err := http.ReadResponse(br, req) + if err != nil { + return nil, fmt.Errorf("failed to read CONNECT response: %w", err) + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return nil, fmt.Errorf("forward proxy CONNECT failed: %s", res.Status) + } + + if err := conn.SetDeadline(time.Time{}); err != nil { + return nil, err + } + + if br.Buffered() > 0 { + return &bufferedConn{Conn: conn, br: br}, nil + } + return conn, nil +} + +// bufferedConn drains bytes the CONNECT response parser buffered before reading +// from the underlying connection. The bufio.Reader transparently falls through +// to the underlying conn once its buffer is exhausted. +type bufferedConn struct { + net.Conn + br *bufio.Reader +} + +func (c *bufferedConn) Read(p []byte) (int, error) { + return c.br.Read(p) +} diff --git a/internal/httputil/proxy_test.go b/internal/httputil/proxy_test.go new file mode 100644 index 00000000..11c9f179 --- /dev/null +++ b/internal/httputil/proxy_test.go @@ -0,0 +1,273 @@ +package httputil + +import ( + "bufio" + "context" + "io" + "net" + "net/url" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/pomerium/cli/internal/testutil" +) + +func TestResolveProxy(t *testing.T) { + httpsEdge := &url.URL{Scheme: "https", Host: "edge.example.com:443"} + httpEdge := &url.URL{Scheme: "http", Host: "edge.example.com:80"} + + for _, tc := range []struct { + name string + override string + env map[string]string + edge *url.URL + want string + wantErr bool + }{ + {name: "override url", override: "http://proxy:8080", edge: httpsEdge, want: "http://proxy:8080"}, + {name: "override bare host:port", override: "proxy:8080", edge: httpsEdge, want: "http://proxy:8080"}, + {name: "override socks5", override: "socks5://proxy:1080", edge: httpsEdge, want: "socks5://proxy:1080"}, + {name: "override socks5h", override: "socks5h://proxy:1080", edge: httpsEdge, want: "socks5h://proxy:1080"}, + {name: "override trims whitespace", override: " http://proxy:8080 ", edge: httpsEdge, want: "http://proxy:8080"}, + {name: "override trailing slash ok", override: "http://proxy:8080/", edge: httpsEdge, want: "http://proxy:8080"}, + {name: "override socks5 trailing slash ok", override: "socks5://proxy:1080/", edge: httpsEdge, want: "socks5://proxy:1080"}, + {name: "override bare host trailing slash ok", override: "proxy:8080/", edge: httpsEdge, want: "http://proxy:8080"}, + {name: "https_proxy", env: map[string]string{"HTTPS_PROXY": "http://hp:3128"}, edge: httpsEdge, want: "http://hp:3128"}, + {name: "http_proxy", env: map[string]string{"HTTP_PROXY": "http://hp:3128"}, edge: httpEdge, want: "http://hp:3128"}, + {name: "no_proxy", env: map[string]string{"HTTPS_PROXY": "http://hp:3128", "NO_PROXY": "edge.example.com"}, edge: httpsEdge, want: ""}, + {name: "all_proxy socks5", env: map[string]string{"ALL_PROXY": "socks5://sp:1080"}, edge: httpsEdge, want: "socks5://sp:1080"}, + {name: "all_proxy honors no_proxy", env: map[string]string{"ALL_PROXY": "socks5://sp:1080", "NO_PROXY": "edge.example.com"}, edge: httpsEdge, want: ""}, + {name: "override beats env", override: "http://override:9", env: map[string]string{"HTTPS_PROXY": "http://hp:3128"}, edge: httpsEdge, want: "http://override:9"}, + {name: "whitespace override is unset", override: " ", env: map[string]string{"HTTPS_PROXY": "http://hp:3128"}, edge: httpsEdge, want: "http://hp:3128"}, + {name: "none", edge: httpsEdge, want: ""}, + {name: "bad scheme", override: "socks4://p:1", edge: httpsEdge, wantErr: true}, + {name: "path rejected", override: "http://p:8080/path", edge: httpsEdge, wantErr: true}, + {name: "query rejected", override: "http://p:8080?q=1", edge: httpsEdge, wantErr: true}, + {name: "no host", override: "http://", edge: httpsEdge, wantErr: true}, + } { + t.Run(tc.name, func(t *testing.T) { + testutil.ClearProxyEnv(t) + for k, v := range tc.env { + t.Setenv(k, v) + } + + got, err := ResolveProxy(tc.override, tc.edge) + if tc.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + if tc.want == "" { + assert.Nil(t, got) + return + } + require.NotNil(t, got) + assert.Equal(t, tc.want, got.String()) + }) + } +} + +func TestResolveProxyRedactsCredentials(t *testing.T) { + edge := &url.URL{Scheme: "https", Host: "edge:443"} + for _, override := range []string{ + "http://user:hunter2@proxy:8080/path", // rejected after parsing + "http://user:hunter2@proxy:badport", // url.Parse failure + "http://user:hunter2%@proxy:8080", // invalid escape in the password itself + } { + _, err := ResolveProxy(override, edge) + require.Error(t, err) + assert.NotContains(t, err.Error(), "hunter2", "override %q", override) + } + + // url.EscapeError quotes the bytes around a bad escape ("%te" here), which + // would leak a password fragment. + _, err := ResolveProxy("http://user:hun%ter2@proxy:8080", edge) + require.Error(t, err) + assert.NotContains(t, err.Error(), "%te") +} + +func TestValidateForwardProxyFlag(t *testing.T) { + // The explicit-flag validator must never consult the environment, otherwise + // env-proxy errors would surface at startup instead of at request time. + t.Setenv("HTTPS_PROXY", "http://env-proxy:3128") + t.Setenv("ALL_PROXY", "socks5://env-proxy:1080") + + u, err := ValidateForwardProxyFlag("") + require.NoError(t, err) + assert.Nil(t, u, "empty flag is valid and must not resolve env proxies") + + u, err = ValidateForwardProxyFlag("proxy:8080") + require.NoError(t, err) + require.NotNil(t, u) + assert.Equal(t, "http://proxy:8080", u.String()) +} + +// Plain-http targets keep the default transport's absolute-form env proxying; +// an explicit override always routes through the custom dialer. +func TestProxyFetchOptions(t *testing.T) { + testutil.ClearProxyEnv(t) + t.Setenv("HTTP_PROXY", "http://hp:3128") + t.Setenv("HTTPS_PROXY", "http://hp:3128") + + httpEdge := &url.URL{Scheme: "http", Host: "edge.example.com:80"} + httpsEdge := &url.URL{Scheme: "https", Host: "edge.example.com:443"} + + opts, err := ProxyFetchOptions("", httpEdge) + require.NoError(t, err) + assert.Empty(t, opts, "env proxy + plain-http target stays on the default transport") + + // the carve-out must not pre-empt env errors the default transport owns. + t.Setenv("ALL_PROXY", "::bad::") + opts, err = ProxyFetchOptions("", httpEdge) + require.NoError(t, err) + assert.Empty(t, opts) + t.Setenv("ALL_PROXY", "") + + opts, err = ProxyFetchOptions("", httpsEdge) + require.NoError(t, err) + assert.Len(t, opts, 1) + + opts, err = ProxyFetchOptions("op:9", httpEdge) + require.NoError(t, err) + assert.Len(t, opts, 1, "explicit override applies to plain-http targets") +} + +func TestDialThroughProxyHTTPConnect(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + backend := echoListener(t) + proxy := testutil.NewConnectProxy(t) + + conn, err := DialThroughProxy(ctx, &url.URL{Scheme: "http", Host: proxy.Addr}, backend) + require.NoError(t, err) + defer conn.Close() + + assert.Equal(t, "ping\n", roundTrip(t, conn)) + assert.Equal(t, backend, proxy.Target()) +} + +func TestDialThroughProxySOCKS5(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + backend := echoListener(t) + proxy := testutil.NewSOCKS5Proxy(t) + + conn, err := DialThroughProxy(ctx, &url.URL{Scheme: "socks5", Host: proxy.Addr}, backend) + require.NoError(t, err) + defer conn.Close() + + assert.Equal(t, "ping\n", roundTrip(t, conn)) + assert.Equal(t, backend, proxy.Target()) +} + +func TestDialThroughProxyConnectRejected(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + proxyLi, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer proxyLi.Close() + go func() { + conn, err := proxyLi.Accept() + if err != nil { + return + } + defer conn.Close() + br := bufio.NewReader(conn) + for { + h, err := br.ReadString('\n') + if err != nil || h == "\r\n" || h == "\n" { + break + } + } + _, _ = io.WriteString(conn, "HTTP/1.1 403 Forbidden\r\n\r\n") + }() + + _, err = DialThroughProxy(ctx, &url.URL{Scheme: "http", Host: proxyLi.Addr().String()}, "edge.example.com:443") + require.Error(t, err) + assert.Contains(t, err.Error(), "403") +} + +// TestDialThroughProxyHTTPSProxyError verifies that a TLS failure to an https +// proxy reports the system-trust limitation without leaking credentials. The +// listener speaks plaintext, so the handshake fails. +func TestDialThroughProxyHTTPSProxyError(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + li, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer li.Close() + go func() { + if c, err := li.Accept(); err == nil { + _ = c.Close() + } + }() + + _, err = DialThroughProxy(ctx, &url.URL{Scheme: "https", User: url.UserPassword("u", "hunter2"), Host: li.Addr().String()}, "edge:443") + require.Error(t, err) + assert.Contains(t, err.Error(), "system-trusted") + assert.NotContains(t, err.Error(), "hunter2") +} + +// TestDialThroughProxyContextCancel verifies a stalled CONNECT exchange unblocks +// promptly when the context is canceled (via the AfterFunc guard). +func TestDialThroughProxyContextCancel(t *testing.T) { + li, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer li.Close() + go func() { + conn, err := li.Accept() + if err != nil { + return + } + defer conn.Close() + _, _ = io.Copy(io.Discard, conn) // read forever, never reply + }() + + ctx, cancel := context.WithCancel(context.Background()) + go func() { time.Sleep(100 * time.Millisecond); cancel() }() + + done := make(chan error, 1) + go func() { + _, err := DialThroughProxy(ctx, &url.URL{Scheme: "http", Host: li.Addr().String()}, "edge:443") + done <- err + }() + + select { + case err := <-done: + require.Error(t, err) + case <-time.After(5 * time.Second): + t.Fatal("DialThroughProxy did not return after context cancellation") + } +} + +func echoListener(t *testing.T) string { + t.Helper() + li, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + t.Cleanup(func() { _ = li.Close() }) + go func() { + conn, err := li.Accept() + if err != nil { + return + } + defer conn.Close() + _, _ = io.Copy(conn, conn) + }() + return li.Addr().String() +} + +func roundTrip(t *testing.T, conn net.Conn) string { + t.Helper() + _, err := io.WriteString(conn, "ping\n") + require.NoError(t, err) + got, err := bufio.NewReader(conn).ReadString('\n') + require.NoError(t, err) + return got +} diff --git a/internal/portal/portal.go b/internal/portal/portal.go index de295881..ff82e3c2 100644 --- a/internal/portal/portal.go +++ b/internal/portal/portal.go @@ -103,7 +103,13 @@ func (p *Portal) listRoutes(ctx context.Context, serverURL *url.URL, rawJWT stri req.Header.Set("Accept", "application/json") req.Header.Set("Authorization", "Bearer Pomerium-"+rawJWT) - bs, err := httputil.Fetch(ctx, p.cfg.tlsConfig, req) + // match the auth client's proxy resolution so the whole portal flow takes + // one path to the server. + opts, err := httputil.ProxyFetchOptions("", serverURL) + if err != nil { + return nil, fmt.Errorf("error resolving forward proxy: %w", err) + } + bs, err := httputil.Fetch(ctx, p.cfg.tlsConfig, req, opts...) if err != nil { return nil, fmt.Errorf("error fetching routes portal: %w", err) } diff --git a/internal/testutil/proxy.go b/internal/testutil/proxy.go new file mode 100644 index 00000000..6e333dd5 --- /dev/null +++ b/internal/testutil/proxy.go @@ -0,0 +1,205 @@ +package testutil + +import ( + "bufio" + "encoding/binary" + "io" + "net" + "strconv" + "strings" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/require" +) + +// RecordingProxy is a fake forward proxy for tests. It records the connections +// it accepts, the CONNECT/SOCKS5 target it was asked to reach, and any +// Proxy-Authorization header, then splices through to that target. +type RecordingProxy struct { + Addr string + upstream string // when set, splice here instead of the requested target + conns atomic.Int64 + target atomic.Value + auth atomic.Value +} + +// Conns returns how many connections the proxy accepted. +func (p *RecordingProxy) Conns() int64 { return p.conns.Load() } + +// Target returns the last target the proxy was asked to reach. +func (p *RecordingProxy) Target() string { s, _ := p.target.Load().(string); return s } + +// ProxyAuth returns the last Proxy-Authorization header value (CONNECT only). +func (p *RecordingProxy) ProxyAuth() string { s, _ := p.auth.Load().(string); return s } + +func listen(t *testing.T) (net.Listener, *RecordingProxy) { + t.Helper() + li, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + t.Cleanup(func() { _ = li.Close() }) + return li, &RecordingProxy{Addr: li.Addr().String()} +} + +// NewConnectProxy starts an HTTP CONNECT proxy that splices to the requested +// target. +func NewConnectProxy(t *testing.T) *RecordingProxy { + return newConnectProxy(t, "") +} + +// NewConnectProxyTo starts an HTTP CONNECT proxy that splices every CONNECT to +// upstream, ignoring the requested target (which is still recorded). Use this +// when the advertised edge host must be non-loopback so HTTP(S)_PROXY engages. +func NewConnectProxyTo(t *testing.T, upstream string) *RecordingProxy { + return newConnectProxy(t, upstream) +} + +func newConnectProxy(t *testing.T, upstream string) *RecordingProxy { + li, p := listen(t) + p.upstream = upstream + go func() { + for { + conn, err := li.Accept() + if err != nil { + return + } + go p.handleConnect(conn) + } + }() + return p +} + +// ClearProxyEnv blanks every proxy environment variable for the test. +func ClearProxyEnv(t *testing.T) { + t.Helper() + for _, k := range []string{"HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "ALL_PROXY", "http_proxy", "https_proxy", "no_proxy", "all_proxy"} { + t.Setenv(k, "") + } +} + +func (p *RecordingProxy) handleConnect(conn net.Conn) { + defer conn.Close() + p.conns.Add(1) + + br := bufio.NewReader(conn) + line, err := br.ReadString('\n') + if err != nil { + return + } + var target string + if f := strings.Fields(line); len(f) >= 2 { + target = f[1] + p.target.Store(target) + } + for { + h, err := br.ReadString('\n') + if err != nil { + return + } + if strings.HasPrefix(strings.ToLower(h), "proxy-authorization:") { + _, val, _ := strings.Cut(h, ":") + p.auth.Store(strings.TrimSpace(val)) + } + if h == "\r\n" || h == "\n" { + break + } + } + _, _ = io.WriteString(conn, "HTTP/1.1 200 Connection established\r\n\r\n") + + dst := target + if p.upstream != "" { + dst = p.upstream + } + out, err := net.Dial("tcp", dst) + if err != nil { + return + } + defer out.Close() + splice(conn, br, out) +} + +// NewSOCKS5Proxy starts a no-auth SOCKS5 proxy that splices to the requested +// target. +func NewSOCKS5Proxy(t *testing.T) *RecordingProxy { + li, p := listen(t) + go func() { + for { + conn, err := li.Accept() + if err != nil { + return + } + go p.handleSOCKS5(conn) + } + }() + return p +} + +func (p *RecordingProxy) handleSOCKS5(conn net.Conn) { + defer conn.Close() + br := bufio.NewReader(conn) + + // greeting: VER, NMETHODS, METHODS... + greeting := make([]byte, 2) + if _, err := io.ReadFull(br, greeting); err != nil { + return + } + if _, err := io.ReadFull(br, make([]byte, int(greeting[1]))); err != nil { + return + } + if _, err := conn.Write([]byte{0x05, 0x00}); err != nil { // no auth + return + } + + // request: VER CMD RSV ATYP DST.ADDR DST.PORT + req := make([]byte, 4) + if _, err := io.ReadFull(br, req); err != nil { + return + } + var host string + switch req[3] { + case 0x01: // IPv4 + b := make([]byte, 4) + if _, err := io.ReadFull(br, b); err != nil { + return + } + host = net.IP(b).String() + case 0x03: // domain + l := make([]byte, 1) + if _, err := io.ReadFull(br, l); err != nil { + return + } + b := make([]byte, int(l[0])) + if _, err := io.ReadFull(br, b); err != nil { + return + } + host = string(b) + default: + return + } + portBytes := make([]byte, 2) + if _, err := io.ReadFull(br, portBytes); err != nil { + return + } + target := net.JoinHostPort(host, strconv.Itoa(int(binary.BigEndian.Uint16(portBytes)))) + p.conns.Add(1) + p.target.Store(target) + + // reply success with a dummy bound address (0.0.0.0:0). + if _, err := conn.Write([]byte{0x05, 0x00, 0x00, 0x01, 0, 0, 0, 0, 0, 0}); err != nil { + return + } + + out, err := net.Dial("tcp", target) + if err != nil { + return + } + defer out.Close() + splice(conn, br, out) +} + +func splice(client net.Conn, clientReader io.Reader, upstream net.Conn) { + errc := make(chan error, 2) + go func() { _, e := io.Copy(upstream, clientReader); errc <- e }() + go func() { _, e := io.Copy(client, upstream); errc <- e }() + <-errc +} diff --git a/tunnel/config.go b/tunnel/config.go index 8fb67ac9..06eb46dc 100644 --- a/tunnel/config.go +++ b/tunnel/config.go @@ -14,6 +14,7 @@ type config struct { serviceAccountFile string tlsConfig *tls.Config browserConfig string + forwardProxy string } func getConfig(options ...Option) *config { @@ -42,6 +43,15 @@ func WithDestinationHost(dstHost string) Option { } } +// WithForwardProxy returns an option to configure the forward proxy used to +// reach the Pomerium edge (host:port or URL). An empty value falls back to the +// standard proxy environment variables. +func WithForwardProxy(forwardProxy string) Option { + return func(cfg *config) { + cfg.forwardProxy = forwardProxy + } +} + // WithJWTCache returns an option to configure the jwt cache. func WithJWTCache(jwtCache jwt.Cache) Option { return func(cfg *config) { diff --git a/tunnel/dial.go b/tunnel/dial.go new file mode 100644 index 00000000..1c405211 --- /dev/null +++ b/tunnel/dial.go @@ -0,0 +1,63 @@ +package tunnel + +import ( + "context" + "crypto/tls" + "net" + "net/url" + + "github.com/pomerium/cli/internal/httputil" +) + +// edgeURL builds the URL of the Pomerium edge for proxy resolution. +func edgeURL(cfg *config) *url.URL { + scheme := "http" + if cfg.tlsConfig != nil { + scheme = "https" + } + return &url.URL{Scheme: scheme, Host: cfg.proxyHost} +} + +// resolveEdgeProxy resolves the forward proxy (if any) for reaching the edge. +// Forward proxying is TCP-only; UDP callers must not use it. +func resolveEdgeProxy(cfg *config) (*url.URL, error) { + return httputil.ResolveProxy(cfg.forwardProxy, edgeURL(cfg)) +} + +// dialEdgeTLS establishes a connection to the Pomerium edge, routing through +// proxyURL when non-nil, and TLS-wraps it when a tls config is configured. +// The config's ALPN is already pinned to http/1.1 by WithTLSConfig. +func dialEdgeTLS(ctx context.Context, cfg *config, proxyURL *url.URL) (net.Conn, error) { + var raw net.Conn + var err error + if proxyURL == nil { + raw, err = (&net.Dialer{}).DialContext(ctx, "tcp", cfg.proxyHost) + } else { + raw, err = httputil.DialThroughProxy(ctx, proxyURL, cfg.proxyHost) + } + if err != nil { + return nil, err + } + + if cfg.tlsConfig == nil { + return raw, nil + } + + tlsCfg := cfg.tlsConfig.Clone() + // tls.Client does not derive ServerName from the dial address; set it or + // certificate verification fails. + if tlsCfg.ServerName == "" { + host, _, err := net.SplitHostPort(cfg.proxyHost) + if err != nil { + host = cfg.proxyHost + } + tlsCfg.ServerName = host + } + + tc := tls.Client(raw, tlsCfg) + if err := tc.HandshakeContext(ctx); err != nil { + _ = raw.Close() + return nil, err + } + return tc, nil +} diff --git a/tunnel/tunnel.go b/tunnel/tunnel.go index 099f0d15..6d9934ea 100644 --- a/tunnel/tunnel.go +++ b/tunnel/tunnel.go @@ -44,7 +44,8 @@ func New(options ...Option) *Tunnel { authclient.WithBrowserCommand(cfg.browserConfig), authclient.WithServiceAccount(cfg.serviceAccount), authclient.WithServiceAccountFile(cfg.serviceAccountFile), - authclient.WithTLSConfig(cfg.tlsConfig)), + authclient.WithTLSConfig(cfg.tlsConfig), + authclient.WithForwardProxy(cfg.forwardProxy)), } } diff --git a/tunnel/tunnel_http1.go b/tunnel/tunnel_http1.go index cb89ef68..66928c17 100644 --- a/tunnel/tunnel_http1.go +++ b/tunnel/tunnel_http1.go @@ -3,7 +3,6 @@ package tunnel import ( "bufio" "context" - "crypto/tls" "fmt" "io" "net" @@ -44,26 +43,17 @@ func (t *http1tunneler) TunnelTCP( Header: hdr, }).WithContext(ctx) - var remote net.Conn - var err error - if t.cfg.tlsConfig != nil { - cfg := t.cfg.tlsConfig.Clone() - cfg.NextProtos = []string{"http/1.1"} - - remote, err = (&tls.Dialer{Config: cfg}).DialContext(ctx, "tcp", t.cfg.proxyHost) - } else { - remote, err = (&net.Dialer{}).DialContext(ctx, "tcp", t.cfg.proxyHost) + proxyURL, err := resolveEdgeProxy(t.cfg) + if err != nil { + return fmt.Errorf("http/1: failed to resolve forward proxy: %w", err) } + remote, err := dialEdgeTLS(ctx, t.cfg, proxyURL) if err != nil { return fmt.Errorf("http/1: failed to establish connection to proxy: %w", err) } defer remote.Close() - if done := ctx.Done(); done != nil { - go func() { - <-done - _ = remote.Close() - }() - } + stop := context.AfterFunc(ctx, func() { _ = remote.Close() }) + defer stop() err = req.Write(remote) if err != nil { @@ -114,18 +104,14 @@ func (t *http1tunneler) TunnelUDP( ) error { eventSink.OnConnecting(ctx) - var remote net.Conn - var err error - if t.cfg.tlsConfig != nil { - remote, err = (&tls.Dialer{Config: t.cfg.tlsConfig}).DialContext(ctx, "tcp", t.cfg.proxyHost) - } else { - remote, err = (&net.Dialer{}).DialContext(ctx, "tcp", t.cfg.proxyHost) - } + // UDP never traverses a forward proxy (TCP-only feature), so dial direct. + remote, err := dialEdgeTLS(ctx, t.cfg, nil) if err != nil { return fmt.Errorf("http/1: failed to establish connection to proxy: %w", err) } defer remote.Close() - context.AfterFunc(ctx, func() { _ = remote.Close() }) + stop := context.AfterFunc(ctx, func() { _ = remote.Close() }) + defer stop() dstHost, dstPort, err := net.SplitHostPort(t.cfg.dstHost) if err != nil { diff --git a/tunnel/tunnel_http1_test.go b/tunnel/tunnel_http1_test.go new file mode 100644 index 00000000..374b7ad3 --- /dev/null +++ b/tunnel/tunnel_http1_test.go @@ -0,0 +1,56 @@ +package tunnel + +import ( + "net" + "runtime" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/pomerium/cli/internal/testutil" +) + +// A completed TunnelTCP must not leave a goroutine parked on ctx.Done while the +// parent context stays alive. Non-parallel so the goroutine count stays stable. +func TestHTTP1TunnelTCPNoGoroutineLeak(t *testing.T) { + testutil.ClearProxyEnv(t) + + edge, _ := fakeEdge(t) + ctx := t.Context() + + run := func() { + tun := &http1tunneler{getConfig( + WithDestinationHost("example.com:9999"), + WithProxyHost(edge), + )} + c1, c2 := net.Pipe() + // one line then EOF: the backend closes per line, unwinding the chain. + go func() { + _, _ = c1.Write([]byte("x\n")) + _ = c1.Close() + }() + require.NoError(t, tun.TunnelTCP(ctx, DiscardEvents(), c2, "")) + _ = c2.Close() + } + + run() // warm up one-time goroutines before sampling the baseline + base := runtime.NumGoroutine() + + const n = 30 + for range n { + run() + } + + // Transient goroutines (edge handlers, copy loops) exit once each connection + // closes; a leaked ctx.Done waiter never would. Poll until the count settles. + var leaked int + for range 200 { + if leaked = runtime.NumGoroutine() - base; leaked < n/2 { + break + } + time.Sleep(10 * time.Millisecond) + } + assert.Less(t, leaked, n/2, "TunnelTCP leaked goroutines after connections closed") +} diff --git a/tunnel/tunnel_http2.go b/tunnel/tunnel_http2.go deleted file mode 100644 index 986699bb..00000000 --- a/tunnel/tunnel_http2.go +++ /dev/null @@ -1,106 +0,0 @@ -package tunnel - -import ( - "context" - "crypto/tls" - "fmt" - "io" - "net/http" - "net/url" - - "github.com/rs/zerolog/log" - "golang.org/x/net/http2" -) - -type http2tunneler struct { - cfg *config -} - -func (t *http2tunneler) TunnelTCP( - ctx context.Context, - eventSink EventSink, - local io.ReadWriter, - rawJWT string, -) error { - ctx = log.Ctx(ctx).With().Str("component", "http2tunneler").Logger().WithContext(ctx) - - eventSink.OnConnecting(ctx) - - hdr := http.Header{} - if rawJWT != "" { - hdr.Set("Authorization", "Pomerium "+rawJWT) - } - - if t.cfg.tlsConfig == nil { - return fmt.Errorf("%w: http2 requires TLS", errUnsupported) - } - - cfg := t.cfg.tlsConfig.Clone() - cfg.NextProtos = []string{"h2"} - - raw, err := (&tls.Dialer{Config: cfg}).DialContext(ctx, "tcp", t.cfg.proxyHost) - if err != nil { - return fmt.Errorf("http/2: failed to establish connection to proxy: %w", err) - } - defer raw.Close() - - remote, ok := raw.(*tls.Conn) - if !ok { - return fmt.Errorf("http/2: unexpected connection type returned from dial: %T", raw) - } - - protocol := remote.ConnectionState().NegotiatedProtocol - if protocol != "h2" { - return fmt.Errorf("%w: unexpected TLS protocol: %s", errUnsupported, protocol) - } - - cc, err := (&http2.Transport{}).NewClientConn(remote) - if err != nil { - return fmt.Errorf("http/2: failed to establish connection: %w", err) - } - defer cc.Close() - - pr, pw := io.Pipe() - - req := (&http.Request{ - Method: "CONNECT", - URL: &url.URL{Opaque: t.cfg.dstHost}, - Host: t.cfg.dstHost, - Header: hdr, - Body: pr, - ContentLength: -1, - }).WithContext(ctx) - - res, err := cc.RoundTrip(req) - if err != nil { - return fmt.Errorf("http/2: error making connect request: %w", err) - } - defer res.Body.Close() - - err = httpStatusCodeToError(res.StatusCode) - if err != nil { - return err - } - - eventSink.OnConnected(ctx) - - errc := make(chan error, 2) - go func() { - _, err := io.Copy(pw, local) - errc <- err - }() - go func() { - _, err := io.Copy(local, res.Body) - errc <- err - }() - - select { - case err = <-errc: - case <-ctx.Done(): - err = context.Cause(ctx) - } - - eventSink.OnDisconnected(ctx, err) - - return err -} diff --git a/tunnel/tunnel_http2_test.go b/tunnel/tunnel_http2_test.go deleted file mode 100644 index 0840e370..00000000 --- a/tunnel/tunnel_http2_test.go +++ /dev/null @@ -1,70 +0,0 @@ -package tunnel - -import ( - "context" - "crypto/tls" - "io" - "net" - "net/http" - "net/http/httptest" - "testing" - "time" - - "github.com/stretchr/testify/assert" -) - -func TestTCPTunnelViaHTTP2(t *testing.T) { - t.Parallel() - - ctx, clearTimeout := context.WithTimeout(context.Background(), time.Second*10) - defer clearTimeout() - - srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if !assert.Equal(t, "CONNECT", r.Method) { - return - } - if !assert.Equal(t, "Pomerium JWT", r.Header.Get("Authorization")) { - return - } - if !assert.Equal(t, "example.com:9999", r.Host) { - return - } - - defer r.Body.Close() - w.WriteHeader(http.StatusOK) - w.(http.Flusher).Flush() - - buf := make([]byte, 4) - _, err := io.ReadFull(r.Body, buf) - assert.NoError(t, err) - assert.Equal(t, []byte{1, 2, 3, 4}, buf) - - _, _ = w.Write([]byte{5, 6, 7, 8}) - })) - srv.EnableHTTP2 = true - srv.StartTLS() - - c1, c2 := net.Pipe() - go func() { - _, _ = c1.Write([]byte{1, 2, 3, 4}) - }() - go func() { - buf := make([]byte, 4) - _, err := io.ReadFull(c1, buf) - assert.NoError(t, err) - assert.Equal(t, []byte{5, 6, 7, 8}, buf) - _ = c1.Close() - }() - - tun := &http2tunneler{ - getConfig( - WithDestinationHost("example.com:9999"), - WithProxyHost(srv.Listener.Addr().String()), - WithTLSConfig(&tls.Config{ - InsecureSkipVerify: true, - }), - ), - } - err := tun.TunnelTCP(ctx, DiscardEvents(), c2, "JWT") - assert.NoError(t, err) -} diff --git a/tunnel/tunnel_proxy_test.go b/tunnel/tunnel_proxy_test.go new file mode 100644 index 00000000..b1789379 --- /dev/null +++ b/tunnel/tunnel_proxy_test.go @@ -0,0 +1,194 @@ +package tunnel + +import ( + "bufio" + "bytes" + "context" + "encoding/base64" + "io" + "net" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/pomerium/cli/internal/testutil" +) + +// fakeEdge starts a backend plus a Pomerium-edge server that terminates the +// tunnel CONNECT and splices to the backend. It returns the edge address and a +// channel that receives the first line the backend reads from the tunnel. +func fakeEdge(t *testing.T) (edgeAddr string, got <-chan string) { + t.Helper() + gotc := make(chan string, 1) + + backend, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + t.Cleanup(func() { _ = backend.Close() }) + go func() { + for { + conn, err := backend.Accept() + if err != nil { + return + } + go func() { + defer conn.Close() + ln, _, _ := bufio.NewReader(conn).ReadLine() + select { + case gotc <- string(ln): + default: + } + }() + } + }() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !assert.Equal(t, "CONNECT", r.Method) { + return + } + w.WriteHeader(200) + in, brw, err := w.(http.Hijacker).Hijack() + if !assert.NoError(t, err) { + return + } + defer in.Close() + out, err := net.Dial("tcp", backend.Addr().String()) + if !assert.NoError(t, err) { + return + } + defer out.Close() + errc := make(chan error, 2) + go func() { _, e := io.Copy(in, out); errc <- e }() + go func() { _, e := io.Copy(out, deBuffer(brw.Reader, in)); errc <- e }() + <-errc + })) + t.Cleanup(srv.Close) + + return srv.Listener.Addr().String(), gotc +} + +// runTCPTunnel drives one "HELLO WORLD" line through the tunnel and asserts +// the backend received it. +func runTCPTunnel(t *testing.T, got <-chan string, opts ...Option) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + var buf bytes.Buffer + err := New(opts...).Run(ctx, readWriter{strings.NewReader("HELLO WORLD\n"), &buf}, DiscardEvents()) + require.NoError(t, err) + + select { + case line := <-got: + assert.Equal(t, "HELLO WORLD", line) + case <-time.After(5 * time.Second): + t.Fatal("backend never received the tunneled data") + } +} + +func TestTunnelViaForwardProxyFlag(t *testing.T) { + testutil.ClearProxyEnv(t) + edge, got := fakeEdge(t) + proxy := testutil.NewConnectProxy(t) + + runTCPTunnel(t, got, + WithDestinationHost("example.com:9999"), + WithProxyHost(edge), + WithForwardProxy(proxy.Addr)) + + assert.Equal(t, edge, proxy.Target()) +} + +// Env-var proxies never apply to loopback edges (Go's httpproxy bypasses +// localhost), so these tests advertise a non-loopback edge name and use a proxy +// that splices to the real loopback edge. NO_PROXY and override precedence at +// the resolution layer are covered by TestResolveProxy. +func TestTunnelHonorsEnvProxy(t *testing.T) { + testutil.ClearProxyEnv(t) + edge, got := fakeEdge(t) + proxy := testutil.NewConnectProxyTo(t, edge) + t.Setenv("HTTP_PROXY", "http://"+proxy.Addr) + + runTCPTunnel(t, got, + WithDestinationHost("example.com:9999"), + WithProxyHost("edge.example.com:443")) + + assert.Equal(t, "edge.example.com:443", proxy.Target(), "tunnel should route through HTTP_PROXY") +} + +func TestTunnelFlagOverridesEnv(t *testing.T) { + testutil.ClearProxyEnv(t) + edge, got := fakeEdge(t) + envProxy := testutil.NewConnectProxyTo(t, edge) + flagProxy := testutil.NewConnectProxyTo(t, edge) + t.Setenv("HTTP_PROXY", "http://"+envProxy.Addr) + + runTCPTunnel(t, got, + WithDestinationHost("example.com:9999"), + WithProxyHost("edge.example.com:443"), + WithForwardProxy(flagProxy.Addr)) + + assert.Positive(t, flagProxy.Conns(), "explicit flag proxy should be used") + assert.Zero(t, envProxy.Conns(), "env proxy should be ignored when the flag is set") +} + +func TestTunnelViaSOCKS5(t *testing.T) { + testutil.ClearProxyEnv(t) + edge, got := fakeEdge(t) + proxy := testutil.NewSOCKS5Proxy(t) + + runTCPTunnel(t, got, + WithDestinationHost("example.com:9999"), + WithProxyHost(edge), + WithForwardProxy("socks5://"+proxy.Addr)) + + assert.Equal(t, edge, proxy.Target()) +} + +func TestTunnelUnsupportedProxyScheme(t *testing.T) { + testutil.ClearProxyEnv(t) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + var buf bytes.Buffer + err := New( + WithDestinationHost("example.com:9999"), + WithProxyHost("edge.example.com:443"), + WithForwardProxy("ftp://proxy:21"), + ).Run(ctx, readWriter{strings.NewReader("HELLO WORLD\n"), &buf}, DiscardEvents()) + require.Error(t, err) + assert.Contains(t, err.Error(), "unsupported forward proxy scheme") +} + +func TestTunnelProxyCredentialsNotLogged(t *testing.T) { + testutil.ClearProxyEnv(t) + edge, got := fakeEdge(t) + proxy := testutil.NewConnectProxy(t) + + var logBuf bytes.Buffer + ctx := zerolog.New(&logBuf).WithContext(context.Background()) + ctx, cancel := context.WithTimeout(ctx, 10*time.Second) + defer cancel() + + var buf bytes.Buffer + err := New( + WithDestinationHost("example.com:9999"), + WithProxyHost(edge), + WithForwardProxy("http://user:s3cret@"+proxy.Addr), + ).Run(ctx, readWriter{strings.NewReader("HELLO WORLD\n"), &buf}, DiscardEvents()) + require.NoError(t, err) + + select { + case <-got: + case <-time.After(5 * time.Second): + t.Fatal("backend never received the tunneled data") + } + + assert.Equal(t, "Basic "+base64.StdEncoding.EncodeToString([]byte("user:s3cret")), proxy.ProxyAuth()) + assert.NotContains(t, logBuf.String(), "s3cret", "credentials must never be logged") +} diff --git a/tunnel/tunnel_tcp.go b/tunnel/tunnel_tcp.go index 9aef8c51..bc864997 100644 --- a/tunnel/tunnel_tcp.go +++ b/tunnel/tunnel_tcp.go @@ -34,6 +34,15 @@ func (tun *Tunnel) pickTCPTunneler(ctx context.Context) TCPTunneler { return fallback } + // a forward proxy can't traverse QUIC, so skip the http3 probe entirely. + if proxyURL, err := resolveEdgeProxy(tun.cfg); err != nil { + log.Ctx(ctx).Error().Err(err).Msg("failed to resolve forward proxy, using http1") + return fallback + } else if proxyURL != nil { + log.Ctx(ctx).Info().Msg("forward proxy configured, using http1") + return fallback + } + client := &http.Client{ Transport: httputil.NewLoggingRoundTripper(log.Logger, &http.Transport{ ForceAttemptHTTP2: true, diff --git a/tunnel/tunnel_udp_test.go b/tunnel/tunnel_udp_test.go index 426aec18..0308afda 100644 --- a/tunnel/tunnel_udp_test.go +++ b/tunnel/tunnel_udp_test.go @@ -77,7 +77,7 @@ func TestUDPSessionManager(t *testing.T) { conn, err := net.ListenUDP("udp", localAddr) require.NoError(t, err) - context.AfterFunc(ctx, func() { _ = conn.Close() })() + context.AfterFunc(ctx, func() { _ = conn.Close() }) n, err := conn.WriteToUDP([]byte("SEND HELLO WORLD"), tunnelAddr) assert.Equal(t, 16, n) @@ -97,3 +97,66 @@ func TestUDPSessionManager(t *testing.T) { } assert.NoError(t, err, "tunnel should shutdown cleanly") } + +// The forward proxy is TCP-only: a UDP tunnel dials the edge directly. +func TestTunnelUDPIgnoresForwardProxy(t *testing.T) { + testutil.ClearProxyEnv(t) + proxy := testutil.NewConnectProxy(t) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + tunnelPort := testutil.GetPort(t) + localPort := testutil.GetPort(t) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Transfer-Encoding", "identity") + w.WriteHeader(200) + w.(http.Flusher).Flush() + in, brw, err := w.(http.Hijacker).Hijack() + require.NoError(t, err) + defer in.Close() + payload, err := readUDPCapsuleDatagram(quicvarint.NewReader(in)) + require.NoError(t, err) + require.Equal(t, []byte("\x00SEND HELLO WORLD"), payload) + require.NoError(t, http3.WriteCapsule(quicvarint.NewWriter(brw), 0, []byte("\x00RECV HELLO WORLD"))) + require.NoError(t, brw.Flush()) + <-ctx.Done() + })) + defer srv.Close() + + // an explicit flag proxy: env proxies would bypass a loopback edge. + tun := New( + WithDestinationHost("example.com:9999"), + WithProxyHost(srv.Listener.Addr().String()), + WithForwardProxy(proxy.Addr)) + + tunnelAddr, err := net.ResolveUDPAddr("udp", "127.0.0.1:"+tunnelPort) + require.NoError(t, err) + tunnelConn, err := net.ListenUDP("udp", tunnelAddr) + require.NoError(t, err) + + tunErrC := make(chan error, 1) + go func() { tunErrC <- tun.RunUDPSessionManager(ctx, tunnelConn, LogEvents()) }() + + localAddr, err := net.ResolveUDPAddr("udp", "127.0.0.1:"+localPort) + require.NoError(t, err) + conn, err := net.ListenUDP("udp", localAddr) + require.NoError(t, err) + context.AfterFunc(ctx, func() { _ = conn.Close() }) + + _, err = conn.WriteToUDP([]byte("SEND HELLO WORLD"), tunnelAddr) + require.NoError(t, err) + + payload := make([]byte, maxUDPPacketSize) + n, _, err := conn.ReadFromUDP(payload) + require.NoError(t, err) + assert.Equal(t, []byte("RECV HELLO WORLD"), payload[:n]) + + assert.Zero(t, proxy.Conns(), "UDP tunnel must not use a forward proxy") + + cancel() + if err := <-tunErrC; err != nil && !errors.Is(err, context.Canceled) { + t.Errorf("tunnel should shut down cleanly: %v", err) + } +}