derphttp: add TLSConfigBypassesTLSDial opt-in flag

Today derphttp.Client.tlsConfig always passes the caller-supplied
TLSConfig through tlsdial.Config, which wraps it with a VerifyConnection
hook that runs system-root verification with a baked-in Let's Encrypt
fallback. tlsdial.Config also panics on base configs that already set
InsecureSkipVerify or VerifyConnection.

That contract works well when DERP is reachable directly over a publicly
trusted PKI, but it breaks for callers who legitimately need to perform
their own server verification — for example when DERP is fronted by a
reverse proxy that presents a non-publicly-trusted certificate, or when
authenticating with an mTLS framework that uses custom CAs / SPIFFE-style
identity / app-name verification (i.e. the standard Go pattern of
InsecureSkipVerify=true paired with a custom VerifyPeerCertificate).

This adds an opt-in TLSConfigBypassesTLSDial bool. When true (and
TLSConfig is non-nil), the supplied config is used as-is after a Clone +
ServerName fallback. tlsdial.Config is bypassed entirely. node-level
InsecureForTests is still honored; node.CertName (a tlsdial-specific
domain-fronting hook) is intentionally ignored on the bypass path —
callers bringing their own verifier are expected to encode any cert
pinning in their own VerifyPeerCertificate / VerifyConnection.

Plumbing:
  - derphttp.Client.TLSConfigBypassesTLSDial bool
  - magicsock.Conn.derpTLSConfigBypassesTLSDial atomic +
    Conn.SetDERPTLSConfigBypassesTLSDial setter
  - magicsock/derp.go propagates the flag onto the constructed DERP
    client alongside the existing TLSConfig plumbing.

Backward compatible: bypass=false (the default) preserves the existing
behavior and existing callers see no change. New unit tests cover the
default path, the bypass path, ServerName behavior, the nil-config
fallback, and node.InsecureForTests under bypass.

Author: ibdafna

derp/derphttp/derphttp_client.go

@@ -53,10 +53,33 @@ import (
 // has been called).
 type Client struct {
 	Header    http.Header
-	TLSConfig *tls.Config        // optional; nil means default
-	DNSCache  *dnscache.Resolver // optional; nil means no caching
-	MeshKey   string             // optional; for trusted clients
-	IsProber  bool               // optional; for probers to optional declare themselves as such
+	TLSConfig *tls.Config // optional; nil means default
+
+	// TLSConfigBypassesTLSDial, if true, causes TLSConfig to be used as-is
+	// (after a Clone and ServerName housekeeping) instead of being passed
+	// through tlsdial.Config. The default behavior wraps TLSConfig in
+	// Tailscale's hosted-DERP verification helper, which installs a
+	// VerifyConnection hook with a baked-in Let's Encrypt fallback and
+	// rejects (panics on) base configs that already set InsecureSkipVerify
+	// or VerifyConnection.
+	//
+	// Set this to true when the caller is responsible for server
+	// verification — e.g. when DERP is fronted by a reverse proxy that
+	// presents a non-publicly-trusted certificate, when using an mTLS
+	// framework that performs its own peer verification (custom CAs,
+	// SPIFFE-style identity), or any case in which the caller has supplied
+	// VerifyPeerCertificate / VerifyConnection on TLSConfig and does not
+	// want the bake-in fallback.
+	//
+	// When true, TLSConfig must be non-nil; the flag is ignored otherwise.
+	// Per-DERPNode overrides are still honored: InsecureForTests still
+	// disables verification, but CertName (which is implemented by
+	// tlsdial) is ignored.
+	TLSConfigBypassesTLSDial bool
+
+	DNSCache *dnscache.Resolver // optional; nil means no caching
+	MeshKey  string             // optional; for trusted clients
+	IsProber bool               // optional; for probers to optional declare themselves as such
 
 	// Allow forcing WebSocket fallback for situations where proxies do not
 	// play well with `Upgrade: derp`. Turning this on will cause the client to
@@ -704,14 +727,34 @@ func (c *Client) DialRegion(ctx context.Context, reg *tailcfg.DERPRegion) (net.C
 }
 
 func (c *Client) tlsConfig(node *tailcfg.DERPNode) *tls.Config {
-	tlsConf := tlsdial.Config(c.tlsServerName(node), c.TLSConfig)
-	if node != nil {
-		if node.InsecureForTests {
+	var tlsConf *tls.Config
+	if c.TLSConfigBypassesTLSDial && c.TLSConfig != nil {
+		// Caller has opted to bring their own server verification (custom
+		// CAs / mTLS / SPIFFE-style identity / non-publicly-trusted PKI).
+		// Use the supplied config as-is, only filling in ServerName when
+		// the caller didn't pin one. node.CertName (a tlsdial-specific
+		// domain-fronting hook) is intentionally not applied here; callers
+		// using bypass are expected to encode any cert-pinning in their
+		// own VerifyPeerCertificate / VerifyConnection.
+		tlsConf = c.TLSConfig.Clone()
+		if tlsConf.ServerName == "" {
+			tlsConf.ServerName = c.tlsServerName(node)
+		}
+		if node != nil && node.InsecureForTests {
 			tlsConf.InsecureSkipVerify = true
 			tlsConf.VerifyConnection = nil
+			tlsConf.VerifyPeerCertificate = nil
 		}
-		if node.CertName != "" {
-			tlsdial.SetConfigExpectedCert(tlsConf, node.CertName)
+	} else {
+		tlsConf = tlsdial.Config(c.tlsServerName(node), c.TLSConfig)
+		if node != nil {
+			if node.InsecureForTests {
+				tlsConf.InsecureSkipVerify = true
+				tlsConf.VerifyConnection = nil
+			}
+			if node.CertName != "" {
+				tlsdial.SetConfigExpectedCert(tlsConf, node.CertName)
+			}
 		}
 	}
 	tlsConf.NextProtos = []string{"http/1.1"}

derp/derphttp/derphttp_test.go

@@ -8,6 +8,7 @@ import (
 	"bytes"
 	"context"
 	"crypto/tls"
+	"crypto/x509"
 	"net"
 	"net/http"
 	"net/http/httptest"
@@ -17,6 +18,7 @@ import (
 
 	"github.com/coder/websocket"
 	"tailscale.com/derp"
+	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
 )
 
@@ -324,3 +326,101 @@ func TestForceWebsockets(t *testing.T) {
 
 	c.Close()
 }
+
+func TestTLSConfigBypassesTLSDial(t *testing.T) {
+	node := &tailcfg.DERPNode{HostName: "derp.example.com"}
+
+	t.Run("default path wraps via tlsdial.Config", func(t *testing.T) {
+		// tlsdial.Config installs its own VerifyConnection. Confirming it
+		// runs is the easiest way to assert we did NOT bypass it. We must
+		// not pass a base config that would cause tlsdial.Config to panic.
+		c := &Client{TLSConfig: &tls.Config{RootCAs: x509.NewCertPool()}}
+		got := c.tlsConfig(node)
+		if got.VerifyConnection == nil {
+			t.Fatal("expected default path to install tlsdial's VerifyConnection")
+		}
+		if !got.InsecureSkipVerify {
+			t.Fatal("expected default path to set InsecureSkipVerify (tlsdial does its own verify)")
+		}
+		if got.ServerName != node.HostName {
+			t.Fatalf("ServerName: got %q, want %q", got.ServerName, node.HostName)
+		}
+	})
+
+	t.Run("bypass path uses caller config as-is", func(t *testing.T) {
+		// A typical "I'm bringing my own verifier" config that would make
+		// tlsdial.Config panic.
+		var verifyCalls int
+		base := &tls.Config{
+			InsecureSkipVerify: true,
+			VerifyPeerCertificate: func(_ [][]byte, _ [][]*x509.Certificate) error {
+				verifyCalls++
+				return nil
+			},
+		}
+		c := &Client{TLSConfig: base, TLSConfigBypassesTLSDial: true}
+		got := c.tlsConfig(node)
+		if got == base {
+			t.Fatal("expected tlsConfig to clone the base config")
+		}
+		if got.VerifyConnection != nil {
+			t.Fatal("bypass path must not install tlsdial's VerifyConnection")
+		}
+		if !got.InsecureSkipVerify {
+			t.Fatal("bypass path must preserve caller's InsecureSkipVerify")
+		}
+		if got.VerifyPeerCertificate == nil {
+			t.Fatal("bypass path must preserve caller's VerifyPeerCertificate")
+		}
+		if got.ServerName != node.HostName {
+			t.Fatalf("ServerName fallback: got %q, want %q", got.ServerName, node.HostName)
+		}
+		if want := []string{"http/1.1"}; len(got.NextProtos) != 1 || got.NextProtos[0] != want[0] {
+			t.Fatalf("NextProtos: got %v, want %v", got.NextProtos, want)
+		}
+	})
+
+	t.Run("bypass path preserves caller-set ServerName", func(t *testing.T) {
+		base := &tls.Config{
+			InsecureSkipVerify: true,
+			VerifyPeerCertificate: func(_ [][]byte, _ [][]*x509.Certificate) error {
+				return nil
+			},
+			ServerName: "explicit.example.com",
+		}
+		c := &Client{TLSConfig: base, TLSConfigBypassesTLSDial: true}
+		got := c.tlsConfig(node)
+		if got.ServerName != "explicit.example.com" {
+			t.Fatalf("ServerName: got %q, want explicit.example.com", got.ServerName)
+		}
+	})
+
+	t.Run("bypass flag without TLSConfig falls back to default path", func(t *testing.T) {
+		c := &Client{TLSConfigBypassesTLSDial: true}
+		got := c.tlsConfig(node)
+		if got.VerifyConnection == nil {
+			t.Fatal("expected fallback to tlsdial path when TLSConfig is nil")
+		}
+	})
+
+	t.Run("bypass path honors node.InsecureForTests", func(t *testing.T) {
+		base := &tls.Config{
+			InsecureSkipVerify: true,
+			VerifyPeerCertificate: func(_ [][]byte, _ [][]*x509.Certificate) error {
+				return nil
+			},
+		}
+		c := &Client{TLSConfig: base, TLSConfigBypassesTLSDial: true}
+		insecureNode := &tailcfg.DERPNode{HostName: "derp.example.com", InsecureForTests: true}
+		got := c.tlsConfig(insecureNode)
+		if !got.InsecureSkipVerify {
+			t.Fatal("expected InsecureForTests to keep InsecureSkipVerify=true")
+		}
+		if got.VerifyPeerCertificate != nil {
+			t.Fatal("expected InsecureForTests to clear VerifyPeerCertificate")
+		}
+		if got.VerifyConnection != nil {
+			t.Fatal("expected InsecureForTests to clear VerifyConnection")
+		}
+	})
+}

wgengine/magicsock/derp.go

@@ -349,6 +349,7 @@ func (c *Conn) derpWriteChanOfAddr(addr netip.AddrPort, peer key.NodePublic) cha
 	dc.DNSCache = dnscache.Get()
 	if tlsCfg := c.derpTLSConfig.Load(); tlsCfg != nil {
 		dc.TLSConfig = tlsCfg
+		dc.TLSConfigBypassesTLSDial = c.derpTLSConfigBypassesTLSDial.Load()
 	}
 	header := c.derpHeader.Load()
 	if header != nil {

wgengine/magicsock/magicsock.go

@@ -171,6 +171,11 @@ type Conn struct {
 	// headers that are passed to the DERP HTTP client
 	derpHeader atomic.Pointer[http.Header]
 
+	// derpTLSConfigBypassesTLSDial mirrors derphttp.Client.TLSConfigBypassesTLSDial.
+	// When true (and derpTLSConfig is non-nil), the DERP client uses the
+	// supplied TLSConfig as-is rather than wrapping it via tlsdial.Config.
+	derpTLSConfigBypassesTLSDial atomic.Bool
+
 	// whether websocket is always used by the DERP HTTP client
 	derpForceWebsockets atomic.Bool
 
@@ -1767,6 +1772,13 @@ func (c *Conn) SetDERPTLSConfig(cfg *tls.Config) {
 	c.derpTLSConfig.Store(cfg)
 }
 
+// SetDERPTLSConfigBypassesTLSDial sets whether the DERP client should use the
+// configured TLS config as-is instead of wrapping it via tlsdial.Config.
+// See derphttp.Client.TLSConfigBypassesTLSDial.
+func (c *Conn) SetDERPTLSConfigBypassesTLSDial(v bool) {
+	c.derpTLSConfigBypassesTLSDial.Store(v)
+}
+
 func (c *Conn) SetDERPRegionDialer(dialer func(ctx context.Context, region *tailcfg.DERPRegion) net.Conn) {
 	c.derpRegionDialer.Store(&dialer)
 	c.mu.Lock()