From a68790aee152003ce7db728772a8b447e3757d68 Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Thu, 24 Sep 2026 00:09:43 +0300 Subject: [PATCH 1/4] feat(netproxy): HTTP CONNECT dialer and proxy resolution Add package netproxy so Pilot components can reach their :443 endpoints from hosts whose only egress is an authenticating HTTP proxy that allows CONNECT host:443 (hosted agent sandboxes: UDP blocked, direct TCP killed, local DNS for the Pilot hostnames poisoned). - Resolver picks the proxy per target: Explicit(url), FromEnvironment() (HTTPS_PROXY/https_proxy, fallback ALL_PROXY/all_proxy; HTTP_PROXY for plain http:// requests; NO_PROXY/no_proxy; loopback never proxied) or Off(). Parse("auto"|"off"|URL) maps a -proxy style setting. The same decision is exposed as ProxyForAddr (raw TCP) and ProxyForRequest (http.Transport.Proxy), so TCP dials and HTTP clients agree. - Dialer.DialContext tunnels through the chosen proxy (plain TCP for http://, TLS to the proxy for https://), sends CONNECT host:port with Host and Basic Proxy-Authorization, parses the reply with http.ReadResponse, keeps bytes read ahead of the tunnel, honours ctx cancellation/deadline plus a dial timeout, and closes the conn on every error path. The target is never resolved locally. - Credentials never reach errors or logs: non-2xx replies become ConnectError ("proxy CONNECT host:port: 407 Proxy Authentication Required", status text from net/http, never the proxy's reason phrase); Redact and Resolver.String render http://***@host:port. - NO_PROXY matching follows x/net/http/httpproxy, reimplemented to keep the module dependency-free. Tests run an in-process CONNECT proxy that only knows made-up .invalid host names: success, Basic auth header, 407 with credential-echoing reason phrase, NO_PROXY bypass, https:// proxy, ctx cancel/deadline and Timeout, read-ahead bytes preserved, hostname-only CONNECT, and an http.Transport driven by ProxyForRequest. Co-Authored-By: Claude Opus 5.5 (1M context) --- netproxy/dialer.go | 248 ++++++++++++++++ netproxy/netproxy.go | 341 ++++++++++++++++++++++ netproxy/noproxy.go | 118 ++++++++ netproxy/zz_dialer_test.go | 530 +++++++++++++++++++++++++++++++++++ netproxy/zz_helpers_test.go | 288 +++++++++++++++++++ netproxy/zz_resolver_test.go | 375 +++++++++++++++++++++++++ 6 files changed, 1900 insertions(+) create mode 100644 netproxy/dialer.go create mode 100644 netproxy/netproxy.go create mode 100644 netproxy/noproxy.go create mode 100644 netproxy/zz_dialer_test.go create mode 100644 netproxy/zz_helpers_test.go create mode 100644 netproxy/zz_resolver_test.go diff --git a/netproxy/dialer.go b/netproxy/dialer.go new file mode 100644 index 0000000..44faf40 --- /dev/null +++ b/netproxy/dialer.go @@ -0,0 +1,248 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package netproxy + +import ( + "bufio" + "context" + "crypto/tls" + "encoding/base64" + "errors" + "fmt" + "io" + "net" + "net/http" + "net/url" + "os" + "strings" + "time" +) + +// DefaultTimeout bounds a Dialer's connection establishment when neither +// Dialer.Timeout nor the context sets a tighter limit. +const DefaultTimeout = 30 * time.Second + +// Dialer opens stream connections, tunnelling them through the HTTP CONNECT +// proxy its Resolver picks for each target and dialing directly when there +// is none. The zero value (or a nil *Dialer) always dials directly. +// +// DialContext has the signature of (*net.Dialer).DialContext, so a Dialer +// drops into anything that accepts a dial function: +// +// d := netproxy.NewDialer(resolver) +// conn, err := d.DialContext(ctx, "tcp", "registry.example.net:443") +// +// The target address is sent to the proxy verbatim ("CONNECT host:port") and +// never resolved locally, so it works when local DNS for the target is +// broken or poisoned. Callers that want TLS wrap the returned conn with +// tls.Client themselves; TLS then runs end-to-end with the real server. +type Dialer struct { + // Resolver picks the proxy per target. nil never proxies. + Resolver *Resolver + + // Timeout bounds the whole establishment: the TCP connect to the proxy + // (or to the target when dialing directly), the TLS handshake with an + // https:// proxy, and the CONNECT exchange. Zero means DefaultTimeout. + // An earlier deadline on the DialContext context wins. + Timeout time.Duration + + // Forward dials the proxy itself, and the target when no proxy applies. + // nil uses a net.Dialer. + Forward func(ctx context.Context, network, addr string) (net.Conn, error) + + // TLSConfig configures the TLS session with an https:// proxy (not with + // the target). nil uses defaults; ServerName defaults to the proxy host. + TLSConfig *tls.Config +} + +// NewDialer returns a Dialer that routes connections as r decides. +func NewDialer(r *Resolver) *Dialer { return &Dialer{Resolver: r} } + +// ConnectError is returned when the proxy answers CONNECT with a non-2xx +// status. Its message never includes credentials or proxy-supplied text. +type ConnectError struct { + // Target is the "host:port" the CONNECT asked for. + Target string + // StatusCode is the proxy's HTTP status, e.g. 407 or 403. + StatusCode int +} + +func (e *ConnectError) Error() string { + // Deliberately not the proxy's reason phrase: a hostile or buggy proxy + // could reflect request headers (credentials) into it. + status := fmt.Sprintf("%d", e.StatusCode) + if text := http.StatusText(e.StatusCode); text != "" { + status += " " + text + } + return fmt.Sprintf("proxy CONNECT %s: %s", e.Target, status) +} + +// Dial is DialContext with a background context. +func (d *Dialer) Dial(network, addr string) (net.Conn, error) { + return d.DialContext(context.Background(), network, addr) +} + +// DialContext connects to addr ("host:port") on network, through the proxy +// the Resolver selects for addr or directly when it selects none. Only +// "tcp", "tcp4" and "tcp6" can be tunnelled. +func (d *Dialer) DialContext(ctx context.Context, network, addr string) (net.Conn, error) { + var resolver *Resolver + timeout := DefaultTimeout + if d != nil { + resolver = d.Resolver + if d.Timeout > 0 { + timeout = d.Timeout + } + } + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + proxyURL, err := resolver.ProxyForAddr(addr) + if err != nil { + return nil, err + } + if proxyURL == nil { + return d.forward(ctx, network, addr) + } + switch network { + case "tcp", "tcp4", "tcp6": + default: + return nil, fmt.Errorf("netproxy: cannot tunnel network %q through proxy %s", network, Redact(proxyURL)) + } + return d.dialConnect(ctx, proxyURL, addr) +} + +func (d *Dialer) forward(ctx context.Context, network, addr string) (net.Conn, error) { + if d != nil && d.Forward != nil { + return d.Forward(ctx, network, addr) + } + var nd net.Dialer + return nd.DialContext(ctx, network, addr) +} + +// aLongTimeAgo is a deadline in the past, used to abort blocked I/O. +var aLongTimeAgo = time.Unix(1, 0) + +// dialConnect opens a CONNECT tunnel to addr through proxyURL. ctx must +// carry a deadline; the conn is closed on every error path. +func (d *Dialer) dialConnect(ctx context.Context, proxyURL *url.URL, addr string) (net.Conn, error) { + if err := validTarget(addr); err != nil { + return nil, err + } + conn, err := d.forward(ctx, "tcp", proxyHostPort(proxyURL)) + if err != nil { + return nil, fmt.Errorf("proxy CONNECT %s: dial proxy %s: %w", addr, Redact(proxyURL), err) + } + + // The context bounds the handshake: its deadline becomes the conn + // deadline, and cancellation poisons the deadline to unblock I/O. + if dl, ok := ctx.Deadline(); ok { + conn.SetDeadline(dl) + } + stop := context.AfterFunc(ctx, func() { conn.SetDeadline(aLongTimeAgo) }) + + tunnel, err := d.connect(ctx, conn, proxyURL, addr) + if !stop() && err == nil { + // The context ended after the exchange completed but before the + // deadline guard was disarmed; the conn's deadline is poisoned. + err = ctx.Err() + } + if err != nil { + conn.Close() + var ce *ConnectError + if errors.As(err, &ce) { + return nil, err + } + if ctxErr := ctx.Err(); ctxErr != nil { + err = ctxErr + } else if errors.Is(err, os.ErrDeadlineExceeded) { + err = context.DeadlineExceeded + } + return nil, fmt.Errorf("proxy CONNECT %s via %s: %w", addr, Redact(proxyURL), err) + } + conn.SetDeadline(time.Time{}) + return tunnel, nil +} + +// connect runs the (optional) TLS handshake with the proxy and the CONNECT +// exchange on conn. It does not close conn; dialConnect does on error. +func (d *Dialer) connect(ctx context.Context, conn net.Conn, proxyURL *url.URL, addr string) (net.Conn, error) { + if proxyURL.Scheme == "https" { + var cfg *tls.Config + if d != nil && d.TLSConfig != nil { + cfg = d.TLSConfig.Clone() + } else { + cfg = &tls.Config{MinVersion: tls.VersionTLS12} + } + if cfg.ServerName == "" { + cfg.ServerName = proxyURL.Hostname() + } + tc := tls.Client(conn, cfg) + if err := tc.HandshakeContext(ctx); err != nil { + return nil, fmt.Errorf("TLS handshake with proxy: %w", err) + } + conn = tc + } + + var req strings.Builder + req.WriteString("CONNECT " + addr + " HTTP/1.1\r\n") + req.WriteString("Host: " + addr + "\r\n") + if u := proxyURL.User; u != nil { + password, _ := u.Password() + cred := base64.StdEncoding.EncodeToString([]byte(u.Username() + ":" + password)) + req.WriteString("Proxy-Authorization: Basic " + cred + "\r\n") + } + req.WriteString("\r\n") + if _, err := io.WriteString(conn, req.String()); err != nil { + return nil, fmt.Errorf("write CONNECT: %w", err) + } + + br := bufio.NewReader(conn) + resp, err := http.ReadResponse(br, &http.Request{Method: http.MethodConnect}) + if err != nil { + return nil, fmt.Errorf("read CONNECT response: %w", err) + } + // The body is never read: on success the rest of the stream belongs to + // the tunnel, and on failure the conn is discarded. + if resp.StatusCode < 200 || resp.StatusCode > 299 { + return nil, &ConnectError{Target: addr, StatusCode: resp.StatusCode} + } + if br.Buffered() > 0 { + // The proxy sent tunnel bytes in the same packet as its response + // (e.g. a server that speaks first); keep them. + return &bufferedConn{Conn: conn, r: br}, nil + } + return conn, nil +} + +// validTarget rejects addresses that are not host:port or that would +// corrupt the CONNECT request line (whitespace, control characters). +func validTarget(addr string) error { + host, port, err := net.SplitHostPort(addr) + if err != nil || host == "" || port == "" { + return fmt.Errorf("netproxy: invalid target address %q", addr) + } + for i := 0; i < len(addr); i++ { + if c := addr[i]; c <= ' ' || c == 0x7f { + return fmt.Errorf("netproxy: invalid target address %q", addr) + } + } + return nil +} + +// bufferedConn serves bytes the CONNECT response reader over-read before +// reading from the underlying conn again. +type bufferedConn struct { + net.Conn + r *bufio.Reader +} + +func (c *bufferedConn) Read(p []byte) (int, error) { + if c.r != nil { + if c.r.Buffered() > 0 { + return c.r.Read(p) + } + c.r = nil + } + return c.Conn.Read(p) +} diff --git a/netproxy/netproxy.go b/netproxy/netproxy.go new file mode 100644 index 0000000..0ed447c --- /dev/null +++ b/netproxy/netproxy.go @@ -0,0 +1,341 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +// Package netproxy routes outbound connections through an HTTP CONNECT proxy. +// +// It exists for hosts where the only way out is an egress proxy that allows +// "CONNECT host:443" (hosted agent sandboxes, locked-down corporate VMs): +// outbound UDP is blocked, direct TCP is killed, and local DNS for the target +// hostnames may be poisoned. Everything here therefore keeps the target as a +// HOSTNAME all the way to the proxy — nothing in this package resolves a +// target address locally — and TLS stays end-to-end between the caller and +// the real server (the proxy only ever sees an opaque tunnel). +// +// Two pieces: +// +// - Resolver decides, per target, which proxy (if any) to use: an explicit +// URL, the conventional environment variables (HTTPS_PROXY, ALL_PROXY, +// NO_PROXY, ...), or none. The same decision is exposed as a per-address +// lookup for raw TCP dials (ProxyForAddr) and as an http.Transport.Proxy +// function (ProxyForRequest), so HTTP clients and raw TCP dials follow +// identical rules. +// - Dialer is a DialContext-compatible dialer that tunnels through the +// proxy the Resolver picks, or dials directly when there is none. +// +// Proxy credentials (URL userinfo) are only ever written to the proxy in a +// Proxy-Authorization header. They never appear in errors or in the output +// of Redact / Resolver.String, which is what callers must use for logging. +package netproxy + +import ( + "errors" + "fmt" + "net" + "net/http" + "net/url" + "os" + "strings" +) + +// Mode names. Parse accepts ModeAuto and ModeOff (case-insensitive) in +// addition to an explicit proxy URL; Resolver.Mode reports one of the three. +const ( + // ModeAuto takes the proxy from the environment (see FromEnvironment). + ModeAuto = "auto" + // ModeOff never uses a proxy. + ModeOff = "off" + // ModeExplicit uses one explicitly configured proxy URL for every target. + ModeExplicit = "explicit" +) + +// Resolver decides which proxy, if any, an outbound connection to a given +// target should go through. A nil *Resolver is valid and never proxies. +// Resolvers are immutable and safe for concurrent use. +type Resolver struct { + mode string + + // fixed is the proxy for every target in ModeExplicit. + fixed *url.URL + + // ModeAuto: proxy for TLS / opaque TCP targets (HTTPS_PROXY, https_proxy, + // ALL_PROXY, all_proxy) and the optional override for plain-HTTP request + // URLs (HTTP_PROXY, http_proxy). noProxy lists the bypassed targets. + secure *url.URL + plain *url.URL + noProxy noProxy + // noProxyRaw is kept only for String(). + noProxyRaw string +} + +// Off returns a Resolver that never uses a proxy. +func Off() *Resolver { return &Resolver{mode: ModeOff} } + +// Explicit returns a Resolver that sends every target — including loopback +// and anything NO_PROXY would exempt — through proxyURL. The URL must use +// the http or https scheme ("http://[user:pass@]host[:port]"); a URL with no +// scheme is taken as http. The port defaults to 80 for http and 443 for +// https. +func Explicit(proxyURL string) (*Resolver, error) { + if strings.TrimSpace(proxyURL) == "" { + return nil, errors.New("netproxy: empty proxy URL") + } + u, err := parseProxyURL(proxyURL) + if err != nil { + return nil, err + } + return &Resolver{mode: ModeExplicit, fixed: u}, nil +} + +// FromEnvironment returns a Resolver built from a snapshot of the +// conventional proxy environment variables, taken now: +// +// - TLS and raw TCP targets (ProxyForAddr, and https:// / wss:// requests) +// use the first non-empty of HTTPS_PROXY, https_proxy, ALL_PROXY, +// all_proxy. +// - Plain http:// / ws:// requests use HTTP_PROXY or http_proxy when set, +// and otherwise the same proxy as TLS targets. HTTP_PROXY (upper case) is +// ignored when REQUEST_METHOD is set, as net/http does, so a CGI request +// header cannot inject a proxy. +// - NO_PROXY / no_proxy exempts targets: a comma- or space-separated list +// of host names (matching the name and its subdomains; a leading "." or +// "*." matches subdomains only), IP addresses, CIDR ranges, each +// optionally with ":port", or "*" for everything. localhost and loopback +// addresses are always exempt. +// +// An environment with no proxy variables yields a Resolver that never +// proxies (Enabled reports false). A malformed proxy URL is an error that +// never echoes the URL's credentials. +func FromEnvironment() (*Resolver, error) { + return fromEnv(os.Getenv) +} + +func fromEnv(getenv func(string) string) (*Resolver, error) { + r := &Resolver{mode: ModeAuto} + secureRaw, secureVar := firstEnv(getenv, "HTTPS_PROXY", "https_proxy", "ALL_PROXY", "all_proxy") + if secureRaw != "" { + u, err := parseProxyURL(secureRaw) + if err != nil { + return nil, fmt.Errorf("%s: %w", secureVar, err) + } + r.secure = u + } + plainVars := []string{"HTTP_PROXY", "http_proxy"} + if getenv("REQUEST_METHOD") != "" { + plainVars = plainVars[1:] + } + if plainRaw, plainVar := firstEnv(getenv, plainVars...); plainRaw != "" { + u, err := parseProxyURL(plainRaw) + if err != nil { + return nil, fmt.Errorf("%s: %w", plainVar, err) + } + r.plain = u + } + r.noProxyRaw, _ = firstEnv(getenv, "NO_PROXY", "no_proxy") + r.noProxy = parseNoProxy(r.noProxyRaw) + return r, nil +} + +// Parse builds a Resolver from a -proxy style setting: "auto" (or "") reads +// the environment via FromEnvironment, "off" disables proxying, and anything +// else is an explicit proxy URL (see Explicit). Deciding whether "auto" +// applies at all (for example only in a TCP-only transport mode) is the +// caller's policy; compare the setting against ModeAuto for that. +func Parse(spec string) (*Resolver, error) { + s := strings.TrimSpace(spec) + switch strings.ToLower(s) { + case "", ModeAuto: + return FromEnvironment() + case ModeOff: + return Off(), nil + } + return Explicit(s) +} + +// Mode reports ModeAuto, ModeOff or ModeExplicit. A nil Resolver is ModeOff. +func (r *Resolver) Mode() string { + if r == nil || r.mode == "" { + return ModeOff + } + return r.mode +} + +// Enabled reports whether the Resolver can route any target through a proxy. +// It is false for Off, for a nil Resolver and for an environment without +// proxy variables. +func (r *Resolver) Enabled() bool { + if r == nil { + return false + } + switch r.mode { + case ModeExplicit: + return r.fixed != nil + case ModeAuto: + return r.secure != nil || r.plain != nil + } + return false +} + +// String describes the Resolver for logs, with credentials redacted, e.g. +// "off", "http://***@proxy.internal:3128" or +// "auto: http://***@proxy.internal:3128 (NO_PROXY=localhost,.corp)". +func (r *Resolver) String() string { + switch r.Mode() { + case ModeExplicit: + return Redact(r.fixed) + case ModeAuto: + if !r.Enabled() { + return "auto: no proxy in environment" + } + s := "auto: " + Redact(r.secure) + if r.secure == nil { + s = "auto: http-only " + Redact(r.plain) + } else if r.plain != nil && Redact(r.plain) != Redact(r.secure) { + s += ", http " + Redact(r.plain) + } + if r.noProxyRaw != "" { + s += " (NO_PROXY=" + r.noProxyRaw + ")" + } + return s + } + return ModeOff +} + +// ProxyForAddr returns the proxy to tunnel a raw TCP (or TLS) connection to +// addr ("host:port") through, or nil to dial directly. addr is inspected as +// text only; host names are never resolved. +func (r *Resolver) ProxyForAddr(addr string) (*url.URL, error) { + if !r.Enabled() { + return nil, nil + } + if r.mode == ModeExplicit { + return cloneURL(r.fixed), nil + } + host, port, err := net.SplitHostPort(addr) + if err != nil { + return nil, fmt.Errorf("netproxy: invalid target address %q: %w", addr, err) + } + return cloneURL(r.pick(host, port, true)), nil +} + +// ProxyForRequest returns the proxy for req, or nil for a direct connection. +// Its signature matches http.Transport.Proxy: +// +// tr.Proxy = resolver.ProxyForRequest +// +// https:// and wss:// requests follow exactly the same rules as ProxyForAddr +// (net/http then tunnels them with CONNECT, sending the host name); http:// +// and ws:// requests prefer HTTP_PROXY in ModeAuto. +func (r *Resolver) ProxyForRequest(req *http.Request) (*url.URL, error) { + if !r.Enabled() || req == nil || req.URL == nil { + return nil, nil + } + if r.mode == ModeExplicit { + return cloneURL(r.fixed), nil + } + secure := true + defaultPort := "443" + switch strings.ToLower(req.URL.Scheme) { + case "http", "ws": + secure = false + defaultPort = "80" + } + port := req.URL.Port() + if port == "" { + port = defaultPort + } + return cloneURL(r.pick(req.URL.Hostname(), port, secure)), nil +} + +// pick applies the ModeAuto rules to one target. +func (r *Resolver) pick(host, port string, secure bool) *url.URL { + proxy := r.secure + if !secure && r.plain != nil { + proxy = r.plain + } + if proxy == nil || !r.noProxy.useProxy(host, port) { + return nil + } + return proxy +} + +// Redact renders a proxy URL for logging as scheme://host:port, replacing +// any userinfo (user name and password alike) with "***" and dropping the +// path, query and fragment. Redact(nil) is "". +func Redact(u *url.URL) string { + if u == nil { + return "" + } + var b strings.Builder + if u.Scheme != "" { + b.WriteString(u.Scheme) + b.WriteString("://") + } + if u.User != nil { + b.WriteString("***@") + } + b.WriteString(u.Host) + return b.String() +} + +// parseProxyURL validates a proxy URL. Errors never include the raw value +// when it could carry credentials. +func parseProxyURL(raw string) (*url.URL, error) { + s := strings.TrimSpace(raw) + if !strings.Contains(s, "://") { + // "proxy.internal:3128" / "user:pass@proxy:3128": the scheme is + // conventionally optional and means http. + s = "http://" + s + } + u, err := url.Parse(s) + if err != nil { + if strings.Contains(s, "@") { + return nil, errors.New("netproxy: invalid proxy URL (value withheld: it contains credentials)") + } + var ue *url.Error + if errors.As(err, &ue) { + err = ue.Err + } + return nil, fmt.Errorf("netproxy: invalid proxy URL: %v", err) + } + u.Scheme = strings.ToLower(u.Scheme) + switch u.Scheme { + case "http", "https": + default: + return nil, fmt.Errorf("netproxy: unsupported proxy scheme %q (want http or https)", u.Scheme) + } + if u.Hostname() == "" { + return nil, fmt.Errorf("netproxy: proxy URL %s has no host", Redact(u)) + } + return u, nil +} + +// proxyHostPort is the address to dial for the proxy itself, with the +// scheme's default port filled in. +func proxyHostPort(u *url.URL) string { + port := u.Port() + if port == "" { + port = "80" + if u.Scheme == "https" { + port = "443" + } + } + return net.JoinHostPort(u.Hostname(), port) +} + +func firstEnv(getenv func(string) string, names ...string) (value, name string) { + for _, n := range names { + if v := strings.TrimSpace(getenv(n)); v != "" { + return v, n + } + } + return "", "" +} + +// cloneURL returns a copy so callers cannot mutate the Resolver's URLs. +// url.Userinfo is immutable, so a shallow copy is enough. +func cloneURL(u *url.URL) *url.URL { + if u == nil { + return nil + } + c := *u + return &c +} diff --git a/netproxy/noproxy.go b/netproxy/noproxy.go new file mode 100644 index 0000000..03b4ad7 --- /dev/null +++ b/netproxy/noproxy.go @@ -0,0 +1,118 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package netproxy + +import ( + "net" + "strings" +) + +// noProxy is a parsed NO_PROXY list. The matching rules follow +// golang.org/x/net/http/httpproxy (what net/http.ProxyFromEnvironment uses), +// reimplemented here so the common module stays dependency-free. +type noProxy struct { + all bool + ips []ipMatch + cidrs []*net.IPNet + domains []domainMatch +} + +type ipMatch struct { + ip net.IP + port string // "" matches any port +} + +type domainMatch struct { + // suffix always starts with ".", e.g. ".example.com". + suffix string + port string // "" matches any port + // matchHost also matches the bare domain ("example.com" itself); false + // for entries written as ".example.com" or "*.example.com". + matchHost bool +} + +// parseNoProxy parses a NO_PROXY value. Entries are separated by commas +// and/or whitespace; malformed entries are ignored, as net/http does. +func parseNoProxy(s string) noProxy { + var n noProxy + fields := strings.FieldsFunc(s, func(r rune) bool { + return r == ',' || r == ' ' || r == '\t' || r == '\n' || r == '\r' + }) + for _, p := range fields { + p = strings.ToLower(p) + if p == "*" { + return noProxy{all: true} + } + if _, cidr, err := net.ParseCIDR(p); err == nil { + n.cidrs = append(n.cidrs, cidr) + continue + } + host, port, err := net.SplitHostPort(p) + if err == nil { + if host == "" { + continue + } + } else { + host, port = p, "" + } + host = strings.TrimSuffix(strings.TrimPrefix(host, "["), "]") + if ip := net.ParseIP(host); ip != nil { + n.ips = append(n.ips, ipMatch{ip: ip, port: port}) + continue + } + if host == "" { + continue + } + host = strings.TrimPrefix(host, "*") + matchHost := false + if !strings.HasPrefix(host, ".") { + matchHost = true + host = "." + host + } + if host == "." { + continue + } + n.domains = append(n.domains, domainMatch{suffix: host, port: port, matchHost: matchHost}) + } + return n +} + +// useProxy reports whether a connection to host:port should go through the +// proxy. localhost and loopback addresses never do. +func (n *noProxy) useProxy(host, port string) bool { + host = strings.ToLower(strings.TrimSpace(host)) + if host == "" { + return true + } + if host == "localhost" { + return false + } + ip := net.ParseIP(host) + if ip != nil && ip.IsLoopback() { + return false + } + if n.all { + return false + } + if ip != nil { + for _, m := range n.ips { + if m.ip.Equal(ip) && (m.port == "" || m.port == port) { + return false + } + } + for _, c := range n.cidrs { + if c.Contains(ip) { + return false + } + } + return true + } + for _, m := range n.domains { + if strings.HasSuffix(host, m.suffix) || (m.matchHost && host == m.suffix[1:]) { + if m.port == "" || m.port == port { + return false + } + } + } + return true +} diff --git a/netproxy/zz_dialer_test.go b/netproxy/zz_dialer_test.go new file mode 100644 index 0000000..1561cbc --- /dev/null +++ b/netproxy/zz_dialer_test.go @@ -0,0 +1,530 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package netproxy + +import ( + "bufio" + "context" + "crypto/tls" + "encoding/base64" + "errors" + "io" + "net" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync" + "testing" + "time" +) + +// Targets use the reserved .invalid TLD (RFC 2606): they cannot resolve +// locally, so a successful round-trip proves the dialer handed the proxy +// the host name instead of resolving it. +const ( + echoHost = "echo.pilot.invalid" + bypassHost = "bypass.pilot.invalid" + apiHost = "api.pilot.invalid" +) + +// recordingForward is a Dialer.Forward that records every address it is +// asked to dial and maps the fake host names to loopback. +type recordingForward struct { + mu sync.Mutex + addrs []string + hosts map[string]string +} + +func (f *recordingForward) dial(ctx context.Context, network, addr string) (net.Conn, error) { + f.mu.Lock() + f.addrs = append(f.addrs, addr) + f.mu.Unlock() + host, port, err := net.SplitHostPort(addr) + if err != nil { + return nil, err + } + if real, ok := f.hosts[host]; ok { + addr = net.JoinHostPort(real, port) + } + var d net.Dialer + return d.DialContext(ctx, network, addr) +} + +func (f *recordingForward) seen() []string { + f.mu.Lock() + defer f.mu.Unlock() + return append([]string(nil), f.addrs...) +} + +func roundTrip(t *testing.T, c net.Conn, msg string) { + t.Helper() + c.SetDeadline(time.Now().Add(5 * time.Second)) + if _, err := io.WriteString(c, msg); err != nil { + t.Fatalf("write: %v", err) + } + buf := make([]byte, len(msg)) + if _, err := io.ReadFull(c, buf); err != nil { + t.Fatalf("read: %v", err) + } + if string(buf) != msg { + t.Fatalf("echo mismatch: got %q want %q", buf, msg) + } +} + +func TestDialerTunnelsByHostnameAndNeverResolvesTarget(t *testing.T) { + t.Parallel() + echoIP, echoPort := newEchoServer(t) + proxy := newTestProxy(t, map[string]string{echoHost: echoIP}) + fwd := &recordingForward{} + d := &Dialer{Resolver: mustExplicit(t, proxy.url("")), Forward: fwd.dial} + + target := net.JoinHostPort(echoHost, echoPort) + c, err := d.DialContext(context.Background(), "tcp", target) + if err != nil { + t.Fatalf("DialContext: %v", err) + } + defer c.Close() + roundTrip(t, c, "ping through the tunnel") + + targets, hostHdrs, auths := proxy.seen() + if len(targets) != 1 || targets[0] != target { + t.Fatalf("CONNECT targets = %q, want [%q]", targets, target) + } + if hostHdrs[0] != target { + t.Fatalf("Host header = %q, want %q", hostHdrs[0], target) + } + if auths[0] != "" { + t.Fatalf("unexpected Proxy-Authorization %q for a proxy URL without userinfo", auths[0]) + } + // The only address dialed locally is the proxy's: the target host name + // is never looked up or connected to directly. + if got := fwd.seen(); len(got) != 1 || got[0] != proxy.addr() { + t.Fatalf("local dials = %q, want only the proxy %q", got, proxy.addr()) + } +} + +func TestDialerSendsBasicProxyAuthorization(t *testing.T) { + t.Parallel() + echoIP, echoPort := newEchoServer(t) + const user, pass = "muse-agent", "s3cr3t:with@chars/and%" + proxy := newTestProxy(t, map[string]string{echoHost: echoIP}, withAuth(user, pass)) + userinfo := url.UserPassword(user, pass).String() + d := NewDialer(mustExplicit(t, proxy.url(userinfo))) + + c, err := d.DialContext(context.Background(), "tcp", net.JoinHostPort(echoHost, echoPort)) + if err != nil { + t.Fatalf("DialContext: %v", err) + } + defer c.Close() + roundTrip(t, c, "authenticated") + + _, _, auths := proxy.seen() + want := "Basic " + base64.StdEncoding.EncodeToString([]byte(user+":"+pass)) + if len(auths) != 1 || auths[0] != want { + t.Fatalf("Proxy-Authorization = %q, want %q", auths, want) + } +} + +func TestDialerRejectedCONNECTNeverLeaksCredentials(t *testing.T) { + t.Parallel() + const user, pass = "leaky-user", "hunter2-password" + encoded := base64.StdEncoding.EncodeToString([]byte(user + ":" + pass)) + // The proxy echoes the password and the encoded header in its reason + // phrase; none of it may reach the error. + proxy := newTestProxy(t, nil, withReject(http.StatusProxyAuthRequired, "Denied "+user+" "+pass+" "+encoded)) + d := NewDialer(mustExplicit(t, proxy.url(user+":"+pass))) + + target := net.JoinHostPort(echoHost, "443") + _, err := d.DialContext(context.Background(), "tcp", target) + if err == nil { + t.Fatal("expected an error for a 407 response") + } + want := "proxy CONNECT " + target + ": 407 Proxy Authentication Required" + if err.Error() != want { + t.Fatalf("error = %q, want %q", err.Error(), want) + } + var ce *ConnectError + if !errors.As(err, &ce) || ce.StatusCode != http.StatusProxyAuthRequired || ce.Target != target { + t.Fatalf("error %#v is not a ConnectError{407, %q}", err, target) + } + for _, secret := range []string{user, pass, encoded} { + if strings.Contains(err.Error(), secret) { + t.Fatalf("error %q leaks %q", err.Error(), secret) + } + } + waitFor(t, "client to close the rejected conn", func() bool { return proxy.clientClosed.Load() == 1 }) +} + +func TestDialerNon2xxStatuses(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + status int + want string + }{ + {http.StatusForbidden, "403 Forbidden"}, + {http.StatusBadGateway, "502 Bad Gateway"}, + {599, "599"}, + } { + proxy := newTestProxy(t, nil, withReject(tc.status, "whatever the proxy says")) + _, err := NewDialer(mustExplicit(t, proxy.url(""))).Dial("tcp", "registry.pilot.invalid:443") + if err == nil || err.Error() != "proxy CONNECT registry.pilot.invalid:443: "+tc.want { + t.Fatalf("status %d: error = %v, want suffix %q", tc.status, err, tc.want) + } + } +} + +func TestDialerProxyUnreachableNeverLeaksCredentials(t *testing.T) { + t.Parallel() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + dead := ln.Addr().String() + ln.Close() + + d := NewDialer(mustExplicit(t, "http://someone:topsecret@"+dead)) + _, err = d.DialContext(context.Background(), "tcp", "registry.pilot.invalid:443") + if err == nil { + t.Fatal("expected a dial error") + } + msg := err.Error() + if strings.Contains(msg, "someone") || strings.Contains(msg, "topsecret") { + t.Fatalf("error leaks credentials: %q", msg) + } + if !strings.Contains(msg, "http://***@"+dead) || !strings.Contains(msg, "registry.pilot.invalid:443") { + t.Fatalf("error should name the redacted proxy and the target: %q", msg) + } +} + +func TestDialerHonorsNoProxy(t *testing.T) { + t.Parallel() + echoIP, echoPort := newEchoServer(t) + proxy := newTestProxy(t, map[string]string{echoHost: echoIP}) + r, err := fromEnv(envMap(map[string]string{ + "HTTPS_PROXY": proxy.url("u:p"), + "NO_PROXY": "localhost, " + bypassHost, + })) + if err != nil { + t.Fatalf("fromEnv: %v", err) + } + fwd := &recordingForward{hosts: map[string]string{bypassHost: echoIP, "api." + bypassHost: echoIP}} + d := &Dialer{Resolver: r, Forward: fwd.dial} + + for _, host := range []string{bypassHost, "api." + bypassHost} { + c, err := d.DialContext(context.Background(), "tcp", net.JoinHostPort(host, echoPort)) + if err != nil { + t.Fatalf("direct dial %s: %v", host, err) + } + roundTrip(t, c, "direct") + c.Close() + } + if targets, _, _ := proxy.seen(); len(targets) != 0 { + t.Fatalf("NO_PROXY targets went through the proxy: %q", targets) + } + + c, err := d.DialContext(context.Background(), "tcp", net.JoinHostPort(echoHost, echoPort)) + if err != nil { + t.Fatalf("proxied dial: %v", err) + } + roundTrip(t, c, "proxied") + c.Close() + targets, _, _ := proxy.seen() + if len(targets) != 1 || targets[0] != net.JoinHostPort(echoHost, echoPort) { + t.Fatalf("proxy targets = %q", targets) + } + want := []string{ + net.JoinHostPort(bypassHost, echoPort), + net.JoinHostPort("api."+bypassHost, echoPort), + proxy.addr(), + } + if got := fwd.seen(); strings.Join(got, ",") != strings.Join(want, ",") { + t.Fatalf("local dials = %q, want %q", got, want) + } +} + +func TestDialerHTTPSProxy(t *testing.T) { + t.Parallel() + echoIP, echoPort := newEchoServer(t) + cert, pool := genCert(t, []string{"proxy.pilot.invalid"}, []net.IP{net.ParseIP("127.0.0.1")}) + proxy := newTestProxy(t, map[string]string{echoHost: echoIP}, withTLS(cert), withAuth("u", "p")) + target := net.JoinHostPort(echoHost, echoPort) + + // ServerName defaults to the proxy host (here the IP in the URL). + d := &Dialer{ + Resolver: mustExplicit(t, "https://u:p@"+proxy.addr()), + TLSConfig: &tls.Config{RootCAs: pool}, + } + c, err := d.DialContext(context.Background(), "tcp", target) + if err != nil { + t.Fatalf("DialContext via https proxy: %v", err) + } + roundTrip(t, c, "tls to the proxy, then a tunnel") + c.Close() + + // A proxy certificate the client does not trust fails closed, and the + // error still carries no credentials. + d.TLSConfig = nil + _, err = d.DialContext(context.Background(), "tcp", target) + if err == nil { + t.Fatal("expected an x509 error without the test root") + } + var cve *tls.CertificateVerificationError + if !errors.As(err, &cve) { + t.Fatalf("expected a certificate verification error, got %v", err) + } + if strings.Contains(err.Error(), "u:p") { + t.Fatalf("error leaks credentials: %v", err) + } +} + +func TestDialerContextDeadline(t *testing.T) { + t.Parallel() + proxy := newTestProxy(t, nil, withHang()) + d := NewDialer(mustExplicit(t, proxy.url(""))) + + ctx, cancel := context.WithTimeout(context.Background(), 150*time.Millisecond) + defer cancel() + start := time.Now() + _, err := d.DialContext(ctx, "tcp", "registry.pilot.invalid:443") + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("error = %v, want context.DeadlineExceeded", err) + } + if elapsed := time.Since(start); elapsed > 3*time.Second { + t.Fatalf("deadline not honoured: took %v", elapsed) + } + waitFor(t, "client to close the timed-out conn", func() bool { return proxy.clientClosed.Load() == 1 }) +} + +func TestDialerContextCancel(t *testing.T) { + t.Parallel() + proxy := newTestProxy(t, nil, withHang()) + d := NewDialer(mustExplicit(t, proxy.url(""))) + + ctx, cancel := context.WithCancel(context.Background()) + time.AfterFunc(100*time.Millisecond, cancel) + _, err := d.DialContext(ctx, "tcp", "registry.pilot.invalid:443") + if !errors.Is(err, context.Canceled) { + t.Fatalf("error = %v, want context.Canceled", err) + } + waitFor(t, "client to close the cancelled conn", func() bool { return proxy.clientClosed.Load() == 1 }) +} + +func TestDialerTimeoutField(t *testing.T) { + t.Parallel() + proxy := newTestProxy(t, nil, withHang()) + d := &Dialer{Resolver: mustExplicit(t, proxy.url("")), Timeout: 150 * time.Millisecond} + start := time.Now() + _, err := d.Dial("tcp", "registry.pilot.invalid:443") + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("error = %v, want context.DeadlineExceeded", err) + } + if elapsed := time.Since(start); elapsed > 3*time.Second { + t.Fatalf("Timeout not honoured: took %v", elapsed) + } +} + +func TestDialerAlreadyCancelledContext(t *testing.T) { + t.Parallel() + proxy := newTestProxy(t, nil) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + _, err := NewDialer(mustExplicit(t, proxy.url(""))).DialContext(ctx, "tcp", "registry.pilot.invalid:443") + if !errors.Is(err, context.Canceled) { + t.Fatalf("error = %v, want context.Canceled", err) + } +} + +// TestDialerPreservesBytesReadAheadOfCONNECTResponse uses net.Pipe so the +// proxy's response and the first tunnel bytes arrive in one read, +// deterministically: the bufio reader behind http.ReadResponse swallows +// them and the returned conn must hand them back. +func TestDialerPreservesBytesReadAheadOfCONNECTResponse(t *testing.T) { + t.Parallel() + clientEnd, proxyEnd := net.Pipe() + defer proxyEnd.Close() + go func() { + br := bufio.NewReader(proxyEnd) + req, err := http.ReadRequest(br) + if err != nil || req.Method != http.MethodConnect { + proxyEnd.Close() + return + } + // One write: response head immediately followed by a server greeting. + io.WriteString(proxyEnd, "HTTP/1.1 200 Connection established\r\n\r\nHELLO") + io.Copy(proxyEnd, br) // then echo + }() + d := &Dialer{ + Resolver: mustExplicit(t, "http://proxy.pilot.invalid:3128"), + Forward: func(ctx context.Context, network, addr string) (net.Conn, error) { + if addr != "proxy.pilot.invalid:3128" { + t.Errorf("forward dial to %q, want the proxy", addr) + } + return clientEnd, nil + }, + } + c, err := d.DialContext(context.Background(), "tcp", "server-speaks-first.pilot.invalid:443") + if err != nil { + t.Fatalf("DialContext: %v", err) + } + defer c.Close() + if _, ok := c.(*bufferedConn); !ok { + t.Fatalf("conn type %T, want *bufferedConn when bytes were read ahead", c) + } + c.SetDeadline(time.Now().Add(5 * time.Second)) + greeting := make([]byte, 5) + if _, err := io.ReadFull(c, greeting); err != nil || string(greeting) != "HELLO" { + t.Fatalf("greeting = %q, %v; want HELLO", greeting, err) + } + roundTrip(t, c, "and the stream continues") +} + +func TestDialerReturnsRawConnWithoutReadAhead(t *testing.T) { + t.Parallel() + echoIP, echoPort := newEchoServer(t) + proxy := newTestProxy(t, map[string]string{echoHost: echoIP}) + c, err := NewDialer(mustExplicit(t, proxy.url(""))).Dial("tcp", net.JoinHostPort(echoHost, echoPort)) + if err != nil { + t.Fatalf("Dial: %v", err) + } + defer c.Close() + if _, ok := c.(*net.TCPConn); !ok { + t.Fatalf("conn type %T, want the raw *net.TCPConn", c) + } + if dl := deadlineCleared(t, c); !dl { + t.Fatal("handshake deadline was left on the conn") + } +} + +// deadlineCleared checks that a blocking read is not cut short by a stale +// handshake deadline (the echo server sends nothing unprompted). +func deadlineCleared(t *testing.T, c net.Conn) bool { + t.Helper() + errc := make(chan error, 1) + go func() { + _, err := c.Read(make([]byte, 1)) + errc <- err + }() + select { + case err := <-errc: + t.Logf("read returned early: %v", err) + return false + case <-time.After(200 * time.Millisecond): + c.SetReadDeadline(time.Now()) // unblock the reader + <-errc + return true + } +} + +func TestDialerDirectWithoutProxy(t *testing.T) { + t.Parallel() + for name, r := range map[string]*Resolver{"nil": nil, "off": Off()} { + fwd := &recordingForward{hosts: map[string]string{"direct.pilot.invalid": "127.0.0.1"}} + echoIP, echoPort := newEchoServer(t) + fwd.hosts["direct.pilot.invalid"] = echoIP + d := &Dialer{Resolver: r, Forward: fwd.dial} + c, err := d.Dial("tcp", net.JoinHostPort("direct.pilot.invalid", echoPort)) + if err != nil { + t.Fatalf("%s: Dial: %v", name, err) + } + roundTrip(t, c, "direct") + c.Close() + if got := fwd.seen(); len(got) != 1 || got[0] != net.JoinHostPort("direct.pilot.invalid", echoPort) { + t.Fatalf("%s: forward dials = %q", name, got) + } + } + + // A nil *Dialer dials directly too. + echoIP, echoPort := newEchoServer(t) + var nd *Dialer + c, err := nd.Dial("tcp", net.JoinHostPort(echoIP, echoPort)) + if err != nil { + t.Fatalf("nil Dialer: %v", err) + } + c.Close() +} + +func TestDialerRejectsBadTargetsBeforeDialing(t *testing.T) { + t.Parallel() + d := &Dialer{ + Resolver: mustExplicit(t, "http://proxy.pilot.invalid:3128"), + Forward: func(context.Context, string, string) (net.Conn, error) { + t.Error("forward dialer must not be called for a rejected target") + return nil, errors.New("unreachable") + }, + } + for _, addr := range []string{ + "no-port.pilot.invalid", + ":443", + "evil.pilot.invalid:443\r\nX-Injected: 1", + "evil.pilot.invalid:443 HTTP/1.0", + } { + if _, err := d.Dial("tcp", addr); err == nil || !strings.Contains(err.Error(), "invalid target address") { + t.Fatalf("Dial(%q) error = %v, want invalid target address", addr, err) + } + } + if _, err := d.Dial("udp", "dns.pilot.invalid:53"); err == nil || !strings.Contains(err.Error(), `cannot tunnel network "udp"`) { + t.Fatalf("udp through proxy: error = %v", err) + } +} + +// TestProxyForRequestDrivesHTTPTransport shows that an http.Transport using +// ProxyForRequest takes the same path as the Dialer: CONNECT by host name, +// Basic credentials from the URL, TLS end-to-end with the real server. +func TestProxyForRequestDrivesHTTPTransport(t *testing.T) { + t.Parallel() + cert, pool := genCert(t, []string{apiHost}, nil) + srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + io.WriteString(w, "hello "+r.Host) + })) + srv.TLS = &tls.Config{Certificates: []tls.Certificate{cert}} + srv.StartTLS() + defer srv.Close() + srvIP, srvPort, _ := net.SplitHostPort(srv.Listener.Addr().String()) + + proxy := newTestProxy(t, map[string]string{apiHost: srvIP}, withAuth("agent", "pw")) + r, err := fromEnv(envMap(map[string]string{"HTTPS_PROXY": proxy.url("agent:pw")})) + if err != nil { + t.Fatalf("fromEnv: %v", err) + } + tr := &http.Transport{Proxy: r.ProxyForRequest, TLSClientConfig: &tls.Config{RootCAs: pool}} + defer tr.CloseIdleConnections() + client := &http.Client{Transport: tr, Timeout: 10 * time.Second} + + target := net.JoinHostPort(apiHost, srvPort) + resp, err := client.Get("https://" + target + "/v1") + if err != nil { + t.Fatalf("GET via proxy: %v", err) + } + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + if resp.StatusCode != http.StatusOK || string(body) != "hello "+target { + t.Fatalf("response %d %q", resp.StatusCode, body) + } + targets, _, auths := proxy.seen() + if len(targets) != 1 || targets[0] != target { + t.Fatalf("CONNECT targets = %q, want [%q]", targets, target) + } + if auths[0] != basicAuth("agent", "pw") { + t.Fatalf("Proxy-Authorization = %q", auths[0]) + } + // Same decision the Dialer would make for the same target. + fromAddr, _ := r.ProxyForAddr(target) + req, _ := http.NewRequest(http.MethodGet, "https://"+target, nil) + fromReq, _ := r.ProxyForRequest(req) + if fromAddr.String() != fromReq.String() { + t.Fatalf("ProxyForAddr %v != ProxyForRequest %v", fromAddr, fromReq) + } +} + +func waitFor(t *testing.T, what string, cond func() bool) { + t.Helper() + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if cond() { + return + } + time.Sleep(10 * time.Millisecond) + } + t.Fatalf("timed out waiting for %s", what) +} diff --git a/netproxy/zz_helpers_test.go b/netproxy/zz_helpers_test.go new file mode 100644 index 0000000..62673ca --- /dev/null +++ b/netproxy/zz_helpers_test.go @@ -0,0 +1,288 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package netproxy + +import ( + "bufio" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "fmt" + "io" + "math/big" + "net" + "net/http" + "sync" + "sync/atomic" + "testing" + "time" +) + +// Test fixtures shared by the netproxy tests: an in-process HTTP CONNECT +// proxy that maps made-up host names (which never resolve locally) to +// loopback, an echo server, and a throwaway certificate generator. + +// testProxy is a minimal CONNECT proxy. Targets are only reachable under +// the names in hosts, which is how the tests prove that the client asked +// for a host name and never resolved it itself. +type testProxy struct { + ln net.Listener + hosts map[string]string // target host name -> real host to dial + + // wantAuth, when set, is the only Proxy-Authorization value accepted; + // anything else gets rejectStatus (407 by default) with rejectReason. + wantAuth string + rejectStatus int + rejectReason string + // hang makes the proxy read the CONNECT request and never answer. + hang bool + + mu sync.Mutex + targets []string // CONNECT request-targets, in arrival order + hostHdrs []string // Host header values, in arrival order + auths []string // Proxy-Authorization values, in arrival order + + // clientClosed counts rejected/hung requests after which the client + // closed its side of the connection. + clientClosed atomic.Int32 + wg sync.WaitGroup + live map[net.Conn]struct{} // guarded by mu; closed on cleanup +} + +type proxyOpt func(*testProxy) + +func withAuth(user, pass string) proxyOpt { + return func(p *testProxy) { p.wantAuth = basicAuth(user, pass) } +} + +func withReject(status int, reason string) proxyOpt { + return func(p *testProxy) { p.wantAuth = "never-matches"; p.rejectStatus = status; p.rejectReason = reason } +} + +func withHang() proxyOpt { return func(p *testProxy) { p.hang = true } } + +func withTLS(cert tls.Certificate) proxyOpt { + return func(p *testProxy) { + p.ln = tls.NewListener(p.ln, &tls.Config{Certificates: []tls.Certificate{cert}, MinVersion: tls.VersionTLS12}) + } +} + +func newTestProxy(t *testing.T, hosts map[string]string, opts ...proxyOpt) *testProxy { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("proxy listen: %v", err) + } + p := &testProxy{ln: ln, hosts: hosts, rejectStatus: http.StatusProxyAuthRequired, rejectReason: "Proxy Authentication Required"} + for _, o := range opts { + o(p) + } + p.wg.Add(1) + go p.acceptLoop() + t.Cleanup(func() { + p.ln.Close() + p.mu.Lock() + for c := range p.live { + c.Close() + } + p.mu.Unlock() + p.wg.Wait() + }) + return p +} + +func (p *testProxy) track(c net.Conn) { + p.mu.Lock() + if p.live == nil { + p.live = make(map[net.Conn]struct{}) + } + p.live[c] = struct{}{} + p.mu.Unlock() +} + +func (p *testProxy) addr() string { return p.ln.Addr().String() } + +func (p *testProxy) url(userinfo string) string { + if userinfo != "" { + return "http://" + userinfo + "@" + p.addr() + } + return "http://" + p.addr() +} + +func (p *testProxy) seen() (targets, hostHdrs, auths []string) { + p.mu.Lock() + defer p.mu.Unlock() + return append([]string(nil), p.targets...), append([]string(nil), p.hostHdrs...), append([]string(nil), p.auths...) +} + +func (p *testProxy) acceptLoop() { + defer p.wg.Done() + for { + conn, err := p.ln.Accept() + if err != nil { + return + } + p.track(conn) + p.wg.Add(1) + go func() { + defer p.wg.Done() + p.serve(conn) + }() + } +} + +func (p *testProxy) serve(conn net.Conn) { + defer conn.Close() + conn.SetDeadline(time.Now().Add(10 * time.Second)) + br := bufio.NewReader(conn) + req, err := http.ReadRequest(br) + if err != nil { + return + } + p.mu.Lock() + p.targets = append(p.targets, req.RequestURI) + p.hostHdrs = append(p.hostHdrs, req.Host) + p.auths = append(p.auths, req.Header.Get("Proxy-Authorization")) + p.mu.Unlock() + + waitClientClose := func() { + if _, err := br.ReadByte(); err == io.EOF { + p.clientClosed.Add(1) + } + } + if p.hang { + waitClientClose() + return + } + if req.Method != http.MethodConnect { + fmt.Fprintf(conn, "HTTP/1.1 405 Method Not Allowed\r\nContent-Length: 0\r\n\r\n") + return + } + if p.wantAuth != "" && req.Header.Get("Proxy-Authorization") != p.wantAuth { + fmt.Fprintf(conn, "HTTP/1.1 %d %s\r\nProxy-Authenticate: Basic realm=\"test\"\r\nContent-Length: 0\r\n\r\n", p.rejectStatus, p.rejectReason) + waitClientClose() + return + } + host, port, err := net.SplitHostPort(req.RequestURI) + real, ok := p.hosts[host] + if err != nil || !ok { + fmt.Fprintf(conn, "HTTP/1.1 502 Bad Gateway\r\nContent-Length: 0\r\n\r\n") + return + } + up, err := net.DialTimeout("tcp", net.JoinHostPort(real, port), 5*time.Second) + if err != nil { + fmt.Fprintf(conn, "HTTP/1.1 502 Bad Gateway\r\nContent-Length: 0\r\n\r\n") + return + } + defer up.Close() + p.track(up) + if _, err := io.WriteString(conn, "HTTP/1.1 200 Connection established\r\n\r\n"); err != nil { + return + } + conn.SetDeadline(time.Time{}) + pipe(conn, br, up) +} + +// pipe copies client<->upstream until either side closes. clientR carries +// any bytes already buffered from the client. +func pipe(client net.Conn, clientR io.Reader, up net.Conn) { + done := make(chan struct{}, 2) + go func() { io.Copy(up, clientR); closeWrite(up); done <- struct{}{} }() + go func() { io.Copy(client, up); closeWrite(client); done <- struct{}{} }() + <-done + <-done +} + +func closeWrite(c net.Conn) { + if cw, ok := c.(interface{ CloseWrite() error }); ok { + cw.CloseWrite() + return + } + c.Close() +} + +// newEchoServer echoes every byte back on each connection. +func newEchoServer(t *testing.T) (host, port string) { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("echo listen: %v", err) + } + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + for { + c, err := ln.Accept() + if err != nil { + return + } + wg.Add(1) + go func() { + defer wg.Done() + defer c.Close() + io.Copy(c, c) + }() + } + }() + t.Cleanup(func() { ln.Close(); wg.Wait() }) + h, p, _ := net.SplitHostPort(ln.Addr().String()) + return h, p +} + +// genCert returns a self-signed leaf for the given names/IPs plus a pool +// that trusts it. +func genCert(t *testing.T, dnsNames []string, ips []net.IP) (tls.Certificate, *x509.CertPool) { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("genkey: %v", err) + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(time.Now().UnixNano()), + Subject: pkix.Name{CommonName: "netproxy-test"}, + 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, + DNSNames: dnsNames, + IPAddresses: ips, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatalf("create cert: %v", err) + } + leaf, err := x509.ParseCertificate(der) + if err != nil { + t.Fatalf("parse cert: %v", err) + } + pool := x509.NewCertPool() + pool.AddCert(leaf) + return tls.Certificate{Certificate: [][]byte{der}, PrivateKey: key, Leaf: leaf}, pool +} + +func basicAuth(user, pass string) string { + req := &http.Request{Header: http.Header{}} + req.SetBasicAuth(user, pass) + return req.Header.Get("Authorization") +} + +// mustExplicit is Explicit for tests. +func mustExplicit(t *testing.T, raw string) *Resolver { + t.Helper() + r, err := Explicit(raw) + if err != nil { + t.Fatalf("Explicit: %v", err) + } + return r +} + +// envMap turns a map into a getenv function for fromEnv. +func envMap(m map[string]string) func(string) string { + return func(k string) string { return m[k] } +} diff --git a/netproxy/zz_resolver_test.go b/netproxy/zz_resolver_test.go new file mode 100644 index 0000000..9288f98 --- /dev/null +++ b/netproxy/zz_resolver_test.go @@ -0,0 +1,375 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package netproxy + +import ( + "net/http" + "net/url" + "strings" + "testing" +) + +func proxyFor(t *testing.T, r *Resolver, addr string) string { + t.Helper() + u, err := r.ProxyForAddr(addr) + if err != nil { + t.Fatalf("ProxyForAddr(%q): %v", addr, err) + } + if u == nil { + return "" + } + return u.String() +} + +func proxyForURL(t *testing.T, r *Resolver, rawURL string) string { + t.Helper() + req, err := http.NewRequest(http.MethodGet, rawURL, nil) + if err != nil { + t.Fatalf("NewRequest(%q): %v", rawURL, err) + } + u, err := r.ProxyForRequest(req) + if err != nil { + t.Fatalf("ProxyForRequest(%q): %v", rawURL, err) + } + if u == nil { + return "" + } + return u.String() +} + +func TestFromEnvPrecedence(t *testing.T) { + t.Parallel() + const target = "registry.pilotprotocol.network:443" + cases := []struct { + name string + env map[string]string + want string + }{ + {"none", map[string]string{}, ""}, + {"HTTPS_PROXY", map[string]string{"HTTPS_PROXY": "http://a:1", "https_proxy": "http://b:1", "ALL_PROXY": "http://c:1"}, "http://a:1"}, + {"https_proxy", map[string]string{"https_proxy": "http://b:1", "ALL_PROXY": "http://c:1"}, "http://b:1"}, + {"ALL_PROXY fallback", map[string]string{"ALL_PROXY": "http://c:1", "all_proxy": "http://d:1"}, "http://c:1"}, + {"all_proxy fallback", map[string]string{"all_proxy": "http://d:1"}, "http://d:1"}, + {"blank values skipped", map[string]string{"HTTPS_PROXY": " ", "all_proxy": "http://d:1"}, "http://d:1"}, + {"HTTP_PROXY alone does not cover TCP/TLS targets", map[string]string{"HTTP_PROXY": "http://h:1"}, ""}, + {"scheme-less value means http", map[string]string{"HTTPS_PROXY": "proxy.internal:3128"}, "http://proxy.internal:3128"}, + } + for _, tc := range cases { + r, err := fromEnv(envMap(tc.env)) + if err != nil { + t.Fatalf("%s: fromEnv: %v", tc.name, err) + } + if r.Mode() != ModeAuto { + t.Fatalf("%s: mode %q", tc.name, r.Mode()) + } + if got := proxyFor(t, r, target); got != tc.want { + t.Fatalf("%s: ProxyForAddr = %q, want %q", tc.name, got, tc.want) + } + if got := proxyForURL(t, r, "https://"+target+"/x"); got != tc.want { + t.Fatalf("%s: ProxyForRequest(https) = %q, want %q", tc.name, got, tc.want) + } + if got := proxyForURL(t, r, "wss://"+target+"/x"); got != tc.want { + t.Fatalf("%s: ProxyForRequest(wss) = %q, want %q", tc.name, got, tc.want) + } + } +} + +func TestFromEnvPlainHTTPRequests(t *testing.T) { + t.Parallel() + r, err := fromEnv(envMap(map[string]string{"HTTPS_PROXY": "http://secure:1", "HTTP_PROXY": "http://plain:1"})) + if err != nil { + t.Fatal(err) + } + if got := proxyForURL(t, r, "http://example.pilot.invalid/"); got != "http://plain:1" { + t.Fatalf("http:// request proxy = %q, want HTTP_PROXY", got) + } + if got := proxyForURL(t, r, "ws://example.pilot.invalid/"); got != "http://plain:1" { + t.Fatalf("ws:// request proxy = %q, want HTTP_PROXY", got) + } + if got := proxyForURL(t, r, "https://example.pilot.invalid/"); got != "http://secure:1" { + t.Fatalf("https:// request proxy = %q, want HTTPS_PROXY", got) + } + + // Without HTTP_PROXY, plain requests share the TLS proxy: in a sandbox + // where direct egress is killed, "no proxy" is never the safer default. + r, _ = fromEnv(envMap(map[string]string{"HTTPS_PROXY": "http://secure:1"})) + if got := proxyForURL(t, r, "http://example.pilot.invalid/"); got != "http://secure:1" { + t.Fatalf("http:// request proxy = %q, want the HTTPS_PROXY fallback", got) + } + + // CGI: a request header can set HTTP_PROXY, so it is ignored when + // REQUEST_METHOD is present; lower-case http_proxy still counts. + r, _ = fromEnv(envMap(map[string]string{"REQUEST_METHOD": "GET", "HTTP_PROXY": "http://evil:1"})) + if r.Enabled() { + t.Fatalf("HTTP_PROXY honoured under CGI: %s", r) + } + r, _ = fromEnv(envMap(map[string]string{"REQUEST_METHOD": "GET", "HTTP_PROXY": "http://evil:1", "http_proxy": "http://ok:1"})) + if got := proxyForURL(t, r, "http://example.pilot.invalid/"); got != "http://ok:1" { + t.Fatalf("CGI http_proxy = %q", got) + } +} + +func TestFromEnvironmentReadsProcessEnv(t *testing.T) { + t.Setenv("HTTPS_PROXY", "http://user:pw@proxy.pilot.invalid:3128") + t.Setenv("NO_PROXY", ".internal") + r, err := FromEnvironment() + if err != nil { + t.Fatalf("FromEnvironment: %v", err) + } + if got := proxyFor(t, r, "registry.pilotprotocol.network:443"); got != "http://user:pw@proxy.pilot.invalid:3128" { + t.Fatalf("proxy = %q", got) + } + if got := proxyFor(t, r, "db.internal:5432"); got != "" { + t.Fatalf("NO_PROXY target proxied via %q", got) + } + // Parse("auto") and Parse("") are the same thing. + for _, spec := range []string{"auto", "AUTO", " auto ", ""} { + p, err := Parse(spec) + if err != nil || p.Mode() != ModeAuto || !p.Enabled() { + t.Fatalf("Parse(%q) = %v, %v", spec, p, err) + } + } +} + +func TestAutoAlwaysBypassesLoopback(t *testing.T) { + t.Parallel() + r, _ := fromEnv(envMap(map[string]string{"HTTPS_PROXY": "http://p:1"})) + for _, addr := range []string{"localhost:443", "LOCALHOST:1", "127.0.0.1:9000", "127.9.9.9:1", "[::1]:443"} { + if got := proxyFor(t, r, addr); got != "" { + t.Fatalf("%s proxied via %q", addr, got) + } + } + if got := proxyFor(t, r, "10.0.0.1:443"); got != "http://p:1" { + t.Fatalf("non-loopback IP not proxied: %q", got) + } +} + +func TestExplicitProxiesEverything(t *testing.T) { + t.Parallel() + r := mustExplicit(t, "http://user:pw@proxy.internal:3128") + if r.Mode() != ModeExplicit || !r.Enabled() { + t.Fatalf("mode %q enabled %v", r.Mode(), r.Enabled()) + } + for _, addr := range []string{"registry.pilotprotocol.network:443", "localhost:1", "127.0.0.1:9000"} { + if got := proxyFor(t, r, addr); got != "http://user:pw@proxy.internal:3128" { + t.Fatalf("%s: proxy %q", addr, got) + } + } + if got := proxyForURL(t, r, "http://plain.pilot.invalid/"); got != "http://user:pw@proxy.internal:3128" { + t.Fatalf("plain request: proxy %q", got) + } + // Returned URLs are copies. + u, _ := r.ProxyForAddr("a.invalid:1") + u.Host = "mutated:1" + if got := proxyFor(t, r, "a.invalid:1"); !strings.Contains(got, "proxy.internal:3128") { + t.Fatalf("resolver state mutated through a returned URL: %q", got) + } +} + +func TestOffAndNilNeverProxy(t *testing.T) { + t.Parallel() + var nilResolver *Resolver + off, err := Parse(" OFF ") + if err != nil { + t.Fatal(err) + } + for name, r := range map[string]*Resolver{"nil": nilResolver, "Off()": Off(), "Parse(off)": off} { + if r.Enabled() || r.Mode() != ModeOff || r.String() != "off" { + t.Fatalf("%s: enabled=%v mode=%q string=%q", name, r.Enabled(), r.Mode(), r.String()) + } + if got := proxyFor(t, r, "registry.pilotprotocol.network:443"); got != "" { + t.Fatalf("%s: proxied via %q", name, got) + } + if got := proxyForURL(t, r, "https://registry.pilotprotocol.network/"); got != "" { + t.Fatalf("%s: request proxied via %q", name, got) + } + if u, err := r.ProxyForRequest(nil); u != nil || err != nil { + t.Fatalf("%s: ProxyForRequest(nil) = %v, %v", name, u, err) + } + } +} + +func TestParseExplicitURLs(t *testing.T) { + t.Parallel() + cases := map[string]string{ + "http://proxy:3128": "http://proxy:3128", + "HTTP://Proxy:3128": "http://Proxy:3128", + "https://u:p@proxy.example": "https://u:p@proxy.example", + "proxy.example:8080": "http://proxy.example:8080", + "u:p@proxy.example:8080": "http://u:p@proxy.example:8080", + " http://[::1]:3128 ": "http://[::1]:3128", + "http://u%40corp:p%3Aw@h:1/x": "http://u%40corp:p%3Aw@h:1/x", + } + for in, want := range cases { + r, err := Parse(in) + if err != nil { + t.Fatalf("Parse(%q): %v", in, err) + } + if r.Mode() != ModeExplicit { + t.Fatalf("Parse(%q) mode %q", in, r.Mode()) + } + if got := proxyFor(t, r, "t.invalid:1"); got != want { + t.Fatalf("Parse(%q) proxy = %q, want %q", in, got, want) + } + } + // Percent-encoded userinfo is decoded before it goes on the wire. + r := mustExplicit(t, "http://u%40corp:p%3Aw@h:1") + u, _ := r.ProxyForAddr("t.invalid:1") + if pw, _ := u.User.Password(); u.User.Username() != "u@corp" || pw != "p:w" { + t.Fatalf("userinfo decoded to %q / %q", u.User.Username(), pw) + } +} + +func TestParseErrorsNeverLeakCredentials(t *testing.T) { + t.Parallel() + bad := []string{ + "socks5://user:hunter2@proxy:1080", + "http://user:hunter2@proxy:bad-port", + "http://user:hunter2@", + "http://user:hunter2@proxy%zz:1", + "ftp://proxy:21", + "http://", + } + for _, raw := range bad { + _, err := Explicit(raw) + if err == nil { + t.Fatalf("Explicit(%q) accepted", raw) + } + if strings.Contains(err.Error(), "hunter2") || strings.Contains(err.Error(), "user:") { + t.Fatalf("Explicit(%q) error leaks credentials: %v", raw, err) + } + _, err = fromEnv(envMap(map[string]string{"HTTPS_PROXY": raw})) + if err == nil { + t.Fatalf("HTTPS_PROXY=%q accepted", raw) + } + if strings.Contains(err.Error(), "hunter2") { + t.Fatalf("HTTPS_PROXY=%q error leaks credentials: %v", raw, err) + } + if !strings.HasPrefix(err.Error(), "HTTPS_PROXY: ") { + t.Fatalf("env error should name the variable: %v", err) + } + } + if _, err := Explicit(" "); err == nil { + t.Fatal("empty explicit URL accepted") + } + if _, err := fromEnv(envMap(map[string]string{"HTTP_PROXY": "gopher://x:1"})); err == nil || !strings.HasPrefix(err.Error(), "HTTP_PROXY: ") { + t.Fatalf("bad HTTP_PROXY: %v", err) + } +} + +func TestProxyForAddrRejectsMalformedAddr(t *testing.T) { + t.Parallel() + r, _ := fromEnv(envMap(map[string]string{"HTTPS_PROXY": "http://p:1"})) + if _, err := r.ProxyForAddr("no-port"); err == nil { + t.Fatal("expected an error for an address without a port") + } +} + +func TestProxyForRequestDefaultPorts(t *testing.T) { + t.Parallel() + // Port-scoped NO_PROXY entries see the scheme's default port. + r, _ := fromEnv(envMap(map[string]string{"HTTPS_PROXY": "http://p:1", "NO_PROXY": "a.invalid:443,b.invalid:80"})) + if got := proxyForURL(t, r, "https://a.invalid/"); got != "" { + t.Fatalf("https default port not matched: %q", got) + } + if got := proxyForURL(t, r, "https://b.invalid/"); got == "" { + t.Fatal("b.invalid:80 entry wrongly matched an https request") + } + if got := proxyForURL(t, r, "http://b.invalid/"); got != "" { + t.Fatalf("http default port not matched: %q", got) + } +} + +func TestRedact(t *testing.T) { + t.Parallel() + cases := map[string]string{ + "http://user:pass@proxy:3128": "http://***@proxy:3128", + "http://user@proxy:3128": "http://***@proxy:3128", + "https://proxy.example": "https://proxy.example", + "http://u:p@proxy:1/path?token=x#fr": "http://***@proxy:1", + "http://u:p@[::1]:3128": "http://***@[::1]:3128", + } + for in, want := range cases { + u, err := url.Parse(in) + if err != nil { + t.Fatal(err) + } + if got := Redact(u); got != want { + t.Fatalf("Redact(%q) = %q, want %q", in, got, want) + } + } + if Redact(nil) != "" { + t.Fatal("Redact(nil) should be empty") + } +} + +func TestResolverStringRedacts(t *testing.T) { + t.Parallel() + if got := mustExplicit(t, "http://user:pw@proxy:3128").String(); got != "http://***@proxy:3128" { + t.Fatalf("explicit String = %q", got) + } + r, _ := fromEnv(envMap(map[string]string{"HTTPS_PROXY": "http://user:pw@proxy:3128", "NO_PROXY": "localhost,.corp"})) + if got := r.String(); got != "auto: http://***@proxy:3128 (NO_PROXY=localhost,.corp)" { + t.Fatalf("auto String = %q", got) + } + r, _ = fromEnv(envMap(map[string]string{"HTTPS_PROXY": "http://u:pw@a:1", "HTTP_PROXY": "http://u:pw@b:1"})) + if got := r.String(); got != "auto: http://***@a:1, http http://***@b:1" { + t.Fatalf("auto String with HTTP_PROXY = %q", got) + } + r, _ = fromEnv(envMap(map[string]string{"HTTP_PROXY": "http://u:pw@b:1"})) + if got := r.String(); got != "auto: http-only http://***@b:1" { + t.Fatalf("auto http-only String = %q", got) + } + r, _ = fromEnv(envMap(nil)) + if got := r.String(); got != "auto: no proxy in environment" { + t.Fatalf("empty auto String = %q", got) + } +} + +func TestNoProxyMatching(t *testing.T) { + t.Parallel() + cases := []struct { + noProxy string + host string + port string + bypass bool + }{ + {"", "registry.pilotprotocol.network", "443", false}, + {"*", "anything.example", "1", true}, + {"example.com", "example.com", "443", true}, + {"example.com", "api.example.com", "443", true}, + {"example.com", "notexample.com", "443", false}, + {".example.com", "api.example.com", "443", true}, + {".example.com", "example.com", "443", false}, + {"*.example.com", "api.example.com", "443", true}, + {"*.example.com", "example.com", "443", false}, + {"EXAMPLE.com", "Api.Example.COM", "443", true}, + {"example.com:8443", "example.com", "8443", true}, + {"example.com:8443", "example.com", "443", false}, + {"10.1.2.3", "10.1.2.3", "443", true}, + {"10.1.2.3:9000", "10.1.2.3", "443", false}, + {"10.1.2.3:9000", "10.1.2.3", "9000", true}, + {"10.0.0.0/8", "10.200.1.1", "443", true}, + {"10.0.0.0/8", "11.0.0.1", "443", false}, + {"fd00::/8", "fd00::1", "443", true}, + {"[2001:db8::1]:443", "2001:db8::1", "443", true}, + {"2001:db8::1", "2001:db8::1", "80", true}, + {" a.invalid ,\tb.invalid c.invalid ", "c.invalid", "1", true}, + {",,:443,.,*.,", "x.invalid", "1", false}, + {"example.com", "10.0.0.1", "443", false}, + } + for _, tc := range cases { + n := parseNoProxy(tc.noProxy) + if got := !n.useProxy(tc.host, tc.port); got != tc.bypass { + t.Fatalf("NO_PROXY=%q host=%s:%s bypass=%v, want %v", tc.noProxy, tc.host, tc.port, got, tc.bypass) + } + } +} + +func TestConnectErrorMessage(t *testing.T) { + t.Parallel() + e := &ConnectError{Target: "registry.pilotprotocol.network:443", StatusCode: 407} + if e.Error() != "proxy CONNECT registry.pilotprotocol.network:443: 407 Proxy Authentication Required" { + t.Fatalf("ConnectError = %q", e.Error()) + } +} From 65ea5b1a3d2d7dcd9a061657ec236460bd39b9e8 Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Thu, 24 Sep 2026 00:09:43 +0300 Subject: [PATCH 2/4] feat(registry/client): WithDialer routes every registry connection Add DialOption / WithDialer(DialContextFunc) as a trailing variadic option on Dial, DialPool, DialTLS, DialTLSPool, DialTLSPinned and DialBinary, plus DialTLSPinnedPool (already referenced by the DialTLSPool error message). When set, the dialer opens every connection: the primary, each pool conn, Client.reconnect, reconnectEntry and BinaryClient.reconnect. The registry address reaches the dialer unresolved, so a netproxy.Dialer asks the proxy to CONNECT by host name. For TLS the handshake runs on top of the dialer's conn via tls.Client; ServerName defaults to the address host (as crypto/tls.Dial does) on a clone, so SNI, CA verification and pinned-fingerprint verification are unchanged, and the caller's tls.Config is never mutated. The dial and handshake are bounded by the existing 5s dialTimeout. Without WithDialer every path performs exactly the direct dials it did before (same timeouts and ctx handling); existing call sites compile unchanged. Tests reach a TLS registry stand-in only through a CONNECT proxy under registry.pilot.invalid (never resolvable locally): DialTLS (SNI and CA check), untrusted cert, DialTLSPinned (match and mismatch), DialTLSPool / DialTLSPinnedPool with concurrent sends, reconnect on the primary, pinned and pool paths, plain Dial / DialPool, DialBinary with reconnect, and the WithDialer contract (unresolved addr, bounded ctx, error propagation, conn closed on handshake failure, nil keeps direct). Co-Authored-By: Claude Opus 5.5 (1M context) --- registry/client/binary_client.go | 14 +- registry/client/client.go | 95 ++-- registry/client/dialer.go | 93 ++++ registry/client/zz_client_proxy_test.go | 653 ++++++++++++++++++++++++ 4 files changed, 809 insertions(+), 46 deletions(-) create mode 100644 registry/client/dialer.go create mode 100644 registry/client/zz_client_proxy_test.go diff --git a/registry/client/binary_client.go b/registry/client/binary_client.go index 70e5bb6..056e8b7 100644 --- a/registry/client/binary_client.go +++ b/registry/client/binary_client.go @@ -22,12 +22,16 @@ type BinaryClient struct { mu sync.Mutex addr string closed bool + dial DialContextFunc // optional; see WithDialer } // DialBinary connects to a registry server and negotiates the binary wire protocol. // The server detects the magic bytes and switches to binary mode for this connection. -func DialBinary(addr string) (*BinaryClient, error) { - conn, err := net.DialTimeout("tcp", addr, 5*time.Second) +// Pass WithDialer to route the connection (and every reconnect) through a +// custom dialer such as an HTTP CONNECT proxy. +func DialBinary(addr string, opts ...DialOption) (*BinaryClient, error) { + o := applyDialOptions(opts) + conn, err := dialConn(context.Background(), o.dial, addr, nil) if err != nil { return nil, fmt.Errorf("dial registry: %w", err) } @@ -41,7 +45,7 @@ func DialBinary(addr string) (*BinaryClient, error) { return nil, fmt.Errorf("binary handshake: %w", err) } - return &BinaryClient{conn: conn, addr: addr}, nil + return &BinaryClient{conn: conn, addr: addr, dial: o.dial}, nil } // Close shuts down the binary client connection. @@ -75,9 +79,7 @@ func (c *BinaryClient) reconnect() error { var lastErr error for attempts := 0; attempts < 5; attempts++ { - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - conn, err := (&net.Dialer{}).DialContext(ctx, "tcp", c.addr) - cancel() + conn, err := dialConn(context.Background(), c.dial, c.addr, nil) if err != nil { lastErr = err slog.Warn("binary client reconnect failed", "attempt", attempts+1, "err", err) diff --git a/registry/client/client.go b/registry/client/client.go index a096d44..1568520 100644 --- a/registry/client/client.go +++ b/registry/client/client.go @@ -52,6 +52,10 @@ const dialTimeout = 5 * time.Second // conn is in use), eliminating the head-of-line wait. The primary // c.conn / c.mu / c.closed fields are retained for backward compatibility // with tests that touch them directly. +// +// Every Dial* constructor accepts DialOptions; WithDialer routes all of the +// client's connections (initial, pool and reconnect) through a custom +// dialer, e.g. an HTTP CONNECT proxy from the netproxy package. type Client struct { // Primary connection. Always present; tests in this package read // c.conn / c.mu / c.closed directly so the field set must stay stable. @@ -61,6 +65,10 @@ type Client struct { closed bool tlsConfig *tls.Config signer func(challenge string) string // H3 fix: optional message signer + // dial, when non-nil, opens every connection (initial, pool, reconnect) + // instead of a direct net.Dialer — see WithDialer. Immutable after + // construction. + dial DialContextFunc // Optional pool of secondary connections used to parallelise Send. // nil / empty when DialPool was not used. @@ -131,12 +139,16 @@ func (c *Client) sign(challenge string) (string, error) { return sig, nil } -func Dial(addr string) (*Client, error) { - conn, err := net.DialTimeout("tcp", addr, dialTimeout) +// Dial connects to a registry server over plain TCP. Pass WithDialer to +// route the connection (and every reconnect) through a custom dialer such +// as an HTTP CONNECT proxy. +func Dial(addr string, opts ...DialOption) (*Client, error) { + o := applyDialOptions(opts) + conn, err := dialConn(context.Background(), o.dial, addr, nil) if err != nil { return nil, fmt.Errorf("dial registry: %w", err) } - return &Client{conn: conn, addr: addr}, nil + return &Client{conn: conn, addr: addr, dial: o.dial}, nil } // DialPool connects to a registry server over plain TCP and pre-warms a @@ -153,15 +165,16 @@ func Dial(addr string) (*Client, error) { // // On any pool conn dial failure DialPool closes the conns it had already // opened and returns an error. -func DialPool(addr string, size int) (*Client, error) { +func DialPool(addr string, size int, opts ...DialOption) (*Client, error) { if size <= 0 { size = 1 } - primary, err := net.DialTimeout("tcp", addr, dialTimeout) + o := applyDialOptions(opts) + primary, err := dialConn(context.Background(), o.dial, addr, nil) if err != nil { return nil, fmt.Errorf("dial registry: %w", err) } - c := &Client{conn: primary, addr: addr} + c := &Client{conn: primary, addr: addr, dial: o.dial} if err := c.initPool(size, nil); err != nil { primary.Close() return nil, err @@ -171,30 +184,35 @@ func DialPool(addr string, size int) (*Client, error) { // DialTLS connects to a registry server over TLS. // A non-nil tlsConfig is required. For certificate pinning, use DialTLSPinned. -func DialTLS(addr string, tlsConfig *tls.Config) (*Client, error) { +func DialTLS(addr string, tlsConfig *tls.Config, opts ...DialOption) (*Client, error) { if tlsConfig == nil { return nil, fmt.Errorf("TLS config required; use DialTLSPinned for certificate pinning") } - conn, err := tls.DialWithDialer(&net.Dialer{Timeout: dialTimeout}, "tcp", addr, tlsConfig) + o := applyDialOptions(opts) + conn, err := dialConn(context.Background(), o.dial, addr, tlsConfig) if err != nil { return nil, fmt.Errorf("dial registry TLS: %w", err) } - return &Client{conn: conn, addr: addr, tlsConfig: tlsConfig}, nil + return &Client{conn: conn, addr: addr, tlsConfig: tlsConfig, dial: o.dial}, nil } // DialTLSPool is the TLS variant of DialPool. -func DialTLSPool(addr string, tlsConfig *tls.Config, size int) (*Client, error) { +func DialTLSPool(addr string, tlsConfig *tls.Config, size int, opts ...DialOption) (*Client, error) { if tlsConfig == nil { return nil, fmt.Errorf("TLS config required; use DialTLSPinnedPool for certificate pinning") } + return dialTLSPool(addr, tlsConfig, size, "dial registry TLS", applyDialOptions(opts)) +} + +func dialTLSPool(addr string, tlsConfig *tls.Config, size int, errPrefix string, o dialOptions) (*Client, error) { if size <= 0 { size = 1 } - primary, err := tls.DialWithDialer(&net.Dialer{Timeout: dialTimeout}, "tcp", addr, tlsConfig) + primary, err := dialConn(context.Background(), o.dial, addr, tlsConfig) if err != nil { - return nil, fmt.Errorf("dial registry TLS: %w", err) + return nil, fmt.Errorf("%s: %w", errPrefix, err) } - c := &Client{conn: primary, addr: addr, tlsConfig: tlsConfig} + c := &Client{conn: primary, addr: addr, tlsConfig: tlsConfig, dial: o.dial} if err := c.initPool(size, tlsConfig); err != nil { primary.Close() return nil, err @@ -204,7 +222,8 @@ func DialTLSPool(addr string, tlsConfig *tls.Config, size int) (*Client, error) // initPool dials size-1 additional connections and registers them in c.pool. // It assumes c.conn (primary) is already set. tlsCfg, when non-nil, is used -// for TLS dialing; otherwise plain TCP. +// for TLS dialing; otherwise plain TCP. Every conn goes through c.dial when +// one is configured. func (c *Client) initPool(size int, tlsCfg *tls.Config) error { if size <= 1 { // No secondary conns — single-conn legacy path; pool stays empty. @@ -213,13 +232,7 @@ func (c *Client) initPool(size int, tlsCfg *tls.Config) error { entries := make([]*pooledConn, 0, size) entries = append(entries, &pooledConn{conn: c.conn}) for i := 1; i < size; i++ { - var conn net.Conn - var err error - if tlsCfg != nil { - conn, err = tls.DialWithDialer(&net.Dialer{Timeout: dialTimeout}, "tcp", c.addr, tlsCfg) - } else { - conn, err = net.DialTimeout("tcp", c.addr, dialTimeout) - } + conn, err := dialConn(context.Background(), c.dial, c.addr, tlsCfg) if err != nil { // Close any conns we already opened (excluding primary — // caller closes that on failure). @@ -242,8 +255,25 @@ func (c *Client) initPool(size int, tlsCfg *tls.Config) error { // DialTLSPinned connects to a registry server over TLS with certificate pinning. // The fingerprint is a hex-encoded SHA-256 hash of the server's DER-encoded certificate. -func DialTLSPinned(addr, fingerprint string) (*Client, error) { - tlsConfig := &tls.Config{ +func DialTLSPinned(addr, fingerprint string, opts ...DialOption) (*Client, error) { + o := applyDialOptions(opts) + tlsConfig := pinnedTLSConfig(fingerprint) + conn, err := dialConn(context.Background(), o.dial, addr, tlsConfig) + if err != nil { + return nil, fmt.Errorf("dial registry TLS pinned: %w", err) + } + return &Client{conn: conn, addr: addr, tlsConfig: tlsConfig, dial: o.dial}, nil +} + +// DialTLSPinnedPool is the pooled variant of DialTLSPinned (see DialPool). +func DialTLSPinnedPool(addr, fingerprint string, size int, opts ...DialOption) (*Client, error) { + return dialTLSPool(addr, pinnedTLSConfig(fingerprint), size, "dial registry TLS pinned", applyDialOptions(opts)) +} + +// pinnedTLSConfig accepts exactly the server certificate whose DER SHA-256 +// is the hex fingerprint, instead of verifying a CA chain. +func pinnedTLSConfig(fingerprint string) *tls.Config { + return &tls.Config{ // InsecureSkipVerify disables the default CA chain check so we can // use VerifyPeerCertificate for certificate pinning (SHA-256 fingerprint). // This is the standard Go pattern — the custom callback below provides @@ -261,11 +291,6 @@ func DialTLSPinned(addr, fingerprint string) (*Client, error) { return nil }, } - conn, err := tls.DialWithDialer(&net.Dialer{Timeout: dialTimeout}, "tcp", addr, tlsConfig) - if err != nil { - return nil, fmt.Errorf("dial registry TLS pinned: %w", err) - } - return &Client{conn: conn, addr: addr, tlsConfig: tlsConfig}, nil } func (c *Client) Close() error { @@ -327,12 +352,7 @@ func (c *Client) reconnect(ctx context.Context) error { maxBackoff := 10 * time.Second for attempts := 0; attempts < 5; attempts++ { - if c.tlsConfig != nil { - dialer := &tls.Dialer{Config: c.tlsConfig, NetDialer: &net.Dialer{Timeout: 5 * time.Second}} - conn, err = dialer.DialContext(ctx, "tcp", c.addr) - } else { - conn, err = net.DialTimeout("tcp", c.addr, 5*time.Second) - } + conn, err = dialConn(ctx, c.dial, c.addr, c.tlsConfig) if err == nil { c.conn = conn slog.Info("registry reconnected", "addr", c.addr) @@ -473,12 +493,7 @@ func (c *Client) reconnectEntry(ctx context.Context, entry *pooledConn) error { backoff := 500 * time.Millisecond maxBackoff := 10 * time.Second for attempts := 0; attempts < 5; attempts++ { - if c.tlsConfig != nil { - dialer := &tls.Dialer{Config: c.tlsConfig, NetDialer: &net.Dialer{Timeout: 5 * time.Second}} - conn, err = dialer.DialContext(ctx, "tcp", c.addr) - } else { - conn, err = net.DialTimeout("tcp", c.addr, 5*time.Second) - } + conn, err = dialConn(ctx, c.dial, c.addr, c.tlsConfig) if err == nil { entry.conn = conn // Keep c.conn (primary) in sync if this is the primary entry. diff --git a/registry/client/dialer.go b/registry/client/dialer.go new file mode 100644 index 0000000..983706c --- /dev/null +++ b/registry/client/dialer.go @@ -0,0 +1,93 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package client + +import ( + "context" + "crypto/tls" + "net" +) + +// DialContextFunc opens a raw stream connection. It has the signature of +// (*net.Dialer).DialContext and (*netproxy.Dialer).DialContext, so either +// method value can be passed to WithDialer. +type DialContextFunc func(ctx context.Context, network, addr string) (net.Conn, error) + +// DialOption customises how a Client or BinaryClient establishes its +// connections. Options are accepted by every Dial* constructor. +type DialOption func(*dialOptions) + +type dialOptions struct { + dial DialContextFunc +} + +// WithDialer makes the client open EVERY connection through dial: the +// initial connection, each pool connection, and every reconnect. The +// registry address is passed to dial unresolved, so a proxy dialer (for +// example netproxy.Dialer.DialContext, which issues an HTTP CONNECT by host +// name) sees the registry host name, never a locally resolved IP. +// +// For the TLS constructors the TLS handshake runs on top of the returned +// conn, end-to-end with the registry. When the tls.Config has no ServerName +// the host part of the registry address is used, exactly as a direct TLS +// dial does, so SNI, CA verification and DialTLSPinned's fingerprint check +// behave the same through the dialer. dial is called with a context bounded +// by the client's per-attempt dial timeout. +// +// A nil dial (or not passing WithDialer) keeps the default direct dial. +func WithDialer(dial DialContextFunc) DialOption { + return func(o *dialOptions) { o.dial = dial } +} + +func applyDialOptions(opts []DialOption) dialOptions { + var o dialOptions + for _, opt := range opts { + if opt != nil { + opt(&o) + } + } + return o +} + +// dialConn opens one registry connection to addr, plus a TLS handshake when +// tlsCfg is non-nil, bounded by ctx and dialTimeout. +// +// With a nil dial it performs exactly the direct dials the client always +// has: a TLS dial that observes ctx, and a plain TCP dial bounded only by +// dialTimeout (the reconnect loops check ctx between attempts). +func dialConn(ctx context.Context, dial DialContextFunc, addr string, tlsCfg *tls.Config) (net.Conn, error) { + if dial == nil { + if tlsCfg != nil { + d := &tls.Dialer{NetDialer: &net.Dialer{Timeout: dialTimeout}, Config: tlsCfg} + return d.DialContext(ctx, "tcp", addr) + } + return net.DialTimeout("tcp", addr, dialTimeout) + } + + ctx, cancel := context.WithTimeout(ctx, dialTimeout) + defer cancel() + raw, err := dial(ctx, "tcp", addr) + if err != nil { + return nil, err + } + if tlsCfg == nil { + return raw, nil + } + cfg := tlsCfg + if cfg.ServerName == "" { + // Mirror crypto/tls.Dial: the SNI / verification name is the host + // part of the dialed address. + host, _, err := net.SplitHostPort(addr) + if err != nil { + host = addr + } + cfg = cfg.Clone() + cfg.ServerName = host + } + tc := tls.Client(raw, cfg) + if err := tc.HandshakeContext(ctx); err != nil { + raw.Close() + return nil, err + } + return tc, nil +} diff --git a/registry/client/zz_client_proxy_test.go b/registry/client/zz_client_proxy_test.go new file mode 100644 index 0000000..9488f3e --- /dev/null +++ b/registry/client/zz_client_proxy_test.go @@ -0,0 +1,653 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package client + +import ( + "bufio" + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/sha256" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "io" + "math/big" + "net" + "net/http" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/pilot-protocol/common/netproxy" + "github.com/pilot-protocol/common/registry/wire" +) + +// Registry dials through an HTTP CONNECT proxy (WithDialer + netproxy). +// +// The registry is only reachable as registry.pilot.invalid — a reserved TLD +// that never resolves locally — and only through the test proxy, which maps +// that name to the loopback listener. A successful round-trip therefore +// proves the client handed the proxy the host name, and every CONNECT the +// proxy records must name registry.pilot.invalid, never an IP. + +const proxiedRegistryHost = "registry.pilot.invalid" + +// --- CONNECT proxy ---------------------------------------------------------- + +type regTestProxy struct { + ln net.Listener + realHost string // what proxiedRegistryHost maps to + wantAuth string + + mu sync.Mutex + targets []string + live []net.Conn + wg sync.WaitGroup +} + +func newRegTestProxy(t *testing.T, realHost, user, pass string) *regTestProxy { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("proxy listen: %v", err) + } + p := ®TestProxy{ln: ln, realHost: realHost} + if user != "" { + req := &http.Request{Header: http.Header{}} + req.SetBasicAuth(user, pass) + p.wantAuth = req.Header.Get("Authorization") + } + p.wg.Add(1) + go p.accept() + t.Cleanup(func() { + ln.Close() + p.mu.Lock() + for _, c := range p.live { + c.Close() + } + p.mu.Unlock() + p.wg.Wait() + }) + return p +} + +func (p *regTestProxy) track(c net.Conn) { + p.mu.Lock() + p.live = append(p.live, c) + p.mu.Unlock() +} + +func (p *regTestProxy) connects() []string { + p.mu.Lock() + defer p.mu.Unlock() + return append([]string(nil), p.targets...) +} + +func (p *regTestProxy) accept() { + defer p.wg.Done() + for { + conn, err := p.ln.Accept() + if err != nil { + return + } + p.track(conn) + p.wg.Add(1) + go func() { + defer p.wg.Done() + p.serve(conn) + }() + } +} + +func (p *regTestProxy) serve(conn net.Conn) { + defer conn.Close() + br := bufio.NewReader(conn) + req, err := http.ReadRequest(br) + if err != nil { + return + } + p.mu.Lock() + p.targets = append(p.targets, req.RequestURI) + p.mu.Unlock() + if req.Method != http.MethodConnect { + fmt.Fprint(conn, "HTTP/1.1 405 Method Not Allowed\r\nContent-Length: 0\r\n\r\n") + return + } + if p.wantAuth != "" && req.Header.Get("Proxy-Authorization") != p.wantAuth { + fmt.Fprint(conn, "HTTP/1.1 407 Proxy Authentication Required\r\nContent-Length: 0\r\n\r\n") + return + } + host, port, err := net.SplitHostPort(req.RequestURI) + if err != nil || host != proxiedRegistryHost { + fmt.Fprint(conn, "HTTP/1.1 502 Bad Gateway\r\nContent-Length: 0\r\n\r\n") + return + } + up, err := net.DialTimeout("tcp", net.JoinHostPort(p.realHost, port), 5*time.Second) + if err != nil { + fmt.Fprint(conn, "HTTP/1.1 502 Bad Gateway\r\nContent-Length: 0\r\n\r\n") + return + } + p.track(up) + defer up.Close() + if _, err := io.WriteString(conn, "HTTP/1.1 200 Connection established\r\n\r\n"); err != nil { + return + } + done := make(chan struct{}, 2) + go func() { io.Copy(up, br); up.Close(); done <- struct{}{} }() + go func() { io.Copy(conn, up); conn.Close(); done <- struct{}{} }() + <-done + <-done +} + +// --- registry-like servers --------------------------------------------------- + +// genProxiedRegistryCert issues a self-signed certificate for +// registry.pilot.invalid only (no IP SANs), plus a pool trusting it and the +// DER-encoded leaf for pin fingerprints. +func genProxiedRegistryCert(t *testing.T) (tls.Certificate, *x509.CertPool, []byte) { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("genkey: %v", err) + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(time.Now().UnixNano()), + Subject: pkix.Name{CommonName: proxiedRegistryHost}, + 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, + DNSNames: []string{proxiedRegistryHost}, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatalf("create cert: %v", err) + } + leaf, _ := x509.ParseCertificate(der) + pool := x509.NewCertPool() + pool.AddCert(leaf) + return tls.Certificate{Certificate: [][]byte{der}, PrivateKey: key, Leaf: leaf}, pool, der +} + +// proxiedRegistry is a JSON-wire registry stand-in (TLS or plain TCP) on +// loopback. Its handler closes the connection without answering the first +// {"type":"drop"} request, which forces the client down its reconnect path. +type proxiedRegistry struct { + ln net.Listener + pool *x509.CertPool + der []byte + conns atomic.Int32 + dropped atomic.Bool + + mu sync.Mutex + snis []string + live []net.Conn + wg sync.WaitGroup +} + +func newProxiedRegistry(t *testing.T, useTLS bool) *proxiedRegistry { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + s := &proxiedRegistry{} + if useTLS { + cert, pool, der := genProxiedRegistryCert(t) + s.pool, s.der = pool, der + ln = tls.NewListener(ln, &tls.Config{ + Certificates: []tls.Certificate{cert}, + MinVersion: tls.VersionTLS12, + GetConfigForClient: func(hello *tls.ClientHelloInfo) (*tls.Config, error) { + s.mu.Lock() + s.snis = append(s.snis, hello.ServerName) + s.mu.Unlock() + return nil, nil + }, + }) + } + s.ln = ln + s.wg.Add(1) + go func() { + defer s.wg.Done() + for { + conn, err := ln.Accept() + if err != nil { + return + } + s.conns.Add(1) + s.mu.Lock() + s.live = append(s.live, conn) + s.mu.Unlock() + s.wg.Add(1) + go func() { + defer s.wg.Done() + handleJSONOverConn(conn, s.handle) + }() + } + }() + t.Cleanup(func() { + ln.Close() + s.mu.Lock() + for _, c := range s.live { + c.Close() + } + s.mu.Unlock() + s.wg.Wait() + }) + return s +} + +func (s *proxiedRegistry) handle(req map[string]interface{}) map[string]interface{} { + if req["type"] == "drop" && s.dropped.CompareAndSwap(false, true) { + return nil // close without answering + } + return map[string]interface{}{"type": "ok", "echo": req} +} + +// target is the only address the client is given: the unresolvable name +// plus the real listener's port. +func (s *proxiedRegistry) target() string { + _, port, _ := net.SplitHostPort(s.ln.Addr().String()) + return net.JoinHostPort(proxiedRegistryHost, port) +} + +func (s *proxiedRegistry) realHost() string { + host, _, _ := net.SplitHostPort(s.ln.Addr().String()) + return host +} + +func (s *proxiedRegistry) fingerprint() string { + sum := sha256.Sum256(s.der) + return hex.EncodeToString(sum[:]) +} + +func (s *proxiedRegistry) serverNames() []string { + s.mu.Lock() + defer s.mu.Unlock() + return append([]string(nil), s.snis...) +} + +// proxyDialer is the production wiring: a netproxy.Dialer for an explicit +// authenticated proxy URL, passed as a method value. +func proxyDialer(t *testing.T, p *regTestProxy) DialOption { + t.Helper() + r, err := netproxy.Explicit("http://muse:s3cret@" + p.ln.Addr().String()) + if err != nil { + t.Fatalf("netproxy.Explicit: %v", err) + } + return WithDialer(netproxy.NewDialer(r).DialContext) +} + +func mustSendOK(t *testing.T, c *Client, typ string) { + t.Helper() + resp, err := c.Send(map[string]interface{}{"type": typ}) + if err != nil { + t.Fatalf("Send(%s): %v", typ, err) + } + if resp["type"] != "ok" { + t.Fatalf("Send(%s) type = %v", typ, resp["type"]) + } +} + +// assertConnects checks that the proxy saw exactly n CONNECTs, all for the +// registry host name. +func assertConnects(t *testing.T, p *regTestProxy, s *proxiedRegistry, n int) { + t.Helper() + got := p.connects() + if len(got) != n { + t.Fatalf("proxy saw %d CONNECTs %q, want %d", len(got), got, n) + } + for _, target := range got { + if target != s.target() { + t.Fatalf("CONNECT target %q, want %q (host name, never an IP)", target, s.target()) + } + } +} + +// --- TLS: primary, pinned, pools, reconnects -------------------------------- + +func TestDialTLSThroughProxyVerifiesRegistryHostname(t *testing.T) { + t.Parallel() + srv := newProxiedRegistry(t, true) + proxy := newRegTestProxy(t, srv.realHost(), "muse", "s3cret") + + // No ServerName: it must come from the registry address, as with a + // direct tls.Dial, and the caller's config must not be mutated. + cfg := &tls.Config{RootCAs: srv.pool, MinVersion: tls.VersionTLS12} + c, err := DialTLS(srv.target(), cfg, proxyDialer(t, proxy)) + if err != nil { + t.Fatalf("DialTLS via proxy: %v", err) + } + defer c.Close() + mustSendOK(t, c, "hello") + + if cfg.ServerName != "" { + t.Fatalf("caller tls.Config mutated: ServerName=%q", cfg.ServerName) + } + if _, ok := c.conn.(*tls.Conn); !ok { + t.Fatalf("conn type %T, want *tls.Conn", c.conn) + } + if snis := srv.serverNames(); len(snis) != 1 || snis[0] != proxiedRegistryHost { + t.Fatalf("registry saw SNI %q, want %q", snis, proxiedRegistryHost) + } + assertConnects(t, proxy, srv, 1) +} + +func TestDialTLSThroughProxyRejectsUntrustedCert(t *testing.T) { + t.Parallel() + srv := newProxiedRegistry(t, true) + proxy := newRegTestProxy(t, srv.realHost(), "muse", "s3cret") + _, err := DialTLS(srv.target(), &tls.Config{MinVersion: tls.VersionTLS12}, proxyDialer(t, proxy)) + // UnknownAuthorityError, or SystemRootsError on a host without a CA + // bundle; crypto/tls wraps both in CertificateVerificationError. + var cve *tls.CertificateVerificationError + if !errors.As(err, &cve) { + t.Fatalf("DialTLS with system roots against a self-signed registry: %v, want a certificate verification error", err) + } +} + +func TestDialTLSPinnedThroughProxy(t *testing.T) { + t.Parallel() + srv := newProxiedRegistry(t, true) + proxy := newRegTestProxy(t, srv.realHost(), "muse", "s3cret") + + c, err := DialTLSPinned(srv.target(), srv.fingerprint(), proxyDialer(t, proxy)) + if err != nil { + t.Fatalf("DialTLSPinned via proxy: %v", err) + } + mustSendOK(t, c, "hello") + c.Close() + // Pinned mode still sends SNI, so an SNI-routed :443 front end works. + if snis := srv.serverNames(); len(snis) != 1 || snis[0] != proxiedRegistryHost { + t.Fatalf("registry saw SNI %q, want %q", snis, proxiedRegistryHost) + } + + wrong := strings.Repeat("00", sha256.Size) + _, err = DialTLSPinned(srv.target(), wrong, proxyDialer(t, proxy)) + if err == nil || !strings.Contains(err.Error(), "certificate fingerprint mismatch") { + t.Fatalf("wrong pin: err = %v, want fingerprint mismatch", err) + } + assertConnects(t, proxy, srv, 2) +} + +func TestDialTLSPoolThroughProxy(t *testing.T) { + t.Parallel() + srv := newProxiedRegistry(t, true) + proxy := newRegTestProxy(t, srv.realHost(), "muse", "s3cret") + + c, err := DialTLSPool(srv.target(), &tls.Config{RootCAs: srv.pool, MinVersion: tls.VersionTLS12}, 3, proxyDialer(t, proxy)) + if err != nil { + t.Fatalf("DialTLSPool via proxy: %v", err) + } + defer c.Close() + if len(c.pool.entries) != 3 { + t.Fatalf("pool entries = %d, want 3", len(c.pool.entries)) + } + var wg sync.WaitGroup + errs := make(chan error, 12) + for i := 0; i < 12; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + if _, err := c.Send(map[string]interface{}{"type": fmt.Sprintf("q%d", i)}); err != nil { + errs <- err + } + }(i) + } + wg.Wait() + close(errs) + for err := range errs { + t.Fatalf("concurrent Send over proxied pool: %v", err) + } + assertConnects(t, proxy, srv, 3) + if got := srv.conns.Load(); got != 3 { + t.Fatalf("registry accepted %d conns, want 3", got) + } +} + +func TestDialTLSPinnedPoolThroughProxy(t *testing.T) { + t.Parallel() + srv := newProxiedRegistry(t, true) + proxy := newRegTestProxy(t, srv.realHost(), "muse", "s3cret") + + c, err := DialTLSPinnedPool(srv.target(), srv.fingerprint(), 2, proxyDialer(t, proxy)) + if err != nil { + t.Fatalf("DialTLSPinnedPool via proxy: %v", err) + } + defer c.Close() + mustSendOK(t, c, "hello") + assertConnects(t, proxy, srv, 2) + + _, err = DialTLSPinnedPool(srv.target(), strings.Repeat("ab", sha256.Size), 2, proxyDialer(t, proxy)) + if err == nil || !strings.Contains(err.Error(), "dial registry TLS pinned") { + t.Fatalf("wrong pin: err = %v", err) + } +} + +func TestReconnectTLSThroughProxy(t *testing.T) { + t.Parallel() + srv := newProxiedRegistry(t, true) + proxy := newRegTestProxy(t, srv.realHost(), "muse", "s3cret") + + c, err := DialTLS(srv.target(), &tls.Config{RootCAs: srv.pool, MinVersion: tls.VersionTLS12}, proxyDialer(t, proxy)) + if err != nil { + t.Fatalf("DialTLS: %v", err) + } + defer c.Close() + // The server drops the conn on "drop"; Send must reconnect through the + // proxy (not directly) and retry. + mustSendOK(t, c, "drop") + mustSendOK(t, c, "after") + assertConnects(t, proxy, srv, 2) + if got := srv.conns.Load(); got != 2 { + t.Fatalf("registry accepted %d conns, want 2", got) + } +} + +func TestReconnectPinnedThroughProxy(t *testing.T) { + t.Parallel() + srv := newProxiedRegistry(t, true) + proxy := newRegTestProxy(t, srv.realHost(), "muse", "s3cret") + + c, err := DialTLSPinned(srv.target(), srv.fingerprint(), proxyDialer(t, proxy)) + if err != nil { + t.Fatalf("DialTLSPinned: %v", err) + } + defer c.Close() + mustSendOK(t, c, "drop") + assertConnects(t, proxy, srv, 2) +} + +func TestPoolReconnectTLSThroughProxy(t *testing.T) { + t.Parallel() + srv := newProxiedRegistry(t, true) + proxy := newRegTestProxy(t, srv.realHost(), "muse", "s3cret") + + c, err := DialTLSPool(srv.target(), &tls.Config{RootCAs: srv.pool, MinVersion: tls.VersionTLS12}, 2, proxyDialer(t, proxy)) + if err != nil { + t.Fatalf("DialTLSPool: %v", err) + } + defer c.Close() + // reconnectEntry redials only the broken entry, through the proxy. + mustSendOK(t, c, "drop") + mustSendOK(t, c, "after") + assertConnects(t, proxy, srv, 3) + if got := srv.conns.Load(); got != 3 { + t.Fatalf("registry accepted %d conns, want 3", got) + } + // Every pool entry is a TLS conn to the registry host. + for i, e := range c.pool.entries { + tc, ok := e.conn.(*tls.Conn) + if !ok { + t.Fatalf("entry %d conn %T, want *tls.Conn", i, e.conn) + } + if sn := tc.ConnectionState().ServerName; sn != proxiedRegistryHost { + t.Fatalf("entry %d ServerName %q", i, sn) + } + } +} + +// --- plain TCP and binary clients ------------------------------------------- + +func TestDialAndDialPoolThroughProxy(t *testing.T) { + t.Parallel() + srv := newProxiedRegistry(t, false) + proxy := newRegTestProxy(t, srv.realHost(), "muse", "s3cret") + + c, err := Dial(srv.target(), proxyDialer(t, proxy)) + if err != nil { + t.Fatalf("Dial via proxy: %v", err) + } + mustSendOK(t, c, "drop") // primary reconnect through the proxy + c.Close() + assertConnects(t, proxy, srv, 2) + + pc, err := DialPool(srv.target(), 3, proxyDialer(t, proxy)) + if err != nil { + t.Fatalf("DialPool via proxy: %v", err) + } + defer pc.Close() + mustSendOK(t, pc, "hello") + assertConnects(t, proxy, srv, 5) +} + +func TestDialBinaryThroughProxy(t *testing.T) { + t.Parallel() + srv := newFakeBinaryServer(t, func(msgType byte, payload []byte) (byte, []byte) { + body, _ := json.Marshal(map[string]interface{}{"type": "ok"}) + return wire.MsgJSON, body + }) + host, port, _ := net.SplitHostPort(srv.addr()) + proxy := newRegTestProxy(t, host, "muse", "s3cret") + target := net.JoinHostPort(proxiedRegistryHost, port) + + c, err := DialBinary(target, proxyDialer(t, proxy)) + if err != nil { + t.Fatalf("DialBinary via proxy: %v", err) + } + defer c.Close() + if resp, err := c.SendJSON(map[string]interface{}{"type": "x"}); err != nil || resp["type"] != "ok" { + t.Fatalf("SendJSON: %v %v", resp, err) + } + // Break the conn; the reconnect must go through the proxy again. + c.mu.Lock() + c.conn.Close() + c.mu.Unlock() + if resp, err := c.SendJSON(map[string]interface{}{"type": "y"}); err != nil || resp["type"] != "ok" { + t.Fatalf("SendJSON after reconnect: %v %v", resp, err) + } + got := proxy.connects() + if len(got) != 2 || got[0] != target || got[1] != target { + t.Fatalf("CONNECTs = %q, want 2 x %q", got, target) + } + if n := srv.handshakes.Load(); n != 2 { + t.Fatalf("binary handshakes = %d, want 2", n) + } +} + +// --- WithDialer contract ------------------------------------------------------ + +func TestWithDialerReceivesUnresolvedAddrAndBoundedContext(t *testing.T) { + t.Parallel() + srv := newProxiedRegistry(t, false) + var calls atomic.Int32 + dial := func(ctx context.Context, network, addr string) (net.Conn, error) { + calls.Add(1) + if network != "tcp" || addr != srv.target() { + return nil, fmt.Errorf("dial(%q, %q): want tcp and the unresolved registry address", network, addr) + } + dl, ok := ctx.Deadline() + if !ok || time.Until(dl) > dialTimeout { + return nil, fmt.Errorf("dial ctx deadline %v (set=%v), want within %v", dl, ok, dialTimeout) + } + var d net.Dialer + return d.DialContext(ctx, network, net.JoinHostPort(srv.realHost(), strings.TrimPrefix(addr, proxiedRegistryHost+":"))) + } + c, err := DialPool(srv.target(), 2, WithDialer(dial)) + if err != nil { + t.Fatalf("DialPool: %v", err) + } + defer c.Close() + mustSendOK(t, c, "drop") + if got := calls.Load(); got != 3 { + t.Fatalf("dialer called %d times, want 3 (2 pool conns + 1 reconnect)", got) + } +} + +func TestWithDialerErrorsPropagate(t *testing.T) { + t.Parallel() + boom := errors.New("egress denied") + dial := func(context.Context, string, string) (net.Conn, error) { return nil, boom } + for name, fn := range map[string]func() error{ + "Dial": func() error { _, err := Dial("r.invalid:1", WithDialer(dial)); return err }, + "DialPool": func() error { _, err := DialPool("r.invalid:1", 2, WithDialer(dial)); return err }, + "DialTLS": func() error { _, err := DialTLS("r.invalid:1", &tls.Config{}, WithDialer(dial)); return err }, + "DialTLSPool": func() error { _, err := DialTLSPool("r.invalid:1", &tls.Config{}, 2, WithDialer(dial)); return err }, + "DialTLSPinned": func() error { _, err := DialTLSPinned("r.invalid:1", "00", WithDialer(dial)); return err }, + "DialTLSPinnedPool": func() error { _, err := DialTLSPinnedPool("r.invalid:1", "00", 2, WithDialer(dial)); return err }, + "DialBinary": func() error { _, err := DialBinary("r.invalid:1", WithDialer(dial)); return err }, + } { + if err := fn(); !errors.Is(err, boom) { + t.Fatalf("%s: err = %v, want wrapped %v", name, err, boom) + } + } +} + +// closeRecordingConn records Close so tests can prove a failed TLS +// handshake does not leak the dialer's conn. +type closeRecordingConn struct { + net.Conn + closed atomic.Bool +} + +func (c *closeRecordingConn) Close() error { + c.closed.Store(true) + return c.Conn.Close() +} + +func TestWithDialerClosesConnOnTLSHandshakeFailure(t *testing.T) { + t.Parallel() + srv := newProxiedRegistry(t, false) // plain TCP: a TLS handshake cannot succeed + var rec *closeRecordingConn + dial := func(ctx context.Context, network, addr string) (net.Conn, error) { + var d net.Dialer + raw, err := d.DialContext(ctx, network, srv.ln.Addr().String()) + if err != nil { + return nil, err + } + rec = &closeRecordingConn{Conn: raw} + return rec, nil + } + _, err := DialTLS(srv.target(), &tls.Config{MinVersion: tls.VersionTLS12}, WithDialer(dial)) + if err == nil { + t.Fatal("expected a TLS handshake error against a plain TCP server") + } + if rec == nil || !rec.closed.Load() { + t.Fatal("dialer conn was not closed after the failed handshake") + } +} + +func TestWithDialerNilKeepsDirectDial(t *testing.T) { + t.Parallel() + srv := newProxiedRegistry(t, false) + c, err := Dial(srv.ln.Addr().String(), WithDialer(nil), nil) + if err != nil { + t.Fatalf("Dial with nil dialer/option: %v", err) + } + defer c.Close() + if c.dial != nil { + t.Fatal("nil WithDialer should leave the default direct dial") + } + mustSendOK(t, c, "hello") +} From 5285f11ec709014ff370374d285f5566f2153781 Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Thu, 24 Sep 2026 00:52:29 +0300 Subject: [PATCH 3/4] fix(netproxy): independent proxy env vars; never misparse credentials FromEnvironment parsed every proxy variable and failed as a whole when any of them was unusable, so an http_proxy Go cannot use (socks5h://, stray whitespace) or a socks ALL_PROXY threw away a valid HTTPS_PROXY. Callers then fell back to direct dials, which a proxy-only sandbox (Meta Muse) kills. Each variable is now parsed on its own: an unusable HTTP_PROXY, http_proxy, ALL_PROXY or all_proxy is skipped (the next variable in its list applies) and reported per variable by the new Resolver.Warnings as *EnvError; String() names the skipped variables. Only an unusable HTTPS_PROXY / https_proxy, which explicitly names the TLS proxy, is still an error (an *EnvError naming it). parseProxyURL used url.Parse, which ends the authority at the first '/', '?' or '#'. Credentials holding those characters unescaped (base64 tokens) had part of the secret parsed as the proxy host, which then appeared in String()/Redact, in dial errors, and in a DNS lookup. The userinfo is now split off at the LAST '@' and percent-decoded by hand; only the part after it goes through url.Parse. Unusable values are rejected with errors that never echo credentials (including a "scheme" that may be a user name). Redact also withholds everything before the last '@' when handed a URL that url.Parse misparsed this way. Tests cover both findings at the resolver level and end to end through the in-process CONNECT proxy (Proxy-Authorization, dialed addresses, errors). Co-Authored-By: Claude Opus 5.5 (1M context) --- netproxy/netproxy.go | 237 +++++++++++++++++++++++++++----- netproxy/zz_dialer_test.go | 64 +++++++++ netproxy/zz_resolver_test.go | 254 ++++++++++++++++++++++++++++++++++- 3 files changed, 520 insertions(+), 35 deletions(-) diff --git a/netproxy/netproxy.go b/netproxy/netproxy.go index 0ed447c..33ceaf6 100644 --- a/netproxy/netproxy.go +++ b/netproxy/netproxy.go @@ -64,8 +64,25 @@ type Resolver struct { noProxy noProxy // noProxyRaw is kept only for String(). noProxyRaw string + // warnings lists the proxy variables that were set but skipped because + // their values are unusable (*EnvError each). + warnings []error } +// EnvError reports a proxy environment variable whose value cannot be used. +// Its message names the variable and never includes the value's +// credentials. +type EnvError struct { + // Var is the environment variable, e.g. "HTTP_PROXY". + Var string + // Err says why the value is unusable. + Err error +} + +func (e *EnvError) Error() string { return e.Var + ": " + e.Err.Error() } + +func (e *EnvError) Unwrap() error { return e.Err } + // Off returns a Resolver that never uses a proxy. func Off() *Resolver { return &Resolver{mode: ModeOff} } @@ -73,7 +90,8 @@ func Off() *Resolver { return &Resolver{mode: ModeOff} } // and anything NO_PROXY would exempt — through proxyURL. The URL must use // the http or https scheme ("http://[user:pass@]host[:port]"); a URL with no // scheme is taken as http. The port defaults to 80 for http and 443 for -// https. +// https. Everything up to the last "@" is the userinfo, so credentials may +// hold unescaped '/', '?', '#' or '@'; percent-escapes in them are decoded. func Explicit(proxyURL string) (*Resolver, error) { if strings.TrimSpace(proxyURL) == "" { return nil, errors.New("netproxy: empty proxy URL") @@ -89,51 +107,94 @@ func Explicit(proxyURL string) (*Resolver, error) { // conventional proxy environment variables, taken now: // // - TLS and raw TCP targets (ProxyForAddr, and https:// / wss:// requests) -// use the first non-empty of HTTPS_PROXY, https_proxy, ALL_PROXY, +// use the first usable one of HTTPS_PROXY, https_proxy, ALL_PROXY, // all_proxy. -// - Plain http:// / ws:// requests use HTTP_PROXY or http_proxy when set, -// and otherwise the same proxy as TLS targets. HTTP_PROXY (upper case) is -// ignored when REQUEST_METHOD is set, as net/http does, so a CGI request -// header cannot inject a proxy. +// - Plain http:// / ws:// requests use the first usable one of HTTP_PROXY, +// http_proxy, and otherwise the same proxy as TLS targets. HTTP_PROXY +// (upper case) is ignored when REQUEST_METHOD is set, as net/http does, +// so a CGI request header cannot inject a proxy. // - NO_PROXY / no_proxy exempts targets: a comma- or space-separated list // of host names (matching the name and its subdomains; a leading "." or // "*." matches subdomains only), IP addresses, CIDR ranges, each // optionally with ":port", or "*" for everything. localhost and loopback // addresses are always exempt. // -// An environment with no proxy variables yields a Resolver that never -// proxies (Enabled reports false). A malformed proxy URL is an error that -// never echoes the URL's credentials. +// Each variable is parsed on its own, so one bad value never disables +// another. A value is unusable when it is malformed or names a scheme other +// than http or https (socks5://, for example). An unusable HTTP_PROXY, +// http_proxy, ALL_PROXY or all_proxy is skipped (the next variable in its +// list applies) and reported by Warnings. Only an unusable HTTPS_PROXY or +// https_proxy, which explicitly names the TLS proxy, makes FromEnvironment +// fail, with an *EnvError naming it. +// +// An environment with no usable proxy variables yields a Resolver that +// never proxies (Enabled reports false). Neither errors nor warnings ever +// echo a value's credentials. func FromEnvironment() (*Resolver, error) { return fromEnv(os.Getenv) } func fromEnv(getenv func(string) string) (*Resolver, error) { r := &Resolver{mode: ModeAuto} - secureRaw, secureVar := firstEnv(getenv, "HTTPS_PROXY", "https_proxy", "ALL_PROXY", "all_proxy") - if secureRaw != "" { - u, err := parseProxyURL(secureRaw) + for _, name := range []string{"HTTPS_PROXY", "https_proxy", "ALL_PROXY", "all_proxy"} { + u, err := envProxy(getenv, name) if err != nil { - return nil, fmt.Errorf("%s: %w", secureVar, err) + if name == "HTTPS_PROXY" || name == "https_proxy" { + return nil, err + } + r.warnings = append(r.warnings, err) + continue + } + if u != nil { + r.secure = u + break } - r.secure = u } plainVars := []string{"HTTP_PROXY", "http_proxy"} if getenv("REQUEST_METHOD") != "" { plainVars = plainVars[1:] } - if plainRaw, plainVar := firstEnv(getenv, plainVars...); plainRaw != "" { - u, err := parseProxyURL(plainRaw) + for _, name := range plainVars { + u, err := envProxy(getenv, name) if err != nil { - return nil, fmt.Errorf("%s: %w", plainVar, err) + r.warnings = append(r.warnings, err) + continue + } + if u != nil { + r.plain = u + break } - r.plain = u } r.noProxyRaw, _ = firstEnv(getenv, "NO_PROXY", "no_proxy") r.noProxy = parseNoProxy(r.noProxyRaw) return r, nil } +// envProxy parses one proxy variable: nil, nil when it is unset or blank, +// and an *EnvError when its value is unusable. +func envProxy(getenv func(string) string, name string) (*url.URL, error) { + raw := strings.TrimSpace(getenv(name)) + if raw == "" { + return nil, nil + } + u, err := parseProxyURL(raw) + if err != nil { + return nil, &EnvError{Var: name, Err: err} + } + return u, nil +} + +// Warnings reports the proxy environment variables FromEnvironment skipped +// because their values are unusable, one *EnvError per variable, in +// precedence order. It is empty for other Resolvers. The messages are safe +// to log. +func (r *Resolver) Warnings() []error { + if r == nil || len(r.warnings) == 0 { + return nil + } + return append([]error(nil), r.warnings...) +} + // Parse builds a Resolver from a -proxy style setting: "auto" (or "") reads // the environment via FromEnvironment, "off" disables proxying, and anything // else is an explicit proxy URL (see Explicit). Deciding whether "auto" @@ -183,7 +244,7 @@ func (r *Resolver) String() string { return Redact(r.fixed) case ModeAuto: if !r.Enabled() { - return "auto: no proxy in environment" + return "auto: no proxy in environment" + r.ignoredSuffix() } s := "auto: " + Redact(r.secure) if r.secure == nil { @@ -194,11 +255,27 @@ func (r *Resolver) String() string { if r.noProxyRaw != "" { s += " (NO_PROXY=" + r.noProxyRaw + ")" } - return s + return s + r.ignoredSuffix() } return ModeOff } +// ignoredSuffix names the skipped variables for String, e.g. +// " (ignored unusable HTTP_PROXY, ALL_PROXY)". +func (r *Resolver) ignoredSuffix() string { + if len(r.warnings) == 0 { + return "" + } + names := make([]string, 0, len(r.warnings)) + for _, w := range r.warnings { + var ee *EnvError + if errors.As(w, &ee) { + names = append(names, ee.Var) + } + } + return " (ignored unusable " + strings.Join(names, ", ") + ")" +} + // ProxyForAddr returns the proxy to tunnel a raw TCP (or TLS) connection to // addr ("host:port") through, or nil to dial directly. addr is inspected as // text only; host names are never resolved. @@ -260,6 +337,12 @@ func (r *Resolver) pick(host, port string, secure bool) *url.URL { // Redact renders a proxy URL for logging as scheme://host:port, replacing // any userinfo (user name and password alike) with "***" and dropping the // path, query and fragment. Redact(nil) is "". +// +// A URL with "@" in its path, query, fragment or opaque part is taken to be +// a proxy URL whose credentials held an unescaped '/', '?' or '#', which +// url.Parse splits so that part of the credentials lands in Host. Redact +// then withholds everything before the last "@" and shows only what follows +// it as the host. func Redact(u *url.URL) string { if u == nil { return "" @@ -269,6 +352,15 @@ func Redact(u *url.URL) string { b.WriteString(u.Scheme) b.WriteString("://") } + if tail := u.Opaque + u.Path + "?" + u.RawQuery + "#" + u.Fragment; strings.Contains(tail, "@") { + host := tail[strings.LastIndex(tail, "@")+1:] + if i := strings.IndexAny(host, "/?#"); i >= 0 { + host = host[:i] + } + b.WriteString("***@") + b.WriteString(host) + return b.String() + } if u.User != nil { b.WriteString("***@") } @@ -276,19 +368,52 @@ func Redact(u *url.URL) string { return b.String() } -// parseProxyURL validates a proxy URL. Errors never include the raw value -// when it could carry credentials. +// errWithheld is the parse error for a proxy URL with credentials: the +// value is never echoed. +var errWithheld = errors.New("netproxy: invalid proxy URL (value withheld: it contains credentials)") + +// parseProxyURL validates a proxy URL. Errors never include credentials. +// +// The userinfo is split off by hand at the LAST "@" rather than by +// url.Parse, which ends the authority at the first '/', '?' or '#'. Proxy +// credentials are often tokens that contain those characters unescaped +// (base64 uses '/'); url.Parse would take part of such a token as the proxy +// host, which then shows up in logs and errors and is looked up in DNS. +// Here everything before the last "@" is credentials, and is only ever +// percent-decoded and sent in Proxy-Authorization. func parseProxyURL(raw string) (*url.URL, error) { s := strings.TrimSpace(raw) - if !strings.Contains(s, "://") { + scheme, rest, ok := strings.Cut(s, "://") + if !ok || !validScheme(scheme) { // "proxy.internal:3128" / "user:pass@proxy:3128": the scheme is // conventionally optional and means http. - s = "http://" + s + scheme, rest = "http", s + } + scheme = strings.ToLower(scheme) + switch scheme { + case "http", "https": + default: + if strings.Contains(rest, "@") && !knownScheme[scheme] { + // The "scheme" could be a user name ("user://pass@host"). + return nil, errors.New("netproxy: unsupported proxy scheme (want http or https)") + } + return nil, fmt.Errorf("netproxy: unsupported proxy scheme %q (want http or https)", scheme) + } + + var user *url.Userinfo + hostPart := rest + if at := strings.LastIndex(rest, "@"); at >= 0 { + var err error + if user, err = parseUserinfo(rest[:at]); err != nil { + return nil, err + } + hostPart = rest[at+1:] } - u, err := url.Parse(s) + // hostPart holds no credentials: everything up to the last "@" is gone. + u, err := url.Parse(scheme + "://" + hostPart) if err != nil { - if strings.Contains(s, "@") { - return nil, errors.New("netproxy: invalid proxy URL (value withheld: it contains credentials)") + if user != nil { + return nil, errWithheld } var ue *url.Error if errors.As(err, &ue) { @@ -296,18 +421,64 @@ func parseProxyURL(raw string) (*url.URL, error) { } return nil, fmt.Errorf("netproxy: invalid proxy URL: %v", err) } - u.Scheme = strings.ToLower(u.Scheme) - switch u.Scheme { - case "http", "https": - default: - return nil, fmt.Errorf("netproxy: unsupported proxy scheme %q (want http or https)", u.Scheme) - } + u.User = user if u.Hostname() == "" { return nil, fmt.Errorf("netproxy: proxy URL %s has no host", Redact(u)) } return u, nil } +// parseUserinfo decodes "user[:password]" (percent-escapes allowed, other +// characters taken literally). The error never includes the value. An +// empty userinfo ("http://@proxy") means no credentials. +func parseUserinfo(s string) (*url.Userinfo, error) { + if s == "" { + return nil, nil + } + for i := 0; i < len(s); i++ { + if c := s[i]; c < ' ' || c == 0x7f { + return nil, errWithheld + } + } + name, password, hasPassword := strings.Cut(s, ":") + name, err := url.PathUnescape(name) + if err != nil { + return nil, errWithheld + } + if !hasPassword { + return url.User(name), nil + } + if password, err = url.PathUnescape(password); err != nil { + return nil, errWithheld + } + return url.UserPassword(name, password), nil +} + +// validScheme reports whether s is a syntactically valid URL scheme +// (RFC 3986: ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )). +func validScheme(s string) bool { + if s == "" { + return false + } + for i := 0; i < len(s); i++ { + c := s[i] + switch { + case 'a' <= c && c <= 'z', 'A' <= c && c <= 'Z': + case i > 0 && ('0' <= c && c <= '9' || c == '+' || c == '-' || c == '.'): + default: + return false + } + } + return true +} + +// knownScheme lists schemes that are safe to echo in an error even when +// the value carries credentials. +var knownScheme = map[string]bool{ + "socks": true, "socks4": true, "socks4a": true, "socks5": true, "socks5h": true, + "ftp": true, "ws": true, "wss": true, "quic": true, "h2": true, "file": true, +} + // proxyHostPort is the address to dial for the proxy itself, with the // scheme's default port filled in. func proxyHostPort(u *url.URL) string { diff --git a/netproxy/zz_dialer_test.go b/netproxy/zz_dialer_test.go index 1561cbc..a10e2aa 100644 --- a/netproxy/zz_dialer_test.go +++ b/netproxy/zz_dialer_test.go @@ -528,3 +528,67 @@ func waitFor(t *testing.T, what string, cond func() bool) { } t.Fatalf("timed out waiting for %s", what) } + +// The Muse shape end to end: a valid HTTPS_PROXY whose credentials hold +// unescaped '/', '?' and '#', next to an HTTP_PROXY Go cannot use. The +// registry dial must still tunnel through the proxy with the right +// Proxy-Authorization, only the real proxy address may be dialed (no DNS +// lookup of a credential fragment), and a failed dial must not echo the +// secret. +func TestDialerUnescapedCredentialsBesideUnusableHTTPProxy(t *testing.T) { + t.Parallel() + const user, pass = "tok/en?x", "pa#ss/w?rd" + echoIP, echoPort := newEchoServer(t) + proxy := newTestProxy(t, map[string]string{echoHost: echoIP}, withAuth(user, pass)) + r, err := fromEnv(envMap(map[string]string{ + "HTTPS_PROXY": "http://" + user + ":" + pass + "@" + proxy.addr(), + "http_proxy": "socks5h://127.0.0.1:1080", + })) + if err != nil { + t.Fatalf("fromEnv: %v", err) + } + if len(r.Warnings()) != 1 { + t.Fatalf("Warnings() = %v", r.Warnings()) + } + fwd := &recordingForward{} + d := &Dialer{Resolver: r, Forward: fwd.dial} + target := net.JoinHostPort(echoHost, echoPort) + c, err := d.DialContext(context.Background(), "tcp", target) + if err != nil { + t.Fatalf("dial through proxy: %v", err) + } + roundTrip(t, c, "muse") + c.Close() + targets, _, auths := proxy.seen() + if len(targets) != 1 || targets[0] != target || auths[0] != basicAuth(user, pass) { + t.Fatalf("proxy saw targets %q auths %q", targets, auths) + } + if got := fwd.seen(); len(got) != 1 || got[0] != proxy.addr() { + t.Fatalf("dialed %q, want only the proxy %s", got, proxy.addr()) + } + + // Same credentials, proxy down: the error names the real proxy only. + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + dead := ln.Addr().String() + ln.Close() + fwd = &recordingForward{} + d = &Dialer{Resolver: mustExplicit(t, "http://"+user+":"+pass+"@"+dead), Forward: fwd.dial} + _, err = d.DialContext(context.Background(), "tcp", "registry.pilot.invalid:443") + if err == nil { + t.Fatal("dial through a dead proxy succeeded") + } + for _, secret := range []string{"tok", "en?x", "pa#", "ss/w", "rd"} { + if strings.Contains(err.Error(), secret) || strings.Contains(d.Resolver.String(), secret) { + t.Fatalf("secret fragment %q leaked: err=%q resolver=%q", secret, err, d.Resolver) + } + } + if !strings.Contains(err.Error(), "http://***@"+dead) { + t.Fatalf("error should name the redacted real proxy: %q", err) + } + if got := fwd.seen(); len(got) != 1 || got[0] != dead { + t.Fatalf("dialed %q, want only %s", got, dead) + } +} diff --git a/netproxy/zz_resolver_test.go b/netproxy/zz_resolver_test.go index 9288f98..173ba66 100644 --- a/netproxy/zz_resolver_test.go +++ b/netproxy/zz_resolver_test.go @@ -3,6 +3,7 @@ package netproxy import ( + "errors" "net/http" "net/url" "strings" @@ -252,8 +253,257 @@ func TestParseErrorsNeverLeakCredentials(t *testing.T) { if _, err := Explicit(" "); err == nil { t.Fatal("empty explicit URL accepted") } - if _, err := fromEnv(envMap(map[string]string{"HTTP_PROXY": "gopher://x:1"})); err == nil || !strings.HasPrefix(err.Error(), "HTTP_PROXY: ") { - t.Fatalf("bad HTTP_PROXY: %v", err) + // An unusable HTTP_PROXY is a warning, not an error (see + // TestFromEnvVariablesAreIndependent), and is just as tight-lipped. + r, err := fromEnv(envMap(map[string]string{"HTTP_PROXY": "gopher://user:hunter2@x:1"})) + if err != nil { + t.Fatalf("bad HTTP_PROXY failed the resolver: %v", err) + } + w := r.Warnings() + if len(w) != 1 || !strings.HasPrefix(w[0].Error(), "HTTP_PROXY: ") || strings.Contains(w[0].Error(), "hunter2") || strings.Contains(w[0].Error(), "user") { + t.Fatalf("bad HTTP_PROXY warning: %v", w) + } +} + +// A proxy variable that cannot be used must not take down another one that +// can: a Muse-style VM with a valid HTTPS_PROXY and an http_proxy that Go +// cannot use still proxies registry traffic. +func TestFromEnvVariablesAreIndependent(t *testing.T) { + t.Parallel() + const ( + registry = "registry.pilotprotocol.network:443" + secure = "http://u:p@egress:3128" + ) + cases := []struct { + name string + env map[string]string + wantTLS string // proxy for registry and https:// requests + wantPlain string // proxy for http:// requests + wantWarned []string // variables reported by Warnings, in order + }{ + { + name: "socks HTTP_PROXY beside a valid HTTPS_PROXY", + env: map[string]string{"HTTPS_PROXY": secure, "HTTP_PROXY": "socks5://proxy:1080"}, + wantTLS: secure, + wantPlain: secure, + wantWarned: []string{"HTTP_PROXY"}, + }, + { + name: "garbage http_proxy beside a valid https_proxy", + env: map[string]string{"https_proxy": secure, "http_proxy": "garbage with space"}, + wantTLS: secure, + wantPlain: secure, + wantWarned: []string{"http_proxy"}, + }, + { + name: "unusable HTTP_PROXY falls through to http_proxy", + env: map[string]string{"HTTPS_PROXY": secure, "HTTP_PROXY": "socks5h://x:1", "http_proxy": "http://plain:8080"}, + wantTLS: secure, + wantPlain: "http://plain:8080", + wantWarned: []string{"HTTP_PROXY"}, + }, + { + name: "socks ALL_PROXY alone is ignored", + env: map[string]string{"ALL_PROXY": "socks5://127.0.0.1:1080"}, + wantWarned: []string{"ALL_PROXY"}, + }, + { + name: "unusable ALL_PROXY falls through to all_proxy", + env: map[string]string{"ALL_PROXY": "socks5://127.0.0.1:1080", "all_proxy": secure}, + wantTLS: secure, + wantPlain: secure, + wantWarned: []string{"ALL_PROXY"}, + }, + { + name: "everything unusable except HTTPS_PROXY", + env: map[string]string{"HTTPS_PROXY": secure, "ALL_PROXY": "socks5://a:1", "HTTP_PROXY": "http://bad port", "http_proxy": "ftp://c:21"}, + wantTLS: secure, + wantPlain: secure, + wantWarned: []string{"HTTP_PROXY", "http_proxy"}, + }, + } + for _, tc := range cases { + r, err := fromEnv(envMap(tc.env)) + if err != nil { + t.Fatalf("%s: fromEnv: %v", tc.name, err) + } + if r.Enabled() != (tc.wantTLS != "") { + t.Fatalf("%s: Enabled() = %v", tc.name, r.Enabled()) + } + if got := proxyFor(t, r, registry); got != tc.wantTLS { + t.Fatalf("%s: registry proxy = %q, want %q", tc.name, got, tc.wantTLS) + } + if got := proxyForURL(t, r, "https://"+registry+"/"); got != tc.wantTLS { + t.Fatalf("%s: https proxy = %q, want %q", tc.name, got, tc.wantTLS) + } + if got := proxyForURL(t, r, "http://plain.pilot.invalid/"); got != tc.wantPlain { + t.Fatalf("%s: http proxy = %q, want %q", tc.name, got, tc.wantPlain) + } + var warned []string + for _, w := range r.Warnings() { + var ee *EnvError + if !errors.As(w, &ee) { + t.Fatalf("%s: warning %v is not an *EnvError", tc.name, w) + } + if !strings.HasPrefix(w.Error(), ee.Var+": netproxy: ") { + t.Fatalf("%s: warning %q does not name its variable", tc.name, w) + } + warned = append(warned, ee.Var) + } + if strings.Join(warned, ",") != strings.Join(tc.wantWarned, ",") { + t.Fatalf("%s: warned about %q, want %q", tc.name, warned, tc.wantWarned) + } + if s := r.String(); !strings.Contains(s, "(ignored unusable "+strings.Join(tc.wantWarned, ", ")+")") || strings.Contains(s, "u:p") { + t.Fatalf("%s: String() = %q", tc.name, s) + } + } + + // Warnings are a copy, and absent for non-environment resolvers. + r, _ := fromEnv(envMap(map[string]string{"HTTP_PROXY": "socks5://x:1"})) + r.Warnings()[0] = nil + if r.Warnings()[0] == nil { + t.Fatal("Warnings exposed the resolver's slice") + } + var nilResolver *Resolver + for _, other := range []*Resolver{nilResolver, Off(), mustExplicit(t, secure)} { + if w := other.Warnings(); w != nil { + t.Fatalf("%s: Warnings() = %v", other, w) + } + } + if got := r.String(); got != "auto: no proxy in environment (ignored unusable HTTP_PROXY)" { + t.Fatalf("String() = %q", got) + } +} + +// Only the variables that explicitly name the TLS proxy are fatal when +// unusable; the error names the variable and withholds the value. +func TestFromEnvUnusableTLSProxyIsAnError(t *testing.T) { + t.Parallel() + for _, env := range []map[string]string{ + {"HTTPS_PROXY": "socks5://user:hunter2@a:1", "https_proxy": "http://ok:1", "HTTP_PROXY": "http://ok:1"}, + {"https_proxy": "http://user:hunter2@a:bad", "ALL_PROXY": "http://ok:1"}, + } { + _, err := fromEnv(envMap(env)) + var ee *EnvError + if !errors.As(err, &ee) { + t.Fatalf("%v: err = %v, want *EnvError", env, err) + } + if _, set := env["HTTPS_PROXY"]; (set && ee.Var != "HTTPS_PROXY") || (!set && ee.Var != "https_proxy") { + t.Fatalf("%v: error names %q", env, ee.Var) + } + if strings.Contains(err.Error(), "hunter2") || strings.Contains(err.Error(), "user") { + t.Fatalf("%v: error leaks credentials: %v", env, err) + } + } +} + +// Proxy credentials often hold unescaped '/', '?' or '#' (base64 tokens). +// url.Parse ends the authority at the first of those, which would turn part +// of the secret into the proxy host. The userinfo runs to the LAST '@', so +// the credentials decode intact and never appear in String, Redact, errors +// or a DNS lookup. +func TestUnescapedDelimitersInUserinfo(t *testing.T) { + t.Parallel() + cases := []struct { + raw, user, pass, host string + hasPass bool + secrets []string + }{ + {"http://abcDEF/ghi+jkl@proxy.muse:3128", "abcDEF/ghi+jkl", "", "proxy.muse:3128", false, []string{"abcDEF", "ghi"}}, + {"http://user:9876/zz@proxy:3128", "user", "9876/zz", "proxy:3128", true, []string{"user", "9876"}}, + {"http://user:9876?zz@proxy:3128", "user", "9876?zz", "proxy:3128", true, []string{"user", "9876"}}, + {"http://user:98#76@proxy:3128", "user", "98#76", "proxy:3128", true, []string{"user", "98"}}, + {"http://AbC/dEf+ghi==@proxy:3128", "AbC/dEf+ghi==", "", "proxy:3128", false, []string{"AbC", "dEf"}}, + {"https://tok:a/b?c#d@egress.internal", "tok", "a/b?c#d", "egress.internal", true, []string{"tok", "a/b"}}, + {"user:pa/ss@proxy:3128", "user", "pa/ss", "proxy:3128", true, []string{"user", "pa/ss"}}, + {"user:pa://ss@proxy:3128", "user", "pa://ss", "proxy:3128", true, []string{"user", "pa:"}}, + {"http://us@er:p@ss@proxy:3128", "us@er", "p@ss", "proxy:3128", true, []string{"us@er", "p@ss"}}, + {"http://u%2Fx:p%23w/q@proxy:3128/", "u/x", "p#w/q", "proxy:3128", true, []string{"u%2F", "p%23"}}, + } + for _, tc := range cases { + explicit, err := Explicit(tc.raw) + if err != nil { + t.Fatalf("Explicit(%q): %v", tc.raw, err) + } + auto, err := fromEnv(envMap(map[string]string{"HTTPS_PROXY": tc.raw})) + if err != nil { + t.Fatalf("HTTPS_PROXY=%q: %v", tc.raw, err) + } + for _, r := range []*Resolver{explicit, auto} { + u, err := r.ProxyForAddr("registry.pilotprotocol.network:443") + if err != nil || u == nil { + t.Fatalf("%q: ProxyForAddr = %v, %v", tc.raw, u, err) + } + if u.Host != tc.host { + t.Fatalf("%q: proxy host %q, want %q", tc.raw, u.Host, tc.host) + } + pass, hasPass := u.User.Password() + if u.User.Username() != tc.user || pass != tc.pass || hasPass != tc.hasPass { + t.Fatalf("%q: userinfo %q/%q/%v, want %q/%q/%v", tc.raw, u.User.Username(), pass, hasPass, tc.user, tc.pass, tc.hasPass) + } + // The URL round-trips: what net/http reads back is the same. + if back, err := url.Parse(u.String()); err != nil || back.Host != tc.host || back.User.String() != u.User.String() { + t.Fatalf("%q: String() %q does not round-trip: %v", tc.raw, u.String(), err) + } + for _, logged := range []string{r.String(), Redact(u)} { + if !strings.Contains(logged, "***@"+tc.host) { + t.Fatalf("%q: logged form %q does not show the real host", tc.raw, logged) + } + for _, secret := range tc.secrets { + if strings.Contains(logged, secret) { + t.Fatalf("%q: logged form %q leaks %q", tc.raw, logged, secret) + } + } + } + } + } +} + +// Values that stay unusable are rejected without echoing any part of the +// credentials, however the delimiters fall. +func TestUnusableUserinfoNeverLeaks(t *testing.T) { + t.Parallel() + for _, raw := range []string{ + "http://se/cret:hunter2@proxy:bad-port", + "http://se?cret:hun#ter2@", + "http://se/cret:hun%zzter2@proxy:1", + "http://se/cret:hunter2\x7f@proxy:1", + "http://se/cret:hunter2@proxy%zz:1", + "secret://hunter2@proxy:1", + "socks5://se/cret:hunter2@proxy:1", + } { + _, err := Explicit(raw) + if err == nil { + t.Fatalf("Explicit(%q) accepted", raw) + } + for _, secret := range []string{"se/cret", "se?cret", "cret", "hunter2", "hun", "secret"} { + if strings.Contains(err.Error(), secret) { + t.Fatalf("Explicit(%q) error %q leaks %q", raw, err, secret) + } + } + } +} + +func TestRedactMisparsedURL(t *testing.T) { + t.Parallel() + // URLs built by url.Parse from unescaped credentials: the secret sits + // in Host (and Path/Query/Fragment), the real host after the last '@'. + cases := map[string]string{ + "http://abcDEF/ghi+jkl@proxy.muse:3128": "http://***@proxy.muse:3128", + "http://user:9876/zz@proxy:3128": "http://***@proxy:3128", + "http://user:9876?zz@proxy:3128": "http://***@proxy:3128", + "http://user:98#76@proxy:3128": "http://***@proxy:3128", + "http://u@x/pa@ss@proxy:3128/": "http://***@proxy:3128", + "http:user/pw@proxy:1": "http://***@proxy:1", + } + for in, want := range cases { + u, err := url.Parse(in) + if err != nil { + t.Fatalf("url.Parse(%q): %v", in, err) + } + if got := Redact(u); got != want { + t.Fatalf("Redact(%q) = %q, want %q", in, got, want) + } } } From 6496dc8eda0e222076e1fd7a3fbe17385d40c0bf Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Thu, 24 Sep 2026 00:58:39 +0300 Subject: [PATCH 4/4] build: require go 1.25.13 (stdlib security fixes) The required govulncheck check has failed on every PR since mid-August: go1.25.12's standard library carries GO-2026-6218 (net/url), GO-2026-6090 (crypto/tls), GO-2026-5972 (encoding/asn1) and GO-2026-5026 (net/http idna), all fixed in go1.25.13 and reachable from code already on main (registry/wire, decision, driver). CI takes its Go version from go.mod. pilotprotocol already requires go 1.25.13, so this adds no new requirement for it. With 1.25.13: govulncheck reports no vulnerabilities. Co-Authored-By: Claude Opus 5.5 (1M context) --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index c860d5f..0b25dc8 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ module github.com/pilot-protocol/common -go 1.25.12 +go 1.25.13