From 7f3bbc98657182da74fa497d63efc5bbd68b0a0f Mon Sep 17 00:00:00 2001 From: Achille Roussel Date: Sun, 31 May 2026 08:54:20 -0700 Subject: [PATCH] net/netutil: add NewDefaultTransport to avoid http.DefaultTransport panics Several packages built their HTTP transports with http.DefaultTransport.(*http.Transport).Clone() The standard library only documents http.DefaultTransport as an http.RoundTripper, so an application is free to replace it with a RoundTripper that is not a *http.Transport (e.g. an instrumented or tracing wrapper). When such an application embeds tsnet.Server, the unchecked type assertion panics as soon as tsnet brings up its control connection, DNS bootstrap, or log uploader. Add netutil.NewDefaultTransport, which returns a clone of the global when it is still the standard *http.Transport (preserving existing behavior) and otherwise returns a fresh transport mirroring the stdlib defaults. Route every clone site through it. Updates #19937 Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Achille Roussel --- clientupdate/distsign/distsign.go | 3 +- cmd/tailscale/cli/debug.go | 3 +- control/controlclient/direct.go | 3 +- control/controlhttp/client.go | 2 +- k8s-operator/api-proxy/proxy.go | 3 +- logpolicy/logpolicy.go | 3 +- net/dnsfallback/dnsfallback.go | 3 +- net/netutil/netutil.go | 30 +++++++++++++++++ net/netutil/netutil_test.go | 55 +++++++++++++++++++++++++++++++ net/tsdial/tsdial.go | 3 +- prober/derp.go | 3 +- prober/http.go | 4 ++- sessionrecording/connect.go | 3 +- 13 files changed, 107 insertions(+), 11 deletions(-) diff --git a/clientupdate/distsign/distsign.go b/clientupdate/distsign/distsign.go index c804b855c..c13ed8497 100644 --- a/clientupdate/distsign/distsign.go +++ b/clientupdate/distsign/distsign.go @@ -56,6 +56,7 @@ "github.com/hdevalence/ed25519consensus" "golang.org/x/crypto/blake2s" "tailscale.com/feature" + "tailscale.com/net/netutil" "tailscale.com/types/logger" "tailscale.com/util/httpm" "tailscale.com/util/must" @@ -329,7 +330,7 @@ func fetch(url string, limit int64) ([]byte, error) { // download writes the response body of url into a local file at dst, up to // limit bytes. On success, the returned value is a BLAKE2s hash of the file. func (c *Client) download(ctx context.Context, url, dst string, limit int64) ([]byte, int64, error) { - tr := http.DefaultTransport.(*http.Transport).Clone() + tr := netutil.NewDefaultTransport() tr.Proxy = feature.HookProxyFromEnvironment.GetOrNil() defer tr.CloseIdleConnections() hc := &http.Client{ diff --git a/cmd/tailscale/cli/debug.go b/cmd/tailscale/cli/debug.go index 1e9314744..2697ac5d1 100644 --- a/cmd/tailscale/cli/debug.go +++ b/cmd/tailscale/cli/debug.go @@ -39,6 +39,7 @@ "tailscale.com/net/ace" "tailscale.com/net/dnscache" "tailscale.com/net/netmon" + "tailscale.com/net/netutil" "tailscale.com/net/tsaddr" "tailscale.com/net/tsdial" "tailscale.com/paths" @@ -989,7 +990,7 @@ func runTS2021(ctx context.Context, args []string) error { keysURL := "https://" + ts2021Args.host + "/key?v=" + strconv.Itoa(ts2021Args.version) - keyTransport := http.DefaultTransport.(*http.Transport).Clone() + keyTransport := netutil.NewDefaultTransport() if ts2021Args.aceHost != "" { log.Printf("using ACE server %q", ts2021Args.aceHost) keyTransport.Proxy = nil diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 8104ad0ec..062af8dd5 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -43,6 +43,7 @@ "tailscale.com/net/dnscache" "tailscale.com/net/dnsfallback" "tailscale.com/net/netmon" + "tailscale.com/net/netutil" "tailscale.com/net/netx" "tailscale.com/net/tlsdial" "tailscale.com/net/tsdial" @@ -347,7 +348,7 @@ func NewDirect(opts Options) (*Direct, error) { } var interceptedDial *atomic.Bool if httpc == nil { - tr := http.DefaultTransport.(*http.Transport).Clone() + tr := netutil.NewDefaultTransport() if buildfeatures.HasUseProxy { tr.Proxy = feature.HookProxyFromEnvironment.GetOrNil() if f, ok := feature.HookProxySetTransportGetProxyConnectHeader.GetOk(); ok { diff --git a/control/controlhttp/client.go b/control/controlhttp/client.go index 2aabcbb64..d93e19b95 100644 --- a/control/controlhttp/client.go +++ b/control/controlhttp/client.go @@ -459,7 +459,7 @@ func (a *Dialer) tryURLUpgrade(ctx context.Context, u *url.URL, optAddr netip.Ad }() } - tr := http.DefaultTransport.(*http.Transport).Clone() + tr := netutil.NewDefaultTransport() defer tr.CloseIdleConnections() if optACEHost != "" { // If using ACE, we don't want to use any HTTP proxy. diff --git a/k8s-operator/api-proxy/proxy.go b/k8s-operator/api-proxy/proxy.go index acc7b6234..73a9884b2 100644 --- a/k8s-operator/api-proxy/proxy.go +++ b/k8s-operator/api-proxy/proxy.go @@ -31,6 +31,7 @@ "tailscale.com/client/tailscale/apitype" ksr "tailscale.com/k8s-operator/sessionrecording" "tailscale.com/kube/kubetypes" + "tailscale.com/net/netutil" "tailscale.com/net/netx" "tailscale.com/sessionrecording" "tailscale.com/tailcfg" @@ -64,7 +65,7 @@ func NewAPIServerProxy(zlog *zap.SugaredLogger, restConfig *rest.Config, ts *tsn return nil, fmt.Errorf("could not get rest.TransportConfig(): %w", err) } - tr := http.DefaultTransport.(*http.Transport).Clone() + tr := netutil.NewDefaultTransport() tr.TLSClientConfig, err = transport.TLSConfigFor(cfg) if err != nil { return nil, fmt.Errorf("could not get transport.TLSConfigFor(): %w", err) diff --git a/logpolicy/logpolicy.go b/logpolicy/logpolicy.go index 7a0027dad..901a713f1 100644 --- a/logpolicy/logpolicy.go +++ b/logpolicy/logpolicy.go @@ -43,6 +43,7 @@ "tailscale.com/net/netknob" "tailscale.com/net/netmon" "tailscale.com/net/netns" + "tailscale.com/net/netutil" "tailscale.com/net/netx" "tailscale.com/net/tlsdial" "tailscale.com/paths" @@ -878,7 +879,7 @@ func (opts TransportOptions) New() http.RoundTripper { opts.NetMon = netmon.NewStatic() } // Start with a copy of http.DefaultTransport and tweak it a bit. - tr := http.DefaultTransport.(*http.Transport).Clone() + tr := netutil.NewDefaultTransport() if opts.TLSClientConfig != nil { tr.TLSClientConfig = opts.TLSClientConfig.Clone() } diff --git a/net/dnsfallback/dnsfallback.go b/net/dnsfallback/dnsfallback.go index 546712776..1d74826d9 100644 --- a/net/dnsfallback/dnsfallback.go +++ b/net/dnsfallback/dnsfallback.go @@ -30,6 +30,7 @@ "tailscale.com/health" "tailscale.com/net/netmon" "tailscale.com/net/netns" + "tailscale.com/net/netutil" "tailscale.com/net/tlsdial" "tailscale.com/tailcfg" "tailscale.com/types/logger" @@ -133,7 +134,7 @@ type nameIP struct { // ht may be nil. func bootstrapDNSMap(ctx context.Context, serverName string, serverIP netip.Addr, queryName string, logf logger.Logf, ht *health.Tracker, netMon *netmon.Monitor) (dnsMap, error) { dialer := netns.NewDialer(logf, netMon) - tr := http.DefaultTransport.(*http.Transport).Clone() + tr := netutil.NewDefaultTransport() tr.DisableKeepAlives = true // This transport is meant to be used once. tr.Proxy = feature.HookProxyFromEnvironment.GetOrNil() tr.DialContext = func(ctx context.Context, netw, addr string) (net.Conn, error) { diff --git a/net/netutil/netutil.go b/net/netutil/netutil.go index 138829885..e765138d0 100644 --- a/net/netutil/netutil.go +++ b/net/netutil/netutil.go @@ -8,10 +8,40 @@ "bufio" "io" "net" + "net/http" + "time" "tailscale.com/syncs" ) +// NewDefaultTransport returns a new *http.Transport configured identically to +// the Go standard library's http.DefaultTransport. +// +// Unlike http.DefaultTransport.(*http.Transport).Clone(), it does not panic +// when a program has replaced http.DefaultTransport with a RoundTripper that +// is not a *http.Transport. In the common case (the global is still the +// standard *http.Transport) it returns a clone of it, preserving the existing +// behavior exactly; otherwise it returns a fresh transport mirroring the +// stdlib defaults. +func NewDefaultTransport() *http.Transport { + if tr, ok := http.DefaultTransport.(*http.Transport); ok { + return tr.Clone() + } + // Values copied verbatim from net/http's DefaultTransport. + return &http.Transport{ + Proxy: http.ProxyFromEnvironment, + DialContext: (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + }).DialContext, + ForceAttemptHTTP2: true, + MaxIdleConns: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + } +} + // NewOneConnListener returns a net.Listener that returns c on its // first Accept and EOF thereafter. // diff --git a/net/netutil/netutil_test.go b/net/netutil/netutil_test.go index 2c40d8d9e..38be2f410 100644 --- a/net/netutil/netutil_test.go +++ b/net/netutil/netutil_test.go @@ -6,8 +6,10 @@ import ( "io" "net" + "net/http" "runtime" "testing" + "time" ) type conn struct { @@ -53,6 +55,59 @@ func TestOneConnListener(t *testing.T) { } } +// roundTripperFunc is an http.RoundTripper that is not a *http.Transport, +// used to exercise the fallback path of NewDefaultTransport. +type roundTripperFunc func(*http.Request) (*http.Response, error) + +func (f roundTripperFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +func TestNewDefaultTransport(t *testing.T) { + // Standard case: http.DefaultTransport is still a *http.Transport, so we + // get a clone of it with the stdlib defaults. + tr := NewDefaultTransport() + if tr == nil { + t.Fatal("got nil transport") + } + if got, want := tr.MaxIdleConns, 100; got != want { + t.Errorf("MaxIdleConns = %d; want %d", got, want) + } + if got, want := tr.IdleConnTimeout, 90*time.Second; got != want { + t.Errorf("IdleConnTimeout = %v; want %v", got, want) + } + if !tr.ForceAttemptHTTP2 { + t.Error("ForceAttemptHTTP2 = false; want true") + } + + // Regression case: an application has replaced http.DefaultTransport with + // a RoundTripper that is not a *http.Transport. NewDefaultTransport must + // not panic and must still return a usable transport with stdlib defaults. + orig := http.DefaultTransport + defer func() { http.DefaultTransport = orig }() + http.DefaultTransport = roundTripperFunc(func(*http.Request) (*http.Response, error) { + return nil, nil + }) + + tr = NewDefaultTransport() + if tr == nil { + t.Fatal("got nil transport on fallback path") + } + if got, want := tr.MaxIdleConns, 100; got != want { + t.Errorf("fallback MaxIdleConns = %d; want %d", got, want) + } + if got, want := tr.IdleConnTimeout, 90*time.Second; got != want { + t.Errorf("fallback IdleConnTimeout = %v; want %v", got, want) + } + if !tr.ForceAttemptHTTP2 { + t.Error("fallback ForceAttemptHTTP2 = false; want true") + } + if tr.DialContext == nil { + t.Error("fallback DialContext = nil; want non-nil") + } + if tr.Proxy == nil { + t.Error("fallback Proxy = nil; want non-nil") + } +} + func TestIPForwardingEnabledLinux(t *testing.T) { if runtime.GOOS != "linux" { t.Skipf("skipping on %s", runtime.GOOS) diff --git a/net/tsdial/tsdial.go b/net/tsdial/tsdial.go index d84d74f46..53fc97d0f 100644 --- a/net/tsdial/tsdial.go +++ b/net/tsdial/tsdial.go @@ -27,6 +27,7 @@ "tailscale.com/net/netknob" "tailscale.com/net/netmon" "tailscale.com/net/netns" + "tailscale.com/net/netutil" "tailscale.com/net/netx" "tailscale.com/net/tsaddr" "tailscale.com/syncs" @@ -684,7 +685,7 @@ func (d *Dialer) PeerAPIHTTPClient() *http.Client { panic("unreachable") } d.peerClientOnce.Do(func() { - t := http.DefaultTransport.(*http.Transport).Clone() + t := netutil.NewDefaultTransport() t.Dial = nil t.DialContext = d.dialPeerAPI // Do not use the environment proxy for PeerAPI. diff --git a/prober/derp.go b/prober/derp.go index dadda6fce..07208c4cd 100644 --- a/prober/derp.go +++ b/prober/derp.go @@ -36,6 +36,7 @@ "tailscale.com/derp" "tailscale.com/derp/derphttp" "tailscale.com/net/netmon" + "tailscale.com/net/netutil" "tailscale.com/net/stun" "tailscale.com/net/tstun" "tailscale.com/syncs" @@ -1210,7 +1211,7 @@ func newConn(ctx context.Context, dm *tailcfg.DERPMap, n *tailcfg.DERPNode, isPr var httpOrFileClient = &http.Client{Transport: httpOrFileTransport()} func httpOrFileTransport() http.RoundTripper { - tr := http.DefaultTransport.(*http.Transport).Clone() + tr := netutil.NewDefaultTransport() tr.RegisterProtocol("file", http.NewFileTransport(http.Dir("/"))) return tr } diff --git a/prober/http.go b/prober/http.go index 144ed3fb5..53996e655 100644 --- a/prober/http.go +++ b/prober/http.go @@ -9,6 +9,8 @@ "fmt" "io" "net/http" + + "tailscale.com/net/netutil" ) const maxHTTPBody = 4 << 20 // MiB @@ -35,7 +37,7 @@ func probeHTTP(ctx context.Context, url string, want []byte) error { // Get a completely new transport each time, so we don't reuse a // past connection. - tr := http.DefaultTransport.(*http.Transport).Clone() + tr := netutil.NewDefaultTransport() defer tr.CloseIdleConnections() c := &http.Client{ Transport: tr, diff --git a/sessionrecording/connect.go b/sessionrecording/connect.go index 6135688ca..927ba7d30 100644 --- a/sessionrecording/connect.go +++ b/sessionrecording/connect.go @@ -18,6 +18,7 @@ "sync/atomic" "time" + "tailscale.com/net/netutil" "tailscale.com/net/netx" "tailscale.com/tailcfg" "tailscale.com/util/httpm" @@ -380,7 +381,7 @@ func (u *readCounter) Read(buf []byte) (int, error) { // clientHTTP1 returns a claassic http.Client with a per-dial context. It uses // dialCtx and adds a 5s timeout to it. func clientHTTP1(dialCtx context.Context, dial netx.DialFunc) *http.Client { - tr := http.DefaultTransport.(*http.Transport).Clone() + tr := netutil.NewDefaultTransport() tr.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { perAttemptCtx, cancel := context.WithTimeout(ctx, perDialAttemptTimeout) defer cancel()