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()